From b3d9fbd3d5268e25ee61f21d5df72a0b3668188b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Dufays Date: Mon, 21 Nov 2022 15:09:43 +0000 Subject: [PATCH 1/4] [IMP] mail: allows unfollowing record from inbox Allow users to unfollow easily document from the inbox for which he/she doesn't want to be notified anymore. When hovering on a follow up message with a related document followed by the user, the interface displays an action to unfollow it that allows the user to remove himself/herself as follower of the document. As a result of performing that action a toast is displayed as a feedback. Technical note: The unfollow link is displayed on a message.canUnfollow is true which check that the thread related to the message is followed by the user. In order to know if the user follows the record related to the message, we now transmit with each message the id of the follower table that link the user to the related record if it follows it. This allows to insert the follower on client side so that the unfollow action can use the standard mechaism already developed to unfollow a record. Unfortunately, that information coudn't be added in the already public method message_format of mail message because that method is not always called on the user retreiving the message but also by the one posting message. We didn't want also to include all followers in the formated message so we rely on additional method that add that information per partner. To support unfollowing a document in the inbox no matter the current company, we have modified message_unsubscribe in mail_thread to allow internal user to unsubscribe themself without checking any rights. Indeed, some document have record rule that prevents reading it if the user is not in the right company (ex. crm.lead) and then was preventing the user to unfollow the document using that method. Task-3061864 Part-of: odoo/odoo#107978 --- addons/mail/controllers/mailbox.py | 5 +- addons/mail/models/mail_message.py | 68 ++++++++++++++++++- addons/mail/models/mail_thread.py | 23 +++++-- .../mail/static/src/core/message_service.js | 22 ++++++ addons/mail/static/src/core_ui/message.js | 4 ++ addons/mail/static/src/core_ui/message.xml | 1 + .../static/tests/discuss_app/discuss_tests.js | 60 ++++++++++++++++ .../mock_server/controllers/discuss.js | 2 +- .../mock_server/models/mail_message.js | 25 +++++++ 9 files changed, 199 insertions(+), 11 deletions(-) diff --git a/addons/mail/controllers/mailbox.py b/addons/mail/controllers/mailbox.py index a5f335449a9..26397082de6 100644 --- a/addons/mail/controllers/mailbox.py +++ b/addons/mail/controllers/mailbox.py @@ -7,8 +7,11 @@ from odoo.http import request class MailboxController(http.Controller): @http.route("/mail/inbox/messages", methods=["POST"], type="json", auth="user") def discuss_inbox_messages(self, before=None, after=None, limit=30, around=None): + partner_id = request.env.user.partner_id.id domain = [("needaction", "=", True)] - return request.env["mail.message"]._message_fetch(domain, before, after, around, limit).message_format() + return (request.env["mail.message"] + ._message_fetch(domain, before, after, around, limit) + ._message_format_personalize(partner_id)) @http.route("/mail/history/messages", methods=["POST"], type="json", auth="user") def discuss_history_messages(self, before=None, after=None, limit=30): diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index d80dc68f2fe..156b7bf35e0 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -10,7 +10,7 @@ from collections import defaultdict from odoo import _, api, Command, fields, models, modules, tools from odoo.exceptions import AccessError from odoo.osv import expression -from odoo.tools.misc import clean_context +from odoo.tools.misc import clean_context, groupby as tools_groupby _logger = logging.getLogger(__name__) _image_dataurl = re.compile(r'(data:image/[a-z]+?);base64,([a-z0-9+/\n]{3,}=*)\n*([\'"])(?: data-filename="([^"]*)")?', re.I) @@ -1005,7 +1005,6 @@ class Message(models.Model): com_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment') note_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note') - # fetch scheduled notifications once, only if msg_vals is not given to # avoid useless queries when notifying Inbox right after a message_post scheduled_dt_by_msg_id = {} @@ -1034,6 +1033,71 @@ class Message(models.Model): vals['module_icon'] = modules.module.get_module_icon(self.env[vals['model']]._original_module) return vals_list + @api.model + def _message_format_personalized_prepare(self, messages_formatted, partner_ids=None): + """ Prepare message to be personalized by partner. + + This method add partner information in batch to the messages so that the + messages could be personalized for each partner by using + _message_format_personalize with no or a limited number of queries. + + For example, it gathers all followers of the record related to the messages + in one go (or limit it to the partner given in parameter), so that the method + _message_format_personalize could then personalize each message for each partner. + + Note that followers for message related to discuss.channel are not fetched. + + :param list messages_formatted: list of message formatted using the method + message_format + :param list partner_ids: (optional) limit value computation to the partners + of the given list; if not set, all partners are considered (all partners + following each record will be included in follower_id_by_partner_id). + + :return: list of messages_formatted with added value: + 'follower_id_by_partner_id': dict partner_id -> follower_id of the record + """ + domain = expression.OR([ + [('res_model', '=', model), ('res_id', 'in', list({value['res_id'] for value in values}))] + for model, values in tools_groupby( + (vals for vals in messages_formatted + if vals.get("res_id") and vals.get("model") not in {None, False, '', 'discuss.channel'}), + key=lambda r: r["model"] + ) + ]) + if partner_ids: + domain = expression.AND([domain, [('partner_id', 'in', partner_ids)]]) + records_followed = self.env['mail.followers'].sudo().search(domain) + followers_by_record_ref = ( + {(res_model, res_id): {value['partner_id'][0]: value['id'] for value in values} + for (res_model, res_id), values in tools_groupby( + records_followed.read(['res_id', 'res_model', 'partner_id']), + key=lambda r: (r["res_model"], r['res_id']) + )}) + for vals in messages_formatted: + vals['follower_id_by_partner_id'] = followers_by_record_ref.get((vals['model'], vals['res_id']), dict()) + return messages_formatted + + def _message_format_personalize(self, partner_id, messages_formatted=None, format_reply=True, msg_vals=None): + """ Personalize the messages for the partner. + + :param integer partner_id: id of the partner to personalize the messages for + :param list messages_formatted: (optional) list of message formatted using + the method _message_format_personalized_prepare. + If not provided message_format is called on self with the 2 next parameters + to format the messages and then the messages are personalized. + :param bool format_reply: (optional) see method message_format + :param dict msg_vals: (optional) see method message_format + :return: list of messages_formatted personalized for the partner + """ + if not messages_formatted: + messages_formatted = self.message_format(format_reply=format_reply, msg_vals=msg_vals) + self._message_format_personalized_prepare(messages_formatted, [partner_id]) + for vals in messages_formatted: + # set value for user being a follower, fallback to False if not prepared + follower_id_by_pid = vals.pop('follower_id_by_partner_id', {}) + vals['user_follower_id'] = follower_id_by_pid.get(partner_id, False) + return messages_formatted + def _get_message_format_fields(self): return [ 'id', 'body', 'date', 'email_from', # base message fields diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index ac1d0023ab1..cb40936dae9 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2995,9 +2995,15 @@ class MailThread(models.AbstractModel): } for pid in inbox_pids] self.env['mail.notification'].sudo().create(notif_create_values) - message_format_values = message.message_format(msg_vals=msg_vals)[0] + MailMessage = self.env['mail.message'] + messages_format_prepared = MailMessage._message_format_personalized_prepare( + message.message_format(msg_vals=msg_vals), partner_ids=inbox_pids) for partner_id in inbox_pids: - bus_notifications.append((self.env['res.partner'].browse(partner_id), 'mail.message/inbox', dict(message_format_values))) + bus_notifications.append( + (self.env['res.partner'].browse(partner_id), + 'mail.message/inbox', + MailMessage._message_format_personalize(partner_id, messages_format_prepared)[0]) + ) self.env['bus.bus'].sudo()._sendmany(bus_notifications) def _notify_thread_by_email(self, message, recipients_data, msg_vals=False, @@ -3820,16 +3826,19 @@ class MailThread(models.AbstractModel): # not necessary for computation, but saves an access right check if not partner_ids: return True - if set(partner_ids) == set([self.env.user.partner_id.id]): - self.check_access_rights('read') - self.check_access_rule('read') - else: + # To support unfollowing a document in the inbox no matter the current + # company, we allow internal users to unsubscribe themselves without + # checking any rights. + if set(partner_ids) != {self.env.user.partner_id.id}: self.check_access_rights('write') self.check_access_rule('write') + elif not self.env.user._is_internal(): + self.check_access_rights('read') + self.check_access_rule('read') self.env['mail.followers'].sudo().search([ ('res_model', '=', self._name), ('res_id', 'in', self.ids), - ('partner_id', 'in', partner_ids or []), + ('partner_id', 'in', partner_ids), ]).unlink() def _message_auto_subscribe_followers(self, updated_values, default_subtype_ids): diff --git a/addons/mail/static/src/core/message_service.js b/addons/mail/static/src/core/message_service.js index c758c22aca0..a3346a695dc 100644 --- a/addons/mail/static/src/core/message_service.js +++ b/addons/mail/static/src/core/message_service.js @@ -3,7 +3,9 @@ import { Message } from "./message_model"; import { removeFromArrayWithPredicate } from "../utils/arrays"; import { convertBrToLineBreak, prettifyMessageContent } from "../utils/format"; +import { _t } from "@web/core/l10n/translation"; import { registry } from "@web/core/registry"; +import { sprintf } from "@web/core/utils/strings"; import { MessageReactions } from "./message_reactions_model"; import { Notification } from "./notification_model"; import { LinkPreview } from "./link_preview_model"; @@ -141,6 +143,18 @@ export class MessageService { }); } + async unfollow(message) { + if (message.isNeedaction) { + await this.setDone(message); + } + const thread = message.originThread; + await this.env.services["mail.thread"].removeFollower(thread.followerOfSelf); + this.env.services.notification.add( + sprintf(_t('You are no longer following "%(thread_name)s".'), { thread_name: thread.name }), + { type: "success" } + ); + } + async unstarAll() { // apply the change immediately for faster feedback this.store.discuss.starred.counter = 0; @@ -285,6 +299,14 @@ export class MessageService { if (data.res_model_name) { message.originThread.modelName = data.res_model_name; } + if ("user_follower_id" in data && data.user_follower_id && this.store.self) { + this.env.services["mail.thread"].insertFollower({ + followedThread: message.originThread, + id: data.user_follower_id, + isActive: true, + partner: this.store.self, + }); + } this._updateReactions(message, data.messageReactionGroups); if (message.isNotification && !message.notificationType) { const parser = new DOMParser(); diff --git a/addons/mail/static/src/core_ui/message.js b/addons/mail/static/src/core_ui/message.js index 746f5e14276..9b1ab2a4971 100644 --- a/addons/mail/static/src/core_ui/message.js +++ b/addons/mail/static/src/core_ui/message.js @@ -216,6 +216,10 @@ export class Message extends Component { return Boolean(!this.message.isTransient && this.message.resId); } + get canUnfollow() { + return Boolean(this.message.originThread?.followerOfSelf); + } + /** * Determines whether clicking on the author's avatar opens a chat with the * author. diff --git a/addons/mail/static/src/core_ui/message.xml b/addons/mail/static/src/core_ui/message.xml index effc93503bd..1c5641109c3 100644 --- a/addons/mail/static/src/core_ui/message.xml +++ b/addons/mail/static/src/core_ui/message.xml @@ -145,6 +145,7 @@ View Reactions Mark as Read + Unfollow Mark as Unread Edit diff --git a/addons/mail/static/tests/discuss_app/discuss_tests.js b/addons/mail/static/tests/discuss_app/discuss_tests.js index d2a70faadf5..76261d419d5 100644 --- a/addons/mail/static/tests/discuss_app/discuss_tests.js +++ b/addons/mail/static/tests/discuss_app/discuss_tests.js @@ -827,6 +827,66 @@ QUnit.test("rendering of inbox message", async (assert) => { assert.containsOnce($message, "[title='Mark as Read']"); }); +QUnit.test('Unfollow message', async function (assert) { + assert.expect(12); + + const pyEnv = await startServer(); + const currentPartnerId = pyEnv.currentPartnerId; + const [threadFollowedId, threadNotFollowedId] = pyEnv["res.partner"].create([{ + name: 'Thread followed', + }, { + name: 'Thread not followed', + }]); + pyEnv["mail.followers"].create({ + partner_id: currentPartnerId, + res_id: threadFollowedId, + res_model: "res.partner", + }); + for (let threadId of [threadFollowedId, threadFollowedId, threadNotFollowedId]) { + const messageId = pyEnv["mail.message"].create({ + body: "not empty", + model: "res.partner", + needaction: true, + needaction_partner_ids: [currentPartnerId], + res_id: threadId, + }); + pyEnv["mail.notification"].create({ + mail_message_id: messageId, + notification_status: "sent", + notification_type: "inbox", + res_partner_id: currentPartnerId, + }); + } + const { openDiscuss } = await start(); + await openDiscuss(); + // 2 messages about "Thread followed" with unfollow button and 1 message about "Thread not followed" without it + assert.containsN($, ".o-mail-Message", 3); + for (let messageN of [0, 1]) { + const $message = $(`.o-mail-Message:eq(${messageN})`); + assert.containsOnce($message, ".o-mail-Message-header:contains(on Thread followed)"); + await afterNextRender(() => $message.find("[title='Expand']").click()); + assert.containsOnce($message, "[title='Unfollow']"); + } + const $messageNotFollowed = $(".o-mail-Message:eq(2)"); + assert.containsOnce($messageNotFollowed, ".o-mail-Message-header:contains(on Thread not followed)"); + await afterNextRender(() => $messageNotFollowed.find("[title='Expand']").click()); + assert.containsNone($messageNotFollowed, "[title='Unfollow']"); + + const $message0Followed = $(".o-mail-Message:eq(0)"); + await afterNextRender(() => $message0Followed.find("[title='Expand']").click()); + await afterNextRender(() => $message0Followed.find("[title='Unfollow']").click()); + + assert.containsN($, ".o-mail-Message", 2, "Unfollowing message 0 marks it as read -> Message removed"); + assert.containsOnce($, ".o-mail-Message-header:contains(on Thread followed)"); + assert.containsOnce($, ".o-mail-Message-header:contains(on Thread not followed)"); + for (let messageN of [0, 1]) { + const $message = $(`.o-mail-Message:eq(${messageN})`); + await afterNextRender(() => $message.find("[title='Expand']").click()); + assert.containsNone($, "button[title='Unfollow']", + "Unfollowing message 0 -> unfollowing 'Thread followed' -> no more unfollow action on any message"); + } +}); + QUnit.test('messages marked as read move to "History" mailbox', async (assert) => { const pyEnv = await startServer(); const channelId = pyEnv["discuss.channel"].create({ name: "other-disco" }); diff --git a/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js b/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js index 96ec2cb3771..603447ccaf0 100644 --- a/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js +++ b/addons/mail/static/tests/helpers/mock_server/controllers/discuss.js @@ -336,7 +336,7 @@ patch(MockServer.prototype, "mail/controllers/discuss", { _mockRouteMailMessageInbox(after = false, before = false, around = false, limit = 30) { const domain = [["needaction", "=", true]]; const messages = this._mockMailMessage_MessageFetch(domain, before, after, around, limit); - return this._mockMailMessageMessageFormat(messages.map((message) => message.id)); + return this._mockMailMessageFormatPersonalize(messages.map((message) => message.id)); }, /** * Simulates the `/mail/starred/messages` route. diff --git a/addons/mail/static/tests/helpers/mock_server/models/mail_message.js b/addons/mail/static/tests/helpers/mock_server/models/mail_message.js index 3b6ba58ed24..07ebf4660fe 100644 --- a/addons/mail/static/tests/helpers/mock_server/models/mail_message.js +++ b/addons/mail/static/tests/helpers/mock_server/models/mail_message.js @@ -302,6 +302,31 @@ patch(MockServer.prototype, "mail/models/mail_message", { return response; }); }, + /** + * Simulate `_message_format_personalize` on `mail.message` for the current partner. + * + * @private + * @returns {integer[]} ids + * @returns {Object[]} + */ + _mockMailMessageFormatPersonalize(ids) { + const messages = this._mockMailMessageMessageFormat(ids); + messages.forEach(message => { + let user_follower_id = false; + if (message.model && message.res_id) { + const follower = this.getRecords("mail.followers", [ + ["res_model", "=", message.model], + ["res_id", "=", message.res_id], + ["partner_id", "=", this.pyEnv.currentPartnerId], + ]); + if (follower.length !== 0) { + user_follower_id = follower[0].id; + } + } + message.user_follower_id = user_follower_id; + }); + return messages; + }, /** * Simulates `_message_notification_format` on `mail.message`. * From 3e57dc295c81cd03ca1a7238b0ed1a41c9e64b63 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Dufays Date: Thu, 24 Nov 2022 07:29:29 +0000 Subject: [PATCH 2/4] [IMP] mail, portal, website_slides, mass_mailing: unfollow record from email Allow partner to unfollow a document from a follow up email of that document through an unsubscribe URL in the email even if not connected. It works for internal user for follow up on any document and for any partner on follow up of document tagged as authorizing being unfollowed by any partner ( slide.channel and slide.slide). Technical note: The unfollow block is rendered in mail_thread and updated for each recipient in mail_mail. We don't render it in mail_mail because we don't have the language of the message at that point. Depending on the partner and the related document, the unfollow block is: - either removed if the user cannot unfollow the document (for example if it doesn't follow it) - or updated with partner and document information + a security token Task-3061864 Part-of: odoo/odoo#107978 --- addons/mail/__manifest__.py | 1 + addons/mail/controllers/mail.py | 27 ++++++++- .../data/mail_templates_email_layouts.xml | 10 +++- addons/mail/models/mail_followers.py | 27 +++++++++ addons/mail/models/mail_mail.py | 56 +++++++++++++++---- addons/mail/views/mail_templates_public.xml | 33 +++++++++++ addons/mass_mailing/models/mail_mail.py | 4 +- addons/portal/__manifest__.py | 1 + addons/portal/controllers/mail.py | 5 ++ addons/portal/views/mail_templates_public.xml | 8 +++ addons/website_slides/models/slide_channel.py | 1 + addons/website_slides/models/slide_slide.py | 1 + 12 files changed, 159 insertions(+), 15 deletions(-) create mode 100644 addons/mail/views/mail_templates_public.xml create mode 100644 addons/portal/views/mail_templates_public.xml diff --git a/addons/mail/__manifest__.py b/addons/mail/__manifest__.py index a9b70d82eb1..57014d155b9 100644 --- a/addons/mail/__manifest__.py +++ b/addons/mail/__manifest__.py @@ -101,6 +101,7 @@ For more specific needs, you may also assign custom-defined actions 'views/mail_gateway_allowed_views.xml', 'views/mail_guest_views.xml', 'views/mail_message_reaction_views.xml', + 'views/mail_templates_public.xml', 'views/res_users_views.xml', 'views/res_users_settings_views.xml', 'views/mail_template_views.xml', diff --git a/addons/mail/controllers/mail.py b/addons/mail/controllers/mail.py index 62fcc30019e..a3abb1213ac 100644 --- a/addons/mail/controllers/mail.py +++ b/addons/mail/controllers/mail.py @@ -4,7 +4,7 @@ import logging from werkzeug.urls import url_encode -from odoo import http +from odoo import _, http from odoo.exceptions import AccessError from odoo.http import request from odoo.tools import consteq @@ -164,3 +164,28 @@ class MailController(http.Controller): except ValueError: res_id = False return self._redirect_to_record(model, res_id, access_token, **kwargs) + + # csrf is disabled here because it will be called by the MUA with unpredictable session at that time + @http.route('/mail/unfollow', type='http', auth='public', csrf=False) + def mail_action_unfollow(self, model, res_id, pid, token, **kwargs): + comparison, record, __ = MailController._check_token_and_record_or_redirect(model, int(res_id), token) + if not comparison or not record: + raise AccessError(_('Non existing record or wrong token.')) + + pid = int(pid) + record_sudo = record.sudo() + record_sudo.message_unsubscribe([pid]) + + display_link = True + if request.session.uid: + try: + record.check_access_rights('read') + record.check_access_rule('read') + except AccessError: + display_link = False + + return request.render('mail.message_document_unfollowed', { + 'name': record_sudo.display_name, + 'model_name': request.env['ir.model'].sudo()._get(model).display_name, + 'access_url': record._notify_get_action_link('view', model=model, res_id=res_id) if display_link else False, + }) diff --git a/addons/mail/data/mail_templates_email_layouts.xml b/addons/mail/data/mail_templates_email_layouts.xml index 91c06bccd2e..b631713e0bb 100644 --- a/addons/mail/data/mail_templates_email_layouts.xml +++ b/addons/mail/data/mail_templates_email_layouts.xml @@ -100,9 +100,12 @@

