From a3119d560c7575e41fbaa05cc090738e6b9e83e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Busqu=C3=A9?= Date: Tue, 10 Jan 2023 13:12:50 +0100 Subject: [PATCH] Remove indirection to set current host for ActiveStorage We no longer support Rails v5.2, so there's no need to support the old way [1]. As we only need to include a Rails module, there's neither the need to go through a custom module. [1] - https://github.com/rails/rails/commit/e33c3cd8ccbecaca6c6af0438956431b02cb3fb2 --- .../controllers/spree/api/base_controller.rb | 2 +- core/app/controllers/spree/base_controller.rb | 2 +- core/lib/spree/core.rb | 1 - .../core/controller_helpers/current_host.rb | 6 +++++- core/lib/spree/rails_compatibility.rb | 17 ---------------- .../controller_helpers/current_host_spec.rb | 20 +++++++++++++++++++ 6 files changed, 27 insertions(+), 21 deletions(-) create mode 100644 core/spec/lib/spree/core/controller_helpers/current_host_spec.rb diff --git a/api/app/controllers/spree/api/base_controller.rb b/api/app/controllers/spree/api/base_controller.rb index e68b5db294f..ec48d666e52 100644 --- a/api/app/controllers/spree/api/base_controller.rb +++ b/api/app/controllers/spree/api/base_controller.rb @@ -10,7 +10,7 @@ class BaseController < ActionController::Base protect_from_forgery unless: -> { request.format.json? } include CanCan::ControllerAdditions - include Spree::Core::ControllerHelpers::CurrentHost + include ActiveStorage::SetCurrent include Spree::Core::ControllerHelpers::Store include Spree::Core::ControllerHelpers::Pricing include Spree::Core::ControllerHelpers::StrongParameters diff --git a/core/app/controllers/spree/base_controller.rb b/core/app/controllers/spree/base_controller.rb index fbdba199c08..31aea6bc3fc 100644 --- a/core/app/controllers/spree/base_controller.rb +++ b/core/app/controllers/spree/base_controller.rb @@ -4,7 +4,7 @@ require_dependency 'spree/core/controller_helpers/strong_parameters' class Spree::BaseController < ApplicationController - include Spree::Core::ControllerHelpers::CurrentHost + include ActiveStorage::SetCurrent include Spree::Core::ControllerHelpers::Auth include Spree::Core::ControllerHelpers::Common include Spree::Core::ControllerHelpers::PaymentParameters diff --git a/core/lib/spree/core.rb b/core/lib/spree/core.rb index 83484f1a419..f69671a4b9b 100644 --- a/core/lib/spree/core.rb +++ b/core/lib/spree/core.rb @@ -108,7 +108,6 @@ class GatewayError < RuntimeError; end require 'spree/core/product_duplicator' require 'spree/core/controller_helpers/auth' require 'spree/core/controller_helpers/common' -require 'spree/core/controller_helpers/current_host' require 'spree/core/controller_helpers/order' require 'spree/core/controller_helpers/payment_parameters' require 'spree/core/controller_helpers/pricing' diff --git a/core/lib/spree/core/controller_helpers/current_host.rb b/core/lib/spree/core/controller_helpers/current_host.rb index 4a322b9b600..136f264bae9 100644 --- a/core/lib/spree/core/controller_helpers/current_host.rb +++ b/core/lib/spree/core/controller_helpers/current_host.rb @@ -7,7 +7,11 @@ module CurrentHost extend ActiveSupport::Concern included do - Spree::RailsCompatibility.active_storage_set_current(self) + Spree::Deprecation.warn <<~MSG + 'Spree::Core::ControllerHelpers::CurrentHost' is deprecated. + Please, include 'ActiveStorage::SetCurrent' instead. + MSG + include ActiveStorage::SetCurrent end end end diff --git a/core/lib/spree/rails_compatibility.rb b/core/lib/spree/rails_compatibility.rb index 508dd773a9a..a63e4fe3add 100644 --- a/core/lib/spree/rails_compatibility.rb +++ b/core/lib/spree/rails_compatibility.rb @@ -72,23 +72,6 @@ def self.default_taxon_attachment_module end end - # Set current host for ActiveStorage in a controller - # - # Changed from `#host` to including a module in Rails 6 - # - # See https://github.com/rails/rails/commit/e33c3cd8ccbecaca6c6af0438956431b02cb3fb2 - # - # TODO: Remove when deprecating Rails 5.2 - def self.active_storage_set_current(controller) - if version_gte?('6') - controller.include ActiveStorage::SetCurrent - else - controller.before_action do - ActiveStorage::Current.host = request.base_url - end - end - end - # Set current host for ActiveStorage # # Changed from `#host` to `#url_options` on Rails 7 diff --git a/core/spec/lib/spree/core/controller_helpers/current_host_spec.rb b/core/spec/lib/spree/core/controller_helpers/current_host_spec.rb new file mode 100644 index 00000000000..58991f8593f --- /dev/null +++ b/core/spec/lib/spree/core/controller_helpers/current_host_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'spree/core/controller_helpers/current_host' + +RSpec.describe Spree::Core::ControllerHelpers::CurrentHost do + it 'is deprecated' do + expect(Spree::Deprecation).to receive(:warn).with(/'#{described_class.name}' is deprecated/) + + Class.new(ApplicationController).include(described_class) + end + + it 'includes ActiveStorage::SetCurrent module' do + Spree::Deprecation.silence do + mod = Class.new(ApplicationController).include(described_class) + + expect(mod.ancestors).to include(ActiveStorage::SetCurrent) + end + end +end