[MERGE][FIX] base(_automation), mail, sms: fix automation / server actions usage and model sync

Fix various issues after last refactoring of base automation. Various usability issues arose when trying to configure some server actions and automated actions during some tests. Fixes mainly cover model check on server action, as well as checks and synchronization with automated rule model. Some additional checks are added. See sub commits for more details. Followup of odoo/odoo#114352 Task-3527758 (Base Automation Refactor Fiximp) Part of Task-3527752 (Mail: The Pre-Major Freeze FixImpLint) closes odoo/odoo#137133 Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
2023-10-02 06:54:38 +00:00
parent a92e88d547 42958e2f5e
commit 4c0de8db17
14 changed files with 194 additions and 111 deletions
@@ -19,6 +19,7 @@ trigger an automatic reminder email.
    'data': [
        'security/ir.model.access.csv',
        'data/base_automation_data.xml',
+        'views/ir_actions_server_views.xml',
        'views/base_automation_views.xml',
    ],
    'assets': {
@@ -8,7 +8,7 @@ from collections import defaultdict

 from dateutil.relativedelta import relativedelta

-from odoo import _, api, Command, exceptions, fields, models
+from odoo import _, api, exceptions, fields, models
 from odoo.tools import DEFAULT_SERVER_DATETIME_FORMAT
 from odoo.tools import safe_eval

@@ -177,6 +177,19 @@ class BaseAutomation(models.Model):
    CRITICAL_FIELDS = ['model_id', 'active', 'trigger', 'on_change_field_ids']
    RANGE_FIELDS = ['trg_date_range', 'trg_date_range_type']

+    @api.constrains('model_id', 'action_server_ids')
+    def _check_action_server_model(self):
+        for rule in self:
+            failing_actions = rule.action_server_ids.filtered(
+                lambda action: action.model_id != rule.model_id
+            )
+            if failing_actions:
+                raise exceptions.ValidationError(
+                    _('Target model of actions %(action_names)s are different from rule model.',
+                      action_names=', '.join(failing_actions.mapped('name'))
+                     )
+                )
+
    @api.constrains('trigger', 'action_server_ids')
    def _check_trigger_state(self):
        for record in self:
@@ -196,7 +209,14 @@ class BaseAutomation(models.Model):

    @api.depends('model_id')
    def _compute_action_server_ids(self):
-        self.action_server_ids = [Command.clear()]
+        """ When changing / setting model, remove actions that are not targeting
+        the same model anymore. """
+        for rule in self.filtered('model_id'):
+            actions_to_remove = rule.action_server_ids.filtered(
+                lambda action: action.model_id != rule.model_id
+            )
+            if actions_to_remove:
+                rule.action_server_ids = [(3, action.id) for action in actions_to_remove]

    @api.depends('trigger', 'trigger_field_ids')
    def _compute_trg_date_id(self):
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.

-from odoo import fields, models
+from odoo import api, exceptions, fields, models, _


 class ServerAction(models.Model):
@@ -11,3 +11,23 @@ class ServerAction(models.Model):
        ('base_automation', 'Automation Rule')
    ], ondelete={'base_automation': 'cascade'})
    base_automation_id = fields.Many2one('base.automation', string='Automation Rule', ondelete='cascade')
+
+    @api.constrains('model_id', 'base_automation_id')
+    def _check_model_coherency_with_automation(self):
+        for action in self.filtered('base_automation_id'):
+            if action.model_id != action.base_automation_id.model_id:
+                raise exceptions.ValidationError(
+                    _("Model of action %(action_name)s should match the one from automated rule %(rule_name)s.",
+                      action_name=action.name,
+                      rule_name=action.base_automation_id.name
+                     )
+                )
+
+    @api.depends('usage')
+    def _compute_available_model_ids(self):
+        """ Stricter model limit: based on automation rule """
+        super()._compute_available_model_ids()
+        rule_based = self.filtered(lambda action: action.usage == 'base_automation')
+        for action in rule_based:
+            rule_model = action.base_automation_id.model_id
+            action.available_model_ids = rule_model.ids if rule_model in action.available_model_ids else []
@@ -88,7 +88,7 @@
                                    name="action_server_ids"
                                    widget="one2many"
                                    class="o_base_automation_actions_field"
