jaculabilis/danbooru
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix #4568: Send appealed posts back to the mod queue

Browse Source 
* Include appealed posts in the modqueue.

* Add `status` field to appeals. Appeals start out as `pending`, then
  become `rejected` if the post isn't approved within three days. If the
  post is approved, the appeal's status becomes `succeeded`.

* Add `status` field to flags. Flags start out as `pending` then become
  `rejected` if the post is approved within three days. If the post
  isn't approved, the flag's status becomes `succeeded`.

* Leave behind a "Unapproved in three days" dummy flag when an appeal
  goes unapproved, just like when a pending post is unapproved.

* Only allow deleted posts to be appealed. Don't allow flagged posts to be appealed.

* Add `status:appealed` metatag. `status:appealed` is separate from `status:pending`.

* Include appealed posts in `status:modqueue`. Search `status:modqueue order:modqueue`
  to view the modqueue as a normal search.

* Retroactively set old flags and appeals as succeeded or rejected. This
  may not be correct for posts that were appealed or flagged multiple
  times. This is difficult to set correctly because we don't have
  approval records for old posts, so we can't tell the actual outcome of
  old flags and appeals.

* Deprecate the `is_resolved` field on post flags. A resolved flag is a
  flag that isn't pending.

* Known bug: appealed posts have a black border instead of a blue
  border. Checking whether a post has been appealed would require either
  an extra query on the posts/index page, or an is_appealed flag on
  posts, neither of which are very desirable.

* Known bug: you can't use `status:appealed` in blacklists, for the same
  reason as above.
This commit is contained in:
evazion
2020-08-03 14:40:04 -05:00
parent e31afd0827
commit 0a0a85ee70
31 changed files with 372 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
test/factories/post_flag.rb
View File

@@ -1,7 +1,7 @@
FactoryBot.define do
factory(:post_flag) do
creator
post
post { build(:post, is_flagged: true) }
reason {"xxx"}
is_resolved {false}
end

8
test/functional/modqueue_controller_test.rb
View File

@@ -13,6 +13,14 @@ class ModqueueControllerTest < ActionDispatch::IntegrationTest
get_auth modqueue_index_path, @admin
assert_response :success
end
should "include appealed posts in the modqueue" do
@appeal = create(:post_appeal)
get_auth modqueue_index_path, @admin
assert_response :success
assert_select "#post-#{@appeal.post_id}"
end
end
end
end

52
test/functional/post_appeals_controller_test.rb
View File

@@ -48,14 +48,54 @@ class PostAppealsControllerTest < ActionDispatch::IntegrationTest
end
context "create action" do
setup do
@post = as(@user) { create(:post, is_deleted: true) }
context "appealing a deleted post" do
should "create a new appeal" do
@post = create(:post, is_deleted: true)
assert_difference("PostAppeal.count", 1) do
post_auth post_appeals_path, @user, params: { post_appeal: { post_id: @post.id, reason: "xxx" }}, as: :json
end
assert_response :success
end
end
should "create a new appeal" do
assert_difference("PostAppeal.count", 1) do
post_auth post_appeals_path, @user, params: {:format => "js", :post_appeal => {:post_id => @post.id, :reason => "xxx"}}
assert_response :success
context "appealing a flagged post" do
should "fail" do
@flag = create(:post_flag)
assert_no_difference("PostAppeal.count") do
post_auth post_appeals_path, @user, params: { post_appeal: { post_id: @flag.post.id, reason: "xxx" }}, as: :json
end
assert_response 422
assert_equal(["cannot be appealed"], response.parsed_body.dig("errors", "post"))
end
end
context "appealing a pending post" do
should "fail" do
@post = create(:post, is_pending: true)
assert_no_difference("PostAppeal.count") do
post_auth post_appeals_path, @user, params: { post_appeal: { post_id: @post.id, reason: "xxx" }}, as: :json
end
assert_response 422
assert_equal(["cannot be appealed"], response.parsed_body.dig("errors", "post"))
end
end
context "appealing an already appealed post" do
should "fail" do
@appeal = create(:post_appeal)
assert_no_difference("PostAppeal.count") do
post_auth post_appeals_path, @user, params: { post_appeal: { post_id: @appeal.post.id, reason: "xxx" }}, as: :json
end
assert_response 422
assert_equal(["cannot be appealed"], response.parsed_body.dig("errors", "post"))
end
end
end

