track incoming links, amend share link to include user

fix pm styling

app/assets/javascripts/discourse/models/post.js

@@ -8,6 +8,10 @@
 **/
 Discourse.Post = Discourse.Model.extend({
 
+  shareUrl: function(){
+    var user = Discourse.get('currentUser');
+    return '/p/' + this.get('id') + (user ? '/' + user.get('id') : '');
+  }.property('id'),
 
   new_user:(function(){
     return this.get('trust_level') === 0;

app/assets/javascripts/discourse/models/topic.js

@@ -36,13 +36,18 @@ Discourse.Topic = Discourse.Model.extend({
     }
   }).property('categoryName', 'categories'),
 
-  url: (function() {
+  shareUrl: function(){
+    var user = Discourse.get('currentUser');
+    return '/st/' + this.get('id') + (user ? '/' + user.get('id') : '');
+  }.property('id'),
+
+  url: function() {
     var slug = this.get('slug');
     if (slug.isBlank()) {
       slug = "topic";
     }
     return Discourse.getURL("/t/") + slug + "/" + (this.get('id'));
-  }).property('id', 'slug'),
+  }.property('id', 'slug'),
 
   // Helper to build a Url with a post number
   urlForPostNumber: function(postNumber) {
@@ -53,20 +58,20 @@ Discourse.Topic = Discourse.Model.extend({
     return url;
   },
 
-  lastReadUrl: (function() {
+  lastReadUrl: function() {
     return this.urlForPostNumber(this.get('last_read_post_number'));
-  }).property('url', 'last_read_post_number'),
+  }.property('url', 'last_read_post_number'),
 
-  lastPostUrl: (function() {
+  lastPostUrl: function() {
     return this.urlForPostNumber(this.get('highest_post_number'));
-  }).property('url', 'highest_post_number'),
+  }.property('url', 'highest_post_number'),
 
   // The last post in the topic
   lastPost: function() {
     return this.get('posts').last();
   },
 
-  postsChanged: (function() {
+  postsChanged: function() {
     var last, posts;
     posts = this.get('posts');
     last = posts.last();
@@ -76,12 +81,12 @@ Discourse.Topic = Discourse.Model.extend({
     });
     last.set('lastPost', true);
     return true;
-  }).observes('posts.@each', 'posts'),
+  }.observes('posts.@each', 'posts'),
 
   // The amount of new posts to display. It might be different than what the server
   // tells us if we are still asynchronously flushing our "recently read" data.
   // So take what the browser has seen into consideration.
-  displayNewPosts: (function() {
+  displayNewPosts: function() {
     var delta, highestSeen, result;
     if (highestSeen = Discourse.get('highestSeenByTopic')[this.get('id')]) {
       delta = highestSeen - this.get('last_read_post_number');
@@ -94,10 +99,10 @@ Discourse.Topic = Discourse.Model.extend({
       }
     }
     return this.get('new_posts');
-  }).property('new_posts', 'id'),
+  }.property('new_posts', 'id'),
 
   // The coldmap class for the age of the topic
-  ageCold: (function() {
+  ageCold: function() {
     var createdAt, createdAtDays, daysSinceEpoch, lastPost, nowDays;
     if (!(lastPost = this.get('last_posted_at'))) return;
     if (!(createdAt = this.get('created_at'))) return;
@@ -115,7 +120,7 @@ Discourse.Topic = Discourse.Model.extend({
       if (createdAtDays < nowDays - 14) return 'coldmap-low';
     }
     return null;
-  }).property('age', 'created_at'),
+  }.property('age', 'created_at'),
 
   viewsHeat: function() {
     var v = this.get('views');

app/assets/javascripts/discourse/templates/post.js.handlebars

@@ -32,7 +32,7 @@
         <h3 {{bindAttr class="moderator new_user"}}><a href='{{unbound usernameUrl}}'>{{breakUp username}}</a></h3>
 
         <div class='post-info'>
-          <a href='#' class='post-date' {{bindAttr data-share-url="url"}}>{{date created_at}}</a>
+          <a href='#' class='post-date' {{bindAttr data-share-url="shareUrl"}}>{{date created_at}}</a>
         </div>
         {{#if hasHistory}}
           <div class='post-info'>

app/assets/javascripts/discourse/views/post_menu_view.js

@@ -132,7 +132,7 @@ Discourse.PostMenuView = Discourse.View.extend({
   renderShare: function(post, buffer) {
     buffer.push("<button title=\"" +
                  (Em.String.i18n("post.controls.share")) +
-                 "\" data-share-url=\"" + (post.get('url')) + "\"><i class=\"icon-link\"></i></button>");
+                 "\" data-share-url=\"" + (post.get('shareUrl')) + "\"><i class=\"icon-link\"></i></button>");
   },
 
   // Reply button

app/assets/javascripts/discourse/views/topic_footer_buttons_view.js

@@ -62,7 +62,7 @@ Discourse.TopicFooterButtonsView = Ember.ContainerView.extend({
         this.addObject(Discourse.ButtonView.create({
           textKey: 'topic.share.title',
           helpKey: 'topic.share.help',
-          'data-share-url': topic.get('url'),
+          'data-share-url': topic.get('shareUrl'),
 
           renderIcon: function(buffer) {
             buffer.push("<i class='icon icon-share'></i>");

app/assets/stylesheets/application/compose.css.scss

@@ -3,6 +3,10 @@
 @import "foundation/variables";
 @import "foundation/mixins";
 
+// hack, this needs to be done cleaner
+.private-message input.span8 {
+  width: 47%;
+}
 
 .composer-popup {
 

app/controllers/application_controller.rb

@@ -240,12 +240,7 @@ class ApplicationController < ActionController::Base
     alias :requires_parameter :requires_parameters
 
     def store_incoming_links
-      if request.referer.present?
-       parsed = URI.parse(request.referer)
-        if parsed.host != request.host
-          IncomingLink.create(url: request.url, referer: request.referer[0..999])
-        end
-      end
+      IncomingLink.add(request)
     end
 
     def check_xhr

app/controllers/posts_controller.rb

@@ -4,8 +4,17 @@ require_dependency 'post_destroyer'
 class PostsController < ApplicationController
 
   # Need to be logged in for all actions here
-  before_filter :ensure_logged_in, except: [:show, :replies, :by_number]
+  before_filter :ensure_logged_in, except: [:show, :replies, :by_number, :short_link]
 
+  skip_before_filter :store_incoming_links, only: [:short_link]
+  skip_before_filter :check_xhr, only: [:short_link]
+
+  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)
+    redirect_to post.url
+  end
 
   def create
     requires_parameter(:post)

app/controllers/topics_controller.rb

@@ -19,9 +19,16 @@ class TopicsController < ApplicationController
 
   before_filter :consider_user_for_promotion, only: :show
 
-  skip_before_filter :check_xhr, only: [:avatar, :show, :feed]
+  skip_before_filter :check_xhr, only: [:avatar, :show, :feed, :short_link]
+  skip_before_filter :store_incoming_links, only: [:short_link]
   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

@@ -9,6 +9,20 @@ class IncomingLink < ActiveRecord::Base
   before_validation :extract_topic_and_post
   after_create :update_link_counts
 
+  def self.add(request, user_id = nil)
+    host, referer = nil
+
+    if request.referer.present?
+      host = URI.parse(request.referer).host
+      referer = request.referer[0..999]
+
+      if host != request.host
+        IncomingLink.create(url: request.url, referer: referer, user_id: user_id)
+      end
+    end
+
+  end
+
   # Internal: Extract the domain from link.
   def extract_domain
     if referer.present?

app/models/post.rb

@@ -270,11 +270,11 @@ class Post < ActiveRecord::Base
   end
 
   def url
-    Post.url(topic.title, topic.id, post_number)
+    Post.url(topic.slug, topic.id, post_number)
   end
 
-  def self.url(title, topic_id, post_number)
-    "/t/#{Slug.for(title)}/#{topic_id}/#{post_number}"
+  def self.url(slug, topic_id, post_number)
+    "/t/#{slug}/#{topic_id}/#{post_number}"
   end
 
   def self.urls(post_ids)
@@ -282,9 +282,9 @@ class Post < ActiveRecord::Base
     if ids.length > 0
       urls = {}
       Topic.joins(:posts).where('posts.id' => ids).
-        select(['posts.id as post_id','post_number', 'topics.title', 'topics.id']).
+        select(['posts.id as post_id','post_number', 'topics.slug', 'topics.title', 'topics.id']).
       each do |t|
-        urls[t.post_id.to_i] = url(t.title, t.id, t.post_number)
+        urls[t.post_id.to_i] = url(t.slug, t.id, t.post_number)
       end
       urls
     else

app/models/site_setting.rb

@@ -17,7 +17,7 @@ class SiteSetting < ActiveRecord::Base
   setting(:company_domain, 'www.example.com')
   setting(:api_key, '')
   client_setting(:traditional_markdown_linebreaks, false)
-  client_setting(:top_menu, 'latest|hot|new|unread|favorited|categories')
+  client_setting(:top_menu, 'latest|new|unread|favorited|categories')
   client_setting(:post_menu, 'like|edit|flag|delete|share|bookmark|reply')
   client_setting(:share_links, 'twitter|facebook|google+|email')
   client_setting(:track_external_right_clicks, false)

config/routes.rb

@@ -129,6 +129,9 @@ Discourse::Application.routes.draw do
     end
   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/20130424055025_add_user_id_to_incoming_links.rb

@@ -0,0 +1,5 @@
+class AddUserIdToIncomingLinks < ActiveRecord::Migration
+  def change
+    add_column :incoming_links, :user_id, :integer
+  end
+end

spec/controllers/posts_controller_spec.rb

@@ -3,6 +3,15 @@ require 'spec_helper'
 describe PostsController do
 
 
+  describe 'short_link' do
+    it 'logs the incoming link once' do
+      IncomingLink.expects(:add).once.returns(true)
+      p = Fabricate(:post)
+      get :short_link, post_id: p.id, user_id: 999
+      response.should be_redirect
+    end
+  end
+
   describe 'show' do
 
     let(:post) { Fabricate(:post, user: log_in) }

spec/models/incoming_link_spec.rb

@@ -8,68 +8,75 @@ describe IncomingLink do
   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
+
+  let :topic do
+    post.topic
+  end
+
+  let :incoming_link do
+    IncomingLink.create(url: "/t/slug/#{topic.id}/#{post.post_number}",
+                                             referer: "http://twitter.com")
+  end
+
   describe 'local topic link' do
 
     it 'should validate properly' do
       Fabricate.build(:incoming_link).should be_valid
     end
 
+    describe 'tracking link counts' do
+      it "increases the incoming link counts" do
+        incoming_link
+        lambda { post.reload }.should change(post, :incoming_link_count).by(1)
+        lambda { topic.reload }.should change(topic, :incoming_link_count).by(1)
+      end
+    end
+
     describe 'saving local link' do
-
-      before do
-        @post = Fabricate(:post)
-        @topic = @post.topic
-        @incoming_link = IncomingLink.create(url: "/t/slug/#{@topic.id}/#{@post.post_number}",
-                                             referer: "http://twitter.com")
+      it 'has correct info set' do
+        incoming_link.domain.should == "twitter.com"
+        incoming_link.topic_id.should == topic.id
+        incoming_link.post_number.should == post.post_number
       end
 
-      describe 'incoming link counts' do
-        it "increases the post's incoming link count" do
-          lambda { @incoming_link.save; @post.reload }.should change(@post, :incoming_link_count).by(1)
-        end
+    end
+  end
 
-        it "increases the topic's incoming link count" do
-          lambda { @incoming_link.save; @topic.reload }.should change(@topic, :incoming_link_count).by(1)
-        end
+  describe 'add' do
+    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)
+    end
 
-      end
-
-      describe 'after save' do
-        before do
-          @incoming_link.save
-        end
-
-        it 'has a domain' do
-          @incoming_link.domain.should == "twitter.com"
-        end
-
-        it 'has the topic_id' do
-          @incoming_link.topic_id.should == @topic.id
-        end
-
-        it 'has the post_number' do
-          @incoming_link.post_number.should == @post.post_number
-        end
-      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)
+    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)
     end
   end
 
   describe 'non-topic url' do
-
-    before do
-      @link = Fabricate(:incoming_link_not_topic)
-    end
-
-    it 'has no topic_id' do
-      @link.topic_id.should be_blank
-    end
-
-    it 'has no post_number' do
-      @link.topic_id.should be_blank
+    it 'has nothing set' do
+      link = Fabricate(:incoming_link_not_topic)
+      link.topic_id.should be_blank
+      link.user_id.should be_blank
     end
 
   end
 
-
 end