Merge pull request #4002 from luisramos0/more_rails4

Adapting queries in permissions to Rails 4

.rubocop_manual_todo.yml

@@ -151,12 +151,10 @@ Metrics/LineLength:
   - lib/open_food_network/order_and_distributor_report.rb
   - lib/open_food_network/order_cycle_form_applicator.rb
   - lib/open_food_network/order_cycle_management_report.rb
-  - lib/open_food_network/order_cycle_permissions.rb
   - lib/open_food_network/order_grouper.rb
   - lib/open_food_network/orders_and_fulfillments_report.rb
   - lib/open_food_network/payments_report.rb
   - lib/open_food_network/permalink_generator.rb
-  - lib/open_food_network/permissions.rb
   - lib/open_food_network/products_cache.rb
   - lib/open_food_network/products_renderer.rb
   - lib/open_food_network/proxy_order_syncer.rb
@@ -468,6 +466,7 @@ Metrics/AbcSize:
   - lib/open_food_network/orders_and_fulfillments_report.rb
   - lib/open_food_network/packing_report.rb
   - lib/open_food_network/payments_report.rb
+  - lib/open_food_network/permissions.rb
   - lib/open_food_network/products_and_inventory_report.rb
   - lib/open_food_network/reports/line_items.rb
   - lib/open_food_network/sales_tax_report.rb

app/models/enterprise_relationship.rb

@@ -25,8 +25,8 @@ class EnterpriseRelationship < ActiveRecord::Base
     where('parent_id IN (?) OR child_id IN (?)', enterprises, enterprises)
   }
 
-  scope :permitting, ->(enterprises) { where('child_id IN (?)', enterprises) }
-  scope :permitted_by, ->(enterprises) { where('parent_id IN (?)', enterprises) }
+  scope :permitting, ->(enterprise_ids) { where('child_id IN (?)', enterprise_ids) }
+  scope :permitted_by, ->(enterprise_ids) { where('parent_id IN (?)', enterprise_ids) }
 
   scope :with_permission, ->(permission) {
     joins(:permissions).

app/models/spree/product_decorator.rb

@@ -118,7 +118,7 @@ Spree::Product.class_eval do
 
   scope :stockable_by, lambda { |enterprise|
     return where('1=0') if enterprise.blank?
-    permitted_producer_ids = EnterpriseRelationship.joins(:parent).permitting(enterprise)
+    permitted_producer_ids = EnterpriseRelationship.joins(:parent).permitting(enterprise.id)
       .with_permission(:add_to_order_cycle).where(enterprises: { is_primary_producer: true }).pluck(:parent_id)
     return where('spree_products.supplier_id IN (?)', [enterprise.id] | permitted_producer_ids)
   }

app/services/subscription_variants_service.rb

@@ -24,7 +24,7 @@ class SubscriptionVariantsService
 
   def self.permitted_producer_ids(distributor)
     other_permitted_producer_ids = EnterpriseRelationship.joins(:parent)
-      .permitting(distributor).with_permission(:add_to_order_cycle)
+      .permitting(distributor.id).with_permission(:add_to_order_cycle)
       .merge(Enterprise.is_primary_producer)
       .pluck(:parent_id)
 

lib/open_food_network/order_cycle_permissions.rb

@@ -9,66 +9,105 @@ module OpenFoodNetwork
     end
 
     # List of any enterprises whose exchanges I should be able to see in order_cycle
-    # NOTE: the enterprises a given user can see actually in the OC interface depend on the relationships
-    # of their enterprises to the coordinator of the order cycle, rather than on the order cycle itself
+    # NOTE: the enterprises a given user can see actually in the OC interface depend on the
+    # relationships of their enterprises to the coordinator of the OC, rather than on the OC itself
     def visible_enterprises
       return Enterprise.where("1=0") if @coordinator.blank?
       if managed_enterprises.include? @coordinator
-        coordinator_permitted = [@coordinator]
-        all_active = []
+        coordinator_permitted_ids = [@coordinator]
+        all_active_ids = []
 
         if @coordinator.sells == "any"
           # If the coordinator sells any, relationships come into play
-          related_enterprises_granting(:add_to_order_cycle, to: [@coordinator]).pluck(:id).each do |enterprise_id|
-            coordinator_permitted << enterprise_id
+          related_enterprises_granting(:add_to_order_cycle,
+                                       to: [@coordinator.id]).each do |enterprise_id|
+            coordinator_permitted_ids << enterprise_id
           end
 
-          # As a safety net, we should load all of the enterprises invloved in existing exchanges in this order cycle
-          all_active = @order_cycle.suppliers.pluck(:id) | @order_cycle.distributors.pluck(:id)
+          # As a safety net, we load all the enterprises involved in existing exchanges in this OC
+          all_active_ids = @order_cycle.suppliers.pluck(:id) | @order_cycle.distributors.pluck(:id)
         end
 
-        Enterprise.where(id: coordinator_permitted | all_active)
+        Enterprise.where(id: coordinator_permitted_ids | all_active_ids)
       else
         # Any enterprises that I manage directly, which have granted P-OC to the coordinator
-        managed_permitted = related_enterprises_granting(:add_to_order_cycle, to: [@coordinator], scope: managed_participating_enterprises ).pluck(:id)
+        managed_permitted_ids = related_enterprises_granting(
+          :add_to_order_cycle,
+          to: [@coordinator.id],
+          scope: managed_participating_enterprises
+        )
 
         # Any hubs in this OC that have been granted P-OC by producers I manage in this OC
-        hubs_permitted = related_enterprises_granted(:add_to_order_cycle, by: managed_participating_producers, scope: @order_cycle.distributors).pluck(:id)
+        hubs_permitted_ids = related_enterprises_granted(
+          :add_to_order_cycle,
+          by: managed_participating_producers.select("enterprises.id"),
+          scope: @order_cycle.distributors
+        )
 
         # Any hubs in this OC that have granted P-OC to producers I manage in this OC
-        hubs_permitting = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_producers, scope: @order_cycle.distributors).pluck(:id)
+        hubs_permitting_ids = related_enterprises_granting(
+          :add_to_order_cycle,
+          to: managed_participating_producers.select("enterprises.id"),
+          scope: @order_cycle.distributors
+        )
 
         # Any producers in this OC that have been granted P-OC by hubs I manage in this OC
