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

Remove default address dependency part 2 #2802

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def short_ship
flash[:error] = t('spree.no_inventory_selected')
redirect_to admin_order_cancellations_path(@order)
else
@order.cancellations.short_ship(inventory_units, whodunnit: whodunnit)
@order.cancellations.short_ship(inventory_units, created_by: created_by)

flash[:success] = t('spree.inventory_canceled')
redirect_to edit_admin_order_url(@order)
Expand All @@ -29,7 +29,7 @@ def short_ship

private

def whodunnit
def created_by
try_spree_current_user.try(:email)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class ReimbursementsController < ResourceController
rescue_from Spree::Core::GatewayError, with: :spree_core_gateway_error

def perform
@reimbursement.perform!
@reimbursement.perform!(created_by: try_spree_current_user)
redirect_to location_after_save
end

Expand Down Expand Up @@ -57,7 +57,7 @@ def load_stock_locations
end

def load_simulated_refunds
@reimbursement_objects = @reimbursement.simulate
@reimbursement_objects = @reimbursement.simulate(created_by: try_spree_current_user)
end

def spree_core_gateway_error(error)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false)
end

let(:user) { stub_model(Spree::LegacyUser, has_spree_role?: true, id: 1) }

before do
allow_any_instance_of(described_class).to receive(:try_spree_current_user).
and_return(user)
end

describe '#edit' do
let(:reimbursement) { create(:reimbursement) }
let(:order) { reimbursement.order }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@

before do
Spree::RefundReason.create!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false)
allow_any_instance_of(Spree::Admin::ReimbursementsController).to receive(:try_spree_current_user).
and_return(user)
end

let!(:order) { create(:shipped_order) }
let(:user) { create(:admin_user) }

# Regression test for https://github.com/spree/spree/issues/6229
it "refunds and has outstanding_balance of zero", js: true do
Expand Down
41 changes: 31 additions & 10 deletions core/app/models/spree/order_cancellations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,16 @@ def initialize(order)
# @api public
#
# @param [Array<InventoryUnit>] inventory_units the inventory units to be short shipped
# @param [String] whodunnit the system or person that is short shipping the inventory units
# @param (deprecated) [String] whodunnit the system or person that is short shipping the inventory unit
# @param [String] created_by the system or person that is short shipping the inventory unit
#
# @return [Array<UnitCancel>] the units that have been canceled due to short shipping
def short_ship(inventory_units, whodunnit: nil)
def short_ship(inventory_units, whodunnit: nil, created_by: nil)
if whodunnit
created_by ||= whodunnit
Spree::Deprecation.warn("Calling #short_ship on #{self} with whodunnit is deprecated, use created_by instead")
end

if inventory_units.map(&:order_id).uniq != [@order.id]
raise ArgumentError, "Not all inventory units belong to this order"
end
Expand All @@ -36,7 +42,7 @@ def short_ship(inventory_units, whodunnit: nil)
Spree::OrderMutex.with_lock!(@order) do
Spree::InventoryUnit.transaction do
inventory_units.each do |iu|
unit_cancels << short_ship_unit(iu, whodunnit: whodunnit)
unit_cancels << short_ship_unit(iu, created_by: created_by)
end

update_shipped_shipments(inventory_units)
Expand All @@ -59,17 +65,23 @@ def short_ship(inventory_units, whodunnit: nil)
#
# @param [InventoryUnit] inventory_unit the inventory unit to be canceled
# @param [String] reason the reason that you are canceling the inventory unit
# @param [String] whodunnit the system or person that is canceling the inventory unit
# @param (deprecated) [String] whodunnit the system or person that is canceling the inventory unit
# @param [String] created_by the system or person that is canceling the inventory unit
#
# @return [UnitCancel] the unit that has been canceled
def cancel_unit(inventory_unit, reason: Spree::UnitCancel::DEFAULT_REASON, whodunnit: nil)
def cancel_unit(inventory_unit, reason: Spree::UnitCancel::DEFAULT_REASON, whodunnit: nil, created_by: nil)
if whodunnit
created_by ||= whodunnit
Spree::Deprecation.warn("Calling #cancel_unit on #{self} with whodunnit is deprecated, use created_by instead")
end

unit_cancel = nil

Spree::OrderMutex.with_lock!(@order) do
unit_cancel = Spree::UnitCancel.create!(
inventory_unit: inventory_unit,
reason: reason,
created_by: whodunnit
created_by: created_by
)

