Improve code, specs and comments in the shipping_method selection process in the checkout controller

app/controllers/checkout_controller.rb

@@ -22,8 +22,7 @@ class CheckoutController < Spree::CheckoutController
   end
 
   def update
-    shipping_method_id = object_params[:shipping_method_id]
-    object_params.delete(:shipping_method_id)
+    shipping_method_id = object_params.delete(:shipping_method_id)
 
     return update_failed unless @order.update_attributes(object_params)
 

app/models/concerns/order_shipping_method.rb

@@ -19,9 +19,12 @@ module OrderShippingMethod
     shipments.first.shipping_method
   end
 
-  # Finds the shipment shipping_rate for the given shipping_method_id and selects that shipping_rate
+  # 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 [Shipment]
+  # @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

spec/models/concerns/order_shipping_method_spec.rb

@@ -67,12 +67,17 @@ describe OrderShippingMethod do
       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" do
+
+        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