From 7f7e7fe5163e3036334b7cb3647369f3d65f0d28 Mon Sep 17 00:00:00 2001 From: David Battersby Date: Wed, 13 Sep 2023 19:33:22 +0800 Subject: [PATCH] Revert "FEATURE: Add chat message notifications for personal chats (#23307)" (#23559) This reverts commit 0a1a07fff8e199aafba551a80e67e17019d5f390. --- .../discourse/initializers/chat-user-menu.js | 55 --------------- plugins/chat/config/locales/client.en.yml | 7 -- .../notification_consolidation_extension.rb | 66 ------------------ plugins/chat/lib/chat/notifier.rb | 44 ------------ plugins/chat/plugin.rb | 5 -- plugins/chat/spec/lib/chat/notifier_spec.rb | 67 ------------------- 6 files changed, 244 deletions(-) delete mode 100644 plugins/chat/lib/chat/notification_consolidation_extension.rb diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js index 6ed8440431e..b2c0d4ca8c5 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js @@ -105,60 +105,6 @@ export default { }; } ); - - api.registerNotificationTypeRenderer( - "chat_message", - (NotificationItemBase) => { - return class extends NotificationItemBase { - linkTitle = I18n.t("notifications.titles.chat_message"); - icon = "comment"; - - get linkHref() { - const slug = slugifyChannel({ - title: this.notification.data.chat_channel_title, - slug: this.notification.data.chat_channel_slug, - }); - return `/chat/c/${slug || "-"}/${ - this.notification.data.chat_channel_id - }/${this.notification.data.chat_message_id}`; - } - - get label() { - return formatUsername(this.notification.data.username); - } - - get description() { - const data = this.notification.data; - - if (!data.is_group_message) { - return I18n.t("notifications.chat_message.personal", { - username: data.username, - }); - } - - if (!data.username2) { - return I18n.t("notifications.chat_message.group", { - username: data.username, - }); - } - - if (data.user_ids.length > 2) { - return I18n.t("notifications.chat_message.group_multiple", { - username: data.username, - count: data.user_ids.length - 1, - }); - } else { - // only 2 users so we show the second username - return I18n.t("notifications.chat_message.group_multiple", { - username: data.username, - username2: data.username2, - count: 1, - }); - } - } - }; - } - ); } if (api.registerUserMenuTab) { @@ -179,7 +125,6 @@ export default { get count() { return ( this.getUnreadCountForType("chat_mention") + - this.getUnreadCountForType("chat_message") + this.getUnreadCountForType("chat_invitation") ); } diff --git a/plugins/chat/config/locales/client.en.yml b/plugins/chat/config/locales/client.en.yml index 3b42cc0292a..4b36fb9d635 100644 --- a/plugins/chat/config/locales/client.en.yml +++ b/plugins/chat/config/locales/client.en.yml @@ -595,12 +595,6 @@ en: chat_invitation: "invited you to join a chat channel" chat_invitation_html: "%{username} invited you to join a chat channel" chat_quoted: "%{username} %{description}" - chat_message: - personal: "messaged you in a personal chat" - group: "messaged you in a group personal chat" - group_multiple: - one: "and %{username2} messaged you in a group personal chat" - other: "and %{count} others messaged you in a group personal chat" popup: chat_mention: @@ -622,7 +616,6 @@ en: chat_mention: "Chat mention" chat_invitation: "Chat invitation" chat_quoted: "Chat quoted" - chat_message: "Chat message" action_codes: chat: enabled: '%{who} enabled %{when}' diff --git a/plugins/chat/lib/chat/notification_consolidation_extension.rb b/plugins/chat/lib/chat/notification_consolidation_extension.rb deleted file mode 100644 index 8c0f3f7bc34..00000000000 --- a/plugins/chat/lib/chat/notification_consolidation_extension.rb +++ /dev/null @@ -1,66 +0,0 @@ -# frozen_string_literal: true - -module Chat - module NotificationConsolidationExtension - CONSOLIDATION_WINDOW = 60.minutes - CONSOLIDATION_THRESHOLD = 1 - - def self.chat_message_plan - Notifications::ConsolidateNotifications.new( - from: Notification.types[:chat_message], - to: Notification.types[:chat_message], - threshold: CONSOLIDATION_THRESHOLD, - consolidation_window: CONSOLIDATION_WINDOW, - unconsolidated_query_blk: - Proc.new do |notifications, data| - notifications.where("data::json ->> 'consolidated' IS NULL").where( - "data::json ->> 'chat_channel_id' = ?", - data[:chat_channel_id].to_s, - ) - end, - consolidated_query_blk: - Proc.new do |notifications, data| - notifications.where("(data::json ->> 'consolidated')::bool").where( - "data::json ->> 'chat_channel_id' = ?", - data[:chat_channel_id].to_s, - ) - end, - ).set_mutations( - set_data_blk: - lambda do |notification| - data = notification.data_hash - - last_chat_message_notification = - Notification - .where(user_id: notification.user_id) - .order("notifications.id DESC") - .where("data::json ->> 'chat_channel_id' = ?", data[:chat_channel_id].to_s) - .where(notification_type: Notification.types[:chat_message]) - .where("created_at > ?", CONSOLIDATION_WINDOW.ago) - .first - - return data if !last_chat_message_notification - - consolidated_data = last_chat_message_notification.data_hash - - if data[:last_read_message_id].to_i <= consolidated_data[:chat_message_id].to_i - data[:chat_message_id] = consolidated_data[:chat_message_id] - end - - if !consolidated_data[:username2] && data[:username] != consolidated_data[:username] - data.merge( - username2: consolidated_data[:username], - user_ids: consolidated_data[:user_ids].concat(data[:user_ids]), - ) - else - data.merge( - username: consolidated_data[:username], - username2: consolidated_data[:username2], - user_ids: (consolidated_data[:user_ids].concat(data[:user_ids])).uniq, - ) - end - end, - ) - end - end -end diff --git a/plugins/chat/lib/chat/notifier.rb b/plugins/chat/lib/chat/notifier.rb index f94100c5c73..0bc4ab7a765 100644 --- a/plugins/chat/lib/chat/notifier.rb +++ b/plugins/chat/lib/chat/notifier.rb @@ -78,8 +78,6 @@ module Chat notify_mentioned_users(to_notify) notify_watching_users(except: all_mentioned_user_ids << @user.id) - notify_personal_chat_users(to_notify, except: all_mentioned_user_ids << @user.id) - to_notify end @@ -123,48 +121,6 @@ module Chat [to_notify, inaccessible, all_mentioned_user_ids] end - def notify_personal_chat_users(to_notify, except: []) - return if !@chat_channel.direct_message_channel? - notify_user_ids = - User.where(id: @chat_channel.allowed_user_ids).not_suspended.pluck(:id) - except - notified_user_ids = [] - - notify_user_ids.each do |user_id| - membership = @chat_channel.membership_for(user_id) - next if !membership || membership.muted? - - screener = UserCommScreener.new(acting_user: @user, target_user_ids: user_id) - next if screener.ignoring_or_muting_actor?(user_id) - - notified_user_ids << user_id - create_notification_for(membership, notification_type: Notification.types[:chat_message]) - end - - to_notify[:direct_messages] = notified_user_ids - end - - def create_notification_for(membership, notification_type:) - if notification_type == Notification.types[:chat_message] - data = { - username: @user.username, - chat_message_id: @chat_message.id, - chat_channel_id: @chat_channel.id, - last_read_message_id: membership&.last_read_message_id, - is_direct_message_channel: @chat_channel.direct_message_channel?, - is_group_message: @chat_channel.allowed_user_ids.size > 2, - user_ids: [@user.id], - } - - data[:chat_thread_id] = @chat_message.thread_id if @chat_message.in_thread? - end - - Notification.consolidate_or_create!( - notification_type: notification_type, - user_id: membership.user_id, - data: data.to_json, - ) - end - def expand_global_mention(to_notify, already_covered_ids) has_all_mention = @parsed_mentions.has_global_mention diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index 3cee9df58f5..1a9789b85b9 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -59,7 +59,6 @@ after_initialize do Guardian.prepend Chat::GuardianExtensions UserNotifications.prepend Chat::UserNotificationsExtension - Notifications::ConsolidationPlan.prepend Chat::NotificationConsolidationExtension UserOption.prepend Chat::UserOptionExtension Category.prepend Chat::CategoryExtension Reviewable.prepend Chat::ReviewableExtension @@ -473,10 +472,6 @@ after_initialize do ) register_bookmarkable(Chat::MessageBookmarkable) - - register_notification_consolidation_plan( - Chat::NotificationConsolidationExtension.chat_message_plan, - ) end if Rails.env == "test" diff --git a/plugins/chat/spec/lib/chat/notifier_spec.rb b/plugins/chat/spec/lib/chat/notifier_spec.rb index 7e3761d7656..88c9ddde707 100644 --- a/plugins/chat/spec/lib/chat/notifier_spec.rb +++ b/plugins/chat/spec/lib/chat/notifier_spec.rb @@ -689,72 +689,5 @@ describe Chat::Notifier do ) end end - - describe "personal chat messages" do - fab!(:dm_channel) { Fabricate(:direct_message_channel) } - - before do - dm_user_ids = dm_channel.allowed_user_ids - @dm_user_1 = User.find(dm_user_ids.first) - @dm_user_2 = User.find(dm_user_ids.last) - end - - it "notifies the other user when a new message is sent" do - msg = build_cooked_msg("Hey guys", @dm_user_1, chat_channel: dm_channel) - to_notify = described_class.new(msg, msg.created_at).notify_new - - expect(to_notify[:direct_messages]).to contain_exactly(@dm_user_2.id) - end - - it "does not notify users who have muted the chat channel" do - dm_channel.membership_for(@dm_user_1.id).update!(muted: true) - msg = build_cooked_msg("How are you?", @dm_user_2, chat_channel: dm_channel) - - expect { described_class.new(msg, msg.created_at).notify_new }.not_to change { - @dm_user_1.notifications.count - } - end - - it "does not notify users who have muted the other user" do - Fabricate(:muted_user, user: @dm_user_1, muted_user: @dm_user_2) - msg = build_cooked_msg("How are you?", @dm_user_2, chat_channel: dm_channel) - - expect { described_class.new(msg, msg.created_at).notify_new }.not_to change { - @dm_user_1.notifications.count - } - end - - it "does not notify users who have ignored the other user" do - Fabricate(:ignored_user, user: @dm_user_1, ignored_user: @dm_user_2) - msg = build_cooked_msg("How are you?", @dm_user_2, chat_channel: dm_channel) - - expect { described_class.new(msg, msg.created_at).notify_new }.not_to change { - @dm_user_1.notifications.count - } - end - - it "does not notify users who are suspended" do - @dm_user_1.update!(suspended_till: 2.years.from_now) - msg = build_cooked_msg("How are you?", @dm_user_2, chat_channel: dm_channel) - - expect { described_class.new(msg, msg.created_at).notify_new }.not_to change { - @dm_user_1.notifications.count - } - end - - it "adds correct data to the notification" do - msg = build_cooked_msg("Hey guys", @dm_user_1, chat_channel: dm_channel) - to_notify = described_class.new(msg, msg.created_at).notify_new - notification = Notification.where(user: @dm_user_2).first - data = notification.data_hash - - expect(data[:username]).to eq(@dm_user_1.username) - expect(data[:chat_channel_id]).to eq(dm_channel.id) - expect(data[:chat_message_id]).to eq(msg.id) - expect(data[:is_direct_message_channel]).to eq(true) - expect(data[:is_group_message]).to eq(false) - expect(data[:user_ids]).to eq([@dm_user_1.id]) - end - end end end