[MERGE][IMP] mail, rating: delay rating request sending

FUNCTIONAL ISSUE

When they receive a rating request email, customers can click on this email to
pick a satisfaction level. However, this feedback is currently effectively
posted only when customers submit the feedback form.

Many customers do not go that far in the process and just close that form. This
means that while the feedback is caught and stored in DB nothing is posted and
users are not notified of this rating.

PURPOSE

When the customer clicks on an emoji, post a message in the chatter without
launching notifications. After 2 hours notify followers. If before this delay
the customer fills in the form, update the post content (new face, feedback)
and launch notification process with the feedback (inbox, emails, ...).

By default, the user has 2 hours to fill the feedback, after those 2 hours
we notify the message containing only the first rating without feedback.

TECHNICAL

Currently, rating from email (route with token) is the following

  * Customer receives an email;
  * Customer clicks one of the smiley face that redirects to a ``/rate`` route
    with the token and rate value;
  * the linked ``rating.rating`` record is updated and set consumed;
  * Customer may change its face and add a feedback. In that case a message
    is posted with the rating face in it;

Issues raised by this

  * first rating update does not notify anyone, no message is posted;
  * if Customer does not give a feedback, process is stuck at that point;
  * the rating is not really linked to the posted message (see ``message_id``
    field of ``rating`` record);

Fix that issue to have message directly at beginning of the flow and improve
log and notifications. Update ``rating_apply`` to post the message and update
it when feedback is given.

Allow to delay sending of notifications using a ``mail.message.schedule``
queue model that stores when to launch the notification process for a given
message.

See sub commits for more details.

LINKS

Task-2826699 (Mail: use datetime for scheduled_date mail field instead of char)
Task-2207626 (Rating: Log ratings, post feedbacks)

closes odoo/odoo#95623

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

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']]
 

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.')

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',

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,

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>

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

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

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:

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
     # ------------------------------------------------------

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

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
@@ -205,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,

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
@@ -433,14 +434,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"))
 
     # ------------------------------------------------------
@@ -1417,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'))
@@ -2298,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()``)
         """
@@ -2306,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
 
@@ -2355,9 +2381,9 @@ 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
-                                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.
 
@@ -2384,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
@@ -2449,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'),
@@ -2654,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;
@@ -2690,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):
@@ -3122,8 +3170,66 @@ class MailThread(models.AbstractModel):
         msg_not_comment.write(msg_vals)
         return True
 
+    def _message_update_content(self, message, body, attachment_ids=None,
+                                strict=True, **kwargs):
+        """ 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, ...
+
+        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:
+            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)
+
+        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()
+
+        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

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

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:

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"

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>

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
                 )
 

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;">

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})

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

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):

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;
             }
         }
     }
-}
+}

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>

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),

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()

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):

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):

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)

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)

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

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'),

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")

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):