Orphaned Inventory Units in Backend, Possibly Related to Spree::Order#associate_user!

[joshhepworth]

It seems like when I use the admin interface to create orders in the following order, Spree::InventoryUnit rows associated with a destroyed shipment will be left behind. To reproduce in 2.4 (I don't have a 3.0 install to test on):

Setup

Ensure you have at least one user and the bare minimum to complete an order with shipping method/rate.

Steps

1. In the backend, click "New Order"
2. Add one variant, quantity 1 to the order. (More will also reproduce the issue.)
3. Click Customer Details and select an existing user with an address. Update the order.

In the Rails console, I can then run Spree::Order.find_by(number: "ORDER NUMBER").inventory_units and there will be two records returned. One will be associated with a shipment that exists, the other will not. Running the following will return 1 in the above scenario from a clean install after taking the above steps:

Spree::InventoryUnit.where(pending: true).where.not(shipment_id: Spree::Shipment.pluck('id')).count

I believe this occurs because of the call to update_all on the order in the Spree::Order#associate_user! method. However, I'm not sure if this is acceptable behavior or not. Is it Spree's intention to have these orphaned inventory units, or would the expectation be that they are cleaned up when the new shipment is created for the order by associating a user with an address?

[JDutil]

I believe this is a duplicate of #6605

[joshhepworth]

I was able to reproduce the orphaned InventoryUnit while using @tesserakt's modified Spree::Stock::InventoryUnitBuilder. As such, I do not believe these are related issues.

If you take the steps outlined above with the following in the application:

Spree::Stock::InventoryUnitBuilder.class_eval do
  def initialize(order)
    @order = order
  end

  def units
    @order.line_items.flat_map do |line_item|
      line_item.quantity.times.map do |i|
        Spree::InventoryUnit.new(
          pending: true,
          variant: line_item.variant,
          line_item: line_item,
          order: @order
        )
      end
    end
  end
end

The issue above is still reproducible.

[joshhepworth]

Perhaps it would be useful to look at additional output after following the steps in the issue above. The following are the InventoryUnit associated with the Order after taking the steps above.

[
  #<Spree::InventoryUnit id: 372, 
    state: "on_hand", 
    variant_id: 2079, 
    order_id: 152, 
    shipment_id: 297, 
    created_at: "2015-08-18 14:40:59", 
    updated_at: "2015-08-18 14:40:59", 
    pending: true, 
    line_item_id: 282>,
  #<Spree::InventoryUnit id: 373, 
    state: "on_hand", 
    variant_id: 2079, 
    order_id: 152, 
    shipment_id: 298, 
    created_at: "2015-08-18 14:41:21", 
    updated_at: "2015-08-18 14:41:21", 
    pending: true, 
    line_item_id: 282>]

The InventoryUnit hasn't actually been doubled. They are created at different times and associated with different shipments. The issue is that Spree::Shipment.find(297) doesn't exist. The process of associating the user creates the new InventoryUnit above (id=373), but it doesn't remove the InventoryUnit that existed prior to associating the user (id=372).