From a2eb2b0490cb7948fa968bf7d297879ab16e35ec Mon Sep 17 00:00:00 2001 From: Jan Cernik <66427541+jancernik@users.noreply.github.com> Date: Wed, 26 Jul 2023 07:46:23 -0300 Subject: [PATCH] DEV: Remove experimental site setting for chat threads (#22720) We are removing the experimental site setting. Admins can now decide on a per channel basis to enable/disable threading. It's disabled by default. --- .../chat/mark_all_channel_threads_read.rb | 1 - .../regular/chat/update_thread_reply_count.rb | 2 - plugins/chat/app/models/chat/channel.rb | 4 +- plugins/chat/app/models/chat/thread.rb | 1 - .../serializers/chat/channel_serializer.rb | 4 - .../serializers/chat/message_serializer.rb | 2 +- .../chat/structured_channel_serializer.rb | 4 - .../app/serializers/chat/view_serializer.rb | 2 +- .../app/services/chat/channel_view_builder.rb | 3 +- .../services/chat/lookup_channel_threads.rb | 5 - .../chat/app/services/chat/lookup_thread.rb | 10 +- plugins/chat/app/services/chat/publisher.rb | 5 +- .../chat/app/services/chat/tracking_state.rb | 6 - .../chat/app/services/chat/update_thread.rb | 10 +- .../components/chat-channel-settings-view.hbs | 44 ++-- .../components/chat-channel-settings-view.js | 8 - .../components/chat/modal/create-channel.hbs | 36 ++- .../components/chat/modal/create-channel.js | 7 - .../services/chat-channel-composer.js | 6 +- plugins/chat/config/locales/server.de.yml | 1 - plugins/chat/config/locales/server.en.yml | 1 - plugins/chat/config/locales/server.he.yml | 1 - plugins/chat/config/locales/server.it.yml | 1 - plugins/chat/config/locales/server.pl_PL.yml | 1 - plugins/chat/config/locales/server.sv.yml | 1 - plugins/chat/config/settings.yml | 3 - plugins/chat/lib/chat/channel_fetcher.rb | 3 +- plugins/chat/lib/chat/message_mover.rb | 3 +- plugins/chat/lib/discourse_dev/thread.rb | 4 - plugins/chat/plugin.rb | 16 +- ...hread_replies_count_cache_accuracy_spec.rb | 5 +- .../regular/chat/notify_mentioned_spec.rb | 5 +- .../mark_all_channel_threads_read_spec.rb | 59 ++--- .../regular/update_thread_reply_count_spec.rb | 11 +- plugins/chat/spec/models/chat/thread_spec.rb | 11 +- .../queries/chat/thread_unreads_query_spec.rb | 1 - .../api/channel_threads_controller_spec.rb | 28 --- .../chat/api/channels_controller_spec.rb | 2 - .../chat/chat_message_serializer_spec.rb | 21 +- .../chat/channel_view_builder_spec.rb | 12 +- .../chat/lookup_channel_threads_spec.rb | 10 - .../spec/services/chat/lookup_thread_spec.rb | 62 +++-- .../chat/spec/services/chat/publisher_spec.rb | 70 +----- .../spec/services/chat/tracking_state_spec.rb | 215 ++++++++---------- .../spec/services/chat/update_thread_spec.rb | 126 +++++----- .../spec/system/channel_settings_page_spec.rb | 1 - .../channel_thread_message_echoing_spec.rb | 25 +- .../spec/system/chat/composer/channel_spec.rb | 5 +- .../chat/composer/shortcuts/channel_spec.rb | 5 +- .../chat/composer/shortcuts/thread_spec.rb | 1 - .../spec/system/chat/composer/thread_spec.rb | 1 - .../spec/system/chat_message/channel_spec.rb | 5 +- .../spec/system/chat_message/thread_spec.rb | 1 - .../chat/spec/system/create_channel_spec.rb | 2 - .../chat/spec/system/deleted_message_spec.rb | 1 - .../system/message_thread_indicator_spec.rb | 23 +- plugins/chat/spec/system/navigation_spec.rb | 1 - .../system/reply_to_message/drawer_spec.rb | 1 - .../system/reply_to_message/full_page_spec.rb | 1 - .../system/reply_to_message/mobile_spec.rb | 1 - .../system/reply_to_message/smoke_spec.rb | 1 - .../spec/system/select_message/thread_spec.rb | 6 +- .../chat/spec/system/single_thread_spec.rb | 22 +- .../spec/system/thread_list/drawer_spec.rb | 1 - .../spec/system/thread_list/full_page_spec.rb | 1 - .../system/thread_tracking/drawer_spec.rb | 1 - .../system/thread_tracking/full_page_spec.rb | 1 - 67 files changed, 287 insertions(+), 653 deletions(-) diff --git a/plugins/chat/app/jobs/regular/chat/mark_all_channel_threads_read.rb b/plugins/chat/app/jobs/regular/chat/mark_all_channel_threads_read.rb index a257e865b02..bc190b25c89 100644 --- a/plugins/chat/app/jobs/regular/chat/mark_all_channel_threads_read.rb +++ b/plugins/chat/app/jobs/regular/chat/mark_all_channel_threads_read.rb @@ -6,7 +6,6 @@ module Jobs sidekiq_options queue: "critical" def execute(args = {}) - return if !SiteSetting.enable_experimental_chat_threaded_discussions channel = ::Chat::Channel.find_by(id: args[:channel_id]) return if channel.blank? channel.mark_all_threads_as_read diff --git a/plugins/chat/app/jobs/regular/chat/update_thread_reply_count.rb b/plugins/chat/app/jobs/regular/chat/update_thread_reply_count.rb index 4173c6ecc10..5e15cb35808 100644 --- a/plugins/chat/app/jobs/regular/chat/update_thread_reply_count.rb +++ b/plugins/chat/app/jobs/regular/chat/update_thread_reply_count.rb @@ -4,8 +4,6 @@ module Jobs module Chat class UpdateThreadReplyCount < Jobs::Base def execute(args = {}) - return if !SiteSetting.enable_experimental_chat_threaded_discussions - thread = ::Chat::Thread.find_by(id: args[:thread_id]) return if thread.blank? return if thread.replies_count_cache_recently_updated? diff --git a/plugins/chat/app/models/chat/channel.rb b/plugins/chat/app/models/chat/channel.rb index 977e82bd6c4..8c108a05534 100644 --- a/plugins/chat/app/models/chat/channel.rb +++ b/plugins/chat/app/models/chat/channel.rb @@ -188,9 +188,7 @@ module Chat end def mark_all_threads_as_read(user: nil) - if !(self.threading_enabled || SiteSetting.enable_experimental_chat_threaded_discussions) - return - end + return if !self.threading_enabled DB.exec(<<~SQL, channel_id: self.id) UPDATE user_chat_thread_memberships diff --git a/plugins/chat/app/models/chat/thread.rb b/plugins/chat/app/models/chat/thread.rb index 236bc72dba4..c1a7c8bc2c6 100644 --- a/plugins/chat/app/models/chat/thread.rb +++ b/plugins/chat/app/models/chat/thread.rb @@ -108,7 +108,6 @@ module Chat end def self.ensure_consistency! - return if !SiteSetting.enable_experimental_chat_threaded_discussions update_counts end diff --git a/plugins/chat/app/serializers/chat/channel_serializer.rb b/plugins/chat/app/serializers/chat/channel_serializer.rb index 6664410ec85..fe154125dff 100644 --- a/plugins/chat/app/serializers/chat/channel_serializer.rb +++ b/plugins/chat/app/serializers/chat/channel_serializer.rb @@ -25,10 +25,6 @@ module Chat has_one :last_message, serializer: Chat::LastMessageSerializer, embed: :objects - def threading_enabled - SiteSetting.enable_experimental_chat_threaded_discussions && object.threading_enabled - end - def initialize(object, opts) super(object, opts) diff --git a/plugins/chat/app/serializers/chat/message_serializer.rb b/plugins/chat/app/serializers/chat/message_serializer.rb index 05df1d5baa3..ffe59d2a467 100644 --- a/plugins/chat/app/serializers/chat/message_serializer.rb +++ b/plugins/chat/app/serializers/chat/message_serializer.rb @@ -177,7 +177,7 @@ module Chat end def include_threading_data? - SiteSetting.enable_experimental_chat_threaded_discussions && channel.threading_enabled + channel.threading_enabled end def include_thread_id? diff --git a/plugins/chat/app/serializers/chat/structured_channel_serializer.rb b/plugins/chat/app/serializers/chat/structured_channel_serializer.rb index 10b31b4a0e9..5911191230c 100644 --- a/plugins/chat/app/serializers/chat/structured_channel_serializer.rb +++ b/plugins/chat/app/serializers/chat/structured_channel_serializer.rb @@ -8,10 +8,6 @@ module Chat object[:tracking] end - def include_unread_thread_overview? - SiteSetting.enable_experimental_chat_threaded_discussions - end - def unread_thread_overview object[:unread_thread_overview] end diff --git a/plugins/chat/app/serializers/chat/view_serializer.rb b/plugins/chat/app/serializers/chat/view_serializer.rb index f1399bf522a..027387b3962 100644 --- a/plugins/chat/app/serializers/chat/view_serializer.rb +++ b/plugins/chat/app/serializers/chat/view_serializer.rb @@ -36,7 +36,7 @@ module Chat end def include_thread_data? - channel.threading_enabled && SiteSetting.enable_experimental_chat_threaded_discussions + channel.threading_enabled end def channel diff --git a/plugins/chat/app/services/chat/channel_view_builder.rb b/plugins/chat/app/services/chat/channel_view_builder.rb index 4cc54252cfb..aa3c104524a 100644 --- a/plugins/chat/app/services/chat/channel_view_builder.rb +++ b/plugins/chat/app/services/chat/channel_view_builder.rb @@ -112,8 +112,7 @@ module Chat end def determine_threads_enabled(channel:, **) - context.threads_enabled = - SiteSetting.enable_experimental_chat_threaded_discussions && channel.threading_enabled + context.threads_enabled = channel.threading_enabled end def determine_include_thread_messages(contract:, threads_enabled:, **) diff --git a/plugins/chat/app/services/chat/lookup_channel_threads.rb b/plugins/chat/app/services/chat/lookup_channel_threads.rb index 307df0cec5d..60255561906 100644 --- a/plugins/chat/app/services/chat/lookup_channel_threads.rb +++ b/plugins/chat/app/services/chat/lookup_channel_threads.rb @@ -24,7 +24,6 @@ module Chat # @param [Integer] offset # @return [Service::Base::Context] - policy :threaded_discussions_enabled contract step :set_limit step :set_offset @@ -55,10 +54,6 @@ module Chat context.offset = [contract.offset || 0, 0].max end - def threaded_discussions_enabled - ::SiteSetting.enable_experimental_chat_threaded_discussions - end - def fetch_channel(contract:, **) ::Chat::Channel.strict_loading.includes(:chatable).find_by(id: contract.channel_id) end diff --git a/plugins/chat/app/services/chat/lookup_thread.rb b/plugins/chat/app/services/chat/lookup_thread.rb index ba6c3f66471..789665ae588 100644 --- a/plugins/chat/app/services/chat/lookup_thread.rb +++ b/plugins/chat/app/services/chat/lookup_thread.rb @@ -2,10 +2,7 @@ module Chat # Finds a thread within a channel. The thread_id and channel_id must - # match. For now we do not want to allow fetching threads if the - # enable_experimental_chat_threaded_discussions hidden site setting - # is not turned on, and the channel must specifically have threading - # enabled. + # match, and the channel must specifically have threading enabled. # # @example # Chat::LookupThread.call(thread_id: 88, channel_id: 2, guardian: guardian) @@ -19,7 +16,6 @@ module Chat # @param [Guardian] guardian # @return [Service::Base::Context] - policy :threaded_discussions_enabled contract model :thread, :fetch_thread policy :invalid_access @@ -37,10 +33,6 @@ module Chat private - def threaded_discussions_enabled - SiteSetting.enable_experimental_chat_threaded_discussions - end - def fetch_thread(contract:, **) Chat::Thread.includes( :channel, diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index 1f9e22502c4..5d5a39bdadd 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -32,7 +32,7 @@ module Chat end def self.allow_publish_to_thread?(channel) - SiteSetting.enable_experimental_chat_threaded_discussions && channel.threading_enabled + channel.threading_enabled end def self.publish_new!(chat_channel, chat_message, staged_id, staged_thread_id: nil) @@ -161,8 +161,7 @@ module Chat def self.publish_delete!(chat_channel, chat_message) message_bus_targets = calculate_publish_targets(chat_channel, chat_message) latest_not_deleted_message_id = - if chat_message.thread_reply? && chat_channel.threading_enabled && - SiteSetting.enable_experimental_chat_threaded_discussions + if chat_message.thread_reply? && chat_channel.threading_enabled chat_message.thread.latest_not_deleted_message_id(anchor_message_id: chat_message.id) else chat_channel.latest_not_deleted_message_id(anchor_message_id: chat_message.id) diff --git a/plugins/chat/app/services/chat/tracking_state.rb b/plugins/chat/app/services/chat/tracking_state.rb index 74c6b08d5d5..17cec53be51 100644 --- a/plugins/chat/app/services/chat/tracking_state.rb +++ b/plugins/chat/app/services/chat/tracking_state.rb @@ -34,7 +34,6 @@ module Chat # @return [Service::Base::Context] contract - policy :threaded_discussions_settings_ok step :cast_thread_and_channel_ids_to_integer model :report @@ -49,11 +48,6 @@ module Chat private - def threaded_discussions_settings_ok(contract:, **) - return true if !contract.include_threads - SiteSetting.enable_experimental_chat_threaded_discussions - end - def cast_thread_and_channel_ids_to_integer(contract:, **) contract.thread_ids = contract.thread_ids.map(&:to_i) contract.channel_ids = contract.channel_ids.map(&:to_i) diff --git a/plugins/chat/app/services/chat/update_thread.rb b/plugins/chat/app/services/chat/update_thread.rb index 4b7e7327a66..b4ccad8673e 100644 --- a/plugins/chat/app/services/chat/update_thread.rb +++ b/plugins/chat/app/services/chat/update_thread.rb @@ -2,10 +2,7 @@ module Chat # Updates a thread. The thread_id and channel_id must - # match. For now we do not want to allow updating threads if the - # enable_experimental_chat_threaded_discussions hidden site setting - # is not turned on, and the channel must specifically have threading - # enabled. + # match, and the channel must specifically have threading enabled. # # Only the thread title can be updated. # @@ -22,7 +19,6 @@ module Chat # @option params_to_edit [String,nil] title # @return [Service::Base::Context] - policy :threaded_discussions_enabled contract model :thread, :fetch_thread policy :can_view_channel @@ -43,10 +39,6 @@ module Chat private - def threaded_discussions_enabled - SiteSetting.enable_experimental_chat_threaded_discussions - end - def fetch_thread(contract:, **) Chat::Thread.find_by(id: contract.thread_id, channel_id: contract.channel_id) end diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-settings-view.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-channel-settings-view.hbs index 78ca5ba2726..e295144af33 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-settings-view.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-settings-view.hbs @@ -127,31 +127,29 @@ {{/if}} - {{#if this.togglingThreadingAvailable}} -
-
- - +
+
+ +
- {{/if}} +
{{/if}} {{#unless @channel.isDirectMessageChannel}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-settings-view.js b/plugins/chat/assets/javascripts/discourse/components/chat-channel-settings-view.js index dc5560886ef..a5c980d5ed1 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-settings-view.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-settings-view.js @@ -59,14 +59,6 @@ export default class ChatChannelSettingsView extends Component { return this.args.channel.isCategoryChannel; } - get togglingThreadingAvailable() { - return ( - this.siteSettings.enable_experimental_chat_threaded_discussions && - this.args.channel.isCategoryChannel && - this.currentUser?.admin - ); - } - get autoJoinAvailable() { return ( this.siteSettings.max_chat_auto_joined_users > 0 && diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/modal/create-channel.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/modal/create-channel.hbs index 06e8213a245..cd30124048e 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/modal/create-channel.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/modal/create-channel.hbs @@ -104,25 +104,23 @@ {{/if}} - {{#if this.threadingAvailable}} -
- -
- {{/if}} +
+ +
<:footer>