From 8406a4230c04d7d3d1f715db15f89343381a1302 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Fri, 19 Jul 2013 01:26:23 +0200 Subject: [PATCH] FIX: click tracking on attachments wasn't working --- app/models/topic_link.rb | 5 ++- app/models/upload.rb | 2 +- spec/models/topic_link_spec.rb | 36 +++++++++++++++++++ spec/models/upload_spec.rb | 2 +- test/javascripts/components/utilities_test.js | 6 ++-- 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index b4ada546cf7..1e86d11add4 100644 --- a/app/models/topic_link.rb +++ b/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) diff --git a/app/models/upload.rb b/app/models/upload.rb index 828eacee54a..fedb1ac4f19 100644 --- a/app/models/upload.rb +++ b/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) diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb index d2d960afb4c..0318089a154 100644 --- a/spec/models/topic_link_spec.rb +++ b/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: 'ruby.rb') } + + 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: 'ruby.rb') } + + 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 diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 268a52b01c7..f0a07fcbb63 100644 --- a/spec/models/upload_spec.rb +++ b/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 diff --git a/test/javascripts/components/utilities_test.js b/test/javascripts/components/utilities_test.js index 20b4b1f2a0f..23e3af6902c 100644 --- a/test/javascripts/components/utilities_test.js +++ b/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") === ''); - ok(getUploadMarkdown("important.txt") === 'important.txt(42 Bytes)'); + ok(getUploadMarkdown("lolcat.gif") === ''); + ok(getUploadMarkdown("important.txt") === 'important.txt(42 Bytes)'); }); test("isAnImage", function() {