Skip to content

[No QA] Fix home Awaiting approval count to exclude IOU and personal-policy expenses#91847

Open
mountiny wants to merge 7 commits into
mainfrom
vit-fix-awaiting-approval-iou-91392
Open

[No QA] Fix home Awaiting approval count to exclude IOU and personal-policy expenses#91847
mountiny wants to merge 7 commits into
mainfrom
vit-fix-awaiting-approval-iou-91392

Conversation

@mountiny
Copy link
Copy Markdown
Contributor

@mountiny mountiny commented May 27, 2026

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:

  1. New account, no workspace created yet.
  2. Submit an expense to a user in a DM (creates an IOU).
  3. Home → "Awaiting approval" not shown (applicability gate is off).
  4. Create a workspace.
  5. Home → "Awaiting approval" appears, but the total includes the prior IOU.

Root cause: buildAwaitingApprovalQuery in src/pages/home/YourSpendSection/queries.ts issued a type:expense status:outstanding from:{accountID} reimbursable:yes query. The Auth backend explicitly includes IOU reports under type:expense (see SearchExpenseUtils.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 isPaidGroupPolicy helper) by adding a policyID:[...] filter. The filter naturally excludes:

  • IOU reports — no policyID,
  • personal-policy expensesPOLICY.TYPE.PERSONAL is not a paid group policy,
  • Submit-only policies — intentionally excluded from this PR to be revisited separately, even though they default to ADVANCED approval mode.

policyID is 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:

  • getYourSpendApplicability now does a single pass over policies to compute isApprovalApplicable, isPaymentApplicable, and a sorted paidGroupPolicyIDs: string[]. The sort keeps the resulting query hash stable across unrelated Onyx key shuffles. isApprovalApplicable is derived from paidGroupPolicyIDs.length > 0, and isPaymentApplicable keeps its existing meaning (a paid group policy with payments enabled).
  • useYourSpendData reads policies before building the approval query and passes paidGroupPolicyIDs into buildAwaitingApprovalQuery.
  • The search-firing effect's applicabilityKey now incorporates the sorted IDs so the snapshot re-fetches when the user joins/leaves a workspace.

Alternatives considered and rejected:

  • Switching to type:expense-report drops the intentional reimbursable:yes (not allowed for that data type and the backend's transactionList CTE has no per-transaction reimbursable filter).
  • A new reportType filter would be a phantom key not exposed anywhere in the Search UI.
  • Using searchKey to 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 include searchKey).
  • Reusing action:approve produces the correct SQL when combined with from:, but the "Action: Approve" chip on tap-through misreads as "things I need to approve".
  • Scoping by 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 whose approvalMode is OPTIONAL or unset.
  • Scoping by 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

  1. Sign in to a fresh account; do not create a workspace yet.
  2. Open a DM with any other user and submit an expense to them (creates an IOU).
  3. Go to Home → confirm "Awaiting approval" is not shown (applicability gate, unchanged).
  4. Create a workspace.
  5. Go to Home → confirm "Awaiting approval" now appears, and its total/count reflect only the new workspace's outstanding expense reports (currently 0). The prior IOU should not be counted.
  6. Submit an expense in the new workspace; confirm Home updates to track that expense's amount and that the IOU still does not contribute.
  7. Tap the "Awaiting approval" row → confirm the Search page renders with the Workspace filter chip pre-filled with the new workspace, and the result count matches the Home widget.

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 new policyID and paidGroupPolicyIDs cases).

  • 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

  1. On staging, sign in as a user who has both an outstanding IOU expense (submitted in a DM) and a paid group workspace (Team or Corporate) they belong to.
  2. Open Home → "Awaiting approval" total/count should reflect only the workspace's outstanding expense reports; the IOU should not be included.
  3. Tap the "Awaiting approval" row → the Search page should open with the Workspace filter chip pre-filled, and the result count should match the Home widget.
  4. Confirm no regression for a user with only a personal workspace: the "Awaiting approval" row should remain hidden.
  5. Confirm no regression for a user with multiple paid group workspaces: the count should sum across all of them.
  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

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).

