diff --git a/plugins/chat/lib/chat/notifier.rb b/plugins/chat/lib/chat/notifier.rb index aead57b628b..84c151e3ecf 100644 --- a/plugins/chat/lib/chat/notifier.rb +++ b/plugins/chat/lib/chat/notifier.rb @@ -132,6 +132,7 @@ module Chat .global_mentions .not_suspended .where(user_options: { ignore_channel_wide_mention: [false, nil] }) + .where.not(username_lower: @user.username_lower) .where.not(id: already_covered_ids) .pluck(:id) @@ -150,6 +151,7 @@ module Chat .here_mentions .not_suspended .where(user_options: { ignore_channel_wide_mention: [false, nil] }) + .where.not(username_lower: @user.username_lower) .where.not(id: already_covered_ids) .pluck(:id) @@ -187,9 +189,12 @@ module Chat direct_mentions = [] else direct_mentions = - @chat_message.parsed_mentions.direct_mentions.not_suspended.where.not( - id: already_covered_ids, - ) + @chat_message + .parsed_mentions + .direct_mentions + .not_suspended + .where.not(username_lower: @user.username_lower) + .where.not(id: already_covered_ids) end grouped = group_users_to_notify(direct_mentions) @@ -209,6 +214,7 @@ module Chat .group_mentions .not_suspended .where("user_count <= ?", SiteSetting.max_users_notified_per_group_mention) + .where.not(username_lower: @user.username_lower) .where.not(id: already_covered_ids) too_many_members, mentionable = diff --git a/plugins/chat/lib/chat/parsed_mentions.rb b/plugins/chat/lib/chat/parsed_mentions.rb index 14745b0d0ca..d6add90f626 100644 --- a/plugins/chat/lib/chat/parsed_mentions.rb +++ b/plugins/chat/lib/chat/parsed_mentions.rb @@ -91,7 +91,6 @@ module Chat .joins(:user_option) .real .where(user_options: { chat_enabled: true }) - .where.not(username_lower: @message.user.username.downcase) end def parse_mentions(message) diff --git a/plugins/chat/spec/components/chat/message_creator_spec.rb b/plugins/chat/spec/components/chat/message_creator_spec.rb index 192f5529bc6..b99a64ae877 100644 --- a/plugins/chat/spec/components/chat/message_creator_spec.rb +++ b/plugins/chat/spec/components/chat/message_creator_spec.rb @@ -146,9 +146,10 @@ describe Chat::MessageCreator do "this is a @#{user1.username} message with @system @mentions @#{user2.username} and @#{user3.username}", ).chat_message - # a mention for the user himself wasn't created + # a mention for the user himself was created, but a notification wasn't user1_mention = user1.chat_mentions.where(chat_message: message).first - expect(user1_mention).to be_nil + expect(user1_mention).to be_present + expect(user1_mention.notification).to be_nil system_user_mention = Discourse.system_user.chat_mentions.where(chat_message: message).first expect(system_user_mention).to be_nil @@ -172,59 +173,94 @@ describe Chat::MessageCreator do }.to change { user2.chat_mentions.count }.by(1) end - it "notifies @all properly" do - expect { - described_class.create(chat_channel: public_chat_channel, user: user1, content: "@all") - }.to change { Chat::Mention.count }.by(4) + context "when mentioning @all" do + it "creates a chat mention record for every user" do + expect { + described_class.create(chat_channel: public_chat_channel, user: user1, content: "@all") + }.to change { Chat::Mention.count }.by(5) - Chat::UserChatChannelMembership.where( - user: user2, - chat_channel: public_chat_channel, - ).update_all(following: false) - expect { - described_class.create( + Chat::UserChatChannelMembership.where( + user: user2, chat_channel: public_chat_channel, - user: user1, - content: "again! @all", - ) - }.to change { Chat::Mention.count }.by(3) + ).update_all(following: false) + + expect { + described_class.create( + chat_channel: public_chat_channel, + user: user1, + content: "again! @all", + ) + }.to change { Chat::Mention.count }.by(4) + end + + it "does not create a notification for acting user" do + message = + described_class.create( + chat_channel: public_chat_channel, + user: user1, + content: "@all", + ).chat_message + + mention = user1.chat_mentions.where(chat_message: message).first + expect(mention).to be_present + expect(mention.notification).to be_blank + end end - it "notifies @here properly" do - admin1.update(last_seen_at: 1.year.ago) - admin2.update(last_seen_at: 1.year.ago) - user1.update(last_seen_at: Time.now) - user2.update(last_seen_at: Time.now) - user3.update(last_seen_at: Time.now) - expect { - described_class.create(chat_channel: public_chat_channel, user: user1, content: "@here") - }.to change { Chat::Mention.count }.by(2) - end + context "when mentioning @here" do + it "creates a chat mention record for every active user" do + admin1.update(last_seen_at: 1.year.ago) + admin2.update(last_seen_at: 1.year.ago) - it "doesn't sent double notifications when '@here' is mentioned" do - user2.update(last_seen_at: Time.now) - expect { - described_class.create( - chat_channel: public_chat_channel, - user: user1, - content: "@here @#{user2.username}", - ) - }.to change { user2.chat_mentions.count }.by(1) - end + user1.update(last_seen_at: Time.now) + user2.update(last_seen_at: Time.now) + user3.update(last_seen_at: Time.now) - it "notifies @here plus other mentions" do - admin1.update(last_seen_at: Time.now) - admin2.update(last_seen_at: 1.year.ago) - user1.update(last_seen_at: 1.year.ago) - user2.update(last_seen_at: 1.year.ago) - user3.update(last_seen_at: 1.year.ago) - expect { - described_class.create( - chat_channel: public_chat_channel, - user: user1, - content: "@here plus @#{user3.username}", - ) - }.to change { user3.chat_mentions.count }.by(1) + expect { + described_class.create(chat_channel: public_chat_channel, user: user1, content: "@here") + }.to change { Chat::Mention.count }.by(3) + end + + it "does not create a notification for acting user" do + user1.update(last_seen_at: Time.now) + + message = + described_class.create( + chat_channel: public_chat_channel, + user: user1, + content: "@here", + ).chat_message + + mention = user1.chat_mentions.where(chat_message: message).first + expect(mention).to be_present + expect(mention.notification).to be_blank + end + + it "doesn't send double notifications" do + user2.update(last_seen_at: Time.now) + expect { + described_class.create( + chat_channel: public_chat_channel, + user: user1, + content: "@here @#{user2.username}", + ) + }.to change { user2.chat_mentions.count }.by(1) + end + + it "notifies @here plus other mentions" do + admin1.update(last_seen_at: Time.now) + admin2.update(last_seen_at: 1.year.ago) + user1.update(last_seen_at: 1.year.ago) + user2.update(last_seen_at: 1.year.ago) + user3.update(last_seen_at: 1.year.ago) + expect { + described_class.create( + chat_channel: public_chat_channel, + user: user1, + content: "@here plus @#{user3.username}", + ) + }.to change { user3.chat_mentions.count }.by(1) + end end it "doesn't create mention notifications for users without a membership record" do @@ -371,6 +407,19 @@ describe Chat::MessageCreator do end end + it "creates a chat_mention record without notification when self mentioning" do + message = + described_class.create( + chat_channel: direct_message_channel, + user: user1, + content: "hello @#{user1.username}", + ).chat_message + + mention = user1.chat_mentions.where(chat_message: message).first + expect(mention).to be_present + expect(mention.notification).to be_nil + end + context "when ignore_channel_wide_mention is enabled" do before { user2.user_option.update(ignore_channel_wide_mention: true) } @@ -887,6 +936,19 @@ describe Chat::MessageCreator do ) }.not_to change { Chat::Mention.count } end + + it "creates a chat mention without notification for acting user" do + message = + described_class.create( + chat_channel: public_chat_channel, + user: user1, + content: "@#{user_group.name}", + ).chat_message + + mention = user1.chat_mentions.where(chat_message: message).first + expect(mention).to be_present + expect(mention.notification).to be_blank + end end describe "push notifications" do diff --git a/plugins/chat/spec/components/chat/message_updater_spec.rb b/plugins/chat/spec/components/chat/message_updater_spec.rb index 19247162dc3..cb2682bf604 100644 --- a/plugins/chat/spec/components/chat/message_updater_spec.rb +++ b/plugins/chat/spec/components/chat/message_updater_spec.rb @@ -213,6 +213,21 @@ describe Chat::MessageUpdater do expect(mention.notification).to be_nil end + it "creates a chat_mention record without notification when self mentioning" do + chat_message = create_chat_message(user1, "I will mention myself soon", public_chat_channel) + new_content = "hello @#{user1.username}" + + described_class.update( + guardian: guardian, + chat_message: chat_message, + new_content: new_content, + ) + + mention = user1.chat_mentions.where(chat_message: chat_message).first + expect(mention).to be_present + expect(mention.notification).to be_nil + end + context "when updating a mentioned user" do it "updates the mention record" do chat_message = create_chat_message(user1, "ping @#{user2.username}", public_chat_channel) diff --git a/plugins/chat/spec/lib/chat/parsed_mentions_spec.rb b/plugins/chat/spec/lib/chat/parsed_mentions_spec.rb index ed1d679dcd6..f03ca7850d0 100644 --- a/plugins/chat/spec/lib/chat/parsed_mentions_spec.rb +++ b/plugins/chat/spec/lib/chat/parsed_mentions_spec.rb @@ -95,6 +95,21 @@ RSpec.describe Chat::ParsedMentions do expect(result).to contain_exactly(channel_member_1.username, channel_member_2.username) end + it "returns a user when self-mentioning" do + message = + Fabricate( + :chat_message, + chat_channel: chat_channel, + message: "Hey @#{channel_member_1.username}", + user: channel_member_1, + ) + + mentions = described_class.new(message) + + result = mentions.direct_mentions.pluck(:username) + expect(result).to contain_exactly(channel_member_1.username) + end + it "returns a mentioned user even if he's not a member of the channel" do message = create_message("mentioning @#{not_a_channel_member.username}")