diff options
-rw-r--r-- | app/models/user.rb | 12 | ||||
-rw-r--r-- | app/services/users/build_service.rb | 2 | ||||
-rw-r--r-- | config/feature_flags/development/create_user_ns_outside_model.yml | 8 | ||||
-rw-r--r-- | db/fixtures/development/01_admin.rb | 4 | ||||
-rw-r--r-- | db/fixtures/development/02_users.rb | 4 | ||||
-rw-r--r-- | db/fixtures/development/18_abuse_reports.rb | 4 | ||||
-rw-r--r-- | db/fixtures/development/33_triage_ops.rb | 21 | ||||
-rw-r--r-- | db/fixtures/development/37_timelogs.rb | 6 | ||||
-rw-r--r-- | lib/tasks/gitlab/seed/group_seed.rake | 6 | ||||
-rw-r--r-- | lib/users/internal.rb | 1 | ||||
-rw-r--r-- | spec/factories/users.rb | 4 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 67 |
12 files changed, 112 insertions, 27 deletions
diff --git a/app/models/user.rb b/app/models/user.rb index a7c90765be1..b9dbba27a94 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -151,6 +151,7 @@ class User < MainClusterwide::ApplicationRecord # Namespace for personal projects has_one :namespace, -> { where(type: Namespaces::UserNamespace.sti_name) }, + required: true, dependent: :destroy, # rubocop:disable Cop/ActiveRecordDependent foreign_key: :owner_id, inverse_of: :owner, @@ -1602,7 +1603,7 @@ class User < MainClusterwide::ApplicationRecord if namespace namespace.path = username if username_changed? namespace.name = name if name_changed? - else + elsif Feature.disabled?(:create_user_ns_outside_model) # TODO: we should no longer need the `type` parameter once we can make the # the `has_one :namespace` association use the correct class. # issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070 @@ -1611,6 +1612,15 @@ class User < MainClusterwide::ApplicationRecord end end + def assign_personal_namespace + return namespace if namespace + + build_namespace(path: username, name: name) + namespace.build_namespace_settings + + namespace + end + def set_username_errors namespace_path_errors = self.errors.delete(:"namespace.path") diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index b51684c6899..2a9e4be91d3 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -36,6 +36,8 @@ module Users else standard_build_user end + + user.assign_personal_namespace end def admin? diff --git a/config/feature_flags/development/create_user_ns_outside_model.yml b/config/feature_flags/development/create_user_ns_outside_model.yml new file mode 100644 index 00000000000..2a97dff9e73 --- /dev/null +++ b/config/feature_flags/development/create_user_ns_outside_model.yml @@ -0,0 +1,8 @@ +--- +name: create_user_ns_outside_model +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/136402 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/431542 +milestone: '16.7' +type: development +group: group::tenant scale +default_enabled: false diff --git a/db/fixtures/development/01_admin.rb b/db/fixtures/development/01_admin.rb index b4778a6ecbd..99989da3ac6 100644 --- a/db/fixtures/development/01_admin.rb +++ b/db/fixtures/development/01_admin.rb @@ -9,7 +9,9 @@ Gitlab::Seeder.quiet do admin: true, confirmed_at: DateTime.now, password_expires_at: DateTime.now - ) + ) do |user| + user.assign_personal_namespace + end print '.' end diff --git a/db/fixtures/development/02_users.rb b/db/fixtures/development/02_users.rb index ceed0d6d12a..cdb2fa062e4 100644 --- a/db/fixtures/development/02_users.rb +++ b/db/fixtures/development/02_users.rb @@ -78,7 +78,9 @@ class Gitlab::Seeder::Users email: FFaker::Internet.email, confirmed_at: DateTime.now, password: random_password - ) + ) do |user| + user.assign_personal_namespace + end print '.' rescue ActiveRecord::RecordInvalid diff --git a/db/fixtures/development/18_abuse_reports.rb b/db/fixtures/development/18_abuse_reports.rb index 7dd93069122..773b0443985 100644 --- a/db/fixtures/development/18_abuse_reports.rb +++ b/db/fixtures/development/18_abuse_reports.rb @@ -12,7 +12,9 @@ module Db email: FFaker::Internet.email, confirmed_at: DateTime.now, password: ::User.random_password - ) + ) do |user| + user.assign_personal_namespace + end ::AbuseReport.create(reporter: ::User.take, user: reported_user, message: 'User sends spam') print '.' diff --git a/db/fixtures/development/33_triage_ops.rb b/db/fixtures/development/33_triage_ops.rb index 5cdb1496c3d..ecec4c9ba93 100644 --- a/db/fixtures/development/33_triage_ops.rb +++ b/db/fixtures/development/33_triage_ops.rb @@ -121,14 +121,7 @@ class Gitlab::Seeder::TriageOps private def ensure_bot_user - bot = User.find_by_username('triagebot') - bot ||= User.create!( - username: 'triagebot', - name: 'Triage Bot', - email: 'triagebot@example.com', - confirmed_at: DateTime.now, - password: SecureRandom.hex.slice(0, 16) - ) + bot = User.find_by_username('triagebot') || build_bot_user! ensure_group('gitlab-org').add_maintainer(bot) ensure_group('gitlab-com').add_maintainer(bot) @@ -148,6 +141,18 @@ class Gitlab::Seeder::TriageOps bot end + def build_bot_user! + User.create!( + username: 'triagebot', + name: 'Triage Bot', + email: 'triagebot@example.com', + confirmed_at: DateTime.now, + password: SecureRandom.hex.slice(0, 16) + ) do |user| + user.assign_personal_namespace + end + end + def ensure_webhook_for(group_path) group = Group.find_by_full_path(group_path) diff --git a/db/fixtures/development/37_timelogs.rb b/db/fixtures/development/37_timelogs.rb index b5be9d21cb3..9c4e980e890 100644 --- a/db/fixtures/development/37_timelogs.rb +++ b/db/fixtures/development/37_timelogs.rb @@ -63,13 +63,15 @@ class Gitlab::Seeder::Timelogs if user.nil? print "\nCreating user '#{username}' with password: '#{password}'" - user = User.create!( + User.create!( username: username, name: FFaker::Name.name, email: FFaker::Internet.email, confirmed_at: DateTime.now, password: password - ) + ) do |user| + user.assign_personal_namespace + end end user diff --git a/lib/tasks/gitlab/seed/group_seed.rake b/lib/tasks/gitlab/seed/group_seed.rake index 4f5df7841e2..cc9180d56a3 100644 --- a/lib/tasks/gitlab/seed/group_seed.rake +++ b/lib/tasks/gitlab/seed/group_seed.rake @@ -120,13 +120,17 @@ class GroupSeeder end def create_user + # rubocop:disable Style/SymbolProc -- Incorrect rubocop advice. User.create!( username: FFaker::Internet.user_name, name: FFaker::Name.name, email: FFaker::Internet.email, confirmed_at: DateTime.now, password: Devise.friendly_token - ) + ) do |user| + user.assign_personal_namespace + end + # rubocop:enable Style/SymbolProc end def create_member(user_id, group_id) diff --git a/lib/users/internal.rb b/lib/users/internal.rb index 30ef20dbd7b..4b6df4ed928 100644 --- a/lib/users/internal.rb +++ b/lib/users/internal.rb @@ -138,6 +138,7 @@ module Users email: email, &creation_block ) + user.assign_personal_namespace Users::UpdateService.new(user, user: user).execute(validate: false) user diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 8b42631040e..7361d778388 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -12,6 +12,10 @@ FactoryBot.define do can_create_group { true } color_scheme_id { 1 } + after(:build) do |user, evaluator| + user.assign_personal_namespace if Feature.enabled?(:create_user_ns_outside_model) + end + trait :admin do admin { true } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index bb32c1a2896..6a21ab644bc 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -467,7 +467,7 @@ RSpec.describe User, feature_category: :user_profile do describe 'validations' do describe 'password' do - let!(:user) { build_stubbed(:user) } + let!(:user) { build(:user) } before do allow(Devise).to receive(:password_length).and_return(8..128) @@ -547,9 +547,7 @@ RSpec.describe User, feature_category: :user_profile do context 'namespace_move_dir_allowed' do context 'when the user is not a new record' do - before do - expect(user.new_record?).to eq(false) - end + let!(:user) { create(:user) } it 'checks when username changes' do expect(user).to receive(:namespace_move_dir_allowed) @@ -5688,21 +5686,35 @@ RSpec.describe User, feature_category: :user_profile do describe '#ensure_namespace_correct' do context 'for a new user' do - let(:user) { build(:user) } + let(:user) { described_class.new attributes_for(:user) } - it 'creates the namespace' do + it 'does not create the namespace' do expect(user.namespace).to be_nil - user.save! + user.valid? - expect(user.namespace).not_to be_nil - expect(user.namespace).to be_kind_of(Namespaces::UserNamespace) + expect(user.namespace).to be_nil end - it 'creates the namespace setting' do - user.save! + context 'when create_user_ns_outside_model feature flag is disabled' do + before do + stub_feature_flags(create_user_ns_outside_model: false) + end - expect(user.namespace.namespace_settings).to be_persisted + it 'creates the namespace' do + expect(user.namespace).to be_nil + + user.save! + + expect(user.namespace).to be_present + expect(user.namespace).to be_kind_of(Namespaces::UserNamespace) + end + + it 'creates the namespace setting' do + user.save! + + expect(user.namespace.namespace_settings).to be_persisted + end end end @@ -5770,6 +5782,37 @@ RSpec.describe User, feature_category: :user_profile do end end + describe '#assign_personal_namespace' do + subject(:personal_namespace) { user.assign_personal_namespace } + + context 'when namespace exists' do + let(:user) { build(:user) } + + it 'leaves the namespace untouched' do + expect { personal_namespace }.not_to change(user, :namespace) + end + + it 'returns the personal namespace' do + expect(personal_namespace).to eq(user.namespace) + end + end + + context 'when namespace does not exist' do + let(:user) { described_class.new attributes_for(:user) } + + it 'builds a new namespace' do + subject + + expect(user.namespace).to be_kind_of(Namespaces::UserNamespace) + expect(user.namespace.namespace_settings).to be_present + end + + it 'returns the personal namespace' do + expect(personal_namespace).to eq(user.namespace) + end + end + end + describe '#username_changed_hook' do context 'for a new user' do let(:user) { build(:user) } |