diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 49d2a6d5d..9bbe66330 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -15,6 +15,7 @@ import warnings from django.core import validators from django.core.exceptions import ValidationError from django.conf import settings +from django.db.models.fields import BLANK_CHOICE_DASH from django import forms from django.forms import widgets from django.utils.encoding import is_protected_type @@ -407,6 +408,8 @@ class ChoiceField(WritableField): def __init__(self, choices=(), *args, **kwargs): super(ChoiceField, self).__init__(*args, **kwargs) self.choices = choices + if not self.required: + self.choices = BLANK_CHOICE_DASH + self.choices def _get_choices(self): return self._choices diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 7707de7a7..500bb3066 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -705,15 +705,14 @@ class ModelSerializer(Serializer): Creates a default instance of a basic non-relational field. """ kwargs = {} - has_default = model_field.has_default() - if model_field.null or model_field.blank or has_default: + if model_field.null or model_field.blank: kwargs['required'] = False if isinstance(model_field, models.AutoField) or not model_field.editable: kwargs['read_only'] = True - if has_default: + if model_field.has_default(): kwargs['default'] = model_field.get_default() if issubclass(model_field.__class__, models.TextField): diff --git a/rest_framework/tests/fields.py b/rest_framework/tests/fields.py index 5b5ce835a..6b1cdfc77 100644 --- a/rest_framework/tests/fields.py +++ b/rest_framework/tests/fields.py @@ -659,3 +659,29 @@ class DecimalFieldTest(TestCase): self.assertFalse(s.is_valid()) self.assertEqual(s.errors, {'decimal_field': ['Ensure that there are no more than 4 digits in total.']}) + + +class ChoiceFieldTests(TestCase): + """ + Tests for the ChoiceField options generator + """ + + SAMPLE_CHOICES = [ + ('red', 'Red'), + ('green', 'Green'), + ('blue', 'Blue'), + ] + + def test_choices_required(self): + """ + Make sure proper choices are rendered if field is required + """ + f = serializers.ChoiceField(required=True, choices=self.SAMPLE_CHOICES) + self.assertEqual(f.choices, self.SAMPLE_CHOICES) + + def test_choices_not_required(self): + """ + Make sure proper choices (plus blank) are rendered if the field isn't required + """ + f = serializers.ChoiceField(required=False, choices=self.SAMPLE_CHOICES) + self.assertEqual(f.choices, models.fields.BLANK_CHOICE_DASH + self.SAMPLE_CHOICES) diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 34acbaab4..c043f4175 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -1,4 +1,6 @@ from __future__ import unicode_literals +from django.db import models +from django.db.models.fields import BLANK_CHOICE_DASH from django.utils.datastructures import MultiValueDict from django.test import TestCase from rest_framework import serializers @@ -1001,6 +1003,73 @@ class SerializerPickleTests(TestCase): repr(pickle.loads(pickle.dumps(data, 0))) +# test for issue #725 +class SeveralChoicesModel(models.Model): + color = models.CharField( + max_length=10, + choices=[('red', 'Red'), ('green', 'Green'), ('blue', 'Blue')], + blank=False + ) + drink = models.CharField( + max_length=10, + choices=[('beer', 'Beer'), ('wine', 'Wine'), ('cider', 'Cider')], + blank=False, + default='beer' + ) + os = models.CharField( + max_length=10, + choices=[('linux', 'Linux'), ('osx', 'OSX'), ('windows', 'Windows')], + blank=True + ) + music_genre = models.CharField( + max_length=10, + choices=[('rock', 'Rock'), ('metal', 'Metal'), ('grunge', 'Grunge')], + blank=True, + default='metal' + ) + + +class SerializerChoiceFields(TestCase): + + def setUp(self): + super(SerializerChoiceFields, self).setUp() + + class SeveralChoicesSerializer(serializers.ModelSerializer): + class Meta: + model = SeveralChoicesModel + fields = ('color', 'drink', 'os', 'music_genre') + + self.several_choices_serializer = SeveralChoicesSerializer + + def test_choices_blank_false_not_default(self): + serializer = self.several_choices_serializer() + self.assertEqual( + serializer.fields['color'].choices, + [('red', 'Red'), ('green', 'Green'), ('blue', 'Blue')] + ) + + def test_choices_blank_false_with_default(self): + serializer = self.several_choices_serializer() + self.assertEqual( + serializer.fields['drink'].choices, + [('beer', 'Beer'), ('wine', 'Wine'), ('cider', 'Cider')] + ) + + def test_choices_blank_true_not_default(self): + serializer = self.several_choices_serializer() + self.assertEqual( + serializer.fields['os'].choices, + BLANK_CHOICE_DASH + [('linux', 'Linux'), ('osx', 'OSX'), ('windows', 'Windows')] + ) + + def test_choices_blank_true_with_default(self): + serializer = self.several_choices_serializer() + self.assertEqual( + serializer.fields['music_genre'].choices, + BLANK_CHOICE_DASH + [('rock', 'Rock'), ('metal', 'Metal'), ('grunge', 'Grunge')] + ) + + class DepthTest(TestCase): def test_implicit_nesting(self):