Skip to content

Commit

Permalink
Fixed #29282 -- Prevented some admin checks from crashing with TypeEr…
Browse files Browse the repository at this point in the history
…ror.

Co-authored-by: David Sanders <dsanders11@ucsbalum.com>
  • Loading branch information
2 people authored and timgraham committed Nov 20, 2018
1 parent ced0bdd commit a7d6cab
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 12 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,7 @@ answer newbie questions, and generally made Django that much better:
Sam Newman <http://www.magpiebrain.com/>
Sander Dijkhuis <sander.dijkhuis@gmail.com>
Sanket Saurav <sanketsaurav@gmail.com>
Sanyam Khurana <sanyam.khurana01@gmail.com>
Sarthak Mehrish <sarthakmeh03@gmail.com>
schwank@gmail.com
Scot Hacker <shacker@birdhouse.org>
Expand Down
34 changes: 27 additions & 7 deletions django/contrib/admin/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@
from django.utils.inspect import get_func_args


def _issubclass(cls, classinfo):
"""
issubclass() variant that doesn't raise an exception if cls isn't a
class.
"""
try:
return issubclass(cls, classinfo)
except TypeError:
return False


def check_admin_app(app_configs, **kwargs):
from django.contrib.admin.sites import all_sites
errors = []
Expand Down Expand Up @@ -341,7 +352,7 @@ def _check_exclude(self, obj):

def _check_form(self, obj):
""" Check that form subclasses BaseModelForm. """
if not issubclass(obj.form, BaseModelForm):
if not _issubclass(obj.form, BaseModelForm):
return must_inherit_from(parent='BaseModelForm', option='form',
obj=obj, id='admin.E016')
else:
Expand Down Expand Up @@ -640,11 +651,20 @@ def _check_inlines(self, obj):

def _check_inlines_item(self, obj, inline, label):
""" Check one inline model admin. """
inline_label = inline.__module__ + '.' + inline.__name__
try:
inline_label = inline.__module__ + '.' + inline.__name__
except AttributeError:
return [
checks.Error(
"'%s' must inherit from 'InlineModelAdmin'." % obj,
obj=obj.__class__,
id='admin.E104',
)
]

from django.contrib.admin.options import InlineModelAdmin

if not issubclass(inline, InlineModelAdmin):
if not _issubclass(inline, InlineModelAdmin):
return [
checks.Error(
"'%s' must inherit from 'InlineModelAdmin'." % inline_label,
Expand All @@ -660,7 +680,7 @@ def _check_inlines_item(self, obj, inline, label):
id='admin.E105',
)
]
elif not issubclass(inline.model, models.Model):
elif not _issubclass(inline.model, models.Model):
return must_be('a Model', option='%s.model' % inline_label, obj=obj, id='admin.E106')
else:
return inline(obj.model, obj.admin_site).check()
Expand Down Expand Up @@ -763,7 +783,7 @@ def _check_list_filter_item(self, obj, item, label):

if callable(item) and not isinstance(item, models.Field):
# If item is option 3, it should be a ListFilter...
if not issubclass(item, ListFilter):
if not _issubclass(item, ListFilter):
return must_inherit_from(parent='ListFilter', option=label,
obj=obj, id='admin.E113')
# ... but not a FieldListFilter.
Expand All @@ -780,7 +800,7 @@ def _check_list_filter_item(self, obj, item, label):
elif isinstance(item, (tuple, list)):
# item is option #2
field, list_filter_class = item
if not issubclass(list_filter_class, FieldListFilter):
if not _issubclass(list_filter_class, FieldListFilter):
return must_inherit_from(parent='FieldListFilter', option='%s[1]' % label, obj=obj, id='admin.E115')
else:
return []
Expand Down Expand Up @@ -1041,7 +1061,7 @@ def _check_min_num(self, obj):
def _check_formset(self, obj):
""" Check formset is a subclass of BaseModelFormSet. """

if not issubclass(obj.formset, BaseModelFormSet):
if not _issubclass(obj.formset, BaseModelFormSet):
return must_inherit_from(parent='BaseModelFormSet', option='formset', obj=obj, id='admin.E206')
else:
return []
Expand Down
113 changes: 108 additions & 5 deletions tests/modeladmin/test_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,16 @@ class FakeForm:
class TestModelAdmin(ModelAdmin):
form = FakeForm

self.assertIsInvalid(
TestModelAdmin, ValidationTestModel,
"The value of 'form' must inherit from 'BaseModelForm'.",
'admin.E016'
)
class TestModelAdminWithNoForm(ModelAdmin):
form = 'not a form'

