Repository navigation
Fetch policy categories and tags on demand in shared pickers - #99125
Conversation
High-traffic accounts load Onyx lazily, so on a fresh sign-in the policyCategories/policyTags collections can be missing or contain only the value already on the expense, leaving pickers stuck showing just the selected item across inline cells, bulk edit, and the IOU tag step. Add useLoadPolicyCategories/useLoadPolicyTags hooks that backfill the full list on demand (once per policyID per session, deduped at module level since API.read does not dedupe reads) and wire them into the shared CategoryPicker and TagPicker so every consumer benefits. Show the list loading state while the fetch is in flight instead of flashing the selected-only fallback. Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
✅ Web test results — PASSRan the testing steps on the dev NewDot web build against this PR's code (branch What I did: created a workspace, enabled Categories (full default list) and Tags (added Engineering, Marketing, Operations, Sales), created a €25.00 expense on it, then opened the expense and drove both pickers before and after selecting a value.
Both pickers display the full workspace list rather than just the selected item — confirming the on-demand fetch works. Caveats / notes
|
EvidenceCategory picker showing full workspace category list before selectionCategory picker with Professional Services selected (checked, pinned top) and full list still shown belowTag picker showing all 4 workspace tags before selectionTag picker with Marketing selected (checked, pinned top) and full list still shown below |
|
No code fix is needed — the checks that were pending have all passed, and the only remaining failing check is expected. Current status: The only failing check is When you first pinged me, the substantive checks (
So there's nothing broken to push a fix for — the PR just needs an independent reviewer approval to clear |
✅ Android test results — PASSRan the testing steps on the standalone NewDot Android build (emulator) against this PR's code (branch What I did: created a workspace with Categories (full default list) + Tags (Engineering, Marketing, Sales, Travel), created and submitted a cash expense on it (Category = Professional Services, Tag = Sales), then reopened the persisted expense and drove both pickers.
Both pickers display the full workspace list rather than just the selected item — confirming the on-demand fetch works on Android. CaveatThe original bug reproduces specifically on a fresh sign-in with a high-traffic account (where Onyx loads Screenshots attached in the following comment. |
Evidence (Android)Category picker showing full list on new (unsaved) expenseTag picker showing full list (4 tags) on new (unsaved) expenseCategory picker showing full list plus selected category on reopened/persisted expenseTag picker showing full list plus selected tag on reopened/persisted expense |
|
When the categories are not loaded and the expense has a missing category, the inline edit icon won't show. Not sure if that's considered a bug or not. w.mp4 |
|
Please merge main and fix this issue @MelvinBot |
Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
…unloaded The inline edit permission gated canEditCategory on hasEnabledOptions(policyCategories), so an expense with a missing category on a lazy-loaded account had no edit icon: the collection was absent, the icon stayed hidden, the CategoryPicker never mounted, and the on-demand fetch it now performs never ran. Treat "categories enabled on the policy but the collection has never loaded" as unknown rather than empty and keep the cell editable, matching the existing loading treatment in DynamicIOURequestStepCategory. A loaded-but-empty collection still disables editing. Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
Merged Root cause: Fix: an absent collection only tells us the account hasn't fetched it yet — it isn't the same as an empty one. So when the policy has categories enabled but Details, scope decisions, and verificationChange — const areCategoriesEnabledButNotLoaded = !!policy?.areCategoriesEnabled && policyCategories === undefined;It's Tests — added three cases to One existing test ( Two things I deliberately did NOT change — flagging rather than deciding:
Checks run locally: Web verification did not run. The CI browser session went permanently blank — the rsbuild dev server restarted about 10 minutes after the tab had loaded, and the already-open tab never reconnected (0 accessibility nodes, blank screenshots, no network activity). That's test infra, not this branch; the tester correctly refused to reload, since that would drop auth. So the change is covered by unit tests only, and a manual hover-the-empty-Category-cell pass on a high-traffic account is still worth doing.
|
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.
|
…es lint rule Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
Fixed the failing check — ESLint check is now green (pushed as Root cause: the previous commit ( Fix: renamed it to DetailsThe rule ( Verified locally before pushing:
Confirmed green in CI after the push: ESLint, typecheck, spellcheck, Oxfmt, React Compiler Compliance, and all 8 Jest jobs. Still red: |
|
Please merge main, replace all |
Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
@thelullabyy 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] |
|
Done — all three steps:
Re-ran the local checks after the merge, since Two things you should look at before a reviewer does, because the now-checked boxes assert them and they aren't true yet:
The only failing check remains |
|
I merged the latest Git merged it without any manual conflict resolution. The PR's 15 changed files are unchanged. TypeScript typecheck passes. I couldn't run ESLint or Jest here because of local environment limits, so CI will cover those. |
|
@MelvinBot ESLint check is failed |
Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
Fixed the failing ESLint check in c17b681. ESLint flagged ESLint and typecheck now pass locally on that file. I couldn't run Jest here, so CI will confirm the test. |
|
@Valforte I think you can continue your review now |
|
@MelvinBot Please resolve conflicts and merge main |
|
@Valforte Friendly bump in case you missed this one |
…egoriesTagsOnDemand # Conflicts: # src/components/CategoryPicker/index.tsx
|
Merged Conflict: Local checksI couldn't run typecheck, lint, or Jest locally. Package install scripts and patches don't run in my sandbox, so those tools fail before they reach the code. CI will run the full checks on this push. |
|
@thelullabyy can you please approve again so that melvin requests a different approver? |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #98838 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
🚧 carlosmiceli 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/carlosmiceli in version: 9.5.3-0 🚀
|
|
@thelullabyy @bernhardoj What should QA team to test in this PR? |
|
No help site update needed. This is a bug fix: category and tag pickers now show the workspace's full list instead of only the selected value, which is what users already expect. |
|
@kavimuru Could you please follow this issue steps and verify that it is no longer reproducible? |
|
@thelullabyy Do you mean this issue #98838?
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.5.3-4 🚀
Bundle Size Analysis (Sentry): |








Explanation of Change
High-traffic accounts load Onyx data lazily, so on a fresh sign-in the
policyCategories/policyTagscollections for a workspace can be missing entirely or contain only the value already on the expense. Every picker surface that reads those collections from Onyx without fetching them then falls through to a synthesized "selected-only" list — so the picker shows just the currently-selected category/tag instead of the full workspace list. This is one shared pattern reproduced across several flows (inline category/tag pickers on report/search rows, the Search bulk-edit category/tag pages, and the IOU tag step); only the full-page IOU category step fetched on its own today.This PR fixes the pattern in one place rather than per-surface:
useLoadPolicyCategoriesanduseLoadPolicyTags— that backfill a policy's categories/tags on demand (via the existinggetPolicyCategories/openPolicyTagsPagereads).CategoryPickerandTagPicker, so every consumer inherits the fetch at once.Setof already-requested policyIDs) becauseAPI.readdoes not dedupe in-flight reads, so multiple picker instances for the same policy won't fire redundant requests.Scope note: the Search filter Tag selector (
TagSelector) and a truly narrow per-transaction tag read are intentionally out of scope here — the tag search-filter has no equivalent read command and the only client tag read is the broadOpenPolicyTagsPage; closing those requires backend work and should be tracked separately.Fixed Issues
$ #98838
PROPOSAL: #98838 (comment)
Tests
// TODO: The human co-author must fill out the manual test steps before marking this PR "Ready for review".
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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