Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#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


I have problem with contrib.admin application. Consider we have demo application with this three files:


    from django.db import models

    class Demo(models.Model):
        name = models.CharField(max_length=100)
        note = models.TextField(blank=True, null=True)


    from django.contrib import admin
    from models import Demo
    from forms import DemoAdminForm

    class DemoAdmin(admin.ModelAdmin):
        readonly_fields = ('name', )
        form = DemoAdminForm, DemoAdmin)


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

14496.custom-admin-form-exclude.diff (6.8 KB) - added by Julien Phalip 8 years ago.
14496.custom-admin-form-exclude.alternative.diff (6.0 KB) - added by Julien Phalip 8 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by Daniel F Moisset

Triage Stage: UnreviewedAccepted

comment:2 Changed 8 years ago by Julien Phalip

Component: Contrib appsdjango.contrib.admin

comment:3 Changed 8 years ago by Julien Phalip

Severity: Normal
Type: Bug

Changed 8 years ago by Julien Phalip

comment:4 Changed 8 years ago by Julien Phalip

Easy pickings: unset
Has patch: set

The reason this is happening is that django.forms.models.modelform_factory() systematically overrides the form's Meta.exclude attribute with the exclude kwarg that it receives. So, in this case, DemoAdminForm.Meta.exclude is overridden by ('name', ) which comes from the DemoAdmin.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 that ModelAdmin.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 in ModelAdmin.exclude and ModelAdmin.readonly_fields. The attached patch rectifies this behaviour both for ModelAdmin.get_form() and InlineModelAdmin.get_formset().

Changed 8 years ago by Julien Phalip

comment:5 Changed 8 years ago by Julien Phalip

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 Changed 8 years ago by Preston Timmons

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 Changed 8 years ago by Jannis Leidel

Triage Stage: AcceptedReady for checkin

comment:8 Changed 7 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [16602]:

Fixed #14496 -- Fixed conflict between ModelForm exclude and ModelAdmin readonly values. Thanks, Julien Phalip.

comment:9 Changed 7 years ago by anonymous

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 (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]
        return form
Note: See TracTickets for help on using tickets.
Back to Top