Skip to content

Commit 11af04a

Browse files
slax57slax57
authored
Merge pull request #11007 from marmelab/fix-middlewares-not-executed-in-optimistic-modes
Fix middlewares might not be applied in `optimistic` and `undoable` modes when they are unregistered before the actual mutation is called
2 parents b125279 + 98b6fe5 commit 11af04a

File tree

7 files changed

+501
-25
lines changed

7 files changed

+501
-25
lines changed

‎packages/ra-core/src/dataProvider/useCreate.spec.tsx‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
WithMiddlewaresSuccess as WithMiddlewaresSuccessUndoable,
2727
WithMiddlewaresError as WithMiddlewaresErrorUndoable,
2828
} from './useCreate.undoable.stories';
29-
import { MutationMode, Params } from './useCreate.stories';
29+
import { Middleware, MutationMode, Params } from './useCreate.stories';
3030

3131
describe('useCreate', () => {
3232
it('returns a callback that can be used with create arguments', async () => {
@@ -574,5 +574,57 @@ describe('useCreate', () => {
574574
).toBeNull();
575575
expect(screen.queryByText('mutating')).toBeNull();
576576
}, 6000);
577+
578+
it(`it calls the middlewares in undoable mode even when they got unregistered`, async () => {
579+
const middlewareSpy = jest.fn();
580+
render(
581+
<Middleware
582+
mutationMode="undoable"
583+
timeout={0}
584+
middleware={middlewareSpy}
585+
/>
586+
);
587+
588+
fireEvent.change(screen.getByLabelText('title'), {
589+
target: { value: 'Bazinga' },
590+
});
591+
fireEvent.click(screen.getByText('Save'));
592+
await screen.findByText('resources.posts.notifications.created');
593+
expect(middlewareSpy).not.toHaveBeenCalled();
594+
fireEvent.click(screen.getByText('Refresh'));
595+
expect(screen.queryByText('Bazinga')).toBeNull();
596+
fireEvent.click(screen.getByText('Close'));
597+
await waitFor(() => {
598+
expect(middlewareSpy).toHaveBeenCalledWith('posts', {
599+
data: { id: 2, title: 'Bazinga' },
600+
meta: undefined,
601+
});
602+
});
603+
fireEvent.click(screen.getByText('Refresh'));
604+
await screen.findByText('Bazinga');
605+
});
606+
it(`it calls the middlewares in optimistic mode even when they got unregistered`, async () => {
607+
const middlewareSpy = jest.fn();
608+
render(
609+
<Middleware
610+
mutationMode="optimistic"
611+
timeout={0}
612+
middleware={middlewareSpy}
613+
/>
614+
);
615+
616+
fireEvent.change(screen.getByLabelText('title'), {
617+
target: { value: 'Bazinga' },
618+
});
619+
fireEvent.click(screen.getByText('Save'));
620+
await screen.findByText('resources.posts.notifications.created');
621+
fireEvent.click(screen.getByText('Close'));
622+
expect(middlewareSpy).toHaveBeenCalledWith('posts', {
623+
data: { id: 2, title: 'Bazinga' },
624+
meta: undefined,
625+
});
626+
fireEvent.click(screen.getByText('Refresh'));
627+
await screen.findByText('Bazinga');
628+
});
577629
});
578630
});

‎packages/ra-core/src/dataProvider/useCreate.stories.tsx‎

Lines changed: 176 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,22 @@
11
import * as React from 'react';
22
import { QueryClient, useIsMutating } from '@tanstack/react-query';
33

4-
import { CoreAdminContext } from '../core';
4+
import { CoreAdmin, CoreAdminContext, Resource } from '../core';
55
import { useCreate } from './useCreate';
66
import { useGetOne } from './useGetOne';
77
import type { MutationMode as MutationModeType } from '../types';
8+
import {
9+
CreateBase,
10+
ListBase,
11+
RecordsIterator,
12+
useRegisterMutationMiddleware,
13+
} from '../controller';
14+
import { useNotificationContext } from '../notification';
15+
import { useTakeUndoableMutation } from './undo';
16+
import { Form, InputProps, useInput } from '../form';
17+
import { TestMemoryRouter } from '../routing';
18+
import { testDataProvider } from './testDataProvider';
19+
import { useRefresh } from './useRefresh';
820

921
export default { title: 'ra-core/dataProvider/useCreate' };
1022

