Implement initial spike for detecting and using counts by state

This commit is contained in:
William Martin 2023-05-17 18:44:00 +02:00
parent bfd3c7b20b
commit ebb7d26f3a
5 changed files with 395 additions and 13 deletions

View file

@ -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{

View file

@ -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,

View file

@ -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"
}
]
}
}
}

View file

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

View file

@ -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) {