Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#12804 closed (fixed)

decorating admin views doesn't work

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

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 :/

Attachments (0)

Change History (8)

comment:1 follow-up: Changed 4 years ago by Charles Leifer

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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:2 in reply to: ↑ 1 Changed 4 years ago by anonymous

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 Changed 4 years ago by lukeplant

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 Changed 4 years ago by lukeplant

  • Resolution set to invalid
  • Status changed from new to closed

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 Changed 4 years ago by apollo13

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 Changed 4 years ago by apollo13

  • Resolution invalid deleted
  • Status changed from closed to reopened

comment:7 Changed 4 years ago by lukeplant

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

(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 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.