zuul
/
zuul
Code Issues Proposed changes

GitHub Reporter: Fix User Email in Merge Commit Message 

GitHub does not provide a review users email address via the
API pull request resources.
(cf. https://developer.github.com/v3/pulls/reviews/#list-reviews-for-a-pull-request)
Therefore, in its current implementation, the GitHub Reporter
always includes ``None`` as an email address in the ``Reviewed-by``
clause(s) of a merge commit message.

An email address of a GitHub user must be retrieved via the
user resource as done as per this change, so the GitHub Reporter
will add correct email addresses of reviewers.

Please note that if a user decides not to set a publicly visible
email address, the GibHub API will not include an email field for
the user object, and thus the ``Reviewed-by`` clause will still
state ``None`` as a users email in these cases.

Change-Id: I43e1fbe54e85c4daee18aae1eaf30bc00356d58c
This commit is contained in:
Benjamin Schanzel 2020-06-30 09:54:32 +02:00
parent cb5f4007a6
commit 4c05056e04
2 changed files with 20 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
tests/unit/test_github_driver.py
View File

@ -691,7 +691,7 @@ class TestGithubDriver(ZuulTestCase):
# assert that single 'Reviewed-By' is in merge commit message
self.assertThat(B.merge_message,
MatchesRegex(
r'.*Reviewed-by: derp <derp@derp.com>.*',
r'.*Reviewed-by: derp <github.user@example.com>.*',
re.DOTALL))
C = self.fake_github.openFakePullRequest('org/project', 'master', 'C')
@ -704,11 +704,11 @@ class TestGithubDriver(ZuulTestCase):
# assert that multiple 'Reviewed-By's are in merge commit message
self.assertThat(C.merge_message,
MatchesRegex(
r'.*Reviewed-by: derp <derp@derp.com>.*',
r'.*Reviewed-by: derp <github.user@example.com>.*',
re.DOTALL))
self.assertThat(C.merge_message,
MatchesRegex(
r'.*Reviewed-by: herp <herp@derp.com>.*',
r'.*Reviewed-by: herp <github.user@example.com>.*',
re.DOTALL))
@simple_layout('layouts/dependent-github.yaml', driver='github')

31
zuul/driver/github/githubconnection.py
View File

@ -1617,11 +1617,13 @@ class GithubConnection(BaseConnection):
reviews = {}
for rev in revs:
user = rev.get('user').get('login')
login = rev.get('user').get('login')
user = self.getUser(login, project)
review = {
'by': {
'username': user,
'email': rev.get('user').get('email'),
'username': user.get('username'),
'email': user.get('email'),
},
'grantedOn': int(time.mktime(self._ghTimestampToDate(
rev.get('submitted_at')))),
@ -1633,19 +1635,19 @@ class GithubConnection(BaseConnection):
# Get user's rights. A user always has read to leave a review
review['permission'] = 'read'
if user in permissions:
permission = permissions[user]
if login in permissions:
permission = permissions[login]
else:
permission = self.getRepoPermission(project.name, user)
permissions[user] = permission
permission = self.getRepoPermission(project.name, login)
permissions[login] = permission
if permission == 'write':
review['permission'] = 'write'
if permission == 'admin':
review['permission'] = 'admin'
if user not in reviews:
reviews[user] = review
if login not in reviews:
reviews[login] = review
else:
# if there are multiple reviews per user, keep the newest
# note that this breaks the ability to set the 'older-than'
@ -1654,15 +1656,16 @@ class GithubConnection(BaseConnection):
# previous review was 'approved' or 'changes_requested', as
# the GitHub model does not change the vote if a comment is
# added after the fact. THANKS GITHUB!
if review['grantedOn'] > reviews[user]['grantedOn']:
if (review['type'] == 'commented' and reviews[user]['type']
in ('approved', 'changes_requested')):
if review['grantedOn'] > reviews[login]['grantedOn']:
if (review['type'] == 'commented' and
reviews[login]['type'] in
('approved', 'changes_requested')):
log.debug("Discarding comment review %s due to "
"an existing vote %s" % (review,
reviews[user]))
reviews[login]))
pass
else:
reviews[user] = review
reviews[login] = review
return reviews.values()