From d921edb32f8939ed463fff56a351266813cfbebe Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Thu, 26 Oct 2023 09:08:50 +0200 Subject: [PATCH 01/11] Refactor save method in PersistedRequests --- src/libs/actions/PersistedRequests.ts | 43 +++++++++++++++++++++++++-- src/libs/actions/Report.js | 1 + src/types/onyx/Request.ts | 1 + 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index c35de9ee94c4..cf714271263b 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,5 +1,5 @@ +import Onyx, {OnyxUpdate} from 'react-native-onyx'; import isEqual from 'lodash/isEqual'; -import Onyx from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import {Request} from '@src/types/onyx'; @@ -17,10 +17,49 @@ function clear() { return Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, []); } +function mergeOnyxUpdateData(oldData: OnyxUpdate[] = [], newData: OnyxUpdate[] = []): OnyxUpdate[] { + const mergedData = [...newData]; + + oldData.forEach((oldUpdate) => { + const hasSameKey = newData.some((newUpdate) => newUpdate.key === oldUpdate.key); + + if (!hasSameKey) { + mergedData.push(oldUpdate); + } + }); + + return mergedData; +} + +function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request { + const updatedRequest = { + failureData: mergeOnyxUpdateData(oldRequest.failureData, newRequest.failureData), + successData: mergeOnyxUpdateData(oldRequest.successData, newRequest.successData), + ...newRequest, + }; + + const updatedOptimisticData = mergeOnyxUpdateData(oldRequest.optimisticData, newRequest.optimisticData); + + if (updatedOptimisticData.length > 0) { + updatedRequest.optimisticData = updatedOptimisticData; + } + + return updatedRequest; +} + function save(requestsToPersist: Request[]) { let requests: Request[] = []; + if (persistedRequests.length) { - requests = persistedRequests.concat(requestsToPersist); + requests = [...persistedRequests]; + requestsToPersist.forEach((requestToPersist) => { + const index = persistedRequests.findIndex((persistedRequest) => !!requestToPersist.idempotencyKey && requestToPersist.idempotencyKey === persistedRequest.idempotencyKey); + if (index > -1) { + requests[index] = createUpdatedRequest(requests[index], requestToPersist); + } else { + requests.push(requestToPersist); + } + }); } else { requests = requestsToPersist; } diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 3f7dc76b174d..afbe96273ca2 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -521,6 +521,7 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p optimisticData: optimisticReportData, successData: reportSuccessData, failureData: reportFailureData, + idempotencyKey: `OpenReport_${reportID}`, }; const params = { diff --git a/src/types/onyx/Request.ts b/src/types/onyx/Request.ts index 836138ca99ba..8f0121a31fcb 100644 --- a/src/types/onyx/Request.ts +++ b/src/types/onyx/Request.ts @@ -5,6 +5,7 @@ type OnyxData = { successData?: OnyxUpdate[]; failureData?: OnyxUpdate[]; optimisticData?: OnyxUpdate[]; + idempotencyKey?: string; }; type RequestData = { From b37e1bad02ee4ae474702f01266d873e260ae837 Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Mon, 30 Oct 2023 10:19:33 +0100 Subject: [PATCH 02/11] Run prettier --- src/libs/actions/PersistedRequests.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index cf714271263b..ea1f4e9b852d 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,5 +1,5 @@ -import Onyx, {OnyxUpdate} from 'react-native-onyx'; import isEqual from 'lodash/isEqual'; +import Onyx, {OnyxUpdate} from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import {Request} from '@src/types/onyx'; From f2141610cab49d0fd454c080016bff94c36a413c Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Tue, 31 Oct 2023 10:08:43 +0100 Subject: [PATCH 03/11] Merge requests params --- src/libs/actions/PersistedRequests.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index ea1f4e9b852d..27e52a13c743 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,3 +1,4 @@ +import {merge} from 'lodash'; import isEqual from 'lodash/isEqual'; import Onyx, {OnyxUpdate} from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -33,6 +34,7 @@ function mergeOnyxUpdateData(oldData: OnyxUpdate[] = [], newData: OnyxUpdate[] = function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request { const updatedRequest = { + data: merge({...oldRequest.data}, newRequest.data), failureData: mergeOnyxUpdateData(oldRequest.failureData, newRequest.failureData), successData: mergeOnyxUpdateData(oldRequest.successData, newRequest.successData), ...newRequest, From 059306ca76764c86aebbed608234ff3e5f1d0e45 Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Tue, 31 Oct 2023 11:20:46 +0100 Subject: [PATCH 04/11] Add comments --- src/libs/actions/PersistedRequests.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index 27e52a13c743..59959318d0eb 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -18,6 +18,10 @@ function clear() { return Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, []); } +/** + * Method to merge two arrays of OnyxUpdate elements. + * Elements from the old array which keys are not present in the new array are merged with the data of the new array. + */ function mergeOnyxUpdateData(oldData: OnyxUpdate[] = [], newData: OnyxUpdate[] = []): OnyxUpdate[] { const mergedData = [...newData]; @@ -33,6 +37,9 @@ function mergeOnyxUpdateData(oldData: OnyxUpdate[] = [], newData: OnyxUpdate[] = } function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request { + /** + * In order to create updated request, properties: data, failureData, successData and optimisticData have to be merged + */ const updatedRequest = { data: merge({...oldRequest.data}, newRequest.data), failureData: mergeOnyxUpdateData(oldRequest.failureData, newRequest.failureData), @@ -54,6 +61,10 @@ function save(requestsToPersist: Request[]) { if (persistedRequests.length) { requests = [...persistedRequests]; + /** + * When we add a new request to the persistedRequests array, firstly we should check if the array already contains a request with the same idempotency key as the new request. + * If we find a matching request, we should update it, otherwise the new request will be added to the array. + */ requestsToPersist.forEach((requestToPersist) => { const index = persistedRequests.findIndex((persistedRequest) => !!requestToPersist.idempotencyKey && requestToPersist.idempotencyKey === persistedRequest.idempotencyKey); if (index > -1) { From d2122bc79b62a401d7d8499a430854b8973e1ec1 Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Tue, 31 Oct 2023 15:57:11 +0100 Subject: [PATCH 05/11] Refactor import of lodash merge function --- src/libs/actions/PersistedRequests.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index 59959318d0eb..87881d610d14 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,5 +1,5 @@ -import {merge} from 'lodash'; import isEqual from 'lodash/isEqual'; +import lodashMerge from 'lodash/merge'; import Onyx, {OnyxUpdate} from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import {Request} from '@src/types/onyx'; @@ -41,7 +41,7 @@ function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request * In order to create updated request, properties: data, failureData, successData and optimisticData have to be merged */ const updatedRequest = { - data: merge({...oldRequest.data}, newRequest.data), + data: lodashMerge({...oldRequest.data}, newRequest.data), failureData: mergeOnyxUpdateData(oldRequest.failureData, newRequest.failureData), successData: mergeOnyxUpdateData(oldRequest.successData, newRequest.successData), ...newRequest, From 405269a838d034ced6ccdd5e9c4be0af92cfe9b7 Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Fri, 3 Nov 2023 10:29:07 +0100 Subject: [PATCH 06/11] Refactor methods to send requests --- src/libs/Network/SequentialQueue.ts | 2 +- src/libs/Network/enhanceParameters.ts | 3 ++ src/libs/actions/PersistedRequests.ts | 71 +++++---------------------- src/libs/actions/Report.js | 7 +-- src/types/onyx/Request.ts | 2 +- 5 files changed, 20 insertions(+), 65 deletions(-) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index d4aee4a221e5..5da032baaf45 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 6ff54f94bc88..3fadeea7447c 100644 --- a/src/libs/Network/enhanceParameters.ts +++ b/src/libs/Network/enhanceParameters.ts @@ -37,5 +37,8 @@ export default function enhanceParameters(command: string, parameters: Record<st // Include current user's email in every request and the server logs finalParameters.email = parameters.email ?? NetworkStore.getCurrentUserEmail(); + // idempotencyKey declared in JS is front-end-only. We delete it here so it doesn't interfere with idempotency in other layers. + delete finalParameters.idempotencyKey; + return finalParameters; } diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index 87881d610d14..dbe1c6e8bef2 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,6 +1,6 @@ import isEqual from 'lodash/isEqual'; -import lodashMerge from 'lodash/merge'; -import Onyx, {OnyxUpdate} from 'react-native-onyx'; +import merge from 'lodash/merge'; +import Onyx from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import {Request} from '@src/types/onyx'; @@ -18,66 +18,17 @@ function clear() { return Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, []); } -/** - * Method to merge two arrays of OnyxUpdate elements. - * Elements from the old array which keys are not present in the new array are merged with the data of the new array. - */ -function mergeOnyxUpdateData(oldData: OnyxUpdate[] = [], newData: OnyxUpdate[] = []): OnyxUpdate[] { - const mergedData = [...newData]; - - oldData.forEach((oldUpdate) => { - const hasSameKey = newData.some((newUpdate) => newUpdate.key === oldUpdate.key); - - if (!hasSameKey) { - mergedData.push(oldUpdate); - } - }); - - return mergedData; -} - -function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request { - /** - * In order to create updated request, properties: data, failureData, successData and optimisticData have to be merged - */ - const updatedRequest = { - data: lodashMerge({...oldRequest.data}, newRequest.data), - failureData: mergeOnyxUpdateData(oldRequest.failureData, newRequest.failureData), - successData: mergeOnyxUpdateData(oldRequest.successData, newRequest.successData), - ...newRequest, - }; - - const updatedOptimisticData = mergeOnyxUpdateData(oldRequest.optimisticData, newRequest.optimisticData); - - if (updatedOptimisticData.length > 0) { - updatedRequest.optimisticData = updatedOptimisticData; - } - - return updatedRequest; -} - -function save(requestsToPersist: Request[]) { - let requests: Request[] = []; - - if (persistedRequests.length) { - requests = [...persistedRequests]; - /** - * When we add a new request to the persistedRequests array, firstly we should check if the array already contains a request with the same idempotency key as the new request. - * If we find a matching request, we should update it, otherwise the new request will be added to the array. - */ - requestsToPersist.forEach((requestToPersist) => { - const index = persistedRequests.findIndex((persistedRequest) => !!requestToPersist.idempotencyKey && requestToPersist.idempotencyKey === persistedRequest.idempotencyKey); - if (index > -1) { - requests[index] = createUpdatedRequest(requests[index], requestToPersist); - } else { - requests.push(requestToPersist); - } - }); +function save(requestToPersist: Request) { + // Check for a request w/ matching idempotencyKey in the queue + const existingRequestIndex = persistedRequests.findIndex((request) => 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, merge(persistedRequests[existingRequestIndex], requestToPersist)); } else { - requests = requestsToPersist; + // If not, push the new request to the end of the queue + persistedRequests.push(requestToPersist); } - persistedRequests = requests; - Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, requests); + Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests); } function remove(requestToRemove: Request) { diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index afbe96273ca2..87a14aac89cb 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -463,6 +463,7 @@ function reportActionsExist(reportID) { * @param {Array} participantAccountIDList The list of accountIDs that are included in a new chat, not including the user creating it */ function openReport(reportID, participantLoginList = [], newReportObject = {}, parentReportActionID = '0', isFromDeepLink = false, participantAccountIDList = []) { + const commandName = 'OpenReport'; const optimisticReportData = [ { onyxMethod: Onyx.METHOD.MERGE, @@ -521,7 +522,6 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p optimisticData: optimisticReportData, successData: reportSuccessData, failureData: reportFailureData, - idempotencyKey: `OpenReport_${reportID}`, }; const params = { @@ -529,6 +529,7 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p emailList: participantLoginList ? participantLoginList.join(',') : '', accountIDList: participantAccountIDList ? participantAccountIDList.join(',') : '', parentReportActionID, + idempotencyKey: `${commandName}_${reportID}`, }; if (isFromDeepLink) { @@ -626,12 +627,12 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p if (isFromDeepLink) { // eslint-disable-next-line rulesdir/no-api-side-effects-method - API.makeRequestWithSideEffects('OpenReport', params, onyxData).finally(() => { + API.makeRequestWithSideEffects(commandName, params, onyxData).finally(() => { Onyx.set(ONYXKEYS.IS_CHECKING_PUBLIC_ROOM, false); }); } else { // eslint-disable-next-line rulesdir/no-multiple-api-calls - API.write('OpenReport', params, onyxData); + API.write(commandName, params, onyxData); } } diff --git a/src/types/onyx/Request.ts b/src/types/onyx/Request.ts index 8f0121a31fcb..c97a5a21f488 100644 --- a/src/types/onyx/Request.ts +++ b/src/types/onyx/Request.ts @@ -5,7 +5,6 @@ type OnyxData = { successData?: OnyxUpdate[]; failureData?: OnyxUpdate[]; optimisticData?: OnyxUpdate[]; - idempotencyKey?: string; }; type RequestData = { @@ -16,6 +15,7 @@ type RequestData = { shouldUseSecure?: boolean; successData?: OnyxUpdate[]; failureData?: OnyxUpdate[]; + idempotencyKey?: string; resolve?: (value: Response) => void; reject?: (value?: unknown) => void; From 8616bd2293d096636449dc7303aa27999c3f9a5e Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Fri, 3 Nov 2023 13:41:51 +0100 Subject: [PATCH 07/11] Add some basic tests for PersistedRequests methods --- src/libs/actions/PersistedRequests.ts | 42 ++++++++++++++++- tests/unit/PersistedRequestsTest.ts | 65 +++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 tests/unit/PersistedRequestsTest.ts diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index dbe1c6e8bef2..459b24a9a1b6 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,6 +1,6 @@ import isEqual from 'lodash/isEqual'; import merge from 'lodash/merge'; -import Onyx from 'react-native-onyx'; +import Onyx, {OnyxUpdate} from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import {Request} from '@src/types/onyx'; @@ -18,12 +18,50 @@ function clear() { return Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, []); } +/** + * Method to merge two arrays of OnyxUpdate elements. + * Elements from the old array which keys are not present in the new array are merged with the data of the new array. + */ +function mergeOnyxUpdateData(oldData: OnyxUpdate[] = [], newData: OnyxUpdate[] = []): OnyxUpdate[] { + const mergedData = newData; + + oldData.forEach((oldUpdate) => { + const hasSameKey = newData.some((newUpdate) => newUpdate.key === oldUpdate.key); + + if (!hasSameKey) { + mergedData.push(oldUpdate); + } + }); + + return mergedData; +} + +function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request { + /** + * In order to create updated request, properties: data, failureData, successData and optimisticData have to be merged + */ + const updatedRequest = { + data: merge(oldRequest.data, newRequest.data), + failureData: mergeOnyxUpdateData(oldRequest.failureData, newRequest.failureData), + successData: mergeOnyxUpdateData(oldRequest.successData, newRequest.successData), + ...newRequest, + }; + + const updatedOptimisticData = mergeOnyxUpdateData(oldRequest.optimisticData, newRequest.optimisticData); + + if (updatedOptimisticData.length > 0) { + updatedRequest.optimisticData = updatedOptimisticData; + } + + return updatedRequest; +} + function save(requestToPersist: Request) { // Check for a request w/ matching idempotencyKey in the queue const existingRequestIndex = persistedRequests.findIndex((request) => 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, merge(persistedRequests[existingRequestIndex], requestToPersist)); + persistedRequests.splice(existingRequestIndex, 1, createUpdatedRequest(persistedRequests[existingRequestIndex], requestToPersist)); } else { // If not, push the new request to the end of the queue persistedRequests.push(requestToPersist); diff --git a/tests/unit/PersistedRequestsTest.ts b/tests/unit/PersistedRequestsTest.ts new file mode 100644 index 000000000000..18b9b4536772 --- /dev/null +++ b/tests/unit/PersistedRequestsTest.ts @@ -0,0 +1,65 @@ +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 PersistedRequests', () => { + 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('merge a new request with one existing in PersistedRequests array', () => { + 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(2); + expect(mergedRequest.failureData?.length).toBe(2); + }); + + it('remove a request from the PersistedRequests array', () => { + PersistedRequests.remove(request); + expect(PersistedRequests.getAll().length).toBe(0); + }); +}); From 26a8e176bede7d8a4c7d430f28cdf3e3f49fc060 Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Mon, 6 Nov 2023 10:33:38 +0100 Subject: [PATCH 08/11] Refactor logic of updating requests --- src/libs/actions/PersistedRequests.ts | 42 ++++++--------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index 459b24a9a1b6..2df9fcbf8c3c 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,5 +1,5 @@ import isEqual from 'lodash/isEqual'; -import merge from 'lodash/merge'; +import mergeWith from 'lodash/mergeWith'; import Onyx, {OnyxUpdate} from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import {Request} from '@src/types/onyx'; @@ -18,42 +18,18 @@ function clear() { return Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, []); } -/** - * Method to merge two arrays of OnyxUpdate elements. - * Elements from the old array which keys are not present in the new array are merged with the data of the new array. - */ function mergeOnyxUpdateData(oldData: OnyxUpdate[] = [], newData: OnyxUpdate[] = []): OnyxUpdate[] { - const mergedData = newData; - - oldData.forEach((oldUpdate) => { - const hasSameKey = newData.some((newUpdate) => newUpdate.key === oldUpdate.key); - - if (!hasSameKey) { - mergedData.push(oldUpdate); - } - }); - - return mergedData; + return oldData.concat(newData); } function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request { - /** - * In order to create updated request, properties: data, failureData, successData and optimisticData have to be merged - */ - const updatedRequest = { - data: merge(oldRequest.data, newRequest.data), - failureData: mergeOnyxUpdateData(oldRequest.failureData, newRequest.failureData), - successData: mergeOnyxUpdateData(oldRequest.successData, newRequest.successData), - ...newRequest, - }; - - const updatedOptimisticData = mergeOnyxUpdateData(oldRequest.optimisticData, newRequest.optimisticData); - - if (updatedOptimisticData.length > 0) { - updatedRequest.optimisticData = updatedOptimisticData; - } - - return updatedRequest; + // Merge the requests together, but concat Onyx update arrays together + return mergeWith(oldRequest, newRequest, (objValue, srcValue) => { + if (!Array.isArray(objValue) || !objValue.some((obj) => 'onyxMethod' in obj)) { + return; + } + return mergeOnyxUpdateData(objValue, srcValue); + }); } function save(requestToPersist: Request) { From fb2c8a5596b8480e0833d4512c263c9da6b56b22 Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Mon, 6 Nov 2023 11:00:09 +0100 Subject: [PATCH 09/11] Refactor createUpdatedRequest method --- src/libs/actions/PersistedRequests.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index 2df9fcbf8c3c..1683d850dfc6 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,6 +1,5 @@ import isEqual from 'lodash/isEqual'; -import mergeWith from 'lodash/mergeWith'; -import Onyx, {OnyxUpdate} from 'react-native-onyx'; +import Onyx from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import {Request} from '@src/types/onyx'; @@ -18,18 +17,14 @@ function clear() { return Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, []); } -function mergeOnyxUpdateData(oldData: OnyxUpdate[] = [], newData: OnyxUpdate[] = []): OnyxUpdate[] { - return oldData.concat(newData); -} - function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request { // Merge the requests together, but concat Onyx update arrays together - return mergeWith(oldRequest, newRequest, (objValue, srcValue) => { - if (!Array.isArray(objValue) || !objValue.some((obj) => 'onyxMethod' in obj)) { - return; - } - return mergeOnyxUpdateData(objValue, srcValue); - }); + return { + ...newRequest, + successData: [...(oldRequest.successData ?? []), ...(newRequest.successData ?? [])], + failureData: [...(oldRequest.failureData ?? []), ...(newRequest.failureData ?? [])], + optimisticData: [...(oldRequest.optimisticData ?? []), ...(newRequest.optimisticData ?? [])], + }; } function save(requestToPersist: Request) { From d9df77aae18b544edcf25b5a76001d753d14c40b Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Wed, 8 Nov 2023 09:25:59 +0100 Subject: [PATCH 10/11] Refactor createUpdatedRequest --- src/libs/actions/PersistedRequests.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index 1683d850dfc6..0cc3c334aa9e 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,5 +1,6 @@ import isEqual from 'lodash/isEqual'; -import Onyx from 'react-native-onyx'; +import mergeWith from 'lodash/mergeWith'; +import Onyx, {OnyxUpdate} from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import {Request} from '@src/types/onyx'; @@ -19,12 +20,12 @@ function clear() { function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request { // Merge the requests together, but concat Onyx update arrays together - return { - ...newRequest, - successData: [...(oldRequest.successData ?? []), ...(newRequest.successData ?? [])], - failureData: [...(oldRequest.failureData ?? []), ...(newRequest.failureData ?? [])], - optimisticData: [...(oldRequest.optimisticData ?? []), ...(newRequest.optimisticData ?? [])], - }; + return mergeWith(oldRequest, newRequest, (objValue, srcValue) => { + if (!Array.isArray(objValue) || !objValue.some((obj) => 'onyxMethod' in obj)) { + return; + } + return (objValue as OnyxUpdate[]).concat(srcValue); + }); } function save(requestToPersist: Request) { From bed4e1952e3aaba7a17244bf19432b378d7a747a Mon Sep 17 00:00:00 2001 From: Wojciech Boman <wojciechboman@gmail.com> Date: Thu, 16 Nov 2023 11:04:48 +0100 Subject: [PATCH 11/11] Refactor the save method in PersistedRequests, refactor tests --- src/libs/actions/PersistedRequests.ts | 15 ++------------- src/libs/actions/Report.js | 1 + tests/unit/PersistedRequestsTest.ts | 10 ++++++---- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index 0cc3c334aa9e..c788d69de70e 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -1,6 +1,5 @@ import isEqual from 'lodash/isEqual'; -import mergeWith from 'lodash/mergeWith'; -import Onyx, {OnyxUpdate} from 'react-native-onyx'; +import Onyx from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import {Request} from '@src/types/onyx'; @@ -18,22 +17,12 @@ function clear() { return Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, []); } -function createUpdatedRequest(oldRequest: Request, newRequest: Request): Request { - // Merge the requests together, but concat Onyx update arrays together - return mergeWith(oldRequest, newRequest, (objValue, srcValue) => { - if (!Array.isArray(objValue) || !objValue.some((obj) => 'onyxMethod' in obj)) { - return; - } - return (objValue as OnyxUpdate[]).concat(srcValue); - }); -} - function save(requestToPersist: Request) { // Check for a request w/ matching idempotencyKey in the queue const existingRequestIndex = persistedRequests.findIndex((request) => 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, createUpdatedRequest(persistedRequests[existingRequestIndex], requestToPersist)); + persistedRequests.splice(existingRequestIndex, 1, requestToPersist); } else { // If not, push the new request to the end of the queue persistedRequests.push(requestToPersist); diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 4d3c4eadd617..f067a4c63eb6 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -616,6 +616,7 @@ 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) { diff --git a/tests/unit/PersistedRequestsTest.ts b/tests/unit/PersistedRequestsTest.ts index 18b9b4536772..79153efdc806 100644 --- a/tests/unit/PersistedRequestsTest.ts +++ b/tests/unit/PersistedRequestsTest.ts @@ -20,7 +20,7 @@ afterEach(() => { }); describe('PersistedRequests', () => { - it('save a new request with an idempotency key which currently exists in 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); }); @@ -36,7 +36,7 @@ describe('PersistedRequests', () => { expect(PersistedRequests.getAll().length).toBe(2); }); - it('merge a new request with one existing in PersistedRequests array', () => { + it('replace a request existing in the PersistedRequests array with a new one', () => { const newRequest: Request = { command: 'OpenReport', data: { @@ -54,8 +54,10 @@ describe('PersistedRequests', () => { const mergedRequest = persistedRequests[0]; - expect(mergedRequest.successData?.length).toBe(2); - expect(mergedRequest.failureData?.length).toBe(2); + 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', () => {