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

Custom save doesn't redirect onSuccess due to warnWhenUnsavedChanges #7608

Open
nathan-britten opened this issue Apr 29, 2022 · 13 comments
Open

Comments

@nathan-britten
Copy link

What you were expecting:
When I submit the form the data is saved, the user is notified, and then the user is redirected to the specified page

What happened instead:
The notification is fired however the user is not redirected

Steps to reproduce:

  1. Navigate to https://codesandbox.io/s/nervous-zeh-bxj6ym?file=/src/posts/PostEdit.tsx:2071-2093
  2. Open a post
  3. Edit the title
  4. Click Custom Save
  5. Notification is fired but you are not redirected.

Then remove warnWhenUnsavedChanges from the tabbed form.

Repeat the steps and you will be redirected.

Related code:
https://codesandbox.io/s/nervous-zeh-bxj6ym?file=/src/posts/PostEdit.tsx:2071-2093

Other information:

It appears that warnWhenUnsavedChanges is interfering with the redirection of the page

Environment

  • React-admin version: 4.0.1
  • Last version that did not exhibit the issue (if applicable):
  • React version: 17.0.2
  • Browser: Chrome
  • Stack trace (in case of a JS error):
@slax57
Copy link
Contributor

slax57 commented May 2, 2022

Reproduced, thanks!

@slax57 slax57 added the bug label May 2, 2022
@djhi
Copy link
Collaborator

djhi commented May 3, 2022

In the case you return a promise, you should handle the side effects when the promise resolves:

update(
    'posts',
    {
        id: values.id,
        data: values,
    },
    {
        returnPromise: true,
        mutationMode: 'pessimistic',
        onError: error => {
            notify((error as Error).message, { type: 'error' });
        },
    }
).then(() => {
    notify('it worked');
    redirect('show', 'posts', values.id);
});

I'm marking this as a documentation issue

@Oktay28
Copy link

Oktay28 commented Jun 1, 2022

In the case you return a promise, you should handle the side effects when the promise resolves:

update(
    'posts',
    {
        id: values.id,
        data: values,
    },
    {
        returnPromise: true,
        mutationMode: 'pessimistic',
        onError: error => {
            notify((error as Error).message, { type: 'error' });
        },
    }
).then(() => {
    notify('it worked');
    redirect('show', 'posts', values.id);
});

I'm marking this as a documentation issue

How this works together with server side validation?
https://marmelab.com/react-admin/Validation.html#server-side-validation

If I await update and return errors it does not redirect, if I don't await update it does not show server side errors

@FlorianChristie
Copy link

Is there any way to make this work together with server side validation? I would also be very much interested in that. 😄

@megantaylor
Copy link
Contributor

i'm seeing this problem with a create form where i want to redirect to the edit after a custom save and it only works without warnWhenUnsavedChanges.

i am doing

dataProvider.create(resource, {data}).then(()=>{
// sideeffects
})

@abhijithp05
Copy link

Is this issue still open?

@megantaylor
Copy link
Contributor

it should be, i am still having the same issues with warnWhenUnsavedChanges and custom saves

@archfz
Copy link

archfz commented Mar 23, 2023

I was experiencing the same issue. I think the redirect navigation is attempted at a point where the form is still blocked for navigating, since it was not yet registered that the form is not dirty anymore. So using patch package I made the following modifications in useEditController and useCreateController.

@@ -151,7 +152,7 @@ export var useEditController = function (props) {
                             messageArgs: { smart_count: 1 },
                             undoable: mutationMode === 'undoable',
                         });
-                        redirect(redirectTo, resource, data.id, data);
+                        setTimeout(() => redirect(redirectTo, resource, data.id, data), 50);
                         return [2 /*return*/];
                     });
                 }); },

@PawelSuwinski
Copy link

PawelSuwinski commented Apr 4, 2023

I have the same with mutationOptions={{ onSuccess }}. An example from the docs (https://marmelab.com/react-admin/Edit.html#mutationoptions) doesn't work if warnWhenUnsavedChanges is on. Only redirecting after render is done in useEffect() makes it works together, ex:

[changed, setChanged] = useState();
const onSuccess = () => {
      // (...)
      setChanged(true);
}
useEffect(() => {
      changed && redirect('/posts');
 }, [changed]);
<Edit mutationOptions={{ onSuccess }} warnWhenUnsavedChanges={true} >
//(...)

@slax57
Copy link
Contributor

slax57 commented Apr 18, 2023

EDIT: per #7608 (comment) it seems like I still haven't figured out 100% of the issue.

Especially the following assumption I made, is wrong:

in pessimistic mode the side-effects are executed before the dataProvider call resolves

So please keep a critical mind when reading my answer below 😇


Thought I'd add some clarifications about this issue.

You will run into this issue if both conditions are true:

  • You are inside a Create, or inside an Edit with mutationMode='pessimistic'
  • You set warnWhenUnsavedChanges to true on the form

If you provide neither custom mutationOptions nor custom onSubmit, then you should no longer run into this issue once #8839 has been merged.

If you provide a custom onSuccess side-effect, either via the mutationOptions prop or the onSubmit prop, then you need to move this side-effect into a .then(), or await the result of the create or the update before triggering the side-effects, because in pessimistic mode the side-effects are executed before the dataProvider call resolves.

Regarding the questions about server-side validation, I'm not sure I understand the issue there, but to my understanding you can handle the errors in a catch block:

const update = useUpdate(resource, data, {mutationMode: 'pessimistic', returnPromise: true});

try {
  const result = await update(resource, data)
  console.log(result)
} catch (error) {
  // handle server side errors here
  console.error(error)
} finally {
  console.log('done')
}

Hope this helps!

@fzaninotto
Copy link
Member

in pessimistic mode the side-effects are executed before the dataProvider call resolves.

This is counter-intuitive. Are you sure about this one?

@slax57
Copy link
Contributor

slax57 commented Apr 20, 2023

@fzaninotto I have double-checked, and indeed it appears I was wrong about this!!
Hence my PR is probably targeting the wrong culprit, I need to push the analysis further...
Thanks for pointing this out!

@slax57
Copy link
Contributor

slax57 commented May 5, 2023

Now that #8882 has been merged I'm wondering if we can consider this issue fixed as well.

Can s.o. give it a try to check if their issue is resolved or not?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants