[REF] mail: improve default record-based data in composer

Purpose is to ease understanding of those default values and prepare move
towards editable stored computed fields by rewriting a bit the code to better
understand its purpose.

While being at it, some tests introduced recently are moved at their right
place now that everything is ready to rewrite the posting API and the
composer. Some tests are added to ensure notification-specific methods are
called along with the newly-introduced '_message_compute_subject', aka methods
to add custom header and custom mail values.

Task-2710804 (Mail: Clean MailThread Posting API)
Prepares Task-2088884 (Mail: Use editable computed stored fields in compo

Part-of: odoo/odoo#99482

addons/mail/models/mail_thread.py

@@ -2544,10 +2544,10 @@ class MailThread(models.AbstractModel):
         return parent_id
 
     def _message_compute_subject(self):
-        """Overridable method to get the default subject for a message related to this record.
+        """ Get the default subject for a message posted in this record's
+        discussion thread.
 
-        :return str:
-        """
+        :return str: default subject """
         self.ensure_one()
         return self.name_get()[0][1]
 
@@ -3107,17 +3107,14 @@ class MailThread(models.AbstractModel):
         to notify (mail_message_id and its fields), server, references, subject.
         """
         mail_subject = message.subject
-        if not message.subject:
-            thread = self
-            if not thread and message.model:
-                thread = self.env[message.model].browse(message.res_id)
-            if not thread:
-                mail_subject = message.record_name
-            else:
-                mail_subject = thread._message_compute_subject()
+        if not mail_subject and self:
+            mail_subject = self._message_compute_subject()
+        if not mail_subject:
+            mail_subject = message.record_name
+        if mail_subject:
+            # replace new lines by spaces to conform to email headers requirements
+            mail_subject = ' '.join(mail_subject.splitlines())
 
-        # Replace new lines by spaces to conform to email headers requirements
-        mail_subject = ' '.join((mail_subject or '').splitlines())
         # compute references: set references to the parent and add current message just to
         # have a fallback in case replies mess with Messsage-Id in the In-Reply-To (e.g. amazon
         # SES SMTP may replace Message-Id and In-Reply-To refers an internal ID not stored in Odoo)

addons/mail/wizard/mail_compose_message.py

@@ -290,25 +290,31 @@ class MailComposer(models.TransientModel):
         wizard when sending an email related a previous email (parent_id) or
         a document (model, res_id). This is based on previously computed default
         values. """
-        result, subject = {}, False
+        result, record_name, subject = {}, False, False
+        model, res_id = values.get('model'), values.get('res_id')
         if values.get('parent_id'):
             parent = self.env['mail.message'].browse(values.get('parent_id'))
-            result['record_name'] = parent.record_name
-            if not values.get('model'):
-                result['model'] = parent.model
-            if not values.get('res_id'):
-                result['res_id'] = parent.res_id
-            partner_ids = values.get('partner_ids', list()) + parent.partner_ids.ids
-            result['partner_ids'] = partner_ids
-            record = self.env[values.get('model') or result['model']].browse(values.get('res_id') or result['res_id'])
-            parent_subject = tools.ustr(parent.subject or '')
-            subject = parent_subject or record._message_compute_subject()
-        elif values.get('model') and values.get('res_id'):
-            record = self.env[values['model']].browse(values['res_id'])
-            doc_name = record.display_name
-            result['record_name'] = doc_name or ''
+            if not model:
+                model = parent.model
+                result['model'] = model
+            if not res_id:
+                res_id = parent.res_id
+                result['res_id'] = res_id
+            result['partner_ids'] = values.get('partner_ids', list()) + parent.partner_ids.ids
+            record_name = parent.record_name
+            subject = tools.ustr(parent.subject or '')
+            if (not subject or not record_name) and model and res_id:
+                record = self.env[model].browse(res_id)
+                if not record_name:
+                    record_name = record.display_name
+                if not subject:
+                    subject = record._message_compute_subject()
+        elif model and res_id:
+            record = self.env[model].browse(res_id)
+            record_name = record.display_name or ''
             subject = record._message_compute_subject()
 
+        result['record_name'] = record_name
         result['subject'] = subject
 
         return result

addons/test_mail/models/test_mail_models.py

@@ -14,6 +14,21 @@ class MailTestSimple(models.Model):
     name = fields.Char()
     email_from = fields.Char()
 
+    def _message_compute_subject(self):
+        """ To ease mocks """
+        _a = super()._message_compute_subject()
+        return _a
+
+    def _notify_by_email_get_final_mail_values(self, *args, **kwargs):
+        """ To ease mocks """
+        _a = super()._notify_by_email_get_final_mail_values(*args, **kwargs)
+        return _a
+
+    def _notify_by_email_get_headers(self):
+        headers = super()._notify_by_email_get_headers()
+        headers['X-Custom'] = 'Done'
+        return headers
+
 
 class MailTestGateway(models.Model):
     """ A very simple model only inheriting from mail.thread to test pure mass

addons/test_mail/tests/test_mail_composer.py

@@ -121,6 +121,8 @@ class TestComposerForm(TestMailComposer):
         self.assertFalse(composer_form.reply_to_force_new)
         self.assertEqual(composer_form.res_id, self.test_record.id)
         self.assertEqual(composer_form.subject, self.test_record._message_compute_subject())
+        self.assertIn(f'Ticket for {self.test_record.name}', composer_form.subject,
+                      'Check effective content')
         self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment'))
 
     @users('employee')
@@ -626,6 +628,8 @@ class TestComposerInternals(TestMailComposer):
                     self.assertEqual(composer.record_name, 'Manual update',
                                      'TODO: Reset not called')
                     self.assertEqual(composer.subject, self.test_record._message_compute_subject())
+                    self.assertIn(f'Ticket for {self.test_record.name}', composer.subject,
+                                  'Check effective content')
                 else:
                     self.assertFalse(composer.body)
                     # self.assertFalse(composer.mail_server_id.id)
@@ -693,25 +697,6 @@ class TestComposerInternals(TestMailComposer):
                 self.assertEqual(composer.record_name, 'CustomName')
                 self.assertEqual(composer.subject, 'My amazing subject')
 
-    @users('employee')
-    def test_mail_composer_default_subject(self):
-        """Make sure the default subject is applied in the composer."""
-        simple_record = self.env['mail.test.simple'].create({'name': 'TestA'})
-        ticket_record = self.env['mail.test.ticket'].create({'name': 'Test1'})
-        # default behaviour, use the record name
-        ctx = self._get_web_context(simple_record, add_web=False, default_composition_mode='comment')
-        _, message = self.env['mail.compose.message'].with_context(ctx).create({
-            'body': '<p>Test Body</p>',
-        })._action_send_mail()
-
-        self.assertEqual(message.subject, simple_record.name)
-        # custom subject
-        ctx = self._get_web_context(ticket_record, add_web=False, default_composition_mode='comment')
-        _, messages = self.env['mail.compose.message'].with_context(ctx).create({
-            'body': '<p>Test Body</p>',
-        })._action_send_mail()
-        self.assertEqual(messages.subject, ticket_record._message_compute_subject())
-
     @users('employee')
     @mute_logger('odoo.models.unlink')
     def test_mail_composer_recipients(self):
@@ -884,10 +869,12 @@ class TestComposerInternals(TestMailComposer):
     def test_mail_composer_parent(self):
         """ Test specific management in comment mode when having parent_id set:
         record_name, subject, parent's partners. """
-        subject = "Parent Subject"
-        parent = self.test_record.message_post(subject=subject,
-                                               body='Test',
-                                               partner_ids=(self.partner_1 + self.partner_2).ids)
+        parent_subject = "Parent Subject"
+        parent = self.test_record.message_post(
+            body='Test',
+            partner_ids=(self.partner_1 + self.partner_2).ids,
+            subject=parent_subject,
+        )
 
         composer = self.env['mail.compose.message'].with_context(
             self._get_web_context(self.test_record, add_web=False, default_parent_id=parent.id)
@@ -900,7 +887,7 @@ class TestComposerInternals(TestMailComposer):
         self.assertEqual(composer.parent_id, parent)
         self.assertEqual(composer.partner_ids, self.partner_1 + self.partner_2)
         self.assertEqual(composer.record_name, self.test_record.name)
-        self.assertEqual(composer.subject, subject)
+        self.assertEqual(composer.subject, parent_subject)
 
     @users('user_rendering_restricted')
     @mute_logger('odoo.tests', 'odoo.addons.base.models.ir_rule', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
@@ -992,6 +979,38 @@ class TestComposerResultsComment(TestMailComposer):
     """ Test global output of composer used in comment mode. Test notably
     notification and emails generated during this process. """
 
+    @users('employee')
+    def test_mail_composer_default_subject(self):
+        """ Make sure the default subject is applied in the composer. """
+        simple_record = self.env['mail.test.simple'].create({'name': 'TestSimple'})
+        ticket_record = self.env['mail.test.ticket'].create({'name': 'TestTicket'})
+
+        # default behavior: use record name
+        _, message = self.env['mail.compose.message'].with_context(
+            self._get_web_context(simple_record, add_web=False, composition_mode='comment')
+        ).create({
+            'body': '<p>Test Body</p>',
+        })._action_send_mail()
+        self.assertEqual(message.subject, simple_record.name)
+
+        # overridden in model
+        _, message = self.env['mail.compose.message'].with_context(
+            self._get_web_context(ticket_record, add_web=False, composition_mode='comment')
+        ).create({
+            'body': '<p>Test Body</p>',
+        })._action_send_mail()
+        self.assertEqual(message.subject, ticket_record._message_compute_subject())
+        self.assertIn(f'Ticket for {ticket_record.name}', message.subject)
+
+        # forced value
+        _, message = self.env['mail.compose.message'].with_context(
+            self._get_web_context(ticket_record, add_web=False, composition_mode='comment')
+        ).create({
+            'body': '<p>Test Body</p>',
+            'subject': 'Forced Subject',
+        })._action_send_mail()
+        self.assertEqual(message.subject, 'Forced Subject',)
+
     @users('employee')
     @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
     def test_mail_composer_notifications_delete(self):

addons/test_mail/tests/test_message_post.py

@@ -325,20 +325,6 @@ class TestMessageNotify(TestMessagePostCommon):
                 subject='Notify Subject',
             )
 
-    @users('employee')
-    def test_notify_default_subject(self):
-        """ Test notify in batch. Currently not supported. """
-        test_partner = self.env['res.partner'].sudo().create([{'name': 'John', 'email': 'john@test.lan'}])
-        test_record = self.env['mail.test.ticket'].create({'name': 'Test'})
-        mt_comment = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment')
-        test_record.message_subscribe([test_partner.id], subtype_ids=[mt_comment])
-
-        with self.mock_mail_gateway(), self.mock_mail_app():
-            test_record.message_post(body="Test Message", message_type="comment", subtype_id=mt_comment)
-
-        self.assertEqual(len(self._new_msgs), 1)
-        self.assertEqual(self._new_msgs.subject, test_record._message_compute_subject())
-
     @users('employee')
     @mute_logger('odoo.addons.mail.models.mail_mail')
     def test_notify_from_user_id(self):
@@ -630,20 +616,35 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase):
                 partner_ids=self.partner_portal.ids,
             )
 
