fix unique=True validation for ChoiceField

Author: Ekluv

rest_framework/utils/field_mapping.py

@@ -123,18 +123,70 @@ def get_field_kwargs(field_name, model_field):
             kwargs['allow_folders'] = model_field.allow_folders
 
     if model_field.choices:
-        # If this model field contains choices, then return early.
-        # Further keyword arguments are not valid.
         kwargs['choices'] = model_field.choices
-        return kwargs
-
-    # Our decimal validation is handled in the field code, not validator code.
-    # (In Django 1.9+ this differs from previous style)
-    if isinstance(model_field, models.DecimalField) and DecimalValidator:
-        validator_kwarg = [
-            validator for validator in validator_kwarg
-            if not isinstance(validator, DecimalValidator)
-        ]
+    else:
+        # Ensure that max_value is passed explicitly as a keyword arg,
+        # rather than as a validator.
+        max_value = next((
+            validator.limit_value for validator in validator_kwarg
+            if isinstance(validator, validators.MaxValueValidator)
+        ), None)
+        if max_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES):
+            kwargs['max_value'] = max_value
+            validator_kwarg = [
+                validator for validator in validator_kwarg
+                if not isinstance(validator, validators.MaxValueValidator)
+            ]
+
+        # Ensure that max_value is passed explicitly as a keyword arg,
+        # rather than as a validator.
+        min_value = next((
+            validator.limit_value for validator in validator_kwarg
+            if isinstance(validator, validators.MinValueValidator)
+        ), None)
+        if min_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES):
+            kwargs['min_value'] = min_value
+            validator_kwarg = [
+                validator for validator in validator_kwarg
+                if not isinstance(validator, validators.MinValueValidator)
+            ]
+
+        # URLField does not need to include the URLValidator argument,
+        # as it is explicitly added in.
+        if isinstance(model_field, models.URLField):
+            validator_kwarg = [
+                validator for validator in validator_kwarg
+                if not isinstance(validator, validators.URLValidator)
+            ]
+
+        # EmailField does not need to include the validate_email argument,
+        # as it is explicitly added in.
+        if isinstance(model_field, models.EmailField):
+            validator_kwarg = [
+                validator for validator in validator_kwarg
+                if validator is not validators.validate_email
+            ]
+
+        # SlugField do not need to include the 'validate_slug' argument,
+        if isinstance(model_field, models.SlugField):
+            validator_kwarg = [
+                validator for validator in validator_kwarg
+                if validator is not validators.validate_slug
+            ]
+
+        # IPAddressField do not need to include the 'validate_ipv46_address' argument,
+        if isinstance(model_field, models.GenericIPAddressField):
+            validator_kwarg = [
+                validator for validator in validator_kwarg
+                if validator is not validators.validate_ipv46_address
+            ]
+        # Our decimal validation is handled in the field code, not validator code.
+        # (In Django 1.9+ this differs from previous style)
+        if isinstance(model_field, models.DecimalField) and DecimalValidator:
+            validator_kwarg = [
+                validator for validator in validator_kwarg
+                if not isinstance(validator, DecimalValidator)
+            ]
 
     # Ensure that max_length is passed explicitly as a keyword arg,
     # rather than as a validator.
@@ -160,62 +212,6 @@ def get_field_kwargs(field_name, model_field):
             if not isinstance(validator, validators.MinLengthValidator)
         ]
 
-    # Ensure that max_value is passed explicitly as a keyword arg,
-    # rather than as a validator.
-    max_value = next((
-        validator.limit_value for validator in validator_kwarg
-        if isinstance(validator, validators.MaxValueValidator)
-    ), None)
-    if max_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES):
-        kwargs['max_value'] = max_value
-        validator_kwarg = [
-            validator for validator in validator_kwarg
-            if not isinstance(validator, validators.MaxValueValidator)
-        ]
-
-    # Ensure that max_value is passed explicitly as a keyword arg,
-    # rather than as a validator.
-    min_value = next((
-        validator.limit_value for validator in validator_kwarg
-        if isinstance(validator, validators.MinValueValidator)
-    ), None)
-    if min_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES):
-        kwargs['min_value'] = min_value
-        validator_kwarg = [
-            validator for validator in validator_kwarg
-            if not isinstance(validator, validators.MinValueValidator)
-        ]
-
-    # URLField does not need to include the URLValidator argument,
-    # as it is explicitly added in.
-    if isinstance(model_field, models.URLField):
-        validator_kwarg = [
-            validator for validator in validator_kwarg
-            if not isinstance(validator, validators.URLValidator)
-        ]
-
-    # EmailField does not need to include the validate_email argument,
-    # as it is explicitly added in.
-    if isinstance(model_field, models.EmailField):
-        validator_kwarg = [
-            validator for validator in validator_kwarg
-            if validator is not validators.validate_email
-        ]
-
-    # SlugField do not need to include the 'validate_slug' argument,
-    if isinstance(model_field, models.SlugField):
-        validator_kwarg = [
-            validator for validator in validator_kwarg
-            if validator is not validators.validate_slug
-        ]
-
-    # IPAddressField do not need to include the 'validate_ipv46_address' argument,
-    if isinstance(model_field, models.GenericIPAddressField):
-        validator_kwarg = [
-            validator for validator in validator_kwarg
-            if validator is not validators.validate_ipv46_address
-        ]
-
     if getattr(model_field, 'unique', False):
         unique_error_message = model_field.error_messages.get('unique', None)
         if unique_error_message:

tests/test_serializer.py

@@ -519,3 +519,29 @@ class Grandchild(Child):
         assert len(Parent().get_fields()) == 2
         assert len(Child().get_fields()) == 2
         assert len(Grandchild().get_fields()) == 2
+
+
+class Poll(models.Model):
+    CHOICES = (
+        ('choice1', 'choice 1'),
+        ('choice2', 'choice 1'),
+    )
+
+    name = models.CharField(
+        'name', max_length=254, unique=True, choices=CHOICES
+    )
+
+
+@pytest.mark.django_db
+class Test5004UniqueChoiceField:
+    def test_unique_choice_field(self):
+        Poll.objects.create(name='choice1')
+
+        class PollSerializer(serializers.ModelSerializer):
+            class Meta:
+                model = Poll
+                fields = '__all__'
+
+        serializer = PollSerializer(data={'name': 'choice1'})
+        assert not serializer.is_valid()
+        assert serializer.errors == {'name': ['poll with this name already exists.']}