From c00eead4fdffd31237265a74211dc2d29a3ff2f6 Mon Sep 17 00:00:00 2001 From: Aleksandar Mirilovic Date: Sat, 1 Apr 2017 06:59:02 +0200 Subject: [PATCH 1/4] Add admin enforcement to protected branches API (#606) --- github/github-accessors.go | 8 ++++++++ github/repos.go | 8 ++++++++ github/repos_test.go | 6 +++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index 8723cdf7fe2..8c6e8ba48d6 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -20,6 +20,14 @@ func (a *AbuseRateLimitError) GetRetryAfter() time.Duration { return *a.RetryAfter } +// GetURL returns the URL field if it's non-nil, zero value otherwise. +func (a *AdminEnforcement) GetURL() string { + if a == nil || a.URL == nil { + return "" + } + return *a.URL +} + // GetVerifiablePasswordAuthentication returns the VerifiablePasswordAuthentication field if it's non-nil, zero value otherwise. func (a *APIMeta) GetVerifiablePasswordAuthentication() bool { if a == nil || a.VerifiablePasswordAuthentication == nil { diff --git a/github/repos.go b/github/repos.go index 22dc42de954..2f82765a2ed 100644 --- a/github/repos.go +++ b/github/repos.go @@ -511,6 +511,7 @@ type Branch struct { type Protection struct { RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` RequiredPullRequestReviews *RequiredPullRequestReviews `json:"required_pull_request_reviews"` + EnforceAdmins *AdminEnforcement `json:"enforce_admins"` Restrictions *BranchRestrictions `json:"restrictions"` } @@ -518,6 +519,7 @@ type Protection struct { type ProtectionRequest struct { RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` RequiredPullRequestReviews *RequiredPullRequestReviews `json:"required_pull_request_reviews"` + EnforceAdmins bool `json:"enforce_admins"` Restrictions *BranchRestrictionsRequest `json:"restrictions"` } @@ -538,6 +540,12 @@ type RequiredPullRequestReviews struct { IncludeAdmins bool `json:"include_admins"` } +// AdminEnforcement represents the configuration to enforce required status checks for repository administrators. +type AdminEnforcement struct { + URL *string `json:"url,omitempty"` + Enabled bool `json:"enabled"` +} + // BranchRestrictions represents the restriction that only certain users or // teams may push to a branch. type BranchRestrictions struct { diff --git a/github/repos_test.go b/github/repos_test.go index 3a98b4fe48e..7995dca1f25 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -482,7 +482,7 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { testMethod(t, r, "GET") testHeader(t, r, "Accept", mediaTypeProtectedBranchesPreview) - fmt.Fprintf(w, `{"required_status_checks":{"include_admins":true,"strict":true,"contexts":["continuous-integration"]},"required_pull_request_reviews":{"include_admins":true},"restrictions":{"users":[{"id":1,"login":"u"}],"teams":[{"id":2,"slug":"t"}]}}`) + fmt.Fprintf(w, `{"required_status_checks":{"include_admins":true,"strict":true,"contexts":["continuous-integration"]},"required_pull_request_reviews":{"include_admins":true},"enforce_admins":{"url":"/repos/o/r/branches/b/protection/enforce_admins","enabled":true},"restrictions":{"users":[{"id":1,"login":"u"}],"teams":[{"id":2,"slug":"t"}]}}`) }) protection, _, err := client.Repositories.GetBranchProtection(context.Background(), "o", "r", "b") @@ -499,6 +499,10 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { RequiredPullRequestReviews: &RequiredPullRequestReviews{ IncludeAdmins: true, }, + EnforceAdmins: &AdminEnforcement{ + URL: String("/repos/o/r/branches/b/protection/enforce_admins"), + Enabled: true, + }, Restrictions: &BranchRestrictions{ Users: []*User{ {Login: String("u"), ID: Int(1)}, From 02a1f4de7b8255935645d39eeeb763fe8335b233 Mon Sep 17 00:00:00 2001 From: Aleksandar Mirilovic Date: Fri, 7 Apr 2017 00:30:07 +0200 Subject: [PATCH 2/4] Remove RequiredPullRequestReviews and deprecate IncludeAdmins in RequiredStatusChecks --- github/repos.go | 21 +++++++-------------- github/repos_test.go | 11 +---------- 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/github/repos.go b/github/repos.go index 2f82765a2ed..1bdb1ac34a9 100644 --- a/github/repos.go +++ b/github/repos.go @@ -509,22 +509,21 @@ type Branch struct { // Protection represents a repository branch's protection. type Protection struct { - RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` - RequiredPullRequestReviews *RequiredPullRequestReviews `json:"required_pull_request_reviews"` - EnforceAdmins *AdminEnforcement `json:"enforce_admins"` - Restrictions *BranchRestrictions `json:"restrictions"` + RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` + EnforceAdmins *AdminEnforcement `json:"enforce_admins"` + Restrictions *BranchRestrictions `json:"restrictions"` } // ProtectionRequest represents a request to create/edit a branch's protection. type ProtectionRequest struct { - RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` - RequiredPullRequestReviews *RequiredPullRequestReviews `json:"required_pull_request_reviews"` - EnforceAdmins bool `json:"enforce_admins"` - Restrictions *BranchRestrictionsRequest `json:"restrictions"` + RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` + EnforceAdmins bool `json:"enforce_admins"` + Restrictions *BranchRestrictionsRequest `json:"restrictions"` } // RequiredStatusChecks represents the protection status of a individual branch. type RequiredStatusChecks struct { + // Deprecated: Use EnforceAdmins instead. // Enforce required status checks for repository administrators. (Required.) IncludeAdmins bool `json:"include_admins"` // Require branches to be up to date before merging. (Required.) @@ -534,12 +533,6 @@ type RequiredStatusChecks struct { Contexts []string `json:"contexts"` } -// RequiredPullRequestReviews represents the protection configuration for pull requests. -type RequiredPullRequestReviews struct { - // Enforce pull request reviews for repository administrators. (Required.) - IncludeAdmins bool `json:"include_admins"` -} - // AdminEnforcement represents the configuration to enforce required status checks for repository administrators. type AdminEnforcement struct { URL *string `json:"url,omitempty"` diff --git a/github/repos_test.go b/github/repos_test.go index 7995dca1f25..754f22a2767 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -482,7 +482,7 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { testMethod(t, r, "GET") testHeader(t, r, "Accept", mediaTypeProtectedBranchesPreview) - fmt.Fprintf(w, `{"required_status_checks":{"include_admins":true,"strict":true,"contexts":["continuous-integration"]},"required_pull_request_reviews":{"include_admins":true},"enforce_admins":{"url":"/repos/o/r/branches/b/protection/enforce_admins","enabled":true},"restrictions":{"users":[{"id":1,"login":"u"}],"teams":[{"id":2,"slug":"t"}]}}`) + fmt.Fprintf(w, `{"required_status_checks":{"include_admins":true,"strict":true,"contexts":["continuous-integration"]},"enforce_admins":{"url":"/repos/o/r/branches/b/protection/enforce_admins","enabled":true},"restrictions":{"users":[{"id":1,"login":"u"}],"teams":[{"id":2,"slug":"t"}]}}`) }) protection, _, err := client.Repositories.GetBranchProtection(context.Background(), "o", "r", "b") @@ -496,9 +496,6 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { Strict: true, Contexts: []string{"continuous-integration"}, }, - RequiredPullRequestReviews: &RequiredPullRequestReviews{ - IncludeAdmins: true, - }, EnforceAdmins: &AdminEnforcement{ URL: String("/repos/o/r/branches/b/protection/enforce_admins"), Enabled: true, @@ -527,9 +524,6 @@ func TestRepositoriesService_UpdateBranchProtection(t *testing.T) { Strict: true, Contexts: []string{"continuous-integration"}, }, - RequiredPullRequestReviews: &RequiredPullRequestReviews{ - IncludeAdmins: true, - }, Restrictions: &BranchRestrictionsRequest{ Users: []string{"u"}, Teams: []string{"t"}, @@ -559,9 +553,6 @@ func TestRepositoriesService_UpdateBranchProtection(t *testing.T) { Strict: true, Contexts: []string{"continuous-integration"}, }, - RequiredPullRequestReviews: &RequiredPullRequestReviews{ - IncludeAdmins: true, - }, Restrictions: &BranchRestrictions{ Users: []*User{ {Login: String("u"), ID: Int(1)}, From 62a5b3ae0e2ab324974b605669cde8ba4089046d Mon Sep 17 00:00:00 2001 From: Aleksandar Mirilovic Date: Sun, 9 Apr 2017 20:03:26 +0200 Subject: [PATCH 3/4] Fix comment order --- github/repos.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/repos.go b/github/repos.go index 1bdb1ac34a9..f05e88b6c83 100644 --- a/github/repos.go +++ b/github/repos.go @@ -523,8 +523,8 @@ type ProtectionRequest struct { // RequiredStatusChecks represents the protection status of a individual branch. type RequiredStatusChecks struct { - // Deprecated: Use EnforceAdmins instead. // Enforce required status checks for repository administrators. (Required.) + // Deprecated: Use EnforceAdmins instead. IncludeAdmins bool `json:"include_admins"` // Require branches to be up to date before merging. (Required.) Strict bool `json:"strict"` From 9237137b985a1df3d0f04e4707a66410e144b80a Mon Sep 17 00:00:00 2001 From: Aleksandar Mirilovic Date: Fri, 14 Apr 2017 01:33:56 +0200 Subject: [PATCH 4/4] Revert "Remove RequiredPullRequestReviews and deprecate IncludeAdmins in RequiredStatusChecks" This reverts commit 02a1f4de7b8255935645d39eeeb763fe8335b233. --- github/repos.go | 21 +++++++++++++++------ github/repos_test.go | 11 ++++++++++- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/github/repos.go b/github/repos.go index f05e88b6c83..e2bfa1034bf 100644 --- a/github/repos.go +++ b/github/repos.go @@ -509,16 +509,18 @@ type Branch struct { // Protection represents a repository branch's protection. type Protection struct { - RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` - EnforceAdmins *AdminEnforcement `json:"enforce_admins"` - Restrictions *BranchRestrictions `json:"restrictions"` + RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` + RequiredPullRequestReviews *RequiredPullRequestReviews `json:"required_pull_request_reviews"` + EnforceAdmins *AdminEnforcement `json:"enforce_admins"` + Restrictions *BranchRestrictions `json:"restrictions"` } // ProtectionRequest represents a request to create/edit a branch's protection. type ProtectionRequest struct { - RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` - EnforceAdmins bool `json:"enforce_admins"` - Restrictions *BranchRestrictionsRequest `json:"restrictions"` + RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"` + RequiredPullRequestReviews *RequiredPullRequestReviews `json:"required_pull_request_reviews"` + EnforceAdmins bool `json:"enforce_admins"` + Restrictions *BranchRestrictionsRequest `json:"restrictions"` } // RequiredStatusChecks represents the protection status of a individual branch. @@ -533,6 +535,13 @@ type RequiredStatusChecks struct { Contexts []string `json:"contexts"` } +// RequiredPullRequestReviews represents the protection configuration for pull requests. +type RequiredPullRequestReviews struct { + // Enforce pull request reviews for repository administrators. (Required.) + // Deprecated: Use EnforceAdmins instead. + IncludeAdmins bool `json:"include_admins"` +} + // AdminEnforcement represents the configuration to enforce required status checks for repository administrators. type AdminEnforcement struct { URL *string `json:"url,omitempty"` diff --git a/github/repos_test.go b/github/repos_test.go index 754f22a2767..7995dca1f25 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -482,7 +482,7 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { testMethod(t, r, "GET") testHeader(t, r, "Accept", mediaTypeProtectedBranchesPreview) - fmt.Fprintf(w, `{"required_status_checks":{"include_admins":true,"strict":true,"contexts":["continuous-integration"]},"enforce_admins":{"url":"/repos/o/r/branches/b/protection/enforce_admins","enabled":true},"restrictions":{"users":[{"id":1,"login":"u"}],"teams":[{"id":2,"slug":"t"}]}}`) + fmt.Fprintf(w, `{"required_status_checks":{"include_admins":true,"strict":true,"contexts":["continuous-integration"]},"required_pull_request_reviews":{"include_admins":true},"enforce_admins":{"url":"/repos/o/r/branches/b/protection/enforce_admins","enabled":true},"restrictions":{"users":[{"id":1,"login":"u"}],"teams":[{"id":2,"slug":"t"}]}}`) }) protection, _, err := client.Repositories.GetBranchProtection(context.Background(), "o", "r", "b") @@ -496,6 +496,9 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { Strict: true, Contexts: []string{"continuous-integration"}, }, + RequiredPullRequestReviews: &RequiredPullRequestReviews{ + IncludeAdmins: true, + }, EnforceAdmins: &AdminEnforcement{ URL: String("/repos/o/r/branches/b/protection/enforce_admins"), Enabled: true, @@ -524,6 +527,9 @@ func TestRepositoriesService_UpdateBranchProtection(t *testing.T) { Strict: true, Contexts: []string{"continuous-integration"}, }, + RequiredPullRequestReviews: &RequiredPullRequestReviews{ + IncludeAdmins: true, + }, Restrictions: &BranchRestrictionsRequest{ Users: []string{"u"}, Teams: []string{"t"}, @@ -553,6 +559,9 @@ func TestRepositoriesService_UpdateBranchProtection(t *testing.T) { Strict: true, Contexts: []string{"continuous-integration"}, }, + RequiredPullRequestReviews: &RequiredPullRequestReviews{ + IncludeAdmins: true, + }, Restrictions: &BranchRestrictions{ Users: []*User{ {Login: String("u"), ID: Int(1)},