From 7ee2e2d5d3bb186042b54ee16bae6cd3cf1816ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 19 Dec 2022 14:45:12 +0000 Subject: [PATCH] [REF] mail: move composer 'auto_delete_*' fields from onchange to computed RATIONALE Remove _onchange_template_id / defaults usage for composer fields. It lacks control and make value computation hard to understand and predict. PURPOSE Split _onchange_template_id + default_get + get_record_data usage into editable computed stored fields. Each field should have its own method with its triggers (template, composition mode, ...). SPECIFICATIONS Improve composer to correctly compute field ``auto_delete(_keep_log)`` in compute methods. For auto_delete, computation is coming either from template, either from composition mode. When having a template, its value copied. Without template it is True in comment mode to remove notification emails by default. In email mode we keep emails (backward compatibility mode). For auto_delete_keep_log, it is used only in email mode. It is used to keep the core message when unlinking sent emails. It allows to keep the message as a trace in the record's chatter. In other modes it has no use and can be set to False. When auto_delete is turned off it has no usage. Task-2088884 (Mail: Use editable computed stored fields in composer) Part-of: odoo/odoo#107356 --- .../tests/test_mail_performance.py | 4 +- addons/mail/wizard/mail_compose_message.py | 48 ++++++++++++------- addons/test_mail/tests/test_mail_composer.py | 30 ++++++------ addons/test_mail/tests/test_message_post.py | 2 +- 4 files changed, 48 insertions(+), 36 deletions(-) diff --git a/addons/l10n_generic_coa/tests/test_mail_performance.py b/addons/l10n_generic_coa/tests/test_mail_performance.py index 716e3d8d491..7a4d19b58b3 100644 --- a/addons/l10n_generic_coa/tests/test_mail_performance.py +++ b/addons/l10n_generic_coa/tests/test_mail_performance.py @@ -302,7 +302,7 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance): )), }, fields_values={ - 'auto_delete': False, + 'auto_delete': True, 'email_from': self.user_account_other.email_formatted, 'is_notification': True, # should keep logs by default 'mail_server_id': self.mail_server_global, @@ -406,7 +406,7 @@ class TestAccountComposerPerformance(BaseMailAccountPerformance): )), }, fields_values={ - 'auto_delete': False, + 'auto_delete': True, 'email_from': self.user_account_other.email_formatted, 'is_notification': True, # should keep logs by default 'mail_server_id': self.mail_server_global, diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 07433cad0d0..bda09c5cc3d 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -101,9 +101,6 @@ class MailComposer(models.TransientModel): if 'create_uid' in fields_list and 'create_uid' not in result: result['create_uid'] = self.env.uid - # comment mode by default removes emails - if 'auto_delete' in fields_list and 'auto_delete' not in result and result.get('composition_mode') == 'comment': - result['auto_delete'] = True # batch post mode by default use queues for notifications if 'force_send' in fields_list and 'force_send' not in result: result['force_send'] = ( @@ -186,10 +183,13 @@ class MailComposer(models.TransientModel): 'wizard_id', 'partner_id', 'Additional Contacts', domain=_partner_ids_domain) # sending - auto_delete = fields.Boolean('Delete Emails', + auto_delete = fields.Boolean( + 'Delete Emails', + compute="_compute_auto_delete", readonly=False, store=True, help='This option permanently removes any track of email after it\'s been sent, including from the Technical menu in the Settings, in order to preserve storage space of your Odoo database.') auto_delete_keep_log = fields.Boolean( - 'Keep Message Copy', default=True, + 'Keep Message Copy', + compute="_compute_auto_delete_keep_log", readonly=False, store=True, help='Keep a copy of the email content if emails are removed (mass mailing only)') force_send = fields.Boolean( 'Send mailing or notifications directly') @@ -282,6 +282,31 @@ class MailComposer(models.TransientModel): for composer in self: composer.reply_to_force_new = composer.reply_to_mode == 'new' + @api.depends('composition_mode', 'template_id') + def _compute_auto_delete(self): + """ Computation is coming either from template, either from composition + mode. When having a template, its value is copied. Without template it + is True in comment mode to remove notification emails by default. In + email mode we keep emails (backward compatibility mode). """ + for composer in self: + if composer.template_id: + composer.auto_delete = composer.template_id.auto_delete + else: + composer.auto_delete = composer.composition_mode == 'comment' + + @api.depends('composition_mode', 'auto_delete') + def _compute_auto_delete_keep_log(self): + """ Keep logs is used only in email mode. It is used to keep the core + message when unlinking sent emails. It allows to keep the message as + a trace in the record's chatter. In other modes it has no use and + can be set to False. When auto_delete is turned off it has no usage. """ + toreset = self.filtered( + lambda comp: comp.composition_mode != 'mass_mail' or + not comp.auto_delete + ) + toreset.auto_delete_keep_log = False + (self - toreset).auto_delete_keep_log = True + # Overrides of mail.render.mixin @api.depends('model') def _compute_render_model(self): @@ -324,8 +349,7 @@ class MailComposer(models.TransientModel): template = self.env['mail.template'].browse(template_id) values = dict( (field, template[field]) - for field in ('auto_delete', - 'email_from', + for field in ('email_from', 'reply_to', 'scheduled_date', 'subject', @@ -346,7 +370,6 @@ class MailComposer(models.TransientModel): self.env['mail.template'].browse(template_id), template_res_ids, ('attachment_ids', - 'auto_delete', 'body_html', 'email_cc', 'email_from', @@ -380,8 +403,6 @@ class MailComposer(models.TransientModel): default_model=model, default_res_ids=res_ids ).default_get(['attachment_ids', - 'auto_delete', - 'auto_delete_keep_log', 'body', 'composition_mode', 'email_from', @@ -397,8 +418,6 @@ class MailComposer(models.TransientModel): values = dict( (key, default_values[key]) for key in ('attachment_ids', - 'auto_delete', - 'auto_delete_keep_log', 'body', 'email_from', 'mail_server_id', @@ -761,7 +780,6 @@ class MailComposer(models.TransientModel): # some fields are specific to mail or message **( { - 'auto_delete': self.auto_delete, 'body_html': bodies[res_id], 'res_id': res_id, } if email_mode else {}), @@ -775,7 +793,6 @@ class MailComposer(models.TransientModel): self.template_id, res_ids, ('attachment_ids', - 'auto_delete', 'email_to', 'email_cc', 'mail_server_id', @@ -787,9 +804,6 @@ class MailComposer(models.TransientModel): for res_id in res_ids: # remove attachments from template values as they should not be rendered template_values[res_id].pop('attachment_ids', None) - # auto_delete is not supported in batch post, only in mass mail - if not email_mode: - template_values[res_id].pop('auto_delete', False) mail_values_all[res_id].update(template_values[res_id]) # Handle recipients. Without template, if no partner_ids is given, update diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index b21b05714e0..92025741e3e 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -128,7 +128,7 @@ class TestComposerForm(TestMailComposer): self._get_web_context(self.test_record, add_web=True) )) self.assertTrue(composer_form.auto_delete, 'MailComposer: comment mode should remove notification emails by default') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertFalse(composer_form.body) self.assertFalse(composer_form.composition_batch) @@ -206,7 +206,7 @@ class TestComposerForm(TestMailComposer): self._get_web_context(self.test_record, add_web=True, default_template_id=self.template.id) )) self.assertTrue(composer_form.auto_delete, 'Should take template value') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertEqual(composer_form.body, f'