-

+

 @@ -172,6 +175,9 @@ Powered by Odoo + + | Unfollow + diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py index 31f75e7cdf7..4f4b66b6ea1 100644 --- a/addons/mail/models/mail_followers.py +++ b/addons/mail/models/mail_followers.py @@ -74,6 +74,33 @@ class Followers(models.Model): # Private tools methods to fetch followers data # -------------------------------------------------- + @api.model + def _get_mail_recipients_follower_status(self, mail_ids): + """ Get partner mail recipients that follows the related record of the mails. + + Note that followers for message related to discuss.channel are not fetched. + + :param list mail_ids: mail_mail ids + :return: followers of the related record of the mails limited to the + recipients of the mails as a set of tuple (model, res_id, partner_id). + :rtype: set + """ + self.env['mail.mail'].flush_model(['message_id', 'recipient_ids']) + self.env['mail.followers'].flush_model(['partner_id', 'res_model', 'res_id']) + self.env['mail.message'].flush_model(['model', 'res_id']) + # mail_mail_res_partner_rel is the join table for the m2m recipient_ids field + self.env.cr.execute(""" + SELECT message.model, message.res_id, mail_partner.res_partner_id + FROM mail_mail mail + JOIN mail_mail_res_partner_rel mail_partner ON mail_partner.mail_mail_id = mail.id + JOIN mail_message message ON mail.mail_message_id = message.id AND message.model != 'discuss.channel' + JOIN mail_followers follower ON message.model = follower.res_model + AND message.res_id = follower.res_id + AND mail_partner.res_partner_id = follower.partner_id + WHERE mail.id IN %(mail_ids)s + """, {'mail_ids': tuple(mail_ids)}) + return set(self.env.cr.fetchall()) + def _get_recipient_data(self, records, message_type, subtype_id, pids=None): """ Private method allowing to fetch recipients data based on a subtype. Purpose of this method is to fetch all data necessary to notify recipients diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index 2a8d9beecac..baf9fa11374 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -19,6 +19,7 @@ from odoo import tools from odoo.addons.base.models.ir_mail_server import MailDeliveryException _logger = logging.getLogger(__name__) +_UNFOLLOW_REGEX = re.compile(r'', re.DOTALL) class MailMail(models.Model): @@ -320,16 +321,39 @@ class MailMail(models.Model): return '' return self.env['mail.render.mixin']._replace_local_links(self.body_html) - def _prepare_outgoing_list(self): + def _personalize_outgoing_body(self, body, partner=False, recipients_follower_status=None): + """ Return a modified body based on the recipient (partner). + + It must be called when using standard notification layouts + even for message without partners. + + :param str body: body to personalize for the recipient + :param partner: recipient + :param set recipients_follower_status: see ``Followers._get_mail_recipients_follower_status()`` + """ + self.ensure_one() + + if (recipients_follower_status and '/mail/unfollow' in body and partner and + self.model and self.res_id and (self.model, self.res_id, partner.id) in recipients_follower_status and + (getattr(self.env[self.model], '_partner_unfollow_enabled', False) or + any(user._is_internal() for user in partner.user_ids))): + unfollow_url = self.env['mail.thread']._notify_get_action_link( + 'unfollow', model=self.model, res_id=self.res_id, pid=partner.id) + body = body.replace('/mail/unfollow', unfollow_url) + else: + body = re.sub(_UNFOLLOW_REGEX, '', body) + return body + + def _prepare_outgoing_list(self, recipients_follower_status=None): """ Return a list of emails to send based on current mail.mail. Each is a dictionary for specific email values, depending on a partner, or generic to the whole recipients given by mail.email_to. + :param set recipients_follower_status: see ``Followers._get_mail_recipients_follower_status()`` :return list: list of dicts used in IrMailServer.build_email() """ self.ensure_one() body = self._prepare_outgoing_body() - body_alternative = tools.html2plaintext(body) # headers headers = {} @@ -410,11 +434,15 @@ class MailMail(models.Model): else: email_attachments = [] - return [ - { + # Build final list of email values with personalized body for recipient + results = [] + for email_values in email_list: + partner_id = email_values['partner_id'] + body_personalized = self._personalize_outgoing_body(body, partner_id, recipients_follower_status) + results.append({ 'attachments': email_attachments, - 'body': body, - 'body_alternative': body_alternative, + 'body': body_personalized, + 'body_alternative': tools.html2plaintext(body_personalized), 'email_cc': email_values['email_cc'], 'email_from': self.email_from, 'email_to': email_values['email_to'], @@ -422,12 +450,13 @@ class MailMail(models.Model): 'headers': headers, 'message_id': self.message_id, 'object_id': f'{self.res_id}-{self.model}' if self.res_id else '', - 'partner_id': email_values['partner_id'], + 'partner_id': partner_id, 'references': self.references, 'reply_to': self.reply_to, 'subject': self.subject, - } for email_values in email_list - ] + }) + + return results def _split_by_mail_configuration(self): """Group the based on their "email_from" and their "mail_server_id". @@ -513,6 +542,13 @@ class MailMail(models.Model): def _send(self, auto_commit=False, raise_exception=False, smtp_session=None): IrMailServer = self.env['ir.mail_server'] + # Only retrieve recipient followers of the mails if needed + mails_with_unfollow_link = self.filtered(lambda m: m.body_html and '/mail/unfollow' in m.body_html) + recipients_follower_status = ( + None if not mails_with_unfollow_link + else self.env['mail.followers']._get_mail_recipients_follower_status(mails_with_unfollow_link.ids) + ) + for mail_id in self.ids: success_pids = [] failure_type = None @@ -553,7 +589,7 @@ class MailMail(models.Model): res = None # TDE note: could be great to pre-detect missing to/cc and skip sending it # to go directly to failed state update - email_list = mail._prepare_outgoing_list() + email_list = mail._prepare_outgoing_list(recipients_follower_status) # send each sub-email for email in email_list: diff --git a/addons/mail/views/mail_templates_public.xml b/addons/mail/views/mail_templates_public.xml new file mode 100644 index 00000000000..1e0eaa2d8fc --- /dev/null +++ b/addons/mail/views/mail_templates_public.xml @@ -0,0 +1,33 @@ + + + + + + diff --git a/addons/mass_mailing/models/mail_mail.py b/addons/mass_mailing/models/mail_mail.py index 5505cd1b451..07525652f29 100644 --- a/addons/mass_mailing/models/mail_mail.py +++ b/addons/mass_mailing/models/mail_mail.py @@ -62,9 +62,9 @@ class MailMail(models.Model): ) return body - def _prepare_outgoing_list(self): + def _prepare_outgoing_list(self, recipients_follower_status=None): """ Update mailing specific links to add tracking based on res_id """ - email_list = super()._prepare_outgoing_list() + email_list = super()._prepare_outgoing_list(recipients_follower_status) if not self.res_id or not self.mailing_id: return email_list diff --git a/addons/portal/__manifest__.py b/addons/portal/__manifest__.py index eb9fb69df1c..3561e462e5a 100644 --- a/addons/portal/__manifest__.py +++ b/addons/portal/__manifest__.py @@ -20,6 +20,7 @@ a dependency towards website editing and customization capabilities.""", 'security/ir.model.access.csv', 'data/mail_template_data.xml', 'data/mail_templates.xml', + 'views/mail_templates_public.xml', 'views/portal_templates.xml', 'views/res_config_settings_views.xml', 'wizard/portal_share_views.xml', diff --git a/addons/portal/controllers/mail.py b/addons/portal/controllers/mail.py index aa324962eaa..20467ca104d 100644 --- a/addons/portal/controllers/mail.py +++ b/addons/portal/controllers/mail.py @@ -265,3 +265,8 @@ class MailController(mail.MailController): url = url.replace(query=urls.url_encode(url_params)).to_url() return request.redirect(url) return super(MailController, cls)._redirect_to_record(model, res_id, access_token=access_token, **kwargs) + + # Add website=True to support the portal layout + @http.route('/mail/unfollow', type='http', website=True) + def mail_action_unfollow(self, model, res_id, pid, token, **kwargs): + return super().mail_action_unfollow(model, res_id, pid, token, **kwargs) diff --git a/addons/portal/views/mail_templates_public.xml b/addons/portal/views/mail_templates_public.xml new file mode 100644 index 00000000000..13765d20790 --- /dev/null +++ b/addons/portal/views/mail_templates_public.xml @@ -0,0 +1,8 @@ + + + + diff --git a/addons/website_slides/models/slide_channel.py b/addons/website_slides/models/slide_channel.py index 2e3d10248aa..36d063c0b5b 100644 --- a/addons/website_slides/models/slide_channel.py +++ b/addons/website_slides/models/slide_channel.py @@ -287,6 +287,7 @@ class Channel(models.Model): 'website.searchable.mixin', ] _order = 'sequence, id' + _partner_unfollow_enabled = True def _default_cover_properties(self): """ Cover properties defaults are overridden to keep a consistent look for the slides diff --git a/addons/website_slides/models/slide_slide.py b/addons/website_slides/models/slide_slide.py index fb5257ae1ff..b27a8282590 100644 --- a/addons/website_slides/models/slide_slide.py +++ b/addons/website_slides/models/slide_slide.py @@ -108,6 +108,7 @@ class Slide(models.Model): 'latest': 'date_published desc', } _order = 'sequence asc, is_category asc, id asc' + _partner_unfollow_enabled = True YOUTUBE_VIDEO_ID_REGEX = r'^.*(youtu.be\/|v\/|u\/\w\/|embed\/|watch\?v=|&v=)([^#&?]*).*' GOOGLE_DRIVE_DOCUMENT_ID_REGEX = r'(^https:\/\/docs.google.com|^https:\/\/drive.google.com).*\/d\/([^\/]*)' From c8b27bd3a005a820647b0e16769d7584a27cd7a0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Dufays Date: Wed, 19 Apr 2023 07:11:22 +0000 Subject: [PATCH 3/4] [IMP] crm, hr_work_entry_holidays, test_mail{_full}: update query count In order to display the unfollow link or not in emails or in the inbox, the system need to query the followers of the document. This is what explains the added queries. See odoo/odoo#107978 Task-3061864 --- .../tests/test_performance.py | 2 +- addons/test_mail/tests/test_performance.py | 40 +++++++++---------- .../tests/test_mail_performance.py | 2 +- addons/test_mail_full/tests/test_rating.py | 12 +++--- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/addons/hr_work_entry_holidays/tests/test_performance.py b/addons/hr_work_entry_holidays/tests/test_performance.py index 746a1a8f283..7272d7730a6 100644 --- a/addons/hr_work_entry_holidays/tests/test_performance.py +++ b/addons/hr_work_entry_holidays/tests/test_performance.py @@ -32,7 +32,7 @@ class TestWorkEntryHolidaysPerformance(TestWorkEntryHolidaysBase): self.richard_emp.generate_work_entries(date(2018, 1, 1), date(2018, 1, 2)) leave = self.create_leave(datetime(2018, 1, 1, 7, 0), datetime(2018, 1, 1, 18, 0)) - with self.assertQueryCount(__system__=92, admin=96): + with self.assertQueryCount(__system__=94, admin=98): leave.action_validate() leave.action_refuse() diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 4824a08b860..031775f6b29 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -348,7 +348,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'partner_ids': [(4, customer_id)], }) - with self.assertQueryCount(admin=34, employee=34): + with self.assertQueryCount(admin=36, employee=36): composer._action_send_mail() @users('admin', 'employee') @@ -369,7 +369,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'partner_ids': [(4, customer.id)], }) - with self.assertQueryCount(admin=35, employee=35): + with self.assertQueryCount(admin=37, employee=37): composer._action_send_mail() @users('admin', 'employee') @@ -393,7 +393,7 @@ class TestMailAPIPerformance(BaseMailPerformance): composer_form.attachment_ids.add(attachment) composer = composer_form.save() - with self.assertQueryCount(admin=46, employee=46): # tm+com 45/45 + with self.assertQueryCount(admin=49, employee=49): # tm+com 48/48 composer._action_send_mail() # notifications @@ -437,7 +437,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'partner_ids': [(4, customer_id)], }) - with self.assertQueryCount(admin=34, employee=34): + with self.assertQueryCount(admin=36, employee=36): composer._action_send_mail() @users('admin', 'employee') @@ -455,7 +455,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_template_id': test_template.id, }).create({}) - with self.assertQueryCount(admin=34, employee=34): + with self.assertQueryCount(admin=36, employee=36): composer._action_send_mail() # notifications @@ -479,7 +479,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_template_id': test_template.id, }).create({}) - with self.assertQueryCount(admin=42, employee=42): + with self.assertQueryCount(admin=44, employee=44): composer._action_send_mail() # notifications @@ -511,7 +511,7 @@ class TestMailAPIPerformance(BaseMailPerformance): ) composer = composer_form.save() - with self.assertQueryCount(admin=43, employee=43): + with self.assertQueryCount(admin=46, employee=46): composer._action_send_mail() # notifications @@ -541,7 +541,7 @@ class TestMailAPIPerformance(BaseMailPerformance): ) composer = composer_form.save() - with self.assertQueryCount(admin=61, employee=61): + with self.assertQueryCount(admin=64, employee=64): composer._action_send_mail() # notifications @@ -568,7 +568,7 @@ class TestMailAPIPerformance(BaseMailPerformance): # use another user already pre-defined with the email notification type, # so the ormcache is preserved. record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(admin=37, employee=37): + with self.assertQueryCount(admin=39, employee=39): record.write({ 'user_id': self.user_test_email.id, }) @@ -577,7 +577,7 @@ class TestMailAPIPerformance(BaseMailPerformance): @warmup def test_message_assignation_inbox(self): record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(admin=18, employee=18): + with self.assertQueryCount(admin=19, employee=19): record.write({ 'user_id': self.user_test_inbox.id, }) @@ -651,7 +651,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=30, employee=30): + with self.assertQueryCount(admin=31, employee=31): record.message_post( body=Markup('

