Opened 14 years ago

Closed 14 years ago

Last modified 12 years ago

#12804 closed (fixed)

decorating admin views doesn't work

Reported by: Florian Apolloner Owned by: nobody
Component: Uncategorized Version: dev
Severity: Keywords:
Cc: Florian Apolloner Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

from django.contrib import admin
from django.contrib.flatpages.models import FlatPage
from django.contrib.flatpages.admin import FlatPageAdmin as FPAdmin

def decorator(func):
    def wrap(self, request, *args, **kwargs):
        return func(self, request, *args, **kwargs)
    return  wrap

class FlatPageAdmin(FPAdmin):
    change_view = decorator(FPAdmin.change_view)

admin.site.unregister(FlatPage)
admin.site.register(FlatPage, FlatPageAdmin)

raises the following error:

Traceback:
File "/home/florian/sources/django-svn.git/django/core/handlers/base.py" in get_response
  101.                     response = callback(request, *callback_args, **callback_kwargs)
File "/home/florian/sources/django-svn.git/django/contrib/admin/options.py" in wrapper
  238.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/home/florian/sources/django-svn.git/django/utils/decorators.py" in __call__
  36.         return self.decorator(self.func)(*args, **kwargs)
File "/home/florian/sources/django-svn.git/django/utils/decorators.py" in _wrapped_view
  86.                     response = view_func(request, *args, **kwargs)
File "/home/florian/sources/django-svn.git/django/utils/decorators.py" in __call__
  36.         return self.decorator(self.func)(*args, **kwargs)
File "/home/florian/sources/django-svn.git/django/views/decorators/cache.py" in _wrapped_view_func
  70.         response = view_func(request, *args, **kwargs)
File "/home/florian/sources/django-svn.git/django/contrib/admin/sites.py" in inner
  190.             return view(request, *args, **kwargs)
File "/home/florian/.virtualenvs/djangoobjperms/testing/objperms/admin.py" in wrap
  7.         return func(self, request, *args, **kwargs)
File "/home/florian/sources/django-svn.git/django/utils/decorators.py" in _wrapped_view
  82.                     result = middleware.process_view(request, view_func, args, kwargs)
File "/home/florian/sources/django-svn.git/django/middleware/csrf.py" in process_view
  83.             request.META["CSRF_COOKIE"] = request.COOKIES[settings.CSRF_COOKIE_NAME]

Exception Type: AttributeError at /admin/flatpages/flatpage/2/
Exception Value: 'FlatPageAdmin' object has no attribute 'COOKIES'

I tried using auto_adapt_to_methods etc, no luck. The goal is to give the wrapping function access to the instance! From what I can see the problem is caused by decorator_from_middleware; but I can't see through it :/

Change History (8)

comment:1 by Charles Leifer, 14 years ago

looks like you're passing in self as the first parameter of your decorator. If you want the wrapping function to have access to the instance, move the decorator into the class proper.

in reply to:  1 comment:2 by anonymous, 14 years ago

Replying to Charles Leifer:

looks like you're passing in self as the first parameter of your decorator. If you want the wrapping function to have access to the instance, move the decorator into the class proper.

Comment rescinded -- this one's trickier than first glance.

comment:3 by Luke Plant, 14 years ago

If your decorator is assuming a method, rather than a function (which it does as written above), then it does not need 'auto_adapt_to_methods', which will actually thwart your attempt by hiding the 'self' parameter.

I haven't got to the bottom of this at all, but note:

  1. You are passing an unbound method (FPAdmin.change_view) into the decorator, and then using it like a function (in assigning it to change_view). That is a very strange thing to do.
  1. If you change your code to:
    class FlatPageAdmin(FPAdmin):
        @decorator
        def change_view(*args, **kwargs):
            return super(FlatPageAdmin, self).change_view(*args, **kwargs)
    

...then it works (in my tests anyway).

comment:4 by Luke Plant, 14 years ago

Resolution: invalid
Status: newclosed

OK, if you remove the 'decorator' call from your code, it has the same problem. If you strip 'csrf_protect' out, and replace it with some other decorator that uses auto_adapt_to_methods, the fundamental problem is still there, i.e.:

class FlatPageAdmin(FPAdmin):
    change_view = FPAdmin.change_view

It boils down to the fact that using an unbound method in the way you are doing breaks auto_adapt_to_methods. I can't see that it is possible to fix code like this. Use super() instead, which is the normal way.

comment:5 by Florian Apolloner, 14 years ago

Yes, I am aware that super works and what the problem is, but it's imo still a bug or at least should get documented as it's nothing you'd expect. Aside from that it's a backwards incompatible change since CSRF protection got merged!

comment:6 by Florian Apolloner, 14 years ago

Resolution: invalid
Status: closedreopened

comment:7 by Luke Plant, 14 years ago

Resolution: fixed
Status: reopenedclosed

(In [12399]) Fixed #12804 - regression with decorating admin views.

This is a BACKWARDS INCOMPATIBLE change, because it removes the flawed
'auto_adapt_to_methods' decorator, and replaces it with 'method_decorator'
which must be applied manually when necessary, as described in the 1.2
release notes.

For users of 1.1 and 1.0, this affects the decorators:

  • login_required
  • permission_required
  • user_passes_test

For those following trunk, this also affects:

  • csrf_protect
  • anything created with decorator_from_middleware

If a decorator does not depend on the signature of the function it is
supposed to decorate (for example if it only does post-processing of the
result), it will not be affected.

comment:8 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

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