From 747293a570abfdf8fbd8798353aa2a670b170248 Mon Sep 17 00:00:00 2001
From: Martin Meyerhoff <martin@stembolt.com>
Date: Tue, 14 Mar 2017 08:26:04 +0100
Subject: [PATCH 01/12] Remove order association from inventory units

Inventory units link line items to shipments. Both of these have an
order. Let's make this a has_many association, through :shipments, which
are required.

This made a number of changes in the codebase necessary, such as
checking for order instead of order id identity.
---
 core/app/models/spree/inventory_unit.rb                   | 4 ++--
 core/app/models/spree/order.rb                            | 7 +++++--
 core/app/models/spree/order_cancellations.rb              | 2 +-
 core/app/models/spree/reimbursement.rb                    | 2 +-
 core/app/models/spree/shipment.rb                         | 3 +--
 core/app/models/spree/shipping_manifest.rb                | 2 +-
 .../testing_support/factories/inventory_unit_factory.rb   | 5 ++---
 .../spree/testing_support/factories/shipment_factory.rb   | 1 -
 core/spec/models/spree/shipment_spec.rb                   | 3 ++-
 core/spec/models/spree/shipping_manifest_spec.rb          | 8 ++++----
 10 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/core/app/models/spree/inventory_unit.rb b/core/app/models/spree/inventory_unit.rb
index 18d5f91500b..5d27275268c 100644
--- a/core/app/models/spree/inventory_unit.rb
+++ b/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,9 @@ 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
+    validates_presence_of :shipment, :line_item, :variant
 
     before_destroy :ensure_can_destroy
 
diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb
index a628e49e18e..3bd3d30c4e5 100644
--- a/core/app/models/spree/order.rb
+++ b/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
diff --git a/core/app/models/spree/order_cancellations.rb b/core/app/models/spree/order_cancellations.rb
index aa27b2f679a..56623334188 100644
--- a/core/app/models/spree/order_cancellations.rb
+++ b/core/app/models/spree/order_cancellations.rb
@@ -25,7 +25,7 @@ def initialize(order)
   #
   # @return [Array<UnitCancel>] the units that have been canceled due to short shipping
   def short_ship(inventory_units, whodunnit:nil)
-    if inventory_units.map(&:order_id).uniq != [@order.id]
+    if inventory_units.map(&:order).uniq != [@order]
       raise ArgumentError, "Not all inventory units belong to this order"
     end
 
diff --git a/core/app/models/spree/reimbursement.rb b/core/app/models/spree/reimbursement.rb
index b21865cc00d..555187464be 100644
--- a/core/app/models/spree/reimbursement.rb
+++ b/core/app/models/spree/reimbursement.rb
@@ -154,7 +154,7 @@ def generate_number
     end
 
     def validate_return_items_belong_to_same_order
-      if return_items.any? { |ri| ri.inventory_unit.order_id != order_id }
+      if return_items.any? { |ri| ri.inventory_unit.order != order }
         errors.add(:base, :return_items_order_id_does_not_match)
       end
     end
diff --git a/core/app/models/spree/shipment.rb b/core/app/models/spree/shipment.rb
index 9a1f793c12d..aaaf70514b7 100644
--- a/core/app/models/spree/shipment.rb
+++ b/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
diff --git a/core/app/models/spree/shipping_manifest.rb b/core/app/models/spree/shipping_manifest.rb
index c8671591bc6..c132e5e563d 100644
--- a/core/app/models/spree/shipping_manifest.rb
+++ b/core/app/models/spree/shipping_manifest.rb
@@ -7,7 +7,7 @@ def initialize(inventory_units:)
 
   def for_order(order)
     Spree::ShippingManifest.new(
-      inventory_units: @inventory_units.select { |iu| iu.order_id == order.id }
+      inventory_units: @inventory_units.select { |iu| iu.shipment.order_id == order.id }
     )
   end
 
diff --git a/core/lib/spree/testing_support/factories/inventory_unit_factory.rb b/core/lib/spree/testing_support/factories/inventory_unit_factory.rb
index 283b525ace4..a5ffd800ec2 100644
--- a/core/lib/spree/testing_support/factories/inventory_unit_factory.rb
+++ b/core/lib/spree/testing_support/factories/inventory_unit_factory.rb
@@ -6,10 +6,9 @@
 FactoryBot.define do
   factory :inventory_unit, class: 'Spree::InventoryUnit' do
     variant
