From 6f2a6f7f2ec3ebdb6bfc4a45534dbc78298998ff Mon Sep 17 00:00:00 2001 From: Laurent Smet Date: Fri, 20 Nov 2020 08:00:31 +0000 Subject: [PATCH] [FIX] core: invalidation of computed editable field in one2many This fixes the issue introduced by revision fd50ba9fb838067fab0b920470cf79b9c94d785c The way computed fields are invalidated depends on the order of the dict passed as a parameter to method onchange(). This causes some unexpected behavior when dealing with computed editable fields: the user loses the value in that field because of the onchange. Explanation: during the onchange, the modified field is cached by record._update_cache(changed_values, validate=True) If the field is a one2many, 'value' contains an update command for each modified line. Because of 'validate=True', the assignment triggers field computations on the lines, which are already handled by another call to onchange(). In case anything on the parent model modifies some line, the whole one2many values are returned to the user, and accidentally recomputed fields show up in the interface. closes odoo/odoo#62748 Signed-off-by: Raphael Collet (rco) --- .../test_new_api/models/test_new_api.py | 32 ++++++++++++++++ .../test_new_api/security/ir.model.access.csv | 2 + .../test_new_api/tests/test_new_fields.py | 2 +- .../test_new_api/tests/test_onchange.py | 38 ++++++++++++++++++- .../test_new_api/views/test_new_api_views.xml | 26 +++++++++++++ odoo/fields.py | 5 ++- odoo/models.py | 2 +- 7 files changed, 102 insertions(+), 5 deletions(-) diff --git a/odoo/addons/test_new_api/models/test_new_api.py b/odoo/addons/test_new_api/models/test_new_api.py index 147cf78188c..3041daeadc0 100644 --- a/odoo/addons/test_new_api/models/test_new_api.py +++ b/odoo/addons/test_new_api/models/test_new_api.py @@ -1147,3 +1147,35 @@ class ComputeMember(models.Model): container = self.env['test_new_api.compute.container'] for member in self: member.container_id = container.search([('name', '=', member.name)], limit=1) + + +class ComputeEditable(models.Model): + _name = _description = 'test_new_api.compute_editable' + + line_ids = fields.One2many('test_new_api.compute_editable.line', 'parent_id') + + @api.onchange('line_ids') + def _onchange_line_ids(self): + for line in self.line_ids: + # even if 'same' is not in the view, it should be the same as 'value' + line.count += line.same + + +class ComputeEditableLine(models.Model): + _name = _description = 'test_new_api.compute_editable.line' + + parent_id = fields.Many2one('test_new_api.compute_editable') + value = fields.Integer() + same = fields.Integer(compute='_compute_same', store=True) + edit = fields.Integer(compute='_compute_edit', store=True, readonly=False) + count = fields.Integer() + + @api.depends('value') + def _compute_same(self): + for line in self: + line.same = line.value + + @api.depends('value') + def _compute_edit(self): + for line in self: + line.edit = line.value diff --git a/odoo/addons/test_new_api/security/ir.model.access.csv b/odoo/addons/test_new_api/security/ir.model.access.csv index 71a309d5d1a..021d7127b05 100644 --- a/odoo/addons/test_new_api/security/ir.model.access.csv +++ b/odoo/addons/test_new_api/security/ir.model.access.csv @@ -66,3 +66,5 @@ access_test_new_api_model_shared_cache_compute_parent,access_test_new_api.model_ access_test_new_api_model_shared_cache_compute_line,access_test_new_api.model_shared_cache_compute_line,model_test_new_api_model_shared_cache_compute_line,,1,1,1,1 access_test_new_api_compute_container,access_test_new_api_compute_container,model_test_new_api_compute_container,,1,1,1,1 access_test_new_api_compute_member,access_test_new_api_compute_member,model_test_new_api_compute_member,,1,1,1,1 +access_test_new_api_compute_editable,access_test_new_api_compute_editable,model_test_new_api_compute_editable,,1,1,1,1 +access_test_new_api_compute_editable_line,access_test_new_api_compute_editable_line,model_test_new_api_compute_editable_line,,1,1,1,1 diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py index 634da243664..76be6c6f9b5 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -1338,7 +1338,7 @@ class TestFields(TransactionCaseWithUserDemo): new_origin = Model.new({'name': 'Bar'}, origin=real_record) new_record = Model.new({'name': 'Baz'}) self.assertEqual(real_record.display_name, 'Foo') - self.assertEqual(new_origin.display_name, 'Foo') + self.assertEqual(new_origin.display_name, 'Bar') self.assertEqual(new_record.display_name, 'Baz') # computed stored field with recomputation: always computed diff --git a/odoo/addons/test_new_api/tests/test_onchange.py b/odoo/addons/test_new_api/tests/test_onchange.py index 62da8a1a8b0..c417ef9dba5 100644 --- a/odoo/addons/test_new_api/tests/test_onchange.py +++ b/odoo/addons/test_new_api/tests/test_onchange.py @@ -4,7 +4,7 @@ from unittest.mock import patch from odoo.addons.base.tests.common import SavepointCaseWithUserDemo -from odoo.tests import common +from odoo.tests import common, Form from odoo import Command def strip_prefix(prefix, names): @@ -752,3 +752,39 @@ class TestComputeOnchange(common.TransactionCase): with form.child_ids.edit(2) as line: line.cost = 30 self.assertEqual(form.cost, 61) + + def test_onchange_editable_compute_one2many(self): + # create a record with a computed editable field ('edit') on lines + record = self.env['test_new_api.compute_editable'].create({'line_ids': [(0, 0, {'value': 7})]}) + record.flush() + line = record.line_ids + self.assertRecordValues(line, [{'value': 7, 'edit': 7, 'count': 0}]) + + # retrieve the onchange spec for calling 'onchange' + spec = Form(record)._view['onchange'] + + # The onchange on 'line_ids' should increment 'count' and keep the value + # of 'edit' (this field should not be recomputed), whatever the order of + # the fields in the dictionary. This ensures that the value set by the + # user on a computed editable field on a line is not lost. + line_ids = [ + Command.update(line.id, {'value': 8, 'edit': 9, 'count': 0}), + Command.create({'value': 8, 'edit': 9, 'count': 0}), + ] + result = record.onchange({'line_ids': line_ids}, 'line_ids', spec) + expected = {'value': { + 'line_ids': [ + Command.clear(), + Command.update(line.id, {'value': 8, 'edit': 9, 'count': 8}), + Command.create({'value': 8, 'edit': 9, 'count': 8}), + ], + }} + self.assertEqual(result, expected) + + # change dict order in lines, and try again + line_ids = [ + (op, id_, dict(reversed(list(vals.items())))) + for op, id_, vals in line_ids + ] + result = record.onchange({'line_ids': line_ids}, 'line_ids', spec) + self.assertEqual(result, expected) diff --git a/odoo/addons/test_new_api/views/test_new_api_views.xml b/odoo/addons/test_new_api/views/test_new_api_views.xml index 7fbce13b316..56e6a773b28 100644 --- a/odoo/addons/test_new_api/views/test_new_api_views.xml +++ b/odoo/addons/test_new_api/views/test_new_api_views.xml @@ -341,5 +341,31 @@ + + + + test_new_api.compute_editable.form + test_new_api.compute_editable + +
+ + + + + + + + + + + + + + +
+ + + + diff --git a/odoo/fields.py b/odoo/fields.py index 4758b4949fd..e1417bb6017 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -961,7 +961,8 @@ class Field(MetaField('DummyField', (object,), {})): # not stored and not computed -> default # # on a new record w/ origin: - # stored -> fetch from origin (computation done above) + # stored and not (computed and readonly) -> fetch from origin + # stored and computed and readonly -> compute # not stored and computed -> compute # not stored and not computed -> default # @@ -987,7 +988,7 @@ class Field(MetaField('DummyField', (object,), {})): ])) value = env.cache.get(record, self) - elif self.store and record._origin: + elif self.store and record._origin and not (self.compute and self.readonly): # new record with origin: fetch from origin value = self.convert_to_cache(record._origin[self.name], record) env.cache.set(record, self, value) diff --git a/odoo/models.py b/odoo/models.py index c9071c269dd..bfe6f3d09cd 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -6132,7 +6132,7 @@ Fields: # store changed values in cache; also trigger recomputations based on # subfields (e.g., line.a has been modified, line.b is computed stored # and depends on line.a, but line.b is not in the form view) - record._update_cache(changed_values, validate=True) + record._update_cache(changed_values, validate=False) # update snapshot0 with changed values for name in names: