diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-18 19:00:14 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-18 19:00:14 +0000 |
| commit | 05f0ebba3a2c8ddf39e436f412dc2ab5bf1353b2 (patch) | |
| tree | 11d0f2a6ec31c7793c184106cedc2ded3d9a2cc5 /spec/models | |
| parent | ec73467c23693d0db63a797d10194da9e72a74af (diff) | |
| download | gitlab-ce-15.8.0-rc42.tar.gz | |
Add latest changes from gitlab-org/gitlab@15-8-stable-eev15.8.0-rc42
Diffstat (limited to 'spec/models')
77 files changed, 2225 insertions, 571 deletions
diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 3871b18fdd5..b07fafabbb5 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe AbuseReport do +RSpec.describe AbuseReport, feature_category: :insider_threat do let_it_be(:report, reload: true) { create(:abuse_report) } let_it_be(:user, reload: true) { create(:admin) } @@ -20,10 +20,29 @@ RSpec.describe AbuseReport do end describe 'validations' do + let(:http) { 'http://gitlab.com' } + let(:https) { 'https://gitlab.com' } + let(:ftp) { 'ftp://example.com' } + let(:javascript) { 'javascript:alert(window.opener.document.location)' } + it { is_expected.to validate_presence_of(:reporter) } it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:message) } - it { is_expected.to validate_uniqueness_of(:user_id).with_message('has already been reported') } + it { is_expected.to validate_presence_of(:category) } + + it do + is_expected.to validate_uniqueness_of(:user_id) + .scoped_to([:reporter_id, :category]) + .with_message('You have already reported this user') + end + + it { is_expected.to validate_length_of(:reported_from_url).is_at_most(512).allow_blank } + it { is_expected.to allow_value(http).for(:reported_from_url) } + it { is_expected.to allow_value(https).for(:reported_from_url) } + it { is_expected.not_to allow_value(ftp).for(:reported_from_url) } + it { is_expected.not_to allow_value(javascript).for(:reported_from_url) } + it { is_expected.to allow_value('http://localhost:9000').for(:reported_from_url) } + it { is_expected.to allow_value('https://gitlab.com').for(:reported_from_url) } end describe '#remove_user' do @@ -54,4 +73,21 @@ RSpec.describe AbuseReport do report.notify end end + + describe 'enums' do + let(:categories) do + { + spam: 1, + offensive: 2, + phishing: 3, + crypto: 4, + credentials: 5, + copyright: 6, + malware: 7, + other: 8 + } + end + + it { is_expected.to define_enum_for(:category).with_values(**categories) } + end end diff --git a/spec/models/achievements/achievement_spec.rb b/spec/models/achievements/achievement_spec.rb index 10c04d184af..9a5f4eee229 100644 --- a/spec/models/achievements/achievement_spec.rb +++ b/spec/models/achievements/achievement_spec.rb @@ -5,6 +5,9 @@ require 'spec_helper' RSpec.describe Achievements::Achievement, type: :model, feature_category: :users do describe 'associations' do it { is_expected.to belong_to(:namespace).required } + + it { is_expected.to have_many(:user_achievements).inverse_of(:achievement) } + it { is_expected.to have_many(:users).through(:user_achievements).inverse_of(:achievements) } end describe 'modules' do diff --git a/spec/models/achievements/user_achievement_spec.rb b/spec/models/achievements/user_achievement_spec.rb new file mode 100644 index 00000000000..a91cba2b5e2 --- /dev/null +++ b/spec/models/achievements/user_achievement_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Achievements::UserAchievement, type: :model, feature_category: :users do + describe 'associations' do + it { is_expected.to belong_to(:achievement).inverse_of(:user_achievements).required } + it { is_expected.to belong_to(:user).inverse_of(:user_achievements).required } + + it { is_expected.to belong_to(:awarded_by_user).class_name('User').inverse_of(:awarded_user_achievements).optional } + it { is_expected.to belong_to(:revoked_by_user).class_name('User').inverse_of(:revoked_user_achievements).optional } + end +end diff --git a/spec/models/analytics/cycle_analytics/aggregation_spec.rb b/spec/models/analytics/cycle_analytics/aggregation_spec.rb index 2fb40852791..a51c21dc87e 100644 --- a/spec/models/analytics/cycle_analytics/aggregation_spec.rb +++ b/spec/models/analytics/cycle_analytics/aggregation_spec.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Analytics::CycleAnalytics::Aggregation, type: :model do +RSpec.describe Analytics::CycleAnalytics::Aggregation, type: :model, feature_category: :value_stream_management do describe 'associations' do - it { is_expected.to belong_to(:group).required } + it { is_expected.to belong_to(:namespace).required } end describe 'validations' do - it { is_expected.not_to validate_presence_of(:group) } + it { is_expected.not_to validate_presence_of(:namespace) } it { is_expected.not_to validate_presence_of(:enabled) } %i[incremental_runtimes_in_seconds incremental_processed_records full_runtimes_in_seconds full_processed_records].each do |column| @@ -18,6 +18,10 @@ RSpec.describe Analytics::CycleAnalytics::Aggregation, type: :model do expect(record.errors).to have_key(column) end end + + it_behaves_like 'value stream analytics namespace models' do + let(:factory_name) { :cycle_analytics_aggregation } + end end describe 'attribute updater methods' do @@ -126,19 +130,19 @@ RSpec.describe Analytics::CycleAnalytics::Aggregation, type: :model do end end - describe '#safe_create_for_group' do + describe '#safe_create_for_namespace' do let_it_be(:group) { create(:group) } let_it_be(:subgroup) { create(:group, parent: group) } it 'creates the aggregation record' do - record = described_class.safe_create_for_group(group) + record = described_class.safe_create_for_namespace(group) expect(record).to be_persisted end context 'when non top-level group is given' do it 'creates the aggregation record for the top-level group' do - record = described_class.safe_create_for_group(subgroup) + record = described_class.safe_create_for_namespace(subgroup) expect(record).to be_persisted end @@ -146,11 +150,11 @@ RSpec.describe Analytics::CycleAnalytics::Aggregation, type: :model do context 'when the record is already present' do it 'does nothing' do - described_class.safe_create_for_group(group) + described_class.safe_create_for_namespace(group) expect do - described_class.safe_create_for_group(group) - described_class.safe_create_for_group(subgroup) + described_class.safe_create_for_namespace(group) + described_class.safe_create_for_namespace(subgroup) end.not_to change { described_class.count } end end diff --git a/spec/models/analytics/cycle_analytics/project_stage_spec.rb b/spec/models/analytics/cycle_analytics/project_stage_spec.rb index 697b7aee022..3c7fde17355 100644 --- a/spec/models/analytics/cycle_analytics/project_stage_spec.rb +++ b/spec/models/analytics/cycle_analytics/project_stage_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Analytics::CycleAnalytics::ProjectStage do describe 'associations' do - it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:project).required } end it 'default stages must be valid' do diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 289408231a9..54dc280d7ac 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -14,7 +14,7 @@ RSpec.describe Appearance do subject(:appearance) { described_class.new } it { expect(appearance.title).to eq('') } - it { expect(appearance.short_title).to eq('') } + it { expect(appearance.pwa_short_name).to eq('') } it { expect(appearance.description).to eq('') } it { expect(appearance.new_project_guidelines).to eq('') } it { expect(appearance.profile_image_guidelines).to eq('') } @@ -77,7 +77,7 @@ RSpec.describe Appearance do end end - %i(logo header_logo favicon).each do |logo_type| + %i(logo header_logo pwa_icon favicon).each do |logo_type| it_behaves_like 'logo paths', logo_type end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 1454c82c531..5b99c68ec80 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ApplicationSetting do +RSpec.describe ApplicationSetting, feature_category: :not_owned, type: :model do using RSpec::Parameterized::TableSyntax subject(:setting) { described_class.create_from_defaults } @@ -128,6 +128,10 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_presence_of(:max_terraform_state_size_bytes) } it { is_expected.to validate_numericality_of(:max_terraform_state_size_bytes).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to allow_value(true).for(:user_defaults_to_private_profile) } + it { is_expected.to allow_value(false).for(:user_defaults_to_private_profile) } + it { is_expected.not_to allow_value(nil).for(:user_defaults_to_private_profile) } + it 'ensures max_pages_size is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do is_expected.to validate_numericality_of(:max_pages_size).only_integer.is_greater_than_or_equal_to(0) .is_less_than(::Gitlab::Pages::MAX_SIZE / 1.megabyte) @@ -220,6 +224,10 @@ RSpec.describe ApplicationSetting do it { is_expected.to allow_value(false).for(:bulk_import_enabled) } it { is_expected.not_to allow_value(nil).for(:bulk_import_enabled) } + it { is_expected.to allow_value(true).for(:allow_runner_registration_token) } + it { is_expected.to allow_value(false).for(:allow_runner_registration_token) } + it { is_expected.not_to allow_value(nil).for(:allow_runner_registration_token) } + context 'when deactivate_dormant_users is enabled' do before do stub_application_setting(deactivate_dormant_users: true) @@ -717,35 +725,7 @@ RSpec.describe ApplicationSetting do end context 'housekeeping settings' do - it { is_expected.not_to allow_value(0).for(:housekeeping_incremental_repack_period) } - - it 'wants the full repack period to be at least the incremental repack period' do - subject.housekeeping_incremental_repack_period = 2 - subject.housekeeping_full_repack_period = 1 - - expect(subject).not_to be_valid - end - - it 'wants the gc period to be at least the full repack period' do - subject.housekeeping_full_repack_period = 100 - subject.housekeeping_gc_period = 90 - - expect(subject).not_to be_valid - end - - it 'allows the same period for incremental repack and full repack, effectively skipping incremental repack' do - subject.housekeeping_incremental_repack_period = 2 - subject.housekeeping_full_repack_period = 2 - - expect(subject).to be_valid - end - - it 'allows the same period for full repack and gc, effectively skipping full repack' do - subject.housekeeping_full_repack_period = 100 - subject.housekeeping_gc_period = 100 - - expect(subject).to be_valid - end + it { is_expected.not_to allow_value(0).for(:housekeeping_optimize_repository_period) } end context 'gitaly timeouts' do diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index f4f2b174a7b..b1c65c6b9ee 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -325,4 +325,24 @@ RSpec.describe BulkImports::Entity, type: :model do expect(project_entity.update_service).to eq(::Projects::UpdateService) end end + + describe '#full_path' do + it 'returns group full path for project entity' do + group_entity = build(:bulk_import_entity, :group_entity, group: build(:group)) + + expect(group_entity.full_path).to eq(group_entity.group.full_path) + end + + it 'returns project full path for project entity' do + project_entity = build(:bulk_import_entity, :project_entity, project: build(:project)) + + expect(project_entity.full_path).to eq(project_entity.project.full_path) + end + + it 'returns nil when not associated with group or project' do + entity = build(:bulk_import_entity, group: nil, project: nil) + + expect(entity.full_path).to eq(nil) + end + end end diff --git a/spec/models/chat_name_spec.rb b/spec/models/chat_name_spec.rb index 02c38479d1a..0838c232872 100644 --- a/spec/models/chat_name_spec.rb +++ b/spec/models/chat_name_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ChatName do +RSpec.describe ChatName, feature_category: :integrations do let_it_be(:chat_name) { create(:chat_name) } subject { chat_name } @@ -11,17 +11,15 @@ RSpec.describe ChatName do it { is_expected.to belong_to(:user) } it { is_expected.to validate_presence_of(:user) } - it { is_expected.to validate_presence_of(:integration) } it { is_expected.to validate_presence_of(:team_id) } it { is_expected.to validate_presence_of(:chat_id) } - it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(:integration_id) } - it { is_expected.to validate_uniqueness_of(:chat_id).scoped_to(:integration_id, :team_id) } + it { is_expected.to validate_uniqueness_of(:chat_id).scoped_to(:team_id) } - it 'is removed when the project is deleted' do - expect { subject.reload.integration.project.delete }.to change { ChatName.count }.by(-1) + it 'is not removed when the project is deleted' do + expect { subject.reload.integration.project.delete }.not_to change { ChatName.count } - expect(ChatName.where(id: subject.id)).not_to exist + expect(ChatName.where(id: subject.id)).to exist end describe '#update_last_used_at', :clean_gitlab_redis_shared_state do diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 169b00b9c74..70e977e37ba 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -21,8 +21,8 @@ RSpec.describe Ci::Bridge, feature_category: :continuous_integration do { trigger: { project: 'my/project', branch: 'master' } } end - it 'has many sourced pipelines' do - expect(bridge).to have_many(:sourced_pipelines) + it 'has one sourced pipeline' do + expect(bridge).to have_one(:sourced_pipeline) end it_behaves_like 'has ID tokens', :ci_bridge @@ -34,24 +34,6 @@ RSpec.describe Ci::Bridge, feature_category: :continuous_integration do expect(bridge).to have_one(:downstream_pipeline) end - describe '#sourced_pipelines' do - subject { bridge.sourced_pipelines } - - it 'raises error' do - expect { subject }.to raise_error RuntimeError, 'Ci::Bridge does not have sourced_pipelines association' - end - - context 'when ci_bridge_remove_sourced_pipelines is disabled' do - before do - stub_feature_flags(ci_bridge_remove_sourced_pipelines: false) - end - - it 'returns the sourced_pipelines association' do - expect(bridge.sourced_pipelines).to eq([]) - end - end - end - describe '#retryable?' do let(:bridge) { create(:ci_bridge, :success) } @@ -393,25 +375,6 @@ RSpec.describe Ci::Bridge, feature_category: :continuous_integration do { key: 'VAR7', value: 'value7 $VAR1', raw: true } ) end - - context 'when the FF ci_raw_variables_in_yaml_config is disabled' do - before do - stub_feature_flags(ci_raw_variables_in_yaml_config: false) - end - - it 'ignores the raw attribute' do - expect(downstream_variables).to contain_exactly( - { key: 'BRIDGE', value: 'cross' }, - { key: 'VAR1', value: 'value1' }, - { key: 'VAR2', value: 'value2 value1' }, - { key: 'VAR3', value: 'value3 value1' }, - { key: 'VAR4', value: 'value4 value1' }, - { key: 'VAR5', value: 'value5 value1' }, - { key: 'VAR6', value: 'value6 value1' }, - { key: 'VAR7', value: 'value7 value1' } - ) - end - end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c978e33bf54..dd1fbd7d0d5 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1136,6 +1136,19 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration do it do is_expected.to all(a_hash_including(key: a_string_matching(/-protected$/))) end + + context 'and the cache has the `unprotect` option' do + let(:options) do + { cache: [ + { key: "key", paths: ["public"], policy: "pull-push", unprotect: true }, + { key: "key2", paths: ["public"], policy: "pull-push", unprotect: true } + ] } + end + + it do + is_expected.to all(a_hash_including(key: a_string_matching(/-non_protected$/))) + end + end end context 'when pipeline is not on a protected ref' do @@ -3533,6 +3546,52 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration do end end + context 'for the apple_app_store integration' do + let_it_be(:apple_app_store_integration) { create(:apple_app_store_integration) } + + let(:apple_app_store_variables) do + [ + { key: 'APP_STORE_CONNECT_API_KEY_ISSUER_ID', value: apple_app_store_integration.app_store_issuer_id, masked: true, public: false }, + { key: 'APP_STORE_CONNECT_API_KEY_KEY', value: Base64.encode64(apple_app_store_integration.app_store_private_key), masked: true, public: false }, + { key: 'APP_STORE_CONNECT_API_KEY_KEY_ID', value: apple_app_store_integration.app_store_key_id, masked: true, public: false } + ] + end + + context 'when the apple_app_store exists' do + context 'when a build is protected' do + before do + allow(build.pipeline).to receive(:protected_ref?).and_return(true) + build.project.update!(apple_app_store_integration: apple_app_store_integration) + end + + it 'includes apple_app_store variables' do + is_expected.to include(*apple_app_store_variables) + end + end + + context 'when a build is not protected' do + before do + allow(build.pipeline).to receive(:protected_ref?).and_return(false) + build.project.update!(apple_app_store_integration: apple_app_store_integration) + end + + it 'does not include the apple_app_store variables' do + expect(subject.find { |v| v[:key] == 'APP_STORE_CONNECT_API_KEY_ISSUER_ID' }).to be_nil + expect(subject.find { |v| v[:key] == 'APP_STORE_CONNECT_API_KEY_KEY' }).to be_nil + expect(subject.find { |v| v[:key] == 'APP_STORE_CONNECT_API_KEY_KEY_ID' }).to be_nil + end + end + end + + context 'when the apple_app_store integration does not exist' do + it 'does not include apple_app_store variables' do + expect(subject.find { |v| v[:key] == 'APP_STORE_CONNECT_API_KEY_ISSUER_ID' }).to be_nil + expect(subject.find { |v| v[:key] == 'APP_STORE_CONNECT_API_KEY_KEY' }).to be_nil + expect(subject.find { |v| v[:key] == 'APP_STORE_CONNECT_API_KEY_KEY_ID' }).to be_nil + end + end + end + context 'when build has dependency which has dotenv variable' do let!(:prepare) { create(:ci_build, pipeline: pipeline, stage_idx: 0) } let!(:build) { create(:ci_build, pipeline: pipeline, stage_idx: 1, options: { dependencies: [prepare.name] }) } @@ -5664,17 +5723,22 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration do expect(prefix).to eq(ci_testing_partition_id) end + end - context 'when ci_build_partition_id_token_prefix is disabled' do - before do - stub_feature_flags(ci_build_partition_id_token_prefix: false) - end + describe '#remove_token!' do + it 'removes the token' do + expect(build.token).to be_present - it 'does not include partition_id as a token prefix' do - prefix = ci_build.token.split('_').first.to_i(16) + build.remove_token! - expect(prefix).not_to eq(ci_testing_partition_id) - end + expect(build.token).to be_nil + expect(build.changes).to be_empty + end + + it 'does not remove the token when FF is disabled' do + stub_feature_flags(remove_job_token_on_completion: false) + + expect { build.remove_token! }.not_to change(build, :token) end end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 18aaab1d1f3..a1fd51f60ea 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -134,6 +134,38 @@ RSpec.describe Ci::JobArtifact do end end + describe 'artifacts_public?' do + subject { artifact.public_access? } + + context 'when job artifact created by default' do + let!(:artifact) { create(:ci_job_artifact) } + + it { is_expected.to be_truthy } + end + + context 'when job artifact created as public' do + let!(:artifact) { create(:ci_job_artifact, :public) } + + it { is_expected.to be_truthy } + end + + context 'when job artifact created as private' do + let!(:artifact) { build(:ci_job_artifact, :private) } + + it { is_expected.to be_falsey } + + context 'and the non_public_artifacts feature flag is disabled' do + let!(:artifact) { build(:ci_job_artifact, :private) } + + before do + stub_feature_flags(non_public_artifacts: false) + end + + it { is_expected.to be_truthy } + end + end + end + describe '.file_types_for_report' do it 'returns the report file types for the report type' do expect(described_class.file_types_for_report(:test)).to match_array(%w[junit]) @@ -690,8 +722,8 @@ RSpec.describe Ci::JobArtifact do end it 'updates project statistics' do - expect(ProjectStatistics).to receive(:increment_statistic).once - .with(project, :build_artifacts_size, -job_artifact.file.size) + expect(ProjectStatistics).to receive(:bulk_increment_statistic).once + .with(project, :build_artifacts_size, [have_attributes(amount: -job_artifact.file.size)]) pipeline.destroy! end diff --git a/spec/models/ci/namespace_mirror_spec.rb b/spec/models/ci/namespace_mirror_spec.rb index 29447cbc89d..63e6e9e6b26 100644 --- a/spec/models/ci/namespace_mirror_spec.rb +++ b/spec/models/ci/namespace_mirror_spec.rb @@ -96,7 +96,7 @@ RSpec.describe Ci::NamespaceMirror do describe '.by_namespace_id' do subject(:result) { described_class.by_namespace_id(group2.id) } - it 'returns namesapce mirrors of namespace id' do + it 'returns namespace mirrors of namespace id' do expect(result).to contain_exactly(group2.ci_namespace_mirror) end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b72693d9994..5888f9d109c 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do +RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: :continuous_integration do include ProjectForksHelper include StubRequests include Ci::SourcePipelineHelpers @@ -1322,6 +1322,21 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + context 'when tag is not found' do + let(:pipeline) do + create(:ci_pipeline, project: project, ref: 'not_found_tag', tag: true) + end + + it 'does not expose tag variables' do + expect(subject.to_hash.keys) + .not_to include( + 'CI_COMMIT_TAG', + 'CI_COMMIT_TAG_MESSAGE', + 'CI_BUILD_TAG' + ) + end + end + context 'without a commit' do let(:pipeline) { build(:ci_empty_pipeline, :created, sha: nil) } diff --git a/spec/models/ci/runner_machine_spec.rb b/spec/models/ci/runner_machine_spec.rb new file mode 100644 index 00000000000..e39f987110f --- /dev/null +++ b/spec/models/ci/runner_machine_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::RunnerMachine, feature_category: :runner_fleet, type: :model do + it_behaves_like 'having unique enum values' + + it { is_expected.to belong_to(:runner) } + + describe 'validation' do + it { is_expected.to validate_presence_of(:runner) } + it { is_expected.to validate_presence_of(:machine_xid) } + it { is_expected.to validate_length_of(:machine_xid).is_at_most(64) } + it { is_expected.to validate_length_of(:version).is_at_most(2048) } + it { is_expected.to validate_length_of(:revision).is_at_most(255) } + it { is_expected.to validate_length_of(:platform).is_at_most(255) } + it { is_expected.to validate_length_of(:architecture).is_at_most(255) } + it { is_expected.to validate_length_of(:ip_address).is_at_most(1024) } + + context 'when runner has config' do + it 'is valid' do + runner_machine = build(:ci_runner_machine, config: { gpus: "all" }) + + expect(runner_machine).to be_valid + end + end + + context 'when runner has an invalid config' do + it 'is invalid' do + runner_machine = build(:ci_runner_machine, config: { test: 1 }) + + expect(runner_machine).not_to be_valid + end + end + end + + describe '.stale', :freeze_time do + subject { described_class.stale.ids } + + let!(:runner_machine1) { create(:ci_runner_machine, created_at: 8.days.ago, contacted_at: 7.days.ago) } + let!(:runner_machine2) { create(:ci_runner_machine, created_at: 7.days.ago, contacted_at: nil) } + let!(:runner_machine3) { create(:ci_runner_machine, created_at: 5.days.ago, contacted_at: nil) } + let!(:runner_machine4) do + create(:ci_runner_machine, created_at: (7.days - 1.second).ago, contacted_at: (7.days - 1.second).ago) + end + + it 'returns stale runner machines' do + is_expected.to match_array([runner_machine1.id, runner_machine2.id]) + end + end +end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 803b766c822..b7c7b67b98f 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -11,6 +11,13 @@ RSpec.describe Ci::Runner, feature_category: :runner do let(:factory_name) { :ci_runner } end + context 'loose foreign key on ci_runners.creator_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:user) } + let!(:model) { create(:ci_runner, creator: parent) } + end + end + describe 'groups association' do # Due to other associations such as projects this whole spec is allowed to # generate cross-database queries. So we have this temporary spec to @@ -530,7 +537,7 @@ RSpec.describe Ci::Runner, feature_category: :runner do end end - describe '.stale' do + describe '.stale', :freeze_time do subject { described_class.stale } let!(:runner1) { create(:ci_runner, :instance, created_at: 4.months.ago, contacted_at: 3.months.ago + 10.seconds) } @@ -1090,6 +1097,23 @@ RSpec.describe Ci::Runner, feature_category: :runner do expect(runner.runner_version).to be_nil end + + context 'with only ip_address specified', :freeze_time do + subject(:heartbeat) do + runner.heartbeat(ip_address: '1.1.1.1') + end + + it 'updates only ip_address' do + attrs = Gitlab::Json.dump(ip_address: '1.1.1.1', contacted_at: Time.current) + + Gitlab::Redis::Cache.with do |redis| + redis_key = runner.send(:cache_attribute_key) + expect(redis).to receive(:set).with(redis_key, attrs, any_args) + end + + heartbeat + end + end end context 'when database was not updated recently' do diff --git a/spec/models/ci/runner_version_spec.rb b/spec/models/ci/runner_version_spec.rb index 552b271fe85..dfaa2201859 100644 --- a/spec/models/ci/runner_version_spec.rb +++ b/spec/models/ci/runner_version_spec.rb @@ -35,12 +35,6 @@ RSpec.describe Ci::RunnerVersion, feature_category: :runner_fleet do end describe 'validation' do - context 'when runner version is too long' do - let(:runner_version) { build(:ci_runner_version, version: 'a' * 2049) } - - it 'is not valid' do - expect(runner_version).to be_invalid - end - end + it { is_expected.to validate_length_of(:version).is_at_most(2048) } end end diff --git a/spec/models/clusters/providers/aws_spec.rb b/spec/models/clusters/providers/aws_spec.rb index 2afed663edf..cb2960e1557 100644 --- a/spec/models/clusters/providers/aws_spec.rb +++ b/spec/models/clusters/providers/aws_spec.rb @@ -75,39 +75,6 @@ RSpec.describe Clusters::Providers::Aws do end end - describe '#api_client' do - let(:provider) { create(:cluster_provider_aws) } - let(:credentials) { double } - let(:client) { double } - - subject { provider.api_client } - - before do - allow(provider).to receive(:credentials).and_return(credentials) - - expect(Aws::CloudFormation::Client).to receive(:new) - .with(credentials: credentials, region: provider.region) - .and_return(client) - end - - it { is_expected.to eq client } - end - - describe '#credentials' do - let(:provider) { create(:cluster_provider_aws) } - let(:credentials) { double } - - subject { provider.credentials } - - before do - expect(Aws::Credentials).to receive(:new) - .with(provider.access_key_id, provider.secret_access_key, provider.session_token) - .and_return(credentials) - end - - it { is_expected.to eq credentials } - end - describe '#created_by_user' do let(:provider) { create(:cluster_provider_aws) } diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb index a1f00069937..afd5699091a 100644 --- a/spec/models/clusters/providers/gcp_spec.rb +++ b/spec/models/clusters/providers/gcp_spec.rb @@ -111,31 +111,6 @@ RSpec.describe Clusters::Providers::Gcp do end end - describe '#api_client' do - subject { gcp.api_client } - - context 'when status is creating' do - let(:gcp) { build(:cluster_provider_gcp, :creating) } - - it 'returns Cloud Platform API clinet' do - expect(subject).to be_an_instance_of(GoogleApi::CloudPlatform::Client) - expect(subject.access_token).to eq(gcp.access_token) - end - end - - context 'when status is created' do - let(:gcp) { build(:cluster_provider_gcp, :created) } - - it { is_expected.to be_nil } - end - - context 'when status is errored' do - let(:gcp) { build(:cluster_provider_gcp, :errored) } - - it { is_expected.to be_nil } - end - end - describe '#nullify_credentials' do let(:provider) { create(:cluster_provider_gcp, :creating) } diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb index 93c696cae54..6dd34c3e21f 100644 --- a/spec/models/commit_collection_spec.rb +++ b/spec/models/commit_collection_spec.rb @@ -15,26 +15,34 @@ RSpec.describe CommitCollection do end describe '.committers' do + subject(:collection) { described_class.new(project, [commit]) } + it 'returns a relation of users when users are found' do user = create(:user, email: commit.committer_email.upcase) - collection = described_class.new(project, [commit]) expect(collection.committers).to contain_exactly(user) end it 'returns empty array when committers cannot be found' do - collection = described_class.new(project, [commit]) - expect(collection.committers).to be_empty end it 'excludes authors of merge commits' do commit = project.commit("60ecb67744cb56576c30214ff52294f8ce2def98") create(:user, email: commit.committer_email.upcase) - collection = described_class.new(project, [commit]) expect(collection.committers).to be_empty end + + context 'when committer email is nil' do + before do + allow(commit).to receive(:committer_email).and_return(nil) + end + + it 'returns empty array when committers cannot be found' do + expect(collection.committers).to be_empty + end + end end describe '#without_merge_commits' do diff --git a/spec/models/commit_signatures/ssh_signature_spec.rb b/spec/models/commit_signatures/ssh_signature_spec.rb index 629d9c5ec53..235fd099c93 100644 --- a/spec/models/commit_signatures/ssh_signature_spec.rb +++ b/spec/models/commit_signatures/ssh_signature_spec.rb @@ -2,24 +2,30 @@ require 'spec_helper' -RSpec.describe CommitSignatures::SshSignature do +RSpec.describe CommitSignatures::SshSignature, feature_category: :source_code_management do # This commit is seeded from https://gitlab.com/gitlab-org/gitlab-test # For instructions on how to add more seed data, see the project README let_it_be(:commit_sha) { '7b5160f9bb23a3d58a0accdbe89da13b96b1ece9' } let_it_be(:project) { create(:project, :repository, path: 'sample-project') } let_it_be(:commit) { create(:commit, project: project, sha: commit_sha) } let_it_be(:ssh_key) { create(:ed25519_key_256) } + let_it_be(:user) { ssh_key.user } + let_it_be(:key_fingerprint) { ssh_key.fingerprint_sha256 } + + let(:signature) do + create(:ssh_signature, commit_sha: commit_sha, key: ssh_key, key_fingerprint_sha256: key_fingerprint, user: user) + end let(:attributes) do { commit_sha: commit_sha, project: project, - key: ssh_key + key: ssh_key, + key_fingerprint_sha256: key_fingerprint, + user: user } end - let(:signature) { create(:ssh_signature, commit_sha: commit_sha, key: ssh_key) } - it_behaves_like 'having unique enum values' it_behaves_like 'commit signature' it_behaves_like 'signature with type checking', :ssh @@ -39,9 +45,31 @@ RSpec.describe CommitSignatures::SshSignature do end end + describe '#key_fingerprint_sha256' do + it 'returns the fingerprint_sha256 associated with the SSH key' do + expect(signature.key_fingerprint_sha256).to eq(key_fingerprint) + end + + context 'when the SSH key is no longer associated with the signature' do + it 'returns the fingerprint_sha256 stored in signature' do + signature.update!(key_id: nil) + + expect(signature.key_fingerprint_sha256).to eq(key_fingerprint) + end + end + end + describe '#signed_by_user' do it 'returns the user associated with the SSH key' do expect(signature.signed_by_user).to eq(ssh_key.user) end + + context 'when the SSH key is no longer associated with the signature' do + it 'returns the user stored in signature' do + signature.update!(key_id: nil) + + expect(signature.signed_by_user).to eq(user) + end + end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 4b5aabe745b..36d0e37454d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -847,20 +847,6 @@ eos expect(unsigned_commit.has_signature?).to be_falsey expect(commit.has_signature?).to be_falsey end - - context 'when feature flag "ssh_commit_signatures" is disabled' do - before do - stub_feature_flags(ssh_commit_signatures: false) - end - - it 'reports no signature' do - expect(ssh_signed_commit).not_to have_signature - end - - it 'does not return signature data' do - expect(ssh_signed_commit.signature).to be_nil - end - end end describe '#has_been_reverted?' do diff --git a/spec/models/concerns/counter_attribute_spec.rb b/spec/models/concerns/counter_attribute_spec.rb index 1dd9b78d642..c8224c64ba2 100644 --- a/spec/models/concerns/counter_attribute_spec.rb +++ b/spec/models/concerns/counter_attribute_spec.rb @@ -37,6 +37,50 @@ RSpec.describe CounterAttribute, :counter_attribute, :clean_gitlab_redis_shared_ end end + describe '#initiate_refresh!' do + context 'when counter attribute is enabled' do + let(:attribute) { :build_artifacts_size } + + it 'initiates refresh on the BufferedCounter' do + expect_next_instance_of(Gitlab::Counters::BufferedCounter, model, attribute) do |counter| + expect(counter).to receive(:initiate_refresh!) + end + + model.initiate_refresh!(attribute) + end + end + + context 'when counter attribute is not enabled' do + let(:attribute) { :snippets_size } + + it 'raises error' do + expect { model.initiate_refresh!(attribute) }.to raise_error(ArgumentError) + end + end + end + + describe '#finalize_refresh' do + let(:attribute) { :build_artifacts_size } + + context 'when counter attribute is enabled' do + it 'initiates refresh on the BufferedCounter' do + expect_next_instance_of(Gitlab::Counters::BufferedCounter, model, attribute) do |counter| + expect(counter).to receive(:finalize_refresh) + end + + model.finalize_refresh(attribute) + end + end + + context 'when counter attribute is not enabled' do + let(:attribute) { :snippets_size } + + it 'raises error' do + expect { model.finalize_refresh(attribute) }.to raise_error(ArgumentError) + end + end + end + describe '#counter' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index 462b28f99be..bd128112113 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -5,7 +5,8 @@ require 'spec_helper' RSpec.describe User do specify 'types consistency checks', :aggregate_failures do expect(described_class::USER_TYPES.keys) - .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot visual_review_bot migration_bot automation_bot admin_bot]) + .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot visual_review_bot + migration_bot automation_bot admin_bot suggested_reviewers_bot]) expect(described_class::USER_TYPES).to include(*described_class::BOT_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::NON_INTERNAL_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::INTERNAL_USER_TYPES) diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index 82aca13c929..383ed68816e 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -63,6 +63,82 @@ RSpec.describe Noteable do end end + # rubocop:disable RSpec/MultipleMemoizedHelpers + describe '#commenters' do + shared_examples 'commenters' do + it 'does not automatically include the noteable author' do + expect(commenters).not_to include(noteable.author) + end + + context 'with no user' do + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter) + end + end + + context 'with non project member' do + let(:current_user) { create(:user) } + + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter) + end + + it 'does not include a commenter from another noteable' do + expect(commenters).not_to include(other_noteable_commenter) + end + end + end + + let_it_be(:commenter) { create(:user) } + let_it_be(:another_commenter) { create(:user) } + let_it_be(:internal_commenter) { create(:user) } + let_it_be(:other_noteable_commenter) { create(:user) } + + let(:current_user) {} + let(:commenters) { noteable.commenters(user: current_user) } + + let!(:comments) { create_list(:note, 2, author: commenter, noteable: noteable, project: noteable.project) } + let!(:more_comments) { create_list(:note, 2, author: another_commenter, noteable: noteable, project: noteable.project) } + + context 'when noteable is an issue' do + let(:noteable) { create(:issue) } + + let!(:internal_comments) { create_list(:note, 2, author: internal_commenter, noteable: noteable, project: noteable.project, internal: true) } + let!(:other_noteable_comments) { create_list(:note, 2, author: other_noteable_commenter, noteable: create(:issue, project: noteable.project), project: noteable.project) } + + it_behaves_like 'commenters' + + context 'with reporter' do + let(:current_user) { create(:user) } + + before do + noteable.project.add_reporter(current_user) + end + + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter, internal_commenter) + end + + context 'with noteable author' do + let(:current_user) { noteable.author } + + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter, internal_commenter) + end + end + end + end + + context 'when noteable is a merge request' do + let(:noteable) { create(:merge_request) } + + let!(:other_noteable_comments) { create_list(:note, 2, author: other_noteable_commenter, noteable: create(:merge_request, source_project: noteable.project, source_branch: 'feat123'), project: noteable.project) } + + it_behaves_like 'commenters' + end + end + # rubocop:enable RSpec/MultipleMemoizedHelpers + describe '#discussion_ids_relation' do it 'returns ordered discussion_ids' do discussion_ids = subject.discussion_ids_relation.pluck(:discussion_id) diff --git a/spec/models/concerns/safely_change_column_default_spec.rb b/spec/models/concerns/safely_change_column_default_spec.rb new file mode 100644 index 00000000000..36782170eaf --- /dev/null +++ b/spec/models/concerns/safely_change_column_default_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SafelyChangeColumnDefault, feature_category: :database do + include Gitlab::Database::DynamicModelHelpers + before do + ApplicationRecord.connection.execute(<<~SQL) + CREATE TABLE _test_gitlab_main_data( + id bigserial primary key not null, + value bigint default 1 + ); + SQL + end + + let!(:model) do + define_batchable_model('_test_gitlab_main_data', connection: ApplicationRecord.connection).tap do |model| + model.include(described_class) + model.columns_changing_default(:value) + model.columns # Force the schema cache to populate + end + end + + def alter_default(new_default) + ApplicationRecord.connection.execute(<<~SQL) + ALTER TABLE _test_gitlab_main_data ALTER COLUMN value SET DEFAULT #{new_default} + SQL + end + + def recorded_insert_queries(&block) + recorder = ActiveRecord::QueryRecorder.new + recorder.record(&block) + + recorder.log.select { |q| q.include?('INSERT INTO') } + end + + def query_includes_value_column?(query) + parsed = PgQuery.parse(query) + parsed.tree.stmts.first.stmt.insert_stmt.cols.any? { |node| node.res_target.name == 'value' } + end + + it 'forces the column to be written on a change' do + queries = recorded_insert_queries do + model.create!(value: 1) + end + + expect(queries.length).to eq(1) + + expect(query_includes_value_column?(queries.first)).to be_truthy + end + + it 'does not write the column without a change' do + queries = recorded_insert_queries do + model.create! + end + + expect(queries.length).to eq(1) + expect(query_includes_value_column?(queries.first)).to be_falsey + end + + it 'does not send the old column value if the default has changed' do + alter_default(2) + model.create! + + expect(model.pluck(:value)).to contain_exactly(2) + end + + it 'prevents writing new default in place of the old default' do + alter_default(2) + + model.create!(value: 1) + + expect(model.pluck(:value)).to contain_exactly(1) + end +end diff --git a/spec/models/concerns/sensitive_serializable_hash_spec.rb b/spec/models/concerns/sensitive_serializable_hash_spec.rb index 591a4383a03..0bfd2d6a7de 100644 --- a/spec/models/concerns/sensitive_serializable_hash_spec.rb +++ b/spec/models/concerns/sensitive_serializable_hash_spec.rb @@ -95,7 +95,7 @@ RSpec.describe SensitiveSerializableHash do expect(model.attributes).to include(attribute) # double-check the attribute does exist expect(model.serializable_hash).not_to include(attribute) - expect(model.to_json).not_to include(attribute) + expect(model.to_json).not_to match(/\b#{attribute}\b/) expect(model.as_json).not_to include(attribute) end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index fb3883820fd..f0fdc62e6c7 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -170,18 +170,7 @@ RSpec.describe Deployment do end end - it 'executes Deployments::DropOlderDeploymentsWorker asynchronously' do - stub_feature_flags(prevent_outdated_deployment_jobs: false) - - expect(Deployments::DropOlderDeploymentsWorker) - .to receive(:perform_async).once.with(deployment.id) - - deployment.run! - end - - it 'does not execute Deployments::DropOlderDeploymentsWorker when FF enabled' do - stub_feature_flags(prevent_outdated_deployment_jobs: true) - + it 'does not execute Deployments::DropOlderDeploymentsWorker' do expect(Deployments::DropOlderDeploymentsWorker) .not_to receive(:perform_async).with(deployment.id) @@ -413,6 +402,16 @@ RSpec.describe Deployment do it { is_expected.to be_falsey } end + + context 'when environment is undefined' do + let(:deployment) { build(:deployment, :success, project: project, environment: environment) } + + before do + deployment.environment = nil + end + + it { is_expected.to be_falsey } + end end describe '#success?' do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 2670127442e..0d53ebdefe9 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Environment, :use_clean_rails_memory_store_caching do +RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_category: :continuous_delivery do include ReactiveCachingHelpers using RSpec::Parameterized::TableSyntax include RepoHelpers @@ -2029,4 +2029,40 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do subject end end + + describe '#deployed_and_updated_before' do + subject do + described_class.deployed_and_updated_before(project_id, before) + end + + let(:project_id) { project.id } + let(:before) { 1.week.ago.to_date.to_s } + let(:environment) { create(:environment, project: project, updated_at: 2.weeks.ago) } + let!(:stale_deployment) { create(:deployment, environment: environment, updated_at: 2.weeks.ago) } + + it 'excludes environments with recent deployments' do + create(:deployment, environment: environment, updated_at: Date.current) + + is_expected.to match_array([]) + end + + it 'includes environments with no deployments' do + environment1 = create(:environment, project: project, updated_at: 2.weeks.ago) + + is_expected.to match_array([environment, environment1]) + end + + it 'excludes environments that have been recently updated with no deployments' do + create(:environment, project: project) + + is_expected.to match_array([environment]) + end + + it 'excludes environments that have been recently updated with stale deployments' do + environment1 = create(:environment, project: project) + create(:deployment, environment: environment1, updated_at: 2.weeks.ago) + + is_expected.to match_array([environment]) + end + end end diff --git a/spec/models/event_collection_spec.rb b/spec/models/event_collection_spec.rb index 40b7930f02b..13983dcfde3 100644 --- a/spec/models/event_collection_spec.rb +++ b/spec/models/event_collection_spec.rb @@ -89,6 +89,25 @@ RSpec.describe EventCollection do expect(events).to contain_exactly(closed_issue_event) end + context 'when there are multiple issue events' do + let!(:work_item_event) do + create( + :event, + :created, + project: project, + target: create(:work_item, :task, project: project), + target_type: 'WorkItem' + ) + end + + it 'includes work item events too' do + filter = EventFilter.new(EventFilter::ISSUE) + events = described_class.new(projects, filter: filter).to_a + + expect(events).to contain_exactly(closed_issue_event, work_item_event) + end + end + it 'allows filtering of events using an EventFilter, returning several items' do filter = EventFilter.new(EventFilter::MERGED) events = described_class.new(projects, filter: filter).to_a diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 667f3ddff72..f170eeb5841 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -102,7 +102,20 @@ RSpec.describe Event, feature_category: :users do end describe 'scopes' do - describe 'created_at' do + describe '.for_issue' do + let(:issue_event) { create(:event, :for_issue, project: project) } + let(:work_item_event) { create(:event, :for_work_item, project: project) } + + before do + create(:event, :for_design, project: project) + end + + it 'returns events for Issue and WorkItem target_type' do + expect(described_class.for_issue).to contain_exactly(issue_event, work_item_event) + end + end + + describe '.created_at' do it 'can find the right event' do time = 1.day.ago event = create(:event, created_at: time, project: project) diff --git a/spec/models/factories_spec.rb b/spec/models/factories_spec.rb index 4915c0bd870..d6e746986d6 100644 --- a/spec/models/factories_spec.rb +++ b/spec/models/factories_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' # `:saas` is used to test `gitlab_subscription` factory. # It's not available on FOSS but also this very factory is not. -RSpec.describe 'factories', :saas do +RSpec.describe 'factories', :saas, :with_license, feature_category: :tooling do include Database::DatabaseHelpers # Used in `skipped` and indicates whether to skip any traits including the @@ -188,7 +188,13 @@ RSpec.describe 'factories', :saas do before do factories_based_on_view.each do |factory| view = build(factory).class.table_name - swapout_view_for_table(view) + view_gitlab_schema = Gitlab::Database::GitlabSchema.table_schema(view) + Gitlab::Database.database_base_models.each_value.select do |base_model| + connection = base_model.connection + next unless Gitlab::Database.gitlab_schemas_for_connection(connection).include?(view_gitlab_schema) + + swapout_view_for_table(view, connection: connection) + end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index dfba0470d35..4605c086763 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Group do +RSpec.describe Group, feature_category: :subgroups do include ReloadHelpers include StubGitlabCalls @@ -11,9 +11,11 @@ RSpec.describe Group do describe 'associations' do it { is_expected.to have_many :projects } it { is_expected.to have_many(:group_members).dependent(:destroy) } + it { is_expected.to have_many(:namespace_members) } it { is_expected.to have_many(:users).through(:group_members) } it { is_expected.to have_many(:owners).through(:group_members) } it { is_expected.to have_many(:requesters).dependent(:destroy) } + it { is_expected.to have_many(:namespace_requesters) } it { is_expected.to have_many(:members_and_requesters) } it { is_expected.to have_many(:project_group_links).dependent(:destroy) } it { is_expected.to have_many(:shared_projects).through(:project_group_links) } @@ -45,7 +47,7 @@ RSpec.describe Group do it { is_expected.to have_one(:group_feature) } it { is_expected.to have_one(:harbor_integration) } - describe '#members & #requesters' do + describe '#namespace_members' do let(:requester) { create(:user) } let(:developer) { create(:user) } @@ -54,6 +56,98 @@ RSpec.describe Group do group.add_developer(developer) end + it 'includes the correct users' do + expect(group.namespace_members).to include Member.find_by(user: developer) + expect(group.namespace_members).not_to include Member.find_by(user: requester) + end + + it 'is equivelent to #group_members' do + expect(group.namespace_members).to eq group.group_members + end + + it_behaves_like 'query without source filters' do + subject { group.namespace_members } + end + end + + describe '#namespace_requesters' do + let(:requester) { create(:user) } + let(:developer) { create(:user) } + + before do + group.request_access(requester) + group.add_developer(developer) + end + + it 'includes the correct users' do + expect(group.namespace_requesters).to include Member.find_by(user: requester) + expect(group.namespace_requesters).not_to include Member.find_by(user: developer) + end + + it 'is equivalent to #requesters' do + expect(group.namespace_requesters).to eq group.requesters + end + + it_behaves_like 'query without source filters' do + subject { group.namespace_requesters } + end + end + + shared_examples 'polymorphic membership relationship' do + it do + expect(membership.attributes).to include( + 'source_type' => 'Namespace', + 'source_id' => group.id + ) + end + end + + shared_examples 'member_namespace membership relationship' do + it do + expect(membership.attributes).to include( + 'member_namespace_id' => group.id + ) + end + end + + describe '#namespace_members setters' do + let(:user) { create(:user) } + let(:membership) { group.namespace_members.create!(user: user, access_level: Gitlab::Access::DEVELOPER) } + + it { expect(membership).to be_instance_of(GroupMember) } + it { expect(membership.user).to eq user } + it { expect(membership.group).to eq group } + it { expect(membership.requested_at).to be_nil } + + it_behaves_like 'polymorphic membership relationship' + it_behaves_like 'member_namespace membership relationship' + end + + describe '#namespace_requesters setters' do + let(:requested_at) { Time.current } + let(:user) { create(:user) } + let(:membership) do + group.namespace_requesters.create!(user: user, requested_at: requested_at, access_level: Gitlab::Access::DEVELOPER) + end + + it { expect(membership).to be_instance_of(GroupMember) } + it { expect(membership.user).to eq user } + it { expect(membership.group).to eq group } + it { expect(membership.requested_at).to eq requested_at } + + it_behaves_like 'polymorphic membership relationship' + it_behaves_like 'member_namespace membership relationship' + end + + describe '#members & #requesters' do + let_it_be(:requester) { create(:user) } + let_it_be(:developer) { create(:user) } + + before do + group.request_access(requester) + group.add_developer(developer) + end + it_behaves_like 'members and requesters associations' do let(:namespace) { group } end @@ -2648,7 +2742,81 @@ RSpec.describe Group do end end - context 'disabled_with_override' do + context 'disabled_and_overridable' do + subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_AND_OVERRIDABLE) } + + context 'top level group' do + let_it_be(:group) { create(:group, :shared_runners_disabled) } + let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } + let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } + + it 'enables allow descendants to override only for itself' do + expect { subject_and_reload(group, sub_group, project) } + .to change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and not_change { group.shared_runners_enabled } + .and not_change { sub_group.allow_descendants_override_disabled_shared_runners } + .and not_change { sub_group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } + end + end + + context 'group that its ancestors have shared Runners disabled but allows to override' do + let_it_be(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } + let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } + let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } + + it 'enables allow descendants to override' do + expect { subject_and_reload(parent, group, project) } + .to not_change { parent.allow_descendants_override_disabled_shared_runners } + .and not_change { parent.shared_runners_enabled } + .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and not_change { group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } + end + end + + context 'when parent does not allow' do + let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false) } + let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } + + it 'raises exception' do + expect { subject } + .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it') + end + + it 'does not allow descendants to override' do + expect do + begin + subject + rescue StandardError + nil + end + + parent.reload + group.reload + end.to not_change { parent.allow_descendants_override_disabled_shared_runners } + .and not_change { parent.shared_runners_enabled } + .and not_change { group.allow_descendants_override_disabled_shared_runners } + .and not_change { group.shared_runners_enabled } + end + end + + context 'top level group that has shared Runners enabled' do + let_it_be(:group) { create(:group, shared_runners_enabled: true) } + let_it_be(:sub_group) { create(:group, shared_runners_enabled: true, parent: group) } + let_it_be(:project) { create(:project, shared_runners_enabled: true, group: sub_group) } + + it 'enables allow descendants to override & disables shared runners everywhere' do + expect { subject_and_reload(group, sub_group, project) } + .to change { group.shared_runners_enabled }.from(true).to(false) + .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and change { sub_group.shared_runners_enabled }.from(true).to(false) + .and change { project.shared_runners_enabled }.from(true).to(false) + end + end + end + + context 'disabled_with_override (deprecated)' do subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_WITH_OVERRIDE) } context 'top level group' do @@ -3486,4 +3654,26 @@ RSpec.describe Group do it { is_expected.to be_nil } end end + + describe '#usage_quotas_enabled?', feature_category: :subscription_cost_management, unless: Gitlab.ee? do + using RSpec::Parameterized::TableSyntax + + where(:feature_enabled, :root_group, :result) do + false | true | false + false | false | false + true | false | false + true | true | true + end + + with_them do + before do + stub_feature_flags(usage_quotas_for_all_editions: feature_enabled) + allow(group).to receive(:root?).and_return(root_group) + end + + it 'returns the expected result' do + expect(group.usage_quotas_enabled?).to eq result + end + end + end end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 78b30221a24..9b3250e3c08 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -29,6 +29,7 @@ RSpec.describe Integration do it { is_expected.to be_tag_push_events } it { is_expected.to be_wiki_page_events } it { is_expected.not_to be_active } + it { is_expected.not_to be_incident_events } it { expect(subject.category).to eq(:common) } end @@ -153,6 +154,7 @@ RSpec.describe Integration do include_examples 'hook scope', 'confidential_note' include_examples 'hook scope', 'alert' include_examples 'hook scope', 'archive_trace' + include_examples 'hook scope', 'incident' end describe '#operating?' do diff --git a/spec/models/integrations/apple_app_store_spec.rb b/spec/models/integrations/apple_app_store_spec.rb new file mode 100644 index 00000000000..1a57f556895 --- /dev/null +++ b/spec/models/integrations/apple_app_store_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::AppleAppStore, feature_category: :mobile_devops do + describe 'Validations' do + context 'when active' do + before do + subject.active = true + end + + it { is_expected.to validate_presence_of :app_store_issuer_id } + it { is_expected.to validate_presence_of :app_store_key_id } + it { is_expected.to validate_presence_of :app_store_private_key } + it { is_expected.to allow_value('aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee').for(:app_store_issuer_id) } + it { is_expected.not_to allow_value('abcde').for(:app_store_issuer_id) } + it { is_expected.to allow_value(File.read('spec/fixtures/ssl_key.pem')).for(:app_store_private_key) } + it { is_expected.not_to allow_value("foo").for(:app_store_private_key) } + it { is_expected.to allow_value('ABCD1EF12G').for(:app_store_key_id) } + it { is_expected.not_to allow_value('ABC').for(:app_store_key_id) } + it { is_expected.not_to allow_value('abc1').for(:app_store_key_id) } + it { is_expected.not_to allow_value('-A0-').for(:app_store_key_id) } + end + end + + context 'when integration is enabled' do + let(:apple_app_store_integration) { build(:apple_app_store_integration) } + + describe '#fields' do + it 'returns custom fields' do + expect(apple_app_store_integration.fields.pluck(:name)).to eq(%w[app_store_issuer_id app_store_key_id + app_store_private_key]) + end + end + + describe '#test' do + it 'returns true for a successful request' do + allow(AppStoreConnect::Client).to receive_message_chain(:new, :apps).and_return({}) + expect(apple_app_store_integration.test[:success]).to be true + end + + it 'returns false for an invalid request' do + allow(AppStoreConnect::Client).to receive_message_chain(:new, +:apps).and_return({ errors: [title: "error title"] }) + expect(apple_app_store_integration.test[:success]).to be false + end + end + + describe '#help' do + it 'renders prompt information' do + expect(apple_app_store_integration.help).not_to be_empty + end + end + + describe '.to_param' do + it 'returns the name of the integration' do + expect(described_class.to_param).to eq('apple_app_store') + end + end + + describe '#ci_variables' do + let(:apple_app_store_integration) { build_stubbed(:apple_app_store_integration) } + + it 'returns vars when the integration is activated' do + ci_vars = [ + { + key: 'APP_STORE_CONNECT_API_KEY_ISSUER_ID', + value: apple_app_store_integration.app_store_issuer_id, + masked: true, + public: false + }, + { + key: 'APP_STORE_CONNECT_API_KEY_KEY', + value: Base64.encode64(apple_app_store_integration.app_store_private_key), + masked: true, + public: false + }, + { + key: 'APP_STORE_CONNECT_API_KEY_KEY_ID', + value: apple_app_store_integration.app_store_key_id, + masked: true, + public: false + } + ] + + expect(apple_app_store_integration.ci_variables).to match_array(ci_vars) + end + + it 'returns an empty array when the integration is disabled' do + apple_app_store_integration = build_stubbed(:apple_app_store_integration, active: false) + expect(apple_app_store_integration.ci_variables).to match_array([]) + end + end + end + + context 'when integration is disabled' do + let(:apple_app_store_integration) { build_stubbed(:apple_app_store_integration, active: false) } + + describe '#ci_variables' do + it 'returns an empty array' do + expect(apple_app_store_integration.ci_variables).to match_array([]) + end + end + end +end diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index 67fc09fd8b5..1527ffd7278 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::BaseChatNotification do +RSpec.describe Integrations::BaseChatNotification, feature_category: :integrations do describe 'default values' do it { expect(subject.category).to eq(:chat) } end @@ -134,6 +134,12 @@ RSpec.describe Integrations::BaseChatNotification do it_behaves_like 'notifies the chat integration' end + + context 'Incident events' do + let(:data) { issue.to_hook_data(user).merge!({ object_kind: 'incident' }) } + + it_behaves_like 'notifies the chat integration' + end end context 'when labels_to_be_notified_behavior is not defined' do diff --git a/spec/models/integrations/chat_message/issue_message_spec.rb b/spec/models/integrations/chat_message/issue_message_spec.rb index ff9f30efdca..cd40e4c361e 100644 --- a/spec/models/integrations/chat_message/issue_message_spec.rb +++ b/spec/models/integrations/chat_message/issue_message_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Integrations::ChatMessage::IssueMessage do let(:args) do { + object_kind: 'issue', user: { name: 'Test User', username: 'test.user', diff --git a/spec/models/integrations/chat_message/pipeline_message_spec.rb b/spec/models/integrations/chat_message/pipeline_message_spec.rb index 413cb097327..4d371ca0899 100644 --- a/spec/models/integrations/chat_message/pipeline_message_spec.rb +++ b/spec/models/integrations/chat_message/pipeline_message_spec.rb @@ -80,18 +80,6 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do expect(name_field[:value]).to eq('Build pipeline') end - context 'when pipeline_name feature flag is disabled' do - before do - stub_feature_flags(pipeline_name: false) - end - - it 'does not return pipeline name' do - name_field = subject.attachments.first[:fields].find { |a| a[:title] == 'Pipeline name' } - - expect(name_field).to be nil - end - end - context "when the pipeline failed" do before do args[:object_attributes][:status] = 'failed' diff --git a/spec/models/integrations/every_integration_spec.rb b/spec/models/integrations/every_integration_spec.rb index 33e89b3dabc..8666ef512fc 100644 --- a/spec/models/integrations/every_integration_spec.rb +++ b/spec/models/integrations/every_integration_spec.rb @@ -11,9 +11,9 @@ RSpec.describe 'Every integration' do let(:integration) { integration_class.new } context 'secret fields', :aggregate_failures do - it "uses type: 'password' for all secret fields" do + it "uses type: 'password' for all secret fields, except when bypassed" do integration.fields.each do |field| - next unless Integrations::Field::SECRET_NAME.match?(field[:name]) + next unless Integrations::Field::SECRET_NAME.match?(field[:name]) && field[:is_secret] expect(field[:type]).to eq('password'), "Field '#{field[:name]}' should use type 'password'" diff --git a/spec/models/integrations/field_spec.rb b/spec/models/integrations/field_spec.rb index 642fb1fbf7f..c30f9ef0d7b 100644 --- a/spec/models/integrations/field_spec.rb +++ b/spec/models/integrations/field_spec.rb @@ -83,6 +83,8 @@ RSpec.describe ::Integrations::Field do be false when :type eq 'text' + when :is_secret + eq true else be_nil end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 7c147067714..fdb397932e0 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Issue, feature_category: :project_management do using RSpec::Parameterized::TableSyntax let_it_be(:user) { create(:user) } - let_it_be(:reusable_project) { create(:project) } + let_it_be_with_reload(:reusable_project) { create(:project) } describe "Associations" do it { is_expected.to belong_to(:milestone) } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 0ebccf1cb65..4b28f619d94 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -303,7 +303,7 @@ RSpec.describe Member do @requested_member = project.requesters.find_by(user_id: requested_user.id) accepted_request_user = create(:user).tap { |u| project.request_access(u) } - @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request } + @accepted_request_member = project.requesters.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request(@owner_user) } @member_with_minimal_access = create(:group_member, :minimal_access, source: group) end @@ -777,18 +777,25 @@ RSpec.describe Member do describe '#accept_request' do let(:member) { create(:project_member, requested_at: Time.current.utc) } - it { expect(member.accept_request).to be_truthy } + it { expect(member.accept_request(@owner_user)).to be_truthy } + it { expect(member.accept_request(nil)).to be_truthy } it 'clears requested_at' do - member.accept_request + member.accept_request(@owner_user) expect(member.requested_at).to be_nil end + it 'saves the approving user' do + member.accept_request(@owner_user) + + expect(member.created_by).to eq(@owner_user) + end + it 'calls #after_accept_request' do expect(member).to receive(:after_accept_request) - member.accept_request + member.accept_request(@owner_user) end end @@ -799,33 +806,27 @@ RSpec.describe Member do end describe '#request?' do - context 'when request for project' do - subject { create(:project_member, requested_at: Time.current.utc) } + shared_examples 'calls notification service and todo service' do + subject { create(source_type, requested_at: Time.current.utc) } - it 'calls notification service but not todo service' do + specify do expect_next_instance_of(NotificationService) do |instance| expect(instance).to receive(:new_access_request) end - expect(TodoService).not_to receive(:new) + expect_next_instance_of(TodoService) do |instance| + expect(instance).to receive(:create_member_access_request_todos) + end is_expected.to be_request end end - context 'when request for group' do - subject { create(:group_member, requested_at: Time.current.utc) } - - it 'calls notification and todo service' do - expect_next_instance_of(NotificationService) do |instance| - expect(instance).to receive(:new_access_request) - end - - expect_next_instance_of(TodoService) do |instance| - expect(instance).to receive(:create_member_access_request) + context 'when requests for project and group are raised' do + %i[project_member group_member].each do |source_type| + it_behaves_like 'calls notification service and todo service' do + let_it_be(:source_type) { source_type } end - - is_expected.to be_request end end end diff --git a/spec/models/members/member_role_spec.rb b/spec/models/members/member_role_spec.rb index f9d6757bb90..b118a3c0968 100644 --- a/spec/models/members/member_role_spec.rb +++ b/spec/models/members/member_role_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MemberRole do +RSpec.describe MemberRole, feature_category: :authentication_and_authorization do describe 'associations' do it { is_expected.to belong_to(:namespace) } it { is_expected.to have_many(:members) } @@ -14,6 +14,27 @@ RSpec.describe MemberRole do it { is_expected.to validate_presence_of(:namespace) } it { is_expected.to validate_presence_of(:base_access_level) } + context 'for attributes_locked_after_member_associated' do + context 'when assigned to member' do + it 'cannot be changed' do + member_role.save! + member_role.members << create(:project_member) + + expect(member_role).not_to be_valid + expect(member_role.errors.messages[:base]).to include( + s_("MemberRole|cannot be changed because it is already assigned to a user. "\ + "Please create a new Member Role instead") + ) + end + end + + context 'when not assigned to member' do + it 'can be changed' do + expect(member_role).to be_valid + end + end + end + context 'when for namespace' do let_it_be(:root_group) { create(:group) } diff --git a/spec/models/merge_request/approval_removal_settings_spec.rb b/spec/models/merge_request/approval_removal_settings_spec.rb index 5f879207a72..7e375c7ff39 100644 --- a/spec/models/merge_request/approval_removal_settings_spec.rb +++ b/spec/models/merge_request/approval_removal_settings_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MergeRequest::ApprovalRemovalSettings do +RSpec.describe MergeRequest::ApprovalRemovalSettings, :with_license do describe 'validations' do let(:reset_approvals_on_push) {} let(:selective_code_owner_removals) {} diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index a17b90930f0..1ecc4356672 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MergeRequestDiff do +RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do using RSpec::Parameterized::TableSyntax include RepoHelpers @@ -412,7 +412,19 @@ RSpec.describe MergeRequestDiff do describe '#diffs_in_batch' do let(:diff_options) { {} } + shared_examples_for 'measuring diffs metrics' do + specify do + allow(Gitlab::Metrics).to receive(:measure).and_call_original + expect(Gitlab::Metrics).to receive(:measure).with(:diffs_reorder).and_call_original + expect(Gitlab::Metrics).to receive(:measure).with(:diffs_collection).and_call_original + + diff_with_commits.diffs_in_batch(0, 10, diff_options: diff_options) + end + end + shared_examples_for 'fetching full diffs' do + it_behaves_like 'measuring diffs metrics' + it 'returns diffs from repository comparison' do expect_next_instance_of(Compare) do |comparison| expect(comparison).to receive(:diffs) @@ -435,6 +447,13 @@ RSpec.describe MergeRequestDiff do expect(diffs.pagination_data).to eq(total_pages: nil) end + + it 'measures diffs_comparison' do + allow(Gitlab::Metrics).to receive(:measure).and_call_original + expect(Gitlab::Metrics).to receive(:measure).with(:diffs_comparison).and_call_original + + diff_with_commits.diffs_in_batch(1, 10, diff_options: diff_options) + end end context 'when no persisted files available' do @@ -454,6 +473,8 @@ RSpec.describe MergeRequestDiff do end context 'when persisted files available' do + it_behaves_like 'measuring diffs metrics' + it 'returns paginated diffs' do diffs = diff_with_commits.diffs_in_batch(0, 10, diff_options: diff_options) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 05586cbfc64..a059d5cae9b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MergeRequest, factory_default: :keep do +RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_review_workflow do include RepoHelpers include ProjectForksHelper include ReactiveCachingHelpers @@ -165,6 +165,25 @@ RSpec.describe MergeRequest, factory_default: :keep do expect(described_class.drafts).to eq([merge_request4]) end end + + describe '.without_hidden', feature_category: :insider_threat do + let_it_be(:banned_user) { create(:user, :banned) } + let_it_be(:hidden_merge_request) { create(:merge_request, :unique_branches, author: banned_user) } + + it 'only returns public issuables' do + expect(described_class.without_hidden).not_to include(hidden_merge_request) + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(hide_merge_requests_from_banned_users: false) + end + + it 'returns public and hidden issuables' do + expect(described_class.without_hidden).to include(hidden_merge_request) + end + end + end end describe '#squash?' do @@ -4546,6 +4565,34 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe 'transition to merged' do + context 'when reset_merge_error_on_transition feature flag is on' do + before do + stub_feature_flags(reset_merge_error_on_transition: true) + end + + it 'resets the merge error' do + subject.update!(merge_error: 'temp') + + expect { subject.mark_as_merged }.to change { subject.merge_error.present? } + .from(true) + .to(false) + end + end + + context 'when reset_merge_error_on_transition feature flag is off' do + before do + stub_feature_flags(reset_merge_error_on_transition: false) + end + + it 'does not reset the merge error' do + subject.update!(merge_error: 'temp') + + expect { subject.mark_as_merged }.not_to change { subject.merge_error.present? } + end + end + end + describe 'transition to cannot_be_merged' do let(:notification_service) { double(:notification_service) } let(:todo_service) { double(:todo_service) } @@ -5456,4 +5503,27 @@ RSpec.describe MergeRequest, factory_default: :keep do it { is_expected.to be_empty } end + + describe '#hidden?', feature_category: :insider_threat do + let_it_be(:author) { create(:user) } + let(:merge_request) { build_stubbed(:merge_request, author: author) } + + subject { merge_request.hidden? } + + it { is_expected.to eq(false) } + + context 'when the author is banned' do + let_it_be(:author) { create(:user, :banned) } + + it { is_expected.to eq(true) } + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(hide_merge_requests_from_banned_users: false) + end + + it { is_expected.to eq(false) } + end + end + end end diff --git a/spec/models/ml/candidate_spec.rb b/spec/models/ml/candidate_spec.rb index 9ce411191f0..fa8952dc0f4 100644 --- a/spec/models/ml/candidate_spec.rb +++ b/spec/models/ml/candidate_spec.rb @@ -4,6 +4,14 @@ require 'spec_helper' RSpec.describe Ml::Candidate, factory_default: :keep do let_it_be(:candidate) { create(:ml_candidates, :with_metrics_and_params) } + let_it_be(:candidate2) { create(:ml_candidates, experiment: candidate.experiment) } + + let_it_be(:candidate_artifact) do + FactoryBot.create(:generic_package, + name: candidate.package_name, + version: candidate.package_version, + project: candidate.project) + end let(:project) { candidate.experiment.project } @@ -22,13 +30,13 @@ RSpec.describe Ml::Candidate, factory_default: :keep do describe '.artifact_root' do subject { candidate.artifact_root } - it { is_expected.to eq("/ml_candidate_#{candidate.iid}/-/") } + it { is_expected.to eq("/ml_candidate_#{candidate.id}/-/") } end describe '.package_name' do subject { candidate.package_name } - it { is_expected.to eq("ml_candidate_#{candidate.iid}") } + it { is_expected.to eq("ml_candidate_#{candidate.id}") } end describe '.package_version' do @@ -38,27 +46,45 @@ RSpec.describe Ml::Candidate, factory_default: :keep do end describe '.artifact' do - subject { candidate.artifact } + let(:tested_candidate) { candidate } - context 'when has logged artifacts' do - let(:package) do - create(:generic_package, name: candidate.package_name, version: candidate.package_version, project: project) - end + subject { tested_candidate.artifact } - it 'returns the package' do - package + before do + candidate_artifact + end - is_expected.to eq(package) + context 'when has logged artifacts' do + it 'returns the package' do + expect(subject.name).to eq(tested_candidate.package_name) end end context 'when does not have logged artifacts' do - let(:tested_candidate) { create(:ml_candidates, :with_metrics_and_params) } + let(:tested_candidate) { candidate2 } it { is_expected.to be_nil } end end + describe '.artifact_lazy' do + context 'when candidates have same the same iid' do + before do + BatchLoader::Executor.clear_current + end + + it 'loads the correct artifacts', :aggregate_failures do + candidate.artifact_lazy + candidate2.artifact_lazy + + expect(Packages::Package).to receive(:joins).once.and_call_original # Only one database call + + expect(candidate.artifact.name).to eq(candidate.package_name) + expect(candidate2.artifact).to be_nil + end + end + end + describe '#by_project_id_and_iid' do let(:project_id) { candidate.experiment.project_id } let(:iid) { candidate.iid } @@ -95,12 +121,13 @@ RSpec.describe Ml::Candidate, factory_default: :keep do end end - describe "#including_metrics_and_params" do - subject { described_class.including_metrics_and_params.find_by(id: candidate.id) } + describe "#including_relationships" do + subject { described_class.including_relationships.find_by(id: candidate.id) } it 'loads latest metrics and params', :aggregate_failures do expect(subject.association_cached?(:latest_metrics)).to be(true) expect(subject.association_cached?(:params)).to be(true) + expect(subject.association_cached?(:user)).to be(true) end end end diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index e06a6a30f9a..0bf6fdf4fa0 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe NamespaceSetting, type: :model do +RSpec.describe NamespaceSetting, feature_category: :subgroups, type: :model do it_behaves_like 'sanitizable', :namespace_settings, %i[default_branch_name] # Relationships @@ -235,6 +235,80 @@ RSpec.describe NamespaceSetting, type: :model do end end + describe '#allow_runner_registration_token?' do + subject(:group_setting) { group.allow_runner_registration_token? } + + context 'when a top-level group' do + let_it_be(:settings) { create(:namespace_settings) } + let_it_be(:group) { create(:group, namespace_settings: settings) } + + before do + group.update!(allow_runner_registration_token: allow_runner_registration_token) + end + + context 'when :allow_runner_registration_token is false' do + let(:allow_runner_registration_token) { false } + + it 'returns false', :aggregate_failures do + is_expected.to be_falsey + + expect(settings.allow_runner_registration_token).to be_falsey + end + + it 'does not query the db' do + expect { group_setting }.not_to exceed_query_limit(0) + end + end + + context 'when :allow_runner_registration_token is true' do + let(:allow_runner_registration_token) { true } + + it 'returns true', :aggregate_failures do + is_expected.to be_truthy + + expect(settings.allow_runner_registration_token).to be_truthy + end + + context 'when disallowed by application setting' do + before do + stub_application_setting(allow_runner_registration_token: false) + end + + it { is_expected.to be_falsey } + end + end + end + + context 'when a group has parent groups' do + let_it_be_with_refind(:parent) { create(:group) } + let_it_be_with_refind(:group) { create(:group, parent: parent) } + + before do + parent.update!(allow_runner_registration_token: allow_runner_registration_token) + end + + context 'when a parent group has runner registration disabled' do + let(:allow_runner_registration_token) { false } + + it { is_expected.to be_falsey } + end + + context 'when all parent groups have runner registration enabled' do + let(:allow_runner_registration_token) { true } + + it { is_expected.to be_truthy } + + context 'when disallowed by application setting' do + before do + stub_application_setting(allow_runner_registration_token: false) + end + + it { is_expected.to be_falsey } + end + end + end + end + describe '#delayed_project_removal' do it_behaves_like 'a cascading namespace setting boolean attribute', settings_attribute_name: :delayed_project_removal end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 80721e11049..d063f4713c7 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -35,6 +35,7 @@ RSpec.describe Namespace do it { is_expected.to have_one :cluster_enabled_grant } it { is_expected.to have_many(:work_items) } it { is_expected.to have_many :achievements } + it { is_expected.to have_many(:namespace_commit_emails).class_name('Users::NamespaceCommitEmail') } it do is_expected.to have_one(:ci_cd_settings).class_name('NamespaceCiCdSetting').inverse_of(:namespace).autosave(true) @@ -363,6 +364,10 @@ RSpec.describe Namespace do it { is_expected.to delegate_method(:name).to(:owner).with_prefix.allow_nil } it { is_expected.to delegate_method(:avatar_url).to(:owner).allow_nil } it { is_expected.to delegate_method(:prevent_sharing_groups_outside_hierarchy).to(:namespace_settings).allow_nil } + it { is_expected.to delegate_method(:runner_registration_enabled).to(:namespace_settings) } + it { is_expected.to delegate_method(:runner_registration_enabled?).to(:namespace_settings) } + it { is_expected.to delegate_method(:allow_runner_registration_token).to(:namespace_settings) } + it { is_expected.to delegate_method(:allow_runner_registration_token?).to(:namespace_settings) } it { is_expected.to delegate_method(:maven_package_requests_forwarding).to(:package_settings) } it { is_expected.to delegate_method(:pypi_package_requests_forwarding).to(:package_settings) } it { is_expected.to delegate_method(:npm_package_requests_forwarding).to(:package_settings) } @@ -371,6 +376,16 @@ RSpec.describe Namespace do is_expected.to delegate_method(:prevent_sharing_groups_outside_hierarchy=).to(:namespace_settings) .with_arguments(:args).allow_nil end + + it do + is_expected.to delegate_method(:runner_registration_enabled=).to(:namespace_settings) + .with_arguments(:args) + end + + it do + is_expected.to delegate_method(:allow_runner_registration_token=).to(:namespace_settings) + .with_arguments(:args) + end end describe "Respond to" do @@ -2114,7 +2129,7 @@ RSpec.describe Namespace do where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :shared_runners_setting) do true | true | Namespace::SR_ENABLED true | false | Namespace::SR_ENABLED - false | true | Namespace::SR_DISABLED_WITH_OVERRIDE + false | true | Namespace::SR_DISABLED_AND_OVERRIDABLE false | false | Namespace::SR_DISABLED_AND_UNOVERRIDABLE end @@ -2133,12 +2148,15 @@ RSpec.describe Namespace do where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :other_setting, :result) do true | true | Namespace::SR_ENABLED | false true | true | Namespace::SR_DISABLED_WITH_OVERRIDE | true + true | true | Namespace::SR_DISABLED_AND_OVERRIDABLE | true true | true | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | true false | true | Namespace::SR_ENABLED | false false | true | Namespace::SR_DISABLED_WITH_OVERRIDE | false + false | true | Namespace::SR_DISABLED_AND_OVERRIDABLE | false false | true | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | true false | false | Namespace::SR_ENABLED | false false | false | Namespace::SR_DISABLED_WITH_OVERRIDE | false + false | false | Namespace::SR_DISABLED_AND_OVERRIDABLE | false false | false | Namespace::SR_DISABLED_AND_UNOVERRIDABLE | false end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 328d3ba7dda..4b574540500 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1622,6 +1622,50 @@ RSpec.describe Note do expect(described_class.with_suggestions).not_to include(note_without_suggestion) end end + + describe '.inc_relations_for_view' do + subject { note.noteable.notes.inc_relations_for_view(noteable) } + + context 'when noteable can not have diffs' do + let_it_be(:note) { create(:note_on_issue) } + let(:noteable) { note.noteable } + + it 'does not include additional associations' do + expect { subject.reload }.to match_query_count(0).for_model(NoteDiffFile).and( + match_query_count(0).for_model(DiffNotePosition)) + end + + context 'when noteable is not set' do + let(:noteable) { nil } + + it 'includes additional diff associations' do + expect { subject.reload }.to match_query_count(1).for_model(NoteDiffFile).and( + match_query_count(1).for_model(DiffNotePosition)) + end + end + + context 'when skip_notes_diff_include flag is disabled' do + before do + stub_feature_flags(skip_notes_diff_include: false) + end + + it 'includes additional diff associations' do + expect { subject.reload }.to match_query_count(1).for_model(NoteDiffFile).and( + match_query_count(1).for_model(DiffNotePosition)) + end + end + end + + context 'when noteable can have diffs' do + let_it_be(:note) { create(:note_on_commit) } + let(:noteable) { note.noteable } + + it 'includes additional diff associations' do + expect { subject.reload }.to match_query_count(1).for_model(NoteDiffFile).and( + match_query_count(1).for_model(DiffNotePosition)) + end + end + end end describe 'banzai_render_context' do diff --git a/spec/models/oauth_access_token_spec.rb b/spec/models/oauth_access_token_spec.rb index 92e1ae8ac60..fc53d926dd6 100644 --- a/spec/models/oauth_access_token_spec.rb +++ b/spec/models/oauth_access_token_spec.rb @@ -53,4 +53,22 @@ RSpec.describe OauthAccessToken do expect(described_class.matching_token_for(app_one, token.resource_owner, token.scopes)).to be_nil end end + + describe '#expires_in' do + context 'when token has expires_in value set' do + it 'uses the expires_in value' do + token = OauthAccessToken.new(expires_in: 1.minute) + + expect(token.expires_in).to eq 1.minute + end + end + + context 'when token has nil expires_in' do + it 'uses default value' do + token = OauthAccessToken.new(expires_in: nil) + + expect(token.expires_in).to eq 2.hours + end + end + end end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index a244ed34e54..9b341034aaa 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Packages::PackageFile, type: :model do let_it_be(:package_file1) { create(:package_file, :xml, file_name: 'FooBar') } let_it_be(:package_file2) { create(:package_file, :xml, file_name: 'ThisIsATest') } let_it_be(:package_file3) { create(:package_file, :xml, file_name: 'formatted.zip') } + let_it_be(:package_file4) { create(:package_file, :nuget) } let_it_be(:debian_package) { create(:debian_package, project: project) } it_behaves_like 'having unique enum values' @@ -98,6 +99,12 @@ RSpec.describe Packages::PackageFile, type: :model do it { is_expected.to contain_exactly(package_file3) } end + + describe '.with_nuget_format' do + subject { described_class.with_nuget_format } + + it { is_expected.to contain_exactly(package_file4) } + end end context 'updating project statistics' do diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index d6f71f2401c..a8bcda1242f 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Packages::Package, type: :model do +RSpec.describe Packages::Package, type: :model, feature_category: :package_registry do include SortingHelper using RSpec::Parameterized::TableSyntax @@ -14,6 +14,7 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to have_many(:dependency_links).inverse_of(:package) } it { is_expected.to have_many(:tags).inverse_of(:package) } it { is_expected.to have_many(:build_infos).inverse_of(:package) } + it { is_expected.to have_many(:installable_nuget_package_files).inverse_of(:package) } it { is_expected.to have_one(:conan_metadatum).inverse_of(:package) } it { is_expected.to have_one(:maven_metadatum).inverse_of(:package) } it { is_expected.to have_one(:debian_publication).inverse_of(:package).class_name('Packages::Debian::Publication') } @@ -713,7 +714,7 @@ RSpec.describe Packages::Package, type: :model do subject(:destroy!) { package.destroy! } it 'updates the project statistics' do - expect(project_statistics).to receive(:increment_counter).with(:packages_size, -package_file.size) + expect(project_statistics).to receive(:increment_counter).with(:packages_size, have_attributes(amount: -package_file.size)) destroy! end diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 6f684eceaec..ef79ba28d5d 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Pages::LookupPath do +RSpec.describe Pages::LookupPath, feature_category: :pages do let(:project) { create(:project, :pages_private, pages_https_only: true) } subject(:lookup_path) { described_class.new(project) } @@ -126,14 +126,18 @@ RSpec.describe Pages::LookupPath do describe '#prefix' do it 'returns "/" for pages group root projects' do - project = instance_double(Project, pages_group_root?: true) + project = instance_double(Project, pages_namespace_url: "namespace.test", pages_url: "namespace.test") lookup_path = described_class.new(project, trim_prefix: 'mygroup') expect(lookup_path.prefix).to eq('/') end it 'returns the project full path with the provided prefix removed' do - project = instance_double(Project, pages_group_root?: false, full_path: 'mygroup/myproject') + project = instance_double( + Project, + pages_namespace_url: "namespace.test", + pages_url: "namespace.other", + full_path: 'mygroup/myproject') lookup_path = described_class.new(project, trim_prefix: 'mygroup') expect(lookup_path.prefix).to eq('/myproject/') diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index e5f2e849a0a..f054fde78e7 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -567,7 +567,7 @@ RSpec.describe PagesDomain do it 'returns the virual domain when there are pages deployed for the project' do expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.lookup_paths).not_to be_empty - expect(virtual_domain.cache_key).to match(/pages_domain_for_project_#{project.id}_/) + expect(virtual_domain.cache_key).to match(/pages_domain_for_domain_#{pages_domain.id}_/) end context 'when :cache_pages_domain_api is disabled' do diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 9d4c53f8d55..f65b5ff824b 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PersonalAccessToken do +RSpec.describe PersonalAccessToken, feature_category: :authentication_and_authorization do subject { described_class } describe '.build' do @@ -210,6 +210,12 @@ RSpec.describe PersonalAccessToken do expect(personal_access_token).to be_valid end + it "allows creating a token with `admin_mode` scope" do + personal_access_token.scopes = [:api, :admin_mode] + + expect(personal_access_token).to be_valid + end + context 'when registry is disabled' do before do stub_container_registry_config(enabled: false) @@ -340,4 +346,27 @@ RSpec.describe PersonalAccessToken do end end end + + # During the implementation of Admin Mode for API, tokens of + # administrators should automatically get the `admin_mode` scope as well + # See https://gitlab.com/gitlab-org/gitlab/-/issues/42692 + describe '`admin_mode scope' do + subject { create(:personal_access_token, user: user, scopes: ['api']) } + + context 'with administrator user' do + let_it_be(:user) { create(:user, :admin) } + + it 'adds `admin_mode` scope before created' do + expect(subject.scopes).to contain_exactly('api', 'admin_mode') + end + end + + context 'with normal user' do + let_it_be(:user) { create(:user) } + + it 'does not add `admin_mode` scope before created' do + expect(subject.scopes).to contain_exactly('api') + end + end + end end diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index d4e550657c8..3705cab7ef5 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -219,14 +219,21 @@ RSpec.describe PlanLimits do ci_daily_pipeline_schedule_triggers repository_size security_policy_scan_execution_schedules + enforcement_limit + notification_limit ] + disabled_max_artifact_size_columns end + let(:datetime_columns) do + %w[dashboard_limit_enabled_at] + end + it "has positive values for enabled limits" do attributes = plan_limits.attributes attributes = attributes.except(described_class.primary_key) attributes = attributes.except(described_class.reflections.values.map(&:foreign_key)) attributes = attributes.except(*columns_with_zero) + attributes = attributes.except(*datetime_columns) expect(attributes).to all(include(be_positive)) end diff --git a/spec/models/project_import_state_spec.rb b/spec/models/project_import_state_spec.rb index ba1a29a8b27..e5232026c39 100644 --- a/spec/models/project_import_state_spec.rb +++ b/spec/models/project_import_state_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ProjectImportState, type: :model do +RSpec.describe ProjectImportState, type: :model, feature_category: :importers do let_it_be(:correlation_id) { 'cid' } let_it_be(:import_state, refind: true) { create(:import_state, correlation_id_value: correlation_id) } @@ -17,22 +17,19 @@ RSpec.describe ProjectImportState, type: :model do end describe 'Project import job' do - let_it_be(:import_state) { create(:import_state, import_url: generate(:url)) } - let_it_be(:project) { import_state.project } + let_it_be(:project) { create(:project) } - before do - allow_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository) - .with(project.import_url, http_authorization_header: '', mirror: false, resolved_address: '').and_return(true) + let(:import_state) { create(:import_state, import_url: generate(:url), project: project) } + let(:jid) { '551d3ceac5f67a116719ce41' } + before do # Works around https://github.com/rspec/rspec-mocks/issues/910 allow(Project).to receive(:find).with(project.id).and_return(project) - expect(project).to receive(:after_import).and_call_original + allow(project).to receive(:add_import_job).and_return(jid) end it 'imports a project', :sidekiq_might_not_need_inline do - expect(RepositoryImportWorker).to receive(:perform_async).and_call_original - - expect { import_state.schedule }.to change { import_state.status }.from('none').to('finished') + expect { import_state.schedule }.to change { import_state.status }.from('none').to('scheduled') end it 'records job and correlation IDs', :sidekiq_might_not_need_inline do @@ -40,7 +37,8 @@ RSpec.describe ProjectImportState, type: :model do import_state.schedule - expect(import_state.jid).to be_an_instance_of(String) + expect(project).to have_received(:add_import_job) + expect(import_state.jid).to eq(jid) expect(import_state.correlation_id).to eq(correlation_id) end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f33001b9c5b..4ed85844a53 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Project, factory_default: :keep do +RSpec.describe Project, factory_default: :keep, feature_category: :projects do include ProjectForksHelper include ExternalAuthorizationServiceHelpers include ReloadHelpers @@ -28,8 +28,10 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:incident_management_issuable_escalation_statuses).through(:issues).inverse_of(:project).class_name('IncidentManagement::IssuableEscalationStatus') } it { is_expected.to have_many(:milestones) } it { is_expected.to have_many(:project_members).dependent(:delete_all) } + it { is_expected.to have_many(:namespace_members) } it { is_expected.to have_many(:users).through(:project_members) } it { is_expected.to have_many(:requesters).dependent(:delete_all) } + it { is_expected.to have_many(:namespace_requesters) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:snippets).class_name('ProjectSnippet') } it { is_expected.to have_many(:deploy_keys_projects) } @@ -47,6 +49,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_one(:webex_teams_integration) } it { is_expected.to have_one(:packagist_integration) } it { is_expected.to have_one(:pushover_integration) } + it { is_expected.to have_one(:apple_app_store_integration) } it { is_expected.to have_one(:asana_integration) } it { is_expected.to have_many(:boards) } it { is_expected.to have_one(:campfire_integration) } @@ -163,6 +166,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:wiki_page_hooks_integrations).class_name('Integration') } it { is_expected.to have_many(:deployment_hooks_integrations).class_name('Integration') } it { is_expected.to have_many(:alert_hooks_integrations).class_name('Integration') } + it { is_expected.to have_many(:incident_hooks_integrations).class_name('Integration') } # GitLab Pages it { is_expected.to have_many(:pages_domains) } @@ -346,6 +350,108 @@ RSpec.describe Project, factory_default: :keep do end end + shared_examples 'query without source filters' do + it do + expect(subject.where_values_hash.keys).not_to include('source_id', 'source_type') + end + end + + describe '#namespace_members' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:requester) { create(:user) } + let_it_be(:developer) { create(:user) } + + before_all do + project.request_access(requester) + project.add_developer(developer) + end + + it 'includes the correct users' do + expect(project.namespace_members).to include Member.find_by(user: developer) + expect(project.namespace_members).not_to include Member.find_by(user: requester) + end + + it 'is equivalent to #project_members' do + expect(project.namespace_members).to match_array(project.project_members) + end + + it_behaves_like 'query without source filters' do + subject { project.namespace_members } + end + end + + describe '#namespace_requesters' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:requester) { create(:user) } + let_it_be(:developer) { create(:user) } + + before_all do + project.request_access(requester) + project.add_developer(developer) + end + + it 'includes the correct users' do + expect(project.namespace_requesters).to include Member.find_by(user: requester) + expect(project.namespace_requesters).not_to include Member.find_by(user: developer) + end + + it 'is equivalent to #project_members' do + expect(project.namespace_requesters).to eq project.requesters + end + + it_behaves_like 'query without source filters' do + subject { project.namespace_requesters } + end + end + + shared_examples 'polymorphic membership relationship' do + it do + expect(membership.attributes).to include( + 'source_type' => 'Project', + 'source_id' => project.id + ) + end + end + + shared_examples 'member_namespace membership relationship' do + it do + expect(membership.attributes).to include( + 'member_namespace_id' => project.project_namespace_id + ) + end + end + + describe '#namespace_members setters' do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:membership) { project.namespace_members.create!(user: user, access_level: Gitlab::Access::DEVELOPER) } + + it { expect(membership).to be_instance_of(ProjectMember) } + it { expect(membership.user).to eq user } + it { expect(membership.project).to eq project } + it { expect(membership.requested_at).to be_nil } + + it_behaves_like 'polymorphic membership relationship' + it_behaves_like 'member_namespace membership relationship' + end + + describe '#namespace_requesters setters' do + let_it_be(:requested_at) { Time.current } + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:membership) do + project.namespace_requesters.create!(user: user, requested_at: requested_at, access_level: Gitlab::Access::DEVELOPER) + end + + it { expect(membership).to be_instance_of(ProjectMember) } + it { expect(membership.user).to eq user } + it { expect(membership.project).to eq project } + it { expect(membership.requested_at).to eq requested_at } + + it_behaves_like 'polymorphic membership relationship' + it_behaves_like 'member_namespace membership relationship' + end + describe '#members & #requesters' do let_it_be(:project) { create(:project, :public) } let_it_be(:requester) { create(:user) } @@ -2490,16 +2596,28 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#pages_url' do + describe '#pages_url', feature_category: :pages do let(:group) { create(:group, name: group_name) } - let(:project) { create(:project, namespace: group, name: project_name) } + + let(:project_path) { project_name.downcase } + let(:project) do + create( + :project, + namespace: group, + name: project_name, + path: project_path) + end + let(:domain) { 'Example.com' } + let(:port) { nil } subject { project.pages_url } before do allow(Settings.pages).to receive(:host).and_return(domain) - allow(Gitlab.config.pages).to receive(:url).and_return('http://example.com') + allow(Gitlab.config.pages) + .to receive(:url) + .and_return(['http://example.com', port].compact.join(':')) end context 'group page' do @@ -2509,9 +2627,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to eq("http://group.example.com") } context 'mixed case path' do - before do - project.update!(path: 'Group.example.com') - end + let(:project_path) { 'Group.example.com' } it { is_expected.to eq("http://group.example.com") } end @@ -2524,22 +2640,88 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to eq("http://group.example.com/project") } context 'mixed case path' do + let(:project_path) { 'Project' } + + it { is_expected.to eq("http://group.example.com/Project") } + end + end + + context 'when there is an explicit port' do + let(:port) { 3000 } + + context 'when not in dev mode' do before do - project.update!(path: 'Project') + stub_rails_env('production') end - it { is_expected.to eq("http://group.example.com/Project") } + context 'group page' do + let(:group_name) { 'Group' } + let(:project_name) { 'group.example.com' } + + it { is_expected.to eq('http://group.example.com:3000/group.example.com') } + + context 'mixed case path' do + let(:project_path) { 'Group.example.com' } + + it { is_expected.to eq('http://group.example.com:3000/Group.example.com') } + end + end + + context 'project page' do + let(:group_name) { 'Group' } + let(:project_name) { 'Project' } + + it { is_expected.to eq("http://group.example.com:3000/project") } + + context 'mixed case path' do + let(:project_path) { 'Project' } + + it { is_expected.to eq("http://group.example.com:3000/Project") } + end + end + end + + context 'when in dev mode' do + before do + stub_rails_env('development') + end + + context 'group page' do + let(:group_name) { 'Group' } + let(:project_name) { 'group.example.com' } + + it { is_expected.to eq('http://group.example.com:3000') } + + context 'mixed case path' do + let(:project_path) { 'Group.example.com' } + + it { is_expected.to eq('http://group.example.com:3000') } + end + end + + context 'project page' do + let(:group_name) { 'Group' } + let(:project_name) { 'Project' } + + it { is_expected.to eq("http://group.example.com:3000/project") } + + context 'mixed case path' do + let(:project_path) { 'Project' } + + it { is_expected.to eq("http://group.example.com:3000/Project") } + end + end end end end - describe '#pages_group_url' do + describe '#pages_namespace_url', feature_category: :pages do let(:group) { create(:group, name: group_name) } let(:project) { create(:project, namespace: group, name: project_name) } let(:domain) { 'Example.com' } let(:port) { 1234 } - subject { project.pages_group_url } + subject { project.pages_namespace_url } before do allow(Settings.pages).to receive(:host).and_return(domain) @@ -5808,22 +5990,6 @@ RSpec.describe Project, factory_default: :keep do expect(recorder.count).to be_zero end - - context 'with cache_project_integrations disabled' do - before do - stub_feature_flags(cache_project_integrations: false) - end - - it 'triggers extra queries when called multiple times' do - integration.project.execute_integrations({}, :push_hooks) - - recorder = ActiveRecord::QueryRecorder.new do - integration.project.execute_integrations({}, :push_hooks) - end - - expect(recorder.count).not_to be_zero - end - end end describe '#has_active_hooks?' do @@ -6653,8 +6819,8 @@ RSpec.describe Project, factory_default: :keep do where(:shared_runners_setting, :project_shared_runners_enabled, :valid_record) do :shared_runners_enabled | true | true :shared_runners_enabled | false | true - :disabled_with_override | true | true - :disabled_with_override | false | true + :disabled_and_overridable | true | true + :disabled_and_overridable | false | true :disabled_and_unoverridable | true | false :disabled_and_unoverridable | false | true end @@ -6902,21 +7068,6 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#pages_group_root?' do - it 'returns returns true if pages_url is same as pages_group_url' do - project = build(:project) - expect(project).to receive(:pages_url).and_return(project.pages_group_url) - - expect(project.pages_group_root?).to eq(true) - end - - it 'returns returns false if pages_url is different than pages_group_url' do - project = build(:project) - - expect(project.pages_group_root?).to eq(false) - end - end - describe '#closest_setting' do shared_examples_for 'fetching closest setting' do let!(:namespace) { create(:namespace) } @@ -7038,8 +7189,8 @@ RSpec.describe Project, factory_default: :keep do create_list(:chat_name, 5, integration: integration) end - it 'removes chat names on removal' do - expect { subject.destroy! }.to change { ChatName.count }.by(-5) + it 'does not remove chat names on removal' do + expect { subject.destroy! }.not_to change { ChatName.count } end end @@ -7612,32 +7763,6 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#increment_statistic_value' do - let(:project) { build_stubbed(:project) } - - subject(:increment) do - project.increment_statistic_value(:build_artifacts_size, -10) - end - - it 'increments the value' do - expect(ProjectStatistics) - .to receive(:increment_statistic) - .with(project, :build_artifacts_size, -10) - - increment - end - - context 'when the project is scheduled for removal' do - let(:project) { build_stubbed(:project, pending_delete: true) } - - it 'does not increment the value' do - expect(ProjectStatistics).not_to receive(:increment_statistic) - - increment - end - end - end - describe 'topics' do let_it_be(:project) { create(:project, name: 'topic-project', topic_list: 'topic1, topic2, topic3') } diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index a6e2bcf1525..ef53de6ad82 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -455,35 +455,50 @@ RSpec.describe ProjectStatistics do end describe '.increment_statistic' do - shared_examples 'a statistic that increases storage_size' do + shared_examples 'a statistic that increases storage_size synchronously' do + let(:increment) { Gitlab::Counters::Increment.new(amount: 13) } + it 'increases the statistic by that amount' do - expect { described_class.increment_statistic(project, stat, 13) } + expect { described_class.increment_statistic(project, stat, increment) } .to change { statistics.reload.send(stat) || 0 } - .by(13) + .by(increment.amount) end it 'increases also storage size by that amount' do - expect { described_class.increment_statistic(project, stat, 20) } + expect { described_class.increment_statistic(project, stat, increment) } .to change { statistics.reload.storage_size } - .by(20) + .by(increment.amount) end it 'schedules a namespace aggregation worker' do expect(Namespaces::ScheduleAggregationWorker).to receive(:perform_async) .with(statistics.project.namespace.id) - described_class.increment_statistic(project, stat, 20) + described_class.increment_statistic(project, stat, increment) + end + + context 'when the project is pending delete' do + before do + project.update_attribute(:pending_delete, true) + end + + it 'does not change the statistics' do + expect { described_class.increment_statistic(project, stat, increment) } + .not_to change { statistics.reload.send(stat) } + end end end shared_examples 'a statistic that increases storage_size asynchronously' do + let(:increment) { Gitlab::Counters::Increment.new(amount: 13) } + it 'stores the increment temporarily in Redis', :clean_gitlab_redis_shared_state do - described_class.increment_statistic(project, stat, 13) + described_class.increment_statistic(project, stat, increment) Gitlab::Redis::SharedState.with do |redis| key = statistics.counter(stat).key - increment = redis.get(key) - expect(increment.to_i).to eq(13) + value = redis.get(key) + expect(value.to_i).to eq(increment.amount) end end @@ -493,9 +508,20 @@ RSpec.describe ProjectStatistics do .with(Gitlab::Counters::BufferedCounter::WORKER_DELAY, described_class.name, statistics.id, stat) .and_call_original - expect { described_class.increment_statistic(project, stat, 20) } - .to change { statistics.reload.send(stat) }.by(20) - .and change { statistics.reload.send(:storage_size) }.by(20) + expect { described_class.increment_statistic(project, stat, increment) } + .to change { statistics.reload.send(stat) }.by(increment.amount) + .and change { statistics.reload.send(:storage_size) }.by(increment.amount) + end + + context 'when the project is pending delete' do + before do + project.update_attribute(:pending_delete, true) + end + + it 'does not change the statistics' do + expect { described_class.increment_statistic(project, stat, increment) } + .not_to change { [statistics.reload.send(stat), statistics.reload.send(:storage_size)] } + end end end @@ -508,7 +534,7 @@ RSpec.describe ProjectStatistics do context 'when adjusting :pipeline_artifacts_size' do let(:stat) { :pipeline_artifacts_size } - it_behaves_like 'a statistic that increases storage_size' + it_behaves_like 'a statistic that increases storage_size synchronously' end context 'when adjusting :packages_size' do @@ -518,9 +544,11 @@ RSpec.describe ProjectStatistics do end context 'when the amount is 0' do + let(:increment) { Gitlab::Counters::Increment.new(amount: 0) } + it 'does not execute a query' do project - expect { described_class.increment_statistic(project, :build_artifacts_size, 0) } + expect { described_class.increment_statistic(project, :build_artifacts_size, increment) } .not_to exceed_query_limit(0) end end @@ -532,4 +560,116 @@ RSpec.describe ProjectStatistics do end end end + + describe '.bulk_increment_statistic' do + let(:increments) { [10, 3].map { |amount| Gitlab::Counters::Increment.new(amount: amount) } } + let(:total_amount) { increments.sum(&:amount) } + + shared_examples 'a statistic that increases storage_size synchronously' do + it 'increases the statistic by that amount' do + expect { described_class.bulk_increment_statistic(project, stat, increments) } + .to change { statistics.reload.send(stat) || 0 } + .by(total_amount) + end + + it 'increases also storage size by that amount' do + expect { described_class.bulk_increment_statistic(project, stat, increments) } + .to change { statistics.reload.storage_size } + .by(total_amount) + end + + it 'schedules a namespace aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker).to receive(:perform_async) + .with(statistics.project.namespace.id) + + described_class.bulk_increment_statistic(project, stat, increments) + end + + context 'when the project is pending delete' do + before do + project.update_attribute(:pending_delete, true) + end + + it 'does not change the statistics' do + expect { described_class.bulk_increment_statistic(project, stat, increments) } + .not_to change { statistics.reload.send(stat) } + end + end + end + + shared_examples 'a statistic that increases storage_size asynchronously' do + it 'stores the increment temporarily in Redis', :clean_gitlab_redis_shared_state do + described_class.bulk_increment_statistic(project, stat, increments) + + Gitlab::Redis::SharedState.with do |redis| + key = statistics.counter(stat).key + increment = redis.get(key) + expect(increment.to_i).to eq(total_amount) + end + end + + it 'schedules a worker to update the statistic and storage_size async', :sidekiq_inline do + expect(FlushCounterIncrementsWorker) + .to receive(:perform_in) + .with(Gitlab::Counters::BufferedCounter::WORKER_DELAY, described_class.name, statistics.id, stat) + .and_call_original + + expect { described_class.bulk_increment_statistic(project, stat, increments) } + .to change { statistics.reload.send(stat) }.by(total_amount) + .and change { statistics.reload.send(:storage_size) }.by(total_amount) + end + + context 'when the project is pending delete' do + before do + project.update_attribute(:pending_delete, true) + end + + it 'does not change the statistics' do + expect { described_class.bulk_increment_statistic(project, stat, increments) } + .not_to change { [statistics.reload.send(stat), statistics.reload.send(:storage_size)] } + end + end + end + + context 'when adjusting :build_artifacts_size' do + let(:stat) { :build_artifacts_size } + + it_behaves_like 'a statistic that increases storage_size asynchronously' + + context 'when :project_statistics_bulk_increment flag is disabled' do + before do + stub_feature_flags(project_statistics_bulk_increment: false) + end + + it 'calls increment_statistic on once with the sum of the increments' do + total_amount = increments.sum(&:amount) + expect(statistics) + .to receive(:increment_statistic).with(stat, have_attributes(amount: total_amount)).and_call_original + + described_class.bulk_increment_statistic(project, stat, increments) + end + + it_behaves_like 'a statistic that increases storage_size asynchronously' + end + end + + context 'when adjusting :pipeline_artifacts_size' do + let(:stat) { :pipeline_artifacts_size } + + it_behaves_like 'a statistic that increases storage_size synchronously' + end + + context 'when adjusting :packages_size' do + let(:stat) { :packages_size } + + it_behaves_like 'a statistic that increases storage_size asynchronously' + end + + context 'when using an invalid column' do + it 'raises an error' do + expect { described_class.bulk_increment_statistic(project, :id, increments) } + .to raise_error(ArgumentError, "Cannot increment attribute: id") + end + end + end end diff --git a/spec/models/projects/branch_rule_spec.rb b/spec/models/projects/branch_rule_spec.rb new file mode 100644 index 00000000000..6910fbbb6db --- /dev/null +++ b/spec/models/projects/branch_rule_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::BranchRule, feature_category: :source_code_management do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:protected_branch) { create(:protected_branch, project: project, name: 'feature*') } + + subject { described_class.new(protected_branch.project, protected_branch) } + + it 'delegates methods to protected branch' do + expect(subject).to delegate_method(:name).to(:protected_branch) + expect(subject).to delegate_method(:group).to(:protected_branch) + expect(subject).to delegate_method(:default_branch?).to(:protected_branch) + expect(subject).to delegate_method(:created_at).to(:protected_branch) + expect(subject).to delegate_method(:updated_at).to(:protected_branch) + end + + it 'is protected' do + expect(subject.protected?).to eq(true) + end + + it 'branch protection returns protected branch' do + expect(subject.branch_protection).to eq(protected_branch) + end + + describe '#matching_branches_count' do + it 'returns the number of branches that are matching the protected branch name' do + expect(subject.matching_branches_count).to eq(2) + end + end +end diff --git a/spec/models/projects/build_artifacts_size_refresh_spec.rb b/spec/models/projects/build_artifacts_size_refresh_spec.rb index caff06262d9..7255c8ac89b 100644 --- a/spec/models/projects/build_artifacts_size_refresh_spec.rb +++ b/spec/models/projects/build_artifacts_size_refresh_spec.rb @@ -14,10 +14,11 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do end describe 'scopes' do - let_it_be(:refresh_1) { create(:project_build_artifacts_size_refresh, :running, updated_at: (described_class::STALE_WINDOW + 1.second).ago) } + let_it_be(:refresh_1) { create(:project_build_artifacts_size_refresh, :stale) } let_it_be(:refresh_2) { create(:project_build_artifacts_size_refresh, :running, updated_at: 1.hour.ago) } let_it_be(:refresh_3) { create(:project_build_artifacts_size_refresh, :pending) } let_it_be(:refresh_4) { create(:project_build_artifacts_size_refresh, :created) } + let_it_be(:refresh_5) { create(:project_build_artifacts_size_refresh, :finalizing) } describe 'stale' do it 'returns records in running state and has not been updated for more than 2 hours' do @@ -26,15 +27,23 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do end describe 'remaining' do - it 'returns stale, created, and pending records' do + it 'returns stale, created and pending records' do expect(described_class.remaining).to match_array([refresh_1, refresh_3, refresh_4]) end + + it 'does not include finalizing' do + expect(described_class.processing_queue).not_to include(refresh_5) + end end describe 'processing_queue' do it 'prioritizes pending -> stale -> created' do expect(described_class.processing_queue).to eq([refresh_3, refresh_1, refresh_4]) end + + it 'does not include finalizing' do + expect(described_class.processing_queue).not_to include(refresh_5) + end end end @@ -58,10 +67,7 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do let_it_be_with_reload(:refresh) do create( :project_build_artifacts_size_refresh, - :created, - updated_at: 2.days.ago, - refresh_started_at: nil, - last_job_artifact_id: nil + :created ) end @@ -70,8 +76,8 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do let(:statistics) { refresh.project.statistics } before do - stats = create(:project_statistics, project: refresh.project, build_artifacts_size: 120) - stats.increment_counter(:build_artifacts_size, 30) + statistics.update!(build_artifacts_size: 120) + statistics.increment_counter(:build_artifacts_size, Gitlab::Counters::Increment.new(amount: 30)) end it 'transitions the state to running' do @@ -91,11 +97,11 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do end it 'resets the build artifacts size stats' do - expect { refresh.process! }.to change { statistics.build_artifacts_size }.to(0) + expect { refresh.process! }.to change { statistics.reload.build_artifacts_size }.from(120).to(0) end - it 'resets the counter attribute to zero' do - expect { refresh.process! }.to change { statistics.counter(:build_artifacts_size).get }.to(0) + it 'resets the buffered counter value to zero' do + expect { refresh.process! }.to change { Gitlab::Counters::BufferedCounter.new(statistics, :build_artifacts_size).get }.to(0) end end @@ -170,6 +176,22 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do expect { refresh.requeue!(last_job_artifact_id) }.to change { refresh.reload.last_job_artifact_id.to_i }.to(last_job_artifact_id) end end + + describe '#schedule_finalize!' do + let!(:refresh) { create(:project_build_artifacts_size_refresh, :running) } + + it 'transitions refresh state from running to finalizing' do + expect { refresh.schedule_finalize! }.to change { refresh.reload.state }.to(described_class::STATES[:finalizing]) + end + + it 'schedules Projects::FinalizeProjectStatisticsRefreshWorker' do + expect(Projects::FinalizeProjectStatisticsRefreshWorker) + .to receive(:perform_in) + .with(described_class::FINALIZE_DELAY, refresh.class.to_s, refresh.id) + + refresh.schedule_finalize! + end + end end describe '.process_next_refresh!' do @@ -210,6 +232,26 @@ RSpec.describe Projects::BuildArtifactsSizeRefresh, type: :model do end end + describe '#finalize!' do + let!(:refresh) { create(:project_build_artifacts_size_refresh, :finalizing) } + + let(:statistics) { refresh.project.statistics } + + before do + allow(statistics).to receive(:finalize_refresh) + end + + it 'stores the refresh amount into the buffered counter' do + expect(statistics).to receive(:finalize_refresh).with(described_class::COUNTER_ATTRIBUTE_NAME) + + refresh.finalize! + end + + it 'destroys the refresh record' do + expect { refresh.finalize! }.to change { described_class.count }.by(-1) + end + end + describe '#next_batch' do let!(:project) { create(:project) } let!(:artifact_1) { create(:ci_job_artifact, project: project, created_at: 14.days.ago) } diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 180a76ff593..5ed4eb7d233 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -86,6 +86,10 @@ RSpec.describe Release do context 'when updating existing release without author' do let(:release) { create(:release, :legacy) } + before do + stub_feature_flags(validate_release_with_author: false) + end + it 'updates successfully' do release.description += 'Update' diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 969a279dd52..a3d2f9a09fb 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Repository do +RSpec.describe Repository, feature_category: :source_code_management do include RepoHelpers before do @@ -534,21 +534,48 @@ RSpec.describe Repository do end describe '#find_commits_by_message' do - it 'returns commits with messages containing a given string' do - commit_ids = repository.find_commits_by_message('submodule').map(&:id) + subject(:find_commits_by_message) { repository.find_commits_by_message(query) } - expect(commit_ids).to include( - '5937ac0a7beb003549fc5fd26fc247adbce4a52e', - '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9', - 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' - ) + let(:commit_ids) { find_commits_by_message.map(&:id) } + let(:query) { 'submodule' } + let(:expected_commit_ids) do + %w[ + 5937ac0a7beb003549fc5fd26fc247adbce4a52e + 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + cfe32cf61b73a0d5e9f13e774abde7ff789b1660 + ] + end + + it 'returns commits with messages containing a given string' do + expect(commit_ids).to include(*expected_commit_ids) expect(commit_ids).not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') end - it 'is case insensitive' do - commit_ids = repository.find_commits_by_message('SUBMODULE').map(&:id) + context 'when query is in UPCASE' do + let(:query) { 'SUBMODULE' } + + it 'is case insensitive' do + expect(commit_ids).to include(*expected_commit_ids) + end + end + + context 'when message has surrounding spaces' do + let(:query) { ' submodule ' } + + it 'removes spaces and returns commits by message' do + expect(commit_ids).to include(*expected_commit_ids) + expect(commit_ids).not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') + end + + context 'when feature flag "commit_search_trailing_spaces" is disabled' do + before do + stub_feature_flags(commit_search_trailing_spaces: false) + end - expect(commit_ids).to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') + it 'returns an empty list' do + expect(commit_ids).to be_empty + end + end end describe 'when storage is broken', :broken_storage do @@ -2691,12 +2718,26 @@ RSpec.describe Repository do end it 'caches the response' do - expect(repository.head_tree).to receive(:readme_path).and_call_original.once + expect(repository).to receive(:search_files_by_regexp).and_call_original.once 2.times do expect(repository.readme_path).to eq("README.md") end end + + context 'when "readme_from_gitaly" FF is disabled' do + before do + stub_feature_flags(readme_from_gitaly: false) + end + + it 'caches the response' do + expect(repository.head_tree).to receive(:readme_path).and_call_original.once + + 2.times do + expect(repository.readme_path).to eq("README.md") + end + end + end end end end diff --git a/spec/models/resource_event_spec.rb b/spec/models/resource_event_spec.rb new file mode 100644 index 00000000000..f40c192ab2b --- /dev/null +++ b/spec/models/resource_event_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ResourceEvent, feature_category: :team_planing, type: :model do + let(:dummy_resource_label_event_class) do + Class.new(ResourceEvent) do + self.table_name = 'resource_label_events' + end + end + + it 'raises error on not implemented `issuable` method' do + expect { dummy_resource_label_event_class.new.issuable }.to raise_error(NoMethodError) + end + + it 'raises error on not implemented `synthetic_note_class` method' do + expect { dummy_resource_label_event_class.new.synthetic_note_class }.to raise_error(NoMethodError) + end +end diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb index 5087a8e8524..87f3b9fb2bb 100644 --- a/spec/models/resource_label_event_spec.rb +++ b/spec/models/resource_label_event_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ResourceLabelEvent, type: :model do +RSpec.describe ResourceLabelEvent, feature_category: :team_planing, type: :model do let_it_be(:project) { create(:project, :repository) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:merge_request) { create(:merge_request, source_project: project) } @@ -15,6 +15,7 @@ RSpec.describe ResourceLabelEvent, type: :model do it_behaves_like 'a resource event' it_behaves_like 'a resource event for issues' it_behaves_like 'a resource event for merge requests' + it_behaves_like 'a note for work item resource event' describe 'associations' do it { is_expected.to belong_to(:label) } @@ -154,4 +155,19 @@ RSpec.describe ResourceLabelEvent, type: :model do expect(event_1.discussion_id).not_to eq(event_2.discussion_id) end end + + context 'with multiple label events' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:work_item) { create(:work_item, :task, project: project, author: user) } + let_it_be(:events) { create_pair(:resource_label_event, issue: work_item) } + + it 'builds synthetic note' do + first_event = events.first + synthetic_note = first_event.work_item_synthetic_system_note(events: events) + + expect(synthetic_note.class.name).to eq(first_event.synthetic_note_class.name) + expect(synthetic_note.events).to match_array(events) + end + end end diff --git a/spec/models/resource_milestone_event_spec.rb b/spec/models/resource_milestone_event_spec.rb index c1761e5b2e8..11b704ceadf 100644 --- a/spec/models/resource_milestone_event_spec.rb +++ b/spec/models/resource_milestone_event_spec.rb @@ -2,10 +2,11 @@ require 'spec_helper' -RSpec.describe ResourceMilestoneEvent, type: :model do +RSpec.describe ResourceMilestoneEvent, feature_category: :team_planing, type: :model do it_behaves_like 'a resource event' it_behaves_like 'a resource event for issues' it_behaves_like 'a resource event for merge requests' + it_behaves_like 'a note for work item resource event' it_behaves_like 'having unique enum values' it_behaves_like 'timebox resource event validations' diff --git a/spec/models/resource_state_event_spec.rb b/spec/models/resource_state_event_spec.rb index f84634bd220..04e4359a3ff 100644 --- a/spec/models/resource_state_event_spec.rb +++ b/spec/models/resource_state_event_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ResourceStateEvent, type: :model do +RSpec.describe ResourceStateEvent, feature_category: :team_planing, type: :model do subject { build(:resource_state_event, issue: issue) } let(:issue) { create(:issue) } @@ -11,6 +11,7 @@ RSpec.describe ResourceStateEvent, type: :model do it_behaves_like 'a resource event' it_behaves_like 'a resource event for issues' it_behaves_like 'a resource event for merge requests' + it_behaves_like 'a note for work item resource event' describe 'validations' do describe 'Issuable validation' do diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index f96d02e6a82..515057a862b 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Timelog do +RSpec.describe Timelog, feature_category: :team_planning do subject { create(:timelog) } let_it_be(:issue) { create(:issue) } @@ -149,4 +149,30 @@ RSpec.describe Timelog do end end end + + describe 'sorting' do + let_it_be(:user) { create(:user) } + let_it_be(:timelog_a) { create(:issue_timelog, time_spent: 7200, spent_at: 1.hour.ago, user: user) } + let_it_be(:timelog_b) { create(:issue_timelog, time_spent: 5400, spent_at: 2.hours.ago, user: user) } + let_it_be(:timelog_c) { create(:issue_timelog, time_spent: 1800, spent_at: 30.minutes.ago, user: user) } + let_it_be(:timelog_d) { create(:issue_timelog, time_spent: 3600, spent_at: 1.day.ago, user: user) } + + describe '.sort_by_field' do + it 'sorts timelogs by time spent in ascending order' do + expect(user.timelogs.sort_by_field('time_spent', :asc)).to eq([timelog_c, timelog_d, timelog_b, timelog_a]) + end + + it 'sorts timelogs by time spent in descending order' do + expect(user.timelogs.sort_by_field('time_spent', :desc)).to eq([timelog_a, timelog_b, timelog_d, timelog_c]) + end + + it 'sorts timelogs by spent at in ascending order' do + expect(user.timelogs.sort_by_field('spent_at', :asc)).to eq([timelog_d, timelog_b, timelog_a, timelog_c]) + end + + it 'sorts timelogs by spent at in descending order' do + expect(user.timelogs.sort_by_field('spent_at', :desc)).to eq([timelog_c, timelog_a, timelog_b, timelog_d]) + end + end + end end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 221f09dd87f..8669db4af16 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -175,6 +175,15 @@ RSpec.describe Todo do end describe '#target_reference' do + shared_examples 'returns full_path' do + specify do + subject.target = target + subject.action = Todo::MEMBER_ACCESS_REQUESTED + + expect(subject.target_reference).to eq target.full_path + end + end + it 'returns commit full reference with short id' do project = create(:project, :repository) commit = project.commit @@ -193,13 +202,10 @@ RSpec.describe Todo do end context 'when target is member access requested' do - it 'returns group full path' do - group = create(:group) - - subject.target = group - subject.action = Todo::MEMBER_ACCESS_REQUESTED - - expect(subject.target_reference).to eq group.full_path + %i[project group].each do |target_type| + it_behaves_like 'returns full_path' do + let(:target) { create(target_type, :public) } + end end end end @@ -525,4 +531,46 @@ RSpec.describe Todo do expect(described_class.for_internal_notes).to contain_exactly(todo) end end + + describe '#access_request_url' do + shared_examples 'returns member access requests tab url/path' do + it 'returns group access requests tab url/path if target is group' do + group = create(:group) + subject.target = group + + expect(subject.access_request_url(only_path: only_path)).to eq(Gitlab::Routing.url_helpers.group_group_members_url(group, tab: 'access_requests', only_path: only_path)) + end + + it 'returns project access requests tab url/path if target is project' do + project = create(:project) + subject.target = project + + expect(subject.access_request_url(only_path: only_path)).to eq(Gitlab::Routing.url_helpers.project_project_members_url(project, tab: 'access_requests', only_path: only_path)) + end + + it 'returns empty string if target is neither group nor project' do + subject.target = issue + + expect(subject.access_request_url(only_path: only_path)).to eq("") + end + end + + context 'when only_path param is false' do + it_behaves_like 'returns member access requests tab url/path' do + let_it_be(:only_path) { false } + end + end + + context 'when only_path param is nil' do + it_behaves_like 'returns member access requests tab url/path' do + let_it_be(:only_path) { nil } + end + end + + context 'when only_path param is true' do + it_behaves_like 'returns member access requests tab url/path' do + let_it_be(:only_path) { true } + end + end + end end diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index ed55aca49b7..1893b6530a5 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -68,26 +68,34 @@ RSpec.describe UserDetail do end end - describe '.user_fields_changed?' do - let(:user) { create(:user) } - - context 'when user detail fields unchanged' do - it 'returns false' do - expect(described_class.user_fields_changed?(user)).to be false - end - - %i[linkedin location organization skype twitter website_url].each do |attr| - context "when #{attr} is changed" do - before do - user[attr] = 'new value' - end - - it 'returns true' do - expect(described_class.user_fields_changed?(user)).to be true - end - end + describe '#save' do + let(:user_detail) do + create(:user_detail, + bio: 'bio', + linkedin: 'linkedin', + twitter: 'twitter', + skype: 'skype', + location: 'location', + organization: 'organization', + website_url: 'https://example.com') + end + + shared_examples 'prevents `nil` value' do |attr| + it 'converts `nil` to the empty string' do + user_detail[attr] = nil + expect { user_detail.save! } + .to change { user_detail[attr] }.to('') + .and not_change { user_detail.attributes.except(attr.to_s) } end end + + it_behaves_like 'prevents `nil` value', :bio + it_behaves_like 'prevents `nil` value', :linkedin + it_behaves_like 'prevents `nil` value', :twitter + it_behaves_like 'prevents `nil` value', :skype + it_behaves_like 'prevents `nil` value', :location + it_behaves_like 'prevents `nil` value', :organization + it_behaves_like 'prevents `nil` value', :website_url end describe '#sanitize_attrs' do @@ -137,45 +145,4 @@ RSpec.describe UserDetail do details.save! end end - - describe '#assign_changed_fields_from_user' do - let(:user_detail) { build(:user_detail) } - - shared_examples 'syncs field with `user_details`' do |field| - it 'does not sync the field to `user_details` if unchanged' do - expect { user_detail.assign_changed_fields_from_user } - .to not_change { user_detail.public_send(field) } - end - - it 'syncs the field to `user_details` if changed' do - user_detail.user[field] = "new_value" - expect { user_detail.assign_changed_fields_from_user } - .to change { user_detail.public_send(field) } - .to("new_value") - end - - it 'truncates the field if too long' do - user_detail.user[field] = 'a' * (UserDetail::DEFAULT_FIELD_LENGTH + 1) - expect { user_detail.assign_changed_fields_from_user } - .to change { user_detail.public_send(field) } - .to('a' * UserDetail::DEFAULT_FIELD_LENGTH) - end - - it 'properly syncs nil field to `user_details' do - user_detail.user[field] = 'Test' - user_detail.user.save!(validate: false) - user_detail.user[field] = nil - expect { user_detail.assign_changed_fields_from_user } - .to change { user_detail.public_send(field) } - .to('') - end - end - - it_behaves_like 'syncs field with `user_details`', :linkedin - it_behaves_like 'syncs field with `user_details`', :location - it_behaves_like 'syncs field with `user_details`', :organization - it_behaves_like 'syncs field with `user_details`', :skype - it_behaves_like 'syncs field with `user_details`', :twitter - it_behaves_like 'syncs field with `user_details`', :website_url - end end diff --git a/spec/models/user_highest_role_spec.rb b/spec/models/user_highest_role_spec.rb index 3ae672cf7f7..7ef04466b6f 100644 --- a/spec/models/user_highest_role_spec.rb +++ b/spec/models/user_highest_role_spec.rb @@ -8,7 +8,7 @@ RSpec.describe UserHighestRole do end describe 'validations' do - it { is_expected.to validate_inclusion_of(:highest_access_level).in_array([nil, *Gitlab::Access.all_values]) } + it { is_expected.to validate_inclusion_of(:highest_access_level).in_array(Gitlab::Access.all_values).allow_nil } end describe 'scopes' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4dbcc68af19..e2e4e4248d8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe User do +RSpec.describe User, feature_category: :users do include ProjectForksHelper include TermsHelper include ExclusiveLeaseHelpers @@ -101,6 +101,24 @@ RSpec.describe User do it { is_expected.to delegate_method(:requires_credit_card_verification).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:requires_credit_card_verification=).to(:user_detail).with_arguments(:args).allow_nil } + + it { is_expected.to delegate_method(:linkedin).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:linkedin=).to(:user_detail).with_arguments(:args).allow_nil } + + it { is_expected.to delegate_method(:twitter).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:twitter=).to(:user_detail).with_arguments(:args).allow_nil } + + it { is_expected.to delegate_method(:skype).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:skype=).to(:user_detail).with_arguments(:args).allow_nil } + + it { is_expected.to delegate_method(:website_url).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:website_url=).to(:user_detail).with_arguments(:args).allow_nil } + + it { is_expected.to delegate_method(:location).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:location=).to(:user_detail).with_arguments(:args).allow_nil } + + it { is_expected.to delegate_method(:organization).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:organization=).to(:user_detail).with_arguments(:args).allow_nil } end describe 'associations' do @@ -148,6 +166,11 @@ RSpec.describe User do it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout') } it { is_expected.to have_many(:project_callouts).class_name('Users::ProjectCallout') } it { is_expected.to have_many(:created_projects).dependent(:nullify).class_name('Project') } + it { is_expected.to have_many(:user_achievements).class_name('Achievements::UserAchievement').inverse_of(:user) } + it { is_expected.to have_many(:awarded_user_achievements).class_name('Achievements::UserAchievement').with_foreign_key('awarded_by_user_id').inverse_of(:awarded_by_user) } + it { is_expected.to have_many(:revoked_user_achievements).class_name('Achievements::UserAchievement').with_foreign_key('revoked_by_user_id').inverse_of(:revoked_by_user) } + it { is_expected.to have_many(:achievements).through(:user_achievements).class_name('Achievements::Achievement').inverse_of(:users) } + it { is_expected.to have_many(:namespace_commit_emails).class_name('Users::NamespaceCommitEmail') } describe 'default values' do let(:user) { described_class.new } @@ -160,7 +183,7 @@ RSpec.describe User do it { expect(user.hide_no_password).to be_falsey } it { expect(user.project_view).to eq('files') } it { expect(user.notified_of_own_activity).to be_falsey } - it { expect(user.preferred_language).to eq(I18n.default_locale.to_s) } + it { expect(user.preferred_language).to eq(Gitlab::CurrentSettings.default_preferred_language) } it { expect(user.theme_id).to eq(described_class.gitlab_config.default_theme) } end @@ -169,17 +192,51 @@ RSpec.describe User do expect(create(:user).user_detail).not_to be_persisted end - it 'creates `user_detail` when `bio` is given' do - user = create(:user, bio: 'my bio') + shared_examples 'delegated field' do |field| + it 'creates `user_detail` when the field is given' do + user = create(:user, field => 'my field') + + expect(user.user_detail).to be_persisted + expect(user.user_detail[field]).to eq('my field') + end + + it 'delegates to `user_detail`' do + user = create(:user, field => 'my field') + + expect(user.public_send(field)).to eq(user.user_detail[field]) + end + + it 'creates `user_detail` when first updated' do + user = create(:user) + + expect { user.update!(field => 'my field') }.to change { user.user_detail.persisted? }.from(false).to(true) + end + end + + it_behaves_like 'delegated field', :bio + it_behaves_like 'delegated field', :linkedin + it_behaves_like 'delegated field', :twitter + it_behaves_like 'delegated field', :skype + it_behaves_like 'delegated field', :location + it_behaves_like 'delegated field', :organization + + it 'creates `user_detail` when `website_url` is given' do + user = create(:user, website_url: 'https://example.com') expect(user.user_detail).to be_persisted - expect(user.user_detail.bio).to eq('my bio') + expect(user.user_detail.website_url).to eq('https://example.com') + end + + it 'delegates `website_url` to `user_detail`' do + user = create(:user, website_url: 'http://example.com') + + expect(user.website_url).to eq(user.user_detail.website_url) end - it 'delegates `bio` to `user_detail`' do - user = create(:user, bio: 'my bio') + it 'creates `user_detail` when `website_url` is first updated' do + user = create(:user) - expect(user.bio).to eq(user.user_detail.bio) + expect { user.update!(website_url: 'https://example.com') }.to change { user.user_detail.persisted? }.from(false).to(true) end it 'delegates `pronouns` to `user_detail`' do @@ -193,30 +250,24 @@ RSpec.describe User do expect(user.pronunciation).to eq(user.user_detail.pronunciation) end - - it 'creates `user_detail` when `bio` is first updated' do - user = create(:user) - - expect { user.update!(bio: 'my bio') }.to change { user.user_detail.persisted? }.from(false).to(true) - end end - describe '#abuse_report' do + describe '#abuse_reports' do let(:current_user) { create(:user) } let(:other_user) { create(:user) } - it { is_expected.to have_one(:abuse_report) } + it { is_expected.to have_many(:abuse_reports) } it 'refers to the abuse report whose user_id is the current user' do abuse_report = create(:abuse_report, reporter: other_user, user: current_user) - expect(current_user.abuse_report).to eq(abuse_report) + expect(current_user.abuse_reports.last).to eq(abuse_report) end it 'does not refer to the abuse report whose reporter_id is the current user' do create(:abuse_report, reporter: current_user, user: other_user) - expect(current_user.abuse_report).to be_nil + expect(current_user.abuse_reports.last).to be_nil end it 'does not update the user_id of an abuse report when the user is updated' do @@ -436,18 +487,25 @@ RSpec.describe User do end describe 'preferred_language' do - context 'when its value is nil in the database' do - let(:user) { build(:user, preferred_language: nil) } + subject(:preferred_language) { user.preferred_language } - it 'falls back to I18n.default_locale when empty in the database' do - expect(user.preferred_language).to eq I18n.default_locale.to_s - end + context 'when preferred_language is set' do + let(:user) { build(:user, preferred_language: 'de_DE') } + + it { is_expected.to eq 'de_DE' } + end + + context 'when preferred_language is nil' do + let(:user) { build(:user) } - it 'falls back to english when I18n.default_locale is not an available language' do - allow(I18n).to receive(:default_locale) { :kl } - default_preferred_language = user.send(:default_preferred_language) + it { is_expected.to eq 'en' } - expect(user.preferred_language).to eq default_preferred_language + context 'when Gitlab::CurrentSettings.default_preferred_language is set' do + before do + allow(::Gitlab::CurrentSettings).to receive(:default_preferred_language).and_return('zh_CN') + end + + it { is_expected.to eq 'zh_CN' } end end end @@ -1230,17 +1288,6 @@ RSpec.describe User do end describe 'before save hook' do - describe '#default_private_profile_to_false' do - let(:user) { create(:user, private_profile: true) } - - it 'converts nil to false' do - user.private_profile = nil - user.save! - - expect(user.private_profile).to eq false - end - end - context 'when saving an external user' do let(:user) { create(:user) } let(:external_user) { create(:user, external: true) } @@ -2675,7 +2722,7 @@ RSpec.describe User do expect(user.can_create_group).to eq(Gitlab::CurrentSettings.can_create_group) expect(user.theme_id).to eq(Gitlab.config.gitlab.default_theme) expect(user.external).to be_falsey - expect(user.private_profile).to eq(false) + expect(user.private_profile).to eq(Gitlab::CurrentSettings.user_defaults_to_private_profile) end end @@ -3672,19 +3719,14 @@ RSpec.describe User do describe '#sanitize_attrs' do let(:user) { build(:user, name: 'test <& user', skype: 'test&user') } - it 'encodes HTML entities in the Skype attribute' do - expect { user.sanitize_attrs }.to change { user.skype }.to('test&user') - end - it 'does not encode HTML entities in the name attribute' do expect { user.sanitize_attrs }.not_to change { user.name } end it 'sanitizes attr from html tags' do - user = create(:user, name: '<a href="//example.com">Test<a>', twitter: '<a href="//evil.com">https://twitter.com<a>') + user = create(:user, name: '<a href="//example.com">Test<a>') expect(user.name).to eq('Test') - expect(user.twitter).to eq('https://twitter.com') end it 'sanitizes attr from js scripts' do @@ -5253,36 +5295,16 @@ RSpec.describe User do describe '#invalidate_issue_cache_counts' do let(:user) { build_stubbed(:user) } - before do - stub_feature_flags(limit_assigned_issues_count: false) - end - it 'invalidates cache for issue counter' do cache_mock = double expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_issues_count']) + expect(cache_mock).to receive(:delete).with(['users', user.id, 'max_assigned_open_issues_count']) allow(Rails).to receive(:cache).and_return(cache_mock) user.invalidate_issue_cache_counts end - - context 'when limit_assigned_issues_count is enabled' do - before do - stub_feature_flags(limit_assigned_issues_count: true) - end - - it 'invalidates cache for issue counter' do - cache_mock = double - - expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_issues_count']) - expect(cache_mock).to receive(:delete).with(['users', user.id, 'max_assigned_open_issues_count']) - - allow(Rails).to receive(:cache).and_return(cache_mock) - - user.invalidate_issue_cache_counts - end - end end describe '#invalidate_merge_request_cache_counts' do @@ -5506,41 +5528,6 @@ RSpec.describe User do end end - describe '#ensure_user_detail_assigned' do - let(:user) { build(:user) } - - context 'when no user detail field has been changed' do - before do - allow(UserDetail) - .to receive(:user_fields_changed?) - .and_return(false) - end - - it 'does not assign user details before save' do - expect(user.user_detail) - .not_to receive(:assign_changed_fields_from_user) - - user.save! - end - end - - context 'when a user detail field has been changed' do - before do - allow(UserDetail) - .to receive(:user_fields_changed?) - .and_return(true) - end - - it 'assigns user details before save' do - expect(user.user_detail) - .to receive(:assign_changed_fields_from_user) - .and_call_original - - user.save! - end - end - end - describe '#username_changed_hook' do context 'for a new user' do let(:user) { build(:user) } @@ -7429,4 +7416,84 @@ RSpec.describe User do end end end + + describe '#namespace_commit_email_for_project' do + let_it_be(:user) { create(:user) } + + let(:emails) { user.namespace_commit_email_for_project(project) } + + context 'when project is nil' do + let(:project) {} + + it 'returns nil' do + expect(emails).to be(nil) + end + end + + context 'with a group project' do + let_it_be(:root_group) { create(:group) } + let_it_be(:group) { create(:group, parent: root_group) } + let_it_be(:project) { create(:project, group: group) } + + context 'without a defined root group namespace_commit_email' do + context 'without a defined project namespace_commit_email' do + it 'returns nil' do + expect(emails).to be(nil) + end + end + + context 'with a defined project namespace_commit_email' do + it 'returns the defined namespace_commit_email' do + project_commit_email = create(:namespace_commit_email, + user: user, + namespace: project.project_namespace) + + expect(emails).to eq(project_commit_email) + end + end + end + + context 'with a defined root group namespace_commit_email' do + let_it_be(:root_group_commit_email) do + create(:namespace_commit_email, user: user, namespace: root_group) + end + + context 'without a defined project namespace_commit_email' do + it 'returns the defined namespace_commit_email' do + expect(emails).to eq(root_group_commit_email) + end + end + + context 'with a defined project namespace_commit_email' do + it 'returns the defined namespace_commit_email' do + project_commit_email = create(:namespace_commit_email, + user: user, + namespace: project.project_namespace) + + expect(emails).to eq(project_commit_email) + end + end + end + end + + context 'with personal project' do + let_it_be(:project) { create(:project, namespace: user.namespace) } + + context 'without a defined project namespace_commit_email' do + it 'returns nil' do + expect(emails).to be(nil) + end + end + + context 'with a defined project namespace_commit_email' do + it 'returns the defined namespace_commit_email' do + project_commit_email = create(:namespace_commit_email, + user: user, + namespace: project.project_namespace) + + expect(emails).to eq(project_commit_email) + end + end + end + end end diff --git a/spec/models/users/namespace_commit_email_spec.rb b/spec/models/users/namespace_commit_email_spec.rb index 696dac25f9b..23fed85ab3e 100644 --- a/spec/models/users/namespace_commit_email_spec.rb +++ b/spec/models/users/namespace_commit_email_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Users::NamespaceCommitEmail, type: :model do +RSpec.describe Users::NamespaceCommitEmail, type: :model, feature_category: :source_code_management do subject { build(:namespace_commit_email) } describe 'associations' do @@ -15,7 +15,39 @@ RSpec.describe Users::NamespaceCommitEmail, type: :model do it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:namespace) } it { is_expected.to validate_presence_of(:email) } + + it { is_expected.to validate_uniqueness_of(:user).scoped_to(:namespace_id) } + + describe 'validate_root_group' do + let_it_be(:root_group) { create(:group) } + + context 'when root group' do + subject { build(:namespace_commit_email, namespace: root_group) } + + it { is_expected.to be_valid } + end + + context 'when subgroup' do + subject { build(:namespace_commit_email, namespace: create(:group, parent: root_group)) } + + it 'is invalid and reports the relevant error' do + expect(subject).to be_invalid + expect(subject.errors[:namespace]).to include('must be a root group.') + end + end + end end it { is_expected.to be_valid } + + describe '.delete_for_namespace' do + let_it_be(:group) { create(:group) } + + it 'deletes all records for namespace' do + create_list(:namespace_commit_email, 3, namespace: group) + create(:namespace_commit_email) + + expect { described_class.delete_for_namespace(group) }.to change { described_class.count }.by(-3) + end + end end diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index 1c34936c5c2..0bedcc9791f 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -21,6 +21,13 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do .with_foreign_key('work_item_id') end + it 'has many `work_item_children_by_created_at`' do + is_expected.to have_many(:work_item_children_by_created_at) + .order(created_at: :asc) + .class_name('WorkItem') + .with_foreign_key('work_item_id') + end + it 'has many `child_links`' do is_expected.to have_many(:child_links) .class_name('::WorkItems::ParentLink') diff --git a/spec/models/work_items/parent_link_spec.rb b/spec/models/work_items/parent_link_spec.rb index 82e79e8fbdf..f1aa81f46d2 100644 --- a/spec/models/work_items/parent_link_spec.rb +++ b/spec/models/work_items/parent_link_spec.rb @@ -218,4 +218,16 @@ RSpec.describe WorkItems::ParentLink, feature_category: :portfolio_management do end end end + + context 'with relative positioning' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:work_item_parent) { create(:work_item, project: project) } + + it_behaves_like "a class that supports relative positioning" do + let(:factory) { :parent_link } + let(:default_params) { { work_item_parent: work_item_parent } } + let(:items_with_nil_position_sample_quantity) { 90 } + end + end end diff --git a/spec/models/work_items/widgets/hierarchy_spec.rb b/spec/models/work_items/widgets/hierarchy_spec.rb index c847f2694fe..43670b30645 100644 --- a/spec/models/work_items/widgets/hierarchy_spec.rb +++ b/spec/models/work_items/widgets/hierarchy_spec.rb @@ -2,11 +2,11 @@ require 'spec_helper' -RSpec.describe WorkItems::Widgets::Hierarchy do +RSpec.describe WorkItems::Widgets::Hierarchy, feature_category: :team_planning do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } let_it_be(:task) { create(:work_item, :task, project: project) } - let_it_be(:work_item_parent) { create(:work_item, project: project) } + let_it_be_with_reload(:work_item_parent) { create(:work_item, project: project) } describe '.type' do subject { described_class.type } @@ -21,7 +21,7 @@ RSpec.describe WorkItems::Widgets::Hierarchy do end describe '#parent' do - let_it_be(:parent_link) { create(:parent_link, work_item: task, work_item_parent: work_item_parent).reload } + let_it_be_with_reload(:parent_link) { create(:parent_link, work_item: task, work_item_parent: work_item_parent) } subject { described_class.new(parent_link.work_item).parent } @@ -29,11 +29,21 @@ RSpec.describe WorkItems::Widgets::Hierarchy do end describe '#children' do - let_it_be(:parent_link1) { create(:parent_link, work_item_parent: work_item_parent, work_item: task).reload } - let_it_be(:parent_link2) { create(:parent_link, work_item_parent: work_item_parent).reload } + let_it_be_with_reload(:parent_link1) { create(:parent_link, work_item_parent: work_item_parent, work_item: task) } + let_it_be_with_reload(:parent_link2) { create(:parent_link, work_item_parent: work_item_parent) } subject { described_class.new(work_item_parent).children } it { is_expected.to contain_exactly(parent_link1.work_item, parent_link2.work_item) } + + context 'with default order by created_at' do + let_it_be(:oldest_child) { create(:work_item, :task, project: project, created_at: 5.minutes.ago) } + + let_it_be_with_reload(:link_to_oldest_child) do + create(:parent_link, work_item_parent: work_item_parent, work_item: oldest_child) + end + + it { is_expected.to eq([link_to_oldest_child, parent_link1, parent_link2].map(&:work_item)) } + end end end |