[IMP] hr_holidays: Clarify the value of manager_id

Currently the manager fields have this meaning:

 * manager_id: employee that approved the leave first;
 * manager_id2: employee that approved the leave secondly;

It makes no sense. Furthermore, on the Leaves Dashboard, when you group
by Manager, you group by first approver, which is also a nonsense.

This commit improves the behavior

 * rename manager_id and manager_id2 into first_approver_id and
   second_approver_id in order to understand their meaning
 * add a stored related field manager_id, readonly to avoid modifications
   on the employee
 * modify the views to only display the real employee manager, the other
   values are not valuable on the views
 * in code regularly a manager variable is used but is actually the
   current employee; a renaming is then applied

2 files changed, 16 insertions, 14 deletions

diff --git a/addons/hr_holidays/models/hr_holidays.py b/addons/hr_holidays/models/hr_holidays.py
index d5b756ec5da..a1840e44bbf 100644
--- a/addons/hr_holidays/models/hr_holidays.py
+++ b/addons/hr_holidays/models/hr_holidays.py
@@ -180,8 +180,7 @@ class Holidays(models.Model):
         states={'draft': [('readonly', False)], 'confirm': [('readonly', False)]})
     employee_id = fields.Many2one('hr.employee', string='Employee', index=True, readonly=True,
         states={'draft': [('readonly', False)], 'confirm': [('readonly', False)]}, default=_default_employee, track_visibility='onchange')
-    manager_id = fields.Many2one('hr.employee', string='First Approval', readonly=True, copy=False,
-        help='This area is automatically filled by the user who validate the leave')
+    manager_id = fields.Many2one('hr.employee', related='employee_id.parent_id', string='Manager', readonly=True, store=True)
     notes = fields.Text('Reasons', readonly=True, states={'draft': [('readonly', False)], 'confirm': [('readonly', False)]})
     number_of_days_temp = fields.Float(
         'Allocation', copy=False, readonly=True,
@@ -207,7 +206,9 @@ class Holidays(models.Model):
     ], string='Allocation Mode', readonly=True, required=True, default='employee',
         states={'draft': [('readonly', False)], 'confirm': [('readonly', False)]},
         help='By Employee: Allocation/Request for individual Employee, By Employee Tag: Allocation/Request for group of employees in category')
-    manager_id2 = fields.Many2one('hr.employee', string='Second Approval', readonly=True, copy=False,
+    first_approver_id = fields.Many2one('hr.employee', string='First Approval', readonly=True, copy=False,
+        help='This area is automatically filled by the user who validate the leave', oldname='manager_id')
+    second_approver_id = fields.Many2one('hr.employee', string='Second Approval', readonly=True, copy=False, oldname='manager_id2',
         help='This area is automaticly filled by the user who validate the leave with second level (If Leave type need second validation)')
     double_validation = fields.Boolean('Apply Double Validation', related='holiday_status_id.double_validation')
     can_reset = fields.Boolean('Can reset', compute='_compute_can_reset')
@@ -402,8 +403,8 @@ class Holidays(models.Model):
                 raise UserError(_('Leave request state must be "Refused" or "To Approve" in order to reset to Draft.'))
             holiday.write({
                 'state': 'draft',
-                'manager_id': False,
-                'manager_id2': False,
+                'first_approver_id': False,
+                'second_approver_id': False,
             })
             linked_requests = holiday.mapped('linked_request_ids')
             for linked_request in linked_requests:
@@ -424,13 +425,13 @@ class Holidays(models.Model):
         if not self.env.user.has_group('hr_holidays.group_hr_holidays_user'):
             raise UserError(_('Only an HR Officer or Manager can approve leave requests.'))
 
-        manager = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1)
+        current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1)
         for holiday in self:
             if holiday.state != 'confirm':
                 raise UserError(_('Leave request must be confirmed ("To Approve") in order to approve it.'))
 
             if holiday.double_validation:
-                return holiday.write({'state': 'validate1', 'manager_id': manager.id if manager else False})
+                return holiday.write({'state': 'validate1', 'first_approver_id': current_employee.id})
             else:
                 holiday.action_validate()
 
@@ -439,7 +440,7 @@ class Holidays(models.Model):
         if not self.env.user.has_group('hr_holidays.group_hr_holidays_user'):
             raise UserError(_('Only an HR Officer or Manager can approve leave requests.'))
 
-        manager = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1)
+        current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1)
         for holiday in self:
             if holiday.state not in ['confirm', 'validate1']:
                 raise UserError(_('Leave request must be confirmed in order to approve it.'))
@@ -448,9 +449,9 @@ class Holidays(models.Model):
 
             holiday.write({'state': 'validate'})
             if holiday.double_validation:
-                holiday.write({'manager_id2': manager.id})
+                holiday.write({'second_approver_id': current_employee.id})
             else:
-                holiday.write({'manager_id': manager.id})
+                holiday.write({'first_approver_id': current_employee.id})
             if holiday.holiday_type == 'employee' and holiday.type == 'remove':
                 meeting_values = {
                     'name': holiday.display_name,
@@ -498,15 +499,15 @@ class Holidays(models.Model):
         if not self.env.user.has_group('hr_holidays.group_hr_holidays_user'):
             raise UserError(_('Only an HR Officer or Manager can refuse leave requests.'))
 
-        manager = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1)
+        current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1)
         for holiday in self:
             if holiday.state not in ['confirm', 'validate', 'validate1']:
                 raise UserError(_('Leave request must be confirmed or validated in order to refuse it.'))
 
             if holiday.state == 'validate1':
-                holiday.write({'state': 'refuse', 'manager_id': manager.id})
+                holiday.write({'state': 'refuse', 'first_approver_id': current_employee.id})
             else:
-                holiday.write({'state': 'refuse', 'manager_id2': manager.id})
+                holiday.write({'state': 'refuse', 'second_approver_id': current_employee.id})
             # Delete the meeting
             if holiday.meeting_id:
                 holiday.meeting_id.unlink()
diff --git a/addons/hr_holidays/views/hr_holidays_views.xml b/addons/hr_holidays/views/hr_holidays_views.xml
index b189a04c349..dd6e2b94c88 100644
--- a/addons/hr_holidays/views/hr_holidays_views.xml
+++ b/addons/hr_holidays/views/hr_holidays_views.xml
@@ -49,7 +49,8 @@
                         <filter name="group_name" string="Description" context="{'group_by':'name'}"/>
                         <filter name="group_employee" string="Employee" context="{'group_by':'employee_id'}"/>
                         <filter name="group_category" string="Category" context="{'group_by':'category_id'}"/>
-                        <filter string="Manager" context="{'group_by':'manager_id'}"/>
+                        <filter name="group_manager" string="Manager" context="{'group_by':'manager_id'}"/>
+                        <filter name="group_second_approver_id" string="Validator" context="{'group_by':'second_approver_id'}"/>
                         <filter name="group_type" string="Type" context="{'group_by':'holiday_status_id'}"/>
                         <separator/>
                         <filter name="group_date_from" string="Start Month" context="{'group_by':'date_from'}"/>