PERF: Fix N+1 queries when loading groups.

This commit is contained in:
Guo Xiang Tan 2016-11-25 15:26:49 +08:00
parent 712ff01f38
commit 5794f1619d
14 changed files with 73 additions and 44 deletions

View File

@ -2,7 +2,7 @@ import NotificationsButton from 'discourse/components/notifications-button';
export default NotificationsButton.extend({ export default NotificationsButton.extend({
classNames: ['notification-options', 'group-notification-menu'], classNames: ['notification-options', 'group-notification-menu'],
notificationLevel: Em.computed.alias('group.notification_level'), notificationLevel: Em.computed.alias('group.group_user.notification_level'),
i18nPrefix: 'groups.notifications', i18nPrefix: 'groups.notifications',
clicked(id) { clicked(id) {

View File

@ -41,8 +41,16 @@ export default Ember.Controller.extend({
}); });
}, },
@computed('model.is_member') @computed('model.is_group_user')
getTabs(isMember) { getTabs(isGroupUser) {
return this.get('tabs').filter(t => isMember || !t.get('requiresMembership')); return this.get('tabs').filter(t => {
let isMember = false;
if (this.currentUser) {
isMember = this.currentUser.admin || isGroupUser;
}
return isMember || !t.get('requiresMembership');
});
} }
}); });

View File

@ -151,7 +151,7 @@ const Group = Discourse.Model.extend({
}, },
setNotification(notification_level) { setNotification(notification_level) {
this.set("notification_level", notification_level); this.set("group_user.notification_level", notification_level);
return ajax(`/groups/${this.get("name")}/notifications`, { return ajax(`/groups/${this.get("name")}/notifications`, {
data: { notification_level }, data: { notification_level },
type: "POST" type: "POST"

View File

@ -341,7 +341,15 @@ const User = RestModel.extend({
} }
if (!Em.isEmpty(json.user.groups)) { if (!Em.isEmpty(json.user.groups)) {
json.user.groups = json.user.groups.map(g => Group.create(g)); const groups = [];
for(let i = 0; i < json.user.groups.length; i++) {
const group = Group.create(json.user.groups[i]);
group.group_user = json.user.group_users[i];
groups.push(group);
}
json.user.groups = groups;
} }
if (json.user.invited_by) { if (json.user.invited_by) {

View File

@ -1,7 +1,7 @@
class Admin::GroupsController < Admin::AdminController class Admin::GroupsController < Admin::AdminController
def index def index
groups = Group.order(:name).where("id <> ?", Group::AUTO_GROUPS[:everyone]) groups = Group.order(:name).where("groups.id <> ?", Group::AUTO_GROUPS[:everyone])
if search = params[:search].to_s if search = params[:search].to_s
groups = groups.where("name ILIKE ?", "%#{search}%") groups = groups.where("name ILIKE ?", "%#{search}%")

View File

@ -4,7 +4,7 @@ class GroupsController < ApplicationController
skip_before_filter :preload_json, :check_xhr, only: [:posts_feed, :mentions_feed] skip_before_filter :preload_json, :check_xhr, only: [:posts_feed, :mentions_feed]
def show def show
render_serialized(find_group(:id), BasicGroupSerializer) render_serialized(find_group(:id), GroupShowSerializer, root: 'basic_group')
end end
def counts def counts

View File

@ -11,10 +11,7 @@ class BasicGroupSerializer < ApplicationSerializer
:title, :title,
:grant_trust_level, :grant_trust_level,
:incoming_email, :incoming_email,
:notification_level,
:has_messages, :has_messages,
:is_member,
:mentionable,
:flair_url, :flair_url,
:flair_bg_color, :flair_bg_color,
:flair_color :flair_color
@ -22,30 +19,4 @@ class BasicGroupSerializer < ApplicationSerializer
def include_incoming_email? def include_incoming_email?
scope.is_staff? scope.is_staff?
end end
def notification_level
fetch_group_user&.notification_level
end
def include_notification_level?
scope.authenticated?
end
def mentionable
object.mentionable?(scope.user, object.id)
end
def is_member
scope.is_admin? || fetch_group_user.present?
end
def include_is_member?
scope.authenticated?
end
private
def fetch_group_user
@group_user ||= object.group_users.find_by(user_id: scope.user.id)
end
end end

View File

@ -0,0 +1,3 @@
class BasicGroupUserSerializer < ApplicationSerializer
attributes :group_id, :user_id, :notification_level
end

View File

@ -0,0 +1,11 @@
class GroupSerializer < BasicGroupSerializer
attributes :mentionable
def mentionable
object.mentionable?(scope.user, object.id)
end
def include_mentionable?
authenticated?
end
end

View File

@ -0,0 +1,11 @@
class GroupShowSerializer < BasicGroupSerializer
attributes :is_group_user
def include_is_group_user?
scope.authenticated?
end
def is_group_user
object.users.include?(scope.user)
end
end

View File

@ -73,6 +73,7 @@ class UserSerializer < BasicUserSerializer
has_one :invited_by, embed: :object, serializer: BasicUserSerializer has_one :invited_by, embed: :object, serializer: BasicUserSerializer
has_many :groups, embed: :object, serializer: BasicGroupSerializer has_many :groups, embed: :object, serializer: BasicGroupSerializer
has_many :group_users, embed: :object, serializer: BasicGroupUserSerializer
has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges
has_one :card_badge, embed: :object, serializer: BadgeSerializer has_one :card_badge, embed: :object, serializer: BadgeSerializer
has_one :user_option, embed: :object, serializer: UserOptionSerializer has_one :user_option, embed: :object, serializer: UserOptionSerializer
@ -127,13 +128,19 @@ class UserSerializer < BasicUserSerializer
end end
def groups def groups
groups = object.groups.order(:id)
if scope.is_admin? || object.id == scope.user.try(:id) if scope.is_admin? || object.id == scope.user.try(:id)
object.groups groups
else else
object.groups.where(visible: true) groups.where(visible: true)
end end
end end
def group_users
object.group_users.order(:group_id)
end
def include_email? def include_email?
object.id && object.id == scope.user.try(:id) object.id && object.id == scope.user.try(:id)
end end

View File

@ -34,10 +34,7 @@ describe Admin::GroupsController do
"primary_group"=>false, "primary_group"=>false,
"grant_trust_level"=>nil, "grant_trust_level"=>nil,
"incoming_email"=>nil, "incoming_email"=>nil,
"notification_level"=>2,
"has_messages"=>false, "has_messages"=>false,
"is_member"=>true,
"mentionable"=>false,
"flair_url"=>nil, "flair_url"=>nil,
"flair_bg_color"=>nil, "flair_bg_color"=>nil,
"flair_color"=>nil "flair_color"=>nil

View File

@ -1,4 +1,5 @@
import { acceptance } from "helpers/qunit-helpers"; import { acceptance, logIn } from "helpers/qunit-helpers";
acceptance("Groups"); acceptance("Groups");
test("Browsing Groups", () => { test("Browsing Groups", () => {
@ -24,6 +25,18 @@ test("Browsing Groups", () => {
visit("/groups/discourse/messages"); visit("/groups/discourse/messages");
andThen(() => { andThen(() => {
ok($('.action-list li').length === 4, 'it should not show messages tab');
ok(count('.user-stream .item') > 0, "it lists stream items"); ok(count('.user-stream .item') > 0, "it lists stream items");
}); });
}); });
test("Messages tab", () => {
logIn();
Discourse.reset();
visit("/groups/discourse");
andThen(() => {
ok($('.action-list li').length === 5, 'it should show messages tab if user is admin');
});
});