From cd5850d83d2579465db6f72ec04137b9a2779f92 Mon Sep 17 00:00:00 2001
From: std-odoo <std@odoo.com>
Date: Tue, 28 Jun 2022 06:53:17 +0000
Subject: [PATCH 1/7] [REF] mail: use a datetime instead of a char for the mail
 scheduled date
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Purpose
=======

Historically, we use a char field on the <mail.mail> to match the field type
on the <mail.template>. But even it's useful to have a char field on the mail
template (which can contains QWeb code or Jinja previously to Odoo 15.0) it is
not that useful for the <mail.mail> to keep this char type, because the value
is already rendered. Moreover this forces to have some code convert and store
datetime values using the standard format and in UTC.

We now correctly use a datetime field, as all other fields of that kind in
Odoo.

Task-2826699 (Mail: use datetime for scheduled_date mail field instead of char)
Prepares Task-2207626 (Rating: Delay rating notification to ease feedback)

Part-of: odoo/odoo#95623
Co-authored-by: Thibault Delavallée <tde@odoo.com>
---
 addons/mail/models/mail_mail.py          | 14 +++++---------
 addons/mail/models/mail_template.py      |  1 -
 addons/test_mail/tests/test_mail_mail.py |  7 +++----
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py
index afef9897340..02f8edbb129 100644
--- a/addons/mail/models/mail_mail.py
+++ b/addons/mail/models/mail_mail.py
@@ -85,7 +85,7 @@ class MailMail(models.Model):
         'Auto Delete',
         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.")
     to_delete = fields.Boolean('To Delete', help='If set, the mail will be deleted during the next Email Queue CRON run.')
-    scheduled_date = fields.Char('Scheduled Send Date',
+    scheduled_date = fields.Datetime('Scheduled Send Date',
         help="If set, the queue manager will send the email after the date. If not set, the email will be send as soon as possible. Unless a timezone is specified, it is considered as being in UTC timezone.")
     fetchmail_server_id = fields.Many2one('fetchmail.server', "Inbound Mail Server", readonly=True)
 
@@ -132,10 +132,9 @@ class MailMail(models.Model):
                 values['is_notification'] = True
             if values.get('scheduled_date'):
                 parsed_datetime = self._parse_scheduled_datetime(values['scheduled_date'])
-                if parsed_datetime:
-                    values['scheduled_date'] = parsed_datetime.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT)
-                else:
-                    values['scheduled_date'] = False
+                values['scheduled_date'] = parsed_datetime.replace(tzinfo=None) if parsed_datetime else False
+            else:
+                values['scheduled_date'] = False  # void string crashes
         new_mails = super(MailMail, self).create(values_list)
 
         new_mails_w_attach = self
@@ -150,10 +149,7 @@ class MailMail(models.Model):
     def write(self, vals):
         if vals.get('scheduled_date'):
             parsed_datetime = self._parse_scheduled_datetime(vals['scheduled_date'])
-            if parsed_datetime:
-                vals['scheduled_date'] = parsed_datetime.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT)
-            else:
-                vals['scheduled_date'] = False
+            vals['scheduled_date'] = parsed_datetime.replace(tzinfo=None) if parsed_datetime else False
         res = super(MailMail, self).write(vals)
         if vals.get('attachment_ids'):
             for mail in self:
diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py
index 0040412c4e2..e149c1271f3 100644
--- a/addons/mail/models/mail_template.py
+++ b/addons/mail/models/mail_template.py
@@ -4,7 +4,6 @@
 import base64
 import logging
 
-
 from odoo import _, api, fields, models, tools, Command
 from odoo.exceptions import UserError
 from odoo.tools import is_html_empty
diff --git a/addons/test_mail/tests/test_mail_mail.py b/addons/test_mail/tests/test_mail_mail.py
index 646cbf86945..76d9ca4fcd2 100644
--- a/addons/test_mail/tests/test_mail_mail.py
+++ b/addons/test_mail/tests/test_mail_mail.py
@@ -231,7 +231,7 @@ class TestMailMail(TestMailCommon):
             (now + timedelta(hours=-3)).strftime("%H:%M:%S %d-%m-%Y") + " -0400",
         ]
         expected_datetimes = [
-            False, '', False,
+            False, False, False,
             now, now - pytz.timezone('Australia/Brisbane').utcoffset(now),
             now - timedelta(days=1), now + timedelta(days=1), now + timedelta(days=1),
             now + timedelta(hours=-1),
@@ -253,9 +253,8 @@ class TestMailMail(TestMailCommon):
         ])
 
         for mail, expected_datetime, scheduled_datetime in zip(mails, expected_datetimes, scheduled_datetimes):
-            expected = expected_datetime.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT) if expected_datetime else expected_datetime
-            self.assertEqual(mail.scheduled_date, expected,
-                             'Scheduled date: %s should be stored as %s, received %s' % (scheduled_datetime, expected, mail.scheduled_date))
+            self.assertEqual(mail.scheduled_date, expected_datetime,
+                             'Scheduled date: %s should be stored as %s, received %s' % (scheduled_datetime, expected_datetime, mail.scheduled_date))
             self.assertEqual(mail.state, 'outgoing')
 
         with freeze_time(now):

From eb75a77646d073e184d6eb101b3bf84dd3967f95 Mon Sep 17 00:00:00 2001
From: std-odoo <std@odoo.com>
Date: Thu, 30 Jun 2022 07:54:07 +0000
Subject: [PATCH 2/7] [REF] mail: update the message content in the mail thread
 model
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Purpose
=======

Move the code which updates the mail message from the message model to mail
thread. Most other thread methods (like `_message_update_content_after_hook`)
are defined at record model level. It makes sense to delegate the update to
documents and not to the message. Message is a low-level technical object
that should not really hold business code.

While modifying this code, an update is done in the update content. We now
also allow to update the body without removing all attachments.

Finally tests are added as this feature was added without really testing
model code.

Task-2207626 (Rating: Delay rating notification to ease feedback)

Part-of: odoo/odoo#95623
Co-authored-by: Thibault Delavallée <tde@odoo.com>
---
 addons/im_livechat/models/mail_channel.py     |  2 +-
 addons/mail/controllers/discuss.py            |  8 +-
 addons/mail/models/mail_message.py            | 19 ----
 addons/mail/models/mail_thread.py             | 60 +++++++++++--
 addons/test_mail/tests/test_mail_message.py   |  8 +-
 .../tests/test_mail_thread_internals.py       | 88 +++++++++++++++++++
 6 files changed, 155 insertions(+), 30 deletions(-)

diff --git a/addons/im_livechat/models/mail_channel.py b/addons/im_livechat/models/mail_channel.py
index 26d3e4c70e5..79bb445f835 100644
--- a/addons/im_livechat/models/mail_channel.py
+++ b/addons/im_livechat/models/mail_channel.py
@@ -148,7 +148,7 @@ class MailChannel(models.Model):
                 'id': message.id,
                 'body': message.body,
             })
-        super()._message_update_content_after_hook(message=message)
+        return super()._message_update_content_after_hook(message=message)
 
     def _get_visitor_leave_message(self, operator=False, cancel=False):
         return _('Visitor has left the conversation.')
diff --git a/addons/mail/controllers/discuss.py b/addons/mail/controllers/discuss.py
index 7a748fea508..ebeacc8a6a0 100644
--- a/addons/mail/controllers/discuss.py
+++ b/addons/mail/controllers/discuss.py
@@ -241,7 +241,13 @@ class DiscussController(http.Controller):
         message_sudo = guest.env['mail.message'].browse(message_id).sudo().exists()
         if not message_sudo.is_current_user_or_guest_author and not guest.env.user._is_admin():
             raise NotFound()
