Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #6337. Refs #3632 -- Fixed ModelForms subclassing, to the exten…

…t that it can be made to work.

This ended up being an almost complete rewrite of ModelForms.__new__, but
should be backwards compatible (although the text of one error message has
changed, which is only user visible and only if you pass in invalid code).

Documentation updated, also.

This started out as a patch from semenov (many thanks!), but by the time all
the problems were hammered out, little of the original was left. Still, it was
a good starting point.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@7112 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 37962ecea79cb9d296645a5324cd91818fed65b3 1 parent 5078010
Malcolm Tredinnick authored February 14, 2008
25  django/newforms/forms.py
@@ -22,23 +22,26 @@ def pretty_name(name):
22 22
     name = name[0].upper() + name[1:]
23 23
     return name.replace('_', ' ')
24 24
 
  25
+def get_declared_fields(bases, attrs):
  26
+    fields = [(field_name, attrs.pop(field_name)) for field_name, obj in attrs.items() if isinstance(obj, Field)]
  27
+    fields.sort(lambda x, y: cmp(x[1].creation_counter, y[1].creation_counter))
  28
+
  29
+    # If this class is subclassing another Form, add that Form's fields.
  30
+    # Note that we loop over the bases in *reverse*. This is necessary in
  31
+    # order to preserve the correct order of fields.
  32
+    for base in bases[::-1]:
  33
+        if hasattr(base, 'base_fields'):
  34
+            fields = base.base_fields.items() + fields
  35
+
  36
+    return SortedDict(fields)
  37
+
25 38
 class DeclarativeFieldsMetaclass(type):
26 39
     """
27 40
     Metaclass that converts Field attributes to a dictionary called
28 41
     'base_fields', taking into account parent class 'base_fields' as well.
29 42
     """
30 43
     def __new__(cls, name, bases, attrs):
31  
-        fields = [(field_name, attrs.pop(field_name)) for field_name, obj in attrs.items() if isinstance(obj, Field)]
32  
-        fields.sort(lambda x, y: cmp(x[1].creation_counter, y[1].creation_counter))
33  
-
34  
-        # If this class is subclassing another Form, add that Form's fields.
35  
-        # Note that we loop over the bases in *reverse*. This is necessary in
36  
-        # order to preserve the correct order of fields.
37  
-        for base in bases[::-1]:
38  
-            if hasattr(base, 'base_fields'):
39  
-                fields = base.base_fields.items() + fields
40  
-
41  
-        attrs['base_fields'] = SortedDict(fields)
  44
+        attrs['base_fields'] = get_declared_fields(bases, attrs)
42 45
         return type.__new__(cls, name, bases, attrs)
43 46
 
44 47
 class BaseForm(StrAndUnicode):
92  django/newforms/models.py
@@ -11,7 +11,7 @@
11 11
 from django.core.exceptions import ImproperlyConfigured
12 12
 
13 13
 from util import ValidationError, ErrorList
14  
-from forms import BaseForm
  14
+from forms import BaseForm, get_declared_fields
15 15
 from fields import Field, ChoiceField, EMPTY_VALUES
16 16
 from widgets import Select, SelectMultiple, MultipleHiddenInput
17 17
 
@@ -211,57 +211,58 @@ def __init__(self, options=None):
211 211
         self.fields = getattr(options, 'fields', None)
212 212
         self.exclude = getattr(options, 'exclude', None)
213 213
 
  214
+
214 215
 class ModelFormMetaclass(type):
215 216
     def __new__(cls, name, bases, attrs,
216 217
                 formfield_callback=lambda f: f.formfield()):
217  
-        fields = [(field_name, attrs.pop(field_name)) for field_name, obj in attrs.items() if isinstance(obj, Field)]
218  
-        fields.sort(lambda x, y: cmp(x[1].creation_counter, y[1].creation_counter))
219  
-
220  
-        # If this class is subclassing another Form, add that Form's fields.
221  
-        # Note that we loop over the bases in *reverse*. This is necessary in
222  
-        # order to preserve the correct order of fields.
223  
-        for base in bases[::-1]:
224  
-            if hasattr(base, 'base_fields'):
225  
-                fields = base.base_fields.items() + fields
226  
-        declared_fields = SortedDict(fields)
227  
-
228  
-        opts = ModelFormOptions(attrs.get('Meta', None))
229  
-        attrs['_meta'] = opts
230  
-
231  
-        # Don't allow more than one Meta model definition in bases. The fields
232  
-        # would be generated correctly, but the save method won't deal with
233  
-        # more than one object.
234  
-        base_models = []
235  
-        for base in bases:
  218
