Fix migrations: use historical models to access model fields present …

…at the time of the migration, and current models to access the objects to assign permissions to

apps/accounts/migrations/0053_provider_permissions.py

@@ -1,85 +1,25 @@
 # Generated by Django 3.2.19 on 2023-05-23 15:28
 
 from django.db import migrations
-from django.contrib.auth import get_user_model
-from django.core.exceptions import FieldError
-from guardian.shortcuts import assign_perm, remove_perm
 from .. import permissions
 
 
-def assign_object_permissions(apps, schema_editor):
-    """
-    Assign object-level permissions for existing Hostingprovider and Datacenter objects
-    """
-    db_alias = schema_editor.connection.alias
-
-    # assign manage_datacenter
-    Datacenter = apps.get_model("accounts", "Datacenter")
-
-    for dc in Datacenter.objects.using(db_alias).filter(user__isnull=False).iterator():
-        assign_perm(permissions.manage_datacenter.codename, dc.user, dc)
-
-    # assign manage_provider
-    # GOTCHA: catch a FieldError so that this migration does not fail when execued by a test runner
-    User = get_user_model()
-    try:
-        for user in (
-            User.objects.using(db_alias)
-            .filter(hostingprovider__isnull=False)
-            .iterator()
-        ):
-            assign_perm(
-                permissions.manage_provider.codename, user, user.hostingprovider
-            )
-    except FieldError:
-        pass
-
-
-def remove_object_permissions(apps, schema_editor):
-    """
-    Remove object-level permissions for existing Hostingprovider and Datacenter objects
-    """
-    db_alias = schema_editor.connection.alias
-
-    # remove manage_datacenter
-    Datacenter = apps.get_model("accounts", "Datacenter")
-    for dc in Datacenter.objects.using(db_alias).filter(user__isnull=False).iterator():
-        remove_perm(permissions.manage_datacenter.codename, dc.user, dc)
-
-    # remove manage_provider
-    # GOTCHA: catch a FieldError so that this migration does not fail when execued by a test runner
-    User = get_user_model()
-    try:
-        for user in (
-            User.objects.using(db_alias)
-            .filter(hostingprovider__isnull=False)
-            .iterator()
-        ):
-            remove_perm(
-                permissions.manage_provider.codename, user, user.hostingprovider
-            )
-    except FieldError:
-        pass
-
-
 class Migration(migrations.Migration):
     dependencies = [
         ("accounts", "0052_merge_20230510_1057"),
+        ("guardian", "__latest__"),
     ]
 
     operations = [
         migrations.AlterModelOptions(
             name="datacenter",
-            options={"permissions": [permissions.manage_datacenter.astuple()]},
+            options={"permissions": (permissions.manage_datacenter.astuple(),)},
         ),
         migrations.AlterModelOptions(
             name="hostingprovider",
             options={
-                "permissions": [permissions.manage_provider.astuple()],
+                "permissions": (permissions.manage_provider.astuple(),),
                 "verbose_name": "Hosting Provider",
             },
         ),
-        migrations.RunPython(
-            code=assign_object_permissions, reverse_code=remove_object_permissions
-        ),
     ]

apps/accounts/migrations/0054_hostingprovider_datacenter_created_by.py

@@ -1,10 +1,7 @@
 # Generated by Django 3.2.19 on 2023-05-30 16:23
 
 from django.conf import settings
-from django.contrib.auth import get_user_model
 from django.db import migrations, models
-from django.core.exceptions import FieldError
-
 import django.db.models.deletion
 
 
