[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#67953

X-original-commit: e09a44db6a04cd37ad6f28b859f927970ac93fc5
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>

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')