[FIX] hr_holidays: clean allocation validation

PURPOSE

When we made the b2b on Time Off, we designed the allocation settings to be like this :

    1. Allow Allocation : NO (stop here)
    2. If YES
    Can the Employee requests Allocation himself ?
2.1. NO (stop here, cause the HR will create the allocation and it will be automatically accepted)
2.2. If YES
Who will approve the Allocation made by the Employee ?
2.2.1. Nobody, it's directly approved
2.2.2. The time off officer must approve
2.2.3. It's SET by the time off officer.

In the day to day live, the case 2.2.3. never happen. It's the same settings as 2.1. "the employee will not make an allocaiton request, it will be the HR officer"
In fact, nobody understand what is behind that settings and the technical code behind it seems obsolete.

SPECIFICATION

1. Remove the "set by time off officer"
2. If Employee Requests is set on "Not Allowed", don't display Approval : https://tinyurl.com/yxwwql83 if an allocation is created, it will be done by the HR officer and automatically approved.

task-2705150

closes odoo/odoo#81914

Related: odoo/upgrade#3247
Related: odoo/enterprise#23105
Signed-off-by: Kevin Baptiste <kba@odoo.com>

addons/hr_holidays/models/hr_leave_allocation.py

@@ -696,10 +696,7 @@ class HolidaysAllocation(models.Model):
         self.ensure_one()
         responsible = self.env.user
 
-        if self.validation_type == 'manager' or (self.validation_type == 'both' and self.state == 'confirm'):
-            if self.employee_id.leave_manager_id:
-                responsible = self.employee_id.leave_manager_id
-        elif self.validation_type == 'hr' or (self.validation_type == 'both' and self.state == 'validate1'):
+        if self.validation_type == 'officer':
             if self.holiday_status_id.responsible_id:
                 responsible = self.holiday_status_id.responsible_id
 
@@ -708,29 +705,30 @@ class HolidaysAllocation(models.Model):
     def activity_update(self):
         to_clean, to_do = self.env['hr.leave.allocation'], self.env['hr.leave.allocation']
         for allocation in self:
-            note = _(
-                'New Allocation Request created by %(user)s: %(count)s Days of %(allocation_type)s',
-                user=allocation.create_uid.name,
-                count=allocation.number_of_days,
-                allocation_type=allocation.holiday_status_id.name
-            )
-            if allocation.state == 'draft':
-                to_clean |= allocation
-            elif allocation.state == 'confirm':
-                allocation.activity_schedule(
-                    'hr_holidays.mail_act_leave_allocation_approval',
-                    note=note,
-                    user_id=allocation.sudo()._get_responsible_for_approval().id or self.env.user.id)
-            elif allocation.state == 'validate1':
-                allocation.activity_feedback(['hr_holidays.mail_act_leave_allocation_approval'])
-                allocation.activity_schedule(
-                    'hr_holidays.mail_act_leave_allocation_second_approval',
-                    note=note,
-                    user_id=allocation.sudo()._get_responsible_for_approval().id or self.env.user.id)
-            elif allocation.state == 'validate':
-                to_do |= allocation
-            elif allocation.state == 'refuse':
-                to_clean |= allocation
+            if allocation.validation_type != 'no':
+                note = _(
+                    'New Allocation Request created by %(user)s: %(count)s Days of %(allocation_type)s',
+                    user=allocation.create_uid.name,
+                    count=allocation.number_of_days,
+                    allocation_type=allocation.holiday_status_id.name
+                )
+                if allocation.state == 'draft':
+                    to_clean |= allocation
+                elif allocation.state == 'confirm':
+                    allocation.activity_schedule(
+                        'hr_holidays.mail_act_leave_allocation_approval',
+                        note=note,
+                        user_id=allocation.sudo()._get_responsible_for_approval().id or self.env.user.id)
+                elif allocation.state == 'validate1':
+                    allocation.activity_feedback(['hr_holidays.mail_act_leave_allocation_approval'])
+                    allocation.activity_schedule(
+                        'hr_holidays.mail_act_leave_allocation_second_approval',
+                        note=note,
+                        user_id=allocation.sudo()._get_responsible_for_approval().id or self.env.user.id)
+                elif allocation.state == 'validate':
+                    to_do |= allocation
+                elif allocation.state == 'refuse':
+                    to_clean |= allocation
         if to_clean:
             to_clean.activity_unlink(['hr_holidays.mail_act_leave_allocation_approval', 'hr_holidays.mail_act_leave_allocation_second_approval'])
         if to_do:

