Skip to content

Commit

Permalink
Fix pr status for GHE 2.22 and older
Browse files Browse the repository at this point in the history 
This queries for the availability of the `branchProtectionRule` field on
"Ref" before trying to request it from GraphQL.
  • Loading branch information
@mislav
mislav committed Feb 18, 2021
1 parent 2f563ba commit dcff6c4
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 49 deletions.
61 changes: 44 additions & 17 deletions api/queries_pr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/cli/cli/internal/ghinstance"
"github.com/cli/cli/internal/ghrepo"
"github.com/shurcooL/githubv4"
"golang.org/x/sync/errgroup"
)

type PullRequestsPayload struct {
Expand Down Expand Up @@ -232,14 +233,16 @@ func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNumber int) (io.Rea
}

type pullRequestFeature struct {
HasReviewDecision bool
HasStatusCheckRollup bool
HasReviewDecision bool
HasStatusCheckRollup bool
HasBranchProtectionRule bool
}

func determinePullRequestFeatures(httpClient *http.Client, hostname string) (prFeatures pullRequestFeature, err error) {
if !ghinstance.IsEnterprise(hostname) {
prFeatures.HasReviewDecision = true
prFeatures.HasStatusCheckRollup = true
prFeatures.HasBranchProtectionRule = true
return
}

Expand All @@ -256,8 +259,26 @@ func determinePullRequestFeatures(httpClient *http.Client, hostname string) (prF
} `graphql:"Commit: __type(name: \"Commit\")"`
}

// needs to be a separate query because the backend only supports 2 `__type` expressions in one query
var featureDetection2 struct {
Ref struct {
Fields []struct {
Name string
} `graphql:"fields(includeDeprecated: true)"`
} `graphql:"Ref: __type(name: \"Ref\")"`
}

v4 := graphQLClient(httpClient, hostname)
err = v4.QueryNamed(context.Background(), "PullRequest_fields", &featureDetection, nil)

g := new(errgroup.Group)
g.Go(func() error {
return v4.QueryNamed(context.Background(), "PullRequest_fields", &featureDetection, nil)
})
g.Go(func() error {
return v4.QueryNamed(context.Background(), "PullRequest_fields2", &featureDetection2, nil)
})

err = g.Wait()
if err != nil {
return
}
Expand All @@ -274,6 +295,12 @@ func determinePullRequestFeatures(httpClient *http.Client, hostname string) (prF
prFeatures.HasStatusCheckRollup = true
}
}
for _, field := range featureDetection2.Ref.Fields {
switch field.Name {
case "branchProtectionRule":
prFeatures.HasBranchProtectionRule = true
}
}
return
}

Expand Down Expand Up @@ -333,6 +360,16 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
`
}

var requiresStrictStatusChecks string
if prFeatures.HasBranchProtectionRule {
requiresStrictStatusChecks = `
baseRef {
branchProtectionRule {
requiresStrictStatusChecks
}
}`
}

fragments := fmt.Sprintf(`
fragment pr on PullRequest {
number
Expand All @@ -344,11 +381,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
headRepositoryOwner {
login
}
baseRef {
branchProtectionRule {
requiresStrictStatusChecks
}
}
%s
isCrossRepository
isDraft
%s
Expand All @@ -357,16 +390,13 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
...pr
%s
}
`, statusesFragment, reviewsFragment)
`, requiresStrictStatusChecks, statusesFragment, reviewsFragment)

