From 8fb59b55d9ffa3cdcd81508b5c522cef19da0027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 27 Jan 2023 10:52:50 +0000 Subject: [PATCH] [IMP] mail: improve 'real author' detection when posting a message When posting a message, author is added in followers in order to receive answers. They also do not receive its own messages, to avoid being notified of something they just posted. However the real author is not always the message author. In this commit we better define this author * when current user is active, real author is the current user. It means posting on behalf of someone will send answers to you, not to the spoofed partner; * when current user is inactive, real author is the message author. An inactive current user means the post was probably done as sudo, aka in the mailgateway or frontend. In that case we don't want to add odoobot in followers, but the actual message author; Task-3093257 (Mail: The Composer Update) Part-of: odoo/odoo#107356 --- addons/mail/models/mail_thread.py | 34 ++++++++++-- addons/test_mail/tests/test_message_post.py | 55 +++++++++++++++++++ .../tests/test_website_blog_flow.py | 2 +- 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 4ea545361f7..21a5f9d77c3 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1927,9 +1927,25 @@ class MailThread(models.AbstractModel): ) # attachement_ids, body new_message = self._message_create([msg_values]) - if msg_values['author_id'] and msg_values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): - if self.env['res.partner'].browse(msg_values['author_id']).active: # we dont want to add odoobot/inactive as a follower - self._message_subscribe(partner_ids=[msg_values['author_id']]) + # subscribe author(s) so that they receive answers; do it only when it is + # a manual post by the author (aka not a system notification, not a message + # posted 'in behalf of', and if still active). + author_subscribe = (not self._context.get('mail_create_nosubscribe') and + msg_values['message_type'] != 'notification') + if author_subscribe: + real_author_id = False + # if current user is active, they are the one doing the action and should + # be notified of answers. If they are inactive they are posting on behalf + # of someone else (a custom, mailgateway, ...) and the real author is the + # message author + if self.env.user.active: + real_author_id = self.env.user.partner_id.id + elif msg_values['author_id']: + author = self.env['res.partner'].browse(msg_values['author_id']) + if author.active: + real_author_id = author.id + if real_author_id: + self._message_subscribe(partner_ids=[real_author_id]) self._message_post_after_hook(new_message, msg_values) self._notify_thread(new_message, msg_values, **notif_kwargs) @@ -3220,9 +3236,17 @@ class MailThread(models.AbstractModel): # notify author of its own messages, False by default notify_author = kwargs.get('notify_author') or self.env.context.get('mail_notify_author') - author_id = msg_vals.get('author_id') or message.author_id.id + real_author_id = False + if not notify_author: + if self.env.user.active: + real_author_id = self.env.user.partner_id.id + elif msg_vals.get('author_id'): + real_author_id = msg_vals['author_id'] + else: + real_author_id = message.author_id.id + for pid, pdata in res.items(): - if pid and not notify_author and pid == author_id: + if pid and pid == real_author_id: continue if pdata['active'] is False: continue diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 10d7fd9905f..d4e7994173b 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -659,6 +659,61 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase): partner_ids=self.partner_portal.ids, ) + @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') + @users('employee') + def test_message_post_author(self): + """ Test author recognition """ + test_record = self.test_record.with_env(self.env) + + # when a user spoofs the author: the actual author is the current user + # and not the message author + with self.assertSinglePostNotifications( + [{'partner': self.partner_admin, 'type': 'email'}], + {'content': 'Body'} + ): + new_message = test_record.message_post( + author_id=self.partner_employee_2.id, + body='Body', + message_type='comment', + subtype_xmlid='mail.mt_comment', + partner_ids=[self.partner_admin.id], + ) + + self.assertMessageFields( + new_message, + {'author_id': self.partner_employee_2, + 'email_from': formataddr((self.partner_employee_2.name, self.partner_employee_2.email_normalized)), + 'message_type': 'comment', + 'notified_partner_ids': self.partner_admin, + 'subtype_id': self.env.ref('mail.mt_comment'), + } + ) + self.assertEqual(test_record.message_partner_ids, self.partner_employee, + 'Real author is added in followers, not message author') + + # should be skipped with notifications + test_record.message_unsubscribe(partner_ids=self.partner_employee.ids) + _new_message = test_record.message_post( + author_id=self.partner_employee_2.id, + body='Body', + message_type='notification', + subtype_xmlid='mail.mt_comment', + partner_ids=[self.partner_admin.id], + ) + self.assertFalse(test_record.message_partner_ids, 'Notification should not add author in followers') + + # inactive users are not considered as authors + self.env.user.with_user(self.user_admin).active = False + _new_message = test_record.message_post( + author_id=self.partner_employee_2.id, + body='Body', + message_type='comment', + subtype_xmlid='mail.mt_comment', + partner_ids=[self.partner_admin.id], + ) + self.assertEqual(test_record.message_partner_ids, self.partner_employee_2, + 'Author is the message author when user is inactive, and shoud be added in followers') + @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests') @users('employee') def test_message_post_defaults(self): diff --git a/addons/website_blog/tests/test_website_blog_flow.py b/addons/website_blog/tests/test_website_blog_flow.py index 29276e935cd..0bf8859fade 100644 --- a/addons/website_blog/tests/test_website_blog_flow.py +++ b/addons/website_blog/tests/test_website_blog_flow.py @@ -56,7 +56,7 @@ class TestWebsiteBlogFlow(TestWebsiteBlogCommon): 'website_blog: peuple following a blog should be notified of a published post') # Armand posts a message -> becomes follower - self.test_blog_post.sudo().message_post( + self.test_blog_post.with_user(self.user_employee).message_post( body='Armande BlogUser Commented', message_type='comment', author_id=self.user_employee.partner_id.id,