From cf622392c0b9c7afc78c4ba730b7359e967a91c8 Mon Sep 17 00:00:00 2001
From: Raphael Michel <michel@rami.io>
Date: Tue, 20 Sep 2022 10:20:56 +0200
Subject: [PATCH] Ensure uniqueness of question identifiers (#2358)

---
 ...21_clean_nonunique_question_identifiers.py | 28 +++++++++++++++++++
 .../0222_alter_question_unique_together.py    | 17 +++++++++++
 src/pretix/base/models/items.py               |  3 +-
 src/pretix/control/forms/item.py              |  5 ++++
 4 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 src/pretix/base/migrations/0221_clean_nonunique_question_identifiers.py
 create mode 100644 src/pretix/base/migrations/0222_alter_question_unique_together.py

diff --git a/src/pretix/base/migrations/0221_clean_nonunique_question_identifiers.py b/src/pretix/base/migrations/0221_clean_nonunique_question_identifiers.py
new file mode 100644
index 0000000000..a3f54b4cdb
--- /dev/null
+++ b/src/pretix/base/migrations/0221_clean_nonunique_question_identifiers.py
@@ -0,0 +1,28 @@
+# Generated by Django 3.2.4 on 2021-12-01 11:55
+
+from django.db import migrations
+from django.db.models import Count
+
+
+def change_unique_identifiers(apps, schema_editor):
+    # We cannot really know if a position was bundled or an add-on, but we can at least guess
+    Question = apps.get_model("pretixbase", "Question")
+
+    for r in Question.objects.values('event', 'identifier').order_by().annotate(c=Count('*')).filter(c__gt=1):
+        qs = Question.objects.filter(identifier=r['identifier'], event_id=r['event'])
+        for i, q in enumerate(qs[1:]):
+            q.identifier += f'_{i + 2}'
+            q.save(update_fields=['identifier'])
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('pretixbase', '0220_auto_20220811_1002'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            change_unique_identifiers,
+            migrations.RunPython.noop,
+        ),
+    ]
diff --git a/src/pretix/base/migrations/0222_alter_question_unique_together.py b/src/pretix/base/migrations/0222_alter_question_unique_together.py
new file mode 100644
index 0000000000..3d96f16fce
--- /dev/null
+++ b/src/pretix/base/migrations/0222_alter_question_unique_together.py
@@ -0,0 +1,17 @@
+# Generated by Django 3.2.4 on 2021-12-01 12:04
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('pretixbase', '0221_clean_nonunique_question_identifiers'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='question',
+            unique_together={('event', 'identifier')},
+        ),
+    ]
diff --git a/src/pretix/base/models/items.py b/src/pretix/base/models/items.py
index 7e60a79954..69a2511de0 100644
--- a/src/pretix/base/models/items.py
+++ b/src/pretix/base/models/items.py
@@ -1329,6 +1329,7 @@ class Question(LoggedModel):
         verbose_name = _("Question")
         verbose_name_plural = _("Questions")
         ordering = ('position', 'id')
+        unique_together = (('event', 'identifier'),)
 
     def __str__(self):
         return str(self.question)
@@ -1344,7 +1345,7 @@ class Question(LoggedModel):
     @staticmethod
     def _clean_identifier(event, code, instance=None):
         qs = Question.objects.filter(event=event, identifier__iexact=code)
-        if instance:
+        if instance and instance.pk:
             qs = qs.exclude(pk=instance.pk)
         if qs.exists():
             raise ValidationError(_('This identifier is already used for a different question.'))
diff --git a/src/pretix/control/forms/item.py b/src/pretix/control/forms/item.py
index 08ce92f4e2..6bd53a0c83 100644
--- a/src/pretix/control/forms/item.py
+++ b/src/pretix/control/forms/item.py
@@ -129,6 +129,11 @@ class QuestionForm(I18nModelForm):
 
         return val
 
+    def clean_identifier(self):
+        val = self.cleaned_data.get('identifier')
+        Question._clean_identifier(self.instance.event, val, self.instance)
+        return val
+
     def clean(self):
         d = super().clean()
         if d.get('dependency_question') and not d.get('dependency_values'):