diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index c1fe12ce53..71e9239523 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -152,7 +152,7 @@ class CheckoutController < Spree::CheckoutController end def update_failed - clear_ship_address + current_order.updater.shipping_address_from_distributor RestartCheckout.new(@order).call respond_to do |format| @@ -165,15 +165,6 @@ class CheckoutController < Spree::CheckoutController end end - # When we have a pickup Shipping Method, - # we clone the distributor address into ship_address before_save - # We don't want this data in the form, so we clear it out - def clear_ship_address - unless current_order.shipping_method.andand.require_ship_address - current_order.ship_address = Spree::Address.default - end - end - def load_order @order = current_order redirect_to(main_app.shop_path) && return unless @order && @order.checkout_allowed? diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index ac7f180e2d..b67a23b8ac 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -78,30 +78,19 @@ describe CheckoutController, type: :controller do allow(controller).to receive(:current_order).and_return(order) end - it "does not clone the ship address from distributor when shipping method requires address" do - get :edit - expect(assigns[:order].ship_address.address1).to be_nil - end - - it "clears the ship address when re-rendering edit" do - expect(controller).to receive(:clear_ship_address).and_return true + it "set shipping_address_from_distributor when re-rendering edit" do + expect(order.updater).to receive(:shipping_address_from_distributor) allow(order).to receive(:update_attributes).and_return false spree_post :update, format: :json, order: {} end - it "clears the ship address when the order state cannot be advanced" do - expect(controller).to receive(:clear_ship_address).and_return true + it "set shipping_address_from_distributor when the order state cannot be advanced" do + expect(order.updater).to receive(:shipping_address_from_distributor) allow(order).to receive(:update_attributes).and_return true allow(order).to receive(:next).and_return false spree_post :update, format: :json, order: {} end - it "only clears the ship address with a pickup shipping method" do - allow(order).to receive_message_chain(:shipping_method, :andand, :require_ship_address).and_return false - expect(order).to receive(:ship_address=) - controller.send(:clear_ship_address) - end - context "#update with shipping_method_id" do let(:test_shipping_method_id) { "111" } @@ -266,10 +255,11 @@ describe CheckoutController, type: :controller do before do controller.instance_variable_set(:@order, order) allow(RestartCheckout).to receive(:new) { restart_checkout } + allow(controller).to receive(:current_order) { order } end - it "clears the shipping address and restarts the checkout" do - expect(controller).to receive(:clear_ship_address) + it "set shipping_address_from_distributor and restarts the checkout" do + expect(order.updater).to receive(:shipping_address_from_distributor) expect(restart_checkout).to receive(:call) expect(controller).to receive(:respond_to) diff --git a/spec/services/restart_checkout_spec.rb b/spec/services/restart_checkout_spec.rb index 2413408994..f0514c9b5f 100644 --- a/spec/services/restart_checkout_spec.rb +++ b/spec/services/restart_checkout_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe RestartCheckout do - let(:order) { create(:order) } + let(:order) { create(:order_with_distributor) } describe "#call" do context "when the order is already in the 'cart' state" do @@ -12,6 +12,7 @@ describe RestartCheckout do end context "when the order is in a subsequent state" do + # 'pending' is the only shipment state possible for incomplete orders let!(:shipment_pending) { create(:shipment, order: order, state: 'pending') } let!(:payment_failed) { create(:payment, order: order, state: 'failed') } let!(:payment_checkout) { create(:payment, order: order, state: 'checkout') } @@ -20,12 +21,43 @@ describe RestartCheckout do order.update_attribute(:state, "payment") end - # NOTE: at the time of writing, it was not possible to create a shipment - # with a state other than 'pending' when the order has not been - # completed, so this is not a case that requires testing. - it "resets the order state, and clears incomplete shipments and payments" do - RestartCheckout.new(order).call + context "when order ship address is nil" do + before { order.ship_address = nil } + it "resets the order state, and clears incomplete shipments and payments" do + RestartCheckout.new(order).call + + expect_cart_state_and_reset_adjustments + end + end + + context "when order ship address is not empty" do + before { order.ship_address = order.address_from_distributor } + + it "resets the order state, and clears incomplete shipments and payments" do + RestartCheckout.new(order).call + + expect_cart_state_and_reset_adjustments + end + end + + context "when order ship address is empty" do + before { order.ship_address = Spree::Address.default } + + it "does not reset the order state nor clears incomplete shipments and payments" do + expect do + RestartCheckout.new(order).call + end.to raise_error(StateMachine::InvalidTransition) + + expect(order.state).to eq 'payment' + expect(order.shipments.count).to eq 1 + expect(order.adjustments.shipping.count).to eq 0 + expect(order.payments.count).to eq 2 + expect(order.adjustments.payment_fee.count).to eq 2 + end + end + + def expect_cart_state_and_reset_adjustments expect(order.state).to eq 'cart' expect(order.shipments.count).to eq 0 expect(order.adjustments.shipping.count).to eq 0