diff --git a/app/controllers/clicks_controller.rb b/app/controllers/clicks_controller.rb index 3bc093f01bd..8472c9e7ff2 100644 --- a/app/controllers/clicks_controller.rb +++ b/app/controllers/clicks_controller.rb @@ -7,15 +7,15 @@ class ClicksController < ApplicationController if params[:topic_id].present? || params[:post_id].present? params.merge!({ user_id: current_user.id }) if current_user.present? - TopicLinkClick.create_from(params) + @redirect_url = TopicLinkClick.create_from(params) end # 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' + if params[:redirect] == 'false' || @redirect_url.blank? render nothing: true else - redirect_to(params[:url]) + redirect_to(@redirect_url) end end diff --git a/spec/controllers/clicks_controller_spec.rb b/spec/controllers/clicks_controller_spec.rb index 1c127d92299..a28d575c30d 100644 --- a/spec/controllers/clicks_controller_spec.rb +++ b/spec/controllers/clicks_controller_spec.rb @@ -6,44 +6,53 @@ describe ClicksController do context 'missing params' do it 'raises an error without the url param' do - lambda { xhr :get, :track, post_id: 123 }.should raise_error(ActionController::ParameterMissing) + lambda { xhr :get, :track, post_id: 123 }.should raise_error(ActionController::ParameterMissing) end it "redirects to the url even without the topic_id or post_id params" do xhr :get, :track, url: 'http://google.com' - response.should redirect_to("http://google.com") + response.should_not be_redirect end - end context 'correct params' do + let(:url) { "http://discourse.org" } before do request.stubs(:remote_ip).returns('192.168.0.1') end + context "with a made up url" do + it "doesn't redirect" do + TopicLinkClick.expects(:create_from).returns(nil) + xhr :get, :track, url: 'http://discourse.org', post_id: 123 + response.should_not be_redirect + end + + end + context 'with a post_id' do it 'calls create_from' do - TopicLinkClick.expects(:create_from).with('url' => 'http://discourse.org', 'post_id' => '123', 'ip' => '192.168.0.1') - xhr :get, :track, url: 'http://discourse.org', post_id: 123 - response.should redirect_to("http://discourse.org") + TopicLinkClick.expects(:create_from).with('url' => url, 'post_id' => '123', 'ip' => '192.168.0.1').returns(url) + xhr :get, :track, url: url, post_id: 123 + response.should redirect_to(url) end it 'redirects to the url' do - TopicLinkClick.stubs(:create_from) - xhr :get, :track, url: 'http://discourse.org', post_id: 123 - response.should redirect_to("http://discourse.org") + TopicLinkClick.stubs(:create_from).returns(url) + xhr :get, :track, url: url, post_id: 123 + response.should redirect_to(url) end it 'will pass the user_id to create_from' do - TopicLinkClick.expects(:create_from).with('url' => 'http://discourse.org', 'post_id' => '123', 'ip' => '192.168.0.1') - xhr :get, :track, url: 'http://discourse.org', post_id: 123 - response.should redirect_to("http://discourse.org") + TopicLinkClick.expects(:create_from).with('url' => url, 'post_id' => '123', 'ip' => '192.168.0.1').returns(url) + xhr :get, :track, url: url, post_id: 123 + response.should redirect_to(url) end it "doesn't redirect with the redirect=false param" do - TopicLinkClick.expects(:create_from).with('url' => 'http://discourse.org', 'post_id' => '123', 'ip' => '192.168.0.1', 'redirect' => 'false') - xhr :get, :track, url: 'http://discourse.org', post_id: 123, redirect: 'false' + TopicLinkClick.expects(:create_from).with('url' => url, 'post_id' => '123', 'ip' => '192.168.0.1', 'redirect' => 'false').returns(url) + xhr :get, :track, url: url, post_id: 123, redirect: 'false' response.should_not be_redirect end @@ -51,9 +60,9 @@ describe ClicksController do context 'with a topic_id' do it 'calls create_from' do - TopicLinkClick.expects(:create_from).with('url' => 'http://discourse.org', 'topic_id' => '789', 'ip' => '192.168.0.1') - xhr :get, :track, url: 'http://discourse.org', topic_id: 789 - response.should redirect_to("http://discourse.org") + TopicLinkClick.expects(:create_from).with('url' => url, 'topic_id' => '789', 'ip' => '192.168.0.1').returns(url) + xhr :get, :track, url: url, topic_id: 789 + response.should redirect_to(url) end end diff --git a/spec/models/topic_link_click_spec.rb b/spec/models/topic_link_click_spec.rb index 1580de1ac57..1ccade351fc 100644 --- a/spec/models/topic_link_click_spec.rb +++ b/spec/models/topic_link_click_spec.rb @@ -5,7 +5,6 @@ describe TopicLinkClick do it { should belong_to :topic_link } it { should belong_to :user } - it { should validate_presence_of :topic_link_id } def test_uri @@ -47,8 +46,10 @@ describe TopicLinkClick do context 'create_from' do context 'without a url' do - it "doesn't raise an exception" do - TopicLinkClick.create_from(url: "url that doesn't exist", post_id: @post.id, ip: '127.0.0.1') + let(:click) { TopicLinkClick.create_from(url: "url that doesn't exist", post_id: @post.id, ip: '127.0.0.1') } + + it "returns nil" do + click.should be_nil end end @@ -57,13 +58,12 @@ describe TopicLinkClick do lambda { TopicLinkClick.create_from(url: @topic_link.url, post_id: @post.id, ip: '127.0.0.1', user_id: @post.user_id) }.should_not change(TopicLinkClick, :count) - end end context 'with a valid url and post_id' do before do - TopicLinkClick.create_from(url: @topic_link.url, post_id: @post.id, ip: '127.0.0.1') + @url = TopicLinkClick.create_from(url: @topic_link.url, post_id: @post.id, ip: '127.0.0.1') @click = TopicLinkClick.last end @@ -75,6 +75,10 @@ describe TopicLinkClick do @click.topic_link.should == @topic_link end + it "returns the url clicked on" do + @url.should == @topic_link.url + end + context "clicking again" do it "should not record the click due to rate limiting" do -> { TopicLinkClick.create_from(url: @topic_link.url, post_id: @post.id, ip: '127.0.0.1') }.should_not change(TopicLinkClick, :count) @@ -84,7 +88,7 @@ describe TopicLinkClick do context 'with a valid url and topic_id' do before do - TopicLinkClick.create_from(url: @topic_link.url, topic_id: @topic.id, ip: '127.0.0.1') + @url = TopicLinkClick.create_from(url: @topic_link.url, topic_id: @topic.id, ip: '127.0.0.1') @click = TopicLinkClick.last end @@ -95,6 +99,10 @@ describe TopicLinkClick do it 'has the topic_link id' do @click.topic_link.should == @topic_link end + + it "returns the url linked to" do + @url.should == @topic_link.url + end end