addons/hr_holidays/models/hr_leave_type.py

@@ -85,14 +85,22 @@ class HolidaysType(models.Model):
         ('both', "By Employee's Approver and Time Off Officer")], default='hr', string='Leave Validation')
     requires_allocation = fields.Selection([
         ('yes', 'Yes'),
-        ('no', 'No Limit')], default="yes", required=True, string='Requires allocation')
+        ('no', 'No Limit')], default="yes", required=True, string='Requires allocation',
+        help="""Yes: Time off requests need to have a valid allocation.\n
+              No Limit: Time Off requests can be taken without any prior allocation.""")
     employee_requests = fields.Selection([
         ('yes', 'Extra Days Requests Allowed'),
-        ('no', 'Not Allowed')], default="no", required=True, string="Employee Requests")
+        ('no', 'Not Allowed')], default="no", required=True, string="Employee Requests",
+        help="""Extra Days Requests Allowed: User can request an allocation for himself.\n
+        Not Allowed: User cannot request an allocation.""")
     allocation_validation_type = fields.Selection([
-        ('no', 'No validation needed'),
         ('officer', 'Approved by Time Off Officer'),
-        ('set', "Set by Time Off Officer")], default='officer', string='Approval')
+        ('no', 'No validation needed')], default='officer', string='Approval',
+        compute='_compute_allocation_validation_type', store=True, readonly=False,
+        help="""Approved by Time Off Officer: The requested allocation must be validated by the time off approver set on the
+        employee's profile or by a member of the time off officer security group.\n
+        No validation needed: The requested allocation is automatically approved.
+        """)
     has_valid_allocation = fields.Boolean(compute='_compute_valid', search='_search_valid', help='This indicates if it is still possible to use this type of leave')
     time_type = fields.Selection([('leave', 'Time Off'), ('other', 'Other')], default='leave', string="Kind of Leave",
                                  help="Whether this should be computed as a holiday or as work time (eg: formation)")
@@ -387,6 +395,11 @@ class HolidaysType(models.Model):
         for leave_type in self:
             leave_type.accrual_count = mapped_data.get(leave_type.id, 0)
 
+    @api.depends('employee_requests')
+    def _compute_allocation_validation_type(self):
+        for leave_type in self:
+            leave_type.allocation_validation_type = 'no' if leave_type.employee_requests == 'no' else 'officer'
+
     def requested_name_get(self):
         return self._context.get('holiday_status_name_get', True) and self._context.get('employee_id')
 

addons/hr_holidays/security/hr_holidays_security.xml

@@ -169,7 +169,7 @@
                     ('employee_id.user_id', '=', user.id),
                     ('state', '!=', 'validate'),
                 '&',
-                    ('validation_type', 'in', ['officer', 'set']),
+                    ('validation_type', '=', 'officer'),
                     ('employee_id.leave_manager_id', '=', user.id),
         ]</field>
         <field name="perm_read" eval="False"/>

addons/hr_holidays/tests/test_allocation_access_rights.py

@@ -32,13 +32,6 @@ class TestAllocationRights(TestHrHolidaysCommon):
             'employee_requests': 'yes',
         })
 
