From e686a4f6275dbea26842864b73bda9a245aa7677 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 14 May 2021 14:22:00 +0100
Subject: [PATCH 01/17] Move raising of errors when payment intent state !=
 "requires_capture" out of PaymentIntentValidator service

When we're fetching the payment intent via PaymentIntentValidator in StripeSCA#purhcase (to capture it), we want it to fail loudly if it's not in "requires_capture" state. We're now also re-using the same PaymentIntentValidator service to check if payment processing was *successful*, in which case we need it *not* to fail loudly if the state == "succeeded", eg != "requires_capture".
---
 app/models/spree/gateway/stripe_sca.rb | 14 +++++++++++++-
 app/services/process_payment_intent.rb | 10 ++++++----
 lib/stripe/payment_intent_validator.rb | 10 +---------
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb
index a2fcc94717..b804f8613d 100644
--- a/app/models/spree/gateway/stripe_sca.rb
+++ b/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 unless state != 'requires_capture'
+
+        raise Stripe::StripeError, I18n.t(:invalid_payment_state, state: state)
       end
 
       def fetch_payment(creditcard, gateway_options)
diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb
index ddf6f7496e..775f20f399 100644
--- a/app/services/process_payment_intent.rb
+++ b/app/services/process_payment_intent.rb
@@ -30,7 +30,6 @@ class ProcessPaymentIntent
   end
 
   def call!
-    validate_intent!
     return Result.new(ok: false) unless valid?
 
     OrderWorkflow.new(order).next
@@ -53,11 +52,14 @@ class ProcessPaymentIntent
   attr_reader :order, :payment_intent, :last_payment
 
   def valid?
-    order.present? && matches_last_payment?
+    order.present? && matches_last_payment? && ready_for_capture?
   end
 
-  def validate_intent!
-    Stripe::PaymentIntentValidator.new.call(payment_intent, stripe_account_id)
+  def ready_for_capture?
+    payment_intent_response = Stripe::PaymentIntentValidator.new.
+      call(payment_intent, stripe_account_id)
+
+    payment_intent_response.status == 'requires_capture'
   end
 
   def matches_last_payment?
diff --git a/lib/stripe/payment_intent_validator.rb b/lib/stripe/payment_intent_validator.rb
index 075e064fcc..879a49eac2 100644
--- a/lib/stripe/payment_intent_validator.rb
+++ b/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

From 5805104d13df91b203e948511a46cb242c184ffa Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 14 May 2021 14:31:57 +0100
Subject: [PATCH 02/17] Tidy up controller and add explanatory comment

---
 app/controllers/spree/orders_controller.rb | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb
index 51bacbb588..5b9a8f5cf2 100644
--- a/app/controllers/spree/orders_controller.rb
+++ b/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
@@ -162,6 +156,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[: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

From 37177e720774afc586f5d1e12d5eb23cb2aa1463 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 14 May 2021 15:12:47 +0100
Subject: [PATCH 03/17] Add test coverage to StripeSCA spec

---
 spec/models/spree/gateway/stripe_sca_spec.rb | 29 ++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/spec/models/spree/gateway/stripe_sca_spec.rb b/spec/models/spree/gateway/stripe_sca_spec.rb
index 8ddfe99373..3eea62c02d 100644
--- a/spec/models/spree/gateway/stripe_sca_spec.rb
+++ b/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)

From 471a7903f652ef52d8a0cfd2b05245356b9359dc Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 14 May 2021 15:23:28 +0100
Subject: [PATCH 04/17] Update PaymentIntentValidator spec

---
 spec/lib/stripe/payment_intent_validator_spec.rb | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/spec/lib/stripe/payment_intent_validator_spec.rb b/spec/lib/stripe/payment_intent_validator_spec.rb
index ac4a06a83c..efebbb6577 100644
--- a/spec/lib/stripe/payment_intent_validator_spec.rb
+++ b/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" })

From 227bdd7d4c1bdee1426c6ef4cf295a9a2d5ec307 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 14 May 2021 16:01:38 +0100
Subject: [PATCH 05/17] Update ProcessPaymentIntent spec

---
 spec/services/process_payment_intent_spec.rb | 50 ++++++++++++++------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/spec/services/process_payment_intent_spec.rb b/spec/services/process_payment_intent_spec.rb
index 5aabe3edbe..bcf76d6240 100644
--- a/spec/services/process_payment_intent_spec.rb
+++ b/spec/services/process_payment_intent_spec.rb
@@ -23,35 +23,54 @@ 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
@@ -97,11 +116,12 @@ describe ProcessPaymentIntent do
 
     context "when the payment can't be completed" do
       let(:intent) { "pi_123" }
