From 350b4c85c08d98b02a65953304c3a88ee12d0201 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 11:11:02 -0700 Subject: [PATCH 01/11] Add failing test --- command/pr_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/command/pr_test.go b/command/pr_test.go index 528e2ee77..153cd7a3c 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1093,3 +1093,26 @@ func TestPrMerge_alreadyMerged(t *testing.T) { 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": 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 ready for review`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} From fad9f24f39f7851855f30f34ea0af37c10c990e6 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 11:11:13 -0700 Subject: [PATCH 02/11] Make reopen work --- api/queries_pr.go | 22 ++++++++++++++++++++++ command/pr.go | 43 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index c0b16f3c4..6e5306f78 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -953,6 +953,28 @@ func PullRequestMerge(client *Client, repo ghrepo.Interface, pr *PullRequest, m return err } +func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) error { + var mutation struct { + MarkPullRequestReadyForReviewInput struct { + PullRequest struct { + ID githubv4.ID + } + } `graphql:"markPullRequestReadyForReview(input: $input)"` + } + + input := struct { + // ID of the pull request to be reopened. (Required.) + PullRequestID githubv4.ID `json:"pullRequestId"` + }{ + PullRequestID: pr.ID, + } + + v4 := githubv4.NewClient(client.http) + err := v4.Mutate(context.Background(), &mutation, input, nil) + + return err +} + func min(a, b int) int { if a < b { return a diff --git a/command/pr.go b/command/pr.go index ee529aa9f..fdf957c3b 100644 --- a/command/pr.go +++ b/command/pr.go @@ -29,6 +29,7 @@ func init() { prMergeCmd.Flags().BoolP("merge", "m", true, "Merge the commits with the base branch") prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch") prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch") + prCmd.AddCommand(prReadyCmd) prCmd.AddCommand(prListCmd) prListCmd.Flags().IntP("limit", "L", 30, "Maximum number of items to fetch") @@ -84,13 +85,18 @@ var prReopenCmd = &cobra.Command{ Args: cobra.ExactArgs(1), RunE: prReopen, } - var prMergeCmd = &cobra.Command{ Use: "merge [ | | ]", Short: "Merge a pull request", Args: cobra.MaximumNArgs(1), RunE: prMerge, } +var prReadyCmd = &cobra.Command{ + Use: "ready [ | | ]", + Short: "Make a pull request as ready for review", + Args: cobra.MaximumNArgs(1), + RunE: prReady, +} func prStatus(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) @@ -551,6 +557,41 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error { return nil } +func prReady(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) + if err != nil { + return err + } + + pr, err := prFromArg(apiClient, baseRepo, args[0]) + if err != nil { + return err + } + + // if pr.State == "MERGED" { + // err := fmt.Errorf("%s Pull request #%d can't be closed because it was already merged", utils.Red("!"), pr.Number) + // return err + // } else if pr.Closed { + // fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is already closed\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 ready for review\n", utils.Green("✔"), pr.Number) + + return nil +} + // Ref. https://developer.github.com/v4/enum/pullrequestreviewstate/ const ( requestedReviewState = "REQUESTED" // This is our own state for review request From 922b6e06e0482b89287d9c509ce89bcf1afb486a Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 12:42:16 -0700 Subject: [PATCH 03/11] Can be marked ready for review --- api/queries_pr.go | 7 +++---- command/pr.go | 23 ++++++++++++++++++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 6e5306f78..b3d380240 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -962,13 +962,12 @@ func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) er } `graphql:"markPullRequestReadyForReview(input: $input)"` } - input := struct { - // ID of the pull request to be reopened. (Required.) + type MarkPullRequestReadyForReviewInput struct { PullRequestID githubv4.ID `json:"pullRequestId"` - }{ - PullRequestID: pr.ID, } + input := MarkPullRequestReadyForReviewInput{PullRequestID: pr.ID} + v4 := githubv4.NewClient(client.http) err := v4.Mutate(context.Background(), &mutation, input, nil) diff --git a/command/pr.go b/command/pr.go index fdf957c3b..26b2bc76d 100644 --- a/command/pr.go +++ b/command/pr.go @@ -569,9 +569,26 @@ func prReady(cmd *cobra.Command, args []string) error { return err } - pr, err := prFromArg(apiClient, baseRepo, args[0]) - if err != nil { - return err + var pr *api.PullRequest + if len(args) > 0 { + pr, err = prFromArg(apiClient, baseRepo, args[0]) + if err != nil { + return err + } + } else { + prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx, baseRepo) + if err != nil { + return err + } + + if prNumber != 0 { + pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber) + } else { + pr, err = api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner) + } + if err != nil { + return err + } } // if pr.State == "MERGED" { From 0b0070e725c52db9394604e826bf88234c4ce52b Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 13:03:43 -0700 Subject: [PATCH 04/11] Add "already marked" test --- command/pr.go | 16 ++++++++-------- command/pr_test.go | 27 +++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/command/pr.go b/command/pr.go index 26b2bc76d..87658da42 100644 --- a/command/pr.go +++ b/command/pr.go @@ -591,20 +591,20 @@ func prReady(cmd *cobra.Command, args []string) error { } } - // if pr.State == "MERGED" { - // err := fmt.Errorf("%s Pull request #%d can't be closed because it was already merged", utils.Red("!"), pr.Number) - // return err - // } else if pr.Closed { - // fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is already closed\n", utils.Yellow("!"), pr.Number) - // return nil - // } + if !pr.IsDraft { + fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d was already marked as \"ready for review\"\n", utils.Yellow("!"), pr.Number) + return nil + } else if pr.Closed { + err := fmt.Errorf("%s Pull request #%d is closed and can't be marked \"ready for review\"\n", utils.Red("!"), pr.Number) + return err + } 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 ready for review\n", utils.Green("✔"), pr.Number) + 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_test.go b/command/pr_test.go index 153cd7a3c..2a79ed4cf 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1100,7 +1100,7 @@ func TestPRReady(t *testing.T) { http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 444, "closed": true} + "pullRequest": { "number": 444, "closed": false, "isDraft": true} } } } `)) http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) @@ -1110,7 +1110,30 @@ func TestPRReady(t *testing.T) { t.Fatalf("error running command `pr ready`: %v", err) } - r := regexp.MustCompile(`Pull request #444 is marked ready for review`) + 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": 444, "closed": false, "isDraft": false} + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand("pr ready 444") + if err != nil { + t.Fatalf("expected an error running command `pr ready` on a review that is already ready!: %v", err) + } + + r := regexp.MustCompile(`Pull request #444 was already 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()) From 78bc7260e0f73d6de2cca54b0327d3de61a68c52 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 13:08:57 -0700 Subject: [PATCH 05/11] Add closed test --- command/pr.go | 9 +++++---- command/pr_test.go | 31 +++++++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/command/pr.go b/command/pr.go index 87658da42..7df7522b7 100644 --- a/command/pr.go +++ b/command/pr.go @@ -591,12 +591,13 @@ func prReady(cmd *cobra.Command, args []string) error { } } - if !pr.IsDraft { + if pr.Closed { + err := fmt.Errorf("%s Pull request #%d is closed and can't be marked \"ready for review\"\n", utils.Red("!"), pr.Number) + fmt.Printf("🌭 %+v\n", "huh") + return err + } else if !pr.IsDraft { fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d was already marked as \"ready for review\"\n", utils.Yellow("!"), pr.Number) return nil - } else if pr.Closed { - err := fmt.Errorf("%s Pull request #%d is closed and can't be marked \"ready for review\"\n", utils.Red("!"), pr.Number) - return err } err = api.PullRequestReady(apiClient, baseRepo, pr) diff --git a/command/pr_test.go b/command/pr_test.go index 2a79ed4cf..64b1151fb 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1123,19 +1123,42 @@ func TestPRReady_alreadyReady(t *testing.T) { http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 444, "closed": false, "isDraft": false} + "pullRequest": { "number": 445, "closed": false, "isDraft": false} } } } `)) http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - output, err := RunCommand("pr ready 444") + output, err := RunCommand("pr ready 445") if err != nil { - t.Fatalf("expected an error running command `pr ready` on a review that is already ready!: %v", err) + t.Fatalf("error running command `pr ready`: %v", err) } - r := regexp.MustCompile(`Pull request #444 was already marked as "ready for review"`) + r := regexp.MustCompile(`Pull request #445 was already 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_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 and can't be marked "ready for review"`) + + if !r.MatchString(err.Error()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, err.Error()) + } +} From b42f5527fd81639d85bf2691c05fb2fb8322fd1e Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 13:27:23 -0700 Subject: [PATCH 06/11] Remove hot dog --- command/pr.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/command/pr.go b/command/pr.go index 7df7522b7..863e7c043 100644 --- a/command/pr.go +++ b/command/pr.go @@ -592,8 +592,7 @@ func prReady(cmd *cobra.Command, args []string) error { } if pr.Closed { - err := fmt.Errorf("%s Pull request #%d is closed and can't be marked \"ready for review\"\n", utils.Red("!"), pr.Number) - fmt.Printf("🌭 %+v\n", "huh") + err := fmt.Errorf("%s Pull request #%d is closed and can't be marked \"ready for review\"", utils.Red("!"), pr.Number) return err } else if !pr.IsDraft { fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d was already marked as \"ready for review\"\n", utils.Yellow("!"), pr.Number) From 9721f75b2b1370ada29b81d8d69e67394d90fdb1 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 14:39:29 -0700 Subject: [PATCH 07/11] Update command/pr.go Co-authored-by: Billy Griffin <5091167+billygriffin@users.noreply.github.com> --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 863e7c043..de85878d6 100644 --- a/command/pr.go +++ b/command/pr.go @@ -592,7 +592,7 @@ func prReady(cmd *cobra.Command, args []string) error { } if pr.Closed { - err := fmt.Errorf("%s Pull request #%d is closed and can't be marked \"ready for review\"", utils.Red("!"), pr.Number) + 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 was already marked as \"ready for review\"\n", utils.Yellow("!"), pr.Number) From 6eab3751d05528bd94c9679d10baed495ae468c7 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 14:39:43 -0700 Subject: [PATCH 08/11] Update command/pr.go Co-authored-by: Billy Griffin <5091167+billygriffin@users.noreply.github.com> --- command/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index de85878d6..cf3eaa8f7 100644 --- a/command/pr.go +++ b/command/pr.go @@ -595,7 +595,7 @@ func prReady(cmd *cobra.Command, args []string) error { 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 was already marked as \"ready for review\"\n", utils.Yellow("!"), pr.Number) + fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is already \"ready for review\"\n", utils.Yellow("!"), pr.Number) return nil } From be927b34ae186488663c8c2f3aa7553daaef3270 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 14:39:52 -0700 Subject: [PATCH 09/11] Update command/pr_test.go Co-authored-by: Billy Griffin <5091167+billygriffin@users.noreply.github.com> --- command/pr_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr_test.go b/command/pr_test.go index 64b1151fb..ebbee4aff 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1133,7 +1133,7 @@ func TestPRReady_alreadyReady(t *testing.T) { t.Fatalf("error running command `pr ready`: %v", err) } - r := regexp.MustCompile(`Pull request #445 was already marked as "ready for review"`) + 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()) From fde5409a701d24acf8b0df3e32c670fc5024f08e Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 18 May 2020 14:40:04 -0700 Subject: [PATCH 10/11] Update command/pr_test.go Co-authored-by: Billy Griffin <5091167+billygriffin@users.noreply.github.com> --- command/pr_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr_test.go b/command/pr_test.go index ebbee4aff..24fce9d63 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1156,7 +1156,7 @@ func TestPRReady_closed(t *testing.T) { 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 and can't be marked "ready for review"`) + 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()) From d209c0be010a203c37ee5b3f71a5e0c2b5c8a469 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Tue, 19 May 2020 11:04:43 -0700 Subject: [PATCH 11/11] Allow pr urls --- command/pr.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index cf3eaa8f7..83ddf7d13 100644 --- a/command/pr.go +++ b/command/pr.go @@ -571,7 +571,15 @@ func prReady(cmd *cobra.Command, args []string) error { var pr *api.PullRequest if len(args) > 0 { - pr, err = prFromArg(apiClient, baseRepo, args[0]) + var prNumber string + n, _ := prFromURL(args[0]) + if n != "" { + prNumber = n + } else { + prNumber = args[0] + } + + pr, err = prFromArg(apiClient, baseRepo, prNumber) if err != nil { return err }