Merge pull request #4252 from luisramos0/remove_clear_ship_address

On checkout of pickup orders, set ship_address to shipping_address_from_distributor instead of empty Spree::Address.default

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?

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)
 

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