Skip to content

Commit

Permalink
privacy_consent: Separate automated emails send process
Browse files Browse the repository at this point in the history
Before OCA#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()`.
  • Loading branch information
yajo authored and fkantelberg committed Jul 4, 2019
1 parent dc1e2b6 commit e3948fb
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 45 deletions.
4 changes: 2 additions & 2 deletions privacy_consent/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions privacy_consent/data/mail.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

<!-- Mail templates -->
<record id="template_consent" model="mail.template">
<field name="auto_delete" eval="False"/>
<field name="name">Personal data processing consent request</field>
<field name="subject">Data processing consent request for ${object.activity_id.display_name|safe}</field>
<field name="model_id" ref="model_privacy_consent"/>
Expand Down
30 changes: 17 additions & 13 deletions privacy_consent/i18n/es.po
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ 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 <yajo.sk8@gmail.com>\n"
"Language-Team: \n"
"Language: es_ES\n"
"MIME-Version: 1.0\n"
"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
Expand Down Expand Up @@ -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"
Expand All @@ -394,20 +399,22 @@ 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
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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -656,6 +663,3 @@ msgstr "Ha <b class=\"text-danger\">rechazado</b> dicho tratamiento."
#: model:ir.ui.view,arch_db:privacy_consent.form
msgid "You have <b class=\"text-success\">accepted</b> such processing."
msgstr "Ha <b class=\"text-success\">aceptado</b> dicho tratamiento."

#~ msgid "Email composition wizard"
#~ msgstr "Asistente de redacción de correo electrónico"
13 changes: 10 additions & 3 deletions privacy_consent/i18n/privacy_consent.pot
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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 ""
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions privacy_consent/models/privacy_activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,6 @@ def action_new_consents(self):
"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
Expand Down
18 changes: 3 additions & 15 deletions privacy_consent/models/privacy_consent.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,23 +101,10 @@ def _url(self, accept):

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."""
Expand All @@ -135,7 +122,8 @@ def _run_action(self):
@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
Expand Down
4 changes: 2 additions & 2 deletions privacy_consent/readme/USAGE.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions privacy_consent/static/description/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,8 @@ <h1><a class="toc-backref" href="#id3">Usage</a></h1>
<ul class="simple">
<li>If you chose <em>Manual</em> mode, all missing consent request are created as
drafts, and nothing else is done now.</li>
<li>If you chose <em>Automatic</em> mode, also those requests are sent to subjects
and set as <em>Sent</em>.</li>
<li>If you chose <em>Automatic</em> mode, also those request e-mails are enqueued
and, when the mail queue is cleared, they will be set as <em>Sent</em>.</li>
</ul>
</li>
<li><p class="first">You will be presented with the list of just-created consent requests.
Expand Down
14 changes: 11 additions & 3 deletions privacy_consent/tests/test_consent.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ def setUp(self):
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")
Expand Down Expand Up @@ -93,11 +95,17 @@ def check_activity_auto_properly_sent(self):
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,
Expand Down Expand Up @@ -167,7 +175,7 @@ def test_generate_manually(self):
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)
Expand Down
4 changes: 2 additions & 2 deletions privacy_consent/views/privacy_activity.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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?"
/>
</div>
</group>
Expand Down

0 comments on commit e3948fb

Please sign in to comment.