From 24d8de6102f8e2c0808ef963d8a02eb0f4303444 Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Fri, 2 Oct 2020 12:10:30 +0200 Subject: [PATCH 1/2] Whitespace Replace a non-breaking-space with a regular one. --- .../solidus/install/templates/config/initializers/spree.rb.tt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt b/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt index 542d57b533f..07bdb3da584 100644 --- a/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt +++ b/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt @@ -13,7 +13,7 @@ Spree.config do |config| # Use combined first and last name attribute in HTML views and API responses config.use_combined_first_and_last_name_in_address = true - # Use legacy Spree::Order state machine + # Use legacy Spree::Order state machine config.use_legacy_order_state_machine = false # Use the legacy address' state validation logic From 7ade8cabb5dc452183011967446402a60dd57d38 Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Wed, 5 Feb 2020 16:17:24 +0100 Subject: [PATCH 2/2] Move the reimbursement category logic to Spree::Reimbursement Add Spree::Reimbursement.store_credit_category_proc as a simple way to redefine how the reimbursement category should be fetched. Add Spree::Reimbursement#store_credit_category as the most natural way to fetch a given category for a reimbursement. This fixes reimbursement_category_name ending up in a translation missing error. We need to lazily load the translation because, when the file is loaded, Rails will still have to load translations from the main application. Co-Authored-By: Filippo Liverani --- core/app/models/spree/reimbursement.rb | 17 ++++++++++ .../reimbursement_helpers.rb | 2 +- .../app/models/spree/store_credit_category.rb | 33 +++++++++++++++++-- core/db/default/spree/store_credit.rb | 4 +-- core/lib/spree/app_configuration.rb | 6 ++++ core/lib/spree/testing_support/dummy_app.rb | 1 + .../store_credit_category_factory.rb | 8 +++++ core/spec/models/spree/reimbursement_spec.rb | 26 +++++++++++++++ .../reimbursement_type/store_credit_spec.rb | 2 +- .../spree/store_credit_category_spec.rb | 22 +++++++++++++ 10 files changed, 114 insertions(+), 7 deletions(-) diff --git a/core/app/models/spree/reimbursement.rb b/core/app/models/spree/reimbursement.rb index 1a4472616bd..f1291166128 100644 --- a/core/app/models/spree/reimbursement.rb +++ b/core/app/models/spree/reimbursement.rb @@ -163,6 +163,23 @@ def return_all(created_by: nil) perform!(created_by: created_by) end + # The returned category is used as the category + # for Spree::Reimbursement::Credit.default_creditable_class. + # + # @return [Spree::StoreCreditCategory] + def store_credit_category + if Spree::Config.use_legacy_store_credit_reimbursement_category_name + Spree::Deprecation.warn("Using the legacy reimbursement_category_name is deprecated. "\ + "Set Spree::Config.use_legacy_store_credit_reimbursement_category_name to false to use "\ + "the new version instead.", caller) + + name = Spree::StoreCreditCategory.reimbursement_category_name + return Spree::StoreCreditCategory.find_by(name: name) || Spree::StoreCreditCategory.first + end + + Spree::StoreCreditCategory.find_by(name: Spree::StoreCreditCategory::REIMBURSEMENT) + end + private def calculate_total diff --git a/core/app/models/spree/reimbursement_type/reimbursement_helpers.rb b/core/app/models/spree/reimbursement_type/reimbursement_helpers.rb index abfc08efdc8..849f75dac16 100644 --- a/core/app/models/spree/reimbursement_type/reimbursement_helpers.rb +++ b/core/app/models/spree/reimbursement_type/reimbursement_helpers.rb @@ -61,7 +61,7 @@ def create_creditable(reimbursement, unpaid_amount, created_by:) Spree::Reimbursement::Credit.default_creditable_class.new( user: reimbursement.order.user, amount: unpaid_amount, - category: Spree::StoreCreditCategory.reimbursement_category(reimbursement), + category: reimbursement.store_credit_category, created_by: created_by, memo: "Refund for uncreditable payments on order #{reimbursement.order.number}", currency: reimbursement.order.currency diff --git a/core/app/models/spree/store_credit_category.rb b/core/app/models/spree/store_credit_category.rb index cc0aa8e1773..067c0276279 100644 --- a/core/app/models/spree/store_credit_category.rb +++ b/core/app/models/spree/store_credit_category.rb @@ -1,18 +1,45 @@ # frozen_string_literal: true class Spree::StoreCreditCategory < Spree::Base + GIFT_CARD = 'Gift Card' + REIMBURSEMENT = 'Reimbursement' + class_attribute :non_expiring_credit_types self.non_expiring_credit_types = [Spree::StoreCreditType::NON_EXPIRING] + # @deprecated class_attribute :reimbursement_category_name self.reimbursement_category_name = I18n.t('spree.store_credit_category.default') - def self.reimbursement_category(_reimbursement) - Spree::StoreCreditCategory.find_by(name: reimbursement_category_name) || - Spree::StoreCreditCategory.first + # @deprecated + def self.reimbursement_category(reimbursement) + reimbursement.store_credit_category end def non_expiring? self.class.non_expiring_credit_types.include? name end + + public_instance_methods.grep(/^reimbursement_category_name/).each do |method| + deprecate( + method => 'Use Spree::Reimbursement#store_credit_category.name instead', + deprecator: Spree::Deprecation + ) + end + + class << self + public_instance_methods.grep(/^reimbursement_category_name/).each do |method| + deprecate( + method => 'Use Spree::Reimbursement.store_credit_category.name instead', + deprecator: Spree::Deprecation + ) + end + + public_instance_methods.grep(/^reimbursement_category$/).each do |method| + deprecate( + method => 'Use Spree::Reimbursement.store_credit_category instead', + deprecator: Spree::Deprecation + ) + end + end end diff --git a/core/db/default/spree/store_credit.rb b/core/db/default/spree/store_credit.rb index bd3028103ac..a3492db48f8 100644 --- a/core/db/default/spree/store_credit.rb +++ b/core/db/default/spree/store_credit.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -Spree::StoreCreditCategory.find_or_create_by!(name: I18n.t('spree.store_credit_category.default')) Spree::PaymentMethod.create_with( name: "Store Credit", @@ -18,6 +17,7 @@ Spree::ReimbursementType.create_with(name: "Store Credit").find_or_create_by!(type: 'Spree::ReimbursementType::StoreCredit') Spree::ReimbursementType.create_with(name: "Original").find_or_create_by!(type: 'Spree::ReimbursementType::OriginalPayment') -Spree::StoreCreditCategory.find_or_create_by!(name: 'Gift Card') +Spree::StoreCreditCategory.find_or_create_by!(name: Spree::StoreCreditCategory::GIFT_CARD) +Spree::StoreCreditCategory.find_or_create_by!(name: Spree::StoreCreditCategory::REIMBURSEMENT) Spree::StoreCreditReason.find_or_create_by!(name: 'Credit Given In Error') diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index 2bae9747a84..b6b47746883 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -317,6 +317,12 @@ class AppConfiguration < Preferences::Configuration # (default: +false+) preference :use_legacy_order_state_machine, :boolean, default: true + # The legacy_store_credit_category_name allows to control whether the legacy + # way of fetching the category should be used. + # + # @param [Boolean] enable/disable the legacy way of fetching the store category name + preference :use_legacy_store_credit_reimbursement_category_name, :boolean, default: true + # Other configurations # Allows restricting what currencies will be available. diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index 48e0b911b56..3dff94fd34f 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -123,6 +123,7 @@ class Application < ::Rails::Application config.use_legacy_order_state_machine = false config.use_custom_cancancan_actions = false config.consider_actionless_promotion_active = false + config.use_legacy_store_credit_reimbursement_category_name = false if ENV['ENABLE_ACTIVE_STORAGE'] config.image_attachment_module = 'Spree::Image::ActiveStorageAttachment' diff --git a/core/lib/spree/testing_support/factories/store_credit_category_factory.rb b/core/lib/spree/testing_support/factories/store_credit_category_factory.rb index 4f45203d080..7079a6eca21 100644 --- a/core/lib/spree/testing_support/factories/store_credit_category_factory.rb +++ b/core/lib/spree/testing_support/factories/store_credit_category_factory.rb @@ -3,5 +3,13 @@ FactoryBot.define do factory :store_credit_category, class: 'Spree::StoreCreditCategory' do name { "Exchange" } + + trait :reimbursement do + name { Spree::StoreCreditCategory::REIMBURSEMENT } + end + + trait :gift_card do + name { Spree::StoreCreditCategory::GIFT_CARD } + end end end diff --git a/core/spec/models/spree/reimbursement_spec.rb b/core/spec/models/spree/reimbursement_spec.rb index 36cadc4aa9a..ac15677cfbd 100644 --- a/core/spec/models/spree/reimbursement_spec.rb +++ b/core/spec/models/spree/reimbursement_spec.rb @@ -275,4 +275,30 @@ expect { subject }.to change { reimbursement.refunds.count }.by(1) end end + + describe '#store_credit_category' do + let(:reimbursement) { create(:reimbursement) } + + before do + create(:store_credit_category, name: 'foo') + create(:store_credit_category, :reimbursement) + end + + context 'when using the legacy version' do + before do + stub_spree_preferences(use_legacy_store_credit_reimbursement_category_name: true) + end + + it 'issues a deprecation warning and returns the first category created' do + expect(Spree::StoreCreditCategory).to receive(:reimbursement_category_name) + expect(Spree::Deprecation).to receive(:warn) + expect(reimbursement.store_credit_category.name).to eq('foo') + end + end + + it 'fetches the the default reimbursement store category' do + expect(Spree::Config.use_legacy_store_credit_reimbursement_category_name).to eq(false) + expect(reimbursement.store_credit_category.name).to eq('Reimbursement') + end + end end diff --git a/core/spec/models/spree/reimbursement_type/store_credit_spec.rb b/core/spec/models/spree/reimbursement_type/store_credit_spec.rb index 74b229d0a20..00e14e4eef1 100644 --- a/core/spec/models/spree/reimbursement_type/store_credit_spec.rb +++ b/core/spec/models/spree/reimbursement_type/store_credit_spec.rb @@ -13,7 +13,7 @@ module Spree let!(:primary_credit_type) { create(:primary_credit_type) } let(:created_by_user) { create(:user, email: 'user@email.com') } - let!(:default_reimbursement_category) { create(:store_credit_category) } + let!(:default_reimbursement_category) { create(:store_credit_category, :reimbursement) } subject { Spree::ReimbursementType::StoreCredit.reimburse(reimbursement, [return_item, return_item2], simulate, created_by: created_by_user) } diff --git a/core/spec/models/spree/store_credit_category_spec.rb b/core/spec/models/spree/store_credit_category_spec.rb index bf6361edbe0..ad6c3dfe709 100644 --- a/core/spec/models/spree/store_credit_category_spec.rb +++ b/core/spec/models/spree/store_credit_category_spec.rb @@ -16,4 +16,26 @@ it { expect(store_credit_category).not_to be_non_expiring } end end + + describe '.reimbursement_category' do + it 'raises a dreprecation warning' do + allow(Spree::Deprecation).to receive(:warn) + + described_class.reimbursement_category(Spree::Reimbursement.new) + + expect(Spree::Deprecation).to have_received(:warn) + .with(/reimbursement_category /, any_args) + end + end + + describe '.reimbursement_category_name' do + it 'raises a dreprecation warning' do + allow(Spree::Deprecation).to receive(:warn) + + described_class.reimbursement_category_name + + expect(Spree::Deprecation).to have_received(:warn) + .with(/reimbursement_category_name /, any_args) + end + end end