Hubs can see incoming exchanges of producers who have given them P-OC or whose variants they are actively distributing

app/serializers/api/admin/exchange_serializer.rb

@@ -7,7 +7,7 @@ class Api::Admin::ExchangeSerializer < ActiveModel::Serializer
     permitted = Spree::Variant.where("1=0")
     if object.incoming
       permitted = OpenFoodNetwork::Permissions.new(options[:current_user]).
-      visible_variants_for_incoming_exchanges_between(object.sender, object.receiver)
+      visible_variants_for_incoming_exchanges_between(object.sender, object.receiver, order_cycle: object.order_cycle)
     else
       permitted = OpenFoodNetwork::Permissions.new(options[:current_user]).
       visible_variants_for_outgoing_exchanges_between(object.sender, object.receiver, order_cycle: object.order_cycle)

lib/open_food_network/order_cycle_form_applicator.rb

@@ -108,7 +108,7 @@ module OpenFoodNetwork
     # TODO Need to use editable rather than visible
     def editable_variant_ids_for_incoming_exchange_between(sender, receiver)
       OpenFoodNetwork::Permissions.new(@spree_current_user).
-        visible_variants_for_incoming_exchanges_between(sender, receiver).
+        visible_variants_for_incoming_exchanges_between(sender, receiver, order_cycle: @order_cycle).
         pluck(:id)
     end
 

lib/open_food_network/permissions.rb

@@ -57,6 +57,9 @@ module OpenFoodNetwork
         # Any hubs that have been granted P-OC by producers I manage
         hubs_permitted = granted(:add_to_order_cycle, by: managed_enterprises.is_primary_producer, scope: Enterprise.is_hub).pluck(:id)
 
+        # Any producers that have granted P-OC to hubs I manage
+        producers_permitted = granting(:add_to_order_cycle, to: managed_enterprises.is_hub, scope: Enterprise.is_primary_producer).pluck(:id)
+
         managed_active = []
         hubs_active = []
         if order_cycle
@@ -71,7 +74,7 @@ module OpenFoodNetwork
           hubs_active = active_exchanges.map(&:receiver_id)
         end
 
-        Enterprise.where(id: coordinator_permitted | managed_permitted | managed_active | hubs_permitted | hubs_active)
+        Enterprise.where(id: coordinator_permitted | managed_permitted | managed_active | hubs_permitted | producers_permitted | hubs_active)
       end
     end
 
@@ -118,18 +121,28 @@ module OpenFoodNetwork
 
     # Find the exchanges of an order cycle that an admin can manage
     def order_cycle_exchanges(order_cycle)
-      ids = order_cycle_exchange_ids_involving_my_enterprises(order_cycle) | order_cycle_exchange_ids_distributing_my_variants(order_cycle)
+      ids = order_cycle_exchange_ids_involving_my_enterprises(order_cycle) |
+        order_cycle_exchange_ids_distributing_my_variants(order_cycle) |
+        order_cycle_exchange_ids_with_distributable_variants(order_cycle)
 
       Exchange.where(id: ids, order_cycle_id: order_cycle)
     end
 
     # Find the variants that a user can POTENTIALLY see within incoming exchanges
-    def visible_variants_for_incoming_exchanges_between(producer, coordinator)
+    def visible_variants_for_incoming_exchanges_between(producer, coordinator, options={})
+      return Spree::Variant.where("1=0") unless options[:order_cycle]
       if managed_enterprises.pluck(:id).include?(producer.id) || managed_enterprises.pluck(:id).include?(coordinator.id)
         # All variants belonging to the producer
         Spree::Variant.joins(:product).where('spree_products.supplier_id = (?)', producer)
       else
-        Spree::Variant.where("1=0") # None
+        # All variants of the producer if it has granted P-OC to any of my managed hubs that are in this order cycle
+        permitted = EnterpriseRelationship.permitting(managed_hubs_in(options[:order_cycle])).
+        permitted_by(producer).with_permission(:add_to_order_cycle).present?
+        if permitted
+          Spree::Variant.joins(:product).where('spree_products.supplier_id = (?)', producer)
+        else
+          Spree::Variant.where("1=0")
+        end
       end
     end
 
