Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate Spree::Deprecation in favor of Spree.deprecator #5289

Merged
merged 4 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ Lint/SuppressedException:

Lint/MissingSuper:
Exclude:
- 'core/lib/spree/deprecation.rb' # this is a known class that doesn't require super
- 'core/lib/spree/deprecated_instance_variable_proxy.rb' # this is a known class that doesn't require super
- 'core/lib/spree/preferences/configuration.rb' # this class has no superclass defining `self.inherited`

Rails/FindEach:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def load_stock_management_data
view_context,
:@stock_locations,
:stock_item_stock_locations,
Spree::Deprecation,
Spree.deprecator,
"Please, do not use @stock_item_stock_locations anymore in the views, use @stock_locations",
)
@variant_display_attributes = self.class.variant_display_attributes
Expand Down
4 changes: 2 additions & 2 deletions backend/app/helpers/spree/admin/navigation_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ def tab(*args, &block)
css_classes = Array(options[:css_class])

if options.key?(:route)
Spree::Deprecation.warn "Passing a route to #tab is deprecated. Please pass a url instead."
Spree.deprecator.warn "Passing a route to #tab is deprecated. Please pass a url instead."
options[:url] ||= spree.send("#{options[:route]}_path")
end

if args.any?
Spree::Deprecation.warn "Passing resources to #tab is deprecated. Please use the `label:` and `match_path:` options instead."
Spree.deprecator.warn "Passing resources to #tab is deprecated. Please use the `label:` and `match_path:` options instead."
options[:label] ||= args.first
options[:url] ||= spree.send("admin_#{args.first}_path")
options[:selected] = args.include?(controller.controller_name.to_sym)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% Spree::Deprecation.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
<% Spree.deprecator.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>

<ul class="admin-subnav" data-hook="admin_product_sub_tabs">
<% if can? :admin, Spree::Product %>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% Spree::Deprecation.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
<% Spree.deprecator.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>

<ul class="admin-subnav" data-hook="admin_promotion_sub_tabs">
<% if can? :admin, Spree::Promotion %>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% Spree::Deprecation.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
<% Spree.deprecator.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>

<ul class="admin-subnav" data-hook="admin_settings_sub_tabs">
<% if can?(:admin, Spree::Store) %>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

