From e27f3323182ce79d112911b0ae9e5b27c1e7d2d9 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Thu, 3 Oct 2019 21:48:56 +0300 Subject: [PATCH] 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 * Make limit 100 --- .../components/about-page-users.js.es6 | 24 +++++++++++++ .../discourse/components/user-info.js.es6 | 2 +- .../javascripts/discourse/templates/about.hbs | 12 ++----- .../templates/components/about-page-users.hbs | 22 ++++++++++++ app/models/about.rb | 35 +++++++++++++++---- spec/models/about_spec.rb | 27 ++++++++++---- 6 files changed, 99 insertions(+), 23 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/about-page-users.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/components/about-page-users.hbs diff --git a/app/assets/javascripts/discourse/components/about-page-users.js.es6 b/app/assets/javascripts/discourse/components/about-page-users.js.es6 new file mode 100644 index 00000000000..e2d65135dbf --- /dev/null +++ b/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) + }; + }); + }) +}); diff --git a/app/assets/javascripts/discourse/components/user-info.js.es6 b/app/assets/javascripts/discourse/components/user-info.js.es6 index e507b305707..21a13477750 100644 --- a/app/assets/javascripts/discourse/components/user-info.js.es6 +++ b/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(); } diff --git a/app/assets/javascripts/discourse/templates/about.hbs b/app/assets/javascripts/discourse/templates/about.hbs index e9be3440321..b09252ff5e8 100644 --- a/app/assets/javascripts/discourse/templates/about.hbs +++ b/app/assets/javascripts/discourse/templates/about.hbs @@ -28,9 +28,7 @@

{{d-icon "users"}} {{i18n 'about.our_admins'}}

- {{#each model.admins as |a|}} - {{user-info user=a}} - {{/each}} + {{about-page-users users=model.admins}}
@@ -45,9 +43,7 @@

{{d-icon "users"}} {{i18n 'about.our_moderators'}}

- {{#each model.moderators as |m|}} - {{user-info user=m}} - {{/each}} + {{about-page-users users=model.moderators}}
@@ -62,9 +58,7 @@

{{category-link cm.category}}{{i18n "about.moderators"}}

- {{#each cm.moderators as |m|}} - {{user-info user=m}} - {{/each}} + {{about-page-users users=cm.moderators}}
diff --git a/app/assets/javascripts/discourse/templates/components/about-page-users.hbs b/app/assets/javascripts/discourse/templates/components/about-page-users.hbs new file mode 100644 index 00000000000..4c946af4669 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/about-page-users.hbs @@ -0,0 +1,22 @@ +{{#each usersTemplates as |userTemplate|}} + +{{/each}} diff --git a/app/models/about.rb b/app/models/about.rb index b24a62cd0f6..df2d3ac3d49 100644 --- a/app/models/about.rb +++ b/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 - JOIN group_users gu - ON gu.group_id = reviewable_by_group_id + CROSS JOIN LATERAL ( + SELECT ARRAY( + SELECT u.id + FROM users u + JOIN group_users gu + 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 diff --git a/spec/models/about_spec.rb b/spec/models/about_spec.rb index c4ab5aba806..4659ff7a208 100644 --- a/spec/models/about_spec.rb +++ b/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