Merge pull request #7645 from Matt-Yorkley/stripe-payment-processing

Stripe payment processing

app/controllers/spree/orders_controller.rb

@@ -27,13 +27,7 @@ module Spree
     def show
       @order = Spree::Order.find_by!(number: params[:id])
 
-      if params.key?("payment_intent")
-        result = ProcessPaymentIntent.new(params["payment_intent"], @order).call!
-        unless result.ok?
-          flash[:error] = "#{I18n.t("payment_could_not_process")}. #{result.error}"
-        end
-        @order.reload
-      end
+      handle_stripe_response
     end
 
     def empty
@@ -146,6 +140,19 @@ module Spree
 
     private
 
+    # Stripe can redirect here after a payment is processed in the backoffice.
+    # We verify if it was successful here and persist the changes.
+    def handle_stripe_response
+      return unless params.key?("payment_intent")
+
+      result = ProcessPaymentIntent.new(params["payment_intent"], @order).call!
+
+      unless result.ok?
+        flash.now[:error] = "#{I18n.t("payment_could_not_process")}. #{result.error}"
+      end
+      @order.reload
+    end
+
     def discard_empty_line_items
       @order.line_items = @order.line_items.select { |li| li.quantity > 0 }
     end

app/models/spree/gateway/stripe_sca.rb

@@ -129,7 +129,19 @@ module Spree
         payment = fetch_payment(creditcard, gateway_options)
         raise Stripe::StripeError, I18n.t(:no_pending_payments) unless payment&.response_code
 
-        Stripe::PaymentIntentValidator.new.call(payment.response_code, stripe_account_id)
+        payment_intent_response = Stripe::PaymentIntentValidator.new.
+          call(payment.response_code, stripe_account_id)
+
+        raise_if_not_in_capture_state(payment_intent_response)
+
+        payment.response_code
+      end
+
+      def raise_if_not_in_capture_state(payment_intent_response)
+        state = payment_intent_response.status
+        return if state == 'requires_capture'
+
+        raise Stripe::StripeError, I18n.t(:invalid_payment_state, state: state)
       end
 
       def fetch_payment(creditcard, gateway_options)

app/models/spree/payment.rb

@@ -50,6 +50,7 @@ module Spree
     scope :failed, -> { with_state('failed') }
     scope :valid, -> { where('state NOT IN (?)', %w(failed invalid)) }
     scope :authorization_action_required, -> { where.not(cvv_response_message: nil) }
+    scope :with_payment_intent, ->(code) { where(response_code: code) }
 
     # order state machine (see http://github.com/pluginaweek/state_machine/tree/master for details)
     state_machine initial: :checkout do

app/services/process_payment_intent.rb

@@ -23,21 +23,20 @@ class ProcessPaymentIntent
     end
   end
 
-  def initialize(payment_intent, order, last_payment = nil)
+  def initialize(payment_intent, order)
     @payment_intent = payment_intent
     @order = order
-    @last_payment = last_payment.presence || OrderPaymentFinder.new(order).last_payment
+    @payment = order.payments.pending.with_payment_intent(payment_intent).first
   end
 
   def call!
-    validate_intent!
-    return Result.new(ok: false) unless valid?
+    return Result.new(ok: false) unless payment.present? && ready_for_capture?
+    return Result.new(ok: true) if already_processed?
 
-    OrderWorkflow.new(order).next
+    process_payment
 
-    if last_payment.can_complete?
-      last_payment.complete!
-      last_payment.mark_as_processed
+    if payment.reload.completed?
+      payment.mark_as_processed
 
       Result.new(ok: true)
     else
@@ -50,18 +49,29 @@ class ProcessPaymentIntent
 
   private
 
-  attr_reader :order, :payment_intent, :last_payment
+  attr_reader :order, :payment_intent, :payment
 
-  def valid?
-    order.present? && matches_last_payment?
+  def process_payment
+    if order.state == "payment"
+      # Moves the order to completed, which calls #process_payments!
+      OrderWorkflow.new(order).next
+    else
+      order.process_payments!
+    end
   end
 
