Merge pull request #51093 from gedu/gedu/open_report_de_dupe

Replace duplicated openReports
Expensify · Oct 21, 2024 · da68c0b · da68c0b
2 parents 8ca4576 + ed2f424
commit da68c0b
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 14 deletions.
diff --git a/src/libs/API/index.ts b/src/libs/API/index.ts
@@ -208,23 +208,32 @@ function paginate<TRequestType extends typeof CONST.API_REQUEST_TYPE.MAKE_REQUES
     onyxData: OnyxData,
     config: PaginationConfig,
 ): Promise<Response | void>;
-function paginate<TRequestType extends typeof CONST.API_REQUEST_TYPE.READ | typeof CONST.API_REQUEST_TYPE.WRITE, TCommand extends CommandOfType<TRequestType>>(
+function paginate<TRequestType extends typeof CONST.API_REQUEST_TYPE.READ, TCommand extends CommandOfType<TRequestType>>(
     type: TRequestType,
     command: TCommand,
     apiCommandParameters: ApiRequestCommandParameters[TCommand],
     onyxData: OnyxData,
     config: PaginationConfig,
 ): void;
+function paginate<TRequestType extends typeof CONST.API_REQUEST_TYPE.WRITE, TCommand extends CommandOfType<TRequestType>>(
+    type: TRequestType,
+    command: TCommand,
+    apiCommandParameters: ApiRequestCommandParameters[TCommand],
+    onyxData: OnyxData,
+    config: PaginationConfig,
+    conflictResolver?: RequestConflictResolver,
+): void;
 function paginate<TRequestType extends ApiRequestType, TCommand extends CommandOfType<TRequestType>>(
     type: TRequestType,
     command: TCommand,
     apiCommandParameters: ApiRequestCommandParameters[TCommand],
     onyxData: OnyxData,
     config: PaginationConfig,
+    conflictResolver: RequestConflictResolver = {},
 ): Promise<Response | void> | void {
     Log.info('[API] Called API.paginate', false, {command, ...apiCommandParameters});
     const request: PaginatedRequest = {
-        ...prepareRequest(command, type, apiCommandParameters, onyxData),
+        ...prepareRequest(command, type, apiCommandParameters, onyxData, conflictResolver),
         ...config,
         ...{
             isPaginated: true,

diff --git a/src/libs/actions/App.ts b/src/libs/actions/App.ts
@@ -257,7 +257,7 @@ function openApp() {
     return getPolicyParamsForOpenOrReconnect().then((policyParams: PolicyParamsForOpenOrReconnect) => {
         const params: OpenAppParams = {enablePriorityModeFilter: true, ...policyParams};
         return API.write(WRITE_COMMANDS.OPEN_APP, params, getOnyxDataForOpenOrReconnect(true), {
-            checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, WRITE_COMMANDS.OPEN_APP),
+            checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, (request) => request.command === WRITE_COMMANDS.OPEN_APP),
         });
     });
 }
@@ -287,7 +287,7 @@ function reconnectApp(updateIDFrom: OnyxEntry<number> = 0) {
         }
 
         API.write(WRITE_COMMANDS.RECONNECT_APP, params, getOnyxDataForOpenOrReconnect(), {
-            checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, WRITE_COMMANDS.RECONNECT_APP),
+            checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, (request) => request.command === WRITE_COMMANDS.RECONNECT_APP),
         });
     });
 }

diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts
@@ -111,6 +111,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject';
 import * as CachedPDFPaths from './CachedPDFPaths';
 import * as Modal from './Modal';
 import navigateFromNotification from './navigateFromNotification';
+import resolveDuplicationConflictAction from './RequestConflictUtils';
 import * as Session from './Session';
 import * as Welcome from './Welcome';
 import * as OnboardingFlow from './Welcome/OnboardingFlow';
@@ -977,7 +978,10 @@ function openReport(
         });
     } else {
         // eslint-disable-next-line rulesdir/no-multiple-api-calls
-        API.paginate(CONST.API_REQUEST_TYPE.WRITE, WRITE_COMMANDS.OPEN_REPORT, parameters, {optimisticData, successData, failureData}, paginationConfig);
+        API.paginate(CONST.API_REQUEST_TYPE.WRITE, WRITE_COMMANDS.OPEN_REPORT, parameters, {optimisticData, successData, failureData}, paginationConfig, {
+            checkAndFixConflictingRequest: (persistedRequests) =>
+                resolveDuplicationConflictAction(persistedRequests, (request) => request.command === WRITE_COMMANDS.OPEN_REPORT && request.data?.reportID === reportID),
+        });
     }
 }
 

diff --git a/src/libs/actions/RequestConflictUtils.ts b/src/libs/actions/RequestConflictUtils.ts
@@ -1,16 +1,17 @@
-import type {WriteCommand} from '@libs/API/types';
 import type OnyxRequest from '@src/types/onyx/Request';
 import type {ConflictActionData} from '@src/types/onyx/Request';
 
