From a4927a162e384d69ed6251730466e0e3879e469d Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Mon, 13 May 2019 18:13:10 +0100 Subject: [PATCH] [FIX] privacy_consent: Separate automated emails send process Before https://github.com/OCA/data-protection/pull/29 there was a race condition where an email could be sent while the same transaction that created the `privacy.consent` record still wasn't committed, producing a 404 error if the user clicked on "Accept" or "Reject" before all mails were sent. To avoid that, a raw `cr.commit()` was issued, but this produced another situation where the user had to wait until the full email queue is cleared to get his page loaded. It wasn't an error, but a long queue meant several minutes waiting, and it's ulikely that an average human is so patient. So, here's the final fix (I hope!). The main problem was that I was looking in the wrong place to send the email. It turns out that the `self.post_message_with_template()` method is absolutely helpless in the case at hand, where these criteria must be met: * E-mail must be enqueued, no matter if there are less or more than 50 consents to send. * The template must be processed per record. * In an ideal world, a `cr.commit()` must be issued after each sent mail. The metod that was being used: * Didn't allow to use `auto_commit` mode. * Only allowed to render the template per record if called with `composition_mode="mass_mail"`. * Only allowed to enqueue emails if called with `composition_mode="mass_post"`. Obviously, I cannot set 2 different values for `composition_mode`, so a different strategy had to be used. I discovered that the `mail.template` model has a helpful method called `send_mail()` that, by default: * Renders the template per record * Enqueues the email * The email queue is cleared in `auto_commit=True` mode. So, from now on, problems are gone: * The user click, or the cron run, will just generate the missing `privacy.consent` records and enqueue mails for them. * The mail queue manager will send them later, in `auto_commit` mode. * After sending the e-mail, this module will set the `privacy.consent` record as `sent`. * Thanks to *not* sending the email, the process the user faces when he hits the "generate" button is faster. * Instructions in the README and text in the "generate" button are updated to reflect this new behavior. * Thanks to the `auto_commit` feature, if Odoo is rebooted in the middle of a mail queue clearance, the records that were sent remain properly marked as sent, and the missing mails will be sent after the next boot. * No hardcoded commits. * No locked transactions. * BTW I discovered that 2 different emails were created when creating a new consent. I started using `mail_create_nolog=True` to avoid that problem and only log a single creation message. Note to self: never use again `post_message_with_template()`. --- privacy_consent/README.rst | 4 +-- privacy_consent/data/mail.xml | 1 + privacy_consent/i18n/es.po | 30 +++++++++++-------- privacy_consent/i18n/privacy_consent.pot | 13 ++++++-- privacy_consent/models/privacy_activity.py | 3 -- privacy_consent/models/privacy_consent.py | 18 ++--------- privacy_consent/readme/USAGE.rst | 4 +-- privacy_consent/static/description/index.html | 4 +-- privacy_consent/tests/test_consent.py | 14 +++++++-- privacy_consent/views/privacy_activity.xml | 4 +-- 10 files changed, 50 insertions(+), 45 deletions(-) diff --git a/privacy_consent/README.rst b/privacy_consent/README.rst index 4f92832..c15174b 100644 --- a/privacy_consent/README.rst +++ b/privacy_consent/README.rst @@ -106,8 +106,8 @@ New options for data processing activities: * If you chose *Manual* mode, all missing consent request are created as drafts, and nothing else is done now. - * If you chose *Automatic* mode, also those requests are sent to subjects - and set as *Sent*. + * If you chose *Automatic* mode, also those request e-mails are enqueued + and, when the mail queue is cleared, they will be set as *Sent*. #. You will be presented with the list of just-created consent requests. See below. diff --git a/privacy_consent/data/mail.xml b/privacy_consent/data/mail.xml index d7dad6c..f4ab84e 100644 --- a/privacy_consent/data/mail.xml +++ b/privacy_consent/data/mail.xml @@ -7,6 +7,7 @@ + Personal data processing consent request Data processing consent request for ${object.activity_id.display_name|safe} diff --git a/privacy_consent/i18n/es.po b/privacy_consent/i18n/es.po index 6bc168a..5c47d5f 100644 --- a/privacy_consent/i18n/es.po +++ b/privacy_consent/i18n/es.po @@ -6,8 +6,8 @@ msgid "" msgstr "" "Project-Id-Version: Odoo Server 10.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2018-07-11 08:38+0000\n" -"PO-Revision-Date: 2018-07-11 11:07+0200\n" +"POT-Creation-Date: 2019-05-13 17:04+0000\n" +"PO-Revision-Date: 2019-05-13 18:08+0100\n" "Last-Translator: Jairo Llopis \n" "Language-Team: \n" "Language: es_ES\n" @@ -15,7 +15,7 @@ msgstr "" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1);\n" -"X-Generator: Poedit 2.0.8\n" +"X-Generator: Poedit 2.2.1\n" #. module: privacy_consent #: model:mail.template,body_html:privacy_consent.template_consent @@ -372,6 +372,11 @@ msgstr "Contacto duplicado en esta actividad de tratamiento" msgid "Email Templates" msgstr "Plantillas de correo electrónico" +#. module: privacy_consent +#: model:ir.model,name:privacy_consent.model_mail_compose_message +msgid "Email composition wizard" +msgstr "Asistente de redacción de correo electrónico" + #. module: privacy_consent #: model:ir.model.fields,field_description:privacy_consent.field_privacy_activity_consent_template_id msgid "Email template" @@ -394,12 +399,14 @@ msgid "" "Enable if you need to track any kind of consent from the affected subjects" msgstr "" "Actívelo si necesita registrar cualquier tipo de consentimiento de los " -"interesados." +"interesados" #. module: privacy_consent #: model:ir.ui.view,arch_db:privacy_consent.activity_form -msgid "Generate and send missing consent requests" -msgstr "Generar y enviar solicitudes de consentimiento faltantes" +msgid "Generate and enqueue missing consent requests" +msgstr "" +"Generar y colocar en la bandeja de salida las solicitudes de consentimiento " +"que falten" #. module: privacy_consent #: model:ir.ui.view,arch_db:privacy_consent.activity_form @@ -407,7 +414,7 @@ msgid "Generate missing draft consent requests" msgstr "Generar borradores de las solicitudes de consentimiento faltantes" #. module: privacy_consent -#: code:addons/privacy_consent/models/privacy_activity.py:142 +#: code:addons/privacy_consent/models/privacy_activity.py:139 #, python-format msgid "Generated consents" msgstr "Consentimientos generados" @@ -623,10 +630,10 @@ msgstr "Gracias por su respuesta." #. module: privacy_consent #: model:ir.ui.view,arch_db:privacy_consent.activity_form -msgid "This could send many consent emails, are you sure to proceed?" +msgid "This could enqueue many consent emails, are you sure to proceed?" msgstr "" -"Esto podría enviar muchos correos electrónicos solicitando consentimiento " -"para el tratamiento de datos, ¿seguro que quiere continuar?" +"Esto podría poner en la cola muchos correos electrónicos solicitando " +"consentimiento para el tratamiento de datos, ¿seguro que quiere continuar?" #. module: privacy_consent #: model:ir.actions.server,name:privacy_consent.update_opt_out @@ -656,6 +663,3 @@ msgstr "Ha rechazado dicho tratamiento." #: model:ir.ui.view,arch_db:privacy_consent.form msgid "You have accepted such processing." msgstr "Ha aceptado dicho tratamiento." - -#~ msgid "Email composition wizard" -#~ msgstr "Asistente de redacción de correo electrónico" diff --git a/privacy_consent/i18n/privacy_consent.pot b/privacy_consent/i18n/privacy_consent.pot index 275b3d2..cf3cd1a 100644 --- a/privacy_consent/i18n/privacy_consent.pot +++ b/privacy_consent/i18n/privacy_consent.pot @@ -6,6 +6,8 @@ msgid "" msgstr "" "Project-Id-Version: Odoo Server 10.0\n" "Report-Msgid-Bugs-To: \n" +"POT-Creation-Date: 2019-05-13 17:04+0000\n" +"PO-Revision-Date: 2019-05-13 17:04+0000\n" "Last-Translator: <>\n" "Language-Team: \n" "MIME-Version: 1.0\n" @@ -237,6 +239,11 @@ msgstr "" msgid "Email Templates" msgstr "" +#. module: privacy_consent +#: model:ir.model,name:privacy_consent.model_mail_compose_message +msgid "Email composition wizard" +msgstr "" + #. module: privacy_consent #: model:ir.model.fields,field_description:privacy_consent.field_privacy_activity_consent_template_id msgid "Email template" @@ -254,7 +261,7 @@ msgstr "" #. module: privacy_consent #: model:ir.ui.view,arch_db:privacy_consent.activity_form -msgid "Generate and send missing consent requests" +msgid "Generate and enqueue missing consent requests" msgstr "" #. module: privacy_consent @@ -263,7 +270,7 @@ msgid "Generate missing draft consent requests" msgstr "" #. module: privacy_consent -#: code:addons/privacy_consent/models/privacy_activity.py:142 +#: code:addons/privacy_consent/models/privacy_activity.py:139 #, python-format msgid "Generated consents" msgstr "" @@ -452,7 +459,7 @@ msgstr "" #. module: privacy_consent #: model:ir.ui.view,arch_db:privacy_consent.activity_form -msgid "This could send many consent emails, are you sure to proceed?" +msgid "This could enqueue many consent emails, are you sure to proceed?" msgstr "" #. module: privacy_consent diff --git a/privacy_consent/models/privacy_activity.py b/privacy_consent/models/privacy_activity.py index 6a37e53..578f2a7 100644 --- a/privacy_consent/models/privacy_activity.py +++ b/privacy_consent/models/privacy_activity.py @@ -131,9 +131,6 @@ class PrivacyActivity(models.Model): "accepted": one.default_consent, "activity_id": one.id, }) - # Avoid race condition where a user could click on accept/reject link - # in his email before its consent record is saved to the database - consents.env.cr.commit() # Send consent request emails for automatic activitys consents.action_auto_ask() # Redirect user to new consent requests generated diff --git a/privacy_consent/models/privacy_consent.py b/privacy_consent/models/privacy_consent.py index fd90678..866d4fb 100644 --- a/privacy_consent/models/privacy_consent.py +++ b/privacy_consent/models/privacy_consent.py @@ -101,23 +101,10 @@ class PrivacyConsent(models.Model): def _send_consent_notification(self): """Send email notification to subject.""" - consents_by_template = {} for one in self.with_context(tpl_force_default_to=True, mail_notify_user_signature=False, mail_auto_subscribe_no_notify=True): - # Group consents by template, to send in batch where possible - template_id = one.activity_id.consent_template_id.id - consents_by_template.setdefault(template_id, one) - consents_by_template[template_id] |= one - # Send emails - for template_id, consents in consents_by_template.items(): - consents.message_post_with_template( - template_id, - # This mode always sends email, regardless of partner's - # notification preferences; we use it here because it's very - # likely that we are asking authorisation to send emails - composition_mode="mass_mail", - ) + one.activity_id.consent_template_id.send_mail(one.id) def _run_action(self): """Execute server action defined in data processing activity.""" @@ -135,7 +122,8 @@ class PrivacyConsent(models.Model): @api.model def create(self, vals): """Run server action on create.""" - result = super(PrivacyConsent, self).create(vals) + result = super(PrivacyConsent, + self.with_context(mail_create_nolog=True)).create(vals) # Sync the default acceptance status result.sudo()._run_action() return result diff --git a/privacy_consent/readme/USAGE.rst b/privacy_consent/readme/USAGE.rst index 68d4aec..e621578 100644 --- a/privacy_consent/readme/USAGE.rst +++ b/privacy_consent/readme/USAGE.rst @@ -44,8 +44,8 @@ New options for data processing activities: * If you chose *Manual* mode, all missing consent request are created as drafts, and nothing else is done now. - * If you chose *Automatic* mode, also those requests are sent to subjects - and set as *Sent*. + * If you chose *Automatic* mode, also those request e-mails are enqueued + and, when the mail queue is cleared, they will be set as *Sent*. #. You will be presented with the list of just-created consent requests. See below. diff --git a/privacy_consent/static/description/index.html b/privacy_consent/static/description/index.html index 58af3e3..058144a 100644 --- a/privacy_consent/static/description/index.html +++ b/privacy_consent/static/description/index.html @@ -456,8 +456,8 @@ partner’s Elegible for mass mailings option.

  • If you chose Manual mode, all missing consent request are created as drafts, and nothing else is done now.
    • -
  • If you chose Automatic mode, also those requests are sent to subjects -and set as Sent.
    • +
  • If you chose Automatic mode, also those request e-mails are enqueued +and, when the mail queue is cleared, they will be set as Sent.

  • You will be presented with the list of just-created consent requests. diff --git a/privacy_consent/tests/test_consent.py b/privacy_consent/tests/test_consent.py index 6a68e4f..4c6f11c 100644 --- a/privacy_consent/tests/test_consent.py +++ b/privacy_consent/tests/test_consent.py @@ -17,6 +17,8 @@ class ActivityCase(HttpCase): self.env = self._oldenv(self.cursor()) # HACK end self.cron = self.env.ref("privacy_consent.cron_auto_consent") + self.cron_mail_queue = self.env.ref( + "mail.ir_cron_mail_scheduler_action") self.update_opt_out = self.env.ref("privacy_consent.update_opt_out") self.mt_consent_consent_new = self.env.ref( "privacy_consent.mt_consent_consent_new") @@ -93,11 +95,17 @@ class ActivityCase(HttpCase): consents = self.env["privacy.consent"].search([ ("activity_id", "=", self.activity_auto.id), ]) + # Check pending mails + for consent in consents: + self.assertEqual(consent.state, "draft") + messages = consent.message_ids + self.assertEqual(len(messages), 2) # Check sent mails + self.cron_mail_queue.method_direct_trigger() for consent in consents: self.assertEqual(consent.state, "sent") - messages = consent.mapped("message_ids") - self.assertEqual(len(messages), 4) + messages = consent.message_ids + self.assertEqual(len(messages), 3) # 2nd message notifies creation self.assertEqual( messages[2].subtype_id, @@ -167,7 +175,7 @@ class ActivityCase(HttpCase): self.assertEqual(consents.mapped("last_metadata"), [False] * 2) # Check sent mails messages = consents.mapped("message_ids") - self.assertEqual(len(messages), 4) + self.assertEqual(len(messages), 2) subtypes = messages.mapped("subtype_id") self.assertTrue(subtypes & self.mt_consent_consent_new) self.assertFalse(subtypes & self.mt_consent_acceptance_changed) diff --git a/privacy_consent/views/privacy_activity.xml b/privacy_consent/views/privacy_activity.xml index 35c9e5f..b13bce2 100644 --- a/privacy_consent/views/privacy_activity.xml +++ b/privacy_consent/views/privacy_activity.xml @@ -46,8 +46,8 @@ icon="fa-user-plus" name="action_new_consents" type="object" - string="Generate and send missing consent requests" - confirm="This could send many consent emails, are you sure to proceed?" + string="Generate and enqueue missing consent requests" + confirm="This could enqueue many consent emails, are you sure to proceed?" />