diff --git a/lib/onebox/engine/allowlisted_generic_onebox.rb b/lib/onebox/engine/allowlisted_generic_onebox.rb index a9f0be6d7aa..cbb8a0fb6df 100644 --- a/lib/onebox/engine/allowlisted_generic_onebox.rb +++ b/lib/onebox/engine/allowlisted_generic_onebox.rb @@ -90,9 +90,13 @@ module Onebox html_entities = HTMLEntities.new d = { link: link }.merge(raw) - if d[:title].present? - d[:title] = html_entities.decode(Onebox::Helpers.truncate(d[:title], 80)) - end + d[:title] = ( + if d[:title].present? + html_entities.decode(Onebox::Helpers.truncate(d[:title], 80)) + else + nil + end + ) d[:description] ||= d[:summary] if d[:description].present? @@ -156,7 +160,7 @@ module Onebox ) end - skip_missing_tags = [:video] + skip_missing_tags = %i[video description] d.each do |k, v| next if skip_missing_tags.include?(k) if v == nil || v == "" @@ -199,7 +203,7 @@ module Onebox end def has_text? - has_title? && data[:description].present? + has_title? end def has_title? @@ -219,7 +223,8 @@ module Onebox end def is_video? - data[:type] =~ %r{^video[/\.]} && data[:video_type] == "video/mp4" && data[:video].present? # Many sites include 'videos' with text/html types (i.e. iframes) + # Many sites include 'videos' with text/html types (i.e. iframes) + data[:type] =~ %r{^video[/\.]} && data[:video_type] == "video/mp4" && data[:video].present? end def is_embedded? diff --git a/plugins/chat/assets/stylesheets/common/chat-onebox.scss b/plugins/chat/assets/stylesheets/common/chat-onebox.scss index 8430f77ba43..80bbd525b58 100644 --- a/plugins/chat/assets/stylesheets/common/chat-onebox.scss +++ b/plugins/chat/assets/stylesheets/common/chat-onebox.scss @@ -59,10 +59,6 @@ font-weight: 500; font-size: var(--font-down-1); } - - p { - display: none; - } } .has-full-page-chat .chat-message .onebox:not(img), diff --git a/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb b/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb index 95c700566cf..d83491e845c 100644 --- a/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb +++ b/plugins/chat/spec/jobs/regular/chat/process_message_spec.rb @@ -15,7 +15,7 @@ describe Jobs::Chat::ProcessMessage do it "updates cooked with oneboxes" do described_class.new.execute(chat_message_id: chat_message.id) expect(chat_message.reload.cooked).to eq( - "

https://discourse.org/team

", + "\n", ) end diff --git a/spec/fixtures/onebox/title_no_description.response b/spec/fixtures/onebox/title_no_description.response new file mode 100644 index 00000000000..5b40faf3acb --- /dev/null +++ b/spec/fixtures/onebox/title_no_description.response @@ -0,0 +1,112 @@ + + + + + + + + + + + Discontinuation of Earning My Nintendo Gold Points | Nintendo Support + + + + + + + + + + + + +
+ +
+
+
+ + + + + + + + + + + + + + + + + + + + + + diff --git a/spec/lib/onebox/engine/allowlisted_generic_onebox_spec.rb b/spec/lib/onebox/engine/allowlisted_generic_onebox_spec.rb index 1c4679583fc..95bd5d039e7 100644 --- a/spec/lib/onebox/engine/allowlisted_generic_onebox_spec.rb +++ b/spec/lib/onebox/engine/allowlisted_generic_onebox_spec.rb @@ -191,6 +191,30 @@ RSpec.describe Onebox::Engine::AllowlistedGenericOnebox do end describe "missing description" do + let(:url) { "https://en-americas-support.nintendo.com/app/answers/detail/a_id/67660" } + before do + stub_request(:get, url).to_return(status: 200, body: onebox_response("title_no_description")) + stub_request( + :get, + "https://en-americas-support.nintendo.com/app/answers/detail/a_id/67660/~/discontinuation-of-earning-my-nintendo-gold-points", + ).to_return(status: 200, body: onebox_response("title_no_description")) + stub_request( + :head, + "https://en-americas-support.nintendo.com/app/answers/detail/a_id/67660/~/discontinuation-of-earning-my-nintendo-gold-points", + ).to_return(status: 200, body: "") + end + + it "shows a title-only onebox" do + onebox = described_class.new(url) + expect(onebox.to_html).not_to be_nil + end + + it "does not consider this an error" do + onebox = described_class.new(url) + onebox.data + expect(onebox.errors.key?(:description)).to be(false) + end + context "when working without description if image is present" do before do stub_request( diff --git a/spec/lib/oneboxer_spec.rb b/spec/lib/oneboxer_spec.rb index d8b705f1b69..4b5b496a385 100644 --- a/spec/lib/oneboxer_spec.rb +++ b/spec/lib/oneboxer_spec.rb @@ -781,20 +781,13 @@ RSpec.describe Oneboxer do let(:url) { "https://example.com/fake-url/" } - it "handles a missing description" do + it "handles a missing description, title-only oneboxes are fine" do stub_request(:get, url).to_return(body: response("missing_description")) - expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to include( + expect(Oneboxer.preview(url, invalidate_oneboxes: true)).not_to include( "could not be found: description", ) end - it "handles a missing description and image" do - stub_request(:get, url).to_return(body: response("missing_description_and_image")) - expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to include( - "could not be found: description, image", - ) - end - it "handles a missing image" do # Note: If the only error is a missing image, we shouldn't return an error stub_request(:get, url).to_return(body: response("missing_image"))