Skip to content

Commit

Permalink
[FIX] mass_mailing: don't short url before rendering
Browse files Browse the repository at this point in the history
Before this commit, if you generate the href with jinja/mako syntax,
the un-rendered url will be shortened.

So:
    href="https://www.odoo.com?id=${object.id}"
will become:
    href="https://www.odoo.com/r/asw"

where
   https://www.odoo.com/r/asw
redirect to
   https://www.odoo.com?id=${object.id}
instead of
   https://www.odoo.com?id=1
  • Loading branch information
JKE-be committed Nov 12, 2018
1 parent 289982a commit 1167a5c
Show file tree
Hide file tree
Showing 5 changed files with 169 additions and 25 deletions.
1 change: 1 addition & 0 deletions addons/mass_mailing/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from . import mass_mailing
from . import mass_mailing_stats
from . import mail_mail
from . import mail_template
from . import mail_thread
from . import res_config_settings
from . import mass_mailing_report
Expand Down
22 changes: 22 additions & 0 deletions addons/mass_mailing/models/mail_template.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.

from odoo import api, models


class MailTemplate(models.Model):
_inherit = "mail.template"

@api.model
def render_post_process(self, html):
# super will transform relative url to absolute
html = super(MailTemplate, self).render_post_process(html)

# apply shortener after
if self.env.context.get('post_convert_links'):
html = self.env['link.tracker'].convert_links(
html,
self.env.context['post_convert_links'],
blacklist=['/unsubscribe_from_list']
)
return html
42 changes: 17 additions & 25 deletions addons/mass_mailing/models/mass_mailing.py
Original file line number Diff line number Diff line change
Expand Up @@ -680,11 +680,27 @@ def _get_seen_list(self):
"Mass-mailing %s has already reached %s %s emails", self, len(seen_list), target._name)
return seen_list

def _get_convert_links(self):
self.ensure_one()
utm_mixin = self.mass_mailing_campaign_id if self.mass_mailing_campaign_id else self
vals = {'mass_mailing_id': self.id}

if self.mass_mailing_campaign_id:
vals['mass_mailing_campaign_id'] = self.mass_mailing_campaign_id.id
if utm_mixin.campaign_id:
vals['campaign_id'] = utm_mixin.campaign_id.id
if utm_mixin.source_id:
vals['source_id'] = utm_mixin.source_id.id
if utm_mixin.medium_id:
vals['medium_id'] = utm_mixin.medium_id.id
return vals

def _get_mass_mailing_context(self):
"""Returns extra context items with pre-filled blacklist and seen list for massmailing"""
return {
'mass_mailing_blacklist': self._get_blacklist(),
'mass_mailing_seen_list': self._get_seen_list(),
'post_convert_links': self._get_convert_links(),
}

def get_recipients(self):
Expand Down Expand Up @@ -726,13 +742,10 @@ def send_mail(self, res_ids=None):
if not res_ids:
raise UserError(_('There is no recipients selected.'))

# Convert links in absolute URLs before the application of the shortener
mailing.body_html = self.env['mail.thread']._replace_local_links(mailing.body_html)