-                                    context="{'default_model_id': model_id, 'default_state': False, 'automatic_action_name': True, 'form_view_ref': 'base_automation.view_server_action_form'}"
+                                    context="{'default_model_id': model_id, 'default_state': False, 'form_view_ref': 'base_automation.ir_actions_server_view_form_automation'}"
                                >
                                    <kanban>
                                        <control>
@@ -132,28 +132,6 @@
            </field>
        </record>

-        <!-- Automation: Server Action Form View -->
-        <record id="view_server_action_form" model="ir.ui.view">
-            <field name="name">base.automation.server.action.form</field>
-            <field name="model">ir.actions.server</field>
-            <field name="mode">primary</field>
-            <field name="inherit_id" ref="base.view_server_action_form"/>
-            <field name="arch" type="xml">
-                <xpath expr='//field[@name="name"]' position="attributes">
-                    <attribute name="invisible">1</attribute>
-                </xpath>
-                <xpath expr='//field[@name="state"]' position="attributes">
-                    <attribute name="string">Type</attribute>
-                </xpath>
-                <xpath expr="//header" position="attributes">
-                    <attribute name="invisible">1</attribute>
-                </xpath>
-                <xpath expr="//field[@name='model_id']" position="attributes">
-                    <attribute name="invisible">1</attribute>
-                </xpath>
-            </field>
-        </record>
-
        <!-- automation Tree View -->
        <record id="view_base_automation_tree" model="ir.ui.view">
            <field name="name">base.automation.tree</field>
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="utf-8"?>
+<odoo>
+    <!-- Automation: Server Action Form View -->
+    <record id="ir_actions_server_view_form_automation" model="ir.ui.view">
+        <field name="name">ir.actions.server.view.form.automation</field>
+        <field name="model">ir.actions.server</field>
+        <field name="mode">primary</field>
+        <field name="priority">32</field>
+        <field name="inherit_id" ref="base.view_server_action_form"/>
+        <field name="arch" type="xml">
+            <xpath expr='//field[@name="name"]' position="attributes">
+                <attribute name="invisible">1</attribute>
+            </xpath>
+            <xpath expr='//field[@name="state"]' position="attributes">
+                <attribute name="string">Type</attribute>
+            </xpath>
+            <xpath expr="//header" position="attributes">
+                <attribute name="invisible">1</attribute>
+            </xpath>
+            <xpath expr="//field[@name='model_id']" position="attributes">
+                <attribute name="invisible">1</attribute>
+            </xpath>
+        </field>
+    </record>
+</odoo>
@@ -78,22 +78,44 @@ class ServerActions(models.Model):
        'User Field',
        compute='_compute_activity_info', readonly=False, store=True)

-
-    @api.depends('state', 'template_id', 'partner_ids', 'activity_summary')
+    @api.depends('template_id', 'partner_ids', 'activity_summary')
    def _compute_name(self):
-        for action in self:
-            if not action.state or not self.env.context.get('automatic_action_name'):
-                continue
+        to_update = self.filtered(
+            lambda action: action.state in {'mail_post', 'followers', 'remove_followers', 'next_activity'}
+        )
+        for action in to_update:
            if action.state == 'mail_post':
-                action.name = 'Send email: %s' % action.template_id.name
+                action.name = _(
+                    'Send email: %(template_name)s',
+                    template_name=action.template_id.name
+                )
            elif action.state == 'followers':
-                action.name = 'Add followers: %s' % ', '.join(action.partner_ids.mapped('name'))
+                action.name = _(
+                    'Add followers: %(partner_names)s',
+                    partner_names=', '.join(action.partner_ids.mapped('name'))
+                )
            elif action.state == 'remove_followers':
-                action.name = 'Remove followers: %s' % ', '.join(action.partner_ids.mapped('name'))
+                action.name = _(
+                    'Remove followers: %(partner_names)s',
+                    partner_names=', '.join(action.partner_ids.mapped('name'))
+                )
            elif action.state == 'next_activity':
