#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 , 7 years ago
Attachment: | limit_choices_to-bug.tgz added |
---|
comment:1 by , 7 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 , 7 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 , 7 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 , 7 years ago
Has patch: | set |
---|
Simple test project of this bug