Remove use of on_commit

Author: jmsmkn

app/grandchallenge/archives/signals.py

@@ -1,22 +1,42 @@
 from django.db.models.signals import m2m_changed, post_save, pre_delete
 from django.db.transaction import on_commit
 from django.dispatch import receiver
+from guardian.shortcuts import assign_perm
 
 from grandchallenge.archives.models import ArchiveItem
 from grandchallenge.cases.models import Image
 
 
 @receiver(m2m_changed, sender=ArchiveItem.values.through)
 def update_permissions_on_archive_item_changed(
-    instance, action, reverse, pk_set, **_
+    *, instance, action, reverse, model, pk_set, **_
 ):
     if action not in ["post_add", "post_remove", "pre_clear"]:
         # nothing to do for the other actions
         return
 
     if reverse:
         images = Image.objects.filter(componentinterfacevalue__pk=instance.pk)
+
+        if pk_set is None:
+            # When using a _clear action, pk_set is None
+            # https://docs.djangoproject.com/en/2.2/ref/signals/#m2m-changed
+            archive_items = instance.archive_items.all()
+        else:
+            archive_items = model.objects.filter(pk__in=pk_set)
+
+        archive_items = archive_items.select_related(
+            "archive__editors_group",
+            "archive__uploaders_group",
+            "archive__users_group",
+        ).only(
+            "archive__editors_group",
+            "archive__uploaders_group",
+            "archive__users_group",
+        )
     else:
+        archive_items = [instance]
+
         if pk_set is None:
             # When using a _clear action, pk_set is None
             # https://docs.djangoproject.com/en/2.2/ref/signals/#m2m-changed
@@ -29,11 +49,23 @@ def update_permissions_on_archive_item_changed(
                 componentinterfacevalue__pk__in=pk_set
             )
 
-    def update_permissions():
-        for image in images:
-            image.update_viewer_groups_permissions()
+    if action == "post_add":
+        for archive_item in archive_items:
+            for image in images:
+                groups = [
+                    archive_item.archive.editors_group,
+                    archive_item.archive.uploaders_group,
+                    archive_item.archive.users_group,
+                ]
+                assign_perm("view_image", groups, image)
 
-    on_commit(update_permissions)
+    elif action in {"post_remove", "pre_clear"}:
+        for image in images:
+            image.update_viewer_groups_permissions(
+                exclude_archive_items=archive_items
+            )
+    else:
+        raise NotImplementedError
 
 
 @receiver(pre_delete, sender=ArchiveItem)

app/grandchallenge/cases/models.py

@@ -670,7 +670,10 @@ def sitk_image(self):
         return sitk_image
 
     def update_viewer_groups_permissions(  # noqa: C901
-        self, *, exclude_jobs=None
+        self,
+        *,
+        exclude_jobs=None,
+        exclude_archive_items=None,
     ):
         """
         Update the permissions for the algorithm jobs viewers groups to
@@ -685,7 +688,7 @@ def update_viewer_groups_permissions(  # noqa: C901
             signal is sent.
         """
         from grandchallenge.algorithms.models import Job
-        from grandchallenge.archives.models import Archive
+        from grandchallenge.archives.models import ArchiveItem
         from grandchallenge.reader_studies.models import ReaderStudy
 
         expected_groups = set()
@@ -709,14 +712,31 @@ def update_viewer_groups_permissions(  # noqa: C901
                 logger.error(error, exc_info=True)
                 raise
 
-        for archive in Archive.objects.filter(
-            items__values__image=self
-        ).select_related("editors_group", "uploaders_group", "users_group"):
+        archive_items_queryset = (
+            ArchiveItem.objects.filter(values__image=self)
+            .select_related(
+                "archive__editors_group",
+                "archive__uploaders_group",
+                "archive__users_group",
+            )
+            .only(
+                "archive__editors_group",
+                "archive__uploaders_group",
+                "archive__users_group",
+            )
+        )
+
+        if exclude_archive_items is not None:
+            archive_items_queryset = archive_items_queryset.exclude(
+                pk__in={ai.pk for ai in exclude_archive_items}
+            )
+
+        for archive_item in archive_items_queryset:
             expected_groups.update(
                 [
-                    archive.editors_group,
-                    archive.uploaders_group,
-                    archive.users_group,
+                    archive_item.archive.editors_group,
+                    archive_item.archive.uploaders_group,
+                    archive_item.archive.users_group,
                 ]
             )