DEV: Remove old ChatController routes for messages & lookup (#21723)

Since 5cce829 and the new
channel view builder, we have no need of these obsolete
routes which have way too much logic in the controller, which
has been superseded by the view builder anyway.

Remove the routes and update the channel message loading to use it.
This commit is contained in:
Martin Brennan 2023-05-29 09:37:10 +02:00 committed by GitHub
parent 7a9514922b
commit 6ddd17a756
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 486 additions and 498 deletions

View File

@ -12,14 +12,13 @@ module Chat
# these endpoints require a standalone find because they need to be
# able to get deleted channels and recover them.
before_action :find_chatable, only: %i[enable_chat disable_chat]
before_action :find_chat_message, only: %i[lookup_message edit_message rebake message_link]
before_action :find_chat_message, only: %i[edit_message rebake message_link]
before_action :set_channel_and_chatable_with_access_check,
except: %i[
respond
enable_chat
disable_chat
message_link
lookup_message
set_user_chat_status
dismiss_retention_reminder
flag
@ -169,59 +168,6 @@ 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
message_id = params[:message_id]
if page_size > 100 ||
(
message_id.blank? ^ direction.blank? &&
(direction.present? && !CHAT_DIRECTIONS.include?(direction))
)
raise Discourse::InvalidParameters
end
messages = preloaded_chat_message_query.where(chat_channel: @chat_channel)
messages = messages.with_deleted if guardian.can_moderate_chat?(@chatable)
messages = messages.where(thread_id: params[:thread_id]) if params[:thread_id]
messages = exclude_thread_messages(messages) if !include_thread_messages?
if message_id.present?
condition = direction == PAST ? "<" : ">"
messages = messages.where("id #{condition} ?", message_id.to_i)
end
# NOTE: This order is reversed when we return the Chat::View below if the direction
# is not FUTURE.
order = direction == FUTURE ? "ASC" : "DESC"
messages = messages.order("created_at #{order}, id #{order}").limit(page_size).to_a
can_load_more_past = nil
can_load_more_future = nil
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
chat_view =
Chat::View.new(
chat_channel: @chat_channel,
chat_messages: direction == FUTURE ? messages : messages.reverse,
user: current_user,
can_load_more_past: can_load_more_past,
can_load_more_future: can_load_more_future,
)
render_serialized(chat_view, Chat::ViewSerializer, root: false)
end
def react
params.require(%i[message_id emoji react_action])
guardian.ensure_can_react!
@ -252,43 +198,6 @@ 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)
messages = preloaded_chat_message_query.where(chat_channel: @chat_channel)
messages = messages.with_deleted if guardian.can_moderate_chat?(@chatable)
messages = messages.where(thread_id: params[:thread_id]) if params[:thread_id]
messages = exclude_thread_messages(messages) if !include_thread_messages?
past_messages =
messages
.where("created_at < ?", @message.created_at)
.order(created_at: :desc)
.limit(PAST_MESSAGE_LIMIT)
future_messages =
messages
.where("created_at > ?", @message.created_at)
.order(created_at: :asc)
.limit(FUTURE_MESSAGE_LIMIT)
can_load_more_past = past_messages.count == PAST_MESSAGE_LIMIT
can_load_more_future = future_messages.count == FUTURE_MESSAGE_LIMIT
looked_up_message = !include_thread_messages? && @message.thread_reply? ? [] : [@message]
messages = [past_messages.reverse, looked_up_message, future_messages].reduce([], :concat)
chat_view =
Chat::View.new(
chat_channel: @chat_channel,
chat_messages: messages,
user: current_user,
can_load_more_past: can_load_more_past,
can_load_more_future: can_load_more_future,
)
render_serialized(chat_view, Chat::ViewSerializer, root: false)
end
def set_user_chat_status
params.require(:chat_enabled)
@ -423,11 +332,6 @@ module Chat
query
end
def include_thread_messages?
params[:thread_id].present? || !SiteSetting.enable_experimental_chat_threaded_discussions ||
!@chat_channel.threading_enabled
end
def find_chatable
@chatable = Category.find_by(id: params[:chatable_id])
guardian.ensure_can_moderate_chat!(@chatable)
@ -441,15 +345,5 @@ module Chat
@message = @message.find_by(id: params[:message_id])
raise Discourse::NotFound unless @message
end
def exclude_thread_messages(messages)
messages.where(<<~SQL, channel_id: @chat_channel.id)
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
end
end
end

View File

@ -21,6 +21,7 @@ module Chat
PAST = "past"
FUTURE = "future"
VALID_DIRECTIONS = [PAST, FUTURE]
MAX_PAGE_SIZE = 100
# @param channel [Chat::Channel] The channel to query messages within.
# @param guardian [Guardian] The guardian to use for permission checks.
@ -57,10 +58,10 @@ module Chat
)
SQL
if target_message_id.present?
if target_message_id.present? && direction.blank?
query_around_target(target_message_id, channel, messages)
else
query_paginated_messages(direction, page_size, channel, messages)
query_paginated_messages(direction, page_size, channel, messages, target_message_id)
end
end
@ -117,7 +118,18 @@ module Chat
}
end
def self.query_paginated_messages(direction, page_size, channel, messages)
def self.query_paginated_messages(
direction,
page_size,
channel,
messages,
target_message_id = nil
)
if target_message_id.present?
condition = direction == PAST ? "<" : ">"
messages = messages.where("id #{condition} ?", target_message_id.to_i)
end
order = direction == FUTURE ? "ASC" : "DESC"
messages = messages.order("created_at #{order}, id #{order}").limit(page_size).to_a

