DEV: start glimmer-ification and optimisations of chat plugin (#19531)

Note this is a very large PR, and some of it could have been splited, but keeping it one chunk made it to merge conflicts and to revert if necessary. Actual new code logic is also not that much, as most of the changes are removing js tests, adding system specs or moving things around.

To make it possible this commit is doing the following changes:

- converting (and adding new) existing js acceptances tests into system tests. This change was necessary to ensure as little regressions as possible while changing paradigm
- moving away from store. Using glimmer and tracked properties requires to have class objects everywhere and as a result works well with models. However store/adapters are suffering from many bugs and limitations. As a workaround the `chat-api` and `chat-channels-manager` are an answer to this problem by encapsulating backend calls and frontend storage logic; while still using js models.
- dropping `appEvents` as much as possible. Using tracked properties and a better local storage of channel models, allows to be much more reactive and doesn’t require arbitrary manual updates everywhere in the app.
- while working on replacing store, the existing work of a chat api (backend) has been continued to support more cases.
- removing code from the `chat` service to separate concerns, `chat-subscriptions-manager` and `chat-channels-manager`, being the largest examples of where the code has been rewritten/moved.

Future wok:
- improve behavior when closing/deleting a channel, it's already slightly buggy on live, it's rare enough that it's not a big issue, but should be improved
- improve page objects used in chat
- move more endpoints to the API
- finish temporarily skipped tests
- extract more code from the `chat` service
- use glimmer for `chat-messages`
- separate concerns in `chat-live-pane`
- eventually add js tests for `chat-api`, `chat-channels-manager` and `chat-subscriptions-manager`, they are indirectly heavy tested through system tests but it would be nice to at least test the public API

<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
This commit is contained in:
Joffrey JAFFEUX
2022-12-21 13:21:02 +01:00
committed by GitHub
parent 269b6177c1
commit d2e24f9569
210 changed files with 6980 additions and 10753 deletions

View File

@ -1,20 +0,0 @@
# frozen_string_literal: true
class Chat::Api::ChatChannelMembershipsController < Chat::Api::ChatChannelsController
def index
channel = find_chat_channel
offset = (params[:offset] || 0).to_i
limit = (params[:limit] || 50).to_i.clamp(1, 50)
memberships =
ChatChannelMembershipsQuery.call(
channel,
offset: offset,
limit: limit,
username: params[:username],
)
render_serialized(memberships, UserChatChannelMembershipSerializer, root: false)
end
end

View File

@ -1,12 +0,0 @@
# frozen_string_literal: true
MEMBERSHIP_EDITABLE_PARAMS = %i[muted desktop_notification_level mobile_notification_level]
class Chat::Api::ChatChannelNotificationsSettingsController < Chat::Api::ChatChannelsController
def update
settings_params = params.permit(MEMBERSHIP_EDITABLE_PARAMS)
membership = find_membership
membership.update!(settings_params.to_h)
render_serialized(membership, UserChatChannelMembershipSerializer, root: false)
end
end

View File

@ -0,0 +1,42 @@
# frozen_string_literal: true
class Chat::Api::ChatChannelsArchivesController < Chat::Api::ChatChannelsController
def create
existing_archive = channel_from_params.chat_channel_archive
if existing_archive.present?
guardian.ensure_can_change_channel_status!(channel_from_params, :archived)
raise Discourse::InvalidAccess if !existing_archive.failed?
Chat::ChatChannelArchiveService.retry_archive_process(chat_channel: channel_from_params)
else
archive_params =
params
.require(:archive)
.tap do |ca|
ca.require(:type)
ca.permit(:title, :topic_id, :category_id, tags: [])
end
new_topic = archive_params[:type] == "new_topic"
raise Discourse::InvalidParameters if new_topic && archive_params[:title].blank?
raise Discourse::InvalidParameters if !new_topic && archive_params[:topic_id].blank?
if !guardian.can_change_channel_status?(channel_from_params, :read_only)
raise Discourse::InvalidAccess.new(I18n.t("chat.errors.channel_cannot_be_archived"))
end
Chat::ChatChannelArchiveService.begin_archive_process(
chat_channel: channel_from_params,
acting_user: current_user,
topic_params: {
topic_id: archive_params[:topic_id],
topic_title: archive_params[:title],
category_id: archive_params[:category_id],
tags: archive_params[:tags],
},
)
end
render json: success_json
end
end

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true
CHAT_CHANNEL_EDITABLE_PARAMS = %i[name description]
CATEGORY_CHAT_CHANNEL_EDITABLE_PARAMS = %i[auto_join_users allow_channel_wide_mentions]
CHANNEL_EDITABLE_PARAMS = %i[name description]
CATEGORY_CHANNEL_EDITABLE_PARAMS = %i[auto_join_users allow_channel_wide_mentions]
class Chat::Api::ChatChannelsController < Chat::Api
def index
@ -9,6 +9,9 @@ class Chat::Api::ChatChannelsController < Chat::Api
params.permit(:filter, :limit, :offset),
).symbolize_keys!
options[:offset] = options[:offset].to_i
options[:limit] = (options[:limit] || 25).to_i
memberships = Chat::ChatChannelMembershipManager.all_for_user(current_user)
channels = Chat::ChatChannelFetcher.secured_public_channels(guardian, memberships, options)
@ -20,73 +23,166 @@ class Chat::Api::ChatChannelsController < Chat::Api
membership: memberships.find { |membership| membership.chat_channel_id == channel.id },
)
end
render json: serialized_channels, root: false
pagination_options =
options.slice(:offset, :limit, :filter).merge(offset: options[:offset] + options[:limit])
pagination_params = pagination_options.map { |k, v| "#{k}=#{v}" }.join("&")
render json: serialized_channels,
root: "channels",
meta: {
load_more_url: "/chat/api/channels?#{pagination_params}",
}
end
def destroy
confirmation = params.require(:channel).require(:name_confirmation)&.downcase
guardian.ensure_can_delete_chat_channel!
if channel_from_params.title(current_user).downcase != confirmation
raise Discourse::InvalidParameters.new(:name_confirmation)
end
begin
ChatChannel.transaction do
channel_from_params.trash!(current_user)
StaffActionLogger.new(current_user).log_custom(
"chat_channel_delete",
{
chat_channel_id: channel_from_params.id,
chat_channel_name: channel_from_params.title(current_user),
},
)
end
rescue ActiveRecord::Rollback
return render_json_error(I18n.t("chat.errors.delete_channel_failed"))
end
Jobs.enqueue(:chat_channel_delete, { chat_channel_id: channel_from_params.id })
render json: success_json
end
def create
channel_params =
params.require(:channel).permit(:chatable_id, :name, :description, :auto_join_users)
guardian.ensure_can_create_chat_channel!
if channel_params[:name].length > SiteSetting.max_topic_title_length
raise Discourse::InvalidParameters.new(:name)
end
if ChatChannel.exists?(
chatable_type: "Category",
chatable_id: channel_params[:chatable_id],
name: channel_params[:name],
)
raise Discourse::InvalidParameters.new(I18n.t("chat.errors.channel_exists_for_category"))
end
chatable = Category.find_by(id: channel_params[:chatable_id])
raise Discourse::NotFound unless chatable
auto_join_users =
ActiveRecord::Type::Boolean.new.deserialize(channel_params[:auto_join_users]) || false
channel =
chatable.create_chat_channel!(
name: channel_params[:name],
description: channel_params[:description],
user_count: 1,
auto_join_users: auto_join_users,
)
channel.user_chat_channel_memberships.create!(user: current_user, following: true)
if channel.auto_join_users
Chat::ChatChannelMembershipManager.new(channel).enforce_automatic_channel_memberships
end
render_serialized(
channel,
ChatChannelSerializer,
membership: channel.membership_for(current_user),
root: "channel",
)
end
def show
render_serialized(
channel_from_params,
ChatChannelSerializer,
membership: channel_from_params.membership_for(current_user),
root: "channel",
)
end
def update
guardian.ensure_can_edit_chat_channel!
chat_channel = find_chat_channel
if chat_channel.direct_message_channel?
if channel_from_params.direct_message_channel?
raise Discourse::InvalidParameters.new(
I18n.t("chat.errors.cant_update_direct_message_channel"),
)
end
params_to_edit = editable_params(params, chat_channel)
params_to_edit = editable_params(params, channel_from_params)
params_to_edit.each { |k, v| params_to_edit[k] = nil if params_to_edit[k].blank? }
if ActiveRecord::Type::Boolean.new.deserialize(params_to_edit[:auto_join_users])
auto_join_limiter(chat_channel).performed!
auto_join_limiter(channel_from_params).performed!
end
chat_channel.update!(params_to_edit)
channel_from_params.update!(params_to_edit)
ChatPublisher.publish_chat_channel_edit(chat_channel, current_user)
ChatPublisher.publish_chat_channel_edit(channel_from_params, current_user)
if chat_channel.category_channel? && chat_channel.auto_join_users
Chat::ChatChannelMembershipManager.new(chat_channel).enforce_automatic_channel_memberships
if channel_from_params.category_channel? && channel_from_params.auto_join_users
Chat::ChatChannelMembershipManager.new(
channel_from_params,
).enforce_automatic_channel_memberships
end
render_serialized(
chat_channel,
channel_from_params,
ChatChannelSerializer,
root: false,
membership: chat_channel.membership_for(current_user),
root: "channel",
membership: channel_from_params.membership_for(current_user),
)
end
private
def find_chat_channel
chat_channel = ChatChannel.find(params.require(:chat_channel_id))
guardian.ensure_can_join_chat_channel!(chat_channel)
chat_channel
def channel_from_params
@channel ||=
begin
channel = ChatChannel.find(params.require(:channel_id))
guardian.ensure_can_preview_chat_channel!(channel)
channel
end
end
def find_membership
chat_channel = find_chat_channel
membership = Chat::ChatChannelMembershipManager.new(chat_channel).find_for_user(current_user)
raise Discourse::NotFound if membership.blank?
membership
def membership_from_params
@membership ||=
begin
membership =
Chat::ChatChannelMembershipManager.new(channel_from_params).find_for_user(current_user)
raise Discourse::NotFound if membership.blank?
membership
end
end
def auto_join_limiter(chat_channel)
def auto_join_limiter(channel)
RateLimiter.new(
current_user,
"auto_join_users_channel_#{chat_channel.id}",
"auto_join_users_channel_#{channel.id}",
1,
3.minutes,
apply_limit_to_staff: true,
)
end
def editable_params(params, chat_channel)
permitted_params = CHAT_CHANNEL_EDITABLE_PARAMS
permitted_params += CATEGORY_CHAT_CHANNEL_EDITABLE_PARAMS if chat_channel.category_channel?
params.permit(*permitted_params)
def editable_params(params, channel)
permitted_params = CHANNEL_EDITABLE_PARAMS
permitted_params += CATEGORY_CHANNEL_EDITABLE_PARAMS if channel.category_channel?
params.require(:channel).permit(*permitted_params)
end
end

