[IMP] rating: Parent document name update

For objects inherting from rating.mixin, when their parent model change
(e.i.: task from a project to another), the "Parent document name" does
not change. The rating is still considered belonging to the old parent
object (rating stat of project is not correct).

Change the rating API. In rating mixin, instead of having 2 methods
returning parent id and parent model (rating_get_parent_id and
rating_get_parent_model_name), we should have one method returning the
'parent relation field' (inevitably m2o field). Then we can deduce its
parent_res_id/parent_res_model, and check this parent field is in values
of write to trigger the recompute of parent_res_name.

Also, add "ondelete=cascade" on parent_res_model_id

Impacted modules: project, rating, helpdesk, livechat

addons/im_livechat/models/mail_channel.py

@@ -134,8 +134,5 @@ class MailChannel(models.Model):
 
     # Rating Mixin
 
-    def rating_get_parent_model_name(self, values):
-        return 'im_livechat.channel'
-
-    def rating_get_parent_id(self):
-        return self.livechat_channel_id.id
+    def rating_get_parent(self):
+        return 'livechat_channel_id'

addons/project/models/project.py

@@ -1044,11 +1044,8 @@ class Task(models.Model):
     def rating_apply(self, rate, token=None, feedback=None, subtype=None):
         return super(Task, self).rating_apply(rate, token=token, feedback=feedback, subtype="project.mt_task_rating")
 
-    def rating_get_parent_model_name(self, vals):
-        return 'project.project'
-
-    def rating_get_parent_id(self):
-        return self.project_id.id
+    def rating_get_parent(self):
+        return 'project_id'
 
 
 class ProjectTags(models.Model):

addons/project/tests/test_project_flow.py

@@ -1,8 +1,11 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
+import base64
+
 from .test_project_base import TestProjectBase
 from odoo.tools import mute_logger
