[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

16 files changed, 185 insertions, 187 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)
-
-        # 0: Find the message's author, because we need it for private discussion
-        author_id = kwargs.get('author_id')
+        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')
+
+        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)
-
-        # 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)
+        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'])
+
+        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):