[ADD] runbot_merge: support for r- on approved & ready PRs

Can be used by the PR author or a reviewer. Removes reviewed state of
PR, and cancels the staging if the PR is already staged.

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(),

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'})