From 9ed035a938f9262820d5ae4f9112b4c52f3a4b12 Mon Sep 17 00:00:00 2001 From: Marc Durepos Date: Wed, 18 Sep 2024 14:18:31 -0400 Subject: [PATCH] attempted fix for access rights issues. --- bemade_sports_clinic/__manifest__.py | 7 +- .../migrations/16.0.1.7.0/post-migrate.py | 6 + .../migrations/16.0.1.7.0/pre-migrate.py | 10 ++ bemade_sports_clinic/models/res_users.py | 44 +++++-- bemade_sports_clinic/models/sports_team.py | 30 +++-- .../security/sports_clinic_rules.xml | 55 ++------- bemade_sports_clinic/tests/__init__.py | 1 + bemade_sports_clinic/tests/test_rights.py | 111 ++++++++++++++++++ 8 files changed, 203 insertions(+), 61 deletions(-) create mode 100644 bemade_sports_clinic/migrations/16.0.1.7.0/post-migrate.py create mode 100644 bemade_sports_clinic/migrations/16.0.1.7.0/pre-migrate.py create mode 100644 bemade_sports_clinic/tests/test_rights.py diff --git a/bemade_sports_clinic/__manifest__.py b/bemade_sports_clinic/__manifest__.py index 4a66b75..bf12626 100644 --- a/bemade_sports_clinic/__manifest__.py +++ b/bemade_sports_clinic/__manifest__.py @@ -18,7 +18,7 @@ # { "name": "Sports Clinic Management", - "version": "16.0.1.6.0", + "version": "16.0.1.7.0", "summary": "Manage the patients of a sports medicine clinic.", "description": """ Adds the notion of sports teams, players (patients), coaches and treatment @@ -38,6 +38,11 @@ "website": "https://www.bemade.org", "license": "OPL-1", "depends": ["portal", "contacts"], + "external_dependencies": { + "python": [ + "openupgradelib", + ], + }, "data": [ "security/sports_clinic_groups.xml", "security/ir.model.access.csv", diff --git a/bemade_sports_clinic/migrations/16.0.1.7.0/post-migrate.py b/bemade_sports_clinic/migrations/16.0.1.7.0/post-migrate.py new file mode 100644 index 0000000..f96358f --- /dev/null +++ b/bemade_sports_clinic/migrations/16.0.1.7.0/post-migrate.py @@ -0,0 +1,6 @@ +""" Patient access revised to make the team staff relationship central to +access. Everything is calculated from there. Inverse functions deal with +sports.team.staff records instead of having a separate table for storing +access rights.""" +def migrate(cr, version): + cr.execute("DROP TABLE sports_team_res_users_rel") \ No newline at end of file diff --git a/bemade_sports_clinic/migrations/16.0.1.7.0/pre-migrate.py b/bemade_sports_clinic/migrations/16.0.1.7.0/pre-migrate.py new file mode 100644 index 0000000..49ef708 --- /dev/null +++ b/bemade_sports_clinic/migrations/16.0.1.7.0/pre-migrate.py @@ -0,0 +1,10 @@ +import openupgradelib.openupgrade as ou +from odoo import SUPERUSER_ID, api + +def migrate(cr, version): + env = api.Environment(cr, SUPERUSER_ID, {}) + ou.delete_records_safely_by_xml_id(env, [ + "bemade_sports_clinic.restrict_team_access_to_allowed_internal_users", + "bemade_sports_clinic.restrict_patient_access_to_allowed_internal_users", + "bemade_sports_clinic.restrict_injury_access_to_allowed_internal_users", + ]) \ No newline at end of file diff --git a/bemade_sports_clinic/models/res_users.py b/bemade_sports_clinic/models/res_users.py index e522245..d39f45a 100644 --- a/bemade_sports_clinic/models/res_users.py +++ b/bemade_sports_clinic/models/res_users.py @@ -1,22 +1,48 @@ -from odoo import models, fields, api, _ +from odoo import models, fields, api, _, Command class User(models.Model): - _inherit = 'res.users' + _inherit = "res.users" is_treatment_professional = fields.Boolean( - compute="_compute_is_treatment_professional", store=True) + compute="_compute_is_treatment_professional", store=True + ) accessible_team_ids = fields.Many2many( comodel_name="sports.team", - relation="sports_team_res_users_rel", - column1="user_id", - column2="team_id", - string="Accessible Sports Teams", + compute="_compute_accessible_team_ids", + inverse="_inverse_accessible_team_ids", ) - @api.depends('groups_id') + @api.depends("groups_id") def _compute_is_treatment_professional(self): for rec in self: rec.is_treatment_professional = rec.has_group( - 'bemade_sports_clinic.group_sports_clinic_treatment_professional') + "bemade_sports_clinic.group_sports_clinic_treatment_professional" + ) + + def _compute_accessible_team_ids(self): + for rec in self: + rec.accessible_team_ids = rec.partner_id.staff_ids.mapped("team_id") + + def _inverse_accessible_team_ids(self): + for rec in self: + removed_teams = ( + rec.partner_id.staff_ids.mapped("team_id") - rec.accessible_team_ids + ) + added_teams = rec.accessible_team_ids - rec.partner_id.staff_ids.mapped( + "team_id" + ) + rec.partner_id.staff_ids.filtered( + lambda team: team in removed_teams + ).unlink() + self.env["sports.team.staff"].create( + [ + { + "team_id": team.id, + "partner_id": [Command.set([rec.id])], + "role": "other", + } + for team in added_teams + ] + ) diff --git a/bemade_sports_clinic/models/sports_team.py b/bemade_sports_clinic/models/sports_team.py index 2fd671d..78c6dbf 100644 --- a/bemade_sports_clinic/models/sports_team.py +++ b/bemade_sports_clinic/models/sports_team.py @@ -52,18 +52,14 @@ class SportsTeam(models.Model): website = fields.Char() allowed_user_ids = fields.Many2many( comodel_name="res.users", - relation="sports_team_res_users_rel", - column1="team_id", - column2="user_id", - string="Allowed Users", - domain=lambda self: [["groups_id", "in", self.env.ref("base.group_user").ids]], + compute="_compute_allowed_user_ids", + inverse="_inverse_allowed_user_ids", ) def write(self, vals): previous_patient_ids = self.patient_ids res = super().write(vals) if "staff_ids" in vals or "patient_ids" in vals: - self._allow_access_for_staff_internal_users() (self.patient_ids | previous_patient_ids).recompute_followers() return res @@ -101,10 +97,26 @@ class SportsTeam(models.Model): staff = rec.staff_ids.filtered(lambda r: r.role == "head_therapist") rec.head_therapist_id = staff.partner_id if staff else False - def _allow_access_for_staff_internal_users(self): + def _compute_allowed_user_ids(self): for rec in self: - rec.allowed_user_ids |= rec.staff_ids.user_ids.filtered( - lambda user: user.has_group("base.group_user") + rec.allowed_user_ids = rec.staff_ids.user_ids + + def _inverse_allowed_user_ids(self): + for rec in self: + removed_staff = rec.staff_ids.filtered( + lambda staff: staff.user_ids not in rec.allowed_user_ids + ) + added_users = rec.allowed_user_ids - rec.staff_ids.user_ids + removed_staff.unlink() + self.env["sports.team.staff"].create( + [ + { + "team_id": rec.id, + "partner_id": user.partner_id.id, + "role": "other", + } + for user in added_users + ] ) diff --git a/bemade_sports_clinic/security/sports_clinic_rules.xml b/bemade_sports_clinic/security/sports_clinic_rules.xml index 78a97b1..9cfe05b 100644 --- a/bemade_sports_clinic/security/sports_clinic_rules.xml +++ b/bemade_sports_clinic/security/sports_clinic_rules.xml @@ -5,7 +5,8 @@ Restrict Team Staff Access to Their Players Only - + @@ -17,7 +18,8 @@ Restrict Team Staff Access to Their Teams Only - + @@ -29,7 +31,8 @@ Restrict Team Staff Access to Their Teams Only - + @@ -38,44 +41,11 @@ [('patient_id.team_ids.staff_ids.user_ids', 'in', user.id)] - - Restrict Team Access to Allowed Internal Users - - - - - - - - [('allowed_user_ids', 'in', user.id)] + + Restrict Patient Contact Access to Allowed Internal + Users - - - Restrict Patient Access to Allowed Internal Users - - - - - - - - [('team_ids.allowed_user_ids', 'in', user.id)] - - - - Restrict Team Access to Allowed Internal Users - - - - - - - - [('patient_id.team_ids.allowed_user_ids', 'in', user.id)] - - - - Restrict Patient Contact Access to Allowed Internal Users @@ -83,7 +53,7 @@ - [('patient_id.team_ids.allowed_user_ids', 'in', user.id)] + [('patient_id.team_ids.staff_ids.user_ids', 'in', user.id)] @@ -126,7 +96,8 @@ - Allow Sports Patient Contact Access to Administrators + Allow Sports Patient Contact Access to Administrators + diff --git a/bemade_sports_clinic/tests/__init__.py b/bemade_sports_clinic/tests/__init__.py index 145e1df..976c651 100644 --- a/bemade_sports_clinic/tests/__init__.py +++ b/bemade_sports_clinic/tests/__init__.py @@ -1 +1,2 @@ from . import test_patient +from . import test_rights diff --git a/bemade_sports_clinic/tests/test_rights.py b/bemade_sports_clinic/tests/test_rights.py new file mode 100644 index 0000000..b3e306a --- /dev/null +++ b/bemade_sports_clinic/tests/test_rights.py @@ -0,0 +1,111 @@ +from odoo.tests import TransactionCase, Form +from odoo.fields import Date +from datetime import timedelta +from odoo.exceptions import AccessError + + +class TestRights(TransactionCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + # Create one admin user + cls.admin_user = cls.env["res.users"].create( + { + "name": "Admin User", + "login": "admin", + "password": "admin", + "groups_id": [ + ( + 6, + 0, + [ + cls.env.ref( + "bemade_sports_clinic.group_sports_clinic_admin" + ), + ], + ) + ], + } + ) + # Create one treatment professional user + cls.treatment_professional_user = cls.env["res.users"].create( + { + "name": "Treatment Professional User", + "login": "treatment_professional", + "password": "treatment_professional", + "groups_id": [ + ( + 6, + 0, + [ + cls.env.ref( + "bemade_sports_clinic.group_sports_clinic_treatment_professional" + ).id + ], + ) + ], + } + ) + + def test_treatment_pro_has_access_only_to_staffed_teams(self): + """A treatment professional should only have access to teams and, + by extension, patients for which they are a team staff member.""" + team, patients = self._generate_team_with_patient(self.admin_user) + with self.assertRaises(AccessError): + Form( + self.env["sports.team"] + .with_user(self.treatment_professional_user) + .browse(team.id) + ) + with self.assertRaises(AccessError): + Form( + self.env["sports.patient"] + .with_user(self.treatment_professional_user) + .browse(patients.ids) + ) + + def test_treatment_pro_can_remove_patient_from_team(self): + team, patients = self._generate_team_with_patient(self.admin_user) + self.env['sports.team.staff'].create({ + "team_id": team.id, + "partner_id": self.treatment_professional_user.id, + "role": "head_therapist", + }) + # Test removing the patient since we are team staff + # Should not throw an error... + with Form(team.with_user(self.treatment_professional_user)) as team: + team.patient_ids.remove(index=0) + self.assertEqual(len(team.patient_ids), 1) + + def _generate_team_with_patient(self, user=None): + user = user or self.env.user + team = ( + self.env["sports.team"] + .with_user(self.user) + .create( + { + "name": "Test Team", + } + ) + ) + patients = ( + self.env["sports.patient"] + .with_user(self.user) + .create( + [ + { + "first_name": "Test", + "last_name": "Patient One", + "date_of_birth": Date.today() - timedelta(days=-365 * 18), + "team_ids": [6, 0, team.ids], + }, + { + "first_name": "Test", + "last_name": "Patient Two", + "date_of_birth": Date.today() - timedelta(days=-365 * 18), + "team_ids": [6, 0, team.ids], + }, + ] + ) + ) + return team, patients \ No newline at end of file