fix: onboarding back button redirects to blank modal for private-domain VSB/SMB#95001
Conversation
The EMPLOYEES screen back button redirected to a blank Workspaces modal because getDomainPrefix hardcoded both PRIVATE_DOMAIN and WORKSPACES for every private-domain user, even though only one is ever traversed. Gate the private-domain prefix on account validation (mirroring Expensify#93399's public-domain fix): unvalidated users keep PRIVATE_DOMAIN, validated users get WORKSPACES, and validated users without joinable workspaces (which auto-skip that screen) get neither. Back now resolves to the screen the user actually came from and the step counter is accurate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@jayeshmangwani 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] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9d457b59c
ℹ️ 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".
| purposeSelected: purposeSelected ?? undefined, | ||
| isMergeAccountStepSkipped: onboardingValues?.isMergeAccountStepSkipped, | ||
| isAccountValidated: !!account?.validated, | ||
| hasJoinablePolicies: Object.keys(joinablePolicies ?? {}).length > 0, |
There was a problem hiding this comment.
When the only entries in JOINABLE_POLICIES are SUBMIT policies and SUBMIT_2026 is disabled, onboarding treats Workspaces as not shown: BaseOnboardingPrivateDomain counts only policies passing policyType !== SUBMIT || canUseSubmit2026, and BaseOnboardingWorkspaces renders the same filtered list. This unfiltered Object.keys check marks those hidden policies as joinable, so a validated private-domain VSB/SMB user can get an inflated flow and have Back from Employees route to an empty/skipped Workspaces screen. Please derive hasJoinablePolicies with the same visibility filter used by the Workspaces flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, this is valid. The flow branch in BaseOnboardingPrivateDomain filters out SUBMIT policies when SUBMIT_2026 is off, so a validated VSB/SMB user whose only joinable policies are SUBMIT skips WORKSPACES — but hasJoinablePolicies was computed from an unfiltered Object.keys(...) count, adding a phantom WORKSPACES step and routing Back from EMPLOYEES to a blank screen.
Fixed in b15e80b by centralizing the visibility filter in a getVisibleJoinablePoliciesCount helper and using it in the step counter, the EMPLOYEES flow context, and the PrivateDomain branch, so all sites stay in sync. Added a unit test covering the all-SUBMIT / beta-off case.
A validated private-domain VSB/SMB user whose only joinable policies are SUBMIT type (with the SUBMIT_2026 beta off) skips the WORKSPACES screen, because BaseOnboardingPrivateDomain filters those policies out. But the step counter and the EMPLOYEES back button computed hasJoinablePolicies from an unfiltered count, adding a phantom WORKSPACES step and routing Back from EMPLOYEES to a blank, never-visited Workspaces screen. Centralize the visibility filter in getVisibleJoinablePoliciesCount and use it in the step counter, the EMPLOYEES flow context, and the PrivateDomain branch so all sites stay in sync. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the Codex P2 finding in b15e80b: |
isTrackOnboardingChoice is only ever imported as the default export, so exporting it as a named binding too tripped the knip unused-export check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91460260bc
ℹ️ 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".
| if (!context.isAccountValidated) { | ||
| return [ONBOARDING.PERSONAL_DETAILS, ONBOARDING.PRIVATE_DOMAIN]; | ||
| } |
There was a problem hiding this comment.
This branch treats private-domain users as unvalidated unless account.validated is true, but the screen that sends them to the previous step uses isCurrentUserValidated(loginList, session?.email) to choose WORKSPACES vs PRIVATE_DOMAIN (BaseOnboardingPersonalDetails). When the current login has a validatedDate before account.validated is populated, a user can legitimately go Personal details → Workspaces → Employees, but the Employees back route is computed as PRIVATE_DOMAIN, recreating the never-visited/blank modal path this change is trying to avoid. The step counter/back-route context needs to use the same current-login validation signal as the navigation branch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I'm trying to verify this comment to determine whether it's a legitimate bug, but I can't reproduce any issue. I'm also not sure how validatedDate would be set in order to test this scenario.
@dilshodmackbook-sketch Are you seeing any issue when testing this flow?
There was a problem hiding this comment.
@jayeshmangwani you're right, I couldn't reproduce it either. This flow only runs for the signed-in primary login, and its validatedDate and account.validated reflect the same validation state, so I couldn't find a path where the two diverge before this screen renders — the back route stays in sync with navigation.
If we want to be defensive about it, I can point the step-counter context at the same isCurrentUserValidated helper the navigation already uses, so the two can never drift later. It's a one-line change — happy to add it if you think it's worth it, otherwise fine to leave as is.
There was a problem hiding this comment.
Went ahead and added the defensive change we discussed — the step-counter/back-route context now derives validation from isCurrentUserValidated(loginList, session?.email), the same signal BaseOnboardingPersonalDetails navigates on, instead of account.validated. Even though we couldn't reproduce a divergence, this guarantees the back route and navigation can't drift apart later. It's in b024a340b76, touching only the two context builders (useOnboardingStepCounter and BaseOnboardingEmployees). Ready for another look whenever you get a chance.
| if (!context.isAccountValidated) { | ||
| return [ONBOARDING.PERSONAL_DETAILS, ONBOARDING.PRIVATE_DOMAIN]; | ||
| } | ||
| return context.hasJoinablePolicies ? [ONBOARDING.PERSONAL_DETAILS, ONBOARDING.WORKSPACES] : [ONBOARDING.PERSONAL_DETAILS]; |
There was a problem hiding this comment.
Dropping WORKSPACES whenever hasJoinablePolicies is false does not match the current validated private-domain flow: BaseOnboardingPersonalDetails still navigates validated VSB/SMB users to ONBOARDING_WORKSPACES, and BaseOnboardingWorkspaces only fetches/renders the Skip button when the list is empty; it does not auto-skip. In that no-visible-policy case, pressing Skip takes the user to Employees, but this flow makes the Employees back button go to Personal details instead of the Workspaces screen they just visited.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We already check isPrivateDomainAndHasAccessiblePolicies in BaseOnboardingPersonalDetails before navigating, so I don't think this will cause any bug.
There was a problem hiding this comment.
@jayeshmangwani agreed. Excluding WORKSPACES when there are no visible joinable policies is intentional — it uses the same getVisibleJoinablePoliciesCount visibility filter as the rest of the flow, so the back route and step counter only account for screens with something to show. That empty state is what made the back button land on a blank modal in the original report, so keeping the flow consistent with the visible screens is what avoids it. And as you noted, the isPrivateDomainAndHasAccessiblePolicies gate already scopes this to private-domain accounts, so nothing else reaches this branch.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@jayeshmangwani fixed ✅ |
|
@dilshodmackbook-sketch Can we please fix the navigation here? When we press the back button in the header on the navigation-forward.mov |
…ation The EMPLOYEES header back button called Navigation.navigate(previousRoute), which plays a forward (push) transition even though it returns the user to a screen they came from (e.g. PRIVATE_DOMAIN). Switch to Navigation.goBack so the transition animates as a backward pop, matching every other onboarding screen. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Good catch, fixed. The back button was calling Screen.Recording.2026-07-03.at.15.37.42.mov |
…nboarding-back-button # Conflicts: # src/libs/OnboardingUtils.ts
Use isCurrentUserValidated(loginList, session?.email) for the private-domain back-route context, matching the signal BaseOnboardingPersonalDetails branches on, so the step counter and navigation can never drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid.movAndroid: mWeb Chromemweb-chrome.moviOS: HybridAppiOS.moviOS: mWeb Safarimweb-safari.movMacOS: Chrome / Safariweb.mov |
|
@mollfpr this is approved by @jayeshmangwani and all checks are green, so it's ready for your final review whenever you get a chance. Thanks! |
|
🚧 mollfpr has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mollfpr in version: 9.4.32-0 🚀
|
|
🤖 No help site changes required. I reviewed the changes in this PR against the help site articles under Why: This PR is a purely internal bug fix to the onboarding navigation state machine — it corrects the back-button destination and step counter on the How many employees do you have? screen for private-domain VSB/SMB users. Specifically it:
There is no user-facing product change here — no new features, screens, copy, settings, tabs, or buttons. The help site documents how to use Expensify features, and it does not document the internal routing/step-count behavior of the sign-up onboarding wizard (the affected screens are transient sign-up steps, not documented product surfaces). A search of If a reviewer believes a specific help article should cover part of this flow, point me to it and I'll take another look. @dilshodmackbook-sketch — no draft help site PR was created because no documentation changes are warranted for this internal navigation fix. Let me know if you'd like me to reconsider a specific article. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.4.32-3 🚀
Bundle Size Analysis (Sentry): |
5 participants
Explanation of Change
During onboarding, a private-domain VSB/SMB user who signs up with accessible domain policies saw a back arrow on the How many employees do you have? (
OnboardingEmployees) screen that navigated to a blank Workspaces modal.Root cause:
getDomainPrefixingetOnboardingStepCounter.tshardcoded bothPRIVATE_DOMAINandWORKSPACESfor every private-domain user, even though a user only ever traverses one of them beforeEMPLOYEES(gated by account validation inBaseOnboardingPersonalDetails). The phantomWORKSPACESstep inflated the step counter and made theEMPLOYEESback button resolve to a screen the user never visited, which renders blank.This change gates the private-domain prefix on account validation — the same signal
BaseOnboardingPersonalDetailsbranches on, mirroring the public-domain fix from #93399:[PERSONAL_DETAILS, PRIVATE_DOMAIN]— Back lands onPRIVATE_DOMAIN(the screen the user came from).[PERSONAL_DETAILS, WORKSPACES]— Back lands onWORKSPACES.[PERSONAL_DETAILS]— Back lands onPERSONAL_DETAILS.The
hasJoinablePoliciessignal is threaded into the flow context fromJOINABLE_POLICIESin both context builders (useOnboardingStepCounterandBaseOnboardingEmployees). Public-domain and no-domain flows are untouched.Fixed Issues
$ #93660
PROPOSAL: #93660 (comment)
Tests
Offline tests
Same as tests.
QA Steps
tstsg.comaddress) that has accessible policies.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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
Screen.Recording.2026-06-30.at.23.57.37.mp4
Android: mWeb Chrome
androidMweb.mp4
iOS: Native
iosNative.mp4
iOS: mWeb Safari
ios.mov
MacOS: Chrome / Safari
chrome.mp4
safari.mov