From ef03d4ee1b5893687cff78ef2f6a32424df1d312 Mon Sep 17 00:00:00 2001
From: Nicolas Fricke <nicolas.fricke@devex.com>
Date: Tue, 7 Nov 2017 13:26:15 +0100
Subject: [PATCH] Render message for bad OEmbed responses

When a requested OEmbed object returns an error, render a message for
the user explaining that there was an error with the embedded element.

For this, standardize the `OEbedResolver::Base#source_url` method and
add a `#name` method which will be included in the text for the user.

The text for the user will be rendered within the `<figure>` element
within a nested `span` with the class `unavailable-embed`. This also
allows the consumer to have custom CSS for this case.
---
 CHANGELOG.md                                  |  3 ++
 lib/article_json/elements/embed.rb            |  5 +++
 .../export/common/html/elements/embed.rb      | 12 +++++-
 .../utils/o_embed_resolver/base.rb            | 22 +++++++++++
 .../utils/o_embed_resolver/facebook_video.rb  | 12 ++++--
 .../utils/o_embed_resolver/slideshare.rb      | 13 +++++--
 .../utils/o_embed_resolver/tweet.rb           | 12 ++++--
 .../utils/o_embed_resolver/vimeo_video.rb     | 12 ++++--
 .../utils/o_embed_resolver/youtube_video.rb   | 12 ++++--
 .../export/html/elements/embed_spec.rb        | 35 ++++++++++++++----
 .../utils/o_embed_resolver/base_spec.rb       | 37 ++++++++++++++++++-
 .../o_embed_resolver/facebook_video_spec.rb   |  1 +
 .../oembed_resolver_shared.rb                 | 12 ++++++
 .../utils/o_embed_resolver/slideshare_spec.rb |  1 +
 .../utils/o_embed_resolver/tweet_spec.rb      |  1 +
 .../o_embed_resolver/vimeo_video_spec.rb      |  1 +
 .../o_embed_resolver/youtube_video_spec.rb    |  1 +
 17 files changed, 160 insertions(+), 32 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 14eecd6..3f1c717 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+## WIP
+- Handle non-successful OEmbed responses
+
 ## 0.2.0 - 2017/11/03
 In this second release we **added support** to:
 - Export AMP along with required libraries for AMP rendering
diff --git a/lib/article_json/elements/embed.rb b/lib/article_json/elements/embed.rb
index 9d3d02d..f380e94 100644
--- a/lib/article_json/elements/embed.rb
+++ b/lib/article_json/elements/embed.rb
@@ -33,6 +33,11 @@ def oembed_data
         oembed_resolver&.oembed_data
       end
 
+      # @return [Array[ArticleJSON::Elements::Text]|nil]
+      def oembed_unavailable_message
+        oembed_resolver&.unavailable_message
+      end
+
       private
 
       # @return [ArticleJSON::Utils::OEmbedResolver::Base]
diff --git a/lib/article_json/export/common/html/elements/embed.rb b/lib/article_json/export/common/html/elements/embed.rb
index d49a0e2..c12ffb4 100644
--- a/lib/article_json/export/common/html/elements/embed.rb
+++ b/lib/article_json/export/common/html/elements/embed.rb
@@ -19,14 +19,22 @@ def export
 
             def embed_node
               create_element(:div, class: 'embed') do |div|
-                div.add_child(embedded_object) if embedded_object
+                div.add_child(embedded_object)
               end
             end
 
             def embedded_object
-              return unless @element.oembed_data
+              return unavailable_node unless @element.oembed_data
               Nokogiri::HTML.fragment(@element.oembed_data[:html])
             end
+
+            def unavailable_node
+              create_element(:span, class: 'unavailable-embed') do |span|
+                @element.oembed_unavailable_message.each do |element|
+                  span.add_child(base_class.build(element).export)
+                end
+              end
+            end
           end
         end
       end
diff --git a/lib/article_json/utils/o_embed_resolver/base.rb b/lib/article_json/utils/o_embed_resolver/base.rb
index e4df2ca..5d55d3a 100644
--- a/lib/article_json/utils/o_embed_resolver/base.rb
+++ b/lib/article_json/utils/o_embed_resolver/base.rb
@@ -16,6 +16,28 @@ def oembed_data
           resolver.parsed_api_response
         end
 
+        # In case that there was an error with requesting the OEmbed data from
+        # the endpoint (e.g. because the URL is unavailable), this message can
+        # be rendered to let the user know about the issue
+        # @return [Array[ArticleJSON::Elements::Text]|nil]
+        def unavailable_message
+          [
+            ArticleJSON::Elements::Text.new(content: "The #{name} "),
+            ArticleJSON::Elements::Text.new(content: source_url,
+                                            href: source_url),
+            ArticleJSON::Elements::Text.new(content: ' is not available.'),
+          ]
+        end
+
+        def name
+          raise NotImplementedError,
+                '`#name` needs to be implemented by the subclass'
+        end
+
+        def source_url
+          raise NotImplementedError,
+                '`#source_url` needs to be implemented by the subclass'
+        end
 
         protected
 
