FIX: Recover from guardian check when deleting reviewable users. (#17949)

Handles edge-case when a user is an admin and has an associated reviewable. Hitting this exception should be rare since we clear the reviewable when
granting staff to the user.
This commit is contained in:
Roman Rizzi
2022-08-16 11:50:06 -03:00
committed by GitHub
parent e862373899
commit 1434fe3021
2 changed files with 16 additions and 2 deletions

View File

@@ -69,8 +69,8 @@ class ReviewableUser < Reviewable
end end
destroyer.destroy(target, delete_args) destroyer.destroy(target, delete_args)
rescue UserDestroyer::PostsExistError rescue UserDestroyer::PostsExistError, Discourse::InvalidAccess
# If a user has posts, we won't delete them to preserve their content. # If a user has posts or user is an admin, we won't delete them to preserve their content.
# However the reviewable record will be "rejected" and they will remain # However the reviewable record will be "rejected" and they will remain
# unapproved in the database. A staff member can still approve them # unapproved in the database. A staff member can still approve them
# via the admin. # via the admin.

View File

@@ -92,7 +92,9 @@ RSpec.describe ReviewableUser, type: :model do
expect(reviewable.target.approved_at).to be_present expect(reviewable.target.approved_at).to be_present
expect(reviewable.version > 0).to eq(true) expect(reviewable.version > 0).to eq(true)
end end
end
context "when rejecting" do
it "allows us to reject a user" do it "allows us to reject a user" do
result = reviewable.perform(moderator, :delete_user, reject_reason: "reject reason") result = reviewable.perform(moderator, :delete_user, reject_reason: "reject reason")
expect(result.success?).to eq(true) expect(result.success?).to eq(true)
@@ -162,6 +164,18 @@ RSpec.describe ReviewableUser, type: :model do
expect(reviewable.rejected?).to eq(true) expect(reviewable.rejected?).to eq(true)
expect(reviewable.target).to be_blank expect(reviewable.target).to be_blank
end end
it "silently transitions the reviewable if the user is an admin" do
reviewable.target.update!(admin: true)
result = reviewable.perform(moderator, :delete_user)
expect(reviewable.pending?).to eq(false)
expect(reviewable.rejected?).to eq(true)
reviewable.reload
expect(reviewable.target).to be_present
expect(reviewable.target.approved).to eq(false)
end
end end
end end