[Home Page] Fix: Your spend section does not update after updating expense offline#93081
Conversation
Patch awaiting-approval and repaid snapshot search.total aggregates optimistically when money request amounts change, since Home cannot refetch those totals while offline.
Import Your spend query builders directly from @libs/YourSpendQueryUtils instead of a page-local re-export barrel, giving a single source of truth.
|
@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!
|
|
@adamgrzybowski lint |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
Replace unsafe type assertions with a typed createRandomPolicy factory and typed getSnapshotKey/buildSnapshotSearchResults helpers.
|
@grgia fixed |
|
Looks like changes don't work if we reject submitted expenses 2026-06-11.14.48.04.mov |
|
@ZhenjaHorbach Yeah, the question is how big the scope should be for this PR and which actions should be supported. This issue is also mentioned here #93081 (comment) |
I believe all actions should be supported, and the amount should be the same as after the response from BE |
|
@ZhenjaHorbach do you know exactly which actions?
Something else? |
I suppose in the context of this flow, at least retract, unapprove, reject |
|
This PR adds a new |
- Drop removed transactions from snapshot.data so the Search page stays in sync offline - Mirror only each snapshot's search aggregates to keep memory bounded - Align 30-day boundary checks with the exclusive date> filter semantics
| let allPolicies: OnyxCollection<Policy> = {}; | ||
| Onyx.connectWithoutView({ | ||
| key: ONYXKEYS.COLLECTION.POLICY, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
| allPolicies = value ?? {}; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
I think that instead of adding this one here again, maybe we could reuse what is already in the ReportUtils - getPolicy and getAllPolicies, so we do not add another subscription to policies collection in another file. Curious what @tgolen @luacmartins thinks
There was a problem hiding this comment.
Well, ReportUtils is being refactored to remove those eventually, so even that's not a good pattern to follow. Why can't these just be passed from the view component?
There was a problem hiding this comment.
Yeah that is fair, it would just create sort of a singleton pattern that I am not sure if we want.
Why can't these just be passed from the view component?
There was a problem hiding this comment.
These functions run in the IOU action layer (delete/submit/reject/pay/split), not in a component. They are triggered from expense screens when Home isn't always mounted
I am looking for an alternative solution, but so far I don't see a solution without tradeoffs
There was a problem hiding this comment.
It's a pretty hard requirement, yes.
These functions run in the IOU action layer (delete/submit/reject/pay/split), not in a component.
But they are called by a component somewhere up the chain, aren't they? Otherwise, what calls these functions?
They are triggered from expense screens when Home isn't always mounted
How can they be triggered from a screen when Home isn't mounted?
There was a problem hiding this comment.
Whichever component is triggering the actions, there you can subscribe to the policies and thread them through the functions @adamgrzybowski
There was a problem hiding this comment.
How can they be triggered from a screen when Home isn't mounted?
What I meant here is that these actions are triggered by many different components (5 hooks and 8 components), so it's not as straightforward as passing these props from the home component.
However, I understand that we want to get rid of the connectWithoutView.
I will create a commit with the change, so we can judge if this approach is good enough by looking at the diff.
There was a problem hiding this comment.
Yeah that is expected, we need to pull the data from the views
tgolen
left a comment
There was a problem hiding this comment.
I have a few general concerns about this PR. It's a massive fix for the issue that was originally reported.
- Are we sure this is the best fix?
- Are we sure that it needs to be fixed and we can't live with the original problem?
| let allPolicies: OnyxCollection<Policy> = {}; | ||
| Onyx.connectWithoutView({ | ||
| key: ONYXKEYS.COLLECTION.POLICY, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
| allPolicies = value ?? {}; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Well, ReportUtils is being refactored to remove those eventually, so even that's not a good pattern to follow. Why can't these just be passed from the view component?
Split updates now diff reimbursable portions only, and toggling an expense's reimbursable flag offline patches the Your spend totals.
Cover getYourSpendSnapshotReimbursableUpdates in both directions and the getUpdateMoneyRequestParams branch that removes an expense toggled non-reimbursable.
|
I fixed problems mentioned by @grgia, and now I am looking for alternative solutions that don't require |
|
I removed the |
mountiny
left a comment
There was a problem hiding this comment.
Overall lets make sure every new or update utils method or action has appropriate test here
| /** Supplies the paid-group policies and "Your spend" snapshot aggregates the offline snapshot-patch builders need. */ | ||
| function useYourSpendPatchData(): YourSpendPatchData { | ||
| const {accountID} = useCurrentUserPersonalDetails(); | ||
| const [paidPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: selectPaidGroupPolicies}); |
There was a problem hiding this comment.
Scrolling large Search results and any policy-collection update now do extra per-row Onyx work, which bites on accounts with many workspaces.
could we mount useYourSpendPatchData once at the Search page level and pass it to rows via the existing SearchContext?
or narrow selectPaidGroupPolicies to the fields the builders actually use ({id, type, outputCurrency})
| const currentTotal = search.total ?? 0; | ||
| const currentCount = search.count ?? 0; | ||
|
|
||
| if (currentCurrency && currentCurrency !== currency) { | ||
| return {optimisticData: [], successData: [], failureData: []}; | ||
| } | ||
|
|
||
| const updatedTotal = currentTotal + diff; |
There was a problem hiding this comment.
Deleting or rejecting several expenses at once while offline leaves the Home Awaiting approval total wrong (only one expense subtracted) until reconnect.
Could we do a snapshot update per batch instead of one per transaction?
…offline-your-spend # Conflicts: # src/components/MoneyReportHeaderActions/MoneyReportHeaderSecondaryActions.tsx # src/components/MoneyReportHeaderPrimaryAction/useConfirmApproval.ts # src/components/MoneyRequestHeaderSecondaryActions.tsx # src/components/ReportActionItem/MoneyRequestReportPreview/ApproveActionButton.tsx # src/components/ReportActionItem/MoneyRequestReportPreview/PayActionButton.tsx # src/components/ReportActionItem/MoneyRequestReportPreview/SubmitActionButton.tsx # src/components/ReportActionItem/MoneyRequestView.tsx # src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx # src/components/Search/SearchList/ListItem/ReportListItemHeader.tsx # src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx # src/hooks/useDeleteTransactions.ts # src/hooks/useHoldMenuSubmit.ts # src/hooks/useLifecycleActions.tsx # src/hooks/useSearchBulkActions.ts # src/hooks/useSelectionModePayment.ts # src/hooks/useTransactionInlineEdit.ts # src/libs/IOUAmountSubmission.ts # src/libs/PaymentUtils.ts # src/libs/actions/IOU/DeleteMoneyRequest.ts # src/libs/actions/IOU/PayMoneyRequest.ts # src/libs/actions/IOU/RejectMoneyRequest.ts # src/libs/actions/IOU/ReportWorkflow.ts # src/libs/actions/IOU/SplitTransactionUpdate.ts # src/libs/actions/IOU/TrackExpense.ts # src/libs/actions/IOU/UpdateMoneyRequest.ts # src/libs/actions/Search.ts # src/pages/DynamicReportDetailsPage.tsx # src/pages/RejectExpenseReportPage.tsx # src/pages/home/YourSpendSection/useYourSpendData.ts # src/pages/inbox/report/ContextMenu/PopoverReportActionContextMenu.tsx # src/pages/iou/SplitExpensePage.tsx
Explanation of Change
Fixed Issues
$ #91370
PROPOSAL:
Tests
Offline tests
QA Steps
Prerequisites: a paid group workspace (Team/Corporate) where you are a member, and a few of your own reimbursable expenses. Each suite is run while offline (disable your network connection), starting from Home where you note the current "Your spend" totals.
Suite 1 - Awaiting approval increases
Suite 2 - Awaiting approval decreases
Suite 3 - Repaid (last 30 days)
Suite 4 - Rollback and reconciliation
Suite 5 - Negative checks (totals must not change)
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-06-09.at.16.05.31.mov