FIX: Don't require a rejection reason if the user is a spammer. (#12141)

app/models/reviewable_user.rb

@@ -27,13 +27,13 @@ class ReviewableUser < Reviewable
     actions.add(:reject_user_delete, bundle: reject) do |a|
       a.icon = 'user-times'
       a.label = "reviewables.actions.reject_user.delete.title"
-      a.require_reject_reason = true
+      a.require_reject_reason = !is_a_suspect_user?
       a.description = "reviewables.actions.reject_user.delete.description"
     end
     actions.add(:reject_user_block, bundle: reject) do |a|
       a.icon = 'ban'
       a.label = "reviewables.actions.reject_user.block.title"
-      a.require_reject_reason = true
+      a.require_reject_reason = !is_a_suspect_user?
       a.description = "reviewables.actions.reject_user.block.description"
     end
   end
@@ -61,9 +61,7 @@ class ReviewableUser < Reviewable
     if target.present?
       destroyer = UserDestroyer.new(performed_by)
 
-      if reviewable_scores.any? { |rs| rs.reason == 'suspect_user' }
-        DiscourseEvent.trigger(:suspect_user_deleted, target)
-      end
+      DiscourseEvent.trigger(:suspect_user_deleted, target) if is_a_suspect_user?
 
       begin
         self.reject_reason = args[:reject_reason]
@@ -106,6 +104,10 @@ class ReviewableUser < Reviewable
     user.approved_by ||= approved_by
     user.approved_at ||= Time.zone.now
   end
+
+  def is_a_suspect_user?
+    reviewable_scores.any? { |rs| rs.reason == 'suspect_user' }
+  end
 end
 
 # == Schema Information

spec/models/reviewable_user_spec.rb

@@ -27,6 +27,34 @@ RSpec.describe ReviewableUser, type: :model do
       expect(actions.has?(:approve_user)).to eq(false)
       expect(actions.has?(:reject_user_delete)).to eq(false)
     end
+
+    it 'can delete a user without a giving a rejection reason if the user was a spammer' do
+      reviewable.reviewable_scores.build(user: admin, reason: 'suspect_user')
+
+      assert_require_reject_reason(:reject_user_delete, false)
+    end
+
+    it 'requires a rejection reason to delete a user' do
+      assert_require_reject_reason(:reject_user_delete, true)
+    end
+
+    it 'can delete and block a user without giving a rejection reason if the user was a spammer' do
+      reviewable.reviewable_scores.build(user: admin, reason: 'suspect_user')
+
+      assert_require_reject_reason(:reject_user_block, false)
+    end
+
+    it 'requires a rejection reason to delete and block a user' do
+      assert_require_reject_reason(:reject_user_block, true)
+    end
+
+    def assert_require_reject_reason(id, expected)
+      actions = reviewable.actions_for(Guardian.new(moderator))
+
+      expect(actions.to_a.
+        find { |a| a.id == id }.require_reject_reason).
+        to eq(expected)
+    end
   end
 
   context "#update_fields" do