Improve strong params implementation on order_cycle controller and fix corresponding specs

app/controllers/admin/order_cycles_controller.rb

@@ -237,6 +237,8 @@ module Admin
     end
 
     def order_cycle_params
+      return params[:order_cycle] if params[:order_cycle].empty?
+
       params.require(:order_cycle).permit(
         :incoming_exchanges, :outgoing_exchanges,
         :name, :orders_open_at, :orders_close_at, :coordinator_id, :schedule_ids, :coordinator_fee_ids

spec/controllers/admin/order_cycles_controller_spec.rb

@@ -203,7 +203,7 @@ module Admin
 
       context "as a manager of the coordinator" do
         let(:user) { coordinator.owner }
-        let(:expected) { [order_cycle, hash_including(order_cycle: allowed.merge(restricted)), user] }
+        let(:expected) { [order_cycle, hash_including(restricted), user] }
 
         it "allows me to update exchange information for exchanges, name and dates" do
           expect(OrderCycleForm).to receive(:new).with(*expected) { form_mock }
@@ -213,7 +213,7 @@ module Admin
 
       context "as a producer supplying to an order cycle" do
         let(:user) { producer.owner }
-        let(:expected) { [order_cycle, hash_including(order_cycle: allowed), user] }
+        let(:expected) { [order_cycle, {}, user] }
 
         it "allows me to update exchange information for exchanges, but not name or dates" do
           expect(OrderCycleForm).to receive(:new).with(*expected) { form_mock }

spec/services/order_cycle_form_spec.rb

@@ -1,3 +1,5 @@
+require 'spec_helper'
+
 describe OrderCycleForm do
   describe "save" do
     describe "creating a new order cycle from params" do
@@ -6,7 +8,7 @@ describe OrderCycleForm do
       let(:form) { OrderCycleForm.new(order_cycle, params, shop.owner) }
 
       context "when creation is successful" do
-        let(:params) { { order_cycle: { name: "Test Order Cycle", coordinator_id: shop.id } } }
+        let(:params) { { name: "Test Order Cycle", coordinator_id: shop.id } }
 
         it "returns true" do
           expect do
@@ -16,7 +18,7 @@ describe OrderCycleForm do
       end
 
       context "when creation fails" do
-        let(:params) { { order_cycle: { name: "Test Order Cycle" } } }
+        let(:params) { { name: "Test Order Cycle" } }
 
         it "returns false" do
           expect do
@@ -32,7 +34,7 @@ describe OrderCycleForm do
       let(:form) { OrderCycleForm.new(order_cycle, params, shop.owner) }
 
       context "when update is successful" do
-        let(:params) { { order_cycle: { name: "Test Order Cycle", coordinator_id: shop.id } } }
+        let(:params) { { name: "Test Order Cycle", coordinator_id: shop.id } }
 
         it "returns true" do
           expect do
@@ -42,7 +44,7 @@ describe OrderCycleForm do
       end
 
       context "when updating fails" do
-        let(:params) { { order_cycle: { name: nil } } }
+        let(:params) { { name: nil } }
 
         it "returns false" do
           expect do
@@ -73,7 +75,7 @@ describe OrderCycleForm do
       end
 
       context "and I add an schedule that I own, and remove another that I own" do
-        let(:params) { { order_cycle: { schedule_ids: [coordinated_schedule2.id] } } }
+        let(:params) { { schedule_ids: [coordinated_schedule2.id] } }
 
         it "associates the order cycle to the schedule" do
           expect(form.save).to be true
@@ -84,7 +86,7 @@ describe OrderCycleForm do
       end
 
       context "and I add a schedule that I don't own" do
-        let(:params) { { order_cycle: { schedule_ids: [coordinated_schedule.id, uncoordinated_schedule.id] } } }
+        let(:params) { { schedule_ids: [coordinated_schedule.id, uncoordinated_schedule.id] } }
 
         it "ignores the schedule that I don't own" do
           expect(form.save).to be true
@@ -95,7 +97,7 @@ describe OrderCycleForm do
       end
 
       context "when I make no changes to the schedule ids" do
-        let(:params) { { order_cycle: { schedule_ids: [coordinated_schedule.id] } } }
+        let(:params) { { schedule_ids: [coordinated_schedule.id] } }
 
         it "ignores the schedule that I don't own" do
           expect(form.save).to be true
@@ -111,7 +113,7 @@ describe OrderCycleForm do
     let(:order_cycle) { create(:simple_order_cycle) }
     let(:form_applicator_mock) { instance_double(OpenFoodNetwork::OrderCycleFormApplicator) }
     let(:form) { OrderCycleForm.new(order_cycle, params, user) }
-    let(:params) { { order_cycle: { name: 'Some new name' } } }
+    let(:params) { { name: 'Some new name' } }
 
     before do
       allow(OpenFoodNetwork::OrderCycleFormApplicator).to receive(:new) { form_applicator_mock }
@@ -120,7 +122,7 @@ describe OrderCycleForm do
 
     context "when exchange params are provided" do
       let(:exchange_params) { { incoming_exchanges: [], outgoing_exchanges: [] } }
-      before { params[:order_cycle].merge!(exchange_params) }
+      before { params.merge!(exchange_params) }
 
       it "runs the OrderCycleFormApplicator, and saves other changes" do
         expect(form.save).to be true