Move a bunch of callbacks into PostCreator

app/models/post.rb

@@ -47,16 +47,6 @@ class Post < ActiveRecord::Base
   MODERATOR_ACTION = 2
 
   before_save :extract_quoted_post_numbers
-  after_commit :feature_topic_users, on: :create
-  after_commit :trigger_post_process, on: :create
-  after_commit :email_private_message, on: :create
-
-  # Related to unique post tracking
-  after_commit :store_unique_post_key, on: :create
-
-  after_create do
-    TopicUser.auto_track(user_id, topic_id, TopicUser.notification_reasons[:created_post])
-  end
 
   scope :by_newest, order('created_at desc, id desc')
   scope :with_user, includes(:user)
@@ -85,12 +75,6 @@ class Post < ActiveRecord::Base
     end
   end
 
-  # On successful post, store a hash key to prevent the same post from happening again
-  def store_unique_post_key
-    return if SiteSetting.unique_posts_mins == 0
-    $redis.setex(unique_post_key, SiteSetting.unique_posts_mins.minutes.to_i, "1")
-  end
-
   # The key we use in reddit to ensure unique posts
   def unique_post_key
     "post-#{user_id}:#{raw_hash}"
@@ -320,9 +304,6 @@ class Post < ActiveRecord::Base
     attrs[:bumped_at] = created_at unless no_bump
     topic.update_attributes(attrs)
 
-    # Update the user's last posted at date
-    user.update_column(:last_posted_at, created_at)
-
     # Update topic user data
     TopicUser.change(user,
                      topic.id,
@@ -331,19 +312,6 @@ class Post < ActiveRecord::Base
                      seen_post_count: post_number)
   end
 
-  def email_private_message
-    # send a mail to notify users in case of a private message
-    if topic.private_message?
-      topic.allowed_users.where(["users.email_private_messages = true and users.id != ?", self.user_id]).each do |u|
-        Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes, :user_email, type: :private_message, user_id: u.id, post_id: self.id)
-      end
-    end
-  end
-
-  def feature_topic_users
-    Jobs.enqueue(:feature_topic_users, topic_id: self.topic_id)
-  end
-
   # This calculates the geometric mean of the post timings and stores it along with
   # each post.
   def self.calculate_avg_time
@@ -446,7 +414,7 @@ class Post < ActiveRecord::Base
     self.quote_count = quoted_post_numbers.size
   end
 
-  # Process this post after committing it
+  # Enqueue post processing for this post
   def trigger_post_process
     args = { post_id: id }
     args[:image_sizes] = image_sizes if image_sizes.present?

lib/post_creator.rb

@@ -4,13 +4,13 @@ require_dependency 'rate_limiter'
 
 class PostCreator
 
-  # Errors when creating the post
+  attr_reader :errors, :opts
-  attr_reader :errors
 
   # Acceptable options:
   #
   #   raw                     - raw text of post
   #   image_sizes             - We can pass a list of the sizes of images in the post as a shortcut.
+  #   invalidate_oneboxes     - Whether to force invalidation of oneboxes in this post
   #
   #   When replying to a topic:
   #     topic_id              - topic we're replying to
@@ -78,6 +78,7 @@ class PostCreator
                              user: @user,
                              reply_to_post_number: @opts[:reply_to_post_number])
       post.image_sizes = @opts[:image_sizes] if @opts[:image_sizes].present?
+      post.invalidate_oneboxes = @opts[:invalidate_oneboxes] if @opts[:invalidate_oneboxes].present?
       unless post.save
         @errors = post.errors
         raise ActiveRecord::Rollback.new
@@ -85,6 +86,30 @@ class PostCreator
 
       # Extract links
       TopicLink.extract_from(post)
+
+      # Enqueue a job to feature the users in the topic
+      Jobs.enqueue(:feature_topic_users, topic_id: topic.id)
+
+      # Trigger post processing
+      post.trigger_post_process
+
+      # Store unique post key
+      if SiteSetting.unique_posts_mins > 0
+        $redis.setex(post.unique_post_key, SiteSetting.unique_posts_mins.minutes.to_i, "1")
+      end
+
+      # send a mail to notify users in case of a private message
+      if topic.private_message?
+        topic.allowed_users.where(["users.email_private_messages = true and users.id != ?", @user.id]).each do |u|
+          Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes, :user_email, type: :private_message, user_id: u.id, post_id: post.id)
+        end
+      end
+
+      # Track the topic
+      TopicUser.auto_track(@user.id, topic.id, TopicUser.notification_reasons[:created_post])
+
+      # Update `last_posted_at` to match the post's created_at
+      @user.update_column(:last_posted_at, post.created_at)
     end
 
     post

