Opened 5 years ago

Closed 4 years ago

#17428 closed Bug (wontfix)

Admin formfield validation uses form model instead of registered model

Reported by: David Bennett Owned by: dante87
Component: contrib.admin Version: 1.3
Severity: Normal Keywords:
Cc: Per Rosengren Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

The admin check_formfield validation function is checking for fields in the form's model (if specified) by default.

Meanwhile, the ModelAdmin get_form method always overrides the form's model with the registered model.

So, doing something like this generates a validation error saying that field2 is missing from the form:

class Base(models.Model):
    field1 = models.CharField(max_length=10)

class Sub(Base):
    field2 = models.CharField(max_length=10)

class BaseForm(forms.ModelForm):
    class Meta:
        model = Base

class BaseAdmin(admin.ModelAdmin):
    form = BaseForm

class SubAdmin(BaseAdmin):
    fields = ('field1', 'field2')

admin.site.register(Sub, SubAdmin)

It would appear to me that fields shouldn't be validated against the form's model as it isn't actually used.

I've managed to work around it with the following:

class SubAdmin(BaseAdmin):
    form = forms.ModelForm
    fields = ('field1', 'field2')

    def __init__(self, model, admin_site):
        self.form = BaseForm
        super(SubAdmin, self).__init__(model, admin_site)

This causes check_formfield to use the registered model's fields for validation, but then swaps in the original form on init.

Change History (6)

comment:1 Changed 5 years ago by Julien Phalip

Triage Stage: UnreviewedAccepted

comment:2 Changed 5 years ago by dante87

Owner: changed from nobody to dante87
Status: newassigned

comment:3 Changed 4 years ago by Per Rosengren

I submitted a fix for this in https://github.com/django/django/pull/528

It is my first contribution, so it probably has lots of problems.

My fix is this(see github for details and unit test):

def check_formfield(cls, model, opts, label, field):
    form = modelform_factory(model, form=cls.form)
    try:
        form.base_fields[field]

comment:4 Changed 4 years ago by Per Rosengren

Cc: Per Rosengren added
Has patch: set

comment:5 Changed 4 years ago by Jannis Leidel

Triage Stage: AcceptedReady for checkin

comment:6 Changed 4 years ago by Tim Graham

Resolution: wontfix
Status: assignedclosed

This validation appears to have been removed in [1556b1c3b7] and [1e37cb37cec].

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