diff --git a/api/queries_pr.go b/api/queries_pr.go index 8a1d0e421..364784f9f 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -151,17 +151,27 @@ type PullRequestFile struct { type ReviewRequests struct { Nodes []struct { RequestedReviewer struct { - TypeName string `json:"__typename"` - Login string `json:"login"` - Name string `json:"name"` + TypeName string `json:"__typename"` + Login string `json:"login"` + Name string `json:"name"` + Slug string `json:"slug"` + Organization struct { + Login string `json:"login"` + } } } } +const teamTypeName = "Team" + func (r ReviewRequests) Logins() []string { logins := make([]string, len(r.Nodes)) for i, a := range r.Nodes { - logins[i] = a.RequestedReviewer.Login + if a.RequestedReviewer.TypeName == teamTypeName { + logins[i] = fmt.Sprintf("%s/%s", a.RequestedReviewer.Organization.Login, a.RequestedReviewer.Slug) + } else { + logins[i] = a.RequestedReviewer.Login + } } return logins } @@ -393,8 +403,8 @@ func PullRequestStatus(client *Client, repo ghrepo.Interface, options StatusOpti queryPrefix := ` query PullRequestStatus($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { - defaultBranchRef { - name + defaultBranchRef { + name } pullRequests(headRefName: $headRefName, first: $per_page, orderBy: { field: CREATED_AT, direction: DESC }) { totalCount @@ -410,8 +420,8 @@ func PullRequestStatus(client *Client, repo ghrepo.Interface, options StatusOpti queryPrefix = ` query PullRequestStatus($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { - defaultBranchRef { - name + defaultBranchRef { + name } pullRequest(number: $number) { ...prWithReviews diff --git a/api/queries_pr_test.go b/api/queries_pr_test.go index 886f16dd0..537c71268 100644 --- a/api/queries_pr_test.go +++ b/api/queries_pr_test.go @@ -1,12 +1,13 @@ package api import ( - "reflect" + "encoding/json" "testing" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/httpmock" + "github.com/stretchr/testify/assert" ) func TestBranchDeleteRemote(t *testing.T) { @@ -148,13 +149,94 @@ func Test_determinePullRequestFeatures(t *testing.T) { } gotPrFeatures, err := determinePullRequestFeatures(httpClient, tt.hostname) - if (err != nil) != tt.wantErr { - t.Errorf("determinePullRequestFeatures() error = %v, wantErr %v", err, tt.wantErr) + if tt.wantErr { + assert.Error(t, err) return + } else { + assert.NoError(t, err) } - if !reflect.DeepEqual(gotPrFeatures, tt.wantPrFeatures) { - t.Errorf("determinePullRequestFeatures() = %v, want %v", gotPrFeatures, tt.wantPrFeatures) - } + assert.Equal(t, tt.wantPrFeatures, gotPrFeatures) + }) + } +} + +func Test_Logins(t *testing.T) { + rr := ReviewRequests{} + var tests = []struct { + name string + requestedReviews string + want []string + }{ + { + name: "no requested reviewers", + requestedReviews: `{"nodes": []}`, + want: []string{}, + }, + { + name: "user", + requestedReviews: `{"nodes": [ + { + "requestedreviewer": { + "__typename": "User", "login": "testuser" + } + } + ]}`, + want: []string{"testuser"}, + }, + { + name: "team", + requestedReviews: `{"nodes": [ + { + "requestedreviewer": { + "__typename": "Team", + "name": "Test Team", + "slug": "test-team", + "organization": {"login": "myorg"} + } + } + ]}`, + want: []string{"myorg/test-team"}, + }, + { + name: "multiple users and teams", + requestedReviews: `{"nodes": [ + { + "requestedreviewer": { + "__typename": "User", "login": "user1" + } + }, + { + "requestedreviewer": { + "__typename": "User", "login": "user2" + } + }, + { + "requestedreviewer": { + "__typename": "Team", + "name": "Test Team", + "slug": "test-team", + "organization": {"login": "myorg"} + } + }, + { + "requestedreviewer": { + "__typename": "Team", + "name": "Dev Team", + "slug": "dev-team", + "organization": {"login": "myorg"} + } + } + ]}`, + want: []string{"user1", "user2", "myorg/test-team", "myorg/dev-team"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := json.Unmarshal([]byte(tt.requestedReviews), &rr) + assert.NoError(t, err, "Failed to unmarshal json string as ReviewRequests") + logins := rr.Logins() + assert.Equal(t, tt.want, logins) }) } } diff --git a/api/query_builder.go b/api/query_builder.go index 084e97195..3dc97b826 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -41,7 +41,11 @@ var prReviewRequests = shortenQuery(` requestedReviewer { __typename, ...on User{login}, - ...on Team{name} + ...on Team{ + organization{login} + name, + slug + } } } } diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index c543a84dc..f0942ab20 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -112,9 +112,14 @@ func listRun(opts *ListOptions) error { return err } + issueState := strings.ToLower(opts.State) + if issueState == "open" && shared.QueryHasStateClause(opts.Search) { + issueState = "" + } + filterOptions := prShared.FilterOptions{ Entity: "issue", - State: strings.ToLower(opts.State), + State: issueState, Assignee: opts.Assignee, Labels: opts.Labels, Author: opts.Author, diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index faf7a3e24..5810e8d7c 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -116,9 +116,14 @@ func listRun(opts *ListOptions) error { return err } + prState := strings.ToLower(opts.State) + if prState == "open" && shared.QueryHasStateClause(opts.Search) { + prState = "" + } + filters := shared.FilterOptions{ Entity: "pr", - State: strings.ToLower(opts.State), + State: prState, Author: opts.Author, Assignee: opts.Assignee, Labels: opts.Labels, diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index d8348ec2c..93a5f2f58 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -2,6 +2,7 @@ package shared import ( "fmt" + "github.com/google/shlex" "net/url" "strings" @@ -243,6 +244,21 @@ func SearchQueryBuild(options FilterOptions) string { return q.String() } +func QueryHasStateClause(searchQuery string) bool { + argv, err := shlex.Split(searchQuery) + if err != nil { + return false + } + + for _, arg := range argv { + if arg == "is:closed" || arg == "is:merged" || arg == "state:closed" || arg == "state:merged" || strings.HasPrefix(arg, "merged:") || strings.HasPrefix(arg, "closed:") { + return true + } + } + + return false +} + // MeReplacer resolves usages of `@me` to the handle of the currently logged in user. type MeReplacer struct { apiClient *api.Client diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index 74115c5b6..314218754 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -1,6 +1,7 @@ package shared import ( + "github.com/stretchr/testify/assert" "net/http" "reflect" "testing" @@ -151,3 +152,43 @@ func TestMeReplacer_Replace(t *testing.T) { }) } } + +func Test_QueryHasStateClause(t *testing.T) { + tests := []struct { + searchQuery string + hasState bool + }{ + { + searchQuery: "is:closed is:merged", + hasState: true, + }, + { + searchQuery: "author:mislav", + hasState: false, + }, + { + searchQuery: "assignee:g14a mentions:vilmibm", + hasState: false, + }, + { + searchQuery: "merged:>2021-05-20", + hasState: true, + }, + { + searchQuery: "state:merged state:open", + hasState: true, + }, + { + searchQuery: "assignee:g14a is:closed", + hasState: true, + }, + { + searchQuery: "state:closed label:bug", + hasState: true, + }, + } + for _, tt := range tests { + gotState := QueryHasStateClause(tt.searchQuery) + assert.Equal(t, tt.hasState, gotState) + } +}