Break down new topic counts by category if a digest contains many.

This commit is contained in:
Robin Ward 2014-04-17 16:42:40 -04:00
parent 0e56157212
commit 64faee0935
7 changed files with 57 additions and 16 deletions

View file

@ -70,8 +70,15 @@ module UserNotificationsHelper
end end
def email_category(category) def email_category(category, opts=nil)
return "" if category.blank? || category.uncategorized? opts = opts || {}
# If there is no category, bail
return "" if category.blank?
# By default hide uncategorized
return "" if category.uncategorized? && !opts[:show_uncategorized]
result = "" result = ""
if category.parent_category.present? if category.parent_category.present?
result << "<span style='background-color: ##{category.parent_category.color}; font-size: 12px; padding: 4px 2px; font-weight: bold; margin: 0; width: 2px;'>&nbsp;</span>" result << "<span style='background-color: ##{category.parent_category.color}; font-size: 12px; padding: 4px 2px; font-weight: bold; margin: 0; width: 2px;'>&nbsp;</span>"

View file

@ -30,6 +30,7 @@ class UserNotifications < ActionMailer::Base
email_token: opts[:email_token]) email_token: opts[:email_token])
end end
def digest(user, opts={}) def digest(user, opts={})
@user = user @user = user
@base_url = Discourse.base_url @base_url = Discourse.base_url
@ -41,19 +42,28 @@ class UserNotifications < ActionMailer::Base
@last_seen_at = I18n.l(@user.last_seen_at || @user.created_at, format: :short) @last_seen_at = I18n.l(@user.last_seen_at || @user.created_at, format: :short)
# A list of topics to show the user # A list of topics to show the user
@featured_topics = Topic.for_digest(user, min_date).to_a @featured_topics = Topic.for_digest(user, min_date, limit: 20, top_order: true).to_a
# Don't send email unless there is content in it # Don't send email unless there is content in it
if @featured_topics.present? if @featured_topics.present?
@new_topics_since_seen = Topic.listable_topics featured_topic_ids = @featured_topics.map(&:id)
.where("created_at > ?", min_date).count - @featured_topics.length
@new_topics_since_seen = Topic.new_since_last_seen(user, min_date, featured_topic_ids).count
if @new_topics_since_seen > 1000
category_counts = Topic.new_since_last_seen(user, min_date, featured_topic_ids).group(:category_id).count
@new_by_category = []
if category_counts.present?
Category.where(id: category_counts.keys).each do |c|
@new_by_category << [c, category_counts[c.id]]
end
@new_by_category.sort_by! {|c| -c[1]}
end
end
@new_topics_since_seen = 0 if @new_topics_since_seen < 0
@featured_topics, @new_topics = @featured_topics[0..4], @featured_topics[5..-1] @featured_topics, @new_topics = @featured_topics[0..4], @featured_topics[5..-1]
@markdown_linker = MarkdownLinker.new(Discourse.base_url) @markdown_linker = MarkdownLinker.new(Discourse.base_url)
build_email user.email, build_email user.email,
from_alias: I18n.t('user_notifications.digest.from', site_name: SiteSetting.title), from_alias: I18n.t('user_notifications.digest.from', site_name: SiteSetting.title),
subject: I18n.t('user_notifications.digest.subject_template', subject: I18n.t('user_notifications.digest.subject_template',

View file

@ -260,21 +260,28 @@ class Topic < ActiveRecord::Base
end end
# Returns hot topics since a date for display in email digest. # Returns hot topics since a date for display in email digest.
def self.for_digest(user, since) def self.for_digest(user, since, opts=nil)
opts = opts || {}
score = "#{ListController.best_period_for(since)}_score" score = "#{ListController.best_period_for(since)}_score"
topics = Topic topics = Topic
.visible .visible
.secured(Guardian.new(user)) .secured(Guardian.new(user))
.joins("LEFT OUTER JOIN top_topics ON top_topics.topic_id = topics.id")
.joins("LEFT OUTER JOIN topic_users ON topic_users.topic_id = topics.id AND topic_users.user_id = #{user.id.to_i}") .joins("LEFT OUTER JOIN topic_users ON topic_users.topic_id = topics.id AND topic_users.user_id = #{user.id.to_i}")
.where(closed: false, archived: false) .where(closed: false, archived: false)
.where("COALESCE(topic_users.notification_level, 1) <> ?", TopicUser.notification_levels[:muted]) .where("COALESCE(topic_users.notification_level, 1) <> ?", TopicUser.notification_levels[:muted])
.created_since(since) .created_since(since)
.listable_topics .listable_topics
.includes(:category) .includes(:category)
if !!opts[:top_order]
topics = topics.joins("LEFT OUTER JOIN top_topics ON top_topics.topic_id = topics.id")
.order(TopicQuerySQL.order_top_for(score)) .order(TopicQuerySQL.order_top_for(score))
.limit(20) end
if opts[:limit]
topics = topics.limit(opts[:limit])
end
# Remove category topics # Remove category topics
category_topic_ids = Category.pluck(:topic_id).compact! category_topic_ids = Category.pluck(:topic_id).compact!
@ -291,6 +298,12 @@ class Topic < ActiveRecord::Base
topics topics
end end
# Using the digest query, figure out what's new for a user since last seen
def self.new_since_last_seen(user, since, featured_topic_ids)
topics = Topic.for_digest(user, since)
topics.where("topics.id NOT IN (?)", featured_topic_ids)
end
def update_meta_data(data) def update_meta_data(data)
self.meta_data = (self.meta_data || {}).merge(data.stringify_keys) self.meta_data = (self.meta_data || {}).merge(data.stringify_keys)
save save

View file

@ -45,7 +45,16 @@
<%- end -%> <%- end -%>
<%- if @new_topics_since_seen > 0 %> <%- if @new_topics_since_seen > 0 %>
<%- if @new_by_category.present? %>
<p><%= t('user_notifications.digest.more_topics_category', last_seen_at: @last_seen_at, new_topics_since_seen: @new_topics_since_seen) %></p>
<div>
<%- @new_by_category.each do |c| %>
<span style="margin-bottom: 8px; margin-right: 10px; display: inline-block;"><%= email_category(c[0], show_uncategorized: true) %> x <%= c[1] %></span>
<%- end %>
</div>
<%- else %>
<p><%= t('user_notifications.digest.more_topics', last_seen_at: @last_seen_at, new_topics_since_seen: @new_topics_since_seen) %></p> <p><%= t('user_notifications.digest.more_topics', last_seen_at: @last_seen_at, new_topics_since_seen: @new_topics_since_seen) %></p>
<%- end %>
<%- end -%> <%- end -%>
<span class='footer-notice'> <span class='footer-notice'>

View file

@ -1361,6 +1361,7 @@ en:
from: "%{site_name} digest" from: "%{site_name} digest"
read_more: "Read More" read_more: "Read More"
more_topics: "Also, since we last saw you on %{last_seen_at} there have been %{new_topics_since_seen} new topics posted." more_topics: "Also, since we last saw you on %{last_seen_at} there have been %{new_topics_since_seen} new topics posted."
more_topics_category: "Also, since we last saw you on %{last_seen_at} there have been %{new_topics_since_seen} new topics posted in the following categories:"
posts: posts:
one: "1 post" one: "1 post"

View file

@ -52,6 +52,7 @@ describe UserNotifications do
context "with new topics" do context "with new topics" do
before do before do
Topic.expects(:for_digest).returns([Fabricate(:topic, user: Fabricate(:coding_horror))]) Topic.expects(:for_digest).returns([Fabricate(:topic, user: Fabricate(:coding_horror))])
Topic.expects(:new_since_last_seen).returns(Topic.none)
end end
its(:to) { should == [user.email] } its(:to) { should == [user.email] }

View file

@ -1194,17 +1194,17 @@ describe Topic do
let(:user) { Fabricate.build(:user) } let(:user) { Fabricate.build(:user) }
it "returns none when there are no topics" do it "returns none when there are no topics" do
Topic.for_digest(user, 1.year.ago).should be_blank Topic.for_digest(user, 1.year.ago, top_order: true).should be_blank
end end
it "doesn't return category topics" do it "doesn't return category topics" do
Fabricate(:category) Fabricate(:category)
Topic.for_digest(user, 1.year.ago).should be_blank Topic.for_digest(user, 1.year.ago, top_order: true).should be_blank
end end
it "returns regular topics" do it "returns regular topics" do
topic = Fabricate(:topic) topic = Fabricate(:topic)
Topic.for_digest(user, 1.year.ago).should == [topic] Topic.for_digest(user, 1.year.ago, top_order: true).should == [topic]
end end
end end