From 62e72643d9e99213767f6ff17924f91bc14b6d76 Mon Sep 17 00:00:00 2001 From: Eduardo Date: Fri, 18 Oct 2024 15:40:35 +0200 Subject: [PATCH 1/2] Replace duplicated openReports --- src/libs/API/index.ts | 13 +++++- src/libs/actions/App.ts | 4 +- src/libs/actions/Report.ts | 6 ++- src/libs/actions/RequestConflictUtils.ts | 7 ++-- tests/actions/ReportTest.ts | 51 ++++++++++++++++++++++++ tests/unit/RequestConflictUtilsTest.ts | 18 ++++++++- 6 files changed, 89 insertions(+), 10 deletions(-) diff --git a/src/libs/API/index.ts b/src/libs/API/index.ts index 0d1bab053182..ad0650374011 100644 --- a/src/libs/API/index.ts +++ b/src/libs/API/index.ts @@ -208,23 +208,32 @@ function paginate; -function paginate>( +function paginate>( type: TRequestType, command: TCommand, apiCommandParameters: ApiRequestCommandParameters[TCommand], onyxData: OnyxData, config: PaginationConfig, ): void; +function paginate>( + type: TRequestType, + command: TCommand, + apiCommandParameters: ApiRequestCommandParameters[TCommand], + onyxData: OnyxData, + config: PaginationConfig, + conflictResolver?: RequestConflictResolver, +): void; function paginate>( type: TRequestType, command: TCommand, apiCommandParameters: ApiRequestCommandParameters[TCommand], onyxData: OnyxData, config: PaginationConfig, + conflictResolver: RequestConflictResolver = {}, ): Promise | 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 index 71cb5f97e00e..68c255e73be1 100644 --- 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 = 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 index 7071c96f8612..dce8f2d19559 100644 --- 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 index 68c0860389b9..3914c8f16711 100644 --- a/src/libs/actions/RequestConflictUtils.ts +++ b/src/libs/actions/RequestConflictUtils.ts @@ -1,7 +1,8 @@ -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. * @@ -9,8 +10,8 @@ import type {ConflictActionData} from '@src/types/onyx/Request'; * - 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. */ -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 index 0ffb0ee9bc08..5ac32866c77f 100644 --- 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, ['test@user.com'], { + 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, ['test@user.com'], { + 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 index d2d003192456..103834ce52d2 100644 --- 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}}); + }); }); From ed2f4241e79b95b1b6e6de1fb92eec8d83fcc461 Mon Sep 17 00:00:00 2001 From: Eduardo Date: Mon, 21 Oct 2024 09:59:46 +0200 Subject: [PATCH 2/2] Updated JSDoc --- src/libs/actions/RequestConflictUtils.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/RequestConflictUtils.ts b/src/libs/actions/RequestConflictUtils.ts index 3914c8f16711..fcf9ff439b11 100644 --- a/src/libs/actions/RequestConflictUtils.ts +++ b/src/libs/actions/RequestConflictUtils.ts @@ -4,11 +4,11 @@ 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[], requestMatcher: RequestMatcher): ConflictActionData { const index = persistedRequests.findIndex(requestMatcher);