PERF: N+1 queries admin users pages.

approved

#1

PERF: N+1 queries admin users pages.

diff --git a/app/models/user.rb b/app/models/user.rb
index 4c27103..3b1633b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -69,6 +69,11 @@ class User < ActiveRecord::Base
   has_many :oauth2_user_infos, dependent: :destroy
   has_one :instagram_user_info, dependent: :destroy
   has_many :user_second_factors, dependent: :destroy
+
+  has_many :totps, -> {
+    where(method: UserSecondFactor.methods[:totp], enabled: true)
+  }, class_name: "UserSecondFactor"
+
   has_one :user_stat, dependent: :destroy
   has_one :user_profile, dependent: :destroy, inverse_of: :user
   has_one :single_sign_on_record, dependent: :destroy
diff --git a/app/serializers/admin_user_list_serializer.rb b/app/serializers/admin_user_list_serializer.rb
index 4bf2c17..eaa2793 100644
--- a/app/serializers/admin_user_list_serializer.rb
+++ b/app/serializers/admin_user_list_serializer.rb
@@ -119,7 +119,9 @@ class AdminUserListSerializer < BasicUserSerializer
   end
 
   def include_second_factor_enabled?
-    object.totp_enabled?
+    !SiteSetting.enable_sso &&
+      SiteSetting.enable_local_logins &&
+      object.totps.present?
   end
 
   def second_factor_enabled
diff --git a/lib/admin_user_index_query.rb b/lib/admin_user_index_query.rb
index 2341b4d..19df51a 100644
--- a/lib/admin_user_index_query.rb
+++ b/lib/admin_user_index_query.rb
@@ -60,12 +60,15 @@ class AdminUserIndexQuery
       order << "users.username"
     end
 
-    if params[:stats].present? && params[:stats] == false
-      klass.order(order.reject(&:blank?).join(","))
-    else
-      klass.includes(:user_stat, :user_second_factors)
-        .order(order.reject(&:blank?).join(","))
+    query = klass
+      .includes(:totps)
+      .order(order.reject(&:blank?).join(","))
+
+    unless params[:stats].present? && params[:stats] == false
+      query = query.includes(:user_stat)
     end
+
+    query
   end
 
   def filter_by_trust
diff --git a/spec/serializers/admin_user_list_serializer_spec.rb b/spec/serializers/admin_user_list_serializer_spec.rb
index 9a21b68..ec9e648 100644
--- a/spec/serializers/admin_user_list_serializer_spec.rb
+++ b/spec/serializers/admin_user_list_serializer_spec.rb
@@ -2,16 +2,24 @@ require 'rails_helper'
 require_dependency 'user'
 
 describe AdminUserListSerializer do
+  let(:user) { Fabricate(:user_second_factor_totp).user }
+  let(:admin) { Fabricate(:admin) }
+  let(:guardian) { Guardian.new(admin) }
+
+  let(:serializer) do
+    AdminUserListSerializer.new(user, scope: guardian, root: false)
+  end
+
+  it "returns the right values when user has second factor totp enabled" do
+    json = serializer.as_json
+
+    expect(json[:second_factor_enabled]).to eq(true)
+  end
 
   context "emails" do
     let(:admin) { Fabricate(:user_single_email, admin: true, email: "admin@email.com") }
     let(:moderator) { Fabricate(:user_single_email, moderator: true, email: "moderator@email.com") }
     let(:user) { Fabricate(:user_single_email, email: "user@email.com") }
-    let(:guardian) { Guardian.new(admin) }
-
-    let(:serializer) do
-      AdminUserListSerializer.new(user, scope: guardian, root: false)
-    end
 
     def serialize(user, viewed_by, opts = nil)
       AdminUserListSerializer.new(

GitHub sha: c5808a8a


Approved #2