Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PUT/PATCH support is broken #51

Open
JDougherty opened this issue Aug 17, 2016 · 3 comments
Open

PUT/PATCH support is broken #51

JDougherty opened this issue Aug 17, 2016 · 3 comments

Comments

@JDougherty
Copy link

JDougherty commented Aug 17, 2016

I've tried creating a very simple API endpoint following the README:

from django.db import models

from rest_framework import serializers
from rest_framework_bulk import BulkListSerializer, BulkSerializerMixin, ListBulkCreateUpdateAPIView


class Foo(models.Model):
    name = models.CharField(max_length=20)
    class Meta:
        app_label = "foobar"


class FooSerializer(BulkSerializerMixin, serializers.ModelSerializer):
    class Meta(object):
        model = Foo
        list_serializer_class = BulkListSerializer

class FooView(ListBulkCreateUpdateAPIView):
    queryset = Foo.objects.all()
    serializer_class = FooSerializer

I've configured a URL for FooView and am able to load the built in rest_framework html page for this API endpoint. I've also created a few instances of the Foo model for testing.

However, when I attempt to make an HTTP PUT/PATCH request to this endpoint to update one of the existing instances, I get a successful return code but no update actually occurs.

curl 'http://127.0.0.1:8000/foo/' -X PUT -H 'Cookie: csrftoken=M8jkbJtUKwjKB7zya8JYvxqJVxkRaPgG' -H 'X-CSRFToken: M8jkbJtUKwjKB7zya8JYvxqJVxkRaPgG' --data 'id=1&name=D' --compressed

The response is simply an empty list [] and the instance with id=1 retains its old name.

I've created a sample project to reproduce this, and would really appreciate any help. I am unable to determine if this is a bug in DRF, DRF-bulk, or a misunderstanding on my part.

[Update]
I tried testing a POST request, like so, and it did create a new object:

curl 'http://127.0.0.1:8000/foo/' -X POST -H 'Cookie: csrftoken=M8jkbJtUKwjKB7zya8JYvxqJVxkRaPgG' -H 'X-CSRFToken: M8jkbJtUKwjKB7zya8JYvxqJVxkRaPgG' --data 'name=D' --compressed

The response for this request is: {"id":4,"name":"D"}

However, trying to create a new object with PUT does not work:

curl 'http://127.0.0.1:8000/foo/' -X PUT -H 'Cookie: csrftoken=M8jkbJtUKwjKB7zya8JYvxqJVxkRaPgG' -H 'X-CSRFToken: M8jkbJtUKwjKB7zya8JYvxqJVxkRaPgG' --data 'name=E'

The response for this is: []

I believe that PUT/PATCH support for django_rest_framework-bulk is broken at this point. Python package info is:

Django==1.8
djangorestframework==3.4.4
djangorestframework-bulk==0.2.1
@JDougherty JDougherty changed the title Unable to get django-rest-framework-bulk to update instances PUT/PATCH support is broken Aug 17, 2016
@melinath
Copy link

maybe also caused by the confusing readme, like #55?

@bennullgraham
Copy link

bennullgraham commented Dec 13, 2016

Edit: my problem was unrelated. DRF-bulk doesn't seem to work with a hyperlinked API out of the box, this was the cause. Original post quoted below.

Did you gents get anywhere with this?

I'm knee-deep debugging a problem with the same (?) symptoms. I see if the validated data handed to the BulkListSerializer doesn't contain the pk field, then an empty validation error is raised, eventually appearing in the response as a list with an empty string: ['']

# rest_framework_bulk/drf3/serializers.py:47
if not all((bool(i) and not inspect.isclass(i)
            for i in all_validated_data_by_id.keys())):
    raise ValidationError('')

In my situation, that is the case; during PATCH the pk field (id for me) is not present. I know DRF-bulk takes special care to deal with that situation:

[...] To deal with that, you must use BulkSerializerMixin mixin in your serializer class which will add the model primary key field back to the validated_data.

I will be looking into what might be going wrong with that mixin.

@pkainz
Copy link

pkainz commented Feb 2, 2017

The data for each instance that should be updated must be a separate json dict in the payload. Even if you apply a filter to the queryset, only the instances that are explicitly identified in the payload receive the update.
I worked this out for my use case of bulk updating all id-filtered resources with the same value as follows:

PATCH http://api.example.com/resource/?id__in=0,1,2,3,4 HTTP/1.1
Content-Type: 'application/json'
# prepare a dict payload for each object
data = [{
    'id': id__,
    'value': 10000     
} for id__ in id_filter]

However, an empty ValidationError is not sufficiently expressive for the underlying problem, please fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants