diff --git a/lib/topic_query.rb b/lib/topic_query.rb index afadc2ddbbd..2e4066fc5c9 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -112,6 +112,7 @@ class TopicQuery ON topic_allowed_groups.group_id = gu.group_id AND user_id = #{@user.id.to_i} ") + .where("gu.group_id IS NOT NULL") .pluck(:group_id) { @@ -729,10 +730,8 @@ class TopicQuery end def new_messages(params) - TopicQuery.new_filter(messages_for_groups_or_user(params[:my_group_ids]), Time.at(SiteSetting.min_new_topics_time).to_datetime) .limit(params[:count]) - end def unread_messages(params) @@ -744,53 +743,60 @@ class TopicQuery end def related_messages_user(params) - user_ids = ((params[:target_user_ids] || []) << -10) - user_ids = ActiveRecord::Base.send(:sanitize_sql_array, user_ids.join(',')) - group_ids = (((params[:target_group_ids] - params[:my_group_ids]) || []) << -10) - group_ids = ActiveRecord::Base.send(:sanitize_sql_array, group_ids.join(',')) - - result = messages_for_user - .limit(params[:count]) - .joins(" - LEFT JOIN topic_allowed_users ta2 - ON topics.id = ta2.topic_id - AND ta2.user_id IN (#{user_ids})", - ) - .joins(" - LEFT JOIN topic_allowed_groups tg - ON topics.id = tg.topic_id - AND tg.group_id IN (#{group_ids})", - ) - .where("ta2.topic_id IS NOT NULL OR tg.topic_id IS NOT NULL") + messages = messages_for_user.limit(params[:count]) + messages = allowed_messages(messages, params) end def related_messages_group(params) - messages_for_groups_or_user(params[:my_group_ids]) - .limit(params[:count]) - .where('topics.id IN ( - SELECT ta.topic_id - FROM topic_allowed_users ta - WHERE ta.user_id IN (:user_ids) - ) OR - topics.id IN ( - SELECT tg.topic_id - FROM topic_allowed_groups tg - WHERE tg.group_id IN (:group_ids) - ) - ', user_ids: (params[:target_user_ids] || []) + [-10], - group_ids: ((params[:target_group_ids] - params[:my_group_ids]) || []) + [-10]) + messages = messages_for_groups_or_user(params[:my_group_ids]).limit(params[:count]) + messages = allowed_messages(messages, params) + end + def allowed_messages(messages, params) + user_ids = (params[:target_user_ids] || []) + group_ids = ((params[:target_group_ids] - params[:my_group_ids]) || []) + + if user_ids.present? + messages = + messages.joins(" + LEFT JOIN topic_allowed_users ta2 + ON topics.id = ta2.topic_id + AND ta2.user_id IN (#{sanitize_sql_array(user_ids)}) + ") + end + + if group_ids.present? + messages = + messages.joins(" + LEFT JOIN topic_allowed_groups tg2 + ON topics.id = tg2.topic_id + AND tg2.group_id IN (#{sanitize_sql_array(group_ids)}) + ") + end + + messages = + if user_ids.present? && group_ids.present? + messages.where("ta2.topic_id IS NOT NULL OR tg2.topic_id IS NOT NULL") + elsif user_ids.present? + messages.where("ta2.topic_id IS NOT NULL") + elsif group_ids.present? + messages.where("tg2.topic_id IS NOT NULL") + end end def messages_for_groups_or_user(group_ids) if group_ids.present? base_messages - .where('topics.id IN ( - SELECT topic_id - FROM topic_allowed_groups tg - JOIN group_users gu ON gu.user_id = :user_id AND gu.group_id = tg.group_id - WHERE gu.group_id IN (:group_ids) - )', user_id: @user.id, group_ids: group_ids) + .joins(" + LEFT JOIN ( + SELECT * FROM topic_allowed_groups _tg + LEFT JOIN group_users gu + ON gu.user_id = #{@user.id.to_i} + AND gu.group_id = _tg.group_id + WHERE gu.group_id IN (#{sanitize_sql_array(group_ids)}) + ) tg ON topics.id = tg.topic_id + ") + .where("tg.topic_id IS NOT NULL") else messages_for_user end @@ -847,4 +853,10 @@ class TopicQuery result.order('topics.bumped_at DESC') end + + private + + def sanitize_sql_array(input) + ActiveRecord::Base.send(:sanitize_sql_array, input.join(',')) + end end diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index 9a8014ff4cd..b7d38eb0364 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -759,6 +759,65 @@ describe TopicQuery do end + context 'with private messages' do + let(:group_user) { Fabricate(:user) } + let(:group) { Fabricate(:group) } + let(:another_group) { Fabricate(:group) } + + let!(:topic) do + Fabricate(:private_message_topic, + topic_allowed_users: [ + Fabricate.build(:topic_allowed_user, user: user) + ], + topic_allowed_groups: [ + Fabricate.build(:topic_allowed_group, group: group) + ] + ) + end + + let!(:private_message) do + Fabricate(:private_message_topic, + topic_allowed_users: [ + Fabricate.build(:topic_allowed_user, user: user) + ], + topic_allowed_groups: [ + Fabricate.build(:topic_allowed_group, group: group), + Fabricate.build(:topic_allowed_group, group: another_group), + ] + ) + end + + let!(:private_group_topic) do + Fabricate(:private_message_topic, + user: Fabricate(:user), + topic_allowed_groups: [ + Fabricate.build(:topic_allowed_group, group: group) + ] + ) + end + + before do + group.add(group_user) + another_group.add(user) + end + + describe 'as user not part of group' do + let!(:user) { Fabricate(:user) } + + it 'should not return topics by the group user' do + expect(suggested_topics).to eq([private_message.id]) + end + end + + describe 'as user part of group' do + let!(:user) { group_user } + + it 'should return the group topics' do + expect(suggested_topics).to eq([private_group_topic.id, private_message.id]) + end + end + end + context 'with some existing topics' do let!(:partially_read) { Fabricate(:post, user: creator).topic } let!(:new_topic) { Fabricate(:post, user: creator).topic }