+        try:
  219
+            parents = [b for b in bases if issubclass(b, ModelForm)]
  220
+        except NameError:
  221
+            # We are defining ModelForm itself.
  222
+            parents = None
  223
+        if not parents:
  224
+            return super(ModelFormMetaclass, cls).__new__(cls, name, bases,
  225
+                    attrs)
  226
+
  227
+        new_class = type.__new__(cls, name, bases, attrs)
  228
+        declared_fields = get_declared_fields(bases, attrs)
  229
+        opts = new_class._meta = ModelFormOptions(getattr(new_class, 'Meta', None))
  230
+        if opts.model:
  231
+            # If a model is defined, extract form fields from it.
  232
+            fields = fields_for_model(opts.model, opts.fields,
  233
+                                      opts.exclude, formfield_callback)
  234
+            # Fields defined on the base classes override local fields and are
  235
+            # always included.
  236
+            fields.update(declared_fields)
  237
+        else:
  238
+            fields = declared_fields
  239
+        new_class.base_fields = fields
  240
+
  241
+        # XXX: The following is a sanity check for the user to avoid
  242
+        # inadvertent attribute hiding.
  243
+
  244
+        # Search base classes, but don't allow more than one Meta model
  245
+        # definition. The fields would be generated correctly, but the save
  246
+        # method won't deal with more than one object. Also, it wouldn't be
  247
+        # clear what to do with multiple fields and exclude lists.
  248
+        first = None
  249
+        current = opts.model
  250
+        for base in parents:
236 251
             base_opts = getattr(base, '_meta', None)
237 252
             base_model = getattr(base_opts, 'model', None)
238  
-            if base_model is not None:
239  
-                base_models.append(base_model)
240  
-        if len(base_models) > 1:
241  
-            raise ImproperlyConfigured("%s's base classes define more than one model." % name)
242  
-
243  
-        # If a model is defined, extract form fields from it and add them to base_fields
244  
-        if attrs['_meta'].model is not None:
245  
-            # Don't allow a subclass to define a different Meta model than a
246  
-            # parent class has. Technically the right fields would be generated,
247  
-            # but the save method will not deal with more than one model.
248  
-            for base in bases:
249  
-                base_opts = getattr(base, '_meta', None)
250  
-                base_model = getattr(base_opts, 'model', None)
251  
-                if base_model and base_model is not opts.model:
252  
-                    raise ImproperlyConfigured('%s defines a different model than its parent.' % name)
253  
-            model_fields = fields_for_model(opts.model, opts.fields,
254  
-                    opts.exclude, formfield_callback)
255  
-            # fields declared in base classes override fields from the model
256  
-            model_fields.update(declared_fields)
257  
-            attrs['base_fields'] = model_fields
258  
-        else:
259  
-            attrs['base_fields'] = declared_fields
260  
-        return type.__new__(cls, name, bases, attrs)
  253
+            if base_model:
  254
+                if current:
  255
+                    if base_model is not current:
  256
+                        raise ImproperlyConfigured("%s's base classes define more than one model." % name)
  257
+                else:
  258
+                    current = base_model
  259
+
  260
+        return new_class
261 261
 
262 262
 class BaseModelForm(BaseForm):
263 263
     def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
264  
-                 initial=None, error_class=ErrorList, label_suffix=':', instance=None):
  264
+                 initial=None, error_class=ErrorList, label_suffix=':',
  265
+                 instance=None):
265 266
         opts = self._meta
266 267
         if instance is None:
267 268
             # if we didn't get an instance, instantiate a new one
@@ -277,7 +278,8 @@ def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
277 278
 
278 279
     def save(self, commit=True):
