[FIX]hr_employee_stats_sheet:remove invalid or inactive recoveries from total_recovery_hours in timesheet_sheet statistics

hr_employee_stats_sheet/__manifest__.py

@@ -1,6 +1,6 @@
 {
     "name": "hr_employee_stats_sheet",
-    "version": "16.0.3.3.0",
+    "version": "16.0.3.4.0",
     "description": "Add global sheet for employee stats",
     "summary": "Add global sheet for employee stats",
     "author": "Nicolas JEUDY",

hr_employee_stats_sheet/models/hr_employee_stats.py

@@ -109,6 +109,7 @@ class HrEmployeeStats(models.Model):
         return total_planned_hours
 
     def _get_total_recovery_hours(self):
+        #TODO: duplicated code with _get_total_leave_hours, to refactor later
         self.ensure_one()
         recovery = self.env["hr.leave"]
         total_recovery_hours = 0
@@ -119,6 +120,8 @@ class HrEmployeeStats(models.Model):
                     ("holiday_status_id", "=", self._get_holiday_status_id()),
                     ("request_date_from", "<=", self.date),
                     ("request_date_to", ">=", self.date),
+                    ("state", '=', 'validate'),
+                    ("active", '=', True),
                 ],
             )
             for recovery_id in recovery_ids:

hr_employee_stats_sheet/tests/test_employee_stats.py

@@ -155,6 +155,46 @@ class TestHrEmployeeStatsRecovery(TransactionCase):
         self.assertEqual(len(recovery_allocation), 1, "There should be one recovery")
         self.assertEqual(recovery_allocation.number_of_days, -0.7142857142857143, "The recovery allocation should be for -5/7 hours = −0,714285714")
 
+    def test_sheet_totals_update_after_timesheet_change(self):
+        """Regression test: the sheet totals must reflect the addition and
+        removal of analytic lines on a fresh read (new request / hard
+        refresh), without any manual compute call.
+
+        This test fails if the fields are redefined as store=True while the
+        recompute trigger chain is broken (values frozen in the database
+        column), which is what happened with the pivot-view customizations.
+        """
+        self.env['hr.employee.calendar'].create({
+            'employee_id': self.employee.id,
+            'date_start': Date.to_date("2025-01-01"),
+            'date_end': None,
+            'calendar_id': self.base_calendar.id,
+        })
+        # fixed past week (monday 2025-03-03), no public holidays on it
+        start_date = Date.to_date("2025-03-03")
+        timesheet_sheet = self._create_timesheet_sheet(start_date)
+        # no line worked yet: the gap equals minus the planned hours of the
+        # 5 planned days (5 x -7h); recovery is not coef-ed on negative gaps
+        self.assertEqual(timesheet_sheet.timesheet_sheet_gap_hours, -35, "timesheet_sheet_gap_hours should be -35 (empty week)",)
+        self.assertEqual(timesheet_sheet.timesheet_sheet_recovery_hours, -35, "timesheet_sheet_recovery_hours should be -35 (empty week)",)
+        # the employee logs 3h on monday (less than the 7h planned that day)
+        line = self.env['account.analytic.line'].create({
+            'employee_id': self.employee.id,
+            'date': start_date,
+            'unit_amount': 3,
+            'account_id': self.analytic_account.id,
+            'name': 'Work Entry',
+        })
+        # simulate a new request / hard refresh: recompute everything from scratch
+        self.env.invalidate_all()  # simulate a new request / hard refresh: fresh recompute of everything
+        self.assertEqual(timesheet_sheet.timesheet_sheet_gap_hours, -32, "timesheet_sheet_gap_hours should be -32 after adding the line",)
+        self.assertEqual(timesheet_sheet.timesheet_sheet_recovery_hours, -32, "timesheet_sheet_recovery_hours should be -32 after adding the line",)
+        # the employee removes the line
+        line.unlink()
+        self.env.invalidate_all()  # simulate a new request / hard refresh: fresh recompute of everything
+        self.assertEqual(timesheet_sheet.timesheet_sheet_gap_hours, -35, "timesheet_sheet_gap_hours should be back to -35 after removing the line",)
+        self.assertEqual(timesheet_sheet.timesheet_sheet_recovery_hours, -35, "timesheet_sheet_recovery_hours should be back to -35 after removing the line",)
+
     def test_recovery_hours_part_time_employee(self):
         part_time_calendar = self.env['resource.calendar'].create({
             'name': 'Part Time Calendar',
@@ -407,6 +447,12 @@ class TestHrEmployeeStatsLeaveAndRecoveryUnit(TransactionCase):
             'name': 'Dominique',
             'tz': 'Europe/Paris',
         })
