Remove order association from inventory units

core/app/models/spree/inventory_unit.rb

@@ -7,7 +7,6 @@ class InventoryUnit < Spree::Base
     CANCELABLE_STATES = ['on_hand', 'backordered', 'shipped']
 
     belongs_to :variant, -> { with_deleted }, class_name: "Spree::Variant", inverse_of: :inventory_units
-    belongs_to :order, class_name: "Spree::Order", inverse_of: :inventory_units
     belongs_to :shipment, class_name: "Spree::Shipment", touch: true, inverse_of: :inventory_units
     belongs_to :return_authorization, class_name: "Spree::ReturnAuthorization", inverse_of: :inventory_units
     belongs_to :carton, class_name: "Spree::Carton", inverse_of: :inventory_units
@@ -16,8 +15,15 @@ class InventoryUnit < Spree::Base
     has_many :return_items, inverse_of: :inventory_unit, dependent: :destroy
     has_one :original_return_item, class_name: "Spree::ReturnItem", foreign_key: :exchange_inventory_unit_id, dependent: :destroy
     has_one :unit_cancel, class_name: "Spree::UnitCancel"
+    has_one :order, through: :shipment
 
-    validates_presence_of :order, :shipment, :line_item, :variant
+    delegate :order_id, to: :shipment
+
+    def order=(_)
+      raise "The order association has been removed from InventoryUnit. The order is now determined from the shipment."
+    end
+
+    validates_presence_of :shipment, :line_item, :variant
 
     before_destroy :ensure_can_destroy
 

core/app/models/spree/order.rb

@@ -73,13 +73,16 @@ class CannotRebuildShipments < StandardError; end
     has_many :products, through: :variants
 
     # Shipping
-    has_many :inventory_units, inverse_of: :order
-    has_many :cartons, -> { distinct }, through: :inventory_units
     has_many :shipments, dependent: :destroy, inverse_of: :order do
       def states
         pluck(:state).uniq
       end
     end
+    has_many :inventory_units, through: :shipments
+    has_many :cartons, -> { distinct }, through: :inventory_units
+
+    has_many :order_stock_locations, class_name: "Spree::OrderStockLocation"
+    has_many :stock_locations, through: :order_stock_locations
 
     # Adjustments and promotions
     has_many :adjustments, -> { order(:created_at) }, as: :adjustable, inverse_of: :adjustable, dependent: :destroy

core/app/models/spree/return_item.rb

@@ -184,7 +184,7 @@ def build_exchange_inventory_unit
       # for pricing information for if the inventory unit is
       # ever returned. This means that the inventory unit's line_item
       # will have a different variant than the inventory unit itself
-      super(variant: exchange_variant, line_item: inventory_unit.line_item, order: inventory_unit.order) if exchange_required?
+      super(variant: exchange_variant, line_item: inventory_unit.line_item) if exchange_required?
     end
 
     # @return [Spree::Shipment, nil] the exchange inventory unit's shipment if it exists

core/app/models/spree/shipment.rb

@@ -262,11 +262,10 @@ def determine_state(order)
       end
     end
 
-    def set_up_inventory(state, variant, order, line_item)
+    def set_up_inventory(state, variant, _order, line_item)
       inventory_units.create(
         state: state,
         variant_id: variant.id,
-        order_id: order.id,
         line_item_id: line_item.id
       )
     end

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

@@ -11,8 +11,7 @@ def units
             Spree::InventoryUnit.new(
               pending: true,
               variant: line_item.variant,
-              line_item: line_item,
-              order: @order
+              line_item: line_item
             )
           end
         end

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

@@ -44,7 +44,7 @@ def remove(inventory_unit)
       def order
         # Fix regression that removed package.order.
         # Find it dynamically through an inventory_unit.
-        contents.detect { |item| !!item.try(:inventory_unit).try(:order) }.try(:inventory_unit).try(:order)
+        contents.detect { |item| !!item.try(:line_item).try(:order) }.try(:line_item).try(:order)
       end
 
       # @return [Float] the summed weight of the contents of this package

core/db/migrate/20170319191942_remove_order_id_from_inventory_units.rb

