diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-26 18:08:16 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-26 18:08:16 +0300 |
commit | e80e0dd64fbb04f60394cb1bb08e17dbcb22b8ce (patch) | |
tree | 9e538341b9b77e96737964813e10235dbecf47ff /spec/lib | |
parent | ef31adeb0fb9a02b2c6a4529ec4e38d7082a4b2b (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec/lib')
3 files changed, 186 insertions, 0 deletions
diff --git a/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb b/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb new file mode 100644 index 00000000000..8603eb73bd5 --- /dev/null +++ b/spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::MigrateUsersBioToUserDetails, :migration, schema: 20200323074147 do + let(:users) { table(:users) } + + let(:user_details) do + klass = table(:user_details) + klass.primary_key = :user_id + klass + end + + let!(:user_needs_migration) { users.create(name: 'user1', email: 'test1@test.com', projects_limit: 1, bio: 'bio') } + let!(:user_needs_no_migration) { users.create(name: 'user2', email: 'test2@test.com', projects_limit: 1) } + let!(:user_also_needs_no_migration) { users.create(name: 'user3', email: 'test3@test.com', projects_limit: 1, bio: '') } + let!(:user_with_long_bio) { users.create(name: 'user4', email: 'test4@test.com', projects_limit: 1, bio: 'a' * 256) } # 255 is the max + + let!(:user_already_has_details) { users.create(name: 'user5', email: 'test5@test.com', projects_limit: 1, bio: 'my bio') } + let!(:existing_user_details) { user_details.find_or_create_by(user_id: user_already_has_details.id).update(bio: 'my bio') } + + # unlikely scenario since we have triggers + let!(:user_has_different_details) { users.create(name: 'user6', email: 'test6@test.com', projects_limit: 1, bio: 'different') } + let!(:different_existing_user_details) { user_details.find_or_create_by(user_id: user_has_different_details.id).update(bio: 'bio') } + + let(:user_ids) do + [ + user_needs_migration, + user_needs_no_migration, + user_also_needs_no_migration, + user_with_long_bio, + user_already_has_details, + user_has_different_details + ].map(&:id) + end + + subject { described_class.new.perform(user_ids.min, user_ids.max) } + + it 'migrates all relevant records' do + subject + + all_user_details = user_details.all + expect(all_user_details.size).to eq(4) + end + + it 'migrates `bio`' do + subject + + user_detail = user_details.find_by!(user_id: user_needs_migration.id) + + expect(user_detail.bio).to eq('bio') + end + + it 'migrates long `bio`' do + subject + + user_detail = user_details.find_by!(user_id: user_with_long_bio.id) + + expect(user_detail.bio).to eq('a' * 255) + end + + it 'does not change existing user detail' do + expect { subject }.not_to change { user_details.find_by!(user_id: user_already_has_details.id).attributes } + end + + it 'changes existing user detail when the columns are different' do + expect { subject }.to change { user_details.find_by!(user_id: user_has_different_details.id).bio }.from('bio').to('different') + end + + it 'does not migrate record' do + subject + + user_detail = user_details.find_by(user_id: user_needs_no_migration.id) + + expect(user_detail).to be_nil + end + + it 'does not migrate empty bio' do + subject + + user_detail = user_details.find_by(user_id: user_also_needs_no_migration.id) + + expect(user_detail).to be_nil + end + + context 'when `migrate_bio_to_user_details` feature flag is off' do + before do + stub_feature_flags(migrate_bio_to_user_details: false) + end + + it 'does nothing' do + already_existing_user_details = user_details.where(user_id: [ + user_has_different_details.id, + user_already_has_details.id + ]) + + subject + + expect(user_details.all).to match_array(already_existing_user_details) + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 9ac2660908c..8b765ce122d 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -657,6 +657,30 @@ describe Gitlab::Database::MigrationHelpers do end end + context 'when `update_column_in_batches_args` is given' do + let(:column) { UserDetail.columns.find { |c| c.name == "user_id" } } + + it 'uses `user_id` for `update_column_in_batches`' do + allow(model).to receive(:transaction_open?).and_return(false) + allow(model).to receive(:transaction).and_yield + allow(model).to receive(:column_for).with(:user_details, :foo).and_return(column) + allow(model).to receive(:update_column_in_batches).with(:user_details, :foo, 10, batch_column_name: :user_id) + allow(model).to receive(:change_column_null).with(:user_details, :foo, false) + allow(model).to receive(:change_column_default).with(:user_details, :foo, 10) + + expect(model).to receive(:add_column) + .with(:user_details, :foo, :integer, default: nil) + + model.add_column_with_default( + :user_details, + :foo, + :integer, + default: 10, + update_column_in_batches_args: { batch_column_name: :user_id } + ) + end + end + context 'when a column limit is set' do it 'adds the column with a limit' do allow(model).to receive(:transaction_open?).and_return(false) diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 7e2b5ed534f..61e893bfb3c 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -78,6 +78,66 @@ describe Gitlab::ImportExport::MembersMapper do members_mapper.map end + context 'logging' do + let(:logger) { Gitlab::Import::Logger.build } + + before do + allow(logger).to receive(:info) + allow(members_mapper).to receive(:logger).and_return(logger) + end + + it 'logs member addition' do + expected_log_params = ->(user_id) do + { + user_id: user_id, + root_namespace_id: importable.root_ancestor.id, + importable_type: importable.class.to_s, + importable_id: importable.id, + access_level: exported_members.first['access_level'], + message: '[Project/Group Import] Added new member' + } + end + + expect(logger).to receive(:info).with(hash_including(expected_log_params.call(user2.id))).once + expect(logger).to receive(:info).with(hash_including(expected_log_params.call(nil))).once + + members_mapper.map + end + + context 'when exporter member is invalid' do + let(:exported_members) do + [ + { + "id" => 2, + "access_level" => -5, # invalid access level + "source_id" => 14, + "source_type" => source_type, + "notification_level" => 3, + "created_at" => "2016-03-11T10:21:44.822Z", + "updated_at" => "2016-03-11T10:21:44.822Z", + "created_by_id" => nil, + "invite_email" => nil, + "invite_token" => nil, + "invite_accepted_at" => nil, + "user" => + { + "id" => exported_user_id, + "email" => user2.email, + "username" => 'test' + }, + "user_id" => 19 + } + ] + end + + it 'logs member addition failure' do + expect(logger).to receive(:info).with(hash_including(message: a_string_including('Access level is not included in the list'))).once + + members_mapper.map + end + end + end + context 'user is not an admin' do let(:user) { create(:user) } |