-    order
-    line_item { build(:line_item, order: order, variant: variant) }
+    line_item { build(:line_item, variant: variant) }
     state 'on_hand'
-    shipment { build(:shipment, state: 'pending', order: order) }
+    shipment { build(:shipment, state: 'pending', order: line_item.order) }
     # return_authorization
   end
 end
diff --git a/core/lib/spree/testing_support/factories/shipment_factory.rb b/core/lib/spree/testing_support/factories/shipment_factory.rb
index 923707adf77..6bce8ca6011 100644
--- a/core/lib/spree/testing_support/factories/shipment_factory.rb
+++ b/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
           )
diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb
index e22c5183133..a8797971538 100644
--- a/core/spec/models/spree/shipment_spec.rb
+++ b/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 }
diff --git a/core/spec/models/spree/shipping_manifest_spec.rb b/core/spec/models/spree/shipping_manifest_spec.rb
index c1f6d8214bf..95c641d21f0 100644
--- a/core/spec/models/spree/shipping_manifest_spec.rb
+++ b/core/spec/models/spree/shipping_manifest_spec.rb
@@ -9,8 +9,8 @@ module Spree
     let(: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(attrs[:variant])
       InventoryUnit.new(attrs)
     end
 
@@ -68,7 +68,7 @@ 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)] }
         it "has single ManifestItem in correct order" do
@@ -81,7 +81,7 @@ 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(:inventory_units) { [build_unit(variant), build_unit(variant, shipment: order2.shipments.first)] }
         it "has single ManifestItem in first order" do
           expect(manifest.for_order(order).items.count).to eq 1
         end

From 35b73d62cf2478df5b81cbec1d9c91ba55e49bf0 Mon Sep 17 00:00:00 2001
From: Martin Meyerhoff <martin@stembolt.com>
Date: Sun, 19 Mar 2017 21:32:32 +0100
Subject: [PATCH 02/12] Remove order reference from inventory unit table

We do not need this column as the same data is represented
by the `order_id` column on the `spree_shipments` table.

I needed to change some stubbing in the manifest spec.
---
 core/app/models/spree/shipping_manifest.rb    |  2 +-
 ...42_remove_order_id_from_inventory_units.rb | 28 +++++++++++++++++++
 core/spec/models/spree/inventory_unit_spec.rb |  4 ---
 .../models/spree/shipping_manifest_spec.rb    | 27 +++++++++++++-----
 4 files changed, 49 insertions(+), 12 deletions(-)
 create mode 100644 core/db/migrate/20170319191942_remove_order_id_from_inventory_units.rb

diff --git a/core/app/models/spree/shipping_manifest.rb b/core/app/models/spree/shipping_manifest.rb
index c132e5e563d..995f188d07f 100644
--- a/core/app/models/spree/shipping_manifest.rb
+++ b/core/app/models/spree/shipping_manifest.rb
@@ -7,7 +7,7 @@ def initialize(inventory_units:)
 
   def for_order(order)
     Spree::ShippingManifest.new(
-      inventory_units: @inventory_units.select { |iu| iu.shipment.order_id == order.id }
+      inventory_units: @inventory_units.select { |iu| iu.order == order }
     )
   end
 
diff --git a/core/db/migrate/20170319191942_remove_order_id_from_inventory_units.rb b/core/db/migrate/20170319191942_remove_order_id_from_inventory_units.rb
new file mode 100644
index 00000000000..3c36b606890
--- /dev/null
+++ b/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
diff --git a/core/spec/models/spree/inventory_unit_spec.rb b/core/spec/models/spree/inventory_unit_spec.rb
index 95e66864a8e..de41ed67915 100644
--- a/core/spec/models/spree/inventory_unit_spec.rb
+++ b/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
diff --git a/core/spec/models/spree/shipping_manifest_spec.rb b/core/spec/models/spree/shipping_manifest_spec.rb
index 95c641d21f0..86577e860ea 100644
--- a/core/spec/models/spree/shipping_manifest_spec.rb
+++ b/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 = { variant: variant, shipment: shipment }.merge(attrs)
-      attrs[:line_item] = order.contents.add(attrs[:variant])
+      attrs[:line_item] = order.contents.add(variant)
       InventoryUnit.new(attrs)
     end
 
-    subject{ manifest }
-
     describe "#items" do
       context 'empty' do
         let(:inventory_units) { [] }
@@ -70,7 +68,13 @@ def build_unit(variant, attrs = {})
     describe "#for_order" do
       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) { order }
