Skip to content

Hard to stop GitHub auth dance #180697

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.

Sign up for GitHub

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

Jump to bottom
Closed
joaomoreno opened this issue Apr 24, 2023 · 5 comments · Fixed by microsoft/vscode-pull-request-github#4803
Closed

Hard to stop GitHub auth dance #180697

joaomoreno opened this issue Apr 24, 2023 · 5 comments · Fixed by microsoft/vscode-pull-request-github#4803
Assignees
@TylerLeonhardt @alexr00
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone
May 2023

Comments

@joaomoreno
Copy link
Member

  1. Attempt to sign in to GitHub via GHPRI, click Allow
  2. Click Cancel

🐛 I now have to click twice No and once Cancel to stop the notification stream. I expected no extra clicks after step (2).

Screen.Recording.2023-04-24.at.15.55.08.mov
@TylerLeonhardt TylerLeonhardt added the bug Issue identified by VS Code Team member as probable bug label Apr 24, 2023
TylerLeonhardt added a commit that referenced this issue Apr 24, 2023
@TylerLeonhardt
Organize Errors in GitHub Auth and make sure no double prompting happens
c2e87df 
This mostly just moves some strings into variables... but this also fixes the GH Auth side of #180697 so you should only be asked once if you want to try a different way to log in.
@TylerLeonhardt TylerLeonhardt mentioned this issue Apr 24, 2023
Merged
@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Apr 24, 2023 •
edited
Loading

3rd notification is my bug, 4th would be GHPR (@alexr00). Here's the PR for my side ^^^

@alexr00 I throw a new Error('Cancelled') for this so you should be able to handle that accordingly. I would recommend not showing a notification and instead show the badge.

TylerLeonhardt added a commit that referenced this issue Apr 24, 2023
@TylerLeonhardt
Organize Errors in GitHub Auth and make sure no double prompting happ…
1714f71 
…ens (#180734)

* Organize Errors in GitHub Auth and make sure no double prompting happens

This mostly just moves some strings into variables... but this also fixes the GH Auth side of #180697 so you should only be asked once if you want to try a different way to log in.

* add comments
@alexr00 alexr00 added this to the May 2023 milestone Apr 25, 2023
@alexr00 alexr00 mentioned this issue May 3, 2023
Merged
alexr00 added a commit to microsoft/vscode-pull-request-github that referenced this issue May 3, 2023
@alexr00
Respect auth cancellation
8edb3c6 
Fixes microsoft/vscode#180697
alexr00 added a commit to microsoft/vscode-pull-request-github that referenced this issue May 10, 2023
@alexr00
Respect auth cancellation (#4803)
1467a11 
Fixes microsoft/vscode#180697
@andreamah andreamah added the verified Verification succeeded label May 31, 2023
@andreamah
Copy link
Contributor

Just verified this issue and seems okay, but I also noticed this:

  1. Click to sign into github for an extension that needs it.
image
  1. press cancel on the notification in vscode.
image
  1. still go through the steps to authorize github in the browser and open the link that they give to open in vscode.
  2. nothing happens 🐛. I get that nothing should change, but should there be a warning?

@TylerLeonhardt
Copy link
Member

@andreamah what would you like to see in the warning in this case?

@andreamah
Copy link
Contributor

I'm not too sure. The state just leaves me sorta questioning whether the auth did anything I think. But looking at it again, I see that it doesn't do anything if it succeeds, so I guess silently failing also works?

@TylerLeonhardt
Copy link
Member

I think I'd like air on the side of not annoying the user with yet another notification, so I think the silence is ok.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@TylerLeonhardt TylerLeonhardt

@alexr00 alexr00

Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Milestone
May 2023
Development

Successfully merging a pull request may close this issue.

Respect auth cancellation microsoft/vscode-pull-request-github
4 participants
@joaomoreno @TylerLeonhardt @andreamah @alexr00