Merge pull request #3528 from open-formulieren/issue/3527-unique-cons…

…traint

Add unique constraint on form + form definition

bin/check_non_unique_steps.py

@@ -0,0 +1,72 @@
+#!/usr/bin/env python
+import sys
+from pathlib import Path
+
+import django
+
+from tabulate import tabulate
+
+SRC_DIR = Path(__file__).parent.parent / "src"
+sys.path.insert(0, str(SRC_DIR.resolve()))
+
+
+def check_non_unique_steps() -> bool:
+    from django.db.models import Count
+
+    from openforms.forms.models import Form, FormDefinition, FormStep
+
+    # query for form definitions used multiple times in a form
+    qs = (
+        FormStep.objects.values("form", "form_definition")
+        .annotate(occurrences=Count("form_definition"))
+        .filter(occurrences__gt=1)
+        .order_by()  # reset ordering (implicitly added by django-ordered-model)
+    )
+    num = qs.count()
+    if not num:
+        print("No forms found with duplicated form steps.")
+        return True
+
+    # report which forms have issues
+    forms = Form.objects.filter(pk__in=qs.values_list("form")).in_bulk()
+    form_definitions = FormDefinition.objects.filter(
+        pk__in=qs.values_list("form_definition")
+    ).in_bulk()
+
+    duplicates = []
+    for item in qs:
+        form = forms[item["form"]]
+        form_definition = form_definitions[item["form_definition"]]
+
+        duplicates.append(
+            [
+                form.admin_name,
+                form_definition.admin_name,
+                item["occurrences"],
+            ]
+        )
+
+    print("Found forms with duplicated steps.")
+    print("")
+    print(
+        tabulate(
+            duplicates,
+            headers=("Form", "Form step", "Occurrences"),
+        )
+    )
+
+    return False
+
+
+def main(skip_setup=False) -> bool:
+    from openforms.setup import setup_env
+
+    if not skip_setup:
+        setup_env()
+        django.setup()
+
+    return check_non_unique_steps()
+
+
+if __name__ == "__main__":
+    main()

src/openforms/forms/migrations/0095_formstep_form_form_definition_unique_together.py

@@ -0,0 +1,20 @@
+# Generated by Django 3.2.21 on 2023-10-09 10:23
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("forms", "0094_update_config_family"),
+    ]
+
+    operations = [
+        migrations.AddConstraint(
+            model_name="formstep",
+            constraint=models.UniqueConstraint(
+                fields=("form", "form_definition"),
+                name="form_form_definition_unique_together",
+            ),
+        ),
+    ]

src/openforms/forms/models/form_step.py

@@ -75,6 +75,10 @@ class Meta(OrderedModel.Meta):
         verbose_name = _("form step")
         verbose_name_plural = _("form steps")
         constraints = [
+            models.UniqueConstraint(
+                fields=["form", "form_definition"],
+                name="form_form_definition_unique_together",
+            ),
             models.UniqueConstraint(
                 fields=["form", "slug"], name="form_slug_unique_together"
             ),

src/openforms/forms/tests/admin/test_form_definition.py

@@ -47,38 +47,6 @@ def test_used_in_forms_when_form_has_user_input_properly_escaped(self):
             str(response.content),
         )
 
-    def test_used_in_forms_shows_forms_for_each_for_each_form_definition_without_duplicates(
-        self,
-    ):
-        second_form_definition = FormDefinitionFactory.create()
-        second_form = FormFactory.create()
-        second_form_url = reverse(
-            "admin:forms_form_change",
-            kwargs={"object_id": second_form.pk},
-        )
-
-        FormStepFactory.create(form=self.form, form_definition=second_form_definition)
-        FormStepFactory.create(form=second_form, form_definition=self.form_definition)
-        FormStepFactory.create(form=second_form, form_definition=second_form_definition)
-
-        # Duplicate steps
-        FormStepFactory.create(form=self.form, form_definition=second_form_definition)
-        FormStepFactory.create(form=second_form, form_definition=second_form_definition)
-        self.client.force_login(user=self.user)
-
-        response = self.client.get(reverse("admin:forms_formdefinition_changelist"))
-
-        self.assertInHTML(
-            f'<li><a href="{self.form_url}">{self.form.admin_name}</a></li>',
-            str(response.content),
-            count=2,
-        )
-        self.assertInHTML(
-            f'<li><a href="{second_form_url}">{second_form.admin_name}</a></li>',
-            str(response.content),
-            count=2,
-        )
-
     def test_make_copies_action_makes_copy_of_a_form_definition(self):
         response = self.app.get(
             reverse("admin:forms_formdefinition_changelist"), user=self.user

src/openforms/upgrades/upgrade_paths.py

@@ -79,12 +79,20 @@ def run_checks(self) -> bool:
     "2.4": UpgradeConstraint(
         valid_ranges={
             VersionRange(minimum="2.3.0"),  # 2.4.0 squashes migrations again
-        }
+        },
+        scripts=(
+            # run detection again so we can now add the DB constraint
+            "check_non_unique_steps",
+        ),
     ),
     "2.3": UpgradeConstraint(
         valid_ranges={
             VersionRange(minimum="2.1.3"),
-        }
+        },
+        scripts=(
+            # run detection to prevent crashes, see #3527
+            "check_non_unique_steps",
+        ),
     ),
     "2.2": UpgradeConstraint(
         valid_ranges={