+        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, shipment: order2.shipments.first)] }
+        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) { order }
+          allow(inventory_unit_two).to receive(:order) { order_2 }
+        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

From 22ea8c15cb635ff37fba8b8bbadc8deffd6b176d Mon Sep 17 00:00:00 2001
From: Martin Meyerhoff <martin@stembolt.com>
Date: Thu, 29 Jun 2017 17:30:48 +0200
Subject: [PATCH 03/12] Delegate InventoryUnit#order_id to shipment

This method would be lost by the removal of the DB column.
Alternatively, we could call `order.id`, but that would
in many cases unnecessarily instantiate more objects, so
I opt for an explicit delegate.
---
 core/app/models/spree/inventory_unit.rb          | 2 ++
 core/app/models/spree/order_cancellations.rb     | 2 +-
 core/app/models/spree/reimbursement.rb           | 2 +-
 core/app/models/spree/shipping_manifest.rb       | 2 +-
 core/spec/models/spree/shipping_manifest_spec.rb | 6 +++---
 5 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/core/app/models/spree/inventory_unit.rb b/core/app/models/spree/inventory_unit.rb
index 5d27275268c..867c12d8aa2 100644
--- a/core/app/models/spree/inventory_unit.rb
+++ b/core/app/models/spree/inventory_unit.rb
@@ -17,6 +17,8 @@ class InventoryUnit < Spree::Base
     has_one :unit_cancel, class_name: "Spree::UnitCancel"
     has_one :order, through: :shipment
 
+    delegate :order_id, to: :shipment
+
     validates_presence_of :shipment, :line_item, :variant
 
     before_destroy :ensure_can_destroy
diff --git a/core/app/models/spree/order_cancellations.rb b/core/app/models/spree/order_cancellations.rb
index 56623334188..aa27b2f679a 100644
--- a/core/app/models/spree/order_cancellations.rb
+++ b/core/app/models/spree/order_cancellations.rb
@@ -25,7 +25,7 @@ def initialize(order)
   #
   # @return [Array<UnitCancel>] the units that have been canceled due to short shipping
   def short_ship(inventory_units, whodunnit:nil)
-    if inventory_units.map(&:order).uniq != [@order]
+    if inventory_units.map(&:order_id).uniq != [@order.id]
       raise ArgumentError, "Not all inventory units belong to this order"
     end
 
diff --git a/core/app/models/spree/reimbursement.rb b/core/app/models/spree/reimbursement.rb
index 555187464be..b21865cc00d 100644
--- a/core/app/models/spree/reimbursement.rb
+++ b/core/app/models/spree/reimbursement.rb
@@ -154,7 +154,7 @@ def generate_number
     end
 
     def validate_return_items_belong_to_same_order
-      if return_items.any? { |ri| ri.inventory_unit.order != order }
+      if return_items.any? { |ri| ri.inventory_unit.order_id != order_id }
         errors.add(:base, :return_items_order_id_does_not_match)
       end
     end
diff --git a/core/app/models/spree/shipping_manifest.rb b/core/app/models/spree/shipping_manifest.rb
index 995f188d07f..c8671591bc6 100644
--- a/core/app/models/spree/shipping_manifest.rb
+++ b/core/app/models/spree/shipping_manifest.rb
@@ -7,7 +7,7 @@ def initialize(inventory_units:)
 
   def for_order(order)
     Spree::ShippingManifest.new(
-      inventory_units: @inventory_units.select { |iu| iu.order == order }
+      inventory_units: @inventory_units.select { |iu| iu.order_id == order.id }
     )
   end
 
diff --git a/core/spec/models/spree/shipping_manifest_spec.rb b/core/spec/models/spree/shipping_manifest_spec.rb
index 86577e860ea..66fa6fadaf3 100644
--- a/core/spec/models/spree/shipping_manifest_spec.rb
+++ b/core/spec/models/spree/shipping_manifest_spec.rb
@@ -72,7 +72,7 @@ def build_unit(variant, attrs = {})
         let(:inventory_unit) { build_unit(variant) }
 
         before do
-          allow(inventory_unit).to receive(:order) { order }
+          allow(inventory_unit).to receive(:order_id) { order.id }
         end
 
         it "has single ManifestItem in correct order" do
@@ -91,8 +91,8 @@ def build_unit(variant, attrs = {})
         let(:inventory_unit_two) { build_unit(variant) }
 
         before do
-          allow(inventory_unit_one).to receive(:order) { order }
-          allow(inventory_unit_two).to receive(:order) { order_2 }
+          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

