[REM] forwardport: fp_sequence field

It's almost certainly not useful, save as a minor convenience for
tests: decorrelating the branch sequence and the fp sequence seems
like it would only be extremely confusing, and on the mergebot all the
fp_sequence values are set to the default while the sequence values
are set to something useful and sensible (kinda).

Fixes #584

forwardport/data/views.xml

@@ -157,9 +157,6 @@
                 <field name="fp_target" string="FP to"
                        help="This branch will be forward-ported to (from lower ones)"
                 />
-                <field name="fp_sequence" string="FP sequence"
-                       help="Overrides the normal sequence"
-                />
             </xpath>
         </field>
     </record>

forwardport/models/project.py

@@ -34,6 +34,7 @@ from odoo import _, models, fields, api
 from odoo.osv import expression
 from odoo.exceptions import UserError
 from odoo.tools import topological_sort, groupby
+from odoo.tools.sql import reverse_order
 from odoo.tools.appdirs import user_cache_dir
 from odoo.addons.runbot_merge import utils
 from odoo.addons.runbot_merge.models.pull_requests import RPLUS
@@ -199,17 +200,10 @@ class Project(models.Model):
 
     def _forward_port_ordered(self, domain=()):
         Branches = self.env['runbot_merge.branch']
-        ordering_items = re.split(r',\s*', 'fp_sequence,' + Branches._order)
-        ordering = ','.join(
-            # reverse order (desc -> asc, asc -> desc) as we want the "lower"
-            # branches to be first in the ordering
-            f[:-5] if f.lower().endswith(' desc') else f + ' desc'
-            for f in ordering_items
-        )
         return Branches.search(expression.AND([
             [('project_id', '=', self.id)],
             domain or [],
-        ]), order=ordering)
+        ]), order=reverse_order(Branches._order))
 
 class Repository(models.Model):
     _inherit = 'runbot_merge.repository'
@@ -218,7 +212,6 @@ class Repository(models.Model):
 class Branch(models.Model):
     _inherit = 'runbot_merge.branch'
 
-    fp_sequence = fields.Integer(default=50, group_operator=None)
     fp_target = fields.Boolean(default=True)
     fp_enabled = fields.Boolean(compute='_compute_fp_enabled')
 

forwardport/tests/test_conflicts.py

@@ -16,7 +16,7 @@ def test_conflict(env, config, make_repo, users):
     project = env['runbot_merge.project'].search([])
     project.write({
         'branch_ids': [
-            (0, 0, {'name': 'd', 'fp_sequence': 4, 'fp_target': True})
+            (0, 0, {'name': 'd', 'sequence': 40, 'fp_target': True})
         ]
     })
 

forwardport/tests/test_simple.py

@@ -750,7 +750,9 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
         main2.get_pr(pr2c.number).post_comment('%s r+' % project.fp_github_name, config['role_reviewer']['token'])
     env.run_crons()
 
-    stb, stc = env['runbot_merge.stagings'].search([], order='target')
+    env['runbot_merge.stagings'].search([]).mapped('target.display_name')
+    env['runbot_merge.stagings'].search([], order='target').mapped('target.display_name')
+    stc, stb = env['runbot_merge.stagings'].search([], order='target')
     assert stb.target.name == 'b'
     assert stc.target.name == 'c'
 

forwardport/tests/test_updates.py

@@ -152,7 +152,7 @@ def test_update_merged(env, make_repo, config, users):
         prod.make_ref('heads/d', prod.commit('c').id)
     env['runbot_merge.project'].search([]).write({
         'branch_ids': [(0, 0, {
-            'name': 'd', 'fp_sequence': -1, 'fp_target': True,
+            'name': 'd', 'sequence': 40, 'fp_target': True,
         })]
     })
 