-        producers_permitted = related_enterprises_granted(:add_to_order_cycle, by: managed_participating_hubs, scope: @order_cycle.suppliers).pluck(:id)
+        producers_permitted_ids = related_enterprises_granted(
+          :add_to_order_cycle,
+          by: managed_participating_hubs.select("enterprises.id"),
+          scope: @order_cycle.suppliers
+        )
 
         # Any producers in this OC that have granted P-OC to hubs I manage in this OC
-        producers_permitting = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_hubs, scope: @order_cycle.suppliers).pluck(:id)
+        producers_permitting_ids = related_enterprises_granting(
+          :add_to_order_cycle,
+          to: managed_participating_hubs.select("enterprises.id"),
+          scope: @order_cycle.suppliers
+        )
 
-        managed_active = []
-        hubs_active = []
-        producers_active = []
+        managed_active_ids = []
+        hubs_active_ids = []
+        producers_active_ids = []
         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_participating_enterprises.pluck(:id)
+          managed_active_ids = managed_participating_enterprises.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
-          variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', managed_enterprises.is_primary_producer)
+          # Hubs that currently have outgoing exchanges distributing variants of producers I manage
+          variants = Spree::Variant.
+            joins(:product).
+            where("spree_products.supplier_id IN (?)",
+                  managed_enterprises.is_primary_producer.select("enterprises.id"))
+
           active_exchanges = @order_cycle.
             exchanges.outgoing.with_any_variant(variants.select("spree_variants.id"))
-          hubs_active = active_exchanges.map(&:receiver_id)
+
+          hubs_active_ids = active_exchanges.map(&:receiver_id)
 
           # TODO: Remove this when all P-OC are sorted out
           # Any producers of variants that hubs I manage are currently distributing in this OC
-          variants = Spree::Variant.joins(:exchanges).where("exchanges.receiver_id IN (?) AND exchanges.order_cycle_id = (?) AND exchanges.incoming = 'f'", managed_participating_hubs, @order_cycle).pluck(:id).uniq
-          products = Spree::Product.joins(:variants_including_master).where("spree_variants.id IN (?)", variants).pluck(:id).uniq
-          producers_active = Enterprise.joins(:supplied_products).where("spree_products.id IN (?)", products).pluck(:id).uniq
+          variant_ids = Spree::Variant.joins(:exchanges).
+            where(
+              "exchanges.receiver_id IN (?)
+                AND exchanges.order_cycle_id = (?)
+                AND exchanges.incoming = 'f'",
+              managed_participating_hubs.select("enterprises.id"),
+              @order_cycle
+            ).pluck(:id).uniq
+
+          product_ids = Spree::Product.joins(:variants_including_master).
+            where("spree_variants.id IN (?)", variant_ids).pluck(:id).uniq
+
+          producers_active_ids = Enterprise.joins(:supplied_products).
+            where("spree_products.id IN (?)", product_ids).pluck(:id).uniq
         end
 
