DEV: Add "delete user" options to illegal flag review (#29956)

We already add the "delete user" and "delete and block user" options to the drop-down for potential spam, but we should do this for potentially illegal posts as well.

This is entirely based on the implementation for the potential spam one, including caching the status on the reviewable record.

Also note that just as for potential spam, the user must be "deletable" for the option to appear.

I also took the liberty to move the options in the drop-down to what I think is a more intuitive place. (Between delete post and suspend/silence user.)
This commit is contained in:
Ted Johansson 2024-11-27 17:23:57 +08:00 committed by GitHub
parent ea2a0f2c8e
commit f4d0a77d5f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 58 additions and 6 deletions

View File

@ -103,6 +103,7 @@ class Reviewable < ActiveRecord::Base
payload: nil,
reviewable_by_moderator: false,
potential_spam: true,
potentially_illegal: false,
target_created_by: nil
)
reviewable =
@ -113,6 +114,7 @@ class Reviewable < ActiveRecord::Base
reviewable_by_moderator: reviewable_by_moderator,
payload: payload,
potential_spam: potential_spam,
potentially_illegal: potentially_illegal,
target_created_by: target_created_by,
)
reviewable.created_new!
@ -136,12 +138,14 @@ class Reviewable < ActiveRecord::Base
id: target.id,
type: target.class.polymorphic_name,
potential_spam: potential_spam == true ? true : nil,
potentially_illegal: potentially_illegal == true ? true : nil,
}
row = DB.query_single(<<~SQL, update_args)
UPDATE reviewables
SET status = :status,
potential_spam = COALESCE(:potential_spam, reviewables.potential_spam)
potential_spam = COALESCE(:potential_spam, reviewables.potential_spam),
potentially_illegal = COALESCE(:potentially_illegal, reviewables.potentially_illegal)
FROM reviewables AS old_reviewables
WHERE reviewables.target_id = :id
AND reviewables.target_type = :type
@ -776,6 +780,7 @@ end
# updated_at :datetime not null
# force_review :boolean default(FALSE), not null
# reject_reason :text
# potentially_illegal :boolean default(FALSE)
#
# Indexes
#

View File

@ -48,10 +48,6 @@ class ReviewableFlaggedPost < Reviewable
agree_bundle =
actions.add_bundle("#{id}-agree", icon: "thumbs-up", label: "reviewables.actions.agree.title")
if potential_spam? && guardian.can_delete_user?(target_created_by)
delete_user_actions(actions, agree_bundle)
end
if !post.user_deleted? && !post.hidden?
build_action(actions, :agree_and_hide, icon: "far-eye-slash", bundle: agree_bundle)
end
@ -83,6 +79,10 @@ class ReviewableFlaggedPost < Reviewable
end
end
if (potential_spam? || potentially_illegal?) && guardian.can_delete_user?(target_created_by)
delete_user_actions(actions, agree_bundle)
end
if guardian.can_suspend?(target_created_by)
build_action(
actions,
@ -416,6 +416,7 @@ end
# updated_at :datetime not null
# force_review :boolean default(FALSE), not null
# reject_reason :text
# potentially_illegal :boolean default(FALSE)
#
# Indexes
#

View File

@ -155,6 +155,7 @@ end
# updated_at :datetime not null
# force_review :boolean default(FALSE), not null
# reject_reason :text
# potentially_illegal :boolean default(FALSE)
#
# Indexes
#

View File

@ -257,6 +257,7 @@ end
# updated_at :datetime not null
# force_review :boolean default(FALSE), not null
# reject_reason :text
# potentially_illegal :boolean default(FALSE)
#
# Indexes
#

View File

@ -119,6 +119,7 @@ end
# updated_at :datetime not null
# force_review :boolean default(FALSE), not null
# reject_reason :text
# potentially_illegal :boolean default(FALSE)
#
# Indexes
#

View File

@ -0,0 +1,16 @@
# frozen_string_literal: true
#
class AddPotentiallyIllegalToReviewables < ActiveRecord::Migration[7.1]
def change
add_column :reviewables, :potentially_illegal, :boolean
up_only do
# NOTE: Only for records created after this migration. Trying to
# apply this as part of adding the column will attempt to backfill
# `false` into all existing reviewables. This is dangerous (locks
# a potentially huge table) and will create some false negatives.
#
change_column_default :reviewables, :potentially_illegal, false
end
end
end

View File

@ -388,6 +388,7 @@ class PostActionCreator
topic: @post.topic,
reviewable_by_moderator: true,
potential_spam: @post_action_type_id == @post_action_type_view.types[:spam],
potentially_illegal: @post_action_type_id == @post_action_type_view.types[:illegal],
payload: {
targets_topic: @targets_topic,
},

View File

@ -16,6 +16,13 @@ RSpec.describe ReviewableFlaggedPost, type: :model do
expect(reviewable.reload.potential_spam?).to eq(true)
end
it "sets `potentially_illegal` when an illegal flag is added" do
reviewable = PostActionCreator.off_topic(user, post).reviewable
expect(reviewable.potentially_illegal?).to eq(false)
PostActionCreator.illegal(Fabricate(:user, refresh_auto_groups: true), post)
expect(reviewable.reload.potentially_illegal?).to eq(true)
end
describe "actions" do
let!(:result) { PostActionCreator.spam(user, post) }
let(:reviewable) { result.reviewable }
@ -95,7 +102,26 @@ RSpec.describe ReviewableFlaggedPost, type: :model do
end
context "when flagged as potential_spam" do
before { reviewable.update!(potential_spam: true) }
before { reviewable.update!(potential_spam: true, potentially_illegal: false) }
it "excludes delete action if the reviewer cannot delete the user" do
post.user.user_stat.update!(
first_post_created_at: 1.year.ago,
post_count: User::MAX_STAFF_DELETE_POST_COUNT + 1,
)
expect(reviewable.actions_for(guardian).has?(:delete_user)).to be false
expect(reviewable.actions_for(guardian).has?(:delete_user_block)).to be false
end
it "includes delete actions if the reviewer can delete the user" do
expect(reviewable.actions_for(guardian).has?(:delete_user)).to be true
expect(reviewable.actions_for(guardian).has?(:delete_user_block)).to be true
end
end
context "when flagged as illegal" do
before { reviewable.update(potential_spam: false, potentially_illegal: true) }
it "excludes delete action if the reviewer cannot delete the user" do
post.user.user_stat.update!(