From 1159791cd5b706f01e282d6773a17b66b7cdac9f Mon Sep 17 00:00:00 2001 From: Malcolm Tredinnick Date: Thu, 14 Feb 2008 17:38:05 +0000 Subject: [PATCH] Modified [7112] to make things behave more in line with Python subclassing 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 ++++++++++++--- django/newforms/models.py | 23 ++------------- docs/modelforms.txt | 22 +++++++-------- tests/modeltests/model_forms/models.py | 39 ++++++++++++++++---------- 4 files changed, 54 insertions(+), 52 deletions(-) diff --git a/django/newforms/forms.py b/django/newforms/forms.py index 7bc5e2ac02..b4ac80c5aa 100644 --- a/django/newforms/forms.py +++ b/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) diff --git a/django/newforms/models.py b/django/newforms/models.py index 324b7b2bf8..fbcf73c67a 100644 --- a/django/newforms/models.py +++ b/django/newforms/models.py @@ -225,7 +225,7 @@ class ModelFormMetaclass(type): 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 @@ class ModelFormMetaclass(type): 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): diff --git a/docs/modelforms.txt b/docs/modelforms.txt index ec0ecf40cc..b9f0d88165 100644 --- a/docs/modelforms.txt +++ b/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. - diff --git a/tests/modeltests/model_forms/models.py b/tests/modeltests/model_forms/models.py index 0d429b2a71..c480899f84 100644 --- a/tests/modeltests/model_forms/models.py +++ b/tests/modeltests/model_forms/models.py @@ -155,29 +155,25 @@ familiar with the mechanics. ... 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. - -This one is OK since the subclass specifies the same model as the parent. - ->>> class SubCategoryForm(CategoryForm): -... class Meta: -... model = Category - +>>> OddForm.base_fields.keys() +['headline', 'slug', 'pub_date', 'writer', 'article', 'status', 'categories'] 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). @@ -185,13 +181,26 @@ Subclassing without specifying a Meta on the class will use the parent's Meta >>> class CategoryForm(ModelForm): ... class Meta: ... model = Category -... exclude = ['url'] >>> class SubCategoryForm(CategoryForm): ... pass +>>> SubCategoryForm.base_fields.keys() +['name', 'slug', 'url'] + +We can also subclass the Meta inner class to change the fields list. + +>>> class CategoryForm(ModelForm): +... checkbox = forms.BooleanField() +... +... class Meta: +... model = Category +>>> class SubCategoryForm(CategoryForm): +... class Meta(CategoryForm.Meta): +... exclude = ['url'] >>> print SubCategoryForm() + # Old form_for_x tests #######################################################