Remove REPORT_ACTIONS Onyx reference from hasReportBeenForwardedSinceLastSubmit - 3 - #99817
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 890d9b9841
ℹ️ 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".
…or policy, report, and transaction initialization
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd8a30e9f8
ℹ️ 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".
ReviewOne change to make before merge, in
|
| Step | Status | Observation |
|---|---|---|
| Amount editable for submitter on non-forwarded report | pass | Opened the Amount edit screen with the field pre-filled at 1500.00 |
| Merchant editable for submitter on non-forwarded report | pass | Opened the Merchant edit screen pre-filled with the test merchant name |
| Date editable for submitter on non-forwarded report | pass | Opened the Date edit screen pre-filled with 2026-08-31 |
| Report is in the not-forwarded state | pass | Banner read "Outstanding — Waiting for you to approve expenses" right after submit, so still at the first approver |
| Expense More menu renders without error | pass | All 15 actions listed, no error text or blank dialog |
| No JS console errors | pass | No console capture exists for web in this agent-device build, so this wasn't asserted programmatically. No error banners, blank screens or crash UI appeared anywhere in the flow |
Evidence (3)
Amount edit screen opens from a submitted, not-forwarded expense report
Date edit screen opens from a submitted, not-forwarded expense report
Report More menu renders the full action list with no errors
RegressionsOne real regression, at the two call sites that pass a Both arrays are a paginated, display-filtered window of the report's actions. The fallback they replace read the complete Onyx entry. Where the window omits the The chain. Both trace back to Then at const reportActionsArray = Object.values(reportActions ?? allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`] ?? {});
Fix. Pass the full Onyx entry at both sites rather than the render array — Also worth noting: the new assertion at Call sites I checked and cleared
Smoke test — webPass, but it could not reach the affected state. The session is a single user who is both submitter and self-approver, so the report is never forwarded past a first approver and
Evidence (3)Selection toolbar with 2 expenses selected on an open report — Move to report and Edit multiple offered
Search page with 2 expenses selected — same options offered
More menu on the submitted single-expense report still listing Move to report
|
…levant hooks and utility functions for improved data access and clarity
…iRefactor expense action handling to utilize money request report IDs
|
Codex Review: Didn't find any major issues. Hooray! 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". |
ReviewNo blocking bugs. The One thing I'd change before merge. The
|
|
|
|
@situchan This is ready |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
ReviewNo blocking bugs. Head is still Keep the
|
Regression huntNo permissive regressions. I traced all nine migrated call sites against what the module-level fallback used to read, and every one passes a set that is equal to or a superset of it — so nothing that was blocked before becomes editable now. I did find one user-visible behavior change that isn't described anywhere in the PR, and one latent footgun. Search bulk edit gets newly blocked on forwarded reports — intended, but undocumented and untested by QAThree of the migrated sites feed the forwarded check the merged search snapshot ∪ Onyx action set, where before it only saw Onyx:
The snapshot really does carry That's the right outcome and the PR's own test covers it ( Edge worth testing by hand while you're there: Latent footgun:
|
| Site | Passed value | vs. old fallback |
|---|---|---|
MoneyRequestView (7 fields + canEdit) |
useOnyx(REPORT_ACTIONS + parentReportID), report is parentReport |
Identical key |
MoneyRequestReceiptView |
same, report is parentReport |
Identical key |
TransactionInlineEdit |
useOnyxWithoutSnapshots(REPORT_ACTIONS + effectiveParentReportID), report is effectiveParentReport |
Identical key, both branches |
canEditMultipleTransactions |
actionsForReport = snapshot ∪ Onyx |
Superset |
BulkEdit |
transactionReportActions from withSnapshotReportActions(...) |
Superset |
SearchEditMultiplePage |
reportActions from getTransactionEditContext(mergedReportActions, …) |
Superset |
useSelectedTransactionsActions |
allReportActions[REPORT_ACTIONS + moneyRequestReportID] |
Same collection, same derived ID |
Search-snapshot substitution — the main thing I tried to break it with. App's useOnyx replaces a value with the snapshot slice (it does not merge) when rendering inside SearchScopeProvider, and reportActions_ is a snapshot key. Any migrated site inside that scope would hand the check a subset and fail open. It doesn't happen: SearchScopeProvider only wraps the Search list in components/Search/index.tsx and EmptySearchView. useSelectedTransactionsActions is reached only from SelectionToolbar and MoneyReportHeaderSelectionDropdown, SearchEditMultiplePage is an RHP screen, and the one path that does run in search scope — useTransactionInlineEdit — already reads with useOnyxWithoutSnapshots. All get live Onyx.
No paginated view array reaches the check. useSelectedTransactionsActions holds a ReportAction[] prop from usePaginatedReportActions, and this PR deliberately does not pass it — :464 re-derives the report ID and reads the full collection instead. That's the correct choice (a paginated window can omit the FORWARDED action), and it's the reason there's no regression at the two sites I was most suspicious of.
Test changes don't mask anything. The useSelectedTransactionsActions.test.ts fixture fix (transactionID → IOUTransactionID) is required — getIOUActionForTransactionID reads IOUTransactionID, so the old fixture never resolved an action. The test mocks canEditFieldOfMoneyRequest and asserts only on arguments, so it can't hide a behavior change. MoneyRequestViewTest.tsx only adds merchant/date assertions to two existing cases.
CI is green on 5ec460c8 — typecheck, ESLint, React Compiler, all 8 shards, perf. Only Check independent approval is red, which is a process gate.
Browser verification
I couldn't run any of it. The prestarted web session's test account has no workspaces and no expenses at all (Workspaces → "No workspaces yet", Spend → Expenses → "No expenses yet"), so there was nothing to open, inline-edit, multi-select, or bulk-edit. The findings above are static only.
The scenario most worth running by hand is the one that actually changed: as a corporate submitter, submit a report, have the first approver forward it, hard-refresh onto Search, then check whether "Edit multiple" and the per-field edit affordances behave as you intend.
|
Please pull main |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
@cretadn22 could you resolve the conflict? Thanks! |
…ts to handle undefined rules parameter
|
@mollfpr Resolved all conflicts |
|
The failing test is existing from main and will be resolved via #101016 |
|
@mollfpr Resolved all conflicts |
|
@mollfpr Does everything look good to go? If so, could we get this merged soon to prevent any conflicts? |
|
🚧 mollfpr 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/mollfpr in version: 9.4.81-0 🚀
|
|
🤖 No help site changes are required for this PR. I did not create a docs PR. Why: this PR is an internal Onyx refactor with no user-facing change. It threads Nothing in the diff adds, removes, or renames a feature, setting, tab, button, or any user-facing copy, so no article under What I checkedSource changes (all signature/plumbing only):
Test changes: the added assertions in No copy changes: the diff touches no Docs scan: I checked No docs PR was created, so there is nothing for you to review on the help site side. If you think a specific article should describe which expense fields stay editable after a report is forwarded, reply with |
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.4.81-1 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.4.81-1 🚀
|



Explanation of Change
Fixed Issues
$ #66419
PROPOSAL: #66419
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
Screen.Recording.2026-09-04.at.13.15.37.mov