From 6f43977a4e2837cc8a2b47f3cab086f334c20978 Mon Sep 17 00:00:00 2001 From: XavierDo Date: Fri, 12 Apr 2019 11:43:36 +0000 Subject: [PATCH] [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):