Merge pull request #5196 from Matt-Yorkley/customers-performance

Admin Customers performance

app/controllers/admin/customers_controller.rb

@@ -17,8 +17,9 @@ module Admin
       respond_to do |format|
         format.html
         format.json do
-          tag_rule_mapping = TagRule.mapping_for(Enterprise.where(id: params[:enterprise_id]))
-          render_as_json @collection, tag_rule_mapping: tag_rule_mapping
+          render_as_json @collection,
+                         tag_rule_mapping: tag_rule_mapping,
+                         customer_tags: customer_tags_by_id
         end
       end
     end
@@ -64,8 +65,13 @@ module Admin
     def collection
       return Customer.where("1=0") unless json_request? && params[:enterprise_id].present?
 
-      enterprise = Enterprise.managed_by(spree_current_user).find_by_id(params[:enterprise_id])
-      Customer.of(enterprise)
+      Customer.of(managed_enterprise_id).
+        includes(:bill_address, :ship_address, user: :credit_cards)
+    end
+
+    def managed_enterprise_id
+      @managed_enterprise_id ||= Enterprise.managed_by(spree_current_user).
+        select('enterprises.id').find_by_id(params[:enterprise_id])
     end
 
     def load_managed_shops
@@ -80,5 +86,26 @@ module Admin
     def ams_prefix_whitelist
       [:subscription]
     end
+
+    def tag_rule_mapping
+      TagRule.mapping_for(Enterprise.where(id: managed_enterprise_id))
+    end
+
+    # Fetches tags for all customers of the enterprise and returns a hash indexed by customer_id
+    def customer_tags_by_id
+      customer_tags = ::ActsAsTaggableOn::Tag.
+        joins(:taggings).
+        includes(:taggings).
+        where(taggings:
+                { taggable_type: 'Customer',
+                  taggable_id: Customer.of(managed_enterprise_id),
+                  context: 'tags' })
+
+      customer_tags.each_with_object({}) do |tag, indexed_hash|
+        customer_id = tag.taggings.first.taggable_id
+        indexed_hash[customer_id] ||= []
+        indexed_hash[customer_id] << tag.name
+      end
+    end
   end
 end

app/models/spree/user.rb

@@ -125,7 +125,12 @@ module Spree
     end
 
     def default_card
-      credit_cards.where(is_default: true).first
+      # Don't re-fetch associated cards from the DB if they're already eager-loaded
+      if credit_cards.loaded?
+        credit_cards.to_a.find(&:is_default)
+      else
+        credit_cards.where(is_default: true).first
+      end
     end
 
     # Checks whether the specified user is a superadmin, with full control of the

app/serializers/api/admin/customer_serializer.rb

@@ -6,7 +6,7 @@ class Api::Admin::CustomerSerializer < ActiveModel::Serializer
   has_one :bill_address, serializer: Api::AddressSerializer
 
   def tag_list
-    object.tag_list.join(",")
+    customer_tag_list.join(",")
   end
 
   def name
@@ -14,7 +14,7 @@ class Api::Admin::CustomerSerializer < ActiveModel::Serializer
   end
 
   def tags
-    object.tag_list.map do |tag|
+    customer_tag_list.map do |tag|
       tag_rule_map = options[:tag_rule_mapping].andand[tag]
       tag_rule_map || { text: tag, rules: nil }
     end
@@ -25,4 +25,12 @@ class Api::Admin::CustomerSerializer < ActiveModel::Serializer
 
     object.user.default_card.present?
   end
+
+  private
+
+  def customer_tag_list
+    return object.tag_list unless options[:customer_tags]
+
+    options[:customer_tags].andand[object.id] || []
+  end
 end

spec/serializers/api/admin/customer_serializer_spec.rb

@@ -1,12 +1,16 @@
 require 'spec_helper'
 
 describe Api::Admin::CustomerSerializer do
-  let(:customer) { create(:customer, tag_list: "one, two, three") }
+  let(:tag_list) { ["one", "two", "three"] }
+  let(:customer) { create(:customer, tag_list: tag_list) }
   let!(:tag_rule) { create(:tag_rule, enterprise: customer.enterprise, preferred_customer_tags: "two") }
 
   it "serializes a customer with tags" do
     tag_rule_mapping = TagRule.mapping_for(Enterprise.where(id: customer.enterprise_id))
-    serializer = Api::Admin::CustomerSerializer.new customer, tag_rule_mapping: tag_rule_mapping
+    customer_tag_list = { customer.id => tag_list }
+    serializer = Api::Admin::CustomerSerializer.new customer,
+                                                    tag_rule_mapping: tag_rule_mapping,
+                                                    customer_tags: customer_tag_list
     result = JSON.parse(serializer.to_json)
     expect(result['email']).to eq customer.email
     tags = result['tags']
@@ -27,4 +31,11 @@ describe Api::Admin::CustomerSerializer do
       expect(tag['rules']).to be nil
     end
   end
+
+  it 'serializes a customer without customer_tags' do
+    serializer = Api::Admin::CustomerSerializer.new customer
+    result = JSON.parse(serializer.to_json)
+
+    expect(result['tags'].first['text']).to eq tag_list.first
+  end
 end