diff --git a/plugins/chat/app/models/chat/direct_message_channel.rb b/plugins/chat/app/models/chat/direct_message_channel.rb index 30077776344..ede478a9713 100644 --- a/plugins/chat/app/models/chat/direct_message_channel.rb +++ b/plugins/chat/app/models/chat/direct_message_channel.rb @@ -21,7 +21,7 @@ module Chat end def generate_auto_slug - false if !self.slug.present? + self.slug.blank? end end end diff --git a/plugins/chat/app/models/chat/mention_notification.rb b/plugins/chat/app/models/chat/mention_notification.rb index 62c3a254341..0f4f90b9c8c 100644 --- a/plugins/chat/app/models/chat/mention_notification.rb +++ b/plugins/chat/app/models/chat/mention_notification.rb @@ -4,7 +4,7 @@ module Chat class MentionNotification < ActiveRecord::Base self.table_name = "chat_mention_notifications" - belongs_to :chat_mention + belongs_to :chat_mention, class_name: "Chat::Mention" belongs_to :notification, dependent: :destroy end end diff --git a/plugins/chat/app/views/user_notifications/chat_summary.html.erb b/plugins/chat/app/views/user_notifications/chat_summary.html.erb index 8379fe57b28..f05863cd9ce 100644 --- a/plugins/chat/app/views/user_notifications/chat_summary.html.erb +++ b/plugins/chat/app/views/user_notifications/chat_summary.html.erb @@ -2,7 +2,6 @@
- <%- if logo_url.blank? %> <%= SiteSetting.title %> @@ -10,73 +9,68 @@ <%= SiteSetting.title %> <%- end %> -
- <%= I18n.t("user_notifications.chat_summary.description", count: @messages.size) %> + <%= I18n.t("user_notifications.chat_summary.description", count: @count) %>
- <%- @grouped_messages.each do |chat_channel, messages| %> - <%- other_messages_count = messages.size - 2 %> - - - - - - - <%- unless SiteSetting.private_email %> - <%- messages.take(2).each do |chat_message| %> - <%- sender = chat_message.user %> - <%- sender_name = @display_usernames ? sender.username : sender.name %> - - - - - - - - <%- end %> - <%- end %> - - - + + + <%- unless SiteSetting.private_email %> + <%- messages.take(2).each do |chat_message| %> + <%- sender = chat_message.user %> + + + + + + + <%- end %> + <%- end %> + + + + + +
-
- <%- if SiteSetting.private_email %> - <%= I18n.t("system_messages.private_channel_title", id: chat_channel.id) %> - <%- else %> - <%= chat_channel.title(@user) %> - <%- end %> -
-
- <%= sender_name -%> - - <%= sender_name -%> - - - <%= I18n.l(@user_tz.to_local(chat_message.created_at), format: :long) -%> - -
- <%= email_excerpt(chat_message.cooked_for_excerpt) %> -
- - <%- if SiteSetting.private_email %> - <%= I18n.t("user_notifications.chat_summary.view_messages", count: messages.size)%> - <%- else %> - <%- if other_messages_count <= 0 %> - <%= I18n.t("user_notifications.chat_summary.view_messages", count: messages.size)%> + <%- [@grouped_channels, @grouped_dms].each do |grouped_messages| %> + <%- grouped_messages.each do |channel, messages| %> + <%- other_messages_count = messages.size - 2 %> + + + + - - -
+
+ <%- if SiteSetting.private_email %> + <%= I18n.t("system_messages.private_channel_title", id: channel.id) %> <%- else %> - <%= I18n.t("user_notifications.chat_summary.view_more", count: other_messages_count)%> + <%= channel.title(@user) %> <%- end %> - <%- end %> - -
+ +
+ <%= sender.display_name -%> + + <%= sender.display_name -%> + + + <%= I18n.l(@user_tz.to_local(chat_message.created_at), format: :long) -%> + +
+ <%= email_excerpt(chat_message.cooked_for_excerpt) %> +
+ + <%- if SiteSetting.private_email || other_messages_count <= 0 %> + <%= I18n.t("user_notifications.chat_summary.view_messages", count: messages.size) %> + <%- else %> + <%= I18n.t("user_notifications.chat_summary.view_more", count: other_messages_count) %> + <%- end %> + +
+ <%- end %> <%- end %> diff --git a/plugins/chat/app/views/user_notifications/chat_summary.text.erb b/plugins/chat/app/views/user_notifications/chat_summary.text.erb index 76955166f24..98a0785aab1 100644 --- a/plugins/chat/app/views/user_notifications/chat_summary.text.erb +++ b/plugins/chat/app/views/user_notifications/chat_summary.text.erb @@ -1,6 +1,6 @@ <%- site_link = raw(@markdown_linker.create(@site_name, '/')) %> -<%= t('user_notifications.chat_summary.description', count: @messages.size,) %> -<%= raw(@markdown_linker.create(t("user_notifications.chat_summary.view_messages", count: @messages.size), "/chat")) %> +<%= t('user_notifications.chat_summary.description', count: @count,) %> +<%= raw(@markdown_linker.create(t("user_notifications.chat_summary.view_messages", count: @count), "/chat")) %> <%- if @unsubscribe_link %> <%= raw(t :'user_notifications.chat_summary.unsubscribe', site_link: site_link, @@ -11,5 +11,4 @@ site_link: site_link, email_preferences_link: @markdown_linker.create(t('user_notifications.chat_summary.your_chat_settings'), @preferences_path)) %> <%- end %> - -<%= raw(@markdown_linker.references) %> +<%= raw(@markdown_linker.references) %> \ No newline at end of file diff --git a/plugins/chat/config/locales/server.en.yml b/plugins/chat/config/locales/server.en.yml index e828b29bc9f..88b42c31aae 100644 --- a/plugins/chat/config/locales/server.en.yml +++ b/plugins/chat/config/locales/server.en.yml @@ -240,18 +240,20 @@ en: other: "You have new chat messages" from: "%{site_name}" subject: - private_message: "[%{email_prefix}] New message" - direct_message_from_1: "[%{email_prefix}] New message from %{username}" - direct_message_from_2: "[%{email_prefix}] New message from %{username1} and %{username2}" - direct_message_from_more: - one: "[%{email_prefix}] New message from %{username} and %{count} other" - other: "[%{email_prefix}] New message from %{username} and %{count} others" - chat_channel_1: "[%{email_prefix}] New message in %{channel}" - chat_channel_2: "[%{email_prefix}] New message in %{channel1} and %{channel2}" - chat_channel_more: - one: "[%{email_prefix}] New message in %{channel} and %{count} other" - other: "[%{email_prefix}] New message in %{channel} and %{count} others" - chat_channel_and_direct_message: "[%{email_prefix}] New message in %{channel} and from %{username}" + private_email: + one: "[%{site_name}] New message" + other: "[%{site_name}] New messages" + chat_dm_1: + one: "[%{site_name}] New message from %{name}" + other: "[%{site_name}] New messages from %{name}" + chat_dm_2: "[%{site_name}] New messages from %{name_1} and %{name_2}" + chat_dm_3_or_more: "[%{site_name}] New messages from %{name} and %{count} others" + chat_channel_1: + one: "[%{site_name}] New message in %{channel}" + other: "[%{site_name}] New messages in %{channel}" + chat_channel_2: "[%{site_name}] New messages in %{channel_1} and %{channel_2}" + chat_channel_3_or_more: "[%{site_name}] New messages in %{channel} and %{count} others" + chat_channel_and_dm: "[%{site_name}] New messages in %{channel} and from %{name}" unsubscribe: "This chat summary is sent from %{site_link} when you are away. Change your %{email_preferences_link}, or %{unsubscribe_link} to unsubscribe." unsubscribe_no_link: "This chat summary is sent from %{site_link} when you are away. Change your %{email_preferences_link}." view_messages: diff --git a/plugins/chat/lib/chat/mailer.rb b/plugins/chat/lib/chat/mailer.rb index f3cf90fcbcc..56e7806c3df 100644 --- a/plugins/chat/lib/chat/mailer.rb +++ b/plugins/chat/lib/chat/mailer.rb @@ -5,73 +5,63 @@ module Chat def self.send_unread_mentions_summary return unless SiteSetting.chat_enabled - users_with_unprocessed_unread_mentions.find_each do |user| - # Apply modifier to `true` -- this allows other plugins to block the chat summary email send - if !DiscoursePluginRegistry.apply_modifier(:chat_mailer_send_summary_to_user, true, user) - next + User + .real + .activated + .not_staged + .not_suspended + .where(id: users_with_unreads) + .find_each do |user| + if DiscoursePluginRegistry.apply_modifier(:chat_mailer_send_summary_to_user, true, user) + Jobs.enqueue( + :user_email, + type: :chat_summary, + user_id: user.id, + force_respect_seen_recently: true, + ) + end end - - # user#memberships_with_unread_messages is a nested array that looks like [[membership_id, unread_message_id]] - # Find the max unread id per membership. - membership_and_max_unread_mention_ids = - user - .memberships_with_unread_messages - .group_by { |memberships| memberships[0] } - .transform_values do |membership_and_msg_ids| - membership_and_msg_ids.max_by { |membership, msg| msg } - end - .values - - Jobs.enqueue( - :user_email, - type: "chat_summary", - user_id: user.id, - force_respect_seen_recently: true, - memberships_to_update_data: membership_and_max_unread_mention_ids, - ) - end end private - def self.users_with_unprocessed_unread_mentions - when_away_frequency = UserOption.chat_email_frequencies[:when_away] - allowed_group_ids = Chat.allowed_group_ids + def self.users_with_unreads + groups_join_sql = + if Chat.allowed_group_ids.include?(Group::AUTO_GROUPS[:everyone]) + "" + else + "JOIN group_users ON group_users.user_id = users.id AND group_users.group_id IN (#{Chat.allowed_group_ids.join(",")})" + end - users = - User - .joins(:user_option) - .where(user_options: { chat_enabled: true, chat_email_frequency: when_away_frequency }) - .where("users.last_seen_at < ?", 15.minutes.ago) - - if !allowed_group_ids.include?(Group::AUTO_GROUPS[:everyone]) - users = users.joins(:groups).where(groups: { id: allowed_group_ids }) - end - - users - .select( - "users.id", - "ARRAY_AGG(ARRAY[uccm.id, c_msg.id]) AS memberships_with_unread_messages", - ) - .joins("INNER JOIN user_chat_channel_memberships uccm ON uccm.user_id = users.id") - .joins("INNER JOIN chat_channels cc ON cc.id = uccm.chat_channel_id") - .joins("INNER JOIN chat_messages c_msg ON c_msg.chat_channel_id = uccm.chat_channel_id") - .joins("LEFT OUTER JOIN chat_mentions c_mentions ON c_mentions.chat_message_id = c_msg.id") - .joins( - "LEFT OUTER JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = c_mentions.id", - ) - .joins("LEFT OUTER JOIN notifications n ON cmn.notification_id = n.id") - .where("c_msg.deleted_at IS NULL AND c_msg.user_id <> users.id") - .where("c_msg.created_at > ?", 1.week.ago) - .where(<<~SQL) - (uccm.last_read_message_id IS NULL OR c_msg.id > uccm.last_read_message_id) AND - (uccm.last_unread_mention_when_emailed_id IS NULL OR c_msg.id > uccm.last_unread_mention_when_emailed_id) AND - ( - (uccm.user_id = n.user_id AND uccm.following IS true AND cc.chatable_type = 'Category') OR - (cc.chatable_type = 'DirectMessage') + DB.query_single <<~SQL + SELECT uccm.user_id + FROM user_chat_channel_memberships uccm + JOIN users ON users.id = uccm.user_id + JOIN user_options ON user_options.user_id = users.id + #{groups_join_sql} + JOIN chat_channels ON chat_channels.id = uccm.chat_channel_id + JOIN chat_messages ON chat_messages.chat_channel_id = chat_channels.id + JOIN users sender ON sender.id = chat_messages.user_id + LEFT JOIN chat_mentions ON chat_mentions.chat_message_id = chat_messages.id + LEFT JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = chat_mentions.id + LEFT JOIN notifications ON notifications.id = cmn.notification_id AND notifications.user_id = uccm.user_id + WHERE NOT uccm.muted + AND (uccm.last_read_message_id IS NULL OR uccm.last_read_message_id < chat_messages.id) + AND (uccm.last_unread_mention_when_emailed_id IS NULL OR uccm.last_unread_mention_when_emailed_id < chat_messages.id) + AND users.last_seen_at < now() - interval '15 minutes' + AND user_options.chat_enabled + AND user_options.chat_email_frequency = #{UserOption.chat_email_frequencies[:when_away]} + AND user_options.email_level <> #{UserOption.email_level_types[:never]} + AND chat_channels.deleted_at IS NULL + AND chat_messages.deleted_at IS NULL + AND chat_messages.created_at > now() - interval '1 week' + AND chat_messages.user_id <> users.id + AND ( + (chat_channels.chatable_type = 'DirectMessage' AND user_options.allow_private_messages) OR + (chat_channels.chatable_type = 'Category' AND uccm.following AND NOT notifications.read) ) + GROUP BY uccm.user_id SQL - .group("users.id, uccm.user_id") end end end diff --git a/plugins/chat/lib/chat/user_email_extension.rb b/plugins/chat/lib/chat/user_email_extension.rb deleted file mode 100644 index 366fc41bb32..00000000000 --- a/plugins/chat/lib/chat/user_email_extension.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -module Chat - module UserEmailExtension - def execute(args) - super(args) - - if args[:type] == "chat_summary" && args[:memberships_to_update_data].present? - args[:memberships_to_update_data].to_a.each do |membership_id, max_unread_mention_id| - Chat::UserChatChannelMembership.find_by( - user: args[:user_id], - id: membership_id.to_i, - )&.update(last_unread_mention_when_emailed_id: max_unread_mention_id.to_i) - end - end - end - end -end diff --git a/plugins/chat/lib/chat/user_notifications_extension.rb b/plugins/chat/lib/chat/user_notifications_extension.rb index e9070f96ef3..38ced82b762 100644 --- a/plugins/chat/lib/chat/user_notifications_extension.rb +++ b/plugins/chat/lib/chat/user_notifications_extension.rb @@ -6,137 +6,163 @@ module Chat guardian = Guardian.new(user) return unless guardian.can_chat? - @messages = - Chat::Message - .joins(:user, :chat_channel) - .where.not(user: user) - .where("chat_messages.created_at > ?", 1.week.ago) - .joins("LEFT OUTER JOIN chat_mentions cm ON cm.chat_message_id = chat_messages.id") - .joins("LEFT OUTER JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = cm.id") - .joins("LEFT OUTER JOIN notifications n ON cmn.notification_id = n.id") - .joins( - "INNER JOIN user_chat_channel_memberships uccm ON uccm.chat_channel_id = chat_channels.id", - ) - .where(<<~SQL, user_id: user.id) - uccm.user_id = :user_id AND - (uccm.last_read_message_id IS NULL OR chat_messages.id > uccm.last_read_message_id) AND - (uccm.last_unread_mention_when_emailed_id IS NULL OR chat_messages.id > uccm.last_unread_mention_when_emailed_id) AND - ( - (n.user_id = :user_id AND cmn.notification_id IS NOT NULL AND uccm.following IS true AND chat_channels.chatable_type = 'Category') OR - (chat_channels.chatable_type = 'DirectMessage') - ) - SQL - .to_a + # TODO: handle muted & silenced users ? - return if @messages.empty? - @grouped_messages = @messages.group_by { |message| message.chat_channel } - @grouped_messages = - @grouped_messages.select { |channel, _| guardian.can_join_chat_channel?(channel) } - return if @grouped_messages.empty? + # ensures these haven't since the job was enqueued + return if user.last_seen_at > 15.minutes.ago + return if user.user_option.send_chat_email_never? + return if user.user_option.email_level == UserOption.email_level_types[:never] + + unread_mentions = DB.query_array <<~SQL + WITH unread_mentions AS ( + SELECT uccm.id membership_id, uccm.chat_channel_id, MIN(chat_messages.id) first_chat_message_id, MAX(chat_messages.id) last_chat_message_id + FROM user_chat_channel_memberships uccm + JOIN chat_channels ON chat_channels.id = uccm.chat_channel_id + JOIN chat_messages ON chat_messages.chat_channel_id = chat_channels.id + JOIN chat_mentions ON chat_mentions.chat_message_id = chat_messages.id + JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = chat_mentions.id + JOIN notifications ON notifications.id = cmn.notification_id + JOIN users ON users.id = chat_messages.user_id + WHERE uccm.user_id = #{user.id} + AND NOT uccm.muted + AND uccm.following + AND chat_channels.deleted_at IS NULL + AND chat_channels.chatable_type = 'Category' + AND chat_messages.deleted_at IS NULL + AND chat_messages.user_id != uccm.user_id + AND chat_messages.created_at > now() - interval '1 week' + AND (uccm.last_read_message_id IS NULL OR uccm.last_read_message_id < chat_messages.id) + AND (uccm.last_unread_mention_when_emailed_id IS NULL OR uccm.last_unread_mention_when_emailed_id < chat_messages.id) + AND NOT notifications.read + GROUP BY uccm.id + ) + UPDATE user_chat_channel_memberships uccm + SET last_unread_mention_when_emailed_id = um.last_chat_message_id + FROM unread_mentions um + WHERE uccm.id = um.membership_id + AND uccm.user_id = #{user.id} + RETURNING um.membership_id, um.chat_channel_id, um.first_chat_message_id + SQL + + unread_messages = DB.query_array <<~SQL + WITH unread_messages AS ( + SELECT uccm.id membership_id, uccm.chat_channel_id, MIN(chat_messages.id) first_chat_message_id, MAX(chat_messages.id) last_chat_message_id + FROM user_chat_channel_memberships uccm + JOIN chat_channels ON chat_channels.id = uccm.chat_channel_id + JOIN chat_messages ON chat_messages.chat_channel_id = chat_channels.id + JOIN users ON users.id = chat_messages.user_id + WHERE uccm.user_id = #{user.id} + AND NOT uccm.muted + AND chat_channels.deleted_at IS NULL + AND chat_channels.chatable_type = 'DirectMessage' + AND chat_messages.deleted_at IS NULL + AND chat_messages.user_id != uccm.user_id + AND chat_messages.created_at > now() - interval '1 week' + AND (uccm.last_read_message_id IS NULL OR uccm.last_read_message_id < chat_messages.id) + AND (uccm.last_unread_mention_when_emailed_id IS NULL OR uccm.last_unread_mention_when_emailed_id < chat_messages.id) + GROUP BY uccm.id + ) + UPDATE user_chat_channel_memberships uccm + SET last_unread_mention_when_emailed_id = um.last_chat_message_id + FROM unread_messages um + WHERE uccm.id = um.membership_id + AND uccm.user_id = #{user.id} + RETURNING um.membership_id, um.chat_channel_id, um.first_chat_message_id + SQL + + @grouped_channels = chat_messages_for(unread_mentions, guardian) + + @grouped_dms = + user.user_option.allow_private_messages ? chat_messages_for(unread_messages, guardian) : {} + + @count = @grouped_channels.values.sum(&:size) + @grouped_dms.values.sum(&:size) + + return if @count.zero? - @grouped_messages.each do |chat_channel, messages| - @grouped_messages[chat_channel] = messages.sort_by(&:created_at) - end - @user = user @user_tz = UserOption.user_tzinfo(user.id) - @display_usernames = SiteSetting.prioritize_username_in_ux || !SiteSetting.enable_names - - build_summary_for(user) @preferences_path = "#{Discourse.base_url}/my/preferences/chat" - opts = { - from_alias: I18n.t("user_notifications.chat_summary.from", site_name: Email.site_title), - subject: summary_subject(user, @grouped_messages), - } + build_summary_for(user) - build_email(user.email, opts) - end - - def summary_subject(user, grouped_messages) - if SiteSetting.private_email - return( - I18n.t( - "user_notifications.chat_summary.subject.private_message", - email_prefix: @email_prefix, - ) - ) - end - - all_channels = grouped_messages.keys - grouped_channels = all_channels.partition { |c| !c.direct_message_channel? } - channels = grouped_channels.first - - dm_messages = grouped_channels.last.flat_map { |c| grouped_messages[c] } - dm_users = dm_messages.sort_by(&:created_at).uniq { |m| m.user_id }.map(&:user) - - # Prioritize messages from regular channels over direct messages - if channels.any? - channel_notification_text( - channels.sort_by { |channel| [channel.last_message.created_at, channel.created_at] }, - dm_users, - ) - else - direct_message_notification_text(dm_users) - end + build_email( + user.email, + from_alias: chat_summary_from_alias, + subject: chat_summary_subject(@grouped_channels, @grouped_dms, @count), + ) end private - def channel_notification_text(channels, dm_users) - total_count = channels.size + dm_users.size - - if total_count > 2 - I18n.t( - "user_notifications.chat_summary.subject.chat_channel_more", - email_prefix: @email_prefix, - channel: channels.first.title, - count: total_count - 1, + def chat_messages_for(data, guardian) + # Note: we probably want to limit the number of messages we fetch + # since we only display the first 2 per channel in the email + # I've left this as if for now because we also display the total count + # and a count of unread messages per channel + Chat::Message + .includes(:user, :chat_channel) + .where(chat_channel_id: data.map { _1[1] }) + .where( + "chat_messages.id >= ( + SELECT min_unread_id + FROM (VALUES #{data.map { "(#{_1[1]}, #{_1[2]})" }.join(",")}) AS t(channel_id, min_unread_id) + WHERE t.channel_id = chat_messages.chat_channel_id + )", ) - elsif channels.size == 1 && dm_users.size == 0 - I18n.t( - "user_notifications.chat_summary.subject.chat_channel_1", - email_prefix: @email_prefix, - channel: channels.first.title, - ) - elsif channels.size == 1 && dm_users.size == 1 - I18n.t( - "user_notifications.chat_summary.subject.chat_channel_and_direct_message", - email_prefix: @email_prefix, - channel: channels.first.title, - username: dm_users.first.username, - ) - elsif channels.size == 2 - I18n.t( - "user_notifications.chat_summary.subject.chat_channel_2", - email_prefix: @email_prefix, - channel1: channels.first.title, - channel2: channels.second.title, - ) - end + .order(created_at: :asc) + .group_by(&:chat_channel) + .select { |channel, _| guardian.can_join_chat_channel?(channel) } end - def direct_message_notification_text(dm_users) - case dm_users.size - when 1 - I18n.t( - "user_notifications.chat_summary.subject.direct_message_from_1", - email_prefix: @email_prefix, - username: dm_users.first.username, - ) - when 2 - I18n.t( - "user_notifications.chat_summary.subject.direct_message_from_2", - email_prefix: @email_prefix, - username1: dm_users.first.username, - username2: dm_users.second.username, - ) + def chat_summary_from_alias + I18n.t("user_notifications.chat_summary.from", site_name: @site_name) + end + + def subject(type, **args) + I18n.t("user_notifications.chat_summary.subject.#{type}", { site_name: @site_name, **args }) + end + + def chat_summary_subject(grouped_channels, grouped_dms, count) + return subject(:private_email, count:) if SiteSetting.private_email + + # consider "direct messages" with more than 2 users as group messages (aka. channels) + dms, groups = grouped_dms.keys.partition { _1.user_chat_channel_memberships.count == 2 } + + channels = grouped_channels.keys + groups + + if channels.any? + if dms.any? + subject( + :chat_channel_and_dm, + channel: channels.first.title(@user), + name: dms.first.title(@user), + ) + elsif channels.size == 1 + subject( + :chat_channel_1, + channel: channels.first.title(@user), + count: (grouped_channels[channels.first] || grouped_dms[channels.first]).size, + ) + elsif channels.size == 2 + subject( + :chat_channel_2, + channel_1: channels.first.title(@user), + channel_2: channels.second.title(@user), + ) + else + subject( + :chat_channel_3_or_more, + channel: channels.first.title(@user), + count: channels.size - 1, + ) + end + elsif dms.size == 1 + subject(:chat_dm_1, name: dms.first.title(@user), count: grouped_dms[dms.first].size) + elsif dms.size == 2 + subject(:chat_dm_2, name_1: dms.first.title(@user), name_2: dms.second.title(@user)) + elsif dms.size >= 3 + subject(:chat_dm_3_or_more, name: dms.first.title(@user), count: dms.size - 1) else - I18n.t( - "user_notifications.chat_summary.subject.direct_message_from_more", - email_prefix: @email_prefix, - username: dm_users.first.username, - count: dm_users.size - 1, - ) + subject(:private_email, count:) end end end diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index fd73a0d0040..904bd518ce2 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -72,7 +72,6 @@ after_initialize do Bookmark.prepend Chat::BookmarkExtension User.prepend Chat::UserExtension Group.prepend Chat::GroupExtension - Jobs::UserEmail.prepend Chat::UserEmailExtension Plugin::Instance.prepend Chat::PluginInstanceExtension Jobs::ExportCsvFile.prepend Chat::MessagesExporter WebHook.prepend Chat::OutgoingWebHookExtension diff --git a/plugins/chat/spec/components/chat/mailer_spec.rb b/plugins/chat/spec/components/chat/mailer_spec.rb index 86969c4e86a..fadf1fb3d1f 100644 --- a/plugins/chat/spec/components/chat/mailer_spec.rb +++ b/plugins/chat/spec/components/chat/mailer_spec.rb @@ -1,334 +1,282 @@ # frozen_string_literal: true describe Chat::Mailer do - fab!(:chatters_group) { Fabricate(:group) } - fab!(:sender) { Fabricate(:user, group_ids: [chatters_group.id], refresh_auto_groups: true) } - fab!(:user_1) do - Fabricate( - :user, - group_ids: [chatters_group.id], - last_seen_at: 15.minutes.ago, - refresh_auto_groups: true, - ) - end - fab!(:chat_channel) { Fabricate(:category_channel) } - fab!(:chat_message) { Fabricate(:chat_message, user: sender, chat_channel: chat_channel) } - fab!(:user_1_chat_channel_membership) do - Fabricate( - :user_chat_channel_membership, - user: user_1, - chat_channel: chat_channel, - last_read_message_id: nil, - ) - end - fab!(:private_chat_channel) do - result = - Chat::CreateDirectMessageChannel.call( - guardian: sender.guardian, - target_usernames: [sender.username, user_1.username], - ) - service_failed!(result) if result.failure? - result.channel + fab!(:user) { Fabricate(:user, last_seen_at: 1.hour.ago) } + fab!(:other) { Fabricate(:user) } + + fab!(:group) do + Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone], users: [user, other]) end + fab!(:followed_channel) { Fabricate(:category_channel) } + fab!(:non_followed_channel) { Fabricate(:category_channel) } + fab!(:muted_channel) { Fabricate(:category_channel) } + fab!(:unseen_channel) { Fabricate(:category_channel) } + fab!(:direct_message) { Fabricate(:direct_message_channel, users: [user, other]) } + + fab!(:job) { :user_email } + fab!(:args) { { type: :chat_summary, user_id: user.id, force_respect_seen_recently: true } } + before do SiteSetting.chat_enabled = true - SiteSetting.chat_allowed_groups = chatters_group.id - - Fabricate(:user_chat_channel_membership, user: sender, chat_channel: chat_channel) + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] end - def assert_summary_skipped - expect( - job_enqueued?(job: :user_email, args: { type: "chat_summary", user_id: user_1.id }), - ).to eq(false) - end - - def assert_only_queued_once - expect_job_enqueued(job: :user_email, args: { type: "chat_summary", user_id: user_1.id }) + def expect_enqueued + expect_enqueued_with(job:, args:) { described_class.send_unread_mentions_summary } expect(Jobs::UserEmail.jobs.size).to eq(1) end - describe "for chat mentions" do - fab!(:notification) do - Fabricate(:notification, notification_type: Notification.types[:chat_mention], user: user_1) - end - fab!(:mention) do - Fabricate(:user_chat_mention, chat_message: chat_message, notifications: [notification]) - end + def expect_not_enqueued + expect_not_enqueued_with(job:, args:) { described_class.send_unread_mentions_summary } + end - it "skips users without chat access" do - chatters_group.remove(user_1) + # This helper is much faster than `Fabricate(:chat_message_with_service, ...)` + def create_message(chat_channel, message, mention_klass = nil) + chat_message = Fabricate(:chat_message, user: other, chat_channel:, message:) - described_class.send_unread_mentions_summary + if mention_klass + notification_type = Notification.types[:chat_mention] - assert_summary_skipped - end - - it "skips users with summaries disabled" do - user_1.user_option.update(chat_email_frequency: UserOption.chat_email_frequencies[:never]) - - described_class.send_unread_mentions_summary - - assert_summary_skipped - end - - it "skips a job if the user haven't read the channel since the last summary" do - user_1_chat_channel_membership.update!(last_unread_mention_when_emailed_id: chat_message.id) - - described_class.send_unread_mentions_summary - - assert_summary_skipped - end - - it "skips without chat enabled" do - user_1.user_option.update( - chat_enabled: false, - chat_email_frequency: UserOption.chat_email_frequencies[:when_away], - ) - - described_class.send_unread_mentions_summary - - assert_summary_skipped - end - - it "queues a job for users that was mentioned and never read the channel before" do - described_class.send_unread_mentions_summary - - assert_only_queued_once - end - - it "skips the job when the user was mentioned but already read the message" do - user_1_chat_channel_membership.update!(last_read_message_id: chat_message.id) - - described_class.send_unread_mentions_summary - - assert_summary_skipped - end - - it "skips the job when the user is not following a public channel anymore" do - user_1_chat_channel_membership.update!( - last_read_message_id: chat_message.id - 1, - following: false, - ) - - described_class.send_unread_mentions_summary - - assert_summary_skipped - end - - it "doesn’t skip the job when the user is not following a direct channel" do - private_chat_channel - .user_chat_channel_memberships - .where(user_id: user_1.id) - .update!(last_read_message_id: chat_message.id - 1, following: false) - - described_class.send_unread_mentions_summary - - assert_only_queued_once - end - - it "skips users with unread messages from a different channel" do - user_1_chat_channel_membership.update!(last_read_message_id: chat_message.id) - second_channel = Fabricate(:category_channel) Fabricate( - :user_chat_channel_membership, - user: user_1, - chat_channel: second_channel, - last_read_message_id: chat_message.id - 1, + :chat_mention_notification, + notification: Fabricate(:notification, user:, notification_type:), + chat_mention: mention_klass.find_by(chat_message:), ) - - described_class.send_unread_mentions_summary - - assert_summary_skipped end - it "only queues the job once for users who are member of multiple groups with chat access" do - chatters_group_2 = Fabricate(:group, users: [user_1]) - SiteSetting.chat_allowed_groups = [chatters_group, chatters_group_2].map(&:id).join("|") + chat_message + end - described_class.send_unread_mentions_summary + describe "in a followed channel" do + before { followed_channel.add(user) } - assert_only_queued_once - end + describe "user is @direct mentioned" do + let!(:chat_message) do + create_message(followed_channel, "hello @#{user.username}", Chat::UserMention) + end - it "skips users when the mention was deleted" do - chat_message.trash! + it "queues a chat summary email" do + expect_enqueued + end - described_class.send_unread_mentions_summary + it "does not queue a chat summary when chat is globally disabled" do + SiteSetting.chat_enabled = false + expect_not_enqueued + end - assert_summary_skipped - end + it "does not queue a chat summary email when user has chat disabled" do + user.user_option.update!(chat_enabled: false) + expect_not_enqueued + end - it "queues the job if the user has unread mentions and already read all the messages in the previous summary" do - user_1_chat_channel_membership.update!( - last_read_message_id: chat_message.id, - last_unread_mention_when_emailed_id: chat_message.id, - ) - unread_message = Fabricate(:chat_message, chat_channel: chat_channel, user: sender) - notification_2 = - Fabricate(:notification, notification_type: Notification.types[:chat_mention], user: user_1) - Fabricate( - :user_chat_mention, - user: user_1, - chat_message: unread_message, - notifications: [notification_2], - ) + it "does not queue a chat summary email when user has chat email frequency = never" do + user.user_option.update!(chat_email_frequency: UserOption.chat_email_frequencies[:never]) + expect_not_enqueued + end - described_class.send_unread_mentions_summary + it "does not queue a chat summary email when user has email level = never" do + user.user_option.update!(email_level: UserOption.email_level_types[:never]) + expect_not_enqueued + end - expect_job_enqueued(job: :user_email, args: { type: "chat_summary", user_id: user_1.id }) - expect(Jobs::UserEmail.jobs.size).to eq(1) - end + it "does not queue a chat summary email when chat message has been deleted" do + chat_message.trash! + expect_not_enqueued + end - it "skips users who were seen recently" do - user_1.update!(last_seen_at: 2.minutes.ago) + it "does not queue a chat summary email when chat message is older than 1 week" do + chat_message.update!(created_at: 2.weeks.ago) + expect_not_enqueued + end - described_class.send_unread_mentions_summary + it "does not queue a chat summary email when chat channel has been deleted" do + followed_channel.trash! + expect_not_enqueued + end - assert_summary_skipped - end + it "does not queue a chat summary email when user is not part of chat allowed groups" do + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:admins] + expect_not_enqueued + end - it "doesn't mix mentions from other users" do - mention.destroy! - user_2 = Fabricate(:user, groups: [chatters_group], last_seen_at: 20.minutes.ago) - Fabricate( - :user_chat_channel_membership, - user: user_2, - chat_channel: chat_channel, - last_read_message_id: nil, - ) - new_message = Fabricate(:chat_message, chat_channel: chat_channel, user: sender) - notification_2 = - Fabricate(:notification, notification_type: Notification.types[:chat_mention], user: user_2) - Fabricate( - :user_chat_mention, - user: user_2, - chat_message: new_message, - notifications: [notification_2], - ) + it "does not queue a chat summary email when user has read the mention notification" do + Notification.find_by( + user: user, + notification_type: Notification.types[:chat_mention], + ).update!(read: true) - described_class.send_unread_mentions_summary + expect_not_enqueued + end - expect( - job_enqueued?(job: :user_email, args: { type: "chat_summary", user_id: user_1.id }), - ).to eq(false) - expect_job_enqueued(job: :user_email, args: { type: "chat_summary", user_id: user_2.id }) - expect(Jobs::UserEmail.jobs.size).to eq(1) - end + it "does not queue a chat summary email when user has been seen in the past 15 minutes" do + user.update!(last_seen_at: 5.minutes.ago) + expect_not_enqueued + end - it "skips users when the message is older than 1 week" do - chat_message.update!(created_at: 1.5.weeks.ago) + it "does not queue a chat summary email when user has read the message" do + followed_channel.membership_for(user).update!(last_read_message_id: chat_message.id) + expect_not_enqueued + end - described_class.send_unread_mentions_summary - - assert_summary_skipped - end - - it "queues a job when the chat_allowed_groups is set to everyone" do - SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] - - described_class.send_unread_mentions_summary - - assert_only_queued_once - end - - context "with chat_mailer_send_summary_to_user modifier" do - let(:modifier_block) { Proc.new { |_| false } } - it "skips when modifier evaluates to false" do - SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] - - plugin_instance = Plugin::Instance.new - plugin_instance.register_modifier(:chat_mailer_send_summary_to_user, &modifier_block) - - described_class.send_unread_mentions_summary - - assert_summary_skipped - ensure - DiscoursePluginRegistry.unregister_modifier( - plugin_instance, - :chat_mailer_send_summary_to_user, - &modifier_block + it "does not queue a chat summary email when user has received an email for this message" do + followed_channel.membership_for(user).update!( + last_unread_mention_when_emailed_id: chat_message.id, ) + + expect_not_enqueued + end + + it "does not queue a chat summary email when user is not active" do + user.update!(active: false) + expect_not_enqueued + end + + it "does not queue a chat summary email when user is staged" do + user.update!(staged: true) + expect_not_enqueued + end + + it "does not queue a chat summary email when user is suspended" do + user.update!(suspended_till: 1.day.from_now) + expect_not_enqueued + end + + it "does not queue a chat summary email when sender has been deleted" do + other.destroy! + expect_not_enqueued + end + + it "queues a chat summary email even when user has private messages disabled" do + user.user_option.update!(allow_private_messages: false) + expect_enqueued + end + + describe "when another plugin blocks the email" do + let!(:plugin) { Plugin::Instance.new } + let!(:modifier) { :chat_mailer_send_summary_to_user } + let!(:block) { Proc.new { false } } + + before { DiscoursePluginRegistry.register_modifier(plugin, modifier, &block) } + after { DiscoursePluginRegistry.unregister_modifier(plugin, modifier, &block) } + + it "does not queue a chat summary email" do + expect_not_enqueued + end end end - describe "update the user membership after we send the email" do - before { Jobs.run_immediately! } + describe "user is @group mentioned" do + before { create_message(followed_channel, "hello @#{group.name}", Chat::GroupMention) } - it "doesn't send the same summary the summary again if the user haven't read any channel messages since the last one" do - user_1_chat_channel_membership.update!(last_read_message_id: chat_message.id - 1) - described_class.send_unread_mentions_summary - - expect(user_1_chat_channel_membership.reload.last_unread_mention_when_emailed_id).to eq( - chat_message.id, - ) - - another_channel_message = Fabricate(:chat_message, chat_channel: chat_channel, user: sender) - Fabricate(:user_chat_mention, user: user_1, chat_message: another_channel_message) - - expect { described_class.send_unread_mentions_summary }.not_to change( - Jobs::UserEmail.jobs, - :size, - ) + it "queues a chat summary email" do + expect_enqueued end + end - it "only updates the last_message_read_when_emailed_id on the channel with unread mentions" do - another_channel = Fabricate(:category_channel) - another_channel_message = - Fabricate(:chat_message, chat_channel: another_channel, user: sender) - Fabricate(:user_chat_mention, user: user_1, chat_message: another_channel_message) - another_channel_membership = - Fabricate( - :user_chat_channel_membership, - user: user_1, - chat_channel: another_channel, - last_read_message_id: another_channel_message.id, - ) - user_1_chat_channel_membership.update!(last_read_message_id: chat_message.id - 1) + describe "user is @all mentioned" do + before { create_message(followed_channel, "hello @all", Chat::AllMention) } - described_class.send_unread_mentions_summary - - expect(user_1_chat_channel_membership.reload.last_unread_mention_when_emailed_id).to eq( - chat_message.id, - ) - expect(another_channel_membership.reload.last_unread_mention_when_emailed_id).to be_nil + it "queues a chat summary email" do + expect_enqueued end end end - describe "for direct messages" do - before { Fabricate(:chat_message, user: sender, chat_channel: private_chat_channel) } + describe "in a non-followed channel" do + before { non_followed_channel.add(user).update!(following: false) } - it "queue a job when the user has unread private mentions" do - described_class.send_unread_mentions_summary + describe "user is @direct mentioned" do + before { create_message(non_followed_channel, "hello @#{user.username}", Chat::UserMention) } - assert_only_queued_once + it "does not queue a chat summary email" do + expect_not_enqueued + end end - it "only queues the job once when the user has mentions and private messages" do - Fabricate(:user_chat_mention, user: user_1, chat_message: chat_message) + describe "user is @group mentioned" do + before { create_message(non_followed_channel, "hello @#{group.name}", Chat::GroupMention) } - described_class.send_unread_mentions_summary - - assert_only_queued_once + it "does not queue a chat summary email" do + expect_not_enqueued + end end - it "doesn't mix or update mentions from other users when joining tables" do - user_2 = Fabricate(:user, groups: [chatters_group], last_seen_at: 20.minutes.ago) - user_2_membership = - Fabricate( - :user_chat_channel_membership, - user: user_2, - chat_channel: chat_channel, - last_read_message_id: chat_message.id, - ) - Fabricate(:user_chat_mention, user: user_2, chat_message: chat_message) + describe "user is @all mentioned" do + before { create_message(non_followed_channel, "hello @all", Chat::AllMention) } - described_class.send_unread_mentions_summary + it "does not queue a chat summary email" do + expect_not_enqueued + end + end + end - assert_only_queued_once - expect(user_2_membership.reload.last_unread_mention_when_emailed_id).to be_nil + describe "in a muted channel" do + before { muted_channel.add(user).update!(muted: true) } + + describe "user is @direct mentioned" do + before { create_message(muted_channel, "hello @#{user.username}", Chat::UserMention) } + + it "does not queue a chat summary email" do + expect_not_enqueued + end + end + + describe "user is @group mentioned" do + before { create_message(muted_channel, "hello @#{group.name}", Chat::GroupMention) } + + it "does not queue a chat summary email" do + expect_not_enqueued + end + end + + describe "user is @all mentioned" do + before { create_message(muted_channel, "hello @all", Chat::AllMention) } + + it "does not queue a chat summary email" do + expect_not_enqueued + end + end + end + + describe "in an unseen channel" do + describe "user is @direct mentioned" do + before { create_message(unseen_channel, "hello @#{user.username}") } + + it "does not queue a chat summary email" do + expect_not_enqueued + end + end + + describe "user is @group mentioned" do + before { create_message(unseen_channel, "hello @#{group.name}") } + + it "doest not queue a chat summary email" do + expect_not_enqueued + end + end + end + + describe "in a direct message" do + before { create_message(direct_message, "Howdy πŸ‘‹") } + + it "queues a chat summary email" do + expect_enqueued + end + + it "queues a chat summary email when user isn't following the direct message anymore" do + direct_message.membership_for(user).update!(following: false) + expect_enqueued + end + + it "does not queue a chat summary email when user has muted the direct message" do + direct_message.membership_for(user).update!(muted: true) + expect_not_enqueued + end + + it "does not queue a chat summary email when user has private messages disabled" do + user.user_option.update!(allow_private_messages: false) + expect_not_enqueued end end end diff --git a/plugins/chat/spec/fabricators/chat_fabricator.rb b/plugins/chat/spec/fabricators/chat_fabricator.rb index 328a9276be0..3ace343b0a8 100644 --- a/plugins/chat/spec/fabricators/chat_fabricator.rb +++ b/plugins/chat/spec/fabricators/chat_fabricator.rb @@ -115,6 +115,11 @@ Fabricator(:chat_message_with_service, class_name: "Chat::CreateMessage") do end end +Fabricator(:chat_mention_notification, class_name: "Chat::MentionNotification") do + chat_mention { Fabricate(:user_chat_mention) } + notification { Fabricate(:notification) } +end + Fabricator(:user_chat_mention, class_name: "Chat::UserMention") do transient read: false transient high_priority: true diff --git a/plugins/chat/spec/mailers/user_notifications_spec.rb b/plugins/chat/spec/mailers/user_notifications_spec.rb index 6b45609fcbd..117a5942ab0 100644 --- a/plugins/chat/spec/mailers/user_notifications_spec.rb +++ b/plugins/chat/spec/mailers/user_notifications_spec.rb @@ -1,734 +1,468 @@ # frozen_string_literal: true describe UserNotifications do - fab!(:chatters_group) { Fabricate(:group) } - fab!(:sender) { Fabricate(:user, group_ids: [chatters_group.id], refresh_auto_groups: true) } - fab!(:user) { Fabricate(:user, group_ids: [chatters_group.id], refresh_auto_groups: true) } + fab!(:user) { Fabricate(:user, last_seen_at: 1.hour.ago) } + fab!(:other) { Fabricate(:user) } + fab!(:another) { Fabricate(:user) } + fab!(:someone) { Fabricate(:user) } + fab!(:group) { Fabricate(:group, users: [user, other]) } + + fab!(:followed_channel) { Fabricate(:category_channel) } + fab!(:followed_channel_2) { Fabricate(:category_channel) } + fab!(:followed_channel_3) { Fabricate(:category_channel) } + fab!(:non_followed_channel) { Fabricate(:category_channel) } + fab!(:muted_channel) { Fabricate(:category_channel) } + fab!(:unseen_channel) { Fabricate(:category_channel) } + fab!(:private_channel) { Fabricate(:private_category_channel, group:) } + fab!(:direct_message) { Fabricate(:direct_message_channel, users: [user, other]) } + fab!(:direct_message_2) { Fabricate(:direct_message_channel, users: [user, another]) } + fab!(:direct_message_3) { Fabricate(:direct_message_channel, users: [user, someone]) } + fab!(:group_message) { Fabricate(:direct_message_channel, users: [user, other, another]) } + + fab!(:site_name) { SiteSetting.email_prefix.presence || SiteSetting.title } before do SiteSetting.chat_enabled = true - SiteSetting.chat_allowed_groups = chatters_group.id + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] end - def refresh_auto_groups - user.reload - sender.reload + def create_message(chat_channel, message, mention_klass = nil) + chat_message = Fabricate(:chat_message, user: other, chat_channel:, message:) + + if mention_klass + notification_type = Notification.types[:chat_mention] + + Fabricate( + :chat_mention_notification, + notification: Fabricate(:notification, user:, notification_type:), + chat_mention: mention_klass.find_by(chat_message:), + ) + end + + chat_message end - describe ".chat_summary" do - context "with private channel" do - fab!(:channel) do - refresh_auto_groups - create_dm_channel(sender, [sender, user]) + def no_chat_summary_email + email = described_class.chat_summary(user, {}) + expect(email.to).to be_blank + end + + def chat_summary_email + email = described_class.chat_summary(user, {}) + expect(email.to).to contain_exactly(user.email) + email + end + + def chat_summary_with_subject(type, opts = {}) + expect(chat_summary_email.subject).to eq( + I18n.t("user_notifications.chat_summary.subject.#{type}", { site_name:, **opts }), + ) + end + + describe "in a followed channel" do + before { followed_channel.add(user) } + + describe "user is mentioned" do + let!(:chat_mention) do + create_message(followed_channel, "hello @#{user.username}", Chat::UserMention) end - it "calls guardian can_join_chat_channel?" do - Fabricate(:chat_message, user: sender, chat_channel: channel) - Guardian.any_instance.expects(:can_join_chat_channel?).once - email = described_class.chat_summary(user, {}) - email.subject + it "sends a chat summary email" do + chat_summary_with_subject(:chat_channel_1, channel: followed_channel.name, count: 1) end - describe "email subject" do - context "when private_email setting is enabled" do - before { SiteSetting.private_email = true } + it "pluralizes the subject" do + create_message(followed_channel, "how are you?") + chat_summary_with_subject(:chat_channel_1, channel: followed_channel.name, count: 2) + end - it "has a generic subject" do - Fabricate(:chat_message, user: sender, chat_channel: channel) - email = described_class.chat_summary(user, {}) + it "sends a chat summary email with correct body" do + html = chat_summary_email.html_part.body.to_s - expect(email.subject).to eq( - I18n.t( - "user_notifications.chat_summary.subject.private_message", - email_prefix: SiteSetting.email_prefix.presence || SiteSetting.title, - ), - ) - end + expect(html).to include(followed_channel.title(user)) + expect(html).to include(chat_mention.full_url) + expect(html).to include(PrettyText.format_for_email(chat_mention.cooked_for_excerpt)) + expect(html).to include(chat_mention.user.small_avatar_url) + expect(html).to include(chat_mention.user.username) + expect(html).to include( + I18n.l(UserOption.user_tzinfo(user.id).to_local(chat_mention.created_at), format: :long), + ) + expect(html).to include(I18n.t("user_notifications.chat_summary.view_messages", count: 1)) + end + + it "sends a chat summary email with view more link" do + create_message(followed_channel, "how are you...") + create_message(followed_channel, "doing...") + create_message(followed_channel, "today?") + + html = chat_summary_email.html_part.body.to_s + + expect(html).to include(I18n.t("user_notifications.chat_summary.view_more", count: 2)) + end + + describe "SiteSetting.prioritize_username_in_ux is disabled" do + before { SiteSetting.prioritize_username_in_ux = false } + + it "sends a chat summary email with the username instead of the name" do + html = chat_summary_email.html_part.body.to_s + + expect(html).to include(chat_mention.user.name) + expect(html).not_to include(chat_mention.user.username) + end + end + + describe "when using subfolder" do + before { set_subfolder "/community" } + + it "sends a chat summary email with the correct URL" do + html = chat_summary_email.html_part.body.to_s + + expect(html).to include <<~HTML.strip +