From 25525ae30b6d014d04cd37ba3775e587744e8b15 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 9 Mar 2018 12:01:01 +1100 Subject: [PATCH] Move applicator calls to OrderCycleForm --- .../admin/order_cycles_controller.rb | 7 - app/services/order_cycle_form.rb | 16 ++- .../admin/order_cycles_controller_spec.rb | 123 +++++++++--------- spec/services/order_cycle_form_spec.rb | 32 +++++ 4 files changed, 109 insertions(+), 69 deletions(-) diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index ff19519d37..289dd1cf71 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -1,5 +1,3 @@ -require 'open_food_network/order_cycle_form_applicator' - module Admin class OrderCyclesController < ResourceController include OrderCyclesHelper @@ -41,7 +39,6 @@ module Admin @order_cycle_form = OrderCycleForm.new(@order_cycle, params, spree_current_user) if @order_cycle_form.save - OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, spree_current_user).go! flash[:notice] = I18n.t(:order_cycles_create_notice) render json: { success: true } else @@ -53,10 +50,6 @@ module Admin @order_cycle_form = OrderCycleForm.new(@order_cycle, params, spree_current_user) if @order_cycle_form.save - unless params[:order_cycle][:incoming_exchanges].nil? && params[:order_cycle][:outgoing_exchanges].nil? - # Only update apply exchange information if it is actually submmitted - OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, spree_current_user).go! - end flash[:notice] = I18n.t(:order_cycles_update_notice) if params[:reloading] == '1' render json: { :success => true } else diff --git a/app/services/order_cycle_form.rb b/app/services/order_cycle_form.rb index 8bf116869e..d463badfa4 100644 --- a/app/services/order_cycle_form.rb +++ b/app/services/order_cycle_form.rb @@ -1,10 +1,12 @@ require 'open_food_network/permissions' require 'open_food_network/proxy_order_syncer' +require 'open_food_network/order_cycle_form_applicator' class OrderCycleForm def initialize(order_cycle, params, user) @order_cycle = order_cycle @params = params + @user = user @permissions = OpenFoodNetwork::Permissions.new(user) end @@ -14,6 +16,7 @@ class OrderCycleForm return false unless order_cycle.valid? order_cycle.transaction do order_cycle.save! + apply_exchange_changes sync_subscriptions true end @@ -23,7 +26,18 @@ class OrderCycleForm private - attr_accessor :order_cycle, :params, :permissions + attr_accessor :order_cycle, :params, :user, :permissions + + def apply_exchange_changes + return if exchanges_unchanged? + OpenFoodNetwork::OrderCycleFormApplicator.new(order_cycle, user).go! + end + + def exchanges_unchanged? + [:incoming_exchanges, :outgoing_exchanges].all? do |direction| + params[:order_cycle][direction].nil? + end + end def schedule_ids? params[:order_cycle][:schedule_ids].present? diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb index ec2147587f..853ae6a174 100644 --- a/spec/controllers/admin/order_cycles_controller_spec.rb +++ b/spec/controllers/admin/order_cycles_controller_spec.rb @@ -131,6 +131,51 @@ module Admin end describe "update" do + let(:order_cycle) { create(:simple_order_cycle) } + let(:coordinator) { order_cycle.coordinator } + let(:form_mock) { instance_double(OrderCycleForm) } + + before do + allow(OrderCycleForm).to receive(:new) { form_mock } + end + + context "as a manager of the coordinator" do + before { login_as_enterprise_user([coordinator]) } + let(:params) { { format: :json, id: order_cycle.id, order_cycle: {} } } + + context "when updating succeeds" do + before { allow(form_mock).to receive(:save) { true } } + + context "when the page is reloading" do + before { params[:reloading] = '1' } + + it "sets flash message" do + spree_put :update, params + flash[:notice].should == 'Your order cycle has been updated.' + end + end + + context "when the page is not reloading" do + it "does not set flash message" do + spree_put :update, params + flash[:notice].should be nil + end + end + end + + context "when a validation error occurs" do + before { allow(form_mock).to receive(:save) { false } } + + it "returns an error message" do + spree_put :update, params + json_response = JSON.parse(response.body) + expect(json_response['errors']).to be + end + end + end + end + + describe "limiting update scope" do let(:order_cycle) { create(:simple_order_cycle) } let(:producer) { create(:supplier_enterprise) } let(:coordinator) { order_cycle.coordinator } @@ -139,74 +184,30 @@ module Admin let!(:incoming_exchange) { create(:exchange, order_cycle: order_cycle, sender: producer, receiver: coordinator, incoming: true, variants: [v]) } let!(:outgoing_exchange) { create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: hub, incoming: false, variants: [v]) } + let(:allowed) { { incoming_exchanges: [], outgoing_exchanges: [] } } + let(:restricted) { { name: 'some name', orders_open_at: 1.day.from_now, orders_close_at: 1.day.ago } } + let(:params) { { format: :json, id: order_cycle.id, order_cycle: allowed.merge(restricted) } } + let(:form_mock) { instance_double(OrderCycleForm, save: true) } + + before { allow(controller).to receive(:spree_current_user) { user } } + context "as a manager of the coordinator" do - before { login_as_enterprise_user([coordinator]) } + let(:user) { coordinator.owner } + let(:expected) { [order_cycle, hash_including(order_cycle: allowed.merge(restricted)), user] } - it "sets flash message when page is reloading" do - spree_put :update, id: order_cycle.id, reloading: '1', order_cycle: {} - flash[:notice].should == 'Your order cycle has been updated.' - end - - it "does not set flash message otherwise" do - flash[:notice].should be_nil - end - - context "when updating without explicitly submitting exchanges" do - let(:form_applicator_mock) { double(:form_applicator) } - - before do - allow(OpenFoodNetwork::OrderCycleFormApplicator).to receive(:new) { form_applicator_mock } - allow(form_applicator_mock).to receive(:go!) { nil } - end - - it "does not run the OrderCycleFormApplicator" do - expect(order_cycle.exchanges.incoming).to eq [incoming_exchange] - expect(order_cycle.exchanges.outgoing).to eq [outgoing_exchange] - expect(order_cycle.prefers_product_selection_from_coordinator_inventory_only?).to be false - spree_put :update, id: order_cycle.id, order_cycle: { name: 'Some new name', preferred_product_selection_from_coordinator_inventory_only: true } - expect(form_applicator_mock).to_not have_received(:go!) - order_cycle.reload - expect(order_cycle.exchanges.incoming).to eq [incoming_exchange] - expect(order_cycle.exchanges.outgoing).to eq [outgoing_exchange] - expect(order_cycle.name).to eq 'Some new name' - expect(order_cycle.prefers_product_selection_from_coordinator_inventory_only?).to be true - end - end - - context "when a validation error occurs" do - let(:params) { - { - format: :json, - id: order_cycle.id, - order_cycle: { orders_open_at: order_cycle.orders_close_at + 1.day } - } - } - - it "returns an error message" do - spree_put :update, params - json_response = JSON.parse(response.body) - expect(json_response['errors']).to be_present - end + it "allows me to update exchange information for exchanges, name and dates" do + expect(OrderCycleForm).to receive(:new).with(*expected) { form_mock } + spree_put :update, params end end context "as a producer supplying to an order cycle" do - before do - login_as_enterprise_user [producer] - end + let(:user) { producer.owner } + let(:expected) { [order_cycle, hash_including(order_cycle: allowed), user] } - describe "removing a variant from incoming" do - let(:params) do - {order_cycle: { - incoming_exchanges: [{id: incoming_exchange.id, enterprise_id: producer.id, sender_id: producer.id, variants: {v.id => false}}], - outgoing_exchanges: [{id: outgoing_exchange.id, enterprise_id: hub.id, receiver_id: hub.id, variants: {v.id => false}}] } - } - end - - it "removes the variant from outgoing also" do - spree_put :update, {id: order_cycle.id}.merge(params) - Exchange.where(order_cycle_id: order_cycle).with_variant(v).should be_empty - end + it "allows me to update exchange information for exchanges, but not name or dates" do + expect(OrderCycleForm).to receive(:new).with(*expected) { form_mock } + spree_put :update, params end end end diff --git a/spec/services/order_cycle_form_spec.rb b/spec/services/order_cycle_form_spec.rb index 4cd312f63a..476adb3e39 100644 --- a/spec/services/order_cycle_form_spec.rb +++ b/spec/services/order_cycle_form_spec.rb @@ -105,4 +105,36 @@ describe OrderCycleForm do end end end + + describe "updating exchanges" do + let(:user) { instance_double(Spree::User) } + 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' } } } + + before do + allow(OpenFoodNetwork::OrderCycleFormApplicator).to receive(:new) { form_applicator_mock } + allow(form_applicator_mock).to receive(:go!) + end + + context "when exchange params are provided" do + let(:exchange_params) { { incoming_exchanges: [], outgoing_exchanges: [] } } + before { params[:order_cycle].merge!(exchange_params) } + + it "runs the OrderCycleFormApplicator, and saves other changes" do + expect(form.save).to be true + expect(form_applicator_mock).to have_received(:go!) + expect(order_cycle.name).to eq 'Some new name' + end + end + + context "when no exchange params are provided" do + it "does not run the OrderCycleFormApplicator, but saves other changes" do + expect(form.save).to be true + expect(form_applicator_mock).to_not have_received(:go!) + expect(order_cycle.name).to eq 'Some new name' + end + end + end end