[FIX] gamification: update internal users goals

For performance reason, we avoided computing goals for the set of users
that didn't log in recently (See ec0c0f29).
However, users can stay logged in for a while without having a new "log
in event" (password asked), such that active internal users can keep
old values in their challenges when reports are sent, which is not good.

Until an improvement can be implemented in master, we drop this time
constraint for active internal users.

A test is added, checking the behavior of the method called by the cron.

Task-3226408

closes odoo/odoo#121506

X-original-commit: 6c77dd822c719a2181bac1d69130e5c4e8ef70fa
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>

addons/gamification/models/gamification_challenge.py

@@ -263,22 +263,21 @@ class Challenge(models.Model):
         return records._update_all()
 
     def _update_all(self):
-        """Update the challenges and related goals
-
-        :param list(int) ids: the ids of the challenges to update, if False will
-        update only challenges in progress."""
+        """Update the challenges and related goals."""
         if not self:
             return True
 
         Goals = self.env['gamification.goal']
 
         # include yesterday goals to update the goals that just ended
-        # exclude goals for users that did not connect since the last update
+        # exclude goals for portal users that did not connect since the last update
         yesterday = fields.Date.to_string(date.today() - timedelta(days=1))
         self.env.cr.execute("""SELECT gg.id
                         FROM gamification_goal as gg
                         JOIN res_users_log as log ON gg.user_id = log.create_uid
-                       WHERE gg.write_date < log.create_date
+                        JOIN res_users ru on log.create_uid = ru.id
+                       WHERE (gg.write_date < log.create_date OR ru.share IS NOT TRUE)
+                         AND ru.active IS TRUE
                          AND gg.closed IS NOT TRUE
                          AND gg.challenge_id IN %s
                          AND (gg.state = 'inprogress'

addons/gamification/tests/test_challenge.py

@@ -1,8 +1,10 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
+import datetime
 
 from odoo.addons.base.tests.common import TransactionCaseWithUserDemo
 from odoo.exceptions import UserError
+from odoo.tools import mute_logger
 
 
 class TestGamificationCommon(TransactionCaseWithUserDemo):
@@ -59,6 +61,82 @@ class test_challenge(TestGamificationCommon):
         badge_ids = self.env['gamification.badge.user'].search([('badge_id', '=', badge_id), ('user_id', '=', demo.id)])
         self.assertEqual(len(badge_ids), 1, "Demo user has not received the badge")
 
+    @mute_logger('odoo.models.unlink')
+    def test_20_update_all_goals_filter(self):
+        # Enroll two internal and two portal users in the challenge
+        (
+            portal_login_before_update,
+            portal_login_after_update,
+            internal_login_before_update,
+            internal_login_after_update,
+        ) = all_test_users = self.env['res.users'].create([
+            {
+                'name': f'{kind} {age} login',
+                'login': f'{kind}_{age}',
+                'email': f'{kind}_{age}',
+                'groups_id': [(6, 0, groups_id)],
+            }
+            for kind, groups_id in (
+                ('Portal', []),
+                ('Internal', [self.env.ref('base.group_user').id]),
+            )
+            for age in ('Old', 'Recent')
+        ])
+
+        challenge = self.env.ref('gamification.challenge_base_discover')
+        challenge.write({
+            'state': 'inprogress',
+            'user_domain': False,
+            'user_ids': [(6, 0, all_test_users.ids)]
+        })
+
+        # Setup user access logs
+        self.env['res.users.log'].search([('create_uid', 'in', challenge.user_ids.ids)]).unlink()
+        now = datetime.datetime.now()
+
+        # Create "old" log in records
+        self.env['res.users.log'].create([
+            {"create_uid": internal_login_before_update.id, 'create_date': now - datetime.timedelta(minutes=3)},
+            {"create_uid": portal_login_before_update.id, 'create_date': now - datetime.timedelta(minutes=3)},
+        ])
+
+        # Reset goal objective values
+        all_test_users.partner_id.tz = False
+
+        # Regenerate all goals
+        self.env["gamification.goal"].search([]).unlink()
+        self.assertFalse(self.env['gamification.goal'].search([]))
+
+        challenge.action_check()
+        goal_ids = self.env['gamification.goal'].search(
+            [('challenge_id', '=', challenge.id), ('state', '!=', 'draft'), ('user_id', 'in', challenge.user_ids.ids)]
+        )
+        self.assertEqual(len(goal_ids), 4)
+        self.assertEqual(set(goal_ids.mapped('state')), {'inprogress'})
+
+        # Create more recent log in records
+        self.env['res.users.log'].create([
+            {"create_uid": internal_login_after_update.id, 'create_date': now + datetime.timedelta(minutes=3)},
+            {"create_uid": portal_login_after_update.id, 'create_date': now + datetime.timedelta(minutes=3)},
+        ])
+
+        # Update goal objective checked by goal definition
+        all_test_users.partner_id.write({'tz': 'Europe/Paris'})
+
+        # Update goals as done by _cron_update
+        challenge._update_all()
+        unchanged_goal_id = self.env['gamification.goal'].search([
+            ('challenge_id', '=', challenge.id),
+            ('state', '=', 'inprogress'),  # others were updated to "reached"
+            ('user_id', 'in', challenge.user_ids.ids),
+        ])
+        # Check that even though login record for internal user is older than goal update, their goal was reached.
+        self.assertEqual(
+            portal_login_before_update,
+            unchanged_goal_id.user_id,
+            "Only portal user last logged in before last challenge update should not have been updated.",
+        )
+
 
 class test_badge_wizard(TestGamificationCommon):