+type RequestMatcher = (request: OnyxRequest) => boolean;
+
 /**
- * Resolves duplication conflicts between persisted requests and a given command.
+ * Determines the appropriate action for handling duplication conflicts in persisted requests.
  *
- * This method checks if a specific command exists within a list of persisted requests.
- * - If the command is not found, it suggests adding the command to the list, indicating a 'push' action.
- * - If the command is found, it suggests updating the existing entry, indicating a 'replace' action at the found index.
+ * This method checks if any request in the list of persisted requests matches the criteria defined by the request matcher function.
+ * - If no match is found, it suggests adding the request to the list, indicating a 'push' action.
+ * - If a match is found, it suggests updating the existing entry, indicating a 'replace' action at the found index.
  */
-function resolveDuplicationConflictAction(persistedRequests: OnyxRequest[], commandToFind: WriteCommand): ConflictActionData {
-    const index = persistedRequests.findIndex((request) => request.command === commandToFind);
+function resolveDuplicationConflictAction(persistedRequests: OnyxRequest[], requestMatcher: RequestMatcher): ConflictActionData {
+    const index = persistedRequests.findIndex(requestMatcher);
     if (index === -1) {
         return {
             conflictAction: {

diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts
@@ -3,6 +3,7 @@ import {afterEach, beforeAll, beforeEach, describe, expect, it} from '@jest/glob
 import {toZonedTime} from 'date-fns-tz';
 import Onyx from 'react-native-onyx';
 import type {OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx';
+import {WRITE_COMMANDS} from '@libs/API/types';
 import CONST from '@src/CONST';
 import OnyxUpdateManager from '@src/libs/actions/OnyxUpdateManager';
 import * as PersistedRequests from '@src/libs/actions/PersistedRequests';
@@ -757,4 +758,54 @@ describe('actions/Report', () => {
                 expect(reportActionReaction?.[EMOJI.name].users[TEST_USER_ACCOUNT_ID]).toBeUndefined();
             });
     });
+
+    it.only('should send only one OpenReport, replacing any extra ones with same reportIDs', async () => {
+        global.fetch = TestHelper.getGlobalFetchMock();
+
+        const REPORT_ID = '1';
+
+        await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true});
+        await waitForBatchedUpdates();
+
+        for (let i = 0; i < 5; i++) {
+            Report.openReport(REPORT_ID, undefined, ['[email protected]'], {
+                isOptimisticReport: true,
+                reportID: REPORT_ID,
+            });
+        }
+
+        expect(PersistedRequests.getAll().length).toBe(1);
+
+        await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false});
+        await waitForBatchedUpdates();
+
+        TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.OPEN_REPORT, 1);
+    });
+
+    it.only('should replace duplicate OpenReport commands with the same reportID', async () => {
+        global.fetch = TestHelper.getGlobalFetchMock();
+
+        const REPORT_ID = '1';
+
+        await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true});
+        await waitForBatchedUpdates();
+
+        for (let i = 0; i < 8; i++) {
+            let reportID = REPORT_ID;
+            if (i > 4) {
+                reportID = `${i}`;
+            }
+            Report.openReport(reportID, undefined, ['[email protected]'], {
+                isOptimisticReport: true,
+                reportID: REPORT_ID,
+            });
+        }
+
+        expect(PersistedRequests.getAll().length).toBe(4);
+
+        await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false});
+        await waitForBatchedUpdates();
+
+        TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.OPEN_REPORT, 4);
+    });
 });
diff --git a/tests/unit/RequestConflictUtilsTest.ts b/tests/unit/RequestConflictUtilsTest.ts
@@ -5,7 +5,7 @@ describe('RequestConflictUtils', () => {
     it.each([['OpenApp'], ['ReconnectApp']])('resolveDuplicationConflictAction when %s do not exist in the queue should push %i', (command) => {
         const persistedRequests = [{command: 'OpenReport'}, {command: 'AddComment'}, {command: 'CloseAccount'}];
         const commandToFind = command as WriteCommand;
-        const result = resolveDuplicationConflictAction(persistedRequests, commandToFind);
+        const result = resolveDuplicationConflictAction(persistedRequests, (request) => request.command === commandToFind);
         expect(result).toEqual({conflictAction: {type: 'push'}});
     });
 
@@ -15,7 +15,21 @@ describe('RequestConflictUtils', () => {
     ])('resolveDuplicationConflictAction when %s exist in the queue should replace at index %i', (command, index) => {
         const persistedRequests = [{command: 'OpenApp'}, {command: 'AddComment'}, {command: 'ReconnectApp'}];
         const commandToFind = command as WriteCommand;
-        const result = resolveDuplicationConflictAction(persistedRequests, commandToFind);
+        const result = resolveDuplicationConflictAction(persistedRequests, (request) => request.command === commandToFind);
         expect(result).toEqual({conflictAction: {type: 'replace', index}});
     });
+
+    it('replaces the first OpenReport command with reportID 1 in case of duplication conflict', () => {
+        const persistedRequests = [
+            {command: 'OpenApp'},
+            {command: 'AddComment'},
+            {command: 'OpenReport', data: {reportID: 1}},
+            {command: 'OpenReport', data: {reportID: 2}},
+            {command: 'OpenReport', data: {reportID: 3}},
+            {command: 'ReconnectApp'},
+        ];
+        const reportID = 1;
+        const result = resolveDuplicationConflictAction(persistedRequests, (request) => request.command === 'OpenReport' && request.data?.reportID === reportID);
+        expect(result).toEqual({conflictAction: {type: 'replace', index: 2}});
+    });
 });