Merge pull request #1217 from ZogStriP/fix-click-tracking-on-attachments

FIX: click tracking on attachments wasn't working

app/models/topic_link.rb

@@ -102,7 +102,10 @@ class TopicLink < ActiveRecord::Base
           internal = false
           topic_id = nil
           post_number = nil
-          if parsed.host == Discourse.current_hostname || !parsed.host
+
+          if Upload.has_been_uploaded?(url)
+            internal = !Upload.is_on_s3?(url)
+          elsif parsed.host == Discourse.current_hostname || !parsed.host
             internal = true
 
             route = Rails.application.routes.recognize_path(parsed.path)

app/models/upload.rb

@@ -91,7 +91,7 @@ class Upload < ActiveRecord::Base
   end
 
   def self.is_relative?(url)
-    (url =~ /^\/[^\/]/) == 0
+    url.start_with?("/uploads/")
   end
 
   def self.is_local?(url)

spec/models/topic_link_spec.rb

@@ -176,6 +176,42 @@ describe TopicLink do
       end
     end
 
+    context "link to a local attachments" do
+      let(:post) { @topic.posts.create(user: @user, raw: '<a class="attachment" href="/uploads/default/208/87bb3d8428eb4783.rb">ruby.rb</a>') }
+
+      it "extracts the link" do
+        TopicLink.extract_from(post)
+        link = @topic.topic_links.first
+        # extracted the link
+        link.should be_present
+        # is set to internal
+        link.should be_internal
+        # has the correct url
+        link.url.should == "/uploads/default/208/87bb3d8428eb4783.rb"
+        # should not be the reflection
+        link.should_not be_reflection
+      end
+
+    end
+
+    context "link to an attachments uploaded on S3" do
+      let(:post) { @topic.posts.create(user: @user, raw: '<a class="attachment" href="//s3.amazonaws.com/bucket/2104a0211c9ce41ed67989a1ed62e9a394c1fbd1446.rb">ruby.rb</a>') }
+
+      it "extracts the link" do
+        TopicLink.extract_from(post)
+        link = @topic.topic_links.first
+        # extracted the link
+        link.should be_present
+        # is not internal
+        link.should_not be_internal
+        # has the correct url
+        link.url.should == "//s3.amazonaws.com/bucket/2104a0211c9ce41ed67989a1ed62e9a394c1fbd1446.rb"
+        # should not be the reflection
+        link.should_not be_reflection
+      end
+
+    end
+
   end
 
   describe 'internal link from pm' do

spec/models/upload_spec.rb

@@ -153,7 +153,7 @@ describe Upload do
     it "identifies internal or relatives urls" do
       Discourse.expects(:base_url_no_prefix).returns("http://discuss.site.com")
       Upload.has_been_uploaded?("http://discuss.site.com/uploads/default/42/0123456789ABCDEF.jpg").should == true
-      Upload.has_been_uploaded?("/upload/42/0123456789ABCDEF.jpg").should == true
+      Upload.has_been_uploaded?("/uploads/42/0123456789ABCDEF.jpg").should == true
     end
 
     it "identifies internal urls when using a CDN" do

test/javascripts/components/utilities_test.js

@@ -91,13 +91,13 @@ var getUploadMarkdown = function(filename) {
     filesize: 42,
     width: 100,
     height: 200,
-    url: "/upload/123/abcdef.ext"
+    url: "/uploads/123/abcdef.ext"
   });
 };
 
 test("getUploadMarkdown", function() {
-  ok(getUploadMarkdown("lolcat.gif") === '<img src="/upload/123/abcdef.ext" width="100" height="200">');
+  ok(getUploadMarkdown("lolcat.gif") === '<img src="/uploads/123/abcdef.ext" width="100" height="200">');
-  ok(getUploadMarkdown("important.txt") === '<a class="attachment" href="/upload/123/abcdef.ext">important.txt</a><span class="size">(42 Bytes)</span>');
+  ok(getUploadMarkdown("important.txt") === '<a class="attachment" href="/uploads/123/abcdef.ext">important.txt</a><span class="size">(42 Bytes)</span>');
 });
 
 test("isAnImage", function() {