Permalink
Browse files

Fixed #12237 -- Improved the error message for m2m fields with an exp…

…licit 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
  • Loading branch information...
1 parent cd0d7e6 commit bb4062d53b0caae84732c5940a97e3da1b14823f @freakboy3742 freakboy3742 committed Nov 18, 2009
Showing with 31 additions and 5 deletions.
  1. +17 −3 django/contrib/admin/validation.py
  2. +14 −2 tests/regressiontests/admin_validation/models.py
View
20 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):
View
16 tests/regressiontests/admin_validation/models.py
@@ -108,8 +108,9 @@ class AuthorsBooks(models.Model):
>>> validate_inline(TwoAlbumFKAndAnEInline, None, Album)
-# Regression test for #12203 - Fail more gracefully when a M2M field that
-# specifies the 'through' option is included in the 'fields' ModelAdmin option.
+# Regression test for #12203/#12237 - Fail more gracefully when a M2M field that
+# specifies the 'through' option is included in the 'fields' or the 'fieldsets'
+# ModelAdmin options.
>>> class BookAdmin(admin.ModelAdmin):
... fields = ['authors']
@@ -119,6 +120,17 @@ class AuthorsBooks(models.Model):
...
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

0 comments on commit bb4062d

Please sign in to comment.