-        message_sudo._update_content(body=body, attachment_ids=attachment_ids)
+        if not message_sudo.model or not message_sudo.res_id:
+            raise NotFound()
+        request.env[message_sudo.model].browse([message_sudo.res_id])._message_update_content(
+            message_sudo,
+            body,
+            attachment_ids=attachment_ids
+        )
         return {
             'id': message_sudo.id,
             'body': message_sudo.body,
diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py
index 5f86648fcf7..a8bb8e81e6c 100644
--- a/addons/mail/models/mail_message.py
+++ b/addons/mail/models/mail_message.py
@@ -808,25 +808,6 @@ class Message(models.Model):
         reaction.unlink()
         self.env[self.model].browse(self.res_id)._message_remove_reaction_after_hook(message=self, content=content)
 
-    def _update_content(self, body, attachment_ids):
-        self.ensure_one()
-        thread = self.env[self.model].browse(self.res_id)
-        thread._check_can_update_message_content(self)
-        self.body = body
-        if not attachment_ids:
-            self.attachment_ids._delete_and_notify()
-        else:
-            message_values = {
-                'model': self.model,
-                'body': body,
-                'res_id': self.res_id,
-            }
-            attachement_values = thread._message_post_process_attachments([], attachment_ids, message_values)
-            self.update(attachement_values)
-        # Cleanup related message data if the message is empty
-        self.sudo()._filter_empty()._cleanup_side_records()
-        thread._message_update_content_after_hook(self)
-
     # ------------------------------------------------------
     # MESSAGE READ / FETCH / FAILURE API
     # ------------------------------------------------------
diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index 661742115a2..c62648d5255 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -433,14 +433,20 @@ class MailThread(models.AbstractModel):
             return self.with_context(lang=self.env.user.lang)
         return self
 
-    def _check_can_update_message_content(self, message):
-        """" Checks that the current user can update the content of the message. """
+    def _check_can_update_message_content(self, messages):
+        """" Checks that the current user can update the content of the message.
+        Current heuristic is
+
+          * limited to note;
+          * if no tracking;
+          * only for user generated content;
+        """
         note_id = self.env['ir.model.data']._xmlid_to_res_id('mail.mt_note')
-        if not message.subtype_id.id == note_id:
+        if any(message.subtype_id.id != note_id for message in messages):
             raise exceptions.UserError(_("Only logged notes can have their content updated on model '%s'", self._name))
-        if message.tracking_value_ids:
+        if messages.tracking_value_ids:
             raise exceptions.UserError(_("Messages with tracking values cannot be modified"))
-        if not message.message_type == 'comment':
+        if any(message.message_type != 'comment' for message in messages):
             raise exceptions.UserError(_("Only messages type comment can have their content updated"))
 
     # ------------------------------------------------------
@@ -3122,8 +3128,52 @@ class MailThread(models.AbstractModel):
         msg_not_comment.write(msg_vals)
         return True
 
+    def _message_update_content(self, message, body, attachment_ids=None,
+                                strict=True):
+        """ Update message content. Currently does not support attachments
+        specific code (see ``_message_post_process_attachments``), to be added
+        when necessary.
+
+        Private method to use for tooling, do not expose to interface as editing
+        messages should be avoided at all costs (think of: notifications already
+        sent, ...).
+
+        :param <mail.message> message: message to update, should be linked to self through
+          model and res_id;
+        :param str body: new body (None to skip its update);
+        :param list attachment_ids: list of new attachments IDs, replacing old one (None
+          to skip its update);
+        :param bool strict: whether to check for allowance before updating
+          content. This should be skipped only when really necessary as it
+          creates issues with already-sent notifications, lack of content
+          tracking, ...
+        """
+        self.ensure_one()
+        if strict:
+            self._check_can_update_message_content(message.sudo())
+
+        msg_values = {'body': body} if body is not None else {}
+        if attachment_ids:
+            msg_values.update(
+                self._message_post_process_attachments([], attachment_ids, {
+                    'body': body,
+                    'model': self._name,
+                    'res_id': self.id,
+                })
+            )
+        elif attachment_ids is not None:  # None means "no update"
+            message.attachment_ids._delete_and_notify()
+        if msg_values:
+            message.write(msg_values)
+
+        # cleanup related message data if the message is empty
+        message.sudo()._filter_empty()._cleanup_side_records()
+
+        return self._message_update_content_after_hook(message)
+
     def _message_update_content_after_hook(self, message):
         """ Hook to add custom behavior after having updated the message content. """
+        return True
 
     # ------------------------------------------------------
     # CONTROLLERS
diff --git a/addons/test_mail/tests/test_mail_message.py b/addons/test_mail/tests/test_mail_message.py
index d43489c58f6..5de0cdc9de7 100644
--- a/addons/test_mail/tests/test_mail_message.py
+++ b/addons/test_mail/tests/test_mail_message.py
@@ -65,13 +65,13 @@ class TestMessageValues(TestMailCommon):
         self.assertFalse(message.sudo().tracking_value_ids)
 
         # Reset body case
-        message._update_content('<p><br /></p>', attachment_ids=message.attachment_ids.ids)
+        record._message_update_content(message, '<p><br /></p>', attachment_ids=message.attachment_ids.ids)
         self.assertTrue(is_html_empty(message.body))
         self.assertFalse(message.sudo()._filter_empty(), 'Still having attachments')
 
         # Subtype content
         note_subtype.write({'description': 'Very important discussions'})
-        message._update_content('', None)
+        record._message_update_content(message, '', [])
         self.assertFalse(message.attachment_ids)
         self.assertEqual(message.notified_partner_ids, self.partner_admin)
         self.assertEqual(message.starred_partner_ids, self.partner_admin)
@@ -80,7 +80,7 @@ class TestMessageValues(TestMailCommon):
         # Completely void now
         note_subtype.write({'description': ''})
         self.assertEqual(message.sudo()._filter_empty(), message)
-        message._update_content('', None)
+        record._message_update_content(message, '', [])
         self.assertFalse(message.notified_partner_ids)
         self.assertFalse(message.starred_partner_ids)
 
@@ -93,7 +93,7 @@ class TestMessageValues(TestMailCommon):
         self.assertFalse(tracking_message.subtype_id.description)
         self.assertFalse(tracking_message.sudo()._filter_empty(), 'Has tracking values')
         with self.assertRaises(UserError, msg='Tracking values prevent from updating content'):
-            tracking_message._update_content('', None)
+            record._message_update_content(tracking_message, '', [])
 
     @mute_logger('odoo.models.unlink')
     def test_mail_message_format(self):
diff --git a/addons/test_mail/tests/test_mail_thread_internals.py b/addons/test_mail/tests/test_mail_thread_internals.py
index 7ec34dcc778..0a0cb046dad 100644
--- a/addons/test_mail/tests/test_mail_thread_internals.py
+++ b/addons/test_mail/tests/test_mail_thread_internals.py
@@ -12,6 +12,94 @@ from odoo.tests.common import tagged, HttpCase, users
 from odoo.tools import mute_logger
 
 
+@tagged('mail_thread')
+class TestAPI(TestMailCommon, TestRecipients):
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestAPI, cls).setUpClass()
+        cls.ticket_record = cls.env['mail.test.ticket'].with_context(cls._test_context).create({
+            'email_from': '"Paulette Vachette" <paulette@test.example.com>',
+            'name': 'Test',
+            'user_id': cls.user_employee.id,
+        })
+
+    @mute_logger('openerp.addons.mail.models.mail_mail')
+    @users('employee')
+    def test_message_update_content(self):
+        """ Test updating message content. """
+        ticket_record = self.ticket_record.with_env(self.env)
+        attachments = self.env['ir.attachment'].create(
+            self._generate_attachments_data(2, 'mail.compose.message', 0)
+        )
+
+        # post a note
+        message = ticket_record.message_post(
+            attachment_ids=attachments.ids,
+            body="<p>Initial Body</p>",
+            message_type="comment",
+            partner_ids=self.partner_1.ids,
+        )
+        self.assertEqual(message.attachment_ids, attachments)
+        self.assertEqual(set(message.mapped('attachment_ids.res_id')), set(ticket_record.ids))
+        self.assertEqual(set(message.mapped('attachment_ids.res_model')), set([ticket_record._name]))
+        self.assertEqual(message.body, "<p>Initial Body</p>")
+        self.assertEqual(message.subtype_id, self.env.ref('mail.mt_note'))
+
+        # update the content with new attachments
+        new_attachments = self.env['ir.attachment'].create(
+            self._generate_attachments_data(2, 'mail.compose.message', 0)
+        )
+        ticket_record._message_update_content(
+            message, "<p>New Body</p>",
+            attachment_ids=new_attachments.ids
+        )
+        self.assertEqual(message.attachment_ids, attachments + new_attachments)
+        self.assertEqual(set(message.mapped('attachment_ids.res_id')), set(ticket_record.ids))
+        self.assertEqual(set(message.mapped('attachment_ids.res_model')), set([ticket_record._name]))
+        self.assertEqual(message.body, "<p>New Body</p>")
+
+        # void attachments
+        ticket_record._message_update_content(
+            message, "<p>Another Body, void attachments</p>",
+            attachment_ids=[]
+        )
+        self.assertFalse(message.attachment_ids)
+        self.assertFalse((attachments + new_attachments).exists())
+        self.assertEqual(message.body, "<p>Another Body, void attachments</p>")
+
+    @mute_logger('openerp.addons.mail.models.mail_mail')
+    @users('employee')
+    def test_message_update_content_check(self):
+        """ Test cases where updating content should be prevented """
+        ticket_record = self.ticket_record.with_env(self.env)
+
+        # cannot edit user comments (subtype)
+        message = ticket_record.message_post(
+            body="<p>Initial Body</p>",
+            message_type="comment",
+            subtype_id=self.env.ref('mail.mt_comment').id,
+        )
+        with self.assertRaises(exceptions.UserError):
+            ticket_record._message_update_content(
+                message, "<p>New Body</p>"
+            )
+
+        message.sudo().write({'subtype_id': self.env.ref('mail.mt_note')})
+        ticket_record._message_update_content(
+            message, "<p>New Body</p>"
+        )
+
+        # cannot edit notifications
+        for message_type in ['notification', 'user_notification', 'email']:
+            message.sudo().write({'message_type': message_type})
+            with self.assertRaises(exceptions.UserError):
+                ticket_record._message_update_content(
+                    message, "<p>New Body</p>"
+                )
+
+
+@tagged('mail_thread')
 class TestChatterTweaks(TestMailCommon, TestRecipients):
 
     @classmethod

From aed52f3e0aa7c09199478d4087c9610e4500554a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Debauche?= <std@odoo.com>
Date: Mon, 11 Jul 2022 07:15:31 +0000
Subject: [PATCH 3/7] [IMP] mail: correctly support scheduled_date from
 template when sending emails

Purpose of this commit is to correctly generate and support scheduled date
defined on template when using ``send_mail`` tool that send emails directly
from a MailTemplate.

It uses the parsing tool method defined on mail.mail in order to have a
datetime localized in UTC then set timezone agnostic as expected by the
ORM.

Task-2826699 (Mail: use datetime for scheduled_date mail field instead of char)

Part-of: odoo/odoo#95623
---
 addons/mail/models/mail_template.py           |  6 +++
 addons/test_mail/tests/test_mail_template.py  | 39 +++++++++++++++++--
 .../tests/test_mail_template_preview.py       |  6 +--
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py
index e149c1271f3..e647dfcf122 100644
--- a/addons/mail/models/mail_template.py
+++ b/addons/mail/models/mail_template.py
@@ -204,6 +204,12 @@ class MailTemplate(models.Model):
                 values = results[res_id]
                 if values.get('body_html'):
                     values['body'] = tools.html_sanitize(values['body_html'])
