FEATURE: add "email in-reply-to user option"

We no longer include previous replies as "context", instead
we include and excerpt of the post being replied to at the bottom
of notifications, this information was previously missing.

Users may opt in to emailing previous replies if they wish
or opt out of "in-reply-to" which makes sense in some email clients that
are smarter about displaying a tree of replies.

app/assets/javascripts/discourse/models/user.js.es6

@@ -158,6 +158,7 @@ const User = RestModel.extend({
             'external_links_in_new_tab',
             'email_digests',
             'email_direct',
+            'email_in_reply_to',
             'email_private_messages',
             'email_previous_replies',
             'dynamic_favicon',

app/assets/javascripts/discourse/templates/user/preferences.hbs

@@ -180,6 +180,7 @@
         <label>{{i18n 'user.email_previous_replies.title'}}</label>
         {{combo-box valueAttribute="value" content=previousRepliesOptions value=model.user_option.email_previous_replies}}
       </div>
+      {{preference-checkbox labelKey="user.email_in_reply_to" checked=model.user_option.email_in_reply_to}}
       {{preference-checkbox labelKey="user.email_private_messages" checked=model.user_option.email_private_messages}}
       {{preference-checkbox labelKey="user.email_direct" checked=model.user_option.email_direct}}
       <span class="pref-mailing-list-mode">{{preference-checkbox labelKey="user.mailing_list_mode" checked=model.user_option.mailing_list_mode}}</span>

app/mailers/user_notifications.rb

@@ -193,9 +193,9 @@ class UserNotifications < ActionMailer::Base
     include UserNotificationsHelper
   end
 
-  def self.get_context_posts(post, topic_user)
-    user_option = topic_user.try(:user).try(:user_option)
-    if user_option && (user_option.email_previous_replies == UserOption.previous_replies_type[:never])
+  def self.get_context_posts(post, topic_user, user)
+
+    if user.user_option.email_previous_replies == UserOption.previous_replies_type[:never]
       return []
     end
 
@@ -210,7 +210,7 @@ class UserNotifications < ActionMailer::Base
                         .order('created_at desc')
                         .limit(SiteSetting.email_posts_context)
 
-    if topic_user && topic_user.last_emailed_post_number && user_option.try(:email_previous_replies) == UserOption.previous_replies_type[:unless_emailed]
+    if topic_user && topic_user.last_emailed_post_number && user.user_option.email_previous_replies == UserOption.previous_replies_type[:unless_emailed]
       context_posts = context_posts.where("post_number > ?", topic_user.last_emailed_post_number)
     end
 
@@ -280,7 +280,7 @@ class UserNotifications < ActionMailer::Base
 
     context = ""
     tu = TopicUser.get(post.topic_id, user)
-    context_posts = self.class.get_context_posts(post, tu)
+    context_posts = self.class.get_context_posts(post, tu, user)
 
     # make .present? cheaper
     context_posts = context_posts.to_a
@@ -296,11 +296,13 @@ class UserNotifications < ActionMailer::Base
     if opts[:use_template_html]
       topic_excerpt = post.excerpt.gsub("\n", " ") if post.is_first_post? && post.excerpt
     else
+      in_reply_to_post = post.reply_to_post if user.user_option.email_in_reply_to
       html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render(
         template: 'email/notification',
         format: :html,
         locals: { context_posts: context_posts,
                   post: post,
+                  in_reply_to_post: in_reply_to_post,
                   classes: RTL.new(user).css_class
         }
       )

app/models/user_option.rb

@@ -16,6 +16,7 @@ class UserOption < ActiveRecord::Base
     self.automatically_unpin_topics = SiteSetting.default_topics_automatic_unpin
     self.email_private_messages = SiteSetting.default_email_private_messages
     self.email_previous_replies = SiteSetting.default_email_previous_replies
+    self.email_in_reply_to = SiteSetting.default_email_in_reply_to
 
     self.enable_quoting = SiteSetting.default_other_enable_quoting
     self.external_links_in_new_tab = SiteSetting.default_other_external_links_in_new_tab

app/serializers/user_option_serializer.rb

@@ -14,7 +14,8 @@ class UserOptionSerializer < ApplicationSerializer
              :edit_history_public,
              :auto_track_topics_after_msecs,
              :new_topic_duration_minutes,
-             :email_previous_replies
+             :email_previous_replies,
+             :email_in_reply_to
 
 
   def include_edit_history_public?

app/services/user_updater.rb

@@ -21,7 +21,8 @@ class UserUpdater
     :digest_after_days,
     :new_topic_duration_minutes,
     :auto_track_topics_after_msecs,
-    :email_previous_replies
+    :email_previous_replies,
+    :email_in_reply_to
   ]
 
   def initialize(actor, user)
@@ -53,10 +54,10 @@ class UserUpdater
 
 
     save_options = false
-    OPTION_ATTR.each do |attribute|
-      if attributes[attribute].present?
-        save_options = true
 
+    OPTION_ATTR.each do |attribute|
+      if attributes.key?(attribute)
+        save_options = true
 
         if [true,false].include?(user.user_option.send(attribute))
           val = attributes[attribute].to_s == 'true'

app/views/email/_post.html.erb

@@ -1,4 +1,4 @@
-<table class='post-wrapper <%= post.whisper? ? "whisper" : "" %>'>
+<table class='post-wrapper <%= post.whisper? ? "whisper" : "" %> <%= use_excerpt ? "excerpt" : ""%>'>
   <tbody>
     <tr>
       <td>
@@ -23,7 +23,7 @@
       </td>
     </tr>
     <tr>
-      <td class='body'><%= format_for_email(post.cooked) %></td>
+      <td class='body'><%= format_for_email(use_excerpt ? post.excerpt :  post.cooked) %></td>
     </tr>
   </tbody>
 </table>

app/views/email/notification.html.erb

@@ -2,17 +2,22 @@
 
   <div class='header-instructions'>%{header_instructions}</div>
 
-  <%= render partial: 'email/post', locals: { post: post } %>
+  <%= render partial: 'email/post', locals: { post: post, use_excerpt: false } %>
+
+  <% if in_reply_to_post.present? || context_posts.present? %>
+    <div class='footer'>%{respond_instructions}</div>
+    <hr>
+  <% end %>
+
+  <% if in_reply_to_post.present? %>
+    <h4 class='.previous-discussion'><%= t "user_notifications.in_reply_to" %></h4>
+    <%= render partial: 'email/post', locals: { post: in_reply_to_post, use_excerpt: true} %>
+  <% end %>
 
   <% if context_posts.present? %>
-    <div class='footer'>%{respond_instructions}</div>
-
-    <hr>
-
     <h4 class='.previous-discussion'><%= t "user_notifications.previous_discussion" %></h4>
-
     <% context_posts.each do |p| %>
-      <%= render partial: 'email/post', locals: { post: p } %>
+      <%= render partial: 'email/post', locals: { post: p, use_excerpt: false } %>
     <% end %>
   <% end %>
 

config/locales/client.en.yml

@@ -628,7 +628,7 @@ en:
       website: "Web Site"
       email_settings: "Email"
       email_previous_replies:
-        title: "Include previous replies"
+        title: "Include previous replies at the bottom of emails"
         unless_emailed: "unless previously sent"
         always: "always"
         never: "never"
@@ -639,6 +639,7 @@ en:
         weekly: "weekly"
         every_two_weeks: "every two weeks"
 
+      email_in_reply_to: "Include an excerpt of replied to post in emails"
       email_direct: "Send me an email when someone quotes me, replies to my post, mentions my @username, or invites me to a topic"
       email_private_messages: "Send me an email when someone messages me"
       email_always: "Send me email notifications even when I am active on the site"

config/locales/server.en.yml

@@ -1243,6 +1243,8 @@ en:
     default_email_always: "Send an email notification even when the user is active by default."
     default_email_previous_replies: "Include previous replies in emails by default."
 
+    default_email_in_reply_to: "Include excerpt of replied to post in emails by default."
+
     default_other_new_topic_duration_minutes: "Global default condition for which a topic is considered new."
     default_other_auto_track_topics_after_msecs: "Global default time before a topic is automatically tracked."
     default_other_external_links_in_new_tab: "Open external links in a new tab by default."
@@ -1995,6 +1997,7 @@ en:
 
   user_notifications:
     previous_discussion: "Previous Replies"
+    in_reply_to: "In Reply To"
     unsubscribe:
       title: "Unsubscribe"
       description: "Not interested in getting these emails? No problem! Click below to unsubscribe instantly:"

config/site_settings.yml

@@ -1063,7 +1063,9 @@ user_preferences:
   default_email_always: false
   default_email_previous_replies:
     enum: 'PreviousRepliesSiteSetting'
-    default: 1
+    default: 2
+  default_email_in_reply_to:
+    default: true
 
   default_other_new_topic_duration_minutes:
     enum: 'NewTopicDurationSiteSetting'

db/migrate/20160225095306_add_email_in_reply_to_to_user_options.rb

@@ -0,0 +1,11 @@
+class AddEmailInReplyToToUserOptions < ActiveRecord::Migration
+  def up
+    add_column :user_options, :email_in_reply_to, :boolean, null: false, default: true
+    change_column :user_options, :email_previous_replies, :integer, default: 2, null: false
+    execute 'UPDATE user_options SET email_previous_replies = 2'
+  end
+
+  def down
+    remove_column :user_options, :email_in_reply_to
+  end
+end

spec/mailers/user_notifications_spec.rb

@@ -15,11 +15,16 @@ describe UserNotifications do
       _post7 = Fabricate(:post, topic: post1.topic, post_type: Post.types[:whisper])
       last  = Fabricate(:post, topic: post1.topic)
 
+      post1.user.user_option.email_previous_replies = UserOption.previous_replies_type[:always]
+
       # default is only post #1
-      expect(UserNotifications.get_context_posts(last, nil).count).to eq(1)
+      expect(UserNotifications.get_context_posts(last, nil, post1.user).count).to eq(1)
       # staff members can also see the whisper
-      tu = TopicUser.new(topic: post1.topic, user: build(:moderator))
-      expect(UserNotifications.get_context_posts(last, tu).count).to eq(2)
+      moderator = build(:moderator)
+      moderator.user_option = UserOption.new
+      moderator.user_option.email_previous_replies = UserOption.previous_replies_type[:always]
+      tu = TopicUser.new(topic: post1.topic, user: moderator)
+      expect(UserNotifications.get_context_posts(last, tu, tu.user).count).to eq(2)
     end
 
     it "allows users to control context" do
@@ -32,13 +37,15 @@ describe UserNotifications do
       topic_user = TopicUser.find_by(user_id: user.id, topic_id: post1.topic_id)
       # to avoid reloads after update_columns
       user = topic_user.user
-      expect(UserNotifications.get_context_posts(post3, topic_user).count).to eq(1)
+      user.user_option.update_columns(email_previous_replies: UserOption.previous_replies_type[:unless_emailed])
+
+      expect(UserNotifications.get_context_posts(post3, topic_user, user).count).to eq(1)
 
       user.user_option.update_columns(email_previous_replies: UserOption.previous_replies_type[:never])
-      expect(UserNotifications.get_context_posts(post3, topic_user).count).to eq(0)
+      expect(UserNotifications.get_context_posts(post3, topic_user, user).count).to eq(0)
 
       user.user_option.update_columns(email_previous_replies: UserOption.previous_replies_type[:always])
-      expect(UserNotifications.get_context_posts(post3, topic_user).count).to eq(2)
+      expect(UserNotifications.get_context_posts(post3, topic_user, user).count).to eq(2)
 
     end
   end
@@ -110,12 +117,14 @@ describe UserNotifications do
     let(:response_by_user) { Fabricate(:user, name: "John Doe") }
     let(:category) { Fabricate(:category, name: 'India') }
     let(:topic) { Fabricate(:topic, category: category) }
-    let(:post) { Fabricate(:post, topic: topic) }
-    let(:response) { Fabricate(:post, topic: post.topic, user: response_by_user)}
+    let(:post) { Fabricate(:post, topic: topic, raw: 'This is My super duper cool topic') }
+    let(:response) { Fabricate(:post, reply_to_post_number: 1, topic: post.topic, user: response_by_user)}
     let(:user) { Fabricate(:user) }
     let(:notification) { Fabricate(:notification, user: user) }
 
     it 'generates a correct email' do
+
+      # Fabricator is not fabricating this ...
       SiteSetting.enable_names = true
       SiteSetting.display_name_on_posts = true
       mail = UserNotifications.user_replied(response.user,
@@ -123,25 +132,41 @@ describe UserNotifications do
                                              notification_type: notification.notification_type,
                                              notification_data_hash: notification.data_hash
                                            )
-
       # from should include full user name
       expect(mail[:from].display_names).to eql(['John Doe'])
 
       # subject should include category name
       expect(mail.subject).to match(/India/)
 
+      mail_html = mail.html_part.to_s
+
+      expect(mail_html.scan(/My super duper cool topic/).count).to eq(1)
+      expect(mail_html.scan(/In Reply To/).count).to eq(1)
+
       # 2 "visit topic" link
-      expect(mail.html_part.to_s.scan(/Visit Topic/).count).to eq(2)
+      expect(mail_html.scan(/Visit Topic/).count).to eq(2)
 
       # 2 respond to links cause we have 1 context post
-      expect(mail.html_part.to_s.scan(/to respond/).count).to eq(2)
+      expect(mail_html.scan(/to respond/).count).to eq(2)
 
       # 1 unsubscribe
-      expect(mail.html_part.to_s.scan(/To unsubscribe/).count).to eq(1)
+      expect(mail_html.scan(/To unsubscribe/).count).to eq(1)
 
       # side effect, topic user is updated with post number
       tu = TopicUser.get(post.topic_id, response.user)
       expect(tu.last_emailed_post_number).to eq(response.post_number)
+
+
+      # no In Reply To if user opts out
+      response.user.user_option.email_in_reply_to = false
+      mail = UserNotifications.user_replied(response.user,
+                                             post: response,
+                                             notification_type: notification.notification_type,
+                                             notification_data_hash: notification.data_hash
+                                           )
+
+
+      expect(mail.html_part.to_s.scan(/In Reply To/).count).to eq(0)
     end
   end
 
@@ -169,8 +194,8 @@ describe UserNotifications do
       # subject should not include category name
       expect(mail.subject).not_to match(/Uncategorized/)
 
-      # 2 respond to links cause we have 1 context post
-      expect(mail.html_part.to_s.scan(/to respond/).count).to eq(2)
+      # 1 respond to links as no context by default
+      expect(mail.html_part.to_s.scan(/to respond/).count).to eq(1)
 
       # 1 unsubscribe link
       expect(mail.html_part.to_s.scan(/To unsubscribe/).count).to eq(1)

spec/services/user_updater_spec.rb

@@ -48,7 +48,8 @@ describe UserUpdater do
                      mailing_list_mode: true,
                      digest_after_days: "8",
                      new_topic_duration_minutes: 100,
-                     auto_track_topics_after_msecs: 101
+                     auto_track_topics_after_msecs: 101,
+                     email_in_reply_to: false
                     )
       user.reload
 
@@ -58,6 +59,7 @@ describe UserUpdater do
       expect(user.user_option.digest_after_days).to eq 8
       expect(user.user_option.new_topic_duration_minutes).to eq 100
       expect(user.user_option.auto_track_topics_after_msecs).to eq 101
+      expect(user.user_option.email_in_reply_to).to eq false
     end
 
     context 'when update succeeds' do