diff --git a/plugins/chat/app/models/chat/message.rb b/plugins/chat/app/models/chat/message.rb index 2d0c49e688f..59f3d04fc9b 100644 --- a/plugins/chat/app/models/chat/message.rb +++ b/plugins/chat/app/models/chat/message.rb @@ -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") diff --git a/plugins/chat/app/services/chat/create_message.rb b/plugins/chat/app/services/chat/create_message.rb index a943ed6a43c..59200abd8dd 100644 --- a/plugins/chat/app/services/chat/create_message.rb +++ b/plugins/chat/app/services/chat/create_message.rb @@ -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, diff --git a/plugins/chat/app/services/chat/update_message.rb b/plugins/chat/app/services/chat/update_message.rb index 2ea3c6057ea..014994ecd6e 100644 --- a/plugins/chat/app/services/chat/update_message.rb +++ b/plugins/chat/app/services/chat/update_message.rb @@ -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 diff --git a/plugins/chat/lib/chat_sdk/message.rb b/plugins/chat/lib/chat_sdk/message.rb index aeac2c03036..21103cf63ec 100644 --- a/plugins/chat/lib/chat_sdk/message.rb +++ b/plugins/chat/lib/chat_sdk/message.rb @@ -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 diff --git a/plugins/chat/spec/lib/chat_sdk/message_spec.rb b/plugins/chat/spec/lib/chat_sdk/message_spec.rb index d2cc3c8cd4b..07aacdac01a 100644 --- a/plugins/chat/spec/lib/chat_sdk/message_spec.rb +++ b/plugins/chat/spec/lib/chat_sdk/message_spec.rb @@ -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 diff --git a/plugins/chat/spec/services/chat/create_message_spec.rb b/plugins/chat/spec/services/chat/create_message_spec.rb index 79b23b52214..57c5555b2c0 100644 --- a/plugins/chat/spec/services/chat/create_message_spec.rb +++ b/plugins/chat/spec/services/chat/create_message_spec.rb @@ -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) } diff --git a/plugins/chat/spec/services/chat/update_message_spec.rb b/plugins/chat/spec/services/chat/update_message_spec.rb index 646f7716e84..5852eec4322 100644 --- a/plugins/chat/spec/services/chat/update_message_spec.rb +++ b/plugins/chat/spec/services/chat/update_message_spec.rb @@ -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)