PERF: cook message in background (#24227)

This commit starts from a simple observation: cooking messages on the hot path can be slow. Especially with a lot of mentions.

To move cooking from the hot path, this commit has made the following changes:

- updating cooked, inserting mentions and notifying user of new mentions has been moved inside the `process_message` job. It happens right after the `Chat::MessageProcessor` run, which is where the cooking happens.
- the similar existing code in `rebake!` has also been moved to rely on the `process_message`job only
- refactored `create_mentions` and `update_mentions` into one single `upsert_mentions` which can be called invariably
- allows services to decide if their job is ran inline or later. It avoids to need to know you have to use `Jobs.run_immediately!` in this case, in tests it will be inline per default
- made various frontend changes to make the chat-channel component lifecycle clearer. we had to handle `did-update @channel` which was super awkward and creating bugs with listeners which the changes of the PR made clear in failing specs
- adds a new `-processed` (and `-not-processed`) class on the chat message, this is made to have a good lifecyle hook in system specs
This commit is contained in:
Joffrey JAFFEUX 2023-11-06 15:45:30 +01:00 committed by GitHub
parent 4425e99bf9
commit 90efdd7f9d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
46 changed files with 388 additions and 394 deletions

View File

@ -34,7 +34,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController
Chat::MessageRateLimiter.run!(current_user) Chat::MessageRateLimiter.run!(current_user)
with_service(Chat::CreateMessage) do with_service(Chat::CreateMessage) do
on_success { render json: success_json.merge(message_id: result[:message].id) } on_success { render json: success_json.merge(message_id: result[:message_instance].id) }
on_failed_policy(:no_silenced_user) { raise Discourse::InvalidAccess } on_failed_policy(:no_silenced_user) { raise Discourse::InvalidAccess }
on_model_not_found(:channel) { raise Discourse::NotFound } on_model_not_found(:channel) { raise Discourse::NotFound }
on_failed_policy(:allowed_to_join_channel) { raise Discourse::InvalidAccess } on_failed_policy(:allowed_to_join_channel) { raise Discourse::InvalidAccess }
@ -49,7 +49,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController
on_failed_policy(:ensure_thread_matches_parent) do on_failed_policy(:ensure_thread_matches_parent) do
render_json_error(I18n.t("chat.errors.thread_does_not_match_parent")) render_json_error(I18n.t("chat.errors.thread_does_not_match_parent"))
end end
on_model_errors(:message) do |model| on_model_errors(:message_instance) do |model|
render_json_error(model.errors.map(&:full_message).join(", ")) render_json_error(model.errors.map(&:full_message).join(", "))
end end
end end

View File

@ -80,7 +80,7 @@ module Chat
on_failed_policy(:ensure_thread_matches_parent) do on_failed_policy(:ensure_thread_matches_parent) do
render_json_error(I18n.t("chat.errors.thread_does_not_match_parent")) render_json_error(I18n.t("chat.errors.thread_does_not_match_parent"))
end end
on_model_errors(:message) do |model| on_model_errors(:message_instance) do |model|
render_json_error(model.errors.map(&:full_message).join(", ")) render_json_error(model.errors.map(&:full_message).join(", "))
end end
end end

View File

@ -10,14 +10,44 @@ module Jobs
) do ) do
chat_message = ::Chat::Message.find_by(id: args[:chat_message_id]) chat_message = ::Chat::Message.find_by(id: args[:chat_message_id])
return if !chat_message return if !chat_message
processor = ::Chat::MessageProcessor.new(chat_message) processor =
::Chat::MessageProcessor.new(
chat_message,
{ invalidate_oneboxes: args[:invalidate_oneboxes] },
)
processor.run! processor.run!
if args[:is_dirty] || processor.dirty? if processor.dirty?
chat_message.update( chat_message.update!(
cooked: processor.html, cooked: processor.html,
cooked_version: ::Chat::Message::BAKED_VERSION, cooked_version: ::Chat::Message::BAKED_VERSION,
) )
chat_message.upsert_mentions
if args[:edit_timestamp]
::Chat::Publisher.publish_edit!(chat_message.chat_channel, chat_message)
::Chat::Notifier.new(chat_message, args[:edit_timestamp]).notify_edit
DiscourseEvent.trigger(
:chat_message_edited,
chat_message,
chat_message.chat_channel,
chat_message.user,
)
else
::Chat::Publisher.publish_new!(
chat_message.chat_channel,
chat_message,
args[:staged_id],
)
::Chat::Notifier.new(chat_message, chat_message.created_at).notify_new
DiscourseEvent.trigger(
:chat_message_created,
chat_message,
chat_message.chat_channel,
chat_message.user,
)
end
::Chat::Publisher.publish_processed!(chat_message) ::Chat::Publisher.publish_processed!(chat_message)
end end
end end

View File

@ -156,19 +156,8 @@ module Chat
def rebake!(invalidate_oneboxes: false, priority: nil) def rebake!(invalidate_oneboxes: false, priority: nil)
ensure_last_editor_id ensure_last_editor_id
args = { chat_message_id: self.id, invalidate_oneboxes: invalidate_oneboxes }
previous_cooked = self.cooked
new_cooked =
self.class.cook(
message,
invalidate_oneboxes: invalidate_oneboxes,
user_id: self.last_editor_id,
)
update_columns(cooked: new_cooked, cooked_version: BAKED_VERSION)
args = { chat_message_id: self.id }
args[:queue] = priority.to_s if priority && priority != :normal args[:queue] = priority.to_s if priority && priority != :normal
args[:is_dirty] = true if previous_cooked != new_cooked
Jobs.enqueue(Jobs::Chat::ProcessMessage, args) Jobs.enqueue(Jobs::Chat::ProcessMessage, args)
end end
@ -258,19 +247,14 @@ module Chat
end end
end end
def create_mentions def upsert_mentions
insert_mentions(parsed_mentions.all_mentioned_users_ids)
end
def update_mentions
mentioned_user_ids = parsed_mentions.all_mentioned_users_ids mentioned_user_ids = parsed_mentions.all_mentioned_users_ids
old_mentions = chat_mentions.pluck(:user_id) old_mentions = chat_mentions.pluck(:user_id)
updated_mentions = mentioned_user_ids
mentioned_user_ids_to_drop = old_mentions - updated_mentions
mentioned_user_ids_to_add = updated_mentions - old_mentions
mentioned_user_ids_to_drop = old_mentions - mentioned_user_ids
delete_mentions(mentioned_user_ids_to_drop) delete_mentions(mentioned_user_ids_to_drop)
mentioned_user_ids_to_add = mentioned_user_ids - old_mentions
insert_mentions(mentioned_user_ids_to_add) insert_mentions(mentioned_user_ids_to_add)
end end

View File

