[No QA] Fix home Awaiting approval count to exclude IOU and personal-policy expenses#91847
[No QA] Fix home Awaiting approval count to exclude IOU and personal-policy expenses#91847mountiny wants to merge 7 commits into
Conversation
…xpenses The Home widget search query for "Awaiting approval" used type:expense which the backend expands to include IOU reports. Scope the query to the user's policies that pass hasApprovalFlow by adding a policyID filter, matching the same predicate the row's visibility gate already uses. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@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] |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR fixes the Home “Awaiting approval” widget so its count/total only reflects outstanding reimbursable expenses from workspaces that actually have an approval flow, excluding IOU (DM) expenses and personal-policy expenses. It aligns the widget’s underlying Search query with the same policy applicability predicate already used to decide whether the row should be shown.
Changes:
- Updated
buildAwaitingApprovalQueryto optionally scope results bypolicyID:[...]when approval-flow workspace IDs are available. - Refactored
getYourSpendApplicabilityto compute a sortedapprovalPolicyIDslist (stable hash) alongside approval/payment applicability. - Added/updated unit tests to cover the new query scoping behavior and the sorted policy ID plumbing through
useYourSpendData.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/pages/home/YourSpendSection/queries.ts |
Adds optional policyID filter to the awaiting-approval Search query. |
src/pages/home/YourSpendSection/useYourSpendData.ts |
Computes sorted approval-policy IDs and uses them to build the awaiting-approval query + effect dependency key. |
tests/unit/Search/yourSpendQueryBuildersTest.ts |
Adds coverage for policyID emission/omission and hash changes when scoped. |
tests/unit/Search/yourSpendApplicabilityTest.ts |
Adds coverage for approvalPolicyIDs derivation/sorting and applicability behavior. |
tests/unit/HomePage/YourSpendSection/useYourSpendDataTest.ts |
Verifies the hook passes the expected sorted policy IDs into the query builder. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ 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". |
Replace the hasApprovalFlow predicate with isGroupPolicy so the policyID filter includes any non-personal workspace (Team/Corporate/Submit). The count now matches the user's mental model: OUTSTANDING reports they own on a group workspace, regardless of whether that workspace has an approval workflow configured. Rename approvalPolicyIDs to groupPolicyIDs to reflect the broader predicate, and update tests accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>
Switch the predicate from isGroupPolicy back to isPaidGroupPolicy so the policyID filter only includes Team and Corporate workspaces. Submit workspaces are intentionally excluded from this PR even though they default to ADVANCED approval mode — to be revisited separately once the UX has been confirmed. Rename groupPolicyIDs to paidGroupPolicyIDs to reflect the tighter scope, and update the applicability and hook tests accordingly. The paymentRowState tests now explicitly opt the corporate fixture into isPaidGroupPolicy since the helper is mocked at the module level. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
The sort was defensive against unrelated Onyx mutations rearranging the policy collection, but Onyx preserves insertion order in practice, so the snapshot key churn it was guarding against doesn't happen. Trim the verbose comments on YourSpendApplicability, the applicabilityKey, and the policyID filter in queries.ts down to one-liners in plain English. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de47e2b6f5
ℹ️ 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".
When the user's paid-workspace set changed, the approval search query hash changed but cachedApprovalReady wasn't keyed by hash, so a fresh snapshot with count still missing would briefly fall back to the previous workspace set's total. Cache the hash alongside the totals and only reuse the cache when the current query hash matches. Add a regression test that fails without the fix. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@MelvinBot Please review the PR |
Co-authored-by: Cursor <cursoragent@cursor.com>
PR ReviewThe fix is well-motivated and the approach (scoping by 1.
|
Explanation of Change
The Home page "Awaiting approval" widget was counting IOU expenses (DM money requests) and any personal-policy expenses in its total/count. Repro per the issue:
Root cause:
buildAwaitingApprovalQueryinsrc/pages/home/YourSpendSection/queries.tsissued atype:expense status:outstanding from:{accountID} reimbursable:yesquery. The Auth backend explicitly includes IOU reports undertype:expense(seeSearchExpenseUtils.cpp:419), so any IOU report in OUTSTANDING state where the user is the submitter fed the count and total.Fix: scope the query to the user's paid group policies (Team and Corporate, via the existing
isPaidGroupPolicyhelper) by adding apolicyID:[...]filter. The filter naturally excludes:policyID,POLICY.TYPE.PERSONALis not a paid group policy,policyIDis the Workspace chip in the Search UI, so when the user taps the row they land on a Search page with that chip pre-filled and the result set matches the count exactly.Implementation details:
getYourSpendApplicabilitynow does a single pass overpoliciesto computeisApprovalApplicable,isPaymentApplicable, and a sortedpaidGroupPolicyIDs: string[]. The sort keeps the resulting query hash stable across unrelated Onyx key shuffles.isApprovalApplicableis derived frompaidGroupPolicyIDs.length > 0, andisPaymentApplicablekeeps its existing meaning (a paid group policy with payments enabled).useYourSpendDatareadspoliciesbefore building the approval query and passespaidGroupPolicyIDsintobuildAwaitingApprovalQuery.applicabilityKeynow incorporates the sorted IDs so the snapshot re-fetches when the user joins/leaves a workspace.Alternatives considered and rejected:
type:expense-reportdrops the intentionalreimbursable:yes(not allowed for that data type and the backend'stransactionListCTE has no per-transaction reimbursable filter).reportTypefilter would be a phantom key not exposed anywhere in the Search UI.searchKeyto opt into the accounting "exclude IOU" SQL path causes a snapshot-cache race between the Home widget and the Search page (snapshot hash doesn't includesearchKey).action:approveproduces the correct SQL when combined withfrom:, but the "Action: Approve" chip on tap-through misreads as "things I need to approve".hasApprovalFlow(an earlier iteration) was too restrictive — it would have hidden the row even when the user has outstanding reports on a paid group workspace whoseapprovalModeis OPTIONAL or unset.isGroupPolicy(Team/Corporate/Submit) was considered but Submit workspaces are deferred to a follow-up.Fixed Issues
$ #91392
PROPOSAL: Internal fix — no external proposal
Tests
Automated coverage:
npx jest tests/unit/Search/yourSpendQueryBuildersTest.ts tests/unit/Search/yourSpendApplicabilityTest.ts tests/unit/HomePage/YourSpendSection/useYourSpendDataTest.ts(67 tests pass, including the newpolicyIDandpaidGroupPolicyIDscases).Verify that no errors appear in the JS console
Offline tests
N/A — no offline-specific behavior changed. The widget already short-circuits the search when offline and reuses cached totals (existing behavior unchanged by this PR).
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
N/A — no UI changes; query-shape fix only. Screenshots can be added during QA verification on a device build.
Android: mWeb Chrome
N/A — no UI changes; query-shape fix only.
iOS: Native
N/A — no UI changes; query-shape fix only. Screenshots can be added during QA verification on a device build.
iOS: mWeb Safari
N/A — no UI changes; query-shape fix only.
MacOS: Chrome / Safari
N/A — no UI changes; query-shape fix only. The Home widget will visually look identical; only the count/total it shows changes (IOU and personal-policy expenses no longer included).