From 9a5e69a8a455753f4d37732da7913cc15dade7e4 Mon Sep 17 00:00:00 2001 From: Francisco Miamoto Date: Sat, 1 Aug 2020 23:43:36 -0300 Subject: [PATCH] use graphql for repo milestones on issue list Since we can obtain the necessary ID for the query on node ID from the GraphQL API, it makes sense to remove the REST version. --- api/queries_issue.go | 32 ++++++++++++++++++++++++++++---- api/queries_repo.go | 28 ---------------------------- api/queries_repo_test.go | 8 -------- 3 files changed, 28 insertions(+), 40 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index ec34db2e2..b027af472 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -2,8 +2,9 @@ package api import ( "context" + "encoding/base64" "fmt" - "strconv" + "strings" "time" "github.com/cli/cli/internal/ghrepo" @@ -249,13 +250,19 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str } if milestoneString != "" { - milestones, err := RepoMilestonesREST(client, repo) + milestones, err := RepoMilestones(client, repo) if err != nil { return nil, err } + for i := range milestones { - if milestones[i].Title == milestoneString { - variables["milestone"] = strconv.Itoa(milestones[i].ID) + if strings.EqualFold(milestones[i].Title, milestoneString) { + // The query for milestones requires the use of the milestone's database ID (see #1441) + id, err := milestoneNodeIdToDatabaseId(milestones[i].ID) + if err != nil { + return nil, err + } + variables["milestone"] = id break } } @@ -430,3 +437,20 @@ func IssueReopen(client *Client, repo ghrepo.Interface, issue Issue) error { return err } + +// milestoneNodeIdToDatabaseId extracts the REST Database ID from the GraphQL Node ID +func milestoneNodeIdToDatabaseId(id string) (string, error) { + // The node id is base64 obfuscated + decoded, err := base64.StdEncoding.DecodeString(id) + if err != nil { + return "", err + } + + // The decoded node ID has a pattern like '09:Milestone12345' + 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 +} diff --git a/api/queries_repo.go b/api/queries_repo.go index 1d617ae2e..06d4bfdab 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "sort" - "strconv" "strings" "time" @@ -802,30 +801,3 @@ func RepoMilestones(client *Client, repo ghrepo.Interface) ([]RepoMilestone, err return milestones, nil } - -type RepoMilestoneREST struct { - ID int - Title string -} - -func RepoMilestonesREST(client *Client, repo ghrepo.Interface) ([]RepoMilestoneREST, error) { - var allMilestones []RepoMilestoneREST - - path := fmt.Sprintf("repos/%s/%s/milestones", repo.RepoOwner(), repo.RepoName()) - - for page := 1; ; page++ { - var milestonesFromPage []RepoMilestoneREST - - err := client.REST("GET", path+"?page="+strconv.Itoa(page), nil, &milestonesFromPage) - if err != nil { - return nil, err - } - - if len(milestonesFromPage) == 0 { - break - } - allMilestones = append(allMilestones, milestonesFromPage...) - } - - return allMilestones, nil -} diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 2d2cb93ad..ad0e36864 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -54,14 +54,6 @@ func Test_RepoMetadata(t *testing.T) { "pageInfo": { "hasNextPage": false } } } } } `)) - http.Register( - httpmock.REST("GET", "repos/OWNER/REPO/milestones"), - httpmock.StringResponse(` - [ - { "title": "GA", "id": 1 }, - { "title": "Big One.oh", "id": 2 } - ] - `)) http.Register( httpmock.GraphQL(`query RepositoryProjectList\b`), httpmock.StringResponse(`