Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add server side validation support #7938

Merged
merged 9 commits into from
Feb 24, 2023
Merged

Add server side validation support #7938

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
267e755
Add server side validation support
djhi Jul 7, 2022
f96df58
Fix tests
djhi Jul 7, 2022
fa7b88d
Apply changes after review
djhi Jul 7, 2022
0625ac2
Apply review comments
djhi Jul 29, 2022
bb4ed6e
Merge remote-tracking branch 'origin/next' into refactor-saving-status
slax57 Feb 15, 2023
26ba43d
code review
slax57 Feb 15, 2023
71fa7de
fix doc about translation objects support
slax57 Feb 15, 2023
ba28552
fix E2E test
slax57 Feb 15, 2023
03c8bac
Mention server validation only works in pessimistic mode
djhi Feb 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 38 additions & 34 deletions docs/Validation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that work even in optimistic mode?


{% 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 (
<Create>
<SimpleForm onSubmit={save}>
<TextInput label="First Name" source="firstName" />
<TextInput label="Age" source="age" />
</SimpleForm>
</Create>
);
};
```
{% endraw %}
{
body: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the "body" key? Why can't the dataProvider only reject with ({ errors: { source: 'error message' } }?

Also, your snippet is both a type description and an example, and so it's not enough informative. I'd use a more complete example:

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that's the current shape of our HttpError. Our data providers put the response json inside the body property of the error they throw

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.
slax57 marked this conversation as resolved.
Show resolved Hide resolved

**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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't super clear

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to make this section clearer. Please let me know if that's enough.


Here's an example of a dataProvider not using our `httpClient`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, this is confusing. You don't give an eample of the general case but you give one of a particular case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same


```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;
}
}
```
10 changes: 8 additions & 2 deletions examples/simple/src/dataProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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', 400, {
errors: {
title: 'this title cannot be used',
},
})
);
}
return uploadCapableDataProvider[name](resource, params);
},
Expand Down
31 changes: 31 additions & 0 deletions packages/ra-core/src/controller/create/useCreateController.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
SaveContextProvider,
useRegisterMutationMiddleware,
} from '../saveContext';
import { DataProvider } from '../..';

describe('useCreateController', () => {
describe('getRecordFromLocation', () => {
Expand Down Expand Up @@ -502,4 +503,34 @@ describe('useCreateController', () => {
expect.any(Function)
);
});

it('The save function should return errors from the create call', async () => {
djhi marked this conversation as resolved.
Show resolved Hide resolved
const create = jest.fn().mockImplementationOnce(() => {
return Promise.reject({ body: { errors: { foo: 'invalid' } } });
});
const dataProvider = ({
create,
} as unknown) as DataProvider;
let saveCallback;
render(
<CoreAdminContext dataProvider={dataProvider}>
<CreateController {...defaultProps}>
{({ save, record }) => {
saveCallback = save;
return <div />;
}}
</CreateController>
</CoreAdminContext>
);
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));
djhi marked this conversation as resolved.
Show resolved Hide resolved
expect(create).toHaveBeenCalledWith('posts', {
data: { foo: 'bar' },
});
});
});
112 changes: 64 additions & 48 deletions packages/ra-core/src/controller/create/useCreateController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
(
Expand All @@ -91,55 +95,67 @@ export const useCreateController = <
: transform
? transform(data)
: data
).then((data: Partial<RecordType>) => {
).then(async (data: Partial<RecordType>) => {
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird: the save promise resolves when there is a validation error in the save process? I'd expect it to reject.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how the useAugmentedForm has been implemented. If the submit function returns something, we consider this is an error object. This was done like this probably because it worked that way in final-form.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we improve that in a backward compatible way?

}
);
}
djhi marked this conversation as resolved.
Show resolved Hide resolved
}),
[
create,
Expand Down
36 changes: 36 additions & 0 deletions packages/ra-core/src/controller/edit/useEditController.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({ body: { errors: { foo: 'invalid' } } });
});
const dataProvider = ({
getOne: () => Promise.resolve({ data: post }),
update,
} as unknown) as DataProvider;
let saveCallback;
render(
<CoreAdminContext dataProvider={dataProvider}>
<EditController {...defaultProps} mutationMode="pessimistic">
{({ save, record }) => {
saveCallback = save;
return <>{JSON.stringify(record)}</>;
}}
</EditController>
</CoreAdminContext>
);
await new Promise(resolve => setTimeout(resolve, 10));
screen.getByText('{"id":12}');
djhi marked this conversation as resolved.
Show resolved Hide resolved
let errors;
await act(async () => {
errors = await saveCallback({ foo: 'bar' });
});
expect(errors).toEqual({ foo: 'invalid' });
await new Promise(resolve => setTimeout(resolve, 10));
djhi marked this conversation as resolved.
Show resolved Hide resolved
screen.getByText('{"id":12}');
expect(update).toHaveBeenCalledWith('posts', {
id: 12,
data: { foo: 'bar' },
previousData: { id: 12 },
});
});
});
Loading