Skip to content

Commit

Permalink
Remove indirection to set current host for ActiveStorage
Browse files Browse the repository at this point in the history
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] - rails/rails@e33c3cd
  • Loading branch information
waiting-for-dev committed Jan 13, 2023
1 parent 28d72e9 commit a3119d5
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 21 deletions.
2 changes: 1 addition & 1 deletion api/app/controllers/spree/api/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/app/controllers/spree/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion core/lib/spree/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
6 changes: 5 additions & 1 deletion core/lib/spree/core/controller_helpers/current_host.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 0 additions & 17 deletions core/lib/spree/rails_compatibility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions core/spec/lib/spree/core/controller_helpers/current_host_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit a3119d5

Please sign in to comment.