Ticket #6630: 00-fieldsets.diff

django/contrib/admin/options.py

@@ -267 +267 @@
             exclude = []
         else:
             exclude = list(self.exclude)
+        exclude += kwargs.get("exclude", [])
+        if not exclude:
+            exclude = None
         defaults = {
             "form": self.form,
             "fields": fields,
-            "exclude": exclude + kwargs.get("exclude", []),
+            "exclude": exclude,
             "formfield_callback": self.formfield_for_dbfield,
         }
         defaults.update(kwargs)

django/contrib/auth/forms.py

@@ -19 +19 @@
 
     class Meta:
         model = User
-        fields = ("username",)
+        fields = ("username", "password1", "password2")
 
     def clean_username(self):
         username = self.cleaned_data["username"]

django/forms/forms.py

@@ -4 +4 @@
 
 from copy import deepcopy
 
+from django.core.exceptions import ImproperlyConfigured
 from django.utils.datastructures import SortedDict
 from django.utils.html import escape
 from django.utils.encoding import StrAndUnicode, smart_unicode, force_unicode
@@ -22 +23 @@
     name = name[0].upper() + name[1:]
     return name.replace('_', ' ')
 
-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.
+class FormOptions(object):
+    def __init__(self, options=None):
+        self.fieldsets = getattr(options, 'fieldsets', None)
+        self.fields = getattr(options, 'fields', None)
+        self.exclude = getattr(options, 'exclude', None)
 
-    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.
-    Also integrates any additional media definitions
-    """
-    fields = [(field_name, attrs.pop(field_name)) for field_name, obj in attrs.items() if isinstance(obj, Field)]
+def create_declared_fields(cls, attrs):
+    """
+    Create a list of form field instances from the passed in 'attrs'.
+    This is used by both the Form and ModelForm metaclasses.
+    """
+    fields = []
+    for name, possible_field in attrs.items():
+        if isinstance(possible_field, Field):
+            fields.append((name, possible_field))
+            delattr(cls, name)
     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.
-    if with_base_fields:
-        for base in bases[::-1]:
-            if hasattr(base, 'base_fields'):
-                fields = base.base_fields.items() + fields
+    cls.declared_fields = SortedDict(fields)
+
+def create_base_fields_pool_from_declared_fields(cls, attrs):
+    """
+    Create a list of form field instances which are declared in the form and
+    its superclasses (from 'csl.__mro__'). This is used by the Form metaclass.
+
+    Note that we loop over the bases in *reverse*. This is necessary in
+    order to preserve the correct order of fields.
+    """
+    fields = []
+    for base in cls.__mro__[::-1]:
+        try:
+            fields += base.declared_fields.items() # Raise AttributeError if the base is not a form.
+        except AttributeError:
+            pass
+    cls.base_fields_pool = SortedDict(fields)
+
+def create_base_fields_from_base_fields_pool(cls, attrs):
+    """
+    Create a list of form field instances from the base fields pool. Select
+    only the fields which are defined in one of the options 'fieldsets',
+    'fields' and 'exclude'. If no option is set, select all fields.
+    This is used by both the Form and ModelForm metaclasses.
+
+    Also check that only one option is used.
+    """
+    if (cls._meta.fieldsets is None) + (cls._meta.fields is None) + (cls._meta.exclude is None) < 2:
+        raise ImproperlyConfigured("%s cannot have more than one option from fieldsets, fields and exclude." % cls.__name__)
+    if cls._meta.fieldsets:
+        names = []
+        for fieldset in cls._meta.fieldsets:
+            names.extend(fieldset['fields'])
+    elif cls._meta.fields:
+        names = cls._meta.fields
+    elif cls._meta.exclude:
+        names = [name for name in cls.base_fields_pool if name not in cls._meta.exclude]
     else:
-        for base in bases[::-1]:
-            if hasattr(base, 'declared_fields'):
-                fields = base.declared_fields.items() + fields
-
-    return SortedDict(fields)
-
-class DeclarativeFieldsMetaclass(type):
+        names = cls.base_fields_pool.keys()
+    cls.base_fields = SortedDict([(name, cls.base_fields_pool[name]) for name in names])
+
+class FormMetaclass(type):
     """
     Metaclass that converts Field attributes to a dictionary called
     'base_fields', taking into account parent class 'base_fields' as well.
