push some work out of AR callbacks into PostCreator, add a couple of helpers for post and topic creation in test

fix it so the auto_track false marks topics as new

app/models/post.rb

@@ -263,12 +263,6 @@ class Post < ActiveRecord::Base
     PostCreator.before_create_tasks(self)
   end
 
-  # TODO: Move some of this into an asynchronous job?
-  # TODO: Move into PostCreator
-  after_create do
-    PostCreator.after_create_tasks(self)
-  end
-
   # This calculates the geometric mean of the post timings and stores it along with
   # each post.
   def self.calculate_avg_time

app/models/topic_user.rb

@@ -68,7 +68,6 @@ class TopicUser < ActiveRecord::Base
     # since there's more likely to be an existing record than not. If the update returns 0 rows affected
     # it then creates the row instead.
     def change(user_id, topic_id, attrs)
-
       # Sometimes people pass objs instead of the ids. We can handle that.
       topic_id = topic_id.id if topic_id.is_a?(::Topic)
       user_id = user_id.id if user_id.is_a?(::User)

lib/post_creator.rb

@@ -66,6 +66,7 @@ class PostCreator
       store_unique_post_key
       send_notifications_for_private_message
       track_topic
+      update_topic_stats
       update_user_counts
       publish
       @post.advance_draft_sequence
@@ -100,19 +101,6 @@ class PostCreator
     post.last_version_at ||= Time.now
   end
 
-  def self.after_create_tasks(post)
-    # Update attributes on the topic - featured users and last posted.
-    attrs = {last_posted_at: post.created_at, last_post_user_id: post.user_id}
-    attrs[:bumped_at] = post.created_at unless post.no_bump
-    post.topic.update_attributes(attrs)
-
-    # Update topic user data
-    TopicUser.change(post.user.id,
-                     post.topic.id,
-                     posted: true,
-                     last_read_post_number: post.post_number,
-                     seen_post_count: post.post_number)
-  end
 
   protected
 
@@ -184,6 +172,13 @@ class PostCreator
     @topic = topic
   end
 