+from odoo.modules.module import get_resource_path
 
 
 EMAIL_TPL = """Return-Path: <whatever-2a840@postmaster.twitter.com>
@@ -117,3 +120,72 @@ class TestProjectFlow(TestProjectBase):
         })
 
         self.assertEqual(parent_task.partner_id, child_task.partner_id, "Subtask partner should not change when changing project")
+
+    def test_rating(self):
+        """Check if rating works correctly even when task is changed from project A to project B"""
+        Task = self.env['project.task'].with_context({'tracking_disable': True})
+        first_task = Task.create({
+            'name': 'first task',
+            'user_id': self.user_projectuser.id,
+            'project_id': self.project_pigs.id,
+            'partner_id': self.partner_2.id,
+        })
+
+        self.assertEqual(first_task.rating_count, 0, "Task should have no rating associated with it")
+
+        Rating = self.env['rating.rating']
+        rating_good = Rating.create({
+            'res_model_id': self.env['ir.model']._get('project.task').id,
+            'res_id': first_task.id,
+            'parent_res_model_id': self.env['ir.model']._get('project.project').id,
+            'parent_res_id': self.project_pigs.id,
+            'rated_partner_id': self.partner_2.id,
+            'partner_id': self.partner_2.id,
+            'rating': 10,
+            'consumed': False,
+        })
+
+        rating_bad = Rating.create({
+            'res_model_id': self.env['ir.model']._get('project.task').id,
+            'res_id': first_task.id,
+            'parent_res_model_id': self.env['ir.model']._get('project.project').id,
+            'parent_res_id': self.project_pigs.id,
+            'rated_partner_id': self.partner_2.id,
+            'partner_id': self.partner_2.id,
+            'rating': 5,
+            'consumed': True,
+        })
+
+        # We need to invalidate cache since it is not done automatically by the ORM
+        # Our One2Many is linked to a res_id (int) for which the orm doesn't create an inverse
+        first_task.invalidate_cache()
+
+        self.assertEqual(rating_good.rating_text, 'satisfied')
+        self.assertEqual(rating_bad.rating_text, 'not_satisfied')
+        self.assertEqual(first_task.rating_count, 1, "Task should have only one rating associated, since one is not consumed")
+        self.assertEqual(rating_good.parent_res_id, self.project_pigs.id)
+        self.assertEqual(self.project_goats.percentage_satisfaction_task, -1)
+        self.assertEqual(self.project_pigs.percentage_satisfaction_task, -1)
+
+        # Consuming rating_good
+        first_task.rating_apply(10, rating_good.access_token)
+
+        # We need to invalidate cache since it is not done automatically by the ORM
+        # Our One2Many is linked to a res_id (int) for which the orm doesn't create an inverse
+        first_task.invalidate_cache()
+
+        self.assertEqual(first_task.rating_count, 2, "Task should have two ratings associated with it")
+        self.assertEqual(rating_good.parent_res_id, self.project_pigs.id)
+        self.assertEqual(self.project_goats.percentage_satisfaction_task, -1)
+        self.assertEqual(self.project_pigs.percentage_satisfaction_task, 50)
+
+        # We change the task from project_pigs to project_goats, ratings should be associated with the new project
+        first_task.project_id = self.project_goats.id
+
+        # We need to invalidate cache since it is not done automatically by the ORM
+        # Our One2Many is linked to a res_id (int) for which the orm doesn't create an inverse
+        first_task.invalidate_cache()
+
+        self.assertEqual(rating_good.parent_res_id, self.project_goats.id)
+        self.assertEqual(self.project_goats.percentage_satisfaction_task, 50)
+        self.assertEqual(self.project_pigs.percentage_satisfaction_task, -1)

addons/rating/models/rating.py

@@ -37,7 +37,7 @@ class Rating(models.Model):
     res_model = fields.Char(string='Document Model', related='res_model_id.model', store=True, index=True, readonly=True)
     res_id = fields.Integer(string='Document', required=True, help="Identifier of the rated object", index=True)
     parent_res_name = fields.Char('Parent Document Name', compute='_compute_parent_res_name', store=True)
-    parent_res_model_id = fields.Many2one('ir.model', 'Parent Related Document Model', index=True)
+    parent_res_model_id = fields.Many2one('ir.model', 'Parent Related Document Model', index=True, ondelete='cascade')
     parent_res_model = fields.Char('Parent Document Model', store=True, related='parent_res_model_id.model', index=True)
     parent_res_id = fields.Integer('Parent Document', index=True)
     rated_partner_id = fields.Many2one('res.partner', string="Rated person", help="Owner of the rated resource")
@@ -105,10 +105,10 @@ class Rating(models.Model):
             'parent_res_model_id': False,
             'parent_res_id': False,
         }
-        if hasattr(current_record, 'rating_get_parent_model_name'):
-            parent_res_model = current_record.rating_get_parent_model_name(values)
-            data['parent_res_model_id'] = self.env['ir.model']._get(parent_res_model).id
-            data['parent_res_id'] = current_record.rating_get_parent_id()
+        if hasattr(current_record, 'rating_get_parent'):
+            parent_res_model = getattr(current_record, current_record.rating_get_parent())
+            data['parent_res_model_id'] = self.env['ir.model']._get(parent_res_model._name).id
+            data['parent_res_id'] = parent_res_model.id
         return data
 
     @api.multi
@@ -150,6 +150,7 @@ class RatingMixin(models.AbstractModel):
                 record.rating_last_value = ratings.rating
 
     @api.multi
+    @api.depends('rating_ids')
     def _compute_rating_count(self):
         read_group_res = self.env['rating.rating'].read_group(
             [('res_model', '=', self._name), ('res_id', 'in', self.ids), ('consumed', '=', True)],
@@ -161,11 +162,15 @@ class RatingMixin(models.AbstractModel):
             record.rating_count = result.get(record.id)
 
     def write(self, values):
-        """ If the rated ressource name is modified, we should update the rating res_name too. """
+        """ If the rated ressource name is modified, we should update the rating res_name too.
+            If the rated ressource parent is changed we should update the parent_res_id too"""
         result = super(RatingMixin, self).write(values)
-        for r in self:
-            if r._rec_name in values:
-                r.rating_ids._compute_res_name()
+        for record in self:
+            if record._rec_name in values:
+                record.rating_ids._compute_res_name()
+            if record.rating_get_parent() in values:
+                for rating in record.rating_ids:
+                    rating.parent_res_id = record[record.rating_get_parent()].id
         return result
 
     def unlink(self):
@@ -175,12 +180,9 @@ class RatingMixin(models.AbstractModel):
         self.env['rating.rating'].sudo().search([('res_model', '=', self._name), ('res_id', 'in', record_ids)]).unlink()
         return result
 
-    def rating_get_parent_model_name(self, vals):
-        """ Return the parent model name """
-        return None
-
-    def rating_get_parent_id(self):
-        """ Return the parent record id """
+    def rating_get_parent(self):
+        """Return the parent relation field name
+           Should return a Many2One"""
         return None
 
     def rating_get_partner_id(self):