From 3aff7f62e37187d8d2f6112cabda50ad3a6fdd65 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Apr 2020 13:09:07 +0200
Subject: [PATCH] Don't query distributed properties on enterprises that aren't
 active distributors

Cuts page load on /shops by ~75% (with production data) and removes ~300 expensive and superfluous queries.
---
 app/serializers/api/cached_enterprise_serializer.rb  |  6 ++++++
 spec/features/consumer/groups_spec.rb                |  4 ++--
 .../api/cached_enterprise_serializer_spec.rb         | 12 ++----------
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/app/serializers/api/cached_enterprise_serializer.rb b/app/serializers/api/cached_enterprise_serializer.rb
index 05677720db..8a6726e503 100644
--- a/app/serializers/api/cached_enterprise_serializer.rb
+++ b/app/serializers/api/cached_enterprise_serializer.rb
@@ -73,12 +73,16 @@ module Api
 
     # This results in 3 queries per enterprise
     def distributed_properties
+      return [] unless active
+
       (distributed_product_properties + distributed_producer_properties).uniq do |property_object|
         property_object.property.presentation
       end
     end
 
     def distributed_product_properties
+      return [] unless active
+
       properties = Spree::Property
         .joins(products: { variants: { exchanges: :order_cycle } })
         .merge(Exchange.outgoing)
@@ -91,6 +95,8 @@ module Api
     end
 
     def distributed_producer_properties
+      return [] unless active
+
       properties = Spree::Property
         .joins(
           producer_properties: {
diff --git a/spec/features/consumer/groups_spec.rb b/spec/features/consumer/groups_spec.rb
index c59835c344..dc9a3e4b75 100644
--- a/spec/features/consumer/groups_spec.rb
+++ b/spec/features/consumer/groups_spec.rb
@@ -60,8 +60,8 @@ feature 'Groups', js: true do
       let!(:group) { create(:enterprise_group, enterprises: [d1, d2], on_front_page: true) }
       let!(:order_cycle) { create(:simple_order_cycle, distributors: [d1, d2], coordinator: create(:distributor_enterprise)) }
       let(:producer) { create(:supplier_enterprise) }
-      let(:d1) { create(:distributor_enterprise) }
-      let(:d2) { create(:distributor_enterprise) }
+      let(:d1) { create(:distributor_enterprise, with_payment_and_shipping: true) }
+      let(:d2) { create(:distributor_enterprise, with_payment_and_shipping: true) }
       let(:p1) { create(:simple_product, supplier: producer) }
       let(:p2) { create(:simple_product, supplier: create(:supplier_enterprise)) }
       let(:ex_d1) { order_cycle.exchanges.outgoing.where(receiver_id: d1).first }
diff --git a/spec/serializers/api/cached_enterprise_serializer_spec.rb b/spec/serializers/api/cached_enterprise_serializer_spec.rb
index c4e5624fa8..6bcaeffdc3 100644
--- a/spec/serializers/api/cached_enterprise_serializer_spec.rb
+++ b/spec/serializers/api/cached_enterprise_serializer_spec.rb
@@ -53,17 +53,9 @@ describe Api::CachedEnterpriseSerializer do
         instance_double(OpenFoodNetwork::EnterpriseInjectionData, active_distributor_ids: [])
       end
 
-      it 'does not duplicate properties' do
+      it 'does not serialize distributed properties' do
         properties = cached_enterprise_serializer.distributed_properties
-        expect(properties.map(&:presentation)).to eq([property.presentation])
-      end
-
-      it 'fetches producer properties' do
-        distributed_producer_properties = cached_enterprise_serializer
-          .distributed_producer_properties
-
-        expect(distributed_producer_properties.map(&:presentation))
-          .to eq(producer.producer_properties.map(&:property).map(&:presentation))
+        expect(properties).to eq []
       end
     end