Merge pull request #7248 from marmelab/fix-cbgi

Fix prevent `CheckboxGroupInput` from changing selected values type
marmelab · Feb 16, 2022 · d1f36bd · d1f36bd
2 parents fd8ef16 + e5f4246
commit d1f36bd
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 6 deletions.
diff --git a/packages/ra-ui-materialui/src/input/CheckboxGroupInput.spec.tsx b/packages/ra-ui-materialui/src/input/CheckboxGroupInput.spec.tsx
@@ -217,6 +217,66 @@ describe('<CheckboxGroupInput />', () => {
         expect(queryByText('Can I help you?')).not.toBeNull();
     });
 
+    it('should not parse selected values types to numbers if all choices types are non numbers', () => {
+        const handleSubmit = jest.fn();
+        const { getByLabelText } = render(
+            <Form
+                onSubmit={handleSubmit}
+                initialValues={{ notifications: ['31', '42'] }}
+                render={({ handleSubmit }) => (
+                    <form onSubmit={handleSubmit}>
+                        <CheckboxGroupInput
+                            source="notifications"
+                            choices={[
+                                { id: '12', name: 'Ray Hakt' },
+                                { id: '31', name: 'Ann Gullar' },
+                                { id: '42', name: 'Sean Phonee' },
+                            ]}
+                        />
+                        <button type="submit" aria-label="Save" />
+                    </form>
+                )}
+            />
+        );
+        const input = getByLabelText('Ray Hakt') as HTMLInputElement;
+        fireEvent.click(input);
+
+        fireEvent.click(getByLabelText('Save'));
+        expect(handleSubmit.mock.calls[0][0]).toEqual({
+            notifications: ['31', '42', '12'],
+        });
+    });
+
+    it('should parse selected values types to numbers if some choices are numbers', () => {
+        const handleSubmit = jest.fn();
+        const { getByLabelText } = render(
+            <Form
+                onSubmit={handleSubmit}
+                initialValues={{ notifications: [31, 42] }}
+                render={({ handleSubmit }) => (
+                    <form onSubmit={handleSubmit}>
+                        <CheckboxGroupInput
+                            source="notifications"
+                            choices={[
+                                { id: 12, name: 'Ray Hakt' },
+                                { id: 31, name: 'Ann Gullar' },
+                                { id: 42, name: 'Sean Phonee' },
+                            ]}
+                        />
+                        <button type="submit" aria-label="Save" />
+                    </form>
+                )}
+            />
+        );
+        const input = getByLabelText('Ray Hakt') as HTMLInputElement;
+        fireEvent.click(input);
+
+        fireEvent.click(getByLabelText('Save'));
+        expect(handleSubmit.mock.calls[0][0]).toEqual({
+            notifications: [31, 42, 12],
+        });
+    });
+
     describe('error message', () => {
         it('should not be displayed if field is pristine', () => {
             const { container } = render(

diff --git a/packages/ra-ui-materialui/src/input/CheckboxGroupInput.tsx b/packages/ra-ui-materialui/src/input/CheckboxGroupInput.tsx
@@ -138,21 +138,31 @@ const CheckboxGroupInput: FunctionComponent<CheckboxGroupInputProps> = props =>
     const handleCheck = useCallback(
         (event, isChecked) => {
             let newValue;
-            try {
-                // try to convert string value to number, e.g. '123'
-                newValue = JSON.parse(event.target.value);
-            } catch (e) {
-                // impossible to convert value, e.g. 'abc'
+
+            if (
+                choices.every(
+                    item => typeof get(item, optionValue) === 'number'
+                )
+            ) {
+                try {
+                    // try to convert string value to number, e.g. '123'
+                    newValue = JSON.parse(event.target.value);
+                } catch (e) {
+                    // impossible to convert value, e.g. 'abc'
+                    newValue = event.target.value;
+                }
+            } else {
                 newValue = event.target.value;
             }
+
             if (isChecked) {
                 finalFormOnChange([...(value || []), ...[newValue]]);
             } else {
                 finalFormOnChange(value.filter(v => v != newValue)); // eslint-disable-line eqeqeq
             }
             finalFormOnBlur(); // HACK: See https://github.com/final-form/react-final-form/issues/365#issuecomment-515045503
         },
-        [finalFormOnChange, finalFormOnBlur, value]
+        [choices, finalFormOnBlur, optionValue, finalFormOnChange, value]
     );
 
     if (loading) {