PERF: speed up about page render time and limit category mods (#8112)

* PERF: speed up about page render time and limit category mods

* Remove return

* Remove widgets

* Convert admins and mods lists

* Rename component

* Apply Joffrey's patch

Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>

* Make limit 100

app/assets/javascripts/discourse/components/about-page-users.js.es6

@@ -0,0 +1,24 @@
+import { userPath } from "discourse/lib/url";
+import { formatUsername, escapeExpression } from "discourse/lib/utilities";
+import { normalize } from "discourse/components/user-info";
+import { renderAvatar } from "discourse/helpers/user-avatar";
+
+export default Ember.Component.extend({
+  usersTemplates: Ember.computed("users.[]", function() {
+    return (this.users || []).map(user => {
+      let name = "";
+      if (user.name && normalize(user.username) !== normalize(user.name)) {
+        name = user.name;
+      }
+
+      return {
+        username: user.username,
+        name,
+        userPath: userPath(user.username),
+        avatar: renderAvatar(user, { imageSize: "large" }),
+        title: escapeExpression(user.title || ""),
+        formatedUsername: formatUsername(user.username)
+      };
+    });
+  })
+});

app/assets/javascripts/discourse/components/user-info.js.es6

@@ -1,7 +1,7 @@
 import computed from "ember-addons/ember-computed-decorators";
 import { userPath } from "discourse/lib/url";
 
-function normalize(name) {
+export function normalize(name) {
   return name.replace(/[\-\_ \.]/g, "").toLowerCase();
 }
 

app/assets/javascripts/discourse/templates/about.hbs

@@ -28,9 +28,7 @@
         <section class='about admins'>
           <h3>{{d-icon "users"}} {{i18n 'about.our_admins'}}</h3>
 
-          {{#each model.admins as |a|}}
-            {{user-info user=a}}
-          {{/each}}
+          {{about-page-users users=model.admins}}
           <div class='clearfix'></div>
 
         </section>
@@ -45,9 +43,7 @@
           <h3>{{d-icon "users"}} {{i18n 'about.our_moderators'}}</h3>
 
           <div class='users'>
-            {{#each model.moderators as |m|}}
-              {{user-info user=m}}
-            {{/each}}
+            {{about-page-users users=model.moderators}}
           </div>
           <div class='clearfix'></div>
         </section>
@@ -62,9 +58,7 @@
           <section class='about category-moderators moderators-{{cm.category.slug}}'>
             <h3>{{category-link cm.category}}{{i18n "about.moderators"}}</h3>
             <div class='users'>
-              {{#each cm.moderators as |m|}}
-                {{user-info user=m}}
-              {{/each}}
+              {{about-page-users users=cm.moderators}}
             </div>
             <div class='clearfix'></div>
           </section>

app/assets/javascripts/discourse/templates/components/about-page-users.hbs

@@ -0,0 +1,22 @@
+{{#each usersTemplates as |userTemplate|}}
+  <div data-username="{{userTemplate.username}}" class="user-info small">
+    <div class="user-image">
+      <div class="user-image-inner">
+        <a href="{{userTemplate.userPath}}" data-user-card="{{userTemplate.username}}">
+          {{{userTemplate.avatar}}}
+        </a>
+      </div>
+    </div>
+    <div class="user-detail">
+      <div class="name-line">
+        <span class="username">
+          <a href="{{userTemplate.userPath}}" data-user-card="{{userTemplate.username}}">
+            {{userTemplate.username}}
+          </a>
+        </span>
+        <span class="name">{{userTemplate.name}}</span>
+      </div>
+      <div class="title">{{userTemplate.title}}</div>
+    </div>
+  </div>
+{{/each}}

app/models/about.rb

@@ -81,18 +81,31 @@ class About
   end
 
   def category_moderators
-    category_ids = Guardian.new(@user).allowed_category_ids
+    allowed_cats = Guardian.new(@user).allowed_category_ids
+    return [] if allowed_cats.blank?
+    cats_with_mods = Category.where.not(reviewable_by_group_id: nil).pluck(:id)
+    category_ids = cats_with_mods & allowed_cats
     return [] if category_ids.blank?
-    results = DB.query(<<~SQL, category_ids: category_ids)
-      SELECT c.id category_id, array_agg(gu.user_id) user_ids
+
+    per_cat_limit = category_mods_limit / category_ids.size
+    per_cat_limit = 1 if per_cat_limit < 1
+    results = DB.query(<<~SQL, category_ids: category_ids, per_cat_limit: per_cat_limit)
+      SELECT c.id category_id, user_ids
       FROM categories c
+      CROSS JOIN LATERAL (
+        SELECT ARRAY(
+          SELECT u.id
+          FROM users u
           JOIN group_users gu
-      ON gu.group_id = reviewable_by_group_id
+          ON gu.group_id = c.reviewable_by_group_id AND gu.user_id = u.id
+          ORDER BY last_seen_at DESC
+          LIMIT :per_cat_limit
+        ) AS user_ids
+      ) user_ids
       WHERE c.id IN (:category_ids)
-      GROUP BY c.id
     SQL
     moderators = {}
-    User.where(id: results.map(&:user_ids).flatten).each do |user|
+    User.where(id: results.map(&:user_ids).flatten.uniq).each do |user|
       moderators[user.id] = user
     end
     moderators
@@ -100,4 +113,12 @@ class About
       CategoryMods.new(row.category_id, row.user_ids.map { |id| moderators[id] })
     end
   end
+
+  def category_mods_limit
+    @category_mods_limit || 100
+  end
+
+  def category_mods_limit=(number)
+    @category_mods_limit = number
+  end
 end

spec/models/about_spec.rb

@@ -10,14 +10,16 @@ describe About do
 
   describe "#category_moderators" do
     let(:user) { Fabricate(:user) }
-    let(:public_cat_moderator) { Fabricate(:user) }
-    let(:private_cat_moderator) { Fabricate(:user) }
-    let(:common_moderator) { Fabricate(:user) }
+    let(:public_cat_moderator) { Fabricate(:user, last_seen_at: 1.month.ago) }
+    let(:private_cat_moderator) { Fabricate(:user, last_seen_at: 2.month.ago) }
+    let(:common_moderator) { Fabricate(:user, last_seen_at: 3.month.ago) }
+    let(:common_moderator_2) { Fabricate(:user, last_seen_at: 4.month.ago) }
 
     let(:public_group) do
       group = Fabricate(:public_group)
       group.add(public_cat_moderator)
       group.add(common_moderator)
+      group.add(common_moderator_2)
       group
     end
 
@@ -25,6 +27,7 @@ describe About do
       group = Fabricate(:group)
       group.add(private_cat_moderator)
       group.add(common_moderator)
+      group.add(common_moderator_2)
       group
     end
 
@@ -37,16 +40,28 @@ describe About do
       expect(results.map(&:moderators).flatten.map(&:id).uniq).to contain_exactly(
         public_cat_moderator.id,
         common_moderator.id,
+        common_moderator_2.id,
         private_cat_moderator.id
       )
 
       [public_cat_moderator, user, nil].each do |u|
         results = About.new(u).category_moderators
         expect(results.map(&:category_id)).to contain_exactly(public_cat.id)
-        expect(results.map(&:moderators).flatten.map(&:id)).to contain_exactly(
+        expect(results.map(&:moderators).flatten.map(&:id)).to eq([
           public_cat_moderator.id,
-          common_moderator.id
-        )
+          common_moderator.id,
+          common_moderator_2.id
+        ])
+      end
+    end
+
+    it "limit category moderators when there are too many for perf reasons" do
+      about = About.new(private_cat_moderator)
+      about.category_mods_limit = 4
+      results = about.category_moderators
+      expect(results.size).to eq(2)
+      results.each do |res|
+        expect(res.moderators.size).to eq(2)
       end
     end
   end