From 9d58ea7e5b4a2c9a103030062a8d75b1ce0d6677 Mon Sep 17 00:00:00 2001 From: Povilas Zirgulis Date: Fri, 18 Oct 2024 17:21:09 +0300 Subject: [PATCH 1/6] add filterUpdateCommentRequest and filterRequestByCommand utils --- src/libs/actions/App.ts | 6 +++--- src/libs/actions/Report.ts | 10 +++++++++- src/libs/actions/RequestConflictUtils.ts | 22 +++++++++++++++++++--- tests/unit/RequestConflictUtilsTest.ts | 6 +++--- 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/libs/actions/App.ts b/src/libs/actions/App.ts index 71cb5f97e00e..8066fa78e4ed 100644 --- a/src/libs/actions/App.ts +++ b/src/libs/actions/App.ts @@ -27,7 +27,7 @@ import type {OnyxData} from '@src/types/onyx/Request'; import {setShouldForceOffline} from './Network'; import * as PersistedRequests from './PersistedRequests'; import * as Policy from './Policy/Policy'; -import resolveDuplicationConflictAction from './RequestConflictUtils'; +import {filterRequestByCommand, resolveDuplicationConflictAction} from './RequestConflictUtils'; import * as Session from './Session'; import Timing from './Timing'; @@ -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, filterRequestByCommand(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, filterRequestByCommand(WRITE_COMMANDS.RECONNECT_APP)), }); }); } diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 87bec3bc30ea..c805240faf4b 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -107,6 +107,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import * as CachedPDFPaths from './CachedPDFPaths'; import * as Modal from './Modal'; import navigateFromNotification from './navigateFromNotification'; +import {filterUpdateCommentRequest, resolveDuplicationConflictAction} from './RequestConflictUtils'; import * as Session from './Session'; import * as Welcome from './Welcome'; import * as OnboardingFlow from './Welcome/OnboardingFlow'; @@ -1687,7 +1688,14 @@ function editReportComment(reportID: string, originalReportAction: OnyxEntry resolveDuplicationConflictAction(persistedRequests, filterUpdateCommentRequest(reportActionID)), + }, + ); } /** Deletes the draft for a comment report action. */ diff --git a/src/libs/actions/RequestConflictUtils.ts b/src/libs/actions/RequestConflictUtils.ts index 68c0860389b9..591ea72d0096 100644 --- a/src/libs/actions/RequestConflictUtils.ts +++ b/src/libs/actions/RequestConflictUtils.ts @@ -2,6 +2,18 @@ import type {WriteCommand} from '@libs/API/types'; import type OnyxRequest from '@src/types/onyx/Request'; import type {ConflictActionData} from '@src/types/onyx/Request'; +function filterUpdateCommentRequest(reportActionID: string) { + return function (request: OnyxRequest) { + return request.command === 'UpdateComment' && request.data?.reportActionID === reportActionID; + }; +} + +function filterRequestByCommand(requestCommand: WriteCommand) { + return function (request: OnyxRequest) { + return request.command === requestCommand; + }; +} + /** * Resolves duplication conflicts between persisted requests and a given command. * @@ -9,8 +21,12 @@ 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); + +type RequestMatcher = (request: OnyxRequest) => boolean; + +function resolveDuplicationConflictAction(persistedRequests: OnyxRequest[], requestMatcher: RequestMatcher): ConflictActionData { + const index = persistedRequests.findIndex(requestMatcher); + if (index === -1) { return { conflictAction: { @@ -27,4 +43,4 @@ function resolveDuplicationConflictAction(persistedRequests: OnyxRequest[], comm }; } -export default resolveDuplicationConflictAction; +export {resolveDuplicationConflictAction, filterUpdateCommentRequest, filterRequestByCommand}; diff --git a/tests/unit/RequestConflictUtilsTest.ts b/tests/unit/RequestConflictUtilsTest.ts index d2d003192456..e1470be0f71f 100644 --- a/tests/unit/RequestConflictUtilsTest.ts +++ b/tests/unit/RequestConflictUtilsTest.ts @@ -1,11 +1,11 @@ -import resolveDuplicationConflictAction from '@libs/actions/RequestConflictUtils'; +import {filterRequestByCommand, resolveDuplicationConflictAction} from '@libs/actions/RequestConflictUtils'; import type {WriteCommand} from '@libs/API/types'; 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, filterRequestByCommand(commandToFind)); expect(result).toEqual({conflictAction: {type: 'push'}}); }); @@ -15,7 +15,7 @@ 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, filterRequestByCommand(commandToFind)); expect(result).toEqual({conflictAction: {type: 'replace', index}}); }); }); From 5846533b498e723df037382c4de35e67b53c50ee Mon Sep 17 00:00:00 2001 From: Povilas Zirgulis Date: Mon, 21 Oct 2024 11:42:11 +0300 Subject: [PATCH 2/6] use constant instead of string --- src/libs/actions/RequestConflictUtils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/RequestConflictUtils.ts b/src/libs/actions/RequestConflictUtils.ts index 591ea72d0096..af0b91160703 100644 --- a/src/libs/actions/RequestConflictUtils.ts +++ b/src/libs/actions/RequestConflictUtils.ts @@ -1,10 +1,11 @@ +import {WRITE_COMMANDS} from '@libs/API/types'; import type {WriteCommand} from '@libs/API/types'; import type OnyxRequest from '@src/types/onyx/Request'; import type {ConflictActionData} from '@src/types/onyx/Request'; function filterUpdateCommentRequest(reportActionID: string) { return function (request: OnyxRequest) { - return request.command === 'UpdateComment' && request.data?.reportActionID === reportActionID; + return request.command === WRITE_COMMANDS.UPDATE_COMMENT && request.data?.reportActionID === reportActionID; }; } From a7eb4fd61bdda3971c2b04f0559701b078d78811 Mon Sep 17 00:00:00 2001 From: Povilas Zirgulis Date: Mon, 21 Oct 2024 11:45:25 +0300 Subject: [PATCH 3/6] rename matcher fn to be more descriptive --- src/libs/actions/Report.ts | 4 ++-- src/libs/actions/RequestConflictUtils.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index c805240faf4b..306deb564893 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -107,7 +107,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import * as CachedPDFPaths from './CachedPDFPaths'; import * as Modal from './Modal'; import navigateFromNotification from './navigateFromNotification'; -import {filterUpdateCommentRequest, resolveDuplicationConflictAction} from './RequestConflictUtils'; +import {createUpdateCommentMatcher, resolveDuplicationConflictAction} from './RequestConflictUtils'; import * as Session from './Session'; import * as Welcome from './Welcome'; import * as OnboardingFlow from './Welcome/OnboardingFlow'; @@ -1693,7 +1693,7 @@ function editReportComment(reportID: string, originalReportAction: OnyxEntry resolveDuplicationConflictAction(persistedRequests, filterUpdateCommentRequest(reportActionID)), + checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, createUpdateCommentMatcher(reportActionID)), }, ); } diff --git a/src/libs/actions/RequestConflictUtils.ts b/src/libs/actions/RequestConflictUtils.ts index af0b91160703..8276ce425163 100644 --- a/src/libs/actions/RequestConflictUtils.ts +++ b/src/libs/actions/RequestConflictUtils.ts @@ -3,7 +3,7 @@ import type {WriteCommand} from '@libs/API/types'; import type OnyxRequest from '@src/types/onyx/Request'; import type {ConflictActionData} from '@src/types/onyx/Request'; -function filterUpdateCommentRequest(reportActionID: string) { +function createUpdateCommentMatcher(reportActionID: string) { return function (request: OnyxRequest) { return request.command === WRITE_COMMANDS.UPDATE_COMMENT && request.data?.reportActionID === reportActionID; }; @@ -44,4 +44,4 @@ function resolveDuplicationConflictAction(persistedRequests: OnyxRequest[], requ }; } -export {resolveDuplicationConflictAction, filterUpdateCommentRequest, filterRequestByCommand}; +export {resolveDuplicationConflictAction, createUpdateCommentMatcher, filterRequestByCommand}; From 4f752c7f04d7cae0f8910bb7ed08b53de4781e05 Mon Sep 17 00:00:00 2001 From: Povilas Zirgulis Date: Mon, 21 Oct 2024 12:01:18 +0300 Subject: [PATCH 4/6] update requestMatcher by request.command usage --- src/libs/actions/App.ts | 6 +++--- src/libs/actions/RequestConflictUtils.ts | 9 +-------- tests/unit/RequestConflictUtilsTest.ts | 6 +++--- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/libs/actions/App.ts b/src/libs/actions/App.ts index 8066fa78e4ed..fe15b25e5e52 100644 --- a/src/libs/actions/App.ts +++ b/src/libs/actions/App.ts @@ -27,7 +27,7 @@ import type {OnyxData} from '@src/types/onyx/Request'; import {setShouldForceOffline} from './Network'; import * as PersistedRequests from './PersistedRequests'; import * as Policy from './Policy/Policy'; -import {filterRequestByCommand, resolveDuplicationConflictAction} from './RequestConflictUtils'; +import {resolveDuplicationConflictAction} from './RequestConflictUtils'; import * as Session from './Session'; import Timing from './Timing'; @@ -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, filterRequestByCommand(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, filterRequestByCommand(WRITE_COMMANDS.RECONNECT_APP)), + checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, (request) => request.command === WRITE_COMMANDS.RECONNECT_APP), }); }); } diff --git a/src/libs/actions/RequestConflictUtils.ts b/src/libs/actions/RequestConflictUtils.ts index 8276ce425163..f3201ea3ea40 100644 --- a/src/libs/actions/RequestConflictUtils.ts +++ b/src/libs/actions/RequestConflictUtils.ts @@ -1,5 +1,4 @@ import {WRITE_COMMANDS} from '@libs/API/types'; -import type {WriteCommand} from '@libs/API/types'; import type OnyxRequest from '@src/types/onyx/Request'; import type {ConflictActionData} from '@src/types/onyx/Request'; @@ -9,12 +8,6 @@ function createUpdateCommentMatcher(reportActionID: string) { }; } -function filterRequestByCommand(requestCommand: WriteCommand) { - return function (request: OnyxRequest) { - return request.command === requestCommand; - }; -} - /** * Resolves duplication conflicts between persisted requests and a given command. * @@ -44,4 +37,4 @@ function resolveDuplicationConflictAction(persistedRequests: OnyxRequest[], requ }; } -export {resolveDuplicationConflictAction, createUpdateCommentMatcher, filterRequestByCommand}; +export {resolveDuplicationConflictAction, createUpdateCommentMatcher}; diff --git a/tests/unit/RequestConflictUtilsTest.ts b/tests/unit/RequestConflictUtilsTest.ts index e1470be0f71f..adab16989170 100644 --- a/tests/unit/RequestConflictUtilsTest.ts +++ b/tests/unit/RequestConflictUtilsTest.ts @@ -1,11 +1,11 @@ -import {filterRequestByCommand, resolveDuplicationConflictAction} from '@libs/actions/RequestConflictUtils'; +import {resolveDuplicationConflictAction} from '@libs/actions/RequestConflictUtils'; import type {WriteCommand} from '@libs/API/types'; 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, filterRequestByCommand(commandToFind)); + const result = resolveDuplicationConflictAction(persistedRequests, (request) => request.command === commandToFind); expect(result).toEqual({conflictAction: {type: 'push'}}); }); @@ -15,7 +15,7 @@ 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, filterRequestByCommand(commandToFind)); + const result = resolveDuplicationConflictAction(persistedRequests, (request) => request.command === commandToFind); expect(result).toEqual({conflictAction: {type: 'replace', index}}); }); }); From 16d8f96f0214783335f1c6e2a649827fd00d7bfd Mon Sep 17 00:00:00 2001 From: Povilas Zirgulis Date: Mon, 21 Oct 2024 15:47:56 +0300 Subject: [PATCH 5/6] add UpdateComment test --- tests/actions/ReportTest.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 0ffb0ee9bc08..86bc3a6e3ff2 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,27 @@ describe('actions/Report', () => { expect(reportActionReaction?.[EMOJI.name].users[TEST_USER_ACCOUNT_ID]).toBeUndefined(); }); }); + + it('it should only store the last sequential UpdateComment request in Onyx', () => { + const reportID = '123'; + + Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); + + const action: OnyxEntry = { + reportID, + reportActionID: '722', + actionName: 'ADDCOMMENT', + created: '2024-10-21 10:37:59.881', + }; + + Report.editReportComment(reportID, action, 'value1'); + Report.editReportComment(reportID, action, 'value2'); + Report.editReportComment(reportID, action, 'value3'); + + const requests = PersistedRequests?.getAll(); + + expect(requests.length).toBe(1); + expect(requests?.at(0)?.command).toBe(WRITE_COMMANDS.UPDATE_COMMENT); + expect(requests?.at(0)?.data?.reportComment).toBe('value3'); + }); }); From 6bbc963c74145fefd336cb322863b908cfb9b8f2 Mon Sep 17 00:00:00 2001 From: Povilas Zirgulis Date: Mon, 21 Oct 2024 18:47:31 +0300 Subject: [PATCH 6/6] update test with request check --- tests/actions/ReportTest.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 86bc3a6e3ff2..26dbffdd9bd2 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -759,10 +759,11 @@ describe('actions/Report', () => { }); }); - it('it should only store the last sequential UpdateComment request in Onyx', () => { + it('it should only send the last sequential UpdateComment request to BE', async () => { + global.fetch = TestHelper.getGlobalFetchMock(); const reportID = '123'; - Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); + await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); const action: OnyxEntry = { reportID, @@ -780,5 +781,9 @@ describe('actions/Report', () => { expect(requests.length).toBe(1); expect(requests?.at(0)?.command).toBe(WRITE_COMMANDS.UPDATE_COMMENT); expect(requests?.at(0)?.data?.reportComment).toBe('value3'); + + await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false}); + + TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.UPDATE_COMMENT, 1); }); });