diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index b4e2a29017..6d4a4719e3 100644 --- a/.rubocop_manual_todo.yml +++ b/.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 diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index de2ede8440..6acd672f2f 100644 --- a/app/models/enterprise_relationship.rb +++ b/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). diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 2e701eb920..e0ebf327d3 100644 --- a/app/models/spree/product_decorator.rb +++ b/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) } diff --git a/app/services/subscription_variants_service.rb b/app/services/subscription_variants_service.rb index 855c200303..fbdef71e56 100644 --- a/app/services/subscription_variants_service.rb +++ b/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) diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index e49ee2f74f..4d503ed37a 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/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 diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index eec62ed775..ee6f301d80 100644 --- a/lib/open_food_network/permissions.rb +++ b/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 diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 39a15954cb..473a65fb2b 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/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