diff --git a/.travis.yml b/.travis.yml index b77e5121e51..7d64ff84594 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,6 +27,7 @@ branches: - 1-1-stable - 1-2-stable - master + - 2079-refactor-controller-helpers rvm: - 1.8.7 - 1.9.3 diff --git a/core/app/controllers/spree/base_controller.rb b/core/app/controllers/spree/base_controller.rb index 3b8c3835262..820904f34c1 100644 --- a/core/app/controllers/spree/base_controller.rb +++ b/core/app/controllers/spree/base_controller.rb @@ -1,5 +1,8 @@ require 'cancan' class Spree::BaseController < ApplicationController - include Spree::Core::ControllerHelpers + include Spree::Core::ControllerHelpers::Auth + include Spree::Core::ControllerHelpers::RespondWith + include Spree::Core::ControllerHelpers::Common + end diff --git a/core/app/controllers/spree/checkout_controller.rb b/core/app/controllers/spree/checkout_controller.rb index 53bdf1ea7c7..701730ed35f 100644 --- a/core/app/controllers/spree/checkout_controller.rb +++ b/core/app/controllers/spree/checkout_controller.rb @@ -2,7 +2,7 @@ module Spree # Handles checkout logic. This is somewhat contrary to standard REST convention since there is not actually a # Checkout object. There's enough distinct logic specific to checkout which has nothing to do with updating an # order that this approach is waranted. - class CheckoutController < BaseController + class CheckoutController < Spree::StoreController ssl_required before_filter :load_order diff --git a/core/app/controllers/spree/content_controller.rb b/core/app/controllers/spree/content_controller.rb index fd5242da119..a652f8b4a40 100644 --- a/core/app/controllers/spree/content_controller.rb +++ b/core/app/controllers/spree/content_controller.rb @@ -1,5 +1,5 @@ module Spree - class ContentController < BaseController + class ContentController < Spree::StoreController # Don't serve local files or static assets before_filter { render_404 if params[:path] =~ /(\.|\\)/ } diff --git a/core/app/controllers/spree/home_controller.rb b/core/app/controllers/spree/home_controller.rb index 5c1dd586f73..0d0c463221f 100644 --- a/core/app/controllers/spree/home_controller.rb +++ b/core/app/controllers/spree/home_controller.rb @@ -1,5 +1,5 @@ module Spree - class HomeController < BaseController + class HomeController < Spree::StoreController helper 'spree/products' respond_to :html diff --git a/core/app/controllers/spree/locale_controller.rb b/core/app/controllers/spree/locale_controller.rb index e8ad5ed97a8..851f65b46e3 100644 --- a/core/app/controllers/spree/locale_controller.rb +++ b/core/app/controllers/spree/locale_controller.rb @@ -1,5 +1,5 @@ module Spree - class LocaleController < BaseController + class LocaleController < Spree::StoreController def set if request.referer && request.referer.starts_with?('http://' + request.host) session['user_return_to'] = request.referer diff --git a/core/app/controllers/spree/orders_controller.rb b/core/app/controllers/spree/orders_controller.rb index c5c331d8898..645c61177ec 100644 --- a/core/app/controllers/spree/orders_controller.rb +++ b/core/app/controllers/spree/orders_controller.rb @@ -1,5 +1,5 @@ module Spree - class OrdersController < BaseController + class OrdersController < Spree::StoreController rescue_from ActiveRecord::RecordNotFound, :with => :render_404 helper 'spree/products' diff --git a/core/app/controllers/spree/products_controller.rb b/core/app/controllers/spree/products_controller.rb index d7fefd4c462..fb43c48c13d 100644 --- a/core/app/controllers/spree/products_controller.rb +++ b/core/app/controllers/spree/products_controller.rb @@ -1,5 +1,5 @@ module Spree - class ProductsController < BaseController + class ProductsController < Spree::StoreController before_filter :load_product, :only => :show rescue_from ActiveRecord::RecordNotFound, :with => :render_404 helper 'spree/taxons' diff --git a/core/app/controllers/spree/states_controller.rb b/core/app/controllers/spree/states_controller.rb index 53b4f038038..b118c126306 100644 --- a/core/app/controllers/spree/states_controller.rb +++ b/core/app/controllers/spree/states_controller.rb @@ -1,5 +1,5 @@ module Spree - class StatesController < BaseController + class StatesController < Spree::StoreController ssl_allowed :index respond_to :js diff --git a/core/app/controllers/spree/store_controller.rb b/core/app/controllers/spree/store_controller.rb new file mode 100644 index 00000000000..dfbd9db15b5 --- /dev/null +++ b/core/app/controllers/spree/store_controller.rb @@ -0,0 +1,18 @@ +module Spree + class StoreController < Spree::BaseController + include Spree::Core::ControllerHelpers::Order + + # Convenience method for firing instrumentation events with the default payload hash + def fire_event(name, extra_payload = {}) + ActiveSupport::Notifications.instrument(name, default_notification_payload.merge(extra_payload)) + end + + # Creates the hash that is sent as the payload for all notifications. Specific notifications will + # add additional keys as appropriate. Override this method if you need additional data when + # responding to a notification + def default_notification_payload + {:user => try_spree_current_user, :order => current_order} + end + end +end + diff --git a/core/app/controllers/spree/taxons_controller.rb b/core/app/controllers/spree/taxons_controller.rb index 39d7055ab0a..55622d6c4a7 100644 --- a/core/app/controllers/spree/taxons_controller.rb +++ b/core/app/controllers/spree/taxons_controller.rb @@ -1,5 +1,5 @@ module Spree - class TaxonsController < BaseController + class TaxonsController < Spree::StoreController rescue_from ActiveRecord::RecordNotFound, :with => :render_404 helper 'spree/products' diff --git a/core/lib/spree/core/controller_helpers.rb b/core/lib/spree/core/controller_helpers.rb deleted file mode 100755 index 48254b7f47b..00000000000 --- a/core/lib/spree/core/controller_helpers.rb +++ /dev/null @@ -1,185 +0,0 @@ -# This module exists so that controllers that don't inherit from Spree::BaseController -# Can still inherit the important bits of functionality and use the Spree layout by default. -module Spree - module Core - module ControllerHelpers - def self.included(base) - base.class_eval do - include Spree::Core::CurrentOrder - include Spree::Core::RespondWith - include SslRequirement - - helper_method :title - helper_method :title= - helper_method :accurate_title - helper_method :current_order - helper_method :try_spree_current_user - - before_filter :set_user_language - before_filter :set_current_order - - rescue_from CanCan::AccessDenied do |exception| - return unauthorized - end - - layout :get_layout - end - end - # can be used in views as well as controllers. - # e.g. <% title = 'This is a custom title for this view' %> - attr_writer :title - - # proxy method to *possible* spree_current_user method - # Authentication extensions (such as spree_auth_devise) are meant to provide spree_current_user - def try_spree_current_user - respond_to?(:spree_current_user) ? spree_current_user : nil - end - - def title - title_string = @title.present? ? @title : accurate_title - if title_string.present? - if Spree::Config[:always_put_site_name_in_title] - [title_string, default_title].join(' - ') - else - title_string - end - else - default_title - end - end - - def associate_user - if try_spree_current_user && @order - if @order.user.blank? || @order.email.blank? - @order.associate_user!(try_spree_current_user) - end - end - - # This will trigger any "first order" promotions to be triggered - # Assuming of course that this session variable was set correctly in - # the authentication provider's registrations controller - if session[:spree_user_signup] - fire_event('spree.user.signup', :user => try_spree_current_user, :order => current_order(true)) - end - - session[:guest_token] = nil - session[:spree_user_signup] = nil - end - - protected - - def set_current_order - if user = try_spree_current_user - last_incomplete_order = user.last_incomplete_spree_order - if session[:order_id].nil? && last_incomplete_order - session[:order_id] = last_incomplete_order.id - elsif current_order && last_incomplete_order && current_order != last_incomplete_order - current_order.merge!(last_incomplete_order) - end - end - end - - # Needs to be overriden so that we use Spree's Ability rather than anyone else's. - def current_ability - @current_ability ||= Spree::Ability.new(try_spree_current_user) - end - - def store_location - # disallow return to login, logout, signup pages - authentication_routes = [:spree_signup_path, :spree_login_path, :spree_logout_path] - disallowed_urls = [] - authentication_routes.each do |route| - if respond_to?(route) - disallowed_urls << send(route) - end - end - - disallowed_urls.map!{ |url| url[/\/\w+$/] } - unless disallowed_urls.include?(request.fullpath) - session['user_return_to'] = request.fullpath.gsub('//', '/') - end - end - # Redirect as appropriate when an access request fails. The default action is to redirect to the login screen. - # Override this method in your controllers if you want to have special behavior in case the user is not authorized - # to access the requested action. For example, a popup window might simply close itself. - def unauthorized - respond_to do |format| - format.html do - if try_spree_current_user - flash.now[:error] = t(:authorization_failure) - render 'spree/shared/unauthorized', :layout => Spree::Config[:layout], :status => 401 - else - store_location - url = respond_to?(:spree_login_path) ? spree_login_path : root_path - redirect_to url - end - end - format.xml do - request_http_basic_authentication 'Web Password' - end - format.json do - render :text => "Not Authorized \n", :status => 401 - end - end - end - - - def default_title - Spree::Config[:site_name] - end - - # this is a hook for subclasses to provide title - def accurate_title - Spree::Config[:default_seo_title] - end - - def render_404(exception = nil) - respond_to do |type| - type.html { render :status => :not_found, :file => "#{::Rails.root}/public/404", :formats => [:html], :layout => nil} - type.all { render :status => :not_found, :nothing => true } - end - end - - # Convenience method for firing instrumentation events with the default payload hash - def fire_event(name, extra_payload = {}) - ActiveSupport::Notifications.instrument(name, default_notification_payload.merge(extra_payload)) - end - - # Creates the hash that is sent as the payload for all notifications. Specific notifications will - # add additional keys as appropriate. Override this method if you need additional data when - # responding to a notification - def default_notification_payload - {:user => try_spree_current_user, :order => current_order} - end - - private - - def redirect_back_or_default(default) - redirect_to(session["user_return_to"] || default) - session["user_return_to"] = nil - end - - def set_user_language - locale = session[:locale] - locale ||= Rails.application.config.i18n.default_locale - if locale.blank? || !I18n.available_locales.include?(locale.to_sym) - locale ||= I18n.default_locale - end - I18n.locale = locale.to_sym - end - - # Returns which layout to render. - # - # You can set the layout you want to render inside your Spree configuration with the +:layout+ option. - # - # Default layout is: +app/views/spree/layouts/spree_application+ - # - def get_layout - layout ||= Spree::Config[:layout] - end - - - end - end -end - diff --git a/core/lib/spree/core/controller_helpers/auth.rb b/core/lib/spree/core/controller_helpers/auth.rb new file mode 100644 index 00000000000..552e942e79d --- /dev/null +++ b/core/lib/spree/core/controller_helpers/auth.rb @@ -0,0 +1,71 @@ +module Spree + module Core + module ControllerHelpers + module Auth + def self.included(base) + base.class_eval do + include SslRequirement + + helper_method :try_spree_current_user + + rescue_from CanCan::AccessDenied do |exception| + return unauthorized + end + + # Needs to be overriden so that we use Spree's Ability rather than anyone else's. + def current_ability + @current_ability ||= Spree::Ability.new(try_spree_current_user) + end + + # Redirect as appropriate when an access request fails. The default action is to redirect to the login screen. + # Override this method in your controllers if you want to have special behavior in case the user is not authorized + # to access the requested action. For example, a popup window might simply close itself. + def unauthorized + format.html do + if try_spree_current_user + flash.now[:error] = t(:authorization_failure) + render 'spree/shared/unauthorized', :layout => Spree::Config[:layout], :status => 401 + else + store_location + url = respond_to?(:spree_login_path) ? spree_login_path : root_path + redirect_to url + end + end + format.xml do + request_http_basic_authentication 'Web Password' + end + format.json do + render :text => "Not Authorized \n", :status => 401 + end + end + + def store_location + # disallow return to login, logout, signup pages + authentication_routes = [:spree_signup_path, :spree_login_path, :spree_logout_path] + disallowed_urls = [] + authentication_routes.each do |route| + if respond_to?(route) + disallowed_urls << send(route) + end + end + + disallowed_urls.map!{ |url| url[/\/\w+$/] } + unless disallowed_urls.include?(request.fullpath) + session['user_return_to'] = request.fullpath.gsub('//', '/') + end + end + + # proxy method to *possible* spree_current_user method + # Authentication extensions (such as spree_auth_devise) are meant to provide spree_current_user + def try_spree_current_user + respond_to?(:spree_current_user) ? spree_current_user : nil + end + + def redirect_back_or_default(default) + redirect_to(session["user_return_to"] || default) + session["user_return_to"] = nil + end + end + end + end +end diff --git a/core/lib/spree/core/controller_helpers/common.rb b/core/lib/spree/core/controller_helpers/common.rb new file mode 100644 index 00000000000..70c447a3a42 --- /dev/null +++ b/core/lib/spree/core/controller_helpers/common.rb @@ -0,0 +1,72 @@ +module Spree + module Core + module ControllerHelpers + module Common + def self.included(base) + base.class_eval do + helper_method :title + helper_method :title= + helper_method :accurate_title + helper_method :current_order + + layout :get_layout + end + end + + protected + + # can be used in views as well as controllers. + # e.g. <% title = 'This is a custom title for this view' %> + attr_writer :title + + def title + title_string = @title.present? ? @title : accurate_title + if title_string.present? + if Spree::Config[:always_put_site_name_in_title] + [default_title, title_string].join(' - ') + else + title_string + end + else + default_title + end + end + + def default_title + Spree::Config[:site_name] + end + + # this is a hook for subclasses to provide title + def accurate_title + Spree::Config[:default_seo_title] + end + + def render_404(exception = nil) + respond_to do |type| + type.html { render :status => :not_found, :file => "#{::Rails.root}/public/404", :formats => [:html], :layout => nil} + type.all { render :status => :not_found, :nothing => true } + end + end + private + + def set_user_language + locale = session[:locale] + locale ||= Spree::Config[:default_locale] unless Spree::Config[:default_locale].blank? + locale ||= Rails.application.config.i18n.default_locale + locale ||= I18n.default_locale unless I18n.available_locales.include?(locale.to_sym) + I18n.locale = locale.to_sym + end + + # Returns which layout to render. + # + # You can set the layout you want to render inside your Spree configuration with the +:layout+ option. + # + # Default layout is: +app/views/spree/layouts/spree_application+ + # + def get_layout + layout ||= Spree::Config[:layout] + end + end + end + end +end diff --git a/core/lib/spree/core/controller_helpers/order.rb b/core/lib/spree/core/controller_helpers/order.rb new file mode 100644 index 00000000000..ca1ee8d6ca5 --- /dev/null +++ b/core/lib/spree/core/controller_helpers/order.rb @@ -0,0 +1,70 @@ +module Spree + module Core + module ControllerHelpers + module Order + def self.included(base) + base.class_eval do + helper_method :current_order + before_filter :set_current_order + end + end + + # This should be overridden by an auth-related extension which would then have the + # opportunity to associate the new order with the # current user before saving. + def before_save_new_order + end + + # This should be overridden by an auth-related extension which would then have the + # opporutnity to store tokens, etc. in the session # after saving. + def after_save_new_order + end + + # The current incomplete order from the session for use in cart and during checkout + def current_order(create_order_if_necessary = false) + return @current_order if @current_order + if session[:order_id] + current_order = Spree::Order.find_by_id(session[:order_id], :include => :adjustments) + @current_order = current_order unless current_order.try(:completed?) + end + if create_order_if_necessary and (@current_order.nil? or @current_order.completed?) + @current_order = Spree::Order.new + before_save_new_order + @current_order.save! + after_save_new_order + end + session[:order_id] = @current_order ? @current_order.id : nil + @current_order + end + + def associate_user + if try_spree_current_user && @order + if @order.user.blank? || @order.email.blank? + @order.associate_user!(try_spree_current_user) + end + end + + # This will trigger any "first order" promotions to be triggered + # Assuming of course that this session variable was set correctly in + # the authentication provider's registrations controller + if session[:spree_user_signup] + fire_event('spree.user.signup', :user => try_spree_current_user, :order => current_order(true)) + end + + session[:guest_token] = nil + session[:spree_user_signup] = nil + end + + def set_current_order + if user = try_spree_current_user + last_incomplete_order = user.last_incomplete_spree_order + if session[:order_id].nil? && last_incomplete_order + session[:order_id] = last_incomplete_order.id + elsif current_order && last_incomplete_order && current_order != last_incomplete_order + current_order.merge!(last_incomplete_order) + end + end + end + end + end + end +end diff --git a/core/lib/spree/core/respond_with.rb b/core/lib/spree/core/controller_helpers/respond_with.rb similarity index 100% rename from core/lib/spree/core/respond_with.rb rename to core/lib/spree/core/controller_helpers/respond_with.rb diff --git a/core/lib/spree/core/current_order.rb b/core/lib/spree/core/current_order.rb deleted file mode 100644 index 55b93436ac0..00000000000 --- a/core/lib/spree/core/current_order.rb +++ /dev/null @@ -1,38 +0,0 @@ -module Spree - module Core - module CurrentOrder - def self.included(base) - base.class_eval do - helper_method :current_order - end - end - - # This should be overridden by an auth-related extension which would then have the - # opportunity to associate the new order with the # current user before saving. - def before_save_new_order - end - - # This should be overridden by an auth-related extension which would then have the - # opporutnity to store tokens, etc. in the session # after saving. - def after_save_new_order - end - - # The current incomplete order from the session for use in cart and during checkout - def current_order(create_order_if_necessary = false) - return @current_order if @current_order - if session[:order_id] - current_order = Spree::Order.find_by_id(session[:order_id], :include => :adjustments) - @current_order = current_order unless current_order.try(:completed?) - end - if create_order_if_necessary and (@current_order.nil? or @current_order.completed?) - @current_order = Spree::Order.new - before_save_new_order - @current_order.save! - after_save_new_order - end - session[:order_id] = @current_order ? @current_order.id : nil - @current_order - end - end - end -end diff --git a/promo/lib/spree/promo/engine.rb b/promo/lib/spree/promo/engine.rb index 2ce1ac1a148..0353737cc93 100644 --- a/promo/lib/spree/promo/engine.rb +++ b/promo/lib/spree/promo/engine.rb @@ -8,7 +8,7 @@ def self.activate Dir.glob(File.join(File.dirname(__FILE__), '../../../app/**/*_decorator*.rb')) do |c| Rails.configuration.cache_classes ? require(c) : load(c) - Spree::BaseController.class_eval do + Spree::StoreController.class_eval do # Include list of visited paths in notification payload hash def default_notification_payload { :user => try_spree_current_user, :order => current_order, :visited_paths => session[:visited_paths] }