View File

@ -19,7 +19,7 @@ module Chat
@opts = opts
# Avoids an N1 to re-load the thread in the serializer for original_message.
object.original_message.thread = object
object.original_message&.thread = object
@current_user_membership = opts[:membership]
end

View File

@ -57,6 +57,18 @@ module Chat
in: Chat::MessagesQuery::VALID_DIRECTIONS,
},
allow_nil: true
validates :page_size,
numericality: {
less_than_or_equal_to: Chat::MessagesQuery::MAX_PAGE_SIZE,
only_integer: true,
},
allow_nil: true
validate :page_size_present, if: -> { target_message_id.blank? && !fetch_from_last_read }
def page_size_present
errors.add(:page_size, :blank) if page_size.blank?
end
end
private
@ -77,7 +89,11 @@ module Chat
def target_message_exists(contract:, guardian:, **)
return true if contract.target_message_id.blank?
target_message = Chat::Message.unscoped.find_by(id: contract.target_message_id)
target_message =
Chat::Message.with_deleted.find_by(
id: contract.target_message_id,
chat_channel_id: contract.channel_id,
)
return false if target_message.blank?
return true if !target_message.trashed?
target_message.user_id == guardian.user.id || guardian.is_staff?

View File

@ -266,11 +266,11 @@ export default class ChatLivePane extends Component {
};
return this.chatApi
.messages(this.args.channel.id, findArgs)
.then((results) => {
.channel(this.args.channel.id, findArgs)
.then((result) => {
if (
this._selfDeleted ||
this.args.channel.id !== results.meta.channel_id ||
this.args.channel.id !== result.meta.channel_id ||
!this.scrollable
) {
return;
@ -284,14 +284,29 @@ export default class ChatLivePane extends Component {
const [messages, meta] = this.afterFetchCallback(
this.args.channel,
results
result
);
if (result.threads) {
result.threads.forEach((thread) => {
this.args.channel.threadsManager.store(this.args.channel, thread);
});
}
if (result.thread_tracking_overview) {
result.thread_tracking_overview.forEach((threadId) => {
if (!this.args.channel.threadTrackingOverview.includes(threadId)) {
this.args.channel.threadTrackingOverview.push(threadId);
}
});
}
this.args.channel.details = meta;
if (!messages?.length) {
return;
}
this.args.channel.details = meta;
this.args.channel.addMessages(messages);
// Edge case for IOS to avoid blank screens
@ -331,11 +346,11 @@ export default class ChatLivePane extends Component {
}
@bind
afterFetchCallback(channel, results) {
afterFetchCallback(channel, result) {
const messages = [];
let foundFirstNew = false;
results.chat_messages.forEach((messageData, index) => {
result.chat_messages.forEach((messageData, index) => {
if (index === 0) {
messageData.firstOfResults = true;
}
@ -378,7 +393,7 @@ export default class ChatLivePane extends Component {
messages.push(message);
});
return [messages, results.meta];
return [messages, result.meta];
}
@debounce(100)

View File

@ -154,22 +154,22 @@ export default class ChatThreadPanel extends Component {
this.loading = true;
const findArgs = { pageSize: PAGE_SIZE, threadId: this.thread.id };
const findArgs = {
pageSize: PAGE_SIZE,
threadId: this.thread.id,
includeMessages: true,
};
return this.chatApi
.messages(this.channel.id, findArgs)
.then((results) => {
if (this._selfDeleted || this.channel.id !== results.meta.channel_id) {
this.router.transitionTo(
"chat.channel",
"-",
results.meta.channel_id
);
.channel(this.channel.id, findArgs)
.then((result) => {
if (this._selfDeleted || this.channel.id !== result.meta.channel_id) {
this.router.transitionTo("chat.channel", "-", result.meta.channel_id);
}
const [messages, meta] = this.afterFetchCallback(
this.channel,
this.thread,
results
result
);
this.thread.messagesManager.addMessages(messages);
this.thread.details = meta;
@ -186,10 +186,10 @@ export default class ChatThreadPanel extends Component {
}
@bind
afterFetchCallback(channel, thread, results) {
afterFetchCallback(channel, thread, result) {
const messages = [];
results.chat_messages.forEach((messageData) => {
result.chat_messages.forEach((messageData) => {
// If a message has been hidden it is because the current user is ignoring
// the user who sent it, so we want to unconditionally hide it, even if
// we are going directly to the target
@ -205,7 +205,7 @@ export default class ChatThreadPanel extends Component {
messages.push(message);
});
return [messages, results.meta];
return [messages, result.meta];
}
// NOTE: At some point we want to do this based on visible messages

View File

@ -32,6 +32,10 @@ export default class ChatApi extends Service {
} else {
args.page_size = data.pageSize;
if (data.direction) {
args.direction = data.direction;
}
if (data.includeMessages) {
args.include_messages = true;
}
@ -273,43 +277,6 @@ export default class ChatApi extends Service {
);
}
/**
* Returns messages of a channel, from the last message or a specified target.
* @param {number} channelId - The ID of the channel.
* @param {object} data - Params of the query.
* @param {integer} data.targetMessageId - ID of the targeted message.
* @param {integer} data.messageId - ID of the targeted message.
* @param {integer} data.direction - Fetch past or future messages.
* @param {integer} data.pageSize - Max number of messages to fetch.
* @returns {Promise}
*/
messages(channelId, data = {}) {
let path;
const args = {};
if (data.targetMessageId) {
path = `/chat/lookup/${data.targetMessageId}`;
args.chat_channel_id = channelId;
} else {
args.page_size = data.pageSize;
path = `/chat/${channelId}/messages`;
if (data.messageId) {
args.message_id = data.messageId;
}
if (data.direction) {
args.direction = data.direction;
}
if (data.threadId) {
args.thread_id = data.threadId;
}
}
return ajax(path, { data: args });
}
/**
* Update notifications settings of current user for a channel.
* @param {number} channelId - The ID of the channel.

View File

@ -58,14 +58,12 @@ Chat::Engine.routes.draw do
post "/enable" => "chat#enable_chat"
post "/disable" => "chat#disable_chat"
post "/dismiss-retention-reminder" => "chat#dismiss_retention_reminder"
get "/:chat_channel_id/messages" => "chat#messages"
get "/message/:message_id" => "chat#message_link"
put ":chat_channel_id/edit/:message_id" => "chat#edit_message"
put ":chat_channel_id/react/:message_id" => "chat#react"
put "/:chat_channel_id/:message_id/rebake" => "chat#rebake"
post "/:chat_channel_id/:message_id/flag" => "chat#flag"
post "/:chat_channel_id/quote" => "chat#quote_messages"
get "/lookup/:message_id" => "chat#lookup_message"
put "/:chat_channel_id/read/:message_id" => "chat#update_user_last_read"
put "/user_chat_enabled/:user_id" => "chat#set_user_chat_status"
put "/:chat_channel_id/invite" => "chat#invite_users"

View File

@ -159,6 +159,395 @@ RSpec.describe Chat::Api::ChannelsController do
end
end
end
context "when include_messages is true" do
fab!(:current_user) { Fabricate(:user) }
fab!(:channel_1) { Fabricate(:category_channel) }
fab!(:other_user) { Fabricate(:user) }
describe "target message lookup" do
let!(:message) { Fabricate(:chat_message, chat_channel: channel_1) }
let(:chatable) { channel_1.chatable }
before { sign_in(current_user) }
context "when the message doesnt belong to the channel" do
let!(:message) { Fabricate(:chat_message) }
it "returns a 404" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
target_message_id: message.id,
include_messages: true,
}
expect(response.status).to eq(404)
end
end
context "when the chat channel is for a category" do
it "ensures the user can access that category" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
target_message_id: message.id,
include_messages: true,
}
expect(response.status).to eq(200)
expect(response.parsed_body["chat_messages"][0]["id"]).to eq(message.id)
group = Fabricate(:group)
chatable.update!(read_restricted: true)
Fabricate(:category_group, group: group, category: chatable)
get "/chat/api/channels/#{channel_1.id}.json",
params: {
target_message_id: message.id,
include_messages: true,
}
expect(response.status).to eq(403)
GroupUser.create!(user: current_user, group: group)
get "/chat/api/channels/#{channel_1.id}.json",
params: {
target_message_id: message.id,
include_messages: true,
}
expect(response.status).to eq(200)
expect(response.parsed_body["chat_messages"][0]["id"]).to eq(message.id)
end
end
context "when the chat channel is for a direct message channel" do
let(:channel_1) { Fabricate(:direct_message_channel) }
it "ensures the user can access that direct message channel" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
target_message_id: message.id,
include_messages: true,
}
expect(response.status).to eq(403)
Chat::DirectMessageUser.create!(user: current_user, direct_message: chatable)
get "/chat/api/channels/#{channel_1.id}.json",
params: {
target_message_id: message.id,
include_messages: true,
}
expect(response.status).to eq(200)
expect(response.parsed_body["chat_messages"][0]["id"]).to eq(message.id)
end
end
end
describe "messages pagination and direction" do
let(:page_size) { 30 }
message_count = 70
message_count.times do |n|
fab!("message_#{n}") do
Fabricate(
:chat_message,
chat_channel: channel_1,
user: other_user,
message: "message #{n}",
)
end
end
before do
sign_in(current_user)
Group.refresh_automatic_groups!
end
it "errors for user when they are not allowed to chat" do
SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:staff]
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.status).to eq(403)
end
it "errors when page size is over the maximum" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: Chat::MessagesQuery::MAX_PAGE_SIZE + 1,
}
expect(response.status).to eq(400)
expect(response.parsed_body["errors"]).to include(
"Page size must be less than or equal to #{Chat::MessagesQuery::MAX_PAGE_SIZE}",
)
end
it "errors when page size is nil" do
get "/chat/api/channels/#{channel_1.id}.json", params: { include_messages: true }
expect(response.status).to eq(400)
expect(response.parsed_body["errors"]).to include("Page size can't be blank")
end
it "returns the latest messages in created_at, id order" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
messages = response.parsed_body["chat_messages"]
expect(messages.count).to eq(page_size)
expect(messages.first["created_at"].to_time).to be < messages.last["created_at"].to_time
end
it "returns `can_flag=true` for public channels" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_flag"]).to be true
end
it "returns `can_flag=true` for DM channels" do
dm_chat_channel = Fabricate(:direct_message_channel, users: [current_user, other_user])
get "/chat/api/channels/#{dm_chat_channel.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_flag"]).to be true
end
it "returns `can_moderate=true` based on whether the user can moderate the chatable" do
1.upto(4) do |n|
current_user.update!(trust_level: n)
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_moderate"]).to be false
end
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_moderate"]).to be false
current_user.update!(admin: true)
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_moderate"]).to be true
current_user.update!(admin: false)
SiteSetting.enable_category_group_moderation = true
group = Fabricate(:group)
group.add(current_user)
channel_1.category.update!(reviewable_by_group: group)
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_moderate"]).to be true
end
it "serializes `user_flag_status` for user who has a pending flag" do
chat_message = channel_1.chat_messages.last
reviewable = flag_message(chat_message, current_user)
score = reviewable.reviewable_scores.last
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["chat_messages"].last["user_flag_status"]).to eq(
score.status_for_database,
)
end
it "doesn't serialize `reviewable_ids` for non-staff" do
reviewable = flag_message(channel_1.chat_messages.last, Fabricate(:admin))
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["chat_messages"].last["reviewable_id"]).to be_nil
end
it "serializes `reviewable_ids` correctly for staff" do
admin = Fabricate(:admin)
sign_in(admin)
reviewable = flag_message(channel_1.chat_messages.last, admin)
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["chat_messages"].last["reviewable_id"]).to eq(reviewable.id)
end
it "correctly marks reactions as 'reacted' for the current_user" do
heart_emoji = ":heart:"
smile_emoji = ":smile"
last_message = channel_1.chat_messages.last
last_message.reactions.create(user: current_user, emoji: heart_emoji)
last_message.reactions.create(user: Fabricate(:admin), emoji: smile_emoji)
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
reactions = response.parsed_body["chat_messages"].last["reactions"]
heart_reaction = reactions.find { |r| r["emoji"] == heart_emoji }
expect(heart_reaction["reacted"]).to be true
smile_reaction = reactions.find { |r| r["emoji"] == smile_emoji }
expect(smile_reaction["reacted"]).to be false
end
it "sends the last message bus id for the channel" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
page_size: page_size,
}
expect(response.parsed_body["meta"]["channel_message_bus_last_id"]).not_to eq(nil)
end
describe "scrolling to the past" do
it "returns the correct messages in created_at, id order" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
target_message_id: message_40.id,
page_size: page_size,
direction: Chat::MessagesQuery::PAST,
}
messages = response.parsed_body["chat_messages"]
expect(messages.count).to eq(page_size)
expect(messages.first["created_at"].to_time).to eq_time(message_10.created_at)
expect(messages.last["created_at"].to_time).to eq_time(message_39.created_at)
end
it "returns 'can_load...' properly when there are more past messages" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
target_message_id: message_40.id,
page_size: page_size,
direction: Chat::MessagesQuery::PAST,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to be true
expect(response.parsed_body["meta"]["can_load_more_future"]).to be_nil
end
it "returns 'can_load...' properly when there are no past messages" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
target_message_id: message_3.id,
page_size: page_size,
direction: Chat::MessagesQuery::PAST,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to be false
expect(response.parsed_body["meta"]["can_load_more_future"]).to be_nil
end
end
describe "scrolling to the future" do
it "returns the correct messages in created_at, id order when there are many after" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
target_message_id: message_10.id,
page_size: page_size,
direction: Chat::MessagesQuery::FUTURE,
}
messages = response.parsed_body["chat_messages"]
expect(messages.count).to eq(page_size)
expect(messages.first["created_at"].to_time).to eq_time(message_11.created_at)
expect(messages.last["created_at"].to_time).to eq_time(message_40.created_at)
end
it "return 'can_load..' properly when there are future messages" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
target_message_id: message_10.id,
page_size: page_size,
direction: Chat::MessagesQuery::FUTURE,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to be_nil
expect(response.parsed_body["meta"]["can_load_more_future"]).to be true
end
it "returns 'can_load..' properly when there are no future messages" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
include_messages: true,
target_message_id: message_60.id,
page_size: page_size,
direction: Chat::MessagesQuery::FUTURE,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to be_nil
expect(response.parsed_body["meta"]["can_load_more_future"]).to be false
end
end
describe "without direction (latest messages)" do
it "signals there are no future messages" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
page_size: page_size,
include_messages: true,
}
expect(response.parsed_body["meta"]["can_load_more_future"]).to eq(false)
end
it "signals there are more messages in the past" do
get "/chat/api/channels/#{channel_1.id}.json",
params: {
page_size: page_size,
include_messages: true,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to eq(true)
end
it "signals there are no more messages" do
new_channel = Fabricate(:category_channel)
Fabricate(
:chat_message,
chat_channel: new_channel,
user: other_user,
message: "message",
)
chat_messages_qty = 1
get "/chat/api/channels/#{new_channel.id}.json",
params: {
page_size: chat_messages_qty + 1,
include_messages: true,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to eq(false)
end
end
end
end
end
describe "#destroy" do
@ -569,4 +958,8 @@ RSpec.describe Chat::Api::ChannelsController do
end
end
end
def flag_message(message, flagger, flag_type: ReviewableScore.types[:off_topic])
Chat::ReviewQueue.new.flag_message(message, Guardian.new(flagger), flag_type)[:reviewable]
end
end

View File

@ -32,272 +32,6 @@ RSpec.describe Chat::ChatController do
Chat::ReviewQueue.new.flag_message(message, Guardian.new(flagger), flag_type)[:reviewable]
end
describe "#messages" do
let(:page_size) { 30 }
before do
sign_in(user)
Group.refresh_automatic_groups!
end
it "errors for user when they are not allowed to chat" do
SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:staff]
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.status).to eq(403)
end
it "errors when page size is over 100" do
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: 200 }
expect(response.status).to eq(400)
end
it "errors when page size is nil" do
get "/chat/#{chat_channel.id}/messages.json"
expect(response.status).to eq(400)
end
it "returns the latest messages in created_at, id order" do
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
messages = response.parsed_body["chat_messages"]
expect(messages.count).to eq(page_size)
expect(messages.first["created_at"].to_time).to be < messages.last["created_at"].to_time
end
it "returns `can_flag=true` for public channels" do
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["meta"]["can_flag"]).to be true
end
it "returns `can_flag=true` for DM channels" do
get "/chat/#{dm_chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["meta"]["can_flag"]).to be true
end
it "returns `can_moderate=true` based on whether the user can moderate the chatable" do
1.upto(4) do |n|
user.update!(trust_level: n)
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["meta"]["can_moderate"]).to be false
end
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["meta"]["can_moderate"]).to be false
user.update!(admin: true)
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["meta"]["can_moderate"]).to be true
user.update!(admin: false)
SiteSetting.enable_category_group_moderation = true
group = Fabricate(:group)
group.add(user)
category.update!(reviewable_by_group: group)
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["meta"]["can_moderate"]).to be true
end
it "serializes `user_flag_status` for user who has a pending flag" do
chat_message = chat_channel.chat_messages.last
reviewable = flag_message(chat_message, user)
score = reviewable.reviewable_scores.last
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["chat_messages"].last["user_flag_status"]).to eq(
score.status_for_database,
)
end
it "doesn't serialize `reviewable_ids` for non-staff" do
reviewable = flag_message(chat_channel.chat_messages.last, admin)
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["chat_messages"].last["reviewable_id"]).to be_nil
end
it "serializes `reviewable_ids` correctly for staff" do
sign_in(admin)
reviewable = flag_message(chat_channel.chat_messages.last, admin)
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["chat_messages"].last["reviewable_id"]).to eq(reviewable.id)
end
it "correctly marks reactions as 'reacted' for the current_user" do
heart_emoji = ":heart:"
smile_emoji = ":smile"
last_message = chat_channel.chat_messages.last
last_message.reactions.create(user: user, emoji: heart_emoji)
last_message.reactions.create(user: admin, emoji: smile_emoji)
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
reactions = response.parsed_body["chat_messages"].last["reactions"]
heart_reaction = reactions.find { |r| r["emoji"] == heart_emoji }
expect(heart_reaction["reacted"]).to be true
smile_reaction = reactions.find { |r| r["emoji"] == smile_emoji }
expect(smile_reaction["reacted"]).to be false
end
it "sends the last message bus id for the channel" do
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["meta"]["channel_message_bus_last_id"]).not_to eq(nil)
end
context "with mentions" do
it "returns mentioned users" do
last_message = chat_channel.chat_messages.last
user1 = Fabricate(:user)
user2 = Fabricate(:user)
Fabricate(:chat_mention, user: user1, chat_message: last_message)
Fabricate(:chat_mention, user: user2, chat_message: last_message)
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
mentioned_users = response.parsed_body["chat_messages"].last["mentioned_users"]
expect(mentioned_users[0]["id"]).to eq(user1.id)
expect(mentioned_users[0]["username"]).to eq(user1.username)
expect(mentioned_users[1]["id"]).to eq(user2.id)
expect(mentioned_users[1]["username"]).to eq(user2.username)
end
it "returns an empty list if no one was mentioned" do
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
last_message = response.parsed_body["chat_messages"].last
expect(last_message).to have_key("mentioned_users")
expect(last_message["mentioned_users"]).to be_empty
end
context "with user status" do
fab!(:status) { Fabricate(:user_status) }
fab!(:user1) { Fabricate(:user, user_status: status) }
fab!(:chat_mention) do
Fabricate(:chat_mention, user: user1, chat_message: chat_channel.chat_messages.last)
end
it "returns status if enabled in settings" do
SiteSetting.enable_user_status = true
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
mentioned_user = response.parsed_body["chat_messages"].last["mentioned_users"][0]
expect(mentioned_user).to have_key("status")
expect(mentioned_user["status"]["emoji"]).to eq(status.emoji)
expect(mentioned_user["status"]["description"]).to eq(status.description)
end
it "doesn't return status if disabled in settings" do
SiteSetting.enable_user_status = false
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
mentioned_user = response.parsed_body["chat_messages"].last["mentioned_users"][0]
expect(mentioned_user).not_to have_key("status")
end
end
end
describe "scrolling to the past" do
it "returns the correct messages in created_at, id order" do
get "/chat/#{chat_channel.id}/messages.json",
params: {
message_id: message_40.id,
direction: described_class::PAST,
page_size: page_size,
}
messages = response.parsed_body["chat_messages"]
expect(messages.count).to eq(page_size)
expect(messages.first["created_at"].to_time).to eq_time(message_10.created_at)
expect(messages.last["created_at"].to_time).to eq_time(message_39.created_at)
end
it "returns 'can_load...' properly when there are more past messages" do
get "/chat/#{chat_channel.id}/messages.json",
params: {
message_id: message_40.id,
direction: described_class::PAST,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to be true
expect(response.parsed_body["meta"]["can_load_more_future"]).to be_nil
end
it "returns 'can_load...' properly when there are no past messages" do
get "/chat/#{chat_channel.id}/messages.json",
params: {
message_id: message_3.id,
direction: described_class::PAST,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to be false
expect(response.parsed_body["meta"]["can_load_more_future"]).to be_nil
end
end
describe "scrolling to the future" do
it "returns the correct messages in created_at, id order when there are many after" do
get "/chat/#{chat_channel.id}/messages.json",
params: {
message_id: message_10.id,
direction: described_class::FUTURE,
page_size: page_size,
}
messages = response.parsed_body["chat_messages"]
expect(messages.count).to eq(page_size)
expect(messages.first["created_at"].to_time).to eq_time(message_11.created_at)
expect(messages.last["created_at"].to_time).to eq_time(message_40.created_at)
end
it "return 'can_load..' properly when there are future messages" do
get "/chat/#{chat_channel.id}/messages.json",
params: {
message_id: message_10.id,
direction: described_class::FUTURE,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to be_nil
expect(response.parsed_body["meta"]["can_load_more_future"]).to be true
end
it "returns 'can_load..' properly when there are no future messages" do
get "/chat/#{chat_channel.id}/messages.json",
params: {
message_id: message_60.id,
direction: described_class::FUTURE,
page_size: page_size,
}
expect(response.parsed_body["meta"]["can_load_more_past"]).to be_nil
expect(response.parsed_body["meta"]["can_load_more_future"]).to be false
end
end
describe "without direction (latest messages)" do
it "signals there are no future messages" do
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["meta"]["can_load_more_future"]).to eq(false)
end
it "signals there are more messages in the past" do
get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
expect(response.parsed_body["meta"]["can_load_more_past"]).to eq(true)
end
it "signals there are no more messages" do
new_channel = Fabricate(:category_channel)
Fabricate(:chat_message, chat_channel: new_channel, user: other_user, message: "message")
chat_messages_qty = 1
get "/chat/#{new_channel.id}/messages.json", params: { page_size: chat_messages_qty + 1 }
expect(response.parsed_body["meta"]["can_load_more_past"]).to eq(false)
end
end
end
describe "#enable_chat" do
context "with category as chatable" do
let!(:category) { Fabricate(:category) }
@ -1193,63 +927,4 @@ RSpec.describe Chat::ChatController do
get "/chat/message/#{message.id}.json"
end
end
describe "#lookup_message" do
let!(:message) { Fabricate(:chat_message, chat_channel: channel) }
let(:channel) { Fabricate(:direct_message_channel) }
let(:chatable) { channel.chatable }
fab!(:user) { Fabricate(:user) }
before { sign_in(user) }
it "ensures message's channel can be seen" do
Guardian.any_instance.expects(:can_join_chat_channel?).with(channel)
get "/chat/lookup/#{message.id}.json", params: { chat_channel_id: channel.id }
end
context "when the message doesnt belong to the channel" do
let!(:message) { Fabricate(:chat_message) }
it "returns a 404" do
get "/chat/lookup/#{message.id}.json", params: { chat_channel_id: channel.id }
expect(response.status).to eq(404)
end
end
context "when the chat channel is for a category" do
let(:channel) { Fabricate(:category_channel) }
it "ensures the user can access that category" do
get "/chat/lookup/#{message.id}.json", params: { chat_channel_id: channel.id }
expect(response.status).to eq(200)
expect(response.parsed_body["chat_messages"][0]["id"]).to eq(message.id)
group = Fabricate(:group)
chatable.update!(read_restricted: true)
Fabricate(:category_group, group: group, category: chatable)
get "/chat/lookup/#{message.id}.json", params: { chat_channel_id: channel.id }
expect(response.status).to eq(403)
GroupUser.create!(user: user, group: group)
get "/chat/lookup/#{message.id}.json", params: { chat_channel_id: channel.id }
expect(response.status).to eq(200)
expect(response.parsed_body["chat_messages"][0]["id"]).to eq(message.id)
end
end
context "when the chat channel is for a direct message channel" do
let(:channel) { Fabricate(:direct_message_channel) }
it "ensures the user can access that direct message channel" do
get "/chat/lookup/#{message.id}.json", params: { chat_channel_id: channel.id }
expect(response.status).to eq(403)
Chat::DirectMessageUser.create!(user: user, direct_message: chatable)
get "/chat/lookup/#{message.id}.json", params: { chat_channel_id: channel.id }
expect(response.status).to eq(200)
expect(response.parsed_body["chat_messages"][0]["id"]).to eq(message.id)
end
end
end
end

View File

@ -17,7 +17,7 @@ RSpec.describe Chat::ChannelViewBuilder do
let(:channel_id) { channel.id }
let(:guardian) { current_user.guardian }
let(:target_message_id) { nil }
let(:page_size) { nil }
let(:page_size) { 10 }
let(:direction) { nil }
let(:thread_id) { nil }
let(:fetch_from_last_read) { nil }
@ -87,6 +87,12 @@ RSpec.describe Chat::ChannelViewBuilder do
expect(subject.view).to be_a(Chat::View)
end
context "when page_size is null" do
let(:page_size) { nil }
it { is_expected.to fail_a_contract }
end
context "when channel has threading_enabled and enable_experimental_chat_threaded_discussions is true" do
before do
channel.update!(threading_enabled: true)
@ -210,6 +216,12 @@ RSpec.describe Chat::ChannelViewBuilder do
msg
end
context "when page_size is null" do
let(:page_size) { nil }
it { is_expected.not_to fail_a_contract }
end
context "if the user is not a member of the channel" do
it "does not error and still returns messages" do
expect(subject.view.chat_messages).to eq([past_message_2, past_message_1, message])
@ -257,6 +269,12 @@ RSpec.describe Chat::ChannelViewBuilder do
expect(subject.view.chat_messages).to eq([past_message, message, future_message])
end
context "when page_size is null" do
let(:page_size) { nil }
it { is_expected.not_to fail_a_contract }
end
context "when the target message is a thread reply" do
fab!(:thread) { Fabricate(:chat_thread, channel: channel) }