Skip to content

Commit

Permalink
Move the reimbursement category logic to Spree::Reimbursement
Browse files Browse the repository at this point in the history
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 <dev@mailvore.com>
  • Loading branch information
elia and filippoliverani committed Oct 23, 2020
1 parent 24d8de6 commit 7ade8ca
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 7 deletions.
17 changes: 17 additions & 0 deletions core/app/models/spree/reimbursement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 30 additions & 3 deletions core/app/models/spree/store_credit_category.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions core/db/default/spree/store_credit.rb
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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')
6 changes: 6 additions & 0 deletions core/lib/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions core/lib/spree/testing_support/dummy_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
26 changes: 26 additions & 0 deletions core/spec/models/spree/reimbursement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand Down
22 changes: 22 additions & 0 deletions core/spec/models/spree/store_credit_category_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 7ade8ca

Please sign in to comment.