Partial update (PATCH) should return new values after successful save

[hadalin]

When I try to use use partial update, the values are saved correctly but response has old values.

I think the problem is somewhere in UpdateModelMixin.

[tomchristie]

You gotta give us a hand here? Replicable example?

[hadalin]

Question.

Should the serializer.data return updated data on this LoC or this one? If it's the former, then I can write a unit test (something of this manner) otherwise you have to point me to a unit test I can use or I can write a sample Django project and push it to GitHub.

[tomchristie]

serializer.data is accessed after is_valid is called.

We don't necessarily need to see a failing test case, just some more information that demonstrates a failing example. As it stand 'PATCH' is broken isn't something I'm going to spend any time looking at unless I've got more confidence that there's actually a case for me to follow up here. (Ideally demonstrate the issue at the level of the serializer API rather than having to use a who view to replicate.)

[hadalin]

So the example is this.

serializer

class SettingsSerializer(serializers.ModelSerializer):
    class Meta:
        model = Settings
        fields = ('id', 'key', 'value', 'description')
        read_only_fields = ('key', 'description')

    def validate_value(self, value):
        ...

viewset

class SettingsViewSet(viewsets.ModelViewSet):
    model = Settings
    serializer_class = SettingsSerializer
    queryset = Settings.objects.all()

urls.py

settings_detail = SettingsViewSet.as_view({
    'get': 'retrieve',
    'patch': 'partial_update',
})

When I make a patch request to update the value, the value gets saved, but the response has the old value.

[tomchristie]

Created a project to test this, but doesn't replicate for me.

$ pip freeze | grep djangorestframework
djangorestframework==3.0.1
$ http PATCH 127.0.0.1:8000 value=new
HTTP/1.0 200 OK
Allow: GET, PATCH, HEAD, OPTIONS
Content-Type: application/json
Date: Tue, 16 Dec 2014 20:22:27 GMT
Server: WSGIServer/0.1 Python/2.7.2
Vary: Accept, Cookie
X-Frame-Options: SAMEORIGIN

{
    "description": "foo", 
    "id": 1, 
    "key": "k1", 
    "value": "new"
}

Would really need to see something like a pull request for a failing example - seem more feasible that it's some kind of usage issue?

[hadalin]

Huh, maybe, but spent quite some time with this and it always returned the old value. I will try to isolate the problem.

[hadalin]

The problem occurred when I updated to 3. from 2.

[tomchristie]

Best thing would prob be to try writing up a failing test case. Alternatively if you throw up a repo that demonstrates the issue then we could spend more time looking at whatever issue your having.

[carltongibson]

@hadalin — You must be accessing serializer.data somewhere (in custom validation maybe???). If you can show where we might be able to point you to a different approach.

[hadalin]

Yes, in validate_value I fetch regex value of the setting which is used to check a value against regex

regex = Settings.objects.get(pk=self.data['id']).regex

[tomchristie]

Nicely dealt with @carltongibson.

Yes, in validate_value

That's your problem there. You shouldn't (don't need to) be accessing .data in validate_value method.
If you want to validate some data across more than one field then use the .validate() method instead...

def validate(self, data):
    regex = Settings.objects.get(pk=data['id']).regex 
    value = data['value']
    if not re.match(regex, value):
       raise serializers.ValidationError('...')

If you want to mark the error as being against value rather than being a "non field error" you can raise the validation error like so instead...

raise serializers.ValidationError({'value': '...'})

[carltongibson]

OK — that's going to be a "Don't hold it that way". The reason it's okay in v2.x is that we explicitly worked round this in #1117.

You could use initial_data here I think... — or do what Tom said :)

[tomchristie]

See also #2276.

[tomchristie]

Opened #2289 to improve the constraints around this - eg raise a helpful error when this is attempted.

[hadalin]

Thanks.