Skip to content

Commit

Permalink
Remove order association from inventory units
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mamhoff committed Nov 15, 2017
1 parent 87154a0 commit 747293a
Show file tree
Hide file tree
Showing 10 changed files with 19 additions and 18 deletions.
4 changes: 2 additions & 2 deletions core/app/models/spree/inventory_unit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
7 changes: 5 additions & 2 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/order_cancellations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/reimbursement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions core/app/models/spree/shipment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/shipping_manifest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
3 changes: 2 additions & 1 deletion core/spec/models/spree/shipment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }
Expand Down
8 changes: 4 additions & 4 deletions core/spec/models/spree/shipping_manifest_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 747293a

Please sign in to comment.