Simplify overriding `login_form` in AdminSite

[Rémi Lapeyre]

When setting login_form of AdminSite to a subclass of AdminAuthenticationForm an AppRegistryNotReady exception gets raised as documented in ​https://github.com/django/django/blob/master/django/contrib/admin/sites.py#L377.

This problem makes it harder to customize the administration site as it requires to override the whole login method of AdminSite to bypass it.

The attached patch use import_string to let users specify the class to use as a string and import it at runtime.

[Carlton Gibson]

I'm afraid I'm not seeing how this comes up at all.

The expected usage is to subclass AdminSite and to set login_form in your class definition, importing your custom form class there.
If you do this, there is no need to override login().

This is ​demonstrated by the test suite:

class Admin2(admin.AdminSite):
    ...
    login_form = forms.CustomAdminAuthenticationForm

And exercised by ​`tests.admin_views.tests.CustomModelAdminTest.test_custom_admin_site_login_form()`.

This was introduced in cc64fb5c4b4315a4ad66e21458e27ece57266847 for #8342, which was part of Django v1.3.

If you can provide information demonstrating an issue here we can reassess but pending that I'm going to close.

[Rémi Lapeyre]

Hi, I made a small example project at ​https://github.com/remilapeyre/bug_29508

When starting django, we get the following error:

➜  bug_29508 git:(master) pipenv run bug_29508/manage.py runserver
Unhandled exception in thread started by <function check_errors.<locals>.wrapper at 0x1023eb0d0>
Traceback (most recent call last):
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/utils/autoreload.py", line 225, in wrapper
    fn(*args, **kwargs)
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/core/management/commands/runserver.py", line 112, in inner_run
    autoreload.raise_last_exception()
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/utils/autoreload.py", line 248, in raise_last_exception
    raise _exception[1]
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/core/management/__init__.py", line 327, in execute
    autoreload.check_errors(django.setup)()
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/utils/autoreload.py", line 225, in wrapper
    fn(*args, **kwargs)
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/apps/registry.py", line 89, in populate
    app_config = AppConfig.create(entry)
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/apps/config.py", line 90, in create
    module = import_module(entry)
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 994, in _gcd_import
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/Users/remi/home/src/bug_29508/bug_29508/myadmin/__init__.py", line 1, in <module>
    from .sites import site
  File "/Users/remi/home/src/bug_29508/bug_29508/myadmin/sites.py", line 2, in <module>
    from .forms import AdminAuthenticationForm
  File "/Users/remi/home/src/bug_29508/bug_29508/myadmin/forms.py", line 2, in <module>
    from django.contrib.admin import forms as admin_forms
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/contrib/admin/forms.py", line 2, in <module>
    from django.contrib.auth.forms import AuthenticationForm, PasswordChangeForm
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/contrib/auth/forms.py", line 10, in <module>
    from django.contrib.auth.models import User
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/contrib/auth/models.py", line 2, in <module>
    from django.contrib.auth.base_user import AbstractBaseUser, BaseUserManager
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/contrib/auth/base_user.py", line 47, in <module>
    class AbstractBaseUser(models.Model):
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/db/models/base.py", line 100, in __new__
    app_config = apps.get_containing_app_config(module)
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/apps/registry.py", line 244, in get_containing_app_config
    self.check_apps_ready()
  File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/apps/registry.py", line 127, in check_apps_ready
    raise AppRegistryNotReady("Apps aren't loaded yet.")
django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.

The culprit is ​https://github.com/remilapeyre/bug_29508/blob/master/bug_29508/myadmin/__init__.py#L1 that will try to load django.contrib.auth.models.User through django.contrib.auth.forms.AuthenticationForm to add a placeholder in the Authentication form before the apps are registred.

Of course, we could remove the singleton from there but the customized admin would then be less friendly to use than the vanilla one as you could not do myadmin.site.register(MyModel) like we usually can.

I'm not sure why this behavior does not happen in the current tests, maybe apps are already registred when the override of the settings occurs.

If there is anything wrong in the example project that I missed, please point me to it but it's Django's starter project with just a few modifications (​https://github.com/remilapeyre/bug_29508/commit/ef75a02aa4eee09062106801502ba8129d8754b1).

[Tim Graham]

Right, you can't load any models in an app's __init__.py. You could empty that file and use references like this:

from myadmin.sites import site
from django.urls import path

urlpatterns = [
    path('admin/', site.urls),
]

[Rémi Lapeyre]

Right, you can't load any models in an app's init.py. You could empty that file and use references like this

Yes, I understand that but isn't it weird that we can't get the exact same API when extending the admin? It seems more dev-friendly and less confusing to be able to replace admin.site.register(MyModel) by myadmin.site.register(MyModel) even if not everybody do this.

The proposed patch does not increase the complexity of AdminSite but allows for this usage.

[Carlton Gibson]

Hi Rémi.

I don't think there's anything you can't do here. The only constraint is that you don't use models at import time.

If you keep your register calls in admin.py and route the URLs as Tim suggests, keeping your __init__.py files clean, you will be able to do what you want avoiding the AppRegistryNotReady exception.