From a1be12fe193c8f3de8a0b0820f460a302472375f Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Wed, 28 Jun 2017 20:30:19 -0700 Subject: [PATCH] Fixed #28345 -- Applied limit_choices_to during ModelForm.__init__(). field_for_model() now has an additional keyword argument, apply_limit_choices_to, allowing it to continue to be used to create form fields dynamically after ModelForm.__init__() is called. Thanks Tim Graham for the review. --- django/forms/models.py | 34 +++++++++++++++++++++++----------- docs/releases/1.11.3.txt | 3 +++ tests/model_forms/tests.py | 12 +++++++++++- 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index 79e4452b17..bcb417489b 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -93,10 +93,18 @@ def model_to_dict(instance, fields=None, exclude=None): return data +def apply_limit_choices_to_to_formfield(formfield): + """Apply limit_choices_to to the formfield's queryset if needed.""" + if hasattr(formfield, 'queryset') and hasattr(formfield, 'get_limit_choices_to'): + limit_choices_to = formfield.get_limit_choices_to() + if limit_choices_to is not None: + formfield.queryset = formfield.queryset.complex_filter(limit_choices_to) + + def fields_for_model(model, fields=None, exclude=None, widgets=None, formfield_callback=None, localized_fields=None, labels=None, help_texts=None, error_messages=None, - field_classes=None): + field_classes=None, *, apply_limit_choices_to=True): """ Return an ``OrderedDict`` containing form fields for the given model. @@ -123,6 +131,9 @@ def fields_for_model(model, fields=None, exclude=None, widgets=None, ``field_classes`` is a dictionary of model field names mapped to a form field class. + + ``apply_limit_choices_to`` is a boolean indicating if limit_choices_to + should be applied to a field's queryset. """ field_list = [] ignored = [] @@ -166,11 +177,8 @@ def fields_for_model(model, fields=None, exclude=None, widgets=None, formfield = formfield_callback(f, **kwargs) if formfield: - # Apply ``limit_choices_to``. - if hasattr(formfield, 'queryset') and hasattr(formfield, 'get_limit_choices_to'): - limit_choices_to = formfield.get_limit_choices_to() - if limit_choices_to is not None: - formfield.queryset = formfield.queryset.complex_filter(limit_choices_to) + if apply_limit_choices_to: + apply_limit_choices_to_to_formfield(formfield) field_list.append((f.name, formfield)) else: ignored.append(f.name) @@ -241,11 +249,13 @@ class ModelFormMetaclass(DeclarativeFieldsMetaclass): # fields from the model" opts.fields = None - fields = fields_for_model(opts.model, opts.fields, opts.exclude, - opts.widgets, formfield_callback, - opts.localized_fields, opts.labels, - opts.help_texts, opts.error_messages, - opts.field_classes) + fields = fields_for_model( + opts.model, opts.fields, opts.exclude, opts.widgets, + formfield_callback, opts.localized_fields, opts.labels, + opts.help_texts, opts.error_messages, opts.field_classes, + # limit_choices_to will be applied during ModelForm.__init__(). + apply_limit_choices_to=False, + ) # make sure opts.fields doesn't specify an invalid field none_model_fields = [k for k, v in fields.items() if not v] @@ -291,6 +301,8 @@ class BaseModelForm(BaseForm): data, files, auto_id, prefix, object_data, error_class, label_suffix, empty_permitted, use_required_attribute=use_required_attribute, ) + for formfield in self.fields.values(): + apply_limit_choices_to_to_formfield(formfield) def _get_validation_exclusions(self): """ diff --git a/docs/releases/1.11.3.txt b/docs/releases/1.11.3.txt index 227c94abb4..79fd3edb6a 100644 --- a/docs/releases/1.11.3.txt +++ b/docs/releases/1.11.3.txt @@ -57,3 +57,6 @@ Bugfixes * Fixed ``UnboundLocalError`` crash in ``RenameField`` with nonexistent field (:ticket:`28350`). + +* Fixed a regression preventing a model field's ``limit_choices_to`` from being + evaluated when a ``ModelForm`` is instantiated (:ticket:`28345`). diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index 96f6d6985b..9a4ca57786 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -1,7 +1,7 @@ import datetime import os from decimal import Decimal -from unittest import skipUnless +from unittest import mock, skipUnless from django import forms from django.core.exceptions import ( @@ -2906,6 +2906,16 @@ class LimitChoicesToTests(TestCase): fields = fields_for_model(StumpJoke, ['has_fooled_today']) self.assertSequenceEqual(fields['has_fooled_today'].queryset, [self.threepwood]) + def test_callable_called_each_time_form_is_instantiated(self): + field = StumpJokeForm.base_fields['most_recently_fooled'] + with mock.patch.object(field, 'limit_choices_to') as today_callable_dict: + StumpJokeForm() + self.assertEqual(today_callable_dict.call_count, 1) + StumpJokeForm() + self.assertEqual(today_callable_dict.call_count, 2) + StumpJokeForm() + self.assertEqual(today_callable_dict.call_count, 3) + class FormFieldCallbackTests(SimpleTestCase):