From 27e4d3bb1c33e2f070d2289833f4ba741e8d8723 Mon Sep 17 00:00:00 2001 From: Julien Castiaux Date: Mon, 29 Apr 2019 08:07:42 +0000 Subject: [PATCH] [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) --- odoo/addons/test_inherits/ir.model.access.csv | 2 ++ odoo/addons/test_inherits/models.py | 26 ++++++++++++++ .../test_inherits/tests/test_inherits.py | 35 +++++++++++++++++++ odoo/models.py | 19 ++++++---- 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/odoo/addons/test_inherits/ir.model.access.csv b/odoo/addons/test_inherits/ir.model.access.csv index 22ecf366e31..09aeec489d8 100644 --- a/odoo/addons/test_inherits/ir.model.access.csv +++ b/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 diff --git a/odoo/addons/test_inherits/models.py b/odoo/addons/test_inherits/models.py index e31ebcdfff0..efa965c2653 100644 --- a/odoo/addons/test_inherits/models.py +++ b/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") diff --git a/odoo/addons/test_inherits/tests/test_inherits.py b/odoo/addons/test_inherits/tests/test_inherits.py index 85396de5441..3328f23e2ed 100644 --- a/odoo/addons/test_inherits/tests/test_inherits.py +++ b/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}) diff --git a/odoo/models.py b/odoo/models.py index 4af2f7198e1..6914027a7a9 100644 --- a/odoo/models.py +++ b/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))