diff --git a/app/grandchallenge/cases/models.py b/app/grandchallenge/cases/models.py index ef8214e23..6b6f31f9f 100644 --- a/app/grandchallenge/cases/models.py +++ b/app/grandchallenge/cases/models.py @@ -433,6 +433,10 @@ class Image(UUIDModel): def __str__(self): return f"Image {self.name} {self.shape_without_color}" + @property + def title(self): + return self.name + @property def shape_without_color(self) -> list[int]: """ diff --git a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html index 1456b847c..f63a8e8b9 100644 --- a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html +++ b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html @@ -5,11 +5,11 @@ id="widgetSelect-{{ widget.name }}" hx-get="{% url 'cases:select-image-widget' %}" hx-target="#area-{{ widget.name }}" - hx-trigger="widgetSelected, change" + hx-trigger="widgetSelected, change, load" hx-include="[id='values-{{ widget.name }}']" > {% if widget.attrs.current_value %} - + {% else %} {% endif %} @@ -20,7 +20,7 @@
- +
diff --git a/app/grandchallenge/cases/views.py b/app/grandchallenge/cases/views.py index 26878d30b..b47b6cde2 100644 --- a/app/grandchallenge/cases/views.py +++ b/app/grandchallenge/cases/views.py @@ -3,6 +3,7 @@ from django.conf import settings from django.db.models import Q +from django.forms import HiddenInput from django.http import HttpResponse from django.template.loader import render_to_string from django.template.response import TemplateResponse @@ -37,6 +38,7 @@ from grandchallenge.core.renderers import PaginatedCSVRenderer from grandchallenge.datatables.views import Column, PaginatedTableListView from grandchallenge.subdomains.utils import reverse_lazy +from grandchallenge.uploads.models import UserUpload from grandchallenge.uploads.widgets import UserUploadMultipleWidget from grandchallenge.workstations.models import Workstation @@ -142,11 +144,30 @@ def get(self, request, *args, **kwargs): }, ) return HttpResponse(html_content) - elif current_value and Image.objects.filter(pk=current_value).exists(): - # this can happen on the display set update view, where one of the options - # is the current image, this enables switching back from one of the - # above widgets to the chosen image - return HttpResponse() + elif ( + widget_name == WidgetChoices.IMAGE_SELECTED.name + and current_value + and ( + Image.objects.filter(pk=current_value).exists() + or UserUpload.objects.filter(pk=current_value).exists() + ) + ): + # this can happen on the display set update view or redisplay of + # form upon validation, where one of the options is the current + # image, this enables switching back from one of the above widgets + # to the chosen image. This make sure the form element with the + # right name is available on resubmission. + html_content = render_to_string( + HiddenInput.template_name, + { + "widget": { + "name": interface, + "value": current_value, + "type": "hidden", + }, + }, + ) + return HttpResponse(html_content) elif widget_name == WidgetChoices.UNDEFINED.name: # this happens when switching back from one of the # above widgets to the "Choose data source" option diff --git a/app/grandchallenge/cases/widgets.py b/app/grandchallenge/cases/widgets.py index c2097df44..fbc5a31af 100644 --- a/app/grandchallenge/cases/widgets.py +++ b/app/grandchallenge/cases/widgets.py @@ -16,6 +16,7 @@ class WidgetChoices(TextChoices): IMAGE_SEARCH = "IMAGE_SEARCH" IMAGE_UPLOAD = "IMAGE_UPLOAD" + IMAGE_SELECTED = "IMAGE_SELECTED" UNDEFINED = "UNDEFINED" diff --git a/app/grandchallenge/components/form_fields.py b/app/grandchallenge/components/form_fields.py index 96081250a..963fc95f8 100644 --- a/app/grandchallenge/components/form_fields.py +++ b/app/grandchallenge/components/form_fields.py @@ -1,7 +1,10 @@ +from uuid import UUID + from django import forms from django.forms import ModelChoiceField from django.utils.functional import cached_property +from grandchallenge.cases.models import Image from grandchallenge.cases.widgets import ( FlexibleImageField, FlexibleImageWidget, @@ -102,10 +105,27 @@ def get_initial_value(self): return self.initial def get_image_field(self): + current_value = None + + if isinstance(self.initial, ComponentInterfaceValue): + current_value = self.initial.image + elif self.initial: + try: + uuid = UUID(self.initial, version=4) + except ValueError: + uuid = None + + if uuid and Image.objects.filter(pk=uuid).exists(): + current_value = Image.objects.get(pk=uuid) + elif uuid and UserUpload.objects.filter(pk=uuid).exists(): + current_value = UserUpload.objects.get(pk=uuid) + else: + raise RuntimeError(f"Unknown image pk: {self.initial}") + self.kwargs["widget"] = FlexibleImageWidget( help_text=self.help_text, user=self.user, - current_value=self.initial, + current_value=current_value, # also passing the CIV as current value here so that we can # show the image name to the user rather than its pk ) diff --git a/app/grandchallenge/components/forms.py b/app/grandchallenge/components/forms.py index 0e7da6e44..3e03dcef5 100644 --- a/app/grandchallenge/components/forms.py +++ b/app/grandchallenge/components/forms.py @@ -125,14 +125,20 @@ def __init__(self, *args, instance, base_obj, user, **kwargs): for slug in base_obj.values_for_interfaces.keys(): current_value = None - if instance: + interface = ComponentInterface.objects.filter(slug=slug).get() + prefixed_interface_slug = f"{INTERFACE_FORM_FIELD_PREFIX}{slug}" + + if prefixed_interface_slug in self.data: + if interface.kind == ComponentInterface.Kind.ANY: + current_value = self.data.getlist(prefixed_interface_slug) + else: + current_value = self.data[prefixed_interface_slug] + + if not current_value and instance: current_value = instance.values.filter( interface__slug=slug ).first() - interface = ComponentInterface.objects.filter(slug=slug).get() - prefixed_interface_slug = f"{INTERFACE_FORM_FIELD_PREFIX}{slug}" - if ( interface.requires_file and prefixed_interface_slug in self.data.keys() @@ -166,9 +172,16 @@ def __init__(self, *args, instance, base_obj, user, **kwargs): interface = ComponentInterface.objects.filter( slug=interface_slug ).get() + + current_value = None + if interface.kind == ComponentInterface.Kind.ANY: + current_value = self.data.getlist(slug) + else: + current_value = self.data[slug] + self.fields[slug] = InterfaceFormField( instance=interface, - initial=None, + initial=current_value, required=False, user=self.user, form_data=self.data, diff --git a/app/grandchallenge/uploads/models.py b/app/grandchallenge/uploads/models.py index 23f6ab0e6..dfbbc60b0 100644 --- a/app/grandchallenge/uploads/models.py +++ b/app/grandchallenge/uploads/models.py @@ -79,6 +79,10 @@ def save(self, *args, **kwargs): if adding: self.assign_permissions() + @property + def title(self): + return self.filename + @property def _client(self): return _UPLOADS_CLIENT diff --git a/app/tests/cases_tests/test_widget.py b/app/tests/cases_tests/test_widget.py index f937afafc..ad123b609 100644 --- a/app/tests/cases_tests/test_widget.py +++ b/app/tests/cases_tests/test_widget.py @@ -1,5 +1,6 @@ import pytest from django.core.exceptions import ValidationError +from django.utils.html import format_html from guardian.shortcuts import assign_perm from grandchallenge.cases.widgets import FlexibleImageField, WidgetChoices @@ -148,6 +149,22 @@ def test_flexible_image_widget(client): ) assert response3.content == b"" + image = ImageFactory() + assign_perm("cases.view_image", user, image) + response4 = get_view_for_user( + viewname="cases:select-image-widget", + client=client, + user=user, + data={ + f"WidgetChoice-{ci.slug}": WidgetChoices.IMAGE_SELECTED.name, + "interface_slug": ci.slug, + "current_value": image.pk, + }, + ) + assert format_html( + '', ci.slug, image.pk + ) in str(response4.content) + @pytest.mark.django_db def test_flexible_image_widget_prepopulated_value(): @@ -156,5 +173,5 @@ def test_flexible_image_widget_prepopulated_value(): ci = ComponentInterfaceFactory(kind=ComponentInterface.Kind.IMAGE) civ = ComponentInterfaceValueFactory(interface=ci, image=im) field = InterfaceFormField(instance=ci, user=user, initial=civ) - assert field.field.widget.attrs["current_value"] == civ + assert field.field.widget.attrs["current_value"] == civ.image assert field.field.initial == civ.image.pk diff --git a/app/tests/components_tests/test_views.py b/app/tests/components_tests/test_views.py index f406df30b..63f2bbdf4 100644 --- a/app/tests/components_tests/test_views.py +++ b/app/tests/components_tests/test_views.py @@ -4,7 +4,10 @@ from django.conf import settings from grandchallenge.archives.models import ArchiveItem -from grandchallenge.components.models import InterfaceKindChoices +from grandchallenge.components.models import ( + ComponentInterface, + InterfaceKindChoices, +) from grandchallenge.reader_studies.models import DisplaySet, ReaderStudy from grandchallenge.subdomains.utils import reverse from tests.algorithms_tests.factories import AlgorithmFactory @@ -14,7 +17,8 @@ ComponentInterfaceFactory, ComponentInterfaceValueFactory, ) -from tests.factories import UserFactory +from tests.conftest import get_interface_form_data +from tests.factories import ImageFactory, UserFactory from tests.reader_studies_tests.factories import ( DisplaySetFactory, ReaderStudyFactory, @@ -539,3 +543,74 @@ def test_interfaces_list_link_in_new_interface_form( user=u, ) assert reverse(interface_list_viewname) in response.rendered_content + + +@pytest.mark.parametrize( + "base_object_factory,base_obj_lookup,object_factory,viewname", + ( + ( + ReaderStudyFactory, + "reader_study", + DisplaySetFactory, + "reader-studies:display-set-update", + ), + ( + ArchiveFactory, + "archive", + ArchiveItemFactory, + "archives:item-edit", + ), + ), +) +@pytest.mark.django_db +def test_image_widget_populated_value_on_update_view_validation_error( + client, base_object_factory, base_obj_lookup, object_factory, viewname +): + image1 = ImageFactory() + image_ci = ComponentInterfaceFactory(kind=ComponentInterface.Kind.IMAGE) + image1_civ = ComponentInterfaceValueFactory( + interface=image_ci, image=image1 + ) + + annotation = "{}" + annotation_ci = ComponentInterfaceFactory( + kind=ComponentInterface.Kind.TWO_D_BOUNDING_BOX + ) + annotation_civ = ComponentInterfaceValueFactory( + interface=annotation_ci, value=annotation + ) + + editor = UserFactory() + base_obj = base_object_factory() + base_obj.add_editor(editor) + + object = object_factory(**{base_obj_lookup: base_obj}) + object.values.set([image1_civ, annotation_civ]) + + image2 = ImageFactory() + data = { + "interface_slug": image_ci.slug, + "current_value": image2.pk, + } + data.update( + **get_interface_form_data(interface_slug=image_ci.slug, data=image2.pk) + ) + data.update( + **get_interface_form_data( + interface_slug=annotation_ci.slug, data='{"1":1}' + ) + ) + + response = get_view_for_user( + client=client, + viewname=viewname, + reverse_kwargs={"slug": base_obj.slug, "pk": object.pk}, + user=editor, + method=client.post, + follow=True, + data=data, + ) + assert response.status_code == 200 + assert f'' in str( + response.rendered_content + )