Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add redirect_middleware for handling htmx request redirects #406

Closed

Conversation

knowsuchagency
Copy link

@knowsuchagency knowsuchagency commented Dec 31, 2023

This commit adds a new function, redirect_middleware, to handle redirects for htmx requests for login required views.

This should address issues where htmx performs a request against a view decorated with @login_required.

The middleware function checks if the request is an htmx request and if the response status code is 302 (redirection). If so, it parses the redirection location. If the redirection location is the login URL, it changes the redirection URL to point to the login redirect URL and changes the response status code to 204 (No Content). The modified response is then returned.

The redirect_middleware function is added to the django_htmx/middleware.py file.

knowsuchagency and others added 10 commits December 30, 2023 18:40
This commit adds a new function, `redirect_middleware`, to handle redirects for htmx requests for login required views. The middleware function checks if the request is an htmx request and if the response status code is 302 (redirection). If so, it parses the redirection location. If the redirection location is the login URL, it changes the redirection URL to point to the login redirect URL and changes the response status code to 204 (No Content). The modified response is then returned.

The `redirect_middleware` function is added to the `django_htmx/middleware.py` file.
Copy link
Owner

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think this is a good way of handling things. It would be better to have an htmx-ready version of @login_required that returns an htmx redirect response.

But also I am entirely convinced that there should be a feature in django-htmx at all. What to do regarding login is somewhat project-dependent. A section in the “Tips” documentation page might be a better approach.


.. code-block:: python

def redirect_middleware(get_response):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy-pasting the source code into the docs isn't necessary. they would drift over time.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right, it does seem to be project-specific what should be done in cases like these. Thanks for taking the time to review!

def middleware(request):
response = get_response(request)
if request.headers.get("HX-Request") and response.status_code == 302:
location = urlparse(response["Location"])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi urlsplit is preferable https://www.youtube.com/watch?v=ABJvdsIANds

response = get_response(request)
if request.headers.get("HX-Request") and response.status_code == 302:
location = urlparse(response["Location"])
if location.path == settings.LOGIN_URL:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@login_required might use a different login_url, so this is fragile. Wrapping login_required would solve this

Comment on lines +127 to +129
redirect_url = (
f"{settings.LOGIN_URL}?next={settings.LOGIN_REDIRECT_URL}"
)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd wnat to preserve next.

@knowsuchagency knowsuchagency deleted the redirect-middleware branch January 1, 2024 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants