From c9daca22d55fab759eac6dda33e8f2730350dd56 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 14 Aug 2024 17:16:45 +1000 Subject: [PATCH 1/4] Rename spec to match class name --- ...riants_to_search_spec.rb => scope_variants_for_search_spec.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/lib/open_food_network/{scope_variants_to_search_spec.rb => scope_variants_for_search_spec.rb} (100%) diff --git a/spec/lib/open_food_network/scope_variants_to_search_spec.rb b/spec/lib/open_food_network/scope_variants_for_search_spec.rb similarity index 100% rename from spec/lib/open_food_network/scope_variants_to_search_spec.rb rename to spec/lib/open_food_network/scope_variants_for_search_spec.rb From eb547f48612a78d6293de6bdf61cacb17ef7f361 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 14 Aug 2024 17:26:01 +1000 Subject: [PATCH 2/4] Add test on number of db queries Hmm, I think I seen an opportunity to clean up already. --- spec/lib/open_food_network/scope_variants_for_search_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/lib/open_food_network/scope_variants_for_search_spec.rb b/spec/lib/open_food_network/scope_variants_for_search_spec.rb index c8f6104ae5..99e011955b 100644 --- a/spec/lib/open_food_network/scope_variants_for_search_spec.rb +++ b/spec/lib/open_food_network/scope_variants_for_search_spec.rb @@ -65,6 +65,11 @@ RSpec.describe OpenFoodNetwork::ScopeVariantsForSearch do let(:params) { { q: "product", distributor_id: d2.id } } it "returns all products distributed through that distributor" do + expect{ result }.to query_database [ + "Enterprise Load", "Enterprise Load", "Enterprise Load", # loads same enterprise three times + "VariantOverride Load", + "SQL" + ] expect(result).to include v4 expect(result).not_to include v1, v2, v3 end From ffaf1b4ea076dc5369f25ec7af371c29bafabe05 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 14 Aug 2024 17:26:37 +1000 Subject: [PATCH 3/4] Cache distributor --- lib/open_food_network/scope_variants_for_search.rb | 2 +- spec/lib/open_food_network/scope_variants_for_search_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/open_food_network/scope_variants_for_search.rb b/lib/open_food_network/scope_variants_for_search.rb index 14a1bc25df..e154096524 100644 --- a/lib/open_food_network/scope_variants_for_search.rb +++ b/lib/open_food_network/scope_variants_for_search.rb @@ -42,7 +42,7 @@ module OpenFoodNetwork end def distributor - Enterprise.find params[:distributor_id] + @distributor ||= Enterprise.find params[:distributor_id] end def scope_to_schedule diff --git a/spec/lib/open_food_network/scope_variants_for_search_spec.rb b/spec/lib/open_food_network/scope_variants_for_search_spec.rb index 99e011955b..fc16580d15 100644 --- a/spec/lib/open_food_network/scope_variants_for_search_spec.rb +++ b/spec/lib/open_food_network/scope_variants_for_search_spec.rb @@ -66,7 +66,7 @@ RSpec.describe OpenFoodNetwork::ScopeVariantsForSearch do it "returns all products distributed through that distributor" do expect{ result }.to query_database [ - "Enterprise Load", "Enterprise Load", "Enterprise Load", # loads same enterprise three times + "Enterprise Load", "VariantOverride Load", "SQL" ] From f2eb4b05f41d43d699e3009642c2493703dc365b Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 15 Aug 2024 11:54:57 +1000 Subject: [PATCH 4/4] Avoid copying gigantic array --- .../services/order_management/subscriptions/variants_list.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/engines/order_management/app/services/order_management/subscriptions/variants_list.rb b/engines/order_management/app/services/order_management/subscriptions/variants_list.rb index fb06ed8829..f3a817219a 100644 --- a/engines/order_management/app/services/order_management/subscriptions/variants_list.rb +++ b/engines/order_management/app/services/order_management/subscriptions/variants_list.rb @@ -32,7 +32,9 @@ module OrderManagement .merge(Enterprise.is_primary_producer) .pluck(:parent_id) - other_permitted_producer_ids | [distributor.id] + # Append to the potentially gigantic array instead of using union, which creates a new array + # The db IN statement won't care if there's a duplicate. + other_permitted_producer_ids << distributor.id end def self.outgoing_exchange_variant_ids(distributor)