From e910b8e85700ba740140dac2a48baf92f2272868 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 23 Jan 2024 15:57:41 +0100 Subject: [PATCH] [IMP] runbot_merge: move cross-pr properties to batch --- runbot_merge/models/batch.py | 15 +++++++++++- runbot_merge/models/pull_requests.py | 33 ++++++++++---------------- runbot_merge/models/stagings_create.py | 11 +++------ runbot_merge/tests/test_basic.py | 2 +- 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/runbot_merge/models/batch.py b/runbot_merge/models/batch.py index aae71763..99977497 100644 --- a/runbot_merge/models/batch.py +++ b/runbot_merge/models/batch.py @@ -10,7 +10,9 @@ class Batch(models.Model): repositories e.g. change an API in repo1, this breaks use of that API in repo2 which now needs to be updated. """ - _name = _description = 'runbot_merge.batch' + _name = 'runbot_merge.batch' + _description = "batch of pull request" + _inherit = ['mail.thread'] target = fields.Many2one('runbot_merge.branch', required=True, index=True) staging_ids = fields.Many2many('runbot_merge.stagings') @@ -19,3 +21,14 @@ class Batch(models.Model): prs = fields.One2many('runbot_merge.pull_requests', 'batch_id') active = fields.Boolean(default=True) + + skipchecks = fields.Boolean( + string="Skips Checks", + default=False, tracking=True, + help="Forces entire batch to be ready, skips validation and approval", + ) + cancel_staging = fields.Boolean( + string="Cancels Stagings", + default=False, tracking=True, + help="Cancels current staging on target branch when becoming ready" + ) diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index ed61b6ef..b56835d6 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -327,16 +327,8 @@ class PullRequests(models.Model): closed = fields.Boolean(default=False, tracking=True) error = fields.Boolean(string="in error", default=False, tracking=True) - skipchecks = fields.Boolean( - string="Skips Checks", - default=False, tracking=True, - help="Forces entire batch to be ready, skips validation and approval", - ) - cancel_staging = fields.Boolean( - string="Cancels Stagings", - default=False, tracking=True, - help="Cancels current staging on target branch when becoming ready" - ) + skipchecks = fields.Boolean(related='batch_id.skipchecks') + cancel_staging = fields.Boolean(related='batch_id.cancel_staging') merge_date = fields.Datetime(tracking=True) state = fields.Selection([ @@ -532,7 +524,7 @@ class PullRequests(models.Model): 'batch_id.prs.squash', 'batch_id.prs.merge_method', 'batch_id.prs.state', - 'batch_id.prs.skipchecks', + 'batch_id.skipchecks', ) def _compute_is_blocked(self): self.blocked = False @@ -540,7 +532,7 @@ class PullRequests(models.Model): lambda p: not p.draft, lambda p: p.squash or p.merge_method, lambda p: p.state == 'ready' \ - or any(p.batch_id.prs.mapped('skipchecks')) \ + or p.batch_id.skipchecks \ and all(pr.state != 'error' for pr in p.batch_id.prs) ) messages = ('is in draft', 'has no merge method', 'is not ready') @@ -712,20 +704,19 @@ class PullRequests(models.Model): else: msg = self._approve(author, login) case commands.Reject() if is_author: - batch = self.batch_id.prs - if cancellers := batch.filtered('cancel_staging'): - cancellers.cancel_staging = False - if (skippers := batch.filtered('skipchecks')) or self.reviewed_by: + if cancellers := self.batch_id.cancel_staging: + self.batch_id.cancel_staging = False + if self.batch_id.skipchecks or self.reviewed_by: if self.error: self.error = False if self.reviewed_by: self.reviewed_by = False - if skippers: - skippers.skipchecks = False + if self.batch_id.skipchecks: + self.batch_id.skipchecks = False self.env.ref("runbot_merge.command.unapprove.p0")._send( repository=self.repository, pull_request=self.number, - format_args={'user': login, 'pr': skippers[:1]}, + format_args={'user': login, 'pr': self.batch_id.prs[:1]}, ) self.unstage("unreviewed (r-) by %s", login) else: @@ -762,13 +753,13 @@ class PullRequests(models.Model): case commands.Priority() if is_admin: self.priority = str(command) case commands.SkipChecks() if is_admin: - self.skipchecks = True + self.batch_id.skipchecks = True self.reviewed_by = author for p in self.batch_id.prs - self: if not p.reviewed_by: p.reviewed_by = author case commands.CancelStaging() if is_admin: - self.cancel_staging = True + self.batch_id.cancel_staging = True # FIXME: remove this when skipchecks properly affects state, # maybe: staging cancellation should then only occur # when a cancel_staging PR transitions to ready, or diff --git a/runbot_merge/models/stagings_create.py b/runbot_merge/models/stagings_create.py index f352176e..f7f6f971 100644 --- a/runbot_merge/models/stagings_create.py +++ b/runbot_merge/models/stagings_create.py @@ -223,21 +223,16 @@ def ready_prs(for_branch: Branch) -> List[Tuple[int, PullRequests]]: max(pr.priority) as priority, array_agg(pr.id) AS match FROM runbot_merge_pull_requests pr + JOIN runbot_merge_batch b ON (b.id = pr.batch_id) WHERE pr.target = any(%s) -- exclude terminal states (so there's no issue when -- deleting branches & reusing labels) AND pr.state != 'merged' AND pr.state != 'closed' - GROUP BY - pr.target, - CASE - WHEN pr.label SIMILAR TO '%%:patch-[[:digit:]]+' - THEN pr.id::text - ELSE pr.label - END + GROUP BY b.id HAVING bool_and(pr.state = 'ready') - OR (bool_or(pr.skipchecks) AND bool_and(pr.state != 'error')) + OR (bool_or(b.skipchecks) AND bool_and(pr.state != 'error')) ORDER BY max(pr.priority) DESC, min(pr.id) """, [for_branch.ids]) browse = env['runbot_merge.pull_requests'].browse diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index cc4ff9f9..2d0bc440 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -2875,7 +2875,7 @@ class TestBatching(object): assert p_01.skipchecks == False assert p_01.cancel_staging == False - p_01.cancel_staging = True + p_01.batch_id.cancel_staging = True # FIXME: cancel_staging should only cancel when the PR is or # transitions to ready # assert staging_4.active