From ceaae89dee960f80351df14ee6680e63d83687a7 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 2 Mar 2017 14:10:52 +1100
Subject: [PATCH] Prevent order cycles that are linked to schedules from being
 destroyed

---
 .../admin/order_cycles_controller.rb          | 14 ++++++---
 config/locales/en.yml                         |  4 ++-
 .../admin/order_cycles_controller_spec.rb     | 30 ++++++++++++++-----
 3 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb
index e847a96076..4f5a4e151f 100644
--- a/app/controllers/admin/order_cycles_controller.rb
+++ b/app/controllers/admin/order_cycles_controller.rb
@@ -159,11 +159,17 @@ module Admin
     end
 
     def protect_invalid_destroy
-      begin
-        yield
-      rescue ActiveRecord::InvalidForeignKey
+      # Can't delete if OC is linked to any orders or schedules
+      if @order_cycle.schedules.any?
         redirect_to main_app.admin_order_cycles_url
-        flash[:error] = I18n.t(:order_cycles_no_permission_to_delete_error)
+        flash[:error] = I18n.t('admin.order_cycles.destroy_errors.schedule_present')
+      else
+        begin
+          yield
+        rescue ActiveRecord::InvalidForeignKey
+          redirect_to main_app.admin_order_cycles_url
+          flash[:error] = I18n.t('admin.order_cycles.destroy_errors.orders_present')
+        end
       end
     end
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 2dea4fe9ff..c57ebfed41 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -726,6 +726,9 @@ en:
         customer_instructions_placeholder: Pick-up or delivery notes
         products: Products
         fees: Fees
+      destroy_errors:
+        orders_present: That order cycle has been selected by a customer and cannot be deleted. To prevent customers from accessing it, please close it instead.
+        schedule_present: That order cycle is linked to a schedule and cannot be deleted. Please unlink or delete the schedule first.
     producer_properties:
       index:
         title: Producer Properties
@@ -2099,7 +2102,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using
   order_cycles_email_to_producers_notice: 'Emails to be sent to producers have been queued for sending.'
   order_cycles_no_permission_to_coordinate_error: "None of your enterprises have permission to coordinate an order cycle"
   order_cycles_no_permission_to_create_error: "You don't have permission to create an order cycle coordinated by that enterprise"
-  order_cycles_no_permission_to_delete_error: "That order cycle has been selected by a customer and cannot be deleted. To prevent customers from accessing it, please close it instead."
   js:
     saving: 'Saving...'
     changes_saved: 'Changes saved.'
diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb
index 3c7dd0ae28..b726eeb578 100644
--- a/spec/controllers/admin/order_cycles_controller_spec.rb
+++ b/spec/controllers/admin/order_cycles_controller_spec.rb
@@ -276,17 +276,33 @@ module Admin
 
 
     describe "destroy" do
-      let!(:distributor) { create(:distributor_enterprise, owner: distributor_owner) }
+      let(:distributor) { create(:distributor_enterprise, owner: distributor_owner) }
+      let(:oc) { create(:simple_order_cycle, coordinator: distributor) }
 
-      describe "when an order cycle becomes non-deletable, and we attempt to delete it" do
-        let!(:oc)    { create(:simple_order_cycle, coordinator: distributor) }
+      describe "when an order cycle is deleteable" do
+        it "allows the order_cycle to be destroyed" do
+          spree_get :destroy, id: oc.id
+          expect(OrderCycle.find_by_id(oc.id)).to be nil
+        end
+      end
+
+      describe "when an order cycle becomes non-deletable due to the presence of an order" do
         let!(:order) { create(:order, order_cycle: oc) }
 
-        before { spree_get :destroy, id: oc.id }
-
-        it "displays an error message" do
+        it "displays an error message when we attempt to delete it" do
+          spree_get :destroy, id: oc.id
           expect(response).to redirect_to admin_order_cycles_path
-          expect(flash[:error]).to eq "That order cycle has been selected by a customer and cannot be deleted. To prevent customers from accessing it, please close it instead."
+          expect(flash[:error]).to eq I18n.t('admin.order_cycles.destroy_errors.orders_present')
+        end
+      end
+
+      describe "when an order cycle becomes non-deletable because it is linked to a schedule" do
+        let!(:schedule) { create(:schedule, order_cycles: [oc]) }
+
+        it "displays an error message when we attempt to delete it" do
+          spree_get :destroy, id: oc.id
+          expect(response).to redirect_to admin_order_cycles_path
+          expect(flash[:error]).to eq I18n.t('admin.order_cycles.destroy_errors.schedule_present')
         end
       end
     end