+      let(:intent_response) { double(id: "pi_123", status: "requires_capture") }
       let(:service) { ProcessPaymentIntent.new(intent, order, payment) }
 
       before do
         allow(payment).to receive(:can_complete?).and_return(false)
-        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 "returns a failed result" do

From 16e3af9b49f1b62eed54f15d83f519a332d5150e Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 14 May 2021 16:08:52 +0100
Subject: [PATCH 06/17] Update OrderController spec

---
 .../spree/orders_controller_spec.rb            | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb
index 412b706cf5..ae41c46a8d 100644
--- a/spec/controllers/spree/orders_controller_spec.rb
+++ b/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,12 +105,19 @@ 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 }
+
+          allow(order).to receive(:process_payments!) do
+            payment.complete!
+          end
         end
 
         it "completes the payment" do
@@ -120,8 +130,8 @@ describe Spree::OrdersController, type: :controller do
         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)

From 81aac442f2dbcc418f73451464d3829efbecf31c Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 14 May 2021 18:00:56 +0100
Subject: [PATCH 07/17] Improve conditional in raise_if_not_in_capture_state

---
 app/models/spree/gateway/stripe_sca.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb
index b804f8613d..7ede0aa252 100644
--- a/app/models/spree/gateway/stripe_sca.rb
+++ b/app/models/spree/gateway/stripe_sca.rb
@@ -139,7 +139,7 @@ module Spree
 
       def raise_if_not_in_capture_state(payment_intent_response)
         state = payment_intent_response.status
-        return unless state != 'requires_capture'
+        return if state == 'requires_capture'
 
         raise Stripe::StripeError, I18n.t(:invalid_payment_state, state: state)
       end

From 2b9f9fce8667775d0e8199b1430937f69cadc6d2 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sat, 15 May 2021 13:19:41 +0100
Subject: [PATCH 08/17] Fix flash session memory-effect when showing errors
 after payment processing

We're not doing a redirect after setting this flash message, so we need to ensure it's discarded after the current page load.
---
 app/controllers/spree/orders_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb
index 5b9a8f5cf2..4c907ad862 100644
--- a/app/controllers/spree/orders_controller.rb
+++ b/app/controllers/spree/orders_controller.rb
@@ -164,7 +164,7 @@ module Spree
       result = ProcessPaymentIntent.new(params["payment_intent"], @order).call!
 
       unless result.ok?
-        flash[:error] = "#{I18n.t("payment_could_not_process")}. #{result.error}"
+        flash.now[:error] = "#{I18n.t("payment_could_not_process")}. #{result.error}"
       end
       @order.reload
     end

From fd021d4778c85a0cfd64735fa7e6e4385b58ca60 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 12:28:55 +0100
Subject: [PATCH 09/17] Refactor payment intent status checking and return "ok"
 if the payment has already been processed successfully in a previous request

If the page is reloaded after the payment has already gone through, we can skip the processing and give a non-error response; the payment is already completed and Stripe's response confirms it was successful.
---
 app/services/process_payment_intent.rb | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb
index 775f20f399..e7b6534d28 100644
--- a/app/services/process_payment_intent.rb
+++ b/app/services/process_payment_intent.rb
@@ -31,6 +31,7 @@ class ProcessPaymentIntent
 
   def call!
     return Result.new(ok: false) unless valid?
+    return Result.new(ok: true) if already_processed?
 
     OrderWorkflow.new(order).next
 
@@ -56,10 +57,17 @@ class ProcessPaymentIntent
   end
 
   def ready_for_capture?
-    payment_intent_response = Stripe::PaymentIntentValidator.new.
-      call(payment_intent, stripe_account_id)
+    payment_intent_status == 'requires_capture'
+  end
 
-    payment_intent_response.status == 'requires_capture'
+  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 matches_last_payment?

From 05ed98aa0c85fcf43298e0acdc25b51f2e4b37b4 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 13:23:26 +0100
Subject: [PATCH 10/17] Use :order_with_totals in ProcessPaymentIntent

If we just use the :order factory here, it has no line items and no total, which means when we try to push it into complete state, the #requires_payment? check fails because the order total is zero, which means the call to #process_payments is ignored
---
 spec/services/process_payment_intent_spec.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/spec/services/process_payment_intent_spec.rb b/spec/services/process_payment_intent_spec.rb
