summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-03-26 15:08:16 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-03-26 15:08:16 +0000
commite80e0dd64fbb04f60394cb1bb08e17dbcb22b8ce (patch)
tree9e538341b9b77e96737964813e10235dbecf47ff /spec
parentef31adeb0fb9a02b2c6a4529ec4e38d7082a4b2b (diff)
downloadgitlab-ce-e80e0dd64fbb04f60394cb1bb08e17dbcb22b8ce.tar.gz
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
-rw-r--r--spec/graphql/mutations/concerns/mutations/resolves_issuable_spec.rb73
-rw-r--r--spec/lib/gitlab/background_migration/migrate_users_bio_to_user_details_spec.rb102
-rw-r--r--spec/lib/gitlab/database/migration_helpers_spec.rb24
-rw-r--r--spec/lib/gitlab/import_export/members_mapper_spec.rb60
-rw-r--r--spec/models/member_spec.rb100
-rw-r--r--spec/models/user_spec.rb61
-rw-r--r--spec/requests/api/users_spec.rb11
-rw-r--r--spec/support/shared_examples/graphql/resolves_issuable_shared_examples.rb55
8 files changed, 399 insertions, 87 deletions
diff --git a/spec/graphql/mutations/concerns/mutations/resolves_issuable_spec.rb b/spec/graphql/mutations/concerns/mutations/resolves_issuable_spec.rb
index 75b5d31d134..145e42e2a51 100644
--- a/spec/graphql/mutations/concerns/mutations/resolves_issuable_spec.rb
+++ b/spec/graphql/mutations/concerns/mutations/resolves_issuable_spec.rb
@@ -3,76 +3,27 @@
require 'spec_helper'
describe Mutations::ResolvesIssuable do
- let(:mutation_class) do
+ let_it_be(:mutation_class) do
Class.new(Mutations::BaseMutation) do
include Mutations::ResolvesIssuable
end
end
- let(:project) { create(:project) }
- let(:user) { create(:user) }
- let(:context) { { current_user: user } }
- let(:mutation) { mutation_class.new(object: nil, context: context, field: nil) }
-
- shared_examples 'resolving an issuable' do |type|
- context 'when user has access' do
- let(:source) { type == :merge_request ? 'source_project' : 'project' }
- let(:issuable) { create(type, author: user, "#{source}" => project) }
-
- subject { mutation.resolve_issuable(type: type, parent_path: project.full_path, iid: issuable.iid) }
-
- before do
- project.add_developer(user)
- end
-
- it 'resolves issuable by iid' do
- result = type == :merge_request ? subject.sync : subject
- expect(result).to eq(issuable)
- end
-
- it 'uses the correct Resolver to resolve issuable' do
- resolver_class = "Resolvers::#{type.to_s.classify.pluralize}Resolver".constantize
- resolved_project = mutation.resolve_project(full_path: project.full_path)
-
- allow(mutation).to receive(:resolve_project)
- .with(full_path: project.full_path)
- .and_return(resolved_project)
-
- expect(resolver_class).to receive(:new)
- .with(object: resolved_project, context: context, field: nil)
- .and_call_original
-
- subject
- end
-
- it 'uses the ResolvesProject to resolve project' do
- expect(Resolvers::ProjectResolver).to receive(:new)
- .with(object: nil, context: context, field: nil)
- .and_call_original
-
- subject
- end
-
- it 'returns nil if issuable is not found' do
- result = mutation.resolve_issuable(type: type, parent_path: project.full_path, iid: "100")
- result = type == :merge_request ? result.sync : result
-
- expect(result).to be_nil
- end
-
- it 'returns nil if parent path is empty' do
- result = mutation.resolve_issuable(type: type, parent_path: "", iid: issuable.iid)
-
- expect(result).to be_nil
- end
- end
- end
+ let_it_be(:project) { create(:project) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:context) { { current_user: user } }
+ let_it_be(:mutation) { mutation_class.new(object: nil, context: context, field: nil) }
+ let(:parent) { issuable.project }
context 'with issues' do
- it_behaves_like 'resolving an issuable', :issue
+ let(:issuable) { create(:issue, project: project) }
+
+ it_behaves_like 'resolving an issuable in GraphQL', :issue
end
context 'with merge requests' do
- it_behaves_like 'resolving an issuable', :merge_request
+ let(:issuable) { create(:merge_request, source_project: project) }
+
+ it_behaves_like 'resolving an issuable in GraphQL', :merge_request
end
end
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) }
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index ce3ee3fcfb0..13a0426624b 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -586,49 +586,97 @@ describe Member do
end
context 'when after_commit :update_highest_role' do
- where(:member_type, :source_type) do
- :project_member | :project
- :group_member | :group
- end
+ context 'with feature flag enabled' do
+ where(:member_type, :source_type) do
+ :project_member | :project
+ :group_member | :group
+ end
- with_them do
- describe 'create member' do
- it 'initializes a new Members::UpdateHighestRoleService object' do
- source = create(source_type) # source owner initializes a new service object too
- user = create(:user)
+ with_them do
+ describe 'create member' do
+ it 'initializes a new Members::UpdateHighestRoleService object' do
+ source = create(source_type) # source owner initializes a new service object too
+ user = create(:user)
- expect(Members::UpdateHighestRoleService).to receive(:new).with(user.id).and_call_original
+ expect(Members::UpdateHighestRoleService).to receive(:new).with(user.id).and_call_original
- create(member_type, :guest, user: user, source_type => source)
+ create(member_type, :guest, user: user, source_type => source)
+ end
end
- end
- context 'when member exists' do
- let!(:member) { create(member_type) }
+ context 'when member exists' do
+ let!(:member) { create(member_type) }
+
+ describe 'update member' do
+ context 'when access level was changed' do
+ it 'initializes a new Members::UpdateHighestRoleService object' do
+ expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original
+
+ member.update(access_level: Gitlab::Access::GUEST)
+ end
+ end
+
+ context 'when access level was not changed' do
+ it 'does not initialize a new Members::UpdateHighestRoleService object' do
+ expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id)
+
+ member.update(notification_level: NotificationSetting.levels[:disabled])
+ end
+ end
+ end
- describe 'update member' do
- context 'when access level was changed' do
+ describe 'destroy member' do
it 'initializes a new Members::UpdateHighestRoleService object' do
expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original
- member.update(access_level: Gitlab::Access::GUEST)
+ member.destroy
end
end
+ end
+ end
+ end
- context 'when access level was not changed' do
- it 'does not initialize a new Members::UpdateHighestRoleService object' do
- expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id)
+ context 'with feature flag disabled' do
+ before do
+ stub_feature_flags(highest_role_callback: false)
+ end
- member.update(notification_level: NotificationSetting.levels[:disabled])
- end
+ where(:member_type, :source_type) do
+ :project_member | :project
+ :group_member | :group
+ end
+
+ with_them do
+ describe 'create member' do
+ it 'does not initialize a new Members::UpdateHighestRoleService object' do
+ source = create(source_type)
+ user = create(:user)
+
+ expect(Members::UpdateHighestRoleService).not_to receive(:new).with(user.id)
+
+ create(member_type, :guest, user: user, source_type => source)
end
end
- describe 'destroy member' do
- it 'initializes a new Members::UpdateHighestRoleService object' do
- expect(Members::UpdateHighestRoleService).to receive(:new).with(member.user_id).and_call_original
+ context 'when member exists' do
+ let!(:member) { create(member_type) }
- member.destroy
+ describe 'update member' do
+ context 'when access level was changed' do
+ it 'does not initialize a new Members::UpdateHighestRoleService object' do
+ expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id)
+
+ member.update(access_level: Gitlab::Access::GUEST)
+ end
+ end
+ end
+
+ describe 'destroy member' do
+ it 'does not initialize a new Members::UpdateHighestRoleService object' do
+ expect(Members::UpdateHighestRoleService).not_to receive(:new).with(member.user_id)
+
+ member.destroy
+ end
end
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index bbd45afadd7..b7abcb31321 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -55,6 +55,67 @@ describe User, :do_not_mock_admin_mode do
it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') }
it { is_expected.to have_many(:releases).dependent(:nullify) }
+ describe "#bio" do
+ it 'syncs bio with `user_details.bio` on create' do
+ user = create(:user, bio: 'my bio')
+
+ expect(user.bio).to eq(user.user_detail.bio)
+ 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 not sync bio with `user_details.bio`' do
+ user = create(:user, bio: 'my bio')
+
+ expect(user.bio).to eq('my bio')
+ expect(user.user_detail.bio).to eq('')
+ end
+ end
+
+ it 'syncs bio with `user_details.bio` on update' do
+ user = create(:user)
+
+ user.update!(bio: 'my bio')
+
+ expect(user.bio).to eq(user.user_detail.bio)
+ end
+
+ context 'when `user_details` association already exists' do
+ let(:user) { create(:user) }
+
+ before do
+ create(:user_detail, user: user)
+ end
+
+ it 'syncs bio with `user_details.bio`' do
+ user.update!(bio: 'my bio')
+
+ expect(user.bio).to eq(user.user_detail.bio)
+ end
+
+ it 'falls back to "" when nil is given' do
+ user.update!(bio: nil)
+
+ expect(user.bio).to eq(nil)
+ expect(user.user_detail.bio).to eq('')
+ end
+
+ # very unlikely scenario
+ it 'truncates long bio when syncing to user_details' do
+ invalid_bio = 'a' * 256
+ truncated_bio = 'a' * 255
+
+ user.bio = invalid_bio
+ user.save(validate: false)
+
+ expect(user.user_detail.bio).to eq(truncated_bio)
+ end
+ end
+ end
+
describe "#abuse_report" do
let(:current_user) { create(:user) }
let(:other_user) { create(:user) }
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 6d1b76a9aea..92cc6dc887e 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -739,6 +739,17 @@ describe API::Users, :do_not_mock_admin_mode do
expect(user.reload.bio).to eq('new test bio')
end
+ it "updates user with empty bio" do
+ user.bio = 'previous bio'
+ user.save!
+
+ put api("/users/#{user.id}", admin), params: { bio: '' }
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response['bio']).to eq('')
+ expect(user.reload.bio).to eq('')
+ end
+
it "updates user with new password and forces reset on next login" do
put api("/users/#{user.id}", admin), params: { password: '12345678' }
diff --git a/spec/support/shared_examples/graphql/resolves_issuable_shared_examples.rb b/spec/support/shared_examples/graphql/resolves_issuable_shared_examples.rb
new file mode 100644
index 00000000000..b56181371c3
--- /dev/null
+++ b/spec/support/shared_examples/graphql/resolves_issuable_shared_examples.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'resolving an issuable in GraphQL' do |type|
+ subject { mutation.resolve_issuable(type: type, parent_path: parent.full_path, iid: issuable.iid) }
+
+ context 'when user has access' do
+ before do
+ parent.add_developer(user)
+ end
+
+ it 'resolves issuable by iid' do
+ result = type == :merge_request ? subject.sync : subject
+ expect(result).to eq(issuable)
+ end
+
+ it 'uses the correct Resolver to resolve issuable' do
+ resolver_class = "Resolvers::#{type.to_s.classify.pluralize}Resolver".constantize
+ resolve_method = type == :epic ? :resolve_group : :resolve_project
+ resolved_parent = mutation.send(resolve_method, full_path: parent.full_path)
+
+ allow(mutation).to receive(resolve_method)
+ .with(full_path: parent.full_path)
+ .and_return(resolved_parent)
+
+ expect(resolver_class).to receive(:new)
+ .with(object: resolved_parent, context: context, field: nil)
+ .and_call_original
+
+ subject
+ end
+
+ it 'uses correct Resolver to resolve issuable parent' do
+ resolver_class = type == :epic ? 'Resolvers::GroupResolver' : 'Resolvers::ProjectResolver'
+
+ expect(resolver_class.constantize).to receive(:new)
+ .with(object: nil, context: context, field: nil)
+ .and_call_original
+
+ subject
+ end
+
+ it 'returns nil if issuable is not found' do
+ result = mutation.resolve_issuable(type: type, parent_path: parent.full_path, iid: "100")
+ result = type == :merge_request ? result.sync : result
+
+ expect(result).to be_nil
+ end
+
+ it 'returns nil if parent path is not present' do
+ result = mutation.resolve_issuable(type: type, parent_path: "", iid: issuable.iid)
+
+ expect(result).to be_nil
+ end
+ end
+end