Merge pull request #7287 from mkllnk/7166-cancelled-stock

Disallow changes of canceled order

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)

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:

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

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
 

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