[IMP] mail: better prevent activity type removal and add a fallback mechanism

Current activity types master data cannot be unlinked through a quite harsh
override of unlink using xml id existence, introduced at 2602642a78dff6c15d615fadf4c55a6b6ca315df.
Some of them are real master data, other could be unlinked if users want it.
Finally custom activity types may cause errors when removed if used in some
side models like server actions or automated actions.

To allow more flexibility we remove the removal constraint and implement
a fallback mechanism. It is implemented in mail.activity.mixin when calling
activity_schedule(xmlid, ...). If the given xmlid is not found
_default_activity_type() specifies the default activity to use. It can
be overridden in modules if some specific behavior is wanted.

e.g.

def _default_activity_type(self):
    """Define a default fallback activity type when xml id not found
    only used in in activity_schedule() for now.
    """
    try:
        return self.env.ref('mail.mail_activity_data_todo')
    except Exception:
        return False

In this commit we also

  * restrict deletion of activity type (hr.plan and ir.actions.server);
  * fallback on default activity type if xmlid ref not found in
    activity_schedule;
  * correct calls to activity_schedule and replace some manual creation calls
    by activity_schedule;

Task ID 1961156
PR #39013

Related: odoo/enterprise#6238
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
Co-authored-by: Victor Feyens <vfe@odoo.com>
Co-authored-by: Michaël Mattiello <mcm@odoo.com>

8 files changed, 58 insertions, 63 deletions

