#28345 closed Bug (fixed)
limit_choices_to callable is no longer applied during ModelForm instantiation which blocks some use cases
| Reported by: | drunkard | Owned by: | Jon Dufresne |
|---|---|---|---|
| Component: | Forms | Version: | 1.11 |
| Severity: | Release blocker | Keywords: | ModelForm |
| Cc: | Jon Dufresne | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
I'm upgrading to Django 1.11.2 and found that limit_choices_to is applied to forms too early. It's caused by commit 6abd6c598ea23e0a962c87b0075aa2f79f9ead36 (ticket #27563).
I'm using dynamic limit_choices_to function to get user specific filter expr. It just called once during develop server init, and won't be call while open a USER EDIT FORM, which is inherited from ModelForm, but it work in django admin edit page, by reading the code, django admin build form using modelform_factory(), with this commit reverted, my customized form works just fine. So, I think it's a regression.
Below is key part of test project showing the problem:
proj/middleware.py
from threading import current_thread
class CUserMiddleware(object):
"""Get current operating request and user"""
_user = {}
def process_request(self, request):
self._user[current_thread()] = request.user
def process_response(self, request, response):
# TODO cleanup by alive session, if user session expired, remove all
# items related to the user (dict value).
# self._user.pop(current_thread(), None) # clean to avoid it grows
if len(self._user) > 1000:
self._user = {}
return response
@classmethod
def get_user(cls, default=None):
# debug('当前用户: {}'.format(cls._user.get(current_thread(), default)))
return cls._user.get(current_thread(), default)
def current_user():
return CUserMiddleware.get_user()
testapp/models.py
from django.db import models
from django.db.models import Q
from proj.middleware import current_user
def limit_export_branch():
u = current_user()
if u:
print('\tlimit_export_branch worked the right way')
return Q(name__endswith='something')
print('\tlimit_export_branch works in wrong way')
import traceback
traceback.print_stack()
return Q(pk=-1) # deny to avoid info leak
class Branch(models.Model):
name = models.CharField(max_length=16)
def __str__(self):
return self.name
class Export(models.Model):
name = models.CharField(max_length=16)
branch = models.ForeignKey(Branch, limit_choices_to=limit_export_branch)
def __str__(self):
return self.name
testapp/forms.py
from django import forms
from .models import Export
class ExportEditForm(forms.ModelForm):
class Meta:
model = Export
fields = ['name', 'branch']
testapp/views.py
from django.views.generic import UpdateView
from .forms import ExportEditForm
from .models import Export
class ExportEditView(UpdateView):
form_class = ExportEditForm
model = Export
template_name = 'edit.html'
pk_url_kwarg = 'export_pk'
and the ugly template: testapp/templates/edit.html
{{ form.as_table }}
Attachments (1)
Change History (7)
by , 8 years ago
| Attachment: | limit_choices_to-bug.tgz added |
|---|
comment:1 by , 8 years ago
| Cc: | added |
|---|---|
| Description: | modified (diff) |
| Severity: | Normal → Release blocker |
| Summary: | #27563 caused regression to ModelForm → limit_choices_to callable is no longer applied during ModelForm instantiation which blocks some use cases |
| Triage Stage: | Unreviewed → Accepted |
Without much investigation, I'm not sure if we should revert the original change or if some other workaround might restore allowing this use case.
comment:2 by , 8 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
I agree, this is a regression. The docs say:
If a callable is used for
limit_choices_to, it will be invoked every time a new form is instantiated.
I have a fix in mind that will restore previous, correct behavior will also facilitating my use case motivating the original change.
comment:3 by , 8 years ago
The change adds an additional keyword argument, apply_limit_choices_to, to field_for_model(). This arugment allows field_for_model() to continue to be used to create form fields dynamically after ModelForm.__init__() is called.
comment:4 by , 8 years ago
| Has patch: | set |
|---|
Simple test project of this bug