diff --git a/lib/article_json/utils/o_embed_resolver/facebook_video.rb b/lib/article_json/utils/o_embed_resolver/facebook_video.rb
index 676fd19..0cbef7d 100644
--- a/lib/article_json/utils/o_embed_resolver/facebook_video.rb
+++ b/lib/article_json/utils/o_embed_resolver/facebook_video.rb
@@ -2,17 +2,21 @@ module ArticleJSON
   module Utils
     module OEmbedResolver
       class FacebookVideo < Base
+        # Human readable name of the resolver
+        # @return [String]
+        def name
+          'Facebook video'
+        end
+
         # The URL for the oembed API call
         # @return [String]
         def oembed_url
-          "https://www.facebook.com/plugins/video/oembed.json?url=#{video_url}"
+          "https://www.facebook.com/plugins/video/oembed.json?url=#{source_url}"
         end
 
-        private
-
         # The video URL of the element
         # @return [String]
-        def video_url
+        def source_url
           "https://www.facebook.com/facebook/videos/#{@element.embed_id}"
         end
       end
diff --git a/lib/article_json/utils/o_embed_resolver/slideshare.rb b/lib/article_json/utils/o_embed_resolver/slideshare.rb
index d4e5ff9..2751f60 100644
--- a/lib/article_json/utils/o_embed_resolver/slideshare.rb
+++ b/lib/article_json/utils/o_embed_resolver/slideshare.rb
@@ -2,17 +2,22 @@ module ArticleJSON
   module Utils
     module OEmbedResolver
       class Slideshare < Base
+        # Human readable name of the resolver
+        # @return [String]
+        def name
+          'Slideshare deck'
+        end
+
         # The URL for the oembed API call
         # @return [String]
         def oembed_url
-          "https://www.slideshare.net/api/oembed/2?format=json&url=#{slide_url}"
+          'https://www.slideshare.net/api/oembed/2?format=json&url='\
+          "#{source_url}"
         end
 
-        private
-
         # The URL of the slideshow
         # @return [String]
-        def slide_url
+        def source_url
           handle, slug = @element.embed_id.split('/', 2)
           "https://www.slideshare.net/#{handle}/#{slug}"
         end
diff --git a/lib/article_json/utils/o_embed_resolver/tweet.rb b/lib/article_json/utils/o_embed_resolver/tweet.rb
index ce7fc73..aa40ea1 100644
--- a/lib/article_json/utils/o_embed_resolver/tweet.rb
+++ b/lib/article_json/utils/o_embed_resolver/tweet.rb
@@ -2,18 +2,22 @@ module ArticleJSON
   module Utils
     module OEmbedResolver
       class Tweet < Base
+        # Human readable name of the resolver
+        # @return [String]
+        def name
+          'Tweet'
+        end
+
         # The URL for the oembed API call
         # @return [String]
         def oembed_url
           'https://api.twitter.com/1/statuses/oembed.json?align=center' \
-            "&url=#{tweet_url}"
+            "&url=#{source_url}"
         end
 
-        private
-
         # The URL of the tweet
         # @return [String]
-        def tweet_url
+        def source_url
           handle, tweet_id = @element.embed_id.split('/', 2)
           "https://twitter.com/#{handle}/status/#{tweet_id}"
         end
diff --git a/lib/article_json/utils/o_embed_resolver/vimeo_video.rb b/lib/article_json/utils/o_embed_resolver/vimeo_video.rb
index 4d7c054..1242162 100644
--- a/lib/article_json/utils/o_embed_resolver/vimeo_video.rb
+++ b/lib/article_json/utils/o_embed_resolver/vimeo_video.rb
@@ -2,17 +2,21 @@ module ArticleJSON
   module Utils
     module OEmbedResolver
       class VimeoVideo < Base
+        # Human readable name of the resolver
+        # @return [String]
+        def name
+          'Vimeo video'
+        end
+
         # The URL for the oembed API call
         # @return [String]
         def oembed_url
-          "https://vimeo.com/api/oembed.json?url=#{video_url}"
+          "https://vimeo.com/api/oembed.json?url=#{source_url}"
         end
 
-        private
-
         # The video URL of the element
         # @return [String]
-        def video_url
+        def source_url
           "https://vimeo.com/#{@element.embed_id}"
         end
       end
diff --git a/lib/article_json/utils/o_embed_resolver/youtube_video.rb b/lib/article_json/utils/o_embed_resolver/youtube_video.rb
index 9a346fa..ed5967c 100644
--- a/lib/article_json/utils/o_embed_resolver/youtube_video.rb
+++ b/lib/article_json/utils/o_embed_resolver/youtube_video.rb
@@ -2,17 +2,21 @@ module ArticleJSON
   module Utils
     module OEmbedResolver
       class YoutubeVideo < Base