-        ids = managed_permitted | hubs_permitted | hubs_permitting | producers_permitted | producers_permitting | managed_active | hubs_active | producers_active
+        ids = managed_permitted_ids | hubs_permitted_ids | hubs_permitting_ids \
+          | producers_permitted_ids | producers_permitting_ids | managed_active_ids \
+          | hubs_active_ids | producers_active_ids
 
-        Enterprise.where(id: ids.sort )
+        Enterprise.where(id: ids)
       end
     end
 
@@ -89,9 +128,12 @@ module OpenFoodNetwork
         # All variants belonging to the producer
         Spree::Variant.joins(:product).where('spree_products.supplier_id = (?)', producer)
       else
-        # 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_participating_hubs).
-          permitted_by(producer).with_permission(:add_to_order_cycle).present?
+        # Producer variants if it has granted P-OC to any of my managed hubs that are in this OC
+        permitted = EnterpriseRelationship.
+          permitting(managed_participating_hubs.select("enterprises.id")).
+          permitted_by(producer.id).
+          with_permission(:add_to_order_cycle).
+          present?
         if permitted
           Spree::Variant.joins(:product).where('spree_products.supplier_id = (?)', producer)
         else
@@ -125,32 +167,45 @@ module OpenFoodNetwork
         # TODO: isn't this completely redundant given the assignment of hub_variants below?
         coordinator_variants = []
         if hub == @coordinator
-          coordinator_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id = (?)', @coordinator)
+          coordinator_variants = Spree::Variant.joins(:product).
+            where('spree_products.supplier_id = (?)', @coordinator)
         end
 
         # Any variants of any producers that have granted the hub P-OC
-        producers = related_enterprises_granting(:add_to_order_cycle, to: [hub], scope: Enterprise.is_primary_producer)
-        permitted_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', producers)
+        producer_ids = related_enterprises_granting(:add_to_order_cycle,
+                                                    to: [hub.id],
+                                                    scope: Enterprise.is_primary_producer)
+        permitted_variants = Spree::Variant.joins(:product).
+          where('spree_products.supplier_id IN (?)', producer_ids)
 
         hub_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id = (?)', hub)
 
-        # PLUS any variants that are already in an outgoing exchange of this hub, so things don't break
+        # PLUS variants that are already in an outgoing exchange of this hub, so things don't break
         # TODO: Remove this when all P-OC are sorted out
         active_variants = []
         @order_cycle.exchanges.outgoing.where(receiver_id: hub).limit(1).each do |exchange|
           active_variants = exchange.variants
         end
 
-        Spree::Variant.where(id: coordinator_variants | hub_variants | permitted_variants | active_variants)
+        Spree::Variant.
+          where(id: coordinator_variants | hub_variants | permitted_variants | active_variants)
       else
-        # Any variants produced by MY PRODUCERS that are in this order cycle, where my producer has granted P-OC to the hub
-        producers = related_enterprises_granting(:add_to_order_cycle, to: [hub], scope: managed_participating_producers)
-        permitted_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', producers)
+        # Variants produced by MY PRODUCERS that are in this OC,
+        #   where my producer has granted P-OC to the hub
+        producer_ids = related_enterprises_granting(:add_to_order_cycle,
+                                                    to: [hub.id],
+                                                    scope: managed_participating_producers)
+        permitted_variants = Spree::Variant.joins(:product).
+          where('spree_products.supplier_id IN (?)', producer_ids)
 
-        # PLUS any of my incoming producers' variants that are already in an outgoing exchange of this hub, so things don't break
-        # TODO: Remove this when all P-OC are sorted out
+        # PLUS my incoming producers' variants that are already in an outgoing exchange of this hub,
+        #   so things don't break. TODO: Remove this when all P-OC are sorted out
         active_variants = Spree::Variant.joins(:exchanges, :product).
