From 13bb5aa8dde180a48253e9916e343511cefed68f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 25 Apr 2021 17:04:02 +0100 Subject: [PATCH 1/4] Update Payment after_save callback Backport from Spree 2.4 stable: https://github.com/spree/spree/commit/4d652a77fdcb14db6d0fadbd5966a91264889073 --- app/models/spree/payment.rb | 14 ++++++++- .../order_management/order/updater.rb | 6 +++- spec/models/spree/payment_spec.rb | 30 ++++++++++++++----- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index d16e428922..87c5d8955f 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -201,7 +201,19 @@ module Spree end def update_order - order.update! + if completed? + order.updater.update_payment_total + end + + if order.completed? + order.updater.update_payment_state + order.updater.update_shipments + order.updater.update_shipment_state + end + + if self.completed? || order.completed? + order.updater.persist_totals + end end # Necessary because some payment gateways will refuse payments with 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 c432214751..65742b67ad 100644 --- a/engines/order_management/app/services/order_management/order/updater.rb +++ b/engines/order_management/app/services/order_management/order/updater.rb @@ -38,7 +38,7 @@ module OrderManagement # - adjustment_total - total value of all adjustments # - total - order total, it's the equivalent to item_total plus adjustment_total def update_totals - order.payment_total = payments.completed.sum(:amount) + update_payment_total update_item_total update_adjustment_total update_order_total @@ -49,6 +49,10 @@ module OrderManagement shipments.each { |shipment| shipment.update!(order) } end + def update_payment_total + order.payment_total = payments.completed.sum(:amount) + end + def update_item_total order.item_total = line_items.sum('price * quantity') update_order_total diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 03eb892b53..bde1fa3c5a 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -531,15 +531,29 @@ describe Spree::Payment do end context "#save" do - it "should call order#update!" do - gateway.name = 'Gateway' - gateway.distributors << create(:distributor_enterprise) - gateway.save! + context "completed payments" do + it "updates order payment total" do + payment = create(:payment, amount: 100, order: order, state: "completed") + expect(order.payment_total).to eq payment.amount + end + end - order = create(:order) - payment = Spree::Payment.create(amount: 100, order: order, payment_method: gateway) - expect(order).to receive(:update!) - payment.save + context "non-completed payments" do + it "doesn't update order payment total" do + expect { + create(:payment, amount: 100, order: order) + }.not_to change { order.payment_total } + end + end + + context "completed orders" do + before { allow(order).to receive(:completed?) { true } } + + it "updates payment_state and shipments" do + expect(order.updater).to receive(:update_payment_state) + expect(order.updater).to receive(:update_shipment_state) + create(:payment, amount: 100, order: order) + end end context "when profiles are supported" do From 135a311c050d7ae5b68bee79800083568c4f7133 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 25 Apr 2021 22:47:45 +0100 Subject: [PATCH 2/4] Update void payments Backport from Spree 2.4 stable: https://github.com/spree/spree/commit/412199239827b820f5ab0165dae15fcbb4e1618e --- app/models/spree/payment.rb | 2 +- spec/models/spree/payment_spec.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 87c5d8955f..111de34e25 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -201,7 +201,7 @@ module Spree end def update_order - if completed? + if completed? || void? order.updater.update_payment_total end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index bde1fa3c5a..91220d5474 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -546,6 +546,15 @@ describe Spree::Payment do end end + context 'when the payment was completed but now void' do + let(:payment) { create(:payment, amount: 100, order: order, state: 'completed') } + + it 'updates order payment total' do + payment.void + expect(order.payment_total).to eq 0 + end + end + context "completed orders" do before { allow(order).to receive(:completed?) { true } } From 8e10f7af0e2ceda84f4ec52e05b4053c3c0dcffb Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 25 Apr 2021 22:51:22 +0100 Subject: [PATCH 3/4] Update payments controller test setup We need to use a completed order in the test setup here or it doesn't behave correctly. `order.completed_at` is nil, for example. --- .../admin/orders/payments/payments_controller_refunds_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb index 5ec3543894..e7ec842625 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb @@ -8,7 +8,7 @@ describe Spree::Admin::PaymentsController, type: :controller do let!(:shop) { create(:enterprise) } let!(:user) { shop.owner } - let!(:order) { create(:order, distributor: shop, state: 'complete') } + let!(:order) { create(:completed_order_with_totals, distributor: shop) } let!(:line_item) { create(:line_item, order: order, price: 5.0) } before do From 00c4a28d22a927d5f2401ac3e6cc63e268fa941f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 26 Apr 2021 08:55:14 +0100 Subject: [PATCH 4/4] Extract order-updating logic to Order::Updater --- app/models/spree/payment.rb | 14 +------------- .../services/order_management/order/updater.rb | 16 ++++++++++++++++ spec/models/spree/payment_spec.rb | 12 ++++++++++-- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 111de34e25..8cbb93c1c7 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -201,19 +201,7 @@ module Spree end def update_order - if completed? || void? - order.updater.update_payment_total - end - - if order.completed? - order.updater.update_payment_state - order.updater.update_shipments - order.updater.update_shipment_state - end - - if self.completed? || order.completed? - order.updater.persist_totals - end + OrderManagement::Order::Updater.new(order).after_payment_update(self) end # Necessary because some payment gateways will refuse payments with 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 65742b67ad..69eb056922 100644 --- a/engines/order_management/app/services/order_management/order/updater.rb +++ b/engines/order_management/app/services/order_management/order/updater.rb @@ -141,6 +141,22 @@ module OrderManagement order.ship_address = order.address_from_distributor end + def after_payment_update(payment) + if payment.completed? || payment.void? + update_payment_total + end + + if order.completed? + update_payment_state + update_shipments + update_shipment_state + end + + if payment.completed? || order.completed? + persist_totals + end + end + private def round_money(value) diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 91220d5474..230ae645dd 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -556,11 +556,19 @@ describe Spree::Payment do end context "completed orders" do + let(:order_updater) { OrderManagement::Order::Updater.new(order) } + before { allow(order).to receive(:completed?) { true } } it "updates payment_state and shipments" do - expect(order.updater).to receive(:update_payment_state) - expect(order.updater).to receive(:update_shipment_state) + expect(OrderManagement::Order::Updater).to receive(:new).with(order). + and_return(order_updater) + + expect(order_updater).to receive(:after_payment_update).with(kind_of(Spree::Payment)). + and_call_original + + expect(order_updater).to receive(:update_payment_state) + expect(order_updater).to receive(:update_shipment_state) create(:payment, amount: 100, order: order) end end