Skip to content

Commit

Permalink
PERF: defer loading channels (#26155)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jjaffeux committed Mar 18, 2024
1 parent 0bccdc4 commit bbb8595
Show file tree
Hide file tree
Showing 28 changed files with 391 additions and 877 deletions.
Expand Up @@ -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
8 changes: 8 additions & 0 deletions plugins/chat/app/services/chat/list_channel_messages.rb
Expand Up @@ -29,6 +29,7 @@ class ListChannelMessages
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
Expand Down Expand Up @@ -170,5 +171,12 @@ def update_membership_last_viewed_at(guardian:)
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
43 changes: 43 additions & 0 deletions 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
Expand Up @@ -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;
Expand All @@ -23,14 +24,21 @@ 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() {
if (!this.siteSettings.enable_public_channels) {
return false;
}

if (!this.chatStateManager.hasPreloadedChannels) {
return false;
}

if (this.publicMessageChannelsEmpty) {
return (
this.currentUser?.staff ||
Expand All @@ -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
Expand All @@ -57,7 +63,7 @@ export default class ChannelsListPublic extends Component {
}

<template>
{{#if (and this.site.desktopView this.isThreadEnabledInAnyChannel)}}
{{#if (and this.site.desktopView this.hasThreadedChannels)}}
<LinkTo @route="chat.threads" class="chat-channel-row --threads">
<span class="chat-channel-title">
{{dIcon "discourse-threads" class="chat-user-threads__icon"}}
Expand Down
Expand Up @@ -15,22 +15,25 @@ export default class ChatFooter extends Component {
@service chat;
@service siteSettings;
@service currentUser;
@service chatChannelsManager;
@service chatStateManager;

get includeThreads() {
if (!this.siteSettings.chat_threads_enabled) {
return false;
}
return this.currentUser?.chat_channels?.public_channels?.some(
(channel) => channel.threading_enabled
);
return this.chatChannelsManager.hasThreadedChannels;
}

get directMessagesEnabled() {
return this.chat.userCanAccessDirectMessages;
}

get shouldRenderFooter() {
return this.includeThreads || this.directMessagesEnabled;
return (
this.chatStateManager.hasPreloadedChannels &&
(this.includeThreads || this.directMessagesEnabled)
);
}

<template>
Expand Down
Expand Up @@ -148,13 +148,7 @@ export default {

document.body.classList.add("chat-enabled");

const currentUser = api.getCurrentUser();

// NOTE: chat_channels is more than a simple array, it also contains
// tracking and membership data, see Chat::StructuredChannelSerializer
if (currentUser?.chat_channels) {
this.chatService.setupWithPreloadedChannels(currentUser.chat_channels);
}
this.chatService.loadChannels();

const chatNotificationManager = container.lookup(
"service:chat-notification-manager"
Expand Down
115 changes: 60 additions & 55 deletions plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js
Expand Up @@ -48,69 +48,68 @@ export default {
});

withPluginApi("1.3.0", (api) => {
const isThreadEnabledInAnyChannel =
this.currentUser?.chat_channels?.public_channels?.some(
(channel) => channel.threading_enabled === true
);

if (isThreadEnabledInAnyChannel) {
api.addSidebarSection(
(BaseCustomSidebarSection, BaseCustomSidebarSectionLink) => {
const SidebarChatMyThreadsSectionLink = class extends BaseCustomSidebarSectionLink {
route = "chat.threads";
text = I18n.t("chat.my_threads.title");
title = I18n.t("chat.my_threads.title");
name = "user-threads";
prefixType = "icon";
prefixValue = "discourse-threads";
suffixType = "icon";
suffixCSSClass = "unread";
const chatChannelsManager = container.lookup(
"service:chat-channels-manager"
);

constructor() {
super(...arguments);
api.addSidebarSection(
(BaseCustomSidebarSection, BaseCustomSidebarSectionLink) => {
const SidebarChatMyThreadsSectionLink = class extends BaseCustomSidebarSectionLink {
route = "chat.threads";
text = I18n.t("chat.my_threads.title");
title = I18n.t("chat.my_threads.title");
name = "user-threads";
prefixType = "icon";
prefixValue = "discourse-threads";
suffixType = "icon";
suffixCSSClass = "unread";

if (container.isDestroyed) {
return;
}
constructor() {
super(...arguments);

this.chatChannelsManager = container.lookup(
"service:chat-channels-manager"
);
if (container.isDestroyed) {
return;
}
}

get suffixValue() {
return this.chatChannelsManager.publicMessageChannels.some(
(channel) => channel.unreadThreadsCount > 0
)
? "circle"
: "";
}
};
get suffixValue() {
return chatChannelsManager.publicMessageChannels.some(
(channel) => channel.unreadThreadsCount > 0
)
? "circle"
: "";
}
};

const SidebarChatMyThreadsSection = class extends BaseCustomSidebarSection {
// we only show `My Threads` link
hideSectionHeader = true;
const SidebarChatMyThreadsSection = class extends BaseCustomSidebarSection {
@service chatChannelsManager;

name = "user-threads";
// we only show `My Threads` link
hideSectionHeader = true;

// sidebar API doesn’t let you have undefined values
// even if you don't show the section’s header
title = "";
name = "user-threads";

get links() {
return [new SidebarChatMyThreadsSectionLink()];
}
// sidebar API doesn’t let you have undefined values
// even if you don't show the section’s header
title = "";
get text() {
return null;
}
};
get links() {
return [new SidebarChatMyThreadsSectionLink()];
}
return SidebarChatMyThreadsSection;
},
CHAT_PANEL
);
}
get text() {
return null;
}
get displaySection() {
return this.chatChannelsManager.hasThreadedChannels;
}
};
return SidebarChatMyThreadsSection;
},
CHAT_PANEL
);
if (this.siteSettings.enable_public_channels) {
api.addSidebarSection(
Expand Down Expand Up @@ -199,6 +198,7 @@ export default {
const SidebarChatChannelsSection = class extends BaseCustomSidebarSection {
@service currentUser;
@service chatStateManager;
@tracked
currentUserCanJoinPublicChannels =
Expand Down Expand Up @@ -261,8 +261,9 @@ export default {
get displaySection() {
return (
this.sectionLinks.length > 0 ||
this.currentUserCanJoinPublicChannels
this.chatStateManager.hasPreloadedChannels &&
(this.sectionLinks.length > 0 ||
this.currentUserCanJoinPublicChannels)
);
}
};
Expand Down Expand Up @@ -417,6 +418,7 @@ export default {
@service modal;
@service router;
@service currentUser;
@service chatStateManager;

@tracked
userCanDirectMessage = this.chatService.userCanDirectMessage;
Expand Down Expand Up @@ -482,7 +484,10 @@ export default {
}

get displaySection() {
return this.sectionLinks.length > 0 || this.userCanDirectMessage;
return (
this.chatStateManager.hasPreloadedChannels &&
(this.sectionLinks.length > 0 || this.userCanDirectMessage)
);
}
};

Expand Down
15 changes: 9 additions & 6 deletions plugins/chat/assets/javascripts/discourse/routes/chat-index.js
Expand Up @@ -12,9 +12,8 @@ export default class ChatIndexRoute extends DiscourseRoute {
if (!this.siteSettings.chat_threads_enabled) {
return false;
}
return this.currentUser?.chat_channels?.public_channels?.some(
(channel) => channel.threading_enabled
);

return this.chatChannelsManager.hasThreadedChannels;
}

get hasDirectMessages() {
Expand All @@ -25,7 +24,11 @@ export default class ChatIndexRoute extends DiscourseRoute {
this.chat.activeChannel = null;
}

redirect() {
async model() {
return await this.chat.loadChannels();
}

async redirect() {
// on mobile redirect user to the first footer tab route
if (this.site.mobileView) {
if (
Expand All @@ -43,8 +46,8 @@ export default class ChatIndexRoute extends DiscourseRoute {
}
}

// We are on desktop. Check for a channel to enter and transition if so
const id = this.chat.getIdealFirstChannelId();
// We are on desktop. Check for last visited channel and transition if so
const id = this.currentUser.custom_fields.last_chat_channel_id;
if (id) {
return this.chatChannelsManager.find(id).then((c) => {
return this.router.replaceWith("chat.channel", ...c.routeModels);
Expand Down

1 comment on commit bbb8595

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/logs-job-exception-unexpected-return-error/299963/1

Please sign in to comment.