Merge pull request #7208 from mkllnk/7130-double-processing-payment

Avoid double processing payment

app/controllers/checkout_controller.rb

@@ -14,7 +14,7 @@ class CheckoutController < ::BaseController
 
   # We need pessimistic locking to avoid race conditions.
   # Otherwise we fail on duplicate indexes or end up with negative stock.
-  prepend_around_action CurrentOrderLocker, only: :update
+  prepend_around_action CurrentOrderLocker, only: [:edit, :update]
 
   prepend_before_action :check_hub_ready_for_checkout
   prepend_before_action :check_order_cycle_expiry

spec/controllers/checkout_controller_concurrency_spec.rb

@@ -42,16 +42,22 @@ describe CheckoutController, concurrency: true, type: :controller do
     allow(controller).to receive(:spree_current_user).and_return(order.user)
     allow(controller).to receive(:current_distributor).and_return(order.distributor)
     allow(controller).to receive(:current_order_cycle).and_return(order.order_cycle)
+
+    # Avoid setting headers as Rails 5 is not thread-safe here:
+    allow(controller).to receive(:restrict_iframes)
   end
 
-  # This spec does not seem to be running in two threads in Rails 5. There are errors for the
-  # same response headers being set twice, possibly indicating that there is only one response
-  # as opposed to two separate requests in two threads?
-  xit "handles two concurrent orders successfully" do
+  it "handles two concurrent orders successfully" do
     # New threads start running straight away. The breakpoint is after loading
     # the order and before advancing the order's state and making payments.
     breakpoint.lock
 
+    checkout_workflow_original = controller.method(:checkout_workflow)
+    expect(controller).to receive(:checkout_workflow) do |shipping_method_id|
+      breakpoint.synchronize {}
+      checkout_workflow_original.call(shipping_method_id)
+    end
+
     # Starting two checkout threads. The controller code will determine if
     # these two threads are synchronised correctly or run into a race condition.
     #

spec/controllers/checkout_controller_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe CheckoutController, type: :controller do
+  include StripeStubs
+
   let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) }
   let(:order_cycle) { create(:simple_order_cycle) }
   let(:order) { create(:order) }
@@ -97,11 +99,27 @@ describe CheckoutController, type: :controller do
       end
 
       describe "when the order is in payment state and a stripe payment intent is provided" do
+        let(:order) { create(:order_with_totals) }
+        let(:payment_method) { create(:stripe_sca_payment_method) }
+        let(:payment) {
+          create(
+            :payment,
+            amount: order.total,
+            state: "pending",
+            payment_method: payment_method,
+            response_code: "pi_123"
+          )
+        }
+
         before do
+          allow(Stripe).to receive(:api_key) { "sk_test_12345" }
+          stub_payment_intent_get_request
+          stub_successful_capture_request(order: order)
+
           order.update_attribute :state, "payment"
           order.ship_address = create(:address)
           order.save!
-          order.payments << create(:payment, state: "pending", response_code: "pi_123")
+          order.payments << payment
 
           # this is called a 2nd time after order completion from the reset_order_service
           expect(order_cycle_distributed_variants).to receive(:distributes_order_variants?).twice.and_return(true)