Fix channel wide queries

Move filter mute/ignored users to the background job.

config/site_settings.yml

@@ -913,7 +913,6 @@ posting:
   max_mentions_per_post: 10
   max_users_notified_per_group_mention:
     default: 100
-    max: 250
     client: true
   newuser_max_replies_per_topic: 3
   newuser_max_mentions_per_post: 2

plugins/chat/app/jobs/regular/chat_notify_mentioned.rb

@@ -12,21 +12,39 @@ module Jobs
         return
       end
 
+      mention_type = args[:mention_type]&.to_sym
+      return if (
+        mention_type.blank? ||
+        (
+          !Chat::ChatNotifier::STATIC_MENTION_TYPES.include?(mention_type) &&
+          !Group.where("LOWER(name) = ?", mention_type).exists?
+        )
+      )
+
       @creator = @chat_message.user
       @chat_channel = @chat_message.chat_channel
-      @already_notified_user_ids = args[:already_notified_user_ids] || []
+
-      user_ids_to_notify = args[:to_notify_ids_map] || {}
+      user_ids_to_notify = args[:user_ids].to_a
-      user_ids_to_notify.each { |mention_type, ids| process_mentions(ids, mention_type.to_sym) }
+
+      process_mentions(user_ids_to_notify, mention_type)
     end
 
     private
 
     def get_memberships(user_ids)
       query =
-        UserChatChannelMembership.includes(:user).where(
+        UserChatChannelMembership.includes(:user)
-          user_id: (user_ids - @already_notified_user_ids),
+          .where(user_id: user_ids, chat_channel_id: @chat_message.chat_channel_id)
-          chat_channel_id: @chat_message.chat_channel_id,
+          .joins(
-        )
+            <<~SQL
+              LEFT OUTER JOIN chat_mentions cm ON
+                (
+                  cm.user_id = user_chat_channel_memberships.user_id AND
+                  cm.chat_message_id = #{@chat_message.id}
+                )
+            SQL
+          )
+          .where('cm.user_id IS NULL')
       query = query.where(following: true) if @chat_channel.public_channel?
       query
     end
@@ -101,7 +119,7 @@ module Jobs
     end
 
     def create_notification!(membership, notification_data)
-      is_read = Chat::ChatNotifier.user_has_seen_message?(membership, @chat_message.id)
+      is_read = membership.has_seen_message?(@chat_message)
 
       notification =
         Notification.create!(
@@ -121,6 +139,12 @@ module Jobs
     def send_notifications(membership, notification_data, os_payload)
       create_notification!(membership, notification_data)
 
+      ChatPublisher.publish_new_mention(
+        membership.user_id,
+        @chat_channel.id,
+        @chat_message.id
+      )
+
       if !membership.desktop_notifications_never? && !membership.muted?
         MessageBus.publish(
           "/chat/notification-alert/#{membership.user_id}",
@@ -137,7 +161,10 @@ module Jobs
     def process_mentions(user_ids, mention_type)
       memberships = get_memberships(user_ids)
 
+      screener = UserCommScreener.new(acting_user: @chat_message.user, target_user_ids: memberships.map(&:user_id))
+
       memberships.each do |membership|
+        next if screener.ignoring_or_muting_actor?(membership.user_id)
         notification_data = build_data_for(membership, identifier_type: mention_type)
         payload = build_payload_for(membership, identifier_type: mention_type)
 

plugins/chat/app/jobs/regular/chat_notify_watching.rb

@@ -13,14 +13,22 @@ module Jobs
 
       always_notification_level = UserChatChannelMembership::NOTIFICATION_LEVELS[:always]
 
+      direct_mentioned_user_ids = args[:direct_mentioned_user_ids].to_a
+      global_mentions = args[:global_mentions].to_a
+      mentioned_group_ids = args[:mentioned_group_ids].to_a
+
       members =
         UserChatChannelMembership
-          .includes(user: :groups)
+          .includes(:user)
           .joins(user: :user_option)
           .where(user_option: { chat_enabled: true })
-          .where.not(user_id: args[:except_user_ids])
           .where(chat_channel_id: @chat_channel.id)
-          .where(following: true)
+          .where(following: true, muted: false)
+          .where(
+            "COALESCE(user_chat_channel_memberships.last_read_message_id, 0) < ?",
+            @chat_message.id
+          )
+          .where.not(user_id: direct_mentioned_user_ids)
           .where(
             "desktop_notification_level = ? OR mobile_notification_level = ?",
             always_notification_level,
@@ -28,6 +36,19 @@ module Jobs
           )
           .merge(User.not_suspended)
 
+      if mentioned_group_ids.present?
+        members = members
+          .joins("LEFT OUTER JOIN group_users gu ON gu.user_id = users.id")
+          .group("user_chat_channel_memberships.id")
+          .having("COUNT(gu.group_id) = 0 OR bool_and(gu.group_id NOT IN (?))", mentioned_group_ids)
+      end
+
+      if global_mentions.include?(Chat::ChatNotifier::ALL_KEYWORD)
+        members = members.where(user_option: { ignore_channel_wide_mention: true })
+      elsif global_mentions.include?(Chat::ChatNotifier::HERE_KEYWORD)
+        members = members.where("last_seen_at < ?", 5.minutes.ago)
+      end
+
       if @is_direct_message_channel
         UserCommScreener
           .new(acting_user: @creator, target_user_ids: members.map(&:user_id))
@@ -44,9 +65,22 @@ module Jobs
       user = membership.user
       guardian = Guardian.new(user)
       return unless guardian.can_chat? && guardian.can_join_chat_channel?(@chat_channel)
-      return if Chat::ChatNotifier.user_has_seen_message?(membership, @chat_message.id)
       return if online_user_ids.include?(user.id)
 
+      payload = build_watching_payload(user)
+
+      if membership.desktop_notifications_always?
+        MessageBus.publish("/chat/notification-alert/#{user.id}", payload, user_ids: [user.id])
+      end
+
+      PostAlerter.push_notification(user, payload) if membership.mobile_notifications_always?
+    end
+
+    def online_user_ids
+      @online_user_ids ||= PresenceChannel.new("/chat/online").user_ids
+    end
+
+    def build_watching_payload(user)
       translation_key =
         (
           if @is_direct_message_channel
@@ -59,7 +93,7 @@ module Jobs
       translation_args = { username: @creator.username }
       translation_args[:channel] = @chat_channel.title(user) unless @is_direct_message_channel
 
-      payload = {
+      {
         username: @creator.username,
         notification_type: Notification.types[:chat_message],
         post_url: @chat_channel.relative_url,
@@ -67,18 +101,6 @@ module Jobs
         tag: Chat::ChatNotifier.push_notification_tag(:message, @chat_channel.id),
         excerpt: @chat_message.push_notification_excerpt,
       }
-
-      if membership.desktop_notifications_always? && !membership.muted?
-        MessageBus.publish("/chat/notification-alert/#{user.id}", payload, user_ids: [user.id])
-      end
-
-      if membership.mobile_notifications_always? && !membership.muted?
-        PostAlerter.push_notification(user, payload)
-      end
-    end
-
-    def online_user_ids
-      @online_user_ids ||= PresenceChannel.new("/chat/online").user_ids
     end
   end
 end

plugins/chat/app/jobs/regular/send_message_notifications.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module Jobs
+  class SendMessageNotifications < ::Jobs::Base
+    def execute(args)
+      reason = args[:reason]
+      return if (timestamp = args[:timestamp]).blank?
+
+      return if (message = ChatMessage.find_by(id: args[:chat_message_id])).nil?
+
+      if reason == "new"
+        Chat::ChatNotifier.new(message, timestamp).notify_new
+      elsif reason == "edit"
+        Chat::ChatNotifier.new(message, timestamp).notify_edit
+      end
+    end
+  end
+end

plugins/chat/app/models/chat_mention.rb

@@ -3,7 +3,7 @@
 class ChatMention < ActiveRecord::Base
   belongs_to :user
   belongs_to :chat_message
-  belongs_to :notification
+  belongs_to :notification, dependent: :destroy
 end
 
 # == Schema Information

plugins/chat/app/models/user_chat_channel_membership.rb

@@ -13,6 +13,10 @@ class UserChatChannelMembership < ActiveRecord::Base
 
   attribute :unread_count, default: 0
   attribute :unread_mentions, default: 0
+
+  def has_seen_message?(chat_message)
+    (last_read_message_id || 0) >= chat_message.id
+  end
 end
 
 # == Schema Information

plugins/chat/lib/chat_notifier.rb

@@ -27,21 +27,35 @@
 # The ignore/mute filtering is also applied via the ChatNotifyWatching job,
 # which prevents desktop / push notifications being sent.
 class Chat::ChatNotifier
-  class << self
+  DIRECT_MENTIONS = :direct_mentions
-    def user_has_seen_message?(membership, chat_message_id)
+  HERE_MENTIONS = :here_mentions
-      (membership.last_read_message_id || 0) >= chat_message_id
+  GLOBAL_MENTIONS = :global_mentions
-    end
+  STATIC_MENTION_TYPES = [DIRECT_MENTIONS, HERE_MENTIONS, GLOBAL_MENTIONS]
+  HERE_KEYWORD = 'here'
+  ALL_KEYWORD = 'all'
 
+  MENTION_BATCH_SIZE = 250
+  class << self
     def push_notification_tag(type, chat_channel_id)
       "#{Discourse.current_hostname}-chat-#{type}-#{chat_channel_id}"
     end
 
     def notify_edit(chat_message:, timestamp:)
-      new(chat_message, timestamp).notify_edit
+      Jobs.enqueue(
+        :send_message_notifications,
+        chat_message_id: chat_message.id,
+        timestamp: timestamp.iso8601(6),
+        reason: "edit"
+      )
     end
 
     def notify_new(chat_message:, timestamp:)
-      new(chat_message, timestamp).notify_new
+      Jobs.enqueue(
+        :send_message_notifications,
+        chat_message_id: chat_message.id,
+        timestamp: timestamp.iso8601(6),
+        reason: "new"
+      )
     end
   end
 
@@ -55,49 +69,41 @@ class Chat::ChatNotifier
   ### Public API
 
   def notify_new
-    to_notify = list_users_to_notify
+    if (inaccessible_mentions = expand_mentions_and_notify)
-    mentioned_user_ids = to_notify.extract!(:all_mentioned_user_ids)[:all_mentioned_user_ids]
+      notify_creator_of_inaccessible_mentions(inaccessible_mentions)
-
-    mentioned_user_ids.each do |member_id|
-      ChatPublisher.publish_new_mention(member_id, @chat_channel.id, @chat_message.id)
     end
 
-    notify_creator_of_inaccessible_mentions(to_notify)
+    global_mentions = []
+    global_mentions << ALL_KEYWORD if typed_global_mention?
+    global_mentions << HERE_KEYWORD if typed_here_mention?
 
-    notify_mentioned_users(to_notify)
+    notify_watching_users(
-    notify_watching_users(except: mentioned_user_ids << @user.id)
+      mentioned_channel_member_ids,
-
+      global_mentions,
-    to_notify
+      mentionable_groups.map(&:id)
+    )
   end
 
   def notify_edit
-    existing_notifications =
+    purge_outdated_mentions
-      ChatMention.includes(:user, :notification).where(chat_message: @chat_message)
-    already_notified_user_ids = existing_notifications.map(&:user_id)
 
-    to_notify = list_users_to_notify
+    if (inaccessible_mentions = expand_mentions_and_notify)
-    mentioned_user_ids = to_notify.extract!(:all_mentioned_user_ids)[:all_mentioned_user_ids]
+      notify_creator_of_inaccessible_mentions(inaccessible_mentions)
-
-    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!
-      chat_mention.destroy!
     end
-
-    needs_notification_ids = mentioned_user_ids - already_notified_user_ids
-    return if needs_notification_ids.blank?
-
-    notify_creator_of_inaccessible_mentions(to_notify)
-
-    notify_mentioned_users(to_notify, already_notified_user_ids: already_notified_user_ids)
-
-    to_notify
   end
 
   private
 
-  def list_users_to_notify
+  def purge_outdated_mentions
+    ChatMention
+      .joins(user: :groups)
+      .where(chat_message: @chat_message)
+      .where.not(user_id: mentioned_channel_member_ids)
+      .where.not(groups: { id: mentionable_groups.map(&:id) })
+      .destroy_all
+  end
+
+  def expand_mentions_and_notify
     direct_mentions_count = direct_mentions_from_cooked.length
     group_mentions_count = group_name_mentions.length
 
@@ -105,89 +111,90 @@ class Chat::ChatNotifier
       (direct_mentions_count + group_mentions_count) >
         SiteSetting.max_mentions_per_chat_message
 
-    {}.tap do |to_notify|
+    inaccessible_mentions = {
-      # The order of these methods is the precedence
+      welcome_to_join: [],
-      # between different mention types.
+      unreachable: [],
+      group_mentions_disabled: [],
+      too_many_members: []
+    }
 
-      already_covered_ids = []
+    return inaccessible_mentions if skip_notifications
 
-      expand_direct_mentions(to_notify, already_covered_ids, skip_notifications)
+    send_direct_mentions(inaccessible_mentions)
-      expand_group_mentions(to_notify, already_covered_ids, skip_notifications)
+    send_group_mentions(inaccessible_mentions)
-      expand_here_mention(to_notify, already_covered_ids, skip_notifications)
+    filter_invites_ignoring_or_muting_creator(inaccessible_mentions)
-      expand_global_mention(to_notify, already_covered_ids, skip_notifications)
 
-      filter_users_ignoring_or_muting_creator(to_notify, already_covered_ids)
+    if @chat_channel.allow_channel_wide_mentions?
+      send_here_mentions if typed_here_mention?
+      send_global_mentions if typed_global_mention?
+    end
 
-      to_notify[:all_mentioned_user_ids] = already_covered_ids
+    inaccessible_mentions
+  end
+
+  def send_direct_mentions(inaccessible_mentions)
+    direct_mentions = chat_users
+      .includes(:user_chat_channel_memberships, :group_users)
+      .where(username_lower: usernames_mentioned)
+
+    grouped = group_users_to_notify(direct_mentions)
+    inaccessible_mentions[:welcome_to_join] = grouped[:welcome_to_join]
+    inaccessible_mentions[:unreachable] = grouped[:unreachable]
+
+    notify_mentioned_users(DIRECT_MENTIONS, grouped[:already_participating].map(&:id))
+  end
+
+  def send_group_mentions(inaccessible_mentions)
+    return if visible_groups.empty?
+
+    mentions_disabled = visible_groups - mentionable_groups
+
+    too_many_members, mentionable = mentionable_groups.partition do |group|
+      group.user_count > SiteSetting.max_users_notified_per_group_mention
+    end
+
+    inaccessible_mentions[:group_mentions_disabled] = mentions_disabled
+    inaccessible_mentions[:too_many_members] = too_many_members
+
+    return if mentionable.blank?
+
+    mentioned_by_group(mentionable).find_in_batches(batch_size: MENTION_BATCH_SIZE) do |reached_by_group|
+      grouped = group_users_to_notify(reached_by_group)
+      ordered_group_names = group_name_mentions & mentionable.map { |mg| mg.name.downcase }
+
+      classified = grouped[:already_participating].reduce({}) do |memo, member|
+        first_mentioned_group = ordered_group_names.detect { |gn| member.mentioned_group_names.include?(gn) }
+
+        memo[first_mentioned_group] = memo[first_mentioned_group].to_a << member.id
+
+        memo
+      end
+
+      classified.each do |group_name, member_ids|
+        notify_mentioned_users(group_name, member_ids)
+      end
     end
   end
 
-  def chat_users
+  def send_here_mentions
-    users =
+    channel_wide_mentions
-      User.includes(:do_not_disturb_timings, :push_subscriptions, :user_chat_channel_memberships)
+      .where("last_seen_at > ?", 5.minutes.ago)
-
+      .select(:id)
-    users
+      .find_in_batches(batch_size: MENTION_BATCH_SIZE) do |here_users|
-      .distinct
+        notify_mentioned_users(HERE_MENTIONS, here_users.map(&:id))
-      .joins("LEFT OUTER JOIN user_chat_channel_memberships uccm ON uccm.user_id = users.id")
+      end
-      .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
+  def send_global_mentions
-    chat_users.where(
+    global_mentions = channel_wide_mentions
-      user_chat_channel_memberships: {
-        following: true,
-        chat_channel_id: @chat_channel.id,
-      },
-    )
-  end
 
-  def members_accepting_channel_wide_notifications
+    if typed_here_mention?
-    rest_of_the_channel.where(user_options: { ignore_channel_wide_mention: [false, nil] })
+      global_mentions = global_mentions
-  end
+        .where("last_seen_at < ?", 5.minutes.ago)
-
-  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)
+    global_mentions.select(:id).find_in_batches(batch_size: MENTION_BATCH_SIZE) do |user_ids|
-    typed_global_mention = direct_mentions_from_cooked.include?("@all")
+      notify_mentioned_users(GLOBAL_MENTIONS, user_ids.map(&:id))
-
-    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))
-        .where.not(id: already_covered_ids)
-        .pluck(:id)
-
-      already_covered_ids.concat(to_notify[:global_mentions])
-    else
-      to_notify[:global_mentions] = []
-    end
-  end
-
-  def expand_here_mention(to_notify, already_covered_ids, skip)
-    typed_here_mention = direct_mentions_from_cooked.include?("@here")
-
-    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))
-        .where.not(id: already_covered_ids)
-        .pluck(:id)
-
-      already_covered_ids.concat(to_notify[:here_mentions])
-    else
-      to_notify[:here_mentions] = []
     end
   end
 
