From c8dbf4c6f053635647dc0b0adc7e5357d6c3b93c Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 17 Feb 2026 10:52:18 +1100
Subject: [PATCH] Add error handling around payment with credit

For now we log error but don't report it to the user, so they can
proceed through the checkout.
---
 app/controllers/checkout_controller.rb       | 27 ++++++++--
 spec/controllers/checkout_controller_spec.rb | 54 +++++++++++++++++++-
 2 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb
index 3279fb00e7..827d9f566c 100644
--- a/app/controllers/checkout_controller.rb
+++ b/app/controllers/checkout_controller.rb
@@ -127,9 +127,10 @@ class CheckoutController < BaseController
 
     @order.select_shipping_method(params[:shipping_method_id])
 
+    @order.update(order_params)
+
     add_payment_with_credit if credit_available? && details_step?
 
-    @order.update(order_params)
     # We need to update voucher to take into account:
     #  * when moving away from "details" step : potential change in shipping method fees
     #  * when moving away from "payment" step : payment fees
@@ -156,14 +157,27 @@ class CheckoutController < BaseController
   def add_payment_with_credit
     credit_payment_method = @order.distributor.payment_methods.customer_credit
 
+    if credit_payment_method.nil?
+      error_message = "Customer credit payment method is missing, please check configuration"
+      log_error(error_message)
+      return
+    end
+
     return if @order.payments.where(payment_method: credit_payment_method).exists?
 
-    amount = [available_credit, @order.total].min
-
-    ActiveRecord::Base.transaction do
+    # we are already in a transaction because the order is locked, so we force creating a new one
+    # to make sure the rollback works as expected :
+    # https://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Nested+transactions
+    ActiveRecord::Base.transaction(requires_new: true) do
+      amount = [available_credit, @order.total].min
       payment = @order.payments.create!(payment_method: credit_payment_method, amount:)
       payment.internal_purchase!
     end
+  rescue StandardError => e
+    # Even though the transaction rolled back, the order still have a payment in memory,
+    # so we reload the payments so the payment doesn't get saved later on
+    @order.payments.reload
+    log_error(e)
   end
 
   def available_credit
@@ -194,4 +208,9 @@ class CheckoutController < BaseController
 
     @order.back_to_address
   end
+
+  def log_error(error)
+    Rails.logger.error("CheckoutController: #{error}")
+    Alert.raise(error)
+  end
 end
diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb
index 09227afc33..2c79faf1ef 100644
--- a/spec/controllers/checkout_controller_spec.rb
+++ b/spec/controllers/checkout_controller_spec.rb
@@ -308,7 +308,7 @@ RSpec.describe CheckoutController do
           end
         end
 
-        context "with credit availablle" do
+        context "with credit available" do
           let(:checkout_params) do
             {
               order: {
@@ -390,7 +390,57 @@ RSpec.describe CheckoutController do
             end
           end
 
-          # TODO cover error scenarios here
+          context "when payment creation fails" do
+            before do
+              # Add credit
+              create(
+                :customer_account_transaction,
+                amount: 5.00,
+                customer: order.customer,
+                payment_method: credit_payment_method
+              )
+              allow_any_instance_of(Spree::Payment).to receive(:internal_purchase!)
+                .and_raise(Spree::Core::GatewayError)
+            end
+
+            it "logs the error" do
+              expect(Alert).to receive(:raise).with(Spree::Core::GatewayError)
+              put(:update, params:)
+            end
+
+            it "doesn't create a credit payment" do
+              put(:update, params:)
+
+              credit_payment = order.payments.find_by(payment_method: credit_payment_method)
+              expect(credit_payment).to be_nil
+            end
+          end
+
+          context "when credit payment method is missing" do
+            before do
+              # Add credit
+              create(
+                :customer_account_transaction,
+                amount: 5.00,
+                customer: order.customer,
+                payment_method: credit_payment_method
+              )
+              credit_payment_method.destroy!
+            end
+
+            it "logs the error" do
+              expect(Alert).to receive(:raise).with(
+                "Customer credit payment method is missing, please check configuration"
+              )
+              put(:update, params:)
+            end
+
+            it "doesn't create a credit payment" do
+              put(:update, params:)
+
+              expect(order.payments).to be_empty
+            end
+          end
         end
       end
     end