From 14ce1f99a705d9c8e15a21afa62780f6e888eb02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 15 Apr 2020 14:28:07 +0200 Subject: [PATCH 01/47] Ask for an additional `read:org` OAuth scope This is to facilitate: - requesting teams for review on `pr create` - allowing `repo create ORG/REPO --team TEAM` --- auth/oauth.go | 10 ++++++++-- context/config_setup.go | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/auth/oauth.go b/auth/oauth.go index dc7a9d085..3f9d9ddc4 100644 --- a/auth/oauth.go +++ b/auth/oauth.go @@ -11,6 +11,7 @@ import ( "net/http" "net/url" "os" + "strings" "github.com/cli/cli/pkg/browser" ) @@ -29,6 +30,7 @@ type OAuthFlow struct { Hostname string ClientID string ClientSecret string + Scopes []string WriteSuccessHTML func(io.Writer) VerboseStream io.Writer } @@ -45,11 +47,15 @@ func (oa *OAuthFlow) ObtainAccessToken() (accessToken string, err error) { } port := listener.Addr().(*net.TCPAddr).Port + scopes := "repo" + if oa.Scopes != nil { + scopes = strings.Join(oa.Scopes, " ") + } + q := url.Values{} q.Set("client_id", oa.ClientID) q.Set("redirect_uri", fmt.Sprintf("http://127.0.0.1:%d/callback", port)) - // TODO: make scopes configurable - q.Set("scope", "repo") + q.Set("scope", scopes) q.Set("state", state) startURL := fmt.Sprintf("https://%s/login/oauth/authorize?%s", oa.Hostname, q.Encode()) diff --git a/context/config_setup.go b/context/config_setup.go index 56bdfe55a..a55b8dfa6 100644 --- a/context/config_setup.go +++ b/context/config_setup.go @@ -36,6 +36,7 @@ func setupConfigFile(filename string) (*configEntry, error) { Hostname: oauthHost, ClientID: oauthClientID, ClientSecret: oauthClientSecret, + Scopes: []string{"repo", "read:org"}, WriteSuccessHTML: func(w io.Writer) { fmt.Fprintln(w, oauthSuccessPage) }, From 3d566dc5a6bb74abb5f8aa4ae763add14b4027d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 15 Apr 2020 17:25:15 +0200 Subject: [PATCH 02/47] Detect and warn about `read:org` OAuth scope being missing --- api/client.go | 41 +++++++++++++++++++++++++++++++++++++++++ command/root.go | 1 + 2 files changed, 42 insertions(+) diff --git a/api/client.go b/api/client.go index 2fc72aaa6..e1fef7219 100644 --- a/api/client.go +++ b/api/client.go @@ -7,6 +7,7 @@ import ( "io" "io/ioutil" "net/http" + "os" "regexp" "strings" @@ -63,6 +64,46 @@ func ReplaceTripper(tr http.RoundTripper) ClientOption { } } +var issuedScopesWarning bool + +// CheckScopes checks whether an OAuth scope is present in a response +func CheckScopes(wantedScope string) ClientOption { + return func(tr http.RoundTripper) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + res, err := tr.RoundTrip(req) + if err != nil || issuedScopesWarning { + return res, err + } + + isApp := res.Header.Get("X-Oauth-Client-Id") != "" + hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",") + + hasWanted := false + for _, s := range hasScopes { + if wantedScope == strings.TrimSpace(s) { + hasWanted = true + break + } + } + + if !hasWanted { + fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.") + // TODO: offer to take the person through the authentication flow again? + // TODO: retry the original request if it was a read? + if isApp { + fmt.Fprintln(os.Stderr, "To re-authenticate, please `rm ~/.config/gh/config.yml` and try again.") + } else { + // the person has pasted a Personal Access Token + fmt.Fprintln(os.Stderr, "Re-generate your token in `rm ~/.config/gh/config.yml` and try again.") + } + issuedScopesWarning = true + } + + return res, nil + }} + } +} + type funcTripper struct { roundTrip func(*http.Request) (*http.Response, error) } diff --git a/command/root.go b/command/root.go index fa65d7e05..6eafd01d4 100644 --- a/command/root.go +++ b/command/root.go @@ -131,6 +131,7 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { opts = append(opts, apiVerboseLog()) } opts = append(opts, + api.CheckScopes("read:org"), api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), // antiope-preview: Checks From be51d095ea3ea9294851af28ed9b0a10564d03d1 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 21 Apr 2020 13:30:07 -0700 Subject: [PATCH 03/47] Separating core from non-core commands --- command/root.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/command/root.go b/command/root.go index fa65d7e05..60fe77daa 100644 --- a/command/root.go +++ b/command/root.go @@ -47,6 +47,8 @@ func init() { // TODO: // RootCmd.PersistentFlags().BoolP("verbose", "V", false, "enable verbose output") + RootCmd.SetHelpFunc(aTempHelpFuncfunc) + RootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { if err == pflag.ErrHelp { return err @@ -206,3 +208,33 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interfac return baseRepo, nil } + +type helpEntry struct { + Title string + Body string +} + +func aTempHelpFuncfunc(command *cobra.Command, s []string) { + out := colorableOut(command) + fmt.Fprint(out, "\nWork seamlessly with GitHub from the command line.\n\n") + + var commands []string + for _, c := range command.Commands() { + s := c.Name() + ":" + strings.Repeat(" ", c.NamePadding()) + c.Short + commands = append(commands, s) + } + + helpEntries := []helpEntry{ + {"USAGE", ` + gh [flags] + Commands are run inside of a GitHub repository.`}, + {"CORE COMMANDS", strings.Join(commands, "\n")}, + } + + for _, e := range helpEntries { + if e.Title != "" { + fmt.Fprintln(out, utils.Bold(e.Title)) + } + fmt.Fprintln(out, strings.TrimLeft(e.Body, "\n")+"\n") + } +} From b8ecf1fab56c28397806ca1f6b34519380caa1a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Apr 2020 15:16:24 +0200 Subject: [PATCH 04/47] Bump homebrew formula immediately after release --- .github/workflows/releases.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 2f9908167..e9d183592 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -26,6 +26,13 @@ jobs: args: release --release-notes=CHANGELOG.md env: GITHUB_TOKEN: ${{secrets.UPLOAD_GITHUB_TOKEN}} + - name: Bump homebrew-core formula + uses: mislav/bump-homebrew-formula-action@v1 + if: "!contains(github.ref, '-')" # skip prereleases + with: + formula-name: gh + env: + COMMITTER_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} - name: move cards if: "!contains(github.ref, '-')" env: @@ -38,13 +45,6 @@ jobs: api() { bin/hub api -H 'accept: application/vnd.github.inertia-preview+json' "$@"; } cards=$(api projects/columns/$PENDING_COLUMN/cards | jq ".[].id") for card in $cards; do api projects/columns/cards/$card/moves --field position=top --field column_id=$DONE_COLUMN; done - - name: Bump homebrew-core formula - uses: mislav/bump-homebrew-formula-action@v1 - if: "!contains(github.ref, '-')" # skip prereleases - with: - formula-name: gh - env: - COMMITTER_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} msi: needs: goreleaser runs-on: windows-latest From 60a67b852a43469df09035445131b54de5e5e933 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Apr 2020 15:59:44 +0200 Subject: [PATCH 05/47] Automatically generate site docs on release --- .github/workflows/releases.yml | 16 +++++++++++++--- Makefile | 10 ++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index e9d183592..7fe2ca3af 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -33,13 +33,23 @@ jobs: formula-name: gh env: COMMITTER_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} - - name: move cards - if: "!contains(github.ref, '-')" + - name: Checkout documentation site + if: "!contains(github.ref, '-')" # skip prereleases + uses: actions/checkout@v2 + with: + repository: github/cli.github.com + path: site + fetch-depth: 0 + token: ${{secrets.SITE_GITHUB_TOKEN}} + - name: Publish documentation site + if: "!contains(github.ref, '-')" # skip prereleases + run: make site-publish + - name: Move project cards + if: "!contains(github.ref, '-')" # skip prereleases env: GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} PENDING_COLUMN: 8189733 DONE_COLUMN: 7110130 - shell: bash run: | curl -fsSL https://github.com/github/hub/raw/master/script/get | bash -s 2.14.1 api() { bin/hub api -H 'accept: application/vnd.github.inertia-preview+json' "$@"; } diff --git a/Makefile b/Makefile index 5a93dac9b..ffbd5bd7e 100644 --- a/Makefile +++ b/Makefile @@ -34,3 +34,13 @@ site-docs: site git -C site add 'manual/gh*.md' git -C site commit -m 'update help docs' .PHONY: site-docs + +site-publish: site-docs +ifndef GITHUB_REF + $(error GITHUB_REF is not set) +endif + sed -i.bak -E 's/(assign version = )".+"/\1"$(GITHUB_REF:refs/tags/v%=%)"/' site/index.html + rm -f site/index.html.bak + git -C site commit -m '$(GITHUB_REF:refs/tags/v%=%)' index.html + git -C site push +.PHONY: site-publish From 1b12ef863ef533afbf5e47779994a66c25ffe561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Apr 2020 15:59:58 +0200 Subject: [PATCH 06/47] Simplify release documentation --- docs/releasing.md | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/docs/releasing.md b/docs/releasing.md index 00646a18a..dfaebd76a 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -1,27 +1,17 @@ # Releasing -## Test Locally - -`go test ./...` - -## Push new docs - -build docs locally: `make site` - -build and push docs to production: `make site-docs` - -## Release locally for debugging - -A local release can be created for testing without creating anything official on the release page. - -1. `env GH_OAUTH_CLIENT_SECRET= GH_OAUTH_CLIENT_ID= goreleaser --skip-validate --skip-publish --rm-dist` -2. Check and test files in `dist/` - ## Release to production This can all be done from your local terminal. -1. `git tag 'vVERSION_NUMBER' # example git tag 'v0.0.1'` -2. `git push origin vVERSION_NUMBER` -3. Wait a few minutes for the build to run and CI to pass. Look at the [actions tab](https://github.com/cli/cli/actions) to check the progress. -4. Go to and look at the release +1. `git tag v1.2.3` +2. `git push origin v1.2.3` +3. Wait a few minutes for the build to run +4. Check + +## Release locally for debugging + +A local release can be created for testing without creating anything official on the release page. + +1. `goreleaser --skip-validate --skip-publish --rm-dist` +2. Check and test files in `dist/` From 83502fdbae866b5cd5c8aeba4ef6608c80d3f419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 22 Apr 2020 16:13:16 +0200 Subject: [PATCH 07/47] Avoid failing if site docs are already up-to-date --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ffbd5bd7e..f2b4805c8 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,7 @@ site-docs: site for f in site/manual/gh*.md; do sed -i.bak -e '/^### SEE ALSO/,$$d' "$$f"; done rm -f site/manual/*.bak git -C site add 'manual/gh*.md' - git -C site commit -m 'update help docs' + git -C site commit -m 'update help docs' || true .PHONY: site-docs site-publish: site-docs From a4429aae41dab01f68563b0e6e53a3c844caffb7 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 10:00:33 -0700 Subject: [PATCH 08/47] rearrange code --- command/root.go | 73 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 20 deletions(-) diff --git a/command/root.go b/command/root.go index 60fe77daa..9c9b5e692 100644 --- a/command/root.go +++ b/command/root.go @@ -47,7 +47,7 @@ func init() { // TODO: // RootCmd.PersistentFlags().BoolP("verbose", "V", false, "enable verbose output") - RootCmd.SetHelpFunc(aTempHelpFuncfunc) + RootCmd.SetHelpFunc(rootHelpFunc) RootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { if err == pflag.ErrHelp { @@ -72,12 +72,8 @@ func (fe FlagError) Unwrap() error { // RootCmd is the entry point of command-line execution var RootCmd = &cobra.Command{ - Use: "gh", Short: "GitHub CLI", - Long: `Work seamlessly with GitHub from the command line. - -GitHub CLI is in early stages of development, and we'd love to hear your -feedback at `, + Long: `Work seamlessly with GitHub from the command line.`, SilenceErrors: true, SilenceUsage: true, @@ -209,28 +205,50 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interfac return baseRepo, nil } -type helpEntry struct { - Title string - Body string -} +func rootHelpFunc(command *cobra.Command, s []string) { + type helpEntry struct { + Title string + Body string + } -func aTempHelpFuncfunc(command *cobra.Command, s []string) { - out := colorableOut(command) - fmt.Fprint(out, "\nWork seamlessly with GitHub from the command line.\n\n") - - var commands []string + coreCommandNames := []string{"issue", "pr", "repo"} + var coreCommands []string + var additionalCommands []string for _, c := range command.Commands() { - s := c.Name() + ":" + strings.Repeat(" ", c.NamePadding()) + c.Short - commands = append(commands, s) + if c.Short == "" { + continue + } + s := " " + rpad(c.Name()+":", c.NamePadding()) + c.Short + if includes(coreCommandNames, c.Name()) { + coreCommands = append(coreCommands, s) + } else { + additionalCommands = append(additionalCommands, s) + } } helpEntries := []helpEntry{ + { + "", + command.Long}, {"USAGE", ` - gh [flags] - Commands are run inside of a GitHub repository.`}, - {"CORE COMMANDS", strings.Join(commands, "\n")}, + # most gh commands need to be run in a directory containing a GitHub repository. + gh [subcommand] [flags]`}, + {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, + {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, + {"FEEDBACK", ` + Fill out our feedback form + Open an issue using “gh issue create -R cli/cli`}, + {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, + {"EXAMPLES", ` + $ cd your-repository + $ gh pr list + $ gh pr checkout 321`}, + {"LEARN MORE", ` + Use "gh --help" for more information about a command. + Read the manual at `}, } + out := colorableOut(command) for _, e := range helpEntries { if e.Title != "" { fmt.Fprintln(out, utils.Bold(e.Title)) @@ -238,3 +256,18 @@ func aTempHelpFuncfunc(command *cobra.Command, s []string) { fmt.Fprintln(out, strings.TrimLeft(e.Body, "\n")+"\n") } } + +// rpad adds padding to the right of a string. +func rpad(s string, padding int) string { + template := fmt.Sprintf("%%-%ds ", padding) + return fmt.Sprintf(template, s) +} + +func includes(a []string, s string) bool { + for _, x := range a { + if x == s { + return true + } + } + return false +} From b5299ef5d73596b66b06f701379ea188f01ee3fe Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:46:51 -0700 Subject: [PATCH 09/47] pointy brackets ftw Co-Authored-By: Amanda Pinsker --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 9c9b5e692..150a9292c 100644 --- a/command/root.go +++ b/command/root.go @@ -232,7 +232,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { command.Long}, {"USAGE", ` # most gh commands need to be run in a directory containing a GitHub repository. - gh [subcommand] [flags]`}, + gh [flags]`}, {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, {"FEEDBACK", ` From 6d20283a7a66fa6622fd30cf1ca5e2b5439826fd Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:49:42 -0700 Subject: [PATCH 10/47] Move feedback to the bottom --- command/root.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/command/root.go b/command/root.go index 9c9b5e692..fbbb1b091 100644 --- a/command/root.go +++ b/command/root.go @@ -235,17 +235,17 @@ func rootHelpFunc(command *cobra.Command, s []string) { gh [subcommand] [flags]`}, {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, - {"FEEDBACK", ` - Fill out our feedback form - Open an issue using “gh issue create -R cli/cli`}, - {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, {"EXAMPLES", ` $ cd your-repository $ gh pr list $ gh pr checkout 321`}, {"LEARN MORE", ` Use "gh --help" for more information about a command. - Read the manual at `}, + Read the manual at `}, + {"FEEDBACK", ` + Fill out our feedback form + Open an issue using “gh issue create -R cli/cli`}, + {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, } out := colorableOut(command) From 833f8b4f2c8e6b6f540944aa168fad1c41095267 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:50:06 -0700 Subject: [PATCH 11/47] Update command/root.go Co-Authored-By: Amanda Pinsker --- command/root.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/root.go b/command/root.go index 150a9292c..1c02550f1 100644 --- a/command/root.go +++ b/command/root.go @@ -240,8 +240,8 @@ func rootHelpFunc(command *cobra.Command, s []string) { Open an issue using “gh issue create -R cli/cli`}, {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, {"EXAMPLES", ` - $ cd your-repository - $ gh pr list + $ gh issue create + $ gh repo clone $ gh pr checkout 321`}, {"LEARN MORE", ` Use "gh --help" for more information about a command. From 0d792573126b1ae9ba197bc2d5826d87e88af633 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:50:52 -0700 Subject: [PATCH 12/47] use backets Co-Authored-By: Amanda Pinsker --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 1c02550f1..77d81f115 100644 --- a/command/root.go +++ b/command/root.go @@ -244,7 +244,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { $ gh repo clone $ gh pr checkout 321`}, {"LEARN MORE", ` - Use "gh --help" for more information about a command. + Use "gh [command] [subcommand] --help" for more information about a command. Read the manual at `}, } From c064b07e6db27b21a2357be70c19e7d1a9a46455 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 13:51:35 -0700 Subject: [PATCH 13/47] Add quote --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index fbbb1b091..988e12ff8 100644 --- a/command/root.go +++ b/command/root.go @@ -244,7 +244,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { Read the manual at `}, {"FEEDBACK", ` Fill out our feedback form - Open an issue using “gh issue create -R cli/cli`}, + Open an issue using “gh issue create -R cli/cli”`}, {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, } From c9d5935ee33b732e2914e72bc60598eb239a63d3 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 14:00:08 -0700 Subject: [PATCH 14/47] move to use --- command/root.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/command/root.go b/command/root.go index 6f2927a61..4096306b1 100644 --- a/command/root.go +++ b/command/root.go @@ -76,6 +76,7 @@ func (fe FlagError) Unwrap() error { // RootCmd is the entry point of command-line execution var RootCmd = &cobra.Command{ + Use: "gh [flags]", Short: "GitHub CLI", Long: `Work seamlessly with GitHub from the command line.`, @@ -246,9 +247,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { { "", command.Long}, - {"USAGE", ` - # most gh commands need to be run in a directory containing a GitHub repository. - gh [subcommand] [flags]`}, + {"USAGE", command.UsageString()}, {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, {"EXAMPLES", ` From e68be52ed0a11e5d488750713fc685f4a644e41b Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 22 Apr 2020 14:03:44 -0700 Subject: [PATCH 15/47] Use the correct field --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 611022d2d..84c421d46 100644 --- a/command/root.go +++ b/command/root.go @@ -247,7 +247,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { { "", command.Long}, - {"USAGE", command.UsageString()}, + {"USAGE", command.Use}, {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, From cd93e5643c46b2c5c1841840c861fcf2f12a8b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 23 Apr 2020 13:04:40 +0200 Subject: [PATCH 16/47] Help goreleaser find the correct git tag during release --- .github/workflows/releases.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 2f9908167..67486f3aa 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -17,6 +17,7 @@ jobs: go-version: 1.13 - name: Generate changelog run: | + echo ::set-env name=GORELEASER_CURRENT_TAG::${GITHUB_REF#refs/tags/} git fetch --unshallow script/changelog | tee CHANGELOG.md - name: Run GoReleaser From 84ac3d412e42e28c491c655a10930e3b74c528b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 23 Apr 2020 15:41:37 +0200 Subject: [PATCH 17/47] Fix gh updater mechanism for unauthenticated users Per code documentation: "BasicClient returns an API client that borrows from but does not depend on user configuration". This means that any errors while reading `oauth_token` should be tolerated. --- command/root.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/command/root.go b/command/root.go index 40be6a7e0..a6ea851ce 100644 --- a/command/root.go +++ b/command/root.go @@ -111,20 +111,11 @@ func BasicClient() (*api.Client, error) { } opts = append(opts, api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version))) - c, err := config.ParseDefaultConfig() - if err != nil { - return nil, err + if c, err := config.ParseDefaultConfig(); err == nil { + if token, _ := c.Get(defaultHostname, "oauth_token"); token != "" { + opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token))) + } } - - token, err := c.Get(defaultHostname, "oauth_token") - if err != nil { - return nil, err - } - if token == "" { - return nil, fmt.Errorf("no oauth_token set in config") - } - - opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token))) return api.NewClient(opts...), nil } From 3aaa231cc5388ecef05a218314f308cb49fcf7f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 23 Apr 2020 18:15:20 +0200 Subject: [PATCH 18/47] Guide user through re-authorization flow if `read:org` scope is missing How this works for people with existing OAuth tokens: $ gh issue list -L1 Notice: additional authorization required Press Enter to open github.com in your browser... [auth flow in the browser...] Authentication complete. Press Enter to continue... Showing 1 of 132 issues in cli/cli ... Users of Personal Access Tokens get a different notice: Warning: gh now requires the `read:org` OAuth scope. Visit https://github.com/settings/tokens and edit your token to enable `read:org` or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN` --- api/client.go | 27 ++++++++++--------- command/root.go | 37 ++++++++++++++++++++++++-- internal/config/config_setup.go | 47 ++++++++++++++++++++++----------- 3 files changed, 81 insertions(+), 30 deletions(-) diff --git a/api/client.go b/api/client.go index e1fef7219..d7b981bee 100644 --- a/api/client.go +++ b/api/client.go @@ -7,7 +7,6 @@ import ( "io" "io/ioutil" "net/http" - "os" "regexp" "strings" @@ -38,6 +37,16 @@ func AddHeader(name, value string) ClientOption { } } +// AddHeaderFunc is an AddHeader that gets the string value from a function +func AddHeaderFunc(name string, value func() string) ClientOption { + return func(tr http.RoundTripper) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + req.Header.Add(name, value()) + return tr.RoundTrip(req) + }} + } +} + // VerboseLog enables request/response logging within a RoundTripper func VerboseLog(out io.Writer, logTraffic bool, colorize bool) ClientOption { logger := &httpretty.Logger{ @@ -67,15 +76,15 @@ func ReplaceTripper(tr http.RoundTripper) ClientOption { var issuedScopesWarning bool // CheckScopes checks whether an OAuth scope is present in a response -func CheckScopes(wantedScope string) ClientOption { +func CheckScopes(wantedScope string, cb func(string) error) ClientOption { return func(tr http.RoundTripper) http.RoundTripper { return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { res, err := tr.RoundTrip(req) - if err != nil || issuedScopesWarning { + if err != nil || res.StatusCode > 299 || issuedScopesWarning { return res, err } - isApp := res.Header.Get("X-Oauth-Client-Id") != "" + appID := res.Header.Get("X-Oauth-Client-Id") hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",") hasWanted := false @@ -87,14 +96,8 @@ func CheckScopes(wantedScope string) ClientOption { } if !hasWanted { - fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.") - // TODO: offer to take the person through the authentication flow again? - // TODO: retry the original request if it was a read? - if isApp { - fmt.Fprintln(os.Stderr, "To re-authenticate, please `rm ~/.config/gh/config.yml` and try again.") - } else { - // the person has pasted a Personal Access Token - fmt.Fprintln(os.Stderr, "Re-generate your token in `rm ~/.config/gh/config.yml` and try again.") + if err := cb(appID); err != nil { + return res, err } issuedScopesWarning = true } diff --git a/command/root.go b/command/root.go index 1624a57e0..8c0cbd046 100644 --- a/command/root.go +++ b/command/root.go @@ -142,13 +142,46 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { if err != nil { return nil, err } + var opts []api.ClientOption if verbose := os.Getenv("DEBUG"); verbose != "" { opts = append(opts, apiVerboseLog()) } + + getAuthValue := func() string { + return fmt.Sprintf("token %s", token) + } + checkScopesFunc := func(appID string) error { + if config.IsGitHubApp(appID) && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { + newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required") + if err != nil { + return err + } + cfg, err := ctx.Config() + if err != nil { + return err + } + _ = cfg.Set(defaultHostname, "oauth_token", newToken) + _ = cfg.Set(defaultHostname, "user", loginHandle) + // update config file on disk + err = cfg.Write() + if err != nil { + return err + } + // update configuration in memory + token = newToken + config.AuthFlowComplete() + } else { + fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.") + fmt.Fprintln(os.Stderr, "Visit https://github.com/settings/tokens and edit your token to enable `read:org`") + fmt.Fprintln(os.Stderr, "or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`") + } + return nil + } + opts = append(opts, - api.CheckScopes("read:org"), - api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), + api.CheckScopes("read:org", checkScopesFunc), + api.AddHeaderFunc("Authorization", getAuthValue), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), // antiope-preview: Checks api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"), diff --git a/internal/config/config_setup.go b/internal/config/config_setup.go index 9564ecd4c..4d2a929cb 100644 --- a/internal/config/config_setup.go +++ b/internal/config/config_setup.go @@ -24,9 +24,14 @@ var ( oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b" ) -// TODO: have a conversation about whether this belongs in the "context" package -// FIXME: make testable -func setupConfigFile(filename string) (Config, error) { +// IsGitHubApp reports whether an OAuth app is "GitHub CLI" or "GitHub CLI (dev)" +func IsGitHubApp(id string) bool { + // this intentionally doesn't use `oauthClientID` because that is a variable + // that can potentially be changed at build time via GH_OAUTH_CLIENT_ID + return id == "178c6fc778ccc68e1d6a" || id == "4d747ba5675d5d66553f" +} + +func AuthFlow(notice string) (string, string, error) { var verboseStream io.Writer if strings.Contains(os.Getenv("DEBUG"), "oauth") { verboseStream = os.Stderr @@ -43,15 +48,30 @@ func setupConfigFile(filename string) (Config, error) { VerboseStream: verboseStream, } - fmt.Fprintln(os.Stderr, "Notice: authentication required") + fmt.Fprintln(os.Stderr, notice) fmt.Fprintf(os.Stderr, "Press Enter to open %s in your browser... ", flow.Hostname) _ = waitForEnter(os.Stdin) token, err := flow.ObtainAccessToken() if err != nil { - return nil, err + return "", "", err } userLogin, err := getViewer(token) + if err != nil { + return "", "", err + } + + return token, userLogin, nil +} + +func AuthFlowComplete() { + fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ") + _ = waitForEnter(os.Stdin) +} + +// FIXME: make testable +func setupConfigFile(filename string) (Config, error) { + token, userLogin, err := AuthFlow("Notice: authentication required") if err != nil { return nil, err } @@ -62,9 +82,9 @@ func setupConfigFile(filename string) (Config, error) { } yamlHosts := map[string]map[string]string{} - yamlHosts[flow.Hostname] = map[string]string{} - yamlHosts[flow.Hostname]["user"] = userLogin - yamlHosts[flow.Hostname]["oauth_token"] = token + yamlHosts[oauthHost] = map[string]string{} + yamlHosts[oauthHost]["user"] = userLogin + yamlHosts[oauthHost]["oauth_token"] = token defaultConfig := yamlConfig{ Hosts: yamlHosts, @@ -85,14 +105,9 @@ func setupConfigFile(filename string) (Config, error) { if err != nil { return nil, err } - n, err := cfgFile.Write(yamlData) - if err == nil && n < len(yamlData) { - err = io.ErrShortWrite - } - - if err == nil { - fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ") - _ = waitForEnter(os.Stdin) + _, err = cfgFile.Write(yamlData) + if err != nil { + return nil, err } // TODO cleaner error handling? this "should" always work given that we /just/ wrote the file... From 700f86c5bce165a93aa3ecbbc850363ea8e530d8 Mon Sep 17 00:00:00 2001 From: Leonardo Melo Date: Fri, 24 Apr 2020 08:49:03 -0300 Subject: [PATCH 19/47] Update README Add missing `gh config` command usage to README. Also, remove a trailing space in the `Availability` section, hehe. :smiley: --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5c96a685f..c4c0e9a5d 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ the terminal next to where you are already working with `git` and your code. ## Availability -While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It does not currently support repositories hosted on GitHub Enterprise Server or other hosting providers. We are planning support for GitHub Enterprise Server after GitHub CLI is out of beta (likely toward the end of 2020), and we want to ensure that the API endpoints we use are more widely available for GHES versions that most GitHub customers are on. +While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It does not currently support repositories hosted on GitHub Enterprise Server or other hosting providers. We are planning support for GitHub Enterprise Server after GitHub CLI is out of beta (likely toward the end of 2020), and we want to ensure that the API endpoints we use are more widely available for GHES versions that most GitHub customers are on. ## We need your feedback @@ -22,6 +22,7 @@ And if you spot bugs or have features that you'd really like to see in `gh`, ple - `gh pr [status, list, view, checkout, create]` - `gh issue [status, list, view, create]` - `gh repo [view, create, clone, fork]` +- `gh config [get, set]` - `gh help` ## Documentation From fab18ed4a95266530c4fa539460699b367c73522 Mon Sep 17 00:00:00 2001 From: rista404 Date: Sat, 25 Apr 2020 22:57:03 +0200 Subject: [PATCH 20/47] Show awaiting triage when project col is empty --- command/issue.go | 6 +++++- command/pr.go | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/command/issue.go b/command/issue.go index 2f4e145af..f2cb4ba47 100644 --- a/command/issue.go +++ b/command/issue.go @@ -504,7 +504,11 @@ func issueProjectList(issue api.Issue) string { projectNames := make([]string, 0, len(issue.ProjectCards.Nodes)) for _, project := range issue.ProjectCards.Nodes { - projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, project.Column.Name)) + colName := project.Column.Name + if colName == "" { + colName = "Awaiting triage" + } + projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) } list := strings.Join(projectNames, ", ") diff --git a/command/pr.go b/command/pr.go index a5b1f0079..009548061 100644 --- a/command/pr.go +++ b/command/pr.go @@ -417,7 +417,11 @@ func prProjectList(pr api.PullRequest) string { projectNames := make([]string, 0, len(pr.ProjectCards.Nodes)) for _, project := range pr.ProjectCards.Nodes { - projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, project.Column.Name)) + colName := project.Column.Name + if colName == "" { + colName = "Awaiting triage" + } + projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) } list := strings.Join(projectNames, ", ") From a2a6036c6543973a4940a905263e87c63d742dc1 Mon Sep 17 00:00:00 2001 From: rista404 Date: Sat, 25 Apr 2020 23:12:51 +0200 Subject: [PATCH 21/47] Add tests --- command/issue_test.go | 2 +- command/pr_test.go | 2 +- test/fixtures/issueView_previewWithMetadata.json | 8 ++++++++ test/fixtures/prViewPreviewWithMetadataByNumber.json | 8 ++++++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index b7f91031d..e9229ac24 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -312,7 +312,7 @@ func TestIssueView_Preview(t *testing.T) { `Open • marseilles opened about 292 years ago • 9 comments`, `Assignees: marseilles, monaco\n`, `Labels: one, two, three, four, five\n`, - `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, + `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `Milestone: uluru\n`, `bold story`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, diff --git a/command/pr_test.go b/command/pr_test.go index 074420deb..ba0989645 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -429,7 +429,7 @@ func TestPRView_Preview(t *testing.T) { `Open • nobody wants to merge 12 commits into master from blueberries`, `Assignees: marseilles, monaco\n`, `Labels: one, two, three, four, five\n`, - `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, + `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `Milestone: uluru\n`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, diff --git a/test/fixtures/issueView_previewWithMetadata.json b/test/fixtures/issueView_previewWithMetadata.json index a420eec66..246bbd77b 100644 --- a/test/fixtures/issueView_previewWithMetadata.json +++ b/test/fixtures/issueView_previewWithMetadata.json @@ -67,6 +67,14 @@ "column": { "name": "column C" } + }, + { + "project": { + "name": "Project 4" + }, + "column": { + "name": "" + } } ], "totalcount": 3 diff --git a/test/fixtures/prViewPreviewWithMetadataByNumber.json b/test/fixtures/prViewPreviewWithMetadataByNumber.json index de0da816a..e49bcdc85 100644 --- a/test/fixtures/prViewPreviewWithMetadataByNumber.json +++ b/test/fixtures/prViewPreviewWithMetadataByNumber.json @@ -66,6 +66,14 @@ "column": { "name": "column C" } + }, + { + "project": { + "name": "Project 4" + }, + "column": { + "name": "" + } } ], "totalcount": 3 From 3d2ee3c9b27ce1c940ef3659acc69393feced9dd Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 27 Apr 2020 10:07:47 -0700 Subject: [PATCH 22/47] fix whitespace --- command/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/root.go b/command/root.go index 84c421d46..c567394a7 100644 --- a/command/root.go +++ b/command/root.go @@ -257,7 +257,7 @@ func rootHelpFunc(command *cobra.Command, s []string) { $ gh pr checkout 321`}, {"LEARN MORE", ` Use "gh --help" for more information about a command. - Read the manual at `}, + Read the manual at `}, {"FEEDBACK", ` Fill out our feedback form Open an issue using “gh issue create -R cli/cli”`}, From e5fc3e9bbd281aeaaf4d07829f0fa81dfc0ac3c9 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 09:11:19 -0700 Subject: [PATCH 23/47] Update issue_test.go Add test for closing issue Co-Authored-By: Nate Smith --- command/issue_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/command/issue_test.go b/command/issue_test.go index b7f91031d..2185fc390 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -680,3 +680,25 @@ func TestIssueStateTitleWithColor(t *testing.T) { }) } } + +func TestIssueClose(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/issueClose.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand(issueStatusCmd, "issue close 13") + if err != nil { + t.Errorf("error running command `issue close`: %v", err) + } + + r := regexp.MustCompile(`WHATEVER!`) + + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } +} From ad48d8b3c3dc9ff6053f20c662c18f0700ec5f1e Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 09:13:08 -0700 Subject: [PATCH 24/47] Add issue close Co-Authored-By: Nate Smith --- api/queries_issue.go | 23 +++++++++++++++++++++++ command/issue.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/api/queries_issue.go b/api/queries_issue.go index cc01c039f..b9f569a08 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -1,10 +1,12 @@ package api import ( + "context" "fmt" "time" "github.com/cli/cli/internal/ghrepo" + "github.com/shurcooL/githubv4" ) type IssuesPayload struct { @@ -356,3 +358,24 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e return &resp.Repository.Issue, nil } + +func IssueClose(client *Client, repo ghrepo.Interface, issueNumber int) error { + var mutation struct { + closeIssue struct { + } `graphql:"closeIssue(input: $input)"` + } + + input := githubv4.CloseIssueInput{ + IssueID: issueNumber, + } + + v4 := githubv4.NewClient(client.http) + err := v4.Mutate(context.Background(), &mutation, input, nil) + if err != nil { + return nil + } + + fmt.Printf("%v\n", mutation) + + return nil +} diff --git a/command/issue.go b/command/issue.go index 2f4e145af..0c2aae11b 100644 --- a/command/issue.go +++ b/command/issue.go @@ -39,6 +39,8 @@ func init() { issueCmd.AddCommand(issueViewCmd) issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser") + + issueCmd.AddCommand(issueCloseCmd) } var issueCmd = &cobra.Command{ @@ -79,6 +81,12 @@ var issueViewCmd = &cobra.Command{ With '--web', open the issue in a web browser instead.`, RunE: issueView, } +var issueCloseCmd = &cobra.Command{ + Use: "close ", + Short: "close and issue issues", + Args: cobra.ExactArgs(1), + RunE: issueClose, +} func issueList(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) @@ -514,6 +522,31 @@ func issueProjectList(issue api.Issue) string { return list } +func issueClose(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + issueNumber, err := strconv.Atoi(args[0]) + if err != nil { + return fmt.Errorf("expected a number but: %w", err) + } + + err = api.IssueClose(apiClient, baseRepo, issueNumber) + if err != nil { + return fmt.Errorf("API call failed:%w", err) + } + + return nil +} + func displayURL(urlStr string) string { u, err := url.Parse(urlStr) if err != nil { From a2f0cc6de74a6b73ba1a1914a9ee7ea0ccaea261 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 09:31:19 -0700 Subject: [PATCH 25/47] Issue close works Co-Authored-By: Nate Smith --- api/queries_issue.go | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index b9f569a08..7a342ccf3 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -22,6 +22,7 @@ type IssuesAndTotalCount struct { // Ref. https://developer.github.com/v4/object/issue/ type Issue struct { + ID string Number int Title string URL string @@ -298,6 +299,7 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e repository(owner: $owner, name: $repo) { hasIssuesEnabled issue(number: $issue_number) { + id title state body @@ -360,22 +362,29 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e } func IssueClose(client *Client, repo ghrepo.Interface, issueNumber int) error { + issue, err := IssueByNumber(client, repo, issueNumber) + if err != nil { + return fmt.Errorf("Faile to find issue #%d: %w", issueNumber, err) + } + var mutation struct { - closeIssue struct { + CloseIssue struct { + Issue struct { + ID githubv4.ID + } } `graphql:"closeIssue(input: $input)"` } input := githubv4.CloseIssueInput{ - IssueID: issueNumber, + IssueID: issue.ID, } v4 := githubv4.NewClient(client.http) - err := v4.Mutate(context.Background(), &mutation, input, nil) - if err != nil { - return nil - } + err = v4.Mutate(context.Background(), &mutation, input, nil) - fmt.Printf("%v\n", mutation) + if err != nil { + return err + } return nil } From 7ba66159194806fdacc169475b595b60cf7e9148 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 09:51:28 -0700 Subject: [PATCH 26/47] Close test now works Co-Authored-By: Nate Smith --- command/issue.go | 2 ++ command/issue_test.go | 15 +++++++++++---- test/fixtures/issueClose.json | 1 + 3 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/issueClose.json diff --git a/command/issue.go b/command/issue.go index 0c2aae11b..b919cc843 100644 --- a/command/issue.go +++ b/command/issue.go @@ -544,6 +544,8 @@ func issueClose(cmd *cobra.Command, args []string) error { return fmt.Errorf("API call failed:%w", err) } + fmt.Fprintf(colorableErr(cmd), "%s closed issue #%d\n", utils.Green("✔"), issueNumber) + return nil } diff --git a/command/issue_test.go b/command/issue_test.go index 2185fc390..5ba66f3ea 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -686,19 +686,26 @@ func TestIssueClose(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "id": "A-VERY-IMPORTANT-ID"} + } } } + `)) + jsonFile, _ := os.Open("../test/fixtures/issueClose.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := RunCommand(issueStatusCmd, "issue close 13") + output, err := RunCommand(issueCloseCmd, "issue close 13") if err != nil { t.Errorf("error running command `issue close`: %v", err) } - r := regexp.MustCompile(`WHATEVER!`) + r := regexp.MustCompile(`closed issue #13`) - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + if !r.MatchString(output.Stderr()) { + t.Errorf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) return } } diff --git a/test/fixtures/issueClose.json b/test/fixtures/issueClose.json new file mode 100644 index 000000000..047f1a6a8 --- /dev/null +++ b/test/fixtures/issueClose.json @@ -0,0 +1 @@ +{ "id": "hi" } From bf94a3161990e202d0f5af6ab4ed56753b9d2226 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 10:05:56 -0700 Subject: [PATCH 27/47] not on issues disabled Co-Authored-By: Nate Smith --- api/queries_issue.go | 15 ++++++++++++--- command/issue_test.go | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 7a342ccf3..9443ab8bf 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -2,6 +2,7 @@ package api import ( "context" + "errors" "fmt" "time" @@ -64,6 +65,10 @@ type Issue struct { } } +type IssuesDisabledError struct { + error +} + const fragments = ` fragment issue on Issue { number @@ -355,7 +360,8 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e } if !resp.Repository.HasIssuesEnabled { - return nil, fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo)) + + return nil, &IssuesDisabledError{fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo))} } return &resp.Repository.Issue, nil @@ -363,8 +369,11 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e func IssueClose(client *Client, repo ghrepo.Interface, issueNumber int) error { issue, err := IssueByNumber(client, repo, issueNumber) - if err != nil { - return fmt.Errorf("Faile to find issue #%d: %w", issueNumber, err) + var idErr *IssuesDisabledError + if errors.As(err, &idErr) { + return fmt.Errorf("issues disabled for %s", ghrepo.FullName(repo)) + } else if err != nil { + return fmt.Errorf("failed to find issue #%d: %w", issueNumber, err) } var mutation struct { diff --git a/command/issue_test.go b/command/issue_test.go index 5ba66f3ea..18976a4db 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -709,3 +709,26 @@ func TestIssueClose(t *testing.T) { return } } + +func TestIssueClose_issuesDisabled(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } } + `)) + + _, err := RunCommand(issueCloseCmd, "issue close 13") + if err == nil { + t.Fatalf("expected error when issues are disabled") + } + + if !strings.Contains(err.Error(), "issues disabled") { + t.Fatalf("got unexpected error: %s", err) + } + + return +} From f22f584e8d35fc6bb4b472ac64a7be3370bf2cf0 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 11:53:25 -0700 Subject: [PATCH 28/47] Handle closed issues --- api/queries_issue.go | 16 +++++++++++----- command/issue.go | 8 ++++++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 9443ab8bf..153336ea4 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -28,6 +28,7 @@ type Issue struct { Title string URL string State string + Closed bool Body string CreatedAt time.Time UpdatedAt time.Time @@ -307,6 +308,7 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e id title state + closed body author { login @@ -367,13 +369,17 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e return &resp.Repository.Issue, nil } -func IssueClose(client *Client, repo ghrepo.Interface, issueNumber int) error { +func IssueClose(client *Client, repo ghrepo.Interface, issueNumber int) (alreadyClosed bool, _ error) { issue, err := IssueByNumber(client, repo, issueNumber) var idErr *IssuesDisabledError if errors.As(err, &idErr) { - return fmt.Errorf("issues disabled for %s", ghrepo.FullName(repo)) + return false, fmt.Errorf("issues disabled for %s", ghrepo.FullName(repo)) } else if err != nil { - return fmt.Errorf("failed to find issue #%d: %w", issueNumber, err) + return false, fmt.Errorf("failed to find issue #%d: %w", issueNumber, err) + } + + if issue.Closed { + return true, nil } var mutation struct { @@ -392,8 +398,8 @@ func IssueClose(client *Client, repo ghrepo.Interface, issueNumber int) error { err = v4.Mutate(context.Background(), &mutation, input, nil) if err != nil { - return err + return false, err } - return nil + return false, nil } diff --git a/command/issue.go b/command/issue.go index b919cc843..d7562e875 100644 --- a/command/issue.go +++ b/command/issue.go @@ -539,12 +539,16 @@ func issueClose(cmd *cobra.Command, args []string) error { return fmt.Errorf("expected a number but: %w", err) } - err = api.IssueClose(apiClient, baseRepo, issueNumber) + alreadyClosed, err := api.IssueClose(apiClient, baseRepo, issueNumber) if err != nil { return fmt.Errorf("API call failed:%w", err) } - fmt.Fprintf(colorableErr(cmd), "%s closed issue #%d\n", utils.Green("✔"), issueNumber) + if alreadyClosed { + fmt.Fprintf(colorableErr(cmd), "%s issue #%d is already closed\n", utils.Yellow("!"), issueNumber) + } else { + fmt.Fprintf(colorableErr(cmd), "%s closed issue #%d\n", utils.Green("✔"), issueNumber) + } return nil } From 660cce7790fb468887982ced440ff4291733aa83 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 12:15:33 -0700 Subject: [PATCH 29/47] Take a URL --- api/queries_issue.go | 21 ++++----------------- command/issue.go | 23 ++++++++++++++--------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 153336ea4..ba9b43d65 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -2,7 +2,6 @@ package api import ( "context" - "errors" "fmt" "time" @@ -369,19 +368,7 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e return &resp.Repository.Issue, nil } -func IssueClose(client *Client, repo ghrepo.Interface, issueNumber int) (alreadyClosed bool, _ error) { - issue, err := IssueByNumber(client, repo, issueNumber) - var idErr *IssuesDisabledError - if errors.As(err, &idErr) { - return false, fmt.Errorf("issues disabled for %s", ghrepo.FullName(repo)) - } else if err != nil { - return false, fmt.Errorf("failed to find issue #%d: %w", issueNumber, err) - } - - if issue.Closed { - return true, nil - } - +func IssueClose(client *Client, repo ghrepo.Interface, issue Issue) error { var mutation struct { CloseIssue struct { Issue struct { @@ -395,11 +382,11 @@ func IssueClose(client *Client, repo ghrepo.Interface, issueNumber int) (already } v4 := githubv4.NewClient(client.http) - err = v4.Mutate(context.Background(), &mutation, input, nil) + err := v4.Mutate(context.Background(), &mutation, input, nil) if err != nil { - return false, err + return err } - return false, nil + return nil } diff --git a/command/issue.go b/command/issue.go index d7562e875..02a378e30 100644 --- a/command/issue.go +++ b/command/issue.go @@ -534,23 +534,28 @@ func issueClose(cmd *cobra.Command, args []string) error { return err } - issueNumber, err := strconv.Atoi(args[0]) - if err != nil { - return fmt.Errorf("expected a number but: %w", err) + issue, err := issueFromArg(apiClient, baseRepo, args[0]) + var idErr *api.IssuesDisabledError + if errors.As(err, &idErr) { + return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo)) + } else if err != nil { + return fmt.Errorf("failed to find issue #%d: %w", issue.Number, err) } - alreadyClosed, err := api.IssueClose(apiClient, baseRepo, issueNumber) + if issue.Closed { + fmt.Fprintf(colorableErr(cmd), "%s issue #%d is already closed\n", utils.Yellow("!"), issue.Number) + return nil + } + + err = api.IssueClose(apiClient, baseRepo, *issue) if err != nil { return fmt.Errorf("API call failed:%w", err) } - if alreadyClosed { - fmt.Fprintf(colorableErr(cmd), "%s issue #%d is already closed\n", utils.Yellow("!"), issueNumber) - } else { - fmt.Fprintf(colorableErr(cmd), "%s closed issue #%d\n", utils.Green("✔"), issueNumber) - } + fmt.Fprintf(colorableErr(cmd), "%s closed issue #%d\n", utils.Green("✔"), issue.Number) return nil + } func displayURL(urlStr string) string { From 99940fa0627d175b2c31d4438d8564d7e217efc1 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 12:15:37 -0700 Subject: [PATCH 30/47] Update test --- command/issue_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/issue_test.go b/command/issue_test.go index 18976a4db..ea3edbe2a 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -689,7 +689,7 @@ func TestIssueClose(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "id": "A-VERY-IMPORTANT-ID"} + "issue": { "number": 13} } } } `)) From 69b58b95a3dee5f29f813f38282200d9e6a5b407 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 12:25:25 -0700 Subject: [PATCH 31/47] Fix linter errors --- command/issue_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index ea3edbe2a..e723d665f 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -699,14 +699,13 @@ func TestIssueClose(t *testing.T) { output, err := RunCommand(issueCloseCmd, "issue close 13") if err != nil { - t.Errorf("error running command `issue close`: %v", err) + t.Fatalf("error running command `issue close`: %v", err) } r := regexp.MustCompile(`closed issue #13`) if !r.MatchString(output.Stderr()) { - t.Errorf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - return + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) } } @@ -729,6 +728,4 @@ func TestIssueClose_issuesDisabled(t *testing.T) { if !strings.Contains(err.Error(), "issues disabled") { t.Fatalf("got unexpected error: %s", err) } - - return } From 426c1e3460efd74459369f171aca2b4b6df51838 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 12:50:39 -0700 Subject: [PATCH 32/47] Make the checkmark red --- command/issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/issue.go b/command/issue.go index 02a378e30..45d52ab60 100644 --- a/command/issue.go +++ b/command/issue.go @@ -552,7 +552,7 @@ func issueClose(cmd *cobra.Command, args []string) error { return fmt.Errorf("API call failed:%w", err) } - fmt.Fprintf(colorableErr(cmd), "%s closed issue #%d\n", utils.Green("✔"), issue.Number) + fmt.Fprintf(colorableErr(cmd), "%s closed issue #%d\n", utils.Red("✔"), issue.Number) return nil From ab715638a4340423866291516081c84cfec48ca6 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 12:51:06 -0700 Subject: [PATCH 33/47] Case changes --- command/issue.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/issue.go b/command/issue.go index 45d52ab60..daa06e180 100644 --- a/command/issue.go +++ b/command/issue.go @@ -543,7 +543,7 @@ func issueClose(cmd *cobra.Command, args []string) error { } if issue.Closed { - fmt.Fprintf(colorableErr(cmd), "%s issue #%d is already closed\n", utils.Yellow("!"), issue.Number) + fmt.Fprintf(colorableErr(cmd), "%s Issue #%d is already closed\n", utils.Yellow("!"), issue.Number) return nil } @@ -552,7 +552,7 @@ func issueClose(cmd *cobra.Command, args []string) error { return fmt.Errorf("API call failed:%w", err) } - fmt.Fprintf(colorableErr(cmd), "%s closed issue #%d\n", utils.Red("✔"), issue.Number) + fmt.Fprintf(colorableErr(cmd), "%s Closed issue #%d\n", utils.Red("✔"), issue.Number) return nil From 1b37681a6f08ce759d6a778967c4dc8eaab64b81 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 28 Apr 2020 12:57:24 -0700 Subject: [PATCH 34/47] Fix test --- command/issue_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/issue_test.go b/command/issue_test.go index e723d665f..1415386ba 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -702,7 +702,7 @@ func TestIssueClose(t *testing.T) { t.Fatalf("error running command `issue close`: %v", err) } - r := regexp.MustCompile(`closed issue #13`) + r := regexp.MustCompile(`Closed issue #13`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) From fb7b5446d1e40ea43da09bd7ec85a0addaee3d4a Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Tue, 28 Apr 2020 20:19:52 -0500 Subject: [PATCH 35/47] Update command/root.go Co-Authored-By: Corey Johnson --- command/root.go | 1 + 1 file changed, 1 insertion(+) diff --git a/command/root.go b/command/root.go index 8c0cbd046..fcb3dca2b 100644 --- a/command/root.go +++ b/command/root.go @@ -151,6 +151,7 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { getAuthValue := func() string { return fmt.Sprintf("token %s", token) } + checkScopesFunc := func(appID string) error { if config.IsGitHubApp(appID) && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required") From 83ec49582b90dd074f89608d1212fdf773784961 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 29 Apr 2020 11:31:40 -0700 Subject: [PATCH 36/47] get rid of fixture --- command/issue_test.go | 4 +--- test/fixtures/issueClose.json | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) delete mode 100644 test/fixtures/issueClose.json diff --git a/command/issue_test.go b/command/issue_test.go index 1415386ba..cddab4efa 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -693,9 +693,7 @@ func TestIssueClose(t *testing.T) { } } } `)) - jsonFile, _ := os.Open("../test/fixtures/issueClose.json") - defer jsonFile.Close() - http.StubResponse(200, jsonFile) + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) output, err := RunCommand(issueCloseCmd, "issue close 13") if err != nil { diff --git a/test/fixtures/issueClose.json b/test/fixtures/issueClose.json deleted file mode 100644 index 047f1a6a8..000000000 --- a/test/fixtures/issueClose.json +++ /dev/null @@ -1 +0,0 @@ -{ "id": "hi" } From c78c30b32fc73703e3ad9abed02c655f24cc646e Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 29 Apr 2020 11:33:16 -0700 Subject: [PATCH 37/47] Add already closed test --- command/issue_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/command/issue_test.go b/command/issue_test.go index cddab4efa..bddef2e30 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -707,6 +707,32 @@ func TestIssueClose(t *testing.T) { } } +func TestIssueClose_alreadyClosed(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 13, "closed": true} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand(issueCloseCmd, "issue close 13") + if err != nil { + t.Fatalf("error running command `issue close`: %v", err) + } + + r := regexp.MustCompile(`#13 is already closed`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + func TestIssueClose_issuesDisabled(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() From 15a6225f05bf1fef68fe40ed755e3acb464ef158 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 29 Apr 2020 14:17:16 -0500 Subject: [PATCH 38/47] totally inelegant approach to hopefully stopping flakey tests --- command/repo.go | 4 ++-- command/repo_test.go | 21 +++++++++++++++++++++ utils/utils.go | 10 ++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/command/repo.go b/command/repo.go index b96a48164..a611643be 100644 --- a/command/repo.go +++ b/command/repo.go @@ -379,11 +379,11 @@ func repoFork(cmd *cobra.Command, args []string) error { loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(repoToFork))) + utils.Gray("...") s.Suffix = " " + loading s.FinalMSG = utils.Gray(fmt.Sprintf("- %s\n", loading)) - s.Start() + utils.StartSpinner(s) forkedRepo, err := api.ForkRepo(apiClient, repoToFork) if err != nil { - s.Stop() + utils.StopSpinner(s) return fmt.Errorf("failed to fork: %w", err) } diff --git a/command/repo_test.go b/command/repo_test.go index 118e365d5..3e9fca954 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -11,12 +11,24 @@ import ( "testing" "time" + "github.com/briandowns/spinner" + "github.com/cli/cli/context" "github.com/cli/cli/internal/run" "github.com/cli/cli/test" + "github.com/cli/cli/utils" ) +func stubSpinner() { + // not bothering with teardown since we never want spinners when doing tests + utils.StartSpinner = func(_ *spinner.Spinner) { + } + utils.StopSpinner = func(_ *spinner.Spinner) { + } +} + func TestRepoFork_already_forked(t *testing.T) { + stubSpinner() initContext = func() context.Context { ctx := context.NewBlank() ctx.SetBaseRepo("OWNER/REPO") @@ -42,6 +54,7 @@ func TestRepoFork_already_forked(t *testing.T) { } func TestRepoFork_reuseRemote(t *testing.T) { + stubSpinner() initContext = func() context.Context { ctx := context.NewBlank() ctx.SetBaseRepo("OWNER/REPO") @@ -77,6 +90,7 @@ func stubSince(d time.Duration) func() { } func TestRepoFork_in_parent(t *testing.T) { + stubSpinner() initBlankContext("", "OWNER/REPO", "master") defer stubSince(2 * time.Second)() http := initFakeHTTP() @@ -98,6 +112,7 @@ func TestRepoFork_in_parent(t *testing.T) { } func TestRepoFork_outside(t *testing.T) { + stubSpinner() tests := []struct { name string args string @@ -134,6 +149,7 @@ func TestRepoFork_outside(t *testing.T) { } func TestRepoFork_in_parent_yes(t *testing.T) { + stubSpinner() initBlankContext("", "OWNER/REPO", "master") defer stubSince(2 * time.Second)() http := initFakeHTTP() @@ -168,6 +184,7 @@ func TestRepoFork_in_parent_yes(t *testing.T) { } func TestRepoFork_outside_yes(t *testing.T) { + stubSpinner() defer stubSince(2 * time.Second)() http := initFakeHTTP() defer http.StubWithFixture(200, "forkResult.json")() @@ -194,6 +211,7 @@ func TestRepoFork_outside_yes(t *testing.T) { } func TestRepoFork_outside_survey_yes(t *testing.T) { + stubSpinner() defer stubSince(2 * time.Second)() http := initFakeHTTP() defer http.StubWithFixture(200, "forkResult.json")() @@ -227,6 +245,7 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { } func TestRepoFork_outside_survey_no(t *testing.T) { + stubSpinner() defer stubSince(2 * time.Second)() http := initFakeHTTP() defer http.StubWithFixture(200, "forkResult.json")() @@ -261,6 +280,7 @@ func TestRepoFork_outside_survey_no(t *testing.T) { } func TestRepoFork_in_parent_survey_yes(t *testing.T) { + stubSpinner() initBlankContext("", "OWNER/REPO", "master") defer stubSince(2 * time.Second)() http := initFakeHTTP() @@ -303,6 +323,7 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { } func TestRepoFork_in_parent_survey_no(t *testing.T) { + stubSpinner() initBlankContext("", "OWNER/REPO", "master") defer stubSince(2 * time.Second)() http := initFakeHTTP() diff --git a/utils/utils.go b/utils/utils.go index 2efc5a95d..c84860d9e 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -74,6 +74,16 @@ func Humanize(s string) string { return strings.Map(h, s) } +// We do this so we can stub out the spinner in tests -- it made things really flakey. this is not +// an elegant solution. +var StartSpinner = func(s *spinner.Spinner) { + s.Start() +} + +var StopSpinner = func(s *spinner.Spinner) { + s.Stop() +} + func Spinner(w io.Writer) *spinner.Spinner { return spinner.New(spinner.CharSets[11], 400*time.Millisecond, spinner.WithWriter(w)) } From b0eba939e6c887ee8963b305a53e468da4e2b2ce Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 29 Apr 2020 12:21:44 -0700 Subject: [PATCH 39/47] Add test --- command/issue_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/command/issue_test.go b/command/issue_test.go index 4f75cde3a..570115d45 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -753,3 +753,29 @@ func TestIssueClose_issuesDisabled(t *testing.T) { t.Fatalf("got unexpected error: %s", err) } } + +func TestIssueReopen(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 1} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand(issueReopenCmd, "issue reopen 1") + if err != nil { + t.Fatalf("error running command `issue reopen`: %v", err) + } + + r := regexp.MustCompile(`Reopened issue #13`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} From 49e5ef757f571a9251018cf0a74ddd46446ca184 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 29 Apr 2020 12:29:32 -0700 Subject: [PATCH 40/47] Fix test --- command/issue_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 570115d45..05d1049b5 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -762,18 +762,18 @@ func TestIssueReopen(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "number": 1} + "issue": { "number": 2, "closed": true} } } } `)) http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - output, err := RunCommand(issueReopenCmd, "issue reopen 1") + output, err := RunCommand(issueReopenCmd, "issue reopen 2") if err != nil { t.Fatalf("error running command `issue reopen`: %v", err) } - r := regexp.MustCompile(`Reopened issue #13`) + r := regexp.MustCompile(`Reopened issue #2`) if !r.MatchString(output.Stderr()) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) From f7162d7591a28fdb2d0bec6552b48d0b4ef4f6a9 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 29 Apr 2020 12:29:41 -0700 Subject: [PATCH 41/47] Implement issue reopen --- command/issue.go | 45 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/command/issue.go b/command/issue.go index d1b57485e..331e89443 100644 --- a/command/issue.go +++ b/command/issue.go @@ -41,6 +41,7 @@ func init() { issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser") issueCmd.AddCommand(issueCloseCmd) + issueCmd.AddCommand(issueReopenCmd) } var issueCmd = &cobra.Command{ @@ -82,11 +83,17 @@ With '--web', open the issue in a web browser instead.`, RunE: issueView, } var issueCloseCmd = &cobra.Command{ - Use: "close ", - Short: "close and issue issues", + Use: "close ", + Short: "close issue", Args: cobra.ExactArgs(1), RunE: issueClose, } +var issueReopenCmd = &cobra.Command{ + Use: "reopen ", + Short: "reopen issue", + Args: cobra.ExactArgs(1), + RunE: issueReopen, +} func issueList(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) @@ -559,7 +566,41 @@ func issueClose(cmd *cobra.Command, args []string) error { fmt.Fprintf(colorableErr(cmd), "%s Closed issue #%d\n", utils.Red("✔"), issue.Number) return nil +} +func issueReopen(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + issue, err := issueFromArg(apiClient, baseRepo, args[0]) + var idErr *api.IssuesDisabledError + if errors.As(err, &idErr) { + return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo)) + } else if err != nil { + return fmt.Errorf("failed to find issue #%d: %w", issue.Number, err) + } + + if !issue.Closed { + fmt.Fprintf(colorableErr(cmd), "%s Issue #%d was already open\n", utils.Yellow("!"), issue.Number) + return nil + } + + err = api.IssueClose(apiClient, baseRepo, *issue) + if err != nil { + return fmt.Errorf("API call failed:%w", err) + } + + fmt.Fprintf(colorableErr(cmd), "%s Reopened issue #%d\n", utils.Red("✔"), issue.Number) + + return nil } func displayURL(urlStr string) string { From abf6d59ee65103679b494a4d0f4d9691d0269639 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 29 Apr 2020 13:56:00 -0700 Subject: [PATCH 42/47] Green boi --- command/issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/issue.go b/command/issue.go index 331e89443..68491764b 100644 --- a/command/issue.go +++ b/command/issue.go @@ -598,7 +598,7 @@ func issueReopen(cmd *cobra.Command, args []string) error { return fmt.Errorf("API call failed:%w", err) } - fmt.Fprintf(colorableErr(cmd), "%s Reopened issue #%d\n", utils.Red("✔"), issue.Number) + fmt.Fprintf(colorableErr(cmd), "%s Reopened issue #%d\n", utils.Green("✔"), issue.Number) return nil } From 7df1b992db85e003e0ba694f8a49310e0479a46f Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 29 Apr 2020 13:57:53 -0700 Subject: [PATCH 43/47] Add "already open" test --- command/issue.go | 2 +- command/issue_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/command/issue.go b/command/issue.go index 68491764b..0271c4e4c 100644 --- a/command/issue.go +++ b/command/issue.go @@ -589,7 +589,7 @@ func issueReopen(cmd *cobra.Command, args []string) error { } if !issue.Closed { - fmt.Fprintf(colorableErr(cmd), "%s Issue #%d was already open\n", utils.Yellow("!"), issue.Number) + fmt.Fprintf(colorableErr(cmd), "%s Issue #%d is already open\n", utils.Yellow("!"), issue.Number) return nil } diff --git a/command/issue_test.go b/command/issue_test.go index 05d1049b5..eee7218c3 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -779,3 +779,29 @@ func TestIssueReopen(t *testing.T) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) } } + +func TestIssueReopen_alreadyOpen(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 2, "closed": false} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand(issueReopenCmd, "issue reopen 2") + if err != nil { + t.Fatalf("error running command `issue reopen`: %v", err) + } + + r := regexp.MustCompile(`#2 is already open`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} From a9f34069f1b93b851c4a9437d3ddd07cb930fe11 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 29 Apr 2020 13:58:47 -0700 Subject: [PATCH 44/47] Add "issues disabled" test --- command/issue_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/command/issue_test.go b/command/issue_test.go index eee7218c3..e408ce3cc 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -805,3 +805,24 @@ func TestIssueReopen_alreadyOpen(t *testing.T) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) } } + +func TestIssueReopen_issuesDisabled(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } } + `)) + + _, err := RunCommand(issueReopenCmd, "issue reopen 2") + if err == nil { + t.Fatalf("expected error when issues are disabled") + } + + if !strings.Contains(err.Error(), "issues disabled") { + t.Fatalf("got unexpected error: %s", err) + } +} From 23c072b39d60160183c6e8b4a05907556354d523 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 29 Apr 2020 14:14:50 -0700 Subject: [PATCH 45/47] Add api call --- api/queries_issue.go | 23 +++++++++++++++++++++++ command/issue.go | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index ba9b43d65..e4838651f 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -390,3 +390,26 @@ func IssueClose(client *Client, repo ghrepo.Interface, issue Issue) error { return nil } + +func IssueReopen(client *Client, repo ghrepo.Interface, issue Issue) error { + var mutation struct { + ReopenIssue struct { + Issue struct { + ID githubv4.ID + } + } `graphql:"reopenIssue(input: $input)"` + } + + input := githubv4.ReopenIssueInput{ + IssueID: issue.ID, + } + + v4 := githubv4.NewClient(client.http) + err := v4.Mutate(context.Background(), &mutation, input, nil) + + if err != nil { + return err + } + + return nil +} diff --git a/command/issue.go b/command/issue.go index 0271c4e4c..f86ad2cd5 100644 --- a/command/issue.go +++ b/command/issue.go @@ -593,7 +593,7 @@ func issueReopen(cmd *cobra.Command, args []string) error { return nil } - err = api.IssueClose(apiClient, baseRepo, *issue) + err = api.IssueReopen(apiClient, baseRepo, *issue) if err != nil { return fmt.Errorf("API call failed:%w", err) } From e2aab3083685bf6b5ddbbb7f9472f7e670b5eed6 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Thu, 30 Apr 2020 08:59:14 -0700 Subject: [PATCH 46/47] just return the err Co-authored-by: Nate Smith --- api/queries_issue.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index e4838651f..56a04edd9 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -407,9 +407,5 @@ func IssueReopen(client *Client, repo ghrepo.Interface, issue Issue) error { v4 := githubv4.NewClient(client.http) err := v4.Mutate(context.Background(), &mutation, input, nil) - if err != nil { - return err - } - - return nil + return err } From 2089b15e6a4199a41823f18413e69c2f37dbf0a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 4 May 2020 17:09:11 +0200 Subject: [PATCH 47/47] Fix pagination when fetching metadata --- api/queries_org.go | 4 ++-- api/queries_repo.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/queries_org.go b/api/queries_org.go index 07c83b514..bc492a88a 100644 --- a/api/queries_org.go +++ b/api/queries_org.go @@ -60,7 +60,7 @@ func OrganizationProjects(client *Client, owner string) ([]RepoProject, error) { if !query.Organization.Projects.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Organization.Projects.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Organization.Projects.PageInfo.EndCursor) } return projects, nil @@ -103,7 +103,7 @@ func OrganizationTeams(client *Client, owner string) ([]OrgTeam, error) { if !query.Organization.Teams.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Organization.Teams.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Organization.Teams.PageInfo.EndCursor) } return teams, nil diff --git a/api/queries_repo.go b/api/queries_repo.go index b10f8cd2e..e92357cf0 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -607,7 +607,7 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) if !query.Repository.Projects.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Repository.Projects.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Repository.Projects.PageInfo.EndCursor) } return projects, nil @@ -651,7 +651,7 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, if !query.Repository.AssignableUsers.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Repository.AssignableUsers.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Repository.AssignableUsers.PageInfo.EndCursor) } return users, nil @@ -695,7 +695,7 @@ func RepoLabels(client *Client, repo ghrepo.Interface) ([]RepoLabel, error) { if !query.Repository.Labels.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Repository.Labels.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Repository.Labels.PageInfo.EndCursor) } return labels, nil @@ -739,7 +739,7 @@ func RepoMilestones(client *Client, repo ghrepo.Interface) ([]RepoMilestone, err if !query.Repository.Milestones.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Repository.Milestones.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Repository.Milestones.PageInfo.EndCursor) } return milestones, nil