[FIX] *: freeze wizard take 3

Fixes to the new bits which didn't really work:

- Fix borked view layout
- Add some help to the label fields
- Improve the resolution of label -> pr, and fix
- Also make the feature actually work for bump PRs
- Also make pr -> label work more reliably, now allows setting one PR
  and getting the other PRs of the same batch (with the same label)
  even without setting the label by hand

An autocomplete for the label has been considered but there is no
autocomplete field for char/selection fields, and it seems way too
much work for the utility:

- either create a brand new widget for 15.0 which will have to be
  entirely rewritten in 16
- or create a transient model composed entirely of fake records to
  provide an m2o to records which don't actually exist as label
  bearers, which is also a lot of unnecessary work

NOTE: we want to support partial freezing (aka not freeze all the
      branches because some of them have different release models
      than others), so some project repos *not* having a release
      PR is fine and normal, such a validation should not be added.

Fixes #664

runbot_merge/models/project_freeze/__init__.py

@@ -4,8 +4,9 @@ import itertools
 import json
 import logging
 import time
+from collections import Counter
 
-from odoo import models, fields, api
+from odoo import models, fields, api, Command
 from odoo.exceptions import UserError
 from odoo.addons.runbot_merge.exceptions import FastForwardError
 
@@ -24,20 +25,28 @@ class FreezeWizard(models.Model):
         help="Pull requests which must have been merged before the freeze is allowed",
     )
 
-    release_label = fields.Char()
+    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_pr_ids = fields.One2many(
         'runbot_merge.project.freeze.prs', 'wizard_id',
         string="Release pull requests",
         help="Pull requests used as tips for the freeze branches, "
-             "one per repository"
+             "one per repository",
     )
 
