Skip to content

Commit 085d653

Browse files
aleksandernsilvaaleksandernsilva
authored
fix: User admin GUI crash when rendering an invalid custom field type (RocketChat#37154)
1 parent e64c9a8 commit 085d653

4 files changed

Lines changed: 195 additions & 10 deletions

File tree

‎.changeset/beige-planets-suffer.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@rocket.chat/meteor": patch
3+
"@rocket.chat/ui-client": patch
4+
---
5+
6+
Fixes a GUI crash happening in the admin user page when attempting to display an invalid custom field

‎apps/meteor/tests/e2e/admin-users-custom-fields.spec.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,4 +125,39 @@ test.describe('Admin users custom fields', () => {
125125
await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toHaveValue(adminCustomFieldValue2);
126126
});
127127
});
128+
129+
test.describe('with invalid custom field type', () => {
130+
test.beforeAll(async ({ api }) => {
131+
await api.post('/settings/Accounts_CustomFields', {
132+
value: JSON.stringify({
133+
customFieldText1: {
134+
type: 'invalid_type',
135+
required: false,
136+
},
137+
customFieldText2: {
138+
type: 'text',
139+
required: false,
140+
},
141+
}),
142+
});
143+
});
144+
145+
test('should not render fields with invalid custom field type', async () => {
146+
await test.step('should find and click on add test user', async () => {
147+
await poAdmin.inputSearchUsers.fill(addTestUser.data.username);
148+
149+
await expect(poAdmin.getUserRowByUsername(addTestUser.data.username)).toBeVisible();
150+
await poAdmin.getUserRowByUsername(addTestUser.data.username).click();
151+
});
152+
153+
await test.step('should navigate to edit user form', async () => {
154+
await poAdmin.btnEdit.click();
155+
});
156+
157+
await test.step('should verify custom field is not rendered', async () => {
158+
await expect(poAdmin.tabs.users.getCustomField('customFieldText1')).not.toBeVisible();
159+
await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toBeVisible();
160+
});
161+
});
162+
});
128163
});

‎packages/ui-client/src/components/CustomFieldsForm.spec.tsx‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import type { CustomFieldMetadata } from '@rocket.chat/core-typings';
2+
import { mockAppRoot } from '@rocket.chat/mock-providers';
3+
import { render, screen, waitFor, within } from '@testing-library/react';
4+
import userEvent from '@testing-library/user-event';
5+
import type { Control } from 'react-hook-form';
6+
import { useForm } from 'react-hook-form';
7+
8+
import { CustomFieldsForm } from './CustomFieldsForm';
9+
10+
type TestComponentProps = {
11+
metadata: CustomFieldMetadata[];
12+
formName: string;
13+
onSubmit: (data: any) => void;
14+
};
15+
16+
const appRoot = mockAppRoot()
17+
.withTranslations('en', 'core', {
18+
Required_field: '{{field}} required',
19+
})
20+
.build();
21+
22+
const TestComponent = ({ metadata, formName, onSubmit }: TestComponentProps) => {
23+
const { control, handleSubmit } = useForm({ mode: 'onBlur' });
24+
25+
return (
26+
<form onSubmit={handleSubmit(onSubmit)}>
27+
<CustomFieldsForm formName={formName} formControl={control as unknown as Control<any>} metadata={metadata} />
28+
<button type='submit'>Submit</button>
29+
</form>
30+
);
31+
};
32+
33+
describe('CustomFieldsForm', () => {
34+
it('should render all custom fields', () => {
35+
const metadata: CustomFieldMetadata[] = [
36+
{ name: 'field1', type: 'text', label: 'Field 1', required: true, defaultValue: '', options: [] },
37+
{
38+
name: 'field2',
39+
type: 'select',
40+
label: 'Field 2',
41+
required: false,
42+
defaultValue: 'a',
43+
options: [
44+
['a', 'a'],
45+
['b', 'b'],
46+
],
47+
},
48+
];
49+
50+
render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />);
51+
52+
expect(screen.getByRole('textbox', { name: 'Field 1' })).toBeInTheDocument();
53+
expect(within(screen.getByLabelText('Field 2')).getByRole('combobox', { hidden: true })).toBeInTheDocument();
54+
});
55+
56+
it('should render a text input', () => {
57+
const metadata: CustomFieldMetadata[] = [
58+
{ name: 'field1', type: 'text', label: 'Field 1', required: true, defaultValue: '', options: [] },
59+
];
60+
61+
render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });
62+
63+
expect(screen.getByRole('textbox', { name: 'Field 1' })).toBeInTheDocument();
64+
});
65+
66+
it('should render a select input', () => {
67+
const metadata: CustomFieldMetadata[] = [
68+
{
69+
name: 'field2',
70+
type: 'select',
71+
label: 'Field 2',
72+
required: false,
73+
defaultValue: 'a',
74+
options: [
75+
['a', 'a'],
76+
['b', 'b'],
77+
],
78+
},
79+
];
80+
81+
render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });
82+
83+
expect(within(screen.getByLabelText('Field 2')).getByRole('combobox', { hidden: true })).toBeInTheDocument();
84+
});
85+
86+
it('should show required error message', async () => {
87+
const metadata: CustomFieldMetadata[] = [
88+
{ name: 'field1', type: 'text', label: 'Field 1', required: true, defaultValue: '', options: [] },
89+
];
90+
91+
render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />);
92+
93+
const input = screen.getByRole('textbox', { name: 'Field 1' });
94+
await userEvent.click(input);
95+
await userEvent.tab();
96+
97+
await waitFor(() => expect(input).toHaveAccessibleDescription('Field 1 required'));
98+
});
99+
100+
it('should show minLength error message', async () => {
101+
const metadata: CustomFieldMetadata[] = [
102+
{ name: 'field1', type: 'text', label: 'Field 1', required: true, minLength: 5, defaultValue: '', options: [] },
103+
];
104+
105+
render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });
106+
107+
const input = screen.getByRole('textbox', { name: 'Field 1' });
108+
await userEvent.type(input, '123');
109+
await userEvent.tab();
110+
111+
await waitFor(() => expect(input).toHaveAccessibleDescription('Min_length_is'));
112+
});
113+
114+
it('should validate maxLength', async () => {
115+
const metadata: CustomFieldMetadata[] = [
116+
{ name: 'field1', type: 'text', label: 'Field 1', required: true, maxLength: 3, defaultValue: '', options: [] },
117+
];
118+
119+
render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });
120+
121+
const input = screen.getByRole('textbox', { name: 'Field 1' });
122+
await userEvent.type(input, '123456');
123+
await userEvent.tab();
124+
125+
expect(input).toHaveValue('123');
126+
});
127+
128+
it('should not throw when attempting to render invalid field type', () => {
129+
const metadata: CustomFieldMetadata[] = [
130+
{ name: 'field1', type: 'invalid_type' as any, label: 'Field 1', required: true, defaultValue: '', options: [] },
131+
];
132+
133+
expect(() =>
134+
render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot }),
135+
).not.toThrow();
136+
137+
expect(screen.queryByLabelText('Field 1')).not.toBeInTheDocument();
138+
});
139+
});

