[Home Page] Fix: Your spend section does not update after updating expense offline V2 - Greyed out#96966
Conversation
adamgrzybowski
changed the title
|
@ZhenjaHorbach 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] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e38e04291
ℹ️ 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".
| WRITE_COMMANDS.APPROVE_MONEY_REQUEST, | ||
| WRITE_COMMANDS.UNAPPROVE_EXPENSE_REPORT, | ||
| ]); | ||
| const YOUR_SPEND_PAYMENT_COMMANDS = new Set<string>([WRITE_COMMANDS.PAY_MONEY_REQUEST, WRITE_COMMANDS.PAY_MONEY_REQUEST_WITH_WALLET, WRITE_COMMANDS.CANCEL_PAYMENT]); |
There was a problem hiding this comment.
When the submitter uses the Received payment secondary action while offline, markReportPaymentReceived queues WRITE_COMMANDS.MARK_REPORT_PAYMENT_RECEIVED with reportID and optimistically moves the report to REIMBURSED, so it enters the status:paid query. This set filters that request out, and the projection only treats pendingFields.total as an amount change, so isPaymentStale stays false and the Repaid row shows an ungreyed stale snapshot until reconnect. Add MARK_REPORT_PAYMENT_RECEIVED to the payment bucket.
Useful? React with 👍 / 👎.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // refresh the snapshots, so we grey only the affected total to signal it may | ||
| // be stale rather than showing a value we know might be wrong. | ||
| const [reportsProjection] = useOnyx(ONYXKEYS.COLLECTION.REPORT, { | ||
| selector: (reports) => projectYourSpendReports(reports, paidGroupPolicyIDs, accountID), |
There was a problem hiding this comment.
❌ PERF-11 (docs)
This useOnyx selector on the REPORT collection returns projectYourSpendReports(...), whose statusByID field is a Record<string, number> that grows with the number of the user's paid-group reports. Onyx runs deepEqual on this output on every REPORT collection change (a very hot key), and the result is an intermediate structure that getYourSpendPendingBuckets only reduces further. On a high-traffic account this is an expensive deepEqual with no re-render savings — exactly the anti-pattern this rule warns about.
Mirror the sibling outstandingReportsSignature selector above, which reduces to a compact primitive. Compute the final booleans in the selector (or a compact string signature) rather than returning a growing statusByID map, e.g.:
const [reportsProjection] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {
// return a small, stable signature/booleans instead of a per-report Record
selector: (reports) => projectYourSpendReportsSignature(reports, paidGroupPolicyIDs, accountID),
});Reviewed at: 4e38e04 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| }); | ||
| const [queuedSpendRequests] = useOnyx(ONYXKEYS.PERSISTED_REQUESTS, { | ||
| selector: (requests) => | ||
| (requests ?? []).filter((request) => !!request?.command && (YOUR_SPEND_APPROVAL_COMMANDS.has(request.command) || YOUR_SPEND_PAYMENT_COMMANDS.has(request.command))), |
There was a problem hiding this comment.
❌ PERF-11 (docs)
This selector on PERSISTED_REQUESTS returns a filtered array of full AnyRequest objects. Each request carries large nested payloads (optimisticData, successData, failureData, finallyData, etc.), so Onyx's deepEqual on this array runs a deep comparison over every field of every matching request on every queue mutation. This is the "filters/maps a collection into an array — deepEqual on every item" case the rule flags.
Project each request down to only the fields consumed downstream (command and the report ID) so the compared output is small:
const [queuedSpendRequests] = useOnyx(ONYXKEYS.PERSISTED_REQUESTS, {
selector: (requests) =>
(requests ?? [])
.filter((r) => !!r?.command && (YOUR_SPEND_APPROVAL_COMMANDS.has(r.command) || YOUR_SPEND_PAYMENT_COMMANDS.has(r.command)))
.map((r) => ({command: r.command, reportID: r.data?.reportID ?? r.data?.iouReportID})),
});(and adjust getYourSpendPendingBuckets to read the projected shape).
Reviewed at: 4e38e04 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| // title width across stacked rows, mirroring `ForYouSkeleton`. | ||
| skeletonRowIndex: number; | ||
| // Greys the total when a queued offline change may have made it stale. | ||
| isStale?: boolean; |
There was a problem hiding this comment.
❌ CONSISTENCY-10 (docs)
The new isStale prop on SpendSummaryRowProps is documented with a // line comment. Per STYLE.md, component props must be documented with a /** ... */ block comment above the member, not a // comment.
/** Greys the total when a queued offline change may have made it stale. */
isStale?: boolean;Reviewed at: 4e38e04 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
🚧 grgia 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! 🧪🧪
|
1 participant
Explanation of Change
This PR is created as a replacement for a more complicated solution implemented here #93081
Fixed Issues
$ #91370
PROPOSAL:
Tests
Offline tests
QA Steps
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
Screen.Recording.2026-07-24.at.15.41.22.mov