Fixed #12237 -- Improved the error message for m2m fields with an explicit through model being listed in admin fieldsets. Thanks to Pyth for the report and Ramiro Morales for the patch.

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

django/contrib/admin/validation.py

@@ -198,7 +198,9 @@ def validate_base(cls, model):
             check_formfield(cls, model, opts, 'fields', field)
             f = get_field(cls, model, opts, 'fields', field)
             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))
+                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)):
@@ -217,11 +219,23 @@ def validate_base(cls, model):
                 raise ImproperlyConfigured("'fields' key is required in "
                         "%s.fieldsets[%d][1] field options dict."
                         % (cls.__name__, idx))
+            for field in fieldset[1]['fields']:
+                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__)
-        for field in flattened_fieldsets:
+
-            check_formfield(cls, model, opts, "fieldsets[%d][1]['fields']" % idx, field)
 
     # form
     if hasattr(cls, 'form') and not issubclass(cls.form, BaseModelForm):

tests/regressiontests/admin_validation/models.py

@@ -108,8 +108,9 @@ Exception: <class 'regressiontests.admin_validation.models.TwoAlbumFKAndAnE'> ha
 
 >>> validate_inline(TwoAlbumFKAndAnEInline, None, Album)
 
-# Regression test for #12203 - Fail more gracefully when a M2M field that
+# Regression test for #12203/#12237 - Fail more gracefully when a M2M field that
-# specifies the 'through' option is included in the 'fields' ModelAdmin option.
+# specifies the 'through' option is included in the 'fields' or the 'fieldsets'
+# ModelAdmin options.
 
 >>> class BookAdmin(admin.ModelAdmin):
 ...     fields = ['authors']
@@ -119,6 +120,17 @@ Traceback (most recent call last):
     ...
 ImproperlyConfigured: 'BookAdmin.fields' can't include the ManyToManyField field 'authors' because 'authors' manually specifies a 'through' model.
 
+>>> class FieldsetBookAdmin(admin.ModelAdmin):
+...     fieldsets = (
+...         ('Header 1', {'fields': ('name',)}),
+...         ('Header 2', {'fields': ('authors',)}),
+...     )
+
+>>> validate(FieldsetBookAdmin, Book)
+Traceback (most recent call last):
+   ...
+ImproperlyConfigured: 'FieldsetBookAdmin.fieldsets[1][1]['fields']' can't include the ManyToManyField field 'authors' because 'authors' manually specifies a 'through' model.
+
 # Regression test for #12209 -- If the explicitly provided through model
 # is specified as a string, the admin should still be able use
 # Model.m2m_field.through