From 5ba21e486a60bb325dd52286cc783c534b7f2117 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Fri, 20 Oct 2023 15:29:29 +1100
Subject: [PATCH] Recalculate voucher voucher adjusment when needed

We need to recalculate the voucher adjustment(s) in the following
scenarii :
When a voucher as been added to the order and :
  * Moving to the payment step from details step, to take into account
    potential change in shipment fees. (this happen if we return to the
    details step after reaching the summary step)
  * Moving to the summary step from payment step, to take into account
    payment fees
---
 app/controllers/split_checkout_controller.rb  | 12 +++
 app/models/spree/order/checkout.rb            |  5 --
 .../split_checkout_controller_spec.rb         | 75 +++++++++++++++++++
 3 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb
index a855774874..67d52c496a 100644
--- a/app/controllers/split_checkout_controller.rb
+++ b/app/controllers/split_checkout_controller.rb
@@ -98,11 +98,23 @@ class SplitCheckoutController < ::BaseController
 
     @order.select_shipping_method(params[:shipping_method_id])
     @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
+    recalculate_voucher if details_step? || payment_step?
     @order.update_totals_and_states
 
     validate_current_step
   end
 
+  def recalculate_voucher
+    return if @order.voucher_adjustments.empty?
+
+    return if @order.shipment.shipping_method.id == params[:shipping_method_id].to_i
+
+    VoucherAdjustmentsService.new(@order).update
+  end
+
   def validate_current_step
     Checkout::Validation.new(@order, params).call && @order.errors.empty?
   end
diff --git a/app/models/spree/order/checkout.rb b/app/models/spree/order/checkout.rb
index 6bd9a5b919..523e366f28 100644
--- a/app/models/spree/order/checkout.rb
+++ b/app/models/spree/order/checkout.rb
@@ -90,11 +90,6 @@ module Spree
                 order.update_totals_and_states
               end
 
-              after_transition to: :confirmation do |order|
-                VoucherAdjustmentsService.new(order).update
-                order.update_totals_and_states
-              end
-
               after_transition to: :complete, do: :finalize!
               after_transition to: :resumed,  do: :after_resume
               after_transition to: :canceled, do: :after_cancel
diff --git a/spec/controllers/split_checkout_controller_spec.rb b/spec/controllers/split_checkout_controller_spec.rb
index afa3d89274..2d45f1b661 100644
--- a/spec/controllers/split_checkout_controller_spec.rb
+++ b/spec/controllers/split_checkout_controller_spec.rb
@@ -172,6 +172,56 @@ describe SplitCheckoutController, type: :controller do
             expect(order.user.ship_address).to eq(order.ship_address)
           end
         end
+
+        describe "with a voucher" do
+          let(:checkout_params) do
+            {
+              order: {
+                email: user.email,
+                bill_address_attributes: address.to_param,
+                ship_address_attributes: address.to_param
+              },
+              shipping_method_id: order.shipment.shipping_method.id.to_s
+            }
+          end
+
+          let(:voucher) { create(:voucher_flat_rate, enterprise: distributor) }
+          let(:service) { mock_voucher_adjustment_service }
+
+          before do
+            voucher.create_adjustment(voucher.code, order)
+          end
+
+          it "doesn't recalculate the voucher adjustment" do
+            expect(service).to_not receive(:update)
+
+            put(:update, params:)
+
+            expect(response).to redirect_to checkout_step_path(:payment)
+          end
+
+          context "when updating shipping method" do
+            let(:checkout_params) do
+              {
+                order: {
+                  email: user.email,
+                  bill_address_attributes: address.to_param,
+                  ship_address_attributes: address.to_param
+                },
+                shipping_method_id: new_shipping_method.id.to_s
+              }
+            end
+            let(:new_shipping_method) { create(:shipping_method, distributors: [distributor]) }
+
+            it "recalculates the voucher adjustment" do
+              expect(service).to receive(:update)
+
+              put(:update, params:)
+
+              expect(response).to redirect_to checkout_step_path(:payment)
+            end
+          end
+        end
       end
     end
 
@@ -214,6 +264,24 @@ describe SplitCheckoutController, type: :controller do
           expect(response).to redirect_to checkout_step_path(:summary)
           expect(order.reload.state).to eq "confirmation"
         end
+
+        describe "with a voucher" do
+          let(:voucher) { create(:voucher_flat_rate, enterprise: distributor) }
+
+          before do
+            voucher.create_adjustment(voucher.code, order)
+          end
+
+          # so we need to recalculate voucher to account for payment fees
+          it "recalculates the voucher adjustment" do
+            service = mock_voucher_adjustment_service
+            expect(service).to receive(:update)
+
+            put(:update, params:)
+
+            expect(response).to redirect_to checkout_step_path(:summary)
+          end
+        end
       end
 
       context "with payment fees" do
@@ -399,4 +467,11 @@ describe SplitCheckoutController, type: :controller do
     it_behaves_like "handling stock issues", "payment"
     it_behaves_like "handling stock issues", "summary"
   end
+
+  def mock_voucher_adjustment_service
+    voucher_adjustment_service = instance_double(VoucherAdjustmentsService)
+    allow(VoucherAdjustmentsService).to receive(:new).and_return(voucher_adjustment_service)
+
+    voucher_adjustment_service
+  end
 end