diff --git a/api/export_pr.go b/api/export_pr.go index 9a0e10702..96411aead 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -49,11 +49,34 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { data[f] = nil } case "statusCheckRollup": - if n := pr.Commits.Nodes; len(n) > 0 { + if n := pr.StatusCheckRollup.Nodes; len(n) > 0 { data[f] = n[0].Commit.StatusCheckRollup.Contexts.Nodes } else { data[f] = nil } + case "commits": + commits := make([]interface{}, 0, len(pr.Commits.Nodes)) + for _, c := range pr.Commits.Nodes { + commit := c.Commit + authors := make([]interface{}, 0, len(commit.Authors.Nodes)) + for _, author := range commit.Authors.Nodes { + authors = append(authors, map[string]interface{}{ + "name": author.Name, + "email": author.Email, + "id": author.User.ID, + "login": author.User.Login, + }) + } + commits = append(commits, map[string]interface{}{ + "oid": commit.OID, + "messageHeadline": commit.MessageHeadline, + "messageBody": commit.MessageBody, + "committedDate": commit.CommittedDate, + "authoredDate": commit.AuthoredDate, + "authors": authors, + }) + } + data[f] = commits case "comments": data[f] = pr.Comments.Nodes case "assignees": diff --git a/api/export_pr_test.go b/api/export_pr_test.go index 4e02f9f09..9243e6a8d 100644 --- a/api/export_pr_test.go +++ b/api/export_pr_test.go @@ -129,7 +129,7 @@ func TestPullRequest_ExportData(t *testing.T) { name: "status checks", fields: []string{"statusCheckRollup"}, inputJSON: heredoc.Doc(` - { "commits": { "nodes": [ + { "statusCheckRollup": { "nodes": [ { "commit": { "statusCheckRollup": { "contexts": { "nodes": [ { "__typename": "CheckRun", diff --git a/api/pull_request_test.go b/api/pull_request_test.go index 9fb1d9e72..2e4fa73b1 100644 --- a/api/pull_request_test.go +++ b/api/pull_request_test.go @@ -10,7 +10,7 @@ import ( func TestPullRequest_ChecksStatus(t *testing.T) { pr := PullRequest{} payload := ` - { "commits": { "nodes": [{ "commit": { + { "statusCheckRollup": { "nodes": [{ "commit": { "statusCheckRollup": { "contexts": { "nodes": [ diff --git a/api/queries_comments.go b/api/queries_comments.go index f02322c22..999c39033 100644 --- a/api/queries_comments.go +++ b/api/queries_comments.go @@ -4,7 +4,6 @@ import ( "context" "time" - "github.com/cli/cli/internal/ghrepo" "github.com/shurcooL/githubv4" "github.com/shurcooL/graphql" ) @@ -12,7 +11,10 @@ import ( type Comments struct { Nodes []Comment TotalCount int - PageInfo PageInfo + PageInfo struct { + HasNextPage bool + EndCursor string + } } type Comment struct { @@ -26,83 +28,6 @@ type Comment struct { ReactionGroups ReactionGroups `json:"reactionGroups"` } -type PageInfo struct { - HasNextPage bool - EndCursor string -} - -func CommentsForIssue(client *Client, repo ghrepo.Interface, issue *Issue) (*Comments, error) { - type response struct { - Repository struct { - Issue struct { - Comments Comments `graphql:"comments(first: 100, after: $endCursor)"` - } `graphql:"issue(number: $number)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - - variables := map[string]interface{}{ - "owner": githubv4.String(repo.RepoOwner()), - "repo": githubv4.String(repo.RepoName()), - "number": githubv4.Int(issue.Number), - "endCursor": (*githubv4.String)(nil), - } - - gql := graphQLClient(client.http, repo.RepoHost()) - - var comments []Comment - for { - var query response - err := gql.QueryNamed(context.Background(), "CommentsForIssue", &query, variables) - if err != nil { - return nil, err - } - - comments = append(comments, query.Repository.Issue.Comments.Nodes...) - if !query.Repository.Issue.Comments.PageInfo.HasNextPage { - break - } - variables["endCursor"] = githubv4.String(query.Repository.Issue.Comments.PageInfo.EndCursor) - } - - return &Comments{Nodes: comments, TotalCount: len(comments)}, nil -} - -func CommentsForPullRequest(client *Client, repo ghrepo.Interface, pr *PullRequest) (*Comments, error) { - type response struct { - Repository struct { - PullRequest struct { - Comments Comments `graphql:"comments(first: 100, after: $endCursor)"` - } `graphql:"pullRequest(number: $number)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - - variables := map[string]interface{}{ - "owner": githubv4.String(repo.RepoOwner()), - "repo": githubv4.String(repo.RepoName()), - "number": githubv4.Int(pr.Number), - "endCursor": (*githubv4.String)(nil), - } - - gql := graphQLClient(client.http, repo.RepoHost()) - - var comments []Comment - for { - var query response - err := gql.QueryNamed(context.Background(), "CommentsForPullRequest", &query, variables) - if err != nil { - return nil, err - } - - comments = append(comments, query.Repository.PullRequest.Comments.Nodes...) - if !query.Repository.PullRequest.Comments.PageInfo.HasNextPage { - break - } - variables["endCursor"] = githubv4.String(query.Repository.PullRequest.Comments.PageInfo.EndCursor) - } - - return &Comments{Nodes: comments, TotalCount: len(comments)}, nil -} - type CommentCreateInput struct { Body string SubjectId string diff --git a/api/queries_pr.go b/api/queries_pr.go index dfeb8608d..eefbcdd49 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -75,6 +75,33 @@ type PullRequest struct { TotalCount int Nodes []PullRequestCommit } + StatusCheckRollup struct { + Nodes []struct { + Commit struct { + StatusCheckRollup struct { + Contexts struct { + Nodes []struct { + TypeName string `json:"__typename"` + Name string `json:"name"` + Context string `json:"context,omitempty"` + State string `json:"state,omitempty"` + Status string `json:"status"` + Conclusion string `json:"conclusion"` + StartedAt time.Time `json:"startedAt"` + CompletedAt time.Time `json:"completedAt"` + DetailsURL string `json:"detailsUrl"` + TargetURL string `json:"targetUrl,omitempty"` + } + PageInfo struct { + HasNextPage bool + EndCursor string + } + } + } + } + } + } + Assignees Assignees Labels Labels ProjectCards ProjectCards @@ -89,6 +116,7 @@ type PRRepository struct { Name string } +// Commit loads just the commit SHA and nothing else type Commit struct { OID string `json:"oid"` } @@ -97,25 +125,20 @@ type PullRequestCommit struct { Commit PullRequestCommitCommit } -// PullRequestCommitCommit is like "Commit" but with StatusCheckRollup +// PullRequestCommitCommit contains full information about a commit type PullRequestCommitCommit struct { - Oid string - StatusCheckRollup struct { - Contexts struct { - Nodes []struct { - TypeName string `json:"__typename"` - Name string `json:"name"` - Context string `json:"context,omitempty"` - State string `json:"state,omitempty"` - Status string `json:"status"` - Conclusion string `json:"conclusion"` - StartedAt time.Time `json:"startedAt"` - CompletedAt time.Time `json:"completedAt"` - DetailsURL string `json:"detailsUrl"` - TargetURL string `json:"targetUrl,omitempty"` - } + OID string `json:"oid"` + Authors struct { + Nodes []struct { + Name string + Email string + User GitHubUser } } + MessageHeadline string + MessageBody string + CommittedDate time.Time + AuthoredDate time.Time } type PullRequestFile struct { @@ -132,7 +155,6 @@ type ReviewRequests struct { Name string `json:"name"` } } - TotalCount int } func (r ReviewRequests) Logins() []string { @@ -189,10 +211,10 @@ type PullRequestChecksStatus struct { } func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) { - if len(pr.Commits.Nodes) == 0 { + if len(pr.StatusCheckRollup.Nodes) == 0 { return } - commit := pr.Commits.Nodes[0].Commit + commit := pr.StatusCheckRollup.Nodes[0].Commit for _, c := range commit.StatusCheckRollup.Contexts.Nodes { state := c.State // StatusContext if state == "" { diff --git a/api/queries_pr_review.go b/api/queries_pr_review.go index 030472d75..53eac3e78 100644 --- a/api/queries_pr_review.go +++ b/api/queries_pr_review.go @@ -22,8 +22,11 @@ type PullRequestReviewInput struct { } type PullRequestReviews struct { - Nodes []PullRequestReview - PageInfo PageInfo + Nodes []PullRequestReview + PageInfo struct { + HasNextPage bool + EndCursor string + } TotalCount int } @@ -66,42 +69,6 @@ func AddReview(client *Client, repo ghrepo.Interface, pr *PullRequest, input *Pu return gql.MutateNamed(context.Background(), "PullRequestReviewAdd", &mutation, variables) } -func ReviewsForPullRequest(client *Client, repo ghrepo.Interface, pr *PullRequest) (*PullRequestReviews, error) { - type response struct { - Repository struct { - PullRequest struct { - Reviews PullRequestReviews `graphql:"reviews(first: 100, after: $endCursor)"` - } `graphql:"pullRequest(number: $number)"` - } `graphql:"repository(owner: $owner, name: $repo)"` - } - - variables := map[string]interface{}{ - "owner": githubv4.String(repo.RepoOwner()), - "repo": githubv4.String(repo.RepoName()), - "number": githubv4.Int(pr.Number), - "endCursor": (*githubv4.String)(nil), - } - - gql := graphQLClient(client.http, repo.RepoHost()) - - var reviews []PullRequestReview - for { - var query response - err := gql.QueryNamed(context.Background(), "ReviewsForPullRequest", &query, variables) - if err != nil { - return nil, err - } - - reviews = append(reviews, query.Repository.PullRequest.Reviews.Nodes...) - if !query.Repository.PullRequest.Reviews.PageInfo.HasNextPage { - break - } - variables["endCursor"] = githubv4.String(query.Repository.PullRequest.Reviews.PageInfo.EndCursor) - } - - return &PullRequestReviews{Nodes: reviews, TotalCount: len(reviews)}, nil -} - func (prr PullRequestReview) AuthorLogin() string { return prr.Author.Login } diff --git a/api/query_builder.go b/api/query_builder.go index 25862e179..a97d5eee9 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -1,6 +1,7 @@ package api import ( + "fmt" "strings" ) @@ -18,7 +19,7 @@ func shortenQuery(q string) string { } var issueComments = shortenQuery(` - comments(last: 100) { + comments(first: 100) { nodes { author{login}, authorAssociation, @@ -29,25 +30,25 @@ var issueComments = shortenQuery(` minimizedReason, reactionGroups{content,users{totalCount}} }, + pageInfo{hasNextPage,endCursor}, totalCount } `) var prReviewRequests = shortenQuery(` - reviewRequests(last: 100) { + reviewRequests(first: 100) { nodes { requestedReviewer { __typename, ...on User{login}, ...on Team{name} } - }, - totalCount + } } `) var prReviews = shortenQuery(` - reviews(last: 100) { + reviews(first: 100) { nodes { author{login}, authorAssociation, @@ -56,6 +57,7 @@ var prReviews = shortenQuery(` state, reactionGroups{content,users{totalCount}} } + pageInfo{hasNextPage,endCursor} } `) @@ -69,14 +71,38 @@ var prFiles = shortenQuery(` } `) -var prStatusCheckRollup = shortenQuery(` - commits(last: 1) { - totalCount, +var prCommits = shortenQuery(` + commits(first: 100) { nodes { commit { + authors(first:100) { + nodes { + name, + email, + user{id,login} + } + }, + messageHeadline, + messageBody, oid, + committedDate, + authoredDate + } + } + } +`) + +func StatusCheckRollupGraphQL(after string) string { + var afterClause string + if after != "" { + afterClause = ",after:" + after + } + return fmt.Sprintf(shortenQuery(` + statusCheckRollup: commits(last: 1) { + nodes { + commit { statusCheckRollup { - contexts(last: 100) { + contexts(first:100%s) { nodes { __typename ...on StatusContext { @@ -92,13 +118,14 @@ var prStatusCheckRollup = shortenQuery(` completedAt, detailsUrl } - } + }, + pageInfo{hasNextPage,endCursor} } } } } - } -`) + }`), afterClause) +} var IssueFields = []string{ "assignees", @@ -124,6 +151,7 @@ var PullRequestFields = append(IssueFields, "additions", "baseRefName", "changedFiles", + "commits", "deletions", "files", "headRefName", @@ -178,8 +206,12 @@ func PullRequestGraphQL(fields []string) string { q = append(q, prReviews) case "files": q = append(q, prFiles) + case "commits": + q = append(q, prCommits) + case "commitsCount": // pseudo-field + q = append(q, `commits{totalCount}`) case "statusCheckRollup": - q = append(q, prStatusCheckRollup) + q = append(q, StatusCheckRollupGraphQL("")) default: q = append(q, field) } diff --git a/pkg/cmd/issue/view/fixtures/issueView_previewFullComments.json b/pkg/cmd/issue/view/fixtures/issueView_previewFullComments.json index 9ab620ecd..f45459e24 100644 --- a/pkg/cmd/issue/view/fixtures/issueView_previewFullComments.json +++ b/pkg/cmd/issue/view/fixtures/issueView_previewFullComments.json @@ -1,7 +1,6 @@ { "data": { - "repository": { - "issue": { + "node": { "comments": { "nodes": [ { @@ -315,6 +314,5 @@ "totalCount": 6 } } - } } } diff --git a/pkg/cmd/issue/view/http.go b/pkg/cmd/issue/view/http.go new file mode 100644 index 000000000..c4f87d677 --- /dev/null +++ b/pkg/cmd/issue/view/http.go @@ -0,0 +1,50 @@ +package view + +import ( + "context" + "net/http" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/internal/ghrepo" + "github.com/shurcooL/githubv4" + "github.com/shurcooL/graphql" +) + +func preloadIssueComments(client *http.Client, repo ghrepo.Interface, issue *api.Issue) error { + type response struct { + Node struct { + Issue struct { + Comments api.Comments `graphql:"comments(first: 100, after: $endCursor)"` + } `graphql:"...on Issue"` + } `graphql:"node(id: $id)"` + } + + variables := map[string]interface{}{ + "id": githubv4.ID(issue.ID), + "endCursor": (*githubv4.String)(nil), + } + if issue.Comments.PageInfo.HasNextPage { + variables["endCursor"] = githubv4.String(issue.Comments.PageInfo.EndCursor) + } else { + issue.Comments.Nodes = issue.Comments.Nodes[0:0] + } + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), client) + for { + var query response + err := gql.QueryNamed(context.Background(), "CommentsForIssue", &query, variables) + if err != nil { + return err + } + + issue.Comments.Nodes = append(issue.Comments.Nodes, query.Node.Issue.Comments.Nodes...) + if !query.Node.Issue.Comments.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(query.Node.Issue.Comments.PageInfo.EndCursor) + } + + issue.Comments.PageInfo.HasNextPage = false + return nil +} diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 6888cc791..c24733c13 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -16,6 +16,7 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/markdown" + "github.com/cli/cli/pkg/set" "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -82,9 +83,17 @@ func viewRun(opts *ViewOptions) error { if err != nil { return err } - apiClient := api.NewClientFromHTTP(httpClient) - issue, repo, err := issueShared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg) + loadComments := opts.Comments + if !loadComments && opts.Exporter != nil { + fields := set.NewStringSet() + fields.AddValues(opts.Exporter.Fields()) + loadComments = fields.Contains("comments") + } + + opts.IO.StartProgressIndicator() + issue, err := findIssue(httpClient, opts.BaseRepo, opts.SelectorArg, loadComments) + opts.IO.StopProgressIndicator() if err != nil { return err } @@ -97,21 +106,9 @@ func viewRun(opts *ViewOptions) error { return opts.Browser.Browse(openURL) } - if opts.Comments { - opts.IO.StartProgressIndicator() - comments, err := api.CommentsForIssue(apiClient, repo, issue) - opts.IO.StopProgressIndicator() - if err != nil { - return err - } - issue.Comments = *comments - } - opts.IO.DetectTerminalTheme() - - err = opts.IO.StartPager() - if err != nil { - return err + if err := opts.IO.StartPager(); err != nil { + fmt.Fprintf(opts.IO.ErrOut, "error starting pager: %v", err) } defer opts.IO.StopPager() @@ -131,6 +128,19 @@ func viewRun(opts *ViewOptions) error { return printRawIssuePreview(opts.IO.Out, issue) } +func findIssue(client *http.Client, baseRepoFn func() (ghrepo.Interface, error), selector string, loadComments bool) (*api.Issue, error) { + apiClient := api.NewClientFromHTTP(client) + issue, repo, err := issueShared.IssueFromArg(apiClient, baseRepoFn, selector) + if err != nil { + return issue, err + } + + if loadComments { + err = preloadIssueComments(client, repo, issue) + } + return issue, err +} + func printRawIssuePreview(out io.Writer, issue *api.Issue) error { assignees := issueAssigneeList(*issue) labels := shared.IssueLabelList(*issue) diff --git a/pkg/cmd/pr/checks/checks.go b/pkg/cmd/pr/checks/checks.go index e97d1e46f..b21631820 100644 --- a/pkg/cmd/pr/checks/checks.go +++ b/pkg/cmd/pr/checks/checks.go @@ -92,11 +92,11 @@ func checksRun(opts *ChecksOptions) error { return opts.Browser.Browse(openURL) } - if len(pr.Commits.Nodes) == 0 { + if len(pr.StatusCheckRollup.Nodes) == 0 { return fmt.Errorf("no commit found on the pull request") } - rollup := pr.Commits.Nodes[0].Commit.StatusCheckRollup.Contexts.Nodes + rollup := pr.StatusCheckRollup.Nodes[0].Commit.StatusCheckRollup.Contexts.Nodes if len(rollup) == 0 { return fmt.Errorf("no checks reported on the '%s' branch", pr.BaseRefName) } @@ -118,7 +118,7 @@ func checksRun(opts *ChecksOptions) error { outputs := []output{} - for _, c := range pr.Commits.Nodes[0].Commit.StatusCheckRollup.Contexts.Nodes { + for _, c := range pr.StatusCheckRollup.Nodes[0].Commit.StatusCheckRollup.Contexts.Nodes { mark := "✓" bucket := "pass" state := c.State diff --git a/pkg/cmd/pr/checks/checks_test.go b/pkg/cmd/pr/checks/checks_test.go index 6cedd6d32..0a189bd4d 100644 --- a/pkg/cmd/pr/checks/checks_test.go +++ b/pkg/cmd/pr/checks/checks_test.go @@ -83,7 +83,7 @@ func Test_checksRun(t *testing.T) { }, { name: "no checks", - prJSON: `{ "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]}, "baseRefName": "master" }`, + prJSON: `{ "number": 123, "statusCheckRollup": { "nodes": [{"commit": {"oid": "abc"}}]}, "baseRefName": "master" }`, wantOut: "", wantErr: "no checks reported on the 'master' branch", }, @@ -114,7 +114,7 @@ func Test_checksRun(t *testing.T) { { name: "no checks", nontty: true, - prJSON: `{ "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]}, "baseRefName": "master" }`, + prJSON: `{ "number": 123, "statusCheckRollup": { "nodes": [{"commit": {"oid": "abc"}}]}, "baseRefName": "master" }`, wantOut: "", wantErr: "no checks reported on the 'master' branch", }, diff --git a/pkg/cmd/pr/checks/fixtures/allPassing.json b/pkg/cmd/pr/checks/fixtures/allPassing.json index 5116b5916..8d1f33510 100644 --- a/pkg/cmd/pr/checks/fixtures/allPassing.json +++ b/pkg/cmd/pr/checks/fixtures/allPassing.json @@ -1,6 +1,6 @@ { "number": 123, - "commits": { + "statusCheckRollup": { "nodes": [ { "commit": { diff --git a/pkg/cmd/pr/checks/fixtures/someFailing.json b/pkg/cmd/pr/checks/fixtures/someFailing.json index 887e22ab3..f407cd4b5 100644 --- a/pkg/cmd/pr/checks/fixtures/someFailing.json +++ b/pkg/cmd/pr/checks/fixtures/someFailing.json @@ -1,6 +1,6 @@ { "number": 123, - "commits": { + "statusCheckRollup": { "nodes": [ { "commit": { diff --git a/pkg/cmd/pr/checks/fixtures/somePending.json b/pkg/cmd/pr/checks/fixtures/somePending.json index 5214a7930..2d558f39e 100644 --- a/pkg/cmd/pr/checks/fixtures/somePending.json +++ b/pkg/cmd/pr/checks/fixtures/somePending.json @@ -1,6 +1,6 @@ { "number": 123, - "commits": { + "statusCheckRollup": { "nodes": [ { "commit": { diff --git a/pkg/cmd/pr/checks/fixtures/withStatuses.json b/pkg/cmd/pr/checks/fixtures/withStatuses.json index 2b4a808c7..ddc7374ba 100644 --- a/pkg/cmd/pr/checks/fixtures/withStatuses.json +++ b/pkg/cmd/pr/checks/fixtures/withStatuses.json @@ -1,6 +1,6 @@ { "number": 123, - "commits": { + "statusCheckRollup": { "nodes": [ { "commit": { diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 298d59b7f..f25732a7b 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -184,7 +184,7 @@ func mergeRun(opts *MergeOptions) error { if opts.SelectorArg == "" && len(pr.Commits.Nodes) > 0 { if localBranchLastCommit, err := git.LastCommit(); err == nil { - if localBranchLastCommit.Sha != pr.Commits.Nodes[0].Commit.Oid { + if localBranchLastCommit.Sha != pr.Commits.Nodes[len(pr.Commits.Nodes)-1].Commit.OID { fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) has diverged from local branch\n", cs.Yellow("!"), pr.Number, pr.Title) } diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index 4abb88aca..abd0d9f55 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -205,7 +205,7 @@ func baseRepo(owner, repo, branch string) ghrepo.Interface { func stubCommit(pr *api.PullRequest, oid string) { pr.Commits.Nodes = append(pr.Commits.Nodes, api.PullRequestCommit{ - Commit: api.PullRequestCommitCommit{Oid: oid}, + Commit: api.PullRequestCommitCommit{OID: oid}, }) } diff --git a/pkg/cmd/pr/shared/finder.go b/pkg/cmd/pr/shared/finder.go index 52b2a436a..1ad253be2 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -1,6 +1,7 @@ package shared import ( + "context" "errors" "fmt" "net/http" @@ -11,11 +12,15 @@ import ( "strings" "github.com/cli/cli/api" - "github.com/cli/cli/context" + remotes "github.com/cli/cli/context" "github.com/cli/cli/git" + "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/set" + "github.com/shurcooL/githubv4" + "github.com/shurcooL/graphql" + "golang.org/x/sync/errgroup" ) type PRFinder interface { @@ -30,7 +35,7 @@ type progressIndicator interface { type finder struct { baseRepoFn func() (ghrepo.Interface, error) branchFn func() (string, error) - remotesFn func() (context.Remotes, error) + remotesFn func() (remotes.Remotes, error) httpClient func() (*http.Client, error) branchConfig func(string) git.BranchConfig progress progressIndicator @@ -120,20 +125,43 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err defer f.progress.StopProgressIndicator() } + fields := set.NewStringSet() + fields.AddValues(opts.Fields) + numberFieldOnly := fields.Len() == 1 && fields.Contains("number") + fields.Add("id") // for additional preload queries below + + var pr *api.PullRequest if f.prNumber > 0 { - if len(opts.Fields) == 1 && opts.Fields[0] == "number" { + if numberFieldOnly { // avoid hitting the API if we already have all the information return &api.PullRequest{Number: f.prNumber}, f.repo, nil } - pr, err := findByNumber(httpClient, f.repo, f.prNumber, opts.Fields) + pr, err = findByNumber(httpClient, f.repo, f.prNumber, fields.ToSlice()) + } else { + pr, err = findForBranch(httpClient, f.repo, opts.BaseBranch, f.branchName, opts.States, fields.ToSlice()) + } + if err != nil { return pr, f.repo, err } - pr, err := findForBranch(httpClient, f.repo, opts.BaseBranch, f.branchName, opts.States, opts.Fields) + g, _ := errgroup.WithContext(context.Background()) + if fields.Contains("reviews") { + g.Go(func() error { + return preloadPrReviews(httpClient, f.repo, pr) + }) + } + if fields.Contains("comments") { + g.Go(func() error { + return preloadPrComments(httpClient, f.repo, pr) + }) + } + if fields.Contains("statusCheckRollup") { + g.Go(func() error { + return preloadPrChecks(httpClient, f.repo, pr) + }) + } - // TODO: preload view: api.ReviewsForPullRequest, api.CommentsForPullRequest - // TODO: preload checks: get all checks - return pr, f.repo, err + return pr, f.repo, g.Wait() } var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`) @@ -291,6 +319,139 @@ func findForBranch(httpClient *http.Client, repo ghrepo.Interface, baseBranch, h return nil, &NotFoundError{fmt.Errorf("no pull requests found for branch %q", headBranch)} } +func preloadPrReviews(httpClient *http.Client, repo ghrepo.Interface, pr *api.PullRequest) error { + if !pr.Reviews.PageInfo.HasNextPage { + return nil + } + + type response struct { + Node struct { + PullRequest struct { + Reviews api.PullRequestReviews `graphql:"reviews(first: 100, after: $endCursor)"` + } `graphql:"...on PullRequest"` + } `graphql:"node(id: $id)"` + } + + variables := map[string]interface{}{ + "id": githubv4.ID(pr.ID), + "endCursor": githubv4.String(pr.Reviews.PageInfo.EndCursor), + } + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), httpClient) + + for { + var query response + err := gql.QueryNamed(context.Background(), "ReviewsForPullRequest", &query, variables) + if err != nil { + return err + } + + pr.Reviews.Nodes = append(pr.Reviews.Nodes, query.Node.PullRequest.Reviews.Nodes...) + pr.Reviews.TotalCount = len(pr.Reviews.Nodes) + + if !query.Node.PullRequest.Reviews.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(query.Node.PullRequest.Reviews.PageInfo.EndCursor) + } + + pr.Reviews.PageInfo.HasNextPage = false + return nil +} + +func preloadPrComments(client *http.Client, repo ghrepo.Interface, pr *api.PullRequest) error { + if !pr.Comments.PageInfo.HasNextPage { + return nil + } + + type response struct { + Node struct { + PullRequest struct { + Comments api.Comments `graphql:"comments(first: 100, after: $endCursor)"` + } `graphql:"...on PullRequest"` + } `graphql:"node(id: $id)"` + } + + variables := map[string]interface{}{ + "id": githubv4.ID(pr.ID), + "endCursor": githubv4.String(pr.Comments.PageInfo.EndCursor), + } + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), client) + + for { + var query response + err := gql.QueryNamed(context.Background(), "CommentsForPullRequest", &query, variables) + if err != nil { + return err + } + + pr.Comments.Nodes = append(pr.Comments.Nodes, query.Node.PullRequest.Comments.Nodes...) + pr.Comments.TotalCount = len(pr.Comments.Nodes) + + if !query.Node.PullRequest.Comments.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(query.Node.PullRequest.Comments.PageInfo.EndCursor) + } + + pr.Comments.PageInfo.HasNextPage = false + return nil +} + +func preloadPrChecks(client *http.Client, repo ghrepo.Interface, pr *api.PullRequest) error { + if len(pr.StatusCheckRollup.Nodes) == 0 { + return nil + } + statusCheckRollup := &pr.StatusCheckRollup.Nodes[0].Commit.StatusCheckRollup.Contexts + if !statusCheckRollup.PageInfo.HasNextPage { + return nil + } + + endCursor := statusCheckRollup.PageInfo.EndCursor + + type response struct { + Node *api.PullRequest + } + + query := fmt.Sprintf(` + query PullRequestStatusChecks($id: ID!, $endCursor: String!) { + node(id: $id) { + ...on PullRequest { + %s + } + } + }`, api.StatusCheckRollupGraphQL("$endCursor")) + + variables := map[string]interface{}{ + "id": pr.ID, + } + + apiClient := api.NewClientFromHTTP(client) + for { + variables["endCursor"] = endCursor + var resp response + err := apiClient.GraphQL(repo.RepoHost(), query, variables, &resp) + if err != nil { + return err + } + + result := resp.Node.StatusCheckRollup.Nodes[0].Commit.StatusCheckRollup.Contexts + statusCheckRollup.Nodes = append( + statusCheckRollup.Nodes, + result.Nodes..., + ) + + if !result.PageInfo.HasNextPage { + break + } + endCursor = result.PageInfo.EndCursor + } + + statusCheckRollup.PageInfo.HasNextPage = false + return nil +} + type NotFoundError struct { error } diff --git a/pkg/cmd/pr/status/fixtures/prStatusChecks.json b/pkg/cmd/pr/status/fixtures/prStatusChecks.json index 55035ae36..dd1605b1d 100644 --- a/pkg/cmd/pr/status/fixtures/prStatusChecks.json +++ b/pkg/cmd/pr/status/fixtures/prStatusChecks.json @@ -17,7 +17,7 @@ "url": "https://github.com/cli/cli/pull/8", "headRefName": "strawberries", "reviewDecision": "CHANGES_REQUESTED", - "commits": { + "statusCheckRollup": { "nodes": [ { "commit": { @@ -44,7 +44,7 @@ "url": "https://github.com/cli/cli/pull/7", "headRefName": "banananana", "reviewDecision": "APPROVED", - "commits": { + "statusCheckRollup": { "nodes": [ { "commit": { @@ -72,7 +72,7 @@ "url": "https://github.com/cli/cli/pull/6", "headRefName": "avo", "reviewDecision": "REVIEW_REQUIRED", - "commits": { + "statusCheckRollup": { "nodes": [ { "commit": { diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index fd924387c..951ce7953 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -78,11 +78,10 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman var defaultFields = []string{ "url", "number", "title", "state", "body", "author", - "isDraft", "maintainerCanModify", "mergeable", "additions", "deletions", + "isDraft", "maintainerCanModify", "mergeable", "additions", "deletions", "commitsCount", "baseRefName", "headRefName", "headRepositoryOwner", "headRepository", "isCrossRepository", "reviewRequests", "reviews", "assignees", "labels", "projectCards", "milestone", - "comments", // TODO: fetch only 1 last comment unless `opts.Comments` was set - "reactionGroups", + "comments", "reactionGroups", } func viewRun(opts *ViewOptions) error {