From 8c3f82fb18a58b8e0983612ef3cc35b3c3950b66 Mon Sep 17 00:00:00 2001 From: Susan Dreher Date: Tue, 27 Jan 2015 16:18:51 -0500 Subject: [PATCH 1/3] :bug: ManyRelatedField get_value clearing field on partial update A PATCH to a serializer's non-related CharField was clearing an ancillary StringRelatedField(many=True) field. The issue appears to be in the ManyRelatedField's get_value method, which was returning a [] instead of empty when the request data was a MultiDict. This fix mirrors code in fields.py, class Field, get_value, Ln. 272, which explicitly returns empty on a partial update. Tests added to demonstrate the issue. --- rest_framework/relations.py | 5 +++++ tests/test_relations.py | 44 +++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index aa0c2defe..13793f375 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -338,7 +338,12 @@ class ManyRelatedField(Field): # We override the default field access in order to support # lists in HTML forms. if html.is_html_input(dictionary): + # Don't return [] if the update is partial + if self.field_name not in dictionary: + if getattr(self.root, 'partial', False): + return empty return dictionary.getlist(self.field_name) + return dictionary.get(self.field_name, empty) def to_internal_value(self, data): diff --git a/tests/test_relations.py b/tests/test_relations.py index 62353dc25..143e835ca 100644 --- a/tests/test_relations.py +++ b/tests/test_relations.py @@ -1,9 +1,14 @@ -from .utils import mock_reverse, fail_reverse, BadType, MockObject, MockQueryset -from django.core.exceptions import ImproperlyConfigured -from rest_framework import serializers -from rest_framework.test import APISimpleTestCase import pytest +from django.core.exceptions import ImproperlyConfigured +from django.utils.datastructures import MultiValueDict + +from rest_framework import serializers +from rest_framework.fields import empty +from rest_framework.test import APISimpleTestCase + +from .utils import mock_reverse, fail_reverse, BadType, MockObject, MockQueryset + class TestStringRelatedField(APISimpleTestCase): def setUp(self): @@ -134,3 +139,34 @@ class TestSlugRelatedField(APISimpleTestCase): def test_representation(self): representation = self.field.to_representation(self.instance) assert representation == self.instance.name + + +class TestManyRelatedField(APISimpleTestCase): + def setUp(self): + self.instance = MockObject(pk=1, name='foo') + self.field = serializers.StringRelatedField(many=True) + self.field.field_name = 'foo' + + def test_get_value_regular_dictionary_full(self): + assert 'bar' == self.field.get_value({'foo': 'bar'}) + assert empty == self.field.get_value({'baz': 'bar'}) + + def test_get_value_regular_dictionary_partial(self): + setattr(self.field.root, 'partial', True) + assert 'bar' == self.field.get_value({'foo': 'bar'}) + assert empty == self.field.get_value({'baz': 'bar'}) + + def test_get_value_multi_dictionary_full(self): + mvd = MultiValueDict({'foo': ['bar1', 'bar2']}) + assert ['bar1', 'bar2'] == self.field.get_value(mvd) + + mvd = MultiValueDict({'baz': ['bar1', 'bar2']}) + assert [] == self.field.get_value(mvd) + + def test_get_value_multi_dictionary_partial(self): + setattr(self.field.root, 'partial', True) + mvd = MultiValueDict({'foo': ['bar1', 'bar2']}) + assert ['bar1', 'bar2'] == self.field.get_value(mvd) + + mvd = MultiValueDict({'baz': ['bar1', 'bar2']}) + assert empty == self.field.get_value(mvd) From 1714ceae9f468bc1479f0d7a32b0bf26ae9cf15f Mon Sep 17 00:00:00 2001 From: Susan Dreher Date: Tue, 27 Jan 2015 16:31:25 -0500 Subject: [PATCH 2/3] reorganize imports --- tests/test_relations.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_relations.py b/tests/test_relations.py index 143e835ca..67f49c6b0 100644 --- a/tests/test_relations.py +++ b/tests/test_relations.py @@ -1,13 +1,13 @@ -import pytest +from .utils import mock_reverse, fail_reverse, BadType, MockObject, MockQueryset from django.core.exceptions import ImproperlyConfigured from django.utils.datastructures import MultiValueDict from rest_framework import serializers from rest_framework.fields import empty -from rest_framework.test import APISimpleTestCase -from .utils import mock_reverse, fail_reverse, BadType, MockObject, MockQueryset +from rest_framework.test import APISimpleTestCase +import pytest class TestStringRelatedField(APISimpleTestCase): From e7da266a866adddd5c37453fab33812ee412752b Mon Sep 17 00:00:00 2001 From: Susan Dreher Date: Tue, 27 Jan 2015 16:32:15 -0500 Subject: [PATCH 3/3] reorganize imports --- tests/test_relations.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_relations.py b/tests/test_relations.py index 67f49c6b0..d478d8550 100644 --- a/tests/test_relations.py +++ b/tests/test_relations.py @@ -1,11 +1,8 @@ from .utils import mock_reverse, fail_reverse, BadType, MockObject, MockQueryset - from django.core.exceptions import ImproperlyConfigured from django.utils.datastructures import MultiValueDict - from rest_framework import serializers from rest_framework.fields import empty - from rest_framework.test import APISimpleTestCase import pytest