Skip to content

Commit

Permalink
[IMP] rating: introduce the parent mixin
Browse files Browse the repository at this point in the history
When creating a task, the satisfaction of the project is recomputed even
if the task has not rating. This can lead to performance issue:
concurrency error in Postgresql can happen if a task is created when
somebody else tries to update the project.
The satisfaction recomputation will give and concurrency error whereas
it will return the same satisfaction percentage, since the task has no
rating.  The same problem will happens in every modules implementing
the rating feature: project, helpdesk and livechat.

This problem leads us to create the rating.parent.mixin in order to
retrigger the computation only when a new rating appears on the
parent object. We also unstore the field to make its computation lazy.
Maybe in a near future, this field will be computed in SQL directly.
The parent mixin allow us to standardize the behavior to all parent
rating models.
These optimizations should reduce the number of computation and speed
up the task creation.

opw-1902502
Task-1903565
  • Loading branch information
jem-odoo committed Dec 4, 2018
1 parent 605e999 commit 4747940
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
1 change: 1 addition & 0 deletions addons/project/tests/test_project_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ def test_rating(self):
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)

Expand Down
39 changes: 37 additions & 2 deletions addons/rating/models/rating.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
import base64
from datetime import timedelta
import uuid

from odoo import api, fields, models, tools, _
Expand Down Expand Up @@ -133,6 +134,41 @@ def action_open_rated_object(self):
}


class RatingParentMixin(models.AbstractModel):
_name = 'rating.parent.mixin'
_description = "Rating Parent Mixin"
_rating_satisfaction_days = False # Number of last days used to compute parent satisfaction. Set to False to include all existing rating.

rating_ids = fields.One2many('rating.rating', 'parent_res_id', string='Ratings', domain=lambda self: [('parent_res_model', '=', self._name)], auto_join=True)
rating_percentage_satisfaction = fields.Integer("Rating Satisfaction", compute="_compute_rating_percentage_satisfaction", store=False, help="Percentage of happy ratings")

@api.depends('rating_ids.rating', 'rating_ids.consumed')
def _compute_rating_percentage_satisfaction(self):
# build domain and fetch data
domain = [('parent_res_model', '=', self._name), ('parent_res_id', 'in', self.ids), ('rating', '>=', 1), ('consumed', '=', True)]
if self._rating_satisfaction_days:
domain += [('create_date', '>=', fields.Datetime.to_string(fields.datetime.now() - timedelta(days=self._rating_satisfaction_days)))]
data = self.env['rating.rating'].read_group(domain, ['parent_res_id', 'rating'], ['parent_res_id', 'rating'], lazy=False)

# get repartition of grades per parent id
default_grades = {'great': 0, 'okay': 0, 'bad': 0}
grades_per_parent = dict((parent_id, dict(default_grades)) for parent_id in self.ids) # map: {parent_id: {'great': 0, 'bad': 0, 'ok': 0}}
for item in data:
parent_id = item['parent_res_id']
rating = item['rating']
if rating >= RATING_LIMIT_SATISFIED:
grades_per_parent[parent_id]['great'] += item['__count']
elif rating > RATING_LIMIT_OK:
grades_per_parent[parent_id]['okay'] += item['__count']
else:
grades_per_parent[parent_id]['bad'] += item['__count']

# compute percentage per parent
for record in self:
repartition = grades_per_parent.get(record.id)
record.rating_percentage_satisfaction = repartition['great'] * 100 / sum(repartition.values()) if sum(repartition.values()) else -1


class RatingMixin(models.AbstractModel):
_name = 'rating.mixin'
_description = "Rating Mixin"
Expand Down Expand Up @@ -171,8 +207,7 @@ def write(self, values):
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
record.rating_ids.write({'parent_res_id': record[record.rating_get_parent()].id})
return result

def unlink(self):
Expand Down

0 comments on commit 4747940

Please sign in to comment.