[Part 2] Remove Onyx.connect(ONYXKEYS.SESSION) in /Session/index.ts: thread sesion through signOutAndRedirectToSignIn - #97963
[Part 2] Remove Onyx.connect(ONYXKEYS.SESSION) in /Session/index.ts: thread sesion through signOutAndRedirectToSignIn#97963truph01 wants to merge 16 commits into
Conversation
|
@shubham1206agra 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 014e759db5
ℹ️ 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 signOutPromise: Promise<void | Response<never>> = | ||
| !shouldRestoreStashedSession && !shouldStashSession | ||
| ? signOut({autoGeneratedLogin: credentials?.autoGeneratedLogin, signedInWithSAML: !!deprecatedSession.signedInWithSAML, authToken: deprecatedSession.authToken}) | ||
| ? signOut({autoGeneratedLogin: credentials?.autoGeneratedLogin, signedInWithSAML: !!session.signedInWithSAML, authToken: deprecatedSession.authToken}) |
There was a problem hiding this comment.
Use the supplied session token for logout
When callers now pass an explicit session to avoid relying on module-level Onyx state, this branch still sends deprecatedSession.authToken to LOG_OUT/SAML while using the supplied session for the other logout decisions. If deprecatedSession is stale during an account/session transition, the server invalidates the wrong token or leaves the current token valid. Use session.authToken consistently here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
In the next PR, we will remove deprecatedSession. I'm not doing it in this PR because it would require a lot of changes.
| openLink(setupLink, environmentURL); | ||
| }, [environmentURL, setupLink]); | ||
| openLink(setupLink, environmentURL, false, session); | ||
| }, [environmentURL, setupLink, session]); |
There was a problem hiding this comment.
Avoid re-opening HR setup on session updates
When the web HR flow stays mounted after openLink opens the OldDot setup URL, any subsequent SESSION Onyx update re-runs this effect because session is now in the dependency array, opening another setup URL/tab for the same click. The native variants guard this as a one-time open; keep the web flow mount-only as well, while still passing the latest session to openLink.
Useful? React with 👍 / 👎.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@truph01 Fix ESLint please |
|
@shubham1206agra I resolved conflicts |
Explanation of Change
Fixed Issues
$ #67782
PROPOSAL:
Tests
Offline tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
MacOS: Chrome / Safari
Screen.Recording.2026-08-06.at.18.50.16.mov