Merge pull request #9191 from marmelab/fix-validation-translations-when-using-resolvers

fzaninotto · web-flow · commit 316114975708 · 2023-08-21T09:35:37.000+02:00
Fix validation errors from resolvers are not translated
diff --git a/packages/ra-core/package.json b/packages/ra-core/package.json
@@ -26,7 +26,7 @@
         "watch": "tsc --outDir dist/esm --module es2015 --watch"
     },
     "devDependencies": {
-        "@hookform/resolvers": "^2.8.8",
+        "@hookform/resolvers": "^3.2.0",
         "@testing-library/react": "^11.2.3",
         "@testing-library/react-hooks": "^7.0.2",
         "@types/jest": "^29.5.2",
@@ -46,7 +46,8 @@
         "recharts": "^2.1.15",
         "rimraf": "^3.0.2",
         "typescript": "^5.1.3",
-        "yup": "^0.32.11"
+        "yup": "^0.32.11",
+        "zod": "^3.22.1"
     },
     "peerDependencies": {
         "history": "^5.1.0",
diff --git a/packages/ra-core/src/form/Form.spec.tsx b/packages/ra-core/src/form/Form.spec.tsx
@@ -4,14 +4,22 @@ import { useFormState, useFormContext } from 'react-hook-form';
 import { yupResolver } from '@hookform/resolvers/yup';
 import * as yup from 'yup';
 import assert from 'assert';
+import polyglotI18nProvider from 'ra-i18n-polyglot';
+import englishMessages from 'ra-language-english';
 
 import { CoreAdminContext } from '../core';
 import { Form } from './Form';
 import { useNotificationContext } from '../notification';
 import { useInput } from './useInput';
 import { required } from './validate';
-import { SanitizeEmptyValues } from './Form.stories';
-import { NullValue } from './Form.stories';
+import {
+    FormLevelValidation,
+    InputLevelValidation,
+    ZodResolver,
+    SanitizeEmptyValues,
+    NullValue,
+} from './Form.stories';
+import { mergeTranslations } from '../i18n';
 
 describe('Form', () => {
     const Input = props => {
@@ -661,4 +669,65 @@ describe('Form', () => {
             expect(validate).toHaveBeenCalled();
         });
     });
+
+    const i18nProvider = polyglotI18nProvider(() =>
+        mergeTranslations(englishMessages, {
+            app: {
+                validation: { required: 'This field must be provided' },
+            },
+        })
+    );
+    it('should support validation messages translations at the form level without warnings', async () => {
+        const mock = jest.spyOn(console, 'error').mockImplementation(() => {});
+        const translate = jest.spyOn(i18nProvider, 'translate');
+        render(<FormLevelValidation i18nProvider={i18nProvider} />);
+        fireEvent.click(screen.getByText('Submit'));
+        await screen.findByText('Required');
+        await screen.findByText('This field is required');
+        await screen.findByText('This field must be provided');
+        await screen.findByText('app.validation.missing');
+        expect(mock).not.toHaveBeenCalledWith(
+            expect.stringContaining('Missing translation for key:')
+        );
+        // Ensure we don't have double translations
+        expect(translate).not.toHaveBeenCalledWith('Required');
+        expect(translate).not.toHaveBeenCalledWith('This field is required');
+        mock.mockRestore();
+    });
+
+    it('should support validation messages translations at the input level without warnings', async () => {
+        const mock = jest.spyOn(console, 'error').mockImplementation(() => {});
+        const translate = jest.spyOn(i18nProvider, 'translate');
+        render(<InputLevelValidation i18nProvider={i18nProvider} />);
+        fireEvent.click(screen.getByText('Submit'));
+        await screen.findByText('Required');
+        await screen.findByText('This field is required');
+        await screen.findByText('This field must be provided');
+        await screen.findByText('app.validation.missing');
+        expect(mock).not.toHaveBeenCalledWith(
+            expect.stringContaining('Missing translation for key:')
+        );
+        // Ensure we don't have double translations
+        expect(translate).not.toHaveBeenCalledWith('Required');
+        expect(translate).not.toHaveBeenCalledWith('This field is required');
+        mock.mockRestore();
+    });
+
+    it('should support validation messages translations when using a custom resolver without warnings', async () => {
+        const mock = jest.spyOn(console, 'error').mockImplementation(() => {});
+        const translate = jest.spyOn(i18nProvider, 'translate');
+        render(<ZodResolver i18nProvider={i18nProvider} />);
+        fireEvent.click(screen.getByText('Submit'));
+        await screen.findByText('Required');
+        await screen.findByText('This field is required');
+        await screen.findByText('This field must be provided');
+        await screen.findByText('app.validation.missing');
+        expect(mock).not.toHaveBeenCalledWith(
+            expect.stringContaining('Missing translation for key:')
+        );
+        // Ensure we don't have double translations
+        expect(translate).not.toHaveBeenCalledWith('Required');
+        expect(translate).not.toHaveBeenCalledWith('This field is required');
+        mock.mockRestore();
+    });
 });
