From 4f4fc00549c1b7966ee6f18606fd7a663433cdf7 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 9 Dec 2018 19:29:06 +0000 Subject: [PATCH] Improve code, specs and comments in the shipping_method selection process in the checkout controller --- app/controllers/checkout_controller.rb | 3 +-- app/models/concerns/order_shipping_method.rb | 7 +++++-- spec/models/concerns/order_shipping_method_spec.rb | 7 ++++++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index a0c2c44618..464d87b642 100644 --- a/app/controllers/checkout_controller.rb +++ b/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) diff --git a/app/models/concerns/order_shipping_method.rb b/app/models/concerns/order_shipping_method.rb index 203dc217bb..e8a711f95d 100644 --- a/app/models/concerns/order_shipping_method.rb +++ b/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 diff --git a/spec/models/concerns/order_shipping_method_spec.rb b/spec/models/concerns/order_shipping_method_spec.rb index b01758e746..0165d49b58 100644 --- a/spec/models/concerns/order_shipping_method_spec.rb +++ b/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