From 69cf7dff2cc11316c9cc771f24116343613317e3 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 23 Apr 2020 12:24:18 +0200 Subject: [PATCH 1/3] Memoize :active in enterprise serializers that call it multiple times --- app/serializers/api/cached_enterprise_serializer.rb | 2 +- app/serializers/api/enterprise_shopfront_serializer.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/serializers/api/cached_enterprise_serializer.rb b/app/serializers/api/cached_enterprise_serializer.rb index 8a6726e503..fcb0970e53 100644 --- a/app/serializers/api/cached_enterprise_serializer.rb +++ b/app/serializers/api/cached_enterprise_serializer.rb @@ -113,7 +113,7 @@ module Api end def active - data.active_distributor_ids.andand.include? enterprise.id + @active ||= data.active_distributor_ids.andand.include? enterprise.id end # Map svg icons. diff --git a/app/serializers/api/enterprise_shopfront_serializer.rb b/app/serializers/api/enterprise_shopfront_serializer.rb index 693f7442f5..dd76500227 100644 --- a/app/serializers/api/enterprise_shopfront_serializer.rb +++ b/app/serializers/api/enterprise_shopfront_serializer.rb @@ -18,7 +18,8 @@ module Api end def active - enterprise.ready_for_checkout? && OrderCycle.active.with_distributor(enterprise).exists? + @active ||= + enterprise.ready_for_checkout? && OrderCycle.active.with_distributor(enterprise).exists? end def pickup From 728326c2a5f6f1c74dbf3a9bf4daf8e5776bf722 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 23 Apr 2020 13:09:54 +0200 Subject: [PATCH 2/3] Eager-load :properties on supplied products in enterprise_shopfront_serializer --- app/serializers/api/enterprise_shopfront_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/api/enterprise_shopfront_serializer.rb b/app/serializers/api/enterprise_shopfront_serializer.rb index dd76500227..6332222d5b 100644 --- a/app/serializers/api/enterprise_shopfront_serializer.rb +++ b/app/serializers/api/enterprise_shopfront_serializer.rb @@ -119,7 +119,7 @@ module Api private def product_properties - enterprise.supplied_products.flat_map(&:properties) + enterprise.supplied_products.includes(:properties).flat_map(&:properties) end def producer_properties From 43ba73ac1937a551d201864f233a2a5bd145fe89 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 23 Apr 2020 12:25:47 +0200 Subject: [PATCH 3/3] Avoid expensive queries for supplied product properties if enterprise is not a supplier --- .../api/cached_enterprise_serializer.rb | 4 +++ .../api/enterprise_shopfront_serializer.rb | 4 +++ .../api/cached_enterprise_serializer_spec.rb | 31 ++++++++++++++----- .../api/enterprise_serializer_spec.rb | 2 +- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/app/serializers/api/cached_enterprise_serializer.rb b/app/serializers/api/cached_enterprise_serializer.rb index fcb0970e53..17588d4a67 100644 --- a/app/serializers/api/cached_enterprise_serializer.rb +++ b/app/serializers/api/cached_enterprise_serializer.rb @@ -62,10 +62,14 @@ module Api end def supplied_taxons + return [] unless enterprise.is_primary_producer + ids_to_objs data.supplied_taxons[enterprise.id] end def supplied_properties + return [] unless enterprise.is_primary_producer + (product_properties + producer_properties).uniq do |property_object| property_object.property.presentation end diff --git a/app/serializers/api/enterprise_shopfront_serializer.rb b/app/serializers/api/enterprise_shopfront_serializer.rb index 6332222d5b..035f8fde51 100644 --- a/app/serializers/api/enterprise_shopfront_serializer.rb +++ b/app/serializers/api/enterprise_shopfront_serializer.rb @@ -74,12 +74,16 @@ module Api end def supplied_taxons + return [] unless enterprise.is_primary_producer + ActiveModel::ArraySerializer.new( enterprise.supplied_taxons, each_serializer: Api::TaxonSerializer ) end def supplied_properties + return [] unless enterprise.is_primary_producer + (product_properties + producer_properties).uniq do |property_object| property_object.property.presentation end diff --git a/spec/serializers/api/cached_enterprise_serializer_spec.rb b/spec/serializers/api/cached_enterprise_serializer_spec.rb index 6bcaeffdc3..c1046fb7b2 100644 --- a/spec/serializers/api/cached_enterprise_serializer_spec.rb +++ b/spec/serializers/api/cached_enterprise_serializer_spec.rb @@ -9,18 +9,35 @@ describe Api::CachedEnterpriseSerializer do let(:duplicate_property) { create(:property, presentation: 'One') } let(:different_property) { create(:property, presentation: 'Two') } - let(:enterprise) do - create(:enterprise, properties: [duplicate_property, different_property]) - end - before do product = create(:product, properties: [property]) enterprise.supplied_products << product end - it "removes duplicate product and producer properties" do - properties = cached_enterprise_serializer.supplied_properties - expect(properties).to eq([property, different_property]) + context "when the enterprise is a producer" do + let(:enterprise) do + create(:enterprise, + is_primary_producer: true, + properties: [duplicate_property, different_property]) + end + + it "serializes combined product and producer properties without duplicates" do + properties = cached_enterprise_serializer.supplied_properties + expect(properties).to eq([property, different_property]) + end + end + + context "when the enterprise is not a producer" do + let(:enterprise) do + create(:enterprise, + is_primary_producer: false, + properties: [duplicate_property, different_property]) + end + + it "does not serialize supplied properties" do + properties = cached_enterprise_serializer.supplied_properties + expect(properties).to eq([]) + end end end diff --git a/spec/serializers/api/enterprise_serializer_spec.rb b/spec/serializers/api/enterprise_serializer_spec.rb index 5eaa3d3e48..a388fe98d1 100644 --- a/spec/serializers/api/enterprise_serializer_spec.rb +++ b/spec/serializers/api/enterprise_serializer_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Api::EnterpriseSerializer do let(:serializer) { Api::EnterpriseSerializer.new enterprise, data: data } - let(:enterprise) { create(:distributor_enterprise) } + let(:enterprise) { create(:distributor_enterprise, is_primary_producer: true) } let(:taxon) { create(:taxon) } let(:data) { OpenStruct.new(earliest_closing_times: {},