mirror of
https://github.com/discourse/discourse.git
synced 2025-06-03 11:05:12 +08:00
FIX: Allow oneboxes with no description (#31518)
This behaviour was allowed in
cb82dce86a
but then inexplicably removed a few months later in
https://github.com/discourse/onebox/pull/448, but showing
title-only oneboxes is valid. The original Meta topic that
this was discussed in was
https://meta.discourse.org/t/abc-news-not-oneboxing-due-to-missing-description/155933
.
This commit re-introduces allowing this behaviour to avoid the need for
a plugin,
c.f. https://meta.discourse.org/t/allow-title-only-onebox/354306
For example
<https://en-americas-support.nintendo.com/app/answers/detail/a_id/67660>
This commit also unhides onebox descriptions in chat, it's not
clear why they were ever hidden in the first place
This commit is contained in:
Martin Brennan
@ -90,9 +90,13 @@ module Onebox
|
||||
html_entities = HTMLEntities.new
|
||||
d = { link: link }.merge(raw)
|
||||
|
||||
d[:title] = (
|
||||
if d[:title].present?
|
||||
d[:title] = html_entities.decode(Onebox::Helpers.truncate(d[:title], 80))
|
||||
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?
|
||||
|
||||
@ -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),
|
||||
|
||||
@ -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(
|
||||
"<p><a href=\"https://discourse.org/team\" class=\"onebox\" target=\"_blank\" rel=\"noopener nofollow ugc\">https://discourse.org/team</a></p>",
|
||||
"<aside class=\"onebox allowlistedgeneric\" data-onebox-src=\"https://discourse.org/team\">\n <header class=\"source\">\n\n <a href=\"https://discourse.org/team\" target=\"_blank\" rel=\"nofollow ugc noopener\">discourse.org</a>\n </header>\n\n <article class=\"onebox-body\">\n \n\n<h3><a href=\"https://discourse.org/team\" target=\"_blank\" rel=\"nofollow ugc noopener\">a</a></h3>\n\n\n\n </article>\n\n <div class=\"onebox-metadata\">\n \n \n </div>\n\n <div style=\"clear: both\"></div>\n</aside>\n",
|
||||
)
|
||||
end
|
||||
|
||||
|
||||
112
spec/fixtures/onebox/title_no_description.response
vendored
Normal file
112
spec/fixtures/onebox/title_no_description.response
vendored
Normal file
@ -0,0 +1,112 @@
|
||||
|
||||
<!DOCTYPE html>
|
||||
<html class="no-js en-americas-support consumer NOA" lang="en-US">
|
||||
|
||||
|
||||
<head>
|
||||
<!-- Google Tag Manager -->
|
||||
<script>(function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start':
|
||||
new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0],
|
||||
j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src=
|
||||
'https://www.googletagmanager.com/gtm.js?id='+i+dl;f.parentNode.insertBefore(j,f);
|
||||
})(window,document,'script','dataLayer','GTM-M2KCXD8');</script>
|
||||
<!-- End Google Tag Manager -->
|
||||
<meta charset="utf-8" />
|
||||
<title>Discontinuation of Earning My Nintendo Gold Points | Nintendo Support</title>
|
||||
<meta name="description" content=""/>
|
||||
<link rel="search" type="application/opensearchdescription+xml" title="Support Home Page Search" href="/ci/browserSearch/desc/https%3A%2F%2Fen-americas-support.nintendo.com%2Fapp%2Fanswers%2Flist%2Fkw%2F%7BsearchTerms%7D/Support+Home+Page+Search/Support+Home+Page+Search/%2Feuf%2Fassets%2Fimages%2Ficons%2Ffavicon_browserSearchPlugin.ico"/>
|
||||
|
||||
<link rel="shortcut icon" href="https://csassets.nintendo.com/image/upload/t_CP_default/v1703876943/favicon.ico" type="image/x-icon" />
|
||||
<link rel='canonical' href='https://en-americas-support.nintendo.com/app/answers/detail/a_id/67660/~/discontinuation-of-earning-my-nintendo-gold-points'/>
|
||||
<style type='text/css'>
|
||||
<!--
|
||||
.rn_ScreenReaderOnly{position:absolute; height:1px; left:-10000px; overflow:hidden; top:auto; width:1px;}
|
||||
.rn_Hidden{display:none !important;}
|
||||
--></style>
|
||||
<base href='https://en-americas-support.nintendo.com/euf/generated/optimized/1738781035/themes/nintendo/'/>
|
||||
<link href='/euf/generated/optimized/1738781035/templates/nintendo.themes.nintendo.SITE.css' rel='stylesheet' type='text/css' media='all'/>
|
||||
<style type="text/css">
|
||||
<!--
|
||||
.rn_googleAnalytics{}
|
||||
.rn_Flyout{}
|
||||
.rn_RelatedArticles.rn_TopAnswers{}
|
||||
.rn_Breadcrumb{}
|
||||
.rn_GuidedAssistant .rn_Node{position:relative;word-wrap:break-word;}
|
||||
.rn_GuidedAssistant .rn_QuestionText, .rn_GuidedAssistant .rn_ResultHeading{clear:both;overflow:hidden;}
|
||||
.rn_QuestionText p{line-height:1em;}
|
||||
.rn_GuidedAssistant .rn_ChatLink{display:block;margin:2px 20px 0 80%;}
|
||||
.rn_GuidedAssistant .rn_AgentText{background-color:#F4F4F4;border:1px solid #DDD;font-family:sans-serif;margin-top:10px;padding:4px 6px;}
|
||||
.rn_GuidedAssistant .rn_AgentText em{font-style:normal;font-weight:bold;display:block;}
|
||||
.rn_GuidedAssistant .rn_LinkQuestion label{cursor:pointer;}
|
||||
.rn_GuidedAssistant .rn_TransparentScreenReaderOnly{opacity:0;position:absolute;left:0;}
|
||||
.rn_GuidedAssistant .rn_ImageQuestion img{overflow:hidden;}
|
||||
.rn_GuidedAssistant .rn_ImageQuestion div{display:inline-block;*display:inline;margin-bottom:16px;}
|
||||
.rn_GuidedAssistant .rn_ImageQuestion label{cursor:pointer;display:inline-block;position:relative;zoom:1;}
|
||||
.rn_GuidedAssistant .rn_ImageQuestion .rn_ImageCaption{bottom:0;font-weight:bold;left:10px;position:absolute;top:10px;text-shadow:0 1px 1px #FFF;width:100%;}
|
||||
.rn_GuidedAssistant .rn_ButtonQuestion button{margin: 0 6px 6px 0;}
|
||||
@media print{.rn_GuidedAssistant .rn_Question{display:block;}
|
||||
}
|
||||
.rn_DecisionTree{}
|
||||
.rn_mize{}
|
||||
.rn_IncidentThreadDisplay .rn_ThreadContent{word-wrap:break-word;}
|
||||
.rn_IncidentThreadDisplay p.MsoNormal, .rn_IncidentThreadDisplay li.MsoNormal, .rn_IncidentThreadDisplay div.MsoNormal{margin:0;}
|
||||
.rn_IncidentThreadDisplay p.MsoListParagraph, .rn_IncidentThreadDisplay li.MsoListParagraph, .rn_IncidentThreadDisplay div.MsoListParagraph{margin: 0 0 0 48px;}
|
||||
-->
|
||||
</style>
|
||||
|
||||
|
||||
|
||||
<div class="rn_CapabilityDetector" id="rn_CapabilityDetector_3">
|
||||
<noscript>
|
||||
<div class="MessageContainer MessageContainerFailure">
|
||||
Your browser either does not have JavaScript enabled or does not appear to support enough features of JavaScript to be used well on this site. </div>
|
||||
</noscript>
|
||||
<div id="rn_CapabilityDetector_3_MessageContainer">
|
||||
</div>
|
||||
<script type="text/javascript"><!--
|
||||
/*<![CDATA[*/(function () {
|
||||
var runJSTests = function() {
|
||||
var messageContainerDiv = document.getElementById('rn_CapabilityDetector_3_MessageContainer');
|
||||
function showFailure() {
|
||||
messageContainerDiv.className = 'MessageContainer MessageContainerFailure';
|
||||
messageContainerDiv.innerHTML = 'Your browser either does not have JavaScript enabled or does not appear to support enough features of JavaScript to be used well on this site.';
|
||||
}
|
||||
function xhrTestFails() {
|
||||
return !window.XMLHttpRequest;
|
||||
}
|
||||
if(typeof RightNowTesting !== 'undefined' && RightNowTesting._isTesting) {
|
||||
RightNowTesting._xhrTestFails = xhrTestFails;
|
||||
}
|
||||
if (xhrTestFails()) {
|
||||
showFailure();
|
||||
return;
|
||||
}
|
||||
}
|
||||
if(typeof RightNowTesting !== 'undefined' && RightNowTesting._isTesting) {
|
||||
RightNowTesting._runJSTests = runJSTests;
|
||||
}
|
||||
runJSTests();
|
||||
})();
|
||||
/*]]>*/ // --></script></div>
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1.0">
|
||||
|
||||
|
||||
<script src="https://nintendo.quiq-api.com/app/chat-ui/index.js" charset="UTF-8"></script>
|
||||
|
||||
<link rel="stylesheet" href="https://cdn.icomoon.io/225569/SSR/style.css?bescnr"> <!--Prod link for iconoon-->
|
||||
|
||||
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/tooltipster/4.2.8/css/plugins/tooltipster/sideTip/themes/tooltipster-sideTip-light.min.css">
|
||||
<link rel="preload" as="image" href="https://csassets.nintendo.com/image/upload/t_CP_default/f_auto/q_auto/v1703876947/mario_and_friends">
|
||||
|
||||
<!-- Structured Data -->
|
||||
<!-- Alps -->
|
||||
<link rel="stylesheet" href="https://alps.cdn.nintendo.net/v1/css/alps.css">
|
||||
<script src="https://alps.cdn.nintendo.net/v1/js/account_en_USA.js"></script>
|
||||
<!-- End Alps -->
|
||||
</head>
|
||||
|
||||
|
||||
<body class="en_us answers answers-detail">
|
||||
</body>
|
||||
|
||||
</html>
|
||||
@ -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(
|
||||
|
||||
@ -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"))
|
||||
|
||||
Reference in New Issue
Block a user