@@ -208,30 +215,147 @@ class Chat::ChatNotifier
       end
 
     {
-      already_participating: participants || [],
+      already_participating: participants.to_a,
-      welcome_to_join: welcome_to_join || [],
+      welcome_to_join: welcome_to_join.to_a,
-      unreachable: unreachable || [],
+      unreachable: unreachable.to_a,
     }
   end
 
-  def expand_direct_mentions(to_notify, already_covered_ids, skip)
+  def notify_creator_of_inaccessible_mentions(inaccessible_mentions)
-    if skip
+    return if inaccessible_mentions.values.all?(&:blank?)
-      direct_mentions = []
-    else
-      direct_mentions =
-        chat_users
-          .where(username_lower: normalized_mentions(direct_mentions_from_cooked))
-          .where.not(id: already_covered_ids)
-    end
 
-    grouped = group_users_to_notify(direct_mentions)
+    ChatPublisher.publish_inaccessible_mentions(
-
+      @user.id,
-    to_notify[:direct_mentions] = grouped[:already_participating].map(&:id)
+      @chat_message,
-    to_notify[:welcome_to_join] = grouped[:welcome_to_join]
+      inaccessible_mentions[:unreachable],
-    to_notify[:unreachable] = grouped[:unreachable]
+      inaccessible_mentions[:welcome_to_join],
-    already_covered_ids.concat(to_notify[:direct_mentions])
+      inaccessible_mentions[:too_many_members],
+      inaccessible_mentions[:group_mentions_disabled]
+    )
   end
 
