Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/beige-planets-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/ui-client": patch
---

Fixes a GUI crash happening in the admin user page when attempting to display an invalid custom field
35 changes: 35 additions & 0 deletions apps/meteor/tests/e2e/admin-users-custom-fields.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,39 @@ test.describe('Admin users custom fields', () => {
await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toHaveValue(adminCustomFieldValue2);
});
});

test.describe('with invalid custom field type', () => {
test.beforeAll(async ({ api }) => {
await api.post('/settings/Accounts_CustomFields', {
value: JSON.stringify({
customFieldText1: {
type: 'invalid_type',
required: false,
},
customFieldText2: {
type: 'text',
required: false,
},
}),
});
});

test('should not render fields with invalid custom field type', async () => {
await test.step('should find and click on add test user', async () => {
await poAdmin.inputSearchUsers.fill(addTestUser.data.username);

await expect(poAdmin.getUserRowByUsername(addTestUser.data.username)).toBeVisible();
await poAdmin.getUserRowByUsername(addTestUser.data.username).click();
});

await test.step('should navigate to edit user form', async () => {
await poAdmin.btnEdit.click();
});

await test.step('should verify custom field is not rendered', async () => {
await expect(poAdmin.tabs.users.getCustomField('customFieldText1')).not.toBeVisible();
await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toBeVisible();
});
});
});
});
139 changes: 139 additions & 0 deletions packages/ui-client/src/components/CustomFieldsForm.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import type { CustomFieldMetadata } from '@rocket.chat/core-typings';
import { mockAppRoot } from '@rocket.chat/mock-providers';
import { render, screen, waitFor, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { Control } from 'react-hook-form';
import { useForm } from 'react-hook-form';

import { CustomFieldsForm } from './CustomFieldsForm';

type TestComponentProps = {
metadata: CustomFieldMetadata[];
formName: string;
onSubmit: (data: any) => void;
};

const appRoot = mockAppRoot()
.withTranslations('en', 'core', {
Required_field: '{{field}} required',
})
.build();

const TestComponent = ({ metadata, formName, onSubmit }: TestComponentProps) => {
const { control, handleSubmit } = useForm({ mode: 'onBlur' });

return (
<form onSubmit={handleSubmit(onSubmit)}>
<CustomFieldsForm formName={formName} formControl={control as unknown as Control<any>} metadata={metadata} />
<button type='submit'>Submit</button>
</form>
);
};

describe('CustomFieldsForm', () => {
it('should render all custom fields', () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, defaultValue: '', options: [] },
{
name: 'field2',
type: 'select',
label: 'Field 2',
required: false,
defaultValue: 'a',
options: [
['a', 'a'],
['b', 'b'],
],
},
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />);

expect(screen.getByRole('textbox', { name: 'Field 1' })).toBeInTheDocument();
expect(within(screen.getByLabelText('Field 2')).getByRole('combobox', { hidden: true })).toBeInTheDocument();
});

it('should render a text input', () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, defaultValue: '', options: [] },
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });

expect(screen.getByRole('textbox', { name: 'Field 1' })).toBeInTheDocument();
});

it('should render a select input', () => {
const metadata: CustomFieldMetadata[] = [
{
name: 'field2',
type: 'select',
label: 'Field 2',
required: false,
defaultValue: 'a',
options: [
['a', 'a'],
['b', 'b'],
],
},
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });

expect(within(screen.getByLabelText('Field 2')).getByRole('combobox', { hidden: true })).toBeInTheDocument();
});

it('should show required error message', async () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, defaultValue: '', options: [] },
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />);

const input = screen.getByRole('textbox', { name: 'Field 1' });
await userEvent.click(input);
await userEvent.tab();

await waitFor(() => expect(input).toHaveAccessibleDescription('Field 1 required'));
});

it('should show minLength error message', async () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, minLength: 5, defaultValue: '', options: [] },
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });

const input = screen.getByRole('textbox', { name: 'Field 1' });
await userEvent.type(input, '123');
await userEvent.tab();

await waitFor(() => expect(input).toHaveAccessibleDescription('Min_length_is'));
});
Comment thread
aleksandernsilva marked this conversation as resolved.

it('should validate maxLength', async () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, maxLength: 3, defaultValue: '', options: [] },
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });

const input = screen.getByRole('textbox', { name: 'Field 1' });
await userEvent.type(input, '123456');
await userEvent.tab();

expect(input).toHaveValue('123');
});

it('should not throw when attempting to render invalid field type', () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'invalid_type' as any, label: 'Field 1', required: true, defaultValue: '', options: [] },
];

expect(() =>
render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot }),
).not.toThrow();

expect(screen.queryByLabelText('Field 1')).not.toBeInTheDocument();
});
});
25 changes: 15 additions & 10 deletions packages/ui-client/src/components/CustomFieldsForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ const CustomField = <T extends FieldValues>({
const error = get(errors, name);
const errorMessage = useMemo(() => getErrorMessage(error), [error, getErrorMessage]);

if (!Component) {
return null;
}

return (
<Controller<T, any>
name={name}
Expand All @@ -72,23 +76,25 @@ const CustomField = <T extends FieldValues>({
rules={{ minLength: props.minLength, maxLength: props.maxLength, validate: { required: validateRequired } }}
render={({ field }) => (
<Field rcx-field-group__item>
<FieldLabel htmlFor={fieldId} required={required}>
<FieldLabel is='span' id={fieldId} required={required}>
{label || t(name as TranslationKey)}
</FieldLabel>
<FieldRow>
<Component
{...props}
{...field}
id={fieldId}
aria-describedby={`${fieldId}-error`}
aria-labelledby={fieldId}
aria-describedby={errorMessage && `${fieldId}-error`}
error={errorMessage}
options={selectOptions as SelectOption[]}
flexGrow={1}
/>
</FieldRow>
<FieldError aria-live='assertive' id={`${fieldId}-error`}>
{errorMessage}
</FieldError>
{errorMessage ? (
<FieldError aria-live='assertive' id={`${fieldId}-error`}>
{errorMessage}
</FieldError>
) : null}
</Field>
)}
/>
Expand All @@ -98,9 +104,8 @@ const CustomField = <T extends FieldValues>({
// eslint-disable-next-line react/no-multi-comp
export const CustomFieldsForm = <T extends FieldValues>({ formName, formControl, metadata }: CustomFieldFormProps<T>) => (
<>
{metadata.map(({ name: fieldName, ...props }) => {
props.label = props.label ?? fieldName;
return <CustomField key={fieldName} name={`${formName}.${fieldName}`} control={formControl} {...props} />;
})}
{metadata.map(({ name: fieldName, label, ...props }) => (
<CustomField key={fieldName} name={`${formName}.${fieldName}`} control={formControl} label={label ?? fieldName} {...props} />
))}
</>
);
Loading