Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions api/queries_pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package api
import (
"fmt"
"strings"
"time"

"github.com/cli/cli/internal/ghrepo"
)
Expand Down Expand Up @@ -67,6 +66,7 @@ type PullRequest struct {
RequestedReviewer struct {
TypeName string `json:"__typename"`
Login string
Name string
}
}
TotalCount int
Expand All @@ -76,9 +76,7 @@ type PullRequest struct {
Author struct {
Login string
}
State string
CreatedAt time.Time
PublishedAt time.Time
State string
}
}
Assignees struct {
Expand Down Expand Up @@ -376,6 +374,9 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu
...on User {
login
}
...on Team {
name
}
}
}
totalCount
Expand All @@ -386,8 +387,6 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu
login
}
state
createdAt
publishedAt
}
totalCount
}
Expand Down Expand Up @@ -475,6 +474,9 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea
...on User {
login
}
...on Team {
name
}
}
}
totalCount
Expand All @@ -485,8 +487,6 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea
login
}
state
createdAt
publishedAt
}
totalCount
}
Expand Down
118 changes: 116 additions & 2 deletions command/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"io"
"regexp"
"sort"
"strconv"
"strings"

Expand Down Expand Up @@ -339,10 +340,13 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error {
pr.BaseRefName,
pr.HeadRefName,
)))
fmt.Fprintln(out)

// Metadata
// TODO: Reviewers
fmt.Fprintln(out)
if reviewers := prReviewerList(*pr); reviewers != "" {
fmt.Fprint(out, utils.Bold("Reviewers: "))
fmt.Fprintln(out, reviewers)
}
if assignees := prAssigneeList(*pr); assignees != "" {
fmt.Fprint(out, utils.Bold("Assignees: "))
fmt.Fprintln(out, assignees)
Expand Down Expand Up @@ -376,6 +380,116 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error {
return nil
}

// Ref. https://developer.github.com/v4/enum/pullrequestreviewstate/
const (
requestedReviewState = "REQUESTED" // This is our own state for review request
approvedReviewState = "APPROVED"
changesRequestedReviewState = "CHANGES_REQUESTED"
commentedReviewState = "COMMENTED"
)

type reviewerState struct {
Name string
State string
}

// colorFuncForReviewerState returns a color function for a reviewer state
func colorFuncForReviewerState(state string) func(string) string {
switch state {
case requestedReviewState:
return utils.Yellow
case approvedReviewState:
return utils.Green
case changesRequestedReviewState:
return utils.Red
case commentedReviewState:
return func(str string) string { return str } // Do nothing
default:
return nil
}
}

// formattedReviewerState formats a reviewerState with state color
func formattedReviewerState(reviewer *reviewerState) string {
stateColorFunc := colorFuncForReviewerState(reviewer.State)
return fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(reviewer.State)), "_", " ")))
}

// prReviewerList generates a reviewer list with their last state
func prReviewerList(pr api.PullRequest) string {
reviewerStates := parseReviewers(pr)
reviewers := make([]string, 0, len(reviewerStates))

sortReviewerStates(reviewerStates)

for _, reviewer := range reviewerStates {
reviewers = append(reviewers, formattedReviewerState(reviewer))
}

reviewerList := strings.Join(reviewers, ", ")

return reviewerList
}

// Ref. https://developer.github.com/v4/union/requestedreviewer/
const teamTypeName = "Team"

const ghostName = "ghost"

// parseReviewers parses given Reviews and ReviewRequests
func parseReviewers(pr api.PullRequest) []*reviewerState {
reviewerStates := make(map[string]*reviewerState)

for _, review := range pr.Reviews.Nodes {
if review.Author.Login != pr.Author.Login {
name := review.Author.Login
if name == "" {
name = ghostName
}
reviewerStates[name] = &reviewerState{
Name: name,
State: review.State,
}
}
}

// Overwrite reviewer's state if a review request for the same reviewer exists.
for _, reviewRequest := range pr.ReviewRequests.Nodes {
name := reviewRequest.RequestedReviewer.Login
if reviewRequest.RequestedReviewer.TypeName == teamTypeName {
name = reviewRequest.RequestedReviewer.Name
}
reviewerStates[name] = &reviewerState{
Name: name,
State: requestedReviewState,
}
}

// Convert map to slice for ease of sort
result := make([]*reviewerState, 0, len(reviewerStates))
for _, reviewer := range reviewerStates {
result = append(result, reviewer)
}

return result
}