+  # Filters out users from global, here, group, and direct mentions that are
+  # ignoring or muting the creator of the message, so they will not receive
+  # a notification via the ChatNotifyMentioned job and are not prompted for
+  # invitation by the creator.
+  def filter_invites_ignoring_or_muting_creator(inaccessible_mentions)
+    screen_targets = inaccessible_mentions[:welcome_to_join].map(&:id)
+
+    return if screen_targets.blank?
+
+    screener = UserCommScreener.new(acting_user: @user, target_user_ids: screen_targets)
+
+    # :welcome_to_join contains users because it's serialized by MB.
+    inaccessible_mentions[:welcome_to_join] = inaccessible_mentions[:welcome_to_join].reject do |user|
+      screener.ignoring_or_muting_actor?(user.id)
+    end
+  end
+
+  # Query helpers
+
+  def mentioned_by_group(mentionable_groups)
+    chat_users
+      .includes(:user_chat_channel_memberships, :group_users)
+      .where.not(id: mentioned_channel_member_ids)
+      .joins(:groups)
+      .where(groups: { id: mentionable_groups.map(&:id) })
+      .group('users.id')
+      .select("users.*", "ARRAY_AGG(LOWER(groups.name)) AS mentioned_group_names")
+  end
+
+  def mentioned_channel_member_ids
+    @mentioned_channel_member_ids ||= begin
+      where_params = { chat_channel_id: @chat_channel.id }
+      where_params[:following] = true if @chat_channel.public_channel?
+
+      chat_users.where(uccm: where_params).where(username_lower: usernames_mentioned).pluck(:id)
+    end
+  end
+
+  def visible_groups
+    @visible_groups ||=
+      Group
+        .where("LOWER(name) IN (?)", group_name_mentions)
+        .visible_groups(@user)
+  end
+
+  def mentionable_groups
+    @mentioned_groups ||= Group
+      .mentionable(@user, include_public: false)
+      .where(id: visible_groups.map(&:id))
+  end
+
+  def channel_wide_mentions
+    query = members_accepting_channel_wide_notifications
+      .where.not(id: mentioned_channel_member_ids)
+
+    return query if mentionable_groups.blank?
+
+    query
+      .distinct
+      .joins(:group_users)
+      .group("users.id")
+      .having("
+        bool_and(group_users.group_id NOT IN (?))",
+        mentionable_groups.map(&:id)
+      )
+  end
+
+  def chat_users
+    User
+      .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 channel_members
+    chat_users.where(
+      uccm: {
+        following: true,
+        chat_channel_id: @chat_channel.id,
+      },
+    )
+  end
+
+  def members_accepting_channel_wide_notifications
+    channel_members.where(user_options: { ignore_channel_wide_mention: [false, nil] })
+  end
+
+  # Jobs to create notifications
+
+  def notify_mentioned_users(mention_type, user_ids)
+    return if user_ids.blank? || mention_type.blank?
+
+    Jobs.enqueue(
+      :chat_notify_mentioned,
+      {
+        chat_message_id: @chat_message.id,
+        user_ids: user_ids,
+        mention_type: mention_type,
+        timestamp: @timestamp,
+      },
+    )
+  end
+
+  def notify_watching_users(direct_mentioned_user_ids, global_mentions, mentioned_group_ids)
+    Jobs.enqueue(
+      :chat_notify_watching,
+      {
+        chat_message_id: @chat_message.id,
+        timestamp: @timestamp,
+        direct_mentioned_user_ids: direct_mentioned_user_ids,
+        global_mentions: global_mentions,
+        mentioned_group_ids: mentioned_group_ids
+      },
+    )
+  end
+
+  # Helper methods for capturing mentions
+
   def group_name_mentions
     @group_mentions_from_cooked ||=
       normalized_mentions(
@@ -239,113 +363,27 @@ class Chat::ChatNotifier
       )
   end
 
-  def visible_groups
+  def direct_mentions_from_cooked
-    @visible_groups ||=
+    @direct_mentions_from_cooked ||=
-        Group
+      Nokogiri::HTML5.fragment(@chat_message.cooked)
-          .where("LOWER(name) IN (?)", group_name_mentions)
+        .css(".mention").map { |node| node.text.downcase }
-          .visible_groups(@user)
   end
 
-  def expand_group_mentions(to_notify, already_covered_ids, skip)
+  def usernames_mentioned
-    return [] if skip || visible_groups.empty?
+    @usernames_mentioned ||= normalized_mentions(direct_mentions_from_cooked)
-
-    mentionable_groups = Group
-      .mentionable(@user, include_public: false)
-      .where(id: visible_groups.map(&:id))
-
-    mentions_disabled = visible_groups - mentionable_groups
-
-    too_many_members, mentionable = mentionable_groups.partition do |group|
-      group.user_count > SiteSetting.max_users_notified_per_group_mention
-    end
-
-    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.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 }
-      user_group_names = user.groups.map { |ug| ug.name.downcase }
-      group_name = ordered_group_names.detect { |gn| user_group_names.include?(gn) }
-
-      to_notify[group_name] << user.id
-      already_covered_ids << user.id
-    end
-
-    to_notify[:welcome_to_join] = to_notify[:welcome_to_join].concat(grouped[:welcome_to_join])
-    to_notify[:unreachable] = to_notify[:unreachable].concat(grouped[:unreachable])
   end
 
-  def notify_creator_of_inaccessible_mentions(to_notify)
+  def normalized_mentions(mentions)
-    inaccessible = to_notify.extract!(:unreachable, :welcome_to_join, :too_many_members, :group_mentions_disabled)
+    mentions.reduce([]) do |memo, mention|
-    return if inaccessible.values.all?(&:blank?)
+      %w[@here @all].include?(mention) ? memo : (memo << mention[1..-1])
-
-    ChatPublisher.publish_inaccessible_mentions(
-      @user.id,
-      @chat_message,
-      inaccessible[:unreachable].to_a,
-      inaccessible[:welcome_to_join].to_a,
-      inaccessible[:too_many_members].to_a,
-      inaccessible[:group_mentions_disabled].to_a
-    )
-  end
-
-  # Filters out users from global, here, group, and direct mentions that are
-  # ignoring or muting the creator of the message, so they will not receive
-  # a notification via the ChatNotifyMentioned job and are not prompted for
-  # invitation by the creator.
-  def filter_users_ignoring_or_muting_creator(to_notify, already_covered_ids)
-    screen_targets = already_covered_ids.concat(to_notify[:welcome_to_join].map(&:id))
-
-    return if screen_targets.blank?
-
-    screener = UserCommScreener.new(acting_user: @user, target_user_ids: screen_targets)
-    to_notify
-      .except(:unreachable, :welcome_to_join)
-      .each do |key, user_ids|
-        to_notify[key] = user_ids.reject do |user_id|
-          screener.ignoring_or_muting_actor?(user_id)
-        end
-      end
-
-    # :welcome_to_join contains users because it's serialized by MB.
-    to_notify[:welcome_to_join] = to_notify[:welcome_to_join].reject do |user|
-      screener.ignoring_or_muting_actor?(user.id)
-    end
-
-    already_covered_ids.reject! do |already_covered|
-      screener.ignoring_or_muting_actor?(already_covered)
     end
   end
 
-  def notify_mentioned_users(to_notify, already_notified_user_ids: [])
+  def typed_global_mention?
-    Jobs.enqueue(
+    direct_mentions_from_cooked.include?("@all")
-      :chat_notify_mentioned,
-      {
-        chat_message_id: @chat_message.id,
-        to_notify_ids_map: to_notify.as_json,
-        already_notified_user_ids: already_notified_user_ids,
-        timestamp: @timestamp.iso8601(6),
-      },
-    )
   end
 
-  def notify_watching_users(except: [])
+  def typed_here_mention?
-    Jobs.enqueue(
+    direct_mentions_from_cooked.include?("@here")
-      :chat_notify_watching,
-      {
-        chat_message_id: @chat_message.id,
-        except_user_ids: except,
-        timestamp: @timestamp.iso8601(6),
-      },
-    )
   end
 end

plugins/chat/plugin.rb

@@ -198,6 +198,7 @@ after_initialize do
   load File.expand_path("../app/jobs/regular/chat_notify_watching.rb", __FILE__)
   load File.expand_path("../app/jobs/regular/update_channel_user_count.rb", __FILE__)
   load File.expand_path("../app/jobs/regular/delete_user_messages.rb", __FILE__)
+  load File.expand_path("../app/jobs/regular/send_message_notifications.rb", __FILE__)
   load File.expand_path("../app/jobs/scheduled/delete_old_chat_messages.rb", __FILE__)
   load File.expand_path("../app/jobs/scheduled/update_user_counts_for_chat_channels.rb", __FILE__)
   load File.expand_path("../app/jobs/scheduled/email_chat_notifications.rb", __FILE__)

plugins/chat/spec/jobs/regular/chat_notify_mentioned_spec.rb

@@ -7,6 +7,8 @@ describe Jobs::ChatNotifyMentioned do
   fab!(:user_2) { Fabricate(:user) }
   fab!(:public_channel) { Fabricate(:category_channel) }
 
+  let(:user_ids) { [user_2.id] }
+
   before do
     Group.refresh_automatic_groups!
     user_1.reload
@@ -28,33 +30,34 @@ describe Jobs::ChatNotifyMentioned do
   def track_desktop_notification(
     user: user_2,
     message:,
-    to_notify_ids_map:,
+    user_ids:,
-    already_notified_user_ids: []
+    mention_type:
   )
     MessageBus
       .track_publish("/chat/notification-alert/#{user.id}") do
         subject.execute(
           chat_message_id: message.id,
           timestamp: message.created_at,
-          to_notify_ids_map: to_notify_ids_map,
+          user_ids: user_ids,
-          already_notified_user_ids: already_notified_user_ids,
+          mention_type: mention_type
         )
       end
       .first
   end
 
-  def track_core_notification(user: user_2, message:, to_notify_ids_map:)
+  def track_core_notification(user: user_2, message:, user_ids:, mention_type:)
     subject.execute(
       chat_message_id: message.id,
       timestamp: message.created_at,
-      to_notify_ids_map: to_notify_ids_map,
+      user_ids: user_ids,
+      mention_type: mention_type
     )
 
     Notification.where(user: user, notification_type: Notification.types[:chat_mention]).last
   end
 
   describe "scenarios where we should skip sending notifications" do
-    let(:to_notify_ids_map) { { here_mentions: [user_2.id] } }
+    let(:mention_type) { Chat::ChatNotifier::HERE_MENTIONS }
 
     it "does nothing if there is a newer version of the message" do
       message = create_chat_message
@@ -63,7 +66,7 @@ describe Jobs::ChatNotifyMentioned do
       PostAlerter.expects(:push_notification).never
 
       desktop_notification =
-        track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+        track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
       expect(desktop_notification).to be_nil
 
       created_notification =
@@ -81,7 +84,7 @@ describe Jobs::ChatNotifyMentioned do
       PostAlerter.expects(:push_notification).never
 
       desktop_notification =
-        track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+        track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
       expect(desktop_notification).to be_nil
 
       created_notification =
@@ -97,7 +100,7 @@ describe Jobs::ChatNotifyMentioned do
       PostAlerter.expects(:push_notification).never
 
       desktop_notification =
-        track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+        track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
       expect(desktop_notification).to be_nil
 
       created_notification =
@@ -105,17 +108,14 @@ describe Jobs::ChatNotifyMentioned do
       expect(created_notification).to be_nil
     end
 
-    it "does nothing if user is included in the already_notified_user_ids" do
+    it "does nothing if we already created a mention for the user" do
       message = create_chat_message
+      Fabricate(:chat_mention, chat_message: message, user: user_2)
 
       PostAlerter.expects(:push_notification).never
 
       desktop_notification =
-        track_desktop_notification(
+        track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
-          message: message,
-          to_notify_ids_map: to_notify_ids_map,
-          already_notified_user_ids: [user_2.id],
-        )
       expect(desktop_notification).to be_nil
 
       created_notification =
@@ -123,17 +123,31 @@ describe Jobs::ChatNotifyMentioned do
       expect(created_notification).to be_nil
     end
 
+    it "works if the mention belongs to a different message" do
+      message_1 = create_chat_message
+      message_2 = create_chat_message
+
+      Fabricate(:chat_mention, chat_message: message_1, user: user_2)
+
+      PostAlerter.expects(:push_notification).once
+
+      desktop_notification =
+        track_desktop_notification(message: message_2, user_ids: user_ids, mention_type: mention_type)
+
+      expect(desktop_notification).to be_present
+    end
+
     it "does nothing if user is not participating in a private channel" do
       user_3 = Fabricate(:user)
       @chat_group.add(user_3)
-      to_notify_map = { direct_mentions: [user_3.id] }
-
       message = create_chat_message(channel: @personal_chat_channel)
 
       PostAlerter.expects(:push_notification).never
 
       desktop_notification =
-        track_desktop_notification(message: message, to_notify_ids_map: to_notify_map)
+        track_desktop_notification(
+          message: message, user_ids: [user_3.id], mention_type: Chat::ChatNotifier::HERE_MENTIONS
+        )
       expect(desktop_notification).to be_nil
 
       created_notification =
@@ -148,7 +162,7 @@ describe Jobs::ChatNotifyMentioned do
       )
 
       desktop_notification =
-        track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+        track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
       expect(desktop_notification).to be_nil
     end
@@ -164,7 +178,8 @@ describe Jobs::ChatNotifyMentioned do
       subject.execute(
         chat_message_id: message.id,
         timestamp: message.created_at,
-        to_notify_ids_map: to_notify_ids_map,
+        user_ids: user_ids,
+        mention_type: mention_type
       )
     end
 
@@ -176,7 +191,7 @@ describe Jobs::ChatNotifyMentioned do
       )
 
       desktop_notification =
-        track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+        track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
       expect(desktop_notification).to be_nil
     end
@@ -193,9 +208,56 @@ describe Jobs::ChatNotifyMentioned do
       subject.execute(
         chat_message_id: message.id,
         timestamp: message.created_at,
-        to_notify_ids_map: to_notify_ids_map,
+        user_ids: user_ids,
+        mention_type: mention_type
       )
     end
+
+    it "does nothing when the mention type is invalid" do
+      message = create_chat_message
+
+      PostAlerter.expects(:push_notification).never
+
+      desktop_notification =
+        track_desktop_notification(message: message, user_ids: user_ids, mention_type: "invalid")
+      expect(desktop_notification).to be_nil
+
+      created_notification =
+        Notification.where(user: user_2, notification_type: Notification.types[:chat_mention]).last
+      expect(created_notification).to be_nil
+    end
+
+    context "when the user is muting the message sender" 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)
+        message = create_chat_message
+
+        PostAlerter.expects(:push_notification).never
+
+        desktop_notification =
+          track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
+        expect(desktop_notification).to be_nil
+
+        created_notification =
+          Notification.where(user: user_2, notification_type: Notification.types[:chat_mention]).last
+        expect(created_notification).to be_nil
+      end
+
+      it "does not send notifications to the user who is ignoring the acting user" do
+        Fabricate(:ignored_user, user: user_2, ignored_user: user_1, expiring_at: 1.day.from_now)
+        message = create_chat_message
+
+        PostAlerter.expects(:push_notification).never
+
+        desktop_notification =
+          track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
+        expect(desktop_notification).to be_nil
+
+        created_notification =
+          Notification.where(user: user_2, notification_type: Notification.types[:chat_mention]).last
+        expect(created_notification).to be_nil
+      end
+    end
   end
 
   shared_examples "creates different notifications with basic data" do
@@ -205,7 +267,7 @@ describe Jobs::ChatNotifyMentioned do
       message = create_chat_message
 
       desktop_notification =
-        track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+        track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
       expect(desktop_notification).to be_present
       expect(desktop_notification.data[:notification_type]).to eq(Notification.types[:chat_mention])
@@ -238,7 +300,8 @@ describe Jobs::ChatNotifyMentioned do
       subject.execute(
         chat_message_id: message.id,
         timestamp: message.created_at,
-        to_notify_ids_map: to_notify_ids_map,
+        user_ids: user_ids,
+        mention_type: mention_type
       )
     end
 
@@ -246,7 +309,7 @@ describe Jobs::ChatNotifyMentioned do
       message = create_chat_message
 
       created_notification =
-        track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+        track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
       expect(created_notification).to be_present
       expect(created_notification.high_priority).to eq(true)
@@ -265,11 +328,29 @@ describe Jobs::ChatNotifyMentioned do
         ChatMention.where(notification: created_notification, user: user_2, chat_message: message)
       expect(chat_mention).to be_present
     end
+
+    it "works for publishing new mention updates" do
+      message = create_chat_message
+
+      new_mention = MessageBus
+        .track_publish(ChatPublisher.new_mentions_message_bus_channel(message.chat_channel_id)) do
+          subject.execute(
+            chat_message_id: message.id,
+            timestamp: message.created_at,
+            user_ids: user_ids,
+            mention_type: mention_type
+          )
+        end.first
+
+      expect(new_mention).to be_present
+      expect(new_mention.data["message_id"]).to eq(message.id)
+      expect(new_mention.data["channel_id"]).to eq(message.chat_channel_id)
+    end
   end
 
   describe "#execute" do
     describe "global mention notifications" do
