Merge pull request cli#624 from rista404/pr-state-merged-closed-613

Show state instead of checks/reviews for closed/merged PRs
duncanhealy · Mar 11, 2020 · 14603f4 · 14603f4
2 parents c5499c8 + e803470
commit 14603f4
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 27 deletions.
diff --git a/command/pr.go b/command/pr.go
@@ -391,45 +391,51 @@ func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) {
 	for _, pr := range prs {
 		prNumber := fmt.Sprintf("#%d", pr.Number)
 
-		prNumberColorFunc := utils.Green
+		prStateColorFunc := utils.Green
 		if pr.IsDraft {
-			prNumberColorFunc = utils.Gray
+			prStateColorFunc = utils.Gray
 		} else if pr.State == "MERGED" {
-			prNumberColorFunc = utils.Magenta
+			prStateColorFunc = utils.Magenta
 		} else if pr.State == "CLOSED" {
-			prNumberColorFunc = utils.Red
+			prStateColorFunc = utils.Red
 		}
 
-		fmt.Fprintf(w, "  %s  %s %s", prNumberColorFunc(prNumber), text.Truncate(50, replaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]"))
+		fmt.Fprintf(w, "  %s  %s %s", prStateColorFunc(prNumber), text.Truncate(50, replaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]"))
 
 		checks := pr.ChecksStatus()
 		reviews := pr.ReviewStatus()
-		if checks.Total > 0 || reviews.ChangesRequested || reviews.Approved {
-			fmt.Fprintf(w, "\n  ")
-		}
 
-		if checks.Total > 0 {
-			var summary string
-			if checks.Failing > 0 {
-				if checks.Failing == checks.Total {
-					summary = utils.Red("All checks failing")
-				} else {
-					summary = utils.Red(fmt.Sprintf("%d/%d checks failing", checks.Failing, checks.Total))
+		if pr.State == "OPEN" {
+			if checks.Total > 0 || reviews.ChangesRequested || reviews.Approved {
+				fmt.Fprintf(w, "\n  ")
+			}
+
+			if checks.Total > 0 {
+				var summary string
+				if checks.Failing > 0 {
+					if checks.Failing == checks.Total {
+						summary = utils.Red("All checks failing")
+					} else {
+						summary = utils.Red(fmt.Sprintf("%d/%d checks failing", checks.Failing, checks.Total))
+					}
+				} else if checks.Pending > 0 {
+					summary = utils.Yellow("Checks pending")
+				} else if checks.Passing == checks.Total {
+					summary = utils.Green("Checks passing")
 				}
-			} else if checks.Pending > 0 {
-				summary = utils.Yellow("Checks pending")
-			} else if checks.Passing == checks.Total {
-				summary = utils.Green("Checks passing")
+				fmt.Fprintf(w, " - %s", summary)
 			}
-			fmt.Fprintf(w, " - %s", summary)
-		}
 
-		if reviews.ChangesRequested {
-			fmt.Fprintf(w, " - %s", utils.Red("Changes requested"))
-		} else if reviews.ReviewRequired {
-			fmt.Fprintf(w, " - %s", utils.Yellow("Review required"))
-		} else if reviews.Approved {
-			fmt.Fprintf(w, " - %s", utils.Green("Approved"))
+			if reviews.ChangesRequested {
+				fmt.Fprintf(w, " - %s", utils.Red("Changes requested"))
+			} else if reviews.ReviewRequired {
+				fmt.Fprintf(w, " - %s", utils.Yellow("Review required"))
+			} else if reviews.Approved {
+				fmt.Fprintf(w, " - %s", utils.Green("Approved"))
+			}
+		} else {
+			s := strings.Title(strings.ToLower(pr.State))
+			fmt.Fprintf(w, " - %s", prStateColorFunc(s))
 		}
 
 		fmt.Fprint(w, "\n")

diff --git a/command/pr_test.go b/command/pr_test.go
@@ -155,6 +155,33 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) {
 	}
 }
 
+func TestPRStatus_closedMerged(t *testing.T) {
+	initBlankContext("OWNER/REPO", "blueberries")
+	http := initFakeHTTP()
+	http.StubRepoResponse("OWNER", "REPO")
+
+	jsonFile, _ := os.Open("../test/fixtures/prStatusClosedMerged.json")
+	defer jsonFile.Close()
+	http.StubResponse(200, jsonFile)
+
+	output, err := RunCommand(prStatusCmd, "pr status")
+	if err != nil {
+		t.Errorf("error running command `pr status`: %v", err)
+	}
+
+	expected := []string{
+		"- Checks passing - Changes requested",
+		"- Closed",
+		"- Merged",
+	}
+
+	for _, line := range expected {
+		if !strings.Contains(output.String(), line) {
+			t.Errorf("output did not contain %q: %q", line, output.String())
+		}
+	}
+}
+
 func TestPRStatus_blankSlate(t *testing.T) {
 	initBlankContext("OWNER/REPO", "blueberries")
 	http := initFakeHTTP()

diff --git a/test/fixtures/prStatusChecks.json b/test/fixtures/prStatusChecks.json
@@ -13,6 +13,7 @@
           "node": {
             "number": 8,
             "title": "Strawberries are not actually berries",
+            "state": "OPEN",
             "url": "https://github.com/cli/cli/pull/8",
             "headRefName": "strawberries",
             "reviewDecision": "CHANGES_REQUESTED",
@@ -39,6 +40,7 @@
           "node": {
             "number": 7,
             "title": "Bananas are berries",
+            "state": "OPEN",
             "url": "https://github.com/cli/cli/pull/7",
             "headRefName": "banananana",
             "reviewDecision": "APPROVED",
@@ -66,6 +68,7 @@
           "node": {
             "number": 6,
             "title": "Avocado is probably not a berry",
+            "state": "OPEN",
             "url": "https://github.com/cli/cli/pull/6",
             "headRefName": "avo",
             "reviewDecision": "REVIEW_REQUIRED",

diff --git a/test/fixtures/prStatusClosedMerged.json b/test/fixtures/prStatusClosedMerged.json
@@ -0,0 +1,66 @@
+{
+	"data": {
+	  "repository": {
+		"pullRequests": {
+		  "totalCount": 1,
+		  "edges": [
+			{
+				"node": {
+				  "number": 8,
+				  "title": "Blueberries are a good fruit",
+				  "state": "OPEN",
+				  "url": "https://github.com/cli/cli/pull/8",
+				  "headRefName": "blueberries",
+				  "reviewDecision": "CHANGES_REQUESTED",
+				  "commits": {
+					"nodes": [
+					  {
+						"commit": {
+						  "statusCheckRollup": {
+							"contexts": {
+							  "nodes": [
+								{
+								  "state": "SUCCESS"
+								}
+							  ]
+							}
+						  }
+						}
+					  }
+					]
+				  }
+				}
+			  }
+		  ]
+		}
+	  },
+	  "viewerCreated": {
+		"totalCount": 1,
+		"edges": [
+		  {
+			"node": {
+			  "number": 8,
+			  "state": "CLOSED",
+			  "title": "Strawberries are not actually berries",
+			  "url": "https://github.com/cli/cli/pull/8",
+			  "headRefName": "strawberries"
+			}
+		  },
+		  {
+			"node": {
+			  "number": 8,
+			  "state": "MERGED",
+			  "title": "Bananas are berries",
+			  "url": "https://github.com/cli/cli/pull/8",
+			  "headRefName": "banananana"
+			}
+		  }
+		]
+	  },
+	  "reviewRequested": {
+		"totalCount": 0,
+		"edges": []
+	  }
+	}
+  }
+