[ADD] runbot_merge: small wizard to split a PR off of its batch

runbot_merge/models/pull_requests.py

@@ -19,7 +19,7 @@ import werkzeug
 from markupsafe import Markup
 
 from odoo import api, fields, models, tools, Command
-from odoo.exceptions import AccessError
+from odoo.exceptions import AccessError, UserError
 from odoo.osv import expression
 from odoo.tools import html_escape, Reverse
 from . import commands
@@ -318,6 +318,19 @@ class Branch(models.Model):
             b.active_staging_id = b.with_context(active_test=True).staging_ids
 
 
+class SplitOffWizard(models.TransientModel):
+    _name = "runbot_merge.pull_requests.split_off"
+    _description = "wizard to split a PR off of its current batch and into a different one"
+
+    pr_id = fields.Many2one("runbot_merge.pull_requests", required=True)
+    new_label = fields.Char(string="New Label")
+
+    def button_apply(self):
+        self.pr_id._split_off(self.new_label)
+        self.unlink()
+        return {'type': 'ir.actions.act_window_close'}
+
+
 ACL = collections.namedtuple('ACL', 'is_admin is_reviewer is_author')
 class PullRequests(models.Model):
     _name = 'runbot_merge.pull_requests'
@@ -1605,6 +1618,37 @@ For your own safety I've ignored *everything in your entire comment*.
             format_args=format_args,
         )
 
+    def button_split(self):
+        if len(self.batch_id.prs) == 1:
+            raise UserError("Splitting a batch with a single PR is dumb")
+
+        w = self.env['runbot_merge.pull_requests.split_off'].create({
+            'pr_id': self.id,
+            'new_label': self.label,
+        })
+        return {
+            'type': 'ir.actions.act_window',
+            'res_model': w._name,
+            'res_id': w.id,
+            'target': 'new',
+            'views': [(False, 'form')],
+        }
+
+    def _split_off(self, new_label):
+        # should not be usable to move a PR between batches (maybe later)
+        batch = self.env['runbot_merge.batch']
+        if not re.search(r':patch-\d+$', new_label):
+            if batch.search([
+                ('merge_date', '=', False),
+                ('prs.label', '=', new_label),
+            ]):
+                raise UserError("Can not split off to an existing batch")
+
+        self.write({
+            'label': new_label,
+            'batch_id': batch.create({}).id,
+        })
+
 # ordering is a bit unintuitive because the lowest sequence (and name)
 # is the last link of the fp chain, reasoning is a bit more natural the
 # other way around (highest object is the last), especially with Python

runbot_merge/security/ir.model.access.csv

@@ -9,6 +9,7 @@ access_runbot_merge_repository_status_admin,Admin access to repo statuses,model_
 access_runbot_merge_branch_admin,Admin access to branches,model_runbot_merge_branch,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_pull_requests_admin,Admin access to PR,model_runbot_merge_pull_requests,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_pull_requests_tagging_admin,Admin access to tagging,model_runbot_merge_pull_requests_tagging,runbot_merge.group_admin,1,1,1,1
+access_runbot_merge_pull_requests_split_admin,Admin access to batch split wizard,model_runbot_merge_pull_requests_split_off,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_commit_admin,Admin access to commits,model_runbot_merge_commit,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_stagings_admin,Admin access to stagings,model_runbot_merge_stagings,runbot_merge.group_admin,1,1,1,1
 access_runbot_merge_stagings_heads_admin,Admin access to staging heads,model_runbot_merge_stagings_heads,runbot_merge.group_admin,1,1,1,1

runbot_merge/tests/test_batch_consistency.py

@@ -1,6 +1,8 @@
 """This module tests edge cases specific to the batch objects themselves,
 without wider relevance and thus other location.
 """
+import pytest
+
 from utils import Commit, to_pr
 
 
@@ -79,7 +81,7 @@ def test_close_multiple(env, make_repo2):
     assert not batch_id.active
     assert Batches.search_count([]) == 0
 
-def test_inconsistent_target(env, project, make_repo2):
+def test_inconsistent_target(env, project, make_repo2, users):
     """If a batch's PRs have inconsistent targets,
 
     - only open PRs should count
@@ -87,6 +89,7 @@ def test_inconsistent_target(env, project, make_repo2):
     - the dash should not get hopelessly lost
     - there should be a wizard to split the batch / move a PR to a separate batch
     """
