From d57ecf40ebad564fafa36825b17b6c15f13cfc01 Mon Sep 17 00:00:00 2001 From: Adam Chainz Date: Mon, 2 Jan 2017 16:42:27 -0500 Subject: [PATCH] Fixed #27673 -- Made admin's checks run at check time instead of during registration. Thanks Morgan Aubert for the test. --- AUTHORS | 1 + django/contrib/admin/checks.py | 10 ++++++---- django/contrib/admin/sites.py | 29 +++++++++++++++++++++++------ tests/admin_checks/tests.py | 27 ++++++++++++++++++++++++--- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/AUTHORS b/AUTHORS index 7b383d5d7d..c356dce59f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -557,6 +557,7 @@ answer newbie questions, and generally made Django that much better: mitakummaa@gmail.com mmarshall Moayad Mardini + Morgan Aubert Moritz Sichert Morten Bagai msaelices diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index 90f5396b19..286c1169a4 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -18,10 +18,12 @@ from django.forms.models import ( from django.template.engine import Engine -def check_admin_app(**kwargs): - from django.contrib.admin.sites import system_check_errors - - return system_check_errors +def check_admin_app(app_configs, **kwargs): + from django.contrib.admin.sites import all_sites + errors = [] + for site in all_sites: + errors.extend(site.check(app_configs)) + return errors def check_dependencies(**kwargs): diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index dfe91e23e1..d056ef337c 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -1,4 +1,5 @@ from functools import update_wrapper +from weakref import WeakSet from django.apps import apps from django.conf import settings @@ -16,7 +17,7 @@ from django.views.decorators.cache import never_cache from django.views.decorators.csrf import csrf_protect from django.views.i18n import JavaScriptCatalog -system_check_errors = [] +all_sites = WeakSet() class AlreadyRegistered(Exception): @@ -63,6 +64,26 @@ class AdminSite(object): self.name = name self._actions = {'delete_selected': actions.delete_selected} self._global_actions = self._actions.copy() + all_sites.add(self) + + def check(self, app_configs): + """ + Run the system checks on all ModelAdmins, except if they aren't + customized at all. + """ + if not settings.DEBUG: + return [] + + if app_configs is None: + app_configs = apps.get_app_configs() + app_configs = set(app_configs) # Speed up lookups below + + errors = [] + modeladmins = (o for o in self._registry.values() if o.__class__ is not ModelAdmin) + for modeladmin in modeladmins: + if modeladmin.model._meta.app_config in app_configs: + errors.extend(modeladmin.check()) + return errors def register(self, model_or_iterable, admin_class=None, **options): """ @@ -105,11 +126,7 @@ class AdminSite(object): admin_class = type("%sAdmin" % model.__name__, (admin_class,), options) # Instantiate the admin class to save in the registry - admin_obj = admin_class(model, self) - if admin_class is not ModelAdmin and settings.DEBUG: - system_check_errors.extend(admin_obj.check()) - - self._registry[model] = admin_obj + self._registry[model] = admin_class(model, self) def unregister(self, model_or_iterable): """ diff --git a/tests/admin_checks/tests.py b/tests/admin_checks/tests.py index fa1bcc1518..d243a56640 100644 --- a/tests/admin_checks/tests.py +++ b/tests/admin_checks/tests.py @@ -7,7 +7,9 @@ from django.contrib.contenttypes.admin import GenericStackedInline from django.core import checks from django.test import SimpleTestCase, override_settings -from .models import Album, Book, City, Influence, Song, State, TwoAlbumFKAndAnE +from .models import ( + Album, Author, Book, City, Influence, Song, State, TwoAlbumFKAndAnE, +) class SongForm(forms.ModelForm): @@ -52,7 +54,6 @@ class SystemChecksTestCase(SimpleTestCase): self.assertEqual(errors, expected) finally: admin.site.unregister(Song) - admin.sites.system_check_errors = [] @override_settings(INSTALLED_APPS=['django.contrib.admin']) def test_contenttypes_dependency(self): @@ -105,7 +106,27 @@ class SystemChecksTestCase(SimpleTestCase): self.assertEqual(errors, expected) finally: custom_site.unregister(Song) - admin.sites.system_check_errors = [] + + @override_settings(DEBUG=True) + def test_allows_checks_relying_on_other_modeladmins(self): + class MyBookAdmin(admin.ModelAdmin): + def check(self, **kwargs): + errors = super(MyBookAdmin, self).check(**kwargs) + author_admin = self.admin_site._registry.get(Author) + if author_admin is None: + errors.append('AuthorAdmin missing!') + return errors + + class MyAuthorAdmin(admin.ModelAdmin): + pass + + admin.site.register(Book, MyBookAdmin) + admin.site.register(Author, MyAuthorAdmin) + try: + self.assertEqual(admin.site.check(None), []) + finally: + admin.site.unregister(Book) + admin.site.unregister(Author) def test_field_name_not_in_list_display(self): class SongAdmin(admin.ModelAdmin):