Block changing interfaces for interactive algorithms (#4614)

Related to DIAGNijmegen/rse-roadmap#400

Author: koopmant

app/grandchallenge/algorithms/models.py

@@ -391,7 +391,18 @@ def api_url(self) -> str:
 
     @property
     def algorithm_interfaces_locked(self):
-        return False
+        if self.interactive_algorithms.exists():
+            return True
+        else:
+            return False
+
+    @property
+    def algorithm_interfaces_locked_message(self):
+        return (
+            "Interfaces cannot be changed because this is an interactive algorithm. "
+            "Please contact support if this algorithm requires changes to its "
+            "interfaces."
+        )
 
     def save(self, *args, **kwargs):
         adding = self._state.adding
@@ -667,6 +678,21 @@ class Meta:
             ),
         ]
 
+    def clean(self):
+        super().clean()
+        if self.algorithm.algorithm_interfaces_locked:
+            raise ValidationError(
+                self.algorithm.algorithm_interfaces_locked_message
+            )
+
+    def save(self, *args, **kwargs):
+        self.clean()
+        return super().save(*args, **kwargs)
+
+    def delete(self, *args, **kwargs):
+        self.clean()
+        return super().delete(*args, **kwargs)
+
     def __str__(self):
         return str(self.interface)
 

app/grandchallenge/algorithms/signals.py

@@ -1,8 +1,9 @@
+from django.core.exceptions import ValidationError
 from django.db.models.signals import m2m_changed, pre_delete
 from django.dispatch import receiver
 from guardian.shortcuts import assign_perm, remove_perm
 
-from grandchallenge.algorithms.models import Job
+from grandchallenge.algorithms.models import Algorithm, Job
 from grandchallenge.cases.models import Image
 
 
@@ -132,3 +133,14 @@ def update_view_image_permissions_on_job_deletion(*_, instance: Job, **__):
         # We cannot remove image permissions directly as the groups
         # may have permissions through another object
         image.update_viewer_groups_permissions(exclude_jobs=jobs)
+
+
+@receiver(m2m_changed, sender=Algorithm.interfaces.through)
+def prevent_interfaces_update_for_interactive_algorithms(
+    *_, instance, action, **__
+):
+    if (
+        action in {"pre_add", "pre_remove", "pre_clear"}
+        and instance.algorithm_interfaces_locked
+    ):
+        raise ValidationError(instance.algorithm_interfaces_locked_message)

app/grandchallenge/algorithms/templates/algorithms/partials/algorithminterface_list.html

@@ -5,9 +5,15 @@
 
 <h2>Algorithm Interfaces for {{ base_obj }}</h2>
 
+{% if base_obj.algorithm_interfaces_locked %}
+    <div class="alert alert-info">
+        {{ base_obj.algorithm_interfaces_locked_message }}
+    </div>
+{% endif %}
 <p>The following interfaces (i.e. input-output combinations) are configured for your {{ base_obj|verbose_name }}:</p>
 <p>
-    <span class="d-inline-block" tabindex="0" data-toggle="tooltip" title="{% if base_obj.algorithm_interfaces_locked %}Disabled because this phase is a parent or has a parent phase.{% endif %}">
+    <span class="d-inline-block" tabindex="0" data-toggle="tooltip"
+          title="{% if base_obj.algorithm_interfaces_locked %}{{ base_obj.algorithm_interfaces_locked_message }}{% endif %}">
         <a class="btn btn-primary {% if base_obj.algorithm_interfaces_locked %} disabled {% endif %}"
            href="{{ base_obj.algorithm_interface_create_url }}"
         >

app/grandchallenge/algorithms/templates/algorithms/partials/algorithminterface_table.html

@@ -33,7 +33,7 @@
                     {% if delete_option %}
                         <td class="align-middle">
                             <span class="d-inline-block" tabindex="0" data-toggle="tooltip"
-                                  title="{% if base_obj.algorithm_interfaces_locked %}Disabled because this phase is a parent or has a parent phase.{% endif %}">
+                                  title="{% if base_obj.algorithm_interfaces_locked %}{{ base_obj.algorithm_interfaces_locked_message }}{% endif %}">
                                 <a class="btn btn-sm btn-danger m-0 {% if base_obj.algorithm_interfaces_locked %} disabled {% endif %}"
                                    href="{% url base_obj.algorithm_interface_delete_viewname slug=base_obj.slug interface_pk=interface.pk %}"
                                 >

app/grandchallenge/algorithms/views.py

@@ -1246,8 +1246,21 @@ def get_form_kwargs(self):
         return kwargs
 
 
+class AlgorithmInterfacesLockedMixin(AccessMixin):
+    def dispatch(self, request, *args, **kwargs):
+        if self.algorithm.algorithm_interfaces_locked:
+            messages.error(
+                request,
+                self.algorithm.algorithm_interfaces_locked_message,
+            )
+            return self.handle_no_permission()
+        return super().dispatch(request, *args, **kwargs)
+
+
 class AlgorithmInterfaceForAlgorithmCreate(
-    AlgorithmInterfacePermissionMixin, AlgorithmInterfaceCreateBase
+    AlgorithmInterfacePermissionMixin,
+    AlgorithmInterfacesLockedMixin,
+    AlgorithmInterfaceCreateBase,
 ):
     @property
     def base_obj(self):
