-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix connecting rich integrations on login to GK Dev (GLVSC-631) #3501
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
Conversation
18b1092
to
4191a79
Compare
|
||
private onUserCheckedIn() { | ||
void this.syncCloudIntegrations(false); | ||
void this.syncCloudIntegrations(true); |
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.
@sergeibbb This will cause check-ins to always force reconnection to providers, even when the user manually disconnects and disables them.
check-in is considered a passive sync case and should never force reconnections, because it happens automatically in the background every 12 hours (in my repro here, I can trigger it by refreshing the account view. Note that my github integration was reconnected even though I chose to disable it previously, which is undesirable behavior).
Instead, we should check the logout flow and ensure that integrations can be disconnected "softly" when a logout or some other passive sync case triggers the disconnect. Then those integrations would reconnect upon login even when forceConnect
is false
on syncCloudIntegrations
.
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.
fixed
4191a79
to
35a877c
Compare
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.
The side-effect of this reset is similar to the previous issue - by clearing the "connected" key, we lose context on manual disconnects the user did, so any integrations the user manually disabled are automatically reconnected when logging back in (which is unexpected).
To repro:
- Go to remotes view and disconnect GitHub, then choose "disable and sign out"
- Log out of your GitKraken account in the client
- Log back in to your GitKraken account in the client
- Notice that GitHub is automatically reconnected (should still be disconnected)
I think it's fine to just use this.disconnect
here, but we may need a new flag to use (not currentSessionOnly
, because setting that causes this.container.integrations.disconnected(this, this.key)
not to fire). The flag would just ensure that we do not store "false" for the connected key of this integration in storage.
@sergeibbb Hope you can forgive me committing to your branch. Since we want this in the release next week, I went ahead and updated the PR: it is back to using |
Description
When you logout from GK Dev and then login back, all configured cloud integrations should become connected.
Checklist
Fixes $XXX -
orCloses #XXX -
prefix to auto-close the issue that your PR addresses