queryPrefix := `
query PullRequestStatus($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) {
repository(owner: $owner, name: $repo) {
defaultBranchRef {
name
branchProtectionRule {
requiresStrictStatusChecks
}
}
pullRequests(headRefName: $headRefName, first: $per_page, orderBy: { field: CREATED_AT, direction: DESC }) {
totalCount
Expand All @@ -382,12 +412,9 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
queryPrefix = `
query PullRequestStatus($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) {
repository(owner: $owner, name: $repo) {
defaultBranchRef {
name
branchProtectionRule {
requiresStrictStatusChecks
defaultBranchRef {
name
}
}
pullRequest(number: $number) {
...prWithReviews
}
Expand Down
84 changes: 52 additions & 32 deletions api/queries_pr_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,66 +52,88 @@ func Test_determinePullRequestFeatures(t *testing.T) {
tests := []struct {
name string
hostname string
queryResponse string
queryResponse map[string]string
wantPrFeatures pullRequestFeature
wantErr bool
}{
{
name: "github.com",
hostname: "github.com",
wantPrFeatures: pullRequestFeature{
HasReviewDecision: true,
HasStatusCheckRollup: true,
HasReviewDecision: true,
HasStatusCheckRollup: true,
HasBranchProtectionRule: true,
},
wantErr: false,
},
{
name: "GHE empty response",
hostname: "git.my.org",
queryResponse: heredoc.Doc(`
{"data": {}}
`),
queryResponse: map[string]string{
`query PullRequest_fields\b`: `{"data": {}}`,
`query PullRequest_fields2\b`: `{"data": {}}`,
},
wantPrFeatures: pullRequestFeature{
HasReviewDecision: false,
HasStatusCheckRollup: false,
HasReviewDecision: false,
HasStatusCheckRollup: false,
HasBranchProtectionRule: false,
},
wantErr: false,
},
{
name: "GHE has reviewDecision",
hostname: "git.my.org",
queryResponse: heredoc.Doc(`
{"data": {
"PullRequest": {
"fields": [
queryResponse: map[string]string{
`query PullRequest_fields\b`: heredoc.Doc(`
{ "data": { "PullRequest": { "fields": [
{"name": "foo"},
{"name": "reviewDecision"}
]
}
} }
`),
] } } }
`),
`query PullRequest_fields2\b`: `{"data": {}}`,
},
wantPrFeatures: pullRequestFeature{
HasReviewDecision: true,
HasStatusCheckRollup: false,
HasReviewDecision: true,
HasStatusCheckRollup: false,
HasBranchProtectionRule: false,
},
wantErr: false,
},
{
name: "GHE has statusCheckRollup",
hostname: "git.my.org",
queryResponse: heredoc.Doc(`
{"data": {
"Commit": {
"fields": [
queryResponse: map[string]string{
`query PullRequest_fields\b`: heredoc.Doc(`
{ "data": { "Commit": { "fields": [
{"name": "foo"},
{"name": "statusCheckRollup"}
]
}
} }
`),
] } } }
`),
`query PullRequest_fields2\b`: `{"data": {}}`,
},
wantPrFeatures: pullRequestFeature{
HasReviewDecision: false,
HasStatusCheckRollup: true,
HasBranchProtectionRule: false,
},
wantErr: false,
},
{
name: "GHE has branchProtectionRule",
hostname: "git.my.org",
queryResponse: map[string]string{
`query PullRequest_fields\b`: `{"data": {}}`,
`query PullRequest_fields2\b`: heredoc.Doc(`
{ "data": { "Ref": { "fields": [
{"name": "foo"},
{"name": "branchProtectionRule"}
] } } }
`),
},
wantPrFeatures: pullRequestFeature{
HasReviewDecision: false,
HasStatusCheckRollup: true,
HasReviewDecision: false,
HasStatusCheckRollup: false,
HasBranchProtectionRule: true,
},
wantErr: false,
},
Expand All @@ -121,10 +143,8 @@ func Test_determinePullRequestFeatures(t *testing.T) {
fakeHTTP := &httpmock.Registry{}
httpClient := NewHTTPClient(ReplaceTripper(fakeHTTP))

if tt.queryResponse != "" {
fakeHTTP.Register(
httpmock.GraphQL(`query PullRequest_fields\b`),
httpmock.StringResponse(tt.queryResponse))
for query, resp := range tt.queryResponse {
fakeHTTP.Register(httpmock.GraphQL(query), httpmock.StringResponse(resp))
}

gotPrFeatures, err := determinePullRequestFeatures(httpClient, tt.hostname)
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ require (
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.6.1
golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897
golang.org/x/sync v0.0.0-20190423024810-112230192c58
golang.org/x/text v0.3.4 // indirect
gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
)
Expand Down
1 change: 1 addition & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
Expand Down

0 comments on commit dcff6c4

Please sign in to comment.