From 93c61a83b2b9a87e37edc9d6f415376b55119f7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 7 May 2020 20:53:03 +0200 Subject: [PATCH] Fix `pr status -R` crash with closed PR on the default branch At the time we have a reference to `baseRepo`, we might still not have contacted the API nor obtained any information about the default branch for the repository. This expands the `PullRequests()` query to always report the default branch so we may choose how to render entries that belong on the current branch. --- api/queries_pr.go | 9 +++++- command/pr.go | 12 ++++---- command/pr_test.go | 24 +++++++++++++-- test/fixtures/prStatusCurrentBranch.json | 6 ++-- .../fixtures/prStatusCurrentBranchClosed.json | 1 + ...tusCurrentBranchClosedOnDefaultBranch.json | 29 +++++++++++++++++++ .../fixtures/prStatusCurrentBranchMerged.json | 1 + ...tusCurrentBranchMergedOnDefaultBranch.json | 29 +++++++++++++++++++ 8 files changed, 98 insertions(+), 13 deletions(-) create mode 100644 test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json create mode 100644 test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json diff --git a/api/queries_pr.go b/api/queries_pr.go index ccbe1f390..95ded6008 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -14,6 +14,7 @@ type PullRequestsPayload struct { ViewerCreated PullRequestAndTotalCount ReviewRequested PullRequestAndTotalCount CurrentPR *PullRequest + DefaultBranch string } type PullRequestAndTotalCount struct { @@ -190,6 +191,9 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu type response struct { Repository struct { + DefaultBranchRef struct { + Name string + } PullRequests edges PullRequest *PullRequest } @@ -238,6 +242,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu queryPrefix := ` query($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { + defaultBranchRef { name } pullRequests(headRefName: $headRefName, first: $per_page, orderBy: { field: CREATED_AT, direction: DESC }) { totalCount edges { @@ -252,6 +257,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu queryPrefix = ` query($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { + defaultBranchRef { name } pullRequest(number: $number) { ...prWithReviews } @@ -331,7 +337,8 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu PullRequests: reviewRequested, TotalCount: resp.ReviewRequested.TotalCount, }, - CurrentPR: currentPR, + CurrentPR: currentPR, + DefaultBranch: resp.Repository.DefaultBranchRef.Name, } return &payload, nil diff --git a/command/pr.go b/command/pr.go index 126d3db01..957188831 100644 --- a/command/pr.go +++ b/command/pr.go @@ -117,17 +117,15 @@ func prStatus(cmd *cobra.Command, args []string) error { printHeader(out, "Current branch") currentPR := prPayload.CurrentPR currentBranch, _ := ctx.Branch() - noPRMessage := fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]")) + if currentPR != nil && currentPR.State != "OPEN" && prPayload.DefaultBranch == currentBranch { + currentPR = nil + } if currentPR != nil { - if baseRepo.(*api.Repository).DefaultBranchRef.Name == currentBranch && currentPR.State != "OPEN" { - printMessage(out, noPRMessage) - } else { - printPrs(out, 0, *currentPR) - } + printPrs(out, 1, *currentPR) } else if currentPRHeadRef == "" { printMessage(out, " There is no current branch") } else { - printMessage(out, noPRMessage) + printMessage(out, fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]"))) } fmt.Fprintln(out) diff --git a/command/pr_test.go b/command/pr_test.go index 0aff5e14a..c3acb47d9 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -162,6 +162,26 @@ func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { } } +func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { + initBlankContext("", "OWNER/REPO", "blueberries") + http := initFakeHTTP() + + jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := RunCommand("pr status -R OWNER/REPO") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\]`) + if expectedLine.MatchString(output.String()) { + t.Errorf("output not expected to match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + func TestPRStatus_currentBranch_Closed(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() @@ -188,7 +208,7 @@ func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { http := initFakeHTTP() http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosed.json") + jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) @@ -230,7 +250,7 @@ func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { http := initFakeHTTP() http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchMerged.json") + jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) diff --git a/test/fixtures/prStatusCurrentBranch.json b/test/fixtures/prStatusCurrentBranch.json index d024d48c3..8426a1bc6 100644 --- a/test/fixtures/prStatusCurrentBranch.json +++ b/test/fixtures/prStatusCurrentBranch.json @@ -13,7 +13,7 @@ "headRefName": "blueberries", "isDraft": false, "headRepositoryOwner": { - "login": "OWNER/REPO" + "login": "OWNER" }, "isCrossRepository": false } @@ -27,7 +27,7 @@ "headRefName": "blueberries", "isDraft": false, "headRepositoryOwner": { - "login": "OWNER/REPO" + "login": "OWNER" }, "isCrossRepository": false } @@ -41,7 +41,7 @@ "headRefName": "blueberries", "isDraft": false, "headRepositoryOwner": { - "login": "OWNER/REPO" + "login": "OWNER" }, "isCrossRepository": false } diff --git a/test/fixtures/prStatusCurrentBranchClosed.json b/test/fixtures/prStatusCurrentBranchClosed.json index d5d366bd1..e41d35928 100644 --- a/test/fixtures/prStatusCurrentBranchClosed.json +++ b/test/fixtures/prStatusCurrentBranchClosed.json @@ -1,6 +1,7 @@ { "data": { "repository": { + "defaultBranchRef": { "name": "master" }, "pullRequests": { "totalCount": 1, "edges": [ diff --git a/test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json b/test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json new file mode 100644 index 000000000..af80773d3 --- /dev/null +++ b/test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json @@ -0,0 +1,29 @@ +{ + "data": { + "repository": { + "defaultBranchRef": { "name": "blueberries" }, + "pullRequests": { + "totalCount": 1, + "edges": [ + { + "node": { + "number": 8, + "title": "Blueberries are a good fruit", + "state": "CLOSED", + "url": "https://github.com/cli/cli/pull/8", + "headRefName": "blueberries" + } + } + ] + } + }, + "viewerCreated": { + "totalCount": 0, + "edges": [] + }, + "reviewRequested": { + "totalCount": 0, + "edges": [] + } + } +} diff --git a/test/fixtures/prStatusCurrentBranchMerged.json b/test/fixtures/prStatusCurrentBranchMerged.json index da54fdf64..6c11093a7 100644 --- a/test/fixtures/prStatusCurrentBranchMerged.json +++ b/test/fixtures/prStatusCurrentBranchMerged.json @@ -1,6 +1,7 @@ { "data": { "repository": { + "defaultBranchRef": { "name": "master" }, "pullRequests": { "totalCount": 1, "edges": [ diff --git a/test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json b/test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json new file mode 100644 index 000000000..85153d62b --- /dev/null +++ b/test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json @@ -0,0 +1,29 @@ +{ + "data": { + "repository": { + "defaultBranchRef": { "name": "blueberries" }, + "pullRequests": { + "totalCount": 1, + "edges": [ + { + "node": { + "number": 8, + "title": "Blueberries are a good fruit", + "state": "MERGED", + "url": "https://github.com/cli/cli/pull/8", + "headRefName": "blueberries" + } + } + ] + } + }, + "viewerCreated": { + "totalCount": 0, + "edges": [] + }, + "reviewRequested": { + "totalCount": 0, + "edges": [] + } + } +}