Remove Spree::Shipment#address association.

CHANGELOG.md

@@ -1,5 +1,14 @@
 ## Solidus 1.4.0 (master, unreleased)
 
+*   Deprecate `Spree::Shipment#address` (column renamed)
+
+    `Spree::Shipment#address` was not actually being used for anything in
+    particular, so the association has been deprecated and delegated to
+    `Spree::Order#ship_address` instead. The database column has been renamed
+    `spree_shipments.deprecated_address_id`.
+
+    https://github.com/solidusio/solidus/pull/1138
+
 *   Coupon code application has been separated from the Continue button on the Payment checkout page
 
     * JavaScript for it has been moved from address.js into its own `spree/frontend/checkout/coupon-code`

api/spec/controllers/spree/api/checkouts_controller_spec.rb

@@ -136,7 +136,7 @@ module Spree
 
       it "can update shipping method and transition from delivery to payment" do
         order.update_column(:state, "delivery")
-        shipment = create(:shipment, order: order, address: order.ship_address)
+        shipment = create(:shipment, order: order)
         shipment.refresh_rates
         shipping_rate = shipment.shipping_rates.where(selected: false).first
         api_put :update, id: order.to_param, order_token: order.guest_token,

api/spec/controllers/spree/api/shipments_controller_spec.rb

@@ -2,7 +2,7 @@
 
 describe Spree::Api::ShipmentsController, type: :controller do
   render_views
-  let!(:shipment) { create(:shipment, address: create(:address), inventory_units: [build(:inventory_unit, shipment: nil)]) }
+  let!(:shipment) { create(:shipment, inventory_units: [build(:inventory_unit, shipment: nil)]) }
   let!(:attributes) { [:id, :tracking, :tracking_url, :number, :cost, :shipped_at, :stock_location_name, :order_id, :shipping_rates, :shipping_methods] }
 
   before do

core/app/models/spree/order_shipping.rb

