Fix returning None when allow_none is True in CharField #1834
Conversation
| serializer = CharFieldSerializer(data={'char': None}) | ||
| self.assertTrue(serializer.fields['char'].allow_none) | ||
| self.assertTrue(serializer.is_valid()) | ||
| self.assertIsNone(serializer.data['char']) |
There was a problem hiding this comment.
I don't see what this assert does. You probably meant serializer.object['char'] ?
There was a problem hiding this comment.
Ok I've changed it to serializer.object.char
| if not self.allow_none: | ||
| return '' | ||
| else: | ||
| # return None implicity because smart_text(None) == 'None' |
There was a problem hiding this comment.
Actually, it's explicitly. BTW, could you add a reference to #1834 in the comment ?
piotrjakimiak
force-pushed
the
master
branch
from
e01c5d9 to
6022b9d
Compare
Sorry, do me me that the literal string |
|
Yes. Serialized value of None is string "None". During serialization of CharField when value is None and allow_none == True smart_text is called. |
|
@tomchristie PR looks fine to me, was about to merge it but got interrupted. Anything against this ? |
| class NullableCharFieldModel(models.Model): | ||
| char = models.CharField(null=True, blank=True, max_length=4) | ||
|
|
||
|
|
There was a problem hiding this comment.
Let's not use a model for this test - cleaner if we can narrow it down to a regular serializer.
|
One inline comment. Also I'm unclear if this is a regression or not. |
|
In version 2.3.14 None was returned, as in my commit. I've deleted model and used Serializer |
| class CharFieldSerializer(serializers.Serializer): | ||
| char = serializers.CharField(allow_none=True, required=False) | ||
| serializer = CharFieldSerializer(data={'char': None}) | ||
| self.assertTrue(serializer.fields['char'].allow_none) |
There was a problem hiding this comment.
We don't need this assert any more.
|
One minor inline comment left, otherwise looking great. |
|
Should I delete that inline comment? |
|
That assert line that checks the field attribute should be deleted - it's not really useful. |
|
Should i squash all commits into one? |
|
Your call - I don't mind too much either way. |
|
I hope everything is ok now. |
Fix returning None when allow_none is True in CharField
|
Nice work: thanks! |
When We have model with nullable char field allow_none is set to True.
Setting None in this field leads to serializing this field to "None" instead of null.