Fieldsets' validation doesn't call get_form

[Krzysztof Jurewicz]

In django.contrib.admin.validation, function check_formfield checks presence of a field based on "form" attribute on admin class. If get_form method is overriden and it returns form class which adds some additional fields, those cannot be used in "fieldsets" attribute because ImproperlyConfigured exception is raised.

The workaround is to use undocumented get_fieldsets method to specify additional fields instead of "fieldsets" attribute.

The easiest way to fix this would be probably to disable fieldsets' validation entirely (as we are not able to anticipate what will be the result of calling get_form) or to make get_fieldsets method a documented one and make a note that if we add fields in get_form, then we should not set "fieldsets" attribute.

[Russell Keith-Magee]

To be clear -- both the proposed solutions (documenting get_fieldsets(), and removing validation if get_form is defined) are worth implementing.

[Nick Sandford]

There was a pull request for this, but it was missing disabling validation when get_form is defined on the sub class of ModelAdmin. I've added a small fix for this and a test and opened a pull request here: ​https://github.com/django/django/pull/636.

[Julien Phalip <jphalip@…>]

In 0694d2196f0fadde37ff2d002a9a4a8edb3ca504:

Fixed #19445 -- Skip admin fieldsets validation when the ModelAdmin.get_form() method is overridden.

[Julien Phalip <jphalip@…>]

In e18bd68dbc64dfe08ae0fbb54591d5c8f7e456eb:

[1.5.x] Fixed #19445 -- Skip admin fieldsets validation when the ModelAdmin.get_form() method is overridden.
Backport of 0694d2196f0fad

[Julien Phalip <jphalip@…>]

In e18bd68dbc64dfe08ae0fbb54591d5c8f7e456eb:

[1.5.x] Fixed #19445 -- Skip admin fieldsets validation when the ModelAdmin.get_form() method is overridden.
Backport of 0694d2196f0fad

[Luke Plant]

This fix is, unfortunately, inadequate.

Even if it isn't overridden, get_form will return a ModelForm class that can be significantly different from the ModelAdmin.form attribute.

For example, the following code will work fine if you disable validation, but validation will fail:

class MyModel(models.Model):
    field1 = models.BooleanField()

class MyForm(forms.ModelForm):
    my_additional_field = forms.BooleanField()

class MyAdmin(admin.ModelAdmin):
    form = MyForm
    fieldsets = (
        ('Group', {
              'fields': ['field1', 'my_additional_field']
          }),
    )

This is because get_form creates a class based on MyForm, but not the same - it adds an inner Meta class, so it has a base_fields attribute that will work, while the statically defined one won't pass validation.

We can't even call get_form with a dummy request object in the validation method, because it could call an overridden get_readonly_fields which assumes a real request object.

And there are more bugs with the validation routine - for example if get_readonly_fields dynamically returns fields that don't exist, the validate routine won't catch it.

Basically, I don't think we can do any static validation of ModelAdmin.fieldsets etc. against the form, because we don't know what form will actually be used, because it is defined and generated dynamically, not statically. I can't see a way of fixing this.

BTW, I'm hitting this for my work on #19733.

[Luke Plant]

Discussion: ​https://groups.google.com/d/topic/django-developers/FhNM9ajjKGA/discussion

[Luke Plant <L.Plant.98@…>]

In 1556b1c3b7a18cadf4d66d2ab6301d7fe8646ba5:

Removed fragile admin validation of fields on ModelForm

Refs #19445

[Luke Plant]

This ticket should probably be considered closed now, although it is 'fixed' by skipping the validation, and just providing a more helpful error message later down the line if necessary.