Skip to content

Commit d23460a

Browse files
mislavmislav
committed
Fix filtering issues by milestone
The milestone filter in the `Repository.issues` GraphQL connection is broken, so switch to the Search API for any milestone filtering. Previously, we used to work around this by obtaining the milestone database ID from decoding the GraphQL ID, but that no longer works since the GraphQL ID format has changed.
1 parent 8198cce commit d23460a

File tree

4 files changed

+42
-174
lines changed

4 files changed

+42
-174
lines changed

‎api/queries_repo.go‎

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,46 +1137,6 @@ func RepoMilestones(client *Client, repo ghrepo.Interface, state string) ([]Repo
11371137
return milestones, nil
11381138
}
11391139

1140-
func MilestoneByTitle(client *Client, repo ghrepo.Interface, state, title string) (*RepoMilestone, error) {
1141-
milestones, err := RepoMilestones(client, repo, state)
1142-
if err != nil {
1143-
return nil, err
1144-
}
1145-
1146-
for i := range milestones {
1147-
if strings.EqualFold(milestones[i].Title, title) {
1148-
return &milestones[i], nil
1149-
}
1150-
}
1151-
return nil, fmt.Errorf("no milestone found with title %q", title)
1152-
}
1153-
1154-
func MilestoneByNumber(client *Client, repo ghrepo.Interface, number int32) (*RepoMilestone, error) {
1155-
var query struct {
1156-
Repository struct {
1157-
Milestone *RepoMilestone `graphql:"milestone(number: $number)"`
1158-
} `graphql:"repository(owner: $owner, name: $name)"`
1159-
}
1160-
1161-
variables := map[string]interface{}{
1162-
"owner": githubv4.String(repo.RepoOwner()),
1163-
"name": githubv4.String(repo.RepoName()),
1164-
"number": githubv4.Int(number),
1165-
}
1166-
1167-
gql := graphQLClient(client.http, repo.RepoHost())
1168-
1169-
err := gql.QueryNamed(context.Background(), "RepositoryMilestoneByNumber", &query, variables)
1170-
if err != nil {
1171-
return nil, err
1172-
}
1173-
if query.Repository.Milestone == nil {
1174-
return nil, fmt.Errorf("no milestone found with number '%d'", number)
1175-
}
1176-
1177-
return query.Repository.Milestone, nil
1178-
}
1179-
11801140
func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []string) ([]string, error) {
11811141
var paths []string
11821142
projects, err := RepoAndOrgProjects(client, repo)

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

Lines changed: 6 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package list
22

33
import (
4-
"encoding/base64"
54
"fmt"
6-
"strconv"
7-
"strings"
85

96
"github.com/cli/cli/v2/api"
107
"github.com/cli/cli/v2/internal/ghrepo"
@@ -26,10 +23,10 @@ func listIssues(client *api.Client, repo ghrepo.Interface, filters prShared.Filt
2623

2724
fragments := fmt.Sprintf("fragment issue on Issue {%s}", api.PullRequestGraphQL(filters.Fields))
2825
query := fragments + `
29-
query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $assignee: String, $author: String, $mention: String, $milestone: String) {
26+
query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $assignee: String, $author: String, $mention: String) {
3027
repository(owner: $owner, name: $repo) {
3128
hasIssuesEnabled
32-
issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, filterBy: {assignee: $assignee, createdBy: $author, mentioned: $mention, milestone: $milestone}) {
29+
issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, filterBy: {assignee: $assignee, createdBy: $author, mentioned: $mention}) {
3330
totalCount
3431
nodes {
3532
...issue
@@ -59,24 +56,10 @@ func listIssues(client *api.Client, repo ghrepo.Interface, filters prShared.Filt
5956
}
6057

6158
if filters.Milestone != "" {
62-
var milestone *api.RepoMilestone
63-
if milestoneNumber, err := strconv.ParseInt(filters.Milestone, 10, 32); err == nil {
64-
milestone, err = api.MilestoneByNumber(client, repo, int32(milestoneNumber))
65-
if err != nil {
66-
return nil, err
67-
}
68-
} else {
69-
milestone, err = api.MilestoneByTitle(client, repo, "all", filters.Milestone)
70-
if err != nil {
71-
return nil, err
72-
}
73-
}
74-
75-
milestoneRESTID, err := milestoneNodeIdToDatabaseId(milestone.ID)
76-
if err != nil {
77-
return nil, err
78-
}
79-
variables["milestone"] = milestoneRESTID
59+
// The "milestone" filter in the GraphQL connection doesn't work as documented and accepts neither a
60+
// milestone number nor a title. It does accept a numeric database ID, but we cannot obtain one
61+
// using the GraphQL API.
62+
return nil, fmt.Errorf("cannot filter by milestone using the `Repository.issues` GraphQL connection")
8063
}
8164

8265
type responseData struct {
@@ -204,23 +187,6 @@ loop:
204187
return &ic, nil
205188
}
206189

207-
// milestoneNodeIdToDatabaseId extracts the REST Database ID from the GraphQL Node ID
208-
// This conversion is necessary since the GraphQL API requires the use of the milestone's database ID
209-
// for querying the related issues.
210-
func milestoneNodeIdToDatabaseId(nodeId string) (string, error) {
211-
// The Node ID is Base64 obfuscated, with an underlying pattern:
212-
// "09:Milestone12345", where "12345" is the database ID
213-
decoded, err := base64.StdEncoding.DecodeString(nodeId)
214-
if err != nil {
215-
return "", err
216-
}
217-
splitted := strings.Split(string(decoded), "Milestone")
218-
if len(splitted) != 2 {
219-
return "", fmt.Errorf("couldn't get database id from node id")
220-
}
221-
return splitted[1], nil
222-
}
223-
224190
func min(a, b int) int {
225191
if a < b {
226192
return a

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package list
22

33
import (
4+
"context"
45
"fmt"
56
"net/http"
67
"strconv"
@@ -9,13 +10,16 @@ import (
910
"github.com/MakeNowJust/heredoc"
1011
"github.com/cli/cli/v2/api"
1112
"github.com/cli/cli/v2/internal/config"
13+
"github.com/cli/cli/v2/internal/ghinstance"
1214
"github.com/cli/cli/v2/internal/ghrepo"
1315
issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared"
1416
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
1517
prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared"
1618
"github.com/cli/cli/v2/pkg/cmdutil"
1719
"github.com/cli/cli/v2/pkg/iostreams"
1820
"github.com/cli/cli/v2/utils"
21+
graphql "github.com/cli/shurcooL-graphql"
22+
"github.com/shurcooL/githubv4"
1923
"github.com/spf13/cobra"
2024
)
2125

@@ -182,9 +186,9 @@ func listRun(opts *ListOptions) error {
182186
func issueList(client *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.IssuesAndTotalCount, error) {
183187
apiClient := api.NewClientFromHTTP(client)
184188

185-
if filters.Search != "" || len(filters.Labels) > 0 {
189+
if filters.Search != "" || len(filters.Labels) > 0 || filters.Milestone != "" {
186190
if milestoneNumber, err := strconv.ParseInt(filters.Milestone, 10, 32); err == nil {
187-
milestone, err := api.MilestoneByNumber(apiClient, repo, int32(milestoneNumber))
191+
milestone, err := milestoneByNumber(client, repo, int32(milestoneNumber))
188192
if err != nil {
189193
return nil, err
190194
}
@@ -211,3 +215,27 @@ func issueList(client *http.Client, repo ghrepo.Interface, filters prShared.Filt
211215

212216
return listIssues(apiClient, repo, filters, limit)
213217
}
218+
219+
func milestoneByNumber(client *http.Client, repo ghrepo.Interface, number int32) (*api.RepoMilestone, error) {
220+
var query struct {
221+
Repository struct {
222+
Milestone *api.RepoMilestone `graphql:"milestone(number: $number)"`
223+
} `graphql:"repository(owner: $owner, name: $name)"`
224+
}
225+
226+
variables := map[string]interface{}{
227+
"owner": githubv4.String(repo.RepoOwner()),
228+
"name": githubv4.String(repo.RepoName()),
229+
"number": githubv4.Int(number),
230+
}
231+
232+
gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), client)
233+
if err := gql.QueryNamed(context.Background(), "RepositoryMilestoneByNumber", &query, variables); err != nil {
234+
return nil, err
235+
}
236+
if query.Repository.Milestone == nil {
237+
return nil, fmt.Errorf("no milestone found with number '%d'", number)
238+
}
239+
240+
return query.Repository.Milestone, nil
241+
}

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

Lines changed: 6 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -121,20 +121,10 @@ func TestIssueList_tty_withFlags(t *testing.T) {
121121
assert.Equal(t, "probablyCher", params["assignee"].(string))
122122
assert.Equal(t, "foo", params["author"].(string))
123123
assert.Equal(t, "me", params["mention"].(string))
124-
assert.Equal(t, "12345", params["milestone"].(string))
125124
assert.Equal(t, []interface{}{"OPEN"}, params["states"].([]interface{}))
126125
}))
127126

128-
http.Register(
129-
httpmock.GraphQL(`query RepositoryMilestoneList\b`),
130-
httpmock.StringResponse(`
131-
{ "data": { "repository": { "milestones": {
132-
"nodes": [{ "title":"1.x", "id": "MDk6TWlsZXN0b25lMTIzNDU=" }],
133-
"pageInfo": { "hasNextPage": false }
134-
} } } }
135-
`))
136-
137-
output, err := runCommand(http, true, "-a probablyCher -s open -A foo --mention me --milestone 1.x")
127+
output, err := runCommand(http, true, "-a probablyCher -s open -A foo --mention me")
138128
if err != nil {
139129
t.Errorf("error running command `issue list`: %v", err)
140130
}
@@ -269,45 +259,7 @@ func Test_issueList(t *testing.T) {
269259
httpmock.GraphQL(`query RepositoryMilestoneByNumber\b`),
270260
httpmock.StringResponse(`
271261
{ "data": { "repository": { "milestone": {
272-
"id": "MDk6TWlsZXN0b25lMTIzNDU="
273-
} } } }
274-
`))
275-
reg.Register(
276-
httpmock.GraphQL(`query IssueList\b`),
277-
httpmock.GraphQLQuery(`
278-
{ "data": { "repository": {
279-
"hasIssuesEnabled": true,
280-
"issues": { "nodes": [] }
281-
} } }`, func(_ string, params map[string]interface{}) {
282-
assert.Equal(t, map[string]interface{}{
283-
"owner": "OWNER",
284-
"repo": "REPO",
285-
"limit": float64(30),
286-
"states": []interface{}{"OPEN"},
287-
"milestone": "12345",
288-
}, params)
289-
}))
290-
},
291-
},
292-
{
293-
name: "milestone by number with search",
294-
args: args{
295-
limit: 30,
296-
repo: ghrepo.New("OWNER", "REPO"),
297-
filters: prShared.FilterOptions{
298-
Entity: "issue",
299-
State: "open",
300-
Milestone: "13",
301-
Search: "auth bug",
302-
},
303-
},
304-
httpStubs: func(reg *httpmock.Registry) {
305-
reg.Register(
306-
httpmock.GraphQL(`query RepositoryMilestoneByNumber\b`),
307-
httpmock.StringResponse(`
308-
{ "data": { "repository": { "milestone": {
309-
"title": "Big 1.0",
310-
"id": "MDk6TWlsZXN0b25lMTIzNDU="
262+
"title": "1.x"
311263
} } } }
312264
`))
313265
reg.Register(
@@ -324,22 +276,21 @@ func Test_issueList(t *testing.T) {
324276
"owner": "OWNER",
325277
"repo": "REPO",
326278
"limit": float64(30),
327-
"query": "repo:OWNER/REPO is:issue is:open milestone:\"Big 1.0\" auth bug",
279+
"query": "repo:OWNER/REPO is:issue is:open milestone:1.x",
328280
"type": "ISSUE",
329281
}, params)
330282
}))
331283
},
332284
},
333285
{
334-
name: "milestone by title with search",
286+
name: "milestone by title",
335287
args: args{
336288
limit: 30,
337289
repo: ghrepo.New("OWNER", "REPO"),
338290
filters: prShared.FilterOptions{
339291
Entity: "issue",
340292
State: "open",
341-
Milestone: "Big 1.0",
342-
Search: "auth bug",
293+
Milestone: "1.x",
343294
},
344295
},
345296
httpStubs: func(reg *httpmock.Registry) {
@@ -357,49 +308,12 @@ func Test_issueList(t *testing.T) {
357308
"owner": "OWNER",
358309
"repo": "REPO",
359310
"limit": float64(30),
360-
"query": "repo:OWNER/REPO is:issue is:open milestone:\"Big 1.0\" auth bug",
311+
"query": "repo:OWNER/REPO is:issue is:open milestone:1.x",
361312
"type": "ISSUE",
362313
}, params)
363314
}))
364315
},
365316
},
366-
{
367-
name: "milestone by title",
368-
args: args{
369-
limit: 30,
370-
repo: ghrepo.New("OWNER", "REPO"),
371-
filters: prShared.FilterOptions{
372-
Entity: "issue",
373-
State: "open",
374-
Milestone: "1.x",
375-
},
376-
},
377-
httpStubs: func(reg *httpmock.Registry) {
378-
reg.Register(
379-
httpmock.GraphQL(`query RepositoryMilestoneList\b`),
380-
httpmock.StringResponse(`
381-
{ "data": { "repository": { "milestones": {
382-
"nodes": [{ "title":"1.x", "id": "MDk6TWlsZXN0b25lMTIzNDU=" }],
383-
"pageInfo": { "hasNextPage": false }
384-
} } } }
385-
`))
386-
reg.Register(
387-
httpmock.GraphQL(`query IssueList\b`),
388-
httpmock.GraphQLQuery(`
389-
{ "data": { "repository": {
390-
"hasIssuesEnabled": true,
391-
"issues": { "nodes": [] }
392-
} } }`, func(_ string, params map[string]interface{}) {
393-
assert.Equal(t, map[string]interface{}{
394-
"owner": "OWNER",
395-
"repo": "REPO",
396-
"limit": float64(30),
397-
"states": []interface{}{"OPEN"},
398-
"milestone": "12345",
399-
}, params)
400-
}))
401-
},
402-
},
403317
{
404318
name: "@me syntax",
405319
args: args{

0 commit comments

Comments
 (0)