[MERGE] mail: cleanup mail.message model (shortcode, link)

Remove useless link between message and shortcodes

There is a o2m / m2o link between mail.message and mail.shortcode. However
this link is not used: 'messages' (m2o from shortcode to message) is never
set. Anyway there is no usage of this link: shortcodes are replaced in
message body and that's it.

Improve link preview code
  * remove useless indirections;
  * improve performance of link creation, cleanly support batch-creation;
  * name methods according to ORM;
  * use tools for html empty;

Also cleanup link preview tests: concatenate tests when possible, improve
setup, use real-life scenarios (using 'message_post' notably), remove useless
data creation.

Task-3557985 (Mail: Message cleanup (shortcode, link))
Prepares Task-36879 (Mail: Support MultiCompany Aliases)

closes odoo/odoo#138938

Related: odoo/upgrade#5292
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>

addons/mail/controllers/link_preview.py

@@ -18,8 +18,8 @@ class LinkPreviewController(http.Controller):
         if not message.is_current_user_or_guest_author and not guest.env.user._is_admin():
             return
         if clear:
-            guest.env["mail.link.preview"]._clear_link_previews(message.sudo())
-        guest.env["mail.link.preview"].sudo()._create_link_previews(message)
+            message.sudo().link_preview_ids._unlink_and_notify()
+        guest.env["mail.link.preview"].sudo()._create_from_message_and_notify(message)
 
     @http.route("/mail/link_preview/delete", methods=["POST"], type="json", auth="public")
     @add_guest_to_context
@@ -30,4 +30,4 @@ class LinkPreviewController(http.Controller):
             return
         if not link_preview_sudo.message_id.is_current_user_or_guest_author and not guest.env.user._is_admin():
             return
-        link_preview_sudo._delete_and_notify()
+        link_preview_sudo._unlink_and_notify()

addons/mail/models/mail_link_preview.py

@@ -7,7 +7,7 @@ from lxml import html
 from urllib.parse import urlparse
 import requests
 
-from odoo import api, models, fields
+from odoo import api, models, fields, tools
 from odoo.addons.mail.tools import link_preview
 
 
@@ -27,48 +27,38 @@ class LinkPreview(models.Model):
     create_date = fields.Datetime(index=True)
 
     @api.model
-    def _clear_link_previews(self, message):
-        message.link_preview_ids._delete_and_notify()
-
-    @api.model
-    def _create_link_previews(self, message):
-        if not message.body:
-            return
-        tree = html.fromstring(message.body)
-        urls = tree.xpath('//a[not(@data-oe-model)]/@href')
+    def _create_from_message_and_notify(self, message):
+        if tools.is_html_empty(message.body):
+            return self
+        urls = set(html.fromstring(message.body).xpath('//a[not(@data-oe-model)]/@href'))
         link_previews = self.env['mail.link.preview']
         requests_session = requests.Session()
         # Some websites are blocking non browser user agent.
         requests_session.headers.update({
             'User-Agent': 'Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101 Firefox/91.0'
         })
-        for url in set(urls):
-            if len(link_previews) >= 5:
+        link_preview_values = []
+        for url in list(urls):
+            if preview := link_preview.get_link_preview_from_url(url, requests_session):
+                preview['message_id'] = message.id
+                link_preview_values.append(preview)
+            if len(link_preview_values) > 5:
                 break
-            link_previews |= self.env['mail.link.preview']._create_link_preview(url, message.id, requests_session)
-        if not link_previews:
-            return
-        self.env['bus.bus']._sendone(message._bus_notification_target(), 'mail.record/insert', {
-            'LinkPreview': link_previews._link_preview_format()
-        })
+        if link_preview_values:
+            link_previews = self.env['mail.link.preview'].create(link_preview_values)
+            self.env['bus.bus']._sendone(message._bus_notification_target(), 'mail.record/insert', {
+                'LinkPreview': link_previews._link_preview_format()
+            })
 
