From d917b2d072a837d0d7deb0429b7552d680f26b6f Mon Sep 17 00:00:00 2001 From: XavierDo Date: Fri, 12 Apr 2019 11:42:43 +0000 Subject: [PATCH 1/9] [IMP] mail: improve user notification model in mail.message Currently if a message has a res_id and a res_model it appears in the matching record's chatter. Its access rights are computed based on this record. This is the standard behavior of mail.message model. User notifications are currently built on messages not having model and res_id in order to avoid appearing on chatter. This has several drawbacks notably access rights, redirection to the record, systray, finding back records, ... This commit solves those issues by adding new message_type 'user_notification' that should be as a classic mail.message with model and res_id but without the whole notification mechanism and without being displayed in the chatter. An user notification is now a classic message pushed to a given partner only and not displayed in the chatter. Related to task 1943901 Linked to PR #32404 --- addons/mail/models/mail_activity.py | 5 +- addons/mail/models/mail_mail.py | 4 +- addons/mail/models/mail_message.py | 122 ++++++++++++------ addons/mail/models/mail_thread.py | 35 ++--- .../static/src/js/models/messages/message.js | 2 +- .../test_mail/tests/test_message_compose.py | 4 +- addons/test_mail/tests/test_performance.py | 2 +- 7 files changed, 111 insertions(+), 63 deletions(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index a989ac4c983..ab098ed52a1 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -386,13 +386,14 @@ class MailActivity(models.Model): engine='ir.qweb', minimal_qcontext=True ) - self.env['mail.thread'].message_notify( + record = self.env[activity.res_model].browse(activity.res_id) + record.message_notify( partner_ids=activity.user_id.partner_id.ids, body=body, subject=_('%s: %s assigned to you') % (activity.res_name, activity.summary or activity.activity_type_id.name), record_name=activity.res_name, model_description=model_description, - notif_layout='mail.mail_notification_light' + notif_layout='mail.mail_notification_light', ) @api.multi diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index a2ba820656f..ffc8aa33399 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -168,7 +168,7 @@ class MailMail(models.Model): 'failure_type': failure_type, 'failure_reason': failure_reason, }) - messages = notifications.mapped('mail_message_id').filtered(lambda m: m.res_id and m.model) + messages = notifications.mapped('mail_message_id').filtered(lambda m: m.is_thread_message()) messages._notify_failure_update() # notify user that we have a failure (notifications - failed).sudo().write({ 'email_status': 'sent', @@ -312,7 +312,7 @@ class MailMail(models.Model): bounce_alias = ICP.get_param("mail.bounce.alias") catchall_domain = ICP.get_param("mail.catchall.domain") if bounce_alias and catchall_domain: - if mail.model and mail.res_id: + if mail.mail_message_id.is_thread_message(): headers['Return-Path'] = '%s+%d-%s-%d@%s' % (bounce_alias, mail.id, mail.model, mail.res_id, catchall_domain) else: headers['Return-Path'] = '%s+%d@%s' % (bounce_alias, mail.id, catchall_domain) diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index c9a54701ca7..0e595654885 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -59,7 +59,8 @@ class Message(models.Model): message_type = fields.Selection([ ('email', 'Email'), ('comment', 'Comment'), - ('notification', 'System notification')], + ('notification', 'System notification'), + ('user_notification', 'User Specific Notification')], 'Type', required=True, default='email', help="Message type: email for email message, notification for system " "message, comment for other messages such as user replies", @@ -79,8 +80,7 @@ class Message(models.Model): author_avatar = fields.Binary("Author's avatar", related='author_id.image_small', readonly=False) # recipients: include inactive partners (they may have been archived after # the message was sent, but they should remain visible in the relation) - partner_ids = fields.Many2many('res.partner', string='Recipients', - context={'active_test': False}) + partner_ids = fields.Many2many('res.partner', string='Recipients', context={'active_test': False}) needaction_partner_ids = fields.Many2many( 'res.partner', 'mail_message_res_partner_needaction_rel', string='Partners with Need Action', context={'active_test': False}) @@ -393,7 +393,8 @@ class Message(models.Model): customer_email_data.append((partner_tree[notification.res_partner_id.id][0], partner_tree[notification.res_partner_id.id][1], notification.email_status)) has_access_to_model = message.model and self.env[message.model].check_access_rights('read', raise_exception=False) - main_attachment = has_access_to_model and message.res_id and self.env[message.model].search([('id', '=',message.res_id)]) and getattr(self.env[message.model].browse(message.res_id), 'message_main_attachment_id') + if message.attachment_ids: + main_attachment = has_access_to_model and message.res_id and self.env[message.model].search([('id', '=', message.res_id)]) and getattr(self.env[message.model].browse(message.res_id), 'message_main_attachment_id') attachment_ids = [] for attachment in message.attachment_ids: if attachment.id in attachments_tree: @@ -417,7 +418,13 @@ class Message(models.Model): @api.multi def message_fetch_failed(self): - messages = self.search([('has_error', '=', True), ('author_id.id', '=', self.env.user.partner_id.id), ('res_id', '!=', 0), ('model', '!=', False)]) + messages = self.search([ + ('has_error', '=', True), + ('author_id.id', '=', self.env.user.partner_id.id), + ('res_id', '!=', 0), + ('model', '!=', False), + ('message_type', '!=', 'user_notification') + ]) return messages._format_mail_failures() @api.model @@ -515,7 +522,7 @@ class Message(models.Model): message['needaction_partner_ids'] = notif_dict.get(message['id'], dict()).get('partner_id', []) message['is_note'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['id'] == note_id message['is_discussion'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['id'] == com_id - message['is_notification'] = message['is_note'] and not message['model'] and not message['res_id'] + message['is_notification'] = message['message_type'] == 'user_notification' message['subtype_description'] = message['subtype_id'] and subtypes_dict[message['subtype_id'][0]]['description'] if message['model'] and self.env[message['model']]._original_module: message['module_icon'] = modules.module.get_module_icon(self.env[message['model']]._original_module) @@ -583,6 +590,19 @@ class Message(models.Model): if not self._cr.fetchone(): self._cr.execute("""CREATE INDEX mail_message_model_res_id_idx ON mail_message (model, res_id)""") + @api.multi + def is_thread_message(self, vals=None): + if vals: + res_id = vals.get('res_id') + model = vals.get('model') + message_type = vals.get('message_type') + else: + self.ensure_one() + res_id = self.res_id + model = self.model + message_type = self.message_type + return res_id and model and message_type != 'user_notification' + @api.model def _find_allowed_model_wise(self, doc_model, doc_dict): doc_ids = list(doc_dict) @@ -640,7 +660,7 @@ class Message(models.Model): super(Message, self.sudo(access_rights_uid or self._uid)).check_access_rights('read') self._cr.execute(""" - SELECT DISTINCT m.id, m.model, m.res_id, m.author_id, + SELECT DISTINCT m.id, m.model, m.res_id, m.author_id, m.message_type, COALESCE(partner_rel.res_partner_id, needaction_rel.res_partner_id), channel_partner.channel_id as channel_id FROM "%s" m @@ -656,14 +676,14 @@ class Message(models.Model): ON channel_partner.channel_id = channel.id AND channel_partner.partner_id = %%(pid)s WHERE m.id = ANY (%%(ids)s)""" % self._table, dict(pid=pid, ids=ids)) - for id, rmod, rid, author_id, partner_id, channel_id in self._cr.fetchall(): + for id, rmod, rid, author_id, message_type, partner_id, channel_id in self._cr.fetchall(): if author_id == pid: author_ids.add(id) elif partner_id == pid: partner_ids.add(id) elif channel_id: channel_ids.add(id) - elif rmod and rid: + elif rmod and rid and message_type != 'user_notification': model_ids.setdefault(rmod, {}).setdefault(rid, set()).add(id) allowed_ids = self._find_allowed_doc_ids(model_ids) @@ -733,13 +753,14 @@ class Message(models.Model): (self._description, operation)) # Read mail_message.ids to have their values - message_values = dict((res_id, {}) for res_id in self.ids) + message_values = dict((message_id, {}) for message_id in self.ids) if operation == 'read': self._cr.execute(""" SELECT DISTINCT m.id, m.model, m.res_id, m.author_id, m.parent_id, COALESCE(partner_rel.res_partner_id, needaction_rel.res_partner_id), - channel_partner.channel_id as channel_id, m.moderation_status + channel_partner.channel_id as channel_id, m.moderation_status, + m.message_type as message_type FROM "%s" m LEFT JOIN "mail_message_res_partner_rel" partner_rel ON partner_rel.mail_message_id = m.id AND partner_rel.res_partner_id = %%(pid)s @@ -752,7 +773,7 @@ class Message(models.Model): LEFT JOIN "mail_channel_partner" channel_partner ON channel_partner.channel_id = channel.id AND channel_partner.partner_id = %%(pid)s WHERE m.id = ANY (%%(ids)s)""" % self._table, dict(pid=self.env.user.partner_id.id, ids=self.ids)) - for mid, rmod, rid, author_id, parent_id, partner_id, channel_id, moderation_status in self._cr.fetchall(): + for mid, rmod, rid, author_id, parent_id, partner_id, channel_id, moderation_status, message_type in self._cr.fetchall(): message_values[mid] = { 'model': rmod, 'res_id': rid, @@ -760,13 +781,15 @@ class Message(models.Model): 'parent_id': parent_id, 'moderation_status': moderation_status, 'moderator_id': False, - 'notified': any((message_values[mid].get('notified'), partner_id, channel_id)) + 'notified': any((message_values[mid].get('notified'), partner_id, channel_id)), + 'message_type': message_type, } elif operation == 'write': self._cr.execute(""" SELECT DISTINCT m.id, m.model, m.res_id, m.author_id, m.parent_id, m.moderation_status, COALESCE(partner_rel.res_partner_id, needaction_rel.res_partner_id), - channel_partner.channel_id as channel_id, channel_moderator_rel.res_users_id as moderator_id + channel_partner.channel_id as channel_id, channel_moderator_rel.res_users_id as moderator_id, + m.message_type as message_type FROM "%s" m LEFT JOIN "mail_message_res_partner_rel" partner_rel ON partner_rel.mail_message_id = m.id AND partner_rel.res_partner_id = %%(pid)s @@ -783,7 +806,7 @@ class Message(models.Model): LEFT JOIN "mail_channel_moderator_rel" channel_moderator_rel ON channel_moderator_rel.mail_channel_id = moderated_channel.id AND channel_moderator_rel.res_users_id = %%(uid)s WHERE m.id = ANY (%%(ids)s)""" % self._table, dict(pid=self.env.user.partner_id.id, uid=self.env.user.id, ids=self.ids)) - for mid, rmod, rid, author_id, parent_id, moderation_status, partner_id, channel_id, moderator_id in self._cr.fetchall(): + for mid, rmod, rid, author_id, parent_id, moderation_status, partner_id, channel_id, moderator_id, message_type in self._cr.fetchall(): message_values[mid] = { 'model': rmod, 'res_id': rid, @@ -791,35 +814,38 @@ class Message(models.Model): 'parent_id': parent_id, 'moderation_status': moderation_status, 'moderator_id': moderator_id, - 'notified': any((message_values[mid].get('notified'), partner_id, channel_id)) + 'notified': any((message_values[mid].get('notified'), partner_id, channel_id)), + 'message_type': message_type, } elif operation == 'create': - self._cr.execute("""SELECT DISTINCT id, model, res_id, author_id, parent_id, moderation_status FROM "%s" WHERE id = ANY (%%s)""" % self._table, (self.ids,)) - for mid, rmod, rid, author_id, parent_id, moderation_status in self._cr.fetchall(): + self._cr.execute("""SELECT DISTINCT id, model, res_id, author_id, parent_id, moderation_status, message_type FROM "%s" WHERE id = ANY (%%s)""" % self._table, (self.ids,)) + for mid, rmod, rid, author_id, parent_id, moderation_status, message_type in self._cr.fetchall(): message_values[mid] = { 'model': rmod, 'res_id': rid, 'author_id': author_id, 'parent_id': parent_id, 'moderation_status': moderation_status, - 'moderator_id': False + 'moderator_id': False, + 'message_type': message_type, } else: # unlink - self._cr.execute("""SELECT DISTINCT m.id, m.model, m.res_id, m.author_id, m.parent_id, m.moderation_status, channel_moderator_rel.res_users_id as moderator_id + self._cr.execute("""SELECT DISTINCT m.id, m.model, m.res_id, m.author_id, m.parent_id, m.moderation_status, channel_moderator_rel.res_users_id as moderator_id, m.message_type as message_type FROM "%s" m LEFT JOIN "mail_channel" moderated_channel ON m.moderation_status = 'pending_moderation' AND m.res_id = moderated_channel.id LEFT JOIN "mail_channel_moderator_rel" channel_moderator_rel ON channel_moderator_rel.mail_channel_id = moderated_channel.id AND channel_moderator_rel.res_users_id = (%%s) WHERE m.id = ANY (%%s)""" % self._table, (self.env.user.id, self.ids,)) - for mid, rmod, rid, author_id, parent_id, moderation_status, moderator_id in self._cr.fetchall(): + for mid, rmod, rid, author_id, parent_id, moderation_status, moderator_id, message_type in self._cr.fetchall(): message_values[mid] = { 'model': rmod, 'res_id': rid, 'author_id': author_id, 'parent_id': parent_id, 'moderation_status': moderation_status, - 'moderator_id': moderator_id + 'moderator_id': moderator_id, + 'message_type': message_type, } # Author condition (READ, WRITE, CREATE (private)) @@ -832,7 +858,7 @@ class Message(models.Model): if message.get('moderation_status') != 'pending_moderation' and message.get('author_id') == self.env.user.partner_id.id] elif operation == 'create': author_ids = [mid for mid, message in message_values.items() - if not message.get('model') and not message.get('res_id')] + if not self.is_thread_message(message)] # Parent condition, for create (check for received notifications for the created message parent) notified_ids = [] @@ -873,7 +899,10 @@ class Message(models.Model): ]) fol_mids = [follower.res_id for follower in followers] notified_ids += [mid for mid, message in message_values.items() - if message.get('model') == doc_model and message.get('res_id') in fol_mids] + if message.get('model') == doc_model and + message.get('res_id') in fol_mids and + message.get('message_type') != 'user_notification' + ] # CRUD: Access rights related to the document other_ids = other_ids.difference(set(notified_ids)) @@ -886,13 +915,13 @@ class Message(models.Model): DocumentModel.check_mail_message_access(mids.ids, operation) # ?? mids ? else: self.env['mail.thread'].check_mail_message_access(mids.ids, operation, model_name=model) - if operation in ['write', 'unlink']: - document_related_ids += [mid for mid, message in message_values.items() - if message.get('model') == model and message.get('res_id') in mids.ids and - message.get('moderation_status') != 'pending_moderation'] - else: - document_related_ids += [mid for mid, message in message_values.items() - if message.get('model') == model and message.get('res_id') in mids.ids] + document_related_ids += [ + mid for mid, message in message_values.items() + if (message.get('model') == model and + message.get('res_id') in mids.ids and + message.get('message_type') != 'user_notification' and + (message.get('moderation_status') != 'pending_moderation' or + operation not in ['write', 'unlink']))] # Calculate remaining ids: if not void, raise an error other_ids = other_ids.difference(set(document_related_ids)) @@ -915,15 +944,22 @@ class Message(models.Model): @api.model def _get_reply_to(self, values): """ Return a specific reply_to for the document """ - model, res_id, email_from = values.get('model', self._context.get('default_model')), values.get('res_id', self._context.get('default_res_id')), values.get('email_from') # ctx values / defualt_get res ? - records = self.env[model].browse([res_id]) if model and res_id else None - return self.env['mail.thread']._notify_get_reply_to_on_records(default=email_from, records=records)[res_id or False] + model = values.get('model', self._context.get('default_model')) + res_id = values.get('res_id', self._context.get('default_res_id')) + email_from = values.get('email_from') + message_type = values.get('message_type') + records = None + if self.is_thread_message({'model': model, 'res_id': res_id, 'message_type': message_type}): + records = self.env[model].browse([res_id]) + else: + res_id = False + return self.env['mail.thread']._notify_get_reply_to_on_records(default=email_from, records=records)[res_id] @api.model def _get_message_id(self, values): if values.get('no_auto_thread', False) is True: message_id = tools.generate_tracking_message_id('reply_to') - elif values.get('res_id') and values.get('model'): + elif self.is_thread_message(values): message_id = tools.generate_tracking_message_id('%(res_id)s-%(model)s' % values) else: message_id = tools.generate_tracking_message_id('private') @@ -933,7 +969,7 @@ class Message(models.Model): def _invalidate_documents(self): """ Invalidate the cache of the documents followed by ``self``. """ for record in self: - if record.model and record.res_id and 'message_ids' in self.env[record.model]: + if record.is_thread_message() and 'message_ids' in self.env[record.model]: self.env[record.model].invalidate_cache(fnames=[ 'message_ids', 'message_unread', @@ -996,7 +1032,7 @@ class Message(models.Model): if other_cmd: message.sudo().write({'tracking_value_ids': tracking_values_cmd}) - if values.get('model') and values.get('res_id'): + if message.is_thread_message(values): message._invalidate_documents() return message @@ -1010,13 +1046,14 @@ class Message(models.Model): @api.multi def write(self, vals): - if 'model' in vals or 'res_id' in vals: + record_changed = 'model' in vals or 'res_id' in vals + if record_changed or 'message_type' in vals: self._invalidate_documents() res = super(Message, self).write(vals) if vals.get('attachment_ids'): for mail in self: mail.attachment_ids.check(mode='read') - if 'notification_ids' in vals or 'model' in vals or 'res_id' in vals: + if 'notification_ids' in vals or record_changed: self._invalidate_documents() return res @@ -1058,6 +1095,7 @@ class Message(models.Model): notification templates); :param mail_auto_delete: delete notification emails once sent; """ + msg_vals = msg_vals if msg_vals else {} rdata = self._notify_compute_recipients(record, msg_vals) return self._notify_recipients( @@ -1079,7 +1117,9 @@ class Message(models.Model): 'partners': [], 'channels': [], } - res = self.env['mail.followers']._get_recipient_data(record, subtype_id, pids, cids) + res = [] + user_notification = msg_vals.get('message_type') == 'user_notification' + res = self.env['mail.followers']._get_recipient_data(not user_notification and record, subtype_id, pids, cids) author_id = msg_vals.get('author_id') or self.author_id.id if res else False for pid, cid, active, pshare, ctype, notif, groups in res: if pid and pid == author_id and not self.env.context.get('mail_notify_author'): # do not notify the author of its own messages @@ -1214,7 +1254,7 @@ class Message(models.Model): }) # proceed with notification process to send notification emails and Inbox messages for message in self: - record = self.env[message.model].browse(message.res_id) if message.model and message.res_id else None + record = self.env[message.model].browse(message.res_id) if message.is_thread_message() else None message._notify(record, {}) @api.multi diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index b55f5cc0937..b53f9713583 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -91,7 +91,7 @@ class MailThread(models.AbstractModel): compute='_get_followers', search='_search_follower_channels') message_ids = fields.One2many( 'mail.message', 'res_id', string='Messages', - domain=lambda self: [('model', '=', self._name)], auto_join=True) + domain=lambda self: [('model', '=', self._name), ('message_type', '!=', 'user_notification')], auto_join=True) message_unread = fields.Boolean( 'Unread Messages', compute='_get_message_unread', help="If checked new messages require your attention.") @@ -187,8 +187,9 @@ class MailThread(models.AbstractModel): ON (cp.channel_id = rel.mail_channel_id AND cp.partner_id = %s AND (cp.seen_message_id IS NULL OR cp.seen_message_id < msg.id)) WHERE msg.model = %s AND msg.res_id = ANY(%s) AND + msg.message_type != 'user_notification' AND (msg.author_id IS NULL OR msg.author_id != %s) AND - (msg.message_type != 'notification' OR msg.model != 'mail.channel')""", + (msg.message_type not in ('notification', 'user_notification') OR msg.model != 'mail.channel')""", (partner_id, self._name, list(self.ids), partner_id,)) for result in self._cr.fetchall(): res[result[0]] += 1 @@ -205,7 +206,7 @@ class MailThread(models.AbstractModel): self._cr.execute(""" SELECT msg.res_id FROM mail_message msg RIGHT JOIN mail_message_res_partner_needaction_rel rel ON rel.mail_message_id = msg.id AND rel.res_partner_id = %s AND (rel.is_read = false OR rel.is_read IS NULL) - WHERE msg.model = %s AND msg.res_id in %s""", + WHERE msg.model = %s AND msg.res_id in %s AND msg.message_type != 'user_notification'""", (self.env.user.partner_id.id, self._name, tuple(self.ids),)) for result in self._cr.fetchall(): res[result[0]] += 1 @@ -225,7 +226,7 @@ class MailThread(models.AbstractModel): self._cr.execute(""" SELECT msg.res_id, COUNT(msg.res_id) FROM mail_message msg RIGHT JOIN mail_message_res_partner_needaction_rel rel ON rel.mail_message_id = msg.id AND rel.email_status in ('exception','bounce') - WHERE msg.author_id = %s AND msg.model = %s AND msg.res_id in %s + WHERE msg.author_id = %s AND msg.model = %s AND msg.res_id in %s AND msg.message_type != 'user_notification' GROUP BY msg.res_id""", (self.env.user.partner_id.id, self._name, tuple(self.ids),)) res.update(self._cr.fetchall()) @@ -341,7 +342,7 @@ class MailThread(models.AbstractModel): cascaded, because link is done through (res_model, res_id). """ if not self: return True - self.env['mail.message'].search([('model', '=', self._name), ('res_id', 'in', self.ids)]).unlink() + self.env['mail.message'].search([('model', '=', self._name), ('res_id', 'in', self.ids), ('message_type', '!=', 'user_notification')]).unlink() res = super(MailThread, self).unlink() self.env['mail.followers'].sudo().search( [('res_model', '=', self._name), ('res_id', 'in', self.ids)] @@ -534,10 +535,12 @@ class MailThread(models.AbstractModel): msg_comment = MailMessage.search([ ('model', '=', self._name), ('res_id', '=', self.id), + ('message_type', '!=', 'user_notification'), ('subtype_id', '=', subtype_comment)]) msg_not_comment = MailMessage.search([ ('model', '=', self._name), ('res_id', '=', self.id), + ('message_type', '!=', 'user_notification'), ('subtype_id', '!=', subtype_comment)]) # update the messages @@ -1948,7 +1951,7 @@ class MailThread(models.AbstractModel): # set in the context. model = False if self.ids: - self.ensure_one() + self.ensure_one() # not so usefull model = kwargs.get('model', False) if self._name == 'mail.thread' else self._name if model and model != self._name and hasattr(self.env[model], 'message_post'): return self.env[model].browse(self.ids).message_post( @@ -1991,8 +1994,10 @@ class MailThread(models.AbstractModel): subtype = 'mail.%s' % subtype subtype_id = self.env['ir.model.data'].xmlid_to_res_id(subtype) + user_notification = message_type == 'user_notification' + # automatically subscribe recipients if asked to - if self._context.get('mail_post_autofollow') and self.ids and partner_ids: + if self._context.get('mail_post_autofollow') and self.ids and not user_notification and partner_ids: partner_to_subscribe = partner_ids if self._context.get('mail_post_autofollow_partner_ids'): partner_to_subscribe = [p for p in partner_ids if p in self._context.get('mail_post_autofollow_partner_ids')] @@ -2000,8 +2005,8 @@ class MailThread(models.AbstractModel): # _mail_flat_thread: automatically set free messages to the first posted message MailMessage = self.env['mail.message'] - if self._mail_flat_thread and model and not parent_id and self.ids: - messages = MailMessage.search(['&', ('res_id', '=', self.ids[0]), ('model', '=', model)], order="id ASC", limit=1) + if self._mail_flat_thread and model and not parent_id and self.ids and not user_notification: + messages = MailMessage.search(['&', ('res_id', '=', self.ids[0]), ('model', '=', model), ('message_type', '!=', 'user_notification')], order="id ASC", limit=1) parent_id = messages.ids and messages.ids[0] or False # we want to set a parent: force to set the parent_id to the oldest ancestor, to avoid having more than 1 level of thread elif parent_id: @@ -2047,7 +2052,8 @@ class MailThread(models.AbstractModel): canned_response_ids = values.pop('canned_response_ids', False) new_message = MailMessage.create(values) values['canned_response_ids'] = canned_response_ids - self._message_post_after_hook(new_message, values, model_description=model_description, mail_auto_delete=mail_auto_delete) + record = self.env['mail.thread'] if user_notification else self + record._message_post_after_hook(new_message, values, model_description=model_description, mail_auto_delete=mail_auto_delete) return new_message def _message_post_after_hook(self, message, msg_vals, model_description=False, mail_auto_delete=True): @@ -2137,8 +2143,8 @@ class MailThread(models.AbstractModel): return composer.send_mail() def message_notify(self, partner_ids, body='', subject=False, **kwargs): - """ Shortcut allowing to notify partners of messages not linked to - any document. It pushes notifications on inbox or by email depending + """ Shortcut allowing to notify partners of messages that shouldn't be + displayed on a document. It pushes notifications on inbox or by email depending on the user configuration, like other notifications. """ kw_author = kwargs.pop('author_id', False) if kw_author: @@ -2154,16 +2160,15 @@ class MailThread(models.AbstractModel): 'body': body, 'author_id': author.id, 'email_from': email_from, - 'message_type': 'notification', + 'message_type': 'user_notification', 'partner_ids': partner_ids, - 'model': False, 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note'), 'record_name': False, 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from, records=None)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), } msg_values.update(kwargs) - return self.env['mail.thread'].message_post(**msg_values) + return self.message_post(**msg_values) def _message_log(self, body='', subject=False, message_type='notification', **kwargs): """ Shortcut allowing to post note on a document. It does not perform diff --git a/addons/mail/static/src/js/models/messages/message.js b/addons/mail/static/src/js/models/messages/message.js index 2e49975c9d9..73c4ece9025 100644 --- a/addons/mail/static/src/js/models/messages/message.js +++ b/addons/mail/static/src/js/models/messages/message.js @@ -368,7 +368,7 @@ var Message = AbstractMessage.extend(Mixins.EventDispatcherMixin, ServicesMixin * @return {boolean} */ isLinkedToDocumentThread: function () { - return !!(this._documentModel !== 'mail.channel' && this._documentID); + return !!(this._documentModel !== 'mail.channel' && this._documentID && this._type !== 'user_notification'); }, /** * State whether the current user is the author of this message diff --git a/addons/test_mail/tests/test_message_compose.py b/addons/test_mail/tests/test_message_compose.py index 2397023db24..b756d52d510 100644 --- a/addons/test_mail/tests/test_message_compose.py +++ b/addons/test_mail/tests/test_message_compose.py @@ -205,17 +205,19 @@ class TestMessagePost(BaseFunctionalTest, MockEmails, TestRecipients): def test_post_notify(self): self.user_employee.write({'notification_type': 'inbox'}) - new_notification = self.env['mail.thread'].message_notify( + new_notification = self.test_record.message_notify( subject='This should be a subject', body='

You have received a notification

', partner_ids=[(4, self.partner_1.id), (4, self.user_employee.partner_id.id)], ) self.assertEqual(new_notification.subtype_id, self.env.ref('mail.mt_note')) + self.assertEqual(new_notification.message_type, 'user_notification') self.assertEqual(new_notification.body, '

You have received a notification

