Fix transition from OD to ND as delegate#83553
Conversation
melvin-bot
bot
requested review from
aimane-chnaif and
joekaufmanexpensify
and removed request for
a team
|
@aimane-chnaif 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] |
|
@MelvinBot please fill in "Explanation of Change" in description |
|
Done! I've filled in the Explanation of Change section in the PR description. |
![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: e310450b08
ℹ️ 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".
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.
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-03-05.at.4.58.26.pm.mov |
|
@aimane-chnaif thanks for review! I applied your suggestions, so please take another look :D |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a18c4fb58d
ℹ️ 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".
| const linkedDelegatorEmail = getDelegatorEmailFromURL(transitionURL) ?? null; | ||
|
|
||
| return linkedEmail !== sessionEmail && linkedDelegatorEmail !== sessionEmail; |
There was a problem hiding this comment.
When transitionURL is a full URL and delegatorEmail is the first query parameter, URLSearchParams misses it and this path falls back to getDelegatorEmailFromURL(), which returns the raw (possibly URL-encoded) value. In OldDot web flows where emails are encoded (e.g. user%40example.com), comparing that raw string to sessionEmail (user@example.com) makes isLoggingInAsNewUser() incorrectly return true, so delegate transitions still hit the sign-out flow instead of being treated as same-user delegate switches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Does this make sense?
There was a problem hiding this comment.
@aimane-chnaif I feel like it replicates logic that was here earlier and didn't cause any troubles so it shouldn't break anything
There was a problem hiding this comment.
This comment makes sense to me, @jnowakow can we test if this case is true and address it in this PR if so?
aimane-chnaif
left a comment
aimane-chnaif
left a comment
There was a problem hiding this comment.
Please check #83553 (comment). And pull main.
Otherwise looks good.
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #82330 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a18c4fb58d
ℹ️ 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".
| const delegatorEmailParamRegex = /[?&]delegatorEmail=([^&]*)/g; | ||
| const delegatorMatches = delegatorEmailParamRegex.exec(url ?? ''); | ||
| return delegatorMatches?.[1]; |
There was a problem hiding this comment.
In isLoggingInAsNewUser, the fallback path compares linkedDelegatorEmail directly to sessionEmail, but getDelegatorEmailFromURL() returns the raw query substring. For hybrid transitions we pass a full URL (new-expensify://...), so when delegatorEmail is the first query param URLSearchParams misses it and this fallback is used; if OldDot web encoded the value (%40), the comparison fails and the delegate flow is still classified as a new-user login, which sends users through sign-out in LogOutPreviousUserPage instead of switching as delegate.
Useful? React with 👍 / 👎.
|
@grgia can you please review this when you have a moment 🙏 |
| const linkedDelegatorEmail = getDelegatorEmailFromURL(transitionURL) ?? null; | ||
|
|
||
| return linkedEmail !== sessionEmail && linkedDelegatorEmail !== sessionEmail; |
There was a problem hiding this comment.
This comment makes sense to me, @jnowakow can we test if this case is true and address it in this PR if so?
| ['should return true when delegatorEmail value contains encoded characters', '?delegatorEmail=user%40example.com', true], | ||
| ])('%s', (_description, transitionURL, expectedResult) => { |
There was a problem hiding this comment.
Yup, it proves that it works as expected :D
There was a problem hiding this comment.
So I think we're good to merge it, right @grgia? 😸
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @grgia 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.3.37-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.37-10 🚀
|
6 participants
Explanation of Change
When transitioning from OldDot to NewDot as a Copilot (delegate), the transition URL includes a
delegatorEmailparameter. Previously,isLoggingInAsNewUserinSessionUtils.tsdid not account for this parameter, so it incorrectly treated the delegate transition as a new user login. This causedLogOutPreviousUserPageto sign the user out instead of allowing the Copilot switch.This PR fixes the issue by:
isLoggingInAsNewUserto also check thedelegatorEmailparameter — if it matches the current session email, the user is not a new user.isLoggingInAsDelegatehelper that detects when a transition URL contains adelegatorEmailparameter.isLoggingInAsDelegateinLogOutPreviousUserPageto return early (skip re-authentication) when the user is switching to Copilot mode, preventing the unwanted sign-out. This is needed becauseSignInWithShortLivedAuthTokenoverrides session with original account. This can be fixed in a follow-upFixed Issues
$ #82330
PROPOSAL: N/A
Tests
Offline tests
N/A
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
copilot-login.mov