279 280
         """
280  
-        Saves this ``form``'s cleaned_data into model instance ``self.instance``.
  281
+        Saves this ``form``'s cleaned_data into model instance
  282
+        ``self.instance``.
281 283
 
282 284
         If commit=True, then the changes to ``instance`` will be saved to the
283 285
         database. Returns ``instance``.
38  docs/modelforms.txt
@@ -320,3 +320,41 @@ parameter when declaring the form field::
320 320
    ...
321 321
    ...     class Meta:
322 322
    ...         model = Article
  323
+
  324
+Form inheritance
  325
+----------------
  326
+As with the basic forms, you can extend and reuse ``ModelForms`` by inheriting
  327
+them. Normally, this will be useful if you need to declare some extra fields
  328
+or extra methods on a parent class for use in a number of forms derived from
  329
+models. For example, using the previous ``ArticleForm`` class::
  330
+
  331
+    >>> class EnhancedArticleForm(ArticleForm):
  332
+    ...     def clean_pub_date(self):
  333
+    ...         ...
  334
+
  335
+This creates a form that behaves identically to ``ArticleForm``, except there
  336
+is some extra validation and cleaning for the ``pub_date`` field.
  337
+
  338
+There are a couple of things to note, however. Most of these won't normally be
  339
+of concern unless you are trying to do something tricky with subclassing.
  340
+
  341
+ * All the fields from the parent classes will appear in the child
  342
+   ``ModelForm``. This means you cannot change a parent's ``Meta.exclude``
  343
+   attribute, for example, and except it to have an effect, since the field is
  344
+   already part of the field list in the parent class.
  345
+
  346
+ * Normal Python name resolution rules apply. If you have multiple base
  347
+   classes that declare a ``Meta`` inner class, only the first one will be
  348
+   used.  This means the child's ``Meta``, if it exists, otherwise the
  349
+   ``Meta`` of the first parent, etc.
  350
+
  351
+ * For technical reasons, you cannot have a subclass that is inherited from
  352
+   both a ``ModelForm`` and a ``Form`` simultaneously.
  353
+
  354
+Because of the "child inherits all fields from parents" behaviour, you
  355
+shouldn't try to declare model fields in multiple classes (parent and child).
  356
+Instead, declare all the model-related stuff in one class and use inheritance
  357
+to add "extra" non-model fields and methods to the final result. Whether you
  358
+put the "extra" functions in the parent class or the child class will depend
  359
+on how you intend to reuse them.
  360
+
20  tests/modeltests/model_forms/models.py
@@ -64,11 +64,11 @@ class TextFile(models.Model):
64 64
 
65 65
     def __unicode__(self):
66 66
         return self.description
67  
-        
  67
+
68 68
 class ImageFile(models.Model):
69 69
     description = models.CharField(max_length=20)
70 70
     image = models.FileField(upload_to=tempfile.gettempdir())
71  
-    
  71
+
72 72
     def __unicode__(self):
73 73
         return self.description
74 74
 
@@ -160,7 +160,7 @@ def __unicode__(self):
160 160
 ...         model = Article
161 161
 Traceback (most recent call last):
162 162
 ...
163  
-ImproperlyConfigured: BadForm defines a different model than its parent.
  163
+ImproperlyConfigured: BadForm's base classes define more than one model.
164 164
 
165 165
 >>> class ArticleForm(ModelForm):
166 166
 ...     class Meta:
@@ -179,6 +179,20 @@ def __unicode__(self):
179 179
 ...         model = Category
180 180
 
181 181
 
  182
+Subclassing without specifying a Meta on the class will use the parent's Meta
  183
+(or the first parent in the MRO if there are multiple parent classes).
  184
+
  185
+>>> class CategoryForm(ModelForm):
  186
+...     class Meta:
  187
+...         model = Category
  188
+...         exclude = ['url']
  189
+>>> class SubCategoryForm(CategoryForm):
  190
+...     pass
  191
+
  192
+>>> print SubCategoryForm()
  193
+<tr><th><label for="id_name">Name:</label></th><td><input id="id_name" type="text" name="name" maxlength="20" /></td></tr>
  194
+<tr><th><label for="id_slug">Slug:</label></th><td><input id="id_slug" type="text" name="slug" maxlength="20" /></td></tr>
  195
+
182 196
 # Old form_for_x tests #######################################################
183 197
 
184 198
 >>> from django.newforms import ModelForm, CharField

0 notes on commit 37962ec

Please sign in to comment.
Something went wrong with that request. Please try again.