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 6c311dade7..97b77ace28 100644 --- a/engines/order_management/app/services/order_management/order/updater.rb +++ b/engines/order_management/app/services/order_management/order/updater.rb @@ -38,13 +38,6 @@ module OrderManagement update_pending_payment end - def update_pending_payment - return unless order.state.in? ["payment", "confirmation"] - return unless order.pending_payments.any? - - order.pending_payments.first.update_attribute :amount, order.total - end - # Updates the following Order total values: # # - payment_total - total value of all finalized Payments (excludes non-finalized Payments) @@ -239,6 +232,16 @@ module OrderManagement def requires_authorization? payments.requires_authorization.any? && payments.completed.empty? end + + def update_pending_payment + # We only want to update complete order pending payment when it's a cash payment. We assume + # that if the payment was a credit card it would alread have been processed, so we don't + # bother checking the payment type + return unless order.state.in? ["payment", "confirmation", "complete"] + return unless order.pending_payments.any? + + order.pending_payments.first.update_attribute :amount, order.total + end end end 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 921c4cc18a..0447e84cb8 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 @@ -8,7 +8,7 @@ module OrderManagement let(:order) { create(:order) } let(:updater) { OrderManagement::Order::Updater.new(order) } - context "updating order totals" do + describe "#update_totals" do before do 2.times { create(:line_item, order:, price: 10) } end @@ -19,11 +19,23 @@ module OrderManagement updater.update_totals expect(order.payment_total).to eq(10) end + end + + describe "#update_item_total" do + before do + 2.times { create(:line_item, order:, price: 10) } + end it "updates item total" do updater.update_item_total expect(order.item_total).to eq(20) end + end + + describe "#update_adjustment_total" do + before do + 2.times { create(:line_item, order:, price: 10) } + end it "updates adjustment totals" do allow(order).to receive_message_chain(:all_adjustments, :additional, :eligible, @@ -40,7 +52,7 @@ module OrderManagement end end - context "updating shipment state" do + describe "#update_shipment_state" do let(:shipment) { build(:shipment) } before do @@ -85,19 +97,111 @@ module OrderManagement order.state_changed('shipment') end - context "completed order" do + describe "#update" do + it "updates totals once" do + expect(updater).to receive(:update_totals).once + updater.update + end + + it "updates all adjustments" do + expect(updater).to receive(:update_all_adjustments) + updater.update + end + + context "completed order" do + before { allow(order).to receive(:completed?) { true } } + + it "updates payment state" do + expect(updater).to receive(:update_payment_state) + updater.update + end + + it "updates shipment state" do + expect(updater).to receive(:update_shipment_state) + updater.update + end + + context "whith pending payments" do + let(:order) { create(:completed_order_with_totals) } + + it "updates pending payments" do + payment = create(:payment, order:, amount: order.total) + + # update order so the order total will change + update_order_quantity(order) + order.payments.reload + + expect { updater.update }.to change { payment.reload.amount }.from(50).to(60) + end + end + end + + context "incompleted order" do + before { allow(order).to receive_messages completed?: false } + + it "doesnt update payment state" do + expect(updater).not_to receive(:update_payment_state) + updater.update + end + + it "doesnt update shipment state" do + expect(updater).not_to receive(:update_shipment_state) + updater.update + end + + it "doesnt update the order shipment" do + shipment = build(:shipment) + allow(order).to receive_messages shipments: [shipment] + + expect(shipment).not_to receive(:update!).with(order) + expect(updater).not_to receive(:update_shipments).with(order) + updater.update + end + + context "with pending payments" do + let!(:payment) { create(:payment, order:, amount: order.total) } + + context "with order in payment state" do + let(:order) { create(:order_with_totals, state: "payment") } + + it "updates pending payments" do + # update order so the order total will change + update_order_quantity(order) + order.payments.reload + + expect { updater.update }.to change { payment.reload.amount }.from(10).to(20) + end + end + + context "with order in confirmation state" do + let(:order) { create(:order_with_totals, state: "confirmation") } + + it "updates pending payments" do + # update order so the order total will change + update_order_quantity(order) + order.payments.reload + + expect { updater.update }.to change { payment.reload.amount }.from(10).to(20) + end + end + + context "with order in cart" do + let(:order) { create(:order_with_totals) } + + it "doesn't update pending payments" do + # update order so the order total will change + update_order_quantity(order) + + expect { updater.update }.not_to change { payment.reload.amount } + end + end + end + end + end + + describe "#update_shipments" do before { allow(order).to receive(:completed?) { true } } - it "updates payment state" do - expect(updater).to receive(:update_payment_state) - updater.update - end - - it "updates shipment state" do - expect(updater).to receive(:update_shipment_state) - updater.update - end - it "updates the order shipment" do shipment = build(:shipment) allow(order).to receive_messages shipments: [shipment] @@ -107,39 +211,6 @@ module OrderManagement end end - context "incompleted order" do - before { allow(order).to receive_messages completed?: false } - - it "doesnt update payment state" do - expect(updater).not_to receive(:update_payment_state) - updater.update - end - - it "doesnt update shipment state" do - expect(updater).not_to receive(:update_shipment_state) - updater.update - end - - it "doesnt update the order shipment" do - shipment = build(:shipment) - allow(order).to receive_messages shipments: [shipment] - - expect(shipment).not_to receive(:update!).with(order) - expect(updater).not_to receive(:update_shipments).with(order) - updater.update - end - end - - it "updates totals once" do - expect(updater).to receive(:update_totals).once - updater.update - end - - it "updates all adjustments" do - expect(updater).to receive(:update_all_adjustments) - updater.update - end - describe "#update_payment_state" do context "when the order has no valid payments" do it "is failed" do @@ -277,9 +348,28 @@ 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 - context '#shipping_address_from_distributor' do + describe '#shipping_address_from_distributor' do let(:distributor) { build(:distributor_enterprise) } let(:shipment) { create(:shipment_with, :shipping_method, shipping_method:) @@ -313,69 +403,52 @@ module OrderManagement end end - describe "updating order totals" do - describe "#update_totals_and_states" do - it "deals with legacy taxes" do - expect(updater).to receive(:handle_legacy_taxes) + describe "#update_totals_and_states" do + it "deals with legacy taxes" do + expect(updater).to receive(:handle_legacy_taxes) - updater.update_totals_and_states + updater.update_totals_and_states + end + end + + describe "#handle_legacy_taxes" do + context "when the order is incomplete" do + it "doesn't touch taxes" do + allow(order).to receive(:completed?) { false } + + expect(order).to_not receive(:create_tax_charge!) + updater.__send__(:handle_legacy_taxes) end end - describe "#handle_legacy_taxes" do - context "when the order is incomplete" do - it "doesn't touch taxes" do - allow(order).to receive(:completed?) { false } + context "when the order is complete" do + before { allow(order).to receive(:completed?) { true } } + + context "and the order has legacy taxes" do + let!(:legacy_tax_adjustment) { + create(:adjustment, order:, adjustable: order, included: false, + originator_type: "Spree::TaxRate") + } + + it "re-applies order taxes" do + expect(order).to receive(:create_tax_charge!) - expect(order).to_not receive(:create_tax_charge!) updater.__send__(:handle_legacy_taxes) end end - context "when the order is complete" do - before { allow(order).to receive(:completed?) { true } } + context "and the order has no legacy taxes" do + it "leaves taxes untouched" do + expect(order).to_not receive(:create_tax_charge!) - context "and the order has legacy taxes" do - let!(:legacy_tax_adjustment) { - create(:adjustment, order:, adjustable: order, included: false, - originator_type: "Spree::TaxRate") - } - - it "re-applies order taxes" do - expect(order).to receive(:create_tax_charge!) - - updater.__send__(:handle_legacy_taxes) - end - end - - context "and the order has no legacy taxes" do - it "leaves taxes untouched" do - expect(order).to_not receive(:create_tax_charge!) - - updater.__send__(:handle_legacy_taxes) - end + updater.__send__(:handle_legacy_taxes) end 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 + def update_order_quantity(order) + order.line_items.first.update_attribute(:quantity, 2) end end end diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index 237659e876..ff96e97a60 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -239,8 +239,11 @@ describe SubscriptionConfirmJob do context "when payments are processed without error" do before do - expect(payment).to receive(:process_offline!) { true } - expect(payment).to receive(:completed?) { true } + allow(payment).to receive(:process_offline!) do + # Mark payment as complete like it would be if sucessfully processed offline + payment.state = "complete" + true + end end it "sends only a subscription confirm email, no regular confirmation emails" do