') self.assertEqual(new_notification.author_id, self.env.user.partner_id) self.assertEqual(new_notification.email_from, formataddr((self.env.user.name, self.env.user.email))) self.assertEqual(new_notification.needaction_partner_ids, self.partner_1 | self.user_employee.partner_id) + self.assertNotIn(new_notification, self.test_record.message_ids) class TestComposer(BaseFunctionalTest, MockEmails, TestRecipients): diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 08ded3c4769..f87686cdf5d 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -485,7 +485,7 @@ class TestHeavyMailPerformance(TransactionCase): 'user_id': self.env.uid, }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(__system__=60, emp=75): # com runbot: 60 - 75 // test_mail only: 60 - 72 + with self.assertQueryCount(__system__=61, emp=75): # com runbot: 60 - 75 // test_mail only: 60 - 72 rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) # write tracking message From 2bfc0bc8931d33bf588c4899e50ac2ce77f0199a Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 12 Apr 2019 11:42:46 +0000 Subject: [PATCH 2/9] [IMP] test_mail: add a complex performance test for message_post It includes notification on inbox / email as well as attachments management. This latter one adds quite a lot of queries and is not currently tested for performances. Future commits will improve those counters. Related to task 1943901 Linked to PR #32404 --- addons/test_mail/tests/test_performance.py | 157 +++++++++++++++++++++ 1 file changed, 157 insertions(+) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index f87686cdf5d..4af461cef67 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import base64 from email.utils import formataddr @@ -617,3 +618,159 @@ class TestHeavyMailPerformance(TransactionCase): self.assertEqual(rec.message_ids[2].subtype_id, self.env.ref('test_mail.st_mail_test_full_umbrella_upd')) self.assertEqual(rec.message_ids[2].needaction_partner_ids, self.partners | self.user_portal.partner_id) self.assertEqual(len(rec.message_ids), 3) + + +@tagged('mail_performance_post') +class TestMailPerformancePost(TransactionCase): + + def setUp(self): + super(TestMailPerformancePost, self).setUp() + + self._quick_create_ctx = { + 'no_reset_password': True, + 'mail_create_nolog': True, + 'mail_create_nosubscribe': True, + 'mail_notrack': True, + } + self.user_employee = self.env['res.users'].with_context(self._quick_create_ctx).create({ + 'name': 'Ernest Employee', + 'login': 'emp', + 'email': 'e.e@example.com', + 'signature': '--\nErnest', + 'notification_type': 'inbox', + 'groups_id': [(6, 0, [self.env.ref('base.group_user').id])], + }) + # record + self.customer = self.env['res.partner'].with_context(self._quick_create_ctx).create({ + 'name': 'customer', + 'email': 'customer@example.com', + }) + self.record = self.env['mail.test'].with_context(mail_create_nosubscribe=True).create({ + 'name': 'Test record', + 'customer_id': self.customer.id, + 'alias_name': 'test-alias', + }) + # followers + self.user_follower_email = self.env['res.users'].with_context(self._quick_create_ctx).create({ + 'name': 'user_follower_email', + 'login': 'user_follower_email', + 'email': 'user_follower_email@example.com', + 'notification_type': 'email', + 'groups_id': [(6, 0, [self.env.ref('base.group_user').id])], + }) + self.user_follower_inbox = self.env['res.users'].with_context(self._quick_create_ctx).create({ + 'name': 'user_follower_inbox', + 'login': 'user_follower_inbox', + 'email': 'user_follower_inbox@example.com', + 'notification_type': 'inbox', + 'groups_id': [(6, 0, [self.env.ref('base.group_user').id])], + }) + self.partner_follower = self.env['res.partner'].with_context(self._quick_create_ctx).create({ + 'name': 'partner_follower', + 'email': 'partner_follower@example.com', + }) + self.record.message_subscribe([ + self.partner_follower.id, + self.user_follower_inbox.partner_id.id, + self.user_follower_email.partner_id.id + ]) + + # partner_ids + self.user_inbox = self.env['res.users'].with_context(self._quick_create_ctx).create({ + 'name': 'user_inbox', + 'login': 'user_inbox', + 'email': 'user_inbox@example.com', + 'notification_type': 'inbox', + 'groups_id': [(6, 0, [self.env.ref('base.group_user').id])], + }) + self.user_email = self.env['res.users'].with_context(self._quick_create_ctx).create({ + 'name': 'user_email', + 'login': 'user_email', + 'email': 'user_email@example.com', + 'notification_type': 'email', + 'groups_id': [(6, 0, [self.env.ref('base.group_user').id])], + }) + self.partner = self.env['res.partner'].with_context(self._quick_create_ctx).create({ + 'name': 'partner', + 'email': 'partner@example.com', + }) + # channels user/partner + self.partner_channel_inbox = self.env['res.partner'].with_context(self._quick_create_ctx).create({ + 'name': 'partner_channel_inbox', + 'email': 'partner_channel_inbox@example.com', + }) + self.partner_channel_email = self.env['res.partner'].with_context(self._quick_create_ctx).create({ + 'name': 'partner_channel_email', + 'email': 'partner_channel_email@example.com', + }) + self.user_channel_inbox = self.env['res.users'].with_context(self._quick_create_ctx).create({ + 'name': 'user_channel_inbox', + 'login': 'user_channel_inbox', + 'email': 'user_channel_inbox@example.com', + 'notification_type': 'inbox', + 'groups_id': [(6, 0, [self.env.ref('base.group_user').id])], + }) + self.user_channel_email = self.env['res.users'].with_context(self._quick_create_ctx).create({ + 'name': 'user_channel_email', + 'login': 'user_channel_email', + 'email': 'user_channel_email@example.com', + 'notification_type': 'inbox', + 'groups_id': [(6, 0, [self.env.ref('base.group_user').id])], + }) + # channels + self.channel_inbox = self.env['mail.channel'].with_context(self._quick_create_ctx).create({ + 'name': 'channel_inbox', + 'channel_partner_ids': [(4, self.partner_channel_inbox.id), (4, self.user_channel_inbox.partner_id.id)] + }) + self.channel_email = self.env['mail.channel'].with_context(self._quick_create_ctx).create({ + 'name': 'channel_email', + 'email_send': True, + 'channel_partner_ids': [(4, self.partner_channel_email.id), (4, self.user_channel_email.partner_id.id)] + }) + self.vals = [{ + 'datas': base64.b64encode(bytes("attachement content %s" % i, 'utf-8')), + 'name': 'attach %s' % i, + 'datas_fname': 'fileText_test%s.txt' % i, + 'mimetype': 'text/plain', + 'res_model': 'mail.compose.message', + 'res_id': 0, + } for i in range(3)] + + self.patch(self.env.registry, 'ready', True) + + @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') + @users('emp') + @warmup + def test_complete_message_post(self): + # aims to cover as much features of message_post as possible + partner_ids = [self.user_inbox.partner_id.id, self.user_email.partner_id.id, self.partner.id] + channel_ids = [(4, self.channel_inbox.id), (4, self.channel_email.id)] + record = self.record.sudo(self.env.user) + attachements = [ # not linear on number of attachements + ('attach tuple 1', "attachement tupple content 1"), + ('attach tuple 2', "attachement tupple content 2", {'cid': 'cid1'}), + ('attach tuple 3', "attachement tupple content 3", {'cid': 'cid2'}), + ] + self.attachements = self.env['ir.attachment'].sudo(self.env.user).create(self.vals) #-> 163-> 165 query + attachement_ids = self.attachements.ids + with self.assertQueryCount(emp=248): # test_mail only: 210 + self.cr.sql_log = self.warm and self.cr.sql_log_count + record.with_context({}).message_post( + body='

Test body

', + subject='Test Subject', + message_type='notification', + subtype=None, + partner_ids=partner_ids, + channel_ids=channel_ids, + parent_id=False, + attachments=attachements, + attachment_ids=attachement_ids, + add_sign=True, + model_description=False, + mail_auto_delete=True + ) + self.cr.sql_log = False + self.assertTrue(record.message_ids[0].body.startswith('

Test body Congratulations ! May I recommand you to setup an onboarding plan ?') % (url)) + employee._message_log(body=_('Congratulations ! May I recommand you to setup an onboarding plan ?') % (url)) if employee.department_id: self.env['mail.channel'].sudo().search([ ('subscription_department_ids', 'in', employee.department_id.id) @@ -363,8 +363,8 @@ class Employee(models.Model): self = self.sudo(real_user) return self - def _message_log(self, *args, **kwargs): - return super(Employee, self._post_author())._message_log(*args, **kwargs) + def _message_log(self, **kwargs): + return super(Employee, self._post_author())._message_log(**kwargs) @api.multi @api.returns('mail.message', lambda value: value.id) diff --git a/addons/hr_recruitment/models/hr_recruitment.py b/addons/hr_recruitment/models/hr_recruitment.py index 7bd1021769a..4efd09b9da0 100644 --- a/addons/hr_recruitment/models/hr_recruitment.py +++ b/addons/hr_recruitment/models/hr_recruitment.py @@ -414,7 +414,7 @@ class Applicant(models.Model): defaults.update(custom_values) return super(Applicant, self).message_new(msg, custom_values=defaults) - def _message_post_after_hook(self, message, *args, **kwargs): + def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): if self.email_from and not self.partner_id: # we consider that posting a message with a specified recipient (not a follower, a specific one) # on a document without customer means that it was created through the chatter using @@ -425,7 +425,7 @@ class Applicant(models.Model): ('partner_id', '=', False), ('email_from', '=', new_partner.email), ('stage_id.fold', '=', False)]).write({'partner_id': new_partner.id}) - return super(Applicant, self)._message_post_after_hook(message, *args, **kwargs) + return super(Applicant, self)._message_post_after_hook(message, msg_vals, model_description, mail_auto_delete) @api.multi def create_employee_from_applicant(self): diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 7c76f1162a1..6c1856154e1 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -301,7 +301,7 @@ class Channel(models.Model): if not self.email_send: notification = _('

left #%s
') % (self.id, self.name,) # post 'channel left' message as root since the partner just unsubscribed from the channel - self.sudo().message_post(body=notification, message_type="notification", subtype="mail.mt_comment", author_id=partner.id) + self.sudo().message_post(body=notification, subtype="mail.mt_comment", author_id=partner.id) return result @api.multi @@ -346,7 +346,7 @@ class Channel(models.Model): return super(Channel, self)._message_receive_bounce(email, partner, mail_id=mail_id) @api.multi - def _notify_email_recipients(self, message, recipient_ids): + def _notify_email_recipients(self, recipient_ids): # Excluded Blacklisted whitelist = self.env['res.partner'].sudo().search([('id', 'in', recipient_ids)]).filtered(lambda p: not p.is_blacklisted) # real mailing list: multiple recipients (hidden by X-Forge-To) @@ -355,7 +355,7 @@ class Channel(models.Model): 'email_to': ','.join(formataddr((partner.name, partner.email_normalized)) for partner in whitelist if partner.email_normalized), 'recipient_ids': [], } - return super(Channel, self)._notify_email_recipients(message, whitelist.ids) + return super(Channel, self)._notify_email_recipients(whitelist.ids) def _extract_moderation_values(self, message_type, **kwargs): """ This method is used to compute moderation status before the creation diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 0e595654885..d974f772b0c 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -1075,7 +1075,7 @@ class Message(models.Model): #------------------------------------------------------ @api.multi - def _notify(self, record, msg_vals, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True): + def _notify(self, record, msg_vals, force_send=False, model_description=False, mail_auto_delete=True): """ Main notification method. This method basically does two things * call ``_notify_compute_recipients`` that computes recipients to @@ -1089,8 +1089,6 @@ class Message(models.Model): simple cases where no notification is actually required; :param force_send: tells whether to send notification emails within the current transaction or to use the email queue; - :param send_after_commit: if force_send, tells whether to send emails after - the transaction has been committed using a post-commit hook; :param model_description: optional data used in notification process (see notification templates); :param mail_auto_delete: delete notification emails once sent; @@ -1100,7 +1098,7 @@ class Message(models.Model): rdata = self._notify_compute_recipients(record, msg_vals) return self._notify_recipients( rdata, record, msg_vals, - force_send=force_send, send_after_commit=send_after_commit, + force_send=force_send, model_description=model_description, mail_auto_delete=mail_auto_delete) @api.multi @@ -1131,21 +1129,19 @@ class Message(models.Model): pdata = {'id': pid, 'active': active, 'share': pshare, 'groups': groups} if notif == 'inbox': recipient_data['partners'].append(dict(pdata, notif=notif, type='user')) - else: - if not pshare and notif: # has an user and is not shared, is therefore user - recipient_data['partners'].append(dict(pdata, notif='email', type='user')) - elif pshare and notif: # has an user but is shared, is therefore portal - recipient_data['partners'].append(dict(pdata, notif='email', type='portal')) - else: # has no user, is therefore customer - recipient_data['partners'].append(dict(pdata, notif='email', type='customer')) + elif not pshare and notif: # has an user and is not shared, is therefore user + recipient_data['partners'].append(dict(pdata, notif='email', type='user')) + elif pshare and notif: # has an user but is shared, is therefore portal + recipient_data['partners'].append(dict(pdata, notif='email', type='portal')) + else: # has no user, is therefore customer + recipient_data['partners'].append(dict(pdata, notif='email', type='customer')) elif cid: recipient_data['channels'].append({'id': cid, 'notif': notif, 'type': ctype}) return recipient_data @api.multi def _notify_recipients(self, rdata, record, msg_vals, - force_send=False, send_after_commit=True, - model_description=False, mail_auto_delete=True): + force_send=False, model_description=False, mail_auto_delete=True): """ Main method implementing the notification process. :param rdata: dict containing recipients data: { @@ -1183,7 +1179,7 @@ class Message(models.Model): partner_email_rdata = [r for r in rdata['partners'] if r['notif'] == 'email'] if partner_email_rdata: - self.env['res.partner']._notify(self, partner_email_rdata, record, force_send=force_send, send_after_commit=send_after_commit, model_description=model_description, mail_auto_delete=mail_auto_delete) + self.env['res.partner']._notify(self, partner_email_rdata, record, force_send=force_send, model_description=model_description, mail_auto_delete=mail_auto_delete) if inbox_pids: self.env['res.partner'].browse(inbox_pids)._notify_by_chat(self) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index b53f9713583..07fcca7cb70 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -913,13 +913,12 @@ class MailThread(models.AbstractModel): return self._notify_specific_email_values(message) @api.multi - def _notify_email_recipients(self, message, recipient_ids): + def _notify_email_recipients(self, recipient_ids): """ Format email notification recipient values to store on the notification mail.mail. Basic method just set the recipient partners as mail_mail recipients. Override to generate other mail values like email_to or email_cc. - :param message: mail.message record being notified by email :param recipient_ids: res.partner recordset to notify """ return { @@ -927,13 +926,13 @@ class MailThread(models.AbstractModel): } @api.model - def _notify_email_recipients_on_records(self, message, recipient_ids, records=None): + def _notify_email_recipients_on_records(self, recipient_ids, records=None): """ Generic wrapper on ``_notify_email_recipients`` checking mail.thread inheritance and allowing to call model-specific implementation in a one liner. This method should not be overridden. """ if records and hasattr(records, '_notify_email_recipients'): - return records._notify_email_recipients(message, recipient_ids) - return self._notify_email_recipients(message, recipient_ids) + return records._notify_email_recipients(recipient_ids) + return self._notify_email_recipients(recipient_ids) # ------------------------------------------------------ # Mail gateway @@ -1346,7 +1345,7 @@ class MailThread(models.AbstractModel): if message_dict.get('parent_id'): parent_message = self.env['mail.message'].sudo().browse(message_dict['parent_id']) if parent_message.author_id: - partner_ids = [(4, parent_message.author_id.id)] + partner_ids = [parent_message.author_id.id] else: subtype_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') @@ -1650,8 +1649,7 @@ class MailThread(models.AbstractModel): msg_dict['email_from'] = tools.decode_smtp_header(message.get('from')) recipient_emails = ', '.join([tools.decode_smtp_header(message.get(h)) for h in ['To', 'Cc'] if message.get(h)]) partner_ids = [x.id for x in self._mail_find_partner_from_emails(tools.email_split(recipient_emails), records=self) if x] - msg_dict['partner_ids'] = [(4, partner_id) for partner_id in partner_ids] - + msg_dict['partner_ids'] = partner_ids if message.get('Date'): try: date_hdr = tools.decode_smtp_header(message.get('Date')) @@ -1965,19 +1963,9 @@ class MailThread(models.AbstractModel): if author_id is None: # keep False values author_id = self.env['mail.message']._get_default_author().id - # 2: Private message: add recipients (recipients and author of parent message) - current author - # + legacy-code management (! we manage only 4 and 6 commands) - partner_ids = set() - kwargs_partner_ids = kwargs.pop('partner_ids', []) - for partner_id in kwargs_partner_ids: - if isinstance(partner_id, (list, tuple)) and partner_id[0] == 4 and len(partner_id) == 2: - partner_ids.add(partner_id[1]) - if isinstance(partner_id, (list, tuple)) and partner_id[0] == 6 and len(partner_id) == 3: - partner_ids |= set(partner_id[2]) - elif isinstance(partner_id, int): - partner_ids.add(partner_id) - else: - pass # we do not manage anything else + partner_ids = set(kwargs.pop('partner_ids', [])) + channel_ids = set(kwargs.pop('channel_ids', [])) + if parent_id and not model: parent_message = self.env['mail.message'].browse(parent_id) private_followers = set([partner.id for partner in parent_message.partner_ids]) @@ -2031,7 +2019,7 @@ class MailThread(models.AbstractModel): 'parent_id': parent_id, 'subtype_id': subtype_id, 'partner_ids': [(4, pid) for pid in partner_ids], - 'channel_ids': kwargs.get('channel_ids', []), + 'channel_ids': [(4, cid) for cid in channel_ids], 'add_sign': add_sign }) if notif_layout: @@ -2053,10 +2041,10 @@ class MailThread(models.AbstractModel): new_message = MailMessage.create(values) values['canned_response_ids'] = canned_response_ids record = self.env['mail.thread'] if user_notification else self - record._message_post_after_hook(new_message, values, model_description=model_description, mail_auto_delete=mail_auto_delete) + record._message_post_after_hook(new_message, values, model_description, mail_auto_delete) return new_message - def _message_post_after_hook(self, message, msg_vals, model_description=False, mail_auto_delete=True): + def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): """ Hook to add custom behavior after having posted the message. Both message and computed value are given, to try to lessen query count by using already-computed values instead of having to rebrowse things. """ @@ -2073,7 +2061,6 @@ class MailThread(models.AbstractModel): message._notify( self, msg_vals, force_send=self.env.context.get('mail_notify_force_send', True), - send_after_commit=True, model_description=model_description, mail_auto_delete=mail_auto_delete, ) @@ -2354,7 +2341,7 @@ class MailThread(models.AbstractModel): record.message_notify( subject=_('You have been assigned to %s') % record.display_name, body=assignation_msg, - partner_ids=[(4, pid) for pid in partner_ids], + partner_ids=partner_ids, record_name=record.display_name, notif_layout='mail.mail_notification_light', model_description=model_description, diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 966d0bb2dbd..8174534ea26 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -147,7 +147,7 @@ class Partner(models.Model): # send email for email_chunk in split_every(50, group_tpl_values['recipients']): - recipient_values = self.env['mail.thread']._notify_email_recipients_on_records(message, email_chunk, records=record) + recipient_values = self.env['mail.thread']._notify_email_recipients_on_records(email_chunk, records=record) create_values = { 'body_html': mail_body, 'subject': mail_subject, diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 72076826085..fe09f0ff18c 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -184,9 +184,9 @@ class MailComposer(models.TransientModel): result['model'] = parent.model if not values.get('res_id'): result['res_id'] = parent.res_id - partner_ids = values.get('partner_ids', list()) + [(4, id) for id in parent.partner_ids.ids] + partner_ids = values.get('partner_ids', list()) + parent.partner_ids.ids if self._context.get('is_private') and parent.author_id: # check message is private then add author also in partner list. - partner_ids += [(4, parent.author_id.id)] + partner_ids += [parent.author_id.id] result['partner_ids'] = partner_ids elif values.get('model') and values.get('res_id'): doc_name_get = self.env[values.get('model')].browse(values.get('res_id')).name_get() diff --git a/addons/mail_bot/models/mail_thread.py b/addons/mail_bot/models/mail_thread.py index 81e9fa44711..9a5e2e05a99 100644 --- a/addons/mail_bot/models/mail_thread.py +++ b/addons/mail_bot/models/mail_thread.py @@ -7,6 +7,6 @@ from odoo import models class MailThread(models.AbstractModel): _inherit = 'mail.thread' - def _message_post_after_hook(self, message, values, model_description=False, mail_auto_delete=True): - self.env['mail.bot']._apply_logic(self, values) - return super(MailThread, self)._message_post_after_hook(message, values, model_description=model_description, mail_auto_delete=mail_auto_delete) + def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): + self.env['mail.bot']._apply_logic(self, msg_vals) + return super(MailThread, self)._message_post_after_hook(message, msg_vals, model_description, mail_auto_delete) diff --git a/addons/mass_mailing/controllers/main.py b/addons/mass_mailing/controllers/main.py index 3c75c7934a3..fda77b95c95 100644 --- a/addons/mass_mailing/controllers/main.py +++ b/addons/mass_mailing/controllers/main.py @@ -20,7 +20,7 @@ class MassMailController(http.Controller): def _log_blacklist_action(self, blacklist_entry, mailing_id, description): mailing = request.env['mail.mass_mailing'].sudo().browse(mailing_id) model_display = mailing.mailing_model_id.display_name - blacklist_entry._message_log(description + " ({})".format(model_display)) + blacklist_entry._message_log(body=description + " ({})".format(model_display)) @http.route(['/unsubscribe_from_list'], type='http', website=True, multilang=False, auth='public') def unsubscribe_placeholder_link(self, **post): diff --git a/addons/mass_mailing/models/mail_thread.py b/addons/mass_mailing/models/mail_thread.py index 9411d7300c3..4797fdfbff1 100644 --- a/addons/mass_mailing/models/mail_thread.py +++ b/addons/mass_mailing/models/mail_thread.py @@ -73,4 +73,4 @@ class MailThread(models.AbstractModel): if max(stats) > min(stats) + datetime.timedelta(weeks=1): blacklist_rec = self.env['mail.blacklist'].sudo()._add(email) blacklist_rec._message_log( - 'This email has been automatically blacklisted because of too much bounced.') + body='This email has been automatically blacklisted because of too much bounced.') diff --git a/addons/project/models/project.py b/addons/project/models/project.py index b9c597edd8e..9ebf3f8ee11 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -922,7 +922,7 @@ class Task(models.Model): res['headers'] = repr(headers) return res - def _message_post_after_hook(self, message, *args, **kwargs): + def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): if self.email_from and not self.partner_id: # we consider that posting a message with a specified recipient (not a follower, a specific one) # on a document without customer means that it was created through the chatter using @@ -933,7 +933,7 @@ class Task(models.Model): ('partner_id', '=', False), ('email_from', '=', new_partner.email), ('stage_id.fold', '=', False)]).write({'partner_id': new_partner.id}) - return super(Task, self)._message_post_after_hook(message, *args, **kwargs) + return super(Task, self)._message_post_after_hook(message, msg_vals, model_description, mail_auto_delete) def action_assign_to_me(self): self.write({'user_id': self.env.user.id}) diff --git a/addons/test_mail/tests/test_discuss.py b/addons/test_mail/tests/test_discuss.py index 2606a6aef3e..a14b10ffdd6 100644 --- a/addons/test_mail/tests/test_discuss.py +++ b/addons/test_mail/tests/test_discuss.py @@ -17,21 +17,21 @@ class TestChatterTweaks(BaseFunctionalTest, TestRecipients): def test_post_no_subscribe_recipients(self): original = self.test_record.message_follower_ids self.test_record.sudo(self.user_employee).with_context({'mail_create_nosubscribe': True}).message_post( - body='Test Body', message_type='comment', subtype='mt_comment', partner_ids=[(4, self.partner_1.id), (4, self.partner_2.id)]) + body='Test Body', message_type='comment', subtype='mt_comment', partner_ids=[self.partner_1.id, self.partner_2.id]) self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id')) self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) def test_post_subscribe_recipients(self): original = self.test_record.message_follower_ids self.test_record.sudo(self.user_employee).with_context({'mail_create_nosubscribe': True, 'mail_post_autofollow': True}).message_post( - body='Test Body', message_type='comment', subtype='mt_comment', partner_ids=[(4, self.partner_1.id), (4, self.partner_2.id)]) + body='Test Body', message_type='comment', subtype='mt_comment', partner_ids=[self.partner_1.id, self.partner_2.id]) self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id') | self.partner_1 | self.partner_2) self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) def test_post_subscribe_recipients_partial(self): original = self.test_record.message_follower_ids self.test_record.sudo(self.user_employee).with_context({'mail_create_nosubscribe': True, 'mail_post_autofollow': True, 'mail_post_autofollow_partner_ids': [self.partner_2.id]}).message_post( - body='Test Body', message_type='comment', subtype='mt_comment', partner_ids=[(4, self.partner_1.id), (4, self.partner_2.id)]) + body='Test Body', message_type='comment', subtype='mt_comment', partner_ids=[self.partner_1.id, self.partner_2.id]) self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id') | self.partner_2) self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) diff --git a/addons/test_mail/tests/test_message_compose.py b/addons/test_mail/tests/test_message_compose.py index b756d52d510..5b40c56e7cf 100644 --- a/addons/test_mail/tests/test_message_compose.py +++ b/addons/test_mail/tests/test_message_compose.py @@ -208,7 +208,7 @@ class TestMessagePost(BaseFunctionalTest, MockEmails, TestRecipients): new_notification = self.test_record.message_notify( subject='This should be a subject', body='

You have received a notification

', - partner_ids=[(4, self.partner_1.id), (4, self.user_employee.partner_id.id)], + partner_ids=[self.partner_1.id, self.user_employee.partner_id.id], ) self.assertEqual(new_notification.subtype_id, self.env.ref('mail.mt_note')) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 4af461cef67..01bc729d321 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -744,7 +744,7 @@ class TestMailPerformancePost(TransactionCase): def test_complete_message_post(self): # aims to cover as much features of message_post as possible partner_ids = [self.user_inbox.partner_id.id, self.user_email.partner_id.id, self.partner.id] - channel_ids = [(4, self.channel_inbox.id), (4, self.channel_email.id)] + channel_ids = [self.channel_inbox.id, self.channel_email.id] record = self.record.sudo(self.env.user) attachements = [ # not linear on number of attachements ('attach tuple 1', "attachement tupple content 1"), diff --git a/addons/website_crm_partner_assign/models/crm_lead.py b/addons/website_crm_partner_assign/models/crm_lead.py index a4fad8e8732..153aa703939 100644 --- a/addons/website_crm_partner_assign/models/crm_lead.py +++ b/addons/website_crm_partner_assign/models/crm_lead.py @@ -183,7 +183,7 @@ class CrmLead(models.Model): if comment: message += '

%s

' % comment for lead in self: - lead.message_post(body=message, subtype="mail.mt_note") + lead.message_post(body=message) lead.sudo().convert_opportunity(lead.partner_id.id) # sudo required to convert partner data @api.multi @@ -197,7 +197,7 @@ class CrmLead(models.Model): self.message_unsubscribe(partner_ids=partner_ids.ids) if comment: message += '

%s