-    @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
+    @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink', 'odoo.tests')
     @users('employee')
     def test_message_post_defaults(self):
         """ Test default values when posting a classic message. """
+        _original_compute_subject = MailTestSimple._message_compute_subject
+        _original_notify_headers = MailTestSimple._notify_by_email_get_headers
+        _original_notify_mailvals = MailTestSimple._notify_by_email_get_final_mail_values
         test_record = self.env['mail.test.simple'].create([{'name': 'Defaults'}])
         creation_msg = test_record.message_ids
         self.assertEqual(len(creation_msg), 1)
 
-        with self.mock_mail_app():
+        with patch.object(MailTestSimple, '_message_compute_subject',
+                          autospec=True, side_effect=_original_compute_subject) as mock_compute_subject, \
+             patch.object(MailTestSimple, '_notify_by_email_get_headers',
+                          autospec=True, side_effect=_original_notify_headers) as mock_notify_headers, \
+             patch.object(MailTestSimple, '_notify_by_email_get_final_mail_values',
+                          autospec=True, side_effect=_original_notify_mailvals) as mock_notify_mailvals, \
+             self.mock_mail_app():
             new_message = test_record.message_post(
                 body='Body',
                 partner_ids=[self.partner_employee_2.id],
             )
 
+        self.assertEqual(mock_compute_subject.call_count, 1,
+                         'Should call model-based subject computation for outgoing emails')
+        self.assertEqual(mock_notify_headers.call_count, 1,
+                         'Should call model-based headers computation for outgoing emails')
+        self.assertEqual(mock_notify_mailvals.call_count, 1,
+                         'Should call model-based headers computation for outgoing emails')
         self.assertMessageFields(
             new_message,
             {'author_id': self.partner_employee,
@@ -657,6 +658,7 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase):
              'record_name': test_record.name,
              'reply_to': formataddr((f'{self.company_admin.name} {test_record.name}', f'{self.alias_catchall}@{self.alias_domain}')),
              'res_id': test_record.id,
+             'subject': test_record.name,
              'subtype_id': self.env.ref('mail.mt_note'),
             }
         )