Skip to content

Commit f8bfc09

Browse files
committed
Fix WarnWhenUnsavedChanges alert after deleting a record
Closes #11274 ## Problem The warning cannot prevent the deletion, therefore when the user clicks on cancel, they see a non-coherent state. ## Solution Disable the warnwhenunsavedchanges warning when a deletion was requested
1 parent bdce64d commit f8bfc09

7 files changed

Lines changed: 299 additions & 13 deletions

File tree

packages/ra-core/src/controller/button/useDeleteController.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useCallback, useMemo } from 'react';
2+
import { useFormContext } from 'react-hook-form';
23

34
import { useDelete, UseDeleteOptions } from '../../dataProvider/useDelete';
45
import { useUnselect } from '../list/useUnselect';
@@ -75,6 +76,8 @@ export const useDeleteController = <
7576
const unselect = useUnselect(resource);
7677
const redirect = useRedirect();
7778
const translate = useTranslate();
79+
// null when the delete button is used outside a form (e.g. in a list)
80+
const form = useFormContext();
7881

7982
const [deleteOne, { isPending }] = useDelete<RecordType, ErrorType>(
8083
resource,
@@ -96,6 +99,11 @@ export const useDeleteController = <
9699
}
97100
);
98101
record && unselect([record.id], true);
102+
// Clear the form dirty state so warnWhenUnsavedChanges doesn't
103+
// block the redirect: the record is gone, so the unsaved
104+
// changes are moot. keepValues avoids a flash of reverted
105+
// fields before the redirect commits.
106+
form?.reset(undefined, { keepValues: true });
99107
redirect(redirectTo, resource);
100108
},
101109
onError: (error: any) => {

packages/ra-core/src/form/Form.spec.tsx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
InNonDataRouter,
2323
ServerSideValidation,
2424
MultiRoutesForm,
25+
WarnWhenUnsavedChangesWithDelete,
2526
} from './Form.stories';
2627
import { mergeTranslations } from '../i18n';
2728

@@ -1008,4 +1009,37 @@ describe('Form', () => {
10081009
).toEqual(false);
10091010
}
10101011
);
1012+
1013+
describe('warnWhenUnsavedChanges with a delete button', () => {
1014+
it.each(['undoable', 'pessimistic', 'optimistic'] as const)(
1015+
'should not warn about unsaved changes after deleting a dirty record (%s)',
1016+
async mutationMode => {
1017+
// spy on "cancel": if the dialog were shown the navigation
1018+
// would be cancelled, failing the redirect assertion
1019+
const confirmSpy = jest
1020+
.spyOn(window, 'confirm')
1021+
.mockReturnValue(false);
1022+
render(
1023+
<WarnWhenUnsavedChangesWithDelete
1024+
mutationMode={mutationMode}
1025+
/>
1026+
);
1027+
// wait for the record to load
1028+
const input =
1029+
await screen.findByDisplayValue<HTMLInputElement>('Hello');
1030+
// make the form dirty
1031+
fireEvent.change(input, {
1032+
target: { value: 'Hello modified' },
1033+
});
1034+
fireEvent.blur(input);
1035+
// delete the record (the headless DeleteButton has no
1036+
// confirmation dialog and always uses the controller onSuccess)
1037+
fireEvent.click(screen.getByText('Delete'));
1038+
// the app should redirect to the list without warning
1039+
await screen.findByText('Post list');
1040+
expect(confirmSpy).not.toHaveBeenCalled();
1041+
confirmSpy.mockRestore();
1042+
}
1043+
);
1044+
});
10111045
});

