From 5404fb2362b1885bb98c863cd8e844bf8e926128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 6 Aug 2020 17:53:29 +0200 Subject: [PATCH] Wrap up migrating all `pr` commands --- command/pr.go | 146 --------------------- command/pr_lookup.go | 34 ----- command/pr_test.go | 209 ------------------------------- command/root.go | 19 +-- command/testing.go | 9 ++ pkg/cmd/pr/close/close.go | 81 ++++++++++++ pkg/cmd/pr/close/close_test.go | 101 +++++++++++++++ pkg/cmd/pr/pr.go | 66 ++++++++++ pkg/cmd/pr/ready/ready.go | 86 +++++++++++++ pkg/cmd/pr/ready/ready_test.go | 135 ++++++++++++++++++++ pkg/cmd/pr/reopen/reopen.go | 83 ++++++++++++ pkg/cmd/pr/reopen/reopen_test.go | 123 ++++++++++++++++++ 12 files changed, 686 insertions(+), 406 deletions(-) delete mode 100644 command/pr.go delete mode 100644 command/pr_lookup.go delete mode 100644 command/pr_test.go create mode 100644 pkg/cmd/pr/close/close.go create mode 100644 pkg/cmd/pr/close/close_test.go create mode 100644 pkg/cmd/pr/pr.go create mode 100644 pkg/cmd/pr/ready/ready.go create mode 100644 pkg/cmd/pr/ready/ready_test.go create mode 100644 pkg/cmd/pr/reopen/reopen.go create mode 100644 pkg/cmd/pr/reopen/reopen_test.go diff --git a/command/pr.go b/command/pr.go deleted file mode 100644 index 083400419..000000000 --- a/command/pr.go +++ /dev/null @@ -1,146 +0,0 @@ -package command - -import ( - "fmt" - - "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/api" - "github.com/cli/cli/utils" - "github.com/spf13/cobra" -) - -func init() { - prCmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") - - RootCmd.AddCommand(prCmd) - prCmd.AddCommand(prCloseCmd) - prCmd.AddCommand(prReopenCmd) - prCmd.AddCommand(prReadyCmd) -} - -var prCmd = &cobra.Command{ - Use: "pr ", - Short: "Create, view, and checkout pull requests", - Long: `Work with GitHub pull requests`, - Example: heredoc.Doc(` - $ gh pr checkout 353 - $ gh pr create --fill - $ gh pr view --web - `), - Annotations: map[string]string{ - "IsCore": "true", - "help:arguments": `A pull request can be supplied as argument in any of the following formats: -- by number, e.g. "123"; -- by URL, e.g. "https://github.com/OWNER/REPO/pull/123"; or -- by the name of its head branch, e.g. "patch-1" or "OWNER:patch-1".`}, -} -var prCloseCmd = &cobra.Command{ - Use: "close { | | }", - Short: "Close a pull request", - Args: cobra.ExactArgs(1), - RunE: prClose, -} -var prReopenCmd = &cobra.Command{ - Use: "reopen { | | }", - Short: "Reopen a pull request", - Args: cobra.ExactArgs(1), - RunE: prReopen, -} -var prReadyCmd = &cobra.Command{ - Use: "ready [ | | ]", - Short: "Mark a pull request as ready for review", - Args: cobra.MaximumNArgs(1), - RunE: prReady, -} - -func prClose(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args) - if err != nil { - return err - } - - if pr.State == "MERGED" { - err := fmt.Errorf("%s Pull request #%d (%s) can't be closed because it was already merged", utils.Red("!"), pr.Number, pr.Title) - return err - } else if pr.Closed { - fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d (%s) is already closed\n", utils.Yellow("!"), pr.Number, pr.Title) - return nil - } - - err = api.PullRequestClose(apiClient, baseRepo, pr) - if err != nil { - return fmt.Errorf("API call failed: %w", err) - } - - fmt.Fprintf(colorableErr(cmd), "%s Closed pull request #%d (%s)\n", utils.Red("✔"), pr.Number, pr.Title) - - return nil -} - -func prReopen(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args) - if err != nil { - return err - } - - if pr.State == "MERGED" { - err := fmt.Errorf("%s Pull request #%d (%s) can't be reopened because it was already merged", utils.Red("!"), pr.Number, pr.Title) - return err - } - - if !pr.Closed { - fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d (%s) is already open\n", utils.Yellow("!"), pr.Number, pr.Title) - return nil - } - - err = api.PullRequestReopen(apiClient, baseRepo, pr) - if err != nil { - return fmt.Errorf("API call failed: %w", err) - } - - fmt.Fprintf(colorableErr(cmd), "%s Reopened pull request #%d (%s)\n", utils.Green("✔"), pr.Number, pr.Title) - - return nil -} - -func prReady(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args) - if err != nil { - return err - } - - if pr.Closed { - err := fmt.Errorf("%s Pull request #%d is closed. Only draft pull requests can be marked as \"ready for review\"", utils.Red("!"), pr.Number) - return err - } else if !pr.IsDraft { - fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is already \"ready for review\"\n", utils.Yellow("!"), pr.Number) - return nil - } - - err = api.PullRequestReady(apiClient, baseRepo, pr) - if err != nil { - return fmt.Errorf("API call failed: %w", err) - } - - fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is marked as \"ready for review\"\n", utils.Green("✔"), pr.Number) - - return nil -} diff --git a/command/pr_lookup.go b/command/pr_lookup.go deleted file mode 100644 index 727259f7b..000000000 --- a/command/pr_lookup.go +++ /dev/null @@ -1,34 +0,0 @@ -package command - -import ( - "fmt" - - "github.com/cli/cli/api" - "github.com/cli/cli/context" - "github.com/cli/cli/internal/ghrepo" - "github.com/cli/cli/pkg/cmd/pr/shared" - "github.com/spf13/cobra" -) - -func prFromArgs(ctx context.Context, apiClient *api.Client, cmd *cobra.Command, args []string) (*api.PullRequest, ghrepo.Interface, error) { - var arg string - if len(args) > 0 { - arg = args[0] - } - - return shared.PRFromArgs( - apiClient, - func() (ghrepo.Interface, error) { - repo, err := determineBaseRepo(apiClient, cmd, ctx) - if err != nil { - return nil, fmt.Errorf("could not determine base repo: %w", err) - } - return repo, nil - }, - func() (string, error) { - return ctx.Branch() - }, - func() (context.Remotes, error) { - return ctx.Remotes() - }, arg) -} diff --git a/command/pr_test.go b/command/pr_test.go deleted file mode 100644 index 3ac35d624..000000000 --- a/command/pr_test.go +++ /dev/null @@ -1,209 +0,0 @@ -package command - -import ( - "bytes" - "reflect" - "regexp" - "testing" -) - -func eq(t *testing.T, got interface{}, expected interface{}) { - t.Helper() - if !reflect.DeepEqual(got, expected) { - t.Errorf("expected: %v, got: %v", expected, got) - } -} - -func TestPrClose(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 96, "title": "The title of the PR" } - } } } - `)) - - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("pr close 96") - if err != nil { - t.Fatalf("error running command `pr close`: %v", err) - } - - r := regexp.MustCompile(`Closed pull request #96 \(The title of the PR\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPrClose_alreadyClosed(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 101, "title": "The title of the PR", "closed": true } - } } } - `)) - - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("pr close 101") - if err != nil { - t.Fatalf("error running command `pr close`: %v", err) - } - - r := regexp.MustCompile(`Pull request #101 \(The title of the PR\) 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 TestPRReopen(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 666, "title": "The title of the PR", "closed": true} - } } } - `)) - - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("pr reopen 666") - if err != nil { - t.Fatalf("error running command `pr reopen`: %v", err) - } - - r := regexp.MustCompile(`Reopened pull request #666 \(The title of the PR\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPRReopen_alreadyOpen(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 666, "title": "The title of the PR", "closed": false} - } } } - `)) - - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("pr reopen 666") - if err != nil { - t.Fatalf("error running command `pr reopen`: %v", err) - } - - r := regexp.MustCompile(`Pull request #666 \(The title of the PR\) is already open`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPRReopen_alreadyMerged(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 666, "title": "The title of the PR", "closed": true, "state": "MERGED"} - } } } - `)) - - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("pr reopen 666") - if err == nil { - t.Fatalf("expected an error running command `pr reopen`: %v", err) - } - - r := regexp.MustCompile(`Pull request #666 \(The title of the PR\) can't be reopened because it was already merged`) - - if !r.MatchString(err.Error()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPRReady(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 444, "closed": false, "isDraft": true} - } } } - `)) - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("pr ready 444") - if err != nil { - t.Fatalf("error running command `pr ready`: %v", err) - } - - r := regexp.MustCompile(`Pull request #444 is marked as "ready for review"`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPRReady_alreadyReady(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 445, "closed": false, "isDraft": false} - } } } - `)) - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("pr ready 445") - if err != nil { - t.Fatalf("error running command `pr ready`: %v", err) - } - - r := regexp.MustCompile(`Pull request #445 is already "ready for review"`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPRReady_closed(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 446, "closed": true, "isDraft": true} - } } } - `)) - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - _, err := RunCommand("pr ready 446") - if err == nil { - t.Fatalf("expected an error running command `pr ready` on a review that is closed!: %v", err) - } - - r := regexp.MustCompile(`Pull request #446 is closed. Only draft pull requests can be marked as "ready for review"`) - - if !r.MatchString(err.Error()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, err.Error()) - } -} diff --git a/command/root.go b/command/root.go index f24a0233d..ad64676f3 100644 --- a/command/root.go +++ b/command/root.go @@ -23,14 +23,7 @@ import ( "github.com/cli/cli/internal/run" apiCmd "github.com/cli/cli/pkg/cmd/api" gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" - prCheckoutCmd "github.com/cli/cli/pkg/cmd/pr/checkout" - prCreateCmd "github.com/cli/cli/pkg/cmd/pr/create" - prDiffCmd "github.com/cli/cli/pkg/cmd/pr/diff" - prListCmd "github.com/cli/cli/pkg/cmd/pr/list" - prMergeCmd "github.com/cli/cli/pkg/cmd/pr/merge" - prReviewCmd "github.com/cli/cli/pkg/cmd/pr/review" - prStatusCmd "github.com/cli/cli/pkg/cmd/pr/status" - prViewCmd "github.com/cli/cli/pkg/cmd/pr/view" + prCmd "github.com/cli/cli/pkg/cmd/pr" repoCmd "github.com/cli/cli/pkg/cmd/repo" repoCloneCmd "github.com/cli/cli/pkg/cmd/repo/clone" repoCreateCmd "github.com/cli/cli/pkg/cmd/repo/create" @@ -175,15 +168,7 @@ func init() { repoCmd.Cmd.AddCommand(repoCreateCmd.NewCmdCreate(cmdFactory, nil)) repoCmd.Cmd.AddCommand(creditsCmd.NewCmdRepoCredits(&repoResolvingCmdFactory, nil)) - prCmd.AddCommand(prReviewCmd.NewCmdReview(&repoResolvingCmdFactory, nil)) - prCmd.AddCommand(prDiffCmd.NewCmdDiff(&repoResolvingCmdFactory, nil)) - prCmd.AddCommand(prCheckoutCmd.NewCmdCheckout(&repoResolvingCmdFactory, nil)) - prCmd.AddCommand(prViewCmd.NewCmdView(&repoResolvingCmdFactory, nil)) - prCmd.AddCommand(prMergeCmd.NewCmdMerge(&repoResolvingCmdFactory, nil)) - prCmd.AddCommand(prStatusCmd.NewCmdStatus(&repoResolvingCmdFactory, nil)) - prCmd.AddCommand(prCreateCmd.NewCmdCreate(&repoResolvingCmdFactory, nil)) - prCmd.AddCommand(prListCmd.NewCmdList(&repoResolvingCmdFactory, nil)) - + RootCmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory)) RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil)) } diff --git a/command/testing.go b/command/testing.go index 367d510bb..fe9b76ee7 100644 --- a/command/testing.go +++ b/command/testing.go @@ -3,6 +3,8 @@ package command import ( "bytes" "fmt" + "reflect" + "testing" "github.com/cli/cli/api" "github.com/cli/cli/context" @@ -13,6 +15,13 @@ import ( "github.com/spf13/pflag" ) +func eq(t *testing.T, got interface{}, expected interface{}) { + t.Helper() + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %v, got: %v", expected, got) + } +} + const defaultTestConfig = `hosts: github.com: user: OWNER diff --git a/pkg/cmd/pr/close/close.go b/pkg/cmd/pr/close/close.go new file mode 100644 index 000000000..9bce6bb5d --- /dev/null +++ b/pkg/cmd/pr/close/close.go @@ -0,0 +1,81 @@ +package close + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type CloseOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + SelectorArg string +} + +func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Command { + opts := &CloseOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + BaseRepo: f.BaseRepo, + } + + cmd := &cobra.Command{ + Use: "close { | | }", + Short: "Close a pull request", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + if runF != nil { + return runF(opts) + } + return closeRun(opts) + }, + } + + return cmd +} + +func closeRun(opts *CloseOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, nil, nil, opts.SelectorArg) + if err != nil { + return err + } + + if pr.State == "MERGED" { + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) can't be closed because it was already merged", utils.Red("!"), pr.Number, pr.Title) + return cmdutil.SilentError + } else if pr.Closed { + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) is already closed\n", utils.Yellow("!"), pr.Number, pr.Title) + return nil + } + + err = api.PullRequestClose(apiClient, baseRepo, pr) + if err != nil { + return fmt.Errorf("API call failed: %w", err) + } + + fmt.Fprintf(opts.IO.ErrOut, "%s Closed pull request #%d (%s)\n", utils.Green("✔"), pr.Number, pr.Title) + + return nil +} diff --git a/pkg/cmd/pr/close/close_test.go b/pkg/cmd/pr/close/close_test.go new file mode 100644 index 000000000..1bb530637 --- /dev/null +++ b/pkg/cmd/pr/close/close_test.go @@ -0,0 +1,101 @@ +package close + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdClose(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestPrClose(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 96, "title": "The title of the PR" } + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := runCommand(http, true, "96") + if err != nil { + t.Fatalf("error running command `pr close`: %v", err) + } + + r := regexp.MustCompile(`Closed pull request #96 \(The title of the PR\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrClose_alreadyClosed(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 101, "title": "The title of the PR", "closed": true } + } } } + `)) + + output, err := runCommand(http, true, "101") + if err != nil { + t.Fatalf("error running command `pr close`: %v", err) + } + + r := regexp.MustCompile(`Pull request #101 \(The title of the PR\) is already closed`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} diff --git a/pkg/cmd/pr/pr.go b/pkg/cmd/pr/pr.go new file mode 100644 index 000000000..599b01758 --- /dev/null +++ b/pkg/cmd/pr/pr.go @@ -0,0 +1,66 @@ +package pr + +import ( + "github.com/cli/cli/internal/ghrepo" + cmdCheckout "github.com/cli/cli/pkg/cmd/pr/checkout" + cmdClose "github.com/cli/cli/pkg/cmd/pr/close" + cmdCreate "github.com/cli/cli/pkg/cmd/pr/create" + cmdDiff "github.com/cli/cli/pkg/cmd/pr/diff" + cmdList "github.com/cli/cli/pkg/cmd/pr/list" + cmdMerge "github.com/cli/cli/pkg/cmd/pr/merge" + cmdReady "github.com/cli/cli/pkg/cmd/pr/ready" + cmdReopen "github.com/cli/cli/pkg/cmd/pr/reopen" + cmdReview "github.com/cli/cli/pkg/cmd/pr/review" + cmdStatus "github.com/cli/cli/pkg/cmd/pr/status" + cmdView "github.com/cli/cli/pkg/cmd/pr/view" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/pkg/cmdutil" + "github.com/spf13/cobra" +) + +func NewCmdPR(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "pr ", + Short: "Manage pull requests", + Long: "Work with GitHub pull requests", + Example: heredoc.Doc(` + $ gh pr checkout 353 + $ gh pr create --fill + $ gh pr view --web + `), + Annotations: map[string]string{ + "IsCore": "true", + "help:arguments": heredoc.Doc(` + A pull request can be supplied as argument in any of the following formats: + - by number, e.g. "123"; + - by URL, e.g. "https://github.com/OWNER/REPO/pull/123"; or + - by the name of its head branch, e.g. "patch-1" or "OWNER:patch-1". + `), + }, + PersistentPreRun: func(cmd *cobra.Command, args []string) { + if repo, _ := cmd.Flags().GetString("repo"); repo != "" { + // NOTE: this mutates the factory + f.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName(repo) + } + } + }, + } + + cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") + + cmd.AddCommand(cmdCheckout.NewCmdCheckout(f, nil)) + cmd.AddCommand(cmdClose.NewCmdClose(f, nil)) + cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) + cmd.AddCommand(cmdDiff.NewCmdDiff(f, nil)) + cmd.AddCommand(cmdList.NewCmdList(f, nil)) + cmd.AddCommand(cmdMerge.NewCmdMerge(f, nil)) + cmd.AddCommand(cmdReady.NewCmdReady(f, nil)) + cmd.AddCommand(cmdReopen.NewCmdReopen(f, nil)) + cmd.AddCommand(cmdReview.NewCmdReview(f, nil)) + cmd.AddCommand(cmdStatus.NewCmdStatus(f, nil)) + cmd.AddCommand(cmdView.NewCmdView(f, nil)) + + return cmd +} diff --git a/pkg/cmd/pr/ready/ready.go b/pkg/cmd/pr/ready/ready.go new file mode 100644 index 000000000..ab5989267 --- /dev/null +++ b/pkg/cmd/pr/ready/ready.go @@ -0,0 +1,86 @@ +package ready + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/api" + "github.com/cli/cli/context" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ReadyOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Remotes func() (context.Remotes, error) + Branch func() (string, error) + + SelectorArg string +} + +func NewCmdReady(f *cmdutil.Factory, runF func(*ReadyOptions) error) *cobra.Command { + opts := &ReadyOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + BaseRepo: f.BaseRepo, + Remotes: f.Remotes, + Branch: f.Branch, + } + + cmd := &cobra.Command{ + Use: "ready [ | | ]", + Short: "Mark a pull request as ready for review", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + if runF != nil { + return runF(opts) + } + return readyRun(opts) + }, + } + + return cmd +} + +func readyRun(opts *ReadyOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) + if err != nil { + return err + } + + if pr.Closed { + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is closed. Only draft pull requests can be marked as \"ready for review\"", utils.Red("!"), pr.Number) + return cmdutil.SilentError + } else if !pr.IsDraft { + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is already \"ready for review\"\n", utils.Yellow("!"), pr.Number) + return nil + } + + err = api.PullRequestReady(apiClient, baseRepo, pr) + if err != nil { + return fmt.Errorf("API call failed: %w", err) + } + + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is marked as \"ready for review\"\n", utils.Green("✔"), pr.Number) + + return nil +} diff --git a/pkg/cmd/pr/ready/ready_test.go b/pkg/cmd/pr/ready/ready_test.go new file mode 100644 index 000000000..8f5df5832 --- /dev/null +++ b/pkg/cmd/pr/ready/ready_test.go @@ -0,0 +1,135 @@ +package ready + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "testing" + + "github.com/cli/cli/context" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Remotes: func() (context.Remotes, error) { + return context.Remotes{ + { + Remote: &git.Remote{Name: "origin"}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + }, nil + }, + Branch: func() (string, error) { + return "main", nil + }, + } + + cmd := NewCmdReady(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestPRReady(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 444, "closed": false, "isDraft": true} + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := runCommand(http, true, "444") + if err != nil { + t.Fatalf("error running command `pr ready`: %v", err) + } + + r := regexp.MustCompile(`Pull request #444 is marked as "ready for review"`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRReady_alreadyReady(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 445, "closed": false, "isDraft": false} + } } } + `)) + + output, err := runCommand(http, true, "445") + if err != nil { + t.Fatalf("error running command `pr ready`: %v", err) + } + + r := regexp.MustCompile(`Pull request #445 is already "ready for review"`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRReady_closed(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 446, "closed": true, "isDraft": true} + } } } + `)) + + output, err := runCommand(http, true, "446") + if err == nil { + t.Fatalf("expected an error running command `pr ready` on a review that is closed!: %v", err) + } + + r := regexp.MustCompile(`Pull request #446 is closed. Only draft pull requests can be marked as "ready for review"`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} diff --git a/pkg/cmd/pr/reopen/reopen.go b/pkg/cmd/pr/reopen/reopen.go new file mode 100644 index 000000000..b71587250 --- /dev/null +++ b/pkg/cmd/pr/reopen/reopen.go @@ -0,0 +1,83 @@ +package reopen + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ReopenOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + SelectorArg string +} + +func NewCmdReopen(f *cmdutil.Factory, runF func(*ReopenOptions) error) *cobra.Command { + opts := &ReopenOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + BaseRepo: f.BaseRepo, + } + + cmd := &cobra.Command{ + Use: "reopen { | | }", + Short: "Reopen a pull request", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + if runF != nil { + return runF(opts) + } + return reopenRun(opts) + }, + } + + return cmd +} + +func reopenRun(opts *ReopenOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, nil, nil, opts.SelectorArg) + if err != nil { + return err + } + + if pr.State == "MERGED" { + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) can't be reopened because it was already merged", utils.Red("!"), pr.Number, pr.Title) + return cmdutil.SilentError + } + + if !pr.Closed { + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) is already open\n", utils.Yellow("!"), pr.Number, pr.Title) + return nil + } + + err = api.PullRequestReopen(apiClient, baseRepo, pr) + if err != nil { + return fmt.Errorf("API call failed: %w", err) + } + + fmt.Fprintf(opts.IO.ErrOut, "%s Reopened pull request #%d (%s)\n", utils.Green("✔"), pr.Number, pr.Title) + + return nil +} diff --git a/pkg/cmd/pr/reopen/reopen_test.go b/pkg/cmd/pr/reopen/reopen_test.go new file mode 100644 index 000000000..24dfa488f --- /dev/null +++ b/pkg/cmd/pr/reopen/reopen_test.go @@ -0,0 +1,123 @@ +package reopen + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdReopen(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestPRReopen(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 666, "title": "The title of the PR", "closed": true} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := runCommand(http, true, "666") + if err != nil { + t.Fatalf("error running command `pr reopen`: %v", err) + } + + r := regexp.MustCompile(`Reopened pull request #666 \(The title of the PR\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRReopen_alreadyOpen(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 666, "title": "The title of the PR", "closed": false} + } } } + `)) + + output, err := runCommand(http, true, "666") + if err != nil { + t.Fatalf("error running command `pr reopen`: %v", err) + } + + r := regexp.MustCompile(`Pull request #666 \(The title of the PR\) is already open`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRReopen_alreadyMerged(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 666, "title": "The title of the PR", "closed": true, "state": "MERGED"} + } } } + `)) + + output, err := runCommand(http, true, "666") + if err == nil { + t.Fatalf("expected an error running command `pr reopen`: %v", err) + } + + r := regexp.MustCompile(`Pull request #666 \(The title of the PR\) can't be reopened because it was already merged`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +}