From eefc88feefec0c3685bfb102714530b751b4ae90 Mon Sep 17 00:00:00 2001 From: Christopher Adams Date: Sat, 1 Feb 2014 14:23:31 -0500 Subject: [PATCH] Fixed #2445 -- Allowed limit_choices_to attribute to be a callable. ForeignKey or ManyToManyField attribute ``limit_choices_to`` can now be a callable that returns either a ``Q`` object or a dict. Thanks michael at actrix.gen.nz for the original suggestion. --- AUTHORS | 2 +- django/contrib/admin/options.py | 5 +++- django/contrib/admin/utils.py | 12 ++++---- django/contrib/admin/widgets.py | 5 +++- django/db/models/fields/__init__.py | 4 +-- django/db/models/fields/related.py | 38 +++++++++++++++++++++++-- django/forms/fields.py | 11 ++++++++ django/forms/models.py | 13 ++++++++- docs/ref/models/fields.txt | 44 ++++++++++++++++++++++------- docs/releases/1.7.txt | 4 +++ tests/admin_views/admin.py | 3 +- tests/admin_views/models.py | 27 ++++++++++++++++++ tests/admin_views/tests.py | 29 ++++++++++++++++++- tests/model_forms/models.py | 20 ++++++++++++- tests/model_forms/tests.py | 40 +++++++++++++++++++++++++- 15 files changed, 228 insertions(+), 29 deletions(-) diff --git a/AUTHORS b/AUTHORS index fe1ede2f544..dbdc38230cf 100644 --- a/AUTHORS +++ b/AUTHORS @@ -57,7 +57,7 @@ answer newbie questions, and generally made Django that much better: Gisle Aas Chris Adams - Christopher Adams + Christopher Adams Mathieu Agopian Roberto Aguilar ajs diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index e3c3e8e57ea..7b0dd8ebcb2 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -240,7 +240,7 @@ class BaseModelAdmin(six.with_metaclass(RenameBaseModelAdminMethods)): if related_admin is not None: ordering = related_admin.get_ordering(request) if ordering is not None and ordering != (): - return db_field.rel.to._default_manager.using(db).order_by(*ordering).complex_filter(db_field.rel.limit_choices_to) + return db_field.rel.to._default_manager.using(db).order_by(*ordering) return None def formfield_for_foreignkey(self, db_field, request=None, **kwargs): @@ -383,6 +383,9 @@ class BaseModelAdmin(six.with_metaclass(RenameBaseModelAdminMethods)): # ForeignKeyRawIdWidget, on the basis of ForeignKey.limit_choices_to, # are allowed to work. for l in model._meta.related_fkey_lookups: + # As ``limit_choices_to`` can be a callable, invoke it here. + if callable(l): + l = l() for k, v in widgets.url_params_from_lookup_dict(l).items(): if k == lookup and v == value: return True diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index 0b74a6e75f7..a2f2e9fa7b7 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -459,17 +459,17 @@ def get_limit_choices_to_from_path(model, path): """ Return Q object for limiting choices if applicable. If final model in path is linked via a ForeignKey or ManyToManyField which - has a `limit_choices_to` attribute, return it as a Q object. + has a ``limit_choices_to`` attribute, return it as a Q object. """ - fields = get_fields_from_path(model, path) fields = remove_trailing_data_field(fields) - limit_choices_to = ( + get_limit_choices_to = ( fields and hasattr(fields[-1], 'rel') and - getattr(fields[-1].rel, 'limit_choices_to', None)) - if not limit_choices_to: + getattr(fields[-1].rel, 'get_limit_choices_to', None)) + if not get_limit_choices_to: return models.Q() # empty Q - elif isinstance(limit_choices_to, models.Q): + limit_choices_to = get_limit_choices_to() + if isinstance(limit_choices_to, models.Q): return limit_choices_to # already a Q else: return models.Q(**limit_choices_to) # convert dict to Q diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index f527d72f3f7..f501ffb1a85 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -180,7 +180,10 @@ class ForeignKeyRawIdWidget(forms.TextInput): return mark_safe(''.join(output)) def base_url_parameters(self): - return url_params_from_lookup_dict(self.rel.limit_choices_to) + limit_choices_to = self.rel.limit_choices_to + if callable(limit_choices_to): + limit_choices_to = limit_choices_to() + return url_params_from_lookup_dict(limit_choices_to) def url_parameters(self): from django.contrib.admin.views.main import TO_FIELD_VAR diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 77a48d0723f..e2fef7fb71f 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -742,11 +742,11 @@ class Field(RegisterLookupMixin): lst = [(getattr(x, self.rel.get_related_field().attname), smart_text(x)) for x in rel_model._default_manager.complex_filter( - self.rel.limit_choices_to)] + self.get_limit_choices_to())] else: lst = [(x._get_pk_val(), smart_text(x)) for x in rel_model._default_manager.complex_filter( - self.rel.limit_choices_to)] + self.get_limit_choices_to())] return first_choice + lst def get_choices_default(self): diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 6bd91817ddf..f490066516d 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -309,6 +309,35 @@ class RelatedField(Field): if not cls._meta.abstract: self.contribute_to_related_class(other, self.related) + def get_limit_choices_to(self): + """Returns 'limit_choices_to' for this model field. + + If it is a callable, it will be invoked and the result will be + returned. + """ + if callable(self.rel.limit_choices_to): + return self.rel.limit_choices_to() + return self.rel.limit_choices_to + + def formfield(self, **kwargs): + """Passes ``limit_choices_to`` to field being constructed. + + Only passes it if there is a type that supports related fields. + This is a similar strategy used to pass the ``queryset`` to the field + being constructed. + """ + defaults = {} + if hasattr(self.rel, 'get_related_field'): + # If this is a callable, do not invoke it here. Just pass + # it in the defaults for when the form class will later be + # instantiated. + limit_choices_to = self.rel.limit_choices_to + defaults.update({ + 'limit_choices_to': limit_choices_to, + }) + defaults.update(kwargs) + return super(RelatedField, self).formfield(**defaults) + def related_query_name(self): # This method defines the name that can be used to identify this # related object in a table-spanning query. It uses the lower-cased @@ -1525,6 +1554,9 @@ class ForeignObject(RelatedField): # and swapped models don't get a related descriptor. if not self.rel.is_hidden() and not related.model._meta.swapped: setattr(cls, related.get_accessor_name(), self.related_accessor_class(related)) + # While 'limit_choices_to' might be a callable, simply pass + # it along for later - this is too early because it's still + # model load time. if self.rel.limit_choices_to: cls._meta.related_fkey_lookups.append(self.rel.limit_choices_to) @@ -1633,7 +1665,7 @@ class ForeignKey(ForeignObject): qs = self.rel.to._default_manager.using(using).filter( **{self.rel.field_name: value} ) - qs = qs.complex_filter(self.rel.limit_choices_to) + qs = qs.complex_filter(self.get_limit_choices_to()) if not qs.exists(): raise exceptions.ValidationError( self.error_messages['invalid'], @@ -1691,7 +1723,7 @@ class ForeignKey(ForeignObject): (self.name, self.rel.to)) defaults = { 'form_class': forms.ModelChoiceField, - 'queryset': self.rel.to._default_manager.using(db).complex_filter(self.rel.limit_choices_to), + 'queryset': self.rel.to._default_manager.using(db), 'to_field_name': self.rel.field_name, } defaults.update(kwargs) @@ -2127,7 +2159,7 @@ class ManyToManyField(RelatedField): db = kwargs.pop('using', None) defaults = { 'form_class': forms.ModelMultipleChoiceField, - 'queryset': self.rel.to._default_manager.using(db).complex_filter(self.rel.limit_choices_to) + 'queryset': self.rel.to._default_manager.using(db), } defaults.update(kwargs) # If initial is passed in, it's a list of related objects, but the diff --git a/django/forms/fields.py b/django/forms/fields.py index 1ce36c199c7..629aa69c5d1 100644 --- a/django/forms/fields.py +++ b/django/forms/fields.py @@ -170,6 +170,17 @@ class Field(object): """ return {} + def get_limit_choices_to(self): + """ + Returns ``limit_choices_to`` for this form 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 + def _has_changed(self, initial, data): """ Return True if data differs from initial. diff --git a/django/forms/models.py b/django/forms/models.py index 37a1b93bf54..a0b47e64b48 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -324,6 +324,15 @@ class BaseModelForm(BaseForm): self._validate_unique = False super(BaseModelForm, self).__init__(data, files, auto_id, prefix, object_data, error_class, label_suffix, empty_permitted) + # Apply ``limit_choices_to`` to each field. + for field_name in self.fields: + formfield = self.fields[field_name] + if hasattr(formfield, 'queryset'): + limit_choices_to = formfield.limit_choices_to + if limit_choices_to is not None: + if callable(limit_choices_to): + limit_choices_to = limit_choices_to() + formfield.queryset = formfield.queryset.complex_filter(limit_choices_to) def _get_validation_exclusions(self): """ @@ -1082,7 +1091,8 @@ class ModelChoiceField(ChoiceField): def __init__(self, queryset, empty_label="---------", cache_choices=False, required=True, widget=None, label=None, initial=None, - help_text='', to_field_name=None, *args, **kwargs): + help_text='', to_field_name=None, limit_choices_to=None, + *args, **kwargs): if required and (initial is not None): self.empty_label = None else: @@ -1094,6 +1104,7 @@ class ModelChoiceField(ChoiceField): Field.__init__(self, required, widget, label, initial, help_text, *args, **kwargs) self.queryset = queryset + self.limit_choices_to = limit_choices_to # limit the queryset later. self.choice_cache = None self.to_field_name = to_field_name diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt index d4f0c1a4505..cecfddca5fc 100644 --- a/docs/ref/models/fields.txt +++ b/docs/ref/models/fields.txt @@ -1078,21 +1078,45 @@ define the details of how the relation works. .. attribute:: ForeignKey.limit_choices_to - A dictionary of lookup arguments and values (see :doc:`/topics/db/queries`) - that limit the available admin or :class:`ModelForm ` - choices for this object. For example:: + Sets a limit to the available choices for this field when this field is + rendered using a ``ModelForm`` or the admin (by default, all objects + in the queryset are available to choose). Either a dictionary, a + :class:`~django.db.models.Q` object, or a callable returning a + dictionary or :class:`~django.db.models.Q` object can be used. + + For example:: staff_member = models.ForeignKey(User, limit_choices_to={'is_staff': True}) causes the corresponding field on the ``ModelForm`` to list only ``Users`` - that have ``is_staff=True``. + that have ``is_staff=True``. This may be helpful in the Django admin. - Instead of a dictionary this can also be a :class:`Q object - ` for more :ref:`complex queries - `. However, if ``limit_choices_to`` is a :class:`Q - object ` then it will only have an effect on the - choices available in the admin when the field is not listed in - ``raw_id_fields`` in the ``ModelAdmin`` for the model. + The callable form can be helpful, for instance, when used in conjunction + with the Python ``datetime`` module to limit selections by date range. For + example:: + + limit_choices_to = lambda: {'pub_date__lte': datetime.date.utcnow()} + + If ``limit_choices_to`` is or returns a :class:`Q object + `, which is useful for :ref:`complex queries + `, then it will only have an effect on the choices + available in the admin when the field is not listed in + :attr:`~django.contrib.admin.ModelAdmin.raw_id_fields` in the + ``ModelAdmin`` for the model. + + .. versionchanged:: 1.7 + + Previous versions of Django do not allow passing a callable as a value + for ``limit_choices_to``. + + .. note:: + + If a callable is used for ``limit_choices_to``, it will be invoked + every time a new form is instantiated. It may also be invoked when a + model is validated, for example by management commands or the admin. + The admin constructs querysets to validate its form inputs in various + edge cases multiple times, so there is a possibility your callable may + be invoked several times. .. attribute:: ForeignKey.related_name diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index bda84e847f7..3519ac5bdaf 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -608,6 +608,10 @@ Models * It is now possible to use ``None`` as a query value for the :lookup:`iexact` lookup. +* It is now possible to pass a callable as value for the attribute + :attr:`ForeignKey.limit_choices_to` when defining a ``ForeignKey`` or + ``ManyToManyField``. + Signals ^^^^^^^ diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index ae127648973..e17942e8311 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -34,7 +34,7 @@ from .models import (Article, Chapter, Child, Parent, Picture, Widget, UnchangeableObject, UserMessenger, Simple, Choice, ShortMessage, Telegram, FilteredManager, EmptyModelHidden, EmptyModelVisible, EmptyModelMixin, State, City, Restaurant, Worker, ParentWithDependentChildren, - DependentChild) + DependentChild, StumpJoke) def callable_year(dt_value): @@ -884,6 +884,7 @@ site.register(ParentWithDependentChildren, ParentWithDependentChildrenAdmin) site.register(EmptyModelHidden, EmptyModelHiddenAdmin) site.register(EmptyModelVisible, EmptyModelVisibleAdmin) site.register(EmptyModelMixin, EmptyModelMixinAdmin) +site.register(StumpJoke) # Register core models we need in our tests from django.contrib.auth.models import User, Group diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index ac4d4f20c55..4cc58f5e88e 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -173,6 +173,33 @@ class Sketch(models.Model): return self.title +def today_callable_dict(): + return {"last_action__gte": datetime.datetime.today()} + + +def today_callable_q(): + return models.Q(last_action__gte=datetime.datetime.today()) + + +@python_2_unicode_compatible +class Character(models.Model): + username = models.CharField(max_length=100) + last_action = models.DateTimeField() + + def __str__(self): + return self.username + + +@python_2_unicode_compatible +class StumpJoke(models.Model): + variation = models.CharField(max_length=100) + most_recently_fooled = models.ForeignKey(Character, limit_choices_to=today_callable_dict, related_name="+") + has_fooled_today = models.ManyToManyField(Character, limit_choices_to=today_callable_q, related_name="+") + + def __str__(self): + return self.variation + + class Fabric(models.Model): NG_CHOICES = ( ('Textured', ( diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 2bfc3258f52..f5c90bc1d00 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -52,7 +52,7 @@ from .models import (Article, BarAccount, CustomArticle, EmptyModel, FooAccount, Report, MainPrepopulated, RelatedPrepopulated, UnorderedObject, Simple, UndeletableObject, UnchangeableObject, Choice, ShortMessage, Telegram, Pizza, Topping, FilteredManager, City, Restaurant, Worker, - ParentWithDependentChildren) + ParentWithDependentChildren, Character) from .admin import site, site2, CityAdmin @@ -3661,6 +3661,33 @@ class ReadonlyTest(TestCase): self.assertEqual(response.status_code, 200) +@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) +class LimitChoicesToInAdminTest(TestCase): + urls = "admin_views.urls" + fixtures = ['admin-views-users.xml'] + + def setUp(self): + self.client.login(username='super', password='secret') + + def tearDown(self): + self.client.logout() + + def test_limit_choices_to_as_callable(self): + """Test for ticket 2445 changes to admin.""" + threepwood = Character.objects.create( + username='threepwood', + last_action=datetime.datetime.today() + datetime.timedelta(days=1), + ) + marley = Character.objects.create( + username='marley', + last_action=datetime.datetime.today() - datetime.timedelta(days=1), + ) + response = self.client.get('/test_admin/admin/admin_views/stumpjoke/add/') + # The allowed option should appear twice; the limited option should not appear. + self.assertContains(response, threepwood.username, count=2) + self.assertNotContains(response, marley.username) + + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) class RawIdFieldsTest(TestCase): urls = "admin_views.urls" diff --git a/tests/model_forms/models.py b/tests/model_forms/models.py index 72bf1ff26a7..2b83a179edb 100644 --- a/tests/model_forms/models.py +++ b/tests/model_forms/models.py @@ -8,6 +8,7 @@ words, most of these tests should be rewritten. """ from __future__ import unicode_literals +import datetime import os import tempfile @@ -71,7 +72,6 @@ class Article(models.Model): status = models.PositiveIntegerField(choices=ARTICLE_STATUS, blank=True, null=True) def save(self): - import datetime if not self.id: self.created = datetime.date.today() return super(Article, self).save() @@ -329,3 +329,21 @@ class CustomErrorMessage(models.Model): def clean(self): if self.name1 == 'FORBIDDEN_VALUE': raise ValidationError({'name1': [ValidationError('Model.clean() error messages.')]}) + + +def today_callable_dict(): + return {"last_action__gte": datetime.datetime.today()} + + +def today_callable_q(): + return models.Q(last_action__gte=datetime.datetime.today()) + + +class Character(models.Model): + username = models.CharField(max_length=100) + last_action = models.DateTimeField() + + +class StumpJoke(models.Model): + most_recently_fooled = models.ForeignKey(Character, limit_choices_to=today_callable_dict, related_name="+") + has_fooled_today = models.ManyToManyField(Character, limit_choices_to=today_callable_q, related_name="+") diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index c64bad5241c..a0507a1e1ca 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -22,7 +22,8 @@ from .models import (Article, ArticleStatus, BetterWriter, BigInt, Book, DerivedPost, ExplicitPK, FlexibleDatePost, ImprovedArticle, ImprovedArticleWithParentLink, Inventory, Post, Price, Product, TextFile, Writer, WriterProfile, Colour, ColourfulItem, - ArticleStatusNote, DateTimePost, CustomErrorMessage, test_images) + ArticleStatusNote, DateTimePost, CustomErrorMessage, test_images, + StumpJoke, Character) if test_images: from .models import ImageFile, OptionalImageFile @@ -521,6 +522,12 @@ class FieldOverridesTroughFormMetaForm(forms.ModelForm): } +class StumpJokeForm(forms.ModelForm): + class Meta: + model = StumpJoke + fields = '__all__' + + class TestFieldOverridesTroughFormMeta(TestCase): def test_widget_overrides(self): form = FieldOverridesTroughFormMetaForm() @@ -1921,3 +1928,34 @@ class ModelFormInheritanceTests(TestCase): self.assertEqual(list(type(str('NewForm'), (ModelForm, Mixin, Form), {})().fields.keys()), ['name']) self.assertEqual(list(type(str('NewForm'), (ModelForm, Form, Mixin), {})().fields.keys()), ['name', 'age']) self.assertEqual(list(type(str('NewForm'), (ModelForm, Form), {'age': None})().fields.keys()), ['name']) + + +class LimitChoicesToTest(TestCase): + """ + Tests the functionality of ``limit_choices_to``. + """ + def setUp(self): + self.threepwood = Character.objects.create( + username='threepwood', + last_action=datetime.datetime.today() + datetime.timedelta(days=1), + ) + self.marley = Character.objects.create( + username='marley', + last_action=datetime.datetime.today() - datetime.timedelta(days=1), + ) + + def test_limit_choices_to_callable_for_fk_rel(self): + """ + A ForeignKey relation can use ``limit_choices_to`` as a callable, re #2554. + """ + stumpjokeform = StumpJokeForm() + self.assertIn(self.threepwood, stumpjokeform.fields['most_recently_fooled'].queryset) + self.assertNotIn(self.marley, stumpjokeform.fields['most_recently_fooled'].queryset) + + def test_limit_choices_to_callable_for_m2m_rel(self): + """ + A ManyToMany relation can use ``limit_choices_to`` as a callable, re #2554. + """ + stumpjokeform = StumpJokeForm() + self.assertIn(self.threepwood, stumpjokeform.fields['has_fooled_today'].queryset) + self.assertNotIn(self.marley, stumpjokeform.fields['has_fooled_today'].queryset)