diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb index e7e7e184eb..c4ed320e17 100644 --- a/app/controllers/api/v0/shipments_controller.rb +++ b/app/controllers/api/v0/shipments_controller.rb @@ -31,10 +31,12 @@ module Api unlock = params[:shipment].delete(:unlock) if unlock == 'yes' - @shipment.fee_adjustment.open + @shipment.fee_adjustment.fire_events(:open) end - @shipment.update(shipment_params[:shipment]) + if @shipment.update(shipment_params) + @order.updater.update_totals_and_states + end if unlock == 'yes' @shipment.fee_adjustment.close @@ -94,7 +96,7 @@ module Api def find_and_update_shipment @shipment = @order.shipments.find_by!(number: params[:id]) - @shipment.update(shipment_params[:shipment]) if shipment_params[:shipment].present? + @shipment.update(shipment_params) @shipment.reload end @@ -113,10 +115,9 @@ module Api end def shipment_params - params.permit( - [:id, :order_id, :variant_id, :quantity, - { shipment: [:tracking, :selected_shipping_rate_id] }] - ) + return {} unless params.has_key? :shipment + + params.require(:shipment).permit(:tracking, :selected_shipping_rate_id) end end end diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb index c432214751..0aa3a932ef 100644 --- a/engines/order_management/app/services/order_management/order/updater.rb +++ b/engines/order_management/app/services/order_management/order/updater.rb @@ -20,6 +20,10 @@ module OrderManagement # associations try to save and then in turn try to call +update!+ again.) def update update_all_adjustments + update_totals_and_states + end + + def update_totals_and_states update_totals if order.completed? diff --git a/spec/controllers/api/v0/shipments_controller_spec.rb b/spec/controllers/api/v0/shipments_controller_spec.rb index b266c1c3e8..9720b8df57 100644 --- a/spec/controllers/api/v0/shipments_controller_spec.rb +++ b/spec/controllers/api/v0/shipments_controller_spec.rb @@ -176,7 +176,88 @@ describe Api::V0::ShipmentsController, type: :controller do end end - context "can transition a shipment from ready to ship" do + describe "#update" do + let!(:distributor) { create(:distributor_enterprise) } + let!(:shipping_method1) { + create(:shipping_method_with, :flat_rate, distributors: [distributor], amount: 10) + } + let!(:shipping_method2) { + create(:shipping_method_with, :flat_rate, distributors: [distributor], amount: 20) + } + let!(:order_cycle) { create(:order_cycle, distributors: [distributor]) } + let!(:order) { + create(:completed_order_with_totals, order_cycle: order_cycle, distributor: distributor) + } + let(:new_shipping_rate) { + order.shipment.shipping_rates.select{ |sr| sr.shipping_method == shipping_method2 }.first + } + let(:params) { + { + id: order.shipment.number, + order_id: order.number, + shipment: { + selected_shipping_rate_id: new_shipping_rate.id + } + } + } + + before do + order.shipments.first.shipping_methods = [shipping_method1, shipping_method2] + order.select_shipping_method(shipping_method1.id) + order.update! + order.update_columns( + payment_total: 60, + payment_state: "paid" + ) + end + + context "when an order has multiple shipping methods available which could be chosen" do + context "changing the selected shipping method" do + it "updates the order's totals and states" do + expect(order.shipment.shipping_method).to eq shipping_method1 + expect(order.shipment.cost).to eq 10 + expect(order.total).to eq 60 # item total is 50, shipping cost is 10 + expect(order.payment_state).to eq "paid" # order is fully paid for + + api_put :update, params + expect(response.status).to eq 200 + + order.reload + + expect(order.shipment.shipping_method).to eq shipping_method2 + expect(order.shipment.cost).to eq 20 + expect(order.total).to eq 70 # item total is 50, shipping cost is 20 + expect(order.payment_state).to eq "balance_due" # total changed, payment is due + end + + context "using the 'unlock' parameter with closed adjustments" do + before do + order.shipment_adjustments.each(&:close) + end + + it "does not update closed adjustments without unlock option" do + params[:shipment][:unlock] = "no" + + expect { + api_put :update, params + expect(response.status).to eq 200 + }.to_not change { order.reload.shipment.fee_adjustment.amount } + end + + it "updates closed adjustments with unlock option selected" do + params[:shipment][:unlock] = "yes" + + expect { + api_put :update, params + expect(response.status).to eq 200 + }.to change { order.reload.shipment.fee_adjustment.amount } + end + end + end + end + end + + context "#ship" do before do allow_any_instance_of(Spree::Order).to receive_messages(paid?: true, complete?: true) # For the shipment notification email diff --git a/spec/factories/shipping_method_factory.rb b/spec/factories/shipping_method_factory.rb index d67a20aaf7..d6957e78bd 100644 --- a/spec/factories/shipping_method_factory.rb +++ b/spec/factories/shipping_method_factory.rb @@ -41,7 +41,8 @@ FactoryBot.define do end trait :flat_rate do - calculator { Calculator::FlatRate.new(preferred_amount: 50.0) } + transient { amount { 50.0 } } + calculator { Calculator::FlatRate.new(preferred_amount: amount) } end trait :expensive_name do