Fixed #25374 -- Made ModelAdmin checks work on instances instead of classes.

This allows dynamically-generated attributes to be specified in
checked ModelAdmin attributes without triggering errors.
This commit is contained in:
Malcolm Box 2015-09-10 14:05:31 +01:00 committed by Tim Graham
parent cf99bae53a
commit 1d8eb0cae5
8 changed files with 348 additions and 343 deletions

File diff suppressed because it is too large Load Diff

View File

@ -109,9 +109,8 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)):
show_full_result_count = True
checks_class = BaseModelAdminChecks
@classmethod
def check(cls, model, **kwargs):
return cls.checks_class().check(cls, model, **kwargs)
def check(self, **kwargs):
return self.checks_class().check(self, **kwargs)
def __init__(self):
overrides = FORMFIELD_FOR_DBFIELD_DEFAULTS.copy()

View File

@ -103,11 +103,12 @@ class AdminSite(object):
options['__module__'] = __name__
admin_class = type("%sAdmin" % model.__name__, (admin_class,), options)
if admin_class is not ModelAdmin and settings.DEBUG:
system_check_errors.extend(admin_class.check(model))
# Instantiate the admin class to save in the registry
self._registry[model] = admin_class(model, self)
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
def unregister(self, model_or_iterable):
"""

View File

@ -15,55 +15,55 @@ from django.forms.models import modelform_defines_fields
class GenericInlineModelAdminChecks(InlineModelAdminChecks):
def _check_exclude_of_parent_model(self, cls, parent_model):
def _check_exclude_of_parent_model(self, obj, parent_model):
# There's no FK to exclude, so no exclusion checks are required.
return []
def _check_relation(self, cls, parent_model):
def _check_relation(self, obj, parent_model):
# There's no FK, but we do need to confirm that the ct_field and ct_fk_field are valid,
# and that they are part of a GenericForeignKey.
gfks = [
f for f in cls.model._meta.virtual_fields
f for f in obj.model._meta.virtual_fields
if isinstance(f, GenericForeignKey)
]
if len(gfks) == 0:
return [
checks.Error(
"'%s.%s' has no GenericForeignKey." % (
cls.model._meta.app_label, cls.model._meta.object_name
obj.model._meta.app_label, obj.model._meta.object_name
),
hint=None,
obj=cls,
obj=obj.__class__,
id='admin.E301'
)
]
else:
# Check that the ct_field and ct_fk_fields exist
try:
cls.model._meta.get_field(cls.ct_field)
obj.model._meta.get_field(obj.ct_field)
except FieldDoesNotExist:
return [
checks.Error(
"'ct_field' references '%s', which is not a field on '%s.%s'." % (
cls.ct_field, cls.model._meta.app_label, cls.model._meta.object_name
obj.ct_field, obj.model._meta.app_label, obj.model._meta.object_name
),
hint=None,
obj=cls,
obj=obj.__class__,
id='admin.E302'
)
]
try:
cls.model._meta.get_field(cls.ct_fk_field)
obj.model._meta.get_field(obj.ct_fk_field)
except FieldDoesNotExist:
return [
checks.Error(
"'ct_fk_field' references '%s', which is not a field on '%s.%s'." % (
cls.ct_fk_field, cls.model._meta.app_label, cls.model._meta.object_name
obj.ct_fk_field, obj.model._meta.app_label, obj.model._meta.object_name
),
hint=None,
obj=cls,
obj=obj.__class__,
id='admin.E303'
)
]
@ -71,16 +71,16 @@ class GenericInlineModelAdminChecks(InlineModelAdminChecks):
# There's one or more GenericForeignKeys; make sure that one of them
# uses the right ct_field and ct_fk_field.
for gfk in gfks:
if gfk.ct_field == cls.ct_field and gfk.fk_field == cls.ct_fk_field:
if gfk.ct_field == obj.ct_field and gfk.fk_field == obj.ct_fk_field:
return []
return [
checks.Error(
"'%s.%s' has no GenericForeignKey using content type field '%s' and object ID field '%s'." % (
cls.model._meta.app_label, cls.model._meta.object_name, cls.ct_field, cls.ct_fk_field
obj.model._meta.app_label, obj.model._meta.object_name, obj.ct_field, obj.ct_fk_field
),
hint=None,
obj=cls,
obj=obj.__class__,
id='admin.E304'
)
]

View File

@ -1064,6 +1064,9 @@ Miscellaneous
* By default :class:`~django.test.LiveServerTestCase` attempts to find an
available port in the 8081-8179 range instead of just trying port 8081.
* The system checks for :class:`~django.contrib.admin.ModelAdmin` now check
instances rather than classes.
.. _deprecated-features-1.9:
Features deprecated in 1.9

View File

@ -2,6 +2,7 @@ from __future__ import unicode_literals
from django import forms
from django.contrib import admin
from django.contrib.admin import AdminSite
from django.contrib.contenttypes.admin import GenericStackedInline
from django.core import checks
from django.test import SimpleTestCase, override_settings
@ -32,8 +33,7 @@ class ValidFormFieldsets(admin.ModelAdmin):
class MyAdmin(admin.ModelAdmin):
@classmethod
def check(cls, model, **kwargs):
def check(self, **kwargs):
return ['error!']
@ -73,7 +73,7 @@ class SystemChecksTestCase(SimpleTestCase):
class SongAdmin(admin.ModelAdmin):
list_editable = ["original_release"]
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"The value of 'list_editable[0]' refers to 'original_release', "
@ -95,8 +95,7 @@ class SystemChecksTestCase(SimpleTestCase):
"fields": ["title", "original_release"],
}),
]
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
("The value of 'list_editable[0]' refers to 'original_release', "
@ -118,15 +117,14 @@ class SystemChecksTestCase(SimpleTestCase):
}),
]
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_custom_modelforms_with_fields_fieldsets(self):
"""
# Regression test for #8027: custom ModelForms with fields/fieldsets
"""
errors = ValidFields.check(model=Song)
errors = ValidFields(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_custom_get_form_with_fieldsets(self):
@ -135,8 +133,7 @@ class SystemChecksTestCase(SimpleTestCase):
is overridden.
Refs #19445.
"""
errors = ValidFormFieldsets.check(model=Song)
errors = ValidFormFieldsets(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_fieldsets_fields_non_tuple(self):
@ -152,7 +149,7 @@ class SystemChecksTestCase(SimpleTestCase):
}),
]
errors = NotATupleAdmin.check(model=Song)
errors = NotATupleAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"The value of 'fieldsets[0][1]['fields']' must be a list or tuple.",
@ -177,7 +174,7 @@ class SystemChecksTestCase(SimpleTestCase):
}),
]
errors = NotATupleAdmin.check(model=Song)
errors = NotATupleAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"The value of 'fieldsets[1][1]['fields']' must be a list or tuple.",
@ -192,11 +189,10 @@ class SystemChecksTestCase(SimpleTestCase):
"""
Tests for basic system checks of 'exclude' option values (#12689)
"""
class ExcludedFields1(admin.ModelAdmin):
exclude = 'foo'
errors = ExcludedFields1.check(model=Book)
errors = ExcludedFields1(Book, AdminSite()).check()
expected = [
checks.Error(
"The value of 'exclude' must be a list or tuple.",
@ -211,7 +207,7 @@ class SystemChecksTestCase(SimpleTestCase):
class ExcludedFields2(admin.ModelAdmin):
exclude = ('name', 'name')
errors = ExcludedFields2.check(model=Book)
errors = ExcludedFields2(Book, AdminSite()).check()
expected = [
checks.Error(
"The value of 'exclude' contains duplicate field(s).",
@ -231,7 +227,7 @@ class SystemChecksTestCase(SimpleTestCase):
model = Album
inlines = [ExcludedFieldsInline]
errors = ExcludedFieldsAlbumAdmin.check(model=Album)
errors = ExcludedFieldsAlbumAdmin(Album, AdminSite()).check()
expected = [
checks.Error(
"The value of 'exclude' must be a list or tuple.",
@ -247,7 +243,6 @@ class SystemChecksTestCase(SimpleTestCase):
Regression test for #9932 - exclude in InlineModelAdmin should not
contain the ForeignKey field used in ModelAdmin.model
"""
class SongInline(admin.StackedInline):
model = Song
exclude = ['album']
@ -256,7 +251,7 @@ class SystemChecksTestCase(SimpleTestCase):
model = Album
inlines = [SongInline]
errors = AlbumAdmin.check(model=Album)
errors = AlbumAdmin(Album, AdminSite()).check()
expected = [
checks.Error(
("Cannot exclude the field 'album', because it is the foreign key "
@ -273,14 +268,13 @@ class SystemChecksTestCase(SimpleTestCase):
Regression test for #22034 - check that generic inlines don't look for
normal ForeignKey relations.
"""
class InfluenceInline(GenericStackedInline):
model = Influence
class SongAdmin(admin.ModelAdmin):
inlines = [InfluenceInline]
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_generic_inline_model_admin_non_generic_model(self):
@ -288,14 +282,13 @@ class SystemChecksTestCase(SimpleTestCase):
Ensure that a model without a GenericForeignKey raises problems if it's included
in an GenericInlineModelAdmin definition.
"""
class BookInline(GenericStackedInline):
model = Book
class SongAdmin(admin.ModelAdmin):
inlines = [BookInline]
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"'admin_checks.Book' has no GenericForeignKey.",
@ -308,7 +301,6 @@ class SystemChecksTestCase(SimpleTestCase):
def test_generic_inline_model_admin_bad_ct_field(self):
"A GenericInlineModelAdmin raises problems if the ct_field points to a non-existent field."
class InfluenceInline(GenericStackedInline):
model = Influence
ct_field = 'nonexistent'
@ -316,7 +308,7 @@ class SystemChecksTestCase(SimpleTestCase):
class SongAdmin(admin.ModelAdmin):
inlines = [InfluenceInline]
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"'ct_field' references 'nonexistent', which is not a field on 'admin_checks.Influence'.",
@ -329,7 +321,6 @@ class SystemChecksTestCase(SimpleTestCase):
def test_generic_inline_model_admin_bad_fk_field(self):
"A GenericInlineModelAdmin raises problems if the ct_fk_field points to a non-existent field."
class InfluenceInline(GenericStackedInline):
model = Influence
ct_fk_field = 'nonexistent'
@ -337,7 +328,7 @@ class SystemChecksTestCase(SimpleTestCase):
class SongAdmin(admin.ModelAdmin):
inlines = [InfluenceInline]
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"'ct_fk_field' references 'nonexistent', which is not a field on 'admin_checks.Influence'.",
@ -350,7 +341,6 @@ class SystemChecksTestCase(SimpleTestCase):
def test_generic_inline_model_admin_non_gfk_ct_field(self):
"A GenericInlineModelAdmin raises problems if the ct_field points to a field that isn't part of a GenericForeignKey"
class InfluenceInline(GenericStackedInline):
model = Influence
ct_field = 'name'
@ -358,7 +348,7 @@ class SystemChecksTestCase(SimpleTestCase):
class SongAdmin(admin.ModelAdmin):
inlines = [InfluenceInline]
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"'admin_checks.Influence' has no GenericForeignKey using content type field 'name' and object ID field 'object_id'.",
@ -371,7 +361,6 @@ class SystemChecksTestCase(SimpleTestCase):
def test_generic_inline_model_admin_non_gfk_fk_field(self):
"A GenericInlineModelAdmin raises problems if the ct_fk_field points to a field that isn't part of a GenericForeignKey"
class InfluenceInline(GenericStackedInline):
model = Influence
ct_fk_field = 'name'
@ -379,7 +368,7 @@ class SystemChecksTestCase(SimpleTestCase):
class SongAdmin(admin.ModelAdmin):
inlines = [InfluenceInline]
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"'admin_checks.Influence' has no GenericForeignKey using content type field 'content_type' and object ID field 'name'.",
@ -394,11 +383,10 @@ class SystemChecksTestCase(SimpleTestCase):
"""
Regression test for #15669 - Include app label in admin system check messages
"""
class RawIdNonexistingAdmin(admin.ModelAdmin):
raw_id_fields = ('nonexisting',)
errors = RawIdNonexistingAdmin.check(model=Album)
errors = RawIdNonexistingAdmin(Album, AdminSite()).check()
expected = [
checks.Error(
("The value of 'raw_id_fields[0]' refers to 'nonexisting', which is "
@ -416,7 +404,6 @@ class SystemChecksTestCase(SimpleTestCase):
given) make sure fk_name is honored or things blow up when there is more
than one fk to the parent model.
"""
class TwoAlbumFKAndAnEInline(admin.TabularInline):
model = TwoAlbumFKAndAnE
exclude = ("e",)
@ -425,7 +412,7 @@ class SystemChecksTestCase(SimpleTestCase):
class MyAdmin(admin.ModelAdmin):
inlines = [TwoAlbumFKAndAnEInline]
errors = MyAdmin.check(model=Album)
errors = MyAdmin(Album, AdminSite()).check()
self.assertEqual(errors, [])
def test_inline_self_check(self):
@ -435,7 +422,7 @@ class SystemChecksTestCase(SimpleTestCase):
class MyAdmin(admin.ModelAdmin):
inlines = [TwoAlbumFKAndAnEInline]
errors = MyAdmin.check(model=Album)
errors = MyAdmin(Album, AdminSite()).check()
expected = [
checks.Error(
"'admin_checks.TwoAlbumFKAndAnE' has more than one ForeignKey to 'admin_checks.Album'.",
@ -454,14 +441,14 @@ class SystemChecksTestCase(SimpleTestCase):
class MyAdmin(admin.ModelAdmin):
inlines = [TwoAlbumFKAndAnEInline]
errors = MyAdmin.check(model=Album)
errors = MyAdmin(Album, AdminSite()).check()
self.assertEqual(errors, [])
def test_readonly(self):
class SongAdmin(admin.ModelAdmin):
readonly_fields = ("title",)
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_readonly_on_method(self):
@ -471,7 +458,7 @@ class SystemChecksTestCase(SimpleTestCase):
class SongAdmin(admin.ModelAdmin):
readonly_fields = (my_function,)
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_readonly_on_modeladmin(self):
@ -481,21 +468,35 @@ class SystemChecksTestCase(SimpleTestCase):
def readonly_method_on_modeladmin(self, obj):
pass
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_readonly_dynamic_attribute_on_modeladmin(self):
class SongAdmin(admin.ModelAdmin):
readonly_fields = ("dynamic_method",)
def __getattr__(self, item):
if item == "dynamic_method":
def method(obj):
pass
return method
raise AttributeError
errors = SongAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_readonly_method_on_model(self):
class SongAdmin(admin.ModelAdmin):
readonly_fields = ("readonly_method_on_model",)
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_nonexistent_field(self):
class SongAdmin(admin.ModelAdmin):
readonly_fields = ("title", "nonexistent")
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
("The value of 'readonly_fields[1]' is not a callable, an attribute "
@ -512,7 +513,7 @@ class SystemChecksTestCase(SimpleTestCase):
model = City
readonly_fields = ['i_dont_exist'] # Missing attribute
errors = CityInline.check(State)
errors = CityInline(State, AdminSite()).check()
expected = [
checks.Error(
("The value of 'readonly_fields[0]' is not a callable, an attribute "
@ -531,14 +532,14 @@ class SystemChecksTestCase(SimpleTestCase):
return "Best Ever!"
return "Status unknown."
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_readonly_lambda(self):
class SongAdmin(admin.ModelAdmin):
readonly_fields = (lambda obj: "test",)
errors = SongAdmin.check(model=Song)
errors = SongAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_graceful_m2m_fail(self):
@ -547,11 +548,10 @@ class SystemChecksTestCase(SimpleTestCase):
specifies the 'through' option is included in the 'fields' or the 'fieldsets'
ModelAdmin options.
"""
class BookAdmin(admin.ModelAdmin):
fields = ['authors']
errors = BookAdmin.check(model=Book)
errors = BookAdmin(Book, AdminSite()).check()
expected = [
checks.Error(
("The value of 'fields' cannot include the ManyToManyField 'authors', "
@ -570,7 +570,7 @@ class SystemChecksTestCase(SimpleTestCase):
('Header 2', {'fields': ('authors',)}),
)
errors = FieldsetBookAdmin.check(model=Book)
errors = FieldsetBookAdmin(Book, AdminSite()).check()
expected = [
checks.Error(
("The value of 'fieldsets[1][1][\"fields\"]' cannot include the ManyToManyField "
@ -586,7 +586,7 @@ class SystemChecksTestCase(SimpleTestCase):
class NestedFieldsAdmin(admin.ModelAdmin):
fields = ('price', ('name', 'subtitle'))
errors = NestedFieldsAdmin.check(model=Book)
errors = NestedFieldsAdmin(Book, AdminSite()).check()
self.assertEqual(errors, [])
def test_nested_fieldsets(self):
@ -595,7 +595,7 @@ class SystemChecksTestCase(SimpleTestCase):
('Main', {'fields': ('price', ('name', 'subtitle'))}),
)
errors = NestedFieldsetAdmin.check(model=Book)
errors = NestedFieldsetAdmin(Book, AdminSite()).check()
self.assertEqual(errors, [])
def test_explicit_through_override(self):
@ -604,14 +604,13 @@ class SystemChecksTestCase(SimpleTestCase):
is specified as a string, the admin should still be able use
Model.m2m_field.through
"""
class AuthorsInline(admin.TabularInline):
model = Book.authors.through
class BookAdmin(admin.ModelAdmin):
inlines = [AuthorsInline]
errors = BookAdmin.check(model=Book)
errors = BookAdmin(Book, AdminSite()).check()
self.assertEqual(errors, [])
def test_non_model_fields(self):
@ -619,7 +618,6 @@ class SystemChecksTestCase(SimpleTestCase):
Regression for ensuring ModelAdmin.fields can contain non-model fields
that broke with r11737
"""
class SongForm(forms.ModelForm):
extra_data = forms.CharField()
@ -627,7 +625,7 @@ class SystemChecksTestCase(SimpleTestCase):
form = SongForm
fields = ['title', 'extra_data']
errors = FieldsOnFormOnlyAdmin.check(model=Song)
errors = FieldsOnFormOnlyAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_non_model_first_field(self):
@ -635,7 +633,6 @@ class SystemChecksTestCase(SimpleTestCase):
Regression for ensuring ModelAdmin.field can handle first elem being a
non-model field (test fix for UnboundLocalError introduced with r16225).
"""
class SongForm(forms.ModelForm):
extra_data = forms.CharField()
@ -647,14 +644,14 @@ class SystemChecksTestCase(SimpleTestCase):
form = SongForm
fields = ['extra_data', 'title']
errors = FieldsOnFormOnlyAdmin.check(model=Song)
errors = FieldsOnFormOnlyAdmin(Song, AdminSite()).check()
self.assertEqual(errors, [])
def test_check_sublists_for_duplicates(self):
class MyModelAdmin(admin.ModelAdmin):
fields = ['state', ['state']]
errors = MyModelAdmin.check(model=Song)
errors = MyModelAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"The value of 'fields' contains duplicate field(s).",
@ -673,7 +670,7 @@ class SystemChecksTestCase(SimpleTestCase):
}),
]
errors = MyModelAdmin.check(model=Song)
errors = MyModelAdmin(Song, AdminSite()).check()
expected = [
checks.Error(
"There are duplicate field(s) in 'fieldsets[0][1]'.",
@ -696,7 +693,7 @@ class SystemChecksTestCase(SimpleTestCase):
# if the value of 'list_filter' refers to a 'through__field'.
Book._meta.apps.ready = False
try:
errors = BookAdminWithListFilter.check(model=Book)
errors = BookAdminWithListFilter(Book, AdminSite()).check()
self.assertEqual(errors, [])
finally:
Book._meta.apps.ready = True

View File

@ -6,7 +6,7 @@ import os
import re
import unittest
from django.contrib.admin import ModelAdmin
from django.contrib.admin import AdminSite, ModelAdmin
from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
from django.contrib.admin.models import ADDITION, DELETION, LogEntry
from django.contrib.admin.options import TO_FIELD_VAR
@ -6128,14 +6128,15 @@ class AdminViewOnSiteTests(TestCase):
def test_check(self):
"Ensure that the view_on_site value is either a boolean or a callable"
try:
admin = CityAdmin(City, AdminSite())
CityAdmin.view_on_site = True
self.assertEqual(CityAdmin.check(City), [])
self.assertEqual(admin.check(), [])
CityAdmin.view_on_site = False
self.assertEqual(CityAdmin.check(City), [])
self.assertEqual(admin.check(), [])
CityAdmin.view_on_site = lambda obj: obj.get_absolute_url()
self.assertEqual(CityAdmin.check(City), [])
self.assertEqual(admin.check(), [])
CityAdmin.view_on_site = []
self.assertEqual(CityAdmin.check(City), [
self.assertEqual(admin.check(), [
Error(
"The value of 'view_on_site' must be a callable or a boolean value.",
hint=None,

View File

@ -572,7 +572,8 @@ class CheckTestCase(SimpleTestCase):
def assertIsInvalid(self, model_admin, model, msg,
id=None, hint=None, invalid_obj=None):
invalid_obj = invalid_obj or model_admin
errors = model_admin.check(model=model)
admin_obj = model_admin(model, AdminSite())
errors = admin_obj.check()
expected = [
Error(
msg,
@ -589,7 +590,8 @@ class CheckTestCase(SimpleTestCase):
Same as assertIsInvalid but treats the given msg as a regexp.
"""
invalid_obj = invalid_obj or model_admin
errors = model_admin.check(model=model)
admin_obj = model_admin(model, AdminSite())
errors = admin_obj.check()
self.assertEqual(len(errors), 1)
error = errors[0]
self.assertEqual(error.hint, hint)
@ -598,7 +600,8 @@ class CheckTestCase(SimpleTestCase):
six.assertRegex(self, error.msg, msg)
def assertIsValid(self, model_admin, model):
errors = model_admin.check(model=model)
admin_obj = model_admin(model, AdminSite())
errors = admin_obj.check()
expected = []
self.assertEqual(errors, expected)