inventory_unit.cancel!
Expand All @@ -82,26 +94,35 @@ def cancel_unit(inventory_unit, reason: Spree::UnitCancel::DEFAULT_REASON, whodu
#
# @api public
# @param [Array<InventoryUnit>] inventory_units the inventory units to be reimbursed
# @param [Spree.user_class] created_by the user that is performing this action
# @return [Reimbursement] the reimbursement for inventory being canceled
def reimburse_units(inventory_units)
def reimburse_units(inventory_units, created_by: nil)
unless created_by
Spree::Deprecation.warn("Calling #reimburse_units on #{self} without created_by is deprecated")
end
reimbursement = nil

Spree::OrderMutex.with_lock!(@order) do
return_items = inventory_units.map(&:current_or_new_return_item)
reimbursement = Spree::Reimbursement.new(order: @order, return_items: return_items)
reimbursement.return_all
reimbursement.return_all(created_by: created_by)
end

reimbursement
end

private

def short_ship_unit(inventory_unit, whodunnit: nil)
def short_ship_unit(inventory_unit, whodunnit: nil, created_by: nil)
if whodunnit
created_by ||= whodunnit
Spree::Deprecation.warn("Calling #short_ship_unit on #{self} with whodunnit is deprecated, use created_by instead")
end

unit_cancel = Spree::UnitCancel.create!(
inventory_unit: inventory_unit,
reason: Spree::UnitCancel::SHORT_SHIP,
created_by: whodunnit
created_by: created_by
)
unit_cancel.adjust!
inventory_unit.cancel!
Expand Down
22 changes: 16 additions & 6 deletions core/app/models/spree/reimbursement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,15 @@ def unpaid_amount
total - paid_amount
end

def perform!
def perform!(created_by: nil)
unless created_by
Spree::Deprecation.warn("Calling #perform on #{self} without created_by is deprecated")
end
reimbursement_tax_calculator.call(self)
reload
update!(total: calculated_total)

reimbursement_performer.perform(self)
reimbursement_performer.perform(self, created_by: created_by)

if unpaid_amount_within_tolerance?
reimbursed!
Expand All @@ -116,12 +119,15 @@ def perform!
end
end

def simulate
def simulate(created_by: nil)
unless created_by
Spree::Deprecation.warn("Calling #simulate on #{self} without created_by is deprecated")
end
reimbursement_simulator_tax_calculator.call(self)
reload
update!(total: calculated_total)

reimbursement_performer.simulate(self)
reimbursement_performer.simulate(self, created_by: created_by)
end

def return_items_requiring_exchange
Expand All @@ -139,11 +145,15 @@ def all_exchanges?
# Accepts all return items, saves the reimbursement, and performs the reimbursement
#
# @api public
# @param [Spree.user_class] created_by the user that is performing this action
# @return [void]
def return_all
def return_all(created_by: nil)
unless created_by
Spree::Deprecation.warn("Calling #return_all on #{self} without created_by is deprecated")
end
return_items.each(&:accept!)
save!
perform!
perform!(created_by: created_by)
end

private
Expand Down
18 changes: 12 additions & 6 deletions core/app/models/spree/reimbursement_performer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,28 @@ class << self
# - #description
# - #display_amount
# so they can be displayed in the Admin UI appropriately.
def simulate(reimbursement)
execute(reimbursement, true)
def simulate(reimbursement, created_by: nil)
unless created_by
Spree::Deprecation.warn("Calling #simulate on #{self} without created_by is deprecated")
end
execute(reimbursement, true, created_by: created_by)
end

# Actually perform the reimbursement
def perform(reimbursement)
execute(reimbursement, false)
def perform(reimbursement, created_by: nil)
unless created_by
Spree::Deprecation.warn("Calling #perform on #{self} without created_by is deprecated")
end
execute(reimbursement, false, created_by: created_by)
end

private

def execute(reimbursement, simulate)
def execute(reimbursement, simulate, created_by:)
reimbursement_type_hash = calculate_reimbursement_types(reimbursement)

reimbursement_type_hash.flat_map do |reimbursement_type, return_items|
reimbursement_type.reimburse(reimbursement, return_items, simulate)
reimbursement_type.reimburse(reimbursement, return_items, simulate, created_by: created_by)
end
end

Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/reimbursement_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class ReimbursementType < Spree::Base
# This method will reimburse the return items based on however it child implements it
# By default it takes a reimbursement, the return items it needs to reimburse, and if
# it is a simulation or a real reimbursement. This should return an array
def self.reimburse(_reimbursement, _return_items, _simulate)
def self.reimburse(_reimbursement, _return_items, _simulate, *_optional_args)
raise "Implement me"
end
end
Expand Down
7 changes: 5 additions & 2 deletions core/app/models/spree/reimbursement_type/credit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ class ReimbursementType::Credit < Spree::ReimbursementType
extend Spree::ReimbursementType::ReimbursementHelpers

class << self
def reimburse(reimbursement, return_items, simulate)
def reimburse(reimbursement, return_items, simulate, created_by: nil)
unless created_by
Spree::Deprecation.warn("Calling #reimburse on #{self} without created_by is deprecated")
end
unpaid_amount = return_items.sum(&:total).round(2, :down)
reimbursement_list, _unpaid_amount = create_credits(reimbursement, unpaid_amount, simulate)
reimbursement_list, _unpaid_amount = create_credits(reimbursement, unpaid_amount, simulate, created_by: created_by)
reimbursement_list
end
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/reimbursement_type/exchange.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

class Spree::ReimbursementType::Exchange < Spree::ReimbursementType
def self.reimburse(reimbursement, return_items, simulate)
def self.reimburse(reimbursement, return_items, simulate, *_optional_args)
return [] unless return_items.present?

exchange = Spree::Exchange.new(reimbursement.order, return_items)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Spree::ReimbursementType::OriginalPayment < Spree::ReimbursementType
extend Spree::ReimbursementType::ReimbursementHelpers

class << self
def reimburse(reimbursement, return_items, simulate)
def reimburse(reimbursement, return_items, simulate, _created_by)
unpaid_amount = return_items.sum(&:total).round(2, :down)
payments = reimbursement.order.payments.completed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ def create_refunds(reimbursement, payments, unpaid_amount, simulate, reimburseme
[reimbursement_list, unpaid_amount]
end

def create_credits(reimbursement, unpaid_amount, simulate, reimbursement_list = [])
credits = [create_credit(reimbursement, unpaid_amount, simulate)]
def create_credits(reimbursement, unpaid_amount, simulate, reimbursement_list = [], created_by:)
credits = [create_credit(reimbursement, unpaid_amount, simulate, created_by: created_by)]
unpaid_amount -= credits.sum(&:amount)
reimbursement_list += credits

Expand All @@ -43,19 +43,19 @@ def create_refund(reimbursement, payment, amount, simulate)

# If you have multiple methods of crediting a customer, overwrite this method
# Must return an array of objects the respond to #description, #display_amount
def create_credit(reimbursement, unpaid_amount, simulate)
creditable = create_creditable(reimbursement, unpaid_amount)
def create_credit(reimbursement, unpaid_amount, simulate, created_by:)
creditable = create_creditable(reimbursement, unpaid_amount, created_by: created_by)
credit = reimbursement.credits.build(creditable: creditable, amount: unpaid_amount)
simulate ? credit.readonly! : credit.save!
credit
end

def create_creditable(reimbursement, unpaid_amount)
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),
created_by: Spree::StoreCredit.default_created_by,
created_by: created_by,
memo: "Refund for uncreditable payments on order #{reimbursement.order.number}",
currency: reimbursement.order.currency
)
Expand Down
21 changes: 18 additions & 3 deletions core/app/models/spree/reimbursement_type/store_credit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,32 @@ class Spree::ReimbursementType::StoreCredit < Spree::ReimbursementType
extend Spree::ReimbursementType::ReimbursementHelpers

