From 4ef97f21257a6e647d3bcefece423e59dc7bbf43 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Wed, 3 Mar 2021 12:21:58 +0000 Subject: [PATCH] [FIX] base_automation: mis-ordered computation of stored fields Client issue: when updating the company of a contact, the Display Name keeps using the previous company's name, so given Bob in company A, if Bob is moved to company B the form's title remains "A, Bob" instead of becoming "B, Bob". More annoying, if Bob is moved back to A the name becomes "B, Bob". On res.partner, `display_name` is a stored computed field which depends on `commercial_company_name` (via `name_get` -> `_get_name` -> `_get_contact_name`). This is an other stored computed name, which depends on `commercial_partner_id`, which is yet another stored computed name, which depends on the `parent_id`. The dependencies are meh but usually resolve fine, the issue occurs when a base.automation rule is created with a non-empty domain (including an empty literal list, which was the case here): when the first field of the sequence is computed, base.automation's `_compute_field_value` is called. This calls `_filter_pre`, which (because `filter_pre_domain` is non-empty) calls `search` on the model. This would normally be innocuous as `search` will only flush the fields used in the search, however for `res.partner` the default `_order` is... `display_name`. Meaning we flush that computation, forcing the computation of `commercial_company_name`, but since `commercial_partner_id` is being computed we reuse its old value (or something), which is not re-recomputed after the `commercial_partner_id` computation ends. So rather than resolve a full search involving an order, filter the records in-place. OPW-2427264 closes odoo/odoo#67987 X-original-commit: 221ea6079b2beeb66c4cfa48b237791e1b380c5e Signed-off-by: Raphael Collet (rco) Signed-off-by: Xavier Morel (xmo) --- .../base_automation/models/base_automation.py | 4 +- .../test_base_automation/tests/test_flow.py | 52 ++++++++++++++++++- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/addons/base_automation/models/base_automation.py b/addons/base_automation/models/base_automation.py index 201b16417f7..9c596578261 100644 --- a/addons/base_automation/models/base_automation.py +++ b/addons/base_automation/models/base_automation.py @@ -206,8 +206,8 @@ class BaseAutomation(models.Model): """ Filter the records that satisfy the precondition of action ``self``. """ self_sudo = self.sudo() if self_sudo.filter_pre_domain and records: - domain = [('id', 'in', records.ids)] + safe_eval.safe_eval(self_sudo.filter_pre_domain, self._get_eval_context()) - return records.sudo().search(domain).with_env(records.env) + domain = safe_eval.safe_eval(self_sudo.filter_pre_domain, self._get_eval_context()) + return records.sudo().filtered_domain(domain).with_env(records.env) else: return records diff --git a/addons/test_base_automation/tests/test_flow.py b/addons/test_base_automation/tests/test_flow.py index 780a037eadd..107b167a838 100644 --- a/addons/test_base_automation/tests/test_flow.py +++ b/addons/test_base_automation/tests/test_flow.py @@ -4,7 +4,7 @@ from unittest.mock import patch from odoo.addons.base.tests.common import TransactionCaseWithUserDemo -from odoo.tests import tagged +from odoo.tests import common, tagged from odoo.exceptions import AccessError @@ -354,3 +354,53 @@ record['name'] = record.name + 'X'""", rec3.write({'name': 'a first record'}) rec4 = Model.with_user(self.user_demo).create({'name': 'again another record'}) rec4.write({'name': 'another value'}) + +@common.tagged('post_install','-at_install') +class TestCompute(common.TransactionCase): + def test_inversion(self): + """ If a stored field B depends on A, an update to the trigger for A + should trigger the recomputaton of A, then B. + + However if a search() is performed during the computation of A + ??? and _order is affected ??? a flush will be triggered, forcing the + computation of B, based on the previous A. + + This happens if a rule has has a non-empty filter_pre_domain, even if + it's an empty list (``'[]'`` as opposed to ``False``). + """ + company1 = self.env['res.partner'].create({ + 'name': "Gorofy", + 'is_company': True, + }) + company2 = self.env['res.partner'].create({ + 'name': "Awiclo", + 'is_company': True + }) + r = self.env['res.partner'].create({ + 'name': 'Bob', + 'is_company': False, + 'parent_id': company1.id + }) + self.assertEqual(r.display_name, 'Gorofy, Bob') + r.parent_id = company2 + self.assertEqual(r.display_name, 'Awiclo, Bob') + + self.env['base.automation'].create({ + 'name': "test rule", + 'filter_pre_domain': False, + 'trigger': 'on_create_or_write', + 'state': 'code', # no-op action + 'model_id': self.env.ref('base.model_res_partner').id, + }) + r.parent_id = company1 + self.assertEqual(r.display_name, 'Gorofy, Bob') + + self.env['base.automation'].create({ + 'name': "test rule", + 'filter_pre_domain': '[]', + 'trigger': 'on_create_or_write', + 'state': 'code', # no-op action + 'model_id': self.env.ref('base.model_res_partner').id, + }) + r.parent_id = company2 + self.assertEqual(r.display_name, 'Awiclo, Bob')