From eae7e7561188cba5ec62ab41b62714a82e566771 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 6 Sep 2013 11:50:05 -0400 Subject: [PATCH] FIX: recover post by a non-staff user fails because the post is not unique. Uniqueness check shouldn't happen when recovering a deleted post. --- app/models/post.rb | 2 +- lib/post_destroyer.rb | 1 + lib/validators/post_validator.rb | 1 + spec/components/post_destroyer_spec.rb | 40 +++++++++---------- .../validators/post_validator_spec.rb | 34 ++++++++++++++++ 5 files changed, 57 insertions(+), 21 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index c30d4df925b..0af2e472d62 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -35,7 +35,7 @@ class Post < ActiveRecord::Base validates_with ::Validators::PostValidator # We can pass several creating options to a post via attributes - attr_accessor :image_sizes, :quoted_post_numbers, :no_bump, :invalidate_oneboxes, :cooking_options + attr_accessor :image_sizes, :quoted_post_numbers, :no_bump, :invalidate_oneboxes, :cooking_options, :skip_unique_check SHORT_POST_CHARS = 1200 diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 799872b9853..b02eb8f0b50 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -114,6 +114,7 @@ class PostDestroyer def user_recovered Post.transaction do @post.update_column(:user_deleted, false) + @post.skip_unique_check = true @post.revise(@user, @post.versions.last.modifications["raw"][0], force_new_version: true) @post.update_flagged_posts_count end diff --git a/lib/validators/post_validator.rb b/lib/validators/post_validator.rb index e9ae7704c61..1386f11c9cd 100644 --- a/lib/validators/post_validator.rb +++ b/lib/validators/post_validator.rb @@ -58,6 +58,7 @@ class Validators::PostValidator < ActiveModel::Validator # Stop us from posting the same thing too quickly def unique_post_validator(post) return if SiteSetting.unique_posts_mins == 0 + return if post.skip_unique_check return if post.acting_user.admin? || post.acting_user.moderator? # If the post is empty, default to the validates_presence_of diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 0570f41d6eb..50de3b5b386 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -94,28 +94,28 @@ describe PostDestroyer do describe 'basic destroying' do - context "as the creator of the post" do - before do - @orig = post.cooked - PostDestroyer.new(post.user, post).destroy - post.reload - end + it "as the creator of the post, doesn't delete the post" do + SiteSetting.stubs(:unique_posts_mins).returns(5) + SiteSetting.stubs(:delete_removed_posts_after).returns(24) - it "doesn't delete the post" do - SiteSetting.stubs(:delete_removed_posts_after).returns(24) - post.deleted_at.should be_blank - post.deleted_by.should be_blank - post.user_deleted.should be_true - post.raw.should == I18n.t('js.post.deleted_by_author', {count: 24}) - post.version.should == 2 + post2 = create_post # Create it here instead of with "let" so unique_posts_mins can do its thing - # lets try to recover - PostDestroyer.new(post.user, post).recover - post.reload - post.version.should == 3 - post.user_deleted.should be_false - post.cooked.should == @orig - end + @orig = post2.cooked + PostDestroyer.new(post2.user, post2).destroy + post2.reload + + post2.deleted_at.should be_blank + post2.deleted_by.should be_blank + post2.user_deleted.should be_true + post2.raw.should == I18n.t('js.post.deleted_by_author', {count: 24}) + post2.version.should == 2 + + # lets try to recover + PostDestroyer.new(post2.user, post2).recover + post2.reload + post2.version.should == 3 + post2.user_deleted.should be_false + post2.cooked.should == @orig end context "as a moderator" do diff --git a/spec/components/validators/post_validator_spec.rb b/spec/components/validators/post_validator_spec.rb index cc6e4afb1f1..00e39f21cdb 100644 --- a/spec/components/validators/post_validator_spec.rb +++ b/spec/components/validators/post_validator_spec.rb @@ -31,4 +31,38 @@ describe Validators::PostValidator do end end + describe "unique_post_validator" do + before do + SiteSetting.stubs(:unique_posts_mins).returns(5) + end + + context "post is unique" do + before do + $redis.stubs(:exists).with(post.unique_post_key).returns(nil) + end + + it "should not add an error" do + validator.unique_post_validator(post) + post.errors.count.should == 0 + end + end + + context "post is not unique" do + before do + $redis.stubs(:exists).with(post.unique_post_key).returns('1') + end + + it "should add an error" do + validator.unique_post_validator(post) + expect(post.errors.count).to be > 0 + end + + it "should not add an error if post.skip_unique_check is true" do + post.skip_unique_check = true + validator.unique_post_validator(post) + post.errors.count.should == 0 + end + end + end + end