diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index 0a3b61045b5..377e6d7d640 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -143,9 +143,7 @@ class HolidaysRequest(models.Model): """ for holiday in self: # User is holiday manager and has no manager - manager = self.user_has_groups('hr_holidays.group_hr_holidays_manager') \ - and not holiday.employee_id.parent_id \ - and not holiday.department_id.manager_id + manager = self.user_has_groups('hr_holidays.group_hr_holidays_manager') holiday.can_approve = (holiday.employee_id.user_id.id != self.env.uid) or manager @api.onchange('holiday_type') @@ -412,10 +410,10 @@ class HolidaysRequest(models.Model): for holiday in self: validation_type = holiday.holiday_status_id.validation_type manager = holiday.employee_id.parent_id or holiday.employee_id.department_id.manager_id - if (validation_type in ['manager', 'both']) and (manager and manager != current_employee)\ + if (validation_type in ['hr', 'both']) and (manager and manager != current_employee)\ and not self.env.user.has_group('hr_holidays.group_hr_holidays_manager'): raise UserError(_('You must be %s manager to approve this leave') % (holiday.employee_id.name)) - elif validation_type == 'hr' and not self.env.user.has_group('hr_holidays.group_hr_holidays_manager'): + elif validation_type == 'manager' and not self.env.user.has_group('hr_holidays.group_hr_holidays_manager'): raise UserError(_('You must be a Human Resource Manager to approve this Leave')) self.filtered(lambda hol: hol.validation_type == 'both').write({'state': 'validate1', 'first_approver_id': current_employee.id}) diff --git a/addons/hr_holidays/models/hr_leave_allocation.py b/addons/hr_holidays/models/hr_leave_allocation.py index 75d2e0995dc..018866da480 100644 --- a/addons/hr_holidays/models/hr_leave_allocation.py +++ b/addons/hr_holidays/models/hr_leave_allocation.py @@ -115,9 +115,7 @@ class HolidaysAllocation(models.Model): """ for holiday in self: # User is holiday manager and has no manager - manager = self.user_has_groups('hr_holidays.group_hr_holidays_manager') \ - and not holiday.employee_id.parent_id \ - and not holiday.department_id.manager_id + manager = self.user_has_groups('hr_holidays.group_hr_holidays_manager') holiday.can_approve = (holiday.employee_id.user_id.id != self.env.uid) or manager @api.onchange('holiday_type') @@ -255,10 +253,10 @@ class HolidaysAllocation(models.Model): for holiday in self: validation_type = holiday.holiday_status_id.validation_type manager = holiday.employee_id.parent_id or holiday.employee_id.department_id.manager_id - if (validation_type in ['manager', 'both']) and (manager and manager != current_employee)\ + if (validation_type in ['hr', 'both']) and (manager and manager != current_employee)\ and not self.env.user.has_group('hr_holidays.group_hr_holidays_manager'): raise UserError(_('You must be %s manager to approve this leave') % (holiday.employee_id.name)) - elif validation_type == 'hr' and not self.env.user.has_group('hr_holidays.group_hr_holidays_manager'): + elif validation_type == 'manager' and not self.env.user.has_group('hr_holidays.group_hr_holidays_manager'): raise UserError(_('You must be a Human Resource Manager to approve this Leave')) self.filtered(lambda hol: hol.validation_type == 'both').write({'state': 'validate1', 'first_approver_id': current_employee.id}) diff --git a/addons/hr_holidays/tests/test_holidays_flow.py b/addons/hr_holidays/tests/test_holidays_flow.py index ff5f3a80307..6bbb81d57c0 100644 --- a/addons/hr_holidays/tests/test_holidays_flow.py +++ b/addons/hr_holidays/tests/test_holidays_flow.py @@ -101,14 +101,9 @@ class TestHolidaysFlow(TestHrHolidaysBase): hol1_employee_group.action_approve() self.assertEqual(hol1_manager_group.state, 'confirm', 'hr_holidays: employee should not be able to validate its own leave request') - # HrUser validates the employee leave request -> should not work - with self.assertRaises(UserError): - hol1_user_group.action_approve() - self.assertEqual(hol1_manager_group.state, 'confirm', 'hr_holidays: hr user should not be able to validate manager only leaves') - - # HrManager validates the employee leave request - hol1_manager_group.action_approve() - self.assertEqual(hol1_manager_group.state, 'validate', 'hr_holidays: validates leave request should be in validate state') + # HrUser validates the employee leave request -> should work + hol1_user_group.action_approve() + self.assertEqual(hol1_manager_group.state, 'validate', 'hr_holidays: validated leave request should be in validate state') # Employee creates a leave request in a no-limit category department manager only hol12_employee_group = HolidaysEmployeeGroup.create({ @@ -128,8 +123,12 @@ class TestHolidaysFlow(TestHrHolidaysBase): hol12_employee_group.action_approve() self.assertEqual(hol12_user_group.state, 'confirm', 'hr_holidays: employee should not be able to validate its own leave request') - # HrUser validates the employee leave request - hol12_user_group.action_approve() + # HrUser validates the employee leave request -> should not work + with self.assertRaises(UserError): + hol12_user_group.action_approve() + + # HrManager validate the employee leave request + hol12_manager_group.action_approve() self.assertEqual(hol1_user_group.state, 'validate', 'hr_holidays: validates leave request should be in validate state') # -------------------------------------------------- @@ -328,7 +327,7 @@ class TestHolidaysFlow(TestHrHolidaysBase): with self.assertRaises(UserError): hol50.action_approve() - # Manager should not be able to approve it's own leave + # Manager should be able to approve it's own leave hol51 = HolidaysEmployeeGroup.sudo(self.user_hrmanager_2_id).create({ 'name': 'Hol51', 'employee_id': self.employee_hrmanager_2_id, @@ -338,8 +337,7 @@ class TestHolidaysFlow(TestHrHolidaysBase): 'number_of_days_temp': 1, }) - with self.assertRaises(UserError): - hol51.action_approve() + hol51.action_approve() # Unless there is not manager above hol52 = HolidaysEmployeeGroup.sudo(self.user_hrmanager_id).create({