@@ -158,8 +171,8 @@ module OpenFoodNetwork
 
         Spree::Variant.where(id: coordinator_variants | permitted_variants | active_variants)
       else
-        # Any variants produced by MY PRODUCERS, where my producer has granted P-OC to the hub
-        producers = granting(:add_to_order_cycle, to: [hub], scope: managed_enterprises.is_primary_producer)
+        # Any variants produced by MY PRODUCERS that are in this order cycle, where my producer has granted P-OC to the hub
+        producers = granting(:add_to_order_cycle, to: [hub], scope: managed_producers_in(options[:order_cycle]))
         permitted_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', producers)
 
         # PLUS any of my producers variants that are already in an outgoing exchange of this hub, so things don't break
@@ -201,6 +214,16 @@ module OpenFoodNetwork
       Enterprise.managed_by(@user)
     end
 
+    def managed_hubs_in(order_cycle)
+      Enterprise.with_order_cycles_as_distributor_outer.where("order_cycles.id = (?)", order_cycle.id)
+      .merge(managed_enterprises.is_hub)
+    end
+
+    def managed_producers_in(order_cycle)
+      Enterprise.with_order_cycles_as_supplier_outer.where("order_cycles.id = (?)", order_cycle.id)
+      .merge(managed_enterprises.is_primary_producer)
+    end
+
     def related_enterprises_with(permission)
       parent_ids = EnterpriseRelationship.
         permitting(managed_enterprises).
@@ -241,16 +264,37 @@ module OpenFoodNetwork
       order_cycle.exchanges.involving(managed_enterprises).pluck :id
     end
 
+    def order_cycle_exchange_ids_with_distributable_variants(order_cycle)
+      # Find my managed hubs in this order cycle
+      hubs = managed_hubs_in(order_cycle)
+      # Any incoming exchange where the producer has granted P-OC to one or more of those hubs
+      producers = granting(:add_to_order_cycle, to: hubs, scope: Enterprise.is_primary_producer).pluck :id
+      permitted_exchanges = order_cycle.exchanges.incoming.where(sender_id: producers).pluck :id
+
+      # TODO: remove active_exchanges when we think it is safe to do so
+      # active_exchanges is for backward compatability, before we restricted variants in each
+      # outgoing exchange to those where the producer had granted P-OC to the distributor
+      # For any of my managed hubs in this OC, any incoming exchanges supplying variants in my outgoing exchanges
+      variants = Spree::Variant.joins(:exchanges).where("exchanges.receiver_id IN (?) AND exchanges.order_cycle_id = (?) AND exchanges.incoming = 'f'", hubs, order_cycle).pluck(:id).uniq
+      products = Spree::Product.joins(:variants_including_master).where("spree_variants.id IN (?)", variants).pluck(:id).uniq
+      producers = Enterprise.joins(:supplied_products).where("spree_products.id IN (?)", products).pluck(:id).uniq
+      active_exchanges = order_cycle.exchanges.incoming.where(sender_id: producers).pluck :id
+
+      permitted_exchanges | active_exchanges
+    end
+
     def order_cycle_exchange_ids_distributing_my_variants(order_cycle)
-      # Any outgoing exchange where the distributor has been granted P-OC by one or more of my producers
-      hubs = granted(:add_to_order_cycle, by: managed_enterprises.is_primary_producer, scope: Enterprise.is_hub).pluck(:id)
-      permitted_exchanges = order_cycle.exchanges.outgoing.where(receiver_id: hubs)
+      # Find my producers in this order cycle
+      producers = managed_producers_in(order_cycle).pluck :id
+      # Any outgoing exchange where the distributor has been granted P-OC by one or more of those producers
+      hubs = granted(:add_to_order_cycle, by: producers, scope: Enterprise.is_hub)
+      permitted_exchanges = order_cycle.exchanges.outgoing.where(receiver_id: hubs).pluck :id
 
       # TODO: remove active_exchanges when we think it is safe to do so
       # active_exchanges is for backward compatability, before we restricted variants in each
       # outgoing exchange to those where the producer had granted P-OC to the distributor
       # For any of my managed producers, any outgoing exchanges with their variants
-      variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', managed_enterprises.is_primary_producer)
+      variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', producers)
       active_exchanges = order_cycle.exchanges.outgoing.with_any_variant(variants).pluck :id
 
       permitted_exchanges | active_exchanges

