REFACTOR: Separate post-send warnings logic.

After #19666, we send notifications asynchronously. However, this has dramatically delayed the different post-send warnings we send.

This change moves all the post-send warnings logic from `Chat::ChatNotifier` into a separate service, allowing us to run this part synchronously, displaying the warnings almost instantly without doing all the heavy lifting for sending notifications. It also simplifies the notifier, making another refactor we planned simpler.

Finally, this removes the need to run jobs on the system specs that test the post-warning logic, hopefully making them faster.

plugins/chat/app/services/chat_publisher.rb

@@ -174,25 +174,12 @@ module ChatPublisher
     end
   end
 
-  def self.publish_inaccessible_mentions(
+  def self.publish_inaccessible_mentions(user_id, chat_message, warnings)
-    user_id,
-    chat_message,
-    cannot_chat_users,
-    without_membership,
-    too_many_members,
-    mentions_disabled
-  )
     MessageBus.publish(
       "/chat/#{chat_message.chat_channel_id}",
-      {
+      type: :mention_warnings,
-        type: :mention_warning,
+      chat_message_id: chat_message.id,
-        chat_message_id: chat_message.id,
+      warnings: warnings,
-        cannot_see: cannot_chat_users.map { |u| { username: u.username, id: u.id } }.as_json,
-        without_membership:
-          without_membership.map { |u| { username: u.username, id: u.id } }.as_json,
-        groups_with_too_many_members: too_many_members.map(&:name).as_json,
-        group_mentions_disabled: mentions_disabled.map(&:name).as_json,
-      },
       user_ids: [user_id],
     )
   end

plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js

