diff --git a/api/queries_pr.go b/api/queries_pr.go index 0b2fd378a83..f2f4179e921 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -3,7 +3,6 @@ package api import ( "fmt" "strings" - "time" "github.com/cli/cli/internal/ghrepo" ) @@ -67,6 +66,7 @@ type PullRequest struct { RequestedReviewer struct { TypeName string `json:"__typename"` Login string + Name string } } TotalCount int @@ -76,9 +76,7 @@ type PullRequest struct { Author struct { Login string } - State string - CreatedAt time.Time - PublishedAt time.Time + State string } } Assignees struct { @@ -376,6 +374,9 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu ...on User { login } + ...on Team { + name + } } } totalCount @@ -386,8 +387,6 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu login } state - createdAt - publishedAt } totalCount } @@ -475,6 +474,9 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea ...on User { login } + ...on Team { + name + } } } totalCount @@ -485,8 +487,6 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea login } state - createdAt - publishedAt } totalCount } diff --git a/command/pr.go b/command/pr.go index 00954806163..8bbfd6b9de0 100644 --- a/command/pr.go +++ b/command/pr.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "regexp" + "sort" "strconv" "strings" @@ -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) @@ -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 "" diff --git a/command/pr_test.go b/command/pr_test.go index ba09896454d..e43d83d8fbe 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -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`, @@ -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", diff --git a/test/fixtures/prViewPreviewWithMetadataByNumber.json b/test/fixtures/prViewPreviewWithMetadataByNumber.json index e49bcdc85c8..0ca6124e650 100644 --- a/test/fixtures/prViewPreviewWithMetadataByNumber.json +++ b/test/fixtures/prViewPreviewWithMetadataByNumber.json @@ -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": [ { diff --git a/test/fixtures/prViewPreviewWithReviewersByNumber.json b/test/fixtures/prViewPreviewWithReviewersByNumber.json new file mode 100644 index 00000000000..791512d4141 --- /dev/null +++ b/test/fixtures/prViewPreviewWithReviewersByNumber.json @@ -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 + } + } + } +}