From 8f3a5068ca6c5e65fb97b968701c5c883eb5c132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20Lef=C3=A8vre?= Date: Fri, 2 Aug 2019 15:02:39 +0200 Subject: [PATCH 1/3] [IMP] hr_holidays: Add specific approval for allocations Purpose ======= We want to allow different approvals for allocations and leaves of a same type. e.g. In case of compensation days, a manager would approve the allocation request, but would later not approve leave requests. Specification ============= Allocation modes ---------------- No change here except renames. 1. No limit (employee doesn't need an allocation to request a leave) 2. Allow employee requests 3. Fixed by Time Off Officer Allocation approval ------------------- A radio field to choose the approval mechanism appears if the employee is allowed to request an allocation. The following approval modes are possible: 1. Time Off Officer 2. Employee's Time Off Manager (leave_responsible_id) 3. Employee's Time Off Manager and Time Off Officer These options are the same as leave approval options (except the missing "No Validation"). If option 1 or 2 is selected, a o2m field should allow to select which Time Off Officer is responsible for approving those allocations. Allocation approval policy is the same as the leave approval policy. Task id: 1936742 Access rights based on task 1950998 --- addons/hr_holidays/data/hr_holidays_data.xml | 9 +- addons/hr_holidays/data/hr_holidays_demo.xml | 3 +- addons/hr_holidays/models/hr_leave.py | 24 +- .../hr_holidays/models/hr_leave_allocation.py | 29 +-- addons/hr_holidays/models/hr_leave_type.py | 26 +- .../security/hr_holidays_security.xml | 43 +++- addons/hr_holidays/tests/__init__.py | 1 + .../hr_holidays/tests/test_access_rights.py | 20 +- .../tests/test_allocation_access_rights.py | 243 ++++++++++++++++++ .../tests/test_automatic_leave_dates.py | 3 +- .../tests/test_change_department.py | 1 - .../hr_holidays/tests/test_holidays_flow.py | 11 +- .../hr_holidays/tests/test_leave_requests.py | 10 +- .../hr_holidays/views/hr_leave_type_views.xml | 23 +- 14 files changed, 360 insertions(+), 86 deletions(-) create mode 100644 addons/hr_holidays/tests/test_allocation_access_rights.py diff --git a/addons/hr_holidays/data/hr_holidays_data.xml b/addons/hr_holidays/data/hr_holidays_data.xml index 47383587686..ca89cd2ef43 100644 --- a/addons/hr_holidays/data/hr_holidays_data.xml +++ b/addons/hr_holidays/data/hr_holidays_data.xml @@ -5,7 +5,8 @@ Paid Time Off fixed - both + both + both black @@ -27,7 +28,8 @@ Compensatory Days fixed_allocation - both + manager + manager hour lavender @@ -39,7 +41,8 @@ Unpaid no - both + both + both brown hour diff --git a/addons/hr_holidays/data/hr_holidays_demo.xml b/addons/hr_holidays/data/hr_holidays_demo.xml index 6a78a978ef0..f0f04cc9b03 100644 --- a/addons/hr_holidays/data/hr_holidays_demo.xml +++ b/addons/hr_holidays/data/hr_holidays_demo.xml @@ -11,7 +11,8 @@ Parental Time Off fixed brown - both + both + both diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index 1710dd47264..d11575a8f12 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -77,7 +77,7 @@ class HolidaysRequest(models.Model): lt = LeaveType.search([('valid', '=', True)], limit=1) defaults['holiday_status_id'] = lt.id if lt else defaults.get('holiday_status_id') - defaults['state'] = 'confirm' if lt and lt.validation_type != 'no_validation' else 'draft' + defaults['state'] = 'confirm' if lt and lt.leave_validation_type != 'no_validation' else 'draft' return defaults def _default_employee(self): @@ -126,7 +126,7 @@ class HolidaysRequest(models.Model): "hr.leave.type", string="Time Off Type", required=True, readonly=True, states={'draft': [('readonly', False)], 'confirm': [('readonly', False)]}, domain=[('valid', '=', True)]) - validation_type = fields.Selection('Validation Type', related='holiday_status_id.validation_type', readonly=False) + validation_type = fields.Selection('Validation Type', related='holiday_status_id.leave_validation_type', readonly=False) # HR data employee_id = fields.Many2one( 'hr.employee', string='Employee', index=True, readonly=True, ondelete="restrict", @@ -501,7 +501,7 @@ class HolidaysRequest(models.Model): def _compute_can_approve(self): for holiday in self: try: - if holiday.state == 'confirm' and holiday.holiday_status_id.validation_type == 'both': + if holiday.state == 'confirm' and holiday.validation_type == 'both': holiday._check_approval_update('validate1') else: holiday._check_approval_update('validate') @@ -637,11 +637,11 @@ class HolidaysRequest(models.Model): values.update({'department_id': self.env['hr.employee'].browse(employee_id).department_id.id}) # Handle no_validation - if leave_type.validation_type == 'no_validation': + if leave_type.leave_validation_type == 'no_validation': values.update({'state': 'confirm'}) # Handle double validation - if leave_type.validation_type == 'both': + if leave_type.leave_validation_type == 'both': self._check_double_validation_rules(employee_id, values.get('state', False)) holiday = super(HolidaysRequest, self.with_context(mail_create_nosubscribe=True)).create(values) @@ -662,7 +662,7 @@ class HolidaysRequest(models.Model): holiday_sudo.add_follower(employee_id) if holiday.validation_type == 'manager': holiday_sudo.message_subscribe(partner_ids=holiday.employee_id.leave_manager_id.partner_id.ids) - if leave_type.validation_type == 'no_validation': + if holiday.validation_type == 'no_validation': # Automatic validation should be done in sudo, because user might not have the rights to do it by himself holiday_sudo.action_validate() holiday_sudo.message_subscribe(partner_ids=[holiday._get_responsible_for_approval().partner_id.id]) @@ -924,16 +924,16 @@ class HolidaysRequest(models.Model): is_manager = self.env.user.has_group('hr_holidays.group_hr_holidays_manager') for holiday in self: - val_type = holiday.holiday_status_id.validation_type + val_type = holiday.validation_type if not is_manager and state != 'confirm': if state == 'draft': if holiday.state == 'refuse': - raise UserError(_('Only a Leave Manager can reset a refused leave.')) + raise UserError(_('Only a Time Off Manager can reset a refused leave.')) if holiday.date_from.date() <= fields.Date.today(): - raise UserError(_('Only a Leave Manager can reset a started leave.')) + raise UserError(_('Only a Time Off Manager can reset a started leave.')) if holiday.employee_id != current_employee: - raise UserError(_('Only a Leave Manager can reset other people leaves.')) + raise UserError(_('Only a Time Off Manager can reset other people leaves.')) else: if val_type == 'no_validation' and current_employee == holiday.employee_id: continue @@ -942,11 +942,11 @@ class HolidaysRequest(models.Model): # This handles states validate1 validate and refuse if holiday.employee_id == current_employee: - raise UserError(_('Only a Leave Manager can approve/refuse its own requests.')) + raise UserError(_('Only a Time Off Manager can approve/refuse its own requests.')) if (state == 'validate1' and val_type == 'both') or (state == 'validate' and val_type == 'manager') and holiday.holiday_type == 'employee': if not is_officer and self.env.user != holiday.employee_id.leave_manager_id: - raise UserError(_('You must be either %s\'s manager or Leave manager to approve this leave') % (holiday.employee_id.name)) + raise UserError(_('You must be either %s\'s manager or Time off Manager to approve this leave') % (holiday.employee_id.name)) # ------------------------------------------------------------ # Activity methods diff --git a/addons/hr_holidays/models/hr_leave_allocation.py b/addons/hr_holidays/models/hr_leave_allocation.py index 4ef3219f7b8..20a878dc0a2 100644 --- a/addons/hr_holidays/models/hr_leave_allocation.py +++ b/addons/hr_holidays/models/hr_leave_allocation.py @@ -86,11 +86,11 @@ class HolidaysAllocation(models.Model): linked_request_ids = fields.One2many('hr.leave.allocation', 'parent_id', string='Linked Requests') 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 allocation') + help='This area is automatically filled by the user who validates the allocation') second_approver_id = fields.Many2one( 'hr.employee', string='Second Approval', readonly=True, copy=False, - help='This area is automatically filled by the user who validate the allocation with second level (If allocation type need second validation)') - validation_type = fields.Selection('Validation Type', related='holiday_status_id.validation_type', readonly=True) + help='This area is automaticly filled by the user who validates the allocation with second level (If allocation type need second validation)') + validation_type = fields.Selection('Validation Type', related='holiday_status_id.allocation_validation_type', readonly=True) can_reset = fields.Boolean('Can reset', compute='_compute_can_reset') can_approve = fields.Boolean('Can Approve', compute='_compute_can_approve') type_request_unit = fields.Selection(related='holiday_status_id.request_unit', readonly=True) @@ -251,7 +251,7 @@ class HolidaysAllocation(models.Model): def _compute_can_approve(self): for allocation in self: try: - if allocation.state == 'confirm' and allocation.holiday_status_id.validation_type == 'both': + if allocation.state == 'confirm' and allocation.validation_type == 'both': allocation._check_approval_update('validate1') else: allocation._check_approval_update('validate') @@ -489,7 +489,7 @@ class HolidaysAllocation(models.Model): ).create(self._prepare_holiday_values(employee)) # TODO is it necessary to interleave the calls? childs.action_approve() - if childs and self.holiday_status_id.validation_type == 'both': + if childs and self.validation_type == 'both': childs.action_validate() return childs @@ -518,7 +518,7 @@ class HolidaysAllocation(models.Model): is_officer = self.env.user.has_group('hr_holidays.group_hr_holidays_user') is_manager = self.env.user.has_group('hr_holidays.group_hr_holidays_manager') for holiday in self: - val_type = holiday.holiday_status_id.validation_type + val_type = holiday.holiday_status_id.allocation_validation_type if state == 'confirm': continue @@ -541,12 +541,12 @@ class HolidaysAllocation(models.Model): if self.env.user == holiday.employee_id.leave_manager_id and self.env.user != holiday.employee_id.user_id: continue manager = holiday.employee_id.parent_id or holiday.employee_id.department_id.manager_id - if (manager and manager != current_employee) and not self.env.user.has_group('hr_holidays.group_hr_holidays_manager'): + if (manager != current_employee) and not is_manager: raise UserError(_('You must be either %s\'s manager or time off manager to approve this time off') % (holiday.employee_id.name)) if state == 'validate' and val_type == 'both': - if not self.env.user.has_group('hr_holidays.group_hr_holidays_manager'): - raise UserError(_('Only a Time off Manager can apply the second approval on allocation requests.')) + if not is_officer: + raise UserError(_('Only a Time off Approver can apply the second approval on allocation requests.')) # ------------------------------------------------------------ # Activity methods @@ -554,15 +554,14 @@ class HolidaysAllocation(models.Model): def _get_responsible_for_approval(self): self.ensure_one() - responsible = self.env['res.users'] + responsible = self.env.user - if self.validation_type == 'hr' or (self.validation_type == 'both' and self.state == 'validate1'): + 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.holiday_status_id.responsible_id: responsible = self.holiday_status_id.responsible_id - if self.state == 'confirm' and self.employee_id.parent_id.user_id: - responsible = self.employee_id.parent_id.user_id - elif self.department_id.manager_id.user_id: - responsible = self.department_id.manager_id.user_id return responsible diff --git a/addons/hr_holidays/models/hr_leave_type.py b/addons/hr_holidays/models/hr_leave_type.py index 1357371b1dc..f3c3b925062 100644 --- a/addons/hr_holidays/models/hr_leave_type.py +++ b/addons/hr_holidays/models/hr_leave_type.py @@ -70,21 +70,25 @@ class HolidaysType(models.Model): company_id = fields.Many2one('res.company', string='Company', default=lambda self: self.env.company) responsible_id = fields.Many2one('res.users', 'Responsible', domain=lambda self: [('groups_id', 'in', self.env.ref('hr_holidays.group_hr_holidays_user').id)], - help="This user will be responsible for approving this type of times off" + help="This user will be responsible for approving this type of times off. " "This is only used when validation is 'hr' or 'both'",) - validation_type = fields.Selection([ + leave_validation_type = fields.Selection([ ('no_validation', 'No Validation'), - ('hr', 'Time Off Officer'), - ('manager', 'Team Leader'), - ('both', 'Team Leader and Time Off Officer')], default='hr', string='Validation') + ('hr', 'By Time Off Officer'), + ('manager', "By Employee's Manager"), + ('both', "By Employee's Manager and Time Off Officer")], default='hr', string='Leave Validation') + allocation_validation_type = fields.Selection([ + ('hr', 'By Time Off Officer'), + ('manager', "By Employee's Manager"), + ('both', "By Employee's Manager and Time Off Officer")], default='manager', string='Allocation Validation') allocation_type = fields.Selection([ - ('no', 'No Allocation Needed'), - ('fixed_allocation', 'Free Allocation Request'), - ('fixed', 'Allocated by HR only')], + ('no', 'No Limit'), + ('fixed_allocation', 'Allow Employees Requests'), + ('fixed', 'Fixed by Time Off Officer')], default='no', string='Mode', - help='\tNo Allocation Needed: no allocation by default, users can freely request time off;' - '\tFree Allocation Request: allocated by HR and users can request time off and allocations;' - '\tAllocated by HR only: allocated by HR and cannot be bypassed; users can request time off;') + help='\tNo Limit: no allocation by default, users can freely request time off; ' + '\tAllow Employees Requests: allocated by HR and users can request time off and allocations; ' + '\tFixed by Time Off Officer: allocated by HR and cannot be bypassed; users can request time off;') validity_start = fields.Date("From", help='Adding validity to types of time off so that it cannot be selected outside this time period') validity_stop = fields.Date("To") diff --git a/addons/hr_holidays/security/hr_holidays_security.xml b/addons/hr_holidays/security/hr_holidays_security.xml index 387a3516d1e..f416e939808 100644 --- a/addons/hr_holidays/security/hr_holidays_security.xml +++ b/addons/hr_holidays/security/hr_holidays_security.xml @@ -98,7 +98,7 @@ [ '|', ('employee_id.leave_manager_id', '=', user.id), - ('employee_id.user_id', '=', user.id) + ('employee_id.user_id', '=', user.id), ] @@ -107,19 +107,21 @@ - Allocations: employee: create/write/unlink not validated own + Allocations: base.group_user create/write - - [('holiday_type', '=', 'employee'), + [ + ('holiday_status_id.allocation_type', '=', 'fixed_allocation'), + ('holiday_type', '=', 'employee'), '|', '&', ('employee_id.user_id', '=', user.id), ('state', 'not in', ['validate', 'validate1']), '&', - ('validation_type', 'in', ['manager', 'both', 'no_validation']), - ('employee_id.leave_manager_id', '=', user.id),] - + ('validation_type', 'in', ['manager', 'both']), + ('employee_id.leave_manager_id', '=', user.id), + ] + @@ -133,12 +135,29 @@ - - Allocations: see all time off: create/write/unlink own + managed employee + without department + same or managed department + + Allocations base.group_user unlink - ['|', -'|', ('employee_id.user_id', '=', user.id), ('employee_id.parent_id.user_id', '=', user.id), -'|', '|', ('department_id', '=', False), ('department_id.member_ids.user_id', 'in', [user.id]), ('department_id.manager_id.user_id', '=', user.id)] + [('employee_id.user_id', '=', user.id), ('state', '=', 'draft')] + + + + + + + + Allocations: holiday user : create/write + + [ + ('holiday_type', '=', 'employee'), + '|', + '&', + ('employee_id.user_id', '=', user.id), + ('state', '!=', 'validate'), + '|', + ('employee_id.user_id', '!=', user.id), + ('employee_id.user_id', '=', False) + ] diff --git a/addons/hr_holidays/tests/__init__.py b/addons/hr_holidays/tests/__init__.py index 3c15cd779a1..bc355646af0 100644 --- a/addons/hr_holidays/tests/__init__.py +++ b/addons/hr_holidays/tests/__init__.py @@ -3,6 +3,7 @@ from . import test_access_rights from . import test_automatic_leave_dates +from . import test_allocation_access_rights from . import test_holidays_flow from . import test_hr_leave_type from . import test_accrual_allocations diff --git a/addons/hr_holidays/tests/test_access_rights.py b/addons/hr_holidays/tests/test_access_rights.py index 94bf1c9beca..1a8632f51a9 100644 --- a/addons/hr_holidays/tests/test_access_rights.py +++ b/addons/hr_holidays/tests/test_access_rights.py @@ -16,7 +16,7 @@ class TestHrHolidaysAccessRightsCommon(TestHrHolidaysCommon): super(TestHrHolidaysAccessRightsCommon, self).setUp() self.leave_type = self.env['hr.leave.type'].create({ 'name': 'Unlimited', - 'validation_type': 'hr', + 'leave_validation_type': 'hr', 'allocation_type': 'no', 'validity_start': False, }) @@ -35,28 +35,28 @@ class TestHrHolidaysAccessRightsCommon(TestHrHolidaysCommon): self.lt_no_validation = self.env['hr.leave.type'].create({ 'name': 'Validation = no_validation', - 'validation_type': 'hr', + 'leave_validation_type': 'hr', 'allocation_type': 'no', 'validity_start': False, }) self.lt_validation_hr = self.env['hr.leave.type'].create({ 'name': 'Validation = HR', - 'validation_type': 'hr', + 'leave_validation_type': 'hr', 'allocation_type': 'no', 'validity_start': False, }) self.lt_validation_manager = self.env['hr.leave.type'].create({ 'name': 'Validation = manager', - 'validation_type': 'hr', + 'leave_validation_type': 'hr', 'allocation_type': 'no', 'validity_start': False, }) self.lt_validation_both = self.env['hr.leave.type'].create({ 'name': 'Validation = both', - 'validation_type': 'hr', + 'leave_validation_type': 'hr', 'allocation_type': 'no', 'validity_start': False, }) @@ -670,7 +670,7 @@ class TestAccessRightsWrite(TestHrHolidaysAccessRightsCommon): @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') def test_leave_manager_to_validate_by_user(self): """ A simple user can validate in manager mode if he is leave_manager_id """ - self.leave_type.write({'validation_type': 'manager'}) + self.leave_type.write({'leave_validation_type': 'manager'}) values = { 'name': 'Hol HrUser', 'employee_id': self.employee_hruser_id, @@ -688,7 +688,7 @@ class TestAccessRightsWrite(TestHrHolidaysAccessRightsCommon): @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') def test_leave_manager_to_validate_by_holiday_user(self): """ A holiday user can validate in manager mode """ - self.leave_type.write({'validation_type': 'manager'}) + self.leave_type.write({'leave_validation_type': 'manager'}) values = { 'name': 'Hol HrUser', 'employee_id': self.employee_emp_id, @@ -704,7 +704,7 @@ class TestAccessRightsWrite(TestHrHolidaysAccessRightsCommon): @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') def test_leave_double_validate(self): - self.leave_type.write({'validation_type': 'both'}) + self.leave_type.write({'leave_validation_type': 'both'}) values = { 'name': 'double HrManager', 'employee_id': self.employee_hrmanager_id, @@ -728,7 +728,7 @@ class TestAccessRightsWrite(TestHrHolidaysAccessRightsCommon): @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') def test_leave_double_validate_holiday_manager(self): - self.leave_type.write({'validation_type': 'both'}) + self.leave_type.write({'leave_validation_type': 'both'}) values = { 'name': 'double HrManager', 'employee_id': self.employee_emp_id, @@ -780,7 +780,7 @@ class TestMultiCompany(TestHrHolidaysCommon): self.leave_type = self.env['hr.leave.type'].create({ 'name': 'Unlimited - Company New', 'company_id': self.new_company.id, - 'validation_type': 'hr', + 'leave_validation_type': 'hr', 'allocation_type': 'no', }) self.rd_dept.manager_id = False diff --git a/addons/hr_holidays/tests/test_allocation_access_rights.py b/addons/hr_holidays/tests/test_allocation_access_rights.py new file mode 100644 index 00000000000..156b083e499 --- /dev/null +++ b/addons/hr_holidays/tests/test_allocation_access_rights.py @@ -0,0 +1,243 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import tests +from odoo.addons.hr_holidays.tests.common import TestHrHolidaysCommon +from odoo.exceptions import AccessError, UserError + + +@tests.tagged('access_rights', 'post_install', '-at_install') +class TestAllocationRights(TestHrHolidaysCommon): + + def setUp(self): + super().setUp() + self.rd_dept.manager_id = False + self.hr_dept.manager_id = False + self.employee_emp.parent_id = False + self.employee_emp.leave_manager_id = False + + self.lt_validation_hr = self.env['hr.leave.type'].create({ + 'name': 'Validation = HR', + 'allocation_validation_type': 'hr', + 'allocation_type': 'fixed_allocation', + 'validity_start': False, + }) + + self.lt_validation_manager = self.env['hr.leave.type'].create({ + 'name': 'Validation = manager', + 'allocation_validation_type': 'manager', + 'allocation_type': 'fixed_allocation', + 'validity_start': False, + }) + + self.lt_validation_both = self.env['hr.leave.type'].create({ + 'name': 'Validation = both', + 'allocation_validation_type': 'both', + 'allocation_type': 'fixed_allocation', + 'validity_start': False, + }) + + def request_allocation(self, user, values={}): + values = dict(values, **{ + 'name': 'Allocation', + 'number_of_days': 1, + }) + return self.env['hr.leave.allocation'].with_user(user).create(values) + + +class TestAccessRightsSimpleUser(TestAllocationRights): + + def test_simple_user_request_allocation(self): + """ A simple user can request an allocation but not approve it """ + values = { + 'employee_id': self.employee_emp.id, + 'holiday_status_id': self.lt_validation_hr.id, + } + allocation = self.request_allocation(self.user_employee.id, values) + with self.assertRaises(UserError): + allocation.action_approve() + + def test_simple_user_request_fixed_allocation(self): + """ A simple user cannot request an allocation if fixed by HR """ + self.lt_validation_hr.allocation_type = 'fixed' + values = { + 'employee_id': self.employee_emp.id, + 'holiday_status_id': self.lt_validation_hr.id, + } + with self.assertRaises(AccessError): + self.request_allocation(self.user_employee.id, values) + + def test_simple_user_reset_to_draft(self): + """ A simple user can reset to draft only his own allocation """ + values = { + 'employee_id': self.employee_emp.id, + 'holiday_status_id': self.lt_validation_hr.id, + } + allocation = self.request_allocation(self.user_employee.id, values) + self.assertEqual(allocation.state, 'confirm') + allocation.action_draft() + self.assertEqual(allocation.state, 'draft', "It should be reset to draft state") + + +class TestAccessRightsEmployeeManager(TestAllocationRights): + + def setUp(self): + super().setUp() + self.managed_employee = self.env['hr.employee'].create({ + 'name': 'Jolly Jumper', + 'leave_manager_id': self.user_employee.id, + }) + + def test_manager_request_allocation_other(self): + """ A manager cannot request and approve an allocation for employees he doesn't manage """ + values = { + 'employee_id': self.employee_hruser.id, + 'holiday_status_id': self.lt_validation_manager.id, + } + with self.assertRaises(AccessError): + self.request_allocation(self.user_employee.id, values) # user is not the employee's manager + + def test_manager_approve_request_allocation(self): + """ A manager can request and approve an allocation for managed employees """ + values = { + 'employee_id': self.managed_employee.id, + 'holiday_status_id': self.lt_validation_manager.id, + } + allocation = self.request_allocation(self.user_employee.id, values) + allocation.action_approve() + self.assertEqual(allocation.state, 'validate', "The allocation should be validated") + + def test_manager_refuse_request_allocation(self): + """ A manager can request and refuse an allocation for managed employees """ + values = { + 'employee_id': self.managed_employee.id, + 'holiday_status_id': self.lt_validation_manager.id, + } + allocation = self.request_allocation(self.user_employee.id, values) + allocation.action_refuse() + self.assertEqual(allocation.state, 'refuse', "The allocation should be validated") + + def test_manager_batch_allocation(self): + """ A manager cannot create batch allocation """ + values = { + 'holiday_status_id': self.lt_validation_manager.id, + 'holiday_type': 'company', + 'mode_company_id': self.user_employee.company_id.id, + } + with self.assertRaises(AccessError): + self.request_allocation(self.user_employee.id, values) + + def test_manager_approve_own(self): + """ A manager cannot approve his own allocation """ + values = { + 'employee_id': self.user_employee.employee_id.id, + 'holiday_status_id': self.lt_validation_manager.id, + } + allocation = self.request_allocation(self.user_employee.id, values) + with self.assertRaises(UserError): + allocation.action_approve() + + def test_manager_only_first_approval(self): + """ A manager can only do the first approval """ + values = { + 'employee_id': self.managed_employee.id, + 'holiday_status_id': self.lt_validation_both.id, + } + allocation = self.request_allocation(self.user_employee.id, values) + allocation.action_approve() + with self.assertRaises(UserError): + allocation.action_validate() + + +class TestAccessRightsHolidayUser(TestAllocationRights): + + def test_holiday_user_request_allocation(self): + """ A holiday user can request and approve an allocation for any employee """ + values = { + 'employee_id': self.employee_emp.id, + 'holiday_status_id': self.lt_validation_hr.id, + } + allocation = self.request_allocation(self.user_hruser.id, values) + allocation.action_approve() + 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 fixed by HR """ + self.lt_validation_hr.allocation_type = 'fixed' + values = { + 'employee_id': self.employee_emp.id, + 'holiday_status_id': self.lt_validation_hr.id, + } + allocation = self.request_allocation(self.user_hruser.id, values) + allocation.action_approve() + self.assertEqual(allocation.state, 'validate', "It should have been validated") + + def test_holiday_user_both_second_approval(self): + """ A holiday user can only do the second approval when double validation """ + values = { + 'employee_id': self.employee_emp.id, + 'holiday_status_id': self.lt_validation_both.id, + } + allocation = self.request_allocation(self.user_hruser.id, values) + with self.assertRaises(UserError): + allocation.action_approve() + allocation.sudo().action_approve() # First approval by someone else + 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 = { + 'holiday_status_id': self.lt_validation_hr.id, + 'holiday_type': 'company', + 'mode_company_id': self.user_employee.company_id.id, + } + with self.assertRaises(AccessError): + self.request_allocation(self.user_hruser.id, values) + + def test_holiday_user_cannot_approve_own(self): + """ A holiday user cannot approve his own allocation """ + values = { + 'employee_id': self.employee_hruser.id, + 'holiday_status_id': self.lt_validation_hr.id, + } + allocation = self.request_allocation(self.user_hruser.id, values) + with self.assertRaises(UserError): + allocation.action_approve() + + +class TestAccessRightsHolidayManager(TestAllocationRights): + + def test_holiday_manager_can_approve_own(self): + """ A holiday manager can approve his own allocation """ + values = { + 'employee_id': self.employee_hrmanager.id, + 'holiday_status_id': self.lt_validation_hr.id, + } + allocation = self.request_allocation(self.user_hrmanager.id, values) + allocation.action_approve() + self.assertEqual(allocation.state, 'validate', "It should have been validated") + + def test_holiday_manager_both_validation(self): + """ A holiday manager can perform both validation """ + values = { + 'employee_id': self.employee_emp.id, + 'holiday_status_id': self.lt_validation_both.id, + } + allocation = self.request_allocation(self.user_hrmanager.id, values) + allocation.action_approve() + self.assertEqual(allocation.state, 'validate1', "It should have been validated one time") + allocation.action_validate() + self.assertEqual(allocation.state, 'validate', "It should have been completely validated") + + def test_holiday_manager_refuse_validated(self): + """ A holiday manager can refuse a validated allocation """ + values = { + 'employee_id': self.employee_emp.id, + 'holiday_status_id': self.lt_validation_hr.id, + } + allocation = self.request_allocation(self.user_hrmanager.id, values) + allocation.action_approve() + self.assertEqual(allocation.state, 'validate', "It should have been validated") + allocation.action_refuse() + self.assertEqual(allocation.state, 'refuse', "It should have been refused") diff --git a/addons/hr_holidays/tests/test_automatic_leave_dates.py b/addons/hr_holidays/tests/test_automatic_leave_dates.py index d3f76ee547d..01a7d0b8183 100644 --- a/addons/hr_holidays/tests/test_automatic_leave_dates.py +++ b/addons/hr_holidays/tests/test_automatic_leave_dates.py @@ -1,12 +1,11 @@ # -*- coding: utf-8 -*- from datetime import date, datetime -from odoo.tests.common import Form, tagged +from odoo.tests.common import Form from odoo.addons.hr_holidays.tests.common import TestHrHolidaysCommon -@tagged('prout') class TestAutomaticLeaveDates(TestHrHolidaysCommon): def setUp(self): super(TestAutomaticLeaveDates, self).setUp() diff --git a/addons/hr_holidays/tests/test_change_department.py b/addons/hr_holidays/tests/test_change_department.py index 7698d920a65..83ebb1581ee 100644 --- a/addons/hr_holidays/tests/test_change_department.py +++ b/addons/hr_holidays/tests/test_change_department.py @@ -15,7 +15,6 @@ class TestChangeDepartment(TestHrHolidaysCommon): self.holidays_status_1 = HolidayStatusManagerGroup.create({ 'name': 'NotLimitedHR', 'allocation_type': 'no', - 'validation_type': 'hr', 'validity_start': False, }) diff --git a/addons/hr_holidays/tests/test_holidays_flow.py b/addons/hr_holidays/tests/test_holidays_flow.py index 61f18e5afd9..b63c4486a5d 100644 --- a/addons/hr_holidays/tests/test_holidays_flow.py +++ b/addons/hr_holidays/tests/test_holidays_flow.py @@ -30,13 +30,13 @@ class TestHolidaysFlow(TestHrHolidaysCommon): self.holidays_status_hr = HolidayStatusManagerGroup.create({ 'name': 'NotLimitedHR', 'allocation_type': 'no', - 'validation_type': 'hr', + 'leave_validation_type': 'hr', 'validity_start': False, }) self.holidays_status_manager = HolidayStatusManagerGroup.create({ 'name': 'NotLimitedManager', 'allocation_type': 'no', - 'validation_type': 'manager', + 'leave_validation_type': 'manager', 'validity_start': False, }) @@ -87,7 +87,7 @@ class TestHolidaysFlow(TestHrHolidaysCommon): holiday_status_paid_time_off = self.env['hr.leave.type'].create({ 'name': 'Paid Time Off', 'allocation_type': 'fixed', - 'validation_type': 'both', + 'leave_validation_type': 'both', 'validity_start': time.strftime('%Y-%m-01'), 'responsible_id': self.env.ref('base.user_admin').id, }) @@ -129,7 +129,8 @@ class TestHolidaysFlow(TestHrHolidaysCommon): self.holidays_status_limited = HolidayStatusManagerGroup.create({ 'name': 'Limited', 'allocation_type': 'fixed', - 'validation_type': 'both', + 'allocation_validation_type': 'both', + 'leave_validation_type': 'both', 'validity_start': False, }) HolidaysEmployeeGroup = Requests.with_user(self.user_employee_id) @@ -243,7 +244,7 @@ class TestHolidaysFlow(TestHrHolidaysCommon): holiday_status_paid_time_off = self.env['hr.leave.type'].create({ 'name': 'Paid Time Off', 'allocation_type': 'fixed', - 'validation_type': 'both', + 'leave_validation_type': 'both', 'validity_start': time.strftime('%Y-%m-01'), 'responsible_id': self.env.ref('base.user_admin').id, }) diff --git a/addons/hr_holidays/tests/test_leave_requests.py b/addons/hr_holidays/tests/test_leave_requests.py index 18354a2fa01..c4414640fe2 100644 --- a/addons/hr_holidays/tests/test_leave_requests.py +++ b/addons/hr_holidays/tests/test_leave_requests.py @@ -31,19 +31,19 @@ class TestLeaveRequests(TestHrHolidaysCommon): self.holidays_type_1 = LeaveType.create({ 'name': 'NotLimitedHR', 'allocation_type': 'no', - 'validation_type': 'hr', + 'leave_validation_type': 'hr', 'validity_start': False, }) self.holidays_type_2 = LeaveType.create({ 'name': 'Limited', - 'allocation_type': 'fixed', - 'validation_type': 'hr', + 'allocation_type': 'fixed_allocation', + 'leave_validation_type': 'hr', 'validity_start': False, }) self.holidays_type_3 = LeaveType.create({ 'name': 'TimeNotLimited', 'allocation_type': 'no', - 'validation_type': 'manager', + 'leave_validation_type': 'manager', 'validity_start': fields.Datetime.from_string('2017-01-01 00:00:00'), 'validity_stop': fields.Datetime.from_string('2017-06-01 00:00:00'), }) @@ -174,8 +174,8 @@ class TestLeaveRequests(TestHrHolidaysCommon): """ Create an allocation request """ # employee should be set to current user allocation_form = Form(self.env['hr.leave.allocation'].with_user(self.user_employee)) - allocation_form.holiday_status_id = self.holidays_type_1 allocation_form.name = 'New Allocation Request' + allocation_form.holiday_status_id = self.holidays_type_2 allocation = allocation_form.save() @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') diff --git a/addons/hr_holidays/views/hr_leave_type_views.xml b/addons/hr_holidays/views/hr_leave_type_views.xml index 291d8636df0..a69bd1b5a01 100644 --- a/addons/hr_holidays/views/hr_leave_type_views.xml +++ b/addons/hr_holidays/views/hr_leave_type_views.xml @@ -22,7 +22,7 @@
-