Code

Ticket #5374: modeladmin_validation2.diff

File modeladmin_validation2.diff, 18.4 KB (added by mrts, 6 years ago)

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

Line 
1Index: django/core/management/validation.py
2===================================================================
3--- django/core/management/validation.py        (revision 7884)
4+++ django/core/management/validation.py        (working copy)
5@@ -143,59 +143,6 @@
6                     if r.get_accessor_name() == rel_query_name:
7                         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))
8 
9-        # Check admin attribute.
10-        if opts.admin is not None:
11-            # prepopulated_fields
12-            if not isinstance(opts.admin.prepopulated_fields, dict):
13-                e.add(opts, '"%s": prepopulated_fields should be a dictionary.' % f.name)
14-            else:
15-                for field_name, field_list in opts.admin.prepopulated_fields.items():
16-                    if not isinstance(field_list, (list, tuple)):
17-                        e.add(opts, '"%s": prepopulated_fields "%s" value should be a list or tuple.' % (f.name, field_name))
18-
19-            # list_display
20-            if not isinstance(opts.admin.list_display, (list, tuple)):
21-                e.add(opts, '"admin.list_display", if given, must be set to a list or tuple.')
22-            else:
23-                for fn in opts.admin.list_display:
24-                    try:
25-                        f = opts.get_field(fn)
26-                    except models.FieldDoesNotExist:
27-                        if not hasattr(cls, fn):
28-                            e.add(opts, '"admin.list_display" refers to %r, which isn\'t an attribute, method or property.' % fn)
29-                    else:
30-                        if isinstance(f, models.ManyToManyField):
31-                            e.add(opts, '"admin.list_display" doesn\'t support ManyToManyFields (%r).' % fn)
32-            # list_display_links
33-            if opts.admin.list_display_links and not opts.admin.list_display:
34-                e.add(opts, '"admin.list_display" must be defined for "admin.list_display_links" to be used.')
35-            if not isinstance(opts.admin.list_display_links, (list, tuple)):
36-                e.add(opts, '"admin.list_display_links", if given, must be set to a list or tuple.')
37-            else:
38-                for fn in opts.admin.list_display_links:
39-                    try:
40-                        f = opts.get_field(fn)
41-                    except models.FieldDoesNotExist:
42-                        if not hasattr(cls, fn):
43-                            e.add(opts, '"admin.list_display_links" refers to %r, which isn\'t an attribute, method or property.' % fn)
44-                    if fn not in opts.admin.list_display:
45-                        e.add(opts, '"admin.list_display_links" refers to %r, which is not defined in "admin.list_display".' % fn)
46-            # list_filter
47-            if not isinstance(opts.admin.list_filter, (list, tuple)):
48-                e.add(opts, '"admin.list_filter", if given, must be set to a list or tuple.')
49-            else:
50-                for fn in opts.admin.list_filter:
51-                    try:
52-                        f = opts.get_field(fn)
53-                    except models.FieldDoesNotExist:
54-                        e.add(opts, '"admin.list_filter" refers to %r, which isn\'t a field.' % fn)
55-            # date_hierarchy
56-            if opts.admin.date_hierarchy:
57-                try:
58-                    f = opts.get_field(opts.admin.date_hierarchy)
59-                except models.FieldDoesNotExist:
60-                    e.add(opts, '"admin.date_hierarchy" refers to %r, which isn\'t a field.' % opts.admin.date_hierarchy)
61-
62         # Check ordering attribute.
63         if opts.ordering:
64             for field_name in opts.ordering:
65@@ -213,18 +160,6 @@
66                 except models.FieldDoesNotExist:
67                     e.add(opts, '"ordering" refers to "%s", a field that doesn\'t exist.' % field_name)
68 
69-        # Check core=True, if needed.
70-        for related in opts.get_followed_related_objects():
71-            if not related.edit_inline:
72-                continue
73-            try:
74-                for f in related.opts.fields:
75-                    if f.core:
76-                        raise StopIteration
77-                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))
78-            except StopIteration:
79-                pass
80-
81         # Check unique_together.
82         for ut in opts.unique_together:
83             for field_name in ut:
84Index: django/contrib/admin/validation.py
85===================================================================
86--- django/contrib/admin/validation.py  (revision 0)
87+++ django/contrib/admin/validation.py  (revision 0)
88@@ -0,0 +1,289 @@
89+# Possible improvements: gather all errors and raise in the end instead of
90+# bailing out on the first one (like core/management/validation.py does)
91+# ---
92+# Another issue brought up by jkocherhans:
93+# <jkocherhans> mrts: we're probably not going to be able to design a nice
94+# system for validation before 1.0, so a function that does the validation is
95+# probably fine... no need to over-engieer anything or provide hook for custom
96+# classes to allow their own validation.
97+# <mrts> it doesn't hurt though...
98+# <jkocherhans> I'd argue that it *does* hurt though, cause then we're
99+# committing to an api even if it isn't documented, people will use it
100+# <mrts> should I remove the calls to _call_validation_hook(cls, model) then?
101+# <jkocherhans> mrts: I'd say yes, but you should probably get a second opinion
102+
103+from django.core.exceptions import ImproperlyConfigured
104+from django.db import models
105+from django.newforms.forms import BaseForm
106+from django.newforms.formsets import BaseFormSet
107+from django.contrib.admin.options import flatten_fieldsets, BaseModelAdmin
108+from django.contrib.admin.options import HORIZONTAL, VERTICAL
109+
110+def validate(cls, model):
111+    """
112+    Does basic ModelAdmin option validation. Calls custom validation
113+    classmethod in the end if it is provided in cls. The signature of the
114+    custom validation classmethod should be: def validate(cls, model).
115+    """
116+    opts = model._meta
117+    _validate_base(cls, model)
118+
119+    # currying is expensive, use wrappers instead
120+    def _check_istuplew(label, obj):
121+        _check_istuple(cls, label, obj)
122+
123+    def _check_isdictw(label, obj):
124+        _check_isdict(cls, label, obj)
125+
126+    def _check_field_existsw(label, field):
127+        return _check_field_exists(cls, model, opts, label, field)
128+
129+    def _check_attr_existsw(label, field):
130+        _check_attr_exists(cls, model, opts, label, field)
131+
132+    # list_display
133+    if not cls.list_display:
134+        raise ImproperlyConfigured("%s.list_display can not be empty." %
135+                cls.__name__)
136+    _check_istuplew('list_display', cls.list_display)
137+    for idx, field in enumerate(cls.list_display):
138+        f = _check_attr_existsw("list_display['%d']" % idx, field)
139+        if isinstance(f, models.ManyToManyField):
140+            raise ImproperlyConfigured("`%s.list_display['%d']`, `%s` is a "
141+                    "ManyToManyField which is not supported."
142+                    % (cls.__name__, idx, field))
143+
144+    # list_display_links
145+    if cls.list_display_links:
146+        _check_istuplew('list_display_links', cls.list_display_links)
147+        for idx, field in enumerate(cls.list_display_links):
148+            _check_attr_existsw('list_display_links[%d]' % idx, field)
149+            if field not in cls.list_display:
150+                raise ImproperlyConfigured("`%s.list_display_links['%d']`"
151+                        "refers to `%s` which is not defined in `list_display`."
152+                        % (cls.__name__, idx, field))
153+
154+    # list_filter
155+    if cls.list_filter:
156+        _check_istuplew('list_filter', cls.list_filter)
157+        for idx, field in enumerate(cls.list_filter):
158+            _check_field_existsw('list_filter[%d]' % idx, field)
159+
160+    # list_per_page = 100
161+    if not isinstance(cls.list_per_page, int):
162+        raise ImproperlyConfigured("`%s.list_per_page` should be a integer."
163+                % cls.__name__)
164+
165+    # search_fields = ()
166+    if cls.search_fields:
167+        _check_istuplew('search_fields', cls.search_fields)
168+        # TODO: search field validation is quite complex (restrictions,
169+        # follow fields etc), will skip it as of now
170+        # for idx, field in enumerate(cls.search_fields):
171+            # _check_field_existsw('search_fields[%d]' % idx, field)
172+
173+    # date_hierarchy = None
174+    if cls.date_hierarchy:
175+        f = _check_field_existsw('date_hierarchy', cls.date_hierarchy)
176+        if not isinstance(f, (models.DateField, models.DateTimeField)):
177+            raise ImproperlyConfigured("`%s.date_hierarchy is "
178+                    "neither an instance of DateField nor DateTimeField."
179+                    % cls.__name__)
180+
181+    # ordering = None
182+    if cls.ordering:
183+        _check_istuplew('ordering', cls.ordering)
184+        for idx, field in enumerate(cls.ordering):
185+            if field == '?' and len(cls.ordering) != 1:
186+                raise ImproperlyConfigured("`%s.ordering` has the random "
187+                        "ordering marker `?`, but contains other fields as "
188+                        "well. Please either remove `?` or the other fields."
189+                        % cls.__name__)
190+            if field.startswith('-'):
191+                field = field[1:]
192+            _check_field_existsw('ordering[%d]' % idx, field)
193+
194+    # list_select_related = False
195+    # save_as = False
196+    # save_on_top = False
197+    for attr in ('list_select_related', 'save_as', 'save_on_top'):
198+        if not isinstance(getattr(cls, attr), bool):
199+            raise ImproperlyConfigured("`%s.%s` should be a boolean."
200+                    % (cls.__name__, attr))
201+
202+    # inlines = []
203+    if cls.inlines:
204+        _check_istuplew('inlines', cls.inlines)
205+        for idx, inline in enumerate(cls.inlines):
206+            if not issubclass(inline, BaseModelAdmin):
207+                raise ImproperlyConfigured("`%s.inlines[%d]` does not inherit "
208+                        "from BaseModelAdmin." % (cls.__name__, idx))
209+            if not inline.model:
210+                raise ImproperlyConfigured("`model` is a required attribute "
211+                        "of `%s.inlines[%d]`." % (cls.__name__, idx))
212+            if not issubclass(inline.model, models.Model):
213+                raise ImproperlyConfigured("`%s.inlines[%d].model` does not "
214+                        "inherit from models.Model." % (cls.__name__, idx))
215+            _validate_base(inline, inline.model)
216+            _validate_inline(inline)
217+
218+    # TODO: check that the templates exist if given
219+    # change_form_template = None
220+    # change_list_template = None
221+    # delete_confirmation_template = None
222+    # object_history_template = None
223+
224+    # hook for custom validation
225+    _call_validation_hook(cls, model)
226+
227+def _validate_inline(cls):
228+    # model is already verified to exist and be a Model
229+    if cls.fk_name:
230+        f = _check_field_exists(cls, cls.model, cls.model._meta,
231+                'fk_name', cls.fk_name)
232+        if not isinstance(f, models.ForeignKey):
233+            raise ImproperlyConfigured("`%s.fk_name is not an instance of "
234+                    "models.ForeignKey." % cls.__name__)
235+    # extra = 3
236+    # max_num = 0
237+    for attr in ('extra', 'max_num'):
238+        if not isinstance(getattr(cls, attr), int):
239+            raise ImproperlyConfigured("`%s.%s` should be a integer."
240+                    % (cls.__name__, attr))
241+
242+    # formset
243+    if cls.formset and not issubclass(cls.formset, BaseFormSet):
244+        raise ImproperlyConfigured("`%s.formset` does not inherit from "
245+                "BaseFormSet." % cls.__name__)
246+
247+    # TODO: check the following as other templates above
248+    # template = None
249+
250+    # TODO: is there a need to validate the following?
251+    # verbose_name = None
252+    # verbose_name_plural = None
253+
254+    # hook for custom validation
255+    _call_validation_hook(cls, cls.model)
256+
257+def _validate_base(cls, model):
258+    opts = model._meta
259+    # currying is expensive, use wrappers instead
260+    def _check_istuplew(label, obj):
261+        _check_istuple(cls, label, obj)
262+
263+    def _check_isdictw(label, obj):
264+        _check_isdict(cls, label, obj)
265+
266+    def _check_field_existsw(label, field):
267+        return _check_field_exists(cls, model, opts, label, field)
268+
269+    # raw_id_fields
270+    if cls.raw_id_fields:
271+        _check_istuplew('raw_id_fields', cls.raw_id_fields)
272+        for field in cls.raw_id_fields:
273+            _check_field_existsw('raw_id_fields', field)
274+
275+    # fields
276+    if cls.fields:
277+        for field in cls.fields:
278+            _check_field_existsw('fields', field)
279+
280+    # fieldsets
281+    if cls.fieldsets:
282+        _check_istuplew('fieldsets', cls.fieldsets)
283+        for idx, fieldset in enumerate(cls.fieldsets):
284+            _check_istuplew('fieldsets[%d]' % idx, fieldset)
285+            if len(fieldset) != 2:
286+                raise ImproperlyConfigured("`%s.fieldsets[%d]` does not "
287+                        "have exactly two elements." % (cls.__name__, idx))
288+            _check_isdictw('fieldsets[%d][1]' % idx, fieldset[1])
289+            if 'fields' not in fieldset[1]:
290+                raise ImproperlyConfigured("`fields` key is required in "
291+                        "%s.fieldsets[%d][1] field options dict."
292+                        % (cls.__name__, idx))
293+        for field in flatten_fieldsets(cls.fieldsets):
294+            _check_field_existsw("fieldsets[%d][1]['fields']" % idx, field)
295+
296+    # form
297+    if cls.form and not issubclass(cls.form, BaseForm):
298+        raise ImproperlyConfigured("%s.form does not inherit from BaseForm."
299+                % cls.__name__)
300+
301+    # filter_vertical
302+    if cls.filter_vertical:
303+        _check_istuplew('filter_vertical', cls.filter_vertical)
304+        for field in cls.filter_vertical:
305+            _check_field_existsw('filter_vertical', field)
306+
307+    # filter_horizontal
308+    if cls.filter_horizontal:
309+        _check_istuplew('filter_horizontal', cls.filter_horizontal)
310+        for field in cls.filter_horizontal:
311+            _check_field_existsw('filter_horizontal', field)
312+
313+    # radio_fields
314+    if cls.radio_fields:
315+        _check_isdictw('radio_fields', cls.radio_fields)
316+        for field, val in cls.radio_fields.items():
317+            f = _check_field_existsw('radio_fields', field)
318+            if not (isinstance(f, models.ForeignKey) or f.choices):
319+                raise ImproperlyConfigured("`%s.radio_fields['%s']` "
320+                        "is neither an instance of ForeignKey nor does "
321+                        "have choices set." % (cls.__name__, field))
322+            if not val in (HORIZONTAL, VERTICAL):
323+                raise ImproperlyConfigured("`%s.radio_fields['%s']` "
324+                        "is neither admin.HORIZONTAL nor admin.VERTICAL."
325+                        % (cls.__name__, field))
326+
327+    # prepopulated_fields
328+    if cls.prepopulated_fields:
329+        _check_isdictw('prepopulated_fields', cls.prepopulated_fields)
330+        for field, val in cls.prepopulated_fields.items():
331+            f = _check_field_existsw('prepopulated_fields', field)
332+            if isinstance(f, (models.DateTimeField, models.ForeignKey,
333+                models.ManyToManyField)):
334+                raise ImproperlyConfigured("`%s.prepopulated_fields['%s']` "
335+                        "is either a DateTimeField, ForeignKey or "
336+                        "ManyToManyField. This isn't allowed."
337+                        % (cls.__name__, field))
338+            _check_istuplew("prepopulated_fields['%s']" % field, val)
339+            for idx, f in enumerate(val):
340+                _check_field_existsw("prepopulated_fields['%s'][%d]"
341+                        % (f, idx), f)
342+
343+def _call_validation_hook(cls, model):
344+    if hasattr(cls, 'validate'):
345+        if not callable(cls.validate):
346+            raise ImproperlyConfigured("`%s.validate` should be a callable "
347+                    "(class method)." % cls.__name__)
348+        cls.validate(model)
349+
350+def _check_istuple(cls, label, obj):
351+    if not isinstance(obj, (list, tuple)):
352+        raise ImproperlyConfigured("`%s.%s` must be a "
353+                "list or tuple." % (cls.__name__, label))
354+
355+def _check_isdict(cls, label, obj):
356+    if not isinstance(obj, dict):
357+        raise ImproperlyConfigured("`%s.%s` must be a dictionary."
358+                % (cls.__name__, label))
359+
360+def _check_field_exists(cls, model, opts, label, field):
361+    try:
362+        return opts.get_field(field)
363+    except models.FieldDoesNotExist:
364+        raise ImproperlyConfigured("`%s.%s` refers to "
365+                "field `%s` that is missing from model `%s`."
366+                % (cls.__name__, label, field, model.__name__))
367+
368+def _check_attr_exists(cls, model, opts, label, field):
369+    try:
370+        return opts.get_field(field)
371+    except models.FieldDoesNotExist:
372+        if not hasattr(model, field):
373+            raise ImproperlyConfigured("`%s.%s` refers to "
374+                    "`%s` that is neither a field, method or property "
375+                    "of model `%s`."
376+                    % (cls.__name__, label, field, model.__name__))
377+        return getattr(model, field)
378Index: django/contrib/admin/sites.py
379===================================================================
380--- django/contrib/admin/sites.py       (revision 7884)
381+++ django/contrib/admin/sites.py       (working copy)
382@@ -7,6 +7,7 @@
383 from django.utils.text import capfirst
384 from django.utils.translation import ugettext_lazy, ugettext as _
385 from django.views.decorators.cache import never_cache
386+from django.conf import settings
387 import base64
388 import cPickle as pickle
389 import datetime
390@@ -65,6 +66,10 @@
391 
392         If a model is already registered, this will raise AlreadyRegistered.
393         """
394+        do_validate = admin_class and settings.DEBUG
395+        if do_validate:
396+            # don't import the humongous validation code unless required
397+            from django.contrib.admin.validation import validate
398         admin_class = admin_class or ModelAdmin
399         # TODO: Handle options
400         if isinstance(model_or_iterable, ModelBase):
401@@ -72,6 +77,8 @@
402         for model in model_or_iterable:
403             if model in self._registry:
404                 raise AlreadyRegistered('The model %s is already registered' % model.__name__)
405+            if do_validate:
406+                validate(admin_class, model)
407             self._registry[model] = admin_class(model, self)
408 
409     def unregister(self, model_or_iterable):