packages/ra-core/src/form/Form.stories.tsx

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { yupResolver } from '@hookform/resolvers/yup';
1010
import * as yup from 'yup';
1111
import polyglotI18nProvider from 'ra-i18n-polyglot';
1212
import englishMessages from 'ra-language-english';
13+
import fakeRestDataProvider from 'ra-data-fakerest';
1314
import {
1415
Route,
1516
Routes,
@@ -20,7 +21,12 @@ import {
2021
} from 'react-router-dom';
2122

2223
import { CoreAdminContext } from '../core';
23-
import { RecordContextProvider, SaveContextProvider } from '../controller';
24+
import {
25+
EditBase,
26+
RecordContextProvider,
27+
SaveContextProvider,
28+
} from '../controller';
29+
import { DeleteButton } from '../test-ui/DeleteButton';
2430
import { Form, FormProps } from './Form';
2531
import { useInput } from './useInput';
2632
import { required, ValidationError } from './validation';
@@ -415,6 +421,69 @@ export const InNonDataRouter = ({
415421
</HashRouter>
416422
);
417423

424+
const PostEditWithDelete = ({
425+
mutationMode,
426+
}: {
427+
mutationMode: 'undoable' | 'optimistic' | 'pessimistic';
428+
}) => (
429+
<EditBase resource="posts" id={1} mutationMode={mutationMode}>
430+
<Form warnWhenUnsavedChanges>
431+
<Input source="title" />
432+
<Input source="body" />
433+
<DeleteButton
434+
label="Delete"
435+
mutationMode={mutationMode}
436+
redirect="/posts"
437+
/>
438+
<button type="submit">Submit</button>
439+
</Form>
440+
</EditBase>
441+
);
442+
443+
/**
444+
* Edit a field (making the form dirty), then click Delete. The record is
445+
* deleted and the app redirects to the list, but the unsaved-changes blocker
446+
* pops a spurious "unsaved changes" confirm dialog even though the record is
447+
* already gone.
448+
*/
449+
export const WarnWhenUnsavedChangesWithDelete = ({
450+
i18nProvider = defaultI18nProvider,
451+
mutationMode = 'undoable',
452+
}: {
453+
i18nProvider?: I18nProvider;
454+
mutationMode?: 'undoable' | 'optimistic' | 'pessimistic';
455+
}) => (
456+
<TestMemoryRouter initialEntries={['/posts/1']}>
457+
<CoreAdminContext
458+
dataProvider={fakeRestDataProvider(
459+
{
460+
posts: [
461+
{ id: 1, title: 'Hello', body: 'world' },
462+
{ id: 2, title: 'Foo', body: 'bar' },
463+
],
464+
},
465+
process.env.NODE_ENV !== 'test'
466+
)}
467+
i18nProvider={i18nProvider}
468+
>
469+
<Routes>
470+
<Route
471+
path="/posts/:id"
472+
element={<PostEditWithDelete mutationMode={mutationMode} />}
473+
/>
474+
<Route path="/posts" element={<p>Post list</p>} />
475+
</Routes>
476+
</CoreAdminContext>
477+
</TestMemoryRouter>
478+
);
479+
480+
WarnWhenUnsavedChangesWithDelete.argTypes = {
481+
mutationMode: {
482+
options: ['undoable', 'optimistic', 'pessimistic'],
483+
control: { type: 'select' },
484+
},
485+
};
486+
418487
const Notifications = () => {
419488
const { notifications } = useNotificationContext();
420489
return (

packages/ra-core/src/form/useWarnWhenUnsavedChanges.tsx

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,21 +40,24 @@ export const useWarnWhenUnsavedChanges = (
4040
});
4141

4242
useEffect(() => {
43-
if (blocker.state === 'blocked') {
44-
// Corner case: the blocker might be triggered by a redirect in the onSuccess side effect,
45-
// happening during the same tick the form is reset after a successful save.
46-
// In that case, the blocker will block but shouldNotBlock will be true one tick after.
47-
// If we are in that case, we can proceed immediately.
48-
if (shouldNotBlock) {
49-
blocker.proceed();
50-
return;
51-
}
43+
if (blocker.state !== 'blocked') return;
5244

53-
setShouldNotify(true);
45+
// Proceed automatically when the form becomes safe to leave while the
46+
// navigation is blocked. This covers two cases where the redirect and
47+
// the form reset happen during the same tick (and the reset may only be
48+
// reflected one tick after the block):
49+
// - a successful save resets the form;
50+
// - a successful delete resets the form (the record no longer exists,
51+
// so the unsaved changes are moot).
52+
if (shouldNotBlock) {
53+
blocker.proceed && blocker.proceed();
54+
return;
5455
}
55-
// This effect should only run when the blocker state changes, not when shouldNotBlock changes.
56+
57+
setShouldNotify(true);
58+
// Can't use blocker in the dependency array because it is not stable across rerenders
5659
// eslint-disable-next-line react-hooks/exhaustive-deps
57-
}, [blocker.state]);
60+
}, [blocker.state, shouldNotBlock]);
5861

5962
useEffect(() => {
6063
if (shouldNotify) {

packages/ra-ui-materialui/src/button/DeleteWithConfirmButton.spec.tsx

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
CoreAdminContext,
1212
MutationMode,
1313
testDataProvider,
14+
TestMemoryRouter,
1415
useNotificationContext,
1516
} from 'ra-core';
1617
import { createTheme, ThemeProvider } from '@mui/material/styles';
@@ -453,4 +454,109 @@ describe('<DeleteWithConfirmButton />', () => {
453454
const buttons = await screen.findAllByTestId('themed');
454455
expect(buttons[0].classList).toContain('MuiButton-outlined');
455456
});
457+
458+
describe('with warnWhenUnsavedChanges and a dirty form', () => {
459+
// Use the router-agnostic TestMemoryRouter + locationCallback (instead of
460+
// react-router's Routes) so the test does not depend on the router adapter.
461+
const renderDirtyEditForm = (
462+
mutationMode: MutationMode,
463+
locationCallback: (l: any) => void
464+
) => {
465+
const dataProvider = testDataProvider({
466+
getOne: () =>
467+
// @ts-ignore
468+
Promise.resolve({ data: { id: 123, title: 'lorem' } }),
469+
delete: jest.fn().mockResolvedValue({ data: { id: 123 } }),
470+
});
471+
const EditToolbar = props => (
472+
<Toolbar {...props}>
473+
<DeleteWithConfirmButton mutationMode={mutationMode} />
474+
</Toolbar>
475+
);
476+
render(
477+
<TestMemoryRouter
478+
initialEntries={['/posts/123']}
479+
locationCallback={locationCallback}
480+
>
481+
<ThemeProvider theme={theme}>
482+
<CoreAdminContext dataProvider={dataProvider}>
483+
<Edit resource="posts" id={123}>
484+
<SimpleForm
485+
warnWhenUnsavedChanges
486+
toolbar={<EditToolbar />}
487+
>
488+
<TextInput source="title" />
489+
</SimpleForm>
490+
</Edit>
491+
</CoreAdminContext>
492+
</ThemeProvider>
493+
</TestMemoryRouter>
494+
);
495+
return dataProvider;
496+
};
497+
498+
const makeFormDirty = async () => {
499+
const input =
500+
await screen.findByDisplayValue<HTMLInputElement>('lorem');
501+
fireEvent.change(input, { target: { value: 'lorem modified' } });
502+
fireEvent.blur(input);
503+
};
504+
505+
it('should delete the record and redirect without warning when confirming the dialog', async () => {
506+
// spy on "cancel": if the unsaved-changes dialog were shown, the
507+
// navigation would be cancelled and the redirect assertion would fail
508+
const confirmSpy = jest
509+
.spyOn(window, 'confirm')
510+
.mockReturnValue(false);
511+
let location;
512+
const dataProvider = renderDirtyEditForm('pessimistic', l => {
513+
location = l;
514+
});
515+
await makeFormDirty();
516+
// open the confirmation dialog and confirm
517+
fireEvent.click(
518+
await screen.findByLabelText('resources.posts.action.delete')
519+
);
520+
fireEvent.click(screen.getByText('ra.action.confirm'));
521+
// the record is deleted and the app redirects to the list
522+
await waitFor(() => {
523+
expect(location.pathname).toEqual('/posts');
524+
});
525+
expect(dataProvider.delete).toHaveBeenCalledWith('posts', {
526+
id: 123,
527+
previousData: { id: 123, title: 'lorem' },
528+
});
529+
// no spurious unsaved-changes warning
530+
expect(confirmSpy).not.toHaveBeenCalled();
531+
confirmSpy.mockRestore();
532+
});
533+
534+
it('should not delete the record nor leave the form when cancelling the dialog (optimistic)', async () => {
535+
const confirmSpy = jest
536+
.spyOn(window, 'confirm')
537+
.mockReturnValue(false);
538+
let location;
539+
const dataProvider = renderDirtyEditForm('optimistic', l => {
540+
location = l;
541+
});
542+
await makeFormDirty();
543+
// open the confirmation dialog and cancel
544+
fireEvent.click(
545+
await screen.findByLabelText('resources.posts.action.delete')
546+
);
547+
fireEvent.click(screen.getByText('ra.action.cancel'));
548+
// the dialog closes
549+
await waitFor(() => {
550+
expect(screen.queryByText('ra.action.confirm')).toBeNull();
551+
});
552+
// the deletion never happened (no optimistic removal to roll back)
553+
expect(dataProvider.delete).not.toHaveBeenCalled();
554+
// the user stays on the dirty form, edits preserved
555+
expect(location.pathname).toEqual('/posts/123');
556+
expect(screen.getByDisplayValue('lorem modified')).not.toBeNull();
557+
// no unsaved-changes warning was triggered either
558+
expect(confirmSpy).not.toHaveBeenCalled();
559+
confirmSpy.mockRestore();
560+
});
561+
});
456562
});

packages/ra-ui-materialui/src/button/DeleteWithConfirmButton.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import React, { Fragment, isValidElement, ReactEventHandler } from 'react';
2+
import { useFormContext } from 'react-hook-form';
23
import ActionDelete from '@mui/icons-material/Delete';
34
import {
45
ComponentsOverrides,
@@ -59,6 +60,8 @@ export const DeleteWithConfirmButton = React.forwardRef(
5960
const notify = useNotify();
6061
const unselect = useUnselect(resource);
6162
const redirect = useRedirect();
63+
// null when the delete button is used outside a form (e.g. in a list)
64+
const form = useFormContext();
6265
const [open, setOpen] = React.useState(false);
6366
if (!resource) {
6467
throw new Error(
@@ -95,6 +98,10 @@ export const DeleteWithConfirmButton = React.forwardRef(
9598
}
9699
);
97100
record && unselect([record.id]);
101+
// Clear the form dirty state so warnWhenUnsavedChanges
102+
// doesn't block the redirect: the record is gone, so the
103+
// unsaved changes are moot.
104+
form?.reset(undefined, { keepValues: true });
98105
redirect(redirectTo, resource);
99106
}
100107
},

0 commit comments

Comments
 (0)