From 11e813d7f37a3416207da665ea905d02b26b05f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 23 May 2022 09:26:59 +0200 Subject: [PATCH] [REF] mass_mailing: cleanup subscriptions update (optin / optout from lists) PURPOSE Overall cleaning of subscription and exclusion management code from portal. This code comes mainly from v12 and can now benefit from cleaning and update. SPECIFICATIONS Purpose of this commit is to cleanup code that manages chosen lists from portal and updates opt-in and opt-out accordingly. Code is now located on mailing list model. When opting-it, new subscriptions have to be created for mailing lists so that email is part of the list. When opting-out we just have to toggle the opt_out flag on subscription model. Logged messages are also improved. We log on contact model the updated mailing lists for opt-in or opt-out. Task-2669037 (Mass Mailing: Refactor js/portal for subscription) Part-of: odoo/odoo#86084 --- addons/mass_mailing/controllers/main.py | 31 +++++-- addons/mass_mailing/models/mailing.py | 19 ---- addons/mass_mailing/models/mailing_list.py | 89 ++++++++++++++++++- .../js/mailing_portal_subscription_form.js | 2 +- addons/mass_mailing/tests/common.py | 7 +- .../tests/test_mailing_controllers.py | 6 +- .../mass_mailing/tests/test_mailing_list.py | 46 ++++++++++ 7 files changed, 167 insertions(+), 33 deletions(-) diff --git a/addons/mass_mailing/controllers/main.py b/addons/mass_mailing/controllers/main.py index b95074ee3e8..71513f06b61 100644 --- a/addons/mass_mailing/controllers/main.py +++ b/addons/mass_mailing/controllers/main.py @@ -34,6 +34,13 @@ class MassMailController(http.Controller): [('email', '=', tools.email_normalize(email))] ) + def _fetch_contacts(self, email): + if not email or not tools.email_normalize(email): + return request.env['mailing.contact'] + return request.env['mailing.contact'].sudo().search( + [('email_normalized', '=', tools.email_normalize(email))] + ) + # ------------------------------------------------------------ # SUBSCRIPTION MANAGEMENT # ------------------------------------------------------------ @@ -51,7 +58,7 @@ class MassMailController(http.Controller): def _mailing_unsubscribe_from_list(self, mailing, document_id, email, hash_token): # Unsubscribe directly + Let the user choose their subscriptions - mailing.update_opt_out(email, mailing.contact_list_ids.ids, True) + mailing.contact_list_ids._update_subscription_from_email(email, opt_out=True) contacts = request.env['mailing.contact'].sudo().search([('email_normalized', '=', tools.email_normalize(email))]) subscription_list_ids = contacts.mapped('subscription_list_ids') @@ -128,18 +135,28 @@ class MassMailController(http.Controller): @http.route('/mailing/list/update', type='json', auth='public', csrf=True) def mailing_update_list_subscription(self, mailing_id=None, document_id=None, email=None, hash_token=None, - opt_in_ids=None, opt_out_ids=None, **post): + lists_optin_ids=None, **post): try: - mailing_sudo = self._check_mailing_email_token(mailing_id, document_id, email, hash_token) + _mailing_sudo = self._check_mailing_email_token(mailing_id, document_id, email, hash_token) except BadRequest: return 'error' except (NotFound, Unauthorized): return 'unauthorized' - if opt_in_ids: - mailing_sudo.update_opt_out(email, opt_in_ids, False) - if opt_out_ids: - mailing_sudo.update_opt_out(email, opt_out_ids, True) + contacts = self._fetch_contacts(email) + lists_optin = request.env['mailing.list'].sudo().browse(lists_optin_ids or []).exists() + # opt-out all not chosen lists + lists_to_optout = contacts.subscription_list_ids.filtered( + lambda sub: not sub.opt_out and sub.list_id not in lists_optin + ).list_id + # opt-in in either already member, either public (to avoid trying to opt-in + # in private lists) + lists_to_optin = lists_optin.filtered( + lambda mlist: mlist.is_public or mlist in contacts.lists_ids + ) + lists_to_optout._update_subscription_from_email(email, opt_out=True) + lists_to_optin._update_subscription_from_email(email, opt_out=False) + return True @http.route('/mailing/feedback', type='json', auth='public', csrf=True) diff --git a/addons/mass_mailing/models/mailing.py b/addons/mass_mailing/models/mailing.py index 2fd89771c74..d08bf3454e5 100644 --- a/addons/mass_mailing/models/mailing.py +++ b/addons/mass_mailing/models/mailing.py @@ -785,25 +785,6 @@ class MassMailing(models.Model): return values_list - def update_opt_out(self, email, list_ids, value): - if len(list_ids) > 0: - model = self.env['mailing.contact'].with_context(active_test=False) - records = model.search([('email_normalized', '=', tools.email_normalize(email))]) - opt_out_records = self.env['mailing.contact.subscription'].search([ - ('contact_id', 'in', records.ids), - ('list_id', 'in', list_ids), - ('opt_out', '!=', value) - ]) - - opt_out_records.write({'opt_out': value}) - message = Markup(_('The recipient unsubscribed from %s mailing list(s)')) \ - if value else Markup(_('The recipient subscribed to %s mailing list(s)')) - for record in records: - # filter the list_id by record - record_lists = opt_out_records.filtered(lambda rec: rec.contact_id.id == record.id) - if len(record_lists) > 0: - record.sudo().message_post(body=message % ', '.join(str(list.name) for list in record_lists.mapped('list_id'))) - # ------------------------------------------------------ # A/B Test # ------------------------------------------------------ diff --git a/addons/mass_mailing/models/mailing_list.py b/addons/mass_mailing/models/mailing_list.py index 7e634f35c04..a850c67332b 100644 --- a/addons/mass_mailing/models/mailing_list.py +++ b/addons/mass_mailing/models/mailing_list.py @@ -1,7 +1,9 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import _, api, Command, fields, models +from markupsafe import Markup + +from odoo import _, api, Command, fields, models, tools from odoo.exceptions import UserError @@ -267,6 +269,91 @@ class MassMailingList(models.Model): def close_dialog(self): return {'type': 'ir.actions.act_window_close'} + # ------------------------------------------------------ + # SUBSCRIPTION MANAGEMENT + # ------------------------------------------------------ + + def _update_subscription_from_email(self, email, opt_out=True, force_message=None): + """ When opting-out: we have to switch opted-in subscriptions. We don't + need to create subscription for other lists as opt-out = not being a + member. + + When opting-in: we have to switch opted-out subscriptions and create + subscription for other mailing lists id they are public. Indeed a + contact is opted-in when being subscribed in a mailing list. + + :param str email: email address that should opt-in or opt-out from + mailing lists; + :param boolean opt_out: if True, opt-out from lists given by self if + 'email' is member of it. If False, opt-in in lists givben by self + and create membership if not already member; + :param str force_message: if given, post a note using that body on + contact instead of generated update message. Give False to entirely + skip the note step; + """ + email_normalized = tools.email_normalize(email) + if not self or not email_normalized: + return + + contacts = self.env['mailing.contact'].with_context(active_test=False).search( + [('email_normalized', '=', email_normalized)] + ) + if not contacts: + return + + # switch opted-in subscriptions + if opt_out: + current_opt_in = contacts.subscription_list_ids.filtered( + lambda sub: not sub.opt_out and sub.list_id in self + ) + if current_opt_in: + current_opt_in.write({'opt_out': True}) + # switch opted-out subscription and create missing subscriptions + else: + subscriptions = contacts.subscription_list_ids.filtered(lambda sub: sub.list_id in self) + current_opt_out = subscriptions.filtered('opt_out') + if current_opt_out: + current_opt_out.write({'opt_out': False}) + + # create a subscription (for a single contact) for missing lists + missing_lists = self - subscriptions.list_id + if missing_lists: + self.env['mailing.contact.subscription'].create([ + {'contact_id': contacts[0].id, + 'list_id': mailing_list.id} + for mailing_list in missing_lists + ]) + + for contact in contacts: + # do not log if no opt-out / opt-in was actually done + if opt_out: + updated = current_opt_in.filtered(lambda sub: sub.contact_id == contact).list_id + else: + updated = current_opt_out.filtered(lambda sub: sub.contact_id == contact).list_id + missing_lists + if not updated: + continue + + if force_message is False: + continue + if force_message: + body = force_message + elif opt_out: + body = Markup('