+
+    Also integrates any additional media definitions
     """
     def __new__(cls, name, bases, attrs):
-        attrs['base_fields'] = get_declared_fields(bases, attrs)
-        new_class = super(DeclarativeFieldsMetaclass,
-                     cls).__new__(cls, name, bases, attrs)
+        new_class = type.__new__(cls, name, bases, attrs)
+        new_class._meta = FormOptions(getattr(new_class, 'Meta', None))
+        create_declared_fields(new_class, attrs)
+        create_base_fields_pool_from_declared_fields(new_class, attrs)
+        create_base_fields_from_base_fields_pool(new_class, attrs)
         if 'media' not in attrs:
             new_class.media = media_property(new_class)
         return new_class
@@ -105 +140 @@
             raise KeyError('Key %r not found in Form' % name)
         return BoundField(self, field, name)
 
+    def has_fieldsets(self):
+        return self._meta.fieldsets is not None
+    
+    def fieldsets(self):
+        if self.has_fieldsets():
+            for fieldset in self._meta.fieldsets:
+                yield {
+                    'attrs': fieldset.get('attrs', {}),
+                    'legend': fieldset.get('legend', u''),
+                    'fields': [self[name] for name in fieldset['fields']],
+                } 
+    
     def _get_errors(self):
         "Returns an ErrorDict for the data provided for the form"
         if self._errors is None:
@@ -310 +357 @@
     # fancy metaclass stuff purely for the semantic sugar -- it allows one
     # to define a form using declarative syntax.
     # BaseForm itself has no way of designating self.fields.
-    __metaclass__ = DeclarativeFieldsMetaclass
+    __metaclass__ = FormMetaclass
 
 class BoundField(StrAndUnicode):
     "A Field plus data"

django/forms/models.py

@@ -9 +9 @@
 from django.utils.translation import ugettext_lazy as _
 
 from util import ValidationError, ErrorList
-from forms import BaseForm, get_declared_fields
+from forms import FormOptions, BaseForm, create_declared_fields
+from forms import create_base_fields_from_base_fields_pool
 from fields import Field, ChoiceField, IntegerField, EMPTY_VALUES
 from widgets import Select, SelectMultiple, HiddenInput, MultipleHiddenInput
 from widgets import media_property
@@ -154 +155 @@
             field_list.append((f.name, formfield))
     return SortedDict(field_list)
 
-class ModelFormOptions(object):
+class ModelFormOptions(FormOptions):
     def __init__(self, options=None):
+        super(ModelFormOptions, self).__init__(options)
         self.model = getattr(options, 'model', None)
-        self.fields = getattr(options, 'fields', None)
-        self.exclude = getattr(options, 'exclude', None)
-
+
+def create_model_fields(cls, attrs):
+    """
+    Create a list of form field instances from the option 'model'.
+    This is used by the ModelForm metaclass.
+    """
+    formfield_callback = attrs.pop('formfield_callback', lambda f: f.formfield())
+    fields = []
+    if cls._meta.model:
+        for dbfield in cls._meta.model._meta.fields + cls._meta.model._meta.many_to_many:
+            if dbfield.editable:
+                formfield = formfield_callback(dbfield)
+                if formfield:
+                    fields.append((dbfield.name, formfield))
+    cls.model_fields = SortedDict(fields)
+
+def create_base_fields_pool_from_model_fields_and_declared_fields(cls, attrs):
+    """
+    Create a list of form field instances which are declared in the form and
+    its superclasses (from 'csl.__mro__'). Add fields from the last form
+    with a model. This is used by the MetaclasForm metaclass.
+
+    Note that we loop over the bases in *reverse*. This is necessary in
+    order to preserve the correct order of fields.
+    """
+    model_fields, declared_fields = [], []
+    for base in cls.__mro__[::-1]:
+        try:
+            declared_fields += base.declared_fields.items() # Raise AttributeError if the base is not a form.
+            if base._meta.model: # Raise AttributeError if the base is not a model form.
+                model_fields = base.model_fields.items()
+        except AttributeError:
+            pass
+    cls.base_fields_pool = SortedDict(model_fields + declared_fields)
 
 class ModelFormMetaclass(type):
+    """
+    Metaclass that converts Field attributes to a dictionary called
+    'base_fields', taking into account parent class 'base_fields' as well.
+    Add fields from the class' model.
+
+    Also integrates any additional media definitions
+    """
     def __new__(cls, name, bases, attrs):
-        formfield_callback = attrs.pop('formfield_callback',
-                lambda f: f.formfield())
-        try:
-            parents = [b for b in bases if issubclass(b, ModelForm)]
-        except NameError:
-            # We are defining ModelForm itself.
-            parents = None
-        declared_fields = get_declared_fields(bases, attrs, False)
-        new_class = super(ModelFormMetaclass, cls).__new__(cls, name, bases,
-                attrs)
-        if not parents:
-            return new_class
-
+        new_class = type.__new__(cls, name, bases, attrs)
+        new_class._meta = ModelFormOptions(getattr(new_class, 'Meta', None))
+        create_model_fields(new_class, attrs)
+        create_declared_fields(new_class, attrs)
+        create_base_fields_pool_from_model_fields_and_declared_fields(new_class, attrs)
+        create_base_fields_from_base_fields_pool(new_class, attrs)
         if 'media' not in attrs:
             new_class.media = media_property(new_class)
-        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)
-            # Override default model fields with any custom declared ones
-            # (plus, include all the other declared fields).
-            fields.update(declared_fields)
-        else:
-            fields = declared_fields
-        new_class.declared_fields = declared_fields
-        new_class.base_fields = fields
         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=':',
                  empty_permitted=False, instance=None):
-        opts = self._meta
         if instance is None:
             # if we didn't get an instance, instantiate a new one
-            self.instance = opts.model()
+            self.instance = self._meta.model()
             object_data = {}
         else:
             self.instance = instance
-            object_data = model_to_dict(instance, opts.fields, opts.exclude)
+            object_data = model_to_dict(instance)
         # if initial was provided, it should override the values from instance
         if initial is not None:
             object_data.update(initial)
@@ -309 +329 @@
             fail_message = 'created'
         else:
             fail_message = 'changed'
-        return save_instance(self, self.instance, self._meta.fields, fail_message, commit)
+        return save_instance(self, self.instance, self.fields.keys(), fail_message, commit)
 
 class ModelForm(BaseModelForm):
     __metaclass__ = ModelFormMetaclass
 
-def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
+def modelform_factory(model, form=ModelForm, fields=None, exclude=None, fieldsets=None,
                        formfield_callback=lambda f: f.formfield()):
     # HACK: we should be able to construct a ModelForm without creating
     # and passing in a temporary inner class
     class Meta:
         pass
     setattr(Meta, 'model', model)
+    setattr(Meta, 'fieldsets', fieldsets)
     setattr(Meta, 'fields', fields)
     setattr(Meta, 'exclude', exclude)
     class_name = model.__name__ + 'Form'
@@ -430 +451 @@
 def modelformset_factory(model, form=ModelForm, formfield_callback=lambda f: f.formfield(),
                          formset=BaseModelFormSet,
                          extra=1, can_delete=False, can_order=False,
-                         max_num=0, fields=None, exclude=None):
+                         max_num=0, fields=None, exclude=None, fieldsets=None):
     """
     Returns a FormSet class for the given Django model class.
     """
     form = modelform_factory(model, form=form, fields=fields, exclude=exclude,
+                             fieldsets=fieldsets,
                              formfield_callback=formfield_callback)
     FormSet = formset_factory(form, formset, extra=extra, max_num=max_num,
                               can_order=can_order, can_delete=can_delete)
@@ -524 +546 @@
 
 def inlineformset_factory(parent_model, model, form=ModelForm,
                           formset=BaseInlineFormSet, fk_name=None,
-                          fields=None, exclude=None,
+                          fields=None, exclude=None, fieldsets=None,
                           extra=3, can_order=False, can_delete=True, max_num=0,
                           formfield_callback=lambda f: f.formfield()):
     """
