-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Conversation
b7b25e0
to
1f22453
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Can you add some usage documentation in https://marmelab.com/react-admin/CreateEdit.html#validation, and a unit test in useMutation.spec.tsx
?
ef37260
to
4714fd4
Compare
PR has been updated to add:
I've tested it on a project. With this code: const Create = (props) => {
const [mutate] = useMutation()
const save = useCallback(
async (values) => {
try {
await mutate({
type: 'create',
resource: 'origins',
payload: { data: values },
}, { returnPromise: true });
} catch (error) {
return {
url: 'a custom server error',
};
}
},
[mutate],
)
return (
<Wrapper>
<RACreate undoable={false} {...props}>
<SimpleForm toolbar={<Toolbar />} save={save} redirect='/origins'>
<InputGuesser source='name' fullWidth variant='outlined' />
<InputGuesser source='url' fullWidth variant='outlined' />
<InputGuesser source='comment' fullWidth variant='outlined' />
</SimpleForm>
</RACreate>
</Wrapper>
)
} The result is: submission_errors.mp4 |
959ea9a
to
964b7ab
Compare
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 | ||
setTimeout(() => { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
react-admin/cypress/integration/edit.js
Lines 260 to 277 in baaf3da
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
)
964b7ab
to
fab3760
Compare
fab3760
to
19e3c5c
Compare
Indeed. Looking into it |
19e3c5c
to
fffdd9d
Compare
Thank you very much! |
Fixes #4703.
Fixes #4351.
Returning a promise in the
submit
function is essential for handling correctly submission errors: https://final-form.org/docs/react-final-form/types/FormProps#3-asynchronous-with-a-promise.It has been discussed in this issue too: #4351.
useCreateController
anduseEditController
hooks are already returning a promise resolving toundefined
for thesave
function: it will not break anything.To handle submission errors, the custom
save
function could be implemented by using theuseDataProvider
hook, like this: bmihelac@1a9f02b.In order to be able to use the
useMutation
hook too, areturnPromise
option has been added, like asked in @fzaninotto's comment: #4703 (comment).I hope it will be OK, it would be really nice to have a way to handle submission errors easily in React-Admin and API Platform Admin!