-          where("exchanges.receiver_id = (?) AND spree_products.supplier_id IN (?) AND incoming = 'f'", hub, managed_enterprises.is_primary_producer)
+          where("exchanges.receiver_id = (?)
+                  AND spree_products.supplier_id IN (?)
+                  AND incoming = 'f'",
+                hub.id,
+                managed_enterprises.is_primary_producer.select("enterprises.id"))
 
         Spree::Variant.where(id: permitted_variants | active_variants)
       end
@@ -164,30 +219,41 @@ module OpenFoodNetwork
         # Any variants produced by the coordinator, for outgoing exchanges with itself
         coordinator_variants = []
         if hub == @coordinator
-          coordinator_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id = (?)', @coordinator)
+          coordinator_variants = Spree::Variant.joins(:product).
+            where('spree_products.supplier_id = (?)', @coordinator)
         end
 
         # Any variants of any producers that have granted the hub P-OC
-        producers = related_enterprises_granting(:add_to_order_cycle, to: [hub], scope: Enterprise.is_primary_producer)
-        permitted_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', producers)
+        producer_ids = related_enterprises_granting(:add_to_order_cycle,
+                                                    to: [hub.id],
+                                                    scope: Enterprise.is_primary_producer)
+        permitted_variants = Spree::Variant.joins(:product).
+          where('spree_products.supplier_id IN (?)', producer_ids)
 
         hub_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id = (?)', hub)
 
-        # PLUS any variants that are already in an outgoing exchange of this hub, so things don't break
+        # PLUS variants that are already in an outgoing exchange of this hub, so things don't break
         # TODO: Remove this when all P-OC are sorted out
         active_variants = []
         @order_cycle.exchanges.outgoing.where(receiver_id: hub).limit(1).each do |exchange|
           active_variants = exchange.variants
         end
 
-        Spree::Variant.where(id: coordinator_variants | hub_variants | permitted_variants | active_variants)
+        Spree::Variant.
+          where(id: coordinator_variants | hub_variants | permitted_variants | active_variants)
       else
         # Any of my managed producers in this order cycle granted P-OC by the hub
-        granted_producers = related_enterprises_granted(:add_to_order_cycle, by: [hub], scope: managed_participating_producers)
+        granted_producers = related_enterprises_granted(:add_to_order_cycle,
+                                                        by: [hub.id],
+                                                        scope: managed_participating_producers)
 
-        # Any variants produced by MY PRODUCERS that are in this order cycle, where my producer has granted P-OC to the hub
-        granting_producers = related_enterprises_granting(:add_to_order_cycle, to: [hub], scope: granted_producers)
-        permitted_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', granting_producers)
+        # Variants produced by MY PRODUCERS that are in this OC,
+        #   where my producer has granted P-OC to the hub
+        granting_producer_ids = related_enterprises_granting(:add_to_order_cycle,
+                                                             to: [hub.id],
+                                                             scope: granted_producers)
+        permitted_variants = Spree::Variant.joins(:product).
+          where('spree_products.supplier_id IN (?)', granting_producer_ids)
 
         Spree::Variant.where(id: permitted_variants)
       end
@@ -196,12 +262,14 @@ module OpenFoodNetwork
     private
 
     def user_manages_coordinator_or(enterprise)
-      managed_enterprises.pluck(:id).include?(@coordinator.id) || managed_enterprises.pluck(:id).include?(enterprise.id)
+      managed_enterprises.pluck(:id).include?(@coordinator.id) ||
+        managed_enterprises.pluck(:id).include?(enterprise.id)
     end
 
     def managed_participating_enterprises
       return @managed_participating_enterprises unless @managed_participating_enterprises.nil?
-      @managed_participating_enterprises = managed_enterprises.where(id: @order_cycle.suppliers | @order_cycle.distributors)
+      @managed_participating_enterprises = managed_enterprises.
+        where(id: @order_cycle.suppliers | @order_cycle.distributors)
     end
 
     def managed_participating_hubs
@@ -216,45 +284,61 @@ module OpenFoodNetwork
 
     def order_cycle_exchange_ids_involving_my_enterprises
       # Any exchanges that my managed enterprises are involved in directly