-    @api.model
-    def _create_link_preview(self, url, message_id, request_session):
-        if self._is_domain_throttled(url):
-            return self.env['mail.link.preview']
-        link_preview_data = link_preview.get_link_preview_from_url(url, request_session)
-        if link_preview_data:
-            link_preview_data['message_id'] = message_id
-            return self.create(link_preview_data)
-        return self.env['mail.link.preview']
-
-    def _delete_and_notify(self):
-        notifications = []
-        for link_preview in self:
-            notifications.append((link_preview.message_id._bus_notification_target(), 'mail.link.preview/delete', {
-                'id': link_preview.id,
-                'message_id': link_preview.message_id.id,
-            }))
+    def _unlink_and_notify(self):
+        if not self:
+            return True
+        notifications = [
+            (
+                link_preview.message_id._bus_notification_target(),
+                'mail.link.preview/delete', {'id': link_preview.id, 'message_id': link_preview.message_id.id}
+            ) for link_preview in self
+        ]
         self.env['bus.bus']._sendmany(notifications)
         self.unlink()
 

addons/mail/models/mail_message.py

@@ -178,7 +178,6 @@ class Message(models.Model):
     # as the cache value for this inverse one2many is up-to-date.
     # Besides for new messages, and messages never sending emails, there was no mail, and it was searching for nothing.
     mail_ids = fields.One2many('mail.mail', 'mail_message_id', string='Mails', groups="base.group_system")
-    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')

addons/mail/models/mail_shortcode.py

@@ -19,5 +19,4 @@ class MailShortcode(models.Model):
     substitution = fields.Text('Substitution', required=True,
         help="Content that will automatically replace the shortcut of your choosing. This content can still be adapted before sending your message.")
     description = fields.Char('Description')
-    message_ids = fields.Many2one('mail.message', string="Messages", store=False)
     last_used = fields.Datetime('Last Used', help="Last time this shortcode was used")

addons/mail/models/mail_thread.py

@@ -2746,7 +2746,7 @@ class MailThread(models.AbstractModel):
         for values in values_list:
             create_values = dict(values)
             # Avoid warnings about non-existing fields
-            for x in ('from', 'to', 'cc', 'canned_response_ids'):
+            for x in ('from', 'to', 'cc'):
                 create_values.pop(x, None)
             create_values['partner_ids'] = [Command.link(pid) for pid in (create_values.get('partner_ids') or [])]
             create_values_list.append(create_values)

addons/mail/models/res_company.py

@@ -36,7 +36,7 @@ class Company(models.Model):
                 company.catchall_email = ''
                 company.catchall_formatted = ''
 
-    @api.depends('partner_id.email_formatted', 'catchall_formatted')
+    @api.depends('partner_id', 'catchall_formatted')
     def _compute_email_formatted(self):
         for company in self:
             if company.partner_id.email_formatted:

addons/mail/tests/test_link_preview.py

@@ -1,28 +1,25 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
-from functools import partial
-import io
 
-from odoo.addons.mail.tests.common import mail_new_test_user
-from odoo.addons.mail.tests.common import MailCommon
-from odoo.addons.mail.tools import link_preview
+from markupsafe import Markup
 from unittest.mock import patch
+
+import io
 import requests
 
-discuss_channel_new_test_user = partial(mail_new_test_user, context={'discuss_channel_nosubscribe': False})
+from odoo.addons.mail.tests.common import MailCommon
+from odoo.addons.mail.tools import link_preview
+from odoo.tests import tagged
 
 
+@tagged('mail_link_preview', 'mail_message')
 class TestLinkPreview(MailCommon):
 
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
-        cls.user_1 = discuss_channel_new_test_user(
-            cls.env, login='user_1',
-            name='User 1',
-            groups='base.group_user')
-
-        cls.thread = cls.env['res.partner'].create({'name': 'a partner'})
+        cls.test_partner = cls.env['res.partner'].create({'name': 'a partner'})
+        cls.existing_message = cls.test_partner.message_post(body='Test')
         cls.title = 'Test title'
         cls.og_title = 'Test OG title'
         cls.og_description = 'Test OG description'
