Fix middlewares might not be applied in optimistic and undoable modes when they are unregistered before the actual mutation is called

Author: djhi

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

@@ -26,7 +26,7 @@ import {
     WithMiddlewaresSuccess as WithMiddlewaresSuccessUndoable,
     WithMiddlewaresError as WithMiddlewaresErrorUndoable,
 } from './useCreate.undoable.stories';
-import { MutationMode, Params } from './useCreate.stories';
+import { Middleware, MutationMode, Params } from './useCreate.stories';
 
 describe('useCreate', () => {
     it('returns a callback that can be used with create arguments', async () => {
@@ -574,5 +574,57 @@ describe('useCreate', () => {
             ).toBeNull();
             expect(screen.queryByText('mutating')).toBeNull();
         }, 6000);
+
+        it(`it calls the middlewares in undoable mode even when they got unregistered`, async () => {
+            const middlewareSpy = jest.fn();
+            render(
+                <Middleware
+                    mutationMode="undoable"
+                    timeout={0}
+                    middleware={middlewareSpy}
+                />
+            );
+
+            fireEvent.change(screen.getByLabelText('title'), {
+                target: { value: 'Bazinga' },
+            });
+            fireEvent.click(screen.getByText('Save'));
+            await screen.findByText('resources.posts.notifications.created');
+            expect(middlewareSpy).not.toHaveBeenCalled();
+            fireEvent.click(screen.getByText('Refresh'));
+            expect(screen.queryByText('Bazinga')).toBeNull();
+            fireEvent.click(screen.getByText('Close'));
+            await waitFor(() => {
+                expect(middlewareSpy).toHaveBeenCalledWith('posts', {
+                    data: { id: 2, title: 'Bazinga' },
+                    meta: undefined,
+                });
+            });
+            fireEvent.click(screen.getByText('Refresh'));
+            await screen.findByText('Bazinga');
+        });
+        it(`it calls the middlewares in optimistic mode even when they got unregistered`, async () => {
+            const middlewareSpy = jest.fn();
+            render(
+                <Middleware
+                    mutationMode="optimistic"
+                    timeout={0}
+                    middleware={middlewareSpy}
+                />
+            );
+
+            fireEvent.change(screen.getByLabelText('title'), {
+                target: { value: 'Bazinga' },
+            });
+            fireEvent.click(screen.getByText('Save'));
+            await screen.findByText('resources.posts.notifications.created');
+            fireEvent.click(screen.getByText('Close'));
+            expect(middlewareSpy).toHaveBeenCalledWith('posts', {
+                data: { id: 2, title: 'Bazinga' },
+                meta: undefined,
+            });
+            fireEvent.click(screen.getByText('Refresh'));
+            await screen.findByText('Bazinga');
+        });
     });
 });

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

@@ -1,10 +1,23 @@
 import * as React from 'react';
 import { QueryClient, useIsMutating } from '@tanstack/react-query';
 
-import { CoreAdminContext } from '../core';
+import { CoreAdmin, CoreAdminContext, Resource } from '../core';
 import { useCreate } from './useCreate';
 import { useGetOne } from './useGetOne';
 import type { MutationMode as MutationModeType } from '../types';
+import {
+    CreateBase,
+    ListBase,
+    RecordsIterator,
+    useRegisterMutationMiddleware,
+} from '../controller';
+import { useNotificationContext } from '../notification';
+import { useTakeUndoableMutation } from './undo';
+import { Form, InputProps, useInput } from '../form';
+import { TestMemoryRouter } from '../routing';
+import { Link } from 'react-router-dom';
+import { testDataProvider } from './testDataProvider';
+import { useRefresh } from './useRefresh';
 
 export default { title: 'ra-core/dataProvider/useCreate' };
 
@@ -208,3 +221,166 @@ const ParamsCore = () => {
         </>
     );
 };
