diff --git a/runbot_merge/models/batch.py b/runbot_merge/models/batch.py index c7b82b30..cb378940 100644 --- a/runbot_merge/models/batch.py +++ b/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, diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 4413857a..f2adaa9d 100644 --- a/runbot_merge/models/pull_requests.py +++ b/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("status", "reviewed_by", 'batch_id.merge_date', "closed", "error") + @api.depends( + "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 - # and override all statuses + # mark current user as reviewer vals.setdefault('reviewed_by', self.env.user.partner_id.id) - # override all known statuses just for safety - vals.setdefault('overrides', json.dumps({ - st.context: { - '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 p in self.batch_id.prs - self: + if not p.reviewed_by: + p.reviewed_by = self.env.user.partner_id.id for pr in self: if (t := vals.get('target')) is not None and pr.target.id != t: diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index d7653513..77fb3859 100644 --- a/runbot_merge/tests/test_basic.py +++ b/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 diff --git a/runbot_merge/tests/test_oddities.py b/runbot_merge/tests/test_oddities.py index 43448656..b5d9d5b2 100644 --- a/runbot_merge/tests/test_oddities.py +++ b/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