Skip to content

Commit

Permalink
fix: trim name and description before validation (#8275)
Browse files Browse the repository at this point in the history
This fixes a bug where you can input just whitespace for
name/description. It also means that you can no longer have both "my
role" and "my role    " as separate roles.

API fix will follow.
  • Loading branch information
thomasheartman authored Sep 26, 2024
1 parent eb01b44 commit e20ef56
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 7 deletions.
4 changes: 2 additions & 2 deletions frontend/src/component/admin/roles/RoleForm/RoleForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ const StyledInputFullWidth = styled(Input)({
interface IRoleFormProps {
type?: PredefinedRoleType;
name: string;
setName: React.Dispatch<React.SetStateAction<string>>;
setName: (name: string) => void;
validateName: (name: string) => boolean;
description: string;
setDescription: React.Dispatch<React.SetStateAction<string>>;
setDescription: (description: string) => void;
validateDescription: (description: string) => boolean;
checkedPermissions: ICheckedPermissions;
setCheckedPermissions: React.Dispatch<
Expand Down
50 changes: 50 additions & 0 deletions frontend/src/component/admin/roles/RoleForm/useRoleForm.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { renderHook } from '@testing-library/react';
import { useRoleForm } from './useRoleForm';
import { act } from 'react-test-renderer';
import { test } from 'vitest';

describe('trim names and description', () => {
test('name is trimmed before being set', () => {
const { result } = renderHook(() => useRoleForm());

act(() => {
result.current.setName(' my role ');
});

expect(result.current.name).toBe('my role');
});

test('description is trimmed before being set', () => {
const { result } = renderHook(() => useRoleForm());

act(() => {
result.current.setDescription(' my description ');
});

expect(result.current.description).toBe('my description');
});

test('name that is just whitespace triggers an error', () => {
const { result } = renderHook(() => useRoleForm());

act(() => {
result.current.validateName(' ');
});

expect(result.current.errors).toMatchObject({
name: 'Name is required.',
});
});

test('description that is just whitespace triggers an error', () => {
const { result } = renderHook(() => useRoleForm());

act(() => {
result.current.validateDescription(' ');
});

expect(result.current.errors).toMatchObject({
description: 'Description is required.',
});
});
});
15 changes: 10 additions & 5 deletions frontend/src/component/admin/roles/RoleForm/useRoleForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ export const useRoleForm = (
const { roles, projectRoles } = useRoles();

const [name, setName] = useState(initialName);
const setTrimmedName = (newName: string) => setName(newName.trim());
const [description, setDescription] = useState(initialDescription);
const setTrimmedDescription = (newDescription: string) =>
setDescription(newDescription.trim());
const [checkedPermissions, setCheckedPermissions] =
useState<ICheckedPermissions>({});
const [errors, setErrors] = useState<IRoleFormErrors>(DEFAULT_ERRORS);
Expand Down Expand Up @@ -78,12 +81,13 @@ export const useRoleForm = (
};

const validateName = (name: string) => {
if (!isNotEmpty(name)) {
const trimmedName = name.trim();
if (!isNotEmpty(trimmedName)) {
setError(ErrorField.NAME, 'Name is required.');
return false;
}

if (!isNameUnique(name)) {
if (!isNameUnique(trimmedName)) {
setError(ErrorField.NAME, 'Name must be unique.');
return false;
}
Expand All @@ -93,7 +97,8 @@ export const useRoleForm = (
};

const validateDescription = (description: string) => {
if (!isNotEmpty(description)) {
const trimmedDescription = description.trim();
if (!isNotEmpty(trimmedDescription)) {
setError(ErrorField.DESCRIPTION, 'Description is required.');
return false;
}
Expand Down Expand Up @@ -139,10 +144,10 @@ export const useRoleForm = (

return {
name,
setName,
setName: setTrimmedName,
validateName,
description,
setDescription,
setDescription: setTrimmedDescription,
validateDescription,
checkedPermissions,
setCheckedPermissions,
Expand Down

0 comments on commit e20ef56

Please sign in to comment.