From b8a8ad27de40e856d713b0d57c1f8f591e1e4523 Mon Sep 17 00:00:00 2001
From: Kevin Baptiste <kba@odoo.com>
Date: Thu, 9 Apr 2020 14:55:52 +0000
Subject: [PATCH] [FIX] hr_holidays: "Everyone" view on calendar_event

Purpose
=======

Avoid to show to much information to leave responsibles.

Specification
=============

- Dynamically add/remove the leave_manager_ids on the group
  group_hr_holidays_responsible, on employee creation/modification
- Retrict the available employees when creating a leave. For
  manager/users : Everyone. For responsibles: The employee for
  who the user is responsible. Otherwise: Only me.
- Don't obfuscate the leave name on employees for who I'm the
  responsible.
- Add a new SQL view for "Everyone" report. To select only the
  information we want to share instead of showing the hr.leave
  model directly.

closes odoo/odoo#47659

closes odoo/odoo#49765

Taskid: 2206862
Related: odoo/enterprise#9239
Related: odoo/enterprise#10021
X-original-commit: 781147dcaf1552b8f2ab687ed73034b8fd1f7352
Signed-off-by: Yannick Tivisse (yti) <yti@odoo.com>
---
 addons/hr_holidays/__manifest__.py            |  1 +
 addons/hr_holidays/models/hr_employee.py      | 29 ++++++++++
 addons/hr_holidays/models/hr_leave.py         |  3 +-
 addons/hr_holidays/models/res_users.py        | 16 ++++++
 addons/hr_holidays/report/__init__.py         |  1 +
 .../report/hr_leave_report_calendar.py        | 42 ++++++++++++++
 .../report/hr_leave_report_calendar.xml       | 39 +++++++++++++
 .../security/hr_holidays_security.xml         | 55 ++++++++++++++++++-
 .../hr_holidays/security/ir.model.access.csv  |  1 +
 .../hr_holidays/tests/test_access_rights.py   | 19 +++----
 .../hr_holidays/views/hr_holidays_views.xml   |  1 +
 .../views/hr_leave_allocation_views.xml       |  6 +-
 addons/hr_holidays/views/hr_leave_views.xml   | 17 +-----
 13 files changed, 197 insertions(+), 33 deletions(-)
 create mode 100644 addons/hr_holidays/report/hr_leave_report_calendar.py
 create mode 100644 addons/hr_holidays/report/hr_leave_report_calendar.xml

diff --git a/addons/hr_holidays/__manifest__.py b/addons/hr_holidays/__manifest__.py
index dcac711d856..167ccba5d6c 100644
--- a/addons/hr_holidays/__manifest__.py
+++ b/addons/hr_holidays/__manifest__.py
@@ -45,6 +45,7 @@ A synchronization with an internal agenda (Meetings of the CRM module) is also p
         'report/hr_holidays_templates.xml',
         'report/hr_holidays_reports.xml',
         'report/hr_leave_reports.xml',
+        'report/hr_leave_report_calendar.xml',
 
         'views/hr_views.xml',
         'views/hr_leave_template.xml',
diff --git a/addons/hr_holidays/models/hr_employee.py b/addons/hr_holidays/models/hr_employee.py
index 4a2343fda12..22e9804aa67 100644
--- a/addons/hr_holidays/models/hr_employee.py
+++ b/addons/hr_holidays/models/hr_employee.py
@@ -148,8 +148,37 @@ class HrEmployeeBase(models.AbstractModel):
         ])
         return [('id', 'in', holidays.mapped('employee_id').ids)]
 
+    @api.model
+    def create(self, values):
+        if 'parent_id' in values:
+            manager = self.env['hr.employee'].browse(values['parent_id']).user_id
+            values['leave_manager_id'] = values.get('leave_manager_id', manager.id)
+        if values.get('leave_manager_id', False):
+            approver_group = self.env.ref('hr_holidays.group_hr_holidays_responsible', raise_if_not_found=False)
+            if approver_group:
+                approver_group.sudo().write({'users': [(4, values['leave_manager_id'])]})
+        return super(HrEmployeeBase, self).create(values)
+
     def write(self, values):
