Conversation
Guard the upgrade path on policy hydration, let a valid active workspace rescue the user before the upgrade early return, and pass the current user login so the employeeList role fallback is reachable. Co-authored-by: MitchExpensify <MitchExpensify@users.noreply.github.com>
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.
|
|
🤖 Web test steps verification results: The reports empty state and the FAB create-report flow both show the correct non-paywall UI for a Collect-plan group workspace member. The hard-reload/cold-start race (VERIFY 3) could not be exercised, because a page reload during a driven web session risks dropping client auth into a magic-code wall this run cannot clear. This PR still has no documented testing steps — the
Same result shape as the Android run: the reachable flows are clean on both platforms, and the cold-start hydration race is unverified on both — blocked by app relaunch on Android and by page reload on Web. Since that race is the case the hydration guard exists to fix, it's worth covering with a unit test on the hook rather than through UI automation. Also note the |








Explanation of Change
Members of a group workspace were sometimes shown the "Reports are available on the Collect plan, starting at $5 per member per month" upgrade screen, even though their workspace already supports reports. This is not a plan gate —
reportshas norequiredPlanand is not inSUBMIT_FEATURE_IDS. The screen is the App's "you have no eligible group workspace" prompt, andUpgradeIntrois rendered without apolicyID, so it always shows the hardcoded Collect wording no matter what plan the user is actually on.usePolicyForMovingExpensesgates that prompt, and it treated "I could not resolve a policy" as "this user has no workspace" in three ways that are wrong for a real member:No hydration guard. The policy collection read at the top of the hook discarded its load status, so the moment the collection read back empty — including during Onyx cold start — the hook returned
shouldNavigateToUpgradePath: true.useCreateReportalready guards the identical situation about 40 lines away, with a comment describing this exact bug. This change reads the load status off the sameuseOnyxcall (so the value and the status always come from the same source, including when the search-snapshot redirect is in play) and no longer claims "no workspace" before we know.The active-workspace fallback was unreachable. The
activePolicyfallback sat below the unconditional early return, so a fully loaded, valid active workspace could never rescue the user. It now runs before the upgrade path. It is validated withisPolicyValidForMovingExpensesrather than the looser check the old fallback used, so an active workspace is only used when it is genuinely a valid destination — this cannot select something the old code would have rejected.EmptySearchViewdropped the login.getGroupPoliciesWhereReportCanBeCreatedwas called withoutcurrentUserLogin.getPolicyRolereturns early when there is no login, which makes itsemployeeList[login].rolefallback dead code, so any policy that carries the member's role only inemployeeListwas filtered out and the user looked workspace-less. This was the only one of seven call sites that omitted the login; the FAB, Search router, workspace selector, and navigation guard all pass it.One more ordering fix came out of this: the
isUnreportedManagedCardTransactionbranch never depends on a locally resolved policy — it deliberately returns an emptypolicyIDso the backend auto-selects — but it sat below the early return, so that flow also hit the upgrade screen whenever no policy resolved. It is now answered first.Reviewer note on the hydration guard: during the brief window where
shouldNavigateToUpgradePathis nowfalseinstead oftrue, the call sites fall through instead of no-op'ing. I checked each one and they all land on already-supported paths (the workspace selector, the report picker, or the same backend-auto-select path used for managed card transactions) rather than the wrong paywall, so no caller changes were needed. If you would rather have the entry points no-op while policies load — the wayuseCreateReportdoes withif (!arePoliciesLoaded) return;— the hook can exposearePoliciesLoadedand the ~8 acting call sites can gate on it. Happy to do that in this PR if you prefer it.Also worth noting:
isLoadingOnyxValueonly reflects the local cache read, not network hydration. The guard closes the cold-start window; the active-workspace fallback in item 2 is what covers the longer "response has not landed yet" window.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/681029
PROPOSAL: https://github.com/Expensify/Expensify/issues/681029#issuecomment-5702368350
AI Tests
Required local AI tests for this repo and these files could not be run in this environment, so CI is the only signal so far. This is a limitation of where the change was authored, not a skipped step:
package.jsonpinsnodeto26.5.0; only22.20.0is installed here, and.npmrcsetsengine-strict=true, sonpm cifails withEBADENGINEand nonode_modulescould be installed.npm run lint-changed,npm run typecheck,npm run prettier,npm test, ornpm run react-compiler-compliance-check check-changedcould execute.What was verified instead, by reading the code:
isLoadingOnyxValueis a default export takingResultMetadata, and the second element of auseOnyxtuple is exactly that — the same patternuseCreateReportalready uses.activePolicyisOnyxEntry<Policy>andactivePolicyIDisstring | undefined, matchingisPolicyValidForMovingExpensesandPolicyForMovingExpenses.PersonalDetails['login']isstring | undefined, matching the optionalcurrentUserLoginparameter.useOnyxcall still runs before the first early return, so the rules of hooks and React Compiler compliance are unaffected.shouldNavigateToUpgradePathconsumers were read to confirm the fall-through behavior described in the Explanation above.Please run the standard checks locally before marking this ready for review.
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Offline tests
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas do they need to test for regressions.
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