From f6ef7103fc0183715e85c2efc87be7882b7c36b9 Mon Sep 17 00:00:00 2001
From: Martin Meyerhoff <martin@stembolt.com>
Date: Sat, 18 Mar 2017 12:20:53 +0100
Subject: [PATCH 1/2] Allow fulfilling all items of a ready shipment from
 another location

Prior to this commit, this would not work because the destruction
of the original shipment would result in an exception being thrown.

Ready shipments can be edited and thus have to be able to be destroyed, too.

Fixes https://github.com/solidusio/solidus/issues/1684
---
 .../features/admin/orders/shipments_spec.rb   | 21 +++++++++++++++++++
 core/app/models/spree/inventory_unit.rb       |  2 +-
 core/app/models/spree/shipment.rb             |  2 +-
 core/spec/models/spree/inventory_unit_spec.rb |  7 +++----
 core/spec/models/spree/shipment_spec.rb       |  7 +++----
 5 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/backend/spec/features/admin/orders/shipments_spec.rb b/backend/spec/features/admin/orders/shipments_spec.rb
index c04a08987f7..db38be271bd 100644
--- a/backend/spec/features/admin/orders/shipments_spec.rb
+++ b/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
diff --git a/core/app/models/spree/inventory_unit.rb b/core/app/models/spree/inventory_unit.rb
index c9440cd87ea..ac3acaf42f6 100644
--- a/core/app/models/spree/inventory_unit.rb
+++ b/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
diff --git a/core/app/models/spree/shipment.rb b/core/app/models/spree/shipment.rb
index 424382bf0b1..e64de7426a2 100644
--- a/core/app/models/spree/shipment.rb
+++ b/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
diff --git a/core/spec/models/spree/inventory_unit_spec.rb b/core/spec/models/spree/inventory_unit_spec.rb
index 458b89455d9..fc250b79115 100644
--- a/core/spec/models/spree/inventory_unit_spec.rb
+++ b/core/spec/models/spree/inventory_unit_spec.rb
@@ -289,11 +289,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
diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb
index 06ea05db799..fd85241dc09 100644
--- a/core/spec/models/spree/shipment_spec.rb
+++ b/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

From ea228cdc29d0b3e4b8964f84d8b493b37647b952 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@stembolt.com>
Date: Wed, 5 Apr 2017 16:39:13 -0700
Subject: [PATCH 2/2] Update CHANGELOG

---
 CHANGELOG.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 638bbffdf2c..0eaea9aade6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,6 +31,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]`