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 01/35] 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 02/35] 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 03/35] 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 04/35] 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 df1da58e5f31bf75e4b8f40c9bae235b1727f2ef Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Fri, 13 Jan 2023 12:44:10 +0100 Subject: [PATCH 05/35] Introduce withRefreshAuth --- docs/navigation.html | 1 + docs/withRefreshAuth.md | 88 ++++++++++++++++++++ packages/ra-core/src/auth/index.ts | 1 + packages/ra-core/src/auth/withRefreshAuth.ts | 56 +++++++++++++ 4 files changed, 146 insertions(+) create mode 100644 docs/withRefreshAuth.md create mode 100644 packages/ra-core/src/auth/withRefreshAuth.ts diff --git a/docs/navigation.html b/docs/navigation.html index 78e988e0178..77aaa114199 100644 --- a/docs/navigation.html +++ b/docs/navigation.html @@ -56,6 +56,7 @@
  • usePermissions
  • useCanAccess
  • canAccess
  • +
  • withRefreshAuth
    • List Page
      diff --git a/docs/withRefreshAuth.md b/docs/withRefreshAuth.md new file mode 100644 index 00000000000..ada321c02fc --- /dev/null +++ b/docs/withRefreshAuth.md @@ -0,0 +1,88 @@ +--- +layout: default +title: "withRefreshAuth" +--- + +# `withRefreshAuth` + +This helper function wraps existing [`dataProviders`](./DataProviderIntroduction.md) and [`authProviders`]('./Authentication.md') to support authentication token refreshing mechanisms. + +## Usage + +Use `withRefreshAuth` to decorate an existing data provider or auth provider (usually both). In addition to the base provider, this function takes a function responsible for refreshing the authentication token if needed. + +Here is a simple example that refreshes an expired JWT token when needed: + +```jsx +// in src/refreshAuth.js +import { getAuthTokensFromLocalStorage } from './getAuthTokensFromLocalStorage'; +import { refreshAuthTokens } from './refreshAuthTokens'; + +export const refreshAuth = () => { + const { accessToken, refreshToken } = getAuthTokensFromLocalStorage(); + if (accessToken.exp < Date.now().getTime() / 1000) { + // This function will fetch the new tokens from the authentication service and update them in localStorage + return refreshAuthTokens(refreshToken); + } + return Promise.resolve(); +} + +// in src/dataProvider.js +import { withRefreshAuth } from 'react-admin'; +import simpleRestProvider from 'ra-data-simple-rest'; +import { refreshAuth } from 'refreshAuth'; + +const baseDataProvider = simpleRestProvider('http://path.to.my.api/'); + +export const dataProvider = withRefreshAuth(baseDataProvider, refreshAuth); + +// in src/authProvider.js +import { withRefreshAuth } from 'react-admin'; +import { refreshAuth } from 'refreshAuth'; + +const myAuthProvider = { + // ...Usual AuthProvider methods +}; + +export const authProvider = withRefreshAuth(myAuthProvider, refreshAuth); +``` + +Then, inject the decorated providers in the `` component: + +```jsx +// in src/App.js +import { Admin } from 'react-admin'; +import { authProvider } from './authProvider'; +import { dataProvider } from './dataProvider'; + +export const App = () => ( + + {/* ... */} + +) +``` + +## `provider` + +The first argument must be a valid `dataProvider` or `authProvider` object - for instance, [any third-party data provider](./DataProviderList.md). + +```jsx +// in src/dataProvider.js +import { withRefreshAuth } from 'react-admin'; +import simpleRestProvider from 'ra-data-simple-rest'; + +const baseDataProvider = simpleRestProvider('http://path.to.my.api/'); + +export const dataProvider = withRefreshAuth(baseDataProvider, [ /* refreshAuth function */ ]); +``` + +## `refreshAuth` + +The second argument is a function responsible for refreshing the authentication tokens if needed. It must return a promise. + +```jsx +import jsonServerProvider from "ra-data-json-server"; +import { refreshAuth } from "./refreshAuth"; + +export const dataProvider = withRefreshAuth(baseDataProvider, refreshAuth); +``` diff --git a/packages/ra-core/src/auth/index.ts b/packages/ra-core/src/auth/index.ts index 64f4fa8e8b5..cabd733ce55 100644 --- a/packages/ra-core/src/auth/index.ts +++ b/packages/ra-core/src/auth/index.ts @@ -16,6 +16,7 @@ export * from './useAuthenticated'; export * from './useCheckAuth'; export * from './useGetIdentity'; export * from './useHandleAuthCallback'; +export * from './withRefreshAuth'; export { AuthContext, diff --git a/packages/ra-core/src/auth/withRefreshAuth.ts b/packages/ra-core/src/auth/withRefreshAuth.ts new file mode 100644 index 00000000000..97e89485e2a --- /dev/null +++ b/packages/ra-core/src/auth/withRefreshAuth.ts @@ -0,0 +1,56 @@ +import { AuthProvider, DataProvider } from '../types'; + +/** + * A higher-order function which wraps a dataProvider or authProvider to handle refreshing authentication. + * This is useful when the authentication service supports a refresh token mechanism. + * The wrapped provider will call the refreshAuth function before calling any dataProvider methods and before + * calling the authProvider checkAuth, getIdentity and getPermissions methods. + * + * The refreshAuth function should return a Promise that resolves when the authentication token has been refreshed. + * It might throw an error if the refresh failed. In this case, react-admin will handle the error as usual. + * + * @param provider Either a dataProvider or an authProvider + * @param refreshAuth A function that refreshes the authentication token if needed and returns a Promise. + * @returns A wrapped dataProvider or authProvider. + * + * @example + * import { withRefreshAuth } from 'react-admin'; + * import { jsonServerProvider } from 'ra-data-json-server'; + * import { authProvider } from './authProvider'; + * import { refreshAuth } from './refreshAuth'; + * + * const dataProvider = withRefreshAuth(jsonServerProvider('http://localhost:3000'), refreshAuth); + * const authProvider = withRefreshAuth(authProvider, refreshAuth); + */ +export const withRefreshAuth = < + ProviderType extends AuthProvider | DataProvider +>( + provider: ProviderType, + refreshAuth: () => Promise +): ProviderType => { + const proxy = new Proxy(provider, { + get(_, name) { + const isDataProvider = provider.hasOwnProperty('getList'); + const shouldIntercept = + isDataProvider || + AuthProviderInterceptedMethods.includes(name.toString()); + + if (shouldIntercept) { + return async (...args: any[]) => { + await refreshAuth(); + return provider[name.toString()](...args); + }; + } + + return provider[name.toString()]; + }, + }); + + return proxy; +}; + +const AuthProviderInterceptedMethods = [ + 'checkAuth', + 'getIdentity', + 'getPermissions', +]; From 7c6d62c6378afc83f9f3a20606f5a89044b07cb3 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Kaiser Date: Wed, 15 Feb 2023 12:26:31 +0100 Subject: [PATCH 06/35] code review changes --- docs/addRefreshAuthToAuthProvider.md | 85 +++++++++++++++++++ ...uth.md => addRefreshAuthToDataProvider.md} | 41 ++++----- docs/navigation.html | 3 +- ...uth.ts => addRefreshAuthToAuthProvider.ts} | 31 +++---- .../src/auth/addRefreshAuthToDataProvider.ts | 36 ++++++++ packages/ra-core/src/auth/index.ts | 3 +- 6 files changed, 156 insertions(+), 43 deletions(-) create mode 100644 docs/addRefreshAuthToAuthProvider.md rename docs/{withRefreshAuth.md => addRefreshAuthToDataProvider.md} (51%) rename packages/ra-core/src/auth/{withRefreshAuth.ts => addRefreshAuthToAuthProvider.ts} (55%) create mode 100644 packages/ra-core/src/auth/addRefreshAuthToDataProvider.ts diff --git a/docs/addRefreshAuthToAuthProvider.md b/docs/addRefreshAuthToAuthProvider.md new file mode 100644 index 00000000000..030d6346b02 --- /dev/null +++ b/docs/addRefreshAuthToAuthProvider.md @@ -0,0 +1,85 @@ +--- +layout: default +title: "addRefreshAuthToAuthProvider" +--- + +# `addRefreshAuthToAuthProvider` + +This helper function wraps an existing [`authProvider`]('./Authentication.md') to support authentication token refreshing mechanisms. + +## Usage + +Use `addRefreshAuthToAuthProvider` to decorate an existing auth provider. In addition to the base provider, this function takes a function responsible for refreshing the authentication token if needed. + +Here is a simple example that refreshes an expired JWT token when needed: + +```jsx +// in src/refreshAuth.js +import { getAuthTokensFromLocalStorage } from './getAuthTokensFromLocalStorage'; +import { refreshAuthTokens } from './refreshAuthTokens'; + +export const refreshAuth = () => { + const { accessToken, refreshToken } = getAuthTokensFromLocalStorage(); + if (accessToken.exp < Date.now().getTime() / 1000) { + // This function will fetch the new tokens from the authentication service and update them in localStorage + return refreshAuthTokens(refreshToken); + } + return Promise.resolve(); +} + +// in src/authProvider.js +import { addRefreshAuthToAuthProvider } from 'react-admin'; +import { refreshAuth } from 'refreshAuth'; + +const myAuthProvider = { + // ...Usual AuthProvider methods +}; + +export const authProvider = addRefreshAuthToAuthProvider(myAuthProvider, refreshAuth); +``` + +Then, inject the decorated provider in the `` component: + +```jsx +// in src/App.js +import { Admin } from 'react-admin'; +import { dataProvider } from './dataProvider'; +import { authProvider } from './authProvider'; + +export const App = () => ( + + {/* ... */} + +) +``` + +**Tip:** We usually wrap the data provider's methods in the same way. You can use the [`addRefreshAuthToDataProvider`](./addRefreshAuthToDataProvider.md) helper function to do so. + +## `provider` + +The first argument must be a valid `authProvider` object - for instance, [any third-party auth provider](./AuthProviderList.md). + +```jsx +// in src/authProvider.js +import { addRefreshAuthToAuthProvider } from 'react-admin'; + +const myAuthProvider = { + // ...Usual AuthProvider methods +}; + +export const authProvider = addRefreshAuthToAuthProvider(myAuthProvider, [ /* refreshAuth function */ ]); +``` + +## `refreshAuth` + +The second argument is a function responsible for refreshing the authentication tokens if needed. It must return a promise. + +```jsx +import { refreshAuth } from "./refreshAuth"; + +export const authProvider = addRefreshAuthToAuthProvider(myAuthProvider, refreshAuth); +``` + +## See Also + +- [`addRefreshAuthToDataProvider`](./addRefreshAuthToDataProvider.md) diff --git a/docs/withRefreshAuth.md b/docs/addRefreshAuthToDataProvider.md similarity index 51% rename from docs/withRefreshAuth.md rename to docs/addRefreshAuthToDataProvider.md index ada321c02fc..99ba93a8bd8 100644 --- a/docs/withRefreshAuth.md +++ b/docs/addRefreshAuthToDataProvider.md @@ -1,15 +1,15 @@ --- layout: default -title: "withRefreshAuth" +title: "addRefreshAuthToDataProvider" --- -# `withRefreshAuth` +# `addRefreshAuthToDataProvider` -This helper function wraps existing [`dataProviders`](./DataProviderIntroduction.md) and [`authProviders`]('./Authentication.md') to support authentication token refreshing mechanisms. +This helper function wraps an existing [`dataProvider`](./DataProviderIntroduction.md) to support authentication token refreshing mechanisms. ## Usage -Use `withRefreshAuth` to decorate an existing data provider or auth provider (usually both). In addition to the base provider, this function takes a function responsible for refreshing the authentication token if needed. +Use `addRefreshAuthToDataProvider` to decorate an existing data provider. In addition to the base provider, this function takes a function responsible for refreshing the authentication token if needed. Here is a simple example that refreshes an expired JWT token when needed: @@ -28,52 +28,43 @@ export const refreshAuth = () => { } // in src/dataProvider.js -import { withRefreshAuth } from 'react-admin'; +import { addRefreshAuthToDataProvider } from 'react-admin'; import simpleRestProvider from 'ra-data-simple-rest'; import { refreshAuth } from 'refreshAuth'; const baseDataProvider = simpleRestProvider('http://path.to.my.api/'); -export const dataProvider = withRefreshAuth(baseDataProvider, refreshAuth); - -// in src/authProvider.js -import { withRefreshAuth } from 'react-admin'; -import { refreshAuth } from 'refreshAuth'; - -const myAuthProvider = { - // ...Usual AuthProvider methods -}; - -export const authProvider = withRefreshAuth(myAuthProvider, refreshAuth); +export const dataProvider = addRefreshAuthToDataProvider(baseDataProvider, refreshAuth); ``` -Then, inject the decorated providers in the `` component: +Then, inject the decorated provider in the `` component: ```jsx // in src/App.js import { Admin } from 'react-admin'; -import { authProvider } from './authProvider'; import { dataProvider } from './dataProvider'; export const App = () => ( - + {/* ... */} ) ``` +**Tip:** We usually wrap the auth provider's methods in the same way. You can use the [`addRefreshAuthToAuthProvider`](./addRefreshAuthToAuthProvider.md) helper function to do so. + ## `provider` -The first argument must be a valid `dataProvider` or `authProvider` object - for instance, [any third-party data provider](./DataProviderList.md). +The first argument must be a valid `dataProvider` object - for instance, [any third-party data provider](./DataProviderList.md). ```jsx // in src/dataProvider.js -import { withRefreshAuth } from 'react-admin'; +import { addRefreshAuthToDataProvider } from 'react-admin'; import simpleRestProvider from 'ra-data-simple-rest'; const baseDataProvider = simpleRestProvider('http://path.to.my.api/'); -export const dataProvider = withRefreshAuth(baseDataProvider, [ /* refreshAuth function */ ]); +export const dataProvider = addRefreshAuthToDataProvider(baseDataProvider, [ /* refreshAuth function */ ]); ``` ## `refreshAuth` @@ -84,5 +75,9 @@ The second argument is a function responsible for refreshing the authentication import jsonServerProvider from "ra-data-json-server"; import { refreshAuth } from "./refreshAuth"; -export const dataProvider = withRefreshAuth(baseDataProvider, refreshAuth); +export const dataProvider = addRefreshAuthToDataProvider(baseDataProvider, refreshAuth); ``` + +## See Also + +- [`addRefreshAuthToAuthProvider`](./addRefreshAuthToAuthProvider.md) diff --git a/docs/navigation.html b/docs/navigation.html index 38a9e982a83..663e89fc479 100644 --- a/docs/navigation.html +++ b/docs/navigation.html @@ -56,7 +56,8 @@
    • usePermissions
    • useCanAccess
    • canAccess
    • -
    • withRefreshAuth
    • +
    • addRefreshAuthToAuthProvider
    • +
    • addRefreshAuthToDataProvider