diff --git a/addons/hr/models/hr_plan.py b/addons/hr/models/hr_plan.py
index c1d897e9d38..dd45a280a6f 100644
--- a/addons/hr/models/hr_plan.py
+++ b/addons/hr/models/hr_plan.py
@@ -13,7 +13,8 @@ class HrPlanActivityType(models.Model):
     activity_type_id = fields.Many2one(
         'mail.activity.type', 'Activity Type',
         default=lambda self: self.env.ref('mail.mail_activity_data_todo'),
-        domain=lambda self: ['|', ('res_model_id', '=', False), ('res_model_id', '=', self.env['ir.model']._get('hr.employee').id)]
+        domain=lambda self: ['|', ('res_model_id', '=', False), ('res_model_id', '=', self.env['ir.model']._get('hr.employee').id)],
+        ondelete='restrict'
     )
     summary = fields.Char('Summary')
     responsible = fields.Selection([
diff --git a/addons/hr/wizard/hr_plan_wizard.py b/addons/hr/wizard/hr_plan_wizard.py
index 7dc74bbb035..c0f2d663e4c 100644
--- a/addons/hr/wizard/hr_plan_wizard.py
+++ b/addons/hr/wizard/hr_plan_wizard.py
@@ -24,14 +24,12 @@ class HrPlanWizard(models.TransientModel):
             responsible = activity_type.get_responsible_id(self.employee_id)
 
             if self.env['hr.employee'].with_user(responsible).check_access_rights('read', raise_exception=False):
-                self.env['mail.activity'].create({
-                    'res_id': self.employee_id.id,
-                    'res_model_id': self.env['ir.model']._get('hr.employee').id,
-                    'summary': activity_type.summary,
-                    'note': activity_type.note,
-                    'activity_type_id': activity_type.activity_type_id.id,
-                    'user_id': responsible.id,
-                })
+                self.employee_id.activity_schedule(
+                    activity_type_id=activity_type.activity_type_id.id,
+                    summary=activity_type.summary,
+                    note=activity_type.note,
+                    user_id=responsible.id
+                )
 
         return {
             'type': 'ir.actions.act_window',
diff --git a/addons/l10n_it_edi/models/ir_mail_server.py b/addons/l10n_it_edi/models/ir_mail_server.py
index 2674cfba5bf..8df9763f147 100644
--- a/addons/l10n_it_edi/models/ir_mail_server.py
+++ b/addons/l10n_it_edi/models/ir_mail_server.py
@@ -244,11 +244,10 @@ class FetchmailServer(models.Model):
             related_invoice.message_post(
                 body=(_("Errors in the E-Invoice :<br/>%s") % (error))
             )
-            activity_vals = {
-                'activity_type_id': self.env.ref('mail.mail_activity_data_todo').id,
-                'invoice_user_id': related_invoice.invoice_user_id.id if related_invoice.invoice_user_id else self.env.user.id
-            }
-            related_invoice.activity_schedule(summary='Rejection notice', **activity_vals)
+            related_invoice.activity_schedule(
+                'mail.mail_activity_data_todo',
+                summary='Rejection notice',
+                user_id=related_invoice.invoice_user_id.id if related_invoice.invoice_user_id else self.env.user.id)
 
         elif receipt_type == 'MC':
             # Failed delivery notice
@@ -304,11 +303,10 @@ class FetchmailServer(models.Model):
                 body=(_("Outcome notice: %s<br/>%s") % (related_invoice.l10n_it_send_state, info))
             )
             if related_invoice.l10n_it_send_state == 'delivered_refused':
-                activity_vals = {
-                    'activity_type_id': self.env.ref('mail.mail_activity_data_todo').id,
-                    'invoice_user_id': related_invoice.invoice_user_id.id if related_invoice.invoice_user_id else self.env.user.id
-                }
-                related_invoice.activity_schedule(summary='Outcome notice: Refused', **activity_vals)
+                related_invoice.activity_schedule(
+                    'mail.mail_activity_todo',
+                    user_id=related_invoice.invoice_user_id.id if related_invoice.invoice_user_id else self.env.user.id,
+                    summary='Outcome notice: Refused')
 
         # elif receipt_type == 'MT':
             # Metadata file
diff --git a/addons/mail/models/ir_actions.py b/addons/mail/models/ir_actions.py
index ef9495f52fd..22a39a29164 100644
--- a/addons/mail/models/ir_actions.py
+++ b/addons/mail/models/ir_actions.py
@@ -29,7 +29,8 @@ class ServerActions(models.Model):
     # Next Activity
     activity_type_id = fields.Many2one(
         'mail.activity.type', string='Activity',
-        domain="['|', ('res_model_id', '=', False), ('res_model_id', '=', model_id)]")
+        domain="['|', ('res_model_id', '=', False), ('res_model_id', '=', model_id)]",
+        ondelete='restrict')
     activity_summary = fields.Char('Summary')
     activity_note = fields.Html('Note')
     activity_date_deadline_range = fields.Integer(string='Due Date In')
diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py
index 8fc502ee6de..55e2db647bd 100644
--- a/addons/mail/models/mail_activity.py
+++ b/addons/mail/models/mail_activity.py
@@ -59,7 +59,7 @@ class MailActivityType(models.Model):
         help='Specify a model if the activity should be specific to a model'
              ' and not available when managing activities for other models.')
     default_next_type_id = fields.Many2one('mail.activity.type', 'Default Next Activity',
-        domain="['|', ('res_model_id', '=', False), ('res_model_id', '=', res_model_id)]")
+        domain="['|', ('res_model_id', '=', False), ('res_model_id', '=', res_model_id)]", ondelete='restrict')
     force_next = fields.Boolean("Trigger Next Activity", default=False)
     next_type_ids = fields.Many2many(
         'mail.activity.type', 'mail_activity_rel', 'activity_id', 'recommended_id',
@@ -91,11 +91,6 @@ class MailActivityType(models.Model):
         for activity_type in self:
             activity_type.initial_res_model_id = activity_type.res_model_id
 
-    def unlink(self):
-        if any(self.get_external_id().values()) and not self._context.get(MODULE_UNINSTALL_FLAG):
-            raise exceptions.ValidationError("You can not delete activity type that are used as master data.")
-        return super(MailActivityType, self).unlink()
-
 
 class MailActivity(models.Model):
     """ An actual activity to perform. Activities are linked to
@@ -575,6 +570,16 @@ class MailActivityMixin(models.AbstractModel):
     _name = 'mail.activity.mixin'
     _description = 'Activity Mixin'
 
+    def _default_activity_type(self):
+        """Define a default fallback activity type when requested xml id wasn't found.
+
+        Can be overriden to specify the default activity type of a model.
+        It is only called in in activity_schedule() for now.
+        """
+        return self.env.ref('mail.mail_activity_data_todo', raise_if_not_found=False) \
+            or self.env['mail.activity.type'].search([('res_model', '=', self._name)], limit=1) \
+            or self.env['mail.activity.type'].search([('res_model_id', '=', False)], limit=1)
+
     activity_ids = fields.One2many(
         'mail.activity', 'res_id', 'Activities',
         auto_join=True,
@@ -732,15 +737,16 @@ class MailActivityMixin(models.AbstractModel):
         if isinstance(date_deadline, datetime):
             _logger.warning("Scheduled deadline should be a date (got %s)", date_deadline)
         if act_type_xmlid:
-            activity_type = self.sudo().env.ref(act_type_xmlid)
+            activity_type = self.env.ref(act_type_xmlid, raise_if_not_found=False) or self._default_activity_type()
         else:
-            activity_type = self.env['mail.activity.type'].sudo().browse(act_values['activity_type_id'])
+            activity_type_id = act_values.get('activity_type_id', False)
+            activity_type = activity_type_id and self.env['mail.activity.type'].sudo().browse(activity_type_id)
 
         model_id = self.env['ir.model']._get(self._name).id
         activities = self.env['mail.activity']
         for record in self:
             create_vals = {
-                'activity_type_id': activity_type.id,
+                'activity_type_id': activity_type and activity_type.id,
                 'summary': summary or activity_type.summary,
                 'automated': True,
                 'note': note or activity_type.default_description,
@@ -792,7 +798,10 @@ class MailActivityMixin(models.AbstractModel):
             return False
 
         Data = self.env['ir.model.data'].sudo()
-        activity_types_ids = [Data.xmlid_to_res_id(xmlid) for xmlid in act_type_xmlids]
+        activity_types_ids = [Data.xmlid_to_res_id(xmlid, raise_if_not_found=False) for xmlid in act_type_xmlids]
+        activity_types_ids = [act_type_id for act_type_id in activity_types_ids if act_type_id]
+        if not any(activity_types_ids):
+            return False
         domain = [
             '&', '&', '&',
             ('res_model', '=', self._name),
@@ -819,7 +828,10 @@ class MailActivityMixin(models.AbstractModel):
             return False
 
         Data = self.env['ir.model.data'].sudo()
-        activity_types_ids = [Data.xmlid_to_res_id(xmlid) for xmlid in act_type_xmlids]
+        activity_types_ids = [Data.xmlid_to_res_id(xmlid, raise_if_not_found=False) for xmlid in act_type_xmlids]
+        activity_types_ids = [act_type_id for act_type_id in activity_types_ids if act_type_id]
+        if not any(activity_types_ids):
+            return False
         domain = [
             '&', '&', '&',
             ('res_model', '=', self._name),
@@ -841,7 +853,10 @@ class MailActivityMixin(models.AbstractModel):
             return False
 
         Data = self.env['ir.model.data'].sudo()
-        activity_types_ids = [Data.xmlid_to_res_id(xmlid) for xmlid in act_type_xmlids]
+        activity_types_ids = [Data.xmlid_to_res_id(xmlid, raise_if_not_found=False) for xmlid in act_type_xmlids]
+        activity_types_ids = [act_type_id for act_type_id in activity_types_ids if act_type_id]
+        if not any(activity_types_ids):
+            return False
         domain = [
             '&', '&', '&',
             ('res_model', '=', self._name),
diff --git a/addons/note/controllers/note.py b/addons/note/controllers/note.py
index bdcf2e85059..6cafea730cf 100644
--- a/addons/note/controllers/note.py
+++ b/addons/note/controllers/note.py
@@ -12,16 +12,9 @@ class NoteController(http.Controller):
         """ Route to create note and their activity directly from the systray """
         note = request.env['note.note'].create({'memo': note})
         if date_deadline:
-            activity_values = {
-                'note': note.memo,
-                'date_deadline': date_deadline,
-                'res_model_id': request.env.ref("note.model_note_note").id,
-                'res_id': note.id,
-                'note_id': note.id,
-            }
-            if not activity_type_id:
-                activity_type_id = request.env['mail.activity.type'].sudo().search([('category', '=', 'reminder')], limit=1).id
-            if activity_type_id:
-                activity_values['activity_type_id'] = activity_type_id
-            request.env['mail.activity'].create(activity_values)
+            note.activity_schedule(
+                activity_type_id=activity_type_id or request.env['mail.activity.type'].sudo().search([('category', '=', 'reminder')], limit=1).id,
+                note=note.memo,
+                date_deadline=date_deadline
+            )
         return note.id
diff --git a/addons/purchase_stock/models/purchase.py b/addons/purchase_stock/models/purchase.py
index d697538db9f..7616fa86269 100644
--- a/addons/purchase_stock/models/purchase.py
+++ b/addons/purchase_stock/models/purchase.py
@@ -306,13 +306,9 @@ class PurchaseOrderLine(models.Model):
                 if float_compare(line.product_qty, line.qty_invoiced, line.product_uom.rounding) == -1:
                     # If the quantity is now below the invoiced quantity, create an activity on the vendor bill
                     # inviting the user to create a refund.
-                    activity = self.env['mail.activity'].sudo().create({
-                        'activity_type_id': self.env.ref('mail.mail_activity_data_warning').id,
-                        'note': _('The quantities on your purchase order indicate less than billed. You should ask for a refund. '),
-                        'res_id': line.invoice_lines[0].invoice_id.id,
-                        'res_model_id': self.env.ref('account.model_account_move').id,
-                    })
-                    activity._onchange_activity_type_id()
+                    line.invoice_lines[0].move_id.activity_schedule(
+                        'mail.mail_activity_data_warning',
+                        note=_('The quantities on your purchase order indicate less than billed. You should ask for a refund.'))
 
                 # If the user increased quantity of existing line or created a new line
                 pickings = line.order_id.picking_ids.filtered(lambda x: x.state not in ('done', 'cancel') and x.location_dest_id.usage in ('internal', 'transit'))
diff --git a/addons/stock/models/stock_rule.py b/addons/stock/models/stock_rule.py
index b8add883f27..94b6b9ea646 100644
--- a/addons/stock/models/stock_rule.py
+++ b/addons/stock/models/stock_rule.py
@@ -298,18 +298,11 @@ class StockRule(models.Model):
         existing_activity = self.env['mail.activity'].search([('res_id', '=',  product_id.product_tmpl_id.id), ('res_model_id', '=', self.env.ref('product.model_product_template').id),
                                                               ('note', '=', note)])
         if not existing_activity:
-            # If the user deleted warning activity type.
-            try:
-                activity_type_id = self.env.ref('mail.mail_activity_data_warning').id
-            except:
-                activity_type_id = False
-            self.env['mail.activity'].create({
-                'activity_type_id': activity_type_id,
-                'note': note,
-                'user_id': product_id.responsible_id.id or SUPERUSER_ID,
-                'res_id': product_id.product_tmpl_id.id,
-                'res_model_id': self.env.ref('product.model_product_template').id,
-            })
+            product_id.product_tmpl_id.activity_schedule(
+                'mail.mail_activity_data_warning',
+                note=note,
+                user_id=product_id.responsible_id.id or SUPERUSER_ID,
+            )
 
 
 class ProcurementGroup(models.Model):