diff --git a/packages/ra-core/src/form/Form.stories.tsx b/packages/ra-core/src/form/Form.stories.tsx
@@ -4,10 +4,18 @@ import {
     UseControllerProps,
     useFormState,
 } from 'react-hook-form';
+import { zodResolver } from '@hookform/resolvers/zod';
+import * as z from 'zod';
+import polyglotI18nProvider from 'ra-i18n-polyglot';
+import englishMessages from 'ra-language-english';
 
 import { CoreAdminContext } from '../core';
 import { Form } from './Form';
 import { useInput } from './useInput';
+import { required } from './validate';
+import ValidationError from './ValidationError';
+import { mergeTranslations } from '../i18n';
+import { I18nProvider } from '../types';
 
 export default {
     title: 'ra-core/form/Form',
@@ -32,7 +40,9 @@ const Input = props => {
                 aria-invalid={fieldState.invalid}
                 {...field}
             />
-            <p>{fieldState.error?.message}</p>
+            {fieldState.error && fieldState.error.message ? (
+                <ValidationError error={fieldState.error.message} />
+            ) : null}
         </div>
     );
 };
@@ -164,3 +174,119 @@ export const UndefinedValue = () => {
         </CoreAdminContext>
     );
 };
+
+const defaultI18nProvider = polyglotI18nProvider(() =>
+    mergeTranslations(englishMessages, {
+        app: { validation: { required: 'This field must be provided' } },
+    })
+);
+
+export const FormLevelValidation = ({
+    i18nProvider = defaultI18nProvider,
+}: {
+    i18nProvider?: I18nProvider;
+}) => {
+    const [submittedData, setSubmittedData] = React.useState<any>();
+    return (
+        <CoreAdminContext i18nProvider={i18nProvider}>
+            <Form
+                onSubmit={data => setSubmittedData(data)}
+                record={{ id: 1, field1: 'bar', field6: null }}
+                validate={(values: any) => {
+                    const errors: any = {};
+                    if (!values.defaultMessage) {
+                        errors.defaultMessage = 'ra.validation.required';
+                    }
+                    if (!values.customMessage) {
+                        errors.customMessage = 'This field is required';
+                    }
+                    if (!values.customMessageTranslationKey) {
+                        errors.customMessageTranslationKey =
+                            'app.validation.required';
+                    }
+                    if (!values.missingCustomMessageTranslationKey) {
+                        errors.missingCustomMessageTranslationKey =
+                            'app.validation.missing';
+                    }
+                    return errors;
+                }}
+            >
+                <Input source="defaultMessage" />
+                <Input source="customMessage" />
+                <Input source="customMessageTranslationKey" />
+                <Input source="missingCustomMessageTranslationKey" />
+                <button type="submit">Submit</button>
+            </Form>
+            <pre>{JSON.stringify(submittedData, null, 2)}</pre>
+        </CoreAdminContext>
+    );
+};
+
+export const InputLevelValidation = ({
+    i18nProvider = defaultI18nProvider,
+}: {
+    i18nProvider?: I18nProvider;
+}) => {
+    const [submittedData, setSubmittedData] = React.useState<any>();
+    return (
+        <CoreAdminContext i18nProvider={i18nProvider}>
+            <Form
+                onSubmit={data => setSubmittedData(data)}
+                record={{ id: 1, field1: 'bar', field6: null }}
+            >
+                <Input source="defaultMessage" validate={required()} />
+                <Input
+                    source="customMessage"
+                    validate={required('This field is required')}
+                />
+                <Input
+                    source="customMessageTranslationKey"
+                    validate={required('app.validation.required')}
+                />
+                <Input
+                    source="missingCustomMessageTranslationKey"
+                    validate={required('app.validation.missing')}
+                />
+                <button type="submit">Submit</button>
+            </Form>
+            <pre>{JSON.stringify(submittedData, null, 2)}</pre>
+        </CoreAdminContext>
+    );
+};
+
+const zodSchema = z.object({
+    defaultMessage: z.string(), //.min(1),
+    customMessage: z.string({
+        required_error: 'This field is required',
+    }),
+    customMessageTranslationKey: z.string({
+        required_error: 'app.validation.required',
+    }),
+    missingCustomMessageTranslationKey: z.string({
+        required_error: 'app.validation.missing',
+    }),
+});
+
+export const ZodResolver = ({
+    i18nProvider = defaultI18nProvider,
+}: {
+    i18nProvider?: I18nProvider;
+}) => {
+    const [result, setResult] = React.useState<any>();
+    return (
+        <CoreAdminContext i18nProvider={i18nProvider}>
+            <Form
+                record={{}}
+                onSubmit={data => setResult(data)}
+                resolver={zodResolver(zodSchema)}
+            >
+                <Input source="defaultMessage" />
+                <Input source="customMessage" />
+                <Input source="customMessageTranslationKey" />
+                <Input source="missingCustomMessageTranslationKey" />
+                <button type="submit">Submit</button>
+            </Form>
+            <pre>{JSON.stringify(result, null, 2)}</pre>
+        </CoreAdminContext>
+    );
+};
diff --git a/packages/ra-core/src/form/ValidationError.spec.tsx b/packages/ra-core/src/form/ValidationError.spec.tsx
@@ -4,7 +4,9 @@ import { render } from '@testing-library/react';
 import ValidationError from './ValidationError';
 import { TestTranslationProvider } from '../i18n';
 
