From 872826e2eb997fc1c66099364828ca529a317ca5 Mon Sep 17 00:00:00 2001 From: Laetitia Da Costa Date: Fri, 16 Oct 2026 15:19:27 +0200 Subject: [PATCH] [FIX]hr_employee_stats_sheet:remove invalid or inactive recoveries from total_recovery_hours in timesheet_sheet statistics --- hr_employee_stats_sheet/__manifest__.py | 2 +- .../models/hr_employee_stats.py | 3 + .../tests/test_employee_stats.py | 84 ++++++++++++++++++- 3 files changed, 84 insertions(+), 5 deletions(-) diff --git a/hr_employee_stats_sheet/__manifest__.py b/hr_employee_stats_sheet/__manifest__.py index ca680be..c133458 100755 --- a/hr_employee_stats_sheet/__manifest__.py +++ b/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", diff --git a/hr_employee_stats_sheet/models/hr_employee_stats.py b/hr_employee_stats_sheet/models/hr_employee_stats.py index f191dfa..4071de9 100644 --- a/hr_employee_stats_sheet/models/hr_employee_stats.py +++ b/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: diff --git a/hr_employee_stats_sheet/tests/test_employee_stats.py b/hr_employee_stats_sheet/tests/test_employee_stats.py index 9a84dce..2b1903b 100644 --- a/hr_employee_stats_sheet/tests/test_employee_stats.py +++ b/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")