Added ability to describe grouping of form fields in the same row to the `fields` ModelAdmin attribute.

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

django/contrib/admin/validation.py

@@ -222,6 +222,40 @@ def validate_inline(cls, parent, parent_model):
     if hasattr(cls, "readonly_fields"):
         check_readonly_fields(cls, cls.model, cls.model._meta)
 
+def validate_fields_spec(cls, model, opts, flds, label):
+    """
+    Validate the fields specification in `flds` from a ModelAdmin subclass
+    `cls` for the `model` model. `opts` is `model`'s Meta inner class.
+    Use `label` for reporting problems to the user.
+
+    The fields specification can be a ``fields`` option or a ``fields``
+    sub-option from a ``fieldsets`` option component.
+    """
+    for fields in flds:
+        # The entry in fields might be a tuple. If it is a standalone
+        # field, make it into a tuple to make processing easier.
+        if type(fields) != tuple:
+            fields = (fields,)
+        for field in fields:
+            if field in cls.readonly_fields:
+                # Stuff can be put in fields that isn't actually a
+                # model field if it's in readonly_fields,
+                # readonly_fields will handle the validation of such
+                # things.
+                continue
+            check_formfield(cls, model, opts, label, field)
+            try:
+                f = opts.get_field(field)
+            except models.FieldDoesNotExist:
+                # If we can't find a field on the model that matches,
+                # it could be an extra field on the form.
+                pass
+            if isinstance(f, models.ManyToManyField) and not f.rel.through._meta.auto_created:
+                raise ImproperlyConfigured("'%s.%s' "
+                    "can't include the ManyToManyField field '%s' because "
+                    "'%s' manually specifies a 'through' model." % (
+                        cls.__name__, label, field, field))
+
 def validate_base(cls, model):
     opts = model._meta
 
@@ -238,23 +272,7 @@ def validate_base(cls, model):
     # fields
     if cls.fields: # default value is None
         check_isseq(cls, 'fields', cls.fields)
-        for field in cls.fields:
+        validate_fields_spec(cls, model, opts, cls.fields, 'fields')
-            if field in cls.readonly_fields:
-                # Stuff can be put in fields that isn't actually a model field
-                # if it's in readonly_fields, readonly_fields will handle the
-                # validation of such things.
-                continue
-            check_formfield(cls, model, opts, 'fields', field)
-            try:
-                f = opts.get_field(field)
-            except models.FieldDoesNotExist:
-                # If we can't find a field on the model that matches,
-                # it could be an extra field on the form.
-                continue
-            if isinstance(f, models.ManyToManyField) and not f.rel.through._meta.auto_created:
-                raise ImproperlyConfigured("'%s.fields' can't include the ManyToManyField "
-                    "field '%s' because '%s' manually specifies "
-                    "a 'through' model." % (cls.__name__, field, field))
         if cls.fieldsets:
             raise ImproperlyConfigured('Both fieldsets and fields are specified in %s.' % cls.__name__)
         if len(cls.fields) > len(set(cls.fields)):
@@ -273,30 +291,7 @@ def validate_base(cls, model):
                 raise ImproperlyConfigured("'fields' key is required in "
                         "%s.fieldsets[%d][1] field options dict."
                         % (cls.__name__, idx))
-            for fields in fieldset[1]['fields']:
+            validate_fields_spec(cls, model, opts, fieldset[1]['fields'], "fieldsets[%d][1]['fields']" % idx)
-                # The entry in fields might be a tuple. If it is a standalone
-                # field, make it into a tuple to make processing easier.
-                if type(fields) != tuple:
-                    fields = (fields,)
-                for field in fields:
-                    if field in cls.readonly_fields:
-                        # Stuff can be put in fields that isn't actually a
-                        # model field if it's in readonly_fields,
-                        # readonly_fields will handle the validation of such
-                        # things.
-                        continue
-                    check_formfield(cls, model, opts, "fieldsets[%d][1]['fields']" % idx, field)
-                    try:
-                        f = opts.get_field(field)
-                        if isinstance(f, models.ManyToManyField) and not f.rel.through._meta.auto_created:
-                            raise ImproperlyConfigured("'%s.fieldsets[%d][1]['fields']' "
-                                "can't include the ManyToManyField field '%s' because "
-                                "'%s' manually specifies a 'through' model." % (
-                                    cls.__name__, idx, field, field))
-                    except models.FieldDoesNotExist:
-                        # If we can't find a field on the model that matches,
-                        # it could be an extra field on the form.
-                        pass
         flattened_fieldsets = flatten_fieldsets(cls.fieldsets)
         if len(flattened_fieldsets) > len(set(flattened_fieldsets)):
             raise ImproperlyConfigured('There are duplicate field(s) in %s.fieldsets' % cls.__name__)