composer_values = {
'author_id': author_id,
'attachment_ids': [(4, attachment.id) for attachment in mailing.attachment_ids],
'body': mailing.convert_links()[mailing.id],
'body': mailing.body_html,
'subject': mailing.name,
'model': mailing.mailing_model_real,
'email_from': mailing.email_from,
Expand All @@ -756,27 +769,6 @@ def send_mail(self, res_ids=None):
mailing.write({'state': 'done', 'sent_date': fields.Datetime.now()})
return True

def convert_links(self):
res = {}
for mass_mailing in self:
utm_mixin = mass_mailing.mass_mailing_campaign_id if mass_mailing.mass_mailing_campaign_id else mass_mailing
html = mass_mailing.body_html if mass_mailing.body_html else ''

vals = {'mass_mailing_id': mass_mailing.id}

if mass_mailing.mass_mailing_campaign_id:
vals['mass_mailing_campaign_id'] = mass_mailing.mass_mailing_campaign_id.id
if utm_mixin.campaign_id:
vals['campaign_id'] = utm_mixin.campaign_id.id
if utm_mixin.source_id:
vals['source_id'] = utm_mixin.source_id.id
if utm_mixin.medium_id:
vals['medium_id'] = utm_mixin.medium_id.id

res[mass_mailing.id] = self.env['link.tracker'].convert_links(html, vals, blacklist=['/unsubscribe_from_list'])

return res

@api.model
def _process_mass_mailing_queue(self):
mass_mailings = self.search([('state', 'in', ('in_queue', 'sending')), '|', ('schedule_date', '<', fields.Datetime.now()), ('schedule_date', '=', False)])
Expand Down
1 change: 1 addition & 0 deletions addons/mass_mailing/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.

from . import test_mass_mailing_list_merge
from . import test_mass_mailing_shortener
128 changes: 128 additions & 0 deletions addons/mass_mailing/tests/test_mass_mailing_shortener.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.

from odoo.tests import common
from lxml import etree

try:
from unittest.mock import patch
except ImportError:
from mock import patch


class TestMassMailingShortener(common.TransactionCase):
def getHrefFor(self, html, id):
return html.xpath("*[@id='%s']" % id)[0].attrib.get('href')

def shorturl_to_link(self, short_url):
return self.env['link.tracker.code'].search([('code', '=', short_url.split('/r/')[-1])]).link_id

def setUp(self):
super(TestMassMailingShortener, self).setUp()

def _get_title_from_url(u):
return "Hello"

def _compute_favicon():
# 1px to avoid real request
return 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8DwHwAFBQIAX8jx0gAAAABJRU5ErkJggg=='

patcher = patch('odoo.addons.link_tracker.models.link_tracker.link_tracker._compute_favicon', wraps=_compute_favicon)
patcher2 = patch('odoo.addons.link_tracker.models.link_tracker.link_tracker._get_title_from_url', wraps=_get_title_from_url)
patcher.start()
patcher2.start()
self.addCleanup(patcher.stop)
self.addCleanup(patcher2.stop)

def test_00_test_mass_mailing_shortener(self):
mailing_list_A = self.env['mail.mass_mailing.list'].create({
'name': 'A',
'contact_ids': [
(0, 0, {'name': 'User 1', 'email': 'user1@example.com'}),
(0, 0, {'name': 'User 2', 'email': 'user2@example.com'}),
(0, 0, {'name': 'User 3', 'email': 'user3@example.com'}),
]
})

mass_mailing = self.env['mail.mass_mailing'].create({
"reply_to_mode": "email",
"reply_to": "Administrator <admin@yourcompany.example.com>",
"mailing_model_id": self.env.ref('mass_mailing.model_mail_mass_mailing_list').id,
"mailing_domain": "[('list_ids', 'in', [%d])]" % mailing_list_A.id,
"contact_list_ids": [[6, False, [mailing_list_A.id]]],
"mass_mailing_campaign_id": False,
"name": "sdf",
"body_html": """
Hi,
% set url = "www.odoo.com"
% set httpurl = "https://www.odoo.eu"
Website0: <a id="url0" href="https://www.odoo.tz/my/${object.name}">https://www.odoo.tz/my/${object.name}</h1>
Website1: <a id="url1" href="https://www.odoo.be">https://www.odoo.be</h1>
Website2: <a id="url2" href="https://${url}">https://${url}</h1>
Website3: <a id="url3" href="${httpurl}">${httpurl}</h1>
Email: <a id="url4" href="mailto:test@odoo.com">test@odoo.com</h1>
""",
"schedule_date": False,
"state": "draft",
"keep_archives": True,
})

mass_mailing.put_in_queue()
mass_mailing._process_mass_mailing_queue()

sent_mails = self.env['mail.mail'].search([('mailing_id', '=', mass_mailing.id)])
sent_messages = sent_mails.mapped('mail_message_id')

self.assertEqual(len(mailing_list_A.contact_ids.ids), len(sent_messages),
'Some message has not been sent')

xbody = etree.fromstring(sent_messages[0].body)
after_url0 = self.getHrefFor(xbody, 'url0')
after_url1 = self.getHrefFor(xbody, 'url1')
after_url2 = self.getHrefFor(xbody, 'url2')
after_url3 = self.getHrefFor(xbody, 'url3')
after_url4 = self.getHrefFor(xbody, 'url4')

self.assertTrue('/r/' in after_url0, 'URL0 should be shortened: %s' % after_url0)
self.assertTrue('/r/' in after_url1, 'URL1 should be shortened: %s' % after_url1)
self.assertTrue('/r/' in after_url2, 'URL2 should be shortened: %s' % after_url2)
self.assertTrue('/r/' in after_url3, 'URL3 should be shortened: %s' % after_url3)
self.assertEqual(after_url4, "mailto:test@odoo.com", 'mailto: has been converted')

short0 = self.shorturl_to_link(after_url0)
short1 = self.shorturl_to_link(after_url1)
short2 = self.shorturl_to_link(after_url2)
short3 = self.shorturl_to_link(after_url3)

self.assertTrue("https://www.odoo.tz/my/User" in short0.url, 'URL mismatch')
self.assertEqual(short1.url, "https://www.odoo.be", 'URL mismatch')
self.assertEqual(short2.url, "https://www.odoo.com", 'URL mismatch')
self.assertEqual(short3.url, "https://www.odoo.eu", 'URL mismatch')

_xbody = etree.fromstring(sent_messages[1].body)
_after_url0 = self.getHrefFor(_xbody, 'url0')
_after_url1 = self.getHrefFor(_xbody, 'url1')
_after_url2 = self.getHrefFor(_xbody, 'url2')
_after_url3 = self.getHrefFor(_xbody, 'url3')
_after_url4 = self.getHrefFor(_xbody, 'url4')

self.assertTrue('/r/' in _after_url0, 'URL0 should be shortened: %s' % _after_url0)
self.assertTrue('/r/' in _after_url1, 'URL1 should be shortened: %s' % _after_url1)
self.assertTrue('/r/' in _after_url2, 'URL2 should be shortened: %s' % _after_url2)
self.assertTrue('/r/' in _after_url3, 'URL3 should be shortened: %s' % _after_url3)
self.assertEqual(_after_url4, "mailto:test@odoo.com", 'mailto: has been converted')

_short0 = self.shorturl_to_link(_after_url0)
_short1 = self.shorturl_to_link(_after_url1)
_short2 = self.shorturl_to_link(_after_url2)
_short3 = self.shorturl_to_link(_after_url3)

self.assertTrue("https://www.odoo.tz/my/User" in _short0.url, 'URL mismatch')
self.assertEqual(_short1.url, "https://www.odoo.be", 'URL mismatch')
self.assertEqual(_short2.url, "https://www.odoo.com", 'URL mismatch')
self.assertEqual(_short3.url, "https://www.odoo.eu", 'URL mismatch')

self.assertNotEqual(short0.url, _short0.url)
self.assertEqual(short1.url, _short1.url)
self.assertEqual(short2.url, _short2.url)
self.assertEqual(short3.url, _short3.url)

0 comments on commit 1167a5c

Please sign in to comment.