' % comment - self.message_post(body=message, subtype="mail.mt_note") + self.message_post(body=message) values = { 'partner_assigned_id': False, } diff --git a/addons/website_event_track/models/event_track.py b/addons/website_event_track/models/event_track.py index 40c11351603..bdecccfd03c 100644 --- a/addons/website_event_track/models/event_track.py +++ b/addons/website_event_track/models/event_track.py @@ -163,7 +163,7 @@ class Track(models.Model): track._message_add_suggested_recipient(recipients, email=track.partner_email, reason=_('Speaker Email')) return recipients - def _message_post_after_hook(self, message, *args, **kwargs): + def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): if self.partner_email and not self.partner_id: # we consider that posting a message with a specified recipient (not a follower, a specific one) # on a document without customer means that it was created through the chatter using @@ -175,7 +175,7 @@ class Track(models.Model): ('partner_email', '=', new_partner.email), ('stage_id.is_cancel', '=', False), ]).write({'partner_id': new_partner.id}) - return super(Track, self)._message_post_after_hook(message, *args, **kwargs) + return super(Track, self)._message_post_after_hook(message, msg_vals, model_description, mail_auto_delete) @api.multi def open_track_speakers_list(self): diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index 98e64eeac77..db8c71ecf64 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -784,7 +784,6 @@ class Post(models.Model): @api.multi @api.returns('mail.message', lambda value: value.id) def message_post(self, message_type='notification', **kwargs): - question_followers = self.env['res.partner'] if self.ids and message_type == 'comment': # user comments have a restriction on karma # add followers of comments on the parent post if self.parent_id: @@ -795,7 +794,7 @@ class Post(models.Model): ('res_id', '=', self.parent_id.id), ('partner_id', '!=', False), ]).filtered(lambda fol: comment_subtype in fol.subtype_ids).mapped('partner_id') - partner_ids += [(4, partner.id) for partner in question_followers] + partner_ids += question_followers.ids kwargs['partner_ids'] = partner_ids self.ensure_one() From 6f43977a4e2837cc8a2b47f3cab086f334c20978 Mon Sep 17 00:00:00 2001 From: XavierDo Date: Fri, 12 Apr 2019 11:43:36 +0000 Subject: [PATCH 4/9] [REF] mail: improve message_post and message_notify API Message post should always be called on a record (ensure_one). That way we ensure posting a message is always done in a record's context with right values computed (reply_to, followers, ...) Message_notify can be called on record or on mail_thread and must have partner_ids. It is based on the recently modified user_notification mechanism and allow to notify a partner on a record or just to push him a message (aka, not linked to a record). Small performance improvement * browse recipients instead of search in _notify_email_recipients; * todo in future optimizations: mayybe be improve by searching on ids and is_blacklist immediately; Related to task 1943901 Linked to PR #32404 --- addons/crm/models/crm_lead.py | 2 +- addons/event/models/event.py | 4 +- addons/gamification/models/challenge.py | 4 +- addons/gamification/models/goal.py | 3 +- .../hr_recruitment/models/hr_recruitment.py | 4 +- addons/mail/models/mail_activity.py | 17 +- addons/mail/models/mail_channel.py | 14 +- addons/mail/models/mail_message.py | 31 +-- addons/mail/models/mail_thread.py | 211 ++++++++++-------- addons/mail/wizard/mail_compose_message.py | 10 +- addons/mail_bot/models/mail_bot.py | 2 +- addons/mail_bot/models/mail_thread.py | 4 +- addons/portal/controllers/mail.py | 12 +- addons/project/models/project.py | 4 +- addons/test_mail/tests/test_mail_gateway.py | 42 ---- .../website_event_track/models/event_track.py | 4 +- 16 files changed, 183 insertions(+), 185 deletions(-) diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index 3daa9c0879f..cf532292dec 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -1211,7 +1211,7 @@ class Lead(models.Model): defaults['company_id'] = self.env['crm.team'].browse(defaults['team_id']).company_id.id return super(Lead, self).message_new(msg_dict, custom_values=defaults) - def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): + def _message_post_after_hook(self, message, msg_vals): if self.email_from and not self.partner_id: # we consider that posting a message with a specified recipient (not a follower, a specific one) # on a document without customer means that it was created through the chatter using diff --git a/addons/event/models/event.py b/addons/event/models/event.py index 9ab5aee8e07..e2b54887bbd 100644 --- a/addons/event/models/event.py +++ b/addons/event/models/event.py @@ -503,7 +503,7 @@ class EventRegistration(models.Model): 'email_cc': False} for r in self} - def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): + def _message_post_after_hook(self, message, msg_vals): if self.email and not self.partner_id: # we consider that posting a message with a specified recipient (not a follower, a specific one) # on a document without customer means that it was created through the chatter using @@ -515,7 +515,7 @@ class EventRegistration(models.Model): ('email', '=', new_partner.email), ('state', 'not in', ['cancel']), ]).write({'partner_id': new_partner.id}) - return super(EventRegistration, self)._message_post_after_hook(message, msg_vals, model_description, mail_auto_delete) + return super(EventRegistration, self)._message_post_after_hook(message, msg_vals) @api.multi def action_send_badge_email(self): diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index 2c5c6c7e84d..c95e1f291d7 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -586,8 +586,8 @@ class Challenge(models.Model): 'gamification.challenge', challenge.id) - # send message only to users, not on the challenge - self.env['gamification.challenge'].message_post( + # notify message only to users, do not post on the challenge + challenge.message_notify( body=body_html, partner_ids=[user.partner_id.id], subtype='mail.mt_comment', diff --git a/addons/gamification/models/goal.py b/addons/gamification/models/goal.py index 506e34ceb83..81e8616b316 100644 --- a/addons/gamification/models/goal.py +++ b/addons/gamification/models/goal.py @@ -222,8 +222,7 @@ class Goal(models.Model): .get_email_template(self.id) body_html = self.env['mail.template'].with_context(template._context)\ ._render_template(template.body_html, 'gamification.goal', self.id) - # TDE NOTE: should be a notification (see with xdo) - self.env['mail.thread'].message_post( + self.message_notify( body=body_html, partner_ids=[self.user_id.partner_id.id], subtype='mail.mt_comment', diff --git a/addons/hr_recruitment/models/hr_recruitment.py b/addons/hr_recruitment/models/hr_recruitment.py index 4efd09b9da0..393125ba8dc 100644 --- a/addons/hr_recruitment/models/hr_recruitment.py +++ b/addons/hr_recruitment/models/hr_recruitment.py @@ -414,7 +414,7 @@ class Applicant(models.Model): defaults.update(custom_values) return super(Applicant, self).message_new(msg, custom_values=defaults) - def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): + def _message_post_after_hook(self, message, msg_vals): if self.email_from and not self.partner_id: # we consider that posting a message with a specified recipient (not a follower, a specific one) # on a document without customer means that it was created through the chatter using @@ -425,7 +425,7 @@ class Applicant(models.Model): ('partner_id', '=', False), ('email_from', '=', new_partner.email), ('stage_id.fold', '=', False)]).write({'partner_id': new_partner.id}) - return super(Applicant, self)._message_post_after_hook(message, msg_vals, model_description, mail_auto_delete) + return super(Applicant, self)._message_post_after_hook(message, msg_vals) @api.multi def create_employee_from_applicant(self): diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index ab098ed52a1..6b9ce7aacf2 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -387,14 +387,15 @@ class MailActivity(models.Model): minimal_qcontext=True ) record = self.env[activity.res_model].browse(activity.res_id) - record.message_notify( - partner_ids=activity.user_id.partner_id.ids, - body=body, - subject=_('%s: %s assigned to you') % (activity.res_name, activity.summary or activity.activity_type_id.name), - record_name=activity.res_name, - model_description=model_description, - notif_layout='mail.mail_notification_light', - ) + if activity.user_id: + record.message_notify( + partner_ids=activity.user_id.partner_id.ids, + body=body, + subject=_('%s: %s assigned to you') % (activity.res_name, activity.summary or activity.activity_type_id.name), + record_name=activity.res_name, + model_description=model_description, + notif_layout='mail.mail_notification_light', + ) @api.multi def action_done(self): diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 6c1856154e1..2c57c93d00b 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -348,7 +348,7 @@ class Channel(models.Model): @api.multi def _notify_email_recipients(self, recipient_ids): # Excluded Blacklisted - whitelist = self.env['res.partner'].sudo().search([('id', 'in', recipient_ids)]).filtered(lambda p: not p.is_blacklisted) + whitelist = self.env['res.partner'].sudo().browse(recipient_ids).filtered(lambda p: not p.is_blacklisted) # real mailing list: multiple recipients (hidden by X-Forge-To) if self.alias_domain and self.alias_name: return { @@ -525,6 +525,18 @@ class Channel(models.Model): notifications = self._channel_message_notifications(message) self.env['bus.bus'].sendmany(notifications) + def _notify_thread(self, message, msg_vals, model_description=False, mail_auto_delete=True): + # When posting a message on a mail channel, manage moderation and postpone notify users + if msg_vals.get('moderation_status') != 'pending_moderation': + super(Channel, self)._notify_thread( + message, + msg_vals, + model_description=model_description, + mail_auto_delete=mail_auto_delete, + ) + else: + message._notify_pending_by_chat() + @api.multi def _channel_message_notifications(self, message): """ Generate the bus notifications for the given message diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index d974f772b0c..515fc5dcfbe 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -1106,18 +1106,17 @@ class Message(models.Model): """ Compute recipients to notify based on subtype and followers. This method returns data structured as expected for ``_notify_recipients``. """ msg_sudo = self.sudo() - - pids = [x[1] for x in msg_vals.get('partner_ids')] if 'partner_ids' in msg_vals else msg_sudo.partner_ids.ids - cids = [x[1] for x in msg_vals.get('channel_ids')] if 'channel_ids' in msg_vals else msg_sudo.channel_ids.ids - subtype_id = msg_vals.get('subtype_id') if 'subtype_id' in msg_vals else msg_sudo.subtype_id.id - + # get values from msg_vals or from message if msg_vals doen't exists + pids = msg_vals.get('partner_ids', []) if msg_vals else msg_sudo.partner_ids.ids + cids = msg_vals.get('channel_ids', []) if msg_vals else msg_sudo.channel_ids.ids + subtype_id = msg_vals.get('subtype_id') if msg_vals else msg_sudo.subtype_id.id + # is it possible to have record but no subtype_id ? recipient_data = { 'partners': [], 'channels': [], } res = [] - user_notification = msg_vals.get('message_type') == 'user_notification' - res = self.env['mail.followers']._get_recipient_data(not user_notification and record, subtype_id, pids, cids) + res = self.env['mail.followers']._get_recipient_data(record, subtype_id, pids, cids) author_id = msg_vals.get('author_id') or self.author_id.id if res else False for pid, cid, active, pshare, ctype, notif, groups in res: if pid and pid == author_id and not self.env.context.get('mail_notify_author'): # do not notify the author of its own messages @@ -1169,13 +1168,18 @@ class Message(models.Model): # notify partners and channels if email_cids: + author_id = msg_vals.get('author_id') or self.author_id.id + email_from = msg_vals.get('email_from') or self.email_from + exept_partner = [r['id'] for r in rdata['partners']] + if author_id: + exept_partner.append(author_id) new_pids = self.env['res.partner'].sudo().search([ - ('id', 'not in', [r['id'] for r in rdata['partners']]), + ('id', 'not in', exept_partner), ('channel_ids', 'in', email_cids), - ('email', 'not in', [self.author_id.email, self.email_from]), + ('email', 'not in', [email_from]), ]) for partner in new_pids: - rdata['partners'].append({'id': partner.id, 'share': True, 'notif': 'email', 'type': 'customer', 'groups': []}) + rdata['partners'].append({'id': partner.id, 'share': True, 'active': True, 'notif': 'email', 'type': 'customer', 'groups': []}) partner_email_rdata = [r for r in rdata['partners'] if r['notif'] == 'email'] if partner_email_rdata: @@ -1250,8 +1254,9 @@ class Message(models.Model): }) # proceed with notification process to send notification emails and Inbox messages for message in self: - record = self.env[message.model].browse(message.res_id) if message.is_thread_message() else None - message._notify(record, {}) + if message.is_thread_message(): # note, since we will only intercept _notify_thread for message posted on channel, + # message will always be a thread_message. This check should always be true. + self.env[message.model].browse(message.res_id)._notify_thread(message, {}) @api.multi def _moderate_send_reject_email(self, subject, comment): @@ -1348,7 +1353,7 @@ class Message(models.Model): MailThread = self.env['mail.thread'].with_context(mail_notify_author=True) for moderator in moderators_to_notify: MailThread.message_notify( - moderator.partner_id.ids, + partner_ids=moderator.partner_id.ids, subject=_('Message are pending moderation'), # tocheck: target language body=template.render({'record': moderator.partner_id}, engine='ir.qweb', minimal_qcontext=True), email_from=moderator.company_id.catchall or moderator.company_id.email, diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 07fcca7cb70..9a7b157260c 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1344,15 +1344,17 @@ class MailThread(models.AbstractModel): subtype_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note') if message_dict.get('parent_id'): parent_message = self.env['mail.message'].sudo().browse(message_dict['parent_id']) - if parent_message.author_id: + if parent_message and parent_message.author_id: partner_ids = [parent_message.author_id.id] else: subtype_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') post_params = dict(subtype_id=subtype_id, partner_ids=partner_ids, **message_dict) - if not hasattr(thread, 'message_post'): - post_params['model'] = model - new_msg = thread.message_post(**post_params) + new_msg = False + if thread._name == 'mail.thread': # message with parent_id not linked to record + new_msg = thread.message_notify(**post_params) + else: + new_msg = thread.message_post(**post_params) if new_msg and original_partner_ids: # postponed after message_post, because this is an external message and we don't want to create @@ -1918,10 +1920,10 @@ class MailThread(models.AbstractModel): @api.multi @api.returns('mail.message', lambda value: value.id) - def message_post(self, body='', subject=None, - message_type='notification', subtype=None, - parent_id=False, attachments=None, - notif_layout=False, add_sign=True, model_description=False, + def message_post(self, body='', subject=None, subtype_id=False, + message_type='notification', subtype=None, author_id=None, partner_ids=None, channel_ids=None, + parent_id=False, attachments=None, attachment_ids=None, model=False, + add_sign=True, model_description=False, mail_auto_delete=True, **kwargs): """ Post a new message in an existing thread, returning the new mail.message ID. @@ -1940,52 +1942,33 @@ class MailThread(models.AbstractModel): to the related document. Should only be set by Chatter. :return int: ID of newly created mail.message """ - if attachments is None: - attachments = {} - if self.ids and not self.ensure_one(): - raise exceptions.Warning(_('Invalid record set: should be called as model (without records) or on single-record recordset')) + self.ensure_one() # should always be posted on a record, use message_notify if no record - # if we're processing a message directly coming from the gateway, the destination model was - # set in the context. - model = False - if self.ids: - self.ensure_one() # not so usefull - model = kwargs.get('model', False) if self._name == 'mail.thread' else self._name - if model and model != self._name and hasattr(self.env[model], 'message_post'): - return self.env[model].browse(self.ids).message_post( - body=body, subject=subject, message_type=message_type, - subtype=subtype, parent_id=parent_id, attachments=attachments, - notif_layout=notif_layout, add_sign=add_sign, - mail_auto_delete=mail_auto_delete, model_description=model_description, **kwargs) + if self._name == 'mail.thread' or not self.id or message_type == 'user_notification': + raise ValueError('message_post should only be call to post message on record. Use message_notify instead') - # 0: Find the message's author, because we need it for private discussion - author_id = kwargs.get('author_id') + if 'model' in kwargs or 'res_id' in kwargs: + raise ValueError("message_post doesn't support model and res_id parameters anymore. Please call message_post on record") + + # Find the message's author, because we need it for private discussion if author_id is None: # keep False values - author_id = self.env['mail.message']._get_default_author().id + author_id = self.env['mail.message']._get_default_author().id # self.env.user.partner_id - partner_ids = set(kwargs.pop('partner_ids', [])) - channel_ids = set(kwargs.pop('channel_ids', [])) + partner_ids = set(partner_ids or []) + channel_ids = set(channel_ids or []) - if parent_id and not model: - parent_message = self.env['mail.message'].browse(parent_id) - private_followers = set([partner.id for partner in parent_message.partner_ids]) - if parent_message.author_id: - private_followers.add(parent_message.author_id.id) - private_followers -= set([author_id]) - partner_ids |= private_followers + for pc_id in partner_ids | channel_ids: + if not isinstance(pc_id, int): + raise ValueError('message_post partner_ids and channel_ids must be integer list, not commands') - # 4: mail.message.subtype - subtype_id = kwargs.get('subtype_id', False) if not subtype_id: subtype = subtype or 'mt_note' if '.' not in subtype: subtype = 'mail.%s' % subtype subtype_id = self.env['ir.model.data'].xmlid_to_res_id(subtype) - user_notification = message_type == 'user_notification' - # automatically subscribe recipients if asked to - if self._context.get('mail_post_autofollow') and self.ids and not user_notification and partner_ids: + if self._context.get('mail_post_autofollow') and partner_ids: partner_to_subscribe = partner_ids if self._context.get('mail_post_autofollow_partner_ids'): partner_to_subscribe = [p for p in partner_ids if p in self._context.get('mail_post_autofollow_partner_ids')] @@ -1993,8 +1976,8 @@ class MailThread(models.AbstractModel): # _mail_flat_thread: automatically set free messages to the first posted message MailMessage = self.env['mail.message'] - if self._mail_flat_thread and model and not parent_id and self.ids and not user_notification: - messages = MailMessage.search(['&', ('res_id', '=', self.ids[0]), ('model', '=', model), ('message_type', '!=', 'user_notification')], order="id ASC", limit=1) + if self._mail_flat_thread and not parent_id: + messages = MailMessage.search(['&', ('res_id', '=', self.ids[0]), ('model', '=', self._name), ('message_type', '!=', 'user_notification')], order="id ASC", limit=1) parent_id = messages.ids and messages.ids[0] or False # we want to set a parent: force to set the parent_id to the oldest ancestor, to avoid having more than 1 level of thread elif parent_id: @@ -2011,65 +1994,57 @@ class MailThread(models.AbstractModel): values = kwargs values.update({ 'author_id': author_id, - 'model': model, - 'res_id': model and self.ids[0] or False, + 'model': self._name, + 'res_id': self._name and self.id or False, 'body': body, 'subject': subject or False, 'message_type': message_type, 'parent_id': parent_id, 'subtype_id': subtype_id, - 'partner_ids': [(4, pid) for pid in partner_ids], - 'channel_ids': [(4, cid) for cid in channel_ids], - 'add_sign': add_sign + 'partner_ids': partner_ids, + 'channel_ids': channel_ids, + 'add_sign': add_sign, }) - if notif_layout: - values['layout'] = notif_layout # 3. Attachments # - HACK TDE FIXME: Chatter: attachments linked to the document (not done JS-side), load the message - attachment_ids = self._message_post_process_attachments(attachments, kwargs.pop('attachment_ids', []), values) - values['attachment_ids'] = attachment_ids - # Avoid warnings about non-existing fields - for x in ('from', 'to', 'cc'): - values.pop(x, None) + values['attachment_ids'] = self._message_post_process_attachments(attachments or [], attachment_ids or [], values) + new_message = self._message_create(values) + # Set main attachment field if necessary + self._message_set_main_attachment_id(values['attachment_ids']) - # Post the message - # canned_response_ids are added by js to be used by other computations (odoobot) - # we need to pop it from values since it is not stored on mail.message - canned_response_ids = values.pop('canned_response_ids', False) - new_message = MailMessage.create(values) - values['canned_response_ids'] = canned_response_ids - record = self.env['mail.thread'] if user_notification else self - record._message_post_after_hook(new_message, values, model_description, mail_auto_delete) + if values['author_id'] and values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): + #if self.env['res.partner'].browse(values['author_id']).active: # we dont want to add odoobot/inactive as a follower + self._message_subscribe([values['author_id']]) + + self._message_post_after_hook(new_message, values) + self._notify_thread(new_message, values, model_description=model_description, mail_auto_delete=mail_auto_delete) return new_message - def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): - """ Hook to add custom behavior after having posted the message. Both - message and computed value are given, to try to lessen query count by - using already-computed values instead of having to rebrowse things. """ - # Set main attachment field if necessary - attachment_ids = msg_vals['attachment_ids'] - if not self._abstract and attachment_ids and self.ids and not self.message_main_attachment_id: + def _message_set_main_attachment_id(self, attachment_ids): # todo move this out of mail.thread + if not self._abstract and attachment_ids and not self.message_main_attachment_id: all_attachments = self.env['ir.attachment'].browse([attachment_tuple[1] for attachment_tuple in attachment_ids]) prioritary_attachments = all_attachments.filtered(lambda x: x.mimetype.endswith('pdf')) \ or all_attachments.filtered(lambda x: x.mimetype.startswith('image')) \ or all_attachments self.sudo().write({'message_main_attachment_id': prioritary_attachments[0].id}) - # Notify recipients of the newly-created message (Inbox / Email + channels) - if msg_vals.get('moderation_status') != 'pending_moderation': - message._notify( - self, msg_vals, - force_send=self.env.context.get('mail_notify_force_send', True), - model_description=model_description, - mail_auto_delete=mail_auto_delete, - ) - # Post-process: subscribe author - if msg_vals['author_id'] and msg_vals['model'] and self.ids and msg_vals['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): - self._message_subscribe([msg_vals['author_id']]) - else: - message._notify_pending_by_chat() + def _message_post_after_hook(self, message, msg_vals): + """ Hook to add custom behavior after having posted the message. Both + message and computed value are given, to try to lessen query count by + using already-computed values instead of having to rebrowse things. """ + pass + + @api.multi + def _notify_thread(self, message, msg_vals, model_description=False, mail_auto_delete=True): + # Can be overide to intercept and postpone notification mecanism + message._notify( + self, msg_vals, + force_send=self.env.context.get('mail_notify_force_send', True), + model_description=model_description, + mail_auto_delete=mail_auto_delete, + ) @api.multi def message_post_with_view(self, views_or_xmlid, **kwargs): @@ -2129,35 +2104,65 @@ class MailThread(models.AbstractModel): composer.write(update_values) return composer.send_mail() - def message_notify(self, partner_ids, body='', subject=False, **kwargs): + def message_notify(self, partner_ids=False, parent_id=False, model=False, res_id=False, + author_id=False, body='', subject=False, model_description=False, + mail_auto_delete=True, **kwargs): """ Shortcut allowing to notify partners of messages that shouldn't be displayed on a document. It pushes notifications on inbox or by email depending on the user configuration, like other notifications. """ - kw_author = kwargs.pop('author_id', False) - if kw_author: - author = self.env['res.partner'].sudo().browse(kw_author) + + # should we handle channel_ids here? + if self: + self.ensure_one() + + if author_id: + author = self.env['res.partner'].sudo().browse(author_id) else: author = self.env.user.partner_id + if not author.email: raise exceptions.UserError(_("Unable to notify message, please configure the sender's email address.")) email_from = formataddr((author.name, author.email)) - msg_values = { + partner_ids = partner_ids or set() + if parent_id: # looks like no test case are going throug this condition. Linked to private discussion. This may be removed soon. + parent_message = self.env['mail.message'].browse(parent_id) + private_followers = set([partner.id for partner in parent_message.partner_ids]) + if parent_message.author_id: + private_followers.add(parent_message.author.id) + private_followers -= set([author.id]) + partner_ids |= private_followers + + if not partner_ids: + _logger.warning('Message notify called without recipient_ids, skipping') + return self.env['mail_message'] + + if not (model and res_id): # both value should be set or none should be set (record) + model = False + res_id = False + + MailThread = self.env['mail.thread'] + values = { + 'parent_id': parent_id, + 'model': self._name if self else False, + 'res_id': self.id if self else False, + 'message_type': 'user_notification', 'subject': subject, 'body': body, 'author_id': author.id, 'email_from': email_from, - 'message_type': 'user_notification', 'partner_ids': partner_ids, 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note'), 'record_name': False, - 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from, records=None)[False], + 'reply_to': MailThread._notify_get_reply_to(default=email_from, records=None)[False], 'message_id': tools.generate_tracking_message_id('message-notify'), } - msg_values.update(kwargs) - return self.message_post(**msg_values) + values.update(kwargs) + new_message = MailThread._message_create(values) + MailThread._notify_thread(new_message, values, model_description=model_description, mail_auto_delete=mail_auto_delete) + return new_message - def _message_log(self, body='', subject=False, message_type='notification', **kwargs): + def _message_log(self, body='', author_id=None, subject=False, message_type='notification', **kwargs): """ 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 @@ -2167,14 +2172,15 @@ class MailThread(models.AbstractModel): if len(self.ids) > 1: raise exceptions.Warning(_('Invalid record set: should be called as model (without records) or on single-record recordset')) - kw_author = kwargs.pop('author_id', False) - if kw_author: - author = self.env['res.partner'].sudo().browse(kw_author) + if author_id: + author = self.env['res.partner'].sudo().browse(author_id) else: author = self.env.user.partner_id + author_id = author.id + if not author.email: raise exceptions.UserError(_("Unable to log message, please configure the sender's email address.")) - email_from = formataddr((author.name, author.email)) + email_from = author.email_formatted message_values = { 'subject': subject, @@ -2193,6 +2199,17 @@ class MailThread(models.AbstractModel): message = self.env['mail.message'].sudo().create(message_values) return message + def _message_create(self, values): + create_values = dict(values) + if 'notif_layout' in create_values: + create_values['layout'] = create_values.pop('notif_layout') + # Avoid warnings about non-existing fields + for x in ('from', 'to', 'cc', 'canned_response_ids'): + create_values.pop(x, None) + create_values['partner_ids'] = [(4, pid) for pid in create_values.get('partner_ids', [])] + create_values['channel_ids'] = [(4, cid) for cid in create_values.get('channel_ids', [])] + return self.env['mail.message'].create(create_values) + # ------------------------------------------------------ # Followers API # ------------------------------------------------------ diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index fe09f0ff18c..8053dfb4484 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -279,9 +279,13 @@ class MailComposer(models.TransientModel): mail_auto_delete=wizard.template_id.auto_delete if wizard.template_id else False, model_description=model_description, **mail_values) - if ActiveModel._name == 'mail.thread' and wizard.model: - post_params['model'] = wizard.model - ActiveModel.browse(res_id).message_post(**post_params) + if ActiveModel._name == 'mail.thread': + if wizard.model: + post_params['model'] = wizard.model + post_params['res_id'] = res_id + ActiveModel.message_notify(**post_params) + else: + ActiveModel.browse(res_id).message_post(**post_params) if wizard.composition_mode == 'mass_mail': batch_mails.send(auto_commit=auto_commit) diff --git a/addons/mail_bot/models/mail_bot.py b/addons/mail_bot/models/mail_bot.py index 96a7a1988dc..2e0ab860255 100644 --- a/addons/mail_bot/models/mail_bot.py +++ b/addons/mail_bot/models/mail_bot.py @@ -214,7 +214,7 @@ class MailBot(models.AbstractModel): def _is_bot_pinged(self, values): odoobot_id = self.env['ir.model.data'].xmlid_to_res_id("base.partner_root") - return (4, odoobot_id) in values.get('partner_ids', []) + return odoobot_id in values.get('partner_ids', []) def _is_bot_in_private_channel(self, record): odoobot_id = self.env['ir.model.data'].xmlid_to_res_id("base.partner_root") diff --git a/addons/mail_bot/models/mail_thread.py b/addons/mail_bot/models/mail_thread.py index 9a5e2e05a99..e1be43e7ca2 100644 --- a/addons/mail_bot/models/mail_thread.py +++ b/addons/mail_bot/models/mail_thread.py @@ -7,6 +7,6 @@ from odoo import models class MailThread(models.AbstractModel): _inherit = 'mail.thread' - def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): + def _message_post_after_hook(self, message, msg_vals): self.env['mail.bot']._apply_logic(self, msg_vals) - return super(MailThread, self)._message_post_after_hook(message, msg_vals, model_description, mail_auto_delete) + return super(MailThread, self)._message_post_after_hook(message, msg_vals) diff --git a/addons/portal/controllers/mail.py b/addons/portal/controllers/mail.py index 8d1378218a1..17849cdcda7 100644 --- a/addons/portal/controllers/mail.py +++ b/addons/portal/controllers/mail.py @@ -81,11 +81,13 @@ def _message_post_helper(res_model, res_id, message, token='', nosubscribe=True, kw.pop('attachment_ids', None) kw.pop('hash', None) kw.pop('pid', None) - return record.with_context(mail_create_nosubscribe=nosubscribe).message_post(body=message, - message_type=kw.pop('message_type', "comment"), - subtype=kw.pop('subtype', "mt_comment"), - author_id=author_id, - **kw) + return record.with_context(mail_create_nosubscribe=nosubscribe).message_post( + body=message, + message_type=kw.pop('message_type', "comment"), + subtype=kw.pop('subtype', "mt_comment"), + author_id=author_id, + **kw # TODO xdo check list of kw + ) class PortalChatter(http.Controller): diff --git a/addons/project/models/project.py b/addons/project/models/project.py index 9ebf3f8ee11..d20c5de5c1d 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -922,7 +922,7 @@ class Task(models.Model): res['headers'] = repr(headers) return res - def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): + def _message_post_after_hook(self, message, msg_vals): if self.email_from and not self.partner_id: # we consider that posting a message with a specified recipient (not a follower, a specific one) # on a document without customer means that it was created through the chatter using @@ -933,7 +933,7 @@ class Task(models.Model): ('partner_id', '=', False), ('email_from', '=', new_partner.email), ('stage_id.fold', '=', False)]).write({'partner_id': new_partner.id}) - return super(Task, self)._message_post_after_hook(message, msg_vals, model_description, mail_auto_delete) + return super(Task, self)._message_post_after_hook(message, msg_vals) def action_assign_to_me(self): self.write({'user_id': self.env.user.id}) diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py index 03373e585ee..f0868225e63 100644 --- a/addons/test_mail/tests/test_mail_gateway.py +++ b/addons/test_mail/tests/test_mail_gateway.py @@ -595,45 +595,3 @@ class TestMailgateway(BaseFunctionalTest, MockEmails): self.assertEqual(len(record), 1) self.assertEqual(record.name, 'Spammy') self.assertEqual(record._name, 'mail.test.gateway') - - @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models', 'odoo.addons.mail.models.mail_mail') - def test_private_discussion(self): - """ Testing private discussion between partners. """ - self.partner_2 = self.env['res.partner'].with_context(BaseFunctionalTest._test_context).create({ - 'name': 'Admin', - 'email': 'admin@agrolait.com', - }) - msg1_pids = [self.partner_1.id, self.partner_2.id] - - # Do: Raoul writes to Bert and Administrator, with a model specific by parameter that should not be taken into account - msg1 = self.env['mail.thread'].sudo(self.user_employee).message_post(partner_ids=msg1_pids, subtype='mail.mt_comment', model='mail.test') - - # Test: message recipients - msg = self.env['mail.message'].browse(msg1.id) - self.assertEqual(msg.partner_ids, self.partner_2 | self.partner_1, - 'message_post: private discussion: incorrect recipients') - self.assertEqual(msg.model, False, - 'message_post: private discussion: parameter model not correctly ignored when having no res_id') - # Test: message-id - self.assertIn('openerp-private', msg.message_id.split('@')[0], 'message_post: private discussion: message-id should contain the private keyword') - - # Do: Bert replies through mailgateway (is a customer) - self.format_and_process(MAIL_TEMPLATE, 'valid.lelitre@agrolait.com', 'not_important@mydomain.com', extra='In-Reply-To: %s' % msg.message_id) - - # Test: last mail_message created - msg2 = self.env['mail.message'].search([], limit=1) - # Test: message recipients - self.assertEqual(msg2.author_id, self.partner_1, - 'message_post: private discussion: wrong author through mailgateway based on email') - self.assertEqual(msg2.partner_ids, self.user_employee.partner_id | self.partner_2, - 'message_post: private discussion: incorrect recipients when replying') - - # Do: Bert replies through chatter (is a customer) - msg3 = self.env['mail.thread'].message_post(author_id=self.partner_1.id, parent_id=msg1.id, subtype='mail.mt_comment') - - # Test: message recipients - msg = self.env['mail.message'].browse(msg3.id) - self.assertEqual(msg.partner_ids, self.user_employee.partner_id | self.partner_2, - 'message_post: private discussion: incorrect recipients when replying') - self.assertEqual(msg.needaction_partner_ids, self.user_employee.partner_id | self.partner_2, - 'message_post: private discussion: incorrect notified recipients when replying') diff --git a/addons/website_event_track/models/event_track.py b/addons/website_event_track/models/event_track.py index bdecccfd03c..8d325150719 100644 --- a/addons/website_event_track/models/event_track.py +++ b/addons/website_event_track/models/event_track.py @@ -163,7 +163,7 @@ class Track(models.Model): track._message_add_suggested_recipient(recipients, email=track.partner_email, reason=_('Speaker Email')) return recipients - def _message_post_after_hook(self, message, msg_vals, model_description, mail_auto_delete): + def _message_post_after_hook(self, message, msg_vals): if self.partner_email and not self.partner_id: # we consider that posting a message with a specified recipient (not a follower, a specific one) # on a document without customer means that it was created through the chatter using @@ -175,7 +175,7 @@ class Track(models.Model): ('partner_email', '=', new_partner.email), ('stage_id.is_cancel', '=', False), ]).write({'partner_id': new_partner.id}) - return super(Track, self)._message_post_after_hook(message, msg_vals, model_description, mail_auto_delete) + return super(Track, self)._message_post_after_hook(message, msg_vals) @api.multi def open_track_speakers_list(self): From 7a6269719fb8663dd62e6313a04a549283989541 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 12 Apr 2019 11:43:39 +0000 Subject: [PATCH 5/9] [MOV] mail: move notification methods from mail.message / res.partner to mail.thread Purpose of this commit is to move notification methods to mail.thread. Indeed currently they are split among several models: thread, message, partner. It makes code difficult to understand. Base record has to be propagated in order to run methods on it. It is therefore simpler to make those methods on mail.thread and correctly have model methods. All notification methods are moved into a single section in the mail.thread file. This implies some more code move inside mail.thread but allow to have methods classified by their purpose. Related to task 1943901 Linked to PR #32404 --- addons/mail/models/mail_message.py | 123 --- addons/mail/models/mail_thread.py | 839 +++++++++++++------ addons/mail/models/res_partner.py | 164 +--- addons/portal/controllers/mail.py | 2 +- addons/test_mail/tests/test_message_track.py | 4 +- 5 files changed, 564 insertions(+), 568 deletions(-) diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 515fc5dcfbe..16f449c3301 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -1070,129 +1070,6 @@ class Message(models.Model): self._invalidate_documents() return super(Message, self).unlink() - #------------------------------------------------------ - # Messaging API - #------------------------------------------------------ - - @api.multi - def _notify(self, record, msg_vals, force_send=False, model_description=False, mail_auto_delete=True): - """ Main notification method. This method basically does two things - - * call ``_notify_compute_recipients`` that computes recipients to - notify based on message record or message creation values if given - (to optimize performance if we already have data computed); - * call ``_notify_recipients`` that performs the notification process; - - :param record: record on which the message is posted, if any; - :param msg_vals: dictionary of values used to create the message. If given - it is used instead of accessing ``self`` to lesen query count in some - simple cases where no notification is actually required; - :param force_send: tells whether to send notification emails within the - current transaction or to use the email queue; - :param model_description: optional data used in notification process (see - notification templates); - :param mail_auto_delete: delete notification emails once sent; - """ - - msg_vals = msg_vals if msg_vals else {} - rdata = self._notify_compute_recipients(record, msg_vals) - return self._notify_recipients( - rdata, record, msg_vals, - force_send=force_send, - model_description=model_description, mail_auto_delete=mail_auto_delete) - - @api.multi - def _notify_compute_recipients(self, record, msg_vals): - """ Compute recipients to notify based on subtype and followers. This - method returns data structured as expected for ``_notify_recipients``. """ - msg_sudo = self.sudo() - # get values from msg_vals or from message if msg_vals doen't exists - pids = msg_vals.get('partner_ids', []) if msg_vals else msg_sudo.partner_ids.ids - cids = msg_vals.get('channel_ids', []) if msg_vals else msg_sudo.channel_ids.ids - subtype_id = msg_vals.get('subtype_id') if msg_vals else msg_sudo.subtype_id.id - # is it possible to have record but no subtype_id ? - recipient_data = { - 'partners': [], - 'channels': [], - } - res = [] - res = self.env['mail.followers']._get_recipient_data(record, subtype_id, pids, cids) - author_id = msg_vals.get('author_id') or self.author_id.id if res else False - for pid, cid, active, pshare, ctype, notif, groups in res: - if pid and pid == author_id and not self.env.context.get('mail_notify_author'): # do not notify the author of its own messages - continue - if pid: - if active is False: - # avoid to notify inactive partner by email (odoobot) - continue - pdata = {'id': pid, 'active': active, 'share': pshare, 'groups': groups} - if notif == 'inbox': - recipient_data['partners'].append(dict(pdata, notif=notif, type='user')) - elif not pshare and notif: # has an user and is not shared, is therefore user - recipient_data['partners'].append(dict(pdata, notif='email', type='user')) - elif pshare and notif: # has an user but is shared, is therefore portal - recipient_data['partners'].append(dict(pdata, notif='email', type='portal')) - else: # has no user, is therefore customer - recipient_data['partners'].append(dict(pdata, notif='email', type='customer')) - elif cid: - recipient_data['channels'].append({'id': cid, 'notif': notif, 'type': ctype}) - return recipient_data - - @api.multi - def _notify_recipients(self, rdata, record, msg_vals, - force_send=False, model_description=False, mail_auto_delete=True): - """ Main method implementing the notification process. - - :param rdata: dict containing recipients data: { - 'partners': list of dict containing partner data: id, share status (boolean), - notification type ('email' or 'inbox'), type (main classification group used - in notification process), groups (user groups) - 'channels': list of dict containing channel data: id, notification type - ('email' for mailing list otherwise 'inbox'), type (channel_type) - } - """ - self.ensure_one() - - email_cids = [r['id'] for r in rdata['channels'] if r['notif'] == 'email'] - inbox_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'inbox'] - - message_values = {} - if rdata['channels']: - message_values['channel_ids'] = [(6, 0, [r['id'] for r in rdata['channels']])] - if rdata['partners']: - message_values['needaction_partner_ids'] = [(6, 0, [r['id'] for r in rdata['partners']])] - if message_values and record and hasattr(record, '_notify_customize_recipients'): - message_values.update(record._notify_customize_recipients(self, message_values, rdata)) - if message_values: - self.write(message_values) - - # notify partners and channels - if email_cids: - author_id = msg_vals.get('author_id') or self.author_id.id - email_from = msg_vals.get('email_from') or self.email_from - exept_partner = [r['id'] for r in rdata['partners']] - if author_id: - exept_partner.append(author_id) - new_pids = self.env['res.partner'].sudo().search([ - ('id', 'not in', exept_partner), - ('channel_ids', 'in', email_cids), - ('email', 'not in', [email_from]), - ]) - for partner in new_pids: - rdata['partners'].append({'id': partner.id, 'share': True, 'active': True, 'notif': 'email', 'type': 'customer', 'groups': []}) - - partner_email_rdata = [r for r in rdata['partners'] if r['notif'] == 'email'] - if partner_email_rdata: - self.env['res.partner']._notify(self, partner_email_rdata, record, force_send=force_send, model_description=model_description, mail_auto_delete=mail_auto_delete) - - if inbox_pids: - self.env['res.partner'].browse(inbox_pids)._notify_by_chat(self) - - if rdata['channels']: - self.env['mail.channel'].sudo().browse([r['id'] for r in rdata['channels']])._notify(self) - - return True - # -------------------------------------------------- # Moderation # -------------------------------------------------- diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 9a7b157260c..7dbb9d4ef4b 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -665,275 +665,6 @@ class MailThread(models.AbstractModel): return tracking - # ------------------------------------------------------ - # Email Notification - # ------------------------------------------------------ - - @api.model - def _notify_encode_link(self, base_link, params): - secret = self.env['ir.config_parameter'].sudo().get_param('database.secret') - token = '%s?%s' % (base_link, ' '.join('%s=%s' % (key, params[key]) for key in sorted(params))) - hm = hmac.new(secret.encode('utf-8'), token.encode('utf-8'), hashlib.sha1).hexdigest() - return hm - - @api.multi - def _notify_get_action_link(self, link_type, **kwargs): - local_kwargs = dict(kwargs) # do not modify in-place, modify copy instead - base_params = { - 'model': kwargs.get('model', self._name), - 'res_id': kwargs.get('res_id', self.ids and self.ids[0] or False), - } - - local_kwargs.pop('message_id', None) - local_kwargs.pop('model', None) - local_kwargs.pop('res_id', None) - - if link_type in ['view', 'assign', 'follow', 'unfollow']: - params = dict(base_params, **local_kwargs) - base_link = '/mail/%s' % link_type - elif link_type == 'controller': - controller = local_kwargs.pop('controller') - params = dict(base_params, **local_kwargs) - params.pop('model') - base_link = '%s' % controller - else: - return '' - - if link_type not in ['view']: - token = self._notify_encode_link(base_link, params) - params['token'] = token - - link = '%s?%s' % (base_link, url_encode(params)) - return link - - @api.multi - def _notify_get_groups(self, message, groups): - """ Return groups used to classify recipients of a notification email. - Groups is a list of tuple containing of form (group_name, group_func, - group_data) where - - * group_name is an identifier used only to be able to override and manipulate - groups. Default groups are user (recipients linked to an employee user), - portal (recipients linked to a portal user) and customer (recipients not - linked to any user). An example of override use would be to add a group - linked to a res.groups like Hr Officers to set specific action buttons to - them. - * group_func is a function pointer taking a partner record as parameter. This - method will be applied on recipients to know whether they belong to a given - group or not. Only first matching group is kept. Evaluation order is the - list order. - * group_data is a dict containing parameters for the notification email - - * has_button_access: whether to display Access in email. True - by default for new groups, False for portal / customer. - * button_access: dict with url and title of the button - * actions: list of action buttons to display in the notification email. - Each action is a dict containing url and title of the button. - - Groups has a default value that you can find in mail_thread - ``_notify_classify_recipients`` method. - """ - return groups - - @api.multi - def _notify_classify_recipients(self, message, recipient_data): - """ Classify recipients to be notified of a message in groups to have - specific rendering depending on their group. For example users could - have access to buttons customers should not have in their emails. - - Module-specific grouping should be done by overriding ``_notify_get_groups`` - method defined here-under. - - :param message: mail.message record about to be notified - :param recipients: res.partner recordset to notify UPDATE ME - """ - result = {} - - access_link = self._notify_get_action_link('view') - - if message.model: - model = self.with_lang().env['ir.model'] - model_name = model._get(message.model).display_name - view_title = _('View %s') % model_name - else: - view_title = _('View') - - default_groups = [ - ('user', lambda pdata: pdata['type'] == 'user', {}), - ('portal', lambda pdata: pdata['type'] == 'portal', { - 'has_button_access': False, - }), - ('customer', lambda pdata: True, { - 'has_button_access': False, - }) - ] - - groups = self._notify_get_groups(message, default_groups) - - for group_name, group_func, group_data in groups: - group_data.setdefault('has_button_access', True) - group_data.setdefault('button_access', { - 'url': access_link, - 'title': view_title}) - group_data.setdefault('actions', list()) - group_data.setdefault('recipients', list()) - - for recipient in recipient_data: - for group_name, group_func, group_data in groups: - if group_func(recipient): - group_data['recipients'].append(recipient['id']) - break - - for group_name, group_method, group_data in groups: - result[group_name] = group_data - - return result - - def _notify_classify_recipients_on_records(self, message, recipient_data, records=None): - """ Generic wrapper on ``_notify_classify_recipients`` checking mail.thread - inheritance and allowing to call model-specific implementation in a one liner. - This method should not be overridden. """ - if records and hasattr(records, '_notify_classify_recipients'): - return records._notify_classify_recipients(message, recipient_data) - return self._notify_classify_recipients(message, recipient_data) - - @api.multi - def _notify_get_reply_to(self, default=None, records=None, company=None, doc_names=None): - """ Returns the preferred reply-to email address when replying to a thread - on documents. Documents are either given by self it this method is called - as a true multi method on a record set or can be given using records to - have a generic implementation available for all models. - - Reply-to is formatted like "MyCompany MyDocument ". - Heuristic it the following: - - * search for specific aliases as they always have priority; it is limited - to aliases linked to documents (like project alias for task for example); - * use catchall address; - * use default; - - This method works on documents - - * as a true multi method for models inheriting from mail.thread; call - ``records._notify_get_reply_to(...)``; - * as a generic implementation if records are given; call ``MailThread. - _notify_get_reply_to(records=records, ...)``; - * as a generic implementation is self is a void mail.thread record set - meaning catchall is computed; call ``MailThread._notify_get_reply_to - (records=None)``; - - Tweak this method on a specific model by overriding if it inherits from - mail.thread. An example would be tasks taking their reply-to alias from - their project. - - :param default: default email if no alias or catchall is found; - :param records: record_set if self if a generic mail.thread record allowing - generic implementation of finding reply-to; - :param company: used to compute company name part of the from name; provide - it if already known, otherwise fall back on user company; - :param doc_names: dict(res_id, doc_name) used to compute doc name part of - the from name; provide it if already known to avoid queries, otherwise - name_get on document will be performed; - - :return result: dictionary. Keys are record IDs and value is formatted - like an email "Company_name Document_name "/ - """ - _records = self if self and self._name != 'mail.thread' else records - model = _records._name if _records and _records._name != 'mail.thread' else False - res_ids = _records.ids if _records and model else [] - _res_ids = res_ids or [False] # always have a default value located in False - - alias_domain = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.domain") - result = dict.fromkeys(_res_ids, False) - result_email = dict() - doc_names = doc_names if doc_names else dict() - - if alias_domain: - if model and res_ids: - if not doc_names: - doc_names = dict((rec.id, rec.display_name) for rec in _records) - - mail_aliases = self.env['mail.alias'].sudo().search([ - ('alias_parent_model_id.model', '=', model), - ('alias_parent_thread_id', 'in', res_ids), - ('alias_name', '!=', False)]) - # take only first found alias for each thread_id, to match order (1 found -> limit=1 for each res_id) - for alias in mail_aliases: - result_email.setdefault(alias.alias_parent_thread_id, '%s@%s' % (alias.alias_name, alias_domain)) - - # left ids: use catchall - left_ids = set(_res_ids) - set(result_email) - if left_ids: - catchall = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.alias") - if catchall: - result_email.update(dict((rid, '%s@%s' % (catchall, alias_domain)) for rid in left_ids)) - - # compute name of reply-to - TDE tocheck: quotes and stuff like that - company_name = company.name if company else self.env.company.name - for res_id in result_email.keys(): - name = '%s%s%s' % (company_name, ' ' if doc_names.get(res_id) else '', doc_names.get(res_id, '')) - result[res_id] = formataddr((name, result_email[res_id])) - - left_ids = set(_res_ids) - set(result_email) - if left_ids: - result.update(dict((res_id, default) for res_id in left_ids)) - - return result - - @api.model - def _notify_get_reply_to_on_records(self, default=None, records=None, company=None, doc_names=None): - """ Generic wrapper on ``_notify_get_reply_to`` checking mail.thread inheritance - and allowing to call model-specific implementation in a one liner. This - method should not be overridden. """ - if records and hasattr(records, '_notify_get_reply_to'): - return records._notify_get_reply_to(default=default, company=company, doc_names=doc_names) - return self._notify_get_reply_to(default=default, records=records, company=company, doc_names=doc_names) - - @api.multi - def _notify_specific_email_values(self, message): - """ Get specific notification email values to store on the notification - mail.mail. Override to add values related to a specific model. - - :param message: mail.message record being notified by email - """ - if not self: - return {} - self.ensure_one() - return {'headers': repr({ - 'X-Odoo-Objects': "%s-%s" % (self._name, self.id), - })} - - @api.model - def _notify_specific_email_values_on_records(self, message, records=None): - """ Generic wrapper on ``_notify_specific_email_values`` checking mail.thread - inheritance and allowing to call model-specific implementation in a one liner. - This method should not be overridden. """ - if records and hasattr(records, '_notify_specific_email_values'): - return records._notify_specific_email_values(message) - return self._notify_specific_email_values(message) - - @api.multi - def _notify_email_recipients(self, recipient_ids): - """ Format email notification recipient values to store on the notification - mail.mail. Basic method just set the recipient partners as mail_mail - recipients. Override to generate other mail values like email_to or - email_cc. - - :param recipient_ids: res.partner recordset to notify - """ - return { - 'recipient_ids': [(4, pid) for pid in recipient_ids] - } - - @api.model - def _notify_email_recipients_on_records(self, recipient_ids, records=None): - """ Generic wrapper on ``_notify_email_recipients`` checking mail.thread - inheritance and allowing to call model-specific implementation in a one liner. - This method should not be overridden. """ - if records and hasattr(records, '_notify_email_recipients'): - return records._notify_email_recipients(recipient_ids) - return self._notify_email_recipients(recipient_ids) - # ------------------------------------------------------ # Mail gateway # ------------------------------------------------------ @@ -2036,16 +1767,6 @@ class MailThread(models.AbstractModel): using already-computed values instead of having to rebrowse things. """ pass - @api.multi - def _notify_thread(self, message, msg_vals, model_description=False, mail_auto_delete=True): - # Can be overide to intercept and postpone notification mecanism - message._notify( - self, msg_vals, - force_send=self.env.context.get('mail_notify_force_send', True), - model_description=model_description, - mail_auto_delete=mail_auto_delete, - ) - @api.multi def message_post_with_view(self, views_or_xmlid, **kwargs): """ Helper method to send a mail / post a message using a view_id to @@ -2210,6 +1931,562 @@ class MailThread(models.AbstractModel): create_values['channel_ids'] = [(4, cid) for cid in create_values.get('channel_ids', [])] return self.env['mail.message'].create(create_values) + #------------------------------------------------------ + # Notification API + #------------------------------------------------------ + + @api.multi + def _notify_thread(self, message, msg_vals, model_description=False, mail_auto_delete=True): + # Can be overide to intercept and postpone notification mecanism + self._notify_message( + message, msg_vals, + force_send=self.env.context.get('mail_notify_force_send', True), + model_description=model_description, + mail_auto_delete=mail_auto_delete, + ) + + @api.multi + def _notify_message(self, message, msg_vals, force_send=False, model_description=False, mail_auto_delete=True): + """ Main notification method. This method basically does two things + + * call ``_notify_compute_recipients`` that computes recipients to + notify based on message record or message creation values if given + (to optimize performance if we already have data computed); + * call ``_notify_recipients`` that performs the notification process; + + :param record: record on which the message is posted, if any; + :param msg_vals: dictionary of values used to create the message. If given + it is used instead of accessing ``self`` to lesen query count in some + simple cases where no notification is actually required; + :param force_send: tells whether to send notification emails within the + current transaction or to use the email queue; + :param model_description: optional data used in notification process (see + notification templates); + :param mail_auto_delete: delete notification emails once sent; + """ + + msg_vals = msg_vals if msg_vals else {} + rdata = self._notify_compute_recipients(message, msg_vals) + return self._notify_recipients( + rdata, message, msg_vals, + force_send=force_send, + model_description=model_description, mail_auto_delete=mail_auto_delete) + + @api.multi + def _notify_recipients(self, rdata, message, msg_vals, + force_send=False, model_description=False, mail_auto_delete=True): + """ Main method implementing the notification process. + + :param rdata: dict containing recipients data: { + 'partners': list of dict containing partner data: id, share status (boolean), + notification type ('email' or 'inbox'), type (main classification group used + in notification process), groups (user groups) + 'channels': list of dict containing channel data: id, notification type + ('email' for mailing list otherwise 'inbox'), type (channel_type) + } + """ + message.ensure_one() + + email_cids = [r['id'] for r in rdata['channels'] if r['notif'] == 'email'] + inbox_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'inbox'] + + message_values = {} + if rdata['channels']: + message_values['channel_ids'] = [(6, 0, [r['id'] for r in rdata['channels']])] + if rdata['partners']: + message_values['needaction_partner_ids'] = [(6, 0, [r['id'] for r in rdata['partners']])] + if message_values and self and hasattr(self, '_notify_customize_recipients'): + message_values.update(self._notify_customize_recipients(message, msg_vals, rdata)) + if message_values: + message.write(message_values) + + # notify partners and channels + if email_cids: + author_id = msg_vals.get('author_id') or message.author_id.id + email_from = msg_vals.get('email_from') or message.email_from + exept_partner = [r['id'] for r in rdata['partners']] + if author_id: + exept_partner.append(author_id) + new_pids = self.env['res.partner'].sudo().search([ + ('id', 'not in', exept_partner), + ('channel_ids', 'in', email_cids), + ('email', 'not in', [email_from]), + ]) + for partner in new_pids: + rdata['partners'].append({'id': partner.id, 'share': True, 'active': True, 'notif': 'email', 'type': 'customer', 'groups': []}) + + partner_email_rdata = [r for r in rdata['partners'] if r['notif'] == 'email'] + if partner_email_rdata: + self._partner_notify(message, partner_email_rdata, force_send=force_send, model_description=model_description, mail_auto_delete=mail_auto_delete) + + if inbox_pids: + self.env['res.partner'].browse(inbox_pids)._notify_by_chat(message) + + if rdata['channels']: + self.env['mail.channel'].sudo().browse([r['id'] for r in rdata['channels']])._notify(message) + + return True + + @api.model + def _partner_notify(self, message, rdata, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True): + """ Method to send email linked to notified messages. The recipients are + the recordset on which this method is called. + + :param message: mail.message record to notify; + :param rdata: recipient data (see mail.message _notify); + :param force_send: tells whether to send notification emails within the + current transaction or to use the email queue; + :param send_after_commit: if force_send, tells whether to send emails after + the transaction has been committed using a post-commit hook; + :param model_description: optional data used in notification process (see + notification templates); + :param mail_auto_delete: delete notification emails once sent; + """ + if not rdata: + return True + + base_template_ctx = self._notify_prepare_template_context(message, model_description=model_description) + template_xmlid = message.layout if message.layout else 'mail.message_notification_email' + try: + base_template = self.env.ref(template_xmlid, raise_if_not_found=True).with_context(lang=base_template_ctx['lang']) + except ValueError: + _logger.warning('QWeb template %s not found when sending notification emails. Sending without layouting.' % (template_xmlid)) + base_template = False + + # prepare notification mail values + base_mail_values = { + 'mail_message_id': message.id, + 'mail_server_id': message.mail_server_id.id, + 'auto_delete': mail_auto_delete, + 'references': message.parent_id.message_id if message.parent_id else False + } + if self: + base_mail_values.update(self.env['mail.thread']._notify_specific_email_values_on_records(message, records=self)) + + # classify recipients: actions / no action + recipients = self.env['mail.thread']._notify_classify_recipients_on_records(message, rdata, records=self) + + Mail = self.env['mail.mail'].sudo() + emails = self.env['mail.mail'].sudo() + email_pids = set() + for group_tpl_values in [group for group in recipients.values() if group['recipients']]: + # generate notification email content + template_ctx = {**base_template_ctx, **group_tpl_values} + mail_body = base_template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) + mail_body = self.env['mail.thread']._replace_local_links(mail_body) + mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) + + # send email + for email_chunk in split_every(50, group_tpl_values['recipients']): + recipient_values = self.env['mail.thread']._notify_email_recipients_on_records(email_chunk, records=self) + create_values = { + 'body_html': mail_body, + 'subject': mail_subject, + } + create_values.update(base_mail_values) + create_values.update(recipient_values) + recipient_ids = [r[1] for r in create_values.get('recipient_ids', [])] + email = Mail.create(create_values) + + if email and recipient_ids: + notifications = self.env['mail.notification'].sudo().search([ + ('mail_message_id', '=', email.mail_message_id.id), + ('res_partner_id', 'in', list(recipient_ids)) + ]) + notifications.write({ + 'is_email': True, + 'mail_id': email.id, + 'is_read': True, # handle by email discards Inbox notification + 'email_status': 'ready', + }) + + emails |= email + email_pids.update(recipient_ids) + + # NOTE: + # 1. for more than 50 followers, use the queue system + # 2. do not send emails immediately if the registry is not loaded, + # to prevent sending email during a simple update of the database + # using the command-line. + test_mode = getattr(threading.currentThread(), 'testing', False) + if force_send and len(emails) < recipients_max and \ + (not self.pool._init or test_mode): + email_ids = emails.ids + dbname = self.env.cr.dbname + _context = self._context + + def send_notifications(): + db_registry = registry(dbname) + with api.Environment.manage(), db_registry.cursor() as cr: + env = api.Environment(cr, SUPERUSER_ID, _context) + env['mail.mail'].browse(email_ids).send() + + # unless asked specifically, send emails after the transaction to + # avoid side effects due to emails being sent while the transaction fails + if not test_mode and send_after_commit: + self._cr.after('commit', send_notifications) + else: + emails.send() + + return True + + @api.model + def _notify_prepare_template_context(self, message, model_description=False, mail_auto_delete=True): + # compute send user and its related signature + signature = '' + if message.author_id and message.author_id.user_ids: + user = message.author_id.user_ids[0] + if message.add_sign: + signature = user.signature + else: + user = self.env.user + if message.add_sign: + signature = "