@@ -77,58 +74,13 @@ class TestLinkPreview(MailCommon):
         """
         return self._patched_get_html('image/png', content)
 
-    def test_01_link_preview_throttle(self):
-        with patch.object(requests.Session, 'get', self._patch_with_og_properties), patch.object(requests.Session, 'head', self._patch_head_html):
-            throttle = int(self.env['ir.config_parameter'].sudo().get_param('mail.link_preview_throttle', 99))
-            link_previews = []
-            for _ in range(throttle):
-                link_previews.append({'source_url': self.source_url, 'message_id': 1})
-            self.env['mail.link.preview'].create(link_previews)
-            message = self.env['mail.message'].create({
-                'model': self.thread._name,
-                'res_id': self.thread.id,
-                'body': f'<a href={self.source_url}>Nothing link</a>',
-            })
-            self.env['mail.link.preview']._create_link_previews(message)
-            link_preview_count = self.env['mail.link.preview'].search_count([('source_url', '=', self.source_url)])
-            self.assertEqual(link_preview_count, throttle + 1)
-
-    def test_02_link_preview_create(self):
-        with patch.object(requests.Session, 'get', self._patch_with_og_properties), patch.object(requests.Session, 'head', self._patch_head_html):
-            message = self.env['mail.message'].create({
-                'model': self.thread._name,
-                'res_id': self.thread.id,
-                'body': f'<a href={self.source_url}>Nothing link</a>',
-            })
-            self.env['mail.link.preview']._create_link_previews(message)
-            self.assertBusNotifications(
-                [(self.cr.dbname, 'res.partner', self.env.user.partner_id.id)],
-                message_items=[{
-                    'type': 'mail.record/insert',
-                    'payload': {
-                        'LinkPreview': [{
-                            'id': link_preview.id,
-                            'message': {'id': message.id},
-                            'image_mimetype': False,
-                            'og_description': self.og_description,
-                            'og_image': self.og_image,
-                            'og_mimetype': False,
-                            'og_title': self.og_title,
-                            'og_type': False,
-                            'og_site_name': False,
-                            'source_url': self.source_url,
-                        }] for link_preview in message.link_preview_ids
-                    }
-                }]
-            )
-
     def test_get_link_preview_from_url(self):
         test_cases = [
             (self._patch_with_og_properties, self.source_url),
             (self._patch_without_og_properties, self.source_url),
             (self._patch_with_image_mimetype, self.og_image),
         ]
-        test_asserts = [
+        expected_values = [
             {
                 'og_description': self.og_description,
                 'og_image': self.og_image,
@@ -154,7 +106,41 @@ class TestLinkPreview(MailCommon):
             },
         ]
         session = requests.Session()
-        for (get_patch, url), expected in zip(test_cases, test_asserts):
+        for (get_patch, url), expected in zip(test_cases, expected_values):
             with self.subTest(get_patch=get_patch, url=url, expected=expected), patch.object(requests.Session, 'get', get_patch):
                 preview = link_preview.get_link_preview_from_url(url, session)
                 self.assertEqual(preview, expected)
+
+    def test_link_preview(self):
+        with patch.object(requests.Session, 'get', self._patch_with_og_properties), patch.object(requests.Session, 'head', self._patch_head_html):
+            throttle = int(self.env['ir.config_parameter'].sudo().get_param('mail.link_preview_throttle', 99))
+            self.env['mail.link.preview'].create([
+                {'source_url': self.source_url, 'message_id': self.existing_message.id}
+                for _ in range(throttle)
+            ])
+            message = self.test_partner.message_post(
+                body=Markup(f'<a href={self.source_url}>Nothing link</a>'),
+            )
+            self.env['mail.link.preview']._create_from_message_and_notify(message)
+            link_preview_count = self.env['mail.link.preview'].search_count([('source_url', '=', self.source_url)])
+            self.assertEqual(link_preview_count, throttle + 1)
+            self.assertBusNotifications(
+                [(self.cr.dbname, 'res.partner', self.env.user.partner_id.id)],
+                message_items=[{
+                    'type': 'mail.record/insert',
+                    'payload': {
+                        'LinkPreview': [{
+                            'id': link_preview.id,
+                            'message': {'id': message.id},
+                            'image_mimetype': False,
+                            'og_description': self.og_description,
+                            'og_image': self.og_image,
+                            'og_mimetype': False,
+                            'og_title': self.og_title,
+                            'og_type': False,
+                            'og_site_name': False,
+                            'source_url': self.source_url,
+                        }] for link_preview in message.link_preview_ids
+                    }
+                }]
+            )

addons/test_mail/tests/test_message_post.py

@@ -499,7 +499,6 @@ class TestMessageNotify(TestMessagePostCommon):
 
         for parameters in [
             {'message_type': 'comment'},
-            {'canned_response_ids': []},
             {'child_ids': []},
             {'mail_ids': []},
             {'notification_ids': []},