Skip to content

Commit

Permalink
feat(KFLUXUI-272): remove validateUsername and add warning for it
Browse files Browse the repository at this point in the history
  • Loading branch information
testcara committed Jan 17, 2025
1 parent 38a7c3b commit 7a2701e
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 80 deletions.
35 changes: 18 additions & 17 deletions src/components/UserAccess/UserAccessForm/UsernameSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { useField } from 'formik';
import HelpPopover from '../../../components/HelpPopover';
import { getFieldId } from '../../../shared/components/formik-fields/field-utils';
import { useDebounceCallback } from '../../../shared/hooks/useDebounceCallback';
import { validateUsername } from './form-utils';

import './UsernameSection.scss';

Expand All @@ -36,28 +35,25 @@ export const UsernameSection: React.FC<React.PropsWithChildren<Props>> = ({ disa
setValidating(true);
setValidHelpText('');
setError('');
// username is mandantory
if (!username) {
setValidating(false);
return;
}
// usename format should be valid
if (!usernameRegex.test(username)) {
setValidating(false);
setError('Invalid username format.');
return;
}
void validateUsername(username).then((valid) => {
setValidating(false);
if (valid) {
setError('');
setValidHelpText('Validated');
if (!usernames.includes(username)) {
void setValue([...usernames, username]);
}
setUsername('');
} else {
setError('Username not found.');
}
});
// for valid username
setValidating(false);
setError('');
setValidHelpText('Valid username format.');
if (!usernames.includes(username)) {
void setValue([...usernames, username]);
}
setUsername('');
}, [setError, setValue, username, usernames]),
);