docs/ref/contrib/admin/index.txt

@@ -160,27 +160,45 @@ subclass::
 
 .. attribute:: ModelAdmin.fields
 
-    Use this option as an alternative to ``fieldsets`` if the layout does not
+    If you need to achieve simple changes in the layout of fields in the forms
-    matter and if you want to only show a subset of the available fields in the
+    of the "add" and "change" pages like only showing a subset of the available
-    form. For example, you could define a simpler version of the admin form for
+    fields, modifying their order or grouping them in rows you can use the
-    the ``django.contrib.flatpages.FlatPage`` model as follows::
+    ``fields`` option (for more complex layout needs see the
+    :attr:`~ModelAdmin.fieldsets` option described in the next section). For
+    example, you could define a simpler version of the admin form for the
+    ``django.contrib.flatpages.FlatPage`` model as follows::
 
         class FlatPageAdmin(admin.ModelAdmin):
             fields = ('url', 'title', 'content')
 
-    In the above example, only the fields 'url', 'title' and 'content' will be
+    In the above example, only the fields ``url``, ``title`` and ``content``
-    displayed, sequentially, in the form.
+    will be displayed, sequentially, in the form.
 
     .. versionadded:: 1.2
 
     ``fields`` can contain values defined in :attr:`ModelAdmin.readonly_fields`
     to be displayed as read-only.
 
+    .. versionadded:: 1.4
+
+    To display multiple fields on the same line, wrap those fields in their own
+    tuple. In this example, the ``url`` and ``title`` fields will display on the
+    same line and the ``content`` field will be displayed below them in its
+    own line::
+
+        class FlatPageAdmin(admin.ModelAdmin):
+            fields = (('url', 'title'), 'content')
+
     .. admonition:: Note
 
         This ``fields`` option should not be confused with the ``fields``
-        dictionary key that is within the ``fieldsets`` option, as described in
+        dictionary key that is within the :attr:`~ModelAdmin.fieldsets` option,
-        the previous section.
+        as described in the next section.
+
+    If neither ``fields`` nor :attr:`~ModelAdmin.fieldsets` options are present,
+    Django will default to displaying each field that isn't an ``AutoField`` and
+    has ``editable=True``, in a single fieldset, in the same order as the fields
+    are defined in the model.
 
 .. attribute:: ModelAdmin.fieldsets
 
@@ -213,9 +231,10 @@ subclass::
 
         .. image:: _images/flatfiles_admin.png
 
-    If ``fieldsets`` isn't given, Django will default to displaying each field
+    If neither ``fieldsets`` nor :attr:`~ModelAdmin.fields` options are present,
-    that isn't an ``AutoField`` and has ``editable=True``, in a single
+    Django will default to displaying each field that isn't an ``AutoField`` and
-    fieldset, in the same order as the fields are defined in the model.
+    has ``editable=True``, in a single fieldset, in the same order as the fields
+    are defined in the model.
 
     The ``field_options`` dictionary can have the following keys:
 
@@ -229,9 +248,10 @@ subclass::
                 'fields': ('first_name', 'last_name', 'address', 'city', 'state'),
                 }
 
-            To display multiple fields on the same line, wrap those fields in
+            Just like with the :attr:`~ModelAdmin.fields` option, to display
-            their own tuple. In this example, the ``first_name`` and
+            multiple fields on the same line, wrap those fields in their own
-            ``last_name`` fields will display on the same line::
+            tuple. In this example, the ``first_name`` and ``last_name`` fields
+            will display on the same line::
 
                 {
                 'fields': (('first_name', 'last_name'), 'address', 'city', 'state'),

tests/regressiontests/admin_validation/tests.py

@@ -201,7 +201,7 @@ class ValidationTestCase(TestCase):
             validate,
             BookAdmin, Book)
 
-    def test_cannon_include_through(self):
+    def test_cannot_include_through(self):
         class FieldsetBookAdmin(admin.ModelAdmin):
             fieldsets = (
                 ('Header 1', {'fields': ('name',)}),
@@ -212,6 +212,11 @@ class ValidationTestCase(TestCase):
             validate,
             FieldsetBookAdmin, Book)
 
+    def test_nested_fields(self):
+        class NestedFieldsAdmin(admin.ModelAdmin):
+           fields = ('price', ('name', 'subtitle'))
+        validate(NestedFieldsAdmin, Book)
+
     def test_nested_fieldsets(self):
         class NestedFieldsetAdmin(admin.ModelAdmin):
            fieldsets = (