Index: django/core/management/validation.py =================================================================== --- django/core/management/validation.py (revision 7884) +++ django/core/management/validation.py (working copy) @@ -143,59 +143,6 @@ if r.get_accessor_name() == rel_query_name: e.add(opts, "Reverse query name for m2m field '%s' clashes with related field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.get_accessor_name(), f.name)) - # Check admin attribute. - if opts.admin is not None: - # prepopulated_fields - if not isinstance(opts.admin.prepopulated_fields, dict): - e.add(opts, '"%s": prepopulated_fields should be a dictionary.' % f.name) - else: - for field_name, field_list in opts.admin.prepopulated_fields.items(): - if not isinstance(field_list, (list, tuple)): - e.add(opts, '"%s": prepopulated_fields "%s" value should be a list or tuple.' % (f.name, field_name)) - - # list_display - if not isinstance(opts.admin.list_display, (list, tuple)): - e.add(opts, '"admin.list_display", if given, must be set to a list or tuple.') - else: - for fn in opts.admin.list_display: - try: - f = opts.get_field(fn) - except models.FieldDoesNotExist: - if not hasattr(cls, fn): - e.add(opts, '"admin.list_display" refers to %r, which isn\'t an attribute, method or property.' % fn) - else: - if isinstance(f, models.ManyToManyField): - e.add(opts, '"admin.list_display" doesn\'t support ManyToManyFields (%r).' % fn) - # list_display_links - if opts.admin.list_display_links and not opts.admin.list_display: - e.add(opts, '"admin.list_display" must be defined for "admin.list_display_links" to be used.') - if not isinstance(opts.admin.list_display_links, (list, tuple)): - e.add(opts, '"admin.list_display_links", if given, must be set to a list or tuple.') - else: - for fn in opts.admin.list_display_links: - try: - f = opts.get_field(fn) - except models.FieldDoesNotExist: - if not hasattr(cls, fn): - e.add(opts, '"admin.list_display_links" refers to %r, which isn\'t an attribute, method or property.' % fn) - if fn not in opts.admin.list_display: - e.add(opts, '"admin.list_display_links" refers to %r, which is not defined in "admin.list_display".' % fn) - # list_filter - if not isinstance(opts.admin.list_filter, (list, tuple)): - e.add(opts, '"admin.list_filter", if given, must be set to a list or tuple.') - else: - for fn in opts.admin.list_filter: - try: - f = opts.get_field(fn) - except models.FieldDoesNotExist: - e.add(opts, '"admin.list_filter" refers to %r, which isn\'t a field.' % fn) - # date_hierarchy - if opts.admin.date_hierarchy: - try: - f = opts.get_field(opts.admin.date_hierarchy) - except models.FieldDoesNotExist: - e.add(opts, '"admin.date_hierarchy" refers to %r, which isn\'t a field.' % opts.admin.date_hierarchy) - # Check ordering attribute. if opts.ordering: for field_name in opts.ordering: @@ -213,18 +160,6 @@ except models.FieldDoesNotExist: e.add(opts, '"ordering" refers to "%s", a field that doesn\'t exist.' % field_name) - # Check core=True, if needed. - for related in opts.get_followed_related_objects(): - if not related.edit_inline: - continue - try: - for f in related.opts.fields: - if f.core: - raise StopIteration - e.add(related.opts, "At least one field in %s should have core=True, because it's being edited inline by %s.%s." % (related.opts.object_name, opts.module_name, opts.object_name)) - except StopIteration: - pass - # Check unique_together. for ut in opts.unique_together: for field_name in ut: Index: django/contrib/admin/validation.py =================================================================== --- django/contrib/admin/validation.py (revision 0) +++ django/contrib/admin/validation.py (revision 0) @@ -0,0 +1,289 @@ +# Possible improvements: gather all errors and raise in the end instead of +# bailing out on the first one (like core/management/validation.py does) +# --- +# Another issue brought up by jkocherhans: +# mrts: we're probably not going to be able to design a nice +# system for validation before 1.0, so a function that does the validation is +# probably fine... no need to over-engieer anything or provide hook for custom +# classes to allow their own validation. +# it doesn't hurt though... +# I'd argue that it *does* hurt though, cause then we're +# committing to an api even if it isn't documented, people will use it +# should I remove the calls to _call_validation_hook(cls, model) then? +# mrts: I'd say yes, but you should probably get a second opinion + +from django.core.exceptions import ImproperlyConfigured +from django.db import models +from django.newforms.forms import BaseForm +from django.newforms.formsets import BaseFormSet +from django.contrib.admin.options import flatten_fieldsets, BaseModelAdmin +from django.contrib.admin.options import HORIZONTAL, VERTICAL + +def validate(cls, model): + """ + Does basic ModelAdmin option validation. Calls custom validation + classmethod in the end if it is provided in cls. The signature of the + custom validation classmethod should be: def validate(cls, model). + """ + opts = model._meta + _validate_base(cls, model) + + # currying is expensive, use wrappers instead + def _check_istuplew(label, obj): + _check_istuple(cls, label, obj) + + def _check_isdictw(label, obj): + _check_isdict(cls, label, obj) + + def _check_field_existsw(label, field): + return _check_field_exists(cls, model, opts, label, field) + + def _check_attr_existsw(label, field): + _check_attr_exists(cls, model, opts, label, field) + + # list_display + if not cls.list_display: + raise ImproperlyConfigured("%s.list_display can not be empty." % + cls.__name__) + _check_istuplew('list_display', cls.list_display) + for idx, field in enumerate(cls.list_display): + f = _check_attr_existsw("list_display['%d']" % idx, field) + if isinstance(f, models.ManyToManyField): + raise ImproperlyConfigured("`%s.list_display['%d']`, `%s` is a " + "ManyToManyField which is not supported." + % (cls.__name__, idx, field)) + + # list_display_links + if cls.list_display_links: + _check_istuplew('list_display_links', cls.list_display_links) + for idx, field in enumerate(cls.list_display_links): + _check_attr_existsw('list_display_links[%d]' % idx, field) + if field not in cls.list_display: + raise ImproperlyConfigured("`%s.list_display_links['%d']`" + "refers to `%s` which is not defined in `list_display`." + % (cls.__name__, idx, field)) + + # list_filter + if cls.list_filter: + _check_istuplew('list_filter', cls.list_filter) + for idx, field in enumerate(cls.list_filter): + _check_field_existsw('list_filter[%d]' % idx, field) + + # list_per_page = 100 + if not isinstance(cls.list_per_page, int): + raise ImproperlyConfigured("`%s.list_per_page` should be a integer." + % cls.__name__) + + # search_fields = () + if cls.search_fields: + _check_istuplew('search_fields', cls.search_fields) + # TODO: search field validation is quite complex (restrictions, + # follow fields etc), will skip it as of now + # for idx, field in enumerate(cls.search_fields): + # _check_field_existsw('search_fields[%d]' % idx, field) + + # date_hierarchy = None + if cls.date_hierarchy: + f = _check_field_existsw('date_hierarchy', cls.date_hierarchy) + if not isinstance(f, (models.DateField, models.DateTimeField)): + raise ImproperlyConfigured("`%s.date_hierarchy is " + "neither an instance of DateField nor DateTimeField." + % cls.__name__) + + # ordering = None + if cls.ordering: + _check_istuplew('ordering', cls.ordering) + for idx, field in enumerate(cls.ordering): + if field == '?' and len(cls.ordering) != 1: + raise ImproperlyConfigured("`%s.ordering` has the random " + "ordering marker `?`, but contains other fields as " + "well. Please either remove `?` or the other fields." + % cls.__name__) + if field.startswith('-'): + field = field[1:] + _check_field_existsw('ordering[%d]' % idx, field) + + # list_select_related = False + # save_as = False + # save_on_top = False + for attr in ('list_select_related', 'save_as', 'save_on_top'): + if not isinstance(getattr(cls, attr), bool): + raise ImproperlyConfigured("`%s.%s` should be a boolean." + % (cls.__name__, attr)) + + # inlines = [] + if cls.inlines: + _check_istuplew('inlines', cls.inlines) + for idx, inline in enumerate(cls.inlines): + if not issubclass(inline, BaseModelAdmin): + raise ImproperlyConfigured("`%s.inlines[%d]` does not inherit " + "from BaseModelAdmin." % (cls.__name__, idx)) + if not inline.model: + raise ImproperlyConfigured("`model` is a required attribute " + "of `%s.inlines[%d]`." % (cls.__name__, idx)) + if not issubclass(inline.model, models.Model): + raise ImproperlyConfigured("`%s.inlines[%d].model` does not " + "inherit from models.Model." % (cls.__name__, idx)) + _validate_base(inline, inline.model) + _validate_inline(inline) + + # TODO: check that the templates exist if given + # change_form_template = None + # change_list_template = None + # delete_confirmation_template = None + # object_history_template = None + + # hook for custom validation + _call_validation_hook(cls, model) + +def _validate_inline(cls): + # model is already verified to exist and be a Model + if cls.fk_name: + f = _check_field_exists(cls, cls.model, cls.model._meta, + 'fk_name', cls.fk_name) + if not isinstance(f, models.ForeignKey): + raise ImproperlyConfigured("`%s.fk_name is not an instance of " + "models.ForeignKey." % cls.__name__) + # extra = 3 + # max_num = 0 + for attr in ('extra', 'max_num'): + if not isinstance(getattr(cls, attr), int): + raise ImproperlyConfigured("`%s.%s` should be a integer." + % (cls.__name__, attr)) + + # formset + if cls.formset and not issubclass(cls.formset, BaseFormSet): + raise ImproperlyConfigured("`%s.formset` does not inherit from " + "BaseFormSet." % cls.__name__) + + # TODO: check the following as other templates above + # template = None + + # TODO: is there a need to validate the following? + # verbose_name = None + # verbose_name_plural = None + + # hook for custom validation + _call_validation_hook(cls, cls.model) + +def _validate_base(cls, model): + opts = model._meta + # currying is expensive, use wrappers instead + def _check_istuplew(label, obj): + _check_istuple(cls, label, obj) + + def _check_isdictw(label, obj): + _check_isdict(cls, label, obj) + + def _check_field_existsw(label, field): + return _check_field_exists(cls, model, opts, label, field) + + # raw_id_fields + if cls.raw_id_fields: + _check_istuplew('raw_id_fields', cls.raw_id_fields) + for field in cls.raw_id_fields: + _check_field_existsw('raw_id_fields', field) + + # fields + if cls.fields: + for field in cls.fields: + _check_field_existsw('fields', field) + + # fieldsets + if cls.fieldsets: + _check_istuplew('fieldsets', cls.fieldsets) + for idx, fieldset in enumerate(cls.fieldsets): + _check_istuplew('fieldsets[%d]' % idx, fieldset) + if len(fieldset) != 2: + raise ImproperlyConfigured("`%s.fieldsets[%d]` does not " + "have exactly two elements." % (cls.__name__, idx)) + _check_isdictw('fieldsets[%d][1]' % idx, fieldset[1]) + if 'fields' not in fieldset[1]: + raise ImproperlyConfigured("`fields` key is required in " + "%s.fieldsets[%d][1] field options dict." + % (cls.__name__, idx)) + for field in flatten_fieldsets(cls.fieldsets): + _check_field_existsw("fieldsets[%d][1]['fields']" % idx, field) + + # form + if cls.form and not issubclass(cls.form, BaseForm): + raise ImproperlyConfigured("%s.form does not inherit from BaseForm." + % cls.__name__) + + # filter_vertical + if cls.filter_vertical: + _check_istuplew('filter_vertical', cls.filter_vertical) + for field in cls.filter_vertical: + _check_field_existsw('filter_vertical', field) + + # filter_horizontal + if cls.filter_horizontal: + _check_istuplew('filter_horizontal', cls.filter_horizontal) + for field in cls.filter_horizontal: + _check_field_existsw('filter_horizontal', field) + + # radio_fields + if cls.radio_fields: + _check_isdictw('radio_fields', cls.radio_fields) + for field, val in cls.radio_fields.items(): + f = _check_field_existsw('radio_fields', field) + if not (isinstance(f, models.ForeignKey) or f.choices): + raise ImproperlyConfigured("`%s.radio_fields['%s']` " + "is neither an instance of ForeignKey nor does " + "have choices set." % (cls.__name__, field)) + if not val in (HORIZONTAL, VERTICAL): + raise ImproperlyConfigured("`%s.radio_fields['%s']` " + "is neither admin.HORIZONTAL nor admin.VERTICAL." + % (cls.__name__, field)) + + # prepopulated_fields + if cls.prepopulated_fields: + _check_isdictw('prepopulated_fields', cls.prepopulated_fields) + for field, val in cls.prepopulated_fields.items(): + f = _check_field_existsw('prepopulated_fields', field) + if isinstance(f, (models.DateTimeField, models.ForeignKey, + models.ManyToManyField)): + raise ImproperlyConfigured("`%s.prepopulated_fields['%s']` " + "is either a DateTimeField, ForeignKey or " + "ManyToManyField. This isn't allowed." + % (cls.__name__, field)) + _check_istuplew("prepopulated_fields['%s']" % field, val) + for idx, f in enumerate(val): + _check_field_existsw("prepopulated_fields['%s'][%d]" + % (f, idx), f) + +def _call_validation_hook(cls, model): + if hasattr(cls, 'validate'): + if not callable(cls.validate): + raise ImproperlyConfigured("`%s.validate` should be a callable " + "(class method)." % cls.__name__) + cls.validate(model) + +def _check_istuple(cls, label, obj): + if not isinstance(obj, (list, tuple)): + raise ImproperlyConfigured("`%s.%s` must be a " + "list or tuple." % (cls.__name__, label)) + +def _check_isdict(cls, label, obj): + if not isinstance(obj, dict): + raise ImproperlyConfigured("`%s.%s` must be a dictionary." + % (cls.__name__, label)) + +def _check_field_exists(cls, model, opts, label, field): + try: + return opts.get_field(field) + except models.FieldDoesNotExist: + raise ImproperlyConfigured("`%s.%s` refers to " + "field `%s` that is missing from model `%s`." + % (cls.__name__, label, field, model.__name__)) + +def _check_attr_exists(cls, model, opts, label, field): + try: + return opts.get_field(field) + except models.FieldDoesNotExist: + if not hasattr(model, field): + raise ImproperlyConfigured("`%s.%s` refers to " + "`%s` that is neither a field, method or property " + "of model `%s`." + % (cls.__name__, label, field, model.__name__)) + return getattr(model, field) Index: django/contrib/admin/sites.py =================================================================== --- django/contrib/admin/sites.py (revision 7884) +++ django/contrib/admin/sites.py (working copy) @@ -7,6 +7,7 @@ from django.utils.text import capfirst from django.utils.translation import ugettext_lazy, ugettext as _ from django.views.decorators.cache import never_cache +from django.conf import settings import base64 import cPickle as pickle import datetime @@ -65,6 +66,10 @@ If a model is already registered, this will raise AlreadyRegistered. """ + do_validate = admin_class and settings.DEBUG + if do_validate: + # don't import the humongous validation code unless required + from django.contrib.admin.validation import validate admin_class = admin_class or ModelAdmin # TODO: Handle options if isinstance(model_or_iterable, ModelBase): @@ -72,6 +77,8 @@ for model in model_or_iterable: if model in self._registry: raise AlreadyRegistered('The model %s is already registered' % model.__name__) + if do_validate: + validate(admin_class, model) self._registry[model] = admin_class(model, self) def unregister(self, model_or_iterable):