diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 8cba0d8a0e..464d87b642 100644 --- a/app/controllers/checkout_controller.rb +++ b/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 diff --git a/app/models/concerns/order_shipping_method.rb b/app/models/concerns/order_shipping_method.rb index 75e729f24e..e8a711f95d 100644 --- a/app/models/concerns/order_shipping_method.rb +++ b/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 diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index c5d86ace69..8f3397ac5f 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/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' diff --git a/spec/models/concerns/order_shipping_method_spec.rb b/spec/models/concerns/order_shipping_method_spec.rb index bc05352f20..0165d49b58 100644 --- a/spec/models/concerns/order_shipping_method_spec.rb +++ b/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