-  def validate_intent!
-    Stripe::PaymentIntentValidator.new.call(payment_intent, stripe_account_id)
+  def ready_for_capture?
+    payment_intent_status == 'requires_capture'
   end
 
-  def matches_last_payment?
-    last_payment&.state == "pending" && last_payment&.response_code == payment_intent
+  def already_processed?
+    payment_intent_status == 'succeeded'
+  end
+
+  def payment_intent_status
+    @payment_intent_status ||= Stripe::PaymentIntentValidator.new.
+      call(payment_intent, stripe_account_id).
+      status
   end
 
   def stripe_account_id
@@ -69,6 +79,6 @@ class ProcessPaymentIntent
   end
 
   def preferred_enterprise_id
-    last_payment.payment_method.preferred_enterprise_id
+    payment.payment_method.preferred_enterprise_id
   end
 end

lib/stripe/payment_intent_validator.rb

@@ -8,9 +8,8 @@ module Stripe
                                                                stripe_account: stripe_account_id)
 
       raise_if_last_payment_error_present(payment_intent_response)
-      raise_if_not_in_capture_state(payment_intent_response)
 
-      payment_intent_id
+      payment_intent_response
     end
 
     private
@@ -21,12 +20,5 @@ module Stripe
 
       raise Stripe::StripeError, payment_intent_response.last_payment_error.message
     end
-
-    def raise_if_not_in_capture_state(payment_intent_response)
-      state = payment_intent_response.status
-      return unless state != 'requires_capture'
-
-      raise Stripe::StripeError, I18n.t(:invalid_payment_state, state: state)
-    end
   end
 end

spec/controllers/spree/orders_controller_spec.rb

@@ -82,7 +82,10 @@ describe Spree::OrdersController, type: :controller do
 
   describe "confirming a payment intent" do
     let(:customer) { create(:customer) }
