Avoid error when creating new item with empty value for a linked socket (#4373)

Close #4371

Creating new items (e.g. display sets or archive items) with an
undefined value for one or more sockets would raise an error since
#4333. This is a common use case and is allowed.

Also add `__repr__` to `CIVData` for easier debugging.

Author: koopmant

app/grandchallenge/components/models.py

@@ -2262,6 +2262,12 @@ def file_civ(self):
     def dicom_upload_with_name(self):
         return self._dicom_upload_with_name
 
+    def __repr__(self):
+        return f"CIVData(interface_slug={self._interface_slug!r}, value={self._initial_value!r})"
+
+    def __str__(self):
+        return f"CIVData: {self.__dict__}"
+
     def __init__(self, *, interface_slug, value):
         self._interface_slug = interface_slug
         self._initial_value = value
@@ -2412,13 +2418,13 @@ def validate_civ_data_objects_and_execute_linked_task(
         self, *, civ_data_objects, user, linked_task=None
     ):
         for civ_data in civ_data_objects:
-            self.create_civ(
+            self._update_civ(
                 civ_data=civ_data,
                 user=user,
                 linked_task=linked_task,
             )
 
-    def create_civ(self, *, civ_data, user=None, linked_task=None):
+    def _update_civ(self, *, civ_data, user=None, linked_task=None):
         if not self.is_editable:
             raise CIVNotEditableException(
                 f"{self} is not editable. CIVs cannot be added or removed from it.",
@@ -2442,15 +2448,15 @@ def create_civ(self, *, civ_data, user=None, linked_task=None):
         )
 
         if ci.super_kind == ci.SuperKind.VALUE:
-            return self._create_civ_for_value(
+            return self._update_civ_for_value(
                 ci=ci,
                 current_civ=current_civ,
                 new_value=civ_data.value,
                 user=user,
                 linked_task=linked_task,
             )
         elif ci.super_kind == ci.SuperKind.IMAGE:
-            return self._create_civ_for_image(
+            return self._update_civ_for_image(
                 ci=ci,
                 current_civ=current_civ,
                 user=user,
@@ -2461,7 +2467,7 @@ def create_civ(self, *, civ_data, user=None, linked_task=None):
                 linked_task=linked_task,
             )
         elif ci.super_kind == ci.SuperKind.FILE:
-            return self._create_civ_for_file(
+            return self._update_civ_for_file(
                 ci=ci,
                 current_civ=current_civ,
                 file_civ=civ_data.file_civ,
@@ -2473,7 +2479,7 @@ def create_civ(self, *, civ_data, user=None, linked_task=None):
                 f"Unknown interface super kind: {ci.super_kind}"
             )
 
-    def _create_civ_for_value(
+    def _update_civ_for_value(
         self, *, ci, current_civ, new_value, user, linked_task=None
     ):
         current_value = current_civ.value if current_civ else None
@@ -2509,7 +2515,7 @@ def _create_civ_for_value(
             if linked_task is not None:
                 on_commit(signature(linked_task).apply_async)
 
-    def _create_civ_for_image(  # noqa: C901
+    def _update_civ_for_image(  # noqa: C901
         self,
         *,
         ci,
@@ -2633,10 +2639,13 @@ def _create_civ_for_image(  # noqa: C901
                     kwargs={"dicom_imageset_upload_pk": upload.pk}
                 ).apply_async
             )
+        elif current_civ is None:
+            # Nothing to do
+            pass
         else:
             raise NotImplementedError
 
-    def _create_civ_for_file(
+    def _update_civ_for_file(
         self,
         *,
         ci,

app/tests/archives_tests/test_views.py

@@ -4,12 +4,14 @@
 
 import pytest
 from django.contrib.auth.models import Group
+from django.core.exceptions import ObjectDoesNotExist
 from django.core.files.base import ContentFile
 from guardian.shortcuts import assign_perm, remove_perm
 from requests import put
 
 from grandchallenge.archives.models import ArchiveItem
 from grandchallenge.archives.views import ArchiveItemsList
+from grandchallenge.cases.widgets import ImageWidgetChoices
 from grandchallenge.components.form_fields import INTERFACE_FORM_FIELD_PREFIX
 from grandchallenge.components.models import (
     ComponentInterface,
@@ -1251,6 +1253,49 @@ def test_archive_item_create_view(
     assert not UserUpload.objects.filter(pk=upload.pk).exists()
 
 
+@pytest.mark.django_db
+def test_archive_item_create_view_with_empty_value(
+    client, settings, django_capture_on_commit_callbacks
+):
+    settings.task_eager_propagates = (True,)
+    settings.task_always_eager = (True,)
+
+    archive = ArchiveFactory()
+    editor = UserFactory()
+    archive.add_editor(editor)
+    existing_archive_item = ArchiveItemFactory(archive=archive)
+    ci_str = ComponentInterfaceFactory(kind=InterfaceKindChoices.STRING)
+    ci_img = ComponentInterfaceFactory(kind=InterfaceKindChoices.PANIMG_IMAGE)
+    civ_str = ci_str.create_instance(value="foo")
+    civ_img = ci_img.create_instance(image=ImageFactory())
+    existing_archive_item.values.set([civ_str, civ_img])
+
+    with django_capture_on_commit_callbacks(execute=True):
+        response = get_view_for_user(
+            viewname="archives:item-create",
+            client=client,
+            reverse_kwargs={"slug": archive.slug},
+            data={
+                **get_interface_form_data(
+                    interface_slug=ci_str.slug, data="bar"
+                ),
+                f"widget-choice-{INTERFACE_FORM_FIELD_PREFIX}{ci_img.slug}": ImageWidgetChoices.UNDEFINED.name,
+                "title": "archive-item title",
+            },
+            user=editor,
+            method=client.post,
+        )
+
+    assert response.status_code == 302
+    assert ArchiveItem.objects.count() == 2
+    new_archive_item = ArchiveItem.objects.order_by("created").last()
+    assert new_archive_item.title == "archive-item title"
+    assert new_archive_item.values.count() == 1
+    assert new_archive_item.values.get(interface=ci_str).value == "bar"
+    with pytest.raises(ObjectDoesNotExist):
+        new_archive_item.values.get(interface=ci_img)
+
+
 @pytest.mark.django_db
 def test_archive_item_bulk_delete_permissions(client):
     user, editor = UserFactory.create_batch(2)

app/tests/reader_studies_tests/test_views.py

@@ -1,6 +1,7 @@
 import io
 
 import pytest
+from django.core.exceptions import ObjectDoesNotExist
 from django.forms import JSONField
 from django.test import override_settings
 from guardian.shortcuts import assign_perm
@@ -722,6 +723,50 @@ def test_add_display_set_to_reader_study(
     assert ds.values.get(interface=ci_json).file.read() == b'{"foo": "bar"}'
 
 
+@pytest.mark.django_db
+def test_add_display_set_to_reader_study_with_empty_value(
+    client, settings, django_capture_on_commit_callbacks
+):
+    settings.task_eager_propagates = (True,)
+    settings.task_always_eager = (True,)
+
+    editor = UserFactory()
+    reader_study = ReaderStudyFactory()
+    existing_display_set = DisplaySetFactory(reader_study=reader_study)
+    reader_study.add_editor(editor)
+    ci_str = ComponentInterfaceFactory(kind=InterfaceKindChoices.STRING)
+    ci_img = ComponentInterfaceFactory(kind=InterfaceKindChoices.PANIMG_IMAGE)
+    civ_str = ci_str.create_instance(value="foo")
+    civ_img = ci_img.create_instance(image=ImageFactory())
+    existing_display_set.values.set([civ_str, civ_img])
+
+    assert DisplaySet.objects.count() == 1
+
+    with django_capture_on_commit_callbacks(execute=True):
+        response = get_view_for_user(
+            viewname="reader-studies:display-set-create",
+            client=client,
+            reverse_kwargs={"slug": reader_study.slug},
+            data={
+                **get_interface_form_data(
+                    interface_slug=ci_str.slug, data="bar"
+                ),
+                f"widget-choice-{INTERFACE_FORM_FIELD_PREFIX}{ci_img.slug}": ImageWidgetChoices.UNDEFINED.name,
+                "order": 11,
+            },
+            user=editor,
+            method=client.post,
+        )
+
+    assert response.status_code == 302
+    assert DisplaySet.objects.count() == 2
+    new_display_set = DisplaySet.objects.order_by("created").last()
+    assert new_display_set.values.count() == 1
+    assert new_display_set.values.get(interface=ci_str).value == "bar"
+    with pytest.raises(ObjectDoesNotExist):
+        new_display_set.values.get(interface=ci_img)
+
+
 @pytest.mark.django_db
 def test_add_display_set_update_when_disabled(client):
     editor = UserFactory()