morlandi / django-ajax-datatable

A Django app which provides the integration of a Django project with the jQuery Javascript library DataTables.net
MIT License
204 stars 64 forks source link

Fix for Django 4.0 removed functions #52

Closed fraimondo closed 2 years ago

fraimondo commented 2 years ago

This PR makes is compatible with Django 4.0.

morlandi commented 2 years ago

Thank you @fraimondo I will merge this soon ;)

fraimondo commented 2 years ago

Got a better and more "correct" solution. I'll push it soon.

fraimondo commented 2 years ago

Indeed ugettext_lazy has been an alias of gettext_lazy since Django 2.0. And since the readme states that Django 1.x is not supported anymore, no need to do this try stuff.

morlandi commented 2 years ago

Indeed ugettext_lazy has been an alias of gettext_lazy since Django 2.0. And since the readme states that Django 1.x is not supported anymore, no need to do this try stuff.

Totally agree 😉

morlandi commented 2 years ago

@fraimondo could you kindly share a link on why request.accepts('application/json') is a suitable replacement for the deprecated is_ajax() ?

fraimondo commented 2 years ago

That was actually a hard one:

https://github.com/django/django/pull/12091 https://code.djangoproject.com/ticket/30997

By digging in the PR and the associated ticket is_ajax relies on the jquery way of doing ajax calls. Instead of that, the idea is to use the kind of expected content by the request. If the request is expecting a json, then we consider it as an Ajax call.

morlandi commented 2 years ago

@fraimondo I tried to merge this PR but got the following exception:

  File "/Users/morlandi/Projects/github_public/django-ajax-datatable/ajax_datatable/views.py", line 371, in dispatch
    if request.accepts('application/json'):
AttributeError: 'WSGIRequest' object has no attribute 'accepts'

runnng the example project with Django v 3.0.8 and Python 3.8.11

By inspecting the requests namespaces, accepts() seems to be missing, indeed:

ipdb> dir(request) ['COOKIES', 'FILES', 'GET', 'META', 'POST', 'REQUEST', 'class', 'delattr', 'dict', 'dir', 'doc', 'eq', 'format', 'ge', 'getattribute', 'gt', 'hash', 'init', 'init_subclass', 'iter', 'le', 'lt', 'module', 'ne', 'new', 'reduce', 'reduce_ex', 'repr', 'setattr', 'sizeof', 'str', 'subclasshook', 'weakref', '_body', '_current_scheme_host', '_encoding', '_files', '_get_full_path', '_get_post', '_get_raw_host', '_get_scheme', '_initialize_handlers', '_load_post_and_files', '_mark_post_parse_error', '_messages', '_post', '_read_started', '_set_content_type_params', '_set_post', '_stream', '_upload_handlers', 'body', 'build_absolute_uri', 'close', 'content_params', 'content_type', 'encoding', 'environ', 'get_full_path', 'get_full_path_info', 'get_host', 'get_port', 'get_raw_uri', 'get_signed_cookie', 'headers', 'is_ajax', 'is_secure', 'method', 'parse_file_upload', 'path', 'path_info', 'read', 'readline', 'readlines', 'resolver_match', 'scheme', 'session', 'upload_handlers', 'user']

Any suggestion ?

fraimondo commented 2 years ago

Indeed I just figured out it's specific for Django 4.0. I can think of creating is_ajax in utils.py and doing the if there.

PetrDlouhy commented 2 years ago

@fraimondo According to https://stackoverflow.com/questions/63629935/django-3-1-and-is-ajax I fixed this with following function in django-experiments project:

def is_ajax(request):
    return request.META.get('HTTP_X_REQUESTED_WITH') == 'XMLHttpRequest'
fraimondo commented 2 years ago

@fraimondo According to https://stackoverflow.com/questions/63629935/django-3-1-and-is-ajax I fixed this with following function in django-experiments project:

def is_ajax(request):
    return request.META.get('HTTP_X_REQUESTED_WITH') == 'XMLHttpRequest'

The main issue with this approach is that it is exactly why they deprecated and later removed it. The X-Requested-With heather is nonstandard and set by JQuery.

morlandi commented 2 years ago

then why not using either request.accepts('application/json') or request.is_ajax() depending on the version of Django as detected at run-time ?

fraimondo commented 2 years ago

That was my plan, but I did not have time to take care of that yet. I’ll do it soon.

On 16 Dec 2021, at 19:17, morlandi @.***> wrote:

then why not using either request.accepts('application/json') or request.is_ajax() depending on the version of Django as detected at run-time ?

— Reply to this email directly, view it on GitHub https://github.com/morlandi/django-ajax-datatable/pull/52#issuecomment-996063276, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCJDA5E7GT6IDBCBKXHEB3URIULRANCNFSM5FLDHL3A. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.

morlandi commented 2 years ago

never mind, take your time 😉

morlandi commented 2 years ago

I finally opted for this:

        try:
            is_ajax_request = request.accepts("application/json")
        except AttributeError as e:
            # Django < 4.0
            is_ajax_request = request.is_ajax()

        if is_ajax_request:
            ...