@@ -745,8 +745,8 @@ export default Component.extend({
       case "restore":
         this.handleRestoreMessage(data);
         break;
-      case "mention_warning":
+      case "mention_warnings":
-        this.handleMentionWarning(data);
+        this.handleMentionWarnings(data);
         break;
       case "self_flagged":
         this.handleSelfFlaggedMessage(data);
@@ -930,8 +930,11 @@ export default Component.extend({
     return 0;
   },
 
-  handleMentionWarning(data) {
+  handleMentionWarnings(data) {
-    this.messageLookup[data.chat_message_id]?.set("mentionWarning", data);
+    this.messageLookup[data.chat_message_id]?.set(
+      "mentionWarnings",
+      data.warnings
+    );
   },
 
   handleSelfFlaggedMessage(data) {

plugins/chat/assets/javascripts/discourse/components/chat-message.hbs

@@ -187,54 +187,12 @@
             </div>
           {{/if}}
 
-          {{#if this.mentionWarning}}
+          <PostSendMentionWarnings
-            <div class="alert alert-info chat-message-mention-warning">
+            @chatChannelId={{this.details.chat_channel_id}}
-              {{#if this.mentionWarning.invitationSent}}
+            @warnings={{this.mentionWarnings}}
-                {{d-icon "check"}}
+            @messageId={{this.message.id}}
-                <span>
+            @dismiss={{this.dismissMentionWarnings}}
-                  {{i18n
+          />
-                    "chat.mention_warning.invitations_sent"
-                    count=this.mentionWarning.without_membership.length
-                  }}
-                </span>
-              {{else}}
-                <FlatButton
-                  @class="dismiss-mention-warning"
-                  @title="chat.mention_warning.dismiss"
-                  @action={{this.dismissMentionWarning}}
-                  @icon="times"
-                />
-
-                {{#if this.mentionWarning.cannot_see}}
-                  <p class="warning-item cannot-see">
-                    {{this.mentionedCannotSeeText}}
-                  </p>
-                {{/if}}
-
-                {{#if this.mentionWarning.without_membership}}
-                  <p class="warning-item without-membership">
-                    <span>{{this.mentionedWithoutMembershipText}}</span>
-                    <a
-                      class="invite-link"
-                      href
-                      onclick={{this.inviteMentioned}}
-                    >
-                      {{i18n "chat.mention_warning.invite"}}
-                    </a>
-                  </p>
-                {{/if}}
-                {{#if this.mentionWarning.group_mentions_disabled}}
-                  <p class="warning-item">
-                    {{this.groupsWithDisabledMentions}}
-                  </p>
-                {{/if}}
-
-                {{#if this.mentionWarning.groups_with_too_many_members}}
-                  <p class="warning-item">{{this.groupsWithTooManyMembers}}</p>
-                {{/if}}
-              {{/if}}
-            </div>
-          {{/if}}
         </div>
       </div>
     {{/if}}

plugins/chat/assets/javascripts/discourse/components/chat-message.js

@@ -112,8 +112,6 @@ export default class ChatMessage extends Component {
       this,
       "_handleReactionMessage"
     );
-
-    cancel(this._invitationSentTimer);
   }
 
   @bind
@@ -404,75 +402,12 @@ export default class ChatMessage extends Component {
     );
   }
 
-  get mentionWarning() {
+  get mentionWarnings() {
     return this.args.message.get("mentionWarning");
   }
 
-  get mentionedCannotSeeText() {
-    return I18n.t("chat.mention_warning.cannot_see", {
-      username: this.mentionWarning?.cannot_see?.[0]?.username,
-      count: this.mentionWarning?.cannot_see?.length,
-      others: this._othersTranslation(
-        this.mentionWarning?.cannot_see?.length - 1
-      ),
-    });
-  }
-
-  get mentionedWithoutMembershipText() {
-    return I18n.t("chat.mention_warning.without_membership", {
-      username: this.mentionWarning?.without_membership?.[0]?.username,
-      count: this.mentionWarning?.without_membership?.length,
-      others: this._othersTranslation(
-        this.mentionWarning?.without_membership?.length - 1
-      ),
-    });
-  }
-
-  get groupsWithDisabledMentions() {
-    return I18n.t("chat.mention_warning.group_mentions_disabled", {
-      group_name: this.mentionWarning?.group_mentions_disabled?.[0],
-      count: this.mentionWarning?.group_mentions_disabled?.length,
-      others: this._othersTranslation(
-        this.mentionWarning?.group_mentions_disabled?.length - 1
-      ),
-    });
-  }
-
-  get groupsWithTooManyMembers() {
-    return I18n.t("chat.mention_warning.too_many_members", {
-      group_name: this.mentionWarning.groups_with_too_many_members?.[0],
-      count: this.mentionWarning.groups_with_too_many_members?.length,
-      others: this._othersTranslation(
-        this.mentionWarning.groups_with_too_many_members?.length - 1
-      ),
-    });
-  }
-
-  _othersTranslation(othersCount) {
-    return I18n.t("chat.mention_warning.warning_multiple", {
-      count: othersCount,
-    });
-  }
-
   @action
-  inviteMentioned() {
+  dismissMentionWarnings() {
-    const userIds = this.mentionWarning.without_membership.mapBy("id");
-
-    ajax(`/chat/${this.args.message.chat_channel_id}/invite`, {
-      method: "PUT",
-      data: { user_ids: userIds, chat_message_id: this.args.message.id },
-    }).then(() => {
-      this.args.message.set("mentionWarning.invitationSent", true);
-      this._invitationSentTimer = discourseLater(() => {
-        this.args.message.set("mentionWarning", null);
-      }, 3000);
-    });
-
-    return false;
-  }
-
-  @action
-  dismissMentionWarning() {
     this.args.message.set("mentionWarning", null);
   }
 

plugins/chat/assets/javascripts/discourse/components/post-send-mention-warnings.hbs

@@ -0,0 +1,34 @@
+{{#if this.show}}
+  <div class="alert alert-info chat-message-mention-warning">
+    {{#if this.invitesSent}}
+      {{d-icon "check"}}
+      <span>{{i18n
+          "chat.mention_warning.invitations_sent"
+          count=this.invitesSentCount
+        }}</span>
+    {{else}}
+      <FlatButton
+        @class="dismiss-mention-warning"
+        @title="chat.mention_warning.dismiss"
+        @action={{action "dismiss"}}
+        @icon="times"
+      />
+      {{#each this.translatedWarnings as |warning|}}
+        <p class="warning-item {{dasherize warning.type}}">
+          {{#if warning.include_invite_link}}
+            <span>{{warning.translation}}</span>
+            <a
+              class="invite-link"
+              href
+              onclick={{action "inviteMentioned" warning}}
+            >
+              {{i18n "chat.mention_warning.invite"}}
+            </a>
+          {{else}}
+            {{warning.translation}}
+          {{/if}}
+        </p>
+      {{/each}}
+    {{/if}}
+  </div>
+{{/if}}

plugins/chat/assets/javascripts/discourse/components/post-send-mention-warnings.js

@@ -0,0 +1,73 @@
+import Component from "@glimmer/component";
+import { action } from "@ember/object";
+import I18n from "I18n";
+import { tracked } from "@glimmer/tracking";
+import discourseLater from "discourse-common/lib/later";
+import { ajax } from "discourse/lib/ajax";
+import { cancel } from "@ember/runloop";
+
+export default class PostSendMentionWarnings extends Component {
+  @tracked invitesSent = false;
+  @tracked invitesSentCount = 0;
+
+  willDestroy() {
+    cancel(this._invitationSentTimer);
+  }
+
+  get warnings() {
+    return this.args.warnings;
+  }
+
+  get show() {
+    return this.warnings?.length > 0 || this.invitesSent;
+  }
+
+  get translatedWarnings() {
+    return this.warnings.map((warning) => {
+      warning.translation = I18n.t(`chat.mention_warning.${warning.type}`, {
+        mention: warning.mentions[0],
+        count: warning.mentions.length,
+        others: this._othersTranslation(warning.mentions.length - 1),
+      });
+
+      if (warning.type === "without_membership") {
+        warning.include_invite_link = true;
+      }
+
+      return warning;
+    });
+  }
+
+  _othersTranslation(othersCount) {
+    return I18n.t("chat.mention_warning.warning_multiple", {
+      count: othersCount,
+    });
+  }
+
+  @action
+  dismiss() {
+    this.args.dismiss();
+  }
+
+  @action
+  inviteMentioned(warning) {
+    ajax(`/chat/${this.args.chatChannelId}/invite`, {
+      method: "PUT",
+      data: {
+        user_ids: warning.mention_target_ids,
+        chat_message_id: this.args.messageId,
+      },
+    }).then(() => {
+      this.invitesSent = true;
+      this.invitesSentCount = warning.mention_target_ids.length;
+
+      this._invitationSentTimer = discourseLater(() => {
+        this.dismiss();
+        this.invitesSentCount = 0;
+        this.invitesSent = false;
+      }, 3000);
+    });
+
+    return false;
+  }
+}

plugins/chat/config/locales/client.en.yml

@@ -107,21 +107,21 @@ en:
       mention_warning:
         dismiss: "dismiss"
         cannot_see:
-          one: "%{username} cannot access this channel and was not notified."
+          one: "%{mention} cannot access this channel and was not notified."
-          other: "%{username} and %{others} cannot access this channel and were not notified."
+          other: "%{mention} and %{others} cannot access this channel and were not notified."
         invitations_sent:
           one: "Invitation sent"
           other: "Invitations sent"
         invite: "Invite to channel"
         without_membership:
-          one: "%{username} has not joined this channel."
+          one: "%{mention} has not joined this channel."
-          other: "%{username} and %{others} have not joined this channel."
+          other: "%{mention} and %{others} have not joined this channel."
         group_mentions_disabled:
-          one: "%{group_name} doesn't allow mentions"
+          one: "%{mention} doesn't allow mentions"
-          other: "%{group_name} and %{others} doesn't allow mentions"
+          other: "%{mention} and %{others} doesn't allow mentions"
         too_many_members:
-          one: "%{group_name} has too many members. No one was notified"
+          one: "%{mention} has too many members. No one was notified"
-          other: "%{group_name} and %{others} have too many members. No one was notified"
+          other: "%{mention} and %{others} have too many members. No one was notified"
         warning_multiple:
           one: "%{count} other"
           other: "%{count} others"

plugins/chat/lib/chat_message_creator.rb

@@ -58,6 +58,7 @@ class Chat::ChatMessageCreator
       ChatDraft.where(user_id: @user.id, chat_channel_id: @chat_channel.id).destroy_all
       ChatPublisher.publish_new!(@chat_channel, @chat_message, @staged_id)
       Jobs.enqueue(:process_chat_message, { chat_message_id: @chat_message.id })
+      Chat::MessageMentionWarnings.new.dispatch(@chat_message)
       Chat::ChatNotifier.notify_new(
         chat_message: @chat_message,
         timestamp: @chat_message.created_at,

plugins/chat/lib/chat_message_updater.rb

@@ -35,6 +35,7 @@ class Chat::ChatMessageUpdater
       @chat_message.reload
       ChatPublisher.publish_edit!(@chat_channel, @chat_message)
       Jobs.enqueue(:process_chat_message, { chat_message_id: @chat_message.id })
+      Chat::MessageMentionWarnings.new.dispatch(@chat_message)
       Chat::ChatNotifier.notify_edit(chat_message: @chat_message, timestamp: revision.created_at)
       DiscourseEvent.trigger(:chat_message_edited, @chat_message, @chat_channel, @user)
     rescue => error

plugins/chat/lib/chat_notifier.rb

@@ -12,18 +12,9 @@
 # For various reasons a mention may not notify a user:
 #
 # * The target user of the mention is ignoring or muting the user who created the message
-# * The target user either cannot chat or cannot see the chat channel, in which case
-#   they are defined as `unreachable`
-# * The target user is not a member of the channel, in which case they are defined
-#   as `welcome_to_join`
 # * In the case of global @here and @all mentions users with the preference
 #   `ignore_channel_wide_mention` set to true will not be notified
 #
-# For any users that fall under the `unreachable` or `welcome_to_join` umbrellas
-# we send a MessageBus message to the UI and to inform the creating user. The
-# creating user can invite any `welcome_to_join` users to the channel. Target
-# users who are ignoring or muting the creating user _do not_ fall into this bucket.
-#
 # The ignore/mute filtering is also applied via the ChatNotifyWatching job,
 # which prevents desktop / push notifications being sent.
 class Chat::ChatNotifier
@@ -72,8 +63,6 @@ class Chat::ChatNotifier
       ChatPublisher.publish_new_mention(member_id, @chat_channel.id, @chat_message.id)
     end
 
-    notify_creator_of_inaccessible_mentions(to_notify)
-
     notify_mentioned_users(to_notify)
     notify_watching_users(except: mentioned_user_ids << @user.id)
 
@@ -98,8 +87,6 @@ class Chat::ChatNotifier
     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
@@ -198,27 +185,16 @@ class Chat::ChatNotifier
     end
   end
 
-  def group_users_to_notify(users)
+  def select_channel_participants(users)
-    potential_participants, unreachable =
+    users.select do |user|
-      users.partition do |user|
+      guardian = Guardian.new(user)
-        guardian = Guardian.new(user)
+      guardian.can_chat? && guardian.can_join_chat_channel?(@chat_channel) &&
-        guardian.can_chat? && guardian.can_join_chat_channel?(@chat_channel)
+        user.user_chat_channel_memberships.any? do |m|
-      end
-
-    participants, welcome_to_join =
-      potential_participants.partition do |participant|
-        participant.user_chat_channel_memberships.any? do |m|
           predicate = m.chat_channel_id == @chat_channel.id
           predicate = predicate && m.following == true if @chat_channel.public_channel?
           predicate
         end
-      end
+    end
-
-    {
-      already_participating: participants || [],
-      welcome_to_join: welcome_to_join || [],
-      unreachable: unreachable || [],
-    }
   end
 
   def expand_direct_mentions(to_notify, already_covered_ids, skip)
@@ -231,11 +207,7 @@ class Chat::ChatNotifier
           .where.not(id: already_covered_ids)
     end
 
-    grouped = group_users_to_notify(direct_mentions)
+    to_notify[:direct_mentions] = select_channel_participants(direct_mentions).map(&:id)
-
-    to_notify[:direct_mentions] = grouped[:already_participating].map(&:id)
-    to_notify[:welcome_to_join] = grouped[:welcome_to_join]
-    to_notify[:unreachable] = grouped[:unreachable]
     already_covered_ids.concat(to_notify[:direct_mentions])
   end
 
@@ -256,16 +228,11 @@ class Chat::ChatNotifier
     mentionable_groups =
       Group.mentionable(@user, include_public: false).where(id: visible_groups.map(&:id))
 
-    mentions_disabled = visible_groups - mentionable_groups
+    mentionable =
-
+      mentionable_groups.reject do |group|
-    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 =
@@ -275,9 +242,7 @@ class Chat::ChatNotifier
         .where(groups: mentionable)
         .where.not(id: already_covered_ids)
 
-    grouped = group_users_to_notify(reached_by_group)
+    select_channel_participants(reached_by_group).each do |user|
-
-    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 }
@@ -287,29 +252,6 @@ class Chat::ChatNotifier
       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)
-    inaccessible =
-      to_notify.extract!(
-        :unreachable,
-        :welcome_to_join,
-        :too_many_members,
-        :group_mentions_disabled,
-      )
-    return if inaccessible.values.all?(&:blank?)
-
-    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
@@ -317,20 +259,11 @@ class Chat::ChatNotifier
   # 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 already_covered_ids.blank?
 
-    return if screen_targets.blank?
+    screener = UserCommScreener.new(acting_user: @user, target_user_ids: already_covered_ids)
-
+    to_notify.each do |key, user_ids|
-    screener = UserCommScreener.new(acting_user: @user, target_user_ids: screen_targets)
+      to_notify[key] = user_ids.reject { |user_id| screener.ignoring_or_muting_actor?(user_id) }
-    to_notify
-      .except(:unreachable, :welcome_to_join)
-      .each do |key, user_ids|
-        to_notify[key] = user_ids.reject { |user_id| screener.ignoring_or_muting_actor?(user_id) }
-      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|

plugins/chat/lib/message_mention_warnings.rb

@@ -0,0 +1,177 @@
+# frozen_string_literal: true
+
+# For various reasons, the sender may receive a warning when writing a mention:
+#
+# * The target user either cannot chat or cannot see the chat channel, in which case
+#   they are defined as `cannot_see`
+# * The target user is not a member of the channel, in which case they are defined
+#   as `without_membership`
+#
+# For any users that fall under the `cannot_see` or `without_membership` umbrellas
+# we send a MessageBus message to the UI and to inform the sender. The
+# creating user can invite any `without_membership` users to the channel. Target
+# users who are ignoring or muting the creating user _do not_ fall into this bucket.
+class Chat::MessageMentionWarnings
+  def dispatch(chat_message)
+    direct_mentions = direct_mentions_from(chat_message)
+    group_mentions = group_mentions_from(chat_message)
+
+    if (direct_mentions.length + group_mentions.length) > SiteSetting.max_mentions_per_chat_message
+      return
+    end
+
+    warnings = { without_membership: [], cannot_see: [] }
+
+    append_direct_mention_warnings(warnings, chat_message, direct_mentions)
+    append_group_mention_warnings(warnings, chat_message, direct_mentions, group_mentions)
+    filter_users_ignoring_or_muting_creator(warnings, chat_message)
+
+    notify_creator_of_inaccessible_mentions(warnings, chat_message)
+  end
+
+  private
+
+  def not_participating_base_query(message)
+    User
+      .distinct
+      .real
+      .not_suspended
+      .joins(:user_option)
+      .where(user_options: { chat_enabled: true })
+      .where.not(id: message.user_id)
+      .includes(:user_chat_channel_memberships)
+  end
+
+  def normalized_mentions(raw_mentions)
+    raw_mentions.reduce([]) do |memo, mention|
+      %w[@here @all].include?(mention.downcase) ? memo : (memo << mention[1..-1].downcase)
+    end
+  end
+
+  ### Direct mention warning methods
+
+  def direct_mentions_from(message)
+    normalized_mentions(Nokogiri::HTML5.fragment(message.cooked).css(".mention").map(&:text))
+  end
+
+  def direct_mentioned_users_not_participating(message, mentions)
+    not_participating_base_query(message)
+      .where.not(id: message.user_id)
+      .where(username_lower: mentions)
+  end
+
+  def append_direct_mention_warnings(warnings, message, mentions)
+    direct_mentioned_users_not_participating(message, mentions).each do |potential_participant|
+      guardian = Guardian.new(potential_participant)
+
+      if guardian.can_chat? && guardian.can_join_chat_channel?(message.chat_channel)
+        not_a_member =
+          potential_participant.user_chat_channel_memberships.none? do |m|
+            predicate = m.chat_channel_id == message.chat_channel_id
+            predicate = predicate && m.following == true if message.chat_channel.public_channel?
+            predicate
+          end
+
+        warnings[:without_membership] << potential_participant if not_a_member
+      else
+        warnings[:cannot_see] << potential_participant
+      end
+    end
+  end
+
+  ### Group mention warning methods
+
+  def group_mentions_from(message)
+    normalized_mentions(Nokogiri::HTML5.fragment(message.cooked).css(".mention-group").map(&:text))
+  end
+
+  def group_members_not_participating(mentionable_groups, message, direct_mentions)
+    not_participating_base_query(message)
+      .where.not(id: message.user_id)
+      .where.not(username_lower: direct_mentions)
+      .joins(:group_users)
+      .where(group_users: { group_id: mentionable_groups.map(&:id) })
+  end
+
+  def append_group_mention_warnings(warnings, message, direct_mentions, group_mentions)
+    visible_groups = Group.where("LOWER(name) IN (?)", group_mentions).visible_groups(message.user)
+
+    return if visible_groups.empty?
+
+    mentionable_groups =
+      Group.mentionable(message.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
+
+    warnings[:group_mentions_disabled] = mentions_disabled
+    warnings[:too_many_members] = too_many_members
+
+    group_members_not_participating(
+      mentionable,
+      message,
+      direct_mentions,
+    ).each do |potential_participant|
+      guardian = Guardian.new(potential_participant)
+
+      if guardian.can_chat? && guardian.can_join_chat_channel?(message.chat_channel)
+        not_a_member =
+          potential_participant.user_chat_channel_memberships.none? do |m|
+            predicate = m.chat_channel_id == message.chat_channel_id
+            predicate = predicate && m.following == true if message.chat_channel.public_channel?
+            predicate
+          end
+
+        warnings[:without_membership] << potential_participant if not_a_member
+      else
+        warnings[:cannot_see] << potential_participant
+      end
+    end
+  end
+
+  ### Apply ignore/mute filters
+
+  def filter_users_ignoring_or_muting_creator(warnings, message)
+    screen_targets = warnings[:without_membership].map(&:id)
+
+    return if screen_targets.blank?
+
+    screener = UserCommScreener.new(acting_user: message.user, target_user_ids: screen_targets)
+
+    warnings[:without_membership].reject! { |user| screener.ignoring_or_muting_actor?(user.id) }
+  end
+
+  ### Notify client
+
+  def notify_creator_of_inaccessible_mentions(warnings, message)
+    return if warnings.values.all?(&:blank?)
+
+    warnings_payload = [
+      inaccessible_mention_payload(warnings, :cannot_see) { |user| user.username },
+      inaccessible_mention_payload(warnings, :without_membership, include_ids: true) do |user|
+        user.username
+      end,
+      inaccessible_mention_payload(warnings, :too_many_members) { |group| group.name },
+      inaccessible_mention_payload(warnings, :group_mentions_disabled) { |group| group.name },
+    ].compact
+
+    ChatPublisher.publish_inaccessible_mentions(message.user_id, message, warnings_payload)
+  end
+
+  def inaccessible_mention_payload(warnings, type, include_ids: false)
+    return if warnings[type].blank?
+    payload = { type: type, mentions: [] }
+
+    payload[:mention_target_ids] = [] if include_ids
+
+    warnings[type].reduce(payload) do |memo, target|
+      memo[:mentions] << yield(target)
+      memo[:mention_target_ids] << target.id if include_ids
+      memo
+    end
+  end
+end

plugins/chat/plugin.rb

@@ -179,6 +179,7 @@ after_initialize do
   load File.expand_path("../lib/chat_message_rate_limiter.rb", __FILE__)
   load File.expand_path("../lib/chat_message_reactor.rb", __FILE__)
   load File.expand_path("../lib/chat_notifier.rb", __FILE__)
+  load File.expand_path("../lib/message_mention_warnings.rb", __FILE__)
   load File.expand_path("../lib/chat_seeder.rb", __FILE__)
   load File.expand_path("../lib/chat_statistics.rb", __FILE__)
   load File.expand_path("../lib/chat_transcript_service.rb", __FILE__)

plugins/chat/spec/lib/chat_notifier_spec.rb

@@ -348,7 +348,7 @@ describe Chat::ChatNotifier do
           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
+        it "does not send notifications to the uster 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)
@@ -361,275 +361,5 @@ describe Chat::ChatNotifier do
         end
       end
     end
-
-    describe "unreachable users" do
-      fab!(:user_3) { Fabricate(:user) }
-
-      it "notify poster of users who are not allowed to use chat" 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
-
-            expect(to_notify[:direct_mentions]).to be_empty
-          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
-
-      context "when in a personal message" do
-        let(:personal_chat_channel) do
-          Group.refresh_automatic_groups!
-          Chat::DirectMessageChannelCreator.create!(
-            acting_user: user_1,
-            target_users: [user_1, user_2],
-          )
-        end
-
-        before { @chat_group.add(user_3) }
-
-        it "notify posts of users who are not participating in a personal message" do
-          msg =
-            build_cooked_msg(
-              "Hello @#{user_3.username}",
-              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[:direct_mentions]).to be_empty
-            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
-
-          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
-      end
-    end
-
-    describe "users who can be invited to join the channel" do
-      fab!(:user_3) { Fabricate(:user) }
-
-      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
-
-            expect(to_notify[:direct_mentions]).to be_empty
-          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 "cannot invite chat user without channel membership if they are ignoring the user who created the message" do
-        Fabricate(:ignored_user, user: user_3, ignored_user: user_1)
-        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
-
-            expect(to_notify[:direct_mentions]).to be_empty
-          end
-
-        expect(messages).to be_empty
-      end
-
-      it "cannot invite chat user without channel membership if they are muting the user who created the message" do
-        Fabricate(:muted_user, user: user_3, muted_user: user_1)
-        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
-
-            expect(to_notify[:direct_mentions]).to be_empty
-          end
-
-        expect(messages).to be_empty
-      end
-
-      it "can invite chat user who no longer follows the channel" do
-        Fabricate(
-          :user_chat_channel_membership,
-          chat_channel: channel,
-          user: user_3,
-          following: false,
-        )
-        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
-
-            expect(to_notify[:direct_mentions]).to be_empty
-          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
-
-        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 "cannot invite a member of a group who is ignoring the user who created the message" do
-        group =
-          Fabricate(
-            :public_group,
-            users: [user_2, user_3],
-            mentionable_level: Group::ALIAS_LEVELS[:everyone],
-          )
-        Fabricate(:ignored_user, user: user_3, ignored_user: user_1, expiring_at: 1.day.from_now)
-        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
-
-        expect(messages).to be_empty
-      end
-
-      it "cannot invite a member of a group who is muting the user who created the message" do
-        group =
-          Fabricate(
-            :public_group,
-            users: [user_2, user_3],
-            mentionable_level: Group::ALIAS_LEVELS[:everyone],
-          )
-        Fabricate(:muted_user, user: user_3, muted_user: user_1)
-        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
-
-        expect(messages).to be_empty
-      end
-    end
-
-    describe "enforcing limits when mentioning groups" do
-      fab!(:user_3) { Fabricate(:user) }
-      fab!(:group) do
-        Fabricate(
-          :public_group,
-          users: [user_2, user_3],
-          mentionable_level: Group::ALIAS_LEVELS[:everyone],
-        )
-      end
-
-      it "sends a message to the client signaling the group has too many members" do
-        SiteSetting.max_users_notified_per_group_mention = (group.user_count - 1)
-        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[group.name]).to be_nil
-          end
-
-        too_many_members_msg = messages.first
-        expect(too_many_members_msg).to be_present
-        too_many_members = too_many_members_msg.data[:groups_with_too_many_members]
-        expect(too_many_members).to contain_exactly(group.name)
-      end
-
-      it "sends a message to the client signaling the group doesn't allow mentions" do
-        group.update!(mentionable_level: Group::ALIAS_LEVELS[:only_admins])
-        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[group.name]).to be_nil
-          end
-
-        mentions_disabled_msg = messages.first
-        expect(mentions_disabled_msg).to be_present
-        mentions_disabled = mentions_disabled_msg.data[:group_mentions_disabled]
-        expect(mentions_disabled).to contain_exactly(group.name)
-      end
-    end
   end
 end

plugins/chat/spec/lib/message_mention_warnings_spec.rb

@@ -0,0 +1,243 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe Chat::MessageMentionWarnings do
+  def build_cooked_msg(message_body, user, chat_channel: channel)
+    ChatMessage.new(
+      chat_channel: chat_channel,
+      user: user,
+      message: message_body,
+      created_at: 5.minutes.ago,
+    ).tap(&:cook)
+  end
+
+  fab!(:channel) { Fabricate(:category_channel) }
+  fab!(:user_1) { Fabricate(:user) }
+  fab!(:user_2) { Fabricate(:user) }
+  fab!(:user_3) { Fabricate(:user) }
+  fab!(:chat_group) do
+    Fabricate(:group, users: [user_1, user_2], mentionable_level: Group::ALIAS_LEVELS[:everyone])
+  end
+
+  before do
+    SiteSetting.chat_allowed_groups = chat_group.id
+
+    [user_1, user_2].each do |u|
+      Fabricate(:user_chat_channel_membership, chat_channel: channel, user: u)
+    end
+  end
+
+  describe "#dispatch" do
+    context "when mentioned users are unreachable" do
+      it "notify poster of users who are not allowed to use chat" do
+        msg = build_cooked_msg("Hello @#{user_3.username}", user_1)
+
+        messages = MessageBus.track_publish("/chat/#{channel.id}") { subject.dispatch(msg) }
+
+        unreachable_msg = messages.first.data[:warnings].detect { |m| m[:type] == :cannot_see }
+
+        expect(unreachable_msg).to be_present
+        unreachable_users = unreachable_msg[:mentions]
+        expect(unreachable_users).to contain_exactly(user_3.username)
+      end
+
+      context "when in a personal message" do
+        let(:personal_chat_channel) do
+          Group.refresh_automatic_groups!
+          Chat::DirectMessageChannelCreator.create!(
+            acting_user: user_1,
+            target_users: [user_1, user_2],
+          )
+        end
+
+        before { chat_group.add(user_3) }
+
+        it "notify posts of users who are not participating in a personal message" do
+          msg =
+            build_cooked_msg(
+              "Hello @#{user_3.username}",
+              user_1,
+              chat_channel: personal_chat_channel,
+            )
+
+          messages =
+            MessageBus.track_publish("/chat/#{personal_chat_channel.id}") { subject.dispatch(msg) }
+
+          unreachable_msg = messages.first.data[:warnings].detect { |m| m[:type] == :cannot_see }
+
+          expect(unreachable_msg).to be_present
+          unreachable_users = unreachable_msg[:mentions]
+          expect(unreachable_users).to contain_exactly(user_3.username)
+        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}") { subject.dispatch(msg) }
+
+          unreachable_msg = messages.first.data[:warnings].detect { |m| m[:type] == :cannot_see }
+
+          expect(unreachable_msg).to be_present
+          unreachable_users = unreachable_msg[:mentions]
+          expect(unreachable_users).to contain_exactly(user_3.username)
+        end
+      end
+    end
+
+    context "when we can invite mentioned users to the channel" do
+      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}") { subject.dispatch(msg) }
+
+        non_participating_msg =
+          messages.first.data[:warnings].detect { |m| m[:type] == :without_membership }
+
+        expect(non_participating_msg).to be_present
+        non_participating_usernames = non_participating_msg[:mentions]
+        expect(non_participating_usernames).to contain_exactly(user_3.username)
+        non_participating_user_ids = non_participating_msg[:mention_target_ids]
+        expect(non_participating_user_ids).to contain_exactly(user_3.id)
+      end
+
+      it "cannot invite chat user without channel membership if they are ignoring the user who created the message" do
+        Fabricate(:ignored_user, user: user_3, ignored_user: user_1)
+        msg = build_cooked_msg("Hello @#{user_3.username}", user_1)
+
+        messages = MessageBus.track_publish("/chat/#{channel.id}") { subject.dispatch(msg) }
+
+        expect(messages).to be_empty
+      end
+
+      it "cannot invite chat user without channel membership if they are muting the user who created the message" do
+        Fabricate(:muted_user, user: user_3, muted_user: user_1)
+        msg = build_cooked_msg("Hello @#{user_3.username}", user_1)
+
+        messages = MessageBus.track_publish("/chat/#{channel.id}") { subject.dispatch(msg) }
+
+        expect(messages).to be_empty
+      end
+
+      it "can invite chat user who no longer follows the channel" do
+        Fabricate(
+          :user_chat_channel_membership,
+          chat_channel: channel,
+          user: user_3,
+          following: false,
+        )
+        msg = build_cooked_msg("Hello @#{user_3.username}", user_1)
+
+        messages = MessageBus.track_publish("/chat/#{channel.id}") { subject.dispatch(msg) }
+
+        not_participating_msg =
+          messages.first.data[:warnings].detect { |m| m[:type] == :without_membership }
+
+        expect(not_participating_msg).to be_present
+        non_participating_usernames = not_participating_msg[:mentions]
+        expect(non_participating_usernames).to contain_exactly(user_3.username)
+        non_participating_user_ids = not_participating_msg[:mention_target_ids]
+        expect(non_participating_user_ids).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}") { subject.dispatch(msg) }
+
+        not_participating_msg =
+          messages.first.data[:warnings].detect { |m| m[:type] == :without_membership }
+
+        expect(not_participating_msg).to be_present
+        non_participating_usernames = not_participating_msg[:mentions]
+        expect(non_participating_usernames).to contain_exactly(user_3.username)
+        non_participating_user_ids = not_participating_msg[:mention_target_ids]
+        expect(non_participating_user_ids).to contain_exactly(user_3.id)
+      end
+
+      it "cannot invite a member of a group who is ignoring the user who created the message" do
+        group =
+          Fabricate(
+            :public_group,
+            users: [user_2, user_3],
+            mentionable_level: Group::ALIAS_LEVELS[:everyone],
+          )
+        Fabricate(:ignored_user, user: user_3, ignored_user: user_1, expiring_at: 1.day.from_now)
+        msg = build_cooked_msg("Hello @#{group.name}", user_1)
+
+        messages = MessageBus.track_publish("/chat/#{channel.id}") { subject.dispatch(msg) }
+
+        expect(messages).to be_empty
+      end
+
+      it "cannot invite a member of a group who is muting the user who created the message" do
+        group =
+          Fabricate(
+            :public_group,
+            users: [user_2, user_3],
+            mentionable_level: Group::ALIAS_LEVELS[:everyone],
+          )
+        Fabricate(:muted_user, user: user_3, muted_user: user_1)
+        msg = build_cooked_msg("Hello @#{group.name}", user_1)
+
+        messages = MessageBus.track_publish("/chat/#{channel.id}") { subject.dispatch(msg) }
+
+        expect(messages).to be_empty
+      end
+    end
+
+    describe "enforcing limits when mentioning groups" do
+      fab!(:group) do
+        Fabricate(
+          :public_group,
+          users: [user_2, user_3],
+          mentionable_level: Group::ALIAS_LEVELS[:everyone],
+        )
+      end
+
+      it "sends a message to the client signaling the group has too many members" do
+        SiteSetting.max_users_notified_per_group_mention = (group.user_count - 1)
+        msg = build_cooked_msg("Hello @#{group.name}", user_1)
+
+        messages = MessageBus.track_publish("/chat/#{channel.id}") { subject.dispatch(msg) }
+
+        too_many_members_msg =
+          messages.first.data[:warnings].detect { |m| m[:type] == :too_many_members }
+
+        expect(too_many_members_msg).to be_present
+        too_many_members = too_many_members_msg[:mentions]
+        expect(too_many_members).to contain_exactly(group.name)
+      end
+
+      it "sends a message to the client signaling the group doesn't allow mentions" do
+        group.update!(mentionable_level: Group::ALIAS_LEVELS[:only_admins])
+        msg = build_cooked_msg("Hello @#{group.name}", user_1)
+
+        messages = MessageBus.track_publish("/chat/#{channel.id}") { subject.dispatch(msg) }
+
+        mentions_disabled_msg =
+          messages.first.data[:warnings].detect { |m| m[:type] == :group_mentions_disabled }
+
+        expect(mentions_disabled_msg).to be_present
+        mentions_disabled = mentions_disabled_msg[:mentions]
+        expect(mentions_disabled).to contain_exactly(group.name)
+      end
+    end
+  end
+end

plugins/chat/spec/system/post_send_message_warnings_spec.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-RSpec.describe "JIT messages", type: :system, js: true do
+RSpec.describe "Post-send message warnings for mentions", type: :system, js: true do
   fab!(:channel_1) { Fabricate(:chat_channel) }
   fab!(:current_user) { Fabricate(:user) }
   fab!(:other_user) { Fabricate(:user) }
@@ -16,14 +16,13 @@ RSpec.describe "JIT messages", type: :system, js: true do
 
   context "when mentioning a user not on the channel" do
     it "displays a mention warning" do
-      Jobs.run_immediately!
-
       chat.visit_channel(channel_1)
+
       channel.send_message("hi @#{other_user.username}")
 
       expect(page).to have_content(
-        I18n.t("js.chat.mention_warning.without_membership.one", username: other_user.username),
+        I18n.t("js.chat.mention_warning.without_membership.one", mention: other_user.username),
-        wait: 5,
+        wait: 10,
       )
     end
   end
@@ -38,14 +37,13 @@ RSpec.describe "JIT messages", type: :system, js: true do
     end
 
     it "displays a mention warning" do
-      Jobs.run_immediately!
-
       chat.visit_channel(private_channel_1)
+
       channel.send_message("hi @#{other_user.username}")
 
       expect(page).to have_content(
-        I18n.t("js.chat.mention_warning.cannot_see.one", username: other_user.username),
+        I18n.t("js.chat.mention_warning.cannot_see.one", mention: other_user.username),
-        wait: 5,
+        wait: 10,
       )
     end
   end
@@ -55,14 +53,13 @@ RSpec.describe "JIT messages", type: :system, js: true do
       fab!(:group_1) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:nobody]) }
 
       it "displays a mention warning" do
-        Jobs.run_immediately!
-
         chat.visit_channel(channel_1)
+
         channel.send_message("hi @#{group_1.name}")
 
         expect(page).to have_content(
-          I18n.t("js.chat.mention_warning.group_mentions_disabled.one", group_name: group_1.name),
+          I18n.t("js.chat.mention_warning.group_mentions_disabled.one", mention: group_1.name),
-          wait: 5,
+          wait: 10,
         )
       end
     end