beda-software / drf-writable-nested

Writable nested model serializer for Django REST Framework
Other
1.07k stars 116 forks source link

Multipart PATCH requests cause m2m relation to be deleted #58

Open bjornpost opened 5 years ago

bjornpost commented 5 years ago

Hi,

Today I ran into this issue where the Post<>Author m2m relation was deleted when (partially) updating Post.title via a multipart form patch request. These are my findings so far:

My serializers (simplified):

class AuthorSerializer(serializers.ModelSerializer):
    class Meta:
        fields = ('id',)

class PostSerializer(WritableNestedModelSerializer):
    authors = AuthorSerializer(many=True)
    ...

    class Meta:
        fields = ('title', 'authors',)

When updating via the shell, all seems fine, authors relation is not removed:

>>> post = Post.objects.get(pk=9)
>>> serializer = PostSerializer(post, data={'title':'my title'}, partial=True)
>>> serializer.update(post, {'title':'my title'})
<Post: my title>
>>> serializer.is_valid()
True
>>> serializer.data
{'id': 9, 'title': 'my title', ... 'authors': [OrderedDict([('id', 3), ...])],}

But, when I'm updating the same object via a multi part form request, the Post<>Author relationship is removed on update:

curl --request PATCH \
  --url http://... \
  --form 'title=my title'

{
  "id": 9,
  "title": "my title",
  "authors": [], <---
  ...

Tested with djangorestframework==3.8.2, drf-writable-nested==0.5.1.

I'm happy to create a PR that fixes this issue, but I need some direction on how/where to fix this issue.

Related: #30

bjornpost commented 5 years ago

@ruscoder can you help me? :-)

ruscoder commented 5 years ago

Hello @bjornpost! Thanks for the contribution. I'll try to check your issue soon

ruscoder commented 5 years ago

@bjornpost I managed to reproduce the issue. So, it is a problem of DRF and multipart form data. For details, see https://github.com/encode/django-rest-framework/issues/2894.

When you send multipart patch request, QueryDict returns [] for authors even if you didn't send anything and drf-writable-nested clears all authors. It happens because we can't differentiate empty value (null) and empty array ([]) in multipart form data.

ruscoder commented 5 years ago

@bjornpost By the way, how do you create a new post with authors using multipart form data? As a workaround, I can recommend you create a new serailizer without WritableNestedModelSerializer and authors field, and use it for multipart post/put/patch requests. For convenience, you can change serializers on the fly in the method get_serializer_class of your viewset depending on content type.

bjornpost commented 5 years ago

Hi @ruscoder, thanks for your reply! The interesting thing here is that updating directly with partial=True on the serializer yields the expected response (ie. authors are kept as-is), while updating with the same payload via HTTP PATCH causes the relation to be deleted. Maybe something is not going as expected?

Anyway, I'll go with the custom serializer route for now to solve this specific issue.

Ps. I'm only updating over the API for now.

ruscoder commented 5 years ago

@bjornpost When you use the serializer directly it works because your final data does not contains ‘authors: []’. But when you use the api endpoint with multipart content type, the final data contains ‘authors: []’ after internal drf transformations.