Django-rest-framework: Serializer .to_representation() doesn't call field.to_representation() for attributes of value None

[metaljoe]

Not sure if this is a bug/feature/other but I have a situation with an "is_deleted" field in my serializer that takes a source deleted_datetime field from the model. The field is set to NULL/None when the item is not deleted, and a datetime when deleted. The API shows is_deleted as True (datetime set) or False (when NULL).

However, the Serializer .to_representation only calls the field .to_representation when the attribute is not None. This means is_deleted comes back as either True or None.

I've had to override the get_attribute method in the field to call the field's parent get_attribute and then map accordingly, but this doesn't feel right as I'm doing the same work as the .to_representation method.

Am I missing the obvious? It's been a long week already! :-)

[tomchristie]

Normally we want to skip to_representation for the None case. Failing to do so means we end up having to deal with None as a special case in every implementation of to_representation, which users will invariable forget to deal with.

Any thoughts on what would be an appropriate resolution to this from your point of view?

[tomchristie]

I think this one sounds like a good usecase for SerializerMethodField()

is_deleted = SerializerMethodField()

def get_is_deleted(value):
    return False if (value is None) else True

Sound like a decent approach?

[metaljoe]

Aha! Thanks - that definitely looks like a better approach!

[maryokhin]

1,000th closed issue :tada:

[metaljoe]

Cool :-)

[tomchristie]

Yup. 1001 now. And a further 1232 pull requests.

Over slightly less that 4 years, thats 2233 / (365 * 4) ---> More than 1.5 closes per day, every day of the week for a solid 4 years.

That's a whole lotta work.

[maryokhin]

Want to take the chance on this famous issue to thank @tomchristie and all the maintainers and contributors for all their hard work for the past 4 years. Thank you for this awesome framework :)

[GonzaloRizzo]

I would like to reopen this issue since I think that it would be a nice feature to have and right now I'm migrating an old codebase to DRF and I have a field were None needs more parsing.

I think that a good solution is to have an attribute on Field that defaults to False and you can override it to force the call of to_representation

If you like the idea I'll start working on a PR

[xordoquy]

@GonzaloRizzo I don't think it's up to the serializer to do that work. I believe that's part of the business / service layer (could be merged within the model layer) to provide the informations.

[dz0]

What about BooleanField -- as opposed to NullBooleanField it kind of explicitly states that does not leave null/None, but will give it in serialization:

class S(Serializer):
    bf = BooleanField(required=False, read_only=True)

class example:
    bf = None  # I expect it to become **False**

s = S(instance=example)  
assert s.data['bf'] == False # Fails, because {'bf': None} 

For me it looks like a bug..

[tomchristie]

Don’t do that.

[dz0]

not intentionally, but subquery'ing, sth like

Person.objects.annotate(is_occupied=Subquery(
        Event.objects.
            .filter(person=OuterRef('pk'))
            .values('occupied')[:1],
        output_field=BooleanField()
    )
)

when none events found, gives None...

I can/will cast now, but docs could mention this behaviour (at least in "strict" BooleanField topic)
Thanks

[oxfn]

I also think this is unexpected behavior to skip to_representation for None values. None is also value like any other value, and skipping it breaks the idea of customized field. For example I have

class UserSerializer(serializers.ModelSerializer):
    country = CustomCountryField(Country, CountrySerializer)

and I want to use this serializer for read and write, so SerializerModelField does not fit this case as it does not support set_field_name. Besides, the main reason to customize this field in my case is to set fallback value {} on read if country is None.

At least this "optimization" can be made configurable like skip_empty=False.
I can implement this improvement if you guys agree with this idea