DEV: Chat API channel#show changes for threading (#21632)

This commit moves message lookup and querying to the
/chat/api/channel/:id endpoint and adds the ability
to query the tracking state overview for threads as well
as the threads and thread tracking state for any thread
original messages found.

This will allow us to get an initial overview of thread
tracking for a user when they first enter a channel, rather
than pre-emptively loading N threads and tracking state
for those across all channels on the current user serializer,
which would be expensive.

This initial overview will be used in subsequent PRs to
flesh out the thread unread indicators in the UI.

This also moves many chunks of code that were in services
to reusable Query classes, since use of services inside
services is discouraged.
This commit is contained in:
Martin Brennan 2023-05-22 13:59:46 +02:00 committed by GitHub
parent 07061410d8
commit 5cce829901
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 1255 additions and 169 deletions

View File

@ -68,12 +68,29 @@ class Chat::Api::ChannelsController < Chat::ApiController
end
def show
render_serialized(
channel_from_params,
Chat::ChannelSerializer,
membership: channel_from_params.membership_for(current_user),
root: "channel",
)
if params[:target_message_id].present? || params[:include_messages].present?
with_service(
Chat::ChannelViewBuilder,
**params.permit(:channel_id, :target_message_id, :thread_id, :page_size, :direction).slice(
:channel_id,
:target_message_id,
:thread_id,
:page_size,
:direction,
),
) do
on_success { render_serialized(result.view, Chat::ViewSerializer, root: false) }
on_failed_policy(:target_message_exists) { raise Discourse::NotFound }
on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess }
end
else
render_serialized(
channel_from_params,
Chat::ChannelSerializer,
membership: channel_from_params.membership_for(current_user),
root: "channel",
)
end
end
def update

View File

@ -169,6 +169,8 @@ module Chat
render json: success_json
end
# TODO: (martin) This endpoint is deprecated -- the /api/channels/:id?include_messages=true
# endpoint should be used instead. We need to remove this and any JS references.
def messages
page_size = params[:page_size]&.to_i || 1000
direction = params[:direction].to_s
@ -250,6 +252,8 @@ module Chat
)
end
# TODO: (martin) This endpoint is deprecated -- the /api/channels/:id?target_message_id=:message_id
# endpoint should be used instead. We need to remove this and any JS references.
def lookup_message
set_channel_and_chatable_with_access_check(chat_channel_id: @message.chat_channel_id)

View File

@ -0,0 +1,47 @@
# frozen_string_literal: true
module Chat
# Represents a report of the tracking state for a user
# across threads and channels. This is returned by
# Chat::TrackingStateReportQuery.
class TrackingStateReport
attr_accessor :channel_tracking, :thread_tracking
class TrackingStateInfo
attr_accessor :unread_count, :mention_count
def initialize(info)
@unread_count = info[:unread_count]
@mention_count = info[:mention_count]
end
def to_hash
to_h
end
def to_h
{ unread_count: unread_count, mention_count: mention_count }
end
end
def initialize
@channel_tracking = {}
@thread_tracking = {}
end
def find_channel(channel_id)
TrackingStateInfo.new(channel_tracking[channel_id])
end
def find_thread(thread_id)
TrackingStateInfo.new(thread_tracking[thread_id])
end
def find_channel_threads(channel_id)
thread_tracking
.select { |_, thread| thread[:channel_id] == channel_id }
.map { |thread_id, thread| [thread_id, TrackingStateInfo.new(thread)] }
.to_h
end
end
end

View File

@ -2,20 +2,33 @@
module Chat
class View
attr_reader :user, :chat_channel, :chat_messages, :can_load_more_past, :can_load_more_future
attr_reader :user,
:chat_channel,
:chat_messages,
:can_load_more_past,
:can_load_more_future,
:thread_tracking_overview,
:threads,
:tracking
def initialize(
chat_channel:,
chat_messages:,
user:,
can_load_more_past: nil,
can_load_more_future: nil
can_load_more_future: nil,
thread_tracking_overview: nil,
threads: nil,
tracking: nil
)
@chat_channel = chat_channel
@chat_messages = chat_messages
@user = user
@can_load_more_past = can_load_more_past
@can_load_more_future = can_load_more_future
@thread_tracking_overview = thread_tracking_overview
@threads = threads
@tracking = tracking
end
def reviewable_ids

View File