index bcf76d6240..8c55e84f7a 100644
--- a/spec/services/process_payment_intent_spec.rb
+++ b/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,

From 5725535715b9271e922516a46a5f98dff0d50bc0 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 13:30:18 +0100
Subject: [PATCH 11/17] Fix payment state checking

---
 app/services/process_payment_intent.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb
index e7b6534d28..14fe5aee72 100644
--- a/app/services/process_payment_intent.rb
+++ b/app/services/process_payment_intent.rb
@@ -35,8 +35,7 @@ class ProcessPaymentIntent
 
     OrderWorkflow.new(order).next
 
-    if last_payment.can_complete?
-      last_payment.complete!
+    if last_payment.completed?
       last_payment.mark_as_processed
 
       Result.new(ok: true)

From 52e0d84238cf59f5c389f6e21fb5d9b6fb689a00 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 14:18:14 +0100
Subject: [PATCH 12/17] Reload in-memory payment before checking it's new state

The state should have changed in the database if the payment was processed successfully, and the memoized version here will not know that without a reload.
---
 app/services/process_payment_intent.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb
index 14fe5aee72..1aa25b0112 100644
--- a/app/services/process_payment_intent.rb
+++ b/app/services/process_payment_intent.rb
@@ -35,7 +35,7 @@ class ProcessPaymentIntent
 
     OrderWorkflow.new(order).next
 
-    if last_payment.completed?
+    if last_payment.reload.completed?
       last_payment.mark_as_processed
 
       Result.new(ok: true)

From 2e63cd811652c0870fbc7a9626b25ae8b6b94fd2 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 14:18:37 +0100
Subject: [PATCH 13/17] Add explanatory comment on payment processing flow

---
 app/services/process_payment_intent.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb
index 1aa25b0112..b69e0e0d9d 100644
--- a/app/services/process_payment_intent.rb
+++ b/app/services/process_payment_intent.rb
@@ -33,6 +33,8 @@ class ProcessPaymentIntent
     return Result.new(ok: false) unless valid?
     return Result.new(ok: true) if already_processed?
 
+    # Moves the order to competed state, which calls #process_payments! (and #purchase!)
+    # This completes the payment via Stripe and sets the payment's state to completed if successful
     OrderWorkflow.new(order).next
 
     if last_payment.reload.completed?

From 381b0e78d5533f07240a634f7a7ec1cb7da248c1 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 14:19:42 +0100
Subject: [PATCH 14/17] Improve ProcessPaymentIntent specs

---
 spec/services/process_payment_intent_spec.rb | 56 ++++++++++++++++----
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/spec/services/process_payment_intent_spec.rb b/spec/services/process_payment_intent_spec.rb
index 8c55e84f7a..04d98b2a60 100644
--- a/spec/services/process_payment_intent_spec.rb
+++ b/spec/services/process_payment_intent_spec.rb
@@ -76,21 +76,59 @@ describe ProcessPaymentIntent do
       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
 
@@ -122,7 +160,7 @@ describe ProcessPaymentIntent do
       let(:service) { ProcessPaymentIntent.new(intent, order, payment) }
 
       before do
-        allow(payment).to receive(:can_complete?).and_return(false)
+        allow(order).to receive(:process_payments!) { nil }
         allow(validator).to receive(:call).with(intent, anything).and_return(intent_response)
       end
 

From 8429da3d2ae8a2355bc6c47c1911be0795a03f01 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 22:19:01 +0100
Subject: [PATCH 15/17] Remove unused argument from ProcessPaymentIntent

---
 app/services/process_payment_intent.rb       | 4 ++--
 spec/services/process_payment_intent_spec.rb | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb
index b69e0e0d9d..b0a2716821 100644
--- a/app/services/process_payment_intent.rb
+++ b/app/services/process_payment_intent.rb
@@ -23,10 +23,10 @@ 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
+    @last_payment = OrderPaymentFinder.new(order).last_payment
   end
 
   def call!
diff --git a/spec/services/process_payment_intent_spec.rb b/spec/services/process_payment_intent_spec.rb
index 04d98b2a60..13b788395e 100644
--- a/spec/services/process_payment_intent_spec.rb
+++ b/spec/services/process_payment_intent_spec.rb
@@ -157,7 +157,7 @@ describe ProcessPaymentIntent do
     context "when the payment can't be completed" do
       let(:intent) { "pi_123" }
       let(:intent_response) { double(id: "pi_123", status: "requires_capture") }