From 1ceab159fcd906c5b327ba727efa7d2bd4c120da Mon Sep 17 00:00:00 2001
From: Martin Meyerhoff <martin@stembolt.com>
Date: Thu, 29 Jun 2017 22:15:44 +0200
Subject: [PATCH 04/12] Fix Customer return validation spec

This spec was quite brittle as everything was stubbed. This is a minimal fix,
I still do not regard this a well-working spec.
---
 core/spec/models/spree/customer_return_spec.rb | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/core/spec/models/spree/customer_return_spec.rb b/core/spec/models/spree/customer_return_spec.rb
index 774f611e09c..78b0339e0a1 100644
--- a/core/spec/models/spree/customer_return_spec.rb
+++ b/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

From bf86a93c9f018b7b25748d45d13b5dc7269bf72f Mon Sep 17 00:00:00 2001
From: Martin Meyerhoff <martin@stembolt.com>
Date: Thu, 29 Jun 2017 22:27:41 +0200
Subject: [PATCH 05/12] Fix OrderContents#add spec when shipment is given

This shipment in this spec was not associated to the same order,
which is unrealistic. Fixed by adding the necessary association.
---
 core/spec/models/spree/order_contents_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/spec/models/spree/order_contents_spec.rb b/core/spec/models/spree/order_contents_spec.rb
index 2423139ceea..8b0d4e261c6 100644
--- a/core/spec/models/spree/order_contents_spec.rb
+++ b/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)

From 499f5ce0d31fe9bc07cc287a7d563991878f38fd Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@stembolt.com>
Date: Fri, 21 Jul 2017 15:13:46 -0700
Subject: [PATCH 06/12] Avoid assigning order on inventory_unit

---
 core/app/models/spree/return_item.rb                      | 2 +-
 core/app/models/spree/stock/inventory_unit_builder.rb     | 3 +--
 .../calculator/refunds/default_refund_amount_spec.rb      | 3 ++-
 .../eligibility_validator/order_completed_spec.rb         | 3 ++-
 .../eligibility_validator/time_since_purchase_spec.rb     | 2 +-
 core/spec/models/spree/return_item_spec.rb                | 8 ++++----
 core/spec/models/spree/stock/package_spec.rb              | 3 ++-
 7 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/core/app/models/spree/return_item.rb b/core/app/models/spree/return_item.rb
index 89faafe2104..a33e32b1963 100644
--- a/core/app/models/spree/return_item.rb
+++ b/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
diff --git a/core/app/models/spree/stock/inventory_unit_builder.rb b/core/app/models/spree/stock/inventory_unit_builder.rb
index d3d07f5b3c8..ed450dbb658 100644
--- a/core/app/models/spree/stock/inventory_unit_builder.rb
+++ b/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
diff --git a/core/spec/models/spree/calculator/refunds/default_refund_amount_spec.rb b/core/spec/models/spree/calculator/refunds/default_refund_amount_spec.rb
index b9097af7e49..e8bdda4bbc0 100644
--- a/core/spec/models/spree/calculator/refunds/default_refund_amount_spec.rb
+++ b/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 }
diff --git a/core/spec/models/spree/return_item/eligibility_validator/order_completed_spec.rb b/core/spec/models/spree/return_item/eligibility_validator/order_completed_spec.rb
index 0d5b0cfb54a..f7457046123 100644
--- a/core/spec/models/spree/return_item/eligibility_validator/order_completed_spec.rb
+++ b/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) }
 
diff --git a/core/spec/models/spree/return_item/eligibility_validator/time_since_purchase_spec.rb b/core/spec/models/spree/return_item/eligibility_validator/time_since_purchase_spec.rb
index f054c7462af..12c549975d5 100644
--- a/core/spec/models/spree/return_item/eligibility_validator/time_since_purchase_spec.rb
+++ b/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) }
 
diff --git a/core/spec/models/spree/return_item_spec.rb b/core/spec/models/spree/return_item_spec.rb
index 5fae1841840..64c76e8cd16 100644
--- a/core/spec/models/spree/return_item_spec.rb
+++ b/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
diff --git a/core/spec/models/spree/stock/package_spec.rb b/core/spec/models/spree/stock/package_spec.rb
index 84f2365f76f..c4c4a19b279 100644
--- a/core/spec/models/spree/stock/package_spec.rb
+++ b/core/spec/models/spree/stock/package_spec.rb
@@ -6,11 +6,12 @@ module Stock
       let(:variant) { build(:variant, weight: 25.0) }
       let(:stock_location) { build(:stock_location) }
       let(:order) { build(:order) }
