diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb index 43b0b586c0..98ea4ac982 100644 --- a/app/models/spree/shipment.rb +++ b/app/models/spree/shipment.rb @@ -108,19 +108,35 @@ module Spree # The call to Stock::Estimator below will replace the current shipping_method original_shipping_method_id = shipping_method.try(:id) - self.shipping_rates = OrderManagement::Stock::Estimator.new(order).shipping_rates(to_package) - if original_shipping_method_id - selected_rate = shipping_rates.detect { |rate| - rate.shipping_method_id == original_shipping_method_id - } - self.selected_shipping_rate_id = selected_rate.id if selected_rate - end + keep_original_shipping_method_selection(original_shipping_method_id) shipping_rates end + def keep_original_shipping_method_selection(original_shipping_method_id) + return if shipping_method&.id == original_shipping_method_id + + rate_for_original_shipping_method = find_shipping_rate_for(original_shipping_method_id) + if rate_for_original_shipping_method.present? + self.selected_shipping_rate_id = rate_for_original_shipping_method.id + else + # If there's no original ship method to keep, or if it cannot be found on the ship rates + # But there's a new ship method selected (first if clause in this method) + # We need to save the shipment so that callbacks are triggered + save! + end + end + + def find_shipping_rate_for(shipping_method_id) + return unless shipping_method_id + + shipping_rates.detect { |rate| + rate.shipping_method_id == shipping_method_id + } + end + def currency order ? order.currency : Spree::Config[:currency] end diff --git a/spec/models/spree/shipment_spec.rb b/spec/models/spree/shipment_spec.rb index 457df53a6d..30f088133c 100644 --- a/spec/models/spree/shipment_spec.rb +++ b/spec/models/spree/shipment_spec.rb @@ -126,7 +126,8 @@ describe Spree::Shipment do it 'should request new rates, and maintain shipping_method selection' do OrderManagement::Stock::Estimator.should_receive(:new).with(shipment.order).and_return(mock_estimator) - shipment.stub(shipping_method: shipping_method2) + # The first call is for the original shippping method, the second call is after the Estimator was executed + allow(shipment).to receive(:shipping_method).and_return(shipping_method2, shipping_method1) expect(shipment.refresh_rates).to eq shipping_rates expect(shipment.reload.selected_shipping_rate.shipping_method_id).to eq shipping_method2.id