TemplateBody {self.test_record.name}

') self.assertFalse(composer_form.composition_batch) @@ -236,7 +236,7 @@ class TestComposerForm(TestMailComposer): default_template_id=self.template.id), )) self.assertTrue(composer_form.auto_delete, 'Should take composer value') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertEqual(composer_form.body, self.template.body_html, 'MailComposer: comment in batch mode should have template raw body if template') @@ -267,7 +267,7 @@ class TestComposerForm(TestMailComposer): default_template_id=self.template.id, )) self.assertTrue(composer_form.auto_delete, 'Should take composer value') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertEqual(composer_form.body, self.template.body_html, 'MailComposer: comment in batch mode should have template raw body if template') @@ -299,7 +299,7 @@ class TestComposerForm(TestMailComposer): default_template_id=self.template.id, )) self.assertTrue(composer_form.auto_delete, 'Should take composer value') - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: keep_log makes no sense in comment mode, only auto_delete') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertEqual(composer_form.body, '

TemplateBody

') self.assertFalse(composer_form.composition_batch) @@ -327,7 +327,7 @@ class TestComposerForm(TestMailComposer): self._get_web_context(self.test_records, add_web=True) )) self.assertFalse(composer_form.auto_delete) - self.assertTrue(composer_form.auto_delete_keep_log) + self.assertFalse(composer_form.auto_delete_keep_log, 'MailComposer: if emails are kept, logs are automatically kept') self.assertEqual(composer_form.author_id, self.env.user.partner_id) self.assertFalse(composer_form.body) self.assertTrue(composer_form.composition_batch) @@ -659,9 +659,10 @@ class TestComposerInternals(TestMailComposer): # default creation values if composition_mode == 'comment': self.assertTrue(composer.auto_delete, 'By default, remove notification emails') + self.assertFalse(composer.auto_delete_keep_log, 'Not used in comment mode') else: self.assertFalse(composer.auto_delete, 'By default, keep mailing emails') - self.assertTrue(composer.auto_delete_keep_log) + self.assertFalse(composer.auto_delete_keep_log, 'Emails are not unlinked, logs are already kept') self.assertTrue(composer.email_add_signature) self.assertEqual(composer.email_layout_xmlid, 'mail.test_layout') self.assertEqual(composer.message_type, 'comment') @@ -675,7 +676,7 @@ class TestComposerInternals(TestMailComposer): # values come from template if composition_mode == 'comment': self.assertTrue(composer.auto_delete) - self.assertTrue(composer.auto_delete_keep_log) + self.assertFalse(composer.auto_delete_keep_log, 'Not used in comment mode') self.assertTrue(composer.email_add_signature, 'TODO: should be False as template negates this config') self.assertEqual(composer.email_layout_xmlid, 'mail.test_layout') self.assertEqual(composer.message_type, 'comment') @@ -704,14 +705,12 @@ class TestComposerInternals(TestMailComposer): composer._onchange_template_id_wrapper() if composition_mode == 'comment': - # self.assertFalse(composer.auto_delete, 'TODO: should be updated') - self.assertTrue(composer.auto_delete) + self.assertFalse(composer.auto_delete) self.assertEqual(composer.message_type, 'notification') self.assertEqual(composer.subtype_id, self.env.ref('mail.mt_note')) self.assertTrue(composer.subtype_is_log) else: - # self.assertFalse(composer.auto_delete, 'TODO: should be updated') - self.assertTrue(composer.auto_delete) + self.assertFalse(composer.auto_delete) self.assertEqual(composer.message_type, 'notification') self.assertEqual(composer.subtype_id, self.env.ref('mail.mt_note')) self.assertTrue(composer.subtype_is_log) @@ -1238,7 +1237,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): 'partner_ids': [(4, self.partner_1.id), (4, self.partner_2.id)] }) self.assertTrue(composer.auto_delete, 'Comment mode removes notification emails by default') - self.assertTrue(composer.auto_delete_keep_log) + self.assertFalse(composer.auto_delete_keep_log, 'Not used in comment mode') with self.mock_mail_gateway(mail_unlink_sent=True): composer._action_send_mail() @@ -1260,7 +1259,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): 'partner_ids': [(4, self.partner_1.id), (4, self.partner_2.id)] }) self.assertFalse(composer.auto_delete) - self.assertTrue(composer.auto_delete_keep_log) + self.assertFalse(composer.auto_delete_keep_log, 'Not used in comment mode') with self.mock_mail_gateway(mail_unlink_sent=True): composer._action_send_mail() @@ -1684,8 +1683,7 @@ class TestComposerResultsMass(TestMailComposer): self.assertEqual(len(self._mails), 2, 'Should have sent 1 email per record') self.assertEqual(len(self._new_mails), 2, 'Should have created 1 mail.mail per record') - # self.assertEqual(self._new_mails.exists(), self._new_mails, 'Should not have deleted mail.mail records') - self.assertFalse(self._new_mails.exists(), 'TODO: Template is forced over composer value, which is not correct') + self.assertEqual(self._new_mails.exists(), self._new_mails, 'Should not have deleted mail.mail records') self.assertEqual(len(self._new_msgs), 2, 'Should have created 1 mail.mail per record') self.assertEqual(self._new_msgs.exists(), self._new_msgs, 'Should not have deleted mail.message records') diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index d4e7994173b..9555980e5cb 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -1231,7 +1231,7 @@ class TestMessagePostHelpers(TestMessagePostCommon): fields_values={ 'auto_delete': False, 'is_internal': False, - 'is_notification': True, # auto_delete_keep_log -> keep underlying mail.message + 'is_notification': False, # no to_delete -> no keep_log 'message_type': 'email', 'model': test_record._name, 'notified_partner_ids': self.env['res.partner'],