From e9667ab289e4b2ad9679d93f2fb45dafcd2a4ae0 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 6 Apr 2019 21:09:42 +0100 Subject: [PATCH 01/10] Adapt product.managed_by scope to rails 4 --- app/models/spree/product_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 7f5f265289..a23f5501dc 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -112,7 +112,7 @@ Spree::Product.class_eval do if user.has_spree_role?('admin') scoped else - where('supplier_id IN (?)', user.enterprises) + where('supplier_id IN (?)', user.enterprises.select("enterprises.id")) end } From 07a6e62d0929a045e9e61b298bc35c0cacfcab3f Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 7 Apr 2019 00:28:04 +0100 Subject: [PATCH 02/10] Adapt query in product destroy process to rails 4 --- app/models/spree/product_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index a23f5501dc..33367ef319 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -189,7 +189,7 @@ Spree::Product.class_eval do OpenFoodNetwork::ProductsCache.product_deleted(self) do touch_distributors - ExchangeVariant.where('exchange_variants.variant_id IN (?)', variants_including_master.with_deleted).destroy_all + ExchangeVariant.where('exchange_variants.variant_id IN (?)', variants_including_master.with_deleted.select(:id)).destroy_all destroy_without_delete_from_order_cycles end From 0d07bf2a3b25fe80a50fbcd51c1acdcbff6423f0 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 7 Apr 2019 11:24:31 +0100 Subject: [PATCH 03/10] Adapt enterprise.distributing_products scope to rails 4 --- app/models/enterprise.rb | 8 ++++---- app/models/spree/product_decorator.rb | 2 +- spec/models/enterprise_spec.rb | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index a85ffb00e7..3908650eb8 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -111,7 +111,7 @@ class Enterprise < ActiveRecord::Base joins(:shipping_methods). joins(:payment_methods). merge(Spree::PaymentMethod.available). - select('DISTINCT enterprises.*') + select('DISTINCT enterprises.id') } scope :not_ready_for_checkout, lambda { # When ready_for_checkout is empty, return all rows when there are no enterprises ready for @@ -165,14 +165,14 @@ class Enterprise < ActiveRecord::Base select('DISTINCT enterprises.*') } - scope :distributing_products, lambda { |products| + scope :distributing_products, lambda { |product_ids| exchanges = joins(" INNER JOIN exchanges ON (exchanges.receiver_id = enterprises.id AND exchanges.incoming = 'f') "). joins('INNER JOIN exchange_variants ON (exchange_variants.exchange_id = exchanges.id)'). joins('INNER JOIN spree_variants ON (spree_variants.id = exchange_variants.variant_id)'). - where('spree_variants.product_id IN (?)', products).select('DISTINCT enterprises.id') + where('spree_variants.product_id IN (?)', product_ids).select('DISTINCT enterprises.id') where(id: exchanges) } @@ -449,7 +449,7 @@ class Enterprise < ActiveRecord::Base end def touch_distributors - Enterprise.distributing_products(supplied_products). + Enterprise.distributing_products(supplied_products.select(:id)). where('enterprises.id != ?', id). find_each(&:touch) end diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 33367ef319..ef75d1601e 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -216,7 +216,7 @@ Spree::Product.class_eval do end def touch_distributors - Enterprise.distributing_products(self).each(&:touch) + Enterprise.distributing_products(self.id).each(&:touch) end def add_primary_taxon_to_taxons diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index c648031e88..4e3e9d5565 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -360,13 +360,13 @@ describe Enterprise do it "returns enterprises distributing via an order cycle" do order_cycle = create(:simple_order_cycle, distributors: [distributor], variants: [product.master]) - expect(Enterprise.distributing_products(product)).to eq([distributor]) + expect(Enterprise.distributing_products(product.id)).to eq([distributor]) end it "does not return duplicate enterprises" do another_product = create(:product) order_cycle = create(:simple_order_cycle, distributors: [distributor], variants: [product.master, another_product.master]) - expect(Enterprise.distributing_products([product, another_product])).to eq([distributor]) + expect(Enterprise.distributing_products([product.id, another_product.id])).to eq([distributor]) end end From ba91abd20eca2a8eb04420ff691309bbe6746c99 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 7 Apr 2019 11:31:31 +0100 Subject: [PATCH 04/10] Adapt exchanges.with_any_variant scope to rails 4 --- app/models/exchange.rb | 2 +- app/models/order_cycle.rb | 4 ++-- .../order_cycle_permissions.rb | 4 ++-- lib/open_food_network/products_cache.rb | 18 +++++++++--------- spec/models/exchange_spec.rb | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/models/exchange.rb b/app/models/exchange.rb index 3493ebecde..317b4a136c 100644 --- a/app/models/exchange.rb +++ b/app/models/exchange.rb @@ -29,7 +29,7 @@ class Exchange < ActiveRecord::Base scope :involving, lambda { |enterprises| where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises).select('DISTINCT exchanges.*') } scope :supplying_to, lambda { |distributor| where('exchanges.incoming OR exchanges.receiver_id = ?', distributor) } scope :with_variant, lambda { |variant| joins(:exchange_variants).where('exchange_variants.variant_id = ?', variant) } - scope :with_any_variant, lambda { |variants| joins(:exchange_variants).where('exchange_variants.variant_id IN (?)', variants).select('DISTINCT exchanges.*') } + scope :with_any_variant, lambda { |variant_ids| joins(:exchange_variants).where('exchange_variants.variant_id IN (?)', variant_ids).select('DISTINCT exchanges.*') } scope :with_product, lambda { |product| joins(:exchange_variants).where('exchange_variants.variant_id IN (?)', product.variants_including_master) } scope :by_enterprise_name, -> { joins('INNER JOIN enterprises AS sender ON (sender.id = exchanges.sender_id)'). diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 707bdae56a..262c6c7998 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -52,7 +52,7 @@ class OrderCycle < ActiveRecord::Base if user.has_spree_role?('admin') scoped else - where('coordinator_id IN (?)', user.enterprises) + where('coordinator_id IN (?)', user.enterprises.map(&:id)) end } @@ -217,7 +217,7 @@ class OrderCycle < ActiveRecord::Base end def exchanges_supplying(order) - exchanges.supplying_to(order.distributor).with_any_variant(order.variants) + exchanges.supplying_to(order.distributor).with_any_variant(order.variants.map(&:id)) end def coordinated_by?(user) diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index a75c27bad6..db0d4428ac 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -55,7 +55,7 @@ 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) - active_exchanges = @order_cycle.exchanges.outgoing.with_any_variant(variants) + 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 @@ -249,7 +249,7 @@ module OpenFoodNetwork # 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.exchanges.outgoing.with_any_variant(variants).pluck :id + active_exchanges = @order_cycle.exchanges.outgoing.with_any_variant(variants.select("spree_variants.id")).pluck :id permitted_exchanges | active_exchanges end diff --git a/lib/open_food_network/products_cache.rb b/lib/open_food_network/products_cache.rb index b1118cd80d..b818b6bdf8 100644 --- a/lib/open_food_network/products_cache.rb +++ b/lib/open_food_network/products_cache.rb @@ -5,13 +5,13 @@ require 'open_food_network/products_cache_refreshment' module OpenFoodNetwork class ProductsCache def self.variant_changed(variant) - exchanges_featuring_variants(variant).each do |exchange| + exchanges_featuring_variants(variant.id).each do |exchange| refresh_cache exchange.receiver, exchange.order_cycle end end def self.variant_destroyed(variant, &block) - exchanges = exchanges_featuring_variants(variant).to_a + exchanges = exchanges_featuring_variants(variant.id).to_a block.call @@ -21,13 +21,13 @@ module OpenFoodNetwork end def self.product_changed(product) - exchanges_featuring_variants(product.variants).each do |exchange| + exchanges_featuring_variants(product.variants.map(&:id)).each do |exchange| refresh_cache exchange.receiver, exchange.order_cycle end end def self.product_deleted(product, &block) - exchanges = exchanges_featuring_variants(product.reload.variants).to_a + exchanges = exchanges_featuring_variants(product.reload.variants.map(&:id)).to_a block.call @@ -37,7 +37,7 @@ module OpenFoodNetwork end def self.variant_override_changed(variant_override) - exchanges_featuring_variants(variant_override.variant, distributor: variant_override.hub).each do |exchange| + exchanges_featuring_variants(variant_override.variant.id, distributor: variant_override.hub).each do |exchange| refresh_cache exchange.receiver, exchange.order_cycle end end @@ -52,7 +52,7 @@ module OpenFoodNetwork where(is_master: false, deleted_at: nil). where(product_id: products) - exchanges_featuring_variants(variants).each do |exchange| + exchanges_featuring_variants(variants.select(:id)).each do |exchange| refresh_cache exchange.receiver, exchange.order_cycle end end @@ -94,17 +94,17 @@ module OpenFoodNetwork end def self.inventory_item_changed(inventory_item) - exchanges_featuring_variants(inventory_item.variant, distributor: inventory_item.enterprise).each do |exchange| + exchanges_featuring_variants(inventory_item.variant.id, distributor: inventory_item.enterprise).each do |exchange| refresh_cache exchange.receiver, exchange.order_cycle end end private - def self.exchanges_featuring_variants(variants, distributor: nil) + def self.exchanges_featuring_variants(variant_ids, distributor: nil) exchanges = Exchange. outgoing. - with_any_variant(variants). + with_any_variant(variant_ids). joins(:order_cycle). merge(OrderCycle.dated). merge(OrderCycle.not_closed) diff --git a/spec/models/exchange_spec.rb b/spec/models/exchange_spec.rb index bd59675911..3966dbf8e9 100644 --- a/spec/models/exchange_spec.rb +++ b/spec/models/exchange_spec.rb @@ -240,7 +240,7 @@ describe Exchange do ex.variants << v1 ex.variants << v2 - expect(Exchange.with_any_variant([v1, v2, v3])).to eq([ex]) + expect(Exchange.with_any_variant([v1.id, v2.id, v3.id])).to eq([ex]) end it "finds exchanges with a particular product's master variant" do From 9548f5c5f754fd6fd8b9e8578d81d6dcc98b3caa Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 21 Mar 2019 21:20:47 +0000 Subject: [PATCH 05/10] Adapt order_cycle scopes to rail 4 --- app/models/order_cycle.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 262c6c7998..d1ac47fe8f 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -62,7 +62,7 @@ class OrderCycle < ActiveRecord::Base scoped else with_exchanging_enterprises_outer. - where('order_cycles.coordinator_id IN (?) OR enterprises.id IN (?)', user.enterprises, user.enterprises). + where('order_cycles.coordinator_id IN (?) OR enterprises.id IN (?)', user.enterprises.map(&:id), user.enterprises.map(&:id)). select('DISTINCT order_cycles.*') end } @@ -78,7 +78,7 @@ class OrderCycle < ActiveRecord::Base # Order cycles where I managed an enterprise at either end of an outgoing exchange # ie. coordinator or distributor joins(:exchanges).merge(Exchange.outgoing). - where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises). + where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises.pluck(:id), enterprises.pluck(:id)). select('DISTINCT order_cycles.*') } @@ -88,7 +88,7 @@ class OrderCycle < ActiveRecord::Base # Order cycles where I managed an enterprise at either end of an incoming exchange # ie. coordinator or producer joins(:exchanges).merge(Exchange.incoming). - where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises). + where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises.pluck(:id), enterprises.pluck(:id)). select('DISTINCT order_cycles.*') } From 0cdb49818de7dd60748b00efc59c7c0ebdc11938 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 6 Apr 2019 17:45:44 +0100 Subject: [PATCH 06/10] Remove distinct from enterprise.ready_for_checkout scope Adapt use of enterprise scope not_ready_for_checkout to rails 4 by adding enterprises table alias to selected field id --- app/controllers/spree/admin/base_controller_decorator.rb | 2 +- app/models/enterprise.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/base_controller_decorator.rb b/app/controllers/spree/admin/base_controller_decorator.rb index 1d5d6e476a..6bdd57f0e8 100644 --- a/app/controllers/spree/admin/base_controller_decorator.rb +++ b/app/controllers/spree/admin/base_controller_decorator.rb @@ -52,7 +52,7 @@ Spree::Admin::BaseController.class_eval do def active_distributors_not_ready_for_checkout ocs = OrderCycle.managed_by(spree_current_user).active distributors = ocs.map(&:distributors).flatten.uniq - Enterprise.where('id IN (?)', distributors).not_ready_for_checkout + Enterprise.where('enterprises.id IN (?)', distributors).not_ready_for_checkout end def active_distributors_not_ready_for_checkout_message(distributors) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 3908650eb8..ac9252e389 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -111,14 +111,14 @@ class Enterprise < ActiveRecord::Base joins(:shipping_methods). joins(:payment_methods). merge(Spree::PaymentMethod.available). - select('DISTINCT enterprises.id') + select('enterprises.id') } scope :not_ready_for_checkout, lambda { # When ready_for_checkout is empty, return all rows when there are no enterprises ready for # checkout. ready_enterprises = Enterprise.ready_for_checkout if ready_enterprises.present? - where("id NOT IN (?)", ready_enterprises) + where("enterprises.id NOT IN (?)", ready_enterprises) else where("TRUE") end From c15c5435ffe48bd9583f1008a97eacf6a98f8d45 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 29 May 2019 10:35:03 +0100 Subject: [PATCH 07/10] Fix long lines in exchange model --- .rubocop_manual_todo.yml | 1 - app/models/exchange.rb | 38 +++++++++++++++++++++++++++----------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index b8ad5662cc..174858d55d 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -81,7 +81,6 @@ Metrics/LineLength: - app/models/enterprise_fee.rb - app/models/enterprise_group.rb - app/models/enterprise_role.rb - - app/models/exchange.rb - app/models/inventory_item.rb - app/models/order_cycle.rb - app/models/product_import/entry_processor.rb diff --git a/app/models/exchange.rb b/app/models/exchange.rb index 317b4a136c..c85c2f9d8d 100644 --- a/app/models/exchange.rb +++ b/app/models/exchange.rb @@ -26,11 +26,25 @@ class Exchange < ActiveRecord::Base scope :to_enterprise, lambda { |enterprise| where(receiver_id: enterprise) } scope :from_enterprises, lambda { |enterprises| where('exchanges.sender_id IN (?)', enterprises) } scope :to_enterprises, lambda { |enterprises| where('exchanges.receiver_id IN (?)', enterprises) } - scope :involving, lambda { |enterprises| where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises).select('DISTINCT exchanges.*') } - scope :supplying_to, lambda { |distributor| where('exchanges.incoming OR exchanges.receiver_id = ?', distributor) } - scope :with_variant, lambda { |variant| joins(:exchange_variants).where('exchange_variants.variant_id = ?', variant) } - scope :with_any_variant, lambda { |variant_ids| joins(:exchange_variants).where('exchange_variants.variant_id IN (?)', variant_ids).select('DISTINCT exchanges.*') } - scope :with_product, lambda { |product| joins(:exchange_variants).where('exchange_variants.variant_id IN (?)', product.variants_including_master) } + scope :involving, lambda { |enterprises| + where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises). + select('DISTINCT exchanges.*') + } + scope :supplying_to, lambda { |distributor| + where('exchanges.incoming OR exchanges.receiver_id = ?', distributor) + } + scope :with_variant, lambda { |variant| + joins(:exchange_variants).where('exchange_variants.variant_id = ?', variant) + } + scope :with_any_variant, lambda { |variant_ids| + joins(:exchange_variants). + where('exchange_variants.variant_id IN (?)', variant_ids). + select('DISTINCT exchanges.*') + } + scope :with_product, lambda { |product| + joins(:exchange_variants). + where('exchange_variants.variant_id IN (?)', product.variants_including_master) + } scope :by_enterprise_name, -> { joins('INNER JOIN enterprises AS sender ON (sender.id = exchanges.sender_id)'). joins('INNER JOIN enterprises AS receiver ON (receiver.id = exchanges.receiver_id)'). @@ -49,11 +63,12 @@ class Exchange < ActiveRecord::Base if user.has_spree_role?('admin') scoped else - joins('LEFT JOIN enterprises senders ON senders.id = exchanges.sender_id'). - joins('LEFT JOIN enterprises receivers ON receivers.id = exchanges.receiver_id'). - joins('LEFT JOIN enterprise_roles sender_roles ON sender_roles.enterprise_id = senders.id'). - joins('LEFT JOIN enterprise_roles receiver_roles ON receiver_roles.enterprise_id = receivers.id'). - where('sender_roles.user_id = ? AND receiver_roles.user_id = ?', user.id, user.id) + joins("LEFT JOIN enterprises senders ON senders.id = exchanges.sender_id"). + joins("LEFT JOIN enterprises receivers ON receivers.id = exchanges.receiver_id"). + joins("LEFT JOIN enterprise_roles sender_roles ON sender_roles.enterprise_id = senders.id"). + joins("LEFT JOIN enterprise_roles receiver_roles + ON receiver_roles.enterprise_id = receivers.id"). + where("sender_roles.user_id = ? AND receiver_roles.user_id = ?", user.id, user.id) end } @@ -76,7 +91,8 @@ class Exchange < ActiveRecord::Base end def to_h(core_only = false) - h = attributes.merge('variant_ids' => variant_ids.sort, 'enterprise_fee_ids' => enterprise_fee_ids.sort) + h = attributes.merge('variant_ids' => variant_ids.sort, + 'enterprise_fee_ids' => enterprise_fee_ids.sort) h.reject! { |k| %w(id order_cycle_id created_at updated_at).include? k } if core_only h end From 178924af5d374352e1c3b7e68e42bc70c1aa3edf Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 29 May 2019 10:43:46 +0100 Subject: [PATCH 08/10] Fix long lines in order_cycle model --- .rubocop_manual_todo.yml | 1 - app/models/order_cycle.rb | 53 +++++++++++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 174858d55d..6a7fdb1664 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -82,7 +82,6 @@ Metrics/LineLength: - app/models/enterprise_group.rb - app/models/enterprise_role.rb - app/models/inventory_item.rb - - app/models/order_cycle.rb - app/models/product_import/entry_processor.rb - app/models/product_import/entry_validator.rb - app/models/product_import/product_importer.rb diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index d1ac47fe8f..4a37a139f9 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -27,17 +27,30 @@ class OrderCycle < ActiveRecord::Base preference :product_selection_from_coordinator_inventory_only, :boolean, default: false - scope :active, lambda { where('order_cycles.orders_open_at <= ? AND order_cycles.orders_close_at >= ?', Time.zone.now, Time.zone.now) } + scope :active, lambda { + where('order_cycles.orders_open_at <= ? AND order_cycles.orders_close_at >= ?', + Time.zone.now, + Time.zone.now) + } scope :active_or_complete, lambda { where('order_cycles.orders_open_at <= ?', Time.zone.now) } - scope :inactive, lambda { where('order_cycles.orders_open_at > ? OR order_cycles.orders_close_at < ?', Time.zone.now, Time.zone.now) } + scope :inactive, lambda { + where('order_cycles.orders_open_at > ? OR order_cycles.orders_close_at < ?', + Time.zone.now, + Time.zone.now) + } scope :upcoming, lambda { where('order_cycles.orders_open_at > ?', Time.zone.now) } - scope :not_closed, lambda { where('order_cycles.orders_close_at > ? OR order_cycles.orders_close_at IS NULL', Time.zone.now) } - scope :closed, lambda { where('order_cycles.orders_close_at < ?', Time.zone.now).order("order_cycles.orders_close_at DESC") } + scope :not_closed, lambda { + where('order_cycles.orders_close_at > ? OR order_cycles.orders_close_at IS NULL', Time.zone.now) + } + scope :closed, lambda { + where('order_cycles.orders_close_at < ?', + Time.zone.now).order("order_cycles.orders_close_at DESC") + } scope :undated, -> { where('order_cycles.orders_open_at IS NULL OR orders_close_at IS NULL') } scope :dated, -> { where('orders_open_at IS NOT NULL AND orders_close_at IS NOT NULL') } scope :soonest_closing, lambda { active.order('order_cycles.orders_close_at ASC') } - # TODO This method returns all the closed orders. So maybe we can replace it with :recently_closed. + # This scope returns all the closed orders scope :most_recently_closed, lambda { closed.order('order_cycles.orders_close_at DESC') } scope :soonest_opening, lambda { upcoming.order('order_cycles.orders_open_at ASC') } @@ -62,14 +75,17 @@ class OrderCycle < ActiveRecord::Base scoped else with_exchanging_enterprises_outer. - where('order_cycles.coordinator_id IN (?) OR enterprises.id IN (?)', user.enterprises.map(&:id), user.enterprises.map(&:id)). + where('order_cycles.coordinator_id IN (?) OR enterprises.id IN (?)', + user.enterprises.map(&:id), + user.enterprises.map(&:id)). select('DISTINCT order_cycles.*') end } scope :with_exchanging_enterprises_outer, lambda { - joins('LEFT OUTER JOIN exchanges ON (exchanges.order_cycle_id = order_cycles.id)'). - joins('LEFT OUTER JOIN enterprises ON (enterprises.id = exchanges.sender_id OR enterprises.id = exchanges.receiver_id)') + joins("LEFT OUTER JOIN exchanges ON (exchanges.order_cycle_id = order_cycles.id)"). + joins("LEFT OUTER JOIN enterprises + ON (enterprises.id = exchanges.sender_id OR enterprises.id = exchanges.receiver_id)") } scope :involving_managed_distributors_of, lambda { |user| @@ -78,7 +94,9 @@ class OrderCycle < ActiveRecord::Base # Order cycles where I managed an enterprise at either end of an outgoing exchange # ie. coordinator or distributor joins(:exchanges).merge(Exchange.outgoing). - where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises.pluck(:id), enterprises.pluck(:id)). + where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', + enterprises.pluck(:id), + enterprises.pluck(:id)). select('DISTINCT order_cycles.*') } @@ -88,7 +106,9 @@ class OrderCycle < ActiveRecord::Base # Order cycles where I managed an enterprise at either end of an incoming exchange # ie. coordinator or producer joins(:exchanges).merge(Exchange.incoming). - where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises.pluck(:id), enterprises.pluck(:id)). + where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', + enterprises.pluck(:id), + enterprises.pluck(:id)). select('DISTINCT order_cycles.*') } @@ -113,7 +133,8 @@ class OrderCycle < ActiveRecord::Base joins(:order_cycle). merge(OrderCycle.active). group('exchanges.receiver_id'). - select('exchanges.receiver_id AS receiver_id, MIN(order_cycles.orders_close_at) AS earliest_close_at'). + select("exchanges.receiver_id AS receiver_id, + MIN(order_cycles.orders_close_at) AS earliest_close_at"). map { |ex| [ex.receiver_id, ex.earliest_close_at.to_time] } ] end @@ -123,7 +144,9 @@ class OrderCycle < ActiveRecord::Base oc.name = I18n.t("models.order_cycle.cloned_order_cycle_name", order_cycle: oc.name) oc.orders_open_at = oc.orders_close_at = nil oc.coordinator_fee_ids = coordinator_fee_ids + # rubocop:disable Metrics/LineLength oc.preferred_product_selection_from_coordinator_inventory_only = preferred_product_selection_from_coordinator_inventory_only + # rubocop:enable Metrics/LineLength oc.save! exchanges.each { |e| e.clone!(oc) } oc.reload @@ -229,8 +252,12 @@ class OrderCycle < ActiveRecord::Base end def items_bought_by_user(user, distributor) - # The Spree::Order.complete scope only checks for completed_at date, does not ensure state is "complete" - orders = Spree::Order.complete.where(state: "complete", user_id: user, distributor_id: distributor, order_cycle_id: self) + # The Spree::Order.complete scope only checks for completed_at date + # it does not ensure state is "complete" + orders = Spree::Order.complete.where(state: "complete", + user_id: user, + distributor_id: distributor, + order_cycle_id: self) scoper = OpenFoodNetwork::ScopeVariantToHub.new(distributor) items = Spree::LineItem.joins(:order).merge(orders) items.each { |li| scoper.scope(li.variant) } From b025df17980f84d2d39490d9b48107ee5deff9ba Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 29 May 2019 10:48:51 +0100 Subject: [PATCH 09/10] Fix a few more rubocop issues --- app/models/spree/product_decorator.rb | 6 ++++-- lib/open_food_network/order_cycle_permissions.rb | 9 ++++++--- lib/open_food_network/products_cache.rb | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index ef75d1601e..2e701eb920 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -189,7 +189,9 @@ Spree::Product.class_eval do OpenFoodNetwork::ProductsCache.product_deleted(self) do touch_distributors - ExchangeVariant.where('exchange_variants.variant_id IN (?)', variants_including_master.with_deleted.select(:id)).destroy_all + ExchangeVariant. + where('exchange_variants.variant_id IN (?)', variants_including_master.with_deleted. + select(:id)).destroy_all destroy_without_delete_from_order_cycles end @@ -216,7 +218,7 @@ Spree::Product.class_eval do end def touch_distributors - Enterprise.distributing_products(self.id).each(&:touch) + Enterprise.distributing_products(id).each(&:touch) end def add_primary_taxon_to_taxons diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index db0d4428ac..e49ee2f74f 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -55,7 +55,8 @@ 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) - active_exchanges = @order_cycle.exchanges.outgoing.with_any_variant(variants.select("spree_variants.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 @@ -248,8 +249,10 @@ module OpenFoodNetwork # 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.exchanges.outgoing.with_any_variant(variants.select("spree_variants.id")).pluck :id + variants = Spree::Variant. + joins(:product).where('spree_products.supplier_id IN (?)', producers) + active_exchanges = @order_cycle. + exchanges.outgoing.with_any_variant(variants.select("spree_variants.id")).pluck :id permitted_exchanges | active_exchanges end diff --git a/lib/open_food_network/products_cache.rb b/lib/open_food_network/products_cache.rb index b818b6bdf8..8dbf166d43 100644 --- a/lib/open_food_network/products_cache.rb +++ b/lib/open_food_network/products_cache.rb @@ -94,7 +94,8 @@ module OpenFoodNetwork end def self.inventory_item_changed(inventory_item) - exchanges_featuring_variants(inventory_item.variant.id, distributor: inventory_item.enterprise).each do |exchange| + exchanges_featuring_variants(inventory_item.variant.id, + distributor: inventory_item.enterprise).each do |exchange| refresh_cache exchange.receiver, exchange.order_cycle end end From 1da18d3386321a3375fa8cdded89632d0bf00f9b Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 3 Jun 2019 16:01:00 +0100 Subject: [PATCH 10/10] Fix enterprise model scopes by making ready_for_checkout return enterprises and not ids again and by making not_ready_for_checkout select the id field from the ready_for_checkout scope --- app/models/enterprise.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index ac9252e389..09560060df 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -111,12 +111,12 @@ class Enterprise < ActiveRecord::Base joins(:shipping_methods). joins(:payment_methods). merge(Spree::PaymentMethod.available). - select('enterprises.id') + select('DISTINCT enterprises.*') } scope :not_ready_for_checkout, lambda { # When ready_for_checkout is empty, return all rows when there are no enterprises ready for # checkout. - ready_enterprises = Enterprise.ready_for_checkout + ready_enterprises = Enterprise.ready_for_checkout.select('enterprises.id') if ready_enterprises.present? where("enterprises.id NOT IN (?)", ready_enterprises) else