Streamline CIV creation and validation code for archive items and display sets

app/grandchallenge/algorithms/forms.py

@@ -107,7 +107,7 @@ def __init__(self, *args, algorithm, user, **kwargs):
                 instance=inp,
                 initial=inp.default_value,
                 user=self._user,
-                required=(inp.kind != InterfaceKindChoices.BOOL),
+                required=inp.value_required,
                 help_text=clean(inp.description) if inp.description else "",
             ).field
 

app/grandchallenge/archives/models.py

@@ -12,7 +12,10 @@
 
 from grandchallenge.algorithms.models import Algorithm
 from grandchallenge.anatomy.models import BodyStructure
-from grandchallenge.components.models import ComponentInterfaceValue
+from grandchallenge.components.models import (
+    CIVForObjectMixin,
+    ComponentInterfaceValue,
+)
 from grandchallenge.core.models import RequestBase, UUIDModel
 from grandchallenge.core.storage import (
     get_logo_path,
@@ -249,7 +252,7 @@ class ArchiveGroupObjectPermission(GroupObjectPermissionBase):
     content_object = models.ForeignKey(Archive, on_delete=models.CASCADE)
 
 
-class ArchiveItem(UUIDModel):
+class ArchiveItem(CIVForObjectMixin, UUIDModel):
     archive = models.ForeignKey(
         Archive, related_name="items", on_delete=models.PROTECT
     )
@@ -295,6 +298,10 @@ def assign_permissions(self):
             self,
         )
 
+    @property
+    def base_object(self):
+        return self.archive
+
 
 class ArchiveItemUserObjectPermission(UserObjectPermissionBase):
     content_object = models.ForeignKey(ArchiveItem, on_delete=models.CASCADE)

app/grandchallenge/archives/serializers.py

@@ -1,14 +1,9 @@
-from django.db.transaction import on_commit
 from rest_framework import serializers
-from rest_framework.exceptions import ValidationError
+from rest_framework.exceptions import ValidationError as DRFValidationError
 from rest_framework.fields import JSONField, ReadOnlyField, URLField
 from rest_framework.relations import HyperlinkedRelatedField
 
 from grandchallenge.archives.models import Archive, ArchiveItem
-from grandchallenge.archives.tasks import (
-    start_archive_item_update_tasks,
-    update_archive_item_update_kwargs,
-)
 from grandchallenge.components.serializers import (
     ComponentInterfaceValuePostSerializer,
     HyperlinkedComponentInterfaceValueSerializer,
@@ -94,41 +89,17 @@ def __init__(self, *args, **kwargs):
 
     def create(self, validated_data):
         if validated_data.pop("values") != []:
-            raise ValidationError("Values can only be added via update")
+            raise DRFValidationError("Values can only be added via update")
         return super().create(validated_data)
 
     def update(self, instance, validated_data):
-        civs = validated_data.pop("values")
-
-        civ_pks_to_add = set()
-        upload_pks = {}
-
-        for civ in civs:
-            interface = civ.pop("interface", None)
-            upload_session = civ.pop("upload_session", None)
-            value = civ.pop("value", None)
-            image = civ.pop("image", None)
-            user_upload = civ.pop("user_upload", None)
-
-            update_archive_item_update_kwargs(
-                instance=instance,
-                interface=interface,
-                value=value,
-                image=image,
-                user_upload=user_upload,
-                upload_session=upload_session,
-                civ_pks_to_add=civ_pks_to_add,
-                upload_pks=upload_pks,
+        values = validated_data.pop("values")
+        for value in values:
+            interface = value.get("interface", None)
+            user_upload = value.get("user_upload", None)
+            image = value.get("image", None)
+            value = value.get("value", None)
+            instance.create_civ(
+                ci_slug=interface.slug, new_value=user_upload or image or value
             )
-
-        on_commit(
-            start_archive_item_update_tasks.signature(
-                kwargs={
-                    "archive_item_pk": instance.pk,
-                    "civ_pks_to_add": list(civ_pks_to_add),
-                    "upload_pks": upload_pks,
-                }
-            ).apply_async
-        )
-
         return instance

app/grandchallenge/archives/static/archives/js/archive_item_update.mjs

@@ -1,5 +1,5 @@
 $('#ajaxDataTable').on( 'init.dt', function() {
-   allSelectElements = document.querySelectorAll('[id^="interfaceSelect"]');
+   let allSelectElements = document.querySelectorAll('[id^="interfaceSelect"]');
    allSelectElements.forEach(function(elem) {
         elem.addEventListener("change", loadUpdateView);
    });

app/grandchallenge/archives/tasks.py

@@ -1,11 +1,10 @@
-from celery import chain, group, shared_task
+from celery import shared_task
 from django.conf import settings
 from django.db import transaction
 from django.db.transaction import on_commit
 
 from grandchallenge.archives.models import Archive, ArchiveItem
 from grandchallenge.cases.models import Image, RawImageUploadSession
-from grandchallenge.cases.tasks import build_images
 from grandchallenge.components.models import (
     ComponentInterface,
     ComponentInterfaceValue,
@@ -72,114 +71,3 @@ def add_images_to_archive_item(
                 immutable=True,
             ).apply_async
         )
-
-
-def update_archive_item_update_kwargs(
-    instance,
-    interface,
-    civ_pks_to_add,
-    upload_pks,
-    value=None,
-    image=None,
-    user_upload=None,
-    upload_session=None,
-):
-    """
-    Given an interface and a value/image/user_upload/upload_session, this task
-    determines whether to create a new CIV for the specified archive item instance
-    with those values, and whether to delete any existing CIVs from the archive item.
-    It appends the respective CIV pk(s) to the set of to be added and removed
-    civs and returns those. If an upload_session is specified,
-    it also appends the session pk together with the new civ pk to the list of
-    to be processed images.
-    """
-    with transaction.atomic():
-        if interface.is_image_kind:
-            if image:
-                civ, created = ComponentInterfaceValue.objects.get_or_create(
-                    interface=interface, image=image
-                )
-                if created:
-                    civ.full_clean()
-                    civ.save()
-            elif upload_session:
-                civ = ComponentInterfaceValue.objects.create(
-                    interface=interface
-                )
-                upload_pks[civ.pk] = upload_session.pk
-                civ.save()
-            civ_pks_to_add.add(civ.pk)
-        elif interface.requires_file:
-            civ = ComponentInterfaceValue.objects.create(interface=interface)
-            user_upload.copy_object(to_field=civ.file)
-            civ.full_clean()
-            civ.save()
-            user_upload.delete()
-            civ_pks_to_add.add(civ.pk)
-        else:
-            civ = interface.create_instance(value=value)
-            civ_pks_to_add.add(civ.pk)
-
-
-@shared_task(**settings.CELERY_TASK_DECORATOR_KWARGS["acks-late-micro-short"])
-def update_archive_item_values(*, archive_item_pk, civ_pks_to_add):
-    instance = ArchiveItem.objects.get(pk=archive_item_pk)
-    civ_pks_to_remove = []
-    civs = ComponentInterfaceValue.objects.filter(pk__in=civ_pks_to_add)
-    for civ in civs:
-        if instance.values.filter(interface=civ.interface.pk).exists():
-            for civ_pk in instance.values.filter(
-                interface=civ.interface.pk
-            ).values_list("pk", flat=True):
-                civ_pks_to_remove.append(civ_pk)
-        # for images, check if there are any CIVs with the provided image
-        # this is necessary to enable updating the interface
-        # of a given image via the API
-        if civ.interface.is_image_kind and civ.image:
-            if instance.values.filter(image=civ.image).exists():
-                for civ_pk in instance.values.filter(
-                    image=civ.image
-                ).values_list("pk", flat=True):
-                    civ_pks_to_remove.append(civ_pk)
-
-    with transaction.atomic():
-        instance.values.remove(*civ_pks_to_remove)
-        instance.values.add(*civ_pks_to_add)
-
-
-@shared_task(**settings.CELERY_TASK_DECORATOR_KWARGS["acks-late-micro-short"])
-def start_archive_item_update_tasks(
-    archive_item_pk, civ_pks_to_add, upload_pks
-):
-    tasks = update_archive_item_values.signature(
-        kwargs={
-            "archive_item_pk": archive_item_pk,
-            "civ_pks_to_add": civ_pks_to_add,
-        },
-        immutable=True,
-    )
-
-    if len(upload_pks) > 0:
-        image_tasks = group(
-            # Chords and iterator groups are broken in Celery, send a list
-            # instead, see https://github.com/celery/celery/issues/7285
-            [
-                chain(
-                    build_images.signature(
-                        kwargs={"upload_session_pk": upload_pk}
-                    ),
-                    add_image_to_component_interface_value.signature(
-                        kwargs={
-                            "component_interface_value_pk": civ_pk,
-                            "upload_session_pk": upload_pk,
-                        },
-                        immutable=True,
-                    ),
-                )
-                for civ_pk, upload_pk in upload_pks.items()
-            ]
-        )
-        tasks = group(image_tasks, tasks)
-
-    with transaction.atomic():
-        on_commit(tasks.apply_async)

app/grandchallenge/archives/views.py

@@ -2,7 +2,6 @@
 from django.contrib.auth.mixins import PermissionRequiredMixin
 from django.contrib.messages.views import SuccessMessageMixin
 from django.core.exceptions import PermissionDenied, ValidationError
-from django.db.transaction import on_commit
 from django.forms.utils import ErrorList
 from django.http import Http404, HttpResponseRedirect
 from django.shortcuts import get_object_or_404
@@ -48,13 +47,8 @@
     ArchiveItemSerializer,
     ArchiveSerializer,
 )
-from grandchallenge.archives.tasks import (
-    add_images_to_archive,
-    start_archive_item_update_tasks,
-    update_archive_item_update_kwargs,
-)
+from grandchallenge.archives.tasks import add_images_to_archive
 from grandchallenge.cases.models import Image, RawImageUploadSession
-from grandchallenge.cases.widgets import WidgetChoices
 from grandchallenge.components.models import ComponentInterface
 from grandchallenge.core.filters import FilterMixin
 from grandchallenge.core.forms import UserFormKwargsMixin
@@ -394,6 +388,11 @@ def interface(self):
             ComponentInterface, slug=self.kwargs["interface_slug"]
         )
 
+    def get_success_url(self):
+        return reverse(
+            "archives:items-list", kwargs={"slug": self.archive.slug}
+        )
+
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
         kwargs.update(
@@ -407,71 +406,17 @@ def get_context_data(self, *args, **kwargs):
         return context
 
     def form_valid(self, form):
-        def create_upload(image_files):
-            if not image_files:
-                return
-            upload_session = RawImageUploadSession.objects.create(
-                creator=self.request.user
+        for ci_slug, value in form.cleaned_data.items():
+            self.archive_item.create_civ(
+                ci_slug=ci_slug, new_value=value, user=self.request.user
             )
-            upload_session.user_uploads.set(image_files)
-            return upload_session
-
-        upload_pks = {}
-        civ_pks_to_add = set()
-
-        for slug, value in form.cleaned_data.items():
-            if value is None:
-                continue
-
-            ci = ComponentInterface.objects.get(slug=slug)
-
-            if ci.is_image_kind:
-                widget = form.data[f"WidgetChoice-{ci.slug}"]
-                if widget == WidgetChoices.IMAGE_SEARCH.name:
-                    image = value
-                    upload_session = None
-                elif widget == WidgetChoices.IMAGE_UPLOAD.name:
-                    upload_session = create_upload(value)
-                    image = None
-                else:
-                    raise RuntimeError(
-                        f"{widget} is not a valid widget choice."
-                    )
-            else:
-                upload_session = None
-                image = None
-
-            update_archive_item_update_kwargs(
-                instance=self.archive_item,
-                interface=ci,
-                value=value
-                if ci.is_json_kind and not ci.requires_file
-                else None,
-                user_upload=value if ci.requires_file else None,
-                upload_session=upload_session,
-                civ_pks_to_add=civ_pks_to_add,
-                upload_pks=upload_pks,
-                image=image,
-            )
-
-        on_commit(
-            start_archive_item_update_tasks.signature(
-                kwargs={
-                    "archive_item_pk": self.archive_item.pk,
-                    "civ_pks_to_add": list(civ_pks_to_add),
-                    "upload_pks": upload_pks,
-                }
-            ).apply_async
-        )
         messages.add_message(
             self.request,
             messages.SUCCESS,
             "Archive item will be updated. It may take some time for your "
             "changes to become visible.",
         )
-        return HttpResponseRedirect(
-            reverse("archives:items-list", kwargs={"slug": self.archive.slug})
-        )
+        return HttpResponseRedirect(self.get_success_url())
 
 
 class ArchiveItemsList(

app/grandchallenge/cases/serializers.py

@@ -14,13 +14,11 @@
 )
 from grandchallenge.cases.models import Image, ImageFile, RawImageUploadSession
 from grandchallenge.components.models import ComponentInterface
+from grandchallenge.components.tasks import add_image_to_object
 from grandchallenge.core.guardian import filter_by_permission
 from grandchallenge.modalities.serializers import ImagingModalitySerializer
 from grandchallenge.reader_studies.models import Answer, DisplaySet
-from grandchallenge.reader_studies.tasks import (
-    add_image_to_answer,
-    add_image_to_display_set,
-)
+from grandchallenge.reader_studies.tasks import add_image_to_answer
 from grandchallenge.uploads.models import UserUpload
 
 
@@ -278,9 +276,11 @@ def _get_linked_task(*, targets, interface):
             immutable=True,
         )
     elif "display_set" in targets:
-        return add_image_to_display_set.signature(
+        return add_image_to_object.signature(
             kwargs={
-                "display_set_pk": targets["display_set"].pk,
+                "app_label": targets["display_set"]._meta.app_label,
+                "model_name": targets["display_set"]._meta.model_name,
+                "object_pk": targets["display_set"].pk,
                 "interface_pk": interface.pk,
             },
             immutable=True,