FIX: simplify desktop notifications behavior (#29957)

Historically the behavior of this file has been complexified to attempt to answer this use case:

A user has two tabs open, tab 1 is on a topic, tab 2 is on a chat channel. If your active tab is tab 1 and someones sends you a mention in chat. We will show a desktop notification, but in which tab the channel should open if you click it? The changes made years ago said: in tab 2.

I think this is complexifying too much this codepath and is also confusing. You might wonder why this discourse notification you clicked opened in some of your 50 tabs in the background when you had a discourse tab active currently in front of you.

Moreover, a recent change has made the notification to only happen on desktop, but all the subscription stuff was happening regardless of mobile or desktop.

plugins/chat/assets/javascripts/discourse/services/chat-notification-manager.js

@@ -3,165 +3,57 @@ import {
   alertChannel,
   onNotification as onDesktopNotification,
 } from "discourse/lib/desktop-notifications";
-import { withPluginApi } from "discourse/lib/plugin-api";
 import { isTesting } from "discourse-common/config/environment";
 import { bind } from "discourse-common/utils/decorators";
 
 export default class ChatNotificationManager extends Service {
-  @service presence;
   @service chat;
-  @service chatChannelsManager;
-  @service chatStateManager;
   @service currentUser;
   @service appEvents;
   @service site;
 
-  _subscribedToCore = true;
-  _subscribedToChat = false;
-  _countChatInDocTitle = true;
-
   willDestroy() {
     super.willDestroy(...arguments);
 
-    if (!this._shouldRun()) {
+    if (!this.#shouldRun) {
       return;
     }
 
-    this._chatPresenceChannel.off(
+    this.messageBus.unsubscribe(this.messageBusChannel, this.onMessage);
-      "change",
-      this._subscribeToCorrectNotifications
-    );
-    this._chatPresenceChannel.unsubscribe();
-    this._chatPresenceChannel.leave();
-
-    this._corePresenceChannel.off(
-      "change",
-      this._subscribeToCorrectNotifications
-    );
-    this._corePresenceChannel.unsubscribe();
-    this._corePresenceChannel.leave();
   }
 
   start() {
-    if (!this._shouldRun()) {
+    if (!this.#shouldRun) {
       return;
     }
 
-    this.set(
+    this.messageBus.subscribe(this.messageBusChannel, this.onMessage);
-      "_chatPresenceChannel",
-      this.presence.getChannel(`/chat-user/chat/${this.currentUser.id}`)
-    );
-    this.set(
-      "_corePresenceChannel",
-      this.presence.getChannel(`/chat-user/core/${this.currentUser.id}`)
-    );
-    this._chatPresenceChannel.subscribe();
-    this._corePresenceChannel.subscribe();
-
-    withPluginApi("0.12.1", (api) => {
-      api.onPageChange(this._pageChanged);
-    });
-
-    this._pageChanged();
-
-    this._chatPresenceChannel.on(
-      "change",
-      this._subscribeToCorrectNotifications
-    );
-    this._corePresenceChannel.on(
-      "change",
-      this._subscribeToCorrectNotifications
-    );
   }
 
-  shouldCountChatInDocTitle() {
+  get messageBusChannel() {
-    return this._countChatInDocTitle;
-  }
-
-  @bind
-  _pageChanged() {
-    if (this.chatStateManager.isActive) {
-      this._chatPresenceChannel.enter({ onlyWhileActive: false });
-      this._corePresenceChannel.leave();
-    } else {
-      this._chatPresenceChannel.leave();
-      this._corePresenceChannel.enter({ onlyWhileActive: false });
-    }
-  }
-
-  _coreAlertChannel() {
-    return alertChannel(this.currentUser);
-  }
-
-  _chatAlertChannel() {
     return `/chat${alertChannel(this.currentUser)}`;
   }
 
-  @bind
-  _subscribeToCorrectNotifications() {
-    const oneTabForEachOpen =
-      this._chatPresenceChannel.count > 0 &&
-      this._corePresenceChannel.count > 0;
-    if (oneTabForEachOpen) {
-      this.chatStateManager.isActive
-        ? this._subscribeToChat({ only: true })
-        : this._subscribeToCore({ only: true });
-    } else {
-      this._subscribeToBoth();
-    }
-  }
-
-  _subscribeToBoth() {
-    this._subscribeToChat();
-    this._subscribeToCore();
-  }
-
-  _subscribeToChat(opts = { only: false }) {
-    this.set("_countChatInDocTitle", true);
-
-    if (!this._subscribedToChat) {
-      this.messageBus.subscribe(this._chatAlertChannel(), this.onMessage);
-      this.set("_subscribedToChat", true);
-    }
-
-    if (opts.only && this._subscribedToCore) {
-      this.messageBus.unsubscribe(this._coreAlertChannel(), this.onMessage);
-      this.set("_subscribedToCore", false);
-    }
-  }
-
-  _subscribeToCore(opts = { only: false }) {
-    if (opts.only) {
-      this.set("_countChatInDocTitle", false);
-    }
-    if (!this._subscribedToCore) {
-      this.messageBus.subscribe(this._coreAlertChannel(), this.onMessage);
-      this.set("_subscribedToCore", true);
-    }
-
-    if (opts.only && this._subscribedToChat) {
-      this.messageBus.unsubscribe(this._chatAlertChannel(), this.onMessage);
-      this.set("_subscribedToChat", false);
-    }
-  }
-
   @bind
   async onMessage(data) {
-    if (data.channel_id === this.chat.activeChannel?.id) {
+    // if the user is currently focused on this tab and channel,
+    // we don't want to show a desktop notification
+    if (
+      this.session.hasFocus &&
+      data.channel_id === this.chat.activeChannel?.id
+    ) {
       return;
     }
 
-    if (this.site.desktopView) {
+    return onDesktopNotification(
-      return onDesktopNotification(
+      data,
-        data,
+      this.siteSettings,
-        this.siteSettings,
+      this.currentUser,
-        this.currentUser,
+      this.appEvents
-        this.appEvents
+    );
-      );
-    }
   }
 
-  _shouldRun() {
+  get #shouldRun() {
-    return this.chat.userCanChat && !isTesting();
+    return this.site.desktopView && this.chat.userCanChat && !isTesting();
   }
 }

plugins/chat/assets/javascripts/discourse/services/chat.js

@@ -273,9 +273,7 @@ export default class Chat extends Service {
   }
 
   getDocumentTitleCount() {
-    return this.chatNotificationManager.shouldCountChatInDocTitle()
+    return this.chatTrackingStateManager.allChannelUrgentCount;
-      ? this.chatTrackingStateManager.allChannelUrgentCount
-      : 0;
   }
 
   switchChannelUpOrDown(direction, unreadOnly = false) {

plugins/chat/plugin.rb

@@ -331,17 +331,6 @@ after_initialize do
     nil
   end
 
-  register_presence_channel_prefix("chat-user") do |channel_name|
-    if user_id = channel_name[%r{/chat-user/(chat|core)/(\d+)}, 2]
-      user = User.find(user_id)
-      config = PresenceChannel::Config.new
-      config.allowed_user_ids = [user.id]
-      config
-    end
-  rescue ActiveRecord::RecordNotFound
-    nil
-  end
-
   register_push_notification_filter do |user, payload|
     if user.user_option.only_chat_push_notifications && user.user_option.chat_enabled
       payload[:notification_type].in?(::Notification.types.values_at(:chat_mention, :chat_message))