[IMP] base: support ordering on custom models

Up until now, it was impossible to specify the default ordering
on models created manually.

This could somewhat be bypassed by specifying the ordering of records on
views themselves, but this has one main drawback: when using a
relational field that targets a custom model as a group-by key, the
ordering defaulted to the id of the custom record. For example, if I
create a custom field on partners that points to a custom model
'x_grade' on which an 'x_sequence' field exists, I could order my grade
in their own list view according to their sequence, but any read on
partners grouped by this 'x_grade_id' field would order the returned
groups by id while I would prefer to have them ordered according to the
'x_sequence' field.

This commit introduces a new field 'default_order' on the ir.model model
that can store this default ordering clause (as an SQL expression).

Co-Authored-By: Raphaël Collet <rco@odoo.com>

odoo/addons/base/models/ir_model.py

@@ -4,6 +4,7 @@ import datetime
 import dateutil
 import itertools
 import logging
+import re
 import time
 from ast import literal_eval
 from collections import defaultdict, Mapping
@@ -19,7 +20,7 @@ from odoo.tools.safe_eval import safe_eval
 _logger = logging.getLogger(__name__)
 
 MODULE_UNINSTALL_FLAG = '_force_unlink'
-
+RE_ORDER_FIELDS = re.compile(r'"?(\w+)"?\s*(?:asc|desc)?', flags=re.I)
 
 # base environment for doing a safe_eval
 SAFE_EVAL_BASE = {
@@ -99,6 +100,8 @@ class IrModel(models.Model):
 
     name = fields.Char(string='Model Description', translate=True, required=True)
     model = fields.Char(default='x_', required=True, index=True)
+    order = fields.Char(string='Order', default='id', required=True,
+                        help='SQL expression for ordering records in the model; e.g. "x_sequence asc, id desc"')
     info = fields.Text(string='Information')
     field_id = fields.One2many('ir.model.fields', 'model_id', string='Fields', required=True, copy=True,
                                default=_default_field_id)
@@ -156,6 +159,24 @@ class IrModel(models.Model):
             if not models.check_object_name(model.model):
                 raise ValidationError(_("The model name can only contain lowercase characters, digits, underscores and dots."))
 
+    @api.constrains('order', 'field_id')
+    def _check_order(self):
+        for model in self:
+            try:
+                model._check_qorder(model.order)  # regex check for the whole clause ('is it valid sql?')
+            except UserError as e:
+                raise ValidationError(str(e))
+            stored_fields = model.field_id.filtered('store').mapped('name')
+            if self.env.get(model.model) is None:
+                # model hasn't been init'd yet, which means that some fields are not yet in its
+                # list of fields but will be right after its creation - these fields can be used
+                # for ordering, so let's add them to the list of stored fields manually
+                stored_fields += models.MAGIC_COLUMNS
+            order_fields = RE_ORDER_FIELDS.findall(model.order)
+            for field in order_fields:
+                if field not in stored_fields:
+                    raise ValidationError(_("Unable to order by %s: fields used for ordering must be present on the model and stored.") % field)
+
     _sql_constraints = [
         ('obj_name_uniq', 'unique (model)', 'Each model must be unique!'),
     ]
@@ -238,7 +259,12 @@ class IrModel(models.Model):
         # writes (4,id,False) even for non dirty items.
         if 'field_id' in vals:
             vals['field_id'] = [op for op in vals['field_id'] if op[0] != 4]
-        return super(IrModel, self).write(vals)
+        res = super(IrModel, self).write(vals)
+        # ordering has been changed, reload registry to reflect update + signaling
+        if 'order' in vals:
+            self.flush()  # setup_models need to fetch the updated values from the db
+            self.pool.setup_models(self._cr)
+        return res
 
     @api.model
     def create(self, vals):
@@ -264,6 +290,7 @@ class IrModel(models.Model):
         return {
             'model': model._name,
             'name': model._description,
+            'order': model._order,
             'info': next(cls.__doc__ for cls in type(model).mro() if cls.__doc__),
             'state': 'manual' if model._custom else 'base',
             'transient': model._transient,
@@ -299,6 +326,7 @@ class IrModel(models.Model):
             _module = False
             _custom = True
             _transient = bool(model_data['transient'])
+            _order = model_data['order']
             __doc__ = model_data['info']
 
         return CustomModel

odoo/addons/base/tests/test_ir_model.py

@@ -3,7 +3,8 @@
 
 from psycopg2 import IntegrityError
 
-from odoo.tests.common import TransactionCase
+from odoo.exceptions import ValidationError
+from odoo.tests.common import TransactionCase, SavepointCase
 from odoo.tools import mute_logger
 
 
@@ -168,3 +169,105 @@ class TestXMLID(TransactionCase):
         }]
         with self.assertRaisesRegex(IntegrityError, 'ir_model_data_name_nospaces'):
             model._load_records(data_list)
