diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index d607b4bd..a42c21b7 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -535,16 +535,20 @@ class PullRequests(models.Model): self.state = 'ready' elif command == 'review': if param and is_reviewer: - if self.state == 'opened': + newstate = RPLUS.get(self.state) + if newstate: + self.state = newstate ok = True - self.state = 'approved' - elif self.state == 'validated': + elif not param and is_author: + newstate = RMINUS.get(self.state) + if newstate: + self.state = newstate + if self.staging_id: + self.staging_id.cancel( + "unreview (r-) by %s", + author.github_login + ) ok = True - self.state = 'ready' - elif not param and is_author and self.state == 'error': - # TODO: r- on something which isn't in error? - ok = True - self.state = 'validated' elif command == 'delegate': if is_reviewer: ok = True @@ -665,6 +669,16 @@ class PullRequests(models.Model): }) return super().unlink() +# state changes on reviews +RPLUS = { + 'opened': 'approved', + 'validated': 'ready', +} +RMINUS = { + 'approved': 'opened', + 'ready': 'validated', + 'error': 'validated', +} _TAGS = { False: set(), diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index 61f1e5ee..a1740260 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -1758,6 +1758,116 @@ class TestUnknownPR: env['runbot_merge.project']._check_progress() assert pr.staging_id +class TestRMinus: + def test_rminus_approved(self, repo, env): + """ approved -> r- -> opened + """ + m = repo.make_commit(None, 'initial', None, tree={'m': 'm'}) + repo.make_ref('heads/master', m) + + c = repo.make_commit(m, 'first', None, tree={'m': 'c'}) + prx = repo.make_pr('title', None, target='master', ctid=c, user='user') + + pr = env['runbot_merge.pull_requests'].search([ + ('repository.name', '=', repo.name), + ('number', '=', prx.number), + ]) + assert pr.state == 'opened' + + prx.post_comment('hansen r+', 'reviewer') + assert pr.state == 'approved' + + prx.post_comment('hansen r-', 'user') + assert pr.state == 'opened' + prx.post_comment('hansen r+', 'reviewer') + assert pr.state == 'approved' + + prx.post_comment('hansen r-', 'other') + assert pr.state == 'approved' + + prx.post_comment('hansen r-', 'reviewer') + assert pr.state == 'opened' + + def test_rminus_ready(self, repo, env): + """ ready -> r- -> validated + """ + m = repo.make_commit(None, 'initial', None, tree={'m': 'm'}) + repo.make_ref('heads/master', m) + + c = repo.make_commit(m, 'first', None, tree={'m': 'c'}) + prx = repo.make_pr('title', None, target='master', ctid=c, user='user') + repo.post_status(prx.head, 'success', 'ci/runbot') + repo.post_status(prx.head, 'success', 'legal/cla') + + pr = env['runbot_merge.pull_requests'].search([ + ('repository.name', '=', repo.name), + ('number', '=', prx.number), + ]) + assert pr.state == 'validated' + + prx.post_comment('hansen r+', 'reviewer') + assert pr.state == 'ready' + + prx.post_comment('hansen r-', 'user') + assert pr.state == 'validated' + prx.post_comment('hansen r+', 'reviewer') + assert pr.state == 'ready' + + prx.post_comment('hansen r-', 'other') + assert pr.state == 'ready' + + prx.post_comment('hansen r-', 'reviewer') + assert pr.state == 'validated' + + def test_rminus_staged(self, repo, env): + """ staged -> r- -> validated + """ + m = repo.make_commit(None, 'initial', None, tree={'m': 'm'}) + repo.make_ref('heads/master', m) + + c = repo.make_commit(m, 'first', None, tree={'m': 'c'}) + prx = repo.make_pr('title', None, target='master', ctid=c, user='user') + repo.post_status(prx.head, 'success', 'ci/runbot') + repo.post_status(prx.head, 'success', 'legal/cla') + + pr = env['runbot_merge.pull_requests'].search([ + ('repository.name', '=', repo.name), + ('number', '=', prx.number), + ]) + + # if reviewer unreviews, cancel staging & unreview + prx.post_comment('hansen r+', 'reviewer') + env['runbot_merge.project']._check_progress() + st = pr.staging_id + assert st + + prx.post_comment('hansen r-', 'reviewer') + assert not st.active + assert not pr.staging_id + assert pr.state == 'validated' + + # if author unreviews, cancel staging & unreview + prx.post_comment('hansen r+', 'reviewer') + env['runbot_merge.project']._check_progress() + st = pr.staging_id + assert st + + prx.post_comment('hansen r-', 'user') + assert not st.active + assert not pr.staging_id + assert pr.state == 'validated' + + # if rando unreviews, ignore + prx.post_comment('hansen r+', 'reviewer') + env['runbot_merge.project']._check_progress() + st = pr.staging_id + assert st + + prx.post_comment('hansen r-', 'other') + assert pr.staging_id == st + assert pr.state == 'ready' + + class TestComments: def test_address_method(self, repo, env): m = repo.make_commit(None, 'initial', None, tree={'m': 'm'})