FIX: delayed chat summary email (#31255)

Updates the chat summary email to account for:

- unread mentions in category channels (same as before)
- unread direct messages (now excluding threads)
- unread watched thread replies (for both channels and DM channels)

We have also reduced the window from 1 week down to 1 day for all 3
criteria. The DM unreads query is now properly selecting the first
unread message within the window (rather than the first message
regardless of read status).
This commit is contained in:
David Battersby
2025-02-24 14:25:52 +04:00
committed by GitHub
parent 18c8a8ffca
commit 342ab6f082
7 changed files with 228 additions and 36 deletions

View File

@ -242,6 +242,12 @@ en:
private_email:
one: "[%{site_name}] New message"
other: "[%{site_name}] New messages"
watched_thread:
one: "[%{site_name}] New thread message in %{channel}"
other: "[%{site_name}] New thread messages in %{channel}"
watched_threads:
one: "[%{site_name}] New thread messages in %{channel} and %{count} other"
other: "[%{site_name}] New thread messages in %{channel} and %{count} others"
chat_dm_1:
one: "[%{site_name}] New message from %{name}"
other: "[%{site_name}] New messages from %{name}"

View File

@ -0,0 +1,6 @@
# frozen_string_literal: true
class AddLastUnreadMessageWhenEmailedIdToChatThreadMemberships < ActiveRecord::Migration[7.2]
def change
add_column :user_chat_thread_memberships, :last_unread_message_when_emailed_id, :bigint
end
end

View File

@ -35,48 +35,73 @@ module Chat
DB.query_single <<~SQL
WITH eligible_users AS (
SELECT DISTINCT u.id, uo.allow_private_messages
SELECT u.id, uo.allow_private_messages
FROM users u
JOIN user_options uo ON uo.user_id = u.id
#{groups_join_sql}
JOIN user_options uo ON uo.user_id = u.id
AND uo.chat_enabled
AND uo.chat_email_frequency = #{UserOption.chat_email_frequencies[:when_away]}
AND uo.email_level <> #{UserOption.email_level_types[:never]}
WHERE u.last_seen_at < now() - interval '15 minutes'
AND uo.chat_enabled
AND uo.chat_email_frequency = #{UserOption.chat_email_frequencies[:when_away]}
AND uo.email_level <> #{UserOption.email_level_types[:never]}
), channel_messages AS (
SELECT DISTINCT ON (chat_channel_id) chat_channel_id, cm.id AS first_unread_id, user_id AS sender_id
FROM chat_messages cm
JOIN users sender ON sender.id = cm.user_id
WHERE cm.created_at > now() - interval '1 week'
AND cm.deleted_at IS NULL
AND NOT cm.created_by_sdk
ORDER BY chat_channel_id, cm.id
), unread_dms AS (
SELECT DISTINCT uccm.user_id
FROM user_chat_channel_memberships uccm
JOIN chat_channels cc ON cc.id = uccm.chat_channel_id AND cc.deleted_at IS NULL AND cc.chatable_type = 'DirectMessage'
JOIN channel_messages cm ON cm.chat_channel_id = cc.id AND cm.sender_id <> uccm.user_id
JOIN eligible_users eu ON eu.id = uccm.user_id AND eu.allow_private_messages
WHERE NOT uccm.muted
AND (uccm.last_read_message_id IS NULL OR cm.first_unread_id > uccm.last_read_message_id)
AND (uccm.last_unread_mention_when_emailed_id IS NULL OR cm.first_unread_id > uccm.last_unread_mention_when_emailed_id)
), unread_mentions AS (
SELECT DISTINCT uccm.user_id
FROM user_chat_channel_memberships uccm
JOIN chat_channels cc ON cc.id = uccm.chat_channel_id AND cc.deleted_at IS NULL AND cc.chatable_type = 'Category'
JOIN channel_messages cm ON cm.chat_channel_id = cc.id AND cm.sender_id <> uccm.user_id
FROM user_chat_channel_memberships uccm
JOIN eligible_users eu ON eu.id = uccm.user_id
JOIN chat_mentions mn ON mn.chat_message_id = cm.first_unread_id
JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = mn.id
JOIN notifications n ON n.id = cmn.notification_id AND n.user_id = uccm.user_id AND NOT n.read
AND eu.allow_private_messages
JOIN chat_messages cm ON cm.chat_channel_id = uccm.chat_channel_id
AND cm.deleted_at IS NULL
AND (cm.thread_id IS NULL OR cm.thread_id IN (SELECT id FROM chat_threads WHERE original_message_id = cm.id))
AND NOT cm.created_by_sdk
AND cm.created_at > now() - interval '1 day'
JOIN users sender ON sender.id = cm.user_id
JOIN chat_channels cc ON cc.id = cm.chat_channel_id
AND cc.deleted_at IS NULL
AND cc.chatable_type = 'DirectMessage'
WHERE NOT uccm.muted
AND uccm.following
AND (uccm.last_read_message_id IS NULL OR cm.first_unread_id > uccm.last_read_message_id)
AND (uccm.last_unread_mention_when_emailed_id IS NULL OR cm.first_unread_id > uccm.last_unread_mention_when_emailed_id)
AND (uccm.last_read_message_id IS NULL OR uccm.last_read_message_id < cm.id)
AND (uccm.last_unread_mention_when_emailed_id IS NULL OR uccm.last_unread_mention_when_emailed_id < cm.id)
), unread_mentions AS (
SELECT DISTINCT n.user_id
FROM notifications n
JOIN eligible_users eu ON eu.id = n.user_id
JOIN chat_mention_notifications cmn ON cmn.notification_id = n.id
JOIN chat_mentions mn ON mn.id = cmn.chat_mention_id
JOIN chat_messages cm ON cm.id = mn.chat_message_id
AND cm.deleted_at IS NULL
AND cm.thread_id IS NULL
AND NOT cm.created_by_sdk
AND cm.created_at > now() - interval '1 day'
JOIN users sender ON sender.id = cm.user_id
JOIN chat_channels cc ON cc.id = cm.chat_channel_id
AND cc.deleted_at IS NULL
AND cc.chatable_type = 'Category'
JOIN user_chat_channel_memberships uccm ON uccm.chat_channel_id = cc.id
AND uccm.user_id = n.user_id
AND NOT uccm.muted
AND uccm.following
AND (uccm.last_read_message_id IS NULL OR uccm.last_read_message_id < cm.id)
AND (uccm.last_unread_mention_when_emailed_id IS NULL OR uccm.last_unread_mention_when_emailed_id < cm.id)
WHERE NOT n.read
), unread_threads AS (
SELECT DISTINCT uctm.user_id
FROM user_chat_thread_memberships uctm
JOIN eligible_users eu ON eu.id = uctm.user_id
JOIN chat_threads ct ON ct.id = uctm.thread_id
JOIN chat_messages cm ON cm.thread_id = ct.id
AND cm.deleted_at IS NULL
AND NOT cm.created_by_sdk
AND cm.created_at > now() - interval '1 day'
JOIN users sender ON sender.id = cm.user_id
JOIN chat_channels cc ON cc.id = ct.channel_id
AND cc.deleted_at IS NULL
WHERE uctm.notification_level = #{Chat::NotificationLevels.all[:watching]}
AND (uctm.last_read_message_id IS NULL OR uctm.last_read_message_id < cm.id)
)
SELECT user_id FROM unread_dms
UNION
SELECT user_id FROM unread_mentions
UNION
SELECT user_id FROM unread_threads
SQL
end
end

View File

@ -40,7 +40,7 @@ module Chat
AND chat_channels.chatable_type = 'Category'
AND chat_messages.deleted_at IS NULL
AND chat_messages.user_id != uccm.user_id
AND chat_messages.created_at > now() - interval '1 week'
AND chat_messages.created_at > now() - interval '1 day'
AND (uccm.last_read_message_id IS NULL OR uccm.last_read_message_id < chat_messages.id)
AND (uccm.last_unread_mention_when_emailed_id IS NULL OR uccm.last_unread_mention_when_emailed_id < chat_messages.id)
AND (
@ -67,15 +67,17 @@ module Chat
JOIN chat_channels ON chat_channels.id = uccm.chat_channel_id
JOIN chat_messages ON chat_messages.chat_channel_id = chat_channels.id
JOIN users ON users.id = chat_messages.user_id
LEFT JOIN chat_threads om ON om.original_message_id = chat_messages.id
WHERE uccm.user_id = #{user.id}
AND NOT uccm.muted
AND chat_channels.deleted_at IS NULL
AND chat_channels.chatable_type = 'DirectMessage'
AND chat_messages.deleted_at IS NULL
AND chat_messages.user_id != uccm.user_id
AND chat_messages.created_at > now() - interval '1 week'
AND chat_messages.created_at > now() - interval '1 day'
AND (uccm.last_read_message_id IS NULL OR uccm.last_read_message_id < chat_messages.id)
AND (uccm.last_unread_mention_when_emailed_id IS NULL OR uccm.last_unread_mention_when_emailed_id < chat_messages.id)
AND (chat_messages.thread_id IS NULL OR chat_messages.thread_id IN (SELECT id FROM chat_threads WHERE original_message_id = chat_messages.id))
GROUP BY uccm.id
)
UPDATE user_chat_channel_memberships uccm
@ -86,12 +88,39 @@ module Chat
RETURNING um.membership_id, um.chat_channel_id, um.first_chat_message_id
SQL
unread_threads = DB.query_array <<~SQL
WITH unread_threads AS (
SELECT uctm.id membership_id, uctm.thread_id, MIN(chat_messages.id) first_chat_message_id, MAX(chat_messages.id) last_chat_message_id
FROM user_chat_thread_memberships uctm
JOIN chat_threads ON chat_threads.id = uctm.thread_id
JOIN chat_messages ON chat_messages.thread_id = chat_threads.id
JOIN users ON users.id = chat_messages.user_id
WHERE uctm.user_id = #{user.id}
AND uctm.notification_level = #{Chat::NotificationLevels.all[:watching]}
AND chat_messages.deleted_at IS NULL
AND chat_messages.created_at > now() - interval '1 day'
AND (uctm.last_read_message_id IS NULL OR uctm.last_read_message_id < chat_messages.id)
AND (uctm.last_unread_message_when_emailed_id IS NULL OR uctm.last_unread_message_when_emailed_id < chat_messages.id)
GROUP BY uctm.id
)
UPDATE user_chat_thread_memberships uctm
SET last_unread_message_when_emailed_id = ut.last_chat_message_id
FROM unread_threads ut
WHERE uctm.id = ut.membership_id
AND uctm.user_id = #{user.id}
RETURNING ut.membership_id, ut.thread_id, ut.first_chat_message_id
SQL
@grouped_channels = chat_messages_for(unread_mentions, guardian)
@grouped_dms =
user.user_option.allow_private_messages ? chat_messages_for(unread_messages, guardian) : {}
@count = @grouped_channels.values.sum(&:size) + @grouped_dms.values.sum(&:size)
@grouped_threads = chat_messages_for_threads(unread_threads, guardian)
@count =
@grouped_channels.values.sum(&:size) + @grouped_dms.values.sum(&:size) +
@grouped_threads.values.sum(&:size)
return if @count.zero?
@ -103,7 +132,7 @@ module Chat
build_email(
user.email,
from_alias: chat_summary_from_alias,
subject: chat_summary_subject(@grouped_channels, @grouped_dms, @count),
subject: chat_summary_subject(@grouped_channels, @grouped_dms, @grouped_threads, @count),
)
end
@ -129,6 +158,22 @@ module Chat
.select { |channel, _| guardian.can_join_chat_channel?(channel) }
end
def chat_messages_for_threads(data, guardian)
Chat::Message
.includes(:user, :chat_channel)
.where(thread_id: data.map { _1[1] })
.where(
"chat_messages.id >= (
SELECT min_unread_id
FROM (VALUES #{data.map { "(#{_1[1]}, #{_1[2]})" }.join(",")}) AS t(thread_id, min_unread_id)
WHERE t.thread_id = chat_messages.thread_id
)",
)
.order(created_at: :asc)
.group_by(&:chat_channel)
.select { |channel, _| guardian.can_join_chat_channel?(channel) }
end
def chat_summary_from_alias
I18n.t("user_notifications.chat_summary.from", site_name: @site_name)
end
@ -137,7 +182,7 @@ module Chat
I18n.t("user_notifications.chat_summary.subject.#{type}", { site_name: @site_name, **args })
end
def chat_summary_subject(grouped_channels, grouped_dms, count)
def chat_summary_subject(grouped_channels, grouped_dms, grouped_threads, count)
return subject(:private_email, count:) if SiteSetting.private_email
# consider "direct messages" with more than 2 users as group messages (aka. channels)
@ -177,6 +222,15 @@ module Chat
subject(:chat_dm_2, name_1: dms.first.title(@user), name_2: dms.second.title(@user))
elsif dms.size >= 3
subject(:chat_dm_3_or_more, name: dms.first.title(@user), count: dms.size - 1)
elsif grouped_threads.any?
channel = grouped_threads.keys.first
others_count = grouped_threads.keys.size - 1
if grouped_threads.keys.size == 1
subject(:watched_thread, channel: channel.title(@user), count: others_count)
else
subject(:watched_threads, channel: channel.title(@user), count: others_count)
end
else
subject(:private_email, count:)
end

View File

@ -325,5 +325,43 @@ describe Chat::Mailer do
user.user_option.update!(allow_private_messages: false)
expect_not_enqueued
end
it "queues a chat summary email when message is the original thread message" do
Fabricate(:chat_thread, channel: direct_message, original_message: Chat::Message.last)
expect_enqueued
end
end
describe "in direct message channel with threads" do
fab!(:dm_channel) { Fabricate(:direct_message_channel, users: [user, other]) }
fab!(:message) do
Fabricate(:chat_message, chat_channel: dm_channel, user: other, created_at: 2.weeks.ago)
end
fab!(:thread) do
Fabricate(:chat_thread, channel: dm_channel, original_message: message, with_replies: 1)
end
it "does not queue a chat summary email for thread replies" do
expect_not_enqueued
end
it "queues a chat summary email when user is watching the thread" do
Fabricate(
:user_chat_thread_membership,
user: user,
thread: thread,
notification_level: Chat::NotificationLevels.all[:watching],
)
expect_enqueued
end
it "does not queue a chat summary for threads watched by other users" do
thread.membership_for(other).update!(
notification_level: Chat::NotificationLevels.all[:watching],
)
expect_not_enqueued
end
end
end

View File

@ -265,6 +265,7 @@ Fabricator(:chat_thread, class_name: "Chat::Thread") do
transients[:with_replies],
:chat_message,
thread: thread,
chat_channel_id: thread.channel_id,
use_service: transients[:use_service],
)
.each { |message| thread.add(message.user) }