spec/lib/open_food_network/permissions_spec.rb

@@ -45,7 +45,7 @@ module OpenFoodNetwork
             permissions.stub(:managed_enterprises) { Enterprise.where(id: [coordinator]) }
           end
 
-          context "where the coordinator sells own" do
+          context "where the coordinator sells any" do
             it "returns enterprises which have granted P-OC to the coordinator" do
               enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
               expect(enterprises).to include hub
@@ -63,7 +63,7 @@ module OpenFoodNetwork
         end
       end
 
-      context "as a manager of a hub that has granted P-OC to the coordinator" do
+      context "as a manager of a hub" do
         before do
           permissions.stub(:managed_enterprises) { Enterprise.where(id: [hub]) }
         end
@@ -256,64 +256,133 @@ module OpenFoodNetwork
     end
 
     describe "finding exchanges of an order cycle that an admin can manage" do
+      let!(:producer) { create(:supplier_enterprise) }
       let(:oc) { create(:simple_order_cycle) }
-      let!(:ex) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2) }
 
-      before do
-        permissions.stub(:managed_enterprises) { Enterprise.where('1=0') }
-        permissions.stub(:related_enterprises_with) { Enterprise.where('1=0') }
-      end
+      describe "as the manager of the coordinator" do
+        let!(:ex_in) { create(:exchange, order_cycle: oc, sender: producer, receiver: e1, incoming: true) }
+        let!(:ex_out) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2, incoming: false) }
 
-      it "returns exchanges involving enterprises managed by the user" do
-        permissions.stub(:managed_enterprises) { Enterprise.where(id: [e1, e2]) }
-        permissions.order_cycle_exchanges(oc).should == [ex]
-      end
-
-      it "does not return exchanges involving enterprises with E2E permission" do
-        permissions.stub(:related_enterprises_with) { Enterprise.where(id: [e1, e2]) }
-        permissions.order_cycle_exchanges(oc).should == []
-      end
-
-      it "returns exchanges involving only the sender" do
-        permissions.stub(:managed_enterprises) { Enterprise.where(id: [e1]) }
-        permissions.order_cycle_exchanges(oc).should == [ex]
-      end
-
-      it "returns exchanges involving only the receiver" do
-        permissions.stub(:managed_enterprises) { Enterprise.where(id: [e2]) }
-        permissions.order_cycle_exchanges(oc).should == [ex]
-      end
-
-      describe "special permissions for managers of producers" do
-        let!(:producer) { create(:supplier_enterprise) }
         before do
-          ex.incoming = false
-          ex.save
+          permissions.stub(:managed_enterprises) { Enterprise.where(id: [e1]) }
         end
 
-        it "returns outgoing exchanges where the hub has been granted P-OC by a supplier I manage" do
-          permissions.stub(:managed_enterprises) { Enterprise.where(id: [producer]) }
-          create(:enterprise_relationship, parent: producer, child: e2, permissions_list: [:add_to_order_cycle])
+        it "returns all exchanges in the order cycle, regardless of E2E permissions" do
+          permissions.order_cycle_exchanges(oc).should include ex_in, ex_out
+        end
+      end
 
-          permissions.order_cycle_exchanges(oc).should == [ex]
+
+      describe "as the manager of a hub" do
+        let!(:ex_in) { create(:exchange, order_cycle: oc, sender: producer, receiver: e1, incoming: true) }
+
+        before do
+          permissions.stub(:managed_enterprises) { Enterprise.where(id: [e2]) }
         end
 
+        context "where my hub is in the order cycle" do
+          let!(:ex_out) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2, incoming: false) }
 
-        it "does not return outgoing exchanges where only the coordinator has been granted P-OC by a producer I manage" do
-          permissions.stub(:managed_enterprises) { Enterprise.where(id: [producer]) }
-          create(:enterprise_relationship, parent: producer, child: e1, permissions_list: [:add_to_order_cycle])
+          it "returns my hub's outgoing exchange" do
+            permissions.order_cycle_exchanges(oc).should == [ex_out]
+          end
 
