From 6627fc8b4a6588acaad82f65816b4aa7dbb87ad2 Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Thu, 10 May 2018 11:17:04 +0100 Subject: [PATCH] [IMP] mass_mailing_custom_unsubscribe: GDPR compliance (#267) * [IMP] mass_mailing_custom_unsubscribe: GDPR compliance - Record resubscriptions too. - Record action metadata. - Make ESLint happy. - Quick color-based action distinction in tree view. - Add useful quick groupings. - Display (un)subscription metadata. - Pivot & graph views. --- mass_mailing_custom_unsubscribe/README.rst | 5 +- .../__manifest__.py | 4 +- .../controllers/main.py | 21 +++++-- .../data/mail_unsubscription_reason.xml | 2 - mass_mailing_custom_unsubscribe/exceptions.py | 4 ++ .../models/mail_mass_mailing.py | 18 ++++-- .../models/mail_unsubscription.py | 57 +++++++++++++++++-- .../static/src/js/require_details.js | 7 ++- .../static/src/js/unsubscribe.js | 32 +++++++---- .../tests/test_unsubscription.py | 14 ++++- .../views/mail_unsubscription_view.xml | 41 ++++++++++++- 11 files changed, 168 insertions(+), 37 deletions(-) diff --git a/mass_mailing_custom_unsubscribe/README.rst b/mass_mailing_custom_unsubscribe/README.rst index 14efcbcd..568d6530 100644 --- a/mass_mailing_custom_unsubscribe/README.rst +++ b/mass_mailing_custom_unsubscribe/README.rst @@ -10,7 +10,10 @@ This addon extends the unsubscription form to let you: - Choose which mailing lists are not cross-unsubscriptable when unsubscribing from a different one. -- Know why and when a contact has been unsubscribed from a mass mailing. +- Know why and when a contact has been subscribed or unsubscribed from a + mass mailing. +- Provide proof on why you are sending mass mailings to a given contact, as + required by the GDPR in Europe. Configuration ============= diff --git a/mass_mailing_custom_unsubscribe/__manifest__.py b/mass_mailing_custom_unsubscribe/__manifest__.py index c533f2c1..d8bbe90f 100644 --- a/mass_mailing_custom_unsubscribe/__manifest__.py +++ b/mass_mailing_custom_unsubscribe/__manifest__.py @@ -3,9 +3,9 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). { 'name': "Customizable unsubscription process on mass mailing emails", - "summary": "Know unsubscription reasons, track them", + "summary": "Know and track (un)subscription reasons, GDPR compliant", 'category': 'Marketing', - 'version': '10.0.1.0.0', + 'version': '10.0.2.0.0', 'depends': [ 'website_mass_mailing', ], diff --git a/mass_mailing_custom_unsubscribe/controllers/main.py b/mass_mailing_custom_unsubscribe/controllers/main.py index 403efeb9..3b215722 100644 --- a/mass_mailing_custom_unsubscribe/controllers/main.py +++ b/mass_mailing_custom_unsubscribe/controllers/main.py @@ -45,6 +45,8 @@ class CustomUnsubscribe(MassMailController): _logger.debug( "Called `mailing()` with: %r", (mailing_id, email, res_id, token, post)) + if res_id: + res_id = int(res_id) mailing = request.env["mail.mass_mailing"].sudo().browse(mailing_id) mailing._unsubscribe_token(res_id, token) # Mass mailing list contacts are a special case because they have a @@ -90,12 +92,21 @@ class CustomUnsubscribe(MassMailController): token, reason_id=None, details=None): """Store unsubscription reasons when unsubscribing from RPC.""" # Update request context and reset environment + environ = request.httprequest.headers.environ + extra_context = { + "default_metadata": "\n".join( + "%s: %s" % (val, environ.get(val)) for val in ( + "REMOTE_ADDR", + "HTTP_USER_AGENT", + "HTTP_ACCEPT_LANGUAGE", + ) + ), + } if reason_id: - request.context = dict( - request.context, - default_reason_id=int(reason_id), - default_details=details or False, - ) + extra_context["default_reason_id"] = int(reason_id) + if details: + extra_context["default_details"] = details + request.context = dict(request.context, **extra_context) # FIXME Remove token check in version where this is merged: # https://github.com/odoo/odoo/pull/14385 mailing = request.env['mail.mass_mailing'].sudo().browse(mailing_id) diff --git a/mass_mailing_custom_unsubscribe/data/mail_unsubscription_reason.xml b/mass_mailing_custom_unsubscribe/data/mail_unsubscription_reason.xml index 4335f9b7..24a6eda3 100644 --- a/mass_mailing_custom_unsubscribe/data/mail_unsubscription_reason.xml +++ b/mass_mailing_custom_unsubscribe/data/mail_unsubscription_reason.xml @@ -2,7 +2,6 @@ - - diff --git a/mass_mailing_custom_unsubscribe/exceptions.py b/mass_mailing_custom_unsubscribe/exceptions.py index 195b6198..bc9bbcc4 100644 --- a/mass_mailing_custom_unsubscribe/exceptions.py +++ b/mass_mailing_custom_unsubscribe/exceptions.py @@ -7,3 +7,7 @@ from openerp import exceptions class DetailsRequiredError(exceptions.ValidationError): pass + + +class ReasonRequiredError(exceptions.ValidationError): + pass diff --git a/mass_mailing_custom_unsubscribe/models/mail_mass_mailing.py b/mass_mailing_custom_unsubscribe/models/mail_mass_mailing.py index 62fcf04d..2eec3d4a 100644 --- a/mass_mailing_custom_unsubscribe/models/mail_mass_mailing.py +++ b/mass_mailing_custom_unsubscribe/models/mail_mass_mailing.py @@ -40,14 +40,24 @@ class MailMassMailing(models.Model): def update_opt_out(self, email, res_ids, value): """Save unsubscription reason when opting out from mailing.""" self.ensure_one() - if value and self.env.context.get("default_reason_id"): - for res_id in res_ids: + action = "unsubscription" if value else "subscription" + records = self.env[self.mailing_model].browse(res_ids) + previous = self.env["mail.unsubscription"].search(limit=1, args=[ + ("mass_mailing_id", "=", self.id), + ("email", "=", email), + ("action", "=", action), + ]) + for one in records: + # Store action only when something changed, or there was no + # previous subscription record + if one.opt_out != value or (action == "subscription" and + not previous): # reason_id and details are expected from the context self.env["mail.unsubscription"].create({ "email": email, "mass_mailing_id": self.id, - "unsubscriber_id": "%s,%d" % ( - self.mailing_model, int(res_id)), + "unsubscriber_id": "%s,%d" % (one._name, one.id), + "action": action, }) return super(MailMassMailing, self).update_opt_out( email, res_ids, value) diff --git a/mass_mailing_custom_unsubscribe/models/mail_unsubscription.py b/mass_mailing_custom_unsubscribe/models/mail_unsubscription.py index 83999bcb..148ae9da 100644 --- a/mass_mailing_custom_unsubscribe/models/mail_unsubscription.py +++ b/mass_mailing_custom_unsubscribe/models/mail_unsubscription.py @@ -2,7 +2,7 @@ # Copyright 2016 Jairo Llopis # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -from openerp import _, api, fields, models +from odoo import _, api, fields, models from .. import exceptions @@ -10,12 +10,22 @@ class MailUnsubscription(models.Model): _name = "mail.unsubscription" _inherit = "mail.thread" _rec_name = "date" + _order = "date DESC" date = fields.Datetime( default=lambda self: self._default_date(), required=True) email = fields.Char( required=True) + action = fields.Selection( + selection=[ + ("subscription", "Subscription"), + ("unsubscription", "Unsubscription"), + ], + required=True, + default="unsubscription", + help="What did the (un)subscriber choose to do.", + ) mass_mailing_id = fields.Many2one( "mail.mass_mailing", "Mass mailing", @@ -23,19 +33,29 @@ class MailUnsubscription(models.Model): help="Mass mailing from which he was unsubscribed.") unsubscriber_id = fields.Reference( lambda self: self._selection_unsubscriber_id(), - "Unsubscriber", - required=True, - help="Who was unsubscribed.") + "(Un)subscriber", + help="Who was subscribed or unsubscribed.") + mailing_list_id = fields.Many2one( + "mail.mass_mailing.list", + "Mailing list", + ondelete="set null", + compute="_compute_mailing_list_id", + store=True, + help="(Un)subscribed mass mailing list, if any.", + ) reason_id = fields.Many2one( "mail.unsubscription.reason", "Reason", ondelete="restrict", - required=True, help="Why the unsubscription was made.") details = fields.Char( help="More details on why the unsubscription was made.") details_required = fields.Boolean( related="reason_id.details_required") + metadata = fields.Text( + readonly=True, + help="HTTP request metadata used when creating this record.", + ) @api.model def _default_date(self): @@ -46,6 +66,15 @@ class MailUnsubscription(models.Model): """Models that can be linked to a ``mail.mass_mailing``.""" return self.env["mail.mass_mailing"]._get_mailing_model() + @api.multi + @api.constrains("action", "reason_id") + def _check_reason_needed(self): + """Ensure reason is given for unsubscriptions.""" + for one in self: + if one.action == "unsubscription" and not one.reason_id: + raise exceptions.ReasonRequiredError( + _("Please indicate why are you unsubscribing.")) + @api.multi @api.constrains("details", "reason_id") def _check_details_needed(self): @@ -55,6 +84,24 @@ class MailUnsubscription(models.Model): raise exceptions.DetailsRequiredError( _("Please provide details on why you are unsubscribing.")) + @api.multi + @api.depends("unsubscriber_id") + def _compute_mailing_list_id(self): + """Get the mass mailing list, if it is possible.""" + for one in self: + try: + one.mailing_list_id = one.unsubscriber_id.list_id + except AttributeError: + # Possibly model != mail.mass_mailing.contact; no problem + pass + + @api.model + def create(self, vals): + # No reasons for subscriptions + if vals.get("action") == "subscription": + vals = dict(vals, reason_id=False, details=False) + return super(MailUnsubscription, self).create(vals) + class MailUnsubscriptionReason(models.Model): _name = "mail.unsubscription.reason" diff --git a/mass_mailing_custom_unsubscribe/static/src/js/require_details.js b/mass_mailing_custom_unsubscribe/static/src/js/require_details.js index 6df78b75..6c778c68 100644 --- a/mass_mailing_custom_unsubscribe/static/src/js/require_details.js +++ b/mass_mailing_custom_unsubscribe/static/src/js/require_details.js @@ -5,7 +5,7 @@ odoo.define("mass_mailing_custom_unsubscribe.require_details", "use strict"; var animation = require("web_editor.snippets.animation"); - return animation.registry.mass_mailing_custom_unsubscribe_require_details = + animation.registry.mass_mailing_custom_unsubscribe_require_details = animation.Class.extend({ selector: ".js_unsubscription_reason", @@ -19,7 +19,10 @@ odoo.define("mass_mailing_custom_unsubscribe.require_details", toggle: function (event) { this.$details.prop( "required", - $(event.target).is("[data-details-required]")); + $(event.target).is("[data-details-required]") && + $(event.target).is(":visible")); }, }); + + return animation.registry.mass_mailing_custom_unsubscribe_require_details; }); diff --git a/mass_mailing_custom_unsubscribe/static/src/js/unsubscribe.js b/mass_mailing_custom_unsubscribe/static/src/js/unsubscribe.js index 49f92b6d..12ec3a05 100644 --- a/mass_mailing_custom_unsubscribe/static/src/js/unsubscribe.js +++ b/mass_mailing_custom_unsubscribe/static/src/js/unsubscribe.js @@ -7,15 +7,15 @@ * that when it gets merged, and remove most of this file. */ odoo.define("mass_mailing_custom_unsubscribe.unsubscribe", function (require) { "use strict"; - var core = require("web.core"), - ajax = require("web.ajax"), - animation = require("web_editor.snippets.animation"), - _t = core._t; + var core = require("web.core"); + var ajax = require("web.ajax"); + var animation = require("web_editor.snippets.animation"); + var _t = core._t; - return animation.registry.mass_mailing_unsubscribe = + animation.registry.mass_mailing_unsubscribe = animation.Class.extend({ selector: "#unsubscribe_form", - start: function (editable_mode) { + start: function () { this.controller = '/mail/mailing/unsubscribe'; this.$alert = this.$(".alert"); this.$email = this.$("input[name='email']"); @@ -32,7 +32,7 @@ odoo.define("mass_mailing_custom_unsubscribe.unsubscribe", function (require) { // Helper to get list ids, to use in this.$contacts.map() int_val: function (index, element) { - return parseInt($(element).val()); + return parseInt($(element).val(), 10); }, // Get a filtered array of integer IDs of matching lists @@ -50,11 +50,17 @@ odoo.define("mass_mailing_custom_unsubscribe.unsubscribe", function (require) { }); // Hide reasons form if you are only subscribing this.$reasons.toggleClass("hidden", !$disabled.length); + var $radios = this.$reasons.find(":radio"); if (this.$reasons.is(":hidden")) { // Uncheck chosen reason - this.$reasons.find(":radio").prop("checked", false) + $radios.prop("checked", false) + // Unrequire specifying a reason + .prop("required", false) // Remove possible constraints for details .trigger("change"); + } else { + // Require specifying a reason + $radios.prop("required", true); } }, @@ -62,16 +68,18 @@ odoo.define("mass_mailing_custom_unsubscribe.unsubscribe", function (require) { values: function () { var result = { email: this.$email.val(), - mailing_id: parseInt(this.$mailing_id.val()), + mailing_id: parseInt(this.$mailing_id.val(), 10), opt_in_ids: this.contact_ids(true), opt_out_ids: this.contact_ids(false), - res_id: parseInt(this.$res_id.val()), + res_id: parseInt(this.$res_id.val(), 10), token: this.$token.val(), }; // Only send reason and details if an unsubscription was found if (this.$reasons.is(":visible")) { result.reason_id = parseInt( - this.$reasons.find("[name='reason_id']:checked").val()); + this.$reasons.find("[name='reason_id']:checked").val(), + 10 + ); result.details = this.$details.val(); } return result; @@ -108,4 +116,6 @@ odoo.define("mass_mailing_custom_unsubscribe.unsubscribe", function (require) { .addClass("alert-warning"); }, }); + + return animation.registry.mass_mailing_unsubscribe; }); diff --git a/mass_mailing_custom_unsubscribe/tests/test_unsubscription.py b/mass_mailing_custom_unsubscribe/tests/test_unsubscription.py index 06c47507..b6d7043a 100644 --- a/mass_mailing_custom_unsubscribe/tests/test_unsubscription.py +++ b/mass_mailing_custom_unsubscribe/tests/test_unsubscription.py @@ -2,11 +2,11 @@ # Copyright 2016 Jairo Llopis # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -from openerp.tests.common import TransactionCase +from openerp.tests.common import SavepointCase from .. import exceptions -class UnsubscriptionCase(TransactionCase): +class UnsubscriptionCase(SavepointCase): def test_details_required(self): """Cannot create unsubscription without details when required.""" with self.assertRaises(exceptions.DetailsRequiredError): @@ -19,3 +19,13 @@ class UnsubscriptionCase(TransactionCase): self.env.ref( "mass_mailing_custom_unsubscribe.reason_other").id, }) + + def test_reason_required(self): + """Cannot create unsubscription without reason when required.""" + with self.assertRaises(exceptions.ReasonRequiredError): + self.env["mail.unsubscription"].create({ + "email": "axelor@yourcompany.example.com", + "mass_mailing_id": self.env.ref("mass_mailing.mass_mail_1").id, + "unsubscriber_id": + "res.partner,%d" % self.env.ref("base.res_partner_2").id, + }) diff --git a/mass_mailing_custom_unsubscribe/views/mail_unsubscription_view.xml b/mass_mailing_custom_unsubscribe/views/mail_unsubscription_view.xml index da8f23ca..177b5598 100644 --- a/mass_mailing_custom_unsubscribe/views/mail_unsubscription_view.xml +++ b/mass_mailing_custom_unsubscribe/views/mail_unsubscription_view.xml @@ -14,11 +14,15 @@ + - + + +
@@ -36,11 +40,13 @@ Mail Unsubscription Tree mail.unsubscription - + + + @@ -54,6 +60,7 @@ + @@ -63,6 +70,10 @@ context="{'group_by': 'date:month'}"/> + + + + Mail Unsubscription Pivot + mail.unsubscription + + + + + + + + + + + Mail Unsubscription Graph + mail.unsubscription + + + + + + + +