[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

7 files changed, 259 insertions, 11 deletions

diff --git a/addons/hr_expense/models/hr_employee.py b/addons/hr_expense/models/hr_employee.py
index c4388789e15..4a2b7dcb0a5 100644
--- a/addons/hr_expense/models/hr_employee.py
+++ b/addons/hr_expense/models/hr_employee.py
@@ -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.")
diff --git a/addons/hr_expense/models/hr_expense.py b/addons/hr_expense/models/hr_expense.py
index dad6b3261b5..0e5d1339312 100644
--- a/addons/hr_expense/models/hr_expense.py
+++ b/addons/hr_expense/models/hr_expense.py
@@ -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})
diff --git a/addons/hr_expense/security/ir.model.access.csv b/addons/hr_expense/security/ir.model.access.csv
index b0e187d617f..f995ebe78d7 100644
--- a/addons/hr_expense/security/ir.model.access.csv
+++ b/addons/hr_expense/security/ir.model.access.csv
@@ -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

diff --git a/addons/hr_expense/security/ir_rule.xml b/addons/hr_expense/security/ir_rule.xml
index eba7fe52e1a..5836c97b83a 100644
--- a/addons/hr_expense/security/ir_rule.xml
+++ b/addons/hr_expense/security/ir_rule.xml
@@ -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"/>
diff --git a/addons/hr_expense/tests/common.py b/addons/hr_expense/tests/common.py
index 71a4d33e757..0ff8e27fba0 100644
--- a/addons/hr_expense/tests/common.py
+++ b/addons/hr_expense/tests/common.py
@@ -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,
+        })
diff --git a/addons/hr_expense/tests/test_expenses.py b/addons/hr_expense/tests/test_expenses.py
index 976ba9188ae..8d526b02a36 100644
--- a/addons/hr_expense/tests/test_expenses.py
+++ b/addons/hr_expense/tests/test_expenses.py
@@ -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('')
diff --git a/addons/hr_expense/views/hr_expense_views.xml b/addons/hr_expense/views/hr_expense_views.xml
index 76134466aef..6176b2057d5 100644
--- a/addons/hr_expense/views/hr_expense_views.xml
+++ b/addons/hr_expense/views/hr_expense_views.xml
@@ -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