X Tutup
Skip to content

Commit 04dcb32

Browse files
mislavmislav
authored
Merge pull request cli#2996 from cli/ghe-branchprotectionrule
Fix `pr status` for GHE 2.22 and older
2 parents 2f563ba + dcff6c4 commit 04dcb32

File tree

4 files changed

+98
-49
lines changed

4 files changed

+98
-49
lines changed

‎api/queries_pr.go‎

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/cli/cli/internal/ghinstance"
1414
"github.com/cli/cli/internal/ghrepo"
1515
"github.com/shurcooL/githubv4"
16+
"golang.org/x/sync/errgroup"
1617
)
1718

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

234235
type pullRequestFeature struct {
235-
HasReviewDecision bool
236-
HasStatusCheckRollup bool
236+
HasReviewDecision bool
237+
HasStatusCheckRollup bool
238+
HasBranchProtectionRule bool
237239
}
238240

239241
func determinePullRequestFeatures(httpClient *http.Client, hostname string) (prFeatures pullRequestFeature, err error) {
240242
if !ghinstance.IsEnterprise(hostname) {
241243
prFeatures.HasReviewDecision = true
242244
prFeatures.HasStatusCheckRollup = true
245+
prFeatures.HasBranchProtectionRule = true
243246
return
244247
}
245248

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

262+
// needs to be a separate query because the backend only supports 2 `__type` expressions in one query
263+
var featureDetection2 struct {
264+
Ref struct {
265+
Fields []struct {
266+
Name string
267+
} `graphql:"fields(includeDeprecated: true)"`
268+
} `graphql:"Ref: __type(name: \"Ref\")"`
269+
}
270+
259271
v4 := graphQLClient(httpClient, hostname)
260-
err = v4.QueryNamed(context.Background(), "PullRequest_fields", &featureDetection, nil)
272+
273+
g := new(errgroup.Group)
274+
g.Go(func() error {
275+
return v4.QueryNamed(context.Background(), "PullRequest_fields", &featureDetection, nil)
276+
})
277+
g.Go(func() error {
278+
return v4.QueryNamed(context.Background(), "PullRequest_fields2", &featureDetection2, nil)
279+
})
280+
281+
err = g.Wait()
261282
if err != nil {
262283
return
263284
}
@@ -274,6 +295,12 @@ func determinePullRequestFeatures(httpClient *http.Client, hostname string) (prF
274295
prFeatures.HasStatusCheckRollup = true
275296
}
276297
}
298+
for _, field := range featureDetection2.Ref.Fields {
299+
switch field.Name {
300+
case "branchProtectionRule":
301+
prFeatures.HasBranchProtectionRule = true
302+
}
303+
}
277304
return
278305
}
279306

@@ -333,6 +360,16 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
333360
`
334361
}
335362

363+
var requiresStrictStatusChecks string
364+
if prFeatures.HasBranchProtectionRule {
365+
requiresStrictStatusChecks = `
366+
baseRef {
367+
branchProtectionRule {
368+
requiresStrictStatusChecks
369+
}
370+
}`
371+
}
372+
336373
fragments := fmt.Sprintf(`
337374
fragment pr on PullRequest {
338375
number
@@ -344,11 +381,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
344381
headRepositoryOwner {
345382
login
346383
}
347-
baseRef {
348-
branchProtectionRule {
349-
requiresStrictStatusChecks
350-
}
351-
}
384+
%s
352385
isCrossRepository
353386
isDraft
354387
%s
@@ -357,16 +390,13 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
357390
...pr
358391
%s
359392
}
360-
`, statusesFragment, reviewsFragment)
393+
`, requiresStrictStatusChecks, statusesFragment, reviewsFragment)
361394