-      let(:service) { ProcessPaymentIntent.new(intent, order, payment) }
+      let(:service) { ProcessPaymentIntent.new(intent, order) }
 
       before do
         allow(order).to receive(:process_payments!) { nil }

From a2862e604c7f54c0e28b12357b5ee02eaa847a3f Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 22:48:02 +0100
Subject: [PATCH 16/17] Find relevant payment that matches the payment intent,
 not the last payment

---
 app/models/spree/payment.rb            |  1 +
 app/services/process_payment_intent.rb | 20 ++++++--------------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb
index 91fa765b1e..1151d94e92 100644
--- a/app/models/spree/payment.rb
+++ b/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
diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb
index b0a2716821..44013ab12a 100644
--- a/app/services/process_payment_intent.rb
+++ b/app/services/process_payment_intent.rb
@@ -26,19 +26,19 @@ class ProcessPaymentIntent
   def initialize(payment_intent, order)
     @payment_intent = payment_intent
     @order = order
-    @last_payment = OrderPaymentFinder.new(order).last_payment
+    @payment = order.payments.pending.with_payment_intent(payment_intent).first
   end
 
   def call!
-    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?
 
     # Moves the order to competed state, which calls #process_payments! (and #purchase!)
     # This completes the payment via Stripe and sets the payment's state to completed if successful
     OrderWorkflow.new(order).next
 
-    if last_payment.reload.completed?
-      last_payment.mark_as_processed
+    if payment.reload.completed?
+      payment.mark_as_processed
 
       Result.new(ok: true)
     else
@@ -51,11 +51,7 @@ class ProcessPaymentIntent
 
   private
 
-  attr_reader :order, :payment_intent, :last_payment
-
-  def valid?
-    order.present? && matches_last_payment? && ready_for_capture?
-  end
+  attr_reader :order, :payment_intent, :payment
 
   def ready_for_capture?
     payment_intent_status == 'requires_capture'
@@ -71,15 +67,11 @@ class ProcessPaymentIntent
       status
   end
 
-  def matches_last_payment?
-    last_payment&.state == "pending" && last_payment&.response_code == payment_intent
-  end
-
   def stripe_account_id
     StripeAccount.find_by(enterprise_id: preferred_enterprise_id).stripe_user_id
   end
 
   def preferred_enterprise_id
-    last_payment.payment_method.preferred_enterprise_id
+    payment.payment_method.preferred_enterprise_id
   end
 end

From 794216713aaf67a93be97e6f43a4f10727db9fb6 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sun, 16 May 2021 22:53:01 +0100
Subject: [PATCH 17/17] Ensure payment is processed if order is in completed
 state

---
 app/services/process_payment_intent.rb        | 13 +++++--
 .../spree/orders_controller_spec.rb           | 36 ++++++++++++++-----
 2 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb
index 44013ab12a..cd085c3391 100644
--- a/app/services/process_payment_intent.rb
+++ b/app/services/process_payment_intent.rb
@@ -33,9 +33,7 @@ class ProcessPaymentIntent
     return Result.new(ok: false) unless payment.present? && ready_for_capture?
     return Result.new(ok: true) if already_processed?
 
-    # Moves the order to competed state, which calls #process_payments! (and #purchase!)
-    # This completes the payment via Stripe and sets the payment's state to completed if successful
-    OrderWorkflow.new(order).next
+    process_payment
 
     if payment.reload.completed?
       payment.mark_as_processed
@@ -53,6 +51,15 @@ class ProcessPaymentIntent
 
   attr_reader :order, :payment_intent, :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 ready_for_capture?
     payment_intent_status == 'requires_capture'
   end
diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb
index ae41c46a8d..ab29573117 100644
--- a/spec/controllers/spree/orders_controller_spec.rb
+++ b/spec/controllers/spree/orders_controller_spec.rb
@@ -114,19 +114,39 @@ describe Spree::OrdersController, type: :controller do
             .and_return(payment_intent_response)
 
           allow(Spree::Order).to receive(:find_by!) { order }
+        end
 
-          allow(order).to receive(:process_payments!) do
-            payment.complete!
+        context "when the order is in payment state" do
+          it "completes 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
 
-        it "completes the payment" do
-          get :show, params: { id: order.number, payment_intent: payment_intent }
+        context "when the order is already completed" do
+          before do
+            order.update_columns(state: "complete")
+          end
 
-          expect(response.status).to eq 200
-          payment.reload
-          expect(payment.cvv_response_message).to be nil
-          expect(payment.state).to eq("completed")
+          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