[IMP] mail, various: use overridden method in message_notify

Purpose
=======

In message_notify, when called on a recordset, call model methods instead of
base one defined on MailThread. This allows to use internal methods overrides.

Also perform some linting on calls to ``message_notify`` in order to better
spot calls, parameters, ...

Task-2852908

closes odoo/odoo#92868

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

addons/crm/models/crm_lead.py

@@ -1768,6 +1768,9 @@ class Lead(models.Model):
         """ Handle salesman recipients that can convert leads into opportunities
         and set opportunities as won / lost. """
         groups = super(Lead, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
+
         local_msg_vals = dict(msg_vals or {})
 
         self.ensure_one()

addons/hr_holidays/models/hr_leave.py

@@ -1599,7 +1599,10 @@ class HolidaysRequest(models.Model):
                 recipient = leave.employee_id.address_home_id.id
 
             if recipient:
-                self.env['mail.thread'].sudo().message_notify(body=message, partner_ids=[recipient])
+                self.env['mail.thread'].sudo().message_notify(
+                    body=message,
+                    partner_ids=[recipient]
+                )
 
     def _track_subtype(self, init_values):
         if 'state' in init_values and self.state == 'validate':
@@ -1611,6 +1614,9 @@ class HolidaysRequest(models.Model):
         """ Handle HR users and officers recipients that can validate or refuse holidays
         directly from email. """
         groups = super(HolidaysRequest, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
+
         local_msg_vals = dict(msg_vals or {})
 
         self.ensure_one()

addons/hr_holidays/models/hr_leave_allocation.py

@@ -783,6 +783,9 @@ class HolidaysAllocation(models.Model):
         """ Handle HR users and officers recipients that can validate or refuse holidays
         directly from email. """
         groups = super(HolidaysAllocation, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
+
         local_msg_vals = dict(msg_vals or {})
 
         self.ensure_one()

addons/knowledge/models/knowledge_article.py

@@ -1437,8 +1437,10 @@ class Article(models.Model):
                 })
 
             self.with_context(lang=partner_lang).message_notify(
-                partner_ids=partner.ids, body=body, subject=subject,
-                email_layout_xmlid='mail.mail_notification_light'
+                body=body,
+                email_layout_xmlid='mail.mail_notification_light',
+                partner_ids=partner.ids,
+                subject=subject,
             )
 
     # ------------------------------------------------------------

addons/knowledge/tests/test_knowledge_performance.py

@@ -81,7 +81,7 @@ class KnowledgePerformanceCase(KnowledgeCommonWData):
     @users('employee')
     @warmup
     def test_article_invite_members(self):
-        with self.assertQueryCount(employee=86):  # knowledge only: 86
+        with self.assertQueryCount(employee=89):  # knowledge only: 89
             shared_article = self.shared_children.with_env(self.env)
             partners = (self.customer + self.partner_employee_manager + self.partner_employee2).with_env(self.env)
             shared_article.invite_members(partners, 'write')

addons/lunch/models/lunch_alert.py

@@ -192,4 +192,7 @@ class LunchAlert(models.Model):
 
         partners = self.env['lunch.order'].search(order_domain).user_id.partner_id
         if partners:
-            self.env['mail.thread'].message_notify(body=self.message, partner_ids=partners.ids)
+            self.env['mail.thread'].message_notify(
+                body=self.message,
+                partner_ids=partners.ids
+            )

addons/mail/models/mail_thread.py

@@ -2067,11 +2067,12 @@ class MailThread(models.AbstractModel):
             _logger.warning('Message notify called without recipient_ids, skipping')
             return self.env['mail.message']
 
+        # allow to link a notification to a document that does not inherit from
+        # MailThread by supporting model / res_id
         if not (model and res_id):  # both value should be set or none should be set (record)
             model = False
             res_id = False
 
-        MailThread = self.env['mail.thread']
         msg_values = {
             'parent_id': parent_id,
             'model': self._name if self else model,
@@ -2082,18 +2083,21 @@ class MailThread(models.AbstractModel):
             'author_id': author_id,
             'email_from': email_from,
             'partner_ids': partner_ids,
-            'subtype_id': self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note'),
             'is_internal': True,
             'record_name': False,
-            'reply_to': MailThread._notify_get_reply_to(default=email_from)[False],
             'message_id': tools.generate_tracking_message_id('message-notify'),
         }
         msg_values.update(msg_kwargs)
+        # add default-like values afterwards, to avoid useless queries
+        if 'subtype_id' not in msg_values:
+            msg_values['subtype_id'] = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note')
+        if 'reply_to' not in msg_values:
+            msg_values['reply_to'] = self._notify_get_reply_to(default=email_from)[self.id if self else False]
         if 'email_add_signature' not in msg_values:
             msg_values['email_add_signature'] = True
 
-        new_message = MailThread._message_create(msg_values)
-        MailThread._notify_thread(new_message, msg_values, **notif_kwargs)
+        new_message = self._message_create(msg_values)
+        self._notify_thread(new_message, msg_values, **notif_kwargs)
         return new_message
 
     def _message_log_with_view(self, views_or_xmlid, **kwargs):
@@ -2126,6 +2130,7 @@ class MailThread(models.AbstractModel):
             'email_add_signature': False,  # False as no notification -> no need to compute signature
         }
         msg_values.update(kwargs)