--
%s

" % message.author_id.name + + company = self.company_id.sudo() if self and 'company_id' in self else user.company_id + if company.website: + website_url = 'http://%s' % company.website if not company.website.lower().startswith(('http:', 'https:')) else company.website + else: + website_url = False + + # Retrieve the language in which the template was rendered, in order to render the custom + # layout in the same language. + lang = self.env.context.get('lang') + if {'default_template_id', 'default_model', 'default_res_id'} <= self.env.context.keys(): + template = self.env['mail.template'].browse(self.env.context['default_template_id']) + if template and template.lang: + lang = template._render_template(template.lang, self.env.context['default_model'], self.env.context['default_res_id']) + + if not model_description and message.model: + model_description = self.env['ir.model'].with_context(lang=lang)._get(message.model).display_name + + tracking = [] + for tracking_value in self.env['mail.tracking.value'].sudo().search([('mail_message_id', '=', message.id)]): + groups = tracking_value.field_groups + if not groups or self.user_has_groups(groups): + tracking.append((tracking_value.field_desc, + tracking_value.get_old_display_value()[0], + tracking_value.get_new_display_value()[0])) + + is_discussion = message.subtype_id.id == self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') + + return { + 'message': message, + 'signature': signature, + 'website_url': website_url, + 'company': company, + 'model_description': model_description, + 'record': self, + 'record_name': message.record_name, + 'tracking_values': tracking, + 'is_discussion': is_discussion, + 'subtype': message.subtype_id, + 'lang': lang, + } + + @api.multi + def _notify_compute_recipients(self, message, msg_vals): + """ Compute recipients to notify based on subtype and followers. This + method returns data structured as expected for ``_notify_recipients``. """ + msg_sudo = message.sudo() + # get values from msg_vals or from message if msg_vals doen't exists + pids = msg_vals.get('partner_ids', []) if msg_vals else msg_sudo.partner_ids.ids + cids = msg_vals.get('channel_ids', []) if msg_vals else msg_sudo.channel_ids.ids + subtype_id = msg_vals.get('subtype_id') if msg_vals else msg_sudo.subtype_id.id + # is it possible to have record but no subtype_id ? + recipient_data = { + 'partners': [], + 'channels': [], + } + res = [] + res = self.env['mail.followers']._get_recipient_data(self, subtype_id, pids, cids) + author_id = msg_vals.get('author_id') or message.author_id.id if res else False + for pid, cid, active, pshare, ctype, notif, groups in res: + if pid and pid == author_id and not message.env.context.get('mail_notify_author'): # do not notify the author of its own messages + continue + if pid: + if active is False: + # avoid to notify inactive partner by email (odoobot) + continue + pdata = {'id': pid, 'active': active, 'share': pshare, 'groups': groups} + if notif == 'inbox': + recipient_data['partners'].append(dict(pdata, notif=notif, type='user')) + elif not pshare and notif: # has an user and is not shared, is therefore user + recipient_data['partners'].append(dict(pdata, notif='email', type='user')) + elif pshare and notif: # has an user but is shared, is therefore portal + recipient_data['partners'].append(dict(pdata, notif='email', type='portal')) + else: # has no user, is therefore customer + recipient_data['partners'].append(dict(pdata, notif='email', type='customer')) + elif cid: + recipient_data['channels'].append({'id': cid, 'notif': notif, 'type': ctype}) + return recipient_data + + + @api.model + def _notify_encode_link(self, base_link, params): + secret = self.env['ir.config_parameter'].sudo().get_param('database.secret') + token = '%s?%s' % (base_link, ' '.join('%s=%s' % (key, params[key]) for key in sorted(params))) + hm = hmac.new(secret.encode('utf-8'), token.encode('utf-8'), hashlib.sha1).hexdigest() + return hm + + @api.multi + def _notify_get_action_link(self, link_type, **kwargs): + local_kwargs = dict(kwargs) # do not modify in-place, modify copy instead + base_params = { + 'model': kwargs.get('model', self._name), + 'res_id': kwargs.get('res_id', self.ids and self.ids[0] or False), + } + + local_kwargs.pop('message_id', None) + local_kwargs.pop('model', None) + local_kwargs.pop('res_id', None) + + if link_type in ['view', 'assign', 'follow', 'unfollow']: + params = dict(base_params, **local_kwargs) + base_link = '/mail/%s' % link_type + elif link_type == 'controller': + controller = local_kwargs.pop('controller') + params = dict(base_params, **local_kwargs) + params.pop('model') + base_link = '%s' % controller + else: + return '' + + if link_type not in ['view']: + token = self._notify_encode_link(base_link, params) + params['token'] = token + + link = '%s?%s' % (base_link, url_encode(params)) + return link + + @api.multi + def _notify_get_groups(self, message, groups): + """ Return groups used to classify recipients of a notification email. + Groups is a list of tuple containing of form (group_name, group_func, + group_data) where + + * group_name is an identifier used only to be able to override and manipulate + groups. Default groups are user (recipients linked to an employee user), + portal (recipients linked to a portal user) and customer (recipients not + linked to any user). An example of override use would be to add a group + linked to a res.groups like Hr Officers to set specific action buttons to + them. + * group_func is a function pointer taking a partner record as parameter. This + method will be applied on recipients to know whether they belong to a given + group or not. Only first matching group is kept. Evaluation order is the + list order. + * group_data is a dict containing parameters for the notification email + + * has_button_access: whether to display Access in email. True + by default for new groups, False for portal / customer. + * button_access: dict with url and title of the button + * actions: list of action buttons to display in the notification email. + Each action is a dict containing url and title of the button. + + Groups has a default value that you can find in mail_thread + ``_notify_classify_recipients`` method. + """ + return groups + + @api.multi + def _notify_classify_recipients(self, message, recipient_data): + """ Classify recipients to be notified of a message in groups to have + specific rendering depending on their group. For example users could + have access to buttons customers should not have in their emails. + + Module-specific grouping should be done by overriding ``_notify_get_groups`` + method defined here-under. + + :param message: mail.message record about to be notified + :param recipients: res.partner recordset to notify UPDATE ME + """ + result = {} + + access_link = self._notify_get_action_link('view') + + if message.model: + model = self.with_lang().env['ir.model'] + model_name = model._get(message.model).display_name + view_title = _('View %s') % model_name + else: + view_title = _('View') + + default_groups = [ + ('user', lambda pdata: pdata['type'] == 'user', {}), + ('portal', lambda pdata: pdata['type'] == 'portal', { + 'has_button_access': False, + }), + ('customer', lambda pdata: True, { + 'has_button_access': False, + }) + ] + + groups = self._notify_get_groups(message, default_groups) + + for group_name, group_func, group_data in groups: + group_data.setdefault('has_button_access', True) + group_data.setdefault('button_access', { + 'url': access_link, + 'title': view_title}) + group_data.setdefault('actions', list()) + group_data.setdefault('recipients', list()) + + for recipient in recipient_data: + for group_name, group_func, group_data in groups: + if group_func(recipient): + group_data['recipients'].append(recipient['id']) + break + + for group_name, group_method, group_data in groups: + result[group_name] = group_data + + return result + + def _notify_classify_recipients_on_records(self, message, recipient_data, records=None): + """ Generic wrapper on ``_notify_classify_recipients`` checking mail.thread + inheritance and allowing to call model-specific implementation in a one liner. + This method should not be overridden. """ + if records and hasattr(records, '_notify_classify_recipients'): + return records._notify_classify_recipients(message, recipient_data) + return self._notify_classify_recipients(message, recipient_data) + + @api.multi + def _notify_get_reply_to(self, default=None, records=None, company=None, doc_names=None): + """ Returns the preferred reply-to email address when replying to a thread + on documents. Documents are either given by self it this method is called + as a true multi method on a record set or can be given using records to + have a generic implementation available for all models. + + Reply-to is formatted like "MyCompany MyDocument ". + Heuristic it the following: + + * search for specific aliases as they always have priority; it is limited + to aliases linked to documents (like project alias for task for example); + * use catchall address; + * use default; + + This method works on documents + + * as a true multi method for models inheriting from mail.thread; call + ``records._notify_get_reply_to(...)``; + * as a generic implementation if records are given; call ``MailThread. + _notify_get_reply_to(records=records, ...)``; + * as a generic implementation is self is a void mail.thread record set + meaning catchall is computed; call ``MailThread._notify_get_reply_to + (records=None)``; + + Tweak this method on a specific model by overriding if it inherits from + mail.thread. An example would be tasks taking their reply-to alias from + their project. + + :param default: default email if no alias or catchall is found; + :param records: record_set if self if a generic mail.thread record allowing + generic implementation of finding reply-to; + :param company: used to compute company name part of the from name; provide + it if already known, otherwise fall back on user company; + :param doc_names: dict(res_id, doc_name) used to compute doc name part of + the from name; provide it if already known to avoid queries, otherwise + name_get on document will be performed; + + :return result: dictionary. Keys are record IDs and value is formatted + like an email "Company_name Document_name "/ + """ + _records = self if self and self._name != 'mail.thread' else records + model = _records._name if _records and _records._name != 'mail.thread' else False + res_ids = _records.ids if _records and model else [] + _res_ids = res_ids or [False] # always have a default value located in False + + alias_domain = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.domain") + result = dict.fromkeys(_res_ids, False) + result_email = dict() + doc_names = doc_names if doc_names else dict() + + if alias_domain: + if model and res_ids: + if not doc_names: + doc_names = dict((rec.id, rec.display_name) for rec in _records) + + mail_aliases = self.env['mail.alias'].sudo().search([ + ('alias_parent_model_id.model', '=', model), + ('alias_parent_thread_id', 'in', res_ids), + ('alias_name', '!=', False)]) + # take only first found alias for each thread_id, to match order (1 found -> limit=1 for each res_id) + for alias in mail_aliases: + result_email.setdefault(alias.alias_parent_thread_id, '%s@%s' % (alias.alias_name, alias_domain)) + + # left ids: use catchall + left_ids = set(_res_ids) - set(result_email) + if left_ids: + catchall = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.alias") + if catchall: + result_email.update(dict((rid, '%s@%s' % (catchall, alias_domain)) for rid in left_ids)) + + # compute name of reply-to - TDE tocheck: quotes and stuff like that + company_name = company.name if company else self.env.company.name + for res_id in result_email.keys(): + name = '%s%s%s' % (company_name, ' ' if doc_names.get(res_id) else '', doc_names.get(res_id, '')) + result[res_id] = formataddr((name, result_email[res_id])) + + left_ids = set(_res_ids) - set(result_email) + if left_ids: + result.update(dict((res_id, default) for res_id in left_ids)) + + return result + + @api.model + def _notify_get_reply_to_on_records(self, default=None, records=None, company=None, doc_names=None): + """ Generic wrapper on ``_notify_get_reply_to`` checking mail.thread inheritance + and allowing to call model-specific implementation in a one liner. This + method should not be overridden. """ + if records and hasattr(records, '_notify_get_reply_to'): + return records._notify_get_reply_to(default=default, company=company, doc_names=doc_names) + return self._notify_get_reply_to(default=default, records=records, company=company, doc_names=doc_names) + + @api.multi + def _notify_specific_email_values(self, message): + """ Get specific notification email values to store on the notification + mail.mail. Override to add values related to a specific model. + + :param message: mail.message record being notified by email + """ + if not self: + return {} + self.ensure_one() + return {'headers': repr({ + 'X-Odoo-Objects': "%s-%s" % (self._name, self.id), + })} + + @api.model + def _notify_specific_email_values_on_records(self, message, records=None): + """ Generic wrapper on ``_notify_specific_email_values`` checking mail.thread + inheritance and allowing to call model-specific implementation in a one liner. + This method should not be overridden. """ + if records and hasattr(records, '_notify_specific_email_values'): + return records._notify_specific_email_values(message) + return self._notify_specific_email_values(message) + + @api.multi + def _notify_email_recipients(self, recipient_ids): + """ Format email notification recipient values to store on the notification + mail.mail. Basic method just set the recipient partners as mail_mail + recipients. Override to generate other mail values like email_to or + email_cc. + + :param recipient_ids: res.partner recordset to notify + """ + return { + 'recipient_ids': [(4, pid) for pid in recipient_ids] + } + + @api.model + def _notify_email_recipients_on_records(self, recipient_ids, records=None): + """ Generic wrapper on ``_notify_email_recipients`` checking mail.thread + inheritance and allowing to call model-specific implementation in a one liner. + This method should not be overridden. """ + if records and hasattr(records, '_notify_email_recipients'): + return records._notify_email_recipients(recipient_ids) + return self._notify_email_recipients(recipient_ids) + # ------------------------------------------------------ # Followers API # ------------------------------------------------------ @@ -2428,6 +2705,10 @@ class MailThread(models.AbstractModel): return True + # ------------------------------------------------------ + # Controllers + # ------------------------------------------------------ + def _get_mail_redirect_suggested_company(self): """ Return the suggested company to be set on the context in case of a mail redirection to the record. To avoid multi diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 8174534ea26..7a43a36ce38 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -2,11 +2,8 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import logging -import threading -from odoo.tools.misc import split_every - -from odoo import _, api, fields, models, registry, SUPERUSER_ID +from odoo import _, api, fields, models from odoo.addons.bus.models.bus_presence import AWAY_TIMER from odoo.addons.bus.models.bus_presence import DISCONNECTION_TIMER from odoo.osv import expression @@ -40,165 +37,6 @@ class Partner(models.Model): 'email_cc': False} for r in self} - @api.model - def _notify_prepare_template_context(self, message, record, model_description=False, mail_auto_delete=True): - # compute send user and its related signature - signature = '' - if message.author_id and message.author_id.user_ids: - user = message.author_id.user_ids[0] - if message.add_sign: - signature = user.signature - else: - user = self.env.user - if message.add_sign: - signature = "

