jazzband / django-authority

A Django app that provides generic per-object-permissions for Django's auth app and helpers to create custom permission checks.
http://django-authority.readthedocs.org
BSD 3-Clause "New" or "Revised" License
292 stars 57 forks source link

Fix TypeError in edit_permissions #11

Closed gthb closed 10 years ago

gthb commented 10 years ago

Fix a somewhat opaque TypeError failure in edit_permissions when running django-authority under Django 1.5.1, by properly delegating to superclass in creating a form field.

See https://code.djangoproject.com/ticket/20744 for detail on the Django Forms API subtlety that the superclass method works around (removing request from kwargs before passing them on to field creation).

The error looks like this:

TypeError: __init__() got an unexpected keyword argument 'request'

Stacktrace (most recent call last):

  File "django/core/handlers/base.py", line 115, in get_response
    response = callback(request, *callback_args, **callback_kwargs)
  File "django/contrib/admin/options.py", line 372, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "django/utils/decorators.py", line 91, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "django/views/decorators/cache.py", line 89, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "django/contrib/admin/sites.py", line 202, in inner
    return view(request, *args, **kwargs)
  File "django/utils/decorators.py", line 25, in _wrapper
    return bound_func(*args, **kwargs)
  File "django/utils/decorators.py", line 91, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "django/utils/decorators.py", line 21, in bound_func
    return func(self, *args2, **kwargs2)
  File "django/contrib/admin/options.py", line 1205, in changelist_view
    response = self.response_action(request, queryset=cl.get_query_set(request))
  File "django/contrib/admin/options.py", line 960, in response_action
    response = func(self, request, queryset)
  File "authority/admin.py", line 61, in edit_permissions
    FormSet = inline.get_formset(request, obj)
  File "django/contrib/contenttypes/generic.py", line 495, in get_formset
    return generic_inlineformset_factory(self.model, **defaults)
  File "django/contrib/contenttypes/generic.py", line 455, in generic_inlineformset_factory
    fields=fields, exclude=exclude, max_num=max_num)
  File "django/forms/models.py", line 692, in modelformset_factory
    formfield_callback=formfield_callback)
  File "django/forms/models.py", line 424, in modelform_factory
    return type(form)(class_name, (form,), form_class_attrs)
  File "django/forms/models.py", line 212, in __new__
    opts.exclude, opts.widgets, formfield_callback)
  File "django/forms/models.py", line 170, in fields_for_model
    formfield = formfield_callback(f, **kwargs)
  File "authority/admin.py", line 33, in formfield_for_dbfield
    return db_field.formfield(**kwargs)
  File "django/db/models/fields/__init__.py", line 646, in formfield
    return super(CharField, self).formfield(**defaults)
  File "django/db/models/fields/__init__.py", line 499, in formfield
    return form_class(**defaults)
  File "django/forms/fields.py", line 188, in __init__
    super(CharField, self).__init__(*args, **kwargs)
winhamwr commented 10 years ago

Is this fix still Django 1.3 compatible? We don't actually have any test coverage against the admin.

I would really love to see a test case with this pull request that demonstrates this bug. Is that something you would be able to add?

Thanks -Wes

gthb commented 10 years ago

Sorry, I can't spare the time to introduce test coverage here. This is manually verified though. It was in upgrading our codebase to use Django 1.3 that I encountered this problem, and this change got it fixed. The problem itself is described in https://code.djangoproject.com/ticket/20744 including a stack trace, but no full test setup.

We then upgraded further and are now running Django 1.5, where some further changes to django-authority were needed (that's my other pull request here). So we run in production with a patched version of django-authority, including these changes.