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.
This commit is contained in:
Joffrey JAFFEUX 2024-03-18 08:35:07 +01:00 committed by GitHub
parent 0bccdc4dbc
commit bbb8595107
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
28 changed files with 391 additions and 877 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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 {
}
<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"}}

View File

@ -15,14 +15,14 @@ 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() {
@ -30,7 +30,10 @@ export default class ChatFooter extends Component {
}
get shouldRenderFooter() {
return this.includeThreads || this.directMessagesEnabled;
return (
this.chatStateManager.hasPreloadedChannels &&
(this.includeThreads || this.directMessagesEnabled)
);
}
<template>

View File

@ -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"

View File

@ -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
);
const chatChannelsManager = container.lookup(
"service:chat-channels-manager"
);
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";
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";
constructor() {
super(...arguments);
constructor() {
super(...arguments);
if (container.isDestroyed) {
return;
}
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 doesnt let you have undefined values
// even if you don't show the sections header
title = "";
name = "user-threads";
get links() {
return [new SidebarChatMyThreadsSectionLink()];
}
// sidebar API doesnt let you have undefined values
// even if you don't show the sections 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(
@ -199,6 +198,7 @@ export default {
const SidebarChatChannelsSection = class extends BaseCustomSidebarSection {
@service currentUser;
@service chatStateManager;
@tracked
currentUserCanJoinPublicChannels =
@ -261,8 +261,9 @@ export default {
get displaySection() {
return (
this.sectionLinks.length > 0 ||
this.currentUserCanJoinPublicChannels
this.chatStateManager.hasPreloadedChannels &&
(this.sectionLinks.length > 0 ||
this.currentUserCanJoinPublicChannels)
);
}
};
@ -417,6 +418,7 @@ export default {
@service modal;
@service router;
@service currentUser;
@service chatStateManager;
@tracked
userCanDirectMessage = this.chatService.userCanDirectMessage;
@ -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)
);
}
};

View File

@ -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() {
@ -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 (
@ -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);

View File

@ -33,6 +33,7 @@ export default class ChatChannelsManager extends Service {
}
}
@cached
get channels() {
return Object.values(this._cached);
}
@ -100,6 +101,13 @@ export default class ChatChannelsManager extends Service {
delete this._cached[model.id];
}
@cached
get hasThreadedChannels() {
return this.publicMessageChannels?.some(
(channel) => channel.threadingEnabled
);
}
get allChannels() {
return [...this.publicMessageChannels, ...this.directMessageChannels].sort(
(a, b) => {

View File

@ -30,6 +30,7 @@ export default class ChatStateManager extends Service {
@tracked isSidePanelExpanded = false;
@tracked isDrawerExpanded = false;
@tracked isDrawerActive = false;
@tracked hasPreloadedChannels = false;
@tracked _chatURL = null;
@tracked _appURL = null;

View File

@ -175,6 +175,32 @@ export default class Chat extends Service {
this.set("isNetworkUnreliable", false);
}
async loadChannels() {
// We want to be able to call this method multiple times, but only
// actually load the channels once. This is because we might call
// this method before the chat is fully initialized, and we don't
// want to load the channels multiple times in that case.
try {
if (this.chatStateManager.hasPreloadedChannels) {
return;
}
if (this.loadingChannels) {
return this.loadingChannels;
}
this.loadingChannels = new Promise((resolve) => {
this.chatApi.listCurrentUserChannels().then((result) => {
this.setupWithPreloadedChannels(result);
this.chatStateManager.hasPreloadedChannels = true;
resolve();
});
});
} catch (e) {
popupAjaxError(e);
}
}
setupWithPreloadedChannels(channelsView) {
this.chatSubscriptionsManager.startChannelsSubscriptions(
channelsView.meta.message_bus_last_ids
@ -294,68 +320,6 @@ export default class Chat extends Service {
}
}
getIdealFirstChannelId() {
// When user opens chat we need to give them the 'best' channel when they enter.
//
// Look for public channels with mentions. If one exists, enter that.
// Next best is a DM channel with unread messages.
// Next best is a public channel with unread messages.
// Then we fall back to the chat_default_channel_id site setting
// if that is present and in the list of channels the user can access.
// If none of these options exist, then we get the first public channel,
// or failing that the first DM channel.
// Defined in order of significance.
let publicChannelWithMention,
dmChannelWithUnread,
publicChannelWithUnread,
publicChannel,
dmChannel,
defaultChannel;
this.chatChannelsManager.channels.forEach((channel) => {
const membership = channel.currentUserMembership;
if (!membership.following) {
return;
}
if (channel.isDirectMessageChannel) {
if (!dmChannelWithUnread && channel.tracking.unreadCount > 0) {
dmChannelWithUnread = channel.id;
} else if (!dmChannel) {
dmChannel = channel.id;
}
} else {
if (membership.unread_mentions > 0) {
publicChannelWithMention = channel.id;
return; // <- We have a public channel with a mention. Break and return this.
} else if (
!publicChannelWithUnread &&
channel.tracking.unreadCount > 0
) {
publicChannelWithUnread = channel.id;
} else if (
!defaultChannel &&
parseInt(this.siteSettings.chat_default_channel_id || 0, 10) ===
channel.id
) {
defaultChannel = channel.id;
} else if (!publicChannel) {
publicChannel = channel.id;
}
}
});
return (
publicChannelWithMention ||
dmChannelWithUnread ||
publicChannelWithUnread ||
defaultChannel ||
publicChannel ||
dmChannel
);
}
_fireOpenFloatAppEvent(channel, messageId = null) {
messageId
? this.router.transitionTo(

View File

@ -2,6 +2,7 @@
module ::Chat
HAS_CHAT_ENABLED = "has_chat_enabled"
LAST_CHAT_CHANNEL_ID = "last_chat_channel_id"
class Engine < ::Rails::Engine
engine_name PLUGIN_NAME

View File

@ -49,6 +49,9 @@ after_initialize do
register_category_custom_field_type(Chat::HAS_CHAT_ENABLED, :boolean)
register_user_custom_field_type(Chat::LAST_CHAT_CHANNEL_ID, :integer)
DiscoursePluginRegistry.serialized_current_user_fields << Chat::LAST_CHAT_CHANNEL_ID
UserUpdater::OPTION_ATTR.push(:chat_enabled)
UserUpdater::OPTION_ATTR.push(:only_chat_push_notifications)
UserUpdater::OPTION_ATTR.push(:chat_sound)
@ -210,29 +213,6 @@ after_initialize do
).exists?
end
add_to_serializer(:current_user, :chat_channels) do
structured = Chat::ChannelFetcher.structured(self.scope)
structured[:unread_thread_overview] = ::Chat::TrackingStateReportQuery.call(
guardian: self.scope,
channel_ids: structured[:public_channels].map(&:id),
include_threads: true,
include_read: false,
include_last_reply_details: true,
).thread_unread_overview_by_channel
category_ids = structured[:public_channels].map { |c| c.chatable_id }
post_allowed_category_ids =
Category.post_create_allowed(self.scope).where(id: category_ids).pluck(:id)
Chat::ChannelIndexSerializer.new(
structured,
scope: self.scope,
root: false,
post_allowed_category_ids: post_allowed_category_ids,
).as_json
end
add_to_serializer(
:current_user,
:chat_drafts,

View File

@ -273,75 +273,6 @@ describe Chat do
end
end
describe "current_user_serializer#chat_channels" do
before do
SiteSetting.chat_enabled = true
SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
end
fab!(:user)
let(:serializer) { CurrentUserSerializer.new(user, scope: Guardian.new(user)) }
it "returns the global presence channel state" do
expect(serializer.chat_channels[:global_presence_channel_state]).to be_present
end
context "when no channels exist" do
it "returns an empty array" do
expect(serializer.chat_channels[:direct_message_channels]).to eq([])
expect(serializer.chat_channels[:public_channels]).to eq([])
end
end
context "when followed direct message channels exist" do
fab!(:user_2) { Fabricate(:user) }
fab!(:channel) { Fabricate(:direct_message_channel, users: [user, user_2]) }
it "returns them" do
expect(serializer.chat_channels[:public_channels]).to eq([])
expect(serializer.chat_channels[:direct_message_channels].count).to eq(1)
expect(serializer.chat_channels[:direct_message_channels][0].id).to eq(channel.id)
end
end
context "when followed public channels exist" do
fab!(:channel) { Fabricate(:chat_channel) }
before do
Fabricate(:user_chat_channel_membership, user: user, chat_channel: channel, following: true)
Fabricate(:chat_channel)
end
it "returns them" do
expect(serializer.chat_channels[:direct_message_channels]).to eq([])
expect(serializer.chat_channels[:public_channels].count).to eq(1)
expect(serializer.chat_channels[:public_channels][0].id).to eq(channel.id)
end
end
context "when the category is restricted and user has readonly persmissions" do
fab!(:channel_1) { Fabricate(:chat_channel) }
fab!(:group_1) { Fabricate(:group) }
fab!(:private_channel_1) { Fabricate(:private_category_channel, group: group_1) }
before do
private_channel_1.chatable.category_groups.first.update!(
permission_type: CategoryGroup.permission_types[:readonly],
)
group_1.add(user)
channel_1.add(user)
private_channel_1.add(user)
end
it "doesnt list the associated channel" do
expect(serializer.chat_channels[:public_channels].map(&:id)).to contain_exactly(
channel_1.id,
)
end
end
end
describe "current_user_serializer#has_joinable_public_channels" do
before do
SiteSetting.chat_enabled = true

View File

@ -43,93 +43,6 @@ describe Chat::Api::CurrentUserChannelsController do
expect(response.parsed_body["public_channels"][0]["id"]).to eq(channel.id)
end
it "returns limited access public channels with memberships" do
group = Fabricate(:group)
channel = Fabricate(:private_category_channel, group: group)
group.add(current_user)
channel.add(current_user)
get "/chat/api/me/channels"
expect(response.parsed_body["public_channels"][0]["id"]).to eq(channel.id)
end
it "doesnt return unaccessible private channels" do
group = Fabricate(:group)
channel = Fabricate(:private_category_channel, group: group)
channel.add(current_user) # TODO: we should error here
get "/chat/api/me/channels"
expect(response.parsed_body["public_channels"]).to be_blank
end
it "returns dm channels you are part of" do
dm_channel = Fabricate(:direct_message_channel, users: [current_user])
get "/chat/api/me/channels"
expect(response.parsed_body["direct_message_channels"][0]["id"]).to eq(dm_channel.id)
end
it "doesnt return dm channels from other users" do
Fabricate(:direct_message_channel)
get "/chat/api/me/channels"
expect(response.parsed_body["direct_message_channels"]).to be_blank
end
it "includes message bus ids" do
Fabricate(:direct_message_channel, users: [current_user])
channel = Fabricate(:category_channel)
channel.add(current_user)
get "/chat/api/me/channels"
expect(response.status).to eq(200)
response.parsed_body["meta"]["message_bus_last_ids"].tap do |ids|
expect(ids["channel_metadata"]).not_to eq(nil)
expect(ids["channel_edits"]).not_to eq(nil)
expect(ids["channel_status"]).not_to eq(nil)
expect(ids["new_channel"]).not_to eq(nil)
expect(ids["archive_status"]).not_to eq(nil)
end
response.parsed_body["public_channels"][0]["meta"]["message_bus_last_ids"].tap do |ids|
expect(ids["new_messages"]).not_to eq(nil)
expect(ids["new_mentions"]).not_to eq(nil)
end
response.parsed_body["direct_message_channels"][0]["meta"][
"message_bus_last_ids"
].tap do |ids|
expect(ids["new_messages"]).not_to eq(nil)
expect(ids["new_mentions"]).not_to eq(nil)
end
end
context "when the chatable of a channel is destroyed" do
context "when the channel is a category" do
it "doesnt return the channel" do
channel = Fabricate(:category_channel)
channel.add(current_user)
channel.chatable.destroy!
get "/chat/api/me/channels"
expect(response.status).to eq(200)
expect(response.parsed_body["public_channels"]).to be_blank
end
end
context "when the channel is a direct message" do
it "doesnt return the channel" do
channel = Fabricate(:direct_message_channel, users: [current_user])
channel.chatable.destroy!
get "/chat/api/me/channels"
expect(response.status).to eq(200)
expect(response.parsed_body["direct_message_channels"]).to be_blank
end
end
end
end
end
end

View File

@ -1,58 +0,0 @@
# frozen_string_literal: true
describe ListController do
fab!(:current_user) { Fabricate(:user) }
before do
SiteSetting.chat_enabled = true
sign_in(current_user)
end
describe "#latest" do
it "does not do N+1 chat_channel_archive queries based on the number of public and DM channels" do
user_1 = Fabricate(:user)
Fabricate(:direct_message_channel, users: [current_user, user_1])
public_channel_1 = Fabricate(:chat_channel)
public_channel_2 = Fabricate(:chat_channel)
public_channel_1.add(current_user)
# warm up
get "/latest.html"
expect(response.status).to eq(200)
initial_sql_queries_count =
track_sql_queries do
get "/latest.html"
expect(response.status).to eq(200)
expect(response.body).to have_tag("div#data-preloaded") do |element|
current_user_json =
JSON.parse(
JSON.parse(element.current_scope.attribute("data-preloaded").value)["currentUser"],
)
expect(current_user_json["chat_channels"]["direct_message_channels"].count).to eq(1)
expect(current_user_json["chat_channels"]["public_channels"].count).to eq(1)
end
end.count
public_channel_2.add(current_user)
user_2 = Fabricate(:user)
Fabricate(:direct_message_channel, users: [current_user, user_2])
new_sql_queries_count =
track_sql_queries do
get "/latest.html"
expect(response.status).to eq(200)
expect(response.body).to have_tag("div#data-preloaded") do |element|
current_user_json =
JSON.parse(
JSON.parse(element.current_scope.attribute("data-preloaded").value)["currentUser"],
)
expect(current_user_json["chat_channels"]["direct_message_channels"].count).to eq(2)
expect(current_user_json["chat_channels"]["public_channels"].count).to eq(2)
end
end.count
expect(new_sql_queries_count).to be <= initial_sql_queries_count
end
end
end

View File

@ -211,4 +211,12 @@ RSpec.describe Chat::ListChannelMessages do
).of(Time.zone.now)
end
end
context "when update_user_last_channel" do
it "updates the custom field" do
expect { result }.to change { user.custom_fields[Chat::LAST_CHAT_CHANNEL_ID] }.from(nil).to(
channel.id,
)
end
end
end

View File

@ -0,0 +1,76 @@
# frozen_string_literal: true
RSpec.describe Chat::ListUserChannels do
subject(:result) { described_class.call(params) }
fab!(:current_user) { Fabricate(:user) }
fab!(:channel_1) { Fabricate(:chat_channel) }
let(:guardian) { Guardian.new(current_user) }
let(:params) { { guardian: guardian } }
before { channel_1.add(current_user) }
it "returns the structured data" do
expect(result.structured[:post_allowed_category_ids]).to eq(nil)
expect(result.structured[:unread_thread_overview]).to eq({})
expect(result.structured[:memberships].to_a).to eq([channel_1.membership_for(current_user)])
expect(result.structured[:public_channels]).to eq([channel_1])
expect(result.structured[:direct_message_channels]).to eq([])
expect(result.structured[:tracking].channel_tracking[channel_1.id]).to eq(
{ mention_count: 0, unread_count: 0 },
)
end
context "when the category is restricted and user has readonly permissions" do
fab!(:group_1) { Fabricate(:group) }
fab!(:private_channel_1) { Fabricate(:private_category_channel, group: group_1) }
before do
private_channel_1.chatable.category_groups.first.update!(
permission_type: CategoryGroup.permission_types[:readonly],
)
group_1.add(current_user)
private_channel_1.add(current_user)
end
it "doesn't list the associated channel" do
expect(result.structured[:public_channels]).to contain_exactly(channel_1)
end
end
context "when the category is restricted and user has permissions" do
fab!(:group_1) { Fabricate(:group) }
fab!(:private_channel_1) { Fabricate(:private_category_channel, group: group_1) }
before do
group_1.add(current_user)
private_channel_1.add(current_user)
end
it "lists the associated channel" do
expect(result.structured[:public_channels]).to contain_exactly(channel_1, private_channel_1)
end
end
it "doesn't return dm channels from other users" do
Fabricate(:direct_message_channel)
expect(result.structured[:direct_message_channels]).to eq([])
end
it "returns dm channels you are part of" do
dm_channel = Fabricate(:direct_message_channel, users: [current_user])
expect(result.structured[:direct_message_channels]).to eq([dm_channel])
end
it "doesnt return channels with destroyed chatable" do
dm_channel = Fabricate(:direct_message_channel, users: [current_user])
dm_channel.chatable.destroy!
channel_1.chatable.destroy!
expect(result.structured[:direct_message_channels]).to eq([])
expect(result.structured[:public_channels]).to eq([])
end
end

View File

@ -184,7 +184,12 @@ RSpec.describe "Chat channel", type: :system do
)
end
before { channel_1.add(other_user) }
before do
SiteSetting.enable_user_status = true
current_user.set_status!("off to dentist", "tooth")
other_user.set_status!("surfing", "surfing_man")
channel_1.add(other_user)
end
it "highlights the mentions" do
chat_page.visit_channel(channel_1)
@ -200,9 +205,6 @@ RSpec.describe "Chat channel", type: :system do
end
it "renders user status on mentions" do
SiteSetting.enable_user_status = true
current_user.set_status!("off to dentist", "tooth")
other_user.set_status!("surfing", "surfing_man")
Fabricate(:user_chat_mention, user: current_user, chat_message: message)
Fabricate(:user_chat_mention, user: other_user, chat_message: message)
@ -215,6 +217,30 @@ RSpec.describe "Chat channel", type: :system do
".mention .user-status-message img[alt='#{other_user.user_status.emoji}']",
)
end
it "renders user status when expanding collapsed message" do
message_1 =
Fabricate(
:chat_message,
chat_channel: channel_1,
message: "hello @#{other_user.username}",
user: current_user,
)
chat_page.visit_channel(channel_1)
channel_page.messages.delete(message_1)
channel_page.messages.restore(message_1)
expect(page).to have_selector(
".chat-message-container[data-id=\"#{message_1.id}\"] .mention .user-status-message img[alt='#{other_user.user_status.emoji}']",
)
other_user.set_status!("hello", "heart")
expect(page).to have_selector(
".chat-message-container[data-id=\"#{message_1.id}\"] .mention .user-status-message img[alt='#{other_user.user_status.emoji}']",
)
end
end
context "when reply is right under" do

View File

@ -30,8 +30,7 @@ RSpec.describe "Chat New Message from params", type: :system do
it "redirects to chat channel if recipients param is missing" do
visit("/chat/new-message")
# chat service selects public channel from getIdealFirstChannelId
expect(page).to have_current_path("/chat/c/#{public_channel.slug}/#{public_channel.id}")
expect(page).to have_no_current_path("/chat/new-message")
end
end

View File

@ -106,6 +106,7 @@ RSpec.describe "Drawer", type: :system do
fab!(:user_1) { Fabricate(:user) }
before do
current_user.upsert_custom_fields(::Chat::LAST_CHAT_CHANNEL_ID => channel_1.id)
channel_1.add(current_user)
channel_2.add(current_user)
channel_1.add(user_1)
@ -140,6 +141,7 @@ RSpec.describe "Drawer", type: :system do
fab!(:channel) { Fabricate(:chat_channel) }
before do
current_user.upsert_custom_fields(::Chat::LAST_CHAT_CHANNEL_ID => channel.id)
channel.add(current_user)
set_subfolder "/discuss"
end

View File

@ -6,7 +6,7 @@ RSpec.describe "Mentions warnings", type: :system do
fab!(:channel_2) { Fabricate(:chat_channel) }
let(:chat_page) { PageObjects::Pages::Chat.new }
let(:chat_channel_page) { PageObjects::Pages::ChatChannel.new }
let(:channel_page) { PageObjects::Pages::ChatChannel.new }
before do
chat_system_bootstrap(current_user, [channel_1, channel_2])
@ -22,7 +22,7 @@ RSpec.describe "Mentions warnings", type: :system do
it "displays a warning" do
chat_page.visit_channel(channel_1)
chat_channel_page.type_in_composer("@#{admin_mentionable_group.name} ")
channel_page.type_in_composer("@#{admin_mentionable_group.name} ")
expect(page).to have_css(".chat-mention-warnings")
expect(page.find(".chat-mention-warnings-list__simple")).to have_content(
@ -46,7 +46,7 @@ RSpec.describe "Mentions warnings", type: :system do
it "displays a warning" do
chat_page.visit_channel(channel_1)
chat_channel_page.type_in_composer("@#{publicly_mentionable_group.name} ")
channel_page.type_in_composer("@#{publicly_mentionable_group.name} ")
expect(page).to have_css(".chat-mention-warnings")
expect(page.find(".chat-mention-warnings-list__simple")).to have_content(
@ -60,13 +60,32 @@ RSpec.describe "Mentions warnings", type: :system do
it "displays a warning" do
chat_page.visit_channel(channel_1)
chat_channel_page.type_in_composer(
channel_page.type_in_composer(
"@#{user_2.username} @#{publicly_mentionable_group.name} ",
)
expect(page).to have_css(".chat-mention-warnings")
expect(page.find(".chat-mention-warnings-list__simple")).to be_present
end
it "doesnt count duplicates" do
chat_page.visit_channel(channel_1)
channel_page.type_in_composer("@#{user_2.username} @#{user_2.username} ")
expect(page).to have_no_css(".chat-mention-warnings")
end
it "doesn't consider code-blocks when counting mentions" do
raw =
"Hey @#{user_2.username}\n\n```\ndef foo\n @#{publicly_mentionable_group.name} = true\nend\n```\n"
message_1 =
Fabricate(:chat_message, user: current_user, chat_channel: channel_1, message: raw)
chat_page.visit_channel(channel_1)
channel_page.messages.edit(message_1)
expect(page).to have_no_css(".chat-mention-warnings")
end
end
end
end
@ -77,7 +96,7 @@ RSpec.describe "Mentions warnings", type: :system do
%w[@here @all].each do |mention_text|
it "displays a warning" do
chat_page.visit_channel(channel_1)
chat_channel_page.type_in_composer(mention_text)
channel_page.type_in_composer(mention_text)
expect(page).to have_css(".chat-mention-warnings")
expect(page.find(".chat-mention-warnings-list__simple")).to be_present

View File

@ -18,6 +18,7 @@ RSpec.describe "Navigation", type: :system do
let(:chat_drawer_page) { PageObjects::Pages::ChatDrawer.new }
before do
current_user.upsert_custom_fields(::Chat::LAST_CHAT_CHANNEL_ID => category_channel.id)
chat_system_bootstrap(current_user, [category_channel, category_channel_2])
current_user.user_option.update(
chat_separate_sidebar_mode: UserOption.chat_separate_sidebar_modes[:never],
@ -83,13 +84,11 @@ RSpec.describe "Navigation", type: :system do
expect(page).to have_current_path(
chat.channel_path(category_channel.slug, category_channel.id),
)
expect(page).to have_css("html.has-full-page-chat")
expect(page).to have_css(".chat-message-container[data-id='#{message.id}']")
end
end
context "when visiting mobile only routes on desktop" do
it "redirects /chat/channels to ideal first channel" do
it "redirects /chat/channels to browse" do
visit("/chat/channels")
expect(page).to have_current_path(
@ -97,7 +96,7 @@ RSpec.describe "Navigation", type: :system do
)
end
it "redirects /chat/direct-messages to ideal first channel" do
it "redirects /chat/direct-messages to browse" do
visit("/chat/direct-messages")
expect(page).to have_current_path(
@ -153,7 +152,9 @@ RSpec.describe "Navigation", type: :system do
chat_page.open
chat_page.minimize_full_page
expect(page).to have_current_path(latest_path)
expect(page).to have_current_path(
chat.channel_path(category_channel.slug, category_channel.id),
)
end
end

View File

@ -36,7 +36,10 @@ RSpec.describe "Removing channel", type: :system do
fab!(:channel_1) { Fabricate(:chat_channel) }
fab!(:channel_2) { Fabricate(:direct_message_channel, users: [current_user, Fabricate(:user)]) }
before { channel_1.add(current_user) }
before do
current_user.upsert_custom_fields(::Chat::LAST_CHAT_CHANNEL_ID => channel_1.id)
channel_1.add(current_user)
end
it "redirects to another followed channgel" do
chat_page.visit_channel(channel_2)

View File

@ -15,49 +15,54 @@ acceptance("Chat | Hashtag CSS Generator", function (needs) {
needs.settings({ chat_enabled: true });
needs.user({
has_chat_enabled: true,
chat_channels: {
public_channels: [
{
id: 44,
chatable_id: 1,
chatable_type: "Category",
meta: { message_bus_last_ids: {} },
current_user_membership: { following: true },
chatable: category1,
},
{
id: 74,
chatable_id: 2,
chatable_type: "Category",
meta: { message_bus_last_ids: {} },
current_user_membership: { following: true },
chatable: category2,
},
{
id: 88,
chatable_id: 4,
chatable_type: "Category",
meta: { message_bus_last_ids: {} },
current_user_membership: { following: true },
chatable: category3,
},
],
direct_message_channels: [],
meta: { message_bus_last_ids: {} },
tracking: {
channel_tracking: {
44: { unread_count: 0, mention_count: 0 },
74: { unread_count: 0, mention_count: 0 },
88: { unread_count: 0, mention_count: 0 },
},
thread_tracking: {},
},
},
});
needs.site({
categories: [category1, category2, category3],
});
needs.pretender((server, helper) => {
server.get("/chat/api/me/channels", () =>
helper.response({
public_channels: [
{
id: 44,
chatable_id: 1,
chatable_type: "Category",
meta: { message_bus_last_ids: {} },
current_user_membership: { following: true },
chatable: category1,
},
{
id: 74,
chatable_id: 2,
chatable_type: "Category",
meta: { message_bus_last_ids: {} },
current_user_membership: { following: true },
chatable: category2,
},
{
id: 88,
chatable_id: 4,
chatable_type: "Category",
meta: { message_bus_last_ids: {} },
current_user_membership: { following: true },
chatable: category3,
},
],
direct_message_channels: [],
meta: { message_bus_last_ids: {} },
tracking: {
channel_tracking: {
44: { unread_count: 0, mention_count: 0 },
74: { unread_count: 0, mention_count: 0 },
88: { unread_count: 0, mention_count: 0 },
},
thread_tracking: {},
},
})
);
});
test("hashtag CSS classes are generated", async function (assert) {
await visit("/");
const cssTag = document.querySelector("style#hashtag-css-generator");
@ -66,10 +71,7 @@ acceptance("Chat | Hashtag CSS Generator", function (needs) {
".hashtag-category-badge { background-color: var(--primary-medium); }\n" +
".hashtag-color--category-1 { background-color: #ff0000; }\n" +
".hashtag-color--category-2 { background-color: #333; }\n" +
".hashtag-color--category-4 { background-color: #2B81AF; }\n" +
".d-icon.hashtag-color--channel-44 { color: #ff0000 }\n" +
".d-icon.hashtag-color--channel-74 { color: #333 }\n" +
".d-icon.hashtag-color--channel-88 { color: #2B81AF }"
".hashtag-color--category-4 { background-color: #2B81AF; }"
);
});
});

View File

@ -1,107 +0,0 @@
import { fillIn, visit } from "@ember/test-helpers";
import { test } from "qunit";
import pretender, { response } from "discourse/tests/helpers/create-pretender";
import { acceptance } from "discourse/tests/helpers/qunit-helpers";
acceptance("Chat | Mentions", function (needs) {
const channelId = 1;
const actingUser = {
id: 1,
username: "acting_user",
};
const channel = {
id: channelId,
chatable_id: 1,
chatable_type: "Category",
meta: { message_bus_last_ids: {}, can_delete_self: true },
current_user_membership: { following: true },
allow_channel_wide_mentions: false,
chatable: { id: 1 },
title: "Some title",
};
needs.settings({ chat_enabled: true });
needs.user({
...actingUser,
has_chat_enabled: true,
chat_channels: {
public_channels: [channel],
direct_message_channels: [],
meta: { message_bus_last_ids: {} },
tracking: {},
},
});
needs.hooks.beforeEach(function () {
pretender.post(`/chat/drafts`, () => response({}));
pretender.get(`/chat/api/channels/${channelId}/messages`, () =>
response({
messages: [],
meta: {
can_load_more_future: false,
},
})
);
pretender.get("/chat/api/mentions/groups.json", () =>
response({
unreachable: [],
over_members_limit: [],
invalid: ["and"],
})
);
});
test("shows warning when mention limit exceeded", async function (assert) {
this.siteSettings.max_mentions_per_chat_message = 2;
await visit(`/chat/c/-/${channelId}`);
await fillIn(".chat-composer__input", `Hey @user1 @user2 @user3`);
assert.dom(".chat-mention-warnings").exists();
});
test("shows warning for @here mentions when channel-wide mentions are disabled", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await fillIn(".chat-composer__input", `Hey @here`);
assert.dom(".chat-mention-warnings").exists();
});
test("shows warning for @all mention when channel-wide mentions are disabled", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await fillIn(".chat-composer__input", `Hey @all`);
assert.dom(".chat-mention-warnings").exists();
});
test("ignores duplicates when counting mentions", async function (assert) {
this.siteSettings.max_mentions_per_chat_message = 2;
await visit(`/chat/c/-/${channelId}`);
const mention = `@user1`;
await fillIn(
".chat-composer__input",
`Hey ${mention} ${mention} ${mention}`
);
assert.dom(".chat-mention-warnings").doesNotExist();
});
test("doesn't consider code-blocks when counting mentions", async function (assert) {
this.siteSettings.max_mentions_per_chat_message = 2;
await visit(`/chat/c/-/${channelId}`);
// since @bar is inside a code-block it shouldn't be considered a mention
const message = `Hey @user1 @user2
\`\`\`
def foo
@bar = true
end
\`\`\`
`;
await fillIn(".chat-composer__input", message);
assert.dom(".chat-mention-warnings").doesNotExist();
});
});

View File

@ -1,332 +0,0 @@
import { click, triggerEvent, visit, waitFor } from "@ember/test-helpers";
import { skip, test } from "qunit";
import pretender, { response } from "discourse/tests/helpers/create-pretender";
import {
acceptance,
loggedInUser,
publishToMessageBus,
query,
simulateKeys,
} from "discourse/tests/helpers/qunit-helpers";
acceptance("Chat | User status on mentions", function (needs) {
const channelId = 1;
const messageId = 1;
const actingUser = {
id: 1,
username: "acting_user",
};
const mentionedUser1 = {
id: 1000,
username: "user1",
status: {
description: "surfing",
emoji: "surfing_man",
},
};
const mentionedUser2 = {
id: 2000,
username: "user2",
status: {
description: "vacation",
emoji: "desert_island",
},
};
const mentionedUser3 = {
id: 3000,
username: "user3",
status: {
description: "off to dentist",
emoji: "tooth",
},
};
const message = {
id: messageId,
message: `Hey @${mentionedUser1.username}`,
cooked: `<p>Hey <a class="mention" href="/u/${mentionedUser1.username}">@${mentionedUser1.username}</a></p>`,
mentioned_users: [mentionedUser1],
user: actingUser,
created_at: "2020-08-04T15:00:00.000Z",
};
const newStatus = {
description: "working remotely",
emoji: "house",
};
const channel = {
id: channelId,
chatable_id: 1,
chatable_type: "Category",
title: "A category channel",
meta: { message_bus_last_ids: {}, can_delete_self: true },
current_user_membership: { following: true },
chatable: { id: 1 },
};
needs.settings({ chat_enabled: true });
needs.user({
...actingUser,
has_chat_enabled: true,
chat_channels: {
public_channels: [channel],
direct_message_channels: [],
meta: { message_bus_last_ids: {} },
tracking: {},
},
});
needs.hooks.beforeEach(function () {
pretender.post(`/chat/1`, () => response({}));
pretender.put(`/chat/1/edit/${messageId}`, () => response({}));
pretender.post(`/chat/drafts`, () => response({}));
pretender.put(`/chat/api/channels/1/read/1`, () => response({}));
pretender.get(`/chat/api/channels/1/messages`, () =>
response({
messages: [message],
meta: {
can_load_more_future: false,
},
})
);
pretender.delete(`/chat/api/channels/1/messages/${messageId}`, () =>
response({})
);
pretender.put(`/chat/api/channels/1/messages/${messageId}/restore`, () =>
response({})
);
pretender.get("/u/search/users", () =>
response({
users: [mentionedUser2, mentionedUser3],
})
);
pretender.get("/chat/api/mentions/groups.json", () =>
response({
unreachable: [],
over_members_limit: [],
invalid: ["and"],
})
);
});
skip("just posted messages | it shows status on mentions ", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await typeWithAutocompleteAndSend(`mentioning @${mentionedUser2.username}`);
assertStatusIsRendered(
assert,
statusSelector(mentionedUser2.username),
mentionedUser2.status
);
});
skip("just posted messages | it updates status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await typeWithAutocompleteAndSend(`mentioning @${mentionedUser2.username}`);
loggedInUser().appEvents.trigger("user-status:changed", {
[mentionedUser2.id]: newStatus,
});
const selector = statusSelector(mentionedUser2.username);
await waitFor(selector);
assertStatusIsRendered(assert, selector, newStatus);
});
skip("just posted messages | it deletes status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await typeWithAutocompleteAndSend(`mentioning @${mentionedUser2.username}`);
loggedInUser().appEvents.trigger("user-status:changed", {
[mentionedUser2.id]: null,
});
const selector = statusSelector(mentionedUser2.username);
await waitFor(selector, { count: 0 });
assert.dom(selector).doesNotExist("status is deleted");
});
skip("edited messages | it shows status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await editMessage(
".chat-message-content",
`mentioning @${mentionedUser3.username}`
);
assertStatusIsRendered(
assert,
statusSelector(mentionedUser3.username),
mentionedUser3.status
);
});
skip("edited messages | it updates status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await editMessage(
".chat-message-content",
`mentioning @${mentionedUser3.username}`
);
loggedInUser().appEvents.trigger("user-status:changed", {
[mentionedUser3.id]: newStatus,
});
const selector = statusSelector(mentionedUser3.username);
await waitFor(selector);
assertStatusIsRendered(assert, selector, newStatus);
});
skip("edited messages | it deletes status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await editMessage(
".chat-message-content",
`mentioning @${mentionedUser3.username}`
);
loggedInUser().appEvents.trigger("user-status:changed", {
[mentionedUser3.id]: null,
});
const selector = statusSelector(mentionedUser3.username);
await waitFor(selector, { count: 0 });
assert.dom(selector).doesNotExist("status is deleted");
});
test("deleted messages | it shows status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await deleteMessage(".chat-message-content");
await click(".chat-message-expand");
assertStatusIsRendered(
assert,
statusSelector(mentionedUser1.username),
mentionedUser1.status
);
});
test("deleted messages | it updates status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await deleteMessage(".chat-message-content");
await click(".chat-message-expand");
loggedInUser().appEvents.trigger("user-status:changed", {
[mentionedUser1.id]: newStatus,
});
const selector = statusSelector(mentionedUser1.username);
await waitFor(selector);
assertStatusIsRendered(assert, selector, newStatus);
});
test("deleted messages | it deletes status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await deleteMessage(".chat-message-content");
await click(".chat-message-expand");
loggedInUser().appEvents.trigger("user-status:changed", {
[mentionedUser1.id]: null,
});
const selector = statusSelector(mentionedUser1.username);
await waitFor(selector, { count: 0 });
assert.dom(selector).doesNotExist("status is deleted");
});
test("restored messages | it shows status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await deleteMessage(".chat-message-content");
await restoreMessage(".chat-message-text.-deleted");
assertStatusIsRendered(
assert,
statusSelector(mentionedUser1.username),
mentionedUser1.status
);
});
test("restored messages | it updates status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await deleteMessage(".chat-message-content");
await restoreMessage(".chat-message-text.-deleted");
loggedInUser().appEvents.trigger("user-status:changed", {
[mentionedUser1.id]: newStatus,
});
const selector = statusSelector(mentionedUser1.username);
await waitFor(selector);
assertStatusIsRendered(assert, selector, newStatus);
});
test("restored messages | it deletes status on mentions", async function (assert) {
await visit(`/chat/c/-/${channelId}`);
await deleteMessage(".chat-message-content");
await restoreMessage(".chat-message-text.-deleted");
loggedInUser().appEvents.trigger("user-status:changed", {
[mentionedUser1.id]: null,
});
const selector = statusSelector(mentionedUser1.username);
await waitFor(selector, { count: 0 });
assert.dom(selector).doesNotExist("status is deleted");
});
function assertStatusIsRendered(assert, selector, status) {
assert
.dom(selector)
.exists("status is rendered")
.hasAttribute(
"src",
new RegExp(`${status.emoji}.png`),
"status emoji is updated"
);
}
async function deleteMessage(messageSelector) {
await triggerEvent(query(messageSelector), "mouseenter");
await click(".more-buttons .select-kit-header-wrapper");
await click(".select-kit-collection .select-kit-row[data-value='delete']");
await publishToMessageBus(`/chat/${channelId}`, {
type: "delete",
deleted_id: messageId,
deleted_at: "2022-01-01T08:00:00.000Z",
});
}
async function editMessage(messageSelector, text) {
await triggerEvent(query(messageSelector), "mouseenter");
await click(".more-buttons .select-kit-header-wrapper");
await click(".select-kit-collection .select-kit-row[data-value='edit']");
await typeWithAutocompleteAndSend(text);
}
async function restoreMessage(messageSelector) {
await triggerEvent(query(messageSelector), "mouseenter");
await click(".more-buttons .select-kit-header-wrapper");
await click(".select-kit-collection .select-kit-row[data-value='restore']");
await publishToMessageBus(`/chat/${channelId}`, {
type: "restore",
chat_message: message,
});
}
async function typeWithAutocompleteAndSend(text) {
await simulateKeys(query(".chat-composer__input"), text);
await click(".autocomplete.ac-user .selected");
await click(".chat-composer-button.-send");
}
function statusSelector(username) {
return `.mention[href='/u/${username}'] .user-status-message img`;
}
});

View File

@ -51,6 +51,12 @@ module(
meta: { can_delete_self: true },
})
);
pretender.get(`/chat/api/me/channels`, () =>
response({
direct_message_channels: [],
public_channels: [],
})
);
this.channel = fabricators.channel({
id: channelId,