From cfc62dadffc5bb84f9ac99d78117a589d0fc6e17 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Mon, 22 Apr 2013 17:45:03 +1000
Subject: [PATCH] speed up tests add the ability to find the first notify
 private message

---
 .../discourse/views/actions_history_view.js   | 14 ++-
 app/controllers/post_actions_controller.rb    | 11 ++-
 app/models/post.rb                            | 21 +++-
 app/models/post_action.rb                     | 13 ++-
 .../post_action_user_serializer.rb            |  8 ++
 config/locales/client.en.yml                  |  4 +-
 ...626_add_related_post_id_to_post_actions.rb |  5 +
 spec/models/post_action_spec.rb               | 42 ++++----
 spec/models/post_spec.rb                      | 98 ++++---------------
 .../templates/poll_controls.js.handlebars     |  2 +-
 10 files changed, 108 insertions(+), 110 deletions(-)
 create mode 100644 app/serializers/post_action_user_serializer.rb
 create mode 100644 db/migrate/20130422050626_add_related_post_id_to_post_actions.rb

diff --git a/app/assets/javascripts/discourse/views/actions_history_view.js b/app/assets/javascripts/discourse/views/actions_history_view.js
index 24ef5e821..11931ac13 100644
--- a/app/assets/javascripts/discourse/views/actions_history_view.js
+++ b/app/assets/javascripts/discourse/views/actions_history_view.js
@@ -27,10 +27,15 @@ Discourse.ActionsHistoryView = Discourse.View.extend({
       var actionString, iconsHtml;
       buffer.push("<div class='post-action'>");
 
+      // TODO multi line expansion for flags
+      var postUrl;
       if (c.get('users')) {
         iconsHtml = "";
         c.get('users').forEach(function(u) {
           iconsHtml += "<a href=\"" + Discourse.getURL("/users/") + (u.get('username_lower')) + "\">";
+          if (u.post_url) {
+            postUrl = postUrl || u.post_url;
+          }
           iconsHtml += Discourse.Utilities.avatarImg({
             size: 'small',
             username: u.get('username'),
@@ -38,12 +43,17 @@ Discourse.ActionsHistoryView = Discourse.View.extend({
           });
           iconsHtml += "</a>";
         });
-        buffer.push(" " + Em.String.i18n('post.actions.people.' + c.get('actionType.name_key'), { icons: iconsHtml }) + ".");
+
+        var key = 'post.actions.people.' + c.get('actionType.name_key');
+        if(postUrl) {
+          key = key + "_with_url";
+        }
+        buffer.push(" " + Em.String.i18n(key, { icons: iconsHtml, postUrl: postUrl}) + ".");
       } else {
         buffer.push("<a href='#' data-who-acted='" + (c.get('id')) + "'>" + (c.get('description')) + "</a>.");
       }
 
-      if (c.get('can_act')) {
+      if (c.get('can_act') && !c.get('actionType.is_custom_flag')) {
         actionString = Em.String.i18n("post.actions.it_too." + c.get('actionType.name_key'));
         buffer.push(" <a href='#' data-act='" + (c.get('id')) + "'>" + actionString + "</a>.");
       }
diff --git a/app/controllers/post_actions_controller.rb b/app/controllers/post_actions_controller.rb
index 49d3b6dd6..1312a22ae 100644
--- a/app/controllers/post_actions_controller.rb
+++ b/app/controllers/post_actions_controller.rb
@@ -33,10 +33,17 @@ class PostActionsController < ApplicationController
     guardian.ensure_can_see_post_actors!(@post.topic, post_action_type_id)
 
     users = User.
+              select(['null as post_url','users.id', 'users.username', 'users.username_lower', 'users.email','post_actions.related_post_id']).
               joins(:post_actions).
-              where(["post_actions.post_id = ? and post_actions.post_action_type_id = ? and post_actions.deleted_at IS NULL", @post.id, post_action_type_id]).all
+              where(['post_actions.post_id = ? and post_actions.post_action_type_id = ? and post_actions.deleted_at IS NULL', @post.id, post_action_type_id]).all
 
-    render_serialized(users, BasicUserSerializer)
+
+    urls = Post.urls(users.map{|u| u.related_post_id})
+    users.each do |u|
+      u.post_url = urls[u.related_post_id.to_i]
+    end
+
+    render_serialized(users, PostActionUserSerializer)
   end
 
   def destroy
diff --git a/app/models/post.rb b/app/models/post.rb
index ca247d53e..464e5e6cb 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -270,7 +270,26 @@ class Post < ActiveRecord::Base
   end
 
   def url
-    "/t/#{Slug.for(topic.title)}/#{topic.id}/#{post_number}"
+    Post.url(topic.title, topic.id, post_number)
+  end
+
+  def self.url(title, topic_id, post_number)
+    "/t/#{Slug.for(title)}/#{topic_id}/#{post_number}"
+  end
+
+  def self.urls(post_ids)
+    ids = post_ids.map{|u| u}
+    if ids.length > 0
+      urls = {}
+      Topic.joins(:posts).where('posts.id' => ids).
+        select(['posts.id as post_id','post_number', 'topics.title', 'topics.id']).
+      each do |t|
+        urls[t.post_id.to_i] = url(t.title, t.id, t.post_number)
+      end
+      urls
+    else 
+      {}
+    end
   end
 
   def author_readable
diff --git a/app/models/post_action.rb b/app/models/post_action.rb
index 9e0ff86d4..6793ff97c 100644
--- a/app/models/post_action.rb
+++ b/app/models/post_action.rb
@@ -6,7 +6,7 @@ class PostAction < ActiveRecord::Base
 
   include RateLimiter::OnCreateRecord
 
-  attr_accessible :post_action_type_id, :post_id, :user_id, :post, :user, :post_action_type, :message
+  attr_accessible :post_action_type_id, :post_id, :user_id, :post, :user, :post_action_type, :message, :related_post_id
 
   belongs_to :post
   belongs_to :user
@@ -89,16 +89,21 @@ class PostAction < ActiveRecord::Base
         end
       end
 
+      related_post_id = nil
       if target_usernames.present?
-        PostCreator.new(user,
+        related_post_id = PostCreator.new(user,
                               target_usernames: target_usernames,
                               archetype: Archetype.private_message,
                               subtype: subtype,
                               title: title,
                               raw: body
-                       ).create
+                       ).create.id
       end
-      create(post_id: post.id, user_id: user.id, post_action_type_id: post_action_type_id, message: message)
+      create( post_id: post.id,
+              user_id: user.id,
+              post_action_type_id: post_action_type_id,
+              message: message,
+              related_post_id: related_post_id )
     rescue ActiveRecord::RecordNotUnique
       # can happen despite being .create
       # since already bookmarked
diff --git a/app/serializers/post_action_user_serializer.rb b/app/serializers/post_action_user_serializer.rb
new file mode 100644
index 000000000..19a4f6895
--- /dev/null
+++ b/app/serializers/post_action_user_serializer.rb
@@ -0,0 +1,8 @@
+class PostActionUserSerializer < BasicUserSerializer
+  attributes :post_url
+
+  # reserved
+  def post_url
+    object.post_url
+  end
+end
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 0569f14ad..7e94d7663 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -622,8 +622,10 @@ en:
           off_topic: "{{icons}} marked this as off-topic"
           spam: "{{icons}} marked this as spam"
           inappropriate: "{{icons}} marked this as inappropriate"
-          notify_moderators: "{{icons}} flagged this"
+          notify_moderators: "{{icons}} notified moderators"
+          notify_moderators_with_url: "{{icons}} <a href='{{postUrl}}'>notified moderators</a>"
           notify_user: "{{icons}} started a private conversation"
+          notify_user_with_url: "{{icons}} started a <a href='{{postUrl}}'>private conversation</a>"
           bookmark: "{{icons}} bookmarked this"
           like: "{{icons}} liked this"
           vote: "{{icons}} voted for this"
diff --git a/db/migrate/20130422050626_add_related_post_id_to_post_actions.rb b/db/migrate/20130422050626_add_related_post_id_to_post_actions.rb
new file mode 100644
index 000000000..7f3bbcf16
--- /dev/null
+++ b/db/migrate/20130422050626_add_related_post_id_to_post_actions.rb
@@ -0,0 +1,5 @@
+class AddRelatedPostIdToPostActions < ActiveRecord::Migration
+  def change
+    add_column :post_actions, :related_post_id, :integer
+  end
+end
diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb
index f76de8630..7a341ea93 100644
--- a/spec/models/post_action_spec.rb
+++ b/spec/models/post_action_spec.rb
@@ -18,6 +18,23 @@ describe PostAction do
   let(:bookmark) { PostAction.new(user_id: post.user_id, post_action_type_id: PostActionType.types[:bookmark] , post_id: post.id) }
 
   describe "messaging" do
+
+    it "notify moderators integration test" do 
+      mod = moderator
+      action = PostAction.act(codinghorror, post, PostActionType.types[:notify_moderators], "this is my special long message");
+
+      posts = Post.
+                  joins(:topic).
+                  select('posts.id, topics.subtype').
+                  where('topics.archetype' => Archetype.private_message).
+                  to_a
+
+      posts.count.should == 1
+      action.related_post_id.should == posts[0].id.to_i
+      posts[0].subtype.should == TopicSubtype.notify_moderators
+
+    end
+
     describe 'notify_moderators' do
       before do
         PostAction.stubs(:create)
@@ -25,14 +42,8 @@ describe PostAction do
       end
 
       it "sends an email to all moderators if selected" do
-        PostCreator.any_instance.expects(:create).returns(nil)
-        PostAction.act(build(:user), build(:post), PostActionType.types[:notify_moderators], "this is my special message");
-      end
-
-      it "uses the correct topic subtype" do
-        PostCreator.expects(:new).with do |user, opts|
-          opts[:subtype] == TopicSubtype.notify_moderators
-        end.returns(stub_everything)
+        post = build(:post, id: 1000)
+        PostCreator.any_instance.expects(:create).returns(post)
         PostAction.act(build(:user), build(:post), PostActionType.types[:notify_moderators], "this is my special message");
       end
     end
@@ -45,14 +56,7 @@ describe PostAction do
       end
 
       it "sends an email to user if selected" do
-        PostCreator.any_instance.expects(:create).returns(nil)
-        PostAction.act(build(:user), post, PostActionType.types[:notify_user], "this is my special message");
-      end
-
-      it "uses the correct topic subtype" do
-        PostCreator.expects(:new).with do |user, opts|
-          opts[:subtype] == TopicSubtype.notify_user
-        end.returns(stub_everything)
+        PostCreator.any_instance.expects(:create).returns(build(:post))
         PostAction.act(build(:user), post, PostActionType.types[:notify_user], "this is my special message");
       end
     end
@@ -62,11 +66,10 @@ describe PostAction do
     before do
       PostAction.update_flagged_posts_count
     end
-    it "starts of with 0 flag counts" do
-      PostAction.flagged_posts_count.should == 0
-    end
 
     it "increments the numbers correctly" do
+      PostAction.flagged_posts_count.should == 0
+
       PostAction.act(codinghorror, post, PostActionType.types[:off_topic])
       PostAction.flagged_posts_count.should == 1
 
@@ -108,7 +111,6 @@ describe PostAction do
 
   end
 
-
   describe 'when a user votes for something' do
     it 'should increase the vote counts when a user likes' do
       lambda {
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index 9ac625b42..6971bdf53 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -333,18 +333,12 @@ describe Post do
 
     it 'has one version in all_versions' do
       post.all_versions.size.should == 1
+      first_version_at.should be_present
+      post.revise(post.user, post.raw).should be_false
     end
 
-    it "has an initial last_version" do
-      first_version_at.should be_present
-    end
 
     describe 'with the same body' do
-
-      it 'returns false' do
-        post.revise(post.user, post.raw).should be_false
-      end
-
       it "doesn't change cached_version" do
         lambda { post.revise(post.user, post.raw); post.reload }.should_not change(post, :cached_version)
       end
@@ -383,13 +377,7 @@ describe Post do
 
       it 'updates the cached_version' do
         post.cached_version.should == 2
-      end
-
-      it 'creates a new version' do
         post.all_versions.size.should == 2
-      end
-
-      it "updates the last_version_at" do
         post.last_version_at.to_i.should == revised_at.to_i
       end
 
@@ -445,31 +433,13 @@ describe Post do
       let(:changed_by) { Fabricate(:coding_horror) }
       let!(:result) { post.revise(changed_by, 'updated body') }
 
-      it 'returns true' do
+      it 'acts correctly' do
         result.should be_true
-      end
-
-      it 'updates the body' do
         post.raw.should == 'updated body'
-      end
-
-      it 'sets the invalidate oneboxes attribute' do
         post.invalidate_oneboxes.should == true
-      end
-
-      it 'increased the cached_version' do
         post.cached_version.should == 2
-      end
-
-      it 'has the new version in all_versions' do
         post.all_versions.size.should == 2
-      end
-
-      it 'has versions' do
         post.versions.should be_present
-      end
-
-      it "saved the user who made the change in the version" do
         post.versions.first.user.should be_present
       end
 
@@ -482,9 +452,6 @@ describe Post do
 
         it 'is a ninja edit, because the second poster posted again quickly' do
           post.cached_version.should == 2
-        end
-
-        it 'is a ninja edit, because the second poster posted again quickly' do
           post.all_versions.size.should == 2
         end
 
@@ -544,39 +511,15 @@ describe Post do
 
     let(:post) { Fabricate(:post, post_args) }
 
-    it "defaults to not user_deleted" do
+    it "has correct info set" do
       post.user_deleted?.should be_false
-    end
-
-    it 'has a post nubmer' do
       post.post_number.should be_present
-    end
-
-    it 'has an excerpt' do
       post.excerpt.should be_present
-    end
-
-    it 'is of the regular post type' do
       post.post_type.should == Post.types[:regular]
-    end
-
-    it 'has no versions' do
       post.versions.should be_blank
-    end
-
-    it 'has cooked content' do
       post.cooked.should be_present
-    end
-
-    it 'has an external id' do
       post.external_id.should be_present
-    end
-
-    it 'has no quotes' do
       post.quote_count.should == 0
-    end
-
-    it 'has no replies' do
       post.replies.should be_blank
     end
 
@@ -584,19 +527,10 @@ describe Post do
 
       let(:topic_user) { post.user.topic_users.where(topic_id: topic.id).first }
 
-      it 'exists' do
+      it 'is set correctly' do
         topic_user.should be_present
-      end
-
-      it 'has the posted flag set' do
         topic_user.should be_posted
-      end
-
-      it 'recorded the latest post as read' do
         topic_user.last_read_post_number.should == post.post_number
-      end
-
-      it 'recorded the latest post as the last seen' do
         topic_user.seen_post_count.should == post.post_number
       end
 
@@ -670,18 +604,11 @@ describe Post do
           PostCreator.new(other_user, raw: raw, topic_id: topic.id, reply_to_post_number: post.post_number).create
         end
 
-        it 'has two quotes' do
+        it 'has the correct info set' do
           multi_reply.quote_count.should == 2
-        end
-
-        it 'is a child of the parent post' do
           post.replies.include?(multi_reply).should be_true
-        end
-
-        it 'is a child of the second post quoted' do
           reply.replies.include?(multi_reply).should be_true
         end
-
       end
 
     end
@@ -723,4 +650,17 @@ describe Post do
     end
   end
 
+  describe 'urls' do
+    it 'no-ops for empty list' do 
+      Post.urls([]).should == {}
+    end
+
+    # integration test -> should move to centralized integration test 
+    it 'finds urls for posts presented' do 
+      p1 = Fabricate(:post)
+      p2 = Fabricate(:post)
+      Post.urls([p1.id, p2.id]).should == {p1.id => p1.url, p2.id => p2.url}
+    end
+  end
+
 end
diff --git a/vendor/gems/discourse_poll/vendor/assets/javascripts/discourse_poll/templates/poll_controls.js.handlebars b/vendor/gems/discourse_poll/vendor/assets/javascripts/discourse_poll/templates/poll_controls.js.handlebars
index 5373c94f1..6c20d6fe0 100644
--- a/vendor/gems/discourse_poll/vendor/assets/javascripts/discourse_poll/templates/poll_controls.js.handlebars
+++ b/vendor/gems/discourse_poll/vendor/assets/javascripts/discourse_poll/templates/poll_controls.js.handlebars
@@ -14,4 +14,4 @@
   {{#if view.post.voteAction.can_undo}}
     <a href="#" class='undo' {{action undoVote target="view.post"}}>undo</a>
   {{/if}}  
-{{/if}}
\ No newline at end of file
+{{/if}}