diff --git a/command/pr_review.go b/command/pr_review.go index 5013d2bc2..92b19b6ee 100644 --- a/command/pr_review.go +++ b/command/pr_review.go @@ -6,8 +6,12 @@ import ( "strconv" "strings" - "github.com/cli/cli/api" + "github.com/AlecAivazis/survey/v2" "github.com/spf13/cobra" + + "github.com/cli/cli/api" + "github.com/cli/cli/pkg/surveyext" + "github.com/cli/cli/utils" ) func init() { @@ -28,6 +32,8 @@ var prReviewCmd = &cobra.Command{ Examples: + gh pr review # add a review for the current branch's pull request + gh pr review 123 # add a review for pull request 123 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 @@ -56,7 +62,9 @@ func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) { state = api.ReviewComment } - if found != 1 { + if found == 0 { + return nil, nil // signal interactive mode + } else if found > 1 { return nil, errors.New("need exactly one of --approve, --request-changes, or --comment") } @@ -126,6 +134,17 @@ func prReview(cmd *cobra.Command, args []string) error { } } + if input == nil { + input, err = reviewSurvey(cmd) + if err != nil { + return err + } + if input == nil && err == nil { + // Cancelled. + return nil + } + } + err = api.AddReview(apiClient, pr, input) if err != nil { return fmt.Errorf("failed to create review: %w", err) @@ -133,3 +152,105 @@ func prReview(cmd *cobra.Command, args []string) error { return nil } + +func reviewSurvey(cmd *cobra.Command) (*api.PullRequestReviewInput, error) { + editorCommand, err := determineEditor(cmd) + if err != nil { + return nil, err + } + + typeAnswers := struct { + ReviewType string + }{} + typeQs := []*survey.Question{ + { + Name: "reviewType", + Prompt: &survey.Select{ + Message: "What kind of review do you want to give?", + Options: []string{ + "Comment", + "Approve", + "Request changes", + "Cancel", + }, + }, + }, + } + + err = SurveyAsk(typeQs, &typeAnswers) + if err != nil { + return nil, err + } + + reviewState := api.ReviewComment + + switch typeAnswers.ReviewType { + case "Approve": + reviewState = api.ReviewApprove + case "Request Changes": + reviewState = api.ReviewRequestChanges + case "Cancel": + return nil, nil + } + + bodyAnswers := struct { + Body string + }{} + + bodyQs := []*survey.Question{ + &survey.Question{ + Name: "body", + Prompt: &surveyext.GhEditor{ + EditorCommand: editorCommand, + Editor: &survey.Editor{ + Message: "Review body", + FileName: "*.md", + }, + }, + }, + } + + err = SurveyAsk(bodyQs, &bodyAnswers) + if err != nil { + return nil, err + } + + if bodyAnswers.Body == "" && (reviewState == api.ReviewComment || reviewState == api.ReviewRequestChanges) { + return nil, errors.New("this type of review cannot be blank") + } + + if len(bodyAnswers.Body) > 0 { + out := colorableOut(cmd) + renderedBody, err := utils.RenderMarkdown(bodyAnswers.Body) + if err != nil { + return nil, err + } + + fmt.Fprintf(out, "Got:\n%s", renderedBody) + } + + confirm := false + confirmQs := []*survey.Question{ + { + Name: "confirm", + Prompt: &survey.Confirm{ + Message: "Submit?", + Default: true, + }, + }, + } + + err = SurveyAsk(confirmQs, &confirm) + if err != nil { + return nil, err + } + + if !confirm { + return nil, nil + } + + return &api.PullRequestReviewInput{ + Body: bodyAnswers.Body, + State: reviewState, + }, nil +} diff --git a/command/pr_review_test.go b/command/pr_review_test.go index 781b2cb06..17b24a742 100644 --- a/command/pr_review_test.go +++ b/command/pr_review_test.go @@ -4,7 +4,10 @@ import ( "bytes" "encoding/json" "io/ioutil" + "regexp" "testing" + + "github.com/cli/cli/test" ) func TestPRReview_validation(t *testing.T) { @@ -12,7 +15,6 @@ 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`, } { @@ -218,3 +220,165 @@ func TestPRReview(t *testing.T) { eq(t, reqBody.Variables.Input.Body, kase.ExpectedBody) } } + +func TestPRReview_interactive(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": {} }`)) + as, teardown := initAskStubber() + defer teardown() + + as.Stub([]*QuestionStub{ + { + Name: "reviewType", + Value: "Approve", + }, + }) + as.Stub([]*QuestionStub{ + { + Name: "body", + Value: "cool story", + }, + }) + as.Stub([]*QuestionStub{ + { + Name: "confirm", + Value: true, + }, + }) + + output, err := RunCommand(`pr review`) + if err != nil { + t.Fatalf("got unexpected error running pr review: %s", err) + } + + test.ExpectLines(t, output.String(), + "Got:", + "cool.*story") // weird because markdown rendering puts a bunch of junk between works + + 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, "APPROVE") + eq(t, reqBody.Variables.Input.Body, "cool story") +} + +func TestPRReview_interactive_no_body(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": {} }`)) + as, teardown := initAskStubber() + defer teardown() + + as.Stub([]*QuestionStub{ + { + Name: "reviewType", + Value: "Request Changes", + }, + }) + as.Stub([]*QuestionStub{ + { + Name: "body", + Default: true, + }, + }) + as.Stub([]*QuestionStub{ + { + Name: "confirm", + Value: true, + }, + }) + + _, err := RunCommand(`pr review`) + if err == nil { + t.Fatal("expected error") + } + eq(t, err.Error(), "this type of review cannot be blank") +} + +func TestPRReview_interactive_blank_approve(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": {} }`)) + as, teardown := initAskStubber() + defer teardown() + + as.Stub([]*QuestionStub{ + { + Name: "reviewType", + Value: "Approve", + }, + }) + as.Stub([]*QuestionStub{ + { + Name: "body", + Default: true, + }, + }) + as.Stub([]*QuestionStub{ + { + Name: "confirm", + Value: true, + }, + }) + + output, err := RunCommand(`pr review`) + if err != nil { + t.Fatalf("got unexpected error running pr review: %s", err) + } + + unexpect := regexp.MustCompile("Got:") + if unexpect.MatchString(output.String()) { + t.Errorf("did not expect to see body printed in %s", output.String()) + } + + 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, "APPROVE") + eq(t, reqBody.Variables.Input.Body, "") + +} diff --git a/command/root.go b/command/root.go index 3ef268cb7..ef88a3184 100644 --- a/command/root.go +++ b/command/root.go @@ -338,3 +338,17 @@ func formatRemoteURL(cmd *cobra.Command, fullRepoName string) string { return fmt.Sprintf("https://%s/%s.git", defaultHostname, fullRepoName) } + +func determineEditor(cmd *cobra.Command) (string, error) { + editorCommand := os.Getenv("GH_EDITOR") + if editorCommand == "" { + ctx := contextForCommand(cmd) + cfg, err := ctx.Config() + if err != nil { + return "", fmt.Errorf("could not read config: %w", err) + } + editorCommand, _ = cfg.Get(defaultHostname, "editor") + } + + return editorCommand, nil +} diff --git a/command/title_body_survey.go b/command/title_body_survey.go index 06ab14a09..37c202080 100644 --- a/command/title_body_survey.go +++ b/command/title_body_survey.go @@ -2,7 +2,6 @@ package command import ( "fmt" - "os" "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" @@ -127,14 +126,9 @@ func selectTemplate(templatePaths []string) (string, error) { } func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClient *api.Client, repo ghrepo.Interface, providedTitle, providedBody string, defs defaults, templatePaths []string, allowReviewers, allowMetadata bool) error { - editorCommand := os.Getenv("GH_EDITOR") - if editorCommand == "" { - ctx := contextForCommand(cmd) - cfg, err := ctx.Config() - if err != nil { - return fmt.Errorf("could not read config: %w", err) - } - editorCommand, _ = cfg.Get(defaultHostname, "editor") + editorCommand, err := determineEditor(cmd) + if err != nil { + return err } issueState.Title = defs.Title