Merge pull request #2775 from luisramos0/2-0-checkout-ship-method

[Spree Upgrade] Handle shipping_method_id in checkout

app/controllers/checkout_controller.rb

@@ -22,45 +22,48 @@ class CheckoutController < Spree::CheckoutController
   end
 
   def update
-    if @order.update_attributes(object_params)
-      check_order_for_phantom_fees
-      fire_event('spree.checkout.update')
-      while @order.state != "complete"
-        if @order.state == "payment"
-          return if redirect_to_paypal_express_form_if_needed
-        end
+    shipping_method_id = object_params.delete(:shipping_method_id)
 
-        next if advance_order_state(@order)
+    return update_failed unless @order.update_attributes(object_params)
 
-        if @order.errors.present?
-          flash[:error] = @order.errors.full_messages.to_sentence
-        else
-          flash[:error] = t(:payment_processing_failed)
-        end
-        update_failed
-        return
+    check_order_for_phantom_fees
+    fire_event('spree.checkout.update')
+
+    while @order.state != "complete"
+      if @order.state == "payment"
+        return if redirect_to_paypal_express_form_if_needed
       end
-      if @order.state == "complete" ||  @order.completed?
-        set_default_bill_address
-        set_default_ship_address
 
-        ResetOrderService.new(self, current_order).call
-        session[:access_token] = current_order.token
+      if @order.state == "delivery"
+        @order.select_shipping_method(shipping_method_id)
+      end
 
-        flash[:notice] = t(:order_processed_successfully)
-        respond_to do |format|
-          format.html do
-            respond_with(@order, :location => order_path(@order))
-          end
-          format.json do
-            render json: {path: order_path(@order)}, status: 200
-          end
-        end
+      next if advance_order_state(@order)
+
+      if @order.errors.present?
+        flash[:error] = @order.errors.full_messages.to_sentence
       else
-        update_failed
+        flash[:error] = t(:payment_processing_failed)
       end
-    else
       update_failed
+      return
+    end
+    return update_failed unless @order.state == "complete" ||  @order.completed?
+
+    set_default_bill_address
+    set_default_ship_address
+
+    ResetOrderService.new(self, current_order).call
+    session[:access_token] = current_order.token
+
+    flash[:notice] = t(:order_processed_successfully)
+    respond_to do |format|
+      format.html do
+        respond_with(@order, :location => order_path(@order))
+      end
+      format.json do
+        render json: {path: order_path(@order)}, status: 200
+      end
     end
   end
 

app/models/concerns/order_shipping_method.rb

@@ -18,4 +18,21 @@ module OrderShippingMethod
     return if shipments.empty?
     shipments.first.shipping_method
   end
+
+  # Finds the shipment's shipping_rate for the given shipping_method_id and selects that shipping_rate.
+  # If the selection is successful, it persists it in the database by saving the shipment.
+  # If it fails, it does not clear the current shipping_method selection.
+  #
+  # @return [ShippingMethod] the selected shipping method, or nil if the given shipping_method_id is
+  #   empty or if it cannot find the given shipping_method_id in the order
+  def select_shipping_method(shipping_method_id)
+    return if shipping_method_id.blank? || shipments.empty?
+    shipment = shipments.first
+
+    shipping_rate = shipment.shipping_rates.find_by_shipping_method_id(shipping_method_id)
+    return unless shipping_rate
+
+    shipment.selected_shipping_rate_id=(shipping_rate.id)
+    shipment.shipping_method
+  end
 end

spec/controllers/checkout_controller_spec.rb

@@ -85,6 +85,42 @@ describe CheckoutController, type: :controller do
       controller.send(:clear_ship_address)
     end
 
