TrangPham / django-admin-confirm

AdminConfirmMixin is a mixin for ModelAdmin that adds confirmations to changes, additions and actions.
Other
128 stars 16 forks source link

Exception Value: cache_control didn't receive an HttpRequest. If you are decorating a classmethod, be sure to use @method_decorator. #35

Closed TrangPham closed 2 years ago

TrangPham commented 2 years ago

Describe the bug Project doesn't work with Django 4.0.4

To Reproduce Steps to reproduce the behavior: Using Django 4.0.4

  1. Follow the local dev steps
  2. Attempt to run the app after installed package

Additional context

Environment:

Request Method: GET
Request URL: http://127.0.0.1:8000/admin/market/shoppingmall/2/change/

Django Version: 4.0.4
Python Version: 3.9.9
Installed Applications:
['admin_confirm',
 'django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'storages',
 'market']
Installed Middleware:
['django.middleware.security.SecurityMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware']

Traceback (most recent call last):
  File "/Users/thu/.pyenv/versions/django-admin-confirm-3.9.9/lib/python3.9/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "/Users/thu/.pyenv/versions/django-admin-confirm-3.9.9/lib/python3.9/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/Users/thu/.pyenv/versions/django-admin-confirm-3.9.9/lib/python3.9/site-packages/django/contrib/admin/options.py", line 683, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/Users/thu/.pyenv/versions/django-admin-confirm-3.9.9/lib/python3.9/site-packages/django/utils/decorators.py", line 133, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/Users/thu/.pyenv/versions/django-admin-confirm-3.9.9/lib/python3.9/site-packages/django/views/decorators/cache.py", line 62, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/Users/thu/.pyenv/versions/django-admin-confirm-3.9.9/lib/python3.9/site-packages/django/contrib/admin/sites.py", line 242, in inner
    return view(request, *args, **kwargs)
  File "/Users/thu/.pyenv/versions/django-admin-confirm-3.9.9/lib/python3.9/site-packages/django/contrib/admin/options.py", line 1888, in change_view
    return self.changeform_view(request, object_id, form_url, extra_context)
  File "/Users/thu/.pyenv/versions/django-admin-confirm-3.9.9/lib/python3.9/site-packages/django/views/decorators/cache.py", line 35, in _cache_controlled
    raise TypeError(

Exception Type: TypeError at /admin/market/shoppingmall/2/change/
Exception Value: cache_control didn't receive an HttpRequest. If you are decorating a classmethod, be sure to use @method_decorator.
TrangPham commented 2 years ago

cache_control was introduced for: fix(MR-16): Use cache for FileField and ImageField (#17)

TrangPham commented 2 years ago

Tested on Django 3.2 locally and issue seems to not be happening

TrangPham commented 2 years ago

Issue persists on Django 4.0.4 when code was updated to use method_decorator:

# 
from django.utils.decorators import method_decorator
#
from django.views.decorators.cache import cache_control
from admin_confirm.constants import (
    CONFIRMATION_RECEIVED,
    CONFIRM_ADD,
    CONFIRM_CHANGE,
    SAVE,
    SAVE_ACTIONS,
    CACHE_KEYS,
    SAVE_AND_CONTINUE,
    SAVE_AS_NEW,
    CACHE_TIMEOUT,
)
from admin_confirm.file_cache import FileCache

#
@method_decorator([cache_control(private=True)], name="dispatch")
#
class AdminConfirmMixin:
    # Should we ask for confirmation for changes?
    confirm_change = None

    # Should we ask for confirmation for additions?
    confirm_add = None

    # If asking for confirmation, which fields should we confirm for?
    confirmation_fields = None

    # Custom templates (designed to be over-ridden in subclasses)
    change_confirmation_template = None
    action_confirmation_template = None

    _file_cache = FileCache()

    ...

    # @cache_control(private=True)
    def changeform_view(self, request, object_id=None, form_url="", extra_context=None):
        if request.method == "POST":
Rigo-Villalta commented 2 years ago

Hello @TrangPham , Some news respecting this bug?

TrangPham commented 2 years ago

There’s no update on this bug at the moment. Are you experiencing this bug and if so can you post your versions?

TrangPham commented 2 years ago

@Rigo-Villalta I took another look at this. The solution is included in PR #36.

For reference, this is what changed:

    @method_decorator(cache_control(private=True))
    def changeform_view(self, request, object_id=None, form_url="", extra_context=None):

method_decorator needed to be applied on the function for instance functions, instead of on the class (which is only used for class methods I think?) https://github.com/TrangPham/django-admin-confirm/pull/36/files#diff-fd78a9569df4114b07e16d4cf9412096e8e88adb42a9e199eb28cd95d1db04c9L105

guiklimek commented 1 year ago

Thank you for that, unfortunately, this is not up-to-date on official repo https://pypi.org/project/django-admin-confirm/ . Could be nice to post the v1 of your nice work!

TrangPham commented 1 year ago

@guiklimek Thanks for your comment! Look like I forgot to push it to pypi. I'll try to get the testing and push done this week.