Test Post Performances with an email ping

'), partner_ids=self.customer.ids, @@ -663,7 +663,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_message_post_one_inbox_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=17, employee=17): + with self.assertQueryCount(admin=18, employee=18): record.message_post( body=Markup('

Test Post Performances with an inbox ping

'), partner_ids=self.user_test.partner_id.ids, @@ -852,7 +852,7 @@ class TestMailComplexPerformance(BaseMailPerformance): record = self.container.with_user(self.env.user) # about 20 (19?) queries per additional customer group - with self.assertQueryCount(admin=53, employee=52): + with self.assertQueryCount(admin=56, employee=55): record.message_post( body=Markup('

Test Post Performances

'), message_type='comment', @@ -870,7 +870,7 @@ class TestMailComplexPerformance(BaseMailPerformance): template = self.env.ref('test_mail.mail_test_container_tpl') # about 20 (19 ?) queries per additional customer group - with self.assertQueryCount(admin=60, employee=59): + with self.assertQueryCount(admin=63, employee=62): record.message_post_with_source( template, message_type='comment', @@ -894,7 +894,7 @@ class TestMailComplexPerformance(BaseMailPerformance): 'default_template_id': test_template.id, }).create({}) - with self.assertQueryCount(admin=131, employee=131): + with self.assertQueryCount(admin=141, employee=141): messages_as_sudo = test_records.message_post_with_source( 'test_mail.mail_template_simple_test', render_values={'partner': self.user_test.partner_id}, @@ -960,7 +960,7 @@ class TestMailComplexPerformance(BaseMailPerformance): }) rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(admin=37, employee=37): + with self.assertQueryCount(admin=39, employee=39): rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) # write tracking message @@ -980,7 +980,7 @@ class TestMailComplexPerformance(BaseMailPerformance): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(admin=88, employee=88): + with self.assertQueryCount(admin=94, employee=94): rec = self.env['mail.test.ticket'].create({ 'name': 'Test', 'container_id': container_id, @@ -1009,7 +1009,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) self.assertEqual(len(rec1.message_ids), 1) - with self.assertQueryCount(admin=56, employee=56): + with self.assertQueryCount(admin=59, employee=59): rec.write({ 'name': 'Test2', 'container_id': self.container.id, @@ -1046,7 +1046,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(admin=62, employee=62): + with self.assertQueryCount(admin=65, employee=65): rec.write({ 'name': 'Test2', 'container_id': container_id, @@ -1309,7 +1309,7 @@ class TestMailHeavyPerformancePost(BaseMailPerformance): attachments = self.env['ir.attachment'].with_user(self.env.user).create(self.test_attachments_vals) # enable_logging = self.cr._enable_logging() if self.warm else nullcontext() # with self.assertQueryCount(employee=63), enable_logging: - with self.assertQueryCount(employee=60): + with self.assertQueryCount(employee=62): record_container.with_context({}).message_post( body=Markup('