View File

@ -0,0 +1,21 @@
# frozen_string_literal: true
class Chat::Api::ChatChannelsCurrentUserMembershipController < Chat::Api::ChatChannelsController
def create
guardian.ensure_can_join_chat_channel!(channel_from_params)
render_serialized(
channel_from_params.add(current_user),
UserChatChannelMembershipSerializer,
root: "membership",
)
end
def destroy
render_serialized(
channel_from_params.remove(current_user),
UserChatChannelMembershipSerializer,
root: "membership",
)
end
end

View File

@ -0,0 +1,15 @@
# frozen_string_literal: true
MEMBERSHIP_EDITABLE_PARAMS = %i[muted desktop_notification_level mobile_notification_level]
class Chat::Api::ChatChannelsCurrentUserNotificationsSettingsController < Chat::Api::ChatChannelsController
def update
settings_params = params.require(:notifications_settings).permit(MEMBERSHIP_EDITABLE_PARAMS)
membership_from_params.update!(settings_params.to_h)
render_serialized(
membership_from_params,
UserChatChannelMembershipSerializer,
root: "membership",
)
end
end

View File

@ -0,0 +1,29 @@
# frozen_string_literal: true
class Chat::Api::ChatChannelsMembershipsController < Chat::Api::ChatChannelsController
def index
params.permit(:username, :offset, :limit)
offset = params[:offset].to_i
limit = (params[:limit] || 50).to_i.clamp(1, 50)
memberships =
ChatChannelMembershipsQuery.call(
channel_from_params,
offset: offset,
limit: limit,
username: params[:username],
)
render_serialized(
memberships,
UserChatChannelMembershipSerializer,
root: "memberships",
meta: {
total_rows: channel_from_params.user_count,
load_more_url:
"/chat/api/channels/#{channel_from_params.id}/memberships?offset=#{offset + limit}&limit=#{limit}&username=#{params[:username]}",
},
)
end
end

