From fbcc1bf4c05440bcc55d47cd6f2f61326f54aecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 19 Sep 2022 14:23:43 +0000 Subject: [PATCH] [IMP] mail: support 'scheduled_date' from template on composer RATIONALE Improve usage of composer in comment or email mode: support batch-posting in comment, support more configuration from templates, improve global model. SPECIFICATIONS Purpose of this task is to correctly support scheduled_date from template on both comment and mass mode in the composer. It is currently mainly supported at template level, when using a template to directly send emails. In this commit we add a field on composer that takes the value from the template, and propagate it to the mail or messages created when validating it. As most template fields it can contain inline template code to be rendered dynamically on target records, hence using a char field. Its rendering is done using template that calls _parse_scheduled_datetime. It allows to have an UTC and timezone agnostic value. SCHEDULED_DATE SUPPORT When posting a comment, scheduled posts uses the 'mail.message.schedule' mechanism that creates the message but send notifications later. When sending a mailing, emails have a scheduled_date set. As the 'send' method does not check for scheduled_date (only the cron queue) we have to filter emails scheduled in the future before calling the 'send' method. Task-2993872 (Mail: Support scheduled date in all composer flows) Part-of: odoo/odoo#99482 --- addons/mail/__manifest__.py | 2 +- addons/mail/wizard/mail_compose_message.py | 26 ++- .../wizard/mail_compose_message_views.xml | 1 + addons/test_mail/tests/test_mail_composer.py | 172 ++++++++++++++---- 4 files changed, 164 insertions(+), 37 deletions(-) diff --git a/addons/mail/__manifest__.py b/addons/mail/__manifest__.py index 15ab81398f7..b3a38a36a40 100644 --- a/addons/mail/__manifest__.py +++ b/addons/mail/__manifest__.py @@ -2,7 +2,7 @@ { 'name': 'Discuss', - 'version': '1.11', + 'version': '1.12', 'category': 'Productivity/Discuss', 'sequence': 145, 'summary': 'Chat, mail gateway and private channels', diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index d9547eb929e..b92fdb65867 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -3,6 +3,7 @@ import ast import base64 +import datetime import logging from odoo import _, api, fields, models, tools, Command @@ -175,6 +176,11 @@ class MailComposer(models.TransientModel): 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_message = fields.Boolean('Delete Message Copy', help='Do not keep a copy of the email in the document communication history (mass mailing only)') mail_server_id = fields.Many2one('ir.mail_server', 'Outgoing mail server') + scheduled_date = fields.Char( + 'Scheduled Date', + help="In comment mode: if set, postpone notifications sending. " + "In mass mail mode: if sent, send emails after that date. " + "This date is considered as being in UTC timezone.") @api.constrains('res_ids') def _check_res_ids(self): @@ -252,6 +258,7 @@ class MailComposer(models.TransientModel): (field, template[field]) for field in ('email_from', 'reply_to', + 'scheduled_date', 'subject', ) if template[field] @@ -278,6 +285,7 @@ class MailComposer(models.TransientModel): 'partner_ids', 'reply_to', 'report_template', + 'scheduled_date', 'subject', ) )[template_res_ids[0]] @@ -311,6 +319,7 @@ class MailComposer(models.TransientModel): 'partner_ids', 'reply_to', 'res_ids', + 'scheduled_date', 'subject', ]) values = dict( @@ -321,6 +330,7 @@ class MailComposer(models.TransientModel): 'mail_server_id', 'partner_ids', 'reply_to', + 'scheduled_date', 'subject', ) if key in default_values) @@ -479,7 +489,17 @@ class MailComposer(models.TransientModel): records = self.env[self.model].browse(res_ids_iter) if self.model and hasattr(self.env[self.model], 'message_post') else False if records: records._message_mail_after_hook(iter_mails_sudo) - iter_mails_sudo.send(auto_commit=auto_commit) + + # as 'send' does not filter out scheduled mails (only 'process_email_queue' + # does) we need to do it manually + iter_mails_sudo_tosend = iter_mails_sudo.filtered( + lambda mail: ( + not mail.scheduled_date or + mail.scheduled_date <= datetime.datetime.utcnow() + ) + ) + if iter_mails_sudo_tosend: + iter_mails_sudo_tosend.send(auto_commit=auto_commit) return mails_sudo @@ -547,6 +567,7 @@ class MailComposer(models.TransientModel): STA - 'record_name', (to check, single post or False in email) DYN - 'reply_to', STA - 'reply_to_force_new', + DYN - 'scheduled_date', DYN - 'subject', STA - 'subtype_id', @@ -664,6 +685,7 @@ class MailComposer(models.TransientModel): res_id: { 'body': bodies[res_id], # should be void 'email_from': emails_from[res_id], + 'scheduled_date': False, 'subject': subjects[res_id], # some fields are specific to mail or message **( @@ -688,6 +710,7 @@ class MailComposer(models.TransientModel): 'mail_server_id', 'partner_ids', 'report_template', + 'scheduled_date', ) ) for res_id in res_ids: @@ -797,6 +820,7 @@ class MailComposer(models.TransientModel): 'body': self.body or '', 'email_from': self.email_from, 'partner_ids': self.partner_ids.ids, + 'scheduled_date': self.scheduled_date, 'subject': self.subject or '', } for res_id in res_ids diff --git a/addons/mail/wizard/mail_compose_message_views.xml b/addons/mail/wizard/mail_compose_message_views.xml index 6425d72410a..a2627444937 100644 --- a/addons/mail/wizard/mail_compose_message_views.xml +++ b/addons/mail/wizard/mail_compose_message_views.xml @@ -25,6 +25,7 @@ + Test Body />

