Skip to content

Commit 9ca5bd4

Browse files
cristipavalOSBotify
authored andcommitted
Merge pull request #95163 from Expensify/claude-reviewFlaggedExpenseSingleThread
Fix: open flagged expense thread when its report holds multiple transactions (cherry picked from commit 9fb8f63) (cherry-picked to staging by cristipaval)
1 parent 8aad9f6 commit 9ca5bd4

3 files changed

Lines changed: 125 additions & 3 deletions

File tree

src/pages/home/ForYouSection/useReviewFlaggedExpenses.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import useOnyx from '@hooks/useOnyx';
77
import useResponsiveLayout from '@hooks/useResponsiveLayout';
88
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
99
import Navigation from '@libs/Navigation/Navigation';
10-
import {isMoneyRequestReport} from '@libs/ReportUtils';
10+
import {isOneTransactionReport} from '@libs/ReportUtils';
1111
import {getVisibleTransactionViolations} from '@libs/TransactionUtils';
1212
import CONST from '@src/CONST';
1313
import ONYXKEYS from '@src/ONYXKEYS';
@@ -166,8 +166,10 @@ function useReviewFlaggedExpenses(): ReviewFlaggedExpenses {
166166
const firstFlaggedTransaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${firstFlaggedExpense.transactionID}`];
167167

168168
// With a single flagged expense the review carousel has nothing to navigate between, and for a
169-
// one-transaction report the transaction thread is a redundant duplicate of the report itself
170-
if (flaggedExpenses.length === 1 && isMoneyRequestReport(firstFlaggedReport)) {
169+
// one-transaction report the transaction thread is a redundant duplicate of the report itself.
170+
// Gate on the report's transaction count (not its type) so a lone flagged expense inside a
171+
// multi-transaction report still opens that expense's thread rather than the whole report.
172+
if (flaggedExpenses.length === 1 && isOneTransactionReport(firstFlaggedReport)) {
171173
Navigation.navigate(
172174
shouldUseNarrowLayout
173175
? ROUTES.REPORT_WITH_ID.getRoute(firstFlaggedExpense.reportID, undefined, undefined, ROUTES.HOME)

tests/ui/ForYouSectionTest.tsx

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,83 @@ describe('ForYouSection', () => {
390390
expect(mockNavigate).not.toHaveBeenCalled();
391391
});
392392

393+
it('opens the flagged expense thread when a lone flagged expense sits inside a multi-transaction report', async () => {
394+
// Repro of the deploy blocker: an OPEN expense report with two transactions where only one is still
395+
// flagged. transactionCount is 2, so pressing the row must open the flagged expense's thread rather
396+
// than the whole report (which would show both the flagged and unflagged expenses).
397+
await act(async () => {
398+
setTodoCounts(BASE_TODOS);
399+
await Onyx.set(
400+
`${ONYXKEYS.COLLECTION.REPORT}r1`,
401+
createMockReport({
402+
reportID: 'r1',
403+
type: CONST.REPORT.TYPE.EXPENSE,
404+
ownerAccountID: ACCOUNT_ID,
405+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
406+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
407+
transactionCount: 2,
408+
}),
409+
);
410+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t1`, {transactionID: 't1', reportID: 'r1', amount: 100, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'});
411+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t2`, {transactionID: 't2', reportID: 'r1', amount: 200, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'});
412+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}t1`, [
413+
{type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.MISSING_CATEGORY},
414+
] as TransactionViolations);
415+
});
416+
await waitForBatchedUpdatesWithAct();
417+
418+
renderForYouSection();
419+
await waitForBatchedUpdatesWithAct();
420+
421+
pressFirstBeginButton();
422+
423+
expect(mockNavigateToTransactionThread).toHaveBeenCalledTimes(1);
424+
expect(mockNavigateToTransactionThread).toHaveBeenCalledWith(
425+
expect.objectContaining({
426+
transactionID: 't1',
427+
report: expect.objectContaining({reportID: 'r1'}),
428+
siblingTransactionIDs: ['t1'],
429+
backTo: ROUTES.HOME,
430+
}),
431+
);
432+
// The whole-report route must not be used when the report holds more than one transaction.
433+
expect(mockNavigate).not.toHaveBeenCalled();
434+
});
435+
436+
it('opens the report directly when the lone flagged expense is the report only transaction', async () => {
437+
// A genuine one-transaction report keeps the shortcut: the transaction thread would be a redundant
438+
// duplicate of the report, so navigate straight to the expense report.
439+
await act(async () => {
440+
setTodoCounts(BASE_TODOS);
441+
await Onyx.set(
442+
`${ONYXKEYS.COLLECTION.REPORT}r1`,
443+
createMockReport({
444+
reportID: 'r1',
445+
type: CONST.REPORT.TYPE.EXPENSE,
446+
ownerAccountID: ACCOUNT_ID,
447+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
448+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
449+
transactionCount: 1,
450+
}),
451+
);
452+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t1`, {transactionID: 't1', reportID: 'r1', amount: 100, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'});
453+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}t1`, [
454+
{type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.MISSING_CATEGORY},
455+
] as TransactionViolations);
456+
});
457+
await waitForBatchedUpdatesWithAct();
458+
459+
renderForYouSection();
460+
await waitForBatchedUpdatesWithAct();
461+
462+
pressFirstBeginButton();
463+
464+
// Wide layout (default in beforeEach) → EXPENSE_REPORT_RHP.
465+
expect(mockNavigate).toHaveBeenCalledTimes(1);
466+
expect(mockNavigate).toHaveBeenCalledWith(ROUTES.EXPENSE_REPORT_RHP.getRoute({reportID: 'r1', backTo: ROUTES.HOME}));
467+
expect(mockNavigateToTransactionThread).not.toHaveBeenCalled();
468+
});
469+
393470
it('does not render the review row or navigate when a violated transaction is not on a current-user OPEN expense report', async () => {
394471
await act(async () => {
395472
setTodoCounts(BASE_TODOS);

tests/unit/hooks/useReviewFlaggedExpenses.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,49 @@ describe('useReviewFlaggedExpenses', () => {
129129
);
130130
});
131131

132+
it('opens the flagged transaction thread when a lone flagged expense sits inside a multi-transaction report', async () => {
133+
// A single OPEN expense report holding two transactions, but only one is flagged. The report's
134+
// transactionCount is 2, so the "open the report directly" shortcut (reserved for one-transaction
135+
// reports) must NOT apply — pressing the row should open the flagged expense's thread instead.
136+
await act(async () => {
137+
await Onyx.set(
138+
`${ONYXKEYS.COLLECTION.REPORT}r1`,
139+
createMockReport({
140+
reportID: 'r1',
141+
type: CONST.REPORT.TYPE.EXPENSE,
142+
ownerAccountID: ACCOUNT_ID,
143+
stateNum: CONST.REPORT.STATE_NUM.OPEN,
144+
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
145+
transactionCount: 2,
146+
}),
147+
);
148+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t1`, {transactionID: 't1', reportID: 'r1', amount: 100, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'});
149+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}t2`, {transactionID: 't2', reportID: 'r1', amount: 200, currency: 'USD', created: '2024-01-01', merchant: 'Test Merchant'});
150+
// Only t1 is flagged; t2 has no violations.
151+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}t1`, [{type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.MISSING_CATEGORY}]);
152+
});
153+
await waitForBatchedUpdatesWithAct();
154+
155+
const {result} = renderHook(() => useReviewFlaggedExpenses());
156+
await waitForBatchedUpdatesWithAct();
157+
158+
expect(result.current.count).toBe(1);
159+
160+
act(() => {
161+
result.current.reviewExpenses();
162+
});
163+
164+
expect(mockNavigateToTransactionThread).toHaveBeenCalledTimes(1);
165+
expect(mockNavigateToTransactionThread).toHaveBeenCalledWith(
166+
expect.objectContaining({
167+
transactionID: 't1',
168+
report: expect.objectContaining({reportID: 'r1'}),
169+
siblingTransactionIDs: ['t1'],
170+
backTo: ROUTES.HOME,
171+
}),
172+
);
173+
});
174+
132175
it('updates the count live while the Home tab stays focused', async () => {
133176
await act(async () => {
134177
await seedFlaggedExpenses({transactionID: 't1', reportID: 'r1'});

0 commit comments

Comments
 (0)