From ea04357131da789a1a3c3bfd19b4b249011cd72b Mon Sep 17 00:00:00 2001 From: David Date: Thu, 29 Nov 2018 13:03:47 +0100 Subject: [PATCH] [FIX] mail_tracking: wrong bounced partner - Partner get bounced when his associated email is in a hard bounced tracking. It didn't matter if the hard bounce event was solved in that recipient later and leaded to an increasing number of false positives. - We also use the email_bounced_set() method to get the whole bounce info in case positive. --- mail_tracking/README.rst | 1 + mail_tracking/__manifest__.py | 2 +- mail_tracking/models/mail_tracking_email.py | 16 ++++++++++------ mail_tracking/models/res_partner.py | 17 +++++++++++------ mail_tracking/tests/__init__.py | 2 -- mail_tracking/tests/test_mail_tracking.py | 15 +++++++++------ 6 files changed, 32 insertions(+), 21 deletions(-) diff --git a/mail_tracking/README.rst b/mail_tracking/README.rst index fb4118ae..81b7ba25 100644 --- a/mail_tracking/README.rst +++ b/mail_tracking/README.rst @@ -96,6 +96,7 @@ Contributors * Pedro M. Baeza * Antonio Espinosa * David Vidal + * Rafael Blasco Maintainer ---------- diff --git a/mail_tracking/__manifest__.py b/mail_tracking/__manifest__.py index 030c0f67..7ed8713b 100644 --- a/mail_tracking/__manifest__.py +++ b/mail_tracking/__manifest__.py @@ -5,7 +5,7 @@ { "name": "Email tracking", "summary": "Email tracking system for all mails sent", - "version": "11.0.1.1.0", + "version": "11.0.1.2.0", "category": "Social Network", "website": "http://github.com/OCA/social", "author": "Tecnativa, " diff --git a/mail_tracking/models/mail_tracking_email.py b/mail_tracking/models/mail_tracking_email.py index a69e3210..3a2e9b35 100644 --- a/mail_tracking/models/mail_tracking_email.py +++ b/mail_tracking/models/mail_tracking_email.py @@ -94,12 +94,16 @@ class MailTrackingEmail(models.Model): @api.model def email_is_bounced(self, email): - if email: - return self.search_count([ - ('recipient_address', '=', email.lower()), - ('state', 'in', ('error', 'rejected', 'spam', 'bounced')), - ]) > 0 - return False + if not email: + return False + res = self._email_last_tracking_state(email) + return res and res[0].get('state', '') in ['rejected', 'error', + 'spam', 'bounced'] + + @api.model + def _email_last_tracking_state(self, email): + return self.search_read([('recipient_address', '=', email.lower())], + ['state'], limit=1, order='time DESC') @api.model def email_score_from_email(self, email): diff --git a/mail_tracking/models/res_partner.py b/mail_tracking/models/res_partner.py index 6c6a319b..efefed78 100644 --- a/mail_tracking/models/res_partner.py +++ b/mail_tracking/models/res_partner.py @@ -40,10 +40,15 @@ class ResPartner(models.Model): return partners.write({'email_bounced': True}) def write(self, vals): - email = vals.get('email') - if email is not None: - vals['email'] = email.lower() if email else False - vals['email_bounced'] = ( - bool(email) and - self.env['mail.tracking.email'].email_is_bounced(email)) + if 'email' not in vals: + return super(ResPartner, self).write(vals) + email = vals['email'].lower() if vals['email'] else False + mte_obj = self.env['mail.tracking.email'] + if not mte_obj.email_is_bounced(email): + vals['email_bounced'] = False + return super(ResPartner, self).write(vals) + res = mte_obj._email_last_tracking_state(email) + tracking = mte_obj.browse(res[0].get('id')) + event = tracking.tracking_event_ids[:1] + self.email_bounced_set(tracking, event.error_details, event) return super(ResPartner, self).write(vals) diff --git a/mail_tracking/tests/__init__.py b/mail_tracking/tests/__init__.py index 03209262..d40d444b 100644 --- a/mail_tracking/tests/__init__.py +++ b/mail_tracking/tests/__init__.py @@ -1,6 +1,4 @@ -# -*- coding: utf-8 -*- # Copyright 2016 Antonio Espinosa - # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -# flake8: noqa from . import test_mail_tracking diff --git a/mail_tracking/tests/test_mail_tracking.py b/mail_tracking/tests/test_mail_tracking.py index 7dcb65c6..53c62fbe 100644 --- a/mail_tracking/tests/test_mail_tracking.py +++ b/mail_tracking/tests/test_mail_tracking.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- # Copyright 2016 Antonio Espinosa - # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). @@ -50,10 +49,6 @@ class TestMailTracking(TransactionCase): http.request = self.last_request return super(TestMailTracking, self).tearDown(*args, **kwargs) - def test_email_lower(self): - self.recipient.write({'email': 'UPPER@example.com'}) - self.assertEqual('upper@example.com', self.recipient.email) - def test_empty_email(self): self.recipient.write({'email_bounced': True}) self.recipient.write({'email': False}) @@ -61,7 +56,6 @@ class TestMailTracking(TransactionCase): self.assertEqual(False, self.recipient.email_bounced) self.recipient.write({'email_bounced': True}) self.recipient.write({'email': ''}) - self.assertEqual(False, self.recipient.email) self.assertEqual(False, self.recipient.email_bounced) self.assertEqual( False, @@ -303,6 +297,15 @@ class TestMailTracking(TransactionCase): self.assertEqual('bounced', tracking.state) self.assertEqual(0.0, self.recipient.email_score) + def test_bounce_new_partner(self): + mail, tracking = self.mail_send(self.recipient.email) + tracking.event_create('hard_bounce', {}) + new_partner = self.env['res.partner'].create({ + 'name': 'Test New Partner', + }) + new_partner.email = self.recipient.email + self.assertTrue(new_partner.email_bounced) + def test_recordset_email_score(self): """For backwords compatibility sake""" trackings = self.env['mail.tracking.email']