Commit c58e66d1 authored by Pavel Shutsin's avatar Pavel Shutsin

Migrate ghost logic to user_type column

User type column was filled with proper data for `ghost`
users in previous commits, so here we move logic
from old column to new column
parent ed413d76
...@@ -323,7 +323,7 @@ class User < ApplicationRecord ...@@ -323,7 +323,7 @@ class User < ApplicationRecord
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 :active_without_ghosts, -> { with_state(:active).without_ghosts }
scope :without_ghosts, -> { where('ghost IS NOT TRUE') } scope :without_ghosts, -> { humans.or(where.not(user_type: :ghost)) }
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')) }
...@@ -624,7 +624,7 @@ class User < ApplicationRecord ...@@ -624,7 +624,7 @@ class User < ApplicationRecord
# owns records previously belonging to deleted users. # owns records previously belonging to deleted users.
def ghost def ghost
email = 'ghost%s@example.com' email = 'ghost%s@example.com'
unique_internal(where(ghost: true, user_type: :ghost), 'ghost', email) do |u| unique_internal(where(user_type: :ghost), 'ghost', email) do |u|
u.bio = _('This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.') u.bio = _('This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.')
u.name = 'Ghost User' u.name = 'Ghost User'
end end
...@@ -664,17 +664,8 @@ class User < ApplicationRecord ...@@ -664,17 +664,8 @@ class User < ApplicationRecord
ghost? || (bot? && !project_bot?) ghost? || (bot? && !project_bot?)
end end
# We are transitioning from ghost boolean column to user_type
# so we need to read from old column for now
# @see https://gitlab.com/gitlab-org/gitlab/-/issues/210025
def ghost?
ghost
end
# The explicit check for project_bot will be removed with Bot Categorization
# Ref: https://gitlab.com/gitlab-org/gitlab/-/issues/213945
def self.internal def self.internal
where(ghost: true).or(bots_without_project_bot) where(user_type: :ghost).or(bots)
end end
# The explicit check for project_bot will be removed with Bot Categorization # The explicit check for project_bot will be removed with Bot Categorization
......
...@@ -90,7 +90,7 @@ describe User do ...@@ -90,7 +90,7 @@ describe User do
describe '.active_without_ghosts' do describe '.active_without_ghosts' do
let_it_be(:user1) { create(:user, :external) } let_it_be(:user1) { create(:user, :external) }
let_it_be(:user2) { create(:user, state: 'blocked') } let_it_be(:user2) { create(:user, state: 'blocked') }
let_it_be(:user3) { create(:user, ghost: true) } let_it_be(:user3) { create(:user, :ghost) }
let_it_be(:user4) { create(:user, user_type: :support_bot) } let_it_be(:user4) { create(:user, user_type: :support_bot) }
let_it_be(:user5) { create(:user, state: 'blocked', user_type: :support_bot) } let_it_be(:user5) { create(:user, state: 'blocked', user_type: :support_bot) }
...@@ -637,7 +637,7 @@ describe User do ...@@ -637,7 +637,7 @@ describe User do
end end
context 'when user is a ghost' do context 'when user is a ghost' do
let(:user) { create(:user, ghost: true) } let(:user) { create(:user, :ghost) }
it 'returns false' do it 'returns false' do
expect(user.using_license_seat?).to eq false expect(user.using_license_seat?).to eq false
......
...@@ -40,7 +40,7 @@ FactoryBot.define do ...@@ -40,7 +40,7 @@ FactoryBot.define do
end end
trait :ghost do trait :ghost do
ghost { true } user_type { :ghost }
after(:build) { |user, _| user.block! } after(:build) { |user, _| user.block! }
end end
......
...@@ -813,7 +813,7 @@ describe User, :do_not_mock_admin_mode do ...@@ -813,7 +813,7 @@ describe User, :do_not_mock_admin_mode do
describe '.active_without_ghosts' do describe '.active_without_ghosts' do
let_it_be(:user1) { create(:user, :external) } let_it_be(:user1) { create(:user, :external) }
let_it_be(:user2) { create(:user, state: 'blocked') } let_it_be(:user2) { create(:user, state: 'blocked') }
let_it_be(:user3) { create(:user, ghost: true) } let_it_be(:user3) { create(:user, :ghost) }
let_it_be(:user4) { create(:user) } let_it_be(:user4) { create(:user) }
it 'returns all active users but ghost users' do it 'returns all active users but ghost users' do
...@@ -824,7 +824,7 @@ describe User, :do_not_mock_admin_mode do ...@@ -824,7 +824,7 @@ describe User, :do_not_mock_admin_mode do
describe '.without_ghosts' do describe '.without_ghosts' do
let_it_be(:user1) { create(:user, :external) } let_it_be(:user1) { create(:user, :external) }
let_it_be(:user2) { create(:user, state: 'blocked') } let_it_be(:user2) { create(:user, state: 'blocked') }
let_it_be(:user3) { create(:user, ghost: true) } let_it_be(:user3) { create(:user, :ghost) }
it 'returns users without ghosts users' do it 'returns users without ghosts users' do
expect(described_class.without_ghosts).to match_array([user1, user2]) expect(described_class.without_ghosts).to match_array([user1, user2])
...@@ -3275,7 +3275,6 @@ describe User, :do_not_mock_admin_mode do ...@@ -3275,7 +3275,6 @@ describe User, :do_not_mock_admin_mode do
expect(ghost.namespace).not_to be_nil expect(ghost.namespace).not_to be_nil
expect(ghost.namespace).to be_persisted expect(ghost.namespace).to be_persisted
expect(ghost.user_type).to eq 'ghost' expect(ghost.user_type).to eq 'ghost'
expect(ghost.ghost).to eq true
end end
it "does not create a second ghost user if one is already present" do it "does not create a second ghost user if one is already present" do
...@@ -4077,7 +4076,7 @@ describe User, :do_not_mock_admin_mode do ...@@ -4077,7 +4076,7 @@ describe User, :do_not_mock_admin_mode do
context 'in single-user environment' do context 'in single-user environment' do
it 'requires user consent after one week' do it 'requires user consent after one week' do
create(:user, ghost: true) create(:user, :ghost)
expect(user.requires_usage_stats_consent?).to be true expect(user.requires_usage_stats_consent?).to be true
end end
...@@ -4503,7 +4502,7 @@ describe User, :do_not_mock_admin_mode do ...@@ -4503,7 +4502,7 @@ describe User, :do_not_mock_admin_mode do
where(:attributes) do where(:attributes) do
[ [
{ state: 'blocked' }, { state: 'blocked' },
{ ghost: true }, { user_type: :ghost },
{ user_type: :alert_bot } { user_type: :alert_bot }
] ]
end end
...@@ -4546,7 +4545,7 @@ describe User, :do_not_mock_admin_mode do ...@@ -4546,7 +4545,7 @@ describe User, :do_not_mock_admin_mode do
context 'when user is a ghost user' do context 'when user is a ghost user' do
before do before do
user.update(ghost: true) user.update(user_type: :ghost)
end end
it { is_expected.to be false } it { is_expected.to be false }
...@@ -4585,7 +4584,7 @@ describe User, :do_not_mock_admin_mode do ...@@ -4585,7 +4584,7 @@ describe User, :do_not_mock_admin_mode do
context 'when user is an internal user' do context 'when user is an internal user' do
before do before do
user.update(ghost: true) user.update(user_type: :ghost)
end end
it { is_expected.to be User::LOGIN_FORBIDDEN } it { is_expected.to be User::LOGIN_FORBIDDEN }
......
...@@ -9,7 +9,7 @@ describe 'admin/users/_user.html.haml' do ...@@ -9,7 +9,7 @@ describe 'admin/users/_user.html.haml' do
context 'internal users' do context 'internal users' do
context 'when showing a `Ghost User`' do context 'when showing a `Ghost User`' do
let(:user) { create(:user, ghost: true) } let(:user) { create(:user, :ghost) }
it 'does not render action buttons' do it 'does not render action buttons' do
render render
......
...@@ -18,7 +18,6 @@ describe UpdateHighestRoleWorker, :clean_gitlab_redis_shared_state do ...@@ -18,7 +18,6 @@ describe UpdateHighestRoleWorker, :clean_gitlab_redis_shared_state do
let(:active_attributes) do let(:active_attributes) do
{ {
state: 'active', state: 'active',
ghost: false,
user_type: nil user_type: nil
} }
end end
...@@ -54,7 +53,6 @@ describe UpdateHighestRoleWorker, :clean_gitlab_redis_shared_state do ...@@ -54,7 +53,6 @@ describe UpdateHighestRoleWorker, :clean_gitlab_redis_shared_state do
where(:additional_attributes) do where(:additional_attributes) do
[ [
{ state: 'blocked' }, { state: 'blocked' },
{ ghost: true },
{ user_type: :alert_bot } { user_type: :alert_bot }
] ]
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