-                action.name = 'Next activity: %s' % action.activity_summary
-            else:
-                super(ServerActions, action)._compute_name()
+                action.name = _(
+                    'Next activity: %(activity_name)s',
+                    activity_name=action.activity_summary or action.activity_type_id.name
+                )
+        super(ServerActions, self - to_update)._compute_name()
+
+    @api.depends('state')
+    def _compute_available_model_ids(self):
+        mail_thread_based = self.filtered(
+            lambda action: action.state in {'mail_post', 'followers', 'remove_followers', 'next_activity'}
+        )
+        if mail_thread_based:
+            mail_models = self.env['ir.model'].search([('is_mail_thread', '=', True), ('transient', '=', False)])
+            for action in mail_thread_based:
+                action.available_model_ids = mail_models.ids
+        super(ServerActions, self - mail_thread_based)._compute_available_model_ids()

    @api.depends('model_id', 'state')
    def _compute_template_id(self):
@@ -137,7 +159,7 @@ class ServerActions(models.Model):
        if to_reset:
            to_reset.activity_type_id = False

-    @api.depends('state')
+    @api.depends('state', 'activity_type_id')
    def _compute_activity_info(self):
        to_reset = self.filtered(lambda act: act.state != 'next_activity')
        if to_reset:
@@ -149,26 +171,38 @@ class ServerActions(models.Model):
            to_reset.activity_user_id = False
            to_reset.activity_user_field_name = False
        to_default = self.filtered(lambda act: act.state == 'next_activity')
-        for activity in to_default:
-            if not activity.activity_date_deadline_range_type:
-                activity.activity_date_deadline_range_type = 'days'
-            if not activity.activity_user_type:
-                activity.activity_user_type = 'specific'
-            if not activity.activity_user_field_name:
-                activity.activity_user_field_name = 'user_id'
+        for action in to_default:
+            if not action.activity_summary:
+                action.activity_summary = action.activity_type_id.summary
+            if not action.activity_date_deadline_range_type:
+                action.activity_date_deadline_range_type = 'days'
+            if not action.activity_user_type:
+                action.activity_user_type = 'specific'
+            if not action.activity_user_field_name:
+                action.activity_user_field_name = 'user_id'

    @api.constrains('activity_date_deadline_range')
    def _check_activity_date_deadline_range(self):
        if any(action.activity_date_deadline_range < 0 for action in self):
            raise ValidationError(_("The 'Due Date In' value can't be negative."))

+    @api.constrains('model_id', 'template_id')
+    def _check_mail_template_model(self):
+        for action in self.filtered(lambda action: action.state == 'mail_post'):
+            if action.template_id and action.template_id.model_id != action.model_id:
+                raise ValidationError(
+                    _('Mail template model of %(action_name)s does not match action model.',
+                      action_name=action.name
+                     )
+                )
+
    @api.constrains('state', 'model_id')
-    def _check_model_coherency(self):
+    def _check_mail_model_coherency(self):
        for action in self:
-            if action.state in ('followers', 'next_activity') and action.model_id.transient:
+            if action.state in {'mail_post', 'followers', 'remove_followers', 'next_activity'} and action.model_id.transient:
                raise ValidationError(_("This action cannot be done on transient models."))
-            if action.state == 'followers' and not action.model_id.is_mail_thread:
-                raise ValidationError(_("Add Followers can only be done on a mail thread models"))
+            if action.state in {'mail_post', 'followers', 'remove_followers'} and not action.model_id.is_mail_thread:
+                raise ValidationError(_("This action can only be done on a mail thread models"))
            if action.state == 'next_activity' and not action.model_id.is_mail_activity:
                raise ValidationError(_("A next activity can only be planned on models that use activities."))

@@ -26,15 +26,24 @@ class ServerActions(models.Model):
        compute='_compute_sms_method',
        readonly=False, store=True)

-    @api.depends('state', 'sms_template_id')
+    @api.depends('sms_template_id')
    def _compute_name(self):
-        for action in self:
-            if not action.state or not self.env.context.get('automatic_action_name'):
-                continue
-            if action.state == 'sms':
-                action.name = 'Send SMS: %s' % action.sms_template_id.name
-            else:
-                super(ServerActions, action)._compute_name()
+        to_update = self.filtered(lambda action: action.state == 'sms')
+        for action in to_update:
+            action.name = _(
+                'Send SMS: %(template_name)s',
+                template_name=action.sms_template_id.name
+            )
+        super(ServerActions, self - to_update)._compute_name()
+
+    @api.depends('state')
+    def _compute_available_model_ids(self):
+        mail_thread_based = self.filtered(lambda action: action.state == 'sms')
+        if mail_thread_based:
+            mail_models = self.env['ir.model'].search([('is_mail_thread', '=', True), ('transient', '=', False)])
+            for action in mail_thread_based:
+                action.available_model_ids = mail_models.ids
+        super(ServerActions, self - mail_thread_based)._compute_available_model_ids()

    @api.depends('model_id', 'state')
    def _compute_sms_template_id(self):