%s

') % _( + '%(contact_name)s unsubscribed from the following mailing list(s)', + contact_name=contact.display_name + ) + body += Markup('' % ''.join(Markup('
  • %s
    • ') % name for name in updated.mapped('name'))) + else: + body = Markup('

    %s

    ') % _( + '%(contact_name)s subscribed to the following mailing list(s)', + contact_name=contact.display_name + ) + body += Markup('' % ''.join(Markup('
  • %s
    • ') % name for name in updated.mapped('name'))) + contact.with_context(mail_create_nosubscribe=True).message_post( + body=body, + subtype_id=self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note'), + ) + # ------------------------------------------------------ # MAILING # ------------------------------------------------------ diff --git a/addons/mass_mailing/static/src/js/mailing_portal_subscription_form.js b/addons/mass_mailing/static/src/js/mailing_portal_subscription_form.js index d056c63e2f3..f09f4f6473d 100644 --- a/addons/mass_mailing/static/src/js/mailing_portal_subscription_form.js +++ b/addons/mass_mailing/static/src/js/mailing_portal_subscription_form.js @@ -32,8 +32,8 @@ publicWidget.registry.MailingPortalSubscriptionForm = publicWidget.Widget.extend document_id: this.customerData.documentId, email: this.customerData.email, hash_token: this.customerData.hashToken, + lists_optin_ids: mailingListIds, mailing_id: this.customerData.mailingId, - opt_in_ids: mailingListIds, } ).then((result) => { this.trigger_up( diff --git a/addons/mass_mailing/tests/common.py b/addons/mass_mailing/tests/common.py index b6857d8c10d..f67df385eac 100644 --- a/addons/mass_mailing/tests/common.py +++ b/addons/mass_mailing/tests/common.py @@ -326,7 +326,10 @@ class MassMailCommon(MailCommon, MassMailCase): return cls.env['mailing.list'].with_context(cls._test_context).create({ 'name': 'Test List', 'contact_ids': [ - (0, 0, {'name': 'Contact %s' % i, 'email': 'contact%s@example.com' % i}) - for i in range(contacts_nbr) + (0, 0, { + 'name': f'Contact %{idx}', + 'email': f'contact%{idx}@example.com' + }) + for idx in range(contacts_nbr) ], }) diff --git a/addons/mass_mailing/tests/test_mailing_controllers.py b/addons/mass_mailing/tests/test_mailing_controllers.py index f8789c1139f..e3d2483c33b 100644 --- a/addons/mass_mailing/tests/test_mailing_controllers.py +++ b/addons/mass_mailing/tests/test_mailing_controllers.py @@ -278,7 +278,7 @@ class TestMailingControllers(TestMailingControllersCommon): message_unsub = contact_l1.message_ids[1] self.assertEqual( message_unsub.body, - Markup(f'

    The recipient unsubscribed from {self.mailing_list_1.name} mailing list(s)

    ') + Markup(f'

    {contact_l1.display_name} unsubscribed from the following mailing list(s)

    ') ) # posted messages on exclusion list record: activated, deactivated, activated again @@ -346,14 +346,14 @@ class TestMailingControllers(TestMailingControllersCommon): message_unsub = contact_l1.message_ids[1] self.assertEqual( message_unsub.body, - Markup(f'

    The recipient unsubscribed from {self.mailing_list_1.name} mailing list(s)

    ') + Markup(f'

    {contact_l1.display_name} unsubscribed from the following mailing list(s)

    ') ) # posted messages on contact record for mailing list 3: subscription message_unsub = contact_l3.message_ids[0] self.assertEqual( message_unsub.body, - Markup(f'

    The recipient subscribed to {self.mailing_list_3.name} mailing list(s)

    ') + Markup(f'

    {contact_l1.display_name} subscribed to the following mailing list(s)

    ') ) # posted messages on exclusion list record: activated, deactivated, activated again diff --git a/addons/mass_mailing/tests/test_mailing_list.py b/addons/mass_mailing/tests/test_mailing_list.py index 53514b40f35..a0f7be5a79c 100644 --- a/addons/mass_mailing/tests/test_mailing_list.py +++ b/addons/mass_mailing/tests/test_mailing_list.py @@ -274,3 +274,49 @@ class TestMailingContactImport(MassMailCommon): contact = self.env['mailing.contact'].search([('email', '=', 'already_exists_list_1@example.com')]) self.assertEqual(len(contact), 1, 'Should have updated the existing contact instead of creating a new one') + + +@tagged('mailing_list') +class TestSubscriptionManagement(MassMailCommon): + + @users('user_marketing') + def test_mailing_update_optout(self): + _email_formatted = '"Mireille Labeille" ' + _email_formatted_upd = '"Mireille Oreille-Labeille" ' + _email_normalized = 'mireille@test.example.com' + self._create_mailing_list() + ml_1, ml_2 = self.mailing_list_1.with_env(self.env), self.mailing_list_2.with_env(self.env) + ml_3 = self._create_mailing_list_of_x_contacts(3) + self.assertEqual(ml_1.contact_count, 3) + self.assertEqual(ml_1.contact_count_blacklisted, 0) + self.assertEqual(ml_1.contact_count_email, 3) + self.assertEqual(ml_1.contact_count_opt_out, 0) + self.assertEqual(ml_2.contact_count, 4) + self.assertEqual(ml_2.contact_count_blacklisted, 0) + self.assertEqual(ml_2.contact_count_email, 4) + self.assertEqual(ml_2.contact_count_opt_out, 0) + self.assertEqual(ml_3.contact_count, 3) + self.assertEqual(ml_3.contact_count_blacklisted, 0) + self.assertEqual(ml_3.contact_count_email, 3) + self.assertEqual(ml_3.contact_count_opt_out, 0) + + # create a new test contact + contact = self.env['mailing.contact'].browse( + self.env['mailing.contact'].name_create(_email_formatted)[0] + ) + self.assertEqual(contact.email, _email_normalized) + self.assertEqual(contact.name, 'Mireille Labeille') + + # add new subscriptions (and ensure email_normalized is used) + (ml_1 + ml_2)._update_subscription_from_email(_email_formatted_upd, opt_out=False) + subs = self.env['mailing.contact.subscription'].search( + [('contact_id', '=', contact.id)] + ) + self.assertEqual(subs.list_id, ml_1 + ml_2) + + # opt-out from opted-in mailing list + 1 non opted-in mailing list + (ml_2 + ml_3)._update_subscription_from_email(_email_formatted_upd, opt_out=True) + subs = self.env['mailing.contact.subscription'].search( + [('contact_id', '=', contact.id)] + ) + self.assertEqual(subs.list_id, ml_1 + ml_2)