X Tutup
Skip to content

Commit 949df38

Browse files
mislavmislav
committed
BREAKING: lookup all issue/PR labels with "AND" instead of "OR" combinator
This switches to the Search API whenever labels are specified in `issue list` or `pr list`. This ensures that the results match those that would be returned in the web UI.
1 parent 0131541 commit 949df38

File tree

7 files changed

+57
-43
lines changed

7 files changed

+57
-43
lines changed

‎api/queries_issue.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ func IssueStatus(client *Client, repo ghrepo.Interface, currentUsername string)
233233
return &payload, nil
234234
}
235235

236-
func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string, mentionString string, milestoneString string) (*IssuesAndTotalCount, error) {
236+
func IssueList(client *Client, repo ghrepo.Interface, state string, assigneeString string, limit int, authorString string, mentionString string, milestoneString string) (*IssuesAndTotalCount, error) {
237237
var states []string
238238
switch state {
239239
case "open", "":
@@ -247,10 +247,10 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str
247247
}
248248

249249
query := fragments + `
250-
query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: String, $mention: String, $milestone: String) {
250+
query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $assignee: String, $author: String, $mention: String, $milestone: String) {
251251
repository(owner: $owner, name: $repo) {
252252
hasIssuesEnabled
253-
issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee, createdBy: $author, mentioned: $mention, milestone: $milestone}) {
253+
issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, filterBy: {assignee: $assignee, createdBy: $author, mentioned: $mention, milestone: $milestone}) {
254254
totalCount
255255
nodes {
256256
...issue
@@ -269,9 +269,6 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str
269269
"repo": repo.RepoName(),
270270
"states": states,
271271
}
272-
if len(labels) > 0 {
273-
variables["labels"] = labels
274-
}
275272
if assigneeString != "" {
276273
variables["assignee"] = assigneeString
277274
}

‎api/queries_issue_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func TestIssueList(t *testing.T) {
4747
)
4848

4949
repo, _ := ghrepo.FromFullName("OWNER/REPO")
50-
_, err := IssueList(client, repo, "open", []string{}, "", 251, "", "", "")
50+
_, err := IssueList(client, repo, "open", "", 251, "", "", "")
5151
if err != nil {
5252
t.Fatalf("unexpected error: %v", err)
5353
}
@@ -127,7 +127,7 @@ func TestIssueList_pagination(t *testing.T) {
127127
)
128128

129129
repo := ghrepo.New("OWNER", "REPO")
130-
res, err := IssueList(client, repo, "", nil, "", 0, "", "", "")
130+
res, err := IssueList(client, repo, "", "", 0, "", "", "")
131131
if err != nil {
132132
t.Fatalf("IssueList() error = %v", err)
133133
}

‎pkg/cmd/issue/list/list.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func listRun(opts *ListOptions) error {
145145
func issueList(client *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.IssuesAndTotalCount, error) {
146146
apiClient := api.NewClientFromHTTP(client)
147147

148-
if filters.Search != "" {
148+
if filters.Search != "" || len(filters.Labels) > 0 {
149149
if milestoneNumber, err := strconv.ParseInt(filters.Milestone, 10, 32); err == nil {
150150
milestone, err := api.MilestoneByNumber(apiClient, repo, int32(milestoneNumber))
151151
if err != nil {
@@ -176,7 +176,6 @@ func issueList(client *http.Client, repo ghrepo.Interface, filters prShared.Filt
176176
apiClient,
177177
repo,
178178
filters.State,
179-
filters.Labels,
180179
filterAssignee,
181180
limit,
182181
filterAuthor,

‎pkg/cmd/issue/list/list_test.go‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ func TestIssueList_tty_withFlags(t *testing.T) {
123123
assert.Equal(t, "foo", params["author"].(string))
124124
assert.Equal(t, "me", params["mention"].(string))
125125
assert.Equal(t, "12345", params["milestone"].(string))
126-
assert.Equal(t, []interface{}{"web", "bug"}, params["labels"].([]interface{}))
127126
assert.Equal(t, []interface{}{"OPEN"}, params["states"].([]interface{}))
128127
}))
129128

@@ -136,7 +135,7 @@ func TestIssueList_tty_withFlags(t *testing.T) {
136135
} } } }
137136
`))
138137

