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

PATCH with no data is updating ListField values on ModelSerializer instead of leaving them alone #2761

Closed
picturedots opened this issue Mar 24, 2015 · 4 comments
Labels
Milestone

Comments

@picturedots
Copy link

picturedots commented Mar 24, 2015

I've got a model serializer with two ListFields and an email field in it, something like this

class MySettingsSerializer(ModelSerializer):
    location_codes = serializers.ListField(child=serializers.CharField(), 
        required=False)
    permissions = serializers.ListField(child=serializers.CharField(), 
        required=False )

    class Meta:
        model = MySettings
        fields = ('id', 'email', 'location_codes', 'permissions' )

I've also got a view class that uses UpdateModelMixin to provide an update method.

When I do a PATCH to to my view, the expected behavior is that only the values that are sent will be changed. For example, PATCH with
{"permissions": [ "change_mymodel" ] }
should change the permissions field on the MySettings object, and not change any of the other fields. This works as expected.

However, if I do a PATCH with no data at all, both the permissions and locations_codes are set to empty lists. The other field (email) is not changed, which leads me to believe that there is bug related to identifying if a ListField should be updated during a PATCH.

Note that this issue cannot be triggered by the HTML form interface by PATCHING a {} -- I can only see it by using a client like Postman and doing a PATCH with no content.

@tomchristie
Copy link
Member

tomchristie commented Apr 17, 2015

Sounds valid - it'd be good to pull together a more minimal example case that uses a regular serializer rather than a model serializer - that'd help us progress the issue.

@xordoquy xordoquy modified the milestone: 3.1.4 Release Jul 8, 2015
@xordoquy xordoquy modified the milestones: 3.1.4 Release, 3.1.5 Release Jul 16, 2015
@tomchristie tomchristie modified the milestones: 3.1.5 Release, 3.2.1 Release Jul 30, 2015
@tomchristie tomchristie modified the milestone: 3.2.1 Release Aug 7, 2015
@adamsc64
Copy link
Contributor

adamsc64 commented Sep 10, 2015

Hello from DjangoCon sprints. Working on proof of concept solution for this issue. Should have a PR to you in a few days.

@xordoquy
Copy link
Collaborator

xordoquy commented Sep 10, 2015

@adamsc64 Welcome onboard ! It would be wonderfull to start with a failing test case first in order to prove the issue and make sure the fix actually fixes the issue.

@xordoquy xordoquy added this to the 3.2.4 Release milestone Sep 10, 2015
adamsc64 added a commit to adamsc64/django-rest-framework that referenced this issue Sep 13, 2015
- When not submitting key for list fields or multiple choice, partial
  serialization should result in empty state (key not there), not an
  empty list.
adamsc64 added a commit to adamsc64/django-rest-framework that referenced this issue Sep 13, 2015
- Checked ``partial`` state immediately; return ``empty`` if key not submitted.
- Previously, partial serialization for updates, such as in the case of HTTP
  PATCH requests, would truncate any old values for ListField. This is because
  it had relied on the default value of an empty list. But an empty list has
  different functionality than the sentinal value of ``empty``.
adamsc64 added a commit to adamsc64/django-rest-framework that referenced this issue Sep 13, 2015
- When not submitting key for list fields or multiple choice, partial
  serialization should result in empty state (key not there), not an
  empty list.
adamsc64 added a commit to adamsc64/django-rest-framework that referenced this issue Sep 13, 2015
- Checked ``partial`` state immediately; return ``empty`` if key not submitted.
- Previously, partial serialization for updates, such as in the case of HTTP
  PATCH requests, would truncate any old values for ListField. This is because
  it had relied on the default value of an empty list. But an empty list has
  different functionality than the sentinal value of ``empty``.
adamsc64 added a commit to adamsc64/django-rest-framework that referenced this issue Sep 19, 2015
- Checked ``partial`` state when getting value in appropriate field
  classes; return ``empty`` immediately if key not submitted.
adamsc64 added a commit to adamsc64/django-rest-framework that referenced this issue Sep 19, 2015
- Checked ``partial`` state when getting value in appropriate field
  classes; return ``empty`` immediately if key not submitted.
adamsc64 added a commit to adamsc64/django-rest-framework that referenced this issue Sep 19, 2015
- When not submitting key for list fields or multiple choice, partial
  serialization should result in empty state (key not there), not an
  empty list.
@adamsc64
Copy link
Contributor

adamsc64 commented Sep 19, 2015

@xordoquy @tomchristie: I've submitted PR #3415 linked above. Let me know what you think or what else you need from me!

@xordoquy xordoquy modified the milestones: 3.2.5 Release, 3.2.4 Release Sep 21, 2015
xordoquy added a commit that referenced this issue Sep 21, 2015
Fixed #2761 - ListField truncation on HTTP PATCH
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants