[CP Staging] Fix: open flagged expense thread when its report holds multiple transactions#95163
Conversation
…actions 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 <cristipaval@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🤖 I looked into the failing checks. There's no code fix to push — the only failing check is the PR Author Checklist, which is a human-completion gate, not a code problem. What's failing: Why I can't fix it: those checkboxes (plus the Everything else is green or on track: typecheck, ESLint, Prettier, Storybook, React Compiler, unused-styles, and all 8 Jest TL;DR: nothing to fix in code. Complete the PR Author Checklist to turn the |
|
🚧 cristipaval has triggered a test Expensify/App build. You can view the workflow run here. |
|
🤖 I re-checked the checks now that the builds finished. Two checks are red, and neither is a code problem — there's nothing to fix in the PR. 1. The log shows no source-level compile error — no
followed by a generic Evidence it's not caused by the changes here:
I can't re-trigger the job from here (permission-gated in this run). 2. Fails with "You completed 2 out of 41 checklist items." These checkboxes (and the Everything else is green: typecheck, ESLint, Prettier, Storybook, React Compiler, unused-styles, and all 8 Jest |
This comment has been minimized.
This comment has been minimized.
|
@cristipaval Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
🤖 Update: a second check has now failed, but neither failing check is caused by this PR, and there's nothing for me to fix in code. 1.
2. Everything code-related is green: typecheck, ESLint, Prettier, Storybook, React Compiler, unused-styles, and all 8 Jest TL;DR: no code fix to push. The iOS build break is a pre-existing infra issue affecting |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-07-02.15.10.34.movAndroid: mWeb Chrome2026-07-02.15.12.42.moviOS: HybridApp2026-07-02.15.10.34.moviOS: mWeb Safari2026-07-02.15.12.42.movMacOS: Chrome / Safari2026-07-02.15.05.59.mov |
|
@MelvinBot |
|
🤖 Added the concrete reproduction steps from #95089 to the Tests and QA Steps sections of the PR description:
|
|
🚧 cristipaval has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
…ingleThread Fix: open flagged expense thread when its report holds multiple transactions (cherry picked from commit 9fb8f63) (cherry-picked to staging by cristipaval)
|
🚀 Cherry-picked to staging by https://github.com/cristipaval in version: 9.4.26-5 🚀
|
|
🤖 Help site review: no docs changes required. This PR is a pure navigation bug fix. It changes only the internal logic in There is no new feature, no new/renamed UI element, no changed setting, and no change to any documented workflow — it simply restores the expected behavior. Nothing under Because no changes are required, I did not create a draft help site PR. If you disagree and think an article should be updated, let me know which flow to document and I'll draft one.
|
|
🚀 Cherry-picked to staging by https://github.com/cristipaval in version: 9.4.27-1 🚀
|
|
🤖 Help site review — no changes required. This PR is a bug fix that makes the actual behavior match what the help site already documents, so no updates to Why no docs change is neededThe only help article that documents where the Review item on Home navigates is
This PR fixes an edge case where a lone flagged expense living inside a multi-transaction report incorrectly opened the whole report instead of the flagged expense's RHP. After the fix, a lone flagged expense opens the expense thread — which is exactly what the article already describes. The docs were already correct; only the app behavior needed to be brought in line. I checked the other candidate articles ( @cristipaval, since no help site changes are required, there's no linked docs PR to review. If you believe the For you section article should be more explicit about this navigation behavior, let me know and I'll draft the change. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.4.27-2 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
The "Review X expenses" row on Home opened the whole expense report when a single flagged expense lived inside a report that still held other (unflagged) transactions, instead of opening the RHP for the flagged expense.
The handler in
useReviewFlaggedExpensesdecided between "open the report directly" and "open the single transaction thread" with:isMoneyRequestReportonly checks the report type (isIOUReport || isExpenseReport), which is alwaystruefor the OPEN expense reports this hook scans. SoflaggedExpenses.length === 1alone drove the branch, and the "open the report directly" shortcut — intended only for genuine one-transaction reports — fired even when the report contained multiple expenses.The fix swaps the type check for the transaction-count check that expresses the real intent,
isOneTransactionReport(report?.transactionCount === 1):Now a lone flagged expense inside a multi-transaction report falls through to
navigateToTransactionThread, opening the RHP for that single flagged expense (matching the Expected Result). The "open the report directly" shortcut stays reserved for reports that genuinely hold only one expense.Added unit coverage in
tests/unit/hooks/useReviewFlaggedExpenses.test.ts(lone flagged expense in a 2-transaction report → opens the thread) and UI coverage intests/ui/ForYouSectionTest.tsxfor both the multi-transaction (→ thread) and single-transaction (→ report directly) paths.Fixed Issues
$ #95089
PROPOSAL: #95089 (comment)
Tests
Offline tests
Same as tests.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.