[FIX] runbot_merge: make freeze wizard labels lookup not shit

I DECLARE BANKRUPTCY!!!

The previous implementation of labels lookup was really not
intuitive (it was just a char field, and matched labels by equality
including the owner tag), and was also full of broken edge
cases (e.g. traceback if a label matched multiple PRs in the same repo
because people reuse branch names).

Tried messing about with contextual `display_name` and `name_search`
on PRs but the client goes wonky in that case, and there is no clean
autocomplete for non-relational fields.

So created a view which reifies labels, and that can be used as the
basis for our search. It doesn't have to be maintained by hand, can be
searched somewhat flexibly, we can add new view fields in the future
if desirable, and it seems to work fine providing a nice
understandable UX, with the reliability of using a normal Odoo model
the normal way.

Also fixed the handling of bump PRs, clearly clearing the entire field
before trying to update existing records (even with a link_to
inbetween) is not the web client's fancy, re-selecting the current
label would just empty the thing entirely.

So use a two-step process slightly closer to the release PRs instead:

- first update or delete the existing bump PRs
- then add the new ones

The second part is because bump PRs are somewhat less critical than
release, so it can be a bit more DWIM compared to the more deliberate
process of release PRs where first the list of repositories involved
has to be set up just so, then the PRs can be filled in each of them.

Fixes #697
This commit is contained in:
Xavier Morel 2023-01-24 11:19:18 +01:00
parent ae53c87fc9
commit 8d7d6302d3
4 changed files with 74 additions and 31 deletions

View File

@ -847,6 +847,11 @@ def test_freeze(env, config, make_repo, users):
assert not w_id.errors
w_id.action_freeze()
# re-enable forward-port cron after freeze
_, cron_id = env['ir.model.data'].check_object_reference('forwardport', 'port_forward', context={'active_test': False})
env['ir.cron'].browse([cron_id]).active = True
# run crons to process the feedback, run a second time in case of e.g.
# forward porting
env.run_crons()

View File

