store ip address and current user with incoming links

make links long an readable in share dialog

Author: SamSaffron

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'));

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');

app/controllers/application_controller.rb

@@ -240,7 +240,7 @@ def requires_parameters(*required)
     alias :requires_parameter :requires_parameters
 
     def store_incoming_links
-      IncomingLink.add(request)
+      IncomingLink.add(request,current_user) unless request.xhr?
     end
 
     def check_xhr

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
 

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
 

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 @@ def update_link_counts
       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

config/routes.rb

@@ -130,7 +130,6 @@
   end
 
   get 'p/:post_id/:user_id' => 'posts#short_link'
-  get 'st/:topic_id/:user_id' => 'topics#short_link'
 
   resources :notifications
   resources :categories

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

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

spec/models/incoming_link_spec.rb

@@ -5,9 +5,6 @@
   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 @@
   end
 
   describe 'add' do
+    class TestRequest<Rack::Request
+      attr_accessor :remote_ip
+    end
+    def req(url, referer=nil)
+      env = Rack::MockRequest.env_for(url)
+      env['HTTP_REFERER'] = referer if referer
+      TestRequest.new(env)
+    end
+
     it "does nothing if referer is empty" do
-      env = Rack::MockRequest.env_for("http://somesite.com")
-      request = Rack::Request.new(env)
       IncomingLink.expects(:create).never
-      IncomingLink.add(request)
+      IncomingLink.add(req('http://somesite.com'))
     end
 
     it "does nothing if referer is same as host" do
-      env = Rack::MockRequest.env_for("http://somesite.com")
-      env['HTTP_REFERER'] = 'http://somesite.com'
-      request = Rack::Request.new(env)
       IncomingLink.expects(:create).never
-      IncomingLink.add(request)
+      IncomingLink.add(req('http://somesite.com', 'http://somesite.com'))
     end
 
     it "expects to be called with referer and user id" do
-      env = Rack::MockRequest.env_for("http://somesite.com")
-      env['HTTP_REFERER'] = 'http://some.other.site.com'
-      request = Rack::Request.new(env)
       IncomingLink.expects(:create).once.returns(true)
-      IncomingLink.add(request, 100)
+      IncomingLink.add(req('http://somesite.com', 'http://some.other.site.com'), build(:user))
+    end
+
+    it "is able to look up user_id and log it from the GET params" do
+      user = Fabricate(:user, username: "Bob")
+      IncomingLink.add(req('http://somesite.com?u=bob'))
+      first = IncomingLink.first
+      first.user_id.should == user.id
     end
   end