STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

BREAKING: lookup all issue/PR labels with "AND" instead of "OR" combinator

Browse source 
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.
This commit is contained in:
Mislav Marohnić 2021-03-24 17:52:29 +01:00
parent 0131541bb2
commit 949df38d49
7 changed files with 57 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

9
api/queries_issue.go
View file

@ -233,7 +233,7 @@ func IssueStatus(client *Client, repo ghrepo.Interface, currentUsername string)
return &payload, nil
}
func IssueList(client *Client, repo ghrepo.Interface, state string, labels []string, assigneeString string, limit int, authorString string, mentionString string, milestoneString string) (*IssuesAndTotalCount, error) {
func IssueList(client *Client, repo ghrepo.Interface, state string, assigneeString string, limit int, authorString string, mentionString string, milestoneString string) (*IssuesAndTotalCount, error) {
var states []string
switch state {
case "open", "":
@ -247,10 +247,10 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str
}
query := fragments + `
query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String, $author: String, $mention: String, $milestone: String) {
query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $assignee: String, $author: String, $mention: String, $milestone: String) {
repository(owner: $owner, name: $repo) {
hasIssuesEnabled
issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee, createdBy: $author, mentioned: $mention, milestone: $milestone}) {
issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, filterBy: {assignee: $assignee, createdBy: $author, mentioned: $mention, milestone: $milestone}) {
totalCount
nodes {
...issue
@ -269,9 +269,6 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str
"repo": repo.RepoName(),
"states": states,
}
if len(labels) > 0 {
variables["labels"] = labels
}
if assigneeString != "" {
variables["assignee"] = assigneeString
}

4
api/queries_issue_test.go
View file

@ -47,7 +47,7 @@ func TestIssueList(t *testing.T) {
)
repo, _ := ghrepo.FromFullName("OWNER/REPO")
_, err := IssueList(client, repo, "open", []string{}, "", 251, "", "", "")
_, err := IssueList(client, repo, "open", "", 251, "", "", "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@ -127,7 +127,7 @@ func TestIssueList_pagination(t *testing.T) {
)
repo := ghrepo.New("OWNER", "REPO")
res, err := IssueList(client, repo, "", nil, "", 0, "", "", "")
res, err := IssueList(client, repo, "", "", 0, "", "", "")
if err != nil {
t.Fatalf("IssueList() error = %v", err)
}

3
pkg/cmd/issue/list/list.go
View file

@ -145,7 +145,7 @@ func listRun(opts *ListOptions) error {
func issueList(client *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.IssuesAndTotalCount, error) {
apiClient := api.NewClientFromHTTP(client)
if filters.Search != "" {
if filters.Search != "" || len(filters.Labels) > 0 {
if milestoneNumber, err := strconv.ParseInt(filters.Milestone, 10, 32); err == nil {
milestone, err := api.MilestoneByNumber(apiClient, repo, int32(milestoneNumber))
if err != nil {
@ -176,7 +176,6 @@ func issueList(client *http.Client, repo ghrepo.Interface, filters prShared.Filt
apiClient,
repo,
filters.State,
filters.Labels,
filterAssignee,
limit,
filterAuthor,

35
pkg/cmd/issue/list/list_test.go
View file

@ -123,7 +123,6 @@ func TestIssueList_tty_withFlags(t *testing.T) {
assert.Equal(t, "foo", params["author"].(string))
assert.Equal(t, "me", params["mention"].(string))
assert.Equal(t, "12345", params["milestone"].(string))
assert.Equal(t, []interface{}{"web", "bug"}, params["labels"].([]interface{}))
assert.Equal(t, []interface{}{"OPEN"}, params["states"].([]interface{}))
}))
@ -136,7 +135,7 @@ func TestIssueList_tty_withFlags(t *testing.T) {
} } } }
`))
output, err := runCommand(http, true, "-a probablyCher -l web,bug -s open -A foo --mention me --milestone 1.x")
output, err := runCommand(http, true, "-a probablyCher -s open -A foo --mention me --milestone 1.x")
if err != nil {
t.Errorf("error running command `issue list`: %v", err)
}
@ -476,6 +475,38 @@ func Test_issueList(t *testing.T) {
}))
},
},
{
name: "with labels",
args: args{
limit: 30,
repo: ghrepo.New("OWNER", "REPO"),
filters: prShared.FilterOptions{
Entity: "issue",
State: "open",
Labels: []string{"hello", "one world"},
},
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query IssueSearch\b`),
httpmock.GraphQLQuery(`
{ "data": {
"repository": { "hasIssuesEnabled": true },
"search": {
"issueCount": 0,
"nodes": []
}
} }`, func(_ string, params map[string]interface{}) {
assert.Equal(t, map[string]interface{}{
"owner": "OWNER",
"repo": "REPO",
"limit": float64(30),
"query": `repo:OWNER/REPO is:issue is:open label:hello label:"one world"`,
"type": "ISSUE",
}, params)
}))
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

17
pkg/cmd/pr/list/http.go
View file

@ -24,7 +24,7 @@ const fragment = `fragment pr on PullRequest {
}`
func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) {
if filters.Author != "" || filters.Assignee != "" || filters.Search != "" {
if filters.Author != "" || filters.Assignee != "" || filters.Search != "" || len(filters.Labels) > 0 {
return searchPullRequests(httpClient, repo, filters, limit)
}
@ -48,14 +48,12 @@ func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters pr
$limit: Int!,
$endCursor: String,
$baseBranch: String,
$labels: [String!],
$state: [PullRequestState!] = OPEN
) {
repository(owner: $owner, name: $repo) {
pullRequests(
states: $state,
baseRefName: $baseBranch,
labels: $labels,
first: $limit,
after: $endCursor,
orderBy: {field: CREATED_AT, direction: DESC}
@ -74,9 +72,8 @@ func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters pr
pageLimit := min(limit, 100)
variables := map[string]interface{}{
"owner": repo.RepoOwner(),
"repo": repo.RepoName(),
"labels": filters.Labels,
"owner": repo.RepoOwner(),
"repo": repo.RepoName(),
}
switch filters.State {
@ -135,10 +132,6 @@ loop:
}
func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) {
if len(filters.Labels) > 1 {
return nil, fmt.Errorf("multiple labels with --assignee are not supported")
}
type response struct {
Search struct {
Nodes []api.PullRequest
@ -189,8 +182,8 @@ func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters
if filters.Assignee != "" {
q.AssignedTo(filters.Assignee)
}
if len(filters.Labels) > 0 {
q.AddLabel(filters.Labels[0])
for _, label := range filters.Labels {
q.AddLabel(label)
}
if filters.BaseBranch != "" {
q.SetBaseBranch(filters.BaseBranch)

27
pkg/cmd/pr/list/http_test.go
View file

@ -36,11 +36,10 @@ func Test_listPullRequests(t *testing.T) {
httpmock.GraphQL(`query PullRequestList\b`),
httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) {
want := map[string]interface{}{
"owner": "OWNER",
"repo": "REPO",
"state": []interface{}{"OPEN"},
"labels": nil,
"limit": float64(30),
"owner": "OWNER",
"repo": "REPO",
"state": []interface{}{"OPEN"},
"limit": float64(30),
}
if !reflect.DeepEqual(vars, want) {
t.Errorf("got GraphQL variables %#v, want %#v", vars, want)
@ -62,11 +61,10 @@ func Test_listPullRequests(t *testing.T) {
httpmock.GraphQL(`query PullRequestList\b`),
httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) {
want := map[string]interface{}{
"owner": "OWNER",
"repo": "REPO",
"state": []interface{}{"CLOSED", "MERGED"},
"labels": nil,
"limit": float64(30),
"owner": "OWNER",
"repo": "REPO",
"state": []interface{}{"CLOSED", "MERGED"},
"limit": float64(30),
}
if !reflect.DeepEqual(vars, want) {
t.Errorf("got GraphQL variables %#v, want %#v", vars, want)
@ -86,14 +84,11 @@ func Test_listPullRequests(t *testing.T) {
},
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query PullRequestList\b`),
httpmock.GraphQL(`query PullRequestSearch\b`),
httpmock.GraphQLQuery(`{"data":{}}`, func(query string, vars map[string]interface{}) {
want := map[string]interface{}{
"owner": "OWNER",
"repo": "REPO",
"state": []interface{}{"OPEN"},
"labels": []interface{}{"hello", "one world"},
"limit": float64(30),
"q": `repo:OWNER/REPO is:pr is:open label:hello label:"one world" sort:created-desc`,
"limit": float64(30),
}
if !reflect.DeepEqual(vars, want) {
t.Errorf("got GraphQL variables %#v, want %#v", vars, want)

5
pkg/cmd/pr/list/list_test.go
View file

@ -106,10 +106,9 @@ func TestPRList_filtering(t *testing.T) {
httpmock.GraphQL(`query PullRequestList\b`),
httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) {
assert.Equal(t, []interface{}{"OPEN", "CLOSED", "MERGED"}, params["state"].([]interface{}))
assert.Equal(t, []interface{}{"one", "two", "three"}, params["labels"].([]interface{}))
}))
output, err := runCommand(http, true, `-s all -l one,two -l three`)
output, err := runCommand(http, true, `-s all`)
if err != nil {
t.Fatal(err)
}
@ -129,7 +128,7 @@ func TestPRList_filteringRemoveDuplicate(t *testing.T) {
httpmock.GraphQL(`query PullRequestList\b`),
httpmock.FileResponse("./fixtures/prListWithDuplicates.json"))
output, err := runCommand(http, true, "-l one,two")
output, err := runCommand(http, true, "")
if err != nil {
t.Fatal(err)
}