Move shipping_address_from_distributor to OrderUpdater

This is still Spree-1.3 compatible not considering multiple shipments
and shipping methods.

app/models/order_updater.rb

@@ -31,6 +31,10 @@ class OrderUpdater < SimpleDelegator
     order.payment_state
   end
 
+  def before_save_hook
+    shipping_address_from_distributor
+  end
+
   private
 
   # Taken from order.outstanding_balance in Spree 2.4
@@ -38,4 +42,11 @@ class OrderUpdater < SimpleDelegator
   def canceled_and_paid_for?
     order.canceled? && order.payments.present? && !order.payments.completed.empty?
   end
+
+  def shipping_address_from_distributor
+    shipping_method = order.shipments.first.shipping_methods.first
+    return if shipping_method.require_ship_address
+
+    order.ship_address = order.distributor.address
+  end
 end

app/models/spree/order_decorator.rb

@@ -20,7 +20,6 @@ Spree::Order.class_eval do
   validate :disallow_guest_order
   attr_accessible :order_cycle_id, :distributor_id, :customer_id
 
-  before_validation :shipping_address_from_distributor
   before_validation :associate_customer, unless: :customer_id?
   before_validation :ensure_customer, unless: :customer_is_valid?
 
@@ -337,18 +336,6 @@ Spree::Order.class_eval do
 
   private
 
-  def shipping_address_from_distributor
-    if distributor
-      # This method is confusing to conform to the vagaries of the multi-step checkout
-      # We copy over the shipping address when we have no shipping method selected
-      # We can refactor this when we drop the multi-step checkout option
-      #
-      if shipping_method.andand.require_ship_address == false
-        self.ship_address = address_from_distributor
-      end
-    end
-  end
-
   def address_from_distributor
     address = distributor.address.clone
     if bill_address

spec/models/order_updater_spec.rb

@@ -1,10 +1,10 @@
 require 'spec_helper'
 
 describe OrderUpdater do
-  context "#updating_payment_state" do
-    let(:order) { build(:order) }
-    let(:order_updater) { described_class.new(Spree::OrderUpdater.new(order)) }
+  let(:order) { build(:order) }
+  let(:order_updater) { described_class.new(Spree::OrderUpdater.new(order)) }
 
+  context "#updating_payment_state" do
     it "is failed if no valid payments" do
       allow(order).to receive_message_chain(:payments, :valid, :empty?) { true }
 
@@ -98,4 +98,36 @@ describe OrderUpdater do
       end
     end
   end
+
+  context '#before_save_hook' do
+    let(:distributor) { build(:distributor_enterprise) }
+    let(:shipment) { build(:shipment) }
+    let(:order) { build(:order, distributor: distributor) }
+
+    before do
+      shipment.shipping_methods << shipping_method
+      order.shipments << shipment
+    end
+
+    context 'when the shipping method doesn\'t require a delivery address' do
+      let(:shipping_method) { build(:base_shipping_method, require_ship_address: false) }
+
+      it "populates the shipping address" do
+        order_updater.before_save_hook
+        expect(order.ship_address.firstname).to eq(distributor.address.firstname)
+      end
+    end
+
+    context 'when the shipping method requires a delivery address' do
+      let(:shipping_method) { build(:base_shipping_method, require_ship_address: true) }
+      let(:address) { build(:address, firstname: 'will') }
+
+      before { order.ship_address = address }
+
+      it "does not populate the shipping address" do
+        order_updater.before_save_hook
+        expect(order.ship_address.firstname).to eq("will")
+      end
+    end
+  end
 end

spec/models/spree/order_spec.rb

@@ -509,41 +509,6 @@ describe Spree::Order do
     end
   end
 
-  describe "shipping address prepopulation" do
-    let(:distributor) { create(:distributor_enterprise) }
-    let(:order) { build(:order, distributor: distributor) }
-
-    before do
-      order.ship_address = distributor.address.clone
-      order.save # just to trigger our autopopulate the first time ;)
-    end
-
-    it "autopopulates the shipping address on save" do
-      order.should_receive(:shipping_address_from_distributor).and_return true
-      order.save
-    end
-
-    it "populates the shipping address if the shipping method doesn't require a delivery address" do
-      order.shipping_method = create(:shipping_method, require_ship_address: false)
-      order.ship_address.update_attribute :firstname, "will"
-      order.save
-      order.ship_address.firstname.should == distributor.address.firstname
-    end
-
-    it "does not populate the shipping address if the shipping method requires a delivery address" do
-      order.shipping_method = create(:shipping_method, require_ship_address: true)
-      order.ship_address.update_attribute :firstname, "will"
-      order.save
-      order.ship_address.firstname.should == "will"
-    end
-
-    it "doesn't attempt to create a shipment if the order is not yet valid" do
-      order.shipping_method = create(:shipping_method, require_ship_address: false)
-      #Shipment.should_not_r
-      order.create_shipment!
-    end
-  end
-
   describe "checking if an order is an account invoice" do
     let(:accounts_distributor)  { create(:distributor_enterprise) }
     let(:order_account_invoice) { create(:order, distributor: accounts_distributor) }