From 33de80f13ccb3f9e10411c43a3de5589445dc3d0 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Fri, 27 Oct 2023 14:20:31 +1100 Subject: [PATCH] Fix checking if shipping method changed We now check if the shipping method changed before we actually select it. Fix the related spec.The spec was wrong because order.select_shipping_method fails silently. That means the shipping method wasn't getting updated on the order, thus the test was passing. --- app/controllers/split_checkout_controller.rb | 10 +++++++--- spec/controllers/split_checkout_controller_spec.rb | 7 +++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 17d7f36dbd..9bcd6a709e 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -98,21 +98,25 @@ class SplitCheckoutController < ::BaseController def update_order return if params[:confirm_order] || @order.errors.any? + # Checking if shipping method updated before @order get updated. We can't use this guard + # clause in recalculate_voucher as by then the @order.shipping method would be the new one + shipping_method_updated = @order.shipping_method&.id != params[:shipping_method_id].to_i + @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? + recalculate_voucher(shipping_method_updated) if details_step? || payment_step? @order.update_totals_and_states validate_current_step end - def recalculate_voucher + def recalculate_voucher(shipping_method_updated) return if @order.voucher_adjustments.empty? - return if @order.shipping_method&.id == params[:shipping_method_id].to_i + return unless shipping_method_updated VoucherAdjustmentsService.new(@order).update end diff --git a/spec/controllers/split_checkout_controller_spec.rb b/spec/controllers/split_checkout_controller_spec.rb index e6aee5c82e..349b4f71e0 100644 --- a/spec/controllers/split_checkout_controller_spec.rb +++ b/spec/controllers/split_checkout_controller_spec.rb @@ -216,6 +216,13 @@ describe SplitCheckoutController, type: :controller do end let(:new_shipping_method) { create(:shipping_method, distributors: [distributor]) } + before do + # Add a shipping rates for the new shipping method to prevent + # order.select_shipping_method from failing + order.shipment.shipping_rates << + Spree::ShippingRate.create(shipping_method: new_shipping_method, selected: true) + end + it "recalculates the voucher adjustment" do expect(service).to receive(:update)