+        # Human readable name of the resolver
+        # @return [String]
+        def name
+          'Youtube video'
+        end
+
         # The URL for the oembed API call
         # @return [String]
         def oembed_url
-          "http://www.youtube.com/oembed?format=json&url=#{video_url}"
+          "http://www.youtube.com/oembed?format=json&url=#{source_url}"
         end
 
-        private
-
         # The video URL of the element
         # @return [String]
-        def video_url
+        def source_url
           "https://www.youtube.com/watch?v=#{@element.embed_id}"
         end
       end
diff --git a/spec/article_json/export/html/elements/embed_spec.rb b/spec/article_json/export/html/elements/embed_spec.rb
index ef445fe..a54a2c3 100644
--- a/spec/article_json/export/html/elements/embed_spec.rb
+++ b/spec/article_json/export/html/elements/embed_spec.rb
@@ -1,9 +1,10 @@
 describe ArticleJSON::Export::HTML::Elements::Embed do
   subject(:element) { described_class.new(source_element) }
 
+  let(:embed_type) { :something }
   let(:source_element) do
     ArticleJSON::Elements::Embed.new(
-      embed_type: :something,
+      embed_type: embed_type,
       embed_id: 666,
       caption: [ArticleJSON::Elements::Text.new(content: 'Foo Bar')],
       tags: %w(test)
@@ -12,14 +13,32 @@
 
   describe '#export' do
     subject { element.export.to_html(save_with: 0) }
-    let(:expected_html) do
-      '<figure><div class="embed">Embedded Object: something-666</div>' \
-        '<figcaption>Foo Bar</figcaption></figure>'
+
+    context 'when the endpoint successfully returns OEmbed data' do
+      let(:expected_html) do
+        '<figure><div class="embed">Embedded Object: something-666</div>' \
+          '<figcaption>Foo Bar</figcaption></figure>'
+      end
+      let(:oembed_data) { { html: 'Embedded Object: something-666' } }
+      before do
+        allow(source_element).to receive(:oembed_data).and_return(oembed_data)
+      end
+      it { should eq expected_html }
     end
-    let(:oembed_data) { { html: 'Embedded Object: something-666' } }
-    before do
-      allow(source_element).to receive(:oembed_data).and_return(oembed_data)
+
+    context 'when the endpoint does not return OEmbed data' do
+      let(:embed_type) { :youtube_video }
+      let(:expected_html) do
+        '<figure><div class="embed"><span class="unavailable-embed">'\
+        'The Youtube video <a href="https://www.youtube.com/watch?v=666">'\
+        'https://www.youtube.com/watch?v=666</a> is not available.</span>'\
+        '</div><figcaption>Foo Bar</figcaption></figure>'
+      end
+      let(:oembed_data) { { html: 'Embedded Object: something-666' } }
+      before do
+        allow(source_element).to receive(:oembed_data).and_return(nil)
+      end
+      it { should eq expected_html }
     end
-    it { should eq expected_html }
   end
 end
diff --git a/spec/article_json/utils/o_embed_resolver/base_spec.rb b/spec/article_json/utils/o_embed_resolver/base_spec.rb
index 8735cae..1f750ae 100644
--- a/spec/article_json/utils/o_embed_resolver/base_spec.rb
+++ b/spec/article_json/utils/o_embed_resolver/base_spec.rb
@@ -1,6 +1,8 @@
 describe ArticleJSON::Utils::OEmbedResolver::Base do
   subject(:resolver) { described_class.new(element) }
 
+  let(:embed_id) { 'ABC' }
+  let(:embed_type) { :foobar }
   let(:element) do
     ArticleJSON::Elements::Embed.new(
       embed_type: embed_type,
@@ -12,8 +14,6 @@
   describe '#oembed_data' do
     subject { resolver.oembed_data }
 
-    let(:embed_id) { 'ABC' }
-    let(:embed_type) { :foobar }
     let(:something_resolver) { double('something resolver') }
     let(:oembed_data) { { foo: :bar } }
 
@@ -27,6 +27,39 @@
     it { should eq oembed_data }
   end
 
+  describe '#unavailable_message' do
+    subject { resolver.unavailable_message }
+    let(:name) { 'Embed type' }
+    let(:source_url) { 'https://example.com' }
+    before do
+      allow(resolver).to receive(:name).and_return(name)
+      allow(resolver).to receive(:source_url).and_return(source_url)
+    end
+    it { should_not be_nil }
+
+    it 'should return an Array of Text elements' do
+      is_expected.to be_an Array
+      is_expected.to all be_a ArticleJSON::Elements::Text
+      expect(subject.size).to eq 3
+      expect(subject[0].content).to include name
+      expect(subject[1].content).to eq source_url
+      expect(subject[1].href).to eq source_url
+      expect(subject[2].content).to include 'not available'
+    end
+  end
+
+  describe '#name' do
+    it 'should raise a NotImplementedError' do
+      expect { resolver.name }.to raise_error NotImplementedError
+    end
+  end
+
+  describe '#source_url' do
+    it 'should raise a NotImplementedError' do
+      expect { resolver.source_url }.to raise_error NotImplementedError
+    end
+  end
+
   describe '.build' do
     subject { described_class.build(element) }
 
diff --git a/spec/article_json/utils/o_embed_resolver/facebook_video_spec.rb b/spec/article_json/utils/o_embed_resolver/facebook_video_spec.rb
index 66de01a..5d62134 100644
--- a/spec/article_json/utils/o_embed_resolver/facebook_video_spec.rb
+++ b/spec/article_json/utils/o_embed_resolver/facebook_video_spec.rb
@@ -16,5 +16,6 @@
     let(:oembed_response) do
       File.read('spec/fixtures/facebook_video_oembed.json')
     end
+    let(:expected_name) { 'Facebook video' }
   end
 end
diff --git a/spec/article_json/utils/o_embed_resolver/oembed_resolver_shared.rb b/spec/article_json/utils/o_embed_resolver/oembed_resolver_shared.rb
index a5eeedd..9ea33d2 100644
--- a/spec/article_json/utils/o_embed_resolver/oembed_resolver_shared.rb
+++ b/spec/article_json/utils/o_embed_resolver/oembed_resolver_shared.rb
@@ -1,6 +1,12 @@
 shared_context 'for a successful oembed resolution' do
   subject(:resolver) { described_class.new(element) }
 
+  describe '#name' do
+    subject { resolver.name }
+    it { should be_a String }
+    it { should eq expected_name }
+  end
+
   describe '#oembed_url' do
     subject { resolver.oembed_url }
     it { should eq expected_oembed_url }
@@ -32,4 +38,10 @@
       it { should eq nil }
     end
   end
+
+  describe '#source_url' do
+    subject { resolver.source_url }
+    it { should be_a String }
+    it { should start_with 'http' }
+  end
 end
diff --git a/spec/article_json/utils/o_embed_resolver/slideshare_spec.rb b/spec/article_json/utils/o_embed_resolver/slideshare_spec.rb
index e2c1198..efb1166 100644
--- a/spec/article_json/utils/o_embed_resolver/slideshare_spec.rb
+++ b/spec/article_json/utils/o_embed_resolver/slideshare_spec.rb
@@ -14,5 +14,6 @@
         'https://www.slideshare.net/Devex/the-best-global-development-quotes-of-2012'
     end
     let(:oembed_response) { File.read('spec/fixtures/slideshare_oembed.json') }
+    let(:expected_name) { 'Slideshare deck' }
   end
 end
diff --git a/spec/article_json/utils/o_embed_resolver/tweet_spec.rb b/spec/article_json/utils/o_embed_resolver/tweet_spec.rb
index 111d4c6..9d0ddf9 100644
--- a/spec/article_json/utils/o_embed_resolver/tweet_spec.rb
+++ b/spec/article_json/utils/o_embed_resolver/tweet_spec.rb
@@ -16,5 +16,6 @@
     let(:oembed_response) do
       File.read('spec/fixtures/tweet_oembed.json')
     end
+    let(:expected_name) { 'Tweet' }
   end
 end
diff --git a/spec/article_json/utils/o_embed_resolver/vimeo_video_spec.rb b/spec/article_json/utils/o_embed_resolver/vimeo_video_spec.rb
index 13c5db8..5e6f932 100644
--- a/spec/article_json/utils/o_embed_resolver/vimeo_video_spec.rb
+++ b/spec/article_json/utils/o_embed_resolver/vimeo_video_spec.rb
@@ -13,5 +13,6 @@
       'https://vimeo.com/api/oembed.json?url=https://vimeo.com/42315417'
     end
     let(:oembed_response) { File.read('spec/fixtures/vimeo_video_oembed.json') }
+    let(:expected_name) { 'Vimeo video' }
   end
 end
diff --git a/spec/article_json/utils/o_embed_resolver/youtube_video_spec.rb b/spec/article_json/utils/o_embed_resolver/youtube_video_spec.rb
index a43741e..cbbe86e 100644
--- a/spec/article_json/utils/o_embed_resolver/youtube_video_spec.rb
+++ b/spec/article_json/utils/o_embed_resolver/youtube_video_spec.rb
@@ -16,5 +16,6 @@
     let(:oembed_response) do
       File.read('spec/fixtures/youtube_video_oembed.json')
     end
+    let(:expected_name) { 'Youtube video' }
   end
 end