From ab9bc656da6a637596101d1de24c0c5e7356902e Mon Sep 17 00:00:00 2001 From: Florian Charlier Date: Thu, 3 Mar 2022 11:36:08 +0000 Subject: [PATCH] [IMP] mail: improve notification email previews Prepare an informative and nice-looking preview from the main content of the email body, avoiding buttons/images alt etc. Links, images, tables are removed. Whitespace is added after the preview to avoid including the full message in the preview (with markup). Tags and attributes are also added to increase compliance with HTML5 standard, including accessibility. One additional SQL request is required to fetch the preview sub-template. Task-2413355 closes odoo/odoo#86266 Signed-off-by: Thibault Delavallee (tde) --- .../data/mail_templates_email_layouts.xml | 57 ++++++++++++++++--- addons/mail/models/mail_message.py | 25 ++++++-- addons/test_mail/tests/test_message_post.py | 5 +- addons/test_mail/tests/test_performance.py | 6 +- odoo/addons/base/tests/test_mail.py | 15 ++++- odoo/tools/mail.py | 17 ++++++ 6 files changed, 106 insertions(+), 19 deletions(-) diff --git a/addons/mail/data/mail_templates_email_layouts.xml b/addons/mail/data/mail_templates_email_layouts.xml index f14c856daa9..4a2b6a53e6e 100644 --- a/addons/mail/data/mail_templates_email_layouts.xml +++ b/addons/mail/data/mail_templates_email_layouts.xml @@ -2,6 +2,13 @@ + + diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index c7e9521a14a..cb3278cd177 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -3,9 +3,9 @@ import logging import re -from collections import defaultdict - +import textwrap from binascii import Error as binascii_error +from collections import defaultdict from odoo import _, api, Command, fields, models, modules, tools from odoo.exceptions import AccessError @@ -89,6 +89,9 @@ class Message(models.Model): description = fields.Char( 'Short description', compute="_compute_description", help='Message description: either the subject, or the beginning of the body') + preview = fields.Char( + 'Preview', compute='_compute_preview', + help='The text-only beginning of the body used as email preview.') attachment_ids = fields.Many2many( 'ir.attachment', 'message_attachment_rel', 'message_id', 'attachment_id', @@ -178,13 +181,18 @@ class Message(models.Model): canned_response_ids = fields.One2many('mail.shortcode', 'message_ids', string="Canned Responses", store=False) reaction_ids = fields.One2many('mail.message.reaction', 'message_id', string="Reactions", groups="base.group_system") + @api.depends('body', 'subject') def _compute_description(self): for message in self: if message.subject: message.description = message.subject else: - plaintext_ct = '' if not message.body else tools.html2plaintext(message.body) - message.description = plaintext_ct[:30] + '%s' % (' [...]' if len(plaintext_ct) >= 30 else '') + message.description = message._get_message_preview(max_char=30) + + @api.depends('body') + def _compute_preview(self): + for message in self: + message.preview = message._get_message_preview() @api.depends('author_id', 'author_guest_id') @api.depends_context('guest', 'uid') @@ -1027,6 +1035,15 @@ class Message(models.Model): # TOOLS # ------------------------------------------------------ + def _get_message_preview(self, max_char=190): + """Returns an unformatted version of the message body. Unless `max_char=0` is passed, + output will be capped at max_char characters with a ' [...]' suffix if applicable. + Default `max_char` is the longest known mail client preview length (Outlook 2013).""" + self.ensure_one() + + plaintext_ct = tools.html_to_inner_content(self.body) + return textwrap.shorten(plaintext_ct, max_char) if max_char else plaintext_ct + @api.model def _get_record_name(self, values): """ Return the related document name, using name_get. It is done using diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 24a543d1bb8..2d069898df0 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -295,9 +295,10 @@ class TestMessageNotify(TestMessagePostCommon): admin_mails = [mail for mail in self._mails if self.partner_admin.name in mail.get('email_to')[0]] self.assertEqual(len(admin_mails), 1, 'There should be exactly one email sent to admin') admin_mail_body = admin_mails[0].get('body') - admin_access_link = admin_mail_body[admin_mail_body.index('model='):admin_mail_body.index('/>') - 1] if 'model=' in admin_mail_body else None - self.assertIsNotNone(admin_access_link, 'The email sent to admin should contain an access link') + self.assertTrue('model=' in admin_mail_body, 'The email sent to admin should contain an access link') + admin_access_link = admin_mail_body[ + admin_mail_body.index('model='):admin_mail_body.index('/>', admin_mail_body.index('model=')) - 1] self.assertIn('model=%s' % self.test_record._name, admin_access_link, 'The access link should contain a valid model argument') self.assertIn('res_id=%d' % self.test_record.id, admin_access_link, 'The access link should contain a valid res_id argument') diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 7761dc7dc2b..64c3e45aae2 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -575,7 +575,7 @@ class TestMailAPIPerformance(BaseMailPerformance): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(__system__=30, employee=33): + with self.assertQueryCount(__system__=31, employee=34): record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, @@ -726,7 +726,7 @@ class TestMailComplexPerformance(BaseMailPerformance): self.container.message_subscribe(self.user_portal.partner_id.ids) record = self.container.with_user(self.env.user) - with self.assertQueryCount(__system__=53, employee=54): # about 20 (19?) queries per additional customer group + with self.assertQueryCount(__system__=54, employee=55): # about 20 (19?) queries per additional customer group record.message_post( body='