for model_admin in (TestModelAdmin, TestModelAdminWithNoForm):
with self.subTest(model_admin):
self.assertIsInvalid(
model_admin, ValidationTestModel,
"The value of 'form' must inherit from 'BaseModelForm'.",
'admin.E016'
)

def test_fieldsets_with_custom_form_validation(self):

Expand Down Expand Up @@ -598,6 +603,40 @@ class TestModelAdmin(ModelAdmin):
'admin.E112'
)

def test_not_list_filter_class(self):
class TestModelAdmin(ModelAdmin):
list_filter = ['RandomClass']

self.assertIsInvalid(
TestModelAdmin, ValidationTestModel,
"The value of 'list_filter[0]' refers to 'RandomClass', which "
"does not refer to a Field.",
'admin.E116'
)

def test_callable(self):
def random_callable():
pass

class TestModelAdmin(ModelAdmin):
list_filter = [random_callable]

self.assertIsInvalid(
TestModelAdmin, ValidationTestModel,
"The value of 'list_filter[0]' must inherit from 'ListFilter'.",
'admin.E113'
)

def test_not_callable(self):
class TestModelAdmin(ModelAdmin):
list_filter = [[42, 42]]

self.assertIsInvalid(
TestModelAdmin, ValidationTestModel,
"The value of 'list_filter[0][1]' must inherit from 'FieldListFilter'.",
'admin.E115'
)

def test_missing_field(self):
class TestModelAdmin(ModelAdmin):
list_filter = ('non_existent_field',)
Expand Down Expand Up @@ -655,6 +694,19 @@ class TestModelAdmin(ModelAdmin):
'admin.E115'
)

def test_list_filter_is_func(self):
def get_filter():
pass

class TestModelAdmin(ModelAdmin):
list_filter = [get_filter]

self.assertIsInvalid(
TestModelAdmin, ValidationTestModel,
"The value of 'list_filter[0]' must inherit from 'ListFilter'.",
'admin.E113'
)

def test_not_associated_with_field_name(self):
class TestModelAdmin(ModelAdmin):
list_filter = (BooleanFieldListFilter,)
Expand Down Expand Up @@ -918,6 +970,16 @@ class TestModelAdmin(ModelAdmin):
'admin.E103'
)

def test_not_correct_inline_field(self):
class TestModelAdmin(ModelAdmin):
inlines = [42]

self.assertIsInvalidRegexp(
TestModelAdmin, ValidationTestModel,
r"'.*\.TestModelAdmin' must inherit from 'InlineModelAdmin'\.",
'admin.E104'
)

def test_not_model_admin(self):
class ValidationTestInline:
pass
Expand Down Expand Up @@ -960,6 +1022,32 @@ class TestModelAdmin(ModelAdmin):
'admin.E106'
)

def test_invalid_model(self):
class ValidationTestInline(TabularInline):
model = 'Not a class'

class TestModelAdmin(ModelAdmin):
inlines = [ValidationTestInline]

self.assertIsInvalidRegexp(
TestModelAdmin, ValidationTestModel,
r"The value of '.*\.ValidationTestInline.model' must be a Model\.",
'admin.E106'
)

def test_invalid_callable(self):
def random_obj():
pass

class TestModelAdmin(ModelAdmin):
inlines = [random_obj]

self.assertIsInvalidRegexp(
TestModelAdmin, ValidationTestModel,
r"'.*\.random_obj' must inherit from 'InlineModelAdmin'\.",
'admin.E104'
)

def test_valid_case(self):
class ValidationTestInline(TabularInline):
model = ValidationTestInlineModel
Expand Down Expand Up @@ -1102,6 +1190,21 @@ class TestModelAdmin(ModelAdmin):
invalid_obj=ValidationTestInline
)

def test_inline_without_formset_class(self):
class ValidationTestInlineWithoutFormsetClass(TabularInline):
model = ValidationTestInlineModel
formset = 'Not a FormSet Class'

class TestModelAdminWithoutFormsetClass(ModelAdmin):
inlines = [ValidationTestInlineWithoutFormsetClass]

self.assertIsInvalid(
TestModelAdminWithoutFormsetClass, ValidationTestModel,
"The value of 'formset' must inherit from 'BaseModelFormSet'.",
'admin.E206',
invalid_obj=ValidationTestInlineWithoutFormsetClass
)

def test_valid_case(self):
class RealModelFormSet(BaseModelFormSet):
pass
Expand Down

0 comments on commit a7d6cab

Please sign in to comment.