@@ -17,7 +17,7 @@ def ship_shipment(shipment, external_number: nil, tracking_number: nil, suppress
     ship(
       inventory_units: shipment.inventory_units.shippable,
       stock_location: shipment.stock_location,
-      address: shipment.address,
+      address: shipment.order.ship_address,
       shipping_method: shipment.shipping_method,
       shipped_at: Time.current,
       external_number: external_number,

core/app/models/spree/shipment.rb

@@ -1,7 +1,6 @@
 module Spree
   class Shipment < Spree::Base
     belongs_to :order, class_name: 'Spree::Order', touch: true, inverse_of: :shipments
-    belongs_to :address, class_name: 'Spree::Address'
     belongs_to :stock_location, class_name: 'Spree::StockLocation'
 
     has_many :adjustments, as: :adjustable, inverse_of: :adjustable, dependent: :delete_all
@@ -21,7 +20,6 @@ class Shipment < Spree::Base
     # from outside of the state machine and can actually pass variables through.
     attr_accessor :special_instructions, :suppress_mailer
 
-    accepts_nested_attributes_for :address
     accepts_nested_attributes_for :inventory_units
 
     make_permalink field: :number, length: 11, prefix: 'H'
@@ -372,6 +370,11 @@ def requires_shipment?
       !stock_location || stock_location.fulfillable?
     end
 
+    def address
+      ActiveSupport::Deprecation.warn("Calling Shipment#address is deprecated. Use Order#ship_address instead", caller)
+      order.ship_address if order
+    end
+
     private
 
     def after_ship

core/app/models/spree/stock/estimator.rb

@@ -45,7 +45,7 @@ def calculate_shipping_rates(package)
 
       def shipping_methods(package)
         package.shipping_methods
-          .available_for_address(package.shipment.address)
+          .available_for_address(package.shipment.order.ship_address)
           .includes(:calculator, tax_category: :tax_rates)
           .to_a
           .select do |ship_method|

core/app/models/spree/stock/package.rb

@@ -123,7 +123,6 @@ def to_shipment
 
         Spree::Shipment.new(
           order: order,
-          address: order.ship_address,
           stock_location: stock_location,
           inventory_units: contents.map(&:inventory_unit)
         )

core/db/migrate/20160527070401_rename_shipment_address_field.rb

@@ -0,0 +1,7 @@
+class RenameShipmentAddressField < ActiveRecord::Migration
+  def change
+    change_table :spree_shipments do |t|
+      t.rename :address_id, :deprecated_address_id
+    end
+  end
+end

core/lib/spree/testing_support/factories/order_factory.rb

@@ -46,7 +46,7 @@
         end
         order.line_items.reload
 
-        create(:shipment, order: order, cost: evaluator.shipment_cost, shipping_method: evaluator.shipping_method, address: evaluator.ship_address, stock_location: evaluator.stock_location)
+        create(:shipment, order: order, cost: evaluator.shipment_cost, shipping_method: evaluator.shipping_method, stock_location: evaluator.stock_location)
         order.shipments.reload
 
         order.update!
@@ -94,7 +94,7 @@
                 next unless evaluator.with_cartons
                 Spree::Carton.create!(
                   stock_location: shipment.stock_location,
-                  address: shipment.address,
+                  address: order.ship_address,
                   shipping_method: shipment.shipping_method,
                   inventory_units: shipment.inventory_units,
                   shipped_at: Time.current

core/lib/tasks/migrations/copy_shipped_shipments_to_cartons.rake

@@ -49,13 +49,15 @@ namespace 'spree:migrations:copy_shipped_shipments_to_cartons' do
               #{db_concat("'C'", 'spree_shipments.number')}, -- number
               spree_shipments.id, -- imported_from_shipment_id
               spree_shipments.stock_location_id,
-              spree_shipments.address_id,
+              spree_orders.ship_address_id,
               spree_shipping_rates.shipping_method_id,
               spree_shipments.tracking,
               spree_shipments.shipped_at,
               '#{Time.current.to_s(:db)}', -- created_at
               '#{Time.current.to_s(:db)}' -- updated_at
             from spree_shipments
+            left join spree_orders
+              on spree_orders.id = spree_shipments.order_id
             left join spree_shipping_rates
               on spree_shipping_rates.shipment_id = spree_shipments.id
               and spree_shipping_rates.selected = #{Spree::Carton.connection.quoted_true}

core/spec/lib/tasks/migrations/copy_shipped_shipments_to_cartons_spec.rb

@@ -49,7 +49,7 @@
 
       expect(carton.number).to eq "C#{shipped_shipment.number}"
       expect(carton.stock_location).to eq shipped_shipment.stock_location
-      expect(carton.address).to eq shipped_shipment.address
+      expect(carton.address).to eq shipped_shipment.order.ship_address
       expect(carton.shipping_method).to eq shipped_shipment.shipping_method
       expect(carton.tracking).to eq shipped_shipment.tracking
       expect(carton.shipped_at).to eq shipped_shipment.shipped_at

core/spec/models/spree/order_shipping_spec.rb

@@ -57,7 +57,7 @@ def emails
     let(:shipment) { order.shipments.to_a.first }
     let(:inventory_units) { shipment.inventory_units }
     let(:stock_location) { shipment.stock_location }
-    let(:address) { shipment.address }
+    let(:address) { order.ship_address }
     let(:shipping_method) { shipment.shipping_method }
 
     it_behaves_like 'shipment shipping'
@@ -190,7 +190,7 @@ def emails
         order.shipping.ship(
           inventory_units: shipped_inventory,
           stock_location: shipment.stock_location,
-          address: shipment.address,
+          address: order.ship_address,
           shipping_method: shipment.shipping_method
         )
       end
@@ -223,8 +223,7 @@ def emails
       let(:shipment) do
         FactoryGirl.create(
           :shipment,
-          order: order,
-          address: FactoryGirl.create(:address)
+          order: order
         )
       end
 

core/spec/models/spree/shipment_spec.rb

@@ -9,7 +9,6 @@
     order.shipments.create!(
       state: 'pending',
       cost: 1,
-      address: order.ship_address,
       inventory_units: order.inventory_units,
       shipping_rates: [
         Spree::ShippingRate.new(
@@ -454,7 +453,7 @@
     context "when the shipment is canceled" do
       let(:address){ create(:address) }
       let(:order){ create(:order_with_line_items, ship_address: address) }
-      let(:shipment_with_inventory_units) { create(:shipment, order: order, address: address, state: 'canceled') }
+      let(:shipment_with_inventory_units) { create(:shipment, order: order, state: 'canceled') }
       let(:subject) { shipment_with_inventory_units.ship! }
       before do
         allow(order).to receive(:update!)
@@ -682,7 +681,6 @@
     let(:unshippable_shipment) do
       create(
         :shipment,
-        address: create(:address),
         stock_location: stock_location,
         inventory_units: [build(:inventory_unit)]
       )

core/spec/models/spree/stock/coordinator_spec.rb

@@ -26,11 +26,6 @@ module Stock
           expect(subject.shipments.size).to eq(1)
         end
 
-        it "puts the order's ship address on the shipments" do
-          shipments = subject.shipments
-          expect(shipments.map(&:address)).to eq [order.ship_address]
-        end
-
         it "builds a shipment for all active stock locations" do
           subject.shipments.count == StockLocation.count
         end

core/spec/models/spree/stock/package_spec.rb

@@ -113,7 +113,6 @@ def build_inventory_unit
         expect(last_unit.state).to eq 'backordered'
 
         expect(shipment.shipping_method).to eq shipping_method
-        expect(shipment.address).to eq order.ship_address
       end
 
       it 'does not add an inventory unit to a package twice' do