Discourse as a Mailing List improvements

FEATURE: context is not emailed if we previously emailed you the post
FEATURE: site setting to enable_watch_new_topics , false by default.
  When enables users can elect to watch everything by default
FIX: Custom email subjects (x quoted you in [title], x replied to [title])
  was removed, this broke email grouping. TBD, include info in footer somehow
FIX: topic user specs were messy, reduce side effects

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

@@ -180,6 +180,7 @@ Discourse.User = Discourse.Model.extend({
                                'digest_after_days',
                                'new_topic_duration_minutes',
                                'external_links_in_new_tab',
+                               'watch_new_topics',
                                'enable_quoting'),
       type: 'PUT'
     }).then(function(data) {

app/assets/javascripts/discourse/templates/user/preferences.js.handlebars

@@ -109,6 +109,15 @@
         {{combobox valueAttribute="value" content=considerNewTopicOptions value=new_topic_duration_minutes}}
       </div>
 
+      {{#if Discourse.SiteSettings.enable_watch_new_topics}}
+      <div class="controls">
+        <label>
+          {{view Ember.Checkbox checkedBinding="watch_new_topics"}}
+          {{i18n user.watch_new_topics}}
+        </label>
+      </div>
+      {{/if}}
+
       <div class="controls">
         <label>{{view Ember.Checkbox checkedBinding="external_links_in_new_tab"}}
         {{i18n user.external_links_in_new_tab}}</label>

app/mailers/user_notifications.rb

@@ -113,12 +113,21 @@ class UserNotifications < ActionMailer::Base
     notification_type = opts[:notification_type] || Notification.types[@notification.notification_type].to_s
 
     context = ""
+    tu = TopicUser.get(@post.topic_id, user)
+
     context_posts = Post.where(topic_id: @post.topic_id)
                         .where("post_number < ?", @post.post_number)
                         .where(user_deleted: false)
                         .order('created_at desc')
                         .limit(SiteSetting.email_posts_context)
 
+    if tu && tu.last_emailed_post_number
+      context_posts = context_posts.where("post_number > ?", tu.last_emailed_post_number)
+    end
+
+    # make .present? cheaper
+    context_posts = context_posts.to_a
+
     if context_posts.present?
       context << "---\n*#{I18n.t('user_notifications.previous_discussion')}*\n"
       context_posts.each do |cp|
@@ -157,6 +166,8 @@ class UserNotifications < ActionMailer::Base
       email_opts[:from_alias] = username
     end
 
+    TopicUser.change(user.id, @post.topic_id, last_emailed_post_number: @post.post_number)
+
     build_email(user.email, email_opts)
   end
 

app/models/post_alert_observer.rb

@@ -123,17 +123,16 @@ class PostAlertObserver < ActiveRecord::Observer
       reply_to_user = post.reply_notification_target
       notify_users(reply_to_user, :replied, post)
 
-      # find all users watching
-      if post.post_number > 1
-        exclude_user_ids = []
-        exclude_user_ids << post.user_id
-        exclude_user_ids << reply_to_user.id if reply_to_user.present?
-        exclude_user_ids << extract_mentioned_users(post).map(&:id)
-        exclude_user_ids << extract_quoted_users(post).map(&:id)
-        exclude_user_ids.flatten!
-        TopicUser.where(topic_id: post.topic_id, notification_level: TopicUser.notification_levels[:watching]).includes(:user).each do |tu|
+      exclude_user_ids = []
+      exclude_user_ids << post.user_id
+      exclude_user_ids << reply_to_user.id if reply_to_user.present?
+      exclude_user_ids << extract_mentioned_users(post).map(&:id)
+      exclude_user_ids << extract_quoted_users(post).map(&:id)
+      exclude_user_ids.flatten!
+      TopicUser
+        .where(topic_id: post.topic_id, notification_level: TopicUser.notification_levels[:watching])
+        .includes(:user).each do |tu|
           create_notification(tu.user, Notification.types[:posted], post) unless exclude_user_ids.include?(tu.user_id)
         end
-      end
     end
 end

app/models/topic_user.rb

@@ -20,7 +20,7 @@ class TopicUser < ActiveRecord::Base
     end
 
     def notification_reasons
-      @notification_reasons ||= Enum.new(:created_topic, :user_changed, :user_interacted, :created_post)
+      @notification_reasons ||= Enum.new(:created_topic, :user_changed, :user_interacted, :created_post, :auto_watch)
     end
 
     def auto_track(user_id, topic_id, reason)
@@ -64,6 +64,24 @@ class TopicUser < ActiveRecord::Base
       TopicUser.where('topic_id = ? and user_id = ?', topic, user).first
     end
 
+    def auto_watch_new_topic(topic_id)
+      # Can not afford to slow down creation of topics when a pile of users are watching new topics, reverting to SQL for max perf here
+      sql = <<SQL
+      INSERT INTO topic_users(user_id, topic_id, notification_level, notifications_reason_id)
+      SELECT id, :topic_id, :level, :reason
+      FROM users
+      WHERE watch_new_topics AND
+            NOT EXISTS(SELECT 1 FROM topic_users WHERE topic_id = :topic_id AND user_id = users.id)
+SQL
+
+      exec_sql(
+          sql,
+                    topic_id: topic_id,
+                    level: notification_levels[:watching],
+                    reason: notification_reasons[:auto_watch]
+              )
+    end
+
     # Change attributes for a user (creates a record when none is present). First it tries an update
     # since there's more likely to be an existing record than not. If the update returns 0 rows affected
     # it then creates the row instead.

app/serializers/user_serializer.rb

@@ -24,7 +24,7 @@ class UserSerializer < BasicUserSerializer
   has_one :invited_by, embed: :object, serializer: BasicUserSerializer
 
   def self.private_attributes(*attrs)
-    attributes *attrs
+    attributes(*attrs)
     attrs.each do |attr|
       define_method "include_#{attr}?" do
         can_edit
@@ -51,6 +51,7 @@ class UserSerializer < BasicUserSerializer
                      :email_direct,
                      :email_always,
                      :digest_after_days,
+                     :watch_new_topics,
                      :auto_track_topics_after_msecs,
                      :new_topic_duration_minutes,
                      :external_links_in_new_tab,

app/services/user_updater.rb

@@ -30,7 +30,8 @@ class UserUpdater
       :email_private_messages,
       :external_links_in_new_tab,
       :enable_quoting,
-      :dynamic_favicon
+      :dynamic_favicon,
+      :watch_new_topics
     ].each do |attribute|
       if attributes[attribute].present?
         user.send("#{attribute.to_s}=", attributes[attribute] == 'true')

config/locales/client.en.yml

@@ -227,6 +227,7 @@ en:
       deleted: "(deleted)"
       suspended_notice: "This user is suspended until {{date}}."
       suspended_reason: "Reason: "
+      watch_new_topics: "Automatically watch all new topics posted on the forum"
 
       messages:
         all: "All"
@@ -693,6 +694,7 @@ en:
       notifications:
         title: ''
         reasons:
+          "3_5": 'You will receive notifications because you started watching this topic automatically.'
           "3_2": 'You will receive notifications because you are watching this topic.'
           "3_1": 'You will receive notifications because you created this topic.'
           "3": 'You will receive notifications because you are watching this topic.'

config/locales/server.en.yml

@@ -728,6 +728,7 @@ en:
     pop3s_polling_host: "The host to poll for email via POP3S"
     pop3s_polling_username: "The username for the POP3S account to poll for email"
     pop3s_polling_password: "The password for the POP3S account to poll for email"
+    enable_watch_new_topics: "Allow users to watch all new topics (optionally) via a user preference"
 
     minimum_topics_similar: "How many topics need to exist in the database before similar topics are presented."
 
@@ -1096,7 +1097,7 @@ en:
         Please visit this link to view the topic: %{base_url}%{url}
 
     user_replied:
-      subject_template: "[%{site_name}] new reply to your post in '%{topic_title}'"
+      subject_template: "[%{site_name}] %{topic_title}"
       text_body_template: |
         %{message}
 
@@ -1106,7 +1107,7 @@ en:
         %{respond_instructions}
 
     user_quoted:
-      subject_template: "[%{site_name}] %{username} quoted you in '%{topic_title}'"
+      subject_template: "[%{site_name}] %{topic_title}"
       text_body_template: |
         %{message}
 
@@ -1116,7 +1117,7 @@ en:
         %{respond_instructions}
 
     user_mentioned:
-      subject_template: "[%{site_name}] %{username} mentioned you in '%{topic_title}'"
+      subject_template: "[%{site_name}] %{topic_title}"
       text_body_template: |
         %{message}
 
@@ -1126,7 +1127,7 @@ en:
         %{respond_instructions}
 
     user_posted:
-      subject_template: "[%{site_name}] %{subject_prefix}new post in '%{topic_title}'"
+      subject_template: "[%{site_name}] %{topic_title}"
       text_body_template: |
         %{message}
 

config/site_settings.yml

@@ -225,6 +225,9 @@ email:
   pop3s_polling_port: 995
   pop3s_polling_username: ''
   pop3s_polling_password: ''
+  enable_watch_new_topics:
+    default: false
+    client: true
 
 files:
   max_image_size_kb:

db/migrate/20131229221725_add_watch_new_topics_to_users.rb

@@ -0,0 +1,5 @@
+class AddWatchNewTopicsToUsers < ActiveRecord::Migration
+  def change
+    add_column :users, :watch_new_topics, :boolean, default: false, null: false
+  end
+end

db/migrate/20131230010239_add_last_emailed_post_number_to_topic_user.rb

@@ -0,0 +1,5 @@
+class AddLastEmailedPostNumberToTopicUser < ActiveRecord::Migration
+  def change
+    add_column :topic_users, :last_emailed_post_number, :integer
+  end
+end

lib/topic_creator.rb

@@ -32,6 +32,7 @@ class TopicCreator
     unless @opts[:auto_track] == false
       @topic.notifier.watch_topic!(@topic.user_id)
     end
+    TopicUser.auto_watch_new_topic(@topic.id)
   end
 
   def setup

spec/mailers/user_notifications_spec.rb

@@ -64,6 +64,9 @@ describe UserNotifications do
       # 1 unsubscribe
       mail.html_part.to_s.scan(/To unsubscribe/).count.should == 1
 
+      # side effect, topic user is updated with post number
+      tu = TopicUser.get(post.topic_id, response.user)
+      tu.last_emailed_post_number.should == response.post_number
     end
   end
 

spec/models/topic_link_click_spec.rb

@@ -1,4 +1,3 @@
-require 'discourse'
 require 'spec_helper'
 
 describe TopicLinkClick do

spec/models/topic_user_spec.rb

@@ -5,17 +5,12 @@ describe TopicUser do
   it { should belong_to :user }
   it { should belong_to :topic }
 
-  let!(:yesterday) { DateTime.now.yesterday }
+  let(:user) { Fabricate(:user) }
 
-  before do
-    DateTime.expects(:now).at_least_once.returns(yesterday)
-  end
-
-  let!(:user) { Fabricate(:coding_horror) }
-  let!(:topic) {
-    user = Fabricate(:user)
-    guardian = Guardian.new(user)
-    TopicCreator.create(user, guardian, title: "this is my topic title")
+  let(:topic) {
+    u = Fabricate(:user)
+    guardian = Guardian.new(u)
+    TopicCreator.create(u, guardian, title: "this is my topic title")
   }
   let(:topic_user) { TopicUser.get(topic,user) }
   let(:topic_creator_user) { TopicUser.get(topic, topic.user) }
@@ -23,6 +18,7 @@ describe TopicUser do
   let(:post) { Fabricate(:post, topic: topic, user: user) }
   let(:new_user) { Fabricate(:user, auto_track_topics_after_msecs: 1000) }
   let(:topic_new_user) { TopicUser.get(topic, new_user)}
+  let(:yesterday) { DateTime.now.yesterday }
 
 
   describe "unpinned" do
@@ -96,19 +92,22 @@ describe TopicUser do
     end
 
     it 'set upon initial visit' do
-      topic_user.first_visited_at.to_i.should == yesterday.to_i
-      topic_user.last_visited_at.to_i.should == yesterday.to_i
+      freeze_time yesterday do
+        topic_user.first_visited_at.to_i.should == yesterday.to_i
+        topic_user.last_visited_at.to_i.should == yesterday.to_i
+      end
     end
 
     it 'updates upon repeat visit' do
       today = yesterday.tomorrow
-      DateTime.expects(:now).returns(today)
 
-      TopicUser.track_visit!(topic,user)
-      # reload is a no go
-      topic_user = TopicUser.get(topic,user)
-      topic_user.first_visited_at.to_i.should == yesterday.to_i
-      topic_user.last_visited_at.to_i.should == today.to_i
+      freeze_time today do
+        TopicUser.track_visit!(topic,user)
+        # reload is a no go
+        topic_user = TopicUser.get(topic,user)
+        topic_user.first_visited_at.to_i.should == yesterday.to_i
+        topic_user.last_visited_at.to_i.should == today.to_i
+      end
     end
 
     it 'triggers the observer callbacks when updating' do
@@ -128,18 +127,22 @@ describe TopicUser do
       let(:topic_user) { TopicUser.get(topic,user) }
 
       it 'should create a new record for a visit' do
-        topic_user.last_read_post_number.should == 1
-        topic_user.last_visited_at.to_i.should == yesterday.to_i
-        topic_user.first_visited_at.to_i.should == yesterday.to_i
+        freeze_time yesterday do
+          topic_user.last_read_post_number.should == 1
+          topic_user.last_visited_at.to_i.should == yesterday.to_i
+          topic_user.first_visited_at.to_i.should == yesterday.to_i
+        end
       end
 
       it 'should update the record for repeat visit' do
-        Fabricate(:post, topic: topic, user: user)
-        TopicUser.update_last_read(user, topic.id, 2, 0)
-        topic_user = TopicUser.get(topic,user)
-        topic_user.last_read_post_number.should == 2
-        topic_user.last_visited_at.to_i.should == yesterday.to_i
-        topic_user.first_visited_at.to_i.should == yesterday.to_i
+        freeze_time yesterday do
+          Fabricate(:post, topic: topic, user: user)
+          TopicUser.update_last_read(user, topic.id, 2, 0)
+          topic_user = TopicUser.get(topic,user)
+          topic_user.last_read_post_number.should == 2
+          topic_user.last_visited_at.to_i.should == yesterday.to_i
+          topic_user.first_visited_at.to_i.should == yesterday.to_i
+        end
       end
     end
 
@@ -181,12 +184,17 @@ describe TopicUser do
   describe 'change a flag' do
 
     it 'creates a forum topic user record' do
+      user; topic
+
       lambda {
         TopicUser.change(user, topic.id, starred: true)
       }.should change(TopicUser, :count).by(1)
     end
 
     it "only inserts a row once, even on repeated calls" do
+
+      topic; user
+
       lambda {
         TopicUser.change(user, topic.id, starred: true)
         TopicUser.change(user, topic.id, starred: false)
@@ -248,4 +256,22 @@ describe TopicUser do
     tu.seen_post_count.should == 2
   end
 
+  describe "auto_watch_new_topic" do
+
+    it "auto watches topics when called" do
+      user1 = Fabricate(:user)
+      user2 = Fabricate(:user, watch_new_topics: true)
+      user3 = Fabricate(:user, watch_new_topics: true)
+      TopicUser.auto_watch_new_topic(topic.id)
+
+      TopicUser.get(topic, user1).should == nil
+
+      tu = TopicUser.get(topic, user2)
+      tu.notification_level.should == TopicUser.notification_levels[:watching]
+      tu.notifications_reason_id.should == TopicUser.notification_reasons[:auto_watch]
+
+      TopicUser.get(topic, user3).notification_level.should == TopicUser.notification_levels[:watching]
+    end
+  end
+
 end