[Payment due @thesahindia] Hide onboarding "add work email" from validated accounts#91142
[Payment due @thesahindia] Hide onboarding "add work email" from validated accounts#91142blimpich wants to merge 19 commits into
Conversation
The "People you may know are already here!" screen lives at /onboarding/private-domain and assumes the signup email is on a private domain. Validated public-domain users were being routed to it from BaseOnboardingWorkEmail and getting stuck (the screen's redirect only fires after a magic-code submission). Gate routing to it on isFromPublicDomain at the work-email redirect, the initial-path resolver (catches reload), and the screen itself (defense in depth).
signInWithTestUser sets validated:true by default, which now short-circuits the useEffect via the validated branch and prevents the navigation under test. The AddWorkEmail flow is only valid for unvalidated callers.
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.
|
The WorkEmailValidation screen is the merge entry-point for users adding a work email tied to an existing account; gating it on isFromPublicDomain broke the happy path where a public-domain user merges into a private-domain account.
isFromPublicDomain doesn't flip after AddWorkEmail swaps the primary, so the previous gate skipped PRIVATE_DOMAIN for the staging happy path (unvalidated gmail user adds a non-existing work email). Only validated public-domain users — the original bug case — should bypass the screen.
|
@thesahindia 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] |
|
Hello @thesahindia! When you get a chance can you review and test this PR's QA steps? Let me know if you have any questions! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b2393b917
ℹ️ 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".
|
**## Reviewer Checklist
Screenshots/VideosAndroid: HybridAppCouldn't make a video, android is super slow right now Android: mWeb ChromeScreen.Recording.2026-05-24.at.12.44.26.AM.moviOS: HybridAppScreen.Recording.2026-05-23.at.6.15.09.AM.moviOS: mWeb SafariScreen.Recording.2026-05-23.at.6.24.04.AM.movMacOS: Chrome / SafariScreen.Recording.2026-05-23.at.4.18.03.AM.movUploading Screen Recording 2026-05-23 at 5.18.43 AM.mov… |
@blimpich, what does "post-merge onboarding step" mean here? When merging an already verified account, there shouldn’t be any onboarding steps, right? |
|
There are two existing bug in the merge onboarding flow. After validating if user try to go back they get navigated to the same screen again and if you click back again the modal will close. Screen.Recording.2026-05-23.at.5.39.38.AM.mov@blimpich, will you handle it in a follow up PR? Or should I report it on slack |
Yeah that is a good point. We shouldn't 👍
Yes that sounds correct. It's all based off whether their NVP for completing onboarding is true or not.
Can you please report it on slack? Thank you! |
|
@NicolasBonet 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] |
|
🎯 @thesahindia, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
| const {translate} = useLocalize(); | ||
| const [loginList] = useOnyx(ONYXKEYS.LOGIN_LIST); | ||
| const [session] = useOnyx(ONYXKEYS.SESSION); | ||
| const [account] = useOnyx(ONYXKEYS.ACCOUNT); |
There was a problem hiding this comment.
❌ PERF-11 (docs)
useOnyx(ONYXKEYS.ACCOUNT) subscribes to the entire Account object, but this component only reads account?.validated and account?.isFromPublicDomain. Every unrelated change to the Account object will trigger a re-render of this component.
Add a selector to extract only the fields you need:
const [account] = useOnyx(ONYXKEYS.ACCOUNT, {
selector: (acc) => ({
validated: acc?.validated,
isFromPublicDomain: acc?.isFromPublicDomain,
}),
});Reviewed at: b45ad2a | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| const illustrations = useMemoizedLazyIllustrations(['EnvelopeReceipt', 'Gears', 'Profile']); | ||
| const [onboardingValues] = useOnyx(ONYXKEYS.NVP_ONBOARDING); | ||
| const [session] = useOnyx(ONYXKEYS.SESSION); | ||
| const [account] = useOnyx(ONYXKEYS.ACCOUNT); |
There was a problem hiding this comment.
❌ PERF-11 (docs)
useOnyx(ONYXKEYS.ACCOUNT) subscribes to the entire Account object, but this component only reads account?.validated and account?.isFromPublicDomain. Every unrelated change to the Account object will trigger a re-render of this component.
Add a selector to extract only the fields you need:
const [account] = useOnyx(ONYXKEYS.ACCOUNT, {
selector: (acc) => ({
validated: acc?.validated,
isFromPublicDomain: acc?.isFromPublicDomain,
}),
});Reviewed at: b45ad2a | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
LGTM, but let's review the AI comments which are valid @blimpich |
Reviewer Checklist
Screenshots/VideosMacOS: Chrome / SafariScreen.Recording.2026-05-25.at.9.22.54.AM.mov |
NicolasBonet
left a comment
There was a problem hiding this comment.
Waiting for AI-suggested reviews
Explanation of Change
A validated user can end up on the onboarding "add your work email" screen even though they shouldn't. Repro: sign up with a public-domain email, close the tab on that screen, sign back in via the magic code (which validates the account), and onboarding picks back up on the same screen. Submitting a work email from there is the entry point for the account-takeover the companion Auth PR closes on the backend.
This PR skips the work-email step during onboarding routing for validated accounts, and adds a redirect on the screen itself in case anything else lands the user there.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/638726
Tests
Offline tests
N/A — onboarding routing change, no network behavior modified.
QA Steps
Original bug case — validated user lands on "add work email"
Happy path 1 — unvalidated user adds a work email with no existing account
/onboarding/private-domain).Happy path 2 — unvalidated user adds a work email tied to an existing account (merge)
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 / Video
Screen.Recording.2026-05-20.at.1.16.01.PM.mov