From cef525a0a24c1e6a18c033ce1ba6d454c6a787a6 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 8 Apr 2020 15:08:08 +0900 Subject: [PATCH 1/9] Update unit tests for PR metadata --- command/pr_test.go | 37 +++---- .../prViewPreviewWithMetadataByBranch.json | 30 ++++- .../prViewPreviewWithMetadataByNumber.json | 103 ++++++++++++++++++ 3 files changed, 146 insertions(+), 24 deletions(-) create mode 100644 test/fixtures/prViewPreviewWithMetadataByNumber.json diff --git a/command/pr_test.go b/command/pr_test.go index 705792e82..00cabcbb9 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -416,11 +416,25 @@ func TestPRView_Preview(t *testing.T) { expectedOutputs: []string{ `Blueberries are from a fork`, `Open • nobody wants to merge 12 commits into master from blueberries`, - `Participants: marseilles\n`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, }, }, + "Open PR with metadata by number": { + ownerRepo: "master", + args: "pr view 12", + fixture: "../test/fixtures/prViewPreviewWithMetadataByNumber.json", + expectedOutputs: []string{ + `Blueberries are from a fork`, + `Open • nobody wants to merge 12 commits into master from blueberries`, + `Assignees: marseilles, monaco\n`, + `Labels: one, two, three, four, five\n`, + `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, + `Milestone: uluru\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", @@ -429,30 +443,13 @@ func TestPRView_Preview(t *testing.T) { `Blueberries are a good fruit`, `Open • nobody wants to merge 8 commits into master from blueberries`, `Assignees: marseilles, monaco\n`, - `Labels: one, two, three\n`, - `Projects: The GitHub CLI \(to do list\)\n`, + `Labels: one, two, three, four, five\n`, + `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, `Milestone: uluru\n`, - `Participants: marseilles, monaco, montpellier\n`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10\n`, }, }, - "Open PR with lots of metadata": { - ownerRepo: "master", - args: "pr view 12", - fixture: "../test/fixtures/prViewPreviewWithLotsOfMetadata.json", - expectedOutputs: []string{ - `Blueberries are from a fork`, - `Open • nobody wants to merge 12 commits into master from blueberries`, - `Assignees: marseilles, monaco, montpellier, …\n`, - `Labels: one, two, three, …\n`, - `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), …\n`, - `Milestone: uluru\n`, - `Participants: marseilles, monaco, montpellier, …\n`, - `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, - }, - }, "Open PR for the current branch": { ownerRepo: "blueberries", args: "pr view", diff --git a/test/fixtures/prViewPreviewWithMetadataByBranch.json b/test/fixtures/prViewPreviewWithMetadataByBranch.json index 5f58fc0fe..483b4d63c 100644 --- a/test/fixtures/prViewPreviewWithMetadataByBranch.json +++ b/test/fixtures/prViewPreviewWithMetadataByBranch.json @@ -76,22 +76,44 @@ }, { "name": "three" + }, + { + "name": "four" + }, + { + "name": "five" } ], - "totalcount": 3 + "totalcount": 5 }, "projectcards": { "nodes": [ { "project": { - "name": "The GitHub CLI" + "name": "Project 1" }, "column": { - "name": "to do list" + "name": "column A" + } + }, + { + "project": { + "name": "Project 2" + }, + "column": { + "name": "column B" + } + }, + { + "project": { + "name": "Project 3" + }, + "column": { + "name": "column C" } } ], - "totalcount": 1 + "totalcount": 3 }, "milestone": { "title": "uluru" diff --git a/test/fixtures/prViewPreviewWithMetadataByNumber.json b/test/fixtures/prViewPreviewWithMetadataByNumber.json new file mode 100644 index 000000000..24927cb19 --- /dev/null +++ b/test/fixtures/prViewPreviewWithMetadataByNumber.json @@ -0,0 +1,103 @@ +{ + "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" + }, + "assignees": { + "nodes": [ + { + "login": "marseilles" + }, + { + "login": "monaco" + } + ], + "totalcount": 2 + }, + "labels": { + "nodes": [ + { + "name": "one" + }, + { + "name": "two" + }, + { + "name": "three" + }, + { + "name": "four" + }, + { + "name": "five" + } + ], + "totalcount": 5 + }, + "projectcards": { + "nodes": [ + { + "project": { + "name": "Project 1" + }, + "column": { + "name": "column A" + } + }, + { + "project": { + "name": "Project 2" + }, + "column": { + "name": "column B" + } + }, + { + "project": { + "name": "Project 3" + }, + "column": { + "name": "column C" + } + } + ], + "totalcount": 3 + }, + "milestone": { + "title": "uluru" + }, + "participants": { + "nodes": [ + { + "login": "marseilles" + }, + { + "login": "monaco" + }, + { + "login": "montpellier" + } + ], + "totalcount": 3 + }, + "commits": { + "totalCount": 12 + }, + "baseRefName": "master", + "headRefName": "blueberries", + "headRepositoryOwner": { + "login": "hubot" + }, + "isCrossRepository": true, + "isDraft": false + } + } + } +} From bc42228171b8f620805388be504818d75015d7ff Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Tue, 7 Apr 2020 23:53:34 +0900 Subject: [PATCH 2/9] Parses Reviews and ReviewRequests to show reviewers and their state --- command/pr.go | 52 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/command/pr.go b/command/pr.go index 21c3853cd..b6cb34366 100644 --- a/command/pr.go +++ b/command/pr.go @@ -337,10 +337,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) @@ -374,6 +377,51 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error { return nil } +// +const requestedReviewState = "REQUESTED" + +type reviewerState struct { + Name string + State string +} + +// prReviewerList generates a reviewer list with their last state +func prReviewerList(pr api.PullRequest) string { + reviewerStates := parseReviewers(pr) + reviewers := make([]string, 0, len(reviewerStates)) + + for _, reviewer := range reviewerStates { + reviewers = append(reviewers, fmt.Sprintf("%s (%s)", reviewer.Name, strings.Title(strings.ToLower(reviewer.State)))) + } + reviewerList := strings.Join(reviewers, ", ") + + return reviewerList +} + +// parseReviewers parses given Reviews and ReviewRequests +func parseReviewers(pr api.PullRequest) map[string]*reviewerState { + var reviewerStates = map[string]*reviewerState{} + + for _, review := range pr.Reviews.Nodes { + if review.Author.Login != pr.Author.Login { + reviewerStates[review.Author.Login] = &reviewerState{ + Name: review.Author.Login, + State: strings.Title(strings.ToLower(review.State)), + } + } + } + + // Overwrite reviewer's state if a review request for the same reviewer exists. + for _, reviewRequest := range pr.ReviewRequests.Nodes { + reviewerStates[reviewRequest.RequestedReviewer.Login] = &reviewerState{ + Name: reviewRequest.RequestedReviewer.Login, + State: requestedReviewState, + } + } + + return reviewerStates +} + func prAssigneeList(pr api.PullRequest) string { if len(pr.Assignees.Nodes) == 0 { return "" From 5c261e231835cda83b5206b910d8f53800149148 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 8 Apr 2020 16:26:03 +0900 Subject: [PATCH 3/9] Give a color to reviewer states --- command/pr.go | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/command/pr.go b/command/pr.go index b6cb34366..1aeaa77e2 100644 --- a/command/pr.go +++ b/command/pr.go @@ -378,20 +378,42 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error { } // -const requestedReviewState = "REQUESTED" +const ( + requestedReviewState = "REQUESTED" + 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 + } +} + // prReviewerList generates a reviewer list with their last state func prReviewerList(pr api.PullRequest) string { reviewerStates := parseReviewers(pr) reviewers := make([]string, 0, len(reviewerStates)) for _, reviewer := range reviewerStates { - reviewers = append(reviewers, fmt.Sprintf("%s (%s)", reviewer.Name, strings.Title(strings.ToLower(reviewer.State)))) + stateColorFunc := colorFuncForReviewerState(reviewer.State) + reviewers = append(reviewers, fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(reviewer.State)), "_", " ")))) } reviewerList := strings.Join(reviewers, ", ") @@ -406,7 +428,7 @@ func parseReviewers(pr api.PullRequest) map[string]*reviewerState { if review.Author.Login != pr.Author.Login { reviewerStates[review.Author.Login] = &reviewerState{ Name: review.Author.Login, - State: strings.Title(strings.ToLower(review.State)), + State: review.State, } } } From 6223a2c198a7708a91641416bf769ad94a4e774e Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Fri, 10 Apr 2020 00:51:39 +0900 Subject: [PATCH 4/9] Puts completed reviews before review requests and sort reviewer names alphabetically --- api/queries_pr.go | 9 +- command/pr.go | 30 +++++- command/pr_test.go | 12 +++ .../prViewPreviewWithMetadataByNumber.json | 33 +++++++ .../prViewPreviewWithReviewersByNumber.json | 98 +++++++++++++++++++ 5 files changed, 172 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/prViewPreviewWithReviewersByNumber.json diff --git a/api/queries_pr.go b/api/queries_pr.go index ad29f163f..2a38bf909 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" ) @@ -76,9 +75,7 @@ type PullRequest struct { Author struct { Login string } - State string - CreatedAt time.Time - PublishedAt time.Time + State string } } Assignees struct { @@ -392,8 +389,6 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu login } state - createdAt - publishedAt } totalCount } @@ -497,8 +492,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 1aeaa77e2..c008b1292 100644 --- a/command/pr.go +++ b/command/pr.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "regexp" + "sort" "strconv" "strings" @@ -411,17 +412,20 @@ func prReviewerList(pr api.PullRequest) string { reviewerStates := parseReviewers(pr) reviewers := make([]string, 0, len(reviewerStates)) + sortReviewerStates(reviewerStates) + for _, reviewer := range reviewerStates { stateColorFunc := colorFuncForReviewerState(reviewer.State) reviewers = append(reviewers, fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(reviewer.State)), "_", " ")))) } + reviewerList := strings.Join(reviewers, ", ") return reviewerList } // parseReviewers parses given Reviews and ReviewRequests -func parseReviewers(pr api.PullRequest) map[string]*reviewerState { +func parseReviewers(pr api.PullRequest) []*reviewerState { var reviewerStates = map[string]*reviewerState{} for _, review := range pr.Reviews.Nodes { @@ -441,7 +445,29 @@ func parseReviewers(pr api.PullRequest) map[string]*reviewerState { } } - return reviewerStates + // Convert map to slice for ease of sort + result := []*reviewerState{} + for _, reviewer := range reviewerStates { + result = append(result, reviewer) + } + + return result +} + +// sortReviewerStates puts completed reviews before review requests and sort 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 { diff --git a/command/pr_test.go b/command/pr_test.go index 00cabcbb9..ed0e754d1 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\)\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\), xyz \(Approved\), 123 \(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 24927cb19..7c82f438a 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 000000000..fd293c1c7 --- /dev/null +++ b/test/fixtures/prViewPreviewWithReviewersByNumber.json @@ -0,0 +1,98 @@ +{ + "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": "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" + } + ] + }, + "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 + } + } + } +} From fbff7226ac78c9cf2fbd536dbe66528465104398 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Fri, 10 Apr 2020 04:51:42 +0900 Subject: [PATCH 5/9] Add comments on review state values --- command/pr.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/pr.go b/command/pr.go index a8a197c46..66c60a0ad 100644 --- a/command/pr.go +++ b/command/pr.go @@ -380,9 +380,9 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error { return nil } -// +// Ref. https://developer.github.com/v4/enum/pullrequestreviewstate/ const ( - requestedReviewState = "REQUESTED" + requestedReviewState = "REQUESTED" // This is our own state for review request approvedReviewState = "APPROVED" changesRequestedReviewState = "CHANGES_REQUESTED" commentedReviewState = "COMMENTED" From fe27c842bb4b3818d66cfd2144a4a80a053d874f Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Fri, 10 Apr 2020 05:04:10 +0900 Subject: [PATCH 6/9] Fix slice initialization with provided capacity --- command/pr.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/pr.go b/command/pr.go index 66c60a0ad..05d54a08e 100644 --- a/command/pr.go +++ b/command/pr.go @@ -428,7 +428,7 @@ func prReviewerList(pr api.PullRequest) string { // parseReviewers parses given Reviews and ReviewRequests func parseReviewers(pr api.PullRequest) []*reviewerState { - var reviewerStates = map[string]*reviewerState{} + reviewerStates := make(map[string]*reviewerState) for _, review := range pr.Reviews.Nodes { if review.Author.Login != pr.Author.Login { @@ -448,7 +448,7 @@ func parseReviewers(pr api.PullRequest) []*reviewerState { } // Convert map to slice for ease of sort - result := []*reviewerState{} + result := make([]*reviewerState, 0, len(reviewerStates)) for _, reviewer := range reviewerStates { result = append(result, reviewer) } From 502f6709af20c097bc248315d082461955e64c05 Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Fri, 10 Apr 2020 13:55:48 +0900 Subject: [PATCH 7/9] Extract the reviewerstate formatter into a function --- command/pr.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/command/pr.go b/command/pr.go index 05d54a08e..bb60c41fc 100644 --- a/command/pr.go +++ b/command/pr.go @@ -409,6 +409,12 @@ func colorFuncForReviewerState(state string) func(string) string { } } +// 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) @@ -417,8 +423,7 @@ func prReviewerList(pr api.PullRequest) string { sortReviewerStates(reviewerStates) for _, reviewer := range reviewerStates { - stateColorFunc := colorFuncForReviewerState(reviewer.State) - reviewers = append(reviewers, fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(reviewer.State)), "_", " ")))) + reviewers = append(reviewers, formattedReviewerState(reviewer)) } reviewerList := strings.Join(reviewers, ", ") @@ -456,7 +461,7 @@ func parseReviewers(pr api.PullRequest) []*reviewerState { return result } -// sortReviewerStates puts completed reviews before review requests and sort names alphabetically +// 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 && From d70358ea3445cec9d2f341c0448abab966610d8b Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 22 Apr 2020 21:58:01 +0900 Subject: [PATCH 8/9] Support GitHub Team in requested reviewers --- api/queries_pr.go | 7 +++++++ command/pr.go | 11 +++++++++-- command/pr_test.go | 2 +- test/fixtures/prViewPreviewWithMetadataByNumber.json | 2 +- test/fixtures/prViewPreviewWithReviewersByNumber.json | 6 ++++++ 5 files changed, 24 insertions(+), 4 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 8de56be48..f2f4179e9 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -66,6 +66,7 @@ type PullRequest struct { RequestedReviewer struct { TypeName string `json:"__typename"` Login string + Name string } } TotalCount int @@ -373,6 +374,9 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu ...on User { login } + ...on Team { + name + } } } totalCount @@ -470,6 +474,9 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea ...on User { login } + ...on Team { + name + } } } totalCount diff --git a/command/pr.go b/command/pr.go index bb60c41fc..e1a65fcad 100644 --- a/command/pr.go +++ b/command/pr.go @@ -431,6 +431,9 @@ func prReviewerList(pr api.PullRequest) string { return reviewerList } +// Ref. https://developer.github.com/v4/union/requestedreviewer/ +const teamTypeName = "Team" + // parseReviewers parses given Reviews and ReviewRequests func parseReviewers(pr api.PullRequest) []*reviewerState { reviewerStates := make(map[string]*reviewerState) @@ -446,8 +449,12 @@ func parseReviewers(pr api.PullRequest) []*reviewerState { // Overwrite reviewer's state if a review request for the same reviewer exists. for _, reviewRequest := range pr.ReviewRequests.Nodes { - reviewerStates[reviewRequest.RequestedReviewer.Login] = &reviewerState{ - Name: reviewRequest.RequestedReviewer.Login, + name := reviewRequest.RequestedReviewer.Login + if reviewRequest.RequestedReviewer.TypeName == teamTypeName { + name = reviewRequest.RequestedReviewer.Name + } + reviewerStates[name] = &reviewerState{ + Name: name, State: requestedReviewState, } } diff --git a/command/pr_test.go b/command/pr_test.go index ed0e754d1..147621fc2 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -442,7 +442,7 @@ func TestPRView_Preview(t *testing.T) { fixture: "../test/fixtures/prViewPreviewWithReviewersByNumber.json", expectedOutputs: []string{ `Blueberries are from a fork`, - `Reviewers: DEF \(Commented\), def \(Changes requested\), xyz \(Approved\), 123 \(Requested\), abc \(Requested\)\n`, + `Reviewers: DEF \(Commented\), def \(Changes requested\), 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`, }, diff --git a/test/fixtures/prViewPreviewWithMetadataByNumber.json b/test/fixtures/prViewPreviewWithMetadataByNumber.json index 0981f4263..55366c26b 100644 --- a/test/fixtures/prViewPreviewWithMetadataByNumber.json +++ b/test/fixtures/prViewPreviewWithMetadataByNumber.json @@ -14,7 +14,7 @@ "nodes": [ { "requestedReviewer": { - "__typename": "user", + "__typename": "User", "login": "1" } } diff --git a/test/fixtures/prViewPreviewWithReviewersByNumber.json b/test/fixtures/prViewPreviewWithReviewersByNumber.json index fd293c1c7..049718b07 100644 --- a/test/fixtures/prViewPreviewWithReviewersByNumber.json +++ b/test/fixtures/prViewPreviewWithReviewersByNumber.json @@ -18,6 +18,12 @@ "login": "123" } }, + { + "requestedReviewer": { + "__typename": "Team", + "name": "Team 1" + } + }, { "requestedReviewer": { "__typename": "user", From 67907c8b004fc5331900119b9882ca481b6cc68d Mon Sep 17 00:00:00 2001 From: Toshiya Doi Date: Wed, 22 Apr 2020 21:59:09 +0900 Subject: [PATCH 9/9] Support ghost reviews in pr view --- command/pr.go | 10 ++++++++-- command/pr_test.go | 2 +- test/fixtures/prViewPreviewWithReviewersByNumber.json | 6 ++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/command/pr.go b/command/pr.go index e1a65fcad..33de74aee 100644 --- a/command/pr.go +++ b/command/pr.go @@ -434,14 +434,20 @@ func prReviewerList(pr api.PullRequest) string { // 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 { - reviewerStates[review.Author.Login] = &reviewerState{ - Name: review.Author.Login, + name := review.Author.Login + if name == "" { + name = ghostName + } + reviewerStates[name] = &reviewerState{ + Name: name, State: review.State, } } diff --git a/command/pr_test.go b/command/pr_test.go index 147621fc2..06d403efe 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -442,7 +442,7 @@ func TestPRView_Preview(t *testing.T) { fixture: "../test/fixtures/prViewPreviewWithReviewersByNumber.json", expectedOutputs: []string{ `Blueberries are from a fork`, - `Reviewers: DEF \(Commented\), def \(Changes requested\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`, + `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`, }, diff --git a/test/fixtures/prViewPreviewWithReviewersByNumber.json b/test/fixtures/prViewPreviewWithReviewersByNumber.json index 049718b07..791512d41 100644 --- a/test/fixtures/prViewPreviewWithReviewersByNumber.json +++ b/test/fixtures/prViewPreviewWithReviewersByNumber.json @@ -64,6 +64,12 @@ "login": "xyz" }, "state": "APPROVED" + }, + { + "author": { + "login": "" + }, + "state": "APPROVED" } ] },