From 67f23f8c0034aec4f793eaac8a5803b173f94774 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Tue, 21 Jul 2026 16:30:43 +0430 Subject: [PATCH 01/27] Fix merge expense transaction thread navigation --- src/hooks/useSearchBulkActions.ts | 3 +- src/libs/actions/MergeTransaction.ts | 20 ++- src/libs/actions/Report/index.ts | 12 +- .../TransactionMerge/ConfirmationPage.tsx | 40 ++++- tests/actions/MergeTransactionTest.ts | 161 +++++++++++++++++- tests/actions/ReportTest.ts | 33 ++++ 6 files changed, 246 insertions(+), 23 deletions(-) diff --git a/src/hooks/useSearchBulkActions.ts b/src/hooks/useSearchBulkActions.ts index 6e70828aa668..ee8c866b9986 100644 --- a/src/hooks/useSearchBulkActions.ts +++ b/src/hooks/useSearchBulkActions.ts @@ -2024,7 +2024,8 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) { text: translate('common.merge'), icon: expensifyIcons.ArrowCollapse, value: CONST.SEARCH.BULK_ACTION_TYPES.MERGE, - onSelected: () => setupMergeTransactionDataAndNavigate(transactionID, searchedTransactions, localeCompare, getCurrencyDecimals, reports, false, true), + onSelected: () => + setupMergeTransactionDataAndNavigate(transactionID, searchedTransactions, localeCompare, getCurrencyDecimals, reports, false, true, transactionPolicies), }); } } diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index c9c8b6d94906..df0925f36969 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -455,6 +455,7 @@ function mergeTransactionRequest({ // Optimistic delete the source transaction and also delete its report if it was a single expense report const isUnreportedSourceTransaction = sourceTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; const selfDMReportID = selfDMReport?.reportID; + const optimisticDestinationReportID = mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID ? (selfDMReportID ?? mergeTransaction.reportID) : mergeTransaction.reportID; const sourceTransactionOptimisticData: Array< OnyxUpdate @@ -653,7 +654,8 @@ function mergeTransactionRequest({ successData.push(...(onyxTargetTransactionData.successData ?? [])); // User picked a destination report other than the target expense's current report (e.g. the source report). - // Move the surviving expense (targetTransaction) onto mergeTransaction.reportID. + // Move the surviving expense (targetTransaction) onto the destination report. For unreported + // expenses, the API uses reportID "0", but optimistic UI must attach to the real self-DM report. if (mergeTransaction.reportID !== targetTransaction.reportID) { const newIOUAction = buildOptimisticIOUReportAction({ type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, @@ -662,14 +664,14 @@ function mergeTransactionRequest({ comment: mergeTransaction.description, participants: [], transactionID: mergeTransaction.targetTransactionID, - iouReportID: mergeTransaction.reportID, + iouReportID: optimisticDestinationReportID, // delegateAccountIDParam: will be threaded in PR 11; buildOptimisticIOUReportAction falls back to module-level Onyx.connect value (https://github.com/Expensify/App/issues/66425) delegateAccountIDParam: undefined, }); // IOU action for the surviving expense on its original report (not on mergeTransaction.reportID yet). const targetIOUActionOnOriginalReport = getIOUActionForReportID(targetTransaction.reportID, targetTransaction.transactionID); - const targetTransactionThreadReportID = targetIOUActionOnOriginalReport?.childReportID; + const targetTransactionThreadReportID = targetIOUActionOnOriginalReport?.childReportID ?? targetTransactionThreadReport?.reportID; if (targetTransactionThreadReportID) { // Reattach the transaction thread to the new IOU action on the destination report. @@ -678,7 +680,7 @@ function mergeTransactionRequest({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`, value: { - parentReportID: mergeTransaction.reportID, + parentReportID: optimisticDestinationReportID, parentReportActionID: newIOUAction.reportActionID, }, }); @@ -687,15 +689,15 @@ function mergeTransactionRequest({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`, value: { - parentReportID: targetTransaction.reportID, - parentReportActionID: targetIOUActionOnOriginalReport.reportActionID, + parentReportID: targetTransactionThreadReport?.parentReportID ?? targetTransaction.reportID, + parentReportActionID: targetTransactionThreadReport?.parentReportActionID ?? targetIOUActionOnOriginalReport?.reportActionID, }, }); } optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${mergeTransaction.reportID}`, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, value: { [newIOUAction.reportActionID]: newIOUAction, }, @@ -703,7 +705,7 @@ function mergeTransactionRequest({ successData.push({ onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${mergeTransaction.reportID}`, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, value: { [newIOUAction.reportActionID]: {pendingAction: null}, }, @@ -711,7 +713,7 @@ function mergeTransactionRequest({ failureData.push({ onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${mergeTransaction.reportID}`, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, value: { [newIOUAction.reportActionID]: null, }, diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 65849bffdf4b..cdef79a3299d 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -367,6 +367,9 @@ type OpenReportActionParams = { /** Whether the report has report actions or not */ hasReportActions?: boolean; + /** Whether report actions are already available optimistically, so initial loading should be considered complete */ + hasOptimisticReportActions?: boolean; + /** Whether opening the report should update its read state. Set to false when fetching report data without the user actually viewing the conversation */ shouldMarkAsRead?: boolean; @@ -1538,6 +1541,7 @@ function openReport(params: OpenReportActionParams) { isSelfTourViewed, hasCompletedGuidedSetupFlow, hasReportActions, + hasOptimisticReportActions = false, shouldMarkAsRead = true, } = params; if (!reportID) { @@ -1569,7 +1573,8 @@ function openReport(params: OpenReportActionParams) { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportID}`, value: { - isLoadingInitialReportActions: true, + hasOnceLoadedReportActions: hasOptimisticReportActions ? true : undefined, + isLoadingInitialReportActions: !hasOptimisticReportActions, isLoadingOlderReportActions: false, hasLoadingOlderReportActionsError: false, isLoadingNewerReportActions: false, @@ -2233,6 +2238,9 @@ type CreateTransactionThreadReportParams = { /** Whether the user has completed the guided setup flow */ // TODO: This will be required eventually. Refactor issue: https://github.com/Expensify/App/issues/66424 hasCompletedGuidedSetupFlow?: boolean; + + /** Whether the transaction thread actions are fully seeded optimistically */ + hasOptimisticReportActions?: boolean; }; function createTransactionThreadReport(params: CreateTransactionThreadReportParams): OptimisticChatReport | undefined { @@ -2248,6 +2256,7 @@ function createTransactionThreadReport(params: CreateTransactionThreadReportPara personalDetails, isSelfTourViewed, hasCompletedGuidedSetupFlow, + hasOptimisticReportActions, } = params; // Determine if we need selfDM report (for track expenses or unreported transactions) @@ -2310,6 +2319,7 @@ function createTransactionThreadReport(params: CreateTransactionThreadReportPara currentUserAccountID, isSelfTourViewed, hasCompletedGuidedSetupFlow, + hasOptimisticReportActions, betas, }); return optimisticTransactionThread; diff --git a/src/pages/TransactionMerge/ConfirmationPage.tsx b/src/pages/TransactionMerge/ConfirmationPage.tsx index 30c79b5c52bd..d0302514a55f 100644 --- a/src/pages/TransactionMerge/ConfirmationPage.tsx +++ b/src/pages/TransactionMerge/ConfirmationPage.tsx @@ -18,12 +18,14 @@ import useSelfDMReport from '@hooks/useSelfDMReport'; import useThemeStyles from '@hooks/useThemeStyles'; import {mergeTransactionRequest} from '@libs/actions/MergeTransaction'; +import {createTransactionThreadReport} from '@libs/actions/Report'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import {buildMergedTransactionData, getTransactionThreadReportID} from '@libs/MergeTransactionUtils'; import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute'; import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; import type {MergeTransactionNavigatorParamList} from '@libs/Navigation/types'; +import {getIOUActionForReportID} from '@libs/ReportActionsUtils'; import {findSelfDMReportID} from '@libs/ReportUtils'; import type {SkeletonSpanReasonAttributes} from '@libs/telemetry/useSkeletonSpan'; @@ -64,11 +66,13 @@ function ConfirmationPage({route}: ConfirmationPageProps) { const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT); const targetTransactionThreadReportID = getTransactionThreadReportID(targetTransaction); - const [targetTransactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`); + const [targetTransactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(targetTransactionThreadReportID)}`); const [targetTransactionThreadParentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(targetTransactionThreadReport?.parentReportID)}`); const [targetTransactionThreadParentReportNextStep] = useOnyx(`${ONYXKEYS.COLLECTION.NEXT_STEP}${getNonEmptyStringOnyxID(targetTransactionThreadReport?.parentReportID)}`); const [iouReportOwnerLogin] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {selector: personalDetailsLoginSelector(targetTransactionThreadParentReport?.ownerAccountID)}); const [reportPolicyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${getNonEmptyStringOnyxID(targetTransactionThreadParentReport?.policyID)}`); + const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED); + const [betas] = useOnyx(ONYXKEYS.BETAS); const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector}); const selfDMReport = useSelfDMReport(); @@ -85,13 +89,38 @@ function ConfirmationPage({route}: ConfirmationPageProps) { setIsMergingExpenses(true); + let targetTransactionThreadReportIDForMerge = targetTransactionThreadReportID; + let targetTransactionThreadReportForMerge = targetTransactionThreadReport; + let targetTransactionThreadParentReportForMerge = targetTransactionThreadParentReport; + + if (!targetTransactionThreadReportIDForMerge) { + const targetIOUAction = getIOUActionForReportID(targetTransaction.reportID, targetTransaction.transactionID); + const createdTransactionThreadReport = createTransactionThreadReport({ + introSelected, + currentUserLogin: currentUserEmailParam, + currentUserAccountID: currentUserAccountIDParam, + betas, + iouReport: targetTransactionReport, + iouReportAction: targetIOUAction, + transaction: targetTransaction, + transactionViolations: allTransactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${targetTransaction.transactionID}`], + hasOptimisticReportActions: true, + }); + + if (createdTransactionThreadReport) { + targetTransactionThreadReportIDForMerge = createdTransactionThreadReport.reportID; + targetTransactionThreadReportForMerge = createdTransactionThreadReport; + targetTransactionThreadParentReportForMerge = targetTransactionReport; + } + } + mergeTransactionRequest({ mergeTransactionID: transactionID, mergeTransaction, targetTransaction, sourceTransaction, - targetTransactionThreadReport, - targetTransactionThreadParentReport, + targetTransactionThreadReport: targetTransactionThreadReportForMerge, + targetTransactionThreadParentReport: targetTransactionThreadParentReportForMerge, targetTransactionThreadParentReportNextStep, iouReportOwnerLogin, allTransactionViolations, @@ -110,10 +139,11 @@ function ConfirmationPage({route}: ConfirmationPageProps) { const reportIDToDismiss = reportID !== CONST.REPORT.UNREPORTED_REPORT_ID ? reportID : undefined; - const searchReportIDToOpen = targetTransactionThreadReportID ?? reportIDToDismiss; + const searchReportIDToOpen = targetTransactionThreadReportIDForMerge ?? reportIDToDismiss; + const isSearchTopmost = isSearchTopmostFullScreenRoute(); // If we're in search (or the topmost route is search), dismiss the modal and open the expense in the RHP - if ((isOnSearch || isSearchTopmostFullScreenRoute()) && searchReportIDToOpen) { + if ((isOnSearch || isSearchTopmost) && searchReportIDToOpen) { Navigation.dismissModal(); Navigation.setNavigationActionToMicrotaskQueue(() => { Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute({reportID: searchReportIDToOpen})); diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index b929fc7eff3a..e813584b6310 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -3,7 +3,7 @@ import {areTransactionsEligibleForMerge, getTransactionsForMerging, mergeTransac import {addComment, openReport} from '@libs/actions/Report'; import {WRITE_COMMANDS} from '@libs/API/types'; import {getLoginsByAccountIDs} from '@libs/PersonalDetailsUtils'; -import {getOriginalMessage, getReportAction} from '@libs/ReportActionsUtils'; +import {getOriginalMessage, getReportAction, isMoneyRequestAction} from '@libs/ReportActionsUtils'; import {buildTransactionThread} from '@libs/ReportUtils'; import CONST from '@src/CONST'; @@ -66,6 +66,15 @@ function createAllTransactionViolations( return allViolations; } +function isIOUActionForTransaction(reportAction: OnyxEntry, transactionID: string): boolean { + if (!isMoneyRequestAction(reportAction)) { + return false; + } + + const originalMessage = getOriginalMessage(reportAction); + return reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && originalMessage?.type === CONST.IOU.REPORT_ACTION_TYPE.CREATE && originalMessage?.IOUTransactionID === transactionID; +} + type CrossReportMergeToSourceReportFixtures = { targetTransaction: Transaction; sourceTransaction: Transaction; @@ -75,6 +84,7 @@ type CrossReportMergeToSourceReportFixtures = { targetReport: Report; sourceIOUActionID: string; targetIOUActionID: string; + targetIOUAction: ReportAction; targetTransactionThreadID: string; targetTransactionThread: Report; mockViolations: TransactionViolation[]; @@ -176,6 +186,7 @@ async function setupCrossReportMergeToSourceReportFixtures(): Promise { // The source IOU action is no longer removed — only the target report's IOU action is handled in the cross-report branch expect(sourceReportActions?.[sourceIOUActionID]).toBeDefined(); + const newIOUAction = Object.values(sourceReportActions ?? {}).find((action) => isIOUActionForTransaction(action, targetTransaction.transactionID)); + + // Verify the new IOU action is created and points to the merged transaction + expect(newIOUAction).toBeTruthy(); + expect((getOriginalMessage(newIOUAction) as OriginalMessageIOU | undefined)?.IOUTransactionID).toBe(targetTransaction.transactionID); + + const updatedTargetTransactionThread = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, + callback: (report) => { + Onyx.disconnect(connection); + resolve(report ?? null); + }, + }); + }); + + // Verify target transaction thread parent references are rewired to the new IOU action in the source report + expect(updatedTargetTransactionThread?.parentReportID).toBe(sourceExpenseReport.reportID); + expect(updatedTargetTransactionThread?.parentReportActionID).toBe(newIOUAction?.reportActionID); + }); + + it('should reparent a passed transaction thread when the original IOU action has no child report ID', async () => { + const fixtures = await setupCrossReportMergeToSourceReportFixtures(); + const {targetTransaction, sourceExpenseReport, targetReport, targetIOUActionID, targetIOUAction, targetTransactionThreadID, targetTransactionThread} = fixtures; + + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${targetReport.reportID}`, { + [targetIOUActionID]: { + ...targetIOUAction, + childReportID: undefined, + }, + }); + await waitForBatchedUpdates(); + + mockFetch?.pause?.(); + runCrossReportMergeToSourceReportRequest({ + ...fixtures, + targetTransactionThread, + }); + + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + const sourceReportActions = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${sourceExpenseReport.reportID}`, + callback: (actions) => { + Onyx.disconnect(connection); + resolve(actions ?? null); + }, + }); + }); + const newIOUAction = Object.values(sourceReportActions ?? {}).find((action) => { const reportAction = action as OnyxEntry; const originalMessage = getOriginalMessage(reportAction) as OriginalMessageIOU | undefined; @@ -895,9 +958,7 @@ describe('mergeTransactionRequest', () => { ); }) as OnyxEntry; - // Verify the new IOU action is created and points to the merged transaction - expect(newIOUAction).toBeTruthy(); - expect((getOriginalMessage(newIOUAction) as OriginalMessageIOU | undefined)?.IOUTransactionID).toBe(targetTransaction.transactionID); + expect(newIOUAction?.childReportID).toBe(targetTransactionThreadID); const updatedTargetTransactionThread = await new Promise((resolve) => { const connection = Onyx.connect({ @@ -909,11 +970,97 @@ describe('mergeTransactionRequest', () => { }); }); - // Verify target transaction thread parent references are rewired to the new IOU action in the source report expect(updatedTargetTransactionThread?.parentReportID).toBe(sourceExpenseReport.reportID); expect(updatedTargetTransactionThread?.parentReportActionID).toBe(newIOUAction?.reportActionID); }); + it('should attach optimistic cross-report merge data to self DM when destination reportID is unreported', async () => { + const fixtures = await setupCrossReportMergeToSourceReportFixtures(); + const {mergeTransactionID, mergeTransaction, targetTransaction, sourceTransaction, mockViolations, targetTransactionThreadID, targetTransactionThread} = fixtures; + const selfDMReport = { + ...createRandomReport(5), + reportID: 'self-dm-report-123', + type: CONST.REPORT.TYPE.CHAT, + }; + const unreportedSourceTransaction = { + ...sourceTransaction, + reportID: CONST.REPORT.UNREPORTED_REPORT_ID, + }; + const unreportedMergeTransaction = { + ...mergeTransaction, + reportID: CONST.REPORT.UNREPORTED_REPORT_ID, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${unreportedSourceTransaction.transactionID}`, unreportedSourceTransaction); + await Onyx.set(`${ONYXKEYS.COLLECTION.MERGE_TRANSACTION}${mergeTransactionID}`, unreportedMergeTransaction); + await waitForBatchedUpdates(); + + mockFetch?.pause?.(); + mergeTransactionRequest({ + iouReportOwnerLogin: undefined, + mergeTransactionID, + mergeTransaction: unreportedMergeTransaction, + targetTransaction, + sourceTransaction: unreportedSourceTransaction, + policy: undefined, + policyTags: undefined, + policyCategories: undefined, + allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, unreportedSourceTransaction.transactionID, mockViolations, mockViolations), + targetTransactionThreadReport: targetTransactionThread, + targetTransactionThreadParentReport: undefined, + reportPolicyTags: undefined, + targetTransactionThreadParentReportNextStep: undefined, + currentUserAccountIDParam: 123, + currentUserEmailParam: 'existing@example.com', + isASAPSubmitBetaEnabled: false, + selfDMReport, + selfDMReportActions: undefined, + delegateAccountID: undefined, + isTrackIntentUser: false, + }); + + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + const selfDMReportActions = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, + callback: (actions) => { + Onyx.disconnect(connection); + resolve(actions ?? null); + }, + }); + }); + const unreportedReportActions = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${CONST.REPORT.UNREPORTED_REPORT_ID}`, + callback: (actions) => { + Onyx.disconnect(connection); + resolve(actions ?? null); + }, + }); + }); + + const newIOUAction = Object.values(selfDMReportActions ?? {}).find((action) => isIOUActionForTransaction(action, targetTransaction.transactionID)); + + expect(newIOUAction?.childReportID).toBe(targetTransactionThreadID); + expect(unreportedReportActions).toBeNull(); + + const updatedTargetTransactionThread = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, + callback: (report) => { + Onyx.disconnect(connection); + resolve(report ?? null); + }, + }); + }); + + expect(updatedTargetTransactionThread?.parentReportID).toBe(selfDMReport.reportID); + expect(updatedTargetTransactionThread?.parentReportActionID).toBe(newIOUAction?.reportActionID); + }); + it('should not inject the target IOU action into the source report when a cross-report merge fails', async () => { const fixtures = await setupCrossReportMergeToSourceReportFixtures(); const {sourceExpenseReport, targetIOUActionID, targetTransactionThreadID, targetTransactionThread} = fixtures; diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 69cc803ce3fd..f95c8569fb61 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -8394,6 +8394,39 @@ describe('actions/Report', () => { TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.OPEN_REPORT, 1); }); + + it('should mark optimistic transaction thread actions as loaded when requested', async () => { + const parentReport: OnyxTypes.Report = { + ...createRandomReport(600, undefined), + reportID: '600', + type: CONST.REPORT.TYPE.EXPENSE, + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${parentReport.reportID}`, parentReport); + await waitForBatchedUpdates(); + + const reportAction: OnyxTypes.ReportAction = { + ...createRandomReportAction(6), + reportActionID: 'action-6', + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + }; + + const result = Report.createTransactionThreadReport({ + introSelected: TEST_INTRO_SELECTED, + currentUserLogin: TEST_USER_LOGIN, + currentUserAccountID: TEST_USER_ACCOUNT_ID, + betas: undefined, + iouReport: parentReport, + iouReportAction: reportAction, + hasOptimisticReportActions: true, + }); + await waitForBatchedUpdates(); + + const loadingState = await getOnyxValue(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${result?.reportID}`); + + expect(loadingState?.hasOnceLoadedReportActions).toBe(true); + expect(loadingState?.isLoadingInitialReportActions).toBe(false); + }); }); describe('resolveActionableMentionWhisper', () => { From 3bb31b35a9a641f42ec959161345f40fb5b59699 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Tue, 21 Jul 2026 16:43:00 +0430 Subject: [PATCH 02/27] Fix test spelling --- tests/actions/MergeTransactionTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index e813584b6310..7407d9581452 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -917,7 +917,7 @@ describe('mergeTransactionRequest', () => { expect(updatedTargetTransactionThread?.parentReportActionID).toBe(newIOUAction?.reportActionID); }); - it('should reparent a passed transaction thread when the original IOU action has no child report ID', async () => { + it('should move a passed transaction thread when the original IOU action has no child report ID', async () => { const fixtures = await setupCrossReportMergeToSourceReportFixtures(); const {targetTransaction, sourceExpenseReport, targetReport, targetIOUActionID, targetIOUAction, targetTransactionThreadID, targetTransactionThread} = fixtures; From 1009c56d8431ed8f242e0a5864b73fb5e210d760 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Wed, 5 Aug 2026 18:24:39 +0430 Subject: [PATCH 03/27] Fix merge test params --- tests/actions/MergeTransactionTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index 8c5de4c25fbf..b5531d735a92 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -1004,7 +1004,6 @@ describe('mergeTransactionRequest', () => { targetTransactionThreadReport: targetTransactionThread, targetTransactionThreadParentReport: undefined, reportPolicyTags: undefined, - targetTransactionThreadParentReportNextStep: undefined, currentUserAccountIDParam: 123, currentUserEmailParam: 'existing@example.com', isASAPSubmitBetaEnabled: false, From d7895ca39a4901f3f119faf0aa90ebc2e286247b Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Wed, 5 Aug 2026 18:29:41 +0430 Subject: [PATCH 04/27] Fix merge thread action lookup --- src/pages/TransactionMerge/DynamicConfirmationPage.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/TransactionMerge/DynamicConfirmationPage.tsx b/src/pages/TransactionMerge/DynamicConfirmationPage.tsx index 0a990ff66a6d..304665c20fab 100644 --- a/src/pages/TransactionMerge/DynamicConfirmationPage.tsx +++ b/src/pages/TransactionMerge/DynamicConfirmationPage.tsx @@ -20,7 +20,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import {mergeTransactionRequest} from '@libs/actions/MergeTransaction'; import {createTransactionThreadReport} from '@libs/actions/Report'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; -import {buildMergedTransactionData, getTransactionThreadReportID} from '@libs/MergeTransactionUtils'; +import {buildMergedTransactionData, getReportIDForExpense, getTransactionThreadReportID} from '@libs/MergeTransactionUtils'; import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute'; import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; @@ -97,7 +97,7 @@ function DynamicConfirmationPage({route}: DynamicConfirmationPageProps) { let targetTransactionThreadParentReportForMerge = targetTransactionThreadParentReport; if (!targetTransactionThreadReportIDForMerge) { - const targetIOUAction = getIOUActionForReportID(targetTransaction.reportID, targetTransaction.transactionID); + const targetIOUAction = getIOUActionForReportID(getReportIDForExpense(targetTransaction), targetTransaction.transactionID); const createdTransactionThreadReport = createTransactionThreadReport({ introSelected, currentUserLogin: currentUserEmailParam, From 7abd263805aacd7210b48798b8e43b506c1621f8 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Wed, 5 Aug 2026 18:34:51 +0430 Subject: [PATCH 05/27] Fix self DM merge action type --- src/libs/actions/MergeTransaction.ts | 3 ++- tests/actions/MergeTransactionTest.ts | 9 ++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index 68f499a15662..2802922f0d18 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -664,8 +664,9 @@ function mergeTransactionRequest({ // Move the surviving expense (targetTransaction) onto the destination report. For unreported // expenses, the API uses reportID "0", but optimistic UI must attach to the real self-DM report. if (mergeTransaction.reportID !== targetTransaction.reportID) { + const newIOUActionType = mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID ? CONST.IOU.REPORT_ACTION_TYPE.TRACK : CONST.IOU.REPORT_ACTION_TYPE.CREATE; const newIOUAction = buildOptimisticIOUReportAction({ - type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, + type: newIOUActionType, amount: mergeTransaction.amount, currency: mergeTransaction.currency, comment: mergeTransaction.description, diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index b5531d735a92..8c02af0ca4cf 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -66,13 +66,13 @@ function createAllTransactionViolations( return allViolations; } -function isIOUActionForTransaction(reportAction: OnyxEntry, transactionID: string): boolean { +function isIOUActionForTransaction(reportAction: OnyxEntry, transactionID: string, type: OriginalMessageIOU['type'] = CONST.IOU.REPORT_ACTION_TYPE.CREATE): boolean { if (!isMoneyRequestAction(reportAction)) { return false; } const originalMessage = getOriginalMessage(reportAction); - return reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && originalMessage?.type === CONST.IOU.REPORT_ACTION_TYPE.CREATE && originalMessage?.IOUTransactionID === transactionID; + return reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && originalMessage?.type === type && originalMessage?.IOUTransactionID === transactionID; } type CrossReportMergeToSourceReportFixtures = { @@ -1035,9 +1035,12 @@ describe('mergeTransactionRequest', () => { }); }); - const newIOUAction = Object.values(selfDMReportActions ?? {}).find((action) => isIOUActionForTransaction(action, targetTransaction.transactionID)); + const newIOUAction = Object.values(selfDMReportActions ?? {}).find((action) => + isIOUActionForTransaction(action, targetTransaction.transactionID, CONST.IOU.REPORT_ACTION_TYPE.TRACK), + ); expect(newIOUAction?.childReportID).toBe(targetTransactionThreadID); + expect(isIOUActionForTransaction(newIOUAction, targetTransaction.transactionID, CONST.IOU.REPORT_ACTION_TYPE.TRACK)).toBe(true); expect(unreportedReportActions).toBeNull(); const updatedTargetTransactionThread = await new Promise((resolve) => { From 430ffe5df26ee2774f6435adb778c8533a46f906 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Thu, 6 Aug 2026 17:25:15 +0430 Subject: [PATCH 06/27] Fix self DM merge optimistic duplicate --- src/libs/actions/MergeTransaction.ts | 46 +++++++++++++++------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index 2802922f0d18..9f5c0bead01c 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -703,29 +703,33 @@ function mergeTransactionRequest({ }); } - optimisticData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, - value: { - [newIOUAction.reportActionID]: newIOUAction, - }, - }); + // For unreported/Self-DM destinations, the server creates the TRACK action in Self DM. + // Writing it optimistically creates a temporary duplicate expense card until Onyx is cleared. + if (mergeTransaction.reportID !== CONST.REPORT.UNREPORTED_REPORT_ID) { + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, + value: { + [newIOUAction.reportActionID]: newIOUAction, + }, + }); - successData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, - value: { - [newIOUAction.reportActionID]: {pendingAction: null}, - }, - }); + successData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, + value: { + [newIOUAction.reportActionID]: {pendingAction: null}, + }, + }); - failureData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, - value: { - [newIOUAction.reportActionID]: null, - }, - }); + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, + value: { + [newIOUAction.reportActionID]: null, + }, + }); + } // Remove the surviving expense's IOU action from its original report so it does not // appear in both reports during offline/optimistic state. From e322f0912794c06527c3e350851304fcd0b77a43 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Sat, 8 Aug 2026 10:39:29 +0430 Subject: [PATCH 07/27] Fix duplicate expense in self DM merge --- src/libs/actions/MergeTransaction.ts | 56 +++++++++++++++------------ tests/actions/MergeTransactionTest.ts | 23 +++++++++-- 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index 4b88a6b7deca..1ed8c12e1364 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -722,33 +722,39 @@ function mergeTransactionRequest({ }); } - // For unreported/Self-DM destinations, the server creates the TRACK action in Self DM. - // Writing it optimistically creates a temporary duplicate expense card until Onyx is cleared. - if (mergeTransaction.reportID !== CONST.REPORT.UNREPORTED_REPORT_ID) { - optimisticData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, - value: { - [newIOUAction.reportActionID]: newIOUAction, - }, - }); + // The optimistic newIOUAction must always be written so the transaction thread has a parent + // action to load immediately, online or offline. For a Self-DM destination the server creates + // its own separate TRACK action for this transaction via the MergeTransaction response's + // onyxData (it does not reuse createdIOUReportActionID for that path). onyxData is applied + // before successData (see OnyxUpdates.applyHTTPSOnyxUpdates), so the server's real action is + // already in Onyx by the time successData runs — deleting the placeholder there instead of + // un-pending it avoids ending up with both visible as a duplicate expense card. + const isSelfDMDestination = mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; + const destinationReportActionsKey: `${typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS}${string}` = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`; + + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: destinationReportActionsKey, + value: { + [newIOUAction.reportActionID]: newIOUAction, + }, + }); - successData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, - value: { - [newIOUAction.reportActionID]: {pendingAction: null}, - }, - }); + successData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: destinationReportActionsKey, + value: { + [newIOUAction.reportActionID]: isSelfDMDestination ? null : {pendingAction: null}, + }, + }); - failureData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`, - value: { - [newIOUAction.reportActionID]: null, - }, - }); - } + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: destinationReportActionsKey, + value: { + [newIOUAction.reportActionID]: null, + }, + }); // Remove the surviving expense's IOU action from its original report so it does not // appear in both reports during offline/optimistic state. diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index 8c02af0ca4cf..81966e45ab92 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -1013,6 +1013,21 @@ describe('mergeTransactionRequest', () => { isTrackIntentUser: false, }); + await waitForBatchedUpdates(); + + const optimisticSelfDMReportActions = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, + callback: (actions) => { + Onyx.disconnect(connection); + resolve(actions ?? null); + }, + }); + }); + const placeholderReportActionID = Object.values(optimisticSelfDMReportActions ?? {}).find((action) => + isIOUActionForTransaction(action, targetTransaction.transactionID, CONST.IOU.REPORT_ACTION_TYPE.TRACK), + )?.reportActionID; + await mockFetch?.resume?.(); await waitForBatchedUpdates(); @@ -1039,8 +1054,10 @@ describe('mergeTransactionRequest', () => { isIOUActionForTransaction(action, targetTransaction.transactionID, CONST.IOU.REPORT_ACTION_TYPE.TRACK), ); - expect(newIOUAction?.childReportID).toBe(targetTransactionThreadID); - expect(isIOUActionForTransaction(newIOUAction, targetTransaction.transactionID, CONST.IOU.REPORT_ACTION_TYPE.TRACK)).toBe(true); + // The server creates its own TRACK action for the Self-DM destination via the MergeTransaction + // response's onyxData; this mock has none, so on success the client deletes its optimistic + // placeholder rather than leaving it around as a duplicate of the (unmocked) server action. + expect(newIOUAction).toBeUndefined(); expect(unreportedReportActions).toBeNull(); const updatedTargetTransactionThread = await new Promise((resolve) => { @@ -1054,7 +1071,7 @@ describe('mergeTransactionRequest', () => { }); expect(updatedTargetTransactionThread?.parentReportID).toBe(selfDMReport.reportID); - expect(updatedTargetTransactionThread?.parentReportActionID).toBe(newIOUAction?.reportActionID); + expect(updatedTargetTransactionThread?.parentReportActionID).toBe(placeholderReportActionID); }); it('should not inject the target IOU action into the source report when a cross-report merge fails', async () => { From 50f3e084325e9b07974815780566896e704cddcd Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Tue, 11 Aug 2026 15:19:13 +0430 Subject: [PATCH 08/27] Pass getCurrencyDecimals in self DM merge test --- tests/actions/MergeTransactionTest.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index 597234d87d8e..c61a3d8b312f 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -1018,6 +1018,7 @@ describe('mergeTransactionRequest', () => { selfDMReportActions: undefined, delegateAccountID: undefined, isTrackIntentUser: false, + getCurrencyDecimals: getCurrencyDecimalsLocal, }); await waitForBatchedUpdates(); From 009f30d6dc5975ce8217cfe95b4cb9ffe743ab79 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Mon, 17 Aug 2026 16:43:29 +0430 Subject: [PATCH 09/27] Fix policy misalignment in search merge --- src/libs/MergeTransactionUtils.ts | 6 ++++-- src/libs/ReportSecondaryActionUtils.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libs/MergeTransactionUtils.ts b/src/libs/MergeTransactionUtils.ts index 146408b3ec74..638840f081f3 100644 --- a/src/libs/MergeTransactionUtils.ts +++ b/src/libs/MergeTransactionUtils.ts @@ -192,7 +192,7 @@ function getTransactionsAndReportsFromSearch( ): { transactions: Transaction[]; reports: Report[]; - policies: Policy[]; + policies: Array>; } { const transaction1 = searchResults.data[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDs.at(0)}`]; const transaction2 = searchResults.data[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDs.at(1)}`]; @@ -204,7 +204,9 @@ function getTransactionsAndReportsFromSearch( reports: [searchResults.data[`${ONYXKEYS.COLLECTION.REPORT}${transaction1?.reportID}`], searchResults.data[`${ONYXKEYS.COLLECTION.REPORT}${transaction2?.reportID}`]].filter( (report) => !!report, ), - policies: [searchResults.data[`${ONYXKEYS.COLLECTION.POLICY}${policyID1}`], searchResults.data[`${ONYXKEYS.COLLECTION.POLICY}${policyID2}`]].filter((policy) => !!policy), + // Not filtered, unlike the arrays above: this is read by index alongside `transactions`, and a self-DM + // transaction has no policyID, so dropping its undefined slot would pair a policy with the wrong transaction. + policies: [searchResults.data[`${ONYXKEYS.COLLECTION.POLICY}${policyID1}`], searchResults.data[`${ONYXKEYS.COLLECTION.POLICY}${policyID2}`]], }; } diff --git a/src/libs/ReportSecondaryActionUtils.ts b/src/libs/ReportSecondaryActionUtils.ts index 3347a1a72688..389501095085 100644 --- a/src/libs/ReportSecondaryActionUtils.ts +++ b/src/libs/ReportSecondaryActionUtils.ts @@ -782,7 +782,7 @@ function isMergeAction(parentReport: Report, reportTransactions: Transaction[], return isMoneyRequestReportEligibleForMerge(parentReport.reportID, isAdmin); } -function isMergeActionForSelectedTransactions(transactions: Transaction[], reports: Report[], policies: Policy[], currentUserAccountID?: number) { +function isMergeActionForSelectedTransactions(transactions: Transaction[], reports: Report[], policies: Array>, currentUserAccountID?: number) { if ([transactions, reports, policies].some((collection) => collection?.length > 2)) { return false; } From 9887fbccea539e1b9673f2a38176a2762d9bdac7 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Mon, 17 Aug 2026 16:57:40 +0430 Subject: [PATCH 10/27] Avoid optimistic writes to report "0" --- src/libs/actions/MergeTransaction.ts | 47 +++++++++++++++------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index c328bf8124e3..a5e8a4ee5225 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -499,6 +499,7 @@ function mergeTransactionRequest({ // Optimistic delete the source transaction and also delete its report if it was a single expense report const isUnreportedSourceTransaction = sourceTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; const selfDMReportID = selfDMReport?.reportID; + const isSelfDMDestinationWithoutReport = mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID && !selfDMReportID; const optimisticDestinationReportID = mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID ? (selfDMReportID ?? mergeTransaction.reportID) : mergeTransaction.reportID; const sourceTransactionOptimisticData: Array< @@ -719,7 +720,7 @@ function mergeTransactionRequest({ const targetIOUActionOnOriginalReport = getIOUActionForReportID(targetTransaction.reportID, targetTransaction.transactionID); const targetTransactionThreadReportID = targetIOUActionOnOriginalReport?.childReportID ?? targetTransactionThreadReport?.reportID; - if (targetTransactionThreadReportID) { + if (targetTransactionThreadReportID && !isSelfDMDestinationWithoutReport) { // Reattach the transaction thread to the new IOU action on the destination report. newIOUAction.childReportID = targetTransactionThreadReportID; optimisticData.push({ @@ -751,29 +752,31 @@ function mergeTransactionRequest({ const isSelfDMDestination = mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; const destinationReportActionsKey: `${typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS}${string}` = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`; - optimisticData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: destinationReportActionsKey, - value: { - [newIOUAction.reportActionID]: newIOUAction, - }, - }); + if (!isSelfDMDestinationWithoutReport) { + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: destinationReportActionsKey, + value: { + [newIOUAction.reportActionID]: newIOUAction, + }, + }); - successData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: destinationReportActionsKey, - value: { - [newIOUAction.reportActionID]: isSelfDMDestination ? null : {pendingAction: null}, - }, - }); + successData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: destinationReportActionsKey, + value: { + [newIOUAction.reportActionID]: isSelfDMDestination ? null : {pendingAction: null}, + }, + }); - failureData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: destinationReportActionsKey, - value: { - [newIOUAction.reportActionID]: null, - }, - }); + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: destinationReportActionsKey, + value: { + [newIOUAction.reportActionID]: null, + }, + }); + } // Remove the surviving expense's IOU action from its original report so it does not // appear in both reports during offline/optimistic state. From 06412bf5b2dd038a6ef93901299a71c2cbdf01eb Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Mon, 17 Aug 2026 17:25:51 +0430 Subject: [PATCH 11/27] Replace unsafe cast with existing helper --- tests/actions/MergeTransactionTest.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index 9d27f837af6c..ab41f198c3d1 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -954,15 +954,7 @@ describe('mergeTransactionRequest', () => { }); }); - const newIOUAction = Object.values(sourceReportActions ?? {}).find((action) => { - const reportAction = action as OnyxEntry; - const originalMessage = getOriginalMessage(reportAction) as OriginalMessageIOU | undefined; - return ( - reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && - originalMessage?.type === CONST.IOU.REPORT_ACTION_TYPE.CREATE && - originalMessage?.IOUTransactionID === targetTransaction.transactionID - ); - }) as OnyxEntry; + const newIOUAction = Object.values(sourceReportActions ?? {}).find((action) => isIOUActionForTransaction(action, targetTransaction.transactionID)); expect(newIOUAction?.childReportID).toBe(targetTransactionThreadID); From c110fd866773ba7c8f12922eff860d8677488607 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Mon, 17 Aug 2026 18:26:08 +0430 Subject: [PATCH 12/27] Trim self-DM merge comment --- src/libs/actions/MergeTransaction.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index a5e8a4ee5225..47a072a6f7e9 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -742,13 +742,12 @@ function mergeTransactionRequest({ }); } - // The optimistic newIOUAction must always be written so the transaction thread has a parent - // action to load immediately, online or offline. For a Self-DM destination the server creates - // its own separate TRACK action for this transaction via the MergeTransaction response's - // onyxData (it does not reuse createdIOUReportActionID for that path). onyxData is applied - // before successData (see OnyxUpdates.applyHTTPSOnyxUpdates), so the server's real action is - // already in Onyx by the time successData runs — deleting the placeholder there instead of - // un-pending it avoids ending up with both visible as a duplicate expense card. + // Always write the optimistic newIOUAction so the transaction thread has a parent action to + // render immediately, online or offline. For a Self-DM destination the server creates its own + // TRACK action instead of reusing createdIOUReportActionID, so successData deletes our + // placeholder rather than clearing its pendingAction, which would leave a duplicate expense + // card. The server's onyxData is applied first and also updates the thread's parent pointers. + // Verified against a live merge response: https://github.com/Expensify/App/pull/96633 const isSelfDMDestination = mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; const destinationReportActionsKey: `${typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS}${string}` = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`; From c88dda1a876b1a86d5b085821817904fc1dd815e Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Tue, 18 Aug 2026 09:43:12 +0430 Subject: [PATCH 13/27] Pass getCurrencySymbol in self DM merge test --- tests/actions/MergeTransactionTest.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index 0015b086386c..5ecddc09af0e 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -1024,6 +1024,7 @@ describe('mergeTransactionRequest', () => { sourceTransactionThreadReportActions: undefined, sourceIOUAction: undefined, getCurrencyDecimals: getCurrencyDecimalsLocal, + getCurrencySymbol: getCurrencySymbolLocal, }); await waitForBatchedUpdates(); From dc74a7e7ec9262df0b9fd8a07101f168fcf57d40 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Tue, 18 Aug 2026 10:02:03 +0430 Subject: [PATCH 14/27] Trim self-DM merge comment --- src/libs/actions/MergeTransaction.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index eb1405506beb..83c69b879067 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -753,12 +753,10 @@ function mergeTransactionRequest({ }); } - // Always write the optimistic newIOUAction so the transaction thread has a parent action to - // render immediately, online or offline. For a Self-DM destination the server creates its own - // TRACK action instead of reusing createdIOUReportActionID, so successData deletes our - // placeholder rather than clearing its pendingAction, which would leave a duplicate expense - // card. The server's onyxData is applied first and also updates the thread's parent pointers. - // Verified against a live merge response: https://github.com/Expensify/App/pull/96633 + // For a Self-DM destination the server creates its own TRACK action instead of reusing + // createdIOUReportActionID, so successData deletes our placeholder rather than clearing its + // pendingAction, which would leave a duplicate expense card. The server's onyxData is applied + // first and also updates the thread's parentReportID and parentReportActionID, so we do not set them here. const isSelfDMDestination = mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; const destinationReportActionsKey: `${typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS}${string}` = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticDestinationReportID}`; From f892c1549e29c649a28c2257071f66858ab204bb Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Fri, 21 Aug 2026 17:35:38 +0430 Subject: [PATCH 15/27] Fix duplicate self-DM card on offline merge --- src/libs/actions/MergeTransaction.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index 1e4b6b0ba098..cdbdb0d6e493 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -663,6 +663,20 @@ function mergeTransactionRequest({ sourceTransactionOptimisticData.push(...optimisticData); sourceTransactionSuccessData.push(...successData); sourceTransactionFailureData.push(...failureData); + + // With a Self-DM destination the surviving expense's optimistic action is written to this same report, + // so leaving the source's action in a pending-delete state renders two cards side by side while offline. + // Remove it outright instead, which is what the server's response does once it lands. The rollback is + // already covered by the failureData above, which restores the whole action. + if (mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID) { + sourceTransactionOptimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReportID}`, + value: { + [sourceIouAction.reportActionID]: null, + }, + }); + } } } // Optimistic delete the merge transaction From 3d50a0b0f653dc34bc02bb2c666051a73528b72a Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Thu, 27 Aug 2026 11:15:59 +0430 Subject: [PATCH 16/27] Fix negative amount on offline merge to self DM --- src/libs/TransactionUtils/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 984891ffa3dd..2cb700ee7540 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -682,7 +682,9 @@ function getUpdatedTransaction({ getCurrencyDecimals: CurrencyListActionsContextType['getCurrencyDecimals']; getCurrencySymbol: CurrencyListActionsContextType['getCurrencySymbol']; }): Transaction { - const isUnReportedExpense = transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; + // Tracked expenses store the opposite sign, so this has to follow the report the expense ends up on rather than + // the one it is on now: a merge moves the expense to Self-DM in the same change set. + const isUnReportedExpense = transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID || transactionChanges.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; // Only changing the first level fields so no need for deep clone now const updatedTransaction = lodashDeepClone(transaction); From 4102aeebcc0b11178b1462ca9e9083e28f614455 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Thu, 27 Aug 2026 11:28:17 +0430 Subject: [PATCH 17/27] Keep self-DM source action deleted on success --- src/libs/actions/MergeTransaction.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index 92e67c3cf42d..8e61332ea87d 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -676,6 +676,17 @@ function mergeTransactionRequest({ [sourceIouAction.reportActionID]: null, }, }); + + // Keep it deleted on success too. The successData above clears pendingAction and errors on the same + // action, and Onyx strips nested nulls, so on a key we just removed that would leave an empty entry + // behind unless the response happens to send the action back first. + sourceTransactionSuccessData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReportID}`, + value: { + [sourceIouAction.reportActionID]: null, + }, + }); } } } From 88109ed864095c1a4841ae8dd8332c71485bd696 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Tue, 1 Sep 2026 12:12:49 +0430 Subject: [PATCH 18/27] Resolve target IOU action from snapshot --- src/pages/TransactionMerge/DynamicConfirmationPage.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/pages/TransactionMerge/DynamicConfirmationPage.tsx b/src/pages/TransactionMerge/DynamicConfirmationPage.tsx index 470160f74d7b..7d6e312027ea 100644 --- a/src/pages/TransactionMerge/DynamicConfirmationPage.tsx +++ b/src/pages/TransactionMerge/DynamicConfirmationPage.tsx @@ -123,7 +123,14 @@ function DynamicConfirmationPage({route}: DynamicConfirmationPageProps) { let targetTransactionThreadParentReportForMerge = targetTransactionThreadParentReport; if (!targetTransactionThreadReportIDForMerge) { - const targetIOUAction = getIOUActionForReportID(getReportIDForExpense(targetTransaction), targetTransaction.transactionID); + // Merges started from Search can have the target's report actions only in the snapshot, so fall back to it. + // Without the existing IOU action, createTransactionThreadReport takes openReport's legacy path and builds a + // second optimistic action for an expense that already has one. + const targetExpenseReportID = getReportIDForExpense(targetTransaction); + const targetSnapshotReportActions = currentSearchResults?.data?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getNonEmptyStringOnyxID(targetExpenseReportID)}`]; + const targetIOUAction = + getIOUActionForReportID(targetExpenseReportID, targetTransaction.transactionID) ?? + getIOUActionForTransactionID(Object.values(targetSnapshotReportActions ?? {}), targetTransaction.transactionID); const createdTransactionThreadReport = createTransactionThreadReport({ introSelected, currentUserLogin: currentUserEmailParam, From c226404dea68e3c27501b7e4661486f01b1b475d Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Fri, 11 Sep 2026 14:58:10 +0430 Subject: [PATCH 19/27] Pass rules param in merge test --- tests/actions/MergeTransactionTest.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index c88c588136ab..7242272b971b 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -1034,6 +1034,7 @@ describe('mergeTransactionRequest', () => { sourceIOUAction: undefined, getCurrencyDecimals: getCurrencyDecimalsLocal, getCurrencySymbol: getCurrencySymbolLocal, + rules: undefined, }); await waitForBatchedUpdates(); From 86964e40faf2beaab4a722554f17022a6cd8a944 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Mon, 21 Sep 2026 10:01:04 +0430 Subject: [PATCH 20/27] Skip loading skeleton offline when actions cached --- src/libs/TransactionUtils/index.ts | 4 +- src/libs/actions/Report/index.ts | 4 +- tests/actions/ReportTest.ts | 75 ++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index f90a910ae775..d5f5dae895e7 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -811,7 +811,7 @@ function getRecalculatedDistanceMerchant( } /** - * Given the edit made to the expense, return an updated transaction object. + * Given the edit made to the expense, return an updated transaction object. Tracked expenses store the opposite sign, so the amount follows the report the expense ends up on: a merge moves it to Self-DM in the same change set. */ function getUpdatedTransaction({ transaction, @@ -836,8 +836,6 @@ function getUpdatedTransaction({ getCurrencyDecimals: CurrencyListActionsContextType['getCurrencyDecimals']; getCurrencySymbol: CurrencyListActionsContextType['getCurrencySymbol']; }): Transaction { - // Tracked expenses store the opposite sign, so this has to follow the report the expense ends up on rather than - // the one it is on now: a merge moves the expense to Self-DM in the same change set. const isUnReportedExpense = transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID || transactionChanges.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; // Only changing the first level fields so no need for deep clone now diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 47d31154f7bf..acded5fd83cc 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -1795,7 +1795,9 @@ function openReport(params: OpenReportActionParams) { key: `${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportID}`, value: { hasOnceLoadedReportActions: hasOptimisticReportActions ? true : undefined, - isLoadingInitialReportActions: !hasOptimisticReportActions, + // Offline this only ever clears from the OpenReport response, which never arrives, so setting it for a + // report whose actions are already in Onyx pins the skeleton over content we could render right away. + isLoadingInitialReportActions: !hasOptimisticReportActions && !(isOfflineNetwork() && hasReportActions), isLoadingOlderReportActions: false, hasLoadingOlderReportActionsError: false, isLoadingNewerReportActions: false, diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 3b25fd0f1d14..51fe2af3c422 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -10044,6 +10044,81 @@ describe('actions/Report', () => { expect(loadingState?.isLoadingInitialReportActions).toBe(false); }); + it('should not pin an already-loaded report on the loading skeleton while offline', async () => { + // Given a transaction thread that already exists with its actions in Onyx. Merging into an expense that + // already has a thread skips createTransactionThreadReport, so nothing marks the thread as loaded. + const existingThread: OnyxTypes.Report = { + ...createRandomReport(900, undefined), + reportID: '900', + type: CONST.REPORT.TYPE.CHAT, + }; + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${existingThread.reportID}`, existingThread); + await waitForBatchedUpdates(); + + // When the report screen opens it while offline, so the OpenReport response never arrives + const mockFetchOffline = TestHelper.createGlobalFetchMock(); + global.fetch = mockFetchOffline; + mockFetchOffline.pause(); + setHasRadio(false); + + Report.openReport({ + reportID: existingThread.reportID, + introSelected: TEST_INTRO_SELECTED, + conciergeChat: undefined, + currentUserLogin: TEST_USER_LOGIN, + currentUserAccountID: TEST_USER_ACCOUNT_ID, + betas: undefined, + personalDetails: undefined, + hasReportActions: true, + }); + await waitForBatchedUpdates(); + + // Then it is not left loading, because isLoadingInitialReportActions only clears from that response + const loadingStateOffline = await getOnyxValue(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${existingThread.reportID}`); + + expect(loadingStateOffline?.isLoadingInitialReportActions).toBe(false); + + setHasRadio(true); + await mockFetchOffline.resume(); + }); + + it('should still show the loading state offline for a report with no actions yet', async () => { + // Given a report whose actions are not in Onyx, so there is nothing to render in place of the skeleton + const emptyReport: OnyxTypes.Report = { + ...createRandomReport(902, undefined), + reportID: '902', + type: CONST.REPORT.TYPE.CHAT, + }; + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${emptyReport.reportID}`, emptyReport); + await waitForBatchedUpdates(); + + // When the report screen opens it while offline + const mockFetchOffline = TestHelper.createGlobalFetchMock(); + global.fetch = mockFetchOffline; + mockFetchOffline.pause(); + setHasRadio(false); + + Report.openReport({ + reportID: emptyReport.reportID, + introSelected: TEST_INTRO_SELECTED, + conciergeChat: undefined, + currentUserLogin: TEST_USER_LOGIN, + currentUserAccountID: TEST_USER_ACCOUNT_ID, + betas: undefined, + personalDetails: undefined, + hasReportActions: false, + }); + await waitForBatchedUpdates(); + + // Then the loading state is kept, since showing an empty report would be worse than the skeleton + const loadingStateOffline = await getOnyxValue(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${emptyReport.reportID}`); + + expect(loadingStateOffline?.isLoadingInitialReportActions).toBe(true); + + setHasRadio(true); + await mockFetchOffline.resume(); + }); + it('threads the conciergeChat report into the guided setup data sent with OpenReport', async () => { await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, {hasCompletedGuidedSetupFlow: false}); const conciergeChat: OnyxTypes.Report = {...createRandomReport(777, undefined), reportID: 'concierge-thread-1'}; From d121c9003e3e4479605c29741a2d7a681bbd4596 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Mon, 21 Sep 2026 12:15:27 +0430 Subject: [PATCH 21/27] Show source card as pending delete offline --- src/libs/actions/MergeTransaction.ts | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index e268900970c9..2785baec34c4 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -683,22 +683,10 @@ function mergeTransactionRequest({ sourceTransactionSuccessData.push(...successData); sourceTransactionFailureData.push(...failureData); - // With a Self-DM destination the surviving expense's optimistic action is written to this same report, - // so leaving the source's action in a pending-delete state renders two cards side by side while offline. - // Remove it outright instead, which is what the server's response does once it lands. The rollback is - // already covered by the failureData above, which restores the whole action. + // With a Self-DM destination the surviving expense's optimistic action is written to this same report, so + // the source keeps its pending-delete styling while offline and is removed once the merge succeeds. The + // successData above only clears pendingAction and errors, which would leave the card behind, so drop it. if (mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID) { - sourceTransactionOptimisticData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReportID}`, - value: { - [sourceIouAction.reportActionID]: null, - }, - }); - - // Keep it deleted on success too. The successData above clears pendingAction and errors on the same - // action, and Onyx strips nested nulls, so on a key we just removed that would leave an empty entry - // behind unless the response happens to send the action back first. sourceTransactionSuccessData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReportID}`, From 98a6b29df574df5ec117b1fa9ce3813590b8c8ee Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Mon, 19 Oct 2026 15:12:11 +0430 Subject: [PATCH 22/27] fix: offline empty actions skeleton --- .../MoneyRequestReportView.tsx | 4 +++- tests/ui/MoneyRequestReportViewTest.tsx | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx index ebef8dc41714..bb2ea35cb156 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx @@ -195,7 +195,9 @@ function MoneyRequestReportView({report, reportIDFromRoute, reportLoadingState, }; }, [reportID]); - const shouldShowEmptyActionsSkeleton = reportActions.length === 0; + // Offline there is no OpenReport response coming to fill the actions in, so this skeleton could only ever be + // permanent. Every other skeleton on this screen is already gated the same way. + const shouldShowEmptyActionsSkeleton = reportActions.length === 0 && !isOffline; const shouldShowAppLoadSkeleton = !!report && isAppLoadPending; // These skeletons render before the report lands in Onyx, so the mark uses the route id. useMarkOpenReportEndOnSkeleton(reportIDFromRoute, shouldShowOpenReportLoadingSkeleton || shouldShowEmptyActionsSkeleton || shouldShowAppLoadSkeleton); diff --git a/tests/ui/MoneyRequestReportViewTest.tsx b/tests/ui/MoneyRequestReportViewTest.tsx index 4b8a39aa05f4..8c31e2bfa342 100644 --- a/tests/ui/MoneyRequestReportViewTest.tsx +++ b/tests/ui/MoneyRequestReportViewTest.tsx @@ -212,6 +212,21 @@ describe('MoneyRequestReportView', () => { expect(mockMoneyRequestReportActionsList).not.toHaveBeenCalled(); }); + it('renders the report instead of a permanent cover when report actions are empty while offline', () => { + // Given a report whose actions are not cached, opened offline. This is the state a merge lands in when the + // target expense already has a transaction thread that was never loaded in this session. + jest.spyOn(ReportActionsUtils, 'getFilteredReportActionsForReportView').mockReturnValue([]); + mockUseNetwork.mockReturnValue({isOffline: true}); + + // When the view renders + renderMoneyRequestReportView(jest.fn()); + + // Then no skeleton is shown, because the OpenReport response that would fill the actions in never arrives + // offline, so the cover would stay up forever + expect(screen.queryByTestId('ReportActionsSkeletonCover')).toBeNull(); + expect(mockReportActionsListBody).toHaveBeenCalled(); + }); + it('uses the bottom-padded cover while the app is loading', () => { mockUseIsAppLoadPending.mockReturnValue(true); From 4466b0935a4a2f72a78cfe9d435daf7dc48b576d Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Wed, 21 Oct 2026 11:29:31 +0430 Subject: [PATCH 23/27] fix: reuse existing transaction thread --- .../DynamicConfirmationPage.tsx | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/pages/TransactionMerge/DynamicConfirmationPage.tsx b/src/pages/TransactionMerge/DynamicConfirmationPage.tsx index a3ce1dcf866c..8448f0d7162c 100644 --- a/src/pages/TransactionMerge/DynamicConfirmationPage.tsx +++ b/src/pages/TransactionMerge/DynamicConfirmationPage.tsx @@ -32,7 +32,7 @@ import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; import type {MergeTransactionNavigatorParamList} from '@libs/Navigation/types'; import {getFilteredReportActionsForReportView, getIOUActionForReportID, getIOUActionForTransactionID, isMoneyRequestAction} from '@libs/ReportActionsUtils'; -import {findSelfDMReportID} from '@libs/ReportUtils'; +import {findSelfDMReportID, getReportOrDraftReport} from '@libs/ReportUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -141,23 +141,37 @@ function DynamicConfirmationPage({route}: DynamicConfirmationPageProps) { const targetIOUAction = getIOUActionForReportID(targetExpenseReportID, targetTransaction.transactionID) ?? getIOUActionForTransactionID(Object.values(targetSnapshotReportActions ?? {}), targetTransaction.transactionID); - const createdTransactionThreadReport = createTransactionThreadReport({ - introSelected, - currentUserLogin: currentUserEmailParam, - currentUserAccountID: currentUserAccountIDParam, - iouReport: targetTransactionReport, - iouReportAction: targetIOUAction, - transaction: targetTransaction, - transactionViolations: allTransactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${targetTransaction.transactionID}`], - personalDetails, - conciergeChat, - hasOptimisticReportActions: true, - }); - if (createdTransactionThreadReport) { - targetTransactionThreadReportIDForMerge = createdTransactionThreadReport.reportID; - targetTransactionThreadReportForMerge = createdTransactionThreadReport; + // That action can already point at a thread: getTransactionThreadReportID reads childReportID through the + // main Onyx collection only, so a thread known just to the snapshot reads as missing here. Reuse it rather + // than calling createTransactionThreadReport, which always mints a fresh ID and would leave the expense + // with a second thread. + const existingTargetThreadReportID = targetIOUAction?.childReportID; + if (existingTargetThreadReportID) { + targetTransactionThreadReportIDForMerge = existingTargetThreadReportID; + targetTransactionThreadReportForMerge = getReportOrDraftReport(existingTargetThreadReportID, [ + currentSearchResults?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${existingTargetThreadReportID}`], + ]); targetTransactionThreadParentReportForMerge = targetTransactionReport; + } else { + const createdTransactionThreadReport = createTransactionThreadReport({ + introSelected, + currentUserLogin: currentUserEmailParam, + currentUserAccountID: currentUserAccountIDParam, + iouReport: targetTransactionReport, + iouReportAction: targetIOUAction, + transaction: targetTransaction, + transactionViolations: allTransactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${targetTransaction.transactionID}`], + personalDetails, + conciergeChat, + hasOptimisticReportActions: true, + }); + + if (createdTransactionThreadReport) { + targetTransactionThreadReportIDForMerge = createdTransactionThreadReport.reportID; + targetTransactionThreadReportForMerge = createdTransactionThreadReport; + targetTransactionThreadParentReportForMerge = targetTransactionReport; + } } } From 6c496c1848c179a22ea81e872d1bbe6d2db90f90 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Fri, 23 Oct 2026 14:59:23 +0430 Subject: [PATCH 24/27] revert: offline empty actions skeleton --- .../MoneyRequestReportView.tsx | 4 +--- tests/ui/MoneyRequestReportViewTest.tsx | 15 --------------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx index bb2ea35cb156..ebef8dc41714 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportView.tsx @@ -195,9 +195,7 @@ function MoneyRequestReportView({report, reportIDFromRoute, reportLoadingState, }; }, [reportID]); - // Offline there is no OpenReport response coming to fill the actions in, so this skeleton could only ever be - // permanent. Every other skeleton on this screen is already gated the same way. - const shouldShowEmptyActionsSkeleton = reportActions.length === 0 && !isOffline; + const shouldShowEmptyActionsSkeleton = reportActions.length === 0; const shouldShowAppLoadSkeleton = !!report && isAppLoadPending; // These skeletons render before the report lands in Onyx, so the mark uses the route id. useMarkOpenReportEndOnSkeleton(reportIDFromRoute, shouldShowOpenReportLoadingSkeleton || shouldShowEmptyActionsSkeleton || shouldShowAppLoadSkeleton); diff --git a/tests/ui/MoneyRequestReportViewTest.tsx b/tests/ui/MoneyRequestReportViewTest.tsx index 8c31e2bfa342..4b8a39aa05f4 100644 --- a/tests/ui/MoneyRequestReportViewTest.tsx +++ b/tests/ui/MoneyRequestReportViewTest.tsx @@ -212,21 +212,6 @@ describe('MoneyRequestReportView', () => { expect(mockMoneyRequestReportActionsList).not.toHaveBeenCalled(); }); - it('renders the report instead of a permanent cover when report actions are empty while offline', () => { - // Given a report whose actions are not cached, opened offline. This is the state a merge lands in when the - // target expense already has a transaction thread that was never loaded in this session. - jest.spyOn(ReportActionsUtils, 'getFilteredReportActionsForReportView').mockReturnValue([]); - mockUseNetwork.mockReturnValue({isOffline: true}); - - // When the view renders - renderMoneyRequestReportView(jest.fn()); - - // Then no skeleton is shown, because the OpenReport response that would fill the actions in never arrives - // offline, so the cover would stay up forever - expect(screen.queryByTestId('ReportActionsSkeletonCover')).toBeNull(); - expect(mockReportActionsListBody).toHaveBeenCalled(); - }); - it('uses the bottom-padded cover while the app is loading', () => { mockUseIsAppLoadPending.mockReturnValue(true); From 00241a53ee6f7025927dc324030e1fe49bcab18c Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Fri, 23 Oct 2026 16:19:17 +0430 Subject: [PATCH 25/27] fix: seed thread report missing from Onyx --- src/libs/actions/MergeTransaction.ts | 44 ++++++-- tests/actions/MergeTransactionTest.ts | 147 ++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 9 deletions(-) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index 217cbbb1576a..718e9efafc52 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -23,6 +23,7 @@ import {isPaidGroupPolicy, isPolicyAdmin, resolveCurrentTaxCode} from '@libs/Pol import {getIOUActionForReportID, getReportAction, getTrackExpenseActionableWhisper} from '@libs/ReportActionsUtils'; import { buildOptimisticIOUReportAction, + buildTransactionThread, getReportOrDraftReport, getTransactionDetails, isCurrentUserSubmitter, @@ -798,22 +799,47 @@ function mergeTransactionRequest({ if (targetTransactionThreadReportID && !isSelfDMDestinationWithoutReport) { // Reattach the transaction thread to the new IOU action on the destination report. newIOUAction.childReportID = targetTransactionThreadReportID; + + // The thread can be one that was never opened, so Onyx has no report for it even though we hold a copy + // from the Search snapshot. Merging only the parent fields onto that key would leave a report with no + // reportID, and we navigate straight to it after the merge, so the screen would show a skeleton that + // offline never clears. Seed the whole report in that case instead. + const threadReportInOnyx = getReportOrDraftReport(targetTransactionThreadReportID); + const reattachedThreadFields = { + parentReportID: optimisticDestinationReportID, + parentReportActionID: newIOUAction.reportActionID, + }; + optimisticData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`, - value: { - parentReportID: optimisticDestinationReportID, - parentReportActionID: newIOUAction.reportActionID, - }, + value: threadReportInOnyx + ? reattachedThreadFields + : { + ...buildTransactionThread( + newIOUAction, + getReportOrDraftReport(optimisticDestinationReportID) ?? selfDMReport, + currentUserAccountIDParam, + undefined, + targetTransactionThreadReportID, + ), + ...targetTransactionThreadReport, + reportID: targetTransactionThreadReportID, + ...reattachedThreadFields, + }, }); failureData.push({ - onyxMethod: Onyx.METHOD.MERGE, + // Onyx held nothing before the merge in the seeded case, so a failure drops the report rather than + // restoring fields onto one that should not exist. + onyxMethod: threadReportInOnyx ? Onyx.METHOD.MERGE : Onyx.METHOD.SET, key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`, - value: { - parentReportID: targetTransactionThreadReport?.parentReportID ?? targetTransaction.reportID, - parentReportActionID: targetTransactionThreadReport?.parentReportActionID ?? targetIOUActionOnOriginalReport?.reportActionID, - }, + value: threadReportInOnyx + ? { + parentReportID: threadReportInOnyx.parentReportID ?? targetTransaction.reportID, + parentReportActionID: threadReportInOnyx.parentReportActionID ?? targetIOUActionOnOriginalReport?.reportActionID, + } + : null, }); } diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index 6bd1ac95c79f..67b187eb353a 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -1139,6 +1139,153 @@ describe('mergeTransactionRequest', () => { expect(updatedTargetTransactionThread?.parentReportActionID).toBe(placeholderReportActionID); }); + it('should seed the whole transaction thread when Onyx has no report for it', async () => { + // Given a thread the confirmation page resolved from the Search snapshot, so a report object is passed in + // while Onyx itself holds nothing under that ID. This is the state a thread that was never opened lands in. + const fixtures = await setupCrossReportMergeToSourceReportFixtures(); + const {mergeTransactionID, mergeTransaction, targetTransaction, sourceTransaction, mockViolations, targetTransactionThreadID, targetTransactionThread} = fixtures; + const selfDMReport = { + ...createRandomReport(5), + reportID: 'self-dm-report-123', + type: CONST.REPORT.TYPE.CHAT, + }; + const unreportedSourceTransaction = {...sourceTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; + const unreportedMergeTransaction = {...mergeTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; + + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, null); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${unreportedSourceTransaction.transactionID}`, unreportedSourceTransaction); + await Onyx.set(`${ONYXKEYS.COLLECTION.MERGE_TRANSACTION}${mergeTransactionID}`, unreportedMergeTransaction); + await waitForBatchedUpdates(); + + // When the merge moves the expense to another destination and re-points the thread + mockFetch?.pause?.(); + mergeTransactionRequest({ + isVendorMatchingBetaEnabled: false, + iouReportOwnerLogin: undefined, + mergeTransactionID, + mergeTransaction: unreportedMergeTransaction, + targetTransaction, + transactionsOfDeletableReportCollection: {[unreportedSourceTransaction.transactionID]: unreportedSourceTransaction}, + sourceTransaction: unreportedSourceTransaction, + policy: undefined, + policyTags: undefined, + policyCategories: undefined, + allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, unreportedSourceTransaction.transactionID, mockViolations, mockViolations), + sourceIOUActionThreadReport: undefined, + // Truthy, like the snapshot copy the page passes, even though Onyx has nothing under this ID + targetTransactionThreadReport: targetTransactionThread, + targetTransactionThreadParentReport: undefined, + reportPolicyTags: undefined, + currentUserAccountIDParam: 123, + currentUserEmailParam: 'existing@example.com', + isASAPSubmitBetaEnabled: false, + selfDMReport, + selfDMReportActions: undefined, + delegateAccountID: undefined, + isTrackIntentUser: false, + sourceTransactionThreadReportActions: undefined, + sourceIOUAction: undefined, + sourceActionIOUReport: undefined, + sourceActionChatReport: undefined, + getCurrencyDecimals: getCurrencyDecimalsLocal, + getCurrencySymbol: getCurrencySymbolLocal, + rules: undefined, + }); + await waitForBatchedUpdates(); + + // Then the report carries a reportID rather than only the two parent fields. The merge navigates straight to + // this report, and a report without a reportID reads as unloaded, so the screen sits on a loading skeleton + // that offline never clears. + const seededThread = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, + callback: (report) => { + Onyx.disconnect(connection); + resolve(report ?? null); + }, + }); + }); + + expect(seededThread?.reportID).toBe(targetTransactionThreadID); + expect(seededThread?.parentReportID).toBe(selfDMReport.reportID); + + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); + }); + + it('should drop the seeded transaction thread when the merge fails', async () => { + // Given the same thread, which exists only because the merge seeded it + const fixtures = await setupCrossReportMergeToSourceReportFixtures(); + const {mergeTransactionID, mergeTransaction, targetTransaction, sourceTransaction, mockViolations, targetTransactionThreadID, targetTransactionThread} = fixtures; + const selfDMReport = { + ...createRandomReport(5), + reportID: 'self-dm-report-123', + type: CONST.REPORT.TYPE.CHAT, + }; + const unreportedSourceTransaction = {...sourceTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; + const unreportedMergeTransaction = {...mergeTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; + + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, null); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${unreportedSourceTransaction.transactionID}`, unreportedSourceTransaction); + await Onyx.set(`${ONYXKEYS.COLLECTION.MERGE_TRANSACTION}${mergeTransactionID}`, unreportedMergeTransaction); + await waitForBatchedUpdates(); + + // When the server rejects the merge + mockFetch?.pause?.(); + mergeTransactionRequest({ + isVendorMatchingBetaEnabled: false, + iouReportOwnerLogin: undefined, + mergeTransactionID, + mergeTransaction: unreportedMergeTransaction, + targetTransaction, + transactionsOfDeletableReportCollection: {[unreportedSourceTransaction.transactionID]: unreportedSourceTransaction}, + sourceTransaction: unreportedSourceTransaction, + policy: undefined, + policyTags: undefined, + policyCategories: undefined, + allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, unreportedSourceTransaction.transactionID, mockViolations, mockViolations), + sourceIOUActionThreadReport: undefined, + targetTransactionThreadReport: targetTransactionThread, + targetTransactionThreadParentReport: undefined, + reportPolicyTags: undefined, + currentUserAccountIDParam: 123, + currentUserEmailParam: 'existing@example.com', + isASAPSubmitBetaEnabled: false, + selfDMReport, + selfDMReportActions: undefined, + delegateAccountID: undefined, + isTrackIntentUser: false, + sourceTransactionThreadReportActions: undefined, + sourceIOUAction: undefined, + sourceActionIOUReport: undefined, + sourceActionChatReport: undefined, + getCurrencyDecimals: getCurrencyDecimalsLocal, + getCurrencySymbol: getCurrencySymbolLocal, + rules: undefined, + }); + await waitForBatchedUpdates(); + + mockFetch?.fail?.(); + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + // Then the report is removed rather than left behind: Onyx held nothing before the merge, so restoring the + // parent fields would recreate the same half-written report the seeding exists to avoid. + const rolledBackThread = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, + callback: (report) => { + Onyx.disconnect(connection); + resolve(report ?? null); + }, + }); + }); + + expect(rolledBackThread).toBeNull(); + }); + it('should not inject the target IOU action into the source report when a cross-report merge fails', async () => { const fixtures = await setupCrossReportMergeToSourceReportFixtures(); const {sourceExpenseReport, targetIOUActionID, targetTransactionThreadID, targetTransactionThread} = fixtures; From 64f7525aad058f5405f6c4b49ac72c55cee17695 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Mon, 26 Oct 2026 10:57:50 +0430 Subject: [PATCH 26/27] fix: seed missing thread report on merge --- src/libs/actions/MergeTransaction.ts | 141 +++++++++---- tests/actions/MergeTransactionTest.ts | 287 ++++++++++++++------------ 2 files changed, 258 insertions(+), 170 deletions(-) diff --git a/src/libs/actions/MergeTransaction.ts b/src/libs/actions/MergeTransaction.ts index 718e9efafc52..dc51bb34fa7f 100644 --- a/src/libs/actions/MergeTransaction.ts +++ b/src/libs/actions/MergeTransaction.ts @@ -22,6 +22,7 @@ import {getLoginByAccountID} from '@libs/PersonalDetailsUtils'; import {isPaidGroupPolicy, isPolicyAdmin, resolveCurrentTaxCode} from '@libs/PolicyUtils'; import {getIOUActionForReportID, getReportAction, getTrackExpenseActionableWhisper} from '@libs/ReportActionsUtils'; import { + buildOptimisticCreatedReportAction, buildOptimisticIOUReportAction, buildTransactionThread, getReportOrDraftReport, @@ -762,14 +763,18 @@ function mergeTransactionRequest({ }, ); - const optimisticData: Array> = [ + const optimisticData: Array> = [ ...(onyxTargetTransactionData.optimisticData ?? []), optimisticMergeTransactionData, ...optimisticTransactionViolations, ...sourceTransactionOptimisticData, ]; - const failureData: Array> = [...(onyxTargetTransactionData.failureData ?? []), ...failureTransactionViolations, ...sourceTransactionFailureData]; + const failureData: Array> = [ + ...(onyxTargetTransactionData.failureData ?? []), + ...failureTransactionViolations, + ...sourceTransactionFailureData, + ]; const successData: Array> = []; successData.push(...sourceTransactionSuccessData); @@ -800,47 +805,103 @@ function mergeTransactionRequest({ // Reattach the transaction thread to the new IOU action on the destination report. newIOUAction.childReportID = targetTransactionThreadReportID; - // The thread can be one that was never opened, so Onyx has no report for it even though we hold a copy - // from the Search snapshot. Merging only the parent fields onto that key would leave a report with no - // reportID, and we navigate straight to it after the merge, so the screen would show a skeleton that - // offline never clears. Seed the whole report in that case instead. + // The thread can be one that was never opened, so Onyx has no report for it even though the confirmation + // page holds a copy from the Search snapshot. A report saved by an earlier merge counts as missing too: + // it holds the two parent fields and nothing else, so it has no reportID and would otherwise never heal. + // Merging the parent fields onto either leaves a report the screen reads as unloaded, and we navigate + // straight to this thread after the merge, so it is what the user lands on. const threadReportInOnyx = getReportOrDraftReport(targetTransactionThreadReportID); - const reattachedThreadFields = { - parentReportID: optimisticDestinationReportID, - parentReportActionID: newIOUAction.reportActionID, - }; + const threadReportKey: `${typeof ONYXKEYS.COLLECTION.REPORT}${string}` = `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`; + const threadReportActionsKey: `${typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS}${string}` = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${targetTransactionThreadReportID}`; + const threadLoadingStateKey: `${typeof ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${string}` = `${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${targetTransactionThreadReportID}`; - optimisticData.push({ - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`, - value: threadReportInOnyx - ? reattachedThreadFields - : { - ...buildTransactionThread( - newIOUAction, - getReportOrDraftReport(optimisticDestinationReportID) ?? selfDMReport, - currentUserAccountIDParam, - undefined, - targetTransactionThreadReportID, - ), - ...targetTransactionThreadReport, - reportID: targetTransactionThreadReportID, - ...reattachedThreadFields, - }, - }); + if (threadReportInOnyx?.reportID) { + optimisticData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: threadReportKey, + value: { + parentReportID: optimisticDestinationReportID, + parentReportActionID: newIOUAction.reportActionID, + }, + }); - failureData.push({ - // Onyx held nothing before the merge in the seeded case, so a failure drops the report rather than - // restoring fields onto one that should not exist. - onyxMethod: threadReportInOnyx ? Onyx.METHOD.MERGE : Onyx.METHOD.SET, - key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`, - value: threadReportInOnyx - ? { - parentReportID: threadReportInOnyx.parentReportID ?? targetTransaction.reportID, - parentReportActionID: threadReportInOnyx.parentReportActionID ?? targetIOUActionOnOriginalReport?.reportActionID, - } - : null, - }); + failureData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: threadReportKey, + value: { + parentReportID: threadReportInOnyx.parentReportID ?? targetTransaction.reportID, + parentReportActionID: threadReportInOnyx.parentReportActionID ?? targetIOUActionOnOriginalReport?.reportActionID, + }, + }); + } else { + // The snapshot copy, when there is one, carries the thread's real fields, so it is the better base. + const seededThreadReport = { + ...buildTransactionThread( + newIOUAction, + getReportOrDraftReport(optimisticDestinationReportID) ?? selfDMReport, + currentUserAccountIDParam, + undefined, + targetTransactionThreadReportID, + ), + ...targetTransactionThreadReport, + reportID: targetTransactionThreadReportID, + parentReportID: optimisticDestinationReportID, + parentReportActionID: newIOUAction.reportActionID, + }; + const seededCreatedAction = buildOptimisticCreatedReportAction({ + emailCreatingAction: currentUserEmailParam, + currentUserAccountID: currentUserAccountIDParam, + currentUserEmail: currentUserEmailParam, + }); + + optimisticData.push( + { + onyxMethod: Onyx.METHOD.SET, + key: threadReportKey, + value: seededThreadReport, + }, + { + // An empty thread still reads as loading, so the placeholder keeps the screen off the skeleton. + onyxMethod: Onyx.METHOD.SET, + key: threadReportActionsKey, + value: {[seededCreatedAction.reportActionID]: seededCreatedAction}, + }, + { + // Only an OpenReport response clears these, and offline that response never arrives. + onyxMethod: Onyx.METHOD.MERGE, + key: threadLoadingStateKey, + value: {hasOnceLoadedReportActions: true, isLoadingInitialReportActions: false}, + }, + ); + + // The response carries the thread's real actions, so the placeholder is dropped rather than left + // alongside them. + successData.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: threadReportActionsKey, + value: {[seededCreatedAction.reportActionID]: null}, + }); + + // Nothing usable was stored before the merge, so a failure drops the thread instead of restoring + // fields onto a report that should not exist. + failureData.push( + { + onyxMethod: Onyx.METHOD.SET, + key: threadReportKey, + value: null, + }, + { + onyxMethod: Onyx.METHOD.SET, + key: threadReportActionsKey, + value: null, + }, + { + onyxMethod: Onyx.METHOD.SET, + key: threadLoadingStateKey, + value: null, + }, + ); + } } // For a Self-DM destination the server creates its own TRACK action instead of reusing diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index 67b187eb353a..6be6d55bde0f 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -1139,151 +1139,178 @@ describe('mergeTransactionRequest', () => { expect(updatedTargetTransactionThread?.parentReportActionID).toBe(placeholderReportActionID); }); - it('should seed the whole transaction thread when Onyx has no report for it', async () => { - // Given a thread the confirmation page resolved from the Search snapshot, so a report object is passed in - // while Onyx itself holds nothing under that ID. This is the state a thread that was never opened lands in. - const fixtures = await setupCrossReportMergeToSourceReportFixtures(); - const {mergeTransactionID, mergeTransaction, targetTransaction, sourceTransaction, mockViolations, targetTransactionThreadID, targetTransactionThread} = fixtures; - const selfDMReport = { - ...createRandomReport(5), - reportID: 'self-dm-report-123', - type: CONST.REPORT.TYPE.CHAT, - }; - const unreportedSourceTransaction = {...sourceTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; - const unreportedMergeTransaction = {...mergeTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; + describe('when the destination thread report is missing from Onyx', () => { + const SELF_DM_REPORT_ID = 'self-dm-report-123'; + + /** + * Runs a merge into Self DM for the given fixtures, with the thread report the confirmation page resolved. + * Passing `undefined` covers the merge being started outside Search, where only the IOU action's + * childReportID identifies the thread. + */ + async function runSelfDMMerge(fixtures: CrossReportMergeToSourceReportFixtures, resolvedThreadReport: Report | undefined) { + const {mergeTransactionID, mergeTransaction, targetTransaction, sourceTransaction, mockViolations} = fixtures; + const selfDMReport = { + ...createRandomReport(5), + reportID: SELF_DM_REPORT_ID, + type: CONST.REPORT.TYPE.CHAT, + }; + const unreportedSourceTransaction = {...sourceTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; + const unreportedMergeTransaction = {...mergeTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, null); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${unreportedSourceTransaction.transactionID}`, unreportedSourceTransaction); - await Onyx.set(`${ONYXKEYS.COLLECTION.MERGE_TRANSACTION}${mergeTransactionID}`, unreportedMergeTransaction); - await waitForBatchedUpdates(); + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${unreportedSourceTransaction.transactionID}`, unreportedSourceTransaction); + await Onyx.set(`${ONYXKEYS.COLLECTION.MERGE_TRANSACTION}${mergeTransactionID}`, unreportedMergeTransaction); + await waitForBatchedUpdates(); - // When the merge moves the expense to another destination and re-points the thread - mockFetch?.pause?.(); - mergeTransactionRequest({ - isVendorMatchingBetaEnabled: false, - iouReportOwnerLogin: undefined, - mergeTransactionID, - mergeTransaction: unreportedMergeTransaction, - targetTransaction, - transactionsOfDeletableReportCollection: {[unreportedSourceTransaction.transactionID]: unreportedSourceTransaction}, - sourceTransaction: unreportedSourceTransaction, - policy: undefined, - policyTags: undefined, - policyCategories: undefined, - allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, unreportedSourceTransaction.transactionID, mockViolations, mockViolations), - sourceIOUActionThreadReport: undefined, - // Truthy, like the snapshot copy the page passes, even though Onyx has nothing under this ID - targetTransactionThreadReport: targetTransactionThread, - targetTransactionThreadParentReport: undefined, - reportPolicyTags: undefined, - currentUserAccountIDParam: 123, - currentUserEmailParam: 'existing@example.com', - isASAPSubmitBetaEnabled: false, - selfDMReport, - selfDMReportActions: undefined, - delegateAccountID: undefined, - isTrackIntentUser: false, - sourceTransactionThreadReportActions: undefined, - sourceIOUAction: undefined, - sourceActionIOUReport: undefined, - sourceActionChatReport: undefined, - getCurrencyDecimals: getCurrencyDecimalsLocal, - getCurrencySymbol: getCurrencySymbolLocal, - rules: undefined, + mergeTransactionRequest({ + isVendorMatchingBetaEnabled: false, + iouReportOwnerLogin: undefined, + mergeTransactionID, + mergeTransaction: unreportedMergeTransaction, + targetTransaction, + transactionsOfDeletableReportCollection: {[unreportedSourceTransaction.transactionID]: unreportedSourceTransaction}, + sourceTransaction: unreportedSourceTransaction, + policy: undefined, + policyTags: undefined, + policyCategories: undefined, + allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, unreportedSourceTransaction.transactionID, mockViolations, mockViolations), + sourceIOUActionThreadReport: undefined, + targetTransactionThreadReport: resolvedThreadReport, + targetTransactionThreadParentReport: undefined, + reportPolicyTags: undefined, + currentUserAccountIDParam: 123, + currentUserEmailParam: 'existing@example.com', + isASAPSubmitBetaEnabled: false, + selfDMReport, + selfDMReportActions: undefined, + delegateAccountID: undefined, + isTrackIntentUser: false, + sourceTransactionThreadReportActions: undefined, + sourceIOUAction: undefined, + sourceActionIOUReport: undefined, + sourceActionChatReport: undefined, + getCurrencyDecimals: getCurrencyDecimalsLocal, + getCurrencySymbol: getCurrencySymbolLocal, + rules: undefined, + }); + await waitForBatchedUpdates(); + } + + it('should seed the whole thread when the page resolved it from the Search snapshot', async () => { + // Given a thread the confirmation page resolved from the snapshot, so a report object is passed in while + // Onyx itself holds nothing under that ID + const fixtures = await setupCrossReportMergeToSourceReportFixtures(); + const {targetTransactionThreadID, targetTransactionThread} = fixtures; + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, null); + + // When the merge re-points that thread onto Self DM + mockFetch?.pause?.(); + await runSelfDMMerge(fixtures, targetTransactionThread); + + // Then the report carries a reportID instead of only the two parent fields, and comes with an action and + // a loaded state. The merge navigates straight here, and an unloaded or empty thread shows a skeleton + // that offline never clears. + const seededThread = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`); + const seededThreadActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${targetTransactionThreadID}`); + const seededLoadingState = await getOnyxValue(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${targetTransactionThreadID}`); + + expect(seededThread?.reportID).toBe(targetTransactionThreadID); + expect(seededThread?.parentReportID).toBe(SELF_DM_REPORT_ID); + expect(Object.keys(seededThreadActions ?? {}).length).toBe(1); + expect(seededLoadingState?.hasOnceLoadedReportActions).toBe(true); + expect(seededLoadingState?.isLoadingInitialReportActions).toBe(false); + + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); }); - await waitForBatchedUpdates(); - // Then the report carries a reportID rather than only the two parent fields. The merge navigates straight to - // this report, and a report without a reportID reads as unloaded, so the screen sits on a loading skeleton - // that offline never clears. - const seededThread = await new Promise((resolve) => { - const connection = Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, - callback: (report) => { - Onyx.disconnect(connection); - resolve(report ?? null); - }, - }); + it('should seed the whole thread when only the IOU action identifies it', async () => { + // Given a merge started outside Search, so no snapshot copy is passed and the thread is known only from + // the target IOU action's childReportID + const fixtures = await setupCrossReportMergeToSourceReportFixtures(); + const {targetTransactionThreadID} = fixtures; + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, null); + + // When the merge re-points that thread onto Self DM + mockFetch?.pause?.(); + await runSelfDMMerge(fixtures, undefined); + + // Then the thread is still built from scratch rather than written as two loose fields + const seededThread = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`); + const seededThreadActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${targetTransactionThreadID}`); + + expect(seededThread?.reportID).toBe(targetTransactionThreadID); + expect(seededThread?.parentReportID).toBe(SELF_DM_REPORT_ID); + expect(Object.keys(seededThreadActions ?? {}).length).toBe(1); + + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); }); - expect(seededThread?.reportID).toBe(targetTransactionThreadID); - expect(seededThread?.parentReportID).toBe(selfDMReport.reportID); + it('should repair a thread an earlier merge stored without a reportID', async () => { + // Given the two-field report an earlier build wrote for this thread. Onyx has an entry, but it is not a + // usable report, so treating it as present would leave that state stuck for good. + const fixtures = await setupCrossReportMergeToSourceReportFixtures(); + const {targetTransactionThreadID} = fixtures; + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, null); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, { + parentReportID: 'stale-parent-report', + parentReportActionID: 'stale-parent-action', + }); - await mockFetch?.resume?.(); - await waitForBatchedUpdates(); - }); + // When the merge re-points that thread onto Self DM + mockFetch?.pause?.(); + await runSelfDMMerge(fixtures, undefined); - it('should drop the seeded transaction thread when the merge fails', async () => { - // Given the same thread, which exists only because the merge seeded it - const fixtures = await setupCrossReportMergeToSourceReportFixtures(); - const {mergeTransactionID, mergeTransaction, targetTransaction, sourceTransaction, mockViolations, targetTransactionThreadID, targetTransactionThread} = fixtures; - const selfDMReport = { - ...createRandomReport(5), - reportID: 'self-dm-report-123', - type: CONST.REPORT.TYPE.CHAT, - }; - const unreportedSourceTransaction = {...sourceTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; - const unreportedMergeTransaction = {...mergeTransaction, reportID: CONST.REPORT.UNREPORTED_REPORT_ID}; + // Then the broken entry is replaced by a usable report + const repairedThread = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, null); - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${unreportedSourceTransaction.transactionID}`, unreportedSourceTransaction); - await Onyx.set(`${ONYXKEYS.COLLECTION.MERGE_TRANSACTION}${mergeTransactionID}`, unreportedMergeTransaction); - await waitForBatchedUpdates(); + expect(repairedThread?.reportID).toBe(targetTransactionThreadID); + expect(repairedThread?.parentReportID).toBe(SELF_DM_REPORT_ID); - // When the server rejects the merge - mockFetch?.pause?.(); - mergeTransactionRequest({ - isVendorMatchingBetaEnabled: false, - iouReportOwnerLogin: undefined, - mergeTransactionID, - mergeTransaction: unreportedMergeTransaction, - targetTransaction, - transactionsOfDeletableReportCollection: {[unreportedSourceTransaction.transactionID]: unreportedSourceTransaction}, - sourceTransaction: unreportedSourceTransaction, - policy: undefined, - policyTags: undefined, - policyCategories: undefined, - allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, unreportedSourceTransaction.transactionID, mockViolations, mockViolations), - sourceIOUActionThreadReport: undefined, - targetTransactionThreadReport: targetTransactionThread, - targetTransactionThreadParentReport: undefined, - reportPolicyTags: undefined, - currentUserAccountIDParam: 123, - currentUserEmailParam: 'existing@example.com', - isASAPSubmitBetaEnabled: false, - selfDMReport, - selfDMReportActions: undefined, - delegateAccountID: undefined, - isTrackIntentUser: false, - sourceTransactionThreadReportActions: undefined, - sourceIOUAction: undefined, - sourceActionIOUReport: undefined, - sourceActionChatReport: undefined, - getCurrencyDecimals: getCurrencyDecimalsLocal, - getCurrencySymbol: getCurrencySymbolLocal, - rules: undefined, + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); }); - await waitForBatchedUpdates(); - mockFetch?.fail?.(); - await mockFetch?.resume?.(); - await waitForBatchedUpdates(); + it('should drop the placeholder action once the merge succeeds', async () => { + // Given a seeded thread whose single action is the placeholder + const fixtures = await setupCrossReportMergeToSourceReportFixtures(); + const {targetTransactionThreadID, targetTransactionThread} = fixtures; + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, null); - // Then the report is removed rather than left behind: Onyx held nothing before the merge, so restoring the - // parent fields would recreate the same half-written report the seeding exists to avoid. - const rolledBackThread = await new Promise((resolve) => { - const connection = Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, - callback: (report) => { - Onyx.disconnect(connection); - resolve(report ?? null); - }, - }); + // When the merge succeeds, so the response brings the thread's real actions + mockFetch?.pause?.(); + await runSelfDMMerge(fixtures, targetTransactionThread); + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + // Then the placeholder is removed rather than left sitting alongside them + const threadActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${targetTransactionThreadID}`); + + expect(Object.values(threadActions ?? {}).filter(Boolean).length).toBe(0); }); - expect(rolledBackThread).toBeNull(); + it('should drop the seeded thread when the merge fails', async () => { + // Given a thread that exists only because the merge seeded it + const fixtures = await setupCrossReportMergeToSourceReportFixtures(); + const {targetTransactionThreadID, targetTransactionThread} = fixtures; + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`, null); + + // When the server rejects the merge + mockFetch?.pause?.(); + await runSelfDMMerge(fixtures, targetTransactionThread); + mockFetch?.fail?.(); + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + // Then it is removed rather than left behind: Onyx held nothing usable before the merge, so restoring + // the parent fields would recreate the same broken report the seeding exists to avoid. + const rolledBackThread = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`); + const rolledBackThreadActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${targetTransactionThreadID}`); + + expect(rolledBackThread).toBeUndefined(); + expect(rolledBackThreadActions).toBeUndefined(); + }); }); it('should not inject the target IOU action into the source report when a cross-report merge fails', async () => { From c1fc32e9bd924f10efa15c921383bcb2af8f3697 Mon Sep 17 00:00:00 2001 From: Maruf Sharifi Date: Fri, 30 Oct 2026 12:10:16 +0430 Subject: [PATCH 27/27] remove unused import --- tests/actions/MergeTransactionTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/actions/MergeTransactionTest.ts b/tests/actions/MergeTransactionTest.ts index 92d730690b64..aa1f44b5fbbe 100644 --- a/tests/actions/MergeTransactionTest.ts +++ b/tests/actions/MergeTransactionTest.ts @@ -4,7 +4,6 @@ import {addComment, openReport} from '@libs/actions/Report'; import * as API from '@libs/API'; import {WRITE_COMMANDS} from '@libs/API/types'; import {getMergeFieldUpdatedValues} from '@libs/MergeTransactionUtils'; -import {getLoginsByAccountIDs} from '@libs/PersonalDetailsUtils'; import {getOriginalMessage, getReportAction, isActionOfType, isMoneyRequestAction} from '@libs/ReportActionsUtils'; import {buildTransactionThread} from '@libs/ReportUtils';