Test Post Performances

', message_type='comment', @@ -743,7 +743,7 @@ class TestMailComplexPerformance(BaseMailPerformance): record = self.container.with_user(self.env.user) template_id = self.env.ref('test_mail.mail_test_container_tpl').id - with self.assertQueryCount(__system__=61, employee=62): # about 20 (19 ?) queries per additional customer group + with self.assertQueryCount(__system__=62, employee=63): # about 20 (19 ?) queries per additional customer group record.message_post_with_template(template_id, message_type='comment', composition_mode='comment') self.assertEqual(record.message_ids[0].body, '

Adding stuff on %s

' % record.name) diff --git a/odoo/addons/base/tests/test_mail.py b/odoo/addons/base/tests/test_mail.py index 5cd017f75ee..90bab7b4cef 100644 --- a/odoo/addons/base/tests/test_mail.py +++ b/odoo/addons/base/tests/test_mail.py @@ -11,7 +11,7 @@ import threading from odoo.addons.base.models.ir_mail_server import extract_rfc2822_addresses from odoo.tests.common import BaseCase, TransactionCase from odoo.tools import ( - is_html_empty, html_sanitize, append_content_to_html, plaintext2html, + is_html_empty, html_to_inner_content, html_sanitize, append_content_to_html, plaintext2html, email_split, email_domain_normalize, misc, formataddr, prepend_html_content, @@ -306,6 +306,19 @@ class TestHtmlTools(BaseCase): html = plaintext2html(content, container_tag) self.assertEqual(html, expected, 'plaintext2html is broken') + def test_html_html_to_inner_content(self): + cases = [ + ('

First
Second
Third Paragraph

--
Signature paragraph with a link

', + 'First Second Third Paragraph -- Signature paragraph with a link'), + ('

Now => processing entities​and extra whitespace too.

', + 'Now => processing\xa0entities\u200band extra whitespace too.'), + ('
Look what happens with

unmatched tags

', 'Look what happens with unmatched tags'), + ('
Look what happens with

Are we good?', 'Look what happens with Are we good?') + ] + for content, expected in cases: + text = html_to_inner_content(content) + self.assertEqual(text, expected, 'html_html_to_inner_content is broken') + def test_append_to_html(self): test_samples = [ ('some content', '--\nYours truly', True, True, False, diff --git a/odoo/tools/mail.py b/odoo/tools/mail.py index 15b94d199c7..75b931f0ac7 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -260,6 +260,8 @@ URL_REGEX = r'(\bhref=[\'"](?!mailto:|tel:|sms:)([^\'"]+)[\'"])' TEXT_URL_REGEX = r'https?://[\w@:%.+&~#=/-]+(?:\?\S+)?' # retrieve inner content of the link HTML_TAG_URL_REGEX = URL_REGEX + r'([^<>]*>([^<>]+)<\/)?' +HTML_TAGS_REGEX = re.compile('<.*?>') +HTML_NEWLINES_REGEX = re.compile('<(div|p|br|tr)[^>]*>|\n') def validate_url(url): @@ -295,6 +297,21 @@ def html_keep_url(text): return final +def html_to_inner_content(html): + """Returns unformatted text after removing html tags and excessive whitespace from a + string/Markup. Passed strings will first be sanitized. + """ + if is_html_empty(html): + return '' + if not isinstance(html, markupsafe.Markup): + html = html_sanitize(html) + processed = re.sub(HTML_NEWLINES_REGEX, ' ', html) + processed = re.sub(HTML_TAGS_REGEX, '', processed) + processed = re.sub(r' {2,}|\t', ' ', processed) + processed = processed.strip() + return processed + + 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