Rename Shipment#update! to Shipment#update_state

core/app/models/spree/exchange.rb

@@ -27,7 +27,7 @@ def perform!
       @order.shipments += shipments
       @order.save!
       shipments.each do |shipment|
-        shipment.update!(@order)
+        shipment.update_state
         shipment.finalize!
       end
     end

core/app/models/spree/order.rb

@@ -417,7 +417,7 @@ def finalize!
       # update payment and shipment(s) states, and save
       updater.update_payment_state
       shipments.each do |shipment|
-        shipment.update!(self)
+        shipment.update_state
         shipment.finalize!
       end
 
@@ -431,7 +431,7 @@ def finalize!
     end
 
     def fulfill!
-      shipments.each { |shipment| shipment.update!(self) if shipment.persisted? }
+      shipments.each { |shipment| shipment.update_state if shipment.persisted? }
       updater.update_shipment_state
       save!
     end

core/app/models/spree/order_updater.rb

@@ -141,7 +141,7 @@ def update_shipment_amounts
     # give each of the shipments a chance to update themselves
     def update_shipments
       shipments.each do |shipment|
-        shipment.update!(order)
+        shipment.update_state
       end
     end
 

core/app/models/spree/shipment.rb

@@ -305,10 +305,11 @@ def update_attributes_and_order(params = {})
       end
     end
 
-    # Updates various aspects of the Shipment while bypassing any callbacks.  Note that this method takes an explicit reference to the
-    # Order object.  This is necessary because the association actually has a stale (and unsaved) copy of the Order and so it will not
-    # yield the correct results.
-    def update!(order)
+    # Updates the state of the Shipment bypassing any callbacks.
+    #
+    # If this moves the shipmnent to the 'shipped' state, after_ship will be
+    # called.
+    def update_state
       old_state = state
       new_state = determine_state(order)
       if new_state != old_state
@@ -320,6 +321,18 @@ def update!(order)
       end
     end
 
+    def update!(order_or_attrs)
+      if order_or_attrs.is_a?(Spree::Order)
+        Spree::Deprecation.warn "Calling Shipment#update! with an order to update the shipments state is deprecated. Please use Shipment#update_state instead."
+        if order_or_attrs.object_id != order.object_id
+          Spree::Deprecation.warn "Additionally, update! is being passed an instance of order which isn't the same object as the shipment's order association"
+        end
+        update_state
+      else
+        super
+      end
+    end
+
     def transfer_to_location(variant, quantity, stock_location)
       if quantity <= 0
         raise ArgumentError

core/spec/models/spree/order/finalizing_spec.rb

@@ -18,7 +18,7 @@
 
     it "should sell inventory units" do
       order.shipments.each do |shipment|
-        expect(shipment).to receive(:update!)
+        expect(shipment).to receive(:update_state)
         expect(shipment).to receive(:finalize!)
       end
       order.finalize!

core/spec/models/spree/order_updater_spec.rb

@@ -483,7 +483,7 @@ def create_adjustment(label, amount)
         let(:shipment){ order.shipments[0] }
 
         it "updates each shipment" do
-          expect(shipment).to receive(:update!)
+          expect(shipment).to receive(:update_state)
           updater.update
         end
 

core/spec/models/spree/shipment_spec.rb

@@ -265,13 +265,13 @@
     end
   end
 
-  context "#update!" do
+  context "#update_state" do
     shared_examples_for "immutable once shipped" do
       before { shipment.update_columns(state: 'shipped') }
 
       it "should remain in shipped state once shipped" do
         expect {
-          shipment.update!(order)
+          shipment.update_state
         }.not_to change { shipment.state }
       end
     end
@@ -283,7 +283,7 @@
 
         allow(shipment).to receive_messages(inventory_units: [mock_model(Spree::InventoryUnit, allow_ship?: false, canceled?: false)])
         expect(shipment).to receive(:update_columns).with(state: 'pending', updated_at: kind_of(Time))
-        shipment.update!(order)
+        shipment.update_state
       end
     end
 
@@ -293,15 +293,15 @@
         # Set as ready so we can test for change
         shipment.update_attributes!(state: 'ready')
         expect(shipment).to receive(:update_columns).with(state: 'pending', updated_at: kind_of(Time))
-        shipment.update!(order)
+        shipment.update_state
       end
     end
 
     context "when order is paid" do
       before { allow(order).to receive_messages paid?: true }
       it "should result in a 'ready' state" do
         expect(shipment).to receive(:update_columns).with(state: 'ready', updated_at: kind_of(Time))
-        shipment.update!(order)
+        shipment.update_state
       end
       it_should_behave_like 'immutable once shipped'
       it_should_behave_like 'pending if backordered'
@@ -314,7 +314,7 @@
 
       it "should result in a 'ready' state" do
         expect(shipment).to receive(:update_columns).with(state: 'ready', updated_at: kind_of(Time))
-        shipment.update!(order)
+        shipment.update_state
       end
       it_should_behave_like 'immutable once shipped'
       it_should_behave_like 'pending if backordered'
@@ -325,7 +325,7 @@
       it "should result in a 'pending' state" do
         shipment.state = 'ready'
         expect(shipment).to receive(:update_columns).with(state: 'pending', updated_at: kind_of(Time))
-        shipment.update!(order)
+        shipment.update_state
       end
       it_should_behave_like 'immutable once shipped'
       it_should_behave_like 'pending if backordered'
@@ -336,7 +336,7 @@
       it "should result in a 'ready' state" do
         shipment.state = 'pending'
         expect(shipment).to receive(:update_columns).with(state: 'ready', updated_at: kind_of(Time))
-        shipment.update!(order)
+        shipment.update_state
       end
       it_should_behave_like 'immutable once shipped'
       it_should_behave_like 'pending if backordered'
@@ -348,7 +348,7 @@
         expect(shipment).to receive :after_ship
         allow(shipment).to receive_messages determine_state: 'shipped'
         expect(shipment).to receive(:update_columns).with(state: 'shipped', updated_at: kind_of(Time))
-        shipment.update!(order)
+        shipment.update_state
       end
 
       # Regression test for https://github.com/spree/spree/issues/4347