@@ -1290,7 +1303,9 @@ def get_context_data(self, *args, **kwargs):
 
 
 class AlgorithmInterfaceForAlgorithmDelete(
-    AlgorithmInterfacePermissionMixin, DeleteView
+    AlgorithmInterfacePermissionMixin,
+    AlgorithmInterfacesLockedMixin,
+    DeleteView,
 ):
     model = AlgorithmAlgorithmInterface
     form_class = AlgorithmAlgorithmInterfaceDeleteForm

app/grandchallenge/evaluation/models.py

@@ -914,6 +914,10 @@ def algorithm_interfaces_locked(self):
         else:
             return False
 
+    @property
+    def algorithm_interfaces_locked_message(self):
+        return "Disabled because this phase is a parent or has a parent phase."
+
     @cached_property
     def valid_metrics(self):
         return (

app/tests/algorithms_tests/test_models.py

@@ -1599,3 +1599,65 @@ def test_interactive_algorithm_mark_non_invoke_image_raises_validation_error():
         "Only algorithm images that implement the invoke API can be activated because this is an interactive algorithm"
         in str(error)
     )
+
+
+@pytest.mark.django_db
+def test_cannot_set_interface_for_interactive_algorithm():
+    algorithm = AlgorithmFactory()
+    new_interface = AlgorithmInterfaceFactory()
+    InteractiveAlgorithmFactory(algorithm=algorithm)
+
+    with pytest.raises(ValidationError) as error:
+        algorithm.interfaces.set([new_interface])
+
+    assert (
+        "Interfaces cannot be changed because this is an interactive algorithm"
+        in str(error)
+    )
+
+
+@pytest.mark.django_db
+def test_cannot_add_interface_for_interactive_algorithm():
+    algorithm = AlgorithmFactory()
+    new_interface = AlgorithmInterfaceFactory()
+    InteractiveAlgorithmFactory(algorithm=algorithm)
+
+    with pytest.raises(ValidationError) as error:
+        algorithm.interfaces.add(new_interface)
+
+    assert (
+        "Interfaces cannot be changed because this is an interactive algorithm"
+        in str(error.value)
+    )
+
+
+@pytest.mark.django_db
+def test_cannot_remove_interface_for_interactive_algorithm():
+    algorithm = AlgorithmFactory()
+    interface = AlgorithmInterfaceFactory()
+    algorithm.interfaces.add(interface)
+    InteractiveAlgorithmFactory(algorithm=algorithm)
+
+    with pytest.raises(ValidationError) as error:
+        algorithm.interfaces.remove(interface)
+
+    assert (
+        "Interfaces cannot be changed because this is an interactive algorithm"
+        in str(error.value)
+    )
+
+
+@pytest.mark.django_db
+def test_cannot_clear_interface_for_interactive_algorithm():
+    algorithm = AlgorithmFactory()
+    interface = AlgorithmInterfaceFactory()
+    algorithm.interfaces.add(interface)
+    InteractiveAlgorithmFactory(algorithm=algorithm)
+
+    with pytest.raises(ValidationError) as error:
+        algorithm.interfaces.clear()
+
+    assert (
+        "Interfaces cannot be changed because this is an interactive algorithm"
+        in str(error.value)
+    )

app/tests/algorithms_tests/test_views.py

@@ -40,6 +40,7 @@
     AlgorithmJobFactory,
     AlgorithmModelFactory,
     AlgorithmPermissionRequestFactory,
+    InteractiveAlgorithmFactory,
 )
 from tests.cases_tests import RESOURCE_PATH
 from tests.components_tests.factories import (
@@ -2518,3 +2519,61 @@ def test_algorithm_users_list_is_filtered(client):
             url=reverse("algorithms:users-list"), client=client, user=user
         )
         assert expected_algorithms == {*response.context[-1]["object_list"]}
+
+
+@pytest.mark.django_db
+def test_interactive_algorithm_interfaces_locked(client):
+    algorithm = AlgorithmFactory()
+    editor = UserFactory()
+    algorithm.add_editor(editor)
+    assign_perm("algorithms.add_algorithm", editor)
+    interface = AlgorithmInterfaceFactory()
+    algorithm.interfaces.add(interface)
+
+    response = get_view_for_user(
+        viewname="algorithms:interface-create",
+        client=client,
+        user=editor,
+        reverse_kwargs={
+            "slug": algorithm.slug,
+        },
+    )
+
+    assert response.status_code == 200
+
+    response = get_view_for_user(
+        viewname="algorithms:interface-delete",
+        client=client,
+        user=editor,
+        reverse_kwargs={
+            "slug": algorithm.slug,
+            "interface_pk": interface.pk,
+        },
+    )
+
+    assert response.status_code == 200
+
+    InteractiveAlgorithmFactory(algorithm=algorithm)
+
+    response = get_view_for_user(
+        viewname="algorithms:interface-create",
+        client=client,
+        user=editor,
+        reverse_kwargs={
+            "slug": algorithm.slug,
+        },
+    )
+
+    assert response.status_code == 403
+
+    response = get_view_for_user(
+        viewname="algorithms:interface-delete",
+        client=client,
+        user=editor,
+        reverse_kwargs={
+            "slug": algorithm.slug,
+            "interface_pk": interface.pk,
+        },
+    )
+
+    assert response.status_code == 403