-      @order_cycle.exchanges.involving(managed_enterprises).pluck :id
+      @order_cycle.exchanges.involving(managed_enterprises.select("enterprises.id")).pluck :id
     end
 
     def order_cycle_exchange_ids_with_distributable_variants
       # Find my managed hubs in this order cycle
       hubs = managed_participating_hubs
       # Any incoming exchange where the producer has granted P-OC to one or more of those hubs
-      producers = related_enterprises_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
+      producer_ids = related_enterprises_granting(:add_to_order_cycle,
+                                                  to: hubs.select("enterprises.id"),
+                                                  scope: Enterprise.is_primary_producer)
+      permitted_exchange_ids = @order_cycle.
+        exchanges.incoming.where(sender_id: producer_ids).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
+      # For any of my managed hubs in this OC,
+      #   any incoming exchanges supplying variants in my outgoing exchanges
+      variant_ids = Spree::Variant.joins(:exchanges).
+        where("exchanges.receiver_id IN (?)
+                AND exchanges.order_cycle_id = (?)
+                AND exchanges.incoming = 'f'",
+              hubs.select("enterprises.id"),
+              @order_cycle).pluck(:id).uniq
 
-      permitted_exchanges | active_exchanges
+      product_ids = Spree::Product.joins(:variants_including_master).
+        where("spree_variants.id IN (?)", variant_ids).pluck(:id).uniq
+
+      producer_ids = Enterprise.joins(:supplied_products).
+        where("spree_products.id IN (?)", product_ids).pluck(:id).uniq
+
+      active_exchange_ids = @order_cycle.exchanges.incoming.where(sender_id: producer_ids).pluck :id
+
+      permitted_exchange_ids | active_exchange_ids
     end
 
     def order_cycle_exchange_ids_distributing_my_variants
       # Find my producers in this order cycle
-      producers = managed_participating_producers.pluck :id
-      # Any outgoing exchange where the distributor has been granted P-OC by one or more of those producers
-      hubs = related_enterprises_granted(:add_to_order_cycle, by: producers, scope: Enterprise.is_hub)
-      permitted_exchanges = @order_cycle.exchanges.outgoing.where(receiver_id: hubs).pluck :id
+      producer_ids = managed_participating_producers.pluck :id
+      # Outgoing exchanges with distributor that has been granted P-OC by 1 or more of the producers
+      hub_ids = related_enterprises_granted(:add_to_order_cycle,
+                                            by: producer_ids,
+                                            scope: Enterprise.is_hub)
+      permitted_exchange_ids = @order_cycle.exchanges.outgoing.where(receiver_id: hub_ids).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 (?)', producers)
-      active_exchanges = @order_cycle.
+        joins(:product).where('spree_products.supplier_id IN (?)', producer_ids)
+      active_exchange_ids = @order_cycle.
         exchanges.outgoing.with_any_variant(variants.select("spree_variants.id")).pluck :id
 
-      permitted_exchanges | active_exchanges
+      permitted_exchange_ids | active_exchange_ids
     end
   end
 end

lib/open_food_network/permissions.rb

@@ -10,17 +10,17 @@ module OpenFoodNetwork
       end
     end
 
-    # Find enterprises that an admin is allowed to add to an order cycle
+    # Enterprises that an admin is allowed to add to an order cycle
     def visible_enterprises_for_order_reports
       managed_and_related_enterprises_with :add_to_order_cycle
     end
 
+    # Enterprises that the user manages and those that have granted P-OC to managed enterprises
     def visible_enterprises
-      # Return enterprises that the user manages and those that have granted P-OC to managed enterprises
       managed_and_related_enterprises_granting :add_to_order_cycle
     end
 
-    # Find enterprises for which an admin is allowed to edit their profile
+    # Enterprises for which an admin is allowed to edit their profile
     def editable_enterprises
       managed_and_related_enterprises_granting :edit_profile
     end
