[FW][FIX] mail: prevent to call _message_compute_subject outside of a thread model
While we run cron `Data Merge: Find Duplicate Records` then it raises a traceback because `data_merge. model` has no method `_message_compute_subject` This thread method is called in various steps when trying to post or notify a message. With this PR we ensure notifying non-thread models is still working as intended by being defensive when calling this method. sentry-3947126655 Task-3254379 closes odoo/odoo#117763 Forward-port-of: odoo/odoo#117636 Forward-port-of: odoo/odoo#113460 Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
@@ -884,7 +884,8 @@ class Message(models.Model):
|
||||
if message_sudo.model and message_sudo.res_id:
|
||||
record_sudo = self.env[message_sudo.model].browse(message_sudo.res_id).sudo()
|
||||
record_name = record_sudo.with_prefetch(thread_ids_by_model_name[message_sudo.model]).display_name
|
||||
default_subject = record_sudo._message_compute_subject()
|
||||
# if not a thread, display_name is the default subject
|
||||
default_subject = record_sudo._message_compute_subject() if hasattr(record_sudo, '_message_compute_subject') else record_name
|
||||
else:
|
||||
record_name = False
|
||||
default_subject = False
|
||||
|
||||
@@ -3347,7 +3347,7 @@ class MailThread(models.AbstractModel):
|
||||
:return: dictionary of values suitable for a <mail.mail> create;
|
||||
"""
|
||||
mail_subject = message.subject
|
||||
if not mail_subject and self:
|
||||
if not mail_subject and self and hasattr(self, '_message_compute_subject'):
|
||||
mail_subject = self._message_compute_subject()
|
||||
if not mail_subject:
|
||||
mail_subject = message.record_name
|
||||
|
||||
@@ -195,8 +195,8 @@ class MailComposer(models.TransientModel):
|
||||
for composer in self:
|
||||
composer._evaluate_res_domain()
|
||||
|
||||
@api.depends('composition_mode', 'model', 'parent_id', 'res_domain',
|
||||
'res_ids', 'template_id')
|
||||
@api.depends('composition_mode', 'model', 'parent_id', 'record_name',
|
||||
'res_domain', 'res_ids', 'template_id')
|
||||
def _compute_subject(self):
|
||||
""" Computation is coming either form template, either from context.
|
||||
When having a template with a value set, copy it (in batch mode) or
|
||||
@@ -212,8 +212,11 @@ class MailComposer(models.TransientModel):
|
||||
if (not subject and composer.model and
|
||||
composer.composition_mode == 'comment' and
|
||||
not composer.composition_batch):
|
||||
res_ids = composer._evaluate_res_ids()
|
||||
subject = self.env[composer.model].browse(res_ids)._message_compute_subject()
|
||||
if composer.model_is_thread:
|
||||
res_ids = composer._evaluate_res_ids()
|
||||
subject = self.env[composer.model].browse(res_ids)._message_compute_subject()
|
||||
else:
|
||||
subject = composer.record_name
|
||||
composer.subject = subject
|
||||
|
||||
@api.depends('composition_mode', 'model', 'res_domain', 'res_ids',
|
||||
|
||||
@@ -1201,34 +1201,41 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
|
||||
@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'})
|
||||
simple_record = self.env['mail.test.simple'].create({'name': 'TestA'})
|
||||
ticket_record = self.env['mail.test.ticket'].create({'name': 'Test1'})
|
||||
nonthread_record = self.env['mail.test.nothread'].create({'name': 'TestNoThread'})
|
||||
|
||||
# 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({
|
||||
# default behavior: 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)
|
||||
|
||||
# overridden in model
|
||||
_, message = self.env['mail.compose.message'].with_context(
|
||||
self._get_web_context(ticket_record, add_web=False, composition_mode='comment')
|
||||
).create({
|
||||
# default behavior without thread: use record name
|
||||
ctx = self._get_web_context(nonthread_record, add_web=False, default_composition_mode='comment')
|
||||
_, message = self.env['mail.compose.message'].with_context(ctx).create({
|
||||
'body': '<p>Test Body</p>',
|
||||
'partner_ids': self.env.user.partner_id.ids,
|
||||
})._action_send_mail()
|
||||
self.assertEqual(message.record_name, nonthread_record.name)
|
||||
self.assertEqual(message.subject, nonthread_record.name)
|
||||
|
||||
# custom subject
|
||||
ctx = self._get_web_context(ticket_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, ticket_record._message_compute_subject())
|
||||
self.assertIn(f'Ticket for {ticket_record.name}', message.subject)
|
||||
self.assertEqual(message.subject, f'Ticket for {ticket_record.name} on {ticket_record.datetime.strftime("%m/%d/%Y, %H:%M:%S")}')
|
||||
|
||||
# forced value
|
||||
_, message = self.env['mail.compose.message'].with_context(
|
||||
self._get_web_context(ticket_record, add_web=False, composition_mode='comment')
|
||||
).create({
|
||||
ctx = self._get_web_context(ticket_record, add_web=False, default_composition_mode='comment')
|
||||
_, message = self.env['mail.compose.message'].with_context(ctx).create({
|
||||
'body': '<p>Test Body</p>',
|
||||
'subject': 'Forced Subject',
|
||||
})._action_send_mail()
|
||||
self.assertEqual(message.subject, 'Forced Subject',)
|
||||
self.assertEqual(message.subject, 'Forced Subject')
|
||||
|
||||
@users('employee')
|
||||
@mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
|
||||
|
||||
@@ -97,16 +97,18 @@ class TestMessageValues(MailCommon):
|
||||
@mute_logger('odoo.models.unlink')
|
||||
def test_mail_message_format(self):
|
||||
record1 = self.env['mail.test.simple'].create({'name': 'Test1'})
|
||||
message = self.env['mail.message'].create([{
|
||||
'model': 'mail.test.simple',
|
||||
'res_id': record1.id,
|
||||
}])
|
||||
res = message.message_format()
|
||||
self.assertEqual(res[0].get('record_name'), 'Test1')
|
||||
|
||||
record1.write({"name": "Test2"})
|
||||
res = message.message_format()
|
||||
self.assertEqual(res[0].get('record_name'), 'Test2')
|
||||
record2 = self.env['mail.test.nothread'].create({'name': 'Test2'})
|
||||
messages = self.env['mail.message'].create([{
|
||||
'model': record._name,
|
||||
'res_id': record.id,
|
||||
} for record in [record1, record2]])
|
||||
for message, record in zip(messages, [record1, record2]):
|
||||
with self.subTest(record=record):
|
||||
formatted = message.message_format()[0]
|
||||
self.assertEqual(formatted['record_name'], record.name)
|
||||
record.write({'name': 'Just Test'})
|
||||
formatted = message.message_format()[0]
|
||||
self.assertEqual(formatted['record_name'], 'Just Test')
|
||||
|
||||
@mute_logger('odoo.models.unlink')
|
||||
def test_mail_message_format_access(self):
|
||||
|
||||
@@ -404,36 +404,63 @@ class TestNoThread(MailCommon, TestRecipients):
|
||||
""" Specific tests for cross models thread features """
|
||||
|
||||
@users('employee')
|
||||
def test_message_notify(self):
|
||||
""" Test notifying using model / res_id linking to a model not being
|
||||
mail.thread enabled. """
|
||||
def test_message_format(self):
|
||||
""" Test formatting of messages when linked to non-thread models.
|
||||
Format could be asked notably if an inbox notification due to a
|
||||
'message_notify' happens. """
|
||||
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,
|
||||
'subject': 'Test Notify',
|
||||
},
|
||||
'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,
|
||||
partner_ids=self.partner_2.ids,
|
||||
res_id=test_record.id,
|
||||
subject='Test Notify',
|
||||
)
|
||||
message = self.env['mail.message'].create({
|
||||
'model': test_record._name,
|
||||
'record_name': 'Not used in message_format',
|
||||
'res_id': test_record.id,
|
||||
})
|
||||
formatted = message.message_format()[0]
|
||||
self.assertEqual(formatted['default_subject'], test_record.name)
|
||||
self.assertEqual(formatted['record_name'], test_record.name)
|
||||
|
||||
test_record.write({'name': 'Just Test'})
|
||||
formatted = message.message_format()[0]
|
||||
self.assertEqual(formatted['default_subject'], 'Just Test')
|
||||
self.assertEqual(formatted['record_name'], 'Just Test')
|
||||
|
||||
@users('employee')
|
||||
def test_message_notify(self):
|
||||
""" Test notifying on non-thread models, using MailThread as an abstract
|
||||
class with model and res_id giving the record used for notification.
|
||||
|
||||
Test default subject computation is also tested. """
|
||||
test_record = self.env['mail.test.nothread'].create({
|
||||
'customer_id': self.partner_1.id,
|
||||
'name': 'Not A Thread',
|
||||
})
|
||||
|
||||
for subject in ["Test Notify", False]:
|
||||
with self.subTest():
|
||||
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,
|
||||
partner_ids=self.partner_2.ids,
|
||||
res_id=test_record.id,
|
||||
subject=subject,
|
||||
)
|
||||
|
||||
@users('employee')
|
||||
def test_message_notify_composer(self):
|
||||
|
||||
Reference in New Issue
Block a user