FEATURE: More granular mailing list mode (#4068)

* Rearrange frontend to account for mailing list mode

* Allow update of user preference for mailing list frequency

* Add mailing list frequency estimate

* Simplify frequency estimate; disable activity summary for mailing list mode

* Remove combined updates

* Add specs for enqueue mailing list mode job

* Write mailing list method for mailer

* Fix linting error

* Account for stale topics

* Add translations for default mailing list setting

* One query for mailing list topics

* Fix failing spec

* WIP

* Flesh out html template

* First pass at text-based mailing list summary

* Add user avatar

* Properly format posts for mailing list

* Move make_all_links_absolute into Email::Styles

* Apply first_seen_at to user

* Send mailing list email summary hourly based on first_seen_at

* Branch and test cleanup

* Use existing mailing list mode estimate

* Fix failing specs

app/assets/javascripts/discourse/controllers/preferences.js.es6

@@ -62,6 +62,24 @@ export default Ember.Controller.extend(CanCheckEmails, {
     return this.siteSettings.available_locales.split('|').map(s => ({ name: s, value: s }));
   },
 
+  @computed()
+  frequencyEstimate() {
+    var estimate = this.get('model.mailing_list_posts_per_day');
+    if (!estimate || estimate < 2) {
+      return I18n.t('user.mailing_list_mode.few_per_day');
+    } else {
+      return I18n.t('user.mailing_list_mode.many_per_day', { dailyEmailEstimate: estimate });
+    }
+  },
+
+  @computed()
+  mailingListModeOptions() {
+    return [
+      {name: I18n.t('user.mailing_list_mode.daily'), value: 0},
+      {name: this.get('frequencyEstimate'), value: 1}
+    ];
+  },
+
   previousRepliesOptions: [
     {name: I18n.t('user.email_previous_replies.always'), value: 0},
     {name: I18n.t('user.email_previous_replies.unless_emailed'), value: 1},

app/assets/javascripts/discourse/templates/components/preference-checkbox.hbs

@@ -1,4 +1,4 @@
 <label class='checkbox-label'>
-  {{input type="checkbox" checked=checked}}
+  {{input type="checkbox" disabled=disabled checked=checked}}
   {{label}}
 </label>

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

@@ -185,9 +185,6 @@
       {{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}}
-      {{#unless siteSettings.disable_mailing_list_mode}}
-        {{preference-checkbox warning="checkMailingList" labelKey="user.mailing_list_mode" checked=model.user_option.mailing_list_mode}}
-      {{/unless}}
       {{preference-checkbox labelKey="user.email_always" checked=model.user_option.email_always}}
       {{#unless model.user_option.email_always}}
       <div class='instructions'>
@@ -198,11 +195,33 @@
         {{/if}}
       </div>
       {{/unless}}
-
-
-
     </div>
 
+    <div class='control-group pref-activity-summary'>
+      <label class="control-label">{{i18n 'user.email_activity_summary'}}</label>
+      {{#if canReceiveDigest}}
+        {{preference-checkbox labelKey="user.email_digests.title" disabled=model.user_option.mailing_list_mode checked=model.user_option.email_digests}}
+        {{#if model.user_option.email_digests}}
+          <div class='controls controls-dropdown'>
+            {{combo-box valueAttribute="value" content=digestFrequencies value=model.user_option.digest_after_minutes}}
+          </div>
+        {{/if}}
+      {{/if}}
+    </div>
+
+    {{#unless siteSettings.disable_mailing_list_mode}}
+      <div class='control-group pref-mailing-list-mode'>
+        <label class="control-label">{{i18n 'user.mailing_list_mode.label'}}</label>
+        {{preference-checkbox labelKey="user.mailing_list_mode.enabled" warning="checkMailingList" checked=model.user_option.mailing_list_mode}}
+        <div class='instructions'>{{{i18n 'user.mailing_list_mode.instructions'}}}</div>
+        {{#if model.user_option.mailing_list_mode}}
+          <div class='controls controls-dropdown'>
+            {{combo-box valueAttribute="value" content=mailingListModeOptions value=model.user_option.mailing_list_mode_frequency}}
+          </div>
+        {{/if}}
+      </div>
+    {{/unless}}
+
     <div class="control-group notifications">
       <label class="control-label">{{i18n 'user.desktop_notifications.label'}}</label>
       {{desktop-notification-config}}

app/helpers/user_notifications_helper.rb

@@ -65,9 +65,9 @@ module UserNotificationsHelper
       normalize_name(post.user.name) != normalize_name(post.user.username)
   end
 
-  def format_for_email(post, use_excerpt)
+  def format_for_email(post, use_excerpt, style: nil)
     html = use_excerpt ? post.excerpt : post.cooked
-    PrettyText.format_for_email(html, post).html_safe
+    PrettyText.format_for_email(html, post, style: style).html_safe
   end
 
 end

app/jobs/regular/notify_mailing_list_subscribers.rb

@@ -16,7 +16,7 @@ module Jobs
       users =
           User.activated.not_blocked.not_suspended.real
           .joins(:user_option)
-          .where(user_options: {mailing_list_mode: true})
+          .where(user_options: {mailing_list_mode: true, mailing_list_mode_frequency: 1})
           .where('NOT EXISTS(
                       SELECT 1
                       FROM topic_users tu

app/jobs/scheduled/enqueue_mailing_list_emails.rb

@@ -0,0 +1,27 @@
+module Jobs
+
+  class EnqueueMailingListEmails < Jobs::Scheduled
+    every 1.hour
+
+    def execute(args)
+      return if SiteSetting.disable_mailing_list_mode?
+      target_user_ids.each do |user_id|
+        Jobs.enqueue(:user_email, type: :mailing_list, user_id: user_id)
+      end
+    end
+
+    def target_user_ids
+      # Users who want to receive daily mailing list emails
+      User.real
+          .not_suspended
+          .joins(:user_option)
+          .where(active: true, staged: false, user_options: {mailing_list_mode: true, mailing_list_mode_frequency: 0})
+          .where("#{!SiteSetting.must_approve_users?} OR approved OR moderator OR admin")
+          .where("date_part('hour', first_seen_at) = date_part('hour', CURRENT_TIMESTAMP)")           # where the hour of first_seen_at is the same as the current hour
+          .where("COALESCE(first_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - '23 HOURS'::INTERVAL") # don't send unless you've been around for a day already
+          .pluck(:id)
+    end
+
+  end
+
+end

app/mailers/user_notifications.rb

@@ -73,16 +73,27 @@ class UserNotifications < ActionMailer::Base
     end
   end
 
-  def digest(user, opts={})
-    @user = user
-    @base_url = Discourse.base_url
+  def mailing_list(user, opts={})
+    @since = opts[:since] || 1.day.ago
+    @since_formatted = short_date(@since)
 
+    @new_topic_posts      = Post.mailing_list_new_topics(user, @since).group_by(&:topic) || {}
+    @existing_topic_posts = Post.mailing_list_updates(user, @since).group_by(&:topic) || {}
+    @posts_by_topic       = @new_topic_posts.merge @existing_topic_posts
+    return unless @posts_by_topic.present?
+
+    build_summary_for(user)
+    build_email @user.email,
+                from_alias: I18n.t('user_notifications.mailing_list.from', site_name: SiteSetting.title),
+                subject: I18n.t('user_notifications.mailing_list.subject_template',
+                                site_name: @site_name,
+                                date: @date)
+  end
+
+  def digest(user, opts={})
+    build_summary_for(user)
     min_date = opts[:since] || @user.last_emailed_at || @user.last_seen_at || 1.month.ago
 
-    @site_name = SiteSetting.email_prefix.presence || SiteSetting.title
-
-    @header_color = ColorScheme.hex_for_name('header_background')
-    @anchor_color = ColorScheme.hex_for_name('tertiary')
     @last_seen_at = short_date(@user.last_seen_at || @user.created_at)
 
     # A list of topics to show the user
@@ -106,10 +117,8 @@ class UserNotifications < ActionMailer::Base
       end
 
       @featured_topics, @new_topics = @featured_topics[0..4], @featured_topics[5..-1]
-      @markdown_linker = MarkdownLinker.new(Discourse.base_url)
-      @unsubscribe_key = DigestUnsubscribeKey.create_key_for(@user)
 
-      build_email user.email,
+      build_email @user.email,
                   from_alias: I18n.t('user_notifications.digest.from', site_name: SiteSetting.title),
                   subject: I18n.t('user_notifications.digest.subject_template',
                                   site_name: @site_name,
@@ -396,4 +405,17 @@ class UserNotifications < ActionMailer::Base
 
     build_email(user.email, email_opts)
   end
+
+  private
+
+  def build_summary_for(user)
+    @user            = user
+    @date            = short_date(Time.now)
+    @base_url        = Discourse.base_url
+    @site_name       = SiteSetting.email_prefix.presence || SiteSetting.title
+    @header_color    = ColorScheme.hex_for_name('header_background')
+    @anchor_color    = ColorScheme.hex_for_name('tertiary')
+    @markdown_linker = MarkdownLinker.new(@base_url)
+    @unsubscribe_key = DigestUnsubscribeKey.create_key_for(@user)
+  end
 end

app/models/mailing_list_mode_site_setting.rb

@@ -0,0 +1,19 @@
+require_dependency 'enum_site_setting'
+
+class MailingListModeSiteSetting < EnumSiteSetting
+  def self.valid_value?(val)
+    val.to_i.to_s == val.to_s &&
+    values.any? { |v| v[:value] == val.to_i }
+  end
+
+  def self.values
+    @values ||= [
+      { name: 'user.mailing_list_mode.daily',      value:  0 },
+      { name: 'user.mailing_list_mode.individual', value:  1 }
+    ]
+  end
+
+  def self.translate_names?
+    true
+  end
+end

app/models/post.rb

@@ -65,6 +65,14 @@ class Post < ActiveRecord::Base
   scope :with_topic_subtype, ->(subtype) { joins(:topic).where('topics.subtype = ?', subtype) }
   scope :visible, -> { joins(:topic).where('topics.visible = true').where(hidden: false) }
   scope :secured, lambda { |guardian| where('posts.post_type in (?)', Topic.visible_post_types(guardian && guardian.user))}
+  scope :for_mailing_list, ->(user, since) {
+     created_since(since)
+    .joins(:topic)
+    .where(topic: Topic.for_digest(user, 100.years.ago)) # we want all topics with new content, regardless when they were created
+    .order('posts.created_at ASC')
+  }
+  scope :mailing_list_new_topics, ->(user, since) { for_mailing_list(user, since).where('topics.created_at > ?', since) }
+  scope :mailing_list_updates,    ->(user, since) { for_mailing_list(user, since).where('topics.created_at <= ?', since) }
 
   delegate :username, to: :user
 

app/models/user.rb

@@ -476,6 +476,7 @@ class User < ActiveRecord::Base
     update_previous_visit(now)
     # using update_column to avoid the AR transaction
     update_column(:last_seen_at, now)
+    update_column(:first_seen_at, now) unless self.first_seen_at
   end
 
   def self.gravatar_template(email)

app/models/user_option.rb

@@ -24,6 +24,7 @@ class UserOption < ActiveRecord::Base
   def set_defaults
     self.email_always = SiteSetting.default_email_always
     self.mailing_list_mode = SiteSetting.default_email_mailing_list_mode
+    self.mailing_list_mode_frequency = SiteSetting.default_email_mailing_list_mode_frequency
     self.email_direct = SiteSetting.default_email_direct
     self.automatically_unpin_topics = SiteSetting.default_topics_automatic_unpin
     self.email_private_messages = SiteSetting.default_email_private_messages

app/serializers/user_option_serializer.rb

@@ -2,6 +2,7 @@ class UserOptionSerializer < ApplicationSerializer
   attributes :user_id,
              :email_always,
              :mailing_list_mode,
+             :mailing_list_mode_frequency,
              :email_digests,
              :email_private_messages,
              :email_direct,

app/services/user_updater.rb

@@ -9,6 +9,7 @@ class UserUpdater
   OPTION_ATTR = [
     :email_always,
     :mailing_list_mode,
+    :mailing_list_mode_frequency,
     :email_digests,
     :email_direct,
     :email_private_messages,

app/views/user_notifications/mailing_list.html.erb

@@ -0,0 +1,76 @@
+<div id="top"></div>
+<table style='width: 100%; border: 20px solid #eee;' cellspacing='0' cellpadding='0'>
+  <thead>
+    <td style='padding: 10px 10px; background-color: #<%= @header_color %>;'>
+      <a href='<%= Discourse.base_url %>' style='color: #<%= @anchor_color %>'>
+        <% if logo_url.blank? %>
+          <%= SiteSetting.title %>
+        <% else %>
+          <img src='<%= logo_url %>' style='max-height: 35px; min-height: 35px; height: 35px;' class='site-logo'></a>
+        <% end %>
+        <br />
+        <div style='padding-top: 10px;'>
+          <%= raw(t 'user_notifications.mailing_list.why', site_link: html_site_link(@anchor_color), date: @since_formatted) %>
+        </div>
+        <hr />
+      </a>
+    </td>
+  </thead>
+  <tr>
+    <td>
+      <h4 style="padding: 0 25px; margin: 10px 0;"><%= t('user_notifications.mailing_list.new_topics') %></h3>
+      <ul>
+        <% @new_topic_posts.keys.each do |topic| %>
+          <li>
+            <a href='<%= Discourse.base_url + topic.relative_url %>' style='color: #<%= @anchor_color %>'><%= raw format_topic_title(topic.title) %></a>
+            <span><%= @new_topic_posts[topic].length %></span>
+            <%= category_badge(topic.category, inline_style: true, absolute_url: true) %>
+          </li>
+        <% end %>
+      </ul>
+      <h4 style="padding: 0 25px; margin: 10px 0;"><%= t('user_notifications.mailing_list.topic_updates') %></h3>
+      <ul>
+        <% @existing_topic_posts.keys.each do |topic| %>
+          <li>
+            <a href='<%= Discourse.base_url + topic.relative_url %>' style='color: #<%= @anchor_color %>'><%= raw format_topic_title(topic.title) %></a>
+            <span><%= @existing_topic_posts[topic].length %></span>
+            <%= category_badge(topic.category, inline_style: true, absolute_url: true) %>
+          </li>
+        <% end %>
+      </ul>
+    </td>
+  </tr>
+</table>
+<table style='width: 100%; background: #eee;' cellspacing='0' cellpadding='0'>
+  <% @posts_by_topic.keys.each do |topic| %>
+    <tr>
+      <td>
+        <h3 style='padding: 20px 20px 10px; margin: 0;'>
+          <a href='<%= Discourse.base_url + topic.relative_url %>' style='color: #<%= @anchor_color %>'><%= raw format_topic_title(topic.title) %></a>
+        </h3>
+      </td>
+    </tr>
+    <tr>
+      <td style='border-left: 20px solid #eee; border-right: 20px solid #eee; border-bottom: 10px solid #eee; padding: 10px 10px; background: #fff;'>
+        <% @posts_by_topic[topic].each do |post| %>
+          <div>
+            <img style="float: left; width: 20px; padding-right: 5px;" src="<%= post.user.small_avatar_url %>" title="<%= post.user.username%>">
+            <p style="font-size: 15px; color: #888">
+              <a href='<%= [Discourse.base_url, :users, post.user.username].join('/') %>'><%= post.user.name || post.user.username %></a>
+              <span> - </span>
+              <span><%= I18n.l(post.created_at, format: :long) %></span>
+            </p>
+            <%= raw format_for_email(post, false, style: :notification) %>
+            <hr />
+          </div>
+        <% end %>
+        <a style='font-size: 12px; float: right;' href='<%= Discourse.base_url + topic.relative_url %>'><%= t('user_notifications.mailing_list.view_this_topic') %></a>
+      </td>
+    </tr>
+    <tr>
+      <td style='padding: 0 20px; font-size: 12px;'>
+        <a href="#top"><%= t('user_notifications.mailing_list.back_to_top') %></a>
+      </td>
+    </tr>
+  <% end %>
+</table>

app/views/user_notifications/mailing_list.text.erb

@@ -0,0 +1,31 @@
+<%- site_link = raw(@markdown_linker.create(@site_name, '/')) %>
+<%= raw(t 'user_notifications.mailing_list.why', site_link: site_link, date: @since_formatted) %>
+
+<%- if @new_topic_posts.keys.present? -%>
+  ### <%= t 'user_notifications.mailing_list.new_topics' %>
+  <%- @new_topic_posts.keys.each do |topic| %>
+    <%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %>
+  <%- end -%>
+<%- end -%>
+
+<%- if @existing_topic_posts.keys.present? -%>
+  ### <%= t 'user_notifications.mailing_list.new_topics' %>
+  <%- @existing_topic_posts.keys.each do |topic| -%>
+    <%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %>
+    <%= @existing_topic_posts[topic].length %>
+  <%- end -%>
+<%- end -%>
+
+--------------------------------------------------------------------------------
+
+<%- @posts_by_topic.keys.each do |topic| %>
+  ### <%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %>
+
+  <%- @posts_by_topic[topic].each do |post| -%>
+    <%= post.user.name || post.user.username %> - <%= post.created_at %>
+--------------------------------------------------------------------------------
+    <%= post.raw %>
+
+    
+  <%- end -%>
+<%- end %>

config/locales/client.en.yml

@@ -504,7 +504,17 @@ en:
       suspended_notice: "This user is suspended until {{date}}."
       suspended_reason: "Reason: "
       github_profile: "Github"
-      mailing_list_mode: "Send me an email for every new post (unless I mute the topic or category)"
+      email_activity_summary: "Activity Summary"
+      mailing_list_mode:
+        label: "Mailing list mode"
+        enabled: "Enable mailing list mode."
+        instructions: |
+          This setting overrides the activity summary.<br />
+          Muted topics and categories are not included in these emails.
+        daily: "Send daily updates."
+        individual: "Send an email for every new post."
+        many_per_day: "Send me an email for every new post (about {{dailyEmailEstimate}} per day)."
+        few_per_day: "Send me an email for every new post (less than 2 per day)."
       watched_categories: "Watched"
       watched_categories_instructions: "You will automatically watch all new topics in these categories. You will be notified of all new posts and topics, and a count of new posts will also appear next to the topic."
       tracked_categories: "Tracked"

config/locales/server.en.yml

@@ -28,6 +28,7 @@ en:
       short_no_year: "%B %-d"
       # Format directives: http://ruby-doc.org/core-2.2.0/Time.html#method-i-strftime
       date_only: "%B %-d, %Y"
+      long: "%B %-d, %Y, %l:%M%P"
   date:
     # Do not remove the brackets and commas and do not translate the first month name. It should be "null".
     month_names:
@@ -35,6 +36,8 @@ en:
     <<: *datetime_formats
   time:
     <<: *datetime_formats
+    am: "am"
+    pm: "pm"
 
   title: "Discourse"
   topics: "Topics"
@@ -1300,6 +1303,7 @@ en:
     default_email_private_messages: "Send an email when someone messages the user by default."
     default_email_direct: "Send an email when someone quotes/replies to/mentions or invites the user by default."
     default_email_mailing_list_mode: "Send an email for every new post by default."
+    default_email_mailing_list_mode_frequency: "Users who enable mailing list mode will receive emails this often by default."
     disable_mailing_list_mode: "Disallow users from enabling mailing list mode."
     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."
@@ -2278,6 +2282,15 @@ en:
       more_topics: "There were %{new_topics_since_seen} other new topics."
       more_topics_category: "More new topics:"
 
+    mailing_list:
+      why: "All activity on %{site_link} for %{date}"
+      subject_template: "[%{site_name}] Summary for %{date}"
+      unsubscribe: "This summary is sent daily due to mailing list mode being enabled. To unsubscribe %{unsubscribe_link}."
+      from: "%{site_name} summary"
+      new_topics: "New topics"
+      topic_updates: "Topic updates"
+      view_this_topic: "View this topic"
+      back_to_top: "Back to top"
     forgot_password:
       subject_template: "[%{site_name}] Password reset"
       text_body_template: |

config/site_settings.yml

@@ -1128,6 +1128,9 @@ user_preferences:
   default_email_private_messages: true
   default_email_direct: true
   default_email_mailing_list_mode: false
+  default_email_mailing_list_mode_frequency:
+    enum: 'MailingListModeSiteSetting'
+    default: 1
   disable_mailing_list_mode:
     default: false
     client: true

db/migrate/20160309073132_add_mailing_list_mode_frequency.rb

@@ -0,0 +1,5 @@
+class AddMailingListModeFrequency < ActiveRecord::Migration
+  def change
+    add_column :user_options, :mailing_list_mode_frequency, :integer, default: 0, null: false
+  end
+end

db/migrate/20160326001747_add_user_first_visit.rb

@@ -0,0 +1,5 @@
+class AddUserFirstVisit < ActiveRecord::Migration
+  def change
+    add_column :users, :first_seen_at, :datetime
+  end
+end

lib/email/styles.rb

@@ -90,6 +90,7 @@ module Email
       style('.rtl', 'direction: rtl;')
       style('td.body', 'padding-top:5px;', colspan: "2")
       style('.whisper td.body', 'font-style: italic; color: #9c9c9c;')
+      style('.lightbox-wrapper .meta', 'display: none')
       correct_first_body_margin
       correct_footer_style
       reset_tables
@@ -186,6 +187,17 @@ module Email
       @fragment.to_s
     end
 
+    def make_all_links_absolute
+      site_uri = URI(Discourse.base_url)
+      @fragment.css("a").each do |link|
+        begin
+          link["href"] = "#{site_uri}#{link['href']}" unless URI(link["href"].to_s).host.present?
+        rescue URI::InvalidURIError, URI::InvalidComponentError
+          # leave it
+        end
+      end
+    end
+
     private
 
     def replace_relative_urls

lib/pretty_text.rb

@@ -386,31 +386,16 @@ module PrettyText
     fragment.to_html
   end
 
-  # Given a Nokogiri doc, convert all links to absolute
-  def self.make_all_links_absolute(doc)
-    site_uri = nil
-    doc.css("a").each do |link|
-      href = link["href"].to_s
-      begin
-        uri = URI(href)
-        site_uri ||= URI(Discourse.base_url)
-        link["href"] = "#{site_uri}#{link['href']}" unless uri.host.present?
-      rescue URI::InvalidURIError, URI::InvalidComponentError
-        # leave it
-      end
-    end
-  end
-
   def self.strip_image_wrapping(doc)
     doc.css(".lightbox-wrapper .meta").remove
   end
 
-  def self.format_for_email(html, post = nil)
-    doc = Nokogiri::HTML.fragment(html)
-    DiscourseEvent.trigger(:reduce_cooked, doc, post)
-    make_all_links_absolute(doc)
-    strip_image_wrapping(doc)
-    doc.to_html
+  def self.format_for_email(html, post = nil, style: nil)
+    Email::Styles.new(html, style: style).tap do |doc|
+      doc.make_all_links_absolute
+      doc.send :"format_#{style}" if style
+      DiscourseEvent.trigger(:reduce_cooked, doc, post)
+    end.to_html
   end
 
   protected

spec/components/pretty_text_spec.rb

@@ -286,41 +286,6 @@ HTML
     end
   end
 
-  describe "make_all_links_absolute" do
-    let(:base_url) { "http://baseurl.net" }
-
-    def make_abs_string(html)
-      doc = Nokogiri::HTML.fragment(html)
-      described_class.make_all_links_absolute(doc)
-      doc.to_html
-    end
-
-    before do
-      Discourse.stubs(:base_url).returns(base_url)
-    end
-
-    it "adds base url to relative links" do
-      html = "<p><a class=\"mention\" href=\"/users/wiseguy\">@wiseguy</a>, <a class=\"mention\" href=\"/users/trollol\">@trollol</a> what do you guys think? </p>"
-      output = make_abs_string(html)
-      expect(output).to eq("<p><a class=\"mention\" href=\"#{base_url}/users/wiseguy\">@wiseguy</a>, <a class=\"mention\" href=\"#{base_url}/users/trollol\">@trollol</a> what do you guys think? </p>")
-    end
-
-    it "doesn't change external absolute links" do
-      html = "<p>Check out <a href=\"http://mywebsite.com/users/boss\">this guy</a>.</p>"
-      expect(make_abs_string(html)).to eq(html)
-    end
-
-    it "doesn't change internal absolute links" do
-      html = "<p>Check out <a href=\"#{base_url}/users/boss\">this guy</a>.</p>"
-      expect(make_abs_string(html)).to eq(html)
-    end
-
-    it "can tolerate invalid URLs" do
-      html = "<p>Check out <a href=\"not a real url\">this guy</a>.</p>"
-      expect { make_abs_string(html) }.to_not raise_error
-    end
-  end
-
   describe "strip_image_wrapping" do
     def strip_image_wrapping(html)
       doc = Nokogiri::HTML.fragment(html)
@@ -339,8 +304,36 @@ HTML
   end
 
   describe 'format_for_email' do
+    let(:base_url) { "http://baseurl.net" }
+    let(:post) { Fabricate(:post) }
+
+    before do
+      Discourse.stubs(:base_url).returns(base_url)
+    end
+
     it 'does not crash' do
-      PrettyText.format_for_email('<a href="mailto:michael.brown@discourse.org?subject=Your%20post%20at%20http://try.discourse.org/t/discussion-happens-so-much/127/1000?u=supermathie">test</a>')
+      PrettyText.format_for_email('<a href="mailto:michael.brown@discourse.org?subject=Your%20post%20at%20http://try.discourse.org/t/discussion-happens-so-much/127/1000?u=supermathie">test</a>', post)
+    end
+
+    it "adds base url to relative links" do
+      html = "<p><a class=\"mention\" href=\"/users/wiseguy\">@wiseguy</a>, <a class=\"mention\" href=\"/users/trollol\">@trollol</a> what do you guys think? </p>"
+      output = described_class.format_for_email(html, post)
+      expect(output).to eq("<p><a href=\"#{base_url}/users/wiseguy\">@wiseguy</a>, <a href=\"#{base_url}/users/trollol\">@trollol</a> what do you guys think? </p>")
+    end
+
+    it "doesn't change external absolute links" do
+      html = "<p>Check out <a href=\"http://mywebsite.com/users/boss\">this guy</a>.</p>"
+      expect(described_class.format_for_email(html, post)).to eq(html)
+    end
+
+    it "doesn't change internal absolute links" do
+      html = "<p>Check out <a href=\"#{base_url}/users/boss\">this guy</a>.</p>"
+      expect(described_class.format_for_email(html, post)).to eq(html)
+    end
+
+    it "can tolerate invalid URLs" do
+      html = "<p>Check out <a href=\"not a real url\">this guy</a>.</p>"
+      expect { described_class.format_for_email(html, post) }.to_not raise_error
     end
   end
 

spec/jobs/enqueue_mailing_list_emails_spec.rb

@@ -0,0 +1,128 @@
+require 'rails_helper'
+require_dependency 'jobs/base'
+
+describe Jobs::EnqueueMailingListEmails do
+
+  describe '#target_users' do
+
+    context 'unapproved users' do
+      Given!(:unapproved_user) { Fabricate(:active_user, approved: false, first_seen_at: 24.hours.ago) }
+      When do
+        SiteSetting.must_approve_users = true
+        unapproved_user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0)
+      end
+      Then { expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(unapproved_user.id)).to eq(false) }
+
+      # As a moderator
+      And { unapproved_user.update_column(:moderator, true) }
+      And { expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(unapproved_user.id)).to eq(true) }
+
+      # As an admin
+      And { unapproved_user.update_attributes(admin: true, moderator: false) }
+      And { expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(unapproved_user.id)).to eq(true) }
+
+      # As an approved user
+      And { unapproved_user.update_attributes(admin: false, moderator: false, approved: true ) }
+      And { expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(unapproved_user.id)).to eq(true) }
+    end
+
+    context 'staged users' do
+      let!(:staged_user) { Fabricate(:active_user, staged: true, last_emailed_at: 1.year.ago, last_seen_at: 1.year.ago) }
+
+      it "doesn't return staged users" do
+        expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(staged_user.id)).to eq(false)
+      end
+    end
+
+    context "inactive user" do
+      let!(:inactive_user) { Fabricate(:user, active: false) }
+
+      it "doesn't return users who have been emailed recently" do
+        expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(inactive_user.id)).to eq(false)
+      end
+    end
+
+    context "suspended user" do
+      let!(:suspended_user) { Fabricate(:user, suspended_till: 1.week.from_now, suspended_at: 1.day.ago) }
+
+      it "doesn't return users who are suspended" do
+        expect(Jobs::EnqueueMailingListEmails.new.target_user_ids.include?(suspended_user.id)).to eq(false)
+      end
+    end
+
+    context 'users with mailing list mode on' do
+      let(:user) { Fabricate(:active_user, first_seen_at: 24.hours.ago) }
+      let(:user_option) { user.user_option }
+      subject { Jobs::EnqueueMailingListEmails.new.target_user_ids }
+      before do
+        user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0)
+      end
+
+      it "returns a user whose first_seen_at matches the current hour" do
+        expect(subject).to include user.id
+      end
+
+      it "returns a user seen multiple days ago" do
+        user.update(first_seen_at: 72.hours.ago)
+        expect(subject).to include user.id
+      end
+
+      it "doesn't return a user who has never been seen" do
+        user.update(first_seen_at: nil)
+        expect(subject).to_not include user.id
+      end
+
+      it "doesn't return users with mailing list mode off" do
+        user_option.update(mailing_list_mode: false)
+        expect(subject).to_not include user.id
+      end
+
+      it "doesn't return users with mailing list mode set to 'individual'" do
+        user_option.update(mailing_list_mode_frequency: 1)
+        expect(subject).to_not include user.id
+      end
+
+      it "doesn't return a user who has received the mailing list summary earlier" do
+        user.update(first_seen_at: 5.hours.ago)
+        expect(subject).to_not include user.id
+      end
+
+      it "doesn't return a user who was first seen today" do
+        user.update(first_seen_at: 2.minutes.ago)
+        expect(subject).to_not include user.id
+      end
+    end
+
+  end
+
+  describe '#execute' do
+
+    let(:user) { Fabricate(:user) }
+
+    context "mailing list emails are enabled" do
+      before do
+        Jobs::EnqueueMailingListEmails.any_instance.expects(:target_user_ids).returns([user.id])
+      end
+
+      it "enqueues the mailing list email job" do
+        Jobs.expects(:enqueue).with(:user_email, type: :mailing_list, user_id: user.id)
+        Jobs::EnqueueMailingListEmails.new.execute({})
+      end
+    end
+
+    context "mailing list emails are disabled" do
+      before do
+        Jobs::EnqueueMailingListEmails.any_instance.expects(:target_user_ids).never
+      end
+
+      it "does not enqueue the mailing list email job" do
+        SiteSetting.disable_mailing_list_mode = true
+        Jobs.expects(:enqueue).with(:user_email, type: :mailing_list, user_id: user.id).never
+        Jobs::EnqueueMailingListEmails.new.execute({})
+      end
+    end
+
+  end
+
+
+end

spec/jobs/notify_mailing_list_subscribers_spec.rb

@@ -34,10 +34,17 @@ describe Jobs::NotifyMailingListSubscribers do
     context "with a valid post" do
       let!(:post) { Fabricate(:post, user: user) }
 
-      it "sends the email to the user" do
+      it "sends the email to the user if the frequency is set to 'always'" do
+        user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 1)
         UserNotifications.expects(:mailing_list_notify).with(user, post).once
         Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
       end
+
+      it "does not send the email to the user if the frequency is set to 'daily'" do
+        user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0)
+        UserNotifications.expects(:mailing_list_notify).never
+        Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
+      end
     end
 
     context "with a deleted post" do

spec/mailers/user_notifications_spec.rb

@@ -76,6 +76,69 @@ describe UserNotifications do
 
   end
 
+  describe '.mailing_list' do
+    subject { UserNotifications.mailing_list(user) }
+
+    context "without new posts" do
+      it "doesn't send the email" do
+        expect(subject.to).to be_blank
+      end
+    end
+
+    context "with new posts" do
+      let(:user) { Fabricate(:user) }
+      let(:topic) { Fabricate(:topic, user: user) }
+      let!(:new_post) { Fabricate(:post, topic: topic, created_at: 2.hours.ago, raw: "Feel the Bern") }
+      let!(:old_post) { Fabricate(:post, topic: topic, created_at: 25.hours.ago, raw: "Make America Great Again") }
+      let(:old_topic) { Fabricate(:topic, user: user, created_at: 10.days.ago) }
+      let(:new_post_in_old_topic) { Fabricate(:post, topic: old_topic, created_at: 2.hours.ago, raw: "Yes We Can") }
+      let(:stale_post) { Fabricate(:post, topic: old_topic, created_at: 2.days.ago, raw: "A New American Century") }
+
+      it "works" do
+        expect(subject.to).to eq([user.email])
+        expect(subject.subject).to be_present
+        expect(subject.from).to eq([SiteSetting.notification_email])
+        expect(subject.html_part.body.to_s).to include topic.title
+        expect(subject.text_part.body.to_s).to be_present
+      end
+
+      it "includes posts less than 24 hours old" do
+        expect(subject.html_part.body.to_s).to include new_post.cooked
+      end
+
+      it "does not include posts older than 24 hours old" do
+        expect(subject.html_part.body.to_s).to_not include old_post.cooked
+      end
+
+      it "includes topics created over 24 hours ago which have new posts" do
+        new_post_in_old_topic
+        expect(subject.html_part.body.to_s).to include old_topic.title
+        expect(subject.html_part.body.to_s).to include new_post_in_old_topic.cooked
+        expect(subject.html_part.body.to_s).to_not include stale_post.cooked
+      end
+
+      it "includes multiple topics" do
+        new_post_in_old_topic
+        expect(subject.html_part.body.to_s).to include topic.title
+        expect(subject.html_part.body.to_s).to include old_topic.title
+      end
+
+      it "does not include topics not updated for the past 24 hours" do
+        stale_post
+        expect(subject.html_part.body.to_s).to_not include old_topic.title
+        expect(subject.html_part.body.to_s).to_not include stale_post.cooked
+      end
+
+      it "includes email_prefix in email subject instead of site title" do
+        SiteSetting.email_prefix = "Try Discourse"
+        SiteSetting.title = "Discourse Meta"
+
+        expect(subject.subject).to match(/Try Discourse/)
+        expect(subject.subject).not_to match(/Discourse Meta/)
+      end
+    end
+  end
+
   describe '.digest' do
 
     subject { UserNotifications.digest(user) }

spec/models/mailing_list_mode_site_setting_spec.rb

@@ -0,0 +1,17 @@
+require 'rails_helper'
+
+describe MailingListModeSiteSetting do
+  describe 'valid_value?' do
+    it 'returns true for a valid value as an int' do
+      expect(DigestEmailSiteSetting.valid_value?(0)).to eq true
+    end
+
+    it 'returns false for a valid value as a string' do
+      expect(DigestEmailSiteSetting.valid_value?('0')).to eq true
+    end
+
+    it 'returns false for an out of range value' do
+      expect(DigestEmailSiteSetting.valid_value?(3)).to eq false
+    end
+  end
+end

spec/models/user_spec.rb

@@ -596,6 +596,29 @@ describe User do
 
   end
 
+  describe "update_last_seen!" do
+    let (:user) { Fabricate(:user) }
+    let!(:first_visit_date) { Time.zone.now }
+    let!(:second_visit_date) { 2.hours.from_now }
+
+    it "should update the last seen value" do
+      expect(user.last_seen_at).to eq nil
+      user.update_last_seen!(first_visit_date)
+      expect(user.reload.last_seen_at).to be_within_one_second_of(first_visit_date)
+    end
+
+    it "should update the first seen value if it doesn't exist" do
+      user.update_last_seen!(first_visit_date)
+      expect(user.reload.first_seen_at).to be_within_one_second_of(first_visit_date)
+    end
+
+    it "should not update the first seen value if it doesn't exist" do
+      user.update_last_seen!(first_visit_date)
+      user.update_last_seen!(second_visit_date)
+      expect(user.reload.first_seen_at).to be_within_one_second_of(first_visit_date)
+    end
+  end
+
   describe "last_seen_at" do
     let(:user) { Fabricate(:user) }