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.
This commit is contained in:
Martin Brennan 2023-05-09 19:19:26 +02:00 committed by GitHub
parent 9ae5ddb330
commit 7a1d60c60e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 35 additions and 5 deletions

View File

@ -291,6 +291,7 @@ class UserNotifications < ActionMailer::Base
end end
@counts = [ @counts = [
{ {
id: "new_topics",
label_key: "user_notifications.digest.new_topics", label_key: "user_notifications.digest.new_topics",
value: new_topics_count, value: new_topics_count,
href: "#{Discourse.base_url}/new", href: "#{Discourse.base_url}/new",
@ -303,6 +304,7 @@ class UserNotifications < ActionMailer::Base
value = user.unread_notifications + user.unread_high_priority_notifications value = user.unread_notifications + user.unread_high_priority_notifications
if value > 0 if value > 0
@counts << { @counts << {
id: "unread_notifications",
label_key: "user_notifications.digest.unread_notifications", label_key: "user_notifications.digest.unread_notifications",
value: value, value: value,
href: "#{Discourse.base_url}/my/notifications", href: "#{Discourse.base_url}/my/notifications",
@ -310,9 +312,10 @@ class UserNotifications < ActionMailer::Base
end end
if @counts.size < 3 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 if value > 0
@counts << { @counts << {
id: "likes_received",
label_key: "user_notifications.digest.liked_received", label_key: "user_notifications.digest.liked_received",
value: value, value: value,
href: "#{Discourse.base_url}/my/notifications", href: "#{Discourse.base_url}/my/notifications",
@ -324,6 +327,7 @@ class UserNotifications < ActionMailer::Base
value = summary_new_users_count(min_date) value = summary_new_users_count(min_date)
if value > 0 if value > 0
@counts << { @counts << {
id: "new_users",
label_key: "user_notifications.digest.new_users", label_key: "user_notifications.digest.new_users",
value: value, value: value,
href: "#{Discourse.base_url}/about", href: "#{Discourse.base_url}/about",

View File

@ -607,7 +607,7 @@ class User < ActiveRecord::Base
@muted_user_ids ||= muted_users.pluck(:id) @muted_user_ids ||= muted_users.pluck(:id)
end end
def unread_notifications_of_type(notification_type) def unread_notifications_of_type(notification_type, since: nil)
# perf critical, much more efficient than AR # perf critical, much more efficient than AR
sql = <<~SQL sql = <<~SQL
SELECT COUNT(*) SELECT COUNT(*)
@ -617,10 +617,11 @@ class User < ActiveRecord::Base
AND n.notification_type = :notification_type AND n.notification_type = :notification_type
AND n.user_id = :user_id AND n.user_id = :user_id
AND NOT read AND NOT read
#{since ? "AND n.created_at > :since" : ""}
SQL SQL
# to avoid coalesce we do to_i # 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 end
def unread_notifications_of_priority(high_priority:) def unread_notifications_of_priority(high_priority:)

View File

@ -56,14 +56,14 @@
<tbody> <tbody>
<tr class="header-stat-count"> <tr class="header-stat-count">
<%- @counts.each do |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> <a class="with-accent-colors" href="<%= count[:href] -%>"><strong><%= count[:value] -%></strong></a>
</td> </td>
<%- end -%> <%- end -%>
</tr> </tr>
<tr class="header-stat-description"> <tr class="header-stat-description">
<%- @counts.each do |count| -%> <%- @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> <a class="with-accent-colors" href="<%= count[:href] -%>"><strong><%=t count[:label_key] -%></strong></a>
</td> </td>
<%- end -%> <%- end -%>

View File

@ -214,6 +214,31 @@ RSpec.describe UserNotifications do
expect(subject.subject).not_to match(/Discourse Meta/) expect(subject.subject).not_to match(/Discourse Meta/)
end 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 it "excludes deleted topics and their posts" do
deleted = deleted =
Fabricate( Fabricate(