diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb index ec452b2869..d144fa96ba 100644 --- a/app/controllers/api/v0/shipments_controller.rb +++ b/app/controllers/api/v0/shipments_controller.rb @@ -8,6 +8,7 @@ module Api respond_to :json before_action :find_order + before_action :refuse_changing_cancelled_orders, only: [:add, :remove] before_action :find_and_update_shipment, only: [:ship, :ready, :add, :remove] def create @@ -95,6 +96,10 @@ module Api @shipment.reload end + def refuse_changing_cancelled_orders + render status: :unprocessable_entity if @order.canceled? + end + def scoped_variant(variant_id) variant = Spree::Variant.find(variant_id) OpenFoodNetwork::ScopeVariantToHub.new(@order.distributor).scope(variant) diff --git a/app/controllers/spree/admin/adjustments_controller.rb b/app/controllers/spree/admin/adjustments_controller.rb index ed6b9d2cde..f4fccde7ca 100644 --- a/app/controllers/spree/admin/adjustments_controller.rb +++ b/app/controllers/spree/admin/adjustments_controller.rb @@ -5,6 +5,7 @@ module Spree prepend_before_action :set_included_tax, only: [:create, :update] before_action :set_order_id, only: [:create, :update] + before_action :skip_changing_canceled_orders, only: [:create, :update] after_action :update_order, only: [:create, :update, :destroy] before_action :set_default_tax_rate, only: :edit before_action :enable_updates, only: :update @@ -28,6 +29,13 @@ module Spree @adjustment.order_id = parent.id end + def skip_changing_canceled_orders + return unless @order.canceled? + + flash[:error] = t("admin.adjustments.skipped_changing_canceled_order") + redirect_to admin_order_adjustments_path(@order) if @order.canceled? + end + # Choose a default tax rate to show on the edit form. The adjustment stores its included # tax in dollars, but doesn't store the source of the tax (ie. TaxRate that generated it). # We guess which tax rate here, choosing: diff --git a/config/locales/en.yml b/config/locales/en.yml index a110afc92e..cbd249e33a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -366,6 +366,8 @@ en: rename: "Rename" admin: + adjustments: + skipped_changing_canceled_order: "You can't change a cancelled order." # Common properties / models begins_at: Begins At begins_on: Begins On diff --git a/spec/controllers/api/v0/shipments_controller_spec.rb b/spec/controllers/api/v0/shipments_controller_spec.rb index 26e6a6b29c..722b821dc5 100644 --- a/spec/controllers/api/v0/shipments_controller_spec.rb +++ b/spec/controllers/api/v0/shipments_controller_spec.rb @@ -65,7 +65,7 @@ describe Api::V0::ShipmentsController, type: :controller do spree_post :create, params - expect(json_response["id"]). to eq(original_shipment_id) + expect(json_response["id"]).to eq(original_shipment_id) expect_valid_response expect(order.shipment.reload.inventory_units.size).to eq 2 expect(order.reload.line_items.first.variant.price).to eq(variant.price) @@ -110,28 +110,69 @@ describe Api::V0::ShipmentsController, type: :controller do expect(response.status).to eq(422) end - context 'for completed shipments' do + describe "#add and #remove" do let(:order) { create :completed_order_with_totals } + let(:line_item) { order.line_items.first } + let(:existing_variant) { line_item.variant } + let(:new_variant) { create(:variant) } + let(:params) { + { + quantity: 2, + order_id: order.to_param, + id: order.shipments.first.to_param + } + } - it 'adds a variant to a shipment' do - api_put :add, variant_id: variant.to_param, - quantity: 2, - order_id: order.to_param, - id: order.shipments.first.to_param - - expect(response.status).to eq(200) - expect(inventory_units_for(variant).size).to eq 2 + before do + line_item.update!(quantity: 3) end - it 'removes a variant from a shipment' do - order.contents.add(variant, 2) - api_put :remove, variant_id: variant.to_param, - quantity: 1, - order_id: order.to_param, - id: order.shipments.first.to_param + context 'for completed shipments' do + it 'adds a variant to a shipment' do + expect { + api_put :add, params.merge(variant_id: new_variant.to_param) + expect(response.status).to eq(200) + }.to change { inventory_units_for(new_variant).size }.by(2) + end - expect(response.status).to eq(200) - expect(inventory_units_for(variant).size).to eq(1) + it 'adjusts stock when adding a variant' do + expect { + api_put :add, params.merge(variant_id: new_variant.to_param) + }.to change { new_variant.reload.on_hand }.by(-2) + end + + it 'removes a variant from a shipment' do + expect { + api_put :remove, params.merge(variant_id: existing_variant.to_param) + expect(response.status).to eq(200) + }.to change { inventory_units_for(existing_variant).size }.by(-2) + end + + it 'adjusts stock when removing a variant' do + expect { + api_put :remove, params.merge(variant_id: existing_variant.to_param) + }.to change { existing_variant.reload.on_hand }.by(2) + end + end + + context "for canceled orders" do + before do + expect(order.cancel).to eq true + end + + it "doesn't adjust stock when adding a variant" do + expect { + api_put :add, params.merge(variant_id: existing_variant.to_param) + expect(response.status).to eq(422) + }.to_not change { existing_variant.reload.on_hand } + end + + it "doesn't adjust stock when removing a variant" do + expect { + api_put :remove, params.merge(variant_id: existing_variant.to_param) + expect(response.status).to eq(422) + }.to_not change { existing_variant.reload.on_hand } + end end end diff --git a/spec/controllers/spree/admin/adjustments_controller_spec.rb b/spec/controllers/spree/admin/adjustments_controller_spec.rb index 87b372c3b9..8dafb83493 100644 --- a/spec/controllers/spree/admin/adjustments_controller_spec.rb +++ b/spec/controllers/spree/admin/adjustments_controller_spec.rb @@ -90,5 +90,33 @@ module Spree end end end + + describe "with a cancelled order" do + let(:order) { create(:completed_order_with_totals) } + let(:tax_rate) { create(:tax_rate, amount: 0.1, calculator: ::Calculator::DefaultTax.new) } + let(:adjustment) { + create(:adjustment, adjustable: order, order: order, amount: 1100, included_tax: 100) + } + + before do + expect(order.cancel).to eq true + end + + it "doesn't create adjustments" do + expect { + spree_post :create, order_id: order.number, adjustment: { label: "Testing", amount: "110" }, tax_rate_id: "" + }.to_not change { [Adjustment.count, order.reload.total] } + + expect(response).to redirect_to spree.admin_order_adjustments_path(order) + end + + it "doesn't change adjustments" do + expect { + spree_put :update, order_id: order.number, id: adjustment.id, adjustment: { label: "Testing", amount: "110" }, tax_rate_id: "" + }.to_not change { [adjustment.reload.amount, order.reload.total] } + + expect(response).to redirect_to spree.admin_order_adjustments_path(order) + end + end end end