From b9aaa9718df45fc41aafccc3f7dc47ac7e3126a3 Mon Sep 17 00:00:00 2001
From: Roman Rizzi <rizziromanalejandro@gmail.com>
Date: Wed, 11 Mar 2020 17:05:44 -0300
Subject: [PATCH] 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   |  2 ++
 ...ar_approved_users_from_the_review_queue.rb | 34 +++++++++++++++++++
 spec/jobs/enqueue_suspect_users_spec.rb       | 17 ++++++++++
 3 files changed, 53 insertions(+)
 create mode 100644 db/migrate/20200311135425_clear_approved_users_from_the_review_queue.rb

diff --git a/app/jobs/scheduled/enqueue_suspect_users.rb b/app/jobs/scheduled/enqueue_suspect_users.rb
index 9fc5196d828..1b94dcb3098 100644
--- a/app/jobs/scheduled/enqueue_suspect_users.rb
+++ b/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")
diff --git a/db/migrate/20200311135425_clear_approved_users_from_the_review_queue.rb b/db/migrate/20200311135425_clear_approved_users_from_the_review_queue.rb
new file mode 100644
index 00000000000..07cb662d74e
--- /dev/null
+++ b/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
diff --git a/spec/jobs/enqueue_suspect_users_spec.rb b/spec/jobs/enqueue_suspect_users_spec.rb
index 30514abed11..b614899823b 100644
--- a/spec/jobs/enqueue_suspect_users_spec.rb
+++ b/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