FIX: When must_approve_users is enabled, we don't want to send suspect users to the review queue. Only non-approved users should be sent. Provide a migration to auto-approve every problematic review item (#9179)

app/jobs/scheduled/enqueue_suspect_users.rb

@@ -6,10 +6,12 @@ module Jobs
 
     def execute(_args)
       return unless SiteSetting.approve_suspect_users
+      return if SiteSetting.must_approve_users
 
       users = User
         .activated
         .human_users
+        .where(approved: false)
         .joins(:user_profile, :user_stat)
         .where("users.created_at <= ?", 1.day.ago)
         .where("LENGTH(COALESCE(user_profiles.bio_raw, user_profiles.website, '')) > 0")

db/migrate/20200311135425_clear_approved_users_from_the_review_queue.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+class ClearApprovedUsersFromTheReviewQueue < ActiveRecord::Migration[6.0]
+  def up
+    reviewables = DB.query_single <<~SQL
+      UPDATE reviewables r
+      SET status = #{Reviewable.statuses[:approved]}
+      FROM users u
+      WHERE u.approved = true AND r.type = 'ReviewableUser' AND r.status = #{Reviewable.statuses[:pending]}
+      RETURNING r.id
+    SQL
+
+    system_user_id = Discourse::SYSTEM_USER_ID
+    scores = reviewables.map do |id|
+      "(#{id}, 1, #{Reviewable.statuses[:approved]}, #{system_user_id}, NOW(), NOW())"
+    end
+
+    if scores.present?
+      DB.exec <<~SQL
+        INSERT INTO reviewable_histories (
+          reviewable_id,
+          reviewable_history_type,
+          status,
+          created_by_id,
+          created_at,
+          updated_at
+        )
+        VALUES #{scores.join(',') << ';'}
+      SQL
+    end
+  end
+
+  def down
+  end
+end

spec/jobs/enqueue_suspect_users_spec.rb

@@ -39,5 +39,22 @@ describe Jobs::EnqueueSuspectUsers do
 
       expect(score.reason).to eq('suspect_user')
     end
+
+    it 'only enqueues non-approved users' do
+      suspect_user.update!(approved: true)
+
+      subject.execute({})
+
+      expect(ReviewableUser.where(target: suspect_user).exists?).to eq(false)
+    end
+
+    it 'does nothing if must_approve_users is set to true' do
+      SiteSetting.must_approve_users = true
+      suspect_user.update!(approved: false)
+
+      subject.execute({})
+
+      expect(ReviewableUser.where(target: suspect_user).exists?).to eq(false)
+    end
   end
 end