diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb index 52b0ee00708..d282cef8df6 100644 --- a/app/models/topic_list.rb +++ b/app/models/topic_list.rb @@ -110,8 +110,7 @@ class TopicList user_ids << ft.user_id << ft.last_post_user_id << ft.featured_user_ids << ft.allowed_user_ids end - avatar_lookup = AvatarLookup.new(user_ids) - primary_group_lookup = PrimaryGroupLookup.new(user_ids) + user_lookup = UserLookup.new(user_ids) @topics.each do |ft| ft.user_data = @topic_lookup[ft.id] if @topic_lookup.present? @@ -122,11 +121,13 @@ class TopicList end ft.posters = ft.posters_summary( - avatar_lookup: avatar_lookup, - primary_group_lookup: primary_group_lookup + user_lookup: user_lookup ) - ft.participants = ft.participants_summary(avatar_lookup: avatar_lookup, user: @current_user) + ft.participants = ft.participants_summary( + user_lookup: user_lookup, + user: @current_user + ) ft.topic_list = self end diff --git a/app/models/topic_participants_summary.rb b/app/models/topic_participants_summary.rb index df3f385670f..ba365075228 100644 --- a/app/models/topic_participants_summary.rb +++ b/app/models/topic_participants_summary.rb @@ -27,7 +27,7 @@ class TopicParticipantsSummary end def top_participants - user_ids.map { |id| avatar_lookup[id] }.compact.uniq.take(PARTICIPANT_COUNT) + user_ids.map { |id| user_lookup[id] }.compact.uniq.take(PARTICIPANT_COUNT) end def user_ids @@ -35,7 +35,7 @@ class TopicParticipantsSummary [topic.user_id] + topic.allowed_user_ids - [@user.id] end - def avatar_lookup - @avatar_lookup ||= options[:avatar_lookup] || AvatarLookup.new(user_ids) + def user_lookup + @user_lookup ||= options[:user_lookup] || UserLookup.new(user_ids) end end diff --git a/app/models/topic_posters_summary.rb b/app/models/topic_posters_summary.rb index a4299561e20..9eb40bf3d56 100644 --- a/app/models/topic_posters_summary.rb +++ b/app/models/topic_posters_summary.rb @@ -31,7 +31,7 @@ class TopicPostersSummary topic_poster = TopicPoster.new topic_poster.user = user topic_poster.description = descriptions_for(user) - topic_poster.primary_group = primary_group_lookup[user.id] + topic_poster.primary_group = user_lookup.primary_groups[user.id] if topic.last_post_user_id == user.id topic_poster.extras = +'latest' topic_poster.extras << ' single' if user_ids.uniq.size == 1 @@ -70,7 +70,7 @@ class TopicPostersSummary def shuffle_last_poster_to_back_in(summary) unless last_poster_is_topic_creator? summary.reject! { |u| u.id == topic.last_post_user_id } - summary << avatar_lookup[topic.last_post_user_id] + summary << user_lookup[topic.last_post_user_id] end summary end @@ -84,18 +84,14 @@ class TopicPostersSummary end def top_posters - user_ids.map { |id| avatar_lookup[id] }.compact.uniq.take(5) + user_ids.map { |id| user_lookup[id] }.compact.uniq.take(5) end def user_ids [ topic.user_id, topic.last_post_user_id, *topic.featured_user_ids ] end - def avatar_lookup - @avatar_lookup ||= options[:avatar_lookup] || AvatarLookup.new(user_ids) - end - - def primary_group_lookup - @primary_group_lookup ||= options[:primary_group_lookup] || PrimaryGroupLookup.new(user_ids) + def user_lookup + @user_lookup ||= options[:user_lookup] || UserLookup.new(user_ids) end end diff --git a/app/models/user_action.rb b/app/models/user_action.rb index 5346c066258..e41716772b3 100644 --- a/app/models/user_action.rb +++ b/app/models/user_action.rb @@ -167,7 +167,7 @@ class UserAction < ActiveRecord::Base 'u.name AS acting_name' ] - AvatarLookup.lookup_columns.each do |c| + UserLookup.lookup_columns.each do |c| next if c == :id || c['.'] acting_cols << "u.#{c} AS acting_#{c}" end diff --git a/app/models/user_summary.rb b/app/models/user_summary.rb index a62e7d1d850..4b0fc54899f 100644 --- a/app/models/user_summary.rb +++ b/app/models/user_summary.rb @@ -194,7 +194,7 @@ protected def user_counts(user_hash) user_ids = user_hash.keys - lookup = AvatarLookup.new(user_ids) + lookup = UserLookup.new(user_ids) user_ids.map do |user_id| lookup_hash = lookup[user_id] diff --git a/lib/avatar_lookup.rb b/lib/avatar_lookup.rb deleted file mode 100644 index 535c64ebf2a..00000000000 --- a/lib/avatar_lookup.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -class AvatarLookup - - def initialize(user_ids = []) - @user_ids = user_ids.tap(&:compact!).tap(&:uniq!).tap(&:flatten!) - end - - # Lookup a user by id - def [](user_id) - users[user_id] - end - - private - - def self.lookup_columns - @lookup_columns ||= %i{id user_emails.email username name uploaded_avatar_id} - end - - def users - @users ||= user_lookup_hash - end - - def user_lookup_hash - hash = {} - User.joins(:user_emails) - .where(id: @user_ids) - .select(AvatarLookup.lookup_columns) - .each { |user| hash[user.id] = user } - hash - end -end diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 2a9b4c8245f..6b5ece1cd34 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -192,8 +192,8 @@ class Plugin::Instance def custom_avatar_column(column) reloadable_patch do |plugin| - AvatarLookup.lookup_columns << column - AvatarLookup.lookup_columns.uniq! + UserLookup.lookup_columns << column + UserLookup.lookup_columns.uniq! end end diff --git a/lib/primary_group_lookup.rb b/lib/primary_group_lookup.rb deleted file mode 100644 index 8110a3792ba..00000000000 --- a/lib/primary_group_lookup.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -class PrimaryGroupLookup - def initialize(user_ids = []) - @user_ids = user_ids.tap(&:compact!).tap(&:uniq!).tap(&:flatten!) - end - - # Lookup primary group for a given user id - def [](user_id) - users[user_id] - end - - private - - def self.lookup_columns - @lookup_columns ||= %i{id name flair_icon flair_upload_id flair_bg_color flair_color} - end - - def users - @users ||= user_lookup_hash - end - - def user_lookup_hash - users_with_primary_group = User.where(id: @user_ids) - .where.not(primary_group_id: nil) - .select(:id, :primary_group_id) - - group_lookup = {} - group_ids = users_with_primary_group.map(&:primary_group_id) - group_ids.uniq! - - Group.includes(:flair_upload).where(id: group_ids).select(self.class.lookup_columns) - .each { |g| group_lookup[g.id] = g } - - hash = {} - users_with_primary_group.each do |u| - hash[u.id] = group_lookup[u.primary_group_id] - end - hash - end -end diff --git a/lib/topic_query.rb b/lib/topic_query.rb index d8cdfe828bc..bf614e700ec 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -433,16 +433,14 @@ class TopicQuery user_ids << ft.user_id << ft.last_post_user_id << ft.featured_user_ids << ft.allowed_user_ids end - avatar_lookup = AvatarLookup.new(user_ids) - primary_group_lookup = PrimaryGroupLookup.new(user_ids) + user_lookup = UserLookup.new(user_ids) # memoize for loop so we don't keep looking these up translations = TopicPostersSummary.translations topics.each do |t| t.posters = t.posters_summary( - avatar_lookup: avatar_lookup, - primary_group_lookup: primary_group_lookup, + user_lookup: user_lookup, translations: translations ) end diff --git a/lib/user_lookup.rb b/lib/user_lookup.rb new file mode 100644 index 00000000000..faced05ee0d --- /dev/null +++ b/lib/user_lookup.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +class UserLookup + + def initialize(user_ids = []) + @user_ids = user_ids.tap(&:compact!).tap(&:uniq!).tap(&:flatten!) + end + + # Lookup a user by id + def [](user_id) + users[user_id] + end + + def primary_groups + @groups ||= group_lookup_hash + end + + private + + def self.lookup_columns + @user_lookup_columns ||= %i{id username name uploaded_avatar_id primary_group_id} + end + + def self.group_lookup_columns + @group_lookup_columns ||= %i{id name flair_icon flair_upload_id flair_bg_color flair_color} + end + + def users + @users ||= user_lookup_hash + end + + def user_lookup_hash + hash = {} + User.where(id: @user_ids) + .select(self.class.lookup_columns) + .each { |user| hash[user.id] = user } + hash + end + + def group_lookup_hash + users_with_primary_group = users.values.reject { |u| u.primary_group_id.nil? } + + group_lookup = {} + group_ids = users_with_primary_group.map { |u| u.primary_group_id } + group_ids.uniq! + + Group.includes(:flair_upload) + .where(id: group_ids) + .select(self.class.group_lookup_columns) + .each { |g| group_lookup[g.id] = g } + + hash = {} + users_with_primary_group.each do |u| + hash[u.id] = group_lookup[u.primary_group_id] + end + hash + end + +end diff --git a/spec/components/avatar_lookup_spec.rb b/spec/components/avatar_lookup_spec.rb deleted file mode 100644 index 24112bd5d42..00000000000 --- a/spec/components/avatar_lookup_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# encoding: utf-8 -# frozen_string_literal: true - -require 'rails_helper' - -describe AvatarLookup do - fab!(:user) { Fabricate(:user, username: "john_doe", name: "John Doe") } - - describe '#[]' do - before do - @avatar_lookup = AvatarLookup.new([user.id, nil]) - end - - it 'returns nil if user_id does not exists' do - expect(@avatar_lookup[0]).to eq(nil) - end - - it 'returns nil if user_id is nil' do - expect(@avatar_lookup[nil]).to eq(nil) - end - - it 'returns user if user_id exists' do - avatar_lookup_user = @avatar_lookup[user.id] - expect(avatar_lookup_user).to eq(user) - expect(avatar_lookup_user.username).to eq("john_doe") - expect(avatar_lookup_user.name).to eq("John Doe") - end - end -end diff --git a/spec/components/user_lookup_spec.rb b/spec/components/user_lookup_spec.rb new file mode 100644 index 00000000000..b9fce92e67f --- /dev/null +++ b/spec/components/user_lookup_spec.rb @@ -0,0 +1,56 @@ +# encoding: utf-8 +# frozen_string_literal: true + +require 'rails_helper' + +describe UserLookup do + fab!(:user) { Fabricate(:user, username: "john_doe", name: "John Doe") } + + describe '#[]' do + before do + @user_lookup = UserLookup.new([user.id, nil]) + end + + it 'returns nil if user_id does not exists' do + expect(@user_lookup[0]).to eq(nil) + end + + it 'returns nil if user_id is nil' do + expect(@user_lookup[nil]).to eq(nil) + end + + it 'returns user if user_id exists' do + user_lookup_user = @user_lookup[user.id] + expect(user_lookup_user).to eq(user) + expect(user_lookup_user.username).to eq("john_doe") + expect(user_lookup_user.name).to eq("John Doe") + end + end + + describe '#primary_groups' do + fab!(:group) { Fabricate(:group, name: 'testgroup') } + fab!(:user2) { Fabricate(:user, username: "jane_doe", name: "Jane Doe", primary_group: group) } + + before do + @user_lookup = UserLookup.new([user.id, user2.id, nil]) + end + + it 'returns nil if user_id does not exists' do + expect(@user_lookup.primary_groups[0]).to eq(nil) + end + + it 'returns nil if user_id is nil' do + expect(@user_lookup.primary_groups[nil]).to eq(nil) + end + + it 'returns nil if user has no primary group' do + expect(@user_lookup.primary_groups[user.id]).to eq(nil) + end + + it 'returns group if user has primary group' do + user_lookup_group = @user_lookup.primary_groups[user2.id] + expect(user_lookup_group).to eq(group) + expect(user_lookup_group.name).to eq("testgroup") + end + end +end