From def87485c0e334b6cb093c145b5a43aae87b5a61 Mon Sep 17 00:00:00 2001
From: Cillian O'Ruanaidh <contact@cilliano.com>
Date: Wed, 21 Sep 2022 21:51:21 +0100
Subject: [PATCH] Don't delete schedules on the order_cycles#update action just
 because the :schedule_ids was not specified in the parameters

Before this schedules were being deleted when the Checkout Options step of editing order cycles was saved because this step wasn't sending the :schedule_ids parameter. It's more awkard for API users if they always need to explicity send the :schedule_ids parameter even if they don't want to add/remove any of the schedules, for example if they just want to update an order cycle name.
---
 app/services/order_cycle_form.rb       | 15 ++++++++-------
 spec/services/order_cycle_form_spec.rb | 11 +++++++++++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/app/services/order_cycle_form.rb b/app/services/order_cycle_form.rb
index 68308dd9bb..252c3bc783 100644
--- a/app/services/order_cycle_form.rb
+++ b/app/services/order_cycle_form.rb
@@ -8,6 +8,7 @@ class OrderCycleForm
   def initialize(order_cycle, order_cycle_params, user)
     @order_cycle = order_cycle
     @order_cycle_params = order_cycle_params
+    @specified_params = order_cycle_params.keys
     @user = user
     @permissions = OpenFoodNetwork::Permissions.new(user)
     @schedule_ids = order_cycle_params.delete(:schedule_ids)
@@ -23,7 +24,7 @@ class OrderCycleForm
 
     order_cycle.transaction do
       order_cycle.save!
-      order_cycle.schedule_ids = schedule_ids
+      order_cycle.schedule_ids = schedule_ids if parameter_specified?(:schedule_ids)
       order_cycle.save!
       apply_exchange_changes
       attach_selected_distributor_shipping_methods
@@ -81,12 +82,8 @@ class OrderCycleForm
     @selected_distributor_shipping_method_ids
   end
 
-  def schedule_ids?
-    @schedule_ids.present?
-  end
-
   def build_schedule_ids
-    return unless schedule_ids?
+    return unless parameter_specified?(:schedule_ids)
 
     result = existing_schedule_ids
     result |= (requested_schedule_ids & permitted_schedule_ids) # Add permitted and requested
@@ -95,7 +92,7 @@ class OrderCycleForm
   end
 
   def sync_subscriptions
-    return unless schedule_ids?
+    return unless parameter_specified?(:schedule_ids)
     return unless schedule_sync_required?
 
     OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscriptions_to_sync).sync!
@@ -113,6 +110,10 @@ class OrderCycleForm
     @schedule_ids.map(&:to_i)
   end
 
+  def parameter_specified?(key)
+    @specified_params.map(&:to_s).include?(key.to_s)
+  end
+
   def permitted_schedule_ids
     Schedule.where(id: requested_schedule_ids | existing_schedule_ids)
       .merge(permissions.editable_schedules).pluck(:id)
diff --git a/spec/services/order_cycle_form_spec.rb b/spec/services/order_cycle_form_spec.rb
index 612a90a058..0117a1537a 100644
--- a/spec/services/order_cycle_form_spec.rb
+++ b/spec/services/order_cycle_form_spec.rb
@@ -55,6 +55,17 @@ describe OrderCycleForm do
           end.to_not change{ order_cycle.reload.name }
         end
       end
+
+      context "when schedules are present but updating something other than the :schedule_ids" do
+        let(:params) { { name: "New Order Cycle Name" } }
+        before { create(:schedule, order_cycles: [order_cycle]) }
+
+        it "doesn't delete the schedules" do
+          expect(order_cycle.schedules).to be_present
+          form.save
+          expect(order_cycle.schedules).to be_present
+        end
+      end
     end
   end