View File

@ -0,0 +1,35 @@
# frozen_string_literal: true
class Chat::Api::ChatChannelsMessagesMovesController < Chat::Api::ChatChannelsController
def create
move_params = params.require(:move)
move_params.require(:message_ids)
move_params.require(:destination_channel_id)
raise Discourse::InvalidAccess if !guardian.can_move_chat_messages?(channel_from_params)
destination_channel =
Chat::ChatChannelFetcher.find_with_access_check(
move_params[:destination_channel_id],
guardian,
)
begin
message_ids = move_params[:message_ids].map(&:to_i)
moved_messages =
Chat::MessageMover.new(
acting_user: current_user,
source_channel: channel_from_params,
message_ids: message_ids,
).move_to_channel(destination_channel)
rescue Chat::MessageMover::NoMessagesFound, Chat::MessageMover::InvalidChannel => err
return render_json_error(err.message)
end
render json:
success_json.merge(
destination_channel_id: destination_channel.id,
destination_channel_title: destination_channel.title(current_user),
first_moved_message_id: moved_messages.first.id,
)
end
end

View File

@ -0,0 +1,18 @@
# frozen_string_literal: true
class Chat::Api::ChatChannelsStatusController < Chat::Api::ChatChannelsController
def update
status = params.require(:status)
# we only want to use this endpoint for open/closed status changes,
# the others are more "special" and are handled by the archive endpoint
if !ChatChannel.statuses.keys.include?(status) || status == "read_only" || status == "archive"
raise Discourse::InvalidParameters
end
guardian.ensure_can_change_channel_status!(channel_from_params, status.to_sym)
channel_from_params.public_send("#{status}!", current_user)
render_serialized(channel_from_params, ChatChannelSerializer, root: "channel")
end
end

