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.
This commit is contained in:
Cillian O'Ruanaidh
2022-09-21 21:51:21 +01:00
committed by Filipe
parent 6542b47e72
commit def87485c0
2 changed files with 19 additions and 7 deletions

View File

@@ -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)

View File

@@ -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