From 60677bce4f4bfc33d2479fe60dbfac9bc6b0b187 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 25 Aug 2021 17:17:28 -0700 Subject: [PATCH 1/5] void payments requiring auth upon marking order paid --- app/models/spree/order.rb | 7 +++++++ spec/models/spree/order_spec.rb | 15 +++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 8bb09d4e5d..12c3909731 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -344,6 +344,7 @@ module Spree # update payment and shipment(s) states, and save updater.update_payment_state + cancel_payments_requiring_auth shipments.each do |shipment| shipment.update!(self) shipment.finalize! @@ -611,6 +612,12 @@ module Spree private + def cancel_payments_requiring_auth + return unless payment_state == "paid" + + payments.requires_authorization.each(&:void_transaction!) + end + def fee_handler @fee_handler ||= OrderFeesHandler.new(self) end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index ecc4d5dae8..2f25e205f9 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -195,6 +195,21 @@ describe Spree::Order do expect(order.updater).to receive(:before_save_hook) order.finalize! end + + context "extra payments exist that require authorization" do + let!(:cash_payment) { build(:payment, state: "completed", amount: order.new_outstanding_balance) } + let!(:stripe_payment) { build(:payment, state: "requires_authorization") } + before do + order.payments << cash_payment + order.payments << stripe_payment + allow_any_instance_of(Spree::Payment).to receive(:void_transaction!) {} + end + + it "cancels payments requiring authorization" do + expect_any_instance_of(Spree::Payment).to receive(:void_transaction!) + order.finalize! + end + end end context "#process_payments!" do From 0ef07023ad858c6f261fb55412c505a0c45656f0 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 23 Sep 2021 14:24:18 -0700 Subject: [PATCH 2/5] remove any_instance_of in order_spec Co-authored-by: Maikel --- spec/models/spree/order_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 2f25e205f9..53ab50cc5f 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -202,7 +202,6 @@ describe Spree::Order do before do order.payments << cash_payment order.payments << stripe_payment - allow_any_instance_of(Spree::Payment).to receive(:void_transaction!) {} end it "cancels payments requiring authorization" do From 48a867ac996b90431b61c671b6524b5758e4e424 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 23 Sep 2021 14:25:24 -0700 Subject: [PATCH 3/5] expect specific payment to receive message Co-authored-by: Maikel --- spec/models/spree/order_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 53ab50cc5f..633639d137 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -205,7 +205,8 @@ describe Spree::Order do end it "cancels payments requiring authorization" do - expect_any_instance_of(Spree::Payment).to receive(:void_transaction!) + expect(stripe_payment).to receive(:void_transaction!) + expect(cash_payment).to_not receive(:void_transaction!) order.finalize! end end From 0b3d78b2a5455950d0f3fa45d1c1a6f4a91156c7 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 23 Sep 2021 14:35:47 -0700 Subject: [PATCH 4/5] void transactions in memory instead of fetching from db Co-authored-by: Maikel --- app/models/spree/order.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 12c3909731..332af710e8 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -615,7 +615,7 @@ module Spree def cancel_payments_requiring_auth return unless payment_state == "paid" - payments.requires_authorization.each(&:void_transaction!) + payments.to_a.select(&:requires_authorization?).each(&:void_transaction!) end def fee_handler From 3e02023bf8b06a766678615edd055daf92d03248 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 15 Nov 2021 15:31:12 +0000 Subject: [PATCH 5/5] Move handling of unused payments to Order::Updater An order can be set to paid in various cases that are unrelated to the order being finalized, so this bit of logic needs to be called at the point the order actually gets paid. --- app/models/spree/order.rb | 7 ------- .../services/order_management/order/updater.rb | 7 +++++++ .../order_management/order/updater_spec.rb | 16 ++++++++++++++++ spec/models/spree/order_spec.rb | 15 --------------- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 332af710e8..8bb09d4e5d 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -344,7 +344,6 @@ module Spree # update payment and shipment(s) states, and save updater.update_payment_state - cancel_payments_requiring_auth shipments.each do |shipment| shipment.update!(self) shipment.finalize! @@ -612,12 +611,6 @@ module Spree private - def cancel_payments_requiring_auth - return unless payment_state == "paid" - - payments.to_a.select(&:requires_authorization?).each(&:void_transaction!) - end - def fee_handler @fee_handler ||= OrderFeesHandler.new(self) end diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb index e17a9e3ad6..41d0374047 100644 --- a/engines/order_management/app/services/order_management/order/updater.rb +++ b/engines/order_management/app/services/order_management/order/updater.rb @@ -122,6 +122,7 @@ module OrderManagement last_payment_state = order.payment_state order.payment_state = infer_payment_state + cancel_payments_requiring_auth unless last_payment_state == "paid" track_payment_state_change(last_payment_state) order.payment_state @@ -162,6 +163,12 @@ module OrderManagement private + def cancel_payments_requiring_auth + return unless order.payment_state == "paid" + + payments.to_a.select(&:requires_authorization?).each(&:void_transaction!) + end + def round_money(value) (value * 100).round / 100.0 end diff --git a/engines/order_management/spec/services/order_management/order/updater_spec.rb b/engines/order_management/spec/services/order_management/order/updater_spec.rb index 142c042c81..3c46b8ebc4 100644 --- a/engines/order_management/spec/services/order_management/order/updater_spec.rb +++ b/engines/order_management/spec/services/order_management/order/updater_spec.rb @@ -360,6 +360,22 @@ module OrderManagement end end end + + context "when unused payments records exist which require authorization, but the order is fully paid" do + let!(:cash_payment) { build(:payment, state: "completed", amount: order.new_outstanding_balance) } + let!(:stripe_payment) { build(:payment, state: "requires_authorization") } + before do + order.payments << cash_payment + order.payments << stripe_payment + end + + it "cancels unused payments requiring authorization" do + expect(stripe_payment).to receive(:void_transaction!) + expect(cash_payment).to_not receive(:void_transaction!) + + order.updater.update_payment_state + end + end end end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 633639d137..ecc4d5dae8 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -195,21 +195,6 @@ describe Spree::Order do expect(order.updater).to receive(:before_save_hook) order.finalize! end - - context "extra payments exist that require authorization" do - let!(:cash_payment) { build(:payment, state: "completed", amount: order.new_outstanding_balance) } - let!(:stripe_payment) { build(:payment, state: "requires_authorization") } - before do - order.payments << cash_payment - order.payments << stripe_payment - end - - it "cancels payments requiring authorization" do - expect(stripe_payment).to receive(:void_transaction!) - expect(cash_payment).to_not receive(:void_transaction!) - order.finalize! - end - end end context "#process_payments!" do