From ed40ebace61abde4b14d9f89466d326bcd45547d Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Thu, 18 Feb 2016 09:53:35 +1100
Subject: [PATCH] Existing Exchange Variants must be explicitly set to true by
 form data to remain in an exchange when an order cycle is updated

---
 .../order_cycle_form_applicator.rb            |  9 +++--
 .../order_cycle_form_applicator_spec.rb       | 35 ++++++++++++++++---
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/lib/open_food_network/order_cycle_form_applicator.rb b/lib/open_food_network/order_cycle_form_applicator.rb
index 3e81d6c9f1..6d2e3e3e53 100644
--- a/lib/open_food_network/order_cycle_form_applicator.rb
+++ b/lib/open_food_network/order_cycle_form_applicator.rb
@@ -140,8 +140,13 @@ module OpenFoodNetwork
     end
 
     def persisted_variants_hash(exchange)
-      exchange ||= OpenStruct.new(variants: [])
-      Hash[ exchange.variants.map{ |v| [v.id, true] } ]
+      return {} unless exchange
+
+      # When we have permission to edit a variant, mark it for removal here, assuming it will be included again if that is what the use wants
+      # When we don't have permission to edit a variant and it is already in the exchange, keep it in the exchange.
+      method_name = "editable_variant_ids_for_#{ exchange.incoming? ? 'incoming' : 'outgoing' }_exchange_between"
+      editable = send(method_name, exchange.sender, exchange.receiver)
+      Hash[ exchange.variants.map { |v| [v.id, editable.exclude?(v.id)] } ]
     end
 
     def incoming_exchange_variant_ids(attrs)
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 6cbe6693aa..89dcb364b8 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
@@ -160,12 +160,27 @@ module OpenFoodNetwork
 
           context "when an exchange is passed in" do
             let(:v1) { create(:variant) }
-            let(:exchange) { create(:exchange, variants: [v1]) }
+            let(:v2) { create(:variant) }
+            let(:v3) { create(:variant) }
+            let(:exchange) { create(:exchange, variants: [v1, v2, v3]) }
             let(:hash) { applicator.send(:persisted_variants_hash, exchange) }
 
-            it "returns a hash with variant ids as keys an all values set to true" do
-              expect(hash.length).to be 1
-              expect(hash[v1.id]).to be true
+            before do
+              allow(applicator).to receive(:editable_variant_ids_for_outgoing_exchange_between) { [ v1.id, v2.id ] }
+            end
+
+            it "returns a hash with variant ids as keys" do
+              expect(hash.length).to be 3
+              expect(hash.keys).to include v1.id, v2.id, v3.id
+             end
+
+             it "editable variant ids are set to false" do
+               expect(hash[v1.id]).to be false
+               expect(hash[v2.id]).to be false
+             end
+
+             it "and non-editable variant ids are set to true" do
+               expect(hash[v3.id]).to be true
              end
           end
         end
@@ -209,7 +224,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).
+            allow(applicator).to receive(:editable_variant_ids_for_outgoing_exchange_between).
             with(coordinator_mock, enterprise_mock) { [1, 2, 3] }
           end
 
@@ -234,6 +249,16 @@ module OpenFoodNetwork
             expect(ids).to eq [1, 3]
           end
 
+          it "removes variants which the user has permission to remove and that are not included in the submitted data" do
+            allow(exchange_mock).to receive(:incoming?) { false }
+            allow(exchange_mock).to receive(:variants) { [double(:variant, id: 1), double(:variant, id: 2), double(:variant, id: 3)] }
+            allow(exchange_mock).to receive(:sender) { coordinator_mock }
+            allow(exchange_mock).to receive(:receiver) { enterprise_mock }
+            applicator.stub(:incoming_variant_ids) { [1, 2, 3] }
+            ids = applicator.send(:outgoing_exchange_variant_ids, {:enterprise_id => 123, :variants => {'1' => true, '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} }