[FIX] hr_holidays: prevent the archiving of allocations

Archiving an allocation creates bad behaviour
in the use of the time off application.

For example, an employee who has several allocations some of which are archived
in the same period will create a problem in the counting of remaining days off.

Steps to reproduce:
- for an employee;
- create a 5 days type A allocation with a validity from 01/01/2023 to 31/01/2023;
- create a 5 days type A allocation with validity from 01/01/2023;
- archive the first allowance;
- set time off for this employee in this period.

Issue:
The employee's days off are not deducted until he has taken at least 5 days off.

Cause:
The process of deciding which allocation to use first will depend
on whether it has an end date or not.
It will therefore use the allocation with an end date first
(in our case the archived one).

Solution:
Archived allocations cannot be ignored by removing them
from the calculation process, as they have a use.
The solution that respects the business flow is to prevent archiving
for allocations that are not in a draft or refuse state.

opw-2991368

closes odoo/odoo#111092

Signed-off-by: Kevin Baptiste <kba@odoo.com>

addons/hr_holidays/models/hr_leave_allocation.py

@@ -534,6 +534,9 @@ class HolidaysAllocation(models.Model):
         return holidays
 
     def write(self, values):
+        if not bool(values.get('active', True)):
+            if any(allocation.state not in ['draft', 'cancel', 'refuse'] for allocation in self):
+                raise UserError(_('You cannot archive an allocation which is in confirm or validate state.'))
         employee_id = values.get('employee_id', False)
         if values.get('state'):
             self._check_approval_update(values['state'])

addons/hr_holidays/tests/test_leave_requests.py

@@ -13,6 +13,8 @@ from odoo.tools import mute_logger
 from odoo.tests.common import Form
 from odoo.tests import tagged
 
+from odoo.exceptions import UserError
+
 from odoo.addons.hr_holidays.tests.common import TestHrHolidaysCommon
 
 @tagged('leave_requests')
@@ -801,34 +803,41 @@ class TestLeaveRequests(TestHrHolidaysCommon):
                 ml=10, lt=5, rl=5, vrl=5, vlt=5,
             )
 
-            allocation_2021.active = False
-
-            # If the allocation is archived, the leaves taken are still counted on this allocation
-            # but the max leaves and remaining leaves are not counted anymore
-            self._check_holidays_count(
-                self.holidays_type_2.get_employees_days([self.employee_emp_id], date=date(2021, 12, 1))[self.employee_emp_id][self.holidays_type_2.id],
-                ml=0, lt=5, rl=0, vrl=0, vlt=5,
-            )
-
-            # The holidays count in 2022 is not affected by the archived allocation in 2021
-            self._check_holidays_count(
-                self.holidays_type_2.get_employees_days([self.employee_emp_id])[self.employee_emp_id][self.holidays_type_2.id],
-                ml=20, lt=4, rl=16, vrl=16, vlt=4,
-            )
-
-            allocation_2021.active = True
-
-            # The holidays count in 2021 is back to what it was when the allocation was active
-            self._check_holidays_count(
-                self.holidays_type_2.get_employees_days([self.employee_emp_id], date=date(2021, 12, 1))[self.employee_emp_id][self.holidays_type_2.id],
-                ml=10, lt=5, rl=5, vrl=5, vlt=5,
-            )
-
-            # The holidays count in 2022 is still not affected by the allocation in 2021
-            self._check_holidays_count(
-                self.holidays_type_2.get_employees_days([self.employee_emp_id])[self.employee_emp_id][self.holidays_type_2.id],
-                ml=20, lt=4, rl=16, vrl=16, vlt=4,
-            )
+            with self.assertRaisesRegex(UserError,
+                r'You cannot archive an allocation which is in confirm or validate state.'):
+
+                # The logic of the test is relevant, so we do not remove it.
+                # However, the behaviour will change.
+                # Indeed, a confirmed or validated allocation cannot be archived
+
+                allocation_2021.active = False
+
+                # If the allocation is archived, the leaves taken are still counted on this allocation
+                # but the max leaves and remaining leaves are not counted anymore
+                self._check_holidays_count(
+                    self.holidays_type_2.get_employees_days([self.employee_emp_id], date=date(2021, 12, 1))[self.employee_emp_id][self.holidays_type_2.id],
+                    ml=0, lt=5, rl=0, vrl=0, vlt=5,
+                )
+
+                # The holidays count in 2022 is not affected by the archived allocation in 2021
+                self._check_holidays_count(
+                    self.holidays_type_2.get_employees_days([self.employee_emp_id])[self.employee_emp_id][self.holidays_type_2.id],
+                    ml=20, lt=4, rl=16, vrl=16, vlt=4,
+                )
+
+                allocation_2021.active = True
+
+                # The holidays count in 2021 is back to what it was when the allocation was active
+                self._check_holidays_count(
+                    self.holidays_type_2.get_employees_days([self.employee_emp_id], date=date(2021, 12, 1))[self.employee_emp_id][self.holidays_type_2.id],
+                    ml=10, lt=5, rl=5, vrl=5, vlt=5,
+                )
+
+                # The holidays count in 2022 is still not affected by the allocation in 2021
+                self._check_holidays_count(
+                    self.holidays_type_2.get_employees_days([self.employee_emp_id])[self.employee_emp_id][self.holidays_type_2.id],
+                    ml=20, lt=4, rl=16, vrl=16, vlt=4,
+                )
 
     def test_create_support_document_in_the_past(self):
         with freeze_time('2022-10-19'):