--
%s

" % message.author_id.name - - company = record.company_id.sudo() if record and 'company_id' in record else user.company_id - if company.website: - website_url = 'http://%s' % company.website if not company.website.lower().startswith(('http:', 'https:')) else company.website - else: - website_url = False - - # Retrieve the language in which the template was rendered, in order to render the custom - # layout in the same language. - lang = self.env.context.get('lang') - if {'default_template_id', 'default_model', 'default_res_id'} <= self.env.context.keys(): - template = self.env['mail.template'].browse(self.env.context['default_template_id']) - if template and template.lang: - lang = template._render_template(template.lang, self.env.context['default_model'], self.env.context['default_res_id']) - - if not model_description and message.model: - model_description = self.env['ir.model'].with_context(lang=lang)._get(message.model).display_name - - tracking = [] - for tracking_value in self.env['mail.tracking.value'].sudo().search([('mail_message_id', '=', message.id)]): - groups = tracking_value.field_groups - if not groups or self.user_has_groups(groups): - tracking.append((tracking_value.field_desc, - tracking_value.get_old_display_value()[0], - tracking_value.get_new_display_value()[0])) - - is_discussion = message.subtype_id.id == self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') - - return { - 'message': message, - 'signature': signature, - 'website_url': website_url, - 'company': company, - 'model_description': model_description, - 'record': record, - 'record_name': message.record_name, - 'tracking_values': tracking, - 'is_discussion': is_discussion, - 'subtype': message.subtype_id, - 'lang': lang, - } - - @api.model - def _notify(self, message, rdata, record, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True): - """ Method to send email linked to notified messages. The recipients are - the recordset on which this method is called. - - :param message: mail.message record to notify; - :param rdata: recipient data (see mail.message _notify); - :param record: optional record on which the message was posted; - :param force_send: tells whether to send notification emails within the - current transaction or to use the email queue; - :param send_after_commit: if force_send, tells whether to send emails after - the transaction has been committed using a post-commit hook; - :param model_description: optional data used in notification process (see - notification templates); - :param mail_auto_delete: delete notification emails once sent; - """ - if not rdata: - return True - - base_template_ctx = self._notify_prepare_template_context(message, record, model_description=model_description) - template_xmlid = message.layout if message.layout else 'mail.message_notification_email' - try: - base_template = self.env.ref(template_xmlid, raise_if_not_found=True).with_context(lang=base_template_ctx['lang']) - except ValueError: - _logger.warning('QWeb template %s not found when sending notification emails. Sending without layouting.' % (template_xmlid)) - base_template = False - - # prepare notification mail values - base_mail_values = { - 'mail_message_id': message.id, - 'mail_server_id': message.mail_server_id.id, - 'auto_delete': mail_auto_delete, - 'references': message.parent_id.message_id if message.parent_id else False - } - if record: - base_mail_values.update(self.env['mail.thread']._notify_specific_email_values_on_records(message, records=record)) - - # classify recipients: actions / no action - recipients = self.env['mail.thread']._notify_classify_recipients_on_records(message, rdata, records=record) - - Mail = self.env['mail.mail'].sudo() - emails = self.env['mail.mail'].sudo() - email_pids = set() - recipients_nbr, recipients_max = 0, 50 - for group_tpl_values in [group for group in recipients.values() if group['recipients']]: - # generate notification email content - template_ctx = {**base_template_ctx, **group_tpl_values} - mail_body = base_template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) - mail_body = self.env['mail.thread']._replace_local_links(mail_body) - mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) - - # send email - for email_chunk in split_every(50, group_tpl_values['recipients']): - recipient_values = self.env['mail.thread']._notify_email_recipients_on_records(email_chunk, records=record) - create_values = { - 'body_html': mail_body, - 'subject': mail_subject, - } - create_values.update(base_mail_values) - create_values.update(recipient_values) - recipient_ids = [r[1] for r in create_values.get('recipient_ids', [])] - email = Mail.create(create_values) - - if email and recipient_ids: - notifications = self.env['mail.notification'].sudo().search([ - ('mail_message_id', '=', email.mail_message_id.id), - ('res_partner_id', 'in', list(recipient_ids)) - ]) - notifications.write({ - 'is_email': True, - 'mail_id': email.id, - 'is_read': True, # handle by email discards Inbox notification - 'email_status': 'ready', - }) - - emails |= email - email_pids.update(recipient_ids) - - # NOTE: - # 1. for more than 50 followers, use the queue system - # 2. do not send emails immediately if the registry is not loaded, - # to prevent sending email during a simple update of the database - # using the command-line. - test_mode = getattr(threading.currentThread(), 'testing', False) - if force_send and len(emails) < recipients_max and \ - (not self.pool._init or test_mode): - email_ids = emails.ids - dbname = self.env.cr.dbname - _context = self._context - - def send_notifications(): - db_registry = registry(dbname) - with api.Environment.manage(), db_registry.cursor() as cr: - env = api.Environment(cr, SUPERUSER_ID, _context) - env['mail.mail'].browse(email_ids).send() - - # unless asked specifically, send emails after the transaction to - # avoid side effects due to emails being sent while the transaction fails - if not test_mode and send_after_commit: - self._cr.after('commit', send_notifications) - else: - emails.send() - - return True - @api.multi def _notify_by_chat(self, message): """ Broadcast the message to all the partner since """ diff --git a/addons/portal/controllers/mail.py b/addons/portal/controllers/mail.py index 17849cdcda7..e9603edbaa9 100644 --- a/addons/portal/controllers/mail.py +++ b/addons/portal/controllers/mail.py @@ -86,7 +86,7 @@ def _message_post_helper(res_model, res_id, message, token='', nosubscribe=True, message_type=kw.pop('message_type', "comment"), subtype=kw.pop('subtype', "mt_comment"), author_id=author_id, - **kw # TODO xdo check list of kw + **kw ) diff --git a/addons/test_mail/tests/test_message_track.py b/addons/test_mail/tests/test_message_track.py index 6de2b4373f0..95bf2a5155f 100644 --- a/addons/test_mail/tests/test_message_track.py +++ b/addons/test_mail/tests/test_message_track.py @@ -163,8 +163,8 @@ class TestTracking(common.BaseFunctionalTest, common.MockEmails): self.record._fields['email_from'].groups = 'base.group_erp_manager' # patch the group attribute self.record.sudo().write({'email_from': 'X'}) - msg_emp = self.partner_employee.sudo(self.user_employee)._notify_prepare_template_context(self.record.message_ids, self.record) - msg_admin = self.partner_admin.sudo(self.user_admin)._notify_prepare_template_context(self.record.message_ids, self.record) + msg_emp = self.record.sudo(self.user_employee)._notify_prepare_template_context(self.record.message_ids) + msg_admin = self.record.sudo(self.user_admin)._notify_prepare_template_context(self.record.message_ids) self.assertFalse(msg_emp.get('tracking_values'), "should not have protected tracking values") self.assertTrue(msg_admin.get('tracking_values'), "should have protected tracking values") From b4e9b3fb45fc9816d178e4664156889a386175c5 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 12 Apr 2019 14:45:18 +0000 Subject: [PATCH 6/9] [FIX] mail: avoid crash when sending emails if no notification layout is found If no email layout is found when trying to render the layout around the message content, just send body in notification emails instead of crashing. See notably 89051058f4ab7069024f7f6cf2caac05bd68b499 . Related to task 1943901 Linked to PR #32404 --- addons/mail/models/mail_thread.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 7dbb9d4ef4b..98e9c29ce97 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2072,7 +2072,12 @@ class MailThread(models.AbstractModel): for group_tpl_values in [group for group in recipients.values() if group['recipients']]: # generate notification email content template_ctx = {**base_template_ctx, **group_tpl_values} - mail_body = base_template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) + # {company, is_discussion, lang, message, model_description, record, record_name, signature, subtype, tracking_values, website_url} + # {actions, button_access, has_button_access, recipients} + if base_template: + mail_body = base_template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) + else: + mail_body = message.body mail_body = self.env['mail.thread']._replace_local_links(mail_body) mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) @@ -2221,7 +2226,6 @@ class MailThread(models.AbstractModel): recipient_data['channels'].append({'id': cid, 'notif': notif, 'type': ctype}) return recipient_data - @api.model def _notify_encode_link(self, base_link, params): secret = self.env['ir.config_parameter'].sudo().get_param('database.secret') From 581a74143e9d215654b1988e2480c6516a3a4887 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 12 Apr 2019 11:43:43 +0000 Subject: [PATCH 7/9] [REF] mail: simplify notification code Purpose of this commit is to clean notification process: calls, methods API, method name, variable propagation. Contains notably * simplify API of methods used to group recipients when sending notification emails; * improve and rename methods used in email notification process; * move some methods on model itself as non mail thread records could be mass-mailed and _notify_email_headers could be called on other records; Related to task 1943901 Linked to PR #32404 --- addons/account/models/account_invoice.py | 9 +- addons/crm/models/crm_lead.py | 4 +- addons/hr_holidays/models/hr_leave.py | 4 +- .../hr_holidays/models/hr_leave_allocation.py | 4 +- addons/mail/models/mail_channel.py | 25 +- addons/mail/models/mail_message.py | 4 +- addons/mail/models/mail_thread.py | 358 ++++++++---------- addons/mail/models/models.py | 15 + addons/mail/models/res_partner.py | 1 + addons/mail/wizard/invite.py | 14 +- addons/mail/wizard/mail_compose_message.py | 7 +- addons/mail/wizard/mail_resend_message.py | 18 +- addons/portal/models/portal_mixin.py | 7 +- addons/project/models/project.py | 20 +- addons/rating/models/rating_mixin.py | 2 +- addons/sale/models/sale.py | 9 +- addons/test_mail/tests/test_message_track.py | 4 +- addons/website_blog/models/website_blog.py | 4 +- addons/website_forum/models/forum.py | 4 +- .../models/mail_channel.py | 11 +- addons/website_slides/models/slide_slide.py | 4 +- 21 files changed, 243 insertions(+), 285 deletions(-) diff --git a/addons/account/models/account_invoice.py b/addons/account/models/account_invoice.py index 9213b347573..47c0cb42b27 100644 --- a/addons/account/models/account_invoice.py +++ b/addons/account/models/account_invoice.py @@ -1060,17 +1060,16 @@ class AccountInvoice(models.Model): return self.filtered(lambda inv: inv.state != 'cancel').action_cancel() @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Give access button to users and portal customer as portal is integrated in account. Customer and portal group have probably no right to see the document so they don't have the access button. """ - groups = super(AccountInvoice, self)._notify_get_groups(message, groups) + groups = super(AccountInvoice, self)._notify_get_groups() if self.state not in ('draft', 'cancel'): for group_name, group_method, group_data in groups: - if group_name in ('customer', 'portal'): - continue - group_data['has_button_access'] = True + if group_name not in ('customer', 'portal'): + group_data['has_button_access'] = True return groups diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py index cf532292dec..29685f8e2b1 100644 --- a/addons/crm/models/crm_lead.py +++ b/addons/crm/models/crm_lead.py @@ -1114,10 +1114,10 @@ class Lead(models.Model): return super(Lead, self)._track_subtype(init_values) @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Handle salesman recipients that can convert leads into opportunities and set opportunities as won / lost. """ - groups = super(Lead, self)._notify_get_groups(message, groups) + groups = super(Lead, self)._notify_get_groups() self.ensure_one() if self.type == 'lead': diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index 061b76dbe1b..5789bee8aba 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -851,10 +851,10 @@ class HolidaysRequest(models.Model): return super(HolidaysRequest, self)._track_subtype(init_values) @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Handle HR users and officers recipients that can validate or refuse holidays directly from email. """ - groups = super(HolidaysRequest, self)._notify_get_groups(message, groups) + groups = super(HolidaysRequest, self)._notify_get_groups() self.ensure_one() hr_actions = [] diff --git a/addons/hr_holidays/models/hr_leave_allocation.py b/addons/hr_holidays/models/hr_leave_allocation.py index c953d32860f..91aa37b2e2d 100644 --- a/addons/hr_holidays/models/hr_leave_allocation.py +++ b/addons/hr_holidays/models/hr_leave_allocation.py @@ -611,10 +611,10 @@ class HolidaysAllocation(models.Model): return super(HolidaysAllocation, self)._track_subtype(init_values) @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Handle HR users and officers recipients that can validate or refuse holidays directly from email. """ - groups = super(HolidaysAllocation, self)._notify_get_groups(message, groups) + groups = super(HolidaysAllocation, self)._notify_get_groups() self.ensure_one() hr_actions = [] diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 2c57c93d00b..e2163810dab 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -305,24 +305,20 @@ class Channel(models.Model): return result @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ All recipients of a message on a channel are considered as partners. This means they will receive a minimal email, without a link to access in the backend. Mailing lists should indeed send minimal emails to avoid the noise. """ - groups = super(Channel, self)._notify_get_groups(message, groups) + groups = super(Channel, self)._notify_get_groups() for (index, (group_name, group_func, group_data)) in enumerate(groups): if group_name != 'customer': groups[index] = (group_name, lambda partner: False, group_data) return groups @api.multi - def _notify_specific_email_values(self, message): - res = super(Channel, self)._notify_specific_email_values(message) - try: - headers = safe_eval(res.get('headers', dict())) - except Exception: - headers = {} + def _notify_email_header_dict(self): + headers = super(Channel, self)._notify_email_header_dict() headers['Precedence'] = 'list' # avoid out-of-office replies from MS Exchange # http://blogs.technet.com/b/exchange/archive/2006/10/06/3395024.aspx @@ -334,8 +330,7 @@ class Channel(models.Model): # X-Forge-To: will replace To: after SMTP envelope is determined by ir.mail.server list_to = '"%s" <%s@%s>' % (self.name, self.alias_name, self.alias_domain) headers['X-Forge-To'] = list_to - res['headers'] = repr(headers) - return res + return headers @api.multi def _message_receive_bounce(self, email, partner, mail_id=None): @@ -346,7 +341,7 @@ class Channel(models.Model): return super(Channel, self)._message_receive_bounce(email, partner, mail_id=mail_id) @api.multi - def _notify_email_recipients(self, recipient_ids): + def _notify_email_recipient_values(self, recipient_ids): # Excluded Blacklisted whitelist = self.env['res.partner'].sudo().browse(recipient_ids).filtered(lambda p: not p.is_blacklisted) # real mailing list: multiple recipients (hidden by X-Forge-To) @@ -355,7 +350,7 @@ class Channel(models.Model): 'email_to': ','.join(formataddr((partner.name, partner.email_normalized)) for partner in whitelist if partner.email_normalized), 'recipient_ids': [], } - return super(Channel, self)._notify_email_recipients(whitelist.ids) + return super(Channel, self)._notify_email_recipient_values(whitelist.ids) def _extract_moderation_values(self, message_type, **kwargs): """ This method is used to compute moderation status before the creation @@ -525,12 +520,12 @@ class Channel(models.Model): notifications = self._channel_message_notifications(message) self.env['bus.bus'].sendmany(notifications) - def _notify_thread(self, message, msg_vals, model_description=False, mail_auto_delete=True): + def _notify_thread(self, message, msg_vals=False, model_description=False, mail_auto_delete=True): # When posting a message on a mail channel, manage moderation and postpone notify users - if msg_vals.get('moderation_status') != 'pending_moderation': + if not msg_vals or msg_vals.get('moderation_status') != 'pending_moderation': super(Channel, self)._notify_thread( message, - msg_vals, + msg_vals=msg_vals, model_description=model_description, mail_auto_delete=mail_auto_delete, ) diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 16f449c3301..5db3abe850f 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -1131,9 +1131,9 @@ class Message(models.Model): }) # proceed with notification process to send notification emails and Inbox messages for message in self: - if message.is_thread_message(): # note, since we will only intercept _notify_thread for message posted on channel, + if message.is_thread_message(): # note, since we will only intercept _notify_thread for message posted on channel, # message will always be a thread_message. This check should always be true. - self.env[message.model].browse(message.res_id)._notify_thread(message, {}) + self.env[message.model].browse(message.res_id)._notify_thread(message) @api.multi def _moderate_send_reject_email(self, subject, comment): diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 98e9c29ce97..b17937f1734 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -13,6 +13,7 @@ import pytz import re import socket import time +import threading try: from xmlrpc import client as xmlrpclib except ImportError: @@ -25,12 +26,12 @@ from lxml import etree from werkzeug import url_encode from werkzeug import urls -from odoo import _, api, exceptions, fields, models, tools +from odoo import _, api, exceptions, fields, models, tools, registry, SUPERUSER_ID from odoo.osv import expression -from odoo.tools import pycompat, ustr -from odoo.tools.misc import clean_context -from odoo.tools.safe_eval import safe_eval +from odoo.tools import pycompat, ustr +from odoo.tools.misc import clean_context, split_every +from odoo.tools.safe_eval import safe_eval _logger = logging.getLogger(__name__) @@ -1069,14 +1070,15 @@ class MailThread(models.AbstractModel): # replies to internal message are considered as notes, but parent message # author is added in recipients to ensure he is notified of a private answer + parent_message = False + if message_dict.get('parent_id'): + parent_message = self.env['mail.message'].sudo().browse(message_dict['parent_id']) partner_ids = [] if not subtype_id: if message_dict.pop('internal', False): subtype_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note') - if message_dict.get('parent_id'): - parent_message = self.env['mail.message'].sudo().browse(message_dict['parent_id']) - if parent_message and parent_message.author_id: - partner_ids = [parent_message.author_id.id] + if parent_message and parent_message.author_id: + partner_ids = [parent_message.author_id.id] else: subtype_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') @@ -1890,9 +1892,7 @@ class MailThread(models.AbstractModel): 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. """ - if len(self.ids) > 1: - raise exceptions.Warning(_('Invalid record set: should be called as model (without records) or on single-record recordset')) - + self.ensure_one() if author_id: author = self.env['res.partner'].sudo().browse(author_id) else: @@ -1914,10 +1914,10 @@ class MailThread(models.AbstractModel): 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note'), 'record_name': False, 'reply_to': self.env['mail.thread']._notify_get_reply_to(default=email_from, records=None)[False], - 'message_id': tools.generate_tracking_message_id('message-notify'), + 'message_id': tools.generate_tracking_message_id('message-notify'), # why? this is all but a notify } message_values.update(kwargs) - message = self.env['mail.message'].sudo().create(message_values) + message = self.sudo()._message_create(message_values) return message def _message_create(self, values): @@ -1931,30 +1931,19 @@ class MailThread(models.AbstractModel): create_values['channel_ids'] = [(4, cid) for cid in create_values.get('channel_ids', [])] return self.env['mail.message'].create(create_values) - #------------------------------------------------------ + # ------------------------------------------------------ # Notification API - #------------------------------------------------------ + # ------------------------------------------------------ @api.multi - def _notify_thread(self, message, msg_vals, model_description=False, mail_auto_delete=True): - # Can be overide to intercept and postpone notification mecanism - self._notify_message( - message, msg_vals, - force_send=self.env.context.get('mail_notify_force_send', True), - model_description=model_description, - mail_auto_delete=mail_auto_delete, - ) - - @api.multi - def _notify_message(self, message, msg_vals, force_send=False, model_description=False, mail_auto_delete=True): + def _notify_thread(self, message, msg_vals=False, model_description=False, mail_auto_delete=True): """ Main notification method. This method basically does two things - * call ``_notify_compute_recipients`` that computes recipients to notify based on message record or message creation values if given (to optimize performance if we already have data computed); - * call ``_notify_recipients`` that performs the notification process; - - :param record: record on which the message is posted, if any; + * performs the notification process; + Can be overridden to intercept and postpone notification mecanism (mail.channel moderation) + :param message: posted message; :param msg_vals: dictionary of values used to create the message. If given it is used instead of accessing ``self`` to lesen query count in some simple cases where no notification is actually required; @@ -1967,85 +1956,60 @@ class MailThread(models.AbstractModel): msg_vals = msg_vals if msg_vals else {} rdata = self._notify_compute_recipients(message, msg_vals) - return self._notify_recipients( - rdata, message, msg_vals, - force_send=force_send, - model_description=model_description, mail_auto_delete=mail_auto_delete) - - @api.multi - def _notify_recipients(self, rdata, message, msg_vals, - force_send=False, model_description=False, mail_auto_delete=True): - """ Main method implementing the notification process. - - :param rdata: dict containing recipients data: { - 'partners': list of dict containing partner data: id, share status (boolean), - notification type ('email' or 'inbox'), type (main classification group used - in notification process), groups (user groups) - 'channels': list of dict containing channel data: id, notification type - ('email' for mailing list otherwise 'inbox'), type (channel_type) - } - """ - message.ensure_one() - - email_cids = [r['id'] for r in rdata['channels'] if r['notif'] == 'email'] - inbox_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'inbox'] + if not rdata: + return False message_values = {} if rdata['channels']: message_values['channel_ids'] = [(6, 0, [r['id'] for r in rdata['channels']])] if rdata['partners']: - message_values['needaction_partner_ids'] = [(6, 0, [r['id'] for r in rdata['partners']])] + message_values['needaction_partner_ids'] = [(6, 0, [r['id'] for r in rdata['partners'] if rdata['partners'] != 'channel_email'])] + # change of behaviour to check: since email_cids partner are added in _notify_compute_recipients, + # they will be added to needaction_partner_ids to. + # we may want to filter them (example with channel_email, a cleaner solution may be great) + # -> instead of using _notify_customize_recipients, we could add a flag on rdata + # (would work for needactions, not if we want to erase partner_ids, ids) + # (could also be interresting for, we could add partners with r['notif'] = 'ocn_client' and r['needaction']=False) + # then overide a notify_recipients (as it was before) to effectively send ocn notifications. + # enveloppe will contain more needaction, those for the member of a email channel. if message_values and self and hasattr(self, '_notify_customize_recipients'): message_values.update(self._notify_customize_recipients(message, msg_vals, rdata)) if message_values: message.write(message_values) - # notify partners and channels - if email_cids: - author_id = msg_vals.get('author_id') or message.author_id.id - email_from = msg_vals.get('email_from') or message.email_from - exept_partner = [r['id'] for r in rdata['partners']] - if author_id: - exept_partner.append(author_id) - new_pids = self.env['res.partner'].sudo().search([ - ('id', 'not in', exept_partner), - ('channel_ids', 'in', email_cids), - ('email', 'not in', [email_from]), - ]) - for partner in new_pids: - rdata['partners'].append({'id': partner.id, 'share': True, 'active': True, 'notif': 'email', 'type': 'customer', 'groups': []}) - + inbox_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'inbox'] partner_email_rdata = [r for r in rdata['partners'] if r['notif'] == 'email'] if partner_email_rdata: - self._partner_notify(message, partner_email_rdata, force_send=force_send, model_description=model_description, mail_auto_delete=mail_auto_delete) - + self._notify_record_by_email(message, partner_email_rdata, msg_vals=msg_vals, model_description=model_description, mail_auto_delete=mail_auto_delete) if inbox_pids: self.env['res.partner'].browse(inbox_pids)._notify_by_chat(message) - if rdata['channels']: + # send a notification on bus to update channel self.env['mail.channel'].sudo().browse([r['id'] for r in rdata['channels']])._notify(message) - return True - @api.model - def _partner_notify(self, message, rdata, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True): - """ Method to send email linked to notified messages. The recipients are - the recordset on which this method is called. - + def _notify_record_by_email(self, message, partners_data, msg_vals=False, model_description=False, mail_auto_delete=True, send_after_commit=False): + """ Method to send email linked to notified messages. :param message: mail.message record to notify; - :param rdata: recipient data (see mail.message _notify); - :param force_send: tells whether to send notification emails within the - current transaction or to use the email queue; + :param partners_data: partner to notify by email coming from _notify_compute_recipients + :param msg_vals: message creation values if available :param send_after_commit: if force_send, tells whether to send emails after the transaction has been committed using a post-commit hook; :param model_description: optional data used in notification process (see notification templates); :param mail_auto_delete: delete notification emails once sent; """ - if not rdata: + model = msg_vals.get('model') if msg_vals else message.model + model_name = model_description or (self.with_lang().env['ir.model']._get(model).display_name if model else False) + recipients_groups_data = self._notify_classify_recipients(partners_data, model_name) + + if not recipients_groups_data: return True - base_template_ctx = self._notify_prepare_template_context(message, model_description=model_description) + force_send = self.env.context.get('mail_notify_force_send', True) + + base_template_ctx = self._notify_prepare_template_context(message, msg_vals, model_description=model_description) + template_xmlid = message.layout if message.layout else 'mail.message_notification_email' try: base_template = self.env.ref(template_xmlid, raise_if_not_found=True).with_context(lang=base_template_ctx['lang']) @@ -2060,37 +2024,41 @@ class MailThread(models.AbstractModel): 'auto_delete': mail_auto_delete, 'references': message.parent_id.message_id if message.parent_id else False } - if self: - base_mail_values.update(self.env['mail.thread']._notify_specific_email_values_on_records(message, records=self)) - - # classify recipients: actions / no action - recipients = self.env['mail.thread']._notify_classify_recipients_on_records(message, rdata, records=self) + headers = self._notify_email_headers() + if headers: + base_mail_values['headers'] = headers Mail = self.env['mail.mail'].sudo() emails = self.env['mail.mail'].sudo() email_pids = set() - for group_tpl_values in [group for group in recipients.values() if group['recipients']]: + + # loop on groups (customer, portal, user, ... + model specific like group_sale_salesman) + recipients_max = 50 + for recipients_group_data in recipients_groups_data: # generate notification email content - template_ctx = {**base_template_ctx, **group_tpl_values} + template_ctx = {**base_template_ctx, **recipients_group_data} # {company, is_discussion, lang, message, model_description, record, record_name, signature, subtype, tracking_values, website_url} # {actions, button_access, has_button_access, recipients} if base_template: mail_body = base_template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) else: mail_body = message.body - mail_body = self.env['mail.thread']._replace_local_links(mail_body) + mail_body = self._replace_local_links(mail_body) mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) - # send email - for email_chunk in split_every(50, group_tpl_values['recipients']): - recipient_values = self.env['mail.thread']._notify_email_recipients_on_records(email_chunk, records=self) + for email_chunk in split_every(recipients_max, recipients_group_data['recipients']): + recipient_values = self._notify_email_recipient_values(email_chunk) + email_to = recipient_values['email_to'] + recipient_ids = recipient_values['recipient_ids'] + create_values = { 'body_html': mail_body, 'subject': mail_subject, + 'recipient_ids': [(4, pid) for pid in recipient_ids], } - create_values.update(base_mail_values) - create_values.update(recipient_values) - recipient_ids = [r[1] for r in create_values.get('recipient_ids', [])] + if email_to: + create_values['email_to'] = email_to + create_values.update(base_mail_values) # mail_message_id, mail_server_id, auto_delete, references, headers email = Mail.create(create_values) if email and recipient_ids: @@ -2104,7 +2072,6 @@ class MailThread(models.AbstractModel): 'is_read': True, # handle by email discards Inbox notification 'email_status': 'ready', }) - emails |= email email_pids.update(recipient_ids) @@ -2114,21 +2081,19 @@ class MailThread(models.AbstractModel): # to prevent sending email during a simple update of the database # using the command-line. test_mode = getattr(threading.currentThread(), 'testing', False) - if force_send and len(emails) < recipients_max and \ - (not self.pool._init or test_mode): + if force_send and len(emails) < recipients_max and (not self.pool._init or test_mode): email_ids = emails.ids dbname = self.env.cr.dbname _context = self._context - def send_notifications(): - db_registry = registry(dbname) - with api.Environment.manage(), db_registry.cursor() as cr: - env = api.Environment(cr, SUPERUSER_ID, _context) - env['mail.mail'].browse(email_ids).send() - # unless asked specifically, send emails after the transaction to # avoid side effects due to emails being sent while the transaction fails if not test_mode and send_after_commit: + def send_notifications(): + db_registry = registry(dbname) + with api.Environment.manage(), db_registry.cursor() as cr: + env = api.Environment(cr, SUPERUSER_ID, _context) + env['mail.mail'].browse(email_ids).send() self._cr.after('commit', send_notifications) else: emails.send() @@ -2136,15 +2101,15 @@ class MailThread(models.AbstractModel): return True @api.model - def _notify_prepare_template_context(self, message, model_description=False, mail_auto_delete=True): + def _notify_prepare_template_context(self, message, msg_vals, model_description=False, mail_auto_delete=True): # compute send user and its related signature signature = '' + user = self.env.user if message.author_id and message.author_id.user_ids: user = message.author_id.user_ids[0] if message.add_sign: signature = user.signature else: - user = self.env.user if message.add_sign: signature = "

