make enterprises_for_order_cycle permissions more correct

lib/open_food_network/permissions.rb

@@ -47,13 +47,19 @@ module OpenFoodNetwork
         Enterprise.where(id: coordinator_permitted)
       else
         # Any enterprises that I manage directly, which have granted P-OC to the coordinator
-        managed_permitted = granting(:add_to_order_cycle, to: [coordinator], scope: managed_enterprises).pluck(:id)
+        managed_permitted = granting(:add_to_order_cycle, to: [coordinator], scope: managed_enterprises_in(order_cycle) ).pluck(:id)
 
-        # 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 hubs in this OC that have been granted P-OC by producers I manage in this OC
+        hubs_permitted = granted(:add_to_order_cycle, by: managed_producers_in(order_cycle), scope: order_cycle.distributors).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)
+        # Any hubs in this OC that have granted P-OC to producers I manage in this OC
+        hubs_permitting = granting(:add_to_order_cycle, to: managed_producers_in(order_cycle), scope: order_cycle.distributors).pluck(:id)
+
+        # Any producers in this OC that have been granted P-OC by hubs I manage in this OC
+        producers_permitted = granted(:add_to_order_cycle, by: managed_hubs_in(order_cycle), scope: order_cycle.suppliers).pluck(:id)
+
+        # Any producers in this OC that have granted P-OC to hubs I manage in this OC
+        producers_permitting = granting(:add_to_order_cycle, to: managed_hubs_in(order_cycle), scope: order_cycle.suppliers).pluck(:id)
 
         managed_active = []
         hubs_active = []
@@ -61,7 +67,7 @@ module OpenFoodNetwork
         if order_cycle
           # TODO: Remove this when all P-OC are sorted out
           # Any enterprises that I manage that are already in the order_cycle
-          managed_active = managed_enterprises.where(id: order_cycle.suppliers | order_cycle.distributors).pluck(:id)
+          managed_active = managed_enterprises_in(order_cycle).pluck(:id)
 
           # TODO: Remove this when all P-OC are sorted out
           # Any hubs that currently have outgoing exchanges distributing variants of producers I manage
@@ -76,7 +82,7 @@ module OpenFoodNetwork
           producers_active = Enterprise.joins(:supplied_products).where("spree_products.id IN (?)", products).pluck(:id).uniq
         end
 
-        ids = managed_permitted | hubs_permitted | producers_permitted | managed_active | hubs_active | producers_active
+        ids = managed_permitted | hubs_permitted | hubs_permitting | producers_permitted | producers_permitting | managed_active | hubs_active | producers_active
 
         Enterprise.where(id: ids.sort )
       end
@@ -261,14 +267,16 @@ module OpenFoodNetwork
       Enterprise.managed_by(@user)
     end
 
+    def managed_enterprises_in(order_cycle)
+      managed_enterprises.where(id: order_cycle.suppliers | order_cycle.distributors)
+    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)
+      managed_enterprises_in(order_cycle).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)
+      managed_enterprises_in(order_cycle).is_primary_producer
     end
 
     def related_enterprises_with(permission)

spec/lib/open_food_network/permissions_spec.rb

@@ -73,19 +73,70 @@ module OpenFoodNetwork
             create(:enterprise_relationship, parent: hub, child: coordinator, permissions_list: [:add_to_order_cycle])
           end
 
-          it "returns my hub" do
-            enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
-            expect(enterprises).to include hub
-            expect(enterprises).to_not include producer, coordinator
+          context "where my hub is in the order cycle" do
+            let!(:ex_outgoing) { create(:exchange, order_cycle: oc, sender: coordinator, receiver: hub, incoming: false) }
+
+            it "returns my hub" do
+              enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+              expect(enterprises).to include hub
+              expect(enterprises).to_not include producer, coordinator
+            end
+
+            context "and has been granted P-OC by a producer" do
+              before do
+                create(:enterprise_relationship, parent: producer, child: hub, permissions_list: [:add_to_order_cycle])
+              end
+
+              context "where the producer is in the order cycle" do
+                let!(:ex_incoming) { create(:exchange, order_cycle: oc, sender: producer, receiver: coordinator, incoming: true) }
+
+                it "returns the producer" do
+                  enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+                  expect(enterprises).to include producer, hub
+                end
+              end
+
+              context "where the producer is not in the order cycle" do
+                # No incoming exchange
+
+                it "does not return the producer" do
+                  enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+                  expect(enterprises).to_not include producer
+                end
+              end
+            end
+
+            context "and has granted P-OC to a producer" do
+              before do
+                create(:enterprise_relationship, parent: hub, child: producer, permissions_list: [:add_to_order_cycle])
+              end
+
+              context "where the producer is in the order cycle" do
+                let!(:ex_incoming) { create(:exchange, order_cycle: oc, sender: producer, receiver: coordinator, incoming: true) }
+
+                it "returns the producer" do
+                  enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+                  expect(enterprises).to include producer, hub
+                end
+              end
+
+              context "where the producer is not in the order cycle" do
+                # No incoming exchange
+
+                it "does not return the producer" do
+                  enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+                  expect(enterprises).to_not include producer
+                end
+              end
+            end
           end
 