Spree::Deprecation.warn(
Spree.deprecator.warn(
"Spree::BackendConfiguration::*_TABS is deprecated. Please use Spree::BackendConfiguration::MenuItem(match_path:) instead."
)

Expand Down
12 changes: 6 additions & 6 deletions backend/lib/spree/backend_configuration/menu_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ class MenuItem
def sections # rubocop:disable Style/TrivialAccessors
@sections
end
deprecate sections: :label, deprecator: Spree::Deprecation
deprecate sections: :label, deprecator: Spree.deprecator

attr_accessor :position # rubocop:disable Layout/EmptyLinesAroundAttributeAccessor
deprecate position: nil, deprecator: Spree::Deprecation
deprecate "position=": nil, deprecator: Spree::Deprecation
deprecate position: nil, deprecator: Spree.deprecator
deprecate "position=": nil, deprecator: Spree.deprecator

# @param icon [String] The icon to draw for this menu item
# @param condition [Proc] A proc which returns true if this menu item
Expand Down Expand Up @@ -41,16 +41,16 @@ def initialize(
if args.length == 2
sections, icon = args
label ||= sections.first.to_s
Spree::Deprecation.warn "Passing sections to #{self.class.name} is deprecated. Please pass a label instead."
Spree::Deprecation.warn "Passing icon to #{self.class.name} is deprecated. Please use the keyword argument instead."
Spree.deprecator.warn "Passing sections to #{self.class.name} is deprecated. Please pass a label instead."
Spree.deprecator.warn "Passing icon to #{self.class.name} is deprecated. Please use the keyword argument instead."
elsif args.any?
raise ArgumentError, "wrong number of arguments (given #{args.length}, expected 0..2)"
end

if partial.present? && children.blank?
# We only show the deprecation if there are no children, because if there are children,
# then the menu item is already future-proofed.
Spree::Deprecation.warn "Passing a partial to #{self.class.name} is deprecated. Please use the children keyword argument instead."
Spree.deprecator.warn "Passing a partial to #{self.class.name} is deprecated. Please use the children keyword argument instead."
end

@condition = condition || -> { true }
Expand Down
2 changes: 1 addition & 1 deletion backend/spec/helpers/admin/navigation_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
without_partial_double_verification do
allow(helper).to receive(:admin_orders_path).and_return("/admin/orders")
end
expect(Spree::Deprecation).to receive(:warn)
expect(Spree.deprecator).to receive(:warn)
.with("Passing a route to #tab is deprecated. Please pass a url instead.")
expect(helper.tab(label: :orders, route: :admin_orders)).to include('href="/admin/orders"')
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
RSpec.describe Spree::BackendConfiguration::MenuItem do
describe '#children' do
it 'is the replacement for the deprecated #partial method' do
expect(Spree::Deprecation).to receive(:warn).with(a_string_matching(/partial/))
expect(Spree.deprecator).to receive(:warn).with(a_string_matching(/partial/))

described_class.new(partial: 'foo')
end
Expand Down Expand Up @@ -67,8 +67,8 @@
describe "deprecated behavior" do
describe "passing `sections` and `icon` sequentially" do
it "warns about the deprecation" do
expect(Spree::Deprecation).to receive(:warn).with(a_string_matching(/sections/))
expect(Spree::Deprecation).to receive(:warn).with(a_string_matching(/icon/))
expect(Spree.deprecator).to receive(:warn).with(a_string_matching(/sections/))
expect(Spree.deprecator).to receive(:warn).with(a_string_matching(/icon/))

described_class.new([:foo, :bar], 'icon')
end
Expand Down
2 changes: 1 addition & 1 deletion backend/spec/lib/spree/backend_configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def product_path(product)
describe "deprecated behavior" do
describe "loading *_TABS constants" do
it "warns about the deprecation" do
expect(Spree::Deprecation).to receive(:warn).with(a_string_matching(/Spree::BackendConfiguration::\*_TABS is deprecated\./))
expect(Spree.deprecator).to receive(:warn).with(a_string_matching(/Spree::BackendConfiguration::\*_TABS is deprecated\./))

described_class::ORDER_TABS
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/adjustment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Adjustment < Spree::Base
scope :is_included, -> { where(included: true) }
scope :additional, -> { where(included: false) }

singleton_class.deprecate :return_authorization, deprecator: Spree::Deprecation
singleton_class.deprecate :return_authorization, deprecator: Spree.deprecator

extend DisplayMoney
money_methods :amount
Expand Down
4 changes: 2 additions & 2 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ def recalculator
@recalculator ||= Spree::Config.order_recalculator_class.new(self)
end
alias_method :updater, :recalculator
deprecate updater: :recalculator, deprecator: Spree::Deprecation
deprecate updater: :recalculator, deprecator: Spree.deprecator

def assign_billing_to_shipping_address
self.ship_address = bill_address if bill_address
Expand Down Expand Up @@ -513,7 +513,7 @@ def check_shipments_and_restart_checkout
end

alias_method :ensure_updated_shipments, :check_shipments_and_restart_checkout
deprecate ensure_updated_shipments: :check_shipments_and_restart_checkout, deprecator: Spree::Deprecation
deprecate ensure_updated_shipments: :check_shipments_and_restart_checkout, deprecator: Spree.deprecator

def restart_checkout_flow
return if state == 'cart'
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/order_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def recalculate
end
end
alias_method :update, :recalculate
deprecate update: :recalculate, deprecator: Spree::Deprecation
deprecate update: :recalculate, deprecator: Spree.deprecator

# Updates the +shipment_state+ attribute according to the following logic:
#
Expand Down
2 changes: 1 addition & 1 deletion core/lib/generators/spree/dummy/templates/rails/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@

# Raise on deprecation warnings
if ENV['SOLIDUS_RAISE_DEPRECATIONS'].present?
Spree::Deprecation.behavior = :raise
Spree.deprecator.behavior = :raise
end
end
10 changes: 8 additions & 2 deletions core/lib/spree/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
require "active_storage/engine"
require "sprockets/railtie"

require 'active_support/deprecation'
require 'spree/deprecated_instance_variable_proxy'
require 'acts_as_list'
require 'awesome_nested_set'
require 'cancan'
Expand All @@ -19,13 +21,17 @@
require 'ransack'
require 'state_machines-activerecord'

require 'spree/deprecation'

# This is required because ActiveModel::Validations#invalid? conflicts with the
# invalid state of a Payment. In the future this should be removed.
StateMachines::Machine.ignore_method_conflicts = true

module Spree
def self.deprecator
@deprecator ||= ActiveSupport::Deprecation.new('5.0', 'Solidus')
end

autoload :Deprecation, 'spree/deprecation'

mattr_accessor :user_class, default: 'Spree::LegacyUser'

def self.user_class
Expand Down
6 changes: 6 additions & 0 deletions core/lib/spree/core/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ class Engine < ::Rails::Engine
ActionMailer::Base.preview_path = app.config.action_mailer.preview_path
end

initializer "spree.deprecator" do |app|
if app.respond_to?(:deprecators)
app.deprecators[:spree] = Spree.deprecator
end
end

config.after_initialize do
Spree::Config.check_load_defaults_called('Spree::Config')
Spree::Config.static_model_preferences.validate!
Expand Down
57 changes: 57 additions & 0 deletions core/lib/spree/deprecated_instance_variable_proxy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# frozen_string_literal: true

require 'active_support/deprecation'

module Spree
# This DeprecatedInstanceVariableProxy transforms instance variable to
# deprecated instance variable.
#
# It differs from ActiveSupport::DeprecatedInstanceVariableProxy since
# it allows to define a custom message.
#
# class Example
# def initialize(deprecator)
# @request = Spree::DeprecatedInstanceVariableProxy.new(self, :request, :@request, deprecator, "Please, do not use this thing.")
# @_request = :a_request
# end
#
# def request
# @_request
# end
#
# def old_request
# @request
# end
# end
#
# When someone execute any method on @request variable this will trigger
# +warn+ method on +deprecator_instance+ and will fetch <tt>@_request</tt>
# variable via +request+ method and execute the same method on non-proxy
# instance variable.
#
# Default deprecator is <tt>Spree.deprecator</tt>.
class DeprecatedInstanceVariableProxy < ActiveSupport::Deprecation::DeprecationProxy
def initialize(instance, method_or_var, var = "@#{method}", deprecator = Spree.deprecator, message = nil)
@instance = instance
@method_or_var = method_or_var
@var = var
@deprecator = deprecator
@message = message
end

private

def target
return @instance.instance_variable_get(@method_or_var) if @instance.instance_variable_defined?(@method_or_var)

@instance.__send__(@method_or_var)
end

def warn(callstack, called, args)
message = @message || "#{@var} is deprecated! Call #{@method_or_var}.#{called} instead of #{@var}.#{called}."
message = [message, "Args: #{args.inspect}"].join(" ") unless args.empty?

@deprecator.warn(message, callstack)
end
end
end
56 changes: 3 additions & 53 deletions core/lib/spree/deprecation.rb
Original file line number Diff line number Diff line change
@@ -1,59 +1,9 @@
# frozen_string_literal: true

require 'active_support/deprecation'
require 'spree/core'

module Spree
Deprecation = ActiveSupport::Deprecation.new('5.0', 'Solidus')
Deprecation = Spree.deprecator

# This DeprecatedInstanceVariableProxy transforms instance variable to
# deprecated instance variable.
#
# It differs from ActiveSupport::DeprecatedInstanceVariableProxy since
# it allows to define a custom message.
#
# class Example
# def initialize(deprecator)
# @request = Spree::DeprecatedInstanceVariableProxy.new(self, :request, :@request, deprecator, "Please, do not use this thing.")
# @_request = :a_request
# end
#
# def request
# @_request
# end
#
# def old_request
# @request
# end
# end
#
# When someone execute any method on @request variable this will trigger
# +warn+ method on +deprecator_instance+ and will fetch <tt>@_request</tt>
# variable via +request+ method and execute the same method on non-proxy
# instance variable.
#
# Default deprecator is <tt>Spree::Deprecation</tt>.
class DeprecatedInstanceVariableProxy < ActiveSupport::Deprecation::DeprecationProxy
def initialize(instance, method_or_var, var = "@#{method}", deprecator = Spree::Deprecation, message = nil)
@instance = instance
@method_or_var = method_or_var
@var = var
@deprecator = deprecator
@message = message
end

private

def target
return @instance.instance_variable_get(@method_or_var) if @instance.instance_variable_defined?(@method_or_var)

@instance.__send__(@method_or_var)
end

def warn(callstack, called, args)
message = @message || "#{@var} is deprecated! Call #{@method_or_var}.#{called} instead of #{@var}.#{called}."
message = [message, "Args: #{args.inspect}"].join(" ") unless args.empty?

@deprecator.warn(message, callstack)
end
end
Spree.deprecator.warn "Spree::Deprecation is deprecated. Please use Spree.deprecator instead.", caller(2)
end
2 changes: 1 addition & 1 deletion core/lib/spree/preferences/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def check_load_defaults_called(instance_constant_name = nil)
return if load_defaults_called || !Spree::Core.has_install_generator_been_run?

target_name = instance_constant_name || "#{self.class.name}.new"
Spree::Deprecation.warn <<~MSG
Spree.deprecator.warn <<~MSG
It's recommended that you explicitly load the default configuration for
your current Solidus version. You can do it by adding the following call
to your Solidus initializer within the #{target_name} block:
Expand Down
1 change: 0 additions & 1 deletion core/lib/spree/preferences/preferable_class_methods.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# frozen_string_literal: true

require 'spree/deprecation'
require 'spree/encryptor'

module Spree::Preferences
Expand Down
2 changes: 1 addition & 1 deletion core/lib/spree/testing_support/dummy_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,5 @@ class Application < ::Rails::Application

# Raise on deprecation warnings
if ENV['SOLIDUS_RAISE_DEPRECATIONS'].present?
Spree::Deprecation.behavior = :raise
Spree.deprecator.behavior = :raise
end
2 changes: 1 addition & 1 deletion core/lib/spree/testing_support/factory_bot.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def self.check_version
MSG
end
end
deprecate :check_version, deprecator: Spree::Deprecation
deprecate :check_version, deprecator: Spree.deprecator

def self.add_definitions!
::FactoryBot.definition_file_paths.unshift(*definition_file_paths).uniq!
Expand Down
2 changes: 1 addition & 1 deletion core/lib/spree/testing_support/silence_deprecations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

RSpec.configure do |config|
config.around(:each, silence_deprecations: true) do |example|
Spree::Deprecation.silence do
Spree.deprecator.silence do
example.run
end
end
Expand Down
4 changes: 2 additions & 2 deletions core/spec/lib/spree/migration_helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
context "when the column exists" do
context "and the index does" do
it "passes compatible arguments to index_exists?" do
expect { subject }.to_not raise_error(ArgumentError)
expect { subject }.to raise_error(NotImplementedError) # not ArgumentError
end
end

Expand All @@ -27,7 +27,7 @@
end

it "passes compatible arguments to add_index" do
expect { subject }.to_not raise_error(ArgumentError)
expect { subject }.to raise_error(TypeError) # not ArgumentError
end
end
end
Expand Down
Loading
Loading