--
%s

" % message.author_id.name @@ -2166,12 +2131,13 @@ class MailThread(models.AbstractModel): model_description = self.env['ir.model'].with_context(lang=lang)._get(message.model).display_name tracking = [] - for tracking_value in self.env['mail.tracking.value'].sudo().search([('mail_message_id', '=', message.id)]): - groups = tracking_value.field_groups - if not groups or self.user_has_groups(groups): - tracking.append((tracking_value.field_desc, - tracking_value.get_old_display_value()[0], - tracking_value.get_new_display_value()[0])) + if msg_vals.get('tracking_value_ids') if msg_vals else bool(self): # could be tracking + for tracking_value in self.env['mail.tracking.value'].sudo().search([('mail_message_id', '=', message.id)]): + groups = tracking_value.field_groups + if not groups or self.user_has_groups(groups): + tracking.append((tracking_value.field_desc, + tracking_value.get_old_display_value()[0], + tracking_value.get_new_display_value()[0])) is_discussion = message.subtype_id.id == self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') @@ -2203,15 +2169,16 @@ class MailThread(models.AbstractModel): 'partners': [], 'channels': [], } - res = [] res = self.env['mail.followers']._get_recipient_data(self, subtype_id, pids, cids) - author_id = msg_vals.get('author_id') or message.author_id.id if res else False + if not res: + return recipient_data + + author_id = msg_vals.get('author_id') or message.author_id.id for pid, cid, active, pshare, ctype, notif, groups in res: - if pid and pid == author_id and not message.env.context.get('mail_notify_author'): # do not notify the author of its own messages + if pid and pid == author_id and not self.env.context.get('mail_notify_author'): # do not notify the author of its own messages continue if pid: if active is False: - # avoid to notify inactive partner by email (odoobot) continue pdata = {'id': pid, 'active': active, 'share': pshare, 'groups': groups} if notif == 'inbox': @@ -2224,6 +2191,28 @@ class MailThread(models.AbstractModel): recipient_data['partners'].append(dict(pdata, notif='email', type='customer')) elif cid: recipient_data['channels'].append({'id': cid, 'notif': notif, 'type': ctype}) + + # add partner ids in email channels + email_cids = [r['id'] for r in recipient_data['channels'] if r['notif'] == 'email'] + if email_cids: + # we are doing a similar search in ocn_client + # Could be interresting to make everything in a single query. + # ocn_client: (searching all partners linked to channels of type chat). + # here : (searching all partners linked to channels with notif email if email is not the author one) + email_from = msg_vals.get('email_from') or message.email_from + exept_partner = [r['id'] for r in recipient_data['partners']] + if author_id: + exept_partner.append(author_id) + new_pids = self.env['res.partner'].sudo().search([ + ('id', 'not in', exept_partner), + ('channel_ids', 'in', email_cids), + ('email', 'not in', [email_from]), + ]) + for partner in new_pids: + # caution: side effect, if user has notif type inbox, will receive en email anyway? + # ocn_client: will add partners to recipient recipient_data. more ocn notifications. We neeed to filter them maybe + recipient_data['partners'].append({'id': partner.id, 'share': True, 'active': True, 'notif': 'email', 'type': 'channel_email', 'groups': []}) + return recipient_data @api.model @@ -2264,11 +2253,10 @@ class MailThread(models.AbstractModel): return link @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Return groups used to classify recipients of a notification email. Groups is a list of tuple containing of form (group_name, group_func, group_data) where - * group_name is an identifier used only to be able to override and manipulate groups. Default groups are user (recipients linked to an employee user), portal (recipients linked to a portal user) and customer (recipients not @@ -2280,97 +2268,108 @@ class MailThread(models.AbstractModel): group or not. Only first matching group is kept. Evaluation order is the list order. * group_data is a dict containing parameters for the notification email - * has_button_access: whether to display Access in email. True by default for new groups, False for portal / customer. * button_access: dict with url and title of the button * actions: list of action buttons to display in the notification email. Each action is a dict containing url and title of the button. - Groups has a default value that you can find in mail_thread ``_notify_classify_recipients`` method. """ - return groups + return [ + ( + 'user', + lambda pdata: pdata['type'] == 'user', + {} + ), ( + 'portal', + lambda pdata: pdata['type'] == 'portal', + {'has_button_access': False} + ), ( + 'customer', + lambda pdata: True, + {'has_button_access': False} + ) + ] @api.multi - def _notify_classify_recipients(self, message, recipient_data): + def _notify_classify_recipients(self, recipient_data, model_name): """ Classify recipients to be notified of a message in groups to have specific rendering depending on their group. For example users could have access to buttons customers should not have in their emails. - Module-specific grouping should be done by overriding ``_notify_get_groups`` method defined here-under. - - :param message: mail.message record about to be notified - :param recipients: res.partner recordset to notify UPDATE ME + :param recipient_data:todo xdo UPDATE ME + return example: + [{ + 'actions': [], + 'button_access': {'title': 'View Simple Chatter Model', + 'url': '/mail/view?model=mail.test.simple&res_id=1497'}, + 'has_button_access': False, + 'recipients': [11] + }, + { + 'actions': [], + 'button_access': {'title': 'View Simple Chatter Model', + 'url': '/mail/view?model=mail.test.simple&res_id=1497'}, + 'has_button_access': False, + 'recipients': [4, 5, 6] + }, + { + 'actions': [], + 'button_access': {'title': 'View Simple Chatter Model', + 'url': '/mail/view?model=mail.test.simple&res_id=1497'}, + 'has_button_access': True, + 'recipients': [10, 11, 12] + }] + only return groups with recipients """ - result = {} + + groups = self._notify_get_groups() access_link = self._notify_get_action_link('view') - if message.model: - model = self.with_lang().env['ir.model'] - model_name = model._get(message.model).display_name + if model_name: view_title = _('View %s') % model_name else: view_title = _('View') - default_groups = [ - ('user', lambda pdata: pdata['type'] == 'user', {}), - ('portal', lambda pdata: pdata['type'] == 'portal', { - 'has_button_access': False, - }), - ('customer', lambda pdata: True, { - 'has_button_access': False, - }) - ] - - groups = self._notify_get_groups(message, default_groups) - + # fill group_data with default_values if they are not complete for group_name, group_func, group_data in groups: group_data.setdefault('has_button_access', True) - group_data.setdefault('button_access', { - 'url': access_link, - 'title': view_title}) + group_button_access = group_data.setdefault('button_access', {}) + group_button_access.setdefault('url', access_link) + group_button_access.setdefault('title', view_title) group_data.setdefault('actions', list()) group_data.setdefault('recipients', list()) + # classify recipients in each group for recipient in recipient_data: for group_name, group_func, group_data in groups: if group_func(recipient): group_data['recipients'].append(recipient['id']) break + result = [] for group_name, group_method, group_data in groups: - result[group_name] = group_data + if group_data['recipients']: + result.append(group_data) return result - def _notify_classify_recipients_on_records(self, message, recipient_data, records=None): - """ Generic wrapper on ``_notify_classify_recipients`` checking mail.thread - inheritance and allowing to call model-specific implementation in a one liner. - This method should not be overridden. """ - if records and hasattr(records, '_notify_classify_recipients'): - return records._notify_classify_recipients(message, recipient_data) - return self._notify_classify_recipients(message, recipient_data) - @api.multi def _notify_get_reply_to(self, default=None, records=None, company=None, doc_names=None): """ Returns the preferred reply-to email address when replying to a thread on documents. Documents are either given by self it this method is called as a true multi method on a record set or can be given using records to have a generic implementation available for all models. - Reply-to is formatted like "MyCompany MyDocument ". Heuristic it the following: - * search for specific aliases as they always have priority; it is limited to aliases linked to documents (like project alias for task for example); * use catchall address; * use default; - This method works on documents - * as a true multi method for models inheriting from mail.thread; call ``records._notify_get_reply_to(...)``; * as a generic implementation if records are given; call ``MailThread. @@ -2378,11 +2377,9 @@ class MailThread(models.AbstractModel): * as a generic implementation is self is a void mail.thread record set meaning catchall is computed; call ``MailThread._notify_get_reply_to (records=None)``; - Tweak this method on a specific model by overriding if it inherits from mail.thread. An example would be tasks taking their reply-to alias from their project. - :param default: default email if no alias or catchall is found; :param records: record_set if self if a generic mail.thread record allowing generic implementation of finding reply-to; @@ -2391,7 +2388,6 @@ class MailThread(models.AbstractModel): :param doc_names: dict(res_id, doc_name) used to compute doc name part of the from name; provide it if already known to avoid queries, otherwise name_get on document will be performed; - :return result: dictionary. Keys are record IDs and value is formatted like an email "Company_name Document_name "/ """ @@ -2447,50 +2443,18 @@ class MailThread(models.AbstractModel): return self._notify_get_reply_to(default=default, records=records, company=company, doc_names=doc_names) @api.multi - def _notify_specific_email_values(self, message): - """ Get specific notification email values to store on the notification - mail.mail. Override to add values related to a specific model. - - :param message: mail.message record being notified by email - """ - if not self: - return {} - self.ensure_one() - return {'headers': repr({ - 'X-Odoo-Objects': "%s-%s" % (self._name, self.id), - })} - - @api.model - def _notify_specific_email_values_on_records(self, message, records=None): - """ Generic wrapper on ``_notify_specific_email_values`` checking mail.thread - inheritance and allowing to call model-specific implementation in a one liner. - This method should not be overridden. """ - if records and hasattr(records, '_notify_specific_email_values'): - return records._notify_specific_email_values(message) - return self._notify_specific_email_values(message) - - @api.multi - def _notify_email_recipients(self, recipient_ids): + def _notify_email_recipient_values(self, recipient_ids): """ Format email notification recipient values to store on the notification mail.mail. Basic method just set the recipient partners as mail_mail recipients. Override to generate other mail values like email_to or email_cc. - :param recipient_ids: res.partner recordset to notify """ return { - 'recipient_ids': [(4, pid) for pid in recipient_ids] + 'email_to': False, + 'recipient_ids': recipient_ids, } - @api.model - def _notify_email_recipients_on_records(self, recipient_ids, records=None): - """ Generic wrapper on ``_notify_email_recipients`` checking mail.thread - inheritance and allowing to call model-specific implementation in a one liner. - This method should not be overridden. """ - if records and hasattr(records, '_notify_email_recipients'): - return records._notify_email_recipients(recipient_ids) - return self._notify_email_recipients(recipient_ids) - # ------------------------------------------------------ # Followers API # ------------------------------------------------------ diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py index 4c85b08d2af..e15bad70166 100644 --- a/addons/mail/models/models.py +++ b/addons/mail/models/models.py @@ -17,3 +17,18 @@ class BaseModel(models.AbstractModel): activity_box = E.div(field, {'t-name': "activity-box"}) templates = E.templates(activity_box) return E.activity(templates, string=self._description) + + @api.multi + def _notify_email_headers(self): + """ + Generate the email headers based on record + """ + if not self: + return {} + self.ensure_one() + return repr(self._notify_email_header_dict()) + + def _notify_email_header_dict(self): + return { + 'X-Odoo-Objects': "%s-%s" % (self._name, self.id), + } diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 7a43a36ce38..4cd62ac35c3 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -132,3 +132,4 @@ class Partner(models.Model): return self.env.cr.dictfetchall() else: return {} + diff --git a/addons/mail/wizard/invite.py b/addons/mail/wizard/invite.py index e8d9cbcbda3..5a899e1a7c6 100644 --- a/addons/mail/wizard/invite.py +++ b/addons/mail/wizard/invite.py @@ -72,11 +72,13 @@ class Invite(models.TransientModel): 'no_auto_thread': True, 'add_sign': True, }) - self.env['res.partner'].with_context(auto_delete=True)._notify( - message, - [{'id': pid, 'share': True, 'notif': 'email', 'type': 'customer', 'groups': []} for pid in new_partners.ids], - document, - force_send=True, - send_after_commit=False) + partners_data = [{ + 'id': pid, + 'share': True, + 'notif': 'email', + 'type': 'customer', + 'groups': [] + } for pid in new_partners.ids] + document._notify_record_by_email(message, partners_data, send_after_commit=False) message.unlink() return {'type': 'ir.actions.act_window_close'} diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 8053dfb4484..77a6dbd632d 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -277,8 +277,8 @@ class MailComposer(models.TransientModel): notif_layout=notif_layout, add_sign=not bool(wizard.template_id), mail_auto_delete=wizard.template_id.auto_delete if wizard.template_id else False, - model_description=model_description, - **mail_values) + model_description=model_description) + post_params.update(mail_values) if ActiveModel._name == 'mail.thread': if wizard.model: post_params['model'] = wizard.model @@ -336,7 +336,8 @@ class MailComposer(models.TransientModel): # mass mailing: rendering override wizard static values if mass_mail_mode and self.model: - mail_values.update(self.env['mail.thread']._notify_specific_email_values_on_records(False, records=self.env[self.model].browse(res_id))) + record = self.env[self.model].browse(res_id) + mail_values['headers'] = record._notify_email_headers() # keep a copy unless specifically requested, reset record name (avoid browsing records) mail_values.update(notification=not self.auto_delete_message, model=self.model, res_id=res_id, record_name=False) # auto deletion of mail_mail diff --git a/addons/mail/wizard/mail_resend_message.py b/addons/mail/wizard/mail_resend_message.py index 12255735871..bcff9dc9840 100644 --- a/addons/mail/wizard/mail_resend_message.py +++ b/addons/mail/wizard/mail_resend_message.py @@ -63,26 +63,20 @@ class MailResendMessage(models.TransientModel): notif_to_cancel.sudo().write({'email_status': 'canceled'}) if to_send: message = wizard.mail_message_id - record = self.env[message.model].browse(message.res_id) if message.model and message.res_id else None + record = self.env[message.model].browse(message.res_id) if message.is_thread_message() else self.env['mail.thread'] - rdata = [] + email_partners_data = [] for pid, cid, active, pshare, ctype, notif, groups in self.env['mail.followers']._get_recipient_data(None, False, pids=to_send.ids): if pid and notif == 'email' or not notif: pdata = {'id': pid, 'share': pshare, 'active': active, 'notif': 'email', 'groups': groups or []} if not pshare and notif: # has an user and is not shared, is therefore user - rdata.append(dict(pdata, type='user')) + email_partners_data.append(dict(pdata, type='user')) elif pshare and notif: # has an user and is shared, is therefore portal - rdata.append(dict(pdata, type='portal')) + email_partners_data.append(dict(pdata, type='portal')) else: # has no user, is therefore customer - rdata.append(dict(pdata, type='customer')) + email_partners_data.append(dict(pdata, type='customer')) - self.env['res.partner']._notify( - message, - rdata, - record, - force_send=True, - send_after_commit=False - ) + record._notify_record_by_email(message, email_partners_data, send_after_commit=False) self.mail_message_id._notify_failure_update() return {'type': 'ir.actions.act_window_close'} diff --git a/addons/portal/models/portal_mixin.py b/addons/portal/models/portal_mixin.py index 343c2b67809..9d085a14efc 100644 --- a/addons/portal/models/portal_mixin.py +++ b/addons/portal/models/portal_mixin.py @@ -72,10 +72,10 @@ class PortalMixin(models.AbstractModel): return '%s?%s' % ('/mail/view' if redirect else self.access_url, url_encode(params)) @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): access_token = self._portal_ensure_token() customer = self['partner_id'] - + groups = super(PortalMixin, self)._notify_get_groups() if access_token and customer: additional_params = { 'access_token': self.access_token, @@ -88,13 +88,12 @@ class PortalMixin(models.AbstractModel): 'has_button_access': False, 'button_access': { 'url': access_link, - 'title': _('View %s') % self.env['ir.model']._get(message.model).display_name, }, }) ] else: new_group = [] - return super(PortalMixin, self)._notify_get_groups(message, new_group + groups) + return new_group + groups @api.multi def get_access_action(self, access_uid=None): diff --git a/addons/project/models/project.py b/addons/project/models/project.py index d20c5de5c1d..3fe07c176fd 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -812,12 +812,12 @@ class Task(models.Model): return super(Task, self)._track_subtype(init_values) @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Handle project users and managers recipients that can assign tasks and create new one directly from notification emails. Also give access button to portal users and portal customers. If they are notified they should probably have access to the document. """ - groups = super(Task, self)._notify_get_groups(message, groups) + groups = super(Task, self)._notify_get_groups() self.ensure_one() @@ -836,9 +836,8 @@ class Task(models.Model): groups = [new_group] + groups for group_name, group_method, group_data in groups: - if group_name == 'customer': - continue - group_data['has_button_access'] = True + if group_name != 'customer': + group_data['has_button_access'] = True return groups @@ -907,20 +906,15 @@ class Task(models.Model): return recipients @api.multi - def _notify_specific_email_values(self, message): - res = super(Task, self)._notify_specific_email_values(message) - try: - headers = safe_eval(res.get('headers', dict())) - except Exception: - headers = {} + def _notify_email_header_dict(self): + headers = super(Task, self)._notify_email_header_dict() if self.project_id: current_objects = [h for h in headers.get('X-Odoo-Objects', '').split(',') if h] current_objects.insert(0, 'project.project-%s, ' % self.project_id.id) headers['X-Odoo-Objects'] = ','.join(current_objects) if self.tag_ids: headers['X-Odoo-Tags'] = ','.join(self.tag_ids.mapped('name')) - res['headers'] = repr(headers) - return res + return headers def _message_post_after_hook(self, message, msg_vals): if self.email_from and not self.partner_id: diff --git a/addons/rating/models/rating_mixin.py b/addons/rating/models/rating_mixin.py index 932772c4dd6..75e131301cc 100644 --- a/addons/rating/models/rating_mixin.py +++ b/addons/rating/models/rating_mixin.py @@ -155,7 +155,7 @@ class RatingMixin(models.AbstractModel): if subtype_id is False: subtype_id = self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note') if force_send: - self = self.with_context(mail_notify_force_send=True) + self = self.with_context(mail_notify_force_send=True) # default value is True, should be set to false if not? for record in self: record.message_post_with_template( template.id, diff --git a/addons/sale/models/sale.py b/addons/sale/models/sale.py index 952fbbdde05..adc130e8c0c 100644 --- a/addons/sale/models/sale.py +++ b/addons/sale/models/sale.py @@ -782,18 +782,17 @@ class SaleOrder(models.Model): return (self.state == 'sent' or (self.state == 'draft' and include_draft)) and not self.is_expired and self.require_payment and transaction.state != 'done' and self.amount_total @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Give access button to users and portal customer as portal is integrated in sale. Customer and portal group have probably no right to see the document so they don't have the access button. """ - groups = super(SaleOrder, self)._notify_get_groups(message, groups) + groups = super(SaleOrder, self)._notify_get_groups() self.ensure_one() if self.state not in ('draft', 'cancel'): for group_name, group_method, group_data in groups: - if group_name in ('customer', 'portal'): - continue - group_data['has_button_access'] = True + if group_name not in ('customer', 'portal'): + group_data['has_button_access'] = True return groups diff --git a/addons/test_mail/tests/test_message_track.py b/addons/test_mail/tests/test_message_track.py index 95bf2a5155f..271aaa785af 100644 --- a/addons/test_mail/tests/test_message_track.py +++ b/addons/test_mail/tests/test_message_track.py @@ -163,8 +163,8 @@ class TestTracking(common.BaseFunctionalTest, common.MockEmails): self.record._fields['email_from'].groups = 'base.group_erp_manager' # patch the group attribute self.record.sudo().write({'email_from': 'X'}) - msg_emp = self.record.sudo(self.user_employee)._notify_prepare_template_context(self.record.message_ids) - msg_admin = self.record.sudo(self.user_admin)._notify_prepare_template_context(self.record.message_ids) + msg_emp = self.record.sudo(self.user_employee)._notify_prepare_template_context(self.record.message_ids, {}) + msg_admin = self.record.sudo(self.user_admin)._notify_prepare_template_context(self.record.message_ids, {}) self.assertFalse(msg_emp.get('tracking_values'), "should not have protected tracking values") self.assertTrue(msg_admin.get('tracking_values'), "should have protected tracking values") diff --git a/addons/website_blog/models/website_blog.py b/addons/website_blog/models/website_blog.py index c6a4fda0b3b..12676691977 100644 --- a/addons/website_blog/models/website_blog.py +++ b/addons/website_blog/models/website_blog.py @@ -248,9 +248,9 @@ class BlogPost(models.Model): } @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Add access button to everyone if the document is published. """ - groups = super(BlogPost, self)._notify_get_groups(message, groups) + groups = super(BlogPost, self)._notify_get_groups() if self.website_published: for group_name, group_method, group_data in groups: diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index db8c71ecf64..b2673a24351 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -771,9 +771,9 @@ class Post(models.Model): } @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Add access button to everyone if the document is active. """ - groups = super(Post, self)._notify_get_groups(message, groups) + groups = super(Post, self)._notify_get_groups() if self.state == 'active': for group_name, group_method, group_data in groups: diff --git a/addons/website_mail_channel/models/mail_channel.py b/addons/website_mail_channel/models/mail_channel.py index 108127b2bb4..c7612aebcea 100644 --- a/addons/website_mail_channel/models/mail_channel.py +++ b/addons/website_mail_channel/models/mail_channel.py @@ -14,18 +14,13 @@ class MailGroup(models.Model): _inherit = 'mail.channel' @api.multi - def _notify_specific_email_values(self, message): - res = super(MailGroup, self)._notify_specific_email_values(message) - try: - headers = safe_eval(res.get('headers', dict())) - except Exception: - headers = {} + def _notify_email_header_dict(self): + headers = super(MailGroup, self)._notify_email_header_dict() base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url') headers['List-Archive'] = '<%s/groups/%s>' % (base_url, slug(self)), headers['List-Subscribe'] = '<%s/groups>' % (base_url), headers['List-Unsubscribe'] = '<%s/groups?unsubscribe>' % (base_url,), - res['headers'] = repr(headers) - return res + return headers @api.multi def _send_confirmation_email(self, partner_ids, unsubscribe=False): diff --git a/addons/website_slides/models/slide_slide.py b/addons/website_slides/models/slide_slide.py index 91c327b9090..ab84f80d71e 100644 --- a/addons/website_slides/models/slide_slide.py +++ b/addons/website_slides/models/slide_slide.py @@ -337,9 +337,9 @@ class Slide(models.Model): return super(Slide, self).get_access_action(access_uid) @api.multi - def _notify_get_groups(self, message, groups): + def _notify_get_groups(self): """ Add access button to everyone if the document is active. """ - groups = super(Slide, self)._notify_get_groups(message, groups) + groups = super(Slide, self)._notify_get_groups() if self.website_published: for group_name, group_method, group_data in groups: From 2fcb3c658df3e504b92ff9b5a805d150cfa7effd Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 12 Apr 2019 13:16:48 +0000 Subject: [PATCH 8/9] [REF] mail: rename notif_layout to email_layout_xmlid on message model Purpose is to make field use more obvious by correctly stating in the name it should contain an xml id to a qweb layout used for email notifications. This renaming is propagated through various calls and addons. Related to task 1943901 Linked to PR #32404 --- addons/gamification/models/challenge.py | 6 +++--- addons/gamification/models/goal.py | 2 +- addons/hr_expense/models/hr_expense.py | 2 +- addons/hr_recruitment/models/hr_recruitment.py | 2 +- addons/mail/models/mail_activity.py | 2 +- addons/mail/models/mail_message.py | 2 +- addons/mail/models/mail_thread.py | 12 +++++------- addons/mail/wizard/mail_compose_message.py | 2 +- addons/portal/wizard/portal_share.py | 4 ++-- addons/project/models/project.py | 2 +- addons/rating/models/rating_mixin.py | 2 +- addons/sale/models/payment.py | 2 +- addons/sale/models/sale.py | 2 +- addons/sale_coupon/models/sale_order.py | 2 +- addons/website_event_track/models/event_track.py | 2 +- addons/website_slides/models/slide_slide.py | 2 +- 16 files changed, 23 insertions(+), 25 deletions(-) diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index c95e1f291d7..5e2515e797c 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -567,7 +567,7 @@ class Challenge(models.Model): body=body_html, partner_ids=challenge.mapped('user_ids.partner_id.id'), subtype='mail.mt_comment', - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', ) if challenge.report_message_group_id: challenge.report_message_group_id.message_post( @@ -591,13 +591,13 @@ class Challenge(models.Model): body=body_html, partner_ids=[user.partner_id.id], subtype='mail.mt_comment', - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', ) if challenge.report_message_group_id: challenge.report_message_group_id.message_post( body=body_html, subtype='mail.mt_comment', - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', ) return challenge.write({'last_report_date': fields.Date.today()}) diff --git a/addons/gamification/models/goal.py b/addons/gamification/models/goal.py index 81e8616b316..c5a280f13f2 100644 --- a/addons/gamification/models/goal.py +++ b/addons/gamification/models/goal.py @@ -226,7 +226,7 @@ class Goal(models.Model): body=body_html, partner_ids=[self.user_id.partner_id.id], subtype='mail.mt_comment', - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', ) return {'to_update': True} diff --git a/addons/hr_expense/models/hr_expense.py b/addons/hr_expense/models/hr_expense.py index 7c028f4d47e..5ece94762a4 100644 --- a/addons/hr_expense/models/hr_expense.py +++ b/addons/hr_expense/models/hr_expense.py @@ -532,7 +532,7 @@ class HrExpense(models.Model): subject='Re: %s' % msg_dict.get('subject', ''), body=body, subtype_id=self.env.ref('mail.mt_note').id, - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', ) else: self.env['mail.mail'].create({ diff --git a/addons/hr_recruitment/models/hr_recruitment.py b/addons/hr_recruitment/models/hr_recruitment.py index 393125ba8dc..38570a550e0 100644 --- a/addons/hr_recruitment/models/hr_recruitment.py +++ b/addons/hr_recruitment/models/hr_recruitment.py @@ -353,7 +353,7 @@ class Applicant(models.Model): res['stage_id'] = (applicant.stage_id.template_id, { 'auto_delete_message': True, 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note'), - 'notif_layout': 'mail.mail_notification_light' + 'email_layout_xmlid': 'mail.mail_notification_light' }) return res diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index 6b9ce7aacf2..3dee568ca10 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -394,7 +394,7 @@ class MailActivity(models.Model): subject=_('%s: %s assigned to you') % (activity.res_name, activity.summary or activity.activity_type_id.name), record_name=activity.res_name, model_description=model_description, - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', ) @api.multi diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 5db3abe850f..95fc750066b 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -124,7 +124,7 @@ class Message(models.Model): moderator_id = fields.Many2one('res.users', string="Moderated By", index=True) need_moderation = fields.Boolean('Need moderation', compute='_compute_need_moderation', search='_search_need_moderation') #keep notification layout informations to be able to generate mail again - layout = fields.Char('Layout', copy=False) # xml id of layout + email_layout_xmlid = fields.Char('Layout', copy=False, oldname='layout') # xml id of layout add_sign = fields.Boolean(default=True) @api.multi diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index b17937f1734..7f29e0d13c4 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1795,7 +1795,7 @@ class MailThread(models.AbstractModel): record.message_post_with_template(False, **kwargs) @api.multi - def message_post_with_template(self, template_id, **kwargs): + def message_post_with_template(self, template_id, email_layout_xmlid=None, **kwargs): """ Helper method to send a mail with a template :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) @@ -1807,7 +1807,6 @@ class MailThread(models.AbstractModel): 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 - notif_layout = kwargs.pop('notif_layout', None) # Create the composer composer = self.env['mail.compose.message'].with_context( @@ -1818,7 +1817,7 @@ class MailThread(models.AbstractModel): default_model=kwargs.get('model', self._name), default_res_id=res_id, default_template_id=template_id, - custom_layout=notif_layout, + custom_layout=email_layout_xmlid, ).create(kwargs) # Simulate the onchange (like trigger in form the view) only # when having a template in single-email mode @@ -1922,8 +1921,6 @@ class MailThread(models.AbstractModel): def _message_create(self, values): create_values = dict(values) - if 'notif_layout' in create_values: - create_values['layout'] = create_values.pop('notif_layout') # Avoid warnings about non-existing fields for x in ('from', 'to', 'cc', 'canned_response_ids'): create_values.pop(x, None) @@ -1931,6 +1928,7 @@ class MailThread(models.AbstractModel): create_values['channel_ids'] = [(4, cid) for cid in create_values.get('channel_ids', [])] return self.env['mail.message'].create(create_values) + # ------------------------------------------------------ # Notification API # ------------------------------------------------------ @@ -2010,7 +2008,7 @@ class MailThread(models.AbstractModel): base_template_ctx = self._notify_prepare_template_context(message, msg_vals, model_description=model_description) - template_xmlid = message.layout if message.layout else 'mail.message_notification_email' + template_xmlid = message.email_layout_xmlid if message.email_layout_xmlid else 'mail.message_notification_email' try: base_template = self.env.ref(template_xmlid, raise_if_not_found=True).with_context(lang=base_template_ctx['lang']) except ValueError: @@ -2605,7 +2603,7 @@ class MailThread(models.AbstractModel): body=assignation_msg, partner_ids=partner_ids, record_name=record.display_name, - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', model_description=model_description, ) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 77a6dbd632d..2db6a3d8324 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -274,7 +274,7 @@ class MailComposer(models.TransientModel): post_params = dict( message_type=wizard.message_type, subtype_id=subtype_id, - notif_layout=notif_layout, + email_layout_xmlid=notif_layout, add_sign=not bool(wizard.template_id), mail_auto_delete=wizard.template_id.auto_delete if wizard.template_id else False, model_description=model_description) diff --git a/addons/portal/wizard/portal_share.py b/addons/portal/wizard/portal_share.py index 0da5ce44a7b..a4597edbb62 100644 --- a/addons/portal/wizard/portal_share.py +++ b/addons/portal/wizard/portal_share.py @@ -58,7 +58,7 @@ class PortalShare(models.TransientModel): 'share_link': share_link}, subject=_("You are invited to access %s" % active_record.display_name), subtype_id=note.id, - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', partner_ids=[(6, 0, partner.ids)]) # when partner not user send individual mail with signup token for partner in self.partner_ids - partner_ids: @@ -70,6 +70,6 @@ class PortalShare(models.TransientModel): 'share_link': share_link}, subject=_("You are invited to access %s" % active_record.display_name), subtype_id=note.id, - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', partner_ids=[(6, 0, partner.ids)]) return {'type': 'ir.actions.act_window_close'} diff --git a/addons/project/models/project.py b/addons/project/models/project.py index 3fe07c176fd..2da62547014 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -792,7 +792,7 @@ class Task(models.Model): res['stage_id'] = (test_task.stage_id.mail_template_id, { 'auto_delete_message': True, 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note'), - 'notif_layout': 'mail.mail_notification_light' + 'email_layout_xmlid': 'mail.mail_notification_light' }) return res diff --git a/addons/rating/models/rating_mixin.py b/addons/rating/models/rating_mixin.py index 75e131301cc..f7f35080a18 100644 --- a/addons/rating/models/rating_mixin.py +++ b/addons/rating/models/rating_mixin.py @@ -160,7 +160,7 @@ class RatingMixin(models.AbstractModel): record.message_post_with_template( template.id, composition_mode=composition_mode, - notif_layout=notif_layout if notif_layout is not None else 'mail.mail_notification_light', + email_layout_xmlid=notif_layout if notif_layout is not None else 'mail.mail_notification_light', subtype_id=subtype_id ) diff --git a/addons/sale/models/payment.py b/addons/sale/models/payment.py index ae73e9ba0e7..8580878a45d 100644 --- a/addons/sale/models/payment.py +++ b/addons/sale/models/payment.py @@ -106,7 +106,7 @@ class PaymentTransaction(models.Model): for trans in self.filtered(lambda t: t.sale_order_ids): trans = trans.with_context(ctx_company) for invoice in trans.invoice_ids: - invoice.message_post_with_template(int(default_template), notif_layout="mail.mail_notification_paynow") + invoice.message_post_with_template(int(default_template), email_layout_xmlid="mail.mail_notification_paynow") return res @api.multi diff --git a/addons/sale/models/sale.py b/addons/sale/models/sale.py index adc130e8c0c..63d219df0c8 100644 --- a/addons/sale/models/sale.py +++ b/addons/sale/models/sale.py @@ -667,7 +667,7 @@ class SaleOrder(models.Model): template_id = self._find_mail_template(force_confirmation_template=True) if template_id: for order in self: - order.with_context(force_send=True).message_post_with_template(template_id, composition_mode='comment', notif_layout="mail.mail_notification_paynow") + order.with_context(force_send=True).message_post_with_template(template_id, composition_mode='comment', email_layout_xmlid="mail.mail_notification_paynow") @api.multi def action_done(self): diff --git a/addons/sale_coupon/models/sale_order.py b/addons/sale_coupon/models/sale_order.py index f19668de7b5..3487317330e 100644 --- a/addons/sale_coupon/models/sale_order.py +++ b/addons/sale_coupon/models/sale_order.py @@ -246,7 +246,7 @@ class SaleOrder(models.Model): self.message_post_with_template( template.id, composition_mode='comment', model='sale.coupon', res_id=coupon.id, - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', ) def _get_applicable_programs(self): diff --git a/addons/website_event_track/models/event_track.py b/addons/website_event_track/models/event_track.py index 8d325150719..deb31e167c2 100644 --- a/addons/website_event_track/models/event_track.py +++ b/addons/website_event_track/models/event_track.py @@ -142,7 +142,7 @@ class Track(models.Model): 'composition_mode': 'comment', 'auto_delete_message': True, 'subtype_id': self.env['ir.model.data'].xmlid_to_res_id('mail.mt_note'), - 'notif_layout': 'mail.mail_notification_light' + 'email_layout_xmlid': 'mail.mail_notification_light' }) return res diff --git a/addons/website_slides/models/slide_slide.py b/addons/website_slides/models/slide_slide.py index ab84f80d71e..2d2404c65c9 100644 --- a/addons/website_slides/models/slide_slide.py +++ b/addons/website_slides/models/slide_slide.py @@ -361,7 +361,7 @@ class Slide(models.Model): subject=subject, body=html_body, subtype='website_slides.mt_channel_slide_published', - notif_layout='mail.mail_notification_light', + email_layout_xmlid='mail.mail_notification_light', ) return True From de7c119e666d108564b3889d1c08e5bf2e4da082 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 12 Apr 2019 13:21:18 +0000 Subject: [PATCH 9/9] [IMP] mail: reduce message_post query count and clean code Purpose of this commit is to improve performance of post and notify through various code improvements. Containing * making of _notify_customize_recipients a mail.thread method; * _message_auto_subscribe_followers optimized; * renaming and optimisation of _notify_partners; * message_post_attachements optization; * remove unused mail_post_autofollow_partner_ids context key; * minimize message create queries; * only check record related access right for thread messages not in pending moderation; * get attachements from values instead of db; * storing lang in ctx from the beginning; * don't track on write on attachments; Related to task 1943901 Linked to PR #32404 --- addons/im_livechat/models/mail_channel.py | 4 +- addons/mail/models/mail_channel.py | 22 +- addons/mail/models/mail_message.py | 127 ++++-- addons/mail/models/mail_thread.py | 381 ++++++++++-------- addons/mail/models/res_partner.py | 11 - addons/mail/wizard/mail_compose_message.py | 2 +- addons/test_mail/tests/test_discuss.py | 7 - .../test_mail/tests/test_message_compose.py | 1 + addons/test_mail/tests/test_performance.py | 49 ++- addons/website_blog/models/website_blog.py | 4 +- addons/website_forum/models/forum.py | 9 +- odoo/addons/base/models/ir_attachment.py | 23 +- 12 files changed, 363 insertions(+), 277 deletions(-) diff --git a/addons/im_livechat/models/mail_channel.py b/addons/im_livechat/models/mail_channel.py index cb3f57f689c..bb9a07a8158 100644 --- a/addons/im_livechat/models/mail_channel.py +++ b/addons/im_livechat/models/mail_channel.py @@ -48,7 +48,7 @@ class MailChannel(models.Model): record.is_chat = True @api.multi - def _channel_message_notifications(self, message): + def _channel_message_notifications(self, message, message_format=False): """ When a anonymous user create a mail.channel, the operator is not notify (to avoid massive polling when clicking on livechat button). So when the anonymous person is sending its FIRST message, the channel header should be added to the notification, since the user cannot be listining to the channel. @@ -56,7 +56,7 @@ class MailChannel(models.Model): livechat_channels = self.filtered(lambda x: x.channel_type == 'livechat') other_channels = self.filtered(lambda x: x.channel_type != 'livechat') notifications = super(MailChannel, livechat_channels)._channel_message_notifications(message.with_context(im_livechat_use_username=True)) + \ - super(MailChannel, other_channels)._channel_message_notifications(message) + super(MailChannel, other_channels)._channel_message_notifications(message, message_format) for channel in self: # add uuid for private livechat channels to allow anonymous to listen if channel.channel_type == 'livechat' and channel.public == 'private': diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index e2163810dab..70e2ba63ea4 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -506,20 +506,6 @@ class Channel(models.Model): notifications.append([(self._cr.dbname, 'res.partner', partner.id), channel_info]) return notifications - @api.multi - def _notify(self, message): - """ Broadcast the given message on the current channels. - Send the message on the Bus Channel (uuid for public mail.channel, and partner private bus channel (the tuple)). - A partner will receive only on message on its bus channel, even if this message belongs to multiple mail channel. Then 'channel_ids' field - of the received message indicates on wich mail channel the message should be displayed. - :param : mail.message to broadcast - """ - if not self: - return - message.ensure_one() - notifications = self._channel_message_notifications(message) - self.env['bus.bus'].sendmany(notifications) - def _notify_thread(self, message, msg_vals=False, model_description=False, mail_auto_delete=True): # When posting a message on a mail channel, manage moderation and postpone notify users if not msg_vals or msg_vals.get('moderation_status') != 'pending_moderation': @@ -533,18 +519,18 @@ class Channel(models.Model): message._notify_pending_by_chat() @api.multi - def _channel_message_notifications(self, message): + def _channel_message_notifications(self, message, message_format=False): """ Generate the bus notifications for the given message :param message : the mail.message to sent :returns list of bus notifications (tuple (bus_channe, message_content)) """ - message_values = message.message_format()[0] + message_format = message_format or message.message_format()[0] notifications = [] for channel in self: - notifications.append([(self._cr.dbname, 'mail.channel', channel.id), dict(message_values)]) + notifications.append([(self._cr.dbname, 'mail.channel', channel.id), dict(message_format)]) # add uuid to allow anonymous to listen if channel.public == 'public': - notifications.append([channel.uuid, dict(message_values)]) + notifications.append([channel.uuid, dict(message_format)]) return notifications @api.model diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 95fc750066b..e1fc850d433 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -393,8 +393,8 @@ class Message(models.Model): customer_email_data.append((partner_tree[notification.res_partner_id.id][0], partner_tree[notification.res_partner_id.id][1], notification.email_status)) has_access_to_model = message.model and self.env[message.model].check_access_rights('read', raise_exception=False) - if message.attachment_ids: - main_attachment = has_access_to_model and message.res_id and self.env[message.model].search([('id', '=', message.res_id)]) and getattr(self.env[message.model].browse(message.res_id), 'message_main_attachment_id') + if message.attachment_ids and message.res_id and issubclass(self.pool[message.model], self.pool['mail.thread']) and has_access_to_model: + main_attachment = self.env[message.model].browse(message.res_id).message_main_attachment_id attachment_ids = [] for attachment in message.attachment_ids: if attachment.id in attachments_tree: @@ -860,6 +860,55 @@ class Message(models.Model): author_ids = [mid for mid, message in message_values.items() if not self.is_thread_message(message)] + # Moderator condition: allow to WRITE, UNLINK if moderator of a pending message + moderator_ids = [] + if operation in ['write', 'unlink']: + moderator_ids = [mid for mid, message in message_values.items() if message.get('moderator_id')] + messages_to_check = self.ids + messages_to_check = set(messages_to_check).difference(set(author_ids), set(moderator_ids)) + if not messages_to_check: + return + + # Recipients condition, for read and write (partner_ids) + # keep on top, usefull for systray notifications + notified_ids = [] + model_record_ids = _generate_model_record_ids(message_values, messages_to_check) + if operation in ['read', 'write']: + notified_ids = [mid for mid, message in message_values.items() if message.get('notified')] + + messages_to_check = set(messages_to_check).difference(set(notified_ids)) + if not messages_to_check: + return + + # CRUD: Access rights related to the document + document_related_ids = [] + document_related_candidate_ids = [mid for mid, message in message_values.items() + if (message.get('model') and message.get('res_id') and + message.get('message_type') != 'user_notification' and + (message.get('moderation_status') != 'pending_moderation' or operation not in ['write', 'unlink']))] + model_record_ids = _generate_model_record_ids(message_values, document_related_candidate_ids) + for model, doc_ids in model_record_ids.items(): + DocumentModel = self.env[model] + if hasattr(DocumentModel, 'get_mail_message_access'): + check_operation = DocumentModel.get_mail_message_access(doc_ids, operation) ## why not giving model here? + else: + check_operation = self.env['mail.thread'].get_mail_message_access(doc_ids, operation, model_name=model) + records = DocumentModel.browse(doc_ids) + records.check_access_rights(check_operation) + mids = records.browse(doc_ids)._filter_access_rules(check_operation) + document_related_ids += [ + mid for mid, message in message_values.items() + if (message.get('model') == model and + message.get('res_id') in mids.ids and + message.get('message_type') != 'user_notification' and + (message.get('moderation_status') != 'pending_moderation' or + operation not in ['write', 'unlink']))] + + messages_to_check = messages_to_check.difference(set(document_related_ids)) + + if not messages_to_check: + return + # Parent condition, for create (check for received notifications for the created message parent) notified_ids = [] if operation == 'create': @@ -880,17 +929,12 @@ class Message(models.Model): notified_ids += [mid for mid, message in message_values.items() if message.get('parent_id') in not_parent_ids] - # Moderator condition: allow to WRITE, UNLINK if moderator of a pending message - moderator_ids = [] - if operation in ['write', 'unlink']: - moderator_ids = [mid for mid, message in message_values.items() if message.get('moderator_id')] + messages_to_check = messages_to_check.difference(set(notified_ids)) + if not messages_to_check: + return - # Recipients condition, for read and write (partner_ids) and create (message_follower_ids) - other_ids = set(self.ids).difference(set(author_ids), set(notified_ids), set(moderator_ids)) - model_record_ids = _generate_model_record_ids(message_values, other_ids) - if operation in ['read', 'write']: - notified_ids = [mid for mid, message in message_values.items() if message.get('notified')] - elif operation == 'create': + # Recipients condition for create (message_follower_ids) + if operation == 'create': for doc_model, doc_ids in model_record_ids.items(): followers = self.env['mail.followers'].sudo().search([ ('res_model', '=', doc_model), @@ -904,28 +948,11 @@ class Message(models.Model): message.get('message_type') != 'user_notification' ] - # CRUD: Access rights related to the document - other_ids = other_ids.difference(set(notified_ids)) - model_record_ids = _generate_model_record_ids(message_values, other_ids) - document_related_ids = [] - for model, doc_ids in model_record_ids.items(): - DocumentModel = self.env[model] - mids = DocumentModel.browse(doc_ids).exists() - if hasattr(DocumentModel, 'check_mail_message_access'): - DocumentModel.check_mail_message_access(mids.ids, operation) # ?? mids ? - else: - self.env['mail.thread'].check_mail_message_access(mids.ids, operation, model_name=model) - document_related_ids += [ - mid for mid, message in message_values.items() - if (message.get('model') == model and - message.get('res_id') in mids.ids and - message.get('message_type') != 'user_notification' and - (message.get('moderation_status') != 'pending_moderation' or - operation not in ['write', 'unlink']))] + messages_to_check = messages_to_check.difference(set(notified_ids)) + if not messages_to_check: + return - # Calculate remaining ids: if not void, raise an error - other_ids = other_ids.difference(set(document_related_ids)) - if not (other_ids and self.browse(other_ids).exists()): + if not self.browse(messages_to_check).exists(): return raise AccessError( _('The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: %s, Operation: %s)') % @@ -966,17 +993,19 @@ class Message(models.Model): return message_id @api.multi - def _invalidate_documents(self): + def _invalidate_documents(self, model=None, res_id=None): """ Invalidate the cache of the documents followed by ``self``. """ for record in self: - if record.is_thread_message() and 'message_ids' in self.env[record.model]: - self.env[record.model].invalidate_cache(fnames=[ + model = model or record.model + res_id = res_id or record.res_id + if 'message_ids' in self.env[model]: + self.env[model].invalidate_cache(fnames=[ 'message_ids', 'message_unread', 'message_unread_counter', 'message_needaction', 'message_needaction_counter', - ], ids=[record.res_id]) + ], ids=[res_id]) @api.model def create(self, values): @@ -994,8 +1023,7 @@ class Message(models.Model): values['record_name'] = self._get_record_name(values) if 'attachment_ids' not in values: - values.setdefault('attachment_ids', []) - + values['attachment_ids'] = [] # extract base64 images if 'body' in values: Attachments = self.env['ir.attachment'] @@ -1021,8 +1049,21 @@ class Message(models.Model): tracking_values_cmd = values.pop('tracking_value_ids', False) message = super(Message, self).create(values) + # check attachement access if values.get('attachment_ids'): - message.attachment_ids.check(mode='read') + attachment_ids = [] + if all(isinstance(command, int) or command[0] in (4, 6) for command in values.get('attachment_ids')): + attachment_ids = [] + for command in values.get('attachment_ids'): + if isinstance(command, int): + attachment_ids += [command] + elif command[0] == 6: + attachment_ids += command[2] + else: # command[0] == 4: + attachment_ids += [command[1]] + else: + attachment_ids = message.mapped('attachment_ids').ids # fallback on read if any unknow command + self.env['ir.attachment'].browse(attachment_ids).check(mode='read') if tracking_values_cmd: vals_lst = [dict(cmd[2], mail_message_id=message.id) for cmd in tracking_values_cmd if len(cmd) == 3 and cmd[0] == 0] @@ -1033,7 +1074,7 @@ class Message(models.Model): message.sudo().write({'tracking_value_ids': tracking_values_cmd}) if message.is_thread_message(values): - message._invalidate_documents() + message._invalidate_documents(values.get('model'), values.get('res_id')) return message @@ -1067,7 +1108,9 @@ class Message(models.Model): self.mapped('attachment_ids').filtered( lambda attach: attach.res_model == self._name and (attach.res_id in self.ids or attach.res_id == 0) ).unlink() - self._invalidate_documents() + for elem in self: + if elem.is_thread_message(): + elem._invalidate_documents() return super(Message, self).unlink() # -------------------------------------------------- diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 7f29e0d13c4..17d6941377b 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -488,19 +488,14 @@ class MailThread(models.AbstractModel): return True @api.model - def check_mail_message_access(self, res_ids, operation, model_name=None): + def get_mail_message_access(self, res_ids, operation, model_name=None): """ mail.message check permission rules for related document. This method is meant to be inherited in order to implement addons-specific behavior. A common behavior would be to allow creating messages when having read access rule on the document, for portal document such as issues. """ - if model_name: - DocModel = self.env[model_name] - else: - DocModel = self - if hasattr(DocModel, '_mail_post_access'): - create_allow = DocModel._mail_post_access - else: - create_allow = 'write' + + DocModel = self.env[model_name] if model_name else self + create_allow = getattr(DocModel, '_mail_post_access', 'write') if operation in ['write', 'unlink']: check_operation = 'write' @@ -510,9 +505,7 @@ class MailThread(models.AbstractModel): check_operation = 'write' else: check_operation = operation - - DocModel.check_access_rights(check_operation) - DocModel.browse(res_ids).check_access_rule(check_operation) + return check_operation @api.multi def message_change_thread(self, new_thread): @@ -1582,97 +1575,137 @@ class MailThread(models.AbstractModel): # Post / Send message API # ------------------------------------------------------ - def _message_post_process_attachments(self, attachments, attachment_ids, message_data): + def _message_post_process_attachments(self, attachments, attachment_ids, message_values): """ Preprocess attachments for mail_thread.message_post() or mail_mail.create(). - :param list attachments: list of attachment tuples in the form ``(name,content)``, + :param list attachments: list of attachment tuples in the form ``(name,content)``, #todo xdo update that where content is NOT base64 encoded :param list attachment_ids: a list of attachment ids, not in tomany command form :param dict message_data: model: the model of the attachments parent record, res_id: the id of the attachments parent record """ - IrAttachment = self.env['ir.attachment'] + return_values = {} + body = message_values.get('body') + model = message_values['model'] + res_id = message_values['res_id'] + m2m_attachment_ids = [] - cid_mapping = {} - fname_mapping = {} if attachment_ids: - filtered_attachment_ids = self.env['ir.attachment'].sudo().search([ - ('res_model', '=', 'mail.compose.message'), - ('create_uid', '=', self._uid), - ('id', 'in', attachment_ids)]) + # taking advantage of cache looks better in this case, to check + filtered_attachment_ids = self.env['ir.attachment'].sudo().browse(attachment_ids).filtered( + lambda a: a.res_model == 'mail.compose.message' and a.create_uid.id == self._uid) if filtered_attachment_ids: - filtered_attachment_ids.write({'res_model': message_data['model'], 'res_id': message_data['res_id']}) + filtered_attachment_ids.write({'res_model': model, 'res_id': res_id}) m2m_attachment_ids += [(4, id) for id in attachment_ids] # Handle attachments parameter, that is a dictionary of attachments - for attachment in attachments: - cid = False - if len(attachment) == 2: - name, content = attachment - elif len(attachment) == 3: - name, content, info = attachment - cid = info and info.get('cid') - else: - continue - if isinstance(content, str): - content = content.encode('utf-8') - elif content is None: - continue - data_attach = { - 'name': name, - 'datas': base64.b64encode(content), - 'type': 'binary', - 'datas_fname': name, - 'description': name, - 'res_model': message_data['model'], - 'res_id': message_data['res_id'], - } - new_attachment = IrAttachment.create(data_attach) - m2m_attachment_ids.append((4, new_attachment.id)) - if cid: - cid_mapping[cid] = new_attachment - fname_mapping[name] = new_attachment - if cid_mapping and message_data.get('body'): - root = lxml.html.fromstring(tools.ustr(message_data['body'])) - postprocessed = False - for node in root.iter('img'): - if node.get('src', '').startswith('cid:'): - cid = node.get('src').split('cid:')[1] - attachment = cid_mapping.get(cid) - if not attachment: - attachment = fname_mapping.get(node.get('data-filename'), '') - if attachment: - attachment.generate_access_token() - node.set('src', '/web/image/%s?access_token=%s' % (attachment.id, attachment.access_token)) + if attachments: # generate + cids_in_body = set() + names_in_body = set() + cid_list = [] + name_list = [] + + if body: + root = lxml.html.fromstring(tools.ustr(body)) + # first list all attachments that will be needed in body + for node in root.iter('img'): + if node.get('src', '').startswith('cid:'): + cids_in_body.add(node.get('src').split('cid:')[1]) + elif node.get('data-filename'): + names_in_body.add(node.get('data-filename')) + attachement_values_list = [] + + # generate values + for attachment in attachments: + cid = False + if len(attachment) == 2: + name, content = attachment + elif len(attachment) == 3: + name, content, info = attachment + cid = info and info.get('cid') + else: + continue + if isinstance(content, str): + content = content.encode('utf-8') + elif content is None: + continue + attachement_values= { + 'name': name, + 'datas': base64.b64encode(content), + 'type': 'binary', + 'datas_fname': name, + 'description': name, + 'res_model': model, + 'res_id': res_id, + } + if body and (cid and cid in cids_in_body or name in names_in_body): + attachement_values['access_token'] = self.env['ir.attachment']._generate_access_token() + attachement_values_list.append(attachement_values) + # keep cid and name list synced with attachement_values_list length to match ids latter + cid_list.append(cid) + name_list.append(name) + new_attachments = self.env['ir.attachment'].create(attachement_values_list) + cid_mapping = {} + name_mapping = {} + for counter, new_attachment in enumerate(new_attachments): + cid = cid_list[counter] + if 'access_token' in attachement_values_list[counter]: + if cid: + cid_mapping[cid] = (new_attachment.id, attachement_values_list[counter]['access_token']) + name = name_list[counter] + name_mapping[name] = (new_attachment.id, attachement_values_list[counter]['access_token']) + m2m_attachment_ids.append((4, new_attachment.id)) + + # note: right know we are only taking attachments and ignoring attachment_ids. + if (cid_mapping or name_mapping) and body: + postprocessed = False + for node in root.iter('img'): + attachment_data = False + if node.get('src', '').startswith('cid:'): + cid = node.get('src').split('cid:')[1] + attachment_data = cid_mapping.get(cid) + if not attachment_data and node.get('data-filename'): + attachment_data = name_mapping.get(node.get('data-filename'), False) + if attachment_data: + node.set('src', '/web/image/%s?access_token=%s' % attachment_data) postprocessed = True - if postprocessed: - body = lxml.html.tostring(root, pretty_print=False, encoding='UTF-8') - message_data['body'] = body - - return m2m_attachment_ids + if postprocessed: + return_values['body'] = lxml.html.tostring(root, pretty_print=False, encoding='UTF-8') + return_values['attachment_ids'] = m2m_attachment_ids + return return_values @api.multi @api.returns('mail.message', lambda value: value.id) - def message_post(self, body='', subject=None, subtype_id=False, - message_type='notification', subtype=None, author_id=None, partner_ids=None, channel_ids=None, - parent_id=False, attachments=None, attachment_ids=None, model=False, - add_sign=True, model_description=False, - mail_auto_delete=True, **kwargs): + def message_post(self, + body='', subject=None, message_type='notification', + email_from=False, author_id=None, parent_id=False, + subtype_id=False, subtype=None, partner_ids=None, channel_ids=None, + attachments=None, attachment_ids=None, + add_sign=True, model_description=False, mail_auto_delete=True, record_name=False, + **kwargs): """ Post a new message in an existing thread, returning the new mail.message ID. - :param int thread_id: thread ID to post into, or list with one ID; - if False/0, mail.message model will also be set as False :param str body: body of the message, usually raw HTML that will be sanitized - :param str type: see mail_message.message_type field + :param str subject: subject of the message + :param str message_type: see mail_message.message_type field. Can be anything but + user_notification, reserved for message_notify :param int parent_id: handle reply to a previous message by adding the parent partners to the message in case of private discussion - :param tuple(str,str) attachments or list id: list of attachment tuples in the form - ``(name,content)``, where content is NOT base64 encoded + :param int subtype_id: subtype_id of the message, mainly use fore + followers mechanism + :param int subtype: xmlid that will be used to compute subtype_id + if subtype_id is not given. + :param list(int) partner_ids: partner_ids to notify + :param list(int) channel_ids: channel_ids to notify + :param list(tuple(str,str), tuple(str,str, dict) or int) attachments : list of attachment tuples in the form + ``(name,content)`` or ``(name,content, info)``, where content is NOT base64 encoded + :param list id attachment_ids: list of existing attachement to link to this message + -Should only be setted by chatter + -Attachement object attached to mail.compose.message(0) will be attached + to the related document. Extra keyword arguments will be used as default column values for the - new mail.message record. Special cases: - - attachment_ids: supposed not attached to any document; attach them - to the related document. Should only be set by Chatter. + new mail.message record. :return int: ID of newly created mail.message """ self.ensure_one() # should always be posted on a record, use message_notify if no record @@ -1683,16 +1716,21 @@ class MailThread(models.AbstractModel): if 'model' in kwargs or 'res_id' in kwargs: raise ValueError("message_post doesn't support model and res_id parameters anymore. Please call message_post on record") - # Find the message's author, because we need it for private discussion - if author_id is None: # keep False values - author_id = self.env['mail.message']._get_default_author().id # self.env.user.partner_id + self = self.with_lang() # add lang to context imediatly since it will be usefull in various flows latter. + + record_name = record_name or self.display_name partner_ids = set(partner_ids or []) channel_ids = set(channel_ids or []) - for pc_id in partner_ids | channel_ids: - if not isinstance(pc_id, int): - raise ValueError('message_post partner_ids and channel_ids must be integer list, not commands') + if any(not isinstance(pc_id, int) for pc_id in partner_ids | channel_ids): + raise ValueError('message_post partner_ids and channel_ids must be integer list, not commands') + + # Find the message's author, because we need it for private discussion + if author_id is None: # keep False values + author_id = self.env.user.partner_id.id + if not email_from: + email_from = self.env['res.partner'].browse(author_id).sudo().email_formatted if author_id else self.env['mail.message']._get_default_from() if not subtype_id: subtype = subtype or 'mt_note' @@ -1702,33 +1740,30 @@ class MailThread(models.AbstractModel): # automatically subscribe recipients if asked to if self._context.get('mail_post_autofollow') and partner_ids: - partner_to_subscribe = partner_ids - if self._context.get('mail_post_autofollow_partner_ids'): - partner_to_subscribe = [p for p in partner_ids if p in self._context.get('mail_post_autofollow_partner_ids')] - self.message_subscribe(list(partner_to_subscribe)) + self.message_subscribe(list(partner_ids)) - # _mail_flat_thread: automatically set free messages to the first posted message - MailMessage = self.env['mail.message'] + MailMessage_sudo = self.env['mail.message'].sudo() if self._mail_flat_thread and not parent_id: - messages = MailMessage.search(['&', ('res_id', '=', self.ids[0]), ('model', '=', self._name), ('message_type', '!=', 'user_notification')], order="id ASC", limit=1) - parent_id = messages.ids and messages.ids[0] or False - # we want to set a parent: force to set the parent_id to the oldest ancestor, to avoid having more than 1 level of thread - elif parent_id: - messages = MailMessage.sudo().search([('id', '=', parent_id), ('parent_id', '!=', False)], limit=1) + parent_message = MailMessage_sudo.search([('res_id', '=', self.id), ('model', '=', self._name), ('message_type', '!=', 'user_notification')], order="id ASC", limit=1) + # parent_message searched in sudo for performance, only used for id. + # Note that with sudo we will match message with internal subtypes. + parent_id = parent_message.id if parent_message else False + elif parent_id: + old_parent_id = parent_id + parent_message = MailMessage_sudo.search([('id', '=', parent_id), ('parent_id', '!=', False)], limit=1) # avoid loops when finding ancestors processed_list = [] - if messages: - message = messages[0] - while (message.parent_id and message.parent_id.id not in processed_list): - processed_list.append(message.parent_id.id) - message = message.parent_id - parent_id = message.id - - values = kwargs + if parent_message: + new_parent_id = parent_message.parent_id and parent_message.parent_id.id + while (new_parent_id and new_parent_id not in processed_list): + processed_list.append(new_parent_id) + parent_message = parent_message.parent_id + parent_id = parent_message.id + values = dict(kwargs) values.update({ 'author_id': author_id, 'model': self._name, - 'res_id': self._name and self.id or False, + 'res_id': self.id, 'body': body, 'subject': subject or False, 'message_type': message_type, @@ -1737,13 +1772,16 @@ class MailThread(models.AbstractModel): 'partner_ids': partner_ids, 'channel_ids': channel_ids, 'add_sign': add_sign, + 'record_name': record_name, + 'email_from': email_from, }) + attachments = attachments or [] + attachment_ids = attachment_ids or [] + attachement_values = self._message_post_process_attachments(attachments, attachment_ids, values) + values.update(attachement_values) # attachement_ids, [body] - # 3. Attachments - # - HACK TDE FIXME: Chatter: attachments linked to the document (not done JS-side), load the message + new_message= self._message_create(values) - values['attachment_ids'] = self._message_post_process_attachments(attachments or [], attachment_ids or [], values) - new_message = self._message_create(values) # Set main attachment field if necessary self._message_set_main_attachment_id(values['attachment_ids']) @@ -1761,7 +1799,7 @@ class MailThread(models.AbstractModel): prioritary_attachments = all_attachments.filtered(lambda x: x.mimetype.endswith('pdf')) \ or all_attachments.filtered(lambda x: x.mimetype.startswith('image')) \ or all_attachments - self.sudo().write({'message_main_attachment_id': prioritary_attachments[0].id}) + self.sudo().with_context(tracking_disable=True).write({'message_main_attachment_id': prioritary_attachments[0].id}) def _message_post_after_hook(self, message, msg_vals): """ Hook to add custom behavior after having posted the message. Both @@ -1833,7 +1871,6 @@ class MailThread(models.AbstractModel): displayed on a document. It pushes notifications on inbox or by email depending on the user configuration, like other notifications. """ - # should we handle channel_ids here? if self: self.ensure_one() @@ -1844,7 +1881,7 @@ class MailThread(models.AbstractModel): if not author.email: raise exceptions.UserError(_("Unable to notify message, please configure the sender's email address.")) - email_from = formataddr((author.name, author.email)) + email_from = author.email_formatted partner_ids = partner_ids or set() if parent_id: # looks like no test case are going throug this condition. Linked to private discussion. This may be removed soon. @@ -1928,7 +1965,6 @@ class MailThread(models.AbstractModel): create_values['channel_ids'] = [(4, cid) for cid in create_values.get('channel_ids', [])] return self.env['mail.message'].create(create_values) - # ------------------------------------------------------ # Notification API # ------------------------------------------------------ @@ -1970,20 +2006,28 @@ class MailThread(models.AbstractModel): # (could also be interresting for, we could add partners with r['notif'] = 'ocn_client' and r['needaction']=False) # then overide a notify_recipients (as it was before) to effectively send ocn notifications. # enveloppe will contain more needaction, those for the member of a email channel. - if message_values and self and hasattr(self, '_notify_customize_recipients'): - message_values.update(self._notify_customize_recipients(message, msg_vals, rdata)) + if message_values and self: + message_values.update(self._notify_customize_recipients(message, msg_vals)) if message_values: message.write(message_values) inbox_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'inbox'] partner_email_rdata = [r for r in rdata['partners'] if r['notif'] == 'email'] + channel_ids = [r['id'] for r in rdata['channels']] + + notifications = [] + if inbox_pids or channel_ids: + message_values = False + if inbox_pids: + message_values = message.message_format()[0] + for partner in self.env['res.partner'].browse(inbox_pids): + notifications.append([(self._cr.dbname, 'ir.needaction', partner), dict(message_values)]) + if channel_ids: + notifications += self.env['mail.channel'].sudo().browse(channel_ids)._channel_message_notifications(message, message_values) if partner_email_rdata: self._notify_record_by_email(message, partner_email_rdata, msg_vals=msg_vals, model_description=model_description, mail_auto_delete=mail_auto_delete) - if inbox_pids: - self.env['res.partner'].browse(inbox_pids)._notify_by_chat(message) - if rdata['channels']: - # send a notification on bus to update channel - self.env['mail.channel'].sudo().browse([r['id'] for r in rdata['channels']])._notify(message) + if notifications: + self.env['bus.bus'].sudo().sendmany(notifications) return True def _notify_record_by_email(self, message, partners_data, msg_vals=False, model_description=False, mail_auto_delete=True, send_after_commit=False): @@ -1998,7 +2042,7 @@ class MailThread(models.AbstractModel): :param mail_auto_delete: delete notification emails once sent; """ model = msg_vals.get('model') if msg_vals else message.model - model_name = model_description or (self.with_lang().env['ir.model']._get(model).display_name if model else False) + model_name = model_description or (self.with_lang().env['ir.model']._get(model).display_name if model else False) # one query for display name recipients_groups_data = self._notify_classify_recipients(partners_data, model_name) if not recipients_groups_data: @@ -2006,21 +2050,25 @@ class MailThread(models.AbstractModel): force_send = self.env.context.get('mail_notify_force_send', True) - base_template_ctx = self._notify_prepare_template_context(message, msg_vals, model_description=model_description) + template_values = self._notify_prepare_template_context(message, msg_vals, model_description=model_description) # 10 queries - template_xmlid = message.email_layout_xmlid if message.email_layout_xmlid else 'mail.message_notification_email' + email_layout_xmlid = msg_vals.get('email_layout_xmlid') if msg_vals else message.email_layout_xmlid + template_xmlid = email_layout_xmlid if email_layout_xmlid else 'mail.message_notification_email' try: - base_template = self.env.ref(template_xmlid, raise_if_not_found=True).with_context(lang=base_template_ctx['lang']) + base_template = self.env.ref(template_xmlid, raise_if_not_found=True).with_context(lang=template_values['lang']) # 1 query except ValueError: _logger.warning('QWeb template %s not found when sending notification emails. Sending without layouting.' % (template_xmlid)) base_template = False + + mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) # in cache, no queries # prepare notification mail values base_mail_values = { 'mail_message_id': message.id, - 'mail_server_id': message.mail_server_id.id, + 'mail_server_id': message.mail_server_id.id, # 2 query, check acces + read, may be useless, Falsy, when will it be used? 'auto_delete': mail_auto_delete, - 'references': message.parent_id.message_id if message.parent_id else False + 'references': message.parent_id.message_id if message.parent_id else False, + 'subject': mail_subject, } headers = self._notify_email_headers() if headers: @@ -2028,24 +2076,24 @@ class MailThread(models.AbstractModel): Mail = self.env['mail.mail'].sudo() emails = self.env['mail.mail'].sudo() - email_pids = set() # loop on groups (customer, portal, user, ... + model specific like group_sale_salesman) recipients_max = 50 for recipients_group_data in recipients_groups_data: # generate notification email content - template_ctx = {**base_template_ctx, **recipients_group_data} + recipients_ids = recipients_group_data.pop('recipients') + render_values = {**template_values, **recipients_group_data} # {company, is_discussion, lang, message, model_description, record, record_name, signature, subtype, tracking_values, website_url} # {actions, button_access, has_button_access, recipients} + if base_template: - mail_body = base_template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) + mail_body = base_template.render(render_values, engine='ir.qweb', minimal_qcontext=True) else: mail_body = message.body mail_body = self._replace_local_links(mail_body) - mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) # send email - for email_chunk in split_every(recipients_max, recipients_group_data['recipients']): - recipient_values = self._notify_email_recipient_values(email_chunk) + for recipients_ids_chunk in split_every(recipients_max, recipients_ids): + recipient_values = self._notify_email_recipient_values(recipients_ids_chunk) email_to = recipient_values['email_to'] recipient_ids = recipient_values['recipient_ids'] @@ -2062,7 +2110,11 @@ class MailThread(models.AbstractModel): if email and recipient_ids: notifications = self.env['mail.notification'].sudo().search([ ('mail_message_id', '=', email.mail_message_id.id), - ('res_partner_id', 'in', list(recipient_ids)) + ('res_partner_id', 'in', list(recipient_ids)) # not sure to check. + # TODO XDO what if recipient_ids are empty because of _notify_email_recipient_values + # should we use recipients_ids_chunk? + # should we unlink recipients_ids_chunk - recipient_ids ? + # should we avoid to create needation? by calling _notify_email_recipient_values at the same place _notify_customize_recipients does? (but no chubnk at this step) ]) notifications.write({ 'is_email': True, @@ -2071,7 +2123,6 @@ class MailThread(models.AbstractModel): 'email_status': 'ready', }) emails |= email - email_pids.update(recipient_ids) # NOTE: # 1. for more than 50 followers, use the queue system @@ -2080,13 +2131,12 @@ class MailThread(models.AbstractModel): # using the command-line. test_mode = getattr(threading.currentThread(), 'testing', False) if force_send and len(emails) < recipients_max and (not self.pool._init or test_mode): - email_ids = emails.ids - dbname = self.env.cr.dbname - _context = self._context - # unless asked specifically, send emails after the transaction to # avoid side effects due to emails being sent while the transaction fails if not test_mode and send_after_commit: + email_ids = emails.ids + dbname = self.env.cr.dbname + _context = self._context def send_notifications(): db_registry = registry(dbname) with api.Environment.manage(), db_registry.cursor() as cr: @@ -2103,13 +2153,22 @@ class MailThread(models.AbstractModel): # compute send user and its related signature signature = '' user = self.env.user - if message.author_id and message.author_id.user_ids: - user = message.author_id.user_ids[0] - if message.add_sign: + author = message.env['res.partner'].browse(msg_vals.get('author_id')) if msg_vals else message.author_id + model = msg_vals.get('model') if msg_vals else message.model + add_sign = msg_vals.get('add_sign') if msg_vals else message.add_sign + subtype_id = msg_vals.get('subtype_id') if msg_vals else message.subtype_id.id + message_id = message.id + record_name = msg_vals.get('record_name') if msg_vals else message.record_name + author_user = user if user.partner_id == author else author.user_ids[0] if author and author.user_ids else False + # trying to use user (self.env.user) instead of browing user_ids if he is the author will give a sudo user, + # improving access performances and cache usage. + if author_user: + user = author_user + if add_sign: signature = user.signature else: - if message.add_sign: - signature = "

