#28345: limit_choices_to callable is no longer applied during ModelForm
instantiation which blocks some use cases
---------------------------------+------------------------------------
Reporter: drunkard | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Severity: Release blocker | Resolution:
Keywords: ModelForm | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by Tim Graham):
* cc: Jon Dufresne (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Old description:
> This change breaks inheriting from django.forms.models.ModelForm
> directly.
>
> I'm upgrading to python3.6 + django-1.11.2, and gets wrong, the choice
> field is always empty, caused by this commit
> 6abd6c598ea23e0a962c87b0075aa2f79f9ead36, and it's ticket is
> https://code.djangoproject.com/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.
>
> Or, the usage of ModelForm changed? but newest doc not mentioned.
>
> Below is key part of test project saying 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 }}
> }}}
New description:
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 }}
}}}
--
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28345#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.6f543fa7ef65420774f5078ba6b1fc86%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
