Fix DeleteButton undoable={false} in List does not refresh List

[BartoGabriel]

After making an delete, by default you must refresh.

Closes #2425

[fzaninotto]

Can you describe the bug you’re trying to fix ?

[BartoGabriel]

Sorry, I put it in the commit message. #2425

[normanzb]

the same change need to be done in crudUpdate as well.

[normanzb]

i think the complete fix for this issue should:

1. add refresh in crudDelete and crudUpdate
2. remove the refresh here

   react-admin/packages/ra-core/src/sideEffect/undo.ts

   Line 41 in 29fc75e

   onSuccess: { refresh: true },

The refresh of ui shouldn't be triggered by undo effect, it should be triggered by the event of "data updated".
Without removing bullet point 2 above, I am afraid the refresh will be triggered twice when undoable is enabled?

[te-online]

Thanks for writing a patch for that issue @BartoGabriel 🎉 I can confirm that it solves the problem outlined in #2425.

@normanzb Sounds plausible. But with the crudUpdate I never noticed that the list is not refreshed, because I normally don't update any record while being on the list. The fact, that I'm changing routes when updating an element, ensures that the list is refreshed.

When using the delete button on the list, the exact same route is pushed to the router which ignores the request, because we're already on that page and hence doesn't trigger a reload.

As to your fear of triggering refresh twice: I couldn't reproduce that. With or without undo, it seems to refresh just once.

Until this is merged, I found that you can solve the problem with a dirty trick (that introduces another drawback – you decide which weighs more on your users): Create a custom copy of the DeleteButton and instead of calling dispatchCrudDelete(resource, record.id, record, basePath, redirect) when undoable is false, you run startUndoable(crudDelete(resource, record.id, record, basePath, redirect)), and immediately after that call complete();. This directly executes the action, refreshes the list, but still displays the Undo button on the toast message...

@fzaninotto Is there anything aside from your review that's preventing this PR from being merged?

[djhi]

This directly executes the action, refreshes the list, but still displays the Undo button on the toast message...

It displays the undo but it won't have the expected effect as the underlying delete action will have been dispatched

[te-online]

It displays the undo but it won't have the expected effect as the underlying delete action will have been dispatched

Exactly :-) That's why it's a drawback – it suggests you can undo the action, but you can't. I wasn't clear on that, thanks for pointing it out.

[normanzb]

But with the crudUpdate I never noticed that the list is not refreshed, because I normally don't update any record while being on the list.

That does happens to me on edit view.

I have a custom made edit view, for my edit view, I prefer to stay on edit view once the editing is finished. if I enable undoable for that edit view, even after the update is sent and responsed, the interface stay unchanged.

Anyway I think it would be nice to keep the code consistent:

1. If the undo notification snackbar triggers refresh after edit, why not do it the same when the snackbar is disabled? Right now if you do undoable=true on edit view the interface updates just fine, but if you do undoable=false the interface stay unchanged, the behaviour doesn't sound very consistent.
2. Why would a trivial ui component "undo snackbar" is made to dictate when the interface should be refreshed. Shouldn't that be made driven by the data change itself?
3. Why would crudDelete triggers refresh but crudUpdate doesn't triggers the refresh? Again it sounds inconsistent.

[fzaninotto]

This will add a useless rerender even on optimistic actions, but it's the proper way to fix the bug.

[fzaninotto]

Thanks!