From 8abe0458808f27a513b47ddcc2274bca98d25d39 Mon Sep 17 00:00:00 2001 From: Graeme Nathan Date: Fri, 6 Apr 2018 14:50:20 -0700 Subject: [PATCH 1/8] Show dependency on 'spree@example.com' in spec Without a user with email address equal to 'spree@example.com', when a reimbursement is created a Spree::StoreCredit is not. This spec illustrates this behaviour. --- .../spree/reimbursement_type/store_credit_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 c93402cab77..c1623431d79 100644 --- a/core/spec/models/spree/reimbursement_type/store_credit_spec.rb +++ b/core/spec/models/spree/reimbursement_type/store_credit_spec.rb @@ -92,6 +92,17 @@ module Spree expect { subject }.to change { Spree::StoreCredit.count }.by(1) expect(Spree::StoreCredit.last.currency).to eq reimbursement.order.currency end + + context 'without a user with email address "spree@example.com" in the database' do + before do + Spree::LegacyUser.find_by(email: "spree@example.com").destroy + end + + it "creates a store credit with the same currency as the reimbursement's order" do + expect { subject }.to change { Spree::StoreCredit.count }.by(1) + expect(Spree::StoreCredit.last.currency).to eq reimbursement.order.currency + end + end end end end From 5a3b5eac027462a989211204d4e224ab207b2b38 Mon Sep 17 00:00:00 2001 From: Graeme Nathan Date: Fri, 6 Apr 2018 16:47:47 -0700 Subject: [PATCH 2/8] Update reimbursement helpers Previously, reimbursement helpers did not take a parameter for the creator of the reimbursement. Instead, they relied on a hard-coded dependency on a user in the database existing with the email address 'spree@example.com'. --- .../reimbursement_type/reimbursement_helpers.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/app/models/spree/reimbursement_type/reimbursement_helpers.rb b/core/app/models/spree/reimbursement_type/reimbursement_helpers.rb index b2d6a0b59f4..5a16c50f509 100644 --- a/core/app/models/spree/reimbursement_type/reimbursement_helpers.rb +++ b/core/app/models/spree/reimbursement_type/reimbursement_helpers.rb @@ -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, creator, reimbursement_list = []) + credits = [create_credit(reimbursement, unpaid_amount, simulate, creator)] unpaid_amount -= credits.sum(&:amount) reimbursement_list += credits @@ -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, creator) + creditable = create_creditable(reimbursement, unpaid_amount, creator) 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, creator) 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: creator, memo: "Refund for uncreditable payments on order #{reimbursement.order.number}", currency: reimbursement.order.currency ) From 8994b5b74828089f4401b4d6bd1d610c4bc6df87 Mon Sep 17 00:00:00 2001 From: Graeme Nathan Date: Fri, 6 Apr 2018 16:59:00 -0700 Subject: [PATCH 3/8] Pass creator as param when performing reimbursement --- .../spree/admin/reimbursements_controller.rb | 4 ++-- core/app/models/spree/order_cancellations.rb | 8 +++++-- core/app/models/spree/reimbursement.rb | 24 ++++++++++++++----- .../models/spree/reimbursement_performer.rb | 20 +++++++++++----- core/app/models/spree/reimbursement_type.rb | 2 +- .../models/spree/reimbursement_type/credit.rb | 8 +++++-- .../spree/reimbursement_type/exchange.rb | 2 +- .../reimbursement_type/original_payment.rb | 2 +- .../spree/reimbursement_type/store_credit.rb | 22 ++++++++++++++--- 9 files changed, 68 insertions(+), 24 deletions(-) diff --git a/backend/app/controllers/spree/admin/reimbursements_controller.rb b/backend/app/controllers/spree/admin/reimbursements_controller.rb index 95b2577aa37..9023ee0cea7 100644 --- a/backend/app/controllers/spree/admin/reimbursements_controller.rb +++ b/backend/app/controllers/spree/admin/reimbursements_controller.rb @@ -13,7 +13,7 @@ class ReimbursementsController < ResourceController rescue_from Spree::Core::GatewayError, with: :spree_core_gateway_error def perform - @reimbursement.perform! + @reimbursement.perform!(try_spree_current_user) redirect_to location_after_save end @@ -57,7 +57,7 @@ def load_stock_locations end def load_simulated_refunds - @reimbursement_objects = @reimbursement.simulate + @reimbursement_objects = @reimbursement.simulate(try_spree_current_user) end def spree_core_gateway_error(error) diff --git a/core/app/models/spree/order_cancellations.rb b/core/app/models/spree/order_cancellations.rb index 5e3e808e4b7..c2ecebea1b9 100644 --- a/core/app/models/spree/order_cancellations.rb +++ b/core/app/models/spree/order_cancellations.rb @@ -83,13 +83,17 @@ def cancel_unit(inventory_unit, reason: Spree::UnitCancel::DEFAULT_REASON, whodu # @api public # @param [Array] inventory_units the inventory units to be reimbursed # @return [Reimbursement] the reimbursement for inventory being canceled - def reimburse_units(inventory_units) + def reimburse_units(inventory_units, creator = nil) + unless creator + creator = Spree.user_class.where(email: 'spree@example.com').first + Spree::Deprecation.warn("Calling #reimburse_units on #{self} without creator 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(creator) end reimbursement diff --git a/core/app/models/spree/reimbursement.rb b/core/app/models/spree/reimbursement.rb index acd54703c49..ffd48db7f63 100644 --- a/core/app/models/spree/reimbursement.rb +++ b/core/app/models/spree/reimbursement.rb @@ -98,12 +98,16 @@ def unpaid_amount total - paid_amount end - def perform! + def perform!(creator = nil) + unless creator + creator = Spree.user_class.where(email: 'spree@example.com').first + Spree::Deprecation.warn("Calling #perform on #{self} without creator is deprecated") + end reimbursement_tax_calculator.call(self) reload update!(total: calculated_total) - reimbursement_performer.perform(self) + reimbursement_performer.perform(self, creator) if unpaid_amount_within_tolerance? reimbursed! @@ -116,12 +120,16 @@ def perform! end end - def simulate + def simulate(creator = nil) + unless creator + creator = Spree.user_class.where(email: 'spree@example.com').first + Spree::Deprecation.warn("Calling #simulate on #{self} without creator is deprecated") + end reimbursement_simulator_tax_calculator.call(self) reload update!(total: calculated_total) - reimbursement_performer.simulate(self) + reimbursement_performer.simulate(self, creator) end def return_items_requiring_exchange @@ -140,10 +148,14 @@ def all_exchanges? # # @api public # @return [void] - def return_all + def return_all(creator = nil) + unless creator + creator = Spree.user_class.where(email: 'spree@example.com').first + Spree::Deprecation.warn("Calling #return_all on #{self} without creator is deprecated") + end return_items.each(&:accept!) save! - perform! + perform!(creator) end private diff --git a/core/app/models/spree/reimbursement_performer.rb b/core/app/models/spree/reimbursement_performer.rb index e1b79e046f2..4ef62be9bc8 100644 --- a/core/app/models/spree/reimbursement_performer.rb +++ b/core/app/models/spree/reimbursement_performer.rb @@ -11,22 +11,30 @@ class << self # - #description # - #display_amount # so they can be displayed in the Admin UI appropriately. - def simulate(reimbursement) - execute(reimbursement, true) + def simulate(reimbursement, creator = nil) + unless creator + creator = Spree.user_class.where(email: 'spree@example.com').first + Spree::Deprecation.warn("Calling #simulate on #{self} without creator is deprecated") + end + execute(reimbursement, true, creator) end # Actually perform the reimbursement - def perform(reimbursement) - execute(reimbursement, false) + def perform(reimbursement, creator = nil) + unless creator + creator = Spree.user_class.find_by(email: 'spree@example.com') + Spree::Deprecation.warn("Calling #perform on #{self} without creator is deprecated") + end + execute(reimbursement, false, creator) end private - def execute(reimbursement, simulate) + def execute(reimbursement, simulate, creator) 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, creator) end end diff --git a/core/app/models/spree/reimbursement_type.rb b/core/app/models/spree/reimbursement_type.rb index fb2db58da1a..1fea623757f 100644 --- a/core/app/models/spree/reimbursement_type.rb +++ b/core/app/models/spree/reimbursement_type.rb @@ -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, _creator) raise "Implement me" end end diff --git a/core/app/models/spree/reimbursement_type/credit.rb b/core/app/models/spree/reimbursement_type/credit.rb index 16e35a07814..7c62b07d01d 100644 --- a/core/app/models/spree/reimbursement_type/credit.rb +++ b/core/app/models/spree/reimbursement_type/credit.rb @@ -5,9 +5,13 @@ class ReimbursementType::Credit < Spree::ReimbursementType extend Spree::ReimbursementType::ReimbursementHelpers class << self - def reimburse(reimbursement, return_items, simulate) + def reimburse(reimbursement, return_items, simulate, creator = nil) + unless creator + creator = Spree.user_class.where(email: 'spree@example.com').first + Spree::Deprecation.warn("Calling #reimburse on #{self} without creator 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, creator) reimbursement_list end end diff --git a/core/app/models/spree/reimbursement_type/exchange.rb b/core/app/models/spree/reimbursement_type/exchange.rb index 7c4867f96e5..e6f3141b7a5 100644 --- a/core/app/models/spree/reimbursement_type/exchange.rb +++ b/core/app/models/spree/reimbursement_type/exchange.rb @@ -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, _creator) return [] unless return_items.present? exchange = Spree::Exchange.new(reimbursement.order, return_items) diff --git a/core/app/models/spree/reimbursement_type/original_payment.rb b/core/app/models/spree/reimbursement_type/original_payment.rb index 72acae86dd2..f8f2323d27d 100644 --- a/core/app/models/spree/reimbursement_type/original_payment.rb +++ b/core/app/models/spree/reimbursement_type/original_payment.rb @@ -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, _creator) unpaid_amount = return_items.sum(&:total).round(2, :down) payments = reimbursement.order.payments.completed diff --git a/core/app/models/spree/reimbursement_type/store_credit.rb b/core/app/models/spree/reimbursement_type/store_credit.rb index 5c2aff0b7cb..7156fc1f734 100644 --- a/core/app/models/spree/reimbursement_type/store_credit.rb +++ b/core/app/models/spree/reimbursement_type/store_credit.rb @@ -4,17 +4,33 @@ class Spree::ReimbursementType::StoreCredit < Spree::ReimbursementType extend Spree::ReimbursementType::ReimbursementHelpers class << self - def reimburse(reimbursement, return_items, simulate) + def reimburse(reimbursement, return_items, simulate, creator = nil) + unless creator + creator = Spree.user_class.where(email: 'spree@example.com').first + Spree::Deprecation.warn("Calling #reimburse on #{self} without creator 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, + creator, + reimbursement_list + ) end reimbursement_list From a1a783a8a5c6c6f418058223833d4d8cbd6d07cd Mon Sep 17 00:00:00 2001 From: Graeme Nathan Date: Fri, 6 Apr 2018 17:09:59 -0700 Subject: [PATCH 4/8] Update reimbursement controller spec --- .../spree/admin/reimbursements_controller_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/backend/spec/controllers/spree/admin/reimbursements_controller_spec.rb b/backend/spec/controllers/spree/admin/reimbursements_controller_spec.rb index a36b095ee3d..3266d9b3aaa 100644 --- a/backend/spec/controllers/spree/admin/reimbursements_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/reimbursements_controller_spec.rb @@ -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 } From 18c51b777ac085d21011f2bae68e05a4edb5126b Mon Sep 17 00:00:00 2001 From: Graeme Nathan Date: Fri, 6 Apr 2018 17:13:20 -0700 Subject: [PATCH 5/8] Make reimbursement specs pass without special user --- .../models/spree/reimbursement_type/store_credit_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 c1623431d79..17aea92481b 100644 --- a/core/spec/models/spree/reimbursement_type/store_credit_spec.rb +++ b/core/spec/models/spree/reimbursement_type/store_credit_spec.rb @@ -12,10 +12,10 @@ module Spree let!(:default_refund_reason) { Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) } let!(:primary_credit_type) { create(:primary_credit_type) } - let!(:created_by_user) { create(:user, email: Spree::StoreCredit::DEFAULT_CREATED_BY_EMAIL) } + let!(:created_by_user) { create(:user, email: 'user@email.com') } let!(:default_reimbursement_category) { create(:store_credit_category) } - subject { Spree::ReimbursementType::StoreCredit.reimburse(reimbursement, [return_item, return_item2], simulate) } + subject { Spree::ReimbursementType::StoreCredit.reimburse(reimbursement, [return_item, return_item2], simulate, created_by_user) } before do reimbursement.update!(total: reimbursement.calculated_total) @@ -95,7 +95,8 @@ module Spree context 'without a user with email address "spree@example.com" in the database' do before do - Spree::LegacyUser.find_by(email: "spree@example.com").destroy + default_user = Spree::LegacyUser.find_by(email: "spree@example.com") + default_user.destroy if default_user end it "creates a store credit with the same currency as the reimbursement's order" do From 34860726f224b1aaa6cdbf62b4efff52f5e3a78e Mon Sep 17 00:00:00 2001 From: Graeme Nathan Date: Fri, 6 Apr 2018 17:17:37 -0700 Subject: [PATCH 6/8] Remove hard-code DEFAULT_CREATED_BY_EMAIL --- core/app/models/spree/store_credit.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/core/app/models/spree/store_credit.rb b/core/app/models/spree/store_credit.rb index 9eaafc5530b..69a21eef2d5 100644 --- a/core/app/models/spree/store_credit.rb +++ b/core/app/models/spree/store_credit.rb @@ -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" @@ -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 = {}) From cecc03cfcc73f62b6fbe7b0f04266ab0e51b972d Mon Sep 17 00:00:00 2001 From: Graeme Nathan Date: Fri, 6 Apr 2018 18:31:17 -0700 Subject: [PATCH 7/8] Update existing specs to pass new param --- core/spec/models/spree/customer_return_spec.rb | 3 ++- .../spree/order/outstanding_balance_integration_spec.rb | 3 ++- core/spec/models/spree/order_cancellations_spec.rb | 3 ++- core/spec/models/spree/refund_spec.rb | 3 ++- core/spec/models/spree/reimbursement_performer_spec.rb | 9 +++++---- core/spec/models/spree/reimbursement_spec.rb | 6 ++++-- core/spec/models/spree/reimbursement_type/credit_spec.rb | 3 ++- .../models/spree/reimbursement_type/exchange_spec.rb | 3 ++- .../spree/reimbursement_type/original_payment_spec.rb | 3 ++- .../models/spree/reimbursement_type/store_credit_spec.rb | 2 +- 10 files changed, 24 insertions(+), 14 deletions(-) diff --git a/core/spec/models/spree/customer_return_spec.rb b/core/spec/models/spree/customer_return_spec.rb index f98a0cdccac..a0572c33e58 100644 --- a/core/spec/models/spree/customer_return_spec.rb +++ b/core/spec/models/spree/customer_return_spec.rb @@ -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_user) } it { is_expected.to be true } end diff --git a/core/spec/models/spree/order/outstanding_balance_integration_spec.rb b/core/spec/models/spree/order/outstanding_balance_integration_spec.rb index 08206556c5e..8242dca6e96 100644 --- a/core/spec/models/spree/order/outstanding_balance_integration_spec.rb +++ b/core/spec/models/spree/order/outstanding_balance_integration_spec.rb @@ -83,13 +83,14 @@ context 'with a cancelled item' do let(:cancelations) { Spree::OrderCancellations.new(order) } let(:cancelled_item) { item_1 } + let(:created_by_user) { create(:user, email: 'user@email.com') } before do # Required to refund Spree::RefundReason.create!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) cancelations.cancel_unit(cancelled_item.inventory_units.first) - cancelations.reimburse_units(cancelled_item.inventory_units) + cancelations.reimburse_units(cancelled_item.inventory_units, created_by_user) order.reload end diff --git a/core/spec/models/spree/order_cancellations_spec.rb b/core/spec/models/spree/order_cancellations_spec.rb index ad801ea6140..394b1b8df8d 100644 --- a/core/spec/models/spree/order_cancellations_spec.rb +++ b/core/spec/models/spree/order_cancellations_spec.rb @@ -47,10 +47,11 @@ end describe "#reimburse_units" do - subject { Spree::OrderCancellations.new(order).reimburse_units(inventory_units) } + subject { Spree::OrderCancellations.new(order).reimburse_units(inventory_units, created_by_user) } let(:order) { create(:shipped_order, line_items_count: 2) } let(:inventory_units) { order.inventory_units } let!(:default_refund_reason) { Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) } + let(:created_by_user) { create(:user, email: 'user@email.com') } it "creates and performs a reimbursement" do expect { subject }.to change { Spree::Reimbursement.count }.by(1) diff --git a/core/spec/models/spree/refund_spec.rb b/core/spec/models/spree/refund_spec.rb index 249e89e4c49..bbbd36b3019 100644 --- a/core/spec/models/spree/refund_spec.rb +++ b/core/spec/models/spree/refund_spec.rb @@ -176,11 +176,12 @@ let(:customer_return) { reimbursement.customer_return } let(:reimbursement) { create(:reimbursement) } let!(:default_refund_reason) { Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) } + let(:created_by_user) { create(:user, email: 'user@email.com') } subject { Spree::Refund.total_amount_reimbursed_for(reimbursement) } context 'with reimbursements performed' do - before { reimbursement.perform! } + before { reimbursement.perform!(created_by_user) } it 'returns the total amount' do amount = Spree::Refund.total_amount_reimbursed_for(reimbursement) diff --git a/core/spec/models/spree/reimbursement_performer_spec.rb b/core/spec/models/spree/reimbursement_performer_spec.rb index 5f870e52450..57fb27567b9 100644 --- a/core/spec/models/spree/reimbursement_performer_spec.rb +++ b/core/spec/models/spree/reimbursement_performer_spec.rb @@ -7,25 +7,26 @@ let(:return_item) { reimbursement.return_items.first } let(:reimbursement_type) { double("ReimbursementType") } let(:reimbursement_type_hash) { { reimbursement_type => [return_item] } } + let(:created_by_user) { create(:user, email: 'user@email.com') } before do expect(Spree::ReimbursementPerformer).to receive(:calculate_reimbursement_types).and_return(reimbursement_type_hash) end describe ".simulate" do - subject { Spree::ReimbursementPerformer.simulate(reimbursement) } + subject { Spree::ReimbursementPerformer.simulate(reimbursement, created_by_user) } it "reimburses each calculated reimbursement types with the correct return items as a simulation" do - expect(reimbursement_type).to receive(:reimburse).with(reimbursement, [return_item], true) + expect(reimbursement_type).to receive(:reimburse).with(reimbursement, [return_item], true, created_by_user) subject end end describe '.perform' do - subject { Spree::ReimbursementPerformer.perform(reimbursement) } + subject { Spree::ReimbursementPerformer.perform(reimbursement, created_by_user) } it "reimburses each calculated reimbursement types with the correct return items as a simulation" do - expect(reimbursement_type).to receive(:reimburse).with(reimbursement, [return_item], false) + expect(reimbursement_type).to receive(:reimburse).with(reimbursement, [return_item], false, created_by_user) subject end end diff --git a/core/spec/models/spree/reimbursement_spec.rb b/core/spec/models/spree/reimbursement_spec.rb index 9815bb96dd8..53c9bd3489d 100644 --- a/core/spec/models/spree/reimbursement_spec.rb +++ b/core/spec/models/spree/reimbursement_spec.rb @@ -66,8 +66,9 @@ let!(:default_refund_reason) { Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) } let(:reimbursement) { create(:reimbursement, customer_return: customer_return, order: order, return_items: [return_item]) } + let(:created_by_user) { create(:user, email: 'user@email.com') } - subject { reimbursement.perform! } + subject { reimbursement.perform!(created_by_user) } before do order.shipments.each do |shipment| @@ -230,13 +231,14 @@ end describe "#return_all" do - subject { reimbursement.return_all } + subject { reimbursement.return_all(created_by_user) } let!(:default_refund_reason) { Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) } let(:order) { create(:shipped_order, line_items_count: 1) } let(:inventory_unit) { order.inventory_units.first } let(:return_item) { build(:return_item, inventory_unit: inventory_unit) } let(:reimbursement) { build(:reimbursement, order: order, return_items: [return_item]) } + let(:created_by_user) { create(:user, email: 'user@email.com') } it "accepts all the return items" do expect { subject }.to change { return_item.acceptance_status }.to "accepted" diff --git a/core/spec/models/spree/reimbursement_type/credit_spec.rb b/core/spec/models/spree/reimbursement_type/credit_spec.rb index 18100d74110..4702a0e307f 100644 --- a/core/spec/models/spree/reimbursement_type/credit_spec.rb +++ b/core/spec/models/spree/reimbursement_type/credit_spec.rb @@ -10,13 +10,14 @@ module Spree let(:simulate) { false } let!(:default_refund_reason) { Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) } let(:creditable) { DummyCreditable.new(amount: 99.99) } + let(:created_by_user) { create(:user, email: 'user@email.com') } class DummyCreditable < Spree::Base attr_accessor :amount self.table_name = 'spree_payments' # Your creditable class should not use this table end - subject { Spree::ReimbursementType::Credit.reimburse(reimbursement, [return_item], simulate) } + subject { Spree::ReimbursementType::Credit.reimburse(reimbursement, [return_item], simulate, created_by_user) } before do reimbursement.update!(total: reimbursement.calculated_total) diff --git a/core/spec/models/spree/reimbursement_type/exchange_spec.rb b/core/spec/models/spree/reimbursement_type/exchange_spec.rb index 3a699391869..38757f35991 100644 --- a/core/spec/models/spree/reimbursement_type/exchange_spec.rb +++ b/core/spec/models/spree/reimbursement_type/exchange_spec.rb @@ -9,8 +9,9 @@ module Spree let(:return_items) { reimbursement.return_items } let(:new_exchange) { double("Exchange") } let(:simulate) { true } + let(:created_by_user) { create(:user, email: 'user@email.com') } - subject { Spree::ReimbursementType::Exchange.reimburse(reimbursement, return_items, simulate) } + subject { Spree::ReimbursementType::Exchange.reimburse(reimbursement, return_items, simulate, created_by_user) } context 'return items are supplied' do before do diff --git a/core/spec/models/spree/reimbursement_type/original_payment_spec.rb b/core/spec/models/spree/reimbursement_type/original_payment_spec.rb index 85cddeaeca9..ea97ade5af7 100644 --- a/core/spec/models/spree/reimbursement_type/original_payment_spec.rb +++ b/core/spec/models/spree/reimbursement_type/original_payment_spec.rb @@ -9,8 +9,9 @@ module Spree let(:payment) { reimbursement.order.payments.first } let(:simulate) { false } let!(:default_refund_reason) { Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) } + let(:created_by_user) { create(:user, email: 'user@email.com') } - subject { Spree::ReimbursementType::OriginalPayment.reimburse(reimbursement, [return_item], simulate) } + subject { Spree::ReimbursementType::OriginalPayment.reimburse(reimbursement, [return_item], simulate, created_by_user) } before { reimbursement.update!(total: reimbursement.calculated_total) } 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 17aea92481b..cf0f49f2219 100644 --- a/core/spec/models/spree/reimbursement_type/store_credit_spec.rb +++ b/core/spec/models/spree/reimbursement_type/store_credit_spec.rb @@ -12,7 +12,7 @@ module Spree let!(:default_refund_reason) { Spree::RefundReason.find_or_create_by!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) } let!(:primary_credit_type) { create(:primary_credit_type) } - let!(:created_by_user) { create(:user, email: 'user@email.com') } + let(:created_by_user) { create(:user, email: 'user@email.com') } let!(:default_reimbursement_category) { create(:store_credit_category) } subject { Spree::ReimbursementType::StoreCredit.reimburse(reimbursement, [return_item, return_item2], simulate, created_by_user) } From f7cd81e7b556c14519c80ab26a9fab123348a490 Mon Sep 17 00:00:00 2001 From: Graeme Nathan Date: Thu, 12 Apr 2018 13:30:15 -0700 Subject: [PATCH 8/8] Extend reimbursement feature spec Reimbursements only work if there is a user with the email address 'spree@example.com', or if you explicitly pass the creator when creating a reimbursement. This commit adds to the testing set-up for this spec by mocking a signed in admin creating the reimbursement, who can be passed as the creator. --- .../spec/features/admin/orders/return_payment_state_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/spec/features/admin/orders/return_payment_state_spec.rb b/backend/spec/features/admin/orders/return_payment_state_spec.rb index b0a42f45580..4fe9ffb8726 100644 --- a/backend/spec/features/admin/orders/return_payment_state_spec.rb +++ b/backend/spec/features/admin/orders/return_payment_state_spec.rb @@ -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