From 54b2a85b27b103b05d23c7cdd7861bd3536cc182 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 9 May 2023 13:00:19 +0200 Subject: [PATCH] FIX: ensures `all_mentioned_user_ids` is not used as identifier (#21452) When making the list of users to notify we set `all_mentioned_user_ids` key on the `to_notify` Hash. This hash will be passed around until the actual moment where we send the notifications: ```ruby identifier_text = case identifier_type when :here_mentions "@here" when :global_mentions "@all" when :direct_mentions "" else "@#{identifier_type}" end ``` As not found `all_mentioned_user_ids` would end up being sent as `@all_mentioned_user_ids` which is obviously incorrect. This commit is a direct fix to the issue and will remove the key as soon as we have used it sooner up in the chain. This bug was reproducible when doing this sequence of events: - create a message with a direct mention: `@bob hi` - edit this message into a global mention `@all hi` --- plugins/chat/lib/chat/notifier.rb | 2 +- plugins/chat/spec/lib/chat/notifier_spec.rb | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/plugins/chat/lib/chat/notifier.rb b/plugins/chat/lib/chat/notifier.rb index db1a95b36a5..aead57b628b 100644 --- a/plugins/chat/lib/chat/notifier.rb +++ b/plugins/chat/lib/chat/notifier.rb @@ -95,7 +95,7 @@ module Chat notify_creator_of_inaccessible_mentions(to_notify) notify_mentioned_users(to_notify, already_notified_user_ids: already_notified_user_ids) - to_notify + to_notify.except(:all_mentioned_user_ids) end private diff --git a/plugins/chat/spec/lib/chat/notifier_spec.rb b/plugins/chat/spec/lib/chat/notifier_spec.rb index f9ff6d7e1a2..9c55e86bf3a 100644 --- a/plugins/chat/spec/lib/chat/notifier_spec.rb +++ b/plugins/chat/spec/lib/chat/notifier_spec.rb @@ -121,6 +121,23 @@ describe Chat::Notifier do include_examples "channel-wide mentions" include_examples "ensure only channel members are notified" + describe "editing a direct mention into a global mention" do + let(:mention) { "hello @#{user_2.username}!" } + + it "doesn't return :all_mentioned_user_ids" do + msg = build_cooked_msg(mention, user_1) + + Chat::MessageUpdater.update( + guardian: user_1.guardian, + chat_message: msg, + new_content: "hello @all", + ) + + to_notify = described_class.new(msg, msg.created_at).notify_edit + expect(to_notify[:all_mentioned_user_ids]).not_to be_present + end + end + describe "users ignoring or muting the user creating the message" do it "does not send notifications to the user who is muting the acting user" do Fabricate(:muted_user, user: user_2, muted_user: user_1)