+
         return self.sudo()._message_create(msg_values)
 
     def _message_log_batch(self, bodies, author_id=None, email_from=None, subject=False, message_type='notification'):

addons/portal/models/portal_mixin.py

@@ -65,8 +65,11 @@ class PortalMixin(models.AbstractModel):
         return '%s?%s' % ('/mail/view' if redirect else self.access_url, url_encode(params))
 
     def _notify_get_recipients_groups(self, msg_vals=None):
-        access_token = self._portal_ensure_token()
         groups = super(PortalMixin, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
+
+        access_token = self._portal_ensure_token()
         local_msg_vals = dict(msg_vals or {})
 
         if access_token and 'partner_id' in self._fields and self['partner_id']:

addons/project/models/project.py

@@ -2222,6 +2222,9 @@ class Task(models.Model):
         access button to portal users and portal customers. If they are notified
         they should probably have access to the document. """
         groups = super(Task, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
+
         local_msg_vals = dict(msg_vals or {})
         self.ensure_one()
 

addons/purchase/models/purchase.py

@@ -340,7 +340,10 @@ class PurchaseOrder(models.Model):
         """ Tweak 'view document' button for portal customers, calling directly
         routes for confirm specific to PO model. """
         groups = super(PurchaseOrder, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
 
+        self.ensure_one()
         customer_portal_group = next(group for group in groups if group[0] == 'portal_customer')
         if customer_portal_group:
             access_opt = customer_portal_group[2].setdefault('button_access', {})

addons/sale/models/sale_order.py

@@ -1146,6 +1146,10 @@ class SaleOrder(models.Model):
         in sale. Customer and portal group have probably no right to see
         the document so they don't have the access button. """
         groups = super(SaleOrder, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
+
+        self.ensure_one()
         if self._context.get('proforma'):
             for group in [g for g in groups if g[0] in ('portal_customer', 'portal', 'follower', 'customer')]:
                 group[2]['has_button_access'] = False

addons/test_event_full/tests/test_performance.py

@@ -225,7 +225,7 @@ class TestRegistrationPerformance(EventPerformanceCase):
         """
         event = self.env['event.event'].browse(self.test_event.ids)
 
-        with freeze_time(self.reference_now), self.assertQueryCount(event_user=713):  # tef only: 672? - com runbot 710 - ent runbot 713
+        with freeze_time(self.reference_now), self.assertQueryCount(event_user=734):  # tef only: 672? - com runbot 731 - ent runbot 734
             self.env.cr._now = self.reference_now  # force create_date to check schedulers
             registration_values = [
                 dict(reg_data,
@@ -271,7 +271,7 @@ class TestRegistrationPerformance(EventPerformanceCase):
         form like) """
         event = self.env['event.event'].browse(self.test_event.ids)
 
-        with freeze_time(self.reference_now), self.assertQueryCount(event_user=724):  # tef only: 683? - com runbot 721 - ent runbot: 725
+        with freeze_time(self.reference_now), self.assertQueryCount(event_user=745):  # tef only: 683? - com runbot 742 - ent runbot: 745
             self.env.cr._now = self.reference_now  # force create_date to check schedulers
             registration_values = [
                 dict(reg_data,
@@ -292,7 +292,7 @@ class TestRegistrationPerformance(EventPerformanceCase):
         """ Test a single registration creation using Form """
         event = self.env['event.event'].browse(self.test_event.ids)
 
-        with freeze_time(self.reference_now), self.assertQueryCount(event_user=210):  # tef only: 208? - com runbot: 198 - ent runbot: 210
+        with freeze_time(self.reference_now), self.assertQueryCount(event_user=212):  # tef only: 208? - com runbot: 200 - ent runbot: 212
             self.env.cr._now = self.reference_now  # force create_date to check schedulers
             with Form(self.env['event.registration']) as reg_form:
                 reg_form.event_id = event
@@ -308,7 +308,7 @@ class TestRegistrationPerformance(EventPerformanceCase):
         """ Test a single registration creation using Form """
         event = self.env['event.event'].browse(self.test_event.ids)
 
-        with freeze_time(self.reference_now), self.assertQueryCount(event_user=212):  # tef only: 211? - com runbot: 199 - ent runbot: 212
+        with freeze_time(self.reference_now), self.assertQueryCount(event_user=214):  # tef only: 211? - com runbot: 201 - ent runbot: 214
             self.env.cr._now = self.reference_now  # force create_date to check schedulers
             with Form(self.env['event.registration']) as reg_form:
                 reg_form.event_id = event
@@ -335,7 +335,7 @@ class TestRegistrationPerformance(EventPerformanceCase):
         event = self.env['event.event'].browse(self.test_event.ids)
 
         # simple customer data
-        with freeze_time(self.reference_now), self.assertQueryCount(event_user=136):  # tef only: 133? - com runbot: 134 - ent runbot: 136
+        with freeze_time(self.reference_now), self.assertQueryCount(event_user=138):  # tef only: 133? - com runbot: 136 - ent runbot: 138
             self.env.cr._now = self.reference_now  # force create_date to check schedulers
             registration_values = dict(
                 self.customer_data[0],
@@ -349,7 +349,7 @@ class TestRegistrationPerformance(EventPerformanceCase):
         event = self.env['event.event'].browse(self.test_event.ids)
 
         # partner-based customer
-        with freeze_time(self.reference_now), self.assertQueryCount(event_user=142):  # tef only: 141? - com runbot: 141 - ent runbot: 142
+        with freeze_time(self.reference_now), self.assertQueryCount(event_user=144):  # tef only: 141? - com runbot: 143 - ent runbot: 144
             self.env.cr._now = self.reference_now  # force create_date to check schedulers
             registration_values = {
                 'event_id': event.id,
@@ -379,7 +379,7 @@ class TestRegistrationPerformance(EventPerformanceCase):
         event = self.env['event.event'].browse(self.test_event.ids)
 
         # website customer data
-        with freeze_time(self.reference_now), self.assertQueryCount(event_user=144):  # tef only: 140? - com runbot: 140 - ent runbot: 144
+        with freeze_time(self.reference_now), self.assertQueryCount(event_user=146):  # tef only: 140? - com runbot: 142 - ent runbot: 146
             self.env.cr._now = self.reference_now  # force create_date to check schedulers
             registration_values = dict(
                 self.website_customer_data[0],

addons/test_mail/__manifest__.py

@@ -9,7 +9,10 @@
     'description': """This module contains tests related to mail. Those are
 present in a separate module as it contains models used only to perform
 tests independently to functional aspects of other models. """,
-    'depends': ['test_performance', 'mail'],
+    'depends': [
+        'mail',
+        'test_performance',
+    ],
     'data': [
         'security/ir.model.access.csv',
         'security/test_mail_security.xml',

addons/test_mail/models/test_mail_corner_case_models.py

@@ -68,6 +68,7 @@ class MailTestLang(models.Model):
 
     def _notify_get_recipients_groups(self, msg_vals=None):
         groups = super(MailTestLang, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+
         local_msg_vals = dict(msg_vals or {})
 
         for group in [g for g in groups if g[0] in('follower', 'customer')]:
@@ -79,6 +80,9 @@ class MailTestLang(models.Model):
             ]
         return groups
 
+# ------------------------------------------------------------
+# TRACKING MODELS
+# ------------------------------------------------------------
 
 class MailTestTrackCompute(models.Model):
     _name = 'mail.test.track.compute'
@@ -90,6 +94,7 @@ class MailTestTrackCompute(models.Model):
     partner_email = fields.Char(related='partner_id.email', store=True, tracking=True)
     partner_phone = fields.Char(related='partner_id.phone', tracking=True)
 
+
 class MailTestTrackMonetary(models.Model):
     _name = 'mail.test.track.monetary'
     _description = 'Test tracking monetary field'
@@ -99,6 +104,7 @@ class MailTestTrackMonetary(models.Model):
     company_currency = fields.Many2one("res.currency", string='Currency', related='company_id.currency_id', readonly=True, tracking=True)
     revenue = fields.Monetary('Revenue', currency_field='company_currency', tracking=True)
 
+
 class MailTestTrackAll(models.Model):
     _name = 'mail.test.track.all'
     _description = 'Test tracking on all field types'
@@ -118,6 +124,10 @@ class MailTestTrackAll(models.Model):
     selection_field = fields.Selection(string='Selection', selection=[['first', 'FIRST']], tracking=True)
     text_field = fields.Text('Text', tracking=True)
 
+# ------------------------------------------------------------
+# OTHER
+# ------------------------------------------------------------
+
 class MailTestMultiCompany(models.Model):
     """ This model can be used in multi company tests"""
     _name = 'mail.test.multi.company'
@@ -126,3 +136,14 @@ class MailTestMultiCompany(models.Model):
 
     name = fields.Char()
     company_id = fields.Many2one('res.company')
+
+
+class MailTestNotMailThread(models.Model):
+    """ Models not inheriting from mail.thread but using some cross models
+    capabilities of mail. """
+    _name = 'mail.test.nothread'
+    _description = "NoThread Model"
+
+    name = fields.Char()
+    company_id = fields.Many2one('res.company')
+    customer_id = fields.Many2one('res.partner')

addons/test_mail/security/ir.model.access.csv

@@ -29,6 +29,8 @@ access_mail_test_lang_portal,mail.test.lang.portal,model_mail_test_lang,base.gro
 access_mail_test_lang_user,mail.test.lang.user,model_mail_test_lang,base.group_user,1,1,1,1
 access_mail_test_multi_company_user,mail.test.multi.company.user,model_mail_test_multi_company,base.group_user,1,1,1,1
 access_mail_test_multi_company_portal,mail.test.multi.company.portal,model_mail_test_multi_company,base.group_portal,1,0,0,0
+access_mail_test_nothread_user,mail.test.nothread.user,model_mail_test_nothread,base.group_user,1,1,1,1
+access_mail_test_nothread_portal,mail.test.nothread.portal,model_mail_test_nothread,base.group_portal,1,0,0,0
 access_mail_test_track_compute,mail.test.track.compute,model_mail_test_track_compute,base.group_user,1,1,1,1
 access_mail_test_track_monetary,mail.test.track.monetary,model_mail_test_track_monetary,base.group_user,1,1,1,1
 access_mail_test_track_all,mail.test.track.all,model_mail_test_track_all,base.group_user,1,1,1,1

addons/test_mail/tests/test_mail_thread_internals.py

@@ -263,13 +263,15 @@ class TestDiscuss(TestMailCommon, TestRecipients):
     @users("employee")
     def test_unlink_notification_message(self):
         channel = self.env['mail.channel'].create({'name': 'testChannel'})
-        channel.message_notify(
+        notification_msg = channel.with_user(self.user_admin).message_notify(
             body='test',
             message_type='user_notification',
             partner_ids=[self.partner_2.id],
-            author_id=2
         )
 
+        with self.assertRaises(exceptions.AccessError):
+            notification_msg.with_env(self.env)._message_format(['id', 'body', 'date', 'author_id', 'email_from'])
+
         channel_message = self.env['mail.message'].sudo().search([('model', '=', 'mail.channel'), ('res_id', 'in', channel.ids)])
         self.assertEqual(len(channel_message), 1, "Test message should have been posted")
 
@@ -338,3 +340,37 @@ class TestMultiCompany(HttpCase):
         action = url_decode(fragment)['action']
 
         self.assertEqual(action, 'mail.action_discuss')
+
+
+@tagged('mail_thread')
+class TestNoThread(TestMailCommon, TestRecipients):
+    """ Specific tests for cross models thread features """
+
+    @users('employee')
+    def test_message_notify(self):
+        test_record = self.env['mail.test.nothread'].create({
+            'customer_id': self.partner_1.id,
+            'name': 'Not A Thread',
+        })
+        with self.assertPostNotifications([{
+                'content': 'Hello Paulo',
+                'email_values': {
+                    'reply_to': self.company_admin.catchall_formatted,
+                },
+                'message_type': 'user_notification',
+                'notif': [{
+                    'check_send': True,
+                    'is_read': True,
+                    'partner': self.partner_2,
+                    'status': 'sent',
+                    'type': 'email',
+                }],
+                'subtype': 'mail.mt_note',
+            }]):
+            _message = self.env['mail.thread'].message_notify(
+                body='<p>Hello Paulo</p>',
+                model=test_record._name,
+                res_id=test_record.id,
+                subject='Test Notify',
+                partner_ids=self.partner_2.ids
+            )

addons/test_mail/tests/test_performance.py

@@ -522,7 +522,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
     def test_message_assignation_email(self):
         self.user_test.write({'notification_type': 'email'})
         record = self.env['mail.test.track'].create({'name': 'Test'})
-        with self.assertQueryCount(__system__=26, employee=27):
+        with self.assertQueryCount(__system__=27, employee=28):
             record.write({
                 'user_id': self.user_test.id,
             })
@@ -531,7 +531,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
     @warmup
     def test_message_assignation_inbox(self):
         record = self.env['mail.test.track'].create({'name': 'Test'})
-        with self.assertQueryCount(__system__=17, employee=19):
+        with self.assertQueryCount(__system__=18, employee=20):
             record.write({
                 'user_id': self.user_test.id,
             })
@@ -806,7 +806,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
         })
         rec1 = rec.with_context(active_test=False)      # to see inactive records
         self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id)
-        with self.assertQueryCount(__system__=23, employee=24):
+        with self.assertQueryCount(__system__=24, employee=25):
             rec.write({'user_id': self.user_portal.id})
         self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id)
         # write tracking message
@@ -826,7 +826,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
         customer_id = self.customer.id
         user_id = self.user_portal.id
 
-        with self.assertQueryCount(__system__=55, employee=56):
+        with self.assertQueryCount(__system__=56, employee=57):
             rec = self.env['mail.test.ticket'].create({
                 'name': 'Test',
                 'container_id': container_id,

addons/website_blog/models/website_blog.py

@@ -276,7 +276,10 @@ class BlogPost(models.Model):
     def _notify_get_recipients_groups(self, msg_vals=None):
         """ Add access button to everyone if the document is published. """
         groups = super(BlogPost, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
 
+        self.ensure_one()
         if self.website_published:
             for _group_name, _group_method, group_data in groups:
                 group_data['has_button_access'] = True

addons/website_forum/models/forum.py

@@ -923,7 +923,10 @@ class Post(models.Model):
     def _notify_get_recipients_groups(self, msg_vals=None):
         """ Add access button to everyone if the document is active. """
         groups = super(Post, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
 
+        self.ensure_one()
         if self.state == 'active':
             for _group_name, _group_method, group_data in groups:
                 group_data['has_button_access'] = True

addons/website_sale/models/sale_order.py

@@ -382,7 +382,10 @@ class SaleOrder(models.Model):
         """ In case of cart recovery email, update link to redirect directly
         to the cart (like ``mail_template_sale_cart_recovery`` template). """
         groups = super(SaleOrder, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
 
+        self.ensure_one()
         customer_portal_group = next(group for group in groups if group[0] == 'portal_customer')
         if customer_portal_group:
             access_opt = customer_portal_group[2].setdefault('button_access', {})

addons/website_slides/models/slide_slide.py

@@ -730,7 +730,10 @@ class Slide(models.Model):
     def _notify_get_recipients_groups(self, msg_vals=None):
         """ Add access button to everyone if the document is active. """
         groups = super(Slide, self)._notify_get_recipients_groups(msg_vals=msg_vals)
+        if not self:
+            return groups
 
+        self.ensure_one()
         if self.website_published:
             for _group_name, _group_method, group_data in groups:
                 group_data['has_button_access'] = True