FIX: stuck notification for mentions within threads (#30546)

For mentions within threads, the mentioned user can experience a stuck notification. This is due to thread memberships only being created for users who interact with a thread. Without the membership we cannot track if the message containing the mention was read by the user.

The solution to this explored in this PR is:

- auto add memberships for mentioned users (only direct mentions for performance reasons).
- update channel/thread unread queries to check notification read status AND thread membership last read message id when counting mentions.

Previously the mention count would remain until the user notification (containing the mention) was read. However this only happens if the user clicks the notification or clicks dismiss all notifications. When a user navigated to the thread without clicking the notification, the green/urgent badge on chat would remain even after a hard page refresh.
This commit is contained in:
David Battersby 2025-01-06 17:26:37 +04:00 committed by GitHub
parent 27c557bc89
commit 67d568f709
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 34 additions and 10 deletions

View File

@ -381,6 +381,10 @@ module Chat
new_mentions = parsed_mentions.direct_mentions.pluck(:id)
delete_mentions("Chat::UserMention", old_mentions - new_mentions)
insert_mentions("Chat::UserMention", new_mentions - old_mentions)
# add users to threads when they are mentioned to track read status
return if new_mentions.empty? || !in_thread?
User.where(id: new_mentions).each { |user| thread.add(user, notification_level: :normal) }
end
end
end

View File

@ -37,15 +37,17 @@ module Chat
(
SELECT COUNT(*) AS mention_count
FROM notifications
INNER JOIN user_chat_channel_memberships ON user_chat_channel_memberships.user_id = :user_id
INNER JOIN user_chat_channel_memberships AS uccm ON uccm.user_id = :user_id
INNER JOIN chat_messages ON (data::json->>'chat_message_id')::bigint = chat_messages.id
LEFT JOIN chat_threads ON chat_threads.id = chat_messages.thread_id
LEFT JOIN user_chat_thread_memberships AS uctm ON uctm.thread_id = chat_messages.thread_id AND uctm.user_id = :user_id
WHERE NOT read
AND user_chat_channel_memberships.chat_channel_id = memberships.chat_channel_id
AND uccm.chat_channel_id = memberships.chat_channel_id
AND notifications.user_id = :user_id
AND notifications.notification_type = :notification_type_mention
AND (data::json->>'chat_message_id')::bigint > COALESCE(user_chat_channel_memberships.last_read_message_id, 0)
AND (data::json->>'chat_channel_id')::bigint = memberships.chat_channel_id
AND (((chat_messages.thread_id IS NULL OR chat_messages.id = chat_threads.original_message_id) AND chat_messages.id > COALESCE(uccm.last_read_message_id, 0))
OR (chat_messages.thread_id IS NOT NULL AND uctm.id IS NOT NULL AND chat_messages.id > COALESCE(uctm.last_read_message_id, 0)))
) AS mention_count,
(
SELECT COUNT(*) AS watched_threads_unread_count

View File

@ -64,16 +64,17 @@ module Chat
FROM notifications
INNER JOIN chat_messages ON chat_messages.id = (data::json->>'chat_message_id')::bigint
INNER JOIN chat_channels ON chat_channels.id = chat_messages.chat_channel_id
INNER JOIN user_chat_channel_memberships ON user_chat_channel_memberships.chat_channel_id = chat_messages.chat_channel_id
LEFT JOIN chat_threads ON chat_threads.id = chat_messages.thread_id
INNER JOIN user_chat_channel_memberships AS uccm ON uccm.chat_channel_id = chat_messages.chat_channel_id
INNER JOIN user_chat_thread_memberships AS uctm ON uctm.thread_id = chat_messages.thread_id AND uctm.user_id = :user_id
WHERE NOT read
AND notifications.user_id = :user_id
AND notifications.notification_type = :notification_type_mention
AND user_chat_channel_memberships.user_id = :user_id
AND uccm.user_id = :user_id
AND chat_channels.threading_enabled
AND chat_messages.deleted_at IS NULL
AND chat_messages.thread_id = memberships.thread_id
AND NOT user_chat_channel_memberships.muted
AND (uctm.id IS NOT NULL AND chat_messages.id > COALESCE(uctm.last_read_message_id, 0))
AND NOT uccm.muted
) AS mention_count,
(
SELECT COUNT(*) AS watched_threads_unread_count

View File

@ -39,7 +39,7 @@ export default class ChatChannelUnreadIndicator extends Component {
}
get urgentBadgeCount() {
let totalCount = this.urgentCount;
const totalCount = this.urgentCount;
return totalCount > MAX_UNREAD_COUNT ? `${MAX_UNREAD_COUNT}+` : totalCount;
}

View File

@ -69,7 +69,7 @@ export default class ChatHeaderIconUnreadIndicator extends Component {
}
get urgentCountLabel() {
let totalCount = this.onlyMentions ? this.mentionCount : this.urgentCount;
const totalCount = this.onlyMentions ? this.mentionCount : this.urgentCount;
return totalCount > MAX_UNREAD_COUNT ? `${MAX_UNREAD_COUNT}+` : totalCount;
}

View File

@ -787,6 +787,22 @@ describe Chat::Message do
expect(message.user_mentions.pluck(:target_id)).to match_array(already_mentioned)
expect(message.user_mentions.pluck(:id)).to include(*existing_mention_ids) # the mentions weren't recreated
end
it "creates thread memberships for mentioned users when replying to a thread" do
thread = Fabricate(:chat_thread)
thread_message =
Fabricate(
:chat_message,
chat_channel: thread.channel,
thread: thread,
message: "cc @#{user3.username} and @#{user4.username}",
)
thread_message.cook
thread_message.upsert_mentions
expect(thread.user_chat_thread_memberships.pluck(:user_id)).to include(user3.id, user4.id)
end
end
context "with group mentions" do

View File

@ -210,7 +210,7 @@ describe Chat::ChannelUnreadsQuery do
end
context "for unread mentions in a thread" do
fab!(:thread_om) { Fabricate(:chat_message, chat_channel: channel_1) }
fab!(:thread_om) { Fabricate(:chat_message, chat_channel: channel_1, user: current_user) }
fab!(:thread) { Fabricate(:chat_thread, channel: channel_1, original_message: thread_om) }
it "does include the original message in the mention count" do
@ -230,6 +230,7 @@ describe Chat::ChannelUnreadsQuery do
thread_message_2 = Fabricate(:chat_message, chat_channel: channel_1, thread: thread)
create_mention(thread_message_1, channel_1)
create_mention(thread_message_2, channel_1)
expect(query.first).to eq(
{
mention_count: 2,