+                # if asked in fields to return, parse generated date into tz agnostic UTC as expected by ORM
+                scheduled_date = values.pop('scheduled_date', None)
+                if 'scheduled_date' in fields and scheduled_date:
+                    parsed_datetime = self.env['mail.mail']._parse_scheduled_datetime(scheduled_date)
+                    values['scheduled_date'] = parsed_datetime.replace(tzinfo=None) if parsed_datetime else False
+
                 # technical settings
                 values.update(
                     mail_server_id=template.mail_server_id.id or False,
diff --git a/addons/test_mail/tests/test_mail_template.py b/addons/test_mail/tests/test_mail_template.py
index d9614c7b7f1..1518222fe3f 100644
--- a/addons/test_mail/tests/test_mail_template.py
+++ b/addons/test_mail/tests/test_mail_template.py
@@ -2,18 +2,21 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 import base64
+import datetime
+
+from freezegun import freeze_time
 
 from odoo.addons.test_mail.tests.common import TestMailCommon, TestRecipients
-from odoo.tests import tagged
+from odoo.tests import tagged, users
 from odoo.tools import mute_logger
 
 
 @tagged('mail_template', 'multi_lang')
-class TestMailTemplate(TestMailCommon, TestRecipients):
+class TestMailTemplateCommon(TestMailCommon, TestRecipients):
 
     @classmethod
     def setUpClass(cls):
-        super(TestMailTemplate, cls).setUpClass()
+        super(TestMailTemplateCommon, cls).setUpClass()
         cls.test_record = cls.env['mail.test.lang'].with_context(cls._test_context).create({
             'email_from': 'ignasse@example.com',
             'name': 'Test',
@@ -61,6 +64,36 @@ class TestMailTemplate(TestMailCommon, TestRecipients):
         # Force the attachments of the template to be in the natural order.
         cls.test_template.invalidate_recordset(['attachment_ids'])
 
+
+@tagged('mail_template', 'multi_lang')
+class TestMailTemplate(TestMailTemplateCommon):
+
+    @mute_logger('odoo.addons.mail.models.mail_mail')
+    @users('employee')
+    def test_template_schedule_email(self):
+        """ Test scheduling email sending from template. """
+        now = datetime.datetime.now()
+        test_template = self.test_template.with_env(self.env)
+
+        # schedule the mail in 3 days
+        test_template.scheduled_date = '{{datetime.datetime.now() + datetime.timedelta(days=3)}}'
+        with freeze_time(now):
+            mail_id = test_template.send_mail(self.test_record.id)
+        mail = self.env['mail.mail'].sudo().browse(mail_id)
+        self.assertEqual(
+            mail.scheduled_date.replace(second=0, microsecond=0),
+            (now + datetime.timedelta(days=3)).replace(second=0, microsecond=0),
+        )
+        self.assertEqual(mail.state, 'outgoing')
+
+        # check a wrong format
+        test_template.scheduled_date = '{{"test " * 5}}'
+        with freeze_time(now):
+            mail_id = test_template.send_mail(self.test_record.id)
+        mail = self.env['mail.mail'].sudo().browse(mail_id)
+        self.assertFalse(mail.scheduled_date)
+        self.assertEqual(mail.state, 'outgoing')
+
     @mute_logger('odoo.addons.mail.models.mail_mail')
     def test_template_send_email(self):
         mail_id = self.test_template.send_mail(self.test_record.id)
diff --git a/addons/test_mail/tests/test_mail_template_preview.py b/addons/test_mail/tests/test_mail_template_preview.py
index 87becbaaeb1..2442e46a473 100644
--- a/addons/test_mail/tests/test_mail_template_preview.py
+++ b/addons/test_mail/tests/test_mail_template_preview.py
@@ -1,12 +1,12 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from odoo.addons.test_mail.tests.test_mail_template import TestMailTemplate
+from odoo.addons.test_mail.tests.test_mail_template import TestMailTemplateCommon
 from odoo.tests import tagged
 
 
-@tagged('mail_template')
-class TestMailTemplateTools(TestMailTemplate):
+@tagged('mail_template', 'multi_lang')
+class TestMailTemplateTools(TestMailTemplateCommon):
 
     def test_mail_template_preview_force_lang(self):
         test_record = self.env['mail.test.lang'].browse(self.test_record.ids)

From 9b11a9d82b52b4750f11c56a7b6be3af1fca269d Mon Sep 17 00:00:00 2001
From: std-odoo <std@odoo.com>
Date: Tue, 28 Jun 2022 12:02:07 +0000
Subject: [PATCH 4/7] [IMP] mail: schedule emails and notifications
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Purpose
=======

Currently, only email sending can be scheduled with the `scheduled_date` field
defined on on <mail.mail>. It's not possible to delay the sending of
notifications.

We want to be able to delay the sending of the emails, but also the inbox
and bus bus notifications.

Technical
=========

For that purpose, we created a new model which stores the message we need to
notify with the scheduled date. When a scheduled_datetime is given we skip the
notification process. Instead an entry in that new model is created. A cron
regularly polls the scheduled message and launch the notification process on
messages that are ready to be sent.

Task-2207626 (Rating: Delay rating notification to ease feedback)

Part-of: odoo/odoo#95623
Co-authored-by: Thibault Delavallée <tde@odoo.com>
---
 .../hr_recruitment/models/hr_recruitment.py   |   4 +-
 addons/mail/__manifest__.py                   |   1 +
 addons/mail/data/ir_cron_data.xml             |  11 ++
 addons/mail/models/__init__.py                |   1 +
 addons/mail/models/mail_channel.py            |   3 +-
 addons/mail/models/mail_message_schedule.py   | 135 ++++++++++++++++++
 addons/mail/models/mail_thread.py             |  62 ++++++--
 addons/mail/security/ir.model.access.csv      |   2 +
 addons/mail/tests/common.py                   |  11 ++
 addons/mail/views/mail_menus.xml              |   5 +
 .../views/mail_message_schedule_views.xml     |  52 +++++++
 addons/mail/wizard/mail_resend_message.py     |   2 +-
 addons/sms/models/mail_thread.py              |   6 +-
 addons/sms/wizard/sms_resend.py               |   2 +-
 addons/test_mail/tests/test_message_post.py   | 133 ++++++++++++++++-
 addons/test_mail_sms/tests/test_sms_post.py   |   4 +-
 16 files changed, 410 insertions(+), 24 deletions(-)
 create mode 100644 addons/mail/models/mail_message_schedule.py
 create mode 100644 addons/mail/views/mail_message_schedule_views.xml

diff --git a/addons/hr_recruitment/models/hr_recruitment.py b/addons/hr_recruitment/models/hr_recruitment.py
index 0a033fa8517..b08d43051be 100644
--- a/addons/hr_recruitment/models/hr_recruitment.py
+++ b/addons/hr_recruitment/models/hr_recruitment.py
@@ -445,12 +445,12 @@ class Applicant(models.Model):
             view_id = self.env.ref('hr_recruitment.hr_applicant_view_form_interviewer').id
         return super().get_view(view_id, view_type, **options)
 
-    def _notify_get_recipients(self, message, msg_vals):
+    def _notify_get_recipients(self, message, msg_vals, **kwargs):
         """
             Do not notify members of the Recruitment Interviewer group, as this
             might leak some data they shouldn't have access to.
         """
-        recipients = super()._notify_get_recipients(message, msg_vals)
+        recipients = super()._notify_get_recipients(message, msg_vals, **kwargs)
         interviewer_group = self.env.ref('hr_recruitment.group_hr_recruitment_interviewer').id
         return [recipient for recipient in recipients if interviewer_group not in recipient['groups']]
 
diff --git a/addons/mail/__manifest__.py b/addons/mail/__manifest__.py
index 002c3707485..9f496b8abcc 100644
--- a/addons/mail/__manifest__.py
+++ b/addons/mail/__manifest__.py
@@ -73,6 +73,7 @@ For more specific needs, you may also assign custom-defined actions
         'views/mail_tracking_views.xml',
         'views/mail_notification_views.xml',
         'views/mail_message_views.xml',
+        'views/mail_message_schedule_views.xml',
         'views/mail_mail_views.xml',
         'views/mail_followers_views.xml',
         'views/mail_ice_server_views.xml',
diff --git a/addons/mail/data/ir_cron_data.xml b/addons/mail/data/ir_cron_data.xml
index 8362160d762..5bc687622f1 100644
--- a/addons/mail/data/ir_cron_data.xml
+++ b/addons/mail/data/ir_cron_data.xml
@@ -54,5 +54,16 @@
             <!-- Active flag is set on fetchmail_server.create/write -->
             <field name="active" eval="False"/>
         </record>
+
+        <record id="ir_cron_send_scheduled_message" model="ir.cron">
+            <field name="name">Notification: Send scheduled message notifications</field>
+            <field name="interval_number">1</field>
+            <field name="interval_type">hours</field>
+            <field name="numbercall">-1</field>
+            <field name="doall" eval="False"/>
+            <field name="model_id" ref="model_mail_message_schedule"/>
+            <field name="code">model._send_notifications_cron()</field>
+            <field name="state">code</field>
+        </record>
     </data>
 </odoo>
diff --git a/addons/mail/models/__init__.py b/addons/mail/models/__init__.py
index f1a94f443e4..30f6753923b 100644
--- a/addons/mail/models/__init__.py
+++ b/addons/mail/models/__init__.py
@@ -24,6 +24,7 @@ from . import mail_blacklist
 from . import mail_followers
 from . import mail_gateway_allowed
 from . import mail_message_reaction
+from . import mail_message_schedule
 from . import mail_message_subtype
 from . import mail_message
 from . import mail_mail
diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py
index c8a4891a3df..5b49132a8a2 100644
--- a/addons/mail/models/mail_channel.py
+++ b/addons/mail/models/mail_channel.py
@@ -498,13 +498,14 @@ class Channel(models.Model):
     # MAILING
     # ------------------------------------------------------------
 
-    def _notify_get_recipients(self, message, msg_vals):
+    def _notify_get_recipients(self, message, msg_vals, **kwargs):
         """ Override recipients computation as channel is not a standard
         mail.thread document. Indeed there are no followers on a channel.
         Instead of followers it has members that should be notified.
 
         :param message: see ``MailThread._notify_get_recipients()``;
         :param msg_vals: see ``MailThread._notify_get_recipients()``;
+        :param kwargs: see ``MailThread._notify_get_recipients()``;
 
         :return recipients: structured data holding recipients data. See
           ``MailThread._notify_thread()`` for more details about its content
diff --git a/addons/mail/models/mail_message_schedule.py b/addons/mail/models/mail_message_schedule.py
new file mode 100644
index 00000000000..e8fa207404b
--- /dev/null
+++ b/addons/mail/models/mail_message_schedule.py
@@ -0,0 +1,135 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+import json
+import logging
+
+from datetime import datetime
+
+from odoo import api, fields, models
+
+_logger = logging.getLogger(__name__)
+
+
+class MailMessageSchedule(models.Model):
+    """ Mail message notification schedule queue.
+
+    This model is used to store the mail messages scheduled. So we can
+    delay the sending of the notifications. A scheduled date field already
+    exists on the <mail.mail> but it does not allow us to delay the sending
+    of the <bus.bus> notifications.
+    """
+    _name = 'mail.message.schedule'
+    _description = 'Scheduled Messages'
+    _order = 'scheduled_datetime DESC, id DESC'
+    _rec_name = 'mail_message_id'
+
+    mail_message_id = fields.Many2one(
+        'mail.message', string='Message',
+        ondelete='cascade', required=True)
+    notification_parameters = fields.Text('Notification Parameter')
+    scheduled_datetime = fields.Datetime(
+        'Scheduled Send Date', required=True,
+        help='Datetime at which notification should be sent.')
+
+    @api.model_create_multi
+    def create(self, vals_list):
+        schedules = super().create(vals_list)
+        if schedules:
+            self.env.ref('mail.ir_cron_send_scheduled_message')._trigger_list(
+                set(schedules.mapped('scheduled_datetime'))
+            )
+        return schedules
+
+    @api.model
+    def _send_notifications_cron(self):
+        messages_scheduled = self.env['mail.message.schedule'].search(
+            [('scheduled_datetime', '<=', datetime.utcnow())]
+        )
+        if messages_scheduled:
+            _logger.info('Send %s scheduled messages', len(messages_scheduled))
+            messages_scheduled._send_notifications()
+
+    def force_send(self):
+        """ Launch notification process independently from the expected date. """
+        return self._send_notifications()
+
+    def _send_notifications(self, default_notify_kwargs=None):
+        """ Send notification for scheduled messages.
+
+        :param dict default_notify_kwargs: optional parameters to propagate to
+          ``notify_thread``. Those are default values overridden by content of
+          ``notification_parameters`` field.
+        """
+        for model, schedules in self._group_by_model().items():
+            if model:
+                records = self.env[model].browse(schedules.mapped('mail_message_id.res_id'))
+            else:
+                records = [self.env['mail.thread']] * len(schedules)
+
+            for record, schedule in zip(records, schedules):
+                notify_kwargs = dict(default_notify_kwargs or {}, skip_existing=True)
+                try:
+                    schedule_notify_kwargs = json.loads(schedule.notification_parameters)
+                except Exception:
+                    pass
+                else:
+                    schedule_notify_kwargs.pop('scheduled_date', None)
+                    notify_kwargs.update(schedule_notify_kwargs)
+
+                record._notify_thread(schedule.mail_message_id, msg_vals=False, **notify_kwargs)
+
+        self.unlink()
+        return True
+
+    @api.model
+    def _send_message_notifications(self, messages, default_notify_kwargs=None):
+        """ Send scheduled notification for given messages.
+
+        :param <mail.message> messages: scheduled sending related to those messages
+          will be sent now;
+        :param dict default_notify_kwargs: optional parameters to propagate to
+          ``notify_thread``. Those are default values overridden by content of
+          ``notification_parameters`` field.
+
+        :return bool: False if no schedule has been found, True otherwise
+        """
+        messages_scheduled = self.search(
+            [('mail_message_id', 'in', messages.ids)]
+        )
+        if not messages_scheduled:
+            return False
+
+        messages_scheduled._send_notifications(default_notify_kwargs=default_notify_kwargs)
+        return True
+
+    @api.model
+    def _update_message_scheduled_datetime(self, messages, new_datetime):
+        """ Update scheduled datetime for scheduled sending related to messages.
+
+        :param <mail.message> messages: scheduled sending related to those messages
+          will be updated. Missing one are skipped;
+        :param datetime new_datetime: new datetime for sending. New triggers
+          are created based on it;
+
+        :return bool: False if no schedule has been found, True otherwise
+        """
+        messages_scheduled = self.search(
+            [('mail_message_id', 'in', messages.ids)]
+        )
+        if not messages_scheduled:
+            return False
+
+        messages_scheduled.scheduled_datetime = new_datetime
+        self.env.ref('mail.ir_cron_send_scheduled_message')._trigger(new_datetime)
+        return True
+
+    def _group_by_model(self):
+        grouped = {}
+        for schedule in self:
+            model = schedule.mail_message_id.model if schedule.mail_message_id.model and schedule.mail_message_id.res_id else False
+            if model not in grouped:
+                grouped[model] = schedule
+            else:
+                grouped[model] += schedule
+        return grouped
diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index c62648d5255..c6b1ffab8ee 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -9,6 +9,7 @@ import email
 import email.policy
 import hashlib
 import hmac
+import json
 import lxml
 import logging
 import pytz
@@ -18,7 +19,7 @@ import threading
 
 from collections import namedtuple
 from email.message import EmailMessage
-from email import message_from_string, policy
+from email import message_from_string
 from lxml import etree
 from werkzeug import urls
 from xmlrpc import client as xmlrpclib
@@ -1423,7 +1424,7 @@ class MailThread(models.AbstractModel):
         if email_part:
             if email_part.get_content_type() == 'text/rfc822-headers':
                 # Convert the message body into a message itself
-                email_payload = message_from_string(email_part.get_payload(), policy=policy.SMTP)
+                email_payload = message_from_string(email_part.get_payload(), policy=email.policy.SMTP)
             else:
                 email_payload = email_part.get_payload()[0]
             bounced_msg_id = tools.mail_header_msgid_re.findall(tools.decode_message_header(email_payload, 'Message-Id'))
@@ -2304,7 +2305,11 @@ class MailThread(models.AbstractModel):
           access message content in db;
 
         Kwargs allow to pass various parameters that are given to sub notification
-        methods. See those methods for more details about the additional parameters.
+        methods. See those methods for more details about supported parameters.
+        Specific kwargs used in this method:
+
+          * ``scheduled_date``: delay notification sending if set in the future.
+            This is done using the ``mail.message.schedule`` intermediate model;
 
         :return: recipients data (see ``MailThread._notify_get_recipients()``)
         """
@@ -2312,12 +2317,27 @@ class MailThread(models.AbstractModel):
         self = self._fallback_lang()
 
         msg_vals = msg_vals if msg_vals else {}
-        recipients_data = self._notify_get_recipients(message, msg_vals)
+        recipients_data = self._notify_get_recipients(message, msg_vals, **kwargs)
         if not recipients_data:
             return recipients_data
 
-        self._notify_thread_by_inbox(message, recipients_data, msg_vals=msg_vals, **kwargs)
-        self._notify_thread_by_email(message, recipients_data, msg_vals=msg_vals, **kwargs)
+        # if scheduled for later: add in queue instead of generating notifications
+        scheduled_date = 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,
+                'mail_message_id': message.id,
+                'notification_parameters': json.dumps(kwargs),
+            })
+        else:
+            # generate immediately the <mail.notification>
+            # and send the <mail.mail> and the <bus.bus> notifications
+            self._notify_thread_by_inbox(message, recipients_data, msg_vals=msg_vals, **kwargs)
+            self._notify_thread_by_email(message, recipients_data, msg_vals=msg_vals, **kwargs)
 
         return recipients_data
 
@@ -2363,7 +2383,7 @@ class MailThread(models.AbstractModel):
     def _notify_thread_by_email(self, message, recipients_data, msg_vals=False,
                                 mail_auto_delete=True, # mail.mail
                                 model_description=False, force_email_company=False, force_email_lang=False,  # rendering
-                                check_existing=False, force_send=True, send_after_commit=True,  # email send
+                                resend_existing=False, force_send=True, send_after_commit=True,  # email send
                                 **kwargs):
         """ Method to send email linked to notified messages.
 
@@ -2390,7 +2410,7 @@ class MailThread(models.AbstractModel):
         :param force_email_company: see ``_notify_by_email_prepare_rendering_context``;
         :param force_email_lang: see ``_notify_by_email_prepare_rendering_context``;
 
-        :param check_existing: check for existing notifications to update based on
+        :param resend_existing: check for existing notifications to update based on
           mailed recipient, otherwise create new notifications;
         :param force_send: send emails directly instead of using queue;
         :param send_after_commit: if force_send, tells whether to send emails after
@@ -2455,7 +2475,7 @@ class MailThread(models.AbstractModel):
 
                 if new_email and recipients_ids_chunk:
                     tocreate_recipient_ids = list(recipients_ids_chunk)
-                    if check_existing:
+                    if resend_existing:
                         existing_notifications = self.env['mail.notification'].sudo().search([
                             ('mail_message_id', '=', message.id),
                             ('notification_type', '=', 'email'),
@@ -2660,13 +2680,21 @@ class MailThread(models.AbstractModel):
             final_mail_values.update(additional_values)
         return final_mail_values
 
-    def _notify_get_recipients(self, message, msg_vals):
+    def _notify_get_recipients(self, message, msg_vals, **kwargs):
         """ Compute recipients to notify based on subtype and followers. This
         method returns data structured as expected for ``_notify_recipients``.
 
         TDE/XDO TODO: flag rdata directly, with for example r['notif'] = 'ocn_client' and r['needaction']=False
         and correctly override _notify_get_recipients
 
+        Kwargs allow to pass various parameters that are used by sub notification
+        methods. See those methods for more details about supported parameters.
+        Specific kwargs used in this method:
+
+          * ``skip_existing``: check existing notifications and skip them in order
+            to avoid having several notifications / partner as it would make
+            constraints crash. This is disabled by default to optimize speed;
+
         :return list recipients_data: this is a list of recipients information (see
           ``MailFollowers._get_recipient_data()`` for more details) formatted like
           [{'active': partner.active;
@@ -2696,6 +2724,20 @@ class MailThread(models.AbstractModel):
             if pdata['active'] is False:
                 continue
             recipients_data.append(pdata)
+
+        # avoid double notification (on demand due to additional queries)
+        if kwargs.pop('skip_existing', False):
+            pids = [r['id'] for r in recipients_data]
+            if pids:
+                existing_notifications = self.env['mail.notification'].sudo().search([
+                    ('res_partner_id', 'in', pids),
+                    ('mail_message_id', 'in', message.ids)
+                ])
+                recipients_data = [
+                    r for r in recipients_data
+                    if r['id'] not in existing_notifications.res_partner_id.ids
+                ]
+
         return recipients_data
 
     def _notify_get_recipients_groups(self, msg_vals=None):
diff --git a/addons/mail/security/ir.model.access.csv b/addons/mail/security/ir.model.access.csv
index 8b53ea2c560..5ee0c537886 100644
--- a/addons/mail/security/ir.model.access.csv
+++ b/addons/mail/security/ir.model.access.csv
@@ -3,6 +3,8 @@ access_fetchmail_server,fetchmail.server,model_fetchmail_server,base.group_syste
 access_mail_message_all,mail.message.all,model_mail_message,,1,0,0,0
 access_mail_message_portal,mail.message.portal,model_mail_message,base.group_portal,1,1,1,1
 access_mail_message_user,mail.message.user,model_mail_message,base.group_user,1,1,1,1
+access_mail_message_scheduled_all,mail.message.scheduled.all,model_mail_message_schedule,,0,0,0,0
+access_mail_message_scheduled_system,mail.message.scheduled.system,model_mail_message_schedule,base.group_system,1,1,1,1
 access_mail_mail_all,mail.mail.all,model_mail_mail,,0,0,0,0
 access_mail_mail_portal,mail.mail.portal,model_mail_mail,base.group_portal,0,0,0,0
 access_mail_mail_user,mail.mail.user,model_mail_mail,base.group_user,0,0,0,0
diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py
index 66dd23d8a80..004b2a959ae 100644
--- a/addons/mail/tests/common.py
+++ b/addons/mail/tests/common.py
@@ -721,6 +721,17 @@ class MailCase(MockEmail):
             found_bus_notifs = self.assertBusNotifications(channels, message_items=message_items)
             self.assertEqual(self._new_bus_notifs, found_bus_notifs)
 
+    @contextmanager
+    def assertMsgWithoutNotifications(self, mail_unlink_sent=False):
+        try:
+            with self.mock_mail_gateway(mail_unlink_sent=mail_unlink_sent), self.mock_bus(), self.mock_mail_app():
+                yield
+        finally:
+            self.assertTrue(self._new_msgs)
+            self.assertFalse(bool(self._new_notifs))
+            self.assertFalse(bool(self._new_mails))
+            self.assertFalse(bool(self._mails))
+
     @contextmanager
     def assertNoNotifications(self):
         try:
diff --git a/addons/mail/views/mail_menus.xml b/addons/mail/views/mail_menus.xml
index 9671148395c..4ea13471017 100644
--- a/addons/mail/views/mail_menus.xml
+++ b/addons/mail/views/mail_menus.xml
@@ -62,6 +62,11 @@
         parent="mail.mail_menu_technical"
         action="action_view_mail_message"
         sequence="1"/>
+    <menuitem name="Scheduled Messages"
+        id="mail_message_schedule_menu"
+        parent="mail.mail_menu_technical"
+        action="mail_message_schedule_action"
+        sequence="2"/>
     <menuitem name="Subtypes"
         id="menu_message_subtype"
         parent="mail.mail_menu_technical"
diff --git a/addons/mail/views/mail_message_schedule_views.xml b/addons/mail/views/mail_message_schedule_views.xml
new file mode 100644
index 00000000000..b458d66e097
--- /dev/null
+++ b/addons/mail/views/mail_message_schedule_views.xml
@@ -0,0 +1,52 @@
+<?xml version="1.0"?>
+<odoo><data>
+    <record id="mail_message_schedule_view_form" model="ir.ui.view">
+        <field name="name">mail.message.schedule.view.form</field>
+        <field name="model">mail.message.schedule</field>
+        <field name="arch" type="xml">
+            <form string="Scheduled Message" duplicate="0">
+                <header>
+                    <button name="force_send" string="Force Send" type="object"/>
+                </header>
+                <sheet>
+                    <group>
+                        <field name="mail_message_id"/>
+                        <field name="scheduled_datetime"/>
+                    </group>
+                    <group>
+                        <field name="notification_parameters"/>
+                    </group>
+                </sheet>
+            </form>
+        </field>
+    </record>
+
+    <record id="mail_message_schedule_view_tree" model="ir.ui.view">
+        <field name="name">mail.message.schedule.view.tree</field>
+        <field name="model">mail.message.schedule</field>
+        <field name="arch" type="xml">
+            <tree string="Emails">
+                <field name="mail_message_id"/>
+                <field name="scheduled_datetime"/>
+            </tree>
+        </field>
+    </record>
+
+    <record id="mail_message_schedule_view_search" model="ir.ui.view">
+        <field name="name">mail.message.schedule.view.search</field>
+        <field name="model">mail.message.schedule</field>
+        <field name="arch" type="xml">
+            <search string="Scheduled Messages">
+                <field name="mail_message_id"/>
+            </search>
+        </field>
+    </record>
+
+    <record id="mail_message_schedule_action" model="ir.actions.act_window">
+        <field name="name">Scheduled Messages</field>
+        <field name="res_model">mail.message.schedule</field>
+        <field name="view_mode">tree,form</field>
+        <field name="context">{}</field>
+    </record>
+
+</data></odoo>
diff --git a/addons/mail/wizard/mail_resend_message.py b/addons/mail/wizard/mail_resend_message.py
index d2b197048f5..49068a88783 100644
--- a/addons/mail/wizard/mail_resend_message.py
+++ b/addons/mail/wizard/mail_resend_message.py
@@ -75,7 +75,7 @@ class MailResendMessage(models.TransientModel):
 
                 record._notify_thread_by_email(
                     message, email_partners_data,
-                    check_existing=True,
+                    resend_existing=True,
                     send_after_commit=False
                 )
 
diff --git a/addons/sms/models/mail_thread.py b/addons/sms/models/mail_thread.py
index 136dbcd46b6..cd7a600952e 100644
--- a/addons/sms/models/mail_thread.py
+++ b/addons/sms/models/mail_thread.py
@@ -243,7 +243,7 @@ class MailThread(models.AbstractModel):
 
     def _notify_thread_by_sms(self, message, recipients_data, msg_vals=False,
                               sms_numbers=None, sms_pid_to_number=None,
-                              check_existing=False, put_in_queue=False, **kwargs):
+                              resend_existing=False, put_in_queue=False, **kwargs):
         """ Notification method: by SMS.
 
         :param message: ``mail.message`` record to notify;
@@ -266,7 +266,7 @@ class MailThread(models.AbstractModel):
           and classic recipients;
         :param pid_to_number: force a number to notify for a given partner ID
               instead of taking its mobile / phone number;
-        :param check_existing: check for existing notifications to update based on
+        :param resend_existing: check for existing notifications to update based on
           mailed recipient, otherwise create new notifications;
         :param put_in_queue: use cron to send queued SMS instead of sending them
           directly;
@@ -318,7 +318,7 @@ class MailThread(models.AbstractModel):
         if sms_create_vals:
             sms_all |= self.env['sms.sms'].sudo().create(sms_create_vals)
 
-            if check_existing:
+            if resend_existing:
                 existing = self.env['mail.notification'].sudo().search([
                     '|', ('res_partner_id', 'in', partner_ids),
                     '&', ('res_partner_id', '=', False), ('sms_number', 'in', sms_numbers),
diff --git a/addons/sms/wizard/sms_resend.py b/addons/sms/wizard/sms_resend.py
index 4a262eb933d..ff8f86deb99 100644
--- a/addons/sms/wizard/sms_resend.py
+++ b/addons/sms/wizard/sms_resend.py
@@ -99,7 +99,7 @@ class SMSResend(models.TransientModel):
                 record._notify_thread_by_sms(
                     self.mail_message_id, recipients_data,
                     sms_numbers=numbers, sms_pid_to_number=sms_pid_to_number,
-                    check_existing=True, put_in_queue=False
+                    resend_existing=True, put_in_queue=False
                 )
 
         self.mail_message_id._notify_message_notification_update()
diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py
index 2d069898df0..d3dbe223123 100644
--- a/addons/test_mail/tests/test_message_post.py
+++ b/addons/test_mail/tests/test_message_post.py
@@ -3,9 +3,13 @@
 
 import base64
 
+from datetime import datetime, timedelta
+from freezegun import freeze_time
+from markupsafe import escape
 from unittest.mock import patch
 
 from odoo import tools
+from odoo.addons.base.tests.test_ir_cron import CronMixinCase
 from odoo.addons.mail.tests.common import mail_new_test_user
 from odoo.addons.test_mail.data.test_mail_data import MAIL_TEMPLATE_PLAINTEXT
 from odoo.addons.test_mail.models.test_mail_models import MailTestSimple
@@ -16,8 +20,6 @@ from odoo.tests import tagged
 from odoo.tools import mute_logger, formataddr
 from odoo.tests.common import users
 
-from markupsafe import escape
-
 
 @tagged('mail_post')
 class TestMessagePostCommon(TestMailCommon, TestRecipients):
@@ -77,6 +79,7 @@ class TestMessagePostCommon(TestMailCommon, TestRecipients):
 @tagged('mail_post')
 class TestMailNotifyAPI(TestMessagePostCommon):
 
+    @mute_logger('odoo.models.unlink')
     @users('employee')
     def test_email_notifiction_layouts(self):
         self.user_employee.write({'notification_type': 'email'})
@@ -410,7 +413,7 @@ class TestMessageNotify(TestMessagePostCommon):
 
 
 @tagged('mail_post')
-class TestMessagePost(TestMessagePostCommon):
+class TestMessagePost(TestMessagePostCommon, CronMixinCase):
 
     def test_initial_values(self):
         self.assertFalse(self.test_record.message_ids)
@@ -540,8 +543,130 @@ class TestMessagePost(TestMessagePostCommon):
         # notifications emails should not have been deleted: one for customers, one for user
         self.assertEqual(self.env['mail.mail'].sudo().search_count([('mail_message_id', '=', msg.id)]), 2)
 
-    @mute_logger('odoo.addons.mail.models.mail_mail')
     @users('employee')
+    @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.addons.mail.models.mail_message_schedule', 'odoo.models.unlink')
+    def test_message_post_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)
+        self.user_admin.write({'notification_type': 'inbox'})
+
+        test_record = self.test_record.with_env(self.env)
+        test_record.message_subscribe((self.partner_1 | self.partner_admin).ids)
+
+        with freeze_time(now), \
+             self.assertMsgWithoutNotifications(), \
+             self.capture_triggers(cron_id) as capt:
+            msg = test_record.message_post(
+                body='<p>Test</p>',
+                message_type='comment',
+                subject='Subject',
+                subtype_xmlid='mail.mt_comment',
+                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_mails)
+        self.assertFalse(self._mails)
+
+        schedules = self.env['mail.message.schedule'].sudo().search([('mail_message_id', '=', msg.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 freeze_time(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):
+            self.env['mail.message.schedule'].sudo()._send_notifications_cron()
+        self.assertFalse(schedules.exists(), msg='Should have sent the message')
+        # check notifications have been sent
+        recipients_info = [{'content': '<p>Test</p>', 'notif': [
+            {'partner': self.partner_admin, 'type': 'inbox'},
+            {'partner': self.partner_1, 'type': 'email'},
+        ]}]
+        self.assertMailNotifications(msg, recipients_info)
+
+        # manually create a new schedule date, resend it -> should not crash (aka
+        # don't create duplicate notifications, ...)
+        self.env['mail.message.schedule'].sudo().create({
+            'mail_message_id': msg.id,
+            'scheduled_datetime': scheduled_datetime,
+        })
+
+        # Send the scheduled message from the CRON
+        with freeze_time(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), \
+             self.mock_mail_gateway(mail_unlink_sent=False), \
+             self.capture_triggers(cron_id) as capt:
+            msg = test_record.message_post(
+                body='<p>Test</p>',
+                message_type='comment',
+                subject='Subject',
+                subtype_xmlid='mail.mt_comment',
+                scheduled_date=now,
+            )
+        self.assertFalse(capt.records)
+        recipients_info = [{'content': '<p>Test</p>', 'notif': [
+            {'partner': self.partner_admin, 'type': 'inbox'},
+            {'partner': self.partner_1, 'type': 'email'},
+        ]}]
+        self.assertMailNotifications(msg, recipients_info)
+
+    @users('employee')
+    @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.addons.mail.models.mail_message_schedule', 'odoo.models.unlink')
+    def test_message_post_schedule_update(self):
+        """ Test tools to update scheduled notifications """
+        cron = self.env.ref('mail.ir_cron_send_scheduled_message')
+        now = datetime.utcnow().replace(second=0, microsecond=0)
+        scheduled_datetime = now + timedelta(days=5)
+        self.user_admin.write({'notification_type': 'inbox'})
+
+        test_record = self.test_record.with_env(self.env)
+        test_record.message_subscribe((self.partner_1 | self.partner_admin).ids)
+
+        with freeze_time(now), \
+             self.assertMsgWithoutNotifications():
+            msg = test_record.message_post(
+                body='<p>Test</p>',
+                message_type='comment',
+                subject='Subject',
+                subtype_xmlid='mail.mt_comment',
+                scheduled_date=scheduled_datetime,
+            )
+        schedules = self.env['mail.message.schedule'].sudo().search([('mail_message_id', '=', msg.id)])
+        self.assertEqual(len(schedules), 1, msg='Should have scheduled the message')
+
+        # update scheduled datetime, should create new triggers
+        with freeze_time(now), \
+             self.assertNoNotifications(), \
+             self.capture_triggers(cron.id) as capt:
+            self.env['mail.message.schedule'].sudo()._update_message_scheduled_datetime(msg, now - timedelta(hours=1))
+        self.assertEqual(capt.records.call_at, now - timedelta(hours=1),
+                         msg='Should have created a new cron trigger for the new scheduled sending')
+        self.assertTrue(schedules.exists(), msg='Should not have sent the message')
+
+        # run cron, notifications have been sent
+        with freeze_time(now), self.mock_mail_gateway(mail_unlink_sent=False):
+            schedules._send_notifications_cron()
+        self.assertFalse(schedules.exists(), msg='Should have sent the message')
+        recipients_info = [{'content': '<p>Test</p>', 'notif': [
+            {'partner': self.partner_admin, 'type': 'inbox'},
+            {'partner': self.partner_1, 'type': 'email'},
+        ]}]
+        self.assertMailNotifications(msg, recipients_info)
+
+        self.assertFalse(self.env['mail.message.schedule'].sudo()._update_message_scheduled_datetime(msg, now - timedelta(hours=1)),
+                         'Mail scheduler: should return False when no schedule is found')
+
+    @users('employee')
+    @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.addons.mail.models.mail_message_schedule')
     def test_message_post_w_attachments(self):
         _attachments = [
             ('List1', b'My first attachment'),
diff --git a/addons/test_mail_sms/tests/test_sms_post.py b/addons/test_mail_sms/tests/test_sms_post.py
index 0d870ea605d..deb66ad8574 100644
--- a/addons/test_mail_sms/tests/test_sms_post.py
+++ b/addons/test_mail_sms/tests/test_sms_post.py
@@ -32,7 +32,7 @@ class TestSMSPost(TestSMSCommon, TestSMSRecipients):
         self.assertEqual(messages.subtype_id, self.env.ref('mail.mt_note'))
         self.assertSMSNotification([{'partner': self.partner_1}], 'Mega SMS\nTop moumoutte', messages)
 
-    def test_message_sms_internals_check_existing(self):
+    def test_message_sms_internals_resend_existingd(self):
         with self.with_user('employee'), self.mockSMSGateway(sim_error='wrong_number_format'):
             test_record = self.env['mail.test.sms'].browse(self.test_record.id)
             messages = test_record._message_sms(self._test_body, partner_ids=self.partner_1.ids)
@@ -41,7 +41,7 @@ class TestSMSPost(TestSMSCommon, TestSMSRecipients):
 
         with self.with_user('employee'), self.mockSMSGateway():
             test_record = self.env['mail.test.sms'].browse(self.test_record.id)
-            test_record._notify_thread_by_sms(messages, [{'id': self.partner_1.id, 'notif': 'sms'}], check_existing=True)
+            test_record._notify_thread_by_sms(messages, [{'id': self.partner_1.id, 'notif': 'sms'}], resend_existing=True)
         self.assertSMSNotification([{'partner': self.partner_1}], self._test_body, messages)
 
     def test_message_sms_internals_sms_numbers(self):

From c5afd7c30f99164d2eeedfd9455748db9f80acec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Fri, 8 Jul 2022 14:42:18 +0000
Subject: [PATCH 5/7] [IMP] mail: allow to update scheduled date of
 notifications when updating content
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Purpose of this commit is to allow to update the scheduled date when using
``_message_update_content`` tool method. This allows notably to delay the
sending of notifications in some cases, then send it once all content is
updated according to business requirements.

When giving a scheduled date in kwargs

  * either it is set, and we update message schedulers accordingly;
  * either we force a void value, which means notification should be done
    now and we trigger the schedulers for immediate sending;

Task-2207626 (Rating: Delay rating notification to ease feedback)

Part-of: odoo/odoo#95623
Co-authored-by: Stéphane Debauche <std@odoo.com>
---
 addons/mail/models/mail_thread.py | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index c6b1ffab8ee..7369145896e 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -3171,7 +3171,7 @@ class MailThread(models.AbstractModel):
         return True
 
     def _message_update_content(self, message, body, attachment_ids=None,
-                                strict=True):
+                                strict=True, **kwargs):
         """ Update message content. Currently does not support attachments
         specific code (see ``_message_post_process_attachments``), to be added
         when necessary.
@@ -3189,6 +3189,9 @@ class MailThread(models.AbstractModel):
           content. This should be skipped only when really necessary as it
           creates issues with already-sent notifications, lack of content
           tracking, ...
+
+        Kwargs are supported, notably to match mail.message fields to update.
+        See content of this method for more details about supported keys.
         """
         self.ensure_one()
         if strict:
@@ -3208,6 +3211,17 @@ class MailThread(models.AbstractModel):
         if msg_values:
             message.write(msg_values)
 
+        if 'scheduled_date' in kwargs:
+            # update scheduled datetime
+            if kwargs['scheduled_date']:
+                self.env['mail.message.schedule'].sudo()._update_message_scheduled_datetime(
+                    message,
+                    kwargs['scheduled_date']
+                )
+            # (re)send notifications
+            else:
+                self.env['mail.message.schedule'].sudo()._send_message_notifications(message)
+
         # cleanup related message data if the message is empty
         message.sudo()._filter_empty()._cleanup_side_records()
 

From 6ebb72332e62a6e844bdcd7cba2d953ba9087563 Mon Sep 17 00:00:00 2001
From: std-odoo <std@odoo.com>
Date: Thu, 16 Jun 2022 07:13:23 +0000
Subject: [PATCH 6/7] [IMP] rating: improve rating notification with delaying
 until feedback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Purpose
=======

When someone receives an email with a rating request, he can click on a
smiley. But if he does not write a feedback, nobody is notified. This means
some rating as somehow lost and people are not notified of it.

Specifications
==============

Now, when the user clicks on the smiley, a message is posted but notifications
are not yet sent. This uses the new ``mail.message.schedule`` mechanism added
in this PR. It gives the user some time to write their feedback and send it.

If they write and submit a feedback, the notification process is launched.
Emails and inbox notifications are created and sent.

If they don't write a feedback notification process is launch after 2 hours.

Testing
=======

Some cleaning is done in tests, notably to split some fields tests from
performance test. A bit performance test is also split into sub tests in
order to better understand queries.

Task-2207626 (Rating: Log ratings, post feedbacks)

Part-of: odoo/odoo#95623
Co-authored-by: Thibault Delavallée <tde@odoo.com>
---
 addons/mail/models/mail_thread.py          |  2 +-
 addons/project/models/project.py           |  7 ++-
 addons/rating/controllers/main.py          | 45 ++++++++-------
 addons/rating/models/rating_mixin.py       | 42 ++++++++++----
 addons/test_mail_full/tests/test_rating.py | 67 +++++++++++++---------
 5 files changed, 102 insertions(+), 61 deletions(-)

diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index 7369145896e..7df8b71d74e 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -2381,7 +2381,7 @@ class MailThread(models.AbstractModel):
         self.env['bus.bus'].sudo()._sendmany(bus_notifications)
 
     def _notify_thread_by_email(self, message, recipients_data, msg_vals=False,
-                                mail_auto_delete=True, # mail.mail
+                                mail_auto_delete=True,  # mail.mail
                                 model_description=False, force_email_company=False, force_email_lang=False,  # rendering
                                 resend_existing=False, force_send=True, send_after_commit=True,  # email send
                                 **kwargs):
diff --git a/addons/project/models/project.py b/addons/project/models/project.py
index 49e92abb9bf..3cc11503079 100644
--- a/addons/project/models/project.py
+++ b/addons/project/models/project.py
@@ -2554,8 +2554,11 @@ class Task(models.Model):
             return self.project_id.partner_id
         return res
 
-    def rating_apply(self, rate, token=None, rating=None, feedback=None, subtype_xmlid=None):
-        rating = super(Task, self).rating_apply(rate, token=token, rating=rating, feedback=feedback, subtype_xmlid=subtype_xmlid)
+    def rating_apply(self, rate, token=None, rating=None, feedback=None,
+                     subtype_xmlid=None, notify_delay_send=False):
+        rating = super(Task, self).rating_apply(
+            rate, token=token, rating=rating, feedback=feedback,
+            subtype_xmlid=subtype_xmlid, notify_delay_send=notify_delay_send)
         if self.stage_id and self.stage_id.auto_validation_kanban_state:
             kanban_state = 'done' if rating.rating >= rating_data.RATING_LIMIT_OK else 'blocked'
             self.write({'kanban_state': kanban_state})
diff --git a/addons/rating/controllers/main.py b/addons/rating/controllers/main.py
index e3b3bca5e24..4f546a84cb0 100644
--- a/addons/rating/controllers/main.py
+++ b/addons/rating/controllers/main.py
@@ -24,15 +24,15 @@ class Rating(http.Controller):
         if rate not in (1, 3, 5):
             raise ValueError(_("Incorrect rating: should be 1, 3 or 5 (received %d)"), rate)
 
-        rating = request.env['rating.rating'].sudo().search([('access_token', '=', token)])
-        if not rating:
-            return werkzeug.exceptions.Forbidden()
+        rating, record_sudo = self._get_rating_and_record(token)
 
-        record_sudo = request.env[rating.res_model].sudo().browse(rating.res_id)
-        if not record_sudo.exists():
-            return werkzeug.exceptions.Forbidden()
-
-        rating.write({'rating': rate, 'consumed': True})
+        record_sudo.rating_apply(
+            rate,
+            rating=rating,
+            feedback=_('Customer rated %r.', record_sudo.display_name),
+            subtype_xmlid=None,
+            notify_delay_send=True,
+        )
 
         lang = rating.partner_id.lang or get_lang(request.env).code
         return request.env['ir.ui.view'].with_context(lang=lang)._render_template('rating.rating_external_page_submit', {
@@ -52,22 +52,27 @@ class Rating(http.Controller):
         if rate not in (1, 3, 5):
             raise ValueError(_("Incorrect rating: should be 1, 3 or 5 (received %d)"), rate)
 
-        rating = request.env['rating.rating'].sudo().search([('access_token', '=', token)])
-        if not rating:
-            return werkzeug.exceptions.Forbidden()
+        rating, record_sudo = self._get_rating_and_record(token)
 
-        record_sudo = request.env[rating.res_model].sudo().browse(rating.res_id)
-        if not record_sudo.exists():
-            return werkzeug.exceptions.Forbidden()
-
-        record_sudo.rating_apply(rate,
-                                 rating=rating,
-                                 feedback=kwargs.get('feedback'),
-                                 subtype_xmlid=None,  # force default subtype choice
-                                )
+        record_sudo.rating_apply(
+             rate,
+             rating=rating,
+             feedback=kwargs.get('feedback'),
+             subtype_xmlid=None,  # force default subtype choice
+        )
 
         lang = rating.partner_id.lang or get_lang(request.env).code
         return request.env['ir.ui.view'].with_context(lang=lang)._render_template('rating.rating_external_page_view', {
             'web_base_url': rating.get_base_url(),
             'rating': rating,
         })
+
+    def _get_rating_and_record(self, token):
+        rating_sudo = request.env['rating.rating'].sudo().search([('access_token', '=', token)])
+        if not rating_sudo:
+            raise werkzeug.exceptions.NotFound()
+
+        record_sudo = request.env[rating_sudo.res_model].sudo().browse(rating_sudo.res_id)
+        if not record_sudo.exists():
+            raise werkzeug.exceptions.NotFound()
+        return rating_sudo, record_sudo
diff --git a/addons/rating/models/rating_mixin.py b/addons/rating/models/rating_mixin.py
index 37716eaf860..7ac2b47cd1f 100644
--- a/addons/rating/models/rating_mixin.py
+++ b/addons/rating/models/rating_mixin.py
@@ -1,7 +1,8 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-import operator
+import datetime
+import markupsafe
 
 from odoo import api, fields, models, tools
 from odoo.addons.rating.models import rating_data
@@ -25,7 +26,7 @@ class RatingMixin(models.AbstractModel):
     rating_percentage_satisfaction = fields.Float("Rating Satisfaction", compute='_compute_rating_satisfaction', compute_sudo=True)
     rating_last_text = fields.Selection(string="Rating Text", groups='base.group_user', related="rating_ids.rating_text")
 
-    @api.depends('rating_ids.rating', 'rating_ids.consumed')
+    @api.depends('rating_ids', 'rating_ids.rating', 'rating_ids.consumed')
     def _compute_rating_last_value(self):
         # Pure SQL instead of calling read_group to allow ordering array_agg
         self.flush_model(['rating_ids'])
@@ -198,7 +199,8 @@ class RatingMixin(models.AbstractModel):
                 subtype_id=subtype_id
             )
 
-    def rating_apply(self, rate, token=None, rating=None, feedback=None, subtype_xmlid=None):
+    def rating_apply(self, rate, token=None, rating=None, feedback=None,
+                     subtype_xmlid=None, notify_delay_send=False):
         """ Apply a rating to the record. This rating can either be linked to a
         token (customer flow) or directly a rating record (code flow).
 
@@ -215,6 +217,8 @@ class RatingMixin(models.AbstractModel):
         :param string subtype_xmlid: xml id of a valid mail.message.subtype used
           to post the message (if it applies). If not given a classic comment is
           posted;
+        :param notify_delay_send: Delay the sending by 2 hours of the email so the user
+            can still change his feedback. If False, the email will be sent immediately.
 
         :returns rating: rating.rating record
         """
@@ -226,19 +230,37 @@ class RatingMixin(models.AbstractModel):
             raise ValueError('Invalid token or rating.')
 
         rating.write({'rating': rate, 'feedback': feedback, 'consumed': True})
-        if hasattr(self, 'message_post'):
+        if issubclass(type(self), self.env.registry['mail.thread']):
             if subtype_xmlid is None:
                 subtype_id = self._rating_apply_get_default_subtype_id()
             else:
                 subtype_id = self.env['ir.model.data']._xmlid_to_res_id(subtype_xmlid)
             feedback = tools.plaintext2html(feedback or '')
-            self.message_post(
-                author_id=rating.partner_id.id or None,  # None will set the default author in mail_thread.py
-                body="<img src='%s' alt=':%s/5' style='width:18px;height:18px;float:left;margin-right: 5px;'/>%s"
-                % (rating.rating_image_url, rate, feedback),
-                rating_id=rating.id,
-                subtype_id=subtype_id,
+
+            scheduled_datetime = (
+                fields.Datetime.now() + datetime.timedelta(hours=2)
+                if notify_delay_send else None
             )
+            rating_body = (
+                markupsafe.Markup(
+                    "<img src='%s' alt=':%s/5' style='width:18px;height:18px;float:left;margin-right: 5px;'/>%s"
+                ) % (rating.rating_image_url, rate, feedback)
+            )
+
+            if rating.message_id:
+                self._message_update_content(
+                    rating.message_id, rating_body,
+                    scheduled_date=scheduled_datetime,
+                    strict=False
+                )
+            else:
+                self.message_post(
+                    author_id=rating.partner_id.id or None,  # None will set the default author in mail_thread.py
+                    body=rating_body,
+                    rating_id=rating.id,
+                    scheduled_date=scheduled_datetime,
+                    subtype_id=subtype_id,
+                )
         return rating
 
     def _rating_apply_get_default_subtype_id(self):
diff --git a/addons/test_mail_full/tests/test_rating.py b/addons/test_mail_full/tests/test_rating.py
index 4235571afda..f4ffb5a3c74 100644
--- a/addons/test_mail_full/tests/test_rating.py
+++ b/addons/test_mail_full/tests/test_rating.py
@@ -24,8 +24,9 @@ class TestRatingCommon(TestMailFullCommon, TestSMSRecipients):
 
 @tagged('rating')
 class TestRatingFlow(TestRatingCommon):
+
     def test_initial_values(self):
-        record_rating = self.env['mail.test.rating'].browse(self.record_rating.ids)
+        record_rating = self.record_rating.with_env(self.env)
         self.assertFalse(record_rating.rating_ids)
         self.assertEqual(record_rating.message_partner_ids, self.partner_admin)
         self.assertEqual(len(record_rating.message_ids), 1)
@@ -33,7 +34,7 @@ class TestRatingFlow(TestRatingCommon):
     @users('employee')
     @mute_logger('odoo.addons.mail.models.mail_mail')
     def test_rating_prepare(self):
-        record_rating = self.env['mail.test.rating'].browse(self.record_rating.ids)
+        record_rating = self.record_rating.with_env(self.env)
 
         # prepare rating token
         access_token = record_rating._rating_get_access_token()
@@ -50,15 +51,15 @@ class TestRatingFlow(TestRatingCommon):
     @users('employee')
     @mute_logger('odoo.addons.mail.models.mail_mail')
     def test_rating_rating_apply(self):
-        record_rating = self.env['mail.test.rating'].browse(self.record_rating.ids)
+        record_rating = self.record_rating.with_env(self.env)
         record_messages = record_rating.message_ids
 
         # prepare rating token
         access_token = record_rating._rating_get_access_token()
 
-        # apply a rate as note (first click)
+        # simulate an email click: notification should be delayed
         with self.mock_mail_gateway(mail_unlink_sent=False), self.mock_mail_app():
-            record_rating.rating_apply(5, token=access_token, feedback='Top Feedback', subtype_xmlid='mail.mt_note')
+            record_rating.rating_apply(5, token=access_token, feedback='Top Feedback', notify_delay_send=True)
         message = record_rating.message_ids[0]
         rating = record_rating.rating_ids
 
@@ -69,7 +70,7 @@ class TestRatingFlow(TestRatingCommon):
         self.assertEqual(message.author_id, self.partner_1)
         self.assertEqual(message.rating_ids, rating)
         self.assertFalse(message.notified_partner_ids)
-        self.assertEqual(message.subtype_id, self.env.ref('mail.mt_note'))
+        self.assertEqual(message.subtype_id, self.env.ref('test_mail_full.mt_mail_test_rating_rating_done'))
 
         # check rating update
         self.assertTrue(rating.consumed)
@@ -78,14 +79,14 @@ class TestRatingFlow(TestRatingCommon):
         self.assertEqual(rating.rating, 5)
         self.assertEqual(record_rating.rating_last_value, 5)
 
-        # apply a rate again (second click with feedback)
+        # give a feedback: send notifications (notify_delay_send set to False)
         with self.mock_mail_gateway(mail_unlink_sent=False), self.mock_mail_app():
             record_rating.rating_apply(1, token=access_token, feedback='Bad Feedback')
 
-        # check posted message: a new message is posted with default subtype
+        # check posted message: message is updated
         update_message = record_rating.message_ids[0]
-        self.assertNotEqual(update_message, message)
-        self.assertEqual(record_rating.message_ids, record_messages + message + update_message)
+        self.assertEqual(update_message, message, 'Should update first message')
+        self.assertEqual(record_rating.message_ids, record_messages + update_message)
         self.assertIn('Bad Feedback', update_message.body)
         self.assertIn('/rating/static/src/img/rating_1.png', update_message.body)
         self.assertEqual(update_message.author_id, self.partner_1)
@@ -94,22 +95,22 @@ class TestRatingFlow(TestRatingCommon):
         self.assertEqual(update_message.subtype_id, self.env.ref("test_mail_full.mt_mail_test_rating_rating_done"))
 
         # check rating update
-        rating = record_rating.rating_ids
-        self.assertTrue(rating.consumed)
-        self.assertEqual(rating.feedback, 'Bad Feedback')
-        self.assertEqual(rating.message_id, update_message)
-        self.assertEqual(rating.rating, 1)
+        new_rating = record_rating.rating_ids
+        self.assertEqual(new_rating, rating, 'Should update first rating')
+        self.assertTrue(new_rating.consumed)
+        self.assertEqual(new_rating.feedback, 'Bad Feedback')
+        self.assertEqual(new_rating.message_id, update_message)
+        self.assertEqual(new_rating.rating, 1)
         self.assertEqual(record_rating.rating_last_value, 1)
 
 
-@tagged('rating', 'mail_performance', 'post_install', '-at_install')
-class TestRatingPerformance(TestRatingCommon):
+@tagged('rating')
+class TestRatingMixin(TestRatingCommon):
 
-    @users('__system__')
+    @users('employee')
     @warmup
-    def test_rating_last_value_perfs(self):
-
-        record_rating = self.env['mail.test.rating'].browse(self.record_rating.ids)
+    def test_rating_values(self):
+        record_rating = self.record_rating.with_env(self.env)
 
         # prepare rating token
         access_0 = record_rating._rating_get_access_token()
@@ -127,29 +128,38 @@ class TestRatingPerformance(TestRatingCommon):
         self.assertEqual(record_rating.rating_last_value, 5, "The last rating is kept.")
         self.assertEqual(record_rating.rating_avg, 3, "The average should be equal to 3")
 
+
+@tagged('rating', 'mail_performance', 'post_install', '-at_install')
+class TestRatingPerformance(TestRatingCommon):
+
+    @users('employee')
+    @warmup
+    def test_rating_last_value_perfs(self):
         RECORD_COUNT = 100
-        partners = self.env['res.partner'].create([
+        partners = self.env['res.partner'].sudo().create([
             {'name': 'Jean-Luc %s' % (idx), 'email': 'jean-luc-%s@opoo.com' % (idx)} for idx in range(RECORD_COUNT)])
 
-        with self.assertQueryCount(__system__=5910):  # tmf 4710 / com 5510
+        with self.assertQueryCount(employee=1516):  # tmf 1516 / com 5510
             record_ratings = self.env['mail.test.rating'].create([{
                 'customer_id': partners[idx].id,
                 'name': 'Test Rating',
                 'user_id': self.user_admin.id,
             } for idx in range(RECORD_COUNT)])
+            self.flush_tracking()
+
+        with self.assertQueryCount(employee=2604):  # tmf 2204 / com 2404
             for record in record_ratings:
                 access_token = record._rating_get_access_token()
                 record.rating_apply(1, token=access_token)
+            self.flush_tracking()
 
-            record_ratings.rating_ids.write_date = datetime(2022, 1, 1, 14, 00)
+        with self.assertQueryCount(employee=2603):  # tmf 2203 / com 2403
             for record in record_ratings:
                 access_token = record._rating_get_access_token()
                 record.rating_apply(5, token=access_token)
+            self.flush_tracking()
 
-        new_ratings = record_ratings.rating_ids.filtered(lambda r: r.rating == 1)
-        new_ratings.write_date = datetime(2022, 2, 1, 14, 00)
-        new_ratings.flush_model(['write_date'])
-        with self.assertQueryCount(__system__=1):
+        with self.assertQueryCount(employee=1):
             record_ratings._compute_rating_last_value()
             vals = [val == 5 for val in record_ratings.mapped('rating_last_value')]
             self.assertTrue(all(vals), "The last rating is kept.")
@@ -157,6 +167,7 @@ class TestRatingPerformance(TestRatingCommon):
 
 @tagged('rating')
 class TestRatingRoutes(HttpCase, TestRatingCommon):
+
     def test_open_rating_route(self):
         access_token = self.record_rating._rating_get_access_token()
         self.url_open(f"/rate/{access_token}/5")

From 22c3cc51eda7f126178d70f765f846e411fe2d0a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Wed, 17 Aug 2022 13:10:03 +0000
Subject: [PATCH 7/7] [FIX] rating: fix bootstrap and styling of rating email
 and pages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Rating page is broken since migration to bootstrap5. This commit fixes various
issues linked to btn-group, display and usage of group checkboxes. Overall
html structure is cleaned and bootstrap 5 rules are applied.

Some small display improvements are also applied to the rating email.

Task-2207626 (Rating: Log ratings, post feedbacks)

Part-of: odoo/odoo#95623
Co-authored-by: Adrien Dieudonné <adr@odoo.com>
---
 addons/project/data/mail_template_data.xml    |  2 +-
 .../static/src/scss/rating_templates.scss     | 23 +++++++--------
 addons/rating/views/rating_templates.xml      | 28 +++++++++++++------
 3 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/addons/project/data/mail_template_data.xml b/addons/project/data/mail_template_data.xml
index 8d75fbc9c5e..e524ff084b6 100644
--- a/addons/project/data/mail_template_data.xml
+++ b/addons/project/data/mail_template_data.xml
@@ -56,7 +56,7 @@
             <table border="0" cellpadding="0" cellspacing="0" width="590" summary="o_mail_notification" style="width:100%; margin: 32px 0px 32px 0px;">
                 <tr><td style="font-size: 13px;">
                     <strong>Tell us how you feel about our service</strong><br/>
-                    <span style="text-color: #888888">(click on one of these smileys)</span>
+                    <span style="font-size: 12px; opacity: 0.5; color: #454748;">(click on one of these smileys)</span>
                 </td></tr>
                 <tr><td style="font-size: 13px;">
                     <table style="width:100%;text-align:center;margin-top:2rem;">
diff --git a/addons/rating/static/src/scss/rating_templates.scss b/addons/rating/static/src/scss/rating_templates.scss
index 137e5152d2e..65a62162bcf 100644
--- a/addons/rating/static/src/scss/rating_templates.scss
+++ b/addons/rating/static/src/scss/rating_templates.scss
@@ -9,22 +9,23 @@
         &:hover {
             transform: scale(1.1);
         }
-        a {
+        img {
             &.bg-white:hover,
             &.bg-white:focus {
                 background-color: #ffffff !important;
             }
         }
-        &.active {
-            filter: drop-shadow(0 0 10px rgba(0,0,0,0.25));
-            -webkit-filter: drop-shadow(0 0 10px rgba(0,0,0,0.25));
-            transform: scale(1.1);
-            a {
-                &.bg-white:hover,
-                &.bg-white:focus {
-                    background-color: #ffffff !important;
-                }
+    }
+
+    .btn-check:checked + .o_rating_label {
+        filter: drop-shadow(0 0 10px rgba(0,0,0,0.25));
+        -webkit-filter: drop-shadow(0 0 10px rgba(0,0,0,0.25));
+        transform: scale(1.1);
+        img {
+            &.bg-white:hover,
+            &.bg-white:focus {
+                background-color: #ffffff !important;
             }
         }
     }
-}
+}
\ No newline at end of file
diff --git a/addons/rating/views/rating_templates.xml b/addons/rating/views/rating_templates.xml
index e2aadfc628e..3917dea3a3d 100644
--- a/addons/rating/views/rating_templates.xml
+++ b/addons/rating/views/rating_templates.xml
@@ -23,22 +23,32 @@
                     <div class="row text-center justify-content-center">
                         <h1 class="col-12 mt-5">Thank you for rating our services!</h1>
                         <form class="col-md-6" t-attf-action="/rate/#{token}/submit_feedback" method="post">
-                            <div class="btn-group btn-group-toggle row flex-nowrap justify-content-center w-100 mt-5" data-bs-toggle="buttons">
+                            <div class="btn-group row flex-nowrap justify-content-center w-100 mt-5"
+                                 role="group"
+                                 data-bs-toggle="buttons">
                                 <t t-foreach="rate_names" t-as="rate_name">
-                                    <label t-attf-class="col p-4 btn o_rating_label shadow-none transition-base {{rate == rate_name and 'active' or ''}}">
-                                        <input type="radio" name="rate" t-attf-id="rate_{{rate_name}}" t-att-value="rate_name" t-att-checked="rate == rate_name"/>
-                                        <a class="o_rating d-block bg-white rounded-circle" href="#">
-                                            <img t-attf-src='/rating/static/src/img/rating_#{rate_name}.svg' t-att-alt="rate_name_value" t-att-title="rate_name_value"/>
-                                        </a>
+                                    <input type="radio" name="rate"
+                                           class="btn-check"
+                                           t-attf-id="rate_{{rate_name}}"
+                                           t-att-value="rate_name"
+                                           t-att-checked="rate == rate_name"/>
+                                    <label t-attf-class="col p-4 btn o_rating_label shadow-none transition-base {{rate == rate_name and 'active' or ''}}"
+                                           t-att-for="'rate_%s' % (rate_name)">
+                                        <img t-attf-src='/rating/static/src/img/rating_#{rate_name}.svg'
+                                             t-att-alt="rate_name_value"
+                                             t-att-title="rate_name_value"/>
                                     </label>
                                 </t>
                             </div>
                             <p class="mt-5">
                                 Feel free to write a feedback on your experience:
                             </p>
-                            <input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
-                            <textarea class="form-control" name="feedback" rows="8" t-att-value="rating.feedback"></textarea>
-                            <button type="submit" class="btn btn-primary mt-4" style="margin-top:8px;">Send Feedback</button>
+                            <input type="hidden" name="csrf_token"
+                                   t-att-value="request.csrf_token()"/>
+                            <textarea class="form-control" name="feedback" rows="8"
+                                      t-att-value="rating.feedback"></textarea>
+                            <button type="submit" class="btn btn-primary mt-4"
+                                    style="margin-top:8px;">Send Feedback</button>
                         </form>
                     </div>
                 </div>