View File

@ -579,4 +579,66 @@ describe UserNotifications do
end
end
end
describe "in a direct message channel with threads" do
fab!(:message) do
Fabricate(:chat_message, chat_channel: direct_message, user: other, created_at: 2.days.ago)
end
fab!(:thread) { Fabricate(:chat_thread, channel: direct_message, original_message: message) }
fab!(:reply) { Fabricate(:chat_message, chat_channel: direct_message, thread:, user: other) }
let(:watching) { Chat::NotificationLevels.all[:watching] }
it "does not send a chat summary email for thread replies" do
no_chat_summary_email
end
describe "when the user is watching the thread" do
before do
Fabricate(:user_chat_thread_membership, user: user, thread:, notification_level: watching)
end
it "sends a chat summary email" do
chat_summary_email
end
end
describe "when the user has 2 watched threads" do
fab!(:message_2) do
Fabricate(
:chat_message,
chat_channel: direct_message_2,
user: another,
created_at: 2.days.ago,
)
end
fab!(:thread_2) do
Fabricate(:chat_thread, channel: direct_message_2, original_message: message_2)
end
fab!(:thread_2_reply) do
Fabricate(:chat_message, chat_channel: direct_message_2, thread: thread_2, user: another)
end
before do
Fabricate(:user_chat_thread_membership, user: user, thread:, notification_level: watching)
Fabricate(
:user_chat_thread_membership,
user: user,
thread: thread_2,
notification_level: watching,
)
end
it "sends a chat summary email" do
chat_summary_with_subject(:watched_threads, channel: direct_message.title(user), count: 1)
end
end
describe "when another user is watching a thread" do
before { thread.membership_for(other).update!(notification_level: watching) }
it "does not send current user a chat summary email" do
no_chat_summary_email
end
end
end
end