commit review suggestions
This commit is contained in:
parent
8c9049a364
commit
caa282f0ba
3 changed files with 83 additions and 22 deletions
|
|
@ -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() {
|
||||
|
|
|
|||
|
|
@ -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"},
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue