From 253c214b5b76a13f2ccbf7c4d20142219967cc7f Mon Sep 17 00:00:00 2001 From: Damien Bouvy Date: Wed, 4 Oct 2023 00:37:02 +0200 Subject: [PATCH] [IMP] base: allow server actions to update records across relations Up until now, the 'Update record' server actions could only update the record on which the action was triggered. This is a rather serious limitations, especially in modules that leverage server actions, like base automation - indeed, if you want a server action run on e.g. Leads that will write changes on the leads' customer, you *had* to use Python code. This commit makes it possible to traverse relations when selecting the field to update, and indeed to write across relations. Task-3450200 Part-of: odoo/odoo#138804 --- .../base_automation/tests/test_automation.py | 5 +- .../src/views/form/form_controller.scss | 4 + odoo/addons/base/models/ir_actions.py | 133 ++++++++++++++---- odoo/addons/base/tests/test_ir_actions.py | 48 ++++++- odoo/addons/base/views/ir_actions_views.xml | 6 +- 5 files changed, 162 insertions(+), 34 deletions(-) diff --git a/addons/base_automation/tests/test_automation.py b/addons/base_automation/tests/test_automation.py index 9ce5ea10046..71ec7c04113 100644 --- a/addons/base_automation/tests/test_automation.py +++ b/addons/base_automation/tests/test_automation.py @@ -22,7 +22,7 @@ class TestAutomation(TransactionCase): "name": "Set Active To False", "base_automation_id": automation.id, "state": "object_write", - "update_field_id": self.env.ref("base.field_res_partner__active").id, + "update_path": "active", "value": False, "model_id": model.id, }) @@ -51,9 +51,10 @@ class TestAutomation(TransactionCase): "base_automation_id": automation.id, "model_id": model.id, "state": "object_write", - "update_field_id": self.env.ref("base.field_ir_filters__active").id, + "update_path": "active", "value": False }) + action.flush_recordset() automation.write({"action_server_ids": [Command.link(action.id)]}) # action cached was cached with admin, force CacheMiss automation.env.clear() diff --git a/addons/web/static/src/views/form/form_controller.scss b/addons/web/static/src/views/form/form_controller.scss index 795f6b21048..92ead18ebd1 100644 --- a/addons/web/static/src/views/form/form_controller.scss +++ b/addons/web/static/src/views/form/form_controller.scss @@ -695,6 +695,10 @@ } } + .o_field_DynamicModelFieldSelectorChar.oe_inline { + min-width: 7rem; + } + // One2Many, Many2Many outside of group .o_field_widget { &.o_field_one2many, &.o_field_many2many { diff --git a/odoo/addons/base/models/ir_actions.py b/odoo/addons/base/models/ir_actions.py index 87bd638ed68..4c50c8da4d2 100644 --- a/odoo/addons/base/models/ir_actions.py +++ b/odoo/addons/base/models/ir_actions.py @@ -11,8 +11,9 @@ from odoo.http import request import base64 from collections import defaultdict -import functools +from functools import partial, reduce import logging +from operator import getitem from pytz import timezone @@ -439,6 +440,18 @@ class IrActionsServer(models.Model): # - Command: x2many commands namespace # To return an action, assign: action = {...}\n\n\n\n""" + @api.model + def _default_update_path(self): + if not self.env.context.get('default_model_id'): + return '' + ir_model = self.env['ir.model'].browse(self.env.context['default_model_id']) + model = self.env[ir_model.model] + sensible_default_fields = ['partner_id', 'user_id', 'user_ids', 'stage_id', 'state', 'active'] + for field_name in sensible_default_fields: + if field_name in model._fields: + return field_name + return '' + name = fields.Char(compute='_compute_name', store=True, readonly=False, required=True) type = fields.Char(default='ir.actions.server') usage = fields.Selection([ @@ -479,7 +492,7 @@ class IrActionsServer(models.Model): # Create crud_model_id = fields.Many2one( 'ir.model', string='Record to Create', - compute='_compute_crud_model_id', readonly=False, store=True, + compute='_compute_crud_relations', readonly=False, store=True, help="Specify which kind of record should be created. Set this field only to specify a different model than the base model.") crud_model_name = fields.Char(related='crud_model_id.model', string='Target Model Name', readonly=True) link_field_id = fields.Many2one( @@ -489,8 +502,9 @@ class IrActionsServer(models.Model): groups_id = fields.Many2many('res.groups', 'ir_act_server_group_rel', 'act_id', 'gid', string='Allowed Groups', help='Groups that can execute the server action. Leave empty to allow everybody.') - update_field_id = fields.Many2one('ir.model.fields', string='Field to Update', ondelete='cascade') - update_related_model_id = fields.Many2one('ir.model', compute='_compute_update_related_model_id') + update_field_id = fields.Many2one('ir.model.fields', string='Field to Update', ondelete='cascade', compute='_compute_crud_relations', store=True, readonly=False) + update_path = fields.Char(string='Field to Update Path', help="Path to the field to update, e.g. 'partner_id.name'", default=_default_update_path) + update_related_model_id = fields.Many2one('ir.model', compute='_compute_crud_relations', store=True) value = fields.Text(help="For Python expressions, this field may hold a Python expression " "that can use the same values as for the code field on the server action," @@ -517,10 +531,8 @@ class IrActionsServer(models.Model): def _compute_name(self): for action in self.filtered('state'): if action.state == 'object_write': - action.name = _( - "Update %(field_name)s", - field_name=action.update_field_id.field_description - ) + + action.name = _("Update %s", action._stringify_path()) elif action.state == 'object_create': action.name = _( "Create %(model_name)s with name %(value)s", @@ -537,11 +549,91 @@ class IrActionsServer(models.Model): ) self.available_model_ids = allowed_models.ids - @api.onchange('model_id') - def _compute_crud_model_id(self): - invalid = self.filtered(lambda act: act.crud_model_id != act.model_id) - if invalid: - invalid.crud_model_id = False + @api.depends('model_id', 'update_path', 'state') + def _compute_crud_relations(self): + """ Compute the crud_model_id and update_field_id fields. + + The crud_model_id is the model on which the action will create or update + records. In the case of record creation, it is the same as the main model + of the action. For record update, it will be the model linked to the last + field in the update_path. + This is only used for object_create and object_write actions. + The update_field_id is the field at the end of the update_path that will + be updated by the action - only used for object_write actions. + """ + for action in self: + if action.model_id and action.state in ('object_write', 'object_create'): + if action.state == 'object_create': + action.crud_model_id = action.model_id + action.update_field_id = False + action.update_path = False + elif action.state == 'object_write': + if action.update_path: + # we need to traverse relations to find the target model and field + model, field, _ = action._traverse_path() + action.crud_model_id = model + action.update_field_id = field + need_update_model = action.evaluation_type == 'value' and action.update_field_id and action.update_field_id.relation + action.update_related_model_id = action.env["ir.model"]._get_id(field.relation) if need_update_model else False + else: + action.crud_model_id = action.model_id + action.update_field_id = False + else: + action.crud_model_id = False + action.update_field_id = False + action.update_path = False + + def _traverse_path(self, record=None): + """ Traverse the update_path to find the target model and field, and optionally + the target record of an action of type 'object_write'. + + :param record: optional record to use as starting point for the path traversal + :return: a tuple (model, field, records) where model is the target model and field is the + target field; if no record was provided, records is None, otherwise it is the + recordset at the end of the path starting from the provided record + """ + self.ensure_one() + path = self.update_path.split('.') + Model = self.env[self.model_id.model] + # sanity check: we're starting from a record that belongs to the model + if record and record._name != Model._name: + raise ValidationError(_("I have no idea how you *did that*, but you're trying to use a gibberish configuration: the model of the record on which the action is triggered is not the same as the model of the action.")) + for field_name in path: + is_last_field = field_name == path[-1] + field = Model._fields[field_name] + if field.relational and not is_last_field: + Model = self.env[field.comodel_name] + elif not field.relational: + # sanity check: this should be the last field in the path + if not is_last_field: + raise ValidationError(_("The path to the field to update contains a non-relational field (%s) that is not the last field in the path. You can't traverse non-relational fields (even in the quantum realm). Make sure only the last field in the path is non-relational.", field_name)) + if isinstance(field, fields.Json): + raise ValidationError(_("I'm sorry to say that JSON fields (such as %s) are currently not supported.", field_name)) + elif field.readonly: + raise ValidationError(_("The field to update (%s) is read-only. You can't update a read-only field - even when asking nicely.", field_name)) + target_records = None + if record is not None: + target_records = reduce(getitem, path[:-1], record) + model_id = self.env['ir.model']._get(Model._name) + field_id = self.env['ir.model.fields']._get(Model._name, field_name) + return model_id, field_id, target_records + + def _stringify_path(self): + """ Returns a string representation of the update_path, with the field names + separated by the `>` symbol.""" + self.ensure_one() + path = self.update_path + if not path: + return '' + model = self.env[self.model_id.model] + pretty_path = [] + for field_name in path.split('.'): + field = model._fields[field_name] + field_id = self.env['ir.model.fields']._get(model._name, field_name) + if field.relational: + model = self.env[field.comodel_name] + pretty_path.append(field_id.field_description) + return ' > '.join(pretty_path) @api.depends('model_id') def _compute_link_field_id(self): @@ -576,7 +668,7 @@ class IrActionsServer(models.Model): fn = getattr(t, f'_run_action_{self.state}', None)\ or getattr(t, f'run_action_{self.state}', None) if fn and fn.__name__.startswith('run_action_'): - fn = functools.partial(fn, self) + fn = partial(fn, self) return fn, multi def _register_hook(self): @@ -623,7 +715,9 @@ class IrActionsServer(models.Model): for field, new_value in res.items(): record_cached[field] = new_value else: - self.env[self.model_id.model].browse(self._context.get('active_id')).write(res) + starting_record = self.env[self.model_id.model].browse(self._context.get('active_id')) + _, _, target_records = self._traverse_path(record=starting_record) + target_records.write(res) def _run_action_object_create(self, eval_context=None): """Create specified model object with specified name contained in value. @@ -756,15 +850,6 @@ class IrActionsServer(models.Model): ) return res or False - @api.depends('update_field_id') - def _compute_update_related_model_id(self): - for action in self: - if action.evaluation_type == 'value' and action.update_field_id and action.update_field_id.relation: - relation = action.update_field_id.relation - action.update_related_model_id = action.env["ir.model"]._get_id(relation) - else: - action.update_related_model_id = False - @api.depends('evaluation_type', 'update_field_id') def _compute_value_field_to_show(self): # check if value_field_to_show can be removed and use ttype in xml view instead for action in self: diff --git a/odoo/addons/base/tests/test_ir_actions.py b/odoo/addons/base/tests/test_ir_actions.py index 4121affa172..37787680664 100644 --- a/odoo/addons/base/tests/test_ir_actions.py +++ b/odoo/addons/base/tests/test_ir_actions.py @@ -189,7 +189,7 @@ ZeroDivisionError: division by zero""" % self.test_server_action.id # Do: update partner name self.action.write({ 'state': 'object_write', - 'update_field_id': self.res_partner_name_field.id, + 'update_path': 'name', 'value': 'TestNew', }) run_res = self.action.with_context(self.context).run() @@ -209,7 +209,7 @@ ZeroDivisionError: division by zero""" % self.test_server_action.id 'model_id': self.res_country_model.id, 'model_name': 'res.country', 'state': 'object_write', - 'update_field_id': self.res_country_name_position_field.id, + 'update_path': 'name_position', 'selection_value': selection_value.id, }) action._set_selection_value() # manual onchange @@ -223,6 +223,47 @@ ZeroDivisionError: division by zero""" % self.test_server_action.id # Test: country updated self.assertEqual(self.test_country.name_position, 'after') + def test_37_field_path_traversal(self): + """ Test the update_path field traversal - allowing records to be updated along relational links """ + # update the country's name via the partner + self.action.write({ + 'state': 'object_write', + 'update_path': 'country_id.name', + 'value': 'TestUpdatedCountry', + }) + run_res = self.action.with_context(self.context).run() + self.assertFalse(run_res, 'ir_actions_server: update record action correctly finished should return False') + # Test: partner updated + self.assertEqual(self.test_partner.country_id.name, 'TestUpdatedCountry', 'ir_actions_server: country name should have been updated through relation') + + # input an invalid path + with self.assertRaises(ValidationError): + self.action.write({ + 'state': 'object_write', + 'update_path': 'country_id.name.foo', + 'value': 'DoesNotMatter', + }) + self.action.flush_recordset(['update_path', 'update_field_id']) + + # update a readonly field + self.action.write({ + 'state': 'object_write', + 'update_path': 'country_id.id', + 'value': 0, + }) + self.action.flush_recordset(['update_path', 'update_field_id']) + + def test_38_field_traversal_name_compute(self): + """ Test the automated computed name of an 'update record' action """ + # update the country's name via the partner + action_with_name = self.action.with_context(automatic_action_name=True) + action_with_name.write({ + 'state': 'object_write', + 'update_path': 'country_id.name', + 'value': 'TestUpdatedCountry', + }) + self.assertEqual(action_with_name.name, 'Update Country > Country Name', 'ir_actions_server: name should have been updated automatically') + @mute_logger('odoo.addons.base.models.ir_model', 'odoo.models') def test_40_multi(self): # Data: 2 server actions that will be nested @@ -245,9 +286,8 @@ ZeroDivisionError: division by zero""" % self.test_server_action.id 'name': 'Subaction2', 'sequence': 3, 'model_id': self.res_partner_model.id, - 'crud_model_id': self.res_partner_model.id, 'state': 'object_write', - 'update_field_id': self.res_partner_city_field.id, + 'update_path': 'city', 'value': 'RaoulettePoiluchette', }) action4 = self.action.create({ diff --git a/odoo/addons/base/views/ir_actions_views.xml b/odoo/addons/base/views/ir_actions_views.xml index 161134f9281..695872f0146 100644 --- a/odoo/addons/base/views/ir_actions_views.xml +++ b/odoo/addons/base/views/ir_actions_views.xml @@ -342,10 +342,8 @@
- + + to