From 267e755773a932227ac708ac17d0b9c364c3c8ba Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Thu, 7 Jul 2022 10:58:40 +0200 Subject: [PATCH 1/8] Add server side validation support ## Problem - Server side validation is cumbersome to set up - When using middlewares and as the SaveButton relied on the SaveContext.saving prop, the SaveButton was only disabled for while the main mutation was loading. However, additional work may still be running. ## Solution For `pessismistic` mode only, we now transparently support server side validation. When a server validation occurs, the DataProvider should throw a ServerValidationError that contains an `errors` object matching the form shape. Besides, we now await the mutation in `useCreateController` and `useEditController`. In `pessimistic` mode, that means the `SaveButton` will stay disabled until the mutation is resolved including its middlewares if any. That does not change anything for the other mutation modes. --- docs/Validation.md | 72 +++++------ examples/simple/src/dataProvider.tsx | 10 +- .../create/useCreateController.spec.tsx | 33 +++++- .../controller/create/useCreateController.ts | 112 ++++++++++-------- .../edit/useEditController.spec.tsx | 36 ++++++ .../src/controller/edit/useEditController.ts | 109 +++++++++-------- .../src/controller/saveContext/SaveContext.ts | 3 + .../ra-core/src/dataProvider/useCreate.ts | 7 +- .../ra-core/src/dataProvider/useUpdate.ts | 4 +- .../src/button/SaveButton.tsx | 13 +- 10 files changed, 251 insertions(+), 148 deletions(-) diff --git a/docs/Validation.md b/docs/Validation.md index 601f4da1388..16ed1a0a9b5 100644 --- a/docs/Validation.md +++ b/docs/Validation.md @@ -320,42 +320,46 @@ const CustomerCreate = () => ( ## Server-Side Validation -You can use the errors returned by the dataProvider mutation as a source for the validation. In order to display the validation errors, a custom `save` function needs to be used: +Server-side validation is supported out of the box. It requires that the dataProvider throws an error with the following shape: -{% raw %} -```jsx -import * as React from 'react'; -import { useCallback } from 'react'; -import { Create, SimpleForm, TextInput, useCreate } from 'react-admin'; - -export const UserCreate = () => { - const [create] = useCreate(); - const save = useCallback( - async values => { - try { - await create('users', { data: values }, { returnPromise: true }); - } catch (error) { - if (error.body.errors) { - // The shape of the returned validation errors must match the shape of the form - return error.body.errors; - } - } - }, - [create] - ); - - return ( - - - - - - - ); -}; ``` -{% endraw %} +{ + body: { + errors: { + source: 'error message', + } + } +} +``` -**Tip**: The shape of the returned validation errors must correspond to the form: a 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. **Tip**: The returned validation errors might have any validation format we support (simple strings or object with message and args) for each key. + +**Tip**: If your data provider leverages our [`httpClient`](https://marmelab.com/react-admin/DataProviderWriting.html#example-rest-implementation), this will be handled automatically when your server returns an invalid response with a json body contains the `errors` key. + +Here's an example of a dataProvider not using our `httpClient`: + +```js +const myDataProvider = { + create: async (resource, { data }) => { + const response = await fetch(`${process.env.API_URL}/${resource}`, { + method: 'POST', + body: JSON.stringify(data), + }); + + const body = response.json(); + + if (status < 200 || status >= 300) { + // Here, we expect the body to contains an `errors` key + throw new HttpError( + (body && body.message) || statusText, + status, + body + ); + } + + return body; + } +} +``` \ No newline at end of file diff --git a/examples/simple/src/dataProvider.tsx b/examples/simple/src/dataProvider.tsx index db4520f935a..7b490f1ec6c 100644 --- a/examples/simple/src/dataProvider.tsx +++ b/examples/simple/src/dataProvider.tsx @@ -1,5 +1,5 @@ import fakeRestProvider from 'ra-data-fakerest'; -import { DataProvider } from 'react-admin'; +import { DataProvider, HttpError } from 'react-admin'; import get from 'lodash/get'; import data from './data'; import addUploadFeature from './addUploadFeature'; @@ -80,7 +80,13 @@ const sometimesFailsDataProvider = new Proxy(uploadCapableDataProvider, { params.data && params.data.title === 'f00bar' ) { - return Promise.reject(new Error('this title cannot be used')); + return Promise.reject( + new HttpError('The form is invalid', 404, { + errors: { + title: 'this title cannot be used', + }, + }) + ); } return uploadCapableDataProvider[name](resource, params); }, diff --git a/packages/ra-core/src/controller/create/useCreateController.spec.tsx b/packages/ra-core/src/controller/create/useCreateController.spec.tsx index cbbd952d0f1..c436f5e1348 100644 --- a/packages/ra-core/src/controller/create/useCreateController.spec.tsx +++ b/packages/ra-core/src/controller/create/useCreateController.spec.tsx @@ -1,6 +1,6 @@ import React from 'react'; import expect from 'expect'; -import { render, act } from '@testing-library/react'; +import { render, act, screen } from '@testing-library/react'; import { Location } from 'react-router-dom'; import { getRecordFromLocation } from './useCreateController'; @@ -13,6 +13,7 @@ import { SaveContextProvider, useRegisterMutationMiddleware, } from '../saveContext'; +import { DataProvider } from '../..'; describe('useCreateController', () => { describe('getRecordFromLocation', () => { @@ -502,4 +503,34 @@ describe('useCreateController', () => { expect.any(Function) ); }); + + it('The save function should return errors from the create call', async () => { + const create = jest.fn().mockImplementationOnce(() => { + return Promise.reject({ errors: { foo: 'invalid' } }); + }); + const dataProvider = ({ + create, + } as unknown) as DataProvider; + let saveCallback; + render( + + + {({ save, record }) => { + saveCallback = save; + return
; + }} + + + ); + await new Promise(resolve => setTimeout(resolve, 10)); + let errors; + await act(async () => { + errors = await saveCallback({ foo: 'bar' }); + }); + expect(errors).toEqual({ foo: 'invalid' }); + await new Promise(resolve => setTimeout(resolve, 10)); + expect(create).toHaveBeenCalledWith('posts', { + data: { foo: 'bar' }, + }); + }); }); diff --git a/packages/ra-core/src/controller/create/useCreateController.ts b/packages/ra-core/src/controller/create/useCreateController.ts index 7be1281a7f1..3430fe01ea3 100644 --- a/packages/ra-core/src/controller/create/useCreateController.ts +++ b/packages/ra-core/src/controller/create/useCreateController.ts @@ -6,7 +6,11 @@ import { Location } from 'history'; import { UseMutationOptions } from 'react-query'; import { useAuthenticated } from '../../auth'; -import { useCreate, UseCreateMutateParams } from '../../dataProvider'; +import { + HttpError, + useCreate, + UseCreateMutateParams, +} from '../../dataProvider'; import { useRedirect, RedirectionSideEffect } from '../../routing'; import { useNotify } from '../../notification'; import { SaveContextValue, useMutationMiddlewares } from '../saveContext'; @@ -74,7 +78,7 @@ export const useCreateController = < const [create, { isLoading: saving }] = useCreate< RecordType, MutationOptionsError - >(resource, undefined, otherMutationOptions); + >(resource, undefined, { ...otherMutationOptions, returnPromise: true }); const save = useCallback( ( @@ -91,55 +95,67 @@ export const useCreateController = < : transform ? transform(data) : data - ).then((data: Partial) => { + ).then(async (data: Partial) => { const mutate = getMutateWithMiddlewares(create); - mutate( - resource, - { data, meta }, - { - onSuccess: async (data, variables, context) => { - if (onSuccessFromSave) { - return onSuccessFromSave( - data, - variables, - context - ); - } - if (onSuccess) { - return onSuccess(data, variables, context); - } + try { + await mutate( + resource, + { data, meta }, + { + onSuccess: async (data, variables, context) => { + if (onSuccessFromSave) { + return onSuccessFromSave( + data, + variables, + context + ); + } + if (onSuccess) { + return onSuccess(data, variables, context); + } - notify('ra.notification.created', { - type: 'info', - messageArgs: { smart_count: 1 }, - }); - redirect(finalRedirectTo, resource, data.id, data); - }, - onError: onErrorFromSave - ? onErrorFromSave - : onError - ? onError - : (error: Error) => { - notify( - typeof error === 'string' - ? error - : error.message || - 'ra.notification.http_error', - { - type: 'warning', - messageArgs: { - _: - typeof error === 'string' - ? error - : error && error.message - ? error.message - : undefined, - }, - } - ); - }, + notify('ra.notification.created', { + type: 'info', + messageArgs: { smart_count: 1 }, + }); + redirect( + finalRedirectTo, + resource, + data.id, + data + ); + }, + onError: onErrorFromSave + ? onErrorFromSave + : onError + ? onError + : (error: Error) => { + notify( + typeof error === 'string' + ? error + : error.message || + 'ra.notification.http_error', + { + type: 'warning', + messageArgs: { + _: + typeof error === 'string' + ? error + : error && + error.message + ? error.message + : undefined, + }, + } + ); + }, + } + ); + } catch (error) { + if ((error as HttpError).body.errors != null) { + return (error as HttpError).body.errors; } - ); + } }), [ create, diff --git a/packages/ra-core/src/controller/edit/useEditController.spec.tsx b/packages/ra-core/src/controller/edit/useEditController.spec.tsx index fc681782545..39537d07157 100644 --- a/packages/ra-core/src/controller/edit/useEditController.spec.tsx +++ b/packages/ra-core/src/controller/edit/useEditController.spec.tsx @@ -881,4 +881,40 @@ describe('useEditController', () => { expect.any(Function) ); }); + + it('The save function should return errors from the update call in pessimistic mode', async () => { + let post = { id: 12 }; + const update = jest.fn().mockImplementationOnce(() => { + return Promise.reject({ errors: { foo: 'invalid' } }); + }); + const dataProvider = ({ + getOne: () => Promise.resolve({ data: post }), + update, + } as unknown) as DataProvider; + let saveCallback; + render( + + + {({ save, record }) => { + saveCallback = save; + return <>{JSON.stringify(record)}; + }} + + + ); + await new Promise(resolve => setTimeout(resolve, 10)); + screen.getByText('{"id":12}'); + let errors; + await act(async () => { + errors = await saveCallback({ foo: 'bar' }); + }); + expect(errors).toEqual({ foo: 'invalid' }); + await new Promise(resolve => setTimeout(resolve, 10)); + screen.getByText('{"id":12}'); + expect(update).toHaveBeenCalledWith('posts', { + id: 12, + data: { foo: 'bar' }, + previousData: { id: 12 }, + }); + }); }); diff --git a/packages/ra-core/src/controller/edit/useEditController.ts b/packages/ra-core/src/controller/edit/useEditController.ts index 8e3911e1bf9..2e2d7581da4 100644 --- a/packages/ra-core/src/controller/edit/useEditController.ts +++ b/packages/ra-core/src/controller/edit/useEditController.ts @@ -12,6 +12,7 @@ import { useRefresh, UseGetOneHookValue, UseUpdateMutateParams, + HttpError, } from '../../dataProvider'; import { useTranslate } from '../../i18n'; import { useResourceContext, useGetResourceLabel } from '../../core'; @@ -113,7 +114,11 @@ export const useEditController = < const [update, { isLoading: saving }] = useUpdate< RecordType, MutationOptionsError - >(resource, recordCached, { ...otherMutationOptions, mutationMode }); + >(resource, recordCached, { + ...otherMutationOptions, + mutationMode, + returnPromise: mutationMode === 'pessimistic', + }); const save = useCallback( ( @@ -134,57 +139,65 @@ export const useEditController = < previousData: recordCached.previousData, }) : data - ).then((data: Partial) => { + ).then(async (data: Partial) => { const mutate = getMutateWithMiddlewares(update); - return mutate( - resource, - { id, data, meta: mutationMeta }, - { - onSuccess: async (data, variables, context) => { - if (onSuccessFromSave) { - return onSuccessFromSave( - data, - variables, - context - ); - } - if (onSuccess) { - return onSuccess(data, variables, context); - } + try { + await mutate( + resource, + { id, data, meta: mutationMeta }, + { + onSuccess: async (data, variables, context) => { + if (onSuccessFromSave) { + return onSuccessFromSave( + data, + variables, + context + ); + } + + if (onSuccess) { + return onSuccess(data, variables, context); + } - notify('ra.notification.updated', { - type: 'info', - messageArgs: { smart_count: 1 }, - undoable: mutationMode === 'undoable', - }); - redirect(redirectTo, resource, data.id, data); - }, - onError: onErrorFromSave - ? onErrorFromSave - : onError - ? onError - : (error: Error | string) => { - notify( - typeof error === 'string' - ? error - : error.message || - 'ra.notification.http_error', - { - type: 'warning', - messageArgs: { - _: - typeof error === 'string' - ? error - : error && error.message - ? error.message - : undefined, - }, - } - ); - }, + notify('ra.notification.updated', { + type: 'info', + messageArgs: { smart_count: 1 }, + undoable: mutationMode === 'undoable', + }); + redirect(redirectTo, resource, data.id, data); + }, + onError: onErrorFromSave + ? onErrorFromSave + : onError + ? onError + : (error: Error | string) => { + notify( + typeof error === 'string' + ? error + : error.message || + 'ra.notification.http_error', + { + type: 'warning', + messageArgs: { + _: + typeof error === 'string' + ? error + : error && + error.message + ? error.message + : undefined, + }, + } + ); + }, + } + ); + } catch (error) { + if ((error as HttpError).body.errors != null) { + return (error as HttpError).body.errors; } - ); + } }), [ id, diff --git a/packages/ra-core/src/controller/saveContext/SaveContext.ts b/packages/ra-core/src/controller/saveContext/SaveContext.ts index 8b9797de353..540d3df6fec 100644 --- a/packages/ra-core/src/controller/saveContext/SaveContext.ts +++ b/packages/ra-core/src/controller/saveContext/SaveContext.ts @@ -13,6 +13,9 @@ export interface SaveContextValue< MutateFunc extends (...args: any[]) => any = (...args: any[]) => any > { save?: SaveHandler; + /** + * @deprecated. Rely on the form isSubmitting value instead + */ saving?: boolean; mutationMode?: MutationMode; registerMutationMiddleware?: (callback: Middleware) => void; diff --git a/packages/ra-core/src/dataProvider/useCreate.ts b/packages/ra-core/src/dataProvider/useCreate.ts index 73d7027834f..77f9885c943 100644 --- a/packages/ra-core/src/dataProvider/useCreate.ts +++ b/packages/ra-core/src/dataProvider/useCreate.ts @@ -127,7 +127,10 @@ export const useCreate = < unknown > & { returnPromise?: boolean } = {} ) => { - const { returnPromise, ...reactCreateOptions } = createOptions; + const { + returnPromise = options.returnPromise, + ...reactCreateOptions + } = createOptions; if (returnPromise) { return mutation.mutateAsync( { resource: callTimeResource, ...callTimeParams }, @@ -156,7 +159,7 @@ export type UseCreateOptions< RecordType, MutationError, Partial> ->; +> & { returnPromise?: boolean }; export type UseCreateResult< RecordType extends RaRecord = any, diff --git a/packages/ra-core/src/dataProvider/useUpdate.ts b/packages/ra-core/src/dataProvider/useUpdate.ts index 53a050fde06..b91a8128763 100644 --- a/packages/ra-core/src/dataProvider/useUpdate.ts +++ b/packages/ra-core/src/dataProvider/useUpdate.ts @@ -270,7 +270,7 @@ export const useUpdate = < ) => { const { mutationMode, - returnPromise, + returnPromise = reactMutationOptions.returnPromise, onSuccess, onSettled, onError, @@ -431,7 +431,7 @@ export type UseUpdateOptions< RecordType, MutationError, Partial> -> & { mutationMode?: MutationMode }; +> & { mutationMode?: MutationMode; returnPromise?: boolean }; export type UseUpdateResult< RecordType extends RaRecord = any, diff --git a/packages/ra-ui-materialui/src/button/SaveButton.tsx b/packages/ra-ui-materialui/src/button/SaveButton.tsx index 74f8f5a2aa1..fdf3689a41c 100644 --- a/packages/ra-ui-materialui/src/button/SaveButton.tsx +++ b/packages/ra-ui-materialui/src/button/SaveButton.tsx @@ -54,7 +54,6 @@ export const SaveButton = ( label = 'ra.action.save', onClick, mutationOptions, - saving, disabled: disabledProp, type = 'submit', transform, @@ -72,11 +71,7 @@ export const SaveButton = ( alwaysEnable === false || alwaysEnable === undefined ? undefined : !alwaysEnable, - disabledProp || - !isDirty || - isValidating || - saveContext?.saving || - isSubmitting + disabledProp || !isDirty || isValidating || isSubmitting ); warning( @@ -122,10 +117,6 @@ export const SaveButton = ( ); const displayedLabel = label && translate(label, { _: label }); - const finalSaving = - typeof saving !== 'undefined' - ? saving - : saveContext?.saving || isSubmitting; return ( ( // TODO: find a way to display the loading state (LoadingButton from mui Lab?) {...rest} > - {finalSaving ? : icon} + {isSubmitting ? : icon} {displayedLabel} ); From f96df585f1981df252dcacd561366e310bdbbdb3 Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Thu, 7 Jul 2022 11:14:52 +0200 Subject: [PATCH 2/8] Fix tests --- .../ra-core/src/controller/create/useCreateController.spec.tsx | 2 +- packages/ra-core/src/controller/create/useCreateController.ts | 2 +- packages/ra-core/src/controller/edit/useEditController.spec.tsx | 2 +- packages/ra-core/src/controller/edit/useEditController.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ra-core/src/controller/create/useCreateController.spec.tsx b/packages/ra-core/src/controller/create/useCreateController.spec.tsx index c436f5e1348..43b3318320d 100644 --- a/packages/ra-core/src/controller/create/useCreateController.spec.tsx +++ b/packages/ra-core/src/controller/create/useCreateController.spec.tsx @@ -506,7 +506,7 @@ describe('useCreateController', () => { it('The save function should return errors from the create call', async () => { const create = jest.fn().mockImplementationOnce(() => { - return Promise.reject({ errors: { foo: 'invalid' } }); + return Promise.reject({ body: { errors: { foo: 'invalid' } } }); }); const dataProvider = ({ create, diff --git a/packages/ra-core/src/controller/create/useCreateController.ts b/packages/ra-core/src/controller/create/useCreateController.ts index 3430fe01ea3..5382ccaf058 100644 --- a/packages/ra-core/src/controller/create/useCreateController.ts +++ b/packages/ra-core/src/controller/create/useCreateController.ts @@ -152,7 +152,7 @@ export const useCreateController = < } ); } catch (error) { - if ((error as HttpError).body.errors != null) { + if ((error as HttpError).body?.errors != null) { return (error as HttpError).body.errors; } } diff --git a/packages/ra-core/src/controller/edit/useEditController.spec.tsx b/packages/ra-core/src/controller/edit/useEditController.spec.tsx index 39537d07157..d3643666496 100644 --- a/packages/ra-core/src/controller/edit/useEditController.spec.tsx +++ b/packages/ra-core/src/controller/edit/useEditController.spec.tsx @@ -885,7 +885,7 @@ describe('useEditController', () => { it('The save function should return errors from the update call in pessimistic mode', async () => { let post = { id: 12 }; const update = jest.fn().mockImplementationOnce(() => { - return Promise.reject({ errors: { foo: 'invalid' } }); + return Promise.reject({ body: { errors: { foo: 'invalid' } } }); }); const dataProvider = ({ getOne: () => Promise.resolve({ data: post }), diff --git a/packages/ra-core/src/controller/edit/useEditController.ts b/packages/ra-core/src/controller/edit/useEditController.ts index 2e2d7581da4..3d74e960d86 100644 --- a/packages/ra-core/src/controller/edit/useEditController.ts +++ b/packages/ra-core/src/controller/edit/useEditController.ts @@ -194,7 +194,7 @@ export const useEditController = < } ); } catch (error) { - if ((error as HttpError).body.errors != null) { + if ((error as HttpError).body?.errors != null) { return (error as HttpError).body.errors; } } From fa7b88dcf70666518a9ae39e3ee1d90766200c61 Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Thu, 7 Jul 2022 11:35:46 +0200 Subject: [PATCH 3/8] Apply changes after review --- docs/Validation.md | 2 +- examples/simple/src/dataProvider.tsx | 2 +- .../ra-core/src/controller/create/useCreateController.spec.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/Validation.md b/docs/Validation.md index 16ed1a0a9b5..b25e4ecf23f 100644 --- a/docs/Validation.md +++ b/docs/Validation.md @@ -336,7 +336,7 @@ Server-side validation is supported out of the box. It requires that the dataPro **Tip**: The returned validation errors might have any validation format we support (simple strings or object with message and args) for each key. -**Tip**: If your data provider leverages our [`httpClient`](https://marmelab.com/react-admin/DataProviderWriting.html#example-rest-implementation), this will be handled automatically when your server returns an invalid response with a json body contains the `errors` key. +**Tip**: If your data provider leverages our [`httpClient`](https://marmelab.com/react-admin/DataProviderWriting.html#example-rest-implementation), this will be handled automatically when your server returns an invalid response with a json body containing the `errors` key. Here's an example of a dataProvider not using our `httpClient`: diff --git a/examples/simple/src/dataProvider.tsx b/examples/simple/src/dataProvider.tsx index 7b490f1ec6c..44c8044a1f1 100644 --- a/examples/simple/src/dataProvider.tsx +++ b/examples/simple/src/dataProvider.tsx @@ -81,7 +81,7 @@ const sometimesFailsDataProvider = new Proxy(uploadCapableDataProvider, { params.data.title === 'f00bar' ) { return Promise.reject( - new HttpError('The form is invalid', 404, { + new HttpError('The form is invalid', 400, { errors: { title: 'this title cannot be used', }, diff --git a/packages/ra-core/src/controller/create/useCreateController.spec.tsx b/packages/ra-core/src/controller/create/useCreateController.spec.tsx index 43b3318320d..39930dafc8e 100644 --- a/packages/ra-core/src/controller/create/useCreateController.spec.tsx +++ b/packages/ra-core/src/controller/create/useCreateController.spec.tsx @@ -1,6 +1,6 @@ import React from 'react'; import expect from 'expect'; -import { render, act, screen } from '@testing-library/react'; +import { render, act } from '@testing-library/react'; import { Location } from 'react-router-dom'; import { getRecordFromLocation } from './useCreateController'; From 0625ac2896b3abfcdec18f107e998624c760e831 Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Fri, 29 Jul 2022 09:52:11 +0200 Subject: [PATCH 4/8] Apply review comments --- .../src/controller/create/useCreateController.spec.tsx | 3 +-- .../ra-core/src/controller/edit/useEditController.spec.tsx | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/ra-core/src/controller/create/useCreateController.spec.tsx b/packages/ra-core/src/controller/create/useCreateController.spec.tsx index 39930dafc8e..3be5ba9c9fc 100644 --- a/packages/ra-core/src/controller/create/useCreateController.spec.tsx +++ b/packages/ra-core/src/controller/create/useCreateController.spec.tsx @@ -504,7 +504,7 @@ describe('useCreateController', () => { ); }); - it('The save function should return errors from the create call', async () => { + it('should return errors from the create call', async () => { const create = jest.fn().mockImplementationOnce(() => { return Promise.reject({ body: { errors: { foo: 'invalid' } } }); }); @@ -528,7 +528,6 @@ describe('useCreateController', () => { errors = await saveCallback({ foo: 'bar' }); }); expect(errors).toEqual({ foo: 'invalid' }); - await new Promise(resolve => setTimeout(resolve, 10)); expect(create).toHaveBeenCalledWith('posts', { data: { foo: 'bar' }, }); diff --git a/packages/ra-core/src/controller/edit/useEditController.spec.tsx b/packages/ra-core/src/controller/edit/useEditController.spec.tsx index d3643666496..ac88da755d8 100644 --- a/packages/ra-core/src/controller/edit/useEditController.spec.tsx +++ b/packages/ra-core/src/controller/edit/useEditController.spec.tsx @@ -882,7 +882,7 @@ describe('useEditController', () => { ); }); - it('The save function should return errors from the update call in pessimistic mode', async () => { + it('should return errors from the update call in pessimistic mode', async () => { let post = { id: 12 }; const update = jest.fn().mockImplementationOnce(() => { return Promise.reject({ body: { errors: { foo: 'invalid' } } }); @@ -902,14 +902,12 @@ describe('useEditController', () => { ); - await new Promise(resolve => setTimeout(resolve, 10)); - screen.getByText('{"id":12}'); + await screen.findByText('{"id":12}'); let errors; await act(async () => { errors = await saveCallback({ foo: 'bar' }); }); expect(errors).toEqual({ foo: 'invalid' }); - await new Promise(resolve => setTimeout(resolve, 10)); screen.getByText('{"id":12}'); expect(update).toHaveBeenCalledWith('posts', { id: 12, From 26ba43de87d8cd15684f6e463a8ded723f3de293 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Kaiser Date: Wed, 15 Feb 2023 16:26:03 +0100 Subject: [PATCH 5/8] code review --- docs/Validation.md | 54 ++++++++++++++++--- .../src/button/SaveButton.tsx | 2 - 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/docs/Validation.md b/docs/Validation.md index 4dd209ede25..7d4a790029a 100644 --- a/docs/Validation.md +++ b/docs/Validation.md @@ -368,7 +368,9 @@ Server-side validation is supported out of the box. It requires that the dataPro { body: { errors: { - source: 'error message', + 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" }, } } } @@ -378,11 +380,42 @@ Server-side validation is supported out of the box. It requires that the dataPro **Tip**: The returned validation errors might have any validation format we support (simple strings or object with message and args) for each key. -**Tip**: If your data provider leverages our [`httpClient`](https://marmelab.com/react-admin/DataProviderWriting.html#example-rest-implementation), this will be handled automatically when your server returns an invalid response with a json body containing the `errors` key. +**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. -Here's an example of a dataProvider not using our `httpClient`: +```js +import { fetchUtils } from "react-admin"; + +const httpClient = fetchUtils.fetchJson; + +const apiUrl = 'https://my.api.com/'; +/* + Example response from the API when there are validation errors: + + { + "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" }, + } + } +*/ + +const myDataProvider = { + create: (resource, params) => + httpClient(`${apiUrl}/${resource}`, { + method: 'POST', + body: JSON.stringify(params.data), + }).then(({ json }) => ({ + data: { ...params.data, id: json.id }, + })), +} +``` + +**Tip:** If you are not using React Admin's `httpClient`, you can still wrap errors in an `HttpError` to return them with the correct shape: ```js +import { HttpError } from 'react-admin' + const myDataProvider = { create: async (resource, { data }) => { const response = await fetch(`${process.env.API_URL}/${resource}`, { @@ -391,11 +424,20 @@ const myDataProvider = { }); const body = response.json(); + /* + body should be something like: + { + 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" }, + } + } + */ if (status < 200 || status >= 300) { - // Here, we expect the body to contains an `errors` key throw new HttpError( - (body && body.message) || statusText, + (body && body.message) || status, status, body ); @@ -404,4 +446,4 @@ const myDataProvider = { return body; } } -``` \ No newline at end of file +``` diff --git a/packages/ra-ui-materialui/src/button/SaveButton.tsx b/packages/ra-ui-materialui/src/button/SaveButton.tsx index 14deb8cd86a..08870b6a3b1 100644 --- a/packages/ra-ui-materialui/src/button/SaveButton.tsx +++ b/packages/ra-ui-materialui/src/button/SaveButton.tsx @@ -154,7 +154,6 @@ interface Props< CreateParams | UpdateParams >; transform?: TransformData; - saving?: boolean; variant?: string; } @@ -169,7 +168,6 @@ SaveButton.propTypes = { className: PropTypes.string, invalid: PropTypes.bool, label: PropTypes.string, - saving: PropTypes.bool, variant: PropTypes.oneOf(['text', 'outlined', 'contained']), icon: PropTypes.element, alwaysEnable: PropTypes.bool, From 71fa7de5fd133a02c95caef7af041eb3ae5622a9 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Kaiser Date: Wed, 15 Feb 2023 16:54:09 +0100 Subject: [PATCH 6/8] fix doc about translation objects support --- docs/Validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Validation.md b/docs/Validation.md index 7d4a790029a..55dc3dadf7d 100644 --- a/docs/Validation.md +++ b/docs/Validation.md @@ -378,7 +378,7 @@ Server-side validation is supported out of the box. It requires that the dataPro **Tip**: The shape of the returned validation errors must match the form shape: each key needs to match a `source` prop. -**Tip**: The returned validation errors might have any validation format we support (simple strings or object with message and args) 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. **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. From ba28552733a4c2ec5969e45d4e90d4ccec4bcd7b Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Kaiser Date: Wed, 15 Feb 2023 17:18:58 +0100 Subject: [PATCH 7/8] fix E2E test --- cypress/e2e/edit.cy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/e2e/edit.cy.js b/cypress/e2e/edit.cy.js index 6e0d47f29f1..487b959473d 100644 --- a/cypress/e2e/edit.cy.js +++ b/cypress/e2e/edit.cy.js @@ -319,7 +319,7 @@ describe('Edit Page', () => { cy.get('body').click('left'); // dismiss notification cy.get('div[role="alert"]').should(el => - expect(el).to.have.text('this title cannot be used') + expect(el).to.have.text('The form is invalid') ); cy.get(ListPagePosts.elements.recordRows) From 03c8bac97b022e07284cd8d751989b1ecc4b3634 Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Fri, 24 Feb 2023 12:21:19 +0100 Subject: [PATCH 8/8] Mention server validation only works in pessimistic mode --- docs/Validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Validation.md b/docs/Validation.md index 55dc3dadf7d..2654858e176 100644 --- a/docs/Validation.md +++ b/docs/Validation.md @@ -362,7 +362,7 @@ const CustomerCreate = () => ( ## Server-Side Validation -Server-side validation is supported out of the box. It requires that the dataProvider throws an error with the following shape: +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: ``` {