-          permissions.order_cycle_exchanges(oc).should == []
+          context "where my hub has been granted P-OC by an incoming producer" do
+            before do
+              create(:enterprise_relationship, parent: producer, child: e2, permissions_list: [:add_to_order_cycle])
+            end
+
+            it "returns the producer's incoming exchange" do
+              permissions.order_cycle_exchanges(oc).should include ex_in
+            end
+          end
+
+          context "where my hub has not been granted P-OC by an incoming producer" do
+            it "returns the producers's incoming exchange, and my own outhoing exchange" do
+              permissions.order_cycle_exchanges(oc).should_not include ex_in
+            end
+          end
+        end
+
+        context "where my hub isn't in the order cycle" do
+          it "does not return the producer's incoming exchanges" do
+            permissions.order_cycle_exchanges(oc).should == []
+          end
         end
 
         # TODO: this is testing legacy behaviour for backwards compatability, remove when behaviour no longer required
-        it "returns outgoing exchanges which include variants produced by a producer I manage" do
-          product = create(:product, supplier: producer)
-          variant = create(:variant, product: product)
-          ex.variants << variant
-          permissions.stub(:managed_enterprises) { Enterprise.where(id: [producer]) }
+        describe "legacy compatability" do
+          context "where my hub's outgoing exchange contains variants of a producer I don't manage and has not given my hub P-OC" do
+            let!(:product) { create(:product, supplier: producer) }
+            let!(:variant) { create(:variant, product: product) }
+            let!(:ex_out) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2, incoming: true) }
+            before { ex_out.variants << variant }
 
-          permissions.order_cycle_exchanges(oc).should == [ex]
+            it "returns incoming exchanges supplying the variants in my outgoing exchange" do
+              permissions.order_cycle_exchanges(oc).should include ex_out
+            end
+          end
+        end
+      end
+
+      describe "as the manager of a producer" do
+        let!(:ex_out) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2, incoming: false) }
+
+        before do
+          permissions.stub(:managed_enterprises) { Enterprise.where(id: [producer]) }
+        end
+
+        context "where my producer supplies to the order cycle" do
+          let!(:ex_in) { create(:exchange, order_cycle: oc, sender: producer, receiver: e1, incoming: true) }
+
+          it "returns my producer's incoming exchange" do
+            permissions.order_cycle_exchanges(oc).should == [ex_in]
+          end
+
+          context "my producer has granted P-OC to an outgoing hub" do
+            before do
+              create(:enterprise_relationship, parent: producer, child: e2, permissions_list: [:add_to_order_cycle])
+            end
+
+            it "returns the hub's outgoing exchange" do
+              permissions.order_cycle_exchanges(oc).should include ex_out
+            end
+          end
+
+          context "my producer has not granted P-OC to an outgoing hub" do
+            it "does not return the hub's outgoing exchange" do
+              permissions.order_cycle_exchanges(oc).should_not include ex_out
+            end
+          end
+        end
+
+        context "where my producer doesn't supply the order cycle" do
+          it "does not return the hub's outgoing exchanges" do
+            permissions.order_cycle_exchanges(oc).should == []
+          end
+        end
+
+        # TODO: this is testing legacy behaviour for backwards compatability, remove when behaviour no longer required
+        describe "legacy compatability" do
+          context "where an outgoing exchange contains variants of a producer I manage" do
+            let!(:product) { create(:product, supplier: producer) }
+            let!(:variant) { create(:variant, product: product) }
+            before { ex_out.variants << variant }
+
+            context "where my producer supplies to the order cycle" do
+              let!(:ex_in) { create(:exchange, order_cycle: oc, sender: producer, receiver: e1, incoming: true) }
+
+              it "returns the outgoing exchange" do
+                permissions.order_cycle_exchanges(oc).should include ex_out
+              end
+            end
+
+            context "where my producer doesn't supply to the order cycle" do
+              it "does not return the outgoing exchange" do
+                permissions.order_cycle_exchanges(oc).should_not include ex_out
+              end
+            end
+          end
         end
       end
     end
@@ -332,7 +401,7 @@ module OpenFoodNetwork
           end
 
           it "returns all variants belonging to the sending producer" do
