Add an option 'use_redirect_back_or_to_by_rails' to avoid definition conflicts of redirect_back_or_to
#373
Conversation
atolix
force-pushed
the
rails-7-redirect-back-or-to-fix
branch
from
ce4db7e
to
14be051
Compare
| @@ -96,7 +96,16 @@ def current_user=(user) | |||
|
|
|||
| # used when a user tries to access a page while logged out, is asked to login, | |||
| # and we want to return him back to the page he originally wanted. | |||
| def redirect_back_or_to(url, flash_hash = {}) | |||
| def redirect_back_or_to(...) | |||
| if Config.use_redirect_back_or_to_by_rails | |||
There was a problem hiding this comment.
If you are using Rails 6.1, calling super will cause an exception. Therefore, I want to ensure that super is not called even if Config.use_redirect_back_or_to_by_rails returns true.
Can you update the tests to expect a NoMethodError exception when using Rails 6.1?
The tests are currently failing.
lib/sorcery/controller.rb
Outdated
| if Config.use_redirect_back_or_to_by_rails | ||
| super | ||
| else | ||
| warn('[WARNING] `redirect_back_or_to` overrides the method of the same name defined in Rails 7. If you want to avoid overriding, you can set `config.use_redirect_back_or_to_by_rails = true` and use `redirect_to_before_login_path`.') |
There was a problem hiding this comment.
Since it is desirable for this setting to default to true in the future, I would like to add that "in a future version, config.use_redirect_back_or_to_by_rails = true will become the default."
There was a problem hiding this comment.
I have changed the message in 8ebe075
…conflicts 'redirect_back_or_to' Fix: Sorcery#296 Rails 7 released a new method called `redirect_back_or_to` as a replacement for `redirect_back`. That may conflicts with the method by the same name defined by Sorcery. This commit adds an option to set whether to use `redirect_back_or_to` defined by Rails 7, and add the method `redirect_to_before_login_path` as an alternative to sorcery's `redirect_back_or_to. ref: rails/rails#40671
atolix
force-pushed
the
rails-7-redirect-back-or-to-fix
branch
from
42bab9a
to
f2106da
Compare
atolix
force-pushed
the
rails-7-redirect-back-or-to-fix
branch
from
f2106da
to
7a494fc
Compare
|
@joshbuker As far as I have reviewed, I think the implementation is fine, but I'm not sure if the comments and other texts are clear. Could you please review them? |
Fix: #296
Rails 7 released a new method called
redirect_back_or_toas a replacement forredirect_back. That may conflicts with the method by the same name defined by Sorcery.This commit adds an option to set whether to use
redirect_back_or_todefined by Rails 7, and add a methodredirect_to_before_login_pathas an alternative to sorcery's `redirect_back_or_to.ref: rails/rails#40671
Please ensure your pull request includes the following: