Nudge users unable to push a workflow file to re-authenticate #8357
Conversation
This is legacy logic, when auditing addAccount call sites the only actual user is the sign in store which receives its account from the fetchAccount api method.
Turns out workflow files come in several shapes and sizes
|
I love this @niik! I was worried about how we'd inform people, and this feels like the perfect time to do so.
That's exactly where I'm at - definitely don't think we need to rush this in, especially since it's more complex and I'd like to give it a bit of time on beta. |
niik
moved this from Needs to be Prioritized
to ⭐️ PR Reviewable Priority ⭐️
in PR Priority
niik
added
the
ready-for-review
There was a problem hiding this comment.
This looks great @niik! Such a nice UX, our users will definitely be thankful 🦃
Just noticed one blocking change (indicated by a
Co-Authored-By: Katrina Uychaco <katrina@github.com>
Co-Authored-By: Katrina Uychaco <katrina@github.com>
Co-Authored-By: Katrina Uychaco <katrina@github.com>
|
I am curious how to get a rejected push due to a missing workflow scope? Please advise on any testing notes. Thanks. |
There was a problem hiding this comment.
Tested locally and this looks ✨✨✨
@tierninho for reference here were the steps I took for testing:
- In Dotcom settings revoke access for OAuth token for Desktop-dev
- Settings > Applications > Authorized OAuth Apps > The GitHub Desktop Development App > Revoke access
- Check out the latest release prior to introducing the
workflowscope (git checkout release-2.1.3) yarn,yarn build:dev,yarn start- Clear local storage (Dev tools > Application > Local Storage/file:// > right-click “Clear”) and refresh window to start from scratch at login page
- Ensure other GitHub Desktop instances are closed
- Sign in to github.com via browser, authorize Desktop
- Check in Dotcom settings to ensure that “Update github action workflows” does NOT appear under “Permissions”
- Quit Desktop-dev
- Checkout branch for this pr (
git checkout if-this-then-that) yarn,yarn build:dev,yarn start- Open repo with workflow file, edit and commit
- Push and see prompt asking to update permissions
- Click “Grant” and see successful push
- Check in Dotcom settings to ensure that “Update github action workflows” DOES appear under “Permissions”
- 🎉
| rejectedPath.indexOf('workflow') >= 0 | ||
| ) | ||
| ) { | ||
| if (!pathIsLikelyWorkflowFile) { |
ghost
mentioned this pull request
Overview
#8340 takes care of requesting the new
workflowsscope which lets us push workflow files. That PR is contained and safe but isn't as helpful to users as it could be, requiring users to parse the error message from dotcom, find their way to the Desktop issue tracker and figure out that they need to sign out and sign back in again.Description
This PR detects when a push was rejected due to a missing workflow scope and lets the user choose to grant Desktop the added scope.
I see #8340 as the bare minimum we need to provide users with an out and this as the icing on the cake actually providing a decent experience. As such #8340 is high priority for the next release whereas this can be considered a nice-to-have (would you agree with that @billygriffin).
For testing steps see here
Release notes
Notes: [Improved] Request additional permissions when unable to push workflow file.