@@ -14,19 +11,13 @@ def populate_hostingprovider_created_by(apps, schema_editor):
     """
     db_alias = schema_editor.connection.alias
 
-    User = get_user_model()
+    User = apps.get_model(settings.AUTH_USER_MODEL)
 
-    # GOTCHA: catch a FieldError so that this migration does not fail when execued by a test runner
-    try:
-        for user in (
-            User.objects.using(db_alias)
-            .filter(hostingprovider__isnull=False)
-            .iterator()
-        ):
-            user.hostingprovider.created_by = user
-            user.hostingprovider.save()
-    except FieldError:
-        pass
+    for user in (
+        User.objects.using(db_alias).filter(hostingprovider__isnull=False).iterator()
+    ):
+        user.hostingprovider.created_by = user
+        user.hostingprovider.save()
 
 
 class Migration(migrations.Migration):
@@ -64,5 +55,7 @@ class Migration(migrations.Migration):
                 to=settings.AUTH_USER_MODEL,
             ),
         ),
-        migrations.RunPython(populate_hostingprovider_created_by),
+        migrations.RunPython(
+            code=populate_hostingprovider_created_by, reverse_code=lambda *args: ...
+        ),
     ]

apps/accounts/migrations/0055_populate_existing_permissions.py

@@ -0,0 +1,127 @@
+# Generated by Django 3.2.19 on 2023-05-31 12:38
+
+from django.apps import apps as django_apps
+from django.db import migrations
+from guardian.shortcuts import assign_perm, remove_perm
+from django.contrib.contenttypes.models import ContentType
+from django.contrib.auth.models import Permission, Group
+from django.contrib.auth import get_user_model
+from ..permissions import manage_datacenter, manage_provider
+
+
+def _get_managed_perms(apps, db_alias):
+    Datacenter = apps.get_model("accounts", "Datacenter")
+    Hostingprovider = apps.get_model("accounts", "Hostingprovider")
+
+    dc_perm, _ = Permission.objects.using(db_alias).get_or_create(
+        codename=manage_datacenter.codename,
+        name=manage_datacenter.description,
+        content_type=ContentType.objects.get_for_model(Datacenter),
+    )
+    hp_perm, _ = Permission.objects.using(db_alias).get_or_create(
+        codename=manage_provider.codename,
+        name=manage_provider.description,
+        content_type=ContentType.objects.get_for_model(Hostingprovider),
+    )
+
+    return (hp_perm, dc_perm)
+
+
+def set_admin_global_perms(apps, schema_editor):
+    """
+    Assign global permissions (for all Hostingprovider and Datacenter objects)
+    for all members of the "admin" group.
+    """
+    db_alias = schema_editor.connection.alias
+    admin_group = Group.objects.using(db_alias).filter(name="admin").get()
+
+    for perm in _get_managed_perms(apps, db_alias):
+        assign_perm(perm, admin_group)
+
+
+def remove_admin_global_perms(apps, schema_editor):
+    """
+    Remove global permissions (for all Hostingprovider and Datacenter objects)
+    from all members of the "admin" group.
+    """
+    db_alias = schema_editor.connection.alias
+    admin_group = Group.objects.using(db_alias).filter(name="admin").get()
+
+    for perm in _get_managed_perms(apps, db_alias):
+        remove_perm(perm, admin_group)
+
+
+def _manage_permission(apps, schema_editor, manage_func):
+    """
+    Assign/Remove object-level permissions for existing
+    Hostingprovider and Datacenter objects.
+
+    Parameter `manage_func` is either: assign_perm or remove_perm from guardian.shortcuts
+    """
+    db_alias = schema_editor.connection.alias
+
+    hp_perm, dc_perm = _get_managed_perms(apps, db_alias)
+
+    # to query the objects eligible for assigning permissions we need the historical model
+    # so that we can use queries based on fields present at the moment of the migration
+    Datacenter_historical_model = apps.get_model("accounts", "Datacenter")
+    Hostingprovider_historical_model = apps.get_model("accounts", "Hostingprovider")
+
+    # retrieve data based on queryset using historical model to access "created_by" field
+    eligible_pairs_dc = [
+        (dc.id, dc.created_by.id)
+        for dc in Datacenter_historical_model.objects.using(db_alias).filter(
+            created_by__isnull=False
+        )
+    ]
+    eligible_pairs_hp = [
+        (hp.id, hp.created_by.id)
+        for hp in Hostingprovider_historical_model.objects.using(db_alias).filter(
+            created_by__isnull=False
+        )
+    ]
+    # Get current models to retrieve the objects for assigning permissions.
+    # This is dangerous! Future versions of django-guardian might change the schema
+    # and this migration might fail when ran from scratch. If this happens:
+    # - pin down the version of django-guardian to the latest working version
+    #   so that migrations work for tests
+    # - for production / staging it's a non-issue since we always keep snapshots around
+    #   and don't run older migrations
+    Datacenter = django_apps.get_model("accounts", "Datacenter")
+    Hostingprovider = django_apps.get_model("accounts", "Hostingprovider")
+    User = get_user_model()
+
+    # assign manage_datacenter
+    for dc_id, user_id in eligible_pairs_dc:
+        dc = Datacenter.objects.get(id=dc_id)
+        user = User.objects.get(id=user_id)
+        manage_func(dc_perm, user, dc)
+
+    # assign manage_provider
+    for hp_id, user_id in eligible_pairs_hp:
+        hp = Hostingprovider.objects.get(id=hp_id)
+        user = User.objects.using(db_alias).get(id=user_id)
+        manage_func(hp_perm, user, hp)
+
+
+def assign_object_permissions(apps, schema_editor):
+    _manage_permission(apps, schema_editor, assign_perm)
+
+
+def remove_object_permissions(apps, schema_editor):
+    _manage_permission(apps, schema_editor, remove_perm)
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("accounts", "0054_hostingprovider_datacenter_created_by"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            code=set_admin_global_perms, reverse_code=remove_admin_global_perms
+        ),
+        migrations.RunPython(
+            code=assign_object_permissions, reverse_code=remove_object_permissions
+        ),
+    ]

apps/accounts/migrations/0056_remove_user_hostingprovider_fk.py

@@ -5,7 +5,7 @@
 
 class Migration(migrations.Migration):
     dependencies = [
-        ("accounts", "0055_global_admin_permissions"),
+        ("accounts", "0055_populate_existing_permissions"),
     ]
 
     operations = [

apps/accounts/models/hosting.py

@@ -132,7 +132,7 @@ class Meta:
         indexes = [
             models.Index(fields=["name"], name="dc_name"),
         ]
-        permissions = [manage_datacenter.astuple()]
+        permissions = (manage_datacenter.astuple(),)
 
 
 class DatacenterClassification(models.Model):
@@ -515,7 +515,7 @@ class Meta:
             models.Index(fields=["archived"], name="hp_archived"),
             models.Index(fields=["showonwebsite"], name="hp_showonwebsite"),
         ]
-        permissions = [manage_provider.astuple()]
+        permissions = (manage_provider.astuple(),)
 
 
 class ProviderSharedSecret(TimeStampedModel):