@@ -208,3 +220,166 @@ const ParamsCore = () => {
208220
</>
209221
);
210222
};
223+
224+
export const Middleware = ({
225+
middleware = (resource: string, params: any) => {
226+
console.log(
227+
`Creating resource ${resource} with params:`,
228+
JSON.stringify(params)
229+
);
230+
},
231+
mutationMode = 'undoable',
232+
timeout = 1000,
233+
}: {
234+
mutationMode?: MutationModeType;
235+
timeout?: number;
236+
middleware?: (resource: string, params: any) => void;
237+
}) => {
238+
const posts = [{ id: 1, title: 'Hello', author: 'John Doe' }];
239+
const dataProvider = testDataProvider({
240+
// @ts-ignore
241+
getList: () => {
242+
return Promise.resolve({
243+
data: posts,
244+
total: posts.length,
245+
});
246+
},
247+
create: (resource, params) => {
248+
return new Promise(resolve => {
249+
setTimeout(() => {
250+
const post = { id: posts.length + 1, ...params.data };
251+
// @ts-ignore
252+
posts.push(post);
253+
// @ts-ignore
254+
resolve({ data: post });
255+
}, timeout);
256+
});
257+
},
258+
});
259+
return (
260+
<TestMemoryRouter initialEntries={['/posts/create']}>
261+
<CoreAdmin
262+
queryClient={new QueryClient()}
263+
dataProvider={dataProvider}
264+
layout={({ children }) => (
265+
<>
266+
{children}
267+
<Notification />
268+
</>
269+
)}
270+
>
271+
<Resource
272+
name="posts"
273+
list={
274+
<ListBase>
275+
<ul>
276+
<RecordsIterator
277+
render={record => <li>{record.title}</li>}
278+
/>
279+
</ul>
280+
<RefreshButton />
281+
</ListBase>
282+
}
283+
create={
284+
<CreateBase
285+
mutationMode={mutationMode}
286+
redirect="list"
287+
transform={data => ({
288+
id:
289+
mutationMode === 'pessimistic'
290+
? undefined
291+
: posts.length + 1,
292+
...data,
293+
})}
294+
>
295+
<Form>
296+
<TextInput source="title" />
297+
<CreateMiddleware middleware={middleware} />
298+
<button type="submit">Save</button>
299+
</Form>
300+
</CreateBase>
301+
}
302+
/>
303+
</CoreAdmin>
304+
</TestMemoryRouter>
305+
);
306+
};
307+
308+
Middleware.args = {
309+
timeout: 1000,
310+
mutationMode: 'optimistic',
311+
};
312+
313+
Middleware.argTypes = {
314+
timeout: {
315+
control: {
316+
type: 'number',
317+
},
318+
},
319+
mutationMode: {
320+
control: {
321+
type: 'select',
322+
},
323+
options: ['pessimistic', 'optimistic', 'undoable'],
324+
},
325+
};
326+
327+
const CreateMiddleware = ({
328+
middleware,
329+
}: {
330+
middleware: (resource: string, params: any) => void;
331+
}) => {
332+
useRegisterMutationMiddleware((resource, params, next) => {
333+
middleware(resource, params);
334+
return next(resource, params);
335+
});
336+
337+
return null;
338+
};
339+
340+
const Notification = () => {
341+
const { notifications, resetNotifications } = useNotificationContext();
342+
const takeMutation = useTakeUndoableMutation();
343+
344+
return notifications.length > 0 ? (
345+
<>
346+
<div>{notifications[0].message}</div>
347+
<div style={{ display: 'flex', gap: '16px' }}>
348+
<button
349+
onClick={() => {
350+
if (notifications[0].notificationOptions.undoable) {
351+
const mutation = takeMutation();
352+
if (mutation) {
353+
mutation({ isUndo: false });
354+
}
355+
}
356+
resetNotifications();
357+
}}
358+
>
359+
Close
360+
</button>
361+
</div>
362+
</>
363+
) : null;
364+
};
365+
366+
const TextInput = (props: InputProps) => {
367+
const { field, id } = useInput(props);
368+
369+
return (
370+
<div style={{ display: 'flex', flexDirection: 'column', gap: '5px' }}>
371+
<label htmlFor={id}>{props.label || field.name}</label>
372+
<input id={id} {...field} />
373+
</div>
374+
);
375+
};
376+
377+
const RefreshButton = () => {
378+
const refresh = useRefresh();
379+
380+
return (
381+
<button type="button" onClick={() => refresh()}>
382+
Refresh
383+
</button>
384+
);
385+
};

‎packages/ra-core/src/dataProvider/useCreate.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -198,16 +198,22 @@ export const useCreate = <
198198

