From e0297bdac4eac165a79680de3b1139c2f554d5f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Voet=20=28ryv=29?= Date: Fri, 22 Dec 2023 10:37:44 +0100 Subject: [PATCH] [FIX] core: fix onchange when create new one2many record When we add a new record N to an one2many tree view from an existing record X form, during the onchange() on the one2many comodel, the cache of the N.one2many contains only the new record X (the siblings aren't in it). Because of this, the result of compute methods may be incorrect and the form won't be updated accordingly. See https://github.com/odoo/enterprise/pull/52957 for a concrete example. Technically, this is due to _update_cache() forcing the inverse field value to the single value of the new record ("not cache.contains(inv_rec, invf)" is True), instead of also considering the original values (which is properly done by Field._update()). closes odoo/odoo#146778 Related: odoo/enterprise#53298 Signed-off-by: Raphael Collet --- odoo/addons/test_new_api/models/test_new_api.py | 7 +++++++ odoo/addons/test_new_api/tests/test_onchange.py | 16 ++++++++++++++++ odoo/addons/test_new_api/tests/test_views.py | 2 +- .../test_new_api/views/test_new_api_views.xml | 1 + odoo/fields.py | 6 ++---- odoo/models.py | 15 ++++----------- 6 files changed, 31 insertions(+), 16 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 22234f298f3..13485aa9aee 100644 --- a/odoo/addons/test_new_api/models/test_new_api.py +++ b/odoo/addons/test_new_api/models/test_new_api.py @@ -149,12 +149,19 @@ class Message(models.Model): label = fields.Char(translate=True) priority = fields.Integer() active = fields.Boolean(default=True) + has_important_sibling = fields.Boolean(compute='_compute_has_important_sibling') attributes = fields.Properties( string='Properties', definition='discussion.attributes_definition', ) + @api.depends('discussion.messages.important') + def _compute_has_important_sibling(self): + for record in self: + siblings = record.discussion.messages - record + record.has_important_sibling = any(siblings.mapped('important')) + @api.constrains('author', 'discussion') def _check_author(self): for message in self.with_context(active_test=False): diff --git a/odoo/addons/test_new_api/tests/test_onchange.py b/odoo/addons/test_new_api/tests/test_onchange.py index 50674a434bd..2fdd9ba7c43 100644 --- a/odoo/addons/test_new_api/tests/test_onchange.py +++ b/odoo/addons/test_new_api/tests/test_onchange.py @@ -1236,3 +1236,19 @@ class TestComputeOnchange2(common.TransactionCase): ) with Form(record) as form: form.precision_rounding = 0.0001 + + def test_new_one2many_on_existing_record(self): + discussion = self.env['test_new_api.discussion'].create({ + 'messages': [Command.create({'body': 'Required Body', 'important': True})], + 'name': 'Required field', + 'participants': [Command.set(self.env.user.ids)], + }) + + with Form(discussion, 'test_new_api.discussion_form_2') as discussion_form: + self.assertEqual(len(discussion_form.messages), 1) + with discussion_form.messages.new() as message_form: + # this actually checks that during the onchange, the new message + # has access to its sibling messages through the discussion + self.assertEqual(message_form.has_important_sibling, True) + message_form.body = 'Required Body' + self.assertEqual(len(discussion_form.messages), 2) diff --git a/odoo/addons/test_new_api/tests/test_views.py b/odoo/addons/test_new_api/tests/test_views.py index 51cb4705bb9..f45258700ef 100644 --- a/odoo/addons/test_new_api/tests/test_views.py +++ b/odoo/addons/test_new_api/tests/test_views.py @@ -10,7 +10,7 @@ class TestDefaultView(common.TransactionCase): def test_default_form_view(self): self.assertEqual( etree.tostring(self.env['test_new_api.message']._get_default_form_view()), - b'
' + b'
' ) self.assertEqual( etree.tostring(self.env['test_new_api.creativework.edition']._get_default_form_view()), 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 88d45351ce5..2acac075393 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 @@ -128,6 +128,7 @@ + diff --git a/odoo/fields.py b/odoo/fields.py index 20e97e5b0f6..51b0ab805e6 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -1450,7 +1450,6 @@ class Integer(Field): return value def _update(self, records, value): - # special case, when an integer field is used as inverse for a one2many cache = records.env.cache for record in records: cache.set(record, self, value.id or 0) @@ -4154,9 +4153,8 @@ class _RelationalMulti(_Relational): if value: cache = records.env.cache for record in records: - if cache.contains(record, self): - val = self.convert_to_cache(record[self.name] | value, record, validate=False) - cache.set(record, self, val) + val = self.convert_to_cache(record[self.name] | value, record, validate=False) + cache.set(record, self, val) records.modified([self.name]) def convert_to_cache(self, value, record, validate=True): diff --git a/odoo/models.py b/odoo/models.py index 944028f929b..4b39e7554b0 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -5973,7 +5973,7 @@ class BaseModel(metaclass=MetaModel): cache = self.env.cache fields = self._fields try: - field_values = [(fields[name], value) for name, value in values.items()] + field_values = [(fields[name], value) for name, value in values.items() if name != 'id'] except KeyError as e: raise ValueError("Invalid field %r on model %r" % (e.args[0], self._name)) @@ -5987,17 +5987,10 @@ class BaseModel(metaclass=MetaModel): inv_recs = self[field.name].filtered(lambda r: not r.id) if not inv_recs: continue + # we need to adapt the value of the inverse fields to integrate self into it: + # x2many fields should add self, while many2one fields should replace with self for invf in self.pool.field_inverses[field]: - # DLE P98: `test_40_new_fields` - # /home/dle/src/odoo/master-nochange-fp/odoo/addons/test_new_api/tests/test_new_fields.py - # Be careful to not break `test_onchange_taxes_1`, `test_onchange_taxes_2`, `test_onchange_taxes_3` - # If you attempt to find a better solution - for inv_rec in inv_recs: - if not cache.contains(inv_rec, invf): - val = invf.convert_to_cache(self, inv_rec, validate=False) - cache.set(inv_rec, invf, val) - else: - invf._update(inv_rec, self) + invf._update(inv_recs, self) def _convert_to_record(self, values): """ Convert the ``values`` dictionary from the cache format to the