PERF: Make chat mention notifications async. (#19666)

This PR removes the limit added to max_users_notified_per_group_mention during #19034 and improve the performance when expanding mentions for large channel or groups by removing some N+1 queries and making the whole process async.

* Fully async chat message notifications

* Remove mention setting limit and get rid of N+1 queries

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/send_message_notifications.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Jobs
+  class SendMessageNotifications < ::Jobs::Base
+    def execute(args)
+      reason = args[:reason]
+      valid_reasons = %w[new edit]
+      return unless valid_reasons.include?(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/lib/chat_notifier.rb

@@ -37,11 +37,21 @@ class Chat::ChatNotifier
     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
 
@@ -123,10 +133,8 @@ class Chat::ChatNotifier
   end
 
   def chat_users
-    users =
+    User
-      User.includes(:do_not_disturb_timings, :push_subscriptions, :user_chat_channel_memberships)
+      .includes(:user_chat_channel_memberships, :group_users)
-
-    users
       .distinct
       .joins("LEFT OUTER JOIN user_chat_channel_memberships uccm ON uccm.user_id = users.id")
       .joins(:user_option)
@@ -265,7 +273,9 @@ class Chat::ChatNotifier
     mentionable.each { |g| to_notify[g.name.downcase] = [] }
 
     reached_by_group =
-      chat_users.joins(:groups).where(groups: mentionable).where.not(id: already_covered_ids)
+      chat_users
+        .includes(:groups)
+        .joins(:groups).where(groups: mentionable).where.not(id: already_covered_ids)
 
     grouped = group_users_to_notify(reached_by_group)
 
@@ -333,7 +343,7 @@ class Chat::ChatNotifier
         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),
+        timestamp: @timestamp,
       },
     )
   end
@@ -344,7 +354,7 @@ class Chat::ChatNotifier
       {
         chat_message_id: @chat_message.id,
         except_user_ids: except,
-        timestamp: @timestamp.iso8601(6),
+        timestamp: @timestamp,
       },
     )
   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/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: "new"
+        )
+      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/system/jit_messages_spec.rb

@@ -9,6 +9,7 @@ RSpec.describe "JIT messages", type: :system, js: true do
   let(:channel) { PageObjects::Pages::ChatChannel.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