refactor: PureReportActionItem, add ModifiedExpenseContent and ReimbursementDeQueuedContent#87756
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfde41cb4e
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4896437263
ℹ️ 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".
| prevProps.isChronosReport === nextProps.isChronosReport && | ||
| prevProps.isClosedExpenseReportWithNoExpenses === nextProps.isClosedExpenseReportWithNoExpenses && | ||
| deepEqual(prevProps.missingPaymentMethod, nextProps.missingPaymentMethod) && | ||
| prevProps.reimbursementDeQueuedOrCanceledActionMessage === nextProps.reimbursementDeQueuedOrCanceledActionMessage && | ||
| prevProps.modifiedExpenseMessage === nextProps.modifiedExpenseMessage && | ||
| prevProps.userBillingFundID === nextProps.userBillingFundID && |
There was a problem hiding this comment.
Invalidate memo when extracted message inputs change
After extracting ModifiedExpenseContent and ReimbursementDeQueuedContent, the memo comparator no longer tracks the derived message props that previously forced updates for these action types. As a result, PureReportActionItem can now skip rerenders when values like currentUserEmail or report fields used by those messages change, leaving stale text/link state (for example, a modified-expense message can keep the help URL instead of switching to workspace rules once user/session data hydrates). Please include the underlying dependencies in the comparator (or keep passing derived message props) so these branches re-render when their inputs change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The props that bypass the memo (report?.ownerAccountID, report?.policyID, currentUserEmail) are immutable during the component's lifetime, they're set at report creation / login and don't change. The mutable data (policy, policyTags, movedFrom/ToReport) is handled by self-subscription inside the extracted components, which bypasses the memo entirely. The old string comparisons were only catching changes to these self-subscribing values, not to the immutable ones.
There was a problem hiding this comment.
Technically there is also another case of change which is the first loading of the data. There can be other cases that could cause a re-render in those circumstances but it might be a good idea to still add the necessary dependencies. WDYT @cristipaval
There was a problem hiding this comment.
Rather than re-add the prop/comparator entry, I pushed the fix to ModifiedExpenseContent itself to read it via useCurrentUserPersonalDetails() , the same pattern we use in other decomposed files. Context updates re-render the child directly when session.email hydrates. It will work well with other PRs too
e3354dd to
0e0a44b
Compare
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
0e0a44b to
95e063e
Compare
|
@cristipaval I am auto assigned here shall I review it? |
|
yes please, @FitseTLT . Thanks |
|
@FitseTLT Could you please take a look at this PR? |
|
I pinged him internally |
|
On it |
|
@LukasMod We are missing test steps for |
f5863c1 to
3f50c60
Compare
|
@FitseTLT Right, I added steps for Test 4 for ReimbursementDeQueuedContent, thanks! Unit tests inside PureReportActionItemTest, same as for other branches |
…to perf-send-message-phase-5
|
I pushed some |
Reviewer Checklist
Screenshots/Videos |
|
🚧 @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. |
|
🚀 Deployed to staging by https://github.com/cristipaval in version: 9.3.61-0 🚀
Bundle Size Analysis (Sentry): |
|
This PR is a purely internal refactoring — it extracts No help site changes are required. |





Explanation of Change
Phase 5 of splitting the PureReportActionItem component.
Extracted
ModifiedExpenseContentandReimbursementDeQueuedContentas self-subscribing components, removingusePolicyForMovingExpenses(which scans the entire policy collection), 3 useOnyx calls, and 2 derived computations fromReportActionItem. These now only mount for matching action types instead of firing for every visible item.This removes 2 props and 2 memo comparator entries from
PureReportActionItem, reducing per-item subscription cost by ~6 useOnyx calls across the render path.Fixed Issues
$ #87889
PROPOSAL:
Tests
Test 1: Modified Expense, Manual Field Edits
Steps:
Test 2: Modified Expense, Tag Change
Preconditions:
Steps:
Test 3: Moved Expense, Between Workspace Reports
Steps:
Test 4: Reimbursement Dequeued, Cancel Payment
Preconditions:
A workspace with manual reimbursement
An expense submitted and paid elsewhere / outside Expensify (so the "Cancel payment" option is available on the report)
Steps:
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
Screen.Recording.2026-04-14.at.14.34.29.mov
Android: mWeb Chrome
https://github.com/user-attachments/assets/ac3afe50-7c99-403f-be93-dc3cec4372f2

iOS: Native
https://github.com/user-attachments/assets/e675ecac-2226-47a6-b2a1-1a49a0f35327

iOS: mWeb Safari
Screen.Recording.2026-04-14.at.14.41.26.mov
MacOS: Chrome / Safari
Screen.Recording.2026-04-14.at.14.46.24.mov
Test 4
