diff --git a/api/queries_pr.go b/api/queries_pr.go index 64b9935f3..74b7dd170 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -12,7 +12,6 @@ import ( "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/internal/ghrepo" - "github.com/cli/cli/pkg/githubsearch" "github.com/shurcooL/githubv4" "golang.org/x/sync/errgroup" ) @@ -910,173 +909,6 @@ func isBlank(v interface{}) bool { } } -func PullRequestList(client *Client, repo ghrepo.Interface, vars map[string]interface{}, limit int) (*PullRequestAndTotalCount, error) { - type prBlock struct { - Edges []struct { - Node PullRequest - } - PageInfo struct { - HasNextPage bool - EndCursor string - } - TotalCount int - IssueCount int - } - type response struct { - Repository struct { - PullRequests prBlock - } - Search prBlock - } - - fragment := ` - fragment pr on PullRequest { - number - title - state - url - headRefName - headRepositoryOwner { - login - } - isCrossRepository - isDraft - } - ` - - // If assignee wasn't specified, use `Repository.pullRequest` for ability to - // query by multiple labels - query := fragment + ` - query PullRequestList( - $owner: String!, - $repo: String!, - $limit: Int!, - $endCursor: String, - $baseBranch: String, - $labels: [String!], - $state: [PullRequestState!] = OPEN - ) { - repository(owner: $owner, name: $repo) { - pullRequests( - states: $state, - baseRefName: $baseBranch, - labels: $labels, - first: $limit, - after: $endCursor, - orderBy: {field: CREATED_AT, direction: DESC} - ) { - totalCount - edges { - node { - ...pr - } - } - pageInfo { - hasNextPage - endCursor - } - } - } - }` - - var check = make(map[int]struct{}) - var prs []PullRequest - pageLimit := min(limit, 100) - variables := map[string]interface{}{} - res := PullRequestAndTotalCount{} - - // If assignee was specified, use the `search` API rather than - // `Repository.pullRequests`, but this mode doesn't support multiple labels - if assignee, ok := vars["assignee"].(string); ok { - query = fragment + ` - query PullRequestList( - $q: String!, - $limit: Int!, - $endCursor: String, - ) { - search(query: $q, type: ISSUE, first: $limit, after: $endCursor) { - issueCount - edges { - node { - ...pr - } - } - pageInfo { - hasNextPage - endCursor - } - } - }` - q := githubsearch.NewQuery() - q.SetType(githubsearch.PullRequest) - q.InRepository(ghrepo.FullName(repo)) - q.AssignedTo(assignee) - q.SortBy(githubsearch.CreatedAt, githubsearch.Desc) - if states, ok := vars["state"].([]string); ok && len(states) == 1 { - switch states[0] { - case "OPEN": - q.SetState(githubsearch.Open) - case "CLOSED": - q.SetState(githubsearch.Closed) - case "MERGED": - q.SetState(githubsearch.Merged) - } - } - if labels, ok := vars["labels"].([]string); ok && len(labels) > 0 { - if len(labels) > 1 { - return nil, fmt.Errorf("multiple labels with --assignee are not supported") - } - q.AddLabel(labels[0]) - } - if baseBranch, ok := vars["baseBranch"].(string); ok { - q.SetBaseBranch(baseBranch) - } - variables["q"] = q.String() - } else { - variables["owner"] = repo.RepoOwner() - variables["repo"] = repo.RepoName() - for name, val := range vars { - variables[name] = val - } - } -loop: - for { - variables["limit"] = pageLimit - var data response - err := client.GraphQL(repo.RepoHost(), query, variables, &data) - if err != nil { - return nil, err - } - prData := data.Repository.PullRequests - res.TotalCount = prData.TotalCount - if _, ok := variables["q"]; ok { - prData = data.Search - res.TotalCount = prData.IssueCount - } - - for _, edge := range prData.Edges { - if _, exists := check[edge.Node.Number]; exists { - continue - } - - prs = append(prs, edge.Node) - check[edge.Node.Number] = struct{}{} - if len(prs) == limit { - break loop - } - } - - if prData.PageInfo.HasNextPage { - variables["endCursor"] = prData.PageInfo.EndCursor - pageLimit = min(pageLimit, limit-len(prs)) - } else { - break - } - } - res.PullRequests = prs - return &res, nil -} - func PullRequestClose(client *Client, repo ghrepo.Interface, pr *PullRequest) error { var mutation struct { ClosePullRequest struct { diff --git a/pkg/cmd/pr/list/fixtures/prList.json b/pkg/cmd/pr/list/fixtures/prList.json index e050a27f6..13d124f52 100644 --- a/pkg/cmd/pr/list/fixtures/prList.json +++ b/pkg/cmd/pr/list/fixtures/prList.json @@ -3,40 +3,34 @@ "repository": { "pullRequests": { "totalCount": 3, - "edges": [ + "nodes": [ { - "node": { - "number": 32, - "title": "New feature", - "url": "https://github.com/monalisa/hello/pull/32", - "headRefName": "feature", - "state": "OPEN", - "isDraft": true + "number": 32, + "title": "New feature", + "url": "https://github.com/monalisa/hello/pull/32", + "headRefName": "feature", + "state": "OPEN", + "isDraft": true + }, + { + "number": 29, + "title": "Fixed bad bug", + "url": "https://github.com/monalisa/hello/pull/29", + "headRefName": "bug-fix", + "state": "OPEN", + "isDraft": false, + "isCrossRepository": true, + "headRepositoryOwner": { + "login": "hubot" } }, { - "node": { - "number": 29, - "title": "Fixed bad bug", - "url": "https://github.com/monalisa/hello/pull/29", - "headRefName": "bug-fix", - "state": "OPEN", - "isDraft": false, - "isCrossRepository": true, - "headRepositoryOwner": { - "login": "hubot" - } - } - }, - { - "node": { - "number": 28, - "state": "MERGED", - "isDraft": false, - "title": "Improve documentation", - "url": "https://github.com/monalisa/hello/pull/28", - "headRefName": "docs" - } + "number": 28, + "state": "MERGED", + "isDraft": false, + "title": "Improve documentation", + "url": "https://github.com/monalisa/hello/pull/28", + "headRefName": "docs" } ], "pageInfo": { diff --git a/pkg/cmd/pr/list/fixtures/prListWithDuplicates.json b/pkg/cmd/pr/list/fixtures/prListWithDuplicates.json index a84800f72..295c3c9bd 100644 --- a/pkg/cmd/pr/list/fixtures/prListWithDuplicates.json +++ b/pkg/cmd/pr/list/fixtures/prListWithDuplicates.json @@ -2,33 +2,27 @@ "data": { "repository": { "pullRequests": { - "edges": [ + "nodes": [ { - "node": { - "number": 32, - "title": "New feature", - "url": "https://github.com/monalisa/hello/pull/32", - "headRefName": "feature" - } + "number": 32, + "title": "New feature", + "url": "https://github.com/monalisa/hello/pull/32", + "headRefName": "feature" }, { - "node": { - "number": 32, - "title": "New feature", - "url": "https://github.com/monalisa/hello/pull/32", - "headRefName": "feature" - } + "number": 32, + "title": "New feature", + "url": "https://github.com/monalisa/hello/pull/32", + "headRefName": "feature" }, { - "node": { - "number": 29, - "title": "Fixed bad bug", - "url": "https://github.com/monalisa/hello/pull/29", - "headRefName": "bug-fix", - "isCrossRepository": true, - "headRepositoryOwner": { - "login": "hubot" - } + "number": 29, + "title": "Fixed bad bug", + "url": "https://github.com/monalisa/hello/pull/29", + "headRefName": "bug-fix", + "isCrossRepository": true, + "headRepositoryOwner": { + "login": "hubot" } }, { diff --git a/pkg/cmd/pr/list/http.go b/pkg/cmd/pr/list/http.go new file mode 100644 index 000000000..3a164800f --- /dev/null +++ b/pkg/cmd/pr/list/http.go @@ -0,0 +1,243 @@ +package list + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghrepo" + prShared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/githubsearch" +) + +const fragment = `fragment pr on PullRequest { + number + title + state + url + headRefName + headRepositoryOwner { + login + } + isCrossRepository + isDraft +}` + +func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) { + if filters.Assignee != "" { + return searchPullRequests(httpClient, repo, filters, limit) + } + + type response struct { + Repository struct { + PullRequests struct { + Nodes []api.PullRequest + PageInfo struct { + HasNextPage bool + EndCursor string + } + TotalCount int + } + } + } + + query := fragment + ` + query PullRequestList( + $owner: String!, + $repo: String!, + $limit: Int!, + $endCursor: String, + $baseBranch: String, + $labels: [String!], + $state: [PullRequestState!] = OPEN + ) { + repository(owner: $owner, name: $repo) { + pullRequests( + states: $state, + baseRefName: $baseBranch, + labels: $labels, + first: $limit, + after: $endCursor, + orderBy: {field: CREATED_AT, direction: DESC} + ) { + totalCount + nodes { + ...pr + } + pageInfo { + hasNextPage + endCursor + } + } + } + }` + + pageLimit := min(limit, 100) + variables := map[string]interface{}{ + "owner": repo.RepoOwner(), + "repo": repo.RepoName(), + "labels": filters.Labels, + } + + switch filters.State { + case "open": + variables["state"] = []string{"OPEN"} + case "closed": + variables["state"] = []string{"CLOSED", "MERGED"} + case "merged": + variables["state"] = []string{"MERGED"} + case "all": + variables["state"] = []string{"OPEN", "CLOSED", "MERGED"} + default: + return nil, fmt.Errorf("invalid state: %s", filters.State) + } + + if filters.BaseBranch != "" { + variables["baseBranch"] = filters.BaseBranch + } + + res := api.PullRequestAndTotalCount{} + var check = make(map[int]struct{}) + client := api.NewClientFromHTTP(httpClient) + +loop: + for { + variables["limit"] = pageLimit + var data response + err := client.GraphQL(repo.RepoHost(), query, variables, &data) + if err != nil { + return nil, err + } + prData := data.Repository.PullRequests + res.TotalCount = prData.TotalCount + + for _, pr := range prData.Nodes { + if _, exists := check[pr.Number]; exists { + continue + } + check[pr.Number] = struct{}{} + + res.PullRequests = append(res.PullRequests, pr) + if len(res.PullRequests) == limit { + break loop + } + } + + if prData.PageInfo.HasNextPage { + variables["endCursor"] = prData.PageInfo.EndCursor + pageLimit = min(pageLimit, limit-len(res.PullRequests)) + } else { + break + } + } + + return &res, nil +} + +func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) { + if len(filters.Labels) > 1 { + return nil, fmt.Errorf("multiple labels with --assignee are not supported") + } + + type response struct { + Search struct { + Nodes []api.PullRequest + PageInfo struct { + HasNextPage bool + EndCursor string + } + IssueCount int + } + } + + query := fragment + ` + query PullRequestSearch( + $q: String!, + $limit: Int!, + $endCursor: String, + ) { + search(query: $q, type: ISSUE, first: $limit, after: $endCursor) { + issueCount + nodes { + ...pr + } + pageInfo { + hasNextPage + endCursor + } + } + }` + + q := githubsearch.NewQuery() + q.SetType(githubsearch.PullRequest) + q.InRepository(ghrepo.FullName(repo)) + q.SortBy(githubsearch.CreatedAt, githubsearch.Desc) + + switch filters.State { + case "open": + q.SetState(githubsearch.Open) + case "closed": + q.SetState(githubsearch.Closed) + case "merged": + q.SetState(githubsearch.Merged) + } + + if filters.Assignee != "" { + q.AssignedTo(filters.Assignee) + } + if len(filters.Labels) > 0 { + q.AddLabel(filters.Labels[0]) + } + if filters.BaseBranch != "" { + q.SetBaseBranch(filters.BaseBranch) + } + + pageLimit := min(limit, 100) + variables := map[string]interface{}{ + "q": q.String(), + } + + res := api.PullRequestAndTotalCount{} + var check = make(map[int]struct{}) + client := api.NewClientFromHTTP(httpClient) + +loop: + for { + variables["limit"] = pageLimit + var data response + err := client.GraphQL(repo.RepoHost(), query, variables, &data) + if err != nil { + return nil, err + } + prData := data.Search + res.TotalCount = prData.IssueCount + + for _, pr := range prData.Nodes { + if _, exists := check[pr.Number]; exists { + continue + } + check[pr.Number] = struct{}{} + + res.PullRequests = append(res.PullRequests, pr) + if len(res.PullRequests) == limit { + break loop + } + } + + if prData.PageInfo.HasNextPage { + variables["endCursor"] = prData.PageInfo.EndCursor + pageLimit = min(pageLimit, limit-len(res.PullRequests)) + } else { + break + } + } + + return &res, nil +} + +func min(a, b int) int { + if a < b { + return a + } + return b +} diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index 3ed4b1ea1..25f577c00 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "strconv" + "strings" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" @@ -75,22 +76,23 @@ func listRun(opts *ListOptions) error { if err != nil { return err } - apiClient := api.NewClientFromHTTP(httpClient) baseRepo, err := opts.BaseRepo() if err != nil { return err } + filters := shared.FilterOptions{ + Entity: "pr", + State: strings.ToLower(opts.State), + Assignee: opts.Assignee, + Labels: opts.Labels, + BaseBranch: opts.BaseBranch, + } + if opts.WebMode { prListURL := ghrepo.GenerateRepoURL(baseRepo, "pulls") - openURL, err := shared.ListURLWithQuery(prListURL, shared.FilterOptions{ - Entity: "pr", - State: opts.State, - Assignee: opts.Assignee, - Labels: opts.Labels, - BaseBranch: opts.BaseBranch, - }) + openURL, err := shared.ListURLWithQuery(prListURL, filters) if err != nil { return err } @@ -101,34 +103,7 @@ func listRun(opts *ListOptions) error { return utils.OpenInBrowser(openURL) } - var graphqlState []string - switch opts.State { - case "open": - graphqlState = []string{"OPEN"} - case "closed": - graphqlState = []string{"CLOSED", "MERGED"} - case "merged": - graphqlState = []string{"MERGED"} - case "all": - graphqlState = []string{"OPEN", "CLOSED", "MERGED"} - default: - return fmt.Errorf("invalid state: %s", opts.State) - } - - params := map[string]interface{}{ - "state": graphqlState, - } - if len(opts.Labels) > 0 { - params["labels"] = opts.Labels - } - if opts.BaseBranch != "" { - params["baseBranch"] = opts.BaseBranch - } - if opts.Assignee != "" { - params["assignee"] = opts.Assignee - } - - listResult, err := api.PullRequestList(apiClient, baseRepo, params, opts.LimitResults) + listResult, err := listPullRequests(httpClient, baseRepo, filters, opts.LimitResults) if err != nil { return err } diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go index f4b6c85e1..28be3aee2 100644 --- a/pkg/cmd/pr/list/list_test.go +++ b/pkg/cmd/pr/list/list_test.go @@ -163,7 +163,7 @@ func TestPRList_filteringAssignee(t *testing.T) { defer http.Verify(t) http.Register( - httpmock.GraphQL(`query PullRequestList\b`), + httpmock.GraphQL(`query PullRequestSearch\b`), httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { assert.Equal(t, `repo:OWNER/REPO is:pr is:merged assignee:hubot label:"needs tests" base:develop sort:created-desc`, params["q"].(string)) }))