From 2d48dd4e4cdb4f9edc2abeebf5d0f73cbde33b15 Mon Sep 17 00:00:00 2001 From: Adrien Brunet Date: Thu, 20 Dec 2018 17:35:04 +0100 Subject: [PATCH 1/6] Fix issue1811: take limit_choices_to into account with FK --- rest_framework/relations.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index e8a4ec2ac3..8e73319331 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -169,6 +169,12 @@ def get_queryset(self): # for the field. # Eg: 'MyRelationship(queryset=ExampleModel.objects.all())' queryset = queryset.all() + if hasattr(self.parent, "Meta") and hasattr(self.parent.Meta, "model"): + queryset = queryset.filter( + **self.parent.Meta.model._meta.get_field( + self.source + ).get_limit_choices_to() + ) return queryset def use_pk_only_optimization(self): From 3f95d60a82b634db311f8a7384f757601659699a Mon Sep 17 00:00:00 2001 From: Adrien Brunet Date: Fri, 21 Dec 2018 12:30:14 +0100 Subject: [PATCH 2/6] Issue 1811: Add tests to illustrate issue --- tests/models.py | 7 ++++ tests/test_relations_with_limited_queryset.py | 38 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 tests/test_relations_with_limited_queryset.py diff --git a/tests/models.py b/tests/models.py index 6aeceb9345..55f250e046 100644 --- a/tests/models.py +++ b/tests/models.py @@ -52,6 +52,13 @@ class ForeignKeySource(RESTFrameworkModel): on_delete=models.CASCADE) +class ForeignKeySourceWithLimitedChoices(RESTFrameworkModel): + target = models.ForeignKey(ForeignKeyTarget, help_text='Target', + verbose_name='Target', + limit_choices_to={"name__startswith": "limited-"}, + on_delete=models.CASCADE) + + # Nullable ForeignKey class NullableForeignKeySource(RESTFrameworkModel): name = models.CharField(max_length=100) diff --git a/tests/test_relations_with_limited_queryset.py b/tests/test_relations_with_limited_queryset.py new file mode 100644 index 0000000000..4408520d39 --- /dev/null +++ b/tests/test_relations_with_limited_queryset.py @@ -0,0 +1,38 @@ +from django.test import TestCase + +from rest_framework import serializers + +from .models import ( + ForeignKeySource, + ForeignKeyTarget, + ForeignKeySourceWithLimitedChoices, +) + + +class ForeignKeySourceWithLimitedChoicesSerializer(serializers.ModelSerializer): + class Meta: + model = ForeignKeySourceWithLimitedChoices + fields = ("id", "target") + + +class ForeignKeySourceSerializer(serializers.ModelSerializer): + class Meta: + model = ForeignKeySource + fields = ("id", "target") + + +class LimitedChoicesInQuerySetTests(TestCase): + def setUp(self): + for idx in range(1, 4): + limited_target = ForeignKeyTarget(name="limited-target-%d" % idx) + limited_target.save() + target = ForeignKeyTarget(name="target-%d" % idx) + target.save() + + def test_queryset_size_without_limited_choices(self): + queryset = ForeignKeySourceSerializer().fields["target"].get_queryset() + assert len(queryset) == 6 + + def test_queryset_size_with_limited_choices(self): + queryset = ForeignKeySourceWithLimitedChoicesSerializer().fields["target"].get_queryset() + assert len(queryset) == 3 From ca9dcdffa206359f7f857c03581efbd0a07a9203 Mon Sep 17 00:00:00 2001 From: Adrien Brunet Date: Fri, 21 Dec 2018 15:24:30 +0100 Subject: [PATCH 3/6] Filter queryset only if limit_choices_to exists --- rest_framework/relations.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 8e73319331..20390b6106 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -170,11 +170,11 @@ def get_queryset(self): # Eg: 'MyRelationship(queryset=ExampleModel.objects.all())' queryset = queryset.all() if hasattr(self.parent, "Meta") and hasattr(self.parent.Meta, "model"): - queryset = queryset.filter( - **self.parent.Meta.model._meta.get_field( - self.source - ).get_limit_choices_to() - ) + filter_dict = self.parent.Meta.model._meta.get_field( + self.source + ).get_limit_choices_to() + if filter_dict: + queryset = queryset.filter(**filter_dict) return queryset def use_pk_only_optimization(self): From a66e8ac579af2f47f772209b7761c651c6414e38 Mon Sep 17 00:00:00 2001 From: Adrien Brunet Date: Fri, 21 Dec 2018 16:01:52 +0100 Subject: [PATCH 4/6] Move test_relations_with_limited_querysets file within test_relations_pk --- tests/test_relations_pk.py | 26 +++++++++++-- tests/test_relations_with_limited_queryset.py | 38 ------------------- 2 files changed, 22 insertions(+), 42 deletions(-) delete mode 100644 tests/test_relations_with_limited_queryset.py diff --git a/tests/test_relations_pk.py b/tests/test_relations_pk.py index 3317d6251e..31b6bb8677 100644 --- a/tests/test_relations_pk.py +++ b/tests/test_relations_pk.py @@ -5,10 +5,10 @@ from rest_framework import serializers from tests.models import ( - ForeignKeySource, ForeignKeyTarget, ManyToManySource, ManyToManyTarget, - NullableForeignKeySource, NullableOneToOneSource, - NullableUUIDForeignKeySource, OneToOnePKSource, OneToOneTarget, - UUIDForeignKeyTarget + ForeignKeySource, ForeignKeySourceWithLimitedChoices, ForeignKeyTarget, + ManyToManySource, ManyToManyTarget, NullableForeignKeySource, + NullableOneToOneSource, NullableUUIDForeignKeySource, OneToOnePKSource, + OneToOneTarget, UUIDForeignKeyTarget ) @@ -38,6 +38,12 @@ class Meta: fields = ('id', 'name', 'target') +class ForeignKeySourceWithLimitedChoicesSerializer(serializers.ModelSerializer): + class Meta: + model = ForeignKeySourceWithLimitedChoices + fields = ("id", "target") + + # Nullable ForeignKey class NullableForeignKeySourceSerializer(serializers.ModelSerializer): class Meta: @@ -360,6 +366,18 @@ class Meta(ForeignKeySourceSerializer.Meta): serializer.is_valid(raise_exception=True) assert 'target' not in serializer.validated_data + def test_queryset_size_without_limited_choices(self): + limited_target = ForeignKeyTarget(name="limited-target") + limited_target.save() + queryset = ForeignKeySourceSerializer().fields["target"].get_queryset() + assert len(queryset) == 3 + + def test_queryset_size_with_limited_choices(self): + limited_target = ForeignKeyTarget(name="limited-target") + limited_target.save() + queryset = ForeignKeySourceWithLimitedChoicesSerializer().fields["target"].get_queryset() + assert len(queryset) == 1 + class PKNullableForeignKeyTests(TestCase): def setUp(self): diff --git a/tests/test_relations_with_limited_queryset.py b/tests/test_relations_with_limited_queryset.py deleted file mode 100644 index 4408520d39..0000000000 --- a/tests/test_relations_with_limited_queryset.py +++ /dev/null @@ -1,38 +0,0 @@ -from django.test import TestCase - -from rest_framework import serializers - -from .models import ( - ForeignKeySource, - ForeignKeyTarget, - ForeignKeySourceWithLimitedChoices, -) - - -class ForeignKeySourceWithLimitedChoicesSerializer(serializers.ModelSerializer): - class Meta: - model = ForeignKeySourceWithLimitedChoices - fields = ("id", "target") - - -class ForeignKeySourceSerializer(serializers.ModelSerializer): - class Meta: - model = ForeignKeySource - fields = ("id", "target") - - -class LimitedChoicesInQuerySetTests(TestCase): - def setUp(self): - for idx in range(1, 4): - limited_target = ForeignKeyTarget(name="limited-target-%d" % idx) - limited_target.save() - target = ForeignKeyTarget(name="target-%d" % idx) - target.save() - - def test_queryset_size_without_limited_choices(self): - queryset = ForeignKeySourceSerializer().fields["target"].get_queryset() - assert len(queryset) == 6 - - def test_queryset_size_with_limited_choices(self): - queryset = ForeignKeySourceWithLimitedChoicesSerializer().fields["target"].get_queryset() - assert len(queryset) == 3 From 7954c03cbf922ed380cfedd75af128e17fbc863a Mon Sep 17 00:00:00 2001 From: Adrien Brunet Date: Tue, 8 Jan 2019 14:18:16 +0100 Subject: [PATCH 5/6] move limit_choices_to logic from relations.py to utils/field_mapping.py --- rest_framework/relations.py | 6 ------ rest_framework/utils/field_mapping.py | 3 +++ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 20390b6106..e8a4ec2ac3 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -169,12 +169,6 @@ def get_queryset(self): # for the field. # Eg: 'MyRelationship(queryset=ExampleModel.objects.all())' queryset = queryset.all() - if hasattr(self.parent, "Meta") and hasattr(self.parent.Meta, "model"): - filter_dict = self.parent.Meta.model._meta.get_field( - self.source - ).get_limit_choices_to() - if filter_dict: - queryset = queryset.filter(**filter_dict) return queryset def use_pk_only_optimization(self): diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index d3044ff59d..e8f24b6b9e 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -266,6 +266,9 @@ def get_relation_kwargs(field_name, relation_info): # If this field is read-only, then return early. # No further keyword arguments are valid. return kwargs + limit_choices_to = model_field.get_limit_choices_to() + if limit_choices_to: + kwargs['queryset'] = kwargs['queryset'].filter(**limit_choices_to) if model_field.has_default() or model_field.blank or model_field.null: kwargs['required'] = False From 7fb879c9af9a0d32cbcd83747bee901b8ab4f410 Mon Sep 17 00:00:00 2001 From: Adrien Brunet Date: Tue, 8 Jan 2019 14:38:53 +0100 Subject: [PATCH 6/6] move limit_choices_to above other check to avoid conflicts --- rest_framework/utils/field_mapping.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index e8f24b6b9e..991f20f17a 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -249,6 +249,10 @@ def get_relation_kwargs(field_name, relation_info): if to_field: kwargs['to_field'] = to_field + limit_choices_to = model_field and model_field.get_limit_choices_to() + if limit_choices_to: + kwargs['queryset'] = kwargs['queryset'].filter(**limit_choices_to) + if has_through_model: kwargs['read_only'] = True kwargs.pop('queryset', None) @@ -266,9 +270,6 @@ def get_relation_kwargs(field_name, relation_info): # If this field is read-only, then return early. # No further keyword arguments are valid. return kwargs - limit_choices_to = model_field.get_limit_choices_to() - if limit_choices_to: - kwargs['queryset'] = kwargs['queryset'].filter(**limit_choices_to) if model_field.has_default() or model_field.blank or model_field.null: kwargs['required'] = False