diff --git a/api/queries_repo.go b/api/queries_repo.go index 796c2ed45..1c8a210fb 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1137,46 +1137,6 @@ func RepoMilestones(client *Client, repo ghrepo.Interface, state string) ([]Repo return milestones, nil } -func MilestoneByTitle(client *Client, repo ghrepo.Interface, state, title string) (*RepoMilestone, error) { - milestones, err := RepoMilestones(client, repo, state) - if err != nil { - return nil, err - } - - for i := range milestones { - if strings.EqualFold(milestones[i].Title, title) { - return &milestones[i], nil - } - } - return nil, fmt.Errorf("no milestone found with title %q", title) -} - -func MilestoneByNumber(client *Client, repo ghrepo.Interface, number int32) (*RepoMilestone, error) { - var query struct { - Repository struct { - Milestone *RepoMilestone `graphql:"milestone(number: $number)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - variables := map[string]interface{}{ - "owner": githubv4.String(repo.RepoOwner()), - "name": githubv4.String(repo.RepoName()), - "number": githubv4.Int(number), - } - - gql := graphQLClient(client.http, repo.RepoHost()) - - err := gql.QueryNamed(context.Background(), "RepositoryMilestoneByNumber", &query, variables) - if err != nil { - return nil, err - } - if query.Repository.Milestone == nil { - return nil, fmt.Errorf("no milestone found with number '%d'", number) - } - - return query.Repository.Milestone, nil -} - func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []string) ([]string, error) { var paths []string projects, err := RepoAndOrgProjects(client, repo) diff --git a/pkg/cmd/issue/list/http.go b/pkg/cmd/issue/list/http.go index a992b553d..40a32d748 100644 --- a/pkg/cmd/issue/list/http.go +++ b/pkg/cmd/issue/list/http.go @@ -1,10 +1,7 @@ package list import ( - "encoding/base64" "fmt" - "strconv" - "strings" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" @@ -26,10 +23,10 @@ func listIssues(client *api.Client, repo ghrepo.Interface, filters prShared.Filt fragments := fmt.Sprintf("fragment issue on Issue {%s}", api.PullRequestGraphQL(filters.Fields)) query := fragments + ` - query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $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) { repository(owner: $owner, name: $repo) { hasIssuesEnabled - issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, 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}) { totalCount nodes { ...issue @@ -59,24 +56,10 @@ func listIssues(client *api.Client, repo ghrepo.Interface, filters prShared.Filt } if filters.Milestone != "" { - var milestone *api.RepoMilestone - if milestoneNumber, err := strconv.ParseInt(filters.Milestone, 10, 32); err == nil { - milestone, err = api.MilestoneByNumber(client, repo, int32(milestoneNumber)) - if err != nil { - return nil, err - } - } else { - milestone, err = api.MilestoneByTitle(client, repo, "all", filters.Milestone) - if err != nil { - return nil, err - } - } - - milestoneRESTID, err := milestoneNodeIdToDatabaseId(milestone.ID) - if err != nil { - return nil, err - } - variables["milestone"] = milestoneRESTID + // The "milestone" filter in the GraphQL connection doesn't work as documented and accepts neither a + // milestone number nor a title. It does accept a numeric database ID, but we cannot obtain one + // using the GraphQL API. + return nil, fmt.Errorf("cannot filter by milestone using the `Repository.issues` GraphQL connection") } type responseData struct { @@ -204,23 +187,6 @@ loop: return &ic, nil } -// milestoneNodeIdToDatabaseId extracts the REST Database ID from the GraphQL Node ID -// This conversion is necessary since the GraphQL API requires the use of the milestone's database ID -// for querying the related issues. -func milestoneNodeIdToDatabaseId(nodeId string) (string, error) { - // The Node ID is Base64 obfuscated, with an underlying pattern: - // "09:Milestone12345", where "12345" is the database ID - decoded, err := base64.StdEncoding.DecodeString(nodeId) - if err != nil { - return "", err - } - splitted := strings.Split(string(decoded), "Milestone") - if len(splitted) != 2 { - return "", fmt.Errorf("couldn't get database id from node id") - } - return splitted[1], nil -} - func min(a, b int) int { if a < b { return a diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index 8e26a8afe..bf0e9bae6 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -1,6 +1,7 @@ package list import ( + "context" "fmt" "net/http" "strconv" @@ -9,6 +10,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared" "github.com/cli/cli/v2/pkg/cmd/pr/shared" @@ -16,6 +18,8 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/utils" + graphql "github.com/cli/shurcooL-graphql" + "github.com/shurcooL/githubv4" "github.com/spf13/cobra" ) @@ -182,9 +186,9 @@ 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 != "" || len(filters.Labels) > 0 { + if filters.Search != "" || len(filters.Labels) > 0 || filters.Milestone != "" { if milestoneNumber, err := strconv.ParseInt(filters.Milestone, 10, 32); err == nil { - milestone, err := api.MilestoneByNumber(apiClient, repo, int32(milestoneNumber)) + milestone, err := milestoneByNumber(client, repo, int32(milestoneNumber)) if err != nil { return nil, err } @@ -211,3 +215,27 @@ func issueList(client *http.Client, repo ghrepo.Interface, filters prShared.Filt return listIssues(apiClient, repo, filters, limit) } + +func milestoneByNumber(client *http.Client, repo ghrepo.Interface, number int32) (*api.RepoMilestone, error) { + var query struct { + Repository struct { + Milestone *api.RepoMilestone `graphql:"milestone(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.RepoOwner()), + "name": githubv4.String(repo.RepoName()), + "number": githubv4.Int(number), + } + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), client) + if err := gql.QueryNamed(context.Background(), "RepositoryMilestoneByNumber", &query, variables); err != nil { + return nil, err + } + if query.Repository.Milestone == nil { + return nil, fmt.Errorf("no milestone found with number '%d'", number) + } + + return query.Repository.Milestone, nil +} diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index cce2849f9..732db3488 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -121,20 +121,10 @@ func TestIssueList_tty_withFlags(t *testing.T) { assert.Equal(t, "probablyCher", params["assignee"].(string)) 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{}{"OPEN"}, params["states"].([]interface{})) })) - http.Register( - httpmock.GraphQL(`query RepositoryMilestoneList\b`), - httpmock.StringResponse(` - { "data": { "repository": { "milestones": { - "nodes": [{ "title":"1.x", "id": "MDk6TWlsZXN0b25lMTIzNDU=" }], - "pageInfo": { "hasNextPage": false } - } } } } - `)) - - output, err := runCommand(http, true, "-a probablyCher -s open -A foo --mention me --milestone 1.x") + output, err := runCommand(http, true, "-a probablyCher -s open -A foo --mention me") if err != nil { t.Errorf("error running command `issue list`: %v", err) } @@ -269,45 +259,7 @@ func Test_issueList(t *testing.T) { httpmock.GraphQL(`query RepositoryMilestoneByNumber\b`), httpmock.StringResponse(` { "data": { "repository": { "milestone": { - "id": "MDk6TWlsZXN0b25lMTIzNDU=" - } } } } - `)) - reg.Register( - httpmock.GraphQL(`query IssueList\b`), - httpmock.GraphQLQuery(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issues": { "nodes": [] } - } } }`, func(_ string, params map[string]interface{}) { - assert.Equal(t, map[string]interface{}{ - "owner": "OWNER", - "repo": "REPO", - "limit": float64(30), - "states": []interface{}{"OPEN"}, - "milestone": "12345", - }, params) - })) - }, - }, - { - name: "milestone by number with search", - args: args{ - limit: 30, - repo: ghrepo.New("OWNER", "REPO"), - filters: prShared.FilterOptions{ - Entity: "issue", - State: "open", - Milestone: "13", - Search: "auth bug", - }, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query RepositoryMilestoneByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { "milestone": { - "title": "Big 1.0", - "id": "MDk6TWlsZXN0b25lMTIzNDU=" + "title": "1.x" } } } } `)) reg.Register( @@ -324,40 +276,7 @@ func Test_issueList(t *testing.T) { "owner": "OWNER", "repo": "REPO", "limit": float64(30), - "query": "repo:OWNER/REPO is:issue is:open milestone:\"Big 1.0\" auth bug", - "type": "ISSUE", - }, params) - })) - }, - }, - { - name: "milestone by title with search", - args: args{ - limit: 30, - repo: ghrepo.New("OWNER", "REPO"), - filters: prShared.FilterOptions{ - Entity: "issue", - State: "open", - Milestone: "Big 1.0", - Search: "auth bug", - }, - }, - 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 milestone:\"Big 1.0\" auth bug", + "query": "repo:OWNER/REPO is:issue is:open milestone:1.x", "type": "ISSUE", }, params) })) @@ -376,26 +295,21 @@ func Test_issueList(t *testing.T) { }, httpStubs: func(reg *httpmock.Registry) { reg.Register( - httpmock.GraphQL(`query RepositoryMilestoneList\b`), - httpmock.StringResponse(` - { "data": { "repository": { "milestones": { - "nodes": [{ "title":"1.x", "id": "MDk6TWlsZXN0b25lMTIzNDU=" }], - "pageInfo": { "hasNextPage": false } - } } } } - `)) - reg.Register( - httpmock.GraphQL(`query IssueList\b`), + httpmock.GraphQL(`query IssueSearch\b`), httpmock.GraphQLQuery(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issues": { "nodes": [] } - } } }`, func(_ string, params map[string]interface{}) { + { "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), - "states": []interface{}{"OPEN"}, - "milestone": "12345", + "owner": "OWNER", + "repo": "REPO", + "limit": float64(30), + "query": "repo:OWNER/REPO is:issue is:open milestone:1.x", + "type": "ISSUE", }, params) })) },