Move create_proposed_shipments to OrderShipping

Spree::OrderShipping is a configurable class, which increases the
extensibility for this system behaviour. The Spree::Order model also
shouldn't need to be responsible for the creation of its own shipments.

Separating this concern will allow us to refactor the underlying stock
coordination implementation without introducing additional churn on the
already heavy Spree::Order class.

Co-authored-by: Adam Mueller<adam@super.gd>

Author: sofiabesenski4

core/app/models/spree/order.rb

@@ -164,6 +164,8 @@ def self.find_by_param!(value)
 
     delegate :recalculate, to: :recalculator
 
+    delegate :create_proposed_shipments, to: :shipping
+
     delegate :name, to: :bill_address, prefix: true, allow_nil: true
     alias_method :billing_name, :bill_address_name
 
@@ -501,17 +503,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)
       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

@@ -1247,49 +1247,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) }