Fix undefined variable in TopicCreator.

lib/topic_creator.rb

@@ -203,18 +203,22 @@ class TopicCreator
   def add_emails(topic, emails)
     return unless emails
 
-    emails = emails.split(',').flatten
+    begin
-    len = 0
+      emails = emails.split(',').flatten
+      len = 0
 
-    emails.each do |email|
+      emails.each do |email|
-      display_name = email.split("@").first
+        display_name = email.split("@").first
-      user = find_or_create_user(email, display_name)
+
-      @added_users << user
+        if user = find_or_create_user(email, display_name)
-      topic.topic_allowed_users.build(user_id: user.id)
+          @added_users << user
-      len += 1
+          topic.topic_allowed_users.build(user_id: user.id)
+          len += 1
+        end
+      end
+    ensure
+      rollback_with!(topic, :target_user_not_found) unless len == emails.length
     end
-
-    rollback_with!(topic, :target_user_not_found) unless len == emails.length
   end
 
   def add_groups(topic, groups)
@@ -239,8 +243,9 @@ class TopicCreator
   def find_or_create_user(email, display_name)
     user = User.find_by_email(email)
 
-    if user.nil? && SiteSetting.enable_staged_users
+    if !user && SiteSetting.enable_staged_users
       username = UserNameSuggester.sanitize_username(display_name) if display_name.present?
+
       user = User.create!(
         email: email,
         username: UserNameSuggester.suggest(username.presence || email),
@@ -250,8 +255,6 @@ class TopicCreator
     end
 
     user
-  rescue
-    rollback_with!(topic, :target_user_not_found)
   end
 
 end

spec/components/topic_creator_spec.rb

@@ -8,7 +8,15 @@ describe TopicCreator do
 
   let(:valid_attrs) { Fabricate.attributes_for(:topic) }
   let(:pm_valid_attrs)  { { raw: 'this is a new post', title: 'this is a new title', archetype: Archetype.private_message, target_usernames: moderator.username } }
-  let(:pm_to_email_valid_attrs) { { raw: 'this is a new email', title: 'this is a new subject', archetype: Archetype.private_message, target_emails: 'moderator@example.com' } }
+
+  let(:pm_to_email_valid_attrs) do
+    {
+      raw: 'this is a new email',
+      title: 'this is a new subject',
+      archetype: Archetype.private_message,
+      target_emails: 'moderator@example.com'
+    }
+  end
 
   describe '#create' do
     context 'topic success cases' do
@@ -72,15 +80,25 @@ describe TopicCreator do
         end
 
         it "should be possible for a trusted user to send private messages via email" do
-          SiteSetting.expects(:enable_staged_users).returns(true)
-          SiteSetting.expects(:enable_staged_users).returns(true)
           SiteSetting.expects(:enable_private_email_messages).returns(true)
           SiteSetting.min_trust_to_send_email_messages = TrustLevel[1]
+
           expect(TopicCreator.create(user, Guardian.new(user), pm_to_email_valid_attrs)).to be_valid
         end
       end
 
       context 'failure cases' do
+        it "should be rollback the changes when email is invalid" do
+          SiteSetting.expects(:enable_private_email_messages).returns(true)
+          SiteSetting.min_trust_to_send_email_messages = TrustLevel[1]
+          attrs = pm_to_email_valid_attrs.dup
+          attrs[:target_emails] = "t" * 256
+
+          expect do
+            TopicCreator.create(user, Guardian.new(user), attrs)
+          end.to raise_error(ActiveRecord::Rollback)
+        end
+
         it "min_trust_to_send_messages setting should be checked when sending private message" do
           SiteSetting.min_trust_to_send_messages = TrustLevel[4]