Merge pull request solidusio#6509 from SuperGoodSoft/introduce-extensible-stock-coordinator

Delegate create_proposed_shipments to Spree::OrderShipping

Author: tvdeyen

core/app/models/spree/order.rb

@@ -308,6 +308,14 @@ def all_inventory_units_returned?
       inventory_units.reload.all?(&:returned?)
     end
 
+    # Since this method is called from within the order state machine, we cannot
+    # simply delegate this to :shipping. The state_machines gem uses the arity
+    # of the method to determine if it should pass in the transition as an
+    # argument. The arity of the method is masked when using delegates.
+    def create_proposed_shipments
+      shipping.create_proposed_shipments
+    end
+
     def contents
       @contents ||= Spree::Config.order_contents_class.new(self)
     end
@@ -502,17 +510,6 @@ def billing_address_required?
       Spree::Config.billing_address_required
     end
 
-    def create_proposed_shipments
-      if completed?
-        raise CannotRebuildShipments.new(I18n.t("spree.cannot_rebuild_shipments_order_completed"))
-      elsif shipments.any? { |shipment| !shipment.pending? }
-        raise CannotRebuildShipments.new(I18n.t("spree.cannot_rebuild_shipments_shipments_not_pending"))
-      else
-        shipments.destroy_all
-        shipments.push(*Spree::Config.stock.coordinator_class.new(self).shipments)
-      end
-    end
-
     def create_shipments_for_line_item(line_item)
       inventory_units = Spree::Config.stock.inventory_unit_builder_class.new(self).missing_units_for_line_item(line_item)
 

core/app/models/spree/order_shipping.rb

@@ -8,6 +8,23 @@ def initialize(order)
     @order = order
   end
 
+  # A method that destroys the current shipments on an order, creates new
+  # shipments for the line items, and assigns them to the order.
+  #
+  # @return [Array<Spree::Shipment>] The created shipments
+  # @raise [Spree::Order::CannotRebuildShipments] raised if order is completed
+  #   or there are any non-pending shipments.
+  def create_proposed_shipments
+    if @order.completed?
+      raise Spree::Order::CannotRebuildShipments.new(I18n.t("spree.cannot_rebuild_shipments_order_completed"))
+    elsif @order.shipments.any? { |shipment| !shipment.pending? }
+      raise Spree::Order::CannotRebuildShipments.new(I18n.t("spree.cannot_rebuild_shipments_shipments_not_pending"))
+    else
+      @order.shipments.destroy_all
+      @order.shipments.push(*Spree::Config.stock.coordinator_class.new(@order).shipments)
+    end
+  end
+
   # A shortcut method that ships *all* inventory units in a shipment in a single
   # carton.  See also {#ship}.
   #

core/spec/models/spree/order_shipping_spec.rb

@@ -53,6 +53,50 @@ def emails
     end
   end
 
+  describe "#create_proposed_shipments" do
+    subject(:order) { create(:order) }
+
+    it "assigns the coordinator returned shipments to its shipments" do
+      shipment = build(:shipment)
+      allow_any_instance_of(Spree::Stock::SimpleCoordinator).to receive(:shipments).and_return([shipment])
+      order.shipping.create_proposed_shipments
+      expect(order.shipments).to eq [shipment]
+    end
+
+    it "raises an error if any shipments are ready" do
+      shipment = create(:shipment, order:, state: "ready")
+
+      expect {
+        expect {
+          order.shipping.create_proposed_shipments
+        }.to raise_error(Spree::Order::CannotRebuildShipments)
+      }.not_to change { order.reload.shipments.pluck(:id) }
+
+      expect { shipment.reload }.not_to raise_error
+    end
+
+    it "raises an error if any shipments are shipped" do
+      shipment = create(:shipment, order:, state: "shipped")
+      expect {
+        expect {
+          order.shipping.create_proposed_shipments
+        }.to raise_error(Spree::Order::CannotRebuildShipments)
+      }.not_to change { order.reload.shipments.pluck(:id) }
+
+      expect { shipment.reload }.not_to raise_error
+    end
+
+    context "when the order is already completed" do
+      let(:order) { create(:completed_order_with_pending_payment) }
+
+      it "raises an error" do
+        expect {
+          order.shipping.create_proposed_shipments
+        }.to raise_error(Spree::Order::CannotRebuildShipments)
+      end
+    end
+  end
+
   describe "#ship" do
     subject do
       order.shipping.ship(

core/spec/models/spree/order_spec.rb

@@ -1265,49 +1265,6 @@ def call
     end
   end
 
-  describe "#create_proposed_shipments" do
-    subject(:order) { create(:order) }
-    it "assigns the coordinator returned shipments to its shipments" do
-      shipment = build(:shipment)
-      allow_any_instance_of(Spree::Stock::SimpleCoordinator).to receive(:shipments).and_return([shipment])
-      subject.create_proposed_shipments
-      expect(subject.shipments).to eq [shipment]
-    end
-
-    it "raises an error if any shipments are ready" do
-      shipment = create(:shipment, order: subject, state: "ready")
-
-      expect {
-        expect {
-          subject.create_proposed_shipments
-        }.to raise_error(Spree::Order::CannotRebuildShipments)
-      }.not_to change { subject.reload.shipments.pluck(:id) }
-
-      expect { shipment.reload }.not_to raise_error
-    end
-
-    it "raises an error if any shipments are shipped" do
-      shipment = create(:shipment, order: subject, state: "shipped")
-      expect {
-        expect {
-          subject.create_proposed_shipments
-        }.to raise_error(Spree::Order::CannotRebuildShipments)
-      }.not_to change { subject.reload.shipments.pluck(:id) }
-
-      expect { shipment.reload }.not_to raise_error
-    end
-
-    context "when the order is already completed" do
-      let(:order) { create(:completed_order_with_pending_payment) }
-
-      it "raises an error" do
-        expect {
-          order.create_proposed_shipments
-        }.to raise_error(Spree::Order::CannotRebuildShipments)
-      end
-    end
-  end
-
   describe "#all_inventory_units_returned?" do
     let(:order) { create(:order_with_line_items, line_items_count: 3) }