-            visible = permissions.visible_variants_for_incoming_exchanges_between(producer1, e1)
+            visible = permissions.visible_variants_for_incoming_exchanges_between(producer1, e1, order_cycle: oc)
             expect(visible).to include v1
             expect(visible).to_not include v2
           end
@@ -344,7 +413,7 @@ module OpenFoodNetwork
           end
 
           it "returns all variants belonging to the sending producer" do
-            visible = permissions.visible_variants_for_incoming_exchanges_between(producer1, e1)
+            visible = permissions.visible_variants_for_incoming_exchanges_between(producer1, e1, order_cycle: oc)
             expect(visible).to include v1
             expect(visible).to_not include v2
           end
@@ -356,9 +425,23 @@ module OpenFoodNetwork
             create(:enterprise_relationship, parent: producer1, child: e2, permissions_list: [:add_to_order_cycle])
           end
 
-          it "returns no variants" do
-            visible = permissions.visible_variants_for_incoming_exchanges_between(producer1, e1)
-            expect(visible).to eq []
+          context "where the hub is in the order cycle" do
+            let!(:ex) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2, incoming: false) }
+
+            it "returns variants produced by that producer only" do
+              visible = permissions.visible_variants_for_incoming_exchanges_between(producer1, e1, order_cycle: oc)
+              expect(visible).to include v1
+              expect(visible).to_not include v2
+            end
+          end
+
+          context "where the hub is not in the order cycle" do
+            # No outgoing exchange
+
+            it "does not return variants produced by that producer" do
+              visible = permissions.visible_variants_for_incoming_exchanges_between(producer1, e1, order_cycle: oc)
+              expect(visible).to_not include v1, v2
+            end
           end
         end
       end
@@ -406,7 +489,7 @@ module OpenFoodNetwork
           end
         end
 
-        context "as manager of the outgoing hub" do
+        context "as manager of an outgoing hub" do
           before do
             permissions.stub(:managed_enterprises) { Enterprise.where(id: [e2]) }
             create(:enterprise_relationship, parent: producer1, child: e2, permissions_list: [:add_to_order_cycle])
@@ -432,20 +515,33 @@ module OpenFoodNetwork
           end
         end
 
-        context "as the manager of a producer which has granted P-OC to the outgoing hub" do
+        context "as the manager of a producer which has granted P-OC to an outgoing hub" do
           before do
             permissions.stub(:managed_enterprises) { Enterprise.where(id: [producer1]) }
             create(:enterprise_relationship, parent: producer1, child: e2, permissions_list: [:add_to_order_cycle])
           end
 
-          it "returns all of my produced variants" do
-            visible = permissions.visible_variants_for_outgoing_exchanges_between(e1, e2, order_cycle: oc)
-            expect(visible).to include v1
-            expect(visible).to_not include v2
+          context "where my producer is in the order cycle" do
+            let!(:ex) { create(:exchange, order_cycle: oc, sender: producer1, receiver: e1, incoming: true) }
+
+            it "returns all of my produced variants" do
+              visible = permissions.visible_variants_for_outgoing_exchanges_between(e1, e2, order_cycle: oc)
+              expect(visible).to include v1
+              expect(visible).to_not include v2
+            end
+          end
+
+          context "where my producer isn't in the order cycle" do
+            # No incoming exchange
+
+            it "does not return my variants" do
+              visible = permissions.visible_variants_for_outgoing_exchanges_between(e1, e2, order_cycle: oc)
+              expect(visible).to_not include v1, v2
+            end
           end
         end
 
-        context "as the manager of a producer which has not granted P-OC to the outgoing hub" do
+        context "as the manager of a producer which has not granted P-OC to an outgoing hub" do
           before do
             permissions.stub(:managed_enterprises) { Enterprise.where(id: [producer2]) }
             create(:enterprise_relationship, parent: producer1, child: e2, permissions_list: [:add_to_order_cycle])
@@ -457,7 +553,7 @@ module OpenFoodNetwork
         end
 
         # TODO: for backwards compatability, remove later
-        context "as the manager of a producer which has not granted P-OC to the outgoing hub, but which has variants already in the exchange" do
+        context "as the manager of a producer which has not granted P-OC to an outgoing hub, but which has variants already in the exchange" do
           let!(:ex) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2, incoming: false) }
 
           # This one won't be in the exchange, and so shouldn't be visible