23
test/functional/post_approvals_controller_test.rb
View File

@@ -27,6 +27,29 @@ class PostApprovalsControllerTest < ActionDispatch::IntegrationTest
end
end
context "for an appealed post" do
should "undelete the post and mark the appeal as successful" do
@appeal = create(:post_appeal)
post_auth post_approvals_path(post_id: @appeal.post_id, format: :js), @approver
assert_response :success
assert_equal(false, @appeal.reload.post.is_deleted?)
assert_equal(true, @appeal.succeeded?)
end
end
context "for a flagged post" do
should "approve the post and mark the flag as rejected" do
@flag = create(:post_flag)
post_auth post_approvals_path(post_id: @flag.post_id, format: :js), @approver
assert_response :success
assert_equal(false, @flag.reload.post.is_deleted?)
assert_equal(false, @flag.post.is_flagged?)
assert_equal(true, @flag.rejected?)
end
end
should "not allow non-approvers to approve posts" do
@post = create(:post, is_pending: true)
post_auth post_approvals_path(post_id: @post.id, format: :js), create(:user)

1
test/functional/post_events_controller_test.rb
View File

@@ -10,6 +10,7 @@ class PostEventsControllerTest < ActionDispatch::IntegrationTest
as(@user) do
@post = create(:post)
@post.flag!("aaa")
@post.update(is_deleted: true)
create(:post_appeal, post: @post)
@post.approve!(@mod)
end

3
test/functional/posts_controller_test.rb
View File

@@ -483,7 +483,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
create(:note, post: @post)
create(:artist_commentary, post: @post)
create(:post_flag, post: @post, creator: @user)
create(:post_appeal, post: @post, creator: @user)
#create(:post_appeal, post: @post, creator: @user)
create(:post_vote, post: @post, user: @user)
create(:favorite, post: @post, user: @user)
create(:moderation_report, model: @comment, creator: @builder)
@@ -667,7 +667,6 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
assert_response :success
assert_equal(false, @post.is_deleted?)
end
end
context "revert action" do

2
test/unit/post_appeal_test.rb
View File

@@ -32,7 +32,7 @@ class PostAppealTest < ActiveSupport::TestCase
@post_appeal = build(:post_appeal, post: @post, creator: @alice)
assert_equal(false, @post_appeal.valid?)
assert_equal(["Post is active"], @post_appeal.errors.full_messages)
assert_equal(["Post cannot be appealed"], @post_appeal.errors.full_messages)
end
end
end

1
test/unit/post_event_test.rb
View File

@@ -5,6 +5,7 @@ class PostEventTest < ActiveSupport::TestCase
@user = create(:user, created_at: 2.weeks.ago)
@post = create(:post)
@post_flag = create(:post_flag, creator: @user, post: @post)
@post.update(is_deleted: true)
@post_appeal = create(:post_appeal, creator: @user, post: @post)
end

2
test/unit/post_flag_test.rb
View File

@@ -53,7 +53,7 @@ class PostFlagTest < ActiveSupport::TestCase
@post_flag = build(:post_flag, post: @post, creator: @bob)
@post_flag.save
assert_equal(["Post is deleted"], @post_flag.errors.full_messages)
assert_equal(["Post is deleted and cannot be flagged"], @post_flag.errors.full_messages)
end
should "not be able to flag a pending post" do

118
test/unit/post_pruner_test.rb
View File

