FIX: do not use canonical link for Discourse topic/post

https://meta.discourse.org/t/incorrect-or-failing-oneboxes-for-links-to-other-discourse-instances/67351/7?u=techapj

lib/onebox/engine/standard_embed.rb

@@ -58,11 +58,13 @@ def html_doc
           response = (Onebox::Helpers.fetch_response(url, nil, nil, headers) rescue nil)
           doc = Nokogiri::HTML(response)
 
-          # prefer canonical link
-          canonical_link = doc.at('//link[@rel="canonical"]/@href')
-          if canonical_link && "#{URI(canonical_link).host}#{URI(canonical_link).path}" != "#{URI(url).host}#{URI(url).path}"
-            response = (Onebox::Helpers.fetch_response(canonical_link, nil, nil, headers) rescue nil)
-            doc = Nokogiri::HTML(response) if response
+          unless skip_canonical_link
+            # prefer canonical link
+            canonical_link = doc.at('//link[@rel="canonical"]/@href')
+            if canonical_link && "#{URI(canonical_link).host}#{URI(canonical_link).path}" != "#{URI(url).host}#{URI(url).path}"
+              response = (Onebox::Helpers.fetch_response(canonical_link, nil, nil, headers) rescue nil)
+              doc = Nokogiri::HTML(response) if response
+            end
           end
 
           @html_doc = doc
@@ -123,6 +125,9 @@ def get_twitter
           twitter
         end
 
+        def skip_canonical_link
+          WhitelistedGenericOnebox.probable_discourse(URI(url))
+        end
     end
   end
 end

lib/onebox/version.rb

@@ -1,3 +1,3 @@
 module Onebox
-  VERSION = "1.8.16"
+  VERSION = "1.8.17"
 end

spec/lib/onebox/engine/whitelisted_generic_onebox_spec.rb

@@ -97,20 +97,39 @@ def generic_html
     end
   end
 
-  describe "uses canonical link" do
-    let(:mobile_url) { "http://m.imdb.com/title/tt0944947" }
-    let(:canonical_url) { "http://www.imdb.com/title/tt0944947/" }
-    before do
-      fake(mobile_url, response('imdb_mobile'))
-      fake(canonical_url, response('imdb'))
+  describe 'canonical link' do
+    context 'uses canonical link if available' do
+      let(:mobile_url) { "http://m.imdb.com/title/tt0944947" }
+      let(:canonical_url) { "http://www.imdb.com/title/tt0944947/" }
+      before do
+        fake(mobile_url, response('imdb_mobile'))
+        fake(canonical_url, response('imdb'))
+      end
+
+      it 'fetches opengraph data from canonical link' do
+        onebox = described_class.new(mobile_url)
+        expect(onebox.to_html).not_to be_nil
+        expect(onebox.to_html).to include("Game of Thrones")
+        expect(onebox.to_html).to include("Nine noble families fight for control over the mythical lands of Westeros")
+        expect(onebox.to_html).to include("https://images-na.ssl-images-amazon.com/images/M/MV5BMjE3NTQ1NDg1Ml5BMl5BanBnXkFtZTgwNzY2NDA0MjI@._V1_UY1200_CR90,0,630,1200_AL_.jpg")
+      end
     end
 
-    it 'fetches opengraph data from canonical link' do
-      onebox = described_class.new(mobile_url)
-      expect(onebox.to_html).not_to be_nil
-      expect(onebox.to_html).to include("Game of Thrones")
-      expect(onebox.to_html).to include("Nine noble families fight for control over the mythical lands of Westeros")
-      expect(onebox.to_html).to include("https://images-na.ssl-images-amazon.com/images/M/MV5BMjE3NTQ1NDg1Ml5BMl5BanBnXkFtZTgwNzY2NDA0MjI@._V1_UY1200_CR90,0,630,1200_AL_.jpg")
+    context 'does not use canonical link for Discourse topics' do
+      let(:discourse_topic_url) { "https://meta.discourse.org/t/congratulations-most-stars-in-2013-github-octoverse/12483" }
+      let(:discourse_topic_reply_url) { "https://meta.discourse.org/t/congratulations-most-stars-in-2013-github-octoverse/12483/2" }
+      before do
+        fake(discourse_topic_url, response('discourse_topic'))
+        fake(discourse_topic_reply_url, response('discourse_topic_reply'))
+      end
+
+      it 'fetches opengraph data from original link' do
+        onebox = described_class.new(discourse_topic_reply_url)
+        expect(onebox.to_html).not_to be_nil
+        expect(onebox.to_html).to include("Congratulations, most stars in 2013 GitHub Octoverse!")
+        expect(onebox.to_html).to include("Thanks for that link and thank you -- and everyone else who is contributing to the project!")
+        expect(onebox.to_html).to include("https://cdn-enterprise.discourse.org/meta/user_avatar/meta.discourse.org/codinghorror/200/5297_1.png")
+      end
     end
   end