+    # region setup
     Batches = env['runbot_merge.batch']
     repo1 = make_repo2('whe')
     repo2 = make_repo2('whee')
@@ -99,6 +102,9 @@ def test_inconsistent_target(env, project, make_repo2):
         repo1.make_commits('master', Commit('b', tree={"b": "b"}), ref='heads/a_pr')
         pr1 = repo1.make_pr(head='a_pr', target='master')
 
+        repo1.make_commits('master', Commit('b', tree={"c": "c"}), ref='heads/something_else')
+        pr_other = repo1.make_pr(head='something_else', target='master')
+
     with repo2:
         repo2.make_commits(None, Commit("a", tree={"a": "a"}), ref='heads/master')
         repo2.make_commits(None, Commit("a", tree={"a": "a"}), ref='heads/other')
@@ -111,7 +117,13 @@ def test_inconsistent_target(env, project, make_repo2):
         repo3.make_commits('master', Commit('b', tree={"b": "b"}), ref='heads/a_pr')
         pr3 = repo3.make_pr(head='a_pr', target='master')
 
-    [b] = Batches.search([])
+    assert repo1.owner == repo2.owner == repo3.owner
+    owner = repo1.owner
+    # endregion
+
+    # region closeable consistency
+
+    [b] = Batches.search([('all_prs.label', '=', f'{owner}:a_pr')])
     assert b.target.name == 'master'
     assert len(b.prs) == 3
     assert len(b.all_prs) == 3
@@ -127,3 +139,28 @@ def test_inconsistent_target(env, project, make_repo2):
     assert b.target.name == 'master'
     assert len(b.prs) == 2
     assert len(b.all_prs) == 3
+    # endregion
+
+    # region split batch
+    with repo2:
+        pr2.base = 'other'
+    assert b.target.name == False
+    assert to_pr(env, pr_other).label == f'{owner}:something_else'
+    pr2_id = to_pr(env, pr2)
+    act = pr2_id.button_split()
+    assert act['type'] == 'ir.actions.act_window'
+    assert act['views'] == [[False, 'form']]
+    assert act['target'] == 'new'
+    w = env[act['res_model']].browse([act['res_id']])
+    w.new_label = f"{owner}:something_else"
+    with pytest.raises(Exception):
+        w.button_apply()
+    w.new_label = f"{owner}:blah-blah-blah"
+    w.button_apply()
+
+    assert pr2_id.label == f"{owner}:blah-blah-blah"
+    assert pr2_id.batch_id != to_pr(env, pr1).batch_id
+    assert b.target.name == 'master'
+    assert len(b.prs) == 1, "the PR has been moved off of this batch entirely"
+    assert len(b.all_prs) == 2
+    # endregion

runbot_merge/views/mergebot.xml

@@ -125,8 +125,10 @@
             <form>
                 <div class="o_form_statusbar">
                     <span class="o_statusbar_buttons">
-                    <field name="github_url" widget="url" class="btn btn-secondary" text="Github"/>
-                    <field name="url" widget="url" class="btn btn-secondary" text="Frontend"/></span>
+                        <button type="object" name="button_split" string="Split Off"/>
+                        <field name="github_url" widget="url" class="btn btn-secondary" text="Github"/>
+                        <field name="url" widget="url" class="btn btn-secondary" text="Frontend"/>
+                    </span>
                 </div>
                 <sheet>
                     <field name="project" invisible="1"/>
@@ -261,6 +263,20 @@
         </field>
     </record>
 
+    <record id="runbot_merge_pull_requests_split_off_form" model="ir.ui.view">
+        <field name="name">Split Off Form</field>
+        <field name="model">runbot_merge.pull_requests.split_off</field>
+        <field name="arch" type="xml">
+            <form>
+                <field name="new_label" colspan="4"/>
+                <footer>
+                    <button type="object" name="button_apply" string="Apply" class="btn btn-primary"/>
+                    <button special="cancel" string="Cancel"  class="btn btn-secondary"/>
+                </footer>
+            </form>
+        </field>
+    </record>
+
     <record id="runbot_merge_action_stagings" model="ir.actions.act_window">
         <field name="name">Stagings</field>
         <field name="res_model">runbot_merge.stagings</field>