diff --git a/plugins/chat/app/models/chat/message.rb b/plugins/chat/app/models/chat/message.rb index 8247aa7f2b8..bda702260ae 100644 --- a/plugins/chat/app/models/chat/message.rb +++ b/plugins/chat/app/models/chat/message.rb @@ -73,9 +73,11 @@ module Chat before_save { ensure_last_editor_id } + validate :validate_message + def self.polymorphic_class_mapping = { "ChatMessage" => Chat::Message } - def validate_message(has_uploads:) + def validate_message self.message = TextCleaner.clean(self.message, strip_whitespaces: true, strip_zero_width_spaces: true) @@ -85,7 +87,7 @@ module Chat Chat::DuplicateMessageValidator.new(self).validate end - if !has_uploads && message_too_short? + if uploads.empty? && message_too_short? self.errors.add( :base, I18n.t( @@ -103,23 +105,6 @@ module Chat end end - def attach_uploads(uploads) - return if uploads.blank? || self.new_record? - - now = Time.now - ref_record_attrs = - uploads.map do |upload| - { - upload_id: upload.id, - target_id: self.id, - target_type: self.class.polymorphic_name, - created_at: now, - updated_at: now, - } - end - UploadReference.insert_all!(ref_record_attrs) - end - def excerpt(max_length: 50) # just show the URL if the whole message is a URL, because we cannot excerpt oneboxes return message if UrlHelper.relaxed_parse(message).is_a?(URI) diff --git a/plugins/chat/app/services/chat/channel_view_builder.rb b/plugins/chat/app/services/chat/channel_view_builder.rb index 61ea22c9f99..4cc54252cfb 100644 --- a/plugins/chat/app/services/chat/channel_view_builder.rb +++ b/plugins/chat/app/services/chat/channel_view_builder.rb @@ -184,7 +184,7 @@ module Chat context.threads = ::Chat::Thread .strict_loading - .includes(last_message: [:user], original_message_user: :user_status) + .includes(last_message: %i[user uploads], original_message_user: :user_status) .where(id: messages.map(&:thread_id).compact.uniq) # Saves us having to load the same message we already have. diff --git a/plugins/chat/lib/chat/message_creator.rb b/plugins/chat/lib/chat/message_creator.rb index 2b1c16317f5..31782849eaf 100644 --- a/plugins/chat/lib/chat/message_creator.rb +++ b/plugins/chat/lib/chat/message_creator.rb @@ -48,8 +48,8 @@ module Chat def create begin validate_channel_status! - uploads = get_uploads - validate_message!(has_uploads: uploads.any?) + @chat_message.uploads = get_uploads + validate_message! validate_reply_chain! validate_existing_thread! @@ -60,7 +60,6 @@ module Chat create_chat_webhook_event create_thread - @chat_message.attach_uploads(uploads) Chat::Draft.where(user_id: @user.id, chat_channel_id: @chat_channel.id).destroy_all post_process_resolved_thread update_channel_last_message @@ -154,11 +153,9 @@ module Chat end end - def validate_message!(has_uploads:) - @chat_message.validate_message(has_uploads: has_uploads) - if @chat_message.errors.present? - raise StandardError.new(@chat_message.errors.map(&:full_message).join(", ")) - end + def validate_message! + return if @chat_message.valid? + raise StandardError.new(@chat_message.errors.map(&:full_message).join(", ")) end def create_chat_webhook_event diff --git a/plugins/chat/lib/chat/message_updater.rb b/plugins/chat/lib/chat/message_updater.rb index 26cd9f9ffeb..04eb7ba5e6b 100644 --- a/plugins/chat/lib/chat/message_updater.rb +++ b/plugins/chat/lib/chat/message_updater.rb @@ -28,12 +28,12 @@ module Chat @chat_message.message = @new_content @chat_message.last_editor_id = @user.id upload_info = get_upload_info - validate_message!(has_uploads: upload_info[:uploads].any?) + @chat_message.uploads = upload_info[:uploads] if upload_info[:changed] + validate_message! @chat_message.cook @chat_message.save! @chat_message.update_mentions - update_uploads(upload_info) revision = save_revision! @chat_message.reload @@ -63,11 +63,9 @@ module Chat ) end - def validate_message!(has_uploads:) - @chat_message.validate_message(has_uploads: has_uploads) - if @chat_message.errors.present? - raise StandardError.new(@chat_message.errors.map(&:full_message).join(", ")) - end + def validate_message! + return if @chat_message.valid? + raise StandardError.new(@chat_message.errors.map(&:full_message).join(", ")) end def get_upload_info @@ -85,13 +83,6 @@ module Chat { uploads: uploads, changed: difference.any? } end - def update_uploads(upload_info) - return unless upload_info[:changed] - - UploadReference.where(target: @chat_message).destroy_all - @chat_message.attach_uploads(upload_info[:uploads]) - end - def save_revision! @chat_message.revisions.create!( old_message: @old_message_content, diff --git a/plugins/chat/spec/lib/chat/transcript_service_spec.rb b/plugins/chat/spec/lib/chat/transcript_service_spec.rb index c99809cf108..96d086ce17e 100644 --- a/plugins/chat/spec/lib/chat/transcript_service_spec.rb +++ b/plugins/chat/spec/lib/chat/transcript_service_spec.rb @@ -118,7 +118,6 @@ describe Chat::TranscriptService do it "generates image / attachment / video / audio markdown inside the [chat] bbcode for upload-only messages" do SiteSetting.authorized_extensions = "mp4|mp3|pdf|jpg" - message = Fabricate(:chat_message, user: user1, chat_channel: channel, message: "") video = Fabricate(:upload, original_filename: "test_video.mp4", extension: "mp4") audio = Fabricate(:upload, original_filename: "test_audio.mp3", extension: "mp3") attachment = Fabricate(:upload, original_filename: "test_file.pdf", extension: "pdf") @@ -130,10 +129,14 @@ describe Chat::TranscriptService do original_filename: "test_img.jpg", extension: "jpg", ) - UploadReference.create(target: message, created_at: 10.seconds.ago, upload: video) - UploadReference.create(target: message, created_at: 9.seconds.ago, upload: audio) - UploadReference.create(target: message, created_at: 8.seconds.ago, upload: attachment) - UploadReference.create(target: message, created_at: 7.seconds.ago, upload: image) + message = + Fabricate( + :chat_message, + user: user1, + chat_channel: channel, + message: "", + uploads: [video, audio, attachment, image], + ) video_markdown = UploadMarkdown.new(video).to_markdown audio_markdown = UploadMarkdown.new(audio).to_markdown attachment_markdown = UploadMarkdown.new(attachment).to_markdown diff --git a/plugins/chat/spec/models/chat/message_spec.rb b/plugins/chat/spec/models/chat/message_spec.rb index c5775fa8254..e0a5c232078 100644 --- a/plugins/chat/spec/models/chat/message_spec.rb +++ b/plugins/chat/spec/models/chat/message_spec.rb @@ -293,8 +293,7 @@ describe Chat::Message do it "excerpts upload file name if message is empty" do gif = Fabricate(:upload, original_filename: "cat.gif", width: 400, height: 300, extension: "gif") - message = Fabricate(:chat_message, message: "") - message.attach_uploads([gif]) + message = Fabricate(:chat_message, message: "", uploads: [gif]) expect(message.excerpt).to eq "cat.gif" end @@ -377,7 +376,7 @@ describe Chat::Message do ) image2 = Fabricate(:upload, original_filename: "meme.jpg", width: 10, height: 10, extension: "jpg") - message.attach_uploads([image, image2]) + message.uploads = [image, image2] expect(message.to_markdown).to eq(<<~MSG.chomp) hey friend, what's up?! @@ -410,7 +409,7 @@ describe Chat::Message do Fabricate(:chat_message, message: "this is duplicate", chat_channel: channel, user: user1) message = described_class.new(message: "this is duplicate", chat_channel: channel, user: user2) - message.validate_message(has_uploads: false) + message.valid? expect(message.errors.full_messages).to include(I18n.t("chat.errors.duplicate_message")) end end @@ -526,30 +525,6 @@ describe Chat::Message do end end - describe "#attach_uploads" do - fab!(:chat_message) { Fabricate(:chat_message) } - fab!(:upload_1) { Fabricate(:upload) } - fab!(:upload_2) { Fabricate(:upload) } - - it "creates an UploadReference record for the provided uploads" do - chat_message.attach_uploads([upload_1, upload_2]) - upload_references = UploadReference.where(upload_id: [upload_1, upload_2]) - expect(upload_references.count).to eq(2) - expect(upload_references.map(&:target_id).uniq).to eq([chat_message.id]) - expect(upload_references.map(&:target_type).uniq).to eq([described_class.polymorphic_name]) - end - - it "does nothing if the message record is new" do - expect { described_class.new.attach_uploads([upload_1, upload_2]) }.to not_change { - UploadReference.count - } - end - - it "does nothing for an empty uploads array" do - expect { chat_message.attach_uploads([]) }.to not_change { UploadReference.count } - end - end - describe "#create_mentions" do fab!(:message) { Fabricate(:chat_message) } fab!(:user1) { Fabricate(:user) } diff --git a/plugins/chat/spec/system/channel_message_upload_spec.rb b/plugins/chat/spec/system/channel_message_upload_spec.rb index 94bb365856c..c8e0e5e88d2 100644 --- a/plugins/chat/spec/system/channel_message_upload_spec.rb +++ b/plugins/chat/spec/system/channel_message_upload_spec.rb @@ -21,7 +21,7 @@ RSpec.describe "Channel message selection", type: :system do chat_system_bootstrap channel_1.add(current_user) sign_in(current_user) - message_1.attach_uploads([image]) + message_1.uploads = [image] end it "can collapse/expand an image and still have lightbox working" do