@@ -0,0 +1,28 @@
+class RemoveOrderIdFromInventoryUnits < ActiveRecord::Migration[5.0]
+  class InconsistentInventoryUnitError < StandardError; end
+
+  class InventoryUnit < ActiveRecord::Base
+    self.table_name = "spree_inventory_units"
+    belongs_to :shipment
+  end
+
+  class Shipment < ActiveRecord::Base
+    self.table_name = "spree_shipments"
+    has_many :inventory_units
+  end
+
+  def up
+    if InventoryUnit.
+        joins(:shipment).
+        where.not(
+          'spree_inventory_units.order_id = spree_shipments.order_id'
+        ).exists?
+      raise InconsistentInventoryUnitError, "You have inventory units with inconsistent order references. Please fix those before running this migration"
+    end
+    remove_column :spree_inventory_units, :order_id
+  end
+
+  def down
+    add_reference :spree_inventory_units, :order, index: true
+  end
+end

core/lib/spree/core/importer/order.rb

@@ -69,7 +69,6 @@ def self.create_shipments_from_params(shipments_hash, order)
               # trying to view these units. Note the Importer might not be
               # able to find the line item if line_item.variant_id |= iu.variant_id
               shipment.inventory_units.new(
-                order: order,
                 variant_id: iu[:variant_id],
                 line_item: line_item
               )

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

@@ -5,11 +5,20 @@
 
 FactoryBot.define do
   factory :inventory_unit, class: 'Spree::InventoryUnit' do
+    transient do
+      order nil
+    end
+
     variant
-    order
-    line_item { build(:line_item, order: order, variant: variant) }
+    line_item do
+      if order
+        build(:line_item, variant: variant, order: order)
+      else
+        build(:line_item, variant: variant)
+      end
+    end
     state 'on_hand'
-    shipment { build(:shipment, state: 'pending', order: order) }
+    shipment { build(:shipment, state: 'pending', order: line_item.order) }
     # return_authorization
   end
 end

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

@@ -25,7 +25,6 @@
       shipment.order.line_items.each do |line_item|
         line_item.quantity.times do
           shipment.inventory_units.create!(
-            order_id: shipment.order_id,
             variant_id: line_item.variant_id,
             line_item_id: line_item.id
           )

core/spec/models/spree/calculator/refunds/default_refund_amount_spec.rb

@@ -5,7 +5,8 @@
   let(:line_item_quantity) { 3 }
   let(:line_item_price) { 100.0 }
   let(:line_item) { create(:line_item, price: line_item_price, quantity: line_item_quantity) }
-  let(:inventory_unit) { build(:inventory_unit, order: order, line_item: line_item) }
+  let(:shipment) { create(:shipment, order: order) }
+  let(:inventory_unit) { build(:inventory_unit, shipment: shipment, line_item: line_item) }
   let(:return_item) { build(:return_item, inventory_unit: inventory_unit ) }
   let(:calculator) { Spree::Calculator::Returns::DefaultRefundAmount.new }
   let(:order) { line_item.order }

core/spec/models/spree/customer_return_spec.rb

@@ -9,10 +9,16 @@
     describe "#return_items_belong_to_same_order" do
       let(:customer_return)       { build(:customer_return) }
 
-      let(:first_inventory_unit)  { build(:inventory_unit) }
+      let(:first_order) { create(:order_with_line_items) }
+      let(:second_order) { first_order }
+
+      let(:first_shipment) { first_order.shipments.first }
+      let(:second_shipment) { second_order.shipments.first }
+
+      let(:first_inventory_unit)  { build(:inventory_unit, shipment: first_shipment) }
       let(:first_return_item)     { build(:return_item, inventory_unit: first_inventory_unit) }
 
-      let(:second_inventory_unit) { build(:inventory_unit, order: second_order) }
+      let(:second_inventory_unit) { build(:inventory_unit, shipment: second_shipment) }
       let(:second_return_item)    { build(:return_item, inventory_unit: second_inventory_unit) }
 
       subject { customer_return.valid? }
@@ -23,7 +29,7 @@
       end
 
       context "return items are part of different orders" do
-        let(:second_order) { create(:order) }
+        let(:second_order) { create(:order_with_line_items) }
 
         it "is not valid" do
           expect(subject).to eq false
@@ -36,7 +42,7 @@
       end
 
       context "return items are part of the same order" do
-        let(:second_order) { first_inventory_unit.order }
+        let(:second_order) { first_order }
 
         it "is valid" do
           expect(subject).to eq true

