Add a constraint to UnitRole for AD group based roles

tilavarauspalvelu/migrations/0056_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", "0055_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 AD group based roles.",
+            ),
+        ),
+    ]

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

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)