When an enterprise user saves an order cycle for which it manages only some of the enterprises involved, do not delete the other exchanges

app/controllers/admin/order_cycles_controller.rb

@@ -23,7 +23,7 @@ module Admin
 
       respond_to do |format|
         if @order_cycle.save
-          OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle).go!
+          OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, managed_enterprises).go!
 
           flash[:notice] = 'Your order cycle has been created.'
           format.html { redirect_to admin_order_cycles_path }
@@ -40,7 +40,7 @@ module Admin
 
       respond_to do |format|
         if @order_cycle.update_attributes(params[:order_cycle])
-          OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle).go!
+          OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, managed_enterprises).go!
 
           flash[:notice] = 'Your order cycle has been updated.'
           format.html { redirect_to admin_order_cycles_path }

app/helpers/enterprises_helper.rb

@@ -2,6 +2,10 @@ module EnterprisesHelper
   def current_distributor
     @current_distributor ||= current_order(false).andand.distributor
   end
+
+  def managed_enterprises
+    Enterprise.managed_by(spree_current_user)
+  end
   
   def enterprises_options enterprises
     enterprises.map { |enterprise| [enterprise.name + ": " + enterprise.address.address1 + ", " + enterprise.address.city, enterprise.id.to_i] }

app/models/exchange.rb

@@ -54,6 +54,10 @@ class Exchange < ActiveRecord::Base
     incoming? ? 'supplier' : 'distributor'
   end
 
+  def participant
+    incoming? ? sender : receiver
+  end
+
   def to_h(core_only=false)
     h = attributes.merge({ 'variant_ids' => variant_ids.sort, 'enterprise_fee_ids' => enterprise_fee_ids.sort })
     h.reject! { |k| %w(id order_cycle_id created_at updated_at).include? k } if core_only

lib/open_food_network/order_cycle_form_applicator.rb

@@ -5,8 +5,10 @@ module OpenFoodNetwork
   # translation is more a responsibility of Angular, so I'd be inclined to refactor this class to move
   # as much as possible (if not all) of its logic into Angular.
   class OrderCycleFormApplicator
-    def initialize(order_cycle)
+    # The applicator will only touch exchanges where a permitted enterprise is the participant
+    def initialize(order_cycle, permitted_enterprises=[])
       @order_cycle = order_cycle
+      @permitted_enterprises = permitted_enterprises
     end
 
     def go!
@@ -68,7 +70,7 @@ module OpenFoodNetwork
     end
 
     def destroy_untouched_exchanges
-      untouched_exchanges.each { |exchange| exchange.destroy }
+      with_permission(untouched_exchanges).each(&:destroy)
     end
 
     def untouched_exchanges
@@ -76,6 +78,10 @@ module OpenFoodNetwork
       @order_cycle.exchanges.reject { |ex| touched_exchange_ids.include? ex.id }
     end
 
+    def with_permission(exchanges)
+      exchanges.select { |ex| @permitted_enterprises.include? ex.participant }
+    end
+
 
     def exchange_variant_ids(exchange)
       exchange[:variants].select { |k, v| v }.keys.map { |k| k.to_i }

spec/features/admin/order_cycles_spec.rb

@@ -514,6 +514,15 @@ feature %q{
       # I should not see exchanges for supplier2 or distributor2
       page.all('tr.supplier').count.should == 1
       page.all('tr.distributor').count.should == 1
+
+      # When I save, then those exchanges should remain
+      click_button 'Update'
+      page.should have_content "Your order cycle has been updated."
+
+      oc.reload
+      oc.suppliers.sort.should == [supplier1, supplier2]
+      oc.coordinator.should == supplier1
+      oc.distributors.sort.should == [distributor1, distributor2]
     end
 
     scenario "cloning an order cycle" do

spec/lib/open_food_network/order_cycle_form_applicator_spec.rb

@@ -115,6 +115,18 @@ module OpenFoodNetwork
 
           applicator.send(:untouched_exchanges).should == []
         end
+
+        it "does not destroy exchanges involving enterprises it does not have permission to touch" do
+          applicator = OrderCycleFormApplicator.new(nil)
+          exchanges = double(:exchanges)
+          permitted_exchanges = [double(:exchange), double(:exchange)]
+
+          applicator.should_receive(:with_permission).with(exchanges) { permitted_exchanges }
+          applicator.stub(:untouched_exchanges) { exchanges }
+          permitted_exchanges.each { |ex| ex.should_receive(:destroy) }
+
+          applicator.send(:destroy_untouched_exchanges)
+        end
       end
 
       it "converts exchange variant ids hash to an array of ids" do
@@ -122,6 +134,24 @@ module OpenFoodNetwork
 
         applicator.send(:exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true}}).should == [1, 3]
       end
+
+      describe "filtering exchanges for permission" do
+        it "returns exchanges involving enterprises we have permission to touch" do
+          e = double(:enterprise)
+          ex = double(:exchange, participant: e)
+
+          applicator = OrderCycleFormApplicator.new(nil, [e])
+          applicator.send(:with_permission, [ex]).should == [ex]
+        end
+
+        it "does not return other exchanges" do
+          e = double(:enterprise)
+          ex = double(:exchange, participant: e)
+
+          applicator = OrderCycleFormApplicator.new(nil, [])
+          applicator.send(:with_permission, [ex]).should == []
+        end
+      end
     end
 
     context "integration specs" do

spec/models/exchange_spec.rb

@@ -48,7 +48,7 @@ describe Exchange do
     e.enterprise_fees.count.should == 1
   end
 
-  describe "reporting whether it is an incoming exchange" do
+  describe "exchange directionality" do
     let(:supplier) { create(:supplier_enterprise) }
     let(:coordinator) { create(:distributor_enterprise) }
     let(:distributor) { create(:distributor_enterprise) }
@@ -56,12 +56,24 @@ describe Exchange do
     let(:incoming_exchange) { oc.exchanges.create! sender: supplier, receiver: coordinator, incoming: true }
     let(:outgoing_exchange) { oc.exchanges.create! sender: coordinator, receiver: distributor, incoming: false }
 
-    it "returns true for incoming exchanges" do
-      incoming_exchange.should be_incoming
+    describe "reporting whether it is an incoming exchange" do
+      it "returns true for incoming exchanges" do
+        incoming_exchange.should be_incoming
+      end
+
+      it "returns false for outgoing exchanges" do
+        outgoing_exchange.should_not be_incoming
+      end
     end
 
-    it "returns false for outgoing exchanges" do
-      outgoing_exchange.should_not be_incoming
+    describe "finding the exchange participant (the enterprise other than the coordinator)" do
+      it "returns the sender for incoming exchanges" do
+        incoming_exchange.participant.should == supplier
+      end
+
+      it "returns the receiver for outgoing exchanges" do
+        outgoing_exchange.participant.should == distributor
+      end
     end
   end