@@ -54,12 +63,22 @@ class ServerActions(models.Model):
        if other:
            other.sms_method = 'sms'

-    def _check_model_coherency(self):
-        super()._check_model_coherency()
+    @api.constrains('state', 'model_id')
+    def _check_sms_model_coherency(self):
        for action in self:
            if action.state == 'sms' and (action.model_id.transient or not action.model_id.is_mail_thread):
                raise ValidationError(_("Sending SMS can only be done on a mail.thread or a transient model"))

+    @api.constrains('model_id', 'template_id')
+    def _check_sms_template_model(self):
+        for action in self.filtered(lambda action: action.state == 'sms'):
+            if action.sms_template_id and action.sms_template_id.model_id != action.model_id:
+                raise ValidationError(
+                    _('SMS template model of %(action_name)s does not match action model.',
+                      action_name=action.name
+                     )
+                )
+
    def _run_action_sms_multi(self, eval_context=None):
        # TDE CLEANME: when going to new api with server action, remove action
        if not self.sms_template_id or self._is_recompute():
@@ -42,6 +42,12 @@ class LeadTest(models.Model):
        return result


+class LeadThreadTest(models.Model):
+    _name = "base.automation.lead.thread.test"
+    _description = "Automated Rule Test With Thread"
+    _inherit = ['base.automation.lead.test', 'mail.thread']
+
+
 class LineTest(models.Model):
    _name = "base.automation.line.test"
    _description = "Automated Rule Line Test"
@@ -1,5 +1,6 @@
 id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
 access_base_automation_lead_test,access_base_automation_lead_test,model_base_automation_lead_test,base.group_system,1,1,1,1
+access_base_automation_lead_thread_test,access_base_automation_lead_thread_test,model_base_automation_lead_thread_test,base.group_system,1,1,1,1
 access_base_automation_line_test,access_base_automation_line_test,model_base_automation_line_test,base.group_system,1,1,1,1
 access_base_automation_link_test,access_base_automation_link_test,model_base_automation_link_test,base.group_user,1,1,1,1
 access_base_automation_linked_test,access_base_automation_linked_test,model_base_automation_linked_test,base.group_user,1,1,1,1
@@ -431,28 +431,6 @@ registry.category("web_tour.tours").add("test_form_view_resequence_actions", {
    ],
 });

