Merge pull request #12755 from johansenja/optimise-shops-page6

Optimise shops page: Enable injected enterprise data to be scoped to specific enterprise ids

app/helpers/injection_helper.rb

@@ -8,11 +8,13 @@ module InjectionHelper
   include OrderCyclesHelper
 
   def inject_enterprises(enterprises = nil)
+    enterprises ||= default_enterprise_query
+
     inject_json_array(
       "enterprises",
-      enterprises || default_enterprise_query,
+      enterprises,
       Api::EnterpriseSerializer,
-      enterprise_injection_data,
+      enterprise_injection_data(enterprises.map(&:id)),
     )
   end
 
@@ -57,23 +59,24 @@ def inject_enterprise_and_relatives
     inject_json_array "enterprises",
                       enterprises_and_relatives,
                       Api::EnterpriseSerializer,
-                      enterprise_injection_data
+                      enterprise_injection_data(enterprises_and_relatives.map(&:id))
   end
 
   def inject_group_enterprises(group)
+    enterprises = group.enterprises.activated.visible.all
     inject_json_array(
       "enterprises",
-      group.enterprises.activated.visible.all,
+      enterprises,
       Api::EnterpriseSerializer,
-      enterprise_injection_data,
+      enterprise_injection_data(enterprises.map(&:id)),
     )
   end
 
   def inject_current_hub
     inject_json "currentHub",
                 current_distributor,
                 Api::EnterpriseSerializer,
-                enterprise_injection_data
+                enterprise_injection_data(current_distributor ? [current_distributor.id] : nil)
   end
 
   def inject_current_order
@@ -153,7 +156,9 @@ def default_enterprise_query
     Enterprise.activated.includes(address: [:state, :country]).all
   end
 
-  def enterprise_injection_data
-    @enterprise_injection_data ||= { data: OpenFoodNetwork::EnterpriseInjectionData.new }
+  def enterprise_injection_data(enterprise_ids)
+    {
+      data: OpenFoodNetwork::EnterpriseInjectionData.new(enterprise_ids)
+    }
   end
 end

app/models/order_cycle.rb

@@ -148,16 +148,20 @@ def self.most_recently_closed_for(distributor)
 
   # Find the earliest closing times for each distributor in an active order cycle, and return
   # them in the format {distributor_id => closing_time, ...}
-  def self.earliest_closing_times
-    Hash[
-      Exchange.
-        outgoing.
-        joins(:order_cycle).
-        merge(OrderCycle.active).
-        group('exchanges.receiver_id').
-        pluck(Arel.sql("exchanges.receiver_id AS receiver_id"),
-              Arel.sql("MIN(order_cycles.orders_close_at) AS earliest_close_at"))
-    ]
+  #
+  # Optionally, specify some distributor_ids as a parameter to scope the results
+  def self.earliest_closing_times(distributor_ids = nil)
+    cycles = Exchange.
+      outgoing.
+      joins(:order_cycle).
+      merge(OrderCycle.active).
+      group('exchanges.receiver_id')
+
+    cycles = cycles.where(receiver_id: distributor_ids) if distributor_ids.present?
+
+    cycles.pluck("exchanges.receiver_id AS receiver_id",
+                 "MIN(order_cycles.orders_close_at) AS earliest_close_at")
+      .to_h
   end
 
   def attachable_distributor_payment_methods

app/models/spree/shipping_method.rb

@@ -87,16 +87,20 @@ def delivery?
 
     # Return the services (pickup, delivery) that different distributors provide, in the format:
     # {distributor_id => {pickup: true, delivery: false}, ...}
