Opened 7 years ago

Last modified 7 years ago

#28345 closed Bug

#27563 caused regression to ModelForm — at Initial Version

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

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 }}

Change History (1)

by drunkard, 7 years ago

Attachment: limit_choices_to-bug.tgz added

Simple test project of this bug

Note: See TracTickets for help on using tickets.
Back to Top