#14496 closed Bug (fixed)
Conflict between ModelForm.Meta.exclude and ModelAdmin.readonly attributes
Reported by: | msgre_valise | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | 1.2 |
Severity: | Normal | Keywords: | ModelForm ModelAdmin readonly_fields exclude conflict |
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
I have problem with contrib.admin application. Consider we have demo application with this three files:
demo/models.py:
from django.db import models class Demo(models.Model): name = models.CharField(max_length=100) note = models.TextField(blank=True, null=True)
demo/admin.py:
from django.contrib import admin from models import Demo from forms import DemoAdminForm class DemoAdmin(admin.ModelAdmin): readonly_fields = ('name', ) form = DemoAdminForm admin.site.register(Demo, DemoAdmin)
demo/forms.py:
from django import forms from models import Demo class DemoAdminForm(forms.ModelForm): class Meta: model = Demo exclude = ('note', )
If I go to administration, and edit some Demo object, I see two fields: editable Note and readonly Name. But! I should see only readonly Name (because I use own ModelForm class which excluded note field). Try to comment line "readonly_fields = ('name', )" at admin.py file. You will see, that in this situation (no readonly attribute on ModelAdmin) Django correctly accept Meta class on ModelForm and exclude field Note.
I think that there is a problem in the way, how Django construct ModelForm class. It don't respect Meta classes in Form, if there is at same time readonly attribute on ModelAdmin.
Attachments (2)
Change History (11)
comment:1 by , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 14 years ago
Component: | Contrib apps → django.contrib.admin |
---|
comment:3 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
by , 14 years ago
Attachment: | 14496.custom-admin-form-exclude.diff added |
---|
comment:4 by , 14 years ago
Easy pickings: | unset |
---|---|
Has patch: | set |
by , 14 years ago
Attachment: | 14496.custom-admin-form-exclude.alternative.diff added |
---|
comment:5 by , 14 years ago
I've had second thoughts about this and posted an alternative patch.
I still think this ticket is a valid bug. The confusion mostly comes from the fact that defining readonly_fields
makes the ModelAdmin
ignore the ModelForm
's Meta.exclude
option even if the ModelAdmin
doesn't define its own exclude
. Both patches fix that bug.
However, while the first patch systematically considers the ModelForm.Meta.exclude
option, the new patch only considers it if ModelAdmin
doesn't define its own exclude
. If ModelAdmin
does define exclude
, then it takes precedence. This is a way of transferring control from ModelForm
to ModelAdmin
, which seems like a more explicit way of handling things.
comment:6 by , 13 years ago
UI/UX: | unset |
---|
The patch and tests look good. It'll be nice to have the precedence of exclude options defined and documented.
I also added a parallel patch for generic inlines at #15907.
comment:7 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:9 by , 13 years ago
In case someone wants code to work around this, put this in your ModelAdmin subclass:
def get_form(self, request, obj=None, **kwargs): """Work around https://code.djangoproject.com/ticket/14496 (not fixed in Django 1.3)""" form = super(OffcutAdminRestricted, self).get_form(request, obj, **kwargs) for e in self.form._meta.exclude: if e in form.base_fields: del form.base_fields[e] form._meta.exclude.append(e) return form
The reason this is happening is that
django.forms.models.modelform_factory()
systematically overrides the form'sMeta.exclude
attribute with theexclude
kwarg that it receives. So, in this case,DemoAdminForm.Meta.exclude
is overridden by('name', )
which comes from theDemoAdmin.readonly_fields
declaration, and the "note" field is therefore included in the form.modelform_factory
's behaviour in itself is actually correct, but the problem is thatModelAdmin.get_form()
does not take it properly into account.Like the reporter I think it makes sense to assume that, if a custom form is provided, then that form's
exclude
option should be considered and those fields should be excluded from the generated form, as well as the fields defined inModelAdmin.exclude
andModelAdmin.readonly_fields
. The attached patch rectifies this behaviour both forModelAdmin.get_form()
andInlineModelAdmin.get_formset()
.