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

Code improvements in utils.py #288

Merged
merged 2 commits into from
Apr 4, 2024
Merged

Conversation

Collins-Webdev
Copy link
Contributor

Here's the list of modifications made:

  1. Added set_cache and get_cache functions to simplify cache manipulation.
  2. Added render_quota_error and render_quota_error_response functions to make the code more modular and reusable.
  3. Added the redirect_to_last_visited function to handle redirection to the last visited URL more clearly.
  4. Reorganized imports to improve code readability.

Here's the list of modifications made:

1. Added `set_cache` and `get_cache` functions to simplify cache manipulation.
2. Added `render_quota_error` and `render_quota_error_response` functions to make the code more modular and reusable.
3. Added the `redirect_to_last_visited` function to handle redirection to the last visited URL more clearly.
4. Reorganized imports to improve code readability.
@TreyWW
Copy link
Owner

TreyWW commented Apr 4, 2024

Hey @Collins-Webdev,
Welcome to the project, thank you for your PR!

For the redirect_to_last_visited function, instead of accepting a last visited URL could we maybe figure that out in the function for less repeated code? The previous url is stored in request.session (check middleware file for exact usage). You could maybe accept an optional argument for a URL to redirect to if the user doesn't have a previous URL and if it's not found redirect to the argument or dashboard.

I hope that's okay, thanks again for the PR.

…ion. Instead of accepting a last visited URL as an argument, the function now automatically determines the previous URL from the user's session, in line with Django's session management. Additionally, I've added an optional argument fallback_url to handle cases where no previous URL is found in the session.

Here's the modified function:

def redirect_to_last_visited(request, fallback_url="dashboard"):
    """
    Redirects user to the last visited URL stored in session.
    If no previous URL is found, redirects to the fallback URL.
    :param request: HttpRequest object
    :param fallback_url: URL to redirect to if no previous URL found
    :return: HttpResponseRedirect object
    """
    try:
        last_visited_url = request.session.get("last_visited", fallback_url)
        return HttpResponseRedirect(last_visited_url)
    except KeyError:
        return HttpResponseRedirect(fallback_url)
@Collins-Webdev
Copy link
Contributor Author

Collins-Webdev commented Apr 4, 2024

Hey @Collins-Webdev, Welcome to the project, thank you for your PR!

For the redirect_to_last_visited function, instead of accepting a last visited URL could we maybe figure that out in the function for less repeated code? The previous url is stored in request.session (check middleware file for exact usage). You could maybe accept an optional argument for a URL to redirect to if the user doesn't have a previous URL and if it's not found redirect to the argument or dashboard.

I hope that's okay, thanks again for the PR.

Hello @TreyWW , thank you for the feedback, I appreciate the opportunity to contribute.

I've made the requested modifications to the redirect_to_last_visited function. Instead of accepting the last visited URL as an argument, the function now checks the request.session for the previous URL. If it's found, the user is redirected there; otherwise, they are redirected to the dashboard or the optional URL argument if provided.

Please review the changes and let me know if there's anything else I can assist with.

@TreyWW TreyWW requested review from TreyWW and removed request for TreyWW April 4, 2024 17:33
@TreyWW TreyWW merged commit f72d225 into TreyWW:main Apr 4, 2024
21 checks passed
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