diff --git a/app/models/topic.rb b/app/models/topic.rb index ea884ec19ca..d3fbb268dd9 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -253,11 +253,8 @@ class Topic < ActiveRecord::Base # all users (in groups or directly targetted) that are going to get the pm def all_allowed_users - # TODO we should probably change this to 1 query - allowed_user_ids = allowed_users.select('users.id').to_a - allowed_group_user_ids = allowed_group_users.select('users.id').to_a - allowed_staff_ids = private_message? && has_flags? ? User.where(moderator: true).pluck(:id).to_a : [] - User.where('id IN (?)', allowed_user_ids + allowed_group_user_ids + allowed_staff_ids) + moderators_sql = " UNION #{User.moderators.to_sql}" if private_message? && has_flags? + User.from("(#{allowed_users.to_sql} UNION #{allowed_group_users.to_sql}#{moderators_sql}) as users") end # Additional rate limits on topics: per day and private messages per day diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 3ed9e895d60..9ad310e031e 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1315,6 +1315,47 @@ describe Topic do end end + describe 'all_allowed_users' do + let(:group) { Fabricate(:group) } + let(:topic) { Fabricate(:topic, allowed_groups: [group]) } + let!(:allowed_user) { Fabricate(:user) } + let!(:allowed_group_user) { Fabricate(:user) } + let!(:moderator) { Fabricate(:user, moderator: true) } + let!(:rando) { Fabricate(:user) } + + before do + topic.allowed_users << allowed_user + group.users << allowed_group_user + end + + it 'includes allowed_users' do + expect(topic.all_allowed_users).to include allowed_user + end + + it 'includes allowed_group_users' do + expect(topic.all_allowed_users).to include allowed_group_user + end + + it 'includes moderators if flagged and a pm' do + topic.stubs(:has_flags?).returns(true) + topic.stubs(:private_message?).returns(true) + expect(topic.all_allowed_users).to include moderator + end + + it 'does not include moderators if pm without flags' do + topic.stubs(:private_message?).returns(true) + expect(topic.all_allowed_users).not_to include moderator + end + + it 'does not include moderators for regular topic' do + expect(topic.all_allowed_users).not_to include moderator + end + + it 'does not include randos' do + expect(topic.all_allowed_users).not_to include rando + end + end + describe '#listable_count_per_day' do before(:each) do Timecop.freeze