PATCHing does not work with NestedFields, when the NestedField is omitted

[TheUKDave]

I've installed this latest version that was to resolve this issue, and am trying to patch again, but still getting (new) errors.

Following the example in the original issue 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?

Originally posted by @TheUKDave in https://github.com/yezyilomo/django-restql/issues/143#issuecomment-617774153

[TheUKDave]

FYI - Django 2.2.12 & Python 3.7.6

[yezyilomo]

This test should be covering your case https://github.com/yezyilomo/django-restql/blob/9430fe39267c797bce95a0c70ee8f12e5fb07cc7/tests/test_views.py#L1702-L1727

Its serializer looks like

https://github.com/yezyilomo/django-restql/blob/9430fe39267c797bce95a0c70ee8f12e5fb07cc7/tests/testapp/serializers.py#L116-L123

[yezyilomo]

FYI - Django 2.2.12 & Python 3.7.6

This shouldn't be a problem since the library is tested against the versions which you are using too, Here is the line which includes your versions https://github.com/yezyilomo/django-restql/blob/9430fe39267c797bce95a0c70ee8f12e5fb07cc7/tox.ini#L6

[TheUKDave]

Oh OK, so presumably a test with a serializer that is required=True (which mine is for POST/create operations) and/or allow_null=False (because in my case, it cannot be null at the DB level) would presumably fail in the same way as mine.

I'll try modifying my serializer now (which I can't leave like that for the other operations) just to see if that makes the patch work.

[TheUKDave]

OK, scratch the above, even with my serializer set up as in your test, I still get the same error :-/

I don't suppose you have any clue what's going wrong? Should I get a 'client': empty in my validated_data?

[yezyilomo]

OK, scratch the above, even with my serializer set up as in your test, I still get the same error :-/

I don't suppose you have any clue what's going wrong? Should I get a 'client': empty in my validated_data?

Yeah but it should be filtered out in here https://github.com/yezyilomo/django-restql/blob/9430fe39267c797bce95a0c70ee8f12e5fb07cc7/django_restql/mixins.py#L474-L477

[yezyilomo]

Wait did you inherit NestedModelSerializer? cuz u didn't do that in the original example you posted here https://github.com/yezyilomo/django-restql/issues/143#issue-600248944.

[TheUKDave]

OK wow, sorry, you're spot on, no I was not inheriting NestedModelSerializer, that's now resolved - so thanks.

However, I notice that despite inheriting from both that and the DynamicFieldsMixin, the response to the PATCH does not respect the query in the url args, I get ALL fields for the object, instead of just what I ask for. Is that intentional?

[yezyilomo]

Currently the library handle fields filtering on GET requests only, it's mentioned here https://django-restql.yezyilomo.com/#getting-started, this is because DynamicFieldsMixin changes fields when instantiating a serializer, this means it might remove a field which needs to be updated before update is done, this was causing problems so I removed it, here is the PR made to fix that https://github.com/yezyilomo/django-restql/pull/43/files

[TheUKDave]

Ah. OK Understood. Some of my serializers have large and/or calculated values in them, that you only want to return in certain circumstances. So this too may end up being a showstopper :(

But thanks for your help!

If I understand correctly, if I were to fork and reverse that merge, so long as my query param contains at least the fields I wish to PATCH, then it should work as expected, and return the fields I ask for. Correct?

Failing that, perhaps a bodge would be to have different serializers used by the viewset, per action. But that's pretty ugly and not very maintainable.

Do you plan to attempt to update it at some point to remove the limitation?

[yezyilomo]

Actually I have been having this idea to change fields in serializer's to_representation and not in __init__ so that we don't affect any thing before calling to_representation which includes create and update operations, with this I think we could be able to send POST, PATCH and PUT with query param and get exactly what we want. I have never tested this idea but now that you have mentioned it I think I should give it a try.

[yezyilomo]

Actually I have been having this idea to change fields in serializer's to_representation and not in __init__ so that we don't affect any thing before calling to_representation which includes create and update operations, with this I think we could be able to send POST, PATCH and PUT with query param and get exactly what we want. I have never tested this idea but now that you have mentioned it I think I should give it a try.

Wow this worked perfectly, I have created an issue for this here #149

[TheUKDave]

Amazing, I look forward to it. Thanks!

[yezyilomo]

It's done #152