From 7a1d60c60ec5895579c6ddb55aa888862c1aa38d Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Tue, 9 May 2023 19:19:26 +0200
Subject: [PATCH] FIX: Likes received count in digest email (#21458)

This commit fixes an issue where the Likes Received notification
count in the user digest email was not affected by the
since/last_seen date for the user, which meant that no matter
how long it had been since the user visited the count was
always constant.

Now instead for the Likes Received count, we only count the
unread notifications of that type since the user was last
seen.
---
 app/mailers/user_notifications.rb            |  6 ++++-
 app/models/user.rb                           |  5 ++--
 app/views/user_notifications/digest.html.erb |  4 ++--
 spec/mailers/user_notifications_spec.rb      | 25 ++++++++++++++++++++
 4 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb
index a5545b05bd5..656096d4fe9 100644
--- a/app/mailers/user_notifications.rb
+++ b/app/mailers/user_notifications.rb
@@ -291,6 +291,7 @@ class UserNotifications < ActionMailer::Base
       end
       @counts = [
         {
+          id: "new_topics",
           label_key: "user_notifications.digest.new_topics",
           value: new_topics_count,
           href: "#{Discourse.base_url}/new",
@@ -303,6 +304,7 @@ class UserNotifications < ActionMailer::Base
       value = user.unread_notifications + user.unread_high_priority_notifications
       if value > 0
         @counts << {
+          id: "unread_notifications",
           label_key: "user_notifications.digest.unread_notifications",
           value: value,
           href: "#{Discourse.base_url}/my/notifications",
@@ -310,9 +312,10 @@ class UserNotifications < ActionMailer::Base
       end
 
       if @counts.size < 3
-        value = user.unread_notifications_of_type(Notification.types[:liked])
+        value = user.unread_notifications_of_type(Notification.types[:liked], since: min_date)
         if value > 0
           @counts << {
+            id: "likes_received",
             label_key: "user_notifications.digest.liked_received",
             value: value,
             href: "#{Discourse.base_url}/my/notifications",
@@ -324,6 +327,7 @@ class UserNotifications < ActionMailer::Base
         value = summary_new_users_count(min_date)
         if value > 0
           @counts << {
+            id: "new_users",
             label_key: "user_notifications.digest.new_users",
             value: value,
             href: "#{Discourse.base_url}/about",
diff --git a/app/models/user.rb b/app/models/user.rb
index b8e81d8d0a4..ae589ce8569 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -607,7 +607,7 @@ class User < ActiveRecord::Base
     @muted_user_ids ||= muted_users.pluck(:id)
   end
 
-  def unread_notifications_of_type(notification_type)
+  def unread_notifications_of_type(notification_type, since: nil)
     # perf critical, much more efficient than AR
     sql = <<~SQL
         SELECT COUNT(*)
@@ -617,10 +617,11 @@ class User < ActiveRecord::Base
            AND n.notification_type = :notification_type
            AND n.user_id = :user_id
            AND NOT read
+           #{since ? "AND n.created_at > :since" : ""}
     SQL
 
     # to avoid coalesce we do to_i
-    DB.query_single(sql, user_id: id, notification_type: notification_type)[0].to_i
+    DB.query_single(sql, user_id: id, notification_type: notification_type, since: since)[0].to_i
   end
 
   def unread_notifications_of_priority(high_priority:)
diff --git a/app/views/user_notifications/digest.html.erb b/app/views/user_notifications/digest.html.erb
index 4c6f7344228..71b6531bec9 100644
--- a/app/views/user_notifications/digest.html.erb
+++ b/app/views/user_notifications/digest.html.erb
@@ -56,14 +56,14 @@
                         <tbody>
                           <tr class="header-stat-count">
                             <%- @counts.each do |count| -%>
-                              <td style="text-align:center;font-size:36px;font-weight:400;">
+                              <td id="<%= count[:id] %>_stat_count" style="text-align:center;font-size:36px;font-weight:400;">
                                 <a class="with-accent-colors" href="<%= count[:href] -%>"><strong><%= count[:value] -%></strong></a>
                               </td>
                             <%- end -%>
                           </tr>
                           <tr class="header-stat-description">
                             <%- @counts.each do |count| -%>
-                              <td style="font-size:14px;font-weight:400;text-align:center;">
+                              <td id="<%= count[:id] %>_stat_description" style="font-size:14px;font-weight:400;text-align:center;">
                                 <a class="with-accent-colors" href="<%= count[:href] -%>"><strong><%=t count[:label_key] -%></strong></a>
                               </td>
                             <%- end -%>
diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb
index d4eab5c1a3f..b9ec2cc39c5 100644
--- a/spec/mailers/user_notifications_spec.rb
+++ b/spec/mailers/user_notifications_spec.rb
@@ -214,6 +214,31 @@ RSpec.describe UserNotifications do
         expect(subject.subject).not_to match(/Discourse Meta/)
       end
 
+      it "includes unread likes received count within the since date" do
+        Fabricate(
+          :notification,
+          user: user,
+          notification_type: Notification.types[:liked],
+          created_at: 2.months.ago,
+        )
+        Fabricate(
+          :notification,
+          user: user,
+          notification_type: Notification.types[:liked],
+          read: true,
+        )
+        Fabricate(:notification, user: user, notification_type: Notification.types[:liked])
+        Fabricate(:notification, user: user, notification_type: Notification.types[:liked])
+        digest = UserNotifications.digest(user, since: 1.month.ago.to_date.to_s)
+        parsed_html = Nokogiri::HTML5.fragment(digest.html_part.body.to_s)
+        expect(parsed_html.css(".header-stat-count #likes_received_stat_count strong").text).to eq(
+          "2",
+        )
+        expect(
+          parsed_html.css(".header-stat-description #likes_received_stat_description strong").text,
+        ).to eq("Likes Received")
+      end
+
       it "excludes deleted topics and their posts" do
         deleted =
           Fabricate(