From de70bd6f8371a23652f6b2317165c1720b9469c5 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 8 Feb 2022 11:06:34 +0100 Subject: [PATCH] [IMP] runbot_merge: show PR titles in freeze wizard Currently limited to release/freeze PRs: it can be difficult to be sure the right PR was selected then, and a mistake there seems more impactful than in the PRs being waited for? Note: adds a test to make sure I don't break the check that all release PRs must have the same label (be linked). This was already safe, and in a way this PR adds convenience but not really safety, but better sure than sorry. --- runbot_merge/models/project_freeze/__init__.py | 2 +- runbot_merge/models/project_freeze/views.xml | 3 ++- runbot_merge/models/pull_requests.py | 18 +++++++++++++----- runbot_merge/tests/test_multirepo.py | 14 ++++++++++---- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/runbot_merge/models/project_freeze/__init__.py b/runbot_merge/models/project_freeze/__init__.py index 39941d28..07c26d35 100644 --- a/runbot_merge/models/project_freeze/__init__.py +++ b/runbot_merge/models/project_freeze/__init__.py @@ -41,7 +41,7 @@ class FreezeWizard(models.Model): labels = set(self.mapped('release_pr_ids.pr_id.label')) if len(labels) != 1: - errors.append("* All release PRs must have the same label, found %r." % ', '.join(labels)) + errors.append("* All release PRs must have the same label, found %r." % ', '.join(sorted(labels))) unready = sum(p.state not in ('closed', 'merged') for p in self.required_pr_ids) if unready: diff --git a/runbot_merge/models/project_freeze/views.xml b/runbot_merge/models/project_freeze/views.xml index 87b1baca..2e6ba031 100644 --- a/runbot_merge/models/project_freeze/views.xml +++ b/runbot_merge/models/project_freeze/views.xml @@ -27,7 +27,8 @@ - + diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index c0eae29d..c003d85d 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -531,6 +531,9 @@ class PullRequests(models.Model): url = fields.Char(compute='_compute_url') github_url = fields.Char(compute='_compute_url') + repo_name = fields.Char(related='repository.name') + message_title = fields.Char(compute='_compute_message_title') + @api.depends('repository.name', 'number') def _compute_url(self): base = werkzeug.urls.url_parse(self.env['ir.config_parameter'].sudo().get_param('web.base.url', 'http://localhost:8069')) @@ -540,15 +543,20 @@ class PullRequests(models.Model): pr.url = str(base.join(path)) pr.github_url = str(gh_base.join(path)) - @api.depends('repository.name', 'number') + @api.depends('message') + def _compute_message_title(self): + for pr in self: + pr.message_title = next(iter(pr.message.splitlines()), '') + + @api.depends('repository.name', 'number', 'message') def _compute_display_name(self): return super(PullRequests, self)._compute_display_name() def name_get(self): - return [ - (p.id, '%s#%d' % (p.repository.name, p.number)) - for p in self - ] + name_template = '%(repo_name)s#%(number)d' + if self.env.context.get('pr_include_title'): + name_template += ' (%(message_title)s)' + return [(p.id, name_template % p) for p in self] @api.model def name_search(self, name='', args=None, operator='ilike', limit=100): diff --git a/runbot_merge/tests/test_multirepo.py b/runbot_merge/tests/test_multirepo.py index 7b71d2f6..a51d99b7 100644 --- a/runbot_merge/tests/test_multirepo.py +++ b/runbot_merge/tests/test_multirepo.py @@ -1112,10 +1112,10 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): # have 2 PRs required for the freeze with repo_a: - repo_a.make_commits('master', Commit('super important file', tree={'g': 'x'}), ref='heads/apr') + repo_a.make_commits(masters[0], Commit('super important file', tree={'g': 'x'}), ref='heads/apr') pr_required_a = repo_a.make_pr(target='master', head='apr') with repo_c: - repo_c.make_commits('master', Commit('update thing', tree={'f': '2'}), ref='heads/cpr') + repo_c.make_commits(masters[2], Commit('update thing', tree={'f': '2'}), ref='heads/cpr') pr_required_c = repo_c.make_pr(target='master', head='cpr') # have 3 release PRs, only the first one updates the tree (version file) @@ -1134,6 +1134,8 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): ) pr_rel_b = repo_b.make_pr(target='master', head='release-1.1') with repo_c: + repo_c.make_commits(masters[2], Commit("Some change", tree={'a': '1'}), ref='heads/whocares') + pr_other = repo_c.make_pr(target='master', head='whocares') repo_c.make_commits( masters[2], Commit('Release 1.1 (C)', tree=None), @@ -1147,7 +1149,6 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): repo_b.name: to_pr(env, pr_rel_b), repo_c.name: to_pr(env, pr_rel_c), } - # trigger the ~~tree~~ freeze wizard w = project.action_prepare_freeze() w2 = project.action_prepare_freeze() @@ -1164,9 +1165,14 @@ def test_freeze_complete(env, project, repo_a, repo_b, repo_c, users, config): # configure releases for r in w_id.release_pr_ids: r.pr_id = release_prs[r.repository_id.name].id + w_id.release_pr_ids[-1].pr_id = to_pr(env, pr_other).id r = w_id.action_freeze() assert r == w, "the freeze is not ready so the wizard should redirect to itself" - assert w_id.errors == "* 2 required PRs not ready." + owner = repo_c.owner + assert w_id.errors == f"""\ +* All release PRs must have the same label, found '{owner}:release-1.1, {owner}:whocares'. +* 2 required PRs not ready.""" + w_id.release_pr_ids[-1].pr_id = release_prs[repo_c.name].id with repo_a: pr_required_a.post_comment('hansen r+', config['role_reviewer']['token'])