From 3b6a5dd585c89f0885cdcdf06be70a514f8bd505 Mon Sep 17 00:00:00 2001 From: Piper Merriam Date: Sat, 6 Sep 2014 14:23:10 -0600 Subject: [PATCH] Add support for limit_choices_to on related fields fixing #1811 --- docs/api-guide/relations.md | 1 + rest_framework/relations.py | 28 ++++++ rest_framework/serializers.py | 10 +-- tests/models.py | 8 ++ tests/test_relations.py | 162 +++++++++++++++++++++++++++++++++- 5 files changed, 203 insertions(+), 6 deletions(-) diff --git a/docs/api-guide/relations.md b/docs/api-guide/relations.md index cc4f55851d..67b2538791 100644 --- a/docs/api-guide/relations.md +++ b/docs/api-guide/relations.md @@ -102,6 +102,7 @@ By default this field is read-write, although you can change this behavior using * `many` - If applied to a to-many relationship, you should set this argument to `True`. * `required` - If set to `False`, the field will accept values of `None` or the empty-string for nullable relationships. * `queryset` - By default `ModelSerializer` classes will use the default queryset for the relationship. `Serializer` classes must either set a queryset explicitly, or set `read_only=True`. +* `limit_choices_to` - Used to filter the `queryset` to a limited set of choices. Either a dictionary, a Q object, or a callable returning a dictionary or Q object can be used. ## HyperlinkedRelatedField diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 1acbdce26b..353cf68d10 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -41,6 +41,8 @@ class RelatedField(WritableField): def __init__(self, *args, **kwargs): queryset = kwargs.pop('queryset', None) + self.limit_choices_to = kwargs.pop('limit_choices_to', {}) + self.many = kwargs.pop('many', self.many) if self.many: self.widget = self.many_widget @@ -58,6 +60,16 @@ def __init__(self, *args, **kwargs): def initialize(self, parent, field_name): super(RelatedField, self).initialize(parent, field_name) + + if self.queryset is None and not self.limit_choices_to: + manager = getattr(self.parent.opts.model, self.source or field_name) + try: + self.limit_choices_to = manager.field.rel.limit_choices_to + except AttributeError: + # GenericForeignKey and their reverse relationships don't have + # a `field` property. + pass + if self.queryset is None and not self.read_only: manager = getattr(self.parent.opts.model, self.source or field_name) if hasattr(manager, 'related'): # Forward @@ -65,6 +77,22 @@ def initialize(self, parent, field_name): else: # Reverse self.queryset = manager.field.rel.to._default_manager.all() + if self.queryset is not None: + self.queryset = self.queryset.complex_filter(self.get_limit_choices_to()) + + def get_limit_choices_to(self): + """ + source: `django.db.models.fields.related.RelatedField.get_limit_choices_to` + + Returns 'limit_choices_to' for this serializer field. + + If it is a callable, it will be invoked and the result will be + returned. + """ + if callable(self.limit_choices_to): + return self.limit_choices_to() + return self.limit_choices_to + # We need this stuff to make form choices work... def prepare_value(self, obj): diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index be8ad3f249..064d981bab 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -822,9 +822,6 @@ def get_related_field(self, model_field, related_model, to_many): Note that model_field will be `None` for reverse relationships. """ - # TODO: filter queryset using: - # .using(db).complex_filter(self.rel.limit_choices_to) - kwargs = { 'queryset': related_model._default_manager, 'many': to_many @@ -846,6 +843,9 @@ def get_related_field(self, model_field, related_model, to_many): if model_field.help_text is not None: kwargs['help_text'] = model_field.help_text + if model_field.rel.limit_choices_to: + kwargs['limit_choices_to'] = model_field.rel.limit_choices_to + return PrimaryKeyRelatedField(**kwargs) def get_field(self, model_field): @@ -1101,8 +1101,6 @@ def get_related_field(self, model_field, related_model, to_many): """ Creates a default instance of a flat relational field. """ - # TODO: filter queryset using: - # .using(db).complex_filter(self.rel.limit_choices_to) kwargs = { 'queryset': related_model._default_manager, 'view_name': self._get_default_view_name(related_model), @@ -1115,6 +1113,8 @@ def get_related_field(self, model_field, related_model, to_many): kwargs['help_text'] = model_field.help_text if model_field.verbose_name is not None: kwargs['label'] = model_field.verbose_name + if model_field.rel.limit_choices_to: + kwargs['limit_choices_to'] = model_field.rel.limit_choices_to if self.opts.lookup_field: kwargs['lookup_field'] = self.opts.lookup_field diff --git a/tests/models.py b/tests/models.py index fe064b46ac..dc9534ec01 100644 --- a/tests/models.py +++ b/tests/models.py @@ -134,6 +134,14 @@ class OptionalRelationModel(RESTFrameworkModel): other = models.ForeignKey('OptionalRelationModel', blank=True, null=True) +# Model for issue #1811 +class LimitedChoicesModel(RESTFrameworkModel): + rel = models.ForeignKey( + 'ForeignKeyTarget', + limit_choices_to={'name': 'foo'}, + ) + + # Model for RegexField class Book(RESTFrameworkModel): isbn = models.CharField(max_length=13) diff --git a/tests/test_relations.py b/tests/test_relations.py index bc1db69fcf..33584487c4 100644 --- a/tests/test_relations.py +++ b/tests/test_relations.py @@ -7,7 +7,7 @@ from django.test import TestCase from django.utils import unittest from rest_framework import serializers -from tests.models import BlogPost +from tests.models import BlogPost, LimitedChoicesModel, ForeignKeyTarget class NullModel(models.Model): @@ -147,3 +147,163 @@ def test_blank_option_is_added_to_choice_if_required_equals_false(self): widget_count = len(field.widget.choices) self.assertEqual(widget_count, choice_count + 1, 'BLANK_CHOICE_DASH option should have been added') + + +class LimitChoicesToTest(TestCase): + """ + Test for #1811 "Support `limit_choices_to` on related fields." + + 1. Fully auto-generated relation field: should limit if model field has + limit_choices_to set + 2. Declared relation field with neither queryset or limit_choices_to field + set: should limit if model field has limit_choices_to set + 3. Declared relation field with queryset declared and without + limit_choices_to set where the model field has limit_choices_to set: The + user has explicitly declared a queryset so I don't think we should modify + it. + 4. Declared relation field with limit_choices_to declared and no queryset + declared: Should limit choices based on the limit_choices_to that was + declared on the serializer field. + 5. Declared relation field with both limit_choices_to and queryset + declared: I think that since both were declared, that it makes sense to go + ahead and apply the limit choices to filtering to the provided queryset. + """ + def test_generated_field_with_limit_choices_set_on_model_field(self): + """ + Ensure that for a fully auto-generated serializer field for a model + field which has the `limit_choices_to` value set that the queryset is + filtered correctly on the value from the model field. + """ + # Generate one instance that will match the `limit_choices_to` + ForeignKeyTarget.objects.create(name='foo') + ForeignKeyTarget.objects.create(name='bar') + + class LimitChoicesSerializer(serializers.ModelSerializer): + class Meta: + model = LimitedChoicesModel + fields = ('rel',) + + serializer = LimitChoicesSerializer() + + field = serializer.fields['rel'] + queryset = field.queryset + + self.assertEqual( + set(queryset.all()), + set(ForeignKeyTarget.objects.filter(name='foo')), + ) + + def test_declared_related_field_with_limit_choices_set_on_model(self): + """ + Test that a declared `RelatedField` will correctly filter on it's model + field's `limit_choices_to` when neither the queryset nor the local + `limit_choices_to` has been declared. + + TODO: is this test necessary? + """ + # Generate one instance that will match the `limit_choices_to` + ForeignKeyTarget.objects.create(name='foo') + ForeignKeyTarget.objects.create(name='bar') + + class LimitChoicesSerializer(serializers.ModelSerializer): + rel = serializers.RelatedField(read_only=False) + + class Meta: + model = LimitedChoicesModel + fields = ('rel',) + + serializer = LimitChoicesSerializer() + + field = serializer.fields['rel'] + queryset = field.queryset + + self.assertEqual( + set(queryset.all()), + set(ForeignKeyTarget.objects.filter(name='foo')), + ) + + def test_declared_queryset_on_related_field_is_not_effected_by_model_limit_choices_to(self): + """ + Test that when the `queryset` kwarg is declared for a `RelatedField` + that it isn't further filtered when `limit_choices_to` has been + declared on the model field. + """ + ForeignKeyTarget.objects.create(name='foo') + ForeignKeyTarget.objects.create(name='bar') + + class LimitChoicesSerializer(serializers.ModelSerializer): + rel = serializers.RelatedField( + queryset=ForeignKeyTarget.objects.all(), + read_only=False, + ) + + class Meta: + model = LimitedChoicesModel + fields = ('rel',) + + serializer = LimitChoicesSerializer() + + field = serializer.fields['rel'] + queryset = field.queryset + + self.assertEqual( + set(queryset.all()), + set(ForeignKeyTarget.objects.all()), + ) + + def test_limit_choices_to_on_serializer_field_overrides_model_field(self): + """ + Test that when `limit_choices_to` is declared on a serializer field + that it correctly overrides the value declared on the model field. + """ + ForeignKeyTarget.objects.create(name='foo') + ForeignKeyTarget.objects.create(name='bar') + + class LimitChoicesSerializer(serializers.ModelSerializer): + rel = serializers.RelatedField(limit_choices_to={'name': 'bar'}, read_only=False) + + class Meta: + model = LimitedChoicesModel + fields = ('rel',) + + serializer = LimitChoicesSerializer() + + field = serializer.fields['rel'] + queryset = field.queryset + + self.assertEqual( + set(queryset.all()), + set(ForeignKeyTarget.objects.filter(name='bar')), + ) + + def test_serializer_field_with_both_limit_choices_to_and_queryset_is_filtered(self): + """ + Test that when both the `limit_choices_to` and `queryset` are declared + for a serializer field that the provided queryset is subsequently + filtered using the provided `limit_choices_to`. + """ + ForeignKeyTarget.objects.create(name='foo') + only_choice = ForeignKeyTarget.objects.create(name='bar') + ForeignKeyTarget.objects.create(name='baz') + to_exclude = ForeignKeyTarget.objects.create(name='bar') + + class LimitChoicesSerializer(serializers.ModelSerializer): + rel = serializers.RelatedField( + limit_choices_to={'name': 'bar'}, + queryset=ForeignKeyTarget.objects.exclude(pk=to_exclude.pk), + read_only=False, + ) + + class Meta: + model = LimitedChoicesModel + fields = ('rel',) + + serializer = LimitChoicesSerializer() + + field = serializer.fields['rel'] + queryset = field.queryset + + self.assertEqual( + set(queryset.all()), + set(ForeignKeyTarget.objects.filter(pk=only_choice.pk)), + )