Commit ff11bd9c authored by Shreyas Agarwal's avatar Shreyas Agarwal Committed by Rémy Coutable

Add scopes and refactor code with better naming

Created scopes with better names and
add specs to CE and also EE seperating
the bots use within the users.
parent ecd48254
...@@ -307,6 +307,8 @@ class User < ApplicationRecord ...@@ -307,6 +307,8 @@ class User < ApplicationRecord
scope :blocked, -> { with_states(:blocked, :ldap_blocked) } scope :blocked, -> { with_states(:blocked, :ldap_blocked) }
scope :external, -> { where(external: true) } scope :external, -> { where(external: true) }
scope :active, -> { with_state(:active).non_internal } scope :active, -> { with_state(:active).non_internal }
scope :active_without_ghosts, -> { with_state(:active).without_ghosts }
scope :without_ghosts, -> { where('ghost IS NOT TRUE') }
scope :deactivated, -> { with_state(:deactivated).non_internal } scope :deactivated, -> { with_state(:deactivated).non_internal }
scope :without_projects, -> { joins('LEFT JOIN project_authorizations ON users.id = project_authorizations.user_id').where(project_authorizations: { user_id: nil }) } scope :without_projects, -> { joins('LEFT JOIN project_authorizations ON users.id = project_authorizations.user_id').where(project_authorizations: { user_id: nil }) }
scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) } scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) }
...@@ -470,7 +472,7 @@ class User < ApplicationRecord ...@@ -470,7 +472,7 @@ class User < ApplicationRecord
when 'deactivated' when 'deactivated'
deactivated deactivated
else else
active active_without_ghosts
end end
end end
...@@ -614,7 +616,7 @@ class User < ApplicationRecord ...@@ -614,7 +616,7 @@ class User < ApplicationRecord
end end
def self.non_internal def self.non_internal
where('ghost IS NOT TRUE') without_ghosts
end end
# #
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
= nav_link(html_options: { class: active_when(params[:filter].nil?) }) do = nav_link(html_options: { class: active_when(params[:filter].nil?) }) do
= link_to admin_users_path do = link_to admin_users_path do
= s_('AdminUsers|Active') = s_('AdminUsers|Active')
%small.badge.badge-pill= limited_counter_with_delimiter(User.active) %small.badge.badge-pill= limited_counter_with_delimiter(User.active_without_ghosts)
= nav_link(html_options: { class: active_when(params[:filter] == 'admins') }) do = nav_link(html_options: { class: active_when(params[:filter] == 'admins') }) do
= link_to admin_users_path(filter: "admins") do = link_to admin_users_path(filter: "admins") do
= s_('AdminUsers|Admins') = s_('AdminUsers|Admins')
......
---
title: The Active tab on the Admin Users page should include bots
merge_request: 22543
author:
type: fixed
...@@ -96,6 +96,18 @@ describe User do ...@@ -96,6 +96,18 @@ describe User do
expect(users_with_invalid_tokens).not_to include valid_pat.user expect(users_with_invalid_tokens).not_to include valid_pat.user
end end
end end
describe '.active_without_ghosts' do
let_it_be(:user1) { create(:user, :external) }
let_it_be(:user2) { create(:user, state: 'blocked') }
let_it_be(:user3) { create(:user, ghost: true) }
let_it_be(:user4) { create(:user, bot_type: 'support_bot') }
let_it_be(:user5) { create(:user, state: 'blocked', bot_type: 'support_bot') }
it 'returns all active users including active bots but ghost users' do
expect(described_class.active_without_ghosts).to match_array([user1, user4])
end
end
end end
describe '.find_by_smartcard_identity' do describe '.find_by_smartcard_identity' do
......
...@@ -633,6 +633,27 @@ describe User, :do_not_mock_admin_mode do ...@@ -633,6 +633,27 @@ describe User, :do_not_mock_admin_mode do
end end
end end
end end
describe '.active_without_ghosts' do
let_it_be(:user1) { create(:user, :external) }
let_it_be(:user2) { create(:user, state: 'blocked') }
let_it_be(:user3) { create(:user, ghost: true) }
let_it_be(:user4) { create(:user) }
it 'returns all active users but ghost users' do
expect(described_class.active_without_ghosts).to match_array([user1, user4])
end
end
describe '.without_ghosts' do
let_it_be(:user1) { create(:user, :external) }
let_it_be(:user2) { create(:user, state: 'blocked') }
let_it_be(:user3) { create(:user, ghost: true) }
it 'returns users without ghosts users' do
expect(described_class.without_ghosts).to match_array([user1, user2])
end
end
end end
describe "Respond to" do describe "Respond to" do
...@@ -1252,7 +1273,7 @@ describe User, :do_not_mock_admin_mode do ...@@ -1252,7 +1273,7 @@ describe User, :do_not_mock_admin_mode do
let(:user) { double } let(:user) { double }
it 'filters by active users by default' do it 'filters by active users by default' do
expect(described_class).to receive(:active).and_return([user]) expect(described_class).to receive(:active_without_ghosts).and_return([user])
expect(described_class.filter_items(nil)).to include user expect(described_class.filter_items(nil)).to include user
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment