Merge pull request #40180 from Expensify/Rory-RevertRequestDeduping

Remove all request de-duping logic

(cherry picked from commit 9853832)

src/libs/API/index.ts

@@ -7,7 +7,6 @@ import * as Pusher from '@libs/Pusher/pusher';
 import * as Request from '@libs/Request';
 import CONST from '@src/CONST';
 import type OnyxRequest from '@src/types/onyx/Request';
-import type {RequestConflictResolver} from '@src/types/onyx/Request';
 import type Response from '@src/types/onyx/Response';
 import pkg from '../../../package.json';
 import type {ApiRequest, ApiRequestCommandParameters, ReadCommand, SideEffectRequestCommand, WriteCommand} from './types';
@@ -52,14 +51,8 @@ type OnyxData = {
  * @param [onyxData.successData] - Onyx instructions that will be passed to Onyx.update() when the response has jsonCode === 200.
  * @param [onyxData.failureData] - Onyx instructions that will be passed to Onyx.update() when the response has jsonCode !== 200.
  * @param [onyxData.finallyData] - Onyx instructions that will be passed to Onyx.update() when the response has jsonCode === 200 or jsonCode !== 200.
- * @param [conflictResolver] - callbacks used in special cases to detect and handle conflicting requests in the sequential queue
  */
-function write<TCommand extends WriteCommand>(
-    command: TCommand,
-    apiCommandParameters: ApiRequestCommandParameters[TCommand],
-    onyxData: OnyxData = {},
-    conflictResolver: RequestConflictResolver = {},
-) {
+function write<TCommand extends WriteCommand>(command: TCommand, apiCommandParameters: ApiRequestCommandParameters[TCommand], onyxData: OnyxData = {}) {
     Log.info('Called API write', false, {command, ...apiCommandParameters});
     const {optimisticData, ...onyxDataWithoutOptimisticData} = onyxData;
 
@@ -90,7 +83,6 @@ function write<TCommand extends WriteCommand>(
             canCancel: true,
         },
         ...onyxDataWithoutOptimisticData,
-        ...conflictResolver,
     };
 
     // Write commands can be saved and retried, so push it to the SequentialQueue

src/libs/actions/App.ts

@@ -238,9 +238,7 @@ function reconnectApp(updateIDFrom: OnyxEntry<number> = 0) {
             params.updateIDFrom = updateIDFrom;
         }
 
-        API.write(WRITE_COMMANDS.RECONNECT_APP, params, getOnyxDataForOpenOrReconnect(), {
-            getConflictingRequests: (persistedRequests) => persistedRequests.filter((request) => request?.command === WRITE_COMMANDS.RECONNECT_APP),
-        });
+        API.write(WRITE_COMMANDS.RECONNECT_APP, params, getOnyxDataForOpenOrReconnect());
     });
 }
 

src/libs/actions/PersistedRequests.ts

@@ -18,39 +18,8 @@ function clear() {
 }
 
 function save(requestToPersist: Request) {
-    const requests = [...persistedRequests, requestToPersist];
-
-    // identify and handle any existing requests that conflict with the new one
-    const {getConflictingRequests, handleConflictingRequest, shouldIncludeCurrentRequest} = requestToPersist;
-    if (getConflictingRequests) {
-        // Get all the requests, potentially including the one we're adding, which will always be at the end of the array
-        const potentiallyConflictingRequests = shouldIncludeCurrentRequest ? requests : requests.slice(0, requests.length - 1);
-
-        // Identify conflicting requests according to logic bound to the new request
-        const conflictingRequests = getConflictingRequests(potentiallyConflictingRequests);
-
-        conflictingRequests.forEach((conflictingRequest) => {
-            // delete the conflicting request
-            const index = requests.findIndex((req) => req === conflictingRequest);
-            if (index !== -1) {
-                requests.splice(index, 1);
-            }
-
-            // Allow the new request to perform any additional cleanup for a cancelled request
-            handleConflictingRequest?.(conflictingRequest);
-        });
-    }
-
-    // Don't try to serialize conflict resolution functions
-    persistedRequests = requests.map((request) => {
-        delete request.getConflictingRequests;
-        delete request.handleConflictingRequest;
-        delete request.shouldIncludeCurrentRequest;
-        return request;
-    });
-
-    // Save the updated set of requests
-    Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, requests);
+    persistedRequests.push(requestToPersist);
+    Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests);
 }
 
 function remove(requestToRemove: Request) {

src/libs/actions/Report.ts

@@ -753,19 +753,7 @@ function openReport(
         });
     } else {
         // eslint-disable-next-line rulesdir/no-multiple-api-calls
-        API.write(
-            WRITE_COMMANDS.OPEN_REPORT,
-            parameters,
-            {optimisticData, successData, failureData},
-            {
-                getConflictingRequests: (persistedRequests) =>
-                    // requests conflict only if:
-                    // 1. they are OpenReport commands
-                    // 2. they have the same reportID
-                    // 3. they are not creating a report - all calls to OpenReport that create a report will be unique and have a unique createdReportActionID
-                    persistedRequests.filter((request) => request.command === WRITE_COMMANDS.OPEN_REPORT && request.data?.reportID === reportID && !request.data?.createdReportActionID),
-            },
-        );
+        API.write(WRITE_COMMANDS.OPEN_REPORT, parameters, {optimisticData, successData, failureData});
     }
 }
 
