flags: move flagging inside Post#delete!

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!(params[:reason], :move_favorites => params[:move_favorites].present?)
-          @post.delete!(:reason => params[:reason], :move_favorites => params[:move_favorites].present?)
         end
         redirect_to(post_path(@post))
       end

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

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: #{reason}")
-            ModAction.log("deleted post ##{id}, reason: #{options[:reason]}")
-          else
-            ModAction.log("deleted post ##{id}")
-          end
         end
       end
     end

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)