Allow to easily extend Auth#store_location behavior
#3369
Conversation
spaghetticode
force-pushed
the
spaghetticode/extendable-store-location
branch
2 times, most recently
from
6a99c20 to
8c6dd76
Compare
core/spec/models/spree/user_return_to_storer/rules/authentication_rule_spec.rb
Outdated
Show resolved
Hide resolved
spaghetticode
force-pushed
the
spaghetticode/extendable-store-location
branch
3 times, most recently
from
26385dc to
25a8fad
Compare
spaghetticode
force-pushed
the
spaghetticode/extendable-store-location
branch
from
25a8fad to
a84179e
Compare
core/spec/models/spree/user_return_to_storer/rules/authentication_rule_spec.rb
Outdated
Show resolved
Hide resolved
spaghetticode
force-pushed
the
spaghetticode/extendable-store-location
branch
2 times, most recently
from
edbcdc8 to
35fc1a3
Compare
| # @example This method can be used also to add more rules | ||
| # Spree::UserReturnToStorer.rules << 'CustomRule' | ||
| # | ||
| # @examle it can be used also removing unwanted rules |
| # This service object is responsible for storing the current path into | ||
| # into `session[:spree_user_return_to]` for redirects after successful | ||
| # user/admin authentication. | ||
| class UserReturnToStorer |
There was a problem hiding this comment.
This name is a bit hard to understand maybe, at least for me at the first look. I immediately thought about Returns and I'm wondering if we can use another terminology, which conflicts less with other elements of the Solidus domain, like UserLastLocationStorer maybe? I think it could be better even because this proposed name is not connected with how we'll use it, while ReturnTo is. Not a strong opinion here, what's yours?
There was a problem hiding this comment.
My strong opinion here is that I don't like that name either 😄
I think UserLastLocationStorer is good and better than mine, but it's a bit long. What about UserLastUrlStorer or UserLocationStorer?
As soon as we agree on a name I'll revisit the PR with the other suggestions as well... so much namespace and filenames to change 😄
There was a problem hiding this comment.
I think UserLastUrlStorer is fine. Let's see if we have some other opinion here :)
core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt
Outdated
Show resolved
Hide resolved
| class Configuration | ||
| def rules | ||
| @rules ||= ::Spree::Core::ClassConstantizer::Set.new.tap do |set| | ||
| set << 'Spree::UserReturnToStorer::Rules::AuthenticationRule' |
There was a problem hiding this comment.
I think it could be better setting the default rule into core/lib/spree/app_configuration.rb where we are creating the preference to better split the implementation and how we are actually using it in Solidus and also for consistency with the other preferences that seem to have defaults set there.
spaghetticode
force-pushed
the
spaghetticode/extendable-store-location
branch
3 times, most recently
from
32d8e01 to
71d13fb
Compare
Spree::UserLastUrlStorer can be used to selectively store the current path in `session[:spree_user_return_to]`. This object allows easy extension/change of behavior by using a list of rules. When at least one rule is matched, the request path is not stored in session. The class ships with the default rule AutenticationRule, that was extacted from Spree::Core::ControllerHelpers::Auth#store_location
By using Spree::UserLastUrlStorer one can change ControllerHelpers::Auth#store_location behavior without resorting to monkey patching.
spaghetticode
force-pushed
the
spaghetticode/extendable-store-location
branch
from
71d13fb to
55d1772
Compare
Description
Often makes sense change the behavior of the method
Spree::Core::ControllerHelpers::Auth#store_location, which is responsible for settingsession[:spree_user_return_to]` value to the current URL. When necessity arises, theonly way ATM is to monkey patch Solidus version of this method.
This PR allows adding custom behavior by less obtrusive means, ie. by using custom rules.
Checklist: