diff --git a/app/assets/javascripts/admin/templates/user-index.hbs b/app/assets/javascripts/admin/templates/user-index.hbs index 7a016f150f1..da712cd51ef 100644 --- a/app/assets/javascripts/admin/templates/user-index.hbs +++ b/app/assets/javascripts/admin/templates/user-index.hbs @@ -466,7 +466,7 @@ {{/if}}
-
+
{{#unless model.anonymizeForbidden}} {{d-button label="admin.user.anonymize" @@ -487,7 +487,7 @@ {{#if model.deleteExplanation}}
-
+
{{d-icon "exclamation-triangle"}} {{model.deleteExplanation}}
diff --git a/lib/admin_user_index_query.rb b/lib/admin_user_index_query.rb index f333f5619b9..eb36e4a25d4 100644 --- a/lib/admin_user_index_query.rb +++ b/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 diff --git a/lib/guardian.rb b/lib/guardian.rb index a09d11ae1d1..773dfcb0da0 100644 --- a/lib/guardian.rb +++ b/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) diff --git a/spec/components/admin_user_index_query_spec.rb b/spec/components/admin_user_index_query_spec.rb index 913e997d0c1..f885350202f 100644 --- a/spec/components/admin_user_index_query_spec.rb +++ b/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 diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index dcc5f22937d..87a3413c4f7 100644 --- a/spec/components/guardian_spec.rb +++ b/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