Skip to content
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

Add support for required approving review count #888

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 18 additions & 5 deletions github/Branch.py
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
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
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
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
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}}

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