From 1434fe30210e7a7ab72a47799fb9be971829cf33 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Tue, 16 Aug 2022 11:50:06 -0300 Subject: [PATCH] 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. --- app/models/reviewable_user.rb | 4 ++-- spec/models/reviewable_user_spec.rb | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/models/reviewable_user.rb b/app/models/reviewable_user.rb index 2fbe915c80..6fa1fc56ce 100644 --- a/app/models/reviewable_user.rb +++ b/app/models/reviewable_user.rb @@ -69,8 +69,8 @@ class ReviewableUser < Reviewable end destroyer.destroy(target, delete_args) - rescue UserDestroyer::PostsExistError - # If a user has posts, we won't delete them to preserve their content. + rescue UserDestroyer::PostsExistError, Discourse::InvalidAccess + # 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 # unapproved in the database. A staff member can still approve them # via the admin. diff --git a/spec/models/reviewable_user_spec.rb b/spec/models/reviewable_user_spec.rb index 9ec1b72917..0c3440a00c 100644 --- a/spec/models/reviewable_user_spec.rb +++ b/spec/models/reviewable_user_spec.rb @@ -92,7 +92,9 @@ RSpec.describe ReviewableUser, type: :model do expect(reviewable.target.approved_at).to be_present expect(reviewable.version > 0).to eq(true) end + end + context "when rejecting" do it "allows us to reject a user" do result = reviewable.perform(moderator, :delete_user, reject_reason: "reject reason") expect(result.success?).to eq(true) @@ -162,6 +164,18 @@ RSpec.describe ReviewableUser, type: :model do expect(reviewable.rejected?).to eq(true) expect(reviewable.target).to be_blank 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