Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix validation errors from resolvers are not translated #9191

Merged
merged 9 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 3 additions & 2 deletions packages/ra-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
73 changes: 71 additions & 2 deletions packages/ra-core/src/form/Form.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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();
});
});
128 changes: 127 additions & 1 deletion packages/ra-core/src/form/Form.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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>
);
};
Expand Down Expand Up @@ -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>
);
};
4 changes: 3 additions & 1 deletion packages/ra-core/src/form/ValidationError.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
24 changes: 21 additions & 3 deletions packages/ra-core/src/form/ValidationError.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
1 change: 1 addition & 0 deletions packages/ra-core/src/form/useGetValidationErrorMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 7 additions & 3 deletions packages/ra-core/src/form/useInput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion packages/ra-core/src/form/useUnique.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
DataProvider,
EditBase,
FormDataConsumer,
ValidationError,
mergeTranslations,
useUnique,
} from '..';
Expand All @@ -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}
</>
);
};
Expand Down
20 changes: 11 additions & 9 deletions packages/ra-core/src/form/useUnique.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Loading