// sortReviewerStates puts completed reviews before review requests and sorts names alphabetically
func sortReviewerStates(reviewerStates []*reviewerState) {
sort.Slice(reviewerStates, func(i, j int) bool {
if reviewerStates[i].State == requestedReviewState &&
reviewerStates[j].State != requestedReviewState {
return false
}
if reviewerStates[j].State == requestedReviewState &&
reviewerStates[i].State != requestedReviewState {
return true
}

return reviewerStates[i].Name < reviewerStates[j].Name
})
}

func prAssigneeList(pr api.PullRequest) string {
if len(pr.Assignees.Nodes) == 0 {
return ""
Expand Down
12 changes: 12 additions & 0 deletions command/pr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@ func TestPRView_Preview(t *testing.T) {
expectedOutputs: []string{
`Blueberries are from a fork`,
`Open • nobody wants to merge 12 commits into master from blueberries`,
`Reviewers: 2 \(Approved\), 3 \(Commented\), 1 \(Requested\)\n`,
`Assignees: marseilles, monaco\n`,
`Labels: one, two, three, four, five\n`,
`Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
Expand All @@ -435,6 +436,17 @@ func TestPRView_Preview(t *testing.T) {
`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`,
},
},
"Open PR with reviewers by number": {
ownerRepo: "master",
args: "pr view 12",
fixture: "../test/fixtures/prViewPreviewWithReviewersByNumber.json",
expectedOutputs: []string{
`Blueberries are from a fork`,
`Reviewers: DEF \(Commented\), def \(Changes requested\), ghost \(Approved\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`,
`blueberries taste good`,
`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`,
},
},
"Open PR with metadata by branch": {
ownerRepo: "master",
args: "pr view blueberries",
Expand Down
33 changes: 33 additions & 0 deletions test/fixtures/prViewPreviewWithMetadataByNumber.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,39 @@
"author": {
"login": "nobody"
},
"reviewRequests": {
"nodes": [
{
"requestedReviewer": {
"__typename": "User",
"login": "1"
}
}
],
"totalcount": 1
},
"reviews": {
"nodes": [
{
"author": {
"login": "3"
},
"state": "COMMENTED"
},
{
"author": {
"login": "2"
},
"state": "APPROVED"
},
{
"author": {
"login": "1"
},
"state": "CHANGES_REQUESTED"
}
]
},
"assignees": {
"nodes": [
{
Expand Down
110 changes: 110 additions & 0 deletions test/fixtures/prViewPreviewWithReviewersByNumber.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
{
"data": {
"repository": {
"pullRequest": {
"number": 12,
"title": "Blueberries are from a fork",
"state": "OPEN",
"body": "**blueberries taste good**",
"url": "https://github.com/OWNER/REPO/pull/12",
"author": {
"login": "nobody"
},
"reviewRequests": {
"nodes": [
{
"requestedReviewer": {
"__typename": "user",
"login": "123"
}
},
{
"requestedReviewer": {
"__typename": "Team",
"name": "Team 1"
}
},
{
"requestedReviewer": {
"__typename": "user",
"login": "abc"
}
}
],
"totalcount": 1
},
"reviews": {
"nodes": [
{
"author": {
"login": "123"
},
"state": "COMMENTED"
},
{
"author": {
"login": "def"
},
"state": "CHANGES_REQUESTED"
},
{
"author": {
"login": "abc"
},
"state": "APPROVED"
},
{
"author": {
"login": "DEF"
},
"state": "COMMENTED"
},
{
"author": {
"login": "xyz"
},
"state": "APPROVED"
},
{
"author": {
"login": ""
},
"state": "APPROVED"
}
]
},
"assignees": {
"nodes": [],
"totalcount": 0
},
"labels": {
"nodes": [],
"totalcount": 0
},
"projectcards": {
"nodes": [],
"totalcount": 0
},
"milestone": {},
"participants": {
"nodes": [
{
"login": "marseilles"
}
],
"totalcount": 1
},
"commits": {
"totalCount": 12
},
"baseRefName": "master",
"headRefName": "blueberries",
"headRepositoryOwner": {
"login": "hubot"
},
"isCrossRepository": true,
"isDraft": false
}
}
}
}