diff options
Diffstat (limited to 'spec/models')
72 files changed, 2892 insertions, 632 deletions
diff --git a/spec/models/alert_management/metric_image_spec.rb b/spec/models/alert_management/metric_image_spec.rb new file mode 100644 index 00000000000..dedbd6e501e --- /dev/null +++ b/spec/models/alert_management/metric_image_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe AlertManagement::MetricImage do + subject { build(:alert_metric_image) } + + describe 'associations' do + it { is_expected.to belong_to(:alert) } + end + + describe 'validations' do + it { is_expected.to be_valid } + it { is_expected.to validate_presence_of(:file) } + it { is_expected.to validate_length_of(:url).is_at_most(255) } + it { is_expected.to validate_length_of(:url_text).is_at_most(128) } + end + + describe '.available_for?' do + subject { described_class.available_for?(issue.project) } + + let_it_be_with_refind(:issue) { create(:issue) } + + it { is_expected.to eq(true) } + end +end diff --git a/spec/models/analytics/cycle_analytics/aggregation_spec.rb b/spec/models/analytics/cycle_analytics/aggregation_spec.rb index 4bf737df56a..6071e4b3d21 100644 --- a/spec/models/analytics/cycle_analytics/aggregation_spec.rb +++ b/spec/models/analytics/cycle_analytics/aggregation_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Analytics::CycleAnalytics::Aggregation, type: :model do it { is_expected.not_to validate_presence_of(:group) } it { is_expected.not_to validate_presence_of(:enabled) } - %i[incremental_runtimes_in_seconds incremental_processed_records last_full_run_runtimes_in_seconds last_full_run_processed_records].each do |column| + %i[incremental_runtimes_in_seconds incremental_processed_records full_runtimes_in_seconds full_processed_records].each do |column| it "validates the array length of #{column}" do record = described_class.new(column => [1] * 11) @@ -20,6 +20,81 @@ RSpec.describe Analytics::CycleAnalytics::Aggregation, type: :model do end end + describe 'attribute updater methods' do + subject(:aggregation) { build(:cycle_analytics_aggregation) } + + describe '#cursor_for' do + it 'returns empty cursors' do + aggregation.last_full_issues_id = nil + aggregation.last_full_issues_updated_at = nil + + expect(aggregation.cursor_for(:full, Issue)).to eq({}) + end + + context 'when cursor is not empty' do + it 'returns the cursor values' do + current_time = Time.current + + aggregation.last_full_issues_id = 1111 + aggregation.last_full_issues_updated_at = current_time + + expect(aggregation.cursor_for(:full, Issue)).to eq({ id: 1111, updated_at: current_time }) + end + end + end + + describe '#refresh_last_run' do + it 'updates the run_at column' do + freeze_time do + aggregation.refresh_last_run(:incremental) + + expect(aggregation.last_incremental_run_at).to eq(Time.current) + end + end + end + + describe '#reset_full_run_cursors' do + it 'resets all full run cursors to nil' do + aggregation.last_full_issues_id = 111 + aggregation.last_full_issues_updated_at = Time.current + aggregation.last_full_merge_requests_id = 111 + aggregation.last_full_merge_requests_updated_at = Time.current + + aggregation.reset_full_run_cursors + + expect(aggregation).to have_attributes( + last_full_issues_id: nil, + last_full_issues_updated_at: nil, + last_full_merge_requests_id: nil, + last_full_merge_requests_updated_at: nil + ) + end + end + + describe '#set_cursor' do + it 'sets the cursor values for the given mode' do + aggregation.set_cursor(:full, Issue, { id: 2222, updated_at: nil }) + + expect(aggregation).to have_attributes( + last_full_issues_id: 2222, + last_full_issues_updated_at: nil + ) + end + end + + describe '#set_stats' do + it 'appends stats to the runtime and processed_records attributes' do + aggregation.set_stats(:full, 10, 20) + aggregation.set_stats(:full, 20, 30) + + expect(aggregation).to have_attributes( + full_runtimes_in_seconds: [10, 20], + full_processed_records: [20, 30] + ) + end + end + end + describe '#safe_create_for_group' do let_it_be(:group) { create(:group) } let_it_be(:subgroup) { create(:group, parent: group) } diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 70331e8d78a..541fa1ac77a 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -512,12 +512,8 @@ RSpec.describe ApplicationSetting do end context 'key restrictions' do - it 'supports all key types' do - expect(described_class::SUPPORTED_KEY_TYPES).to eq(Gitlab::SSHPublicKey.supported_types) - end - it 'does not allow all key types to be disabled' do - described_class::SUPPORTED_KEY_TYPES.each do |type| + Gitlab::SSHPublicKey.supported_types.each do |type| setting["#{type}_key_restriction"] = described_class::FORBIDDEN_KEY_VALUE end @@ -526,15 +522,23 @@ RSpec.describe ApplicationSetting do end where(:type) do - described_class::SUPPORTED_KEY_TYPES + Gitlab::SSHPublicKey.supported_types end with_them do let(:field) { :"#{type}_key_restriction" } - it { is_expected.to validate_presence_of(field) } - it { is_expected.to allow_value(*KeyRestrictionValidator.supported_key_restrictions(type)).for(field) } - it { is_expected.not_to allow_value(128).for(field) } + shared_examples 'key validations' do + it { is_expected.to validate_presence_of(field) } + it { is_expected.to allow_value(*KeyRestrictionValidator.supported_key_restrictions(type)).for(field) } + it { is_expected.not_to allow_value(128).for(field) } + end + + it_behaves_like 'key validations' + + context 'FIPS mode', :fips_mode do + it_behaves_like 'key validations' + end end end @@ -1306,4 +1310,31 @@ RSpec.describe ApplicationSetting do end end end + + describe '#database_grafana_api_key' do + it 'is encrypted' do + subject.database_grafana_api_key = 'somesecret' + + aggregate_failures do + expect(subject.encrypted_database_grafana_api_key).to be_present + expect(subject.encrypted_database_grafana_api_key_iv).to be_present + expect(subject.encrypted_database_grafana_api_key).not_to eq(subject.database_grafana_api_key) + end + end + end + + context "inactive project deletion" do + it "validates that inactive_projects_send_warning_email_after_months is less than inactive_projects_delete_after_months" do + subject[:inactive_projects_delete_after_months] = 3 + subject[:inactive_projects_send_warning_email_after_months] = 6 + + expect(subject).to be_invalid + end + + it { is_expected.to validate_numericality_of(:inactive_projects_send_warning_email_after_months).is_greater_than(0) } + + it { is_expected.to validate_numericality_of(:inactive_projects_delete_after_months).is_greater_than(0) } + + it { is_expected.to validate_numericality_of(:inactive_projects_min_size_mb).is_greater_than_or_equal_to(0) } + end end diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index ebd1441f901..4da19267b1c 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -58,6 +58,43 @@ RSpec.describe AwardEmoji do end end end + + context 'custom emoji' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:emoji) { create(:custom_emoji, name: 'partyparrot', namespace: group) } + + before do + group.add_maintainer(user) + end + + %i[issue merge_request note_on_issue snippet].each do |awardable_type| + let_it_be(:project) { create(:project, namespace: group) } + let(:awardable) { create(awardable_type, project: project) } + + it "is accepted on #{awardable_type}" do + new_award = build(:award_emoji, user: user, awardable: awardable, name: emoji.name) + + expect(new_award).to be_valid + end + end + + it 'is accepted on subgroup issue' do + subgroup = create(:group, parent: group) + project = create(:project, namespace: subgroup) + issue = create(:issue, project: project) + new_award = build(:award_emoji, user: user, awardable: issue, name: emoji.name) + + expect(new_award).to be_valid + end + + it 'is not supported on personal snippet (yet)' do + snippet = create(:personal_snippet) + new_award = build(:award_emoji, user: snippet.author, awardable: snippet, name: 'null') + + expect(new_award).not_to be_valid + end + end end describe 'scopes' do @@ -210,4 +247,47 @@ RSpec.describe AwardEmoji do end end end + + describe '#url' do + let_it_be(:custom_emoji) { create(:custom_emoji) } + let_it_be(:project) { create(:project, namespace: custom_emoji.group) } + let_it_be(:issue) { create(:issue, project: project) } + + def build_award(name) + build(:award_emoji, awardable: issue, name: name) + end + + it 'is nil for built-in emoji' do + new_award = build_award('tada') + + count = ActiveRecord::QueryRecorder.new do + expect(new_award.url).to be_nil + end.count + expect(count).to be_zero + end + + it 'is nil for unrecognized emoji' do + new_award = build_award('null') + + expect(new_award.url).to be_nil + end + + it 'is set for custom emoji' do + new_award = build_award(custom_emoji.name) + + expect(new_award.url).to eq(custom_emoji.url) + end + + context 'feature flag disabled' do + before do + stub_feature_flags(custom_emoji: false) + end + + it 'does not query' do + new_award = build_award(custom_emoji.name) + + expect(ActiveRecord::QueryRecorder.new { new_award.url }.count).to be_zero + end + end + end end diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 6eba9ca63b0..9c153f36d8b 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -229,6 +229,20 @@ RSpec.describe Blob do end end + describe '#executable?' do + it 'is true for executables' do + executable_blob = fake_blob(path: 'file', mode: '100755') + + expect(executable_blob.executable?).to eq true + end + + it 'is false for non-executables' do + non_executable_blob = fake_blob(path: 'file', mode: '100655') + + expect(non_executable_blob.executable?).to eq false + end + end + describe '#extension' do it 'returns the extension' do blob = fake_blob(path: 'file.md') diff --git a/spec/models/board_spec.rb b/spec/models/board_spec.rb index 90a06b80f9c..775cccd2aec 100644 --- a/spec/models/board_spec.rb +++ b/spec/models/board_spec.rb @@ -21,10 +21,12 @@ RSpec.describe Board do end describe '#order_by_name_asc' do + # rubocop:disable RSpec/VariableName let!(:board_B) { create(:board, project: project, name: 'B') } let!(:board_C) { create(:board, project: project, name: 'C') } let!(:board_a) { create(:board, project: project, name: 'a') } let!(:board_A) { create(:board, project: project, name: 'A') } + # rubocop:enable RSpec/VariableName it 'returns in case-insensitive alphabetical order and then by ascending id' do expect(project.boards.order_by_name_asc).to eq [board_a, board_A, board_B, board_C] @@ -32,10 +34,12 @@ RSpec.describe Board do end describe '#first_board' do + # rubocop:disable RSpec/VariableName let!(:board_B) { create(:board, project: project, name: 'B') } let!(:board_C) { create(:board, project: project, name: 'C') } let!(:board_a) { create(:board, project: project, name: 'a') } let!(:board_A) { create(:board, project: project, name: 'A') } + # rubocop:enable RSpec/VariableName it 'return the first case-insensitive alphabetical board as a relation' do expect(project.boards.first_board).to eq [board_a] diff --git a/spec/models/bulk_import_spec.rb b/spec/models/bulk_import_spec.rb index ea002a7b174..3430da43f62 100644 --- a/spec/models/bulk_import_spec.rb +++ b/spec/models/bulk_import_spec.rb @@ -3,6 +3,13 @@ require 'spec_helper' RSpec.describe BulkImport, type: :model do + let_it_be(:created_bulk_import) { create(:bulk_import, :created) } + let_it_be(:started_bulk_import) { create(:bulk_import, :started) } + let_it_be(:finished_bulk_import) { create(:bulk_import, :finished) } + let_it_be(:failed_bulk_import) { create(:bulk_import, :failed) } + let_it_be(:stale_created_bulk_import) { create(:bulk_import, :created, created_at: 3.days.ago) } + let_it_be(:stale_started_bulk_import) { create(:bulk_import, :started, created_at: 3.days.ago) } + describe 'associations' do it { is_expected.to belong_to(:user).required } it { is_expected.to have_one(:configuration) } @@ -16,9 +23,15 @@ RSpec.describe BulkImport, type: :model do it { is_expected.to define_enum_for(:source_type).with_values(%i[gitlab]) } end + describe '.stale scope' do + subject { described_class.stale } + + it { is_expected.to contain_exactly(stale_created_bulk_import, stale_started_bulk_import) } + end + describe '.all_human_statuses' do it 'returns all human readable entity statuses' do - expect(described_class.all_human_statuses).to contain_exactly('created', 'started', 'finished', 'failed') + expect(described_class.all_human_statuses).to contain_exactly('created', 'started', 'finished', 'failed', 'timeout') end end diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index e5bbac62dcc..6f6a7c9bcd8 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -151,7 +151,7 @@ RSpec.describe BulkImports::Entity, type: :model do describe '.all_human_statuses' do it 'returns all human readable entity statuses' do - expect(described_class.all_human_statuses).to contain_exactly('created', 'started', 'finished', 'failed') + expect(described_class.all_human_statuses).to contain_exactly('created', 'started', 'finished', 'failed', 'timeout') end end @@ -179,7 +179,7 @@ RSpec.describe BulkImports::Entity, type: :model do entity = create(:bulk_import_entity, :group_entity) entity.create_pipeline_trackers! - expect(entity.trackers.count).to eq(BulkImports::Groups::Stage.new(entity.bulk_import).pipelines.count) + expect(entity.trackers.count).to eq(BulkImports::Groups::Stage.new(entity).pipelines.count) expect(entity.trackers.map(&:pipeline_name)).to include(BulkImports::Groups::Pipelines::GroupPipeline.to_s) end end @@ -189,7 +189,7 @@ RSpec.describe BulkImports::Entity, type: :model do entity = create(:bulk_import_entity, :project_entity) entity.create_pipeline_trackers! - expect(entity.trackers.count).to eq(BulkImports::Projects::Stage.new(entity.bulk_import).pipelines.count) + expect(entity.trackers.count).to eq(BulkImports::Projects::Stage.new(entity).pipelines.count) expect(entity.trackers.map(&:pipeline_name)).to include(BulkImports::Projects::Pipelines::ProjectPipeline.to_s) end end diff --git a/spec/models/bulk_imports/export_status_spec.rb b/spec/models/bulk_imports/export_status_spec.rb index f945ad12244..79ed6b39358 100644 --- a/spec/models/bulk_imports/export_status_spec.rb +++ b/spec/models/bulk_imports/export_status_spec.rb @@ -13,6 +13,10 @@ RSpec.describe BulkImports::ExportStatus do double(parsed_response: [{ 'relation' => 'labels', 'status' => status, 'error' => 'error!' }]) end + let(:invalid_response_double) do + double(parsed_response: [{ 'relation' => 'not_a_real_relation', 'status' => status, 'error' => 'error!' }]) + end + subject { described_class.new(tracker, relation) } before do @@ -36,6 +40,18 @@ RSpec.describe BulkImports::ExportStatus do it 'returns false' do expect(subject.started?).to eq(false) end + + context 'when returned relation is invalid' do + before do + allow_next_instance_of(BulkImports::Clients::HTTP) do |client| + allow(client).to receive(:get).and_return(invalid_response_double) + end + end + + it 'returns false' do + expect(subject.started?).to eq(false) + end + end end end @@ -63,7 +79,7 @@ RSpec.describe BulkImports::ExportStatus do it 'returns true' do expect(subject.failed?).to eq(true) - expect(subject.error).to eq('Empty export status response') + expect(subject.error).to eq('Empty relation export status') end end end diff --git a/spec/models/bulk_imports/tracker_spec.rb b/spec/models/bulk_imports/tracker_spec.rb index a72b628e329..0b6f692a477 100644 --- a/spec/models/bulk_imports/tracker_spec.rb +++ b/spec/models/bulk_imports/tracker_spec.rb @@ -66,8 +66,8 @@ RSpec.describe BulkImports::Tracker, type: :model do describe '#pipeline_class' do it 'returns the pipeline class' do - bulk_import = create(:bulk_import) - pipeline_class = BulkImports::Groups::Stage.new(bulk_import).pipelines.first[1] + entity = create(:bulk_import_entity) + pipeline_class = BulkImports::Groups::Stage.new(entity).pipelines.first[1] tracker = create(:bulk_import_tracker, pipeline_name: pipeline_class) expect(tracker.pipeline_class).to eq(pipeline_class) diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 7c3c02a5ab7..5ee560c4925 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -30,6 +30,12 @@ RSpec.describe Ci::Bridge do expect(bridge).to have_one(:downstream_pipeline) end + describe '#retryable?' do + it 'returns false' do + expect(bridge.retryable?).to eq(false) + end + end + describe '#tags' do it 'only has a bridge tag' do expect(bridge.tags).to eq [:bridge] @@ -282,6 +288,26 @@ RSpec.describe Ci::Bridge do ) end end + + context 'when the pipeline runs from a pipeline schedule' do + let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) } + let(:pipeline) { create(:ci_pipeline, pipeline_schedule: pipeline_schedule) } + + let(:options) do + { trigger: { project: 'my/project', forward: { pipeline_variables: true } } } + end + + before do + pipeline_schedule.variables.create!(key: 'schedule_var_key', value: 'schedule var value') + end + + it 'adds the schedule variable' do + expect(bridge.downstream_variables).to contain_exactly( + { key: 'BRIDGE', value: 'cross' }, + { key: 'schedule_var_key', value: 'schedule var value' } + ) + end + end end end diff --git a/spec/models/ci/build_dependencies_spec.rb b/spec/models/ci/build_dependencies_spec.rb index cd330324840..91048cae064 100644 --- a/spec/models/ci/build_dependencies_spec.rb +++ b/spec/models/ci/build_dependencies_spec.rb @@ -14,7 +14,7 @@ RSpec.describe Ci::BuildDependencies do end let!(:build) { create(:ci_build, pipeline: pipeline, name: 'build', stage_idx: 0, stage: 'build') } - let!(:rspec_test) { create(:ci_build, pipeline: pipeline, name: 'rspec', stage_idx: 1, stage: 'test') } + let!(:rspec_test) { create(:ci_build, :success, pipeline: pipeline, name: 'rspec', stage_idx: 1, stage: 'test') } let!(:rubocop_test) { create(:ci_build, pipeline: pipeline, name: 'rubocop', stage_idx: 1, stage: 'test') } let!(:staging) { create(:ci_build, pipeline: pipeline, name: 'staging', stage_idx: 2, stage: 'deploy') } @@ -48,7 +48,7 @@ RSpec.describe Ci::BuildDependencies do project.add_developer(user) end - let!(:retried_job) { Ci::Build.retry(rspec_test, user) } + let!(:retried_job) { Ci::RetryJobService.new(rspec_test.project, user).execute(rspec_test)[:job] } it 'contains the retried job instead of the original one' do is_expected.to contain_exactly(build, retried_job, rubocop_test) diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 240b932638a..fd87a388442 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1585,6 +1585,31 @@ RSpec.describe Ci::Build do it { is_expected.to eq('review/x') } end + + context 'when environment name uses a nested variable' do + let(:yaml_variables) do + [ + { key: 'ENVIRONMENT_NAME', value: '${CI_COMMIT_REF_NAME}' } + ] + end + + let(:build) do + create(:ci_build, + ref: 'master', + yaml_variables: yaml_variables, + environment: 'review/$ENVIRONMENT_NAME') + end + + it { is_expected.to eq('review/master') } + + context 'when the FF ci_expand_environment_name_and_url is disabled' do + before do + stub_feature_flags(ci_expand_environment_name_and_url: false) + end + + it { is_expected.to eq('review/${CI_COMMIT_REF_NAME}') } + end + end end describe '#expanded_kubernetes_namespace' do @@ -1951,90 +1976,6 @@ RSpec.describe Ci::Build do end end - describe '#retryable?' do - subject { build } - - context 'when build is retryable' do - context 'when build is successful' do - before do - build.success! - end - - it { is_expected.to be_retryable } - end - - context 'when build is failed' do - before do - build.drop! - end - - it { is_expected.to be_retryable } - end - - context 'when build is canceled' do - before do - build.cancel! - end - - it { is_expected.to be_retryable } - end - end - - context 'when build is not retryable' do - context 'when build is running' do - before do - build.run! - end - - it { is_expected.not_to be_retryable } - end - - context 'when build is skipped' do - before do - build.skip! - end - - it { is_expected.not_to be_retryable } - end - - context 'when build is degenerated' do - before do - build.degenerate! - end - - it { is_expected.not_to be_retryable } - end - - context 'when a canceled build has been retried already' do - before do - project.add_developer(user) - build.cancel! - described_class.retry(build, user) - end - - it { is_expected.not_to be_retryable } - end - - context 'when deployment is rejected' do - before do - build.drop!(:deployment_rejected) - end - - it { is_expected.not_to be_retryable } - end - - context 'when build is waiting for deployment approval' do - subject { build_stubbed(:ci_build, :manual, environment: 'production') } - - before do - create(:deployment, :blocked, deployable: subject) - end - - it { is_expected.not_to be_retryable } - end - end - end - describe '#action?' do before do build.update!(when: value) @@ -2308,7 +2249,7 @@ RSpec.describe Ci::Build do describe '#options' do let(:options) do { - image: "ruby:2.7", + image: "image:1.0", services: ["postgres"], script: ["ls -a"] } @@ -2319,7 +2260,7 @@ RSpec.describe Ci::Build do end it 'allows to access with symbolized keys' do - expect(build.options[:image]).to eq('ruby:2.7') + expect(build.options[:image]).to eq('image:1.0') end it 'rejects access with string keys' do @@ -2358,24 +2299,12 @@ RSpec.describe Ci::Build do end context 'when build is retried' do - let!(:new_build) { described_class.retry(build, user) } + let!(:new_build) { Ci::RetryJobService.new(project, user).execute(build)[:job] } it 'does not return any of them' do is_expected.not_to include(build, new_build) end end - - context 'when other build is retried' do - let!(:retried_build) { described_class.retry(other_build, user) } - - before do - retried_build.success - end - - it 'returns a retried build' do - is_expected.to contain_exactly(retried_build) - end - end end describe '#other_scheduled_actions' do @@ -3962,8 +3891,9 @@ RSpec.describe Ci::Build do subject { create(:ci_build, :running, options: { script: ["ls -al"], retry: 3 }, project: project, user: user) } it 'retries build and assigns the same user to it' do - expect(described_class).to receive(:retry) - .with(subject, user) + expect_next_instance_of(::Ci::RetryJobService) do |service| + expect(service).to receive(:execute).with(subject) + end subject.drop! end @@ -3977,10 +3907,10 @@ RSpec.describe Ci::Build do end context 'when retry service raises Gitlab::Access::AccessDeniedError exception' do - let(:retry_service) { Ci::RetryBuildService.new(subject.project, subject.user) } + let(:retry_service) { Ci::RetryJobService.new(subject.project, subject.user) } before do - allow_any_instance_of(Ci::RetryBuildService) + allow_any_instance_of(Ci::RetryJobService) .to receive(:execute) .with(subject) .and_raise(Gitlab::Access::AccessDeniedError) diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index bd0397e0396..24c318d0218 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -279,6 +279,15 @@ RSpec.describe Ci::JobArtifact do end end + describe '.order_expired_asc' do + let_it_be(:first_artifact) { create(:ci_job_artifact, expire_at: 2.days.ago) } + let_it_be(:second_artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) } + + it 'returns ordered artifacts' do + expect(described_class.order_expired_asc).to eq([first_artifact, second_artifact]) + end + end + describe '.for_project' do it 'returns artifacts only for given project(s)', :aggregate_failures do artifact1 = create(:ci_job_artifact) @@ -700,10 +709,6 @@ RSpec.describe Ci::JobArtifact do MSG end - it_behaves_like 'it has loose foreign keys' do - let(:factory_name) { :ci_job_artifact } - end - context 'loose foreign key on ci_job_artifacts.project_id' do it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:project) } diff --git a/spec/models/ci/namespace_mirror_spec.rb b/spec/models/ci/namespace_mirror_spec.rb index 38471f15849..9b4e86916b8 100644 --- a/spec/models/ci/namespace_mirror_spec.rb +++ b/spec/models/ci/namespace_mirror_spec.rb @@ -44,6 +44,53 @@ RSpec.describe Ci::NamespaceMirror do end end + describe '.contains_traversal_ids' do + let!(:other_group1) { create(:group) } + let!(:other_group2) { create(:group, parent: other_group1) } + let!(:other_group3) { create(:group, parent: other_group2) } + let!(:other_group4) { create(:group) } + + subject(:result) { described_class.contains_traversal_ids(all_traversal_ids) } + + context 'when passing a top-level group' do + let(:all_traversal_ids) do + [ + [other_group1.id] + ] + end + + it 'returns only itself and children of that group' do + expect(result.map(&:namespace)).to contain_exactly(other_group1, other_group2, other_group3) + end + end + + context 'when passing many levels of groups' do + let(:all_traversal_ids) do + [ + [other_group2.parent_id, other_group2.id], + [other_group3.parent_id, other_group3.id], + [other_group4.id] + ] + end + + it 'returns only the asked group' do + expect(result.map(&:namespace)).to contain_exactly(other_group2, other_group3, other_group4) + end + end + + context 'when passing invalid data ' do + let(:all_traversal_ids) do + [ + ["; UPDATE"] + ] + end + + it 'data is properly sanitised' do + expect(result.to_sql).to include "((traversal_ids[1])) IN (('; UPDATE'))" + end + end + end + describe '.by_namespace_id' do subject(:result) { described_class.by_namespace_id(group2.id) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 294ec07ee3e..45b51d5bf44 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1146,6 +1146,50 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end end + + describe 'variable CI_GITLAB_FIPS_MODE' do + context 'when FIPS flag is enabled' do + before do + allow(Gitlab::FIPS).to receive(:enabled?).and_return(true) + end + + it "is included with value 'true'" do + expect(subject.to_hash).to include('CI_GITLAB_FIPS_MODE' => 'true') + end + end + + context 'when FIPS flag is disabled' do + before do + allow(Gitlab::FIPS).to receive(:enabled?).and_return(false) + end + + it 'is not included' do + expect(subject.to_hash).not_to have_key('CI_GITLAB_FIPS_MODE') + end + end + end + + context 'without a commit' do + let(:pipeline) { build(:ci_empty_pipeline, :created, sha: nil) } + + it 'does not expose commit variables' do + expect(subject.to_hash.keys) + .not_to include( + 'CI_COMMIT_SHA', + 'CI_COMMIT_SHORT_SHA', + 'CI_COMMIT_BEFORE_SHA', + 'CI_COMMIT_REF_NAME', + 'CI_COMMIT_REF_SLUG', + 'CI_COMMIT_BRANCH', + 'CI_COMMIT_TAG', + 'CI_COMMIT_MESSAGE', + 'CI_COMMIT_TITLE', + 'CI_COMMIT_DESCRIPTION', + 'CI_COMMIT_REF_PROTECTED', + 'CI_COMMIT_TIMESTAMP', + 'CI_COMMIT_AUTHOR') + end + end end describe '#protected_ref?' do @@ -1663,7 +1707,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do expect(upstream_pipeline.reload).to be_failed Sidekiq::Testing.inline! do - new_job = Ci::Build.retry(job, project.users.first) + new_job = Ci::RetryJobService.new(project, project.users.first).execute(job)[:job] expect(downstream_pipeline.reload).to be_running expect(upstream_pipeline.reload).to be_running @@ -1684,7 +1728,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do expect(upstream_pipeline.reload).to be_success Sidekiq::Testing.inline! do - new_job = Ci::Build.retry(job, project.users.first) + new_job = Ci::RetryJobService.new(project, project.users.first).execute(job)[:job] expect(downstream_pipeline.reload).to be_running expect(upstream_pipeline.reload).to be_running @@ -1715,7 +1759,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do expect(upstream_of_upstream_pipeline.reload).to be_failed Sidekiq::Testing.inline! do - new_job = Ci::Build.retry(job, project.users.first) + new_job = Ci::RetryJobService.new(project, project.users.first).execute(job)[:job] expect(downstream_pipeline.reload).to be_running expect(upstream_pipeline.reload).to be_running @@ -2583,8 +2627,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do build.drop project.add_developer(user) - - Ci::Build.retry(build, user) + ::Ci::RetryJobService.new(project, user).execute(build)[:job] end # We are changing a state: created > failed > running @@ -4688,7 +4731,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do project.add_developer(user) retried_build.cancel! - ::Ci::Build.retry(retried_build, user) + Ci::RetryJobService.new(project, user).execute(retried_build)[:job] end it 'does not include retried builds' do @@ -4714,6 +4757,24 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '#has_expired_test_reports?' do + subject { pipeline_with_test_report.has_expired_test_reports? } + + let(:pipeline_with_test_report) { create(:ci_pipeline, :with_test_reports) } + + context 'when artifacts are not expired' do + it { is_expected.to be_falsey } + end + + context 'when artifacts are expired' do + before do + pipeline_with_test_report.job_artifacts.first.update!(expire_at: Date.yesterday) + end + + it { is_expected.to be_truthy } + end + end + it_behaves_like 'it has loose foreign keys' do let(:factory_name) { :ci_pipeline } end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index ac1a8247aaa..71fef3c1b5b 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -14,6 +14,100 @@ RSpec.describe Ci::Processable do it { is_expected.to delegate_method(:legacy_detached_merge_request_pipeline?).to(:pipeline) } end + describe '#retryable' do + shared_examples_for 'retryable processable' do + context 'when processable is successful' do + before do + processable.success! + end + + it { is_expected.to be_retryable } + end + + context 'when processable is failed' do + before do + processable.drop! + end + + it { is_expected.to be_retryable } + end + + context 'when processable is canceled' do + before do + processable.cancel! + end + + it { is_expected.to be_retryable } + end + end + + shared_examples_for 'non-retryable processable' do + context 'when processable is skipped' do + before do + processable.skip! + end + + it { is_expected.not_to be_retryable } + end + + context 'when processable is degenerated' do + before do + processable.degenerate! + end + + it { is_expected.not_to be_retryable } + end + + context 'when a canceled processable has been retried already' do + before do + project.add_developer(create(:user)) + processable.cancel! + processable.update!(retried: true) + end + + it { is_expected.not_to be_retryable } + end + end + + context 'when the processable is a build' do + subject(:processable) { create(:ci_build, pipeline: pipeline) } + + context 'when the processable is retryable' do + it_behaves_like 'retryable processable' + + context 'when deployment is rejected' do + before do + processable.drop!(:deployment_rejected) + end + + it { is_expected.not_to be_retryable } + end + + context 'when build is waiting for deployment approval' do + subject { build_stubbed(:ci_build, :manual, environment: 'production') } + + before do + create(:deployment, :blocked, deployable: subject) + end + + it { is_expected.not_to be_retryable } + end + end + + context 'when the processable is non-retryable' do + it_behaves_like 'non-retryable processable' + + context 'when processable is running' do + before do + processable.run! + end + + it { is_expected.not_to be_retryable } + end + end + end + end + describe '#aggregated_needs_names' do let(:with_aggregated_needs) { pipeline.processables.select_with_aggregated_needs(project) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 42187c3ef99..05b7bc39a74 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -134,28 +134,28 @@ RSpec.describe Ci::Runner do end context 'cost factors validations' do - it 'dissalows :private_projects_minutes_cost_factor being nil' do + it 'disallows :private_projects_minutes_cost_factor being nil' do runner = build(:ci_runner, private_projects_minutes_cost_factor: nil) expect(runner).to be_invalid expect(runner.errors.full_messages).to include('Private projects minutes cost factor needs to be non-negative') end - it 'dissalows :public_projects_minutes_cost_factor being nil' do + it 'disallows :public_projects_minutes_cost_factor being nil' do runner = build(:ci_runner, public_projects_minutes_cost_factor: nil) expect(runner).to be_invalid expect(runner.errors.full_messages).to include('Public projects minutes cost factor needs to be non-negative') end - it 'dissalows :private_projects_minutes_cost_factor being negative' do + it 'disallows :private_projects_minutes_cost_factor being negative' do runner = build(:ci_runner, private_projects_minutes_cost_factor: -1.1) expect(runner).to be_invalid expect(runner.errors.full_messages).to include('Private projects minutes cost factor needs to be non-negative') end - it 'dissalows :public_projects_minutes_cost_factor being negative' do + it 'disallows :public_projects_minutes_cost_factor being negative' do runner = build(:ci_runner, public_projects_minutes_cost_factor: -2.2) expect(runner).to be_invalid diff --git a/spec/models/ci/secure_file_spec.rb b/spec/models/ci/secure_file_spec.rb index 4382385aaf5..f92db3fe8db 100644 --- a/spec/models/ci/secure_file_spec.rb +++ b/spec/models/ci/secure_file_spec.rb @@ -3,14 +3,14 @@ require 'spec_helper' RSpec.describe Ci::SecureFile do - let(:sample_file) { fixture_file('ci_secure_files/upload-keystore.jks') } - - subject { create(:ci_secure_file) } - before do stub_ci_secure_file_object_storage end + let(:sample_file) { fixture_file('ci_secure_files/upload-keystore.jks') } + + subject { create(:ci_secure_file, file: CarrierWaveStringFile.new(sample_file)) } + it { is_expected.to be_a FileStoreMounter } it { is_expected.to belong_to(:project).required } @@ -27,6 +27,26 @@ RSpec.describe Ci::SecureFile do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:permissions) } it { is_expected.to validate_presence_of(:project_id) } + context 'unique filename' do + let_it_be(:project1) { create(:project) } + + it 'ensures the file name is unique within a given project' do + file1 = create(:ci_secure_file, project: project1, name: 'file1') + expect do + create(:ci_secure_file, project: project1, name: 'file1') + end.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Name has already been taken') + + expect(project1.secure_files.where(name: 'file1').count).to be 1 + expect(project1.secure_files.find_by(name: 'file1').id).to eq(file1.id) + end + + it 'allows duplicate file names in different projects' do + create(:ci_secure_file, project: project1) + expect do + create(:ci_secure_file, project: create(:project)) + end.not_to raise_error + end + end end describe '#permissions' do @@ -37,8 +57,6 @@ RSpec.describe Ci::SecureFile do describe '#checksum' do it 'computes SHA256 checksum on the file before encrypted' do - subject.file = CarrierWaveStringFile.new(sample_file) - subject.save! expect(subject.checksum).to eq(Digest::SHA256.hexdigest(sample_file)) end end @@ -51,8 +69,6 @@ RSpec.describe Ci::SecureFile do describe '#file' do it 'returns the saved file' do - subject.file = CarrierWaveStringFile.new(sample_file) - subject.save! expect(Base64.encode64(subject.file.read)).to eq(Base64.encode64(sample_file)) end end diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb index f279e779de5..f10e0cc8fa7 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -117,6 +117,23 @@ RSpec.describe Clusters::Agent do end end + describe '#last_used_agent_tokens' do + let_it_be(:agent) { create(:cluster_agent) } + + subject { agent.last_used_agent_tokens } + + context 'agent has no tokens' do + it { is_expected.to be_empty } + end + + context 'agent has active and inactive tokens' do + let!(:active_token) { create(:cluster_agent_token, agent: agent, last_used_at: 1.minute.ago) } + let!(:inactive_token) { create(:cluster_agent_token, agent: agent, last_used_at: 2.hours.ago) } + + it { is_expected.to contain_exactly(active_token, inactive_token) } + end + end + describe '#activity_event_deletion_cutoff' do let_it_be(:agent) { create(:cluster_agent) } let_it_be(:event1) { create(:agent_activity_event, agent: agent, recorded_at: 1.hour.ago) } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 86ee159b97e..155e0fbb0e9 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -994,4 +994,11 @@ RSpec.describe CommitStatus do let!(:model) { create(:ci_build, project: parent) } end end + + context 'loose foreign key on ci_builds.runner_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:ci_runner) } + let!(:model) { create(:ci_build, runner: parent) } + end + end end diff --git a/spec/models/concerns/approvable_base_spec.rb b/spec/models/concerns/approvable_base_spec.rb index 79053e98db7..2bf6a98a64d 100644 --- a/spec/models/concerns/approvable_base_spec.rb +++ b/spec/models/concerns/approvable_base_spec.rb @@ -36,7 +36,7 @@ RSpec.describe ApprovableBase do subject { merge_request.can_be_approved_by?(user) } before do - merge_request.project.add_developer(user) + merge_request.project.add_developer(user) if user end it 'returns true' do @@ -64,7 +64,7 @@ RSpec.describe ApprovableBase do subject { merge_request.can_be_unapproved_by?(user) } before do - merge_request.project.add_developer(user) + merge_request.project.add_developer(user) if user end it 'returns false' do diff --git a/spec/models/concerns/batch_nullify_dependent_associations_spec.rb b/spec/models/concerns/batch_nullify_dependent_associations_spec.rb new file mode 100644 index 00000000000..933464f301a --- /dev/null +++ b/spec/models/concerns/batch_nullify_dependent_associations_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BatchNullifyDependentAssociations do + before do + test_user = Class.new(ActiveRecord::Base) do + self.table_name = 'users' + + has_many :closed_issues, foreign_key: :closed_by_id, class_name: 'Issue', dependent: :nullify + has_many :issues, foreign_key: :author_id, class_name: 'Issue', dependent: :nullify + has_many :updated_issues, foreign_key: :updated_by_id, class_name: 'Issue' + + include BatchNullifyDependentAssociations + end + + stub_const('TestUser', test_user) + end + + describe '.dependent_associations_to_nullify' do + it 'returns only associations with `dependent: :nullify` associations' do + expect(TestUser.dependent_associations_to_nullify.map(&:name)).to match_array([:closed_issues, :issues]) + end + end + + describe '#nullify_dependent_associations_in_batches' do + let_it_be(:user) { create(:user) } + let_it_be(:updated_by_issue) { create(:issue, updated_by: user) } + + before do + create(:issue, closed_by: user) + create(:issue, closed_by: user) + end + + it 'nullifies multiple settings' do + expect do + test_user = TestUser.find(user.id) + test_user.nullify_dependent_associations_in_batches + end.to change { Issue.where(closed_by_id: user.id).count }.by(-2) + end + + it 'excludes associations' do + expect do + test_user = TestUser.find(user.id) + test_user.nullify_dependent_associations_in_batches(exclude: [:closed_issues]) + end.not_to change { Issue.where(closed_by_id: user.id).count } + end + end +end diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb index 453b6f7f29a..bf104fe1b30 100644 --- a/spec/models/concerns/featurable_spec.rb +++ b/spec/models/concerns/featurable_spec.rb @@ -3,171 +3,101 @@ require 'spec_helper' RSpec.describe Featurable do - let_it_be(:user) { create(:user) } + let!(:klass) do + Class.new(ApplicationRecord) do + include Featurable - let(:project) { create(:project) } - let(:feature_class) { subject.class } - let(:features) { feature_class::FEATURES } + self.table_name = 'project_features' - subject { project.project_feature } + set_available_features %i(feature1 feature2 feature3) - describe '.quoted_access_level_column' do - it 'returns the table name and quoted column name for a feature' do - expected = '"project_features"."issues_access_level"' - - expect(feature_class.quoted_access_level_column(:issues)).to eq(expected) - end - end + def feature1_access_level + Featurable::DISABLED + end - describe '.access_level_attribute' do - it { expect(feature_class.access_level_attribute(:wiki)).to eq :wiki_access_level } + def feature2_access_level + Featurable::ENABLED + end - it 'raises error for unspecified feature' do - expect { feature_class.access_level_attribute(:unknown) } - .to raise_error(ArgumentError, /invalid feature: unknown/) + def feature3_access_level + Featurable::PRIVATE + end end end - describe '.set_available_features' do - let!(:klass) do - Class.new(ApplicationRecord) do - include Featurable + subject { klass.new } - self.table_name = 'project_features' - - set_available_features %i(feature1 feature2) + describe '.set_available_features' do + it { expect(klass.available_features).to match_array [:feature1, :feature2, :feature3] } + end - def feature1_access_level - Featurable::DISABLED - end + describe '#*_enabled?' do + it { expect(subject.feature1_enabled?).to be_falsey } + it { expect(subject.feature2_enabled?).to be_truthy } + end - def feature2_access_level - Featurable::ENABLED - end - end + describe '.quoted_access_level_column' do + it 'returns the table name and quoted column name for a feature' do + expect(klass.quoted_access_level_column(:feature1)).to eq('"project_features"."feature1_access_level"') end - - let!(:instance) { klass.new } - - it { expect(klass.available_features).to eq [:feature1, :feature2] } - it { expect(instance.feature1_enabled?).to be_falsey } - it { expect(instance.feature2_enabled?).to be_truthy } end - describe '.available_features' do - it { expect(feature_class.available_features).to include(*features) } + describe '.access_level_attribute' do + it { expect(klass.access_level_attribute(:feature1)).to eq :feature1_access_level } + + it 'raises error for unspecified feature' do + expect { klass.access_level_attribute(:unknown) } + .to raise_error(ArgumentError, /invalid feature: unknown/) + end end describe '#access_level' do it 'returns access level' do - expect(subject.access_level(:wiki)).to eq(subject.wiki_access_level) + expect(subject.access_level(:feature1)).to eq(subject.feature1_access_level) end end describe '#feature_available?' do - let(:features) { %w(issues wiki builds merge_requests snippets repository pages metrics_dashboard) } - context 'when features are disabled' do - it "returns false" do - update_all_project_features(project, features, ProjectFeature::DISABLED) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" - end + it 'returns false' do + expect(subject.feature_available?(:feature1)).to eq(false) end end context 'when features are enabled only for team members' do - it "returns false when user is not a team member" do - update_all_project_features(project, features, ProjectFeature::PRIVATE) + let_it_be(:user) { create(:user) } - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" - end + before do + expect(subject).to receive(:member?).and_call_original end - it "returns true when user is a team member" do - project.add_developer(user) - - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed" + context 'when user is not present' do + it 'returns false' do + expect(subject.feature_available?(:feature3)).to eq(false) end end - it "returns true when user is a member of project group" do - group = create(:group) - project = create(:project, namespace: group) - group.add_developer(user) + context 'when user can read all resources' do + it 'returns true' do + allow(user).to receive(:can_read_all_resources?).and_return(true) - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed" + expect(subject.feature_available?(:feature3, user)).to eq(true) end end - context 'when admin mode is enabled', :enable_admin_mode do - it "returns true if user is an admin" do - user.update_attribute(:admin, true) - - update_all_project_features(project, features, ProjectFeature::PRIVATE) + context 'when user cannot read all resources' do + it 'raises NotImplementedError exception' do + expect(subject).to receive(:resource_member?).and_call_original - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(true), "#{feature} failed" - end - end - end - - context 'when admin mode is disabled' do - it "returns false when user is an admin" do - user.update_attribute(:admin, true) - - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" - end + expect { subject.feature_available?(:feature3, user) }.to raise_error(NotImplementedError) end end end context 'when feature is enabled for everyone' do - it "returns true" do - expect(project.feature_available?(:issues, user)).to eq(true) + it 'returns true' do + expect(subject.feature_available?(:feature2)).to eq(true) end end end - - describe '#*_enabled?' do - let(:features) { %w(wiki builds merge_requests) } - - it "returns false when feature is disabled" do - update_all_project_features(project, features, ProjectFeature::DISABLED) - - features.each do |feature| - expect(project.public_send("#{feature}_enabled?")).to eq(false), "#{feature} failed" - end - end - - it "returns true when feature is enabled only for team members" do - update_all_project_features(project, features, ProjectFeature::PRIVATE) - - features.each do |feature| - expect(project.public_send("#{feature}_enabled?")).to eq(true), "#{feature} failed" - end - end - - it "returns true when feature is enabled for everyone" do - features.each do |feature| - expect(project.public_send("#{feature}_enabled?")).to eq(true), "#{feature} failed" - end - end - end - - def update_all_project_features(project, features, value) - project_feature_attributes = features.to_h { |f| ["#{f}_access_level", value] } - project.project_feature.update!(project_feature_attributes) - end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index e3c0e3a7a2b..b38135fc0b2 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -625,6 +625,16 @@ RSpec.describe Issuable do end end + describe "#labels_hook_attrs" do + let(:project) { create(:project) } + let(:label) { create(:label) } + let(:issue) { create(:labeled_issue, project: project, labels: [label]) } + + it "returns a list of label hook attributes" do + expect(issue.labels_hook_attrs).to match_array([label.hook_attrs]) + end + end + describe '.labels_hash' do let(:feature_label) { create(:label, title: 'Feature') } let(:second_label) { create(:label, title: 'Second Label') } diff --git a/spec/models/concerns/sensitive_serializable_hash_spec.rb b/spec/models/concerns/sensitive_serializable_hash_spec.rb index 923f9e80c1f..c864ecb4eec 100644 --- a/spec/models/concerns/sensitive_serializable_hash_spec.rb +++ b/spec/models/concerns/sensitive_serializable_hash_spec.rb @@ -30,16 +30,6 @@ RSpec.describe SensitiveSerializableHash do expect(model.serializable_hash(unsafe_serialization_hash: true)).to include('super_secret') end end - - context 'when prevent_sensitive_fields_from_serializable_hash feature flag is disabled' do - before do - stub_feature_flags(prevent_sensitive_fields_from_serializable_hash: false) - end - - it 'includes the field in serializable_hash' do - expect(model.serializable_hash).to include('super_secret') - end - end end describe '#serializable_hash' do diff --git a/spec/models/concerns/taskable_spec.rb b/spec/models/concerns/taskable_spec.rb index 6b41174a046..140f6cda51c 100644 --- a/spec/models/concerns/taskable_spec.rb +++ b/spec/models/concerns/taskable_spec.rb @@ -13,6 +13,10 @@ RSpec.describe Taskable do - [x] Second item * [x] First item * [ ] Second item + + [ ] No-break space (U+00A0) + + [ ] Figure space (U+2007) + + [ ] Narrow no-break space (U+202F) + + [ ] Thin space (U+2009) MARKDOWN end @@ -21,7 +25,11 @@ RSpec.describe Taskable do TaskList::Item.new('- [ ]', 'First item'), TaskList::Item.new('- [x]', 'Second item'), TaskList::Item.new('* [x]', 'First item'), - TaskList::Item.new('* [ ]', 'Second item') + TaskList::Item.new('* [ ]', 'Second item'), + TaskList::Item.new('+ [ ]', 'No-break space (U+00A0)'), + TaskList::Item.new('+ [ ]', 'Figure space (U+2007)'), + TaskList::Item.new('+ [ ]', 'Narrow no-break space (U+202F)'), + TaskList::Item.new('+ [ ]', 'Thin space (U+2009)') ] end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index c8d86edc55f..2ea042fb767 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -122,6 +122,27 @@ RSpec.describe ContainerRepository, :aggregate_failures do expect(repository).to be_import_aborted end end + + context 'already imported' do + it 'finishes the import' do + expect(repository).to receive(:migration_pre_import).and_return(:already_imported) + + expect { subject } + .to change { repository.reload.migration_state }.to('import_done') + .and change { repository.reload.migration_skipped_reason }.to('native_import') + end + end + + context 'non-existing repository' do + it 'finishes the import' do + expect(repository).to receive(:migration_pre_import).and_return(:not_found) + + expect { subject } + .to change { repository.reload.migration_state }.to('import_done') + .and change { repository.migration_skipped_reason }.to('not_found') + .and change { repository.migration_import_done_at }.from(nil) + end + end end shared_examples 'transitioning to importing', skip_import_success: true do @@ -151,6 +172,16 @@ RSpec.describe ContainerRepository, :aggregate_failures do expect(repository).to be_import_aborted end end + + context 'already imported' do + it 'finishes the import' do + expect(repository).to receive(:migration_import).and_return(:already_imported) + + expect { subject } + .to change { repository.reload.migration_state }.to('import_done') + .and change { repository.reload.migration_skipped_reason }.to('native_import') + end + end end shared_examples 'transitioning out of import_aborted' do @@ -193,7 +224,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end - it_behaves_like 'transitioning from allowed states', %w[default] + it_behaves_like 'transitioning from allowed states', %w[default pre_importing importing import_aborted] it_behaves_like 'transitioning to pre_importing' end @@ -208,7 +239,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end - it_behaves_like 'transitioning from allowed states', %w[import_aborted] + it_behaves_like 'transitioning from allowed states', %w[pre_importing importing import_aborted] it_behaves_like 'transitioning to pre_importing' it_behaves_like 'transitioning out of import_aborted' end @@ -218,7 +249,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do subject { repository.finish_pre_import } - it_behaves_like 'transitioning from allowed states', %w[pre_importing import_aborted] + it_behaves_like 'transitioning from allowed states', %w[pre_importing importing import_aborted] it 'sets migration_pre_import_done_at' do expect { subject }.to change { repository.reload.migration_pre_import_done_at } @@ -238,7 +269,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end - it_behaves_like 'transitioning from allowed states', %w[pre_import_done] + it_behaves_like 'transitioning from allowed states', %w[pre_import_done pre_importing importing import_aborted] it_behaves_like 'transitioning to importing' end @@ -253,7 +284,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end - it_behaves_like 'transitioning from allowed states', %w[import_aborted] + it_behaves_like 'transitioning from allowed states', %w[pre_importing importing import_aborted] it_behaves_like 'transitioning to importing' it_behaves_like 'no action when feature flag is disabled' end @@ -263,7 +294,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do subject { repository.finish_import } - it_behaves_like 'transitioning from allowed states', %w[importing import_aborted] + it_behaves_like 'transitioning from allowed states', %w[default pre_importing importing import_aborted] it_behaves_like 'queueing the next import' it 'sets migration_import_done_at and queues the next import' do @@ -302,6 +333,19 @@ RSpec.describe ContainerRepository, :aggregate_failures do expect(repository.migration_aborted_in_state).to eq('importing') expect(repository).to be_import_aborted end + + context 'above the max retry limit' do + before do + stub_application_setting(container_registry_import_max_retries: 1) + end + + it 'skips the migration' do + expect { subject }.to change { repository.migration_skipped_at } + + expect(repository.reload).to be_import_skipped + expect(repository.migration_skipped_reason).to eq('too_many_retries') + end + end end describe '#skip_import' do @@ -309,7 +353,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do subject { repository.skip_import(reason: :too_many_retries) } - it_behaves_like 'transitioning from allowed states', ContainerRepository::ABORTABLE_MIGRATION_STATES + it_behaves_like 'transitioning from allowed states', ContainerRepository::SKIPPABLE_MIGRATION_STATES it 'sets migration_skipped_at and migration_skipped_reason' do expect { subject }.to change { repository.reload.migration_skipped_at } @@ -334,7 +378,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end - it_behaves_like 'transitioning from allowed states', %w[pre_importing import_aborted] + it_behaves_like 'transitioning from allowed states', %w[pre_importing importing import_aborted] it_behaves_like 'transitioning to importing' end end @@ -391,7 +435,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do describe '#retry_aborted_migration' do subject { repository.retry_aborted_migration } - shared_examples 'no action' do + context 'when migration_state is not aborted' do it 'does nothing' do expect { subject }.not_to change { repository.reload.migration_state } @@ -399,104 +443,45 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end - shared_examples 'retrying the pre_import' do - it 'retries the pre_import' do - expect(repository).to receive(:migration_pre_import).and_return(:ok) - - expect { subject }.to change { repository.reload.migration_state }.to('pre_importing') - end - end - - shared_examples 'retrying the import' do - it 'retries the import' do - expect(repository).to receive(:migration_import).and_return(:ok) - - expect { subject }.to change { repository.reload.migration_state }.to('importing') - end - end - - context 'when migration_state is not aborted' do - it_behaves_like 'no action' - end - context 'when migration_state is aborted' do before do repository.abort_import allow(repository.gitlab_api_client) - .to receive(:import_status).with(repository.path).and_return(client_response) + .to receive(:import_status).with(repository.path).and_return(status) end - context 'native response' do - let(:client_response) { 'native' } - - it 'raises an error' do - expect { subject }.to raise_error(described_class::NativeImportError) - end - end + it_behaves_like 'reconciling migration_state' do + context 'error response' do + let(:status) { 'error' } - context 'import_in_progress response' do - let(:client_response) { 'import_in_progress' } - - it_behaves_like 'no action' - end - - context 'import_complete response' do - let(:client_response) { 'import_complete' } - - it 'finishes the import' do - expect { subject }.to change { repository.reload.migration_state }.to('import_done') - end - end - - context 'import_failed response' do - let(:client_response) { 'import_failed' } - - it_behaves_like 'retrying the import' - end - - context 'pre_import_in_progress response' do - let(:client_response) { 'pre_import_in_progress' } - - it_behaves_like 'no action' - end + context 'migration_pre_import_done_at is NULL' do + it_behaves_like 'retrying the pre_import' + end - context 'pre_import_complete response' do - let(:client_response) { 'pre_import_complete' } + context 'migration_pre_import_done_at is not NULL' do + before do + repository.update_columns( + migration_pre_import_started_at: 5.minutes.ago, + migration_pre_import_done_at: Time.zone.now + ) + end - it 'finishes the pre_import and starts the import' do - expect(repository).to receive(:finish_pre_import).and_call_original - expect(repository).to receive(:migration_import).and_return(:ok) - - expect { subject }.to change { repository.reload.migration_state }.to('importing') + it_behaves_like 'retrying the import' + end end end + end + end - context 'pre_import_failed response' do - let(:client_response) { 'pre_import_failed' } - - it_behaves_like 'retrying the pre_import' - end - - context 'error response' do - let(:client_response) { 'error' } - - context 'migration_pre_import_done_at is NULL' do - it_behaves_like 'retrying the pre_import' - end - - context 'migration_pre_import_done_at is not NULL' do - before do - repository.update_columns( - migration_pre_import_started_at: 5.minutes.ago, - migration_pre_import_done_at: Time.zone.now - ) - end + describe '#reconcile_import_status' do + subject { repository.reconcile_import_status(status) } - it_behaves_like 'retrying the import' - end - end + before do + repository.abort_import end + + it_behaves_like 'reconciling migration_state' end describe '#tag' do @@ -667,7 +652,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do context 'supports gitlab api on .com with a recent repository' do before do expect(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(true) - expect(repository.gitlab_api_client).to receive(:repository_details).with(repository.path, with_size: true).and_return(response) + expect(repository.gitlab_api_client).to receive(:repository_details).with(repository.path, sizing: :self).and_return(response) end context 'with a size_bytes field' do @@ -722,12 +707,12 @@ RSpec.describe ContainerRepository, :aggregate_failures do end context 'registry migration' do - shared_examples 'handling the migration step' do |step| - let(:client_response) { :foobar } + before do + allow(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(true) + end - before do - allow(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(true) - end + shared_examples 'gitlab migration client request' do |step| + let(:client_response) { :foobar } it 'returns the same response as the client' do expect(repository.gitlab_api_client) @@ -746,6 +731,10 @@ RSpec.describe ContainerRepository, :aggregate_failures do expect(subject).to eq(:error) end end + end + + shared_examples 'handling the migration step' do |step| + it_behaves_like 'gitlab migration client request', step context 'too many imports' do it 'raises an error when it receives too_many_imports as a response' do @@ -767,6 +756,67 @@ RSpec.describe ContainerRepository, :aggregate_failures do it_behaves_like 'handling the migration step', :import_repository end + + describe '#migration_cancel' do + subject { repository.migration_cancel } + + it_behaves_like 'gitlab migration client request', :cancel_repository_import + end + + describe '#force_migration_cancel' do + subject { repository.force_migration_cancel } + + shared_examples 'returning the same response as the client' do + it 'returns the same response' do + expect(repository.gitlab_api_client) + .to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response) + + expect(subject).to eq(client_response) + end + end + + context 'successful cancellation' do + let(:client_response) { { status: :ok } } + + it_behaves_like 'returning the same response as the client' + + it 'skips the migration' do + expect(repository.gitlab_api_client) + .to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response) + + expect { subject }.to change { repository.reload.migration_state }.to('import_skipped') + .and change { repository.migration_skipped_reason }.to('migration_forced_canceled') + .and change { repository.migration_skipped_at } + end + end + + context 'failed cancellation' do + let(:client_response) { { status: :error } } + + it_behaves_like 'returning the same response as the client' + + it 'does not skip the migration' do + expect(repository.gitlab_api_client) + .to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response) + + expect { subject }.to not_change { repository.reload.migration_state } + .and not_change { repository.migration_skipped_reason } + .and not_change { repository.migration_skipped_at } + end + end + + context 'when the gitlab_api feature is not supported' do + before do + allow(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(false) + end + + it 'returns :error' do + expect(repository.gitlab_api_client).not_to receive(:cancel_repository_import) + + expect(subject).to eq(:error) + end + end + end end describe '.build_from_path' do @@ -1081,6 +1131,43 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end + describe '.all_migrated?' do + let_it_be(:project) { create(:project) } + + subject { project.container_repositories.all_migrated? } + + context 'with no repositories' do + it { is_expected.to be_truthy } + end + + context 'with only recent repositories' do + let_it_be(:container_repository1) { create(:container_repository, project: project) } + let_it_be_with_reload(:container_repository2) { create(:container_repository, project: project) } + + it { is_expected.to be_truthy } + + context 'with one old non migrated repository' do + before do + container_repository2.update!(created_at: described_class::MIGRATION_PHASE_1_ENDED_AT - 3.months) + end + + it { is_expected.to be_falsey } + end + + context 'with one old migrated repository' do + before do + container_repository2.update!( + created_at: described_class::MIGRATION_PHASE_1_ENDED_AT - 3.months, + migration_state: 'import_done', + migration_import_done_at: Time.zone.now + ) + end + + it { is_expected.to be_truthy } + end + end + end + describe '.with_enabled_policy' do let_it_be(:repository) { create(:container_repository) } let_it_be(:repository2) { create(:container_repository) } @@ -1168,6 +1255,17 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end + context 'not found response' do + let(:response) { :not_found } + + it 'completes the migration' do + expect(subject).to eq(false) + + expect(container_repository).to be_import_done + expect(container_repository.reload.migration_skipped_reason).to eq('not_found') + end + end + context 'other response' do let(:response) { :error } @@ -1185,6 +1283,30 @@ RSpec.describe ContainerRepository, :aggregate_failures do end end + describe '#retried_too_many_times?' do + subject { repository.retried_too_many_times? } + + before do + stub_application_setting(container_registry_import_max_retries: 3) + end + + context 'migration_retries_count is equal or greater than max_retries' do + before do + repository.update_column(:migration_retries_count, 3) + end + + it { is_expected.to eq(true) } + end + + context 'migration_retries_count is lower than max_retries' do + before do + repository.update_column(:migration_retries_count, 2) + end + + it { is_expected.to eq(false) } + end + end + context 'with repositories' do let_it_be_with_reload(:repository) { create(:container_repository, :cleanup_unscheduled) } let_it_be(:other_repository) { create(:container_repository, :cleanup_unscheduled) } @@ -1241,11 +1363,12 @@ RSpec.describe ContainerRepository, :aggregate_failures do let_it_be(:import_done_repository) { create(:container_repository, :import_done, migration_pre_import_done_at: 3.days.ago, migration_import_done_at: 2.days.ago) } let_it_be(:import_aborted_repository) { create(:container_repository, :import_aborted, migration_pre_import_done_at: 5.days.ago, migration_aborted_at: 1.day.ago) } let_it_be(:pre_import_done_repository) { create(:container_repository, :pre_import_done, migration_pre_import_done_at: 1.hour.ago) } + let_it_be(:import_skipped_repository) { create(:container_repository, :import_skipped, migration_skipped_at: 90.minutes.ago) } subject { described_class.recently_done_migration_step } it 'returns completed imports by done_at date' do - expect(subject.to_a).to eq([pre_import_done_repository, import_aborted_repository, import_done_repository]) + expect(subject.to_a).to eq([pre_import_done_repository, import_skipped_repository, import_aborted_repository, import_done_repository]) end end @@ -1255,7 +1378,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do subject { described_class.ready_for_import } before do - stub_application_setting(container_registry_import_target_plan: root_group.actual_plan_name) + stub_application_setting(container_registry_import_target_plan: valid_container_repository.migration_plan) end it 'works' do @@ -1266,13 +1389,15 @@ RSpec.describe ContainerRepository, :aggregate_failures do describe '#last_import_step_done_at' do let_it_be(:aborted_at) { Time.zone.now - 1.hour } let_it_be(:pre_import_done_at) { Time.zone.now - 2.hours } + let_it_be(:skipped_at) { Time.zone.now - 3.hours } subject { repository.last_import_step_done_at } before do repository.update_columns( migration_pre_import_done_at: pre_import_done_at, - migration_aborted_at: aborted_at + migration_aborted_at: aborted_at, + migration_skipped_at: skipped_at ) end diff --git a/spec/models/custom_emoji_spec.rb b/spec/models/custom_emoji_spec.rb index 01252a58681..15655d08556 100644 --- a/spec/models/custom_emoji_spec.rb +++ b/spec/models/custom_emoji_spec.rb @@ -38,7 +38,7 @@ RSpec.describe CustomEmoji do new_emoji = build(:custom_emoji, name: old_emoji.name, namespace: old_emoji.namespace, group: group) expect(new_emoji).not_to be_valid - expect(new_emoji.errors.messages).to eq(creator: ["can't be blank"], name: ["has already been taken"]) + expect(new_emoji.errors.messages).to eq(name: ["has already been taken"]) end it 'disallows non http and https file value' do diff --git a/spec/models/customer_relations/contact_spec.rb b/spec/models/customer_relations/contact_spec.rb index 18896962261..86f868b269e 100644 --- a/spec/models/customer_relations/contact_spec.rb +++ b/spec/models/customer_relations/contact_spec.rb @@ -25,7 +25,7 @@ RSpec.describe CustomerRelations::Contact, type: :model do it { is_expected.to validate_length_of(:email).is_at_most(255) } it { is_expected.to validate_length_of(:description).is_at_most(1024) } - it { is_expected.to validate_uniqueness_of(:email).scoped_to(:group_id) } + it { is_expected.to validate_uniqueness_of(:email).case_insensitive.scoped_to(:group_id) } it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email end @@ -87,6 +87,15 @@ RSpec.describe CustomerRelations::Contact, type: :model do too_many_emails = described_class::MAX_PLUCK + 1 expect { described_class.find_ids_by_emails(group, Array(0..too_many_emails)) }.to raise_error(ArgumentError) end + + it 'finds contacts regardless of email casing' do + new_contact = create(:contact, group: group, email: "UPPERCASE@example.com") + emails = [group_contacts[0].email.downcase, group_contacts[1].email.upcase, new_contact.email] + + contact_ids = described_class.find_ids_by_emails(group, emails) + + expect(contact_ids).to contain_exactly(group_contacts[0].id, group_contacts[1].id, new_contact.id) + end end describe '#self.exists_for_group?' do @@ -104,4 +113,33 @@ RSpec.describe CustomerRelations::Contact, type: :model do end end end + + describe '#self.move_to_root_group' do + let!(:old_root_group) { create(:group) } + let!(:contacts) { create_list(:contact, 4, group: old_root_group) } + let!(:project) { create(:project, group: old_root_group) } + let!(:issue) { create(:issue, project: project) } + let!(:issue_contact1) { create(:issue_customer_relations_contact, issue: issue, contact: contacts[0]) } + let!(:issue_contact2) { create(:issue_customer_relations_contact, issue: issue, contact: contacts[1]) } + let!(:new_root_group) { create(:group) } + let!(:dupe_contact1) { create(:contact, group: new_root_group, email: contacts[1].email) } + let!(:dupe_contact2) { create(:contact, group: new_root_group, email: contacts[3].email.upcase) } + + before do + old_root_group.update!(parent: new_root_group) + CustomerRelations::Contact.move_to_root_group(old_root_group) + end + + it 'moves contacts with unique emails and deletes the rest' do + expect(contacts[0].reload.group_id).to eq(new_root_group.id) + expect(contacts[2].reload.group_id).to eq(new_root_group.id) + expect { contacts[1].reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { contacts[3].reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'updates issue_contact.contact_id for dupes and leaves the rest untouched' do + expect(issue_contact1.reload.contact_id).to eq(contacts[0].id) + expect(issue_contact2.reload.contact_id).to eq(dupe_contact1.id) + end + end end diff --git a/spec/models/customer_relations/issue_contact_spec.rb b/spec/models/customer_relations/issue_contact_spec.rb index f1fb574f86f..221378d26b2 100644 --- a/spec/models/customer_relations/issue_contact_spec.rb +++ b/spec/models/customer_relations/issue_contact_spec.rb @@ -92,4 +92,16 @@ RSpec.describe CustomerRelations::IssueContact do expect { described_class.delete_for_project(project.id) }.to change { described_class.count }.by(-3) end end + + describe '.delete_for_group' do + let(:project_for_root_group) { create(:project, group: group) } + + it 'destroys all issue_contacts for projects in group and subgroups' do + create_list(:issue_customer_relations_contact, 2, :for_issue, issue: create(:issue, project: project)) + create_list(:issue_customer_relations_contact, 2, :for_issue, issue: create(:issue, project: project_for_root_group)) + create(:issue_customer_relations_contact) + + expect { described_class.delete_for_group(group) }.to change { described_class.count }.by(-4) + end + end end diff --git a/spec/models/customer_relations/organization_spec.rb b/spec/models/customer_relations/organization_spec.rb index 9fe754b7605..06ba9c5b7ad 100644 --- a/spec/models/customer_relations/organization_spec.rb +++ b/spec/models/customer_relations/organization_spec.rb @@ -50,4 +50,32 @@ RSpec.describe CustomerRelations::Organization, type: :model do expect(described_class.find_by_name(group.id, 'TEST')).to eq([organiztion1]) end end + + describe '#self.move_to_root_group' do + let!(:old_root_group) { create(:group) } + let!(:organizations) { create_list(:organization, 4, group: old_root_group) } + let!(:new_root_group) { create(:group) } + let!(:contact1) { create(:contact, group: new_root_group, organization: organizations[0]) } + let!(:contact2) { create(:contact, group: new_root_group, organization: organizations[1]) } + + let!(:dupe_organization1) { create(:organization, group: new_root_group, name: organizations[1].name) } + let!(:dupe_organization2) { create(:organization, group: new_root_group, name: organizations[3].name.upcase) } + + before do + old_root_group.update!(parent: new_root_group) + CustomerRelations::Organization.move_to_root_group(old_root_group) + end + + it 'moves organizations with unique names and deletes the rest' do + expect(organizations[0].reload.group_id).to eq(new_root_group.id) + expect(organizations[2].reload.group_id).to eq(new_root_group.id) + expect { organizations[1].reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { organizations[3].reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'updates contact.organization_id for dupes and leaves the rest untouched' do + expect(contact1.reload.organization_id).to eq(organizations[0].id) + expect(contact2.reload.organization_id).to eq(dupe_organization1.id) + end + end end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 88451307efb..c48f1fab3c6 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -9,6 +9,7 @@ RSpec.describe DeployToken do it { is_expected.to have_many(:projects).through(:project_deploy_tokens) } it { is_expected.to have_many :group_deploy_tokens } it { is_expected.to have_many(:groups).through(:group_deploy_tokens) } + it { is_expected.to belong_to(:user).with_foreign_key('creator_id') } it_behaves_like 'having unique enum values' diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 47c246d12cc..705b9b4cc65 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -524,6 +524,16 @@ RSpec.describe Deployment do is_expected.to contain_exactly(deployment1, deployment2, deployment3, deployment4, deployment5) end + + it 'has a corresponding database index' do + index = ApplicationRecord.connection.indexes('deployments').find do |i| + i.name == 'index_deployments_for_visible_scope' + end + + scope_values = described_class::VISIBLE_STATUSES.map { |s| described_class.statuses[s] }.to_s + + expect(index.where).to include(scope_values) + end end describe 'upcoming' do @@ -1055,6 +1065,40 @@ RSpec.describe Deployment do end end + describe '#tier_in_yaml' do + context 'when deployable is nil' do + before do + subject.deployable = nil + end + + it 'returns nil' do + expect(subject.tier_in_yaml).to be_nil + end + end + + context 'when deployable is present' do + context 'when tier is specified' do + let(:deployable) { create(:ci_build, :success, :environment_with_deployment_tier) } + + before do + subject.deployable = deployable + end + + it 'returns the tier' do + expect(subject.tier_in_yaml).to eq('testing') + end + + context 'when tier is not specified' do + let(:deployable) { create(:ci_build, :success) } + + it 'returns nil' do + expect(subject.tier_in_yaml).to be_nil + end + end + end + end + end + describe '.fast_destroy_all' do it 'cleans path_refs for destroyed environments' do project = create(:project, :repository) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 6144593395c..b42e73e6d93 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -23,7 +23,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do it { is_expected.to have_one(:upcoming_deployment) } it { is_expected.to have_one(:latest_opened_most_severe_alert) } - it { is_expected.to delegate_method(:stop_action).to(:last_deployment) } it { is_expected.to delegate_method(:manual_actions).to(:last_deployment) } it { is_expected.to validate_presence_of(:name) } @@ -349,15 +348,28 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end describe '.with_deployment' do - subject { described_class.with_deployment(sha) } + subject { described_class.with_deployment(sha, status: status) } let(:environment) { create(:environment, project: project) } let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + let(:status) { nil } context 'when deployment has the specified sha' do let!(:deployment) { create(:deployment, environment: environment, sha: sha) } it { is_expected.to eq([environment]) } + + context 'with success status filter' do + let(:status) { :success } + + it { is_expected.to be_empty } + end + + context 'with created status filter' do + let(:status) { :created } + + it { is_expected.to contain_exactly(environment) } + end end context 'when deployment does not have the specified sha' do @@ -459,8 +471,8 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end - describe '#stop_action_available?' do - subject { environment.stop_action_available? } + describe '#stop_actions_available?' do + subject { environment.stop_actions_available? } context 'when no other actions' do it { is_expected.to be_falsey } @@ -499,10 +511,10 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end - describe '#stop_with_action!' do + describe '#stop_with_actions!' do let(:user) { create(:user) } - subject { environment.stop_with_action!(user) } + subject { environment.stop_with_actions!(user) } before do expect(environment).to receive(:available?).and_call_original @@ -515,9 +527,10 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end it do - subject + actions = subject expect(environment).to be_stopped + expect(actions).to match_array([]) end end @@ -536,18 +549,18 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do context 'when matching action is defined' do let(:pipeline) { create(:ci_pipeline, project: project) } - let(:build) { create(:ci_build, pipeline: pipeline) } + let(:build_a) { create(:ci_build, pipeline: pipeline) } - let!(:deployment) do + before do create(:deployment, :success, - environment: environment, - deployable: build, - on_stop: 'close_app') + environment: environment, + deployable: build_a, + on_stop: 'close_app_a') end context 'when user is not allowed to stop environment' do let!(:close_action) do - create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') + create(:ci_build, :manual, pipeline: pipeline, name: 'close_app_a') end it 'raises an exception' do @@ -565,31 +578,200 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do context 'when action did not yet finish' do let!(:close_action) do - create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') + create(:ci_build, :manual, pipeline: pipeline, name: 'close_app_a') end it 'returns the same action' do - expect(subject).to eq(close_action) - expect(subject.user).to eq(user) + action = subject.first + expect(action).to eq(close_action) + expect(action.user).to eq(user) end end context 'if action did finish' do let!(:close_action) do create(:ci_build, :manual, :success, - pipeline: pipeline, name: 'close_app') + pipeline: pipeline, name: 'close_app_a') end it 'returns a new action of the same type' do - expect(subject).to be_persisted - expect(subject.name).to eq(close_action.name) - expect(subject.user).to eq(user) + action = subject.first + + expect(action).to be_persisted + expect(action.name).to eq(close_action.name) + expect(action.user).to eq(user) + end + end + + context 'close action does not raise ActiveRecord::StaleObjectError' do + let!(:close_action) do + create(:ci_build, :manual, pipeline: pipeline, name: 'close_app_a') + end + + before do + # preload the build + environment.stop_actions + + # Update record as the other process. This makes `environment.stop_action` stale. + close_action.drop! end + + it 'successfully plays the build even if the build was a stale object' do + # Since build is droped. + expect(close_action.processed).to be_falsey + + # it encounters the StaleObjectError at first, but reloads the object and runs `build.play` + expect { subject }.not_to raise_error(ActiveRecord::StaleObjectError) + + # Now the build should be processed. + expect(close_action.reload.processed).to be_truthy + end + end + end + end + + context 'when there are more then one stop action for the environment' do + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build_a) { create(:ci_build, pipeline: pipeline) } + let(:build_b) { create(:ci_build, pipeline: pipeline) } + + let!(:close_actions) do + [ + create(:ci_build, :manual, pipeline: pipeline, name: 'close_app_a'), + create(:ci_build, :manual, pipeline: pipeline, name: 'close_app_b') + ] + end + + before do + project.add_developer(user) + + create(:deployment, :success, + environment: environment, + deployable: build_a, + finished_at: 5.minutes.ago, + on_stop: 'close_app_a') + + create(:deployment, :success, + environment: environment, + deployable: build_b, + finished_at: 1.second.ago, + on_stop: 'close_app_b') + end + + it 'returns the same actions' do + actions = subject + + expect(actions.count).to eq(close_actions.count) + expect(actions.pluck(:id)).to match_array(close_actions.pluck(:id)) + expect(actions.pluck(:user)).to match_array(close_actions.pluck(:user)) + end + + context 'when there are failed deployment jobs' do + before do + create(:ci_build, pipeline: pipeline, name: 'close_app_c') + + create(:deployment, :failed, + environment: environment, + deployable: create(:ci_build, pipeline: pipeline), + on_stop: 'close_app_c') + end + + it 'returns only stop actions from successful deployment jobs' do + actions = subject + + expect(actions).to match_array(close_actions) + expect(actions.count).to eq(environment.successful_deployments.count) + end + end + + context 'when the feature is disabled' do + before do + stub_feature_flags(environment_multiple_stop_actions: false) + end + + it 'returns the last deployment job stop action' do + stop_actions = subject + + expect(stop_actions.first).to eq(close_actions[1]) + expect(stop_actions.count).to eq(1) end end end end + describe '#stop_actions' do + subject { environment.stop_actions } + + context 'when there are no deployments and builds' do + it 'returns empty array' do + is_expected.to match_array([]) + end + end + + context 'when there are multiple deployments with actions' do + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:ci_build_a) { create(:ci_build, project: project, pipeline: pipeline) } + let(:ci_build_b) { create(:ci_build, project: project, pipeline: pipeline) } + let!(:ci_build_c) { create(:ci_build, :manual, project: project, pipeline: pipeline, name: 'close_app_a') } + let!(:ci_build_d) { create(:ci_build, :manual, project: project, pipeline: pipeline, name: 'close_app_b') } + + let!(:deployment_a) do + create(:deployment, + :success, project: project, environment: environment, deployable: ci_build_a, on_stop: 'close_app_a') + end + + let!(:deployment_b) do + create(:deployment, + :success, project: project, environment: environment, deployable: ci_build_b, on_stop: 'close_app_b') + end + + before do + # Create failed deployment without stop_action. + build = create(:ci_build, project: project, pipeline: pipeline) + create(:deployment, :failed, project: project, environment: environment, deployable: build) + end + + it 'returns only the stop actions' do + expect(subject.pluck(:id)).to contain_exactly(ci_build_c.id, ci_build_d.id) + end + end + end + + describe '#last_deployment_group' do + subject { environment.last_deployment_group } + + context 'when there are no deployments and builds' do + it do + is_expected.to eq(Deployment.none) + end + end + + context 'when there are deployments for multiple pipelines' do + let(:pipeline_a) { create(:ci_pipeline, project: project) } + let(:pipeline_b) { create(:ci_pipeline, project: project) } + let(:ci_build_a) { create(:ci_build, project: project, pipeline: pipeline_a) } + let(:ci_build_b) { create(:ci_build, project: project, pipeline: pipeline_b) } + let(:ci_build_c) { create(:ci_build, project: project, pipeline: pipeline_a) } + let(:ci_build_d) { create(:ci_build, project: project, pipeline: pipeline_a) } + + # Successful deployments for pipeline_a + let!(:deployment_a) { create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a) } + let!(:deployment_b) { create(:deployment, :success, project: project, environment: environment, deployable: ci_build_c) } + + before do + # Failed deployment for pipeline_a + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_d) + + # Failed deployment for pipeline_b + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b) + end + + it 'returns the successful deployment jobs for the last deployment pipeline' do + expect(subject.pluck(:id)).to contain_exactly(deployment_a.id, deployment_b.id) + end + end + end + describe 'recently_updated_on_branch?' do subject { environment.recently_updated_on_branch?('feature') } @@ -698,10 +880,29 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do context 'when there is a deployment record with success status' do let!(:deployment) { create(:deployment, :success, environment: environment) } + let!(:old_deployment) { create(:deployment, :success, environment: environment, finished_at: 2.days.ago) } it 'returns the latest successful deployment' do is_expected.to eq(deployment) end + + context 'env_last_deployment_by_finished_at feature flag' do + it 'when enabled it returns the deployment with the latest finished_at' do + stub_feature_flags(env_last_deployment_by_finished_at: true) + + expect(old_deployment.finished_at < deployment.finished_at).to be_truthy + + is_expected.to eq(deployment) + end + + it 'when disabled it returns the deployment with the highest id' do + stub_feature_flags(env_last_deployment_by_finished_at: false) + + expect(old_deployment.finished_at < deployment.finished_at).to be_truthy + + is_expected.to eq(old_deployment) + end + end end end end @@ -728,6 +929,26 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe '#last_deployment_pipeline' do + subject { environment.last_deployment_pipeline } + + let(:pipeline_a) { create(:ci_pipeline, project: project) } + let(:pipeline_b) { create(:ci_pipeline, project: project) } + let(:ci_build_a) { create(:ci_build, project: project, pipeline: pipeline_a) } + let(:ci_build_b) { create(:ci_build, project: project, pipeline: pipeline_b) } + + before do + create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b) + end + + it 'does not join across databases' do + with_cross_joins_prevented do + expect(subject.id).to eq(pipeline_a.id) + end + end + end + describe '#last_visible_deployment' do subject { environment.last_visible_deployment } diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index 1db1171401c..a9aa5698ebb 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -34,6 +34,13 @@ RSpec.describe EnvironmentStatus do subject { environment_status.deployment } it { is_expected.to eq(deployment) } + + context 'multiple deployments' do + it { + new_deployment = create(:deployment, :succeed, environment: deployment.environment, sha: deployment.sha ) + is_expected.to eq(new_deployment) + } + end end # $ git diff --stat pages-deploy-target...pages-deploy diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index d700eb5eaf7..2939a40a84f 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -8,6 +8,8 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do let_it_be(:project) { create(:project) } + let(:sentry_client) { instance_double(ErrorTracking::SentryClient) } + subject(:setting) { build(:project_error_tracking_setting, project: project) } describe 'Associations' do @@ -48,7 +50,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do expect(subject.errors.messages[:project]).to include('is a required field') end - context 'presence validations' do + describe 'presence validations' do using RSpec::Parameterized::TableSyntax valid_api_url = 'http://example.com/api/0/projects/org-slug/proj-slug/' @@ -83,12 +85,12 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do describe 'after_save :create_client_key!' do subject { build(:project_error_tracking_setting, :integrated, project: project) } - context 'no client key yet' do + context 'without client key' do it 'creates a new client key' do expect { subject.save! }.to change { ErrorTracking::ClientKey.count }.by(1) end - context 'sentry backend' do + context 'with sentry backend' do before do subject.integrated = false end @@ -98,7 +100,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - context 'feature disabled' do + context 'when feature disabled' do before do subject.enabled = false end @@ -109,7 +111,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - context 'client key already exists' do + context 'when client key already exists' do let!(:client_key) { create(:error_tracking_client_key, project: project) } it 'does not create a new client key' do @@ -122,13 +124,13 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do describe '.extract_sentry_external_url' do subject { described_class.extract_sentry_external_url(sentry_url) } - describe 'when passing a URL' do + context 'when passing a URL' do let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } it { is_expected.to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project') } end - describe 'when passing nil' do + context 'when passing nil' do let(:sentry_url) { nil } it { is_expected.to be_nil } @@ -159,23 +161,15 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do describe '#list_sentry_issues' do let(:issues) { [:list, :of, :issues] } - - let(:opts) do - { issue_status: 'unresolved', limit: 10 } - end - - let(:result) do - subject.list_sentry_issues(**opts) - end + let(:result) { subject.list_sentry_issues(**opts) } + let(:opts) { { issue_status: 'unresolved', limit: 10 } } context 'when cached' do - let(:sentry_client) { spy(:sentry_client) } - before do stub_reactive_cache(subject, issues, opts) synchronous_reactive_cache(subject) - expect(subject).to receive(:sentry_client).and_return(sentry_client) + allow(subject).to receive(:sentry_client).and_return(sentry_client) end it 'returns cached issues' do @@ -195,8 +189,6 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end context 'when sentry client raises ErrorTracking::SentryClient::Error' do - let(:sentry_client) { spy(:sentry_client) } - before do synchronous_reactive_cache(subject) @@ -214,14 +206,13 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end context 'when sentry client raises ErrorTracking::SentryClient::MissingKeysError' do - let(:sentry_client) { spy(:sentry_client) } - before do synchronous_reactive_cache(subject) allow(subject).to receive(:sentry_client).and_return(sentry_client) allow(sentry_client).to receive(:list_issues).with(opts) - .and_raise(ErrorTracking::SentryClient::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"') + .and_raise(ErrorTracking::SentryClient::MissingKeysError, + 'Sentry API response is missing keys. key not found: "id"') end it 'returns error' do @@ -233,8 +224,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end context 'when sentry client raises ErrorTracking::SentryClient::ResponseInvalidSizeError' do - let(:sentry_client) { spy(:sentry_client) } - let(:error_msg) {"Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}."} + let(:error_msg) { "Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}." } before do synchronous_reactive_cache(subject) @@ -253,8 +243,6 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end context 'when sentry client raises StandardError' do - let(:sentry_client) { spy(:sentry_client) } - before do synchronous_reactive_cache(subject) @@ -270,7 +258,6 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do describe '#list_sentry_projects' do let(:projects) { [:list, :of, :projects] } - let(:sentry_client) { spy(:sentry_client) } it 'calls sentry client' do expect(subject).to receive(:sentry_client).and_return(sentry_client) @@ -284,19 +271,17 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do describe '#issue_details' do let(:issue) { build(:error_tracking_sentry_detailed_error) } - let(:sentry_client) { double('sentry_client', issue_details: issue) } let(:commit_id) { issue.first_release_version } - - let(:result) do - subject.issue_details - end + let(:result) { subject.issue_details(opts) } + let(:opts) { { issue_id: 1 } } context 'when cached' do before do stub_reactive_cache(subject, issue, {}) synchronous_reactive_cache(subject) - expect(subject).to receive(:sentry_client).and_return(sentry_client) + allow(subject).to receive(:sentry_client).and_return(sentry_client) + allow(sentry_client).to receive(:issue_details).with(opts).and_return(issue) end it { expect(result).to eq(issue: issue) } @@ -314,15 +299,15 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end context 'when repo commit matches first relase version' do - let(:commit) { double('commit', id: commit_id) } - let(:repository) { double('repository', commit: commit) } + let(:commit) { instance_double(Commit, id: commit_id) } + let(:repository) { instance_double(Repository, commit: commit) } before do - expect(project).to receive(:repository).and_return(repository) + allow(project).to receive(:repository).and_return(repository) end it { expect(result[:issue].gitlab_commit).to eq(commit_id) } - it { expect(result[:issue].gitlab_commit_path).to eq("/#{project.namespace.path}/#{project.path}/-/commit/#{commit_id}") } + it { expect(result[:issue].gitlab_commit_path).to eq(project_commit_path(project, commit_id)) } end end @@ -333,19 +318,15 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end describe '#update_issue' do - let(:opts) do - { status: 'resolved' } - end + let(:result) { subject.update_issue(**opts) } + let(:opts) { { issue_id: 1, params: {} } } - let(:result) do - subject.update_issue(**opts) + before do + allow(subject).to receive(:sentry_client).and_return(sentry_client) end - let(:sentry_client) { spy(:sentry_client) } - - context 'successful call to sentry' do + context 'when sentry response is successful' do before do - allow(subject).to receive(:sentry_client).and_return(sentry_client) allow(sentry_client).to receive(:update_issue).with(opts).and_return(true) end @@ -354,9 +335,8 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - context 'sentry raises an error' do + context 'when sentry raises an error' do before do - allow(subject).to receive(:sentry_client).and_return(sentry_client) allow(sentry_client).to receive(:update_issue).with(opts).and_raise(StandardError) end @@ -366,7 +346,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - context 'slugs' do + describe 'slugs' do shared_examples_for 'slug from api_url' do |method, slug| context 'when api_url is correct' do before do @@ -393,9 +373,9 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do it_behaves_like 'slug from api_url', :organization_slug, 'org-slug' end - context 'names from api_url' do + describe 'names from api_url' do shared_examples_for 'name from api_url' do |name, titleized_slug| - context 'name is present in DB' do + context 'when name is present in DB' do it 'returns name from DB' do subject[name] = 'Sentry name' subject.api_url = 'http://gitlab.com/api/0/projects/org-slug/project-slug' @@ -404,7 +384,7 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end - context 'name is null in DB' do + context 'when name is null in DB' do it 'titleizes and returns slug from api_url' do subject[name] = nil subject.api_url = 'http://gitlab.com/api/0/projects/org-slug/project-slug' diff --git a/spec/models/group_group_link_spec.rb b/spec/models/group_group_link_spec.rb index 034a5c1dfc6..72c700e7981 100644 --- a/spec/models/group_group_link_spec.rb +++ b/spec/models/group_group_link_spec.rb @@ -29,6 +29,49 @@ RSpec.describe GroupGroupLink do ]) end end + + describe '.distinct_on_shared_with_group_id_with_group_access' do + let_it_be(:sub_shared_group) { create(:group, parent: shared_group) } + let_it_be(:other_group) { create(:group) } + + let_it_be(:group_group_link_2) do + create( + :group_group_link, + shared_group: shared_group, + shared_with_group: other_group, + group_access: Gitlab::Access::GUEST + ) + end + + let_it_be(:group_group_link_3) do + create( + :group_group_link, + shared_group: sub_shared_group, + shared_with_group: group, + group_access: Gitlab::Access::GUEST + ) + end + + let_it_be(:group_group_link_4) do + create( + :group_group_link, + shared_group: sub_shared_group, + shared_with_group: other_group, + group_access: Gitlab::Access::DEVELOPER + ) + end + + it 'returns only one group link per group (with max group access)' do + distinct_group_group_links = described_class.distinct_on_shared_with_group_id_with_group_access + + expect(described_class.all.count).to eq(4) + expect(distinct_group_group_links.count).to eq(2) + expect(distinct_group_group_links).to include(group_group_link) + expect(distinct_group_group_links).not_to include(group_group_link_2) + expect(distinct_group_group_links).not_to include(group_group_link_3) + expect(distinct_group_group_links).to include(group_group_link_4) + end + end end describe 'validation' do @@ -57,4 +100,9 @@ RSpec.describe GroupGroupLink do group_group_link.human_access end end + + describe 'search by group name' do + it { expect(described_class.search(group.name)).to eq([group_group_link]) } + it { expect(described_class.search('not-a-group-name')).to be_empty } + end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 45a2c134077..0ca1fe1c8a6 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -41,6 +41,7 @@ RSpec.describe Group do it { is_expected.to have_many(:contacts).class_name('CustomerRelations::Contact') } it { is_expected.to have_many(:organizations).class_name('CustomerRelations::Organization') } it { is_expected.to have_one(:crm_settings) } + it { is_expected.to have_one(:group_feature) } describe '#members & #requesters' do let(:requester) { create(:user) } @@ -293,6 +294,23 @@ RSpec.describe Group do end end + it_behaves_like 'a BulkUsersByEmailLoad model' + + context 'after initialized' do + it 'has a group_feature' do + expect(described_class.new.group_feature).to be_present + end + end + + context 'when creating a new project' do + let_it_be(:group) { create(:group) } + + it 'automatically creates the groups feature for the group' do + expect(group.group_feature).to be_an_instance_of(Groups::FeatureSetting) + expect(group.group_feature).to be_persisted + end + end + context 'traversal_ids on create' do context 'default traversal_ids' do let(:group) { build(:group) } @@ -533,6 +551,10 @@ RSpec.describe Group do describe '#ancestors_upto' do it { expect(group.ancestors_upto.to_sql).not_to include "WITH ORDINALITY" } end + + describe '.shortest_traversal_ids_prefixes' do + it { expect { described_class.shortest_traversal_ids_prefixes }.to raise_error /Feature not supported since the `:use_traversal_ids` is disabled/ } + end end context 'linear' do @@ -574,6 +596,90 @@ RSpec.describe Group do it { expect(group.ancestors_upto.to_sql).to include "WITH ORDINALITY" } end + describe '.shortest_traversal_ids_prefixes' do + subject { filter.shortest_traversal_ids_prefixes } + + context 'for many top-level namespaces' do + let!(:top_level_groups) { create_list(:group, 4) } + + context 'when querying all groups' do + let(:filter) { described_class.id_in(top_level_groups) } + + it "returns all traversal_ids" do + is_expected.to contain_exactly( + *top_level_groups.map { |group| [group.id] } + ) + end + end + + context 'when querying selected groups' do + let(:filter) { described_class.id_in(top_level_groups.first) } + + it "returns only a selected traversal_ids" do + is_expected.to contain_exactly([top_level_groups.first.id]) + end + end + end + + context 'for namespace hierarchy' do + let!(:group_a) { create(:group) } + let!(:group_a_sub_1) { create(:group, parent: group_a) } + let!(:group_a_sub_2) { create(:group, parent: group_a) } + let!(:group_b) { create(:group) } + let!(:group_b_sub_1) { create(:group, parent: group_b) } + let!(:group_c) { create(:group) } + + context 'when querying all groups' do + let(:filter) { described_class.id_in([group_a, group_a_sub_1, group_a_sub_2, group_b, group_b_sub_1, group_c]) } + + it 'returns only shortest prefixes of top-level groups' do + is_expected.to contain_exactly( + [group_a.id], + [group_b.id], + [group_c.id] + ) + end + end + + context 'when sub-group is reparented' do + let(:filter) { described_class.id_in([group_b_sub_1, group_c]) } + + before do + group_b_sub_1.update!(parent: group_c) + end + + it 'returns a proper shortest prefix of a new group' do + is_expected.to contain_exactly( + [group_c.id] + ) + end + end + + context 'when querying sub-groups' do + let(:filter) { described_class.id_in([group_a_sub_1, group_b_sub_1, group_c]) } + + it 'returns sub-groups as they are shortest prefixes' do + is_expected.to contain_exactly( + [group_a.id, group_a_sub_1.id], + [group_b.id, group_b_sub_1.id], + [group_c.id] + ) + end + end + + context 'when querying group and sub-group of this group' do + let(:filter) { described_class.id_in([group_a, group_a_sub_1, group_c]) } + + it 'returns parent groups as this contains all sub-groups' do + is_expected.to contain_exactly( + [group_a.id], + [group_c.id] + ) + end + end + end + end + context 'when project namespace exists in the group' do let!(:project) { create(:project, group: group) } let!(:project_namespace) { project.project_namespace } @@ -737,11 +843,23 @@ RSpec.describe Group do describe '#add_user' do let(:user) { create(:user) } - before do + it 'adds the user with a blocking refresh by default' do + expect_next_instance_of(GroupMember) do |member| + expect(member).to receive(:refresh_member_authorized_projects).with(blocking: true) + end + group.add_user(user, GroupMember::MAINTAINER) + + expect(group.group_members.maintainers.map(&:user)).to include(user) end - it { expect(group.group_members.maintainers.map(&:user)).to include(user) } + it 'passes the blocking refresh value to member' do + expect_next_instance_of(GroupMember) do |member| + expect(member).to receive(:refresh_member_authorized_projects).with(blocking: false) + end + + group.add_user(user, GroupMember::MAINTAINER, blocking_refresh: false) + end end describe '#add_users' do @@ -3246,4 +3364,57 @@ RSpec.describe Group do it_behaves_like 'no effective expiration interval' end end + + describe '#work_items_feature_flag_enabled?' do + it_behaves_like 'checks self and root ancestor feature flag' do + let(:feature_flag) { :work_items } + let(:feature_flag_method) { :work_items_feature_flag_enabled? } + end + end + + describe 'group shares' do + let!(:sub_group) { create(:group, parent: group) } + let!(:sub_sub_group) { create(:group, parent: sub_group) } + let!(:shared_group_1) { create(:group) } + let!(:shared_group_2) { create(:group) } + let!(:shared_group_3) { create(:group) } + + before do + group.shared_with_groups << shared_group_1 + sub_group.shared_with_groups << shared_group_2 + sub_sub_group.shared_with_groups << shared_group_3 + end + + describe '#shared_with_group_links.of_ancestors' do + using RSpec::Parameterized::TableSyntax + + where(:subject_group, :result) do + ref(:group) | [] + ref(:sub_group) | lazy { [shared_group_1].map(&:id) } + ref(:sub_sub_group) | lazy { [shared_group_1, shared_group_2].map(&:id) } + end + + with_them do + it 'returns correct group shares' do + expect(subject_group.shared_with_group_links.of_ancestors.map(&:shared_with_group_id)).to match_array(result) + end + end + end + + describe '#shared_with_group_links.of_ancestors_and_self' do + using RSpec::Parameterized::TableSyntax + + where(:subject_group, :result) do + ref(:group) | lazy { [shared_group_1].map(&:id) } + ref(:sub_group) | lazy { [shared_group_1, shared_group_2].map(&:id) } + ref(:sub_sub_group) | lazy { [shared_group_1, shared_group_2, shared_group_3].map(&:id) } + end + + with_them do + it 'returns correct group shares' do + expect(subject_group.shared_with_group_links.of_ancestors_and_self.map(&:shared_with_group_id)).to match_array(result) + end + end + end + end end diff --git a/spec/models/groups/feature_setting_spec.rb b/spec/models/groups/feature_setting_spec.rb new file mode 100644 index 00000000000..f1e66744b90 --- /dev/null +++ b/spec/models/groups/feature_setting_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::FeatureSetting do + describe 'associations' do + it { is_expected.to belong_to(:group) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:group) } + end +end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 48d8ba975b6..0f596d3908d 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -60,6 +60,17 @@ RSpec.describe Integration do end describe 'Scopes' do + describe '.third_party_wikis' do + let!(:integration1) { create(:jira_integration) } + let!(:integration2) { create(:redmine_integration) } + let!(:integration3) { create(:confluence_integration) } + let!(:integration4) { create(:shimo_integration) } + + it 'returns the right group integration' do + expect(described_class.third_party_wikis).to contain_exactly(integration3, integration4) + end + end + describe '.with_default_settings' do it 'returns the correct integrations' do instance_integration = create(:integration, :instance) @@ -265,6 +276,20 @@ RSpec.describe Integration do end end + describe '#inheritable?' do + it 'is true for an instance integration' do + expect(create(:integration, :instance)).to be_inheritable + end + + it 'is true for a group integration' do + expect(create(:integration, :group)).to be_inheritable + end + + it 'is false for a project integration' do + expect(create(:integration)).not_to be_inheritable + end + end + describe '.build_from_integration' do context 'when integration is invalid' do let(:invalid_integration) do @@ -462,6 +487,18 @@ RSpec.describe Integration do expect(project.reload.integrations.first.inherit_from_id).to eq(group_integration.id) end + context 'there are multiple inheritable integrations, and a duplicate' do + let!(:group_integration_2) { create(:jenkins_integration, :group, group: group) } + let!(:group_integration_3) { create(:datadog_integration, :instance) } + let!(:duplicate) { create(:jenkins_integration, project: project) } + + it 'returns the number of successfully created integrations' do + expect(described_class.create_from_active_default_integrations(project, :project_id)).to eq 2 + + expect(project.reload.integrations.size).to eq(3) + end + end + context 'passing a group' do let!(:subgroup) { create(:group, parent: group) } @@ -621,6 +658,33 @@ RSpec.describe Integration do end end + describe '#properties=' do + let(:integration_type) do + Class.new(described_class) do + field :foo + field :bar + end + end + + it 'supports indifferent access' do + integration = integration_type.new + + integration.properties = { foo: 1, 'bar' => 2 } + + expect(integration).to have_attributes(foo: 1, bar: 2) + end + end + + describe '#properties' do + it 'is not mutable' do + integration = described_class.new + + integration.properties = { foo: 1, bar: 2 } + + expect { integration.properties[:foo] = 3 }.to raise_error + end + end + describe "{property}_touched?" do let(:integration) do Integrations::Bamboo.create!( @@ -720,7 +784,7 @@ RSpec.describe Integration do describe '#api_field_names' do shared_examples 'api field names' do - it 'filters out sensitive fields' do + it 'filters out secret fields' do safe_fields = %w[some_safe_field safe_field url trojan_gift] expect(fake_integration.new).to have_attributes( @@ -857,7 +921,7 @@ RSpec.describe Integration do end end - describe '#password_fields' do + describe '#secret_fields' do it 'returns all fields with type `password`' do allow(subject).to receive(:fields).and_return([ { name: 'password', type: 'password' }, @@ -865,53 +929,34 @@ RSpec.describe Integration do { name: 'public', type: 'text' } ]) - expect(subject.password_fields).to match_array(%w[password secret]) + expect(subject.secret_fields).to match_array(%w[password secret]) end - it 'returns an empty array if no password fields exist' do - expect(subject.password_fields).to eq([]) + it 'returns an empty array if no secret fields exist' do + expect(subject.secret_fields).to eq([]) end end - describe 'encrypted_properties' do + describe '#to_integration_hash' do let(:properties) { { foo: 1, bar: true } } let(:db_props) { properties.stringify_keys } let(:record) { create(:integration, :instance, properties: properties) } - it 'contains the same data as properties' do - expect(record).to have_attributes( - properties: db_props, - encrypted_properties_tmp: db_props - ) - end - - it 'is persisted' do - encrypted_properties = described_class.id_in(record.id) - - expect(encrypted_properties).to contain_exactly have_attributes(encrypted_properties_tmp: db_props) - end - - it 'is updated when using prop_accessors' do - some_integration = Class.new(described_class) do - prop_accessor :foo - end - - record = some_integration.new - - record.foo = 'the foo' + it 'does not include the properties key' do + hash = record.to_integration_hash - expect(record.encrypted_properties_tmp).to eq({ 'foo' => 'the foo' }) + expect(hash).not_to have_key('properties') end it 'saves correctly using insert_all' do hash = record.to_integration_hash - hash[:project_id] = project + hash[:project_id] = project.id expect do described_class.insert_all([hash]) end.to change(described_class, :count).by(1) - expect(described_class.last).to have_attributes(encrypted_properties_tmp: db_props) + expect(described_class.last).to have_attributes(properties: db_props) end it 'is part of the to_integration_hash' do @@ -921,7 +966,7 @@ RSpec.describe Integration do expect(hash['encrypted_properties']).not_to eq(record.encrypted_properties) expect(hash['encrypted_properties_iv']).not_to eq(record.encrypted_properties_iv) - decrypted = described_class.decrypt(:encrypted_properties_tmp, + decrypted = described_class.decrypt(:properties, hash['encrypted_properties'], { iv: hash['encrypted_properties_iv'] }) @@ -946,7 +991,7 @@ RSpec.describe Integration do end.to change(described_class, :count).by(1) expect(described_class.last).not_to eq record - expect(described_class.last).to have_attributes(encrypted_properties_tmp: db_props) + expect(described_class.last).to have_attributes(properties: db_props) end end end @@ -1021,4 +1066,97 @@ RSpec.describe Integration do ) end end + + describe 'boolean_accessor' do + let(:klass) do + Class.new(Integration) do + boolean_accessor :test_value + end + end + + let(:integration) { klass.new(properties: { test_value: input }) } + + where(:input, :method_result, :predicate_method_result) do + true | true | true + false | false | false + 1 | true | true + 0 | false | false + '1' | true | true + '0' | false | false + 'true' | true | true + 'false' | false | false + 'foobar' | nil | false + '' | nil | false + nil | nil | false + 'on' | true | true + 'off' | false | false + 'yes' | true | true + 'no' | false | false + 'n' | false | false + 'y' | true | true + 't' | true | true + 'f' | false | false + end + + with_them do + it 'has the correct value' do + expect(integration).to have_attributes( + test_value: be(method_result), + test_value?: be(predicate_method_result) + ) + end + end + + it 'returns values when initialized without input' do + integration = klass.new + + expect(integration).to have_attributes( + test_value: be(nil), + test_value?: be(false) + ) + end + end + + describe '#attributes' do + it 'does not include properties' do + expect(create(:integration).attributes).not_to have_key('properties') + end + + it 'can be used in assign_attributes without nullifying properties' do + record = create(:integration, :instance, properties: { url: generate(:url) }) + + attrs = record.attributes + + expect { record.assign_attributes(attrs) }.not_to change(record, :properties) + end + end + + describe '#dup' do + let(:original) { create(:integration, properties: { one: 1, two: 2, three: 3 }) } + + it 'results in distinct ciphertexts, but identical properties' do + copy = original.dup + + expect(copy).to have_attributes(properties: eq(original.properties)) + + expect(copy).not_to have_attributes( + encrypted_properties: eq(original.encrypted_properties) + ) + end + + context 'when the model supports data-fields' do + let(:original) { create(:jira_integration, username: generate(:username), url: generate(:url)) } + + it 'creates distinct but identical data-fields' do + copy = original.dup + + expect(copy).to have_attributes( + username: original.username, + url: original.url + ) + + expect(copy.data_fields).not_to eq(original.data_fields) + end + end + end end diff --git a/spec/models/integrations/base_third_party_wiki_spec.rb b/spec/models/integrations/base_third_party_wiki_spec.rb new file mode 100644 index 00000000000..11e044c2a18 --- /dev/null +++ b/spec/models/integrations/base_third_party_wiki_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::BaseThirdPartyWiki do + describe 'Validations' do + let_it_be_with_reload(:project) { create(:project) } + + describe 'only one third party wiki per project' do + subject(:integration) { create(:shimo_integration, project: project, active: true) } + + before_all do + create(:confluence_integration, project: project, active: true) + end + + context 'when integration is changed manually by user' do + it 'executes the validation' do + valid = integration.valid?(:manual_change) + + expect(valid).to be_falsey + error_message = 'Another third-party wiki is already in use. '\ + 'Only one third-party wiki integration can be active at a time' + expect(integration.errors[:base]).to include _(error_message) + end + end + + context 'when integration is changed internally' do + it 'does not execute the validation' do + expect(integration.valid?).to be_truthy + end + end + + context 'when integration is not on the project level' do + subject(:integration) { create(:shimo_integration, :instance, active: true) } + + it 'executes the validation' do + expect(integration.valid?(:manual_change)).to be_truthy + end + end + end + end +end diff --git a/spec/models/integrations/emails_on_push_spec.rb b/spec/models/integrations/emails_on_push_spec.rb index bdca267f6cb..15aa105e379 100644 --- a/spec/models/integrations/emails_on_push_spec.rb +++ b/spec/models/integrations/emails_on_push_spec.rb @@ -78,9 +78,10 @@ RSpec.describe Integrations::EmailsOnPush do end describe '.valid_recipients' do - let(:recipients) { '<invalid> foobar Valid@recipient.com Dup@lica.te dup@lica.te Dup@Lica.te' } + let(:recipients) { '<invalid> foobar valid@dup@asd Valid@recipient.com Dup@lica.te dup@lica.te Dup@Lica.te' } it 'removes invalid email addresses and removes duplicates by keeping the original capitalization' do + expect(described_class.valid_recipients(recipients)).not_to contain_exactly('valid@dup@asd') expect(described_class.valid_recipients(recipients)).to contain_exactly('Valid@recipient.com', 'Dup@lica.te') end end diff --git a/spec/models/integrations/external_wiki_spec.rb b/spec/models/integrations/external_wiki_spec.rb index e4d6a1c7c84..1621605d39f 100644 --- a/spec/models/integrations/external_wiki_spec.rb +++ b/spec/models/integrations/external_wiki_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Integrations::ExternalWiki do describe 'test' do before do - subject.properties['external_wiki_url'] = url + subject.external_wiki_url = url end let(:url) { 'http://foo' } diff --git a/spec/models/integrations/field_spec.rb b/spec/models/integrations/field_spec.rb index 0d660c4a3ab..c8caf831191 100644 --- a/spec/models/integrations/field_spec.rb +++ b/spec/models/integrations/field_spec.rb @@ -84,17 +84,17 @@ RSpec.describe ::Integrations::Field do end end - describe '#sensitive' do + describe '#secret?' do context 'when empty' do - it { is_expected.not_to be_sensitive } + it { is_expected.not_to be_secret } end - context 'when a password field' do + context 'when a secret field' do before do attrs[:type] = 'password' end - it { is_expected.to be_sensitive } + it { is_expected.to be_secret } end %w[token api_token api_key secret_key secret_sauce password passphrase].each do |name| @@ -103,7 +103,7 @@ RSpec.describe ::Integrations::Field do attrs[:name] = name end - it { is_expected.to be_sensitive } + it { is_expected.to be_secret } end end @@ -112,7 +112,7 @@ RSpec.describe ::Integrations::Field do attrs[:name] = :url end - it { is_expected.not_to be_sensitive } + it { is_expected.not_to be_secret } end end end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 08656bfe543..d244b1d33d5 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -187,7 +187,7 @@ RSpec.describe Integrations::Jira do subject(:integration) { described_class.create!(params) } it 'does not store data into properties' do - expect(integration.properties).to be_nil + expect(integration.properties).to be_empty end it 'stores data in data_fields correctly' do diff --git a/spec/models/integrations/slack_spec.rb b/spec/models/integrations/slack_spec.rb index 9f69f4f51f8..3997d69f947 100644 --- a/spec/models/integrations/slack_spec.rb +++ b/spec/models/integrations/slack_spec.rb @@ -6,12 +6,12 @@ RSpec.describe Integrations::Slack do it_behaves_like Integrations::SlackMattermostNotifier, "Slack" describe '#execute' do + let_it_be(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all') } + before do stub_request(:post, slack_integration.webhook) end - let_it_be(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all') } - it 'uses only known events', :aggregate_failures do described_class::SUPPORTED_EVENTS_FOR_USAGE_LOG.each do |action| expect(Gitlab::UsageDataCounters::HLLRedisCounter.known_event?("i_ecosystem_slack_service_#{action}_notification")).to be true diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 29305ba435c..fe09dadd0db 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -238,6 +238,24 @@ RSpec.describe Issue do end end + context 'order by escalation status' do + let_it_be(:triggered_incident) { create(:incident_management_issuable_escalation_status, :triggered).issue } + let_it_be(:resolved_incident) { create(:incident_management_issuable_escalation_status, :resolved).issue } + let_it_be(:issue_no_status) { create(:issue) } + + describe '.order_escalation_status_asc' do + subject { described_class.order_escalation_status_asc } + + it { is_expected.to eq([triggered_incident, resolved_incident, issue_no_status]) } + end + + describe '.order_escalation_status_desc' do + subject { described_class.order_escalation_status_desc } + + it { is_expected.to eq([resolved_incident, triggered_incident, issue_no_status]) } + end + end + # TODO: Remove when NOT NULL constraint is added to the relationship describe '#work_item_type' do let(:issue) { create(:issue, :incident, project: reusable_project, work_item_type: nil) } @@ -1154,18 +1172,6 @@ RSpec.describe Issue do end end - describe '#hook_attrs' do - it 'delegates to Gitlab::HookData::IssueBuilder#build' do - builder = double - - expect(Gitlab::HookData::IssueBuilder) - .to receive(:new).with(subject).and_return(builder) - expect(builder).to receive(:build) - - subject.hook_attrs - end - end - describe '#check_for_spam?' do let_it_be(:support_bot) { ::User.support_bot } @@ -1314,15 +1320,6 @@ RSpec.describe Issue do subject { create(:issue, updated_at: 1.hour.ago) } end - describe "#labels_hook_attrs" do - let(:label) { create(:label) } - let(:issue) { create(:labeled_issue, project: reusable_project, labels: [label]) } - - it "returns a list of label hook attributes" do - expect(issue.labels_hook_attrs).to eq([label.hook_attrs]) - end - end - context "relative positioning" do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 6cf73de6cef..e1135aa440b 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -123,25 +123,55 @@ RSpec.describe Key, :mailer do end end - context "validation of uniqueness (based on fingerprint uniqueness)" do + context 'validation of uniqueness (based on fingerprint uniqueness)' do let(:user) { create(:user) } - it "accepts the key once" do - expect(build(:key, user: user)).to be_valid + shared_examples 'fingerprint uniqueness' do + it 'accepts the key once' do + expect(build(:rsa_key_4096, user: user)).to be_valid + end + + it 'does not accept the exact same key twice' do + first_key = create(:rsa_key_4096, user: user) + + expect(build(:key, user: user, key: first_key.key)).not_to be_valid + end + + it 'does not accept a duplicate key with a different comment' do + first_key = create(:rsa_key_4096, user: user) + duplicate = build(:key, user: user, key: first_key.key) + duplicate.key << ' extra comment' + + expect(duplicate).not_to be_valid + end end - it "does not accept the exact same key twice" do - first_key = create(:key, user: user) + context 'with FIPS mode off' do + it_behaves_like 'fingerprint uniqueness' + end - expect(build(:key, user: user, key: first_key.key)).not_to be_valid + context 'with FIPS mode', :fips_mode do + it_behaves_like 'fingerprint uniqueness' end + end - it "does not accept a duplicate key with a different comment" do - first_key = create(:key, user: user) - duplicate = build(:key, user: user, key: first_key.key) - duplicate.key << ' extra comment' + context 'fingerprint generation' do + it 'generates both md5 and sha256 fingerprints' do + key = build(:rsa_key_4096) + + expect(key).to be_valid + expect(key.fingerprint).to be_kind_of(String) + expect(key.fingerprint_sha256).to be_kind_of(String) + end - expect(duplicate).not_to be_valid + context 'with FIPS mode', :fips_mode do + it 'generates only sha256 fingerprint' do + key = build(:rsa_key_4096) + + expect(key).to be_valid + expect(key.fingerprint).to be_nil + expect(key.fingerprint_sha256).to be_kind_of(String) + end end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 79491edba94..4ab17ee1e6d 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -682,14 +682,46 @@ RSpec.describe Member do member.accept_invite!(user) end - it "refreshes user's authorized projects", :delete do - project = member.source + context 'authorized projects' do + let(:project) { member.source } - expect(user.authorized_projects).not_to include(project) + before do + expect(user.authorized_projects).not_to include(project) + end - member.accept_invite!(user) + it 'successfully completes a blocking refresh', :delete do + expect(member).to receive(:refresh_member_authorized_projects).with(blocking: true).and_call_original + + member.accept_invite!(user) + + expect(user.authorized_projects.reload).to include(project) + end + + it 'successfully completes a non-blocking refresh', :delete, :sidekiq_inline do + member.blocking_refresh = false + + expect(member).to receive(:refresh_member_authorized_projects).with(blocking: false).and_call_original + + member.accept_invite!(user) + + expect(user.authorized_projects.reload).to include(project) + end - expect(user.authorized_projects.reload).to include(project) + context 'when the feature flag is disabled' do + before do + stub_feature_flags(allow_non_blocking_member_refresh: false) + end + + it 'successfully completes a blocking refresh', :delete, :sidekiq_inline do + member.blocking_refresh = false + + expect(member).to receive(:refresh_member_authorized_projects).with(blocking: true).and_call_original + + member.accept_invite!(user) + + expect(user.authorized_projects.reload).to include(project) + end + end end it 'does not accept the invite if saving a new user fails' do @@ -926,4 +958,64 @@ RSpec.describe Member do end end end + + describe '.sort_by_attribute' do + let_it_be(:user1) { create(:user, created_at: Date.today, last_sign_in_at: Date.today, last_activity_on: Date.today, name: 'Alpha') } + let_it_be(:user2) { create(:user, created_at: Date.today - 1, last_sign_in_at: Date.today - 1, last_activity_on: Date.today - 1, name: 'Omega') } + let_it_be(:user3) { create(:user, created_at: Date.today - 2, name: 'Beta') } + let_it_be(:group) { create(:group) } + let_it_be(:member1) { create(:group_member, :reporter, group: group, user: user1) } + let_it_be(:member2) { create(:group_member, :developer, group: group, user: user2) } + let_it_be(:member3) { create(:group_member, :maintainer, group: group, user: user3) } + + it 'sort users in ascending order by access-level' do + expect(described_class.sort_by_attribute('access_level_asc')).to eq([member1, member2, member3]) + end + + it 'sort users in descending order by access-level' do + expect(described_class.sort_by_attribute('access_level_desc')).to eq([member3, member2, member1]) + end + + context 'when sort by recent_sign_in' do + subject { described_class.sort_by_attribute('recent_sign_in') } + + it 'sorts users by recent sign-in time' do + expect(subject.first).to eq(member1) + expect(subject.second).to eq(member2) + end + + it 'pushes users who never signed in to the end' do + expect(subject.third).to eq(member3) + end + end + + context 'when sort by oldest_sign_in' do + subject { described_class.sort_by_attribute('oldest_sign_in') } + + it 'sorts users by the oldest sign-in time' do + expect(subject.first).to eq(member2) + expect(subject.second).to eq(member1) + end + + it 'pushes users who never signed in to the end' do + expect(subject.third).to eq(member3) + end + end + + it 'sorts users in descending order by their creation time' do + expect(described_class.sort_by_attribute('recent_created_user')).to eq([member1, member2, member3]) + end + + it 'sorts users in ascending order by their creation time' do + expect(described_class.sort_by_attribute('oldest_created_user')).to eq([member3, member2, member1]) + end + + it 'sort users by recent last activity' do + expect(described_class.sort_by_attribute('recent_last_activity')).to eq([member1, member2, member3]) + end + + it 'sort users by oldest last activity' do + expect(described_class.sort_by_attribute('oldest_last_activity')).to eq([member3, member2, member1]) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 0d15851e583..8545c7bc6c7 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1757,18 +1757,6 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe '#hook_attrs' do - it 'delegates to Gitlab::HookData::MergeRequestBuilder#build' do - builder = double - - expect(Gitlab::HookData::MergeRequestBuilder) - .to receive(:new).with(subject).and_return(builder) - expect(builder).to receive(:build) - - subject.hook_attrs - end - end - describe '#diverged_commits_count' do let(:project) { create(:project, :repository) } let(:forked_project) { fork_project(project, nil, repository: true) } @@ -3550,8 +3538,8 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe "#environments" do - subject { merge_request.environments } + describe "#legacy_environments" do + subject { merge_request.legacy_environments } let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') } let(:project) { merge_request.project } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index ebd153f6f10..09ac15429a5 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -2230,4 +2230,10 @@ RSpec.describe Namespace do expect(namespace.storage_enforcement_date).to be(nil) end end + + describe 'serialization' do + let(:object) { build(:namespace) } + + it_behaves_like 'blocks unsafe serialization' + end end diff --git a/spec/models/namespaces/project_namespace_spec.rb b/spec/models/namespaces/project_namespace_spec.rb index 47cf866c143..c995571c3c9 100644 --- a/spec/models/namespaces/project_namespace_spec.rb +++ b/spec/models/namespaces/project_namespace_spec.rb @@ -17,11 +17,11 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do let_it_be(:project) { create(:project) } let_it_be(:project_namespace) { project.project_namespace } - it 'keeps the associated project' do + it 'also deletes associated project' do project_namespace.delete expect { project_namespace.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect(project.reload.project_namespace).to be_nil + expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index cbfedf54ffa..4b262c1f3a9 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -105,6 +105,104 @@ RSpec.describe Note do it { is_expected.to be_valid } end end + + describe 'confidentiality' do + context 'for existing public note' do + let_it_be(:existing_note) { create(:note) } + + it 'is not possible to change the note to confidential' do + existing_note.confidential = true + + expect(existing_note).not_to be_valid + expect(existing_note.errors[:confidential]).to include('can not be changed for existing notes') + end + + it 'is possible to change confidentiality from nil to false' do + existing_note.confidential = false + + expect(existing_note).to be_valid + end + end + + context 'for existing confidential note' do + let_it_be(:existing_note) { create(:note, confidential: true) } + + it 'is not possible to change the note to public' do + existing_note.confidential = false + + expect(existing_note).not_to be_valid + expect(existing_note.errors[:confidential]).to include('can not be changed for existing notes') + end + end + + context 'for a new note' do + let_it_be(:noteable) { create(:issue) } + + let(:note_params) { { confidential: true, noteable: noteable, project: noteable.project } } + + subject { build(:note, **note_params) } + + it 'allows to create a confidential note for an issue' do + expect(subject).to be_valid + end + + context 'when noteable is not allowed to have confidential notes' do + let_it_be(:noteable) { create(:merge_request) } + + it 'can not be set confidential' do + expect(subject).not_to be_valid + expect(subject.errors[:confidential]).to include('can not be set for this resource') + end + end + + context 'when note type is not allowed to be confidential' do + let(:note_params) { { type: 'DiffNote', confidential: true, noteable: noteable, project: noteable.project } } + + it 'can not be set confidential' do + expect(subject).not_to be_valid + expect(subject.errors[:confidential]).to include('can not be set for this type of note') + end + end + + context 'when the note is a discussion note' do + let(:note_params) { { type: 'DiscussionNote', confidential: true, noteable: noteable, project: noteable.project } } + + it { is_expected.to be_valid } + end + + context 'when replying to a note' do + let(:note_params) { { confidential: true, noteable: noteable, project: noteable.project } } + + subject { build(:discussion_note, discussion_id: original_note.discussion_id, **note_params) } + + context 'when the note is reply to a confidential note' do + let_it_be(:original_note) { create(:note, confidential: true, noteable: noteable, project: noteable.project) } + + it { is_expected.to be_valid } + end + + context 'when the note is reply to a public note' do + let_it_be(:original_note) { create(:note, noteable: noteable, project: noteable.project) } + + it 'can not be set confidential' do + expect(subject).not_to be_valid + expect(subject.errors[:confidential]).to include('reply should have same confidentiality as top-level note') + end + end + + context 'when reply note is public but discussion is confidential' do + let_it_be(:original_note) { create(:note, confidential: true, noteable: noteable, project: noteable.project) } + + let(:note_params) { { noteable: noteable, project: noteable.project } } + + it 'can not be set confidential' do + expect(subject).not_to be_valid + expect(subject.errors[:confidential]).to include('reply should have same confidentiality as top-level note') + end + end + end + end + end end describe 'callbacks' do @@ -1169,8 +1267,8 @@ RSpec.describe Note do end describe "#discussion" do - let!(:note1) { create(:discussion_note_on_merge_request) } - let!(:note2) { create(:diff_note_on_merge_request, project: note1.project, noteable: note1.noteable) } + let_it_be(:note1) { create(:discussion_note_on_merge_request) } + let_it_be(:note2) { create(:diff_note_on_merge_request, project: note1.project, noteable: note1.noteable) } context 'when the note is part of a discussion' do subject { create(:discussion_note_on_merge_request, project: note1.project, noteable: note1.noteable, in_reply_to: note1) } @@ -1655,4 +1753,27 @@ RSpec.describe Note do expect(note.commands_changes.keys).to contain_exactly(:emoji_award, :time_estimate, :spend_time) end end + + describe '#bump_updated_at', :freeze_time do + it 'sets updated_at to the current timestamp' do + note = create(:note, updated_at: 1.day.ago) + + note.bump_updated_at + note.reload + + expect(note.updated_at).to be_like_time(Time.current) + end + + context 'with legacy edited note' do + it 'copies updated_at to last_edited_at before bumping the timestamp' do + note = create(:note, updated_at: 1.day.ago, updated_by: create(:user), last_edited_at: nil) + + note.bump_updated_at + note.reload + + expect(note.last_edited_at).to be_like_time(1.day.ago) + expect(note.updated_at).to be_like_time(Time.current) + end + end + end end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index fd453d8e5a9..f6af8f6a951 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -23,6 +23,28 @@ RSpec.describe Packages::PackageFile, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:package) } + + context 'with pypi package' do + let_it_be(:package) { create(:pypi_package) } + + let(:package_file) { package.package_files.first } + let(:status) { :default } + let(:file) { fixture_file_upload('spec/fixtures/dk.png') } + + subject { package.package_files.create!(file: file, file_name: package_file.file_name, status: status) } + + it 'can not save a duplicated file' do + expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: File name has already been taken") + end + + context 'with a pending destruction package duplicated file' do + let(:status) { :pending_destruction } + + it 'can save it' do + expect { subject }.to change { package.package_files.count }.from(1).to(2) + end + end + end end context 'with package filenames' do diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 52ed52de193..6c86db1197f 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1021,13 +1021,13 @@ RSpec.describe Packages::Package, type: :model do context 'ascending direction' do let(:direction) { :asc } - it { is_expected.to eq('projects.name asc NULLS LAST, "packages_packages"."id" ASC') } + it { is_expected.to eq('"projects"."name" ASC NULLS LAST, "packages_packages"."id" ASC') } end context 'descending direction' do let(:direction) { :desc } - it { is_expected.to eq('projects.name desc NULLS FIRST, "packages_packages"."id" DESC') } + it { is_expected.to eq('"projects"."name" DESC NULLS FIRST, "packages_packages"."id" DESC') } end end end diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index 72fda2280e5..381e42978f4 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -214,6 +214,7 @@ RSpec.describe PlanLimits do daily_invites web_hook_calls ci_daily_pipeline_schedule_triggers + repository_size ] + disabled_max_artifact_size_columns end diff --git a/spec/models/preloaders/environments/deployment_preloader_spec.rb b/spec/models/preloaders/environments/deployment_preloader_spec.rb index 3f2f28a069e..4c05d9632de 100644 --- a/spec/models/preloaders/environments/deployment_preloader_spec.rb +++ b/spec/models/preloaders/environments/deployment_preloader_spec.rb @@ -6,14 +6,14 @@ RSpec.describe Preloaders::Environments::DeploymentPreloader do let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project, :repository) } - let_it_be(:pipeline) { create(:ci_pipeline, user: user, project: project, sha: project.commit.sha) } - let_it_be(:ci_build_a) { create(:ci_build, user: user, project: project, pipeline: pipeline) } - let_it_be(:ci_build_b) { create(:ci_build, user: user, project: project, pipeline: pipeline) } - let_it_be(:ci_build_c) { create(:ci_build, user: user, project: project, pipeline: pipeline) } - let_it_be(:environment_a) { create(:environment, project: project, state: :available) } let_it_be(:environment_b) { create(:environment, project: project, state: :available) } + let_it_be(:pipeline) { create(:ci_pipeline, user: user, project: project, sha: project.commit.sha) } + let_it_be(:ci_build_a) { create(:ci_build, user: user, project: project, pipeline: pipeline, environment: environment_a.name) } + let_it_be(:ci_build_b) { create(:ci_build, user: user, project: project, pipeline: pipeline, environment: environment_a.name) } + let_it_be(:ci_build_c) { create(:ci_build, user: user, project: project, pipeline: pipeline, environment: environment_b.name) } + before do create(:deployment, :success, project: project, environment: environment_a, deployable: ci_build_a) create(:deployment, :success, project: project, environment: environment_a, deployable: ci_build_b) diff --git a/spec/models/preloaders/group_root_ancestor_preloader_spec.rb b/spec/models/preloaders/group_root_ancestor_preloader_spec.rb new file mode 100644 index 00000000000..0d622e84ef1 --- /dev/null +++ b/spec/models/preloaders/group_root_ancestor_preloader_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::GroupRootAncestorPreloader do + let_it_be(:user) { create(:user) } + let_it_be(:root_parent1) { create(:group, :private, name: 'root-1', path: 'root-1') } + let_it_be(:root_parent2) { create(:group, :private, name: 'root-2', path: 'root-2') } + let_it_be(:guest_group) { create(:group, name: 'public guest', path: 'public-guest') } + let_it_be(:private_maintainer_group) { create(:group, :private, name: 'b private maintainer', path: 'b-private-maintainer', parent: root_parent1) } + let_it_be(:private_developer_group) { create(:group, :private, project_creation_level: nil, name: 'c public developer', path: 'c-public-developer') } + let_it_be(:public_maintainer_group) { create(:group, :private, name: 'a public maintainer', path: 'a-public-maintainer', parent: root_parent2) } + + let(:root_query_regex) { /\ASELECT.+FROM "namespaces" WHERE "namespaces"."id" = \d+/ } + let(:additional_preloads) { [] } + let(:groups) { [guest_group, private_maintainer_group, private_developer_group, public_maintainer_group] } + let(:pristine_groups) { Group.where(id: groups) } + + shared_examples 'executes N matching DB queries' do |expected_query_count, query_method = nil| + it 'executes the specified root_ancestor queries' do + expect do + pristine_groups.each do |group| + root_ancestor = group.root_ancestor + + root_ancestor.public_send(query_method) if query_method.present? + end + end.to make_queries_matching(root_query_regex, expected_query_count) + end + + it 'strong_memoizes the correct root_ancestor' do + pristine_groups.each do |group| + expected_parent_id = group.root_ancestor.id == group.id ? nil : group.root_ancestor.id + + expect(group.parent_id).to eq(expected_parent_id) + end + end + end + + context 'when the preloader is used' do + before do + preload_ancestors + end + + context 'when no additional preloads are provided' do + it_behaves_like 'executes N matching DB queries', 0 + end + + context 'when additional preloads are provided' do + let(:additional_preloads) { [:route] } + let(:root_query_regex) { /\ASELECT.+FROM "routes" WHERE "routes"."source_id" = \d+/ } + + it_behaves_like 'executes N matching DB queries', 0, :full_path + end + end + + context 'when the preloader is not used' do + it_behaves_like 'executes N matching DB queries', 2 + end + + def preload_ancestors + described_class.new(pristine_groups, additional_preloads).execute + end +end diff --git a/spec/models/programming_language_spec.rb b/spec/models/programming_language_spec.rb index f2201eabd1c..b202c10e30b 100644 --- a/spec/models/programming_language_spec.rb +++ b/spec/models/programming_language_spec.rb @@ -10,4 +10,22 @@ RSpec.describe ProgrammingLanguage do it { is_expected.to allow_value("#000000").for(:color) } it { is_expected.not_to allow_value("000000").for(:color) } it { is_expected.not_to allow_value("#0z0000").for(:color) } + + describe '.with_name_case_insensitive scope' do + let_it_be(:ruby) { create(:programming_language, name: 'Ruby') } + let_it_be(:python) { create(:programming_language, name: 'Python') } + let_it_be(:swift) { create(:programming_language, name: 'Swift') } + + it 'accepts a single name parameter' do + expect(described_class.with_name_case_insensitive('swift')).to( + contain_exactly(swift) + ) + end + + it 'accepts multiple names' do + expect(described_class.with_name_case_insensitive('ruby', 'python')).to( + contain_exactly(ruby, python) + ) + end + end end diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 75e43ed9a67..941f6c0a49d 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' RSpec.describe ProjectFeature do using RSpec::Parameterized::TableSyntax - let(:project) { create(:project) } - let(:user) { create(:user) } + let_it_be_with_reload(:project) { create(:project) } + let_it_be(:user) { create(:user) } it { is_expected.to belong_to(:project) } @@ -242,4 +242,95 @@ RSpec.describe ProjectFeature do end end end + + # rubocop:disable Gitlab/FeatureAvailableUsage + describe '#feature_available?' do + let(:features) { ProjectFeature::FEATURES } + + context 'when features are disabled' do + it 'returns false' do + update_all_project_features(project, features, ProjectFeature::DISABLED) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" + end + end + end + + context 'when features are enabled only for team members' do + it 'returns false when user is not a team member' do + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" + end + end + + it 'returns true when user is a team member' do + project.add_developer(user) + + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(true) + end + end + + it 'returns true when user is a member of project group' do + group = create(:group) + project = create(:project, namespace: group) + group.add_developer(user) + + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(true) + end + end + + context 'when admin mode is enabled', :enable_admin_mode do + it 'returns true if user is an admin' do + user.update_attribute(:admin, true) + + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(true) + end + end + end + + context 'when admin mode is disabled' do + it 'returns false when user is an admin' do + user.update_attribute(:admin, true) + + update_all_project_features(project, features, ProjectFeature::PRIVATE) + + features.each do |feature| + expect(project.feature_available?(feature.to_sym, user)).to eq(false), "#{feature} failed" + end + end + end + end + + context 'when feature is enabled for everyone' do + it 'returns true' do + expect(project.feature_available?(:issues, user)).to eq(true) + end + end + + context 'when feature has any other value' do + it 'returns true' do + project.project_feature.update_attribute(:issues_access_level, 200) + + expect(project.feature_available?(:issues)).to eq(true) + end + end + + def update_all_project_features(project, features, value) + project_feature_attributes = features.to_h { |f| ["#{f}_access_level", value] } + project.project_feature.update!(project_feature_attributes) + end + end + # rubocop:enable Gitlab/FeatureAvailableUsage end diff --git a/spec/models/project_import_state_spec.rb b/spec/models/project_import_state_spec.rb index 4ad2446f8d0..42ca8130734 100644 --- a/spec/models/project_import_state_spec.rb +++ b/spec/models/project_import_state_spec.rb @@ -22,7 +22,7 @@ RSpec.describe ProjectImportState, type: :model do before do allow_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository) - .with(project.import_url).and_return(true) + .with(project.import_url, http_authorization_header: '', mirror: false).and_return(true) # Works around https://github.com/rspec/rspec-mocks/issues/910 allow(Project).to receive(:find).with(project.id).and_return(project) @@ -89,19 +89,6 @@ RSpec.describe ProjectImportState, type: :model do import_state.mark_as_failed(error_message) end.to change { project.reload.import_data }.from(import_data).to(nil) end - - context 'when remove_import_data_on_failure feature flag is disabled' do - it 'removes project import data' do - stub_feature_flags(remove_import_data_on_failure: false) - - project = create(:project, import_data: ProjectImportData.new(data: { 'test' => 'some data' })) - import_state = create(:import_state, :started, project: project) - - expect do - import_state.mark_as_failed(error_message) - end.not_to change { project.reload.import_data } - end - end end describe '#human_status_name' do @@ -114,6 +101,34 @@ RSpec.describe ProjectImportState, type: :model do end end + describe '#expire_etag_cache' do + context 'when project import type has realtime changes endpoint' do + before do + import_state.project.import_type = 'github' + end + + it 'expires revelant etag cache' do + expect_next_instance_of(Gitlab::EtagCaching::Store) do |instance| + expect(instance).to receive(:touch).with(Gitlab::Routing.url_helpers.realtime_changes_import_github_path(format: :json)) + end + + subject.expire_etag_cache + end + end + + context 'when project import type does not have realtime changes endpoint' do + before do + import_state.project.import_type = 'jira' + end + + it 'does not touch etag caches' do + expect(Gitlab::EtagCaching::Store).not_to receive(:new) + + subject.expire_etag_cache + end + end + end + describe 'import state transitions' do context 'state transition: [:started] => [:finished]' do let(:after_import_service) { spy(:after_import_service) } @@ -191,4 +206,20 @@ RSpec.describe ProjectImportState, type: :model do end end end + + describe 'callbacks' do + context 'after_commit :expire_etag_cache' do + before do + import_state.project.import_type = 'github' + end + + it 'expires etag cache' do + expect_next_instance_of(Gitlab::EtagCaching::Store) do |instance| + expect(instance).to receive(:touch).with(Gitlab::Routing.url_helpers.realtime_changes_import_github_path(format: :json)) + end + + subject.save! + end + end + end end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 5572304d666..d03eb3c8bfe 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -4,4 +4,34 @@ require 'spec_helper' RSpec.describe ProjectSetting, type: :model do it { is_expected.to belong_to(:project) } + + describe 'validations' do + it { is_expected.not_to allow_value(nil).for(:target_platforms) } + it { is_expected.to allow_value([]).for(:target_platforms) } + + it 'allows any combination of the allowed target platforms' do + valid_target_platform_combinations.each do |target_platforms| + expect(subject).to allow_value(target_platforms).for(:target_platforms) + end + end + + [nil, 'not_allowed', :invalid].each do |invalid_value| + it { is_expected.not_to allow_value([invalid_value]).for(:target_platforms) } + end + end + + describe 'target_platforms=' do + it 'stringifies and sorts' do + project_setting = build(:project_setting, target_platforms: [:watchos, :ios]) + expect(project_setting.target_platforms).to eq %w(ios watchos) + end + end + + def valid_target_platform_combinations + target_platforms = described_class::ALLOWED_TARGET_PLATFORMS + + 0.upto(target_platforms.size).flat_map do |n| + target_platforms.permutation(n).to_a + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fc7ac35ed41..0bb584845c2 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -536,7 +536,7 @@ RSpec.describe Project, factory_default: :keep do project = build(:project) aggregate_failures do - urls_with_CRLF.each do |url| + urls_with_crlf.each do |url| project.import_url = url expect(project).not_to be_valid @@ -549,7 +549,7 @@ RSpec.describe Project, factory_default: :keep do project = build(:project) aggregate_failures do - valid_urls_with_CRLF.each do |url| + valid_urls_with_crlf.each do |url| project.import_url = url expect(project).to be_valid @@ -635,6 +635,8 @@ RSpec.describe Project, factory_default: :keep do end end + it_behaves_like 'a BulkUsersByEmailLoad model' + describe '#all_pipelines' do let_it_be(:project) { create(:project) } @@ -724,6 +726,33 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#personal_namespace_holder?' do + let_it_be(:group) { create(:group) } + let_it_be(:namespace_user) { create(:user) } + let_it_be(:admin_user) { create(:user, :admin) } + let_it_be(:personal_project) { create(:project, namespace: namespace_user.namespace) } + let_it_be(:group_project) { create(:project, group: group) } + let_it_be(:another_user) { create(:user) } + let_it_be(:group_owner_user) { create(:user).tap { |user| group.add_owner(user) } } + + where(:project, :user, :result) do + ref(:personal_project) | ref(:namespace_user) | true + ref(:personal_project) | ref(:admin_user) | false + ref(:personal_project) | ref(:another_user) | false + ref(:personal_project) | nil | false + ref(:group_project) | ref(:namespace_user) | false + ref(:group_project) | ref(:group_owner_user) | false + ref(:group_project) | ref(:another_user) | false + ref(:group_project) | nil | false + ref(:group_project) | nil | false + ref(:group_project) | ref(:admin_user) | false + end + + with_them do + it { expect(project.personal_namespace_holder?(user)).to eq(result) } + end + end + describe '#default_pipeline_lock' do let(:project) { build_stubbed(:project) } @@ -1189,29 +1218,8 @@ RSpec.describe Project, factory_default: :keep do end describe 'last_activity_date' do - it 'returns the creation date of the project\'s last event if present' do - new_event = create(:event, :closed, project: project, created_at: Time.current) - - project.reload - expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i) - end - - it 'returns the project\'s last update date if it has no events' do - expect(project.last_activity_date).to eq(project.updated_at) - end - - it 'returns the most recent timestamp' do - project.update!(updated_at: nil, - last_activity_at: timestamp, - last_repository_updated_at: timestamp - 1.hour) - - expect(project.last_activity_date).to be_like_time(timestamp) - - project.update!(updated_at: timestamp, - last_activity_at: timestamp - 1.hour, - last_repository_updated_at: nil) - - expect(project.last_activity_date).to be_like_time(timestamp) + it 'returns the project\'s last update date' do + expect(project.last_activity_date).to be_like_time(project.updated_at) end end end @@ -1688,15 +1696,27 @@ RSpec.describe Project, factory_default: :keep do end describe '.sort_by_attribute' do - it 'reorders the input relation by start count desc' do - project1 = create(:project, star_count: 2) - project2 = create(:project, star_count: 1) - project3 = create(:project) + let_it_be(:project1) { create(:project, star_count: 2, updated_at: 1.minute.ago) } + let_it_be(:project2) { create(:project, star_count: 1) } + let_it_be(:project3) { create(:project, updated_at: 2.minutes.ago) } + it 'reorders the input relation by start count desc' do projects = described_class.sort_by_attribute(:stars_desc) expect(projects).to eq([project1, project2, project3]) end + + it 'reorders the input relation by last activity desc' do + projects = described_class.sort_by_attribute(:latest_activity_desc) + + expect(projects).to eq([project2, project1, project3]) + end + + it 'reorders the input relation by last activity asc' do + projects = described_class.sort_by_attribute(:latest_activity_asc) + + expect(projects).to eq([project3, project1, project2]) + end end describe '.with_shared_runners' do @@ -2273,6 +2293,44 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#pages_show_onboarding?' do + let(:project) { create(:project) } + + subject { project.pages_show_onboarding? } + + context "if there is no metadata" do + it { is_expected.to be_truthy } + end + + context 'if onboarding is complete' do + before do + project.pages_metadatum.update_column(:onboarding_complete, true) + end + + it { is_expected.to be_falsey } + end + + context 'if there is metadata, but onboarding is not complete' do + before do + project.pages_metadatum.update_column(:onboarding_complete, false) + end + + it { is_expected.to be_truthy } + end + + # During migration, the onboarding_complete property can still be false, + # but will be updated later. To account for that case, pages_show_onboarding? + # should return false if `deployed` is true. + context "will return false if pages is deployed even if onboarding_complete is false" do + before do + project.pages_metadatum.update_column(:onboarding_complete, false) + project.pages_metadatum.update_column(:deployed, true) + end + + it { is_expected.to be_falsey } + end + end + describe '#pages_deployed?' do let(:project) { create(:project) } @@ -2695,6 +2753,39 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#container_repositories_size' do + let(:project) { build(:project) } + + subject { project.container_repositories_size } + + context 'on gitlab.com' do + where(:no_container_repositories, :all_migrated, :gitlab_api_supported, :returned_size, :expected_result) do + true | nil | nil | nil | 0 + false | false | nil | nil | nil + false | true | false | nil | nil + false | true | true | 555 | 555 + false | true | true | nil | nil + end + + with_them do + before do + stub_container_registry_config(enabled: true, api_url: 'http://container-registry', key: 'spec/fixtures/x509_certificate_pk.key') + allow(Gitlab).to receive(:com?).and_return(true) + allow(project.container_repositories).to receive(:empty?).and_return(no_container_repositories) + allow(project.container_repositories).to receive(:all_migrated?).and_return(all_migrated) + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(gitlab_api_supported) + allow(ContainerRegistry::GitlabApiClient).to receive(:deduplicated_size).with(project.full_path).and_return(returned_size) + end + + it { is_expected.to eq(expected_result) } + end + end + + context 'not on gitlab.com' do + it { is_expected.to eq(nil) } + end + end + describe '#container_registry_enabled=' do let_it_be_with_reload(:project) { create(:project) } @@ -5602,6 +5693,18 @@ RSpec.describe Project, factory_default: :keep do expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) end end + + describe 'project target platforms detection' do + before do + create(:import_state, :started, project: project) + end + + it 'calls enqueue_record_project_target_platforms' do + expect(project).to receive(:enqueue_record_project_target_platforms) + + project.after_import + end + end end describe '#update_project_counter_caches' do @@ -6256,6 +6359,10 @@ RSpec.describe Project, factory_default: :keep do expect(subject.find_or_initialize_integration('prometheus')).to be_nil end + it 'returns nil if integration does not exist' do + expect(subject.find_or_initialize_integration('non-existing')).to be_nil + end + context 'with an existing integration' do subject { create(:project) } @@ -6557,6 +6664,25 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#mark_pages_onboarding_complete' do + let(:project) { create(:project) } + + it "creates new record and sets onboarding_complete to true if none exists yet" do + project.mark_pages_onboarding_complete + + expect(project.pages_metadatum.reload.onboarding_complete).to eq(true) + end + + it "overrides an existing setting" do + pages_metadatum = project.pages_metadatum + pages_metadatum.update!(onboarding_complete: false) + + expect do + project.mark_pages_onboarding_complete + end.to change { pages_metadatum.reload.onboarding_complete }.from(false).to(true) + end + end + describe '#mark_pages_as_deployed' do let(:project) { create(:project) } @@ -8009,12 +8135,112 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#work_items_feature_flag_enabled?' do + shared_examples 'project checking work_items feature flag' do + context 'when work_items FF is disabled globally' do + before do + stub_feature_flags(work_items: false) + end + + it { is_expected.to be_falsey } + end + + context 'when work_items FF is enabled for the project' do + before do + stub_feature_flags(work_items: project) + end + + it { is_expected.to be_truthy } + end + + context 'when work_items FF is enabled globally' do + it { is_expected.to be_truthy } + end + end + + subject { project.work_items_feature_flag_enabled? } + + context 'when a project does not belong to a group' do + let_it_be(:project) { create(:project, namespace: namespace) } + + it_behaves_like 'project checking work_items feature flag' + end + + context 'when project belongs to a group' 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) } + + it_behaves_like 'project checking work_items feature flag' + + context 'when work_items FF is enabled for the root group' do + before do + stub_feature_flags(work_items: root_group) + end + + it { is_expected.to be_truthy } + end + + context 'when work_items FF is enabled for the group' do + before do + stub_feature_flags(work_items: group) + end + + it { is_expected.to be_truthy } + end + end + end + describe 'serialization' do let(:object) { build(:project) } it_behaves_like 'blocks unsafe serialization' end + describe '#enqueue_record_project_target_platforms' do + let_it_be(:project) { create(:project) } + + let(:com) { true } + + before do + allow(Gitlab).to receive(:com?).and_return(com) + end + + it 'enqueues a Projects::RecordTargetPlatformsWorker' do + expect(Projects::RecordTargetPlatformsWorker).to receive(:perform_async).with(project.id) + + project.enqueue_record_project_target_platforms + end + + shared_examples 'does not enqueue a Projects::RecordTargetPlatformsWorker' do + it 'does not enqueue a Projects::RecordTargetPlatformsWorker' do + expect(Projects::RecordTargetPlatformsWorker).not_to receive(:perform_async) + + project.enqueue_record_project_target_platforms + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(record_projects_target_platforms: false) + end + + it_behaves_like 'does not enqueue a Projects::RecordTargetPlatformsWorker' + end + + context 'when not in gitlab.com' do + let(:com) { false } + + it_behaves_like 'does not enqueue a Projects::RecordTargetPlatformsWorker' + end + end + + describe '#inactive?' do + let_it_be_with_reload(:project) { create(:project, name: 'test-project') } + + it_behaves_like 'returns true if project is inactive' + end + private def finish_job(export_job) diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 5fbf1a9c502..20fc14113ef 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -35,7 +35,8 @@ RSpec.describe ProjectStatistics do build_artifacts_size: 1.exabyte, snippets_size: 1.exabyte, pipeline_artifacts_size: 512.petabytes - 1, - uploads_size: 512.petabytes + uploads_size: 512.petabytes, + container_registry_size: 8.exabytes - 1 ) statistics.reload @@ -49,6 +50,7 @@ RSpec.describe ProjectStatistics do expect(statistics.snippets_size).to eq(1.exabyte) expect(statistics.pipeline_artifacts_size).to eq(512.petabytes - 1) expect(statistics.uploads_size).to eq(512.petabytes) + expect(statistics.container_registry_size).to eq(8.exabytes - 1) 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 22c27c986f8..a55e4b31d21 100644 --- a/spec/models/projects/build_artifacts_size_refresh_spec.rb +++ b/spec/models/projects/build_artifacts_size_refresh_spec.rb @@ -14,13 +14,13 @@ 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: 4.days.ago) } - let_it_be(:refresh_2) { create(:project_build_artifacts_size_refresh, :running, updated_at: 2.days.ago) } + 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_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) } describe 'stale' do - it 'returns records in running state and has not been updated for more than 3 days' do + it 'returns records in running state and has not been updated for more than 2 hours' do expect(described_class.stale).to eq([refresh_1]) end end diff --git a/spec/models/projects/topic_spec.rb b/spec/models/projects/topic_spec.rb index aa3230da1e6..8fc4d11f0d9 100644 --- a/spec/models/projects/topic_spec.rb +++ b/spec/models/projects/topic_spec.rb @@ -56,6 +56,14 @@ RSpec.describe Projects::Topic do end end + describe '#find_by_name_case_insensitive' do + it 'returns topic with case insensitive name' do + %w(topic TOPIC Topic).each do |name| + expect(described_class.find_by_name_case_insensitive(name)).to eq(topic) + end + end + end + describe '#search' do it 'returns topics with a matching name' do expect(described_class.search(topic.name)).to eq([topic]) diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index d4491aacd9f..2492521c634 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -5,6 +5,48 @@ require 'spec_helper' RSpec.describe UserPreference do let(:user_preference) { create(:user_preference) } + describe 'validations' do + describe 'diffs_deletion_color and diffs_addition_color' do + using RSpec::Parameterized::TableSyntax + + where(color: [ + '#000000', + '#123456', + '#abcdef', + '#AbCdEf', + '#ffffff', + '#fFfFfF', + '#000', + '#123', + '#abc', + '#AbC', + '#fff', + '#fFf', + '' + ]) + + with_them do + it { is_expected.to allow_value(color).for(:diffs_deletion_color) } + it { is_expected.to allow_value(color).for(:diffs_addition_color) } + end + + where(color: [ + '#1', + '#12', + '#1234', + '#12345', + '#1234567', + '123456', + '#12345x' + ]) + + with_them do + it { is_expected.not_to allow_value(color).for(:diffs_deletion_color) } + it { is_expected.not_to allow_value(color).for(:diffs_addition_color) } + end + end + end + describe 'notes filters global keys' do it 'contains expected values' do expect(UserPreference::NOTES_FILTERS.keys).to match_array([:all_notes, :only_comments, :only_activity]) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6ee38048025..bc425b15c6e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -69,6 +69,12 @@ RSpec.describe User do it { is_expected.to delegate_method(:markdown_surround_selection).to(:user_preference) } it { is_expected.to delegate_method(:markdown_surround_selection=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:diffs_deletion_color).to(:user_preference) } + it { is_expected.to delegate_method(:diffs_deletion_color=).to(:user_preference).with_arguments(:args) } + + it { is_expected.to delegate_method(:diffs_addition_color).to(:user_preference) } + it { is_expected.to delegate_method(:diffs_addition_color=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:job_title).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:job_title=).to(:user_detail).with_arguments(:args).allow_nil } @@ -1554,7 +1560,7 @@ RSpec.describe User do end it 'adds the confirmed primary email to emails' do - expect(user.emails.confirmed.map(&:email)).not_to include(user.email) + expect(user.emails.confirmed.map(&:email)).not_to include(user.unconfirmed_email) user.confirm @@ -1613,14 +1619,23 @@ RSpec.describe User do context 'when the email is changed but not confirmed' do let(:user) { create(:user, email: 'primary@example.com') } - it 'does not add the new email to emails yet' do + before do user.update!(email: 'new_primary@example.com') + end + it 'does not add the new email to emails yet' do expect(user.unconfirmed_email).to eq('new_primary@example.com') expect(user.email).to eq('primary@example.com') expect(user).to be_confirmed expect(user.emails.pluck(:email)).not_to include('new_primary@example.com') end + + it 'adds the new email to emails upon confirmation' do + user.confirm + expect(user.email).to eq('new_primary@example.com') + expect(user).to be_confirmed + expect(user.emails.pluck(:email)).to include('new_primary@example.com') + end end context 'when the user is created as not confirmed' do @@ -1630,6 +1645,11 @@ RSpec.describe User do expect(user).not_to be_confirmed expect(user.emails.pluck(:email)).not_to include('primary@example.com') end + + it 'adds the email to emails upon confirmation' do + user.confirm + expect(user.emails.pluck(:email)).to include('primary@example.com') + end end context 'when the user is created as confirmed' do @@ -2083,6 +2103,74 @@ RSpec.describe User do end end + describe 'needs_new_otp_secret?', :freeze_time do + let(:user) { create(:user) } + + context 'when two-factor is not enabled' do + it 'returns true if otp_secret_expires_at is nil' do + expect(user.needs_new_otp_secret?).to eq(true) + end + + it 'returns true if the otp_secret_expires_at has passed' do + user.update!(otp_secret_expires_at: 10.minutes.ago) + + expect(user.reload.needs_new_otp_secret?).to eq(true) + end + + it 'returns false if the otp_secret_expires_at has not passed' do + user.update!(otp_secret_expires_at: 10.minutes.from_now) + + expect(user.reload.needs_new_otp_secret?).to eq(false) + end + end + + context 'when two-factor is enabled' do + let(:user) { create(:user, :two_factor) } + + it 'returns false even if ttl is expired' do + user.otp_secret_expires_at = 10.minutes.ago + + expect(user.needs_new_otp_secret?).to eq(false) + end + end + end + + describe 'otp_secret_expired?', :freeze_time do + let(:user) { create(:user) } + + it 'returns true if otp_secret_expires_at is nil' do + expect(user.otp_secret_expired?).to eq(true) + end + + it 'returns true if the otp_secret_expires_at has passed' do + user.otp_secret_expires_at = 10.minutes.ago + + expect(user.otp_secret_expired?).to eq(true) + end + + it 'returns false if the otp_secret_expires_at has not passed' do + user.otp_secret_expires_at = 20.minutes.from_now + + expect(user.otp_secret_expired?).to eq(false) + end + end + + describe 'update_otp_secret!', :freeze_time do + let(:user) { create(:user) } + + before do + user.update_otp_secret! + end + + it 'sets the otp_secret' do + expect(user.otp_secret).to have_attributes(length: described_class::OTP_SECRET_LENGTH) + end + + it 'updates the otp_secret_expires_at' do + expect(user.otp_secret_expires_at).to eq(Time.current + described_class::OTP_SECRET_TTL) + end + end + describe 'projects' do before do @user = create(:user) @@ -2653,6 +2741,19 @@ RSpec.describe User do let_it_be(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@example.com') } let_it_be(:email) { create(:email, user: user, email: 'alias@example.com') } + describe 'name user and email relative ordering' do + let_it_be(:named_alexander) { create(:user, name: 'Alexander Person', username: 'abcd', email: 'abcd@example.com') } + let_it_be(:username_alexand) { create(:user, name: 'Joao Alexander', username: 'Alexand', email: 'joao@example.com') } + + it 'prioritizes exact matches' do + expect(described_class.search('Alexand')).to eq([username_alexand, named_alexander]) + end + + it 'falls back to ordering by name' do + expect(described_class.search('Alexander')).to eq([named_alexander, username_alexand]) + end + end + describe 'name matching' do it 'returns users with a matching name with exact match first' do expect(described_class.search(user.name)).to eq([user, user2]) @@ -4251,16 +4352,34 @@ RSpec.describe User do end end - it_behaves_like '#ci_owned_runners' + describe '#ci_owned_runners' do + it_behaves_like '#ci_owned_runners' - context 'when FF ci_owned_runners_cross_joins_fix is disabled' do - before do - skip_if_multiple_databases_are_setup + context 'when FF use_traversal_ids is disabled fallbacks to inefficient implementation' do + before do + stub_feature_flags(use_traversal_ids: false) + end - stub_feature_flags(ci_owned_runners_cross_joins_fix: false) + it_behaves_like '#ci_owned_runners' end - it_behaves_like '#ci_owned_runners' + context 'when FF ci_owned_runners_cross_joins_fix is disabled' do + before do + skip_if_multiple_databases_are_setup + + stub_feature_flags(ci_owned_runners_cross_joins_fix: false) + end + + it_behaves_like '#ci_owned_runners' + end + + context 'when FF ci_owned_runners_unnest_index is disabled uses GIN index' do + before do + stub_feature_flags(ci_owned_runners_unnest_index: false) + end + + it_behaves_like '#ci_owned_runners' + end end describe '#projects_with_reporter_access_limited_to' do @@ -4882,17 +5001,36 @@ RSpec.describe User do end describe '#attention_requested_open_merge_requests_count' do - it 'returns number of open merge requests from non-archived projects' do - user = create(:user) - project = create(:project, :public) - archived_project = create(:project, :public, :archived) + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:archived_project) { create(:project, :public, :archived) } + before do create(:merge_request, source_project: project, author: user, reviewers: [user]) create(:merge_request, :closed, source_project: project, author: user, reviewers: [user]) create(:merge_request, source_project: archived_project, author: user, reviewers: [user]) + end + it 'returns number of open merge requests from non-archived projects' do + expect(Rails.cache).not_to receive(:fetch) expect(user.attention_requested_open_merge_requests_count(force: true)).to eq 1 end + + context 'when uncached_mr_attention_requests_count is disabled' do + before do + stub_feature_flags(uncached_mr_attention_requests_count: false) + end + + it 'fetches from cache' do + expect(Rails.cache).to receive(:fetch).with( + user.attention_request_cache_key, + force: false, + expires_in: described_class::COUNT_CACHE_VALIDITY_PERIOD + ).and_call_original + + expect(user.attention_requested_open_merge_requests_count).to eq 1 + end + end end describe '#assigned_open_issues_count' do @@ -6632,6 +6770,23 @@ RSpec.describe User do end end + describe '.without_forbidden_states' do + let_it_be(:normal_user) { create(:user, username: 'johndoe') } + let_it_be(:admin_user) { create(:user, :admin, username: 'iamadmin') } + let_it_be(:blocked_user) { create(:user, :blocked, username: 'notsorandom') } + let_it_be(:banned_user) { create(:user, :banned, username: 'iambanned') } + let_it_be(:external_user) { create(:user, :external) } + let_it_be(:unconfirmed_user) { create(:user, confirmed_at: nil) } + let_it_be(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') } + let_it_be(:internal_user) { User.alert_bot.tap { |u| u.confirm } } + + it 'does not return blocked or banned users' do + expect(described_class.without_forbidden_states).to match_array([ + normal_user, admin_user, external_user, unconfirmed_user, omniauth_user, internal_user + ]) + end + end + describe 'user_project' do it 'returns users project matched by username and public visibility' do user = create(:user) diff --git a/spec/models/users/in_product_marketing_email_spec.rb b/spec/models/users/in_product_marketing_email_spec.rb index cf08cf7ceed..ca03c3e645d 100644 --- a/spec/models/users/in_product_marketing_email_spec.rb +++ b/spec/models/users/in_product_marketing_email_spec.rb @@ -19,13 +19,6 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do it { is_expected.to validate_uniqueness_of(:user_id).scoped_to([:track, :series]).with_message('has already been sent') } end - describe '.tracks' do - it 'has an entry for every track' do - tracks = [Namespaces::InviteTeamEmailService::TRACK, Namespaces::InProductMarketingEmailsService::TRACKS.keys].flatten - expect(tracks).to match_array(described_class.tracks.keys.map(&:to_sym)) - end - end - describe '.without_track_and_series' do let_it_be(:user) { create(:user) } @@ -135,4 +128,15 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do end end end + + describe '.ACTIVE_TRACKS' do + it 'has an entry for every track' do + tracks = Namespaces::InProductMarketingEmailsService::TRACKS.keys + expect(tracks).to match_array(described_class::ACTIVE_TRACKS.keys.map(&:to_sym)) + end + + it 'does not include INACTIVE_TRACK_NAMES' do + expect(described_class::ACTIVE_TRACKS.keys).not_to include(*described_class::INACTIVE_TRACK_NAMES) + end + end end diff --git a/spec/models/web_ide_terminal_spec.rb b/spec/models/web_ide_terminal_spec.rb index 149fce33f43..fc30bc18f68 100644 --- a/spec/models/web_ide_terminal_spec.rb +++ b/spec/models/web_ide_terminal_spec.rb @@ -41,7 +41,7 @@ RSpec.describe WebIdeTerminal do context 'when image does not have an alias' do let(:config) do - { image: 'ruby:2.7' }.merge(services_with_aliases) + { image: 'image:1.0' }.merge(services_with_aliases) end it 'returns services aliases' do @@ -51,7 +51,7 @@ RSpec.describe WebIdeTerminal do context 'when both image and services have aliases' do let(:config) do - { image: { name: 'ruby:2.7', alias: 'ruby' } }.merge(services_with_aliases) + { image: { name: 'image:1.0', alias: 'ruby' } }.merge(services_with_aliases) end it 'returns all aliases' do @@ -61,7 +61,7 @@ RSpec.describe WebIdeTerminal do context 'when image and services does not have any alias' do let(:config) do - { image: 'ruby:2.7', services: ['postgres'] } + { image: 'image:1.0', services: ['postgres'] } end it 'returns an empty array' do diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 0016d2f517b..51970064c54 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -473,6 +473,21 @@ RSpec.describe WikiPage do end end + describe 'in subdir' do + it 'keeps the page in the same dir when the content is updated' do + title = 'foo/Existing Page' + page = create_wiki_page(title: title) + + expect(page.slug).to eq 'foo/Existing-Page' + expect(page.update(title: title, content: 'new_content')).to be_truthy + + page = wiki.find_page(title) + + expect(page.slug).to eq 'foo/Existing-Page' + expect(page.content).to eq 'new_content' + end + end + context 'when renaming a page' do it 'raises an error if the page already exists' do existing_page = create_wiki_page |