diff --git a/api/queries_pr.go b/api/queries_pr.go index ccbe1f390..2a78519cc 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -6,10 +6,24 @@ import ( "strings" "time" - "github.com/cli/cli/internal/ghrepo" "github.com/shurcooL/githubv4" + + "github.com/cli/cli/internal/ghrepo" ) +type PullRequestReviewState int + +const ( + ReviewApprove PullRequestReviewState = iota + ReviewRequestChanges + ReviewComment +) + +type PullRequestReviewInput struct { + Body string + State PullRequestReviewState +} + type PullRequestsPayload struct { ViewerCreated PullRequestAndTotalCount ReviewRequested PullRequestAndTotalCount @@ -456,6 +470,7 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea repository(owner: $owner, name: $repo) { pullRequests(headRefName: $headRefName, states: OPEN, first: 30) { nodes { + id number title state @@ -593,6 +608,34 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter return &result.CreatePullRequest.PullRequest, nil } +func AddReview(client *Client, pr *PullRequest, input *PullRequestReviewInput) error { + var mutation struct { + AddPullRequestReview struct { + ClientMutationID string + } `graphql:"addPullRequestReview(input:$input)"` + } + + state := githubv4.PullRequestReviewEventComment + switch input.State { + case ReviewApprove: + state = githubv4.PullRequestReviewEventApprove + case ReviewRequestChanges: + state = githubv4.PullRequestReviewEventRequestChanges + } + + body := githubv4.String(input.Body) + + gqlInput := githubv4.AddPullRequestReviewInput{ + PullRequestID: pr.ID, + Event: &state, + Body: &body, + } + + v4 := githubv4.NewClient(client.http) + + return v4.Mutate(context.Background(), &mutation, gqlInput, nil) +} + func PullRequestList(client *Client, vars map[string]interface{}, limit int) (*PullRequestAndTotalCount, error) { type prBlock struct { Edges []struct { diff --git a/command/pr_review.go b/command/pr_review.go new file mode 100644 index 000000000..bd0453784 --- /dev/null +++ b/command/pr_review.go @@ -0,0 +1,134 @@ +package command + +import ( + "errors" + "fmt" + "strconv" + "strings" + + "github.com/cli/cli/api" + "github.com/spf13/cobra" +) + +func init() { + prCmd.AddCommand(prReviewCmd) + + prReviewCmd.Flags().BoolP("approve", "a", false, "Approve pull request") + prReviewCmd.Flags().BoolP("request-changes", "r", false, "Request changes on a pull request") + prReviewCmd.Flags().BoolP("comment", "c", false, "Comment on a pull request") + prReviewCmd.Flags().StringP("body", "b", "", "Specify the body of a review") +} + +var prReviewCmd = &cobra.Command{ + Use: "review [{ | | ]", + Short: "Add a review to a pull request.", + Args: cobra.MaximumNArgs(1), + Long: `Add a review to either a specified pull request or the pull request associated with the current branch. + +Examples: + + gh pr review -a # mark the current branch's pull request as approved + gh pr review -c -b "interesting" # comment on the current branch's pull request + gh pr review 123 -r -b "needs more ascii art" # request changes on pull request 123 + `, + RunE: prReview, +} + +func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) { + found := 0 + flag := "" + var state api.PullRequestReviewState + + if cmd.Flags().Changed("approve") { + found++ + flag = "approve" + state = api.ReviewApprove + } + if cmd.Flags().Changed("request-changes") { + found++ + flag = "request-changes" + state = api.ReviewRequestChanges + } + if cmd.Flags().Changed("comment") { + found++ + flag = "comment" + state = api.ReviewComment + } + + if found != 1 { + return nil, errors.New("need exactly one of --approve, --request-changes, or --comment") + } + + body, err := cmd.Flags().GetString("body") + if err != nil { + return nil, err + } + + if (flag == "request-changes" || flag == "comment") && body == "" { + return nil, fmt.Errorf("body cannot be blank for %s review", flag) + } + + return &api.PullRequestReviewInput{ + Body: body, + State: state, + }, nil +} + +func prReview(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return fmt.Errorf("could not determine base repo: %w", err) + } + + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + var prNum int + branchWithOwner := "" + + if len(args) == 0 { + prNum, branchWithOwner, err = prSelectorForCurrentBranch(ctx, baseRepo) + if err != nil { + return fmt.Errorf("could not query for pull request for current branch: %w", err) + } + } else { + prArg, repo := prFromURL(args[0]) + if repo != nil { + baseRepo = repo + } else { + prArg = strings.TrimPrefix(args[0], "#") + } + prNum, err = strconv.Atoi(prArg) + if err != nil { + return errors.New("could not parse pull request argument") + } + } + + input, err := processReviewOpt(cmd) + if err != nil { + return fmt.Errorf("did not understand desired review action: %w", err) + } + + var pr *api.PullRequest + if prNum > 0 { + pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNum) + if err != nil { + return fmt.Errorf("could not find pull request: %w", err) + } + } else { + pr, err = api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner) + if err != nil { + return fmt.Errorf("could not find pull request: %w", err) + } + } + + err = api.AddReview(apiClient, pr, input) + if err != nil { + return fmt.Errorf("failed to create review: %w", err) + } + + return nil +} diff --git a/command/pr_review_test.go b/command/pr_review_test.go new file mode 100644 index 000000000..96ceabd7e --- /dev/null +++ b/command/pr_review_test.go @@ -0,0 +1,213 @@ +package command + +import ( + "bytes" + "encoding/json" + "io/ioutil" + "testing" +) + +func TestPRReview_validation(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + for _, cmd := range []string{ + `pr review 123`, + `pr review --approve --comment 123`, + `pr review --approve --comment -b"hey" 123`, + } { + http.StubRepoResponse("OWNER", "REPO") + _, err := RunCommand(cmd) + eq(t, err.Error(), "did not understand desired review action: need exactly one of --approve, --request-changes, or --comment") + } +} + +func TestPRReview_url_arg(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "id": "foobar123", + "number": 123, + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } `)) + http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) + + _, err := RunCommand("pr review --approve https://github.com/OWNER/REPO/pull/123") + if err != nil { + t.Fatalf("error running pr review: %s", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) + reqBody := struct { + Variables struct { + Input struct { + PullRequestID string + Event string + Body string + } + } + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.PullRequestID, "foobar123") + eq(t, reqBody.Variables.Input.Event, "APPROVE") + eq(t, reqBody.Variables.Input.Body, "") +} + +func TestPRReview_number_arg(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "id": "foobar123", + "number": 123, + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } `)) + http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) + + _, err := RunCommand("pr review --approve 123") + if err != nil { + t.Fatalf("error running pr review: %s", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) + reqBody := struct { + Variables struct { + Input struct { + PullRequestID string + Event string + Body string + } + } + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.PullRequestID, "foobar123") + eq(t, reqBody.Variables.Input.Event, "APPROVE") + eq(t, reqBody.Variables.Input.Body, "") +} + +func TestPRReview_no_arg(t *testing.T) { + initBlankContext("", "OWNER/REPO", "feature") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "url": "https://github.com/OWNER/REPO/pull/123", + "id": "foobar123", + "headRefName": "feature", + "baseRefName": "master" } + ] } } } }`)) + http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) + + _, err := RunCommand(`pr review --comment -b "cool story"`) + if err != nil { + t.Fatalf("error running pr review: %s", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) + reqBody := struct { + Variables struct { + Input struct { + PullRequestID string + Event string + Body string + } + } + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.PullRequestID, "foobar123") + eq(t, reqBody.Variables.Input.Event, "COMMENT") + eq(t, reqBody.Variables.Input.Body, "cool story") +} + +func TestPRReview_blank_comment(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + _, err := RunCommand(`pr review --comment 123`) + eq(t, err.Error(), "did not understand desired review action: body cannot be blank for comment review") +} + +func TestPRReview_blank_request_changes(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + _, err := RunCommand(`pr review -r 123`) + eq(t, err.Error(), "did not understand desired review action: body cannot be blank for request-changes review") +} + +func TestPRReview(t *testing.T) { + type c struct { + Cmd string + ExpectedEvent string + ExpectedBody string + } + cases := []c{ + c{`pr review --request-changes -b"bad"`, "REQUEST_CHANGES", "bad"}, + c{`pr review --approve`, "APPROVE", ""}, + c{`pr review --approve -b"hot damn"`, "APPROVE", "hot damn"}, + c{`pr review --comment --body "i donno"`, "COMMENT", "i donno"}, + } + + for _, kase := range cases { + initBlankContext("", "OWNER/REPO", "feature") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "url": "https://github.com/OWNER/REPO/pull/123", + "id": "foobar123", + "headRefName": "feature", + "baseRefName": "master" } + ] } } } } + `)) + http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) + + _, err := RunCommand(kase.Cmd) + if err != nil { + t.Fatalf("got unexpected error running %s: %s", kase.Cmd, err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) + reqBody := struct { + Variables struct { + Input struct { + Event string + Body string + } + } + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.Event, kase.ExpectedEvent) + eq(t, reqBody.Variables.Input.Body, kase.ExpectedBody) + } +} diff --git a/command/testing.go b/command/testing.go index 24f2e8b60..5a7f68e84 100644 --- a/command/testing.go +++ b/command/testing.go @@ -135,6 +135,7 @@ func RunCommand(args string) (*cmdOut, error) { // Reset flag values so they don't leak between tests // FIXME: change how we initialize Cobra commands to render this hack unnecessary cmd.Flags().VisitAll(func(f *pflag.Flag) { + f.Changed = false switch v := f.Value.(type) { case pflag.SliceValue: _ = v.Replace([]string{})