@@ -1,30 +1,108 @@
require 'test_helper'
class PostPrunerTest < ActiveSupport::TestCase
def setup
@old_post = FactoryBot.create(:post, :created_at => 5.days.ago, :is_pending => true)
@unresolved_flagged_post = FactoryBot.create(:post, :is_flagged => true)
@resolved_flagged_post = FactoryBot.create(:post, :is_flagged => true)
context "PostPruner" do
context "for a pending post" do
should "prune expired posts" do
@post = create(:post, created_at: 5.days.ago, is_pending: true)
PostPruner.prune!
@flagger = create(:gold_user, created_at: 2.weeks.ago)
@unresolved_post_flag = create(:post_flag, creator: @flagger, created_at: 5.days.ago, is_resolved: false, post: @unresolved_flagged_post)
@resolved_post_flag = create(:post_flag, creator: @flagger, created_at: 5.days.ago, is_resolved: true, post: @resolved_flagged_post)
assert_equal(true, @post.reload.is_deleted?)
assert_equal(false, @post.is_pending?)
PostPruner.new.prune!
end
assert_equal(1, @post.flags.size)
assert_equal("Unapproved in three days", @post.flags.last.reason)
end
end
should "prune old pending posts" do
@old_post.reload
assert(@old_post.is_deleted?)
end
context "for a flagged post" do
should "prune expired flags" do
@post = create(:post, created_at: 4.weeks.ago, is_flagged: true)
@flag = create(:post_flag, post: @post, created_at: 5.days.ago)
PostPruner.prune!
should "prune old flagged posts that are still unresolved" do
@unresolved_flagged_post.reload
assert(@unresolved_flagged_post.is_deleted?)
end
assert_equal(true, @post.reload.is_deleted?)
assert_equal(false, @post.is_pending?)
assert_equal(false, @post.is_flagged?)
assert_equal(true, @flag.reload.succeeded?)
should "not prune old flagged posts that are resolved" do
@resolved_flagged_post.reload
assert(!@resolved_flagged_post.is_deleted?)
assert_equal(2, @post.flags.size)
assert_equal("Unapproved in three days after returning to moderation queue", @post.flags.last.reason)
end
should "not prune unexpired flags" do
@post = create(:post, created_at: 4.weeks.ago, is_flagged: true)
@flag = create(:post_flag, post: @post, created_at: 1.day.ago)
PostPruner.prune!
assert_equal(false, @post.reload.is_deleted?)
assert_equal(false, @post.is_pending?)
assert_equal(true, @post.is_flagged?)
assert_equal(true, @flag.reload.pending?)
assert_equal(1, @post.flags.size)
end
should "leave the status of old flags unchanged" do
@post = create(:post, created_at: 4.weeks.ago, is_flagged: true)
@flag1 = create(:post_flag, post: @post, created_at: 3.weeks.ago, status: :succeeded)
@flag2 = create(:post_flag, post: @post, created_at: 2.weeks.ago, status: :rejected)
@flag3 = create(:post_flag, post: @post, created_at: 1.weeks.ago, status: :pending)
PostPruner.prune!
assert_equal(true, @post.reload.is_deleted?)
assert_equal(false, @post.is_pending?)
assert_equal(false, @post.is_flagged?)
assert_equal(true, @flag1.reload.succeeded?)
assert_equal(true, @flag2.reload.rejected?)
assert_equal(true, @flag3.reload.succeeded?)
end
end
context "for an appealed post" do
should "prune expired appeals" do
@post = create(:post, created_at: 4.weeks.ago, is_deleted: true)
@appeal = create(:post_appeal, post: @post, created_at: 5.days.ago)
PostPruner.prune!
assert_equal(false, @post.reload.is_pending?)
assert_equal(false, @post.is_flagged?)
assert_equal(true, @post.is_deleted?)
assert_equal(true, @appeal.reload.rejected?)
assert_equal(1, @post.flags.size)
assert_equal("Unapproved in three days after returning to moderation queue", @post.flags.last.reason)
end
should "not prune unexpired appeals" do
@post = create(:post, created_at: 4.weeks.ago, is_deleted: true)
@appeal = create(:post_appeal, post: @post, created_at: 1.day.ago)
PostPruner.prune!
assert_equal(false, @post.reload.is_pending?)
assert_equal(false, @post.is_flagged?)
assert_equal(true, @post.is_deleted?)
assert_equal(true, @appeal.reload.pending?)
assert_equal(0, @post.flags.size)
end
should "leave the status of old appeals unchanged" do
@post = create(:post, created_at: 4.weeks.ago, is_deleted: true)
@appeal1 = create(:post_appeal, post: @post, created_at: 3.weeks.ago, status: :succeeded)
@appeal2 = create(:post_appeal, post: @post, created_at: 2.weeks.ago, status: :rejected)
@appeal3 = create(:post_appeal, post: @post, created_at: 1.weeks.ago, status: :pending)
PostPruner.prune!
assert_equal(true, @post.reload.is_deleted?)
assert_equal(false, @post.is_pending?)
assert_equal(false, @post.is_flagged?)
assert_equal(true, @appeal1.reload.succeeded?)
assert_equal(true, @appeal2.reload.rejected?)
assert_equal(false, @appeal3.reload.pending?)
end
end
end
end

