From c04e0fa10a61a034ec319fb379fa5a21ad049c9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 22 Feb 2022 14:43:30 +0000 Subject: [PATCH 1/7] [MOV] rating: split rating mixin It currently holds two different mixins. Let us split it into two files in order to have a file per model. Also a quick reordering of rating model is performed just to sort a bit things Task-2728564 Part-of: odoo/odoo#82792 --- addons/rating/models/__init__.py | 1 + addons/rating/models/rating.py | 19 ++--- addons/rating/models/rating_mixin.py | 71 +----------------- addons/rating/models/rating_parent_mixin.py | 81 +++++++++++++++++++++ 4 files changed, 93 insertions(+), 79 deletions(-) create mode 100644 addons/rating/models/rating_parent_mixin.py diff --git a/addons/rating/models/__init__.py b/addons/rating/models/__init__.py index 44c09740ff3..4492fc6fc0f 100644 --- a/addons/rating/models/__init__.py +++ b/addons/rating/models/__init__.py @@ -2,5 +2,6 @@ from . import rating from . import rating_mixin +from . import rating_parent_mixin from . import mail_thread from . import mail_message diff --git a/addons/rating/models/rating.py b/addons/rating/models/rating.py index a10c839595d..78d412531b2 100644 --- a/addons/rating/models/rating.py +++ b/addons/rating/models/rating.py @@ -23,15 +23,6 @@ class Rating(models.Model): _description = "Rating" _order = 'write_date desc' _rec_name = 'res_name' - _sql_constraints = [ - ('rating_range', 'check(rating >= 0 and rating <= 5)', 'Rating should be between 0 and 5'), - ] - - @api.depends('res_model', 'res_id') - def _compute_res_name(self): - for rating in self: - name = self.env[rating.res_model].sudo().browse(rating.res_id).name_get() - rating.res_name = name and name[0][1] or ('%s/%s') % (rating.res_model, rating.res_id) @api.model def _default_access_token(self): @@ -71,6 +62,16 @@ class Rating(models.Model): access_token = fields.Char('Security Token', default=_default_access_token, help="Access token to set the rating of the value") consumed = fields.Boolean(string="Filled Rating", help="Enabled if the rating has been filled.") + _sql_constraints = [ + ('rating_range', 'check(rating >= 0 and rating <= 5)', 'Rating should be between 0 and 5'), + ] + + @api.depends('res_model', 'res_id') + def _compute_res_name(self): + for rating in self: + name = self.env[rating.res_model].sudo().browse(rating.res_id).name_get() + rating.res_name = name and name[0][1] or ('%s/%s') % (rating.res_model, rating.res_id) + @api.depends('res_model', 'res_id') def _compute_resource_ref(self): for rating in self: diff --git a/addons/rating/models/rating_mixin.py b/addons/rating/models/rating_mixin.py index 13cbe90e000..4b6aad4668e 100644 --- a/addons/rating/models/rating_mixin.py +++ b/addons/rating/models/rating_mixin.py @@ -1,8 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. + import operator -from collections import defaultdict -from datetime import timedelta from odoo import api, fields, models, tools from odoo.addons.rating.models.rating import RATING_LIMIT_SATISFIED, RATING_LIMIT_OK, RATING_LIMIT_MIN, RATING_TEXT @@ -22,74 +21,6 @@ RATING_AVG_OK = 2.33 RATING_AVG_MIN = RATING_LIMIT_MIN -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', - auto_join=True, groups='base.group_user', - domain=lambda self: [('parent_res_model', '=', self._name)]) - rating_percentage_satisfaction = fields.Integer( - "Rating Satisfaction", - compute="_compute_rating_percentage_satisfaction", compute_sudo=True, - store=False, help="Percentage of happy ratings") - rating_count = fields.Integer(string='# Ratings', compute="_compute_rating_percentage_satisfaction", compute_sudo=True) - rating_avg = fields.Float('Average Rating', groups='base.group_user', - compute='_compute_rating_percentage_satisfaction', compute_sudo=True, search='_search_rating_avg') - rating_avg_percentage = fields.Float('Average Rating (%)', groups='base.group_user', - compute='_compute_rating_percentage_satisfaction', compute_sudo=True) - rating_last_value = fields.Float('Rating Last Value', groups='base.group_user', related='rating_ids.rating') - - @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', '>=', RATING_LIMIT_MIN), ('consumed', '=', True)] - if self._rating_satisfaction_days: - domain += [('write_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}} - rating_scores_per_parent = defaultdict(int) # contains the total of the rating values per record - for item in data: - parent_id = item['parent_res_id'] - rating = item['rating'] - if rating > RATING_LIMIT_OK: - grades_per_parent[parent_id]['great'] += item['__count'] - elif rating > RATING_LIMIT_MIN: - grades_per_parent[parent_id]['okay'] += item['__count'] - else: - grades_per_parent[parent_id]['bad'] += item['__count'] - rating_scores_per_parent[parent_id] += rating * item['__count'] - - # compute percentage per parent - for record in self: - repartition = grades_per_parent.get(record.id, default_grades) - rating_count = sum(repartition.values()) - record.rating_count = rating_count - record.rating_percentage_satisfaction = repartition['great'] * 100 / rating_count if rating_count else -1 - record.rating_avg = rating_scores_per_parent[record.id] / rating_count if rating_count else 0 - record.rating_avg_percentage = record.rating_avg / 5 - - def _search_rating_avg(self, operator, value): - if operator not in OPERATOR_MAPPING: - raise NotImplementedError('This operator %s is not supported in this search method.' % operator) - domain = [('parent_res_model', '=', self._name), ('consumed', '=', True), ('rating', '>=', RATING_LIMIT_MIN)] - if self._rating_satisfaction_days: - min_date = fields.datetime.now() - timedelta(days=self._rating_satisfaction_days) - domain = expression.AND([domain, [('write_date', '>=', fields.Datetime.to_string(min_date))]]) - rating_read_group = self.env['rating.rating'].sudo().read_group(domain, ['parent_res_id', 'rating_avg:avg(rating)'], ['parent_res_id']) - parent_res_ids = [ - res['parent_res_id'] - for res in rating_read_group - if OPERATOR_MAPPING[operator](float_compare(res['rating_avg'], value, 2), 0) - ] - return [('id', 'in', parent_res_ids)] - - class RatingMixin(models.AbstractModel): _name = 'rating.mixin' _description = "Rating Mixin" diff --git a/addons/rating/models/rating_parent_mixin.py b/addons/rating/models/rating_parent_mixin.py new file mode 100644 index 00000000000..62c040c4ba1 --- /dev/null +++ b/addons/rating/models/rating_parent_mixin.py @@ -0,0 +1,81 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import operator + +from collections import defaultdict +from datetime import timedelta + +from odoo import api, fields, models +from odoo.addons.rating.models.rating import RATING_LIMIT_OK, RATING_LIMIT_MIN +from odoo.addons.rating.models.rating_mixin import OPERATOR_MAPPING +from odoo.osv import expression +from odoo.tools.float_utils import float_compare + + +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', + auto_join=True, groups='base.group_user', + domain=lambda self: [('parent_res_model', '=', self._name)]) + rating_percentage_satisfaction = fields.Integer( + "Rating Satisfaction", + compute="_compute_rating_percentage_satisfaction", compute_sudo=True, + store=False, help="Percentage of happy ratings") + rating_count = fields.Integer(string='# Ratings', compute="_compute_rating_percentage_satisfaction", compute_sudo=True) + rating_avg = fields.Float('Average Rating', groups='base.group_user', + compute='_compute_rating_percentage_satisfaction', compute_sudo=True, search='_search_rating_avg') + rating_avg_percentage = fields.Float('Average Rating (%)', groups='base.group_user', + compute='_compute_rating_percentage_satisfaction', compute_sudo=True) + rating_last_value = fields.Float('Rating Last Value', groups='base.group_user', related='rating_ids.rating') + + @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', '>=', RATING_LIMIT_MIN), ('consumed', '=', True)] + if self._rating_satisfaction_days: + domain += [('write_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}} + rating_scores_per_parent = defaultdict(int) # contains the total of the rating values per record + for item in data: + parent_id = item['parent_res_id'] + rating = item['rating'] + if rating > RATING_LIMIT_OK: + grades_per_parent[parent_id]['great'] += item['__count'] + elif rating > RATING_LIMIT_MIN: + grades_per_parent[parent_id]['okay'] += item['__count'] + else: + grades_per_parent[parent_id]['bad'] += item['__count'] + rating_scores_per_parent[parent_id] += rating * item['__count'] + + # compute percentage per parent + for record in self: + repartition = grades_per_parent.get(record.id, default_grades) + rating_count = sum(repartition.values()) + record.rating_count = rating_count + record.rating_percentage_satisfaction = repartition['great'] * 100 / rating_count if rating_count else -1 + record.rating_avg = rating_scores_per_parent[record.id] / rating_count if rating_count else 0 + record.rating_avg_percentage = record.rating_avg / 5 + + def _search_rating_avg(self, operator, value): + if operator not in OPERATOR_MAPPING: + raise NotImplementedError('This operator %s is not supported in this search method.' % operator) + domain = [('parent_res_model', '=', self._name), ('consumed', '=', True), ('rating', '>=', RATING_LIMIT_MIN)] + if self._rating_satisfaction_days: + min_date = fields.datetime.now() - timedelta(days=self._rating_satisfaction_days) + domain = expression.AND([domain, [('write_date', '>=', fields.Datetime.to_string(min_date))]]) + rating_read_group = self.env['rating.rating'].sudo().read_group(domain, ['parent_res_id', 'rating_avg:avg(rating)'], ['parent_res_id']) + parent_res_ids = [ + res['parent_res_id'] + for res in rating_read_group + if OPERATOR_MAPPING[operator](float_compare(res['rating_avg'], value, 2), 0) + ] + return [('id', 'in', parent_res_ids)] From 83d2affbe96976af217ce855ac73f8ab281ee8b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 22 Feb 2022 14:51:19 +0000 Subject: [PATCH 2/7] [MOV][FIX] rating: move rating values, thresholds and converters into a single file Purpose is to try to have a single entry point when dealing with rating values. Also fix incoherency introduced with odoo/odoo@32ba41754996727551106a57d51ac70e92430c04 where grades are not coherent in all computation. This new computation seems better as bad is now limited to <= 1 everywhere. Task-2728564 Part-of: odoo/odoo#82792 --- addons/project/report/project_report.py | 2 +- addons/rating/models/__init__.py | 1 + addons/rating/models/rating.py | 33 ++-------- addons/rating/models/rating_data.py | 73 +++++++++++++++++++++ addons/rating/models/rating_mixin.py | 54 ++++----------- addons/rating/models/rating_parent_mixin.py | 24 +++---- 6 files changed, 101 insertions(+), 86 deletions(-) create mode 100644 addons/rating/models/rating_data.py diff --git a/addons/project/report/project_report.py b/addons/project/report/project_report.py index a40c6dfee97..9b3e911fd17 100644 --- a/addons/project/report/project_report.py +++ b/addons/project/report/project_report.py @@ -3,7 +3,7 @@ from odoo import fields, models, tools -from odoo.addons.rating.models.rating import RATING_LIMIT_MIN +from odoo.addons.rating.models.rating_data import RATING_LIMIT_MIN class ReportProjectTaskUser(models.Model): _name = "report.project.task.user" diff --git a/addons/rating/models/__init__.py b/addons/rating/models/__init__.py index 4492fc6fc0f..d236d87301b 100644 --- a/addons/rating/models/__init__.py +++ b/addons/rating/models/__init__.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from . import rating +from . import rating_data from . import rating_mixin from . import rating_parent_mixin from . import mail_thread diff --git a/addons/rating/models/rating.py b/addons/rating/models/rating.py index 78d412531b2..5cae372d02e 100644 --- a/addons/rating/models/rating.py +++ b/addons/rating/models/rating.py @@ -4,19 +4,9 @@ import base64 import uuid from odoo import api, fields, models - +from odoo.addons.rating.models import rating_data from odoo.modules.module import get_resource_path -RATING_LIMIT_SATISFIED = 4 -RATING_LIMIT_OK = 3 -RATING_LIMIT_MIN = 1 -RATING_TEXT = [ - ('top', 'Satisfied'), - ('ok', 'Okay'), - ('ko', 'Dissatisfied'), - ('none', 'No Rating yet'), -] - class Rating(models.Model): _name = "rating.rating" @@ -52,7 +42,7 @@ class Rating(models.Model): partner_id = fields.Many2one('res.partner', string='Customer', help="Author of the rating") rating = fields.Float(string="Rating Value", group_operator="avg", default=0, help="Rating value: 0=Unhappy, 5=Happy") rating_image = fields.Binary('Image', compute='_compute_rating_image') - rating_text = fields.Selection(RATING_TEXT, string='Rating', store=True, compute='_compute_rating_text', readonly=True) + rating_text = fields.Selection(rating_data.RATING_TEXT, string='Rating', store=True, compute='_compute_rating_text', readonly=True) feedback = fields.Text('Comment', help="Reason of the rating") message_id = fields.Many2one( 'mail.message', string="Message", @@ -99,15 +89,7 @@ class Rating(models.Model): def _get_rating_image_filename(self): self.ensure_one() - if self.rating >= RATING_LIMIT_SATISFIED: - rating_int = 5 - elif self.rating >= RATING_LIMIT_OK: - rating_int = 3 - elif self.rating >= RATING_LIMIT_MIN: - rating_int = 1 - else: - rating_int = 0 - return 'rating_%s.png' % rating_int + return 'rating_%s.png' % rating_data._rating_to_threshold(self.rating) def _compute_rating_image(self): for rating in self: @@ -120,14 +102,7 @@ class Rating(models.Model): @api.depends('rating') def _compute_rating_text(self): for rating in self: - if rating.rating >= RATING_LIMIT_SATISFIED: - rating.rating_text = 'top' - elif rating.rating >= RATING_LIMIT_OK: - rating.rating_text = 'ok' - elif rating.rating >= RATING_LIMIT_MIN: - rating.rating_text = 'ko' - else: - rating.rating_text = 'none' + rating.rating_text = rating_data._rating_to_text(rating.rating) @api.model_create_multi def create(self, vals_list): diff --git a/addons/rating/models/rating_data.py b/addons/rating/models/rating_data.py new file mode 100644 index 00000000000..62cd206887a --- /dev/null +++ b/addons/rating/models/rating_data.py @@ -0,0 +1,73 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import operator + +from odoo.tools.float_utils import float_compare + +RATING_AVG_TOP = 3.66 +RATING_AVG_OK = 2.33 +RATING_AVG_MIN = 1 + +RATING_LIMIT_SATISFIED = 4 +RATING_LIMIT_OK = 3 +RATING_LIMIT_MIN = 1 +RATING_TEXT = [ + ('top', 'Satisfied'), + ('ok', 'Okay'), + ('ko', 'Dissatisfied'), + ('none', 'No Rating yet'), +] + +OPERATOR_MAPPING = { + '=': operator.eq, + '!=': operator.ne, + '<': operator.lt, + '<=': operator.le, + '>': operator.gt, + '>=': operator.ge, +} + +def _rating_avg_to_text(rating_avg): + if float_compare(rating_avg, RATING_AVG_TOP, 2) >= 0: + return 'top' + if float_compare(rating_avg, RATING_AVG_OK, 2) >= 0: + return 'ok' + if float_compare(rating_avg, RATING_AVG_MIN, 2) >= 0: + return 'ko' + return 'none' + +def _rating_assert_value(rating_value): + assert 0 <= rating_value <= 5 + +def _rating_to_grade(rating_value): + """ From a rating value give a text-based mean value. """ + _rating_assert_value(rating_value) + if rating_value >= RATING_LIMIT_SATISFIED: + return 'great' + if rating_value >= RATING_LIMIT_OK: + return 'okay' + return 'bad' + +def _rating_to_text(rating_value): + """ From a rating value give a text-based mean value. """ + _rating_assert_value(rating_value) + if rating_value >= RATING_LIMIT_SATISFIED: + return 'top' + if rating_value >= RATING_LIMIT_OK: + return 'ok' + if rating_value >= RATING_LIMIT_MIN: + return 'ko' + return 'none' + +def _rating_to_threshold(rating_value): + """ From a rating value, return the thresholds in form of 0-1-3-5 used + notably for images. """ + _rating_assert_value(rating_value) + if rating_value >= RATING_LIMIT_SATISFIED: + return 5 + if rating_value >= RATING_LIMIT_OK: + return 3 + if rating_value >= RATING_LIMIT_MIN: + return 1 + return 0 diff --git a/addons/rating/models/rating_mixin.py b/addons/rating/models/rating_mixin.py index 4b6aad4668e..2b8ec44136d 100644 --- a/addons/rating/models/rating_mixin.py +++ b/addons/rating/models/rating_mixin.py @@ -4,22 +4,10 @@ import operator from odoo import api, fields, models, tools -from odoo.addons.rating.models.rating import RATING_LIMIT_SATISFIED, RATING_LIMIT_OK, RATING_LIMIT_MIN, RATING_TEXT +from odoo.addons.rating.models import rating_data from odoo.osv import expression from odoo.tools.float_utils import float_compare -OPERATOR_MAPPING = { - '=': operator.eq, - '!=': operator.ne, - '<': operator.lt, - '<=': operator.le, - '>': operator.gt, - '>=': operator.ge, -} -RATING_AVG_TOP = 3.66 -RATING_AVG_OK = 2.33 -RATING_AVG_MIN = RATING_LIMIT_MIN - class RatingMixin(models.AbstractModel): _name = 'rating.mixin' @@ -32,7 +20,7 @@ class RatingMixin(models.AbstractModel): rating_count = fields.Integer('Rating count', compute="_compute_rating_stats", compute_sudo=True) rating_avg = fields.Float("Average Rating", groups='base.group_user', compute='_compute_rating_stats', compute_sudo=True, search='_search_rating_avg') - rating_avg_text = fields.Selection(RATING_TEXT, groups='base.group_user', + rating_avg_text = fields.Selection(rating_data.RATING_TEXT, groups='base.group_user', compute='_compute_rating_avg_text', compute_sudo=True) 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") @@ -46,7 +34,7 @@ class RatingMixin(models.AbstractModel): @api.depends('rating_ids.res_id', 'rating_ids.rating') def _compute_rating_stats(self): """ Compute avg and count in one query, as thoses fields will be used together most of the time. """ - domain = expression.AND([self._rating_domain(), [('rating', '>=', RATING_LIMIT_MIN)]]) + domain = expression.AND([self._rating_domain(), [('rating', '>=', rating_data.RATING_LIMIT_MIN)]]) read_group_res = self.env['rating.rating'].read_group(domain, ['rating:avg'], groupby=['res_id'], lazy=False) # force average on rating column mapping = {item['res_id']: {'rating_count': item['__count'], 'rating_avg': item['rating']} for item in read_group_res} for record in self: @@ -54,48 +42,38 @@ class RatingMixin(models.AbstractModel): record.rating_avg = mapping.get(record.id, {}).get('rating_avg', 0) def _search_rating_avg(self, operator, value): - if operator not in OPERATOR_MAPPING: + if operator not in rating_data.OPERATOR_MAPPING: raise NotImplementedError('This operator %s is not supported in this search method.' % operator) rating_read_group = self.env['rating.rating'].sudo().read_group( - [('res_model', '=', self._name), ('consumed', '=', True), ('rating', '>=', RATING_LIMIT_MIN)], + [('res_model', '=', self._name), ('consumed', '=', True), ('rating', '>=', rating_data.RATING_LIMIT_MIN)], ['res_id', 'rating_avg:avg(rating)'], ['res_id']) res_ids = [ res['res_id'] for res in rating_read_group - if OPERATOR_MAPPING[operator](float_compare(res['rating_avg'], value, 2), 0) + if rating_data.OPERATOR_MAPPING[operator](float_compare(res['rating_avg'], value, 2), 0) ] return [('id', 'in', res_ids)] @api.depends('rating_avg') def _compute_rating_avg_text(self): for record in self: - if float_compare(record.rating_avg, RATING_AVG_TOP, 2) >= 0: - record.rating_avg_text = 'top' - elif float_compare(record.rating_avg, RATING_AVG_OK, 2) >= 0: - record.rating_avg_text = 'ok' - elif float_compare(record.rating_avg, RATING_AVG_MIN, 2) >= 0: - record.rating_avg_text = 'ko' - else: - record.rating_avg_text = 'none' + record.rating_avg_text = rating_data._rating_avg_to_text(record.rating_avg) @api.depends('rating_ids.res_id', 'rating_ids.rating') def _compute_rating_satisfaction(self): """ Compute the rating satisfaction percentage, this is done separately from rating_count and rating_avg since the query is different, to avoid computing if it is not necessary""" - domain = expression.AND([self._rating_domain(), [('rating', '>=', RATING_LIMIT_MIN)]]) + domain = expression.AND([self._rating_domain(), [('rating', '>=', rating_data.RATING_LIMIT_MIN)]]) # See `_compute_rating_percentage_satisfaction` above read_group_res = self.env['rating.rating'].read_group(domain, ['res_id', 'rating'], groupby=['res_id', 'rating'], lazy=False) default_grades = {'great': 0, 'okay': 0, 'bad': 0} grades_per_record = {record_id: default_grades.copy() for record_id in self.ids} + for group in read_group_res: record_id = group['res_id'] - rating = group['rating'] - if rating > RATING_LIMIT_OK: - grades_per_record[record_id]['great'] += group['__count'] - elif rating > RATING_LIMIT_MIN: - grades_per_record[record_id]['okay'] += group['__count'] - else: - grades_per_record[record_id]['bad'] += group['__count'] + grade = rating_data._rating_to_grade(group['rating']) + grades_per_record[record_id][grade] += group['__count'] + for record in self: grade_repartition = grades_per_record.get(record.id, default_grades) grade_count = sum(grade_repartition.values()) @@ -272,12 +250,8 @@ class RatingMixin(models.AbstractModel): data = self._rating_get_repartition(domain=domain) res = dict.fromkeys(['great', 'okay', 'bad'], 0) for key in data: - if key >= RATING_LIMIT_SATISFIED: - res['great'] += data[key] - elif key >= RATING_LIMIT_OK: - res['okay'] += data[key] - else: - res['bad'] += data[key] + grade = rating_data._rating_to_grade(key) + res[grade] += data[key] return res def rating_get_stats(self, domain=None): diff --git a/addons/rating/models/rating_parent_mixin.py b/addons/rating/models/rating_parent_mixin.py index 62c040c4ba1..ed8995613d8 100644 --- a/addons/rating/models/rating_parent_mixin.py +++ b/addons/rating/models/rating_parent_mixin.py @@ -1,14 +1,11 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import operator - from collections import defaultdict from datetime import timedelta from odoo import api, fields, models -from odoo.addons.rating.models.rating import RATING_LIMIT_OK, RATING_LIMIT_MIN -from odoo.addons.rating.models.rating_mixin import OPERATOR_MAPPING +from odoo.addons.rating.models import rating_data from odoo.osv import expression from odoo.tools.float_utils import float_compare @@ -36,7 +33,7 @@ class RatingParentMixin(models.AbstractModel): @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', '>=', RATING_LIMIT_MIN), ('consumed', '=', True)] + domain = [('parent_res_model', '=', self._name), ('parent_res_id', 'in', self.ids), ('rating', '>=', rating_data.RATING_LIMIT_MIN), ('consumed', '=', True)] if self._rating_satisfaction_days: domain += [('write_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) @@ -47,14 +44,9 @@ class RatingParentMixin(models.AbstractModel): rating_scores_per_parent = defaultdict(int) # contains the total of the rating values per record for item in data: parent_id = item['parent_res_id'] - rating = item['rating'] - if rating > RATING_LIMIT_OK: - grades_per_parent[parent_id]['great'] += item['__count'] - elif rating > RATING_LIMIT_MIN: - grades_per_parent[parent_id]['okay'] += item['__count'] - else: - grades_per_parent[parent_id]['bad'] += item['__count'] - rating_scores_per_parent[parent_id] += rating * item['__count'] + grade = rating_data._rating_to_grade(item['rating']) + grades_per_parent[parent_id][grade] += item['__count'] + rating_scores_per_parent[parent_id] += item['rating'] * item['__count'] # compute percentage per parent for record in self: @@ -66,9 +58,9 @@ class RatingParentMixin(models.AbstractModel): record.rating_avg_percentage = record.rating_avg / 5 def _search_rating_avg(self, operator, value): - if operator not in OPERATOR_MAPPING: + if operator not in rating_data.OPERATOR_MAPPING: raise NotImplementedError('This operator %s is not supported in this search method.' % operator) - domain = [('parent_res_model', '=', self._name), ('consumed', '=', True), ('rating', '>=', RATING_LIMIT_MIN)] + domain = [('parent_res_model', '=', self._name), ('consumed', '=', True), ('rating', '>=', rating_data.RATING_LIMIT_MIN)] if self._rating_satisfaction_days: min_date = fields.datetime.now() - timedelta(days=self._rating_satisfaction_days) domain = expression.AND([domain, [('write_date', '>=', fields.Datetime.to_string(min_date))]]) @@ -76,6 +68,6 @@ class RatingParentMixin(models.AbstractModel): parent_res_ids = [ res['parent_res_id'] for res in rating_read_group - if OPERATOR_MAPPING[operator](float_compare(res['rating_avg'], value, 2), 0) + if rating_data.OPERATOR_MAPPING[operator](float_compare(res['rating_avg'], value, 2), 0) ] return [('id', 'in', parent_res_ids)] From 6e12ba51bb1635fca53377bd9b808d869d0e9b6e Mon Sep 17 00:00:00 2001 From: Noe Antoine Date: Fri, 14 Jan 2022 14:18:37 +0000 Subject: [PATCH 3/7] [IMP] portal_rating: remove "Published on" when updating or creating comment When answering to a comment, or editing such an answer, the form shows a "Published on" which is supposed to be followed by the date the comment has been published. However, since when using the form we are editing or creating an answer comment, it remains empty and useless. Therefore, we remove it. It is still on comments once set, but not while editing them. Task-2728564 Part-of: odoo/odoo#82792 --- addons/portal_rating/static/src/xml/portal_chatter.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/addons/portal_rating/static/src/xml/portal_chatter.xml b/addons/portal_rating/static/src/xml/portal_chatter.xml index 661bb0a472d..40f10c0fc5b 100644 --- a/addons/portal_rating/static/src/xml/portal_chatter.xml +++ b/addons/portal_rating/static/src/xml/portal_chatter.xml @@ -94,7 +94,6 @@
-

