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.

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

Handle submission validation errors #5778

Merged
merged 1 commit into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
43 changes: 42 additions & 1 deletion docs/CreateEdit.md
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ export const PostCreate = (props) => (
);
```

**Tip**: `Create` and `Edit` inject more props to their child. So `SimpleForm` also expects these props to be set (but you shouldn't set them yourself):
**Tip**: `Create` and `Edit` inject more props to their child. So `SimpleForm` also expects these props to be set (you should set them yourself only in particular cases like the [submission validation](#submission-validation)):

* `save`: The function invoked when the form is submitted.
* `saving`: A boolean indicating whether a save operation is ongoing.
Expand Down Expand Up @@ -1329,6 +1329,47 @@ export const UserCreate = (props) => (

**Important**: Note that asynchronous validators are not supported on the `<ArrayInput>` component due to a limitation of [react-final-form-arrays](https://github.com/final-form/react-final-form-arrays).

## Submission Validation

The form can be validated by the server after its submission. In order to display the validation errors, a custom `save` function needs to be used:

{% raw %}
```jsx
import { useMutation } from 'react-admin';

export const UserCreate = (props) => {
const [mutate] = useMutation();
const save = useCallback(
async (values) => {
try {
await mutate({
type: 'create',
resource: 'users',
payload: { data: values },
}, { returnPromise: true });
} catch (error) {
if (error.body.errors) {
return error.body.errors;
}
}
},
[mutate],
);

return (
<Create undoable={false} {...props}>
<SimpleForm save={save}>
<TextInput label="First Name" source="firstName" />
<TextInput label="Age" source="age" />
</SimpleForm>
</Create>
);
};
```
{% endraw %}

**Tip**: The shape of the returned validation errors must correspond to the form: a key needs to match a `source` prop.

## Submit On Enter

By default, pressing `ENTER` in any of the form fields submits the form - this is the expected behavior in most cases. However, some of your custom input components (e.g. Google Maps widget) may have special handlers for the `ENTER` key. In that case, to disable the automated form submission on enter, set the `submitOnEnter` prop of the form component to `false`:
Expand Down
3 changes: 2 additions & 1 deletion packages/ra-core/src/dataProvider/Mutation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface Props {
event?: any,
callTimePayload?: any,
callTimeOptions?: any
) => void,
) => void | Promise<any>,
params: ChildrenFuncParams
) => JSX.Element;
type: string;
Expand All @@ -35,6 +35,7 @@ interface Props {
* @param {Object} options
* @param {string} options.action Redux action type
* @param {boolean} options.undoable Set to true to run the mutation locally before calling the dataProvider
* @param {boolean} options.returnPromise Set to true to return the result promise of the mutation
* @param {Function} options.onSuccess Side effect function to be executed upon success or failure, e.g. { onSuccess: response => refresh() }
* @param {Function} options.onFailure Side effect function to be executed upon failure, e.g. { onFailure: error => notify(error.message) }
*
Expand Down
40 changes: 40 additions & 0 deletions packages/ra-core/src/dataProvider/useMutation.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,44 @@ describe('useMutation', () => {
expect(action.meta.resource).toBeUndefined();
expect(dataProvider.mytype).toHaveBeenCalledWith(myPayload);
});

it('should return a promise to dispatch a fetch action when returnPromise option is set and the mutation callback is triggered', async () => {
const dataProvider = {
mytype: jest.fn(() => Promise.resolve({ data: { foo: 'bar' } })),
};

let promise = null;
const myPayload = {};
const { getByText, dispatch } = renderWithRedux(
<DataProviderContext.Provider value={dataProvider}>
<Mutation
type="mytype"
resource="myresource"
payload={myPayload}
options={{ returnPromise: true }}
>
{(mutate, { loading }) => (
<button
className={loading ? 'loading' : 'idle'}
onClick={() => (promise = mutate())}
>
Hello
</button>
)}
</Mutation>
</DataProviderContext.Provider>
);
const buttonElement = getByText('Hello');
fireEvent.click(buttonElement);
const action = dispatch.mock.calls[0][0];
expect(action.type).toEqual('CUSTOM_FETCH');
expect(action.payload).toEqual(myPayload);
expect(action.meta.resource).toEqual('myresource');
await waitFor(() => {
expect(buttonElement.className).toEqual('idle');
});
expect(promise).toBeInstanceOf(Promise);
const result = await promise;
expect(result).toMatchObject({ data: { foo: 'bar' } });
});
});
27 changes: 23 additions & 4 deletions packages/ra-core/src/dataProvider/useMutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import useDataProviderWithDeclarativeSideEffects from './useDataProviderWithDecl
* @param {Object} options
* @param {string} options.action Redux action type
* @param {boolean} options.undoable Set to true to run the mutation locally before calling the dataProvider
* @param {boolean} options.returnPromise Set to true to return the result promise of the mutation
* @param {Function} options.onSuccess Side effect function to be executed upon success or failure, e.g. { onSuccess: response => refresh() }
* @param {Function} options.onFailure Side effect function to be executed upon failure, e.g. { onFailure: error => notify(error.message) }
* @param {boolean} options.withDeclarativeSideEffectsSupport Set to true to support legacy side effects e.g. { onSuccess: { refresh: true } }
Expand All @@ -52,6 +53,7 @@ import useDataProviderWithDeclarativeSideEffects from './useDataProviderWithDecl
* - {Object} options
* - {string} options.action Redux action type
* - {boolean} options.undoable Set to true to run the mutation locally before calling the dataProvider
* - {boolean} options.returnPromise Set to true to return the result promise of the mutation
* - {Function} options.onSuccess Side effect function to be executed upon success or failure, e.g. { onSuccess: response => refresh() }
* - {Function} options.onFailure Side effect function to be executed upon failure, e.g. { onFailure: error => notify(error.message) }
* - {boolean} withDeclarativeSideEffectsSupport Set to true to support legacy side effects e.g. { onSuccess: { refresh: true } }
Expand Down Expand Up @@ -140,7 +142,7 @@ const useMutation = (
(
callTimeQuery?: Mutation | Event,
callTimeOptions?: MutationOptions
): void => {
): void | Promise<any> => {
const finalDataProvider = hasDeclarativeSideEffectsSupport(
options,
callTimeOptions
Expand All @@ -156,21 +158,27 @@ const useMutation = (

setState(prevState => ({ ...prevState, loading: true }));

finalDataProvider[params.type]
const returnPromise = params.options.returnPromise;

const promise = finalDataProvider[params.type]
.apply(
finalDataProvider,
typeof params.resource !== 'undefined'
? [params.resource, params.payload, params.options]
: [params.payload, params.options]
)
.then(({ data, total }) => {
.then(response => {
const { data, total } = response;
setState({
data,
error: null,
loaded: true,
loading: false,
total,
});
if (returnPromise) {
return response;
}
})
.catch(errorFromResponse => {
setState({
Expand All @@ -180,7 +188,14 @@ const useMutation = (
loading: false,
total: null,
});
if (returnPromise) {
throw errorFromResponse;
}
});

if (returnPromise) {
return promise;
}
},
[
// deep equality, see https://github.com/facebook/react/issues/14476#issuecomment-471199055
Expand All @@ -204,13 +219,17 @@ export interface Mutation {
export interface MutationOptions {
action?: string;
undoable?: boolean;
returnPromise?: boolean;
onSuccess?: (response: any) => any | Object;
onFailure?: (error?: any) => any | Object;
withDeclarativeSideEffectsSupport?: boolean;
}

export type UseMutationValue = [
(query?: Partial<Mutation>, options?: Partial<MutationOptions>) => void,
(
query?: Partial<Mutation>,
options?: Partial<MutationOptions>
) => void | Promise<any>,
{
data?: any;
total?: number;
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-core/src/form/FormWithRedirect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ const FormWithRedirect: FC<FormWithRedirectProps> = ({
finalInitialValues,
values
);
onSave.current(sanitizedValues, finalRedirect);
return onSave.current(sanitizedValues, finalRedirect);
} else {
onSave.current(values, finalRedirect);
return onSave.current(values, finalRedirect);
}
};

Expand Down
12 changes: 8 additions & 4 deletions packages/ra-core/src/form/useInitializeFormWithRecord.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@ const useInitializeFormWithRecord = record => {
// Disable this option when re-initializing the form because in this case, it should reset the dirty state of all fields
// We do need to keep this option for dynamically added inputs though which is why it is kept at the form level
form.setConfig('keepDirtyOnReinitialize', false);
// Ignored until next version of final-form is released. See https://github.com/final-form/final-form/pull/376
// @ts-ignore
form.restart(initialValuesMergedWithRecord);
form.setConfig('keepDirtyOnReinitialize', true);
// Since the submit function returns a promise, use setTimeout to prevent the error "Cannot reset() in onSubmit()" in final-form
// It will not be necessary anymore when the next version of final-form will be released (see https://github.com/final-form/final-form/pull/363)
setTimeout(() => {
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 really concerning to me. Any error in the form.restart() won't be caught.

Can you explain why the error occurs, why the setTimeout removes it, and check if there is no other way? And can you add a unit test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the way final-form is telling to do it: https://github.com/final-form/final-form/blob/ee1ef7272882a966644ca1bf0ea6f115a2492251/src/FinalForm.js#L933.
However this constraint seems to have been removed in the next version:
final-form/final-form#363
I don't see how we could add a unit test for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a comment to explain that the setTimeout should be removed when the next version of final-form will be released.
And actually there is already a test for this:

it('should not display a warning about unsaved changes when an array input has been updated', () => {
ListPagePosts.navigate();
ListPagePosts.nextPage(); // Ensure the record is visible in the table
EditPostPage.navigate();
// Select first notification input checkbox
cy.get(
EditPostPage.elements.input('notifications', 'checkbox-group-input')
)
.eq(0)
.click();
EditPostPage.submit();
// If the update succeeded without display a warning about unsaved changes,
// we should have been redirected to the list
cy.url().then(url => expect(url).to.contain('/#/posts'));
});

(it throws the linked error without the setTimeout)

// Ignored until next version of final-form is released. See https://github.com/final-form/final-form/pull/376
// @ts-ignore
form.restart(initialValuesMergedWithRecord);
form.setConfig('keepDirtyOnReinitialize', true);
});
}, [form, JSON.stringify(record)]); // eslint-disable-line react-hooks/exhaustive-deps
};

Expand Down
10 changes: 8 additions & 2 deletions packages/ra-ui-materialui/src/detail/CreateView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ export const CreateView = (props: CreateViewProps) => {
? redirect
: children.props.redirect,
resource,
save,
save:
typeof children.props.save === 'undefined'
? save
: children.props.save,
saving,
version,
})}
Expand All @@ -76,7 +79,10 @@ export const CreateView = (props: CreateViewProps) => {
basePath,
record,
resource,
save,
save:
typeof children.props.save === 'undefined'
? save
: children.props.save,
saving,
version,
})}
Expand Down
10 changes: 8 additions & 2 deletions packages/ra-ui-materialui/src/detail/EditView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ export const EditView = (props: EditViewProps) => {
? redirect
: children.props.redirect,
resource,
save,
save:
typeof children.props.save === 'undefined'
? save
: children.props.save,
saving,
undoable,
version,
Expand All @@ -102,7 +105,10 @@ export const EditView = (props: EditViewProps) => {
record,
resource,
version,
save,
save:
typeof children.props.save === 'undefined'
? save
: children.props.save,
saving,
})}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ const AutocompleteArrayInput: FunctionComponent<
id,
input,
isRequired,
meta: { touched, error },
meta: { touched, error, submitError },
} = useInput({
format,
id: idOverride,
Expand Down Expand Up @@ -427,7 +427,7 @@ const AutocompleteArrayInput: FunctionComponent<
},
onFocus,
}}
error={!!(touched && error)}
error={!!(touched && (error || submitError))}
label={
<FieldTitle
label={label}
Expand All @@ -448,7 +448,7 @@ const AutocompleteArrayInput: FunctionComponent<
helperText={
<InputHelperText
touched={touched}
error={error}
error={error || submitError}
helperText={helperText}
/>
}
Expand Down
6 changes: 3 additions & 3 deletions packages/ra-ui-materialui/src/input/AutocompleteInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ const AutocompleteInput: FunctionComponent<AutocompleteInputProps> = props => {
id,
input,
isRequired,
meta: { touched, error },
meta: { touched, error, submitError },
} = useInput({
format,
id: idOverride,
Expand Down Expand Up @@ -488,7 +488,7 @@ const AutocompleteInput: FunctionComponent<AutocompleteInputProps> = props => {
onFocus,
...InputPropsWithoutEndAdornment,
}}
error={!!(touched && error)}
error={!!(touched && (error || submitError))}
label={
<FieldTitle
label={label}
Expand All @@ -509,7 +509,7 @@ const AutocompleteInput: FunctionComponent<AutocompleteInputProps> = props => {
helperText={
<InputHelperText
touched={touched}
error={error}
error={error || submitError}
helperText={helperText}
/>
}
Expand Down
6 changes: 3 additions & 3 deletions packages/ra-ui-materialui/src/input/BooleanInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const BooleanInput: FunctionComponent<
id,
input: { onChange: finalFormOnChange, type, value, ...inputProps },
isRequired,
meta: { error, touched },
meta: { error, submitError, touched },
} = useInput({
format,
onBlur,
Expand Down Expand Up @@ -77,10 +77,10 @@ const BooleanInput: FunctionComponent<
/>
}
/>
<FormHelperText error={!!error}>
<FormHelperText error={!!(error || submitError)}>
<InputHelperText
touched={touched}
error={error}
error={error || submitError}
helperText={helperText}
/>
</FormHelperText>
Expand Down
6 changes: 3 additions & 3 deletions packages/ra-ui-materialui/src/input/CheckboxGroupInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ const CheckboxGroupInput: FunctionComponent<
id,
input: { onChange: finalFormOnChange, onBlur: finalFormOnBlur, value },
isRequired,
meta: { error, touched },
meta: { error, submitError, touched },
} = useInput({
format,
onBlur,
Expand Down Expand Up @@ -195,7 +195,7 @@ const CheckboxGroupInput: FunctionComponent<
<FormControl
component="fieldset"
margin={margin}
error={touched && !!error}
error={touched && !!(error || submitError)}
className={classnames(classes.root, className)}
{...sanitizeRestProps(rest)}
>
Expand Down Expand Up @@ -225,7 +225,7 @@ const CheckboxGroupInput: FunctionComponent<
<FormHelperText>
<InputHelperText
touched={touched}
error={error}
error={error || submitError}
helperText={helperText}
/>
</FormHelperText>
Expand Down
Loading