+        if 'parent_id' in values:
+            manager = self.env['hr.employee'].browse(values['parent_id']).user_id
+            if manager:
+                to_change = self.filtered(lambda e: e.leave_manager_id == e.parent_id.user_id or not e.leave_manager_id)
+                to_change.write({'leave_manager_id': values.get('leave_manager_id', manager.id)})
+
+        old_managers = self.env['res.users']
+        if 'leave_manager_id' in values:
+            old_managers = self.mapped('leave_manager_id')
+            if values['leave_manager_id']:
+                old_managers -= self.env['res.users'].browse(values['leave_manager_id'])
+                approver_group = self.env.ref('hr_holidays.group_hr_holidays_responsible', raise_if_not_found=False)
+                if approver_group:
+                    approver_group.sudo().write({'users': [(4, values['leave_manager_id'])]})
+
         res = super(HrEmployeeBase, self).write(values)
+        # remove users from the Responsible group if they are no longer leave managers
+        old_managers._clean_leave_responsible_users()
+
         if 'parent_id' in values or 'department_id' in values:
             today_date = fields.Datetime.now()
             hr_vals = {}
diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py
index 39ab4eee64b..31d27441f34 100644
--- a/addons/hr_holidays/models/hr_leave.py
+++ b/addons/hr_holidays/models/hr_leave.py
@@ -128,6 +128,7 @@ class HolidaysRequest(models.Model):
         domain=[('valid', '=', True)])
     validation_type = fields.Selection(string='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",
         states={'draft': [('readonly', False)], 'confirm': [('readonly', False)]}, default=_default_employee, tracking=True)
@@ -829,7 +830,7 @@ class HolidaysRequest(models.Model):
         holidays._create_resource_leave()
         for holiday in holidays.filtered(lambda l: l.holiday_status_id.create_calendar_meeting):
             meeting_values = holiday._prepare_holidays_meeting_values()
-            meeting = self.env['calendar.event'].with_context(no_mail_to_attendees=True).create(meeting_values)
+            meeting = self.env['calendar.event'].with_context(no_mail_to_attendees=True, active_model=self._name).create(meeting_values)
             holiday.write({'meeting_id': meeting.id})
 
     def _prepare_holidays_meeting_values(self):
