[FIX] mail, sms: respect scheduled_date for notification

When a 'scheduled_date' is given to posting API notifications are delayed.
They are send using a cron running on a schedule model. However SMS are
not respecting this parameter. This is now fixed.

We also use sql.now() instead of datetime.now() when checking notification
delay. This leads to values that are consistent through the transaction
and avoid non deterministic behavior.

This leads to fixing a global mock of "cr.now" that has unexpected side
effects in composer tests. Indeed now that the scheduled notification checks
cursor now instead of datetime now this global mock leads to some notification
not being sent. We now mock cr.now() only for creating records, allowing
to effectively test templates using create_date for dynamic scheduled date
computation.

closes odoo/odoo#150911

Related: odoo/enterprise#55052
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>

addons/mail/models/mail_thread.py

@@ -2948,7 +2948,7 @@ class MailThread(models.AbstractModel):
         }
 
     @api.model
-    def _is_notification_scheduled(self, notify_cheduled_date):
+    def _is_notification_scheduled(self, notify_scheduled_date):
         """ Helper to check if notification are about to be scheduled. Eases
         overrides.
 
@@ -2959,12 +2959,10 @@ class MailThread(models.AbstractModel):
         :return bool: True if a valid datetime has been found and is in the
           future; False otherwise.
         """
