From 9215ccc3530fb6a6af44bf6a5b5700e33931d25d Mon Sep 17 00:00:00 2001 From: "Nihal M. Kelanthodika" Date: Mon, 31 Jan 2022 14:20:10 +0530 Subject: [PATCH 1/6] Prevents creation of payment adnustment when refunding or crediting an order --- app/models/spree/payment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index d1ebf2aaeb..6da10ab81c 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -147,7 +147,7 @@ module Spree adjustment.originator = payment_method adjustment.label = adjustment_label adjustment.save - elsif payment_method.present? + elsif amount.positive? && payment_method.present? payment_method.create_adjustment(adjustment_label, self, true) adjustment.reload end From b0174203192725f634332d4c665feb9ac9460294 Mon Sep 17 00:00:00 2001 From: "Nihal M. Kelanthodika" Date: Fri, 28 Jan 2022 16:29:50 +0530 Subject: [PATCH 2/6] Add specs to check ensure payment method fee is not applied when crediting/refunding order --- spec/models/spree/payment_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 420160ae60..0627e811b8 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -405,6 +405,19 @@ describe Spree::Payment do end end + context "if payment method has any payment fees" do + before do + payment.order.stub outstanding_balance: 10 + payment.stub credit_allowed: 200 + end + + it "should not applied any transaction fees" do + payment.credit! + expect(payment.adjustment.try(:finalized?)).to eq(false) + expect(order.all_adjustments.payment_fee.eligible.length).to eq(0) + end + end + context "when outstanding_balance is equal to payment amount" do before do payment.order.stub outstanding_balance: payment.amount From 8084ad00689e3eddb34bdc90b20a886d96d75e69 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 15 Feb 2022 12:17:08 +0000 Subject: [PATCH 3/6] Extract a comment-method so the code conveys it's purpose --- app/models/spree/payment.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 6da10ab81c..8d66fc2757 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -147,7 +147,7 @@ module Spree adjustment.originator = payment_method adjustment.label = adjustment_label adjustment.save - elsif amount.positive? && payment_method.present? + elsif !processing_refund? && payment_method.present? payment_method.create_adjustment(adjustment_label, self, true) adjustment.reload end @@ -163,6 +163,10 @@ module Spree private + def processing_refund? + amount.negative? + end + # Don't charge fees for invalid or failed payments. # This is called twice for failed payments, because the persistence of the 'failed' # state is acheived through some trickery using an after_rollback callback on the From 5be1b139b93d031dcee14bd0849eb9b789efbec9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 15 Feb 2022 12:19:15 +0000 Subject: [PATCH 4/6] Replace .stub with expect(x).to receive(:y) The #stub method is slightly deprecated syntax. Using #expect has the additional benefit here of failing if the object does not receive the expected method call. --- spec/models/spree/payment_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 0627e811b8..ac5600d9be 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -407,8 +407,8 @@ describe Spree::Payment do context "if payment method has any payment fees" do before do - payment.order.stub outstanding_balance: 10 - payment.stub credit_allowed: 200 + expect(payment.order).to receive(:outstanding_balance).at_least(:once) { 10 } + expect(payment).to receive(:credit_allowed) { 200 } end it "should not applied any transaction fees" do From 5fba7811c4c829d8bacfa8ba31270aeff8802ad2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 15 Feb 2022 12:21:34 +0000 Subject: [PATCH 5/6] Remove #try #try is useful when the object might be nil or might not respond to the given method. In this case we expect it to exist and respond to #finalized?, so this is a bit more precise. --- spec/models/spree/payment_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index ac5600d9be..5046ba3a2f 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -413,7 +413,7 @@ describe Spree::Payment do it "should not applied any transaction fees" do payment.credit! - expect(payment.adjustment.try(:finalized?)).to eq(false) + expect(payment.adjustment.finalized?).to eq(false) expect(order.all_adjustments.payment_fee.eligible.length).to eq(0) end end From 711a51f2ff3705823d089b94b5ecac96eeb63c7f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 15 Feb 2022 12:24:28 +0000 Subject: [PATCH 6/6] Remove .eligible scope In this case we actually expect there to be no payment fee adjustments at all on the payment, regardless of their eligibility. --- spec/models/spree/payment_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 5046ba3a2f..49b54b3256 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -414,7 +414,7 @@ describe Spree::Payment do it "should not applied any transaction fees" do payment.credit! expect(payment.adjustment.finalized?).to eq(false) - expect(order.all_adjustments.payment_fee.eligible.length).to eq(0) + expect(order.all_adjustments.payment_fee.length).to eq(0) end end