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

Use new unauthorized redirect handler class endpoints #256

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Dec 21, 2024

Summary

This PR drops support for the old configuration option Spree::Config.redirect_back_on_unauthorized and extracts the redirect handling into classes, which can then be fed into the new extension points from solidusio/solidus#6051.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

`Spree::Config#redirect_back_on_unauthorized` has been removed in
Solidus 3.0, and we're on 4.4 now. This can all go.

If users have followed the deprecation warnings, then they should be
fine with using `redirect_back`.

Note that we're not using redirect_back in the admin, as logging out
redirects to the admin root controller, which in turn redirects back,
leading to an infinite redirect. Infinite redirects are no good.
We have to lambdas, so we need two classes.
This makes the `prepare_{frontend,backend}` methods which instantiate
the base controller and the admin base controller, requiring all of
ActionController::Base, unnecessary to call for newer Solidus versions.

Depends on solidusio/solidus#6051
@mamhoff mamhoff force-pushed the use-new-unauthorized-redirect-handler-class-endpoint branch from 942e1c1 to 79f443c Compare December 21, 2024 22:48
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.

1 participant