github-mirror/discourse
github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-02-24 05:07:23 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: chat was enqueueing too many "chat summary" emails (#31133)

Browse Source 
due to an issue with LEFT JOIN, we were enqueue a "chat summary" email
for every new messages in a channel, instead of for every new mentions 😬

This bloated the sidekiq queue with a lot of unecessary jobs as seen in

- https://meta.discourse.org/t/-/347197
- https://meta.discourse.org/t/-/346542

Thankfully, it wasn't sending those emails as the query for listing the
unread mentions and dms was correct when generating the chat summary
email.
This commit is contained in:
Régis Hanol 2025-02-03 23:56:47 +01:00 committed by GitHub
parent 585c2b9ed3
commit 7245292fe1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 73 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
plugins/chat/lib/chat/mailer.rb
View File

@ -44,29 +44,39 @@ module Chat
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 '7 days'
AND cm.deleted_at IS NULL
AND NOT cm.created_by_sdk
ORDER BY chat_channel_id, cm.id
)
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
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
LEFT JOIN chat_mentions mn ON mn.chat_message_id = cm.first_unread_id
LEFT JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = mn.id
LEFT JOIN notifications n ON n.id = cmn.notification_id AND n.user_id = uccm.user_id
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)
AND (
(cc.chatable_type = 'DirectMessage' AND eu.allow_private_messages) OR
(cc.chatable_type = 'Category' AND uccm.following AND (n.id IS NULL OR NOT n.read))
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
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
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)
)
SELECT user_id FROM unread_dms
UNION
SELECT user_id FROM unread_mentions
SQL
end
end

42
plugins/chat/spec/components/chat/mailer_spec.rb
View File

@ -53,6 +53,14 @@ describe Chat::Mailer do
describe "in a followed channel" do
before { followed_channel.add(user) }
describe "there is a new message" do
let!(:chat_message) { create_message(followed_channel, "hello y'all :wave:") }
it "does not queue a chat summary" do
expect_not_enqueued
end
end
describe "user is @direct mentioned" do
let!(:chat_message) do
create_message(followed_channel, "hello @#{user.username}", Chat::UserMention)
@ -193,6 +201,14 @@ describe Chat::Mailer do
describe "in a non-followed channel" do
before { non_followed_channel.add(user).update!(following: false) }
describe "there is a new message" do
let!(:chat_message) { create_message(non_followed_channel, "hello y'all :wave:") }
it "does not queue a chat summary" do
expect_not_enqueued
end
end
describe "user is @direct mentioned" do
before { create_message(non_followed_channel, "hello @#{user.username}", Chat::UserMention) }
@ -221,6 +237,14 @@ describe Chat::Mailer do
describe "in a muted channel" do
before { muted_channel.add(user).update!(muted: true) }
describe "there is a new message" do
let!(:chat_message) { create_message(muted_channel, "hello y'all :wave:") }
it "does not queue a chat summary" do
expect_not_enqueued
end
end
describe "user is @direct mentioned" do
before { create_message(muted_channel, "hello @#{user.username}", Chat::UserMention) }
@ -247,6 +271,14 @@ describe Chat::Mailer do
end
describe "in an unseen channel" do
describe "there is a new message" do
let!(:chat_message) { create_message(unseen_channel, "hello y'all :wave:") }
it "does not queue a chat summary" do
expect_not_enqueued
end
end
describe "user is @direct mentioned" do
before { create_message(unseen_channel, "hello @#{user.username}") }
@ -262,6 +294,14 @@ describe Chat::Mailer do
expect_not_enqueued
end
end
describe "there is an @all mention" do
before { create_message(unseen_channel, "hello @all", Chat::AllMention) }
it "does not queue a chat summary email" do
expect_not_enqueued
end
end
end
describe "in a direct message" do
@ -271,7 +311,7 @@ describe Chat::Mailer do
expect_enqueued
end
it "queues a chat summary email when user isn't following the direct message anymore" do
it "queues a chat summary email even when user isn't following the direct message anymore" do
direct_message.membership_for(user).update!(following: false)
expect_enqueued
end