139-
output, err := runCommand(http, true, "-a probablyCher -l web,bug -s open -A foo --mention me --milestone 1.x")
138+
output, err := runCommand(http, true, "-a probablyCher -s open -A foo --mention me --milestone 1.x")
140139
if err != nil {
141140
t.Errorf("error running command `issue list`: %v", err)
142141
}
@@ -476,6 +475,38 @@ func Test_issueList(t *testing.T) {
476475
}))
477476
},
478477
},
478+
{
479+
name: "with labels",
480+
args: args{
481+
limit: 30,
482+
repo: ghrepo.New("OWNER", "REPO"),
483+
filters: prShared.FilterOptions{
484+
Entity: "issue",
485+
State: "open",
486+
Labels: []string{"hello", "one world"},
487+
},
488+
},
489+
httpStubs: func(reg *httpmock.Registry) {
490+
reg.Register(
491+
httpmock.GraphQL(`query IssueSearch\b`),
492+
httpmock.GraphQLQuery(`
493+
{ "data": {
494+
"repository": { "hasIssuesEnabled": true },
495+
"search": {
496+
"issueCount": 0,
497+
"nodes": []
498+
}
499+
} }`, func(_ string, params map[string]interface{}) {
500+
assert.Equal(t, map[string]interface{}{
501+
"owner": "OWNER",
502+
"repo": "REPO",
503+
"limit": float64(30),
504+
"query": `repo:OWNER/REPO is:issue is:open label:hello label:"one world"`,
505+
"type": "ISSUE",
506+
}, params)
507+
}))
508+
},
509+
},
479510
}
480511
for _, tt := range tests {
481512
t.Run(tt.name, func(t *testing.T) {

‎pkg/cmd/pr/list/http.go‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const fragment = `fragment pr on PullRequest {
2424
}`
2525

2626
func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) {
27-
if filters.Author != "" || filters.Assignee != "" || filters.Search != "" {
27+
if filters.Author != "" || filters.Assignee != "" || filters.Search != "" || len(filters.Labels) > 0 {
2828
return searchPullRequests(httpClient, repo, filters, limit)
2929
}
3030

@@ -48,14 +48,12 @@ func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters pr
4848
$limit: Int!,
4949
$endCursor: String,
5050
$baseBranch: String,
51-
$labels: [String!],
5251
$state: [PullRequestState!] = OPEN
5352
) {
5453
repository(owner: $owner, name: $repo) {
5554
pullRequests(
5655
states: $state,
5756
baseRefName: $baseBranch,
58-
labels: $labels,
5957
first: $limit,
6058
after: $endCursor,
6159
orderBy: {field: CREATED_AT, direction: DESC}
@@ -74,9 +72,8 @@ func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters pr
7472

7573
pageLimit := min(limit, 100)
7674
variables := map[string]interface{}{
77-
"owner": repo.RepoOwner(),
78-
"repo": repo.RepoName(),
79-
"labels": filters.Labels,
75+
"owner": repo.RepoOwner(),
76+
"repo": repo.RepoName(),
8077
}
8178

8279
switch filters.State {
@@ -135,10 +132,6 @@ loop:
135132
}
136133

137134
func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) {
138-
if len(filters.Labels) > 1 {
139-
return nil, fmt.Errorf("multiple labels with --assignee are not supported")
140-
}
141-
142135
type response struct {
143136
Search struct {
144137
Nodes []api.PullRequest
@@ -189,8 +182,8 @@ func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters
189182
if filters.Assignee != "" {
190183
q.AssignedTo(filters.Assignee)
191184
}
192-
if len(filters.Labels) > 0 {
193-
q.AddLabel(filters.Labels[0])
185+
for _, label := range filters.Labels {
186+
q.AddLabel(label)
194187
}
195188
if filters.BaseBranch != "" {
196189
q.SetBaseBranch(filters.BaseBranch)

‎pkg/cmd/pr/list/http_test.go‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,10 @@ func Test_listPullRequests(t *testing.T) {
3636
httpmock.GraphQL(`query PullRequestList\b`),
3737
httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) {
3838
want := map[string]interface{}{
39-
"owner": "OWNER",
40-
"repo": "REPO",
41-
"state": []interface{}{"OPEN"},
42-
"labels": nil,
43-
"limit": float64(30),
39+
"owner": "OWNER",
40+
"repo": "REPO",
41+
"state": []interface{}{"OPEN"},
42+
"limit": float64(30),
4443
}
4544
if !reflect.DeepEqual(vars, want) {
4645
t.Errorf("got GraphQL variables %#v, want %#v", vars, want)
@@ -62,11 +61,10 @@ func Test_listPullRequests(t *testing.T) {
6261
httpmock.GraphQL(`query PullRequestList\b`),
6362
httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) {
6463
want := map[string]interface{}{
65-
"owner": "OWNER",
66-
"repo": "REPO",
67-
"state": []interface{}{"CLOSED", "MERGED"},
68-
"labels": nil,
69-
"limit": float64(30),
64+
"owner": "OWNER",
65+
"repo": "REPO",
66+
"state": []interface{}{"CLOSED", "MERGED"},
67+
"limit": float64(30),
7068
}
7169
if !reflect.DeepEqual(vars, want) {
7270
t.Errorf("got GraphQL variables %#v, want %#v", vars, want)
@@ -86,14 +84,11 @@ func Test_listPullRequests(t *testing.T) {
8684
},
8785
httpStub: func(r *httpmock.Registry) {
8886
r.Register(
89-
httpmock.GraphQL(`query PullRequestList\b`),
87+
httpmock.GraphQL(`query PullRequestSearch\b`),
9088
httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) {
9189
want := map[string]interface{}{
92-
"owner": "OWNER",
93-
"repo": "REPO",
94-
"state": []interface{}{"OPEN"},
95-
"labels": []interface{}{"hello", "one world"},
96-
"limit": float64(30),
90+
"q": `repo:OWNER/REPO is:pr is:open label:hello label:"one world" sort:created-desc`,
91+
"limit": float64(30),
9792
}
9893
if !reflect.DeepEqual(vars, want) {
9994
t.Errorf("got GraphQL variables %#v, want %#v", vars, want)

‎pkg/cmd/pr/list/list_test.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,9 @@ func TestPRList_filtering(t *testing.T) {
106106
httpmock.GraphQL(`query PullRequestList\b`),
107107
httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) {
108108
assert.Equal(t, []interface{}{"OPEN", "CLOSED", "MERGED"}, params["state"].([]interface{}))
109-
assert.Equal(t, []interface{}{"one", "two", "three"}, params["labels"].([]interface{}))
110109
}))
111110

112-
output, err := runCommand(http, true, `-s all -l one,two -l three`)
111+
output, err := runCommand(http, true, `-s all`)
113112
if err != nil {
114113
t.Fatal(err)
115114
}
@@ -129,7 +128,7 @@ func TestPRList_filteringRemoveDuplicate(t *testing.T) {
129128
httpmock.GraphQL(`query PullRequestList\b`),
130129
httpmock.FileResponse("./fixtures/prListWithDuplicates.json"))
131130

132-
output, err := runCommand(http, true, "-l one,two")
131+
output, err := runCommand(http, true, "")
133132
if err != nil {
134133
t.Fatal(err)
135134
}

0 commit comments

Comments
 (0)
X Tutup