Repository: google/go-github
Pull request #2276 (2022-01-28 22:31:48)

Add checks field to RequiredStatusChecks
Attempt to solve #2274.

From my own experimentation, I discovered that there's two ways that this `Checks` field can be used to update the required status checks for a protected branch:
- [update branch protection](https://docs.github.com/en/rest/reference/branches#update-branch-protection) -> via the `ProtectionRequest` -> `RequiredStatusChecks` -> `RequiredStatusCheck` struct, in this API call we must provide either one of `Checks` or `Contexts` (the other must be `nil`) in `RequiredStatusChecks` to successfully update a branch protection. If you provide both or neither, you will get an `Invalid Request` error.
- [update status check protection](https://docs.github.com/en/rest/reference/branches#update-status-check-protection) -> via the `RequiredStatusChecksRequest` -> `RequiredStatusCheck` struct, in this API call `Checks` and `Contexts` are both optional fields, if you provide both of them only `Checks` will be used to update the status checks.

codecov[bot]: # [Codecov](https://codecov.io/gh/google/go-github/pull/2276?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) Report
> Merging [#2276](https://codecov.io/gh/google/go-github/pull/2276?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) (3de48b4) into [master](https://codecov.io/gh/google/go-github/commit/51df45c7be1462614aed9884f2090a81f8d27154?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) (51df45c) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/google/go-github/pull/2276/graphs/tree.svg?width=650&height=150&src=pr&token=EgYjGjZlI0&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google)](https://codecov.io/gh/google/go-github/pull/2276?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google)

```diff
@@           Coverage Diff           @@
##           master    #2276   +/-   ##
=======================================
  Coverage   97.81%   97.81%           
=======================================
  Files         114      114           
  Lines       10266    10266           
=======================================
  Hits        10042    10042           
  Misses        156      156           
  Partials       68       68           
```


| [Impacted Files](https://codecov.io/gh/google/go-github/pull/2276?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) | Coverage Δ | |
|---|---|---|
| [github/repos.go](https://codecov.io/gh/google/go-github/pull/2276/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-Z2l0aHViL3JlcG9zLmdv) | `98.70% <ø> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-github/pull/2276?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/go-github/pull/2276?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google). Last update [51df45c...3de48b4](https://codecov.io/gh/google/go-github/pull/2276?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).

gmlewis: It looks like the linter has a couple issues as well.

Patch:
diff --git a/github/github-accessors.go b/github/github-accessors.go
index b16aefff..96cfbc59 100644
--- a/github/github-accessors.go
+++ b/github/github-accessors.go
@@ -15444,6 +15444,14 @@ func (r *RequiredReviewer) GetType() string {
 	return *r.Type
 }
 
+// GetAppID returns the AppID field if it's non-nil, zero value otherwise.
+func (r *RequiredStatusCheck) GetAppID() int64 {
+	if r == nil || r.AppID == nil {
+		return 0
+	}
+	return *r.AppID
+}
+
 // GetStrict returns the Strict field if it's non-nil, zero value otherwise.
 func (r *RequiredStatusChecksRequest) GetStrict() bool {
 	if r == nil || r.Strict == nil {
diff --git a/github/repos.go b/github/repos.go
index a1c8c810..42cd1636 100644
--- a/github/repos.go
+++ b/github/repos.go
@@ -877,14 +877,33 @@ type RequiredStatusChecks struct {
 	// Require branches to be up to date before merging. (Required.)
 	Strict bool `json:"strict"`
 	// The list of status checks to require in order to merge into this
-	// branch. (Required; use []string{} instead of nil for empty list.)
-	Contexts []string `json:"contexts"`
+	// branch. (Deprecated. Note: only one of Contexts/Checks can be populated,
+	// but at least one must be populated).
+	Contexts []string `json:"contexts,omitempty"`
+	// The list of status checks to require in order to merge into this
+	// branch.
+	Checks []*RequiredStatusCheck `json:"checks,omitempty"`
 }
 
 // RequiredStatusChecksRequest represents a request to edit a protected branch's status checks.
 type RequiredStatusChecksRequest struct {
-	Strict   *bool    `json:"strict,omitempty"`
-	Contexts []string `json:"contexts,omitempty"`
+	Strict *bool `json:"strict,omitempty"`
+	// Note: if both Contexts and Checks are populated,
+	// the GitHub API will only use Checks.
+	Contexts []string               `json:"contexts,omitempty"`
+	Checks   []*RequiredStatusCheck `json:"checks,omitempty"`
+}
+
+// RequiredStatusCheck represents a status check of a protected branch.
+type RequiredStatusCheck struct {
+	// The name of the required check.
+	Context string `json:"context"`
+	// The ID of the GitHub App that must provide this check.
+	// Omit this field to automatically select the GitHub App
+	// that has recently provided this check,
+	// or any app if it was not set by a GitHub App.
+	// Pass -1 to explicitly allow any app to set the status.
+	AppID *int64 `json:"app_id,omitempty"`
 }
 
 // PullRequestReviewsEnforcement represents the pull request reviews enforcement of a protected branch.


Test patch:
diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go
index 280dc0c7..4aa6d6df 100644
--- a/github/github-accessors_test.go
+++ b/github/github-accessors_test.go
@@ -18043,6 +18043,16 @@ func TestRequiredReviewer_GetType(tt *testing.T) {
 	r.GetType()
 }
 
+func TestRequiredStatusCheck_GetAppID(tt *testing.T) {
+	var zeroValue int64
+	r := &RequiredStatusCheck{AppID: &zeroValue}
+	r.GetAppID()
+	r = &RequiredStatusCheck{}
+	r.GetAppID()
+	r = nil
+	r.GetAppID()
+}
+
 func TestRequiredStatusChecksRequest_GetStrict(tt *testing.T) {
 	var zeroValue bool
 	r := &RequiredStatusChecksRequest{Strict: &zeroValue}
diff --git a/github/repos_test.go b/github/repos_test.go
index 58c8aab5..5739c10b 100644
--- a/github/repos_test.go
+++ b/github/repos_test.go
@@ -1040,7 +1040,13 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) {
 		fmt.Fprintf(w, `{
 				"required_status_checks":{
 					"strict":true,
-					"contexts":["continuous-integration"]
+					"contexts":["continuous-integration"],
+					"checks": [
+						{
+							"context": "continuous-integration",
+							"app_id": null
+						}
+					]
 				},
 				"required_pull_request_reviews":{
 					"dismissal_restrictions":{
@@ -1081,6 +1087,11 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) {
 		RequiredStatusChecks: &RequiredStatusChecks{
 			Strict:   true,
 			Contexts: []string{"continuous-integration"},
+			Checks: []*RequiredStatusCheck{
+				{
+					Context: "continuous-integration",
+				},
+			},
 		},
 		RequiredPullRequestReviews: &PullRequestReviewsEnforcement{
 			DismissStaleReviews: true,
@@ -1141,7 +1152,13 @@ func TestRepositoriesService_GetBranchProtection_noDismissalRestrictions(t *test
 		fmt.Fprintf(w, `{
 				"required_status_checks":{
 					"strict":true,
-					"contexts":["continuous-integration"]
+					"contexts":["continuous-integration"],
+					"checks": [
+						{
+							"context": "continuous-integration",
+							"app_id": null
+						}
+					]
 				},
 				"required_pull_request_reviews":{
 					"dismiss_stale_reviews":true,
@@ -1169,6 +1186,11 @@ func TestRepositoriesService_GetBranchProtection_noDismissalRestrictions(t *test
 		RequiredStatusChecks: &RequiredStatusChecks{
 			Strict:   true,
 			Contexts: []string{"continuous-integration"},
+			Checks: []*RequiredStatusCheck{
+				{
+					Context: "continuous-integration",
+				},
+			},
 		},
 		RequiredPullRequestReviews: &PullRequestReviewsEnforcement{
 			DismissStaleReviews:          true,
@@ -1220,7 +1242,7 @@ func TestRepositoriesService_GetBranchProtection_branchNotProtected(t *testing.T
 	}
 }
 
-func TestRepositoriesService_UpdateBranchProtection(t *testing.T) {
+func TestRepositoriesService_UpdateBranchProtection_Contexts(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
 
@@ -1257,7 +1279,13 @@ func TestRepositoriesService_UpdateBranchProtection(t *testing.T) {
 		fmt.Fprintf(w, `{
 			"required_status_checks":{
 				"strict":true,
-				"contexts":["continuous-integration"]
+				"contexts":["continuous-integration"],
+				"checks": [
+					{
+						"context": "continuous-integration",
+						"app_id": null
+					}
+				]
 			},
 			"required_pull_request_reviews":{
 				"dismissal_restrictions":{
@@ -1291,6 +1319,11 @@ func TestRepositoriesService_UpdateBranchProtection(t *testing.T) {
 		RequiredStatusChecks: &RequiredStatusChecks{
 			Strict:   true,
 			Contexts: []string{"continuous-integration"},
+			Checks: []*RequiredStatusCheck{
+				{
+					Context: "continuous-integration",
+				},
+			},
 		},
 		RequiredPullRequestReviews: &PullRequestReviewsEnforcement{
 			DismissStaleReviews: true,
@@ -1335,6 +1368,122 @@ func TestRepositoriesService_UpdateBranchProtection(t *testing.T) {
 	})
 }
 
+func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	input := &ProtectionRequest{
+		RequiredStatusChecks: &RequiredStatusChecks{
+			Strict: true,
+			Checks: []*RequiredStatusCheck{
+				{
+					Context: "continuous-integration",
+				},
+			},
+		},
+		RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{
+			DismissStaleReviews: true,
+			DismissalRestrictionsRequest: &DismissalRestrictionsRequest{
+				Users: &[]string{"uu"},
+				Teams: &[]string{"tt"},
+			},
+		},
+		Restrictions: &BranchRestrictionsRequest{
+			Users: []string{"u"},
+			Teams: []string{"t"},
+			Apps:  []string{"a"},
+		},
+	}
+
+	mux.HandleFunc("/repos/o/r/branches/b/protection", func(w http.ResponseWriter, r *http.Request) {
+		v := new(ProtectionRequest)
+		json.NewDecoder(r.Body).Decode(v)
+
+		testMethod(t, r, "PUT")
+		if !cmp.Equal(v, input) {
+			t.Errorf("Request body = %+v, want %+v", v, input)
+		}
+
+		// TODO: remove custom Accept header when this API fully launches
+		testHeader(t, r, "Accept", mediaTypeRequiredApprovingReviewsPreview)
+		fmt.Fprintf(w, `{
+			"required_status_checks":{
+				"strict":true,
+				"contexts":["continuous-integration"],
+				"checks": [
+					{
+						"context": "continuous-integration",
+						"app_id": null
+					}
+				]
+			},
+			"required_pull_request_reviews":{
+				"dismissal_restrictions":{
+					"users":[{
+						"id":3,
+						"login":"uu"
+					}],
+					"teams":[{
+						"id":4,
+						"slug":"tt"
+					}]
+				},
+				"dismiss_stale_reviews":true,
+				"require_code_owner_reviews":true
+			},
+			"restrictions":{
+				"users":[{"id":1,"login":"u"}],
+				"teams":[{"id":2,"slug":"t"}],
+				"apps":[{"id":3,"slug":"a"}]
+			}
+		}`)
+	})
+
+	ctx := context.Background()
+	protection, _, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", "b", input)
+	if err != nil {
+		t.Errorf("Repositories.UpdateBranchProtection returned error: %v", err)
+	}
+
+	want := &Protection{
+		RequiredStatusChecks: &RequiredStatusChecks{
+			Strict:   true,
+			Contexts: []string{"continuous-integration"},
+			Checks: []*RequiredStatusCheck{
+				{
+					Context: "continuous-integration",
+				},
+			},
+		},
+		RequiredPullRequestReviews: &PullRequestReviewsEnforcement{
+			DismissStaleReviews: true,
+			DismissalRestrictions: &DismissalRestrictions{
+				Users: []*User{
+					{Login: String("uu"), ID: Int64(3)},
+				},
+				Teams: []*Team{
+					{Slug: String("tt"), ID: Int64(4)},
+				},
+			},
+			RequireCodeOwnerReviews: true,
+		},
+		Restrictions: &BranchRestrictions{
+			Users: []*User{
+				{Login: String("u"), ID: Int64(1)},
+			},
+			Teams: []*Team{
+				{Slug: String("t"), ID: Int64(2)},
+			},
+			Apps: []*App{
+				{Slug: String("a"), ID: Int64(3)},
+			},
+		},
+	}
+	if !cmp.Equal(protection, want) {
+		t.Errorf("Repositories.UpdateBranchProtection returned %+v, want %+v", protection, want)
+	}
+}
+
 func TestRepositoriesService_RemoveBranchProtection(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
@@ -1425,7 +1574,24 @@ func TestRepositoriesService_GetRequiredStatusChecks(t *testing.T) {
 		json.NewDecoder(r.Body).Decode(v)
 
 		testMethod(t, r, "GET")
-		fmt.Fprint(w, `{"strict": true,"contexts": ["x","y","z"]}`)
+		fmt.Fprint(w, `{
+			"strict": true,
+			"contexts": ["x","y","z"],
+			"checks": [
+				{
+					"context": "x",
+					"app_id": null
+				},
+				{
+					"context": "y",
+					"app_id": null
+				},
+				{
+					"context": "z",
+					"app_id": null
+				}
+			]
+		}`)
 	})
 
 	ctx := context.Background()
@@ -1437,6 +1603,17 @@ func TestRepositoriesService_GetRequiredStatusChecks(t *testing.T) {
 	want := &RequiredStatusChecks{
 		Strict:   true,
 		Contexts: []string{"x", "y", "z"},
+		Checks: []*RequiredStatusCheck{
+			{
+				Context: "x",
+			},
+			{
+				Context: "y",
+			},
+			{
+				Context: "z",
+			},
+		},
 	}
 	if !cmp.Equal(checks, want) {
 		t.Errorf("Repositories.GetRequiredStatusChecks returned %+v, want %+v", checks, want)
@@ -1483,7 +1660,7 @@ func TestRepositoriesService_GetRequiredStatusChecks_branchNotProtected(t *testi
 	}
 }
 
-func TestRepositoriesService_UpdateRequiredStatusChecks(t *testing.T) {
+func TestRepositoriesService_UpdateRequiredStatusChecks_Contexts(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
 
@@ -1501,7 +1678,16 @@ func TestRepositoriesService_UpdateRequiredStatusChecks(t *testing.T) {
 			t.Errorf("Request body = %+v, want %+v", v, input)
 		}
 		testHeader(t, r, "Accept", mediaTypeV3)
-		fmt.Fprintf(w, `{"strict":true,"contexts":["continuous-integration"]}`)
+		fmt.Fprintf(w, `{
+			"strict":true,
+			"contexts":["continuous-integration"],
+			"checks": [
+				{
+					"context": "continuous-integration",
+					"app_id": null
+				}
+			]
+		}`)
 	})
 
 	ctx := context.Background()
@@ -1513,6 +1699,11 @@ func TestRepositoriesService_UpdateRequiredStatusChecks(t *testing.T) {
 	want := &RequiredStatusChecks{
 		Strict:   true,
 		Contexts: []string{"continuous-integration"},
+		Checks: []*RequiredStatusCheck{
+			{
+				Context: "continuous-integration",
+			},
+		},
 	}
 	if !cmp.Equal(statusChecks, want) {
 		t.Errorf("Repositories.UpdateRequiredStatusChecks returned %+v, want %+v", statusChecks, want)
@@ -1533,6 +1724,85 @@ func TestRepositoriesService_UpdateRequiredStatusChecks(t *testing.T) {
 	})
 }
 
+func TestRepositoriesService_UpdateRequiredStatusChecks_Checks(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	appID := int64(123)
+	noAppID := int64(-1)
+	input := &RequiredStatusChecksRequest{
+		Strict: Bool(true),
+		Checks: []*RequiredStatusCheck{
+			{
+				Context: "continuous-integration",
+			},
+			{
+				Context: "continuous-integration2",
+				AppID:   &appID,
+			},
+			{
+				Context: "continuous-integration3",
+				AppID:   &noAppID,
+			},
+		},
+	}
+
+	mux.HandleFunc("/repos/o/r/branches/b/protection/required_status_checks", func(w http.ResponseWriter, r *http.Request) {
+		v := new(RequiredStatusChecksRequest)
+		json.NewDecoder(r.Body).Decode(v)
+
+		testMethod(t, r, "PATCH")
+		if !cmp.Equal(v, input) {
+			t.Errorf("Request body = %+v, want %+v", v, input)
+		}
+		testHeader(t, r, "Accept", mediaTypeV3)
+		fmt.Fprintf(w, `{
+			"strict":true,
+			"contexts":["continuous-integration"],
+			"checks": [
+				{
+					"context": "continuous-integration",
+					"app_id": null
+				},
+				{
+					"context": "continuous-integration2",
+					"app_id": 123
+				},
+				{
+					"context": "continuous-integration3",
+					"app_id": null
+				}
+			]
+		}`)
+	})
+
+	ctx := context.Background()
+	statusChecks, _, err := client.Repositories.UpdateRequiredStatusChecks(ctx, "o", "r", "b", input)
+	if err != nil {
+		t.Errorf("Repositories.UpdateRequiredStatusChecks returned error: %v", err)
+	}
+
+	want := &RequiredStatusChecks{
+		Strict:   true,
+		Contexts: []string{"continuous-integration"},
+		Checks: []*RequiredStatusCheck{
+			{
+				Context: "continuous-integration",
+			},
+			{
+				Context: "continuous-integration2",
+				AppID:   &appID,
+			},
+			{
+				Context: "continuous-integration3",
+			},
+		},
+	}
+	if !cmp.Equal(statusChecks, want) {
+		t.Errorf("Repositories.UpdateRequiredStatusChecks returned %+v, want %+v", statusChecks, want)
+	}
+}
+
 func TestRepositoriesService_RemoveRequiredStatusChecks(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()