Published on

From 9355631e89e76cf0d3be5aa2e7b2dfaf1e4014d2 Mon Sep 17 00:00:00 2001 From: Noe Antoine Date: Fri, 28 Jan 2022 09:32:13 +0000 Subject: [PATCH 4/7] [FIX][IMP] portal_rating: round decimal ratings to two decimals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When hovering the number of stars of a course in front-end, for instance, one can see "4,16666666666667 stars on 5". This is not very convenient. The number is rounded up to two decimals instead. -> "4,17 stars on 5". Same fix is done in backend in slides-specific kanban view of ratings. Value is now rounded. Finally _rating_get_repartition is fixed round values to the nearest half value. We generally receive integer values between 0 and 5 but other values may exist. Getting a repartition rounded at nearest half integer is sufficient notably when looking at star-based display which covers only complete or half complete stars. Task-2728564 Part-of: odoo/odoo#82792 Co-authored-by: Noé Antoine Co-authored-by: Thibault Delavallée --- addons/portal_rating/static/src/js/portal_chatter.js | 5 +++-- addons/portal_rating/static/src/xml/portal_tools.xml | 5 ++++- addons/rating/models/rating_mixin.py | 10 ++++++---- addons/website_slides/views/rating_rating_views.xml | 2 +- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/addons/portal_rating/static/src/js/portal_chatter.js b/addons/portal_rating/static/src/js/portal_chatter.js index 450b52be20a..d9107a2b24c 100644 --- a/addons/portal_rating/static/src/js/portal_chatter.js +++ b/addons/portal_rating/static/src/js/portal_chatter.js @@ -138,13 +138,14 @@ PortalChatter.include({ if (!result['rating_stats']) { return; } + const self = this; const ratingData = { 'avg': Math.round(result['rating_stats']['avg'] * 100) / 100, 'percent': [], }; - _.each(_.keys(result['rating_stats']['percent']).reverse(), function (rating) { + _.each(_.sortBy(_.keys(result['rating_stats']['percent'])).reverse(), function (rating) { ratingData['percent'].push({ - 'num': rating, + 'num': self.roundToHalf(rating), 'percent': utils.round_precision(result['rating_stats']['percent'][rating], 0.01), }); }); diff --git a/addons/portal_rating/static/src/xml/portal_tools.xml b/addons/portal_rating/static/src/xml/portal_tools.xml index c52e4ab33c6..1b38cadf85c 100644 --- a/addons/portal_rating/static/src/xml/portal_tools.xml +++ b/addons/portal_rating/static/src/xml/portal_tools.xml @@ -4,7 +4,10 @@ -
+
diff --git a/addons/rating/models/rating_mixin.py b/addons/rating/models/rating_mixin.py index 2b8ec44136d..3018e28762b 100644 --- a/addons/rating/models/rating_mixin.py +++ b/addons/rating/models/rating_mixin.py @@ -6,7 +6,7 @@ import operator from odoo import api, fields, models, tools from odoo.addons.rating.models import rating_data from odoo.osv import expression -from odoo.tools.float_utils import float_compare +from odoo.tools.float_utils import float_compare, float_round class RatingMixin(models.AbstractModel): @@ -224,17 +224,19 @@ class RatingMixin(models.AbstractModel): base_domain = expression.AND([self._rating_domain(), [('rating', '>=', 1)]]) if domain: base_domain += domain - data = self.env['rating.rating'].read_group(base_domain, ['rating'], ['rating', 'res_id']) + rg_data = self.env['rating.rating'].read_group(base_domain, ['rating'], ['rating', 'res_id']) # init dict with all posible rate value, except 0 (no value for the rating) values = dict.fromkeys(range(1, 6), 0) - values.update((d['rating'], d['rating_count']) for d in data) + for rating_rg in rg_data: + rating_val_round = float_round(rating_rg['rating'], precision_digits=1) + values[rating_val_round] = values.get(rating_val_round, 0) + rating_rg['rating_count'] # add other stats if add_stats: rating_number = sum(values.values()) result = { 'repartition': values, 'avg': sum(float(key * values[key]) for key in values) / rating_number if rating_number > 0 else 0, - 'total': sum(it['rating_count'] for it in data), + 'total': sum(it['rating_count'] for it in rg_data), } return result return values diff --git a/addons/website_slides/views/rating_rating_views.xml b/addons/website_slides/views/rating_rating_views.xml index 247e0eed7c9..e94addc7d59 100644 --- a/addons/website_slides/views/rating_rating_views.xml +++ b/addons/website_slides/views/rating_rating_views.xml @@ -12,7 +12,7 @@ - + From aead9a884e9007e8e19a7471608a5a3417fb3d52 Mon Sep 17 00:00:00 2001 From: Noe Antoine Date: Fri, 14 Jan 2022 10:33:33 +0000 Subject: [PATCH 5/7] [IMP] website_slides: allow empty messages when reviewing courses In order to reduce friction and ease the rating process for attendees, we do not force the constraint stating that the message must contain a message or an attachment onto the users. Instead, we allow ratings and replace the empty message by a blank space to avoid triggering the error message. If no stars are selected, an error message is now displayed to the user, asking to select a rating before submission, preventing them to post their review until then. Since we do not really support 0 stars ratings, and do not want the user to see that error message in case of the course, we set the default rating to 4.0 instead of 0.0. -> Therefore, we also remove the grey star contrast coloring on first review since we do not want the user to have the impression a choice has already be made beforehand. In that case, everything will be yellow. Void content detection is done by extracting it in both python (portal chatter post controller) and frontend (submission check) and relaxing it in slides modules. Task-2728564 Part-of: odoo/odoo#82792 --- addons/portal/controllers/mail.py | 58 +++++++++++-------- .../portal/static/src/js/portal_composer.js | 13 ++++- .../static/src/js/portal_composer.js | 21 ++++++- .../portal_rating/views/rating_templates.xml | 1 + addons/website_slides/controllers/mail.py | 9 ++- .../views/website_slides_templates_course.xml | 1 + 6 files changed, 74 insertions(+), 29 deletions(-) diff --git a/addons/portal/controllers/mail.py b/addons/portal/controllers/mail.py index e2bb31c961b..44bc004cd34 100644 --- a/addons/portal/controllers/mail.py +++ b/addons/portal/controllers/mail.py @@ -112,6 +112,10 @@ class PortalChatter(http.Controller): except (AccessError, MissingError): raise UserError(_("The attachment %s does not exist or you do not have the rights to access it.", attachment_id)) + def _portal_post_has_content(self, res_model, res_id, message, attachment_ids=None, **kw): + """ Tells if we can effectively post on the model based on content. """ + return bool(message) or bool(attachment_ids) + @http.route(['/mail/chatter_post'], type='json', methods=['POST'], auth='public', website=True) def portal_chatter_post(self, res_model, res_id, message, attachment_ids=None, attachment_tokens=None, **kw): """Create a new `mail.message` with the given `message` and/or `attachment_ids` and return new message values. @@ -120,38 +124,42 @@ class PortalChatter(http.Controller): `res_model`. The user must have access rights on this target document or must provide valid identifiers through `kw`. See `_message_post_helper`. """ + if not self._portal_post_has_content(res_model, res_id, message, + attachment_ids=attachment_ids, attachment_tokens=attachment_tokens, + **kw): + return + res_id = int(res_id) self._portal_post_check_attachments(attachment_ids, attachment_tokens) - if message or attachment_ids: - result = {'default_message': message} - # message is received in plaintext and saved in html - if message: - message = plaintext2html(message) - post_values = { - 'res_model': res_model, - 'res_id': res_id, - 'message': message, - 'send_after_commit': False, - 'attachment_ids': False, # will be added afterward - } - post_values.update((fname, kw.get(fname)) for fname in self._portal_post_filter_params()) - message = _message_post_helper(**post_values) - result.update({'default_message_id': message.id}) + result = {'default_message': message} + # message is received in plaintext and saved in html + if message: + message = plaintext2html(message) + post_values = { + 'res_model': res_model, + 'res_id': res_id, + 'message': message, + 'send_after_commit': False, + 'attachment_ids': False, # will be added afterward + } + post_values.update((fname, kw.get(fname)) for fname in self._portal_post_filter_params()) + message = _message_post_helper(**post_values) + result.update({'default_message_id': message.id}) - if attachment_ids: - # sudo write the attachment to bypass the read access - # verification in mail message - record = request.env[res_model].browse(res_id) - message_values = {'res_id': res_id, 'model': res_model} - attachments = record._message_post_process_attachments([], attachment_ids, message_values) + if attachment_ids: + # sudo write the attachment to bypass the read access + # verification in mail message + record = request.env[res_model].browse(res_id) + message_values = {'res_id': res_id, 'model': res_model} + attachments = record._message_post_process_attachments([], attachment_ids, message_values) - if attachments.get('attachment_ids'): - message.sudo().write(attachments) + if attachments.get('attachment_ids'): + message.sudo().write(attachments) - result.update({'default_attachment_ids': message.attachment_ids.sudo().read(['id', 'name', 'mimetype', 'file_size', 'access_token'])}) - return result + result.update({'default_attachment_ids': message.attachment_ids.sudo().read(['id', 'name', 'mimetype', 'file_size', 'access_token'])}) + return result @http.route('/mail/chatter_init', type='json', auth='public', website=True) def portal_chatter_init(self, res_model, res_id, domain=False, limit=False, **kwargs): diff --git a/addons/portal/static/src/js/portal_composer.js b/addons/portal/static/src/js/portal_composer.js index 645210c0116..1622f34bda0 100644 --- a/addons/portal/static/src/js/portal_composer.js +++ b/addons/portal/static/src/js/portal_composer.js @@ -158,9 +158,9 @@ var PortalComposer = publicWidget.Widget.extend({ */ _onSubmitButtonClick: function (ev) { ev.preventDefault(); - if (!this.$inputTextarea.val().trim() && !this.attachments.length) { + const error = this._onSubmitCheckContent(); + if (error) { this.$inputTextarea.addClass('border-danger'); - const error = _t('Some fields are required. Please make sure to write a message or attach a document'); this.$(".o_portal_chatter_composer_error").text(error).removeClass('d-none'); return Promise.reject(); } else { @@ -168,6 +168,15 @@ var PortalComposer = publicWidget.Widget.extend({ } }, + /** + * @private + */ + _onSubmitCheckContent: function () { + if (!this.$inputTextarea.val().trim() && !this.attachments.length) { + return _t('Some fields are required. Please make sure to write a message or attach a document'); + }; + }, + //-------------------------------------------------------------------------- // Private //-------------------------------------------------------------------------- diff --git a/addons/portal_rating/static/src/js/portal_composer.js b/addons/portal_rating/static/src/js/portal_composer.js index 3d1bb0ea7fc..7833de0b86c 100644 --- a/addons/portal_rating/static/src/js/portal_composer.js +++ b/addons/portal_rating/static/src/js/portal_composer.js @@ -34,9 +34,10 @@ PortalComposer.include({ // default options this.options = _.defaults(this.options, { + 'rate_with_void_content': false, 'default_message': false, 'default_message_id': false, - 'default_rating_value': 0.0, + 'default_rating_value': 4.0, 'force_submit_url': false, }); // star input widget @@ -61,6 +62,10 @@ PortalComposer.include({ // rating stars self.$input = self.$('input[name="rating_value"]'); self.$star_list = self.$('.stars').find('i'); + // if this is the first review, we do not use grey color contrast, even with default rating value. + if (!self.options.default_message_id) { + self.$star_list.removeClass('text-black-25'); + } // set the default value to trigger the display of star widget and update the hidden input value. self.set("star_value", self.options.default_rating_value); @@ -149,5 +154,19 @@ PortalComposer.include({ $modal.modal('hide'); }); }, + + /** + * @override + * @private + */ + _onSubmitCheckContent: function (ev) { + if (this.options.rate_with_void_content) { + if (this.$input.val() === 0) { + return _t('The rating is required. Please make sure to select one before sending your review.') + } + return false; + } + return this._super.apply(this, arguments); + }, }); }); diff --git a/addons/portal_rating/views/rating_templates.xml b/addons/portal_rating/views/rating_templates.xml index 2fcc31e5894..d5f6b9b7451 100644 --- a/addons/portal_rating/views/rating_templates.xml +++ b/addons/portal_rating/views/rating_templates.xml @@ -67,6 +67,7 @@ t-att-data-default_rating_value="default_rating_value" t-att-data-default_attachment_ids="default_attachment_ids" t-att-data-force_submit_url="force_submit_url" + t-att-data-rate_with_void_content="rate_with_void_content" t-att-data-disable_composer="disable_composer" t-att-data-display_composer="display_composer" t-att-data-link_btn_classes="_link_btn_classes" diff --git a/addons/website_slides/controllers/mail.py b/addons/website_slides/controllers/mail.py index 8c72322e290..7c52fa46b35 100644 --- a/addons/website_slides/controllers/mail.py +++ b/addons/website_slides/controllers/mail.py @@ -13,10 +13,17 @@ from odoo.tools import plaintext2html, html2plaintext class SlidesPortalChatter(PortalChatter): + def _portal_post_has_content(self, res_model, res_id, message, attachment_ids=None, **kw): + """ Relax constraint on slide model: having a rating value is sufficient + to consider we have a content. """ + if res_model == 'slide.channel' and kw.get('rating_value'): + return True + return super()._portal_post_has_content(res_model, res_id, message, attachment_ids=attachment_ids, **kw) + @http.route(['/mail/chatter_post'], type='json', methods=['POST'], auth='public', website=True) def portal_chatter_post(self, res_model, res_id, message, **kw): result = super(SlidesPortalChatter, self).portal_chatter_post(res_model, res_id, message, **kw) - if res_model == 'slide.channel': + if result and res_model == 'slide.channel': rating_value = kw.get('rating_value', False) slide_channel = request.env[res_model].sudo().browse(int(res_id)) if rating_value and slide_channel and request.env.user.partner_id.id == int(kw.get('pid')): diff --git a/addons/website_slides/views/website_slides_templates_course.xml b/addons/website_slides/views/website_slides_templates_course.xml index e753b91f439..b5e5f72bce6 100644 --- a/addons/website_slides/views/website_slides_templates_course.xml +++ b/addons/website_slides/views/website_slides_templates_course.xml @@ -153,6 +153,7 @@ + From 4ddeca9ad05f60761ee61f96cf274d5d07520e13 Mon Sep 17 00:00:00 2001 From: Noe Antoine Date: Fri, 28 Jan 2022 13:08:53 +0000 Subject: [PATCH 6/7] [IMP] portal_rating: remove space between stars in rating star input Before, the inline-block style created some small unbreakable whitespace between the stars in the review/rating composer. It meant that when the user hovered their mouse in that space, they left the star element, but not the stars area, hence displaying the message of the default (current) rating instead of the one that corresponds to the closest star. It is repaired using an inline-flex for the stars area. The stars are also centered in their element, with a small horizontal padding. Task-2728564 Part-of: odoo/odoo#82792 --- addons/portal_rating/static/src/scss/portal_rating.scss | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/addons/portal_rating/static/src/scss/portal_rating.scss b/addons/portal_rating/static/src/scss/portal_rating.scss index 6d02b1cafa9..82c35d07d71 100644 --- a/addons/portal_rating/static/src/scss/portal_rating.scss +++ b/addons/portal_rating/static/src/scss/portal_rating.scss @@ -51,13 +51,14 @@ $o-w-rating-star-color: #FACC2E; .o_rating_star_card{ margin-bottom: 5px; .stars { - display: inline-block; + display: inline-flex; color: #FACC2E; margin-right: 15px; } .stars i { - margin-right: -3px; + padding-right: 1px; + padding-left: 1px; text-align: center; } From db82f55330894f3353dfceb98940a9ad6da15c93 Mon Sep 17 00:00:00 2001 From: Noe Antoine Date: Wed, 2 Feb 2022 14:47:03 +0000 Subject: [PATCH 7/7] [IMP] portal_rating: clean attributes of star element in rating composer When hovering a star, a title is disturbing since a description is already displayed to the user. "One star" is therefore confusing since it will appear on every star, even for the fourth one out of five, for instance. Therefore, all titles are removed from stars. Also, to avoid this confusion in aria label, we do not use numbers in cleaned labels. Task-2728564 Part-of: odoo/odoo#82792 --- addons/portal_rating/static/src/xml/portal_tools.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/addons/portal_rating/static/src/xml/portal_tools.xml b/addons/portal_rating/static/src/xml/portal_tools.xml index 1b38cadf85c..f14a6b46968 100644 --- a/addons/portal_rating/static/src/xml/portal_tools.xml +++ b/addons/portal_rating/static/src/xml/portal_tools.xml @@ -74,13 +74,13 @@
- + - + - - + +