Repository navigation
Fix: open expense report (not pay message thread) for paid expenses in Recently added - #99668
Conversation
…penses in Recently added Co-authored-by: Georgia Monahan <grgia@users.noreply.github.com>
|
🚧 grgia has triggered a test Expensify/App build. You can view the workflow run here. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
This comment has been minimized.
This comment has been minimized.
|
🤖 Ran the testing steps from this PR on Android (standalone NewDot). Outcome: ❌ FAIL — the payment flow works and the paid expense shows up in Home "Recently added", but tapping it from "Recently added" still opens the "marked as paid" system-message thread instead of the expense report. The regression this PR is meant to fix reproduced twice with fresh element refs. The actual expense report was only reachable by additionally tapping the "paid" link inside that thread.
Note: mid-test an Android "Allow notifications?" system permission dialog briefly intercepted taps (device/OS interstitial, not an App bug); it was dismissed and the flow continued normally. ScreenshotsHome → Recently added (paid expense visible) FAIL: tapping Recently added opens the "marked as paid" thread For contrast: the actual expense report (reached via in-thread paid link) |
|
🤖 Ran the testing steps from this PR on Web (dev NewDot). Outcome:
Recommend a clean re-run against a real registered second test account to clear the ScreenshotsHome → Recently added (paid expense visible) PASS: opening from Recently added shows the expense report detail view |
|
@ZhenjaHorbach @thelullabyy One of you needs to 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d737ff0e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@MelvinBot could you please unassigned me from this issue? |
|
Done — I removed you as an assignee from the linked deploy blocker, #99611. You were auto-assigned there as the author of #98611, but the investigation cleared that PR: the root cause was traced to the One thing I left alone: you're still a requested reviewer on this PR (#99668), which is a separate thing from the issue assignment. Reply if you want me to remove that too. |
|
@MelvinBot Please unassign me from this PR as well |
|
Done — you're off this PR too. I removed your review request, so you're no longer on the reviewer list here. Current state:
You're not assigned to either one now. |
|
Can we add test steps to OP so it's clear what QA needs to test when we get to that point? |
|
@MelvinBot |
|
Done — I filled in the Tests, Offline tests, and QA Steps sections of the PR description (and removed the The steps follow the repro from #99611 and add two regression checks that the guard in QA Steps (what QA will run on staging):
The Tests section adds a self-DM expense regression check, and Offline tests cover opening the paid expense from Recently added while offline (the target report is resolved locally from Onyx, so behavior should be identical). One thing worth flagging before QA: the two automated runs on this PR disagree. The Web run confirmed the fix (Recently added → expense report), but the Android run still reproduced the bug — tapping the card opened the "marked as paid" thread. Worth checking Android specifically when testing. |
|
I verified both review comments. Both are valid, and the Codex one is almost certainly why the Android test run still reproduced the bug while Web passed. The guard is in the wrong place — it only sees the snapshot action. The fix — re-apply the guard to the live action at const iouAction = getIOUActionForReportID(reportID, transaction.transactionID) ?? expense.reportAction;
// The live action can be a sent-money (pay) action too. An offline or optimistic expense is absent from
// the snapshot, so expense.reportAction is undefined and the guard above never sees it. Re-apply it here.
if (!iouAction || isSentMoneyReportAction(iouAction)) {
return reportID;
}Keep the existing guard at line 59 — line 66 returns early on the snapshot path, so control never reaches line 72 there. This also stops the unguarded Plus the CONSISTENCY-16 nit at One correction to my own PR descriptionThe description claims the transaction has "two IOU actions with the same Supporting detail: I can only make code changes when directed by the PR author, an assignee, or an existing co-author. You're a requested reviewer here, but the only PR assignee is |
|
@MelvinBot Tests
Offline tests
QA Steps
|
|
I updated the PR description. The Tests, Offline tests, and QA Steps sections now use your steps. The rest of the description is unchanged. |
|
@MelvinBot |
Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>
c7a9806
|
I added your changes and pushed them in I changed one line from your diff. ESLint's These checks pass: |
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
@grgia over to you |
|
@ZhenjaHorbach @grgia looks like the author checklist is failing |
|
@MelvinBot |
|
I updated the PR Author Checklist to match the current These new items are unchecked. Only you can confirm them, so please check them yourself:
The |
|
🚧 JS00001 has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.5.6-0 🚀
|
|
No help site update is needed: this is a bug fix that makes Recently added on Home behave the way users already expect, opening the paid expense itself and not showing a payment you sent as one of your own expenses. No sentence in Expensify Home Overview becomes incorrect. |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.5.6-6 🚀
Bundle Size Analysis (Sentry): |









Explanation of Change
When paying someone in a 1:1 DM and marking it as paid, the transaction has two IOU actions with the same
IOUTransactionID: the original money-request (CREATE) action, whosechildReportIDis the expense/transaction thread, and the PAY action, whosechildReportIDis the "marked as paid" system message thread. The Home "Recently added" slot resolves each expense's action viagetIOUActionForTransactionID, a first-match.find()that can return the PAY action.getReportIDToOpenForExpensethen navigated straight to that action'schildReportID, so opening the paid expense from "Recently added" opened the "marked as paid" message thread instead of the expense report.This adds an
isSentMoneyReportActionguard ingetReportIDToOpenForExpense: when the resolved action is a sent-money (pay) action, it navigates to the parent report (the expense report) rather than the pay action's message thread. This mirrors how the Search page routes single-transaction reports to the report itself. A unit test covers the new behavior.Fixed Issues
$ #99611
PROPOSAL: #99611 (comment)
Tests
25) > Next > confirm the details > Pay > Mark as paid.Offline tests
QA Steps
25) > Next > Pay > Mark as paid.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari