diff --git a/plugins/chat/lib/chat_message_mentions.rb b/plugins/chat/lib/chat_message_mentions.rb new file mode 100644 index 00000000000..84516466b09 --- /dev/null +++ b/plugins/chat/lib/chat_message_mentions.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +class Chat::ChatMessageMentions + def initialize(message) + @message = message + + mentions = parse_mentions(message) + group_mentions = parse_group_mentions(message) + + @has_global_mention = mentions.include?("@all") + @has_here_mention = mentions.include?("@here") + @parsed_direct_mentions = normalize(mentions) + @parsed_group_mentions = normalize(group_mentions) + end + + attr_accessor :has_global_mention, + :has_here_mention, + :parsed_direct_mentions, + :parsed_group_mentions + + def global_mentions + return User.none unless @has_global_mention + channel_members.where.not(username_lower: @parsed_direct_mentions) + end + + def direct_mentions + chat_users.where(username_lower: @parsed_direct_mentions) + end + + def group_mentions + chat_users.includes(:groups).joins(:groups).where(groups: mentionable_groups) + end + + def here_mentions + return User.none unless @has_here_mention + + channel_members + .where("last_seen_at > ?", 5.minutes.ago) + .where.not(username_lower: @parsed_direct_mentions) + end + + def mentionable_groups + @mentionable_groups ||= + Group.mentionable(@message.user, include_public: false).where(id: visible_groups.map(&:id)) + end + + def visible_groups + @visible_groups ||= + Group.where("LOWER(name) IN (?)", @parsed_group_mentions).visible_groups(@message.user) + end + + private + + def channel_members + chat_users.where( + user_chat_channel_memberships: { + following: true, + chat_channel_id: @message.chat_channel.id, + }, + ) + end + + def chat_users + User + .includes(:user_chat_channel_memberships, :group_users) + .distinct + .joins("LEFT OUTER JOIN user_chat_channel_memberships uccm ON uccm.user_id = users.id") + .joins(:user_option) + .real + .not_suspended + .where(user_options: { chat_enabled: true }) + .where.not(username_lower: @message.user.username.downcase) + end + + def parse_mentions(message) + Nokogiri::HTML5.fragment(message.cooked).css(".mention").map(&:text) + end + + def parse_group_mentions(message) + Nokogiri::HTML5.fragment(message.cooked).css(".mention-group").map(&:text) + end + + def normalize(mentions) + mentions.reduce([]) do |memo, mention| + %w[@here @all].include?(mention.downcase) ? memo : (memo << mention[1..-1].downcase) + end + end +end diff --git a/plugins/chat/lib/chat_notifier.rb b/plugins/chat/lib/chat_notifier.rb index 4719b968019..7887cfade9a 100644 --- a/plugins/chat/lib/chat_notifier.rb +++ b/plugins/chat/lib/chat_notifier.rb @@ -60,6 +60,7 @@ class Chat::ChatNotifier @timestamp = timestamp @chat_channel = @chat_message.chat_channel @user = @chat_message.user + @mentions = Chat::ChatMessageMentions.new(chat_message) end ### Public API @@ -108,11 +109,12 @@ class Chat::ChatNotifier private def list_users_to_notify - direct_mentions_count = direct_mentions_from_cooked.length - group_mentions_count = group_name_mentions.length + mentions_count = + @mentions.parsed_direct_mentions.length + @mentions.parsed_group_mentions.length + mentions_count += 1 if @mentions.has_global_mention + mentions_count += 1 if @mentions.has_here_mention - skip_notifications = - (direct_mentions_count + group_mentions_count) > SiteSetting.max_mentions_per_chat_message + skip_notifications = mentions_count > SiteSetting.max_mentions_per_chat_message {}.tap do |to_notify| # The order of these methods is the precedence @@ -131,48 +133,13 @@ class Chat::ChatNotifier end end - def chat_users - User - .includes(:user_chat_channel_memberships, :group_users) - .distinct - .joins("LEFT OUTER JOIN user_chat_channel_memberships uccm ON uccm.user_id = users.id") - .joins(:user_option) - .real - .not_suspended - .where(user_options: { chat_enabled: true }) - .where.not(username_lower: @user.username.downcase) - end - - def rest_of_the_channel - chat_users.where( - user_chat_channel_memberships: { - following: true, - chat_channel_id: @chat_channel.id, - }, - ) - end - - def members_accepting_channel_wide_notifications - rest_of_the_channel.where(user_options: { ignore_channel_wide_mention: [false, nil] }) - end - - def direct_mentions_from_cooked - @direct_mentions_from_cooked ||= - Nokogiri::HTML5.fragment(@chat_message.cooked).css(".mention").map(&:text) - end - - def normalized_mentions(mentions) - mentions.reduce([]) do |memo, mention| - %w[@here @all].include?(mention.downcase) ? memo : (memo << mention[1..-1].downcase) - end - end - def expand_global_mention(to_notify, already_covered_ids, skip) - typed_global_mention = direct_mentions_from_cooked.include?("@all") + has_all_mention = @mentions.has_global_mention - if typed_global_mention && @chat_channel.allow_channel_wide_mentions && !skip - to_notify[:global_mentions] = members_accepting_channel_wide_notifications - .where.not(username_lower: normalized_mentions(direct_mentions_from_cooked)) + if has_all_mention && @chat_channel.allow_channel_wide_mentions && !skip + to_notify[:global_mentions] = @mentions + .global_mentions + .where(user_options: { ignore_channel_wide_mention: [false, nil] }) .where.not(id: already_covered_ids) .pluck(:id) @@ -183,12 +150,12 @@ class Chat::ChatNotifier end def expand_here_mention(to_notify, already_covered_ids, skip) - typed_here_mention = direct_mentions_from_cooked.include?("@here") + has_here_mention = @mentions.has_here_mention - if typed_here_mention && @chat_channel.allow_channel_wide_mentions && !skip - to_notify[:here_mentions] = members_accepting_channel_wide_notifications - .where("last_seen_at > ?", 5.minutes.ago) - .where.not(username_lower: normalized_mentions(direct_mentions_from_cooked)) + if has_here_mention && @chat_channel.allow_channel_wide_mentions && !skip + to_notify[:here_mentions] = @mentions + .here_mentions + .where(user_options: { ignore_channel_wide_mention: [false, nil] }) .where.not(id: already_covered_ids) .pluck(:id) @@ -225,10 +192,7 @@ class Chat::ChatNotifier if skip direct_mentions = [] else - direct_mentions = - chat_users - .where(username_lower: normalized_mentions(direct_mentions_from_cooked)) - .where.not(id: already_covered_ids) + direct_mentions = @mentions.direct_mentions.where.not(id: already_covered_ids) end grouped = group_users_to_notify(direct_mentions) @@ -239,48 +203,31 @@ class Chat::ChatNotifier already_covered_ids.concat(to_notify[:direct_mentions]) end - def group_name_mentions - @group_mentions_from_cooked ||= - normalized_mentions( - Nokogiri::HTML5.fragment(@chat_message.cooked).css(".mention-group").map(&:text), - ) - end - - def visible_groups - @visible_groups ||= Group.where("LOWER(name) IN (?)", group_name_mentions).visible_groups(@user) - end - def expand_group_mentions(to_notify, already_covered_ids, skip) - return [] if skip || visible_groups.empty? + return [] if skip || @mentions.visible_groups.empty? - mentionable_groups = - Group.mentionable(@user, include_public: false).where(id: visible_groups.map(&:id)) - - mentions_disabled = visible_groups - mentionable_groups + reached_by_group = + @mentions + .group_mentions + .where("user_count <= ?", SiteSetting.max_users_notified_per_group_mention) + .where.not(id: already_covered_ids) too_many_members, mentionable = - mentionable_groups.partition do |group| + @mentions.mentionable_groups.partition do |group| group.user_count > SiteSetting.max_users_notified_per_group_mention end + mentions_disabled = @mentions.visible_groups - @mentions.mentionable_groups to_notify[:group_mentions_disabled] = mentions_disabled to_notify[:too_many_members] = too_many_members - mentionable.each { |g| to_notify[g.name.downcase] = [] } - reached_by_group = - chat_users - .includes(:groups) - .joins(:groups) - .where(groups: mentionable) - .where.not(id: already_covered_ids) - grouped = group_users_to_notify(reached_by_group) - grouped[:already_participating].each do |user| # When a user is a member of multiple mentioned groups, # the most far to the left should take precedence. - ordered_group_names = group_name_mentions & mentionable.map { |mg| mg.name.downcase } + ordered_group_names = + @mentions.parsed_group_mentions & mentionable.map { |mg| mg.name.downcase } user_group_names = user.groups.map { |ug| ug.name.downcase } group_name = ordered_group_names.detect { |gn| user_group_names.include?(gn) } diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index c5b2f5c6255..8a4792cb653 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -178,6 +178,7 @@ after_initialize do load File.expand_path("../lib/chat_message_updater.rb", __FILE__) load File.expand_path("../lib/chat_message_rate_limiter.rb", __FILE__) load File.expand_path("../lib/chat_message_reactor.rb", __FILE__) + load File.expand_path("../lib/chat_message_mentions.rb", __FILE__) load File.expand_path("../lib/chat_notifier.rb", __FILE__) load File.expand_path("../lib/chat_seeder.rb", __FILE__) load File.expand_path("../lib/chat_statistics.rb", __FILE__) diff --git a/plugins/chat/spec/lib/chat_message_mentions_spec.rb b/plugins/chat/spec/lib/chat_message_mentions_spec.rb new file mode 100644 index 00000000000..7ff25c873dd --- /dev/null +++ b/plugins/chat/spec/lib/chat_message_mentions_spec.rb @@ -0,0 +1,154 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Chat::ChatMessageMentions do + fab!(:channel_member_1) { Fabricate(:user) } + fab!(:channel_member_2) { Fabricate(:user) } + fab!(:channel_member_3) { Fabricate(:user) } + fab!(:not_a_channel_member) { Fabricate(:user) } + fab!(:chat_channel) { Fabricate(:chat_channel) } + + before do + chat_channel.add(channel_member_1) + chat_channel.add(channel_member_2) + chat_channel.add(channel_member_3) + end + + describe "#global_mentions" do + it "returns all members of the channel" do + message = create_message("mentioning @all") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.global_mentions.pluck(:username) + + expect(result).to contain_exactly( + channel_member_1.username, + channel_member_2.username, + channel_member_3.username, + ) + end + + it "doesn't include users that were also mentioned directly" do + message = create_message("mentioning @all and @#{channel_member_1.username}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.global_mentions.pluck(:username) + + expect(result).to contain_exactly(channel_member_2.username, channel_member_3.username) + end + + it "returns an empty list if there are no global mentions" do + message = create_message("not mentioning anybody") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.global_mentions.pluck(:username) + + expect(result).to be_empty + end + end + + describe "#here_mentions" do + before do + freeze_time + channel_member_1.update(last_seen_at: 2.minutes.ago) + channel_member_2.update(last_seen_at: 2.minutes.ago) + channel_member_3.update(last_seen_at: 5.minutes.ago) + end + + it "returns all members of the channel who were online in the last 5 minutes" do + message = create_message("mentioning @here") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.here_mentions.pluck(:username) + + expect(result).to contain_exactly(channel_member_1.username, channel_member_2.username) + end + + it "doesn't include users that were also mentioned directly" do + message = create_message("mentioning @here and @#{channel_member_1.username}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.here_mentions.pluck(:username) + + expect(result).to contain_exactly(channel_member_2.username) + end + + it "returns an empty list if there are no here mentions" do + message = create_message("not mentioning anybody") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.here_mentions.pluck(:username) + + expect(result).to be_empty + end + end + + describe "#direct_mentions" do + it "returns users who were mentioned directly" do + message = + create_message("mentioning @#{channel_member_1.username} and @#{channel_member_2.username}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.direct_mentions.pluck(:username) + + expect(result).to contain_exactly(channel_member_1.username, channel_member_2.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}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.direct_mentions.pluck(:username) + + expect(result).to contain_exactly(not_a_channel_member.username) + end + + it "returns an empty list if no one was mentioned directly" do + message = create_message("not mentioning anybody") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.direct_mentions.pluck(:username) + + expect(result).to be_empty + end + end + + describe "#group_mentions" do + fab!(:group1) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone]) } + fab!(:group_member_1) { Fabricate(:user, group_ids: [group1.id]) } + fab!(:group_member_2) { Fabricate(:user, group_ids: [group1.id]) } + fab!(:group_member_3) { Fabricate(:user, group_ids: [group1.id]) } + + before do + chat_channel.add(group_member_1) + chat_channel.add(group_member_2) + end + + it "returns members of a mentioned group even if some of them is not members of the channel" do + message = create_message("mentioning @#{group1.name}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.group_mentions.pluck(:username) + + expect(result).to contain_exactly( + group_member_1.username, + group_member_2.username, + group_member_3.username, + ) + end + + it "returns an empty list if no group was mentioned" do + message = create_message("not mentioning anybody") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.group_mentions.pluck(:username) + + expect(result).to be_empty + end + end + + def create_message(text) + Fabricate(:chat_message, chat_channel: chat_channel, message: text) + end +end