diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py
index 8f28628dc9c..e7c97a9d361 100644
--- a/addons/mail/models/mail_message.py
+++ b/addons/mail/models/mail_message.py
@@ -571,7 +571,7 @@ class Message(models.Model):
if 'record_name' not in values and 'default_record_name' not in self.env.context:
values['record_name'] = self._get_record_name(values)
- if 'attachment_ids' not in values:
+ if not values.get('attachment_ids'):
values['attachment_ids'] = []
# extract base64 images
if 'body' in values:
@@ -604,9 +604,9 @@ class Message(models.Model):
messages = super(Message, self).create(values_list)
check_attachment_access = []
- if all(isinstance(command, int) or command[0] in (4, 6) for values in values_list for command in values.get('attachment_ids')):
+ if all(isinstance(command, int) or command[0] in (4, 6) for values in values_list for command in values['attachment_ids']):
for values in values_list:
- for command in values.get('attachment_ids'):
+ for command in values['attachment_ids']:
if isinstance(command, int):
check_attachment_access += [command]
elif command[0] == 6:
diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index 3cbf9e7204a..5cc47b65136 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -1821,36 +1821,61 @@ class MailThread(models.AbstractModel):
tuples in the form ``(name,content)`` or ``(name,content, info)`` where content
is NOT base64 encoded;
:param list attachment_ids: list of existing attachments to link to this message
- -Should only be set by chatter
- -Attachment object attached to mail.compose.message(0) will be attached
- to the related document.
+ Should not be a list of commands. Attachment records attached to mail
+ composer will be attached to the related document.
Extra keyword arguments will be used either
* as default column values for the new mail.message record if they match
mail.message fields;
- * propagated to notification methods;
+ * propagated to notification methods if not;
:return record: newly create mail.message
"""
self.ensure_one() # should always be posted on a record, use message_notify if no record
- # split message additional values from notify additional values
- msg_kwargs = dict((key, val) for key, val in kwargs.items() if key in self.env['mail.message']._fields)
- notif_kwargs = dict((key, val) for key, val in kwargs.items() if key not in msg_kwargs)
# preliminary value safety check
- partner_ids = set(partner_ids or [])
- if self._name == 'mail.thread' or not self.id or message_type == 'user_notification':
- raise ValueError(_('Posting a message should be done on a business document. Use message_notify to send a notification to an user.'))
- if 'channel_ids' in kwargs:
- raise ValueError(_("Posting a message with channels as listeners is not supported since Odoo 14.3+. Please update code accordingly."))
- if 'model' in msg_kwargs or 'res_id' in msg_kwargs:
- raise ValueError(_("message_post does not support model and res_id parameters anymore. Please call message_post on record."))
- if 'subtype' in kwargs:
- raise ValueError(_("message_post does not support subtype parameter anymore. Please give a valid subtype_id or subtype_xmlid value instead."))
- if any(not isinstance(pc_id, int) for pc_id in partner_ids):
- raise ValueError(_('message_post partner_ids and must be integer list, not commands.'))
+ self._raise_for_invalid_parameters(
+ set(kwargs.keys()),
+ forbidden_names={'model', 'res_id', 'subtype'}
+ )
+ if self._name == 'mail.thread' or not self.id:
+ raise ValueError(_("Posting a message should be done on a business document. Use message_notify to send a notification to an user."))
+ if message_type == 'user_notification':
+ raise ValueError(_("Use message_notify to send a notification to an user."))
+ if attachments:
+ # attachments should be a list (or tuples) of 3-elements list (or tuple)
+ format_error = not tools.is_list_of(attachments, list) and not tools.is_list_of(attachments, tuple)
+ if not format_error:
+ format_error = not all(len(attachment) in {2, 3} for attachment in attachments)
+ if format_error:
+ raise ValueError(
+ _('Posting a message should receive attachments as a list of list or tuples (received %(aids)s)',
+ aids=repr(attachment_ids),
+ )
+ )
+ if attachment_ids and not tools.is_list_of(attachment_ids, int):
+ raise ValueError(
+ _('Posting a message should receive attachments records as a list of IDs (received %(aids)s)',
+ aids=repr(attachment_ids),
+ )
+ )
+ attachment_ids = list(attachment_ids or [])
+ if partner_ids and not tools.is_list_of(partner_ids, int):
+ raise ValueError(
+ _('Posting a message should receive partners as a list of IDs (received %(pids)s)',
+ pids=repr(partner_ids),
+ )
+ )
+ partner_ids = list(partner_ids or [])
- self = self._fallback_lang() # add lang to context immediately since it will be useful in various flows latter.
+ # split message additional values from notify additional values
+ msg_kwargs = {key: val for key, val in kwargs.items()
+ if key in self.env['mail.message']._fields}
+ notif_kwargs = {key: val for key, val in kwargs.items()
+ if key not in msg_kwargs}
+
+ # Add lang to context immediately since it will be useful in various flows later
+ self = self._fallback_lang()
# Find the message's author
if self.env.user._is_public() and 'guest' in self.env.context:
@@ -1892,12 +1917,10 @@ class MailThread(models.AbstractModel):
'partner_ids': partner_ids,
})
- attachments = attachments or []
- attachment_ids = attachment_ids or []
- attachement_values = self._process_attachments_for_post(attachments, attachment_ids, msg_values)
- msg_values.update(attachement_values) # attachement_ids, [body]
-
- new_message = self._message_create(msg_values)
+ msg_values.update(
+ self._process_attachments_for_post(attachments, attachment_ids, msg_values)
+ ) # attachement_ids, body
+ new_message = self._message_create([msg_values])
# Set main attachment field if necessary
self._message_set_main_attachment_id(msg_values['attachment_ids'])
@@ -2111,13 +2134,19 @@ class MailThread(models.AbstractModel):
:param template_id : the id of the template to render to create the body of the message
:param **kwargs : parameter to create a mail.compose.message woaerd (which inherit from mail.message)
"""
+ # preliminary value safety check
+ self._raise_for_invalid_parameters(
+ set(kwargs.keys()),
+ forbidden_names={'model', 'res_id'}
+ )
+
# Get composition mode, or force it according to the number of record in self
if not kwargs.get('composition_mode'):
kwargs['composition_mode'] = 'comment' if len(self.ids) == 1 else 'mass_mail'
if not kwargs.get('message_type'):
kwargs['message_type'] = 'notification'
- res_id = kwargs.get('res_id', self.ids and self.ids[0] or 0)
- res_ids = kwargs.get('res_id') and [kwargs['res_id']] or self.ids
+ res_id = self.ids[0] if self.ids else 0
+ res_ids = self.ids
# support xml based subtype id
if kwargs.get('subtype_xmlid') and not kwargs.get('subtype_id'):
@@ -2127,17 +2156,22 @@ class MailThread(models.AbstractModel):
composer = self.env['mail.compose.message'].with_context(
active_id=res_id,
active_ids=res_ids,
- active_model=kwargs.get('model', self._name),
+ active_model=self._name,
default_composition_mode=kwargs['composition_mode'],
default_email_layout_xmlid=email_layout_xmlid,
- default_model=kwargs.get('model', self._name),
+ default_model=self._name,
default_res_id=res_id,
default_template_id=template_id,
).create(kwargs)
# Simulate the onchange (like trigger in form the view) only
# when having a template in single-email mode
if template_id:
- update_values = composer._onchange_template_id(template_id, kwargs['composition_mode'], self._name, res_id)['value']
+ update_values = composer._onchange_template_id(
+ template_id,
+ kwargs['composition_mode'],
+ self._name,
+ res_id
+ )['value']
composer.write(update_values)
return composer._action_send_mail(auto_commit=auto_commit)
@@ -2151,21 +2185,32 @@ class MailThread(models.AbstractModel):
on the user configuration, like other notifications. """
if self:
self.ensure_one()
- # split message additional values from notify additional values
- msg_kwargs = dict((key, val) for key, val in kwargs.items() if key in self.env['mail.message']._fields)
- notif_kwargs = dict((key, val) for key, val in kwargs.items() if key not in msg_kwargs)
-
- author_id, email_from = self._message_compute_author(author_id, email_from, raise_on_email=True)
-
if not partner_ids:
_logger.warning('Message notify called without recipient_ids, skipping')
return self.env['mail.message']
+ # preliminary value safety check
+ self._raise_for_invalid_parameters(
+ set(kwargs.keys()),
+ forbidden_names={'message_id', 'message_type'}
+ )
+ if not tools.is_list_of(partner_ids, int):
+ raise ValueError(
+ _('Notification should receive partners given as a list of IDs (received %(pids)s)',
+ pids=repr(partner_ids),
+ )
+ )
+
+ # split message additional values from notify additional values
+ msg_kwargs = {key: val for key, val in kwargs.items() if key in self.env['mail.message']._fields}
+ notif_kwargs = {key: val for key, val in kwargs.items() if key not in msg_kwargs}
+
+ author_id, email_from = self._message_compute_author(author_id, email_from, raise_on_email=True)
+
# allow to link a notification to a document that does not inherit from
- # MailThread by supporting model / res_id
- if not (model and res_id): # both value should be set or none should be set (record)
- model = False
- res_id = False
+ # MailThread by supporting model / res_id, but then both value should be set
+ if not model or not res_id:
+ model, res_id = False, False
msg_values = {
# author
@@ -2196,7 +2241,7 @@ class MailThread(models.AbstractModel):
if 'email_add_signature' not in msg_values:
msg_values['email_add_signature'] = True
- new_message = self._message_create(msg_values)
+ new_message = self._message_create([msg_values])
self._notify_thread(new_message, msg_values, **notif_kwargs)
return new_message
@@ -2204,13 +2249,17 @@ class MailThread(models.AbstractModel):
""" Helper method to log a note using a view_id without notifying followers. """
return self._message_compose_with_view(views_or_xmlid, message_log=True, **kwargs)
- def _message_log(self, *, body='', author_id=None, email_from=None, subject=False, message_type='notification', **kwargs):
+ def _message_log(self, *, body='', author_id=None, email_from=None, subject=False,
+ message_type='notification',
+ attachment_ids=False, tracking_value_ids=False):
""" Shortcut allowing to post note on a document. It does not perform
any notification and pre-computes some values to have a short code
as optimized as possible. This method is private as it does not check
access rights and perform the message creation as sudo to speedup
the log process. This method should be called within methods where
- access rights are already granted to avoid privilege escalation. """
+ access rights are already granted to avoid privilege escalation.
+
+ :return: created message (as sudo)"""
self.ensure_one()
author_id, email_from = self._message_compute_author(author_id, email_from, raise_on_email=False)
@@ -2219,30 +2268,43 @@ class MailThread(models.AbstractModel):
'author_id': author_id,
'email_from': email_from,
# document
- 'model': kwargs.get('model', self._name),
+ 'model': self._name,
'record_name': False,
- 'res_id': self.ids[0] if self.ids else False,
+ 'res_id': self.id,
# content
+ 'attachment_ids': attachment_ids,
'body': body,
'is_internal': True,
'message_type': message_type,
'subject': subject,
'subtype_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note'),
+ 'tracking_value_ids': tracking_value_ids,
# recipients
'email_add_signature': False, # False as no notification -> no need to compute signature
'message_id': tools.generate_tracking_message_id('message-notify'), # why? this is all but a notify
'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from)[False],
}
- msg_values.update(kwargs)
- return self.sudo()._message_create(msg_values)
+ return self.sudo()._message_create([msg_values])
- def _message_log_batch(self, bodies, author_id=None, email_from=None, subject=False, message_type='notification'):
- """ Shortcut allowing to post notes on a batch of documents. It achieve the
- same purpose as _message_log, done in batch to speedup quick note log.
+ def _message_log_batch(self, bodies, author_id=None, email_from=None, subject=False,
+ message_type='notification',
+ attachment_ids=False, tracking_value_ids=False):
+ """ Shortcut allowing to post notes on a batch of documents. It does not
+ perform any notification and pre-computes some values to have a short code
+ as optimized as possible. This method is private as it does not check
+ access rights and perform the message creation as sudo to speedup
+ the log process. This method should be called within methods where
+ access rights are already granted to avoid privilege escalation.
- :param bodies: dict {record_id: body}
+ :param bodies: dict {record_id: body}
+
+ :return: created messages (as sudo)
"""
+ # protect against side-effect prone usage
+ if len(self) > 1 and (attachment_ids or tracking_value_ids):
+ raise ValueError(_('Batch log cannot support attachments or tracking values on more than 1 document'))
+
author_id, email_from = self._message_compute_author(author_id, email_from, raise_on_email=False)
base_message_values = {
@@ -2253,12 +2315,14 @@ class MailThread(models.AbstractModel):
'model': self._name,
'record_name': False,
# content
+ 'attachment_ids': attachment_ids,
'message_type': message_type,
'is_internal': True,
'subject': subject,
'subtype_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note'),
+ 'tracking_value_ids': tracking_value_ids,
# recipients
- 'email_add_signature': False,
+ 'email_add_signature': False, # False as no notification -> no need to compute signature
'message_id': tools.generate_tracking_message_id('message-notify'), # why? this is all but a notify
'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from)[False],
}
@@ -2337,9 +2401,14 @@ class MailThread(models.AbstractModel):
return self.name_get()[0][1]
def _message_create(self, values_list):
- if not isinstance(values_list, (list)):
- values_list = [values_list]
create_values_list = []
+
+ # preliminary value safety check
+ self._raise_for_invalid_parameters(
+ {key for values in values_list for key in values.keys()},
+ restricting_names=self._get_message_create_valid_field_names()
+ )
+
for values in values_list:
create_values = dict(values)
# Avoid warnings about non-existing fields
@@ -2354,6 +2423,76 @@ class MailThread(models.AbstractModel):
clean_context(self.env.context)
).create(create_values_list)
+ def _get_message_create_valid_field_names(self):
+ """ Some fields should not be given when creating a mail.message from
+ mail.thread main API methods (in addition to some API specific check).
+ Those fields are generally used through UI or dedicated methods. We
+ therefore give an allowed field names list. """
+ return {
+ 'attachment_ids',
+ 'author_guest_id',
+ 'author_id',
+ 'body',
+ 'create_date', # anyway limited to admins
+ 'date',
+ 'email_add_signature',
+ 'email_from',
+ 'email_layout_xmlid',
+ 'is_internal',
+ 'mail_activity_type_id',
+ 'mail_server_id',
+ 'message_id',
+ 'message_type',
+ 'model',
+ 'parent_id',
+ 'partner_ids',
+ 'record_name',
+ 'reply_to',
+ 'reply_to_force_new',
+ 'res_id',
+ 'subject',
+ 'subtype_id',
+ 'tracking_value_ids',
+ }
+
+ def _get_notify_valid_parameters(self):
+ """ Several parameters exist for notification methods as business
+ flows often want to customize the standard notification experience.
+ In order to ease coding kwargs are frequently used. This method
+ acts like a filter, allowing to spot parameters that are not
+ supported. """
+ return {
+ 'force_email_company',
+ 'force_email_lang',
+ 'force_send',
+ 'mail_auto_delete',
+ 'model_description',
+ 'resend_existing',
+ 'scheduled_date',
+ 'send_after_commit',
+ 'skip_existing',
+ 'subtitles',
+ }
+
+ def _raise_for_invalid_parameters(self, parameter_names, forbidden_names=None, restricting_names=None):
+ """ Helper to warn about invalid parameters (or fields).
+
+ :param set parameter_names: a set of parameter names;
+ :param set forbidden_names: set of parameter name that should not be
+ present in parameter_names;
+ :param set restricting_names: set of parameters restricting given
+ parameter_names, parameters not belonging to this list are rejected;
+ """
+ if forbidden_names:
+ conflicting_names = parameter_names & forbidden_names
+ elif restricting_names:
+ conflicting_names = parameter_names - restricting_names
+ if conflicting_names:
+ raise ValueError(
+ _('Those values are not supported when posting or notifying: %(param_names)s',
+ param_names=', '.join(conflicting_names))
+ )
+
# ------------------------------------------------------
# NOTIFICATION API
# ------------------------------------------------------
@@ -2426,6 +2565,10 @@ class MailThread(models.AbstractModel):
"""
# add lang to context immediately since it will be useful in various rendering later
self = self._fallback_lang()
+ self._raise_for_invalid_parameters(
+ set(kwargs.keys()),
+ restricting_names=self._get_notify_valid_parameters()
+ )
msg_vals = msg_vals if msg_vals else {}
recipients_data = self._notify_get_recipients(message, msg_vals, **kwargs)
diff --git a/addons/mail_group/models/mail_group_message.py b/addons/mail_group/models/mail_group_message.py
index 09026ac20a5..ae4fb60c70c 100644
--- a/addons/mail_group/models/mail_group_message.py
+++ b/addons/mail_group/models/mail_group_message.py
@@ -96,6 +96,7 @@ class MailGroupMessage(models.Model):
field: vals.pop(field)
for field in self.env['mail.message']._fields
if field in vals
+ and field in self.env['mail.thread']._get_message_create_valid_field_names()
}).id
return super(MailGroupMessage, self).create(values_list)
diff --git a/addons/rating/models/mail_thread.py b/addons/rating/models/mail_thread.py
index eacfc11931e..c9196018c6c 100644
--- a/addons/rating/models/mail_thread.py
+++ b/addons/rating/models/mail_thread.py
@@ -30,13 +30,3 @@ class MailThread(models.AbstractModel):
self.env['rating.rating'].browse(rating_id).write({'message_id': message.id})
return message
-
- def _message_create(self, values_list):
- """ Force usage of rating-specific methods and API allowing to delegate
- computation to records. Keep methods optimized and skip rating_ids
- support to simplify MailThrad main API. """
- if not isinstance(values_list, (list)):
- values_list = [values_list]
- if any(values.get('rating_ids') for values in values_list):
- raise ValueError(_("Posting a rating should be done using message post API."))
- return super()._message_create(values_list)
diff --git a/addons/sms/models/mail_thread.py b/addons/sms/models/mail_thread.py
index cd7a600952e..0c1f3a02e8c 100644
--- a/addons/sms/models/mail_thread.py
+++ b/addons/sms/models/mail_thread.py
@@ -363,6 +363,9 @@ class MailThread(models.AbstractModel):
return True
+ def _get_notify_valid_parameters(self):
+ return super()._get_notify_valid_parameters() | {'put_in_queue', 'sms_numbers', 'sms_pid_to_number'}
+
@api.model
def notify_cancel_by_type(self, notification_type):
super().notify_cancel_by_type(notification_type)
diff --git a/addons/test_mail/tests/test_mail_thread_internals.py b/addons/test_mail/tests/test_mail_thread_internals.py
index 39b2c3d1f99..f7bb3470d45 100644
--- a/addons/test_mail/tests/test_mail_thread_internals.py
+++ b/addons/test_mail/tests/test_mail_thread_internals.py
@@ -361,7 +361,6 @@ class TestDiscuss(TestMailCommon, TestRecipients):
channel = self.env['mail.channel'].create({'name': 'testChannel'})
notification_msg = channel.with_user(self.user_admin).message_notify(
body='test',
- message_type='user_notification',
partner_ids=[self.partner_2.id],
)
diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py
index a05a89a8554..389e92a54a2 100644
--- a/addons/test_mail/tests/test_message_post.py
+++ b/addons/test_mail/tests/test_message_post.py
@@ -403,6 +403,40 @@ class TestMessageNotify(TestMessagePostCommon):
}
)
+ @users('employee')
+ def test_notify_parameters(self):
+ """ Test usage of parameters in notify, both for unwanted side effects
+ and magic parameters. """
+ test_record = self.test_record.with_env(self.env)
+
+ for parameters in [
+ {'message_type': 'comment'},
+ {'canned_response_ids': []},
+ {'child_ids': []},
+ {'mail_ids': []},
+ {'notification_ids': []},
+ {'notified_partner_ids': []},
+ {'reaction_ids': []},
+ {'starred_partner_ids': []},
+ ]:
+ with self.subTest(parameters=parameters), \
+ self.mock_mail_gateway(), \
+ self.assertRaises(ValueError):
+ _new_message = test_record.message_notify(
+ body='You will not receive a notification
',
+ partner_ids=self.partner_1.ids,
+ subject='This should not be accepted',
+ **parameters
+ )
+
+ # support of subtype xml id
+ new_message = test_record.message_notify(
+ body='You will not receive a notification
',
+ partner_ids=self.partner_1.ids,
+ subtype_xmlid='mail.mt_note',
+ )
+ self.assertEqual(new_message.subtype_id, self.env.ref('mail.mt_note'))
+
@users('employee')
@mute_logger('odoo.addons.mail.models.mail_mail')
def test_notify_thread(self):