IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-02-25 18:55:32 -06:00
Code Issues Packages Projects Releases Wiki Activity

FIX: send notifications after a chat message was updated with new mentions (#21173)

Browse Source 
Steps to reproduce the bug:
1. Send a chat message
2. Edit the message and add a mention to it
3. The mentioned user won't receive a notification

This PR fixes the problem.

Also:
1. There's no need anymore to have a code for removing notifications in the `notify_edit` method, because a call to `@chat_message.update_mentions` in the first line of the `notify_edit` method does that job:
    ff56f403a2/plugins/chat/lib/chat/notifier.rb (L90)

2. There's no need to load mention records from database, it's enough to pluck user ids
This commit is contained in:
Andrei Prigorshnev 2023-04-20 19:05:17 +04:00 committed by GitHub
parent dd495a0e19
commit 8b438767e5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
plugins/chat/lib/chat/notifier.rb
View File

@ -89,24 +89,17 @@ module Chat
def notify_edit def notify_edit
@chat_message.update_mentions(@mentions.all_mentioned_users_ids) @chat_message.update_mentions(@mentions.all_mentioned_users_ids)
existing_notifications = already_notified_user_ids =
Chat::Mention.includes(:user, :notification).where(chat_message: @chat_message) Chat::Mention
already_notified_user_ids = existing_notifications.map(&:user_id) .where(chat_message: @chat_message)
.where.not(notification: nil)
.pluck(:user_id)
to_notify = list_users_to_notify to_notify = list_users_to_notify
mentioned_user_ids = to_notify.extract!(:all_mentioned_user_ids)[:all_mentioned_user_ids] needs_notification_ids = to_notify[:all_mentioned_user_ids] - already_notified_user_ids
needs_deletion = already_notified_user_ids - mentioned_user_ids
needs_deletion.each do |user_id|
chat_mention = existing_notifications.detect { |n| n.user_id == user_id }
chat_mention.notification.destroy! if chat_mention.notification.present?
end
needs_notification_ids = mentioned_user_ids - already_notified_user_ids
return if needs_notification_ids.blank? return if needs_notification_ids.blank?
notify_creator_of_inaccessible_mentions(to_notify) notify_creator_of_inaccessible_mentions(to_notify)
notify_mentioned_users(to_notify, already_notified_user_ids: already_notified_user_ids) notify_mentioned_users(to_notify, already_notified_user_ids: already_notified_user_ids)
to_notify to_notify

15
plugins/chat/spec/components/chat/message_updater_spec.rb
View File

@ -126,16 +126,17 @@ describe Chat::MessageUpdater do
expect(events.map { _1[:event_name] }).to include(:chat_message_edited) expect(events.map { _1[:event_name] }).to include(:chat_message_edited)
end end
it "creates mention notifications for unmentioned users" do it "sends notifications if a message was updated with new mentions" do
chat_message = create_chat_message(user1, "This will be changed", public_chat_channel) message = create_chat_message(user1, "Mentioning @#{user2.username}", public_chat_channel)
expect {
Chat::MessageUpdater.update( Chat::MessageUpdater.update(
guardian: guardian, guardian: guardian,
chat_message: chat_message, chat_message: message,
new_content: new_content: "Mentioning @#{user2.username} and @#{user3.username}",
"this is a message with @system @mentions @#{user2.username} and @#{user3.username}",
) )
}.to change { user2.chat_mentions.count }.by(1).and change { user3.chat_mentions.count }.by(1)
mention = user3.chat_mentions.where(chat_message: message).first
expect(mention.notification).to be_present
end end
it "doesn't create mentions for already mentioned users" do it "doesn't create mentions for already mentioned users" do