From a6c6b7f494fc47c677179c2210a8a2fbdedcb02f Mon Sep 17 00:00:00 2001 From: Carlos Martins Date: Wed, 22 Nov 2023 13:02:01 -0700 Subject: [PATCH] Revert "Refactor save method in PersistedRequests" --- src/libs/Network/SequentialQueue.ts | 2 +- src/libs/Network/enhanceParameters.ts | 3 -- src/libs/actions/PersistedRequests.ts | 16 +++---- src/libs/actions/Report.js | 9 +--- src/types/onyx/Request.ts | 1 - tests/unit/PersistedRequestsTest.ts | 67 --------------------------- 6 files changed, 10 insertions(+), 88 deletions(-) delete mode 100644 tests/unit/PersistedRequestsTest.ts diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 5da032baaf45..d4aee4a221e5 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -160,7 +160,7 @@ NetworkStore.onReconnection(flush); function push(request: OnyxRequest) { // Add request to Persisted Requests so that it can be retried if it fails - PersistedRequests.save(request); + PersistedRequests.save([request]); // If we are offline we don't need to trigger the queue to empty as it will happen when we come back online if (NetworkStore.isOffline()) { diff --git a/src/libs/Network/enhanceParameters.ts b/src/libs/Network/enhanceParameters.ts index 3fadeea7447c..6ff54f94bc88 100644 --- a/src/libs/Network/enhanceParameters.ts +++ b/src/libs/Network/enhanceParameters.ts @@ -37,8 +37,5 @@ export default function enhanceParameters(command: string, parameters: Record request.data?.idempotencyKey && request.data?.idempotencyKey === requestToPersist.data?.idempotencyKey); - if (existingRequestIndex > -1) { - // Merge the new request into the existing one, keeping its place in the queue - persistedRequests.splice(existingRequestIndex, 1, requestToPersist); +function save(requestsToPersist: Request[]) { + let requests: Request[] = []; + if (persistedRequests.length) { + requests = persistedRequests.concat(requestsToPersist); } else { - // If not, push the new request to the end of the queue - persistedRequests.push(requestToPersist); + requests = requestsToPersist; } - Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests); + persistedRequests = requests; + Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, requests); } function remove(requestToRemove: Request) { diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 9bc2aa1b3f2f..a03488429405 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -470,9 +470,6 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p if (!reportID) { return; } - - const commandName = 'OpenReport'; - const optimisticReportData = [ { onyxMethod: Onyx.METHOD.MERGE, @@ -538,7 +535,6 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p emailList: participantLoginList ? participantLoginList.join(',') : '', accountIDList: participantAccountIDList ? participantAccountIDList.join(',') : '', parentReportActionID, - idempotencyKey: `${commandName}_${reportID}`, }; if (isFromDeepLink) { @@ -616,7 +612,6 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p // Add the createdReportActionID parameter to the API call params.createdReportActionID = optimisticCreatedAction.reportActionID; - params.idempotencyKey = `${params.idempotencyKey}_NewReport_${optimisticCreatedAction.reportActionID}`; // If we are creating a thread, ensure the report action has childReportID property added if (newReportObject.parentReportID && parentReportActionID) { @@ -637,12 +632,12 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p if (isFromDeepLink) { // eslint-disable-next-line rulesdir/no-api-side-effects-method - API.makeRequestWithSideEffects(commandName, params, onyxData).finally(() => { + API.makeRequestWithSideEffects('OpenReport', params, onyxData).finally(() => { Onyx.set(ONYXKEYS.IS_CHECKING_PUBLIC_ROOM, false); }); } else { // eslint-disable-next-line rulesdir/no-multiple-api-calls - API.write(commandName, params, onyxData); + API.write('OpenReport', params, onyxData); } } diff --git a/src/types/onyx/Request.ts b/src/types/onyx/Request.ts index 746e7f75b3d5..33105b914b6f 100644 --- a/src/types/onyx/Request.ts +++ b/src/types/onyx/Request.ts @@ -17,7 +17,6 @@ type RequestData = { shouldUseSecure?: boolean; successData?: OnyxUpdate[]; failureData?: OnyxUpdate[]; - idempotencyKey?: string; resolve?: (value: Response) => void; reject?: (value?: unknown) => void; diff --git a/tests/unit/PersistedRequestsTest.ts b/tests/unit/PersistedRequestsTest.ts deleted file mode 100644 index 79153efdc806..000000000000 --- a/tests/unit/PersistedRequestsTest.ts +++ /dev/null @@ -1,67 +0,0 @@ -import * as PersistedRequests from '../../src/libs/actions/PersistedRequests'; -import Request from '../../src/types/onyx/Request'; - -const request: Request = { - command: 'OpenReport', - data: { - idempotencyKey: 'OpenReport_1', - }, - successData: [{key: 'reportMetadata_1', onyxMethod: 'merge', value: {}}], - failureData: [{key: 'reportMetadata_2', onyxMethod: 'merge', value: {}}], -}; - -beforeEach(() => { - PersistedRequests.clear(); - PersistedRequests.save(request); -}); - -afterEach(() => { - PersistedRequests.clear(); -}); - -describe('PersistedRequests', () => { - it('save a new request with an idempotency key which currently exists in the PersistedRequests array', () => { - PersistedRequests.save(request); - expect(PersistedRequests.getAll().length).toBe(1); - }); - - it('save a new request with a new idempotency key', () => { - const newRequest = { - command: 'OpenReport', - data: { - idempotencyKey: 'OpenReport_2', - }, - }; - PersistedRequests.save(newRequest); - expect(PersistedRequests.getAll().length).toBe(2); - }); - - it('replace a request existing in the PersistedRequests array with a new one', () => { - const newRequest: Request = { - command: 'OpenReport', - data: { - idempotencyKey: 'OpenReport_1', - }, - successData: [{key: 'reportMetadata_3', onyxMethod: 'merge', value: {}}], - failureData: [{key: 'reportMetadata_4', onyxMethod: 'merge', value: {}}], - }; - - PersistedRequests.save(newRequest); - - const persistedRequests = PersistedRequests.getAll(); - - expect(persistedRequests.length).toBe(1); - - const mergedRequest = persistedRequests[0]; - - expect(mergedRequest.successData?.length).toBe(1); - expect(mergedRequest.failureData?.length).toBe(1); - expect(mergedRequest.successData?.[0]?.key).toBe('reportMetadata_3'); - expect(mergedRequest.failureData?.[0]?.key).toBe('reportMetadata_4'); - }); - - it('remove a request from the PersistedRequests array', () => { - PersistedRequests.remove(request); - expect(PersistedRequests.getAll().length).toBe(0); - }); -});