Repository navigation
[Part 2] Remove Onyx.connect(ONYXKEYS.SESSION) in /Session/index.ts: thread sesion through signOutAndRedirectToSignIn #97963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 29 commits
5de8203
73939bc
014e759
ecf1362
0d2b466
abafbd4
6d3bcda
1c4427a
93e84dd
12678ba
5bbb459
7611b41
0cdbc71
fbf04d8
1f059d5
1bfe1f5
5928078
4004609
6c5ca28
8b4f74b
cd396ca
66833a6
4c86812
22bf041
fa58304
a003e62
0603298
5a145ee
4947c9b
a5ee201
7463da2
7c404d6
119adb9
779004e
1d15600
750727d
3903963
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; | |
|
|
||
| import React from 'react'; | ||
| import {View} from 'react-native'; | ||
| import Onyx from 'react-native-onyx'; | ||
|
|
||
| import AvatarWithDisplayName from './AvatarWithDisplayName'; | ||
| import Button from './Button'; | ||
|
|
@@ -54,7 +55,7 @@ function AnonymousReportFooter({reportID}: AnonymousReportFooterProps) { | |
| sentryLabel={CONST.SENTRY_LABEL.ANONYMOUS_REPORT_FOOTER.SIGN_IN_BUTTON} | ||
| variant={CONST.BUTTON_VARIANT.SUCCESS} | ||
| onPress={() => { | ||
| signOutAndRedirectToSignIn(); | ||
| Onyx.get(ONYXKEYS.SESSION).then((session) => signOutAndRedirectToSignIn(session)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ CONSISTENCY-3 (docs)The exact Extract a single helper that reads the session and forwards it, then call it from every site. For example, add to function signOutAndRedirectToSignInWithSession(...args: SignOutArgs) {
Onyx.get(ONYXKEYS.SESSION).then((session) => signOutAndRedirectToSignIn(session, ...args));
}and replace the inline Reviewed at: 4947c9b | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’d keep this as is. A |
||
| }} | ||
| > | ||
| <Button.Text>{translate('common.signIn')}</Button.Text> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -373,6 +373,7 @@ const KEYS_TO_PRESERVE_SUPPORTAL = [ | |
| ]; | ||
|
|
||
| function signOutAndRedirectToSignIn( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @truph01 This feels like a place where we can use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @mountiny @fabioh8010 for opinions.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about this comment @mountiny @tgolen @fabioh8010 ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we are a long way away from having a more simple
I don't have a whole lot of context behind this though, so if it is a real problem, I'd like to understand more about this and what problem it creates with the React compiler.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@tgolen We are suppressing warnings in a lot of places with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have other functions need to be migrated like buildOnyxDataToCleanUpAnonymousUser, waitForUserSignIn, ...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I noticed that we use the ref approach in a few places, where the ref is accessed inside
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @truph01 Please keep this PR on hold and continue with other functions that are easy to migrate.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are moving forward with introducing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @tgolen! Updated to use
|
||
| session: OnyxEntry<Session>, | ||
|
truph01 marked this conversation as resolved.
Resolved
|
||
| shouldResetToHome?: boolean, | ||
| shouldStashSession?: boolean, | ||
| shouldSignOutFromOldDot = true, | ||
|
|
@@ -392,7 +393,7 @@ function signOutAndRedirectToSignIn( | |
| return; | ||
| } | ||
|
|
||
| const isSupportal = isSupportAuthToken(); | ||
| const isSupportal = isSupportAuthToken(session); | ||
| const shouldRestoreStashedSession = isSupportal || shouldForceUseStashedSession; | ||
|
|
||
| if (CONFIG.IS_HYBRID_APP && shouldSignOutFromOldDot) { | ||
|
|
@@ -404,7 +405,7 @@ function signOutAndRedirectToSignIn( | |
| // session. | ||
| 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}) | ||
| : Promise.resolve(); | ||
|
|
||
| // The function redirectToSignIn will clear the whole storage, so let's create our onyx params | ||
|
|
@@ -417,7 +418,7 @@ function signOutAndRedirectToSignIn( | |
| if (!isSupportal && shouldStashSession) { | ||
| onyxSetParams = { | ||
| [ONYXKEYS.STASHED_CREDENTIALS]: credentials, | ||
| [ONYXKEYS.STASHED_SESSION]: deprecatedSession, | ||
| [ONYXKEYS.STASHED_SESSION]: session, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -439,7 +440,7 @@ function signOutAndRedirectToSignIn( | |
| newDotCurrentAccountEmail: stashedSession.email ?? '', | ||
| authToken: stashedSession.authToken ?? '', | ||
| policyID: '', | ||
| accountID: deprecatedSession.accountID ? String(deprecatedSession.accountID) : '', | ||
| accountID: session?.accountID ? String(session.accountID) : '', | ||
| }); | ||
| hasSwitchedAccountInHybridMode = true; | ||
| } | ||
|
|
@@ -546,7 +547,7 @@ function callIfAuthenticated<TCallback extends ((...args: any[]) => any) | void> | |
| if (isAnonymousUser(session)) { | ||
| return () => { | ||
| close(() => { | ||
| signOutAndRedirectToSignIn(); | ||
| signOutAndRedirectToSignIn(session); | ||
| }); | ||
| }; | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.