From ea9203c359f8bdbcceaca1336390ee48d37cc925 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 26 Mar 2018 17:29:49 +0200 Subject: [PATCH] [IMP] allow delegate reviewers to retry and r- --- runbot_merge/models/pull_requests.py | 4 ++-- runbot_merge/tests/test_basic.py | 12 ++++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index e511b956..e07f3f02 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -396,9 +396,9 @@ class PullRequests(models.Model): is_admin = (author.reviewer and self.author != author) or (author.self_reviewer and self.author == author) is_reviewer = is_admin or self in author.delegate_reviewer # TODO: should delegate reviewers be able to retry PRs? - is_author = is_admin or self.author == author + is_author = is_reviewer or self.author == author - if not (is_author or is_reviewer or is_admin): + if not is_author: # no point even parsing commands _logger.info("ignoring comment of %s (%s): no ACL to %s:%s", author.github_login, author.display_name, diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index b2826e76..5fb18fb6 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -18,6 +18,10 @@ def repo(gh, env): 'github_login': 'self-reviewer', 'self_reviewer': True, }) + env['res.partner'].create({ + 'name': "Other", + 'github_login': 'other', + }) env['runbot_merge.project'].create({ 'name': 'odoo', 'github_token': 'okokok', @@ -369,7 +373,7 @@ class TestRetry: env['runbot_merge.project']._check_progress() assert pr.state == 'merged' - @pytest.mark.parametrize('retrier', ['user', 'reviewer']) + @pytest.mark.parametrize('retrier', ['user', 'other', 'reviewer']) def test_retry_comment(self, env, repo, retrier): """ An accepted but failed PR should be re-tried when the author or a reviewer asks for it @@ -382,7 +386,7 @@ class TestRetry: prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user') repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'legal/cla') - prx.post_comment('hansen r+', "reviewer") + prx.post_comment('hansen r+ delegate=other', "reviewer") env['runbot_merge.project']._check_progress() assert env['runbot_merge.pull_requests'].search([ ('repository.name', '=', 'odoo/odoo'), @@ -415,7 +419,7 @@ class TestRetry: ('number', '=', prx.number) ]).state == 'merged' - @pytest.mark.parametrize('disabler', ['user', 'reviewer']) + @pytest.mark.parametrize('disabler', ['user', 'other', 'reviewer']) def test_retry_disable(self, env, repo, disabler): m = repo.make_commit(None, 'initial', None, tree={'m': 'm'}) repo.make_ref('heads/master', m) @@ -425,7 +429,7 @@ class TestRetry: prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user') repo.post_status(prx.head, 'success', 'ci/runbot') repo.post_status(prx.head, 'success', 'legal/cla') - prx.post_comment('hansen r+', "reviewer") + prx.post_comment('hansen r+ delegate=other', "reviewer") env['runbot_merge.project']._check_progress() assert env['runbot_merge.pull_requests'].search([ ('repository.name', '=', 'odoo/odoo'),