Fix quoted links giving blank page on click (PR #3142)

[riking1]

https://meta.discourse.org/t/links-in-quotes-sometimes-not-working/24540/

GitHub

[discoursebot]

You’ve signed the CLA, riking. Thank you! This pull request is ready for review.

[SamSaffron]

app/controllers/clicks_controller.rb

@@ -5,21 +5,28 @@ class ClicksController < ApplicationController
   def track
     params = track_params.merge(ip: request.remote_ip)
 
-    if params[:topic_id].present? || params[:post_id].present?
-      params.merge!({ user_id: current_user.id }) if current_user.present?
-      @redirect_url = TopicLinkClick.create_from(params)
-
-      if @redirect_url.blank? && params[:url].index('?')
-        # Check the url without query parameters
-        params[:url].sub!(/\?.*$/, '')
+    begin
+      if params[:topic_id].present? || params[:post_id].present?
+        params.merge!({ user_id: current_user.id }) if current_user.present?
         @redirect_url = TopicLinkClick.create_from(params)
+
+        if @redirect_url.blank? && params[:url].index('?')
+          # Check the url without query parameters
+          params[:url].sub!(/\?.*$/, '')
+          @redirect_url = TopicLinkClick.create_from(params)
+        end
       end
+    rescue => e

hmmm, why is this needed?

[SamSaffron]

can I see some examples in the wild of the problem this is solving (make a meta post)

[coding-horror]

It’s this

https://meta.discourse.org/t/links-in-quotes-sometimes-not-working/24540

[riking1]

Removed

[SamSaffron]

db/migrate/20150129082106_add_from_quote_to_topic_link.rb

@@ -0,0 +1,5 @@
+class AddFromQuoteToTopicLink < ActiveRecord::Migration
+  def change
+    add_column :topic_links, :from_quote, :boolean, default: false, null: false

I dunno, this is adding a feature we don’t really need to add just to fix a bug, can we just fix bug instead?

[riking1]

There’s another problem hiding here - yt-light creates links in javascript that don’t show up in the cooked, so they also give the “link not found” white screen.

I now agree, this is the wrong way to fix it.

[SamSaffron]

I am closing this for now, let’s come up with another fix