[FIX] gamification: perf on write karma_min on gamification.karma.rank
This commits avoid to call recompute when you write on the rank without touch the karma_min value. On write on karma_min, we have 2 cases: - Rank order are the same, just need to recompute the rank for the user that could be impacted. Eg. if karma_min changes from 45 to 50, we need to recheck for all user between 45 and 50 their new rank. - Rank order are not the same, we need to recompute for all users. (could be still improved if needed) When you recompute the rank on a list of users, depending of the number of users (bulk if more than 3 * number of rank) we check for each user what is the new rank, or for each rank whats is the new users. task-internal No way to edit a rank on the prod server. Lock sql -> crash server closes odoo/odoo#34835 Signed-off-by: Jérémy Kersten (jke) <jke@openerp.com>
This commit is contained in:
@@ -47,8 +47,19 @@ class KarmaRank(models.Model):
|
||||
|
||||
@api.multi
|
||||
def write(self, vals):
|
||||
if 'karma_min' in vals:
|
||||
previous_ranks = self.env['gamification.karma.rank'].search([], order="karma_min DESC").ids
|
||||
low = min(vals['karma_min'], self.karma_min)
|
||||
high = max(vals['karma_min'], self.karma_min)
|
||||
|
||||
tools.image_resize_images(vals)
|
||||
res = super(KarmaRank, self).write(vals)
|
||||
users = self.env['res.users'].sudo().search([('karma', '>', 0)])
|
||||
users._recompute_rank()
|
||||
|
||||
if 'karma_min' in vals:
|
||||
after_ranks = self.env['gamification.karma.rank'].search([], order="karma_min DESC").ids
|
||||
if previous_ranks != after_ranks:
|
||||
users = self.env['res.users'].sudo().search([('karma', '>', 0)])
|
||||
else:
|
||||
users = self.env['res.users'].sudo().search([('karma', '>=', low), ('karma', '<=', high)])
|
||||
users._recompute_rank()
|
||||
return res
|
||||
|
||||
@@ -7,9 +7,11 @@ import logging
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class Users(models.Model):
|
||||
_inherit = 'res.users'
|
||||
|
||||
|
||||
karma = fields.Integer('Karma', default=0)
|
||||
karma_position = fields.Integer('Karma position', compute="_compute_karma_position", store=False)
|
||||
badge_ids = fields.One2many('gamification.badge.user', 'user_id', string='Badges', copy=False)
|
||||
@@ -69,18 +71,32 @@ class Users(models.Model):
|
||||
return True
|
||||
|
||||
def _rank_changed(self):
|
||||
if self.rank_id.karma_min > 0:
|
||||
template = self.env.ref('gamification.mail_template_data_new_rank_reached', raise_if_not_found=False)
|
||||
if template:
|
||||
template.send_mail(self.id, force_send=True, notif_layout='mail.mail_notification_light')
|
||||
"""
|
||||
Method that can be called on a batch of users with the same new rank
|
||||
"""
|
||||
template = self.env.ref('gamification.mail_template_data_new_rank_reached', raise_if_not_found=False)
|
||||
if template:
|
||||
for u in self:
|
||||
if u.rank_id.karma_min > 0:
|
||||
template.send_mail(u.id, force_send=len(self) == 1, notif_layout='mail.mail_notification_light')
|
||||
|
||||
def _recompute_rank(self):
|
||||
"""
|
||||
The caller should filter the users on karma > 0 before calling this method
|
||||
to avoid looping on every single users
|
||||
|
||||
Compute rank of each user by user.
|
||||
For each user, check the rank of this user
|
||||
"""
|
||||
|
||||
ranks = [{'rank': rank, 'karma_min': rank.karma_min} for rank in
|
||||
self.env['gamification.karma.rank'].search([], order="karma_min DESC")]
|
||||
|
||||
# 3 is the number of search/requests used by rank in _recompute_rank_bulk()
|
||||
if len(self) > len(ranks) * 3:
|
||||
self._recompute_rank_bulk()
|
||||
return
|
||||
|
||||
for user in self:
|
||||
old_rank = user.rank_id
|
||||
if user.karma == 0 and ranks:
|
||||
@@ -96,6 +112,66 @@ class Users(models.Model):
|
||||
if old_rank != user.rank_id:
|
||||
user._rank_changed()
|
||||
|
||||
def _recompute_rank_bulk(self):
|
||||
"""
|
||||
Compute rank of each user by rank.
|
||||
For each rank, check which users need to be ranked
|
||||
|
||||
"""
|
||||
ranks = [{'rank': rank, 'karma_min': rank.karma_min} for rank in
|
||||
self.env['gamification.karma.rank'].search([], order="karma_min DESC")]
|
||||
|
||||
users_todo = self
|
||||
|
||||
next_rank_id = False
|
||||
# wtf, next_rank_id should be a related on rank_id.next_rank_id and life might get easier.
|
||||
# And we only need to recompute next_rank_id on write with min_karma or in the create on rank model.
|
||||
for r in ranks:
|
||||
rank_id = r['rank'].id
|
||||
dom = [
|
||||
('karma', '>=', r['karma_min']),
|
||||
('id', 'in', users_todo.ids),
|
||||
'|', # noqa
|
||||
'|', ('rank_id', '!=', rank_id), ('rank_id', '=', False),
|
||||
'|', ('next_rank_id', '!=', next_rank_id), ('next_rank_id', '=', False if next_rank_id else -1),
|
||||
]
|
||||
users = self.env['res.users'].search(dom)
|
||||
if users:
|
||||
users_to_notify = self.env['res.users'].search([
|
||||
('karma', '>=', r['karma_min']),
|
||||
'|', ('rank_id', '!=', rank_id), ('rank_id', '=', False),
|
||||
('id', 'in', users.ids),
|
||||
])
|
||||
users.write({
|
||||
'rank_id': rank_id,
|
||||
'next_rank_id': next_rank_id,
|
||||
})
|
||||
users_to_notify._rank_changed()
|
||||
users_todo -= users
|
||||
|
||||
nothing_to_do_users = self.env['res.users'].search([
|
||||
('karma', '>=', r['karma_min']),
|
||||
'|', ('rank_id', '=', rank_id), ('next_rank_id', '=', next_rank_id),
|
||||
('id', 'in', users_todo.ids),
|
||||
])
|
||||
users_todo -= nothing_to_do_users
|
||||
next_rank_id = r['rank'].id
|
||||
|
||||
if ranks:
|
||||
lower_rank = ranks[-1]['rank']
|
||||
users = self.env['res.users'].search([
|
||||
('karma', '>=', 0),
|
||||
('karma', '<', lower_rank.karma_min),
|
||||
'|', ('rank_id', '!=', False), ('next_rank_id', '!=', lower_rank.id),
|
||||
('id', 'in', users_todo.ids),
|
||||
])
|
||||
if users:
|
||||
users.write({
|
||||
'rank_id': False,
|
||||
'next_rank_id': lower_rank.id,
|
||||
})
|
||||
|
||||
|
||||
def _get_next_rank(self):
|
||||
""" For fresh users with 0 karma that don't have a rank_id and next_rank_id yet
|
||||
this method returns the first karma rank (by karma ascending). This acts as a
|
||||
|
||||
@@ -2,3 +2,4 @@
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from . import test_challenge
|
||||
from . import test_recompute_rank
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from odoo.tests import common
|
||||
from unittest.mock import patch
|
||||
|
||||
|
||||
class TestComputeRankCommon(common.TransactionCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestComputeRankCommon, self).setUp()
|
||||
|
||||
def _patched_send_mail(*args, **kwargs):
|
||||
pass
|
||||
|
||||
self.User = self.env['res.users'].with_context(no_reset_password=True, mail_create_nosubscribe=True)
|
||||
self.users = self.User
|
||||
|
||||
patch_email = patch('odoo.addons.mail.models.mail_template.MailTemplate.send_mail', _patched_send_mail)
|
||||
patch_email.start()
|
||||
for k in range(-5, 1030, 30):
|
||||
self.users += self.User.create({
|
||||
'name': str(k),
|
||||
'login': "test_recompute_rank_%s" % k,
|
||||
'karma': k,
|
||||
})
|
||||
|
||||
self.env['gamification.karma.rank'].search([]).unlink()
|
||||
|
||||
self.rank_1 = self.env['gamification.karma.rank'].create({
|
||||
'name': 'rank 1',
|
||||
'karma_min': 0,
|
||||
})
|
||||
|
||||
self.rank_2 = self.env['gamification.karma.rank'].create({
|
||||
'name': 'rank 2',
|
||||
'karma_min': 250,
|
||||
})
|
||||
|
||||
self.rank_3 = self.env['gamification.karma.rank'].create({
|
||||
'name': 'rank 3',
|
||||
'karma_min': 500,
|
||||
})
|
||||
self.rank_4 = self.env['gamification.karma.rank'].create({
|
||||
'name': 'rank 4',
|
||||
'karma_min': 1000,
|
||||
})
|
||||
|
||||
patch_email.stop()
|
||||
|
||||
|
||||
class test_recompute(TestComputeRankCommon):
|
||||
|
||||
def test_00_initial_compute(self):
|
||||
|
||||
self.assertEqual(len(self.users), 35)
|
||||
|
||||
self.assertEqual(
|
||||
len(self.rank_1.user_ids & self.users),
|
||||
len([u for u in self.users if u.karma >= self.rank_1.karma_min and u.karma < self.rank_2.karma_min])
|
||||
)
|
||||
self.assertEqual(
|
||||
len(self.rank_2.user_ids & self.users),
|
||||
len([u for u in self.users if u.karma >= self.rank_2.karma_min and u.karma < self.rank_3.karma_min])
|
||||
)
|
||||
self.assertEqual(
|
||||
len(self.rank_3.user_ids & self.users),
|
||||
len([u for u in self.users if u.karma >= self.rank_3.karma_min and u.karma < self.rank_4.karma_min])
|
||||
)
|
||||
self.assertEqual(
|
||||
len(self.rank_4.user_ids & self.users),
|
||||
len([u for u in self.users if u.karma >= self.rank_4.karma_min])
|
||||
)
|
||||
|
||||
def test_01_switch_rank(self):
|
||||
|
||||
self.assertEqual(len(self.users), 35)
|
||||
|
||||
self.rank_3.karma_min = 100
|
||||
# rank_1 -> rank_3 -> rank_2 -> rank_4
|
||||
|
||||
self.assertEqual(
|
||||
len(self.rank_1.user_ids & self.users),
|
||||
len([u for u in self.users if u.karma >= self.rank_1.karma_min and u.karma < self.rank_3.karma_min])
|
||||
)
|
||||
self.assertEqual(
|
||||
len(self.rank_3.user_ids & self.users),
|
||||
len([u for u in self.users if u.karma >= self.rank_3.karma_min and u.karma < self.rank_2.karma_min])
|
||||
)
|
||||
self.assertEqual(
|
||||
len(self.rank_2.user_ids & self.users),
|
||||
len([u for u in self.users if u.karma >= self.rank_2.karma_min and u.karma < self.rank_4.karma_min])
|
||||
)
|
||||
self.assertEqual(
|
||||
len(self.rank_4.user_ids & self.users),
|
||||
len([u for u in self.users if u.karma >= self.rank_4.karma_min])
|
||||
)
|
||||
|
||||
def test_02_update_rank_without_switch(self):
|
||||
number_of_users = False
|
||||
|
||||
def _patched_recompute_rank(_self, *args, **kwargs):
|
||||
nonlocal number_of_users
|
||||
number_of_users = len(_self & self.users)
|
||||
|
||||
patch_bulk = patch('odoo.addons.gamification.models.res_users.Users._recompute_rank', _patched_recompute_rank)
|
||||
patch_bulk.start()
|
||||
self.rank_3.karma_min = 700
|
||||
self.assertEqual(number_of_users, 7, "Should just recompute for the 7 users between 500 and 700")
|
||||
patch_bulk.stop()
|
||||
|
||||
def test_03_test_bulk_call(self):
|
||||
self.assertEqual(len(self.users), 35)
|
||||
|
||||
def _patched_check_in_bulk(*args, **kwargs):
|
||||
raise
|
||||
|
||||
patch_bulk = patch('odoo.addons.gamification.models.res_users.Users._recompute_rank_bulk', _patched_check_in_bulk)
|
||||
patch_bulk.start()
|
||||
|
||||
# call on 5 users should not trigger the bulk function
|
||||
self.users[0:5]._recompute_rank()
|
||||
|
||||
# call on 50 users should trigger the bulk function
|
||||
with self.assertRaises(Exception):
|
||||
self.users[0:50]._recompute_rank()
|
||||
|
||||
patch_bulk.stop()
|
||||
Reference in New Issue
Block a user