Alter read_only+default behaviour by carltongibson · Pull Request #5886 · encode/django-rest-framework (original) (raw)

Since ≈forever read_only fields with a default have set that default in validated_data on create and update operations.

This has led to issues as we have tried to Fix default value handling for dotted sources #5375: uses such as source=a.b.c, default='x' where e.g. b may be null were returning None, rather than the default x.

#5375 resolved the serialisation issues with dotted source — default will be used if any step in the chain is None — but revealed a latent issue for deserialisation when using read_only plus a default. This comes up in a couple of ways:

• The check for nested writes errors on a read_only field with dotted source and default. _writable_fields in Serializer retrieves read only fields #4666. (Exact reproduce pending on this one.)
• Skipping that, validated_data ends up including a nested dict, which cannot be assigned in e.g. places where a model object is expected. not required read_only Fields with allow_null #5708 (comment)

Both of these cases come up because the default means read_only fields end up in _writable_fields.

Whilst we've been doing it ≈forever, it's still a Que? here. How on earth is a read_only field writable?

This PR adjusts the _writable_fields check to always exclude read_only fields.

This is a backwards incompatible change.

The previous use-case was for e.g. setting the current user as the owner of a object on create.

This change will require an extra workaround — to explicitly add the read_only fields with a writable default to validated_data, probably by overriding save.

class MySerializer(serializers.ModelSerializer): user = serializers.PrimaryKeyRelatedField(read_only=True, default=CreateOnlyDefault(CurrentUserDefault())) ...

def save(self, **kwargs):
    """Include default for read_only `user` field"""
    kwargs["user"] = self.fields["user"].get_default()
    return super().save(**kwargs)

Possibly we could add some convenience to this? (I'm a bit loathe to add API here.)

Note that in the tutorial we demonstrate this kind of this at the view level:

def perform_create(self, serializer):
    serializer.save(owner=self.request.user)

My overall take here is that the previous use-case saves a line or two, and is a convenience, but it inhibits correctness with the dotted source issue, so I think we need to swallow the pill on this one and make the change.

TODO:

• Release notes for BC change and workaround.
• Docs changes