-      let(:to_notify_ids_map) { { global_mentions: [user_2.id] } }
+      let(:mention_type) { Chat::ChatNotifier::GLOBAL_MENTIONS }
 
       let(:payload_translated_title) do
         I18n.t(
@@ -286,7 +367,7 @@ describe Jobs::ChatNotifyMentioned do
         message = create_chat_message
 
         created_notification =
-          track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
         data_hash = created_notification.data_hash
 
@@ -297,7 +378,7 @@ describe Jobs::ChatNotifyMentioned do
         message = create_chat_message
 
         desktop_notification =
-          track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
         expect(desktop_notification.data[:translated_title]).to eq(payload_translated_title)
       end
@@ -307,7 +388,7 @@ describe Jobs::ChatNotifyMentioned do
           message = create_chat_message(channel: @personal_chat_channel)
 
           desktop_notification =
-            track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+            track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
           expected_title =
             I18n.t(
@@ -322,7 +403,7 @@ describe Jobs::ChatNotifyMentioned do
     end
 
     describe "here mention notifications" do
-      let(:to_notify_ids_map) { { here_mentions: [user_2.id] } }
+      let(:mention_type) { Chat::ChatNotifier::HERE_MENTIONS }
 
       let(:payload_translated_title) do
         I18n.t(
@@ -339,7 +420,7 @@ describe Jobs::ChatNotifyMentioned do
         message = create_chat_message
 
         created_notification =
-          track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
         data_hash = created_notification.data_hash
 
         expect(data_hash[:identifier]).to eq("here")
@@ -349,7 +430,7 @@ describe Jobs::ChatNotifyMentioned do
         message = create_chat_message
 
         desktop_notification =
-          track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
         expect(desktop_notification.data[:translated_title]).to eq(payload_translated_title)
       end
@@ -359,7 +440,7 @@ describe Jobs::ChatNotifyMentioned do
           message = create_chat_message(channel: @personal_chat_channel)
 
           desktop_notification =
-            track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+            track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
           expected_title =
             I18n.t(
@@ -374,7 +455,7 @@ describe Jobs::ChatNotifyMentioned do
     end
 
     describe "direct mention notifications" do
-      let(:to_notify_ids_map) { { direct_mentions: [user_2.id] } }
+      let(:mention_type) { Chat::ChatNotifier::DIRECT_MENTIONS }
 
       let(:payload_translated_title) do
         I18n.t(
@@ -391,7 +472,7 @@ describe Jobs::ChatNotifyMentioned do
         message = create_chat_message
 
         created_notification =
-          track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
         data_hash = created_notification.data_hash
 
         expect(data_hash[:identifier]).to be_nil
@@ -401,7 +482,7 @@ describe Jobs::ChatNotifyMentioned do
         message = create_chat_message
 
         desktop_notification =
-          track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
         expect(desktop_notification.data[:translated_title]).to eq(payload_translated_title)
       end
@@ -411,7 +492,7 @@ describe Jobs::ChatNotifyMentioned do
           message = create_chat_message(channel: @personal_chat_channel)
 
           desktop_notification =
-            track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+            track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
           expected_title =
             I18n.t(
@@ -426,7 +507,7 @@ describe Jobs::ChatNotifyMentioned do
     end
 
     describe "group mentions" do
-      let(:to_notify_ids_map) { { @chat_group.name.to_sym => [user_2.id] } }
+      let(:mention_type) { @chat_group.name.to_sym }
 
       let(:payload_translated_title) do
         I18n.t(
@@ -443,7 +524,7 @@ describe Jobs::ChatNotifyMentioned do
         message = create_chat_message
 
         created_notification =
-          track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
         data_hash = created_notification.data_hash
 
         expect(data_hash[:identifier]).to eq(@chat_group.name)
@@ -454,7 +535,7 @@ describe Jobs::ChatNotifyMentioned do
         message = create_chat_message
 
         desktop_notification =
-          track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          track_desktop_notification(message: message,  user_ids: user_ids, mention_type: mention_type)
 
         expect(desktop_notification.data[:translated_title]).to eq(payload_translated_title)
       end
@@ -464,7 +545,7 @@ describe Jobs::ChatNotifyMentioned do
           message = create_chat_message(channel: @personal_chat_channel)
 
           desktop_notification =
-            track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+            track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
 
           expected_title =
             I18n.t(

plugins/chat/spec/jobs/regular/chat_notify_watching_spec.rb

@@ -5,21 +5,158 @@ RSpec.describe Jobs::ChatNotifyWatching do
   fab!(:user2) { Fabricate(:user) }
   fab!(:user3) { Fabricate(:user) }
   fab!(:group) { Fabricate(:group) }
-  let(:except_user_ids) { [] }
 
   before do
     SiteSetting.chat_enabled = true
     SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
   end
 
-  def run_job
+  def listen_for_notifications(user, direct_mentioned_user_ids: [], global_mentions: [], mentioned_group_ids: [])
-    described_class.new.execute(chat_message_id: message.id, except_user_ids: except_user_ids)
+    MessageBus.track_publish("/chat/notification-alert/#{user.id}") do
+      subject.execute(
+        chat_message_id: message.id,
+        direct_mentioned_user_ids: direct_mentioned_user_ids,
+        global_mentions: global_mentions,
+        mentioned_group_ids: mentioned_group_ids
+      )
+    end
   end
 
-  def notification_messages_for(user)
+  def build_notification_translation(channel)
-    MessageBus
+    if channel.direct_message_channel?
-      .track_publish { run_job }
+      "discourse_push_notifications.popup.new_direct_chat_message"
-      .filter { |m| m.channel == "/chat/notification-alert/#{user.id}" }
+    else
+      "discourse_push_notifications.popup.new_chat_message"
+    end
+  end
+
+  def expects_push_notification(sender, receiver, message)
+    PostAlerter.expects(:push_notification).with(
+      receiver,
+      has_entries(
+        {
+          username: sender.username,
+          notification_type: Notification.types[:chat_message],
+          post_url: message.chat_channel.relative_url,
+          translated_title:
+            I18n.t(
+              build_notification_translation(message.chat_channel),
+              { username: sender.username, channel: message.chat_channel.title(receiver) },
+            ),
+          tag: Chat::ChatNotifier.push_notification_tag(:message, message.chat_channel.id),
+          excerpt: message.message,
+        },
+      ),
+    )
+  end
+
+  def assert_notification_alert_is_correct(alert_data, sender, receiver, message)
+    expect(alert_data).to include(
+      {
+        username: sender.username,
+        notification_type: Notification.types[:chat_message],
+        post_url: message.chat_channel.relative_url,
+        translated_title:
+          I18n.t(
+            build_notification_translation(message.chat_channel),
+            { username: sender.username, channel: channel.title(receiver) },
+          ),
+        tag: Chat::ChatNotifier.push_notification_tag(:message, message.chat_channel.id),
+        excerpt: message.message,
+      },
+    )
+  end
+
+  context "when the chat message has mentions" do
+    fab!(:channel) { Fabricate(:category_channel) }
+    fab!(:membership) do
+      Fabricate(:user_chat_channel_membership, user: user2, chat_channel: channel)
+    end
+    fab!(:message) do
+      Fabricate(:chat_message, chat_channel: channel, user: user1, message: "this is a new message")
+    end
+
+    before do
+      membership.update!(
+        desktop_notification_level: UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
+        mobile_notification_level: UserChatChannelMembership::NOTIFICATION_LEVELS[:always],
+      )
+    end
+
+    it "skips the watching notifications if it was mentioned directly" do
+      PostAlerter.expects(:push_notification).never
+
+      messages = listen_for_notifications(user2, direct_mentioned_user_ids: [user2.id])
+
+      expect(messages.size).to be_zero
+    end
+
+    it "skips the watching notifications if it was mentioned through a group" do
+      group.add(user2)
+      PostAlerter.expects(:push_notification).never
+
+      messages = listen_for_notifications(user2, mentioned_group_ids: [group.id])
+
+      expect(messages.size).to be_zero
+    end
+
+    it "doesn't skip watching notifications if the user is not a member of the mentioned group" do
+      PostAlerter.expects(:push_notification).once
+
+      messages = listen_for_notifications(user2, mentioned_group_ids: [group.id])
+
+      expect(messages).to be_present
+    end
+
+    it "skips the watching notifications if it was mentioned via @all mention" do
+      PostAlerter.expects(:push_notification).never
+
+      messages = listen_for_notifications(user2, global_mentions: ["all"])
+
+      expect(messages.size).to be_zero
+    end
+
+    it "doesn't skip the watching notifications on @all and ignoring channel wide mention" do
+      user2.user_option.update!(ignore_channel_wide_mention: true)
+
+      expects_push_notification(user1, user2, message)
+
+      messages = listen_for_notifications(user2, global_mentions: ["all"])
+
+      assert_notification_alert_is_correct(messages.first.data, user1, user2, message)
+    end
+
+    it "skips the watching notifications if it was mentioned via @here mention" do
+      PostAlerter.expects(:push_notification).never
+
+      messages = listen_for_notifications(user2, global_mentions: ["here"])
+
+      expect(messages.size).to be_zero
+    end
+
+    it "doesn't skip the watching notifications on @here if the user last seen is more than 5 minutes ago" do
+      user2.update!(last_seen_at: 6.minutes.ago)
+
+      expects_push_notification(user1, user2, message)
+
+      messages = listen_for_notifications(user2, global_mentions: ["here"])
+
+      assert_notification_alert_is_correct(messages.first.data, user1, user2, message)
+    end
+
+    context "when among the user groups there is a mentioned one" do
+      it 'skips the watching notification' do
+        group.add(user2)
+        another_group = Fabricate(:group)
+        another_group.add(user2)
+
+        PostAlerter.expects(:push_notification).never
+
+        messages = listen_for_notifications(user2, mentioned_group_ids: [group.id])
+
+        expect(messages.size).to be_zero
+      end
+    end
   end
 
   context "for a category channel" do
@@ -44,22 +181,9 @@ RSpec.describe Jobs::ChatNotifyWatching do
     end
 
     it "sends a desktop notification" do
-      messages = notification_messages_for(user2)
+      messages = listen_for_notifications(user2)
 
-      expect(messages.first.data).to include(
+      assert_notification_alert_is_correct(messages.first.data, user1, user2, message)
-        {
-          username: user1.username,
-          notification_type: Notification.types[:chat_message],
-          post_url: channel.relative_url,
-          translated_title:
-            I18n.t(
-              "discourse_push_notifications.popup.new_chat_message",
-              { username: user1.username, channel: channel.title(user2) },
-            ),
-          tag: Chat::ChatNotifier.push_notification_tag(:message, channel.id),
-          excerpt: message.message,
-        },
-      )
     end
 
     context "when the channel is muted via membership preferences" do
@@ -67,7 +191,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
 
       it "does not send a desktop or mobile notification" do
         PostAlerter.expects(:push_notification).never
-        messages = notification_messages_for(user2)
+        messages = listen_for_notifications(user2)
         expect(messages).to be_empty
       end
     end
@@ -80,34 +204,20 @@ RSpec.describe Jobs::ChatNotifyWatching do
         )
       end
 
-      it "sends a mobile notification" do
+      it "only sends a mobile notification" do
-        PostAlerter.expects(:push_notification).with(
+        expects_push_notification(user1, user2, message)
-          user2,
+
-          has_entries(
+        messages = listen_for_notifications(user2)
-            {
+
-              username: user1.username,
-              notification_type: Notification.types[:chat_message],
-              post_url: channel.relative_url,
-              translated_title:
-                I18n.t(
-                  "discourse_push_notifications.popup.new_chat_message",
-                  { username: user1.username, channel: channel.title(user2) },
-                ),
-              tag: Chat::ChatNotifier.push_notification_tag(:message, channel.id),
-              excerpt: message.message,
-            },
-          ),
-        )
-        messages = notification_messages_for(user2)
         expect(messages.length).to be_zero
       end
 
       context "when the channel is muted via membership preferences" do
         before { membership2.update!(muted: true) }
 
-        it "does not send a desktop or mobile notification" do
+        it "does not send any notification" do
           PostAlerter.expects(:push_notification).never
-          messages = notification_messages_for(user2)
+          messages = listen_for_notifications(user2)
           expect(messages).to be_empty
         end
       end
@@ -117,7 +227,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { SiteSetting.chat_allowed_groups = group.id }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
       end
     end
 
@@ -125,7 +235,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { channel.update!(chatable: Fabricate(:private_category, group: group)) }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
       end
     end
 
@@ -133,7 +243,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { membership2.update!(last_read_message_id: message.id) }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
       end
     end
 
@@ -141,7 +251,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { PresenceChannel.any_instance.expects(:user_ids).returns([user2.id]) }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
       end
     end
 
@@ -149,15 +259,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { user2.update!(suspended_till: 1.year.from_now) }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
-      end
-    end
-
-    context "when the target user is inside the except_user_ids array" do
-      let(:except_user_ids) { [user2.id] }
-
-      it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
       end
     end
   end
@@ -184,22 +286,9 @@ RSpec.describe Jobs::ChatNotifyWatching do
     end
 
     it "sends a desktop notification" do
-      messages = notification_messages_for(user2)
+      messages = listen_for_notifications(user2)
 
-      expect(messages.first.data).to include(
+      assert_notification_alert_is_correct(messages.first.data, user1, user2, message)
-        {
-          username: user1.username,
-          notification_type: Notification.types[:chat_message],
-          post_url: channel.relative_url,
-          translated_title:
-            I18n.t(
-              "discourse_push_notifications.popup.new_direct_chat_message",
-              { username: user1.username, channel: channel.title(user2) },
-            ),
-          tag: Chat::ChatNotifier.push_notification_tag(:message, channel.id),
-          excerpt: message.message,
-        },
-      )
     end
 
     context "when the channel is muted via membership preferences" do
@@ -207,7 +296,9 @@ RSpec.describe Jobs::ChatNotifyWatching do
 
       it "does not send a desktop or mobile notification" do
         PostAlerter.expects(:push_notification).never
-        messages = notification_messages_for(user2)
+
+        messages = listen_for_notifications(user2)
+
         expect(messages).to be_empty
       end
     end
@@ -221,24 +312,10 @@ RSpec.describe Jobs::ChatNotifyWatching do
       end
 
       it "sends a mobile notification" do
-        PostAlerter.expects(:push_notification).with(
+        expects_push_notification(user1, user2, message)
-          user2,
+
-          has_entries(
+        messages = listen_for_notifications(user2)
-            {
+
-              username: user1.username,
-              notification_type: Notification.types[:chat_message],
-              post_url: channel.relative_url,
-              translated_title:
-                I18n.t(
-                  "discourse_push_notifications.popup.new_direct_chat_message",
-                  { username: user1.username, channel: channel.title(user2) },
-                ),
-              tag: Chat::ChatNotifier.push_notification_tag(:message, channel.id),
-              excerpt: message.message,
-            },
-          ),
-        )
-        messages = notification_messages_for(user2)
         expect(messages.length).to be_zero
       end
 
@@ -247,7 +324,9 @@ RSpec.describe Jobs::ChatNotifyWatching do
 
         it "does not send a desktop or mobile notification" do
           PostAlerter.expects(:push_notification).never
-          messages = notification_messages_for(user2)
+
+          messages = listen_for_notifications(user2)
+
           expect(messages).to be_empty
         end
       end
@@ -257,7 +336,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { SiteSetting.chat_allowed_groups = group.id }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
       end
     end
 
@@ -265,7 +344,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { membership2.destroy! }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
       end
     end
 
@@ -273,7 +352,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { membership2.update!(last_read_message_id: message.id) }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
       end
     end
 
@@ -281,7 +360,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { PresenceChannel.any_instance.expects(:user_ids).returns([user2.id]) }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
       end
     end
 
@@ -289,15 +368,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { user2.update!(suspended_till: 1.year.from_now) }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
-      end
-    end
-
-    context "when the target user is inside the except_user_ids array" do
-      let(:except_user_ids) { [user2.id] }
-
-      it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
       end
     end
 
@@ -305,7 +376,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       before { UserCommScreener.any_instance.expects(:allowing_actor_communication).returns([]) }
 
       it "does not send a desktop notification" do
-        expect(notification_messages_for(user2).count).to be_zero
+        expect(listen_for_notifications(user2).count).to be_zero
       end
     end
   end

plugins/chat/spec/jobs/regular/send_message_notifications_spec.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+RSpec.describe Jobs::SendMessageNotifications do
+  describe "#execute" do
+    context "when the message doesn't exist" do
+      it "does nothing" do
+        Chat::ChatNotifier.any_instance.expects(:notify_new).never
+        Chat::ChatNotifier.any_instance.expects(:notify_edit).never
+
+        subject.execute(eason: "new", timestamp: 1.minute.ago)
+      end
+    end
+
+    context "when there's a message" do
+      fab!(:chat_message) { Fabricate(:chat_message) }
+
+      it "does nothing when the reason is invalid" do
+        Chat::ChatNotifier.expects(:notify_new).never
+        Chat::ChatNotifier.expects(:notify_edit).never
+
+        subject.execute(
+          chat_message_id: chat_message.id,
+          reason: "invalid",
+          timestamp: 1.minute.ago
+        )
+      end
+
+      it "does nothing if there is no timestamp" do
+        Chat::ChatNotifier.any_instance.expects(:notify_new).never
+        Chat::ChatNotifier.any_instance.expects(:notify_edit).never
+
+        subject.execute(
+          chat_message_id: chat_message.id,
+          reason: "invalid"
+        )
+      end
+
+      it "calls notify_new when the reason is 'new'" do
+        Chat::ChatNotifier.any_instance.expects(:notify_new).once
+        Chat::ChatNotifier.any_instance.expects(:notify_edit).never
+
+        subject.execute(
+          chat_message_id: chat_message.id,
+          reason: "new",
+          timestamp: 1.minute.ago
+        )
+      end
+
+      it "calls notify_edit when the reason is 'edit'" do
+        Chat::ChatNotifier.any_instance.expects(:notify_new).never
+        Chat::ChatNotifier.any_instance.expects(:notify_edit).once
+
+        subject.execute(
+          chat_message_id: chat_message.id,
+          reason: "edit",
+          timestamp: 1.minute.ago
+        )
+      end
+    end
+  end
+end

plugins/chat/spec/lib/chat_notifier_spec.rb

@@ -3,27 +3,39 @@
 require "rails_helper"
 
 describe Chat::ChatNotifier do
-  describe "#notify_new" do
+  fab!(:channel) { Fabricate(:category_channel) }
-    fab!(:channel) { Fabricate(:category_channel) }
+  fab!(:user_1) { Fabricate(:user) }
-    fab!(:user_1) { Fabricate(:user) }
+  fab!(:user_2) { Fabricate(:user) }
-    fab!(:user_2) { Fabricate(:user) }
 
-    before do
+  fab!(:chat_group) do
-      @chat_group =
+    Fabricate(
-        Fabricate(
+      :group,
-          :group,
+      users: [user_1, user_2],
-          users: [user_1, user_2],
+      mentionable_level: Group::ALIAS_LEVELS[:everyone],
-          mentionable_level: Group::ALIAS_LEVELS[:everyone],
+    )
-        )
+  end
-      SiteSetting.chat_allowed_groups = @chat_group.id
 
-      [user_1, user_2].each do |u|
+  fab!(:user_1_membership) { Fabricate(:user_chat_channel_membership, chat_channel: channel, user: user_1) }
-        Fabricate(:user_chat_channel_membership, chat_channel: channel, user: u)
+  fab!(:user_2_membership) { Fabricate(:user_chat_channel_membership, chat_channel: channel, user: user_2) }
-      end
+
+  before { SiteSetting.chat_allowed_groups = chat_group.id }
+
+  def assert_users_were_notifier_with_mention_type(mention_type, user_ids)
+    if user_ids.empty?
+      expect(
+        job_enqueued?(job: :chat_notify_mentioned, args: { mention_type: mention_type.to_s })
+      ).to eq(false)
+    else
+      expect(
+        job_enqueued?(job: :chat_notify_mentioned, args: { mention_type: mention_type.to_s, user_ids: user_ids })
+      ).to eq(true)
     end
+  end
 
+  describe "#notify_new" do
     def build_cooked_msg(message_body, user, chat_channel: channel)
       ChatMessage.new(
+        id: 1,
         chat_channel: chat_channel,
         user: user,
         message: message_body,
@@ -35,54 +47,54 @@ describe Chat::ChatNotifier do
       it "returns an empty list when the message doesn't include a channel mention" do
         msg = build_cooked_msg(mention.gsub("@", ""), user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to be_empty
+        assert_users_were_notifier_with_mention_type(list_key, [])
       end
 
       it "will never include someone who is not accepting channel-wide notifications" do
         user_2.user_option.update!(ignore_channel_wide_mention: true)
         msg = build_cooked_msg(mention, user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to be_empty
+        assert_users_were_notifier_with_mention_type(list_key, [])
       end
 
       it "will never mention when channel is not accepting channel wide mentions" do
         channel.update!(allow_channel_wide_mentions: false)
         msg = build_cooked_msg(mention, user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to be_empty
+        assert_users_were_notifier_with_mention_type(list_key, [])
       end
 
       it "includes all members of a channel except the sender" do
         msg = build_cooked_msg(mention, user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to contain_exactly(user_2.id)
+        assert_users_were_notifier_with_mention_type(list_key, [user_2.id])
       end
     end
 
     shared_examples "ensure only channel members are notified" do
       it "will never include someone outside the channel" do
         user3 = Fabricate(:user)
-        @chat_group.add(user3)
+        chat_group.add(user3)
         another_channel = Fabricate(:category_channel)
         Fabricate(:user_chat_channel_membership, chat_channel: another_channel, user: user3)
         msg = build_cooked_msg(mention, user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to contain_exactly(user_2.id)
+        assert_users_were_notifier_with_mention_type(list_key, [user_2.id])
       end
 
       it "will never include someone not following the channel anymore" do
         user3 = Fabricate(:user)
-        @chat_group.add(user3)
+        chat_group.add(user3)
         Fabricate(
           :user_chat_channel_membership,
           following: false,
@@ -91,14 +103,14 @@ describe Chat::ChatNotifier do
         )
         msg = build_cooked_msg(mention, user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to contain_exactly(user_2.id)
+        assert_users_were_notifier_with_mention_type(list_key, [user_2.id])
       end
 
       it "will never include someone who is suspended" do
         user3 = Fabricate(:user, suspended_till: 2.years.from_now)
-        @chat_group.add(user3)
+        chat_group.add(user3)
         Fabricate(
           :user_chat_channel_membership,
           following: true,
@@ -108,9 +120,9 @@ describe Chat::ChatNotifier do
 
         msg = build_cooked_msg(mention, user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to contain_exactly(user_2.id)
+        assert_users_were_notifier_with_mention_type(list_key, [user_2.id])
       end
     end
 
@@ -120,26 +132,6 @@ describe Chat::ChatNotifier do
 
       include_examples "channel-wide mentions"
       include_examples "ensure only channel members are notified"
-
-      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)
-          msg = build_cooked_msg(mention, user_1)
-
-          to_notify = described_class.new(msg, msg.created_at).notify_new
-
-          expect(to_notify[list_key]).to be_empty
-        end
-
-        it "does not send notifications to the user who is ignoring the acting user" do
-          Fabricate(:ignored_user, user: user_2, ignored_user: user_1, expiring_at: 1.day.from_now)
-          msg = build_cooked_msg(mention, user_1)
-
-          to_notify = described_class.new(msg, msg.created_at).notify_new
-
-          expect(to_notify[:direct_mentions]).to be_empty
-        end
-      end
     end
 
     describe "here_mentions" do
@@ -154,106 +146,58 @@ describe Chat::ChatNotifier do
       it "includes users seen less than 5 minutes ago" do
         msg = build_cooked_msg(mention, user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to contain_exactly(user_2.id)
+        assert_users_were_notifier_with_mention_type(list_key, [user_2.id])
       end
 
       it "excludes users seen more than 5 minutes ago" do
         user_2.update!(last_seen_at: 6.minutes.ago)
         msg = build_cooked_msg(mention, user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to be_empty
+        assert_users_were_notifier_with_mention_type(list_key, [])
       end
 
       it "excludes users mentioned directly" do
         msg = build_cooked_msg("hello @here @#{user_2.username}!", user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[list_key]).to be_empty
+        assert_users_were_notifier_with_mention_type(list_key, [])
-      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)
-          msg = build_cooked_msg(mention, user_1)
-
-          to_notify = described_class.new(msg, msg.created_at).notify_new
-
-          expect(to_notify[list_key]).to be_empty
-        end
-
-        it "does not send notifications to the user who is ignoring the acting user" do
-          Fabricate(:ignored_user, user: user_2, ignored_user: user_1, expiring_at: 1.day.from_now)
-          msg = build_cooked_msg(mention, user_1)
-
-          to_notify = described_class.new(msg, msg.created_at).notify_new
-
-          expect(to_notify[:direct_mentions]).to be_empty
-        end
       end
     end
 
     describe "direct_mentions" do
       it "only include mentioned users who are already in the channel" do
         user_3 = Fabricate(:user)
-        @chat_group.add(user_3)
+        chat_group.add(user_3)
         another_channel = Fabricate(:category_channel)
         Fabricate(:user_chat_channel_membership, chat_channel: another_channel, user: user_3)
         msg = build_cooked_msg("Is @#{user_3.username} here? And @#{user_2.username}", user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[:direct_mentions]).to contain_exactly(user_2.id)
+        assert_users_were_notifier_with_mention_type(:direct_mentions, [user_2.id])
       end
 
       it "include users as direct mentions even if there's a @here mention" do
         msg = build_cooked_msg("Hello @here and @#{user_2.username}", user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[:here_mentions]).to be_empty
+        assert_users_were_notifier_with_mention_type(:here_mentions, [])
-        expect(to_notify[:direct_mentions]).to contain_exactly(user_2.id)
+        assert_users_were_notifier_with_mention_type(:direct_mentions, [user_2.id])
       end
 
       it "include users as direct mentions even if there's a @all mention" do
         msg = build_cooked_msg("Hello @all and @#{user_2.username}", user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[:global_mentions]).to be_empty
+        assert_users_were_notifier_with_mention_type(:global_mentions, [])
-        expect(to_notify[:direct_mentions]).to contain_exactly(user_2.id)
+        assert_users_were_notifier_with_mention_type(:direct_mentions, [user_2.id])
-      end
-
-      describe "users ignoring or muting the user creating the message" do
-        it "does not publish new mentions to these users" do
-          Fabricate(:muted_user, user: user_2, muted_user: user_1)
-          msg = build_cooked_msg("hey @#{user_2.username} stop muting me!", user_1)
-
-          ChatPublisher.expects(:publish_new_mention).never
-          to_notify = described_class.new(msg, msg.created_at).notify_new
-        end
-
-        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)
-          msg = build_cooked_msg("hey @#{user_2.username} stop muting me!", user_1)
-
-          to_notify = described_class.new(msg, msg.created_at).notify_new
-
-          expect(to_notify[:direct_mentions]).to be_empty
-        end
-
-        it "does not send notifications to the user who is ignoring the acting user" do
-          Fabricate(:ignored_user, user: user_2, ignored_user: user_1, expiring_at: 1.day.from_now)
-          msg = build_cooked_msg("hey @#{user_2.username} stop ignoring me!", user_1)
-
-          to_notify = described_class.new(msg, msg.created_at).notify_new
-
-          expect(to_notify[:direct_mentions]).to be_empty
-        end
       end
     end
 
@@ -268,7 +212,7 @@ describe Chat::ChatNotifier do
       end
       fab!(:other_channel) { Fabricate(:category_channel) }
 
-      before { @chat_group.add(user_3) }
+      before { chat_group.add(user_3) }
 
       let(:mention) { "hello @#{group.name}!" }
       let(:list_key) { group.name }
@@ -278,7 +222,7 @@ describe Chat::ChatNotifier do
       it 'calls guardian can_join_chat_channel?' do
         Guardian.any_instance.expects(:can_join_chat_channel?).at_least_once
         msg = build_cooked_msg("Hello @#{group.name} and @#{user_2.username}", user_1)
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
       end
 
       it "establishes a far-left precedence among group mentions" do
@@ -288,19 +232,20 @@ describe Chat::ChatNotifier do
           user: user_3,
           following: true,
         )
-        msg = build_cooked_msg("Hello @#{@chat_group.name} and @#{group.name}", user_1)
+        msg = build_cooked_msg("Hello @#{chat_group.name} and @#{group.name}", user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[@chat_group.name]).to contain_exactly(user_2.id, user_3.id)
+        assert_users_were_notifier_with_mention_type(chat_group.name, [user_2.id, user_3.id])
-        expect(to_notify[list_key]).to be_empty
+        assert_users_were_notifier_with_mention_type(list_key, [])
 
-        second_msg = build_cooked_msg("Hello @#{group.name} and @#{@chat_group.name}", user_1)
+        Jobs::ChatNotifyMentioned.clear
+        second_msg = build_cooked_msg("Hello @#{group.name} and @#{chat_group.name}", user_1)
 
         to_notify_2 = described_class.new(second_msg, second_msg.created_at).notify_new
 
-        expect(to_notify_2[list_key]).to contain_exactly(user_2.id, user_3.id)
+        assert_users_were_notifier_with_mention_type(list_key, [user_2.id, user_3.id])
-        expect(to_notify_2[@chat_group.name]).to be_empty
+        assert_users_were_notifier_with_mention_type(chat_group.name, [])
       end
 
       it "skips groups with too many members" do
@@ -308,9 +253,9 @@ describe Chat::ChatNotifier do
 
         msg = build_cooked_msg("Hello @#{group.name}", user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[group.name]).to be_nil
+        assert_users_were_notifier_with_mention_type(group.name, [])
       end
 
       it "respects the 'max_mentions_per_chat_message' setting and skips notifications" do
@@ -318,10 +263,10 @@ describe Chat::ChatNotifier do
 
         msg = build_cooked_msg("Hello @#{user_2.username} and @#{user_3.username}", user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[:direct_mentions]).to be_empty
+        assert_users_were_notifier_with_mention_type(:direct_mentions, [])
-        expect(to_notify[group.name]).to be_nil
+        assert_users_were_notifier_with_mention_type(group.name, [])
       end
 
       it "respects the max mentions setting and skips notifications when mixing users and groups" do
@@ -329,36 +274,10 @@ describe Chat::ChatNotifier do
 
         msg = build_cooked_msg("Hello @#{user_2.username} and @#{group.name}", user_1)
 
-        to_notify = described_class.new(msg, msg.created_at).notify_new
+        described_class.new(msg, msg.created_at).notify_new
 
-        expect(to_notify[:direct_mentions]).to be_empty
+        assert_users_were_notifier_with_mention_type(:direct_mentions, [])
-        expect(to_notify[group.name]).to be_nil
+        assert_users_were_notifier_with_mention_type(group.name, [])
-      end
-
-      describe "users ignoring or muting the user creating the message" do
-        it "does not send notifications to the user inside the group who is muting the acting user" do
-          group.add(user_3)
-          Fabricate(:user_chat_channel_membership, chat_channel: channel, user: user_3)
-          Fabricate(:muted_user, user: user_2, muted_user: user_1)
-          msg = build_cooked_msg("Hello @#{group.name}", user_1)
-
-          to_notify = described_class.new(msg, msg.created_at).notify_new
-
-          expect(to_notify[:direct_mentions]).to be_empty
-          expect(to_notify[group.name]).to contain_exactly(user_3.id)
-        end
-
-        it "does not send notifications to the user inside the group who is ignoring the acting user" do
-          group.add(user_3)
-          Fabricate(:user_chat_channel_membership, chat_channel: channel, user: user_3)
-          Fabricate(:ignored_user, user: user_2, ignored_user: user_1, expiring_at: 1.day.from_now)
-          msg = build_cooked_msg("Hello @#{group.name}", user_1)
-
-          to_notify = described_class.new(msg, msg.created_at).notify_new
-
-          expect(to_notify[:direct_mentions]).to be_empty
-          expect(to_notify[group.name]).to contain_exactly(user_3.id)
-        end
       end
     end
 
@@ -370,9 +289,9 @@ describe Chat::ChatNotifier do
 
         messages =
           MessageBus.track_publish("/chat/#{channel.id}") do
-            to_notify = described_class.new(msg, msg.created_at).notify_new
+            described_class.new(msg, msg.created_at).notify_new
 
-            expect(to_notify[:direct_mentions]).to be_empty
+            assert_users_were_notifier_with_mention_type(:direct_mentions, [])
           end
 
         unreachable_msg = messages.first
@@ -392,7 +311,7 @@ describe Chat::ChatNotifier do
           )
         end
 
-        before { @chat_group.add(user_3) }
+        before { chat_group.add(user_3) }
 
         it "notify posts of users who are not participating in a personal message" do
           msg =
@@ -404,34 +323,9 @@ describe Chat::ChatNotifier do
 
           messages =
             MessageBus.track_publish("/chat/#{personal_chat_channel.id}") do
-              to_notify = described_class.new(msg, msg.created_at).notify_new
+              described_class.new(msg, msg.created_at).notify_new
 
-              expect(to_notify[:direct_mentions]).to be_empty
+              assert_users_were_notifier_with_mention_type(:direct_mentions, [])
-            end
-
-          unreachable_msg = messages.first
-
-          expect(unreachable_msg).to be_present
-          expect(unreachable_msg.data[:without_membership]).to be_empty
-          unreachable_users = unreachable_msg.data[:cannot_see].map { |u| u["id"] }
-          expect(unreachable_users).to contain_exactly(user_3.id)
-        end
-
-        it "notify posts of users who are part of the mentioned group but participating" do
-          group =
-            Fabricate(
-              :public_group,
-              users: [user_2, user_3],
-              mentionable_level: Group::ALIAS_LEVELS[:everyone],
-            )
-          msg =
-            build_cooked_msg("Hello @#{group.name}", user_1, chat_channel: personal_chat_channel)
-
-          messages =
-            MessageBus.track_publish("/chat/#{personal_chat_channel.id}") do
-              to_notify = described_class.new(msg, msg.created_at).notify_new
-
-              expect(to_notify[group.name]).to contain_exactly(user_2.id)
             end
 
           unreachable_msg = messages.first
@@ -447,16 +341,16 @@ describe Chat::ChatNotifier do
     describe "users who can be invited to join the channel" do
       fab!(:user_3) { Fabricate(:user) }
 
-      before { @chat_group.add(user_3) }
+      before { chat_group.add(user_3) }
 
       it "can invite chat user without channel membership" do
         msg = build_cooked_msg("Hello @#{user_3.username}", user_1)
 
         messages =
           MessageBus.track_publish("/chat/#{channel.id}") do
-            to_notify = described_class.new(msg, msg.created_at).notify_new
+            described_class.new(msg, msg.created_at).notify_new
 
-            expect(to_notify[:direct_mentions]).to be_empty
+            assert_users_were_notifier_with_mention_type(:direct_mentions, [])
           end
 
         not_participating_msg = messages.first
@@ -473,9 +367,9 @@ describe Chat::ChatNotifier do
 
         messages =
           MessageBus.track_publish("/chat/#{channel.id}") do
-            to_notify = described_class.new(msg, msg.created_at).notify_new
+            described_class.new(msg, msg.created_at).notify_new
 
-            expect(to_notify[:direct_mentions]).to be_empty
+            assert_users_were_notifier_with_mention_type(:direct_mentions, [])
           end
 
         expect(messages).to be_empty
@@ -487,9 +381,9 @@ describe Chat::ChatNotifier do
 
         messages =
           MessageBus.track_publish("/chat/#{channel.id}") do
-            to_notify = described_class.new(msg, msg.created_at).notify_new
+            described_class.new(msg, msg.created_at).notify_new
 
-            expect(to_notify[:direct_mentions]).to be_empty
+            assert_users_were_notifier_with_mention_type(:direct_mentions, [])
           end
 
         expect(messages).to be_empty
@@ -506,33 +400,9 @@ describe Chat::ChatNotifier do
 
         messages =
           MessageBus.track_publish("/chat/#{channel.id}") do
-            to_notify = described_class.new(msg, msg.created_at).notify_new
+            described_class.new(msg, msg.created_at).notify_new
 
-            expect(to_notify[:direct_mentions]).to be_empty
+            assert_users_were_notifier_with_mention_type(:direct_mentions, [])
-          end
-
-        not_participating_msg = messages.first
-
-        expect(not_participating_msg).to be_present
-        expect(not_participating_msg.data[:cannot_see]).to be_empty
-        not_participating_users = not_participating_msg.data[:without_membership].map { |u| u["id"] }
-        expect(not_participating_users).to contain_exactly(user_3.id)
-      end
-
-      it "can invite other group members to channel" do
-        group =
-          Fabricate(
-            :public_group,
-            users: [user_2, user_3],
-            mentionable_level: Group::ALIAS_LEVELS[:everyone],
-          )
-        msg = build_cooked_msg("Hello @#{group.name}", user_1)
-
-        messages =
-          MessageBus.track_publish("/chat/#{channel.id}") do
-            to_notify = described_class.new(msg, msg.created_at).notify_new
-
-            expect(to_notify[:direct_mentions]).to be_empty
           end
 
         not_participating_msg = messages.first
@@ -555,9 +425,9 @@ describe Chat::ChatNotifier do
 
         messages =
           MessageBus.track_publish("/chat/#{channel.id}") do
-            to_notify = described_class.new(msg, msg.created_at).notify_new
+            described_class.new(msg, msg.created_at).notify_new
 
-            expect(to_notify[:direct_mentions]).to be_empty
+            assert_users_were_notifier_with_mention_type(:direct_mentions, [])
           end
 
         expect(messages).to be_empty
@@ -575,9 +445,9 @@ describe Chat::ChatNotifier do
 
         messages =
           MessageBus.track_publish("/chat/#{channel.id}") do
-            to_notify = described_class.new(msg, msg.created_at).notify_new
+            described_class.new(msg, msg.created_at).notify_new
 
-            expect(to_notify[:direct_mentions]).to be_empty
+            assert_users_were_notifier_with_mention_type(:direct_mentions, [])
           end
 
         expect(messages).to be_empty
@@ -599,9 +469,9 @@ describe Chat::ChatNotifier do
         msg = build_cooked_msg("Hello @#{group.name}", user_1)
 
         messages = MessageBus.track_publish("/chat/#{channel.id}") do
-          to_notify = described_class.new(msg, msg.created_at).notify_new
+          described_class.new(msg, msg.created_at).notify_new
 
-          expect(to_notify[group.name]).to be_nil
+          assert_users_were_notifier_with_mention_type(group.name, [])
         end
 
         too_many_members_msg = messages.first
@@ -615,9 +485,9 @@ describe Chat::ChatNotifier do
         msg = build_cooked_msg("Hello @#{group.name}", user_1)
 
         messages = MessageBus.track_publish("/chat/#{channel.id}") do
-          to_notify = described_class.new(msg, msg.created_at).notify_new
+          described_class.new(msg, msg.created_at).notify_new
 
-          expect(to_notify[group.name]).to be_nil
+          assert_users_were_notifier_with_mention_type(group.name, [])
         end
 
         mentions_disabled_msg = messages.first
@@ -626,5 +496,87 @@ describe Chat::ChatNotifier do
         expect(mentions_disabled).to contain_exactly(group.name)
       end
     end
+
+    describe "establishing a precedence between mention types" do
+      before { user_2.update!(last_seen_at: 4.minutes.ago) }
+
+      it "gives direct mentions the highest precedence" do
+        msg = build_cooked_msg("@#{user_2.username} @#{chat_group.name} @here @all", user_1)
+
+        described_class.new(msg, msg.created_at).notify_new
+
+        assert_users_were_notifier_with_mention_type(:direct_mentions, [user_2.id])
+        assert_users_were_notifier_with_mention_type(chat_group.name, [])
+        assert_users_were_notifier_with_mention_type(:here_mentions, [])
+        assert_users_were_notifier_with_mention_type(:global_mentions, [])
+      end
+
+      it "gives group mentions the second highest precedence" do
+        msg = build_cooked_msg("@#{chat_group.name} @here @all", user_1)
+
+        described_class.new(msg, msg.created_at).notify_new
+
+        assert_users_were_notifier_with_mention_type(:direct_mentions, [])
+        assert_users_were_notifier_with_mention_type(chat_group.name, [user_2.id])
+        assert_users_were_notifier_with_mention_type(:here_mentions, [])
+        assert_users_were_notifier_with_mention_type(:global_mentions, [])
+      end
+
+      it "gives here mentions the third highest precedence" do
+        msg = build_cooked_msg("@here @all", user_1)
+
+        described_class.new(msg, msg.created_at).notify_new
+
+        assert_users_were_notifier_with_mention_type(:direct_mentions, [])
+        assert_users_were_notifier_with_mention_type(chat_group.name, [])
+        assert_users_were_notifier_with_mention_type(:here_mentions, [user_2.id])
+        assert_users_were_notifier_with_mention_type(:global_mentions, [])
+      end
+    end
+  end
+
+  describe "#notify_edit" do
+    fab!(:chat_message) { Fabricate(:chat_message, chat_channel: channel, user: user_1) }
+    fab!(:user_2_mention) { Fabricate(:chat_mention, user: user_2, chat_message: chat_message) }
+
+    def edit_msg(chat_message, new_body)
+      chat_message.message = new_body
+      chat_message.cook
+
+      described_class.new(chat_message, chat_message.updated_at).notify_edit
+    end
+
+    describe "removing a mention from a message update existing mentions records" do
+      it "deletes everything when removing all mentions" do
+        edit_msg(chat_message, "No more mentions")
+
+        expect { user_2_mention.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it "does nothing if the user still has access through a group" do
+        edit_msg(chat_message, "Hello @#{chat_group.name}")
+
+        expect { user_2_mention.reload }.not_to raise_error
+      end
+
+      it "removes the record when mentioning a different group" do
+        group_2 = Fabricate(:group)
+        edit_msg(chat_message, "Hello @#{group_2.name}")
+
+        expect { user_2_mention.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it "does nothing when we keep the username mention" do
+        edit_msg(chat_message, "Hello @#{user_2.username}")
+
+        expect { user_2_mention.reload }.not_to raise_error
+      end
+
+      it "removes the mention when only mentioning a different user" do
+        edit_msg(chat_message, "Hello @#{user_1.username}")
+
+        expect { user_2_mention.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
   end
 end

plugins/chat/spec/system/jit_messages_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe "JIT messages", type: :system, js: true do
   let(:chat) { PageObjects::Pages::Chat.new }
 
   before do
+    Jobs.run_immediately!
     channel_1.add(current_user)
     chat_system_bootstrap
     sign_in(current_user)

plugins/chat/spec/system/message_notifications_with_sidebar_spec.rb

@@ -89,6 +89,8 @@ RSpec.describe "Message notifications - with sidebar", type: :system, js: true d
           end
 
           context "when a message with mentions is created" do
+            before { Jobs.run_immediately! }
+
             it "correctly renders notifications" do
               visit("/")
               using_session(:user_1) do

plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb

@@ -163,6 +163,7 @@ RSpec.describe "User menu notifications | sidebar", type: :system, js: true do
     fab!(:other_user) { Fabricate(:user) }
 
     before do
+      Jobs.run_immediately!
       channel_1.add(current_user)
     end