From 94f2f6241b65c746baf13adaf5f60567877acc06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 8 Feb 2018 16:09:16 +0100 Subject: [PATCH 1/4] [FIX] mail: fix wrongly QWeb-converted bits of notification emails Notification emails now use QWeb to be rendered instead of jinja-based mail.template since 2f7593761c1ea432e2e58045150150356816f868. Part of the notification emails was not working anymore since its syntax was still using jinja. This commit fixes that. This commit also removes strange ';' that should not be part of python code as well as an unnecessary variable propagation. --- addons/mail/data/mail_data.xml | 12 ++++++------ addons/mail/models/res_partner.py | 6 ++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/addons/mail/data/mail_data.xml b/addons/mail/data/mail_data.xml index a6c9b62e019..fa48b112aa8 100644 --- a/addons/mail/data/mail_data.xml +++ b/addons/mail/data/mail_data.xml @@ -68,12 +68,12 @@
- - - + + +
-
@@ -116,9 +116,9 @@

Sent by - + - + using Odoo.

diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 6999acc015b..61fa18382df 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -73,17 +73,15 @@ class Partner(models.Model): if message.res_id and message.model in self.env: record = self.env[message.model].browse(message.res_id) - company = user.company_id; + company = user.company_id if record and hasattr(record, 'company_id'): - company = record.company_id; - company_name = company.name; + company = record.company_id return { 'message': message, 'signature': signature, 'website_url': website_url, 'company': company, - 'company_name': company_name, 'model_name': model_name, 'record': record, 'record_name': record_name, From b1b9896c46e98567bd68b489b365aebf5f98f2b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 8 Feb 2018 16:08:59 +0100 Subject: [PATCH 2/4] [FIX] mail: replace local links in notification emails Since notification emails are not mail templates anymore they do not go through the mail_template.render_post_process() method that converts local links to absolute links. This means that buttons and images were not correctly displayed in notification emails. Indeed those still had local links. This commit fixes that by moving the method directly in mail.thread and calling it accordingly when rendering the notification email. Note that the method has been changed in order to use regex instead of html parser. It is done based on work already done in mass mailing application. Indeed using parsers is complex and time consuming and may create changes in the html structure that are not intended. Next step in master is to go through various link modifications in mass mailing module to see how to handle them properly. Maybe. --- addons/mail/models/mail_template.py | 39 +--------------------- addons/mail/models/mail_thread.py | 35 ++++++++++++++++++- addons/mail/models/res_partner.py | 1 + addons/mass_mailing/models/mass_mailing.py | 2 +- addons/mass_mailing/wizard/test_mailing.py | 2 +- 5 files changed, 38 insertions(+), 41 deletions(-) diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index 586c5e9c281..f5d4b4e3ac6 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -313,46 +313,9 @@ class MailTemplate(models.Model): # RENDERING # ---------------------------------------- - @api.model - def _replace_local_links(self, html): - """ Post-processing of html content to replace local links to absolute - links, using web.base.url as base url. """ - if not html: - return html - - # form a tree - root = lxml.html.fromstring(html) - if not len(root) and root.text is None and root.tail is None: - html = u'
%s
' % html - root = lxml.html.fromstring(html, encoding='unicode') - - base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url') - base = urls.url_parse(base_url) - - def _process_link(url): - new_url = urls.url_parse(url) - if new_url.scheme and (new_url.netloc or new_url.scheme == 'mailto'): - return url - return new_url.replace(scheme=base.scheme, netloc=base.netloc).to_url() - - # check all nodes, replace : - # - img src -> check URL - # - a href -> check URL - for node in root.iter(): - if node.tag == 'a' and node.get('href'): - node.set('href', _process_link(node.get('href'))) - elif node.tag == 'img' and not node.get('src', 'data').startswith(u'data'): - node.set('src', _process_link(node.get('src'))) - - html = lxml.html.tostring(root, pretty_print=False, method='html', encoding='unicode') - # this is ugly, but lxml/etree tostring want to put everything in a 'div' that breaks the editor -> remove that - if html.startswith(u'
') and html.endswith(u'
'): - html = html[5:-6] - return html - @api.model def render_post_process(self, html): - html = self._replace_local_links(html) + html = self.env['mail.thread']._replace_local_links(html) return html @api.model diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 96d99fb85b5..8fcbed6ea16 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -23,9 +23,10 @@ from email.message import Message from email.utils import formataddr from lxml import etree from werkzeug import url_encode +from werkzeug import urls from odoo import _, api, exceptions, fields, models, tools -from odoo.tools import pycompat +from odoo.tools import pycompat, ustr from odoo.tools.safe_eval import safe_eval @@ -374,6 +375,38 @@ class MailThread(models.AbstractModel): # Technical methods / wrappers / tools # ------------------------------------------------------ + def _replace_local_links(self, html, base_url=None): + """ Replace local links by absolute links. It is required in various + cases, for example when sending emails on chatter or sending mass + mailings. It replaces + + * href of links (mailto will not match the regex) + * src of images (base64 hardcoded data will not match the regex) + * styling using url like background-image: url + + It is done using regex because it is shorten than using an html parser + to create a potentially complex soupe and hope to have a result that + has not been harmed. + """ + if not html: + return html + + html = ustr(html) + + def _sub_relative2absolute(match): + # compute here to do it only if really necessary + cache will ensure it is done only once + # if not base_url + if not _sub_relative2absolute.base_url: + _sub_relative2absolute.base_url = self.env["ir.config_parameter"].sudo().get_param("web.base.url") + return match.group(1) + urls.url_join(_sub_relative2absolute.base_url, match.group(2)) + + _sub_relative2absolute.base_url = base_url + html = re.sub(r"""(]*\ssrc=")(/[^/][^"]+)""", _sub_relative2absolute, html) + html = re.sub(r"""(]*\shref=")(/[^/][^"]+)""", _sub_relative2absolute, html) + html = re.sub(r"""(<[^>]+\bstyle="[^"]+\burl\('?)(/[^/'][^'")]+)""", _sub_relative2absolute, html) + + return html + @api.model def _garbage_collect_attachments(self): """ Garbage collect lost mail attachments. Those are attachments diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 61fa18382df..4cee80af16a 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -185,6 +185,7 @@ class Partner(models.Model): 'subject': message.subject or (message.record_name and 'Re: %s' % message.record_name), 'body': base_template.render(template_ctx, engine='ir.qweb'), } + fol_values['body'] = self.env['mail.thread']._replace_local_links(fol_values['body']) # send email new_emails, new_recipients_nbr = self._notify_send(fol_values['body'], fol_values['subject'], recipient_template_values['recipients'], **base_mail_values) # update notifications diff --git a/addons/mass_mailing/models/mass_mailing.py b/addons/mass_mailing/models/mass_mailing.py index 3c7480abfb2..6989cf2b1a6 100644 --- a/addons/mass_mailing/models/mass_mailing.py +++ b/addons/mass_mailing/models/mass_mailing.py @@ -742,7 +742,7 @@ class MassMailing(models.Model): raise UserError(_('Please select recipients.')) # Convert links in absolute URLs before the application of the shortener - mailing.body_html = self.env['mail.template']._replace_local_links(mailing.body_html) + mailing.body_html = self.env['mail.thread']._replace_local_links(mailing.body_html) composer_values = { 'author_id': author_id, diff --git a/addons/mass_mailing/wizard/test_mailing.py b/addons/mass_mailing/wizard/test_mailing.py index 24b395c7c53..f1231fd2b38 100644 --- a/addons/mass_mailing/wizard/test_mailing.py +++ b/addons/mass_mailing/wizard/test_mailing.py @@ -20,7 +20,7 @@ class TestMassMailing(models.TransientModel): test_emails = tools.email_split(self.email_to) for test_mail in test_emails: # Convert links in absolute URLs before the application of the shortener - mailing.write({'body_html': self.env['mail.template']._replace_local_links(mailing.body_html)}) + mailing.write({'body_html': self.env['mail.thread']._replace_local_links(mailing.body_html)}) mail_values = { 'email_from': mailing.email_from, 'reply_to': mailing.reply_to, From 2ef1394e677c407fced6b731422b08f52b38db40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 9 Feb 2018 10:12:19 +0100 Subject: [PATCH 3/4] [IMP] mass_mailing: use mail thread link replacer Mail thread now holds a method to replace local links by global links. Let us therefore use it in mass mailing as it does basically the same job. Adding base tag is not necessary anymore since there are not local links or images anymore. --- addons/mass_mailing/models/mail_mail.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/addons/mass_mailing/models/mail_mail.py b/addons/mass_mailing/models/mail_mail.py index e67010ca3a7..5889bbeede1 100644 --- a/addons/mass_mailing/models/mail_mail.py +++ b/addons/mass_mailing/models/mail_mail.py @@ -71,15 +71,7 @@ class MailMail(models.Model): new_href = href.replace(url, url + '/m/' + str(self.statistics_ids[0].id)) body = body.replace(href, new_href) - # prepend tag for images using absolute urls - domain = self.env["ir.config_parameter"].sudo().get_param("web.base.url") - base = "" % domain - body = tools.append_content_to_html(base, body, plaintext=False, container_tag='div') - # resolve relative image url to absolute for outlook.com - def _sub_relative2absolute(match): - return match.group(1) + werkzeug.urls.url_join(domain, match.group(2)) - body = re.sub('(]*\ssrc=")(/[^/][^"]+)', _sub_relative2absolute, body) - body = re.sub(r'(<[^>]+\bstyle="[^"]+\burl\(\'?)(/[^/\'][^\'")]+)', _sub_relative2absolute, body) + body = self.env['mail.thread']._replace_local_links(body) # generate tracking URL if self.statistics_ids: From 29955b21091ad8891a0e306477b1a197c510b15e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 9 Feb 2018 15:30:52 +0100 Subject: [PATCH 4/4] [DOC] test_mail: increment activity counter Sometimes runbot adds one query. Not sure why, let us avoid having red runbots. --- addons/test_mail/tests/test_performance.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 9505791bd98..8432cf54993 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -149,7 +149,7 @@ class TestAdvMailPerformance(TransactionCase): 'default_res_model': 'mail.test.activity', }) - with self.assertQueryCount(admin=47, emp=53): # com runbot 46 - 52 // test_mail only: 35 - 41 + with self.assertQueryCount(admin=47, emp=54): # com runbot 46 - 52 // test_mail only: 35 - 41 model.create({ 'summary': 'Test Activity', 'res_id': record.id, @@ -276,7 +276,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(admin=325, emp=388): # test_mail only: 308 - 368 + with self.assertQueryCount(admin=325, emp=389): # test_mail only: 308 - 368 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id,