From 426d2178c3cffead2bb73f65c83618a13f827022 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Thu, 28 Sep 2017 12:25:42 +0800
Subject: [PATCH] Fix undefined variable in `TopicCreator`.

---
 lib/topic_creator.rb                  | 29 +++++++++++++++------------
 spec/components/topic_creator_spec.rb | 24 +++++++++++++++++++---
 2 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb
index 5347f464b6b..36c20247c02 100644
--- a/lib/topic_creator.rb
+++ b/lib/topic_creator.rb
@@ -203,18 +203,22 @@ class TopicCreator
   def add_emails(topic, emails)
     return unless emails
 
-    emails = emails.split(',').flatten
-    len = 0
+    begin
+      emails = emails.split(',').flatten
+      len = 0
 
-    emails.each do |email|
-      display_name = email.split("@").first
-      user = find_or_create_user(email, display_name)
-      @added_users << user
-      topic.topic_allowed_users.build(user_id: user.id)
-      len += 1
+      emails.each do |email|
+        display_name = email.split("@").first
+
+        if user = find_or_create_user(email, display_name)
+          @added_users << user
+          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
diff --git a/spec/components/topic_creator_spec.rb b/spec/components/topic_creator_spec.rb
index 712d7718860..0ed814cae12 100644
--- a/spec/components/topic_creator_spec.rb
+++ b/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]