core/spec/models/spree/inventory_unit_spec.rb

@@ -37,7 +37,6 @@
       unit = shipment.inventory_units.first
       unit.state = 'backordered'
       unit.variant_id = stock_item.variant.id
-      unit.order_id = order.id
       unit.line_item = line_item
       unit.tap(&:save!)
     end
@@ -59,7 +58,6 @@
     it "does not find inventory units that aren't backordered" do
       on_hand_unit = shipment.inventory_units.build
       on_hand_unit.state = 'on_hand'
-      on_hand_unit.order_id = order.id
       on_hand_unit.line_item = line_item
       on_hand_unit.variant = stock_item.variant
       on_hand_unit.save!
@@ -70,7 +68,6 @@
     it "does not find inventory units that don't match the stock item's variant" do
       other_variant_unit = shipment.inventory_units.build
       other_variant_unit.state = 'backordered'
-      other_variant_unit.order_id = order.id
       other_variant_unit.line_item = line_item
       other_variant_unit.variant = create(:variant)
       other_variant_unit.save!
@@ -107,7 +104,6 @@
         unit = other_shipment.inventory_units.build
         unit.state = 'backordered'
         unit.variant_id = stock_item.variant.id
-        unit.order_id = other_order.id
         unit.line_item = line_item
         unit.tap(&:save!)
       end

core/spec/models/spree/order_contents_spec.rb

@@ -19,7 +19,7 @@
     end
 
     context 'given a shipment' do
-      let!(:shipment) { create(:shipment) }
+      let!(:shipment) { create(:shipment, order: order) }
 
       it "ensure shipment calls update_amounts instead of order calling ensure_updated_shipments" do
         expect(subject.order).to_not receive(:ensure_updated_shipments)

core/spec/models/spree/return_item/eligibility_validator/order_completed_spec.rb

@@ -1,7 +1,8 @@
 require 'rails_helper'
 
 RSpec.describe Spree::ReturnItem::EligibilityValidator::OrderCompleted do
-  let(:inventory_unit) { create(:inventory_unit, order: order) }
+  let(:shipment)       { create(:shipment, order: order) }
+  let(:inventory_unit) { create(:inventory_unit, shipment: shipment) }
   let(:return_item)    { create(:return_item, inventory_unit: inventory_unit) }
   let(:validator)      { Spree::ReturnItem::EligibilityValidator::OrderCompleted.new(return_item) }
 

core/spec/models/spree/return_item/eligibility_validator/time_since_purchase_spec.rb

@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 RSpec.describe Spree::ReturnItem::EligibilityValidator::TimeSincePurchase, type: :model do
-  let(:inventory_unit) { create(:inventory_unit, order: create(:shipped_order)) }
+  let(:inventory_unit) { create(:inventory_unit, shipment: create(:shipped_order).shipments.first) }
   let(:return_item)    { create(:return_item, inventory_unit: inventory_unit) }
   let(:validator)      { Spree::ReturnItem::EligibilityValidator::TimeSincePurchase.new(return_item) }
 

core/spec/models/spree/return_item_spec.rb

@@ -19,7 +19,7 @@
   describe '#receive!' do
     let(:now)            { Time.current }
     let(:order)          { create(:shipped_order) }
-    let(:inventory_unit) { create(:inventory_unit, order: order, state: 'shipped') }
+    let(:inventory_unit) { create(:inventory_unit, state: 'shipped') }
     let!(:customer_return) { create(:customer_return_without_return_items, return_items: [return_item], stock_location_id: inventory_unit.shipment.stock_location_id) }
     let(:return_item) { create(:return_item, inventory_unit: inventory_unit) }
 
@@ -58,7 +58,7 @@
     end
 
     context 'when the received item is actually the exchange (aka customer changed mind about exchange)' do
-      let(:exchange_inventory_unit) { create(:inventory_unit, order: order, state: 'shipped') }
+      let(:exchange_inventory_unit) { create(:inventory_unit, state: 'shipped') }
       let!(:return_item_with_exchange) { create(:return_item, inventory_unit: inventory_unit, exchange_inventory_unit: exchange_inventory_unit) }
       let!(:return_item_in_lieu) { create(:return_item, inventory_unit: exchange_inventory_unit) }
 