Expand Down Expand Up @@ -111,9 +107,14 @@ export const UsernameSection: React.FC<React.PropsWithChildren<Props>> = ({ disa
{error}
</HelperTextItem>
) : validHelpText ? (
<HelperTextItem variant="success" hasIcon>
{validHelpText}
</HelperTextItem>
<>
<HelperTextItem variant="success" hasIcon>
{validHelpText}
</HelperTextItem>
<HelperTextItem variant="warning" hasIcon>
We do not validate whether the username is a valid Konflux user.
</HelperTextItem>
</>
) : (
<HelperTextItem>
Provide Konflux usernames for the users you want to invite.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { act, fireEvent, screen, waitFor } from '@testing-library/react';
import identity from 'lodash-es/identity';
import { SpaceBindingRequest, Workspace } from '../../../../types';
import { namespaceRenderer } from '../../../../utils/test-utils';
import { createSBRs, editSBR, validateUsername } from '../form-utils';
import { createSBRs, editSBR } from '../form-utils';
import { UserAccessFormPage } from '../UserAccessFormPage';

jest.mock('react-i18next', () => ({
Expand Down Expand Up @@ -36,7 +36,6 @@ jest.mock('../form-utils', () => ({

const createSBRsMock = createSBRs as jest.Mock;
const editSBRsMock = editSBR as jest.Mock;
const validateUsernameMock = validateUsername as jest.Mock;

describe('UserAccessFormPage', () => {
// beforeEach(jest.useFakeTimers);
Expand All @@ -48,7 +47,6 @@ describe('UserAccessFormPage', () => {

it('should create resources on submit', async () => {
createSBRsMock.mockResolvedValue({});
validateUsernameMock.mockResolvedValue(true);
namespaceRenderer(<UserAccessFormPage />, 'test-ns', {
workspace: 'test-ws',
workspaceResource: {} as Workspace,
Expand All @@ -68,7 +66,6 @@ describe('UserAccessFormPage', () => {
});

it('should create resources for edit when existing sbr is not available', async () => {
validateUsernameMock.mockResolvedValue(true);
namespaceRenderer(<UserAccessFormPage username="myuser" edit />, 'test-ns', {
workspace: 'test-ws',
workspaceResource: {} as Workspace,
Expand All @@ -88,7 +85,6 @@ describe('UserAccessFormPage', () => {

it('should update resources when existing sbr is provided', async () => {
editSBRsMock.mockResolvedValue({});
validateUsernameMock.mockResolvedValue(true);
const mockSBR: SpaceBindingRequest = {
apiVersion: 'appstudio.redhat.com/v1alpha1',
kind: 'SpaceBindingRequest',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import { act, fireEvent, screen, waitFor } from '@testing-library/react';
import { formikRenderer } from '../../../../utils/test-utils';
import { validateUsername } from '../form-utils';
import { UsernameSection } from '../UsernameSection';

jest.mock('../form-utils', () => ({
validateUsername: jest.fn(),
}));

const validateMock = validateUsername as jest.Mock;

describe('UsernameSection', () => {
it('should show usernames field', () => {
formikRenderer(<UsernameSection />, { usernames: [] });
Expand All @@ -18,7 +15,6 @@ describe('UsernameSection', () => {
});

it('should add username chip when entered', async () => {
validateMock.mockResolvedValue(true);
formikRenderer(<UsernameSection />, { usernames: [] });
await act(() => fireEvent.input(screen.getByRole('searchbox'), { target: { value: 'user1' } }));
await waitFor(() =>
Expand All @@ -29,14 +25,12 @@ describe('UsernameSection', () => {
await act(() => fireEvent.click(screen.getByRole('button', { name: 'Remove user1' })));
expect(screen.queryByText('user1')).not.toBeInTheDocument();

validateMock.mockResolvedValue(true);
await act(() => fireEvent.input(screen.getByRole('searchbox'), { target: { value: 'user2' } }));
await waitFor(() => expect(screen.getByText('user2')).toBeVisible());
expect(screen.getByText('user2')).toBeVisible();
});

it('should show correct field status while entering', async () => {
validateMock.mockResolvedValue(false);
formikRenderer(<UsernameSection />, { usernames: [] });
expect(
screen.getByText('Provide Konflux usernames for the users you want to invite.'),
Expand All @@ -51,7 +45,6 @@ describe('UsernameSection', () => {
);
await waitFor(() => expect(screen.getByText('Username not found.')).toBeVisible());

validateMock.mockResolvedValue(true);
await act(() => fireEvent.input(screen.getByRole('searchbox'), { target: { value: 'user1' } }));
await waitFor(() => expect(screen.getByText('Validated')).toBeVisible());
await waitFor(() =>
Expand All @@ -61,7 +54,6 @@ describe('UsernameSection', () => {
});

it('should not add username again if entry already exists', async () => {
validateMock.mockResolvedValue(true);
formikRenderer(<UsernameSection />, { usernames: [] });
await act(() => fireEvent.input(screen.getByRole('searchbox'), { target: { value: 'user1' } }));
await waitFor(() =>
Expand All @@ -74,19 +66,23 @@ describe('UsernameSection', () => {
});

it('should validate username format', async () => {
validateMock.mockResolvedValue(true);
formikRenderer(<UsernameSection />, { usernames: [] });
await act(() =>
fireEvent.input(screen.getByRole('searchbox'), { target: { value: 'user-12.3' } }),
);
await waitFor(() => expect(screen.getByText('Validated')).toBeVisible());
await waitFor(() => expect(screen.getByText('Valid username format.')).toBeVisible());
await waitFor(() =>
expect(
screen.getByText('We do not validate whether the username is a valid Konflux user.'),
).toBeVisible(),
);

await act(() =>
fireEvent.input(screen.getByRole('searchbox'), { target: { value: 'user1!@#' } }),
);
await waitFor(() => expect(screen.getByText('Invalid username format.')).toBeVisible());

await act(() => fireEvent.input(screen.getByRole('searchbox'), { target: { value: '1test' } }));
await waitFor(() => expect(screen.getByText('Validated')).toBeVisible());
await waitFor(() => expect(screen.getByText('Valid username format.')).toBeVisible());
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import '@testing-library/jest-dom';
import { createK8sUtilMock } from '../../../../utils/test-utils';
import { createSBRs, editSBR, validateUsername } from '../form-utils';
import { createSBRs, editSBR } from '../form-utils';

const k8sCreateMock = createK8sUtilMock('K8sQueryCreateResource');
const k8sPatchMock = createK8sUtilMock('K8sQueryPatchResource');
Expand Down Expand Up @@ -52,39 +52,3 @@ describe('editSBR', () => {
);
});
});

describe('validateUsername', () => {
const mockFetch = jest.fn();
beforeAll(() => {
window.fetch = mockFetch;
});

it('should return true if username is valid', async () => {
mockFetch.mockResolvedValue({
json: () => [
{
username: 'user1',
},
],
});
const result = await validateUsername('user1');
expect(mockFetch).toHaveBeenCalledWith('/api/k8s/registration/api/v1/usernames/user1');
expect(result).toBe(true);
});

it('should return false if returned username is not the same', async () => {
mockFetch.mockResolvedValue({
json: () => {
'user';
},
});
const result = await validateUsername('user1');
expect(result).toBe(false);
});

it('should return false if api call fails', async () => {
mockFetch.mockRejectedValue(null);
const result = await validateUsername('user1');
expect(result).toBe(false);
});
});
10 changes: 0 additions & 10 deletions src/components/UserAccess/UserAccessForm/form-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,6 @@ export type UserAccessFormValues = {
role: WorkspaceRole;
};

export const validateUsername = async (username: string) => {
try {
const res = await fetch(`/api/k8s/registration/api/v1/usernames/${username}`);
const [data]: [{ username: string }] = await res.json();
return data.username === username;
} catch {
return false;
}
};

export const userAccessFormSchema = yup.object({
usernames: yup
.array()
Expand Down

0 comments on commit 7a2701e

Please sign in to comment.