diff --git a/app/assets/javascripts/discourse/models/post.js b/app/assets/javascripts/discourse/models/post.js index 258588a6c..baf2f47ff 100755 --- a/app/assets/javascripts/discourse/models/post.js +++ b/app/assets/javascripts/discourse/models/post.js @@ -10,20 +10,24 @@ Discourse.Post = Discourse.Model.extend({ shareUrl: function(){ var user = Discourse.get('currentUser'); - return '/p/' + this.get('id') + (user ? '/' + user.get('id') : ''); - }.property('id'), + if (this.get('postnumber') === 1){ + return this.get('topic.url'); + } else { + return this.get('url') + (user ? '?u=' + user.get('username_lower') : ''); + } + }.property('url'), new_user:(function(){ return this.get('trust_level') === 0; }).property('trust_level'), - url: (function() { + url: function() { return Discourse.Utilities.postUrl(this.get('topic.slug') || this.get('topic_slug'), this.get('topic_id'), this.get('post_number')); - }).property('post_number', 'topic_id', 'topic.slug'), + }.property('post_number', 'topic_id', 'topic.slug'), - originalPostUrl: (function() { + originalPostUrl: function() { return Discourse.getURL("/t/") + (this.get('topic_id')) + "/" + (this.get('reply_to_post_number')); - }).property('reply_to_post_number'), + }.property('reply_to_post_number'), usernameUrl: (function() { return Discourse.getURL("/users/" + this.get('username')); diff --git a/app/assets/javascripts/discourse/models/topic.js b/app/assets/javascripts/discourse/models/topic.js index 45009941a..68334d98f 100755 --- a/app/assets/javascripts/discourse/models/topic.js +++ b/app/assets/javascripts/discourse/models/topic.js @@ -30,16 +30,16 @@ Discourse.Topic = Discourse.Model.extend({ } }, - category: (function() { + category: function() { if (this.get('categories')) { return this.get('categories').findProperty('name', this.get('categoryName')); } - }).property('categoryName', 'categories'), + }.property('categoryName', 'categories'), shareUrl: function(){ var user = Discourse.get('currentUser'); - return '/st/' + this.get('id') + (user ? '/' + user.get('id') : ''); - }.property('id'), + return this.get('url') + (user ? '?u=' + user.get('username_lower') : ''); + }.property('url'), url: function() { var slug = this.get('slug'); diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 52f506f8f..280827dd8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -240,7 +240,7 @@ class ApplicationController < ActionController::Base alias :requires_parameter :requires_parameters def store_incoming_links - IncomingLink.add(request) + IncomingLink.add(request,current_user) unless request.xhr? end def check_xhr diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 1fc6376eb..3bb93c099 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -11,8 +11,7 @@ class PostsController < ApplicationController def short_link post = Post.find(params[:post_id].to_i) - user = User.select(:id).where(id: params[:user_id].to_i).first - IncomingLink.add(request, user ? user.id : nil) + IncomingLink.add(request,current_user) redirect_to post.url end diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 88626c313..74014b41a 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -19,17 +19,9 @@ class TopicsController < ApplicationController before_filter :consider_user_for_promotion, only: :show - skip_before_filter :check_xhr, only: [:avatar, :show, :feed, :short_link] - skip_before_filter :store_incoming_links, only: [:short_link] + skip_before_filter :check_xhr, only: [:avatar, :show, :feed] caches_action :avatar, cache_path: Proc.new {|c| "#{c.params[:post_number]}-#{c.params[:topic_id]}" } - def short_link - topic = Topic.find(params[:topic_id].to_i) - user = User.select(:id).where(id: params[:user_id].to_i).first - IncomingLink.add(request, user ? user.id : nil) - redirect_to topic.relative_url - end - def show create_topic_view diff --git a/app/models/incoming_link.rb b/app/models/incoming_link.rb index f10f606a5..5bd850d5c 100644 --- a/app/models/incoming_link.rb +++ b/app/models/incoming_link.rb @@ -1,28 +1,41 @@ class IncomingLink < ActiveRecord::Base belongs_to :topic - validates :domain, length: { in: 1..100 } - validates :referer, length: { in: 3..1000 } validates :url, presence: true + validate :referer_valid + before_validation :extract_domain before_validation :extract_topic_and_post after_create :update_link_counts - def self.add(request, user_id = nil) - host, referer = nil + def self.add(request,current_user=nil) + user_id, host, referer = nil + + if request['u'] + u = User.select(:id).where(username_lower: request['u'].downcase).first + user_id = u.id if u + end if request.referer.present? host = URI.parse(request.referer).host referer = request.referer[0..999] + end - if host != request.host - IncomingLink.create(url: request.url, referer: referer, user_id: user_id) + if host != request.host && (user_id || referer) + cid = current_user.id if current_user + unless cid && cid == user_id + IncomingLink.create(url: request.url, + referer: referer, + user_id: user_id, + current_user_id: cid, + ip_address: request.remote_ip) end end end + # Internal: Extract the domain from link. def extract_domain if referer.present? @@ -58,4 +71,17 @@ class IncomingLink < ActiveRecord::Base end end end + + protected + + def referer_valid + return true unless referer + if (referer.length < 3 || referer.length > 100) || (domain.length < 1 || domain.length > 100) + # internal, no need to localize + errors.add(:referer, 'referer is invalid') + false + else + true + end + end end diff --git a/config/routes.rb b/config/routes.rb index 408b4bcd9..a770a87c4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -130,7 +130,6 @@ Discourse::Application.routes.draw do end get 'p/:post_id/:user_id' => 'posts#short_link' - get 'st/:topic_id/:user_id' => 'topics#short_link' resources :notifications resources :categories diff --git a/db/migrate/20130426044914_allow_nulls_in_incoming_links.rb b/db/migrate/20130426044914_allow_nulls_in_incoming_links.rb new file mode 100644 index 000000000..5463394c8 --- /dev/null +++ b/db/migrate/20130426044914_allow_nulls_in_incoming_links.rb @@ -0,0 +1,6 @@ +class AllowNullsInIncomingLinks < ActiveRecord::Migration + def change + change_column :incoming_links, :referer, :string, limit:1000, null: true + change_column :incoming_links, :domain, :string, limit:100, null: true + end +end diff --git a/db/migrate/20130426052257_add_incoming_ip_current_user_id_to_incoming_links.rb b/db/migrate/20130426052257_add_incoming_ip_current_user_id_to_incoming_links.rb new file mode 100644 index 000000000..bfc8eb830 --- /dev/null +++ b/db/migrate/20130426052257_add_incoming_ip_current_user_id_to_incoming_links.rb @@ -0,0 +1,6 @@ +class AddIncomingIpCurrentUserIdToIncomingLinks < ActiveRecord::Migration + def change + add_column :incoming_links, :ip_address, :inet + add_column :incoming_links, :current_user_id, :int + end +end diff --git a/spec/models/incoming_link_spec.rb b/spec/models/incoming_link_spec.rb index ae6ea2202..8eaf04b1d 100644 --- a/spec/models/incoming_link_spec.rb +++ b/spec/models/incoming_link_spec.rb @@ -5,9 +5,6 @@ describe IncomingLink do it { should belong_to :topic } it { should validate_presence_of :url } - it { should ensure_length_of(:referer).is_at_least(3).is_at_most(1000) } - it { should ensure_length_of(:domain).is_at_least(1).is_at_most(100) } - let :post do Fabricate(:post) end @@ -46,27 +43,35 @@ describe IncomingLink do end describe 'add' do + class TestRequest