199199
return snapshot;
200200
},
201-
getMutateWithMiddlewares: mutationFn => args => {
202-
// This is necessary to avoid breaking changes in useCreate:
203-
// The mutation function must have the same signature as before (resource, params) and not ({ resource, params })
201+
getMutateWithMiddlewares: mutationFn => {
204202
if (getMutateWithMiddlewares) {
205-
const { resource, ...params } = args;
206-
return getMutateWithMiddlewares(
203+
// Immediately get the function with middlewares applied so that even if the middlewares gets unregistered (because of a redirect for instance),
204+
// we still have them applied when users have called the mutate function.
205+
const mutateWithMiddlewares = getMutateWithMiddlewares(
207206
dataProviderCreate.bind(dataProvider)
208-
)(resource, params);
207+
);
208+
return args => {
209+
// This is necessary to avoid breaking changes in useCreate:
210+
// The mutation function must have the same signature as before (resource, params) and not ({ resource, params })
211+
const { resource, ...params } = args;
212+
return mutateWithMiddlewares(resource, params);
213+
};
209214
}
210-
return mutationFn(args);
215+
216+
return args => mutationFn(args);
211217
},
212218
onUndo: ({ resource, data, meta }) => {
213219
queryClient.removeQueries({

‎packages/ra-core/src/dataProvider/useUpdate.spec.tsx‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {
3131
WithMiddlewaresSuccess as WithMiddlewaresSuccessUndoable,
3232
WithMiddlewaresError as WithMiddlewaresErrorUndoable,
3333
} from './useUpdate.undoable.stories';
34-
import { MutationMode, Params } from './useUpdate.stories';
34+
import { Middleware, MutationMode, Params } from './useUpdate.stories';
3535

3636
describe('useUpdate', () => {
3737
describe('mutate', () => {
@@ -1183,6 +1183,68 @@ describe('useUpdate', () => {
11831183
expect(screen.queryByText('mutating')).toBeNull();
11841184
});
11851185
});
1186+
1187+
it(`it calls the middlewares in undoable mode even when they got unregistered`, async () => {
1188+
const middlewareSpy = jest.fn();
1189+
render(
1190+
<Middleware
1191+
mutationMode="undoable"
1192+
timeout={100}
1193+
middleware={middlewareSpy}
1194+
/>
1195+
);
1196+
1197+
fireEvent.click(await screen.findByText('Hello'));
1198+
fireEvent.change(await screen.findByLabelText('title'), {
1199+
target: { value: 'Bazinga' },
1200+
});
1201+
fireEvent.click(screen.getByText('Save'));
1202+
await screen.findByText('resources.posts.notifications.updated');
1203+
expect(middlewareSpy).not.toHaveBeenCalled();
1204+
fireEvent.click(screen.getByText('Close'));
1205+
await waitFor(() => {
1206+
expect(middlewareSpy).toHaveBeenCalledWith('posts', {
1207+
id: '1',
1208+
data: { author: 'John Doe', id: 1, title: 'Bazinga' },
1209+
meta: undefined,
1210+
previousData: {
1211+
author: 'John Doe',
1212+
id: 1,
1213+
title: 'Bazinga',
1214+
},
1215+
});
1216+
});
1217+
});
1218+
it(`it calls the middlewares in optimistic mode even when they got unregistered`, async () => {
1219+
const middlewareSpy = jest.fn();
1220+
render(
1221+
<Middleware
1222+
mutationMode="optimistic"
1223+
timeout={0}
1224+
middleware={middlewareSpy}
1225+
/>
1226+
);
1227+
1228+
fireEvent.click(await screen.findByText('Hello'));
1229+
fireEvent.change(await screen.findByLabelText('title'), {
1230+
target: { value: 'Bazinga' },
1231+
});
1232+
fireEvent.click(screen.getByText('Save'));
1233+
await screen.findByText('resources.posts.notifications.updated');
1234+
fireEvent.click(screen.getByText('Close'));
1235+
await waitFor(() => {
1236+
expect(middlewareSpy).toHaveBeenCalledWith('posts', {
1237+
id: '1',
1238+
data: { author: 'John Doe', id: 1, title: 'Bazinga' },
1239+
meta: undefined,
1240+
previousData: {
1241+
author: 'John Doe',
1242+
id: 1,
1243+
title: 'Bazinga',
1244+
},
1245+
});
1246+
});
1247+
});
11861248
});
11871249
});
11881250

0 commit comments

Comments
 (0)