diff --git a/app/models/post.rb b/app/models/post.rb index 2eb92b607..77c824999 100644 --- a/app/models/post.rb +++ b/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? diff --git a/lib/post_creator.rb b/lib/post_creator.rb index b36c0209c..e2bf3e944 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -4,13 +4,13 @@ require_dependency 'rate_limiter' class PostCreator - # Errors when creating the post - attr_reader :errors + attr_reader :errors, :opts # 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 diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index 8e4a3df8c..799b03aab 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/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 diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index e5570b64e..1250f5d9a 100644 --- a/spec/components/post_creator_spec.rb +++ b/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) } diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 7ae90800c..9d127272c 100644 --- a/spec/models/post_spec.rb +++ b/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) } diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 712fe7a11..9a7b96766 100644 --- a/spec/models/topic_spec.rb +++ b/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 diff --git a/spec/models/topic_user_spec.rb b/spec/models/topic_user_spec.rb index 60a7e61a3..c1c5debb3 100644 --- a/spec/models/topic_user_spec.rb +++ b/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]