@@ -549 +571 @@
         'extra': extra,
         'can_delete': can_delete,
         'can_order': can_order,
+        'fieldsets': fieldsets,
         'fields': fields,
         'exclude': exclude,
         'max_num': max_num,

tests/modeltests/model_forms/models.py

@@ -217 +217 @@
 ...         model = Category
 ...         fields = ['name', 'url']
 ...         exclude = ['url']
-
->>> CategoryForm.base_fields.keys()
-['name']
+Traceback (most recent call last):
+  File "/home/petr/django/local2/00-forms-fieldsets/django/test/_doctest.py", line 1267, in __run
+    compileflags, 1) in test.globs
+  File "<doctest modeltests.model_forms.models.__test__.API_TESTS[12]>", line 1, in ?
+    class CategoryForm(ModelForm):
+  File "/home/petr/django/local2/00-forms-fieldsets/django/forms/models.py", line 220, in __new__
+    metaclassing.create_base_fields_from_base_fields_pool(new_class)
+  File "/home/petr/django/local2/00-forms-fieldsets/django/forms/metaclassing.py", line 50, in create_base_fields_from_base_fields_pool
+    raise ImproperlyConfigured("%s cannot have more than one option from fieldsets, fields and exclude." % cls.__name__)
+ImproperlyConfigured: CategoryForm cannot have more than one option from fieldsets, fields and exclude.
 
 Don't allow more than one 'model' definition in the inheritance hierarchy.
 Technically, it would generate a valid form, but the fact that the resulting

tests/regressiontests/forms/forms.py

@@ -1265 +1265 @@
 ...     haircut_type = CharField()
 >>> b = Beatle(auto_id=False)
 >>> print b.as_ul()
+<li>Instrument: <input type="text" name="instrument" /></li>
 <li>First name: <input type="text" name="first_name" /></li>
 <li>Last name: <input type="text" name="last_name" /></li>
 <li>Birthday: <input type="text" name="birthday" /></li>
