From 89d259c65da9eef4b13ca72574190a348d208650 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Mon, 11 Mar 2024 15:26:55 +0100 Subject: [PATCH] [FIX] base_automation: double trigger caused by computed field Imagine the following situation: an automated action A is triggered when some stored computed field F has a certain value. When a record is created and no value is given for field F, then the automated action A may be run twice: once when evaluating A's domain forces the computation of F, and once again because A's domain is satisfied. The implementation already uses context flags to reflect which automated actions have already been run, in order to avoid automated actions to be run recursively. The fix consists in enabling those context flags to be shared among the evaluation of the domain and processing of the automated actions. opw-3731182 closes odoo/odoo#157272 Signed-off-by: Raphael Collet Co-authored-by: Nasreddin Boulif (bon) --- .../base_automation/models/base_automation.py | 64 ++++++++++++------- .../models/test_base_automation.py | 14 ++++ .../test_base_automation/tests/test_flow.py | 32 ++++++++++ 3 files changed, 87 insertions(+), 23 deletions(-) diff --git a/addons/base_automation/models/base_automation.py b/addons/base_automation/models/base_automation.py index d52ad54e3bd..ae69079c85c 100644 --- a/addons/base_automation/models/base_automation.py +++ b/addons/base_automation/models/base_automation.py @@ -584,22 +584,30 @@ class BaseAutomation(models.Model): msg = _("Note that this automation rule can be triggered up to %d minutes after its schedule.") self.least_delay_msg = msg % self._get_cron_interval() - def _filter_pre(self, records): + def _filter_pre(self, records, feedback=False): """ Filter the records that satisfy the precondition of automation ``self``. """ self_sudo = self.sudo() if self_sudo.filter_pre_domain and records: + if feedback: + # this context flag enables to detect the executions of + # automations while evaluating their precondition + records = records.with_context(__action_feedback=True) 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 - def _filter_post(self, records): - return self._filter_post_export_domain(records)[0] + def _filter_post(self, records, feedback=False): + return self._filter_post_export_domain(records, feedback)[0] - def _filter_post_export_domain(self, records): + def _filter_post_export_domain(self, records, feedback=False): """ Filter the records that satisfy the postcondition of automation ``self``. """ self_sudo = self.sudo() if self_sudo.filter_domain and records: + if feedback: + # this context flag enables to detect the executions of + # automations while evaluating their postcondition + records = records.with_context(__action_feedback=True) domain = safe_eval.safe_eval(self_sudo.filter_domain, self._get_eval_context()) return records.sudo().filtered_domain(domain).with_env(records.env), domain else: @@ -625,26 +633,36 @@ class BaseAutomation(models.Model): return # mark the remaining records as done (to avoid recursive processing) - automation_done = dict(automation_done) - automation_done[self] = records_done + records - self = self.with_context(__action_done=automation_done) - records = records.with_context(__action_done=automation_done) + if self.env.context.get('__action_feedback'): + # modify the context dict in place: this is useful when fields are + # computed during the pre/post filtering, in order to know which + # automations have already been run by the computation itself + automation_done[self] = records_done + records + else: + automation_done = dict(automation_done) + automation_done[self] = records_done + records + self = self.with_context(__action_done=automation_done) + records = records.with_context(__action_done=automation_done) # modify records if 'date_automation_last' in records._fields: records.date_automation_last = fields.Datetime.now() + # we process the automation on the records for which any watched field + # has been modified, and only mark the automation as done for those + records = records.filtered(self._check_trigger_fields) + automation_done[self] = records_done + records + # prepare the contexts for server actions - contexts = [] - for record in records: - # we process the automation if any watched field has been modified - if self._check_trigger_fields(record): - contexts.append({ - 'active_model': record._name, - 'active_ids': record.ids, - 'active_id': record.id, - 'domain_post': domain_post, - }) + contexts = [ + { + 'active_model': record._name, + 'active_ids': record.ids, + 'active_id': record.id, + 'domain_post': domain_post, + } + for record in records + ] # execute server actions for action in self.sudo().action_server_ids: @@ -703,7 +721,7 @@ class BaseAutomation(models.Model): records = create.origin(self.with_env(automations.env), vals_list, **kw) # check postconditions, and execute actions on the records that satisfy them for automation in automations.with_context(old_values=None): - automation._process(automation._filter_post(records)) + automation._process(automation._filter_post(records, feedback=True)) return records.with_env(self.env) return create @@ -727,7 +745,7 @@ class BaseAutomation(models.Model): write.origin(self.with_env(automations.env), vals, **kw) # check postconditions, and execute actions on the records that satisfy them for automation in automations.with_context(old_values=old_values): - records, domain_post = automation._filter_post_export_domain(pre[automation]) + records, domain_post = automation._filter_post_export_domain(pre[automation], feedback=True) automation._process(records, domain_post=domain_post) return True @@ -760,7 +778,7 @@ class BaseAutomation(models.Model): _compute_field_value.origin(self, field) # check postconditions, and execute automations on the records that satisfy them for automation in automations.with_context(old_values=old_values): - records, domain_post = automation._filter_post_export_domain(pre[automation]) + records, domain_post = automation._filter_post_export_domain(pre[automation], feedback=True) automation._process(records, domain_post=domain_post) return True @@ -774,7 +792,7 @@ class BaseAutomation(models.Model): records = self.with_env(automations.env) # check conditions, and execute actions on the records that satisfy them for automation in automations: - automation._process(automation._filter_post(records)) + automation._process(automation._filter_post(records, feedback=True)) # call original method return unlink.origin(self, **kwargs) @@ -826,7 +844,7 @@ class BaseAutomation(models.Model): mail_trigger = "on_message_received" if message_sudo.author_id.partner_share else "on_message_sent" automations = self.env['base.automation']._get_actions(self, [mail_trigger]) for automation in automations.with_context(old_values=None): - records = automation._filter_pre(self) + records = automation._filter_pre(self, feedback=True) automation._process(records) return message diff --git a/addons/test_base_automation/models/test_base_automation.py b/addons/test_base_automation/models/test_base_automation.py index 18e97818450..ac4fa2b68c2 100644 --- a/addons/test_base_automation/models/test_base_automation.py +++ b/addons/test_base_automation/models/test_base_automation.py @@ -24,6 +24,20 @@ class LeadTest(models.Model): deadline = fields.Boolean(compute='_compute_employee_deadline', store=True) is_assigned_to_admin = fields.Boolean(string='Assigned to admin user') + stage_id = fields.Many2one( + 'test_base_automation.stage', string='Stage', + compute='_compute_stage_id', readonly=False, store=True) + + @api.depends('state') + def _compute_stage_id(self): + Stage = self.env['test_base_automation.stage'] + for task in self: + if not task.stage_id and task.state == 'draft': + task.stage_id = ( + Stage.search([('name', 'ilike', 'new')], limit=1) + or Stage.create({'name': 'New'}) + ) + @api.depends('partner_id.employee', 'priority') def _compute_employee_deadline(self): # this method computes two fields on purpose; don't split it diff --git a/addons/test_base_automation/tests/test_flow.py b/addons/test_base_automation/tests/test_flow.py index 99dd4698a7e..590244fd427 100644 --- a/addons/test_base_automation/tests/test_flow.py +++ b/addons/test_base_automation/tests/test_flow.py @@ -1056,6 +1056,38 @@ class TestCompute(common.TransactionCase): self.assertFalse(obj.active) self.assertEqual(obj.name, "test!") + def test_compute_on_create(self): + lead_model = self.env['ir.model']._get('base.automation.lead.test') + stage_field = self.env['ir.model.fields']._get('base.automation.lead.test', 'stage_id') + new_stage = self.env['test_base_automation.stage'].create({'name': 'New'}) + + create_automation( + self, + model_id=lead_model.id, + trigger='on_stage_set', + trigger_field_ids=[stage_field.id], + _actions={ + 'state': 'object_create', + 'crud_model_id': self.env['ir.model']._get('res.partner').id, + 'value': "Test Partner Automation", + }, + filter_domain=repr([('stage_id', '=', new_stage.id)]), + ) + + # Tricky case: the record is created with 'stage_id' being false, and + # the field is marked for recomputation. The field is then recomputed + # while evaluating 'filter_domain', which causes the execution of the + # automation. And as the domain is satisfied, the automation is + # processed again, but it must detect that it has just been run! + self.env['base.automation.lead.test'].create({ + 'name': 'Test Lead', + }) + + # check that the automation has been run once + partner_count = self.env['res.partner'].search_count([('name', '=', 'Test Partner Automation')]) + self.assertEqual(partner_count, 1, "Only one partner should have been created") + + @common.tagged("post_install", "-at_install") class TestHttp(common.HttpCase): def test_webhook_trigger(self):