[IMP] rating: improve rating notification with delaying until feedback

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

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

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