diff --git a/addons/hr_holidays/models/res_users.py b/addons/hr_holidays/models/res_users.py
index c78f415f923..f18c40fff10 100644
--- a/addons/hr_holidays/models/res_users.py
+++ b/addons/hr_holidays/models/res_users.py
@@ -57,3 +57,19 @@ class User(models.Model):
                             AND res_users.active = 't'
                             AND date_from <= %%s AND date_to >= %%s''' % field, (now, now))
         return [r[0] for r in self.env.cr.fetchall()]
+
+    def _clean_leave_responsible_users(self):
+        # self = old bunch of leave responsibles
+        # This method compares the current leave managers
+        # and remove the access rights to those who don't
+        # need them anymore
+        approver_group = self.env.ref('hr_holidays.group_hr_holidays_responsible', raise_if_not_found=False)
+        if not self or not approver_group:
+            return
+        res = self.env['hr.employee'].read_group(
+            [('leave_manager_id', 'in', self.ids)],
+            ['leave_manager_id'],
+            ['leave_manager_id'])
+        responsibles_to_remove_ids = set(self.ids) - {x['leave_manager_id'][0] for x in res}
+        approver_group.sudo().write({
+            'users': [(3, manager_id) for manager_id in responsibles_to_remove_ids]})
diff --git a/addons/hr_holidays/report/__init__.py b/addons/hr_holidays/report/__init__.py
index 4dcb9db9075..c59b1e68b62 100644
--- a/addons/hr_holidays/report/__init__.py
+++ b/addons/hr_holidays/report/__init__.py
@@ -3,3 +3,4 @@
 
 from . import holidays_summary_report
 from . import hr_leave_report
+from . import hr_leave_report_calendar
diff --git a/addons/hr_holidays/report/hr_leave_report_calendar.py b/addons/hr_holidays/report/hr_leave_report_calendar.py
new file mode 100644
index 00000000000..49418c9a63b
--- /dev/null
+++ b/addons/hr_holidays/report/hr_leave_report_calendar.py
@@ -0,0 +1,42 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo import api, fields, models, tools
+
+from odoo.addons.base.models.res_partner import _tz_get
+
+
+class LeaveReportCalendar(models.Model):
+    _name = "hr.leave.report.calendar"
+    _description = 'Time Off Calendar'
+    _auto = False
+    _order = "start_datetime DESC, employee_id"
+
+    name = fields.Char(string='Name', readonly=True)
+    start_datetime = fields.Datetime(string='From', readonly=True)
+    stop_datetime = fields.Datetime(string='To', readonly=True)
+    tz = fields.Selection(_tz_get, string="Timezone", readonly=True)
+    duration = fields.Float(string='Duration', readonly=True)
+    employee_id = fields.Many2one('hr.employee', readonly=True)
+    company_id = fields.Many2one('res.company', readonly=True)
+
+    def init(self):
+        tools.drop_view_if_exists(self._cr, 'hr_leave_report_calendar')
+
+        self._cr.execute("""CREATE OR REPLACE VIEW hr_leave_report_calendar AS
+        (SELECT 
+            row_number() OVER() AS id,
+            ce.name AS name,
+            ce.start AS start_datetime,
+            ce.stop AS stop_datetime,
+            ce.duration AS duration,
+            hl.employee_id AS employee_id,
+            em.company_id AS company_id
+        FROM hr_leave hl
+            LEFT JOIN calendar_event ce
+                ON ce.id = hl.meeting_id
+            LEFT JOIN hr_employee em
+                ON em.id = hl.employee_id
+        WHERE 
+            hl.state = 'validate');
+        """)
diff --git a/addons/hr_holidays/report/hr_leave_report_calendar.xml b/addons/hr_holidays/report/hr_leave_report_calendar.xml
new file mode 100644
index 00000000000..459fe8810ec
--- /dev/null
+++ b/addons/hr_holidays/report/hr_leave_report_calendar.xml
@@ -0,0 +1,39 @@
+<?xml version='1.0' encoding='UTF-8' ?>
+<odoo>
+    <record id="action_hr_holidays_dashboard" model="ir.actions.act_window">
+        <field name="name">All Time Off</field>
+        <field name="res_model">hr.leave.report.calendar</field>
+        <field name="view_mode">calendar</field>
+        <!-- YTI : Ecrabouille explicitely those fields, as this is deployed on 
+                   a stable release. -->
+        <field name="view_id"/>
+        <field name="search_view_id"/>
+        <field name="domain"/>
+        <field name="context"/>
+    </record>
+
+    <record id="hr_leave_report_calendar_view" model="ir.ui.view">
+        <field name="name">hr.leave.report.calendar.view</field>
+        <field name="model">hr.leave.report.calendar</field>
+        <field name="arch" type="xml">
+            <calendar string="Time Off" date_start="start_datetime" date_stop="stop_datetime" mode="month" quick_add="False" color="employee_id" event_open_popup="True">
+                <field name="name"/>
+            </calendar>
+        </field>
+    </record>
+
+    <record id="hr_leave_report_calendar_view_form" model="ir.ui.view">
+        <field name="name">hr.leave.report.calendar.view.form</field>
+        <field name="model">hr.leave.report.calendar</field>
+        <field name="arch" type="xml">
+            <form string="Time Off">
+                <group>
+                    <field name="name"/>
+                    <field name="start_datetime"/>
+                    <field name="stop_datetime"/>
+                    <field name="employee_id" />
+                </group>
+            </form>
+        </field>
+    </record>
+</odoo>
diff --git a/addons/hr_holidays/security/hr_holidays_security.xml b/addons/hr_holidays/security/hr_holidays_security.xml
index 9b2c42d4d5a..bb94ef5856d 100644
--- a/addons/hr_holidays/security/hr_holidays_security.xml
+++ b/addons/hr_holidays/security/hr_holidays_security.xml
@@ -5,10 +5,16 @@
         <field name="sequence">10</field>
     </record>
 
+    <record id="group_hr_holidays_responsible" model="res.groups">
+        <field name="name">Responsible</field>
+        <field name="category_id" ref="base.module_category_human_resources_time_off"/>
+        <field name="implied_ids" eval="[(4, ref('base.group_user'))]"/>
+    </record>
+
     <record id="group_hr_holidays_user" model="res.groups">
         <field name="name">All Approver</field>
         <field name="category_id" ref="base.module_category_human_resources_time_off"/>
-        <field name="implied_ids" eval="[(4, ref('base.group_user'))]"/>
+        <field name="implied_ids" eval="[(4, ref('hr_holidays.group_hr_holidays_responsible'))]"/>
     </record>
 
     <record id="group_hr_holidays_manager" model="res.groups">
@@ -27,7 +33,7 @@
     <record id="hr_leave_rule_employee" model="ir.rule">
         <field name="name">Time Off base.group_user read</field>
         <field name="model_id" ref="model_hr_leave"/>
-        <field name="domain_force">[(1, '=', 1)]</field>
+        <field name="domain_force">[('employee_id.user_id', '=', user.id)]</field>
         <field name="perm_create" eval="False"/>
         <field name="perm_write" eval="False"/>
         <field name="perm_unlink" eval="False"/>
@@ -62,6 +68,44 @@
         <field name="groups" eval="[(4, ref('base.group_user'))]"/>
     </record>
 
+    <record id="hr_leave_rule_responsible_read" model="ir.rule">
+        <field name="name">Time Off Responsible read</field>
+        <field name="model_id" ref="model_hr_leave"/>
+        <field name="domain_force">[
+                ('employee_id.leave_manager_id', '=', user.id),
+        ]</field>
+        <field name="perm_write" eval="False"/>
+        <field name="perm_create" eval="False"/>
+        <field name="perm_unlink" eval="False"/>
+        <field name="groups" eval="[(4, ref('hr_holidays.group_hr_holidays_responsible'))]"/>
+    </record>
+
+    <record id="hr_leave_rule_responsible_update" model="ir.rule">
+        <field name="name">Time Off Responsible create/write</field>
+        <field name="model_id" ref="model_hr_leave"/>
+        <field name="domain_force">[
+            ('holiday_type', '=', 'employee'),
+            '|',
+                '&amp;',
+                    ('employee_id.user_id', '=', user.id),
+                    ('state', '!=', 'validate'),
+                ('employee_id.leave_manager_id', '=', user.id),
+        ]</field>
+        <field name="perm_read" eval="False"/>
+        <field name="perm_unlink" eval="False"/>
+        <field name="groups" eval="[(4, ref('hr_holidays.group_hr_holidays_responsible'))]"/>
+    </record>
+
+    <record id="hr_leave_rule_responsible_read" model="ir.rule">
+        <field name="name">Time Off All Approver read</field>
+        <field name="model_id" ref="model_hr_leave"/>
+        <field name="domain_force">[(1, '=', 1)]</field>
+        <field name="perm_write" eval="False"/>
+        <field name="perm_create" eval="False"/>
+        <field name="perm_unlink" eval="False"/>
+        <field name="groups" eval="[(4, ref('hr_holidays.group_hr_holidays_user'))]"/>
+    </record>
+
     <record id="hr_leave_rule_officer_update" model="ir.rule">
         <field name="name">Time Off All Approver create/write</field>
         <field name="model_id" ref="model_hr_leave"/>
@@ -193,5 +237,12 @@
         <field name="domain_force">['|', ('company_id', '=', False), ('company_id', 'in', company_ids)]</field>
     </record>
 
+    <record id="hr_leave_report_calendar_rule_multi_company" model="ir.rule">
+        <field name="name">Time Off Report Calendar: multi company global rule</field>
+        <field name="model_id" ref="model_hr_leave_report_calendar"/>
+        <field name="global" eval="True"/>
+        <field name="domain_force">['|', ('company_id', '=', False), ('company_id', 'in', company_ids)]</field>
+    </record>
+
     </data>
 </odoo>
diff --git a/addons/hr_holidays/security/ir.model.access.csv b/addons/hr_holidays/security/ir.model.access.csv
index 5c966878c8a..83de3ad7c7a 100644
--- a/addons/hr_holidays/security/ir.model.access.csv
+++ b/addons/hr_holidays/security/ir.model.access.csv
@@ -15,3 +15,4 @@ access_calendar_event_type_manager,calendar.event.type.manager,calendar.model_ca
 access_calendar_attendee_hr_user,calendar.attendee.hr.user,calendar.model_calendar_attendee,hr_holidays.group_hr_holidays_user,1,1,1,1
 access_mail_activity_type_holidays_manager,mail.activity.type.holidays.manager,mail.model_mail_activity_type,hr_holidays.group_hr_holidays_manager,1,1,1,1
 access_hr_holidays_summary_employee,access.hr.holidays.summary.employee,model_hr_holidays_summary_employee,hr_holidays.group_hr_holidays_user,1,1,1,0
+access_hr_leave_report_calendar,access_hr_leave_report_calendar,model_hr_leave_report_calendar,base.group_user,1,0,0,0
diff --git a/addons/hr_holidays/tests/test_access_rights.py b/addons/hr_holidays/tests/test_access_rights.py
index 0f77cf57caf..c82be054881 100644
--- a/addons/hr_holidays/tests/test_access_rights.py
+++ b/addons/hr_holidays/tests/test_access_rights.py
@@ -528,7 +528,7 @@ class TestAccessRightsRead(TestHrHolidaysAccessRightsCommon):
 
     @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 """
+        """ Users should not be able to read other people requests """
         other_leave = self.env['hr.leave'].with_user(self.user_hruser).create({
             'name': 'Test',
             'holiday_status_id': self.leave_type.id,
@@ -538,15 +538,12 @@ class TestAccessRightsRead(TestHrHolidaysAccessRightsCommon):
             'date_to': datetime.now() + relativedelta(days=1),
             'number_of_days': 1,
         })
-        res = other_leave.with_user(self.user_employee_id).read(['number_of_days', 'state', 'name'])
-        self.assertEqual(
-            res[0]['name'], '*****',
-            'Private information should have been stripped, received %s instead' % res[0]['name']
-        )
+        with self.assertRaises(AccessError), self.cr.savepoint():
+            res = other_leave.with_user(self.user_employee_id).read(['number_of_days', 'state', '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 """
+        """ Users should not be able to browse other people requests """
         other_leave = self.env['hr.leave'].with_user(self.user_hruser).create({
             'name': 'Test',
             'holiday_status_id': self.leave_type.id,
@@ -556,11 +553,9 @@ class TestAccessRightsRead(TestHrHolidaysAccessRightsCommon):
             'date_to': datetime.now() + relativedelta(days=1),
             'number_of_days': 1,
         })
-        other_leave.invalidate_cache(['name'])
-        self.assertEqual(
-            other_leave.with_user(self.user_employee_id).name, '*****',
-            'Private information should have been stripped, received %s instead' % other_leave.with_user(self.user_employee_id).name
-        )
+        with self.assertRaises(AccessError), self.cr.savepoint():
+            other_leave.invalidate_cache(['name'])
+            name = other_leave.with_user(self.user_employee_id).name
 
     @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail')
     def test_leave_read_by_user_own(self):
diff --git a/addons/hr_holidays/views/hr_holidays_views.xml b/addons/hr_holidays/views/hr_holidays_views.xml
index 832de6634fc..5357839bc06 100644
--- a/addons/hr_holidays/views/hr_holidays_views.xml
+++ b/addons/hr_holidays/views/hr_holidays_views.xml
@@ -50,6 +50,7 @@
         id="hr_holidays_menu_manager_approve"
         name="To Approve"
         parent="menu_hr_holidays_approvals"
+        groups="hr_holidays.group_hr_holidays_responsible,hr_holidays.group_hr_holidays_user"
         sequence="1"/>
 
     <menuitem
diff --git a/addons/hr_holidays/views/hr_leave_allocation_views.xml b/addons/hr_holidays/views/hr_leave_allocation_views.xml
index e2d66fb694b..265ca7c6fd7 100644
--- a/addons/hr_holidays/views/hr_leave_allocation_views.xml
+++ b/addons/hr_holidays/views/hr_leave_allocation_views.xml
@@ -200,7 +200,7 @@
                 <button string="Approve" name="action_approve" type="object"
                     icon="fa-check text-success"
                     states="confirm"
-                    groups="hr_holidays.group_hr_holidays_user"/>
+                    groups="hr_holidays.group_hr_holidays_responsible"/>
                 <button string="Validate" name="action_validate" type="object"
                     icon="fa-check text-success"
                     states="validate1"
@@ -371,8 +371,8 @@
         <field name="name">Department Allocation to Approve</field>
         <field name="res_model">hr.leave.allocation</field>
         <field name="view_mode">tree,form,kanban,activity</field>
-        <field name="context">{'search_default_managed_people':1, 'search_default_approve':1}</field>
-        <field name="domain">[]</field>
+        <field name="context">{'search_default_approve':1}</field>
+        <field name="domain">[('employee_id.leave_manager_id', '=', uid)]</field>
         <field name="help" type="html">
             <p class="o_view_nocontent_smiling_face">
                 Create a new time off allocation
diff --git a/addons/hr_holidays/views/hr_leave_views.xml b/addons/hr_holidays/views/hr_leave_views.xml
index e9945f98ea1..ddac0c7336c 100644
--- a/addons/hr_holidays/views/hr_leave_views.xml
+++ b/addons/hr_holidays/views/hr_leave_views.xml
@@ -501,7 +501,7 @@
                 <button string="Approve" name="action_approve" type="object"
                     icon="fa-check text-success"
                     states="confirm"
-                    groups="hr_holidays.group_hr_holidays_user"/>
+                    groups="hr_holidays.group_hr_holidays_responsible"/>
                 <button string="Validate" name="action_validate" type="object"
                     icon="fa-check text-success"
                     states="validate1"
@@ -546,18 +546,6 @@
         </field>
     </record>
 
-    <record id="action_hr_holidays_dashboard" model="ir.actions.act_window">
-        <field name="name">All Time Off</field>
-        <field name="res_model">hr.leave</field>
-        <field name="view_mode">calendar,tree,form</field>
-        <field name="context">{
-            'form_view_ref': 'hr_holidays.hr_leave_view_form_manager',
-        }</field>
-        <field name="domain">[('holiday_type','=','employee'), ('state', '!=', 'refuse')]</field>
-        <field name="search_view_id" ref="view_hr_holidays_filter"/>
-        <field name="view_id" />
-    </record>
-
     <record id="hr_leave_action_new_request" model="ir.actions.act_window">
         <field name="name">Dashboard</field>
         <field name="res_model">hr.leave</field>
@@ -663,11 +651,10 @@
         <field name="res_model">hr.leave</field>
         <field name="view_mode">tree,kanban,form,calendar,activity</field>
         <field name="context">{
-            'search_default_managed_people':1,
             'search_default_my_team_leaves':1,
             'search_default_approve':1}
         </field>
-        <field name="domain">[]</field>
+        <field name="domain">[('employee_id.leave_manager_id', '=', uid)]</field>
         <field name="help" type="html">
             <p class="o_view_nocontent_smiling_face">
                 Create a new time off request