Opened 15 years ago
Closed 14 years ago
#15907 closed Bug (fixed)
Generic inlines ignoring the exclude information from a custom form.
| Reported by: | leonelfreire | Owned by: | nobody |
|---|---|---|---|
| Component: | Forms | Version: | 1.3 |
| Severity: | Normal | Keywords: | form, exclude, generic, inline |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Suppose we have the following models:
from django.db import models
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic
class MainModel(models.Model):
field1 = models.CharField(max_length=255)
class Inline1(models.Model):
main_model = models.ForeignKey(MainModel)
inline1_field1 = models.CharField(max_length=255)
inline1_field2 = models.CharField(max_length=255)
class Inline2(models.Model):
content_type = models.ForeignKey(ContentType)
object_id = models.PositiveIntegerField()
content_object = generic.GenericForeignKey()
inline2_field1 = models.CharField(max_length=255)
inline2_field2 = models.CharField(max_length=255)
and this admin configuration:
from django import forms
from django.contrib.contenttypes import generic
from django.contrib import admin
from bug.models import MainModel, Inline1, Inline2
class Inline1Form(forms.models.ModelForm):
class Meta:
model = Inline1
exclude = ('inline1_field2', )
class Inline2Form(forms.models.ModelForm):
class Meta:
model = Inline2
exclude = ('inline2_field2',)
class Inline1Inline(admin.StackedInline):
model = Inline1
form = Inline1Form
extra = 1
class Inline2Inline(generic.GenericStackedInline):
model = Inline2
form = Inline2Form
extra = 1
class MainModelAdmin(admin.ModelAdmin):
inlines = (Inline1Inline, Inline2Inline)
admin.site.register(MainModel, MainModelAdmin)
Then, the exclude attribute from the inner Meta class from Inline2Form is not used in the final Meta class (look the comments):
# Django 1.3 (file: django/contrib/contenttypes/generic.py, line: 362)
def generic_inlineformset_factory(model, form=ModelForm,
formset=BaseGenericInlineFormSet,
ct_field="content_type", fk_field="object_id",
fields=None, exclude=None,
extra=3, can_order=False, can_delete=True,
max_num=None,
formfield_callback=lambda f: f.formfield()):
"""
Returns an ``GenericInlineFormSet`` for the given kwargs.
You must provide ``ct_field`` and ``object_id`` if they different from the
defaults ``content_type`` and ``object_id`` respectively.
"""
opts = model._meta
# Avoid a circular import.
from django.contrib.contenttypes.models import ContentType
# if there is no field called `ct_field` let the exception propagate
ct_field = opts.get_field(ct_field)
if not isinstance(ct_field, models.ForeignKey) or ct_field.rel.to != ContentType:
raise Exception("fk_name '%s' is not a ForeignKey to ContentType" % ct_field)
fk_field = opts.get_field(fk_field) # let the exception propagate
if exclude is not None:
exclude = list(exclude)
exclude.extend([ct_field.name, fk_field.name])
else:
##################################################################
##################################################################
# ct_field.name and fk_field.name are always in the exclude list #
##################################################################
##################################################################
exclude = [ct_field.name, fk_field.name]
FormSet = modelformset_factory(model, form=form,
formfield_callback=formfield_callback,
formset=formset,
extra=extra, can_delete=can_delete, can_order=can_order,
fields=fields, exclude=exclude, max_num=max_num)
FormSet.ct_field = ct_field
FormSet.ct_fk_field = fk_field
return FormSet
# Django 1.3 (file: django/forms/models.py, line: 370)
def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
formfield_callback=None):
# Create the inner Meta class. FIXME: ideally, we should be able to
# construct a ModelForm without creating and passing in a temporary
# inner class.
# Build up a list of attributes that the Meta object will have.
attrs = {'model': model}
if fields is not None:
attrs['fields'] = fields
if exclude is not None:
###########################################################################################
###########################################################################################
# For an generic inline, exclude is never None (at least ct_field.name and fk_field.name) #
###########################################################################################
###########################################################################################
attrs['exclude'] = exclude
# If parent form class already has an inner Meta, the Meta we're
# creating needs to inherit from the parent's inner meta.
#######################################################################################################
#######################################################################################################
# I think the problem is here: we are not "mixing" attrs['exclude'] with form.Meta.exclude. #
# attrs['exclude'] is overwriting the attribute form.Meta.exclude and the final Meta class is created #
# without the exclude information from the Meta class of the form. #
# I think that, if the generic inline does not provide an exclude list by itself, but have a custom #
# form that do this, the exclude from the Meta of this form should be honored. #
#######################################################################################################
#######################################################################################################
parent = (object,)
if hasattr(form, 'Meta'):
parent = (form.Meta, object)
Meta = type('Meta', parent, attrs)
# Give this new form class a reasonable name.
class_name = model.__name__ + 'Form'
# Class attributes for the new form class.
form_class_attrs = {
'Meta': Meta,
'formfield_callback': formfield_callback
}
return ModelFormMetaclass(class_name, (form,), form_class_attrs)
I've attached the project that I made in order to test this (if this help).
Attachments (7)
Change History (16)
by , 15 years ago
| Attachment: | generic_inline_bug.tar.bz2 added |
|---|
by , 15 years ago
| Attachment: | generic_inline_error.png added |
|---|
by , 15 years ago
| Attachment: | 15907_test_case_at_r16105.diff added |
|---|
comment:1 by , 15 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
I verified this bug occurs and added a patch with a test case.
follow-up: 4 comment:3 by , 15 years ago
I combined the patches with tests and fix into one. Leonel, your patch had lines to combine the fields attribute. I couldn't reproduce a case where fields ever showed up wrong, so I left that out. If there's a bug I'm missing it'd be cool to fix that as well.
comment:4 by , 15 years ago
Replying to prestontimmons:
I combined the patches with tests and fix into one. Leonel, your patch had lines to combine the fields attribute. I couldn't reproduce a case where fields ever showed up wrong, so I left that out. If there's a bug I'm missing it'd be cool to fix that as well.
OK. Thanks for the fix. ;-)
comment:5 by , 14 years ago
| Patch needs improvement: | set |
|---|---|
| UI/UX: | unset |
The attached patch is a bad fix. It conflicts with behavior expected in #8999, which depends on the exclude argument to modelformset_factory trumping exclude information in the ModelForm.Meta.
A better place to fix this is in generic_inlineformset_factory. When exclude information isn't passed in, combine the default contenttype fields with the model form meta fields.
by , 14 years ago
| Attachment: | 15907_at_16452.diff added |
|---|
Patch with fix moved to generic_inlineformset_factory
comment:6 by , 14 years ago
| Patch needs improvement: | unset |
|---|
by , 14 years ago
| Attachment: | 15907.generic-inline-admin-fix.diff added |
|---|
comment:7 by , 14 years ago
This new patch moves the changes into GenericInlineAdmin, and is parallel to changes in the patch for #14496 to ModelAdmin and InlineAdmin, providing consistency for how exclude is handled for all inlines.
comment:8 by , 14 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Test case to demonstrate bug