@ -32,7 +32,8 @@ module Chat
policy :ensure_valid_thread_for_channel policy :ensure_valid_thread_for_channel
policy :ensure_thread_matches_parent policy :ensure_thread_matches_parent
model :uploads, optional: true model :uploads, optional: true
model :message, :instantiate_message model :message_instance, :instantiate_message
transaction do transaction do
step :save_message step :save_message
step :delete_drafts step :delete_drafts
@ -54,6 +55,7 @@ module Chat
attribute :upload_ids, :array attribute :upload_ids, :array
attribute :thread_id, :string attribute :thread_id, :string
attribute :incoming_chat_webhook attribute :incoming_chat_webhook
attribute :process_inline, :boolean, default: Rails.env.test?
validates :chat_channel_id, presence: true validates :chat_channel_id, presence: true
validates :message, presence: true, if: -> { upload_ids.blank? } validates :message, presence: true, if: -> { upload_ids.blank? }
@ -127,38 +129,38 @@ module Chat
) )
end end
def save_message(message:, **) def save_message(message_instance:, **)
message.cook message_instance.save!
message.save!
message.create_mentions
end end
def delete_drafts(channel:, guardian:, **) def delete_drafts(channel:, guardian:, **)
Chat::Draft.where(user: guardian.user, chat_channel: channel).destroy_all Chat::Draft.where(user: guardian.user, chat_channel: channel).destroy_all
end end
def post_process_thread(thread:, message:, guardian:, **) def post_process_thread(thread:, message_instance:, guardian:, **)
return unless thread return unless thread
thread.update!(last_message: message) thread.update!(last_message: message_instance)
thread.increment_replies_count_cache thread.increment_replies_count_cache
thread.add(guardian.user).update!(last_read_message: message) thread.add(guardian.user).update!(last_read_message: message_instance)
thread.add(thread.original_message_user) thread.add(thread.original_message_user)
end end
def create_webhook_event(contract:, message:, **) def create_webhook_event(contract:, message_instance:, **)
return if contract.incoming_chat_webhook.blank? return if contract.incoming_chat_webhook.blank?
message.create_chat_webhook_event(incoming_chat_webhook: contract.incoming_chat_webhook) message_instance.create_chat_webhook_event(
incoming_chat_webhook: contract.incoming_chat_webhook,
)
end end
def update_channel_last_message(channel:, message:, **) def update_channel_last_message(channel:, message_instance:, **)
return if message.in_thread? return if message_instance.in_thread?
channel.update!(last_message: message) channel.update!(last_message: message_instance)
end end
def update_membership_last_read(channel_membership:, message:, **) def update_membership_last_read(channel_membership:, message_instance:, **)
return if message.in_thread? return if message_instance.in_thread?
channel_membership.update!(last_read_message: message) channel_membership.update!(last_read_message: message_instance)
end end
def process_direct_message_channel(channel_membership:, **) def process_direct_message_channel(channel_membership:, **)
@ -173,16 +175,25 @@ module Chat
Chat::Publisher.publish_thread_created!(channel, reply, thread.id) Chat::Publisher.publish_thread_created!(channel, reply, thread.id)
end end
def publish_new_message_events(channel:, message:, contract:, guardian:, **) def publish_new_message_events(channel:, message_instance:, contract:, **)
Chat::Publisher.publish_new!(channel, message, contract.staged_id) staged_id = contract.staged_id
Jobs.enqueue(Jobs::Chat::ProcessMessage, { chat_message_id: message.id })
Chat::Notifier.notify_new(chat_message: message, timestamp: message.created_at) if contract.process_inline
DiscourseEvent.trigger(:chat_message_created, message, channel, guardian.user) Jobs::Chat::ProcessMessage.new.execute(
{ chat_message_id: message_instance.id, staged_id: staged_id },
)
else
Jobs.enqueue(
Jobs::Chat::ProcessMessage,
{ chat_message_id: message_instance.id, staged_id: staged_id },
)
end
end end
def publish_user_tracking_state(message:, channel:, channel_membership:, guardian:, **) def publish_user_tracking_state(message_instance:, channel:, channel_membership:, guardian:, **)
message_to_publish = message message_to_publish = message_instance
message_to_publish = channel_membership.last_read_message || message if message.in_thread? message_to_publish =
channel_membership.last_read_message || message_instance if message_instance.in_thread?
Chat::Publisher.publish_user_tracking_state!(guardian.user, channel, message_to_publish) Chat::Publisher.publish_user_tracking_state!(guardian.user, channel, message_to_publish)
end end
end end

View File

@ -18,6 +18,7 @@ module Chat
contract contract
model :message model :message
model :uploads, optional: true model :uploads, optional: true
step :enforce_system_membership
policy :can_modify_channel_message policy :can_modify_channel_message
policy :can_modify_message policy :can_modify_message
@ -30,19 +31,24 @@ module Chat
class Contract class Contract
attribute :message_id, :string attribute :message_id, :string
validates :message_id, presence: true
attribute :message, :string attribute :message, :string
validates :message, presence: true, if: -> { upload_ids.blank? }
attribute :upload_ids, :array attribute :upload_ids, :array
validates :message_id, presence: true attribute :process_inline, :boolean, default: Rails.env.test?
validates :message, presence: true, if: -> { upload_ids.blank? }
end end
private private
def enforce_system_membership(guardian:, message:, **)
message.chat_channel.add(guardian.user) if guardian.user.is_system_user?
end
def fetch_message(contract:, **) def fetch_message(contract:, **)
::Chat::Message ::Chat::Message.includes(
.strict_loading
.includes(
:chat_mentions, :chat_mentions,
:bookmarks, :bookmarks,
:chat_webhook_event, :chat_webhook_event,
@ -56,8 +62,7 @@ module Chat
chatable: [:topic_only_relative_url, direct_message_users: [user: :user_option]], chatable: [:topic_only_relative_url, direct_message_users: [user: :user_option]],
], ],
user: :user_status, user: :user_status,
) ).find_by(id: contract.message_id)
.find_by(id: contract.message_id)
end end
def fetch_uploads(contract:, guardian:, **) def fetch_uploads(contract:, guardian:, **)
@ -88,9 +93,7 @@ module Chat
end end
def save_message(message:, **) def save_message(message:, **)
message.cook
message.save! message.save!
message.update_mentions
end end
def save_revision(message:, guardian:, **) def save_revision(message:, guardian:, **)
@ -127,13 +130,19 @@ module Chat
chars_edited > max_edited_chars chars_edited > max_edited_chars
end end
def publish(message:, guardian:, **) def publish(message:, guardian:, contract:, **)
::Chat::Publisher.publish_edit!(message.chat_channel, message) edit_timestamp = context.revision&.created_at&.iso8601(6) || Time.zone.now.iso8601(6)
Jobs.enqueue(Jobs::Chat::ProcessMessage, { chat_message_id: message.id })
edit_timestamp = context.revision&.created_at || Time.zone.now if contract.process_inline
::Chat::Notifier.notify_edit(chat_message: message, timestamp: edit_timestamp) Jobs::Chat::ProcessMessage.new.execute(
DiscourseEvent.trigger(:chat_message_edited, message, message.chat_channel, guardian.user) { chat_message_id: message.id, edit_timestamp: edit_timestamp },
)
else
Jobs.enqueue(
Jobs::Chat::ProcessMessage,
{ chat_message_id: message.id, edit_timestamp: edit_timestamp },
)
end
if message.thread.present? if message.thread.present?
::Chat::Publisher.publish_thread_original_message_metadata!(message.thread) ::Chat::Publisher.publish_thread_original_message_metadata!(message.thread)

View File

@ -8,9 +8,7 @@
{{did-insert this.setUploadDropZone}} {{did-insert this.setUploadDropZone}}
{{did-insert this.setupListeners}} {{did-insert this.setupListeners}}
{{will-destroy this.teardownListeners}} {{will-destroy this.teardownListeners}}
{{did-update this.loadMessages @targetMessageId}}
{{did-insert this.didUpdateChannel}} {{did-insert this.didUpdateChannel}}
{{did-update this.didUpdateChannel @channel.id}}
{{did-insert this.addAutoFocusEventListener}} {{did-insert this.addAutoFocusEventListener}}
{{will-destroy this.removeAutoFocusEventListener}} {{will-destroy this.removeAutoFocusEventListener}}
data-id={{@channel.id}} data-id={{@channel.id}}

View File