362395
queryPrefix := `
363396
query PullRequestStatus($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) {
364397
repository(owner: $owner, name: $repo) {
365398
defaultBranchRef {
366399
name
367-
branchProtectionRule {
368-
requiresStrictStatusChecks
369-
}
370400
}
371401
pullRequests(headRefName: $headRefName, first: $per_page, orderBy: { field: CREATED_AT, direction: DESC }) {
372402
totalCount
@@ -382,12 +412,9 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
382412
queryPrefix = `
383413
query PullRequestStatus($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) {
384414
repository(owner: $owner, name: $repo) {
385-
defaultBranchRef {
386-
name
387-
branchProtectionRule {
388-
requiresStrictStatusChecks
415+
defaultBranchRef {
416+
name
389417
}
390-
}
391418
pullRequest(number: $number) {
392419
...prWithReviews
393420
}

‎api/queries_pr_test.go‎

Lines changed: 52 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -52,66 +52,88 @@ func Test_determinePullRequestFeatures(t *testing.T) {
5252
tests := []struct {
5353
name string
5454
hostname string
55-
queryResponse string
55+
queryResponse map[string]string
5656
wantPrFeatures pullRequestFeature
5757
wantErr bool
5858
}{
5959
{
6060
name: "github.com",
6161
hostname: "github.com",
6262
wantPrFeatures: pullRequestFeature{
63-
HasReviewDecision: true,
64-
HasStatusCheckRollup: true,
63+
HasReviewDecision: true,
64+
HasStatusCheckRollup: true,
65+
HasBranchProtectionRule: true,
6566
},
6667
wantErr: false,
6768
},
6869
{
6970
name: "GHE empty response",
7071
hostname: "git.my.org",
71-
queryResponse: heredoc.Doc(`
72-
{"data": {}}
73-
`),
72+
queryResponse: map[string]string{
73+
`query PullRequest_fields\b`: `{"data": {}}`,
74+
`query PullRequest_fields2\b`: `{"data": {}}`,
75+
},
7476
wantPrFeatures: pullRequestFeature{
75-
HasReviewDecision: false,
76-
HasStatusCheckRollup: false,
77+
HasReviewDecision: false,
78+
HasStatusCheckRollup: false,
79+
HasBranchProtectionRule: false,
7780
},
7881
wantErr: false,
7982
},
8083
{
8184
name: "GHE has reviewDecision",
8285
hostname: "git.my.org",
83-
queryResponse: heredoc.Doc(`
84-
{"data": {
85-
"PullRequest": {
86-
"fields": [
86+
queryResponse: map[string]string{
87+
`query PullRequest_fields\b`: heredoc.Doc(`
88+
{ "data": { "PullRequest": { "fields": [
8789
{"name": "foo"},
8890
{"name": "reviewDecision"}
89-
]
90-
}
91-
} }
92-
`),
91+
] } } }
92+
`),
93+
`query PullRequest_fields2\b`: `{"data": {}}`,
94+
},
9395
wantPrFeatures: pullRequestFeature{
94-
HasReviewDecision: true,
95-
HasStatusCheckRollup: false,
96+
HasReviewDecision: true,
97+
HasStatusCheckRollup: false,
98+
HasBranchProtectionRule: false,
9699
},
97100
wantErr: false,
98101
},
99102
{
100103
name: "GHE has statusCheckRollup",
101104
hostname: "git.my.org",
102-
queryResponse: heredoc.Doc(`
103-
{"data": {
104-
"Commit": {
105-
"fields": [
105+
queryResponse: map[string]string{
106+
`query PullRequest_fields\b`: heredoc.Doc(`
107+
{ "data": { "Commit": { "fields": [
106108
{"name": "foo"},
107109
{"name": "statusCheckRollup"}
108-
]
109-
}
110-
} }
111-
`),
110+
] } } }
111+
`),
112+
`query PullRequest_fields2\b`: `{"data": {}}`,
113+
},
114+
wantPrFeatures: pullRequestFeature{
115+
HasReviewDecision: false,
116+
HasStatusCheckRollup: true,
117+
HasBranchProtectionRule: false,
118+
},
119+
wantErr: false,
120+
},
121+
{
122+
name: "GHE has branchProtectionRule",
123+
hostname: "git.my.org",
124+
queryResponse: map[string]string{
125+
`query PullRequest_fields\b`: `{"data": {}}`,
126+
`query PullRequest_fields2\b`: heredoc.Doc(`
127+
{ "data": { "Ref": { "fields": [
128+
{"name": "foo"},
129+
{"name": "branchProtectionRule"}
130+
] } } }
131+
`),
132+
},
112133
wantPrFeatures: pullRequestFeature{
113-
HasReviewDecision: false,
114-
HasStatusCheckRollup: true,
134+
HasReviewDecision: false,
135+
HasStatusCheckRollup: false,
136+
HasBranchProtectionRule: true,
115137
},
116138
wantErr: false,
117139
},
@@ -121,10 +143,8 @@ func Test_determinePullRequestFeatures(t *testing.T) {
121143
fakeHTTP := &httpmock.Registry{}
122144
httpClient := NewHTTPClient(ReplaceTripper(fakeHTTP))
123145

124-
if tt.queryResponse != "" {
125-
fakeHTTP.Register(
126-
httpmock.GraphQL(`query PullRequest_fields\b`),
127-
httpmock.StringResponse(tt.queryResponse))
146+
for query, resp := range tt.queryResponse {
147+
fakeHTTP.Register(httpmock.GraphQL(query), httpmock.StringResponse(resp))
128148
}
129149

130150
gotPrFeatures, err := determinePullRequestFeatures(httpClient, tt.hostname)

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ require (
2929
github.com/spf13/pflag v1.0.5
3030
github.com/stretchr/testify v1.6.1
3131
golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897
32+
golang.org/x/sync v0.0.0-20190423024810-112230192c58
3233
golang.org/x/text v0.3.4 // indirect
3334
gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
3435
)

‎go.sum‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,7 @@ golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJ
331331
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
332332
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
333333
golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
334+
golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
334335
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
335336
golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
336337
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=

0 commit comments

Comments
 (0)
X Tutup