-    def self.services
-      Hash[
-        Spree::ShippingMethod.
-          joins(:distributor_shipping_methods).
-          group('distributor_id').
-          select("distributor_id").
-          select("BOOL_OR(spree_shipping_methods.require_ship_address = 'f') AS pickup").
-          select("BOOL_OR(spree_shipping_methods.require_ship_address = 't') AS delivery").
-          map { |sm| [sm.distributor_id.to_i, { pickup: sm.pickup, delivery: sm.delivery }] }
-      ]
+    #
+    # Optionally, specify some distributor_ids as a parameter to scope the results
+    def self.services(distributor_ids = nil)
+      methods = Spree::ShippingMethod.joins(:distributor_shipping_methods).group('distributor_id')
+
+      if distributor_ids.present?
+        methods = methods.where(distributor_shipping_methods: { distributor_id: distributor_ids })
+      end
+
+      methods.
+        pluck(Arel.sql("distributor_id"),
+              Arel.sql("BOOL_OR(spree_shipping_methods.require_ship_address = 'f') AS pickup"),
+              Arel.sql("BOOL_OR(spree_shipping_methods.require_ship_address = 't') AS delivery")).
+        to_h { |(distributor_id, pickup, delivery)| [distributor_id.to_i, { pickup:, delivery: }] }
     end
 
     def self.backend

app/models/spree/taxon.rb

@@ -25,26 +25,29 @@ def seo_title
 
     # Find all the taxons of supplied products for each enterprise, indexed by enterprise.
     # Format: {enterprise_id => [taxon_id, ...]}
-    def self.supplied_taxons
-      taxons = {}
+    #
+    # Optionally, specify some enterprise_ids to scope the results
+    def self.supplied_taxons(enterprise_ids = nil)
+      taxons = Spree::Taxon.joins(variants: :supplier)
 
-      Spree::Taxon.
-        joins(variants: :supplier).
-        select('spree_taxons.*, enterprises.id AS enterprise_id').
-        each do |t|
-          taxons[t.enterprise_id.to_i] ||= Set.new
-          taxons[t.enterprise_id.to_i] << t.id
-        end
+      taxons = taxons.where(enterprises: { id: enterprise_ids }) if enterprise_ids.present?
 
       taxons
+        .pluck('spree_taxons.id, enterprises.id AS enterprise_id')
+        .each_with_object({}) do |(taxon_id, enterprise_id), collection|
+        collection[enterprise_id.to_i] ||= Set.new
+        collection[enterprise_id.to_i] << taxon_id
+      end
     end
 
     # Find all the taxons of distributed products for each enterprise, indexed by enterprise.
     # May return :all taxons (distributed in open and closed order cycles),
     # or :current taxons (distributed in an open order cycle).
     #
     # Format: {enterprise_id => [taxon_id, ...]}
-    def self.distributed_taxons(which_taxons = :all)
+    #
+    # Optionally, specify some enterprise_ids to scope the results
+    def self.distributed_taxons(which_taxons = :all, enterprise_ids = nil)
       ents_and_vars = ExchangeVariant.joins(exchange: :order_cycle).merge(Exchange.outgoing)
         .select("DISTINCT variant_id, receiver_id AS enterprise_id")
 