-    bump_label = fields.Char()
+    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_pr_ids = fields.One2many(
         'runbot_merge.project.freeze.bumps', 'wizard_id',
         string="Bump pull requests",
         help="Pull requests used as tips of the frozen-off branches, "
-             "one per repository"
+             "one per repository",
     )
 
     _sql_constraints = [
@@ -47,39 +56,57 @@ class FreezeWizard(models.Model):
 
     @api.onchange('release_label')
     def _onchange_release_label(self):
+        if not self.release_label:
+            return
+
         prs = self.env['runbot_merge.pull_requests'].search([
             ('label', '=', self.release_label)
         ])
         for release_pr in self.release_pr_ids:
-            release_pr.pd_id = next((
+            release_pr.pr_id = prs.filtered(lambda p: p.repository == release_pr.repository_id)
-                p.id for p in prs
-                if p.repository == release_pr.repository_id
-            ), False)
 
     @api.onchange('release_pr_ids')
     def _onchange_release_prs(self):
-        labels = {p.pr_id.label for p in self.release_pr_ids}
+        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()
 
     @api.onchange('bump_label')
     def _onchange_bump_label(self):
+        if not self.release_label:
+            return
+
         prs = self.env['runbot_merge.pull_requests'].search([
             ('label', '=', self.bump_label)
         ])
-        for bump_pr in self.bump_pr_ids:
+
-            bump_pr.pd_id = next((
+        commands = [Command.clear()]
-                p.id for p in prs
+        for pr in prs:
-                if p.repository == bump_pr.repository_id
+            current = self.bump_pr_ids.filtered(lambda bump_pr: bump_pr.repository_id == pr.repository)
-            ), False)
+            if current:
+                commands.append(Command.update(current.id, {'pr_id': pr.id}))
+            else:
+                commands.append(Command.create({
+                    'repository_id': pr.repository.id,
+                    'pr_id': pr.id
+                }))
+
+        self.write({'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}
+        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()
 
     @api.depends('release_pr_ids.pr_id.label', 'required_pr_ids.state')
     def _compute_errors(self):
         errors = []
+
+        release_repos = Counter(self.mapped('release_pr_ids.repository_id'))
+        release_repos.subtract(self.project_id.repo_ids)
+        excess = {k.name for k, v in release_repos.items() if v > 0}
+        if excess:
+            errors.append("* Every repository must have one release PR, found multiple for %s" % ', '.join(excess))
+
         without = self.release_pr_ids.filtered(lambda p: not p.pr_id)
         if without:
             errors.append("* Every repository must have a release PR, missing release PRs for %s." % ', '.join(
@@ -93,6 +120,11 @@ class FreezeWizard(models.Model):
         if non_squash:
             errors.append("* Release PRs should have a single commit, found more in %s." % ', '.join(p.display_name for p in non_squash))
 
+        bump_repos = Counter(self.mapped('bump_pr_ids.repository_id'))
+        excess = {k.name for k, v in bump_repos.items() if v > 1}
+        if excess:
+            errors.append("* Every repository may have one bump PR, found multiple for %s" % ', '.join(excess))
+
         bump_labels = set(self.mapped('bump_pr_ids.pr_id.label'))
         if len(bump_labels) > 1:
             errors.append("* All bump PRs must have the same label, found %r" % ', '.join(sorted(bump_labels)))
@@ -327,20 +359,24 @@ class BumpPullRequest(models.Model):
     _description = "links to pull requests used to \"bump\" the development branches"
 
     wizard_id = fields.Many2one('runbot_merge.project.freeze', required=True, index=True, ondelete='cascade')
+    # FIXME: repo = wizard.repo?
     repository_id = fields.Many2one('runbot_merge.repository', required=True)
     pr_id = fields.Many2one(
         'runbot_merge.pull_requests',
         domain='[("repository", "=", repository_id), ("state", "not in", ("closed", "merged"))]',
-        string="Release Pull Request",
+        string="Bump Pull Request",
     )
     label = fields.Char(related='pr_id.label')
 
+    @api.onchange('repository_id')
+    def _onchange_repository(self):
+        self.pr_id = False
+
     def write(self, vals):
         # only the pr should be writeable after initial creation
         assert 'wizard_id' not in vals
-        assert 'repository_id' not in vals
         # and if the PR gets set, it should match the requested repository
-        if 'pr_id' in vals:
+        if vals.get('pr_id'):
             assert self.env['runbot_merge.pull_requests'].browse(vals['pr_id'])\
                        .repository == self.repository_id
 

runbot_merge/models/project_freeze/views.xml

@@ -16,46 +16,40 @@
                                 options="{'color_field': 'state_color', 'no_create': True}"/>
                         </group>
                     </group>
-                    <group>
+                    <group string="Release">
-                        <group colspan="2" string="Release">
+                        <p colspan="2">
-                            <p>
+                            Release (freeze) PRs, provide the first commit
-                                Release (freeze) PRs, provide the first commit
+                            of the new branches. Each PR must have a single
-                                of the new branches. Each PR must have a single
+                            commit.
-                                commit.
+                        </p>
-                            </p>
+                        <p class="alert alert-warning" role="alert" colspan="2">
-                            <p class="alert alert-warning" role="alert">
+                            These PRs will be merged directly, not staged.
-                                These PRs will be merged directly, not staged.
+                        </p>
-                            </p>
+                        <field name="release_label" colspan="2"/>
-                            <field name="release_label"/>
+                        <field name="release_pr_ids" nolabel="1" colspan="2">
-                            <field name="release_pr_ids" nolabel="1">
+                            <tree editable="bottom" create="false" delete="false">
-                                <tree editable="bottom" create="false">
+                                <field name="repository_id" readonly="1"/>
-                                    <field name="repository_id" readonly="1"/>
+                                <field name="pr_id" options="{'no_create': True}"/>
-                                    <field name="pr_id" options="{'no_create': True}"
+                                <field name="label"/>
-                                           context="{'pr_include_title': 1}"/>
+                            </tree>
-                                    <field name="label"/>
+                        </field>
-                                </tree>
-                            </field>
-                        </group>
                     </group>
-                    <group>
+                    <group string="Bump">
-                        <group colspan="2" string="Bump">
+                        <p colspan="2">
-                            <p>
+                            Bump PRs, provide the first commit of the source
-                                Bump PRs, provide the first commit of the source
+                            branches after the release has been cut.
-                                branches after the release has been cut.
+                        </p>
-                            </p>
+                        <p class="alert alert-warning" role="alert" colspan="2">
-                            <p class="alert alert-warning" role="alert">
+                            These PRs will be merged directly, not staged.
-                                These PRs will be merged directly, not staged.
+                        </p>
-                            </p>
+                        <field name="bump_label" colspan="2"/>
-                            <field name="bump_label"/>
+                        <field name="bump_pr_ids" nolabel="1" colspan="2">
-                            <field name="bump_pr_ids" nolabel="1">
+                            <tree editable="bottom">
-                                <tree editable="bottom" create="false">
+                                <field name="repository_id" options="{'no_create': True}"/>
-                                    <field name="repository_id" readonly="1"/>
+                                <field name="pr_id" options="{'no_create': True}"/>
-                                    <field name="pr_id" options="{'no_create': True}"
+                                <field name="label"/>
-                                           context="{'pr_include_title': 1}"/>
+                            </tree>
-                                    <field name="label"/>
+                        </field>
-                                </tree>
-                            </field>
-                        </group>
                     </group>
                     <footer>
                         <!--