[IMP] runbot_merge: make skipchecks impact PR state

It's a bit weird and inconsistent to have a PR being staged while
unreviewed or unapproved or w/e. If we compute the state based on
skipchecks then everything is consistent.

Also remove the implicit override of all statuses when explicitly
marking the pr as `ready`, it risks creating difficult to understand
states, and it's unnecessary since `skipchecks` gets set.

Also as with setting skipchecks, sets the current user as reviewer on
all PRs of the batch without a reviewer.

runbot_merge/models/batch.py

@@ -28,6 +28,8 @@ class Batch(models.Model):
     ], required=True, default="default", string="Forward Port Policy")
 
     merge_date = fields.Datetime(tracking=True)
+    # having skipchecks skip both validation *and approval* makes sense because
+    # it's batch-wise, having to approve individual PRs is annoying
     skipchecks = fields.Boolean(
         string="Skips Checks",
         default=False, tracking=True,

runbot_merge/models/pull_requests.py

@@ -929,8 +929,11 @@ class PullRequests(models.Model):
                     st = 'pending'
             pr.status = st
 
-    # closed, merged, error should be exclusive, so this should probably be a selection
+    @api.depends(
-    @api.depends("status", "reviewed_by", 'batch_id.merge_date', "closed", "error")
+        "status", "reviewed_by", "closed", "error" ,
+        "batch_id.merge_date",
+        "batch_id.skipchecks",
+    )
     def _compute_state(self):
         for pr in self:
             if pr.batch_id.merge_date:
@@ -939,6 +942,8 @@ class PullRequests(models.Model):
                 pr.state = "closed"
             elif pr.error:
                 pr.state = "error"
+            elif pr.batch_id.skipchecks: # skipchecks behaves as both approval and status override
+                pr.state = "ready"
             else:
                 states = ("opened", "approved", "validated", "ready")
                 pr.state = states[bool(pr.reviewed_by) | ((pr.status == "success") << 1)]
@@ -1079,22 +1084,13 @@ class PullRequests(models.Model):
 
         # when explicitly marking a PR as ready
         if vals.get('state') == 'ready':
-            # skip checks anyway
+            # skip validation
             self.batch_id.skipchecks = True
-            # if the state is forced to ready, set current user as reviewer
+            # mark current user as reviewer
-            # and override all statuses
             vals.setdefault('reviewed_by', self.env.user.partner_id.id)
-            # override all known statuses just for safety
+            for p in self.batch_id.prs - self:
-            vals.setdefault('overrides', json.dumps({
+                if not p.reviewed_by:
-                st.context: {
+                    p.reviewed_by = self.env.user.partner_id.id
-                    'state': 'success',
-                    'target_url': None,
-                    'description': f"Forced by @{self.env.user.partner_id.github_login}",
-                }
-                for st in self.env['runbot_merge.repository.status'].search([
-                    ('prs', '=', True),
-                ])
-            }))
 
         for pr in self:
             if (t := vals.get('target')) is not None and pr.target.id != t:

runbot_merge/tests/test_basic.py

@@ -2810,7 +2810,7 @@ class TestBatching(object):
             pr01 = self._pr(repo, 'Urgent1', [{'n': 'n'}, {'o': 'o'}], user=config['role_user']['token'], reviewer=None, statuses=[])
             pr01.post_comment('hansen NOW! rebase-merge', config['role_reviewer']['token'])
         p_01 = to_pr(env, pr01)
-        assert p_01.state == 'approved'
+        assert p_01.state == 'ready'
         assert p_01.priority == 'alone'
         assert p_01.skipchecks == True
 
@@ -2862,7 +2862,7 @@ class TestBatching(object):
             )
         env.run_crons()
         assert not staging_3.active
-        assert p_01.state == 'opened'
+        assert p_01.state == 'ready'
         assert p_01.priority == 'alone'
         assert p_01.skipchecks == True
         assert p_01.staging_id.active

runbot_merge/tests/test_oddities.py

@@ -273,6 +273,6 @@ def test_force_ready(env, make_repo, project, setreviewers, config):
     pr_id.state = 'ready'
 
     assert pr_id.state == 'ready'
+    assert pr_id.status == 'pending'
     reviewer = env['res.users'].browse([env._uid]).partner_id
     assert pr_id.reviewed_by == reviewer
-    assert pr_id.overrides