[IMP] hr_expense : improve access rights of manager/officer
Currently the distinction between expense officers and expense managers doesn't make sense because they both have the same rights. This commit aims at making a clear difference between an officer and a manager. After this commit the rights will look like this : * Expense Manager : - See and approve any expense * Expense Officer : - Can see and approve the expenses of employees in his department or employees for which he is the manager (field parent_id on hr_employee) - Cannot approve his own expenses * Simple user : - Can only see his own expenses - Can only send his expenses to a manager (either the manager of his department, his direct manager [parent_id field] or an expense manager) Related to task #51520 Closes #23033
This commit is contained in:
@@ -8,5 +8,5 @@ class Employee(models.Model):
|
||||
|
||||
expense_manager_id = fields.Many2one(
|
||||
'res.users', string='Expense Responsible',
|
||||
domain=lambda self: [('groups_id', 'in', self.env.ref('hr_expense.group_hr_expense_manager').id)],
|
||||
domain=lambda self: [('groups_id', 'in', self.env.ref('hr_expense.group_hr_expense_user').id)],
|
||||
help="User responsible of expense approval. Should be Expense Manager.")
|
||||
|
||||
@@ -29,9 +29,25 @@ class HrExpense(models.Model):
|
||||
def _default_account_id(self):
|
||||
return self.env['ir.property'].get('property_account_expense_categ_id', 'product.category')
|
||||
|
||||
@api.model
|
||||
def _get_employee_id_domain(self):
|
||||
res = [(1, '=', 0)]
|
||||
|
||||
if self.user_has_groups('hr_expense.group_hr_expense_manager') or self.user_has_groups('account.group_account_user'):
|
||||
res = [(1, '=', 1)]
|
||||
elif self.user_has_groups('hr_expense.group_hr_expense_user') and self.env.user.employee_ids:
|
||||
employee = self.env.user.employee_ids[0]
|
||||
res = ['|', '|', ('department_id.manager_id.id', '=', employee.id),
|
||||
('parent_id.id', '=', employee.id), ('expense_manager_id.id', '=', employee.id)]
|
||||
elif self.env.user.employee_ids:
|
||||
employee = self.env.user.employee_ids[0]
|
||||
res = [('id', '=', employee.id)]
|
||||
|
||||
return res
|
||||
|
||||
name = fields.Char('Description', readonly=True, required=True, states={'draft': [('readonly', False)], 'reported': [('readonly', False)], 'refused': [('readonly', False)]})
|
||||
date = fields.Date(readonly=True, states={'draft': [('readonly', False)], 'reported': [('readonly', False)], 'refused': [('readonly', False)]}, default=fields.Date.context_today, string="Date")
|
||||
employee_id = fields.Many2one('hr.employee', string="Employee", required=True, readonly=True, states={'draft': [('readonly', False)], 'reported': [('readonly', False)], 'refused': [('readonly', False)]}, default=_default_employee_id)
|
||||
employee_id = fields.Many2one('hr.employee', string="Employee", required=True, readonly=True, states={'draft': [('readonly', False)], 'reported': [('readonly', False)], 'refused': [('readonly', False)]}, default=_default_employee_id, domain=lambda self: self._employee_filter)
|
||||
product_id = fields.Many2one('product.product', string='Product', readonly=True, states={'draft': [('readonly', False)], 'reported': [('readonly', False)], 'refused': [('readonly', False)]}, domain=[('can_be_expensed', '=', True)], required=True)
|
||||
product_uom_id = fields.Many2one('uom.uom', string='Unit of Measure', required=True, readonly=True, states={'draft': [('readonly', False)], 'refused': [('readonly', False)]}, default=_default_product_uom_id)
|
||||
unit_amount = fields.Float("Unit Price", readonly=True, required=True, states={'draft': [('readonly', False)], 'reported': [('readonly', False)], 'refused': [('readonly', False)]}, digits=dp.get_precision('Product Price'))
|
||||
@@ -451,7 +467,25 @@ class HrExpense(models.Model):
|
||||
|
||||
|
||||
class HrExpenseSheet(models.Model):
|
||||
"""
|
||||
Here are the rights associated with the expense flow
|
||||
|
||||
Action Group Restriction
|
||||
=================================================================================
|
||||
Submit Employee Only his own
|
||||
Officer If he is expense manager of the employee, manager of the employee
|
||||
or the employee is in the department managed by the officer
|
||||
Manager Always
|
||||
Approve Officer Not his own and he is expense manager of the employee, manager of the employee
|
||||
or the employee is in the department managed by the officer
|
||||
Manager Always
|
||||
Post Anybody State = approve and journal_id defined
|
||||
Done Anybody State = approve and journal_id defined
|
||||
Cancel Officer Not his own and he is expense manager of the employee, manager of the employee
|
||||
or the employee is in the department managed by the officer
|
||||
Manager Always
|
||||
=================================================================================
|
||||
"""
|
||||
_name = "hr.expense.sheet"
|
||||
_inherit = ['mail.thread', 'mail.activity.mixin']
|
||||
_description = "Expense Report"
|
||||
@@ -613,7 +647,16 @@ class HrExpenseSheet(models.Model):
|
||||
@api.multi
|
||||
def approve_expense_sheets(self):
|
||||
if not self.user_has_groups('hr_expense.group_hr_expense_user'):
|
||||
raise UserError(_("Only HR Officers can approve expenses"))
|
||||
raise UserError(_("Only Managers and HR Officers can approve expenses"))
|
||||
elif not self.user_has_groups('hr_expense.group_hr_expense_manager'):
|
||||
current_managers = self.employee_id.parent_id.user_id | self.employee_id.department_id.manager_id.user_id
|
||||
|
||||
if self.employee_id.user_id == self.env.user:
|
||||
raise UserError(_("You cannot approve your own expenses"))
|
||||
|
||||
if not self.env.user in current_managers:
|
||||
raise UserError(_("You can only approve your department expenses"))
|
||||
|
||||
responsible_id = self.user_id.id or self.env.user.id
|
||||
self.write({'state': 'approve', 'user_id': responsible_id})
|
||||
self.activity_update()
|
||||
@@ -625,7 +668,16 @@ class HrExpenseSheet(models.Model):
|
||||
@api.multi
|
||||
def refuse_sheet(self, reason):
|
||||
if not self.user_has_groups('hr_expense.group_hr_expense_user'):
|
||||
raise UserError(_("Only HR Officers can refuse expenses"))
|
||||
raise UserError(_("Only Managers and HR Officers can approve expenses"))
|
||||
elif not self.user_has_groups('hr_expense.group_hr_expense_manager'):
|
||||
current_managers = self.employee_id.parent_id.user_id | self.employee_id.department_id.manager_id.user_id
|
||||
|
||||
if self.employee_id.user_id == self.env.user:
|
||||
raise UserError(_("You cannot refuse your own expenses"))
|
||||
|
||||
if not self.env.user in current_managers:
|
||||
raise UserError(_("You can only refuse your department expenses"))
|
||||
|
||||
self.write({'state': 'cancel'})
|
||||
for sheet in self:
|
||||
sheet.message_post_with_view('hr_expense.hr_expense_template_refuse_reason', values={'reason': reason, 'is_sheet': True, 'name': self.name})
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
|
||||
access_hr_expense_user,hr.expense.user,model_hr_expense,hr_expense.group_hr_expense_user,1,1,1,1
|
||||
access_hr_expense_employee,hr.expense.employee,model_hr_expense,base.group_user,1,1,1,1
|
||||
access_hr_expense_sheet_manager,hr.expense.sheet.manager,model_hr_expense_sheet,hr_expense.group_hr_expense_manager,1,1,1,1
|
||||
access_hr_expense_sheet_user,hr.expense.sheet.user,model_hr_expense_sheet,hr_expense.group_hr_expense_user,1,1,1,1
|
||||
access_hr_expense_sheet_employee,hr.expense.sheet.employee,model_hr_expense_sheet,base.group_user,1,1,1,1
|
||||
access_product_product_hr_expense_user,product.product.hr.expense.user,product.model_product_product,hr_expense.group_hr_expense_user,1,1,1,1
|
||||
|
||||
|
@@ -2,21 +2,56 @@
|
||||
<odoo>
|
||||
<data noupdate="1">
|
||||
|
||||
<record id="property_rule_expense_manager" model="ir.rule">
|
||||
<record id="ir_rule_hr_expense_manager" model="ir.rule">
|
||||
<field name="name">Manager Expense</field>
|
||||
<field name="model_id" ref="model_hr_expense"/>
|
||||
<field name="domain_force">[(1, '=', 1)]</field>
|
||||
<field name="groups" eval="[
|
||||
(4, ref('account.group_account_user')),
|
||||
(4, ref('hr_expense.group_hr_expense_manager')),
|
||||
(4, ref('hr_expense.group_hr_expense_user'))]"/>
|
||||
(4, ref('hr_expense.group_hr_expense_manager'))]"/>
|
||||
</record>
|
||||
<record id="property_rule_expense_employee" model="ir.rule">
|
||||
<record id="ir_rule_hr_expense_user" model="ir.rule">
|
||||
<field name="name">Officer Expense</field>
|
||||
<field name="model_id" ref="model_hr_expense"/>
|
||||
<field name="domain_force">['|', '|', '|',
|
||||
('employee_id.user_id.id', '=', user.id),
|
||||
('employee_id.department_id.manager_id.user_id.id', '=', user.id),
|
||||
('employee_id.parent_id.user_id.id', '=', user.id),
|
||||
('employee_id.expense_manager_id.id', '=', user.id)]</field>
|
||||
<field name="groups" eval="[(4, ref('hr_expense.group_hr_expense_user'))]"/>
|
||||
</record>
|
||||
<record id="ir_rule_hr_expense_employee" model="ir.rule">
|
||||
<field name="name">Employee Expense</field>
|
||||
<field name="model_id" ref="model_hr_expense"/>
|
||||
<field name="domain_force">[('employee_id.user_id', '=', user.id)]</field>
|
||||
<field name="groups" eval="[(4, ref('base.group_user'))]"/>
|
||||
</record>
|
||||
|
||||
<record id="ir_rule_hr_expense_sheet_manager" model="ir.rule">
|
||||
<field name="name">Manager Expense Sheet</field>
|
||||
<field name="model_id" ref="model_hr_expense_sheet"/>
|
||||
<field name="domain_force">[(1, '=', 1)]</field>
|
||||
<field name="groups" eval="[
|
||||
(4, ref('account.group_account_user')),
|
||||
(4, ref('hr_expense.group_hr_expense_manager'))]"/>
|
||||
</record>
|
||||
<record id="ir_rule_hr_expense_sheet_user" model="ir.rule">
|
||||
<field name="name">Officer Expense Sheet</field>
|
||||
<field name="model_id" ref="model_hr_expense_sheet"/>
|
||||
<field name="domain_force">['|', '|', '|',
|
||||
('employee_id.user_id.id', '=', user.id),
|
||||
('employee_id.department_id.manager_id.user_id.id', '=', user.id),
|
||||
('employee_id.parent_id.user_id.id', '=', user.id),
|
||||
('employee_id.expense_manager_id.id', '=', user.id)]</field>
|
||||
<field name="groups" eval="[(4, ref('hr_expense.group_hr_expense_user'))]"/>
|
||||
</record>
|
||||
<record id="ir_rule_hr_expense_sheet_employee" model="ir.rule">
|
||||
<field name="name">Employee Expense Sheet</field>
|
||||
<field name="model_id" ref="model_hr_expense_sheet"/>
|
||||
<field name="domain_force">[('employee_id.user_id', '=', user.id)]</field>
|
||||
<field name="groups" eval="[(4, ref('base.group_user'))]"/>
|
||||
</record>
|
||||
|
||||
<record id="hr_expense_comp_rule" model="ir.rule">
|
||||
<field name="name">Expense multi company rule</field>
|
||||
<field name="model_id" ref="model_hr_expense"/>
|
||||
|
||||
@@ -21,6 +21,7 @@ class TestExpenseCommon(TestAccountNoChartCommon):
|
||||
# create employee
|
||||
cls.employee = cls.env['hr.employee'].create({
|
||||
'name': 'Johnny Employee',
|
||||
'user_id': cls.user_employee.id,
|
||||
'address_home_id': cls.user_employee.partner_id.id,
|
||||
'address_id': cls.user_employee.partner_id.id,
|
||||
})
|
||||
@@ -56,3 +57,66 @@ class TestExpenseCommon(TestAccountNoChartCommon):
|
||||
'employee_id': cls.employee.id,
|
||||
'journal_id': cls.journal.id,
|
||||
})
|
||||
|
||||
Users = cls.env['res.users'].with_context(no_reset_password=True)
|
||||
|
||||
# Find Employee group
|
||||
group_employee_id = cls.env.ref('base.group_user').id
|
||||
|
||||
cls.user_emp2 = Users.create({
|
||||
'name': 'Superboy Employee',
|
||||
'login': 'superboy',
|
||||
'email': 'superboy@example.com',
|
||||
'groups_id': [(6, 0, [group_employee_id])]
|
||||
})
|
||||
|
||||
cls.user_officer = Users.create({
|
||||
'name': 'Batman Officer',
|
||||
'login': 'batman',
|
||||
'email': 'batman.hero@example.com',
|
||||
'groups_id': [(6, 0, [group_employee_id, cls.env.ref('hr_expense.group_hr_expense_user').id])]
|
||||
})
|
||||
|
||||
cls.emp_emp2 = cls.env['hr.employee'].create({
|
||||
'name': 'Superboy',
|
||||
'user_id': cls.user_emp2.id,
|
||||
})
|
||||
|
||||
cls.emp_officer = cls.env['hr.employee'].create({
|
||||
'name': 'Batman',
|
||||
'user_id': cls.user_officer.id,
|
||||
})
|
||||
|
||||
cls.emp_manager = cls.env['hr.employee'].create({
|
||||
'name': 'Superman',
|
||||
'user_id': cls.user_manager.id,
|
||||
})
|
||||
|
||||
cls.rd = cls.env['hr.department'].create({
|
||||
'name': 'R&D',
|
||||
'manager_id': cls.emp_officer.id,
|
||||
'member_ids': [(6, 0, [cls.employee.id])],
|
||||
})
|
||||
|
||||
cls.ps = cls.env['hr.department'].create({
|
||||
'name': 'PS',
|
||||
'manager_id': cls.emp_manager.id,
|
||||
'member_ids': [(6, 0, [cls.emp_emp2.id])],
|
||||
})
|
||||
|
||||
cls.uom_unit = cls.env.ref('uom.product_uom_unit').id
|
||||
cls.uom_dozen = cls.env.ref('uom.product_uom_dozen').id
|
||||
|
||||
cls.product_1 = cls.env['product.product'].create({
|
||||
'name': 'Batmobile repair',
|
||||
'type': 'service',
|
||||
'uom_id': cls.uom_unit,
|
||||
'uom_po_id': cls.uom_unit,
|
||||
})
|
||||
|
||||
cls.product_2 = cls.env['product.product'].create({
|
||||
'name': 'Superboy costume washing',
|
||||
'type': 'service',
|
||||
'uom_id': cls.uom_unit,
|
||||
'uom_po_id': cls.uom_unit,
|
||||
})
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from odoo.exceptions import UserError, AccessError
|
||||
|
||||
from odoo.addons.hr_expense.tests.common import TestExpenseCommon
|
||||
|
||||
|
||||
@@ -72,3 +74,98 @@ class TestAccountEntry(TestExpenseCommon):
|
||||
self.assertEquals(len(self.analytic_account.line_ids), 1, "Analytic Account should have only one line")
|
||||
self.assertAlmostEquals(self.analytic_account.line_ids[0].amount, -636.36, "Amount on the only AAL is wrong")
|
||||
self.assertEquals(self.analytic_account.line_ids[0].product_id, self.product_expense, "Product of AAL should be the one from the expense")
|
||||
|
||||
|
||||
class TestExpenseRights(TestExpenseCommon):
|
||||
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
super(TestExpenseRights, cls).setUpClass()
|
||||
|
||||
def test_expense_create(self):
|
||||
# Employee should be able to create an Expense
|
||||
self.env['hr.expense'].sudo(self.user_employee.id).create({
|
||||
'name': 'Batmobile repair',
|
||||
'employee_id': self.employee.id,
|
||||
'product_id': self.product_1.id,
|
||||
'unit_amount': 1,
|
||||
'quantity': 1,
|
||||
})
|
||||
|
||||
# Employee should not be able to create an Expense for someone else
|
||||
with self.assertRaises(AccessError):
|
||||
self.env['hr.expense'].sudo(self.user_employee.id).create({
|
||||
'name': 'Superboy costume washing',
|
||||
'employee_id': self.emp_emp2.id,
|
||||
'product_id': self.product_2.id,
|
||||
'unit_amount': 1,
|
||||
'quantity': 1,
|
||||
})
|
||||
|
||||
def test_expense_approve(self):
|
||||
sheet = self.env['hr.expense.sheet'].create({
|
||||
'name': 'Furnitures',
|
||||
'employee_id': self.emp_officer.id,
|
||||
})
|
||||
|
||||
sheet_2 = self.env['hr.expense.sheet'].create({
|
||||
'name': 'Services',
|
||||
'employee_id': self.employee.id,
|
||||
})
|
||||
|
||||
sheet_3 = self.env['hr.expense.sheet'].create({
|
||||
'name': 'Services 2',
|
||||
'employee_id': self.emp_emp2.id,
|
||||
})
|
||||
|
||||
# Employee should not be able to approve expense sheet
|
||||
with self.assertRaises(UserError):
|
||||
sheet.sudo(self.user_officer).approve_expense_sheets()
|
||||
# Officer should not be able to approve own expense sheet
|
||||
with self.assertRaises(UserError):
|
||||
sheet.sudo(self.user_officer).approve_expense_sheets()
|
||||
sheet.sudo(self.user_manager).approve_expense_sheets()
|
||||
|
||||
# Officer should be able to approve expense from his department
|
||||
sheet_2.sudo(self.user_officer).approve_expense_sheets()
|
||||
|
||||
# Officer should not be able to approve expense sheet from another department
|
||||
with self.assertRaises(AccessError):
|
||||
sheet_3.sudo(self.user_officer).approve_expense_sheets()
|
||||
sheet_3.sudo(self.user_manager).approve_expense_sheets()
|
||||
|
||||
def test_expense_refuse(self):
|
||||
sheet = self.env['hr.expense.sheet'].create({
|
||||
'name': 'Furnitures',
|
||||
'employee_id': self.emp_officer.id,
|
||||
})
|
||||
|
||||
sheet_2 = self.env['hr.expense.sheet'].create({
|
||||
'name': 'Services',
|
||||
'employee_id': self.employee.id,
|
||||
})
|
||||
|
||||
sheet_3 = self.env['hr.expense.sheet'].create({
|
||||
'name': 'Services 2',
|
||||
'employee_id': self.emp_emp2.id,
|
||||
})
|
||||
|
||||
sheet.sudo(self.user_manager).approve_expense_sheets()
|
||||
sheet_2.sudo(self.user_manager).approve_expense_sheets()
|
||||
sheet_3.sudo(self.user_manager).approve_expense_sheets()
|
||||
|
||||
# Employee should not be able to refuse expense sheet
|
||||
with self.assertRaises(UserError):
|
||||
sheet.sudo(self.user_employee).refuse_sheet('')
|
||||
# Officer should not be able to refuse own expense sheet
|
||||
with self.assertRaises(UserError):
|
||||
sheet.sudo(self.user_officer).refuse_sheet('')
|
||||
sheet.sudo(self.user_manager).refuse_sheet('')
|
||||
|
||||
# Officer should be able to refuse expense from his department
|
||||
sheet_2.sudo(self.user_officer).refuse_sheet('')
|
||||
|
||||
# Officer should not be able to refuse expense sheet from another department
|
||||
with self.assertRaises(AccessError):
|
||||
sheet_3.sudo(self.user_officer).refuse_sheet('')
|
||||
sheet_3.sudo(self.user_manager).refuse_sheet('')
|
||||
|
||||
@@ -245,8 +245,7 @@
|
||||
<field name="res_model">hr.expense</field>
|
||||
<field name="view_mode">tree,kanban,form,graph,pivot</field>
|
||||
<field name="search_view_id" ref="view_hr_expense_filter"/>
|
||||
<field name="domain">[('employee_id.user_id', '=', uid)]</field>
|
||||
<field name="context">{'search_default_to_report': 1}</field>
|
||||
<field name="context">{'search_default_to_report': 1, 'search_default_my_expenses': 1}</field>
|
||||
<field name="help" type="html">
|
||||
<p class="o_view_nocontent_smiling_face">
|
||||
Create a new expense
|
||||
|
||||
Reference in New Issue
Block a user