From caa282f0ba60c3a10244705d17205040c8794204 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Thu, 14 Oct 2021 21:26:31 -0700 Subject: [PATCH] commit review suggestions --- pkg/cmd/repo/delete/delete.go | 25 +++++------ pkg/cmd/repo/delete/delete_test.go | 71 +++++++++++++++++++++++++++--- pkg/cmd/repo/delete/http.go | 9 +++- 3 files changed, 83 insertions(+), 22 deletions(-) diff --git a/pkg/cmd/repo/delete/delete.go b/pkg/cmd/repo/delete/delete.go index de7debd64..9dda14e2d 100644 --- a/pkg/cmd/repo/delete/delete.go +++ b/pkg/cmd/repo/delete/delete.go @@ -40,6 +40,10 @@ To authorize, run "gh auth refresh -s delete_repo"`, Args: cmdutil.ExactArgs(1, "cannot delete: repository argument required"), RunE: func(cmd *cobra.Command, args []string) error { opts.RepoArg = args[0] + if !opts.IO.CanPrompt() && !opts.Confirmed { + return &cmdutil.FlagError{ + Err: errors.New("could not prompt: confirmation with prompt or --confirm flag required")} + } if runF != nil { return runF(opts) } @@ -47,7 +51,7 @@ To authorize, run "gh auth refresh -s delete_repo"`, }, } - cmd.Flags().BoolVarP(&opts.Confirmed, "confirm", "c", false, "confirm deletion without prompting") + cmd.Flags().BoolVar(&opts.Confirmed, "confirm", false, "confirm deletion without prompting") return cmd } @@ -58,36 +62,31 @@ func deleteRun(opts *DeleteOptions) error { } apiClient := api.NewClientFromHTTP(httpClient) - deleteURL := opts.RepoArg + repoSelector := opts.RepoArg var toDelete ghrepo.Interface - if !strings.Contains(deleteURL, "/") { + if !strings.Contains(repoSelector, "/") { currentUser, err := api.CurrentLoginName(apiClient, ghinstance.Default()) if err != nil { return err } - deleteURL = currentUser + "/" + deleteURL + repoSelector = currentUser + "/" + repoSelector } - toDelete, err = ghrepo.FromFullName(deleteURL) + toDelete, err = ghrepo.FromFullName(repoSelector) if err != nil { return fmt.Errorf("argument error: %w", err) } fullName := ghrepo.FullName(toDelete) - doPrompt := opts.IO.CanPrompt() - if !opts.Confirmed && !doPrompt { - return errors.New("could not prompt: confirmation with prompt or --confirm flag required") - } - - if !opts.Confirmed && doPrompt { + if !opts.Confirmed { var valid string err := prompt.SurveyAskOne( &survey.Input{Message: fmt.Sprintf("Type %s to confirm deletion:", fullName)}, &valid, survey.WithValidator( func(val interface{}) error { - if str := val.(string); str != fullName { + if str := val.(string); !strings.EqualFold(str, fullName) { return fmt.Errorf("You entered %s", str) } return nil @@ -99,7 +98,7 @@ func deleteRun(opts *DeleteOptions) error { err = deleteRepo(httpClient, toDelete) if err != nil { - return fmt.Errorf("API call failed: %w", err) + return err } if opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/repo/delete/delete_test.go b/pkg/cmd/repo/delete/delete_test.go index 28b0133fa..d561a3269 100644 --- a/pkg/cmd/repo/delete/delete_test.go +++ b/pkg/cmd/repo/delete/delete_test.go @@ -1,15 +1,78 @@ package delete import ( + "bytes" "net/http" "testing" + "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/pkg/prompt" + "github.com/google/shlex" "github.com/stretchr/testify/assert" ) +func TestNewCmdDelete(t *testing.T) { + tests := []struct { + name string + input string + tty bool + output DeleteOptions + wantErr bool + errMsg string + }{ + { + name: "confirm flag", + input: "OWNER/REPO --confirm", + output: DeleteOptions{RepoArg: "OWNER/REPO", Confirmed: true}, + }, + { + name: "no confirmation no tty", + input: "OWNER/REPO", + output: DeleteOptions{RepoArg: "OWNER/REPO"}, + wantErr: true, + errMsg: "could not prompt: confirmation with prompt or --confirm flag required"}, + { + name: "no argument", + input: "", + wantErr: true, + errMsg: "cannot delete: repository argument required", + tty: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + io.SetStdinTTY(tt.tty) + io.SetStdoutTTY(tt.tty) + f := &cmdutil.Factory{ + IOStreams: io, + } + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + var gotOpts *DeleteOptions + cmd := NewCmdDelete(f, func(opts *DeleteOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + if tt.wantErr { + assert.Error(t, err) + assert.Equal(t, tt.errMsg, err.Error()) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.output.RepoArg, gotOpts.RepoArg) + }) + } +} + func Test_deleteRun(t *testing.T) { tests := []struct { name string @@ -27,7 +90,7 @@ func Test_deleteRun(t *testing.T) { opts: &DeleteOptions{RepoArg: "OWNER/REPO"}, wantStdout: "✓ Deleted repository OWNER/REPO\n", askStubs: func(q *prompt.AskStubber) { - // TODO: survey stubber doesn't have WithValidation support + // TODO: survey stubber doesn't have WithValidator support // so this always passes regardless of prompt input q.StubOne("OWNER/REPO") }, @@ -49,12 +112,6 @@ func Test_deleteRun(t *testing.T) { httpmock.StatusStringResponse(204, "{}")) }, }, - { - name: "no confirmation no tty", - opts: &DeleteOptions{RepoArg: "OWNER/REPO"}, - wantErr: true, - errMsg: "could not prompt: confirmation with prompt or --confirm flag required", - }, { name: "short repo name", opts: &DeleteOptions{RepoArg: "REPO"}, diff --git a/pkg/cmd/repo/delete/http.go b/pkg/cmd/repo/delete/http.go index bd77328ac..b38224fb1 100644 --- a/pkg/cmd/repo/delete/http.go +++ b/pkg/cmd/repo/delete/http.go @@ -25,8 +25,13 @@ func deleteRepo(client *http.Client, repo ghrepo.Interface) error { } defer resp.Body.Close() - if resp.StatusCode > 299 { - return api.HandleHTTPError(resp) + err = api.HandleHTTPError(resp) + + if resp.StatusCode == 403 { + return fmt.Errorf(`%w +Try authorizing the "delete_repo" scope with "gh auth refresh -s delete_repo".`, err) + } else if resp.StatusCode > 299 { + return err } return nil