From 4bd5acec471252d29f590f744de8588120cafb7c Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Mon, 11 Dec 2017 16:27:33 +0800
Subject: [PATCH] FIX: `Topic#featured_link` may contain more than a URL.

---
 app/jobs/onceoff/fix_featured_link_for_topics.rb | 15 +++++++++++++++
 app/models/topic.rb                              | 11 ++---------
 spec/models/topic_spec.rb                        | 16 ++++++++++++++++
 3 files changed, 33 insertions(+), 9 deletions(-)
 create mode 100644 app/jobs/onceoff/fix_featured_link_for_topics.rb

diff --git a/app/jobs/onceoff/fix_featured_link_for_topics.rb b/app/jobs/onceoff/fix_featured_link_for_topics.rb
new file mode 100644
index 00000000000..96f23563a8c
--- /dev/null
+++ b/app/jobs/onceoff/fix_featured_link_for_topics.rb
@@ -0,0 +1,15 @@
+module Jobs
+  class FixFeaturedLinkForTopics < Jobs::Onceoff
+    def execute_onceoff(args)
+      Topic.where("featured_link IS NOT NULL").find_each do |topic|
+        featured_link = topic.featured_link
+
+        begin
+          URI.parse(featured_link)
+        rescue URI::InvalidURIError
+          topic.update!(featured_link: URI.extract(featured_link).first)
+        end
+      end
+    end
+  end
+end
diff --git a/app/models/topic.rb b/app/models/topic.rb
index 4293b629cf7..61000fa8a49 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -88,7 +88,7 @@ class Topic < ActiveRecord::Base
               (!t.user_id || !t.user.staff?)
             }
 
-  validates :featured_link, allow_nil: true, format: URI::regexp(%w(http https))
+  validates :featured_link, allow_nil: true, url: true
   validate if: :featured_link do
     errors.add(:featured_link, :invalid_category) unless !featured_link_changed? ||
       Guardian.new.can_edit_featured_link?(category_id)
@@ -1265,14 +1265,7 @@ SQL
   end
 
   def featured_link_root_domain
-    url = URI.extract(self.featured_link).first
-
-    begin
-      MiniSuffix.domain(URI.parse(url).hostname)
-    rescue URI::InvalidURIError => e
-      Rails.logger.warn("#{e.message}: #{e.backtrace.join("\n")}")
-      nil
-    end
+    MiniSuffix.domain(URI.parse(self.featured_link).hostname)
   end
 
   private
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index f0a8bf0f4d4..facbef73a4a 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -11,6 +11,22 @@ describe Topic do
   context 'validations' do
     let(:topic) { Fabricate.build(:topic) }
 
+    context "#featured_link" do
+      describe 'when featured_link contains more than a URL' do
+        it 'should not be valid' do
+          topic.featured_link = 'http://meta.discourse.org TEST'
+          expect(topic).to_not be_valid
+        end
+      end
+
+      describe 'when featured_link is a valid URL' do
+        it 'should be valid' do
+          topic.featured_link = 'http://meta.discourse.org'
+          expect(topic).to be_valid
+        end
+      end
+    end
+
     context "#title" do
       it { is_expected.to validate_presence_of :title }