@ -9,8 +9,9 @@ from collections import Counter
from markupsafe import Markup
from odoo import models, fields, api, Command
from odoo.exceptions import UserError
from odoo.addons.runbot_merge.exceptions import FastForwardError
from odoo.exceptions import UserError
from odoo.tools import drop_view_if_exists
_logger = logging.getLogger(__name__)
class FreezeWizard(models.Model):
@ -38,11 +39,7 @@ class FreezeWizard(models.Model):
if v
)
release_label = fields.Char(
string="Find by label",
help="Setting a (complete) PR label will automatically find and "
"configure the corresponding PRs as release",
)
release_label = fields.Many2one('runbot_merge.freeze.labels', store=False, string="Release label", help="Find release PRs by label")
release_pr_ids = fields.One2many(
'runbot_merge.project.freeze.prs', 'wizard_id',
string="Release pull requests",
@ -50,11 +47,7 @@ class FreezeWizard(models.Model):
"one per repository",
)
bump_label = fields.Char(
string="Find by label",
help="Setting a (complete) PR label will automatically find and "
"configure the corresponding PRs as bump",
)
bump_label = fields.Many2one('runbot_merge.freeze.labels', store=False, string="Bump label", help="Find bump PRs by label")
bump_pr_ids = fields.One2many(
'runbot_merge.project.freeze.bumps', 'wizard_id',
string="Bump pull requests",
@ -73,42 +66,51 @@ class FreezeWizard(models.Model):
return
prs = self.env['runbot_merge.pull_requests'].search([
('label', '=', self.release_label)
('label', '=', self.release_label.label),
('state', 'not in', ('merged', 'closed')),
])
for release_pr in self.release_pr_ids:
release_pr.pr_id = prs.filtered(lambda p: p.repository == release_pr.repository_id)
p = prs.filtered(lambda p: p.repository == release_pr.repository_id)
if len(p) < 2:
release_pr.pr_id = p
@api.onchange('release_pr_ids')
def _onchange_release_prs(self):
labels = {p.pr_id.label for p in self.release_pr_ids if p.pr_id}
self.release_label = len(labels) == 1 and labels.pop()
self.release_label = len(labels) == 1 and self.env['runbot_merge.freeze.labels'].search([
('label', '=', labels.pop()),
])
@api.onchange('bump_label')
def _onchange_bump_label(self):
if not self.release_label:
if not self.bump_label:
return
prs = self.env['runbot_merge.pull_requests'].search([
('label', '=', self.bump_label)
('label', '=', self.bump_label.label),
('state', 'not in', ('merged', 'closed')),
])
commands = [Command.clear()]
for pr in prs:
current = self.bump_pr_ids.filtered(lambda bump_pr: bump_pr.repository_id == pr.repository)
if current:
commands.append(Command.update(current.id, {'pr_id': pr.id}))
commands = []
for bump_pr in self.bump_pr_ids:
p = prs.filtered(lambda p: p.repository == bump_pr.repository_id)
if len(p) == 1:
commands.append(Command.update(bump_pr.id, {'pr_id': p.id}))
else:
commands.append(Command.create({
'repository_id': pr.repository.id,
'pr_id': pr.id
}))
commands.append(Command.delete(bump_pr.id))
prs -= p
self.write({'bump_pr_ids': commands})
commands.extend(
Command.create({'repository_id': pr.repository.id, 'pr_id': pr.id})
for pr in prs
)
self.bump_pr_ids = commands
@api.onchange('bump_pr_ids')
def _onchange_bump_prs(self):
labels = {p.pr_id.label for p in self.bump_pr_ids if p.pr_id}
self.bump_label = len(labels) == 1 and labels.pop()
self.bump_label = len(labels) == 1 and self.env['runbot_merge.freeze.labels'].search([
('label', '=', labels.pop()),
])
@api.depends('release_pr_ids.pr_id.label', 'required_pr_ids.state')
def _compute_errors(self):
@ -433,3 +435,38 @@ class PullRequest(models.Model):
def _compute_state_color(self):
for p in self:
p.state_color = STATE_COLORMAP[p.state]
class OpenPRLabels(models.Model):
"""Hacking around using contextual display_name to try and autocomplete
labels through PRs doesn't work because the client fucks up the display_name
(apparently they're not keyed on the context), therefore the behaviour
is inconsistent as the label shown in the autocomplete will result in the
PR being shown as its label in the o2m, and the other way around (if a PR
is selected directly in the o2m, then the PR's base display_name will be
shown in the label lookup field).
Therefore create a dumbshit view of label records.
Under the assumption that we'll have less than 256 repositories, the id of a
label record is the PR's id shifted as the high 24 bits, and the repo id as
the low 8.
"""
_name = 'runbot_merge.freeze.labels'
_description = "view representing labels for open PRs so they can autocomplete properly"
_rec_name = "label"
_auto = False
def init(self):
super().init()
drop_view_if_exists(self.env.cr, "runbot_merge_freeze_labels");
self.env.cr.execute("""
CREATE VIEW runbot_merge_freeze_labels AS (
SELECT DISTINCT ON (label)
id << 8 | repository as id,
label
FROM runbot_merge_pull_requests
WHERE state != 'merged' AND state != 'closed'
ORDER BY label, repository, id
)""")
label = fields.Char()

View File

@ -29,7 +29,7 @@
<field name="release_label" colspan="2"/>
<field name="release_pr_ids" nolabel="1" colspan="2">
<tree editable="bottom">
<field name="repository_id" readonly="1"/>
<field name="repository_id" options="{'no_create': True}"/>
<field name="pr_id" options="{'no_create': True}"/>
<field name="label"/>
</tree>

View File

@ -1,8 +1,9 @@
id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
access_runbot_merge_project_admin,Admin access to project,model_runbot_merge_project,runbot_merge.group_admin,1,1,1,1
access_runbot_merge_project_freeze,Admin access to freeze wizard,model_runbot_merge_project_freeze,runbot_merge.group_admin,1,1,0,0
access_runbot_merge_project_freeze_prs,Admin access to freeze wizard release prs,model_runbot_merge_project_freeze_prs,runbot_merge.group_admin,1,1,0,1
access_runbot_merge_project_freeze_prs,Admin access to freeze wizard release prs,model_runbot_merge_project_freeze_prs,runbot_merge.group_admin,1,1,1,1
access_runbot_merge_project_freeze_bumps,Admin access to freeze wizard bump prs,model_runbot_merge_project_freeze_bumps,runbot_merge.group_admin,1,1,1,1
access_runbot_merge_pr_labels,Admin access to labels view,model_runbot_merge_freeze_labels,runbot_merge.group_admin,1,0,0,0
access_runbot_merge_repository_admin,Admin access to repo,model_runbot_merge_repository,runbot_merge.group_admin,1,1,1,1
access_runbot_merge_repository_status_admin,Admin access to repo statuses,model_runbot_merge_repository_status,runbot_merge.group_admin,1,1,1,1
access_runbot_merge_branch_admin,Admin access to branches,model_runbot_merge_branch,runbot_merge.group_admin,1,1,1,1

1 id name model_id:id group_id:id perm_read perm_write perm_create perm_unlink
2 access_runbot_merge_project_admin Admin access to project model_runbot_merge_project runbot_merge.group_admin 1 1 1 1
3 access_runbot_merge_project_freeze Admin access to freeze wizard model_runbot_merge_project_freeze runbot_merge.group_admin 1 1 0 0
4 access_runbot_merge_project_freeze_prs Admin access to freeze wizard release prs model_runbot_merge_project_freeze_prs runbot_merge.group_admin 1 1 0 1 1
5 access_runbot_merge_project_freeze_bumps Admin access to freeze wizard bump prs model_runbot_merge_project_freeze_bumps runbot_merge.group_admin 1 1 1 1
6 access_runbot_merge_pr_labels Admin access to labels view model_runbot_merge_freeze_labels runbot_merge.group_admin 1 0 0 0
7 access_runbot_merge_repository_admin Admin access to repo model_runbot_merge_repository runbot_merge.group_admin 1 1 1 1
8 access_runbot_merge_repository_status_admin Admin access to repo statuses model_runbot_merge_repository_status runbot_merge.group_admin 1 1 1 1
9 access_runbot_merge_branch_admin Admin access to branches model_runbot_merge_branch runbot_merge.group_admin 1 1 1 1