Opened 10 years ago
Closed 9 years ago
#24974 closed Bug (fixed)
Cannot create subclass of form created by `modelform_factory()`, e.g. in `ModelAdmin.get_form()`
Reported by: | Tai Lee | Owned by: | Yoong Kang Lim |
---|---|---|---|
Component: | Forms | Version: | 1.8 |
Severity: | Normal | Keywords: | form modelform modelform_factory modeladmin get_site metaclass modelformmetaclass |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The default implementation of ModelAdmin.get_form()
uses modelform_factory()
to dynamically create a new ModelForm
class, and apply form field overrides, etc.
The docs mention that I can specify a new base form that is passed to this implementation or just return a new ModelForm
class. But when designing a generic ModelAdmin
class that will be used with a number of yet unspecified models, I want to use the form returned by the super class as the base, in case one has already been set on a ModelAdmin
class my class will be mixed into.
I would normally expect that this just works:
def get_form(self, *args, **kwargs): form_class = super(MyModelAdmin, self).get_form(*args, **kwargs) class Form(form_class): # do stuff. return Form
But while form_class
does all the things like form field overrides, Form
does not. All form fields are rendered in the admin with regular form widgets, not the admin variants. The reason appears to be because when Form
is created, ModelFormMetaclass
is executed again but without the formfield_overrides
argument.
To make it work again, I need to do:
def get_form(self, *args, **kwargs): form_class = super(MyModelAdmin, self).get_form(*args, **kwargs) class Form(form_class): # do stuff. kwargs['form'] = Form return super(MyModelAdmin, self).get_form(*args, **kwargs)
This runs through the whole default implementation of get_form()
twice, but it does work.
Alternatively, if I wanted to override the __init__()
method on the form, I could replace it instead of creating a subclass, with something like:
def get_form(self, *args, **kwargs): form_class = super(FluentLayoutsMixin, self).get_form(*args, **kwargs) old_init = form_class.__init__ def new_init(self, *args, **kwargs): old_init.__get__(self, type(self))(*args, **kwargs) # do stuff. form_class.__init__ = new_init.__get__(None, form_class) return form_class
This won't execute the default get_form()
implementation twice, but it's getting pretty hairy.
This took three people a few hours to work out, stepping through many pdb
sessions in django admin and model form code. A note in the docs might save someone else the trouble.
Something like:
Note: You cannot just create and return a subclass of form from the super class in this method. Due to the way
ModelFormMetaclass
works, the subclass will ignoreformfield_overrides
unless you pass it back through the default implementation.
# NO: `formfield_overrides` not applied. def get_form(self, *args, **kwargs): form_class = super(MyModelAdmin, self).get_form(*args, **kwargs) class Form(form_class): # do stuff. return Form # YES: `formfield_overrides` applied. def get_form(self, *args, **kwargs): form_class = super(MyModelAdmin, self).get_form(*args, **kwargs) class Form(form_class): # do stuff. kwargs['form'] = Form return super(MyModelAdmin, self).get_form(*args, **kwargs)
This is actually more of a general problem with modelform_factory()
than a problem with ModelAdmin.get_form()
, so perhaps a note for the modelform_factory()
docs instead, with a link to it from the ModelAdmin.get_form()
docs.
Here is an interactive shell session demonstrating the problem with modelform_factory()
.
>>> from django import forms >>> from django.forms.models import modelform_factory >>> from django.contrib.sites.models import Site # Create a form class where all fields are converted to `DateTimeField` via callback. >>> Form = modelform_factory(Site, formfield_callback=lambda f: forms.DateTimeField) >>> Form.base_fields OrderedDict([(u'id', <class 'django.forms.fields.DateTimeField'>), ('domain', <class 'django.forms.fields.DateTimeField'>), ('name', <class 'django.forms.fields.DateTimeField'>)]) # Note that for any subclass, all base fields have reverted to their original values, instead of being inherited. >>> class FooForm(Form): pass >>> FooForm.base_fields OrderedDict([('domain', <django.forms.fields.CharField object at 0x112db5f90>), ('name', <django.forms.fields.CharField object at 0x112d74a90>)])
Change History (6)
comment:1 by , 10 years ago
Summary: | Cannot create subclass of form created by `modelform_factoy()`, e.g. in `ModelAdmin.get_form()` → Cannot create subclass of form created by `modelform_factory()`, e.g. in `ModelAdmin.get_form()` |
---|
comment:2 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 9 years ago
As another example we're doing this in our common ModelAdmin extension:
def get_form(self, request, obj=None, **kwargs): field_overrides = self.get_field_overrides() readonly_fields = set(self.get_readonly_fields(request, obj)) fields = {name: field for name, field in field_overrides.iteritems() if name not in readonly_fields} form = type(self.form)(self.form.__name__ + b"_", (self.form,), fields) return super(YPModelAdmin, self).get_form(request, obj, form=form, **kwargs)
I ended up here because the more natural approach would be:
def get_form(self, request, obj=None, **kwargs): form = super(YPModelAdmin, self).get_form(request, obj, **kwargs) field_overrides = self.get_field_overrides() readonly_fields = set(self.get_readonly_fields(request, obj)) fields = {name: field for name, field in field_overrides.iteritems() if name not in readonly_fields} return type(form)(form.__name__ + b"_", (form,), fields)
But that doesn't work right due to this issue.
comment:4 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Discussion in #django-dev seemed to indicate that this is a problem we should solve.