From 1d8eb0cae57731b481a88dca272b2cb0d645bd8e Mon Sep 17 00:00:00 2001 From: Malcolm Box Date: Thu, 10 Sep 2015 14:05:31 +0100 Subject: [PATCH] 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. --- django/contrib/admin/checks.py | 513 ++++++++++++++------------- django/contrib/admin/options.py | 5 +- django/contrib/admin/sites.py | 9 +- django/contrib/contenttypes/admin.py | 28 +- docs/releases/1.9.txt | 3 + tests/admin_checks/tests.py | 113 +++--- tests/admin_views/tests.py | 11 +- tests/modeladmin/tests.py | 9 +- 8 files changed, 348 insertions(+), 343 deletions(-) diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index 62cce24af6..60bd236927 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -22,35 +22,35 @@ def check_admin_app(**kwargs): class BaseModelAdminChecks(object): - def check(self, cls, model, **kwargs): + def check(self, admin_obj, **kwargs): errors = [] - errors.extend(self._check_raw_id_fields(cls, model)) - errors.extend(self._check_fields(cls, model)) - errors.extend(self._check_fieldsets(cls, model)) - errors.extend(self._check_exclude(cls, model)) - errors.extend(self._check_form(cls, model)) - errors.extend(self._check_filter_vertical(cls, model)) - errors.extend(self._check_filter_horizontal(cls, model)) - errors.extend(self._check_radio_fields(cls, model)) - errors.extend(self._check_prepopulated_fields(cls, model)) - errors.extend(self._check_view_on_site_url(cls, model)) - errors.extend(self._check_ordering(cls, model)) - errors.extend(self._check_readonly_fields(cls, model)) + errors.extend(self._check_raw_id_fields(admin_obj)) + errors.extend(self._check_fields(admin_obj)) + errors.extend(self._check_fieldsets(admin_obj)) + errors.extend(self._check_exclude(admin_obj)) + errors.extend(self._check_form(admin_obj)) + errors.extend(self._check_filter_vertical(admin_obj)) + errors.extend(self._check_filter_horizontal(admin_obj)) + errors.extend(self._check_radio_fields(admin_obj)) + errors.extend(self._check_prepopulated_fields(admin_obj)) + errors.extend(self._check_view_on_site_url(admin_obj)) + errors.extend(self._check_ordering(admin_obj)) + errors.extend(self._check_readonly_fields(admin_obj)) return errors - def _check_raw_id_fields(self, cls, model): + def _check_raw_id_fields(self, obj): """ Check that `raw_id_fields` only contains field names that are listed on the model. """ - if not isinstance(cls.raw_id_fields, (list, tuple)): - return must_be('a list or tuple', option='raw_id_fields', obj=cls, id='admin.E001') + if not isinstance(obj.raw_id_fields, (list, tuple)): + return must_be('a list or tuple', option='raw_id_fields', obj=obj, id='admin.E001') else: return list(chain(*[ - self._check_raw_id_fields_item(cls, model, field_name, 'raw_id_fields[%d]' % index) - for index, field_name in enumerate(cls.raw_id_fields) + self._check_raw_id_fields_item(obj, obj.model, field_name, 'raw_id_fields[%d]' % index) + for index, field_name in enumerate(obj.raw_id_fields) ])) - def _check_raw_id_fields_item(self, cls, model, field_name, label): + def _check_raw_id_fields_item(self, obj, model, field_name, label): """ Check an item of `raw_id_fields`, i.e. check that field named `field_name` exists in model `model` and is a ForeignKey or a ManyToManyField. """ @@ -59,83 +59,83 @@ class BaseModelAdminChecks(object): field = model._meta.get_field(field_name) except FieldDoesNotExist: return refer_to_missing_field(field=field_name, option=label, - model=model, obj=cls, id='admin.E002') + model=model, obj=obj, id='admin.E002') else: if not isinstance(field, (models.ForeignKey, models.ManyToManyField)): return must_be('a ForeignKey or ManyToManyField', - option=label, obj=cls, id='admin.E003') + option=label, obj=obj, id='admin.E003') else: return [] - def _check_fields(self, cls, model): + def _check_fields(self, obj): """ Check that `fields` only refer to existing fields, doesn't contain duplicates. Check if at most one of `fields` and `fieldsets` is defined. """ - if cls.fields is None: + if obj.fields is None: return [] - elif not isinstance(cls.fields, (list, tuple)): - return must_be('a list or tuple', option='fields', obj=cls, id='admin.E004') - elif cls.fieldsets: + elif not isinstance(obj.fields, (list, tuple)): + return must_be('a list or tuple', option='fields', obj=obj, id='admin.E004') + elif obj.fieldsets: return [ checks.Error( "Both 'fieldsets' and 'fields' are specified.", hint=None, - obj=cls, + obj=obj.__class__, id='admin.E005', ) ] - fields = flatten(cls.fields) + fields = flatten(obj.fields) if len(fields) != len(set(fields)): return [ checks.Error( "The value of 'fields' contains duplicate field(s).", hint=None, - obj=cls, + obj=obj.__class__, id='admin.E006', ) ] return list(chain(*[ - self._check_field_spec(cls, model, field_name, 'fields') - for field_name in cls.fields + self._check_field_spec(obj, obj.model, field_name, 'fields') + for field_name in obj.fields ])) - def _check_fieldsets(self, cls, model): + def _check_fieldsets(self, obj): """ Check that fieldsets is properly formatted and doesn't contain duplicates. """ - if cls.fieldsets is None: + if obj.fieldsets is None: return [] - elif not isinstance(cls.fieldsets, (list, tuple)): - return must_be('a list or tuple', option='fieldsets', obj=cls, id='admin.E007') + elif not isinstance(obj.fieldsets, (list, tuple)): + return must_be('a list or tuple', option='fieldsets', obj=obj, id='admin.E007') else: return list(chain(*[ - self._check_fieldsets_item(cls, model, fieldset, 'fieldsets[%d]' % index) - for index, fieldset in enumerate(cls.fieldsets) + self._check_fieldsets_item(obj, obj.model, fieldset, 'fieldsets[%d]' % index) + for index, fieldset in enumerate(obj.fieldsets) ])) - def _check_fieldsets_item(self, cls, model, fieldset, label): + def _check_fieldsets_item(self, obj, model, fieldset, label): """ Check an item of `fieldsets`, i.e. check that this is a pair of a set name and a dictionary containing "fields" key. """ if not isinstance(fieldset, (list, tuple)): - return must_be('a list or tuple', option=label, obj=cls, id='admin.E008') + return must_be('a list or tuple', option=label, obj=obj, id='admin.E008') elif len(fieldset) != 2: - return must_be('of length 2', option=label, obj=cls, id='admin.E009') + return must_be('of length 2', option=label, obj=obj, id='admin.E009') elif not isinstance(fieldset[1], dict): - return must_be('a dictionary', option='%s[1]' % label, obj=cls, id='admin.E010') + return must_be('a dictionary', option='%s[1]' % label, obj=obj, id='admin.E010') elif 'fields' not in fieldset[1]: return [ checks.Error( "The value of '%s[1]' must contain the key 'fields'." % label, hint=None, - obj=cls, + obj=obj.__class__, id='admin.E011', ) ] elif not isinstance(fieldset[1]['fields'], (list, tuple)): - return must_be('a list or tuple', option="%s[1]['fields']" % label, obj=cls, id='admin.E008') + return must_be('a list or tuple', option="%s[1]['fields']" % label, obj=obj, id='admin.E008') fields = flatten(fieldset[1]['fields']) if len(fields) != len(set(fields)): @@ -143,30 +143,30 @@ class BaseModelAdminChecks(object): checks.Error( "There are duplicate field(s) in '%s[1]'." % label, hint=None, - obj=cls, + obj=obj.__class__, id='admin.E012', ) ] return list(chain(*[ - self._check_field_spec(cls, model, fieldset_fields, '%s[1]["fields"]' % label) + self._check_field_spec(obj, model, fieldset_fields, '%s[1]["fields"]' % label) for fieldset_fields in fieldset[1]['fields'] ])) - def _check_field_spec(self, cls, model, fields, label): + def _check_field_spec(self, obj, model, fields, label): """ `fields` should be an item of `fields` or an item of fieldset[1]['fields'] for any `fieldset` in `fieldsets`. It should be a field name or a tuple of field names. """ if isinstance(fields, tuple): return list(chain(*[ - self._check_field_spec_item(cls, model, field_name, "%s[%d]" % (label, index)) + self._check_field_spec_item(obj, model, field_name, "%s[%d]" % (label, index)) for index, field_name in enumerate(fields) ])) else: - return self._check_field_spec_item(cls, model, fields, label) + return self._check_field_spec_item(obj, model, fields, label) - def _check_field_spec_item(self, cls, model, field_name, label): - if field_name in cls.readonly_fields: + def _check_field_spec_item(self, obj, model, field_name, label): + if field_name in obj.readonly_fields: # Stuff can be put in fields that isn't actually a model field if # it's in readonly_fields, readonly_fields will handle the # validation of such things. @@ -187,68 +187,68 @@ class BaseModelAdminChecks(object): "because that field manually specifies a relationship model.") % (label, field_name), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E013', ) ] else: return [] - def _check_exclude(self, cls, model): + def _check_exclude(self, obj): """ Check that exclude is a sequence without duplicates. """ - if cls.exclude is None: # default value is None + if obj.exclude is None: # default value is None return [] - elif not isinstance(cls.exclude, (list, tuple)): - return must_be('a list or tuple', option='exclude', obj=cls, id='admin.E014') - elif len(cls.exclude) > len(set(cls.exclude)): + elif not isinstance(obj.exclude, (list, tuple)): + return must_be('a list or tuple', option='exclude', obj=obj, id='admin.E014') + elif len(obj.exclude) > len(set(obj.exclude)): return [ checks.Error( "The value of 'exclude' contains duplicate field(s).", hint=None, - obj=cls, + obj=obj.__class__, id='admin.E015', ) ] else: return [] - def _check_form(self, cls, model): + def _check_form(self, obj): """ Check that form subclasses BaseModelForm. """ - if hasattr(cls, 'form') and not issubclass(cls.form, BaseModelForm): + if hasattr(obj, 'form') and not issubclass(obj.form, BaseModelForm): return must_inherit_from(parent='BaseModelForm', option='form', - obj=cls, id='admin.E016') + obj=obj, id='admin.E016') else: return [] - def _check_filter_vertical(self, cls, model): + def _check_filter_vertical(self, obj): """ Check that filter_vertical is a sequence of field names. """ - if not hasattr(cls, 'filter_vertical'): + if not hasattr(obj, 'filter_vertical'): return [] - elif not isinstance(cls.filter_vertical, (list, tuple)): - return must_be('a list or tuple', option='filter_vertical', obj=cls, id='admin.E017') + elif not isinstance(obj.filter_vertical, (list, tuple)): + return must_be('a list or tuple', option='filter_vertical', obj=obj, id='admin.E017') else: return list(chain(*[ - self._check_filter_item(cls, model, field_name, "filter_vertical[%d]" % index) - for index, field_name in enumerate(cls.filter_vertical) + self._check_filter_item(obj, obj.model, field_name, "filter_vertical[%d]" % index) + for index, field_name in enumerate(obj.filter_vertical) ])) - def _check_filter_horizontal(self, cls, model): + def _check_filter_horizontal(self, obj): """ Check that filter_horizontal is a sequence of field names. """ - if not hasattr(cls, 'filter_horizontal'): + if not hasattr(obj, 'filter_horizontal'): return [] - elif not isinstance(cls.filter_horizontal, (list, tuple)): - return must_be('a list or tuple', option='filter_horizontal', obj=cls, id='admin.E018') + elif not isinstance(obj.filter_horizontal, (list, tuple)): + return must_be('a list or tuple', option='filter_horizontal', obj=obj, id='admin.E018') else: return list(chain(*[ - self._check_filter_item(cls, model, field_name, "filter_horizontal[%d]" % index) - for index, field_name in enumerate(cls.filter_horizontal) + self._check_filter_item(obj, obj.model, field_name, "filter_horizontal[%d]" % index) + for index, field_name in enumerate(obj.filter_horizontal) ])) - def _check_filter_item(self, cls, model, field_name, label): + def _check_filter_item(self, obj, model, field_name, label): """ Check one item of `filter_vertical` or `filter_horizontal`, i.e. check that given field exists and is a ManyToManyField. """ @@ -256,28 +256,28 @@ class BaseModelAdminChecks(object): field = model._meta.get_field(field_name) except FieldDoesNotExist: return refer_to_missing_field(field=field_name, option=label, - model=model, obj=cls, id='admin.E019') + model=model, obj=obj, id='admin.E019') else: if not isinstance(field, models.ManyToManyField): - return must_be('a ManyToManyField', option=label, obj=cls, id='admin.E020') + return must_be('a ManyToManyField', option=label, obj=obj, id='admin.E020') else: return [] - def _check_radio_fields(self, cls, model): + def _check_radio_fields(self, obj): """ Check that `radio_fields` is a dictionary. """ - if not hasattr(cls, 'radio_fields'): + if not hasattr(obj, 'radio_fields'): return [] - elif not isinstance(cls.radio_fields, dict): - return must_be('a dictionary', option='radio_fields', obj=cls, id='admin.E021') + elif not isinstance(obj.radio_fields, dict): + return must_be('a dictionary', option='radio_fields', obj=obj, id='admin.E021') else: return list(chain(*[ - self._check_radio_fields_key(cls, model, field_name, 'radio_fields') + - self._check_radio_fields_value(cls, model, val, 'radio_fields["%s"]' % field_name) - for field_name, val in cls.radio_fields.items() + self._check_radio_fields_key(obj, obj.model, field_name, 'radio_fields') + + self._check_radio_fields_value(obj, val, 'radio_fields["%s"]' % field_name) + for field_name, val in obj.radio_fields.items() ])) - def _check_radio_fields_key(self, cls, model, field_name, label): + def _check_radio_fields_key(self, obj, model, field_name, label): """ Check that a key of `radio_fields` dictionary is name of existing field and that the field is a ForeignKey or has `choices` defined. """ @@ -285,7 +285,7 @@ class BaseModelAdminChecks(object): field = model._meta.get_field(field_name) except FieldDoesNotExist: return refer_to_missing_field(field=field_name, option=label, - model=model, obj=cls, id='admin.E022') + model=model, obj=obj, id='admin.E022') else: if not (isinstance(field, models.ForeignKey) or field.choices): return [ @@ -295,14 +295,14 @@ class BaseModelAdminChecks(object): label, field_name ), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E023', ) ] else: return [] - def _check_radio_fields_value(self, cls, model, val, label): + def _check_radio_fields_value(self, obj, val, label): """ Check type of a value of `radio_fields` dictionary. """ from django.contrib.admin.options import HORIZONTAL, VERTICAL @@ -312,21 +312,21 @@ class BaseModelAdminChecks(object): checks.Error( "The value of '%s' must be either admin.HORIZONTAL or admin.VERTICAL." % label, hint=None, - obj=cls, + obj=obj.__class__, id='admin.E024', ) ] else: return [] - def _check_view_on_site_url(self, cls, model): - if hasattr(cls, 'view_on_site'): - if not callable(cls.view_on_site) and not isinstance(cls.view_on_site, bool): + def _check_view_on_site_url(self, obj): + if hasattr(obj, 'view_on_site'): + if not callable(obj.view_on_site) and not isinstance(obj.view_on_site, bool): return [ checks.Error( "The value of 'view_on_site' must be a callable or a boolean value.", hint=None, - obj=cls, + obj=obj.__class__, id='admin.E025', ) ] @@ -335,22 +335,22 @@ class BaseModelAdminChecks(object): else: return [] - def _check_prepopulated_fields(self, cls, model): + def _check_prepopulated_fields(self, obj): """ Check that `prepopulated_fields` is a dictionary containing allowed field types. """ - if not hasattr(cls, 'prepopulated_fields'): + if not hasattr(obj, 'prepopulated_fields'): return [] - elif not isinstance(cls.prepopulated_fields, dict): - return must_be('a dictionary', option='prepopulated_fields', obj=cls, id='admin.E026') + elif not isinstance(obj.prepopulated_fields, dict): + return must_be('a dictionary', option='prepopulated_fields', obj=obj, id='admin.E026') else: return list(chain(*[ - self._check_prepopulated_fields_key(cls, model, field_name, 'prepopulated_fields') + - self._check_prepopulated_fields_value(cls, model, val, 'prepopulated_fields["%s"]' % field_name) - for field_name, val in cls.prepopulated_fields.items() + self._check_prepopulated_fields_key(obj, obj.model, field_name, 'prepopulated_fields') + + self._check_prepopulated_fields_value(obj, obj.model, val, 'prepopulated_fields["%s"]' % field_name) + for field_name, val in obj.prepopulated_fields.items() ])) - def _check_prepopulated_fields_key(self, cls, model, field_name, label): + def _check_prepopulated_fields_key(self, obj, model, field_name, label): """ Check a key of `prepopulated_fields` dictionary, i.e. check that it is a name of existing field and the field is one of the allowed types. """ @@ -365,7 +365,7 @@ class BaseModelAdminChecks(object): field = model._meta.get_field(field_name) except FieldDoesNotExist: return refer_to_missing_field(field=field_name, option=label, - model=model, obj=cls, id='admin.E027') + model=model, obj=obj, id='admin.E027') else: if isinstance(field, forbidden_field_types): return [ @@ -375,26 +375,26 @@ class BaseModelAdminChecks(object): label, field_name ), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E028', ) ] else: return [] - def _check_prepopulated_fields_value(self, cls, model, val, label): + def _check_prepopulated_fields_value(self, obj, model, val, label): """ Check a value of `prepopulated_fields` dictionary, i.e. it's an iterable of existing fields. """ if not isinstance(val, (list, tuple)): - return must_be('a list or tuple', option=label, obj=cls, id='admin.E029') + return must_be('a list or tuple', option=label, obj=obj, id='admin.E029') else: return list(chain(*[ - self._check_prepopulated_fields_value_item(cls, model, subfield_name, "%s[%r]" % (label, index)) + self._check_prepopulated_fields_value_item(obj, model, subfield_name, "%s[%r]" % (label, index)) for index, subfield_name in enumerate(val) ])) - def _check_prepopulated_fields_value_item(self, cls, model, field_name, label): + def _check_prepopulated_fields_value_item(self, obj, model, field_name, label): """ For `prepopulated_fields` equal to {"slug": ("title",)}, `field_name` is "title". """ @@ -402,34 +402,34 @@ class BaseModelAdminChecks(object): model._meta.get_field(field_name) except FieldDoesNotExist: return refer_to_missing_field(field=field_name, option=label, - model=model, obj=cls, id='admin.E030') + model=model, obj=obj, id='admin.E030') else: return [] - def _check_ordering(self, cls, model): + def _check_ordering(self, obj): """ Check that ordering refers to existing fields or is random. """ # ordering = None - if cls.ordering is None: # The default value is None + if obj.ordering is None: # The default value is None return [] - elif not isinstance(cls.ordering, (list, tuple)): - return must_be('a list or tuple', option='ordering', obj=cls, id='admin.E031') + elif not isinstance(obj.ordering, (list, tuple)): + return must_be('a list or tuple', option='ordering', obj=obj, id='admin.E031') else: return list(chain(*[ - self._check_ordering_item(cls, model, field_name, 'ordering[%d]' % index) - for index, field_name in enumerate(cls.ordering) + self._check_ordering_item(obj, obj.model, field_name, 'ordering[%d]' % index) + for index, field_name in enumerate(obj.ordering) ])) - def _check_ordering_item(self, cls, model, field_name, label): + def _check_ordering_item(self, obj, model, field_name, label): """ Check that `ordering` refers to existing fields. """ - if field_name == '?' and len(cls.ordering) != 1: + if field_name == '?' and len(obj.ordering) != 1: return [ checks.Error( ("The value of 'ordering' has the random ordering marker '?', " "but contains other fields as well."), hint='Either remove the "?", or remove the other fields.', - obj=cls, + obj=obj.__class__, id='admin.E032', ) ] @@ -447,27 +447,27 @@ class BaseModelAdminChecks(object): model._meta.get_field(field_name) except FieldDoesNotExist: return refer_to_missing_field(field=field_name, option=label, - model=model, obj=cls, id='admin.E033') + model=model, obj=obj, id='admin.E033') else: return [] - def _check_readonly_fields(self, cls, model): + def _check_readonly_fields(self, obj): """ Check that readonly_fields refers to proper attribute or field. """ - if cls.readonly_fields == (): + if obj.readonly_fields == (): return [] - elif not isinstance(cls.readonly_fields, (list, tuple)): - return must_be('a list or tuple', option='readonly_fields', obj=cls, id='admin.E034') + elif not isinstance(obj.readonly_fields, (list, tuple)): + return must_be('a list or tuple', option='readonly_fields', obj=obj, id='admin.E034') else: return list(chain(*[ - self._check_readonly_fields_item(cls, model, field_name, "readonly_fields[%d]" % index) - for index, field_name in enumerate(cls.readonly_fields) + self._check_readonly_fields_item(obj, obj.model, field_name, "readonly_fields[%d]" % index) + for index, field_name in enumerate(obj.readonly_fields) ])) - def _check_readonly_fields_item(self, cls, model, field_name, label): + def _check_readonly_fields_item(self, obj, model, field_name, label): if callable(field_name): return [] - elif hasattr(cls, field_name): + elif hasattr(obj, field_name): return [] elif hasattr(model, field_name): return [] @@ -478,10 +478,10 @@ class BaseModelAdminChecks(object): return [ checks.Error( "The value of '%s' is not a callable, an attribute of '%s', or an attribute of '%s.%s'." % ( - label, cls.__name__, model._meta.app_label, model._meta.object_name + label, obj.__class__.__name__, model._meta.app_label, model._meta.object_name ), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E035', ) ] @@ -491,52 +491,52 @@ class BaseModelAdminChecks(object): class ModelAdminChecks(BaseModelAdminChecks): - def check(self, cls, model, **kwargs): - errors = super(ModelAdminChecks, self).check(cls, model=model, **kwargs) - errors.extend(self._check_save_as(cls, model)) - errors.extend(self._check_save_on_top(cls, model)) - errors.extend(self._check_inlines(cls, model)) - errors.extend(self._check_list_display(cls, model)) - errors.extend(self._check_list_display_links(cls, model)) - errors.extend(self._check_list_filter(cls, model)) - errors.extend(self._check_list_select_related(cls, model)) - errors.extend(self._check_list_per_page(cls, model)) - errors.extend(self._check_list_max_show_all(cls, model)) - errors.extend(self._check_list_editable(cls, model)) - errors.extend(self._check_search_fields(cls, model)) - errors.extend(self._check_date_hierarchy(cls, model)) + def check(self, admin_obj, **kwargs): + errors = super(ModelAdminChecks, self).check(admin_obj) + errors.extend(self._check_save_as(admin_obj)) + errors.extend(self._check_save_on_top(admin_obj)) + errors.extend(self._check_inlines(admin_obj)) + errors.extend(self._check_list_display(admin_obj)) + errors.extend(self._check_list_display_links(admin_obj)) + errors.extend(self._check_list_filter(admin_obj)) + errors.extend(self._check_list_select_related(admin_obj)) + errors.extend(self._check_list_per_page(admin_obj)) + errors.extend(self._check_list_max_show_all(admin_obj)) + errors.extend(self._check_list_editable(admin_obj)) + errors.extend(self._check_search_fields(admin_obj)) + errors.extend(self._check_date_hierarchy(admin_obj)) return errors - def _check_save_as(self, cls, model): + def _check_save_as(self, obj): """ Check save_as is a boolean. """ - if not isinstance(cls.save_as, bool): + if not isinstance(obj.save_as, bool): return must_be('a boolean', option='save_as', - obj=cls, id='admin.E101') + obj=obj, id='admin.E101') else: return [] - def _check_save_on_top(self, cls, model): + def _check_save_on_top(self, obj): """ Check save_on_top is a boolean. """ - if not isinstance(cls.save_on_top, bool): + if not isinstance(obj.save_on_top, bool): return must_be('a boolean', option='save_on_top', - obj=cls, id='admin.E102') + obj=obj, id='admin.E102') else: return [] - def _check_inlines(self, cls, model): + def _check_inlines(self, obj): """ Check all inline model admin classes. """ - if not isinstance(cls.inlines, (list, tuple)): - return must_be('a list or tuple', option='inlines', obj=cls, id='admin.E103') + if not isinstance(obj.inlines, (list, tuple)): + return must_be('a list or tuple', option='inlines', obj=obj, id='admin.E103') else: return list(chain(*[ - self._check_inlines_item(cls, model, item, "inlines[%d]" % index) - for index, item in enumerate(cls.inlines) + self._check_inlines_item(obj, obj.model, item, "inlines[%d]" % index) + for index, item in enumerate(obj.inlines) ])) - def _check_inlines_item(self, cls, model, inline, label): + def _check_inlines_item(self, obj, model, inline, label): """ Check one inline model admin. """ inline_label = '.'.join([inline.__module__, inline.__name__]) @@ -547,7 +547,7 @@ class ModelAdminChecks(BaseModelAdminChecks): checks.Error( "'%s' must inherit from 'BaseModelAdmin'." % inline_label, hint=None, - obj=cls, + obj=obj.__class__, id='admin.E104', ) ] @@ -556,32 +556,32 @@ class ModelAdminChecks(BaseModelAdminChecks): checks.Error( "'%s' must have a 'model' attribute." % inline_label, hint=None, - obj=cls, + obj=obj.__class__, id='admin.E105', ) ] elif not issubclass(inline.model, models.Model): return must_be('a Model', option='%s.model' % inline_label, - obj=cls, id='admin.E106') + obj=obj, id='admin.E106') else: - return inline.check(model) + return inline(model, obj.admin_site).check() - def _check_list_display(self, cls, model): + def _check_list_display(self, obj): """ Check that list_display only contains fields or usable attributes. """ - if not isinstance(cls.list_display, (list, tuple)): - return must_be('a list or tuple', option='list_display', obj=cls, id='admin.E107') + if not isinstance(obj.list_display, (list, tuple)): + return must_be('a list or tuple', option='list_display', obj=obj, id='admin.E107') else: return list(chain(*[ - self._check_list_display_item(cls, model, item, "list_display[%d]" % index) - for index, item in enumerate(cls.list_display) + self._check_list_display_item(obj, obj.model, item, "list_display[%d]" % index) + for index, item in enumerate(obj.list_display) ])) - def _check_list_display_item(self, cls, model, item, label): + def _check_list_display_item(self, obj, model, item, label): if callable(item): return [] - elif hasattr(cls, item): + elif hasattr(obj, item): return [] elif hasattr(model, item): # getattr(model, item) could be an X_RelatedObjectsDescriptor @@ -598,10 +598,10 @@ class ModelAdminChecks(BaseModelAdminChecks): checks.Error( "The value of '%s' refers to '%s', which is not a " "callable, an attribute of '%s', or an attribute or method on '%s.%s'." % ( - label, item, cls.__name__, model._meta.app_label, model._meta.object_name + label, item, obj.__class__.__name__, model._meta.app_label, model._meta.object_name ), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E108', ) ] @@ -610,7 +610,7 @@ class ModelAdminChecks(BaseModelAdminChecks): checks.Error( "The value of '%s' must not be a ManyToManyField." % label, hint=None, - obj=cls, + obj=obj.__class__, id='admin.E109', ) ] @@ -626,55 +626,55 @@ class ModelAdminChecks(BaseModelAdminChecks): checks.Error( "The value of '%s' refers to '%s', which is not a callable, " "an attribute of '%s', or an attribute or method on '%s.%s'." % ( - label, item, cls.__name__, model._meta.app_label, model._meta.object_name + label, item, obj.__class__.__name__, model._meta.app_label, model._meta.object_name ), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E108', ) ] else: return [] - def _check_list_display_links(self, cls, model): + def _check_list_display_links(self, obj): """ Check that list_display_links is a unique subset of list_display. """ - if cls.list_display_links is None: + if obj.list_display_links is None: return [] - elif not isinstance(cls.list_display_links, (list, tuple)): - return must_be('a list, a tuple, or None', option='list_display_links', obj=cls, id='admin.E110') + elif not isinstance(obj.list_display_links, (list, tuple)): + return must_be('a list, a tuple, or None', option='list_display_links', obj=obj, id='admin.E110') else: return list(chain(*[ - self._check_list_display_links_item(cls, model, field_name, "list_display_links[%d]" % index) - for index, field_name in enumerate(cls.list_display_links) + self._check_list_display_links_item(obj, field_name, "list_display_links[%d]" % index) + for index, field_name in enumerate(obj.list_display_links) ])) - def _check_list_display_links_item(self, cls, model, field_name, label): - if field_name not in cls.list_display: + def _check_list_display_links_item(self, obj, field_name, label): + if field_name not in obj.list_display: return [ checks.Error( "The value of '%s' refers to '%s', which is not defined in 'list_display'." % ( label, field_name ), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E111', ) ] else: return [] - def _check_list_filter(self, cls, model): - if not isinstance(cls.list_filter, (list, tuple)): - return must_be('a list or tuple', option='list_filter', obj=cls, id='admin.E112') + def _check_list_filter(self, obj): + if not isinstance(obj.list_filter, (list, tuple)): + return must_be('a list or tuple', option='list_filter', obj=obj, id='admin.E112') else: return list(chain(*[ - self._check_list_filter_item(cls, model, item, "list_filter[%d]" % index) - for index, item in enumerate(cls.list_filter) + self._check_list_filter_item(obj, obj.model, item, "list_filter[%d]" % index) + for index, item in enumerate(obj.list_filter) ])) - def _check_list_filter_item(self, cls, model, item, label): + def _check_list_filter_item(self, obj, model, item, label): """ Check one item of `list_filter`, i.e. check if it is one of three options: 1. 'field' -- a basic field filter, possibly w/ relationships (e.g. @@ -689,14 +689,14 @@ class ModelAdminChecks(BaseModelAdminChecks): # If item is option 3, it should be a ListFilter... if not issubclass(item, ListFilter): return must_inherit_from(parent='ListFilter', option=label, - obj=cls, id='admin.E113') + obj=obj, id='admin.E113') # ... but not a FieldListFilter. elif issubclass(item, FieldListFilter): return [ checks.Error( "The value of '%s' must not inherit from 'FieldListFilter'." % label, hint=None, - obj=cls, + obj=obj.__class__, id='admin.E114', ) ] @@ -707,7 +707,7 @@ class ModelAdminChecks(BaseModelAdminChecks): field, list_filter_class = item if not issubclass(list_filter_class, FieldListFilter): return must_inherit_from(parent='FieldListFilter', option='%s[1]' % label, - obj=cls, id='admin.E115') + obj=obj, id='admin.E115') else: return [] else: @@ -722,88 +722,88 @@ class ModelAdminChecks(BaseModelAdminChecks): checks.Error( "The value of '%s' refers to '%s', which does not refer to a Field." % (label, field), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E116', ) ] else: return [] - def _check_list_select_related(self, cls, model): + def _check_list_select_related(self, obj): """ Check that list_select_related is a boolean, a list or a tuple. """ - if not isinstance(cls.list_select_related, (bool, list, tuple)): + if not isinstance(obj.list_select_related, (bool, list, tuple)): return must_be('a boolean, tuple or list', option='list_select_related', - obj=cls, id='admin.E117') + obj=obj, id='admin.E117') else: return [] - def _check_list_per_page(self, cls, model): + def _check_list_per_page(self, obj): """ Check that list_per_page is an integer. """ - if not isinstance(cls.list_per_page, int): - return must_be('an integer', option='list_per_page', obj=cls, id='admin.E118') + if not isinstance(obj.list_per_page, int): + return must_be('an integer', option='list_per_page', obj=obj, id='admin.E118') else: return [] - def _check_list_max_show_all(self, cls, model): + def _check_list_max_show_all(self, obj): """ Check that list_max_show_all is an integer. """ - if not isinstance(cls.list_max_show_all, int): - return must_be('an integer', option='list_max_show_all', obj=cls, id='admin.E119') + if not isinstance(obj.list_max_show_all, int): + return must_be('an integer', option='list_max_show_all', obj=obj, id='admin.E119') else: return [] - def _check_list_editable(self, cls, model): + def _check_list_editable(self, obj): """ Check that list_editable is a sequence of editable fields from list_display without first element. """ - if not isinstance(cls.list_editable, (list, tuple)): - return must_be('a list or tuple', option='list_editable', obj=cls, id='admin.E120') + if not isinstance(obj.list_editable, (list, tuple)): + return must_be('a list or tuple', option='list_editable', obj=obj, id='admin.E120') else: return list(chain(*[ - self._check_list_editable_item(cls, model, item, "list_editable[%d]" % index) - for index, item in enumerate(cls.list_editable) + self._check_list_editable_item(obj, obj.model, item, "list_editable[%d]" % index) + for index, item in enumerate(obj.list_editable) ])) - def _check_list_editable_item(self, cls, model, field_name, label): + def _check_list_editable_item(self, obj, model, field_name, label): try: field = model._meta.get_field(field_name) except FieldDoesNotExist: return refer_to_missing_field(field=field_name, option=label, - model=model, obj=cls, id='admin.E121') + model=model, obj=obj, id='admin.E121') else: - if field_name not in cls.list_display: + if field_name not in obj.list_display: return [ checks.Error( "The value of '%s' refers to '%s', which is not " "contained in 'list_display'." % (label, field_name), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E122', ) ] - elif cls.list_display_links and field_name in cls.list_display_links: + elif obj.list_display_links and field_name in obj.list_display_links: return [ checks.Error( "The value of '%s' cannot be in both 'list_editable' and 'list_display_links'." % field_name, hint=None, - obj=cls, + obj=obj.__class__, id='admin.E123', ) ] # Check that list_display_links is set, and that the first values of list_editable and list_display are # not the same. See ticket #22792 for the use case relating to this. - elif (cls.list_display[0] in cls.list_editable and cls.list_display[0] != cls.list_editable[0] and - cls.list_display_links is not None): + elif (obj.list_display[0] in obj.list_editable and obj.list_display[0] != obj.list_editable[0] and + obj.list_display_links is not None): return [ checks.Error( "The value of '%s' refers to the first field in 'list_display' ('%s'), " "which cannot be used unless 'list_display_links' is set." % ( - label, cls.list_display[0] + label, obj.list_display[0] ), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E124', ) ] @@ -814,69 +814,70 @@ class ModelAdminChecks(BaseModelAdminChecks): label, field_name ), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E125', ) ] else: return [] - def _check_search_fields(self, cls, model): + def _check_search_fields(self, obj): """ Check search_fields is a sequence. """ - if not isinstance(cls.search_fields, (list, tuple)): - return must_be('a list or tuple', option='search_fields', obj=cls, id='admin.E126') + if not isinstance(obj.search_fields, (list, tuple)): + return must_be('a list or tuple', option='search_fields', obj=obj, id='admin.E126') else: return [] - def _check_date_hierarchy(self, cls, model): + def _check_date_hierarchy(self, obj): """ Check that date_hierarchy refers to DateField or DateTimeField. """ - if cls.date_hierarchy is None: + if obj.date_hierarchy is None: return [] else: try: - field = model._meta.get_field(cls.date_hierarchy) + field = obj.model._meta.get_field(obj.date_hierarchy) except FieldDoesNotExist: return refer_to_missing_field(option='date_hierarchy', - field=cls.date_hierarchy, - model=model, obj=cls, id='admin.E127') + field=obj.date_hierarchy, + model=obj.model, obj=obj, id='admin.E127') else: if not isinstance(field, (models.DateField, models.DateTimeField)): return must_be('a DateField or DateTimeField', option='date_hierarchy', - obj=cls, id='admin.E128') + obj=obj, id='admin.E128') else: return [] class InlineModelAdminChecks(BaseModelAdminChecks): - def check(self, cls, parent_model, **kwargs): - errors = super(InlineModelAdminChecks, self).check(cls, model=cls.model, **kwargs) - errors.extend(self._check_relation(cls, parent_model)) - errors.extend(self._check_exclude_of_parent_model(cls, parent_model)) - errors.extend(self._check_extra(cls)) - errors.extend(self._check_max_num(cls)) - errors.extend(self._check_min_num(cls)) - errors.extend(self._check_formset(cls)) + def check(self, inline_obj, **kwargs): + errors = super(InlineModelAdminChecks, self).check(inline_obj) + parent_model = inline_obj.parent_model + errors.extend(self._check_relation(inline_obj, parent_model)) + errors.extend(self._check_exclude_of_parent_model(inline_obj, parent_model)) + errors.extend(self._check_extra(inline_obj)) + errors.extend(self._check_max_num(inline_obj)) + errors.extend(self._check_min_num(inline_obj)) + errors.extend(self._check_formset(inline_obj)) return errors - def _check_exclude_of_parent_model(self, cls, parent_model): + def _check_exclude_of_parent_model(self, obj, parent_model): # Do not perform more specific checks if the base checks result in an # error. - errors = super(InlineModelAdminChecks, self)._check_exclude(cls, parent_model) + errors = super(InlineModelAdminChecks, self)._check_exclude(obj) if errors: return [] # Skip if `fk_name` is invalid. - if self._check_relation(cls, parent_model): + if self._check_relation(obj, parent_model): return [] - if cls.exclude is None: + if obj.exclude is None: return [] - fk = _get_foreign_key(parent_model, cls.model, fk_name=cls.fk_name) - if fk.name in cls.exclude: + fk = _get_foreign_key(parent_model, obj.model, fk_name=obj.fk_name) + if fk.name in obj.exclude: return [ checks.Error( "Cannot exclude the field '%s', because it is the foreign key " @@ -884,55 +885,55 @@ class InlineModelAdminChecks(BaseModelAdminChecks): fk.name, parent_model._meta.app_label, parent_model._meta.object_name ), hint=None, - obj=cls, + obj=obj.__class__, id='admin.E201', ) ] else: return [] - def _check_relation(self, cls, parent_model): + def _check_relation(self, obj, parent_model): try: - _get_foreign_key(parent_model, cls.model, fk_name=cls.fk_name) + _get_foreign_key(parent_model, obj.model, fk_name=obj.fk_name) except ValueError as e: - return [checks.Error(e.args[0], hint=None, obj=cls, id='admin.E202')] + return [checks.Error(e.args[0], hint=None, obj=obj.__class__, id='admin.E202')] else: return [] - def _check_extra(self, cls): + def _check_extra(self, obj): """ Check that extra is an integer. """ - if not isinstance(cls.extra, int): - return must_be('an integer', option='extra', obj=cls, id='admin.E203') + if not isinstance(obj.extra, int): + return must_be('an integer', option='extra', obj=obj, id='admin.E203') else: return [] - def _check_max_num(self, cls): + def _check_max_num(self, obj): """ Check that max_num is an integer. """ - if cls.max_num is None: + if obj.max_num is None: return [] - elif not isinstance(cls.max_num, int): - return must_be('an integer', option='max_num', obj=cls, id='admin.E204') + elif not isinstance(obj.max_num, int): + return must_be('an integer', option='max_num', obj=obj, id='admin.E204') else: return [] - def _check_min_num(self, cls): + def _check_min_num(self, obj): """ Check that min_num is an integer. """ - if cls.min_num is None: + if obj.min_num is None: return [] - elif not isinstance(cls.min_num, int): - return must_be('an integer', option='min_num', obj=cls, id='admin.E205') + elif not isinstance(obj.min_num, int): + return must_be('an integer', option='min_num', obj=obj, id='admin.E205') else: return [] - def _check_formset(self, cls): + def _check_formset(self, obj): """ Check formset is a subclass of BaseModelFormSet. """ - if not issubclass(cls.formset, BaseModelFormSet): + if not issubclass(obj.formset, BaseModelFormSet): return must_inherit_from(parent='BaseModelFormSet', option='formset', - obj=cls, id='admin.E206') + obj=obj, id='admin.E206') else: return [] @@ -942,7 +943,7 @@ def must_be(type, option, obj, id): checks.Error( "The value of '%s' must be %s." % (option, type), hint=None, - obj=obj, + obj=obj.__class__, id=id, ), ] @@ -953,7 +954,7 @@ def must_inherit_from(parent, option, obj, id): checks.Error( "The value of '%s' must inherit from '%s'." % (option, parent), hint=None, - obj=obj, + obj=obj.__class__, id=id, ), ] @@ -966,7 +967,7 @@ def refer_to_missing_field(field, option, model, obj, id): option, field, model._meta.app_label, model._meta.object_name ), hint=None, - obj=obj, + obj=obj.__class__, id=id, ), ] diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 348a56a4f0..6ccd3d8014 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -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() diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index 4f08cc19a0..39f22b8d0e 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -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): """ diff --git a/django/contrib/contenttypes/admin.py b/django/contrib/contenttypes/admin.py index 85bf3227e3..0756ce5787 100644 --- a/django/contrib/contenttypes/admin.py +++ b/django/contrib/contenttypes/admin.py @@ -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' ) ] diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 3d256ae29d..463ef62688 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -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 diff --git a/tests/admin_checks/tests.py b/tests/admin_checks/tests.py index 84f4439fe2..4544ddd382 100644 --- a/tests/admin_checks/tests.py +++ b/tests/admin_checks/tests.py @@ -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 diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 942f033b6f..fbeca604d4 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -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, diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index 918daffe3e..beeb493560 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -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)