mirror of
https://github.com/discourse/discourse.git
synced 2025-02-25 18:55:32 -06:00
FIX: return an empty result if response from Amazon is missing expected attributes (#13173)
* FIX: return an empty result if response from Amazon is missing attributes Check we have the basic attributes requires to construct a Onebox for Amazon. This is an attempt to handle scenarios where we receive a valid 200-status response from an Amazon request that does not include the data we’re expecting. * Update lib/onebox/engine/amazon_onebox.rb Co-authored-by: Régis Hanol <regis@hanol.fr> Co-authored-by: Régis Hanol <regis@hanol.fr>
This commit is contained in:
@@ -66,6 +66,10 @@ module Onebox
|
|||||||
to_html
|
to_html
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def verified_data
|
||||||
|
data
|
||||||
|
end
|
||||||
|
|
||||||
def data
|
def data
|
||||||
@data ||= begin
|
@data ||= begin
|
||||||
html_entities = HTMLEntities.new
|
html_entities = HTMLEntities.new
|
||||||
|
|||||||
@@ -46,6 +46,37 @@ module Onebox
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def to_html(ignore_errors = false)
|
||||||
|
unless ignore_errors
|
||||||
|
verified_data # forces a check for missing fields
|
||||||
|
return '' unless errors.empty?
|
||||||
|
end
|
||||||
|
|
||||||
|
super()
|
||||||
|
end
|
||||||
|
|
||||||
|
def placeholder_html
|
||||||
|
to_html(true)
|
||||||
|
end
|
||||||
|
|
||||||
|
def verified_data
|
||||||
|
@verified_data ||= begin
|
||||||
|
result = data
|
||||||
|
|
||||||
|
required_tags = [:title, :description]
|
||||||
|
required_tags.each do |tag|
|
||||||
|
if result[tag].blank?
|
||||||
|
errors[tag] ||= []
|
||||||
|
errors[tag] << 'is blank'
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
result
|
||||||
|
end
|
||||||
|
|
||||||
|
@verified_data
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def has_cached_body
|
def has_cached_body
|
||||||
|
|||||||
@@ -40,6 +40,11 @@ module Onebox
|
|||||||
engine.data
|
engine.data
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def verified_data
|
||||||
|
return {} unless engine
|
||||||
|
engine.verified_data
|
||||||
|
end
|
||||||
|
|
||||||
def options
|
def options
|
||||||
OpenStruct.new(@options)
|
OpenStruct.new(@options)
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -464,7 +464,7 @@ module Oneboxer
|
|||||||
unless error_keys.length == 1 && skip_if_only_error.include?(error_keys.first)
|
unless error_keys.length == 1 && skip_if_only_error.include?(error_keys.first)
|
||||||
missing_attributes = error_keys.map(&:to_s).sort.join(I18n.t("word_connector.comma"))
|
missing_attributes = error_keys.map(&:to_s).sort.join(I18n.t("word_connector.comma"))
|
||||||
error_message = I18n.t("errors.onebox.missing_data", missing_attributes: missing_attributes, count: error_keys.size)
|
error_message = I18n.t("errors.onebox.missing_data", missing_attributes: missing_attributes, count: error_keys.size)
|
||||||
args = r.data.merge(error_message: error_message)
|
args = r.verified_data.merge(error_message: error_message)
|
||||||
|
|
||||||
if result[:preview].blank?
|
if result[:preview].blank?
|
||||||
result[:preview] = preview_error_onebox(args)
|
result[:preview] = preview_error_onebox(args)
|
||||||
|
|||||||
38
spec/fixtures/onebox/amazon-error.response
vendored
Normal file
38
spec/fixtures/onebox/amazon-error.response
vendored
Normal file
@@ -0,0 +1,38 @@
|
|||||||
|
<!doctype html>
|
||||||
|
<html>
|
||||||
|
<head>
|
||||||
|
<meta charset="utf-8">
|
||||||
|
<meta http-equiv="x-ua-compatible" content="ie=edge">
|
||||||
|
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
|
||||||
|
<style>
|
||||||
|
html, body {
|
||||||
|
padding: 0;
|
||||||
|
margin:0
|
||||||
|
}
|
||||||
|
</style>
|
||||||
|
</head>
|
||||||
|
<body>
|
||||||
|
<form id="b" accept-charset="utf-8" action="/s" method="GET" role="search">
|
||||||
|
<a href="/">
|
||||||
|
<img id="c" src="https://images-na.ssl-images-amazon.com/images/G/01/error/logo._TTD_.png" alt="Amazon.com">
|
||||||
|
</a>
|
||||||
|
<div id="e">
|
||||||
|
<input id="f" name="field-keywords" placeholder="Search">
|
||||||
|
<input id="g" type="submit" value="Go">
|
||||||
|
</div>
|
||||||
|
</form>
|
||||||
|
<div id="h">
|
||||||
|
<div>
|
||||||
|
<a href="/">
|
||||||
|
<img src="https://images-na.ssl-images-amazon.com/images/G/01/error/title._TTD_.png" alt="Sorry! We couldn't find that page. Try searching or go to Amazon's home page.">
|
||||||
|
</a>
|
||||||
|
</div>
|
||||||
|
<a href="/dogsofamazon" target="_blank">
|
||||||
|
<img id="d" alt="Dogs of Amazon">
|
||||||
|
<script>
|
||||||
|
document.getElementById("d").src = "https://images-na.ssl-images-amazon.com/images/G/01/error/" + (Math.floor(Math.random() * 200) + 1) + "._TTD_.jpg";
|
||||||
|
</script>
|
||||||
|
</a>
|
||||||
|
</div>
|
||||||
|
</body>
|
||||||
|
</html>
|
||||||
@@ -54,7 +54,7 @@ describe Onebox::Engine::AmazonOnebox do
|
|||||||
check_link("es", "https://www.amazon.es/familia-Pascual-Duarte-Camilo-Jos%C3%A9-ebook/dp/B00EJRTKTW/")
|
check_link("es", "https://www.amazon.es/familia-Pascual-Duarte-Camilo-Jos%C3%A9-ebook/dp/B00EJRTKTW/")
|
||||||
end
|
end
|
||||||
|
|
||||||
it "matches brasilian domains" do
|
it "matches brazilian domains" do
|
||||||
check_link("com.br", "https://www.amazon.com.br/A-p%C3%A1tria-chuteiras-Nelson-Rodrigues-ebook/dp/B00J2B414Y/")
|
check_link("com.br", "https://www.amazon.com.br/A-p%C3%A1tria-chuteiras-Nelson-Rodrigues-ebook/dp/B00J2B414Y/")
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -193,4 +193,28 @@ describe Onebox::Engine::AmazonOnebox do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context "non-standard response from Amazon" do
|
||||||
|
let(:link) { "https://www.amazon.com/dp/B0123ABCD3210" }
|
||||||
|
let(:onebox) { described_class.new(link) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
stub_request(:get, "https://www.amazon.com/dp/B0123ABCD3210")
|
||||||
|
.to_return(status: 200, body: onebox_response("amazon-error"))
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns a blank result" do
|
||||||
|
expect(onebox.to_html).to eq("")
|
||||||
|
end
|
||||||
|
|
||||||
|
it "produces a placeholder" do
|
||||||
|
expect(onebox.placeholder_html).to include('<aside class="onebox amazon">')
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns errors" do
|
||||||
|
onebox.to_html
|
||||||
|
expect(onebox.errors).to eq({ title: ["is blank"], description: ["is blank"] })
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user