-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix copilot still persists #69754
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
Fix copilot still persists #69754
Changes from all commits
bddf03c
d541899
27f58be
65f281b
12de25e
0cd9cf9
14a6a4e
42d3590
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 |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ | |
| import updateSessionUser from './Session/updateSessionUser'; | ||
|
|
||
| let delegatedAccess: DelegatedAccess; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.ACCOUNT, | ||
| callback: (val) => { | ||
| delegatedAccess = val?.delegatedAccess ?? {}; | ||
|
|
@@ -29,31 +29,31 @@ | |
| }); | ||
|
|
||
| let credentials: Credentials = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.CREDENTIALS, | ||
| callback: (value) => (credentials = value ?? {}), | ||
| }); | ||
|
|
||
| let stashedCredentials: Credentials = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.STASHED_CREDENTIALS, | ||
| callback: (value) => (stashedCredentials = value ?? {}), | ||
| }); | ||
|
|
||
| let session: Session = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.SESSION, | ||
| callback: (value) => (session = value ?? {}), | ||
| }); | ||
|
|
||
| let stashedSession: Session = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.STASHED_SESSION, | ||
| callback: (value) => (stashedSession = value ?? {}), | ||
| }); | ||
|
|
||
| let activePolicyID: OnyxEntry<string>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.NVP_ACTIVE_POLICY_ID, | ||
| callback: (newActivePolicyID) => { | ||
| activePolicyID = newActivePolicyID; | ||
|
|
@@ -75,6 +75,8 @@ | |
| // This allows the report screen to load correctly when the delegate token expires and the delegate is returned to their original account. | ||
| ONYXKEYS.IS_SIDEBAR_LOADED, | ||
| ONYXKEYS.NETWORK, | ||
| ONYXKEYS.SHOULD_USE_STAGING_SERVER, | ||
| ONYXKEYS.IS_DEBUG_MODE_ENABLED, | ||
|
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. Same here, we can remove
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. Same as above, I think we should preserve this too. |
||
| ]; | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ | |
|
|
||
| let currentIsOffline: boolean | undefined; | ||
| let currentShouldForceOffline: boolean | undefined; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.NETWORK, | ||
| callback: (network) => { | ||
| currentIsOffline = network?.isOffline; | ||
|
|
@@ -28,7 +28,8 @@ | |
| keysToPreserve.push(ONYXKEYS.PREFERRED_THEME); | ||
| keysToPreserve.push(ONYXKEYS.ACTIVE_CLIENTS); | ||
| keysToPreserve.push(ONYXKEYS.DEVICE_ID); | ||
| keysToPreserve.push(ONYXKEYS.ACCOUNT); | ||
| keysToPreserve.push(ONYXKEYS.SHOULD_USE_STAGING_SERVER); | ||
| keysToPreserve.push(ONYXKEYS.IS_DEBUG_MODE_ENABLED); | ||
|
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. Same here, we can remove
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. And here. |
||
|
|
||
| // After signing out, set ourselves as offline if we were offline before logging out and we are not forcing it. | ||
| // If we are forcing offline, ignore it while signed out, otherwise it would require a refresh because there's no way to toggle the switch to go back online while signed out. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can only include
ONYXKEYS.SHOULD_USE_STAGING_SERVERintoKEYS_TO_PRESERVElistONYXKEYS.IS_DEBUG_MODE_ENABLEDseems isn't kept after clear Onyx or signin/signout.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tested, and the debug mode is persisted.
w.mp4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try to test when turn-on both Debug mode and "Use staging server"?
Screen.Recording.2025-09-04.at.22.28.23.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, that's really weird. Don't you think it's more like a bug, no? Like, why should the debug mode be turned off only if the staging server is on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's because of this
App/src/libs/actions/App.ts
Lines 641 to 643 in 3a05f5b
If the staging server is previously on, after
Onyx.clear, we re-set theshouldUseStagingServerusing Onyx.set. Not sure why this is needed because we already put ACCOUNT in KEYS_TO_PRESERVE.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Julesssss, what are your thoughts on
IS_DEBUG_MODE_ENABLED? It looks like we don't intend to keep this data when switching between OD-ND app, or sign in/out, or resetting Onyx data. Thus, I suggested we can remove it fromKEYS_TO_PRESERVElist, but if you think we can leave them as the current implementation of this PR, I'm happy to keep them in the list. It's a troubleshooting config, thus I'm 50:50 on this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot. So there does seem to be good reason for keeping the staging flag, presumably for testing (original issue).
So do you think that makes the
shouldUseStagingServercondition redundant?But yeah, similar to the staging env config, I think we should keep
IS_DEBUG_MODE_ENABLEDto allow us to test/debug flows that include signout.