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.
guardian939 · May 7, 2020 · 93c61a8 · 93c61a8
1 parent 4ef468c
commit 93c61a8
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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": []
+    }
+  }
+}