Merge pull request #1462 from fsmiamoto/fix-milestone

Fix `--milestone` flag on `issues list` command

api/queries_issue.go

@@ -2,7 +2,10 @@ package api
 
 import (
 	"context"
+	"encoding/base64"
 	"fmt"
+	"strconv"
+	"strings"
 	"time"
 
 	"github.com/cli/cli/internal/ghrepo"
@@ -246,8 +249,26 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str
 	if mentionString != "" {
 		variables["mention"] = mentionString
 	}
+
 	if milestoneString != "" {
-		variables["milestone"] = milestoneString
+		var milestone *RepoMilestone
+		if milestoneNumber, err := strconv.Atoi(milestoneString); err == nil {
+			milestone, err = MilestoneByNumber(client, repo, milestoneNumber)
+			if err != nil {
+				return nil, err
+			}
+		} else {
+			milestone, err = MilestoneByTitle(client, repo, milestoneString)
+			if err != nil {
+				return nil, err
+			}
+		}
+
+		milestoneRESTID, err := milestoneNodeIdToDatabaseId(milestone.ID)
+		if err != nil {
+			return nil, err
+		}
+		variables["milestone"] = milestoneRESTID
 	}
 
 	var response struct {
@@ -419,3 +440,20 @@ func IssueReopen(client *Client, repo ghrepo.Interface, issue Issue) error {
 
 	return err
 }
+
+// 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
+}

api/queries_repo.go

@@ -804,3 +804,43 @@ func RepoMilestones(client *Client, repo ghrepo.Interface) ([]RepoMilestone, err
 
 	return milestones, nil
 }
+
+func MilestoneByTitle(client *Client, repo ghrepo.Interface, title string) (*RepoMilestone, error) {
+	milestones, err := RepoMilestones(client, repo)
+	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 int) (*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
+}

pkg/cmd/issue/list/list.go

@@ -47,6 +47,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
 			$ gh issue list -l "help wanted"
 			$ gh issue list -A monalisa
 			$ gh issue list --web
+			$ gh issue list --milestone 'MVP'
 		`),
 		Args: cmdutil.NoArgsQuoteReminder,
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -71,7 +72,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
 	cmd.Flags().IntVarP(&opts.LimitResults, "limit", "L", 30, "Maximum number of issues to fetch")
 	cmd.Flags().StringVarP(&opts.Author, "author", "A", "", "Filter by author")
 	cmd.Flags().StringVar(&opts.Mention, "mention", "", "Filter by mention")
-	cmd.Flags().StringVarP(&opts.Milestone, "milestone", "m", "", "Filter by milestone `name`")
+	cmd.Flags().StringVarP(&opts.Milestone, "milestone", "m", "", "Filter by milestone `number` or `title`")
 
 	return cmd
 }

pkg/cmd/issue/list/list_test.go

@@ -122,11 +122,20 @@ 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, "1.x", params["milestone"].(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{}))
 		}))
 
+	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 -l web,bug -s open -A foo --mention me --milestone 1.x")
 	if err != nil {
 		t.Errorf("error running command `issue list`: %v", err)
@@ -221,3 +230,48 @@ func TestIssueList_web(t *testing.T) {
 	url := seenCmd.Args[len(seenCmd.Args)-1]
 	eq(t, url, expectedURL)
 }
+
+func TestIssueList_milestoneNotFound(t *testing.T) {
+	http := &httpmock.Registry{}
+	defer http.Verify(t)
+
+	http.Register(
+		httpmock.GraphQL(`query RepositoryMilestoneList\b`),
+		httpmock.StringResponse(`
+		{ "data": { "repository": { "milestones": {
+			"nodes": [{ "title":"1.x", "id": "MDk6TWlsZXN0b25lMTIzNDU=" }],
+			"pageInfo": { "hasNextPage": false }
+		} } } }
+		`))
+
+	_, err := runCommand(http, true, "--milestone NotFound")
+	if err == nil || err.Error() != `no milestone found with title "NotFound"` {
+		t.Errorf("error running command `issue list`: %v", err)
+	}
+}
+
+func TestIssueList_milestoneByNumber(t *testing.T) {
+	http := &httpmock.Registry{}
+	defer http.Verify(t)
+	http.Register(
+		httpmock.GraphQL(`query RepositoryMilestoneByNumber\b`),
+		httpmock.StringResponse(`
+		{ "data": { "repository": { "milestone": {
+			"id": "MDk6TWlsZXN0b25lMTIzNDU="
+		} } } }
+		`))
+	http.Register(
+		httpmock.GraphQL(`query IssueList\b`),
+		httpmock.GraphQLQuery(`
+		{ "data": {	"repository": {
+			"hasIssuesEnabled": true,
+			"issues": { "nodes": [] }
+		} } }`, func(_ string, params map[string]interface{}) {
+			assert.Equal(t, "12345", params["milestone"].(string)) // Database ID for the Milestone (see #1462)
+		}))
+
+	_, err := runCommand(http, true, "--milestone 13")
+	if err != nil {
+		t.Fatalf("error running issue list: %v", err)
+	}
+}