Skip to content

Commit

Permalink
Fixed #27673 -- Made admin's checks run at check time instead of duri…
Browse files Browse the repository at this point in the history
…ng registration.

Thanks Morgan Aubert for the test.
  • Loading branch information
Adam Chainz authored and timgraham committed Jan 10, 2017
1 parent ed8c0c9 commit d57ecf4
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 13 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Expand Up @@ -557,6 +557,7 @@ answer newbie questions, and generally made Django that much better:
mitakummaa@gmail.com
mmarshall
Moayad Mardini <moayad.m@gmail.com>
Morgan Aubert <morgan.aubert@zoho.com>
Moritz Sichert <moritz.sichert@googlemail.com>
Morten Bagai <m@bagai.com>
msaelices <msaelices@gmail.com>
Expand Down
10 changes: 6 additions & 4 deletions django/contrib/admin/checks.py
Expand Up @@ -18,10 +18,12 @@
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):
Expand Down
29 changes: 23 additions & 6 deletions 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
Expand All @@ -16,7 +17,7 @@
from django.views.decorators.csrf import csrf_protect
from django.views.i18n import JavaScriptCatalog

system_check_errors = []
all_sites = WeakSet()


class AlreadyRegistered(Exception):
Expand Down Expand Up @@ -63,6 +64,26 @@ def __init__(self, name='admin'):
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):
"""
Expand Down Expand Up @@ -105,11 +126,7 @@ def register(self, model_or_iterable, admin_class=None, **options):
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):
"""
Expand Down
27 changes: 24 additions & 3 deletions tests/admin_checks/tests.py
Expand Up @@ -7,7 +7,9 @@
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):
Expand Down Expand Up @@ -52,7 +54,6 @@ def test_checks_are_performed(self):
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):
Expand Down Expand Up @@ -105,7 +106,27 @@ class CustomAdminSite(admin.AdminSite):
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):
Expand Down

0 comments on commit d57ecf4

Please sign in to comment.