--
%s

" % message.author_id.name + if add_sign: + signature = "

--
%s

" % author.name company = self.company_id.sudo() if self and 'company_id' in self else user.company_id if company.website: @@ -2125,11 +2184,11 @@ class MailThread(models.AbstractModel): if template and template.lang: lang = template._render_template(template.lang, self.env.context['default_model'], self.env.context['default_res_id']) - if not model_description and message.model: - model_description = self.env['ir.model'].with_context(lang=lang)._get(message.model).display_name + if not model_description and model: + model_description = self.env['ir.model'].with_context(lang=lang)._get(model).display_name tracking = [] - if msg_vals.get('tracking_value_ids') if msg_vals else bool(self): # could be tracking + if msg_vals.get('tracking_value_ids', True) if msg_vals else bool(self): # could be tracking for tracking_value in self.env['mail.tracking.value'].sudo().search([('mail_message_id', '=', message.id)]): groups = tracking_value.field_groups if not groups or self.user_has_groups(groups): @@ -2137,7 +2196,7 @@ class MailThread(models.AbstractModel): tracking_value.get_old_display_value()[0], tracking_value.get_new_display_value()[0])) - is_discussion = message.subtype_id.id == self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') + is_discussion = subtype_id == self.env['ir.model.data'].xmlid_to_res_id('mail.mt_comment') return { 'message': message, @@ -2146,7 +2205,7 @@ class MailThread(models.AbstractModel): 'company': company, 'model_description': model_description, 'record': self, - 'record_name': message.record_name, + 'record_name': record_name, 'tracking_values': tracking, 'is_discussion': is_discussion, 'subtype': message.subtype_id, @@ -2452,6 +2511,9 @@ class MailThread(models.AbstractModel): 'email_to': False, 'recipient_ids': recipient_ids, } + @api.multi + def _notify_customize_recipients(self, message, msg_vals): + return {} # ------------------------------------------------------ # Followers API @@ -2558,19 +2620,16 @@ class MailThread(models.AbstractModel): :param default_subtype_ids: coming from ``_get_auto_subscription_subtypes`` """ fnames = [] - for name, field in self._fields.items(): - if name == 'user_id' and updated_values.get(name) and (getattr(field, 'track_visibility', False) or getattr(field, 'tracking', False)): - if field.comodel_name == 'res.users': - fnames.append(name) - - new_subscriptions = [] - user_ids = [updated_values[fname] for fname in fnames if updated_values[fname]] - if user_ids: - new_pids = self.env['res.partner'].sudo().search([('user_ids', 'in', user_ids), ('active', '=', True)]).ids - for new_pid in new_pids: - new_subscriptions.append((new_pid, default_subtype_ids, 'mail.message_user_assigned' if new_pid != self.env.user.partner_id.id else False)) - - return new_subscriptions + field = self._fields.get('user_id') + user_id = updated_values.get('user_id') + if field and user_id and field.comodel_name == 'res.users' and (getattr(field, 'track_visibility', False) or getattr(field, 'tracking', False)): + user = self.env['res.users'].sudo().browse(user_id) + try: # avoid to make an exists, lets be optimistic and try to read it. + if user.active: + return [(user.partner_id.id, default_subtype_ids, 'mail.message_user_assigned' if user != self.env.user else False)] + except: + pass + return [] @api.multi def _message_auto_subscribe_notify(self, partner_ids, template): diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 4cd62ac35c3..e2abaf7d75b 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -37,17 +37,6 @@ class Partner(models.Model): 'email_cc': False} for r in self} - @api.multi - def _notify_by_chat(self, message): - """ Broadcast the message to all the partner since """ - if not self: - return - message_values = message.message_format()[0] - notifications = [] - for partner in self: - notifications.append([(self._cr.dbname, 'ir.needaction', partner.id), dict(message_values)]) - self.env['bus.bus'].sendmany(notifications) - @api.model def get_needaction_count(self): """ compute the number of needaction of the current user """ diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 2db6a3d8324..1d54406b086 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -367,7 +367,7 @@ class MailComposer(models.TransientModel): mail_values.pop('attachments', []), attachment_ids, {'model': 'mail.message', 'res_id': 0} - ) + )['attachment_ids'] # Filter out the blacklisted records by setting the mail state to cancel -> Used for Mass Mailing stats if res_id in blacklisted_rec_ids: mail_values['state'] = 'cancel' diff --git a/addons/test_mail/tests/test_discuss.py b/addons/test_mail/tests/test_discuss.py index a14b10ffdd6..3cb885b715c 100644 --- a/addons/test_mail/tests/test_discuss.py +++ b/addons/test_mail/tests/test_discuss.py @@ -28,13 +28,6 @@ class TestChatterTweaks(BaseFunctionalTest, TestRecipients): self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id') | self.partner_1 | self.partner_2) self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) - def test_post_subscribe_recipients_partial(self): - original = self.test_record.message_follower_ids - self.test_record.sudo(self.user_employee).with_context({'mail_create_nosubscribe': True, 'mail_post_autofollow': True, 'mail_post_autofollow_partner_ids': [self.partner_2.id]}).message_post( - body='Test Body', message_type='comment', subtype='mt_comment', partner_ids=[self.partner_1.id, self.partner_2.id]) - self.assertEqual(self.test_record.message_follower_ids.mapped('partner_id'), original.mapped('partner_id') | self.partner_2) - self.assertEqual(self.test_record.message_follower_ids.mapped('channel_id'), original.mapped('channel_id')) - def test_chatter_mail_create_nolog(self): """ Test disable of automatic chatter message at create """ rec = self.env['mail.test.simple'].sudo(self.user_employee).with_context({'mail_create_nolog': True}).create({'name': 'Test'}) diff --git a/addons/test_mail/tests/test_message_compose.py b/addons/test_mail/tests/test_message_compose.py index 5b40c56e7cf..32a9a44e3ee 100644 --- a/addons/test_mail/tests/test_message_compose.py +++ b/addons/test_mail/tests/test_message_compose.py @@ -218,6 +218,7 @@ class TestMessagePost(BaseFunctionalTest, MockEmails, TestRecipients): self.assertEqual(new_notification.email_from, formataddr((self.env.user.name, self.env.user.email))) self.assertEqual(new_notification.needaction_partner_ids, self.partner_1 | self.user_employee.partner_id) self.assertNotIn(new_notification, self.test_record.message_ids) + # todo xdo add test message_notify on thread with followers and stuff class TestComposer(BaseFunctionalTest, MockEmails, TestRecipients): diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 01bc729d321..cb2300a6df3 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -83,7 +83,7 @@ class TestMailPerformance(TransactionCase): 'partner_id': self.env.ref('base.res_partner_12').id, }) - with self.assertQueryCount(__system__=5, demo=5): # test_mail only: 5 - 5 + with self.assertQueryCount(__system__=4, demo=4): # test_mail only: 4 - 4 record.track = 'X' @users('__system__', 'demo') @@ -99,13 +99,13 @@ class TestMailPerformance(TransactionCase): @warmup def test_create_mail_with_tracking(self): """ Create records inheriting from 'mail.thread' (with field tracking). """ - with self.assertQueryCount(__system__=10, demo=10): # test_mail only: 10 - 10 + with self.assertQueryCount(__system__=9, demo=9): # test_mail only: 9 - 9 self.env['test_performance.mail'].create({'name': 'X'}) @users('__system__', 'emp') @warmup def test_create_mail_simple(self): - with self.assertQueryCount(__system__=8, emp=8): # test_mail only: 8 - 8 + with self.assertQueryCount(__system__=7, emp=7): # test_mail only: 7 - 7 self.env['mail.test.simple'].create({'name': 'Test'}) @users('__system__', 'emp') @@ -161,7 +161,7 @@ class TestAdvMailPerformance(TransactionCase): def test_adv_activity(self): model = self.env['mail.test.activity'] - with self.assertQueryCount(__system__=9, emp=8): # test_mail only: 9 - 8 + with self.assertQueryCount(__system__=8, emp=7): # test_mail only: 8 - 7 model.create({'name': 'Test'}) @users('__system__', 'emp') @@ -173,7 +173,7 @@ class TestAdvMailPerformance(TransactionCase): 'default_res_model': 'mail.test.activity', }) - with self.assertQueryCount(__system__=10, emp=17): # com runbot: 10 - 16 // test_mail only: 10 - 15 + with self.assertQueryCount(__system__=10, emp=13): # com runbot: 10 - 13 // test_mail only: 10 - 13 activity = MailActivity.create({ 'summary': 'Test Activity', 'res_id': record.id, @@ -183,7 +183,7 @@ class TestAdvMailPerformance(TransactionCase): #voip module read activity_type during create leading to one less query in enterprise on action_feedback category = activity.activity_type_id.category - with self.assertQueryCount(__system__=24, emp=41): # com runbot: 24 - 41 // test_mail only: 24 - 39 + with self.assertQueryCount(__system__=24, emp=31): # com runbot: 24 - 31 // test_mail only: 24 - 31 activity.action_feedback(feedback='Zizisse Done !') @users('__system__', 'emp') @@ -192,7 +192,7 @@ class TestAdvMailPerformance(TransactionCase): def test_adv_activity_mixin(self): record = self.env['mail.test.activity'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=10, emp=17): # com runbot: 10 - 16 // test_mail only: 10 - 15 + with self.assertQueryCount(__system__=10, emp=14): # com runbot: 10 - 14 // test_mail only: 10 - 14 activity = record.action_start('Test Start') #read activity_type to normalize cache between enterprise and community #voip module read activity_type during create leading to one less query in enterprise on action_close @@ -200,7 +200,7 @@ class TestAdvMailPerformance(TransactionCase): record.write({'name': 'Dupe write'}) - with self.assertQueryCount(__system__=26, emp=42): # com runbot: 26 - 42 // test_mail only: 26 - 40 + with self.assertQueryCount(__system__=26, emp=31): # com runbot: 26 - 31 // test_mail only: 26 - 31 record.action_close('Dupe feedback') self.assertEqual(record.activity_ids, self.env['mail.activity']) @@ -211,7 +211,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_assignation_email(self): self.user_test.write({'notification_type': 'email'}) record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=61, emp=76): # com runbot: 61 - 76 // test_mail only: 61 - 71 + with self.assertQueryCount(__system__=56, emp=59): # com runbot: 56 - 59 // test_mail only: 56 - 59 record.write({ 'user_id': self.user_test.id, }) @@ -220,7 +220,7 @@ class TestAdvMailPerformance(TransactionCase): @warmup def test_message_assignation_inbox(self): record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=37, emp=44): # test_mail only: 37 - 44 + with self.assertQueryCount(__system__=34, emp=40): # test_mail only: 34 - 40 record.write({ 'user_id': self.user_test.id, }) @@ -240,7 +240,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_log_with_post(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=7, emp=13): # test_mail only: 7 - 13 + with self.assertQueryCount(__system__=6, emp=7): # test_mail only: 6 - 7 record.message_post( body='

