FIX: Keep original subject in emails to staged users

FIX: Keep original subject in emails to staged users

Renaming a topic shouldn’t be visibile to staged users when the topic was created via email.

diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb
index 9d442da..b52a191 100644
--- a/app/mailers/user_notifications.rb
+++ b/app/mailers/user_notifications.rb
@@ -408,8 +408,25 @@ class UserNotifications < ActionMailer::Base
     allow_reply_by_email = opts[:allow_reply_by_email] unless user.suspended?
     original_username = notification_data[:original_username] || notification_data[:display_username]
 
+    if user.staged && post
+      original_subject = IncomingEmail.joins(:post)
+        .where("posts.topic_id = ? AND posts.post_number = 1", post.topic_id)
+        .pluck(:subject)
+        .first
+    end
+
+    if original_subject
+      topic_title = original_subject
+      opts[:use_site_subject] = false
+      opts[:add_re_to_subject] = true
+      use_topic_title_subject = true
+    else
+      topic_title = notification_data[:topic_title]
+      use_topic_title_subject = false
+    end
+
     email_options = {
-      title: notification_data[:topic_title],
+      title: topic_title,
       post: post,
       username: original_username,
       from_alias: I18n.t('email_from', user_name: user_name, site_name: Email.site_title),
@@ -421,6 +438,7 @@ class UserNotifications < ActionMailer::Base
       show_group_in_subject: opts[:show_group_in_subject],
       notification_type: notification_type,
       use_invite_template: opts[:use_invite_template],
+      use_topic_title_subject: use_topic_title_subject,
       user: user
     }
 
@@ -438,6 +456,7 @@ class UserNotifications < ActionMailer::Base
     allow_reply_by_email = opts[:allow_reply_by_email]
     use_site_subject = opts[:use_site_subject]
     add_re_to_subject = opts[:add_re_to_subject] && post.post_number > 1
+    use_topic_title_subject = opts[:use_topic_title_subject]
     username = opts[:username]
     from_alias = opts[:from_alias]
     notification_type = opts[:notification_type]
@@ -622,6 +641,7 @@ class UserNotifications < ActionMailer::Base
       participants: participants,
       include_respond_instructions: !(user.suspended? || user.staged?),
       template: template,
+      use_topic_title_subject: use_topic_title_subject,
       site_description: SiteSetting.site_description,
       site_title: SiteSetting.title,
       site_title_url_encoded: URI.encode(SiteSetting.title),
diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb
index 146f497..66a9230 100644
--- a/lib/email/message_builder.rb
+++ b/lib/email/message_builder.rb
@@ -65,14 +65,18 @@ module Email
       if @opts[:use_site_subject]
         subject = String.new(SiteSetting.email_subject)
         subject.gsub!("%{site_name}", @template_args[:email_prefix])
-        subject.gsub!("%{optional_re}", @opts[:add_re_to_subject] ? I18n.t('subject_re', @template_args) : '')
+        subject.gsub!("%{optional_re}", @opts[:add_re_to_subject] ? I18n.t('subject_re') : '')
         subject.gsub!("%{optional_pm}", @opts[:private_reply] ? @template_args[:subject_pm] : '')
         subject.gsub!("%{optional_cat}", @template_args[:show_category_in_subject] ? "[#{@template_args[:show_category_in_subject]}] " : '')
         subject.gsub!("%{optional_tags}", @template_args[:show_tags_in_subject] ? "#{@template_args[:show_tags_in_subject]} " : '')
         subject.gsub!("%{topic_title}", @template_args[:topic_title]) if @template_args[:topic_title] # must be last for safety
+      elsif @opts[:use_topic_title_subject]
+        subject = @opts[:add_re_to_subject] ? I18n.t('subject_re') : ''
+        subject = "#{subject}#{@template_args[:topic_title]}"
+      elsif @opts[:template]
+        subject = I18n.t("#{@opts[:template]}.subject_template", @template_args)
       else
         subject = @opts[:subject]
-        subject = I18n.t("#{@opts[:template]}.subject_template", @template_args) if @opts[:template]
       end
       subject
     end
diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb
index 5bd5937..09bc56c 100644
--- a/spec/mailers/user_notifications_spec.rb
+++ b/spec/mailers/user_notifications_spec.rb
@@ -385,11 +385,50 @@ describe UserNotifications do
       expect(mail.html_part.to_s).to_not include(response.raw)
       expect(mail.text_part.to_s).to_not include(response.raw)
     end
+
+    it "uses the original subject for staged users" do
+      incoming_email = Fabricate(
+        :incoming_email,
+        subject: "Original Subject",
+        post: post,
+        topic: post.topic,
+        user: user
+      )
+
+      mail = UserNotifications.user_posted(
+        user,
+        post: response,
+        notification_type: notification.notification_type,
+        notification_data_hash: notification.data_hash
+      )
+      expect(mail.subject).to match(/Super cool topic/)
+
+      user.update!(staged: true)
+      mail = UserNotifications.user_posted(
+        user,
+        post: response,
+        notification_type: notification.notification_type,
+        notification_data_hash: notification.data_hash
+      )
+      expect(mail.subject).to eq("Re: Original Subject")
+
+      another_post = Fabricate(:post, topic: topic)
+      incoming_email.update!(post_id: another_post.id)
+
+      mail = UserNotifications.user_private_message(
+        user,
+        post: response,
+        notification_type: notification.notification_type,
+        notification_data_hash: notification.data_hash
+      )
+      expect(mail.subject).to match(/Super cool topic/)
+    end
   end
 
   describe '.user_private_message' do
     let(:response_by_user) { Fabricate(:user, name: "", username: "john") }
-    let(:topic) { Fabricate(:private_message_topic) }
+    let(:topic) { Fabricate(:private_message_topic, title: "Super cool topic") }
+    let(:post) { Fabricate(:post, topic: topic) }
     let(:response) { Fabricate(:post, topic: topic, user: response_by_user) }
     let(:user) { Fabricate(:user) }
     let(:notification) { Fabricate(:private_message_notification, user: user, post: response) }
@@ -524,6 +563,44 @@ describe UserNotifications do
         end
       end
     end
+
+    it "uses the original subject for staged users when topic was started via email" do
+      incoming_email = Fabricate(
+        :incoming_email,
+        subject: "Original Subject",
+        post: post,
+        topic: topic,
+        user: user
+      )
+
+      mail = UserNotifications.user_private_message(
+        user,
+        post: response,
+        notification_type: notification.notification_type,
+        notification_data_hash: notification.data_hash
+      )
+      expect(mail.subject).to match(/Super cool topic/)
+
+      user.update!(staged: true)
+      mail = UserNotifications.user_private_message(
+        user,
+        post: response,
+        notification_type: notification.notification_type,
+        notification_data_hash: notification.data_hash
+      )
+      expect(mail.subject).to eq("Re: Original Subject")
+
+      another_post = Fabricate(:post, topic: topic)
+      incoming_email.update!(post_id: another_post.id)
+
+      mail = UserNotifications.user_private_message(
+        user,
+        post: response,
+        notification_type: notification.notification_type,
+        notification_data_hash: notification.data_hash
+      )
+      expect(mail.subject).to match(/Super cool topic/)
+    end
   end
 
   it 'adds a warning when mail limit is reached' do

GitHub sha: 197e3fd7

2 Likes