Test body

'), subject='Test Subject', diff --git a/addons/test_mail_full/tests/test_mail_performance.py b/addons/test_mail_full/tests/test_mail_performance.py index 6a7a049c689..f21866eefd7 100644 --- a/addons/test_mail_full/tests/test_mail_performance.py +++ b/addons/test_mail_full/tests/test_mail_performance.py @@ -82,7 +82,7 @@ class TestMailPerformance(BaseMailPerformance): record_ticket = self.env['mail.test.ticket.mc'].browse(self.record_ticket.ids) attachments = self.env['ir.attachment'].create(self.test_attachments_vals) - with self.assertQueryCount(employee=85): # tmf: 85 + with self.assertQueryCount(employee=91): # tmf: 90 new_message = record_ticket.message_post( attachment_ids=attachments.ids, body=Markup('

Test Content

'), diff --git a/addons/test_mail_full/tests/test_rating.py b/addons/test_mail_full/tests/test_rating.py index 17b91df7f68..c03db13c05f 100644 --- a/addons/test_mail_full/tests/test_rating.py +++ b/addons/test_mail_full/tests/test_rating.py @@ -172,25 +172,25 @@ class TestRatingPerformance(TestRatingCommon): @users('employee') @warmup def test_rating_last_value_perfs(self): - with self.assertQueryCount(employee=1213): # tmf 1213 / com 1217 + with self.assertQueryCount(employee=1313): # tmf 1313 / com 1313 self.create_ratings('mail.test.rating.thread') - with self.assertQueryCount(employee=1801): # tmf 1801 + with self.assertQueryCount(employee=1901): # tmf 1901 self.apply_ratings(1) - with self.assertQueryCount(employee=1700): # tmf 1700 + with self.assertQueryCount(employee=1800): # tmf 1800 self.apply_ratings(5) @users('employee') @warmup def test_rating_last_value_perfs_with_rating_mixin(self): - with self.assertQueryCount(employee=1319): # tmf 1319 / com 1319 + with self.assertQueryCount(employee=1419): # tmf 1419 / com 1419 self.create_ratings('mail.test.rating') - with self.assertQueryCount(employee=2004): # tmf 2004 + with self.assertQueryCount(employee=2104): # tmf 2104 self.apply_ratings(1) - with self.assertQueryCount(employee=2003): # tmf 2003 + with self.assertQueryCount(employee=2103): # tmf 2103 self.apply_ratings(5) with self.assertQueryCount(employee=1): From a0cb5580489be4b787f92ccccadf9e12628faf3c Mon Sep 17 00:00:00 2001 From: Pierre-Yves Dufays Date: Wed, 19 Apr 2023 15:04:35 +0000 Subject: [PATCH 4/4] [IMP] test_mail: add unfollow record test Add tests that check the behavior of unfollowing a record from the inbox and the unfollow link in email. To support unfollowing a document in the inbox no matter the current company, we have modified message_unsubscribe in mail_thread to allow internal user to unsubscribe themself without checking any rights. Indeed, some document have record rule that prevents reading it if the user is not in the right company (ex. crm.lead) and then was preventing the user to unfollow the document using that method. We also add a test checking that internal user can unsubscribe a record without any read and write rights. And that a portal user can't in the same circumstance. Task-3061864 Part-of: odoo/odoo#107978 --- addons/test_mail/models/test_mail_models.py | 14 + addons/test_mail/security/ir.model.access.csv | 2 + addons/test_mail/tests/test_mail_followers.py | 313 +++++++++++++++++- 3 files changed, 328 insertions(+), 1 deletion(-) diff --git a/addons/test_mail/models/test_mail_models.py b/addons/test_mail/models/test_mail_models.py index af297ddbfd0..37e77df62eb 100644 --- a/addons/test_mail/models/test_mail_models.py +++ b/addons/test_mail/models/test_mail_models.py @@ -37,6 +37,20 @@ class MailTestSimpleWithMainAttachment(models.Model): _inherit = ['mail.test.simple', 'mail.thread.main.attachment'] +class MailTestSimpleUnfollow(models.Model): + """ A very simple model only inheriting from mail.thread when only + communication history is necessary with unfollow link enabled in + notification emails even for non-internal user. """ + _description = 'Simple Chatter Model' + _name = 'mail.test.simple.unfollow' + _inherit = ['mail.thread'] + _partner_unfollow_enabled = True + + name = fields.Char() + company_id = fields.Many2one('res.company') + email_from = fields.Char() + + class MailTestGateway(models.Model): """ A very simple model only inheriting from mail.thread to test pure mass mailing features and base performances. """ diff --git a/addons/test_mail/security/ir.model.access.csv b/addons/test_mail/security/ir.model.access.csv index 6214fdee0dd..7cfb1d72f56 100644 --- a/addons/test_mail/security/ir.model.access.csv +++ b/addons/test_mail/security/ir.model.access.csv @@ -3,6 +3,8 @@ access_mail_performance_thread,access_mail_performance_thread,model_mail_perform access_mail_performance_tracking_user,mail.performance.tracking,model_mail_performance_tracking,base.group_user,1,1,1,1 access_mail_test_simple_portal,mail.test.simple.portal,model_mail_test_simple,base.group_portal,1,0,0,0 access_mail_test_simple_user,mail.test.simple.user,model_mail_test_simple,base.group_user,1,1,1,1 +access_mail_test_simple_unfollow_portal,mail.test.simple.unfollow.portal,model_mail_test_simple_unfollow,base.group_portal,0,0,0,0 +access_mail_test_simple_unfollow_user,mail.test.simple.unfollow.user,model_mail_test_simple_unfollow,base.group_user,1,1,1,1 access_mail_test_simple_main_attachment_portal,mail.test.simple.main.attachment.portal,model_mail_test_simple_main_attachment,base.group_portal,1,0,0,0 access_mail_test_simple_main_attachment_user,mail.test.simple.main.attachment.user,model_mail_test_simple_main_attachment,base.group_user,1,1,1,1 access_mail_test_gateway_portal,mail.test.gateway.portal,model_mail_test_gateway,base.group_portal,1,0,0,0 diff --git a/addons/test_mail/tests/test_mail_followers.py b/addons/test_mail/tests/test_mail_followers.py index 4d52ba5eaa2..067b02e5669 100644 --- a/addons/test_mail/tests/test_mail_followers.py +++ b/addons/test_mail/tests/test_mail_followers.py @@ -2,11 +2,17 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from markupsafe import Markup +import re +from unittest.mock import patch +from urllib.parse import urlparse, urlencode, parse_qsl +from odoo import tools +from odoo.addons.mail.models.mail_mail import _UNFOLLOW_REGEX from odoo.addons.mail.tests.common import MailCommon from odoo.exceptions import AccessError from odoo.tests import tagged, users -from odoo.tools import mute_logger +from odoo.tests.common import HttpCase +from odoo.tools import mute_logger, email_normalize @tagged('mail_followers') @@ -766,3 +772,308 @@ class RecipientsNotificationTest(MailCommon): pids=test_partners.ids ) self.assertRecipientsData(recipients_data, False, test_partners) + + +@tagged('mail_followers', 'post_install', '-at_install') +class UnfollowUnreadableRecordTest(MailCommon): + """ Test message_unsubscribe on unreadable record. """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.test_record = cls.env['mail.test.simple'].with_context(cls._test_context).create({'name': 'Test'}) + cls.user_portal = cls._create_portal_user() + + def _message_unsubscribe_unreadable_record(self, user, override_check='check_access_rule'): + def raise_access_error(*args, **kwargs): + raise AccessError('Unreadable') + + with patch.object(self.test_record.__class__, override_check, side_effect=raise_access_error): + self.test_record.with_user(user).message_unsubscribe(user.partner_id.ids) + + def test_initial_data(self): + """ Test some initial value. """ + self.assertTrue(self.user_employee._is_internal()) + self.assertFalse(self.user_portal._is_internal()) + record_employee = self.test_record.with_user(self.user_employee) + record_employee.check_access_rights('read') + record_employee.check_access_rule('read') + record_portal = self.test_record.with_user(self.user_portal) + with self.assertRaises(AccessError): + record_portal.check_access_rights('write') + record_portal.check_access_rule('write') + + def test_internal_user_can_unsubscribe_from_unreadable_record(self): + self.test_record._message_subscribe(partner_ids=self.partner_employee.ids) + + self.assertIn(self.partner_employee, self.test_record.message_follower_ids.mapped('partner_id')) + self._message_unsubscribe_unreadable_record(self.user_employee) + self.assertNotIn(self.partner_employee, self.test_record.message_follower_ids.mapped('partner_id')) + + def test_portal_user_cannot_unsubscribe_from_unreadable_record(self): + self.test_record._message_subscribe(partner_ids=self.partner_portal.ids) + + self.assertIn(self.partner_portal, self.test_record.message_follower_ids.mapped('partner_id')) + with self.assertRaises(AccessError): + self._message_unsubscribe_unreadable_record(self.user_portal) + with self.assertRaises(AccessError): + self._message_unsubscribe_unreadable_record(self.user_portal, override_check='check_access_rights') + + +@tagged('mail_followers', 'post_install', '-at_install') +class UnfollowFromInboxTest(MailCommon): + """ Test unfollow mechanism from inbox (server part). """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.test_record = cls.env['mail.test.simple'].with_context(cls._test_context).create({'name': 'Test'}) + cls.user_employee.write({'notification_type': 'inbox'}) + + def _fetch_inbox_message(self, message_id, user=None): + """ Fetch the given message similarly to the controller. """ + MailMessage = self.env['mail.message'].with_user(user) if user else self.env['mail.message'] + partner_id = self.env.user.partner_id.id + return list(filter( + lambda m: m['id'] == message_id, + MailMessage._message_fetch(domain=[('needaction', '=', True)])._message_format_personalize(partner_id))) + + @users('employee') + @mute_logger('odoo.models') + def test_inbox_notification_follower(self): + """ Check follow-up information for displaying inbox messages used to + implement "unfollow" in the inbox. + + Note that the actual mechanism to unfollow a record from a message is + tested in the client part. + """ + test_record = self.env['mail.test.simple'].browse(self.test_record.ids) + message = test_record.with_user(self.user_admin).message_post( + body='test message', subtype_id=self.env.ref('mail.mt_comment').id, partner_ids=self.partner_employee.ids) + + # The user doesn't follow the record + messages = self._fetch_inbox_message(message.id) + self.assertEqual(len(messages), 1) + self.assertFalse(messages[0].get('user_follower_id')) + self.assertFalse('follower_id_by_partner_id' in messages[0]) + + # The user follows the record + test_record._message_subscribe(partner_ids=self.partner_employee.ids) + messages = self._fetch_inbox_message(message.id) + self.assertEqual(len(messages), 1) + follower_id = messages[0]['user_follower_id'] + self.assertTrue(follower_id) + self.assertFalse('follower_id_by_partner_id' in messages[0]) + follower = self.env['mail.followers'].browse(follower_id) + self.assertEqual(follower.res_model, test_record._name) + self.assertEqual(follower.res_id, test_record.id) + self.assertEqual(follower.partner_id, self.partner_employee) + + # The user doesn't follow the record anymore + test_record.message_unsubscribe(partner_ids=self.partner_employee.ids) + messages = self._fetch_inbox_message(message.id) + self.assertFalse(messages[0].get('user_follower_id')) + + +@tagged('mail_followers', 'post_install', '-at_install') +class UnfollowFromEmailTest(MailCommon, HttpCase): + """ Test unfollow mechanism from email. """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.user_portal = cls._create_portal_user() + cls.partner_portal = cls.user_portal.partner_id + cls.test_record = cls.env['mail.test.simple'].with_context(cls._test_context).create({'name': 'Test'}) + cls.test_record_unfollow = cls.env['mail.test.simple.unfollow'].with_context(cls._test_context).create( + {'name': 'unfollow'}) + cls.partner_without_user = cls.env['res.partner'].create({ + 'name': 'Dave', + 'email': 'dave@odoo.com', + }) + cls.user_employee.write({'notification_type': 'email'}) + + def _post_message_and_get_unfollow_urls(self, record, partner_ids): + """ Post a message on the record for the partners and extract the unfollow URLs. """ + Partner = self.env['res.partner'] + with self.mock_mail_gateway(): + record.message_post(body='test message', subtype_id=self.env.ref('mail.mt_comment').id, + partner_ids=partner_ids.ids) + self.assertEqual(len(self._mails), len(partner_ids)) + mail_by_email = {Partner._parse_partner_name(email_to)[1]: mail + for mail in self._mails + for email_to in mail['email_to']} + + # Extract unfollow URL for each partner from the body of the emails + results = [] + for partner in partner_ids: + mail_body = mail_by_email[email_normalize(partner.email)]['body'] + + urls = list({link_url for _, link_url, _, _ in re.findall(tools.HTML_TAG_URL_REGEX, mail_body) + if '/mail/unfollow' in link_url}) + n_url = len(urls) + self.assertLessEqual(n_url, 1) + results.append(urls[0] if urls else False) + self.assertEqual(len(results), len(partner_ids)) + return results + + def _url_with_query_parameters_overridden(self, url, **kwargs): + """ Return the url with overridden query parameters by the additional + parameters. + """ + parsed_url = urlparse(url) + return parsed_url._replace(query=urlencode(dict(parse_qsl(parsed_url.query), **kwargs))).geturl() + + def _test_tampered_unfollow_url(self, record, unfollow_url, partner): + """ Test that tampered urls doesn't work. + + Test that: + - when the following parameters are altered, the browsing the URL returns + a 403 and doesn't unsubscribe the partner. + - when trying to use the same URL with another partner, it also returns a + 403 and doesn't unsubscribe the other partner. + """ + for param, value in (('token', '0000000000000000000000000000000000000000'), + ('model', 'mail.test.gateway'), + ('res_id', record.copy().id)): + with self.subTest(f'Tampered {param}'): + tampered_unfollow_url = self._url_with_query_parameters_overridden(unfollow_url, **{param: value}) + response = self.url_open(tampered_unfollow_url) + self.assertEqual(response.status_code, 403) + self.assertIn(partner, record.message_partner_ids) + + with self.subTest('Tampered partner id'): + record._message_subscribe(partner_ids=self.partner_admin.ids) + tampered_unfollow_url = self._url_with_query_parameters_overridden(unfollow_url, pid=self.partner_admin.id) + response = self.url_open(tampered_unfollow_url) + self.assertEqual(response.status_code, 403) + self.assertIn(partner, record.message_partner_ids) + self.assertIn(self.partner_admin, record.message_partner_ids) + record.message_unsubscribe(partner_ids=self.partner_admin.ids) + + def _test_unfollow_url(self, record, unfollow_url, partner): + """ Test that the unfollow url works. + + Test that: that browsing the unfollow URL unsubscribe the user from the record + """ + with self.subTest('Legitimate unfollow'): + # We test that the URL still work a second time if the user has been re-added + for _ in range(2): + try: + self.assertIn(partner, record.message_partner_ids) + response = self.url_open(unfollow_url) + self.assertEqual(response.status_code, 200) + self.assertNotIn(partner, record.message_partner_ids) + self.assertEqual(urlparse(response.url).path, '/mail/unfollow') + self.assertIn("You are no longer following the document", response.text) + self.assertIn('o_access_record_link', response.text) + finally: + record._message_subscribe(partner_ids=partner.ids) + + def test_initial_data(self): + """ Test some initial value. """ + self.assertTrue(self.user_employee._is_internal()) + self.assertFalse(self.user_portal._is_internal()) + record_employee = self.test_record.with_user(self.user_employee) + record_employee.check_access_rights('read') + record_employee.check_access_rule('read') + record_portal = self.test_record.with_user(self.user_portal) + with self.assertRaises(AccessError): + record_portal.check_access_rights('write') + record_portal.check_access_rule('write') + for template_ref in ('mail.mail_notification_layout', 'mail.mail_notification_light'): + with self.subTest(f'Unfollow link in {template_ref}'): + mail_template_arch = self.env.ref(template_ref).arch + self.assertIn('/mail/unfollow', mail_template_arch) + self.assertNotIn('/mail/unfollow', re.sub(_UNFOLLOW_REGEX, '', mail_template_arch)) + + @mute_logger('odoo.addons.base.models', 'odoo.addons.mail.controllers.mail', 'odoo.http', 'odoo.models') + def test_unfollow_internal_user(self): + """ Internal user must receive an unfollow URL, that cannot be tampered + and redirects to the correct page. + """ + test_partner = self.partner_employee + test_record = self.test_record + + # Test that the user receives an unfollow URL when following the record + test_record._message_subscribe(partner_ids=test_partner.ids) + with self.subTest('Internal user receives unfollow URL'): + unfollow_url = self._post_message_and_get_unfollow_urls(test_record, test_partner)[0] + self.assertTrue(unfollow_url) + + # Test unfollowing URL when user is not logged + self._test_unfollow_url(test_record, unfollow_url, test_partner) + self._test_tampered_unfollow_url(test_record, unfollow_url, test_partner) + + # Test unfollowing URL when user is logged + self.authenticate(self.user_employee.login, self.user_employee.login) + self._test_unfollow_url(test_record, unfollow_url, test_partner) + + # Test that the user doesn't receive the unfollow URL when not following the record + test_record.message_unsubscribe(partner_ids=test_partner.ids) + with self.subTest('Internal user simple notification (without unfollow URL)'): + unfollow_url = self._post_message_and_get_unfollow_urls(test_record, test_partner)[0] + self.assertFalse(unfollow_url) + + @mute_logger('odoo.models') + def test_unfollow_partner_with_no_user(self): + """ External partner must not receive an unfollow URL. """ + test_partner = self.partner_without_user + test_record = self.test_record + + test_record._message_subscribe(partner_ids=test_partner.ids) + with self.subTest('External partner must not receive an unfollow URL'): + unfollow_url = self._post_message_and_get_unfollow_urls(test_record, test_partner)[0] + self.assertFalse(unfollow_url) + + @mute_logger('odoo.addons.mail.controllers.mail', 'odoo.models', 'odoo.http') + def test_unfollow_partner_without_access_on_record_unfollow_enabled(self): + """ Partner without access must receive an unfollow URL for message + related to record with unfollow enabled. + """ + test_record = self.test_record_unfollow + for descr, test_partner in (('Partner without user', self.partner_without_user), + ('Portal partner without access', self.partner_portal)): + with self.subTest(descr): + # Test that the user receives an unfollow URL when following the record + test_record._message_subscribe(partner_ids=test_partner.ids) + with self.subTest('External partner receives an unfollow URL'): + unfollow_url = self._post_message_and_get_unfollow_urls(test_record, test_partner)[0] + self.assertTrue(unfollow_url) + + # Test unfollowing URL when user is not logged + self._test_unfollow_url(test_record, unfollow_url, test_partner) + self._test_tampered_unfollow_url(test_record, unfollow_url, test_partner) + + # Test that the user doesn't receive the unfollow URL when not following the record + test_record.message_unsubscribe(partner_ids=test_partner.ids) + with self.subTest('External partner not following must not receive unfollow URL'): + unfollow_url = self._post_message_and_get_unfollow_urls(test_record, test_partner)[0] + self.assertFalse(unfollow_url) + + def test_unfollow_partner_multi_recipients_multi_messages(self): + """ Test most of the cases above but with multiple recipients and messages. """ + # On a record with unfollow attribute disabled. + test_record = self.test_record + partners = self.partner_without_user + self.partner_portal + self.partner_employee + test_record._message_subscribe(partner_ids=partners.ids) + urls = self._post_message_and_get_unfollow_urls(test_record, partners) + url_partner_without_user, url_partner_portal, url_employee = urls[0], urls[1], urls[2] + + self.assertFalse(url_partner_without_user) + self.assertFalse(url_partner_portal) + self.assertTrue(url_employee) + self._test_unfollow_url(test_record, url_employee, self.partner_employee) + + # On a record with unfollow attribute enabled. + test_record = self.test_record_unfollow + test_record._message_subscribe(partner_ids=partners.ids) + urls = self._post_message_and_get_unfollow_urls(test_record, partners) + url_partner_without_user, url_partner_portal, url_employee = urls[0], urls[1], urls[2] + + self.assertTrue(url_partner_without_user) + self.assertTrue(url_partner_portal) + self.assertTrue(url_employee) + self._test_unfollow_url(test_record, url_partner_without_user, self.partner_without_user) + self._test_unfollow_url(test_record, url_partner_portal, self.partner_portal) + self._test_unfollow_url(test_record, url_employee, self.partner_employee)