From 3500b210386b70741e8b6724dea5d916ad5f0c8f Mon Sep 17 00:00:00 2001 From: Krzysztof Nazarewski Date: Wed, 9 Mar 2016 16:00:47 +0100 Subject: [PATCH 1/3] fixed PrimaryKeyRelatedField(required=False, ...) with missing data --- rest_framework/fields.py | 8 +++++++- rest_framework/relations.py | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 2a08e09ff..587ecc697 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -62,7 +62,7 @@ def is_simple_callable(obj): return len_args <= len_defaults -def get_attribute(instance, attrs): +def get_attribute(instance, attrs, required=True): """ Similar to Python's built in `getattr(instance, attr)`, but takes a list of nested attributes, instead of a single attribute. @@ -80,6 +80,12 @@ def get_attribute(instance, attrs): instance = getattr(instance, attr) except ObjectDoesNotExist: return None + # RelatedObjectDoesNotExist inherits from both ObjectDoesNotExist and + # AttributeError, so ObjectDoesNotExist has to come first + except (AttributeError, KeyError): + if not required: + raise SkipField + raise if is_simple_callable(instance): try: instance = instance() diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 572b69170..f695e79ba 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -154,7 +154,7 @@ class RelatedField(Field): pass # Standard case, return the object instance. - return get_attribute(instance, self.source_attrs) + return get_attribute(instance, self.source_attrs, self.required) @property def choices(self): From bee7b7a18206555d8b570d254401f52700d0275a Mon Sep 17 00:00:00 2001 From: Krzysztof Nazarewski Date: Fri, 11 Mar 2016 12:54:31 +0100 Subject: [PATCH 2/3] added ModelSerializer test failing with the same error --- tests/test_relations_pk.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_relations_pk.py b/tests/test_relations_pk.py index 169f7d9c5..470c51410 100644 --- a/tests/test_relations_pk.py +++ b/tests/test_relations_pk.py @@ -327,6 +327,23 @@ class PKForeignKeyTests(TestCase): serializer = NullableForeignKeySourceSerializer() self.assertEqual(serializer.data['target'], None) + def test_foreign_key_not_required(self): + """ + Let's say we wanted to fill the non-nullable model field inside + Model.save(), we would make it empty and not required. + """ + class ModelSerializer(ForeignKeySourceSerializer): + class Meta(ForeignKeySourceSerializer.Meta): + extra_kwargs = { + 'target': { + 'required': False, + 'allow_empty': True, + } + } + serializer = ModelSerializer(data={'name': 'test'}) + serializer.is_valid(raise_exception=True) + self.assertNotIn('target', serializer.data) + class PKNullableForeignKeyTests(TestCase): def setUp(self): From 45235e19cb5b9a200648da118324fbf8bb38c5d5 Mon Sep 17 00:00:00 2001 From: Krzysztof Nazarewski Date: Fri, 11 Mar 2016 14:09:00 +0100 Subject: [PATCH 3/3] removed everything but required from extra_kwargs --- tests/test_relations_pk.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/test_relations_pk.py b/tests/test_relations_pk.py index 470c51410..e9060d707 100644 --- a/tests/test_relations_pk.py +++ b/tests/test_relations_pk.py @@ -334,12 +334,7 @@ class PKForeignKeyTests(TestCase): """ class ModelSerializer(ForeignKeySourceSerializer): class Meta(ForeignKeySourceSerializer.Meta): - extra_kwargs = { - 'target': { - 'required': False, - 'allow_empty': True, - } - } + extra_kwargs = {'target': {'required': False}} serializer = ModelSerializer(data={'name': 'test'}) serializer.is_valid(raise_exception=True) self.assertNotIn('target', serializer.data)