diff --git a/django/newforms/forms.py b/django/newforms/forms.py index 04bf62e445..7bc5e2ac02 100644 --- a/django/newforms/forms.py +++ b/django/newforms/forms.py @@ -22,23 +22,26 @@ def pretty_name(name): name = name[0].upper() + name[1:] return name.replace('_', ' ') +def get_declared_fields(bases, attrs): + 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 + + return SortedDict(fields) + class DeclarativeFieldsMetaclass(type): """ Metaclass that converts Field attributes to a dictionary called 'base_fields', taking into account parent class 'base_fields' as well. """ def __new__(cls, name, bases, attrs): - 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 - - attrs['base_fields'] = SortedDict(fields) + attrs['base_fields'] = get_declared_fields(bases, attrs) return type.__new__(cls, name, bases, attrs) class BaseForm(StrAndUnicode): diff --git a/django/newforms/models.py b/django/newforms/models.py index 0ee911a82f..324b7b2bf8 100644 --- a/django/newforms/models.py +++ b/django/newforms/models.py @@ -11,7 +11,7 @@ from django.utils.datastructures import SortedDict from django.core.exceptions import ImproperlyConfigured from util import ValidationError, ErrorList -from forms import BaseForm +from forms import BaseForm, get_declared_fields from fields import Field, ChoiceField, EMPTY_VALUES from widgets import Select, SelectMultiple, MultipleHiddenInput @@ -211,57 +211,58 @@ class ModelFormOptions(object): self.fields = getattr(options, 'fields', None) self.exclude = getattr(options, 'exclude', None) + class ModelFormMetaclass(type): def __new__(cls, name, bases, attrs, formfield_callback=lambda f: f.formfield()): - 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)) + try: + parents = [b for b in bases if issubclass(b, ModelForm)] + except NameError: + # We are defining ModelForm itself. + parents = None + if not parents: + return super(ModelFormMetaclass, cls).__new__(cls, name, bases, + attrs) - # 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 - declared_fields = SortedDict(fields) + new_class = type.__new__(cls, name, bases, attrs) + declared_fields = get_declared_fields(bases, attrs) + opts = new_class._meta = ModelFormOptions(getattr(new_class, 'Meta', None)) + if opts.model: + # If a model is defined, extract form fields from it. + fields = fields_for_model(opts.model, opts.fields, + opts.exclude, formfield_callback) + # Fields defined on the base classes override local fields and are + # always included. + fields.update(declared_fields) + else: + fields = declared_fields + new_class.base_fields = fields - opts = ModelFormOptions(attrs.get('Meta', None)) - attrs['_meta'] = opts + # XXX: The following is a sanity check for the user to avoid + # inadvertent attribute hiding. - # Don't allow more than one Meta model definition in bases. The fields - # would be generated correctly, but the save method won't deal with - # more than one object. - base_models = [] - for base in bases: + # 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 is not None: - base_models.append(base_model) - if len(base_models) > 1: - raise ImproperlyConfigured("%s's base classes define more than one model." % name) + 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 - # If a model is defined, extract form fields from it and add them to base_fields - if attrs['_meta'].model is not None: - # Don't allow a subclass to define a different Meta model than a - # parent class has. Technically the right fields would be generated, - # but the save method will not deal with more than one model. - for base in bases: - base_opts = getattr(base, '_meta', None) - base_model = getattr(base_opts, 'model', None) - if base_model and base_model is not opts.model: - raise ImproperlyConfigured('%s defines a different model than its parent.' % name) - model_fields = fields_for_model(opts.model, opts.fields, - opts.exclude, formfield_callback) - # fields declared in base classes override fields from the model - model_fields.update(declared_fields) - attrs['base_fields'] = model_fields - else: - attrs['base_fields'] = declared_fields - return type.__new__(cls, name, bases, attrs) + return new_class class BaseModelForm(BaseForm): def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None, - initial=None, error_class=ErrorList, label_suffix=':', instance=None): + initial=None, error_class=ErrorList, label_suffix=':', + instance=None): opts = self._meta if instance is None: # if we didn't get an instance, instantiate a new one @@ -277,7 +278,8 @@ class BaseModelForm(BaseForm): def save(self, commit=True): """ - Saves this ``form``'s cleaned_data into model instance ``self.instance``. + Saves this ``form``'s cleaned_data into model instance + ``self.instance``. If commit=True, then the changes to ``instance`` will be saved to the database. Returns ``instance``. diff --git a/docs/modelforms.txt b/docs/modelforms.txt index a99e27fff7..ec0ecf40cc 100644 --- a/docs/modelforms.txt +++ b/docs/modelforms.txt @@ -320,3 +320,41 @@ parameter when declaring the form field:: ... ... class Meta: ... model = Article + +Form inheritance +---------------- +As with the basic forms, you can extend and reuse ``ModelForms`` by inheriting +them. Normally, this will be useful if you need to declare some extra fields +or extra methods on a parent class for use in a number of forms derived from +models. For example, using the previous ``ArticleForm`` class:: + + >>> class EnhancedArticleForm(ArticleForm): + ... def clean_pub_date(self): + ... ... + +This creates a form that behaves identically to ``ArticleForm``, except there +is some extra validation and cleaning for the ``pub_date`` 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 + ``Meta`` of the first parent, etc. + + * 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 f1fed8f1e1..0d429b2a71 100644 --- a/tests/modeltests/model_forms/models.py +++ b/tests/modeltests/model_forms/models.py @@ -64,11 +64,11 @@ class TextFile(models.Model): def __unicode__(self): return self.description - + class ImageFile(models.Model): description = models.CharField(max_length=20) image = models.FileField(upload_to=tempfile.gettempdir()) - + def __unicode__(self): return self.description @@ -160,7 +160,7 @@ familiar with the mechanics. ... model = Article Traceback (most recent call last): ... -ImproperlyConfigured: BadForm defines a different model than its parent. +ImproperlyConfigured: BadForm's base classes define more than one model. >>> class ArticleForm(ModelForm): ... class Meta: @@ -179,6 +179,20 @@ This one is OK since the subclass specifies the same model as the parent. ... model = Category +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 CategoryForm(ModelForm): +... class Meta: +... model = Category +... exclude = ['url'] +>>> class SubCategoryForm(CategoryForm): +... pass + +>>> print SubCategoryForm() +