', 'mail_server_id': self.mail_server_global.id, 'record_name': 'Manual update', + 'scheduled_date': '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}', 'subject': 'Back to my amazing subject for {{ record.name }}', }) self.assertEqual(composer.body, '

Back to my amazing body />

') self.assertEqual(composer.mail_server_id, self.mail_server_global) self.assertEqual(composer.record_name, 'Manual update') + self.assertEqual(composer.scheduled_date, '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}') self.assertEqual(composer.subject, 'Back to my amazing subject for {{ record.name }}') else: self.assertEqual(composer.body, '

Back to my amazing body />

'}) composer.write({'template_id': False}) # currently onchange necessary composer._onchange_template_id_wrapper() @@ -666,6 +693,9 @@ class TestComposerInternals(TestMailComposer): # self.assertEqual(composer.record_name, self.test_record.name) self.assertEqual(composer.record_name, 'Manual update', 'TODO: Reset not called') + # self.assertFalse(composer.scheduled_date) + self.assertEqual(composer.scheduled_date, '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}', + 'TODO: Values are kept (should be reset ?)') self.assertEqual(composer.subject, self.test_record._message_compute_subject()) self.assertIn(f'Ticket for {self.test_record.name}', composer.subject, 'Check effective content') @@ -677,6 +707,9 @@ class TestComposerInternals(TestMailComposer): # self.assertFalse(composer.record_name) self.assertEqual(composer.record_name, 'Manual update', 'TODO: Reset not called') + # self.assertFalse(composer.scheduled_date) + self.assertEqual(composer.scheduled_date, '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}', + 'TODO: Values are kept (should be reset ?)') # self.assertFalse(composer.subject) self.assertEqual(composer.subject, 'Back to my amazing subject for {{ record.name }}', 'TODO: Values are kept (should be reset ?)') @@ -694,11 +727,13 @@ class TestComposerInternals(TestMailComposer): self.assertEqual(composer.body, f'

TemplateBody {self.test_record.name}

') self.assertEqual(composer.mail_server_id, self.template.mail_server_id) self.assertEqual(composer.record_name, self.test_record.name) + self.assertEqual(FieldDatetime.from_string(composer.scheduled_date), self.reference_now + timedelta(days=2)) self.assertEqual(composer.subject, f'TemplateSubject {self.test_record.name}') else: self.assertEqual(composer.body, self.template.body_html) self.assertEqual(composer.mail_server_id, self.template.mail_server_id) self.assertFalse(composer.record_name) + self.assertEqual(composer.scheduled_date, self.template.scheduled_date) self.assertEqual(composer.subject, self.template.subject) # 3. check at create @@ -714,11 +749,13 @@ class TestComposerInternals(TestMailComposer): self.assertEqual(composer.body, f'

TemplateBody {self.test_record.name}

') self.assertEqual(composer.mail_server_id, self.template.mail_server_id) self.assertEqual(composer.record_name, self.test_record.name) + self.assertEqual(FieldDatetime.from_string(composer.scheduled_date), self.reference_now + timedelta(days=2)) self.assertEqual(composer.subject, f'TemplateSubject {self.test_record.name}') else: self.assertEqual(composer.body, self.template.body_html) self.assertEqual(composer.mail_server_id, self.template.mail_server_id) self.assertFalse(composer.record_name) + self.assertEqual(composer.scheduled_date, self.template.scheduled_date) self.assertEqual(composer.subject, self.template.subject) # 4. template + user input @@ -727,6 +764,7 @@ class TestComposerInternals(TestMailComposer): 'body': '

