diff --git a/docs/Upgrade.md b/docs/Upgrade.md index 4c76568f36d..de7303709fa 100644 --- a/docs/Upgrade.md +++ b/docs/Upgrade.md @@ -44,6 +44,7 @@ React-admin v5 mostly focuses on removing deprecated features and upgrading depe - [`warnWhenUnsavedChanges` Changes](#warnwhenunsavedchanges-changes) - [Inputs No Longer Require To Be Touched To Display A Validation Error](#inputs-no-longer-require-to-be-touched-to-display-a-validation-error) - [`` Prop Was Removed](#inputhelpertext-touched-prop-was-removed) + - [Global Server Side Validation Error Message Must Be Passed Via The `root.serverError` Key](#global-server-side-validation-error-message-must-be-passed-via-the-rootservererror-key) - [TypeScript](#typescript) - [Fields Components Requires The source Prop](#fields-components-requires-the-source-prop) - [`useRecordContext` Returns undefined When No Record Is Available](#userecordcontext-returns-undefined-when-no-record-is-available) @@ -1004,6 +1005,38 @@ The `` component no longer accepts a `touched` prop. This prop If you were using this prop, you can safely remove it. +### Global Server Side Validation Error Message Must Be Passed Via The `root.serverError` Key + +You can now include a global server-side error message in the response to a failed create or update request. This message will be rendered in a notification. To do so, include the error message in the `root.serverError` key of the `errors` object in the response body: + +```diff +{ + "body": { + "errors": { ++ "root": { "serverError": "Some of the provided values are not valid. Please fix them and retry." }, + "title": "An article with this title already exists. The title must be unique.", + "date": "The date is required", + "tags": { "message": "The tag 'agrriculture' doesn't exist" }, + } + } +} +``` + +**Minor BC:** To avoid a race condition between the notifications sent due to both the http error and the validation error, React Admin will no longer display a notification for the http error if the response contains a non-empty `errors` object and the mutation mode is `pessimistic`. If you relied on this behavior to render a global server-side error message, you should now include the message in the `root.serverError` key of the `errors` object. + +```diff +{ +- "message": "Some of the provided values are not valid. Please fix them and retry.", + "body": { + "errors": { ++ "root": { "serverError": "Some of the provided values are not valid. Please fix them and retry." }, + "title": "An article with this title already exists. The title must be unique.", + "date": "The date is required", + "tags": { "message": "The tag 'agrriculture' doesn't exist" }, + } + } +} +``` ## TypeScript diff --git a/docs/Validation.md b/docs/Validation.md index f51b2fc39b7..6fbe5701232 100644 --- a/docs/Validation.md +++ b/docs/Validation.md @@ -367,21 +367,24 @@ const CustomerCreate = () => ( Server-side validation is supported out of the box for `pessimistic` mode only. It requires that the dataProvider throws an error with the following shape: -``` +```json { - body: { - errors: { - title: 'An article with this title already exists. The title must be unique.', - date: 'The date is required', - tags: { message: "The tag 'agrriculture' doesn't exist" }, + "body": { + "errors": { + // Global validation error message (optional) + "root": { "serverError": "Some of the provided values are not valid. Please fix them and retry." }, + // Field validation error messages + "title": "An article with this title already exists. The title must be unique.", + "date": "The date is required", + "tags": { "message": "The tag 'agrriculture' doesn't exist" }, } } } ``` -**Tip**: The shape of the returned validation errors must match the form shape: each key needs to match a `source` prop. +**Tip**: The shape of the returned validation errors must match the form shape: each key needs to match a `source` prop. The only exception is the `root.serverError` key, which can be used to define a global error message for the form. -**Tip**: The returned validation errors might have any validation format we support (simple strings, translation strings or translation objects with a `message` attribute) for each key. +**Tip**: The returned validation errors might have any validation format we support (simple strings, translation strings or translation objects with a `message` attribute) for each key. However `root.serverError` does not accept translation objects. **Tip**: If your data provider leverages React Admin's [`httpClient`](https://marmelab.com/react-admin/DataProviderWriting.html#example-rest-implementation), all error response bodies are wrapped and thrown as `HttpError`. This means your API only needs to return an invalid response with a json body containing the `errors` key. @@ -396,6 +399,7 @@ const apiUrl = 'https://my.api.com/'; { "errors": { + "root": { "serverError": "Some of the provided values are not valid. Please fix them and retry." }, "title": "An article with this title already exists. The title must be unique.", "date": "The date is required", "tags": { "message": "The tag 'agrriculture' doesn't exist" }, @@ -431,6 +435,7 @@ const myDataProvider = { body should be something like: { errors: { + root: { serverError: "Some of the provided values are not valid. Please fix them and retry." }, title: "An article with this title already exists. The title must be unique.", date: "The date is required", tags: { message: "The tag 'agrriculture' doesn't exist" }, diff --git a/packages/ra-core/src/controller/create/useCreateController.spec.tsx b/packages/ra-core/src/controller/create/useCreateController.spec.tsx index 7c2fed422fe..a300c996f70 100644 --- a/packages/ra-core/src/controller/create/useCreateController.spec.tsx +++ b/packages/ra-core/src/controller/create/useCreateController.spec.tsx @@ -180,6 +180,77 @@ describe('useCreateController', () => { ]); }); + it('should use the default error message in case no message was provided', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + let saveCallback; + const dataProvider = testDataProvider({ + getOne: () => Promise.resolve({ data: { id: 12 } } as any), + create: () => Promise.reject({}), + }); + + let notificationsSpy; + const Notification = () => { + const { notifications } = useNotificationContext(); + React.useEffect(() => { + notificationsSpy = notifications; + }, [notifications]); + return null; + }; + + render( + + + + {({ save }) => { + saveCallback = save; + return null; + }} + + + ); + await act(async () => saveCallback({ foo: 'bar' })); + expect(notificationsSpy).toEqual([ + { + message: 'ra.notification.http_error', + type: 'error', + notificationOptions: { messageArgs: { _: undefined } }, + }, + ]); + }); + + it('should not trigger a notification in case of a validation error (handled by useNotifyIsFormInvalid)', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + let saveCallback; + const dataProvider = testDataProvider({ + getOne: () => Promise.resolve({ data: { id: 12 } } as any), + create: () => + Promise.reject({ body: { errors: { foo: 'invalid' } } }), + }); + + let notificationsSpy; + const Notification = () => { + const { notifications } = useNotificationContext(); + React.useEffect(() => { + notificationsSpy = notifications; + }, [notifications]); + return null; + }; + + render( + + + + {({ save }) => { + saveCallback = save; + return null; + }} + + + ); + await act(async () => saveCallback({ foo: 'bar' })); + expect(notificationsSpy).toEqual([]); + }); + it('should allow mutationOptions to override the default success side effects', async () => { let saveCallback; const dataProvider = testDataProvider({ diff --git a/packages/ra-core/src/controller/create/useCreateController.ts b/packages/ra-core/src/controller/create/useCreateController.ts index dfe97936009..f7336571611 100644 --- a/packages/ra-core/src/controller/create/useCreateController.ts +++ b/packages/ra-core/src/controller/create/useCreateController.ts @@ -108,26 +108,34 @@ export const useCreateController = < if (onError) { return onError(error, variables, context); } - notify( - typeof error === 'string' - ? error - : (error as Error).message || 'ra.notification.http_error', - { - type: 'error', - messageArgs: { - _: - typeof error === 'string' - ? error - : error instanceof Error || - (typeof error === 'object' && - error !== null && - error.hasOwnProperty('message')) - ? // @ts-ignore - error.message - : undefined, - }, - } - ); + // Don't trigger a notification if this is a validation error + // (notification will be handled by the useNotifyIsFormInvalid hook) + const validationErrors = (error as HttpError)?.body?.errors; + const hasValidationErrors = + !!validationErrors && Object.keys(validationErrors).length > 0; + if (!hasValidationErrors) { + notify( + typeof error === 'string' + ? error + : (error as Error).message || + 'ra.notification.http_error', + { + type: 'error', + messageArgs: { + _: + typeof error === 'string' + ? error + : error instanceof Error || + (typeof error === 'object' && + error !== null && + error.hasOwnProperty('message')) + ? // @ts-ignore + error.message + : undefined, + }, + } + ); + } }, ...otherMutationOptions, returnPromise: true, diff --git a/packages/ra-core/src/controller/edit/useEditController.spec.tsx b/packages/ra-core/src/controller/edit/useEditController.spec.tsx index 9713df6db3f..6342f332d4e 100644 --- a/packages/ra-core/src/controller/edit/useEditController.spec.tsx +++ b/packages/ra-core/src/controller/edit/useEditController.spec.tsx @@ -742,6 +742,157 @@ describe('useEditController', () => { ]); }); + it('should use the default error message in case no message was provided', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + let saveCallback; + const dataProvider = ({ + getOne: () => Promise.resolve({ data: { id: 12 } }), + update: () => Promise.reject({}), + } as unknown) as DataProvider; + + let notificationsSpy; + const Notification = () => { + const { notifications } = useNotificationContext(); + React.useEffect(() => { + notificationsSpy = notifications; + }, [notifications]); + return null; + }; + + render( + + + + {({ save }) => { + saveCallback = save; + return
; + }} + + + ); + await act(async () => saveCallback({ foo: 'bar' })); + await new Promise(resolve => setTimeout(resolve, 10)); + expect(notificationsSpy).toEqual([ + { + message: 'ra.notification.http_error', + type: 'error', + notificationOptions: { messageArgs: { _: undefined } }, + }, + ]); + }); + + it('should not trigger a notification in case of a validation error (handled by useNotifyIsFormInvalid)', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + let saveCallback; + const dataProvider = ({ + getOne: () => Promise.resolve({ data: { id: 12 } }), + update: () => + Promise.reject({ body: { errors: { foo: 'invalid' } } }), + } as unknown) as DataProvider; + + let notificationsSpy; + const Notification = () => { + const { notifications } = useNotificationContext(); + React.useEffect(() => { + notificationsSpy = notifications; + }, [notifications]); + return null; + }; + + render( + + + + {({ save }) => { + saveCallback = save; + return
; + }} + + + ); + await act(async () => saveCallback({ foo: 'bar' })); + await new Promise(resolve => setTimeout(resolve, 10)); + expect(notificationsSpy).toEqual([]); + }); + + it('should trigger a notification even in case of a validation error in optimistic mode', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + let saveCallback; + const dataProvider = ({ + getOne: () => Promise.resolve({ data: { id: 12 } }), + update: () => + Promise.reject({ body: { errors: { foo: 'invalid' } } }), + } as unknown) as DataProvider; + + let notificationsSpy; + const Notification = () => { + const { notifications } = useNotificationContext(); + React.useEffect(() => { + notificationsSpy = notifications; + }, [notifications]); + return null; + }; + + render( + + + + {({ save }) => { + saveCallback = save; + return
; + }} + + + ); + await act(async () => saveCallback({ foo: 'bar' })); + await new Promise(resolve => setTimeout(resolve, 10)); + expect(notificationsSpy).toContainEqual({ + message: 'ra.notification.http_error', + type: 'error', + notificationOptions: { messageArgs: { _: undefined } }, + }); + }); + + it('should trigger a notification even in case of a validation error in undoable mode', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + let saveCallback; + const dataProvider = ({ + getOne: () => Promise.resolve({ data: { id: 12 } }), + update: () => + Promise.reject({ body: { errors: { foo: 'invalid' } } }), + } as unknown) as DataProvider; + + let notificationsSpy; + const Notification = () => { + const { notifications } = useNotificationContext(); + React.useEffect(() => { + notificationsSpy = notifications; + }, [notifications]); + return null; + }; + + render( + + + + {({ save }) => { + saveCallback = save; + return
; + }} + + + ); + await act(async () => saveCallback({ foo: 'bar' })); + await new Promise(resolve => setTimeout(resolve, 10)); + undoableEventEmitter.emit('end', { isUndo: false }); + await new Promise(resolve => setTimeout(resolve, 10)); + expect(notificationsSpy).toContainEqual({ + message: 'ra.notification.http_error', + type: 'error', + notificationOptions: { messageArgs: { _: undefined } }, + }); + }); + it('should allow the save onError option to override the failure side effects override', async () => { jest.spyOn(console, 'error').mockImplementation(() => {}); let saveCallback; diff --git a/packages/ra-core/src/controller/edit/useEditController.ts b/packages/ra-core/src/controller/edit/useEditController.ts index ee1aa7ce78a..4b517e78f68 100644 --- a/packages/ra-core/src/controller/edit/useEditController.ts +++ b/packages/ra-core/src/controller/edit/useEditController.ts @@ -159,27 +159,35 @@ export const useEditController = < if (onError) { return onError(error, variables, context); } - notify( - typeof error === 'string' - ? error - : (error as Error).message || - 'ra.notification.http_error', - { - type: 'error', - messageArgs: { - _: - typeof error === 'string' - ? error - : error instanceof Error || - (typeof error === 'object' && - error !== null && - error.hasOwnProperty('message')) - ? // @ts-ignore - error.message - : undefined, - }, - } - ); + // Don't trigger a notification if this is a validation error + // (notification will be handled by the useNotifyIsFormInvalid hook) + const validationErrors = (error as HttpError)?.body?.errors; + const hasValidationErrors = + !!validationErrors && + Object.keys(validationErrors).length > 0; + if (!hasValidationErrors || mutationMode !== 'pessimistic') { + notify( + typeof error === 'string' + ? error + : (error as Error).message || + 'ra.notification.http_error', + { + type: 'error', + messageArgs: { + _: + typeof error === 'string' + ? error + : error instanceof Error || + (typeof error === 'object' && + error !== null && + error.hasOwnProperty('message')) + ? // @ts-ignore + error.message + : undefined, + }, + } + ); + } }, ...otherMutationOptions, mutationMode, diff --git a/packages/ra-core/src/form/Form.spec.tsx b/packages/ra-core/src/form/Form.spec.tsx index 487276ad643..d99356b9907 100644 --- a/packages/ra-core/src/form/Form.spec.tsx +++ b/packages/ra-core/src/form/Form.spec.tsx @@ -19,6 +19,7 @@ import { SanitizeEmptyValues, NullValue, InNonDataRouter, + ServerSideValidation, } from './Form.stories'; import { mergeTranslations } from '../i18n'; @@ -768,4 +769,26 @@ describe('Form', () => { fireEvent.click(screen.getByText('Leave the form')); await screen.findByText('Go to form'); }); + + it('should support server side validation', async () => { + render(); + fireEvent.change(screen.getByLabelText('defaultMessage'), { + target: { value: '' }, + }); + fireEvent.click(screen.getByText('Submit')); + await screen.findByText('Required'); + await screen.findByText('ra.message.invalid_form'); + }); + + it('should support using a custom global message with server side validation', async () => { + render(); + fireEvent.change(screen.getByLabelText('customGlobalMessage'), { + target: { value: '' }, + }); + fireEvent.click(screen.getByText('Submit')); + await screen.findByText('Required'); + await screen.findByText( + 'There are validation errors. Please fix them.' + ); + }); }); diff --git a/packages/ra-core/src/form/Form.stories.tsx b/packages/ra-core/src/form/Form.stories.tsx index 81c7dced708..45cf423956d 100644 --- a/packages/ra-core/src/form/Form.stories.tsx +++ b/packages/ra-core/src/form/Form.stories.tsx @@ -17,6 +17,7 @@ import { required } from './validate'; import ValidationError from './ValidationError'; import { mergeTranslations } from '../i18n'; import { I18nProvider } from '../types'; +import { SaveContextProvider, useNotificationContext } from '..'; export default { title: 'ra-core/form/Form', @@ -179,7 +180,11 @@ export const UndefinedValue = () => { const defaultI18nProvider = polyglotI18nProvider(() => mergeTranslations(englishMessages, { - app: { validation: { required: 'This field must be provided' } }, + app: { + validation: { + required: 'This field must be provided', + }, + }, }) ); @@ -338,3 +343,64 @@ export const InNonDataRouter = ({ ); + +const Notifications = () => { + const { notifications } = useNotificationContext(); + return ( +
    + {notifications.map(({ message }, id) => ( +
  • {message}
    • + ))} +
+ ); +}; + +export const ServerSideValidation = () => { + const save = React.useCallback(values => { + 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'; + } + if (!values.customGlobalMessage) { + errors.customGlobalMessage = 'ra.validation.required'; + errors.root = { + serverError: 'There are validation errors. Please fix them.', + }; + } + return Object.keys(errors).length > 0 ? errors : undefined; + }, []); + return ( + + +
+ + + + + + +
+ + + + ); +}; diff --git a/packages/ra-core/src/form/useNotifyIsFormInvalid.ts b/packages/ra-core/src/form/useNotifyIsFormInvalid.ts index 2dce93d0acc..9731631863a 100644 --- a/packages/ra-core/src/form/useNotifyIsFormInvalid.ts +++ b/packages/ra-core/src/form/useNotifyIsFormInvalid.ts @@ -26,7 +26,13 @@ export const useNotifyIsFormInvalid = ( submitCountRef.current = submitCount; if (Object.keys(errors).length > 0) { - notify('ra.message.invalid_form', { type: 'error' }); + const serverError = + typeof errors.root?.serverError?.message === 'string' + ? errors.root.serverError.message + : undefined; + notify(serverError || 'ra.message.invalid_form', { + type: 'error', + }); } } }, [errors, submitCount, notify, enabled]);