From d85dadbd0998db8905f7de2a99e366ab3c79dccb Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Thu, 31 Oct 2024 15:59:40 +0000 Subject: [PATCH 01/21] Bind CIV value on MultipleCIVForm redisplay --- app/grandchallenge/components/forms.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/app/grandchallenge/components/forms.py b/app/grandchallenge/components/forms.py index 0f4493e2d..99324322f 100644 --- a/app/grandchallenge/components/forms.py +++ b/app/grandchallenge/components/forms.py @@ -116,14 +116,20 @@ def __init__(self, *args, instance, base_obj, user, **kwargs): for slug in base_obj.values_for_interfaces.keys(): current_value = None - if 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 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] + else: + if instance: + current_value = instance.values.filter( + interface__slug=slug + ).first() + if ( interface.requires_file and prefixed_interface_slug in self.data.keys() From a2f1f673f9a1344d1a14251f5b4a8dce9de14453 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:03:36 +0000 Subject: [PATCH 02/21] Fix handling of image CIV value on form redisplay --- .../cases/flexible_image_widget.html | 11 +++++- app/grandchallenge/cases/widgets.py | 4 ++ app/grandchallenge/components/form_fields.py | 37 ++++++++++++++++++- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html index 1456b847c..dfb9856de 100644 --- a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html +++ b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html @@ -8,7 +8,9 @@ hx-trigger="widgetSelected, change" hx-include="[id='values-{{ widget.name }}']" > - {% if widget.attrs.current_value %} + {% if widget.attrs.submitted_value %} + + {% elif widget.attrs.current_value %} {% else %} @@ -23,6 +25,11 @@ -
+
+ {% if widget.attrs.submitted_value %} + + + {% endif %} +
diff --git a/app/grandchallenge/cases/widgets.py b/app/grandchallenge/cases/widgets.py index c2097df44..e679e5242 100644 --- a/app/grandchallenge/cases/widgets.py +++ b/app/grandchallenge/cases/widgets.py @@ -45,6 +45,8 @@ def __init__( help_text=None, user=None, current_value=None, + submitted_value=None, + submitted_widget_choice=None, disabled=False, **kwargs, ): @@ -58,6 +60,8 @@ def __init__( "disabled": disabled, "user": user, "current_value": current_value, + "submitted_value": submitted_value, + "submitted_widget_choice": submitted_widget_choice, "widget_choices": { choice.name: choice.value for choice in WidgetChoices }, diff --git a/app/grandchallenge/components/form_fields.py b/app/grandchallenge/components/form_fields.py index 96081250a..ca72865b1 100644 --- a/app/grandchallenge/components/form_fields.py +++ b/app/grandchallenge/components/form_fields.py @@ -2,9 +2,11 @@ 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, + WidgetChoices, ) from grandchallenge.components.models import ComponentInterfaceValue from grandchallenge.components.schemas import INTERFACE_VALUE_SCHEMA @@ -102,10 +104,43 @@ def get_initial_value(self): return self.initial def get_image_field(self): + + current_value = None + submitted_value = None + submitted_widget_choice = None + + if self.initial is not None and not isinstance( + self.initial, ComponentInterfaceValue + ): + + widget_choice_key = f"WidgetChoice-{INTERFACE_FORM_FIELD_PREFIX}{self.instance.slug}" + + if ( + widget_choice_key in self.form_data + and self.form_data[widget_choice_key] + not in WidgetChoices.names + ): + widget_choice_key = f"SubmittedWidgetChoice-{INTERFACE_FORM_FIELD_PREFIX}{self.instance.slug}" + + if widget_choice_key in self.form_data: + submitted_widget_choice = self.form_data[widget_choice_key] + if self.form_data[widget_choice_key] == "IMAGE_SEARCH": + submitted_value = Image.objects.filter( + pk=self.initial + ).first() + elif self.form_data[widget_choice_key] == "IMAGE_UPLOAD": + submitted_value = UserUpload.objects.filter( + pk=self.initial + ).first() + else: + current_value = self.initial + self.kwargs["widget"] = FlexibleImageWidget( help_text=self.help_text, user=self.user, - current_value=self.initial, + current_value=current_value, + submitted_value=submitted_value, + submitted_widget_choice=submitted_widget_choice, # also passing the CIV as current value here so that we can # show the image name to the user rather than its pk ) From 44c478db6455aeba4ba68e2a6fd119a9475d70d0 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:12:45 +0000 Subject: [PATCH 03/21] Rename variables --- app/grandchallenge/cases/widgets.py | 8 ++++---- app/grandchallenge/components/form_fields.py | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/grandchallenge/cases/widgets.py b/app/grandchallenge/cases/widgets.py index e679e5242..d453712fd 100644 --- a/app/grandchallenge/cases/widgets.py +++ b/app/grandchallenge/cases/widgets.py @@ -45,8 +45,8 @@ def __init__( help_text=None, user=None, current_value=None, - submitted_value=None, - submitted_widget_choice=None, + submit_value=None, + submit_widget_choice=None, disabled=False, **kwargs, ): @@ -60,8 +60,8 @@ def __init__( "disabled": disabled, "user": user, "current_value": current_value, - "submitted_value": submitted_value, - "submitted_widget_choice": submitted_widget_choice, + "submit_value": submit_value, + "submit_widget_choice": submit_widget_choice, "widget_choices": { choice.name: choice.value for choice in WidgetChoices }, diff --git a/app/grandchallenge/components/form_fields.py b/app/grandchallenge/components/form_fields.py index ca72865b1..ad9e87896 100644 --- a/app/grandchallenge/components/form_fields.py +++ b/app/grandchallenge/components/form_fields.py @@ -106,8 +106,8 @@ def get_initial_value(self): def get_image_field(self): current_value = None - submitted_value = None - submitted_widget_choice = None + submit_value = None + submit_widget_choice = None if self.initial is not None and not isinstance( self.initial, ComponentInterfaceValue @@ -120,16 +120,16 @@ def get_image_field(self): and self.form_data[widget_choice_key] not in WidgetChoices.names ): - widget_choice_key = f"SubmittedWidgetChoice-{INTERFACE_FORM_FIELD_PREFIX}{self.instance.slug}" + widget_choice_key = f"SubmitWidgetChoice-{INTERFACE_FORM_FIELD_PREFIX}{self.instance.slug}" if widget_choice_key in self.form_data: - submitted_widget_choice = self.form_data[widget_choice_key] + submit_widget_choice = self.form_data[widget_choice_key] if self.form_data[widget_choice_key] == "IMAGE_SEARCH": - submitted_value = Image.objects.filter( + submit_value = Image.objects.filter( pk=self.initial ).first() elif self.form_data[widget_choice_key] == "IMAGE_UPLOAD": - submitted_value = UserUpload.objects.filter( + submit_value = UserUpload.objects.filter( pk=self.initial ).first() else: @@ -139,8 +139,8 @@ def get_image_field(self): help_text=self.help_text, user=self.user, current_value=current_value, - submitted_value=submitted_value, - submitted_widget_choice=submitted_widget_choice, + submit_value=submit_value, + submit_widget_choice=submit_widget_choice, # also passing the CIV as current value here so that we can # show the image name to the user rather than its pk ) From 8c4c69e2a0e41fcce36f65c2ffdeac5b3b81196e Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:15:08 +0000 Subject: [PATCH 04/21] Rename variables in image widget template --- .../cases/templates/cases/flexible_image_widget.html | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html index dfb9856de..b2efbe1c0 100644 --- a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html +++ b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html @@ -8,8 +8,8 @@ hx-trigger="widgetSelected, change" hx-include="[id='values-{{ widget.name }}']" > - {% if widget.attrs.submitted_value %} - + {% if widget.attrs.submit_value %} + {% elif widget.attrs.current_value %} {% else %} @@ -26,9 +26,9 @@
- {% if widget.attrs.submitted_value %} - - + {% if widget.attrs.submit_value %} + + {% endif %}
From c5db5e519182a5cae2afdc99d04b63617c6d0717 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:42:03 +0000 Subject: [PATCH 05/21] clean condition --- app/grandchallenge/components/form_fields.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/grandchallenge/components/form_fields.py b/app/grandchallenge/components/form_fields.py index ad9e87896..32015c797 100644 --- a/app/grandchallenge/components/form_fields.py +++ b/app/grandchallenge/components/form_fields.py @@ -109,9 +109,7 @@ def get_image_field(self): submit_value = None submit_widget_choice = None - if self.initial is not None and not isinstance( - self.initial, ComponentInterfaceValue - ): + if not isinstance(self.initial, ComponentInterfaceValue): widget_choice_key = f"WidgetChoice-{INTERFACE_FORM_FIELD_PREFIX}{self.instance.slug}" From 3de2e16ab39e7b189659d33160404bc51b46495e Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Sat, 2 Nov 2024 11:14:40 +0000 Subject: [PATCH 06/21] Add title to Image and UserUpload to unify access object name --- app/grandchallenge/cases/models.py | 4 ++++ app/grandchallenge/uploads/models.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/app/grandchallenge/cases/models.py b/app/grandchallenge/cases/models.py index d391108b5..37ca8a7a7 100644 --- a/app/grandchallenge/cases/models.py +++ b/app/grandchallenge/cases/models.py @@ -390,6 +390,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/uploads/models.py b/app/grandchallenge/uploads/models.py index 04e88e808..2066f6d78 100644 --- a/app/grandchallenge/uploads/models.py +++ b/app/grandchallenge/uploads/models.py @@ -93,6 +93,10 @@ def handle_file_validation_failure(self, *, error_message, linked_object): status=linked_object.CANCELLED, error_message=error_message ) + @property + def title(self): + return self.filename + @property def _client(self): return _UPLOADS_CLIENT From a501081f56c146fafd11771bd5fa47271a4e3876 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Sat, 2 Nov 2024 11:41:17 +0000 Subject: [PATCH 07/21] Add hidden input field with current value on form redisplay submission --- app/grandchallenge/cases/views.py | 33 ++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/app/grandchallenge/cases/views.py b/app/grandchallenge/cases/views.py index 26878d30b..7fb8ae2f1 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,32 @@ 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 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. In the case of form redisplay on validation + # error, current_value will be set in the flexible image widget + # select dropdown element with name starting with "WidgetChoice-" + # which will not be captured on resubmission, because the + # current_value is looked up from the form data element with + # interface name (in JobCreateForm and MultipleCIVForm). 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 From 745564fe0e9a89e65d2b0e5d1b5996cc42c2486b Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Sat, 2 Nov 2024 11:42:41 +0000 Subject: [PATCH 08/21] Clean the implementation --- .../cases/flexible_image_widget.html | 17 +++------ app/grandchallenge/cases/widgets.py | 4 -- app/grandchallenge/components/form_fields.py | 37 ++++--------------- 3 files changed, 12 insertions(+), 46 deletions(-) diff --git a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html index b2efbe1c0..b3dc640f5 100644 --- a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html +++ b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html @@ -5,13 +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.submit_value %} - - {% elif widget.attrs.current_value %} - + {% if widget.attrs.current_value %} + {% else %} {% endif %} @@ -22,14 +20,9 @@
- +
-
- {% if widget.attrs.submit_value %} - - - {% endif %} -
+
diff --git a/app/grandchallenge/cases/widgets.py b/app/grandchallenge/cases/widgets.py index d453712fd..c2097df44 100644 --- a/app/grandchallenge/cases/widgets.py +++ b/app/grandchallenge/cases/widgets.py @@ -45,8 +45,6 @@ def __init__( help_text=None, user=None, current_value=None, - submit_value=None, - submit_widget_choice=None, disabled=False, **kwargs, ): @@ -60,8 +58,6 @@ def __init__( "disabled": disabled, "user": user, "current_value": current_value, - "submit_value": submit_value, - "submit_widget_choice": submit_widget_choice, "widget_choices": { choice.name: choice.value for choice in WidgetChoices }, diff --git a/app/grandchallenge/components/form_fields.py b/app/grandchallenge/components/form_fields.py index 32015c797..d74ced350 100644 --- a/app/grandchallenge/components/form_fields.py +++ b/app/grandchallenge/components/form_fields.py @@ -6,7 +6,6 @@ from grandchallenge.cases.widgets import ( FlexibleImageField, FlexibleImageWidget, - WidgetChoices, ) from grandchallenge.components.models import ComponentInterfaceValue from grandchallenge.components.schemas import INTERFACE_VALUE_SCHEMA @@ -104,41 +103,19 @@ def get_initial_value(self): return self.initial def get_image_field(self): - current_value = None - submit_value = None - submit_widget_choice = None - - if not isinstance(self.initial, ComponentInterfaceValue): - - widget_choice_key = f"WidgetChoice-{INTERFACE_FORM_FIELD_PREFIX}{self.instance.slug}" - - if ( - widget_choice_key in self.form_data - and self.form_data[widget_choice_key] - not in WidgetChoices.names - ): - widget_choice_key = f"SubmitWidgetChoice-{INTERFACE_FORM_FIELD_PREFIX}{self.instance.slug}" - - if widget_choice_key in self.form_data: - submit_widget_choice = self.form_data[widget_choice_key] - if self.form_data[widget_choice_key] == "IMAGE_SEARCH": - submit_value = Image.objects.filter( - pk=self.initial - ).first() - elif self.form_data[widget_choice_key] == "IMAGE_UPLOAD": - submit_value = UserUpload.objects.filter( - pk=self.initial - ).first() - else: - current_value = self.initial + + if isinstance(self.initial, ComponentInterfaceValue): + current_value = self.initial.image + elif Image.objects.filter(pk=self.initial).exists(): + current_value = Image.objects.filter(pk=self.initial).first() + elif UserUpload.objects.filter(pk=self.initial).exists(): + current_value = UserUpload.objects.filter(pk=self.initial).first() self.kwargs["widget"] = FlexibleImageWidget( help_text=self.help_text, user=self.user, current_value=current_value, - submit_value=submit_value, - submit_widget_choice=submit_widget_choice, # also passing the CIV as current value here so that we can # show the image name to the user rather than its pk ) From 1f134f2d6cd9c615233988c277ce48b30685b7c7 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Sat, 2 Nov 2024 12:02:57 +0000 Subject: [PATCH 09/21] Fix failing test --- app/tests/cases_tests/test_widget.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/tests/cases_tests/test_widget.py b/app/tests/cases_tests/test_widget.py index f937afafc..12ba3d98f 100644 --- a/app/tests/cases_tests/test_widget.py +++ b/app/tests/cases_tests/test_widget.py @@ -156,5 +156,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 From 194a7154c8e022c2e52166dfdc096a77a3911614 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Mon, 4 Nov 2024 12:04:31 +0000 Subject: [PATCH 10/21] Update comment --- app/grandchallenge/cases/views.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/app/grandchallenge/cases/views.py b/app/grandchallenge/cases/views.py index 7fb8ae2f1..69fb5d418 100644 --- a/app/grandchallenge/cases/views.py +++ b/app/grandchallenge/cases/views.py @@ -151,14 +151,8 @@ def get(self, request, *args, **kwargs): # 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. In the case of form redisplay on validation - # error, current_value will be set in the flexible image widget - # select dropdown element with name starting with "WidgetChoice-" - # which will not be captured on resubmission, because the - # current_value is looked up from the form data element with - # interface name (in JobCreateForm and MultipleCIVForm). This make - # sure the form element with the right name is available on - # resubmission. + # 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, { From d821df6224158ba3b093762c4b532bf4f20f7a12 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Mon, 4 Nov 2024 12:14:03 +0000 Subject: [PATCH 11/21] Use get() got Image and UserUpload --- app/grandchallenge/components/form_fields.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/grandchallenge/components/form_fields.py b/app/grandchallenge/components/form_fields.py index d74ced350..6a5323e45 100644 --- a/app/grandchallenge/components/form_fields.py +++ b/app/grandchallenge/components/form_fields.py @@ -108,9 +108,9 @@ def get_image_field(self): if isinstance(self.initial, ComponentInterfaceValue): current_value = self.initial.image elif Image.objects.filter(pk=self.initial).exists(): - current_value = Image.objects.filter(pk=self.initial).first() + current_value = Image.objects.get(pk=self.initial) elif UserUpload.objects.filter(pk=self.initial).exists(): - current_value = UserUpload.objects.filter(pk=self.initial).first() + current_value = UserUpload.objects.get(pk=self.initial) self.kwargs["widget"] = FlexibleImageWidget( help_text=self.help_text, From 20c2731d3d514663dd3299bf008f7e6a9308abed Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Mon, 4 Nov 2024 12:39:45 +0000 Subject: [PATCH 12/21] Raise RuntimeError if image pk is unknown --- app/grandchallenge/components/form_fields.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/grandchallenge/components/form_fields.py b/app/grandchallenge/components/form_fields.py index 6a5323e45..327a0a796 100644 --- a/app/grandchallenge/components/form_fields.py +++ b/app/grandchallenge/components/form_fields.py @@ -111,6 +111,8 @@ def get_image_field(self): current_value = Image.objects.get(pk=self.initial) elif UserUpload.objects.filter(pk=self.initial).exists(): current_value = UserUpload.objects.get(pk=self.initial) + elif self.initial: + raise RuntimeError(f"Unknown image pk: {self.initial}") self.kwargs["widget"] = FlexibleImageWidget( help_text=self.help_text, From 833c47a80a86a529aa15b0f9d6f0981670d6e7fe Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Mon, 4 Nov 2024 14:45:39 +0000 Subject: [PATCH 13/21] Add WidgetChoice for selected image for semantic consistency --- .../cases/templates/cases/flexible_image_widget.html | 2 +- app/grandchallenge/cases/views.py | 10 +++++++--- app/grandchallenge/cases/widgets.py | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html index b3dc640f5..f63a8e8b9 100644 --- a/app/grandchallenge/cases/templates/cases/flexible_image_widget.html +++ b/app/grandchallenge/cases/templates/cases/flexible_image_widget.html @@ -9,7 +9,7 @@ hx-include="[id='values-{{ widget.name }}']" > {% if widget.attrs.current_value %} - + {% else %} {% endif %} diff --git a/app/grandchallenge/cases/views.py b/app/grandchallenge/cases/views.py index 69fb5d418..b47b6cde2 100644 --- a/app/grandchallenge/cases/views.py +++ b/app/grandchallenge/cases/views.py @@ -144,9 +144,13 @@ def get(self, request, *args, **kwargs): }, ) return HttpResponse(html_content) - elif current_value and ( - Image.objects.filter(pk=current_value).exists() - or UserUpload.objects.filter(pk=current_value).exists() + 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 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" From a920391985f4443db3f73cdb2bbd48650ff2ca28 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Mon, 4 Nov 2024 15:59:39 +0000 Subject: [PATCH 14/21] extend image widget test with IMAGE_SELECTED case --- app/tests/cases_tests/test_widget.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/app/tests/cases_tests/test_widget.py b/app/tests/cases_tests/test_widget.py index 12ba3d98f..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(): From 298966c973d65b9849be61302e655f482df7338c Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:00:44 +0000 Subject: [PATCH 15/21] Remember submitted value of dynamically added interfaces --- app/grandchallenge/components/forms.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/grandchallenge/components/forms.py b/app/grandchallenge/components/forms.py index 99324322f..0e653a7c1 100644 --- a/app/grandchallenge/components/forms.py +++ b/app/grandchallenge/components/forms.py @@ -163,9 +163,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, From 8eb0b65297cfc7262731ab5c7c8adead296843a1 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:05:24 +0000 Subject: [PATCH 16/21] Keep the original value when empty value is submitted --- app/grandchallenge/components/forms.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/grandchallenge/components/forms.py b/app/grandchallenge/components/forms.py index 0e653a7c1..2e321c66a 100644 --- a/app/grandchallenge/components/forms.py +++ b/app/grandchallenge/components/forms.py @@ -124,11 +124,11 @@ def __init__(self, *args, instance, base_obj, user, **kwargs): current_value = self.data.getlist(prefixed_interface_slug) else: current_value = self.data[prefixed_interface_slug] - else: - if instance: - current_value = instance.values.filter( - interface__slug=slug - ).first() + + if not current_value and instance: + current_value = instance.values.filter( + interface__slug=slug + ).first() if ( interface.requires_file From 39c739bdc9266a2b4e2a36e1fc49ee1458d35ce2 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:07:06 +0000 Subject: [PATCH 17/21] Make sure a valid uuid is passed along --- app/grandchallenge/components/form_fields.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/app/grandchallenge/components/form_fields.py b/app/grandchallenge/components/form_fields.py index 327a0a796..bda1444e9 100644 --- a/app/grandchallenge/components/form_fields.py +++ b/app/grandchallenge/components/form_fields.py @@ -1,3 +1,5 @@ +from uuid import UUID + from django import forms from django.forms import ModelChoiceField from django.utils.functional import cached_property @@ -107,12 +109,18 @@ def get_image_field(self): if isinstance(self.initial, ComponentInterfaceValue): current_value = self.initial.image - elif Image.objects.filter(pk=self.initial).exists(): - current_value = Image.objects.get(pk=self.initial) - elif UserUpload.objects.filter(pk=self.initial).exists(): - current_value = UserUpload.objects.get(pk=self.initial) elif self.initial: - raise RuntimeError(f"Unknown image pk: {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: {uuid}") self.kwargs["widget"] = FlexibleImageWidget( help_text=self.help_text, From b955ae0ad900d8f188b74d9830b7ca15665438e8 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:11:11 +0000 Subject: [PATCH 18/21] test image widget populated value on update view validation error --- app/tests/components_tests/test_views.py | 79 +++++++++++++++++++++++- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/app/tests/components_tests/test_views.py b/app/tests/components_tests/test_views.py index fa0d76279..80277d876 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 tests.algorithms_tests.factories import AlgorithmFactory from tests.archives_tests.factories import ArchiveFactory, ArchiveItemFactory @@ -13,7 +16,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, @@ -503,3 +507,74 @@ def test_display_ci_example_value(client): assert response.status_code == 200 assert v.value in response.rendered_content assert v.extra_info 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, title="annotation" + ) + annotation_civ = ComponentInterfaceValueFactory( + interface=annotation_ci, value=annotation + ) + + editor = UserFactory() + base_obj = base_object_factory() + base_obj.add_editor(editor) + + ob = object_factory(**{base_obj_lookup: base_obj}) + ob.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": ob.pk}, + user=editor, + method=client.post, + follow=True, + data=data, + ) + assert response.status_code == 200 + assert f'' in str( + response.rendered_content + ) From 2964e30266ffea9cc0b567c675ad1ad307caf4c6 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:38:34 +0000 Subject: [PATCH 19/21] pre-commit fix --- app/tests/components_tests/test_views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/tests/components_tests/test_views.py b/app/tests/components_tests/test_views.py index 54cab8a8e..348c092e6 100644 --- a/app/tests/components_tests/test_views.py +++ b/app/tests/components_tests/test_views.py @@ -544,7 +544,7 @@ def test_interfaces_list_link_in_new_interface_form( ) assert reverse(interface_list_viewname) in response.rendered_content - + @pytest.mark.parametrize( "base_object_factory,base_obj_lookup,object_factory,viewname", ( @@ -613,4 +613,4 @@ def test_image_widget_populated_value_on_update_view_validation_error( assert response.status_code == 200 assert f'' in str( response.rendered_content - ) \ No newline at end of file + ) From 6cad29cae66b2b668d50a5a743387d56be981145 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Tue, 5 Nov 2024 22:41:02 +0000 Subject: [PATCH 20/21] Update view test --- app/tests/components_tests/test_views.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/tests/components_tests/test_views.py b/app/tests/components_tests/test_views.py index 348c092e6..63f2bbdf4 100644 --- a/app/tests/components_tests/test_views.py +++ b/app/tests/components_tests/test_views.py @@ -574,7 +574,7 @@ def test_image_widget_populated_value_on_update_view_validation_error( annotation = "{}" annotation_ci = ComponentInterfaceFactory( - kind=ComponentInterface.Kind.TWO_D_BOUNDING_BOX, title="annotation" + kind=ComponentInterface.Kind.TWO_D_BOUNDING_BOX ) annotation_civ = ComponentInterfaceValueFactory( interface=annotation_ci, value=annotation @@ -584,8 +584,8 @@ def test_image_widget_populated_value_on_update_view_validation_error( base_obj = base_object_factory() base_obj.add_editor(editor) - ob = object_factory(**{base_obj_lookup: base_obj}) - ob.values.set([image1_civ, annotation_civ]) + object = object_factory(**{base_obj_lookup: base_obj}) + object.values.set([image1_civ, annotation_civ]) image2 = ImageFactory() data = { @@ -604,7 +604,7 @@ def test_image_widget_populated_value_on_update_view_validation_error( response = get_view_for_user( client=client, viewname=viewname, - reverse_kwargs={"slug": base_obj.slug, "pk": ob.pk}, + reverse_kwargs={"slug": base_obj.slug, "pk": object.pk}, user=editor, method=client.post, follow=True, From 54d1e872104d647c5ad62836e651875ea857d8c0 Mon Sep 17 00:00:00 2001 From: Ammar Ammar <43293485+ammar257ammar@users.noreply.github.com> Date: Tue, 5 Nov 2024 22:49:45 +0000 Subject: [PATCH 21/21] Update RuntimeError message --- app/grandchallenge/components/form_fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/grandchallenge/components/form_fields.py b/app/grandchallenge/components/form_fields.py index bda1444e9..963fc95f8 100644 --- a/app/grandchallenge/components/form_fields.py +++ b/app/grandchallenge/components/form_fields.py @@ -120,7 +120,7 @@ def get_image_field(self): elif uuid and UserUpload.objects.filter(pk=uuid).exists(): current_value = UserUpload.objects.get(pk=uuid) else: - raise RuntimeError(f"Unknown image pk: {uuid}") + raise RuntimeError(f"Unknown image pk: {self.initial}") self.kwargs["widget"] = FlexibleImageWidget( help_text=self.help_text,