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