From 63bd5daa3b8ec7aa33b78600bfba52598b31e1d4 Mon Sep 17 00:00:00 2001 From: nonamethanks Date: Mon, 11 Apr 2022 15:44:54 +0200 Subject: [PATCH] Posts: prune disapprovals on new appeal or flag --- app/models/post_appeal.rb | 5 +++ app/models/post_flag.rb | 6 +++ test/unit/post_disapproval_test.rb | 72 +++++++++++++++++++++++++----- 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/app/models/post_appeal.rb b/app/models/post_appeal.rb index 0939aacb2..495e6b87e 100644 --- a/app/models/post_appeal.rb +++ b/app/models/post_appeal.rb @@ -8,6 +8,7 @@ class PostAppeal < ApplicationRecord validate :validate_post_is_appealable, on: :create validate :validate_creator_is_not_limited, on: :create validates :creator, uniqueness: { scope: :post, message: "have already appealed this post" }, on: :create + after_create :prune_disapprovals enum status: { pending: 0, @@ -17,6 +18,10 @@ class PostAppeal < ApplicationRecord scope :expired, -> { pending.where("post_appeals.created_at < ?", Danbooru.config.moderation_period.ago) } + def prune_disapprovals + PostDisapproval.where(post: post).delete_all + end + module SearchMethods def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :reason, :status, :creator, :post) diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index 977274c1d..b207958fd 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -15,6 +15,7 @@ class PostFlag < ApplicationRecord validate :validate_post, on: :create validates :creator_id, uniqueness: { scope: :post_id, on: :create, unless: :is_deletion, message: "have already flagged this post" } before_save :update_post + after_create :prune_disapprovals attr_accessor :is_deletion enum status: { @@ -90,6 +91,11 @@ class PostFlag < ApplicationRecord end end + def prune_disapprovals + return if is_deletion + PostDisapproval.where(post: post).delete_all + end + def update_post post.update_column(:is_flagged, true) unless post.is_flagged? end diff --git a/test/unit/post_disapproval_test.rb b/test/unit/post_disapproval_test.rb index 4e16f401e..e18ce13f4 100644 --- a/test/unit/post_disapproval_test.rb +++ b/test/unit/post_disapproval_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require "test_helper" class PostDisapprovalTest < ActiveSupport::TestCase context "In all cases" do @@ -13,7 +13,7 @@ class PostDisapprovalTest < ActiveSupport::TestCase CurrentUser.ip_addr = nil end - context "A post disapproval" do + context "a post disapproval" do setup do @post_1 = FactoryBot.create(:post, :is_pending => true) @post_2 = FactoryBot.create(:post, :is_pending => true) @@ -35,7 +35,7 @@ class PostDisapprovalTest < ActiveSupport::TestCase end should "remove the associated post from alice's moderation queue" do - assert(!Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_1.id)) + assert_not(Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_1.id)) assert(Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_2.id)) end end @@ -53,15 +53,21 @@ class PostDisapprovalTest < ActiveSupport::TestCase end end - context "for a post that has been approved" do - setup do - @post = FactoryBot.create(:post, is_pending: true) + context "when pruning" do + should "prune old disapprovals" do @user = FactoryBot.create(:user) - @disapproval = create(:post_disapproval, user: @user, post: @post, created_at: 2.months.ago) + @post = FactoryBot.create(:post, is_pending: true) + create(:post_disapproval, user: @user, post: @post, created_at: 2.months.ago) + assert_difference("PostDisapproval.count", -1) do + PostDisapproval.prune! + end end - should "be pruned" do - assert_difference("PostDisapproval.count", -1) do + should "not prune recent disapprovals" do + @user = FactoryBot.create(:user) + @post = FactoryBot.create(:post, is_pending: true) + @disapproval = create(:post_disapproval, user: @user, post: @post, created_at: 7.days.ago) + assert_no_difference("PostDisapproval.count") do PostDisapproval.prune! end end @@ -69,14 +75,56 @@ class PostDisapprovalTest < ActiveSupport::TestCase context "#search" do should "work" do - disapproval1 = FactoryBot.create(:post_disapproval, user: @alice, post: @post_1, reason: "breaks_rules") - disapproval2 = FactoryBot.create(:post_disapproval, user: @alice, post: @post_2, reason: "poor_quality", message: "bad anatomy") + @approver = create(:approver) + @post1 = create(:post, is_pending: true) + @post2 = create(:post, is_pending: true) + disapproval1 = FactoryBot.create(:post_disapproval, user: @approver, post: @post1, reason: "breaks_rules") + disapproval2 = FactoryBot.create(:post_disapproval, user: @approver, post: @post2, reason: "poor_quality", message: "bad anatomy") assert_equal([disapproval1.id], PostDisapproval.search(reason: "breaks_rules").pluck(:id)) assert_equal([disapproval2.id], PostDisapproval.search(message: "bad anatomy").pluck(:id)) - assert_equal([disapproval2.id, disapproval1.id], PostDisapproval.search(creator_name: "alice").pluck(:id)) + assert_equal([disapproval1.id, disapproval2.id], PostDisapproval.where(user: @approver).pluck(:id)) end end end + + context "post deletions" do + should "not prune disapprovals" do + @post = create(:post, is_pending: true) + @user = create(:moderator_user) + create(:post_disapproval, user: @user, post: @post, message: "test") + assert_equal(1, PostDisapproval.where(post: @post).size) + @post.delete!("blah", user: @user) + assert_equal(1, PostDisapproval.where(post: @post).size) + end + end + + context "post appeals" do + should "prune disapprovals" do + @post = create(:post, is_pending: true) + @user = create(:moderator_user) + create(:post_disapproval, user: @user, post: @post, message: "test") + assert_equal(1, PostDisapproval.where(post: @post).size) + @post.delete!("blah", user: @user) + + create(:post_appeal, post: @post, creator: @user) + + assert_equal(0, PostDisapproval.where(post: @post).size) + end + end + + context "post flags" do + should "prune disapprovals" do + @post = create(:post, is_pending: true) + @user = create(:moderator_user) + create(:post_disapproval, user: @user, post: @post, message: "test") + assert_equal(1, PostDisapproval.where(post: @post).size) + + create(:post_approval, post: @post, user: @user) + create(:post_flag, post: @post, creator: @user) + + assert_equal(0, PostDisapproval.where(post: @post).size) + end + end end end