Redirect invited member to workspace chat instead of Home - #95418
Conversation
A first-time invitee opens a validate-login magic link that carries an exitTo (the workspace chat). isUserClickedSignIn matched that case (no cached login, just signed in) and its focus effect force-redirected Home, clobbering handleExitToNavigation's exitTo navigation. Exclude exitTo from isUserClickedSignIn so the Home redirect no longer fires when a deep-link destination exists. Fixes Expensify#94549
|
@hoangzinh 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] |
|
@Abdulloh0109, could you please temporarily close or mark this PR as a draft? We are still waiting for the internal engineer to accept your proposal. |
|
Are we still waiting? Should I close the PR, or should I leave it open? @hoangzinh |
|
just leave it open. I will review the PR soon |
hoangzinh
left a comment
There was a problem hiding this comment.
all goods. Just one minor comment
| // Exclude `exitTo` deep links (e.g. a workspace-chat invite link): those carry a specific | ||
| // destination that `handleExitToNavigation` owns, so this flag's Home redirect (focus effect | ||
| // below) must not fire and clobber it. Without `!exitTo`, a first-time invitee (no cached | ||
| // `login`, just signed in) matches this exactly and gets force-redirected Home instead of the | ||
| // workspace chat. |
There was a problem hiding this comment.
I think we can write it shorter.
| // Exclude `exitTo` deep links (e.g. a workspace-chat invite link): those carry a specific | |
| // destination that `handleExitToNavigation` owns, so this flag's Home redirect (focus effect | |
| // below) must not fire and clobber it. Without `!exitTo`, a first-time invitee (no cached | |
| // `login`, just signed in) matches this exactly and gets force-redirected Home instead of the | |
| // workspace chat. | |
| // Exclude `exitTo` deep links (e.g. a workspace-chat invite link): those carry a specific | |
| // destination that `handleExitToNavigation` owns, so this flag's Home redirect (focus effect | |
| // below) must not fire and clobber it. |
There was a problem hiding this comment.
Good call, done 👍
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-07-21.at.19.26.55.movAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariScreen.Recording.2026-07-21.at.20.30.12.movMacOS: Chrome / SafariScreen.Recording.2026-07-21.at.19.04.38.mov |
|
Why has it taken so long? |
|
Why was I unassigned from this job? What was the reason? @hoangzinh |
Can you elaborate it @Abdulloh0109? I saw you are still assigned to this issue |
|
🚧 madmax330 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/madmax330 in version: 9.4.45-0 🚀
|
Help site review — no changes requiredI reviewed the changes in this PR against the help site articles under Why: This is a purely internal navigation bug fix. It excludes What I checked
@Abdulloh0109, since no help site PR was created, there's nothing to review or mark |
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.4.45-14 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
A first-time invitee opens a validate-login magic link that carries an
exitTodestination (the workspace chat), e.g./v/<accountID>/<validateCode>?exitTo=/r/<reportID>. On web this is handled byValidateLoginPage.The destination reaches the page correctly, but a second, competing navigation to Home fires and overwrites it.
isUserClickedSignInis meant to detect a separate-session magic-link sign-in (no cachedlogin, just signed in) so its focus effect can redirect the user Home. A first-time invitee matches that condition exactly — no cachedlogin, just signed in — and also carries anexitTo. So after sign-in two navigations race:handleExitToNavigation→ the workspace chat, and the focus effect →navigate(ROUTES.HOME, {forceReplace: true}). The HomeforceReplacewins and the member lands on Home.The fix excludes
exitTofromisUserClickedSignIn, so when a deep-link destination exists the Home redirect never fires andhandleExitToNavigationowns the navigation. The separate-session case (nologin, noexitTo) is unchanged, so that behavior isn't regressed.Added a regression test in
tests/ui/ValidateLoginPageTest.tsxthat reproduces the invitee-with-exitTostate and asserts the Home redirect does not fire whilehandleExitToNavigationis invoked with the destination (fails without the fix, passes with it).Fixed Issues
$ #94549
PROPOSAL: #94549 (comment)
Tests
/v/<accountID>/<validateCode>?exitTo=/r/<reportID>).Offline tests
Not applicable — the flow is a validate-login sign-in that requires a network connection to complete. Once signed in, the
exitTodestination navigation runs as in the Tests above.QA Steps
Same as Tests, on staging (web — Windows/Chrome, macOS/Chrome/Safari, mWeb Chrome/Safari):
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
MacOS: Chrome / Safari
6ff31b56-0666-4ddf-b17a-5967515718d6.mp4