We need a validator for ModelAdmin classes

[jkocherhans]

get_validation_errors needs to be updated to deal with ModelAdmin classes instead of the old inline Admin classes.

It may be desirable for ModelAdmin to know how to validate itself (and by 'validate' I basically mean catching invalid attribute names).

[Daniel Roseman]

Initial attempt at patch.

[jkocherhans]

I just thought of a possible issue here, this should work fine for the default admin site, but it won't work for any additional ones unless we use some sort of global registry of all AdminSite instnaces or ModelAdmin subclasses... yuck. I'm starting to think that it would make sense for ModelAdmin classes to know how to validate themselves.

[Brian Rosner]

This does need to be apart of newforms-admin before merging. Tagging with nfa-blocker.

[mrts]

I'm championing this currently. Will upload a patch shortly.

[mrts]

Hooks into admin.site.register, more or less complete coverage of nfa options

[mrts]

typo

[mrts]

form and formset validation added, also comments on validation hook by jkocherhans

[mrts]

Note the following:

<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. 

<mrts> it doesn't hurt though... 

<jkocherhans> 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 

<mrts> should I remove the calls to _call_validation_hook(cls, model) then? 

<jkocherhans> mrts: I'd say yes, but you should probably get a second opinion

<jkocherhans> mrts: I'm not arguing that they're not useful, I'm arguing that
it's a design decision, and I know that malcolm and some other people have opinions about what should happen

[mrts]

I will be away until July 21, so if anyone can write the tests and add a minor documentation note that validation will only be run if settings.DEBUG = True I'd be grateful. If not, I'm back on 21. and will continue on this.

[mrts]

Another note: the initial plan was to provide classmethods in the line of the following for easy overriding:

Index: django/contrib/admin/options.py
===================================================================
--- django/contrib/admin/options.py	(revision 7877)
+++ django/contrib/admin/options.py	(working copy)
@@ -221,6 +221,103 @@
         return None
     declared_fieldsets = property(_declared_fieldsets)
 
+    def validate(cls, model):
+        """Does basic option validation."""
+        # insert validation here
+    validate = classmethod(validate)
+
 class ModelAdmin(BaseModelAdmin):
     "Encapsulates all admin options and functionality for a given model."
     __metaclass__ = forms.MediaDefiningClass
@@ -236,7 +333,7 @@
     save_on_top = False
     ordering = None
     inlines = []
-    
+
     # Custom templates (designed to be over-ridden in subclasses)
     change_form_template = None
     change_list_template = None
@@ -735,6 +832,12 @@
             "admin/object_history.html"
         ], context, context_instance=template.RequestContext(request))
 
+    def validate(cls, model):
+        """Does basic option validation."""
+        super(ModelAdmin, cls).validate()
+        # insert validation here
+    validate = classmethod(validate)
+
 class InlineModelAdmin(BaseModelAdmin):
     """
     Options for inline editing of ``model`` instances.
@@ -779,6 +882,12 @@
         form = self.get_formset(request).form
         return [(None, {'fields': form.base_fields.keys()})]
 
+    def validate(cls, model):
+        """Does basic option validation."""
+        super(InlineModelAdmin, cls).validate()
+        # insert validation here
+    validate = classmethod(validate)
+
 class StackedInline(InlineModelAdmin):
     template = 'admin/edit_inline/stacked.html'
 
Index: django/contrib/admin/sites.py
===================================================================
--- django/contrib/admin/sites.py	(revision 7877)
+++ 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,7 @@
 
         If a model is already registered, this will raise AlreadyRegistered.
         """
+        do_validate = admin_class and settings.DEBUG
         admin_class = admin_class or ModelAdmin
         # TODO: Handle options
         if isinstance(model_or_iterable, ModelBase):
@@ -72,6 +74,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:
+                admin_class.validate(model)
             self._registry[model] = admin_class(model, self)
 
     def unregister(self, model_or_iterable):

However, that would make options.py and ModelAdmin classes needlessly heavy-weight. Thus, validation was moved to a separate module that is loaded only in debug mode. Custom validation is still possible, as there are hooks that will call validate() classmethod if provided in the custom ModelAdmin class. Their usefulness or relevance remains a point of discussion, see previous note.

[Brian Rosner]

(In [7929]) newforms-admin: Fixed #5374 -- Added validation for ModelAdmin and InlineModelAdmin options including tests. Thanks mrts for initial legwork.

[Jacob]

Milestone 1.0 alpha deleted