@@ -57,6 +60,10 @@ def self.distributed_taxons(which_taxons = :all)
           INNER JOIN (#{ents_and_vars.to_sql}) AS ents_and_vars
           ON spree_variants.id = ents_and_vars.variant_id")
 
+      if enterprise_ids.present?
+        taxons = taxons.where(ents_and_vars: { enterprise_id: enterprise_ids })
+      end
+
       taxons.each_with_object({}) do |t, ts|
         ts[t.enterprise_id.to_i] ||= Set.new
         ts[t.enterprise_id.to_i] << t.id

lib/open_food_network/enterprise_injection_data.rb

@@ -2,40 +2,55 @@
 
 module OpenFoodNetwork
   class EnterpriseInjectionData
+    # By default, data is fetched for *every* enterprise in the DB, but we specify some ids of
+    # enterprises that we are interested in, there is a lot less data to fetch
+    def initialize(enterprise_ids = nil)
+      @enterprise_ids = enterprise_ids
+    end
+
     def active_distributor_ids
       @active_distributor_ids ||=
-        Enterprise.distributors_with_active_order_cycles.ready_for_checkout.pluck(:id)
+        begin
+          enterprises = Enterprise.distributors_with_active_order_cycles.ready_for_checkout
+          enterprises = enterprises.where(id: @enterprise_ids) if @enterprise_ids.present?
+          enterprises.pluck(:id)
+        end
     end
 
     def earliest_closing_times
-      @earliest_closing_times ||= OrderCycle.earliest_closing_times
+      @earliest_closing_times ||= OrderCycle.earliest_closing_times(@enterprise_ids)
     end
 
     def shipping_method_services
-      @shipping_method_services ||= CacheService.cached_data_by_class("shipping_method_services",
-                                                                      Spree::ShippingMethod) do
+      @shipping_method_services ||= CacheService.cached_data_by_class(
+        "shipping_method_services_#{@enterprise_ids.hash}",
+        Spree::ShippingMethod
+      ) do
         # This result relies on a simple join with DistributorShippingMethod.
         # Updated DistributorShippingMethod records touch their associated Spree::ShippingMethod.
-        Spree::ShippingMethod.services
+        Spree::ShippingMethod.services(@enterprise_ids)
       end
     end
 
     def supplied_taxons
-      @supplied_taxons ||= CacheService.cached_data_by_class("supplied_taxons", Spree::Taxon) do
+      @supplied_taxons ||= CacheService.cached_data_by_class(
+        "supplied_taxons_#{@enterprise_ids.hash}",
+        Spree::Taxon
+      ) do
         # This result relies on a join with associated supplied products, through the
         # class Classification which maps the relationship. Classification records touch
         # their associated Spree::Taxon when updated. A Spree::Product's primary_taxon
         # is also touched when changed.
-        Spree::Taxon.supplied_taxons
+        Spree::Taxon.supplied_taxons(@enterprise_ids)
       end
     end
 
     def all_distributed_taxons
-      @all_distributed_taxons ||= Spree::Taxon.distributed_taxons(:all)
+      @all_distributed_taxons ||= Spree::Taxon.distributed_taxons(:all, @enterprise_ids)
     end
 
     def current_distributed_taxons
-      @current_distributed_taxons ||= Spree::Taxon.distributed_taxons(:current)
+      @current_distributed_taxons ||= Spree::Taxon.distributed_taxons(:current, @enterprise_ids)
     end
   end
 end

spec/helpers/injection_helper_spec.rb

@@ -16,39 +16,60 @@
   }
   let!(:d2o1) { create(:completed_order_with_totals, distributor: distributor2, user_id: user.id) }
 
+  let(:sm) { create(:shipping_method) }
+  let(:pm) { create(:payment_method) }
+  let(:distributor) {
+    create(:distributor_enterprise, shipping_methods: [sm], payment_methods: [pm])
+  }
+  let(:order) { create(:order, distributor:) }
+
+  before do
+    allow_any_instance_of(EnterprisesHelper).to receive(:current_distributor).and_return distributor
+    allow_any_instance_of(EnterprisesHelper).to receive(:current_order).and_return order
+  end
+
   it "will inject via AMS" do
     expect(helper.inject_json_array("test", [enterprise],
                                     Api::IdSerializer)).to match /#{enterprise.id}/
   end
 
-  it "injects enterprises" do
-    expect(helper.inject_enterprises).to match enterprise.name
-    expect(helper.inject_enterprises).to match enterprise.facebook
+  describe "#inject_enterprises" do
+    it "injects enterprises" do
+      expect(helper.inject_enterprises).to match enterprise.name
+      expect(helper.inject_enterprises).to match enterprise.facebook
+    end
+
+    it "only injects activated enterprises" do
+      inactive_enterprise = create(:enterprise, sells: 'unspecified')
+      expect(helper.inject_enterprises).not_to match inactive_enterprise.name
+    end
   end
 
-  it "only injects activated enterprises" do
-    inactive_enterprise = create(:enterprise, sells: 'unspecified')
-    expect(helper.inject_enterprises).not_to match inactive_enterprise.name
+  describe "#inject_enterprise_and_relatives" do
+    let(:child) { create :distributor_enterprise }
+    let!(:relationship) { create :enterprise_relationship, parent: distributor, child: }
+
+    it "injects the current distributor and its relatives" do
+      expect(helper.inject_enterprise_and_relatives).to match distributor.name
+      expect(helper.inject_enterprise_and_relatives).to match child.name
+    end
   end
 
