From 58c8f91d9acf20bbd8d4ef3bd88ed719564769ec Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 29 Jun 2023 09:20:20 +1000 Subject: [PATCH] DEV: Use same excerpt everywhere in chat (#22319) Followup to c6b43ce68b625a90ec09c62971ef0998676f994f We can just use the rich excerpt everywhere since we know we don't need text_entities -- that introduced security issues just to fix a spec. --- plugins/chat/app/models/chat/message.rb | 24 ++----------------- plugins/chat/app/models/chat/thread.rb | 2 +- .../thread_original_message_serializer.rb | 2 +- .../chat/thread_preview_serializer.rb | 2 +- plugins/chat/spec/plugin_helper.rb | 5 +--- .../spec/system/chat/composer/channel_spec.rb | 3 ++- plugins/chat/spec/system/chat_channel_spec.rb | 4 +++- 7 files changed, 11 insertions(+), 31 deletions(-) diff --git a/plugins/chat/app/models/chat/message.rb b/plugins/chat/app/models/chat/message.rb index b9911713458..8247aa7f2b8 100644 --- a/plugins/chat/app/models/chat/message.rb +++ b/plugins/chat/app/models/chat/message.rb @@ -127,32 +127,12 @@ module Chat # upload-only messages are better represented as the filename return uploads.first.original_filename if cooked.blank? && uploads.present? - # this may return blank for some complex things like quotes, that is acceptable - PrettyText.excerpt(message, max_length, { text_entities: true }) - end - - # TODO (martin) Replace the above #excerpt method usage with this one. The - # issue with the above one is that we cannot actually render nice HTML - # fore replies/excerpts in the UI because text_entities: true will - # allow through even denied HTML because of 07ab20131a15ab907c1974fee405d9bdce0c0723. - # - # For now only the thread index uses this new version since it is not interactive, - # we can go back to the interactive reply/edit cases in another PR. - def rich_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) - - # upload-only messages are better represented as the filename - return uploads.first.original_filename if cooked.blank? && uploads.present? - # this may return blank for some complex things like quotes, that is acceptable PrettyText.excerpt(cooked, max_length) end - def censored_excerpt(rich: false, max_length: 50) - WordWatcher.censor( - rich ? rich_excerpt(max_length: max_length) : excerpt(max_length: max_length), - ) + def censored_excerpt(max_length: 50) + WordWatcher.censor(excerpt(max_length: max_length)) end def cooked_for_excerpt diff --git a/plugins/chat/app/models/chat/thread.rb b/plugins/chat/app/models/chat/thread.rb index bd0f944b8e0..a880f17508e 100644 --- a/plugins/chat/app/models/chat/thread.rb +++ b/plugins/chat/app/models/chat/thread.rb @@ -68,7 +68,7 @@ module Chat end def excerpt - original_message.rich_excerpt(max_length: EXCERPT_LENGTH) + original_message.excerpt(max_length: EXCERPT_LENGTH) end def latest_not_deleted_message_id(anchor_message_id: nil) diff --git a/plugins/chat/app/serializers/chat/thread_original_message_serializer.rb b/plugins/chat/app/serializers/chat/thread_original_message_serializer.rb index 8500fba6bc6..0577290f8b5 100644 --- a/plugins/chat/app/serializers/chat/thread_original_message_serializer.rb +++ b/plugins/chat/app/serializers/chat/thread_original_message_serializer.rb @@ -5,7 +5,7 @@ module Chat has_one :user, serializer: BasicUserWithStatusSerializer, embed: :objects def excerpt - object.censored_excerpt(rich: true, max_length: Chat::Thread::EXCERPT_LENGTH) + object.censored_excerpt(max_length: Chat::Thread::EXCERPT_LENGTH) end def include_available_flags? diff --git a/plugins/chat/app/serializers/chat/thread_preview_serializer.rb b/plugins/chat/app/serializers/chat/thread_preview_serializer.rb index dbf132954e8..53355c0d3f6 100644 --- a/plugins/chat/app/serializers/chat/thread_preview_serializer.rb +++ b/plugins/chat/app/serializers/chat/thread_preview_serializer.rb @@ -28,7 +28,7 @@ module Chat end def last_reply_excerpt - object.last_reply.censored_excerpt(rich: true, max_length: Chat::Thread::EXCERPT_LENGTH) + object.last_reply.excerpt(max_length: Chat::Thread::EXCERPT_LENGTH) end def last_reply_user diff --git a/plugins/chat/spec/plugin_helper.rb b/plugins/chat/spec/plugin_helper.rb index b64ecc022a6..378ff17921d 100644 --- a/plugins/chat/spec/plugin_helper.rb +++ b/plugins/chat/spec/plugin_helper.rb @@ -56,10 +56,7 @@ module ChatSystemHelpers def thread_excerpt(message) CGI.escapeHTML( - message.censored_excerpt(rich: true, max_length: ::Chat::Thread::EXCERPT_LENGTH).gsub( - "…", - "…", - ), + message.censored_excerpt(max_length: ::Chat::Thread::EXCERPT_LENGTH).gsub("…", "…"), ) end end diff --git a/plugins/chat/spec/system/chat/composer/channel_spec.rb b/plugins/chat/spec/system/chat/composer/channel_spec.rb index 3668e4f0f49..0f6980ca6a1 100644 --- a/plugins/chat/spec/system/chat/composer/channel_spec.rb +++ b/plugins/chat/spec/system/chat/composer/channel_spec.rb @@ -18,12 +18,13 @@ RSpec.describe "Chat | composer | channel", type: :system, js: true do describe "reply to message" do it "renders text in the details" do message_1.update!(message: "not marked") + message_1.rebake! chat_page.visit_channel(channel_1) channel_page.reply_to(message_1) expect(channel_page.composer.message_details).to have_message( id: message_1.id, - exact_text: "not marked", + exact_text: "<mark>not marked</mark>", ) end diff --git a/plugins/chat/spec/system/chat_channel_spec.rb b/plugins/chat/spec/system/chat_channel_spec.rb index e6f4d0bc062..87bb7ab4117 100644 --- a/plugins/chat/spec/system/chat_channel_spec.rb +++ b/plugins/chat/spec/system/chat_channel_spec.rb @@ -240,7 +240,9 @@ RSpec.describe "Chat channel", type: :system do it "renders text in the reply-to" do chat.visit_channel(channel_1) - expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq("not marked") + expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq( + "&lt;mark&gt;not marked&lt;/mark&gt;", + ) end end