From bbb8595107efce1653fde5d9468df077d529b6ca Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 18 Mar 2024 08:35:07 +0100 Subject: [PATCH] PERF: defer loading channels (#26155) Prior to this change we would pre-load all the user channels which making initial page load slower. This change will make them be loaded right after initial load. In the past this was not possible as the channels would have to be loaded on each page transition. However since about a year, we made the channels to be cached on the frontend and no other request will be needed. I have decided for now to not show a loading state in the sidebar as I think it would be noise, but we can reconsider this later. Note given we don't have the channels loaded at first certain things where harder to accomplish. The biggest UX change of this commit is that we removed all the complex logic of computing the best channel to display when you load /chat. We will now store the id of the last channel you visited and will use this id to decide which channel to show. --- .../api/current_user_channels_controller.rb | 13 +- .../services/chat/list_channel_messages.rb | 8 + .../app/services/chat/list_user_channels.rb | 43 +++ .../components/channels-list-public.gjs | 18 +- .../discourse/components/chat-footer.gjs | 11 +- .../discourse/initializers/chat-setup.js | 8 +- .../discourse/initializers/chat-sidebar.js | 115 +++--- .../discourse/routes/chat-index.js | 15 +- .../services/chat-channels-manager.js | 8 + .../discourse/services/chat-state-manager.js | 1 + .../javascripts/discourse/services/chat.js | 88 ++--- plugins/chat/lib/chat/engine.rb | 1 + plugins/chat/plugin.rb | 26 +- plugins/chat/spec/plugin_spec.rb | 69 ---- .../chat/api/current_user_channels_spec.rb | 87 ----- .../core_ext/latest_performance_spec.rb | 58 --- .../chat/list_channel_messages_spec.rb | 8 + .../services/chat/list_user_channels_spec.rb | 76 ++++ plugins/chat/spec/system/chat_channel_spec.rb | 34 +- .../chat/spec/system/chat_new_message_spec.rb | 3 +- plugins/chat/spec/system/drawer_spec.rb | 2 + .../chat/spec/system/mention_warnings_spec.rb | 29 +- plugins/chat/spec/system/navigation_spec.rb | 11 +- .../chat/spec/system/removing_channel_spec.rb | 5 +- .../acceptance/hashtag-css-generator-test.js | 86 ++--- .../javascripts/acceptance/mentions-test.js | 107 ------ .../user-status-on-mentions-test.js | 332 ------------------ .../components/chat-channel-test.js | 6 + 28 files changed, 391 insertions(+), 877 deletions(-) create mode 100644 plugins/chat/app/services/chat/list_user_channels.rb delete mode 100644 plugins/chat/spec/requests/core_ext/latest_performance_spec.rb create mode 100644 plugins/chat/spec/services/chat/list_user_channels_spec.rb delete mode 100644 plugins/chat/test/javascripts/acceptance/mentions-test.js delete mode 100644 plugins/chat/test/javascripts/acceptance/user-status-on-mentions-test.js diff --git a/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb b/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb index 613af229090..02660ef5cc5 100644 --- a/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb +++ b/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb @@ -2,7 +2,16 @@ class Chat::Api::CurrentUserChannelsController < Chat::ApiController def index - structured = Chat::ChannelFetcher.structured(guardian) - render_serialized(structured, Chat::ChannelIndexSerializer, root: false) + with_service(Chat::ListUserChannels) do + on_success do + render_serialized( + result.structured, + Chat::ChannelIndexSerializer, + root: false, + post_allowed_category_ids: result.post_allowed_category_ids, + ) + end + on_failure { render(json: failed_json, status: 422) } + end end end diff --git a/plugins/chat/app/services/chat/list_channel_messages.rb b/plugins/chat/app/services/chat/list_channel_messages.rb index 0c9ed6a1594..36daf51e67c 100644 --- a/plugins/chat/app/services/chat/list_channel_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_messages.rb @@ -29,6 +29,7 @@ module Chat step :fetch_thread_participants step :fetch_thread_memberships step :update_membership_last_viewed_at + step :update_user_last_channel class Contract attribute :channel_id, :integer @@ -170,5 +171,12 @@ module Chat context.membership&.update!(last_viewed_at: Time.zone.now) end end + + def update_user_last_channel(guardian:, channel:) + Scheduler::Defer.later "Chat::ListChannelMessages - defer update_user_last_channel" do + return if guardian.user.custom_fields[::Chat::LAST_CHAT_CHANNEL_ID] == channel.id + guardian.user.upsert_custom_fields(::Chat::LAST_CHAT_CHANNEL_ID => channel.id) + end + end end end diff --git a/plugins/chat/app/services/chat/list_user_channels.rb b/plugins/chat/app/services/chat/list_user_channels.rb new file mode 100644 index 00000000000..65a4eae7996 --- /dev/null +++ b/plugins/chat/app/services/chat/list_user_channels.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Chat + # List of the channels a user is tracking + # + # @example + # Chat::ListUserChannels.call(guardian: guardian, **optional_params) + # + class ListUserChannels + include Service::Base + + # @!method call(guardian:) + # @param [Guardian] guardian + # @return [Service::Base::Context] + + model :structured + step :inject_unread_thread_overview + model :post_allowed_category_ids, optional: true + + private + + def fetch_structured(guardian:) + ::Chat::ChannelFetcher.structured(guardian) + end + + def inject_unread_thread_overview(structured:, guardian:) + structured[:unread_thread_overview] = ::Chat::TrackingStateReportQuery.call( + guardian: guardian, + channel_ids: structured[:public_channels].map(&:id), + include_threads: true, + include_read: false, + include_last_reply_details: true, + ).thread_unread_overview_by_channel + end + + def fetch_post_allowed_category_ids(guardian:, structured:) + ::Category + .post_create_allowed(guardian) + .where(id: structured[:public_channels].map { |c| c.chatable_id }) + .pluck(:id) + end + end +end diff --git a/plugins/chat/assets/javascripts/discourse/components/channels-list-public.gjs b/plugins/chat/assets/javascripts/discourse/components/channels-list-public.gjs index f89c2541848..842804615d3 100644 --- a/plugins/chat/assets/javascripts/discourse/components/channels-list-public.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/channels-list-public.gjs @@ -13,6 +13,7 @@ import ChatChannelRow from "./chat-channel-row"; export default class ChannelsListPublic extends Component { @service chatChannelsManager; + @service chatStateManager; @service chatTrackingStateManager; @service site; @service siteSettings; @@ -23,7 +24,10 @@ export default class ChannelsListPublic extends Component { } get publicMessageChannelsEmpty() { - return this.chatChannelsManager.publicMessageChannels?.length === 0; + return ( + this.chatChannelsManager.publicMessageChannels?.length === 0 && + this.chatStateManager.hasPreloadedChannels + ); } get displayPublicChannels() { @@ -31,6 +35,10 @@ export default class ChannelsListPublic extends Component { return false; } + if (!this.chatStateManager.hasPreloadedChannels) { + return false; + } + if (this.publicMessageChannelsEmpty) { return ( this.currentUser?.staff || @@ -45,10 +53,8 @@ export default class ChannelsListPublic extends Component { return this.chatTrackingStateManager.hasUnreadThreads; } - get isThreadEnabledInAnyChannel() { - return this.currentUser?.chat_channels?.public_channels?.some( - (channel) => channel.threading_enabled - ); + get hasThreadedChannels() { + return this.chatChannelsManager.hasThreadedChannels; } @action @@ -57,7 +63,7 @@ export default class ChannelsListPublic extends Component { }