From 5184fa540cf96658c9bc0325e71614f7fc5a2e31 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 6 Apr 2019 21:06:09 +0100 Subject: [PATCH 1/8] Adapt enterprise_relationship permitting and permitted_by scopes to rails 4 --- app/models/enterprise_relationship.rb | 4 ++-- app/models/spree/product_decorator.rb | 2 +- app/services/subscription_variants_service.rb | 2 +- lib/open_food_network/order_cycle_permissions.rb | 4 ++-- lib/open_food_network/permissions.rb | 7 ++++--- 5 files changed, 10 insertions(+), 9 deletions(-) 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..9a3d33e3af 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -90,8 +90,8 @@ module OpenFoodNetwork 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? + permitted = EnterpriseRelationship.permitting(managed_participating_hubs.select(: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 diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index eec62ed775..b1283c68a6 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -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(:id)). with_permission(:create_variant_overrides). group_by(&:child_id). map { |child_id, ers| [child_id, ers.map(&:parent_id)] } @@ -183,7 +183,8 @@ module OpenFoodNetwork def related_enterprises_granting(permission, options = {}) parent_ids = EnterpriseRelationship. - permitting(options[:to] || managed_enterprises). + # this options[:to].id needs to be verified/tested + permitting(options[:to].select(:id) || managed_enterprises.select(:id)). with_permission(permission). pluck(:parent_id) @@ -192,7 +193,7 @@ module OpenFoodNetwork def related_enterprises_granted(permission, options = {}) child_ids = EnterpriseRelationship. - permitted_by(options[:by] || managed_enterprises). + permitted_by(options[:by].select(:id) || managed_enterprises.select(:id)). with_permission(permission). pluck(:child_id) From a82b1d81292b8eb016702691e41952ef3cfc6817 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 6 Apr 2019 22:17:00 +0100 Subject: [PATCH 2/8] Adapt exchange.involving scope to rails 4 --- lib/open_food_network/order_cycle_permissions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index 9a3d33e3af..654bf29220 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -216,7 +216,7 @@ 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(:id)).pluck :id end def order_cycle_exchange_ids_with_distributable_variants From a57a93d414cdc915fbe45a0ef895f41a05aa3454 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 6 Apr 2019 23:42:37 +0100 Subject: [PATCH 3/8] Adapt permissions.rb and order_cycle_permissions to rails 4 --- .../order_cycle_permissions.rb | 84 +++++++++---------- lib/open_food_network/permissions.rb | 39 +++++---- .../lib/open_food_network/permissions_spec.rb | 26 +++--- 3 files changed, 74 insertions(+), 75 deletions(-) diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index 654bf29220..45bbd2192b 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -14,61 +14,61 @@ module OpenFoodNetwork 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) + 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(: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(: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(: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(:id), scope: @order_cycle.suppliers) - managed_active = [] - hubs_active = [] + managed_active_ids = [] + hubs_active_ids = [] producers_active = [] 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) + variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', managed_enterprises.is_primary_producer.select(:id)) active_exchanges = @order_cycle. exchanges.outgoing.with_any_variant(variants.select("spree_variants.id")) hubs_active = 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(:id), @order_cycle).pluck(:id).uniq + product_ids = Spree::Product.joins(:variants_including_master).where("spree_variants.id IN (?)", variant_ids).pluck(:id).uniq + active_producer_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 | active_producer_ids - Enterprise.where(id: ids.sort ) + Enterprise.where(id: ids) end end @@ -129,8 +129,8 @@ module OpenFoodNetwork 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) @@ -144,13 +144,13 @@ module OpenFoodNetwork 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) + 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 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(:id)) Spree::Variant.where(id: permitted_variants | active_variants) end @@ -168,8 +168,8 @@ module OpenFoodNetwork 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) @@ -183,11 +183,11 @@ module OpenFoodNetwork 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) + 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 @@ -223,38 +223,38 @@ module OpenFoodNetwork # 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(: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 + variant_ids = Spree::Variant.joins(:exchanges).where("exchanges.receiver_id IN (?) AND exchanges.order_cycle_id = (?) AND exchanges.incoming = 'f'", hubs.select(:id), @order_cycle).pluck(:id).uniq + 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_exchanges | active_exchanges + 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 + producer_ids = 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 + 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) + joins(:product).where('spree_products.supplier_id IN (?)', producer_ids) active_exchanges = @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 b1283c68a6..36c18bcc07 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -66,12 +66,12 @@ module OpenFoodNetwork # 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) + 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) @@ -105,17 +105,17 @@ 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 + 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 + 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 @@ -154,7 +154,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 +164,10 @@ 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,8 +183,7 @@ module OpenFoodNetwork def related_enterprises_granting(permission, options = {}) parent_ids = EnterpriseRelationship. - # this options[:to].id needs to be verified/tested - permitting(options[:to].select(:id) || managed_enterprises.select(:id)). + permitting(options[:to] || managed_enterprises.select(:id)). with_permission(permission). pluck(:parent_id) @@ -193,19 +192,19 @@ module OpenFoodNetwork def related_enterprises_granted(permission, options = {}) child_ids = EnterpriseRelationship. - permitted_by(options[:by].select(:id) || managed_enterprises.select(:id)). + permitted_by(options[:by] || managed_enterprises.select(: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(: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 From e5f396f97500aa73fe54d59b8354731493a096ab Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 3 Jul 2019 15:23:50 +0100 Subject: [PATCH 4/8] Fix Permissions.related_enterprises_granted by adding explicit reference to table --- lib/open_food_network/permissions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 36c18bcc07..22e06ab134 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -196,7 +196,7 @@ module OpenFoodNetwork with_permission(permission). pluck(:child_id) - (options[:scope] || Enterprise).where('enterprises.id IN (?)', child_ids).select(:id) + (options[:scope] || Enterprise).where('enterprises.id IN (?)', child_ids).select("enterprises.id") end def managed_enterprise_products From bb9c54a44510ffb88b9e72e4bef6ac08c708d542 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 3 Jul 2019 22:03:40 +0100 Subject: [PATCH 5/8] Use enterprises.id instead of :id to remove ambiguous column errors --- .../order_cycle_permissions.rb | 24 +++++++++---------- lib/open_food_network/permissions.rb | 8 +++---- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index 45bbd2192b..2ba364c5b4 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -33,16 +33,16 @@ module OpenFoodNetwork 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_ids = related_enterprises_granted(:add_to_order_cycle, by: managed_participating_producers.select(:id), scope: @order_cycle.distributors) + 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_ids = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_producers.select(:id), scope: @order_cycle.distributors) + 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_ids = related_enterprises_granted(:add_to_order_cycle, by: managed_participating_hubs.select(:id), scope: @order_cycle.suppliers) + 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_ids = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_hubs.select(:id), scope: @order_cycle.suppliers) + producers_permitting_ids = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_hubs.select("enterprises.id"), scope: @order_cycle.suppliers) managed_active_ids = [] hubs_active_ids = [] @@ -54,14 +54,14 @@ module OpenFoodNetwork # 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.select(:id)) + 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) # TODO: Remove this when all P-OC are sorted out # Any producers of variants that hubs I manage are currently distributing in this OC - variant_ids = Spree::Variant.joins(:exchanges).where("exchanges.receiver_id IN (?) AND exchanges.order_cycle_id = (?) AND exchanges.incoming = 'f'", managed_participating_hubs.select(:id), @order_cycle).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 active_producer_ids = Enterprise.joins(:supplied_products).where("spree_products.id IN (?)", product_ids).pluck(:id).uniq end @@ -90,7 +90,7 @@ module OpenFoodNetwork 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.select(:id)). + 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) @@ -150,7 +150,7 @@ module OpenFoodNetwork # 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 active_variants = Spree::Variant.joins(:exchanges, :product). - where("exchanges.receiver_id = (?) AND spree_products.supplier_id IN (?) AND incoming = 'f'", hub.id, managed_enterprises.is_primary_producer.select(:id)) + 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 @@ -216,21 +216,21 @@ 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.select(:id)).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 - producer_ids = related_enterprises_granting(:add_to_order_cycle, to: hubs.select(:id), scope: Enterprise.is_primary_producer) + 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 - variant_ids = Spree::Variant.joins(:exchanges).where("exchanges.receiver_id IN (?) AND exchanges.order_cycle_id = (?) AND exchanges.incoming = 'f'", hubs.select(:id), @order_cycle).pluck(:id).uniq + 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 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 @@ -251,7 +251,7 @@ module OpenFoodNetwork # For any of my managed producers, any outgoing exchanges with their variants variants = Spree::Variant. joins(:product).where('spree_products.supplier_id IN (?)', producer_ids) - active_exchanges = @order_cycle. + active_exchange_ids = @order_cycle. exchanges.outgoing.with_any_variant(variants.select("spree_variants.id")).pluck :id permitted_exchange_ids | active_exchange_ids diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 22e06ab134..63d4e1fbeb 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -43,7 +43,7 @@ module OpenFoodNetwork # Permissions granted by create_variant_overrides relationship from producer to hub permissions = Hash[ EnterpriseRelationship. - permitting(hubs.select(:id)). + permitting(hubs.select("enterprises.id")). with_permission(:create_variant_overrides). group_by(&:child_id). map { |child_id, ers| [child_id, ers.map(&:parent_id)] } @@ -183,16 +183,16 @@ module OpenFoodNetwork def related_enterprises_granting(permission, options = {}) parent_ids = EnterpriseRelationship. - permitting(options[:to] || managed_enterprises.select(:id)). + 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.select(:id)). + permitted_by(options[:by] || managed_enterprises.select("enterprises.id")). with_permission(permission). pluck(:child_id) From dee1c3d1399954bca2f317d2576f749d9e3b61e0 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 3 Jul 2019 22:58:33 +0100 Subject: [PATCH 6/8] Fix typo in order_cycle_permissions.rb --- lib/open_food_network/order_cycle_permissions.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index 2ba364c5b4..1ffcee880b 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -46,7 +46,7 @@ module OpenFoodNetwork managed_active_ids = [] hubs_active_ids = [] - producers_active = [] + 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 @@ -57,16 +57,16 @@ module OpenFoodNetwork 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 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 - active_producer_ids = Enterprise.joins(:supplied_products).where("spree_products.id IN (?)", product_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_ids | hubs_permitted_ids | hubs_permitting_ids | producers_permitted_ids | producers_permitting_ids | managed_active_ids | hubs_active_ids | active_producer_ids + 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) end From ef61310bad092bbede46569f7034bacbf1cb822f Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 4 Jul 2019 17:10:14 +0100 Subject: [PATCH 7/8] Fix long lines in order_cycle_permissions and permissions --- .rubocop_manual_todo.yml | 3 +- .../order_cycle_permissions.rb | 180 +++++++++++++----- lib/open_food_network/permissions.rb | 54 ++++-- 3 files changed, 169 insertions(+), 68 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 6a7fdb1664..8e622cfa22 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -152,12 +152,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 @@ -470,6 +468,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/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index 1ffcee880b..928d825588 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -9,8 +9,8 @@ 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 @@ -19,30 +19,51 @@ module OpenFoodNetwork if @coordinator.sells == "any" # If the coordinator sells any, relationships come into play - related_enterprises_granting(:add_to_order_cycle, to: [@coordinator.id]).each do |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 + # 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_ids | all_active_ids) else # Any enterprises that I manage directly, which have granted P-OC to the coordinator - managed_permitted_ids = related_enterprises_granting(:add_to_order_cycle, to: [@coordinator.id], scope: managed_participating_enterprises ) + 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_ids = related_enterprises_granted(:add_to_order_cycle, by: managed_participating_producers.select("enterprises.id"), scope: @order_cycle.distributors) + 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_ids = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_producers.select("enterprises.id"), scope: @order_cycle.distributors) + 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_ids = related_enterprises_granted(:add_to_order_cycle, by: managed_participating_hubs.select("enterprises.id"), scope: @order_cycle.suppliers) + 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_ids = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_hubs.select("enterprises.id"), scope: @order_cycle.suppliers) + producers_permitting_ids = related_enterprises_granting( + :add_to_order_cycle, + to: managed_participating_hubs.select("enterprises.id"), + scope: @order_cycle.suppliers + ) managed_active_ids = [] hubs_active_ids = [] @@ -53,20 +74,38 @@ module OpenFoodNetwork 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.select("enterprises.id")) + # 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_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 - 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 + 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_ids | hubs_permitted_ids | hubs_permitting_ids | producers_permitted_ids | producers_permitting_ids | managed_active_ids | hubs_active_ids | producers_active_ids + 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) 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.select("enterprises.id")). - permitted_by(producer.id).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 - 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) + 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 - 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) + # 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.id, managed_enterprises.is_primary_producer.select("enterprises.id")) + 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 - 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) + 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.id], 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_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) + # 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 @@ -223,16 +291,30 @@ module OpenFoodNetwork # 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 - 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 + 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 - 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 - 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 + # 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 + + 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 @@ -241,8 +323,10 @@ module OpenFoodNetwork def order_cycle_exchange_ids_distributing_my_variants # Find my producers in this order cycle producer_ids = managed_participating_producers.pluck :id - # Any outgoing exchange where the distributor has been granted P-OC by one or more of those producers - hub_ids = related_enterprises_granted(:add_to_order_cycle, by: producer_ids, scope: Enterprise.is_hub) + # 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 diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 63d4e1fbeb..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 @@ -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_distributor_ids = related_enterprises_granted(:add_to_order_cycle, by: managed_enterprises.is_primary_producer.pluck(:id)) + # 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_distributor_ids, - related_enterprises_granting(:add_to_order_cycle, to: granted_distributor_ids).merge(managed_enterprises.is_primary_producer) + 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 @@ -108,15 +116,18 @@ module OpenFoodNetwork 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) + 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 = product_ids_supplied_by( - related_enterprises_granting(:manage_products) | related_enterprises_granting(:add_to_order_cycle) + 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) + 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 @@ -167,7 +182,8 @@ module OpenFoodNetwork 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) + Enterprise. + where(id: managed_enterprise_ids | granting_enterprise_ids | granted_enterprise_ids) end end @@ -187,7 +203,8 @@ module OpenFoodNetwork with_permission(permission). pluck(:parent_id) - (options[:scope] || Enterprise).where('enterprises.id IN (?)', parent_ids).select("enterprises.id") + (options[:scope] || Enterprise).where('enterprises.id IN (?)', parent_ids). + select("enterprises.id") end def related_enterprises_granted(permission, options = {}) @@ -196,7 +213,8 @@ module OpenFoodNetwork with_permission(permission). pluck(:child_id) - (options[:scope] || Enterprise).where('enterprises.id IN (?)', child_ids).select("enterprises.id") + (options[:scope] || Enterprise).where('enterprises.id IN (?)', child_ids). + select("enterprises.id") end def managed_enterprise_products From a5b5e5de32bbe1b5c784136b6b786ef44914ee78 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 5 Jul 2019 10:58:53 +0100 Subject: [PATCH 8/8] Remove trailing backslash --- lib/open_food_network/order_cycle_permissions.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index 928d825588..4d503ed37a 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -262,8 +262,8 @@ 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