From 565945ab7b129de65a452258bcaf8d13a853024a Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 17 May 2017 22:48:01 -0500 Subject: [PATCH] flags: move flagging inside Post#delete! --- .../moderator/post/posts_controller.rb | 3 +-- app/logical/post_pruner.rb | 6 ++--- app/models/post.rb | 12 ++++----- test/unit/post_test.rb | 26 +++++++++---------- 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/app/controllers/moderator/post/posts_controller.rb b/app/controllers/moderator/post/posts_controller.rb index cb01095e9..df660785d 100644 --- a/app/controllers/moderator/post/posts_controller.rb +++ b/app/controllers/moderator/post/posts_controller.rb @@ -14,8 +14,7 @@ module Moderator def delete @post = ::Post.find(params[:id]) if params[:commit] == "Delete" - @post.flag!(params[:reason], :is_deletion => true) - @post.delete!(:reason => params[:reason], :move_favorites => params[:move_favorites].present?) + @post.delete!(params[:reason], :move_favorites => params[:move_favorites].present?) end redirect_to(post_path(@post)) end diff --git a/app/logical/post_pruner.rb b/app/logical/post_pruner.rb index 0f3ba378a..18c8053f8 100644 --- a/app/logical/post_pruner.rb +++ b/app/logical/post_pruner.rb @@ -13,11 +13,10 @@ protected CurrentUser.scoped(User.system, "127.0.0.1") do Post.where("is_deleted = ? and is_pending = ? and created_at < ?", false, true, 3.days.ago).each do |post| begin - post.flag!("Unapproved in three days") + post.delete!("Unapproved in three days") rescue PostFlag::Error # swallow end - post.delete! end end end @@ -27,11 +26,10 @@ protected Post.where("is_deleted = ? and is_flagged = ?", false, true).each do |post| if post.flags.unresolved.old.any? begin - post.flag!("Unapproved in three days after returning to moderation queue") + post.delete!("Unapproved in three days after returning to moderation queue") rescue PostFlag::Error # swallow end - post.delete! end end end diff --git a/app/models/post.rb b/app/models/post.rb index 665b9435d..aa31fd2b2 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1355,7 +1355,7 @@ class Post < ActiveRecord::Base end ModAction.log("permanently deleted post ##{id}") - delete!(:without_mod_action => true) + delete!("Permanently deleted post ##{id}", :without_mod_action => true) Post.without_timeout do give_favorites_to_parent update_children_on_destroy @@ -1377,13 +1377,15 @@ class Post < ActiveRecord::Base ModAction.log("unbanned post ##{id}") end - def delete!(options = {}) + def delete!(reason, options = {}) if is_status_locked? self.errors.add(:is_status_locked, "; cannot delete post") return false end Post.transaction do + flag!(reason, is_deletion: true) + self.is_deleted = true self.is_pending = false self.is_flagged = false @@ -1398,11 +1400,7 @@ class Post < ActiveRecord::Base update_parent_on_save unless options[:without_mod_action] - if options[:reason] - ModAction.log("deleted post ##{id}, reason: #{options[:reason]}") - else - ModAction.log("deleted post ##{id}") - end + ModAction.log("deleted post ##{id}, reason: #{reason}") end end end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 1cc8d4f0a..e9093e7f7 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -80,7 +80,7 @@ class PostTest < ActiveSupport::TestCase end should "fail" do - @post.delete! + @post.delete!("test") assert_equal(["Is status locked ; cannot delete post"], @post.errors.full_messages) assert_equal(1, Post.where("id = ?", @post.id).count) end @@ -89,7 +89,7 @@ class PostTest < ActiveSupport::TestCase context "with the banned_artist tag" do should "also ban the post" do post = FactoryGirl.create(:post, :tag_string => "banned_artist") - post.delete! + post.delete!("test") post.reload assert(post.is_banned?) end @@ -100,7 +100,7 @@ class PostTest < ActiveSupport::TestCase post = FactoryGirl.create(:post, :tag_string => "aaa") assert_equal(1, Post.fast_count) assert_equal(1, Post.fast_count("aaa")) - post.delete! + post.delete!("test") assert_equal(1, Post.fast_count) assert_equal(1, Post.fast_count("aaa")) end @@ -108,14 +108,14 @@ class PostTest < ActiveSupport::TestCase should "toggle the is_deleted flag" do post = FactoryGirl.create(:post) assert_equal(false, post.is_deleted?) - post.delete! + post.delete!("test") assert_equal(true, post.is_deleted?) end should "not decrement the tag counts" do post = FactoryGirl.create(:post, :tag_string => "aaa") assert_equal(1, Tag.find_by_name("aaa").post_count) - post.delete! + post.delete!("test") assert_equal(1, Tag.find_by_name("aaa").post_count) end end @@ -209,7 +209,7 @@ class PostTest < ActiveSupport::TestCase c1 = FactoryGirl.create(:post, :parent_id => p1.id) user = FactoryGirl.create(:gold_user) c1.add_favorite!(user) - c1.delete! + c1.delete!("test") p1.reload assert(Favorite.exists?(:post_id => c1.id, :user_id => user.id)) assert(!Favorite.exists?(:post_id => p1.id, :user_id => user.id)) @@ -220,7 +220,7 @@ class PostTest < ActiveSupport::TestCase c1 = FactoryGirl.create(:post, :parent_id => p1.id) user = FactoryGirl.create(:gold_user) c1.add_favorite!(user) - c1.delete!(:move_favorites => true) + c1.delete!("test", :move_favorites => true) p1.reload assert(!Favorite.exists?(:post_id => c1.id, :user_id => user.id), "Child should not still have favorites") assert(Favorite.exists?(:post_id => p1.id, :user_id => user.id), "Parent should have favorites") @@ -229,7 +229,7 @@ class PostTest < ActiveSupport::TestCase should "not update the parent's has_children flag" do p1 = FactoryGirl.create(:post) c1 = FactoryGirl.create(:post, :parent_id => p1.id) - c1.delete! + c1.delete!("test") p1.reload assert(p1.has_children?, "Parent should have children") end @@ -239,7 +239,7 @@ class PostTest < ActiveSupport::TestCase should "not remove the has_children flag" do p1 = FactoryGirl.create(:post) c1 = FactoryGirl.create(:post, :parent_id => p1.id) - p1.delete! + p1.delete!("test") p1.reload assert_equal(true, p1.has_children?) end @@ -247,7 +247,7 @@ class PostTest < ActiveSupport::TestCase should "not remove the parent of that child" do p1 = FactoryGirl.create(:post) c1 = FactoryGirl.create(:post, :parent_id => p1.id) - p1.delete! + p1.delete!("test") c1.reload assert_not_nil(c1.parent) end @@ -259,7 +259,7 @@ class PostTest < ActiveSupport::TestCase c1 = FactoryGirl.create(:post, :parent_id => p1.id) c2 = FactoryGirl.create(:post, :parent_id => p1.id) c3 = FactoryGirl.create(:post, :parent_id => p1.id) - p1.delete! + p1.delete!("test") c1.reload c2.reload c3.reload @@ -275,7 +275,7 @@ class PostTest < ActiveSupport::TestCase new_user = FactoryGirl.create(:moderator_user) p1 = FactoryGirl.create(:post) c1 = FactoryGirl.create(:post, :parent_id => p1.id) - c1.delete! + c1.delete!("test") CurrentUser.scoped(new_user, "127.0.0.1") do c1.undelete! end @@ -286,7 +286,7 @@ class PostTest < ActiveSupport::TestCase should "preserve the parent's has_children flag" do p1 = FactoryGirl.create(:post) c1 = FactoryGirl.create(:post, :parent_id => p1.id) - c1.delete! + c1.delete!("test") c1.undelete! p1.reload assert_not_nil(c1.parent_id)