-  it "injects shipping_methods" do
-    sm = create(:shipping_method)
-    current_distributor = create(:distributor_enterprise, shipping_methods: [sm])
-    order = create(:order, distributor: current_distributor)
-    allow(helper).to receive(:current_order) { order }
-    allow(helper).to receive(:spree_current_user) { nil }
+  describe "#inject_group_enterprises" do
+    let(:group) { create :enterprise_group, enterprises: [enterprise] }
+
+    it "injects an enterprise group's enterprises" do
+      expect(helper.inject_group_enterprises(group)).to match enterprise.name
+    end
   end
 
-  it "injects payment methods" do
-    pm = create(:payment_method)
-    current_distributor = create(:distributor_enterprise, payment_methods: [pm])
-    order = create(:order, distributor: current_distributor)
-    allow(helper).to receive(:current_order) { order }
-    allow(helper).to receive(:spree_current_user) { nil }
+  describe "#inject_current_hub" do
+    it "injects the current distributor" do
+      expect(helper.inject_current_hub).to match distributor.name
+    end
   end
 
   it "injects current order" do
-    allow(helper).to receive(:current_order).and_return order = create(:order)
     expect(helper.inject_current_order).to match order.id.to_s
   end
 

spec/lib/open_food_network/enterprise_injection_data_spec.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'open_food_network/enterprise_injection_data'
+
+RSpec.describe OpenFoodNetwork::EnterpriseInjectionData do
+  let(:enterprise1) { create :distributor_enterprise, with_payment_and_shipping: true }
+  let(:enterprise2) { create :distributor_enterprise, with_payment_and_shipping: true }
+  let(:enterprise3) { create :distributor_enterprise, with_payment_and_shipping: true }
+  let(:enterprise4) { create :distributor_enterprise, with_payment_and_shipping: true }
+
+  before do
+    [enterprise1, enterprise2, enterprise3].each do |ent|
+      create :open_order_cycle, distributors: [ent]
+    end
+  end
+
+  let!(:closed_oc) { create :closed_order_cycle, coordinator: enterprise4 }
+
+  context "when scoped to specific enterprises" do
+    subject {
+      described_class.new([enterprise1.id, enterprise2.id])
+    }
+
+    describe "#active_distributor_ids" do
+      it "should include enterprise1.id and enterprise2.id" do
+        ids = subject.active_distributor_ids
+        expect(ids).to include enterprise1.id
+        expect(ids).to include enterprise2.id
+        expect(ids).not_to include enterprise3.id
+      end
+    end
+  end
+
+  context "when unscoped to specific enterprises" do
+    let(:subject) { described_class.new }
+
+    describe "#active_distributor_ids" do
+      it "should include all enterprise ids" do
+        ids = subject.active_distributor_ids
+        expect(ids).to include enterprise1.id
+        expect(ids).to include enterprise2.id
+        expect(ids).to include enterprise3.id
+      end
+    end
+  end
+end

spec/models/order_cycle_spec.rb

@@ -423,6 +423,30 @@
     it "returns the earliest closing time" do
       expect(OrderCycle.earliest_closing_times[e2.id].round).to eq(time2.round)
     end
+
+    context "when scoped by distributors" do
+      it "returns times for the given distributors" do
+        expect(OrderCycle.earliest_closing_times([e1.id])).to have_key e1.id
+        expect(OrderCycle.earliest_closing_times([e2.id])).to have_key e2.id
+      end
+
+      it "doesn't return times for other distributors" do
+        expect(OrderCycle.earliest_closing_times([e1.id])).not_to have_key e2.id
+        expect(OrderCycle.earliest_closing_times([e2.id])).not_to have_key e1.id
+      end
+
+      it "returns the correct values" do
+        expect(OrderCycle.earliest_closing_times([e1.id])[e1.id].round).to eq time1.round
+        expect(OrderCycle.earliest_closing_times([e2.id])[e2.id].round).to eq time2.round
+      end
+    end
+
+    context "when not scoped by distributors" do
+      it "returns times for all distributors" do
+        expect(OrderCycle.earliest_closing_times).to have_key e1.id
+        expect(OrderCycle.earliest_closing_times).to have_key e2.id
+      end
+    end
   end
 
   describe "finding all line items sold by to a user by a given shop" do