+        # hr_employee_calendar_planning auto-creates a calendar_ids line with
+        # the company calendar (undated "Standard 40 hours/week", 8h/day).
+        # If we leave it, _regenerate_calendar merges it with our 7h calendar
+        # (8h-12h/13h-17h instead of 9h-12h/13h-17h) and number_of_hours_display
+        # returns 8h/day instead of 7h -> remove it before adding our planning.
+        self.employee.calendar_ids.unlink()
         # Full-time calendar: Monday to Friday, 9am-12pm and 1pm-5pm (7h/day).
         # The calendar timezone must match the employee's so that hour-based
         # requests fall exactly on the working hours.
@@ -475,9 +521,14 @@ class TestHrEmployeeStatsLeaveAndRecoveryUnit(TransactionCase):
             vals['request_unit_hours'] = True
             vals['request_hour_from'] = hour_from
             vals['request_hour_to'] = hour_to
-        leave = self.env['hr.leave'].create(vals)
+        # leave_fast_create: create() neither forces state nor auto-validates,
+        # so the leave stays in 'draft' and we can compute the dates explicitly
+        # (hr.leave.create() computes date_from/date_to on the current day,
+        # not on the requested dates, in our test setup) then validate.
+        leave = self.env['hr.leave'].with_context(leave_fast_create=True).create(vals)
         leave._compute_date_from_to()
-        leave.action_validate()
+        if leave.state != 'validate':
+            leave.action_validate()
         return leave
 
     def _create_stat(self, day):
@@ -513,6 +564,16 @@ class TestHrEmployeeStatsLeaveAndRecoveryUnit(TransactionCase):
         stat = self._create_stat(self.monday)
         self.assertEqual(stat.total_recovery_hours, 3, "total_recovery_hours should be 3 (9am-12pm)")
 
+    def test_recovery_hours_ignore_refused_leave(self):
+        # regression: une récupération refusée ne doit pas être comptée
+        leave = self._create_leave(self.recovery_type, self.monday, self.monday)
+        self.assertEqual(leave.state, 'validate')
+        leave.action_refuse()
+        self.assertTrue(leave.active)  # refusée mais toujours active : c'est le piège
+        stat = self._create_stat(self.monday)
+        self.assertEqual(stat.total_recovery_hours, 0,
+                         "a refused recovery must not be counted")
+
     def test_recovery_hours_unit_day_several_days(self):
         # 'day' type, recovery taken on Monday AND Tuesday: for each day we
         # fall back to the planned hours of THAT specific day (7h)
@@ -551,9 +612,24 @@ class TestHrEmployeeStatsLeaveAndRecoveryUnit(TransactionCase):
         self.assertEqual(monday_stat.total_leave_hours, 7, "total_leave_hours should be 7 on monday")
         self.assertEqual(tuesday_stat.total_leave_hours, 7, "total_leave_hours should be 7 on tuesday")
 
+    def test_leave_hours_ignore_refused_leave(self):
+        # même piège que pour la récupération : un congé refusé reste actif
+        leave_type = self._create_leave_type('day')
+        leave = self._create_leave(leave_type, self.monday, self.monday)
+        leave.action_refuse()
+        stat = self._create_stat(self.monday)
+        self.assertEqual(stat.total_leave_hours, 0,
+                         "a refused leave must not be counted")
+
     def test_leave_hours_ignore_non_validated_leave(self):
         # _get_total_leave_hours only counts validated and active leaves
-        leave_type = self._create_leave_type('day')
+        leave_type = self.env['hr.leave.type'].create({
+            'name': 'Leave with validation',
+            'request_unit': 'day',
+            'requires_allocation': 'no',
+            'allows_negative': True,
+            'leave_validation_type': 'hr',
+        })
         leave = self.env['hr.leave'].create({
             'name': 'Draft leave',
             'employee_id': self.employee.id,
@@ -561,7 +637,7 @@ class TestHrEmployeeStatsLeaveAndRecoveryUnit(TransactionCase):
             'request_date_from': self.monday,
             'request_date_to': self.monday,
         })
-        leave._compute_date_from_to()  # left unvalidated
+        self.assertEqual(leave.state, 'confirm')  # left unvalidated
         stat = self._create_stat(self.monday)
         self.assertEqual(stat.total_leave_hours, 0, "an unvalidated leave must not be counted")