NestedField breaks partial_update (PATCH request)

[TheUKDave]

I have a nested serializer in which I need to accept a pk for the nested model. So my serialzer looks something like this:

class AccountSerializer(DynamicFieldsMixin, serializers.ModelSerializer):
    client = NestedField(ClientSerializer, accept_pk=True)
    class Meta:
        model = Account
        fields = ('client', 'code')

So when I send a PATCH request to update the Account's code, I send this:

PUT /api/account/1/
{'code': 'Foo'}

Now despite this being a PATCH (not a PUT), and therefore required fields should be ignored (as per standard DRF behaviour), I get a 400 with the error: {"client": ["This field is required."]}

I tried making the field required=False, to see what would happen (despite this being incorrect, since it IS required for create/POST and update/PUT), I get a 400 with the error: {"client": ["This field may not be null."]}

Note: I am not send null, I'm just not sending the field at all.

For interest, I tried also making the field allow_null=True, and indeed, it goes all the way to the database with a null value for client, and explodes.

I think I've narrowed down the issue to: https://github.com/yezyilomo/django-restql/blob/380ce3238b55c6e0b05a045bf0597e41ca116ac1/django_restql/fields.py#L271-L272

I'm not sure why, if the field is empty and not required, that it sets it to "", and then in the next line to None. If the empty value is passed along, it validates correctly. So I'll make a PR for this, and hopefully this won't create any other quirks.

[TheUKDave]

Having made a very basic attempt to fix this, and seeing the tests that fail as a result, I see that the reason those lines are there, are to ensure that a nullable nested field are set to null when a PUT is performed on an object and that field is omitted.

I also see there are no tests for PATCHing/partial_updates. Are these just not supported? Is there any plan to support them?

Right now, without this, it makes using this package in our project a showstopper, which is a real shame 😟

[TheUKDave]

Have updated (and simplified) the code in my PR, all tests pass now, but I haven't added tests for patching. Let me know if you need this from me.

[yezyilomo]

I have a nested serializer in which I need to accept a pk for the nested model. So my serialzer looks something like this:

class AccountSerializer(DynamicFieldsMixin, serializers.ModelSerializer):
    client = NestedField(ClientSerializer, accept_pk=True)
    class Meta:
        model = Account
        fields = ('client', 'code')

So when I send a PATCH request to update the Account's code, I send this:

PUT /api/account/1/
{'code': 'Foo'}

Now despite this being a PATCH (not a PUT), and therefore required fields should be ignored (as per standard DRF behaviour), I get a 400 with the error: {"client": ["This field is required."]}

Thank you @TheUKDave for reporting this, looks like it's a bug, one which occurs because I didn't handle partial updates in NestedModelSerializer, specifically in to_internal_value method, I think here we should to ignore all empty values if the update is partial to prevent to_internal_value from sending them along to update.

[yezyilomo]

https://github.com/yezyilomo/django-restql/blob/380ce3238b55c6e0b05a045bf0597e41ca116ac1/django_restql/fields.py#L271-L272

I don't think if the problem is in here bcuz what's done here is simply check if the user passed nothing to the field, make None the value of that field meaning null to db. To the API client, empty string == None since you can pass None directly to a serializer but the client can't do that, so we chose empty string to represent None.

[yezyilomo]

I've got an idea which might work, I've tested it on 14 PATCH tests I've added, 12 passed and 2 failed, am trying to understand what's missing, so hang in there, it might get fixed soon.

[TheUKDave]

I've installed this latest version, and am trying to patch again, but still getting errors.

Following the example above with AccountSerializer nesting a ClientSerializer, I am trying to patch to update other attributes of the Account (completely omitting client in my request body), and getting the following error: ValueError: Cannot assign "<class 'rest_framework.fields.empty'>": "Account.client" must be a "Client" instance.

It seems that while the request does not have client in the JSON, the serializer's validated_data does, with a value of the rest_framework.fields.empty class.

I presume your updated tests covered this case, could I be doing something wrong?