From d6374fdc53d481ee6a5081b073a009876612f9e5 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 16 Jun 2023 12:08:26 +1000 Subject: [PATCH] FEATURE: Allow users to manually track threads without replying (#22100) This commit adds a tracking dropdown to each individual thread, similar to topics, that allows the user to change the notification level for a thread manually. Previously the user had to reply to a thread to track it and see unread indicators. Since the user can now manually track threads, the thread index has also been changed to only show threads that the user is a member of, rather than threads that they had sent messages in. Unread indicators also respect the notification level -- Normal level thread tracking will not show unread indicators in the UI when new messages are sent in the thread. --- .../dropdown-select-box-header.js | 4 +- .../select-kit/addon/components/select-kit.js | 1 + ..._user_notifications_settings_controller.rb | 18 ++++ plugins/chat/app/models/chat/thread.rb | 4 + .../app/queries/chat/thread_unreads_query.rb | 7 +- .../chat/tracking_state_report_query.rb | 5 ++ .../chat/base_thread_membership_serializer.rb | 5 ++ .../services/chat/lookup_channel_threads.rb | 26 ++++-- .../update_thread_notification_settings.rb | 70 +++++++++++++++ .../chat/update_user_thread_last_read.rb | 2 +- .../components/chat/thread/header.hbs | 4 + .../components/chat/thread/header.js | 42 +++++++++ .../discourse/lib/chat-notification-levels.js | 9 ++ .../models/user-chat-thread-membership.js | 8 ++ .../discourse/services/chat-api.js | 16 ++++ .../services/chat-subscriptions-manager.js | 21 +++-- .../components/thread-notifications-button.js | 14 +++ plugins/chat/config/locales/client.en.yml | 7 ++ plugins/chat/config/routes.rb | 2 + plugins/chat/lib/chat/message_creator.rb | 8 +- ...hread_replies_count_cache_accuracy_spec.rb | 2 +- plugins/chat/spec/plugin_helper.rb | 9 ++ .../queries/chat/thread_unreads_query_spec.rb | 15 ++++ ...r_notification_settings_controller_spec.rb | 83 +++++++++++++++++ .../chat/lookup_channel_threads_spec.rb | 50 +++++------ ...pdate_thread_notification_settings_spec.rb | 88 +++++++++++++++++++ .../chat/update_user_thread_last_read_spec.rb | 2 +- .../channel_thread_message_echoing_spec.rb | 4 +- .../system/message_thread_indicator_spec.rb | 24 +++-- .../system/page_objects/chat/chat_thread.rb | 15 ++++ .../page_objects/chat/chat_thread_list.rb | 38 -------- .../chat/components/thread_indicator.rb | 9 +- .../chat/components/thread_list.rb | 40 +++++++++ .../chat/spec/system/single_thread_spec.rb | 2 +- .../spec/system/thread_list/drawer_spec.rb | 2 +- .../spec/system/thread_list/full_page_spec.rb | 2 +- .../system/thread_tracking/drawer_spec.rb | 2 +- .../system/thread_tracking/full_page_spec.rb | 34 ++++++- 38 files changed, 583 insertions(+), 111 deletions(-) create mode 100644 plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb create mode 100644 plugins/chat/app/services/chat/update_thread_notification_settings.rb create mode 100644 plugins/chat/assets/javascripts/discourse/lib/chat-notification-levels.js create mode 100644 plugins/chat/assets/javascripts/select-kit/addons/components/thread-notifications-button.js create mode 100644 plugins/chat/spec/requests/chat/api/channel_threads_current_user_notification_settings_controller_spec.rb create mode 100644 plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb delete mode 100644 plugins/chat/spec/system/page_objects/chat/chat_thread_list.rb diff --git a/app/assets/javascripts/select-kit/addon/components/dropdown-select-box/dropdown-select-box-header.js b/app/assets/javascripts/select-kit/addon/components/dropdown-select-box/dropdown-select-box-header.js index 0ffd025191f..01ce7edd47e 100644 --- a/app/assets/javascripts/select-kit/addon/components/dropdown-select-box/dropdown-select-box-header.js +++ b/app/assets/javascripts/select-kit/addon/components/dropdown-select-box/dropdown-select-box-header.js @@ -4,10 +4,12 @@ import { readOnly } from "@ember/object/computed"; export default SingleSelectHeaderComponent.extend({ classNames: ["dropdown-select-box-header"], - classNameBindings: ["btnClassName", "btnStyleClass"], + classNameBindings: ["btnClassName", "btnStyleClass", "btnCustomClasses"], showFullTitle: readOnly("selectKit.options.showFullTitle"), customStyle: readOnly("selectKit.options.customStyle"), + btnCustomClasses: readOnly("selectKit.options.btnCustomClasses"), + btnClassName: computed("showFullTitle", function () { return `btn ${this.showFullTitle ? "btn-icon-text" : "no-text btn-icon"}`; }), diff --git a/app/assets/javascripts/select-kit/addon/components/select-kit.js b/app/assets/javascripts/select-kit/addon/components/select-kit.js index 75f2e876e30..fa2d49b1367 100644 --- a/app/assets/javascripts/select-kit/addon/components/select-kit.js +++ b/app/assets/javascripts/select-kit/addon/components/select-kit.js @@ -1135,6 +1135,7 @@ export default Component.extend( minimum: "options.minimum", i18nPostfix: "options.i18nPostfix", i18nPrefix: "options.i18nPrefix", + btnCustomClasses: "options.btnCustomClasses", castInteger: "options.castInteger", }; diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb new file mode 100644 index 00000000000..e54d5665450 --- /dev/null +++ b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class Chat::Api::ChannelThreadsCurrentUserNotificationsSettingsController < Chat::ApiController + def update + with_service(Chat::UpdateThreadNotificationSettings) do + on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound } + on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } + on_model_not_found(:thread) { raise Discourse::NotFound } + on_success do + render_serialized( + result.membership, + Chat::BaseThreadMembershipSerializer, + root: "membership", + ) + end + end + end +end diff --git a/plugins/chat/app/models/chat/thread.rb b/plugins/chat/app/models/chat/thread.rb index 0012c427a5c..a6723a08cfd 100644 --- a/plugins/chat/app/models/chat/thread.rb +++ b/plugins/chat/app/models/chat/thread.rb @@ -23,6 +23,10 @@ module Chat primary_key: :id, class_name: "Chat::Message" has_many :user_chat_thread_memberships + + # Since the `replies` for the thread can all be deleted, to avoid errors + # in lists and previews of the thread, we can consider the original message + # as the last "reply" in this case, so we don't exclude that here. has_one :last_reply, -> { order("created_at DESC, id DESC") }, class_name: "Chat::Message" enum :status, { open: 0, read_only: 1, closed: 2, archived: 3 }, scopes: false diff --git a/plugins/chat/app/queries/chat/thread_unreads_query.rb b/plugins/chat/app/queries/chat/thread_unreads_query.rb index da1649c217d..8132da091e5 100644 --- a/plugins/chat/app/queries/chat/thread_unreads_query.rb +++ b/plugins/chat/app/queries/chat/thread_unreads_query.rb @@ -52,7 +52,7 @@ module Chat AND chat_messages.thread_id IS NOT NULL AND chat_messages.id != chat_threads.original_message_id AND chat_channels.threading_enabled - AND user_chat_thread_memberships.notification_level != :muted_notification_level + AND user_chat_thread_memberships.notification_level NOT IN (:quiet_notification_levels) ) AS unread_count, 0 AS mention_count, chat_threads.channel_id, @@ -94,7 +94,10 @@ module Chat user_id: user_id, notification_type: ::Notification.types[:chat_mention], limit: MAX_THREADS, - muted_notification_level: ::Chat::UserChatThreadMembership.notification_levels[:muted], + quiet_notification_levels: [ + ::Chat::UserChatThreadMembership.notification_levels[:muted], + ::Chat::UserChatThreadMembership.notification_levels[:normal], + ], ) end end diff --git a/plugins/chat/app/queries/chat/tracking_state_report_query.rb b/plugins/chat/app/queries/chat/tracking_state_report_query.rb index 5593c9a2b1d..490b4188eff 100644 --- a/plugins/chat/app/queries/chat/tracking_state_report_query.rb +++ b/plugins/chat/app/queries/chat/tracking_state_report_query.rb @@ -8,6 +8,11 @@ module Chat # Only channels with threading_enabled set to true will have thread # tracking queried. # + # The unread counts are based on the user's last_read_message_id for + # each membership, as well as the notification_level (in the case of + # thread memberships) and the following/muted settings (in the case of + # channel memberships). + # # @param guardian [Guardian] The current user's guardian # @param channel_ids [Array] The channel IDs to query. Must be provided # if thread_ids are not. diff --git a/plugins/chat/app/serializers/chat/base_thread_membership_serializer.rb b/plugins/chat/app/serializers/chat/base_thread_membership_serializer.rb index f555d662b74..1f85493126e 100644 --- a/plugins/chat/app/serializers/chat/base_thread_membership_serializer.rb +++ b/plugins/chat/app/serializers/chat/base_thread_membership_serializer.rb @@ -3,5 +3,10 @@ module Chat class BaseThreadMembershipSerializer < ApplicationSerializer attributes :notification_level, :thread_id, :last_read_message_id + + def notification_level + Chat::UserChatThreadMembership.notification_levels[object.notification_level] || + Chat::UserChatThreadMembership.notification_levels["normal"] + end end end diff --git a/plugins/chat/app/services/chat/lookup_channel_threads.rb b/plugins/chat/app/services/chat/lookup_channel_threads.rb index 0b73bf65d16..fc5b2c2dadf 100644 --- a/plugins/chat/app/services/chat/lookup_channel_threads.rb +++ b/plugins/chat/app/services/chat/lookup_channel_threads.rb @@ -4,7 +4,10 @@ module Chat # Gets a list of threads for a channel to be shown in an index. # In future pagination and filtering will be added -- for now # we just want to return N threads ordered by the latest - # message that the user has sent in a thread. + # message that a user has sent in a thread. + # + # Only threads that the user is a member of with a notification level + # of normal or tracking will be returned. # # @example # Chat::LookupChannelThreads.call(channel_id: 2, guardian: guardian) @@ -64,20 +67,27 @@ module Chat user: :user_status, ], ) - .select("chat_threads.*, MAX(chat_messages.created_at) AS last_posted_at") - .joins( - "LEFT JOIN chat_messages ON chat_threads.id = chat_messages.thread_id AND chat_messages.chat_channel_id = #{channel.id}", - ) + .joins(:chat_messages, :user_chat_thread_memberships) .joins( "LEFT JOIN chat_messages original_messages ON chat_threads.original_message_id = original_messages.id", ) - .where("chat_messages.user_id = ? OR chat_messages.user_id IS NULL", guardian.user.id) - .where(channel_id: channel.id) + .where( + "chat_threads.channel_id = :channel_id AND chat_messages.chat_channel_id = :channel_id", + channel_id: channel.id, + ) + .where("user_chat_thread_memberships.user_id = ?", guardian.user.id) + .where( + "user_chat_thread_memberships.notification_level IN (?)", + [ + Chat::UserChatThreadMembership.notification_levels[:normal], + Chat::UserChatThreadMembership.notification_levels[:tracking], + ], + ) .where( "original_messages.deleted_at IS NULL AND chat_messages.deleted_at IS NULL AND original_messages.id IS NOT NULL", ) .group("chat_threads.id") - .order("last_posted_at DESC NULLS LAST") + .order("MAX(chat_messages.created_at) DESC") .limit(50) end diff --git a/plugins/chat/app/services/chat/update_thread_notification_settings.rb b/plugins/chat/app/services/chat/update_thread_notification_settings.rb new file mode 100644 index 00000000000..ddb333730ec --- /dev/null +++ b/plugins/chat/app/services/chat/update_thread_notification_settings.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Chat + # Updates the thread notification level for a user, or if the thread + # does not exist, adds the user as a member of the thread before setting + # the notification level. + # + # @example + # Chat::UpdateThreadNotificationSettings.call( + # thread_id: 88, + # channel_id: 2, + # guardian: guardian, + # notification_level: notification_level, + # ) + # + class UpdateThreadNotificationSettings + include Service::Base + + # @!method call(thread_id:, channel_id:, guardian:, notification_level:) + # @param [Integer] thread_id + # @param [Integer] channel_id + # @param [Integer] notification_level + # @param [Guardian] guardian + # @return [Service::Base::Context] + + contract + model :thread, :fetch_thread + policy :can_view_channel + policy :threading_enabled_for_channel + transaction { step :create_or_update_membership } + + # @!visibility private + class Contract + attribute :thread_id, :integer + attribute :channel_id, :integer + attribute :notification_level, :integer + + validates :thread_id, :channel_id, :notification_level, presence: true + + validates :notification_level, + inclusion: { + in: Chat::UserChatThreadMembership.notification_levels.values, + } + end + + private + + def fetch_thread(contract:, **) + Chat::Thread.find_by(id: contract.thread_id, channel_id: contract.channel_id) + end + + def can_view_channel(guardian:, thread:, **) + guardian.can_preview_chat_channel?(thread.channel) + end + + def threading_enabled_for_channel(thread:, **) + thread.channel.threading_enabled + end + + def create_or_update_membership(thread:, guardian:, contract:, **) + membership = thread.membership_for(guardian.user) + if !membership + membership = thread.add(guardian.user) + membership.update!(last_read_message_id: thread.last_reply.id) + end + membership.update!(notification_level: contract.notification_level) + context.membership = membership + end + end +end diff --git a/plugins/chat/app/services/chat/update_user_thread_last_read.rb b/plugins/chat/app/services/chat/update_user_thread_last_read.rb index 4748a4ca01e..030d5603557 100644 --- a/plugins/chat/app/services/chat/update_user_thread_last_read.rb +++ b/plugins/chat/app/services/chat/update_user_thread_last_read.rb @@ -73,7 +73,7 @@ module Chat ::Chat::Publisher.publish_user_tracking_state!( guardian.user, thread.channel, - thread.replies.last, + thread.last_reply, ) end end diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.hbs index 2f4db6282eb..a0354bbb6e0 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.hbs @@ -17,6 +17,10 @@
+ {{#if this.canChangeThreadSettings}} { + this.membership.last_read_message_id = + response.membership.last_read_message_id; + }) + .catch((err) => { + this.membership.notificationLevel = currentNotificationLevel; + popupAjaxError(err); + }); + } } diff --git a/plugins/chat/assets/javascripts/discourse/lib/chat-notification-levels.js b/plugins/chat/assets/javascripts/discourse/lib/chat-notification-levels.js new file mode 100644 index 00000000000..d23c3d97823 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/lib/chat-notification-levels.js @@ -0,0 +1,9 @@ +import { + NotificationLevels, + buttonDetails, +} from "discourse/lib/notification-levels"; + +export const threadNotificationButtonLevels = [ + NotificationLevels.TRACKING, + NotificationLevels.REGULAR, +].map(buttonDetails); diff --git a/plugins/chat/assets/javascripts/discourse/models/user-chat-thread-membership.js b/plugins/chat/assets/javascripts/discourse/models/user-chat-thread-membership.js index f45fd0bba94..4dd2ddd94a9 100644 --- a/plugins/chat/assets/javascripts/discourse/models/user-chat-thread-membership.js +++ b/plugins/chat/assets/javascripts/discourse/models/user-chat-thread-membership.js @@ -1,4 +1,5 @@ import { tracked } from "@glimmer/tracking"; +import { NotificationLevels } from "discourse/lib/notification-levels"; export default class UserChatThreadMembership { static create(args = {}) { @@ -12,4 +13,11 @@ export default class UserChatThreadMembership { this.lastReadMessageId = args.last_read_message_id; this.notificationLevel = args.notification_level; } + + get isQuiet() { + return ( + this.notificationLevel === NotificationLevels.REGULAR || + this.notificationLevel === NotificationLevels.MUTED + ); + } } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js index cc332084da9..059a0dc3edb 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js @@ -294,6 +294,22 @@ export default class ChatApi extends Service { ); } + /** + * Update notifications settings of current user for a thread. + * @param {number} channelId - The ID of the channel. + * @param {number} threadId - The ID of the thread. + * @param {object} data - The settings to modify. + * @param {boolean} [data.notification_level] - The new notification level, c.f. Chat::NotificationLevels. Threads only support + * "regular" and "tracking" for now. + * @returns {Promise} + */ + updateCurrentUserThreadNotificationsSettings(channelId, threadId, data) { + return this.#putRequest( + `/channels/${channelId}/threads/${threadId}/notifications-settings/me`, + { notification_level: data.notificationLevel } + ); + } + /** * Saves a draft for the channel, which includes message contents and uploads. * @param {number} channelId - The ID of the channel. diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js index 7c45dad6788..641f0e5a7e4 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js @@ -242,7 +242,8 @@ export default class ChatSubscriptionsManager extends Service { if ( thread.currentUserMembership && busData.message_id > - (thread.currentUserMembership.lastReadMessageId || 0) + (thread.currentUserMembership.lastReadMessageId || 0) && + !thread.currentUserMembership.isQuiet ) { channel.unreadThreadIds.add(busData.thread_id); thread.tracking.unreadCount++; @@ -301,9 +302,7 @@ export default class ChatSubscriptionsManager extends Service { @bind _updateChannelTrackingData(channelId, busData) { this.chatChannelsManager.find(channelId).then((channel) => { - if (busData.thread_id) { - // TODO (martin) Update thread membership last read message ID on client. - } else { + if (!busData.thread_id) { channel.currentUserMembership.lastReadMessageId = busData.last_read_message_id; } @@ -319,9 +318,17 @@ export default class ChatSubscriptionsManager extends Service { channel.threadsManager .find(channelId, busData.thread_id) .then((thread) => { - thread.tracking.unreadCount = busData.thread_tracking.unread_count; - thread.tracking.mentionCount = - busData.thread_tracking.mention_count; + if ( + thread.currentUserMembership && + !thread.currentUserMembership.isQuiet + ) { + thread.currentUserMembership.lastReadMessageId = + busData.last_read_message_id; + thread.tracking.unreadCount = + busData.thread_tracking.unread_count; + thread.tracking.mentionCount = + busData.thread_tracking.mention_count; + } }); } }); diff --git a/plugins/chat/assets/javascripts/select-kit/addons/components/thread-notifications-button.js b/plugins/chat/assets/javascripts/select-kit/addons/components/thread-notifications-button.js new file mode 100644 index 00000000000..8480d2b07bc --- /dev/null +++ b/plugins/chat/assets/javascripts/select-kit/addons/components/thread-notifications-button.js @@ -0,0 +1,14 @@ +import NotificationsButtonComponent from "select-kit/components/notifications-button"; +import { threadNotificationButtonLevels } from "discourse/plugins/chat/discourse/lib/chat-notification-levels"; + +export default NotificationsButtonComponent.extend({ + pluginApiIdentifiers: ["thread-notifications-button"], + classNames: ["thread-notifications-button"], + content: threadNotificationButtonLevels, + + selectKitOptions: { + i18nPrefix: "chat.thread.notifications", + showFullTitle: false, + btnCustomClasses: "btn-flat", + }, +}); diff --git a/plugins/chat/config/locales/client.en.yml b/plugins/chat/config/locales/client.en.yml index dac54a5c841..0e72ff0583f 100644 --- a/plugins/chat/config/locales/client.en.yml +++ b/plugins/chat/config/locales/client.en.yml @@ -565,6 +565,13 @@ en: started_by: "Started by" settings: "Settings" last_reply: "last reply" + notifications: + regular: + title: "Normal" + description: "You will be notified if someone mentions your @name in this thread." + tracking: + title: "Tracking" + description: "A count of new replies for this thread will be shown in the thread list and the channel. You will be notified if someone mentions your @name in this thread." participants_other_count: one: "+%{count} other" other: "+%{count} others" diff --git a/plugins/chat/config/routes.rb b/plugins/chat/config/routes.rb index d00c8c223b9..2ed39767943 100644 --- a/plugins/chat/config/routes.rb +++ b/plugins/chat/config/routes.rb @@ -32,6 +32,8 @@ Chat::Engine.routes.draw do put "/channels/:channel_id/threads/:thread_id" => "channel_threads#update" get "/channels/:channel_id/threads/:thread_id" => "channel_threads#show" put "/channels/:channel_id/threads/:thread_id/read" => "thread_reads#update" + put "/channels/:channel_id/threads/:thread_id/notifications-settings/me" => + "channel_threads_current_user_notifications_settings#update" put "/channels/:channel_id/messages/:message_id/restore" => "channel_messages#restore" delete "/channels/:channel_id/messages/:message_id" => "channel_messages#destroy" diff --git a/plugins/chat/lib/chat/message_creator.rb b/plugins/chat/lib/chat/message_creator.rb index 2ea493f8204..bb1b025bce3 100644 --- a/plugins/chat/lib/chat/message_creator.rb +++ b/plugins/chat/lib/chat/message_creator.rb @@ -231,14 +231,12 @@ module Chat def post_process_resolved_thread return if resolved_thread.blank? + resolved_thread.increment_replies_count_cache - Chat::UserChatThreadMembership.find_or_create_by!(user: @user, thread: resolved_thread) + resolved_thread.add(@user) if resolved_thread.original_message_user != @user - Chat::UserChatThreadMembership.find_or_create_by!( - user: resolved_thread.original_message_user, - thread: resolved_thread, - ) + resolved_thread.add(resolved_thread.original_message_user) end end end diff --git a/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb b/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb index 4f01fe2db00..d829547db11 100644 --- a/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb +++ b/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb @@ -46,7 +46,7 @@ RSpec.describe "Chat::Thread replies_count cache accuracy" do # Lose the cache intentionally. Chat::Thread.clear_caches!(thread.id) - message_to_destroy = thread.replies.last + message_to_destroy = thread.last_reply Chat::TrashMessage.call( message_id: message_to_destroy.id, channel_id: thread.channel_id, diff --git a/plugins/chat/spec/plugin_helper.rb b/plugins/chat/spec/plugin_helper.rb index 4c6efb05e1d..b64ecc022a6 100644 --- a/plugins/chat/spec/plugin_helper.rb +++ b/plugins/chat/spec/plugin_helper.rb @@ -53,6 +53,15 @@ module ChatSystemHelpers last_message.thread.update!(thread_attrs) if thread_attrs.any? last_message.thread end + + def thread_excerpt(message) + CGI.escapeHTML( + message.censored_excerpt(rich: true, max_length: ::Chat::Thread::EXCERPT_LENGTH).gsub( + "…", + "…", + ), + ) + end end RSpec.configure do |config| diff --git a/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb b/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb index d3c5dbabdd4..6b4e83e3f16 100644 --- a/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb +++ b/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb @@ -137,6 +137,21 @@ describe Chat::ThreadUnreadsQuery do end end + context "when the notification_level for the thread is normal" do + before do + thread_1 + .user_chat_thread_memberships + .find_by(user: current_user) + .update!(notification_level: :normal) + end + + it "gets a zeroed out count for the thread" do + expect(subject.map(&:to_h)).to include( + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 }, + ) + end + end + context "when the user is not a member of a thread" do before { thread_1.user_chat_thread_memberships.find_by(user: current_user).destroy! } diff --git a/plugins/chat/spec/requests/chat/api/channel_threads_current_user_notification_settings_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channel_threads_current_user_notification_settings_controller_spec.rb new file mode 100644 index 00000000000..710040233c7 --- /dev/null +++ b/plugins/chat/spec/requests/chat/api/channel_threads_current_user_notification_settings_controller_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +RSpec.describe Chat::Api::ChannelThreadsCurrentUserNotificationsSettingsController do + fab!(:current_user) { Fabricate(:user) } + fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:thread) { Fabricate(:chat_thread, channel: channel) } + fab!(:last_reply) { Fabricate(:chat_message, thread: thread, chat_channel: channel) } + + before do + SiteSetting.chat_enabled = true + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] + sign_in(current_user) + end + + describe "#update" do + context "when the user cannot access the channel" do + fab!(:channel) { Fabricate(:private_category_channel) } + fab!(:thread) { Fabricate(:chat_thread, channel: channel) } + + it "raises invalid access" do + put "/chat/api/channels/#{channel.id}/threads/#{thread.id}/notifications-settings/me.json", + params: { + notification_level: Chat::UserChatThreadMembership.notification_levels[:normal], + } + expect(response.status).to eq(403) + end + end + + context "when the channel_id and thread_id params do not match" do + it "raises a not found" do + put "/chat/api/channels/#{Fabricate(:chat_channel).id}/threads/#{thread.id}/notifications-settings/me.json", + params: { + notification_level: Chat::UserChatThreadMembership.notification_levels[:normal], + } + expect(response.status).to eq(404) + end + end + + context "when the notification_level is invalid" do + it "raises invalid parameters" do + put "/chat/api/channels/#{Fabricate(:chat_channel).id}/threads/#{thread.id}/notifications-settings/me.json", + params: { + notification_level: 100, + } + expect(response.status).to eq(400) + end + end + + context "when the user is a member of the thread" do + before { thread.add(current_user) } + + it "updates the notification_level" do + expect do + put "/chat/api/channels/#{channel.id}/threads/#{thread.id}/notifications-settings/me.json", + params: { + notification_level: Chat::UserChatThreadMembership.notification_levels[:normal], + } + end.not_to change { Chat::UserChatThreadMembership.count } + + expect(response.status).to eq(200) + expect(thread.membership_for(current_user).notification_level).to eq("normal") + end + end + + context "when the user is not a member of the thread" do + it "creates a membership for the user" do + expect do + put "/chat/api/channels/#{channel.id}/threads/#{thread.id}/notifications-settings/me.json", + params: { + notification_level: Chat::UserChatThreadMembership.notification_levels[:normal], + } + end.to change { Chat::UserChatThreadMembership.count }.by(1) + + expect(response.status).to eq(200) + expect(response.parsed_body["membership"]).to eq( + "notification_level" => Chat::UserChatThreadMembership.notification_levels[:normal], + "thread_id" => thread.id, + "last_read_message_id" => last_reply.id, + ) + end + end + end +end diff --git a/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb b/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb index e0e9cc82089..a3fdfbdd8b6 100644 --- a/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb +++ b/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb @@ -24,31 +24,25 @@ RSpec.describe Chat::LookupChannelThreads do end context "when enable_experimental_chat_threaded_discussions is enabled" do - before { SiteSetting.enable_experimental_chat_threaded_discussions = true } + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + [thread_1, thread_2, thread_3].each do |t| + t.original_message.update!(created_at: 1.week.ago) + t.add(current_user) + end + end context "when all steps pass" do before do - Fabricate( - :chat_message, - user: current_user, - chat_channel: channel, - thread: thread_1, - created_at: 10.minutes.ago, - ) - Fabricate( - :chat_message, - user: current_user, - chat_channel: channel, - thread: thread_2, - created_at: 1.day.ago, - ) - Fabricate( - :chat_message, - user: current_user, - chat_channel: channel, - thread: thread_3, - created_at: 2.seconds.ago, - ) + msg_1 = + Fabricate(:chat_message, user: current_user, chat_channel: channel, thread: thread_1) + msg_1.update!(created_at: 10.minutes.ago) + msg_2 = + Fabricate(:chat_message, user: current_user, chat_channel: channel, thread: thread_2) + msg_2.update!(created_at: 1.day.ago) + msg_3 = + Fabricate(:chat_message, user: current_user, chat_channel: channel, thread: thread_3) + msg_3.update!(created_at: 2.seconds.ago) end it "sets the service result as successful" do @@ -70,12 +64,18 @@ RSpec.describe Chat::LookupChannelThreads do end it "does not count deleted messages for sort order" do - Chat::Message.find_by(user: current_user, thread: thread_3).trash! + Chat::Message.where(thread: thread_3).each(&:trash!) expect(result.threads.map(&:id)).to eq([thread_1.id, thread_2.id]) end - it "does not return threads from the channel where the user has not sent a message" do - Fabricate(:chat_thread, channel: channel) + it "only returns threads where the user has their thread notification level as tracking or regular" do + new_thread_1 = Fabricate(:chat_thread, channel: channel) + new_thread_2 = Fabricate(:chat_thread, channel: channel) + new_thread_1.add(current_user) + new_thread_1.membership_for(current_user).update!( + notification_level: Chat::UserChatThreadMembership.notification_levels[:muted], + ) + expect(result.threads.map(&:id)).to eq([thread_3.id, thread_1.id, thread_2.id]) end diff --git a/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb b/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb new file mode 100644 index 00000000000..6d93564be85 --- /dev/null +++ b/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +RSpec.describe Chat::UpdateThreadNotificationSettings do + describe Chat::UpdateThreadNotificationSettings::Contract, type: :model do + it { is_expected.to validate_presence_of :channel_id } + it { is_expected.to validate_presence_of :thread_id } + it { is_expected.to validate_presence_of :notification_level } + end + + describe ".call" do + subject(:result) { described_class.call(params) } + + fab!(:current_user) { Fabricate(:user) } + fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:private_channel) { Fabricate(:private_category_channel, group: Fabricate(:group)) } + fab!(:thread) { Fabricate(:chat_thread, channel: channel) } + fab!(:last_reply) { Fabricate(:chat_message, thread: thread, chat_channel: channel) } + + let(:guardian) { Guardian.new(current_user) } + let(:params) do + { + guardian: guardian, + thread_id: thread.id, + channel_id: thread.channel_id, + notification_level: Chat::UserChatThreadMembership.notification_levels[:normal], + } + end + + context "when all steps pass" do + it "sets the service result as successful" do + expect(result).to be_a_success + end + + context "when the user is a member of the thread" do + fab!(:membership) { thread.add(current_user) } + + it "updates the notification_level" do + expect { result }.not_to change { Chat::UserChatThreadMembership.count } + expect(membership.reload.notification_level).to eq("normal") + end + end + + context "when the user is not a member of the thread yet" do + it "creates the membership and sets the last read message id to the last reply" do + expect { result }.to change { Chat::UserChatThreadMembership.count }.by(1) + expect(result.membership.notification_level).to eq("normal") + expect(result.membership.last_read_message_id).to eq(last_reply.id) + end + end + end + + context "when params are not valid" do + before { params.delete(:thread_id) } + + it { is_expected.to fail_a_contract } + end + + context "when notification_level is not valid" do + before { params[:notification_level] = 100 } + + it { is_expected.to fail_a_contract } + end + + context "when thread is not found because the channel ID differs" do + before { params[:thread_id] = Fabricate(:chat_thread).id } + + it { is_expected.to fail_to_find_a_model(:thread) } + end + + context "when thread is not found" do + before { thread.destroy! } + + it { is_expected.to fail_to_find_a_model(:thread) } + end + + context "when user cannot see channel" do + before { thread.update!(channel: private_channel) } + + it { is_expected.to fail_a_policy(:can_view_channel) } + end + + context "when threading is not enabled for the channel" do + before { channel.update!(threading_enabled: false) } + + it { is_expected.to fail_a_policy(:threading_enabled_for_channel) } + end + end +end diff --git a/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb index 692f1485a09..e076a2cb403 100644 --- a/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb +++ b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb @@ -95,7 +95,7 @@ RSpec.describe Chat::UpdateUserThreadLastRead do it "updates the last_read_message_id of the thread" do result - expect(membership.reload.last_read_message_id).to eq(thread.replies.last.id) + expect(membership.reload.last_read_message_id).to eq(thread.last_reply.id) end end end diff --git a/plugins/chat/spec/system/channel_thread_message_echoing_spec.rb b/plugins/chat/spec/system/channel_thread_message_echoing_spec.rb index c8f045add46..66ce5d47e24 100644 --- a/plugins/chat/spec/system/channel_thread_message_echoing_spec.rb +++ b/plugins/chat/spec/system/channel_thread_message_echoing_spec.rb @@ -79,10 +79,10 @@ describe "Channel thread message echoing", type: :system do current_user .user_chat_channel_memberships .find_by(chat_channel: channel) - .update!(last_read_message_id: thread.replies.last.id) + .update!(last_read_message_id: thread.last_reply.id) chat_page.visit_channel(channel) expect(channel_page).not_to have_css( - channel_page.message_by_id_selector(thread.replies.last.id), + channel_page.message_by_id_selector(thread.last_reply.id), ) end diff --git a/plugins/chat/spec/system/message_thread_indicator_spec.rb b/plugins/chat/spec/system/message_thread_indicator_spec.rb index 0e8cf6d9dfc..bcc748bb0b6 100644 --- a/plugins/chat/spec/system/message_thread_indicator_spec.rb +++ b/plugins/chat/spec/system/message_thread_indicator_spec.rb @@ -128,9 +128,12 @@ describe "Thread indicator for chat messages", type: :system do it "shows an excerpt of the last reply in the thread" do chat_page.visit_channel(channel) - expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_excerpt( - thread_1.replies.last, - ) + + excerpt_text = thread_excerpt(thread_1.last_reply) + + expect( + channel_page.message_thread_indicator(thread_1.original_message).excerpt, + ).to have_content(excerpt_text) end it "updates the last reply excerpt and participants when a new message is added to the thread" do @@ -140,8 +143,10 @@ describe "Thread indicator for chat messages", type: :system do chat_page.visit_channel(channel) - expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_excerpt( - original_last_reply, + excerpt_text = thread_excerpt(original_last_reply) + + expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_content( + excerpt_text, ) using_session(:new_user) do |session| @@ -157,9 +162,12 @@ describe "Thread indicator for chat messages", type: :system do expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_participant( new_user, ) - expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_excerpt( - thread_1.replies.where(user: new_user).first, - ) + + excerpt_text = thread_excerpt(thread_1.replies.where(user: new_user).first) + + expect( + channel_page.message_thread_indicator(thread_1.original_message).excerpt, + ).to have_content(excerpt_text) end end end diff --git a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb index a9995b42ec9..5a787a58d6c 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb @@ -20,6 +20,21 @@ module PageObjects @header ||= PageObjects::Components::Chat::ThreadHeader.new(".chat-thread") end + def notifications_button + @notifications_button ||= + PageObjects::Components::SelectKit.new(".thread-notifications-button") + end + + def notification_level=(level) + notifications_button.expand + notifications_button.select_row_by_value( + ::Chat::UserChatThreadMembership.notification_levels[level.to_sym], + ) + notifications_button.has_selected_value?( + ::Chat::UserChatThreadMembership.notification_levels[level.to_sym], + ) + end + def selection_management @selection_management ||= PageObjects::Components::Chat::SelectionManagement.new(".chat-channel") diff --git a/plugins/chat/spec/system/page_objects/chat/chat_thread_list.rb b/plugins/chat/spec/system/page_objects/chat/chat_thread_list.rb deleted file mode 100644 index 14ab2193186..00000000000 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread_list.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -module PageObjects - module Pages - class ChatThreadList < PageObjects::Pages::Base - def item_by_id(id) - find(item_by_id_selector(id)) - end - - def avatar_selector(user) - ".chat-thread-list-item__om-user-avatar .chat-user-avatar .chat-user-avatar-container[data-user-card=\"#{user.username}\"] img" - end - - def last_reply_datetime_selector(last_reply) - ".chat-thread-list-item__last-reply .relative-date[data-time='#{(last_reply.created_at.to_f * 1000).to_i}']" - end - - def has_no_unread_item?(id) - has_no_css?(item_by_id_selector(id) + ".-is-unread") - end - - def has_unread_item?(id, count: nil) - if count.nil? - has_css?(item_by_id_selector(id) + ".-is-unread") - else - has_css?( - item_by_id_selector(id) + ".-is-unread .chat-thread-list-item-unread-indicator__number", - text: count.to_s, - ) - end - end - - def item_by_id_selector(id) - ".chat-thread-list__items .chat-thread-list-item[data-thread-id=\"#{id}\"]" - end - end - end -end diff --git a/plugins/chat/spec/system/page_objects/chat/components/thread_indicator.rb b/plugins/chat/spec/system/page_objects/chat/components/thread_indicator.rb index fa591f8d239..b894a21364b 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/thread_indicator.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/thread_indicator.rb @@ -37,13 +37,8 @@ module PageObjects ) end - def has_excerpt?(message) - excerpt_text = - message.censored_excerpt(rich: true, max_length: ::Chat::Thread::EXCERPT_LENGTH).gsub( - "…", - "…", - ) - find(@context).has_css?("#{SELECTOR}__last-reply-excerpt", text: excerpt_text) + def excerpt + find(@context).find("#{SELECTOR}__last-reply-excerpt") end end end diff --git a/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb b/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb index 270aa789112..8456872a44b 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb +++ b/plugins/chat/spec/system/page_objects/chat/components/thread_list.rb @@ -14,6 +14,46 @@ module PageObjects component.has_css?(".spinner", wait: 0) component.has_no_css?(".spinner") end + + def has_thread?(thread) + item_by_id(thread.id) + end + + def item_by_id(id) + component.find(item_by_id_selector(id)) + end + + def avatar_selector(user) + ".chat-thread-list-item__om-user-avatar .chat-user-avatar .chat-user-avatar-container[data-user-card=\"#{user.username}\"] img" + end + + def last_reply_datetime_selector(last_reply) + ".chat-thread-list-item__last-reply .relative-date[data-time='#{(last_reply.created_at.to_f * 1000).to_i}']" + end + + def has_no_unread_item?(id) + component.has_no_css?(item_by_id_selector(id) + ".-is-unread") + end + + def has_unread_item?(id, count: nil) + if count.nil? + component.has_css?(item_by_id_selector(id) + ".-is-unread") + else + component.has_css?( + item_by_id_selector(id) + + ".-is-unread .chat-thread-list-item-unread-indicator__number", + text: count.to_s, + ) + end + end + + def has_no_unread_item?(id) + component.has_no_css?(item_by_id_selector(id) + ".-is-unread") + end + + def item_by_id_selector(id) + ".chat-thread-list__items .chat-thread-list-item[data-thread-id=\"#{id}\"]" + end end end end diff --git a/plugins/chat/spec/system/single_thread_spec.rb b/plugins/chat/spec/system/single_thread_spec.rb index cdc71c1bea8..b18344738d0 100644 --- a/plugins/chat/spec/system/single_thread_spec.rb +++ b/plugins/chat/spec/system/single_thread_spec.rb @@ -112,7 +112,7 @@ describe "Single thread in side panel", type: :system do expect(side_panel).to have_open_thread(thread) thread_page.send_message("new thread message") expect(thread_page).to have_message(thread_id: thread.id, text: "new thread message") - thread_message = thread.replies.last + thread_message = thread.last_reply expect(thread_message.chat_channel_id).to eq(channel.id) expect(thread_message.thread.channel_id).to eq(channel.id) end diff --git a/plugins/chat/spec/system/thread_list/drawer_spec.rb b/plugins/chat/spec/system/thread_list/drawer_spec.rb index 2b83488312a..c3b44920506 100644 --- a/plugins/chat/spec/system/thread_list/drawer_spec.rb +++ b/plugins/chat/spec/system/thread_list/drawer_spec.rb @@ -9,7 +9,7 @@ describe "Thread list in side panel | drawer", type: :system do let(:channel_page) { PageObjects::Pages::ChatChannel.new } let(:side_panel) { PageObjects::Pages::ChatSidePanel.new } let(:thread_page) { PageObjects::Pages::ChatThread.new } - let(:thread_list_page) { PageObjects::Pages::ChatThreadList.new } + let(:thread_list_page) { PageObjects::Components::Chat::ThreadList.new } let(:drawer_page) { PageObjects::Pages::ChatDrawer.new } before do diff --git a/plugins/chat/spec/system/thread_list/full_page_spec.rb b/plugins/chat/spec/system/thread_list/full_page_spec.rb index 200c806d45c..3118fcb0a7b 100644 --- a/plugins/chat/spec/system/thread_list/full_page_spec.rb +++ b/plugins/chat/spec/system/thread_list/full_page_spec.rb @@ -9,7 +9,7 @@ describe "Thread list in side panel | full page", type: :system do let(:channel_page) { PageObjects::Pages::ChatChannel.new } let(:side_panel) { PageObjects::Pages::ChatSidePanel.new } let(:thread_page) { PageObjects::Pages::ChatThread.new } - let(:thread_list_page) { PageObjects::Pages::ChatThreadList.new } + let(:thread_list_page) { PageObjects::Components::Chat::ThreadList.new } before do SiteSetting.enable_experimental_chat_threaded_discussions = true diff --git a/plugins/chat/spec/system/thread_tracking/drawer_spec.rb b/plugins/chat/spec/system/thread_tracking/drawer_spec.rb index 33b49c1f8c2..196f81f47e5 100644 --- a/plugins/chat/spec/system/thread_tracking/drawer_spec.rb +++ b/plugins/chat/spec/system/thread_tracking/drawer_spec.rb @@ -9,7 +9,7 @@ describe "Thread tracking state | drawer", type: :system do let(:chat_page) { PageObjects::Pages::Chat.new } let(:channel_page) { PageObjects::Pages::ChatChannel.new } let(:thread_page) { PageObjects::Pages::ChatThread.new } - let(:thread_list_page) { PageObjects::Pages::ChatThreadList.new } + let(:thread_list_page) { PageObjects::Components::Chat::ThreadList.new } let(:drawer_page) { PageObjects::Pages::ChatDrawer.new } before do diff --git a/plugins/chat/spec/system/thread_tracking/full_page_spec.rb b/plugins/chat/spec/system/thread_tracking/full_page_spec.rb index 66fa3d1825c..c74240417e5 100644 --- a/plugins/chat/spec/system/thread_tracking/full_page_spec.rb +++ b/plugins/chat/spec/system/thread_tracking/full_page_spec.rb @@ -9,7 +9,7 @@ describe "Thread tracking state | full page", type: :system do let(:chat_page) { PageObjects::Pages::Chat.new } let(:channel_page) { PageObjects::Pages::ChatChannel.new } let(:thread_page) { PageObjects::Pages::ChatThread.new } - let(:thread_list_page) { PageObjects::Pages::ChatThreadList.new } + let(:thread_list_page) { PageObjects::Components::Chat::ThreadList.new } before do SiteSetting.enable_experimental_chat_threaded_discussions = true @@ -62,5 +62,37 @@ describe "Thread tracking state | full page", type: :system do channel_page.open_thread_list expect(thread_list_page).to have_no_unread_item(thread.id) end + + it "allows the user to change their tracking level for an existing thread" do + chat_page.visit_thread(thread) + thread_page.notification_level = :normal + expect(thread.reload.membership_for(current_user).notification_level).to eq("normal") + end + + it "allows the user to start tracking a thread they have not replied to" do + new_thread = Fabricate(:chat_thread, channel: channel) + Fabricate(:chat_message, chat_channel: channel, thread: new_thread) + chat_page.visit_thread(new_thread) + thread_page.notification_level = :tracking + expect(new_thread.reload.membership_for(current_user).notification_level).to eq("tracking") + chat_page.visit_channel(channel) + channel_page.open_thread_list + expect(thread_list_page).to have_thread(new_thread) + end + + context "when the user's notification level for the thread is set to normal" do + before { thread.membership_for(current_user).update!(notification_level: :normal) } + + it "does not show a the count of threads with unread messages on the thread list button" do + chat_page.visit_channel(channel) + expect(channel_page).to have_no_unread_thread_indicator + end + + it "does not show an indicator on the unread thread in the list" do + chat_page.visit_channel(channel) + channel_page.open_thread_list + expect(thread_list_page).to have_no_unread_item(thread.id) + end + end end end