diff --git a/app/jobs/scheduled/email_chat_notifications.rb b/app/jobs/scheduled/email_chat_notifications.rb new file mode 100644 index 000000000..09462a186 --- /dev/null +++ b/app/jobs/scheduled/email_chat_notifications.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Jobs + class EmailChatNotifications < ::Jobs::Scheduled + every 5.minutes + + def execute(args = {}) + return unless SiteSetting.chat_enabled + + DiscourseChat::ChatMailer.send_unread_mentions_summary + end + end +end diff --git a/app/models/chat_message.rb b/app/models/chat_message.rb index c570ccc5e..d4ddde580 100644 --- a/app/models/chat_message.rb +++ b/app/models/chat_message.rb @@ -68,7 +68,11 @@ def excerpt return uploads.first.original_filename if cooked.blank? && uploads.present? # this may return blank for some complex things like quotes, that is acceptable - PrettyText.excerpt(cooked, 50) + PrettyText.excerpt(cooked, 50, {}) + end + + def cooked_for_excerpt + (cooked.blank? && uploads.present?) ? "

#{uploads.first.original_filename}

" : cooked end def push_notification_excerpt diff --git a/app/models/direct_message_channel.rb b/app/models/direct_message_channel.rb index 3ec247b66..a7b01758b 100644 --- a/app/models/direct_message_channel.rb +++ b/app/models/direct_message_channel.rb @@ -17,7 +17,7 @@ def chat_channel_title_for_user(chat_channel, acting_user) # all users deleted return chat_channel.id if !users.first - usernames_formatted = users.map { |u| "@#{u.username}" } + usernames_formatted = users.sort_by(&:username).map { |u| "@#{u.username}" } if usernames_formatted.size > 5 return I18n.t( "chat.channel.dm_title.multi_user_truncated", diff --git a/app/models/user_chat_channel_membership.rb b/app/models/user_chat_channel_membership.rb index fdb34080b..c72494a4a 100644 --- a/app/models/user_chat_channel_membership.rb +++ b/app/models/user_chat_channel_membership.rb @@ -38,16 +38,17 @@ def changes_for_direct_message_channels # # Table name: user_chat_channel_memberships # -# id :bigint not null, primary key -# user_id :integer not null -# chat_channel_id :integer not null -# last_read_message_id :integer -# following :boolean default(FALSE), not null -# muted :boolean default(FALSE), not null -# desktop_notification_level :integer default("mention"), not null -# mobile_notification_level :integer default("mention"), not null -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint not null, primary key +# user_id :integer not null +# chat_channel_id :integer not null +# last_read_message_id :integer +# following :boolean default(FALSE), not null +# muted :boolean default(FALSE), not null +# desktop_notification_level :integer default("mention"), not null +# mobile_notification_level :integer default("mention"), not null +# created_at :datetime not null +# updated_at :datetime not null +# last_read_message_when_emailed_id :integer # # Indexes # diff --git a/app/views/user_notifications/chat_summary.html.erb b/app/views/user_notifications/chat_summary.html.erb new file mode 100644 index 000000000..6c84a6215 --- /dev/null +++ b/app/views/user_notifications/chat_summary.html.erb @@ -0,0 +1,76 @@ +
+ + + + + + + +
+ + + <%- if logo_url.blank? %> + <%= SiteSetting.title %> + <%- else %> + <%= SiteSetting.title %> + <%- end %> + + +
+ <%= I18n.t("user_notifications.chat_summary.description", count: @messages.size) %> +
+ + <%- @grouped_mentions.each do |chat_channel, messages| %> + <%- other_messages_count = messages.size - 2 %> + + + + + + <%- messages.take(2).each do |chat_message| %> + <%- sender = chat_message.user %> + <%- sender_name = @display_usernames ? sender.username : sender.name %> + + + + + + <%- end %> + + + + +
+
<%= chat_channel.title(@user) %>
+
+ <%= sender_name -%> + + <%= sender_name -%> + + + <%= I18n.l(@user_tz.to_local(chat_message.created_at), format: :long) -%> + + + <%= email_excerpt(chat_message.cooked_for_excerpt) %> +
+ + <%- if 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 %> +
+ + + + + +
+ <%= raw(t 'user_notifications.chat_summary.unsubscribe', + site_link: html_site_link, + email_preferences_link: link_to(t('user_notifications.chat_summary.your_chat_settings'), Discourse.base_url + '/my/preferences/chat')) + %> +
diff --git a/app/views/user_notifications/chat_summary.text.erb b/app/views/user_notifications/chat_summary.text.erb new file mode 100644 index 000000000..806b5eca1 --- /dev/null +++ b/app/views/user_notifications/chat_summary.text.erb @@ -0,0 +1,8 @@ +<%- 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")) %> +<%= raw(t :'user_notifications.chat_summary.unsubscribe', + site_link: site_link, + email_preferences_link: @markdown_linker.create(t('user_notifications.chat_summary.your_chat_settings'), Discourse.base_url + '/my/preferences/chat')) +%> +<%= raw(@markdown_linker.references) %> diff --git a/assets/javascripts/discourse/controllers/preferences-chat.js b/assets/javascripts/discourse/controllers/preferences-chat.js index 92f0e1839..602f3ce6a 100644 --- a/assets/javascripts/discourse/controllers/preferences-chat.js +++ b/assets/javascripts/discourse/controllers/preferences-chat.js @@ -6,15 +6,26 @@ import { action } from "@ember/object"; import { popupAjaxError } from "discourse/lib/ajax-error"; import { CHAT_SOUNDS } from "discourse/plugins/discourse-chat/discourse/initializers/chat-notification-sounds"; -const chatAttrs = [ +const CHAT_ATTRS = [ "chat_enabled", "chat_isolated", "only_chat_push_notifications", "ignore_channel_wide_mention", "chat_sound", + "chat_email_frequency", +]; + +const EMAIL_FREQUENCY_OPTIONS = [ + { name: I18n.t(`chat.email_frequency.never`), value: "never" }, + { name: I18n.t(`chat.email_frequency.when_away`), value: "when_away" }, ]; export default Controller.extend({ + init() { + this._super(...arguments); + this.set("emailFrequencyOptions", EMAIL_FREQUENCY_OPTIONS); + }, + @discourseComputed chatSounds() { return Object.keys(CHAT_SOUNDS).map((value) => { @@ -35,7 +46,7 @@ export default Controller.extend({ save() { this.set("saved", false); return this.model - .save(chatAttrs) + .save(CHAT_ATTRS) .then(() => { this.set("saved", true); if (!isTesting()) { diff --git a/assets/javascripts/discourse/initializers/chat-user-options.js b/assets/javascripts/discourse/initializers/chat-user-options.js index 7455f4db2..a05f39eef 100644 --- a/assets/javascripts/discourse/initializers/chat-user-options.js +++ b/assets/javascripts/discourse/initializers/chat-user-options.js @@ -5,6 +5,7 @@ const CHAT_ISOLATED_FIELD = "chat_isolated"; const ONLY_CHAT_PUSH_NOTI_FIELD = "only_chat_push_notifications"; const IGNORE_CHANNEL_WIDE_MENTION = "ignore_channel_wide_mention"; const CHAT_SOUND = "chat_sound"; +const CHAT_EMAIL_FREQUENCY = "chat_email_frequency"; export default { name: "chat-user-options", @@ -18,6 +19,7 @@ export default { api.addSaveableUserOptionField(ONLY_CHAT_PUSH_NOTI_FIELD); api.addSaveableUserOptionField(IGNORE_CHANNEL_WIDE_MENTION); api.addSaveableUserOptionField(CHAT_SOUND); + api.addSaveableUserOptionField(CHAT_EMAIL_FREQUENCY); } }); }, diff --git a/assets/javascripts/discourse/templates/preferences/chat.hbs b/assets/javascripts/discourse/templates/preferences/chat.hbs index fcb196cf5..dfa6d6496 100644 --- a/assets/javascripts/discourse/templates/preferences/chat.hbs +++ b/assets/javascripts/discourse/templates/preferences/chat.hbs @@ -59,4 +59,18 @@ }} +
+ + {{combo-box + valueProperty="value" + content=emailFrequencyOptions + value=model.user_option.chat_email_frequency + id="user_chat_email_frequency" + onChange=(action (mut model.user_option.chat_email_frequency)) + }} + {{#if (eq model.user_option.chat_email_frequency "when_away")}} +
{{i18n "chat.email_frequency.description"}}
+ {{/if}} +
+ {{save-controls id="user_chat_preference_save" model=model action=(action "save") saved=saved}} diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 75c1bf10d..68483f9d8 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -70,6 +70,11 @@ en: public_available: "There are public channels available for you to follow." start_direct_message: "You can also start a personal chat with one or more users." title: "Get started by joining a channel" + email_frequency: + description: "We'll only email you if we haven't seen you in the last 15 minutes." + never: "Never" + title: "Email Notifications" + when_away: "Only when away" enable: "Enable chat" flag: "Flag" flagged: "This message has been flagged for review" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3fe817428..cbbe5cee0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -116,3 +116,22 @@ en: reviewable_score_types: needs_review: title: "Needs Review" + + user_notifications: + chat_summary: + deleted_user: "Deleted user" + description: + one: "You have a new chat message" + other: "You have new chat messages" + from: "%{site_name}" + subject: + one: "[%{email_prefix}] New chat message" + other: "[%{email_prefix}] New chat messages" + unsubscribe: "This chat summary is sent from %{site_link} when you are away. Change your %{email_preferences_link}." + view_messages: + one: "View message" + other: "View %{count} messages" + view_more: + one: "View %{count} more message" + other: "View %{count} more messages" + your_chat_settings: "chat email frequency preference" diff --git a/db/migrate/20220518140004_track_last_unread_mention_when_emailed.rb b/db/migrate/20220518140004_track_last_unread_mention_when_emailed.rb new file mode 100644 index 000000000..bf914e8da --- /dev/null +++ b/db/migrate/20220518140004_track_last_unread_mention_when_emailed.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class TrackLastUnreadMentionWhenEmailed < ActiveRecord::Migration[7.0] + def change + add_column :user_chat_channel_memberships, :last_unread_mention_when_emailed_id, :integer + end +end diff --git a/db/post_migrate/20220516142658_remove_email_statuses_table.rb b/db/post_migrate/20220516142658_remove_email_statuses_table.rb new file mode 100644 index 000000000..6f40b4ece --- /dev/null +++ b/db/post_migrate/20220516142658_remove_email_statuses_table.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class RemoveEmailStatusesTable < ActiveRecord::Migration[7.0] + def up + remove_index :chat_message_email_statuses, :status + remove_index :chat_message_email_statuses, [:user_id, :chat_message_id] + + Migration::TableDropper.execute_drop("chat_message_email_statuses") + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/post_migrate/20220518180642_remove_user_option_last_emailed_at.rb b/db/post_migrate/20220518180642_remove_user_option_last_emailed_at.rb new file mode 100644 index 000000000..d55b9eec2 --- /dev/null +++ b/db/post_migrate/20220518180642_remove_user_option_last_emailed_at.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class RemoveUserOptionLastEmailedAt < ActiveRecord::Migration[7.0] + def change + remove_column :user_options, :last_emailed_for_chat, :datetime + end +end diff --git a/lib/chat_mailer.rb b/lib/chat_mailer.rb new file mode 100644 index 000000000..3a7bf468a --- /dev/null +++ b/lib/chat_mailer.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +class DiscourseChat::ChatMailer + def send_unread_mentions_summary + return unless SiteSetting.chat_enabled + + users_with_unprocessed_unread_mentions.find_each do |user| + # 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 users_with_unprocessed_unread_mentions + when_away_frequency = UserOption.chat_email_frequencies[:when_away] + allowed_group_ids = DiscourseChat.allowed_group_ids + + User + .select('users.id', 'ARRAY_AGG(ARRAY[uccm.id, c_msg.id]) AS memberships_with_unread_messages') + .joins(:user_option) + .where(user_options: { chat_enabled: true, chat_email_frequency: when_away_frequency }) + .where('users.last_seen_at < ?', 15.minutes.ago) + .joins(:groups) + .where(groups: { id: allowed_group_ids }) + .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') + .where(uccm: { following: true }) + .where('c_msg.deleted_at IS NULL AND c_msg.user_id <> users.id') + .where( + <<~SQL + (c_mentions.user_id = uccm.user_id OR cc.chatable_type = 'DirectMessageChannel') AND + (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) + SQL + ) + .group('users.id, uccm.user_id') + end +end diff --git a/lib/chat_message_creator.rb b/lib/chat_message_creator.rb index 4d597f4b4..c69971118 100644 --- a/lib/chat_message_creator.rb +++ b/lib/chat_message_creator.rb @@ -39,7 +39,10 @@ def create 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 }) - DiscourseChat::ChatNotifier.notify_new(chat_message: @chat_message, timestamp: @chat_message.created_at) + DiscourseChat::ChatNotifier.notify_new( + chat_message: @chat_message, + timestamp: @chat_message.created_at + ) rescue => error @error = error end @@ -53,6 +56,7 @@ def failed? def validate_user_permissions! return if @guardian.can_create_chat_message! + raise StandardError.new( I18n.t("chat.errors.user_cannot_send_message") ) @@ -60,6 +64,7 @@ def validate_user_permissions! def validate_channel_status! return if @guardian.can_create_channel_message?(@chat_channel) + raise StandardError.new( I18n.t("chat.errors.channel_new_message_disallowed", status: @chat_channel.status_name) ) diff --git a/lib/chat_message_updater.rb b/lib/chat_message_updater.rb index 9accdaa08..88e3bb3fd 100644 --- a/lib/chat_message_updater.rb +++ b/lib/chat_message_updater.rb @@ -31,7 +31,10 @@ def update revision = save_revision! ChatPublisher.publish_edit!(@chat_channel, @chat_message) Jobs.enqueue(:process_chat_message, { chat_message_id: @chat_message.id }) - DiscourseChat::ChatNotifier.notify_edit(chat_message: @chat_message, timestamp: revision.created_at) + DiscourseChat::ChatNotifier.notify_edit( + chat_message: @chat_message, + timestamp: revision.created_at + ) rescue => error @error = error end diff --git a/lib/chat_notifier.rb b/lib/chat_notifier.rb index f5fe09527..955242d2c 100644 --- a/lib/chat_notifier.rb +++ b/lib/chat_notifier.rb @@ -185,12 +185,17 @@ def expand_direct_mentions(to_notify, already_covered_ids) end def group_name_mentions - @group_mentions_from_cooked ||= Nokogiri::HTML5.fragment(@chat_message.cooked).css(".mention-group").map(&:text) + @group_mentions_from_cooked ||= normalized_mentions( + Nokogiri::HTML5 + .fragment(@chat_message.cooked) + .css(".mention-group") + .map(&:text) + ) end def mentionable_groups @mentionable_groups ||= Group.mentionable(@user, include_public: false) - .where('LOWER(name) IN (?)', normalized_mentions(group_name_mentions)) + .where('LOWER(name) IN (?)', group_name_mentions) end def expand_group_mentions(to_notify, already_covered_ids) @@ -205,8 +210,13 @@ def expand_group_mentions(to_notify, already_covered_ids) grouped = group_users_to_notify(reached_by_group) grouped[:already_participating].each do |user| - group = user.groups.detect { |g| mentionable_groups.include?(g) } - to_notify[group.name.downcase] << user.id + # 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_groups.map { |mg| mg.name.downcase } + user_group_names = user.groups.map { |ug| ug.name.downcase } + group_name = ordered_group_names.detect { |gn| user_group_names.include?(gn) } + + to_notify[group_name] << user.id end already_covered_ids.concat(grouped[:already_participating]) diff --git a/lib/extensions/user_email_extension.rb b/lib/extensions/user_email_extension.rb new file mode 100644 index 000000000..b52ee566b --- /dev/null +++ b/lib/extensions/user_email_extension.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module DiscourseChat::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| + 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 diff --git a/lib/extensions/user_notifications_extension.rb b/lib/extensions/user_notifications_extension.rb new file mode 100644 index 000000000..fa1963fe4 --- /dev/null +++ b/lib/extensions/user_notifications_extension.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module DiscourseChat::UserNotificationsExtension + def chat_summary(user, opts) + guardian = Guardian.new(user) + return unless guardian.can_chat?(user) + + @messages = ChatMessage + .joins(:user, :chat_channel) + .where.not(user: user) + .joins('LEFT OUTER JOIN chat_mentions cm ON cm.chat_message_id = chat_messages.id') + .joins('INNER JOIN user_chat_channel_memberships uccm ON uccm.chat_channel_id = chat_channels.id') + .where(uccm: { following: true, user_id: user.id }) + .where( + <<~SQL + (cm.user_id = #{user.id} OR chat_channels.chatable_type = 'DirectMessageChannel') 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) + SQL + ).to_a + return if @messages.empty? + + @grouped_mentions = @messages.group_by { |message| message.chat_channel } + @grouped_mentions = @grouped_mentions.select { |channel, _| guardian.can_see_chat_channel?(channel) } + return if @grouped_mentions.empty? + + @grouped_mentions.each do |chat_channel, mentions| + @grouped_mentions[chat_channel] = mentions.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) + opts = { + from_alias: I18n.t('user_notifications.chat_summary.from', site_name: Email.site_title), + subject: I18n.t('user_notifications.chat_summary.subject', count: @messages.size, email_prefix: @email_prefix, date: short_date(Time.now)), + add_unsubscribe_link: false, + } + + build_email(user.email, opts) + end +end diff --git a/lib/extensions/user_option_extension.rb b/lib/extensions/user_option_extension.rb new file mode 100644 index 000000000..c4bdd3966 --- /dev/null +++ b/lib/extensions/user_option_extension.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module DiscourseChat::UserOptionExtension + def self.prepended(base) + def base.chat_email_frequencies + @chat_email_frequencies ||= { + never: 0, + when_away: 1 + } + end + + base.enum chat_email_frequency: base.chat_email_frequencies + end +end diff --git a/plugin.rb b/plugin.rb index e6a232651..52dbfa8cb 100644 --- a/plugin.rb +++ b/plugin.rb @@ -108,6 +108,7 @@ def self.allowed_group_ids load File.expand_path('../app/serializers/user_chat_message_bookmark_serializer.rb', __FILE__) load File.expand_path('../app/serializers/reviewable_chat_message_serializer.rb', __FILE__) load File.expand_path('../lib/chat_channel_fetcher.rb', __FILE__) + load File.expand_path('../lib/chat_mailer.rb', __FILE__) load File.expand_path('../lib/chat_message_creator.rb', __FILE__) load File.expand_path('../lib/chat_message_processor.rb', __FILE__) load File.expand_path('../lib/chat_message_updater.rb', __FILE__) @@ -123,6 +124,9 @@ def self.allowed_group_ids load File.expand_path('../lib/guardian_extensions.rb', __FILE__) load File.expand_path('../lib/extensions/topic_view_serializer_extension.rb', __FILE__) load File.expand_path('../lib/extensions/detailed_tag_serializer_extension.rb', __FILE__) + load File.expand_path('../lib/extensions/user_option_extension.rb', __FILE__) + load File.expand_path('../lib/extensions/user_notifications_extension.rb', __FILE__) + load File.expand_path('../lib/extensions/user_email_extension.rb', __FILE__) load File.expand_path('../lib/slack_compatibility.rb', __FILE__) load File.expand_path('../lib/post_notification_handler.rb', __FILE__) load File.expand_path('../app/jobs/regular/process_chat_message.rb', __FILE__) @@ -132,6 +136,7 @@ def self.allowed_group_ids load File.expand_path('../app/jobs/regular/chat_notify_watching.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__) load File.expand_path('../app/services/chat_publisher.rb', __FILE__) if Discourse.allow_dev_populate? @@ -140,6 +145,8 @@ def self.allowed_group_ids load File.expand_path('../lib/discourse_dev/message.rb', __FILE__) end + UserNotifications.append_view_path(File.expand_path('../app/views', __FILE__)) + register_topic_custom_field_type(DiscourseChat::HAS_CHAT_ENABLED, :boolean) register_category_custom_field_type(DiscourseChat::HAS_CHAT_ENABLED, :boolean) @@ -148,6 +155,7 @@ def self.allowed_group_ids UserUpdater::OPTION_ATTR.push(:only_chat_push_notifications) UserUpdater::OPTION_ATTR.push(:chat_sound) UserUpdater::OPTION_ATTR.push(:ignore_channel_wide_mention) + UserUpdater::OPTION_ATTR.push(:chat_email_frequency) register_reviewable_type ReviewableChatMessage @@ -166,6 +174,8 @@ def self.allowed_group_ids Guardian.class_eval { include DiscourseChat::GuardianExtensions } TopicViewSerializer.class_eval { prepend DiscourseChat::TopicViewSerializerExtension } DetailedTagSerializer.class_eval { prepend DiscourseChat::DetailedTagSerializerExtension } + UserNotifications.class_eval { prepend DiscourseChat::UserNotificationsExtension } + UserOption.class_eval { prepend DiscourseChat::UserOptionExtension } Topic.class_eval { has_one :chat_channel, as: :chatable } @@ -177,6 +187,7 @@ def self.allowed_group_ids has_many :chat_message_reactions, dependent: :destroy has_many :chat_mentions } + Jobs::UserEmail.class_eval { prepend DiscourseChat::UserEmailExtension } Bookmark.register_bookmarkable(ChatMessageBookmarkable) end @@ -317,6 +328,10 @@ def self.allowed_group_ids object.ignore_channel_wide_mention end + add_to_serializer(:user_option, :chat_email_frequency) do + object.chat_email_frequency + end + RETENTION_SETTINGS_TO_USER_OPTION_FIELDS = { chat_channel_retention_days: :dismissed_channel_retention_reminder, chat_dm_retention_days: :dismissed_dm_retention_reminder @@ -483,4 +498,10 @@ def self.allowed_group_ids params: %i[chat_channel_id] } }) + + # Dark mode email styles + Email::Styles.register_plugin_style do |fragment| + fragment.css('.chat-summary-header').each { |element| element[:dm] = 'header' } + fragment.css('.chat-summary-content').each { |element| element[:dm] = 'body' } + end end diff --git a/spec/components/chat_mailer_spec.rb b/spec/components/chat_mailer_spec.rb new file mode 100644 index 000000000..40e9edbaf --- /dev/null +++ b/spec/components/chat_mailer_spec.rb @@ -0,0 +1,219 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe DiscourseChat::ChatMailer do + before do + SiteSetting.chat_enabled = true + + @chatters_group = Fabricate(:group) + SiteSetting.chat_allowed_groups = @chatters_group.id + + @sender = Fabricate(:user, group_ids: [@chatters_group.id]) + @user_1 = Fabricate(:user, group_ids: [@chatters_group.id], last_seen_at: 15.minutes.ago) + + @chat_channel = Fabricate(:chat_channel) + @chat_message = Fabricate(:chat_message, user: @sender, chat_channel: @chat_channel) + + Fabricate(:user_chat_channel_membership, user: @sender, chat_channel: @chat_channel) + + @user_membership = Fabricate( + :user_chat_channel_membership, user: @user_1, + chat_channel: @chat_channel, last_read_message_id: nil + ) + + @private_channel = DiscourseChat::DirectMessageChannelCreator.create!([@sender, @user_1]) + end + + def asert_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 }) + expect(Jobs::UserEmail.jobs.size).to eq(1) + end + + describe 'for chat mentions' do + before do + @mention = Fabricate(:chat_mention, user: @user_1, chat_message: @chat_message) + end + + it 'skips users without chat access' do + @chatters_group.remove(@user_1) + + subject.send_unread_mentions_summary + + asert_summary_skipped + end + + it 'skips users with summaries disabled' do + @user_1.user_option.update(chat_email_frequency: UserOption.chat_email_frequencies[:never]) + + subject.send_unread_mentions_summary + + asert_summary_skipped + end + + it "skips a job if the user haven't read the channel since the last summary" do + @user_membership.update!(last_unread_mention_when_emailed_id: @chat_message.id) + + subject.send_unread_mentions_summary + + asert_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]) + + subject.send_unread_mentions_summary + + asert_summary_skipped + end + + it 'queues a job for users that was mentioned and never read the channel before' do + subject.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_membership.update!(last_read_message_id: @chat_message.id) + + subject.send_unread_mentions_summary + + asert_summary_skipped + end + + it 'skips the job when the user is not following the channel anymore' do + @user_membership.update!(last_read_message_id: @chat_message.id - 1, following: false) + + subject.send_unread_mentions_summary + + asert_summary_skipped + end + + it 'skips users with unread messages from a different channel' do + @user_membership.update!(last_read_message_id: @chat_message.id) + second_channel = Fabricate(:chat_channel) + Fabricate(:user_chat_channel_membership, user: @user_1, chat_channel: second_channel, last_read_message_id: @chat_message.id - 1) + + subject.send_unread_mentions_summary + + asert_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('|') + + subject.send_unread_mentions_summary + + assert_only_queued_once + end + + it 'skips users when the mention was deleted' do + @chat_message.trash! + + subject.send_unread_mentions_summary + + asert_summary_skipped + end + + it 'queues the job if the user has unread mentions and alread read all the messages in the previous summary' do + @user_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) + Fabricate(:chat_mention, user: @user_1, chat_message: unread_message) + + subject.send_unread_mentions_summary + + 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 'skips users who were seen recently' do + @user_1.update!(last_seen_at: 2.minutes.ago) + + subject.send_unread_mentions_summary + + asert_summary_skipped + 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) + user_2_membership = 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) + Fabricate(:chat_mention, user: user_2, chat_message: new_message) + + subject.send_unread_mentions_summary + + 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 + + describe 'update the user membership after we send the email' do + before { Jobs.run_immediately! } + + 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_membership.update!(last_read_message_id: @chat_message.id - 1) + subject.send_unread_mentions_summary + + expect(@user_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(:chat_mention, user: @user_1, chat_message: another_channel_message) + + expect { subject.send_unread_mentions_summary }.to change(Jobs::UserEmail.jobs, :size).by(0) + end + + it 'only updates the last_message_read_when_emailed_id on the channel with unread mentions' do + another_channel = Fabricate(:chat_channel) + another_channel_message = Fabricate(:chat_message, chat_channel: another_channel, user: @sender) + Fabricate(: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_membership.update!(last_read_message_id: @chat_message.id - 1) + + subject.send_unread_mentions_summary + + expect(@user_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 + end + end + end + + describe 'for direct messages' do + before do + Fabricate(:chat_message, user: @sender, chat_channel: @private_channel) + end + + it 'queue a job when the user has unread private mentions' do + subject.send_unread_mentions_summary + + assert_only_queued_once + end + + it 'only queues the job once when the user has mentions and private messages' do + Fabricate(:chat_mention, user: @user_1, chat_message: @chat_message) + + subject.send_unread_mentions_summary + + assert_only_queued_once + 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(:chat_mention, user: user_2, chat_message: @chat_message) + + subject.send_unread_mentions_summary + + assert_only_queued_once + expect(user_2_membership.reload.last_unread_mention_when_emailed_id).to be_nil + end + end +end diff --git a/spec/components/chat_message_creator_spec.rb b/spec/components/chat_message_creator_spec.rb index 68f1995f1..80af00f6b 100644 --- a/spec/components/chat_message_creator_spec.rb +++ b/spec/components/chat_message_creator_spec.rb @@ -12,6 +12,7 @@ fab!(:user_group) { Fabricate(:public_group, users: [user1, user2, user3], mentionable_level: Group::ALIAS_LEVELS[:everyone]) } fab!(:user_without_memberships) { Fabricate(:user) } fab!(:public_chat_channel) { Fabricate(:chat_channel, chatable: Fabricate(:topic)) } + fab!(:dm_chat_channel) { Fabricate(:chat_channel, chatable: Fabricate(:direct_message_channel, users: [user1, user2, user3])) } before do SiteSetting.chat_enabled = true diff --git a/spec/components/chat_message_updater_spec.rb b/spec/components/chat_message_updater_spec.rb index db692b756..56b804c2c 100644 --- a/spec/components/chat_message_updater_spec.rb +++ b/spec/components/chat_message_updater_spec.rb @@ -134,19 +134,20 @@ def create_chat_message(user, message, channel, upload_ids: nil) end describe "group mentions" do - it "sends group mentions on update" do + it "creates group mentions on update" do chat_message = create_chat_message(user1, "ping nobody", public_chat_channel) expect { DiscourseChat::ChatMessageUpdater.update( chat_message: chat_message, new_content: "ping @#{admin_group.name}" ) - }.to change { ChatMention.count }.by(2) + }.to change { ChatMention.where(chat_message: chat_message).count }.by(2) + expect(admin1.chat_mentions.where(chat_message: chat_message)).to be_present expect(admin2.chat_mentions.where(chat_message: chat_message)).to be_present end - it "doesn't repeat mentions when the user is already direct mentioned and then group mentioned" do + it "doesn't duplicate mentions when the user is already direct mentioned and then group mentioned" do chat_message = create_chat_message(user1, "ping @#{admin2.username}", public_chat_channel) expect { DiscourseChat::ChatMessageUpdater.update( @@ -167,6 +168,7 @@ def create_chat_message(user, message, channel, upload_ids: nil) new_content: "ping nobody anymore!" ) }.to change { ChatMention.where(chat_message: chat_message).count }.by(-2) + expect(admin1.chat_mentions.where(chat_message: chat_message)).not_to be_present expect(admin2.chat_mentions.where(chat_message: chat_message)).not_to be_present end diff --git a/spec/lib/chat_notifier_spec.rb b/spec/lib/chat_notifier_spec.rb index 919cedc9f..1ce3bf0fd 100644 --- a/spec/lib/chat_notifier_spec.rb +++ b/spec/lib/chat_notifier_spec.rb @@ -9,7 +9,7 @@ fab!(:user_2) { Fabricate(:user) } before do - @chat_group = Fabricate(:group, users: [user_1, user_2]) + @chat_group = Fabricate(:group, users: [user_1, user_2], mentionable_level: Group::ALIAS_LEVELS[:everyone]) SiteSetting.chat_allowed_groups = @chat_group.id [user_1, user_2].each { |u| Fabricate(:user_chat_channel_membership, chat_channel: channel, user: u) } @@ -176,6 +176,23 @@ def build_cooked_msg(message_body, user, chat_channel: channel) let(:list_key) { group.name } include_examples 'ensure only channel members are notified' + + it 'establishes a far-left precedence among group mentions' do + Fabricate(:user_chat_channel_membership, chat_channel: channel, user: user_3, following: true) + msg = build_cooked_msg("Hello @#{@chat_group.name} and @#{group.name}", user_1) + + to_notify = described_class.new(msg, msg.created_at).notify_new + + expect(to_notify[@chat_group.name]).to contain_exactly(user_2.id, user_3.id) + expect(to_notify[list_key]).to be_empty + + second_msg = build_cooked_msg("Hello @#{group.name} and @#{@chat_group.name}", user_1) + + to_notify_2 = described_class.new(second_msg, second_msg.created_at).notify_new + + expect(to_notify_2[list_key]).to contain_exactly(user_2.id, user_3.id) + expect(to_notify_2[@chat_group.name]).to be_empty + end end describe 'unreachable users' do diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb new file mode 100644 index 000000000..598d3994f --- /dev/null +++ b/spec/mailers/user_notifications_spec.rb @@ -0,0 +1,230 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe UserNotifications do + describe '.chat_summary' do + before do + @chatters_group = Fabricate(:group) + @sender = Fabricate(:user, name: 'A name', username: 'username', group_ids: [@chatters_group.id]) + @user = Fabricate(:user, group_ids: [@chatters_group.id]) + + @chat_channel = Fabricate(:chat_channel) + @chat_message = Fabricate(:chat_message, user: @sender, chat_channel: @chat_channel) + + Fabricate(:user_chat_channel_membership, chat_channel: @chat_channel, user: @sender) + @user_membership = Fabricate(:user_chat_channel_membership, chat_channel: @chat_channel, user: @user, last_read_message_id: @chat_message.id - 2) + + SiteSetting.chat_enabled = true + SiteSetting.chat_allowed_groups = @chatters_group.id + end + + it "doesn't return an email if there are no unread mentions" do + email = described_class.chat_summary(@user, {}) + + expect(email.to).to be_blank + end + + describe 'When there are mentions' do + before { Fabricate(:chat_mention, user: @user, chat_message: @chat_message) } + + describe 'selecting mentions' do + it "doesn't return an email if the user can't see chat" do + SiteSetting.chat_allowed_groups = '' + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to be_blank + end + + it "doesn't return an email if the user can't see any of the included channels" do + @chat_channel.chatable.trash! + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to be_blank + end + + it "doesn't return an email if the user is not following the channel" do + @user_membership.update!(following: false) + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to be_blank + end + + it "doesn't return an email if the membership object doesn't exist" do + @user_membership.destroy! + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to be_blank + end + + it "doesn't return an email if the sender was deleted" do + @sender.destroy! + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to be_blank + end + + it "doesn't return an email when the user already saw the mention" do + @user_membership.update!(last_read_message_id: @chat_message.id) + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to be_blank + end + + it "returns an email when the user haven't read a message yet" do + @user_membership.update!(last_read_message_id: nil) + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to contain_exactly(@user.email) + end + + it "doesn't return an email when the unread count belongs to a different channel" do + @user_membership.update!(last_read_message_id: @chat_message.id) + second_channel = Fabricate(:chat_channel) + Fabricate(:user_chat_channel_membership, chat_channel: second_channel, user: @user, last_read_message_id: @chat_message.id - 1) + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to be_blank + end + + it "doesn't return an email if the message was deleted" do + @chat_message.trash! + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to be_blank + end + + it 'returns an email when the user has unread private messages' do + @user_membership.update!(last_read_message_id: @chat_message.id) + private_channel = DiscourseChat::DirectMessageChannelCreator.create!([@sender, @user]) + Fabricate(:chat_message, user: @sender, chat_channel: private_channel) + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to contain_exactly(@user.email) + end + + it 'returns an email if the user read all the messages included in the previous summary' do + @user_membership.update!(last_read_message_id: @chat_message.id, last_unread_mention_when_emailed_id: @chat_message.id) + + new_message = Fabricate(:chat_message, user: @sender, chat_channel: @chat_channel) + Fabricate(:chat_mention, user: @user, chat_message: new_message) + + email = described_class.chat_summary(@user, {}) + + expect(email.to).to contain_exactly(@user.email) + end + end + + describe 'mail contents' do + it 'returns an email when the user has unread mentions' do + email = described_class.chat_summary(@user, {}) + + expect(email.to).to contain_exactly(@user.email) + expect(email.html_part.body.to_s).to include(@chat_message.cooked_for_excerpt) + + user_avatar = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".message-row img") + + expect(user_avatar.attribute('src').value).to eq(@sender.small_avatar_url) + expect(user_avatar.attribute('alt').value).to eq(@sender.username) + + more_messages_channel_link = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".more-messages-link") + + expect(more_messages_channel_link.attribute('href').value).to eq(@chat_message.url) + expect(more_messages_channel_link.text).to include(I18n.t("user_notifications.chat_summary.view_messages", count: 1)) + end + + it "displays the sender's name when the site is configured to prioritize it" do + SiteSetting.enable_names = true + SiteSetting.prioritize_username_in_ux = false + + email = described_class.chat_summary(@user, {}) + + user_name = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".message-row span") + expect(user_name.text).to include(@sender.name) + + user_avatar = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".message-row img") + + expect(user_avatar.attribute('alt').value).to eq(@sender.name) + end + + it "displays the sender's username when the site is configured to prioritize it" do + SiteSetting.enable_names = true + SiteSetting.prioritize_username_in_ux = true + + email = described_class.chat_summary(@user, {}) + + user_name = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".message-row span") + expect(user_name.text).to include(@sender.username) + + user_avatar = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".message-row img") + + expect(user_avatar.attribute('alt').value).to eq(@sender.username) + end + + it "displays the sender's username when names are disabled" do + SiteSetting.enable_names = false + SiteSetting.prioritize_username_in_ux = false + + email = described_class.chat_summary(@user, {}) + + user_name = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".message-row span") + expect(user_name.text).to include(@sender.username) + + user_avatar = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".message-row img") + + expect(user_avatar.attribute('alt').value).to eq(@sender.username) + end + + it "displays the sender's username when the site is configured to prioritize it" do + SiteSetting.enable_names = false + SiteSetting.prioritize_username_in_ux = true + + email = described_class.chat_summary(@user, {}) + + user_name = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".message-row span") + expect(user_name.text).to include(@sender.username) + + user_avatar = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".message-row img") + + expect(user_avatar.attribute('alt').value).to eq(@sender.username) + end + + it 'includes a view more link when there are more than two mentions' do + 2.times do + msg = Fabricate(:chat_message, user: @sender, chat_channel: @chat_channel) + Fabricate(:chat_mention, user: @user, chat_message: msg) + end + + email = described_class.chat_summary(@user, {}) + more_messages_channel_link = Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".more-messages-link") + + expect(more_messages_channel_link.attribute('href').value).to eq(@chat_message.url) + expect(more_messages_channel_link.text).to include(I18n.t("user_notifications.chat_summary.view_more", count: 1)) + end + + it "doesn't repeat mentions we already sent" do + @user_membership.update!(last_read_message_id: @chat_message.id - 1, last_unread_mention_when_emailed_id: @chat_message.id) + + new_message = Fabricate(:chat_message, user: @sender, chat_channel: @chat_channel, cooked: 'New message') + Fabricate(:chat_mention, user: @user, chat_message: new_message) + + email = described_class.chat_summary(@user, {}) + body = email.html_part.body.to_s + + expect(body).not_to include(@chat_message.cooked_for_excerpt) + expect(body).to include(new_message.cooked_for_excerpt) + end + end + end + end +end diff --git a/spec/models/direct_message_channel_spec.rb b/spec/models/direct_message_channel_spec.rb index 8cd718181..9e9b363ba 100644 --- a/spec/models/direct_message_channel_spec.rb +++ b/spec/models/direct_message_channel_spec.rb @@ -29,7 +29,7 @@ expect(direct_message_channel.chat_channel_title_for_user(chat_channel, user1)).to eq( I18n.t( "chat.channel.dm_title.multi_user_truncated", - users: users[1..5].map { |u| "@#{u.username}" }.join(", "), + users: users[1..5].sort_by(&:username).map { |u| "@#{u.username}" }.join(", "), leftover: 2 ) ) diff --git a/test/javascripts/acceptance/chat-test.js b/test/javascripts/acceptance/chat-test.js index b73a3fbb6..ce445b4d5 100644 --- a/test/javascripts/acceptance/chat-test.js +++ b/test/javascripts/acceptance/chat-test.js @@ -1433,11 +1433,11 @@ acceptance("Discourse Chat - chat preferences", function (needs) { assert.equal(currentURL(), "/latest"); }); - test("There are all 5 settings shown", async function (assert) { + test("There are all 6 settings shown", async function (assert) { this.chatService.set("sidebarActive", true); await visit("/u/eviltrout/preferences/chat"); assert.equal(currentURL(), "/u/eviltrout/preferences/chat"); - assert.equal(queryAll(".chat-setting").length, 5); + assert.equal(queryAll(".chat-setting").length, 6); }); test("The user can save the settings", async function (assert) { @@ -1450,6 +1450,8 @@ acceptance("Discourse Chat - chat preferences", function (needs) { await click("#user_chat_ignore_channel_wide_mention"); await selectKit("#user_chat_sounds").expand(); await selectKit("#user_chat_sounds").selectRowByValue("bell"); + await selectKit("#user_chat_email_frequency").expand(); + await selectKit("#user_chat_email_frequency").selectRowByValue("never"); await click(".save-changes"); @@ -1461,6 +1463,7 @@ acceptance("Discourse Chat - chat preferences", function (needs) { chat_sound: "bell", only_chat_push_notifications: true, ignore_channel_wide_mention: true, + chat_email_frequency: "never", }, type: "PUT", }),