From 4c2d6e19ba8ebdaefbfc4d506571eb6b404a9e72 Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Fri, 25 Oct 2019 16:33:05 -0400
Subject: [PATCH] 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.
---
 app/mailers/user_notifications.rb       | 18 ++++++++++++++++--
 spec/mailers/user_notifications_spec.rb | 17 +++++++++++++++++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb
index a3450b00d6a..3d2de7cb49d 100644
--- a/app/mailers/user_notifications.rb
+++ b/app/mailers/user_notifications.rb
@@ -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
       end
 
-      if @counts.size < 3
-        value = User.real.where(active: true, staged: false).not_suspended.where("created_at > ?", min_date).count
+      if @counts.size < 3 && user.user_option.digest_after_minutes >= 1440
+        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
       end
 
@@ -676,4 +676,18 @@ class UserNotifications < ActionMailer::Base
     @unsubscribe_key = UnsubscribeKey.create_key_for(@user, "digest")
     @disable_email_custom_styles = !SiteSetting.apply_custom_styles_to_digest
   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
diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb
index f0640a0335d..34cffcbe585 100644
--- a/spec/mailers/user_notifications_spec.rb
+++ b/spec/mailers/user_notifications_spec.rb
@@ -107,6 +107,10 @@ describe UserNotifications do
 
     subject { UserNotifications.digest(user) }
 
+    after do
+      $redis.keys('summary-new-users:*').each { |key| $redis.del(key) }
+    end
+
     context "without new topics" 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.text_part.body.to_s).to be_present
         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
 
       it "includes email_prefix in email subject instead of site title" do