Modified [7112] to make things behave more in line with Python subcla…

…ssing when subclassing ModelForms.

Meta can now be subclassed and changes on the child model affect the fields
list. Also removed a block of error checking, since it's harder to mess up in
unexpected ways now (e.g. you can't change the model and get the entirely wrong
fields list), so it was a level of overkill.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@7115 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/newforms/forms.py

@@ -22,16 +22,30 @@ def pretty_name(name):
     name = name[0].upper() + name[1:]
     return name.replace('_', ' ')
 
-def get_declared_fields(bases, attrs):
+def get_declared_fields(bases, attrs, with_base_fields=True):
+    """
+    Create a list of form field instances from the passed in 'attrs', plus any
+    similar fields on the base classes (in 'bases'). This is used by both the
+    Form and ModelForm metclasses.
+
+    If 'with_base_fields' is True, all fields from the bases are used.
+    Otherwise, only fields in the 'declared_fields' attribute on the bases are
+    used. The distinction is useful in ModelForm subclassing.
+    """
     fields = [(field_name, attrs.pop(field_name)) for field_name, obj in attrs.items() if isinstance(obj, Field)]
     fields.sort(lambda x, y: cmp(x[1].creation_counter, y[1].creation_counter))
 
     # If this class is subclassing another Form, add that Form's fields.
     # Note that we loop over the bases in *reverse*. This is necessary in
     # order to preserve the correct order of fields.
-    for base in bases[::-1]:
-        if hasattr(base, 'base_fields'):
-            fields = base.base_fields.items() + fields
+    if with_base_fields:
+        for base in bases[::-1]:
+            if hasattr(base, 'base_fields'):
+                fields = base.base_fields.items() + fields
+    else:
+        for base in bases[::-1]:
+            if hasattr(base, 'declared_fields'):
+                fields = base.declared_fields.items() + fields
 
     return SortedDict(fields)
 

django/newforms/models.py

@@ -225,7 +225,7 @@ def __new__(cls, name, bases, attrs,
                     attrs)
 
         new_class = type.__new__(cls, name, bases, attrs)
-        declared_fields = get_declared_fields(bases, attrs)
+        declared_fields = get_declared_fields(bases, attrs, False)
         opts = new_class._meta = ModelFormOptions(getattr(new_class, 'Meta', None))
         if opts.model:
             # If a model is defined, extract form fields from it.
@@ -236,27 +236,8 @@ def __new__(cls, name, bases, attrs,
             fields.update(declared_fields)
         else:
             fields = declared_fields
+        new_class.declared_fields = declared_fields
         new_class.base_fields = fields
-
-        # XXX: The following is a sanity check for the user to avoid
-        # inadvertent attribute hiding.
-
-        # Search base classes, but don't allow more than one Meta model
-        # definition. The fields would be generated correctly, but the save
-        # method won't deal with more than one object. Also, it wouldn't be
-        # clear what to do with multiple fields and exclude lists.
-        first = None
-        current = opts.model
-        for base in parents:
-            base_opts = getattr(base, '_meta', None)
-            base_model = getattr(base_opts, 'model', None)
-            if base_model:
-                if current:
-                    if base_model is not current:
-                        raise ImproperlyConfigured("%s's base classes define more than one model." % name)
-                else:
-                    current = base_model
-
         return new_class
 
 class BaseModelForm(BaseForm):

docs/modelforms.txt

@@ -335,14 +335,19 @@ models. For example, using the previous ``ArticleForm`` class::
 This creates a form that behaves identically to ``ArticleForm``, except there
 is some extra validation and cleaning for the ``pub_date`` field.
 
+You can also subclass the parent's ``Meta`` inner class if you want to change
+the ``Meta.fields`` or ``Meta.excludes`` lists::
+
+    >>> class RestrictedArticleForm(EnhancedArticleForm):
+    ...     class Meta(ArticleForm.Meta):
+    ...         exclude = ['body']
+
+This adds in the extra method from the ``EnhancedArticleForm`` and modifies
+the original ``ArticleForm.Meta`` to remove one field.
+
 There are a couple of things to note, however. Most of these won't normally be
 of concern unless you are trying to do something tricky with subclassing.
 
- * All the fields from the parent classes will appear in the child
-   ``ModelForm``. This means you cannot change a parent's ``Meta.exclude``
-   attribute, for example, and except it to have an effect, since the field is
-   already part of the field list in the parent class.
-
  * Normal Python name resolution rules apply. If you have multiple base
    classes that declare a ``Meta`` inner class, only the first one will be
    used.  This means the child's ``Meta``, if it exists, otherwise the
@@ -351,10 +356,3 @@ of concern unless you are trying to do something tricky with subclassing.
  * For technical reasons, you cannot have a subclass that is inherited from
    both a ``ModelForm`` and a ``Form`` simultaneously.
 
-Because of the "child inherits all fields from parents" behaviour, you
-shouldn't try to declare model fields in multiple classes (parent and child).
-Instead, declare all the model-related stuff in one class and use inheritance
-to add "extra" non-model fields and methods to the final result. Whether you
-put the "extra" functions in the parent class or the child class will depend
-on how you intend to reuse them.
-

tests/modeltests/model_forms/models.py

@@ -155,43 +155,52 @@ def __unicode__(self):
 ...     class Meta:
 ...         model = Category
 
->>> class BadForm(CategoryForm):
+>>> class OddForm(CategoryForm):
 ...     class Meta:
 ...         model = Article
-Traceback (most recent call last):
-...
-ImproperlyConfigured: BadForm's base classes define more than one model.
+
+OddForm is now an Article-related thing, because BadForm.Meta overrides
+CategoryForm.Meta.
+>>> OddForm.base_fields.keys()
+['headline', 'slug', 'pub_date', 'writer', 'article', 'status', 'categories']
 
 >>> class ArticleForm(ModelForm):
 ...     class Meta:
 ...         model = Article
 
+First class with a Meta class wins.
+
 >>> class BadForm(ArticleForm, CategoryForm):
 ...     pass
-Traceback (most recent call last):
-...
-ImproperlyConfigured: BadForm's base classes define more than one model.
+>>> OddForm.base_fields.keys()
+['headline', 'slug', 'pub_date', 'writer', 'article', 'status', 'categories']
 
-This one is OK since the subclass specifies the same model as the parent.
+Subclassing without specifying a Meta on the class will use the parent's Meta
+(or the first parent in the MRO if there are multiple parent classes).
 
->>> class SubCategoryForm(CategoryForm):
+>>> class CategoryForm(ModelForm):
 ...     class Meta:
 ...         model = Category
+>>> class SubCategoryForm(CategoryForm):
+...     pass
+>>> SubCategoryForm.base_fields.keys()
+['name', 'slug', 'url']
 
-
-Subclassing without specifying a Meta on the class will use the parent's Meta
-(or the first parent in the MRO if there are multiple parent classes).
+We can also subclass the Meta inner class to change the fields list.
 
 >>> class CategoryForm(ModelForm):
+...     checkbox = forms.BooleanField()
+...
 ...     class Meta:
 ...         model = Category
-...         exclude = ['url']
 >>> class SubCategoryForm(CategoryForm):
-...     pass
+...     class Meta(CategoryForm.Meta):
+...         exclude = ['url']
 
 >>> print SubCategoryForm()
 <tr><th><label for="id_name">Name:</label></th><td><input id="id_name" type="text" name="name" maxlength="20" /></td></tr>
 <tr><th><label for="id_slug">Slug:</label></th><td><input id="id_slug" type="text" name="slug" maxlength="20" /></td></tr>
+<tr><th><label for="id_checkbox">Checkbox:</label></th><td><input type="checkbox" name="checkbox" id="id_checkbox" /></td></tr>
 
 # Old form_for_x tests #######################################################