-        if notify_cheduled_date:
-            parsed_datetime = self.env['mail.mail']._parse_scheduled_datetime(notify_cheduled_date)
-            notify_cheduled_date = parsed_datetime.replace(tzinfo=None) if parsed_datetime else False
-        return (
-            notify_cheduled_date and notify_cheduled_date > datetime.datetime.utcnow()
-        )
+        if notify_scheduled_date:
+            parsed_datetime = self.env['mail.mail']._parse_scheduled_datetime(notify_scheduled_date)
+            notify_scheduled_date = parsed_datetime.replace(tzinfo=None) if parsed_datetime else False
+        return notify_scheduled_date if notify_scheduled_date and notify_scheduled_date > self.env.cr.now() else False
 
     def _raise_for_invalid_parameters(self, parameter_names, forbidden_names=None, restricting_names=None):
         """ Helper to warn about invalid parameters (or fields).
@@ -3067,11 +3065,8 @@ class MailThread(models.AbstractModel):
             return recipients_data
 
         # if scheduled for later: add in queue instead of generating notifications
-        scheduled_date = kwargs.pop('scheduled_date', None)
+        scheduled_date = self._is_notification_scheduled(kwargs.pop('scheduled_date', None))
         if scheduled_date:
-            parsed_datetime = self.env['mail.mail']._parse_scheduled_datetime(scheduled_date)
-            scheduled_date = parsed_datetime.replace(tzinfo=None) if parsed_datetime else False
-        if scheduled_date and scheduled_date > datetime.datetime.utcnow():
             # send the message notifications at the scheduled date
             self.env['mail.message.schedule'].sudo().create({
                 'scheduled_datetime': scheduled_date,

addons/mail/tests/common.py

@@ -9,6 +9,7 @@ import time
 from ast import literal_eval
 from collections import defaultdict
 from contextlib import contextmanager
+from freezegun import freeze_time
 from functools import partial
 from lxml import html
 from unittest.mock import patch
@@ -44,6 +45,19 @@ class MockEmail(common.BaseCase, MockSmtplibCase):
         super(MockEmail, cls).setUpClass()
         cls._mc_enabled = False
 
+    # ------------------------------------------------------------
+    # UTILITY MOCKS
+    # ------------------------------------------------------------
+
+    @contextmanager
+    def mock_datetime_and_now(self, mock_dt):
+        """ Used when synchronization date (using env.cr.now()) is important
+        in addition to standard datetime mocks. Used mainly to detect sync
+        issues. """
+        with freeze_time(mock_dt), \
+             patch.object(self.env.cr, 'now', lambda: mock_dt):
+            yield
+
     # ------------------------------------------------------------
     # GATEWAY MOCK
     # ------------------------------------------------------------

addons/sms/models/mail_thread.py

@@ -208,8 +208,10 @@ class MailThread(models.AbstractModel):
         )
 
     def _notify_thread(self, message, msg_vals=False, **kwargs):
+        scheduled_date = self._is_notification_scheduled(kwargs.get('scheduled_date'))
         recipients_data = super(MailThread, self)._notify_thread(message, msg_vals=msg_vals, **kwargs)
-        self._notify_thread_by_sms(message, recipients_data, msg_vals=msg_vals, **kwargs)
+        if not scheduled_date:
+            self._notify_thread_by_sms(message, recipients_data, msg_vals=msg_vals, **kwargs)
         return recipients_data
 
     def _notify_thread_by_sms(self, message, recipients_data, msg_vals=False,

addons/test_mail/tests/test_mail_composer.py

@@ -5,7 +5,6 @@ import base64
 
 from ast import literal_eval
 from datetime import timedelta
-from freezegun import freeze_time
 from itertools import chain, product
 from unittest.mock import DEFAULT, patch
 
@@ -32,7 +31,6 @@ class TestMailComposer(MailCommon, TestRecipients):
 
         # force 'now' to ease test about schedulers
         cls.reference_now = FieldDatetime.from_string('2022-12-24 12:00:00')
-        cls.env.cr._now = cls.reference_now  # force create_date to check schedulers
 
         # ensure employee can create partners, necessary for templates
         cls.user_employee.write({
@@ -56,15 +54,16 @@ class TestMailComposer(MailCommon, TestRecipients):
         )
         cls.env.ref('mail.group_mail_template_editor').users -= cls.user_rendering_restricted
 
-        cls.test_record = cls.env['mail.test.ticket.mc'].with_context(cls._test_context).create({
-            'name': 'TestRecord',
-            'customer_id': cls.partner_1.id,
-            'user_id': cls.user_employee_2.id,
-        })
-        cls.test_records, cls.test_partners = cls._create_records_for_batch(
-            'mail.test.ticket.mc', 2,
-            additional_values={'user_id': cls.user_employee_2.id},
-        )
+        with cls.mock_datetime_and_now(cls, cls.reference_now):
+            cls.test_record = cls.env['mail.test.ticket.mc'].with_context(cls._test_context).create({
+                'name': 'TestRecord',
+                'customer_id': cls.partner_1.id,
+                'user_id': cls.user_employee_2.id,
+            })
+            cls.test_records, cls.test_partners = cls._create_records_for_batch(
+                'mail.test.ticket.mc', 2,
+                additional_values={'user_id': cls.user_employee_2.id},
+            )
 
         cls.test_report, cls.test_report_2, cls.test_report_3 = cls.env['ir.actions.report'].create([
             {
@@ -1679,7 +1678,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
                 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.mock_datetime_and_now(self.reference_now), \
                      self.capture_triggers(schedule_cron_id) as capt:
                     composer._action_send_mail()
 
@@ -1748,7 +1747,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
                     # 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.mock_datetime_and_now(self.reference_now + timedelta(days=3)):
                         self.env['mail.message.schedule'].sudo()._send_notifications_cron()
 
                         # monorecord: force_send notifications
@@ -2191,6 +2190,7 @@ class TestComposerResultsCommentStatus(TestMailComposer):
         cls.template.write({
             'auto_delete': False,
             'model_id': cls.env['ir.model']._get_id(cls.test_records._name),
+            'scheduled_date': False,
         })
 
     def test_assert_initial_data(self):
@@ -2591,7 +2591,7 @@ class TestComposerResultsMass(TestMailComposer):
                 self.assertEqual(composer.email_from, self.template.email_from)
 
                 with self.mock_mail_gateway(mail_unlink_sent=False), \
-                     freeze_time(self.reference_now):
+                     self.mock_datetime_and_now(self.reference_now):
                     composer._action_send_mail()
 
                     # partners created from raw emails
@@ -2616,7 +2616,7 @@ class TestComposerResultsMass(TestMailComposer):
                                          [self.reference_now + timedelta(days=2)] * 2)
 
                         # simulate cron queue at right time for sending
-                        with freeze_time(self.reference_now + timedelta(days=2)):
+                        with self.mock_datetime_and_now(self.reference_now + timedelta(days=2)):
                             self.env['mail.mail'].sudo().process_email_queue()
 
                         # everything should be sent now
@@ -3236,6 +3236,7 @@ class TestComposerResultsMassStatus(TestMailComposer):
         ])
         cls.template.write({
             'model_id': cls.env['ir.model']._get_id(cls.test_records._name),
+            'scheduled_date': False,
         })
 
     def test_assert_initial_data(self):

addons/test_mail/tests/test_message_post.py

@@ -1056,7 +1056,7 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase):
         test_record = self.test_record.with_env(self.env)
         test_record.message_subscribe((self.partner_1 | self.partner_admin).ids)
 
-        with freeze_time(now), \
+        with self.mock_datetime_and_now(now), \
              self.assertMsgWithoutNotifications(), \
              self.capture_triggers(cron_id) as capt:
             msg = test_record.message_post(
@@ -1076,12 +1076,12 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase):
         self.assertEqual(schedules.scheduled_datetime, scheduled_datetime)
 
         # trigger cron now -> should not sent as in future
-        with freeze_time(now):
+        with self.mock_datetime_and_now(now):
             self.env['mail.message.schedule'].sudo()._send_notifications_cron()
         self.assertTrue(schedules.exists(), msg='Should not have sent the message')
 
         # Send the scheduled message from the cron at right date
-        with freeze_time(now + timedelta(days=5)), self.mock_mail_gateway(mail_unlink_sent=True):
+        with self.mock_datetime_and_now(now + timedelta(days=5)), self.mock_mail_gateway(mail_unlink_sent=True):
             self.env['mail.message.schedule'].sudo()._send_notifications_cron()
         self.assertFalse(schedules.exists(), msg='Should have sent the message')
         # check notifications have been sent
@@ -1099,11 +1099,11 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase):
         })
 
         # Send the scheduled message from the CRON
-        with freeze_time(now + timedelta(days=5)), self.assertNoNotifications():
+        with self.mock_datetime_and_now(now + timedelta(days=5)), self.assertNoNotifications():
             self.env['mail.message.schedule'].sudo()._send_notifications_cron()
 
         # schedule in the past = send when posting
-        with freeze_time(now), \
+        with self.mock_datetime_and_now(now), \
              self.mock_mail_gateway(mail_unlink_sent=False), \
              self.capture_triggers(cron_id) as capt:
             msg = test_record.message_post(

addons/test_mail_sms/tests/test_sms_post.py

@@ -1,11 +1,16 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
+from datetime import datetime, timedelta
+
+from odoo.addons.base.tests.test_ir_cron import CronMixinCase
 from odoo.addons.sms.tests.common import SMSCommon
 from odoo.addons.test_mail_sms.tests.common import TestSMSRecipients
+from odoo.tests import tagged
 
 
-class TestSMSPost(SMSCommon, TestSMSRecipients):
+@tagged('sms_post')
+class TestSMSPost(SMSCommon, TestSMSRecipients, CronMixinCase):
     """ TODO
 
       * add tests for new mail.message and mail.thread fields;
