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 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
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
78 changes: 76 additions & 2 deletions packages/ra-core/src/form/Form.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ 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';

describe('Form', () => {
const Input = props => {
Expand Down Expand Up @@ -661,4 +666,73 @@ describe('Form', () => {
expect(validate).toHaveBeenCalled();
});
});

it('should support validation messages translations at the form level without warnings', async () => {
const mock = jest.spyOn(console, 'error').mockImplementation(() => {});
render(<FormLevelValidation />);
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(
'Missing translation for key: "ra.validation.required"'
);
expect(mock).not.toHaveBeenCalledWith(
'Missing translation for key: "app.validation.required"'
);
expect(mock).toHaveBeenCalledWith(
'Warning: Missing translation for key: "This field is required"'
);
expect(mock).toHaveBeenCalledWith(
'Warning: Missing translation for key: "app.validation.missing"'
);
mock.mockRestore();
});

it('should support validation messages translations at the input level without warnings', async () => {
const mock = jest.spyOn(console, 'error').mockImplementation(() => {});
render(<InputLevelValidation />);
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(
'Missing translation for key: "ra.validation.required"'
);
expect(mock).not.toHaveBeenCalledWith(
'Missing translation for key: "app.validation.required"'
);
expect(mock).toHaveBeenCalledWith(
'Warning: Missing translation for key: "This field is required"'
);
expect(mock).toHaveBeenCalledWith(
'Warning: Missing translation for key: "app.validation.missing"'
);
mock.mockRestore();
});

it('should support validation messages translations when using a custom resolver without warnings', async () => {
const mock = jest.spyOn(console, 'error').mockImplementation(() => {});
render(<ZodResolver />);
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(
'Missing translation for key: "ra.validation.required"'
);
expect(mock).not.toHaveBeenCalledWith(
'Missing translation for key: "app.validation.required"'
);
expect(mock).toHaveBeenCalledWith(
'Warning: Missing translation for key: "This field is required"'
);
expect(mock).toHaveBeenCalledWith(
'Warning: Missing translation for key: "app.validation.missing"'
);
mock.mockRestore();
});
});
115 changes: 114 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,17 @@ 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';

export default {
title: 'ra-core/form/Form',
Expand All @@ -32,7 +39,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 +173,107 @@ export const UndefinedValue = () => {
</CoreAdminContext>
);
};

const i18nProvider = polyglotI18nProvider(() =>
mergeTranslations(englishMessages, {
app: { validation: { required: 'This field must be provided' } },
})
);

export const FormLevelValidation = () => {
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 = () => {
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 = () => {
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>
);
};
26 changes: 22 additions & 4 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;
return <>{translate(message, { _: message, ...args })}</>;
// 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, args)}</>;
}

return <>{translate(error as string, { _: error })}</>;
return <>{translate(errorMessage as string)}</>;
};

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();
djhi marked this conversation as resolved.
Show resolved Hide resolved

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
12 changes: 2 additions & 10 deletions packages/ra-ui-materialui/src/input/InputHelperText.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,13 @@
import * as React from 'react';
import { isValidElement, ReactElement } from 'react';
import {
useTranslate,
ValidationError,
ValidationErrorMessage,
ValidationErrorMessageWithArgs,
} from 'ra-core';
import { useTranslate, ValidationError, ValidationErrorMessage } from 'ra-core';

export const InputHelperText = (props: InputHelperTextProps) => {
const { helperText, touched, error } = props;
const translate = useTranslate();

if (touched && error) {
if ((error as ValidationErrorMessageWithArgs).message) {
return <ValidationError error={error} />;
}
return <>{error}</>;
return <ValidationError error={error} />;
}

if (helperText === false) {
Expand Down