PERF: Optimize reviewable queries by eager loading (#36618)

KrisKotlarek · web-flow · commit 171a28ce2fdd · 2025-12-11T15:53:16.000+08:00
Improved query performance by eagerly loading user_stat, primary_email, and topic/category associations when fetching the target. This prevents N+1 queries when accessing these commonly-needed associations in the reviewables list. In addition, running count is breaking eager_loading and causing N+1. Using goldiload to prevent it. Before <img width="846" height="169" alt="Screenshot 2025-12-11 at 11 30 09 am" src="https://github.com/user-attachments/assets/6053ca8f-edd6-4488-8db2-7ec196fe1196" /> After <img width="843" height="170" alt="Screenshot 2025-12-11 at 2 21 02 pm" src="https://github.com/user-attachments/assets/1d0f414c-8872-4c9f-9f32-080fd8a7b67c" />
diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb
@@ -462,8 +462,16 @@ def self.viewable_by(user, order: nil, preload: true)
           .includes(
             { created_by: :user_stat },
             :topic,
-            :target,
-            :target_created_by,
+            {
+              target: [
+                :user_stat,
+                :primary_email,
+                { topic: :category },
+                :user_histories,
+                :user_custom_fields,
+              ],
+            },
+            { target_created_by: [:user_custom_fields] },
             :reviewable_histories,
           )
           .includes(reviewable_scores: { user: :user_stat, meta_topic: :posts })
diff --git a/app/models/user.rb b/app/models/user.rb
@@ -92,6 +92,7 @@ class User < ActiveRecord::Base
   has_many :do_not_disturb_timings, dependent: :delete_all
   has_many :reviewable_histories, foreign_key: :created_by_id, dependent: :delete_all
   has_many :sidebar_sections, dependent: :destroy
+  has_many :user_histories, foreign_key: :target_user_id
   has_one :user_status, dependent: :destroy
 
   # dependent deleting handled via before_destroy (special cases)
@@ -1329,7 +1330,7 @@ def silenced?
   end
 
   def silenced_record
-    UserHistory.for(self, :silence_user).order("id DESC").first
+    user_histories.where(action: UserHistory.actions[:silence_user]).order("id DESC").first
   end
 
   def silence_reason
@@ -1345,7 +1346,7 @@ def silenced_forever?
   end
 
   def suspend_record
-    UserHistory.for(self, :suspend_user).order("id DESC").first
+    user_histories.where(action: UserHistory.actions[:suspend_user]).order("id DESC").first
   end
 
   def full_suspend_reason
@@ -1638,7 +1639,12 @@ def number_of_flagged_posts
   end
 
   def number_of_rejected_posts
-    ReviewableQueuedPost.rejected.where(target_created_by_id: self.id).count
+    goldiload do |ids|
+      ReviewableQueuedPost
+        .where(status: "rejected", target_created_by_id: ids)
+        .group(:target_created_by_id)
+        .count
+    end
   end
 
   def number_of_flags_given
@@ -1650,11 +1656,21 @@ def number_of_flags_given
   end
 
   def number_of_silencings
-    UserHistory.for(self, :silence_user).count
+    goldiload do |ids|
+      UserHistory
+        .where(target_user_id: ids, action: UserHistory.actions[:silence_user])
+        .group(:target_user_id)
+        .count
+    end
   end
 
   def number_of_suspensions
-    UserHistory.for(self, :suspend_user).count
+    goldiload do |ids|
+      UserHistory
+        .where(target_user_id: ids, action: UserHistory.actions[:suspend_user])
+        .group(:target_user_id)
+        .count
+    end
   end
 
   def create_user_profile
diff --git a/app/serializers/flagged_user_serializer.rb b/app/serializers/flagged_user_serializer.rb
@@ -44,15 +44,15 @@ def flags_ignored
   end
 
   def silenced_count
-    object.number_of_silencings
+    object.number_of_silencings.to_i
   end
 
   def suspended_count
-    object.number_of_suspensions
+    object.number_of_suspensions.to_i
   end
 
   def rejected_posts_count
-    object.number_of_rejected_posts
+    object.number_of_rejected_posts.to_i
   end
 
   def custom_fields
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
@@ -2134,7 +2134,7 @@ def hash(password, salt, algorithm = UserPassword::TARGET_PASSWORD_ALGORITHM)
           status: Reviewable.statuses[:approved],
         )
 
-        expect(user.number_of_rejected_posts).to eq(0)
+        expect(user.number_of_rejected_posts.to_i).to eq(0)
       end
     end
 
@@ -2158,6 +2158,10 @@ def hash(password, salt, algorithm = UserPassword::TARGET_PASSWORD_ALGORITHM)
           Fabricate(:user_history, action: UserHistory.actions[:silence_user], target_user: user)
         end
         expect(user.number_of_silencings).to eq(3)
+
+        # ignores other users' history
+        Fabricate(:user_history, action: UserHistory.actions[:silence_user])
+        expect(user.reload.number_of_silencings).to eq(3)
       end
     end
   end
