From 8d0a91b002d7d18a1526e80e9cea0702e18be90e Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 13 Oct 2016 12:43:43 +0100 Subject: [PATCH] Fix 3674 (#4571) Handle ModelSerializer case for relationships to models with custom pk. --- rest_framework/serializers.py | 2 +- rest_framework/utils/field_mapping.py | 2 +- rest_framework/utils/model_meta.py | 15 ++++-- tests/test_model_serializer.py | 68 +++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index f13c92a44..098c3cd23 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -1167,7 +1167,7 @@ class ModelSerializer(Serializer): field_kwargs = get_relation_kwargs(field_name, relation_info) to_field = field_kwargs.pop('to_field', None) - if to_field and not relation_info.related_model._meta.get_field(to_field).primary_key: + if to_field and not relation_info.reverse and not relation_info.related_model._meta.get_field(to_field).primary_key: field_kwargs['slug_field'] = to_field field_class = self.serializer_related_to_field diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index 64df9a08e..29005f6b7 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -238,7 +238,7 @@ def get_relation_kwargs(field_name, relation_info): """ Creates a default instance of a flat relational field. """ - model_field, related_model, to_many, to_field, has_through_model = relation_info + model_field, related_model, to_many, to_field, has_through_model, reverse = relation_info kwargs = { 'queryset': related_model._default_manager, 'view_name': get_detail_view_name(related_model) diff --git a/rest_framework/utils/model_meta.py b/rest_framework/utils/model_meta.py index 94aa46e72..3e3e434e6 100644 --- a/rest_framework/utils/model_meta.py +++ b/rest_framework/utils/model_meta.py @@ -23,7 +23,8 @@ RelationInfo = namedtuple('RelationInfo', [ 'related_model', 'to_many', 'to_field', - 'has_through_model' + 'has_through_model', + 'reverse' ]) @@ -81,7 +82,8 @@ def _get_forward_relationships(opts): related_model=get_related_model(field), to_many=False, to_field=_get_to_field(field), - has_through_model=False + has_through_model=False, + reverse=False ) # Deal with forward many-to-many relationships. @@ -94,7 +96,8 @@ def _get_forward_relationships(opts): to_field=None, has_through_model=( not get_remote_field(field).through._meta.auto_created - ) + ), + reverse=False ) return forward_relations @@ -118,7 +121,8 @@ def _get_reverse_relationships(opts): related_model=related, to_many=get_remote_field(relation.field).multiple, to_field=_get_to_field(relation.field), - has_through_model=False + has_through_model=False, + reverse=True ) # Deal with reverse many-to-many relationships. @@ -135,7 +139,8 @@ def _get_reverse_relationships(opts): has_through_model=( (getattr(get_remote_field(relation.field), 'through', None) is not None) and not get_remote_field(relation.field).through._meta.auto_created - ) + ), + reverse=True ) return reverse_relations diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index cd9b2dfc3..06848302f 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -89,6 +89,15 @@ class ChoicesModel(models.Model): choices_field_with_nonstandard_args = models.DecimalField(max_digits=3, decimal_places=1, choices=DECIMAL_CHOICES, verbose_name='A label') +class Issue3674ParentModel(models.Model): + title = models.CharField(max_length=64) + + +class Issue3674ChildModel(models.Model): + parent = models.ForeignKey(Issue3674ParentModel, related_name='children') + value = models.CharField(primary_key=True, max_length=64) + + class TestModelSerializer(TestCase): def test_create_method(self): class TestSerializer(serializers.ModelSerializer): @@ -996,3 +1005,62 @@ class TestUniquenessOverride(TestCase): fields = TestSerializer().fields self.assertFalse(fields['field_1'].required) self.assertTrue(fields['field_2'].required) + + +class Issue3674Test(TestCase): + def test_nonPK_foreignkey_model_serializer(self): + class TestParentModel(models.Model): + title = models.CharField(max_length=64) + + class TestChildModel(models.Model): + parent = models.ForeignKey(TestParentModel, related_name='children') + value = models.CharField(primary_key=True, max_length=64) + + class TestChildModelSerializer(serializers.ModelSerializer): + class Meta: + model = TestChildModel + fields = ('value', 'parent') + + class TestParentModelSerializer(serializers.ModelSerializer): + class Meta: + model = TestParentModel + fields = ('id', 'title', 'children') + + parent_expected = dedent(""" + TestParentModelSerializer(): + id = IntegerField(label='ID', read_only=True) + title = CharField(max_length=64) + children = PrimaryKeyRelatedField(many=True, queryset=TestChildModel.objects.all()) + """) + self.assertEqual(unicode_repr(TestParentModelSerializer()), parent_expected) + + child_expected = dedent(""" + TestChildModelSerializer(): + value = CharField(max_length=64, validators=[]) + parent = PrimaryKeyRelatedField(queryset=TestParentModel.objects.all()) + """) + self.assertEqual(unicode_repr(TestChildModelSerializer()), child_expected) + + def test_nonID_PK_foreignkey_model_serializer(self): + + class TestChildModelSerializer(serializers.ModelSerializer): + class Meta: + model = Issue3674ChildModel + fields = ('value', 'parent') + + class TestParentModelSerializer(serializers.ModelSerializer): + class Meta: + model = Issue3674ParentModel + fields = ('id', 'title', 'children') + + parent = Issue3674ParentModel.objects.create(title='abc') + child = Issue3674ChildModel.objects.create(value='def', parent=parent) + + parent_serializer = TestParentModelSerializer(parent) + child_serializer = TestChildModelSerializer(child) + + parent_expected = {'children': ['def'], 'id': 1, 'title': 'abc'} + self.assertEqual(parent_serializer.data, parent_expected) + + child_expected = {'parent': 1, 'value': 'def'} + self.assertEqual(child_serializer.data, child_expected)