From 60b6a5eed59d982aca365045dfc7d2d8e0a30f5c Mon Sep 17 00:00:00 2001 From: Lucas Melin Date: Sat, 16 Mar 2024 23:05:34 -0400 Subject: [PATCH] feat: implement `pr revert` --- api/queries_pr.go | 22 +++++- pkg/cmd/pr/pr.go | 2 + pkg/cmd/pr/revert/revert.go | 122 +++++++++++++++++++++++++++++++ pkg/cmd/pr/revert/revert_test.go | 120 ++++++++++++++++++++++++++++++ 4 files changed, 263 insertions(+), 3 deletions(-) create mode 100644 pkg/cmd/pr/revert/revert.go create mode 100644 pkg/cmd/pr/revert/revert_test.go diff --git a/api/queries_pr.go b/api/queries_pr.go index 4262738c3..dd18b3630 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -517,8 +517,8 @@ func parseCheckStatusFromCheckConclusionState(state CheckConclusionState) checkS func (pr *PullRequest) DisplayableReviews() PullRequestReviews { published := []PullRequestReview{} for _, prr := range pr.Reviews.Nodes { - //Dont display pending reviews - //Dont display commenting reviews without top level comment body + // Dont display pending reviews + // Dont display commenting reviews without top level comment body if prr.State != "PENDING" && !(prr.State == "COMMENTED" && prr.Body == "") { published = append(published, prr) } @@ -598,7 +598,7 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter reviewParams["teamIds"] = ids } - //TODO: How much work to extract this into own method and use for create and edit? + // TODO: How much work to extract this into own method and use for create and edit? if len(reviewParams) > 0 { reviewQuery := ` mutation PullRequestCreateRequestReviews($input: RequestReviewsInput!) { @@ -764,6 +764,22 @@ func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) er return client.Mutate(repo.RepoHost(), "PullRequestReadyForReview", &mutation, variables) } +func PullRequestRevert(client *Client, repo ghrepo.Interface, params githubv4.RevertPullRequestInput) error { + var mutation struct { + RevertPullRequest struct { + PullRequest struct { + ID githubv4.ID + } + } `graphql:"revertPullRequest(input: $input)"` + } + + variables := map[string]interface{}{ + "input": params, + } + + return client.Mutate(repo.RepoHost(), "PullRequestRevert", &mutation, variables) +} + func ConvertPullRequestToDraft(client *Client, repo ghrepo.Interface, pr *PullRequest) error { var mutation struct { ConvertPullRequestToDraft struct { diff --git a/pkg/cmd/pr/pr.go b/pkg/cmd/pr/pr.go index 406cc08b7..e73193084 100644 --- a/pkg/cmd/pr/pr.go +++ b/pkg/cmd/pr/pr.go @@ -14,6 +14,7 @@ import ( cmdMerge "github.com/cli/cli/v2/pkg/cmd/pr/merge" cmdReady "github.com/cli/cli/v2/pkg/cmd/pr/ready" cmdReopen "github.com/cli/cli/v2/pkg/cmd/pr/reopen" + cmdRevert "github.com/cli/cli/v2/pkg/cmd/pr/revert" cmdReview "github.com/cli/cli/v2/pkg/cmd/pr/review" cmdStatus "github.com/cli/cli/v2/pkg/cmd/pr/status" cmdUpdateBranch "github.com/cli/cli/v2/pkg/cmd/pr/update-branch" @@ -63,6 +64,7 @@ func NewCmdPR(f *cmdutil.Factory) *cobra.Command { cmdComment.NewCmdComment(f, nil), cmdClose.NewCmdClose(f, nil), cmdReopen.NewCmdReopen(f, nil), + cmdRevert.NewCmdRevert(f, nil), cmdEdit.NewCmdEdit(f, nil), cmdLock.NewCmdLock(f, cmd.Name(), nil), cmdLock.NewCmdUnlock(f, cmd.Name(), nil), diff --git a/pkg/cmd/pr/revert/revert.go b/pkg/cmd/pr/revert/revert.go new file mode 100644 index 000000000..7429f3436 --- /dev/null +++ b/pkg/cmd/pr/revert/revert.go @@ -0,0 +1,122 @@ +package revert + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/pr/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/shurcooL/githubv4" + "github.com/spf13/cobra" +) + +type RevertOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + + Finder shared.PRFinder + + SelectorArg string + + Body string + BodySet bool + Title string + IsDraft bool +} + +func NewCmdRevert(f *cmdutil.Factory, runF func(*RevertOptions) error) *cobra.Command { + opts := &RevertOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + } + + var bodyFile string + + cmd := &cobra.Command{ + Use: "revert { | | }", + Short: "Revert a pull request", + Args: cmdutil.ExactArgs(1, "cannot revert pull request: number, url, or branch required"), + RunE: func(cmd *cobra.Command, args []string) error { + opts.Finder = shared.NewFinder(f) + + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + bodyProvided := cmd.Flags().Changed("body") + bodyFileProvided := bodyFile != "" + + if err := cmdutil.MutuallyExclusive( + "specify only one of `--body` or `--body-file`", + bodyProvided, + bodyFileProvided, + ); err != nil { + return err + } + + if bodyProvided || bodyFileProvided { + opts.BodySet = true + if bodyFileProvided { + b, err := cmdutil.ReadFile(bodyFile, opts.IO.In) + if err != nil { + return err + } + opts.Body = string(b) + } + } + + if runF != nil { + return runF(opts) + } + return revertRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.IsDraft, "draft", "d", false, "Mark revert pull request as a draft") + cmd.Flags().StringVarP(&opts.Title, "title", "t", "", "Title for the revert pull request") + cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Body for the revert pull request") + cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)") + return cmd +} + +func revertRun(opts *RevertOptions) error { + cs := opts.IO.ColorScheme() + + findOptions := shared.FindOptions{ + Selector: opts.SelectorArg, + Fields: []string{"id", "number", "state", "title"}, + } + pr, baseRepo, err := opts.Finder.Find(findOptions) + if err != nil { + return err + } + if pr.State != "MERGED" { + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request %s#%d (%s) can't be reverted because it has not been merged\n", cs.FailureIcon(), ghrepo.FullName(baseRepo), pr.Number, pr.Title) + return cmdutil.SilentError + } + + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + params := githubv4.RevertPullRequestInput{ + PullRequestID: pr.ID, + Title: githubv4.NewString(githubv4.String(opts.Title)), + Body: githubv4.NewString(githubv4.String(opts.Body)), + Draft: githubv4.NewBoolean(githubv4.Boolean(opts.IsDraft)), + } + + err = api.PullRequestRevert(apiClient, baseRepo, params) + if err != nil { + return fmt.Errorf("API call failed: %w", err) + } + + fmt.Fprintf(opts.IO.ErrOut, "%s Created revert PR for pull request %s#%d (%s)\n", cs.SuccessIconWithColor(cs.Green), ghrepo.FullName(baseRepo), pr.Number, pr.Title) + + return nil +} diff --git a/pkg/cmd/pr/revert/revert_test.go b/pkg/cmd/pr/revert/revert_test.go new file mode 100644 index 000000000..edd0a8595 --- /dev/null +++ b/pkg/cmd/pr/revert/revert_test.go @@ -0,0 +1,120 @@ +package revert + +import ( + "bytes" + "io" + "net/http" + "testing" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/pr/shared" + "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/test" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(isTTY) + ios.SetStdinTTY(isTTY) + ios.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: ios, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + } + + cmd := NewCmdRevert(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestPRRevert(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + shared.RunCommandFinder("123", &api.PullRequest{ + ID: "SOME-ID", + Number: 123, + State: "MERGED", + Title: "The title of the PR", + }, ghrepo.New("OWNER", "REPO")) + + http.Register( + httpmock.GraphQL(`mutation PullRequestRevert\b`), + httpmock.GraphQLMutation(`{"id": "SOME-ID"}`, + func(inputs map[string]interface{}) { + assert.Equal(t, inputs["pullRequestId"], "SOME-ID") + }), + ) + + output, err := runCommand(http, true, "123") + assert.NoError(t, err) + assert.Equal(t, "", output.String()) + assert.Equal(t, "✓ Created revert PR for pull request OWNER/REPO#123 (The title of the PR)\n", output.Stderr()) +} + +func TestPRRevert_notRevertable(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + shared.RunCommandFinder("123", &api.PullRequest{ + ID: "SOME-ID", + Number: 123, + State: "OPEN", + Title: "The title of the PR", + }, ghrepo.New("OWNER", "REPO")) + + output, err := runCommand(http, true, "123") + assert.Error(t, err) + assert.Equal(t, "", output.String()) + assert.Equal(t, "X Pull request OWNER/REPO#123 (The title of the PR) can't be reverted because it has not been merged\n", output.Stderr()) +} + +func TestPRRevert_withAllOpts(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + shared.RunCommandFinder("123", &api.PullRequest{ + ID: "THE-ID", + Number: 123, + State: "MERGED", + Title: "The title of the PR", + }, ghrepo.New("OWNER", "REPO")) + + http.Register( + httpmock.GraphQL(`mutation PullRequestRevert\b`), + httpmock.GraphQLMutation(`{"id": "THE-ID" }`, + func(inputs map[string]interface{}) { + assert.Equal(t, inputs["pullRequestId"], "THE-ID") + assert.Equal(t, inputs["title"], "Revert PR title") + assert.Equal(t, inputs["body"], "Revert PR body") + assert.Equal(t, inputs["draft"], true) + }), + ) + + output, err := runCommand(http, true, "123 --title 'Revert PR title' --body 'Revert PR body' --draft") + assert.NoError(t, err) + assert.Equal(t, "", output.String()) + assert.Equal(t, "✓ Created revert PR for pull request OWNER/REPO#123 (The title of the PR)\n", output.Stderr()) +}