AdminForm may crash if model_admin argument is None

[tpazderka]

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.

[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.

[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.

[Tim Graham]

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

[Shawn Zivontsis]

model_admin can't be required because it is set to None in django/contrib/auth/admin.py for use on the change password form: ​https://github.com/django/django/blob/90429625a85f1f77dfea200c91bd2dabab57974f/django/contrib/auth/admin.py#L207. However, that form doesn't use any readonly fields, so it doesn't exhibit this issue.

I realize this is not intended to be a public API but it would be nice to be able to use this for rendering properly stylized custom pages in the admin site.

[Sanghyuk Jeong]

I'd like to work on this ticket.
As Tim suggested in comment:3, I'm taking the deprecation path approach rather than simply adding None guards.
Changes:

• AdminForm.__init__ and AdminReadonlyField.__init__ now emit RemovedInDjango70Warning when model_admin=None, with a fallback to AdminSite.empty_value_display (default "-") so existing code doesn't crash immediately.
• AdminReadonlyField.get_admin_url gracefully handles model_admin=None by passing current_app=None to reverse().
• Fixed django/contrib/auth/admin.py (the change password form) to pass model_admin=self instead of relying on the None default, as noted in comment:4 by Shawn Zivontsis.

This sets up a clean path toward making model_admin a required argument in Django 7.0.
PR: ​https://github.com/django/django/pull/20689

[blighj]

I've recommened updates to the way deprecation is been done on the attached PR. I've also questioned if we need a deprecation cycle at all, since this is a private api.
The counter to that is people may be using this in the wild, I found this on the ​forum which would make a case for the deprecation cycle, but also for extending the guarding and not deprecating at all.
If anyone has opinons on this please chime in.