Fixed #3674 -- Refactored _get_reverse_relationships() to use correct to_field.

rest_framework/serializers.py

@@ -1131,9 +1131,14 @@ def build_relational_field(self, field_name, relation_info):
         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:
-            field_kwargs['slug_field'] = to_field
-            field_class = self.serializer_related_to_field
+        if relation_info.reverse:
+            if to_field and not relation_info.related_model_field.related_fields[0][1].primary_key:
+                field_kwargs['slug_field'] = to_field
+                field_class = self.serializer_related_to_field
+        else:
+            if to_field 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
 
         # `view_name` is only valid for hyperlinked relationships.
         if not issubclass(field_class, HyperlinkedRelatedField):

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, related_model_field, to_many, to_field, has_through_model, reverse = relation_info
     kwargs = {
         'queryset': related_model._default_manager,
         'view_name': get_detail_view_name(related_model)

rest_framework/utils/model_meta.py

@@ -29,9 +29,11 @@
 RelationInfo = namedtuple('RelationInfo', [
     'model_field',
     'related_model',
+    'related_model_field',
     'to_many',
     'to_field',
-    'has_through_model'
+    'has_through_model',
+    'reverse'
 ])
 
 
@@ -108,22 +110,26 @@ def _get_forward_relationships(opts):
         forward_relations[field.name] = RelationInfo(
             model_field=field,
             related_model=_resolve_model(field.rel.to),
+            related_model_field=None,
             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.
     for field in [field for field in opts.many_to_many if field.serialize]:
         forward_relations[field.name] = RelationInfo(
             model_field=field,
             related_model=_resolve_model(field.rel.to),
+            related_model_field=None,
             to_many=True,
             # manytomany do not have to_fields
             to_field=None,
             has_through_model=(
                 not field.rel.through._meta.auto_created
-            )
+            ),
+            reverse=False
         )
 
     return forward_relations
@@ -144,9 +150,11 @@ def _get_reverse_relationships(opts):
         reverse_relations[accessor_name] = RelationInfo(
             model_field=None,
             related_model=related,
+            related_model_field=relation.field,
             to_many=relation.field.rel.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.
@@ -156,13 +164,15 @@ def _get_reverse_relationships(opts):
         reverse_relations[accessor_name] = RelationInfo(
             model_field=None,
             related_model=related,
+            related_model_field=relation.field,
             to_many=True,
             # manytomany do not have to_fields
             to_field=None,
             has_through_model=(
                 (getattr(relation.field.rel, 'through', None) is not None) and
                 not relation.field.rel.through._meta.auto_created
-            )
+            ),
+            reverse=True
         )
 
     return reverse_relations

tests/test_model_serializer.py

@@ -91,6 +91,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):
@@ -940,3 +949,62 @@ class Meta(TestSerializer.Meta):
         self.assertEqual(unicode_repr(ChildSerializer()), child_expected)
         self.assertEqual(unicode_repr(TestSerializer()), test_expected)
         self.assertEqual(unicode_repr(ChildSerializer()), child_expected)
+
+
+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=[<UniqueValidator(queryset=TestChildModel.objects.all())>])
+                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)