diff --git a/app/jobs/regular/bulk_invite.rb b/app/jobs/regular/bulk_invite.rb index edf5bf1d157..f2de4c93a3a 100644 --- a/app/jobs/regular/bulk_invite.rb +++ b/app/jobs/regular/bulk_invite.rb @@ -5,7 +5,6 @@ module Jobs class BulkInvite < Jobs::Base sidekiq_options retry: false - attr_accessor :current_user def initialize super @@ -16,8 +15,11 @@ module Jobs def execute(args) filename = args[:filename] - @current_user = User.find_by(id: args[:current_user_id]) raise Discourse::InvalidParameters.new(:filename) if filename.blank? + + @current_user = User.find_by(id: args[:current_user_id]) + raise Discourse::InvalidParameters.new(:current_user_id) unless @current_user + csv_path = "#{Invite.base_directory}/#{filename}" # read csv file, and send out invitations @@ -29,6 +31,8 @@ module Jobs FileUtils.rm_rf(csv_path) if csv_path end + private + def read_csv_file(csv_path) file = File.open(csv_path, encoding: 'bom|utf-8') CSV.new(file).each do |csv_info| @@ -53,11 +57,15 @@ module Jobs def get_group_ids(group_names, csv_line_number) group_ids = [] + if group_names group_names = group_names.split(';') + guardian = Guardian.new(@current_user) + group_names.each { |group_name| group_detail = Group.find_by_name(group_name) - if group_detail + + if group_detail && guardian.can_edit_group?(group_detail) # valid group group_ids.push(group_detail.id) else diff --git a/spec/fixtures/csv/bulk_invite.csv b/spec/fixtures/csv/bulk_invite.csv new file mode 100644 index 00000000000..ea2d897a277 --- /dev/null +++ b/spec/fixtures/csv/bulk_invite.csv @@ -0,0 +1,2 @@ +test2@discourse.org +test@discourse.org,group1;group2,999 diff --git a/spec/jobs/bulk_invite_spec.rb b/spec/jobs/bulk_invite_spec.rb index 573f27639c9..ef0883613a8 100644 --- a/spec/jobs/bulk_invite_spec.rb +++ b/spec/jobs/bulk_invite_spec.rb @@ -1,80 +1,88 @@ require 'rails_helper' describe Jobs::BulkInvite do + describe '#execute' do + let(:user) { Fabricate(:user) } + let!(:group1) { Fabricate(:group, name: 'group1') } + let!(:group2) { Fabricate(:group, name: 'group2') } + let!(:topic) { Fabricate(:topic, id: 999) } + let(:email) { "test@discourse.org" } + let(:csv_info) { [] } + let(:basename) { "bulk_invite.csv" } + let(:filename) { "#{Invite.base_directory}/#{basename}" } - context '.execute' do + before do + FileUtils.cp( + "#{Rails.root}/spec/fixtures/csv/#{basename}", + filename + ) + end it 'raises an error when the filename is missing' do user = Fabricate(:user) - expect { Jobs::BulkInvite.new.execute(current_user_id: user.id) }.to raise_error(Discourse::InvalidParameters) + + expect { Jobs::BulkInvite.new.execute(current_user_id: user.id) } + .to raise_error(Discourse::InvalidParameters, /filename/) end - context '.read_csv_file' do - let(:user) { Fabricate(:user) } - let(:bulk_invite) { Jobs::BulkInvite.new } - let(:csv_file) { "#{Rails.root}/spec/fixtures/csv/discourse.csv" } + it 'raises an error when current_user_id is not valid' do + user = Fabricate(:user) - it 'reads csv file' do - bulk_invite.current_user = user - bulk_invite.read_csv_file(csv_file) - expect(Invite.where(email: "robin@outlook.com").exists?).to eq(true) - expect(Invite.where(email: "jeff@gmail.com").exists?).to eq(true) # handles BOM - end + expect { Jobs::BulkInvite.new.execute(filename: filename) } + .to raise_error(Discourse::InvalidParameters, /current_user_id/) end - context '.send_invite' do - let(:bulk_invite) { Jobs::BulkInvite.new } - let(:user) { Fabricate(:user) } - let(:group) { Fabricate(:group) } - let(:topic) { Fabricate(:topic) } - let(:email) { "evil@trout.com" } - let(:csv_info) { [] } + it 'creates the right invites' do + described_class.new.execute( + current_user_id: Fabricate(:admin).id, + filename: basename, + ) - it 'creates an invite' do - csv_info[0] = email + invite = Invite.last - bulk_invite.current_user = user - bulk_invite.send_invite(csv_info, 1) - expect(Invite.where(email: email).exists?).to eq(true) - end + expect(invite.email).to eq(email) + expect(Invite.exists?(email: "test2@discourse.org")).to eq(true) - it 'creates an invite with group' do - csv_info[0] = email - csv_info[1] = group.name - - bulk_invite.current_user = user - bulk_invite.send_invite(csv_info, 1) - invite = Invite.where(email: email).first - expect(invite).to be_present - expect(InvitedGroup.where(invite_id: invite.id, group_id: group.id).exists?).to eq(true) - end - - it 'creates an invite with topic' do - csv_info[0] = email - csv_info[2] = topic - - bulk_invite.current_user = user - bulk_invite.send_invite(csv_info, 1) - invite = Invite.where(email: email).first - expect(invite).to be_present - expect(TopicInvite.where(invite_id: invite.id, topic_id: topic.id).exists?).to eq(true) - end - - it 'creates an invite with group and topic' do - csv_info[0] = email - csv_info[1] = group.name - csv_info[2] = topic - - bulk_invite.current_user = user - bulk_invite.send_invite(csv_info, 1) - invite = Invite.where(email: email).first - expect(invite).to be_present - expect(InvitedGroup.where(invite_id: invite.id, group_id: group.id).exists?).to eq(true) - expect(TopicInvite.where(invite_id: invite.id, topic_id: topic.id).exists?).to eq(true) - end + expect(invite.invited_groups.pluck(:group_id)).to contain_exactly( + group1.id, group2.id + ) + expect(invite.topic_invites.pluck(:topic_id)).to contain_exactly(topic.id) end + it 'does not create invited groups for automatic groups' do + group2.update!(automatic: true) + + described_class.new.execute( + current_user_id: Fabricate(:admin).id, + filename: basename, + ) + + invite = Invite.last + + expect(invite.email).to eq(email) + + expect(invite.invited_groups.pluck(:group_id)).to contain_exactly( + group1.id + ) + end + + it 'does not create invited groups record if the user can not manage the group' do + group1.add_owner(user) + + described_class.new.execute( + current_user_id: user.id, + filename: basename + ) + + invite = Invite.last + + expect(invite.email).to eq(email) + + expect(invite.invited_groups.pluck(:group_id)).to contain_exactly( + group1.id + ) + end end end