8000 Fix `pr status` for GHE 2.22 and older by mislav · Pull Request #2996 · cli/cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix pr status for GHE 2.22 and older #2996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 44 additions & 17 deletions api/queries_pr.go
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
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
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
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