Merge pull request cli#2996 from cli/ghe-branchprotectionrule

Fix `pr status` for GHE 2.22 and older

api/queries_pr.go

@@ -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 {
@@ -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
 	}
 
@@ -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
 	}
@@ -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
 }
 
@@ -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
@@ -344,11 +381,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu
 		headRepositoryOwner {
 			login
 		}
-		baseRef {
-			branchProtectionRule {
-				requiresStrictStatusChecks
-			}
-		}
+		%s
 		isCrossRepository
 		isDraft
 		%s
@@ -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
@@ -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
 				}

api/queries_pr_test.go

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

go.mod

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

go.sum

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