@@ -251,10 +251,10 @@ def test_duplicate_fw(env, make_repo, setreviewers, config, users):
         'github_prefix': 'hansen',
         'fp_github_token': config['github']['token'],
         'branch_ids': [
-            (0, 0, {'name': 'master', 'fp_sequence': 0, 'fp_target': True}),
+            (0, 0, {'name': 'master', 'sequence': 0, 'fp_target': True}),
-            (0, 0, {'name': 'v3', 'fp_sequence': 1, 'fp_target': True}),
+            (0, 0, {'name': 'v3', 'sequence': 1, 'fp_target': True}),
-            (0, 0, {'name': 'v2', 'fp_sequence': 2, 'fp_target': True}),
+            (0, 0, {'name': 'v2', 'sequence': 2, 'fp_target': True}),
-            (0, 0, {'name': 'v1', 'fp_sequence': 3, 'fp_target': True}),
+            (0, 0, {'name': 'v1', 'sequence': 3, 'fp_target': True}),
         ],
         'repo_ids': [
             (0, 0, {

forwardport/tests/test_weird.py

@@ -263,9 +263,9 @@ class TestNotAllBranches:
             'github_prefix': 'hansen',
             'fp_github_token': config['github']['token'],
             'branch_ids': [
-                (0, 0, {'name': 'a', 'fp_sequence': 2, 'fp_target': True}),
+                (0, 0, {'name': 'a', 'sequence': 2, 'fp_target': True}),
-                (0, 0, {'name': 'b', 'fp_sequence': 1, 'fp_target': True}),
+                (0, 0, {'name': 'b', 'sequence': 1, 'fp_target': True}),
-                (0, 0, {'name': 'c', 'fp_sequence': 0, 'fp_target': True}),
+                (0, 0, {'name': 'c', 'sequence': 0, 'fp_target': True}),
             ]
         })
         repo_a = env['runbot_merge.repository'].create({
@@ -486,15 +486,15 @@ def test_new_intermediate_branch(env, config, make_repo):
     # insert a branch between "b" and "c"
     project.write({
         'branch_ids': [
-            (1, currents['a'], {'fp_sequence': 3}),
+            (1, currents['a'], {'sequence': 3}),
-            (1, currents['b'], {'fp_sequence': 2, 'active': False}),
+            (1, currents['b'], {'sequence': 2, 'active': False}),
-            (1, currents['c'], {'fp_sequence': 0})
+            (1, currents['c'], {'sequence': 0})
         ]
     })
     env.run_crons()
     project.write({
         'branch_ids': [
-            (0, False, {'name': 'new', 'fp_sequence': 1, 'fp_target': True}),
+            (0, False, {'name': 'new', 'sequence': 1, 'fp_target': True}),
         ]
     })
     env.run_crons()
@@ -707,18 +707,16 @@ def test_retarget_after_freeze(env, config, make_repo, users):
     assert port_id.source_id == original_pr_id
     assert port_id.parent_id == original_pr_id
 
-    # because the module doesn't update the ordering of `branch_ids` to take
+    branch_c, branch_b, branch_a = branches_before = project.branch_ids
-    # `fp_sequence` in account so it's misleading
-    branch_c, branch_b, branch_a = branches_before = project.branch_ids.sorted('fp_sequence')
     assert [branch_a.name, branch_b.name, branch_c.name] == ['a', 'b', 'c']
     # create branch so cron runs correctly
     with prod: prod.make_ref('heads/bprime', prod.get_ref('c'))
     project.write({
         'branch_ids': [
-            (1, branch_c.id, {'sequence': 1, 'fp_sequence': 20}),
+            (1, branch_c.id, {'sequence': 1}),
-            (0, 0, {'name': 'bprime', 'sequence': 2, 'fp_sequence': 20, 'fp_target': True}),
+            (0, 0, {'name': 'bprime', 'sequence': 2, 'fp_target': True}),
-            (1, branch_b.id, {'sequence': 3, 'fp_sequence': 20}),
+            (1, branch_b.id, {'sequence': 3}),
-            (1, branch_a.id, {'sequence': 4, 'fp_sequence': 20}),
+            (1, branch_a.id, {'sequence': 4}),
         ]
     })
     new_branch = project.branch_ids - branches_before

mergebot_test_utils/utils.py

@@ -74,9 +74,9 @@ def make_basic(env, config, make_repo, *, reponame='proj', project_name='myproje
             'github_prefix': 'hansen',
             'fp_github_token': config['github']['token'],
             'branch_ids': [
-                (0, 0, {'name': 'a', 'fp_sequence': 10, 'fp_target': True}),
+                (0, 0, {'name': 'a', 'sequence': 100, 'fp_target': True}),
-                (0, 0, {'name': 'b', 'fp_sequence': 8, 'fp_target': True}),
+                (0, 0, {'name': 'b', 'sequence': 80, 'fp_target': True}),
-                (0, 0, {'name': 'c', 'fp_sequence': 6, 'fp_target': True}),
+                (0, 0, {'name': 'c', 'sequence': 60, 'fp_target': True}),
             ],
         })