spec/components/cooked_post_processor_spec.rb

@@ -60,10 +60,14 @@ EXPECTED
     end
 
     context 'with unsized images in the post' do
+      let(:user) { Fabricate(:user) }
+      let(:topic) { Fabricate(:topic, user: user) }
+
       before do
         FastImage.stubs(:size).returns([123, 456])
         CookedPostProcessor.any_instance.expects(:image_dimensions).returns([123, 456])
-        @post = Fabricate(:post_with_images)
+        creator = PostCreator.new(user, raw: Fabricate.build(:post_with_images).raw, topic_id: topic.id)
+        @post = creator.create
       end
 
       it "adds a topic image if there's one in the post" do

spec/components/post_creator_spec.rb

@@ -38,6 +38,33 @@ describe PostCreator do
         creator.create
       end
 
+      it 'features topic users' do
+        Jobs.stubs(:enqueue).with(:process_post, anything)
+        Jobs.expects(:enqueue).with(:feature_topic_users, has_key(:topic_id))
+        creator.create
+      end
+
+      it 'queues up post processing job when saved' do
+        Jobs.stubs(:enqueue).with(:feature_topic_users, has_key(:topic_id))
+        Jobs.expects(:enqueue).with(:process_post, has_key(:post_id))
+        creator.create
+      end
+
+      it 'passes the invalidate_oneboxes along to the job if present' do
+        Jobs.stubs(:enqueue).with(:feature_topic_users, has_key(:topic_id))
+        Jobs.expects(:enqueue).with(:process_post, has_key(:invalidate_oneboxes))
+        creator.opts[:invalidate_oneboxes] = true
+        creator.create
+      end
+
+      it 'passes the image_sizes along to the job if present' do
+        Jobs.stubs(:enqueue).with(:feature_topic_users, has_key(:topic_id))
+        Jobs.expects(:enqueue).with(:process_post, has_key(:image_sizes))
+        creator.opts[:image_sizes] = {'http://an.image.host/image.jpg' => {'width' => 17, 'height' => 31}}
+        creator.create
+      end
+
+
       it 'assigns a category when supplied' do
         creator_with_category.create.topic.category.should == category
       end
@@ -54,6 +81,52 @@ describe PostCreator do
 
   end
 
+  context 'uniqueness' do
+
+    let!(:topic) { Fabricate(:topic, user: user) }
+    let(:basic_topic_params) { { raw: 'test reply', topic_id: topic.id, reply_to_post_number: 4} }
+    let(:creator) { PostCreator.new(user, basic_topic_params) }
+
+    context "disabled" do
+      before do
+        SiteSetting.stubs(:unique_posts_mins).returns(0)
+        creator.create
+      end
+
+      it "returns true for another post with the same content" do
+        new_creator = PostCreator.new(user, basic_topic_params)
+        new_creator.create.should be_present
+      end
+    end
+
+    context 'enabled' do
+      let(:new_post_creator) { PostCreator.new(user, basic_topic_params) }
+
+      before do
+        SiteSetting.stubs(:unique_posts_mins).returns(10)
+        creator.create
+      end
+
+      it "returns blank for another post with the same content" do
+        new_post_creator.create
+        new_post_creator.errors.should be_present
+      end
+
+      it "returns a post for admins" do
+        user.admin = true
+        new_post_creator.create
+        new_post_creator.errors.should be_blank
+      end
+
+      it "returns a post for moderators" do
+        user.trust_level = TrustLevel.levels[:moderator]
+        new_post_creator.create
+        new_post_creator.errors.should be_blank
+      end
+    end
+
+  end
+
   context 'existing topic' do
     let!(:topic) { Fabricate(:topic, user: user) }
     let(:creator) { PostCreator.new(user, raw: 'test reply', topic_id: topic.id, reply_to_post_number: 4) }

spec/models/post_spec.rb

@@ -66,43 +66,6 @@ describe Post do
 
   end
 
