Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/libs/actions/MergeTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ function mergeTransactionRequest({
iouReportID: mergeTransaction.reportID,
});

const oldIOUAction = getIOUActionForReportID(mergeTransaction.reportID, mergeTransaction.sourceTransactionID);
const oldIOUAction = getIOUActionForReportID(targetTransaction.reportID, targetTransaction.transactionID);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use source report action when mutating source report actions

In the cross-report branch, oldIOUAction is now fetched from the target report, but the subsequent oldIOUAction mutations still write to REPORT_ACTIONS${mergeTransaction.reportID} (the source report). On a failed MERGE_TRANSACTION request, the rollback path re-inserts that target action into the source report, which can leave duplicated/misplaced expense actions in local state after offline retries or server-side merge rejection.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore thread parent report ID during merge rollback

After this change, oldIOUAction may point to the target transaction thread, and the optimistic update moves that thread's parentReportID to mergeTransaction.reportID. However, rollback only restores parentReportActionID, not parentReportID, so if the merge API call fails the thread can remain attached to the wrong report and produce incorrect RHP/thread navigation state.

Useful? React with 👍 / 👎.

const oldTransactionThreadID = oldIOUAction?.childReportID;

if (oldTransactionThreadID) {
Expand Down
36 changes: 29 additions & 7 deletions tests/actions/MergeTransactionTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,26 @@ describe('mergeTransactionRequest', () => {
parentReportID: sourceExpenseReport.reportID,
parentReportActionID: sourceIOUActionID,
};
const targetIOUActionID = 'target-iou-action-456';
const targetTransactionThreadID = 'target-transaction-thread-456';
const targetIOUAction: ReportAction = {
reportActionID: targetIOUActionID,
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
created: '2024-01-01 12:00:00',
originalMessage: {
IOUTransactionID: targetTransaction.transactionID,
type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
IOUReportID: targetReport.reportID,
} as OriginalMessageIOU,
childReportID: targetTransactionThreadID,
message: [{type: 'TEXT', text: 'Target IOU action'}],
};
const targetTransactionThread = {
...createRandomReport(4, CONST.REPORT.CHAT_TYPE.INVOICE),
reportID: targetTransactionThreadID,
parentReportID: targetReport.reportID,
parentReportActionID: targetIOUActionID,
};

// Set up initial state in Onyx
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${targetTransaction.transactionID}`, targetTransaction);
Expand All @@ -674,6 +694,8 @@ describe('mergeTransactionRequest', () => {
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${sourceTransactionThread.reportID}`, sourceTransactionThread);
await Onyx.set(`${ONYXKEYS.COLLECTION.MERGE_TRANSACTION}${mergeTransactionID}`, mergeTransaction);
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetReport.reportID}`, targetReport);
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${targetReport.reportID}`, {[targetIOUAction.reportActionID]: targetIOUAction});
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThread.reportID}`, targetTransactionThread);

mockFetch?.pause?.();
const mockViolations = createMockViolations();
Expand Down Expand Up @@ -724,8 +746,8 @@ describe('mergeTransactionRequest', () => {
});
});

// Verify the source transaction's IOU action is removed from report actions
expect(sourceReportActions?.[sourceIOUActionID]).toBeUndefined();
// 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) => {
const reportAction = action as OnyxEntry<ReportAction>;
Expand All @@ -741,19 +763,19 @@ describe('mergeTransactionRequest', () => {
expect(newIOUAction).toBeTruthy();
expect((getOriginalMessage(newIOUAction) as OriginalMessageIOU | undefined)?.IOUTransactionID).toBe(targetTransaction.transactionID);

const updatedSourceTransactionThread = await new Promise<Report | null>((resolve) => {
const updatedTargetTransactionThread = await new Promise<Report | null>((resolve) => {
const connection = Onyx.connect({
key: `${ONYXKEYS.COLLECTION.REPORT}${sourceTransactionThreadID}`,
key: `${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadID}`,
callback: (report) => {
Onyx.disconnect(connection);
resolve(report ?? null);
},
});
});

// Verify transaction thread parent references are rewired to the new IOU action
expect(updatedSourceTransactionThread?.parentReportID).toBe(sourceExpenseReport.reportID);
expect(updatedSourceTransactionThread?.parentReportActionID).toBe(newIOUAction?.reportActionID);
// 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);
});

describe('Report deletion logic', () => {
Expand Down
Loading