-          context "and has also granted P-OC to a producer" do
-            before do
-              create(:enterprise_relationship, parent: hub, child: producer, permissions_list: [:add_to_order_cycle])
-            end
-            it "does not return that producer" do
+          context "where my hub is not in the order cycle" do
+            # No outgoing exchange for my hub
+
+            it "does not return my hub" do
               enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
-              expect(enterprises).to_not include producer
+              expect(enterprises).to_not include hub, producer, coordinator
             end
           end
         end
@@ -104,17 +155,16 @@ module OpenFoodNetwork
               expect(enterprises).to include hub
               expect(enterprises).to_not include producer, coordinator
             end
-          end
 
-          context "and distributes variants distributed by an unmanaged and unpermitted producer" do
-            let!(:ex_outgoing) { create(:exchange, order_cycle: oc, sender: coordinator, receiver: hub, incoming: false) }
-            before { ex_outgoing.variants << create(:variant, product: create(:product, supplier: producer)) }
+            context "and distributes variants distributed by an unmanaged and unpermitted producer" do
+              before { ex.variants << create(:variant, product: create(:product, supplier: producer)) }
 
-            # TODO: update this when we are confident about P-OCs
-            it "returns that producer as well" do
-              enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
-              expect(enterprises).to include producer, hub
-              expect(enterprises).to_not include coordinator
+              # TODO: update this when we are confident about P-OCs
+              it "returns that producer as well" do
+                enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+                expect(enterprises).to include producer, hub
+                expect(enterprises).to_not include coordinator
+              end
             end
           end
         end
@@ -130,20 +180,72 @@ module OpenFoodNetwork
             create(:enterprise_relationship, parent: producer, child: coordinator, permissions_list: [:add_to_order_cycle])
           end
 
-          it "returns my producer, and the coordindator itself" do
-            enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
-            expect(enterprises).to include producer, coordinator
-            expect(enterprises).to_not include hub
-          end
+          context "where my producer is in the order cycle" do
+            let!(:ex_incoming) { create(:exchange, order_cycle: oc, sender: producer, receiver: coordinator, incoming: true) }
 
-          context "and has also granted P-OC to a hub" do
-            before do
-              create(:enterprise_relationship, parent: producer, child: hub, permissions_list: [:add_to_order_cycle])
+            it "returns my producer" do
+              enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+              expect(enterprises).to include producer
+              expect(enterprises).to_not include hub, coordinator
             end
 
-            it "returns that hub as well" do
+            context "and has been granted P-OC by a hub" do
+              before do
+                create(:enterprise_relationship, parent: hub, child: producer, permissions_list: [:add_to_order_cycle])
+              end
+
+              context "where the hub is also in the order cycle" do
+                let!(:ex_outgoing) { create(:exchange, order_cycle: oc, sender: coordinator, receiver: hub, incoming: false) }
+
+                it "returns the hub as well" do
+                  enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+                  expect(enterprises).to include producer, hub
+                  expect(enterprises).to_not include coordinator
+                end
+              end
+
+              context "where the hub is not in the order cycle" do
+                # No outgoing exchange
+
+                it "does not return the hub" do
+                  enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+                  expect(enterprises).to_not include hub
+                end
+              end
+            end
+
+            context "and has granted P-OC to a hub" do
+              before do
+                create(:enterprise_relationship, parent: producer, child: hub, permissions_list: [:add_to_order_cycle])
+              end
+
+              context "where the hub is also in the order cycle" do
+                let!(:ex_outgoing) { create(:exchange, order_cycle: oc, sender: coordinator, receiver: hub, incoming: false) }
+
+                it "returns the hub as well" do
+                  enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+                  expect(enterprises).to include producer, hub
+                  expect(enterprises).to_not include coordinator
+                end
+              end
+
+              context "where the hub is not in the order cycle" do
+                # No outgoing exchange
+
+                it "does not return the hub" do
+                  enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
+                  expect(enterprises).to_not include hub
+                end
+              end
+            end
+          end
+
+          context "where my producer is not in the order cycle" do
+            # No incoming exchange for producer
+
+            it "does not return my producer" do
               enterprises = permissions.order_cycle_enterprises_for(order_cycle: oc)
-              expect(enterprises).to include producer, coordinator, hub
+              expect(enterprises).to_not include hub, producer, coordinator
             end
           end
         end