From 1fd663a922b91438d5a6f204c0533142f7e26110 Mon Sep 17 00:00:00 2001 From: "Cristi Paval (via MelvinBot)" Date: Thu, 2 Jul 2026 10:27:42 +0000 Subject: [PATCH] Fix: open flagged expense thread when its report holds multiple transactions The Review row shortcut used isMoneyRequestReport (report type) to decide whether to open the report directly. That check is always true for expense reports, so a single flagged expense inside a multi-transaction report opened the whole report instead of the flagged expense. Gate on isOneTransactionReport (transactionCount === 1) so a lone flagged expense in a multi-transaction report opens its own thread. Co-authored-by: Cristi Paval --- .../ForYouSection/useReviewFlaggedExpenses.ts | 8 +- tests/ui/ForYouSectionTest.tsx | 77 +++++++++++++++++++ .../hooks/useReviewFlaggedExpenses.test.ts | 43 +++++++++++ 3 files changed, 125 insertions(+), 3 deletions(-) diff --git a/src/pages/home/ForYouSection/useReviewFlaggedExpenses.ts b/src/pages/home/ForYouSection/useReviewFlaggedExpenses.ts index dedc871f47cb..7bdca21281e4 100644 --- a/src/pages/home/ForYouSection/useReviewFlaggedExpenses.ts +++ b/src/pages/home/ForYouSection/useReviewFlaggedExpenses.ts @@ -7,7 +7,7 @@ import useOnyx from '@hooks/useOnyx'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import Navigation from '@libs/Navigation/Navigation'; -import {isMoneyRequestReport} from '@libs/ReportUtils'; +import {isOneTransactionReport} from '@libs/ReportUtils'; import {getVisibleTransactionViolations} from '@libs/TransactionUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -166,8 +166,10 @@ function useReviewFlaggedExpenses(): ReviewFlaggedExpenses { const firstFlaggedTransaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${firstFlaggedExpense.transactionID}`]; // With a single flagged expense the review carousel has nothing to navigate between, and for a - // one-transaction report the transaction thread is a redundant duplicate of the report itself - if (flaggedExpenses.length === 1 && isMoneyRequestReport(firstFlaggedReport)) { + // one-transaction report the transaction thread is a redundant duplicate of the report itself. + // Gate on the report's transaction count (not its type) so a lone flagged expense inside a + // multi-transaction report still opens that expense's thread rather than the whole report. + if (flaggedExpenses.length === 1 && isOneTransactionReport(firstFlaggedReport)) { Navigation.navigate( shouldUseNarrowLayout ? ROUTES.REPORT_WITH_ID.getRoute(firstFlaggedExpense.reportID, undefined, undefined, ROUTES.HOME) diff --git a/tests/ui/ForYouSectionTest.tsx b/tests/ui/ForYouSectionTest.tsx index a1a4547c57ed..ef87cc482eed 100644 --- a/tests/ui/ForYouSectionTest.tsx +++ b/tests/ui/ForYouSectionTest.tsx @@ -390,6 +390,83 @@ describe('ForYouSection', () => { expect(mockNavigate).not.toHaveBeenCalled(); }); + it('opens the flagged expense thread when a lone flagged expense sits inside a multi-transaction report', async () => { + // Repro of the deploy blocker: an OPEN expense report with two transactions where only one is still + // flagged. transactionCount is 2, so pressing the row must open the flagged expense's thread rather + // than the whole report (which would show both the flagged and unflagged expenses). + await act(async () => { + setTodoCounts(BASE_TODOS); + await Onyx.set( + `${ONYXKEYS.COLLECTION.REPORT}r1`, + createMockReport({ + reportID: 'r1', + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: ACCOUNT_ID, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + transactionCount: 2, + }), + ); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t1`, {transactionID: 't1', reportID: 'r1', amount: 100, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'}); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t2`, {transactionID: 't2', reportID: 'r1', amount: 200, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'}); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}t1`, [ + {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.MISSING_CATEGORY}, + ] as TransactionViolations); + }); + await waitForBatchedUpdatesWithAct(); + + renderForYouSection(); + await waitForBatchedUpdatesWithAct(); + + pressFirstBeginButton(); + + expect(mockNavigateToTransactionThread).toHaveBeenCalledTimes(1); + expect(mockNavigateToTransactionThread).toHaveBeenCalledWith( + expect.objectContaining({ + transactionID: 't1', + report: expect.objectContaining({reportID: 'r1'}), + siblingTransactionIDs: ['t1'], + backTo: ROUTES.HOME, + }), + ); + // The whole-report route must not be used when the report holds more than one transaction. + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('opens the report directly when the lone flagged expense is the report only transaction', async () => { + // A genuine one-transaction report keeps the shortcut: the transaction thread would be a redundant + // duplicate of the report, so navigate straight to the expense report. + await act(async () => { + setTodoCounts(BASE_TODOS); + await Onyx.set( + `${ONYXKEYS.COLLECTION.REPORT}r1`, + createMockReport({ + reportID: 'r1', + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: ACCOUNT_ID, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + transactionCount: 1, + }), + ); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t1`, {transactionID: 't1', reportID: 'r1', amount: 100, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'}); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}t1`, [ + {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.MISSING_CATEGORY}, + ] as TransactionViolations); + }); + await waitForBatchedUpdatesWithAct(); + + renderForYouSection(); + await waitForBatchedUpdatesWithAct(); + + pressFirstBeginButton(); + + // Wide layout (default in beforeEach) → EXPENSE_REPORT_RHP. + expect(mockNavigate).toHaveBeenCalledTimes(1); + expect(mockNavigate).toHaveBeenCalledWith(ROUTES.EXPENSE_REPORT_RHP.getRoute({reportID: 'r1', backTo: ROUTES.HOME})); + expect(mockNavigateToTransactionThread).not.toHaveBeenCalled(); + }); + it('does not render the review row or navigate when a violated transaction is not on a current-user OPEN expense report', async () => { await act(async () => { setTodoCounts(BASE_TODOS); diff --git a/tests/unit/hooks/useReviewFlaggedExpenses.test.ts b/tests/unit/hooks/useReviewFlaggedExpenses.test.ts index f569b1041255..76bbb25df471 100644 --- a/tests/unit/hooks/useReviewFlaggedExpenses.test.ts +++ b/tests/unit/hooks/useReviewFlaggedExpenses.test.ts @@ -129,6 +129,49 @@ describe('useReviewFlaggedExpenses', () => { ); }); + it('opens the flagged transaction thread when a lone flagged expense sits inside a multi-transaction report', async () => { + // A single OPEN expense report holding two transactions, but only one is flagged. The report's + // transactionCount is 2, so the "open the report directly" shortcut (reserved for one-transaction + // reports) must NOT apply — pressing the row should open the flagged expense's thread instead. + await act(async () => { + await Onyx.set( + `${ONYXKEYS.COLLECTION.REPORT}r1`, + createMockReport({ + reportID: 'r1', + type: CONST.REPORT.TYPE.EXPENSE, + ownerAccountID: ACCOUNT_ID, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS_NUM.OPEN, + transactionCount: 2, + }), + ); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t1`, {transactionID: 't1', reportID: 'r1', amount: 100, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'}); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t2`, {transactionID: 't2', reportID: 'r1', amount: 200, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'}); + // Only t1 is flagged; t2 has no violations. + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}t1`, [{type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.MISSING_CATEGORY}]); + }); + await waitForBatchedUpdatesWithAct(); + + const {result} = renderHook(() => useReviewFlaggedExpenses()); + await waitForBatchedUpdatesWithAct(); + + expect(result.current.count).toBe(1); + + act(() => { + result.current.reviewExpenses(); + }); + + expect(mockNavigateToTransactionThread).toHaveBeenCalledTimes(1); + expect(mockNavigateToTransactionThread).toHaveBeenCalledWith( + expect.objectContaining({ + transactionID: 't1', + report: expect.objectContaining({reportID: 'r1'}), + siblingTransactionIDs: ['t1'], + backTo: ROUTES.HOME, + }), + ); + }); + it('updates the count live while the Home tab stays focused', async () => { await act(async () => { await seedFlaggedExpenses({transactionID: 't1', reportID: 'r1'});