-
-  describe 'post uniqueness' do
-
-    context "disabled" do
-      before do
-        SiteSetting.stubs(:unique_posts_mins).returns(0)
-        Fabricate(:post, post_args)
-      end
-
-      it "returns true for another post with the same content" do
-        Fabricate.build(:post, post_args).should be_valid
-      end
-    end
-
-    context 'enabled' do
-      before do
-        SiteSetting.stubs(:unique_posts_mins).returns(10)
-        Fabricate(:post, post_args)
-      end
-
-      it "returns false for another post with the same content" do
-        Fabricate.build(:post, post_args).should_not be_valid
-      end
-
-      it "returns true for admins" do
-        topic.user.admin = true
-        Fabricate.build(:post, post_args).should be_valid
-      end
-
-      it "returns true for moderators" do
-        topic.user.trust_level = TrustLevel.levels[:moderator]
-        Fabricate.build(:post, post_args).should be_valid
-      end
-    end
-
-  end
-
   describe 'flagging helpers' do
     it 'isFlagged is accurate' do
       post = Fabricate(:post)
@@ -476,34 +439,6 @@ describe Post do
     end
   end
 
-  it 'should feature users after create' do
-    Jobs.stubs(:enqueue).with(:process_post, anything)
-    Jobs.expects(:enqueue).with(:feature_topic_users, has_key(:topic_id))
-    Fabricate(:post, post_args)
-  end
-
-  it 'should queue up a post processing job when saved' do
-    Jobs.stubs(:enqueue).with(:feature_topic_users, has_key(:topic_id))
-    Jobs.expects(:enqueue).with(:process_post, has_key(:post_id))
-    Fabricate(:post, post_args)
-  end
-
-  it 'passes the invalidate_oneboxes along to the job if present' do
-    Jobs.stubs(:enqueue).with(:feature_topic_users, has_key(:topic_id))
-    Jobs.expects(:enqueue).with(:process_post, has_key(:invalidate_oneboxes))
-    post = Fabricate.build(:post, post_args)
-    post.invalidate_oneboxes = true
-    post.save
-  end
-
-  it 'passes the image_sizes along to the job if present' do
-    Jobs.stubs(:enqueue).with(:feature_topic_users, has_key(:topic_id))
-    Jobs.expects(:enqueue).with(:process_post, has_key(:image_sizes))
-    post = Fabricate.build(:post, post_args)
-    post.image_sizes = {'http://an.image.host/image.jpg' => {'width' => 17, 'height' => 31}}
-    post.save
-  end
-
   describe 'notifications' do
 
     let(:coding_horror) { Fabricate(:coding_horror) }

spec/models/topic_spec.rb

@@ -432,15 +432,17 @@ describe Topic do
 
     context "other user" do
 
+      let(:creator) { PostCreator.new(topic.user, raw: Fabricate.build(:post).raw, topic_id: topic.id )}
+
       it "sends the other user an email when there's a new post" do
         UserNotifications.expects(:private_message).with(coding_horror, has_key(:post))
-        Fabricate(:post, topic: topic, user: topic.user)
+        creator.create
       end
 
       it "doesn't send the user an email when they have them disabled" do
         coding_horror.update_column(:email_private_messages, false)
         UserNotifications.expects(:private_message).with(coding_horror, has_key(:post)).never
-        Fabricate(:post, topic: topic, user: topic.user)
+        creator.create
       end
 
     end

spec/models/topic_user_spec.rb

@@ -137,18 +137,20 @@ describe TopicUser do
 
     context 'auto tracking' do
 
+      let(:post_creator) { PostCreator.new(new_user, raw: Fabricate.build(:post).raw, topic_id: topic.id) }
+
       before do
         TopicUser.update_last_read(new_user, topic.id, 2, 0)
       end
 
       it 'should automatically track topics you reply to' do
-        post = Fabricate(:post, topic: topic, user: new_user)
+        post_creator.create
         topic_new_user.notification_level.should == TopicUser.notification_levels[:tracking]
         topic_new_user.notifications_reason_id.should == TopicUser.notification_reasons[:created_post]
       end
 
       it 'should not automatically track topics you reply to and have set state manually' do
-        Fabricate(:post, topic: topic, user: new_user)
+        post_creator.create
         TopicUser.change(new_user, topic, notification_level: TopicUser.notification_levels[:regular])
         topic_new_user.notification_level.should == TopicUser.notification_levels[:regular]
         topic_new_user.notifications_reason_id.should == TopicUser.notification_reasons[:user_changed]