Merge pull request #9848 from marmelab/global-form-server-validation-…

…error

Fix race condition between http error notification and server-side validation error notification

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)
     - [`<InputHelperText touched>` 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 `<InputHelperText>` 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
 

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" },

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(
+            <CoreAdminContext dataProvider={dataProvider}>
+                <Notification />
+                <CreateController {...defaultProps}>
+                    {({ save }) => {
+                        saveCallback = save;
+                        return null;
+                    }}
+                </CreateController>
+            </CoreAdminContext>
+        );
+        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(
+            <CoreAdminContext dataProvider={dataProvider}>
+                <Notification />
+                <CreateController {...defaultProps}>
+                    {({ save }) => {
+                        saveCallback = save;
+                        return null;
+                    }}
+                </CreateController>
+            </CoreAdminContext>
+        );
+        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({

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,

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(
+            <CoreAdminContext dataProvider={dataProvider}>
+                <Notification />
+                <EditController {...defaultProps} mutationMode="pessimistic">
+                    {({ save }) => {
+                        saveCallback = save;
+                        return <div />;
+                    }}
+                </EditController>
+            </CoreAdminContext>
+        );
+        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(
+            <CoreAdminContext dataProvider={dataProvider}>
+                <Notification />
+                <EditController {...defaultProps} mutationMode="pessimistic">
+                    {({ save }) => {
+                        saveCallback = save;
+                        return <div />;
+                    }}
+                </EditController>
+            </CoreAdminContext>
+        );
+        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(
+            <CoreAdminContext dataProvider={dataProvider}>
+                <Notification />
+                <EditController {...defaultProps} mutationMode="optimistic">
+                    {({ save }) => {
+                        saveCallback = save;
+                        return <div />;
+                    }}
+                </EditController>
+            </CoreAdminContext>
+        );
+        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(
+            <CoreAdminContext dataProvider={dataProvider}>
+                <Notification />
+                <EditController {...defaultProps} mutationMode="undoable">
+                    {({ save }) => {
+                        saveCallback = save;
+                        return <div />;
+                    }}
+                </EditController>
+            </CoreAdminContext>
+        );
+        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;