Opened 2 years ago

Last modified 2 years ago

#27157 new Bug

AdminForm may crash if model_admin argument is None

Reported by: tpazderka Owned by: nobody
Component: contrib.admin Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using AdminForm without the keyword argument model_admin which defaults to None, we get AttributeError by calling get_empty_value_display without checking for the model_admin type.
This seems to be also true for AdminReadonlyField class.

Change History (3)

comment:1 Changed 2 years ago by Tim Graham

Can you give some example code that causes this error? AdminForm isn't a public API, and I'm not sure if you're calling it directly or triggering the problem some other way.

comment:2 Changed 2 years ago by tpazderka

This is just a stub as our class is not based on BaseFormset but rather reimplements its methods...
I am not sure if this is sufficient. If not, I will try to write more complete example.

class ContactFormset(BaseFormset):
    @property
    def forms(self):
         """
         Iterates through forms and returns them wrapped by `AdminForm`.
         """
         for form, datatype in zip(self._forms, self.get_types()):
             yield AdminForm(form, (self.get_fieldset(datatype), ), {}, self.get_readonly_fields(datatype))

The last line has to be replaced by:

yield AdminForm(form, (self.get_fieldset(datatype), ), {}, self.get_readonly_fields(datatype),  model_admin=ModelAdmin(MojeidUser, AdminSite))

in order to work in django 1.9 and later.

comment:3 Changed 2 years ago by Tim Graham

Summary: AdminForm model_admin cannot be NoneAdminForm may crash if model_admin argument is None
Triage Stage: UnreviewedAccepted

I guess, for example, we need to account for model_admin=None in a place like AdminReadonlyField. It's not so clear to me why model_admin is an optional argument (see bcd9482a2019158f4580c24cd50ee8bfae9b2739), perhaps for backwards compatibility. A better resolution might be a deprecation path toward making those arguments required unless there's argument against it. In your case, if you using that formset for the admin, then a possibility would be to declare the formset in ModelAdmin.get_changelist_formset() so you can access to the ModelAdmin class (self).

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