FIX: Don't allow staff to approve users with unverified emails

app/assets/javascripts/admin/templates/user-index.hbs

@@ -466,7 +466,7 @@
 {{/if}}
 
 <section>
-  <hr/>
+  <hr>
   <div class="pull-right">
     {{#unless model.anonymizeForbidden}}
       {{d-button label="admin.user.anonymize"
@@ -487,7 +487,7 @@
 
   {{#if model.deleteExplanation}}
     <div class="clearfix"></div>
-    <br/>
+    <br>
     <div class="pull-right">
       {{d-icon "exclamation-triangle"}} {{model.deleteExplanation}}
     </div>

lib/admin_user_index_query.rb

@@ -95,7 +95,7 @@ class AdminUserIndexQuery
     when 'moderators' then @query.where(moderator: true)
     when 'blocked'    then @query.blocked
     when 'suspended'  then @query.suspended
-    when 'pending'    then @query.not_suspended.where(approved: false)
+    when 'pending'    then @query.not_suspended.where(approved: false, active: true)
     when 'suspect'    then suspect_users
     end
   end

lib/guardian.rb

@@ -176,7 +176,7 @@ class Guardian
 
   # Can we approve it?
   def can_approve?(target)
-    is_staff? && target && not(target.approved?)
+    is_staff? && target && target.active? && not(target.approved?)
   end
 
   def can_activate?(target)

spec/components/admin_user_index_query_spec.rb

@@ -100,18 +100,20 @@ describe AdminUserIndexQuery do
 
   describe "with a pending user" do
 
-    let!(:user) { Fabricate(:user, approved: false) }
+    let!(:user) { Fabricate(:user, active: true, approved: false) }
+    let!(:inactive_user) { Fabricate(:user, approved: false, active: false) }
 
     it "finds the unapproved user" do
       query = ::AdminUserIndexQuery.new(query: 'pending')
-      expect(query.find_users.count).to eq(1)
+      expect(query.find_users).to include(user)
+      expect(query.find_users).not_to include(inactive_user)
     end
 
     context 'and a suspended pending user' do
       let!(:suspended_user) { Fabricate(:user, approved: false, suspended_at: 1.hour.ago, suspended_till: 20.years.from_now) }
       it "doesn't return the suspended user" do
         query = ::AdminUserIndexQuery.new(query: 'pending')
-        expect(query.find_users.count).to eq(1)
+        expect(query.find_users).not_to include(suspended_user)
       end
     end
 

spec/components/guardian_spec.rb

@@ -1653,6 +1653,11 @@ describe Guardian do
       expect(Guardian.new(admin).can_approve?(user)).to be_falsey
     end
 
+    it "returns false when the user is not active" do
+      user.active = false
+      expect(Guardian.new(admin).can_approve?(user)).to be_falsey
+    end
+
     it "allows an admin to approve a user" do
       expect(Guardian.new(admin).can_approve?(user)).to be_truthy
     end