STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #885 from cli/survey-review

Browse source 
light interactive pr review
This commit is contained in:
Nate Smith 2020-05-11 17:18:16 -05:00 • committed by GitHub
commit a7bcfffdd3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 371 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

155
command/pr_review.go
View file

@ -6,13 +6,16 @@ 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() {
// TODO re-register post release
// prCmd.AddCommand(prReviewCmd)
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")
@ -28,6 +31,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,15 +61,19 @@ func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
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 found == 0 && body == "" {
return nil, nil // signal interactive mode
} else if found == 0 && body != "" {
return nil, errors.New("--body unsupported without --approve, --request-changes, or --comment")
} else if found > 1 {
return nil, errors.New("need exactly one of --approve, --request-changes, or --comment")
}
if (flag == "request-changes" || flag == "comment") && body == "" {
return nil, fmt.Errorf("body cannot be blank for %s review", flag)
}
@ -108,7 +117,7 @@ func prReview(cmd *cobra.Command, args []string) error {
}
}
input, err := processReviewOpt(cmd)
reviewData, err := processReviewOpt(cmd)
if err != nil {
return fmt.Errorf("did not understand desired review action: %w", err)
}
@ -124,12 +133,140 @@ func prReview(cmd *cobra.Command, args []string) error {
if err != nil {
return fmt.Errorf("could not find pull request: %w", err)
}
prNum = pr.Number
}
err = api.AddReview(apiClient, pr, input)
out := colorableOut(cmd)
if reviewData == nil {
reviewData, err = reviewSurvey(cmd)
if err != nil {
return err
}
if reviewData == nil && err == nil {
fmt.Fprint(out, "Discarding.\n")
return nil
}
}
err = api.AddReview(apiClient, pr, reviewData)
if err != nil {
return fmt.Errorf("failed to create review: %w", err)
}
switch reviewData.State {
case api.ReviewComment:
fmt.Fprintf(out, "%s Reviewed pull request #%d\n", utils.Gray("-"), prNum)
case api.ReviewApprove:
fmt.Fprintf(out, "%s Approved pull request #%d\n", utils.Green("✓"), prNum)
case api.ReviewRequestChanges:
fmt.Fprintf(out, "%s Requested changes to pull request #%d\n", utils.Red("+"), prNum)
}
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",
},
},
},
}
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
}
bodyAnswers := struct {
Body string
}{}
blankAllowed := false
if reviewState == api.ReviewApprove {
blankAllowed = true
}
bodyQs := []*survey.Question{
&survey.Question{
Name: "body",
Prompt: &surveyext.GhEditor{
BlankAllowed: blankAllowed,
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
}

205
command/pr_review_test.go
View file

@ -4,26 +4,40 @@ import (
"bytes"
"encoding/json"
"io/ioutil"
"regexp"
"testing"
"github.com/cli/cli/test"
)
func TestPRReview_validation(t *testing.T) {
t.Skip("skipping until release is done")
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)
if err == nil {
t.Fatal("expected error")
}
eq(t, err.Error(), "did not understand desired review action: need exactly one of --approve, --request-changes, or --comment")
}
}
func TestPRReview_bad_body(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
_, err := RunCommand(`pr review -b "radical"`)
if err == nil {
t.Fatal("expected error")
}
eq(t, err.Error(), "did not understand desired review action: --body unsupported without --approve, --request-changes, or --comment")
}
func TestPRReview_url_arg(t *testing.T) {
t.Skip("skipping until release is done")
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -46,11 +60,13 @@ func TestPRReview_url_arg(t *testing.T) {
} } } } `))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
_, err := RunCommand("pr review --approve https://github.com/OWNER/REPO/pull/123")
output, err := RunCommand("pr review --approve https://github.com/OWNER/REPO/pull/123")
if err != nil {
t.Fatalf("error running pr review: %s", err)
}
test.ExpectLines(t, output.String(), "Approved pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {
Variables struct {
@ -69,7 +85,6 @@ func TestPRReview_url_arg(t *testing.T) {
}
func TestPRReview_number_arg(t *testing.T) {
t.Skip("skipping until release is done")
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -92,11 +107,13 @@ func TestPRReview_number_arg(t *testing.T) {
} } } } `))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
_, err := RunCommand("pr review --approve 123")
output, err := RunCommand("pr review --approve 123")
if err != nil {
t.Fatalf("error running pr review: %s", err)
}
test.ExpectLines(t, output.String(), "Approved pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {
Variables struct {
@ -115,24 +132,26 @@ func TestPRReview_number_arg(t *testing.T) {
}
func TestPRReview_no_arg(t *testing.T) {
t.Skip("skipping until release is done")
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",
"number": 123,
"id": "foobar123",
"headRefName": "feature",
"baseRefName": "master" }
] } } } }`))
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
_, err := RunCommand(`pr review --comment -b "cool story"`)
output, err := RunCommand(`pr review --comment -b "cool story"`)
if err != nil {
t.Fatalf("error running pr review: %s", err)
}
test.ExpectLines(t, output.String(), "- Reviewed pull request #123")
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {
Variables struct {
@ -151,7 +170,6 @@ func TestPRReview_no_arg(t *testing.T) {
}
func TestPRReview_blank_comment(t *testing.T) {
t.Skip("skipping until release is done")
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -161,7 +179,6 @@ func TestPRReview_blank_comment(t *testing.T) {
}
func TestPRReview_blank_request_changes(t *testing.T) {
t.Skip("skipping until release is done")
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -171,7 +188,6 @@ func TestPRReview_blank_request_changes(t *testing.T) {
}
func TestPRReview(t *testing.T) {
t.Skip("skipping until release is done")
type c struct {
Cmd string
ExpectedEvent string
@ -218,3 +234,170 @@ 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",
"number": 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(),
"Approved pull request #123",
"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",
"number": 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())
}
test.ExpectLines(t, output.String(), "Approved pull request #123")
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, "")
}

14
command/root.go
View file

@ -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
}

14
command/title_body_survey.go
View file

@ -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
@ -182,7 +176,7 @@ func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClie
qs = append(qs, bodyQuestion)
}
err := SurveyAsk(qs, issueState)
err = SurveyAsk(qs, issueState)
if err != nil {
return fmt.Errorf("could not prompt: %w", err)
}

18
pkg/surveyext/editor.go
View file

@ -38,6 +38,7 @@ func init() {
type GhEditor struct {
*survey.Editor
EditorCommand string
BlankAllowed bool
}
func (e *GhEditor) editorCommand() string {
@ -58,13 +59,14 @@ var EditorQuestionTemplate = `
{{- else }}
{{- if and .Help (not .ShowHelp)}}{{color "cyan"}}[{{ .Config.HelpInput }} for help]{{color "reset"}} {{end}}
{{- if and .Default (not .HideDefault)}}{{color "white"}}({{.Default}}) {{color "reset"}}{{end}}
{{- color "cyan"}}[(e) to launch {{ .EditorCommand }}, enter to skip] {{color "reset"}}
{{- color "cyan"}}[(e) to launch {{ .EditorCommand }}{{- if .BlankAllowed }}, enter to skip{{ end }}] {{color "reset"}}
{{- end}}`
// EXTENDED to pass editor name (to use in prompt)
type EditorTemplateData struct {
survey.Editor
EditorCommand string
BlankAllowed bool
Answer string
ShowAnswer bool
ShowHelp bool
@ -75,9 +77,10 @@ type EditorTemplateData struct {
func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (interface{}, error) {
err := e.Render(
EditorQuestionTemplate,
// EXTENDED to support printing editor in prompt
// EXTENDED to support printing editor in prompt and BlankAllowed
EditorTemplateData{
Editor: *e.Editor,
BlankAllowed: e.BlankAllowed,
EditorCommand: filepath.Base(e.editorCommand()),
Config: config,
},
@ -96,7 +99,7 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int
defer cursor.Show()
for {
// EXTENDED to handle the e to edit / enter to skip behavior
// EXTENDED to handle the e to edit / enter to skip behavior + BlankAllowed
r, _, err := rr.ReadRune()
if err != nil {
return "", err
@ -105,7 +108,11 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int
break
}
if r == '\r' || r == '\n' {
return "", nil
if e.BlankAllowed {
return "", nil
} else {
continue
}
}
if r == terminal.KeyInterrupt {
return "", terminal.InterruptErr
@ -117,8 +124,9 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int
err = e.Render(
EditorQuestionTemplate,
EditorTemplateData{
// EXTENDED to support printing editor in prompt
// EXTENDED to support printing editor in prompt, BlankAllowed
Editor: *e.Editor,
BlankAllowed: e.BlankAllowed,
EditorCommand: filepath.Base(e.editorCommand()),
ShowHelp: true,
Config: config,