Test Body

', 'mail_server_id': False, 'record_name': 'CustomName', + 'scheduled_date': '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}', 'subject': 'My amazing subject', }) @@ -734,6 +772,7 @@ class TestComposerInternals(TestMailComposer): self.assertEqual(composer.body, '

Test Body

') self.assertEqual(composer.mail_server_id.id, False) self.assertEqual(composer.record_name, 'CustomName') + self.assertEqual(composer.scheduled_date, '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}') self.assertEqual(composer.subject, 'My amazing subject') @users('employee') @@ -1015,7 +1054,7 @@ class TestComposerInternals(TestMailComposer): @tagged('mail_composer') -class TestComposerResultsComment(TestMailComposer): +class TestComposerResultsComment(TestMailComposer, CronMixinCase): """ Test global output of composer used in comment mode. Test notably notification and emails generated during this process. """ @@ -1175,14 +1214,24 @@ class TestComposerResultsComment(TestMailComposer): self.assertEqual(message.partner_ids, self.partner_1 | self.partner_2) @users('employee') - @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail', 'odoo.addons.mail.models.mail_message_schedule') def test_mail_composer_wtpl_complete(self): """ Test a posting process using a complex template, holding several additional recipients and attachments. It is done in monorecord and in batch since this is now supported. This tests notifies: 2 new email_to (+ 1 duplicated), 1 email_cc, - test_records followers and partner_admin added in partner_to.""" + test_records followers and partner_admin added in partner_to. + + Global notification + * monorecord: send notifications right away (force_send=True) + * multirecord: delay notification sending (force_send=False) + + Use cases + * scheduled_date: creates mail.message.schedule (no email sent), then + scheduling send notifications with notification parameters kept + * otherwise: global behavior + """ attachment_data = self._generate_attachments_data(2, self.template._name, self.template.id) email_to_1 = 'test.to.1@test.example.com' email_to_2 = 'test.to.2@test.example.com' @@ -1200,8 +1249,14 @@ class TestComposerResultsComment(TestMailComposer): attachs = self.env['ir.attachment'].search([('name', 'in', [a['name'] for a in attachment_data])]) self.assertEqual(len(attachs), 2) - for test_records, batch in ((self.test_record, False), (self.test_records, True)): - with self.subTest(test_records=test_records, batch=batch): + for batch, scheduled_date in product( + (False, True), + (False, '{{ (object.create_date or datetime.datetime(2022, 12, 26, 18, 0, 0)) + datetime.timedelta(days=2) }}') + ): + with self.subTest(batch=batch, scheduled_date=scheduled_date): + self.template.write({'scheduled_date': scheduled_date}) + test_records = self.test_records if batch else self.test_record + # ensure initial data self.assertEqual(len(test_records.customer_id), len(test_records)) self.assertEqual(test_records.user_id, self.user_employee_2) @@ -1211,37 +1266,72 @@ class TestComposerResultsComment(TestMailComposer): composer_form = Form(self.env['mail.compose.message'].with_context( self._get_web_context(test_records, add_web=True, default_composition_mode='comment', - default_template_id=self.template.id) + default_template_id=self.template.id, + # avoid successive tests issues with followers + mail_create_nosubscribe=True, + ) )) composer = composer_form.save() self.assertFalse(composer.reply_to_force_new, 'Mail: thread-enabled models should use auto thread by default') - with self.mock_mail_gateway(mail_unlink_sent=False), self.mock_mail_app(): + + # due to scheduled_date, cron for sending notification will be used + schedule_cron_id = self.env.ref('mail.ir_cron_send_scheduled_message').id + with self.mock_mail_gateway(mail_unlink_sent=False), \ + self.mock_mail_app(), \ + freeze_time(self.reference_now), \ + self.capture_triggers(schedule_cron_id) as capt: composer._action_send_mail() - # check new partners have been created based on emails given - new_partners = self.env['res.partner'].search([ - ('email', 'in', [email_to_1, email_to_2, email_to_3, email_cc_1]) - ]) - self.assertEqual(len(new_partners), 3) - self.assertEqual(set(new_partners.mapped('email')), - set(['test.to.1@test.example.com', 'test.to.2@test.example.com', 'test.cc.1@test.example.com']) - ) - - # global outgoing: one mail.mail (all customer recipients, then all employee recipients) - # and 5 emails, and 1 inbox notification (admin) - self.assertEqual(len(self._new_mails), 2 * len(test_records), - 'Should have created 1 mail.mail per group per record') + # notification process should not have been sent + if scheduled_date: + self.assertFalse(self._new_mails) + self.assertFalse(self._mails) # monorecord: force_send notifications - if not batch: - self.assertEqual(len(self._mails), 5, 'Should have sent 5 emails, one per recipient per record') + elif not batch: self.assertEqual(self._new_mails.mapped('state'), ['sent'] * len(self._new_mails)) - # mutlirecord: use email queue + self.assertEqual(len(self._mails), 5, 'Should have sent 5 emails, one per recipient per record') + # multirecord: use email queue else: - self.assertEqual(len(self._mails), 0, 'Should have put emails in queue and not sent any emails') self.assertEqual(self._new_mails.mapped('state'), ['outgoing'] * len(self._new_mails)) + self.assertEqual(len(self._mails), 0, 'Should have put emails in queue and not sent any emails') # simulate cron sending emails self.env['mail.mail'].sudo().process_email_queue() + # notification process should not have been sent + if scheduled_date: + self.assertEqual( + capt.records.mapped('call_at'), [self.reference_now + timedelta(days=2)] * len(test_records), + msg='Should have created a cron trigger for the scheduled sending' + ) + else: + self.assertFalse(capt.records) + + # check new partners have been created based on emails given + new_partners = self.env['res.partner'].search([ + ('email', 'in', [email_to_1, email_to_2, email_to_3, email_cc_1]) + ]) + self.assertEqual(len(new_partners), 3) + self.assertEqual( + set(new_partners.mapped('email')), + set(['test.to.1@test.example.com', 'test.to.2@test.example.com', 'test.cc.1@test.example.com']) + ) + + # if scheduled_date is set: simulate cron for sending notifications + if scheduled_date: + # Send the scheduled message from the CRON + with self.mock_mail_gateway(mail_unlink_sent=False), \ + self.mock_mail_app(), \ + freeze_time(self.reference_now + timedelta(days=3)): + self.env['mail.message.schedule'].sudo()._send_notifications_cron() + + # global outgoing: one mail.mail (all customer recipients, then all employee recipients) + # and 5 emails, and 1 inbox notification (admin) + self.assertEqual(len(self._new_mails), 2 * len(test_records), + 'Should have created 1 mail.mail per group per record') + self.assertEqual(self._new_mails.mapped('state'), ['sent'] * len(self._new_mails)) + self.assertEqual(len(self._mails), 5 * len(test_records), + 'Should have sent 5 emails, one per recipient per record') + # template is sent only to partners (email_to are transformed) for test_record in test_records: message = test_record.message_ids[0] @@ -1507,6 +1597,9 @@ class TestComposerResultsMass(TestMailComposer): def test_mail_composer_wtpl_complete(self): """ Test a composer in mass mode with a quite complete template, containing notably email-based recipients and attachments. """ + # as we use the email queue, don't have failing tests due to other outgoing emails + self.env['mail.mail'].sudo().search([]).unlink() + attachment_data = self._generate_attachments_data(2, self.template._name, self.template.id) email_to_1 = 'test.to.1@test.example.com' email_to_2 = 'test.to.2@test.example.com' @@ -1534,16 +1627,25 @@ class TestComposerResultsMass(TestMailComposer): default_template_id=self.template.id) )) composer = composer_form.save() - with self.mock_mail_gateway(mail_unlink_sent=False): + with self.mock_mail_gateway(mail_unlink_sent=False), \ + freeze_time(self.reference_now): composer._action_send_mail() - new_partners = self.env['res.partner'].search([ - ('email', 'in', [email_to_1, email_to_2, email_to_3, email_cc_1]) - ]) - self.assertEqual(len(new_partners), 3) + new_partners = self.env['res.partner'].search([ + ('email', 'in', [email_to_1, email_to_2, email_to_3, email_cc_1]) + ]) + self.assertEqual(len(new_partners), 3) + + # global outgoing: emails not sent due to scheduled_date + self.assertEqual(len(self._new_mails), 2, 'Should have created 1 mail.mail per record') + self.assertEqual(len(self._mails), 0, 'Should not send emails, scheduled in the future') + self.assertEqual(self._new_mails.mapped('scheduled_date'), + [self.reference_now + timedelta(days=2)] * 2) + + # simulate cron queue at right time for sending + with freeze_time(self.reference_now + timedelta(days=2)): + self.env['mail.mail'].sudo().process_email_queue() - # global outgoing - self.assertEqual(len(self._new_mails), 2, 'Should have created 1 mail.mail per record') self.assertEqual(len(self._mails), 10, 'Should have sent 5 emails per record') # hack to use assertEmails: filtering on from/to only is not sufficient to distinguish emails