@ -61,7 +61,6 @@ export default class ChatChannel extends Component {
@tracked isScrolling = false; @tracked isScrolling = false;
scrollable = null; scrollable = null;
_loadedChannelId = null;
_mentionWarningsSeen = {}; _mentionWarningsSeen = {};
_unreachableGroupMentions = []; _unreachableGroupMentions = [];
_overMembersLimitGroupMentions = []; _overMembersLimitGroupMentions = [];
@ -98,7 +97,7 @@ export default class ChatChannel extends Component {
teardownListeners() { teardownListeners() {
this.#cancelHandlers(); this.#cancelHandlers();
removeOnPresenceChange(this.onPresenceChangeCallback); removeOnPresenceChange(this.onPresenceChangeCallback);
this.unsubscribeToUpdates(this._loadedChannelId); this.unsubscribeToUpdates(this.args.channel.id);
} }
@action @action
@ -109,12 +108,6 @@ export default class ChatChannel extends Component {
@action @action
didUpdateChannel() { didUpdateChannel() {
this.#cancelHandlers();
if (!this.args.channel) {
return;
}
this.messagesManager.clear(); this.messagesManager.clear();
if ( if (
@ -124,12 +117,6 @@ export default class ChatChannel extends Component {
this.chatChannelsManager.follow(this.args.channel); this.chatChannelsManager.follow(this.args.channel);
} }
if (this._loadedChannelId !== this.args.channel.id) {
this.unsubscribeToUpdates(this._loadedChannelId);
this.pane.selectingMessages = false;
this._loadedChannelId = this.args.channel.id;
}
const existingDraft = this.chatDraftsManager.get({ const existingDraft = this.chatDraftsManager.get({
channelId: this.args.channel.id, channelId: this.args.channel.id,
}); });
@ -647,7 +634,6 @@ export default class ChatChannel extends Component {
return; return;
} }
this.unsubscribeToUpdates(channel.id);
this.messageBus.subscribe( this.messageBus.subscribe(
`/chat/${channel.id}`, `/chat/${channel.id}`,
this.onMessage, this.onMessage,

View File

@ -1,6 +1,6 @@
import Component from "@ember/component"; import Component from "@ember/component";
import { action } from "@ember/object"; import { action } from "@ember/object";
import { cancel, throttle } from "@ember/runloop"; import { cancel, next, throttle } from "@ember/runloop";
import { inject as service } from "@ember/service"; import { inject as service } from "@ember/service";
import { htmlSafe } from "@ember/template"; import { htmlSafe } from "@ember/template";
import DiscourseURL from "discourse/lib/url"; import DiscourseURL from "discourse/lib/url";
@ -188,11 +188,13 @@ export default Component.extend({
}, },
@action @action
openInFullPage() { async openInFullPage() {
this.chatStateManager.storeAppURL(); this.chatStateManager.storeAppURL();
this.chatStateManager.prefersFullPage(); this.chatStateManager.prefersFullPage();
this.chat.activeChannel = null; this.chat.activeChannel = null;
await new Promise((resolve) => next(resolve));
return DiscourseURL.routeTo(this.chatStateManager.lastKnownChatURL); return DiscourseURL.routeTo(this.chatStateManager.lastKnownChatURL);
}, },

View File

@ -16,10 +16,12 @@
{{did-update this.fetchChannel @params.channelId}} {{did-update this.fetchChannel @params.channelId}}
> >
{{#if this.chat.activeChannel}} {{#if this.chat.activeChannel}}
{{#each (array this.chat.activeChannel) as |channel|}}
<ChatChannel <ChatChannel
@targetMessageId={{readonly @params.messageId}} @targetMessageId={{readonly @params.messageId}}
@channel={{this.chat.activeChannel}} @channel={{channel}}
/> />
{{/each}}
{{/if}} {{/if}}
</div> </div>
{{/if}} {{/if}}

View File

@ -509,6 +509,7 @@ export default class ChatMessage extends Component {
(if @message.highlighted "-highlighted") (if @message.highlighted "-highlighted")
(if (eq @message.user.id this.currentUser.id) "is-by-current-user") (if (eq @message.user.id this.currentUser.id) "is-by-current-user")
(if @message.staged "-staged" "-persisted") (if @message.staged "-staged" "-persisted")
(if @message.processed "-processed" "-not-processed")
(if this.hasActiveState "-active") (if this.hasActiveState "-active")
(if @message.bookmark "-bookmarked") (if @message.bookmark "-bookmarked")
(if @message.deletedAt "-deleted") (if @message.deletedAt "-deleted")

View File

@ -1,3 +1,3 @@
{{#if @channel.id}} {{#each (array @channel) as |channel|}}
<ChatChannel @channel={{@channel}} @targetMessageId={{@targetMessageId}} /> <ChatChannel @channel={{channel}} @targetMessageId={{@targetMessageId}} />
{{/if}} {{/each}}

View File

@ -192,6 +192,7 @@ export default class ChatChannel {
async stageMessage(message) { async stageMessage(message) {
message.id = guid(); message.id = guid();
message.staged = true; message.staged = true;
message.processed = false;
message.draft = false; message.draft = false;
message.createdAt = new Date(); message.createdAt = new Date();
message.channel = this; message.channel = this;

View File

@ -26,6 +26,7 @@ export default class ChatMessage {
@tracked selected; @tracked selected;
@tracked channel; @tracked channel;
@tracked staged; @tracked staged;
@tracked processed;
@tracked draftSaved; @tracked draftSaved;
@tracked draft; @tracked draft;
@tracked createdAt; @tracked createdAt;
@ -64,6 +65,7 @@ export default class ChatMessage {
this.draftSaved = args.draftSaved || args.draft_saved || false; this.draftSaved = args.draftSaved || args.draft_saved || false;
this.firstOfResults = args.firstOfResults || args.first_of_results || false; this.firstOfResults = args.firstOfResults || args.first_of_results || false;
this.staged = args.staged || false; this.staged = args.staged || false;
this.processed = args.processed || true;
this.edited = args.edited || false; this.edited = args.edited || false;
this.editing = args.editing || false; this.editing = args.editing || false;
this.availableFlags = args.availableFlags || args.available_flags; this.availableFlags = args.availableFlags || args.available_flags;

View File

@ -61,6 +61,7 @@ export default class ChatThread {
async stageMessage(message) { async stageMessage(message) {
message.id = guid(); message.id = guid();
message.staged = true; message.staged = true;
message.processed = false;
message.draft = false; message.draft = false;
message.createdAt ??= moment.utc().format(); message.createdAt ??= moment.utc().format();
message.thread = this; message.thread = this;

View File

@ -16,7 +16,6 @@ export function handleStagedMessage(channel, messagesManager, data) {
stagedMessage.excerpt = data.chat_message.excerpt; stagedMessage.excerpt = data.chat_message.excerpt;
stagedMessage.channel = channel; stagedMessage.channel = channel;
stagedMessage.createdAt = new Date(data.chat_message.created_at); stagedMessage.createdAt = new Date(data.chat_message.created_at);
stagedMessage.cooked = data.chat_message.cooked;
return stagedMessage; return stagedMessage;
} }
@ -131,6 +130,7 @@ export default class ChatPaneBaseSubscriptionsManager extends Service {
const message = this.messagesManager.findMessage(data.chat_message.id); const message = this.messagesManager.findMessage(data.chat_message.id);
if (message) { if (message) {
message.cooked = data.chat_message.cooked; message.cooked = data.chat_message.cooked;
message.processed = true;
} }
} }
@ -144,8 +144,6 @@ export default class ChatPaneBaseSubscriptionsManager extends Service {
handleEditMessage(data) { handleEditMessage(data) {
const message = this.messagesManager.findMessage(data.chat_message.id); const message = this.messagesManager.findMessage(data.chat_message.id);
if (message) { if (message) {
message.message = data.chat_message.message;
message.cooked = data.chat_message.cooked;
message.excerpt = data.chat_message.excerpt; message.excerpt = data.chat_message.excerpt;
message.uploads = cloneJSON(data.chat_message.uploads || []); message.uploads = cloneJSON(data.chat_message.uploads || []);
message.edited = data.chat_message.edited; message.edited = data.chat_message.edited;

View File

@ -209,7 +209,7 @@ module Chat
end end
def can_edit_chat?(message) def can_edit_chat?(message)
message.user_id == @user.id && !@user.silenced? (message.user_id == @user.id && !@user.silenced?)
end end
def can_react? def can_react?

View File

@ -4,12 +4,12 @@ module Chat
class MessageProcessor class MessageProcessor
include ::CookedProcessorMixin include ::CookedProcessorMixin
def initialize(chat_message) def initialize(chat_message, opts = {})
@model = chat_message @model = chat_message
@previous_cooked = (chat_message.cooked || "").dup @previous_cooked = (chat_message.cooked || "").dup
@with_secure_uploads = false @with_secure_uploads = false
@size_cache = {} @size_cache = {}
@opts = {} @opts = opts
cooked = Chat::Message.cook(chat_message.message, user_id: chat_message.last_editor_id) cooked = Chat::Message.cook(chat_message.message, user_id: chat_message.last_editor_id)
@doc = Loofah.html5_fragment(cooked) @doc = Loofah.html5_fragment(cooked)

View File

@ -63,10 +63,10 @@ end
Fabricator(:chat_message_without_service, class_name: "Chat::Message") do Fabricator(:chat_message_without_service, class_name: "Chat::Message") do
user user
chat_channel chat_channel
message { Faker::Lorem.paragraph_by_chars(number: 500).gsub("...", "").gsub("..", "") } message { Faker::Lorem.words(number: 5).join(" ") }
after_build { |message, attrs| message.cook } after_build { |message, attrs| message.cook }
after_create { |message, attrs| message.create_mentions } after_create { |message, attrs| message.upsert_mentions }
end end
Fabricator(:chat_message_with_service, class_name: "Chat::CreateMessage") do Fabricator(:chat_message_with_service, class_name: "Chat::CreateMessage") do
@ -86,17 +86,26 @@ Fabricator(:chat_message_with_service, class_name: "Chat::CreateMessage") do
Group.refresh_automatic_groups! Group.refresh_automatic_groups!
channel.add(user) channel.add(user)
result =
resolved_class.call( resolved_class.call(
chat_channel_id: channel.id, chat_channel_id: channel.id,
guardian: user.guardian, guardian: user.guardian,
message: message: transients[:message] || Faker::Lorem.words(number: 5).join(" "),
transients[:message] ||
Faker::Lorem.paragraph_by_chars(number: 500).gsub("...", "").gsub("..", ""),
thread_id: transients[:thread]&.id, thread_id: transients[:thread]&.id,
in_reply_to_id: transients[:in_reply_to]&.id, in_reply_to_id: transients[:in_reply_to]&.id,
upload_ids: transients[:upload_ids], upload_ids: transients[:upload_ids],
incoming_chat_webhook: transients[:incoming_chat_webhook], incoming_chat_webhook: transients[:incoming_chat_webhook],
).message process_inline: true,
)
if result.failure?
raise RSpec::Expectations::ExpectationNotMetError.new(
"Service `#{resolved_class}` failed, see below for step details:\n\n" +
result.inspect_steps.inspect,
)
end
result.message_instance
end end
end end

View File

@ -19,23 +19,6 @@ describe Jobs::Chat::ProcessMessage do
) )
end end
context "when is_dirty args is true" do
fab!(:chat_message) { Fabricate(:chat_message, message: "a very lovely cat") }
it "publishes the update" do
Chat::Publisher.expects(:publish_processed!).once
described_class.new.execute(chat_message_id: chat_message.id, is_dirty: true)
end
end
context "when is_dirty args is not true" do
fab!(:chat_message) { Fabricate(:chat_message, message: "a very lovely cat") }
it "doesn’t publish the update" do
Chat::Publisher.expects(:publish_processed!).never
described_class.new.execute(chat_message_id: chat_message.id)
end
context "when the cooked message changed" do context "when the cooked message changed" do
it "publishes the update" do it "publishes the update" do
chat_message.update!(cooked: "another lovely cat") chat_message.update!(cooked: "another lovely cat")
@ -43,7 +26,6 @@ describe Jobs::Chat::ProcessMessage do
described_class.new.execute(chat_message_id: chat_message.id) described_class.new.execute(chat_message_id: chat_message.id)
end end
end end
end
it "does not error when message is deleted" do it "does not error when message is deleted" do
chat_message.destroy chat_message.destroy

View File

@ -510,49 +510,23 @@ describe Chat::Message do
it "keeps the same hashtags the user has permission to after rebake" do it "keeps the same hashtags the user has permission to after rebake" do
group.add(chat_message.user) group.add(chat_message.user)
chat_message.update!( update_message(
message: chat_message,
user: chat_message.user,
text:
"this is the message ##{category.slug} ##{secure_category.slug} ##{chat_message.chat_channel.slug}", "this is the message ##{category.slug} ##{secure_category.slug} ##{chat_message.chat_channel.slug}",
) )
chat_message.cook
chat_message.save!
expect(chat_message.reload.cooked).to include(secure_category.name) expect(chat_message.reload.cooked).to include(secure_category.name)
chat_message.rebake! chat_message.rebake!
expect(chat_message.reload.cooked).to include(secure_category.name) expect(chat_message.reload.cooked).to include(secure_category.name)
end end
end end
end end
describe "#create_mentions" do describe "#upsert_mentions" do
fab!(:message) { Fabricate(:chat_message) }
fab!(:user1) { Fabricate(:user) }
fab!(:user2) { Fabricate(:user) }
it "creates mentions for mentioned usernames" do
message.message = "Mentioning @#{user1.username} and @#{user2.username}"
message.cook
message.create_mentions
message.reload
expect(message.chat_mentions.pluck(:user_id)).to match_array([user1.id, user2.id])
end
it "ignores duplicated mentions" do
message.message =
"Mentioning @#{user1.username} @#{user1.username} @#{user1.username} @#{user1.username}"
message.cook
message.create_mentions
message.reload
expect(message.chat_mentions.pluck(:user_id)).to contain_exactly(user1.id)
end
end
describe "#update_mentions" do
fab!(:user1) { Fabricate(:user) } fab!(:user1) { Fabricate(:user) }
fab!(:user2) { Fabricate(:user) } fab!(:user2) { Fabricate(:user) }
fab!(:user3) { Fabricate(:user) } fab!(:user3) { Fabricate(:user) }
@ -564,11 +538,11 @@ describe Chat::Message do
it "creates newly added mentions" do it "creates newly added mentions" do
existing_mention_ids = message.chat_mentions.pluck(:id) existing_mention_ids = message.chat_mentions.pluck(:id)
message.message = message.message + " @#{user3.username} @#{user4.username} " update_message(
message.cook message,
user: message.user,
message.update_mentions text: message.message + " @#{user3.username} @#{user4.username} ",
message.reload )
expect(message.chat_mentions.pluck(:user_id)).to match_array( expect(message.chat_mentions.pluck(:user_id)).to match_array(
[user1.id, user2.id, user3.id, user4.id], [user1.id, user2.id, user3.id, user4.id],
@ -577,22 +551,15 @@ describe Chat::Message do
end end
it "drops removed mentions" do it "drops removed mentions" do
message.message = "Hey @#{user1.username}" # user 2 is not mentioned anymore # user 2 is not mentioned anymore
message.cook update_message(message, user: message.user, text: "Hey @#{user1.username}")
message.update_mentions
message.reload
expect(message.chat_mentions.pluck(:user_id)).to contain_exactly(user1.id) expect(message.chat_mentions.pluck(:user_id)).to contain_exactly(user1.id)
end end
it "changes nothing if passed mentions are identical to existing mentions" do it "changes nothing if passed mentions are identical to existing mentions" do
existing_mention_ids = message.chat_mentions.pluck(:id) existing_mention_ids = message.chat_mentions.pluck(:id)
message.message = message.message update_message(message, user: message.user, text: message.message)
message.cook
message.update_mentions
message.reload
expect(message.chat_mentions.pluck(:user_id)).to match_array(already_mentioned) expect(message.chat_mentions.pluck(:user_id)).to match_array(already_mentioned)
expect(message.chat_mentions.pluck(:id)).to include(*existing_mention_ids) # the mentions weren't recreated expect(message.chat_mentions.pluck(:id)).to include(*existing_mention_ids) # the mentions weren't recreated

View File

@ -8,7 +8,7 @@ module ChatSystemHelpers
user.activate user.activate
SiteSetting.chat_enabled = true SiteSetting.chat_enabled = true
SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_1] SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
channels_for_membership.each do |channel| channels_for_membership.each do |channel|
membership = channel.add(user) membership = channel.add(user)
@ -42,11 +42,11 @@ module ChatSystemHelpers
in_reply_to_id: in_reply_to, in_reply_to_id: in_reply_to,
thread_id: thread_id, thread_id: thread_id,
guardian: last_user.guardian, guardian: last_user.guardian,
message: Faker::Lorem.paragraph, message: Faker::Lorem.words(number: 5).join(" "),
) )
raise "#{creator.inspect_steps.inspect}\n\n#{creator.inspect_steps.error}" if creator.failure? raise "#{creator.inspect_steps.inspect}\n\n#{creator.inspect_steps.error}" if creator.failure?
last_message = creator.message last_message = creator.message_instance
end end
last_message.thread.set_replies_count_cache(messages_count - 1, update_db: true) last_message.thread.set_replies_count_cache(messages_count - 1, update_db: true)
@ -67,6 +67,19 @@ module ChatSpecHelpers
"Service failed, see below for step details:\n\n" + result.inspect_steps.inspect, "Service failed, see below for step details:\n\n" + result.inspect_steps.inspect,
) )
end end
def update_message(message, text: nil, user: Discourse.system_user, upload_ids: nil)
result =
Chat::UpdateMessage.call(
guardian: user.guardian,
message_id: message.id,
upload_ids: upload_ids,
message: text,
process_inline: true,
)
service_failed!(result) if result.failure?
result.message_instance
end
end end
RSpec.configure do |config| RSpec.configure do |config|

View File

@ -120,11 +120,11 @@ RSpec.describe Chat::Api::ChannelThreadsController do
end end
it "has preloaded chat mentions and users for the thread original message" do it "has preloaded chat mentions and users for the thread original message" do
thread_1.original_message.update!( update_message(
message: "@#{current_user.username} hello and @#{thread_2.original_message_user.username}!", thread_1.original_message,
user: thread_1.original_message.user,
text: "@#{current_user.username} hello and @#{thread_2.original_message_user.username}!",
) )
thread_1.original_message.rebake!
thread_1.original_message.create_mentions
get "/chat/api/channels/#{public_channel.id}/threads" get "/chat/api/channels/#{public_channel.id}/threads"
expect(response.status).to eq(200) expect(response.status).to eq(200)

View File

@ -114,7 +114,6 @@ RSpec.describe Chat::ChatController do
job: Jobs::Chat::ProcessMessage, job: Jobs::Chat::ProcessMessage,
args: { args: {
chat_message_id: chat_message.id, chat_message_id: chat_message.id,
is_dirty: true,
}, },
) do ) do
put "/chat/#{chat_channel.id}/#{chat_message.id}/rebake.json" put "/chat/#{chat_channel.id}/#{chat_message.id}/rebake.json"

View File

@ -34,7 +34,7 @@ RSpec.describe Chat::CreateMessage do
let(:params) do let(:params) do
{ guardian: guardian, chat_channel_id: channel.id, message: content, upload_ids: [upload.id] } { guardian: guardian, chat_channel_id: channel.id, message: content, upload_ids: [upload.id] }
end end
let(:message) { result[:message].reload } let(:message) { result[:message_instance].reload }
shared_examples "creating a new message" do shared_examples "creating a new message" do
it "saves the message" do it "saves the message" do
@ -43,10 +43,12 @@ RSpec.describe Chat::CreateMessage do
end end
it "cooks the message" do it "cooks the message" do
Jobs.run_immediately!
expect(message).to be_cooked expect(message).to be_cooked
end end
it "creates mentions" do it "creates mentions" do
Jobs.run_immediately!
expect { result }.to change { Chat::Mention.count }.by(1) expect { result }.to change { Chat::Mention.count }.by(1)
end end
@ -73,21 +75,15 @@ RSpec.describe Chat::CreateMessage do
result result
end end
it "enqueues a job to process message" do it "can enqueue a job to process message" do
result params[:process_inline] = false
expect_job_enqueued(job: Jobs::Chat::ProcessMessage, args: { chat_message_id: message.id }) expect_enqueued_with(job: Jobs::Chat::ProcessMessage) { result }
end end
it "notifies the new message" do it "can process a message inline" do
result params[:process_inline] = true
expect_job_enqueued( Jobs::Chat::ProcessMessage.any_instance.expects(:execute).once
job: Jobs::Chat::SendMessageNotifications, expect_not_enqueued_with(job: Jobs::Chat::ProcessMessage) { result }
args: {
chat_message_id: message.id,
timestamp: message.created_at.iso8601(6),
reason: "new",
},
)
end end
it "triggers a Discourse event" do it "triggers a Discourse event" do
@ -340,7 +336,7 @@ RSpec.describe Chat::CreateMessage do
context "when message is not valid" do context "when message is not valid" do
let(:content) { "a" * (SiteSetting.chat_maximum_message_length + 1) } let(:content) { "a" * (SiteSetting.chat_maximum_message_length + 1) }
it { is_expected.to fail_with_an_invalid_model(:message) } it { is_expected.to fail_with_an_invalid_model(:message_instance) }
end end
context "when message is valid" do context "when message is valid" do

View File

@ -56,6 +56,7 @@ RSpec.describe Chat::UpdateMessage do
user: user, user: user,
message: message, message: message,
upload_ids: upload_ids, upload_ids: upload_ids,
use_service: true,
) )
end end
@ -153,18 +154,20 @@ RSpec.describe Chat::UpdateMessage do
it "publishes updated message to message bus" do it "publishes updated message to message bus" do
chat_message = create_chat_message(user1, "This will be changed", public_chat_channel) chat_message = create_chat_message(user1, "This will be changed", public_chat_channel)
new_content = "New content" new_content = "New content"
messages =
MessageBus.track_publish("/chat/#{public_chat_channel.id}") do processed_message =
MessageBus
.track_publish("/chat/#{public_chat_channel.id}") do
described_class.call( described_class.call(
guardian: guardian, guardian: guardian,
message_id: chat_message.id, message_id: chat_message.id,
message: new_content, message: new_content,
) )
end end
.detect { |m| m.data["type"] == "edit" }
.data
expect(messages.count).to be(1) expect(processed_message["chat_message"]["message"]).to eq(new_content)
message = messages[0].data
expect(message["chat_message"]["message"]).to eq(new_content)
end end
context "with mentions" do context "with mentions" do
@ -281,20 +284,21 @@ RSpec.describe Chat::UpdateMessage do
chat_message = create_chat_message(user1, "This will be updated", public_chat_channel) chat_message = create_chat_message(user1, "This will be updated", public_chat_channel)
new_content = "Hey @#{user2.username}" new_content = "Hey @#{user2.username}"
messages = processed_message =
MessageBus.track_publish("/chat/#{public_chat_channel.id}") do MessageBus
.track_publish("/chat/#{public_chat_channel.id}") do
described_class.call( described_class.call(
guardian: guardian, guardian: guardian,
message_id: chat_message.id, message_id: chat_message.id,
message: new_content, message: new_content,
) )
end end
.detect { |m| m.data["type"] == "edit" }
.data
expect(messages.count).to be(1) expect(processed_message["chat_message"]["mentioned_users"].count).to eq(1)
message = messages[0].data
expect(message["chat_message"]["mentioned_users"].count).to eq(1)
mentioned_user = message["chat_message"]["mentioned_users"][0]
mentioned_user = processed_message["chat_message"]["mentioned_users"][0]
expect(mentioned_user["id"]).to eq(user2.id) expect(mentioned_user["id"]).to eq(user2.id)
expect(mentioned_user["username"]).to eq(user2.username) expect(mentioned_user["username"]).to eq(user2.username)
expect(mentioned_user["status"]).to be_present expect(mentioned_user["status"]).to be_present
@ -305,21 +309,21 @@ RSpec.describe Chat::UpdateMessage do
SiteSetting.enable_user_status = false SiteSetting.enable_user_status = false
user2.set_status!("dentist", "tooth") user2.set_status!("dentist", "tooth")
chat_message = create_chat_message(user1, "This will be updated", public_chat_channel) chat_message = create_chat_message(user1, "This will be updated", public_chat_channel)
new_content = "Hey @#{user2.username}"
messages = processed_message =
MessageBus.track_publish("/chat/#{public_chat_channel.id}") do MessageBus
.track_publish("/chat/#{public_chat_channel.id}") do
described_class.call( described_class.call(
guardian: guardian, guardian: guardian,
message_id: chat_message.id, message_id: chat_message.id,
message: new_content, message: "Hey @#{user2.username}",
) )
end end
.detect { |m| m.data["type"] == "edit" }
.data
expect(messages.count).to be(1) expect(processed_message["chat_message"]["mentioned_users"].count).to be(1)
message = messages[0].data mentioned_user = processed_message["chat_message"]["mentioned_users"][0]
expect(message["chat_message"]["mentioned_users"].count).to be(1)
mentioned_user = message["chat_message"]["mentioned_users"][0]
expect(mentioned_user["status"]).to be_blank expect(mentioned_user["status"]).to be_blank
end end
@ -775,6 +779,17 @@ RSpec.describe Chat::UpdateMessage do
expect { result }.to not_change { result.message.last_editor_id } expect { result }.to not_change { result.message.last_editor_id }
end end
it "can enqueue a job to process message" do
params[:process_inline] = false
expect_enqueued_with(job: Jobs::Chat::ProcessMessage) { result }
end
it "can process a message inline" do
params[:process_inline] = true
Jobs::Chat::ProcessMessage.any_instance.expects(:execute).once
expect_not_enqueued_with(job: Jobs::Chat::ProcessMessage) { result }
end
end end
context "when params are not valid" do context "when params are not valid" do

View File

@ -68,8 +68,13 @@ RSpec.describe "Archive channel", type: :system do
let(:other_user) { Fabricate(:user) } let(:other_user) { Fabricate(:user) }
before do before do
Jobs.run_immediately!
channel_1.add(current_user) channel_1.add(current_user)
channel_1.add(other_user)
end
it "clears unread indicators" do
Jobs.run_immediately!
Fabricate( Fabricate(
:chat_message, :chat_message,
chat_channel: channel_1, chat_channel: channel_1,
@ -77,9 +82,7 @@ RSpec.describe "Archive channel", type: :system do
message: "this is fine @#{current_user.username}", message: "this is fine @#{current_user.username}",
use_service: true, use_service: true,
) )
end
it "clears unread indicators" do
visit("/") visit("/")
expect(page.find(".chat-channel-unread-indicator")).to have_content(1) expect(page.find(".chat-channel-unread-indicator")).to have_content(1)

View File

@ -16,10 +16,14 @@ RSpec.describe "Chat | composer | channel", type: :system, js: true do
end end
describe "reply to message" do describe "reply to message" do
context "when raw contains html" do
fab!(:message_1) do
Fabricate(:chat_message, chat_channel: channel_1, message: "<mark>not marked</mark>")
end
it "renders text in the details" do it "renders text in the details" do
message_1.update!(message: "<mark>not marked</mark>")
message_1.rebake!
chat_page.visit_channel(channel_1) chat_page.visit_channel(channel_1)
channel_page.reply_to(message_1) channel_page.reply_to(message_1)
expect(channel_page.composer.message_details).to have_message( expect(channel_page.composer.message_details).to have_message(
@ -27,6 +31,7 @@ RSpec.describe "Chat | composer | channel", type: :system, js: true do
exact_text: "<mark>not marked</mark>", exact_text: "<mark>not marked</mark>",
) )
end end
end
context "when threading is disabled" do context "when threading is disabled" do
it "replies to the message" do it "replies to the message" do

View File

@ -92,6 +92,8 @@ RSpec.describe "Chat channel", type: :system do
context "with two sessions opened on same channel" do context "with two sessions opened on same channel" do
it "syncs the messages" do it "syncs the messages" do
Jobs.run_immediately!
using_session(:tab_1) do using_session(:tab_1) do
sign_in(current_user) sign_in(current_user)
chat_page.visit_channel(channel_1) chat_page.visit_channel(channel_1)
@ -275,8 +277,11 @@ RSpec.describe "Chat channel", type: :system do
end end
it "renders safe HTML like mentions (which are just links) in the reply-to" do it "renders safe HTML like mentions (which are just links) in the reply-to" do
message_2.update!(message: "@#{other_user.username} <mark>not marked</mark>") update_message(
message_2.rebake! message_2,
user: other_user,
text: "@#{other_user.username} <mark>not marked</mark>",
)
chat_page.visit_channel(channel_1) chat_page.visit_channel(channel_1)
expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq( expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq(

View File

@ -3,7 +3,7 @@
RSpec.describe "Chat message - channel", type: :system do RSpec.describe "Chat message - channel", type: :system do
fab!(:current_user) { Fabricate(:user) } fab!(:current_user) { Fabricate(:user) }
fab!(:channel_1) { Fabricate(:chat_channel) } fab!(:channel_1) { Fabricate(:chat_channel) }
fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, use_service: true) }
let(:cdp) { PageObjects::CDP.new } let(:cdp) { PageObjects::CDP.new }
let(:chat_page) { PageObjects::Pages::Chat.new } let(:chat_page) { PageObjects::Pages::Chat.new }

View File

@ -3,12 +3,15 @@
RSpec.describe "Chat message - thread", type: :system do RSpec.describe "Chat message - thread", type: :system do
fab!(:current_user) { Fabricate(:user) } fab!(:current_user) { Fabricate(:user) }
fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) }
fab!(:thread_original_message) { Fabricate(:chat_message_with_service, chat_channel: channel_1) }
fab!(:thread_message_1) do fab!(:thread_message_1) do
message_1 = Fabricate(:chat_message, chat_channel: channel_1, use_service: true) Fabricate(
Fabricate(:chat_message, in_reply_to: message_1, use_service: true) :chat_message_with_service,
chat_channel: channel_1,
in_reply_to: thread_original_message,
)
end end
let(:cdp) { PageObjects::CDP.new }
let(:chat_page) { PageObjects::Pages::Chat.new } let(:chat_page) { PageObjects::Pages::Chat.new }
let(:thread_page) { PageObjects::Pages::ChatThread.new } let(:thread_page) { PageObjects::Pages::ChatThread.new }
@ -31,6 +34,8 @@ RSpec.describe "Chat message - thread", type: :system do
end end
context "when copying text of a message" do context "when copying text of a message" do
let(:cdp) { PageObjects::CDP.new }
before { cdp.allow_clipboard } before { cdp.allow_clipboard }
it "[mobile] copies the text of a single message", mobile: true do it "[mobile] copies the text of a single message", mobile: true do

View File

@ -15,7 +15,7 @@ RSpec.describe "Deleted channel", type: :system do
it "redirects to homepage" do it "redirects to homepage" do
chat_page.visit_channel(channel_1) chat_page.visit_channel(channel_1)
expect(page).to have_current_path("/latest") expect(page).to have_content("Not Found") # this is not a translated key
end end
end end
end end

View File

@ -15,19 +15,14 @@ RSpec.describe "Deleted message", type: :system do
end end
context "when deleting a message" do context "when deleting a message" do
fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) }
it "shows as deleted" do it "shows as deleted" do
chat_page.visit_channel(channel_1) chat_page.visit_channel(channel_1)
channel_page.send_message
expect(page).to have_css(".-persisted") channel_page.messages.delete(message_1)
last_message = find(".chat-message-container:last-child") expect(channel_page.messages).to have_deleted_message(message_1, count: 1)
channel_page.messages.delete(OpenStruct.new(id: last_message["data-id"]))
expect(channel_page.messages).to have_deleted_message(
OpenStruct.new(id: last_message["data-id"]),
count: 1,
)
end end
it "does not error when coming back to the channel from another channel" do it "does not error when coming back to the channel from another channel" do

View File

@ -120,18 +120,19 @@ RSpec.describe "Drawer", type: :system do
chat_page.minimize_full_page chat_page.minimize_full_page
drawer_page.maximize drawer_page.maximize
using_session("user_1") do |session| Fabricate(
sign_in(user_1) :chat_message,
chat_page.visit_channel(channel_1) chat_channel: channel_1,
channel_page.send_message("onlyonce") user: user_1,
session.quit use_service: true,
end message: "onlyonce",
)
expect(page).to have_content("onlyonce", count: 1, wait: 20) expect(page).to have_content("onlyonce", count: 1)
chat_page.visit_channel(channel_2) chat_page.visit_channel(channel_2)
expect(page).to have_content("onlyonce", count: 0, wait: 20) expect(page).to have_content("onlyonce", count: 0)
end end
end end

View File

@ -36,7 +36,8 @@ RSpec.describe "Edited message", type: :system do
end end
it "runs decorators on the edited message" do it "runs decorators on the edited message" do
message_1 = Fabricate(:chat_message, chat_channel: channel_1, user: current_user) message_1 =
Fabricate(:chat_message, chat_channel: channel_1, user: current_user, use_service: true)
chat_page.visit_channel(channel_1) chat_page.visit_channel(channel_1)
channel_page.edit_message(message_1, '[date=2025-03-10 timezone="Europe/Paris"]') channel_page.edit_message(message_1, '[date=2025-03-10 timezone="Europe/Paris"]')

View File

@ -11,8 +11,9 @@ RSpec.describe "Message errors", type: :system do
context "when message is too long" do context "when message is too long" do
fab!(:channel) { Fabricate(:chat_channel) } fab!(:channel) { Fabricate(:chat_channel) }
before { channel.add(current_user) }
it "only shows the error, not the message" do it "only shows the error, not the message" do
channel.add(current_user)
sign_in(current_user) sign_in(current_user)
chat_page.visit_channel(channel) chat_page.visit_channel(channel)

View File

@ -12,8 +12,8 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
chat_system_bootstrap chat_system_bootstrap
end end
def create_message(text: "this is fine", channel:, creator: Fabricate(:user)) def create_message(channel, text: "this is fine", user: Fabricate(:user))
Fabricate(:chat_message_with_service, chat_channel: channel, message: text, user: creator) Fabricate(:chat_message_with_service, chat_channel: channel, message: text, user: user)
end end
context "as a user" do context "as a user" do
@ -30,13 +30,9 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
context "when not member of the channel" do context "when not member of the channel" do
context "when a message is created" do context "when a message is created" do
it "doesn't show anything" do it "doesn't show anything" do
Jobs.run_immediately!
visit("/chat") visit("/chat")
using_session(:user_1) do |session|
create_message(channel: channel_1, creator: user_1) create_message(channel_1, user: user_1)
session.quit
end
expect(page).to have_no_css(".chat-header-icon .chat-channel-unread-indicator") expect(page).to have_no_css(".chat-header-icon .chat-channel-unread-indicator")
expect(page).to have_no_css(channel_index_page.channel_row_selector(channel_1)) expect(page).to have_no_css(channel_index_page.channel_row_selector(channel_1))
@ -58,13 +54,9 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
end end
it "doesn’t show indicator in header" do it "doesn’t show indicator in header" do
Jobs.run_immediately!
visit("/chat") visit("/chat")
using_session(:user_1) do |session|
create_message(channel: channel_1, creator: user_1) create_message(channel_1, user: user_1)
session.quit
end
expect(page).to have_css(".do-not-disturb-background") expect(page).to have_css(".do-not-disturb-background")
expect(page).to have_no_css(".chat-header-icon .chat-channel-unread-indicator") expect(page).to have_no_css(".chat-header-icon .chat-channel-unread-indicator")
@ -76,13 +68,9 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
context "when a message is created" do context "when a message is created" do
it "doesn't show anything" do it "doesn't show anything" do
Jobs.run_immediately!
visit("/chat") visit("/chat")
using_session(:user_1) do |session|
create_message(channel: channel_1, creator: user_1) create_message(channel_1, user: user_1)
session.quit
end
expect(page).to have_no_css(".chat-header-icon .chat-channel-unread-indicator") expect(page).to have_no_css(".chat-header-icon .chat-channel-unread-indicator")
expect(channel_index_page).to have_no_unread_channel(channel_1) expect(channel_index_page).to have_no_unread_channel(channel_1)
@ -92,13 +80,9 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
context "when a message is created" do context "when a message is created" do
it "correctly renders notifications" do it "correctly renders notifications" do
Jobs.run_immediately!
visit("/chat") visit("/chat")
using_session(:user_1) do |session|
create_message(channel: channel_1, creator: user_1) create_message(channel_1, user: user_1)
session.quit
end
expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "") expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "")
expect(channel_index_page).to have_unread_channel(channel_1) expect(channel_index_page).to have_unread_channel(channel_1)
@ -110,13 +94,12 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
Jobs.run_immediately! Jobs.run_immediately!
visit("/chat") visit("/chat")
using_session(:user_1) do
create_message( create_message(
channel: channel_1, channel_1,
creator: user_1, user: user_1,
text: "hello @#{current_user.username} what's up?", text: "hello @#{current_user.username} what's up?",
) )
end
expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator") expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator")
expect(channel_index_page).to have_unread_channel(channel_1, count: 1) expect(channel_index_page).to have_unread_channel(channel_1, count: 1)
@ -135,13 +118,9 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
context "when a message is created" do context "when a message is created" do
it "correctly renders notifications" do it "correctly renders notifications" do
Jobs.run_immediately!
visit("/chat") visit("/chat")
using_session(:user_1) do |session|
create_message(channel: dm_channel_1, creator: user_1) create_message(dm_channel_1, user: user_1)
session.quit
end
expect(page).to have_css( expect(page).to have_css(
".chat-header-icon .chat-channel-unread-indicator", ".chat-header-icon .chat-channel-unread-indicator",
@ -150,10 +129,7 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
) )
expect(channel_index_page).to have_unread_channel(dm_channel_1, wait: 25) expect(channel_index_page).to have_unread_channel(dm_channel_1, wait: 25)
using_session(:user_1) do |session| create_message(dm_channel_1, user: user_1)
create_message(channel: dm_channel_1, creator: user_1)
session.quit
end
expect(page).to have_css( expect(page).to have_css(
".chat-header-icon .chat-channel-unread-indicator", ".chat-header-icon .chat-channel-unread-indicator",
@ -163,8 +139,6 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
end end
it "reorders channels" do it "reorders channels" do
Jobs.run_immediately!
visit("/chat") visit("/chat")
expect(page).to have_css( expect(page).to have_css(
@ -174,10 +148,7 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
".chat-channel-row:nth-child(2)[data-chat-channel-id=\"#{dm_channel_2.id}\"]", ".chat-channel-row:nth-child(2)[data-chat-channel-id=\"#{dm_channel_2.id}\"]",
) )
using_session(:user_1) do |session| create_message(dm_channel_2, user: user_2)
create_message(channel: dm_channel_2, creator: user_2)
session.quit
end
expect(page).to have_css( expect(page).to have_css(
".chat-channel-row:nth-child(1)[data-chat-channel-id=\"#{dm_channel_2.id}\"]", ".chat-channel-row:nth-child(1)[data-chat-channel-id=\"#{dm_channel_2.id}\"]",
@ -202,21 +173,14 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
context "when messages are created" do context "when messages are created" do
it "correctly renders notifications" do it "correctly renders notifications" do
Jobs.run_immediately!
visit("/chat") visit("/chat")
using_session(:user_1) do |session|
create_message(channel: channel_1, creator: user_1) create_message(channel_1, user: user_1)
session.quit
end
expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "") expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "")
expect(channel_index_page).to have_unread_channel(channel_1) expect(channel_index_page).to have_unread_channel(channel_1)
using_session(:user_1) do |session| create_message(dm_channel_1, user: user_1)
create_message(channel: dm_channel_1, creator: user_1)
session.quit
end
expect(channel_index_page).to have_unread_channel(dm_channel_1) expect(channel_index_page).to have_unread_channel(dm_channel_1)
expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "1") expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator", text: "1")

View File

@ -52,7 +52,6 @@ RSpec.describe "Message notifications - with sidebar", type: :system do
end end
it "doesn’t show indicator in header" do it "doesn’t show indicator in header" do
Jobs.run_immediately!
visit("/") visit("/")
create_message(channel: channel_1, creator: user_1) create_message(channel: channel_1, creator: user_1)

View File

@ -6,6 +6,7 @@ describe "Thread indicator for chat messages", type: :system do
let(:chat_page) { PageObjects::Pages::Chat.new } let(:chat_page) { PageObjects::Pages::Chat.new }
let(:channel_page) { PageObjects::Pages::ChatChannel.new } let(:channel_page) { PageObjects::Pages::ChatChannel.new }
let(:thread_page) { PageObjects::Pages::ChatThread.new }
let(:side_panel) { PageObjects::Pages::ChatSidePanel.new } let(:side_panel) { PageObjects::Pages::ChatSidePanel.new }
let(:open_thread) { PageObjects::Pages::ChatThread.new } let(:open_thread) { PageObjects::Pages::ChatThread.new }
let(:chat_drawer_page) { PageObjects::Pages::ChatDrawer.new } let(:chat_drawer_page) { PageObjects::Pages::ChatDrawer.new }
@ -123,50 +124,49 @@ describe "Thread indicator for chat messages", type: :system do
end end
it "shows an excerpt of the last reply in the thread" do it "shows an excerpt of the last reply in the thread" do
thread_1.last_message.update!(message: "test for excerpt") update_message(
thread_1.last_message.rebake! thread_1.original_message,
user: thread_1.original_message.user,
text: "test for excerpt",
)
chat_page.visit_channel(channel) chat_page.visit_channel(channel)
expect( expect(
channel_page.message_thread_indicator(thread_1.original_message).excerpt, channel_page.message_thread_indicator(thread_1.original_message.reload).excerpt,
).to have_content(thread_excerpt(thread_1.last_message)) ).to have_content(thread_excerpt(thread_1.last_message.reload))
end end
it "updates the last reply excerpt and participants when a new message is added to the thread" do it "updates the last reply excerpt and participants when a new message is added to the thread" do
new_user = Fabricate(:user) new_user = Fabricate(:user)
chat_system_user_bootstrap(user: new_user, channel: channel) chat_system_user_bootstrap(user: new_user, channel: channel)
original_last_reply = thread_1.last_message original_last_reply = thread_1.last_message
original_last_reply.update!(message: "test for excerpt") update_message(original_last_reply, user: original_last_reply.user, text: "test for excerpt")
original_last_reply.rebake!
chat_page.visit_channel(channel) chat_page.visit_channel(channel)
excerpt_text = thread_excerpt(original_last_reply) excerpt_text = thread_excerpt(original_last_reply.reload)
expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_content( expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_content(
excerpt_text, excerpt_text,
) )
using_session(:new_user) do |session| new_message =
sign_in(new_user) Fabricate(
chat_page.visit_channel(channel) :chat_message,
channel_page.message_thread_indicator(thread_1.original_message).click chat_channel: channel,
thread: thread_1,
expect(side_panel).to have_open_thread(thread_1) user: new_user,
in_reply_to: thread_1.original_message,
open_thread.send_message("wow i am happy to join this thread!") use_service: true,
end )
expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_participant( expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_participant(
new_user, new_user,
) )
new_user_reply = thread_1.replies.where(user: new_user).first
excerpt_text = thread_excerpt(new_user_reply)
expect( expect(
channel_page.message_thread_indicator(thread_1.original_message).excerpt, channel_page.message_thread_indicator(thread_1.original_message).excerpt,
).to have_content(excerpt_text) ).to have_content(thread_excerpt(new_message))
end end
end end
end end

View File

@ -98,7 +98,7 @@ module PageObjects
def edit_message(message, text = nil) def edit_message(message, text = nil)
messages.edit(message) messages.edit(message)
send_message(message.message + text) if text send_message(message.message + " " + text) if text
end end
def send_message(text = nil) def send_message(text = nil)
@ -106,6 +106,7 @@ module PageObjects
text = text.chomp if text.present? # having \n on the end of the string counts as an Enter keypress text = text.chomp if text.present? # having \n on the end of the string counts as an Enter keypress
composer.fill_in(with: text) composer.fill_in(with: text)
click_send_message click_send_message
expect(page).to have_no_css(".chat-message.-not-processed")
text text
end end

View File

@ -106,6 +106,7 @@ module PageObjects
text = text.chomp if text.present? # having \n on the end of the string counts as an Enter keypress text = text.chomp if text.present? # having \n on the end of the string counts as an Enter keypress
composer.fill_in(with: text) composer.fill_in(with: text)
click_send_message click_send_message
expect(page).to have_no_css(".chat-message.-not-processed")
click_composer click_composer
text text
end end
@ -141,7 +142,7 @@ module PageObjects
def edit_message(message, text = nil) def edit_message(message, text = nil)
messages.edit(message) messages.edit(message)
send_message(message.message + text) if text send_message(message.message + " " + text) if text
end end
end end
end end

View File

@ -32,7 +32,7 @@ module PageObjects
def secondary_action(action) def secondary_action(action)
if page.has_css?("html.mobile-view", wait: 0) if page.has_css?("html.mobile-view", wait: 0)
component.click(delay: 0.4) component.click(delay: 0.6)
page.find(".chat-message-actions [data-id=\"#{action}\"]").click page.find(".chat-message-actions [data-id=\"#{action}\"]").click
else else
open_more_menu open_more_menu
@ -52,14 +52,7 @@ module PageObjects
return return
end end
if page.has_css?("html.mobile-view", wait: 0) secondary_action("select")
component.click(delay: 0.6)
page.find(".chat-message-actions [data-id=\"select\"]").click
else
hover
click_more_button
page.find("[data-value='select']").click
end
end end
def find(**args) def find(**args)
@ -103,6 +96,13 @@ module PageObjects
def build_selector(**args) def build_selector(**args)
selector = SELECTOR selector = SELECTOR
if args[:not_processed]
selector += ".-not-processed"
else
selector += ".-processed"
end
selector += "[data-id=\"#{args[:id]}\"]" if args[:id] selector += "[data-id=\"#{args[:id]}\"]" if args[:id]
selector += ".-selected" if args[:selected] selector += ".-selected" if args[:selected]
selector += ".-persisted" if args[:persisted] selector += ".-persisted" if args[:persisted]

View File

@ -13,6 +13,7 @@ RSpec.describe "Reply to message - channel - full page", type: :system do
:chat_message, :chat_message,
chat_channel: channel_1, chat_channel: channel_1,
message: "This is a message to reply to!", message: "This is a message to reply to!",
user: current_user,
use_service: true, use_service: true,
) )
end end
@ -98,8 +99,11 @@ RSpec.describe "Reply to message - channel - full page", type: :system do
it "renders safe HTML from the original message excerpt" do it "renders safe HTML from the original message excerpt" do
other_user = Fabricate(:user) other_user = Fabricate(:user)
original_message.update!(message: "@#{other_user.username} <mark>not marked</mark>") update_message(
original_message.rebake! original_message,
user: current_user,
text: "@#{other_user.username} <mark>not marked</mark>",
)
chat_page.visit_channel(channel_1) chat_page.visit_channel(channel_1)
channel_page.reply_to(original_message) channel_page.reply_to(original_message)

View File

@ -39,7 +39,7 @@ describe "Thread list in side panel | full page", type: :system do
it "does not show new threads in the channel in the thread list if the user is not tracking them" do it "does not show new threads in the channel in the thread list if the user is not tracking them" do
chat_page.visit_channel(channel) chat_page.visit_channel(channel)
Fabricate(:chat_message_with_service, chat_channel: channel, in_reply_to: thread_om) Fabricate(:chat_message, chat_channel: channel, in_reply_to: thread_om, use_service: true)
channel_page.open_thread_list channel_page.open_thread_list
expect(page).to have_content(I18n.t("js.chat.threads.none")) expect(page).to have_content(I18n.t("js.chat.threads.none"))
@ -89,9 +89,12 @@ describe "Thread list in side panel | full page", type: :system do
expect(thread_list_page.item_by_id(thread_1.id)).to have_css("img.emoji[alt='hamburger']") expect(thread_list_page.item_by_id(thread_1.id)).to have_css("img.emoji[alt='hamburger']")
end end
it "shows an excerpt of the original message of the thread" do it "shows an excerpt of the original message of the thread", inline: true do
thread_1.original_message.update!(message: "This is a long message for the excerpt") update_message(
thread_1.original_message.rebake! thread_1.original_message,
user: thread_1.original_message.user,
text: "This is a long message for the excerpt",
)
chat_page.visit_threads_list(channel) chat_page.visit_threads_list(channel)
expect(thread_list_page.item_by_id(thread_1.id)).to have_content( expect(thread_list_page.item_by_id(thread_1.id)).to have_content(

View File

@ -41,7 +41,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system do
context "when quoting a single message into a topic" do context "when quoting a single message into a topic" do
fab!(:post_1) { Fabricate(:post) } fab!(:post_1) { Fabricate(:post) }
fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1) } fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1, use_service: true) }
it "quotes the message" do it "quotes the message" do
chat_page.visit_channel(chat_channel_1) chat_page.visit_channel(chat_channel_1)
@ -62,8 +62,8 @@ RSpec.describe "Quoting chat message transcripts", type: :system do
context "when quoting multiple messages into a topic" do context "when quoting multiple messages into a topic" do
fab!(:post_1) { Fabricate(:post) } fab!(:post_1) { Fabricate(:post) }
fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1) } fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1, use_service: true) }
fab!(:message_2) { Fabricate(:chat_message, chat_channel: chat_channel_1) } fab!(:message_2) { Fabricate(:chat_message, chat_channel: chat_channel_1, use_service: true) }
it "quotes the messages" do it "quotes the messages" do
chat_page.visit_channel(chat_channel_1) chat_page.visit_channel(chat_channel_1)
@ -85,7 +85,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system do
context "when quoting a message containing a onebox" do context "when quoting a message containing a onebox" do
fab!(:post_1) { Fabricate(:post) } fab!(:post_1) { Fabricate(:post) }
fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1) } fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1, use_service: true) }
before do before do
Oneboxer.stubs(:preview).returns( Oneboxer.stubs(:preview).returns(
@ -109,7 +109,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system do
end end
context "when quoting a message in another message" do context "when quoting a message in another message" do
fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1) } fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1, use_service: true) }
it "quotes the message" do it "quotes the message" do
chat_page.visit_channel(chat_channel_1) chat_page.visit_channel(chat_channel_1)
@ -125,7 +125,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system do
end end
context "when quoting into a topic directly" do context "when quoting into a topic directly" do
fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1) } fab!(:message_1) { Fabricate(:chat_message, chat_channel: chat_channel_1, use_service: true) }
let(:topic_title) { "Some topic title for testing" } let(:topic_title) { "Some topic title for testing" }
it "opens the topic composer with correct state" do it "opens the topic composer with correct state" do