From bde55baaa92da0565f654b99dd614ead7faea1c7 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 6 May 2014 11:37:16 +0200 Subject: [PATCH 1/8] [IMP] gamification: performance improvement on the goal computation Removing the inheritance to mail.thread will avoid posting message at creation and other tracking mechanism at each write. Updating a challenge with a large number of users will be significantly faster. bzr revid: mat@openerp.com-20140506093716-xvpe1n2ase2zix0v --- addons/gamification/models/challenge.py | 11 +++++++---- addons/gamification/models/goal.py | 4 +--- addons/gamification/views/goal.xml | 4 ---- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index ecb4c495d94..a6c467f0177 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -355,10 +355,13 @@ class gamification_challenge(osv.Model): if write_op: self.write(cr, uid, [challenge.id], {'user_ids': write_op}, context=context) - if to_remove_ids: - self.message_unsubscribe_users(cr, uid, [challenge.id], to_remove_ids, context=None) - if to_add_ids: - self.message_subscribe_users(cr, uid, [challenge.id], to_add_ids, context=context) + if challenge.report_message_frequency != 'never': + if to_remove_ids: + self.message_unsubscribe_users(cr, uid, [challenge.id], to_remove_ids, context=context) + if to_add_ids: + self.message_subscribe_users(cr, uid, [challenge.id], to_add_ids, context=context) + else: + self.message_unsubscribe_users(cr, uid, [challenge.id], old_user_ids, context=context) return True diff --git a/addons/gamification/models/goal.py b/addons/gamification/models/goal.py index af9a763236d..7fcf8e5f981 100644 --- a/addons/gamification/models/goal.py +++ b/addons/gamification/models/goal.py @@ -138,7 +138,6 @@ class gamification_goal(osv.Model): _name = 'gamification.goal' _description = 'Gamification goal instance' - _inherit = 'mail.thread' def _get_completion(self, cr, uid, ids, field_name, arg, context=None): """Return the percentage of completeness of the goal, between 0 and 100""" @@ -226,8 +225,7 @@ class gamification_goal(osv.Model): temp_obj = self.pool.get('email.template') template_id = self.pool['ir.model.data'].get_object(cr, uid, 'gamification', 'email_template_goal_reminder', context) body_html = temp_obj.render_template(cr, uid, template_id.body_html, 'gamification.goal', goal.id, context=context) - - self.message_post(cr, uid, goal.id, body=body_html, partner_ids=[goal.user_id.partner_id.id], context=context, subtype='mail.mt_comment') + self.pool['mail.thread'].message_post(cr, uid, 0, body=body_html, partner_ids=[goal.user_id.partner_id.id], context=context, subtype='mail.mt_comment') return {'to_update': True} return {} diff --git a/addons/gamification/views/goal.xml b/addons/gamification/views/goal.xml index 528d5db0511..884467404ce 100644 --- a/addons/gamification/views/goal.xml +++ b/addons/gamification/views/goal.xml @@ -86,10 +86,6 @@ -
- - -
From ccf4d142b0674dc6894eff5c9e1c4ac77d78d18d Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 6 May 2014 12:29:42 +0200 Subject: [PATCH 2/8] [FIX] gamification: avoid recomputing every goal for every challenge bzr revid: mat@openerp.com-20140506102942-obopc9j5a2136cxh --- addons/gamification/models/challenge.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index a6c467f0177..f4704f2de29 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -396,9 +396,9 @@ class gamification_challenge(osv.Model): :param list(int) ids: the list of challenge concerned""" goal_obj = self.pool.get('gamification.goal') - to_update = [] for challenge in self.browse(cr, uid, ids, context=context): (start_date, end_date) = start_end_date_for_period(challenge.period) + to_update = [] # if no periodicity, use challenge dates if not start_date and challenge.start_date: From 697dcae728c045ae0cbfb9d48821a8417856e25c Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 6 May 2014 13:21:09 +0200 Subject: [PATCH 3/8] [IMP] gamification: move write in goal update inside the first loop to be able to group the computation by definition (and potentially apply intermediate commits) bzr revid: mat@openerp.com-20140506112109-z4rsx5umet5x6n26 --- addons/gamification/models/goal.py | 31 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/addons/gamification/models/goal.py b/addons/gamification/models/goal.py index 7fcf8e5f981..2cbb8869eb7 100644 --- a/addons/gamification/models/goal.py +++ b/addons/gamification/models/goal.py @@ -241,7 +241,6 @@ class gamification_goal(osv.Model): context = {} goals_by_definition = {} - goals_to_write = {} all_goals = {} for goal in self.browse(cr, uid, ids, context=context): if goal.state in ('draft', 'canceled'): @@ -249,10 +248,10 @@ class gamification_goal(osv.Model): continue goals_by_definition.setdefault(goal.definition_id, []).append(goal) - goals_to_write[goal.id] = {} all_goals[goal.id] = goal for definition, goals in goals_by_definition.items(): + goals_to_write = dict((goal.id, {}) for goal in goals) if definition.computation_mode == 'manually': for goal in goals: goals_to_write[goal.id].update(self._check_remind_delay(cr, uid, goal, context)) @@ -343,22 +342,22 @@ class gamification_goal(osv.Model): if new_value != goal.current: goals_to_write[goal.id]['current'] = new_value - for goal_id, value in goals_to_write.items(): - if not value: - continue - goal = all_goals[goal_id] + for goal_id, value in goals_to_write.items(): + if not value: + continue + goal = all_goals[goal_id] - # check goal target reached - if (goal.definition_condition == 'higher' and value.get('current', goal.current) >= goal.target_goal) \ - or (goal.definition_condition == 'lower' and value.get('current', goal.current) <= goal.target_goal): - value['state'] = 'reached' + # check goal target reached + if (goal.definition_condition == 'higher' and value.get('current', goal.current) >= goal.target_goal) \ + or (goal.definition_condition == 'lower' and value.get('current', goal.current) <= goal.target_goal): + value['state'] = 'reached' - # check goal failure - elif goal.end_date and fields.date.today() > goal.end_date: - value['state'] = 'failed' - value['closed'] = True - if value: - self.write(cr, uid, [goal.id], value, context=context) + # check goal failure + elif goal.end_date and fields.date.today() > goal.end_date: + value['state'] = 'failed' + value['closed'] = True + if value: + self.write(cr, uid, [goal.id], value, context=context) return True def action_start(self, cr, uid, ids, context=None): From a233e26f710bd6cc990d8d1897f379cb93580e87 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 6 May 2014 13:38:31 +0200 Subject: [PATCH 4/8] [IMP] gamification: commit for each goal definition in cron bzr revid: mat@openerp.com-20140506113831-iut3aqbwfwbnertv --- addons/gamification/models/challenge.py | 6 ++++++ addons/gamification/models/goal.py | 3 +++ 2 files changed, 9 insertions(+) diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index f4704f2de29..e7176e8cb09 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -264,6 +264,9 @@ class gamification_challenge(osv.Model): - Create the missing goals (eg: modified the challenge to add lines) - Update every running challenge """ + if context is None: + context = {} + # start scheduled challenges planned_challenge_ids = self.search(cr, uid, [ ('state', '=', 'draft'), @@ -281,6 +284,9 @@ class gamification_challenge(osv.Model): if not ids: ids = self.search(cr, uid, [('state', '=', 'inprogress')], context=context) + # in cron mode, will do intermediate commits + # TODO in trunk: replace by parameter + context.update({'commit_gamification': True}) return self._update_all(cr, uid, ids, context=context) def _update_all(self, cr, uid, ids, context=None): diff --git a/addons/gamification/models/goal.py b/addons/gamification/models/goal.py index 2cbb8869eb7..b4a47a0d07a 100644 --- a/addons/gamification/models/goal.py +++ b/addons/gamification/models/goal.py @@ -239,6 +239,7 @@ class gamification_goal(osv.Model): the target value being reached, the goal is set as failed.""" if context is None: context = {} + commit = context.get('commit_gamification', False) goals_by_definition = {} all_goals = {} @@ -358,6 +359,8 @@ class gamification_goal(osv.Model): value['closed'] = True if value: self.write(cr, uid, [goal.id], value, context=context) + if commit: + cr.commit() return True def action_start(self, cr, uid, ids, context=None): From 409bf269d3f6f6c34b0469bc52feb86730519e64 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 6 May 2014 13:52:45 +0200 Subject: [PATCH 5/8] [FIX] gamification: be sure that not challenge get subscribers depending of the report_message_frequency bzr revid: mat@openerp.com-20140506115245-ktuxhcsml466ym6u --- addons/gamification/models/challenge.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index e7176e8cb09..b1f297abde0 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -240,6 +240,11 @@ class gamification_challenge(osv.Model): write_res = super(gamification_challenge, self).write(cr, uid, ids, vals, context=context) + if vals.get('report_message_frequency', 'never') != 'never': + # _recompute_challenge_users do not set users for challenges with no reports, subscribing them now + for challenge in self.browse(cr, uid, ids, context=context): + self.message_subscribe(cr, uid, [challenge.id], [user.partner_id.id for user in challenge.user_ids], context=context) + if vals.get('state') == 'inprogress': self._recompute_challenge_users(cr, uid, ids, context=context) self._generate_goals_from_challenge(cr, uid, ids, context=context) From 687d675de5eb01dd2e1ef803c79acfb30d9dabc7 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 6 May 2014 15:48:05 +0200 Subject: [PATCH 6/8] [FIX] gamification: using batch mode to check rewards bzr revid: mat@openerp.com-20140506134805-4nsurv82cg9anfvm --- addons/gamification/models/challenge.py | 27 +++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index b1f297abde0..5668f5d8682 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -712,34 +712,35 @@ class gamification_challenge(osv.Model): rewarded_users = [] challenge_ended = end_date == yesterday.strftime(DF) or force if challenge.reward_id and challenge_ended or challenge.reward_realtime: - for user in challenge.user_ids: - reached_goal_ids = self.pool.get('gamification.goal').search(cr, uid, [ - ('challenge_id', '=', challenge.id), - ('user_id', '=', user.id), - ('start_date', '=', start_date), - ('end_date', '=', end_date), - ('state', '=', 'reached') - ], context=context) - if len(reached_goal_ids) == len(challenge.line_ids): + # not using start_date as intemportal goals have a start date but no end_date + reached_goals = self.pool.get('gamification.goal').read_group(cr, uid, [ + ('challenge_id', '=', challenge.id), + ('end_date', '=', end_date), + ('state', '=', 'reached') + ], fields=['user_id'], groupby=['user_id'], context=context) + for reach_goals_user in reached_goals: + if reach_goals_user['user_id_count'] == len(challenge.line_ids): # the user has succeeded every assigned goal + user_id = reach_goals_user['user_id'][0] if challenge.reward_realtime: badges = self.pool['gamification.badge.user'].search(cr, uid, [ ('challenge_id', '=', challenge.id), ('badge_id', '=', challenge.reward_id.id), - ('user_id', '=', user.id), + ('user_id', '=', user_id), ], count=True, context=context) if badges > 0: # has already recieved the badge for this challenge continue - self.reward_user(cr, uid, user.id, challenge.reward_id.id, challenge.id, context=context) - rewarded_users.append(user) + self.reward_user(cr, uid, user_id, challenge.reward_id.id, challenge.id, context=context) + rewarded_users.append(user_id) if challenge_ended: # open chatter message message_body = _("The challenge %s is finished." % challenge.name) if rewarded_users: - message_body += _("
Reward (badge %s) for every succeeding user was sent to %s." % (challenge.reward_id.name, ", ".join([user.name for user in rewarded_users]))) + user_names = self.pool['res.users'].name_get(cr, uid, rewarded_users, context=context) + message_body += _("
Reward (badge %s) for every succeeding user was sent to %s." % (challenge.reward_id.name, ", ".join([user for user in user_names]))) else: message_body += _("
Nobody has succeeded to reach every goal, no badge is rewared for this challenge.") From 3cdb9803d4095165054c3224a8e90d87e2f81005 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 6 May 2014 15:59:53 +0200 Subject: [PATCH 7/8] [IMP] unsubscribing at each cron run is overkill bzr revid: mat@openerp.com-20140506135953-3hssq2lycvy1c3df --- addons/gamification/models/challenge.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index 5668f5d8682..4c1823c655c 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -366,13 +366,12 @@ class gamification_challenge(osv.Model): if write_op: self.write(cr, uid, [challenge.id], {'user_ids': write_op}, context=context) + # TODO for trunk: use proper parameter if challenge.report_message_frequency != 'never': if to_remove_ids: self.message_unsubscribe_users(cr, uid, [challenge.id], to_remove_ids, context=context) if to_add_ids: self.message_subscribe_users(cr, uid, [challenge.id], to_add_ids, context=context) - else: - self.message_unsubscribe_users(cr, uid, [challenge.id], old_user_ids, context=context) return True From 3fa27518556355c108e8a47b66b3475e22430956 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Tue, 6 May 2014 16:38:34 +0200 Subject: [PATCH 8/8] [FIX] name_get returns tuple bzr revid: mat@openerp.com-20140506143834-od28gkhbpbhsgde4 --- addons/gamification/models/challenge.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/gamification/models/challenge.py b/addons/gamification/models/challenge.py index 4c1823c655c..d3297a048d3 100644 --- a/addons/gamification/models/challenge.py +++ b/addons/gamification/models/challenge.py @@ -739,7 +739,7 @@ class gamification_challenge(osv.Model): if rewarded_users: user_names = self.pool['res.users'].name_get(cr, uid, rewarded_users, context=context) - message_body += _("
Reward (badge %s) for every succeeding user was sent to %s." % (challenge.reward_id.name, ", ".join([user for user in user_names]))) + message_body += _("
Reward (badge %s) for every succeeding user was sent to %s." % (challenge.reward_id.name, ", ".join([name for (user_id, name) in user_names]))) else: message_body += _("
Nobody has succeeded to reach every goal, no badge is rewared for this challenge.")