diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index aa1a5621414..4896930c645 100644 --- a/addons/mail/models/mail_thread.py +++ b/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) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index d3cca56b7d7..76a23ba305b 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/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 diff --git a/addons/test_mail/models/test_mail_models.py b/addons/test_mail/models/test_mail_models.py index 60976cb9c9f..dee3b5da83d 100644 --- a/addons/test_mail/models/test_mail_models.py +++ b/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 diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index dfabe4cb5d7..93ff0335c3d 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/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': '

Test Body

', - })._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': '

Test Body

', - })._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': '

Test Body

', + })._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': '

Test Body

', + })._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': '

Test Body

', + '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): diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 2e1f05ccf0b..fe492e9c55d 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/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'), } )