Remove add functionality from admin, refactor form

app/grandchallenge/algorithms/admin.py

@@ -2,14 +2,13 @@
 
 from django.contrib import admin
 from django.contrib.admin import ModelAdmin
-from django.core.exceptions import ObjectDoesNotExist, ValidationError
+from django.core.exceptions import ObjectDoesNotExist
 from django.db.models import Count, Sum
 from django.forms import ModelForm
 from django.urls import reverse
 from django.utils.html import format_html
 from guardian.admin import GuardedModelAdmin
 
-from grandchallenge.algorithms.forms import AlgorithmInterfaceBaseForm
 from grandchallenge.algorithms.models import (
     Algorithm,
     AlgorithmAlgorithmInterface,
@@ -258,16 +257,6 @@ class AlgorithmModelAdmin(GuardedModelAdmin):
     readonly_fields = ("creator", "algorithm", "sha256", "size_in_storage")
 
 
-class AlgorithmInterfaceAdminForm(AlgorithmInterfaceBaseForm):
-    def clean(self):
-        cleaned_data = super().clean()
-        if cleaned_data["existing_io"]:
-            raise ValidationError(
-                "An AlgorithmIO with the same combination of inputs and outputs already exists."
-            )
-        return cleaned_data
-
-
 @admin.register(AlgorithmInterface)
 class AlgorithmInterfaceAdmin(GuardedModelAdmin):
     list_display = (
@@ -279,7 +268,6 @@ class AlgorithmInterfaceAdmin(GuardedModelAdmin):
         "inputs__slug",
         "outputs__slug",
     )
-    form = AlgorithmInterfaceAdminForm
 
     def algorithm_inputs(self, obj):
         return oxford_comma(obj.inputs.all())
@@ -293,6 +281,9 @@ def has_change_permission(self, request, obj=None):
     def has_delete_permission(self, request, obj=None):
         return False
 
+    def has_add_permission(self, request, obj=None):
+        return False
+
 
 @admin.register(AlgorithmAlgorithmInterface)
 class AlgorithmAlgorithmInterfaceAdmin(GuardedModelAdmin):

app/grandchallenge/algorithms/forms.py

@@ -52,7 +52,6 @@
     AlgorithmModel,
     AlgorithmPermissionRequest,
     Job,
-    get_existing_interface_for_inputs_and_outputs,
 )
 from grandchallenge.algorithms.serializers import (
     AlgorithmImageSerializer,
@@ -1348,7 +1347,7 @@ def clean_algorithm_model(self):
         return algorithm_model
 
 
-class AlgorithmInterfaceBaseForm(SaveFormInitMixin, ModelForm):
+class AlgorithmInterfaceForm(SaveFormInitMixin, ModelForm):
     inputs = ModelMultipleChoiceField(
         queryset=ComponentInterface.objects.exclude(
             slug__in=[*NON_ALGORITHM_INTERFACES, "results-json-file"]
@@ -1361,10 +1360,22 @@ class AlgorithmInterfaceBaseForm(SaveFormInitMixin, ModelForm):
         ),
         widget=Select2MultipleWidget,
     )
+    set_as_default = BooleanField(required=False)
 
     class Meta:
         model = AlgorithmInterface
-        fields = ("inputs", "outputs")
+        fields = (
+            "inputs",
+            "outputs",
+            "set_as_default",
+        )
+
+    def __init__(self, *args, algorithm, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._algorithm = algorithm
+
+        if not self._algorithm.default_interface:
+            self.fields["set_as_default"].initial = True
 
     def clean(self):
         cleaned_data = super().clean()
@@ -1385,68 +1396,31 @@ def clean(self):
                 f"{oxford_comma(duplicate_interfaces)} present in both"
             )
 
-        cleaned_data["existing_io"] = (
-            get_existing_interface_for_inputs_and_outputs(
-                inputs=inputs,
-                outputs=outputs,
-            )
-        )
-
-        return cleaned_data
-
-
-class AlgorithmInterfaceGetOrCreateForm(AlgorithmInterfaceBaseForm):
-    set_as_default = BooleanField(required=False)
-
-    class Meta(AlgorithmInterfaceBaseForm.Meta):
-        fields = (
-            *AlgorithmInterfaceBaseForm.Meta.fields,
-            "set_as_default",
-        )
-
-    def __init__(self, *args, algorithm, **kwargs):
-        super().__init__(*args, **kwargs)
-        self._algorithm = algorithm
-
-        if not self._algorithm.default_interface:
-            self.fields["set_as_default"].initial = True
-
-    def clean(self):
-        cleaned_data = super().clean()
-
-        if (
-            cleaned_data["existing_io"]
-            and AlgorithmAlgorithmInterface.objects.filter(
-                algorithm=self._algorithm,
-                interface=cleaned_data["existing_io"],
-            ).exists()
-        ):
-            raise ValidationError(
-                "Your algorithm already has an interface with these inputs "
-                "and outputs. If you would like to update the 'is_default' "
-                "property of the interface, you can do so by updating the "
-                "existing interface on the interface list."
-            )
-
         return cleaned_data
 
     def save(self):
-        io = (
-            self.cleaned_data["existing_io"]
-            if self.cleaned_data["existing_io"]
-            else super().save()
+        interface = AlgorithmInterface.objects.create(
+            inputs=self.cleaned_data["inputs"],
+            outputs=self.cleaned_data["outputs"],
         )
 
         if self.cleaned_data["set_as_default"]:
             AlgorithmAlgorithmInterface.objects.filter(
                 algorithm=self._algorithm
             ).update(is_default=False)
 
-        self._algorithm.interfaces.add(
-            io,
-            through_defaults={
-                "is_default": self.cleaned_data["set_as_default"]
-            },
-        )
+        if AlgorithmAlgorithmInterface.objects.filter(
+            algorithm=self._algorithm, interface=interface
+        ).exists():
+            AlgorithmAlgorithmInterface.objects.filter(
+                algorithm=self._algorithm, interface=interface
+            ).update(is_default=self.cleaned_data["set_as_default"])
+        else:
+            self._algorithm.interfaces.add(
+                interface,
+                through_defaults={
+                    "is_default": self.cleaned_data["set_as_default"]
+                },
+            )
 
-        return io
+        return interface

app/grandchallenge/algorithms/views.py

@@ -47,7 +47,7 @@
     AlgorithmImageForm,
     AlgorithmImageUpdateForm,
     AlgorithmImportForm,
-    AlgorithmInterfaceGetOrCreateForm,
+    AlgorithmInterfaceForm,
     AlgorithmModelForm,
     AlgorithmModelUpdateForm,
     AlgorithmModelVersionControlForm,
@@ -1097,9 +1097,7 @@ def get(self, *_, **__):
             )
 
 
-class AlgorithmInterfacePermissionMixin(
-    VerificationRequiredMixin, AccessMixin
-):
+class AlgorithmInterfacePermissionMixin(AccessMixin):
     @property
     def algorithm(self):
         return get_object_or_404(Algorithm, slug=self.kwargs["slug"])
@@ -1117,7 +1115,7 @@ class AlgorithmInterfaceForAlgorithmCreate(
     AlgorithmInterfacePermissionMixin, CreateView
 ):
     model = AlgorithmInterface
-    form_class = AlgorithmInterfaceGetOrCreateForm
+    form_class = AlgorithmInterfaceForm
     success_message = "Algorithm interface successfully added"
 
     def get_success_url(self):

app/tests/algorithms_tests/test_forms.py

@@ -5,11 +5,10 @@
 from django.core.validators import MaxValueValidator, MinValueValidator
 from factory.django import ImageField
 
-from grandchallenge.algorithms.admin import AlgorithmInterfaceAdminForm
 from grandchallenge.algorithms.forms import (
     AlgorithmForm,
     AlgorithmForPhaseForm,
-    AlgorithmInterfaceGetOrCreateForm,
+    AlgorithmInterfaceForm,
     AlgorithmModelForm,
     AlgorithmModelVersionControlForm,
     AlgorithmPublishForm,
@@ -1299,27 +1298,10 @@ def test_algorithm_for_phase_form_memory_limited():
     assert max_validator.limit_value == 32
 
 
-@pytest.mark.django_db
-def test_algorithm_interface_check_for_uniqueness():
-    interface = AlgorithmInterfaceFactory()
-    ci1, ci2 = ComponentInterfaceFactory.create_batch(2)
-    interface.inputs.set([ci1])
-    interface.outputs.set([ci2])
-
-    form = AlgorithmInterfaceAdminForm(
-        data={"inputs": [ci1], "outputs": [ci2]}
-    )
-    assert form.is_valid() is False
-    assert (
-        "An AlgorithmIO with the same combination of inputs and outputs already exists."
-        in str(form.errors)
-    )
-
-
 @pytest.mark.django_db
 def test_algorithm_interface_disjoint_interfaces():
     ci = ComponentInterfaceFactory()
-    form = AlgorithmInterfaceAdminForm(data={"inputs": [ci], "outputs": [ci]})
+    form = AlgorithmInterfaceForm(data={"inputs": [ci], "outputs": [ci]})
     assert form.is_valid() is False
     assert "The sets of Inputs and Outputs must be unique" in str(form.errors)
 
@@ -1351,12 +1333,12 @@ def test_algorithm_for_phase_form_memory():
     assert max_validator.limit_value == 42
 
 
-class TestAlgorithmInterfaceGetOrCreateForm:
+class TestAlgorithmInterfaceForm:
     @pytest.mark.django_db
     def test_set_as_default_initial_value(self):
         alg = AlgorithmFactory()
 
-        form = AlgorithmInterfaceGetOrCreateForm(
+        form = AlgorithmInterfaceForm(
             algorithm=alg,
         )
         assert form.fields["set_as_default"].initial
@@ -1367,7 +1349,7 @@ def test_set_as_default_initial_value(self):
 
         del alg.default_interface
 
-        form = AlgorithmInterfaceGetOrCreateForm(
+        form = AlgorithmInterfaceForm(
             algorithm=alg,
         )
         assert not form.fields["set_as_default"].initial
@@ -1382,7 +1364,7 @@ def test_existing_io_is_reused(self):
 
         alg = AlgorithmFactory()
 
-        form = AlgorithmInterfaceGetOrCreateForm(
+        form = AlgorithmInterfaceForm(
             algorithm=alg,
             data={
                 "inputs": [inp.pk],
@@ -1404,7 +1386,7 @@ def test_new_default_interface_updates_related_interfaces(self):
 
         old_iot = AlgorithmAlgorithmInterface.objects.get()
 
-        form = AlgorithmInterfaceGetOrCreateForm(
+        form = AlgorithmInterfaceForm(
             algorithm=alg,
             data={
                 "inputs": [ci_1.pk],
@@ -1422,7 +1404,7 @@ def test_new_default_interface_updates_related_interfaces(self):
         assert not old_iot.is_default
 
     @pytest.mark.django_db
-    def test_default_interface_for_algorithm_not_altered_when_adding_new_non_default_interface(
+    def test_default_interface_for_algorithm_not_updated_when_adding_new_non_default_interface(
         self,
     ):
         alg = AlgorithmFactory()
@@ -1432,7 +1414,7 @@ def test_default_interface_for_algorithm_not_altered_when_adding_new_non_default
 
         ci_1, ci_2 = ComponentInterfaceFactory.create_batch(2)
 
-        form = AlgorithmInterfaceGetOrCreateForm(
+        form = AlgorithmInterfaceForm(
             algorithm=alg,
             data={
                 "inputs": [ci_1.pk],
@@ -1448,3 +1430,32 @@ def test_default_interface_for_algorithm_not_altered_when_adding_new_non_default
         assert new_io != io
         assert not new_iot.is_default
         assert old_iot.is_default
+
+    @pytest.mark.django_db
+    def test_is_default_is_updated_when_adding_an_already_existing_interface(
+        self,
+    ):
+        alg = AlgorithmFactory()
+        ci_1, ci_2 = ComponentInterfaceFactory.create_batch(2)
+
+        io = AlgorithmInterfaceFactory()
+        io.inputs.set([ci_1])
+        io.outputs.set([ci_2])
+
+        alg.interfaces.add(io, through_defaults={"is_default": True})
+        old_iot = AlgorithmAlgorithmInterface.objects.get()
+
+        form = AlgorithmInterfaceForm(
+            algorithm=alg,
+            data={
+                "inputs": [ci_1.pk],
+                "outputs": [ci_2.pk],
+                "set_as_default": False,
+            },
+        )
+        form.is_valid()
+        new_io = form.save()
+        old_iot.refresh_from_db()
+
+        assert new_io == io
+        assert not old_iot.is_default

app/tests/algorithms_tests/test_views.py

@@ -2157,33 +2157,19 @@ def test_algorithm_interface_view_permission(client, viewname):
         user_without_alg_add_perm,
         algorithm_editor_with_alg_add,
         algorithm_editor_without_alg_add,
-        verified_alg_editor_with_alg_add,
-        verified_alg_editor_without_alg_add,
-    ) = UserFactory.create_batch(6)
+    ) = UserFactory.create_batch(4)
     assign_perm("algorithms.add_algorithm", user_with_alg_add_perm)
     assign_perm("algorithms.add_algorithm", algorithm_editor_with_alg_add)
-    assign_perm("algorithms.add_algorithm", verified_alg_editor_with_alg_add)
 
     alg = AlgorithmFactory()
     alg.add_editor(algorithm_editor_with_alg_add)
     alg.add_editor(algorithm_editor_without_alg_add)
-    alg.add_editor(verified_alg_editor_with_alg_add)
-    alg.add_editor(verified_alg_editor_without_alg_add)
-
-    VerificationFactory(
-        user=verified_alg_editor_with_alg_add, is_verified=True
-    )
-    VerificationFactory(
-        user=verified_alg_editor_without_alg_add, is_verified=True
-    )
 
     for user, status in [
         [user_with_alg_add_perm, 403],
         [user_without_alg_add_perm, 403],
-        [algorithm_editor_with_alg_add, 403],
+        [algorithm_editor_with_alg_add, 200],
         [algorithm_editor_without_alg_add, 403],
-        [verified_alg_editor_with_alg_add, 200],
-        [verified_alg_editor_without_alg_add, 403],
     ]:
         response = get_view_for_user(
             viewname=viewname,
@@ -2201,8 +2187,6 @@ def test_algorithm_interface_create(client):
     alg = AlgorithmFactory()
     alg.add_editor(user)
 
-    VerificationFactory(user=user, is_verified=True)
-
     ci_1 = ComponentInterfaceFactory()
     ci_2 = ComponentInterfaceFactory()