View File

@ -0,0 +1,82 @@
# frozen_string_literal: true
class Chat::Api::ChatChatablesController < Chat::Api
def index
params.require(:filter)
filter = params[:filter].downcase
memberships = Chat::ChatChannelMembershipManager.all_for_user(current_user)
public_channels =
Chat::ChatChannelFetcher.secured_public_channels(
guardian,
memberships,
filter: filter,
status: :open,
)
users = User.joins(:user_option).where.not(id: current_user.id)
if !Chat.allowed_group_ids.include?(Group::AUTO_GROUPS[:everyone])
users =
users
.joins(:groups)
.where(groups: { id: Chat.allowed_group_ids })
.or(users.joins(:groups).staff)
end
users = users.where(user_option: { chat_enabled: true })
like_filter = "%#{filter}%"
if SiteSetting.prioritize_username_in_ux || !SiteSetting.enable_names
users = users.where("users.username_lower ILIKE ?", like_filter)
else
users =
users.where(
"LOWER(users.name) ILIKE ? OR users.username_lower ILIKE ?",
like_filter,
like_filter,
)
end
users = users.limit(25).uniq
direct_message_channels =
if users.count > 0
# FIXME: investigate the cost of this query
ChatChannel
.includes(chatable: :users)
.joins(direct_message: :direct_message_users)
.group(1)
.having(
"ARRAY[?] <@ ARRAY_AGG(user_id) AND ARRAY[?] && ARRAY_AGG(user_id)",
[current_user.id],
users.map(&:id),
)
else
[]
end
user_ids_with_channel = []
direct_message_channels.each do |dm_channel|
user_ids = dm_channel.chatable.users.map(&:id)
user_ids_with_channel.concat(user_ids) if user_ids.count < 3
end
users_without_channel = users.filter { |u| !user_ids_with_channel.include?(u.id) }
if current_user.username.downcase.start_with?(filter)
# We filtered out the current user for the query earlier, but check to see
# if they should be included, and add.
users_without_channel << current_user
end
render_serialized(
{
public_channels: public_channels,
direct_message_channels: direct_message_channels,
users: users_without_channel,
memberships: memberships,
},
ChatChannelSearchSerializer,
root: false,
)
end
end

View File

@ -0,0 +1,8 @@
# frozen_string_literal: true
class Chat::Api::ChatCurrentUserChannelsController < Chat::Api
def index
structured = Chat::ChatChannelFetcher.structured(guardian)
render_serialized(structured, ChatChannelIndexSerializer, root: false)
end
end