-        cls.lt_allocation_manager = cls.env['hr.leave.type'].create({
-            'name': 'Validation = manager',
-            'allocation_validation_type': 'set',
-            'requires_allocation': 'yes',
-            'employee_requests': 'no',
-        })
-
         cls.lt_allocation_no_validation = cls.env['hr.leave.type'].create({
             'name': 'Validation = user',
             'allocation_validation_type': 'no',
@@ -68,15 +61,6 @@ class TestAccessRightsSimpleUser(TestAllocationRights):
         with self.assertRaises(UserError):
             allocation.action_validate()
 
-    def test_simple_user_request_fixed_allocation(self):
-        """ A simple user cannot request an allocation if employee requests is not enabled """
-        values = {
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.lt_allocation_manager.id,
-        }
-        with self.assertRaises(AccessError):
-            self.request_allocation(self.user_employee.id, values)
-
     def test_simple_user_request_allocation_no_validation(self):
         """ A simple user can request and automatically validate an allocation with no validation """
         values = {
@@ -183,17 +167,6 @@ class TestAccessRightsHolidayUser(TestAllocationRights):
         allocation.action_validate()
         self.assertEqual(allocation.state, 'validate', "It should have been validated")
 
-    def test_holiday_user_request_fixed_allocation(self):
-        """ A holiday user can request and approve an allocation if set by HR """
-        values = {
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.lt_allocation_manager.id,
-        }
-        allocation = self.request_allocation(self.user_hruser.id, values)
-        allocation.action_confirm()
-        allocation.action_validate()
-        self.assertEqual(allocation.state, 'validate', "It should have been validated")
-
     def test_holiday_user_batch_allocation(self):
         """ A holiday user cannot create a batch allocation """
         values = {

addons/hr_holidays/tests/test_holidays_flow.py

@@ -88,7 +88,7 @@ class TestHolidaysFlow(TestHrHolidaysCommon):
                 'name': 'Paid Time Off',
                 'requires_allocation': 'yes',
                 'employee_requests': 'no',
-                'allocation_validation_type': 'set',
+                'allocation_validation_type': 'officer',
                 'leave_validation_type': 'both',
                 'responsible_id': self.env.ref('base.user_admin').id,
             })
@@ -132,7 +132,7 @@ class TestHolidaysFlow(TestHrHolidaysCommon):
                 'name': 'Limited',
                 'requires_allocation': 'yes',
                 'employee_requests': 'no',
-                'allocation_validation_type': 'set',
+                'allocation_validation_type': 'officer',
                 'leave_validation_type': 'both',
             })
             HolidaysEmployeeGroup = Requests.with_user(self.user_employee_id)
@@ -242,7 +242,7 @@ class TestHolidaysFlow(TestHrHolidaysCommon):
             'name': 'Paid Time Off',
             'requires_allocation': 'yes',
             'employee_requests': 'no',
-            'allocation_validation_type': 'set',
+            'allocation_validation_type': 'officer',
             'leave_validation_type': 'both',
             'responsible_id': self.env.ref('base.user_admin').id,
         })

addons/hr_holidays/views/hr_leave_type_views.xml

@@ -75,7 +75,7 @@
                         <h2>Allocation Requests</h2>
                             <field name="requires_allocation" widget="radio" options="{'horizontal':true}"/>
                             <field name="employee_requests" widget="radio" attrs="{'invisible': [('requires_allocation', '=', 'no')]}"/>
-                            <field name="allocation_validation_type" string="Approval" widget="radio" attrs="{'invisible': [('requires_allocation', '=', 'no')]}"/>
+                            <field name="allocation_validation_type" string="Approval" widget="radio" attrs="{'invisible': ['|', ('requires_allocation', '=', 'no'), ('employee_requests', '=', 'no')]}"/>
                         </group>
                     </group>
                     <group>

addons/hr_holidays_attendance/tests/test_holidays_overtime.py

@@ -47,14 +47,6 @@ class TestHolidaysOvertime(TransactionCase):
             'employee_requests': 'yes',
             'overtime_deductible': True,
         })
-        cls.leave_type_manager_alloc = cls.env['hr.leave.type'].create({
-            'name': 'Overtime Compensation Manager Allocation',
-            'company_id': cls.company.id,
-            'requires_allocation': 'yes',
-            'employee_requests': 'no',
-            'allocation_validation_type': 'set',
-            'overtime_deductible': True,
-        })
 
     def new_attendance(self, check_in, check_out=False):
         return self.env['hr.attendance'].create({