18
test/unit/post_query_builder_test.rb
View File

@@ -584,22 +584,26 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
flagged = create(:post, is_flagged: true)
deleted = create(:post, is_deleted: true)
banned = create(:post, is_banned: true)
all = [banned, deleted, flagged, pending]
appealed = create(:post, is_deleted: true)
appeal = create(:post_appeal, post: appealed)
all = [appealed, banned, deleted, flagged, pending]
assert_tag_match([flagged, pending], "status:modqueue")
assert_tag_match([appealed, flagged, pending], "status:modqueue")
assert_tag_match([pending], "status:pending")
assert_tag_match([flagged], "status:flagged")
assert_tag_match([deleted], "status:deleted")
assert_tag_match([appealed], "status:appealed")
assert_tag_match([appealed, deleted], "status:deleted")
assert_tag_match([banned], "status:banned")
assert_tag_match([banned], "status:active")
assert_tag_match([banned], "status:active status:banned")
assert_tag_match(all, "status:any")
assert_tag_match(all, "status:all")
assert_tag_match(all - [flagged, pending], "-status:modqueue")
assert_tag_match(all - [flagged, pending, appealed], "-status:modqueue")
assert_tag_match(all - [pending], "-status:pending")
assert_tag_match(all - [flagged], "-status:flagged")
assert_tag_match(all - [deleted], "-status:deleted")
assert_tag_match(all - [appealed], "-status:appealed")
assert_tag_match(all - [deleted, appealed], "-status:deleted")
assert_tag_match(all - [banned], "-status:banned")
assert_tag_match(all - [banned], "-status:active")
@@ -611,11 +615,13 @@ class PostQueryBuilderTest < ActiveSupport::TestCase
flagged = create(:post, is_flagged: true)
pending = create(:post, is_pending: true)
disapproved = create(:post, is_pending: true)
appealed = create(:post, is_deleted: true)
create(:post_flag, post: flagged, creator: create(:user, created_at: 2.weeks.ago))
create(:post_appeal, post: appealed)
create(:post_disapproval, user: CurrentUser.user, post: disapproved, reason: "disinterest")
assert_tag_match([pending, flagged], "status:unmoderated")
assert_tag_match([appealed, pending, flagged], "status:unmoderated")
assert_tag_match([disapproved], "-status:unmoderated")
end

9
test/unit/post_test.rb
View File

@@ -513,24 +513,23 @@ class PostTest < ActiveSupport::TestCase
end
context "A status locked post" do
setup do
@post = FactoryBot.create(:post, is_status_locked: true)
end
should "not allow new flags" do
assert_raises(PostFlag::Error) do
@post = create(:post, is_status_locked: true)
@post.flag!("wrong")
end
end
should "not allow new appeals" do
@post = create(:post, is_status_locked: true, is_deleted: true)
@appeal = build(:post_appeal, post: @post)
assert_equal(false, @appeal.valid?)
assert_equal(["Post is active"], @appeal.errors.full_messages)
assert_equal(["Post cannot be appealed"], @appeal.errors.full_messages)
end
should "not allow approval" do
@post = create(:post, is_status_locked: true, is_pending: true)
approval = @post.approve!
assert_includes(approval.errors.full_messages, "Post is locked and cannot be approved")
end

2
test/unit/upload_limit_test.rb
View File

@@ -12,7 +12,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)
PostPruner.new.prune!
PostPruner.prune!
assert_equal(967, @user.reload.upload_points)
end
end