From aa74a975d7a710bfc55d586f1898164d5813f34c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 25 Feb 2020 14:26:23 +0000 Subject: [PATCH 1/9] [IMP] base: add tests for partner find_or_create / name_create / _parse_partner_name As find_or_create API and code will be slightly improved soon let us add some tests to ensure behavior and avoid (too much) regressions. Notably about the fallback on faulty emails support aka "name email@domain)" which in parsing is considered as valid. This may happen when people copy-paste emails. LINKS Task ID 1963529 Community PR odoo/odoo#32397 --- odoo/addons/base/tests/test_base.py | 84 ++++++++++++++++++++++------- 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/odoo/addons/base/tests/test_base.py b/odoo/addons/base/tests/test_base.py index 1ca22defd42..160ec0c7f11 100644 --- a/odoo/addons/base/tests/test_base.py +++ b/odoo/addons/base/tests/test_base.py @@ -68,41 +68,89 @@ SAMPLES = [ ('ryu+giga-Sushi@aizubange.fukushima.jp', '', 'ryu+giga-Sushi@aizubange.fukushima.jp'), ('Raoul chirurgiens-dentistes.fr', 'Raoul chirurgiens-dentistes.fr', ''), (" Raoul O'hara ", "Raoul O'hara", '!@historicalsociety.museum'), + ('Raoul Grosbedon ', 'Raoul Grosbedon', 'raoul@CHIRURGIENS-dentistes.fr'), + ('Raoul megaraoul@chirurgiens-dentistes.fr', 'Raoul', 'megaraoul@chirurgiens-dentistes.fr'), ] + class TestBase(TransactionCase): + def _check_find_or_create(self, test_string, expected_name, expected_email, check_partner=False, should_create=False): + partner = self.env['res.partner'].browse(self.env['res.partner'].find_or_create(test_string)) + if should_create and check_partner: + self.assertTrue(partner.id > check_partner.id, 'find_or_create failed - should have found existing') + elif check_partner: + self.assertEqual(partner, check_partner, 'find_or_create failed - should have found existing') + self.assertEqual(partner.name, expected_name) + self.assertEqual(partner.email or '', expected_email) + return partner + def test_00_res_partner_name_create(self): res_partner = self.env['res.partner'] parse = res_partner._parse_partner_name for text, name, mail in SAMPLES: - self.assertEqual((name, mail), parse(text), 'Partner name parsing failed') + self.assertEqual((name, mail), parse(text)) partner_id, dummy = res_partner.name_create(text) partner = res_partner.browse(partner_id) - self.assertEqual(name or mail, partner.name, 'Partner name incorrect') - self.assertEqual(mail or False, partner.email, 'Partner email incorrect') + self.assertEqual(name or mail, partner.name) + self.assertEqual(mail or False, partner.email) + + # name_create supports default_email fallback + partner = self.env['res.partner'].browse( + self.env['res.partner'].with_context( + default_email='John.Wick@example.com' + ).name_create('"Raoulette Vachette" ')[0] + ) + self.assertEqual(partner.name, 'Raoulette Vachette') + self.assertEqual(partner.email, 'Raoul@Grosbedon.fr') + + partner = self.env['res.partner'].browse( + self.env['res.partner'].with_context( + default_email='John.Wick@example.com' + ).name_create('Raoulette Vachette')[0] + ) + self.assertEqual(partner.name, 'Raoulette Vachette') + self.assertEqual(partner.email, 'John.Wick@example.com') def test_10_res_partner_find_or_create(self): res_partner = self.env['res.partner'] - email = SAMPLES[0][0] - partner_id, dummy = res_partner.name_create(email) - found_id = res_partner.find_or_create(email) - self.assertEqual(partner_id, found_id, 'find_or_create failed') - self.assertEqual(SAMPLES[0][1], res_partner.browse([found_id]).name, 'Partner name is incorrect') + partner = res_partner.browse(res_partner.name_create(SAMPLES[0][0])[0]) + self._check_find_or_create( + SAMPLES[0][0], SAMPLES[0][1], SAMPLES[0][2], + check_partner=partner, should_create=False + ) - partner_id2, dummy2 = res_partner.name_create('sarah.john@connor.com') - found_id2 = res_partner.find_or_create('john@connor.com') - self.assertNotEqual(partner_id2, found_id2, 'john@connor.com match sarah.john@connor.com') - self.assertEqual('john@connor.com', res_partner.browse([found_id2]).name, 'Partner name is incorrect') + partner_2 = res_partner.browse(res_partner.name_create('sarah.john@connor.com')[0]) + found_2 = self._check_find_or_create( + 'john@connor.com', 'john@connor.com', 'john@connor.com', + check_partner=partner_2, should_create=True + ) - new_id = res_partner.find_or_create(SAMPLES[1][0]) - self.assertTrue(new_id > partner_id, 'find_or_create failed - should have created new one') - self.assertEqual(SAMPLES[1][2], res_partner.browse([new_id]).name, 'Partner name is incorrect') + new = self._check_find_or_create( + SAMPLES[1][0], SAMPLES[1][2], SAMPLES[1][2], + check_partner=found_2, should_create=True + ) - new_id2 = res_partner.find_or_create(SAMPLES[2][0]) - self.assertTrue(new_id2 > new_id, 'find_or_create failed - should have created new one again') - self.assertEqual(SAMPLES[2][1], res_partner.browse([new_id2]).name, 'Partner name is incorrect') + new2 = self._check_find_or_create( + SAMPLES[2][0], SAMPLES[2][1], SAMPLES[2][2], + check_partner=new, should_create=True + ) + + new3 = self._check_find_or_create( + SAMPLES[3][0], SAMPLES[3][1], SAMPLES[3][2], + check_partner=new2, should_create=True + ) + + new4 = self._check_find_or_create( + SAMPLES[4][0], SAMPLES[0][1], SAMPLES[0][2], + check_partner=partner, should_create=False + ) + + new5 = self._check_find_or_create( + SAMPLES[5][0], SAMPLES[5][1], SAMPLES[5][2], + check_partner=new4, should_create=True + ) def test_15_res_partner_name_search(self): res_partner = self.env['res.partner'] From 6e25a510b3b44cbdd5844b224cc90eb6108853fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 25 Feb 2020 14:26:39 +0000 Subject: [PATCH 2/9] [REF] base, mail: improve partner creation from name / email Rewrite code parsing name (_parse_partner_name) to make it easier to understand. Tweak find_or_create to return directly a recordset with the found partner / new partner instead of a name_get result. Also inherit Partner.find_or_create() in mail in order to benefits from email_normalized field, allowing to improve the search on matching emails and avoid duplicates. LINKS Task ID 1963529 Community PR odoo/odoo#32397 --- addons/mail/models/mail_template.py | 4 +- addons/mail/models/res_partner.py | 18 +++++- addons/mail/tests/__init__.py | 1 + addons/mail/tests/test_res_partner.py | 27 +++++++++ odoo/addons/base/models/res_partner.py | 77 +++++++++++++++++--------- odoo/addons/base/tests/test_base.py | 12 ++-- 6 files changed, 105 insertions(+), 34 deletions(-) create mode 100644 addons/mail/tests/test_res_partner.py diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index be6040a4025..43082e2d47b 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -302,8 +302,8 @@ class MailTemplate(models.Model): if records_company: Partner = Partner.with_context(default_company_id=records_company[res_id]) for mail in mails: - partner_id = Partner.find_or_create(mail) - partner_ids.append(partner_id) + partner = Partner.find_or_create(mail) + partner_ids.append(partner.id) partner_to = values.pop('partner_to', '') if partner_to: # placeholders could generate '', 3, 2 due to some empty field values diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index 70890cb368f..97305962691 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -3,7 +3,7 @@ import logging -from odoo import _, api, fields, models +from odoo import _, api, fields, models, tools from odoo.addons.bus.models.bus_presence import AWAY_TIMER from odoo.addons.bus.models.bus_presence import DISCONNECTION_TIMER from odoo.osv import expression @@ -35,6 +35,22 @@ class Partner(models.Model): 'email_cc': False} for r in self} + @api.model + def find_or_create(self, email, assert_valid_email=False): + """ Override to use the email_normalized field. """ + if not email: + raise ValueError(_('An email is required for find_or_create to work')) + + parsed_name, parsed_email = self._parse_partner_name(email) + if parsed_email: + email_normalized = tools.email_normalize(parsed_email) + if email_normalized: + partners = self.search([('email_normalized', '=', email_normalized)], limit=1) + if partners: + return partners + + return super(Partner, self).find_or_create(email, assert_valid_email=assert_valid_email) + @api.model def get_needaction_count(self): """ compute the number of needaction of the current user """ diff --git a/addons/mail/tests/__init__.py b/addons/mail/tests/__init__.py index aa9c3f5d9f0..4706daea1c6 100644 --- a/addons/mail/tests/__init__.py +++ b/addons/mail/tests/__init__.py @@ -1,3 +1,4 @@ # -*- coding: utf-8 -*- from . import common +from . import test_res_partner diff --git a/addons/mail/tests/test_res_partner.py b/addons/mail/tests/test_res_partner.py new file mode 100644 index 00000000000..de635930512 --- /dev/null +++ b/addons/mail/tests/test_res_partner.py @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.tests.common import TransactionCase + + +class TestPartner(TransactionCase): + + def test_res_partner_find_or_create(self): + Partner = self.env['res.partner'] + + existing = Partner.create({ + 'name': 'Patrick Poilvache', + 'email': '"Patrick Da Beast Poilvache" ', + }) + self.assertEqual(existing.name, 'Patrick Poilvache') + self.assertEqual(existing.email, '"Patrick Da Beast Poilvache" ') + self.assertEqual(existing.email_normalized, 'patrick@example.com') + + new = Partner.find_or_create('Patrick Caché ') + self.assertEqual(new, existing) + + new2 = Partner.find_or_create('Patrick Caché <2patrick@EXAMPLE.COM>') + self.assertTrue(new2.id > new.id) + self.assertEqual(new2.name, 'Patrick Caché') + self.assertEqual(new2.email, '2patrick@example.com') + self.assertEqual(new2.email_normalized, '2patrick@example.com') diff --git a/odoo/addons/base/models/res_partner.py b/odoo/addons/base/models/res_partner.py index 647afd65820..728bd040f1b 100644 --- a/odoo/addons/base/models/res_partner.py +++ b/odoo/addons/base/models/res_partner.py @@ -647,16 +647,33 @@ class Partner(models.Model): res.append((partner.id, name)) return res - def _parse_partner_name(self, text, context=None): - """ Supported syntax: - - 'Raoul ': will find name and email address - - otherwise: default, everything is set as the name """ - emails = tools.email_split(text.replace(' ', ',')) - if emails: - email = emails[0] - name = text[:text.index(email)].replace('"', '').replace('<', '').strip() + def _parse_partner_name(self, text): + """ Parse partner name (given by text) in order to find a name and an + email. Supported syntax: + + * Raoul + * "Raoul le Grand" + * Raoul raoul@grosbedon.fr (strange fault tolerant support from df40926d2a57c101a3e2d221ecfd08fbb4fea30e) + + Otherwise: default, everything is set as the name. Starting from 13.3 + returned email will be normalized to have a coherent encoding. + """ + name, email = '', '' + split_results = tools.email_split_tuples(text) + if split_results: + name, email = split_results[0] + + if email and not name: + fallback_emails = tools.email_split(text.replace(' ', ',')) + if fallback_emails: + email = fallback_emails[0] + name = text[:text.index(email)].replace('"', '').replace('<', '').strip() + + if email: + email = tools.email_normalize(email) else: name, email = text, '' + return name, email @api.model @@ -670,9 +687,11 @@ class Partner(models.Model): name, email = self._parse_partner_name(name) if self._context.get('force_email') and not email: raise UserError(_("Couldn't create contact without email address!")) - if not name and email: - name = email - partner = self.create({self._rec_name: name or email, 'email': email or self.env.context.get('default_email', False)}) + + create_values = {self._rec_name: name or email} + if email: # keep default_email in context + create_values['email'] = email + partner = self.create(create_values) return partner.name_get()[0] @api.model @@ -752,22 +771,30 @@ class Partner(models.Model): return super(Partner, self)._name_search(name, args, operator=operator, limit=limit, name_get_uid=name_get_uid) @api.model - def find_or_create(self, email): + def find_or_create(self, email, assert_valid_email=False): """ Find a partner with the given ``email`` or use :py:method:`~.name_create` - to create one + to create a new one. - :param str email: email-like string, which should contain at least one email, - e.g. ``"Raoul Grosbedon "``""" - assert email, 'an email is required for find_or_create to work' - emails = tools.email_split(email) - name_emails = tools.email_split_and_format(email) - if emails: - email = emails[0] - name_email = name_emails[0] - else: - name_email = email - partners = self.search([('email', '=ilike', email)], limit=1) - return partners.id or self.name_create(name_email)[0] + :param str email: email-like string, which should contain at least one email, + e.g. ``"Raoul Grosbedon "`` + :param boolean assert_valid_email: raise if no valid email is found + :return: newly created record + """ + if not email: + raise ValueError(_('An email is required for find_or_create to work')) + + parsed_name, parsed_email = self._parse_partner_name(email) + if not parsed_email and assert_valid_email: + raise ValueError(_('A valid email is required for find_or_create to work properly.')) + + partners = self.search([('email', '=ilike', parsed_email)], limit=1) + if partners: + return partners + + create_values = {self._rec_name: parsed_name or parsed_email} + if parsed_email: # keep default_email in context + create_values['email'] = parsed_email + return self.create(create_values) def _get_gravatar_image(self, email): email_hash = hashlib.md5(email.lower().encode('utf-8')).hexdigest() diff --git a/odoo/addons/base/tests/test_base.py b/odoo/addons/base/tests/test_base.py index 160ec0c7f11..eef6c749095 100644 --- a/odoo/addons/base/tests/test_base.py +++ b/odoo/addons/base/tests/test_base.py @@ -76,7 +76,7 @@ SAMPLES = [ class TestBase(TransactionCase): def _check_find_or_create(self, test_string, expected_name, expected_email, check_partner=False, should_create=False): - partner = self.env['res.partner'].browse(self.env['res.partner'].find_or_create(test_string)) + partner = self.env['res.partner'].find_or_create(test_string) if should_create and check_partner: self.assertTrue(partner.id > check_partner.id, 'find_or_create failed - should have found existing') elif check_partner: @@ -89,11 +89,11 @@ class TestBase(TransactionCase): res_partner = self.env['res.partner'] parse = res_partner._parse_partner_name for text, name, mail in SAMPLES: - self.assertEqual((name, mail), parse(text)) + self.assertEqual((name, mail.lower()), parse(text)) partner_id, dummy = res_partner.name_create(text) partner = res_partner.browse(partner_id) - self.assertEqual(name or mail, partner.name) - self.assertEqual(mail or False, partner.email) + self.assertEqual(name or mail.lower(), partner.name) + self.assertEqual(mail.lower() or False, partner.email) # name_create supports default_email fallback partner = self.env['res.partner'].browse( @@ -102,7 +102,7 @@ class TestBase(TransactionCase): ).name_create('"Raoulette Vachette" ')[0] ) self.assertEqual(partner.name, 'Raoulette Vachette') - self.assertEqual(partner.email, 'Raoul@Grosbedon.fr') + self.assertEqual(partner.email, 'raoul@grosbedon.fr') partner = self.env['res.partner'].browse( self.env['res.partner'].with_context( @@ -128,7 +128,7 @@ class TestBase(TransactionCase): ) new = self._check_find_or_create( - SAMPLES[1][0], SAMPLES[1][2], SAMPLES[1][2], + SAMPLES[1][0], SAMPLES[1][2].lower(), SAMPLES[1][2].lower(), check_partner=found_2, should_create=True ) From aa31e67716f19ad84145ecb35b0cda07e2143b83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 5 Apr 2019 09:21:18 +0000 Subject: [PATCH 3/9] [REF] mail, sms: reorganize {mail.sms}.template and mail.render.mixin fields and add docstrings PURPOSE Clarify mail.template, mail.render.mixin and sms.template code organization. SPECIFICATIONS Reorganize fields according to their main use. Remove an unnecessary onchange in mail template model. As "model" field on mail template is a computed field its computation is sufficient. Add some docstrings and code separations. Prepare future code change. LINKS Task ID 1963529 Community PR odoo/odoo#32397 --- addons/mail/models/mail_render_mixin.py | 16 ++++---- addons/mail/models/mail_template.py | 50 +++++++++++++------------ addons/sms/models/sms_template.py | 15 +++++--- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/addons/mail/models/mail_render_mixin.py b/addons/mail/models/mail_render_mixin.py index c2fafe1d56e..eb5bcbef882 100644 --- a/addons/mail/models/mail_render_mixin.py +++ b/addons/mail/models/mail_render_mixin.py @@ -3,28 +3,28 @@ from odoo import api, fields, models -class MailRenderMixin(models.AbstractModel): +class MailRenderMixin(models.AbstractModel): _name = 'mail.render.mixin' _description = 'Mail Render Mixin' model_object_field = fields.Many2one( 'ir.model.fields', string="Field", store=False, help="Select target field from the related document model.\n" - "If it is a relationship field you will be able to select " - "a target field at the destination of the relationship.") + "If it is a relationship field you will be able to select " + "a target field at the destination of the relationship.") sub_object = fields.Many2one( 'ir.model', 'Sub-model', readonly=True, store=False, help="When a relationship field is selected as first field, " - "this field shows the document model the relationship goes to.") + "this field shows the document model the relationship goes to.") sub_model_object_field = fields.Many2one( 'ir.model.fields', 'Sub-field', store=False, help="When a relationship field is selected as first field, " - "this field lets you select the target field within the " - "destination document model (sub-model).") + "this field lets you select the target field within the " + "destination document model (sub-model).") null_value = fields.Char('Default Value', store=False, help="Optional value to use if the target field is empty") copyvalue = fields.Char( - 'Placeholder Expression',store=False, + 'Placeholder Expression', store=False, help="Final placeholder expression, to be copy-pasted in the desired template field.") @api.onchange('model_object_field', 'sub_model_object_field', 'null_value') @@ -65,4 +65,4 @@ class MailRenderMixin(models.AbstractModel): if null_value: expression += " or '''%s'''" % null_value expression += "}" - return expression + return expression diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index 43082e2d47b..466d5bbf8eb 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -91,6 +91,7 @@ class MailTemplate(models.Model): res['model_id'] = self.env['ir.model']._get(res.pop('model')).id return res + # description name = fields.Char('Name') model_id = fields.Many2one('ir.model', 'Applies to', help="The type of document this template can be used with") model = fields.Char('Related Document Model', related='model_id.model', index=True, store=True, readonly=True) @@ -105,6 +106,7 @@ class MailTemplate(models.Model): email_from = fields.Char('From', help="Sender address (placeholders may be used here). If not set, the default " "value will be the author's email alias if configured, or email address.") + # recipients use_default_to = fields.Boolean( 'Default recipients', help="Default recipients of the record:\n" @@ -115,32 +117,28 @@ class MailTemplate(models.Model): help="Comma-separated ids of recipient partners (placeholders may be used here)") email_cc = fields.Char('Cc', help="Carbon copy recipients (placeholders may be used here)") reply_to = fields.Char('Reply-To', help="Preferred response address (placeholders may be used here)") - mail_server_id = fields.Many2one('ir.mail_server', 'Outgoing Mail Server', readonly=False, - help="Optional preferred server for outgoing mails. If not set, the highest " - "priority one will be used.") + # content body_html = fields.Html('Body', translate=True, sanitize=False) - report_name = fields.Char('Report Filename', translate=True, - help="Name to use for the generated report file (may contain placeholders)\n" - "The extension can be omitted and will then come from the report type.") - report_template = fields.Many2one('ir.actions.report', 'Optional report to print and attach') - ref_ir_act_window = fields.Many2one('ir.actions.act_window', 'Sidebar action', readonly=True, copy=False, - help="Sidebar action to make this template available on records " - "of the related document model") attachment_ids = fields.Many2many('ir.attachment', 'email_template_attachment_rel', 'email_template_id', 'attachment_id', 'Attachments', help="You may attach files to this template, to be added to all " "emails created from this template") - auto_delete = fields.Boolean('Auto Delete', default=True, help="This option permanently removes any track of email after send, including from the Technical menu in the Settings, in order to preserve storage space of your Odoo database.") - + report_name = fields.Char('Report Filename', translate=True, + help="Name to use for the generated report file (may contain placeholders)\n" + "The extension can be omitted and will then come from the report type.") + report_template = fields.Many2one('ir.actions.report', 'Optional report to print and attach') + # options + mail_server_id = fields.Many2one('ir.mail_server', 'Outgoing Mail Server', readonly=False, + help="Optional preferred server for outgoing mails. If not set, the highest " + "priority one will be used.") scheduled_date = fields.Char('Scheduled Date', help="If set, the queue manager will send the email after the date. If not set, the email will be send as soon as possible. Jinja2 placeholders may be used.") - - @api.onchange('model_id') - def onchange_model_id(self): - # TDE CLEANME: should'nt it be a stored related ? - if self.model_id: - self.model = self.model_id.model - else: - self.model = False + auto_delete = fields.Boolean( + 'Auto Delete', default=True, + help="This option permanently removes any track of email after send, including from the Technical menu in the Settings, in order to preserve storage space of your Odoo database.") + # contextual action + ref_ir_act_window = fields.Many2one('ir.actions.act_window', 'Sidebar action', readonly=True, copy=False, + help="Sidebar action to make this template available on records " + "of the related document model") def unlink(self): self.unlink_action() @@ -178,9 +176,9 @@ class MailTemplate(models.Model): return True - # ---------------------------------------- + # ------------------------------------------------------------ # RENDERING - # ---------------------------------------- + # ------------------------------------------------------------ @api.model def render_post_process(self, html): @@ -246,6 +244,10 @@ class MailTemplate(models.Model): return multi_mode and results or results[res_ids[0]] + # ------------------------------------------------------------ + # MESSAGE/EMAIL VALUES GENERATION + # ------------------------------------------------------------ + def get_email_template(self, res_ids): multi_mode = True if isinstance(res_ids, int): @@ -393,9 +395,9 @@ class MailTemplate(models.Model): return multi_mode and results or results[res_ids[0]] - # ---------------------------------------- + # ------------------------------------------------------------ # EMAIL - # ---------------------------------------- + # ------------------------------------------------------------ def _send_check_access(self, res_ids): records = self.env[self.model].browse(res_ids) diff --git a/addons/sms/models/sms_template.py b/addons/sms/models/sms_template.py index d09dd26a703..46c8841dc2a 100644 --- a/addons/sms/models/sms_template.py +++ b/addons/sms/models/sms_template.py @@ -38,6 +38,10 @@ class SMSTemplate(models.Model): name=_("%s (copy)") % self.name) return super(SMSTemplate, self).copy(default=default) + def unlink(self): + self.sudo().mapped('sidebar_action_id').unlink() + return super(SMSTemplate, self).unlink() + def action_create_sidebar_action(self): ActWindow = self.env['ir.actions.act_window'] view = self.env.ref('sms.sms_composer_view_form') @@ -64,6 +68,10 @@ class SMSTemplate(models.Model): template.sidebar_action_id.unlink() return True + # ------------------------------------------------------------ + # RENDERING + # ------------------------------------------------------------ + def _get_context_lang_per_id(self, res_ids): self.ensure_one() if res_ids is None: @@ -74,7 +82,8 @@ class SMSTemplate(models.Model): results = dict((res_id, self.with_context(lang=lang)) for res_id in res_ids) else: rendered_langs = self._render_template(self.lang, self.model, res_ids) - results = dict((res_id, self.with_context(lang=lang) if lang else self) + results = dict( + (res_id, self.with_context(lang=lang) if lang else self) for res_id, lang in rendered_langs.items()) return results @@ -103,7 +112,3 @@ class SMSTemplate(models.Model): def _render_template(self, template_txt, model, res_ids): """ Render the jinja template """ return self.env['mail.template']._render_template(template_txt, model, res_ids) - - def unlink(self): - self.sudo().mapped('sidebar_action_id').unlink() - return super(SMSTemplate, self).unlink() From a03f6483d6be83a81eac6e8f667ce317c8007dbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 5 Apr 2019 09:15:54 +0000 Subject: [PATCH 4/9] [REF] mail: clean code about rendering in mail_template PURPOSE Clean and rename code about language management and template rendering in mail template model. SPECIFICATIONS Clean method naming and try to make code easier to understand and call. Also add and/or clean docstrings of rendering methods. Notably * ``_classify_per_lang``: for each lang-contextualized template, give the list of record ids; * ``_render_template``: now working only on a valid list of IDs instead of allowing both int / list and having a return type depending on the input type. It allows to simplify code and delegate some processing to callers; Introduce new API method * ``_render_lang``: for each record id return the lang matching it; * ``_render_field``: render a field of mail.template, on given set of record ids. Usage: template._render_field('body_html', records.ids). Language computation is available for this method; Remove the "multi mode" support of rendering that either returned a rendered value, either a dict based on given ids. Now all methods always work in batch and caller have to fetch the correct result if necessary. LINKS Task ID 1963529 Community PR odoo/odoo#32397 --- addons/account/models/account_move.py | 10 +- addons/digest/models/digest.py | 3 +- addons/gamification/models/challenge.py | 8 +- addons/gamification/models/goal.py | 5 +- addons/l10n_ch/models/mail_template.py | 25 +- addons/mail/models/mail_template.py | 262 +++++++++++------- addons/mail/models/mail_thread.py | 3 +- addons/mass_mailing/models/mail_template.py | 17 +- addons/portal/__init__.py | 6 +- addons/purchase/models/purchase.py | 2 +- addons/sale/models/sale.py | 2 +- addons/survey/wizard/survey_invite.py | 4 +- addons/website_slides/models/slide_slide.py | 4 +- .../wizard/slide_channel_invite.py | 4 +- 14 files changed, 213 insertions(+), 142 deletions(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index bde55d46f4c..d6bb81c206b 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -2242,11 +2242,11 @@ class AccountMove(models.Model): """ self.ensure_one() template = self.env.ref('account.email_template_edi_invoice', raise_if_not_found=False) - lang = get_lang(self.env) - if template and template.lang: - lang = template._render_template(template.lang, 'account.move', self.id) - else: - lang = lang.code + lang = False + if template: + lang = template._render_lang(self.ids)[self.id] + if not lang: + lang = get_lang(self.env).code compose_form = self.env.ref('account.account_invoice_send_wizard_form', raise_if_not_found=False) ctx = dict( default_model='account.move', diff --git a/addons/digest/models/digest.py b/addons/digest/models/digest.py index 432625588a8..83a869361c6 100644 --- a/addons/digest/models/digest.py +++ b/addons/digest/models/digest.py @@ -136,7 +136,8 @@ class Digest(models.Model): return False tip.user_ids += user body = tools.html_sanitize(tip.tip_description) - tip_description = self.env['mail.template']._render_template(body, 'digest.tip', self.id) + # FIXME: sanitize ? links , + tip_description = self.env['mail.template']._render_template(body, 'digest.tip', self.ids)[self.id] return tip_description def compute_kpis_actions(self, company, user): diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index 19aa131270e..c424a8c4b28 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -551,11 +551,10 @@ class Challenge(models.Model): challenge = self - MailTemplates = self.env['mail.template'] if challenge.visibility_mode == 'ranking': lines_boards = challenge._get_serialized_challenge_lines(restrict_goals=subset_goals) - body_html = MailTemplates.with_context(challenge_lines=lines_boards)._render_template(challenge.report_template_id.body_html, 'gamification.challenge', challenge.id) + body_html = challenge.report_template_id.with_context(challenge_lines=lines_boards)._render_field('body_html', challenge.ids)[challenge.id] # send to every follower and participant of the challenge challenge.message_post( @@ -576,10 +575,7 @@ class Challenge(models.Model): if not lines: continue - body_html = MailTemplates.with_user(user).with_context(challenge_lines=lines)._render_template( - challenge.report_template_id.body_html, - 'gamification.challenge', - challenge.id) + body_html = challenge.report_template_id.with_user(user).with_context(challenge_lines=lines)._render_field('body_html', challenge.ids)[challenge.id] # notify message only to users, do not post on the challenge challenge.message_notify( diff --git a/addons/gamification/models/goal.py b/addons/gamification/models/goal.py index d6dd9e5a919..3d528a5cab8 100644 --- a/addons/gamification/models/goal.py +++ b/addons/gamification/models/goal.py @@ -213,10 +213,7 @@ class Goal(models.Model): return {} # generate a reminder report - template = self.env.ref('gamification.email_template_goal_reminder')\ - .get_email_template(self.id) - body_html = self.env['mail.template'].with_context(template._context)\ - ._render_template(template.body_html, 'gamification.goal', self.id) + body_html = self.env.ref('gamification.email_template_goal_reminder')._render_field('body_html', self.ids, compute_lang=True)[self.id] self.message_notify( body=body_html, partner_ids=[self.user_id.partner_id.id], diff --git a/addons/l10n_ch/models/mail_template.py b/addons/l10n_ch/models/mail_template.py index b871078bde9..b532acc9b23 100644 --- a/addons/l10n_ch/models/mail_template.py +++ b/addons/l10n_ch/models/mail_template.py @@ -16,28 +16,27 @@ class MailTemplate(models.Model): present on the invoice. Otherwise, no ISR attachment will be created, and the mail will only contain the invoice (as defined in the mother method). """ - rslt = super(MailTemplate, self).generate_email(res_ids, fields) + result = super(MailTemplate, self).generate_email(res_ids, fields) + if self.model != 'account.move': + return result multi_mode = True if isinstance(res_ids, int): res_ids = [res_ids] multi_mode = False - res_ids_to_templates = self.get_email_template(res_ids) - for res_id in res_ids: - related_model = self.env[self.model_id.model].browse(res_id) + for record in self.env[self.model].browse(res_ids): + if record.l10n_ch_isr_valid: + # We add an attachment containing the ISR + report_name = 'ISR-' + self._render_field('report_name', record.ids, compute_lang=True)[record.id] + '.pdf' - if related_model._name == 'account.move' and related_model.l10n_ch_isr_valid: - #We add an attachment containing the ISR - template = res_ids_to_templates[res_id] - report_name = 'ISR-' + self._render_template(template.report_name, template.model, res_id) + '.pdf' - - pdf = self.env.ref('l10n_ch.l10n_ch_isr_report').render_qweb_pdf([res_id])[0] + pdf = self.env.ref('l10n_ch.l10n_ch_isr_report').render_qweb_pdf(record.ids)[0] pdf = base64.b64encode(pdf) - attachments_list = multi_mode and rslt[res_id].get('attachments', False) or rslt.get('attachments', False) + record_dict = multi_mode and result[record.id] or result + attachments_list = record_dict.get('attachments', False) if attachments_list: attachments_list.append((report_name, pdf)) else: - rslt[res_id]['attachments'] = [(report_name, pdf)] - return rslt + record_dict['attachments'] = [(report_name, pdf)] + return result diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index 466d5bbf8eb..df14bfef5f7 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -39,7 +39,7 @@ try: # This is done on purpose: it prevents incidental or malicious execution of # Python code that may break the security of the server. from jinja2.sandbox import SandboxedEnvironment - mako_template_env = SandboxedEnvironment( + jinja_template_env = SandboxedEnvironment( block_start_string="<%", block_end_string="%>", variable_start_string="${", @@ -51,7 +51,7 @@ try: trim_blocks=True, # do not output newline after blocks autoescape=True, # XML/HTML automatic escaping ) - mako_template_env.globals.update({ + jinja_template_env.globals.update({ 'str': str, 'quote': urls.url_quote, 'urlencode': urls.url_encode, @@ -71,8 +71,8 @@ try: # is needed, apparently. 'relativedelta': lambda *a, **kw : relativedelta.relativedelta(*a, **kw), }) - mako_safe_template_env = copy.copy(mako_template_env) - mako_safe_template_env.autoescape = False + jinja_safe_template_env = copy.copy(jinja_template_env) + jinja_safe_template_env.autoescape = False except ImportError: _logger.warning("jinja2 not available, templating features will not work!") @@ -181,53 +181,62 @@ class MailTemplate(models.Model): # ------------------------------------------------------------ @api.model - def render_post_process(self, html): - html = self.env['mail.thread']._replace_local_links(html) - return html + def _render_jinja_eval_context(self): + """ Prepare jinja evaluation context, containing for all rendering - @api.model - def _render_template(self, template_txt, model, res_ids, post_process=False): - """ Render the given template text, replace mako expressions ``${expr}`` - with the result of evaluating these expressions with an evaluation - context containing: - - - ``user``: Model of the current user - - ``object``: record of the document record this mail is related to - - ``context``: the context passed to the mail composition wizard - - :param str template_txt: the template text to render - :param str model: model name of the document record this mail is related to. - :param int res_ids: list of ids of document records those mails are related to. + * ``user``: current user browse record; + * ``ctx```: current context, named ctx to avoid clash with jinja + internals that already uses context; + * various formatting tools; """ - multi_mode = True - if isinstance(res_ids, int): - multi_mode = False - res_ids = [res_ids] - - results = dict.fromkeys(res_ids, u"") - - # try to load the template - try: - mako_env = mako_safe_template_env if self.env.context.get('safe') else mako_template_env - template = mako_env.from_string(tools.ustr(template_txt)) - except Exception: - _logger.info("Failed to load template %r", template_txt, exc_info=True) - return multi_mode and results or results[res_ids[0]] - - # prepare template variables - records = self.env[model].browse(it for it in res_ids if it) # filter to avoid browsing [None] - res_to_rec = dict.fromkeys(res_ids, None) - for record in records: - res_to_rec[record.id] = record - variables = { + render_context = { 'format_date': lambda date, date_format=False, lang_code=False: format_date(self.env, date, date_format, lang_code), 'format_datetime': lambda dt, tz=False, dt_format=False, lang_code=False: format_datetime(self.env, dt, tz, dt_format, lang_code), 'format_amount': lambda amount, currency, lang_code=False: tools.format_amount(self.env, amount, currency, lang_code), 'format_duration': lambda value: tools.format_duration(value), 'user': self.env.user, - 'ctx': self._context, # context kw would clash with mako internals + 'ctx': self._context, } - for res_id, record in res_to_rec.items(): + return render_context + + @api.model + def _render_template_jinja(self, template_txt, model, res_ids): + """ Render a string-based template on records given by a model and a list + of IDs, using jinja. + + In addition to the generic evaluation context given by _render_jinja_eval_context + some new variables are added, depending on each record + + * ``object``: record based on which the template is rendered; + + :param str template_txt: template text to render + :param str model: model name of records on which we want to perform rendering + :param list res_ids: list of ids of records (all belonging to same model) + + :return dict: {res_id: string of rendered template based on record} + """ + # TDE FIXME: remove that brol (6dde919bb9850912f618b561cd2141bffe41340c) + no_autoescape = self._context.get('safe') + results = dict.fromkeys(res_ids, u"") + if not template_txt: + return results + + # try to load the template + try: + jinja_env = jinja_safe_template_env if no_autoescape else jinja_template_env + template = jinja_env.from_string(tools.ustr(template_txt)) + except Exception: + _logger.info("Failed to load template %r", template_txt, exc_info=True) + return results + + # prepare template variables + variables = self._render_jinja_eval_context() + # TDE CHECKME + # records = self.env[model].browse(it for it in res_ids if it) # filter to avoid browsing [None] + if any(r is None for r in res_ids): + raise ValueError(_('Unsuspected None')) + + for record in self.env[model].browse(res_ids): variables['object'] = record try: render_result = template.render(variables) @@ -236,47 +245,124 @@ class MailTemplate(models.Model): raise UserError(_("Failed to render template : %s") % e) if render_result == u"False": render_result = u"" - results[res_id] = render_result + results[record.id] = render_result + return results + + @api.model + def _render_template_postprocess(self, rendered): + """ Tool method for post processing. In this method we ensure local + links ('/shop/Basil-1') are replaced by global links ('https://www. + mygardin.com/hop/Basil-1'). + + :param rendered: result of ``_render_template`` + + :return dict: updated version of rendered + """ + for res_id, html in rendered.items(): + rendered[res_id] = self.env['mail.thread']._replace_local_links(html) + return rendered + + @api.model + def _render_template(self, template_txt, model, res_ids, engine='jinja', post_process=False): + """ Render the given string on records designed by model / res_ids using + the given rendering engine. Currently only jinja is supported. + + :param str template_txt: template text to render + :param str model: model name of records on which we want to perform rendering + :param list res_ids: list of ids of records (all belonging to same model) + :param string engine: jinja + :param post_process: perform rendered str / html post processing (see + ``_render_template_postprocess``) + + :return dict: {res_id: string of rendered template based on record} + """ + if not isinstance(res_ids, (list, tuple)): + raise ValueError(_('Template rendering should be called only using on a list of IDs.')) + if engine != 'jinja': + raise ValueError(_('Template rendering supports only jinja.')) + + rendered = self._render_template_jinja(template_txt, model, res_ids) if post_process: - for res_id, result in results.items(): - results[res_id] = self.render_post_process(result) + rendered = self._render_template_postprocess(rendered) - return multi_mode and results or results[res_ids[0]] + return rendered + + def _render_lang(self, res_ids): + """ Given some record ids, return the lang for each record based on + lang field of template or through specific context-based key. + + :param list res_ids: list of ids of records (all belonging to same model + defined by self.model) + + :return dict: {res_id: lang code (i.e. en_US)} + """ + self.ensure_one() + if not isinstance(res_ids, (list, tuple)): + raise ValueError(_('Template rendering for language should be called with a list of IDs.')) + + if self.env.context.get('template_preview_lang'): + return dict((res_id, self.env.context['template_preview_lang']) for res_id in res_ids) + else: + rendered_langs = self._render_template(self.lang, self.model, res_ids) + return dict((res_id, lang) + for res_id, lang in rendered_langs.items()) + + def _classify_per_lang(self, res_ids): + """ Given some record ids, return for computed each lang a contextualized + template and its subset of res_ids. + + :param list res_ids: list of ids of records (all belonging to same model + defined by self.model) + + :return dict: {lang: (template with lang=lang_code if specific lang computed + or template, res_ids targeted by that language} + """ + self.ensure_one() + + lang_to_res_ids = {} + for res_id, lang in self._render_lang(res_ids).items(): + lang_to_res_ids.setdefault(lang, []).append(res_id) + + return dict( + (lang, (self.with_context(lang=lang) if lang else self, lang_res_ids)) + for lang, lang_res_ids in lang_to_res_ids.items() + ) + + def _render_field(self, field, res_ids, compute_lang=False, set_lang=False, post_process=False): + """ Given some record ids, render a given field of template rendered on + all records. + + :param list res_ids: list of ids of records (all belonging to same model + defined by self.model) + :param compute_lang: compute rendering language based on template.lang + :param set_lang: force language + :param post_process: perform rendered str / html post processing (see + ``_render_template_postprocess``) + + :return dict: {res_id: string of rendered template based on record} + """ + self.ensure_one() + if compute_lang: + templates_res_ids = self._classify_per_lang(res_ids) + elif set_lang: + templates_res_ids = {set_lang: (self.with_context(lang=set_lang), res_ids)} + else: + templates_res_ids = {self._context.get('lang'): (self, res_ids)} + + return dict( + (res_id, rendered) + for lang, (template, tpl_res_ids) in templates_res_ids.items() + for res_id, rendered in template._render_template( + template[field], template.model, tpl_res_ids, + post_process=post_process + ).items() + ) # ------------------------------------------------------------ # MESSAGE/EMAIL VALUES GENERATION # ------------------------------------------------------------ - def get_email_template(self, res_ids): - multi_mode = True - if isinstance(res_ids, int): - res_ids = [res_ids] - multi_mode = False - - if res_ids is None: - res_ids = [None] - results = dict.fromkeys(res_ids, False) - - if not self.ids: - return results - self.ensure_one() - - if self.env.context.get('template_preview_lang'): - lang = self.env.context.get('template_preview_lang') - for res_id in res_ids: - results[res_id] = self.with_context(lang=lang) - else: - langs = self._render_template(self.lang, self.model, res_ids) - for res_id, lang in langs.items(): - if lang: - template = self.with_context(lang=lang) - else: - template = self - results[res_id] = template - - return multi_mode and results or results[res_ids[0]] - def generate_recipients(self, results, res_ids): """Generates the recipients of the template. Default values can ben generated instead of the template values if requested by template or context. @@ -330,24 +416,14 @@ class MailTemplate(models.Model): res_ids = [res_ids] multi_mode = False - res_ids_to_templates = self.get_email_template(res_ids) - - # templates: res_id -> template; template -> res_ids - templates_to_res_ids = {} - for res_id, template in res_ids_to_templates.items(): - templates_to_res_ids.setdefault(template, []).append(res_id) - results = dict() - for template, template_res_ids in templates_to_res_ids.items(): - Template = self.env['mail.template'] - # generate fields value for all res_ids linked to the current template - if template.lang: - Template = Template.with_context(lang=template._context.get('lang')) + for lang, (template, template_res_ids) in self._classify_per_lang(res_ids).items(): for field in fields: - Template = Template.with_context(safe=field in {'subject'}) - generated_field_values = Template._render_template( - getattr(template, field), template.model, template_res_ids, - post_process=(field == 'body_html')) + template = template.with_context(safe=(field == 'subject')) + generated_field_values = template._render_field( + field, template_res_ids, + post_process=(field == 'body_html') + ) for res_id, field_value in generated_field_values.items(): results.setdefault(res_id, dict())[field] = field_value # compute recipients @@ -371,7 +447,7 @@ class MailTemplate(models.Model): if template.report_template: for res_id in template_res_ids: attachments = [] - report_name = self._render_template(template.report_name, template.model, res_id) + report_name = self._render_field('report_name', [res_id])[res_id] report = template.report_template report_service = report.report_name diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 96a25fef64e..300ab439cf9 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2421,11 +2421,12 @@ class MailThread(models.AbstractModel): # Retrieve the language in which the template was rendered, in order to render the custom # layout in the same language. + # TDE FIXME: this whole brol should be cleaned ! lang = self.env.context.get('lang') if {'default_template_id', 'default_model', 'default_res_id'} <= self.env.context.keys(): template = self.env['mail.template'].browse(self.env.context['default_template_id']) if template and template.lang: - lang = template._render_template(template.lang, self.env.context['default_model'], self.env.context['default_res_id']) + lang = template._render_lang([self.env.context['default_res_id']])[self.env.context['default_res_id']] if not model_description and model: model_description = self.env['ir.model'].with_context(lang=lang)._get(model).display_name diff --git a/addons/mass_mailing/models/mail_template.py b/addons/mass_mailing/models/mail_template.py index 68005ee0d10..93b6a0a0145 100644 --- a/addons/mass_mailing/models/mail_template.py +++ b/addons/mass_mailing/models/mail_template.py @@ -8,15 +8,16 @@ class MailTemplate(models.Model): _inherit = "mail.template" @api.model - def render_post_process(self, html): + def _render_template_postprocess(self, rendered): # super will transform relative url to absolute - html = super(MailTemplate, self).render_post_process(html) + rendered = super(MailTemplate, self)._render_template_postprocess(rendered) # 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 + for res_id, html in rendered.items(): + rendered[res_id] = self.env['link.tracker'].convert_links( + html, + self.env.context['post_convert_links'], + blacklist=['/unsubscribe_from_list'] + ) + return rendered diff --git a/addons/portal/__init__.py b/addons/portal/__init__.py index d1a0c30eafd..db7d3b368c8 100644 --- a/addons/portal/__init__.py +++ b/addons/portal/__init__.py @@ -3,14 +3,14 @@ # Updating mako environement in order to be able to use slug try: - from odoo.addons.mail.models.mail_template import mako_template_env, mako_safe_template_env + from odoo.addons.mail.models.mail_template import jinja_template_env, jinja_safe_template_env from odoo.addons.http_routing.models.ir_http import slug - mako_template_env.globals.update({ + jinja_template_env.globals.update({ 'slug': slug, }) - mako_safe_template_env.globals.update({ + jinja_safe_template_env.globals.update({ 'slug': slug, }) except ImportError: diff --git a/addons/purchase/models/purchase.py b/addons/purchase/models/purchase.py index aca1186d7ae..3fe6f3389f6 100644 --- a/addons/purchase/models/purchase.py +++ b/addons/purchase/models/purchase.py @@ -292,7 +292,7 @@ class PurchaseOrder(models.Model): if {'default_template_id', 'default_model', 'default_res_id'} <= ctx.keys(): template = self.env['mail.template'].browse(ctx['default_template_id']) if template and template.lang: - lang = template._render_template(template.lang, ctx['default_model'], ctx['default_res_id']) + lang = template._render_lang([ctx['default_res_id']])[ctx['default_res_id']] self = self.with_context(lang=lang) if self.state in ['draft', 'sent']: diff --git a/addons/sale/models/sale.py b/addons/sale/models/sale.py index ca0b98ef8e0..0e749c76e75 100644 --- a/addons/sale/models/sale.py +++ b/addons/sale/models/sale.py @@ -751,7 +751,7 @@ Reason(s) of this behavior could be: lang = self.env.context.get('lang') template = self.env['mail.template'].browse(template_id) if template.lang: - lang = template._render_template(template.lang, 'sale.order', self.ids[0]) + lang = template._render_lang(self.ids)[self.id] ctx = { 'default_model': 'sale.order', 'default_res_id': self.ids[0], diff --git a/addons/survey/wizard/survey_invite.py b/addons/survey/wizard/survey_invite.py index 241be409333..3450bd4f8cb 100644 --- a/addons/survey/wizard/survey_invite.py +++ b/addons/survey/wizard/survey_invite.py @@ -199,8 +199,8 @@ class SurveyInvite(models.TransientModel): def _send_mail(self, answer): """ Create mail specific for recipient containing notably its access token """ - subject = self.env['mail.template']._render_template(self.subject, 'survey.user_input', answer.id, post_process=True) - body = self.env['mail.template']._render_template(self.body, 'survey.user_input', answer.id, post_process=True) + subject = self.env['mail.template']._render_template(self.subject, 'survey.user_input', answer.ids, post_process=True)[answer.id] + body = self.env['mail.template']._render_template(self.body, 'survey.user_input', answer.ids, post_process=True)[answer.id] # post the message mail_values = { 'email_from': self.email_from, diff --git a/addons/website_slides/models/slide_slide.py b/addons/website_slides/models/slide_slide.py index ff3ac20bb24..a399b4411f0 100644 --- a/addons/website_slides/models/slide_slide.py +++ b/addons/website_slides/models/slide_slide.py @@ -499,8 +499,8 @@ class Slide(models.Model): base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url') for slide in self.filtered(lambda slide: slide.website_published and slide.channel_id.publish_template_id): publish_template = slide.channel_id.publish_template_id - html_body = publish_template.with_context(base_url=base_url)._render_template(publish_template.body_html, 'slide.slide', slide.id) - subject = publish_template._render_template(publish_template.subject, 'slide.slide', slide.id) + html_body = publish_template.with_context(base_url=base_url)._render_field('body_html', slide.ids)[slide.id] + subject = publish_template._render_field('subject', slide.ids)[slide.id] slide.channel_id.with_context(mail_create_nosubscribe=True).message_post( subject=subject, body=html_body, diff --git a/addons/website_slides/wizard/slide_channel_invite.py b/addons/website_slides/wizard/slide_channel_invite.py index 43e384ade30..6dc17de6f94 100644 --- a/addons/website_slides/wizard/slide_channel_invite.py +++ b/addons/website_slides/wizard/slide_channel_invite.py @@ -95,8 +95,8 @@ class SlideChannelInvite(models.TransientModel): def _prepare_mail_values(self, slide_channel_partner): """ Create mail specific for recipient """ - subject = self.env['mail.template']._render_template(self.subject, 'slide.channel.partner', slide_channel_partner.id, post_process=True) - body = self.env['mail.template']._render_template(self.body, 'slide.channel.partner', slide_channel_partner.id, post_process=True) + subject = self.env['mail.template']._render_template(self.subject, 'slide.channel.partner', slide_channel_partner.ids, post_process=True)[slide_channel_partner.id] + body = self.env['mail.template']._render_template(self.body, 'slide.channel.partner', slide_channel_partner.ids, post_process=True)[slide_channel_partner.id] # post the message mail_values = { 'email_from': self.email_from, From 822f468059c9702b0898016e7bfbaf036a2c4036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 25 Feb 2020 11:49:27 +0000 Subject: [PATCH 5/9] [MOV] mail: move rendering code to mail render mixin We also have to update code calling directly the rendering itself. Indeed some code bits does some rendering directly on jinja-enabled input and not through templates. Those calls have to be updated accordingly. LINKS Task ID 1963529 Community PR odoo/odoo#32397 --- addons/digest/models/digest.py | 4 +- addons/mail/models/mail_render_mixin.py | 186 +++++++++++++++++- addons/mail/models/mail_template.py | 174 ---------------- addons/mail/wizard/mail_compose_message.py | 8 +- addons/mass_mailing/models/__init__.py | 2 +- ...{mail_template.py => mail_render_mixin.py} | 6 +- addons/portal/__init__.py | 2 +- addons/sms/models/sms_template.py | 5 - addons/sms/wizard/sms_composer.py | 2 +- addons/survey/wizard/survey_invite.py | 4 +- .../wizard/slide_channel_invite.py | 4 +- 11 files changed, 201 insertions(+), 196 deletions(-) rename addons/mass_mailing/models/{mail_template.py => mail_render_mixin.py} (79%) diff --git a/addons/digest/models/digest.py b/addons/digest/models/digest.py index 83a869361c6..8e5623f4dad 100644 --- a/addons/digest/models/digest.py +++ b/addons/digest/models/digest.py @@ -136,8 +136,8 @@ class Digest(models.Model): return False tip.user_ids += user body = tools.html_sanitize(tip.tip_description) - # FIXME: sanitize ? links , - tip_description = self.env['mail.template']._render_template(body, 'digest.tip', self.ids)[self.id] + # FIXME: sanitize ? links ? + tip_description = self.env['mail.render.mixin']._render_template(body, 'digest.tip', self.ids)[self.id] return tip_description def compute_kpis_actions(self, company, user): diff --git a/addons/mail/models/mail_render_mixin.py b/addons/mail/models/mail_render_mixin.py index eb5bcbef882..cbb70d3aacf 100644 --- a/addons/mail/models/mail_render_mixin.py +++ b/addons/mail/models/mail_render_mixin.py @@ -1,7 +1,79 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import api, fields, models +import babel +import copy +import datetime +import dateutil.relativedelta as relativedelta +import functools +import logging + +from werkzeug import urls + +from odoo import _, api, fields, models, tools +from odoo.exceptions import UserError + +_logger = logging.getLogger(__name__) + + +def format_date(env, date, pattern=False, lang_code=False): + try: + return tools.format_date(env, date, date_format=pattern, lang_code=lang_code) + except babel.core.UnknownLocaleError: + return date + + +def format_datetime(env, dt, tz=False, dt_format='medium', lang_code=False): + try: + return tools.format_datetime(env, dt, tz=tz, dt_format=dt_format, lang_code=lang_code) + except babel.core.UnknownLocaleError: + return dt + +try: + # We use a jinja2 sandboxed environment to render mako templates. + # Note that the rendering does not cover all the mako syntax, in particular + # arbitrary Python statements are not accepted, and not all expressions are + # allowed: only "public" attributes (not starting with '_') of objects may + # be accessed. + # This is done on purpose: it prevents incidental or malicious execution of + # Python code that may break the security of the server. + from jinja2.sandbox import SandboxedEnvironment + jinja_template_env = SandboxedEnvironment( + block_start_string="<%", + block_end_string="%>", + variable_start_string="${", + variable_end_string="}", + comment_start_string="<%doc>", + comment_end_string="", + line_statement_prefix="%", + line_comment_prefix="##", + trim_blocks=True, # do not output newline after blocks + autoescape=True, # XML/HTML automatic escaping + ) + jinja_template_env.globals.update({ + 'str': str, + 'quote': urls.url_quote, + 'urlencode': urls.url_encode, + 'datetime': datetime, + 'len': len, + 'abs': abs, + 'min': min, + 'max': max, + 'sum': sum, + 'filter': filter, + 'reduce': functools.reduce, + 'map': map, + 'round': round, + + # dateutil.relativedelta is an old-style class and cannot be directly + # instanciated wihtin a jinja2 expression, so a lambda "proxy" is + # is needed, apparently. + 'relativedelta': lambda *a, **kw : relativedelta.relativedelta(*a, **kw), + }) + jinja_safe_template_env = copy.copy(jinja_template_env) + jinja_safe_template_env.autoescape = False +except ImportError: + _logger.warning("jinja2 not available, templating features will not work!") class MailRenderMixin(models.AbstractModel): @@ -66,3 +138,115 @@ class MailRenderMixin(models.AbstractModel): expression += " or '''%s'''" % null_value expression += "}" return expression + + # ------------------------------------------------------------ + # RENDERING + # ------------------------------------------------------------ + + @api.model + def _render_jinja_eval_context(self): + """ Prepare jinja evaluation context, containing for all rendering + + * ``user``: current user browse record; + * ``ctx```: current context, named ctx to avoid clash with jinja + internals that already uses context; + * various formatting tools; + """ + render_context = { + 'format_date': lambda date, date_format=False, lang_code=False: format_date(self.env, date, date_format, lang_code), + 'format_datetime': lambda dt, tz=False, dt_format=False, lang_code=False: format_datetime(self.env, dt, tz, dt_format, lang_code), + 'format_amount': lambda amount, currency, lang_code=False: tools.format_amount(self.env, amount, currency, lang_code), + 'format_duration': lambda value: tools.format_duration(value), + 'user': self.env.user, + 'ctx': self._context, + } + return render_context + + @api.model + def _render_template_jinja(self, template_txt, model, res_ids): + """ Render a string-based template on records given by a model and a list + of IDs, using jinja. + + In addition to the generic evaluation context given by _render_jinja_eval_context + some new variables are added, depending on each record + + * ``object``: record based on which the template is rendered; + + :param str template_txt: template text to render + :param str model: model name of records on which we want to perform rendering + :param list res_ids: list of ids of records (all belonging to same model) + + :return dict: {res_id: string of rendered template based on record} + """ + # TDE FIXME: remove that brol (6dde919bb9850912f618b561cd2141bffe41340c) + no_autoescape = self._context.get('safe') + results = dict.fromkeys(res_ids, u"") + if not template_txt: + return results + + # try to load the template + try: + jinja_env = jinja_safe_template_env if no_autoescape else jinja_template_env + template = jinja_env.from_string(tools.ustr(template_txt)) + except Exception: + _logger.info("Failed to load template %r", template_txt, exc_info=True) + return results + + # prepare template variables + variables = self._render_jinja_eval_context() + # TDE CHECKME + # records = self.env[model].browse(it for it in res_ids if it) # filter to avoid browsing [None] + if any(r is None for r in res_ids): + raise ValueError(_('Unsuspected None')) + + for record in self.env[model].browse(res_ids): + variables['object'] = record + try: + render_result = template.render(variables) + except Exception as e: + _logger.info("Failed to render template : %s" % e, exc_info=True) + raise UserError(_("Failed to render template : %s") % e) + if render_result == u"False": + render_result = u"" + results[record.id] = render_result + + return results + + @api.model + def _render_template_postprocess(self, rendered): + """ Tool method for post processing. In this method we ensure local + links ('/shop/Basil-1') are replaced by global links ('https://www. + mygardin.com/hop/Basil-1'). + + :param rendered: result of ``_render_template`` + + :return dict: updated version of rendered + """ + for res_id, html in rendered.items(): + rendered[res_id] = self.env['mail.thread']._replace_local_links(html) + return rendered + + @api.model + def _render_template(self, template_txt, model, res_ids, engine='jinja', post_process=False): + """ Render the given string on records designed by model / res_ids using + the given rendering engine. Currently only jinja is supported. + + :param str template_txt: template text to render + :param str model: model name of records on which we want to perform rendering + :param list res_ids: list of ids of records (all belonging to same model) + :param string engine: jinja + :param post_process: perform rendered str / html post processing (see + ``_render_template_postprocess``) + + :return dict: {res_id: string of rendered template based on record} + """ + if not isinstance(res_ids, (list, tuple)): + raise ValueError(_('Template rendering should be called only using on a list of IDs.')) + if engine != 'jinja': + raise ValueError(_('Template rendering supports only jinja.')) + + rendered = self._render_template_jinja(template_txt, model, res_ids) + if post_process: + rendered = self._render_template_postprocess(rendered) + + return rendered diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index df14bfef5f7..72183aabf01 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -1,15 +1,9 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import babel import base64 -import copy -import datetime -import dateutil.relativedelta as relativedelta -import functools import logging -from werkzeug import urls from odoo import _, api, fields, models, tools from odoo.exceptions import UserError @@ -17,66 +11,6 @@ from odoo.exceptions import UserError _logger = logging.getLogger(__name__) -def format_date(env, date, pattern=False, lang_code=False): - try: - return tools.format_date(env, date, date_format=pattern, lang_code=lang_code) - except babel.core.UnknownLocaleError: - return date - - -def format_datetime(env, dt, tz=False, dt_format='medium', lang_code=False): - try: - return tools.format_datetime(env, dt, tz=tz, dt_format=dt_format, lang_code=lang_code) - except babel.core.UnknownLocaleError: - return dt - -try: - # We use a jinja2 sandboxed environment to render mako templates. - # Note that the rendering does not cover all the mako syntax, in particular - # arbitrary Python statements are not accepted, and not all expressions are - # allowed: only "public" attributes (not starting with '_') of objects may - # be accessed. - # This is done on purpose: it prevents incidental or malicious execution of - # Python code that may break the security of the server. - from jinja2.sandbox import SandboxedEnvironment - jinja_template_env = SandboxedEnvironment( - block_start_string="<%", - block_end_string="%>", - variable_start_string="${", - variable_end_string="}", - comment_start_string="<%doc>", - comment_end_string="", - line_statement_prefix="%", - line_comment_prefix="##", - trim_blocks=True, # do not output newline after blocks - autoescape=True, # XML/HTML automatic escaping - ) - jinja_template_env.globals.update({ - 'str': str, - 'quote': urls.url_quote, - 'urlencode': urls.url_encode, - 'datetime': datetime, - 'len': len, - 'abs': abs, - 'min': min, - 'max': max, - 'sum': sum, - 'filter': filter, - 'reduce': functools.reduce, - 'map': map, - 'round': round, - - # dateutil.relativedelta is an old-style class and cannot be directly - # instanciated wihtin a jinja2 expression, so a lambda "proxy" is - # is needed, apparently. - 'relativedelta': lambda *a, **kw : relativedelta.relativedelta(*a, **kw), - }) - jinja_safe_template_env = copy.copy(jinja_template_env) - jinja_safe_template_env.autoescape = False -except ImportError: - _logger.warning("jinja2 not available, templating features will not work!") - - class MailTemplate(models.Model): "Templates for sending email" _name = "mail.template" @@ -180,114 +114,6 @@ class MailTemplate(models.Model): # RENDERING # ------------------------------------------------------------ - @api.model - def _render_jinja_eval_context(self): - """ Prepare jinja evaluation context, containing for all rendering - - * ``user``: current user browse record; - * ``ctx```: current context, named ctx to avoid clash with jinja - internals that already uses context; - * various formatting tools; - """ - render_context = { - 'format_date': lambda date, date_format=False, lang_code=False: format_date(self.env, date, date_format, lang_code), - 'format_datetime': lambda dt, tz=False, dt_format=False, lang_code=False: format_datetime(self.env, dt, tz, dt_format, lang_code), - 'format_amount': lambda amount, currency, lang_code=False: tools.format_amount(self.env, amount, currency, lang_code), - 'format_duration': lambda value: tools.format_duration(value), - 'user': self.env.user, - 'ctx': self._context, - } - return render_context - - @api.model - def _render_template_jinja(self, template_txt, model, res_ids): - """ Render a string-based template on records given by a model and a list - of IDs, using jinja. - - In addition to the generic evaluation context given by _render_jinja_eval_context - some new variables are added, depending on each record - - * ``object``: record based on which the template is rendered; - - :param str template_txt: template text to render - :param str model: model name of records on which we want to perform rendering - :param list res_ids: list of ids of records (all belonging to same model) - - :return dict: {res_id: string of rendered template based on record} - """ - # TDE FIXME: remove that brol (6dde919bb9850912f618b561cd2141bffe41340c) - no_autoescape = self._context.get('safe') - results = dict.fromkeys(res_ids, u"") - if not template_txt: - return results - - # try to load the template - try: - jinja_env = jinja_safe_template_env if no_autoescape else jinja_template_env - template = jinja_env.from_string(tools.ustr(template_txt)) - except Exception: - _logger.info("Failed to load template %r", template_txt, exc_info=True) - return results - - # prepare template variables - variables = self._render_jinja_eval_context() - # TDE CHECKME - # records = self.env[model].browse(it for it in res_ids if it) # filter to avoid browsing [None] - if any(r is None for r in res_ids): - raise ValueError(_('Unsuspected None')) - - for record in self.env[model].browse(res_ids): - variables['object'] = record - try: - render_result = template.render(variables) - except Exception as e: - _logger.info("Failed to render template : %s" % e, exc_info=True) - raise UserError(_("Failed to render template : %s") % e) - if render_result == u"False": - render_result = u"" - results[record.id] = render_result - - return results - - @api.model - def _render_template_postprocess(self, rendered): - """ Tool method for post processing. In this method we ensure local - links ('/shop/Basil-1') are replaced by global links ('https://www. - mygardin.com/hop/Basil-1'). - - :param rendered: result of ``_render_template`` - - :return dict: updated version of rendered - """ - for res_id, html in rendered.items(): - rendered[res_id] = self.env['mail.thread']._replace_local_links(html) - return rendered - - @api.model - def _render_template(self, template_txt, model, res_ids, engine='jinja', post_process=False): - """ Render the given string on records designed by model / res_ids using - the given rendering engine. Currently only jinja is supported. - - :param str template_txt: template text to render - :param str model: model name of records on which we want to perform rendering - :param list res_ids: list of ids of records (all belonging to same model) - :param string engine: jinja - :param post_process: perform rendered str / html post processing (see - ``_render_template_postprocess``) - - :return dict: {res_id: string of rendered template based on record} - """ - if not isinstance(res_ids, (list, tuple)): - raise ValueError(_('Template rendering should be called only using on a list of IDs.')) - if engine != 'jinja': - raise ValueError(_('Template rendering supports only jinja.')) - - rendered = self._render_template_jinja(template_txt, model, res_ids) - if post_process: - rendered = self._render_template_postprocess(rendered) - - return rendered - def _render_lang(self, res_ids): """ Given some record ids, return the lang for each record based on lang field of template or through specific context-based key. diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index fdc7b23508b..6c2e83baaec 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -455,10 +455,10 @@ class MailComposer(models.TransientModel): multi_mode = False res_ids = [res_ids] - subjects = self.env['mail.template']._render_template(self.subject, self.model, res_ids) - bodies = self.env['mail.template']._render_template(self.body, self.model, res_ids, post_process=True) - emails_from = self.env['mail.template']._render_template(self.email_from, self.model, res_ids) - replies_to = self.env['mail.template']._render_template(self.reply_to, self.model, res_ids) + subjects = self.env['mail.render.mixin']._render_template(self.subject, self.model, res_ids) + bodies = self.env['mail.render.mixin']._render_template(self.body, self.model, res_ids, post_process=True) + emails_from = self.env['mail.render.mixin']._render_template(self.email_from, self.model, res_ids) + replies_to = self.env['mail.render.mixin']._render_template(self.reply_to, self.model, res_ids) default_recipients = {} if not self.partner_ids: records = self.env[self.model].browse(res_ids).sudo() diff --git a/addons/mass_mailing/models/__init__.py b/addons/mass_mailing/models/__init__.py index 64b451ac37b..035ce2d73cc 100644 --- a/addons/mass_mailing/models/__init__.py +++ b/addons/mass_mailing/models/__init__.py @@ -7,7 +7,7 @@ from . import mailing_list from . import mailing_trace from . import mailing from . import mail_mail -from . import mail_template +from . import mail_render_mixin from . import mail_thread from . import res_config_settings from . import res_users diff --git a/addons/mass_mailing/models/mail_template.py b/addons/mass_mailing/models/mail_render_mixin.py similarity index 79% rename from addons/mass_mailing/models/mail_template.py rename to addons/mass_mailing/models/mail_render_mixin.py index 93b6a0a0145..d38667366ed 100644 --- a/addons/mass_mailing/models/mail_template.py +++ b/addons/mass_mailing/models/mail_render_mixin.py @@ -4,13 +4,13 @@ from odoo import api, models -class MailTemplate(models.Model): - _inherit = "mail.template" +class MailRenderMixin(models.AbstractModel): + _inherit = "mail.render.mixin" @api.model def _render_template_postprocess(self, rendered): # super will transform relative url to absolute - rendered = super(MailTemplate, self)._render_template_postprocess(rendered) + rendered = super(MailRenderMixin, self)._render_template_postprocess(rendered) # apply shortener after if self.env.context.get('post_convert_links'): diff --git a/addons/portal/__init__.py b/addons/portal/__init__.py index db7d3b368c8..72caddbddb3 100644 --- a/addons/portal/__init__.py +++ b/addons/portal/__init__.py @@ -3,7 +3,7 @@ # Updating mako environement in order to be able to use slug try: - from odoo.addons.mail.models.mail_template import jinja_template_env, jinja_safe_template_env + from odoo.addons.mail.models.mail_render_mixin import jinja_template_env, jinja_safe_template_env from odoo.addons.http_routing.models.ir_http import slug jinja_template_env.globals.update({ diff --git a/addons/sms/models/sms_template.py b/addons/sms/models/sms_template.py index 46c8841dc2a..a2b2300051b 100644 --- a/addons/sms/models/sms_template.py +++ b/addons/sms/models/sms_template.py @@ -107,8 +107,3 @@ class SMSTemplate(models.Model): template = self.with_context(lang=lang) all_bodies.update(template._render_template(template.body, self.model, rids)) return all_bodies - - @api.model - def _render_template(self, template_txt, model, res_ids): - """ Render the jinja template """ - return self.env['mail.template']._render_template(template_txt, model, res_ids) diff --git a/addons/sms/wizard/sms_composer.py b/addons/sms/wizard/sms_composer.py index df76d2e9c60..60c915d8172 100644 --- a/addons/sms/wizard/sms_composer.py +++ b/addons/sms/wizard/sms_composer.py @@ -281,7 +281,7 @@ class SendSMS(models.TransientModel): if self.template_id and self.body == self.template_id.body: all_bodies = self.template_id._get_translated_bodies(records.ids) else: - all_bodies = self.env['mail.template']._render_template(self.body, records._name, records.ids) + all_bodies = self.env['mail.render.mixin']._render_template(self.body, records._name, records.ids) return all_bodies def _prepare_mass_sms_values(self, records): diff --git a/addons/survey/wizard/survey_invite.py b/addons/survey/wizard/survey_invite.py index 3450bd4f8cb..0068ecdb06c 100644 --- a/addons/survey/wizard/survey_invite.py +++ b/addons/survey/wizard/survey_invite.py @@ -199,8 +199,8 @@ class SurveyInvite(models.TransientModel): def _send_mail(self, answer): """ Create mail specific for recipient containing notably its access token """ - subject = self.env['mail.template']._render_template(self.subject, 'survey.user_input', answer.ids, post_process=True)[answer.id] - body = self.env['mail.template']._render_template(self.body, 'survey.user_input', answer.ids, post_process=True)[answer.id] + subject = self.env['mail.render.mixin']._render_template(self.subject, 'survey.user_input', answer.ids, post_process=True)[answer.id] + body = self.env['mail.render.mixin']._render_template(self.body, 'survey.user_input', answer.ids, post_process=True)[answer.id] # post the message mail_values = { 'email_from': self.email_from, diff --git a/addons/website_slides/wizard/slide_channel_invite.py b/addons/website_slides/wizard/slide_channel_invite.py index 6dc17de6f94..28522e39c4b 100644 --- a/addons/website_slides/wizard/slide_channel_invite.py +++ b/addons/website_slides/wizard/slide_channel_invite.py @@ -95,8 +95,8 @@ class SlideChannelInvite(models.TransientModel): def _prepare_mail_values(self, slide_channel_partner): """ Create mail specific for recipient """ - subject = self.env['mail.template']._render_template(self.subject, 'slide.channel.partner', slide_channel_partner.ids, post_process=True)[slide_channel_partner.id] - body = self.env['mail.template']._render_template(self.body, 'slide.channel.partner', slide_channel_partner.ids, post_process=True)[slide_channel_partner.id] + subject = self.env['mail.render.mixin']._render_template(self.subject, 'slide.channel.partner', slide_channel_partner.ids, post_process=True)[slide_channel_partner.id] + body = self.env['mail.render.mixin']._render_template(self.body, 'slide.channel.partner', slide_channel_partner.ids, post_process=True)[slide_channel_partner.id] # post the message mail_values = { 'email_from': self.email_from, From 2835e0ba38f67f76b7f43948290050dcc68cd9db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 6 Mar 2020 13:17:20 +0000 Subject: [PATCH 6/9] [MOV] mail, sms: move language computation in render mixin directly PURPOSE Merge rendering tools used in mail and sms templates as well as partially in mailings and move them directly in mail.render.mixin. SPECIFICATIONS Move tools related to language computation from mail.template model to the mail.render.mixin. It allows to update both * mail.template: simply update calls accordingly; * sms.template: remove code doing what is now available through mail.render .mixin inherit; Lang field is moved to mail.render.mixin as it is used notably for the language computation. It therefore adds the field on mailing.mailing model although not used currently. LINKS Task ID 1963529 Community PR odoo/odoo#32397 --- addons/mail/models/mail_render_mixin.py | 78 ++++++++++++++++++ addons/mail/models/mail_template.py | 82 ------------------- addons/sms/models/mail_thread.py | 3 +- addons/sms/models/sms_template.py | 43 ---------- addons/sms/wizard/sms_composer.py | 4 +- addons/sms/wizard/sms_template_preview.py | 7 +- .../test_mail_full/tests/test_sms_template.py | 17 ++-- 7 files changed, 94 insertions(+), 140 deletions(-) diff --git a/addons/mail/models/mail_render_mixin.py b/addons/mail/models/mail_render_mixin.py index cbb70d3aacf..5804481f15c 100644 --- a/addons/mail/models/mail_render_mixin.py +++ b/addons/mail/models/mail_render_mixin.py @@ -80,6 +80,13 @@ class MailRenderMixin(models.AbstractModel): _name = 'mail.render.mixin' _description = 'Mail Render Mixin' + # language for rendering + lang = fields.Char( + 'Language', placeholder="${object.partner_id.lang}", + help="Optional translation language (ISO code) to select when sending out an email. " + "If not set, the english version will be used. This should usually be a placeholder expression " + "that provides the appropriate language, e.g. ${object.partner_id.lang}.") + # expression builder model_object_field = fields.Many2one( 'ir.model.fields', string="Field", store=False, help="Select target field from the related document model.\n" @@ -250,3 +257,74 @@ class MailRenderMixin(models.AbstractModel): rendered = self._render_template_postprocess(rendered) return rendered + + def _render_lang(self, res_ids): + """ Given some record ids, return the lang for each record based on + lang field of template or through specific context-based key. + + :param list res_ids: list of ids of records (all belonging to same model + defined by self.model) + + :return dict: {res_id: lang code (i.e. en_US)} + """ + self.ensure_one() + if not isinstance(res_ids, (list, tuple)): + raise ValueError(_('Template rendering for language should be called with a list of IDs.')) + + if self.env.context.get('template_preview_lang'): + return dict((res_id, self.env.context['template_preview_lang']) for res_id in res_ids) + else: + rendered_langs = self._render_template(self.lang, self.model, res_ids) + return dict((res_id, lang) + for res_id, lang in rendered_langs.items()) + + def _classify_per_lang(self, res_ids): + """ Given some record ids, return for computed each lang a contextualized + template and its subset of res_ids. + + :param list res_ids: list of ids of records (all belonging to same model + defined by self.model) + + :return dict: {lang: (template with lang=lang_code if specific lang computed + or template, res_ids targeted by that language} + """ + self.ensure_one() + + lang_to_res_ids = {} + for res_id, lang in self._render_lang(res_ids).items(): + lang_to_res_ids.setdefault(lang, []).append(res_id) + + return dict( + (lang, (self.with_context(lang=lang) if lang else self, lang_res_ids)) + for lang, lang_res_ids in lang_to_res_ids.items() + ) + + def _render_field(self, field, res_ids, compute_lang=False, set_lang=False, post_process=False): + """ Given some record ids, render a given field of template rendered on + all records. + + :param list res_ids: list of ids of records (all belonging to same model + defined by self.model) + :param compute_lang: compute rendering language based on template.lang + :param set_lang: force language + :param post_process: perform rendered str / html post processing (see + ``_render_template_postprocess``) + + :return dict: {res_id: string of rendered template based on record} + """ + self.ensure_one() + if compute_lang: + templates_res_ids = self._classify_per_lang(res_ids) + elif set_lang: + templates_res_ids = {set_lang: (self.with_context(lang=set_lang), res_ids)} + else: + templates_res_ids = {self._context.get('lang'): (self, res_ids)} + + return dict( + (res_id, rendered) + for lang, (template, tpl_res_ids) in templates_res_ids.items() + for res_id, rendered in template._render_template( + template[field], template.model, tpl_res_ids, + post_process=post_process + ).items() + ) diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index 72183aabf01..3fbe4b03e68 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -29,13 +29,6 @@ class MailTemplate(models.Model): name = fields.Char('Name') model_id = fields.Many2one('ir.model', 'Applies to', help="The type of document this template can be used with") model = fields.Char('Related Document Model', related='model_id.model', index=True, store=True, readonly=True) - lang = fields.Char('Language', - help="Optional translation language (ISO code) to select when sending out an email. " - "If not set, the english version will be used. " - "This should usually be a placeholder expression " - "that provides the appropriate language, e.g. " - "${object.partner_id.lang}.", - placeholder="${object.partner_id.lang}") subject = fields.Char('Subject', translate=True, help="Subject (placeholders may be used here)") email_from = fields.Char('From', help="Sender address (placeholders may be used here). If not set, the default " @@ -110,81 +103,6 @@ class MailTemplate(models.Model): return True - # ------------------------------------------------------------ - # RENDERING - # ------------------------------------------------------------ - - def _render_lang(self, res_ids): - """ Given some record ids, return the lang for each record based on - lang field of template or through specific context-based key. - - :param list res_ids: list of ids of records (all belonging to same model - defined by self.model) - - :return dict: {res_id: lang code (i.e. en_US)} - """ - self.ensure_one() - if not isinstance(res_ids, (list, tuple)): - raise ValueError(_('Template rendering for language should be called with a list of IDs.')) - - if self.env.context.get('template_preview_lang'): - return dict((res_id, self.env.context['template_preview_lang']) for res_id in res_ids) - else: - rendered_langs = self._render_template(self.lang, self.model, res_ids) - return dict((res_id, lang) - for res_id, lang in rendered_langs.items()) - - def _classify_per_lang(self, res_ids): - """ Given some record ids, return for computed each lang a contextualized - template and its subset of res_ids. - - :param list res_ids: list of ids of records (all belonging to same model - defined by self.model) - - :return dict: {lang: (template with lang=lang_code if specific lang computed - or template, res_ids targeted by that language} - """ - self.ensure_one() - - lang_to_res_ids = {} - for res_id, lang in self._render_lang(res_ids).items(): - lang_to_res_ids.setdefault(lang, []).append(res_id) - - return dict( - (lang, (self.with_context(lang=lang) if lang else self, lang_res_ids)) - for lang, lang_res_ids in lang_to_res_ids.items() - ) - - def _render_field(self, field, res_ids, compute_lang=False, set_lang=False, post_process=False): - """ Given some record ids, render a given field of template rendered on - all records. - - :param list res_ids: list of ids of records (all belonging to same model - defined by self.model) - :param compute_lang: compute rendering language based on template.lang - :param set_lang: force language - :param post_process: perform rendered str / html post processing (see - ``_render_template_postprocess``) - - :return dict: {res_id: string of rendered template based on record} - """ - self.ensure_one() - if compute_lang: - templates_res_ids = self._classify_per_lang(res_ids) - elif set_lang: - templates_res_ids = {set_lang: (self.with_context(lang=set_lang), res_ids)} - else: - templates_res_ids = {self._context.get('lang'): (self, res_ids)} - - return dict( - (res_id, rendered) - for lang, (template, tpl_res_ids) in templates_res_ids.items() - for res_id, rendered in template._render_template( - template[field], template.model, tpl_res_ids, - post_process=post_process - ).items() - ) - # ------------------------------------------------------------ # MESSAGE/EMAIL VALUES GENERATION # ------------------------------------------------------------ diff --git a/addons/sms/models/mail_thread.py b/addons/sms/models/mail_thread.py index 86713fddc24..820c5230cfe 100644 --- a/addons/sms/models/mail_thread.py +++ b/addons/sms/models/mail_thread.py @@ -180,8 +180,7 @@ class MailThread(models.AbstractModel): if not template and template_xmlid: template = self.env.ref(template_xmlid, raise_if_not_found=False) if template: - template_w_lang = template._get_context_lang_per_id(self.ids)[self.id] - body = template._render_template(template_w_lang.body, self._name, self.ids)[self.id] + body = template._render_field('body', self.ids, compute_lang=True)[self.id] else: body = self.env['sms.template']._render_template(template_fallback, self._name, self.ids)[self.id] return self._message_sms(body, partner_ids=partner_ids, **kwargs) diff --git a/addons/sms/models/sms_template.py b/addons/sms/models/sms_template.py index a2b2300051b..fb5b8877e87 100644 --- a/addons/sms/models/sms_template.py +++ b/addons/sms/models/sms_template.py @@ -24,9 +24,6 @@ class SMSTemplate(models.Model): help="The type of document this template can be used with", ondelete='cascade') model = fields.Char('Related Document Model', related='model_id.model', index=True, store=True, readonly=True) body = fields.Char('Body', translate=True, required=True) - lang = fields.Char('Language', help="Use this field to either force a specific language (ISO code) or dynamically " - "detect the language of your recipient by a placeholder expression " - "(e.g. ${object.partner_id.lang})") # Use to create contextual action (same as for email template) sidebar_action_id = fields.Many2one('ir.actions.act_window', 'Sidebar action', readonly=True, copy=False, help="Sidebar action to make this template available on records " @@ -67,43 +64,3 @@ class SMSTemplate(models.Model): if template.sidebar_action_id: template.sidebar_action_id.unlink() return True - - # ------------------------------------------------------------ - # RENDERING - # ------------------------------------------------------------ - - def _get_context_lang_per_id(self, res_ids): - self.ensure_one() - if res_ids is None: - return {None: self} - - if self.env.context.get('template_preview_lang'): - lang = self.env.context.get('template_preview_lang') - results = dict((res_id, self.with_context(lang=lang)) for res_id in res_ids) - else: - rendered_langs = self._render_template(self.lang, self.model, res_ids) - results = dict( - (res_id, self.with_context(lang=lang) if lang else self) - for res_id, lang in rendered_langs.items()) - - return results - - def _get_ids_per_lang(self, res_ids): - self.ensure_one() - - rids_to_tpl = self._get_context_lang_per_id(res_ids) - tpl_to_rids = {} - for res_id, template in rids_to_tpl.items(): - tpl_to_rids.setdefault(template._context.get('lang', self.env.user.lang), []).append(res_id) - - return tpl_to_rids - - def _get_translated_bodies(self, res_ids): - """ return sms translated bodies into a dict {'res_id':'body'} """ - self.ensure_one() - lang_to_rids = self._get_ids_per_lang(res_ids) - all_bodies = {} - for lang, rids in lang_to_rids.items(): - template = self.with_context(lang=lang) - all_bodies.update(template._render_template(template.body, self.model, rids)) - return all_bodies diff --git a/addons/sms/wizard/sms_composer.py b/addons/sms/wizard/sms_composer.py index 60c915d8172..7d5cbc9bfd2 100644 --- a/addons/sms/wizard/sms_composer.py +++ b/addons/sms/wizard/sms_composer.py @@ -168,7 +168,7 @@ class SendSMS(models.TransientModel): @api.onchange('composition_mode', 'res_model', 'res_id', 'template_id') def _onchange_template_id(self): if self.template_id and self.composition_mode == 'comment' and self.res_id: - self.body = self.template_id._get_translated_bodies([self.res_id])[self.res_id] + self.body = self.template_id._render_field('body', [self.res_id], compute_lang=True)[self.res_id] elif self.template_id: self.body = self.template_id.body @@ -279,7 +279,7 @@ class SendSMS(models.TransientModel): def _prepare_body_values(self, records): if self.template_id and self.body == self.template_id.body: - all_bodies = self.template_id._get_translated_bodies(records.ids) + all_bodies = self.template_id._render_field('body', records.ids, compute_lang=True) else: all_bodies = self.env['mail.render.mixin']._render_template(self.body, records._name, records.ids) return all_bodies diff --git a/addons/sms/wizard/sms_template_preview.py b/addons/sms/wizard/sms_template_preview.py index 9d00185ee2e..837c0b821b8 100644 --- a/addons/sms/wizard/sms_template_preview.py +++ b/addons/sms/wizard/sms_template_preview.py @@ -45,6 +45,7 @@ class SMSTemplatePreview(models.TransientModel): @api.depends('lang', 'resource_ref') def _compute_sms_template_fields(self): for wizard in self: - # Update body depending of the resource_ref - sms_template = wizard.sms_template_id.with_context(lang=wizard.lang) - wizard.body = sms_template._render_template(sms_template.body, sms_template.model, wizard.resource_ref.id) if wizard.resource_ref else sms_template.body + if wizard.sms_template_id and wizard.resource_ref: + wizard.body = wizard.sms_template_id._render_field('body', [wizard.resource_ref.id], set_lang=wizard.lang)[wizard.resource_ref.id] + else: + wizard.body = wizard.sms_template_id.body diff --git a/addons/test_mail_full/tests/test_sms_template.py b/addons/test_mail_full/tests/test_sms_template.py index a39edf8545f..d688b30d0f7 100644 --- a/addons/test_mail_full/tests/test_sms_template.py +++ b/addons/test_mail_full/tests/test_sms_template.py @@ -50,18 +50,20 @@ class TestSmsTemplate(test_mail_full_common.TestSMSCommon, test_mail_full_common self.assertEqual(self.sms_template.body, self.body_en) self.assertEqual(self.sms_template.with_context(lang='fr_FR').body, self.body_fr) - rid_to_tpl = self.sms_template._get_context_lang_per_id((self.test_record | test_record_2).ids) - self.assertEqual(set(rid_to_tpl.keys()), set((self.test_record | test_record_2).ids)) - for rid, tpl in rid_to_tpl.items(): + rid_to_lang = self.sms_template._render_lang((self.test_record | test_record_2).ids) + self.assertEqual(set(rid_to_lang.keys()), set((self.test_record | test_record_2).ids)) + for rid, lang in rid_to_lang.items(): + # TDE FIXME: False or en_US ? if rid == self.test_record.id: - self.assertEqual(tpl._context.get('lang'), 'en_US') + self.assertEqual(lang, 'en_US') elif rid == test_record_2.id: - self.assertEqual(tpl._context.get('lang'), 'fr_FR') + self.assertEqual(lang, 'fr_FR') else: self.assertTrue(False) - lang_to_rids = self.sms_template._get_ids_per_lang((self.test_record | test_record_2).ids) - for lang, rids in lang_to_rids.items(): + tpl_to_rids = self.sms_template._classify_per_lang((self.test_record | test_record_2).ids) + for lang, (tpl, rids) in tpl_to_rids.items(): + # TDE FIXME: False or en_US ? if lang == 'en_US': self.assertEqual(rids, self.test_record.ids) elif lang == 'fr_FR': @@ -87,4 +89,3 @@ class TestSmsTemplate(test_mail_full_common.TestSMSCommon, test_mail_full_common self.sms_template.unlink() self.assertEqual(ActWindow.search_count([('id', '=', action_id)]), 0) - From 3d86a29aeb21c7e5812c988866117a495f20ebed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 11 Mar 2020 11:29:46 +0000 Subject: [PATCH 7/9] [MOV] mail, various: move replace_local_links to render mixin PURPOSE Move links shortening tools on mail.render.mixin to have rendering and post procesing tools available on that mixin. LINKS Task ID 1963529 Community PR odoo/odoo#32397 --- addons/hr_expense/models/hr_expense.py | 2 +- addons/im_livechat/models/mail_channel.py | 2 +- addons/mail/models/mail_render_mixin.py | 39 ++++++++++++++++++- addons/mail/models/mail_template.py | 2 +- addons/mail/models/mail_thread.py | 36 +---------------- addons/mass_mailing/models/mail_mail.py | 2 +- .../wizard/mailing_mailing_test.py | 2 +- addons/payment_paypal/models/payment.py | 2 +- addons/survey/wizard/survey_invite.py | 2 +- .../wizard/slide_channel_invite.py | 2 +- 10 files changed, 48 insertions(+), 43 deletions(-) diff --git a/addons/hr_expense/models/hr_expense.py b/addons/hr_expense/models/hr_expense.py index 1f77dcdc4d4..60a402c0015 100644 --- a/addons/hr_expense/models/hr_expense.py +++ b/addons/hr_expense/models/hr_expense.py @@ -647,7 +647,7 @@ class HrExpense(models.Model): mail_template_id = 'hr_expense.hr_expense_template_register' if expense.employee_id.user_id else 'hr_expense.hr_expense_template_register_no_user' expense_template = self.env.ref(mail_template_id) rendered_body = expense_template.render({'expense': expense}, engine='ir.qweb') - body = self.env['mail.thread']._replace_local_links(rendered_body) + body = self.env['mail.render.mixin']._replace_local_links(rendered_body) # TDE TODO: seems louche, check to use notify if expense.employee_id.user_id.partner_id: expense.message_post( diff --git a/addons/im_livechat/models/mail_channel.py b/addons/im_livechat/models/mail_channel.py index c7ca4098093..2a6f5548d66 100644 --- a/addons/im_livechat/models/mail_channel.py +++ b/addons/im_livechat/models/mail_channel.py @@ -184,7 +184,7 @@ class MailChannel(models.Model): } template = self.env.ref('im_livechat.livechat_email_template') mail_body = template.render(render_context, engine='ir.qweb', minimal_qcontext=True) - mail_body = self.env['mail.thread']._replace_local_links(mail_body) + mail_body = self.env['mail.render.mixin']._replace_local_links(mail_body) mail = self.env['mail.mail'].sudo().create({ 'subject': _('Conversation with %s') % self.livechat_operator_id.name, 'email_from': company.catchall_formatted or company.email_formatted, diff --git a/addons/mail/models/mail_render_mixin.py b/addons/mail/models/mail_render_mixin.py index 5804481f15c..ee989342b42 100644 --- a/addons/mail/models/mail_render_mixin.py +++ b/addons/mail/models/mail_render_mixin.py @@ -7,6 +7,7 @@ import datetime import dateutil.relativedelta as relativedelta import functools import logging +import re from werkzeug import urls @@ -146,6 +147,42 @@ class MailRenderMixin(models.AbstractModel): expression += "}" return expression + # ------------------------------------------------------------ + # 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 = tools.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 + # ------------------------------------------------------------ # RENDERING # ------------------------------------------------------------ @@ -230,7 +267,7 @@ class MailRenderMixin(models.AbstractModel): :return dict: updated version of rendered """ for res_id, html in rendered.items(): - rendered[res_id] = self.env['mail.thread']._replace_local_links(html) + rendered[res_id] = self._replace_local_links(html) return rendered @api.model diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index 3fbe4b03e68..c4d590e2729 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -268,7 +268,7 @@ class MailTemplate(models.Model): 'record': record, } body = template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) - values['body_html'] = self.env['mail.thread']._replace_local_links(body) + values['body_html'] = self.env['mail.render.mixin']._replace_local_links(body) mail = self.env['mail.mail'].sudo().create(values) # manage attachments diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 300ab439cf9..43d705b1c79 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -476,38 +476,6 @@ class MailThread(models.AbstractModel): return self.with_context(lang=self.env.user.lang) return self - 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 @@ -2322,7 +2290,7 @@ class MailThread(models.AbstractModel): mail_body = base_template.render(render_values, engine='ir.qweb', minimal_qcontext=True) else: mail_body = message.body - mail_body = self._replace_local_links(mail_body) + mail_body = self.env['mail.render.mixin']._replace_local_links(mail_body) # create email for recipients_ids_chunk in split_every(recipients_max, recipients_ids): @@ -2899,7 +2867,7 @@ class MailThread(models.AbstractModel): 'access_link': self._notify_get_action_link('view'), } assignation_msg = view.render(values, engine='ir.qweb', minimal_qcontext=True) - assignation_msg = self.env['mail.thread']._replace_local_links(assignation_msg) + assignation_msg = self.env['mail.render.mixin']._replace_local_links(assignation_msg) record.message_notify( subject=_('You have been assigned to %s') % record.display_name, body=assignation_msg, diff --git a/addons/mass_mailing/models/mail_mail.py b/addons/mass_mailing/models/mail_mail.py index 749c6dc6133..68a653a38db 100644 --- a/addons/mass_mailing/models/mail_mail.py +++ b/addons/mass_mailing/models/mail_mail.py @@ -69,7 +69,7 @@ class MailMail(models.Model): if tracking_url: body = tools.append_content_to_html(body, tracking_url, plaintext=False, container_tag='div') - body = self.env['mail.thread']._replace_local_links(body) + body = self.env['mail.render.mixin']._replace_local_links(body) return body diff --git a/addons/mass_mailing/wizard/mailing_mailing_test.py b/addons/mass_mailing/wizard/mailing_mailing_test.py index dab0f70f725..45de0d88019 100644 --- a/addons/mass_mailing/wizard/mailing_mailing_test.py +++ b/addons/mass_mailing/wizard/mailing_mailing_test.py @@ -24,7 +24,7 @@ class TestMassMailing(models.TransientModel): mass_mail_layout = self.env.ref('mass_mailing.mass_mailing_mail_layout') for test_mail in test_emails: # Convert links in absolute URLs before the application of the shortener - body = self.env['mail.thread']._replace_local_links(mailing.body_html) + body = self.env['mail.render.mixin']._replace_local_links(mailing.body_html) body = tools.html_sanitize(body, sanitize_attributes=True, sanitize_style=True) mail_values = { 'email_from': mailing.email_from, diff --git a/addons/payment_paypal/models/payment.py b/addons/payment_paypal/models/payment.py index 38fdb2422ce..f3b99925c85 100644 --- a/addons/payment_paypal/models/payment.py +++ b/addons/payment_paypal/models/payment.py @@ -195,7 +195,7 @@ class TxPaypal(models.Model): render_template = template.render({ 'acquirer': self.acquirer_id, }, engine='ir.qweb') - mail_body = self.env['mail.thread']._replace_local_links(render_template) + mail_body = self.env['mail.render.mixin']._replace_local_links(render_template) mail_values = { 'body_html': mail_body, 'subject': _('Add your Paypal account to Odoo'), diff --git a/addons/survey/wizard/survey_invite.py b/addons/survey/wizard/survey_invite.py index 0068ecdb06c..7896cddfa0e 100644 --- a/addons/survey/wizard/survey_invite.py +++ b/addons/survey/wizard/survey_invite.py @@ -231,7 +231,7 @@ class SurveyInvite(models.TransientModel): 'company': self.env.company, } body = template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) - mail_values['body_html'] = self.env['mail.thread']._replace_local_links(body) + mail_values['body_html'] = self.env['mail.render.mixin']._replace_local_links(body) return self.env['mail.mail'].sudo().create(mail_values) diff --git a/addons/website_slides/wizard/slide_channel_invite.py b/addons/website_slides/wizard/slide_channel_invite.py index 28522e39c4b..9650b4d2f9e 100644 --- a/addons/website_slides/wizard/slide_channel_invite.py +++ b/addons/website_slides/wizard/slide_channel_invite.py @@ -124,6 +124,6 @@ class SlideChannelInvite(models.TransientModel): 'company': self.env.company, } body = template.render(template_ctx, engine='ir.qweb', minimal_qcontext=True) - mail_values['body_html'] = self.env['mail.thread']._replace_local_links(body) + mail_values['body_html'] = self.env['mail.render.mixin']._replace_local_links(body) return mail_values From aaefa35e29f7e87e8e914cf1d9eb68af7d26464a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 11 Mar 2020 12:42:48 +0000 Subject: [PATCH 8/9] [MOV] link_tracker, various: more link shortening tools to mail.render.mixin PURPOSE Move links shortening tools on mail.render.mixin to have rendering and post procesing tools available on that mixin. SPECIFICATIONS Move html and text links shortening methods on mail.render.mixin class. That way rendering and post processing is done in that mixin instead of being specific to link.tracker model. Indeed those methods are tools and not really business related methods. Future commits will improve the way mail and sms composer use the render mixin. This commit is therefore both a cleaning and preparing commit. LINKS Task ID 1963529 Community PR odoo/odoo#32397 --- addons/link_tracker/__manifest__.py | 2 +- addons/link_tracker/models/__init__.py | 1 + addons/link_tracker/models/link_tracker.py | 58 ++--------------- .../link_tracker/models/mail_render_mixin.py | 62 +++++++++++++++++++ addons/link_tracker/models/utm.py | 2 +- addons/mass_mailing/models/mail_mail.py | 4 +- .../mass_mailing/models/mail_render_mixin.py | 2 +- addons/mass_mailing/models/mailing.py | 2 +- addons/mass_mailing_sms/models/sms_sms.py | 6 +- .../mass_mailing_sms/wizard/sms_composer.py | 2 +- addons/test_mail_full/tests/test_sms_sms.py | 8 +-- odoo/tools/mail.py | 16 ++++- 12 files changed, 94 insertions(+), 71 deletions(-) create mode 100644 addons/link_tracker/models/mail_render_mixin.py diff --git a/addons/link_tracker/__manifest__.py b/addons/link_tracker/__manifest__.py index 3f622632cc4..038fe807173 100644 --- a/addons/link_tracker/__manifest__.py +++ b/addons/link_tracker/__manifest__.py @@ -8,7 +8,7 @@ Shorten URLs and use them to track clicks and UTMs """, 'version': '1.0', - 'depends': ['utm'], + 'depends': ['utm', 'mail'], 'data': [ 'views/link_tracker_views.xml', 'views/utm_campaign_views.xml', diff --git a/addons/link_tracker/models/__init__.py b/addons/link_tracker/models/__init__.py index 847f935569e..6f884bd7c2e 100644 --- a/addons/link_tracker/models/__init__.py +++ b/addons/link_tracker/models/__init__.py @@ -2,4 +2,5 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from . import link_tracker +from . import mail_render_mixin from . import utm diff --git a/addons/link_tracker/models/link_tracker.py b/addons/link_tracker/models/link_tracker.py index 3af4accb9ba..ec73046e1be 100644 --- a/addons/link_tracker/models/link_tracker.py +++ b/addons/link_tracker/models/link_tracker.py @@ -1,28 +1,14 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import base64 -import datetime import random -import re import requests import string from lxml import html -from werkzeug import urls, utils +from werkzeug import urls -from odoo import models, fields, api, _ - - -URL_REGEX = r'(\bhref=[\'"](?!mailto:|tel:|sms:)([^\'"]+)[\'"])' -TEXT_URL_REGEX = r'https?://[a-zA-Z0-9@:%._\+~#=/-]+(?:\?\S+)?' - - -def VALIDATE_URL(url): - if urls.url_parse(url).scheme not in ('http', 'https', 'ftp', 'ftps'): - return 'http://' + url - - return url +from odoo import tools, models, fields, api, _ class LinkTracker(models.Model): @@ -101,7 +87,7 @@ class LinkTracker(models.Model): if 'url' not in create_vals: raise ValueError('URL field required') else: - create_vals['url'] = VALIDATE_URL(vals['url']) + create_vals['url'] = tools.validate_url(vals['url']) search_domain = [] for fname, value in create_vals.items(): @@ -129,44 +115,10 @@ class LinkTracker(models.Model): @api.model def convert_links(self, html, vals, blacklist=None): - for match in re.findall(URL_REGEX, html): - - short_schema = self.env['ir.config_parameter'].sudo().get_param('web.base.url') + '/r/' - - href = match[0] - long_url = match[1] - - vals['url'] = utils.unescape(long_url) - - if not blacklist or not [s for s in blacklist if s in long_url] and not long_url.startswith(short_schema): - link = self.create(vals) - shorten_url = self.browse(link.id)[0].short_url - - if shorten_url: - new_href = href.replace(long_url, shorten_url) - html = html.replace(href, new_href) - - return html + raise NotImplementedError('Moved on mail.render.mixin') def _convert_links_text(self, body, vals, blacklist=None): - shortened_schema = self.env['ir.config_parameter'].sudo().get_param('web.base.url') + '/r/' - unsubscribe_schema = self.env['ir.config_parameter'].sudo().get_param('web.base.url') + '/sms/' - for original_url in re.findall(TEXT_URL_REGEX, body): - # don't shorten already-shortened links or links towards unsubscribe page - if original_url.startswith(shortened_schema) or original_url.startswith(unsubscribe_schema): - continue - # support blacklist items in path, like /u/ - parsed = urls.url_parse(original_url, scheme='http') - if blacklist and any(item in parsed.path for item in blacklist): - continue - - vals['url'] = utils.unescape(original_url) - link = self.create(vals) - shortened_url = link.short_url - if shortened_url: - body = body.replace(original_url, shortened_url, 1) - - return body + raise NotImplementedError('Moved on mail.render.mixin') def action_view_statistics(self): action = self.env['ir.actions.act_window'].for_xml_id('link_tracker', 'link_tracker_click_action_statistics') diff --git a/addons/link_tracker/models/mail_render_mixin.py b/addons/link_tracker/models/mail_render_mixin.py new file mode 100644 index 00000000000..0bc30de960b --- /dev/null +++ b/addons/link_tracker/models/mail_render_mixin.py @@ -0,0 +1,62 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import re + +from werkzeug import urls, utils + +from odoo import api, models, tools + + +class MailRenderMixin(models.AbstractModel): + _inherit = "mail.render.mixin" + + # ------------------------------------------------------------ + # TOOLS + # ------------------------------------------------------------ + + @api.model + def _shorten_links(self, html, link_tracker_vals, blacklist=None, base_url=None): + """ + + :param link_tracker_vals: values given to the created link.tracker, containing + for example: campaign_id, medium_id, source_id, and any other relevant fields + like mass_mailing_id in mass_mailing; + + :return: update + """ + base_url = base_url or self.env['ir.config_parameter'].sudo().get_param('web.base.url') + short_schema = base_url + '/r/' + for match in re.findall(tools.URL_REGEX, html): + href = match[0] + long_url = match[1] + + if not blacklist or not [s for s in blacklist if s in long_url] and not long_url.startswith(short_schema): + create_vals = dict(link_tracker_vals, url=utils.unescape(long_url)) + link = self.env['link.tracker'].create(create_vals) + if link.short_url: + new_href = href.replace(long_url, link.short_url) + html = html.replace(href, new_href) + + return html + + @api.model + def _shorten_links_text(self, html, link_tracker_vals, blacklist=None, base_url=None): + base_url = base_url or self.env['ir.config_parameter'].sudo().get_param('web.base.url') + shortened_schema = base_url + '/r/' + unsubscribe_schema = base_url + '/sms/' + for original_url in re.findall(tools.TEXT_URL_REGEX, html): + # don't shorten already-shortened links or links towards unsubscribe page + if original_url.startswith(shortened_schema) or original_url.startswith(unsubscribe_schema): + continue + # support blacklist items in path, like /u/ + parsed = urls.url_parse(original_url, scheme='http') + if blacklist and any(item in parsed.path for item in blacklist): + continue + + create_vals = dict(link_tracker_vals, url= utils.unescape(original_url)) + link = self.env['link.tracker'].create(create_vals) + if link.short_url: + html = html.replace(original_url, link.short_url, 1) + + return html diff --git a/addons/link_tracker/models/utm.py b/addons/link_tracker/models/utm.py index ebb343472e7..75eff850150 100644 --- a/addons/link_tracker/models/utm.py +++ b/addons/link_tracker/models/utm.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import fields, models, api, SUPERUSER_ID +from odoo import fields, models class UtmCampaign(models.Model): diff --git a/addons/mass_mailing/models/mail_mail.py b/addons/mass_mailing/models/mail_mail.py index 68a653a38db..d35ae72f4be 100644 --- a/addons/mass_mailing/models/mail_mail.py +++ b/addons/mass_mailing/models/mail_mail.py @@ -6,8 +6,6 @@ import werkzeug.urls from odoo import api, fields, models, tools -from odoo.addons.link_tracker.models.link_tracker import URL_REGEX - class MailMail(models.Model): """Add the mass mailing campaign data to mail""" @@ -54,7 +52,7 @@ class MailMail(models.Model): body = super(MailMail, self)._send_prepare_body() if self.mailing_id and body and self.mailing_trace_ids: - for match in re.findall(URL_REGEX, self.body_html): + for match in re.findall(tools.URL_REGEX, self.body_html): href = match[0] url = match[1] diff --git a/addons/mass_mailing/models/mail_render_mixin.py b/addons/mass_mailing/models/mail_render_mixin.py index d38667366ed..4447f3931c8 100644 --- a/addons/mass_mailing/models/mail_render_mixin.py +++ b/addons/mass_mailing/models/mail_render_mixin.py @@ -15,7 +15,7 @@ class MailRenderMixin(models.AbstractModel): # apply shortener after if self.env.context.get('post_convert_links'): for res_id, html in rendered.items(): - rendered[res_id] = self.env['link.tracker'].convert_links( + rendered[res_id] = self._shorten_links( html, self.env.context['post_convert_links'], blacklist=['/unsubscribe_from_list'] diff --git a/addons/mass_mailing/models/mailing.py b/addons/mass_mailing/models/mailing.py index 88c5a9c4574..1ce47e6cfa4 100644 --- a/addons/mass_mailing/models/mailing.py +++ b/addons/mass_mailing/models/mailing.py @@ -561,7 +561,7 @@ class MassMailing(models.Model): if mass_mailing.medium_id: vals['medium_id'] = mass_mailing.medium_id.id - res[mass_mailing.id] = self.env['link.tracker'].convert_links(html, vals, blacklist=['/unsubscribe_from_list']) + res[mass_mailing.id] = self._shorten_links(html, vals, blacklist=['/unsubscribe_from_list']) return res diff --git a/addons/mass_mailing_sms/models/sms_sms.py b/addons/mass_mailing_sms/models/sms_sms.py index d83359aa47a..c9c14e144cd 100644 --- a/addons/mass_mailing_sms/models/sms_sms.py +++ b/addons/mass_mailing_sms/models/sms_sms.py @@ -3,9 +3,7 @@ import re -from odoo import fields, models - -TEXT_URL_REGEX = r'https?://[a-zA-Z0-9@:%._+~#=/-]+' +from odoo import fields, models, tools class SmsSms(models.Model): @@ -25,7 +23,7 @@ class SmsSms(models.Model): continue body = sms.body - for url in re.findall(TEXT_URL_REGEX, body): + for url in re.findall(tools.TEXT_URL_REGEX, body): if url.startswith(shortened_schema): body = body.replace(url, url + '/s/%s' % sms.id) res[sms.id] = body diff --git a/addons/mass_mailing_sms/wizard/sms_composer.py b/addons/mass_mailing_sms/wizard/sms_composer.py index fefbeeb0137..b3e50234036 100644 --- a/addons/mass_mailing_sms/wizard/sms_composer.py +++ b/addons/mass_mailing_sms/wizard/sms_composer.py @@ -70,7 +70,7 @@ class SMSComposer(models.TransientModel): if self.mailing_id: tracker_values = self.mailing_id._get_link_tracker_values() for sms_id, body in all_bodies.items(): - body = self.env['link.tracker'].sudo()._convert_links_text(body, tracker_values) + body = self.env['mail.render.mixin'].sudo()._shorten_links_text(body, tracker_values) all_bodies[sms_id] = body return all_bodies diff --git a/addons/test_mail_full/tests/test_sms_sms.py b/addons/test_mail_full/tests/test_sms_sms.py index de7e37ac641..e404eaec572 100644 --- a/addons/test_mail_full/tests/test_sms_sms.py +++ b/addons/test_mail_full/tests/test_sms_sms.py @@ -64,7 +64,7 @@ class TestSMSPost(test_mail_full_common.TestSMSCommon, LinkTrackerMock): def test_body_link_shorten(self): link = 'http://www.example.com' self.env['link.tracker'].search([('url', '=', link)]).unlink() - new_body = self.env['link.tracker']._convert_links_text('Welcome to %s !' % link, self.tracker_values) + new_body = self.env['mail.render.mixin']._shorten_links_text('Welcome to %s !' % link, self.tracker_values) self.assertNotIn(link, new_body) self.assertLinkTracker(link, {'utm_campaign': self.utm_c.name, 'utm_medium': self.utm_m.name}) link = self.env['link.tracker'].search([('url', '=', link)]) @@ -72,7 +72,7 @@ class TestSMSPost(test_mail_full_common.TestSMSCommon, LinkTrackerMock): link = 'https://test.odoo.com/my/super_page?test[0]=42&toto=áâà#title3' self.env['link.tracker'].search([('url', '=', link)]).unlink() - new_body = self.env['link.tracker']._convert_links_text('Welcome to %s !' % link, self.tracker_values) + new_body = self.env['mail.render.mixin']._shorten_links_text('Welcome to %s !' % link, self.tracker_values) self.assertNotIn(link, new_body) self.assertLinkTracker(link, { 'utm_campaign': self.utm_c.name, @@ -86,14 +86,14 @@ class TestSMSPost(test_mail_full_common.TestSMSCommon, LinkTrackerMock): def test_body_link_shorten_wshort(self): link = 'https://test.odoo.com/r/RAOUL' self.env['link.tracker'].search([('url', '=', link)]).unlink() - new_body = self.env['link.tracker']._convert_links_text('Welcome to %s !' % link, self.tracker_values) + new_body = self.env['mail.render.mixin']._shorten_links_text('Welcome to %s !' % link, self.tracker_values) self.assertIn(link, new_body) self.assertFalse(self.env['link.tracker'].search([('url', '=', link)])) def test_body_link_shorten_wunsubscribe(self): link = 'https://test.odoo.com/sms/3/' self.env['link.tracker'].search([('url', '=', link)]).unlink() - new_body = self.env['link.tracker']._convert_links_text('Welcome to %s !' % link, self.tracker_values) + new_body = self.env['mail.render.mixin']._shorten_links_text('Welcome to %s !' % link, self.tracker_values) self.assertIn(link, new_body) self.assertFalse(self.env['link.tracker'].search([('url', '=', link)])) diff --git a/odoo/tools/mail.py b/odoo/tools/mail.py index 21756fe0640..d1bc961e8ff 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -13,6 +13,7 @@ import time from email.utils import getaddresses from lxml import etree +from werkzeug import urls import odoo from odoo.loglevels import ustr @@ -250,9 +251,19 @@ def html_sanitize(src, silent=True, sanitize_tags=True, sanitize_attributes=Fals return cleaned -#---------------------------------------------------------- +# ---------------------------------------------------------- # HTML/Text management -#---------------------------------------------------------- +# ---------------------------------------------------------- + +URL_REGEX = r'(\bhref=[\'"](?!mailto:|tel:|sms:)([^\'"]+)[\'"])' +TEXT_URL_REGEX = r'https?://[a-zA-Z0-9@:%._\+~#=/-]+(?:\?\S+)?' + + +def validate_url(url): + if urls.url_parse(url).scheme not in ('http', 'https', 'ftp', 'ftps'): + return 'http://' + url + + return url def is_html_empty(html_content): @@ -281,6 +292,7 @@ def html_keep_url(text): final += text[idx:] return final + def html2plaintext(html, body_id=None, encoding='utf-8'): """ From an HTML text, convert the HTML to plain text. If @param body_id is provided then this is the tag where the From efa161e25c452aacb9ee39281f94e36cc696cedc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 23 Mar 2020 13:59:16 +0000 Subject: [PATCH 9/9] [IMP] sms: make template required on preview template It makes no sense to make it not required. Task ID 1963529 Community PR odoo/odoo#32397 --- addons/sms/wizard/sms_template_preview.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/addons/sms/wizard/sms_template_preview.py b/addons/sms/wizard/sms_template_preview.py index 837c0b821b8..ffe22a4238a 100644 --- a/addons/sms/wizard/sms_template_preview.py +++ b/addons/sms/wizard/sms_template_preview.py @@ -29,8 +29,7 @@ class SMSTemplatePreview(models.TransientModel): result['resource_ref'] = '%s,%s' % (sms_template.model_id.model, res.id) return result - sms_template_id = fields.Many2one('sms.template') # NOTE This should probably be required - + sms_template_id = fields.Many2one('sms.template', required=True, ondelete='cascade') lang = fields.Selection(_selection_languages, string='Template Preview Language') model_id = fields.Many2one('ir.model', related="sms_template_id.model_id") body = fields.Char('Body', compute='_compute_sms_template_fields')