@@ -43,7 +43,7 @@ module OpenFoodNetwork
       # Permissions granted by create_variant_overrides relationship from producer to hub
       permissions = Hash[
            EnterpriseRelationship.
-             permitting(hubs).
+             permitting(hubs.select("enterprises.id")).
              with_permission(:create_variant_overrides).
              group_by(&:child_id).
              map { |child_id, ers| [child_id, ers.map(&:parent_id)] }
@@ -63,15 +63,20 @@ module OpenFoodNetwork
       # Any orders that I can edit
       editable = editable_orders.pluck(:id)
 
-      # Any orders placed through hubs that my producers have granted P-OC, and which contain my their products
-      # This is pretty complicated but it's looking for order where at least one of my producers has granted
-      # P-OC to the distributor AND the order contains products of at least one of THE SAME producers
-      granted_distributors = related_enterprises_granted(:add_to_order_cycle, by: managed_enterprises.is_primary_producer)
+      # Any orders placed through hubs that my producers have granted P-OC,
+      #   and which contain their products. This is pretty complicated but it's looking for order
+      #   where at least one of my producers has granted P-OC to the distributor
+      #   AND the order contains products of at least one of THE SAME producers
+      granted_distributor_ids = related_enterprises_granted(
+        :add_to_order_cycle,
+        by: managed_enterprises.is_primary_producer.pluck(:id)
+      )
       produced = Spree::Order.with_line_items_variants_and_products_outer.
         where(
           "spree_orders.distributor_id IN (?) AND spree_products.supplier_id IN (?)",
-          granted_distributors,
-          related_enterprises_granting(:add_to_order_cycle, to: granted_distributors).merge(managed_enterprises.is_primary_producer)
+          granted_distributor_ids,
+          related_enterprises_granting(:add_to_order_cycle, to: granted_distributor_ids).
+            merge(managed_enterprises.is_primary_producer)
         ).pluck(:id)
 
       Spree::Order.where(id: editable | produced)
@@ -83,7 +88,9 @@ module OpenFoodNetwork
       managed = Spree::Order.where(distributor_id: managed_enterprises.pluck(:id)).pluck(:id)
 
       # Any order that is placed through an order cycle one of my managed enterprises coordinates
-      coordinated = Spree::Order.where(order_cycle_id: coordinated_order_cycles.pluck(:id)).pluck(:id)
+      coordinated = Spree::Order.
+        where(order_cycle_id: coordinated_order_cycles.pluck(:id)).
+        pluck(:id)
 
       Spree::Order.where(id: managed | coordinated )
     end
@@ -94,7 +101,8 @@ module OpenFoodNetwork
 
       # Any from visible orders, where the product is produced by one of my managed producers
       produced = Spree::LineItem.where(order_id: visible_orders.pluck(:id)).joins(:product).
-        where('spree_products.supplier_id IN (?)', managed_enterprises.is_primary_producer.pluck(:id))
+        where("spree_products.supplier_id IN (?)",
+              managed_enterprises.is_primary_producer.pluck(:id))
 
       Spree::LineItem.where(id: editable | produced)
     end
@@ -105,18 +113,21 @@ module OpenFoodNetwork
 
     def editable_products
       managed_enterprise_products_ids = managed_enterprise_products.pluck :id
-      permitted_enterprise_products_ids = products_supplied_by(
-        related_enterprises_granting(:manage_products).pluck(:id)
-      ).pluck :id
-      Spree::Product.where('spree_products.id IN (?)', managed_enterprise_products_ids | permitted_enterprise_products_ids)
+      permitted_enterprise_products_ids = product_ids_supplied_by(
+        related_enterprises_granting(:manage_products)
+      )
+      Spree::Product.where("spree_products.id IN (?)",
+                           managed_enterprise_products_ids | permitted_enterprise_products_ids)
     end
 
     def visible_products
       managed_enterprise_products_ids = managed_enterprise_products.pluck :id
-      permitted_enterprise_products_ids = products_supplied_by(
-        related_enterprises_granting(:manage_products).pluck(:id) | related_enterprises_granting(:add_to_order_cycle).pluck(:id)
-      ).pluck :id
-      Spree::Product.where('spree_products.id IN (?)', managed_enterprise_products_ids | permitted_enterprise_products_ids)
+      permitted_enterprise_products_ids = product_ids_supplied_by(
+        related_enterprises_granting(:manage_products) \
+          | related_enterprises_granting(:add_to_order_cycle)
+      )
+      Spree::Product.where("spree_products.id IN (?)",
+                           managed_enterprise_products_ids | permitted_enterprise_products_ids)
     end
 
     def managed_product_enterprises
@@ -128,11 +139,15 @@ module OpenFoodNetwork
     end
 
     def editable_schedules
-      Schedule.joins(:order_cycles).where(order_cycles: { id: OrderCycle.managed_by(@user).pluck(:id) }).select("DISTINCT schedules.*")
+      Schedule.joins(:order_cycles).
+        where(order_cycles: { id: OrderCycle.managed_by(@user).pluck(:id) }).
+        select("DISTINCT schedules.*")
     end
 
     def visible_schedules
-      Schedule.joins(:order_cycles).where(order_cycles: { id: OrderCycle.accessible_by(@user).pluck(:id) }).select("DISTINCT schedules.*")
+      Schedule.joins(:order_cycles).
+        where(order_cycles: { id: OrderCycle.accessible_by(@user).pluck(:id) }).
+        select("DISTINCT schedules.*")
     end
 
     def editable_subscriptions
@@ -154,7 +169,7 @@ module OpenFoodNetwork
         Enterprise.scoped
       else
         managed_enterprise_ids = managed_enterprises.pluck :id
-        permitting_enterprise_ids = related_enterprises_granting(permission).pluck :id
+        permitting_enterprise_ids = related_enterprises_granting(permission)
 
         Enterprise.where('id IN (?)', managed_enterprise_ids + permitting_enterprise_ids)
       end
@@ -164,10 +179,11 @@ module OpenFoodNetwork
       if admin?
         Enterprise.scoped
       else
-        managed = managed_enterprises.pluck(:id)
-        granting = related_enterprises_granting(permission).pluck(:id)
-        granted = related_enterprises_granted(permission).pluck(:id)
-        Enterprise.where(id: managed | granting | granted)
+        managed_enterprise_ids = managed_enterprises.pluck(:id)
+        granting_enterprise_ids = related_enterprises_granting(permission)
+        granted_enterprise_ids = related_enterprises_granted(permission)
+        Enterprise.
+          where(id: managed_enterprise_ids | granting_enterprise_ids | granted_enterprise_ids)
       end
     end
 
@@ -183,28 +199,30 @@ module OpenFoodNetwork
 
     def related_enterprises_granting(permission, options = {})
       parent_ids = EnterpriseRelationship.
-        permitting(options[:to] || managed_enterprises).
+        permitting(options[:to] || managed_enterprises.select("enterprises.id")).
         with_permission(permission).
         pluck(:parent_id)
 
-      (options[:scope] || Enterprise).where('enterprises.id IN (?)', parent_ids)
+      (options[:scope] || Enterprise).where('enterprises.id IN (?)', parent_ids).
+        select("enterprises.id")
     end
 
     def related_enterprises_granted(permission, options = {})
       child_ids = EnterpriseRelationship.
-        permitted_by(options[:by] || managed_enterprises).
+        permitted_by(options[:by] || managed_enterprises.select("enterprises.id")).
         with_permission(permission).
         pluck(:child_id)
 
-      (options[:scope] || Enterprise).where('enterprises.id IN (?)', child_ids)
+      (options[:scope] || Enterprise).where('enterprises.id IN (?)', child_ids).
+        select("enterprises.id")
     end
 
     def managed_enterprise_products
       Spree::Product.managed_by(@user)
     end
 
-    def products_supplied_by(suppliers)
-      Spree::Product.where('supplier_id IN (?)', suppliers)
+    def product_ids_supplied_by(supplier_ids)
+      Spree::Product.where('supplier_id IN (?)', supplier_ids).pluck(:id)
     end
   end
 end

spec/lib/open_food_network/permissions_spec.rb

@@ -24,7 +24,7 @@ module OpenFoodNetwork
 
         it "returns only my managed enterprises any that have granting them P-OC" do
           expect(permissions).to receive(:managed_enterprises) { Enterprise.where(id: e1) }
-          expect(permissions).to receive(:related_enterprises_granting).with(:some_permission) { Enterprise.where(id: e3) }
+          expect(permissions).to receive(:related_enterprises_granting).with(:some_permission) { Enterprise.where(id: e3).select(:id) }
           expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to match_array [e1, e3]
         end
       end
@@ -46,8 +46,8 @@ module OpenFoodNetwork
 
         it "returns only my managed enterprises any that have granting them P-OC" do
           expect(permissions).to receive(:managed_enterprises) { Enterprise.where(id: e1) }
-          expect(permissions).to receive(:related_enterprises_granting).with(:some_permission) { Enterprise.where(id: e3) }
-          expect(permissions).to receive(:related_enterprises_granted).with(:some_permission) { Enterprise.where(id: e4) }
+          expect(permissions).to receive(:related_enterprises_granting).with(:some_permission) { Enterprise.where(id: e3).select(:id) }
+          expect(permissions).to receive(:related_enterprises_granted).with(:some_permission) { Enterprise.where(id: e4).select(:id) }
           expect(permissions.send(:managed_and_related_enterprises_with, :some_permission)).to match_array [e1, e3, e4]
         end
       end
@@ -177,7 +177,7 @@ module OpenFoodNetwork
 
       before do
         allow(permissions).to receive(:managed_enterprise_products) { Spree::Product.where('1=0') }
-        allow(permissions).to receive(:related_enterprises_granting).with(:manage_products) { Enterprise.where("1=0") }
+        allow(permissions).to receive(:related_enterprises_granting).with(:manage_products) { Enterprise.where("1=0").select(:id) }
       end
 
       it "returns products produced by managed enterprises" do
@@ -187,7 +187,7 @@ module OpenFoodNetwork
 
       it "returns products produced by permitted enterprises" do
         allow(permissions).to receive(:related_enterprises_granting).
-          with(:manage_products) { Enterprise.where(id: p2.supplier) }
+          with(:manage_products) { Enterprise.where(id: p2.supplier).select(:id) }
         expect(permissions.editable_products).to eq([p2])
       end
     end
@@ -199,8 +199,8 @@ module OpenFoodNetwork
 
       before do
         allow(permissions).to receive(:managed_enterprise_products) { Spree::Product.where("1=0") }
-        allow(permissions).to receive(:related_enterprises_granting).with(:manage_products) { Enterprise.where("1=0") }
-        allow(permissions).to receive(:related_enterprises_granting).with(:add_to_order_cycle) { Enterprise.where("1=0") }
+        allow(permissions).to receive(:related_enterprises_granting).with(:manage_products) { Enterprise.where("1=0").select(:id) }
+        allow(permissions).to receive(:related_enterprises_granting).with(:add_to_order_cycle) { Enterprise.where("1=0").select(:id) }
       end
 
       it "returns products produced by managed enterprises" do
@@ -210,13 +210,13 @@ module OpenFoodNetwork
 
       it "returns products produced by enterprises that have granted manage products" do
         allow(permissions).to receive(:related_enterprises_granting).
-          with(:manage_products) { Enterprise.where(id: p2.supplier) }
+          with(:manage_products) { Enterprise.where(id: p2.supplier).select(:id) }
         expect(permissions.visible_products).to eq([p2])
       end
 
       it "returns products produced by enterprises that have granted P-OC" do
         allow(permissions).to receive(:related_enterprises_granting).
-          with(:add_to_order_cycle) { Enterprise.where(id: p3.supplier) }
+          with(:add_to_order_cycle) { Enterprise.where(id: p3.supplier).select(:id) }
         expect(permissions.visible_products).to eq([p3])
       end
     end
@@ -240,12 +240,12 @@ module OpenFoodNetwork
       let!(:er) { create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [permission]) }
 
       it "returns the enterprises" do
