From f0e5c19f8c70c7a5d7ee604c17a118e9f1467fa5 Mon Sep 17 00:00:00 2001 From: Simone Orsi Date: Thu, 5 Apr 2018 12:32:46 +0200 Subject: [PATCH 1/2] mail_digest: improve tests perf w/ SavepointCase --- mail_digest/tests/test_digest.py | 29 ++++++++++++----------- mail_digest/tests/test_partner_domains.py | 25 +++++++++---------- mail_digest/tests/test_subtypes_conf.py | 23 +++++++++--------- 3 files changed, 40 insertions(+), 37 deletions(-) diff --git a/mail_digest/tests/test_digest.py b/mail_digest/tests/test_digest.py index 879d39a0..880aa3f9 100644 --- a/mail_digest/tests/test_digest.py +++ b/mail_digest/tests/test_digest.py @@ -2,37 +2,38 @@ # Copyright 2017 Simone Orsi # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl). -from odoo.tests.common import TransactionCase +from odoo.tests.common import SavepointCase from odoo import exceptions -class DigestCase(TransactionCase): +class DigestCase(SavepointCase): - def setUp(self): - super(DigestCase, self).setUp() - self.partner_model = self.env['res.partner'] - self.message_model = self.env['mail.message'] - self.subtype_model = self.env['mail.message.subtype'] - self.digest_model = self.env['mail.digest'] - self.conf_model = self.env['partner.notification.conf'] + @classmethod + def setUpClass(cls): + super(DigestCase, cls).setUpClass() + cls.partner_model = cls.env['res.partner'] + cls.message_model = cls.env['mail.message'] + cls.subtype_model = cls.env['mail.message.subtype'] + cls.digest_model = cls.env['mail.digest'] + cls.conf_model = cls.env['partner.notification.conf'] - self.partner1 = self.partner_model.with_context( + cls.partner1 = cls.partner_model.with_context( tracking_disable=1).create({ 'name': 'Partner 1', 'email': 'partner1@test.foo.com', }) - self.partner2 = self.partner_model.with_context( + cls.partner2 = cls.partner_model.with_context( tracking_disable=1).create({ 'name': 'Partner 2', 'email': 'partner2@test.foo.com', }) - self.partner3 = self.partner_model.with_context( + cls.partner3 = cls.partner_model.with_context( tracking_disable=1).create({ 'name': 'Partner 3', 'email': 'partner3@test.foo.com', }) - self.subtype1 = self.subtype_model.create({'name': 'Type 1'}) - self.subtype2 = self.subtype_model.create({'name': 'Type 2'}) + cls.subtype1 = cls.subtype_model.create({'name': 'Type 1'}) + cls.subtype2 = cls.subtype_model.create({'name': 'Type 2'}) def test_get_or_create_digest(self): message1 = self.message_model.create({ diff --git a/mail_digest/tests/test_partner_domains.py b/mail_digest/tests/test_partner_domains.py index 16e10751..b2d5f40b 100644 --- a/mail_digest/tests/test_partner_domains.py +++ b/mail_digest/tests/test_partner_domains.py @@ -2,34 +2,35 @@ # Copyright 2017 Simone Orsi # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl). -from odoo.tests.common import TransactionCase +from odoo.tests.common import SavepointCase -class PartnerDomainCase(TransactionCase): +class PartnerDomainCase(SavepointCase): - def setUp(self): - super(PartnerDomainCase, self).setUp() - self.partner_model = self.env['res.partner'] - self.message_model = self.env['mail.message'] - self.subtype_model = self.env['mail.message.subtype'] + @classmethod + def setUpClass(cls): + super(PartnerDomainCase, cls).setUpClass() + cls.partner_model = cls.env['res.partner'] + cls.message_model = cls.env['mail.message'] + cls.subtype_model = cls.env['mail.message.subtype'] - self.partner1 = self.partner_model.with_context( + cls.partner1 = cls.partner_model.with_context( tracking_disable=1).create({ 'name': 'Partner 1', 'email': 'partner1@test.foo.com', }) - self.partner2 = self.partner_model.with_context( + cls.partner2 = cls.partner_model.with_context( tracking_disable=1).create({ 'name': 'Partner 2', 'email': 'partner2@test.foo.com', }) - self.partner3 = self.partner_model.with_context( + cls.partner3 = cls.partner_model.with_context( tracking_disable=1).create({ 'name': 'Partner 3', 'email': 'partner3@test.foo.com', }) - self.subtype1 = self.subtype_model.create({'name': 'Type 1'}) - self.subtype2 = self.subtype_model.create({'name': 'Type 2'}) + cls.subtype1 = cls.subtype_model.create({'name': 'Type 1'}) + cls.subtype2 = cls.subtype_model.create({'name': 'Type 2'}) def _assert_found(self, domain, not_found=False, partner=None): partner = partner or self.partner1 diff --git a/mail_digest/tests/test_subtypes_conf.py b/mail_digest/tests/test_subtypes_conf.py index 810b1bd8..4bf6f5eb 100644 --- a/mail_digest/tests/test_subtypes_conf.py +++ b/mail_digest/tests/test_subtypes_conf.py @@ -2,29 +2,30 @@ # Copyright 2017 Simone Orsi # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl). -from odoo.tests.common import TransactionCase +from odoo.tests.common import SavepointCase -class SubtypesCase(TransactionCase): +class SubtypesCase(SavepointCase): - def setUp(self): - super(SubtypesCase, self).setUp() - self.partner_model = self.env['res.partner'] - self.message_model = self.env['mail.message'] - self.subtype_model = self.env['mail.message.subtype'] + @classmethod + def setUpClass(cls): + super(SubtypesCase, cls).setUpClass() + cls.partner_model = cls.env['res.partner'] + cls.message_model = cls.env['mail.message'] + cls.subtype_model = cls.env['mail.message.subtype'] - self.partner1 = self.partner_model.with_context( + cls.partner1 = cls.partner_model.with_context( tracking_disable=1).create({ 'name': 'Partner 1!', 'email': 'partner1@test.foo.com', }) - self.partner2 = self.partner_model.with_context( + cls.partner2 = cls.partner_model.with_context( tracking_disable=1).create({ 'name': 'Partner 2!', 'email': 'partner2@test.foo.com', }) - self.subtype1 = self.subtype_model.create({'name': 'Type 1'}) - self.subtype2 = self.subtype_model.create({'name': 'Type 2'}) + cls.subtype1 = cls.subtype_model.create({'name': 'Type 1'}) + cls.subtype2 = cls.subtype_model.create({'name': 'Type 2'}) def _test_subtypes_rel(self): # setup: From 004e0edee3c06ccc4cb77d7020c6af54d81edf0f Mon Sep 17 00:00:00 2001 From: Simone Orsi Date: Fri, 6 Apr 2018 09:15:01 +0200 Subject: [PATCH 2/2] mail_digest: fix behavior w/ `force_send` enabled `force_send` is an option in `mail` core module that allows to send an email immediately. Handling the message w/ digest is wrong and in any case we gonna have a digest w/out the message inside it since is going to be deleted right after send. A typical use case is the notification sent to new followers. --- mail_digest/README.rst | 9 +++++++++ mail_digest/__manifest__.py | 2 +- mail_digest/models/res_partner.py | 20 +++++++++++++------- mail_digest/tests/test_partner_domains.py | 14 ++++++++++++++ 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/mail_digest/README.rst b/mail_digest/README.rst index 37b7b691..5024e223 100644 --- a/mail_digest/README.rst +++ b/mail_digest/README.rst @@ -42,6 +42,15 @@ Specifically: NOTE: under the hood the digest notification logic excludes followers to be notified, since you really want to notify only mail.digest's partner. +NOTE 2: Odoo's mail machinery has an option `force_send` +to send the email immediately without waiting for the mail queue to be processed. +When this option is used the email is sent right away +and the message record is deleted right after. + +A typical use case is the reset password mail. +We assume that if you use that option you really want the email to go out "now" +so when `force_send` is used, digest machinery is completely bypassed. + Global settings --------------- diff --git a/mail_digest/__manifest__.py b/mail_digest/__manifest__.py index 30ca2871..e8ae9b38 100644 --- a/mail_digest/__manifest__.py +++ b/mail_digest/__manifest__.py @@ -5,7 +5,7 @@ { 'name': 'Mail digest', 'summary': """Basic digest mail handling.""", - 'version': '10.0.1.0.1', + 'version': '10.0.1.0.2', 'license': 'AGPL-3', 'author': 'Camptocamp,Odoo Community Association (OCA)', 'website': 'https://github.com/OCA/social', diff --git a/mail_digest/models/res_partner.py b/mail_digest/models/res_partner.py index d39c5b57..1df91824 100644 --- a/mail_digest/models/res_partner.py +++ b/mail_digest/models/res_partner.py @@ -89,7 +89,8 @@ class ResPartner(models.Model): force_send=False, send_after_commit=True, user_signature=True): """Override to delegate domain generation.""" # notify_by_email - email_domain = self._get_notify_by_email_domain(message) + email_domain = self._get_notify_by_email_domain( + message, force_send=force_send) # `sudo` from original odoo method # the reason should be that anybody can write messages to a partner # and you really want to find all ppl to be notified @@ -97,11 +98,12 @@ class ResPartner(models.Model): partners._notify_by_email( message, force_send=force_send, send_after_commit=send_after_commit, user_signature=user_signature) - # notify_by_digest - digest_domain = self._get_notify_by_email_domain( - message, digest=True) - partners = self.sudo().search(digest_domain) - partners._notify_by_digest(message) + if not force_send: + # notify_by_digest + digest_domain = self._get_notify_by_email_domain( + message, force_send=force_send, digest=True) + partners = self.sudo().search(digest_domain) + partners._notify_by_digest(message) # notify_by_chat self._notify_by_chat(message) @@ -127,10 +129,12 @@ class ResPartner(models.Model): self.env['mail.digest'].sudo().create_or_update(self, message) @api.model - def _get_notify_by_email_domain(self, message, digest=False): + def _get_notify_by_email_domain(self, message, + force_send=False, digest=False): """Return domain to collect partners to be notified by email. :param message: instance of mail.message + :param force_send: whether the message should be sent immediately :param digest: include/exclude digest enabled partners NOTE: since mail.mail inherits from mail.message @@ -155,6 +159,8 @@ class ResPartner(models.Model): ('channel_ids', 'in', channels.ids), ('email', '!=', email) ] + if force_send: + return domain if not digest: domain.append(('notify_email', 'not in', ('none', 'digest'))) else: diff --git a/mail_digest/tests/test_partner_domains.py b/mail_digest/tests/test_partner_domains.py index b2d5f40b..5600b607 100644 --- a/mail_digest/tests/test_partner_domains.py +++ b/mail_digest/tests/test_partner_domains.py @@ -91,6 +91,20 @@ class PartnerDomainCase(SavepointCase): domain = partner._get_notify_by_email_domain(message, digest=1) self._assert_found(domain) + def test_notify_domains_digest_force_send(self): + # when `force_send` is true, digest machinery is bypassed + message = self.message_model.create({'body': 'My Body', }) + partner = self.partner1 + partner.notify_email = 'digest' + # even if we have digest mode on, we find the guy + domain = partner._get_notify_by_email_domain(message, force_send=True) + self._assert_found(domain) + # when asking for digest domain we don't get digest-related leaves + # as digest domain part is bypassed + domain = partner._get_notify_by_email_domain( + message, force_send=True, digest=True) + self.assertNotIn('notify_email', [x[0] for x in domain]) + def test_notify_domains_none(self): message = self.message_model.create({'body': 'My Body', }) partner = self.partner1