Questions: Validate type changes (Z#23197118) (#5259)

* Questions: Validate type changes (Z#23197118)

* Update src/pretix/base/forms/questions.py

Co-authored-by: Richard Schreiber <schreiber@rami.io>

* Update src/pretix/base/forms/questions.py

Co-authored-by: Richard Schreiber <schreiber@rami.io>

* Update src/pretix/base/forms/questions.py

Co-authored-by: Richard Schreiber <schreiber@rami.io>

* Update src/pretix/base/models/items.py

Co-authored-by: Richard Schreiber <schreiber@rami.io>

* Fix failing test

---------

Co-authored-by: Richard Schreiber <schreiber@rami.io>

src/pretix/api/serializers/item.py

@@ -505,6 +505,11 @@ class QuestionSerializer(I18nAwareModelSerializer):
         Question._clean_identifier(self.context['event'], value, self.instance)
         return value
 
+    def validate_type(self, value):
+        if self.instance:
+            self.instance.clean_type_change(self.instance.type, value)
+        return value
+
     def validate_dependency_question(self, value):
         if value:
             if value.type not in (Question.TYPE_CHOICE, Question.TYPE_BOOLEAN, Question.TYPE_CHOICE_MULTIPLE):

src/pretix/base/forms/questions.py

@@ -896,10 +896,17 @@ class BaseQuestionsForm(forms.Form):
                             'Please enter a date no later than {max}.',
                             max=date_format(q.valid_date_max, "SHORT_DATE_FORMAT"),
                         )
+                if initial and initial.answer:
+                    try:
+                        _initial = dateutil.parser.parse(initial.answer).date()
+                    except dateutil.parser.ParserError:
+                        _initial = None
+                else:
+                    _initial = None
                 field = forms.DateField(
                     label=label, required=required,
                     help_text=help_text,
-                    initial=dateutil.parser.parse(initial.answer).date() if initial and initial.answer else None,
+                    initial=_initial,
                     widget=DatePickerWidget(attrs),
                 )
                 if q.valid_date_min:
@@ -907,10 +914,17 @@ class BaseQuestionsForm(forms.Form):
                 if q.valid_date_max:
                     field.validators.append(MaxDateValidator(q.valid_date_max))
             elif q.type == Question.TYPE_TIME:
+                if initial and initial.answer:
+                    try:
+                        _initial = dateutil.parser.parse(initial.answer).time()
+                    except dateutil.parser.ParserError:
+                        _initial = None
+                else:
+                    _initial = None
                 field = forms.TimeField(
                     label=label, required=required,
                     help_text=help_text,
-                    initial=dateutil.parser.parse(initial.answer).time() if initial and initial.answer else None,
+                    initial=_initial,
                     widget=TimePickerWidget(time_format=get_format_without_seconds('TIME_INPUT_FORMATS')),
                 )
             elif q.type == Question.TYPE_DATETIME:
@@ -931,10 +945,19 @@ class BaseQuestionsForm(forms.Form):
                             'Please enter a date and time no later than {max}.',
                             max=date_format(q.valid_datetime_max, "SHORT_DATETIME_FORMAT"),
                         )
+
+                if initial and initial.answer:
+                    try:
+                        _initial = dateutil.parser.parse(initial.answer).astimezone(tz)
+                    except dateutil.parser.ParserError:
+                        _initial = None
+                else:
+                    _initial = None
+
                 field = SplitDateTimeField(
                     label=label, required=required,
                     help_text=help_text,
-                    initial=dateutil.parser.parse(initial.answer).astimezone(tz) if initial and initial.answer else None,
+                    initial=_initial,
                     widget=SplitDateTimePickerWidget(
                         time_format=get_format_without_seconds('TIME_INPUT_FORMATS'),
                         min_date=q.valid_datetime_min,

src/pretix/base/models/items.py

@@ -1925,6 +1925,25 @@ class Question(LoggedModel):
             raise ValidationError(_("The maximum value must not be lower than the minimum value."))
         super().clean()
 
+    def clean_type_change(self, old_type, new_type):
+        if old_type == new_type:
+            return True
+        if not self.pk or not self.answers.exists():
+            return True
+        if new_type == self.TYPE_TEXT and old_type != self.TYPE_FILE:
+            # All types can be converted to text except file
+            return True
+        if new_type == self.TYPE_STRING and old_type not in (self.TYPE_TEXT, self.TYPE_FILE):
+            # All types can be converted to string except text or file
+            return True
+        if new_type == self.TYPE_CHOICE_MULTIPLE and old_type == self.TYPE_CHOICE:
+            # Single-choice can be converted to multiple choice without loss
+            return True
+        raise ValidationError(
+            _("The system already contains answers to this question that are not compatible with changing the "
+              "type of question without data loss. Consider hiding this question and creating a new one instead.")
+        )
+
 
 class QuestionOption(models.Model):
     question = models.ForeignKey('Question', related_name='options', on_delete=models.CASCADE)

src/pretix/control/forms/item.py

@@ -201,6 +201,12 @@ class QuestionForm(I18nModelForm):
 
         return val
 
+    def clean_type(self):
+        val = self.cleaned_data.get('type')
+        if self.instance:
+            self.instance.clean_type_change(self.instance.type, val)
+        return val
+
     def clean_identifier(self):
         val = self.cleaned_data.get('identifier')
         Question._clean_identifier(self.instance.event, val, self.instance)

src/tests/api/test_items.py

@@ -2428,6 +2428,45 @@ def test_question_update(token_client, organizer, event, question):
     assert question.type == "N"
 
 
+@pytest.mark.django_db
+def test_question_update_type_changes(token_client, organizer, event, question):
+    # Allowed because no answers exist
+    resp = token_client.patch(
+        '/api/v1/organizers/{}/events/{}/questions/{}/'.format(organizer.slug, event.slug, question.pk),
+        {
+            "type": "B",
+        },
+        format='json'
+    )
+    assert resp.status_code == 200
+
+    with scopes_disabled():
+        question.answers.create(answer="12")
+
+    # Allowed change
+    resp = token_client.patch(
+        '/api/v1/organizers/{}/events/{}/questions/{}/'.format(organizer.slug, event.slug, question.pk),
+        {
+            "type": "S",
+        },
+        format='json'
+    )
+    assert resp.status_code == 200
+
+    # Forbidden change
+    resp = token_client.patch(
+        '/api/v1/organizers/{}/events/{}/questions/{}/'.format(organizer.slug, event.slug, question.pk),
+        {
+            "type": "B",
+        },
+        format='json'
+    )
+    assert resp.status_code == 400
+    assert resp.content.decode() == ('{"type":["The system already contains answers to this question that are not '
+                                     'compatible with changing the type of question without data loss. Consider hiding '
+                                     'this question and creating a new one instead."]}')
+
+
 @pytest.mark.django_db
 def test_question_update_circular_dependency(token_client, organizer, event, question):
     with scopes_disabled():