Skip to content

Commit

Permalink
Add support for required approving review count (PyGithub#888)
Browse files Browse the repository at this point in the history 
The GitHub API currently has a beta extension to the Branch Protection
API that allows inspection and setting of the number of required
approving reviews required for a PR to be merged. Add support for it.
  • Loading branch information
@s-t-e-v-e-n-k @sfdye
s-t-e-v-e-n-k authored and sfdye committed Aug 28, 2018
1 parent 08fa797 commit ef16702
Show file tree
Hide file tree
Showing 17 changed files with 79 additions and 27 deletions.
23 changes: 18 additions & 5 deletions github/Branch.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import github.RequiredPullRequestReviews
import github.RequiredStatusChecks

import Consts

class Branch(github.GithubObject.NonCompletableGithubObject):
"""
Expand Down Expand Up @@ -99,11 +100,12 @@ def get_protection(self):
"""
headers, data = self._requester.requestJsonAndCheck(
"GET",
self.protection_url
self.protection_url,
headers={'Accept': Consts.mediaTypeRequireMultipleApprovingReviews}
)
return github.BranchProtection.BranchProtection(self._requester, headers, data, completed=True)

def edit_protection(self, strict=github.GithubObject.NotSet, contexts=github.GithubObject.NotSet, enforce_admins=github.GithubObject.NotSet, dismissal_users=github.GithubObject.NotSet, dismissal_teams=github.GithubObject.NotSet, dismiss_stale_reviews=github.GithubObject.NotSet, require_code_owner_reviews=github.GithubObject.NotSet, user_push_restrictions=github.GithubObject.NotSet, team_push_restrictions=github.GithubObject.NotSet):
def edit_protection(self, strict=github.GithubObject.NotSet, contexts=github.GithubObject.NotSet, enforce_admins=github.GithubObject.NotSet, dismissal_users=github.GithubObject.NotSet, dismissal_teams=github.GithubObject.NotSet, dismiss_stale_reviews=github.GithubObject.NotSet, require_code_owner_reviews=github.GithubObject.NotSet, required_approving_review_count=github.GithubObject.NotSet, user_push_restrictions=github.GithubObject.NotSet, team_push_restrictions=github.GithubObject.NotSet):
"""
:calls: `PUT /repos/:owner/:repo/branches/:branch/protection <https://developer.github.com/v3/repos/branches>`_
:strict: bool
Expand All @@ -113,6 +115,7 @@ def edit_protection(self, strict=github.GithubObject.NotSet, contexts=github.Git
:dismissal_teams: list of strings
:dismiss_stale_reviews: bool
:require_code_owner_reviews: bool
:required_approving_review_count: int
:user_push_restrictions: list of strings
:team_push_restrictions: list of strings
Expand All @@ -127,6 +130,7 @@ def edit_protection(self, strict=github.GithubObject.NotSet, contexts=github.Git
assert dismissal_teams is github.GithubObject.NotSet or all(isinstance(element, (str, unicode)) or isinstance(element, (str, unicode)) for element in dismissal_teams), dismissal_teams
assert dismiss_stale_reviews is github.GithubObject.NotSet or isinstance(dismiss_stale_reviews, bool), dismiss_stale_reviews
assert require_code_owner_reviews is github.GithubObject.NotSet or isinstance(require_code_owner_reviews, bool), require_code_owner_reviews
assert required_approving_review_count is github.GithubObject.NotSet or isinstance(required_approving_review_count, int), required_approving_review_count

post_parameters = {}
if strict is not github.GithubObject.NotSet or contexts is not github.GithubObject.NotSet:
Expand All @@ -143,12 +147,14 @@ def edit_protection(self, strict=github.GithubObject.NotSet, contexts=github.Git
else:
post_parameters["enforce_admins"] = None

if dismissal_users is not github.GithubObject.NotSet or dismissal_teams is not github.GithubObject.NotSet or dismiss_stale_reviews is not github.GithubObject.NotSet or require_code_owner_reviews is not github.GithubObject.NotSet:
if dismissal_users is not github.GithubObject.NotSet or dismissal_teams is not github.GithubObject.NotSet or dismiss_stale_reviews is not github.GithubObject.NotSet or require_code_owner_reviews is not github.GithubObject.NotSet or required_approving_review_count is not github.GithubObject.NotSet:
post_parameters["required_pull_request_reviews"] = {}
if dismiss_stale_reviews is not github.GithubObject.NotSet:
post_parameters["required_pull_request_reviews"]["dismiss_stale_reviews"] = dismiss_stale_reviews
if require_code_owner_reviews is not github.GithubObject.NotSet:
post_parameters["required_pull_request_reviews"]["require_code_owner_reviews"] = require_code_owner_reviews
if required_approving_review_count is not github.GithubObject.NotSet:
post_parameters["required_pull_request_reviews"]["required_approving_review_count"] = required_approving_review_count
if dismissal_users is not github.GithubObject.NotSet:
post_parameters["required_pull_request_reviews"]["dismissal_restrictions"] = {"users": dismissal_users}
if dismissal_teams is not github.GithubObject.NotSet:
Expand All @@ -169,6 +175,7 @@ def edit_protection(self, strict=github.GithubObject.NotSet, contexts=github.Git
headers, data = self._requester.requestJsonAndCheck(
"PUT",
self.protection_url,
headers={'Accept': Consts.mediaTypeRequireMultipleApprovingReviews},
input=post_parameters
)

Expand Down Expand Up @@ -228,22 +235,25 @@ def get_required_pull_request_reviews(self):
"""
headers, data = self._requester.requestJsonAndCheck(
"GET",
self.protection_url + "/required_pull_request_reviews"
self.protection_url + "/required_pull_request_reviews",
headers={'Accept': Consts.mediaTypeRequireMultipleApprovingReviews}
)
return github.RequiredPullRequestReviews.RequiredPullRequestReviews(self._requester, headers, data, completed=True)

def edit_required_pull_request_reviews(self, dismissal_users=github.GithubObject.NotSet, dismissal_teams=github.GithubObject.NotSet, dismiss_stale_reviews=github.GithubObject.NotSet, require_code_owner_reviews=github.GithubObject.NotSet):
def edit_required_pull_request_reviews(self, dismissal_users=github.GithubObject.NotSet, dismissal_teams=github.GithubObject.NotSet, dismiss_stale_reviews=github.GithubObject.NotSet, require_code_owner_reviews=github.GithubObject.NotSet, required_approving_review_count=github.GithubObject.NotSet):
"""
:calls: `PATCH /repos/:owner/:repo/branches/:branch/protection/required_pull_request_reviews <https://developer.github.com/v3/repos/branches>`_
:dismissal_users: list of strings
:dismissal_teams: list of strings
:dismiss_stale_reviews: bool
:require_code_owner_reviews: bool
:required_approving_review_count: int
"""
assert dismissal_users is github.GithubObject.NotSet or all(isinstance(element, (str, unicode)) or isinstance(element, (str, unicode)) for element in dismissal_users), dismissal_users
assert dismissal_teams is github.GithubObject.NotSet or all(isinstance(element, (str, unicode)) or isinstance(element, (str, unicode)) for element in dismissal_teams), dismissal_teams
assert dismiss_stale_reviews is github.GithubObject.NotSet or isinstance(dismiss_stale_reviews, bool), dismiss_stale_reviews
assert require_code_owner_reviews is github.GithubObject.NotSet or isinstance(require_code_owner_reviews, bool), require_code_owner_reviews
assert required_approving_review_count is github.GithubObject.NotSet or isinstance(required_approving_review_count, int), required_approving_review_count

post_parameters = {}
if dismissal_users is not github.GithubObject.NotSet:
Expand All @@ -256,9 +266,12 @@ def edit_required_pull_request_reviews(self, dismissal_users=github.GithubObject
post_parameters["dismiss_stale_reviews"] = dismiss_stale_reviews
if require_code_owner_reviews is not github.GithubObject.NotSet:
post_parameters["require_code_owner_reviews"] = require_code_owner_reviews
if required_approving_review_count is not github.GithubObject.NotSet:
post_parameters["required_approving_review_count"] = required_approving_review_count
headers, data = self._requester.requestJsonAndCheck(
"PATCH",
self.protection_url + "/required_pull_request_reviews",
headers={'Accept': Consts.mediaTypeRequireMultipleApprovingReviews},
input=post_parameters
)

Expand Down
3 changes: 3 additions & 0 deletions github/Consts.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,6 @@

# https://developer.github.com/changes/2018-01-10-lock-reason-api-preview/
mediaTypeLockReasonPreview = "application/vnd.github.sailor-v-preview+json"

# https://developer.github.com/changes/2018-03-16-protected-branches-required-approving-reviews/
mediaTypeRequireMultipleApprovingReviews = "application/vnd.github.luke-cage-preview+json"
11 changes: 11 additions & 0 deletions github/RequiredPullRequestReviews.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ def require_code_owner_reviews(self):
self._completeIfNotSet(self._require_code_owner_reviews)
return self._require_code_owner_reviews.value

@property
def required_approving_review_count(self):
"""
:type: int
"""
self._completeIfNotSet(self._required_approving_review_count)
return self._required_approving_review_count.value

@property
def url(self):
"""
Expand Down Expand Up @@ -79,6 +87,7 @@ def dismissal_teams(self):
def _initAttributes(self):
self._dismiss_stale_reviews = github.GithubObject.NotSet
self._require_code_owner_reviews = github.GithubObject.NotSet
self._required_approving_review_count = github.GithubObject.NotSet
self._users = github.GithubObject.NotSet
self._teams = github.GithubObject.NotSet

Expand All @@ -92,5 +101,7 @@ def _useAttributes(self, attributes):
self._dismiss_stale_reviews = self._makeBoolAttribute(attributes["dismiss_stale_reviews"])
if "require_code_owner_reviews" in attributes: # pragma no branch
self._require_code_owner_reviews = self._makeBoolAttribute(attributes["require_code_owner_reviews"])
if "required_approving_review_count" in attributes: # pragma no branch
self._required_approving_review_count = self._makeIntAttribute(attributes["required_approving_review_count"])
if "url" in attributes: # pragma no branch
self._url = self._makeStringAttribute(attributes["url"])
18 changes: 16 additions & 2 deletions github/tests/Branch.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,14 @@ def testAttributes(self):
self.assertEqual(self.branch.__repr__(), 'Branch(name="topic/RewriteWithGeneratedCode")')

def testEditProtection(self):
self.protected_branch.edit_protection(strict=True, require_code_owner_reviews=True)
self.protected_branch.edit_protection(strict=True, require_code_owner_reviews=True, required_approving_review_count=2)
branch_protection = self.protected_branch.get_protection()
self.assertTrue(branch_protection.required_status_checks.strict)
self.assertEqual(branch_protection.required_status_checks.contexts, [])
self.assertTrue(branch_protection.enforce_admins)
self.assertFalse(branch_protection.required_pull_request_reviews.dismiss_stale_reviews)
self.assertTrue(branch_protection.required_pull_request_reviews.require_code_owner_reviews)
self.assertEqual(branch_protection.required_pull_request_reviews.required_approving_review_count, 2)

def testEditProtectionDismissalUsersWithUserOwnedBranch(self):
with self.assertRaises(github.GithubException) as raisedexp:
Expand Down Expand Up @@ -127,10 +128,22 @@ def testRemoveRequiredStatusChecks(self):
)

def testEditRequiredPullRequestReviews(self):
self.protected_branch.edit_required_pull_request_reviews(dismiss_stale_reviews=True)
self.protected_branch.edit_required_pull_request_reviews(dismiss_stale_reviews=True, required_approving_review_count=2)
required_pull_request_reviews = self.protected_branch.get_required_pull_request_reviews()
self.assertTrue(required_pull_request_reviews.dismiss_stale_reviews)
self.assertTrue(required_pull_request_reviews.require_code_owner_reviews)
self.assertEqual(required_pull_request_reviews.required_approving_review_count, 2)

def testEditRequiredPullRequestReviewsWithTooLargeApprovingReviewCount(self):
with self.assertRaises(github.GithubException) as raisedexp:
self.protected_branch.edit_required_pull_request_reviews(required_approving_review_count=9)
self.assertEqual(raisedexp.exception.status, 422)
self.assertEqual(
raisedexp.exception.data, {
u'documentation_url': u'https://developer.github.com/v3/repos/branches/#update-pull-request-review-enforcement-of-protected-branch',
u'message': u'Invalid request.\n\n9 must be less than or equal to 6.'
}
)

def testEditRequiredPullRequestReviewsWithUserBranchAndDismissalUsers(self):
with self.assertRaises(github.GithubException) as raisedexp:
Expand All @@ -148,6 +161,7 @@ def testRemoveRequiredPullRequestReviews(self):
required_pull_request_reviews = self.protected_branch.get_required_pull_request_reviews()
self.assertFalse(required_pull_request_reviews.dismiss_stale_reviews)
self.assertFalse(required_pull_request_reviews.require_code_owner_reviews)
self.assertEqual(required_pull_request_reviews.required_approving_review_count, 1)

def testAdminEnforcement(self):
self.protected_branch.remove_admin_enforcement()
Expand Down
8 changes: 4 additions & 4 deletions github/tests/ReplayData/Branch.testEditProtection.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ PUT
api.github.com
None
/repos/jacquev6/PyGithub/branches/integrations/protection
{'Authorization': 'Basic login_and_password_removed', 'Content-Type': 'application/json', 'User-Agent': 'PyGithub/Python'}
{"restrictions": null, "required_pull_request_reviews": {"require_code_owner_reviews": true}, "required_status_checks": {"contexts": [], "strict": true}, "enforce_admins": null}
{'Authorization': 'Basic login_and_password_removed', 'Content-Type': 'application/json', 'User-Agent': 'PyGithub/Python', 'Accept': 'application/vnd.github.luke-cage-preview+json'}
{"restrictions": null, "required_pull_request_reviews": {"require_code_owner_reviews": true, "required_approving_review_count": 2}, "required_status_checks": {"contexts": [], "strict": true}, "enforce_admins": null}
200
[('status', '200 OK'), ('x-ratelimit-remaining', '4994'), ('content-length', '0'), ('server', 'nginx/1.0.13'), ('connection', 'keep-alive'), ('x-ratelimit-limit', '5000'), ('etag', '"e0dc2dfc56971f4a36de1216356ea98b"'), ('date', 'Sat, 05 May 2018 06:05:54 GMT'), ('content-type', 'application/json; charset=utf-8')]
''
Expand All @@ -14,9 +14,9 @@ GET
api.github.com
None
/repos/jacquev6/PyGithub/branches/integrations/protection
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python', 'Accept': 'application/vnd.github.luke-cage-preview+json'}
None
200
[('status', '200 OK'), ('x-ratelimit-remaining', '4994'), ('content-length', '0'), ('server', 'nginx/1.0.13'), ('connection', 'keep-alive'), ('x-ratelimit-limit', '5000'), ('etag', '"81ba94a48ad867b26d48c023ac584f43"'), ('date', 'Mon, 07 May 2018 13:42:41 GMT'), ('content-type', 'application/json; charset=utf-8')]
{"url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection","required_pull_request_reviews":{"url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection/required_pull_request_reviews","dismiss_stale_reviews":false,"require_code_owner_reviews":true},"required_status_checks":{"url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection/required_status_checks","strict":true,"contexts":[],"contexts_url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection/required_status_checks/contexts"},"enforce_admins":{"url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection/enforce_admins","enabled":true}}
{"url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection","required_pull_request_reviews":{"url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection/required_pull_request_reviews","dismiss_stale_reviews":false,"require_code_owner_reviews":true,"required_approving_review_count":2},"required_status_checks":{"url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection/required_status_checks","strict":true,"contexts":[],"contexts_url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection/required_status_checks/contexts"},"enforce_admins":{"url":"https://api.github.com/repos/jacquev6/PyGithub/branches/integrations/protection/enforce_admins","enabled":true}}

2 changes: 1 addition & 1 deletion github/tests/ReplayData/Branch.testEditProtectionDismissalUsersWithUserOwnedBranch.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ PUT
api.github.com
None
/repos/jacquev6/PyGithub/branches/integrations/protection
{'Authorization': 'Basic login_and_password_removed', 'Content-Type': 'application/json', 'User-Agent': 'PyGithub/Python'}
{'Authorization': 'Basic login_and_password_removed', 'Content-Type': 'application/json', 'User-Agent': 'PyGithub/Python', 'Accept': 'application/vnd.github.luke-cage-preview+json'}
{"restrictions": null, "required_pull_request_reviews": {"dismissal_restrictions": {"users": ["jacquev6"]}}, "required_status_checks": null, "enforce_admins": null}
422
[('status', '422 Unprocessable Entity'), ('x-ratelimit-remaining', '4994'), ('content-length', '0'), ('server', 'nginx/1.0.13'), ('connection', 'keep-alive'), ('x-ratelimit-limit', '5000'), ('etag', '"e0dc2dfc56971f4a36de1216356ea98b"'), ('date', 'Sun, 13 May 2018 10:42:14 GMT'), ('content-type', 'application/json; charset=utf-8')]
Expand Down
4 changes: 2 additions & 2 deletions github/tests/ReplayData/Branch.testEditProtectionPushRestrictionsAndDismissalUser.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ PUT
api.github.com
None
/repos/PyGithub/PyGithub/branches/master/protection
{'Authorization': 'Basic login_and_password_removed', 'Content-Type': 'application/json', 'User-Agent': 'PyGithub/Python'}
{'Authorization': 'Basic login_and_password_removed', 'Content-Type': 'application/json', 'User-Agent': 'PyGithub/Python', 'Accept': 'application/vnd.github.luke-cage-preview+json'}
{"restrictions": {"teams": [], "users": ["jacquev6"]}, "required_pull_request_reviews": {"dismissal_restrictions": {"users": ["jacquev6"]}}, "required_status_checks": null, "enforce_admins": null}
200
[('status', '200 OK'), ('x-ratelimit-remaining', '4994'), ('content-length', '0'), ('server', 'nginx/1.0.13'), ('connection', 'keep-alive'), ('x-ratelimit-limit', '5000'), ('etag', '"e0dc2dfc56971f4a36de1216356ea98b"'), ('date', 'Sun, 13 May 2018 13:21:24 GMT'), ('content-type', 'application/json; charset=utf-8')]
Expand All @@ -14,7 +14,7 @@ GET
api.github.com
None
/repos/PyGithub/PyGithub/branches/master/protection
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python', 'Accept': 'application/vnd.github.luke-cage-preview+json'}
None
200
[('x-runtime-rack', '0.049160'), ('vary', 'Accept, Authorization, Cookie, X-GitHub-OTP, Accept-Encoding'), ('x-oauth-scopes', 'public_repo, repo:status'), ('x-xss-protection', '1; mode=block'), ('x-content-type-options', 'nosniff'), ('x-accepted-oauth-scopes', ''), ('etag', 'W/"a4ef2f2bcfdf33fadd5c8fa6867ebc3a"'), ('cache-control', 'private, max-age=60, s-maxage=60'), ('referrer-policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('status', '200 OK'), ('x-ratelimit-remaining', '4986'), ('x-github-media-type', 'github.v3; format=json'), ('access-control-expose-headers', 'ETag, Link, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval'), ('transfer-encoding', 'chunked'), ('x-github-request-id', '860C:2DC5:28789D:34E3E8:5AF7FF18'), ('date', 'Sun, 13 May 2018 09:02:17 GMT'), ('access-control-allow-origin', '*'), ('content-security-policy', "default-src 'none'"), ('content-encoding', 'gzip'), ('strict-transport-security', 'max-age=31536000; includeSubdomains; preload'), ('server', 'GitHub.com'), ('x-ratelimit-limit', '5000'), ('x-frame-options', 'deny'), ('content-type', 'application/json; charset=utf-8'), ('x-ratelimit-reset', '1526204223')]
Expand Down
Loading

0 comments on commit ef16702

Please sign in to comment.