diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 0961ede2b7..3015368422 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -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 .*', + r'.*Reviewed-by: derp .*', 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 .*', + r'.*Reviewed-by: derp .*', re.DOTALL)) self.assertThat(C.merge_message, MatchesRegex( - r'.*Reviewed-by: herp .*', + r'.*Reviewed-by: herp .*', re.DOTALL)) @simple_layout('layouts/dependent-github.yaml', driver='github') diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index ebcf04c327..11e3de8b87 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -1617,13 +1617,11 @@ class GithubConnection(BaseConnection): reviews = {} for rev in revs: - login = rev.get('user').get('login') - user = self.getUser(login, project) - + user = rev.get('user').get('login') review = { 'by': { - 'username': user.get('username'), - 'email': user.get('email'), + 'username': user, + 'email': rev.get('user').get('email'), }, 'grantedOn': int(time.mktime(self._ghTimestampToDate( rev.get('submitted_at')))), @@ -1635,19 +1633,19 @@ class GithubConnection(BaseConnection): # Get user's rights. A user always has read to leave a review review['permission'] = 'read' - if login in permissions: - permission = permissions[login] + if user in permissions: + permission = permissions[user] else: - permission = self.getRepoPermission(project.name, login) - permissions[login] = permission + permission = self.getRepoPermission(project.name, user) + permissions[user] = permission if permission == 'write': review['permission'] = 'write' if permission == 'admin': review['permission'] = 'admin' - if login not in reviews: - reviews[login] = review + if user not in reviews: + reviews[user] = review else: # if there are multiple reviews per user, keep the newest # note that this breaks the ability to set the 'older-than' @@ -1656,16 +1654,15 @@ 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[login]['grantedOn']: - if (review['type'] == 'commented' and - reviews[login]['type'] in - ('approved', 'changes_requested')): + if review['grantedOn'] > reviews[user]['grantedOn']: + if (review['type'] == 'commented' and reviews[user]['type'] + in ('approved', 'changes_requested')): log.debug("Discarding comment review %s due to " "an existing vote %s" % (review, - reviews[login])) + reviews[user])) pass else: - reviews[login] = review + reviews[user] = review return reviews.values()