Import PR list API implementation to pr/list package

Also splits List vs. Search queries into separate methods for better
maintanability.

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 {

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": {

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"
             }
           },
           {

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
+}

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
 	}

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))
 		}))