+  def update_topic_stats
+    # Update attributes on the topic - featured users and last posted.
+    attrs = {last_posted_at: @post.created_at, last_post_user_id: @post.user_id}
+    attrs[:bumped_at] = @post.created_at unless @post.no_bump
+    @topic.update_attributes(attrs)
+  end
+
   def setup_post
     post = @topic.posts.new(raw: @opts[:raw],
                             user: @user,
@@ -267,6 +262,12 @@ class PostCreator
   def track_topic
     unless @opts[:auto_track] == false
       TopicUser.auto_track(@user.id, @topic.id, TopicUser.notification_reasons[:created_post])
+      # Update topic user data
+      TopicUser.change(@post.user.id,
+                       @post.topic.id,
+                       posted: true,
+                       last_read_post_number: @post.post_number,
+                       seen_post_count: @post.post_number)
     end
   end
 

spec/components/jobs/feature_topic_users_spec.rb

@@ -12,12 +12,12 @@ describe Jobs::FeatureTopicUsers do
   end
 
   context 'with a topic' do
-    let!(:post) { Fabricate(:post) }
+    let!(:post) { create_post }
     let(:topic) { post.topic }
     let!(:coding_horror) { Fabricate(:coding_horror) }
     let!(:evil_trout) { Fabricate(:evil_trout) }
-    let!(:second_post) { Fabricate(:post, topic: topic, user: coding_horror)}
-    let!(:third_post) { Fabricate(:post, topic: topic, user: evil_trout)}
+    let!(:second_post) { create_post(topic: topic, user: coding_horror)}
+    let!(:third_post) { create_post(topic: topic, user: evil_trout)}
 
     it "won't feature the OP" do
       Jobs::FeatureTopicUsers.new.execute(topic_id: topic.id)

spec/components/post_creator_spec.rb

@@ -23,8 +23,8 @@ describe PostCreator do
 
     it "can be created with auto tracking disabled" do
       p = PostCreator.create(user, basic_topic_params.merge(auto_track: false))
-      t = TopicUser.where(user_id: p.user_id, topic_id: p.topic_id).first
-      t.notification_level.should == TopicUser.notification_levels[:regular]
+      # must be 0 otherwise it will think we read the topic which is clearly untrue
+      TopicUser.where(user_id: p.user_id, topic_id: p.topic_id).count.should == 0
     end
 
     it "ensures the user can create the topic" do
@@ -154,8 +154,15 @@ describe PostCreator do
 
       it 'increases topic response counts' do
         first_post = creator.create
-        user2 = Fabricate(:coding_horror)
 
+        # ensure topic user is correct
+        topic_user = first_post.user.topic_users.where(topic_id: first_post.topic_id).first
+        topic_user.should be_present
+        topic_user.should be_posted
+        topic_user.last_read_post_number.should == first_post.post_number
+        topic_user.seen_post_count.should == first_post.post_number
+
+        user2 = Fabricate(:coding_horror)
         user2.topic_reply_count.should == 0
         first_post.user.reload.topic_reply_count.should == 0
 

spec/components/post_destroyer_spec.rb

@@ -8,7 +8,7 @@ describe PostDestroyer do
   end
 
   let(:moderator) { Fabricate(:moderator) }
-  let(:post) { Fabricate(:post) }
+  let(:post) { create_post }
 
   describe 'basic destroying' do
 
@@ -56,10 +56,10 @@ describe PostDestroyer do
   context 'deleting the second post in a topic' do
 
     let(:user) { Fabricate(:user) }
-    let!(:post) { Fabricate(:post, user: user) }
-    let(:topic) { post.topic }
+    let!(:post) { create_post(user: user) }
+    let(:topic) { post.topic.reload }
     let(:second_user) { Fabricate(:coding_horror) }
-    let!(:second_post) { Fabricate(:post, topic: topic, user: second_user) }
+    let!(:second_post) { create_post(topic: topic, user: second_user) }
 
     before do
       PostDestroyer.new(moderator, second_post).destroy

spec/components/topic_query_spec.rb

@@ -230,7 +230,7 @@ describe TopicQuery do
     end
 
     context 'created topics' do
-      let!(:created_topic) { Fabricate(:post, user: user).topic }
+      let!(:created_topic) { create_post(user: user).topic }
 
       it "includes the created topic" do
         topics.include?(created_topic).should be_true
@@ -238,8 +238,8 @@ describe TopicQuery do
     end
 
     context "topic you've posted in" do
-      let(:other_users_topic) { Fabricate(:post, user: creator).topic }
-      let!(:your_post) { Fabricate(:post, user: user, topic: other_users_topic )}
+      let(:other_users_topic) { create_post(user: creator).topic }
+      let!(:your_post) { create_post(user: user, topic: other_users_topic )}
 
       it "includes the posted topic" do
         topics.include?(other_users_topic).should be_true

spec/components/topic_view_spec.rb

@@ -3,7 +3,7 @@ require 'topic_view'
 
 describe TopicView do
 
-  let(:topic) { Fabricate(:topic) }
+  let(:topic) { create_topic }
   let(:coding_horror) { Fabricate(:coding_horror) }
   let(:first_poster) { topic.user }
 
@@ -109,53 +109,39 @@ describe TopicView do
       let(:path) { "/1234" }
 
       before do
-        topic.expects(:relative_url).returns(path)
-        described_class.any_instance.expects(:find_topic).with(1234).returns(topic)
+        topic.stubs(:relative_url).returns(path)
+        TopicView.any_instance.stubs(:find_topic).with(1234).returns(topic)
       end
 
-      context "without a post number" do
-        context "without a page" do
-          it "generates a canonical path for a topic" do
-            described_class.new(1234, user).canonical_path.should eql(path)
-          end
-        end
-
-        context "with a page" do
-          let(:path_with_page) { "/1234?page=5" }
-
-          it "generates a canonical path for a topic" do
-            described_class.new(1234, user, page: 5).canonical_path.should eql(path_with_page)
-          end
-        end
+      it "generates canonical path correctly" do
+        TopicView.new(1234, user).canonical_path.should eql(path)
+        TopicView.new(1234, user, page: 5).canonical_path.should eql("/1234?page=5")
       end
-      context "with a post number" do
-        let(:path_with_page) { "/1234?page=10" }
-        before { SiteSetting.stubs(:posts_per_page).returns(5) }
 
-        it "generates a canonical path for a topic" do
-          described_class.new(1234, user, post_number: 50).canonical_path.should eql(path_with_page)
-        end
+      it "generates a canonical correctly for paged results" do
+        SiteSetting.stubs(:posts_per_page).returns(5)
+        TopicView.new(1234, user, post_number: 50).canonical_path.should eql("/1234?page=10")
       end
     end
 
     describe "#next_page" do
       let(:p2) { stub(post_number: 2) }
       let(:topic) do
-        topic = Fabricate(:topic)
+        topic = create_topic
         topic.stubs(:highest_post_number).returns(5)
         topic
       end
       let(:user) { Fabricate(:user) }
 
       before do
-        described_class.any_instance.expects(:find_topic).with(1234).returns(topic)
-        described_class.any_instance.stubs(:filter_posts)
-        described_class.any_instance.stubs(:last_post).returns(p2)
+        TopicView.any_instance.expects(:find_topic).with(1234).returns(topic)
+        TopicView.any_instance.stubs(:filter_posts)
+        TopicView.any_instance.stubs(:last_post).returns(p2)
         SiteSetting.stubs(:posts_per_page).returns(2)
       end
 
       it "should return the next page" do
-        described_class.new(1234, user).next_page.should eql(2)
+        TopicView.new(1234, user).next_page.should eql(2)
       end
     end
 
@@ -183,17 +169,17 @@ describe TopicView do
     end
 
     context '.read?' do
-      it 'is unread with no logged in user' do
+      it 'tracks correctly' do
+        # anon has nothing
         TopicView.new(topic.id).read?(1).should be_false
-      end
 
-      it 'makes posts as unread by default' do
+        # random user has nothing
         topic_view.read?(1).should be_false
-      end
 
-      it 'knows a post is read when it has a PostTiming' do
-        PostTiming.create(topic: topic, user: coding_horror, post_number: 1, msecs: 1000)
-        topic_view.read?(1).should be_true
+        # a real user that just read it should have it marked
+        PostTiming.process_timings(coding_horror, topic.id, 1, [[1,1000]])
+        TopicView.new(topic.id, coding_horror).read?(1).should be_true
+        TopicView.new(topic.id, coding_horror).topic_user.should be_present
       end
     end
 
@@ -201,10 +187,6 @@ describe TopicView do
       it 'returns nil when there is no user' do
         TopicView.new(topic.id, nil).topic_user.should be_blank
       end
-
-      it 'returns a record once the user has some data' do
-        TopicView.new(topic.id, coding_horror).topic_user.should be_present
-      end
     end
 
     context '#recent_posts' do

spec/integration/spam_rules_spec.rb

@@ -19,8 +19,8 @@ describe PostAction do
     Given(:spammer)  { Fabricate(:user, trust_level: TrustLevel.levels[:newuser]) }
 
     context 'spammer post is not flagged enough times' do
-      Given!(:spam_post)  { Fabricate(:post, user: spammer) }
-      Given!(:spam_post2) { Fabricate(:post, user: spammer) }
+      Given!(:spam_post)  { create_post(user: spammer) }
+      Given!(:spam_post2) { create_post(user: spammer) }
       When                { PostAction.act(user1, spam_post, PostActionType.types[:spam]) }
       Then                { expect(spam_post.reload).to_not be_hidden }
 

spec/models/post_action_spec.rb

@@ -14,6 +14,7 @@ describe PostAction do
 
   describe "flagged_posts_report" do
     it "operates correctly" do
+      post = create_post
       PostAction.act(codinghorror, post, PostActionType.types[:spam])
       mod_message = PostAction.act(Fabricate(:user), post, PostActionType.types[:notify_moderators], message: "this is a 10")
 
@@ -30,6 +31,7 @@ describe PostAction do
   describe "messaging" do
 
     it "notify moderators integration test" do
+      post = create_post
       mod = moderator
       action = PostAction.act(codinghorror, post, PostActionType.types[:notify_moderators], message: "this is my special long message");
 
@@ -100,6 +102,7 @@ describe PostAction do
     end
 
     it "should ignore validated flags" do
+      post = create_post
       admin = Fabricate(:admin)
       PostAction.act(codinghorror, post, PostActionType.types[:off_topic])
       post.hidden.should be_false
@@ -193,7 +196,7 @@ describe PostAction do
 
     context "flag_counts_for" do
       it "returns the correct flag counts" do
-        post = Fabricate(:post)
+        post = create_post
 
         SiteSetting.stubs(:flags_required_to_hide_post).returns(7)
 
@@ -244,7 +247,7 @@ describe PostAction do
     end
 
     it 'should follow the rules for automatic hiding workflow' do
-      post = Fabricate(:post)
+      post = create_post
       u1 = Fabricate(:evil_trout)
       u2 = Fabricate(:walter_white)
       admin = Fabricate(:admin) # we need an admin for the messages

spec/models/post_spec.rb

@@ -570,19 +570,6 @@ describe Post do
       post.replies.should be_blank
     end
 
-    describe 'a forum topic user record for the topic' do
-
-      let(:topic_user) { post.user.topic_users.where(topic_id: topic.id).first }
-
-      it 'is set correctly' do
-        topic_user.should be_present
-        topic_user.should be_posted
-        topic_user.last_read_post_number.should == post.post_number
-        topic_user.seen_post_count.should == post.post_number
-      end
-
-    end
-
     describe 'extract_quoted_post_numbers' do
 
       let!(:post) { Fabricate(:post, post_args) }

spec/models/topic_spec.rb

@@ -339,7 +339,7 @@ describe Topic do
     it 'updates the bumped_at field when a new post is made' do
       @topic.bumped_at.should be_present
       lambda {
-        Fabricate(:post, topic: @topic, user: @topic.user)
+        create_post(topic: @topic, user: @topic.user)
         @topic.reload
       }.should change(@topic, :bumped_at)
     end
@@ -621,8 +621,8 @@ describe Topic do
   context 'last_poster info' do
 
     before do
-      @user = Fabricate(:user)
-      @post = Fabricate(:post, user: @user)
+      @post = create_post
+      @user = @post.user
       @topic = @post.topic
     end
 
@@ -633,7 +633,7 @@ describe Topic do
     context 'after a second post' do
       before do
         @second_user = Fabricate(:coding_horror)
-        @new_post = Fabricate(:post, topic: @topic, user: @second_user)
+        @new_post = create_post(topic: @topic, user: @second_user)
         @topic.reload
       end
 

spec/models/topic_tracking_state_spec.rb

@@ -7,7 +7,7 @@ describe TopicTrackingState do
   end
 
   let(:post) do
-    Fabricate(:post)
+    create_post
   end
 
   it "can correctly publish unread" do
@@ -39,7 +39,7 @@ describe TopicTrackingState do
     TopicTrackingState.report([user.id], post.topic_id + 1).should be_empty
 
     # when we reply the poster should have an unread row
-    Fabricate(:post, user: user, topic: post.topic)
+    create_post(user: user, topic: post.topic)
 
     report = TopicTrackingState.report([post.user_id, user.id])
     report.length.should == 1

spec/models/topic_user_spec.rb

@@ -232,6 +232,7 @@ describe TopicUser do
   it "is able to self heal" do
     p1 = Fabricate(:post)
     p2 = Fabricate(:post, user: p1.user, topic: p1.topic, post_number: 2)
+    p1.topic.notifier.watch_topic!(p1.user_id)
 
     TopicUser.exec_sql("UPDATE topic_users set seen_post_count=100, last_read_post_number=0
                        WHERE topic_id = :topic_id AND user_id = :user_id", topic_id: p1.topic_id, user_id: p1.user_id)

spec/spec_helper.rb

@@ -12,6 +12,11 @@ require 'fakeweb'
 FakeWeb.allow_net_connect = false
 
 module Helpers
+
+  def self.next_seq
+    @next_seq = (@next_seq || 0) + 1
+  end
+
   def log_in(fabricator=nil)
     user = Fabricate(fabricator || :user)
     log_in_user(user)
@@ -126,6 +131,21 @@ def build(*args)
   Fabricate.build(*args)
 end
 
+def create_topic(args={})
+  args[:title] ||= "This is my title #{Helpers.next_seq}"
+  user = args.delete(:user) || Fabricate(:user)
+  guardian = Guardian.new(user)
+  TopicCreator.create(user, guardian, args)
+end
+
+def create_post(args={})
+  args[:title] ||= "This is my title #{Helpers.next_seq}"
+  args[:raw] ||= "This is the raw body of my post, it is cool #{Helpers.next_seq}"
+  args[:topic_id] = args[:topic].id if args[:topic]
+  user = args.delete(:user) || Fabricate(:user)
+  PostCreator.create(user, args)
+end
+
 module MessageBus::DiagnosticsHelper
   def publish(channel, data, opts = nil)
     id = super(channel, data, opts)