Opened 5 years ago

Closed 5 years ago

#12237 closed (fixed)

Attempting to use a ManyToManyField with 'through' in ModelAdmin gives uninformative help message if using `fieldsets`

Reported by: Pyth Owned by: ramiro
Component: contrib.admin Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Attempting to use a ManyToManyField with 'through' in ModelAdmin gives an uninformative help message if fieldsets are used to select the fields to use in the form instead of fields. I've confirmed this problem in the latest SVN revision, r11743.

Sample models

Here are three models with a ManyToManyField relationship declared via through.

class Person(models.Model):
    name = models.CharField(max_length=128)

    def __unicode__(self):
        return self.name

class Group(models.Model):
    name = models.CharField(max_length=128)
    members = models.ManyToManyField(Person, through='Membership')

    def __unicode__(self):
        return self.name

class Membership(models.Model):
    person = models.ForeignKey(Person)
    group = models.ForeignKey(Group)
    date_joined = models.DateField()
    invite_reason = models.CharField(max_length=64)

Helpful error when using fields

Creating an admin model using fields to construct it gives an extremely helpful error message when attempting to use the members field in the form.

class GroupAdmin(admin.ModelAdmin):
    fields = ('name', 'members')

Error: ImproperlyConfigured: 'GroupAdmin.fields' can't include the ManyToManyField field 'members' because 'members' manually specifies a 'through' model.

Less helpful error when using fieldsets

However, doing the same thing with a fieldsets approach will give the user a much more confusing error.

class GroupAdmin(admin.ModelAdmin):
    fieldsets = (
        (None, {'fields': ('name', 'members')}),
    )

Error: Caught an exception while rendering: 'NoneType' object has no attribute 'label'

The traceback isn't excessivley informative either:

File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/core/handlers/base.py" in get_response
  99.                     response = callback(request, *callback_args, **callback_kwargs)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/contrib/admin/options.py" in wrapper
  228.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/utils/decorators.py" in __call__
  36.         return self.decorator(self.func)(*args, **kwargs)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/utils/decorators.py" in _wrapped_view
  86.                     response = view_func(request, *args, **kwargs)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/utils/decorators.py" in __call__
  36.         return self.decorator(self.func)(*args, **kwargs)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/views/decorators/cache.py" in _wrapped_view_func
  70.         response = view_func(request, *args, **kwargs)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/contrib/admin/sites.py" in inner
  187.             return view(request, *args, **kwargs)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/utils/decorators.py" in _wrapped_view
  86.                     response = view_func(request, *args, **kwargs)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/db/transaction.py" in _commit_on_success
  240.                 res = func(*args, **kw)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/contrib/admin/options.py" in add_view
  788.         return self.render_change_form(request, context, form_url=form_url, add=True)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/contrib/admin/options.py" in render_change_form
  592.         ], context, context_instance=context_instance)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/shortcuts/__init__.py" in render_to_response
  20.     return HttpResponse(loader.render_to_string(*args, **kwargs), **httpresponse_kwargs)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/loader.py" in render_to_string
  108.     return t.render(context_instance)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/__init__.py" in render
  178.         return self.nodelist.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/__init__.py" in render
  779.                 bits.append(self.render_node(node, context))
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/debug.py" in render_node
  71.             result = node.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/loader_tags.py" in render
  97.         return compiled_parent.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/__init__.py" in render
  178.         return self.nodelist.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/__init__.py" in render
  779.                 bits.append(self.render_node(node, context))
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/debug.py" in render_node
  71.             result = node.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/loader_tags.py" in render
  97.         return compiled_parent.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/__init__.py" in render
  178.         return self.nodelist.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/__init__.py" in render
  779.                 bits.append(self.render_node(node, context))
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/debug.py" in render_node
  71.             result = node.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/loader_tags.py" in render
  24.         result = self.nodelist.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/__init__.py" in render
  779.                 bits.append(self.render_node(node, context))
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/debug.py" in render_node
  71.             result = node.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/defaulttags.py" in render
  172.                 nodelist.append(node.render(context))
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/loader_tags.py" in render
  111.             return self.template.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/__init__.py" in render
  178.         return self.nodelist.render(context)
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/__init__.py" in render
  779.                 bits.append(self.render_node(node, context))
File "/Users/corona/.virtualenvs/grapewire/src/django-trunk/django/template/debug.py" in render_node
  81.             raise wrapped

Attachments (1)

12237-m2m-through-modeladmin-fieldsets.diff (2.4 KB) - added by ramiro 5 years ago.
Patch that implements the validation also for the fielsets ModelAdmin option, includes tests.

Download all attachments as: .zip

Change History (4)

Changed 5 years ago by ramiro

Patch that implements the validation also for the fielsets ModelAdmin option, includes tests.

comment:1 Changed 5 years ago by ramiro

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to ramiro
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 5 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [11744]) 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.

Note: See TracTickets for help on using tickets.
Back to Top