From 8566adf98507e79e02beec3ea6d1907a65dc070e Mon Sep 17 00:00:00 2001 From: Matti Lamppu Date: Mon, 16 Dec 2024 12:24:47 +0200 Subject: [PATCH] Add a constraint to UnitRole for AD group based roles --- ..._unitrole_unique_role_user_for_ad_group.py | 22 +++++++++++++++++++ tilavarauspalvelu/models/unit_role/model.py | 8 +++++++ tilavarauspalvelu/models/user/actions.py | 8 ++++--- 3 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 tilavarauspalvelu/migrations/0055_unitrole_unique_role_user_for_ad_group.py diff --git a/tilavarauspalvelu/migrations/0055_unitrole_unique_role_user_for_ad_group.py b/tilavarauspalvelu/migrations/0055_unitrole_unique_role_user_for_ad_group.py new file mode 100644 index 000000000..7ef54657e --- /dev/null +++ b/tilavarauspalvelu/migrations/0055_unitrole_unique_role_user_for_ad_group.py @@ -0,0 +1,22 @@ +# Generated by Django 5.1.3 on 2024-12-16 10:21 +from __future__ import annotations + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("tilavarauspalvelu", "0054_unit_allow_permissions_from_ad_groups_and_more"), + ] + + operations = [ + migrations.AddConstraint( + model_name="unitrole", + constraint=models.UniqueConstraint( + condition=models.Q(("is_from_ad_group", True)), + fields=("role", "user"), + name="unique_role_user_if_is_from_ad_group", + violation_error_message="Role for user must be unique for AG group based roles.", + ), + ), + ] diff --git a/tilavarauspalvelu/models/unit_role/model.py b/tilavarauspalvelu/models/unit_role/model.py index 4c051a021..70843a862 100644 --- a/tilavarauspalvelu/models/unit_role/model.py +++ b/tilavarauspalvelu/models/unit_role/model.py @@ -52,6 +52,14 @@ class Meta: verbose_name = _("unit role") verbose_name_plural = _("unit roles") ordering = ["pk"] + constraints = [ + models.UniqueConstraint( + fields=["role", "user"], + name="unique_role_user_if_is_from_ad_group", + condition=models.Q(is_from_ad_group=True), + violation_error_message="Role for user must be unique for AG group based roles.", + ), + ] def __str__(self) -> str: return f"Unit Role '{self.role}' for {self.user.first_name} {self.user.last_name} ({self.user.email})" diff --git a/tilavarauspalvelu/models/user/actions.py b/tilavarauspalvelu/models/user/actions.py index 28c31cbb0..31f137639 100644 --- a/tilavarauspalvelu/models/user/actions.py +++ b/tilavarauspalvelu/models/user/actions.py @@ -244,6 +244,8 @@ def get_ad_group_roles(self) -> dict[UserRoleChoice, set[int]]: def update_unit_roles_from_ad_groups(self) -> None: current_ad_roles = self.get_ad_group_roles() + # Assume here that we have one UnitRole per AD group role. + # This assertion is backed up by a unique constraint on UnitRole. existing_ad_roles = { UserRoleChoice(role.role): role # for role in self.user.unit_roles.filter(is_from_ad_group=True).prefetch_related("units") @@ -268,17 +270,17 @@ def update_unit_roles_from_ad_groups(self) -> None: ) continue - exiting_unit_ids: set[int] = {unit.id for unit in role.units.all()} + existing_unit_ids: set[int] = {unit.id for unit in role.units.all()} # If current role has been added to new units, add them to the role. - new_unit_ids = current_unit_ids - exiting_unit_ids + new_unit_ids = current_unit_ids - existing_unit_ids if new_unit_ids: role_units_to_add.extend( # UnitRoleUnit(unitrole=role, unit_id=unit_id) for unit_id in new_unit_ids ) # If current role no longer includes some units, remove those units from the role. - old_unit_ids = exiting_unit_ids - current_unit_ids + old_unit_ids = existing_unit_ids - current_unit_ids if old_unit_ids: condition = models.Q(unit_id__in=list(old_unit_ids), unitrole_id=role.id) role_unit_remove_conditions.append(condition)