-<li>Instrument: <input type="text" name="instrument" /></li>
 <li>Haircut type: <input type="text" name="haircut_type" /></li>
 
 # Forms with prefixes #########################################################
@@ -1749 +1749 @@
 >>> form.is_valid()
 True
 
+# Forms with meta attributes fields, exclude and fielsets #####################
+ 
+>>> class UserForm(Form):
+...     username = CharField()
+...     email = CharField(widget=PasswordInput)
+...     first_name = CharField()
+...     last_name = CharField()
+ 
+>>> t = Template('''
+... <form action="">
+... {% if form.has_fieldsets %}
+... {% for fieldset in form.fieldsets %}
+...     <fieldset>
+...     {% if fieldset.legend %}
+...         <legend>{{ fieldset.legend }}</legend>
+...     {% endif %}
+...     {% for field in fieldset.fields %} 
+...         <p><label>{{ field.label }}: {{ field }}</label></p>
+...     {% endfor %}
+...     </fieldset>
+... {% endfor %}
+... {% else %}
+... {% for field in form %}
+...     <p><label>{{ field.label }}: {{ field }}</label></p>
+... {% endfor %}
+... {% endif %}
+...     <input type="submit" />
+... </form>''')
+ 
+>>> clean_re = re.compile(r'\n( *\n)+')
+>>> clean = lambda text: clean_re.sub('\n', text).strip()
+ 
+>>> print clean(t.render(Context({'form': UserForm()})))
+<form action="">
+    <p><label>Username: <input type="text" name="username" id="id_username" /></label></p>
+    <p><label>Email: <input type="password" name="email" id="id_email" /></label></p>
+    <p><label>First name: <input type="text" name="first_name" id="id_first_name" /></label></p>
+    <p><label>Last name: <input type="text" name="last_name" id="id_last_name" /></label></p>
+    <input type="submit" />
+</form>
+ 
+>>> class OrderingUserForm(UserForm):
+...     class Meta:
+...         fields = ('first_name', 'last_name', 'username', 'email')
+
+>>> print clean(t.render(Context({'form': OrderingUserForm()})))
+<form action="">
+    <p><label>First name: <input type="text" name="first_name" id="id_first_name" /></label></p>
+    <p><label>Last name: <input type="text" name="last_name" id="id_last_name" /></label></p>
+    <p><label>Username: <input type="text" name="username" id="id_username" /></label></p>
+    <p><label>Email: <input type="password" name="email" id="id_email" /></label></p>
+    <input type="submit" />
+</form>
+
+>>> class FilteringUserForm(UserForm):
+...     class Meta:
+...         fields = ('first_name', 'last_name')
+
+>>> print clean(t.render(Context({'form': FilteringUserForm()})))
+<form action="">
+    <p><label>First name: <input type="text" name="first_name" id="id_first_name" /></label></p>
+    <p><label>Last name: <input type="text" name="last_name" id="id_last_name" /></label></p>
+    <input type="submit" />
+</form>
+
+>>> class ExcludingUserForm(UserForm):
+...     class Meta:
+...         exclude = ('first_name', 'last_name')
+
+>>> print clean(t.render(Context({'form': ExcludingUserForm()})))
+<form action="">
+    <p><label>Username: <input type="text" name="username" id="id_username" /></label></p>
+    <p><label>Email: <input type="password" name="email" id="id_email" /></label></p>
+    <input type="submit" />
+</form>
+
+>>> class FieldsetUserForm(UserForm):
+...     class Meta:
+...         fieldsets = (
+...             {'fields': ('username', 'email')},
+...             {'fields': ('first_name', 'last_name'), 'legend': 'Name'},
+...         )
+
+>>> print clean(t.render(Context({'form': FieldsetUserForm()})))
+<form action="">
+    <fieldset>
+        <p><label>Username: <input type="text" name="username" id="id_username" /></label></p>
+        <p><label>Email: <input type="password" name="email" id="id_email" /></label></p>
+    </fieldset>
+    <fieldset>
+        <legend>Name</legend>
+        <p><label>First name: <input type="text" name="first_name" id="id_first_name" /></label></p>
+        <p><label>Last name: <input type="text" name="last_name" id="id_last_name" /></label></p>
+    </fieldset>
+    <input type="submit" />
+</form>
+
 """

tests/regressiontests/modeladmin/models.py

@@ -132 +132 @@
 >>> ma.get_form(request).base_fields.keys() 
 ['name', 'sign_date']
  
-# Using `fields` and `exclude`.
-
->>> class BandAdmin(ModelAdmin): 
-...     fields = ['name', 'bio'] 
-...     exclude = ['bio'] 
->>> ma = BandAdmin(Band, site) 
->>> ma.get_form(request).base_fields.keys() 
-['name']
-
 If we specify a form, it should use it allowing custom validation to work
 properly. This won't, however, break any of the admin widgets or media.