@@ -222,7 +222,8 @@
   end
 
   describe "#receive" do
-    let(:inventory_unit) { create(:inventory_unit, order: create(:shipped_order)) }
+    let(:order) { create(:shipped_order) }
+    let(:inventory_unit) { create(:inventory_unit, shipment: order.shipments.first) }
     let(:return_item)    { create(:return_item, reception_status: status, inventory_unit: inventory_unit) }
 
     subject { return_item.receive! }
@@ -602,7 +603,6 @@
           expect(subject).not_to be_persisted
           expect(subject.original_return_item).to eq return_item
           expect(subject.line_item).to eq return_item.inventory_unit.line_item
-          expect(subject.order).to eq return_item.inventory_unit.order
         end
       end
     end

core/spec/models/spree/shipment_spec.rb

@@ -264,6 +264,7 @@
         end
 
         before do
+          allow(line_item).to receive(:order) { order }
           allow(shipment).to receive(:inventory_units) { inventory_units }
           allow(inventory_units).to receive_message_chain(:includes, :joins).and_return inventory_units
         end
@@ -650,7 +651,7 @@
     let(:inventory_units) { double }
 
     let(:params) do
-      { variant_id: variant.id, state: 'on_hand', order_id: order.id, line_item_id: line_item.id }
+      { variant_id: variant.id, state: 'on_hand', line_item_id: line_item.id }
     end
 
     before { allow(shipment).to receive_messages inventory_units: inventory_units }

core/spec/models/spree/shipping_manifest_spec.rb

@@ -6,16 +6,14 @@ module Spree
     let(:order) { Order.create! }
     let(:variant) { create :variant }
     let!(:shipment) { create(:shipment, state: 'pending', order: order) }
-    let(:manifest) { described_class.new(inventory_units: inventory_units) }
+    subject(:manifest) { described_class.new(inventory_units: inventory_units) }
 
     def build_unit(variant, attrs = {})
-      attrs = { order: order, variant: variant, shipment: shipment }.merge(attrs)
-      attrs[:line_item] = attrs[:order].contents.add(attrs[:variant])
+      attrs = { variant: variant, shipment: shipment }.merge(attrs)
+      attrs[:line_item] = order.contents.add(variant)
       InventoryUnit.new(attrs)
     end
 
-    subject{ manifest }
-
     describe "#items" do
       context 'empty' do
         let(:inventory_units) { [] }
@@ -68,9 +66,15 @@ def build_unit(variant, attrs = {})
     end
 
     describe "#for_order" do
-      let!(:order2) { Order.create! }
+      let!(:order2) { create(:order_with_line_items) }
       context 'single unit' do
-        let(:inventory_units) { [build_unit(variant)] }
+        let(:inventory_units) { [inventory_unit] }
+        let(:inventory_unit) { build_unit(variant) }
+
+        before do
+          allow(inventory_unit).to receive(:order_id) { order.id }
+        end
+
         it "has single ManifestItem in correct order" do
           expect(manifest.for_order(order).items.count).to eq 1
         end
@@ -81,13 +85,22 @@ def build_unit(variant, attrs = {})
       end
 
       context 'one units in each order' do
-        let(:inventory_units) { [build_unit(variant), build_unit(variant, order: order2)] }
+        let(:order_2) { build_stubbed(:order) }
+        let(:inventory_units) { [inventory_unit_one, inventory_unit_two] }
+        let(:inventory_unit_one) { build_unit(variant) }
+        let(:inventory_unit_two) { build_unit(variant) }
+
+        before do
+          allow(inventory_unit_one).to receive(:order_id) { order.id }
+          allow(inventory_unit_two).to receive(:order_id) { order_2.id }
+        end
+
         it "has single ManifestItem in first order" do
           expect(manifest.for_order(order).items.count).to eq 1
         end
 
         it "has single ManifestItem in second order" do
-          expect(manifest.for_order(order2).items.count).to eq 1
+          expect(manifest.for_order(order_2).items.count).to eq 1
         end
       end
     end

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

@@ -26,10 +26,6 @@ module Stock
         it "builds the inventory units as pending" do
           expect(subject.units.map(&:pending).uniq).to eq [true]
         end
-
-        it "associates the inventory units to the order" do
-          expect(subject.units.map(&:order).uniq).to eq [order]
-        end
       end
     end
   end