From 4987dbfe85a443f4493ea83f11c809c62d9cbebe Mon Sep 17 00:00:00 2001 From: cretadn22 Date: Tue, 21 Jul 2026 11:09:16 +0700 Subject: [PATCH 1/2] Update , , , , and related tests to ensure consistent access to report actions across the application --- src/hooks/useDeleteTransactions.ts | 2 +- src/libs/actions/IOU/Duplicate.ts | 45 +++++++++++-------- .../TransactionDuplicate/Confirmation.tsx | 9 ++-- src/pages/iou/SplitExpensePage.tsx | 2 +- tests/actions/IOUTest/DuplicateTest.ts | 36 ++++++++++++--- 5 files changed, 65 insertions(+), 29 deletions(-) diff --git a/src/hooks/useDeleteTransactions.ts b/src/hooks/useDeleteTransactions.ts index 7cf9975cd254..84c625d64f20 100644 --- a/src/hooks/useDeleteTransactions.ts +++ b/src/hooks/useDeleteTransactions.ts @@ -247,7 +247,7 @@ function useDeleteTransactions({report, reportActions, policy}: UseDeleteTransac continue; } const originalTransaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; - const originalTransactionIouActions = getIOUActionForTransactions([transactionID], report?.reportID); + const originalTransactionIouActions = getIOUActionForTransactions([transactionID], allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.reportID}`]); const iouReportID = isMoneyRequestAction(originalTransactionIouActions.at(0)) ? originalTransactionIouActions.at(0)?.reportID : undefined; const candidateIOUReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`]; // For self-DM tracks and split bills, action.reportID resolves to a chat report, not an IOU/expense report. diff --git a/src/libs/actions/IOU/Duplicate.ts b/src/libs/actions/IOU/Duplicate.ts index 16ec5d43afc5..aa0b747900c6 100644 --- a/src/libs/actions/IOU/Duplicate.ts +++ b/src/libs/actions/IOU/Duplicate.ts @@ -59,27 +59,27 @@ import type {PerDiemExpenseInformation} from './PerDiem'; import type {CreateDistanceRequestInformation} from './Split'; import type {CreateTrackExpenseParams} from './TrackExpense'; -import {getAllReportActionsFromIOU, getAllReports, getAllTransactions} from '.'; +import {getAllReports, getAllTransactions} from '.'; import {getCleanUpTransactionThreadReportOnyxData} from './DeleteMoneyRequest'; import {getMoneyRequestParticipantsFromReport} from './MoneyRequest'; import {submitPerDiemExpense} from './PerDiem'; import {createDistanceRequest} from './Split'; import {requestMoney, trackExpense} from './TrackExpense'; -function getIOUActionForTransactions(transactionIDList: Array, iouReportID: string | undefined): Array> { - const allReportActions = getAllReportActionsFromIOU(); - return Object.values(allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReportID}`] ?? {})?.filter( - (reportAction): reportAction is OnyxTypes.ReportAction => { - if (!isMoneyRequestAction(reportAction)) { - return false; - } - const message = getOriginalMessage(reportAction); - if (!message?.IOUTransactionID) { - return false; - } - return transactionIDList.includes(message.IOUTransactionID); - }, - ); +function getIOUActionForTransactions( + transactionIDList: Array, + iouReportActions: OnyxEntry, +): Array> { + return Object.values(iouReportActions ?? {})?.filter((reportAction): reportAction is OnyxTypes.ReportAction => { + if (!isMoneyRequestAction(reportAction)) { + return false; + } + const message = getOriginalMessage(reportAction); + if (!message?.IOUTransactionID) { + return false; + } + return transactionIDList.includes(message.IOUTransactionID); + }); } type DiscardedSource = { @@ -174,6 +174,7 @@ type MergeDuplicatesFuncParams = MergeDuplicatesParams & { taxAmount?: number; taxValue?: string; allTransactionViolations: OnyxCollection; + allReportActionsList: OnyxCollection; }; /** Merge several transactions into one by updating the fields of the one we want to keep and deleting the rest */ @@ -184,6 +185,7 @@ function mergeDuplicates({ taxAmount, taxValue, allTransactionViolations, + allReportActionsList, ...params }: MergeDuplicatesFuncParams) { const allParams: MergeDuplicatesParams = {...params}; @@ -255,7 +257,7 @@ function mergeDuplicates({ if (transaction.reimbursable) { entry.reimbursableAmount += transaction.amount; } - entry.actions.push(...getIOUActionForTransactions([id], transaction.reportID)); + entry.actions.push(...getIOUActionForTransactions([id], allReportActionsList?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transaction.reportID}`])); sources.set(transaction.reportID, entry); } const deletedTime = DateUtils.getDBTime(); @@ -312,7 +314,10 @@ function mergeDuplicates({ const optimisticReportAction = buildOptimisticResolvedDuplicatesReportAction(); const transactionThreadReportID = - optimisticTransactionThreadReportID ?? (params.reportID ? getIOUActionForTransactions([params.transactionID], params.reportID).at(0)?.childReportID : undefined); + optimisticTransactionThreadReportID ?? + (params.reportID + ? getIOUActionForTransactions([params.transactionID], allReportActionsList?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${params.reportID}`]).at(0)?.childReportID + : undefined); const optimisticReportActionData: OnyxUpdate = { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReportID}`, @@ -431,12 +436,14 @@ function resolveDuplicates({ taxValue, transactionThreadReportIDMap, allTransactionViolations, + allReportActionsList, ...params }: MergeDuplicatesParams & { taxAmount?: number; taxValue?: string; transactionThreadReportIDMap: Record; allTransactionViolations: OnyxCollection; + allReportActionsList: OnyxCollection; }) { if (!params.transactionID) { return; @@ -541,7 +548,9 @@ function resolveDuplicates({ }); } - const transactionThreadReportID = params.reportID ? getIOUActionForTransactions([params.transactionID], params.reportID).at(0)?.childReportID : undefined; + const transactionThreadReportID = params.reportID + ? getIOUActionForTransactions([params.transactionID], allReportActionsList?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${params.reportID}`]).at(0)?.childReportID + : undefined; const optimisticReportAction = buildOptimisticDismissedViolationReportAction({ reason: 'manual', violationName: CONST.VIOLATIONS.DUPLICATED_TRANSACTION, diff --git a/src/pages/TransactionDuplicate/Confirmation.tsx b/src/pages/TransactionDuplicate/Confirmation.tsx index 9f76bdfb8da9..2182fa886631 100644 --- a/src/pages/TransactionDuplicate/Confirmation.tsx +++ b/src/pages/TransactionDuplicate/Confirmation.tsx @@ -82,6 +82,7 @@ function Confirmation() { const {goBack} = useReviewDuplicatesNavigation(Object.keys(compareResult.change ?? {}), 'confirmation', route.params.threadReportID, route.params.backTo); const [iouReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${newTransaction?.reportID}`); const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${newTransaction?.reportID}`); + const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS); const reportAction = Object.values(reportActions ?? {}).find( (action) => ReportActionsUtils.isMoneyRequestAction(action) && ReportActionsUtils.getOriginalMessage(action)?.IOUTransactionID === reviewDuplicates?.transactionID, ); @@ -123,7 +124,7 @@ function Confirmation() { // Suppress the NotFound guard for the discarded thread the server tears down on merge. const keptReportRoute = ROUTES.REPORT_WITH_ID.getRoute(mergeParams.reportID); setDeleteTransactionNavigateBackUrl(keptReportRoute); - mergeDuplicates({...mergeParams, ...taxData, currentUserAccountID, currentUserLogin: currentUserLogin ?? '', allTransactionViolations}); + mergeDuplicates({...mergeParams, ...taxData, currentUserAccountID, currentUserLogin: currentUserLogin ?? '', allTransactionViolations, allReportActionsList: allReportActions}); if (isSuperWideRHPDisplayed) { Navigation.dismissToSuperWideRHP(); return; @@ -137,12 +138,12 @@ function Confirmation() { Navigation.dismissModal({ afterTransition: () => Navigation.navigate(keptReportRoute, {forceReplace: true}), }); - }, [childReportID, transactionsMergeParams, taxData, currentUserAccountID, currentUserLogin, isSuperWideRHPDisplayed, allTransactionViolations]); + }, [childReportID, transactionsMergeParams, taxData, currentUserAccountID, currentUserLogin, isSuperWideRHPDisplayed, allTransactionViolations, allReportActions]); const handleResolveDuplicates = useCallback(() => { - resolveDuplicates({...transactionsMergeParams, ...taxData, transactionThreadReportIDMap, allTransactionViolations}); + resolveDuplicates({...transactionsMergeParams, ...taxData, transactionThreadReportIDMap, allTransactionViolations, allReportActionsList: allReportActions}); Navigation.dismissToSuperWideRHP(); - }, [transactionsMergeParams, taxData, transactionThreadReportIDMap, allTransactionViolations]); + }, [transactionsMergeParams, taxData, transactionThreadReportIDMap, allTransactionViolations, allReportActions]); const contextMenuStateValue = useMemo( () => ({ diff --git a/src/pages/iou/SplitExpensePage.tsx b/src/pages/iou/SplitExpensePage.tsx index 9e017c2a4737..12724441dbee 100644 --- a/src/pages/iou/SplitExpensePage.tsx +++ b/src/pages/iou/SplitExpensePage.tsx @@ -208,7 +208,7 @@ function SplitExpensePage({route}: SplitExpensePageProps) { const originalTransactionID = draftTransaction?.comment?.originalTransactionID ?? CONST.IOU.OPTIMISTIC_TRANSACTION_ID; // For selfDM expenses, the IOU action lives in the selfDM report, not in an expense report. const iouReportIDForActions = expenseReport?.reportID ?? (isSelfDM(draftTransactionReport) ? draftTransactionReport?.reportID : undefined); - const iouActions = getIOUActionForTransactions([originalTransactionID], iouReportIDForActions); + const iouActions = getIOUActionForTransactions([originalTransactionID], allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReportIDForActions}`]); const {iouReport} = useGetIOUReportFromReportAction(iouActions.at(0)); const [iouReportNextStep] = useOnyx(`${ONYXKEYS.COLLECTION.NEXT_STEP}${getNonEmptyStringOnyxID(iouReport?.reportID)}`); diff --git a/tests/actions/IOUTest/DuplicateTest.ts b/tests/actions/IOUTest/DuplicateTest.ts index b5f5a0f7bcc2..703ccc7355a2 100644 --- a/tests/actions/IOUTest/DuplicateTest.ts +++ b/tests/actions/IOUTest/DuplicateTest.ts @@ -25,7 +25,7 @@ import type {ReportActionsCollectionDataSet} from '@src/types/onyx/ReportAction' import type Transaction from '@src/types/onyx/Transaction'; import type {TransactionCollectionDataSet} from '@src/types/onyx/Transaction'; -import type {OnyxEntry, OnyxInputValue} from 'react-native-onyx'; +import type {OnyxCollection, OnyxEntry, OnyxInputValue} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; @@ -229,6 +229,10 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate1ID}`]: duplicate1Violations, [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate2ID}`]: duplicate2Violations, }, + allReportActionsList: { + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {action456: iouAction1, action789: iouAction2}, + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${childReportID}`]: {}, + }, }); await waitForBatchedUpdates(); @@ -315,6 +319,7 @@ describe('actions/Duplicate', () => { currentUserLogin: RORY_EMAIL, currentUserAccountID: RORY_ACCOUNT_ID, allTransactionViolations: {[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${mainTransactionID}`]: []}, + allReportActionsList: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {}}, }); await waitForBatchedUpdates(); @@ -374,6 +379,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${mainTransactionID}`]: [], [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate1ID}`]: [], }, + allReportActionsList: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {}}, }); await waitForBatchedUpdates(); @@ -559,6 +565,9 @@ describe('actions/Duplicate', () => { expect(iouAction1?.childVisibleActionCount).toBe(2); expect(iouAction2?.childVisibleActionCount).toBe(1); expect(previewAction?.childVisibleActionCount).toBe(3); + if (!iouAction1 || !iouAction2) { + throw new Error('Expected iouAction1 and iouAction2 to be defined after fetching from Onyx.'); + } await waitForBatchedUpdates(); @@ -589,6 +598,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate1ID}`]: duplicate1Violations, [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate2ID}`]: duplicate2Violations, }, + allReportActionsList: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {[iouAction1ID]: iouAction1, [iouAction2ID]: iouAction2}}, }); await waitForBatchedUpdates(); @@ -697,6 +707,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${mainTransactionID}`]: mainViolations, [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate1ID}`]: duplicate1Violations, }, + allReportActionsList: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {mainAction123: mainIouAction, action456: dupIouAction}}, }); await waitForBatchedUpdates(); @@ -784,6 +795,10 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${mainTransactionID}`]: mainViolations, [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${crossReportDuplicateID}`]: crossDuplicateViolations, }, + allReportActionsList: { + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${keptReportID}`]: {actionMain: mainIouAction}, + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${crossReportID}`]: {actionCross: crossIouAction}, + }, }); await waitForBatchedUpdates(); @@ -866,13 +881,14 @@ describe('actions/Duplicate', () => { {name: CONST.VIOLATIONS.MISSING_CATEGORY, type: CONST.VIOLATION_TYPES.VIOLATION}, ]; - const createMockIouAction = (transactionID: string, reportActionID: string, childReportID: string) => ({ + const createMockIouAction = (transactionID: string, reportActionID: string, childReportID: string): ReportAction => ({ reportActionID, actionName: CONST.REPORT.ACTIONS.TYPE.IOU, - originalMessage: { + created: '2024-01-01 12:00:00', + originalMessage: createMock({ IOUTransactionID: transactionID, type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, - }, + }), message: [{type: 'TEXT', text: 'Test IOU message'}], childReportID, }); @@ -943,6 +959,9 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate1ID}`]: duplicate1Violations, [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate2ID}`]: duplicate2Violations, }, + allReportActionsList: { + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {action456: iouAction1, action789: iouAction2, mainAction123: mainIouAction}, + }, }); await waitForBatchedUpdates(); @@ -1035,7 +1054,7 @@ describe('actions/Duplicate', () => { }; // When: Call resolveDuplicates with undefined transactionID - resolveDuplicates({...resolveParams, allTransactionViolations: {}}); + resolveDuplicates({...resolveParams, allTransactionViolations: {}, allReportActionsList: undefined}); await waitForBatchedUpdates(); // Then: Verify API was not called @@ -1081,6 +1100,7 @@ describe('actions/Duplicate', () => { resolveDuplicates({ ...resolveParams, allTransactionViolations: {[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${mainTransactionID}`]: mainViolations}, + allReportActionsList: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {mainAction123: mainIouAction}}, }); await waitForBatchedUpdates(); @@ -1144,6 +1164,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${mainTransactionID}`]: mainViolations, [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate1ID}`]: duplicateViolations, }, + allReportActionsList: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {}}, }); await waitForBatchedUpdates(); @@ -1229,6 +1250,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${mainTransactionID}`]: mainViolations, [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${crossReportDuplicateID}`]: crossDuplicateViolations, }, + allReportActionsList: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportA}`]: {actionMain: mainIouAction}}, }); await waitForBatchedUpdates(); @@ -2210,6 +2232,9 @@ describe('actions/Duplicate', () => { actions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouAction.reportActionID}`] = iouAction; } const actionCollectionDataSet: ReportActionsCollectionDataSet = {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport.reportID}`]: actions}; + const allReportActionsList: OnyxCollection = { + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport.reportID}`]: Object.fromEntries(iouActions.map((iouAction) => [iouAction.reportActionID, iouAction])), + }; return waitForBatchedUpdates() .then(() => Onyx.multiSet({...transactionCollectionDataSet, ...actionCollectionDataSet})) @@ -2228,6 +2253,7 @@ describe('actions/Duplicate', () => { [transaction2.transactionID]: 'transactionThread-2', }, allTransactionViolations: {}, + allReportActionsList, }); return waitForBatchedUpdates(); }) From 7ca6a188e7e3153c76802f15ca13308db6a19da6 Mon Sep 17 00:00:00 2001 From: cretadn22 Date: Tue, 21 Jul 2026 15:50:59 +0700 Subject: [PATCH 2/2] DuplicateTest to include filtering of non-money-request and IOU actions without transaction IDs in resolveDuplicates --- tests/actions/IOUTest/DuplicateTest.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/actions/IOUTest/DuplicateTest.ts b/tests/actions/IOUTest/DuplicateTest.ts index 703ccc7355a2..ed23c0459db5 100644 --- a/tests/actions/IOUTest/DuplicateTest.ts +++ b/tests/actions/IOUTest/DuplicateTest.ts @@ -1137,7 +1137,14 @@ describe('actions/Duplicate', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${duplicate1ID}`, duplicateTransaction); await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${mainTransactionID}`, mainViolations); await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate1ID}`, duplicateViolations); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {}); + // A non-money-request action and an IOU action with no IOUTransactionID should both be filtered out without matching. + const commentAction = createMock({reportActionID: 'commentAction', actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT}); + const iouActionWithoutTransactionID = createMock({ + reportActionID: 'iouActionWithoutTransactionID', + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + originalMessage: createMock({type: CONST.IOU.REPORT_ACTION_TYPE.CREATE}), + }); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {commentAction, iouActionWithoutTransactionID}); await waitForBatchedUpdates(); const resolveParams = { @@ -1157,14 +1164,14 @@ describe('actions/Duplicate', () => { transactionThreadReportIDMap: {}, }; - // When: Call resolveDuplicates without IOU actions + // When: Call resolveDuplicates without matching IOU actions resolveDuplicates({ ...resolveParams, allTransactionViolations: { [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${mainTransactionID}`]: mainViolations, [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${duplicate1ID}`]: duplicateViolations, }, - allReportActionsList: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {}}, + allReportActionsList: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: {commentAction, iouActionWithoutTransactionID}}, }); await waitForBatchedUpdates();