-registry.category("web_tour.tours").add("test_form_view_debug", {
-    test: true,
-    steps: () => [
-        {
-            trigger:
-                ".o_form_renderer .o_field_widget[name='action_server_ids'] .o_kanban_view .o_cp_buttons button",
-        },
-        {
-            trigger: ".modal-content .o_form_renderer",
-            run() {
-                const allFields = this.$anchor[0].querySelectorAll(".o_field_widget[name]");
-                assertEqual(
-                    Array.from(allFields)
-                        .map((el) => el.getAttribute("name"))
-                        .includes("model_id"),
-                    true
-                );
-            },
-        },
-    ],
-});
-
 let waitOrmCalls;
 registry.category("web_tour.tours").add("test_form_view_model_id", {
    test: true,
@@ -49,7 +49,7 @@ class BaseAutomationTest(TransactionCaseWithUserDemo):
        self.test_mail_template_automation = self.env['mail.template'].create(
            {
                'name': 'Template Automation',
-                'model_id': self.lead_model.id,
+                'model_id': self.env['ir.model']._get_id("base.automation.lead.thread.test"),
                'body_html': """&lt;div&gt;Email automation&lt;/div&gt;""",
            }
        )
@@ -365,7 +365,7 @@ else:
        deadline_field = self.env.ref("test_base_automation.field_base_automation_lead_test__deadline")
        create_automation(
            self,
-            model_id=self.lead_model.id,
+            model_id=self.env['ir.model']._get_id('base.automation.lead.thread.test'),
            trigger='on_create_or_write',
            trigger_field_ids=[Command.link(deadline_field.id)],
            filter_pre_domain="[('deadline', '=', False)]",
@@ -386,7 +386,11 @@ else:
        patcher = patch('odoo.addons.mail.models.mail_template.MailTemplate.send_mail', _patched_send_mail)
        self.startPatcher(patcher)

-        lead = self.create_lead()
+        lead = self.env['base.automation.lead.thread.test'].create({
+            'name': "Lead Test",
+            'user_id': self.user_root.id,
+        })
+        self.addCleanup(lead.unlink)
        self.assertEqual(lead.priority, False)
        self.assertEqual(lead.deadline, False)
        self.assertEqual(send_mail_count, 0)
@@ -265,23 +265,6 @@ class BaseAutomationTestUi(HttpCase):
            ["Set Active To False 2", "Set Active To False 0", "Set Active To False 1"],
        )

-    def test_form_view_debug(self):
-        model = self.env.ref("base.model_res_partner")
-        automation = self.env["base.automation"].create(
-            {
-                "name": "Test",
-                "trigger": "on_create_or_write",
-                "model_id": model.id,
-            }
-        )
-        self.start_tour(
-            (
-                f"/web?debug=1#{_urlencode_kwargs(action=self.env.ref('base_automation.base_automation_act').id, id=automation.id, view_type='form')}"
-            ),
-            "test_form_view_debug",
-            login="admin",
-        )
-
    def test_form_view_model_id(self):
        self.start_tour(
            (
@@ -485,6 +485,7 @@ class IrActionsServer(models.Model):
                                   "based on the sequence. Low number means high priority.")
    model_id = fields.Many2one('ir.model', string='Model', required=True, ondelete='cascade', index=True,
                               help="Model on which the server action runs.")
+    available_model_ids = fields.Many2many('ir.model', string='Available Models', compute='_compute_available_model_ids', store=False)
    model_name = fields.Char(related='model_id.model', string='Model Name', readonly=True, store=True)
    # Python code
    code = fields.Text(string='Python Code', groups='base.group_system',
@@ -533,16 +534,27 @@ class IrActionsServer(models.Model):

    @api.depends('state', 'update_field_id', 'crud_model_id', 'value')
    def _compute_name(self):
-        for action in self:
-            if not action.state or not self.env.context.get('automatic_action_name'):
-                continue
+        for action in self.filtered('state'):
            if action.state == 'object_write':
-                action.name = _("Update %s", action.update_field_id.field_description)
+                action.name = _(
+                    "Update %(field_name)s",
+                    field_name=action.update_field_id.field_description
+                )
            elif action.state == 'object_create':
-                action.name = _("Create %s with name %s", action.crud_model_id.name, action.value)
+                action.name = _(
+                    "Create %(model_name)s with name %(value)s",
+                    model_name=action.crud_model_id.name,
+                    value=action.value
+                )
            else:
-                state_name = dict(action._fields['state']._description_selection(self.env))[action.state]
-                action.name = state_name
+                action.name = dict(action._fields['state']._description_selection(self.env))[action.state]
+
+    @api.depends('state')
+    def _compute_available_model_ids(self):
+        allowed_models = self.env['ir.model'].search(
+            [('model', 'in', list(self.env['ir.model.access']._get_allowed_models()))]
+        )
+        self.available_model_ids = allowed_models.ids

    @api.onchange('model_id')
    def _compute_crud_model_id(self):
@@ -332,9 +332,11 @@
                        <group name="action_wrapper">
                            <group name="action_content">
                                <field name="model_name" invisible="1"/>
-                                <field name="model_id" invisible="1"/>
-                                <field name="model_id" options="{'no_create': True}" groups="base.group_no_one"/>
-                                <field name="model_id" options="{'no_create': True}" groups="!base.group_no_one" invisible="context.get('default_model_id')"/>
+                                <field name="available_model_ids" invisible="1"/>
+                                <field name="model_id" options="{'no_create': True}" groups="base.group_no_one"
+                                       domain="[('id', 'in', available_model_ids)]"/>
+                                <field name="model_id" options="{'no_create': True}" groups="!base.group_no_one" invisible="context.get('default_model_id')"
+                                       domain="[('id', 'in', available_model_ids)]"/>
                                <field name="groups_id" widget="many2many_tags" groups="base.group_no_one" />
                                <field name="state" string="Action Type"/>
                                <field name="update_field_id" invisible="state != 'object_write'"