Test message_post as log

', subtype='mail.mt_note', @@ -251,7 +251,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_no_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=7, emp=13): # test_mail only: 7 - 13 + with self.assertQueryCount(__system__=6, emp=7): # test_mail only: 6 - 7 record.message_post( body='

Test Post Performances basic

', partner_ids=[], @@ -264,7 +264,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=52, emp=72): # com runbot: 49 - 69 // test_mail only: 52 - 69 + with self.assertQueryCount(__system__=47, emp=51): # com runbot: 47 - 51 // test_mail only: 47 - 51 record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, @@ -276,7 +276,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_inbox_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=32, emp=44): # com runbot 31 - 43 // test_mail only: 32 - 44 + with self.assertQueryCount(__system__=30, emp=36): # com runbot 30 - 36 // test_mail only: 30 - 36 record.message_post( body='

Test Post Performances with an inbox ping

', partner_ids=self.user_test.partner_id.ids, @@ -386,8 +386,7 @@ class TestHeavyMailPerformance(TransactionCase): 'recipient_ids': [(4, pid) for pid in self.partners.ids], }) mail_ids = mail.ids - - with self.assertQueryCount(__system__=14, emp=21): # test_mail only: 14 - 21 + with self.assertQueryCount(__system__=16, emp=22): # test_mail only: 16 - 22 self.env['mail.mail'].browse(mail_ids).send() self.assertEqual(mail.body_html, '

Test

') @@ -400,7 +399,7 @@ class TestHeavyMailPerformance(TransactionCase): self.umbrella.message_subscribe(self.user_portal.partner_id.ids) record = self.umbrella.sudo(self.env.user) - with self.assertQueryCount(__system__=85, emp=108): # com runbot: 82 - 105 // test_mail only: 85 - 105 + with self.assertQueryCount(__system__=78, emp=82): # com runbot: 78 - 82 // test_mail only: 78 - 82 record.message_post( body='

Test Post Performances

', message_type='comment', @@ -417,7 +416,7 @@ class TestHeavyMailPerformance(TransactionCase): record = self.umbrella.sudo(self.env.user) template_id = self.env.ref('test_mail.mail_test_tpl').id - with self.assertQueryCount(__system__=102, emp=126): # com runbot: 99 - 123 // test_mail only: 102 - 123 + with self.assertQueryCount(__system__=95, emp=101): # com runbot: 95 - 101 // test_mail only: 95 - 101 record.message_post_with_template(template_id, message_type='comment', composition_mode='comment') self.assertEqual(record.message_ids[0].body, '

Adding stuff on %s

' % record.name) @@ -486,7 +485,7 @@ class TestHeavyMailPerformance(TransactionCase): 'user_id': self.env.uid, }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(__system__=61, emp=75): # com runbot: 60 - 75 // test_mail only: 60 - 72 + with self.assertQueryCount(__system__=55, emp=58): # com runbot: 55 - 58 // test_mail only: 55 - 58 rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) # write tracking message @@ -506,7 +505,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(__system__=155, emp=172): # com runbot: 155 - 172 // test_mail only: 154 - 168 + with self.assertQueryCount(__system__=147, emp=155): # com runbot: 147 - 155 // test_mail only: 147 - 155 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -533,7 +532,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) self.assertEqual(len(rec.message_ids), 1) - with self.assertQueryCount(__system__=99, emp=122): # com runbot: 99 - 122 // test_mail only: 99 - 119 + with self.assertQueryCount(__system__=96, emp=101): # com runbot: 96 - 101 // test_mail only: 96 - 101 rec.write({ 'name': 'Test2', 'umbrella_id': self.umbrella.id, @@ -569,7 +568,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(__system__=105, emp=127): # test_mail only: 104 - 123 + with self.assertQueryCount(__system__=101, emp=106): # test_mail only: 101 - 106 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id, @@ -601,7 +600,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) - with self.assertQueryCount(__system__=52, emp=66): # test_mail only: 51 - 66 + with self.assertQueryCount(__system__=50, emp=62): # test_mail only: 50 - 62 rec.write({ 'name': 'Test2', 'customer_id': customer_id, @@ -620,7 +619,7 @@ class TestHeavyMailPerformance(TransactionCase): self.assertEqual(len(rec.message_ids), 3) -@tagged('mail_performance_post') +@tagged('mail_performance') class TestMailPerformancePost(TransactionCase): def setUp(self): @@ -753,7 +752,7 @@ class TestMailPerformancePost(TransactionCase): ] self.attachements = self.env['ir.attachment'].sudo(self.env.user).create(self.vals) #-> 163-> 165 query attachement_ids = self.attachements.ids - with self.assertQueryCount(emp=248): # test_mail only: 210 + with self.assertQueryCount(emp=163): # com runbot 156 // test_mail only: 133 self.cr.sql_log = self.warm and self.cr.sql_log_count record.with_context({}).message_post( body='

Test body

', diff --git a/addons/website_blog/models/website_blog.py b/addons/website_blog/models/website_blog.py index 12676691977..6ddb0cb06a6 100644 --- a/addons/website_blog/models/website_blog.py +++ b/addons/website_blog/models/website_blog.py @@ -259,14 +259,14 @@ class BlogPost(models.Model): return groups @api.multi - def _notify_customize_recipients(self, message, msg_vals, recipients_vals): + def _notify_customize_recipients(self, message, msg_vals): """ Override to avoid keeping all notified recipients of a comment. We avoid tracking needaction on post comments. Only emails should be sufficient. """ msg_type = msg_vals.get('message_type') or message.message_type if msg_type == 'comment': return {'needaction_partner_ids': []} - return {} + return super(BlogPost, self)._notify_customize_recipients(message, msg_vals) def _default_website_meta(self): res = super(BlogPost, self)._default_website_meta() diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py index b2673a24351..4ec595516a5 100644 --- a/addons/website_forum/models/forum.py +++ b/addons/website_forum/models/forum.py @@ -422,12 +422,13 @@ class Post(models.Model): return post @api.model - def check_mail_message_access(self, res_ids, operation, model_name=None): + def get_mail_message_access(self, res_ids, operation, model_name=None): + # XDO FIXME: to be correctly fixed with new get_mail_message_access and filter access rule if operation in ('write', 'unlink') and (not model_name or model_name == 'forum.post'): # Make sure only author or moderator can edit/delete messages if any(not post.can_edit for post in self.browse(res_ids)): raise KarmaError('Not enough karma to edit a post.') - return super(Post, self).check_mail_message_access(res_ids, operation, model_name=model_name) + return super(Post, self).get_mail_message_access(res_ids, operation, model_name=model_name) @api.multi def write(self, vals): @@ -805,14 +806,14 @@ class Post(models.Model): return super(Post, self).message_post(message_type=message_type, **kwargs) @api.multi - def _notify_customize_recipients(self, message, msg_vals, recipients_vals): + def _notify_customize_recipients(self, message, msg_vals): """ Override to avoid keeping all notified recipients of a comment. We avoid tracking needaction on post comments. Only emails should be sufficient. """ msg_type = msg_vals.get('message_type') or message.message_type if msg_type == 'comment': return {'needaction_partner_ids': [], 'partner_ids': []} - return {} + return super(Post, self)._notify_customize_recipients(message, msg_vals) class PostReason(models.Model): diff --git a/odoo/addons/base/models/ir_attachment.py b/odoo/addons/base/models/ir_attachment.py index a3176dccef8..ce98ee7c296 100644 --- a/odoo/addons/base/models/ir_attachment.py +++ b/odoo/addons/base/models/ir_attachment.py @@ -11,7 +11,7 @@ from collections import defaultdict import uuid from odoo import api, fields, models, tools, SUPERUSER_ID, _ -from odoo.exceptions import AccessError, ValidationError +from odoo.exceptions import AccessError, ValidationError, MissingError from odoo.tools import config, human_size, ustr, html_escape from odoo.tools.mimetypes import guess_mimetype @@ -313,6 +313,8 @@ class IrAttachment(models.Model): def _check_serving_attachments(self): # restrict writing on attachments that could be served by the # ir.http's dispatch exception handling + # XDO note: this should be done in check(write), constraints for access rights? + # XDO note: if read on sudo, read twice, one for constraints, one for _inverse_datas as user if self.env.user._is_admin(): return if self.type == 'binary' and self.url: @@ -326,13 +328,15 @@ class IrAttachment(models.Model): In the 'document' module, it is overriden to relax this hard rule, since more complex ones apply there. """ + if self.env.user._is_superuser(): + return True # collect the records to check (by model) model_ids = defaultdict(set) # {model_name: set(ids)} require_employee = False if self: self._cr.execute('SELECT res_model, res_id, create_uid, public, res_field FROM ir_attachment WHERE id IN %s', [tuple(self.ids)]) for res_model, res_id, create_uid, public, res_field in self._cr.fetchall(): - if self.env.user.id != SUPERUSER_ID and not self.env.user._is_system() and res_field: + if not self.env.user._is_system() and res_field: raise AccessError(_("Sorry, you are not allowed to access this document.")) if public and mode == 'read': continue @@ -488,12 +492,20 @@ class IrAttachment(models.Model): @api.model_create_multi def create(self, vals_list): + record_tuple_set = set() for values in vals_list: # remove computed field depending of datas for field in ('file_size', 'checksum'): values.pop(field, False) values = self._check_contents(values) - self.browse().check('write', values=values) + # 'check()' only uses res_model and res_id from values, and make an exists. + # We can group the values by model, res_id to make only one query when + # creating multiple attachments on a single record. + record_tuple = (values.get('res_model'), values.get('res_id')) + record_tuple_set.add(record_tuple) + for record_tuple in record_tuple_set: + (res_model, res_id) = record_tuple + self.check('write', values={'res_model':res_model, 'res_id':res_id}) return super(IrAttachment, self).create(vals_list) @api.multi @@ -504,10 +516,13 @@ class IrAttachment(models.Model): def generate_access_token(self): if self.access_token: return self.access_token - access_token = str(uuid.uuid4()) + access_token = self._generate_access_token() self.write({'access_token': access_token}) return access_token + def _generate_access_token(self): + return str(uuid.uuid4()) + @api.model def action_get(self): return self.env['ir.actions.act_window'].for_xml_id('base', 'action_attachment')