X Tutup
Skip to content

Commit caa282f

Browse files
meiji163meiji163
committed
commit review suggestions
1 parent 8c9049a commit caa282f

File tree

3 files changed

+83
-22
lines changed

3 files changed

+83
-22
lines changed

‎pkg/cmd/repo/delete/delete.go‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,18 @@ To authorize, run "gh auth refresh -s delete_repo"`,
4040
Args: cmdutil.ExactArgs(1, "cannot delete: repository argument required"),
4141
RunE: func(cmd *cobra.Command, args []string) error {
4242
opts.RepoArg = args[0]
43+
if !opts.IO.CanPrompt() && !opts.Confirmed {
44+
return &cmdutil.FlagError{
45+
Err: errors.New("could not prompt: confirmation with prompt or --confirm flag required")}
46+
}
4347
if runF != nil {
4448
return runF(opts)
4549
}
4650
return deleteRun(opts)
4751
},
4852
}
4953

50-
cmd.Flags().BoolVarP(&opts.Confirmed, "confirm", "c", false, "confirm deletion without prompting")
54+
cmd.Flags().BoolVar(&opts.Confirmed, "confirm", false, "confirm deletion without prompting")
5155
return cmd
5256
}
5357

@@ -58,36 +62,31 @@ func deleteRun(opts *DeleteOptions) error {
5862
}
5963
apiClient := api.NewClientFromHTTP(httpClient)
6064

61-
deleteURL := opts.RepoArg
65+
repoSelector := opts.RepoArg
6266
var toDelete ghrepo.Interface
6367

64-
if !strings.Contains(deleteURL, "/") {
68+
if !strings.Contains(repoSelector, "/") {
6569
currentUser, err := api.CurrentLoginName(apiClient, ghinstance.Default())
6670
if err != nil {
6771
return err
6872
}
69-
deleteURL = currentUser + "/" + deleteURL
73+
repoSelector = currentUser + "/" + repoSelector
7074
}
71-
toDelete, err = ghrepo.FromFullName(deleteURL)
75+
toDelete, err = ghrepo.FromFullName(repoSelector)
7276
if err != nil {
7377
return fmt.Errorf("argument error: %w", err)
7478
}
7579

7680
fullName := ghrepo.FullName(toDelete)
7781

78-
doPrompt := opts.IO.CanPrompt()
79-
if !opts.Confirmed && !doPrompt {
80-
return errors.New("could not prompt: confirmation with prompt or --confirm flag required")
81-
}
82-
83-
if !opts.Confirmed && doPrompt {
82+
if !opts.Confirmed {
8483
var valid string
8584
err := prompt.SurveyAskOne(
8685
&survey.Input{Message: fmt.Sprintf("Type %s to confirm deletion:", fullName)},
8786
&valid,
8887
survey.WithValidator(
8988
func(val interface{}) error {
90-
if str := val.(string); str != fullName {
89+
if str := val.(string); !strings.EqualFold(str, fullName) {
9190
return fmt.Errorf("You entered %s", str)
9291
}
9392
return nil
@@ -99,7 +98,7 @@ func deleteRun(opts *DeleteOptions) error {
9998

10099
err = deleteRepo(httpClient, toDelete)
101100
if err != nil {
102-
return fmt.Errorf("API call failed: %w", err)
101+
return err
103102
}
104103

105104
if opts.IO.IsStdoutTTY() {

‎pkg/cmd/repo/delete/delete_test.go‎

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,78 @@
11
package delete
22

33
import (
4+
"bytes"
45
"net/http"
56
"testing"
67

8+
"github.com/cli/cli/v2/pkg/cmdutil"
79
"github.com/cli/cli/v2/pkg/httpmock"
810
"github.com/cli/cli/v2/pkg/iostreams"
911
"github.com/cli/cli/v2/pkg/prompt"
12+
"github.com/google/shlex"
1013
"github.com/stretchr/testify/assert"
1114
)
1215

16+
func TestNewCmdDelete(t *testing.T) {
17+
tests := []struct {
18+
name string
19+
input string
20+
tty bool
21+
output DeleteOptions
22+
wantErr bool
23+
errMsg string
24+
}{
25+
{
26+
name: "confirm flag",
27+
input: "OWNER/REPO --confirm",
28+
output: DeleteOptions{RepoArg: "OWNER/REPO", Confirmed: true},
29+
},
30+
{
31+
name: "no confirmation no tty",
32+
input: "OWNER/REPO",
33+
output: DeleteOptions{RepoArg: "OWNER/REPO"},
34+
wantErr: true,
35+
errMsg: "could not prompt: confirmation with prompt or --confirm flag required"},
36+
{
37+
name: "no argument",
38+
input: "",
39+
wantErr: true,
40+
errMsg: "cannot delete: repository argument required",
41+
tty: true,
42+
},
43+
}
44+
for _, tt := range tests {
45+
t.Run(tt.name, func(t *testing.T) {
46+
io, _, _, _ := iostreams.Test()
47+
io.SetStdinTTY(tt.tty)
48+
io.SetStdoutTTY(tt.tty)
49+
f := &cmdutil.Factory{
50+
IOStreams: io,
51+
}
52+
argv, err := shlex.Split(tt.input)
53+
assert.NoError(t, err)
54+
var gotOpts *DeleteOptions
55+
cmd := NewCmdDelete(f, func(opts *DeleteOptions) error {
56+
gotOpts = opts
57+
return nil
58+
})
59+
cmd.SetArgs(argv)
60+
cmd.SetIn(&bytes.Buffer{})
61+
cmd.SetOut(&bytes.Buffer{})
62+
cmd.SetErr(&bytes.Buffer{})
63+
64+
_, err = cmd.ExecuteC()
65+
if tt.wantErr {
66+
assert.Error(t, err)
67+
assert.Equal(t, tt.errMsg, err.Error())
68+
return
69+
}
70+
assert.NoError(t, err)
71+
assert.Equal(t, tt.output.RepoArg, gotOpts.RepoArg)
72+
})
73+
}
74+
}
75+
1376
func Test_deleteRun(t *testing.T) {
1477
tests := []struct {
1578
name string
@@ -27,7 +90,7 @@ func Test_deleteRun(t *testing.T) {
2790
opts: &DeleteOptions{RepoArg: "OWNER/REPO"},
2891
wantStdout: "✓ Deleted repository OWNER/REPO\n",
2992
askStubs: func(q *prompt.AskStubber) {
30-
// TODO: survey stubber doesn't have WithValidation support
93+
// TODO: survey stubber doesn't have WithValidator support
3194
// so this always passes regardless of prompt input
3295
q.StubOne("OWNER/REPO")
3396
},
@@ -49,12 +112,6 @@ func Test_deleteRun(t *testing.T) {
49112
httpmock.StatusStringResponse(204, "{}"))
50113
},
51114
},
52-
{
53-
name: "no confirmation no tty",
54-
opts: &DeleteOptions{RepoArg: "OWNER/REPO"},
55-
wantErr: true,
56-
errMsg: "could not prompt: confirmation with prompt or --confirm flag required",
57-
},
58115
{
59116
name: "short repo name",
60117
opts: &DeleteOptions{RepoArg: "REPO"},

‎pkg/cmd/repo/delete/http.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,13 @@ func deleteRepo(client *http.Client, repo ghrepo.Interface) error {
2525
}
2626
defer resp.Body.Close()
2727

28-
if resp.StatusCode > 299 {
29-
return api.HandleHTTPError(resp)
28+
err = api.HandleHTTPError(resp)
29+
30+
if resp.StatusCode == 403 {
31+
return fmt.Errorf(`%w
32+
Try authorizing the "delete_repo" scope with "gh auth refresh -s delete_repo".`, err)
33+
} else if resp.StatusCode > 299 {
34+
return err
3035
}
3136

3237
return nil

0 commit comments

Comments
 (0)
X Tutup