Skip to content

Commit

Permalink
[FIX] hr_employee_document: Remove record rules from hr.employee to a…
Browse files Browse the repository at this point in the history
…void side effects.

The purpose of this module is to allow a basic user (without hr permissions) to view
the attachments related to his employee (hr.employee).

By default a basic user cannot access to hr.employee model and therefore
cannot see the attachments (ir.attachment) linked to it.

This change overrides some things to allow the user's employee attachments to be displayed.

TT44536
  • Loading branch information
victoralmau committed Jul 21, 2023
1 parent bcbc4bf commit e67de4b
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 47 deletions.
1 change: 0 additions & 1 deletion hr_employee_document/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
"summary": "Documents attached to the employee profile",
"depends": ["hr"],
"data": [
"security/security.xml",
"views/hr_employee.xml",
"views/hr_employee_public.xml",
],
Expand Down
1 change: 1 addition & 0 deletions hr_employee_document/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@

from . import hr_employee
from . import hr_employee_public
from . import ir_attachment
20 changes: 19 additions & 1 deletion hr_employee_document/models/hr_employee.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Copyright 2018 Brainbean Apps (https://brainbeanapps.com)
# Copyright 2023 Tecnativa - Víctor Martínez
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from odoo import fields, models
from odoo import api, fields, models


class HrEmployeeBase(models.AbstractModel):
Expand All @@ -22,6 +23,23 @@ def _compute_document_count(self):
for record in self:
record.document_count = count_dict.get(record.id, 0)

@api.model
def check_access_rights(self, operation, raise_exception=True):
"""We need to avoid an access error that would occur when trying to access
the user's employee record in order to view their attachments.
This overwrite will only be necessary if the user does not have HR group."""
if (
not self.env.is_superuser()
and not self.env.user.has_group("hr.group_hr_user")
and operation == "read"
and self._name == "hr.employee"
):
if self == self.env.user.employee_ids:
raise_exception = False

Check warning on line 38 in hr_employee_document/models/hr_employee.py

View check run for this annotation

Codecov / codecov/patch

hr_employee_document/models/hr_employee.py#L38

Added line #L38 was not covered by tests
return super().check_access_rights(
operation=operation, raise_exception=raise_exception
)

def action_get_attachment_tree_view(self):
action = self.env["ir.actions.act_window"]._for_xml_id("base.action_attachment")
action["context"] = {
Expand Down
67 changes: 67 additions & 0 deletions hr_employee_document/models/ir_attachment.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Copyright 2023 Tecnativa - Víctor Martínez
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from odoo import api, models


class IrAttachment(models.Model):
_inherit = "ir.attachment"

@api.model
def _search(
self,
args,
offset=0,
limit=None,
order=None,
count=False,
access_rights_uid=None,
):
"""The base module in ir.attachment removes records to which you do not have
permission, which is correct, but in the case of hr.employee not exactly,
since you should have access to the user's employee attachments.
To avoid creating ACLS related to hr.employee that would cause side effects,
we will apply sudo to get records when necessary.
This overwrite will only be necessary if the user does not have HR group."""
res = super()._search(
args=args,
offset=offset,
limit=limit,
order=order,
count=count,
access_rights_uid=access_rights_uid,
)
if (
not self.env.user.has_group("hr.group_hr_user")
and len(res) == 0
and not self.env.context.get("skip_override_ir_attachment_search")
):
args_to_check_0 = False
args_to_check_1 = False
for arg in args:
if isinstance(args, (list)):
if (
arg[0] == "res_model"
and arg[1] == "="
and arg[2] == "hr.employee"
):
args_to_check_0 = True
elif (
arg[0] == "res_id"
and arg[1] == "="
and arg[2] == self.env.user.employee_id.id
):
args_to_check_1 = True
if args_to_check_0 and args_to_check_1:
_self = self.sudo().with_context(
skip_override_ir_attachment_search=True
)
return super(IrAttachment, _self)._search(
args=args,
offset=offset,
limit=limit,
order=order,
count=count,
access_rights_uid=access_rights_uid,
)
return res
32 changes: 0 additions & 32 deletions hr_employee_document/security/security.xml

This file was deleted.

32 changes: 19 additions & 13 deletions hr_employee_document/tests/test_hr_employee_document.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Copyright 2018 Brainbean Apps (https://brainbeanapps.com)
# Copyright 2021-2022 Tecnativa - Víctor Martínez
# Copyright 2021-2023 Tecnativa - Víctor Martínez
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).

import base64
Expand All @@ -11,17 +11,19 @@
class TestHrEmployeeDocument(common.TransactionCase):
def setUp(self):
super().setUp()
ctx = {
"mail_create_nolog": True,
"mail_create_nosubscribe": True,
"mail_notrack": True,
"no_reset_password": True,
}
self.user_1 = new_test_user(self.env, login="test-user-1", context=ctx)
self.user_2 = new_test_user(self.env, login="test-user-2", context=ctx)
new_test_user(
self.env, login="test-user-manager", groups="hr.group_hr_user", context=ctx
self.env = self.env(
context=dict(
self.env.context,
mail_create_nolog=True,
mail_create_nosubscribe=True,
mail_notrack=True,
no_reset_password=True,
tracking_disable=True,
)
)
self.user_1 = new_test_user(self.env, login="test-user-1")
self.user_2 = new_test_user(self.env, login="test-user-2")
new_test_user(self.env, login="test-user-manager", groups="hr.group_hr_user")
self.employee_1 = self.env["hr.employee"].create(
{"name": "Employee #1", "user_id": self.user_1.id}
)
Expand Down Expand Up @@ -63,7 +65,9 @@ def test_attachments_access_user_1(self):
# create attachments
attachment_1 = self._create_attachment(self.employee_1)
attachment_2 = self._create_attachment(self.employee_2)
records = self.env["ir.attachment"].search([])
records = self.env["ir.attachment"].search(
[("res_model", "=", "hr.employee"), ("res_id", "=", self.employee_1.id)]
)
self.assertIn(attachment_1, records)
self.assertNotIn(attachment_2, records)

Expand All @@ -72,7 +76,9 @@ def test_attachments_access_user_2(self):
# create attachments
attachment_1 = self._create_attachment(self.employee_1)
attachment_2 = self._create_attachment(self.employee_2)
records = self.env["ir.attachment"].search([])
records = self.env["ir.attachment"].search(
[("res_model", "=", "hr.employee"), ("res_id", "=", self.employee_2.id)]
)
self.assertNotIn(attachment_1, records)
self.assertIn(attachment_2, records)

Expand Down

0 comments on commit e67de4b

Please sign in to comment.