Merge pull request #228 from cpfergus1/connorferguson/sol-306-deprecate-redirect_back_or_default

Utilize Devise location helpers for redirecting

Author: waiting-for-dev

lib/controllers/backend/spree/admin/user_sessions_controller.rb

@@ -17,7 +17,7 @@ def create
       respond_to do |format|
         format.html {
           flash[:success] = I18n.t('spree.logged_in_succesfully')
-          redirect_back_or_default(after_sign_in_path_for(spree_current_user))
+          redirect_to stored_spree_user_location_or(after_sign_in_path_for(spree_current_user))
         }
         format.js {
           user = resource.record
@@ -47,9 +47,4 @@ def set_user_language_locale_key
   def accurate_title
     I18n.t('spree.login')
   end
-
-  def redirect_back_or_default(default)
-    redirect_to(session["spree_user_return_to"] || default)
-    session["spree_user_return_to"] = nil
-  end
 end

lib/controllers/frontend/spree/user_sessions_controller.rb

@@ -19,7 +19,7 @@ def create
       respond_to do |format|
         format.html do
           flash[:success] = I18n.t('spree.logged_in_succesfully')
-          redirect_back_or_default(after_sign_in_path_for(spree_current_user))
+          redirect_to stored_spree_user_location_or(after_sign_in_path_for(spree_current_user))
         end
         format.js { render success_json }
       end
@@ -49,11 +49,6 @@ def accurate_title
     I18n.t('spree.login')
   end
 
-  def redirect_back_or_default(default)
-    redirect_to(session["spree_user_return_to"] || default)
-    session["spree_user_return_to"] = nil
-  end
-
   def success_json
     {
       json: {

lib/controllers/frontend/spree/users_controller.rb

@@ -17,7 +17,7 @@ def create
         session[:guest_token] = nil
       end
 
-      redirect_back_or_default(root_url)
+      redirect_to stored_spree_user_location_or(root_url)
     else
       render :new
     end

lib/decorators/frontend/controllers/spree/checkout_controller_decorator.rb

@@ -45,7 +45,6 @@ def check_authorization
     def check_registration
       return unless registration_required?
 
-      store_location
       redirect_to spree.checkout_registration_path
     end
 

lib/spree/auth/engine.rb

@@ -59,7 +59,6 @@ def self.prepare_backend
               redirect_to spree.admin_unauthorized_path
             end
           else
-            store_location
 
             if Spree::Auth::Engine.redirect_back_on_unauthorized?
               redirect_back(fallback_location: spree.admin_login_path)
@@ -70,7 +69,6 @@ def self.prepare_backend
         end
       end
 
-
       def self.prepare_frontend
         Spree::BaseController.unauthorized_redirect = -> do
           if spree_current_user
@@ -82,7 +80,6 @@ def self.prepare_frontend
               redirect_to spree.unauthorized_path
             end
           else
-            store_location
 
             if Spree::Auth::Engine.redirect_back_on_unauthorized?
               redirect_back(fallback_location: spree.login_path)

lib/spree/authentication_helpers.rb

@@ -23,5 +23,30 @@ def spree_current_user
                to: :spree,
                prefix: :spree
     end
+
+    private
+
+    def authenticate_spree_user!
+      store_spree_user_location! if storable_spree_user_location?
+
+      super
+    end
+
+    # It's important that the location is NOT stored if:
+    # - The request method is not GET (non idempotent)
+    # - The request is handled by a Devise controller such as Devise::SessionsController as that could cause an
+    #    infinite redirect loop.
+    # - The request is an Ajax request as this can lead to very unexpected behaviour.
+    def storable_spree_user_location?
+      request.get? && is_navigational_format? && !devise_controller? && !request.xhr?
+    end
+
+    def store_spree_user_location!
+      store_location_for(:spree_current_user, request.fullpath)
+    end
+
+    def stored_spree_user_location_or(fallback_location)
+      stored_location_for(:spree_current_user) || fallback_location
+    end
   end
 end