Deprecate method #redirect_back_or_default#4533
Merged
kennyadsl merged 2 commits intoSep 15, 2022
Merged
Conversation
This was referenced Aug 30, 2022
6 tasks
cpfergus1
force-pushed
the
nebulab/deprecate_redirect_to_or_default
branch
from
159765d to
3af4a82
Compare
github-actions
Bot
added
the
changelog:solidus_core
cpfergus1
changed the title
cpfergus1
force-pushed
the
nebulab/deprecate_redirect_to_or_default
branch
from
3af4a82 to
76c1adf
Compare
waiting-for-dev
suggested changes
Sep 1, 2022
|
|
||
| def redirect_back_or_default(default) | ||
| redirect_to(session["spree_user_return_to"] || default) | ||
| def stored_location_or(fallback_location) |
Contributor
There was a problem hiding this comment.
We should probably implement it in solidus_auth_devise
cpfergus1
force-pushed
the
nebulab/deprecate_redirect_to_or_default
branch
from
76c1adf to
d07e8a3
Compare
waiting-for-dev
approved these changes
Sep 5, 2022
waiting-for-dev
left a comment
waiting-for-dev
left a comment
Contributor
There was a problem hiding this comment.
Thanks, @cpfergus1! Putting it on hold until the PRs on the extensions are ready.
waiting-for-dev
added
the
release:major
jarednorman
approved these changes
Sep 5, 2022
cpfergus1
changed the title
cpfergus1
force-pushed
the
nebulab/deprecate_redirect_to_or_default
branch
from
dbc4f8c to
4f139c9
Compare
This was referenced Sep 13, 2022
kennyadsl
reviewed
Sep 14, 2022
kennyadsl
left a comment
kennyadsl
left a comment
Member
There was a problem hiding this comment.
Thanks Connor, I just left a question.
| # or its subclasses. The controller will be passed to each rule for matching. | ||
| def initialize(controller) | ||
| @controller = controller | ||
| Spree::Deprecation.warn("This class will be removed without replacement on the realease of Solidus 4.0") |
Member
There was a problem hiding this comment.
But are we still using this in core here? If it's not called anymore, that method should be deprecated as well.
Contributor
Author
There was a problem hiding this comment.
Good point!
I also removed that method from solidus_starter_frontend and solidus_auth_devise!
Devise has a nearly identical method for storing a user's location and returning a path to navigate to. Deprecating to simplify the code base.
cpfergus1
force-pushed
the
nebulab/deprecate_redirect_to_or_default
branch
2 times, most recently
from
856f5f8 to
57fb533
Compare
cpfergus1
force-pushed
the
nebulab/deprecate_redirect_to_or_default
branch
from
57fb533 to
16bc67a
Compare
We can safely eliminate the class when #redirect_back_or_default is removed. This commit also deprecates #store_location which is the only method that utilizes this class.
cpfergus1
force-pushed
the
nebulab/deprecate_redirect_to_or_default
branch
from
16bc67a to
a6de90e
Compare
kennyadsl
approved these changes
Sep 15, 2022
kennyadsl
left a comment
kennyadsl
left a comment
Member
There was a problem hiding this comment.
Thanks Connor, excellent work!
waiting-for-dev
added a commit
to nebulab/solidus
that referenced
this pull request
Jan 16, 2023
We deprecated `#redirect_back_or_default` method in solidusio#4533 [1]. However, the original plan was halted because of auth problems in solidus_auth_devise. See solidusio/solidus_auth_devise/232 for details. [1] - solidusio#4533 [2] - solidusio/solidus_auth_devise#232
waiting-for-dev
added a commit
to nebulab/solidus
that referenced
this pull request
Jan 16, 2023
We deprecated `#redirect_back_or_default` method in solidusio#4533 [1]. However, the original plan was halted because of auth problems in solidus_auth_devise. See solidusio/solidus_auth_devise/232 [2] for details. [1] - solidusio#4533 [2] - solidusio/solidus_auth_devise#232
3 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
Rails introduced redirect_back in rails 5+ and redirect_back_or_to in rails 7+. The naming of Solidus method #redirect_back_or_default lead to some confusion between the two methods and it appeared that it could be replaced by the included Rails methods to eliminate redundancy and simplify the code. After further investigation, the two functions create different results and cannot be used interchangeably. Devise, however, already contains a similar function to #redirect_back_or_default, so the functionality would be best moved to solidus_auth_devise.
This will affect the following gems
solidus_frontend - solidusio/solidus_frontend#9
solidus_starter_frontend - solidusio/solidus_starter_frontend#241
solidus_auth_devise - solidusio/solidus_auth_devise#228
solidus_social - solidusio-contrib/solidus_social#109
solidus_active_shipping - archived, No plans to update
solidus_paypal_marketplace - solidusio-contrib/solidus_paypal_marketplace#103
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed (
cross them outif they are not):I have attached screenshots to demo visual changes.I have opened a PR to update the guides.I have updated the readme to account for my changes.