diff --git a/mail_tracking/__init__.py b/mail_tracking/__init__.py index 1c66d89e..e32deff2 100644 --- a/mail_tracking/__init__.py +++ b/mail_tracking/__init__.py @@ -5,4 +5,4 @@ from . import models from . import controllers -from .hooks import post_init_hook +from .hooks import pre_init_hook diff --git a/mail_tracking/__openerp__.py b/mail_tracking/__openerp__.py index 10cb3dcf..fe23e6af 100644 --- a/mail_tracking/__openerp__.py +++ b/mail_tracking/__openerp__.py @@ -5,7 +5,7 @@ { "name": "Email tracking", "summary": "Email tracking system for all mails sent", - "version": "8.0.2.0.0", + "version": "8.0.2.0.1", "category": "Social Network", "website": "http://www.tecnativa.com", "author": "Tecnativa, " @@ -28,5 +28,5 @@ "qweb": [ "static/src/xml/mail_tracking.xml", ], - "post_init_hook": "post_init_hook", + "pre_init_hook": "pre_init_hook", } diff --git a/mail_tracking/hooks.py b/mail_tracking/hooks.py index 2b6b20b1..8d3dc43d 100644 --- a/mail_tracking/hooks.py +++ b/mail_tracking/hooks.py @@ -3,22 +3,34 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). import logging -from openerp import api, SUPERUSER_ID +from psycopg2.extensions import AsIs _logger = logging.getLogger(__name__) -def post_init_hook(cr, registry): - with api.Environment.manage(): - env = api.Environment(cr, SUPERUSER_ID, {}) - # Recalculate all partner tracking_email_ids - partners = env['res.partner'].search([ - ('email', '!=', False), - ]) - emails = partners.mapped('email') - _logger.info( - "Recalculating 'tracking_email_ids' in 'res.partner' " - "model for %d email addresses", len(emails)) - for email in emails: - env['mail.tracking.email'].tracking_ids_recalculate( - 'res.partner', 'email', 'tracking_email_ids', email) +def column_exists(cr, table, column): + cr.execute(""" + SELECT column_name + FROM information_schema.columns + WHERE table_name = %s AND column_name = %s""", (table, column)) + return bool(cr.fetchall()) + + +def column_add_with_value(cr, table, column, field_type, value): + if not column_exists(cr, table, column): + cr.execute(""" + ALTER TABLE %s + ADD COLUMN %s %s""", (AsIs(table), AsIs(column), AsIs(field_type))) + cr.execute(""" + UPDATE %s SET %s = %s""", (AsIs(table), AsIs(column), value)) + + +def pre_init_hook(cr): + _logger.info("Creating res.partner.tracking_emails_count column " + "with value 0") + column_add_with_value( + cr, "res_partner", "tracking_emails_count", "integer", 0) + _logger.info("Creating res.partner.email_score column " + "with value 50.0") + column_add_with_value( + cr, "res_partner", "email_score", "double precision", 50.0) diff --git a/mail_tracking/models/mail_message.py b/mail_tracking/models/mail_message.py index d5d381ee..682ef619 100644 --- a/mail_tracking/models/mail_message.py +++ b/mail_tracking/models/mail_message.py @@ -46,7 +46,7 @@ class MailMessage(models.Model): tracking_email = self.env['mail.tracking.email'].search([ ('mail_message_id', '=', mail_message_id), ('partner_id', '=', partner_id), - ]) + ], limit=1) status = self._partner_tracking_status_get(tracking_email) partner_trackings[str(partner_id)] = ( status, tracking_email.id) diff --git a/mail_tracking/models/mail_tracking_email.py b/mail_tracking/models/mail_tracking_email.py index d0452add..6b2944be 100644 --- a/mail_tracking/models/mail_tracking_email.py +++ b/mail_tracking/models/mail_tracking_email.py @@ -13,6 +13,9 @@ import openerp.addons.decimal_precision as dp _logger = logging.getLogger(__name__) +EVENT_OPEN_DELTA = 10 # seconds +EVENT_CLICK_DELTA = 5 # seconds + class MailTrackingEmail(models.Model): _name = "mail.tracking.email" @@ -104,7 +107,7 @@ class MailTrackingEmail(models.Model): obj.write({ tracking_field: [(5, False, False)] }) - return True + return objects @api.model def _tracking_ids_to_write(self, email): @@ -256,13 +259,41 @@ class MailTrackingEmail(models.Model): _logger.info('Unknown event type: %s' % event_type) return False + def _concurrent_events(self, event_type, metadata): + m_event = self.env['mail.tracking.event'] + self.ensure_one() + concurrent_event_ids = False + if event_type in {'open', 'click'}: + ts = metadata.get('timestamp', time.time()) + delta = EVENT_OPEN_DELTA if event_type == 'open' \ + else EVENT_CLICK_DELTA + domain = [ + ('timestamp', '>=', ts - delta), + ('timestamp', '<=', ts + delta), + ('tracking_email_id', '=', self.id), + ('event_type', '=', event_type), + ] + if event_type == 'click': + domain.append(('url', '=', metadata.get('url', False))) + concurrent_event_ids = m_event.search(domain) + return concurrent_event_ids + @api.multi def event_create(self, event_type, metadata): event_ids = self.env['mail.tracking.event'] for tracking_email in self: - vals = tracking_email._event_prepare(event_type, metadata) - if vals: - event_ids += event_ids.sudo().create(vals) + other_ids = tracking_email._concurrent_events(event_type, metadata) + if not other_ids: + vals = tracking_email._event_prepare(event_type, metadata) + if vals: + event_ids += event_ids.sudo().create(vals) + partners = self.tracking_ids_recalculate( + 'res.partner', 'email', 'tracking_email_ids', + tracking_email.recipient_address) + if partners: + partners.email_score_calculate() + else: + _logger.debug("Concurrent event '%s' discarded", event_type) return event_ids @api.model diff --git a/mail_tracking/models/res_partner.py b/mail_tracking/models/res_partner.py index d4ae19de..c4eb294a 100644 --- a/mail_tracking/models/res_partner.py +++ b/mail_tracking/models/res_partner.py @@ -15,13 +15,18 @@ class ResPartner(models.Model): string="Tracking emails count", store=True, readonly=True, compute="_compute_tracking_emails_count") email_score = fields.Float( - string="Email score", - compute="_compute_email_score", store=True, readonly=True) + string="Email score", readonly=True, default=50.0) - @api.one - @api.depends('tracking_email_ids.state') - def _compute_email_score(self): - self.email_score = self.tracking_email_ids.email_score() + @api.multi + def email_score_calculate(self): + # This is not a compute method because is causing a inter-block + # in mail_tracking_email PostgreSQL table + # We suspect that tracking_email write to state field block that + # table and then inside write ORM try to read from DB + # tracking_email_ids because it's not in cache. + # PostgreSQL blocks read because we have not committed yet the write + for partner in self: + partner.email_score = partner.tracking_email_ids.email_score() @api.one @api.depends('tracking_email_ids') diff --git a/mail_tracking/tests/test_mail_tracking.py b/mail_tracking/tests/test_mail_tracking.py index 816a6bef..a6eacc60 100644 --- a/mail_tracking/tests/test_mail_tracking.py +++ b/mail_tracking/tests/test_mail_tracking.py @@ -4,9 +4,9 @@ import mock import base64 +import time from openerp.tests.common import TransactionCase -from openerp.addons.mail_tracking.controllers.main import \ - MailTrackingController, BLANK +from ..controllers.main import MailTrackingController, BLANK mock_request = 'openerp.http.request' mock_send_email = ('openerp.addons.base.ir.ir_mail_server.' @@ -114,6 +114,77 @@ class TestMailTracking(TransactionCase): res = controller.mail_tracking_open(db, tracking.id) self.assertEqual(image, res.response[0]) + def test_concurrent_open(self): + mail, tracking = self.mail_send() + ts = time.time() + metadata = { + 'ip': '127.0.0.1', + 'user_agent': 'Odoo Test/1.0', + 'os_family': 'linux', + 'ua_family': 'odoo', + 'timestamp': ts, + } + # First open event + tracking.event_create('open', metadata) + opens = tracking.tracking_event_ids.filtered( + lambda r: r.event_type == 'open' + ) + self.assertEqual(len(opens), 1) + # Concurrent open event + metadata['timestamp'] = ts + 2 + tracking.event_create('open', metadata) + opens = tracking.tracking_event_ids.filtered( + lambda r: r.event_type == 'open' + ) + self.assertEqual(len(opens), 1) + # Second open event + metadata['timestamp'] = ts + 350 + tracking.event_create('open', metadata) + opens = tracking.tracking_event_ids.filtered( + lambda r: r.event_type == 'open' + ) + self.assertEqual(len(opens), 2) + + def test_concurrent_click(self): + mail, tracking = self.mail_send() + ts = time.time() + metadata = { + 'ip': '127.0.0.1', + 'user_agent': 'Odoo Test/1.0', + 'os_family': 'linux', + 'ua_family': 'odoo', + 'timestamp': ts, + 'url': 'https://www.example.com/route/1', + } + # First click event (URL 1) + tracking.event_create('click', metadata) + opens = tracking.tracking_event_ids.filtered( + lambda r: r.event_type == 'click' + ) + self.assertEqual(len(opens), 1) + # Concurrent click event (URL 1) + metadata['timestamp'] = ts + 2 + tracking.event_create('click', metadata) + opens = tracking.tracking_event_ids.filtered( + lambda r: r.event_type == 'click' + ) + self.assertEqual(len(opens), 1) + # Second click event (URL 1) + metadata['timestamp'] = ts + 350 + tracking.event_create('click', metadata) + opens = tracking.tracking_event_ids.filtered( + lambda r: r.event_type == 'click' + ) + self.assertEqual(len(opens), 2) + # Concurrent click event (URL 2) + metadata['timestamp'] = ts + 2 + metadata['url'] = 'https://www.example.com/route/2' + tracking.event_create('click', metadata) + opens = tracking.tracking_event_ids.filtered( + lambda r: r.event_type == 'click' + ) + self.assertEqual(len(opens), 3) + def test_smtp_error(self): with mock.patch(mock_send_email) as mock_func: mock_func.side_effect = Warning('Test error')