From eef59bbaae4bbdb348bd584632c3ebcc17481509 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Tue, 5 Apr 2022 21:35:53 +0100
Subject: [PATCH] Improve permissions query building

For larger queries and especially where filtering and paginating, these simpler product queries are way more efficient. It cuts out some very large subqueries with large lists of product ids.
---
 lib/open_food_network/permissions.rb          | 22 ++++++-----
 .../lib/open_food_network/permissions_spec.rb | 39 ++++++++++++++++---
 2 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb
index 47620b4d14..27a0312a7f 100644
--- a/lib/open_food_network/permissions.rb
+++ b/lib/open_food_network/permissions.rb
@@ -60,21 +60,23 @@ module OpenFoodNetwork
     end
 
     def editable_products
-      permitted_enterprise_products_ids = product_ids_supplied_by(
-        related_enterprises_granting(:manage_products)
-      )
-      Spree::Product.where(
-        id: managed_enterprise_products.select(:id) | permitted_enterprise_products_ids
+      return Spree::Product.all if admin?
+
+      Spree::Product.where(supplier_id: @user.enterprises).or(
+        Spree::Product.where(supplier_id: related_enterprises_granting(:manage_products))
       )
     end
 
     def visible_products
-      permitted_enterprise_products_ids = product_ids_supplied_by(
-        related_enterprises_granting(:manage_products) |
-          related_enterprises_granting(:add_to_order_cycle)
-      )
+      return Spree::Product.all if admin?
+
       Spree::Product.where(
-        id: managed_enterprise_products.select(:id) | permitted_enterprise_products_ids
+        supplier_id: @user.enterprises
+      ).or(
+        Spree::Product.where(
+          supplier_id: related_enterprises_granting(:manage_products) |
+            related_enterprises_granting(:add_to_order_cycle)
+        )
       )
     end
 
diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb
index aab09eceb8..3eabe3f0ac 100644
--- a/spec/lib/open_food_network/permissions_spec.rb
+++ b/spec/lib/open_food_network/permissions_spec.rb
@@ -187,7 +187,7 @@ module OpenFoodNetwork
       end
     end
 
-    describe "finding editable products" do
+    describe "#editable_products" do
       let!(:p1) { create(:simple_product, supplier: create(:supplier_enterprise) ) }
       let!(:p2) { create(:simple_product, supplier: create(:supplier_enterprise) ) }
 
@@ -199,15 +199,28 @@ module OpenFoodNetwork
       end
 
       it "returns products produced by managed enterprises" do
-        allow(permissions).to receive(:managed_enterprise_products) { Spree::Product.where(id: p1) }
+        allow(user).to receive(:admin?) { false }
+        allow(user).to receive(:enterprises) { [p1.supplier] }
+
         expect(permissions.editable_products).to eq([p1])
       end
 
       it "returns products produced by permitted enterprises" do
+        allow(user).to receive(:admin?) { false }
+        allow(user).to receive(:enterprises) { [] }
         allow(permissions).to receive(:related_enterprises_granting).
-          with(:manage_products) { Enterprise.where(id: p2.supplier).select(:id) }
+          with(:manage_products) { Enterprise.where(id: p2.supplier) }
+
         expect(permissions.editable_products).to eq([p2])
       end
+
+      context "as superadmin" do
+        it "returns all products" do
+          allow(user).to receive(:admin?) { true }
+
+          expect(permissions.editable_products).to include p1, p2
+        end
+      end
     end
 
     describe "finding visible products" do
@@ -226,21 +239,37 @@ module OpenFoodNetwork
       end
 
       it "returns products produced by managed enterprises" do
-        allow(permissions).to receive(:managed_enterprise_products) { Spree::Product.where(id: p1) }
+        allow(user).to receive(:admin?) { false }
+        allow(user).to receive(:enterprises) { Enterprise.where(id: p1.supplier_id) }
+
         expect(permissions.visible_products).to eq([p1])
       end
 
       it "returns products produced by enterprises that have granted manage products" do
+        allow(user).to receive(:admin?) { false }
+        allow(user).to receive(:enterprises) { [] }
         allow(permissions).to receive(:related_enterprises_granting).
-          with(:manage_products) { Enterprise.where(id: p2.supplier).select(:id) }
+          with(:manage_products) { Enterprise.where(id: p2.supplier) }
+
         expect(permissions.visible_products).to eq([p2])
       end
 
       it "returns products produced by enterprises that have granted P-OC" do
+        allow(user).to receive(:admin?) { false }
+        allow(user).to receive(:enterprises) { [] }
         allow(permissions).to receive(:related_enterprises_granting).
           with(:add_to_order_cycle) { Enterprise.where(id: p3.supplier).select(:id) }
+
         expect(permissions.visible_products).to eq([p3])
       end
+
+      context "as superadmin" do
+        it "returns all products" do
+          allow(user).to receive(:admin?) { true }
+
+          expect(permissions.visible_products.to_a).to include p1, p2, p3
+        end
+      end
     end
 
     describe "finding enterprises that we manage products for" do