…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>
@mountiny mountiny requested review from a team as code owners May 27, 2026 14:10
@melvin-bot melvin-bot Bot requested review from ZhenjaHorbach and flaviadefaria and removed request for a team May 27, 2026 14:10
@melvin-bot
Copy link
Copy Markdown

melvin-bot Bot commented May 27, 2026

@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]

@mountiny mountiny requested a review from Copilot May 27, 2026 14:10
@mountiny
Copy link
Copy Markdown
Contributor Author

@codex review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 buildAwaitingApprovalQuery to optionally scope results by policyID:[...] when approval-flow workspace IDs are available.
  • Refactored getYourSpendApplicability to compute a sorted approvalPolicyIDs list (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.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

ℹ️ 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".

mountiny and others added 2 commits May 27, 2026 16:28
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
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/pages/home/YourSpendSection/queries.ts 100.00% <100.00%> (ø)
...rc/pages/home/YourSpendSection/useYourSpendData.ts 97.45% <100.00%> (+0.13%) ⬆️
... and 48 files with indirect coverage changes

mountiny and others added 2 commits May 27, 2026 17:54
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>
@mountiny mountiny requested a review from Copilot May 27, 2026 16:16
@mountiny
Copy link
Copy Markdown
Contributor Author

@codex review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread src/pages/home/YourSpendSection/useYourSpendData.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/pages/home/YourSpendSection/useYourSpendData.ts
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>
@mountiny
Copy link
Copy Markdown
Contributor Author

@MelvinBot Please review the PR

Co-authored-by: Cursor <cursoragent@cursor.com>
@MelvinBot
Copy link
Copy Markdown
Contributor

PR Review

The fix is well-motivated and the approach (scoping by policyID) is sound. The cache-keying-by-hash fix and test coverage are solid. Two items to address:

1. paidGroupPolicyIDs is not sorted — PR description says it is

The PR body states "The sort keeps the resulting query hash stable across unrelated Onyx key shuffles", but getYourSpendApplicability never sorts the array — it pushes IDs in Object.values() iteration order. I see the reply to Copilot saying this is intentional, but the PR body still claims otherwise.

Object.values() order for non-integer string keys follows insertion order, which in Onyx depends on merge/set timing. If a background Onyx merge re-writes the collection in a different order (e.g. a push notification updates one policy, causing it to be re-inserted), the IDs array changes → different query string → different hash → cache miss + unnecessary re-fetch. Adding .sort() is a one-liner that eliminates this class of bug:

-    return {
-        isApprovalApplicable: paidGroupPolicyIDs.length > 0,
+    paidGroupPolicyIDs.sort();
+    return {
+        isApprovalApplicable: paidGroupPolicyIDs.length > 0,

At minimum, please align the PR description with the code (remove the sort claim), but I'd recommend just adding the sort.

2. type CachedReady defined inside the render function body (nit)

Line 253 defines type CachedReady inside useYourSpendData. TypeScript erases it at runtime so there's no perf impact, but it's unconventional — types inside function bodies can confuse readers and aren't hoisted. Consider moving it near YourSpendRowTotals at the module level.

Everything else looks good

  • The policyID filter in buildAwaitingApprovalQuery correctly excludes IOUs (no policyID) and personal-policy expenses.
  • Switching from hasApprovalFlowisPaidGroupPolicy for the applicability gate makes sense — outstanding reimbursable expenses on paid group workspaces with OPTIONAL approval mode should still appear.
  • Cache keyed by approvalQueryJSON.hash prevents stale totals from flashing when the workspace set changes. The regression test validates this.
  • Test coverage is comprehensive (67 tests including the new policyID, paidGroupPolicyIDs, and cache-hash cases).

Next Steps: Reply with @MelvinBot <your feedback> to discuss further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants