diff --git a/api/query_builder.go b/api/query_builder.go index 3e54f4c60..4274e45e8 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -143,7 +143,31 @@ var autoMergeRequest = shortenQuery(` } `) -func StatusCheckRollupGraphQL(after string) string { +func StatusCheckRollupGraphQLWithCountByState() string { + return shortenQuery(` + statusCheckRollup: commits(last: 1) { + nodes { + commit { + statusCheckRollup { + contexts { + checkRunCount, + checkRunCountsByState { + state, + count + }, + statusContextCount, + statusContextCountsByState { + state, + count + } + } + } + } + } + }`) +} + +func StatusCheckRollupGraphQLWithoutCountByState(after string) string { var afterClause string if after != "" { afterClause = ",after:" + after @@ -268,8 +292,28 @@ var PullRequestFields = append(IssueFields, "statusCheckRollup", ) +type issueGraphQLOpts struct { + useCheckRunAndStatusContextCounts bool +} + +type IssueGraphQLOptFn func(*issueGraphQLOpts) + +func WithUseCheckRunAndStatusContextCounts() IssueGraphQLOptFn { + return func(opts *issueGraphQLOpts) { + opts.useCheckRunAndStatusContextCounts = true + } +} + // IssueGraphQL constructs a GraphQL query fragment for a set of issue fields. -func IssueGraphQL(fields []string) string { +func IssueGraphQL(fields []string, opts ...IssueGraphQLOptFn) string { + issueGraphQLOpts := issueGraphQLOpts{ + useCheckRunAndStatusContextCounts: false, + } + + for _, opt := range opts { + opt(&issueGraphQLOpts) + } + var q []string for _, field := range fields { switch field { @@ -320,7 +364,11 @@ func IssueGraphQL(fields []string) string { case "requiresStrictStatusChecks": // pseudo-field q = append(q, `baseRef{branchProtectionRule{requiresStrictStatusChecks}}`) case "statusCheckRollup": - q = append(q, StatusCheckRollupGraphQL("")) + if issueGraphQLOpts.useCheckRunAndStatusContextCounts { + q = append(q, StatusCheckRollupGraphQLWithCountByState()) + } else { + q = append(q, StatusCheckRollupGraphQLWithoutCountByState("")) + } default: q = append(q, field) } @@ -330,12 +378,12 @@ func IssueGraphQL(fields []string) string { // PullRequestGraphQL constructs a GraphQL query fragment for a set of pull request fields. // It will try to sanitize the fields to just those available on pull request. -func PullRequestGraphQL(fields []string) string { +func PullRequestGraphQL(fields []string, opts ...IssueGraphQLOptFn) string { invalidFields := []string{"isPinned", "stateReason"} s := set.NewStringSet() s.AddValues(fields) s.RemoveValues(invalidFields) - return IssueGraphQL(s.ToSlice()) + return IssueGraphQL(s.ToSlice(), opts...) } var RepositoryFields = []string{ diff --git a/pkg/cmd/pr/shared/finder.go b/pkg/cmd/pr/shared/finder.go index f1886b3fe..4bb79ac8b 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -465,7 +465,7 @@ func preloadPrChecks(client *http.Client, repo ghrepo.Interface, pr *api.PullReq %s } } - }`, api.StatusCheckRollupGraphQL("$endCursor")) + }`, api.StatusCheckRollupGraphQLWithoutCountByState("$endCursor")) variables := map[string]interface{}{ "id": pr.ID, diff --git a/pkg/cmd/pr/status/fixtures/prIntrospection.json b/pkg/cmd/pr/status/fixtures/prIntrospection.json new file mode 100644 index 000000000..e50ca8df5 --- /dev/null +++ b/pkg/cmd/pr/status/fixtures/prIntrospection.json @@ -0,0 +1,317 @@ +{ + "data": { + "PullRequest": { + "fields": [ + { + "name": "activeLockReason" + }, + { + "name": "additions" + }, + { + "name": "assignees" + }, + { + "name": "author" + }, + { + "name": "authorAssociation" + }, + { + "name": "autoMergeRequest" + }, + { + "name": "baseRef" + }, + { + "name": "baseRefName" + }, + { + "name": "baseRefOid" + }, + { + "name": "baseRepository" + }, + { + "name": "body" + }, + { + "name": "bodyHTML" + }, + { + "name": "bodyText" + }, + { + "name": "changedFiles" + }, + { + "name": "checksResourcePath" + }, + { + "name": "checksUrl" + }, + { + "name": "closed" + }, + { + "name": "closedAt" + }, + { + "name": "closingIssuesReferences" + }, + { + "name": "comments" + }, + { + "name": "commits" + }, + { + "name": "createdAt" + }, + { + "name": "createdViaEmail" + }, + { + "name": "databaseId" + }, + { + "name": "deletions" + }, + { + "name": "editor" + }, + { + "name": "files" + }, + { + "name": "headRef" + }, + { + "name": "headRefName" + }, + { + "name": "headRefOid" + }, + { + "name": "headRepository" + }, + { + "name": "headRepositoryOwner" + }, + { + "name": "hovercard" + }, + { + "name": "id" + }, + { + "name": "includesCreatedEdit" + }, + { + "name": "isCrossRepository" + }, + { + "name": "isDraft" + }, + { + "name": "isInMergeQueue" + }, + { + "name": "isMergeQueueEnabled" + }, + { + "name": "isReadByViewer" + }, + { + "name": "labels" + }, + { + "name": "lastEditedAt" + }, + { + "name": "latestOpinionatedReviews" + }, + { + "name": "latestReviews" + }, + { + "name": "locked" + }, + { + "name": "maintainerCanModify" + }, + { + "name": "mergeCommit" + }, + { + "name": "mergeQueue" + }, + { + "name": "mergeQueueEntry" + }, + { + "name": "mergeable" + }, + { + "name": "merged" + }, + { + "name": "mergedAt" + }, + { + "name": "mergedBy" + }, + { + "name": "milestone" + }, + { + "name": "number" + }, + { + "name": "participants" + }, + { + "name": "permalink" + }, + { + "name": "potentialMergeCommit" + }, + { + "name": "projectCards" + }, + { + "name": "projectItems" + }, + { + "name": "projectV2" + }, + { + "name": "projectsV2" + }, + { + "name": "publishedAt" + }, + { + "name": "reactionGroups" + }, + { + "name": "reactions" + }, + { + "name": "repository" + }, + { + "name": "resourcePath" + }, + { + "name": "revertResourcePath" + }, + { + "name": "revertUrl" + }, + { + "name": "reviewDecision" + }, + { + "name": "reviewRequests" + }, + { + "name": "reviewThreads" + }, + { + "name": "reviews" + }, + { + "name": "state" + }, + { + "name": "suggestedReviewers" + }, + { + "name": "timeline" + }, + { + "name": "timelineItems" + }, + { + "name": "title" + }, + { + "name": "titleHTML" + }, + { + "name": "totalCommentsCount" + }, + { + "name": "updatedAt" + }, + { + "name": "url" + }, + { + "name": "userContentEdits" + }, + { + "name": "viewerCanAddAndRemoveFromMergeQueue" + }, + { + "name": "viewerCanApplySuggestion" + }, + { + "name": "viewerCanClose" + }, + { + "name": "viewerCanDeleteHeadRef" + }, + { + "name": "viewerCanDisableAutoMerge" + }, + { + "name": "viewerCanEditFiles" + }, + { + "name": "viewerCanEnableAutoMerge" + }, + { + "name": "viewerCanMergeAsAdmin" + }, + { + "name": "viewerCanReact" + }, + { + "name": "viewerCanReopen" + }, + { + "name": "viewerCanSubscribe" + }, + { + "name": "viewerCanUpdate" + }, + { + "name": "viewerCanUpdateBranch" + }, + { + "name": "viewerCannotUpdateReasons" + }, + { + "name": "viewerDidAuthor" + }, + { + "name": "viewerLatestReview" + }, + { + "name": "viewerLatestReviewRequest" + }, + { + "name": "viewerMergeBodyText" + }, + { + "name": "viewerMergeHeadlineText" + }, + { + "name": "viewerSubscription" + } + ] + } + } +} diff --git a/pkg/cmd/pr/status/http.go b/pkg/cmd/pr/status/http.go index 49bdb147c..1c38e2221 100644 --- a/pkg/cmd/pr/status/http.go +++ b/pkg/cmd/pr/status/http.go @@ -4,8 +4,10 @@ import ( "fmt" "net/http" "strings" + "time" "github.com/cli/cli/v2/api" + fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/set" @@ -47,17 +49,30 @@ func pullRequestStatus(httpClient *http.Client, repo ghrepo.Interface, options r ReviewRequested edges } + // SPIKE: Let's see if we can detect here and wire through + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + detector := fd.NewDetector(cachedClient, repo.RepoHost()) + prFeatures, err := detector.PullRequestFeatures() + if err != nil { + return nil, err + } + + var prGraphQLOpts []api.IssueGraphQLOptFn + if prFeatures.CheckRunAndStatusContextCounts { + prGraphQLOpts = append(prGraphQLOpts, api.WithUseCheckRunAndStatusContextCounts()) + } + var fragments string if len(options.Fields) > 0 { fields := set.NewStringSet() fields.AddValues(options.Fields) // these are always necessary to find the PR for the current branch fields.AddValues([]string{"isCrossRepository", "headRepositoryOwner", "headRefName"}) - gr := api.PullRequestGraphQL(fields.ToSlice()) + gr := api.PullRequestGraphQL(fields.ToSlice(), prGraphQLOpts...) fragments = fmt.Sprintf("fragment pr on PullRequest{%s}fragment prWithReviews on PullRequest{...pr}", gr) } else { var err error - fragments, err = pullRequestFragment(repo.RepoHost(), options.ConflictStatus) + fragments, err = pullRequestFragment(repo.RepoHost(), options.ConflictStatus, prGraphQLOpts...) if err != nil { return nil, err } @@ -146,8 +161,7 @@ func pullRequestStatus(httpClient *http.Client, repo ghrepo.Interface, options r } var resp response - err := apiClient.GraphQL(repo.RepoHost(), query, variables, &resp) - if err != nil { + if err = apiClient.GraphQL(repo.RepoHost(), query, variables, &resp); err != nil { return nil, err } @@ -187,7 +201,7 @@ func pullRequestStatus(httpClient *http.Client, repo ghrepo.Interface, options r return &payload, nil } -func pullRequestFragment(hostname string, conflictStatus bool) (string, error) { +func pullRequestFragment(hostname string, conflictStatus bool, opts ...api.IssueGraphQLOptFn) (string, error) { fields := []string{ "number", "title", "state", "url", "isDraft", "isCrossRepository", "headRefName", "headRepositoryOwner", "mergeStateStatus", @@ -201,6 +215,6 @@ func pullRequestFragment(hostname string, conflictStatus bool) (string, error) { fragments := fmt.Sprintf(` fragment pr on PullRequest {%s} fragment prWithReviews on PullRequest {...pr,%s} - `, api.PullRequestGraphQL(fields), api.PullRequestGraphQL(reviewFields)) + `, api.PullRequestGraphQL(fields, opts...), api.PullRequestGraphQL(reviewFields, opts...)) return fragments, nil } diff --git a/pkg/cmd/pr/status/status_test.go b/pkg/cmd/pr/status/status_test.go index 1b39ddb24..8529c070b 100644 --- a/pkg/cmd/pr/status/status_test.go +++ b/pkg/cmd/pr/status/status_test.go @@ -76,7 +76,10 @@ func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*t } func initFakeHTTP() *httpmock.Registry { - return &httpmock.Registry{} + registry := &httpmock.Registry{} + // SPIKE: perhaps we should inject a mock detector into the command instead? + registry.Register(httpmock.GraphQL(`query PullRequest_fields\b`), httpmock.FileResponse("./fixtures/prIntrospection.json")) + return registry } func TestPRStatus(t *testing.T) {