-        allow(permissions).to receive(:managed_enterprises) { e2 }
+        allow(permissions).to receive(:managed_enterprises) { Enterprise.where(id: e2) }
         expect(permissions.send(:related_enterprises_granting, permission)).to eq([e1])
       end
 
       it "returns an empty array when there are none" do
-        allow(permissions).to receive(:managed_enterprises) { e1 }
+        allow(permissions).to receive(:managed_enterprises) { Enterprise.where(id: e1) }
         expect(permissions.send(:related_enterprises_granting, permission)).to eq([])
       end
     end
@@ -253,7 +253,7 @@ module OpenFoodNetwork
     describe "finding enterprises that are managed or with a particular permission" do
       before do
         allow(permissions).to receive(:managed_enterprises) { Enterprise.where('1=0') }
-        allow(permissions).to receive(:related_enterprises_granting) { Enterprise.where('1=0') }
+        allow(permissions).to receive(:related_enterprises_granting) { Enterprise.where('1=0').select(:id) }
         allow(permissions).to receive(:admin?) { false }
       end
 
@@ -264,7 +264,7 @@ module OpenFoodNetwork
 
       it "returns permitted enterprises" do
         expect(permissions).to receive(:related_enterprises_granting).with(permission).
-          and_return(Enterprise.where(id: e2))
+          and_return(Enterprise.where(id: e2).select(:id))
         expect(permissions.send(:managed_and_related_enterprises_granting, permission)).to eq([e2])
       end
     end