@@ -209,6 +214,44 @@ class TestSMSPost(SMSCommon, TestSMSRecipients):
 
         self.assertSMSNotification([{'partner': self.partner_1}, {'number': self.random_numbers_san[0]}], self._test_body, messages)
 
+    def test_message_sms_schedule(self):
+        """ Test delaying notifications through scheduled_date usage """
+        cron_id = self.env.ref('mail.ir_cron_send_scheduled_message').id
+        now = datetime.utcnow().replace(second=0, microsecond=0)
+        scheduled_datetime = now + timedelta(days=5)
+
+        with self.mock_datetime_and_now(now), \
+             self.with_user('employee'), \
+             self.capture_triggers(cron_id) as capt, \
+             self.mockSMSGateway():
+            test_record = self.env['mail.test.sms'].browse(self.test_record.id)
+            messages = test_record._message_sms(
+                'Testing Scheduled Notifications',
+                partner_ids=self.partner_1.ids,
+                scheduled_date=scheduled_datetime,
+            )
+
+        self.assertEqual(capt.records.call_at, scheduled_datetime,
+                         msg='Should have created a cron trigger for the scheduled sending')
+        self.assertFalse(self._new_sms)
+        self.assertFalse(self._sms)
+
+        schedules = self.env['mail.message.schedule'].sudo().search([('mail_message_id', '=', messages.id)])
+        self.assertEqual(len(schedules), 1, msg='Should have scheduled the message')
+        self.assertEqual(schedules.scheduled_datetime, scheduled_datetime)
+
+        # trigger cron now -> should not sent as in future
+        with self.mock_datetime_and_now(now):
+            self.env['mail.message.schedule'].sudo()._send_notifications_cron()
+        self.assertTrue(schedules.exists(), msg='Should not have sent the message')
+
+        # Send the scheduled message from the cron at right date
+        with self.mock_datetime_and_now(now + timedelta(days=5)), self.mockSMSGateway():
+            self.env['mail.message.schedule'].sudo()._send_notifications_cron()
+        self.assertFalse(schedules.exists(), msg='Should have sent the message')
+        # check notifications have been sent
+        self.assertSMSNotification([{'partner': self.partner_1}], 'Testing Scheduled Notifications', messages)
+
     def test_message_sms_with_template(self):
         sms_template = self.env['sms.template'].create({
             'name': 'Test Template',
@@ -252,6 +295,7 @@ class TestSMSPost(SMSCommon, TestSMSRecipients):
         self.assertSMSNotification([{'partner': self.partner_1, 'number': self.test_numbers_san[1]}], 'Dear %s this is an SMS.' % self.test_record.display_name, messages)
 
 
+@tagged('sms_post')
 class TestSMSPostException(SMSCommon, TestSMSRecipients):
 
     @classmethod