Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fixed Django1.9 compatibility and added test.
  • Loading branch information
bphillips committed Dec 7, 2015
commit dae5a5ce43f63e96c10dce2ad1e9c080db172a22
2 changes: 1 addition & 1 deletion rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1132,7 +1132,7 @@ def build_relational_field(self, field_name, relation_info):

to_field = field_kwargs.pop('to_field', None)
if relation_info.reverse:
if to_field and not relation_info.related_model_field.related_field.primary_key:
if to_field and not relation_info.related_model_field.related_fields[0][1].primary_key:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea what the [0][1] refers to here?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will need to go back in and check what the actual values are. I really should have put a comment in there when I made the change. I'm pretty sure it's grabbing the to_field value for the related field (see the relevant django code here) but you are totally right, it is very hard to decipher just by looking at it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related_fields[0] is the first field in the model's from_fields list; it's a tuple of (from_field, to_field) so [1] here is referring to to_field

field_kwargs['slug_field'] = to_field
field_class = self.serializer_related_to_field
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests don't appear to hit this branch, so hard to verify on first sight - some more digging may be required.

Copy link

@benred42 benred42 Jul 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it looks like we need to add a test case where the to_field argument on the foreign key relationship is set to a non-pk field in order to cover this branch. I hope to get a chance to work on this soon.

else:
Expand Down
35 changes: 35 additions & 0 deletions tests/test_model_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -899,3 +899,38 @@ class Meta:
serializer = TestSerializer()

assert len(serializer.fields['decimal_field'].validators) == 2


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)