From d9633c6817e4c4a3085398c8a52293011670ef46 Mon Sep 17 00:00:00 2001 From: Piotr Jakimiak Date: Fri, 5 Sep 2014 11:08:34 +0200 Subject: [PATCH 1/5] Fix returning None when allow_none is True in CharField --- rest_framework/fields.py | 8 ++++++-- tests/test_fields.py | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 9d707c9b5..5955fa3f1 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -475,8 +475,12 @@ class CharField(WritableField): if isinstance(value, six.string_types): return value - if value is None and not self.allow_none: - return '' + if value is None: + if not self.allow_none: + return '' + else: + # return None implicity because smart_text(None) == 'None' + return None return smart_text(value) diff --git a/tests/test_fields.py b/tests/test_fields.py index 094ac1eb0..b8c9954ea 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -54,6 +54,10 @@ class ChoiceFieldModel(models.Model): choice = models.CharField(choices=SAMPLE_CHOICES, blank=True, max_length=255) +class NullableCharFieldModel(models.Model): + char = models.CharField(null=True, blank=True, max_length=4) + + class ChoiceFieldModelSerializer(serializers.ModelSerializer): class Meta: model = ChoiceFieldModel @@ -1004,6 +1008,20 @@ class BooleanField(TestCase): self.assertFalse(BooleanRequiredSerializer(data={}).is_valid()) +class ModelCharField(TestCase): + """ + Tests for CharField + """ + def test_none_serializing(self): + class CharFieldSerializer(serializers.ModelSerializer): + class Meta: + model = NullableCharFieldModel + serializer = CharFieldSerializer(data={'char': None}) + self.assertTrue(serializer.fields['char'].allow_none) + self.assertTrue(serializer.is_valid()) + self.assertIsNone(serializer.data['char']) + + class SerializerMethodFieldTest(TestCase): """ Tests for the SerializerMethodField field_to_native() behavior From e39422e35fa81c79c149570e3d7e5cf9e0a0909b Mon Sep 17 00:00:00 2001 From: Piotr Jakimiak Date: Fri, 5 Sep 2014 11:27:36 +0200 Subject: [PATCH 2/5] Check object in tests --- tests/test_fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_fields.py b/tests/test_fields.py index b8c9954ea..20d5a97a1 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -1019,7 +1019,7 @@ class ModelCharField(TestCase): serializer = CharFieldSerializer(data={'char': None}) self.assertTrue(serializer.fields['char'].allow_none) self.assertTrue(serializer.is_valid()) - self.assertIsNone(serializer.data['char']) + self.assertIsNone(serializer.object.char) class SerializerMethodFieldTest(TestCase): From 6022b9ddd44fff08435fa2dbe50d165100ed61c1 Mon Sep 17 00:00:00 2001 From: Piotr Jakimiak Date: Fri, 5 Sep 2014 11:56:44 +0200 Subject: [PATCH 3/5] Fix comment --- rest_framework/fields.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 5955fa3f1..e744b1738 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -479,7 +479,8 @@ class CharField(WritableField): if not self.allow_none: return '' else: - # return None implicity because smart_text(None) == 'None' + # return None explicitly because smart_text(None) == 'None' + # see #1834 for details return None return smart_text(value) From c3b841ae449c66102be7305416eca8acb22c8c42 Mon Sep 17 00:00:00 2001 From: Piotr Jakimiak Date: Fri, 5 Sep 2014 14:08:11 +0200 Subject: [PATCH 4/5] Use Serializer instead of ModelSerializer --- rest_framework/fields.py | 3 +-- tests/test_fields.py | 11 +++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index e744b1738..8e15345da 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -479,8 +479,7 @@ class CharField(WritableField): if not self.allow_none: return '' else: - # return None explicitly because smart_text(None) == 'None' - # see #1834 for details + # Return None explicitly because smart_text(None) == 'None'. See #1834 for details return None return smart_text(value) diff --git a/tests/test_fields.py b/tests/test_fields.py index 20d5a97a1..8bf9c56a4 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -54,10 +54,6 @@ class ChoiceFieldModel(models.Model): choice = models.CharField(choices=SAMPLE_CHOICES, blank=True, max_length=255) -class NullableCharFieldModel(models.Model): - char = models.CharField(null=True, blank=True, max_length=4) - - class ChoiceFieldModelSerializer(serializers.ModelSerializer): class Meta: model = ChoiceFieldModel @@ -1013,13 +1009,12 @@ class ModelCharField(TestCase): Tests for CharField """ def test_none_serializing(self): - class CharFieldSerializer(serializers.ModelSerializer): - class Meta: - model = NullableCharFieldModel + class CharFieldSerializer(serializers.Serializer): + char = serializers.CharField(allow_none=True, required=False) serializer = CharFieldSerializer(data={'char': None}) self.assertTrue(serializer.fields['char'].allow_none) self.assertTrue(serializer.is_valid()) - self.assertIsNone(serializer.object.char) + self.assertIsNone(serializer.object['char']) class SerializerMethodFieldTest(TestCase): From cb3cc00edd4a7a7e8b583a9ba3a823fb6ca76346 Mon Sep 17 00:00:00 2001 From: Piotr Jakimiak Date: Fri, 5 Sep 2014 14:36:35 +0200 Subject: [PATCH 5/5] Delete useless assert --- tests/test_fields.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_fields.py b/tests/test_fields.py index 8bf9c56a4..0ddbe48b5 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -1012,7 +1012,6 @@ class ModelCharField(TestCase): class CharFieldSerializer(serializers.Serializer): char = serializers.CharField(allow_none=True, required=False) serializer = CharFieldSerializer(data={'char': None}) - self.assertTrue(serializer.fields['char'].allow_none) self.assertTrue(serializer.is_valid()) self.assertIsNone(serializer.object['char'])