class << self
def reimburse(reimbursement, return_items, simulate)
def reimburse(reimbursement, return_items, simulate, created_by: nil)
unless created_by
Spree::Deprecation.warn("Calling #reimburse on #{self} without created_by is deprecated")
end
unpaid_amount = return_items.sum(&:total).to_d.round(2, :down)
payments = store_credit_payments(reimbursement)
reimbursement_list = []

# Credit each store credit that was used on the order
reimbursement_list, unpaid_amount = create_refunds(reimbursement, payments, unpaid_amount, simulate, reimbursement_list)
reimbursement_list, unpaid_amount = create_refunds(
reimbursement,
payments,
unpaid_amount,
simulate,
reimbursement_list
)

# If there is any amount left to pay out to the customer, then create credit with that amount
if unpaid_amount > 0.0
reimbursement_list, _unpaid_amount = create_credits(reimbursement, unpaid_amount, simulate, reimbursement_list)
reimbursement_list, _unpaid_amount = create_credits(
reimbursement,
unpaid_amount,
simulate,
reimbursement_list,
created_by: created_by
)
end

reimbursement_list
Expand Down
8 changes: 0 additions & 8 deletions core/app/models/spree/store_credit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ class Spree::StoreCredit < Spree::PaymentSource
ADJUSTMENT_ACTION = 'adjustment'
INVALIDATE_ACTION = 'invalidate'

DEFAULT_CREATED_BY_EMAIL = "spree@example.com"

belongs_to :user, class_name: Spree::UserClassHandle.new
belongs_to :created_by, class_name: Spree::UserClassHandle.new
belongs_to :category, class_name: "Spree::StoreCreditCategory"
Expand Down Expand Up @@ -196,12 +194,6 @@ def invalidate(reason, user_performing_invalidation)
end
end

class << self
def default_created_by
Spree.user_class.find_by(email: DEFAULT_CREATED_BY_EMAIL)
end
end

private

def create_credit_record(amount, action_attributes = {})
Expand Down
3 changes: 2 additions & 1 deletion core/spec/models/spree/customer_return_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,8 @@
end

context 'when all reimbursements are reimbursed' do
before { reimbursement.perform! }
let(:created_by_user) { create(:user, email: 'user@email.com') }
before { reimbursement.perform!(created_by: created_by_user) }

it { is_expected.to be true }
end
Expand Down
Loading