@ -16,7 +16,10 @@ module Chat
# @param user_id [Integer] The ID of the user to count for.
# @param include_missing_memberships [Boolean] Whether to include channels
# that the user is not a member of. These counts will always be 0.
def self.call(channel_ids:, user_id:, include_missing_memberships: false)
# @param include_read [Boolean] Whether to include channels that the user
# is a member of where they have read all the messages. This overrides
# include_missing_memberships.
def self.call(channel_ids:, user_id:, include_missing_memberships: false, include_read: true)
sql = <<~SQL
SELECT (
SELECT COUNT(*) AS unread_count
@ -52,7 +55,14 @@ module Chat
#{include_missing_memberships ? "" : "LIMIT :limit"}
SQL
sql += <<~SQL if include_missing_memberships
sql = <<~SQL if !include_read
SELECT * FROM (
#{sql}
) AS channel_tracking
WHERE (unread_count > 0 OR mention_count > 0)
SQL
sql += <<~SQL if include_missing_memberships && include_read
UNION ALL
SELECT 0 AS unread_count, 0 AS mention_count, chat_channels.id AS channel_id
FROM chat_channels

View File

@ -0,0 +1,136 @@
# frozen_string_literal: true
module Chat
# Queries messages for a specific channel. This can be used in two modes:
#
# 1. Query messages around a target_message_id. This is used when a user
# needs to jump to the middle of a messages stream or load around their
# last read message ID. There is no pagination or direction here, just
# a limit on past and future messages.
# 2. Query messages with paginations and direction. This is used for normal
# scrolling of the messages stream of a channel.
#
# In both scenarios a thread_id can be provided to only get messages related
# to that thread within the channel.
#
# It is assumed that the user's permission to view the channel has already been
# established by the caller.
class MessagesQuery
PAST_MESSAGE_LIMIT = 20
FUTURE_MESSAGE_LIMIT = 20
PAST = "past"
FUTURE = "future"
VALID_DIRECTIONS = [PAST, FUTURE]
# @param channel [Chat::Channel] The channel to query messages within.
# @param guardian [Guardian] The guardian to use for permission checks.
# @param thread_id [Integer] (optional) The thread ID to filter messages by.
# @param target_message_id [Integer] (optional) The message ID to query around.
# It is assumed that the caller already checked if this exists.
# @param include_thread_messages [Boolean] (optional) Whether to include messages
# that are linked to a thread.
# @param page_size [Integer] (optional) The number of messages to fetch when not
# using the target_message_id param.
# @param direction [String] (optional) The direction to fetch messages in when not
# using the target_message_id param. Must be valid. If not provided, only the
# latest messages for the channel are loaded.
def self.call(
channel:,
guardian:,
thread_id: nil,
target_message_id: nil,
include_thread_messages: false,
page_size: PAST_MESSAGE_LIMIT + FUTURE_MESSAGE_LIMIT,
direction: nil
)
messages = base_query(channel: channel)
messages = messages.with_deleted if guardian.can_moderate_chat?(channel.chatable)
if thread_id.present?
include_thread_messages = true
messages = messages.where(thread_id: thread_id)
end
messages = messages.where(<<~SQL, channel_id: channel.id) if !include_thread_messages
chat_messages.thread_id IS NULL OR chat_messages.id IN (
SELECT original_message_id
FROM chat_threads
WHERE chat_threads.channel_id = :channel_id
)
SQL
if target_message_id.present?
query_around_target(target_message_id, channel, messages)
else
query_paginated_messages(direction, page_size, channel, messages)
end
end
def self.base_query(channel:)
query =
Chat::Message
.includes(in_reply_to: [:user, chat_webhook_event: [:incoming_chat_webhook]])
.includes(:revisions)
.includes(user: :primary_group)
.includes(chat_webhook_event: :incoming_chat_webhook)
.includes(reactions: :user)
.includes(:bookmarks)
.includes(:uploads)
.includes(chat_channel: :chatable)
.includes(:thread)
.where(chat_channel_id: channel.id)
query = query.includes(user: :user_status) if SiteSetting.enable_user_status
query
end
def self.query_around_target(target_message_id, channel, messages)
target_message = base_query(channel: channel).with_deleted.find_by(id: target_message_id)
past_messages =
messages
.where("created_at < ?", target_message.created_at)
.order(created_at: :desc)
.limit(PAST_MESSAGE_LIMIT)
.to_a
future_messages =
messages
.where("created_at > ?", target_message.created_at)
.order(created_at: :asc)
.limit(FUTURE_MESSAGE_LIMIT)
.to_a
can_load_more_past = past_messages.size == PAST_MESSAGE_LIMIT
can_load_more_future = future_messages.size == FUTURE_MESSAGE_LIMIT
{
past_messages: past_messages,
future_messages: future_messages,
target_message: target_message,
can_load_more_past: can_load_more_past,
can_load_more_future: can_load_more_future,
}
end
def self.query_paginated_messages(direction, page_size, channel, messages)
order = direction == FUTURE ? "ASC" : "DESC"
messages = messages.order("created_at #{order}, id #{order}").limit(page_size).to_a
if direction == FUTURE
can_load_more_future = messages.size == page_size
elsif direction == PAST
can_load_more_past = messages.size == page_size
else
# When direction is blank, we'll return the latest messages.
can_load_more_future = false
can_load_more_past = messages.size == page_size
end
{
messages: direction == FUTURE ? messages : messages.reverse,
can_load_more_past: can_load_more_past,
can_load_more_future: can_load_more_future,
}
end
end
end

View File

@ -2,7 +2,7 @@
module Chat
##
# Handles counting unread messages and mentions scoped to threads for a list
# Handles counting unread messages scoped to threads for a list
# of channels. A list of thread IDs can be provided to further focus the query.
# Alternatively, a list of thread IDs can be provided by itself to only get
# specific threads regardless of channel.
@ -25,8 +25,17 @@ module Chat
# @param user_id [Integer] The ID of the user to count for.
# @param include_missing_memberships [Boolean] Whether to include threads
# that the user is not a member of. These counts will always be 0.
def self.call(channel_ids: [], thread_ids: [], user_id:, include_missing_memberships: false)
return [] if channel_ids.empty? && thread_ids.empty?
# @param include_read [Boolean] Whether to include threads that the user
# is a member of where they have read all the messages. This overrides
# include_missing_memberships.
def self.call(
channel_ids: nil,
thread_ids: nil,
user_id:,
include_missing_memberships: false,
include_read: true
)
return [] if channel_ids.blank? && thread_ids.blank?
sql = <<~SQL
SELECT (
@ -56,7 +65,14 @@ module Chat
#{include_missing_memberships ? "" : "LIMIT :limit"}
SQL
sql += <<~SQL if include_missing_memberships
sql = <<~SQL if !include_read
SELECT * FROM (
#{sql}
) AS thread_tracking
WHERE (unread_count > 0 OR mention_count > 0)
SQL
sql += <<~SQL if include_missing_memberships && include_read
UNION ALL
SELECT 0 AS unread_count, 0 AS mention_count, chat_threads.channel_id, chat_threads.id AS thread_id
FROM chat_channels

View File

@ -0,0 +1,82 @@
# frozen_string_literal: true
module Chat
# This class is responsible for querying the user's current tracking
# (read/unread) state based on membership for one or more channels
# and/or one or more threads.
#
# Only channels with threading_enabled set to true will have thread
# tracking queried.
#
# @param guardian [Guardian] The current user's guardian
# @param channel_ids [Array<Integer>] The channel IDs to query. Must be provided
# if thread_ids are not.
# @param thread_ids [Array<Integer>] The thread IDs to query. Must be provided
# if channel_ids are not. If channel_ids are also provided then these just further
# filter results.
# @param include_missing_memberships [Boolean] If true, will include channels
# and threads where the user does not have a UserChatXMembership record,
# with zeroed out unread counts.
# @param include_threads [Boolean] If true, will include thread tracking
# state in the query, otherwise only channel tracking will be queried.
# @param include_read [Boolean] If true, will include tracking state where
# the user has 0 unread messages. If false, will only include tracking state
# where the user has > 0 unread messages. If include_missing_memberships is
# also true, this overrides that option.
class TrackingStateReportQuery
def self.call(
guardian:,
channel_ids: nil,
thread_ids: nil,
include_missing_memberships: false,
include_threads: false,
include_read: true
)
report = ::Chat::TrackingStateReport.new
if channel_ids.blank?
report.channel_tracking = {}
else
report.channel_tracking =
::Chat::ChannelUnreadsQuery
.call(
channel_ids: channel_ids,
user_id: guardian.user.id,
include_missing_memberships: include_missing_memberships,
include_read: include_read,
)
.map do |ct|
[ct.channel_id, { mention_count: ct.mention_count, unread_count: ct.unread_count }]
end
.to_h
end
if !include_threads || (thread_ids.blank? && channel_ids.blank?)
report.thread_tracking = {}
else
report.thread_tracking =
::Chat::ThreadUnreadsQuery
.call(
channel_ids: channel_ids,
thread_ids: thread_ids,
user_id: guardian.user.id,
include_missing_memberships: include_missing_memberships,
include_read: include_read,
)
.map do |tt|
[
tt.thread_id,
{
channel_id: tt.channel_id,
mention_count: tt.mention_count,
unread_count: tt.unread_count,
},
]
end
.to_h
end
report
end
end
end

View File

@ -2,7 +2,41 @@
module Chat
class ViewSerializer < ApplicationSerializer
attributes :meta, :chat_messages
attributes :meta, :chat_messages, :threads, :tracking, :thread_tracking_overview, :channel
def threads
return [] if !object.threads
ActiveModel::ArraySerializer.new(
object.threads,
each_serializer: Chat::ThreadSerializer,
scope: scope,
)
end
def tracking
object.tracking || {}
end
def thread_tracking_overview
object.thread_tracking_overview || []
end
def include_threads?
include_thread_data?
end
def include_thread_tracking_overview?
include_thread_data?
end
def include_thread_data?
channel.threading_enabled && SiteSetting.enable_experimental_chat_threaded_discussions
end
def channel
object.chat_channel
end
def chat_messages
ActiveModel::ArraySerializer.new(

View File

@ -0,0 +1,195 @@
# frozen_string_literal: true
module Chat
# Builds up a Chat::View object for a channel, and handles several
# different querying scenraios:
#
# * Fetching messages before and after a specific target_message_id,
# or fetching paginated messages.
# * Fetching threads for the found messages.
# * Fetching thread tracking state.
# * Fetching an overview of unread threads for the channel.
#
# @example
# Chat::ChannelViewBuilder.call(channel_id: 2, guardian: guardian, **optional_params)
#
class ChannelViewBuilder
include Service::Base
# @!method call(channel_id:, guardian:)
# @param [Integer] channel_id
# @param [Guardian] guardian
# @option optional_params [Integer] thread_id
# @option optional_params [Integer] target_message_id
# @option optional_params [Integer] page_size
# @option optional_params [String] direction
# @return [Service::Base::Context]
contract
model :channel
policy :can_view_channel
policy :target_message_exists
step :determine_threads_enabled
step :determine_include_thread_messages
step :fetch_messages
step :fetch_thread_tracking_overview
step :fetch_threads_for_messages
step :fetch_tracking
step :build_view
class Contract
attribute :channel_id, :integer
# If this is not present, then we just fetch messages with page_size
# and direction.
attribute :target_message_id, :integer # (optional)
attribute :thread_id, :integer # (optional)
attribute :direction, :string # (optional)
attribute :page_size, :integer # (optional)
validates :channel_id, presence: true
validates :direction,
inclusion: {
in: Chat::MessagesQuery::VALID_DIRECTIONS,
},
allow_nil: true
end
private
def fetch_channel(contract:, **)
Chat::Channel.includes(:chatable).find_by(id: contract.channel_id)
end
def can_view_channel(guardian:, channel:, **)
guardian.can_preview_chat_channel?(channel)
end
def target_message_exists(contract:, **)
return true if contract.target_message_id.blank?
Chat::Message.exists?(id: contract.target_message_id)
end
def determine_threads_enabled(channel:, **)
context.threads_enabled =
SiteSetting.enable_experimental_chat_threaded_discussions && channel.threading_enabled
end
def determine_include_thread_messages(contract:, threads_enabled:, **)
context.include_thread_messages = contract.thread_id.present? || !threads_enabled
end
def fetch_messages(channel:, guardian:, contract:, include_thread_messages:, **)
messages_data =
::Chat::MessagesQuery.call(
channel: channel,
guardian: guardian,
target_message_id: contract.target_message_id,
thread_id: contract.thread_id,
include_thread_messages: include_thread_messages,
page_size: contract.page_size,
direction: contract.direction,
)
context.can_load_more_past = messages_data[:can_load_more_past]
context.can_load_more_future = messages_data[:can_load_more_future]
if !messages_data[:target_message]
context.messages = messages_data[:messages]
else
messages_data[:target_message] = (
if !include_thread_messages && messages_data[:target_message].thread_reply?
[]
else
[messages_data[:target_message]]
end
)
context.messages = [
messages_data[:past_messages].reverse,
messages_data[:target_message],
messages_data[:future_messages],
].reduce([], :concat)
end
end
# The thread tracking overview is a simple array of thread IDs
# that have unread messages, only threads with unread messages
# will be included in this array. This is a low-cost way to know
# how many threads the user has unread across the entire channel.
def fetch_thread_tracking_overview(guardian:, channel:, threads_enabled:, **)
if !threads_enabled
context.thread_tracking_overview = []
else
context.thread_tracking_overview =
::Chat::TrackingStateReportQuery
.call(
guardian: guardian,
channel_ids: [channel.id],
include_threads: true,
include_read: false,
)
.find_channel_threads(channel.id)
.keys
end
end
def fetch_threads_for_messages(guardian:, messages:, channel:, threads_enabled:, **)
if !threads_enabled
context.threads = []
else
context.threads =
::Chat::Thread.includes(original_message_user: :user_status).where(
id: messages.map(&:thread_id).compact.uniq,
)
# Saves us having to load the same message we already have.
context.threads.each do |thread|
thread.original_message =
messages.find { |message| message.id == thread.original_message_id }
end
end
end
# Only thread tracking is necessary to fetch here -- we preload
# channel tracking state for all the current user's tracked channels
# in the CurrentUserSerializer.
def fetch_tracking(guardian:, messages:, channel:, threads_enabled:, **)
thread_ids = messages.map(&:thread_id).compact.uniq
if !threads_enabled || thread_ids.empty?
context.tracking = {}
else
context.tracking =
::Chat::TrackingStateReportQuery.call(
guardian: guardian,
thread_ids: thread_ids,
include_threads: true,
)
end
end
def build_view(
guardian:,
channel:,
messages:,
threads:,
tracking:,
thread_tracking_overview:,
can_load_more_past:,
can_load_more_future:,
**
)
context.view =
Chat::View.new(
chat_channel: channel,
chat_messages: messages,
user: guardian.user,
can_load_more_past: can_load_more_past,
can_load_more_future: can_load_more_future,
thread_tracking_overview: thread_tracking_overview,
threads: threads,
tracking: tracking,
)
end
end
end

View File

@ -1,46 +1,6 @@
# frozen_string_literal: true
module Chat
class TrackingStateReport
attr_accessor :channel_tracking, :thread_tracking
class TrackingStateInfo
attr_accessor :unread_count, :mention_count
def initialize(info)
@unread_count = info[:unread_count]
@mention_count = info[:mention_count]
end
def to_hash
to_h
end
def to_h
{ unread_count: unread_count, mention_count: mention_count }
end
end
def initialize
@channel_tracking = {}
@thread_tracking = {}
end
def find_channel(channel_id)
TrackingStateInfo.new(channel_tracking[channel_id])
end
def find_thread(thread_id)
TrackingStateInfo.new(thread_tracking[thread_id])
end
def find_channel_threads(channel_id)
thread_tracking
.select { |_, thread| thread[:channel_id] == channel_id }
.map { |_, thread| TrackingStateInfo.new(thread) }
end
end
# Produces the current tracking state for a user for one or more
# chat channels. This can be further filtered by providing one or
# more thread IDs for the channel.
@ -84,6 +44,7 @@ module Chat
attribute :thread_ids, default: []
attribute :include_missing_memberships, default: false
attribute :include_threads, default: false
attribute :include_read, default: true
end
private
@ -99,53 +60,14 @@ module Chat
end
def fetch_report(contract:, guardian:, **)
report = TrackingStateReport.new
if contract.channel_ids.empty?
report.channel_tracking = {}
else
report.channel_tracking =
::Chat::ChannelUnreadsQuery
.call(
channel_ids: contract.channel_ids,
user_id: guardian.user.id,
include_missing_memberships: contract.include_missing_memberships,
)
.map do |ct|
[ct.channel_id, { mention_count: ct.mention_count, unread_count: ct.unread_count }]
end
.to_h
end
if contract.include_threads
if contract.thread_ids.empty? && contract.channel_ids.empty?
report.thread_tracking = {}
else
report.thread_tracking =
::Chat::ThreadUnreadsQuery
.call(
channel_ids: contract.channel_ids,
thread_ids: contract.thread_ids,
user_id: guardian.user.id,
include_missing_memberships: contract.include_missing_memberships,
)
.map do |tt|
[
tt.thread_id,
{
channel_id: tt.channel_id,
mention_count: tt.mention_count,
unread_count: tt.unread_count,
},
]
end
.to_h
end
else
report.thread_tracking = {}
end
report
::Chat::TrackingStateReportQuery.call(
guardian: guardian,
channel_ids: contract.channel_ids,
thread_ids: contract.thread_ids,
include_missing_memberships: contract.include_missing_memberships,
include_threads: contract.include_threads,
include_read: contract.include_read,
)
end
end
end

View File

@ -157,28 +157,25 @@ export default class ChatLivePane extends Component {
this.loadingMorePast = true;
const findArgs = { pageSize: PAGE_SIZE };
const findArgs = { pageSize: PAGE_SIZE, includeMessages: true };
const fetchingFromLastRead = !options.fetchFromLastMessage;
if (this.requestedTargetMessageId) {
findArgs["targetMessageId"] = this.requestedTargetMessageId;
findArgs.targetMessageId = this.requestedTargetMessageId;
} else if (fetchingFromLastRead) {
findArgs["targetMessageId"] =
findArgs.targetMessageId =
this.args.channel.currentUserMembership.lastReadMessageId;
}
return this.chatApi
.messages(this.args.channel.id, findArgs)
.then((results) => {
if (
this._selfDeleted ||
this.args.channel.id !== results.meta.channel_id
) {
.channel(this.args.channel.id, findArgs)
.then((result) => {
if (this._selfDeleted || this.args.channel.id !== result.channel.id) {
return;
}
const [messages, meta] = this.afterFetchCallback(
this.args.channel,
results
result
);
this.args.channel.addMessages(messages);
@ -201,6 +198,18 @@ export default class ChatLivePane extends Component {
}
this.scrollToBottom();
if (result.threads) {
result.threads.forEach((thread) => {
this.args.channel.threadsManager.store(this.args.channel, thread);
});
}
if (result.thread_tracking_overview) {
this.args.channel.threadTrackingOverview.push(
...result.thread_tracking_overview
);
}
})
.catch(this._handleErrors)
.finally(() => {
@ -355,7 +364,9 @@ export default class ChatLivePane extends Component {
const message = ChatMessage.create(channel, messageData);
if (messageData.thread_id) {
message.thread = new ChatThread(channel, { id: messageData.thread_id });
message.thread = ChatThread.create(channel, {
id: messageData.thread_id,
});
}
messages.push(message);

View File

@ -1,4 +1,5 @@
import UserChatChannelMembership from "discourse/plugins/chat/discourse/models/user-chat-channel-membership";
import { TrackedArray } from "@ember-compat/tracked-built-ins";
import { ajax } from "discourse/lib/ajax";
import { escapeExpression } from "discourse/lib/utilities";
import { tracked } from "@glimmer/tracking";
@ -90,6 +91,7 @@ export default class ChatChannel {
@tracked archive;
@tracked tracking;
@tracked threadingEnabled = false;
@tracked threadTrackingOverview = new TrackedArray();
threadsManager = new ChatThreadsManager(getOwner(this));
messagesManager = new ChatMessagesManager(getOwner(this));

View File

@ -22,10 +22,31 @@ export default class ChatApi extends Service {
*
* this.chatApi.channel(1).then(channel => { ... })
*/
channel(channelId) {
return this.#getRequest(`/channels/${channelId}`).then((result) =>
this.chatChannelsManager.store(result.channel)
);
channel(channelId, data = {}) {
const args = {};
if (data.targetMessageId) {
args.target_message_id = data.targetMessageId;
} else {
args.page_size = data.pageSize;
if (data.includeMessages) {
args.include_messages = true;
}
if (data.messageId) {
args.target_message_id = data.messageId;
}
if (data.threadId) {
args.thread_id = data.threadId;
}
}
return this.#getRequest(`/channels/${channelId}`, args).then((result) => {
this.chatChannelsManager.store(result.channel);
return result;
});
}
/**

View File

@ -119,9 +119,8 @@ export default class ChatChannelsManager extends Service {
return this.chatApi
.channel(id)
.catch(popupAjaxError)
.then((channel) => {
this.#cache(channel);
return channel;
.then((result) => {
return this.store(result.channel);
});
}

View File

@ -5,6 +5,9 @@ require "rails_helper"
describe Chat::ChannelUnreadsQuery do
fab!(:channel_1) { Fabricate(:category_channel) }
fab!(:current_user) { Fabricate(:user) }
let(:include_missing_memberships) { false }
let(:include_read) { true }
let(:channel_ids) { [channel_1.id] }
before do
SiteSetting.chat_enabled = true
@ -12,13 +15,20 @@ describe Chat::ChannelUnreadsQuery do
channel_1.add(current_user)
end
let(:subject) do
described_class.call(
channel_ids: channel_ids,
user_id: current_user.id,
include_missing_memberships: include_missing_memberships,
include_read: include_read,
).map(&:to_h)
end
context "with unread message" do
before { Fabricate(:chat_message, chat_channel: channel_1) }
it "returns a correct unread count" do
expect(
described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
).to eq({ mention_count: 0, unread_count: 1, channel_id: channel_1.id })
expect(subject.first).to eq({ mention_count: 0, unread_count: 1, channel_id: channel_1.id })
end
context "when the membership has been muted" do
@ -30,9 +40,7 @@ describe Chat::ChannelUnreadsQuery do
end
it "returns a zeroed unread count" do
expect(
described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
).to eq({ mention_count: 0, unread_count: 0, channel_id: channel_1.id })
expect(subject.first).to eq({ mention_count: 0, unread_count: 0, channel_id: channel_1.id })
end
end
@ -41,22 +49,19 @@ describe Chat::ChannelUnreadsQuery do
fab!(:thread) { Fabricate(:chat_thread, channel: channel_1, original_message: thread_om) }
it "does include the original message in the unread count" do
expect(
described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
).to eq({ mention_count: 0, unread_count: 2, channel_id: channel_1.id })
expect(subject.first).to eq({ mention_count: 0, unread_count: 2, channel_id: channel_1.id })
end
it "does not include other thread messages in the unread count" do
Fabricate(:chat_message, chat_channel: channel_1, thread: thread)
Fabricate(:chat_message, chat_channel: channel_1, thread: thread)
expect(
described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
).to eq({ mention_count: 0, unread_count: 2, channel_id: channel_1.id })
expect(subject.first).to eq({ mention_count: 0, unread_count: 2, channel_id: channel_1.id })
end
end
context "for multiple channels" do
fab!(:channel_2) { Fabricate(:category_channel) }
let(:channel_ids) { [channel_1.id, channel_2.id] }
before do
channel_2.add(current_user)
@ -65,12 +70,7 @@ describe Chat::ChannelUnreadsQuery do
end
it "returns accurate counts" do
expect(
described_class.call(
channel_ids: [channel_1.id, channel_2.id],
user_id: current_user.id,
).map(&:to_h),
).to match_array(
expect(subject).to match_array(
[
{ mention_count: 0, unread_count: 1, channel_id: channel_1.id },
{ mention_count: 0, unread_count: 2, channel_id: channel_2.id },
@ -87,29 +87,32 @@ describe Chat::ChannelUnreadsQuery do
end
it "does not return counts for the channels" do
expect(
described_class.call(
channel_ids: [channel_1.id, channel_2.id],
user_id: current_user.id,
).map(&:to_h),
).to match_array([{ mention_count: 0, unread_count: 1, channel_id: channel_1.id }])
expect(subject).to match_array(
[{ mention_count: 0, unread_count: 1, channel_id: channel_1.id }],
)
end
context "when include_missing_memberships is true" do
let(:include_missing_memberships) { true }
it "does return zeroed counts for the channels" do
expect(
described_class.call(
channel_ids: [channel_1.id, channel_2.id],
user_id: current_user.id,
include_missing_memberships: true,
).map(&:to_h),
).to match_array(
expect(subject).to match_array(
[
{ mention_count: 0, unread_count: 1, channel_id: channel_1.id },
{ mention_count: 0, unread_count: 0, channel_id: channel_2.id },
],
)
end
context "when include_read is false" do
let(:include_read) { false }
it "does not return counts for the channels" do
expect(subject).to match_array(
[{ mention_count: 0, unread_count: 1, channel_id: channel_1.id }],
)
end
end
end
end
end
@ -132,9 +135,7 @@ describe Chat::ChannelUnreadsQuery do
message = Fabricate(:chat_message, chat_channel: channel_1)
create_mention(message, channel_1)
expect(
described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
).to eq({ mention_count: 1, unread_count: 1, channel_id: channel_1.id })
expect(subject.first).to eq({ mention_count: 1, unread_count: 1, channel_id: channel_1.id })
end
context "for unread mentions in a thread" do
@ -143,9 +144,7 @@ describe Chat::ChannelUnreadsQuery do
it "does include the original message in the mention count" do
create_mention(thread_om, channel_1)
expect(
described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
).to eq({ mention_count: 1, unread_count: 1, channel_id: channel_1.id })
expect(subject.first).to eq({ mention_count: 1, unread_count: 1, channel_id: channel_1.id })
end
it "does not include other thread messages in the mention count" do
@ -153,14 +152,13 @@ describe Chat::ChannelUnreadsQuery do
thread_message_2 = Fabricate(:chat_message, chat_channel: channel_1, thread: thread)
create_mention(thread_message_1, channel_1)
create_mention(thread_message_2, channel_1)
expect(
described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
).to eq({ mention_count: 0, unread_count: 1, channel_id: channel_1.id })
expect(subject.first).to eq({ mention_count: 0, unread_count: 1, channel_id: channel_1.id })
end
end
context "for multiple channels" do
fab!(:channel_2) { Fabricate(:category_channel) }
let(:channel_ids) { [channel_1.id, channel_2.id] }
it "returns accurate counts" do
message = Fabricate(:chat_message, chat_channel: channel_1)
@ -171,12 +169,7 @@ describe Chat::ChannelUnreadsQuery do
message_2 = Fabricate(:chat_message, chat_channel: channel_2)
create_mention(message_2, channel_2)
expect(
described_class.call(
channel_ids: [channel_1.id, channel_2.id],
user_id: current_user.id,
).map(&:to_h),
).to match_array(
expect(subject).to match_array(
[
{ mention_count: 1, unread_count: 1, channel_id: channel_1.id },
{ mention_count: 1, unread_count: 2, channel_id: channel_2.id },
@ -188,9 +181,15 @@ describe Chat::ChannelUnreadsQuery do
context "with nothing unread" do
it "returns a correct state" do
expect(
described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
).to eq({ mention_count: 0, unread_count: 0, channel_id: channel_1.id })
expect(subject.first).to eq({ mention_count: 0, unread_count: 0, channel_id: channel_1.id })
end
context "when include_read is false" do
let(:include_read) { false }
it "returns nothing" do
expect(subject).to eq([])
end
end
end
end

View File

@ -0,0 +1,165 @@
# frozen_string_literal: true
RSpec.describe Chat::MessagesQuery do
fab!(:channel) { Fabricate(:category_channel) }
fab!(:current_user) { Fabricate(:user) }
let(:include_thread_messages) { false }
let(:thread_id) { nil }
let(:page_size) { nil }
let(:direction) { nil }
let(:target_message_id) { nil }
let(:options) do
{
thread_id: thread_id,
include_thread_messages: include_thread_messages,
page_size: page_size,
direction: direction,
target_message_id: target_message_id,
}
end
let(:subject) do
described_class.call(guardian: current_user.guardian, channel: channel, **options)
end
fab!(:message_1) do
message = Fabricate(:chat_message, chat_channel: channel)
message.update!(created_at: 2.days.ago)
message
end
fab!(:message_2) do
message = Fabricate(:chat_message, chat_channel: channel)
message.update!(created_at: 6.hours.ago)
message
end
fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel) }
context "when target_message_id provided" do
let(:target_message) { message_2 }
let(:target_message_id) { target_message.id }
it "queries messages in the channel and finds the past and future messages" do
expect(subject).to eq(
past_messages: [message_1],
future_messages: [message_3],
target_message: target_message,
can_load_more_past: false,
can_load_more_future: false,
)
end
it "does not include deleted messages" do
message_3.trash!
expect(subject[:future_messages]).to eq([])
end
it "still includes the target message if it is deleted" do
target_message.trash!
expect(subject[:target_message]).to eq(target_message)
end
it "can_load_more_past is true when the past messages reach the limit" do
stub_const(described_class, "PAST_MESSAGE_LIMIT", 1) do
expect(subject[:can_load_more_past]).to be_truthy
end
end
it "can_load_more_future is true when the future messages reach the limit" do
stub_const(described_class, "FUTURE_MESSAGE_LIMIT", 1) do
expect(subject[:can_load_more_future]).to be_truthy
end
end
describe "when some messages are in threads" do
fab!(:thread) { Fabricate(:chat_thread, channel: channel) }
it "does not include messages which are thread replies but does include thread original messages" do
message_3.update!(thread: thread)
expect(subject[:future_messages]).to eq([thread.original_message])
end
context "when include_thread_messages is true" do
let(:include_thread_messages) { true }
it "does include messages which are part of a thread" do
message_3.update!(
thread: thread,
created_at: thread.original_message.created_at + 1.minute,
)
expect(subject[:future_messages]).to eq([thread.original_message, message_3])
end
end
context "when thread_id is provided" do
let(:thread_id) { thread.id }
it "does include messages which are part of a thread" do
message_3.update!(
thread: thread,
created_at: thread.original_message.created_at + 1.minute,
)
expect(subject[:future_messages]).to eq([thread.original_message, message_3])
end
end
end
context "when the user can moderate chat" do
before { current_user.update!(admin: true) }
it "does include deleted messages" do
message_3.trash!
expect(subject[:future_messages]).to eq([message_3])
end
end
end
context "when target_message_id not provided" do
it "queries messages in the channel" do
expect(subject).to eq(
messages: [message_1, message_2, message_3],
can_load_more_past: false,
can_load_more_future: false,
)
end
context "when the messages length is equal to the page_size" do
let(:page_size) { 3 }
it "can_load_more_past is true" do
expect(subject[:can_load_more_past]).to be_truthy
end
end
context "when direction is future" do
let(:direction) { described_class::FUTURE }
it "returns messages in ascending order by created_at" do
expect(subject[:messages]).to eq([message_1, message_2, message_3])
end
context "when the messages length is equal to the page_size" do
let(:page_size) { 3 }
it "can_load_more_future is true" do
expect(subject[:can_load_more_future]).to be_truthy
end
end
end
context "when direction is past" do
let(:direction) { described_class::PAST }
it "returns messages in ascending order by created_at" do
expect(subject[:messages]).to eq([message_1, message_2, message_3])
end
context "when the messages length is equal to the page_size" do
let(:page_size) { 3 }
it "can_load_more_past is true" do
expect(subject[:can_load_more_past]).to be_truthy
end
end
end
end
end

View File

@ -13,6 +13,7 @@ describe Chat::ThreadUnreadsQuery do
let(:params) { { user_id: current_user.id, channel_ids: channel_ids, thread_ids: thread_ids } }
let(:include_missing_memberships) { false }
let(:include_read) { true }
let(:channel_ids) { [] }
let(:thread_ids) { [] }
let(:subject) do
@ -21,6 +22,7 @@ describe Chat::ThreadUnreadsQuery do
thread_ids: thread_ids,
user_id: current_user.id,
include_missing_memberships: include_missing_memberships,
include_read: include_read,
)
end
@ -89,6 +91,23 @@ describe Chat::ThreadUnreadsQuery do
{ channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 },
)
end
context "when include_read is false" do
let(:include_read) { false }
it "does not get threads with no unread messages" do
expect(subject.map(&:to_h)).not_to include(
[
{
channel_id: channel_1.id,
mention_count: 0,
thread_id: thread_2.id,
unread_count: 0,
},
],
)
end
end
end
context "when only the thread_ids are provided" do
@ -155,6 +174,23 @@ describe Chat::ThreadUnreadsQuery do
],
)
end
context "when include_read is false" do
let(:include_read) { false }
it "does not include the thread that the user is not a member of with zeroed out counts" do
expect(subject.map(&:to_h)).to match_array(
[
{
channel_id: channel_2.id,
mention_count: 0,
thread_id: thread_3.id,
unread_count: 1,
},
],
)
end
end
end
end
end

View File

@ -0,0 +1,143 @@
# frozen_string_literal: true
RSpec.describe Chat::TrackingStateReportQuery do
fab!(:current_user) { Fabricate(:user) }
let(:guardian) { current_user.guardian }
let(:channel_ids) { [] }
let(:thread_ids) { [] }
let(:include_missing_memberships) { false }
let(:include_threads) { false }
let(:include_read) { true }
let(:subject) do
described_class.call(
guardian: guardian,
channel_ids: channel_ids,
thread_ids: thread_ids,
include_missing_memberships: include_missing_memberships,
include_threads: include_threads,
include_read: include_read,
)
end
context "when channel_ids empty" do
it "returns empty object for channel_tracking" do
expect(subject.channel_tracking).to eq({})
end
end
context "when channel_ids provided" do
fab!(:channel_1) { Fabricate(:category_channel) }
fab!(:channel_2) { Fabricate(:category_channel) }
let(:channel_ids) { [channel_1.id, channel_2.id] }
it "calls the channel unreads query with the corect params" do
Chat::ChannelUnreadsQuery
.expects(:call)
.with(
channel_ids: channel_ids,
user_id: current_user.id,
include_missing_memberships: include_missing_memberships,
include_read: include_read,
)
.returns([])
subject
end
it "generates a correct unread report for the channels the user is a member of" do
channel_1.add(current_user)
channel_2.add(current_user)
Fabricate(:chat_message, chat_channel: channel_1)
Fabricate(:chat_message, chat_channel: channel_2)
expect(subject.channel_tracking).to eq(
{
channel_1.id => {
unread_count: 1,
mention_count: 0,
},
channel_2.id => {
unread_count: 1,
mention_count: 0,
},
},
)
end
it "does not include threads by default" do
Chat::ThreadUnreadsQuery.expects(:call).never
expect(subject.thread_tracking).to eq({})
end
context "when include_threads is true" do
let(:include_threads) { true }
fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1) }
fab!(:thread_2) { Fabricate(:chat_thread, channel: channel_2) }
before do
channel_1.update!(threading_enabled: true)
channel_2.update!(threading_enabled: true)
end
it "calls the thread unreads query with the corect params" do
Chat::ThreadUnreadsQuery
.expects(:call)
.with(
channel_ids: channel_ids,
thread_ids: thread_ids,
user_id: current_user.id,
include_missing_memberships: include_missing_memberships,
include_read: include_read,
)
.returns([])
subject
end
it "generates a correct unread for the threads the user is a member of in the channels" do
channel_1.add(current_user)
channel_2.add(current_user)
thread_1.add(current_user)
thread_2.add(current_user)
Fabricate(:chat_message, chat_channel: channel_1, thread: thread_1)
Fabricate(:chat_message, chat_channel: channel_2, thread: thread_2)
expect(subject.channel_tracking).to eq(
{
channel_1.id => {
unread_count: 1,
mention_count: 0,
},
channel_2.id => {
unread_count: 1,
mention_count: 0,
},
},
)
expect(subject.thread_tracking).to eq(
{
thread_1.id => {
unread_count: 1,
mention_count: 0,
channel_id: channel_1.id,
},
thread_2.id => {
unread_count: 1,
mention_count: 0,
channel_id: channel_2.id,
},
},
)
end
context "when thread_ids and channel_ids is empty" do
let(:thread_ids) { [] }
let(:channel_ids) { [] }
it "does not query threads" do
Chat::ThreadUnreadsQuery.expects(:call).never
expect(subject.thread_tracking).to eq({})
end
end
end
end
end

View File

@ -0,0 +1,234 @@
# frozen_string_literal: true
RSpec.describe Chat::ChannelViewBuilder do
describe Chat::ChannelViewBuilder::Contract, type: :model do
it { is_expected.to validate_presence_of :channel_id }
it do
is_expected.to validate_inclusion_of(
:direction,
).in_array Chat::MessagesQuery::VALID_DIRECTIONS
end
end
describe ".call" do
fab!(:current_user) { Fabricate(:user) }
fab!(:channel) { Fabricate(:category_channel) }
let(:channel_id) { channel.id }
let(:guardian) { current_user.guardian }
let(:target_message_id) { nil }
let(:page_size) { nil }
let(:direction) { nil }
let(:thread_id) { nil }
let(:params) do
{
guardian: guardian,
channel_id: channel_id,
target_message_id: target_message_id,
page_size: page_size,
direction: direction,
thread_id: thread_id,
}
end
subject(:result) { described_class.call(params) }
it "threads_enabled is false by default" do
expect(subject.threads_enabled).to eq(false)
end
it "include_thread_messages is true by default" do
expect(subject.include_thread_messages).to eq(true)
end
it "queries messages" do
Chat::MessagesQuery
.expects(:call)
.with(
channel: channel,
guardian: guardian,
target_message_id: target_message_id,
thread_id: thread_id,
include_thread_messages: true,
page_size: page_size,
direction: direction,
)
.returns({ messages: [] })
subject
end
it "returns channel messages and thread replies" do
message_1 = Fabricate(:chat_message, chat_channel: channel)
message_2 = Fabricate(:chat_message, chat_channel: channel)
message_3 =
Fabricate(
:chat_message,
chat_channel: channel,
thread: Fabricate(:chat_thread, channel: channel),
)
expect(subject.view.chat_messages).to eq(
[message_1, message_2, message_3.thread.original_message, message_3],
)
end
it "does not query thread tracking overview or state by default" do
Chat::TrackingStateReportQuery.expects(:call).never
subject
end
it "does not query threads by default" do
Chat::Thread.expects(:where).never
subject
end
it "returns a Chat::View" do
expect(subject.view).to be_a(Chat::View)
end
context "when channel has threading_enabled and enable_experimental_chat_threaded_discussions is true" do
before do
channel.update!(threading_enabled: true)
SiteSetting.enable_experimental_chat_threaded_discussions = true
end
it "threads_enabled is true" do
expect(subject.threads_enabled).to eq(true)
end
it "include_thread_messages is false" do
expect(subject.include_thread_messages).to eq(false)
end
it "returns channel messages but not thread replies" do
message_1 = Fabricate(:chat_message, chat_channel: channel)
message_2 = Fabricate(:chat_message, chat_channel: channel)
message_3 =
Fabricate(
:chat_message,
chat_channel: channel,
thread: Fabricate(:chat_thread, channel: channel),
)
expect(subject.view.chat_messages).to eq(
[message_1, message_2, message_3.thread.original_message],
)
end
it "fetches threads for any messages that have a thread id" do
message_1 =
Fabricate(
:chat_message,
chat_channel: channel,
thread: Fabricate(:chat_thread, channel: channel),
)
expect(subject.view.threads).to eq([message_1.thread])
end
it "calls the tracking state report query for thread overview and tracking" do
thread = Fabricate(:chat_thread, channel: channel)
message_1 = Fabricate(:chat_message, chat_channel: channel, thread: thread)
::Chat::TrackingStateReportQuery
.expects(:call)
.with(
guardian: guardian,
channel_ids: [channel.id],
include_threads: true,
include_read: false,
)
.returns(Chat::TrackingStateReport.new)
.once
::Chat::TrackingStateReportQuery
.expects(:call)
.with(guardian: guardian, thread_ids: [thread.id], include_threads: true)
.returns(Chat::TrackingStateReport.new)
.once
subject
end
it "fetches an overview of threads with unread messages in the channel" do
thread = Fabricate(:chat_thread, channel: channel)
thread.add(current_user)
message_1 = Fabricate(:chat_message, chat_channel: channel, thread: thread)
expect(subject.view.thread_tracking_overview).to eq([message_1.thread.id])
end
it "fetches the tracking state of threads in the channel" do
thread = Fabricate(:chat_thread, channel: channel)
thread.add(current_user)
Fabricate(:chat_message, chat_channel: channel, thread: thread)
expect(subject.view.tracking.thread_tracking).to eq(
{ thread.id => { channel_id: channel.id, unread_count: 1, mention_count: 0 } },
)
end
context "when a thread_id is provided" do
let(:thread_id) { Fabricate(:chat_thread, channel: channel).id }
it "include_thread_messages is true" do
expect(subject.include_thread_messages).to eq(true)
end
end
end
context "when channel is not found" do
before { channel.destroy! }
it { is_expected.to fail_to_find_a_model(:channel) }
end
context "when user cannot access the channel" do
fab!(:channel) { Fabricate(:private_category_channel) }
it { is_expected.to fail_a_policy(:can_view_channel) }
end
context "when target_message_id provided" do
fab!(:message) { Fabricate(:chat_message, chat_channel: channel) }
fab!(:past_message) do
msg = Fabricate(:chat_message, chat_channel: channel)
msg.update!(created_at: message.created_at - 1.day)
msg
end
fab!(:future_message) do
msg = Fabricate(:chat_message, chat_channel: channel)
msg.update!(created_at: message.created_at + 1.day)
msg
end
let(:target_message_id) { message.id }
it "includes the target message as well as past and future messages" do
expect(subject.view.chat_messages).to eq([past_message, message, future_message])
end
context "when the target message is a thread reply" do
fab!(:thread) { Fabricate(:chat_thread, channel: channel) }
before { message.update!(thread: thread) }
it "includes it by default" do
expect(subject.view.chat_messages).to eq(
[past_message, message, thread.original_message, future_message],
)
end
context "when not including thread messages" do
before do
channel.update!(threading_enabled: true)
SiteSetting.enable_experimental_chat_threaded_discussions = true
end
it "does not include the target message" do
expect(subject.view.chat_messages).to eq(
[past_message, thread.original_message, future_message],
)
end
end
end
context "when the message does not exist" do
before { message.trash! }
it { is_expected.to fail_a_policy(:target_message_exists) }
end
end
end
end