fix routing and location storage #230
Closed
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
cpfergus1
force-pushed
the
elia+connor/fix-routing-and-location-storage
branch
from
03953ee to
bd712b5
Compare
cpfergus1
changed the title
cpfergus1
force-pushed
the
elia+connor/fix-routing-and-location-storage
branch
2 times, most recently
from
2156469 to
22aea03
Compare
We have some duplication affecting all stores using this extension and this is the first step toward removing it. E.g. the sign_in path was available at both /login and /spree_user/sign_in as `login_path` or `new_spree_user_session_path`. Co-Authored-By: Elia Schito <elia@schito.me>
Co-Authored-By: Elia Schito <elia@schito.me>
Previously we would fall back on authorization to determine if a user should be able to access a certain endpoint. We should be authenticating the user first prior to checking if the specific user is authorized to access a certain endpoint Co-Authored-By: Elia Schito <elia@schito.me>
When checking out, if a user is not registered, they will be redirected to the registration path. This path does not need to be authenticated and therefore will not have the user's last location stored. We need to store the user's location if redirected. Co-Authored-By: Elia Schito <elia@schito.me>
Devise utilizes spree_user scope when executing after_sign_in_path. This is an important distinction when storing the user location as the user would not be redirected as expected if we use spree_current_user. Co-Authored-By: Elia Schito <elia@schito.me>
cpfergus1
force-pushed
the
elia+connor/fix-routing-and-location-storage
branch
from
22aea03 to
0b80068
Compare
Co-Authored-By: Elia Schito <elia@schito.me>
cpfergus1
force-pushed
the
elia+connor/fix-routing-and-location-storage
branch
from
0b80068 to
2f9b1f6
Compare
|
Closing in favor of #231 |
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.
Description
Fixes the new store location feature introduced in #228
Motivation and Context
We replaced the method #redirect_back_or_default and the class user_last_url_storer because Devise already provides functionality similar to redirect_back_or_default. After emerging PR#228 We noticed that the redirects were not working as intended and investigated the cause. We found a couple of issues that we missed and needed to be rectified, specifically:
We need to Authenticate the user and not rely on authorization
Utilize
spree_userinstead ofspree_current_usersince that is the scope being utilizedUse implemented custom paths instead of devise helpers for certain routes
How Has This Been Tested?
The current test suite covers the changes made in the PR - quadrupled checked 👍
Types of changes
New feature (non-breaking change which adds functionality)Breaking change (fix or feature that would cause existing functionality to not work as expected)Checklist: