Opened 8 years ago
Last modified 8 years ago
#28345 closed Bug
limit_choices_to callable is no longer applied during ModelForm instantiation which blocks some use cases — at Version 1
| Reported by: | drunkard | Owned by: | nobody | 
|---|---|---|---|
| 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 }}
      Change History (2)
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.
Simple test project of this bug