PERF: cache new users counts in summary emails

The query to count how many new users there are since a given date
is expensive. It's the least personalized stat and the one we fallback
to last when no better number can be found for the target user.
Give up accuracy so we can aggressively cache the user counts
that appear in this email.
This commit is contained in:
Neil Lalonde 2019-10-25 16:33:05 -04:00
parent d76d0e75ec
commit 4c2d6e19ba
2 changed files with 33 additions and 2 deletions

View File

@ -224,8 +224,8 @@ class UserNotifications < ActionMailer::Base
@counts << { label_key: 'user_notifications.digest.liked_received', value: value, href: "#{Discourse.base_url}/my/notifications" } if value > 0 @counts << { label_key: 'user_notifications.digest.liked_received', value: value, href: "#{Discourse.base_url}/my/notifications" } if value > 0
end end
if @counts.size < 3 if @counts.size < 3 && user.user_option.digest_after_minutes >= 1440
value = User.real.where(active: true, staged: false).not_suspended.where("created_at > ?", min_date).count value = summary_new_users_count(min_date)
@counts << { label_key: 'user_notifications.digest.new_users', value: value, href: "#{Discourse.base_url}/about" } if value > 0 @counts << { label_key: 'user_notifications.digest.new_users', value: value, href: "#{Discourse.base_url}/about" } if value > 0
end end
@ -676,4 +676,18 @@ class UserNotifications < ActionMailer::Base
@unsubscribe_key = UnsubscribeKey.create_key_for(@user, "digest") @unsubscribe_key = UnsubscribeKey.create_key_for(@user, "digest")
@disable_email_custom_styles = !SiteSetting.apply_custom_styles_to_digest @disable_email_custom_styles = !SiteSetting.apply_custom_styles_to_digest
end end
def self.summary_new_users_count_key(min_date_str)
"summary-new-users:#{min_date_str}"
end
def summary_new_users_count(min_date)
min_date_str = min_date.is_a?(String) ? min_date : min_date.strftime('%Y-%m-%d')
key = self.class.summary_new_users_count_key(min_date_str)
((count = $redis.get(key)) && count.to_i) || begin
count = User.real.where(active: true, staged: false).not_suspended.where("created_at > ?", min_date_str).count
$redis.setex(key, 1.day, count)
count
end
end
end end

View File

@ -107,6 +107,10 @@ describe UserNotifications do
subject { UserNotifications.digest(user) } subject { UserNotifications.digest(user) }
after do
$redis.keys('summary-new-users:*').each { |key| $redis.del(key) }
end
context "without new topics" do context "without new topics" do
it "doesn't send the email" do it "doesn't send the email" do
@ -138,6 +142,19 @@ describe UserNotifications do
expect(subject.html_part.body.to_s).to be_present expect(subject.html_part.body.to_s).to be_present
expect(subject.text_part.body.to_s).to be_present expect(subject.text_part.body.to_s).to be_present
expect(subject.header["List-Unsubscribe"].to_s).to match(/\/email\/unsubscribe\/\h{64}/) expect(subject.header["List-Unsubscribe"].to_s).to match(/\/email\/unsubscribe\/\h{64}/)
expect(subject.html_part.body.to_s).to include('New Users')
end
it "doesn't include new user count if digest_after_minutes is low" do
user.user_option.digest_after_minutes = 60
expect(subject.html_part.body.to_s).to_not include('New Users')
end
it "works with min_date string" do
digest = UserNotifications.digest(user, since: 1.month.ago.to_date.to_s)
expect(digest.html_part.body.to_s).to be_present
expect(digest.text_part.body.to_s).to be_present
expect(digest.html_part.body.to_s).to include('New Users')
end end
it "includes email_prefix in email subject instead of site title" do it "includes email_prefix in email subject instead of site title" do