Redirect back if not authorized #192
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aldesantis
requested changes
May 20, 2020
kennyadsl
force-pushed
the
kennyadsl/redirect-back-if-not-authorized
branch
from
0e2de4a to
6556de3
Compare
aldesantis
approved these changes
May 20, 2020
kennyadsl
force-pushed
the
kennyadsl/redirect-back-if-not-authorized
branch
from
6556de3 to
e12636e
Compare
|
@kennyadsl is this okay to merge? |
4 tasks
|
Not yet, with @jarednorman we found a way to avoid the major bump after this PR, see solidusio/solidus#3118 (comment) |
kennyadsl
force-pushed
the
kennyadsl/redirect-back-if-not-authorized
branch
4 times, most recently
from
4e5a2dc to
3a639e0
Compare
|
@spaghetticode @aldesantis can you review again here, please? |
spaghetticode
approved these changes
Oct 6, 2020
There was a problem hiding this comment.
@kennyadsl thanks for the detailed explanation in the commit 👍
|
I noticed that the deprecation warning is emitted also on old versions that do not have the preference yet. Going to push a fix for that. |
kennyadsl
force-pushed
the
kennyadsl/redirect-back-if-not-authorized
branch
from
3a639e0 to
10df4ee
Compare
Only when the `redirect_back_on_unauthorized` preference exists and is set to true. This preference has been introduced in core with solidusio/solidus#3118 and we can rely on that preference to drive the behavior change here as well. The extra if Spree::Config.respond_to?(:redirect_back_on_unauthorized) check might seem useless but it's needed to avoid printing this deprecation warning on Solidus versions that still do not have the preference. If the Solidus verion used does not have the preference yet, the old behavior will be preserved.
kennyadsl
force-pushed
the
kennyadsl/redirect-back-if-not-authorized
branch
from
10df4ee to
ff57ff5
Compare
aldesantis
approved these changes
Oct 7, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a continuation of solidusio/solidus#3118 and tries to redirect back when there's an authentication error.
After this is merged I think we can safely merge the one in core as well.I updated this PR in order to use the preference introduced by the above core PR. Now the new behavior will be only supported on newer versions of Solidus, if that preference will be explicitly changed.