+    context "#update with shipping_method_id" do
+      let(:test_shipping_method_id) { "111" }
+
+      before do
+        # stub order and resetorderservice
+        allow(ResetOrderService).to receive(:new).with(controller, order) { reset_order_service }
+        allow(reset_order_service).to receive(:call)
+        allow(order).to receive(:update_attributes).and_return true
+        allow(controller).to receive(:current_order).and_return order
+
+        # make order workflow pass through delivery
+        allow(order).to receive(:next).twice do
+          if order.state == 'cart'
+            order.update_column :state, 'delivery'
+          else
+            order.update_column :state, 'complete'
+          end
+        end
+      end
+
+      it "does not fail to update" do
+        expect(controller).to_not receive(:clear_ship_address)
+        spree_post :update, order: {shipping_method_id: test_shipping_method_id}
+      end
+
+      it "does not send shipping_method_id to the order model as an attribute" do
+        expect(order).to receive(:update_attributes).with({})
+        spree_post :update, order: {shipping_method_id: test_shipping_method_id}
+      end
+
+      it "selects the shipping_method in the order" do
+        expect(order).to receive(:select_shipping_method).with(test_shipping_method_id)
+        spree_post :update, order: {shipping_method_id: test_shipping_method_id}
+      end
+    end
+
     context 'when completing the order' do
       before do
         order.state = 'complete'

spec/models/concerns/order_shipping_method_spec.rb

@@ -3,15 +3,15 @@ require 'spec_helper'
 describe OrderShippingMethod do
   let(:order) { create(:order) }
 
-  describe '#shipping_method' do
-    context 'when order has no shipments' do
-      it 'returns nil' do
+  describe "#shipping_method" do
+    context "when order has no shipments" do
+      it "returns nil" do
         expect(order.shipping_method).to be_nil
       end
     end
 
-    context 'when order has single shipment' do
-      it 'returns the shipments shipping_method' do
+    context "when order has single shipment" do
+      it "returns the shipments shipping_method" do
         shipping_method = create(:shipping_method_with, :flat_rate)
         shipment = create(:shipment_with, :shipping_method, shipping_method: shipping_method)
         order.shipments = [shipment]
@@ -20,4 +20,66 @@ describe OrderShippingMethod do
       end
     end
   end
+
+  describe "#select_shipping_method" do
+    let(:shipping_method) { create(:shipping_method_with, :flat_rate) }
+
+    context "when order has no shipment" do
+      it "returns nil" do
+        expect(order.select_shipping_method(shipping_method.id)).to be_nil
+      end
+    end
+
+    context "when order has a shipment" do
+      let(:shipment) { create(:shipment_with, :shipping_method, shipping_method: shipping_method) }
+      before { order.shipments = [shipment] }
+
+      context "when no shipping_method_id is provided" do
+        it "returns nil for nil shipping_method_id" do
+          expect(order.select_shipping_method(nil)).to be_nil
+        end
+
+        it "returns nil for empty shipping_method_id" do
+          empty_shipping_method_id = ' '
+          expect(shipment.shipping_rates).to_not receive(:find_by_shipping_method_id).with(empty_shipping_method_id)
+
+          expect(order.select_shipping_method(empty_shipping_method_id)).to be_nil
+        end
+      end
+
+      context "when shipping_method_id is not valid for the order" do
+        it "returns nil" do
+          invalid_shipping_method_id = order.shipment.shipping_method.id + 1000
+          expect(shipment.shipping_rates).to receive(:find_by_shipping_method_id).with(invalid_shipping_method_id) { nil }
+
+          expect(order.select_shipping_method(invalid_shipping_method_id)).to be_nil
+        end
+      end
+
+      context "when shipping_method_id is valid for the order" do
+        it "returns the shipments shipping_method" do
+          expect(shipment).to receive(:selected_shipping_rate_id=)
+
+          expect(order.select_shipping_method(shipping_method.id)).to eq shipping_method
+        end
+      end
+
+      context "when multiple shipping_methods exist in the shipment" do
+        let(:expensive_shipping_method) { create(:shipping_method_with, :expensive_name) }
+        before { shipment.add_shipping_method(expensive_shipping_method, false ) }
+
+        it "selects a shipping method that was not selected by default and persists the selection in the database" do
+          expect(shipment.shipping_method).to eq shipping_method
+
+          expect(order.select_shipping_method(expensive_shipping_method.id)).to eq expensive_shipping_method
+
+          expect(shipment.shipping_method).to eq expensive_shipping_method
+
+          shipment.reload
+
+          expect(shipment.shipping_method).to eq expensive_shipping_method
+        end
+      end
+    end
+  end
 end