+      let(:line_item) { build(:line_item, order: order) }
 
       subject { Package.new(stock_location) }
 
       def build_inventory_unit
-        build(:inventory_unit, variant: variant, order: order)
+        build(:inventory_unit, variant: variant, line_item: line_item)
       end
 
       it 'calculates the weight of all the contents' do

From d9a5315b166b0f8cc1a275239b782a089fd8aad4 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@stembolt.com>
Date: Fri, 28 Jul 2017 13:52:17 -0700
Subject: [PATCH 07/12] Raise when assigning order

---
 core/app/models/spree/inventory_unit.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/core/app/models/spree/inventory_unit.rb b/core/app/models/spree/inventory_unit.rb
index 867c12d8aa2..7c5deb2747a 100644
--- a/core/app/models/spree/inventory_unit.rb
+++ b/core/app/models/spree/inventory_unit.rb
@@ -19,6 +19,10 @@ class InventoryUnit < Spree::Base
 
     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

From 25ffb768215290f9aceee680e917ea7d41982506 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@stembolt.com>
Date: Tue, 1 Aug 2017 12:15:54 -0700
Subject: [PATCH 08/12] Allow passing order to inventory_unit factory

---
 .../factories/inventory_unit_factory.rb              | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/core/lib/spree/testing_support/factories/inventory_unit_factory.rb b/core/lib/spree/testing_support/factories/inventory_unit_factory.rb
index a5ffd800ec2..d3f803e3920 100644
--- a/core/lib/spree/testing_support/factories/inventory_unit_factory.rb
+++ b/core/lib/spree/testing_support/factories/inventory_unit_factory.rb
@@ -5,8 +5,18 @@
 
 FactoryBot.define do
   factory :inventory_unit, class: 'Spree::InventoryUnit' do
+    transient do
+      order nil
+    end
+
     variant
-    line_item { build(:line_item, 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: line_item.order) }
     # return_authorization

From 1344e587bd75bf72e32db9de030638140da26e98 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@stembolt.com>
Date: Tue, 1 Aug 2017 12:16:40 -0700
Subject: [PATCH 09/12] Use line_item to determine order in Stock::Package

---
 core/app/models/spree/stock/package.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/app/models/spree/stock/package.rb b/core/app/models/spree/stock/package.rb
index 0585bcc16f7..af63bf1d2ae 100644
--- a/core/app/models/spree/stock/package.rb
+++ b/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

From 7f15f93f5875f126cacffc389f94e786d55954da Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@stembolt.com>
Date: Tue, 1 Aug 2017 12:16:57 -0700
Subject: [PATCH 10/12] Don't assign order in Order importer

---
 core/lib/spree/core/importer/order.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/core/lib/spree/core/importer/order.rb b/core/lib/spree/core/importer/order.rb
index bbc0f66455f..8fd2589d129 100644
--- a/core/lib/spree/core/importer/order.rb
+++ b/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
               )

From 058d1741f0ecf5695e295538811527ad4fcdfd4d Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@stembolt.com>
Date: Tue, 1 Aug 2017 12:17:17 -0700
Subject: [PATCH 11/12] Remove now invalid test

---
 core/spec/models/spree/stock/inventory_unit_builder_spec.rb | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/core/spec/models/spree/stock/inventory_unit_builder_spec.rb b/core/spec/models/spree/stock/inventory_unit_builder_spec.rb
index 5fbb005183b..6c084d23f1a 100644
--- a/core/spec/models/spree/stock/inventory_unit_builder_spec.rb
+++ b/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

From 64a038dd1a22460cbac8c73f4604021621cb8721 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@stembolt.com>
Date: Tue, 1 Aug 2017 14:37:53 -0700
Subject: [PATCH 12/12] Fix spec

---
 core/spec/models/spree/stock/package_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/spec/models/spree/stock/package_spec.rb b/core/spec/models/spree/stock/package_spec.rb
index c4c4a19b279..2f568753acc 100644
--- a/core/spec/models/spree/stock/package_spec.rb
+++ b/core/spec/models/spree/stock/package_spec.rb
@@ -165,7 +165,7 @@ def build_inventory_unit
 
           it "returns an order" do
             expect(subject.order).to be_a_kind_of Spree::Order
-            expect(subject.order).to eq unit.order
+            expect(subject.order).to eq order
           end
         end