[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 <rco@odoo.com> Co-authored-by: Nasreddin Boulif (bon) <bon@odoo.com>
This commit is contained in:
co-authored by
Nasreddin Boulif
parent
9134358b57
commit
89d259c65d
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user