-    let(:order) { create(:order, customer: customer, distributor: customer.enterprise) }
+    let(:order) {
+      create(:order_with_totals, customer: customer, distributor: customer.enterprise,
+                                 state: "payment")
+    }
     let(:payment_method) { create(:stripe_sca_payment_method) }
     let!(:payment) { create(
       :payment,
@@ -102,26 +105,53 @@ describe Spree::OrdersController, type: :controller do
 
       context "with a valid payment intent" do
         let(:payment_intent) { "pi_123" }
+        let(:payment_intent_response) { double(id: "pi_123", status: "requires_capture") }
 
         before do
           allow_any_instance_of(Stripe::PaymentIntentValidator)
             .to receive(:call)
             .with(payment_intent, kind_of(String))
-            .and_return(payment_intent)
+            .and_return(payment_intent_response)
+
+          allow(Spree::Order).to receive(:find_by!) { order }
         end
 
-        it "completes the payment" do
-          get :show, params: { id: order.number, payment_intent: payment_intent }
+        context "when the order is in payment state" do
+          it "completes the payment" do
+            expect(order).to receive(:process_payments!) do
+              payment.complete!
+            end
 
-          expect(response.status).to eq 200
-          payment.reload
-          expect(payment.cvv_response_message).to be nil
-          expect(payment.state).to eq("completed")
+            get :show, params: { id: order.number, payment_intent: payment_intent }
+
+            expect(response.status).to eq 200
+            payment.reload
+            expect(payment.state).to eq("completed")
+            expect(payment.cvv_response_message).to be nil
+          end
+        end
+
+        context "when the order is already completed" do
+          before do
+            order.update_columns(state: "complete")
+          end
+
+          it "should still process the payment" do
+            expect(order).to receive(:process_payments!) do
+              payment.complete!
+            end
+
+            get :show, params: { id: order.number, payment_intent: payment_intent }
+            expect(response.status).to eq 200
+            payment.reload
+            expect(payment.state).to eq("completed")
+            expect(payment.cvv_response_message).to be nil
+          end
         end
       end
 
-      context "with an invalid payment intent" do
-        let(:payment_intent) { "invalid" }
+      context "when the payment intent response has errors" do
+        let(:payment_intent) { "pi_123" }
 
         before do
           allow_any_instance_of(Stripe::PaymentIntentValidator)

spec/lib/stripe/payment_intent_validator_spec.rb

@@ -27,23 +27,11 @@ module Stripe
         it "returns payment intent id and does not raise" do
           expect {
             result = validator.call(payment_intent_id, stripe_account_id)
-            expect(result).to eq payment_intent_id
+            expect(result).to eq payment_intent_response_body
           }.to_not raise_error Stripe::StripeError
         end
       end
 
-      context "when payment intent status is not requires status" do
-        let(:payment_intent_response_body) {
-          JSON.generate(id: payment_intent_id, status: "failed")
-        }
-
-        it "raises Stripe error with an invalid_payment_state message" do
-          expect {
-            validator.call(payment_intent_id, stripe_account_id)
-          }.to raise_error Stripe::StripeError, "Invalid payment state: failed"
-        end
-      end
-
       context "when payment intent contains an error" do
         let(:payment_intent_response_body) {
           JSON.generate(id: payment_intent_id, last_payment_error: { message: "No money" })

spec/models/spree/gateway/stripe_sca_spec.rb

@@ -50,6 +50,35 @@ describe Spree::Gateway::StripeSCA, type: :model do
       expect(response.success?).to eq false
       expect(response.message).to eq "Invalid payment state: succeeded"
     end
+
+    context "when payment intent state is not in 'requires_capture' state" do
+      before do
+        payment
+      end
+
+      it "succeeds if payment intent state is requires_capture" do
+        stub_request(:post, "https://api.stripe.com/v1/payment_intents/12345/capture").
+          with(body: {"amount_to_capture" => order.total}).
+          to_return(status: 200, body: capture_successful)
+
+        allow(Stripe::PaymentIntentValidator).to receive_message_chain(:new, :call).
+          and_return(double(status: "requires_capture"))
+
+        response = subject.purchase(order.total, credit_card, gateway_options)
+
+        expect(response.success?).to eq true
+      end
+
+      it "does not succeed if payment intent state is not requires_capture" do
+        allow(Stripe::PaymentIntentValidator).to receive_message_chain(:new, :call).
+          and_return(double(status: "not_ready_yet"))
+
+        response = subject.purchase(order.total, credit_card, gateway_options)
+
+        expect(response.success?).to eq false
+        expect(response.message).to eq "Invalid payment state: not_ready_yet"
+      end
+    end
   end
 
   def payment_intent(amount, status)

spec/services/process_payment_intent_spec.rb

@@ -7,7 +7,9 @@ describe ProcessPaymentIntent do
 
   describe "processing a payment intent" do
     let(:customer) { create(:customer) }
-    let(:order) { create(:order, customer: customer, distributor: customer.enterprise, state: "payment") }
+    let(:order) {
+      create(:order_with_totals, customer: customer, distributor: customer.enterprise, state: "payment")
+    }
     let(:payment_method) { create(:stripe_sca_payment_method) }
     let!(:payment) { create(
       :payment,
@@ -23,53 +25,110 @@ describe ProcessPaymentIntent do
       allow(Stripe::PaymentIntentValidator).to receive(:new).and_return(validator)
     end
 
-    context "an invalid intent" do
-      let(:intent) { "invalid" }
+    context "with an invalid intent" do
+      let(:intent) { "pi_123" }
       let(:service) { ProcessPaymentIntent.new(intent, order) }
 
-      before do
-        allow(validator)
-          .to receive(:call).with(intent, anything).and_raise(Stripe::StripeError, "error message")
+      context "which does not match the last payment" do
+        let(:intent) { "pi_456" }
+
+        it "returns false" do
+          result = service.call!
+
+          expect(result.ok?).to eq(false)
+          expect(result.error).to eq("")
+        end
+
+        it "does not complete the payment" do
+          service.call!
+          expect(payment.reload.state).to eq("pending")
+        end
       end
 
-      it "returns the error message" do
-        result = service.call!
+      context "where the stripe payment intent validation responds with errors" do
+        before do
+          allow(validator)
+            .to receive(:call).with(intent, anything).and_raise(Stripe::StripeError, "error message")
+        end
 
-        expect(result.ok?).to eq(false)
-        expect(result.error).to eq("error message")
-      end
+        it "returns returns the error message" do
+          result = service.call!
 
-      it "does not complete the payment" do
-        service.call!
-        expect(payment.reload.state).to eq("pending")
+          expect(result.ok?).to eq(false)
+          expect(result.error).to eq("error message")
+        end
+
+        it "does not complete the payment" do
+          service.call!
+          expect(payment.reload.state).to eq("pending")
+        end
       end
     end
 
     context "a valid intent" do
       let(:intent) { "pi_123" }
+      let(:intent_response) { double(status: "requires_capture") }
       let(:service) { ProcessPaymentIntent.new(intent, order) }
 
       before do
         allow(order).to receive(:deliver_order_confirmation_email)
-        allow(validator).to receive(:call).with(intent, anything).and_return(intent) 
+        allow(validator).to receive(:call).with(intent, anything).and_return(intent_response)
       end
 
       it "validates the intent" do
+        expect(order).to receive(:process_payments!) { true }
         service.call!
         expect(validator).to have_received(:call)
       end
 
-      it "completes the payment" do
+      it "processes the order's payment" do
+        allow(order).to receive(:pending_payments) { [payment] }
+
+        expect(order).to receive(:process_payments!).and_call_original
+        expect(payment).to receive(:purchase!) { true }
         service.call!
-        payment.reload
-        expect(payment.state).to eq("completed")
-        expect(payment.cvv_response_message).to be nil
       end
 
-      it "completes the order" do
-        service.call!
-        expect(order.state).to eq("complete")
-        expect(order).to have_received(:deliver_order_confirmation_email)
+      context "when payment processing succeeds" do
+        before do
+          allow(order).to receive(:process_payments!) do
+            payment.complete!
+          end
+        end
+
+        it "completes the payment" do
+          service.call!
+          payment.reload
+          expect(payment.state).to eq("completed")
+          expect(payment.cvv_response_message).to be nil
+        end
+
+        it "completes the order" do
+          service.call!
+          expect(order.state).to eq("complete")
+          expect(order).to have_received(:deliver_order_confirmation_email)
+        end
+      end
+
+      context "when payment processing fails" do
+        before do
+          allow(order).to receive(:process_payments!) do
+            payment.failure!
+          end
+        end
+
+        it "does not complete the payment" do
+          service.call!
+          payment.reload
+          expect(payment.state).to eq("failed")
+        end
+
+        it "completes the order, but with failed payment state recorded" do
+          service.call!
+          expect(order.state).to eq("complete")
+          expect(order.payment_state).to eq("failed")
+          expect(order).to have_received(:deliver_order_confirmation_email)
+        end
       end
     end
 
@@ -97,11 +156,12 @@ describe ProcessPaymentIntent do
 
     context "when the payment can't be completed" do
       let(:intent) { "pi_123" }
-      let(:service) { ProcessPaymentIntent.new(intent, order, payment) }
+      let(:intent_response) { double(id: "pi_123", status: "requires_capture") }
+      let(:service) { ProcessPaymentIntent.new(intent, order) }
 
       before do
-        allow(payment).to receive(:can_complete?).and_return(false)
-        allow(validator).to receive(:call).with(intent, anything).and_return(intent)
+        allow(order).to receive(:process_payments!) { nil }
+        allow(validator).to receive(:call).with(intent, anything).and_return(intent_response)
       end
 
       it "returns a failed result" do