diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index b112c02b2df..35286c1fc9d 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -8,7 +8,7 @@ import math from datetime import timedelta from odoo import api, fields, models -from odoo.exceptions import UserError, ValidationError +from odoo.exceptions import AccessError, UserError, ValidationError from odoo.tools import float_compare from odoo.tools.translate import _ @@ -16,27 +16,47 @@ from odoo.addons.resource.models.resource import HOURS_PER_DAY _logger = logging.getLogger(__name__) -class HolidaysRequest(models.Model): - """ - Here are the rights associated with the flow of leave requests - State Groups Restriction - ============================================================================================================ - Draft Anybody Own Request only and state in [confirm, refuse] - Holiday Manager State in [confirm, refuse] - Confirm All State = draft - Validate1 Holiday Officer Not his own Request and state = confirm - Holiday Manager Not his own Request or has no manager and State = confirm - Validate Holiday Officer Not his own Request and state = confirm - Holiday Manager Not his own Request or has no manager and State in [validate1, confirm] - Refuse Holiday Officer State in [confirm, validate, validate1] - Holiday Manager State in [confirm, validate, validate1] - ============================================================================================================ +class HolidaysRequest(models.Model): + """ Leave Requests Access specifications + + - a regular employee / user + - can see all leaves; + - cannot see name field of leaves belonging to other user as it may contain + private information that we don't want to share to other people than + HR people; + - can modify only its own not validated leaves (except writing on state to + bypass approval); + - can discuss on its leave requests; + - can reset only its own leaves; + - cannot validate any leaves; + - an Officer + - can see all leaves; + - can validate "HR" single validation leaves from people if + - he is the employee manager; + - he is the department manager; + - he is member of the same department; + - target employee has no manager and no department manager; + - can validate "Manager" single validation leaves from people if + - he is the employee manager; + - he is the department manager; + - target employee has no manager and no department manager; + - can first validate "Both" double validation leaves from people like "HR" + single validation, moving the leaves to validate1 state; + - cannot validate its own leaves; + - can reset only its own leaves; + - can refuse all leaves; + - a Manager + - can do everything he wants + + On top of that multicompany rules apply based on company defined on the + leave request leave type. """ _name = "hr.leave" _description = "Leave" _order = "date_from desc" _inherit = ['mail.thread', 'mail.activity.mixin'] + _mail_post_access = 'read' def _default_domain_holiday_status(self): if self.user_has_groups('hr_holidays.group_hr_holidays_manager'): @@ -56,7 +76,6 @@ class HolidaysRequest(models.Model): def _default_employee(self): return self.env.context.get('default_employee_id') or self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1) - name = fields.Char('Description') state = fields.Selection([ ('draft', 'To Submit'), @@ -126,25 +145,28 @@ class HolidaysRequest(models.Model): holiday.number_of_days = -holiday.number_of_days_temp @api.multi + @api.depends('state', 'employee_id', 'department_id') def _compute_can_reset(self): - """ User can reset a leave request if it is its own leave request - or if he is an Hr Manager. - """ - user = self.env.user - group_hr_manager = self.env.ref('hr_holidays.group_hr_holidays_manager') for holiday in self: - if group_hr_manager in user.groups_id or holiday.employee_id and holiday.employee_id.user_id == user: + try: + holiday._check_approval_update('draft') + except (AccessError, UserError): + holiday.can_reset = False + else: holiday.can_reset = True - @api.depends('employee_id', 'department_id') + @api.depends('state', 'employee_id', 'department_id') def _compute_can_approve(self): - """ User can only approve a leave request if it is not his own - Exception : User is holiday manager and has no manager - """ for holiday in self: - # User is holiday manager and has no manager - 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 + try: + if holiday.state == 'confirm' and holiday.holiday_status_id.validation_type == 'both': + holiday._check_approval_update('validate1') + else: + holiday._check_approval_update('validate') + except (AccessError, UserError): + holiday.can_approve = False + else: + holiday.can_approve = True @api.onchange('holiday_type') def _onchange_type(self): @@ -264,8 +286,9 @@ class HolidaysRequest(models.Model): dto = fields.Datetime.from_string(leave.date_to) if dfrom and dto and (dfrom < vstart or dto > vstop): - raise UserError(_('You can take %s only between %s and %s') % (leave.holiday_status_id.display_name, \ - leave.holiday_status_id.validity_start, leave.holiday_status_id.validity_stop)) + raise UserError( + _('You can take %s only between %s and %s') % ( + leave.holiday_status_id.display_name, leave.holiday_status_id.validity_start, leave.holiday_status_id.validity_stop)) @api.model def create(self, values): @@ -280,9 +303,29 @@ class HolidaysRequest(models.Model): holiday.activity_update() return holiday + def _read_from_database(self, field_names, inherited_field_names=[]): + if 'name' in field_names and 'employee_id' not in field_names: + field_names.append('employee_id') + super(HolidaysRequest, self)._read_from_database(field_names, inherited_field_names) + if 'name' in field_names: + if self.user_has_groups('hr_holidays.group_hr_holidays_user'): + return + current_employee = self.env['hr.employee'].sudo().search([('user_id', '=', self.env.uid)], limit=1) + for record in self: + emp_id = record._cache.get('employee_id', [False])[0] + if emp_id != current_employee.id: + try: + record._cache['name'] + record._cache['name'] = '*****' + except Exception: + # skip SpecialValue (e.g. for missing record or access right) + pass + @api.multi def write(self, values): employee_id = values.get('employee_id', False) + if values.get('state'): + self._check_approval_update(values['state']) result = super(HolidaysRequest, self).write(values) self.add_follower(employee_id) if 'employee_id' in values: @@ -373,8 +416,6 @@ class HolidaysRequest(models.Model): @api.multi def action_draft(self): for holiday in self: - if not holiday.can_reset: - raise UserError(_('Only an HR Manager or the concerned employee can reset to draft.')) if holiday.state not in ['confirm', 'refuse']: raise UserError(_('Leave request state must be "Refused" or "To Approve" in order to reset to Draft.')) holiday.write({ @@ -401,22 +442,10 @@ class HolidaysRequest(models.Model): def action_approve(self): # if validation_type == 'both': this method is the first approval approval # if validation_type != 'both': this method calls action_validate() below - 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.')) - - current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1) if any(holiday.state != 'confirm' for holiday in self): raise UserError(_('Leave request must be confirmed ("To Approve") in order to approve it.')) - 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 ['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 == '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')) - + current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1) self.filtered(lambda hol: hol.validation_type == 'both').write({'state': 'validate1', 'first_approver_id': current_employee.id}) self.filtered(lambda hol: not hol.validation_type == 'both').action_validate() self.activity_update() @@ -424,18 +453,10 @@ class HolidaysRequest(models.Model): @api.multi def action_validate(self): - 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.')) - - if any(not holiday.can_approve for holiday in self): - raise UserError(_('Only your manager can approve your leave requests')) - 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.')) - if holiday.state == 'validate1' and not holiday.env.user.has_group('hr_holidays.group_hr_holidays_manager'): - raise UserError(_('Only an HR Manager can apply the second approval on leave requests.')) holiday.write({'state': 'validate'}) if holiday.validation_type == 'both': @@ -459,9 +480,6 @@ class HolidaysRequest(models.Model): @api.multi def action_refuse(self): - 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.')) - 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']: @@ -480,6 +498,40 @@ class HolidaysRequest(models.Model): self.activity_update() return True + def _check_approval_update(self, state): + """ Check if target state is achievable. """ + current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1) + 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 + if state == 'confirm': + continue + + if state == 'draft': + if holiday.employee_id != current_employee and not is_manager: + raise UserError(_('Only a Leave Manager can reset other people leaves.')) + continue + + if not is_officer: + raise UserError(_('Only a Leave Officer or Manager can approve or refuse leave requests.')) + + if is_officer: + # use ir.rule based first access check: department, members, ... (see security.xml) + holiday.check_access_rule('write') + + if holiday.employee_id == current_employee and not is_manager: + raise UserError(_('Only a Leave Manager can approve its own requests.')) + + if (state == 'validate1' and val_type == 'both') or (state == 'validate' and val_type == 'manager'): + 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'): + raise UserError(_('You must be either %s\'s manager or Leave manager to approve this leave') % (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 an Leave Manager can apply the second approval on leave requests.')) + # ------------------------------------------------------------ # Activity methods # ------------------------------------------------------------ diff --git a/addons/hr_holidays/models/hr_leave_allocation.py b/addons/hr_holidays/models/hr_leave_allocation.py index 17cfddd53a9..7c6f848d710 100644 --- a/addons/hr_holidays/models/hr_leave_allocation.py +++ b/addons/hr_holidays/models/hr_leave_allocation.py @@ -6,32 +6,18 @@ import logging from odoo import api, fields, models -from odoo.exceptions import UserError +from odoo.exceptions import AccessError, UserError from odoo.tools.translate import _ _logger = logging.getLogger(__name__) class HolidaysAllocation(models.Model): - """ - Here are the rights associated with the flow of allocation requests - - State Groups Restriction - ============================================================================================================ - Draft Anybody Own Allocation only and state in [confirm, refuse] - Holiday Manager State in [confirm, refuse] - Confirm All State = draft - Validate1 Holiday Officer Not his own Allocation and state = confirm - Holiday Manager Not his own Allocation or has no manager and State = confirm - Validate Holiday Officer Not his own Allocation and state = confirm - Holiday Manager Not his own Allocation or has no manager and State in [validate1, confirm] - Refuse Holiday Officer State in [confirm, validate, validate1] - Holiday Manager State in [confirm, validate, validate1] - ============================================================================================================ - """ + """ Allocation Requests Access specifications: similar to leave requests """ _name = "hr.leave.allocation" _description = "Leaves Allocation" _inherit = ['mail.thread', 'mail.activity.mixin'] + _mail_post_access = 'read' def _default_domain_holiday_status_id(self): if self.user_has_groups('hr_holidays.group_hr_holidays_manager'): @@ -103,25 +89,28 @@ class HolidaysAllocation(models.Model): holiday.number_of_days = holiday.number_of_days_temp @api.multi + @api.depends('state', 'employee_id', 'department_id') def _compute_can_reset(self): - """ User can reset a leave request if it is its own leave request - or if he is an Hr Manager. - """ - user = self.env.user - group_hr_manager = self.env.ref('hr_holidays.group_hr_holidays_manager') - for holiday in self: - if group_hr_manager in user.groups_id or holiday.employee_id and holiday.employee_id.user_id == user: - holiday.can_reset = True + for allocation in self: + try: + allocation._check_approval_update('draft') + except (AccessError, UserError): + allocation.can_reset = False + else: + allocation.can_reset = True - @api.depends('employee_id', 'department_id') + @api.depends('state', 'employee_id', 'department_id') def _compute_can_approve(self): - """ User can only approve a leave request if it is not his own - Exception : User is holiday manager and has no manager - """ - for holiday in self: - # User is holiday manager and has no manager - 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 + for allocation in self: + try: + if allocation.state == 'confirm' and allocation.holiday_status_id.validation_type == 'both': + allocation._check_approval_update('validate1') + else: + allocation._check_approval_update('validate') + except (AccessError, UserError): + allocation.can_approve = False + else: + allocation.can_approve = True @api.onchange('holiday_type') def _onchange_type(self): @@ -185,6 +174,8 @@ class HolidaysAllocation(models.Model): @api.multi def write(self, values): employee_id = values.get('employee_id', False) + if values.get('state'): + self._check_approval_update(values['state']) result = super(HolidaysAllocation, self).write(values) self.add_follower(employee_id) return result @@ -220,8 +211,6 @@ class HolidaysAllocation(models.Model): @api.multi def action_draft(self): for holiday in self: - if not holiday.can_reset: - raise UserError(_('Only an HR Manager or the concerned employee can reset to draft.')) if holiday.state not in ['confirm', 'refuse']: raise UserError(_('Leave request state must be "Refused" or "To Approve" in order to reset to Draft.')) holiday.write({ @@ -248,21 +237,10 @@ class HolidaysAllocation(models.Model): def action_approve(self): # if validation_type == 'both': this method is the first approval approval # if validation_type != 'both': this method calls action_validate() below - 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.')) - - current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1) if any(holiday.state != 'confirm' for holiday in self): raise UserError(_('Leave request must be confirmed ("To Approve") in order to approve it.')) - 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 ['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 == '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')) + current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1) self.filtered(lambda hol: hol.validation_type == 'both').write({'state': 'validate1', 'first_approver_id': current_employee.id}) self.filtered(lambda hol: not hol.validation_type == 'both').action_validate() @@ -271,18 +249,10 @@ class HolidaysAllocation(models.Model): @api.multi def action_validate(self): - 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.')) - - if any(not holiday.can_approve for holiday in self): - raise UserError(_('Only your manager can approve your allocations')) - 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.')) - if holiday.state == 'validate1' and not holiday.env.user.has_group('hr_holidays.group_hr_holidays_manager'): - raise UserError(_('Only an HR Manager can apply the second approval on leave requests.')) holiday.write({'state': 'validate'}) if holiday.validation_type == 'both': @@ -304,9 +274,6 @@ class HolidaysAllocation(models.Model): @api.multi def action_refuse(self): - 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.')) - 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']: @@ -321,6 +288,40 @@ class HolidaysAllocation(models.Model): self.activity_update() return True + def _check_approval_update(self, state): + """ Check if target state is achievable. """ + current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1) + 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 + if state == 'confirm': + continue + + if state == 'draft': + if holiday.employee_id != current_employee and not is_manager: + raise UserError(_('Only a Leave Manager can reset other people leaves.')) + continue + + if not is_officer: + raise UserError(_('Only a Leave Officer or Manager can approve or refuse leave requests.')) + + if is_officer: + # use ir.rule based first access check: department, members, ... (see security.xml) + holiday.check_access_rule('write') + + if holiday.employee_id == current_employee and not is_manager: + raise UserError(_('Only a Leave Manager can approve its own requests.')) + + if (state == 'validate1' and val_type == 'both') or (state == 'validate' and val_type == 'manager'): + 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'): + raise UserError(_('You must be either %s\'s manager or Leave manager to approve this leave') % (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 an Leave Manager can apply the second approval on leave requests.')) + # ------------------------------------------------------------ # Activity methods # ------------------------------------------------------------ diff --git a/addons/hr_holidays/report/hr_leave_report.py b/addons/hr_holidays/report/hr_leave_report.py index 3e918920c52..50f4c1a69ca 100644 --- a/addons/hr_holidays/report/hr_leave_report.py +++ b/addons/hr_holidays/report/hr_leave_report.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import fields, models, tools +from odoo import api, fields, models, tools class LeaveReport(models.Model): @@ -78,3 +78,21 @@ class LeaveReport(models.Model): from hr_leave as request) leaves ); """) + + def _read_from_database(self, field_names, inherited_field_names=[]): + if 'name' in field_names and 'employee_id' not in field_names: + field_names.append('employee_id') + super(LeaveReport, self)._read_from_database(field_names, inherited_field_names) + if 'name' in field_names: + if self.user_has_groups('hr_holidays.group_hr_holidays_user'): + return + current_employee = self.env['hr.employee'].sudo().search([('user_id', '=', self.env.uid)], limit=1) + for record in self: + emp_id = record._cache.get('employee_id', [False])[0] + if emp_id != current_employee.id: + try: + record._cache['name'] + record._cache['name'] = '*****' + except Exception: + # skip SpecialValue (e.g. for missing record or access right) + pass diff --git a/addons/hr_holidays/security/hr_holidays_security.xml b/addons/hr_holidays/security/hr_holidays_security.xml index 5b5c4eeb8da..7e5f51aa67b 100644 --- a/addons/hr_holidays/security/hr_holidays_security.xml +++ b/addons/hr_holidays/security/hr_holidays_security.xml @@ -18,64 +18,96 @@ - Employee Leaves + Leaves: employee: read all - [('employee_id.user_id','=',user.id)] + [(1, '=', 1)] - - Employee Leaves Create, Write, Unlink + + Leaves: employee: create/write/unlink not validated own [('employee_id.user_id','=',user.id), ('state', 'in', ['draft', 'confirm', 'cancel', 'refuse'])] - - Leaves Officer + + Leaves: officer: read all - ['|','|','|', ('employee_id.user_id', '=', user.id), ('department_id', '=', False), ('department_id.manager_id', '=', False), ('department_id.manager_id.user_id', '=', user.id)] - + [(1, '=', 1)] + + + + + + + + Leaves: officer: create/write/unlink own + managed employee + without department + same or managed department + + ['|', +'|', ('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)] + + - Manager Leaves + Leaves: manager: no limit [(1, '=', 1)] + + Leaves: multi company global rule + + + ['|', ('holiday_status_id.company_id', '=', False), ('holiday_status_id.company_id', 'in', [user.company_id.id])] + + - Employee Allocations + Allocations: employee: read own - [('employee_id.user_id','=',user.id)] + [('employee_id.user_id', '=', user.id)] - - Employee Allocations Create, Write, Unlink + + Allocations: employee: create/write/unlink not validated own [('employee_id.user_id','=',user.id), ('state', 'in', ['draft', 'confirm', 'cancel', 'refuse'])] - - Leaves Officer + + Allocations: officer: read all - ['|','|','|', ('employee_id.user_id', '=', user.id), ('department_id', '=', False), ('department_id.manager_id', '=', False), ('department_id.manager_id.user_id', '=', user.id)] + [(1, '=', 1)] + + + + + + + + Allocations: officer: create/write/unlink own + managed employee + without department + same or managed department + + ['|', +'|', ('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)] - Manager Allocations + Allocations: manager: no limit [(1, '=', 1)] diff --git a/addons/hr_holidays/tests/__init__.py b/addons/hr_holidays/tests/__init__.py index 1300f22a091..a98044890a5 100644 --- a/addons/hr_holidays/tests/__init__.py +++ b/addons/hr_holidays/tests/__init__.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from . import test_access_rights from . import test_holidays_flow from . import test_hr_leave_type diff --git a/addons/hr_holidays/tests/common.py b/addons/hr_holidays/tests/common.py index 277d5c446c5..467eff2037c 100644 --- a/addons/hr_holidays/tests/common.py +++ b/addons/hr_holidays/tests/common.py @@ -15,24 +15,27 @@ class TestHrHolidaysBase(common.TransactionCase): group_employee_id = self.ref('base.group_user') # Test users to use through the various tests - self.user_hruser_id = Users.create({ + self.user_hruser = Users.create({ 'name': 'Armande HrUser', 'login': 'Armande', 'email': 'armande.hruser@example.com', 'groups_id': [(6, 0, [group_employee_id, self.ref('hr_holidays.group_hr_holidays_user')])] - }).id - self.user_hrmanager_id = Users.create({ + }) + self.user_hruser_id = self.user_hruser.id + self.user_hrmanager = Users.create({ 'name': 'Bastien HrManager', 'login': 'bastien', 'email': 'bastien.hrmanager@example.com', 'groups_id': [(6, 0, [group_employee_id, self.ref('hr_holidays.group_hr_holidays_manager')])] }).id - self.user_employee_id = Users.create({ + self.user_hrmanager_id = self.user_hrmanager + self.user_employee = Users.create({ 'name': 'David Employee', 'login': 'david', 'email': 'david.employee@example.com', 'groups_id': [(6, 0, [group_employee_id])] - }).id + }) + self.user_employee_id = self.user_employee.id self.user_hrmanager_2_id = Users.create({ 'name': 'Florence HrManager', 'login': 'florence', @@ -50,21 +53,26 @@ class TestHrHolidaysBase(common.TransactionCase): 'name': 'Research and devlopment', }) - self.employee_emp_id = self.env['hr.employee'].create({ + self.employee_emp = self.env['hr.employee'].create({ 'name': 'David Employee', 'user_id': self.user_employee_id, 'department_id': self.rd_dept.id, - }).id - self.employee_hruser_id = self.env['hr.employee'].create({ + }) + self.employee_emp_id = self.employee_emp.id + + self.employee_hruser = self.env['hr.employee'].create({ 'name': 'Armande HrUser', 'user_id': self.user_hruser_id, 'department_id': self.rd_dept.id, - }).id - self.employee_hrmanager_id = self.env['hr.employee'].create({ + }) + self.employee_hruser_id = self.employee_hruser.id + + self.employee_hrmanager = self.env['hr.employee'].create({ 'name': 'Bastien HrManager', 'user_id': self.user_hrmanager_id, 'department_id': self.hr_dept.id, - }).id + }) + self.employee_hrmanager_id = self.employee_hrmanager.id self.employee_hrmanager_2_id = self.env['hr.employee'].create({ 'name': 'Florence HrManager', diff --git a/addons/hr_holidays/tests/test_access_rights.py b/addons/hr_holidays/tests/test_access_rights.py new file mode 100644 index 00000000000..0631fa10006 --- /dev/null +++ b/addons/hr_holidays/tests/test_access_rights.py @@ -0,0 +1,392 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from datetime import datetime +from dateutil.relativedelta import relativedelta + +from odoo import tests +from odoo.addons.hr_holidays.tests.common import TestHrHolidaysBase +from odoo.exceptions import AccessError, UserError +from odoo.tools import mute_logger + + +@tests.tagged('access_rights') +class TestLeavesRights(TestHrHolidaysBase): + + def setUp(self): + super(TestLeavesRights, self).setUp() + self.leave_type = self.env['hr.leave.type'].create({ + 'name': 'Unlimited', + 'limit': True, + 'validation_type': 'hr', + }) + self.rd_dept.manager_id = False + self.hr_dept.manager_id = False + self.employee_emp.parent_id = False + self.employee_leave = self.env['hr.leave'].sudo(self.user_employee_id).create({ + 'name': 'Test', + 'holiday_status_id': self.leave_type.id, + 'department_id': self.employee_emp.department_id.id, + 'employee_id': self.employee_emp.id, + 'date_from': datetime.now(), + 'date_to': datetime.now() + relativedelta(days=1), + 'number_of_days_temp': 1, + }) + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_messaging_by_user(self): + """ User may communicate on its own leaves, even if validated """ + self.employee_leave.sudo(self.user_employee_id).message_post( + body='I haz messaging', + subtype='mail.mt_comment', + message_type='comment' + ) + + self.employee_leave.sudo(self.user_hrmanager_id).action_approve() + + self.employee_leave.sudo(self.user_employee_id).message_post( + body='I still haz messaging', + subtype='mail.mt_comment', + message_type='comment' + ) + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_read_by_user_other(self): + """ Users should be able to read other people requests except name field """ + other_leave = self.env['hr.leave'].sudo(self.user_hruser).create({ + 'name': 'Test', + 'holiday_status_id': self.leave_type.id, + 'department_id': self.employee_hruser.department_id.id, + 'employee_id': self.employee_hruser.id, + 'date_from': datetime.now(), + 'date_to': datetime.now() + relativedelta(days=1), + 'number_of_days_temp': 1, + }) + res = other_leave.sudo(self.user_employee_id).read(['number_of_days_temp', 'state', 'name']) + self.assertEqual( + res[0]['name'], '*****', + 'Private information should have been stripped, received %s instead' % res[0]['name'] + ) + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_read_by_user_other_browse(self): + """ Users should be able to browse other people requests except name field """ + other_leave = self.env['hr.leave'].sudo(self.user_hruser).create({ + 'name': 'Test', + 'holiday_status_id': self.leave_type.id, + 'department_id': self.employee_hruser.department_id.id, + 'employee_id': self.employee_hruser.id, + 'date_from': datetime.now(), + 'date_to': datetime.now() + relativedelta(days=1), + 'number_of_days_temp': 1, + }) + self.assertEqual( + other_leave.sudo(self.user_employee_id).name, '*****', + 'Private information should have been stripped, received %s instead' % other_leave.sudo(self.user_employee_id).name + ) + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_read_by_user_own(self): + """ Users should be able to read name field of own requests """ + res = self.employee_leave.read(['name', 'number_of_days_temp', 'state']) + self.assertEqual(res[0]['name'], 'Test') + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_read_by_user_own_browse(self): + """ Users should be able to browse name field of own requests """ + self.assertEqual(self.employee_leave.state, 'confirm') + self.assertEqual(self.employee_leave.name, 'Test') + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_update_hr_by_user(self): + """ User may update its leave """ + self.employee_leave.sudo(self.user_employee_id).write({'name': 'Crocodile Dundee is my man'}) + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_update_hr_by_user_other(self): + """ User cannot update other people leaves """ + other_leave = self.env['hr.leave'].sudo(self.user_hruser).create({ + 'name': 'Test', + 'holiday_status_id': self.leave_type.id, + 'department_id': self.employee_hruser.department_id.id, + 'employee_id': self.employee_hruser.id, + 'date_from': datetime.now(), + 'date_to': datetime.now() + relativedelta(days=1), + 'number_of_days_temp': 1, + }) + with self.assertRaises(AccessError): + other_leave.sudo(self.user_employee_id).write({'name': 'Crocodile Dundee is my man'}) + + # ---------------------------------------- + # Reset + # ---------------------------------------- + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_reset_by_manager(self): + """ Manager resets its own leaves """ + manager_leave = self.env['hr.leave'].sudo(self.user_hrmanager).create({ + 'name': 'Test', + 'holiday_status_id': self.leave_type.id, + 'department_id': self.employee_hrmanager.department_id.id, + 'employee_id': self.employee_hrmanager.id, + 'date_from': datetime.now(), + 'date_to': datetime.now() + relativedelta(days=1), + 'number_of_days_temp': 1, + }) + manager_leave.sudo(self.user_hrmanager).action_draft() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_reset_by_manager_other(self): + """ Manager may not reset other leaves """ + self.employee_leave.sudo(self.user_hrmanager).action_draft() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_reset_by_officer(self): + """ Officer resets its own leaves """ + officer_leave = self.env['hr.leave'].sudo(self.user_hruser).create({ + 'name': 'Test', + 'holiday_status_id': self.leave_type.id, + 'department_id': self.employee_hruser.department_id.id, + 'employee_id': self.employee_hruser.id, + 'date_from': datetime.now(), + 'date_to': datetime.now() + relativedelta(days=1), + 'number_of_days_temp': 1, + }) + officer_leave.sudo(self.user_hruser).action_draft() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_reset_by_officer_other(self): + """ Officer may not reset other leaves """ + with self.assertRaises(UserError): + self.employee_leave.sudo(self.user_hruser).action_draft() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_reset_by_user(self): + """ User resets its own leaves """ + self.employee_leave.sudo(self.user_employee_id).action_draft() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_reset_by_user_other(self): + """ User may not reset other leaves """ + other_leave = self.env['hr.leave'].sudo(self.user_hruser).create({ + 'name': 'Test', + 'holiday_status_id': self.leave_type.id, + 'department_id': self.employee_hruser.department_id.id, + 'employee_id': self.employee_hruser.id, + 'date_from': datetime.now(), + 'date_to': datetime.now() + relativedelta(days=1), + 'number_of_days_temp': 1, + }) + with self.assertRaises(UserError): + other_leave.sudo(self.user_employee_id).action_draft() + + # ---------------------------------------- + # Validation: one validation, HR + # ---------------------------------------- + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_hr_by_manager(self): + """ Manager validates hr-only leaves """ + self.employee_leave.sudo(self.user_hrmanager_id).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_hr_by_officer_department(self): + """ Officer validates hr-only leaves for co-workers """ + self.employee_leave.sudo(self.user_hruser).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_hr_by_officer_no_department(self): + """ Officer validates hr-only leaves for workers from no department and with no manager """ + self.employee_hruser.write({'department_id': False}) + # TDE FIXME: not sure for this one + # self.employee_leave.sudo(self.user_hruser).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_hr_by_officer_other_department_with_manager(self): + """ Officer may not validate hr-only leaves for workers from another department that has a manager """ + self.employee_hruser.write({'department_id': self.hr_dept.id}) + with self.assertRaises(AccessError): + self.employee_leave.sudo(self.user_hruser).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_hr_by_officer_other_department_wo_manager(self): + """ Officer may not validate hr-only leaves for workers from another department that has no manager """ + self.employee_hruser.write({'department_id': self.hr_dept.id}) + with self.assertRaises(AccessError): + self.employee_leave.sudo(self.user_hruser).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_hr_by_officer_other_department_manager(self): + """ Officer validates hr-only leaves for workers from another department that he manages """ + self.employee_hruser.write({'department_id': self.hr_dept.id}) + self.employee_leave.sudo().department_id.write({'manager_id': self.employee_hruser.id}) + + self.employee_leave.sudo(self.user_hruser).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_hr_by_user(self): + """ User may not validate any leaves """ + with self.assertRaises(UserError): + self.employee_leave.sudo(self.user_employee_id).action_approve() + + with self.assertRaises(UserError): + self.employee_leave.sudo(self.user_employee_id).write({'state': 'validate'}) + + # ---------------------------------------- + # Validation: one validation, manager + # ---------------------------------------- + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_manager_by_manager(self): + """ Manager validates manager-only leaves """ + self.leave_type.write({'validation_type': 'manager'}) + self.employee_leave.sudo(self.user_hrmanager_id).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_manager_by_officer_department_manager(self): + """ Officer validates manager-only leaves for co-workers from department he manages""" + self.leave_type.write({'validation_type': 'manager'}) + self.employee_hruser.write({'department_id': self.hr_dept.id}) + self.employee_leave.sudo().department_id.write({'manager_id': self.employee_hruser.id}) + self.employee_leave.sudo(self.user_hruser).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_manager_by_officer_department_manager_other(self): + """ Officer may not validates manager-only leaves for co-workers from department he does not""" + self.leave_type.write({'validation_type': 'manager'}) + self.employee_hruser.write({'department_id': self.hr_dept.id}) + self.employee_leave.sudo().department_id.write({'manager_id': self.employee_hrmanager.id}) + with self.assertRaises(AccessError): + self.employee_leave.sudo(self.user_hruser).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_manager_by_officer_manager(self): + """ Officer validates manager-only leaves for co-workers that he manages""" + self.leave_type.write({'validation_type': 'manager'}) + self.employee_emp.write({'parent_id': self.employee_hruser.id}) + self.employee_leave.sudo(self.user_hruser).action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_manager_by_officer_manager_other(self): + """ Officer may not validate manager-only leaves for co-workers he does not manage""" + self.leave_type.write({'validation_type': 'manager'}) + self.employee_emp.write({'parent_id': self.employee_hrmanager.id}) + with self.assertRaises(UserError): + self.employee_leave.sudo(self.user_hruser).action_approve() + + with self.assertRaises(UserError): + self.employee_leave.sudo(self.user_hruser).write({'state': 'validate'}) + + # ---------------------------------------- + # Validation: double + # ---------------------------------------- + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_both_by_manager(self): + """ Manager validates double validation leaves """ + self.leave_type.write({'validation_type': 'both'}) + self.employee_leave.sudo(self.user_hrmanager_id).action_approve() + self.employee_leave.sudo(self.user_hrmanager_id).action_validate() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_both_by_officer(self): + """ Officer may not validate double validation leaves """ + self.leave_type.write({'validation_type': 'both'}) + self.employee_leave.sudo(self.user_hruser).action_approve() + with self.assertRaises(UserError): + self.employee_leave.sudo(self.user_hruser).action_validate() + + with self.assertRaises(UserError): + self.employee_leave.sudo(self.user_hruser).write({'state': 'validate'}) + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_validation_both_by_officer_and_manager(self): + """ Officer + Manager complete double validation leaves """ + self.leave_type.write({'validation_type': 'both'}) + self.employee_leave.sudo(self.user_hruser).action_approve() + self.employee_leave.sudo(self.user_hrmanager_id).action_validate() + + +class TestMultiCompany(TestHrHolidaysBase): + + def setUp(self): + super(TestMultiCompany, self).setUp() + self.new_company = self.env['res.company'].create({ + 'name': 'Crocodile Dundee Company', + }) + self.leave_type = self.env['hr.leave.type'].create({ + 'name': 'Unlimited - Company New', + 'limit': True, + 'company_id': self.new_company.id, + 'validation_type': 'hr', + }) + self.rd_dept.manager_id = False + self.hr_dept.manager_id = False + + self.employee_leave = self.env['hr.leave'].create({ + 'name': 'Test', + 'holiday_status_id': self.leave_type.id, + 'department_id': self.employee_emp.department_id.id, + 'employee_id': self.employee_emp.id, + 'date_from': datetime.now(), + 'date_to': datetime.now() + relativedelta(days=1), + 'number_of_days_temp': 1, + }) + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_access_other_company_user(self): + employee_leave = self.employee_leave.sudo(self.user_employee) + + with self.assertRaises(AccessError): + name = employee_leave.name + + with self.assertRaises(AccessError): + employee_leave.action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_access_other_company_officer(self): + employee_leave_hruser = self.employee_leave.sudo(self.user_hruser) + + with self.assertRaises(AccessError): + name = employee_leave_hruser.name + + with self.assertRaises(AccessError): + employee_leave_hruser.action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_access_other_company_manager(self): + employee_leave_hrmanager = self.employee_leave.sudo(self.user_hrmanager) + + with self.assertRaises(AccessError): + name = employee_leave_hrmanager.name + + with self.assertRaises(AccessError): + employee_leave_hrmanager.action_approve() + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_access_no_company_user(self): + self.leave_type.write({'company_id': False}) + employee_leave = self.employee_leave.sudo(self.user_employee) + + name = employee_leave.name + with self.assertRaises(UserError): + employee_leave.action_approve() + self.assertEqual(employee_leave.state, 'confirm') + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_access_no_company_officer(self): + self.leave_type.write({'company_id': False}) + employee_leave_hruser = self.employee_leave.sudo(self.user_hruser) + + name = employee_leave_hruser.name + employee_leave_hruser.action_approve() + self.assertEqual(employee_leave_hruser.state, 'validate') + + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') + def test_leave_access_no_company_manager(self): + self.leave_type.write({'company_id': False}) + employee_leave_hrmanager = self.employee_leave.sudo(self.user_hrmanager) + + name = employee_leave_hrmanager.name + employee_leave_hrmanager.action_approve() + self.assertEqual(employee_leave_hrmanager.state, 'validate') diff --git a/addons/hr_holidays/tests/test_holidays_flow.py b/addons/hr_holidays/tests/test_holidays_flow.py index 6bbb81d57c0..d48045d2613 100644 --- a/addons/hr_holidays/tests/test_holidays_flow.py +++ b/addons/hr_holidays/tests/test_holidays_flow.py @@ -72,15 +72,6 @@ class TestHolidaysFlow(TestHrHolidaysBase): # Employee creates a leave request for another employee -> should crash HolidaysEmployeeGroup = Requests.sudo(self.user_employee_id) - with self.assertRaises(ValidationError): - HolidaysEmployeeGroup.create({ - 'name': 'Hol10', - 'employee_id': self.employee_hruser_id, - 'holiday_status_id': self.holidays_status_1.id, - 'date_from': (datetime.today() - relativedelta(days=1)), - 'date_to': datetime.today(), - 'number_of_days_temp': 1, - }) Requests.search([('name', '=', 'Hol10')]).unlink() # Employee creates a leave request in a no-limit category hr manager only @@ -97,8 +88,6 @@ class TestHolidaysFlow(TestHrHolidaysBase): self.assertEqual(hol1_user_group.state, 'confirm', 'hr_holidays: newly created leave request should be in confirm state') # Employee validates its leave request -> should not work - with self.assertRaises(UserError): - 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 work @@ -119,14 +108,8 @@ class TestHolidaysFlow(TestHrHolidaysBase): self.assertEqual(hol12_user_group.state, 'confirm', 'hr_holidays: newly created leave request should be in confirm state') # Employee validates its leave request -> should not work - with self.assertRaises(UserError): - 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 -> 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') @@ -169,9 +152,7 @@ class TestHolidaysFlow(TestHrHolidaysBase): }) # HrUser validates the first step aloc1_user_group.action_approve() - # HrUser validates the second step -> should crash - with self.assertRaises(UserError): - aloc1_user_group.action_validate() + # HrManager validates the second step aloc1_user_group.sudo(self.user_hrmanager_id).action_validate() # Checks Employee has effectively some days left @@ -197,9 +178,6 @@ class TestHolidaysFlow(TestHrHolidaysBase): self.assertEqual(hol2.state, 'validate1', 'hr_holidays: first validation should lead to validate1 state') - # HrUser validates the second step -> should crash - with self.assertRaises(UserError): - hol2_user_group.action_validate() # HrManager validates the second step hol2_user_group.sudo(self.user_hrmanager_id).action_validate() self.assertEqual(hol2.state, 'validate', @@ -214,9 +192,6 @@ class TestHolidaysFlow(TestHrHolidaysBase): # Check left days: 2 days left again _check_holidays_status(hol_status_2_employee_group, 2.0, 0.0, 2.0, 2.0) - # Annoyed, HrUser tries to fix its error and tries to reset the leave request -> does not work, only HrManager - with self.assertRaises(UserError): - hol2_user_group.action_draft() self.assertEqual(hol2.state, 'refuse', 'hr_holidays: hr_user should not be able to reset a refused leave request') @@ -302,31 +277,10 @@ class TestHolidaysFlow(TestHrHolidaysBase): 'number_of_days_temp': 1, }) - # But not someone else's leave - with self.assertRaises(AccessError): - hol42.sudo(self.user_employee_id).action_draft() - - # An officer should not be able to reset someone else's leave - with self.assertRaises(UserError): - hol42.sudo(self.user_hruser_id).action_draft() - # A manager should be able to reset someone else's leave hol42.action_draft() hol42.unlink() - # Officer should not be able to approve it's own leave - hol50 = HolidaysEmployeeGroup.sudo(self.user_hruser_id).create({ - 'name': 'Hol50', - 'employee_id': self.employee_hruser_id, - 'holiday_status_id': self.holidays_status_1.id, - 'date_from': (datetime.today() + relativedelta(days=15)).strftime('%Y-%m-%d %H:%M'), - 'date_to': (datetime.today() + relativedelta(days=16)), - 'number_of_days_temp': 1, - }) - - with self.assertRaises(UserError): - hol50.action_approve() - # Manager should be able to approve it's own leave hol51 = HolidaysEmployeeGroup.sudo(self.user_hrmanager_2_id).create({ 'name': 'Hol51',