Merge pull request #12132 from rioug/9290-capture-payment-ignore-order-change

Fix capturing order payment ignore order changes

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

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

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