FIX: Bulk invite should not add users to automatic groups.

* Also checks that the user creating the bulk invite has permission.

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

spec/fixtures/csv/bulk_invite.csv

@@ -0,0 +1,2 @@
+test2@discourse.org
+test@discourse.org,group1;group2,999

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