From 3de887e1d8d3bcd28fac798a9b6221d9f750fde0 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 19 Mar 2020 23:36:16 +0100 Subject: [PATCH 1/3] Remove some N+1s relating to address (found with bullet gem) --- app/controllers/producers_controller.rb | 2 +- app/controllers/shops_controller.rb | 2 +- app/helpers/injection_helper.rb | 8 +++++--- app/helpers/spree/base_helper_decorator.rb | 16 ++++++++++++++++ 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/app/controllers/producers_controller.rb b/app/controllers/producers_controller.rb index 0f45546fde..a5f4752c49 100644 --- a/app/controllers/producers_controller.rb +++ b/app/controllers/producers_controller.rb @@ -8,7 +8,7 @@ class ProducersController < BaseController .activated .visible .is_primary_producer - .includes(address: :state) + .includes(address: [:state, :country]) .includes(:properties) .includes(supplied_products: :properties) .all diff --git a/app/controllers/shops_controller.rb b/app/controllers/shops_controller.rb index 611baa11b8..d4f350b47f 100644 --- a/app/controllers/shops_controller.rb +++ b/app/controllers/shops_controller.rb @@ -8,7 +8,7 @@ class ShopsController < BaseController .activated .visible .is_distributor - .includes(address: :state) + .includes(address: [:state, :country]) .includes(:properties) .includes(supplied_products: :properties) .all diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 6ebf77a8fb..a10a4b1fee 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -3,7 +3,7 @@ require 'open_food_network/enterprise_injection_data' module InjectionHelper include SerializerHelper - def inject_enterprises(enterprises = Enterprise.activated.includes(address: :state).all) + def inject_enterprises(enterprises = Enterprise.activated.includes(address: [:state, :country]).all) inject_json_ams( "enterprises", enterprises, @@ -35,13 +35,15 @@ module InjectionHelper inject_json_ams( "enterprises", - Enterprise.activated.visible.select(select_only).includes(address: :state).all, + Enterprise.activated.visible.select(select_only).includes(address: [:state, :country]).all, Api::EnterpriseShopfrontListSerializer ) end def inject_enterprise_and_relatives - inject_json_ams "enterprises", current_distributor.relatives_including_self.activated.includes(address: :state).all, Api::EnterpriseSerializer, enterprise_injection_data + inject_json_ams "enterprises", + current_distributor.relatives_including_self.activated.includes(address: [:state, :country]).all, + Api::EnterpriseSerializer, enterprise_injection_data end def inject_group_enterprises diff --git a/app/helpers/spree/base_helper_decorator.rb b/app/helpers/spree/base_helper_decorator.rb index 67ea0a711d..8d80c51d9a 100644 --- a/app/helpers/spree/base_helper_decorator.rb +++ b/app/helpers/spree/base_helper_decorator.rb @@ -5,5 +5,21 @@ module Spree def variant_options(v, _options = {}) v.options_text end + + # Overriden to eager-load :states + def available_countries + checkout_zone = Zone.find_by_name(Spree::Config[:checkout_zone]) + + if checkout_zone && checkout_zone.kind == 'country' + countries = checkout_zone.country_list + else + countries = Country.includes(:states).all + end + + countries.collect do |country| + country.name = Spree.t(country.iso, scope: 'country_names', default: country.name) + country + end.sort { |a, b| a.name <=> b.name } + end end end From 7baa875a9189f140b22ea845a4527b8403a5c545 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 19 Mar 2020 23:40:57 +0100 Subject: [PATCH 2/3] Fix big N+1 issues in enterprises#edit for superadmin The page is usable now as superadmin. Roughly 10x faster... --- app/controllers/admin/enterprises_controller.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 8369c5283f..4fad65ce37 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -32,6 +32,11 @@ module Admin end end + def edit + @object = Enterprise.where(permalink: params[:id]).includes(users: [:ship_address, :bill_address]).first + super + end + def welcome render layout: "spree/layouts/bare_admin" end @@ -172,12 +177,14 @@ module Admin end def load_methods_and_fees + enterprise_payment_methods = @enterprise.payment_methods.to_a + enterprise_shipping_methods = @enterprise.shipping_methods.to_a # rubocop:disable Style/TernaryParentheses @payment_methods = Spree::PaymentMethod.managed_by(spree_current_user).sort_by! do |pm| - [(@enterprise.payment_methods.include? pm) ? 0 : 1, pm.name] + [(enterprise_payment_methods.include? pm) ? 0 : 1, pm.name] end @shipping_methods = Spree::ShippingMethod.managed_by(spree_current_user).sort_by! do |sm| - [(@enterprise.shipping_methods.include? sm) ? 0 : 1, sm.name] + [(enterprise_shipping_methods.include? sm) ? 0 : 1, sm.name] end # rubocop:enable Style/TernaryParentheses From d847560d7c9909bc2c6cb1d28c8f2f20b96d9027 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 21 Mar 2020 10:54:02 +0100 Subject: [PATCH 3/3] Fix rubocop issues --- app/controllers/admin/enterprises_controller.rb | 3 ++- app/helpers/injection_helper.rb | 16 +++++++++++++--- app/helpers/spree/base_helper_decorator.rb | 10 +++++----- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 4fad65ce37..6f74043078 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -33,7 +33,8 @@ module Admin end def edit - @object = Enterprise.where(permalink: params[:id]).includes(users: [:ship_address, :bill_address]).first + @object = Enterprise.where(permalink: params[:id]). + includes(users: [:ship_address, :bill_address]).first super end diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index a10a4b1fee..bbc05c938f 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -3,10 +3,10 @@ require 'open_food_network/enterprise_injection_data' module InjectionHelper include SerializerHelper - def inject_enterprises(enterprises = Enterprise.activated.includes(address: [:state, :country]).all) + def inject_enterprises(enterprises = nil) inject_json_ams( "enterprises", - enterprises, + enterprises || default_enterprise_query, Api::EnterpriseSerializer, enterprise_injection_data ) @@ -41,8 +41,14 @@ module InjectionHelper end def inject_enterprise_and_relatives + enterprises_and_relatives = current_distributor. + relatives_including_self. + activated. + includes(address: [:state, :country]). + all + inject_json_ams "enterprises", - current_distributor.relatives_including_self.activated.includes(address: [:state, :country]).all, + enterprises_and_relatives, Api::EnterpriseSerializer, enterprise_injection_data end @@ -140,6 +146,10 @@ module InjectionHelper private + def default_enterprise_query + Enterprise.activated.includes(address: [:state, :country]).all + end + def enterprise_injection_data @enterprise_injection_data ||= OpenFoodNetwork::EnterpriseInjectionData.new { data: @enterprise_injection_data } diff --git a/app/helpers/spree/base_helper_decorator.rb b/app/helpers/spree/base_helper_decorator.rb index 8d80c51d9a..0125347d58 100644 --- a/app/helpers/spree/base_helper_decorator.rb +++ b/app/helpers/spree/base_helper_decorator.rb @@ -10,11 +10,11 @@ module Spree def available_countries checkout_zone = Zone.find_by_name(Spree::Config[:checkout_zone]) - if checkout_zone && checkout_zone.kind == 'country' - countries = checkout_zone.country_list - else - countries = Country.includes(:states).all - end + countries = if checkout_zone && checkout_zone.kind == 'country' + checkout_zone.country_list + else + Country.includes(:states).all + end countries.collect do |country| country.name = Spree.t(country.iso, scope: 'country_names', default: country.name)