@@ -1228,23 +1216,7 @@ function deleteReportComment(reportID: string, reportAction: ReportAction) {
 
     CachedPDFPaths.clearByKey(reportActionID);
 
-    API.write(
-        WRITE_COMMANDS.DELETE_COMMENT,
-        parameters,
-        {optimisticData, successData, failureData},
-        {
-            getConflictingRequests: (persistedRequests) => {
-                const conflictingCommands = (
-                    isDeletedParentAction
-                        ? [WRITE_COMMANDS.UPDATE_COMMENT]
-                        : [WRITE_COMMANDS.ADD_COMMENT, WRITE_COMMANDS.ADD_ATTACHMENT, WRITE_COMMANDS.UPDATE_COMMENT, WRITE_COMMANDS.DELETE_COMMENT]
-                ) as string[];
-                return persistedRequests.filter((request) => conflictingCommands.includes(request.command) && request.data?.reportActionID === reportActionID);
-            },
-            handleConflictingRequest: () => Onyx.update(successData),
-            shouldIncludeCurrentRequest: !isDeletedParentAction,
-        },
-    );
+    API.write(WRITE_COMMANDS.DELETE_COMMENT, parameters, {optimisticData, successData, failureData});
 }
 
 /**

src/types/onyx/Request.ts

@@ -19,40 +19,11 @@ type RequestData = {
     successData?: OnyxUpdate[];
     failureData?: OnyxUpdate[];
     finallyData?: OnyxUpdate[];
-    getConflictingRequests?: (persistedRequests: Request[]) => Request[];
-    handleConflictingRequest?: (persistedRequest: Request) => unknown;
     resolve?: (value: Response) => void;
     reject?: (value?: unknown) => void;
 };
 
-type RequestConflictResolver = {
-    /**
-     * A callback that's provided with all the currently serialized functions in the sequential queue.
-     * Should return a subset of the requests passed in that conflict with the new request.
-     * Any conflicting requests will be cancelled and removed from the queue.
-     *
-     * @example - In ReconnectApp, you'd only want to have one instance of that command serialized to run on reconnect. The callback for that might look like this:
-     * (persistedRequests) => persistedRequests.filter((request) => request.command === 'ReconnectApp')
-     * */
-    getConflictingRequests?: (persistedRequests: Request[]) => Request[];
-
-    /**
-     * Should the requests provided to getConflictingRequests include the new request?
-     * This is useful if the new request and an existing request cancel eachother out completely.
-     *
-     * @example - In DeleteComment, if you're deleting an optimistic comment, you'd want to cancel the optimistic AddComment call AND the DeleteComment call.
-     * */
-    shouldIncludeCurrentRequest?: boolean;
-
-    /**
-     * Callback to handle a single conflicting request.
-     * This is useful if you need to clean up some optimistic data for a request that was queue but never sent.
-     * In these cases the optimisticData will be applied immediately, but the successData, failureData, and/or finallyData will never be applied unless you do it manually in this callback.
-     */
-    handleConflictingRequest?: (persistedRequest: Request) => unknown;
-};
-
-type Request = RequestData & OnyxData & RequestConflictResolver;
+type Request = RequestData & OnyxData;
 
 export default Request;
-export type {OnyxData, RequestType, RequestConflictResolver};
+export type {OnyxData, RequestType};

tests/unit/PersistedRequests.ts

@@ -22,25 +22,6 @@ describe('PersistedRequests', () => {
         expect(PersistedRequests.getAll().length).toBe(2);
     });
 
-    it('save a new request with conflict resolution', () => {
-        const handleConflictingRequest = jest.fn();
-        const newRequest = {
-            command: 'ThingA',
-            getConflictingRequests: (requests: Request[]) => requests,
-            handleConflictingRequest,
-        };
-        const secondRequest = {
-            command: 'ThingB',
-            getConflictingRequests: (requests: Request[]) => requests,
-            shouldIncludeCurrentRequest: true,
-        };
-        PersistedRequests.save(newRequest);
-        PersistedRequests.save(secondRequest);
-        expect(PersistedRequests.getAll().length).toBe(1);
-        expect(handleConflictingRequest).toHaveBeenCalledWith(request);
-        expect(handleConflictingRequest).toHaveBeenCalledTimes(1);
-    });
-
     it('remove a request from the PersistedRequests array', () => {
         PersistedRequests.remove(request);
         expect(PersistedRequests.getAll().length).toBe(0);