+
+export const Middleware = ({
+    middleware = (resource: string, params: any) => {
+        console.log(
+            `Creating resource ${resource} with params:`,
+            JSON.stringify(params)
+        );
+    },
+    mutationMode = 'undoable',
+    timeout = 1000,
+}: {
+    mutationMode?: MutationModeType;
+    timeout?: number;
+    middleware?: (resource: string, params: any) => void;
+}) => {
+    const posts = [{ id: 1, title: 'Hello', author: 'John Doe' }];
+    const dataProvider = testDataProvider({
+        // @ts-ignore
+        getList: (resource, params) => {
+            return Promise.resolve({
+                data: posts,
+                total: posts.length,
+            });
+        },
+        create: (resource, params) => {
+            return new Promise(resolve => {
+                setTimeout(() => {
+                    const post = { id: posts.length + 1, ...params.data };
+                    // @ts-ignore
+                    posts.push(post);
+                    // @ts-ignore
+                    resolve({ data: post });
+                }, timeout);
+            });
+        },
+    });
+    return (
+        <TestMemoryRouter initialEntries={['/posts/create']}>
+            <CoreAdmin
+                queryClient={new QueryClient()}
+                dataProvider={dataProvider}
+                layout={({ children }) => (
+                    <>
+                        {children}
+                        <Notification />
+                    </>
+                )}
+            >
+                <Resource
+                    name="posts"
+                    list={
+                        <ListBase>
+                            <ul>
+                                <RecordsIterator
+                                    render={record => <li>{record.title}</li>}
+                                />
+                            </ul>
+                            <RefreshButton />
+                        </ListBase>
+                    }
+                    create={
+                        <CreateBase
+                            mutationMode={mutationMode}
+                            redirect="list"
+                            transform={data => ({
+                                id:
+                                    mutationMode === 'pessimistic'
+                                        ? undefined
+                                        : posts.length + 1,
+                                ...data,
+                            })}
+                        >
+                            <Form>
+                                <TextInput source="title" />
+                                <CreateMiddleware middleware={middleware} />
+                                <button type="submit">Save</button>
+                            </Form>
+                        </CreateBase>
+                    }
+                />
+            </CoreAdmin>
+        </TestMemoryRouter>
+    );
+};
+
+Middleware.args = {
+    timeout: 1000,
+    mutationMode: 'optimistic',
+};
+
+Middleware.argTypes = {
+    timeout: {
+        control: {
+            type: 'number',
+        },
+    },
+    mutationMode: {
+        control: {
+            type: 'select',
+        },
+        options: ['pessimistic', 'optimistic', 'undoable'],
+    },
+};
+
+const CreateMiddleware = ({
+    middleware,
+}: {
+    middleware: (resource: string, params: any) => void;
+}) => {
+    useRegisterMutationMiddleware((resource, params, next) => {
+        middleware(resource, params);
+        return next(resource, params);
+    });
+
+    return null;
+};
+
+const Notification = () => {
+    const { notifications, resetNotifications } = useNotificationContext();
+    const takeMutation = useTakeUndoableMutation();
+
+    return notifications.length > 0 ? (
+        <>
+            <div>{notifications[0].message}</div>
+            <div style={{ display: 'flex', gap: '16px' }}>
+                <button
+                    onClick={() => {
+                        if (notifications[0].notificationOptions.undoable) {
+                            const mutation = takeMutation();
+                            if (mutation) {
+                                mutation({ isUndo: false });
+                            }
+                        }
+                        resetNotifications();
+                    }}
+                >
+                    Close
+                </button>
+            </div>
+        </>
+    ) : null;
+};
+
+const TextInput = (props: InputProps) => {
+    const { field, id } = useInput(props);
+
+    return (
+        <div style={{ display: 'flex', flexDirection: 'column', gap: '5px' }}>
+            <label htmlFor={id}>{props.label || field.name}</label>
+            <input id={id} {...field} />
+        </div>
+    );
+};
+
+const RefreshButton = () => {
+    const refresh = useRefresh();
+
+    return (
+        <button type="button" onClick={() => refresh()}>
+            Refresh
+        </button>
+    );
+};

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

@@ -198,16 +198,22 @@ export const useCreate = <
 
                 return snapshot;
             },
-            getMutateWithMiddlewares: mutationFn => args => {
-                // This is necessary to avoid breaking changes in useCreate:
-                // The mutation function must have the same signature as before (resource, params) and not ({ resource, params })
+            getMutateWithMiddlewares: mutationFn => {
                 if (getMutateWithMiddlewares) {
-                    const { resource, ...params } = args;
-                    return getMutateWithMiddlewares(
+                    // Immediately get the function with middlewares applied so that even if the middlewares gets unregistered (because of a redirect for instance),
+                    // we still have them applied when users have called the mutate function.
+                    const mutateWithMiddlewares = getMutateWithMiddlewares(
                         dataProviderCreate.bind(dataProvider)
-                    )(resource, params);
+                    );
+                    return args => {
+                        // This is necessary to avoid breaking changes in useUpdate:
+                        // The mutation function must have the same signature as before (resource, params) and not ({ resource, params })
+                        const { resource, ...params } = args;
+                        return mutateWithMiddlewares(resource, params);
+                    };
                 }
-                return mutationFn(args);
+
+                return args => mutationFn(args);
             },
             onUndo: ({ resource, data, meta }) => {
                 queryClient.removeQueries({

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

@@ -31,7 +31,7 @@ import {
     WithMiddlewaresSuccess as WithMiddlewaresSuccessUndoable,
     WithMiddlewaresError as WithMiddlewaresErrorUndoable,
 } from './useUpdate.undoable.stories';
-import { MutationMode, Params } from './useUpdate.stories';
+import { Middleware, MutationMode, Params } from './useUpdate.stories';
 
 describe('useUpdate', () => {
     describe('mutate', () => {
@@ -1183,6 +1183,58 @@ describe('useUpdate', () => {
                 expect(screen.queryByText('mutating')).toBeNull();
             });
         });
+
+        it(`it calls the middlewares in undoable mode even when they got unregistered`, async () => {
+            const middlewareSpy = jest.fn();
+            render(
+                <Middleware
+                    mutationMode="undoable"
+                    timeout={100}
+                    middleware={middlewareSpy}
+                />
+            );
+
+            fireEvent.change(screen.getByLabelText('title'), {
+                target: { value: 'Bazinga' },
+            });
+            fireEvent.click(screen.getByText('Save'));
+            await screen.findByText('resources.posts.notifications.updated');
+            expect(middlewareSpy).not.toHaveBeenCalled();
+            fireEvent.click(screen.getByText('Close'));
+            await waitFor(() => {
+                expect(middlewareSpy).toHaveBeenCalledWith('posts', {
+                    id: '1',
+                    data: { title: 'Bazinga' },
+                    meta: undefined,
+                    previousData: undefined,
+                });
+            });
+        });
+        it(`it calls the middlewares in optimistic mode even when they got unregistered`, async () => {
+            const middlewareSpy = jest.fn();
+            render(
+                <Middleware
+                    mutationMode="optimistic"
+                    timeout={0}
+                    middleware={middlewareSpy}
+                />
+            );
+
+            fireEvent.change(screen.getByLabelText('title'), {
+                target: { value: 'Bazinga' },
+            });
+            fireEvent.click(screen.getByText('Save'));
+            await screen.findByText('resources.posts.notifications.updated');
+            fireEvent.click(screen.getByText('Close'));
+            await waitFor(() => {
+                expect(middlewareSpy).toHaveBeenCalledWith('posts', {
+                    id: '1',
+                    data: { title: 'Bazinga' },
+                    meta: undefined,
+                    previousData: undefined,
+                });
+            });
+        });
     });
 });