Fix merge expense transaction thread navigation

src/hooks/useSearchBulkActions.ts

@@ -3257,7 +3257,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, undefined),
+                        onSelected: () =>
+                            setupMergeTransactionDataAndNavigate(transactionID, searchedTransactions, localeCompare, getCurrencyDecimals, reports, false, true, transactionPolicies),
                     });
                 }
             }

src/libs/MergeTransactionUtils.ts

@@ -198,7 +198,7 @@ function getTransactionsAndReportsFromSearch(
 ): {
     transactions: Transaction[];
     reports: Report[];
-    policies: Policy[];
+    policies: Array<OnyxEntry<Policy>>;
 } {
     const transaction1 = searchResults.data[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDs.at(0)}`];
     const transaction2 = searchResults.data[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDs.at(1)}`];
@@ -210,7 +210,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}`]],
     };
 }
 

src/libs/ReportSecondaryActionUtils.ts

@@ -864,7 +864,7 @@ function isMergeAction(parentReport: Report, reportTransactions: Transaction[],
 function isMergeActionForSelectedTransactions(
     transactions: Transaction[],
     reports: Report[],
-    policies: Policy[],
+    policies: Array<OnyxEntry<Policy>>,
     rules: OnyxCollection<Rule>,
     // The full list is needed because the loop below resolves a different report owner on each pass.
     personalDetails: OnyxEntry<PersonalDetailsList>,

src/libs/TransactionUtils/getUpdatedTransaction.ts

@@ -125,7 +125,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,
@@ -150,7 +150,7 @@ function getUpdatedTransaction({
     getCurrencyDecimals: CurrencyListActionsContextType['getCurrencyDecimals'];
     getCurrencySymbol: CurrencyListActionsContextType['getCurrencySymbol'];
 }): Transaction {
-    const isUnReportedExpense = transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
+    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);

src/libs/actions/MergeTransaction.ts

@@ -22,7 +22,9 @@ import {getLoginByAccountID} from '@libs/PersonalDetailsUtils';
 import {isPaidGroupPolicy, isPolicyAdmin, resolveCurrentTaxCode} from '@libs/PolicyUtils';
 import {getIOUActionForReportID, getReportAction, getTrackExpenseActionableWhisper} from '@libs/ReportActionsUtils';
 import {
+    buildOptimisticCreatedReportAction,
     buildOptimisticIOUReportAction,
+    buildTransactionThread,
     getReportOrDraftReport,
     getTransactionDetails,
     isCurrentUserSubmitter,
@@ -559,6 +561,8 @@ 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<
         OnyxUpdate<typeof ONYXKEYS.COLLECTION.REPORT | typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS | typeof ONYXKEYS.COLLECTION.TRANSACTION | typeof ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS>
@@ -713,6 +717,19 @@ 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
+            // 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) {
+                sourceTransactionSuccessData.push({
+                    onyxMethod: Onyx.METHOD.MERGE,
+                    key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReportID}`,
+                    value: {
+                        [sourceIouAction.reportActionID]: null,
+                    },
+                });
+            }
         }
     }
     // Optimistic delete the merge transaction
@@ -746,84 +763,180 @@ function mergeTransactionRequest({
         },
     );
 
-    const optimisticData: Array<OnyxUpdate<UpdateMoneyRequestDataKeys | typeof ONYXKEYS.COLLECTION.MERGE_TRANSACTION>> = [
+    const optimisticData: Array<OnyxUpdate<UpdateMoneyRequestDataKeys | typeof ONYXKEYS.COLLECTION.MERGE_TRANSACTION | typeof ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE>> = [
         ...(onyxTargetTransactionData.optimisticData ?? []),
         optimisticMergeTransactionData,
         ...optimisticTransactionViolations,
         ...sourceTransactionOptimisticData,
     ];
 
-    const failureData: Array<OnyxUpdate<UpdateMoneyRequestDataKeys>> = [...(onyxTargetTransactionData.failureData ?? []), ...failureTransactionViolations, ...sourceTransactionFailureData];
+    const failureData: Array<OnyxUpdate<UpdateMoneyRequestDataKeys | typeof ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE>> = [
+        ...(onyxTargetTransactionData.failureData ?? []),
+        ...failureTransactionViolations,
+        ...sourceTransactionFailureData,
+    ];
 
     const successData: Array<OnyxUpdate<UpdateMoneyRequestDataKeys>> = [];
     successData.push(...sourceTransactionSuccessData);
     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 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,
             participants: [],
             transactionID: mergeTransaction.targetTransactionID,
-            iouReportID: mergeTransaction.reportID,
+            iouReportID: optimisticDestinationReportID,
             delegateAccountIDParam: delegateAccountID,
             getCurrencyDecimals,
         });
 
         // 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) {
+        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 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 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}`;
+
+            if (threadReportInOnyx?.reportID) {
+                optimisticData.push({
+                    onyxMethod: Onyx.METHOD.MERGE,
+                    key: threadReportKey,
+                    value: {
+                        parentReportID: optimisticDestinationReportID,
+                        parentReportActionID: newIOUAction.reportActionID,
+                    },
+                });
+
+                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
+        // 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}`;
+
+        if (!isSelfDMDestinationWithoutReport) {
             optimisticData.push({
                 onyxMethod: Onyx.METHOD.MERGE,
-                key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`,
+                key: destinationReportActionsKey,
                 value: {
-                    parentReportID: mergeTransaction.reportID,
-                    parentReportActionID: newIOUAction.reportActionID,
+                    [newIOUAction.reportActionID]: newIOUAction,
+                },
+            });
+
+            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}${targetTransactionThreadReportID}`,
+                key: destinationReportActionsKey,
                 value: {
-                    parentReportID: targetTransaction.reportID,
-                    parentReportActionID: targetIOUActionOnOriginalReport.reportActionID,
+                    [newIOUAction.reportActionID]: null,
                 },
             });
         }
 
-        optimisticData.push({
-            onyxMethod: Onyx.METHOD.MERGE,
-            key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${mergeTransaction.reportID}`,
-            value: {
-                [newIOUAction.reportActionID]: newIOUAction,
-            },
-        });
-
-        successData.push({
-            onyxMethod: Onyx.METHOD.MERGE,
-            key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${mergeTransaction.reportID}`,
-            value: {
-                [newIOUAction.reportActionID]: {pendingAction: null},
-            },
-        });
-
-        failureData.push({
-            onyxMethod: Onyx.METHOD.MERGE,
-            key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${mergeTransaction.reportID}`,
-            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.
         if (targetIOUActionOnOriginalReport) {