FIX: Granting staff status should auto-approve users waiting approval (#8533)

* FIX: Granting staff status should auto-approve users waiting approval

* Update app/models/concerns/roleable.rb

Co-Authored-By: Robin Ward <robin.ward@gmail.com>

app/controllers/admin/users_controller.rb

@@ -471,7 +471,6 @@ class Admin::UsersController < Admin::AdminController
     end
 
     user.active = true
-    user.approved = true
     user.save!
     user.grant_admin!
     user.change_trust_level!(4)

app/models/concerns/roleable.rb

@@ -20,6 +20,7 @@ module Roleable
 
   def grant_moderation!
     set_permission('moderator', true)
+    auto_approve_user
     enqueue_staff_welcome_message(:moderator)
   end
 
@@ -29,6 +30,7 @@ module Roleable
 
   def grant_admin!
     set_permission('admin', true)
+    auto_approve_user
     enqueue_staff_welcome_message(:admin)
   end
 
@@ -48,4 +50,14 @@ module Roleable
     save_and_refresh_staff_groups!
   end
 
+  private
+
+  def auto_approve_user
+    if reviewable = ReviewableUser.find_by(target: self, status: Reviewable.statuses[:pending])
+      reviewable.perform(Discourse.system_user, :approve_user, send_email: false)
+    else
+      ReviewableUser.set_approved_fields!(self, Discourse.system_user)
+      self.save!
+    end
+  end
 end

spec/fabricators/reviewable_fabricator.rb

@@ -58,3 +58,11 @@ Fabricator(:reviewable_flagged_post) do
   target_type 'Post'
   target { Fabricate(:post) }
 end
+
+Fabricator(:reviewable_user) do
+  reviewable_by_moderator true
+  type 'ReviewableUser'
+  created_by { Fabricate(:user) }
+  target_type 'User'
+  target { Fabricate(:user) }
+end

spec/models/user_spec.rb

@@ -2240,4 +2240,39 @@ describe User do
       end
     end
   end
+
+  describe 'Granting admin or moderator status' do
+    it 'approves the associated reviewable when granting admin status' do
+      reviewable_user = Fabricate(:reviewable_user)
+
+      reviewable_user.target.grant_admin!
+
+      expect(reviewable_user.reload.status).to eq Reviewable.statuses[:approved]
+    end
+
+    it 'does nothing when the user is already approved' do
+      reviewable_user = Fabricate(:reviewable_user)
+      reviewable_user.perform(Discourse.system_user, :approve_user)
+
+      reviewable_user.target.grant_admin!
+
+      expect(reviewable_user.reload.status).to eq Reviewable.statuses[:approved]
+    end
+
+    it 'approves the associated reviewable when granting moderator status' do
+      reviewable_user = Fabricate(:reviewable_user)
+
+      reviewable_user.target.grant_moderation!
+
+      expect(reviewable_user.reload.status).to eq Reviewable.statuses[:approved]
+    end
+
+    it 'approves the user if there is no reviewable' do
+      user = Fabricate(:user, approved: false)
+
+      user.grant_admin!
+
+      expect(user.approved).to eq(true)
+    end
+  end
 end