From 243ef4ee160d0a5dcc81d9bcc0a456d2736e01da Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Tue, 24 Nov 2015 13:46:40 +1100
Subject: [PATCH] When an outgoing exchange includes a variant that is not in
 an incoming exchange, remove it from the outgoing exchange

---
 .../order_cycle_form_applicator.rb            | 18 +++++++++++--
 .../admin/order_cycles_controller_spec.rb     | 26 +++++++++++++++++++
 .../order_cycle_form_applicator_spec.rb       | 10 +++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/lib/open_food_network/order_cycle_form_applicator.rb b/lib/open_food_network/order_cycle_form_applicator.rb
index a2cc95b12f..3d46bf4022 100644
--- a/lib/open_food_network/order_cycle_form_applicator.rb
+++ b/lib/open_food_network/order_cycle_form_applicator.rb
@@ -167,13 +167,27 @@ module OpenFoodNetwork
       sender = @order_cycle.coordinator
       receiver = exchange.andand.receiver || Enterprise.find(attrs[:enterprise_id])
       permitted = editable_variant_ids_for_outgoing_exchange_between(sender, receiver)
+      incoming = incoming_variant_ids
 
       # Only change visibility for variants I have permission to edit
       attrs[:variants].each do |variant_id, value|
-        variants[variant_id.to_i] = value  if permitted.include?(variant_id.to_i)
+        variant_id = variant_id.to_i
+
+        if !incoming.include? variant_id
+          # When a variant has been removed from incoming but remains
+          # in outgoing, remove it from outgoing too
+          variants[variant_id] = false
+
+        elsif permitted.include? variant_id
+          variants[variant_id] = value
+        end
       end
 
-      variants.select { |k, v| v }.keys.map { |k| k.to_i }.sort
+      variants.select { |k, v| v }.keys.map(&:to_i).sort
+    end
+
+    def incoming_variant_ids
+      @order_cycle.supplied_variants.map &:id
     end
   end
 end
diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb
index 12b03f252f..82bf38ecb7 100644
--- a/spec/controllers/admin/order_cycles_controller_spec.rb
+++ b/spec/controllers/admin/order_cycles_controller_spec.rb
@@ -106,6 +106,32 @@ module Admin
         spree_put :update, id: order_cycle.id, reloading: '0', order_cycle: {}
         flash[:notice].should be_nil
       end
+
+      context "as a producer supplying to an order cycle" do
+        let(:producer) { create(:supplier_enterprise) }
+        let(:coordinator) { order_cycle.coordinator }
+        let(:hub) { create(:distributor_enterprise) }
+
+        before { login_as_enterprise_user [producer] }
+
+        describe "removing a variant from incoming" do
+          let(:v) { create(:variant) }
+          let!(:ex_i) { create(:exchange, order_cycle: order_cycle, sender: producer, receiver: coordinator, incoming: true, variants: [v]) }
+          let!(:ex_o) { create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: hub, incoming: false, variants: [v]) }
+
+          let(:params) do
+            {order_cycle: {
+               incoming_exchanges: [{id: ex_i.id, enterprise_id: producer.id, sender_id: producer.id, variants: {v.id => false}}],
+               outgoing_exchanges: [{id: ex_o.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
+        end
+      end
     end
 
     describe "bulk_update" do
diff --git a/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb b/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb
index 0d6c7478fe..6cbe6693aa 100644
--- a/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb
+++ b/spec/lib/open_food_network/order_cycle_form_applicator_spec.rb
@@ -175,6 +175,7 @@ module OpenFoodNetwork
 
           before do
             applicator.stub(:find_outgoing_exchange) { nil }
+            applicator.stub(:incoming_variant_ids) { [1, 2, 3, 4] }
             expect(applicator).to receive(:editable_variant_ids_for_outgoing_exchange_between).
             with(coordinator_mock, enterprise_mock) { [1, 2, 3] }
           end
@@ -207,6 +208,7 @@ module OpenFoodNetwork
 
           before do
             applicator.stub(:find_outgoing_exchange) { exchange_mock }
+            applicator.stub(:incoming_variant_ids) { [1, 2, 3, 4] }
             expect(applicator).to receive(:editable_variant_ids_for_outgoing_exchange_between).
             with(coordinator_mock, enterprise_mock) { [1, 2, 3] }
           end
@@ -231,6 +233,14 @@ module OpenFoodNetwork
             ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true}})
             expect(ids).to eq [1, 3]
           end
+
+          it "removes variants which are not included in incoming exchanges" do
+            applicator.stub(:incoming_variant_ids) { [1, 2] }
+            applicator.stub(:persisted_variants_hash) { {3 => true} }
+            expect(exchange_mock).to receive(:receiver) { enterprise_mock }
+            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '2' => false, '3' => true}})
+            expect(ids).to eq [1]
+          end
         end
       end