[FIX] models.py: invalid write order in inherits

Create a model and extend this model using an inherits. Define a
constraint in the inherited model that uses a field of the parent model.
Create a valid element using the inherited model then write an invalid
value. This pass the constraint although it should raise an exception.

The problem resides in the `write` function of `models.py`. It first
write values on the inherited model then write values on the parent
model. The order is wrong, it should write on the parent model first
then on the child model.

The solution is to change the order fields are written: first we update
child references to their parents, then we update the parents, then we
update the child.

opw-1962705

closes #32919

closes odoo/odoo#33003

Signed-off-by: Raphael Collet (rco) <rco@openerp.com>

odoo/addons/test_inherits/ir.model.access.csv

@@ -2,3 +2,5 @@
 access_test_unit,access_test_unit,model_test_unit,,1,1,1,1
 access_test_box,access_test_box,model_test_box,,1,1,1,1
 access_test_pallet,access_test_pallet,model_test_pallet,,1,1,1,1
+access_test_another_unit,access_test_another_unit,model_test_another_unit,,1,1,1,1
+access_test_another_box,access_test_another_box,model_test_another_box,,1,1,1,1

odoo/addons/test_inherits/models.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 from odoo import models, fields, api
+from odoo.exceptions import ValidationError
 
 
 # We just create a new model
@@ -38,3 +39,28 @@ class Pallet(models.Model):
     box_id = fields.Many2one('test.box', 'Box', required=True,
                              ondelete='cascade')
     field_in_pallet = fields.Char('Field2')
+
+
+# Another model for another test suite
+class AnotherUnit(models.Model):
+    _name = 'test.another_unit'
+    _description = 'Another Test Unit'
+
+    val1 = fields.Integer('Value 1', required=True)
+
+
+# We want to _inherits from the parent model, add a field and check
+# the new field is always equals to the first one
+class AnotherBox(models.Model):
+    _name = 'test.another_box'
+    _inherits = {'test.another_unit': 'another_unit_id'}
+    _description = 'Another Test Box'
+
+    another_unit_id = fields.Many2one('test.another_unit', 'Another Unit',
+                                      required=True, ondelete='cascade')
+    val2 = fields.Integer('Value 2', required=True)
+
+    @api.constrains('val1', 'val2')
+    def _check(self):
+        if self.val1 != self.val2:
+            raise ValidationError("The two values must be equals")

odoo/addons/test_inherits/tests/test_inherits.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 from odoo.tests import common
+from odoo.exceptions import ValidationError
 
 
 class test_inherits(common.TransactionCase):
@@ -37,3 +38,37 @@ class test_inherits(common.TransactionCase):
         field = IrModelData.search([('name', '=', 'field_test_box__name')])
         self.assertEqual(len(field), 1)
         self.assertEqual(field.module, 'test_inherits')
+
+    def test_constraint_inherits(self):
+        """Validate constraints on inherits when the parent is not updated"""
+        Model = self.env['test.another_box']
+
+        with self.assertRaises(ValidationError):
+            another_box = Model.create({'val1': 1, 'val2': 2})
+        another_box = Model.create({'val1': 1, 'val2': 1})
+
+        with self.assertRaises(ValidationError):
+            another_box.write({'val2': 2})
+        another_box.write({'val1': 2, 'val2': 2})
+
+    def test_constraint_inherits_parent_change(self):
+        """Validate constraints on inherits when parent is updated too"""
+        UnitModel = self.env['test.another_unit']
+        BoxModel = self.env['test.another_box']
+
+        unit1 = UnitModel.create({'val1': 1})
+        box = BoxModel.create({'another_unit_id': unit1.id, 'val2': 1})
+
+        unit2 = UnitModel.create({'val1': 2})
+        box.write({'another_unit_id': unit2.id, 'val2': 2})
+
+        unit3 = UnitModel.create({'val1': 3})
+        box.write({'another_unit_id': unit3.id, 'val1': 4, 'val2': 4})
+
+        unit4 = UnitModel.create({'val1': 5})
+        with self.assertRaises(ValidationError):
+            box.write({'another_unit_id': unit4.id, 'val2': 6})
+
+        unit5 = UnitModel.create({'val1': 7})
+        with self.assertRaises(ValidationError):
+            box.write({'another_unit_id': unit5.id, 'val1': 8, 'val2': 7})

odoo/models.py

@@ -3281,14 +3281,12 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                             self._name, ', '.join(sorted(unknown_names)))
 
         with self.env.protecting(protected_fields, self):
-            # write stored fields with (low-level) method _write
-            if store_vals or inverse_vals or inherited_vals:
-                # if log_access is enabled, this updates 'write_date' and
-                # 'write_uid' and check access rules, even when old_vals is
-                # empty
-                self._write(store_vals)
+            # update references to parents
+            ref_store_vals = {k: store_vals[k] for k in self._inherits.values() if k in store_vals}
+            if ref_store_vals:
+                self._write(ref_store_vals)
 
-            # update parent records (after possibly updating parent fields)
+            # update parent records
             cr = self.env.cr
             for model_name, parent_vals in inherited_vals.items():
                 parent_name = self._inherits[model_name]
@@ -3301,6 +3299,13 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
 
                 self.env[model_name].browse(parent_ids).write(parent_vals)
 
+            # write stored fields with (low-level) method _write
+            if store_vals or inverse_vals or inherited_vals:
+                # if log_access is enabled, this updates 'write_date' and
+                # 'write_uid' and check access rules, even when old_vals is
+                # empty
+                self._write(store_vals)
+
             if inverse_vals:
                 self.check_field_access_rights('write', list(inverse_vals))