FIX: show an error page on click tracking error

app/assets/stylesheets/common/base/redirection.scss

@@ -0,0 +1,6 @@
+.redirection-warning {
+  margin: 0 auto;
+  text-align: center;
+  font-size: $font-up-4;
+  line-height: $line-height-medium;
+}

app/controllers/clicks_controller.rb

@@ -1,4 +1,5 @@
 class ClicksController < ApplicationController
+  layout "no_ember"
 
   skip_before_action :check_xhr, :preload_json
 
@@ -19,10 +20,12 @@ class ClicksController < ApplicationController
 
     # Sometimes we want to record a link without a 302.
     # Since XHR has to load the redirected URL we want it to not return a 302 in those cases.
-    if params[:redirect] == "false" || @redirect_url.blank?
+    if params[:redirect] == "false"
       render body: nil
-    else
+    elsif @redirect_url.present?
       redirect_to(@redirect_url)
+    else
+      render
     end
   end
 

app/views/clicks/track.html.erb

@@ -0,0 +1,4 @@
+<div class="container redirection-warning">
+  <p><%= I18n.t("redirect_warning") %></p>
+  <p><%= link_to params[:url], params[:url] %></p>
+</div>

config/locales/server.en.yml

@@ -50,11 +50,11 @@ en:
   powered_by_html: 'Powered by <a href="https://www.discourse.org">Discourse</a>, best viewed with JavaScript enabled'
   log_in: "Log In"
   submit: "Submit"
-
   purge_reason: "Automatically deleted as abandoned, deactivated account"
   disable_remote_images_download_reason: "Remote images download was disabled because there wasn't enough disk space available."
   anonymous: "Anonymous"
   remove_posts_deleted_by_author: "Deleted by author"
+  redirect_warning: "Click the link below to be redirected to an external site."
 
   themes:
     bad_color_scheme: "Can not update theme, invalid color scheme"

spec/controllers/clicks_controller_spec.rb

@@ -5,15 +5,13 @@ describe ClicksController do
   context 'create' do
 
     context 'missing params' do
-      it 'raises a 404 without the url param' do
+
-        get :track, params: { post_id: 123 }, format: :json
+      it 'raises a 404 without a url' do
+        get :track, params: { post_id: 123 }
+
         expect(response).to be_not_found
       end
 
-      it "redirects to the url even without the topic_id or post_id params" do
-        get :track, params: { url: 'http://google.com' }, format: :json
-        expect(response).not_to be_redirect
-      end
     end
 
     context 'correct params' do
@@ -24,24 +22,23 @@ describe ClicksController do
       end
 
       context "with a made up url" do
+        render_views
+
         it "doesn't redirect" do
           TopicLinkClick.expects(:create_from).returns(nil)
 
-          get :track,
+          get :track, params: { url: 'http://discourse.org', post_id: 123 }
-            params: { url: 'http://discourse.org', post_id: 123 },
-            format: :json
 
           expect(response).not_to be_redirect
+          expect(response.body).to include(I18n.t("redirect_warning"))
         end
       end
 
-      context "with a query string" do
+      context "with a valid url" do
         it "redirects" do
           TopicLinkClick.expects(:create_from).with(has_entries('url' => 'http://discourse.org/?hello=123')).returns(url)
 
-          get :track, params: {
+          get :track, params: { url: 'http://discourse.org/?hello=123', post_id: 123 }
-            url: 'http://discourse.org/?hello=123', post_id: 123, format: :json
-          }
 
           expect(response).to redirect_to(url)
         end
@@ -51,7 +48,7 @@ describe ClicksController do
         it 'redirects' do
           TopicLinkClick.expects(:create_from).with('url' => url, 'post_id' => '123', 'ip' => '192.168.0.1').returns(url)
 
-          get :track, params: { url: url, post_id: 123, format: :json }
+          get :track, params: { url: url, post_id: 123 }
 
           expect(response).to redirect_to(url)
         end
@@ -60,7 +57,7 @@ describe ClicksController do
           log_in(:admin)
           whisper = Fabricate(:post, post_type: Post.types[:whisper])
 
-          get :track, params: { url: url, post_id: whisper.id, format: :json }
+          get :track, params: { url: url, post_id: whisper.id }
 
           expect(response).to redirect_to(url)
         end
@@ -68,9 +65,7 @@ describe ClicksController do
         it "doesn't redirect with the redirect=false param" do
           TopicLinkClick.expects(:create_from).with('url' => url, 'post_id' => '123', 'ip' => '192.168.0.1', 'redirect' => 'false').returns(url)
 
-          get :track, params: {
+          get :track, params: { url: url, post_id: 123, redirect: 'false' }
-            url: url, post_id: 123, redirect: 'false', format: :json
-          }
 
           expect(response).not_to be_redirect
         end
@@ -79,7 +74,9 @@ describe ClicksController do
       context 'with a topic_id' do
         it 'redirects' do
           TopicLinkClick.expects(:create_from).with('url' => url, 'topic_id' => '789', 'ip' => '192.168.0.1').returns(url)
-          get :track, params: { url: url, topic_id: 789, format: :json }
+
+          get :track, params: { url: url, topic_id: 789 }
+
           expect(response).to redirect_to(url)
         end
       end