From b544583b266555de1b1d9fde2fbd8e6e0947585d Mon Sep 17 00:00:00 2001 From: Tarasovych Date: Fri, 13 Mar 2020 14:02:29 +0200 Subject: [PATCH 1/4] added check if pr already exists --- command/pr_create.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/command/pr_create.go b/command/pr_create.go index 3ebbfaa61..53bc1d463 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -55,6 +55,11 @@ func prCreate(cmd *cobra.Command, _ []string) error { return fmt.Errorf("must be on a branch named differently than %q", baseBranch) } + _, err = api.PullRequestForBranch(client, headRepo, headBranch) + if err != fmt.Errorf("no open pull requests found for branch %q", headBranch) { + return fmt.Errorf("pull request for branch %q already exists", headBranch) + } + if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %s\n", utils.Pluralize(ucc, "uncommitted change")) } From f3c9406e76085e4deb13c47f7483a4d33ecee1fd Mon Sep 17 00:00:00 2001 From: Tarasovych Date: Fri, 13 Mar 2020 16:01:37 +0200 Subject: [PATCH 2/4] fixed repo argument --- command/pr_create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pr_create.go b/command/pr_create.go index 53bc1d463..d75001450 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -55,7 +55,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { return fmt.Errorf("must be on a branch named differently than %q", baseBranch) } - _, err = api.PullRequestForBranch(client, headRepo, headBranch) + _, err = api.PullRequestForBranch(client, baseRepo, headBranch) if err != fmt.Errorf("no open pull requests found for branch %q", headBranch) { return fmt.Errorf("pull request for branch %q already exists", headBranch) } From d0c32e7a3dd922850fb125db8c721f171db648d3 Mon Sep 17 00:00:00 2001 From: Tarasovych Date: Mon, 16 Mar 2020 16:01:03 +0200 Subject: [PATCH 3/4] changed error message and pr check --- command/pr_create.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/pr_create.go b/command/pr_create.go index d75001450..5c8ea34c4 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -55,9 +55,9 @@ func prCreate(cmd *cobra.Command, _ []string) error { return fmt.Errorf("must be on a branch named differently than %q", baseBranch) } - _, err = api.PullRequestForBranch(client, baseRepo, headBranch) - if err != fmt.Errorf("no open pull requests found for branch %q", headBranch) { - return fmt.Errorf("pull request for branch %q already exists", headBranch) + existingPr, _ := api.PullRequestForBranch(client, baseRepo, headBranch) + if existingPr != nil { + return fmt.Errorf("pull request for branch %q already exists:\n%s", headBranch, existingPr.URL) } if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { From 5b76881624a09d84a990eacb4b88cc39c667fbba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 18 Mar 2020 13:09:02 +0100 Subject: [PATCH 4/4] Improve detecting existing PR on `pr create` - Only check for existing PRs if `--web` wasn't given - Fix detecting PRs from forks - Improve order of error handling: local validation errors are handled earlier than the ones that have to consult the API --- command/pr_create.go | 25 +++++++++++----- command/pr_create_test.go | 60 +++++++++++++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/command/pr_create.go b/command/pr_create.go index 3b14d7f39..ca7800a19 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -88,11 +88,6 @@ func prCreate(cmd *cobra.Command, _ []string) error { return fmt.Errorf("must be on a branch named differently than %q", baseBranch) } - existingPr, _ := api.PullRequestForBranch(client, baseRepo, headBranch) - if existingPr != nil { - return fmt.Errorf("pull request for branch %q already exists:\n%s", headBranch, existingPr.URL) - } - if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %s\n", utils.Pluralize(ucc, "uncommitted change")) } @@ -128,10 +123,26 @@ func prCreate(cmd *cobra.Command, _ []string) error { if defaultsErr != nil { return fmt.Errorf("could not compute title or body defaults: %w", defaultsErr) } - action = SubmitAction title = defs.Title body = defs.Body - } else { + } + + if !isWeb { + headBranchLabel := headBranch + if headRepo != nil && !ghrepo.IsSame(baseRepo, headRepo) { + headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch) + } + existingPR, err := api.PullRequestForBranch(client, baseRepo, headBranchLabel) + var notFound *api.NotFoundError + if err != nil && !errors.As(err, ¬Found) { + return fmt.Errorf("error checking for existing pull request: %w", err) + } + if err == nil { + return fmt.Errorf("a pull request for branch %q already exists:\n%s", headBranchLabel, existingPR.URL) + } + } + + if !isWeb && !autofill { fmt.Fprintf(colorableErr(cmd), "\nCreating pull request for %s into %s in %s\n\n", utils.Cyan(headBranch), utils.Cyan(baseBranch), diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 2bae80751..688dfd9c8 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -14,6 +14,10 @@ func TestPRCreate(t *testing.T) { initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" @@ -30,7 +34,7 @@ func TestPRCreate(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create -t "my title" -b "my body"`) eq(t, err, nil) - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) reqBody := struct { Variables struct { Input struct { @@ -53,6 +57,32 @@ func TestPRCreate(t *testing.T) { eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } +func TestPRCreate_alreadyExists(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", + "headRefName": "feature" } + ] } } } } + `)) + + cs, cmdTeardown := initCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git status + cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + + _, err := RunCommand(prCreateCmd, `pr create`) + if err == nil { + t.Fatal("error expected, got nil") + } + if err.Error() != "a pull request for branch \"feature\" already exists:\nhttps://github.com/OWNER/REPO/pull/123" { + t.Errorf("got error %q", err) + } +} + func TestPRCreate_web(t *testing.T) { initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() @@ -83,6 +113,10 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" @@ -149,6 +183,10 @@ func TestPRCreate_cross_repo_same_branch(t *testing.T) { "viewerPermission": "WRITE" } } } `)) + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" @@ -165,7 +203,7 @@ func TestPRCreate_cross_repo_same_branch(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create -t "cross repo" -b "same branch"`) eq(t, err, nil) - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) reqBody := struct { Variables struct { Input struct { @@ -194,6 +232,10 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) { initBlankContext("OWNER/REPO", "cool_bug-fixes") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" @@ -231,7 +273,7 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create`) eq(t, err, nil) - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) reqBody := struct { Variables struct { Input struct { @@ -260,6 +302,10 @@ func TestPRCreate_survey_defaults_monocommit(t *testing.T) { initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" @@ -298,7 +344,7 @@ func TestPRCreate_survey_defaults_monocommit(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create`) eq(t, err, nil) - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) reqBody := struct { Variables struct { Input struct { @@ -327,6 +373,10 @@ func TestPRCreate_survey_autofill(t *testing.T) { initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" @@ -346,7 +396,7 @@ func TestPRCreate_survey_autofill(t *testing.T) { output, err := RunCommand(prCreateCmd, `pr create -f`) eq(t, err, nil) - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) reqBody := struct { Variables struct { Input struct {