From abd93a6374a7f7aec3b5e6f2d0fcaedee44268f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 1 Feb 2023 08:06:27 +0000 Subject: [PATCH] [IMP] mail: reset composer mixin fields when removing template Purpose of this commit is to improve behavior or mail.compose.mixin when removing the template. It now voids the value to avoid having half baked definition on composers. Summary of behavior * when choosing a template: existing (non void) values override any value in the composer mixin; * when removing a template: reset values; * when going from templateA to templateB: works like choosing a template which means only non void values are taken. This may lead to half-baked content but trying to remove old template content based on _origin and content comparisons would be complicated for few added value; This makes the 'mail.compose.mixin' behaves like 'mail.compose.message' wizard which was recently updated at odoo/odoo#107356. Task-3093257 (Mail: The Composer Update) Part-of: odoo/odoo#106177 --- addons/mail/models/mail_composer_mixin.py | 19 +++++++++++++------ .../tests/test_mail_composer_mixin.py | 15 ++++----------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/addons/mail/models/mail_composer_mixin.py b/addons/mail/models/mail_composer_mixin.py index ddf9cf995b6..daf0aca55fd 100644 --- a/addons/mail/models/mail_composer_mixin.py +++ b/addons/mail/models/mail_composer_mixin.py @@ -38,18 +38,24 @@ class MailComposerMixin(models.AbstractModel): @api.depends('template_id') def _compute_subject(self): + """ Computation is coming either from template, either reset. When + having a template with a value set, copy it. When removing the + template, reset it. """ for composer_mixin in self: - if composer_mixin.template_id: + if composer_mixin.template_id.subject: composer_mixin.subject = composer_mixin.template_id.subject - elif not composer_mixin.subject: + elif not composer_mixin.template_id: composer_mixin.subject = False @api.depends('template_id') def _compute_body(self): + """ Computation is coming either from template, either reset. When + having a template with a value set, copy it. When removing the + template, reset it. """ for composer_mixin in self: - if composer_mixin.template_id: + if not tools.is_html_empty(composer_mixin.template_id.body_html): composer_mixin.body = composer_mixin.template_id.body_html - elif not composer_mixin.body: + elif not composer_mixin.template_id: composer_mixin.body = False @api.depends('body', 'template_id') @@ -67,8 +73,9 @@ class MailComposerMixin(models.AbstractModel): @api.depends('template_id') def _compute_lang(self): - """ Take value form template when set. When removing the template - reset the value to avoid keeping part of template configuration. """ + """ Computation is coming either from template, either reset. When + having a template with a value set, copy it. When removing the + template, reset it. """ for composer_mixin in self: if composer_mixin.template_id.lang: composer_mixin.lang = composer_mixin.template_id.lang diff --git a/addons/test_mail/tests/test_mail_composer_mixin.py b/addons/test_mail/tests/test_mail_composer_mixin.py index 24d52b076a9..b29b5a13e2a 100644 --- a/addons/test_mail/tests/test_mail_composer_mixin.py +++ b/addons/test_mail/tests/test_mail_composer_mixin.py @@ -79,24 +79,17 @@ class TestMailComposerMixin(TestMailCommon, TestRecipients): # template with void values: should not force void (TODO) composer.template_id = template_void.id - self.assertEqual(composer.body, f'


', - 'TODO: should not force void value') + self.assertEqual(composer.body, '

CustomBody for

') self.assertFalse(composer.body_has_template_value) self.assertEqual(composer.lang, template.lang) - self.assertEqual(composer.subject, False, - 'TODO: should not force void value') - # temporarily reput values - composer.body = template.body_html - composer.subject = template.subject + self.assertEqual(composer.subject, 'CustomSubject for {{ object.name }}') # reset template TOOD should reset composer.write({'template_id': False}) - self.assertEqual(composer.body, self.mail_template.body_html, - 'TODO: should reset') + self.assertFalse(composer.body) self.assertFalse(composer.body_has_template_value) self.assertFalse(composer.lang) - self.assertEqual(composer.subject, self.mail_template.subject, - 'TODO: should reset') + self.assertFalse(composer.subject) @users("employee") def test_rendering_custom(self):