Merge pull request #1784 from mamhoff/fix-fulfilling-ready-shipments-…

…from-another-location

Allow fulfilling all items of a ready shipment from another location

CHANGELOG.md

@@ -35,6 +35,10 @@
 *   Fix an issue where updating a user in the admin without specifying roles in
     the params would clear the user's roles.
 
+*   Allow destruction of shipments in the "ready" state.
+
+    [#1784](https://github.com/solidusio/solidus/pull/1784)
+
 *   Deprecations
 
     * `cache_key_for_taxons` helper has been deprecated in favour of `cache [I18n.locale, @taxons]`

backend/spec/features/admin/orders/shipments_spec.rb

@@ -79,5 +79,26 @@ def ship_shipment
       expect(page).to have_css("#shipment_#{shipment2.id} tr.stock-item", count: 2)
       expect(page).to have_css("#shipment_#{shipment1.id} tr.stock-item", count: 3)
     end
+
+    context "with a ready-to-ship order" do
+      let(:variant) { create(:variant) }
+      let!(:order) do
+        create(
+          :order_ready_to_ship,
+          number: "R100",
+          line_items_attributes: [{ variant: variant, quantity: 5 }]
+        )
+      end
+
+      it "can transfer all items to a new location" do
+        expect(order.shipments.count).to eq(1)
+
+        within_row(1) { click_icon 'arrows-h' }
+        complete_split_to('LA', quantity: 5)
+
+        expect(page).to_not have_content("package from 'NY Warehouse'")
+        expect(page).to have_content("package from 'LA'")
+      end
+    end
   end
 end

core/app/models/spree/inventory_unit.rb

@@ -141,7 +141,7 @@ def ensure_can_destroy
         throw :abort
       end
 
-      unless shipment.pending?
+      if shipment.shipped? || shipment.canceled?
         errors.add(:base, :cannot_destroy_shipment_state, state: shipment.state)
         throw :abort
       end

core/app/models/spree/shipment.rb

@@ -391,7 +391,7 @@ def set_cost_zero_when_nil
     end
 
     def ensure_can_destroy
-      unless pending?
+      if shipped? || canceled?
         errors.add(:state, :cannot_destroy, state: state)
         throw :abort
       end

core/spec/models/spree/inventory_unit_spec.rb

@@ -298,11 +298,10 @@
       expect { inventory_unit.reload }.not_to raise_error
     end
 
-    it "cannot be destroyed if its shipment is ready" do
+    it "can be destroyed if its shipment is ready" do
       inventory_unit = create(:order_ready_to_ship).inventory_units.first
-      expect(inventory_unit.destroy).to eq false
-      expect(inventory_unit.errors.full_messages.join).to match /Cannot destroy/
-      expect { inventory_unit.reload }.not_to raise_error
+      expect(inventory_unit.destroy).to be_truthy
+      expect { inventory_unit.reload }.to raise_error(ActiveRecord::RecordNotFound)
     end
 
     it "cannot be destroyed if its shipment is shipped" do

core/spec/models/spree/shipment_spec.rb

@@ -724,11 +724,10 @@
       expect { shipment.reload }.to raise_error(ActiveRecord::RecordNotFound)
     end
 
-    it "cannot be destroyed when ready" do
+    it "can be destroyed when ready" do
       shipment = create(:shipment, state: "ready")
-      expect(shipment.destroy).to eq false
-      expect(shipment.errors.full_messages.join).to match /Cannot destroy/
-      expect { shipment.reload }.not_to raise_error
+      expect(shipment.destroy).to be_truthy
+      expect { shipment.reload }.to raise_error(ActiveRecord::RecordNotFound)
     end
 
     it "cannot be destroyed when shipped" do