-const translate = jest.fn(key => key);
+const translate = jest.fn(key => {
+    return key;
+});
 
 const renderWithTranslations = content =>
     render(
diff --git a/packages/ra-core/src/form/ValidationError.tsx b/packages/ra-core/src/form/ValidationError.tsx
@@ -9,15 +9,33 @@ export interface ValidationErrorProps {
     error: ValidationErrorMessage;
 }
 
+const ValidationErrorSpecialFormatPrefix = '@@react-admin@@';
 const ValidationError = (props: ValidationErrorProps) => {
     const { error } = props;
+    let errorMessage = error;
     const translate = useTranslate();
-    if ((error as ValidationErrorMessageWithArgs).message) {
-        const { message, args } = error as ValidationErrorMessageWithArgs;
+    // react-hook-form expects errors to be plain strings but our validators can return objects
+    // that have message and args.
+    // To avoid double translation for users that validate with a schema instead of our validators
+    // we use a special format for our validators errors.
+    // The useInput hook handle the special formatting
+    if (
+        typeof error === 'string' &&
+        error.startsWith(ValidationErrorSpecialFormatPrefix)
+    ) {
+        errorMessage = JSON.parse(
+            error.substring(ValidationErrorSpecialFormatPrefix.length)
+        );
+    }
+    if ((errorMessage as ValidationErrorMessageWithArgs).message) {
+        const {
+            message,
+            args,
+        } = errorMessage as ValidationErrorMessageWithArgs;
         return <>{translate(message, { _: message, ...args })}</>;
     }
 
-    return <>{translate(error as string, { _: error })}</>;
+    return <>{translate(errorMessage as string, { _: errorMessage })}</>;
 };
 
 export default ValidationError;
diff --git a/packages/ra-core/src/form/useGetValidationErrorMessage.ts b/packages/ra-core/src/form/useGetValidationErrorMessage.ts
@@ -5,6 +5,7 @@ import {
 import { useTranslate } from '../i18n';
 
 /**
+ * @deprecated
  * This internal hook returns a function that can translate an error message.
  * It handles simple string errors and those which have a message and args.
  * Only useful if you are implementing custom inputs without leveraging our useInput hook.
diff --git a/packages/ra-core/src/form/useInput.ts b/packages/ra-core/src/form/useInput.ts
@@ -13,7 +13,6 @@ import { useRecordContext } from '../controller';
 import { composeValidators, Validator } from './validate';
 import isRequired from './isRequired';
 import { useFormGroupContext } from './useFormGroupContext';
-import { useGetValidationErrorMessage } from './useGetValidationErrorMessage';
 import { useFormGroups } from './useFormGroups';
 import { useApplyInputDefaultValues } from './useApplyInputDefaultValues';
 import { useEvent } from '../util';
@@ -43,7 +42,6 @@ export const useInput = <ValueType = any>(
     const formGroupName = useFormGroupContext();
     const formGroups = useFormGroups();
     const record = useRecordContext();
-    const getValidationErrorMessage = useGetValidationErrorMessage();
 
     useEffect(() => {
         if (!formGroups || formGroupName == null) {
@@ -74,7 +72,13 @@ export const useInput = <ValueType = any>(
                 const error = await sanitizedValidate(value, values, props);
 
                 if (!error) return true;
-                return getValidationErrorMessage(error);
+                // react-hook-form expects errors to be plain strings but our validators can return objects
+                // that have message and args.
+                // To avoid double translation for users that validate with a schema instead of our validators
+                // we use a special format for our validators errors.
+                // The ValidationError component will check for this format and extract the message and args
+                // to translate.
+                return `@@react-admin@@${JSON.stringify(error)}`;
             },
         },
         ...options,
diff --git a/packages/ra-core/src/form/useUnique.stories.tsx b/packages/ra-core/src/form/useUnique.stories.tsx
@@ -10,6 +10,7 @@ import {
     DataProvider,
     EditBase,
     FormDataConsumer,
+    ValidationError,
     mergeTranslations,
     useUnique,
 } from '..';
@@ -30,7 +31,9 @@ const Input = props => {
                 aria-invalid={fieldState.invalid}
                 {...field}
             />
-            <p>{fieldState.error?.message}</p>
+            {fieldState.error && fieldState.error?.message ? (
+                <ValidationError error={fieldState.error?.message} />
+            ) : null}
         </>
     );
 };
diff --git a/packages/ra-core/src/form/useUnique.ts b/packages/ra-core/src/form/useUnique.ts
@@ -103,16 +103,18 @@ export const useUnique = (options?: UseUniqueOptions) => {
                     );
 
                     if (total > 0 && !data.some(r => r.id === record?.id)) {
-                        return translate(message, {
-                            _: message,
-                            source: props.source,
-                            value,
-                            field: translateLabel({
-                                label: props.label,
+                        return {
+                            message,
+                            args: {
                                 source: props.source,
-                                resource,
-                            }),
-                        });
+                                value,
+                                field: translateLabel({
+                                    label: props.label,
+                                    source: props.source,
+                                    resource,
+                                }),
+                            },
+                        };
                     }
                 } catch (error) {
                     return translate('ra.notification.http_error');
diff --git a/packages/ra-core/src/i18n/TestTranslationProvider.tsx b/packages/ra-core/src/i18n/TestTranslationProvider.tsx
diff --git a/packages/ra-ui-materialui/src/input/InputHelperText.tsx b/packages/ra-ui-materialui/src/input/InputHelperText.tsx
diff --git a/packages/ra-ui-materialui/src/input/NumberInput.spec.tsx b/packages/ra-ui-materialui/src/input/NumberInput.spec.tsx
diff --git a/yarn.lock b/yarn.lock
