From 5fd112adb4b333e6c79d595d3bef5a5016a27dda Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Fri, 5 Sep 2025 19:01:35 -0700 Subject: [PATCH 1/2] Add missing tests for shipment state after cancel When cancelling all items in a completed order, if the shipment is not already shipped, it should be marked as cancelled. However, if the shipment has already shipped, we should maintain its state because we treat that state as immutable. --- .../models/spree/order_cancellations_spec.rb | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/core/spec/models/spree/order_cancellations_spec.rb b/core/spec/models/spree/order_cancellations_spec.rb index 3834f61fa2..8de4253c4d 100644 --- a/core/spec/models/spree/order_cancellations_spec.rb +++ b/core/spec/models/spree/order_cancellations_spec.rb @@ -109,6 +109,32 @@ it "adjusts the order" do expect { subject }.to change { order.reload.total }.by(-40.0) end + + context "when cancelling all items in a completed order" do + let!(:order) { create(:completed_order_with_totals, line_items_attributes: [{ quantity: }]) } + + subject { described_class.new(order).short_ship(order.inventory_units) } + + it "updates all of the inventory units" do + expect { subject }.to change { order.inventory_units.canceled.count }.to(quantity) + end + + it "updates the shipment state" do + expect { subject }.to change { order.shipments.first.reload.state }.to("canceled") + end + + context "when the shipment is already shipped" do + let!(:order) { create(:shipped_order, line_items_attributes: [{ quantity: }]) } + + it "updates all of the inventory units" do + expect { subject }.to change { order.inventory_units.canceled.count }.to(quantity) + end + + it "does not update the shipment state" do + expect { subject }.not_to change { order.shipments.first.reload.state }.from("shipped") + end + end + end end it "sends a cancellation email" do From 593d11a3faea9e69b6ed54973e322300f88c8747 Mon Sep 17 00:00:00 2001 From: Adam Mueller Date: Fri, 5 Sep 2025 19:03:30 -0700 Subject: [PATCH 2/2] Ensure shipped shipments are "immutable" At least, in terms of the state. We shouldn't mark shipped shipments as cancelled even if all of the items in the shipment get cancelled. If a shipment has been marked as shipped, then we need to preserve that state. --- core/app/models/spree/shipment.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/app/models/spree/shipment.rb b/core/app/models/spree/shipment.rb index d9326ff83d..2dbb167138 100644 --- a/core/app/models/spree/shipment.rb +++ b/core/app/models/spree/shipment.rb @@ -221,10 +221,10 @@ def determine_state(order) # ready all other cases def recalculate_state self.state = - if order.canceled? || inventory_units.all?(&:canceled?) - "canceled" - elsif shipped? + if shipped? "shipped" + elsif order.canceled? || inventory_units.all?(&:canceled?) + "canceled" elsif !order.can_ship? "pending" elsif can_transition_from_pending_to_ready?