‎packages/ui-client/src/components/CustomFieldsForm.tsx‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ const CustomField = <T extends FieldValues>({
6464
const error = get(errors, name);
6565
const errorMessage = useMemo(() => getErrorMessage(error), [error, getErrorMessage]);
6666

67+
if (!Component) {
68+
return null;
69+
}
70+
6771
return (
6872
<Controller<T, any>
6973
name={name}
@@ -72,23 +76,25 @@ const CustomField = <T extends FieldValues>({
7276
rules={{ minLength: props.minLength, maxLength: props.maxLength, validate: { required: validateRequired } }}
7377
render={({ field }) => (
7478
<Field rcx-field-group__item>
75-
<FieldLabel htmlFor={fieldId} required={required}>
79+
<FieldLabel is='span' id={fieldId} required={required}>
7680
{label || t(name as TranslationKey)}
7781
</FieldLabel>
7882
<FieldRow>
7983
<Component
8084
{...props}
8185
{...field}
82-
id={fieldId}
83-
aria-describedby={`${fieldId}-error`}
86+
aria-labelledby={fieldId}
87+
aria-describedby={errorMessage && `${fieldId}-error`}
8488
error={errorMessage}
8589
options={selectOptions as SelectOption[]}
8690
flexGrow={1}
8791
/>
8892
</FieldRow>
89-
<FieldError aria-live='assertive' id={`${fieldId}-error`}>
90-
{errorMessage}
91-
</FieldError>
93+
{errorMessage ? (
94+
<FieldError aria-live='assertive' id={`${fieldId}-error`}>
95+
{errorMessage}
96+
</FieldError>
97+
) : null}
9298
</Field>
9399
)}
94100
/>
@@ -98,9 +104,8 @@ const CustomField = <T extends FieldValues>({
98104
// eslint-disable-next-line react/no-multi-comp
99105
export const CustomFieldsForm = <T extends FieldValues>({ formName, formControl, metadata }: CustomFieldFormProps<T>) => (
100106
<>
101-
{metadata.map(({ name: fieldName, ...props }) => {
102-
props.label = props.label ?? fieldName;
103-
return <CustomField key={fieldName} name={`${formName}.${fieldName}`} control={formControl} {...props} />;
104-
})}
107+
{metadata.map(({ name: fieldName, label, ...props }) => (
108+
<CustomField key={fieldName} name={`${formName}.${fieldName}`} control={formControl} label={label ?? fieldName} {...props} />
109+
))}
105110
</>
106111
);

0 commit comments

Comments
 (0)