+
+
+class TestIrModel(SavepointCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+
+        # The test mode is necessary in this case.  After each test, we call
+        # registry.reset_changes(), which opens a new cursor to retrieve custom
+        # models and fields.  A regular cursor would correspond to the state of
+        # the database before setUpClass(), which is not correct.  Instead, a
+        # test cursor will correspond to the state of the database of cls.cr at
+        # that point, i.e., before the call to setUp().
+        cls.registry.enter_test_mode(cls.cr)
+        cls.addClassCleanup(cls.registry.leave_test_mode)
+
+        # model and records for bananas
+        cls.bananas_model = cls.env['ir.model'].create({
+            'name': 'Bananas',
+            'model': 'x_bananas',
+            'field_id': [
+                (0, 0, {'name': 'x_name', 'ttype': 'char', 'field_description': 'Name'}),
+                (0, 0, {'name': 'x_length', 'ttype': 'float', 'field_description': 'Length'}),
+                (0, 0, {'name': 'x_color', 'ttype': 'integer', 'field_description': 'Color'}),
+            ]
+        })
+        # add non-stored field that is not valid in order
+        cls.env['ir.model.fields'].create({
+            'name': 'x_is_yellow',
+            'field_description': 'Is the banana yellow?',
+            'ttype': 'boolean',
+            'model_id': cls.bananas_model.id,
+            'store': False,
+            'depends': 'x_color',
+            'compute': "for banana in self:\n    banana['x_is_yellow'] = banana.x_color == 9"
+        })
+        cls.env['x_bananas'].create([{
+            'x_name': 'Banana #1',
+            'x_length': 3.14159,
+            'x_color': 9,
+        }, {
+            'x_name': 'Banana #2',
+            'x_length': 0,
+            'x_color': 6,
+        }, {
+            'x_name': 'Banana #3',
+            'x_length': 10,
+            'x_color': 6,
+        }])
+
+    def setUp(self):
+        # this cleanup is necessary after each test, and must be done last
+        self.addCleanup(self.registry.reset_changes)
+        super().setUp()
+
+    def test_model_order_constraint(self):
+        """Check that the order constraint is properly enforced."""
+        VALID_ORDERS = ['id', 'id desc', 'id asc, x_length', 'x_color, x_length, create_uid']
+        for order in VALID_ORDERS:
+            self.bananas_model.order = order
+
+        INVALID_ORDERS = ['', 'x_wat', 'id esc', 'create_uid,', 'id, x_is_yellow']
+        for order in INVALID_ORDERS:
+            with self.assertRaises(ValidationError), self.cr.savepoint():
+                self.bananas_model.order = order
+
+        # check that the constraint is checked at model creation
+        fields_value = [
+            (0, 0, {'name': 'x_name', 'ttype': 'char', 'field_description': 'Name'}),
+            (0, 0, {'name': 'x_length', 'ttype': 'float', 'field_description': 'Length'}),
+            (0, 0, {'name': 'x_color', 'ttype': 'integer', 'field_description': 'Color'}),
+        ]
+        self.env['ir.model'].create({
+            'name': 'MegaBananas',
+            'model': 'x_mega_bananas',
+            'order': 'x_name asc, id desc',         # valid order
+            'field_id': fields_value,
+        })
+        with self.assertRaises(ValidationError):
+            self.env['ir.model'].create({
+                'name': 'GigaBananas',
+                'model': 'x_giga_bananas',
+                'order': 'x_name asc, x_wat',       # invalid order
+                'field_id': fields_value,
+            })
+
+    def test_model_order_search(self):
+        """Check that custom orders are applied when querying a model."""
+        ORDERS = {
+            'id asc': ['Banana #1', 'Banana #2', 'Banana #3'],
+            'id desc': ['Banana #3', 'Banana #2', 'Banana #1'],
+            'x_color asc, id asc': ['Banana #2', 'Banana #3', 'Banana #1'],
+            'x_color asc, id desc': ['Banana #3', 'Banana #2', 'Banana #1'],
+            'x_length asc, id': ['Banana #2', 'Banana #1', 'Banana #3'],
+        }
+        for order, names in ORDERS.items():
+            self.bananas_model.order = order
+            self.assertEqual(self.env['x_bananas']._order, order)
+
+            bananas = self.env['x_bananas'].search([])
+            self.assertEqual(bananas.mapped('x_name'), names, 'failed to order by %s' % order)

odoo/addons/base/views/ir_model_views.xml

@@ -31,6 +31,7 @@
                             <field name="id" invisible="1"/>
                             <field name="name"/>
                             <field name="model" attrs="{'readonly': [('id', '!=', False)]}"/>
+                            <field name="order"/>
                             <field name="transient" attrs="{'readonly': [('id', '!=', False)]}" groups="base.group_no_one"/>
                         </group>
                         <group>