attempted fix for access rights issues.

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",

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")

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",
+    ])

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",
+        compute="_compute_accessible_team_ids",
-        column1="user_id",
+        inverse="_inverse_accessible_team_ids",
-        column2="team_id",
-        string="Accessible Sports Teams",
     )
 
-    @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
+                ]
+            )

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",
+        compute="_compute_allowed_user_ids",
-        column1="team_id",
+        inverse="_inverse_allowed_user_ids",
-        column2="user_id",
-        string="Allowed Users",
-        domain=lambda self: [["groups_id", "in", self.env.ref("base.group_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(
+            rec.allowed_user_ids = rec.staff_ids.user_ids
-                lambda user: user.has_group("base.group_user")
+
+    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
+                ]
             )
 
 

bemade_sports_clinic/security/sports_clinic_rules.xml

@@ -5,7 +5,8 @@
         <record id="restrict_staff_access_to_team_players" model="ir.rule">
             <field name="name">Restrict Team Staff Access to Their Players Only</field>
             <field name="model_id" ref="model_sports_patient"/>
-            <field name="groups" eval="[(6, 0, [ref('base.group_portal')])]"/>
+            <field name="groups"
+                   eval="[(6, 0, [ref('base.group_portal'), ref('base.group_user')])]"/>
             <field name="perm_create" eval="True"/>
             <field name="perm_write" eval="True"/>
             <field name="perm_read" eval="True"/>
@@ -17,7 +18,8 @@
         <record id="restrict_staff_access_to_teams" model="ir.rule">
             <field name="name">Restrict Team Staff Access to Their Teams Only</field>
             <field name="model_id" ref="model_sports_team"/>
-            <field name="groups" eval="[(6, 0, [ref('base.group_portal')])]"/>
+            <field name="groups"
+                   eval="[(6, 0, [ref('base.group_portal'), ref('base.group_user')])]"/>
             <field name="perm_create" eval="True"/>
             <field name="perm_write" eval="True"/>
             <field name="perm_read" eval="True"/>
@@ -29,7 +31,8 @@
         <record id="restrict_staff_access_to_team_injuries" model="ir.rule">
             <field name="name">Restrict Team Staff Access to Their Teams Only</field>
             <field name="model_id" ref="model_sports_patient_injury"/>
-            <field name="groups" eval="[(6, 0, [ref('base.group_portal')])]"/>
+            <field name="groups"
+                   eval="[(6, 0, [ref('base.group_portal'), ref('base.group_user')])]"/>
             <field name="perm_create" eval="True"/>
             <field name="perm_write" eval="True"/>
             <field name="perm_read" eval="True"/>
@@ -38,44 +41,11 @@
                 [('patient_id.team_ids.staff_ids.user_ids', 'in', user.id)]
             </field>
         </record>
-        <record id="restrict_team_access_to_allowed_internal_users" model="ir.rule">
+        <record id="restrict_patient_contact_access_to_allowed_internal_users"
-            <field name="name">Restrict Team Access to Allowed Internal Users</field>
+                model="ir.rule">
-            <field name="model_id" ref="model_sports_team"/>
+            <field name="name">Restrict Patient Contact Access to Allowed Internal
-            <field name="groups" eval="[(6, 0, [ref('base.group_user')])]"/>
+                Users
-            <field name="perm_create" eval="True"/>
-            <field name="perm_write" eval="True"/>
-            <field name="perm_read" eval="True"/>
-            <field name="perm_unlink" eval="True"/>
-            <field name="domain_force">
-                [('allowed_user_ids', 'in', user.id)]
             </field>
-        </record>
-        <record id="restrict_patient_access_to_allowed_internal_users" model="ir.rule">
-            <field name="name">Restrict Patient Access to Allowed Internal Users</field>
-            <field name="model_id" ref="model_sports_patient"/>
-            <field name="groups" eval="[(6, 0, [ref('base.group_user')])]"/>
-            <field name="perm_create" eval="True"/>
-            <field name="perm_write" eval="True"/>
-            <field name="perm_read" eval="True"/>
-            <field name="perm_unlink" eval="True"/>
-            <field name="domain_force">
-                [('team_ids.allowed_user_ids', 'in', user.id)]
-            </field>
-        </record>
-        <record id="restrict_injury_access_to_allowed_internal_users" model="ir.rule">
-            <field name="name">Restrict Team Access to Allowed Internal Users</field>
-            <field name="model_id" ref="model_sports_patient_injury"/>
-            <field name="groups" eval="[(6, 0, [ref('base.group_user')])]"/>
-            <field name="perm_create" eval="True"/>
-            <field name="perm_write" eval="True"/>
-            <field name="perm_read" eval="True"/>
-            <field name="perm_unlink" eval="True"/>
-            <field name="domain_force">
-                [('patient_id.team_ids.allowed_user_ids', 'in', user.id)]
-            </field>
-        </record>
-        <record id="restrict_patient_contact_access_to_allowed_internal_users" model="ir.rule">
-            <field name="name">Restrict Patient Contact Access to Allowed Internal Users</field>
             <field name="model_id" ref="model_sports_patient_contact"/>
             <field name="groups" eval="[(6, 0, [ref('base.group_user')])]"/>
             <field name="perm_create" eval="True"/>
@@ -83,7 +53,7 @@
             <field name="perm_read" eval="True"/>
             <field name="perm_unlink" eval="True"/>
             <field name="domain_force">
-                [('patient_id.team_ids.allowed_user_ids', 'in', user.id)]
+                [('patient_id.team_ids.staff_ids.user_ids', 'in', user.id)]
             </field>
         </record>
         <record id="allow_team_access_to_admins" model="ir.rule">
@@ -126,7 +96,8 @@
             </field>
         </record>
         <record id="allow_patient_contact_access_to_admins" model="ir.rule">
-            <field name="name">Allow Sports Patient Contact Access to Administrators</field>
+            <field name="name">Allow Sports Patient Contact Access to Administrators
+            </field>
             <field name="model_id" ref="model_sports_patient_contact"/>
             <field name="groups"
                    eval="[(6, 0, [ref('base.group_system'), ref('bemade_sports_clinic.group_sports_clinic_admin')])]"/>

bemade_sports_clinic/tests/__init__.py

@@ -1 +1,2 @@
 from . import test_patient
+from . import test_rights

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