From 341be51f95b9e810b4bc0d932ba8ac12a8dd3466 Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 11 Oct 2021 20:05:09 -0500 Subject: [PATCH] posts: remove unused `flag!` and `approve!` methods. These methods were unused outside of the test suite --- app/models/post.rb | 12 -- app/models/post_approval.rb | 2 +- app/models/post_flag.rb | 2 - .../moderator/dashboards_controller_test.rb | 8 +- .../functional/post_events_controller_test.rb | 2 +- test/unit/approver_pruner_test.rb | 2 +- test/unit/post_approval_test.rb | 66 ++++++-- test/unit/post_flag_test.rb | 2 +- test/unit/post_test.rb | 155 +----------------- test/unit/upload_limit_test.rb | 10 +- 10 files changed, 71 insertions(+), 190 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index ec7ae3685..a074169da 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -254,18 +254,6 @@ class Post < ApplicationRecord !is_active? && uploader != user end - def flag!(reason, is_deletion: false) - flag = flags.create(reason: reason, is_deletion: is_deletion, creator: CurrentUser.user) - - if flag.errors.any? - raise PostFlag::Error, flag.errors.full_messages.join("; ") - end - end - - def approve!(approver = CurrentUser.user) - approvals.create(user: approver) - end - def disapproved_by?(user) PostDisapproval.exists?(user_id: user.id, post_id: id) end diff --git a/app/models/post_approval.rb b/app/models/post_approval.rb index 5b2c86dcd..06b5ee7e5 100644 --- a/app/models/post_approval.rb +++ b/app/models/post_approval.rb @@ -29,7 +29,7 @@ class PostApproval < ApplicationRecord post.appeals.pending.update!(status: :succeeded) post.update(approver: user, is_flagged: false, is_pending: false, is_deleted: false) - ModAction.log("undeleted post ##{post_id}", :post_undelete) if is_undeletion + ModAction.log("undeleted post ##{post_id}", :post_undelete, user) if is_undeletion post.uploader.upload_limit.update_limit!(is_pending, true) end diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index 4e59b8d59..b151773d1 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -1,6 +1,4 @@ class PostFlag < ApplicationRecord - class Error < StandardError; end - module Reasons UNAPPROVED = "Unapproved in three days" REJECTED = "Unapproved in three days after returning to moderation queue%" diff --git a/test/functional/moderator/dashboards_controller_test.rb b/test/functional/moderator/dashboards_controller_test.rb index 8f27ebf4a..0c408c58a 100644 --- a/test/functional/moderator/dashboards_controller_test.rb +++ b/test/functional/moderator/dashboards_controller_test.rb @@ -110,14 +110,8 @@ module Moderator end context "for flags" do - setup do - as(@user) do - @post = create(:post) - @post.flag!("blah") - end - end - should "render" do + create(:post_flag) get_auth moderator_dashboard_path, @admin assert_response :success end diff --git a/test/functional/post_events_controller_test.rb b/test/functional/post_events_controller_test.rb index 79c5c3f7d..70f373ab8 100644 --- a/test/functional/post_events_controller_test.rb +++ b/test/functional/post_events_controller_test.rb @@ -12,7 +12,7 @@ class PostEventsControllerTest < ActionDispatch::IntegrationTest create(:post_flag, post: @post, status: :rejected) @post.update(is_deleted: true) create(:post_appeal, post: @post, status: :succeeded) - @post.approve!(@mod) + create(:post_approval, post: @post, user: @mod) end end diff --git a/test/unit/approver_pruner_test.rb b/test/unit/approver_pruner_test.rb index b1f9b1952..b506ad3c4 100644 --- a/test/unit/approver_pruner_test.rb +++ b/test/unit/approver_pruner_test.rb @@ -14,7 +14,7 @@ class ApproverPrunerTest < ActiveSupport::TestCase should "not demote active approvers" do posts = create_list(:post, ApproverPruner::MINIMUM_APPROVALS + 1, is_pending: true) - posts.each { |post| post.approve!(@approver) } + posts.each { |post| create(:post_approval, post: post, user: @approver) } assert_equal([], ApproverPruner.inactive_approvers.map(&:id)) end diff --git a/test/unit/post_approval_test.rb b/test/unit/post_approval_test.rb index d484e995e..9dd91f871 100644 --- a/test/unit/post_approval_test.rb +++ b/test/unit/post_approval_test.rb @@ -1,26 +1,70 @@ require 'test_helper' class PostApprovalTest < ActiveSupport::TestCase - context "a pending post" do + context "Post approvals:" do setup do @user = create(:user, created_at: 2.weeks.ago) @post = create(:post, uploader: @user, is_pending: true) @approver = create(:user, can_approve_posts: true) end - context "That is approved" do - should "create a postapproval record" do - assert_difference("PostApproval.count") do - @post.approve!(@approver) + context "a pending post" do + context "that is approved" do + should "create a postapproval record" do + create(:post_approval, post: @post, user: @approver) + + assert_equal(1, @post.approvals.count) + assert_equal(@approver, @post.approver) + assert_equal(false, @post.reload.is_pending?) + assert_equal(true, @post.reload.is_active?) + end + + should "prevent an approver from approving the same post twice" do + @approval1 = create(:post_approval, post: @post, user: @approver) + @approval2 = build(:post_approval, post: @post, user: @approver) + + assert_equal(false, @approval2.valid?) + assert_equal(["You have previously approved this post and cannot approve it again"], @approval2.errors[:base]) + end + end + end + + context "a deleted post" do + setup do + create(:post_approval, post: @post, user: @approver) + @post.delete!("Unapproved in three days", user: User.system) + end + + context "that is undeleted by a different approver" do + should "be updated with the new approver" do + @new_approver = create(:user) + create(:post_approval, post: @post, user: @new_approver) + + assert_equal(2, @post.approvals.count) + assert_equal(@new_approver, @post.approver) + assert_equal(false, @post.reload.is_deleted?) + assert_equal(true, @post.reload.is_active?) + assert_equal("post_undelete", ModAction.last.category) + assert_equal("undeleted post ##{@post.id}", ModAction.last.description) end end - should "prevent an approver from approving the same post twice" do - @approval1 = create(:post_approval, post: @post, user: @approver) - @approval2 = build(:post_approval, post: @post, user: @approver) + context "that is undeleted by the same approver" do + should "not be permitted" do + @approval = build(:post_approval, post: @post, user: @approver) - assert_equal(false, @approval2.valid?) - assert_equal(["You have previously approved this post and cannot approve it again"], @approval2.errors[:base]) + assert_equal(false, @approval.valid?) + assert_equal(["You have previously approved this post and cannot approve it again"], @approval.errors.full_messages) + end + end + + context "that is undeleted by the uploader" do + should "not be permitted" do + @approval = build(:post_approval, post: @post, user: @post.uploader) + + assert_equal(false, @approval.valid?) + assert_equal(["You cannot approve a post you uploaded"], @approval.errors.full_messages) + end end end @@ -28,7 +72,7 @@ class PostApprovalTest < ActiveSupport::TestCase should "work" do CurrentUser.scoped(@approver) do @post.update!(tag_string: "touhou") - @approval = @post.approve!(@approver) + @approval = create(:post_approval, post: @post, user: @approver) @approvals = PostApproval.search(user_name: @approver.name, post_tags_match: "touhou", post_id: @post.id) assert_equal([@approval.id], @approvals.map(&:id)) diff --git a/test/unit/post_flag_test.rb b/test/unit/post_flag_test.rb index de30861ce..0e8958adf 100644 --- a/test/unit/post_flag_test.rb +++ b/test/unit/post_flag_test.rb @@ -85,7 +85,7 @@ class PostFlagTest < ActiveSupport::TestCase @users = create_list(:user, 2) @post = create(:post) @flag1 = create(:post_flag, post: @post, creator: @users.first) - as(@mod) { @post.approve! } + create(:post_approval, post: @post, user: @mod) travel_to(Danbooru.config.moderation_period.from_now - 1.minute) do @flag2 = build(:post_flag, post: @post, reason: "something", creator: @users.second) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 8ca018128..a006a04ba 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -152,12 +152,11 @@ class PostTest < ActiveSupport::TestCase context "that is still in cooldown after being flagged" do should "succeed" do - post = FactoryBot.create(:post) - post.flag!("test flag") - post.delete!("test deletion") + flag = create(:post_flag) + flag.post.delete!("test deletion") - assert_equal(true, post.is_deleted) - assert_equal(2, post.flags.size) + assert_equal(true, flag.post.is_deleted) + assert_equal(2, flag.post.flags.size) end end @@ -322,160 +321,18 @@ class PostTest < ActiveSupport::TestCase end end end - - context "Undeleting a post with a parent" do - should "update with a new approver" do - new_user = FactoryBot.create(:moderator_user) - p1 = FactoryBot.create(:post) - c1 = FactoryBot.create(:post, :parent_id => p1.id) - c1.delete!("test") - c1.approve!(new_user) - p1.reload - assert_equal(new_user.id, c1.approver_id) - end - - should "preserve the parent's has_children flag" do - p1 = FactoryBot.create(:post) - c1 = FactoryBot.create(:post, :parent_id => p1.id) - c1.delete!("test") - c1.approve! - p1.reload - assert_not_nil(c1.parent_id) - assert(p1.has_children?, "Parent should have children") - end - end end context "Moderation:" do context "A deleted post" do - setup do - @post = FactoryBot.create(:post, :is_deleted => true) - end - - context "that is undeleted" do - setup do - @mod = FactoryBot.create(:moderator_user) - CurrentUser.user = @mod - end - - context "by the approver" do - setup do - @post.update_attribute(:approver_id, @mod.id) - end - - should "not be permitted" do - approval = @post.approve! - - assert_equal(false, approval.valid?) - assert_equal(["You have previously approved this post and cannot approve it again"], approval.errors.full_messages) - end - end - - context "by the uploader" do - setup do - @post.update_attribute(:uploader_id, @mod.id) - end - - should "not be permitted" do - approval = @post.approve! - - assert_equal(false, approval.valid?) - assert_equal(["You cannot approve a post you uploaded"], approval.errors.full_messages) - end - end - end - - context "when undeleted" do - should "be undeleted" do - @post.approve! - assert_equal(false, @post.reload.is_deleted?) - end - - should "create a mod action" do - @post.approve! - assert_equal("undeleted post ##{@post.id}", ModAction.last.description) - assert_equal("post_undelete", ModAction.last.category) - end - end - - context "when approved" do - should "be undeleted" do - @post.approve! - assert_equal(false, @post.reload.is_deleted?) - end - - should "create a mod action" do - @post.approve! - assert_equal("undeleted post ##{@post.id}", ModAction.last.description) - assert_equal("post_undelete", ModAction.last.category) - end - end - should "be appealed" do + @post = create(:post, is_deleted: true) create(:post_appeal, post: @post) + assert(@post.is_deleted?, "Post should still be deleted") assert_equal(1, @post.appeals.count) end end - - context "An approved post" do - should "be flagged" do - post = FactoryBot.create(:post) - assert_difference("PostFlag.count", 1) do - post.flag!("bad") - end - assert(post.is_flagged?, "Post should be flagged.") - assert_equal(1, post.flags.count) - end - - should "not be flagged if no reason is given" do - post = FactoryBot.create(:post) - assert_difference("PostFlag.count", 0) do - assert_raises(PostFlag::Error) do - post.flag!("") - end - end - end - end - - context "An unapproved post" do - should "preserve the approver's identity when approved" do - post = FactoryBot.create(:post, :is_pending => true) - post.approve! - assert_equal(post.approver_id, CurrentUser.id) - end - - context "that was previously approved by person X" do - setup do - @user = FactoryBot.create(:moderator_user, :name => "xxx") - @user2 = FactoryBot.create(:moderator_user, :name => "yyy") - @post = FactoryBot.create(:post, :approver_id => @user.id) - @post.flag!("bad") - end - - should "not allow person X to reapprove that post" do - approval = @post.approve!(@user) - assert_includes(approval.errors.full_messages, "You have previously approved this post and cannot approve it again") - end - - should "allow person Y to approve the post" do - @post.approve!(@user2) - assert(@post.valid?) - end - end - - context "that has been reapproved" do - should "no longer be flagged or pending" do - post = FactoryBot.create(:post) - post.flag!("bad") - post.approve! - assert(post.errors.empty?, post.errors.full_messages.join(", ")) - post.reload - assert_equal(false, post.is_flagged?) - assert_equal(false, post.is_pending?) - end - end - end end context "Tagging:" do diff --git a/test/unit/upload_limit_test.rb b/test/unit/upload_limit_test.rb index 218d852c9..2549bcfbb 100644 --- a/test/unit/upload_limit_test.rb +++ b/test/unit/upload_limit_test.rb @@ -30,7 +30,7 @@ class UploadLimitTest < ActiveSupport::TestCase @post = create(:post, uploader: @user, is_pending: true, created_at: 7.days.ago) assert_equal(1000, @user.reload.upload_points) - @post.approve!(@approver) + create(:post_approval, post: @post, user: @approver) assert_equal(1010, @user.reload.upload_points) end @@ -40,7 +40,7 @@ class UploadLimitTest < ActiveSupport::TestCase @post = create(:post, uploader: @user, is_pending: true, created_at: 7.days.ago) assert_equal(UploadLimit::MAXIMUM_POINTS, @user.reload.upload_points) - @post.approve!(@approver) + create(:post_approval, post: @post, user: @approver) assert_equal(UploadLimit::MAXIMUM_POINTS, @user.reload.upload_points) end end @@ -50,7 +50,7 @@ class UploadLimitTest < ActiveSupport::TestCase @post = create(:post, uploader: @user, is_pending: true) assert_equal(1000, @user.reload.upload_points) - @post.approve!(@approver) + create(:post_approval, post: @post, user: @approver) assert_equal(1010, @user.reload.upload_points) as(@approver) { @post.delete!("bad") } @@ -64,7 +64,7 @@ class UploadLimitTest < ActiveSupport::TestCase as(@approver) { @post.delete!("bad") } assert_equal(967, @user.reload.upload_points) - @post.approve!(@approver) + create(:post_approval, post: @post, user: @approver) assert_equal(1010, @user.reload.upload_points) end end @@ -77,7 +77,7 @@ class UploadLimitTest < ActiveSupport::TestCase assert_equal(967, @user.reload.upload_points) @appeal = create(:post_appeal, post: @post) - @post.approve!(@approver) + create(:post_approval, post: @post, user: @approver) assert_equal(true, @appeal.reload.succeeded?) assert_equal(false, @post.reload.is_deleted?)