FIX: We need to skip users with associated reviewables when auto-approving (#9080)

* FIX: We need to skip users with associated reviewables when auto-approving them

* Update spec/initializers/track_setting_changes_spec.rb

* Update spec/initializers/track_setting_changes_spec.rb

Co-authored-by: Robin Ward <robin.ward@gmail.com>

config/initializers/014-track-setting-changes.rb

@@ -4,7 +4,10 @@ DiscourseEvent.on(:site_setting_changed) do |name, old_value, new_value|
   # Enabling `must_approve_users` on an existing site is odd, so we assume that the
   # existing users are approved.
   if name == :must_approve_users && new_value == true
-    User.where(approved: false).update_all(approved: true)
+
+    User.where(approved: false)
+      .joins("LEFT JOIN reviewables r ON r.target_id = users.id")
+      .where(r: { id: nil }).update_all(approved: true)
   end
 
   if name == :emoji_set

spec/initializers/track_setting_changes_spec.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe 'Setting changes' do
+  describe '#must_approve_users' do
+    before { SiteSetting.must_approve_users = false }
+
+    it 'does not approve a user with associated reviewables' do
+      user_pending_approval = Fabricate(:reviewable_user).target
+
+      SiteSetting.must_approve_users = true
+
+      expect(user_pending_approval.reload.approved?).to eq(false)
+    end
+
+    it 'approves a user with no associated reviewables' do
+      non_approved_user = Fabricate(:user, approved: false)
+
+      SiteSetting.must_approve_users = true
+
+      expect(non_approved_user.reload.approved?).to eq(true)
+    end
+  end
+end