From 1a980e768cdad166b30e8c3ae706c99c6c709bd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 28 May 2021 14:32:31 +0200 Subject: [PATCH] Fix how teams are displayed in requested reviewers 1. The `--json` export now only renders the `login` field for User types and `name` and `slug` fields for Team types. 2. The `pr view` command now renders team reviewers in the format of `ORG/SLUG` instead of the team name. This is so that the same value can be used in the `pr create -r` flag. --- api/export_pr.go | 16 +++++++-- api/queries_pr.go | 35 +++++++++++-------- .../prViewPreviewWithReviewersByNumber.json | 10 +++--- pkg/cmd/pr/view/view.go | 7 +--- pkg/cmd/pr/view/view_test.go | 4 +-- 5 files changed, 42 insertions(+), 30 deletions(-) diff --git a/api/export_pr.go b/api/export_pr.go index 4bd0aabc7d7..29a5c4a639a 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -80,10 +80,20 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { case "reviewRequests": requests := make([]interface{}, 0, len(pr.ReviewRequests.Nodes)) for _, req := range pr.ReviewRequests.Nodes { - if req.RequestedReviewer.TypeName == "" { - continue + r := req.RequestedReviewer + switch r.TypeName { + case "User": + requests = append(requests, map[string]string{ + "__typename": r.TypeName, + "login": r.Login, + }) + case "Team": + requests = append(requests, map[string]string{ + "__typename": r.TypeName, + "name": r.Name, + "slug": r.LoginOrSlug(), + }) } - requests = append(requests, req.RequestedReviewer) } data[f] = &requests default: diff --git a/api/queries_pr.go b/api/queries_pr.go index 364784f9f31..3032f636422 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -150,28 +150,33 @@ type PullRequestFile struct { type ReviewRequests struct { Nodes []struct { - RequestedReviewer struct { - TypeName string `json:"__typename"` - Login string `json:"login"` - Name string `json:"name"` - Slug string `json:"slug"` - Organization struct { - Login string `json:"login"` - } - } + RequestedReviewer RequestedReviewer + } +} + +type RequestedReviewer struct { + TypeName string `json:"__typename"` + Login string `json:"login"` + Name string `json:"name"` + Slug string `json:"slug"` + Organization struct { + Login string `json:"login"` + } `json:"organization"` +} + +func (r RequestedReviewer) LoginOrSlug() string { + if r.TypeName == teamTypeName { + return fmt.Sprintf("%s/%s", r.Organization.Login, r.Slug) } + return r.Login } const teamTypeName = "Team" func (r ReviewRequests) Logins() []string { logins := make([]string, len(r.Nodes)) - for i, a := range r.Nodes { - if a.RequestedReviewer.TypeName == teamTypeName { - logins[i] = fmt.Sprintf("%s/%s", a.RequestedReviewer.Organization.Login, a.RequestedReviewer.Slug) - } else { - logins[i] = a.RequestedReviewer.Login - } + for i, r := range r.Nodes { + logins[i] = r.RequestedReviewer.LoginOrSlug() } return logins } diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json b/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json index 59b1ed1a7e6..1b1a1e30437 100644 --- a/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json @@ -21,10 +21,12 @@ } }, { - "requestedReviewer": { - "__typename": "Team", - "name": "Team 1" - } + "requestedReviewer": { + "__typename": "Team", + "name": "Team 1", + "slug": "team-1", + "organization": {"login": "my-org"} + } }, { "requestedReviewer": { diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 91087d74b6c..702763befa7 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -294,8 +294,6 @@ func prReviewerList(pr api.PullRequest, cs *iostreams.ColorScheme) string { return reviewerList } -const teamTypeName = "Team" - const ghostName = "ghost" // parseReviewers parses given Reviews and ReviewRequests @@ -317,10 +315,7 @@ 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 { - name := reviewRequest.RequestedReviewer.Login - if reviewRequest.RequestedReviewer.TypeName == teamTypeName { - name = reviewRequest.RequestedReviewer.Name - } + name := reviewRequest.RequestedReviewer.LoginOrSlug() reviewerStates[name] = &reviewerState{ Name: name, State: requestedReviewState, diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index 78d588f1ed6..ed844db5032 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -250,7 +250,7 @@ func TestPRView_Preview_nontty(t *testing.T) { `milestone:\t\n`, `additions:\t100\n`, `deletions:\t10\n`, - `reviewers:\tDEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`, + `reviewers:\tDEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), abc \(Requested\), my-org\/team-1 \(Requested\)\n`, `\*\*blueberries taste good\*\*`, }, }, @@ -383,7 +383,7 @@ func TestPRView_Preview(t *testing.T) { }, expectedOutputs: []string{ `Blueberries are from a fork`, - `Reviewers:.*DEF \(.*Commented.*\), def \(.*Changes requested.*\), ghost \(.*Approved.*\), hubot \(Commented\), xyz \(.*Approved.*\), 123 \(.*Requested.*\), Team 1 \(.*Requested.*\), abc \(.*Requested.*\)\n`, + `Reviewers: DEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), abc \(Requested\), my-org\/team-1 \(Requested\)`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, },