Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DEV: allows to disable strip_whitespaces in messages (#26848) 

The TextCleaner step has been moved from chat message’s validation to create_message/update_message services. It allows us to easily tweak part of its behavior depending on the needs.

For example we will now disable strip_whitespaces by default when streaming messages as we want to keep newlines and spaces at the end of the message.
This commit is contained in:
Joffrey JAFFEUX 2024-05-02 11:59:18 +02:00 committed by GitHub
parent 08e0a6b2cc
commit dda4bb0f7c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 68 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
plugins/chat/app/models/chat/message.rb
View File

@ -96,9 +96,6 @@ module Chat
def self.polymorphic_class_mapping = { "ChatMessage" => Chat::Message }
def validate_message
self.message =
TextCleaner.clean(self.message, strip_whitespaces: true, strip_zero_width_spaces: true)
WatchedWordsValidator.new(attributes: [:message]).validate(self)
if self.new_record? || self.changed.include?("message")

11
plugins/chat/app/services/chat/create_message.rb
View File

@ -34,6 +34,7 @@ module Chat
policy :ensure_valid_thread_for_channel
policy :ensure_thread_matches_parent
model :uploads, optional: true
step :clean_message
model :message_instance, :instantiate_message
transaction do
@ -64,6 +65,7 @@ module Chat
attribute :incoming_chat_webhook
attribute :process_inline, :boolean, default: Rails.env.test?
attribute :force_thread, :boolean, default: false
attribute :strip_whitespaces, :boolean, default: true
validates :chat_channel_id, presence: true
validates :message, presence: true, if: -> { upload_ids.blank? }
@ -133,6 +135,15 @@ module Chat
guardian.user.uploads.where(id: contract.upload_ids)
end
def clean_message(contract:)
contract.message =
TextCleaner.clean(
contract.message,
strip_whitespaces: contract.strip_whitespaces,
strip_zero_width_spaces: true,
)
end
def instantiate_message(channel:, guardian:, contract:, uploads:, thread:, reply:)
channel.chat_messages.new(
user: guardian.user,

12
plugins/chat/app/services/chat/update_message.rb
View File

@ -21,6 +21,7 @@ module Chat
step :enforce_system_membership
policy :can_modify_channel_message
policy :can_modify_message
step :clean_message
transaction do
step :modify_message
@ -41,6 +42,8 @@ module Chat
attribute :streaming, :boolean, default: false
attribute :strip_whitespaces, :boolean, default: true
attribute :process_inline, :boolean, default: Rails.env.test?
end
@ -81,6 +84,15 @@ module Chat
guardian.can_edit_chat?(message)
end
def clean_message(contract:)
contract.message =
TextCleaner.clean(
contract.message,
strip_whitespaces: contract.strip_whitespaces,
strip_zero_width_spaces: true,
)
end
def modify_message(contract:, message:, guardian:, uploads:)
message.message = contract.message
message.last_editor_id = guardian.user.id

5
plugins/chat/lib/chat_sdk/message.rb
View File

@ -37,7 +37,7 @@ module ChatSDK
#
# @see #create
def self.create_with_stream(**params, &block)
self.create(**params, streaming: true, &block)
self.create(**params, streaming: true, strip_whitespaces: false, &block)
end
# Streams to a specific chat message.
@ -114,6 +114,7 @@ module ChatSDK
streaming: false,
enforce_membership: false,
force_thread: false,
strip_whitespaces: true,
&block
)
message =
@ -128,6 +129,7 @@ module ChatSDK
streaming: streaming,
enforce_membership: enforce_membership,
force_thread: force_thread,
strip_whitespaces: strip_whitespaces,
) do
on_model_not_found(:channel) { raise "Couldn't find channel with id: `#{channel_id}`" }
on_model_not_found(:channel_membership) do
@ -179,6 +181,7 @@ module ChatSDK
message: self.message.message + raw,
guardian: self.guardian,
streaming: true,
strip_whitespaces: false,
) { on_failure { raise "Unexpected error" } }
self.message

10
plugins/chat/spec/lib/chat_sdk/message_spec.rb
View File

@ -150,8 +150,6 @@ describe ChatSDK::Message do
fab!(:message_1) { Fabricate(:chat_message, message: "first") }
it "enables streaming" do
initial_message = message_1.message
edit =
MessageBus
.track_publish("/chat/#{message_1.chat_channel.id}") do
@ -168,24 +166,22 @@ describe ChatSDK::Message do
end
describe ".stream" do
fab!(:message_1) { Fabricate(:chat_message, message: "first") }
fab!(:message_1) { Fabricate(:chat_message, message: "first\n") }
before { message_1.update!(streaming: true) }
it "streams" do
initial_message = message_1.message
edit =
MessageBus
.track_publish("/chat/#{message_1.chat_channel.id}") do
described_class.stream(
raw: " test",
raw: " test\n",
message_id: message_1.id,
guardian: message_1.user.guardian,
)
end
.find { |m| m.data["type"] == "edit" }
expect(edit.data["chat_message"]["message"]).to eq("first test")
expect(edit.data["chat_message"]["message"]).to eq("first\n test\n")
expect(message_1.reload.streaming).to eq(true)
end
end

13
plugins/chat/spec/services/chat/create_message_spec.rb
View File

@ -66,6 +66,19 @@ RSpec.describe Chat::CreateMessage do
expect { result }.to change { Chat::Mention.count }.by(1)
end
it "cleans the message" do
params[:message] = "aaaaaaa\n"
expect(message.message).to eq("aaaaaaa")
end
context "when strip_whitespace is disabled" do
it "doesn't strip newlines" do
params[:strip_whitespaces] = false
params[:message] = "aaaaaaa\n"
expect(message.message).to eq("aaaaaaa\n")
end
end
context "when coming from a webhook" do
let(:incoming_webhook) { Fabricate(:incoming_chat_webhook, chat_channel: channel) }

33
plugins/chat/spec/services/chat/update_message_spec.rb
View File

@ -66,12 +66,7 @@ RSpec.describe Chat::UpdateMessage do
new_message = "2 short"
expect do
update =
described_class.call(
guardian: guardian,
message_id: chat_message.id,
message: new_message,
)
described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message)
end.to raise_error(ActiveRecord::RecordInvalid).with_message(
"Validation failed: " +
I18n.t(
@ -137,6 +132,29 @@ RSpec.describe Chat::UpdateMessage do
expect(chat_message.reload.message).to eq(new_message)
end
it "cleans message's content" do
chat_message = create_chat_message(user1, "This will be changed", public_chat_channel)
new_message = "bbbbb\n"
described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message)
expect(chat_message.reload.message).to eq("bbbbb")
end
context "when strip_whitespaces is disabled" do
it "doesn't remove new lines" do
chat_message = create_chat_message(user1, "This will be changed", public_chat_channel)
new_message = "bbbbb\n"
described_class.call(
guardian: guardian,
message_id: chat_message.id,
message: new_message,
strip_whitespaces: false,
)
expect(chat_message.reload.message).to eq("bbbbb\n")
end
end
it "cooks the message" do
chat_message = create_chat_message(user1, "This will be changed", public_chat_channel)
new_message = "Change **to** this!"
@ -510,8 +528,7 @@ RSpec.describe Chat::UpdateMessage do
old_message = "It's a thrsday!"
new_message = "Today is Thursday, it's almost the weekend already!"
chat_message = create_chat_message(user1, old_message, public_chat_channel)
updater =
described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message)
described_class.call(guardian: guardian, message_id: chat_message.id, message: new_message)
revision = chat_message.revisions.last
expect(revision.old_message).to eq(old_message)