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.
This commit is contained in:
Mislav Marohnić
parent
8198cce59b
commit
d23460a590
4 changed files with 52 additions and 184 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}))
|
||||
},
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue