diff options
Diffstat (limited to 'spec/models')
103 files changed, 3335 insertions, 1273 deletions
diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 6500e5fac02..4eb3bd32bfe 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -15,11 +15,12 @@ RSpec.describe AbuseReport, feature_category: :insider_threat do describe 'associations' do it { is_expected.to belong_to(:reporter).class_name('User').inverse_of(:reported_abuse_reports) } it { is_expected.to belong_to(:resolved_by).class_name('User').inverse_of(:resolved_abuse_reports) } - it { is_expected.to belong_to(:assignee).class_name('User').inverse_of(:assigned_abuse_reports) } it { is_expected.to belong_to(:user).inverse_of(:abuse_reports) } it { is_expected.to have_many(:events).class_name('ResourceEvents::AbuseReportEvent').inverse_of(:abuse_report) } it { is_expected.to have_many(:notes) } it { is_expected.to have_many(:user_mentions).class_name('Abuse::Reports::UserMention') } + it { is_expected.to have_many(:admin_abuse_report_assignees).class_name('Admin::AbuseReportAssignee') } + it { is_expected.to have_many(:assignees).class_name('User').through(:admin_abuse_report_assignees) } it "aliases reporter to author" do expect(subject.author).to be(subject.reporter) diff --git a/spec/models/achievements/achievement_spec.rb b/spec/models/achievements/achievement_spec.rb index d3e3e40fc0c..ac34efad5bf 100644 --- a/spec/models/achievements/achievement_spec.rb +++ b/spec/models/achievements/achievement_spec.rb @@ -4,18 +4,12 @@ require 'spec_helper' RSpec.describe Achievements::Achievement, type: :model, feature_category: :user_profile do describe 'associations' do - it { is_expected.to belong_to(:namespace).required } + it { is_expected.to belong_to(:namespace).inverse_of(:achievements).required } it { is_expected.to have_many(:user_achievements).inverse_of(:achievement) } it { is_expected.to have_many(:users).through(:user_achievements).inverse_of(:achievements) } end - describe 'modules' do - subject { described_class } - - it { is_expected.to include_module(Avatarable) } - end - describe 'validations' do subject { create(:achievement) } @@ -27,10 +21,15 @@ RSpec.describe Achievements::Achievement, type: :model, feature_category: :user_ describe '#name' do it 'strips name' do - achievement = described_class.new(name: ' AchievementTest ') + achievement = build(:achievement, name: ' AchievementTest ') + achievement.valid? expect(achievement.name).to eq('AchievementTest') end end + + it_behaves_like Avatarable do + let(:model) { create(:achievement, :with_avatar) } + end end diff --git a/spec/models/activity_pub/releases_subscription_spec.rb b/spec/models/activity_pub/releases_subscription_spec.rb index 0c873a5c18a..0633f293971 100644 --- a/spec/models/activity_pub/releases_subscription_spec.rb +++ b/spec/models/activity_pub/releases_subscription_spec.rb @@ -55,23 +55,37 @@ RSpec.describe ActivityPub::ReleasesSubscription, type: :model, feature_category end end - describe '.find_by_subscriber_url' do + describe '.find_by_project_and_subscriber' do let_it_be(:subscription) { create(:activity_pub_releases_subscription) } it 'returns a record if arguments match' do - result = described_class.find_by_subscriber_url(subscription.subscriber_url) + result = described_class.find_by_project_and_subscriber(subscription.project_id, + subscription.subscriber_url) expect(result).to eq(subscription) end - it 'returns a record if arguments match case insensitively' do - result = described_class.find_by_subscriber_url(subscription.subscriber_url.upcase) + it 'returns a record if subscriber url matches case insensitively' do + result = described_class.find_by_project_and_subscriber(subscription.project_id, + subscription.subscriber_url.upcase) expect(result).to eq(subscription) end + it 'returns nil if project and url do not match' do + result = described_class.find_by_project_and_subscriber(0, 'I really should not exist') + + expect(result).to be(nil) + end + it 'returns nil if project does not match' do - result = described_class.find_by_subscriber_url('I really should not exist') + result = described_class.find_by_project_and_subscriber(0, subscription.subscriber_url) + + expect(result).to be(nil) + end + + it 'returns nil if url does not match' do + result = described_class.find_by_project_and_subscriber(subscription.project_id, 'I really should not exist') expect(result).to be(nil) end diff --git a/spec/models/admin/abuse_report_assignee_spec.rb b/spec/models/admin/abuse_report_assignee_spec.rb new file mode 100644 index 00000000000..3b6233c358f --- /dev/null +++ b/spec/models/admin/abuse_report_assignee_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::AbuseReportAssignee, feature_category: :insider_threat do + let_it_be(:report) { create(:abuse_report) } + let_it_be(:user) { create(:admin) } + + subject(:abuse_report_assignee) { report.admin_abuse_report_assignees.build(assignee: user) } + + it { expect(abuse_report_assignee).to be_valid } + + describe 'associations' do + it { is_expected.to belong_to(:abuse_report) } + it { is_expected.to belong_to(:assignee).class_name('User').with_foreign_key(:user_id) } + end + + describe 'validations' do + it { is_expected.to validate_uniqueness_of(:assignee).scoped_to(:abuse_report_id) } + end + + context 'with loose foreign key on abuse_report_assignees.user_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let_it_be(:parent) { user } + let_it_be(:report) { create(:abuse_report).tap { |r| r.update!(assignees: [parent]) } } + let_it_be(:model) { report.admin_abuse_report_assignees.first } + end + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index a2d6c60fbd0..d16a78be533 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -26,6 +26,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { expect(setting.default_branch_protection_defaults).to eq({}) } it { expect(setting.max_decompressed_archive_size).to eq(25600) } it { expect(setting.decompress_archive_file_timeout).to eq(210) } + it { expect(setting.bulk_import_concurrent_pipeline_batch_limit).to eq(25) } end describe 'validations' do @@ -162,6 +163,8 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_inclusion_of(:user_defaults_to_private_profile).in_array([true, false]) } + it { is_expected.to validate_inclusion_of(:can_create_organization).in_array([true, false]) } + it { is_expected.to validate_inclusion_of(:allow_project_creation_for_guest_and_below).in_array([true, false]) } it { is_expected.to validate_inclusion_of(:deny_all_requests_except_allowed).in_array([true, false]) } @@ -736,6 +739,24 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do end end + describe '#repository_storages_with_default_weight' do + context 'with no extra storage set-up in the config file', fips_mode: false do + it 'keeps existing key restrictions' do + expect(setting.repository_storages_with_default_weight).to eq({ 'default' => 100 }) + end + end + + context 'with extra storage set-up in the config file', fips_mode: false do + before do + stub_storage_settings({ 'default' => {}, 'custom' => {} }) + end + + it 'keeps existing key restrictions' do + expect(setting.repository_storages_with_default_weight).to eq({ 'default' => 100, 'custom' => 0 }) + end + end + end + describe 'setting validated as `addressable_url` configured with external URI' do before do # Use any property that has the `addressable_url` validation. @@ -1321,17 +1342,6 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do expect(subject.errors.messages[:default_group_visibility].first).to eq("cannot be set to a restricted visibility level") expect(subject.errors.messages[:default_project_visibility].first).to eq("cannot be set to a restricted visibility level") end - - context 'when prevent_visibility_restriction FF is disabled' do - before do - stub_feature_flags(prevent_visibility_restriction: false) - end - - it { is_expected.to allow_value(10).for(:default_group_visibility) } - it { is_expected.to allow_value(10).for(:default_project_visibility) } - it { is_expected.to allow_value(20).for(:default_group_visibility) } - it { is_expected.to allow_value(20).for(:default_project_visibility) } - end end describe 'sentry_clientside_traces_sample_rate' do @@ -1342,6 +1352,13 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do .with_message("must be a value between 0 and 1") end end + + describe 'bulk_import_concurrent_pipeline_batch_limit' do + it do + is_expected.to validate_numericality_of(:bulk_import_concurrent_pipeline_batch_limit) + .is_greater_than(0) + end + end end context 'restrict creating duplicates' do @@ -1658,17 +1675,17 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do end context 'with plaintext token only' do - let(:token) { '' } + let(:plaintext_token) { Devise.friendly_token(20) } - it 'ignores the plaintext token' do + it 'encrypts the plaintext token' do subject - described_class.update_all(static_objects_external_storage_auth_token: 'Test') + described_class.update!(static_objects_external_storage_auth_token: plaintext_token) setting.reload expect(setting[:static_objects_external_storage_auth_token]).to be_nil - expect(setting[:static_objects_external_storage_auth_token_encrypted]).to be_nil - expect(setting.static_objects_external_storage_auth_token).to be_nil + expect(setting[:static_objects_external_storage_auth_token_encrypted]).not_to be_nil + expect(setting.static_objects_external_storage_auth_token).to eq(plaintext_token) end end end @@ -1723,4 +1740,8 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do expect(setting.personal_access_tokens_disabled?).to eq(false) end end + + context 'security txt content' do + it { is_expected.to validate_length_of(:security_txt_content).is_at_most(2048) } + end end diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index b179f2df816..a901453ba9f 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -155,6 +155,8 @@ RSpec.describe AwardEmoji, feature_category: :team_planning do end it 'broadcasts updates on the note when destroyed' do + award_emoji.save! + expect(note).to receive(:broadcast_noteable_notes_changed) expect(note).to receive(:trigger_note_subscription_update) @@ -185,6 +187,8 @@ RSpec.describe AwardEmoji, feature_category: :team_planning do end it 'does not broadcast updates on the issue when destroyed' do + award_emoji.save! + expect(issue).not_to receive(:broadcast_noteable_notes_changed) expect(issue).not_to receive(:trigger_note_subscription_update) @@ -315,6 +319,17 @@ RSpec.describe AwardEmoji, feature_category: :team_planning do expect(new_award.url).to eq(custom_emoji.url) end + describe 'when inside subgroup' do + let_it_be(:subgroup) { create(:group, parent: custom_emoji.group) } + let_it_be(:project) { create(:project, namespace: subgroup) } + + it 'is set for custom emoji' do + new_award = build_award(custom_emoji.name) + + expect(new_award.url).to eq(custom_emoji.url) + end + end + context 'feature flag disabled' do before do stub_feature_flags(custom_emoji: false) diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 9c153f36d8b..598c91a64fb 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -387,6 +387,36 @@ RSpec.describe Blob do expect(blob.auxiliary_viewer).to be_a(BlobViewer::License) end end + + context 'when the blob is GitlabCiYml' do + it 'returns a matching viewer for .gitlab-ci.yml' do + blob = fake_blob(path: '.gitlab-ci.yml') + + expect(blob.auxiliary_viewer).to be_a(BlobViewer::GitlabCiYml) + end + + it 'returns nil for non .gitlab-ci.yml' do + blob = fake_blob(path: 'custom-ci.yml') + + expect(blob.auxiliary_viewer).to be_nil + end + + context 'when the project has a custom CI config path' do + let(:project) { build(:project, ci_config_path: 'custom-ci.yml') } + + it 'returns a matching viewer for the custom CI file' do + blob = fake_blob(path: 'custom-ci.yml') + + expect(blob.auxiliary_viewer).to be_a(BlobViewer::GitlabCiYml) + end + + it 'returns nil for the incorrect CI file' do + blob = fake_blob(path: '.gitlab-ci.yml') + + expect(blob.auxiliary_viewer).to be_nil + end + end + end end describe '#rendered_as_text?' do diff --git a/spec/models/bulk_import_spec.rb b/spec/models/bulk_import_spec.rb index ff24f57f7c4..57c6df39167 100644 --- a/spec/models/bulk_import_spec.rb +++ b/spec/models/bulk_import_spec.rb @@ -3,12 +3,12 @@ require 'spec_helper' RSpec.describe BulkImport, type: :model, feature_category: :importers 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(:created_bulk_import) { create(:bulk_import, :created, updated_at: 2.hours.ago) } + let_it_be(:started_bulk_import) { create(:bulk_import, :started, updated_at: 3.hours.ago) } + let_it_be(:finished_bulk_import) { create(:bulk_import, :finished, updated_at: 1.hour.ago) } 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) } + let_it_be(:stale_created_bulk_import) { create(:bulk_import, :created, updated_at: 3.days.ago) } + let_it_be(:stale_started_bulk_import) { create(:bulk_import, :started, updated_at: 2.days.ago) } describe 'associations' do it { is_expected.to belong_to(:user).required } @@ -23,10 +23,27 @@ RSpec.describe BulkImport, type: :model, feature_category: :importers do it { is_expected.to define_enum_for(:source_type).with_values(%i[gitlab]) } end - describe '.stale scope' do - subject { described_class.stale } + describe 'scopes' do + describe '.stale' do + subject { described_class.stale } - it { is_expected.to contain_exactly(stale_created_bulk_import, stale_started_bulk_import) } + it { is_expected.to contain_exactly(stale_created_bulk_import, stale_started_bulk_import) } + end + + describe '.order_by_updated_at_and_id' do + subject { described_class.order_by_updated_at_and_id(:desc) } + + it 'sorts by given direction' do + is_expected.to eq([ + failed_bulk_import, + finished_bulk_import, + created_bulk_import, + started_bulk_import, + stale_started_bulk_import, + stale_created_bulk_import + ]) + end + end end describe '.all_human_statuses' do diff --git a/spec/models/bulk_imports/batch_tracker_spec.rb b/spec/models/bulk_imports/batch_tracker_spec.rb index 336943228c7..1c7cbc0cb8c 100644 --- a/spec/models/bulk_imports/batch_tracker_spec.rb +++ b/spec/models/bulk_imports/batch_tracker_spec.rb @@ -13,4 +13,19 @@ RSpec.describe BulkImports::BatchTracker, type: :model, feature_category: :impor it { is_expected.to validate_presence_of(:batch_number) } it { is_expected.to validate_uniqueness_of(:batch_number).scoped_to(:tracker_id) } end + + describe 'scopes' do + describe '.in_progress' do + it 'returns only batches that are in progress' do + created = create(:bulk_import_batch_tracker, :created) + started = create(:bulk_import_batch_tracker, :started) + create(:bulk_import_batch_tracker, :finished) + create(:bulk_import_batch_tracker, :timeout) + create(:bulk_import_batch_tracker, :failed) + create(:bulk_import_batch_tracker, :skipped) + + expect(described_class.in_progress).to contain_exactly(created, started) + end + end + end end diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index b822786579b..ce143a1aa33 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -191,6 +191,24 @@ RSpec.describe BulkImports::Entity, type: :model, feature_category: :importers d expect(described_class.by_user_id(user.id)).to contain_exactly(entity_1, entity_2) end end + + describe '.stale' do + it 'returns entities that are stale' do + entity_1 = create(:bulk_import_entity, updated_at: 3.days.ago) + create(:bulk_import_entity) + + expect(described_class.stale).to contain_exactly(entity_1) + end + end + + describe '.order_by_updated_at_and_id' do + it 'returns entities ordered by updated_at and id' do + entity_1 = create(:bulk_import_entity, updated_at: 3.days.ago) + entity_2 = create(:bulk_import_entity, updated_at: 2.days.ago) + + expect(described_class.order_by_updated_at_and_id(:desc)).to eq([entity_2, entity_1]) + end + end end describe '.all_human_statuses' do @@ -444,6 +462,13 @@ RSpec.describe BulkImports::Entity, type: :model, feature_category: :importers d expect(entity.has_failures).to eq(true) end + + it 'sets the has_failures flag on the parent import' do + create(:bulk_import_failure, entity: entity) + + expect { entity.update_has_failures } + .to change { entity.bulk_import.has_failures? }.from(false).to(true) + end end context 'when entity does not have failures' do diff --git a/spec/models/bulk_imports/export_status_spec.rb b/spec/models/bulk_imports/export_status_spec.rb index c3faa2db19c..aa3bce78534 100644 --- a/spec/models/bulk_imports/export_status_spec.rb +++ b/spec/models/bulk_imports/export_status_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::ExportStatus, feature_category: :importers do +RSpec.describe BulkImports::ExportStatus, :clean_gitlab_redis_cache, feature_category: :importers do let_it_be(:relation) { 'labels' } let_it_be(:import) { create(:bulk_import) } let_it_be(:config) { create(:bulk_import_configuration, bulk_import: import) } @@ -100,7 +100,7 @@ RSpec.describe BulkImports::ExportStatus, feature_category: :importers do end it 'returns false' do - expect(subject.started?).to eq(false) + expect(subject.failed?).to eq(false) end end @@ -113,8 +113,8 @@ RSpec.describe BulkImports::ExportStatus, feature_category: :importers do end end - it 'returns false' do - expect(subject.started?).to eq(false) + it 'returns true' do + expect(subject.failed?).to eq(true) end end end @@ -156,7 +156,7 @@ RSpec.describe BulkImports::ExportStatus, feature_category: :importers do end it 'returns false' do - expect(subject.started?).to eq(false) + expect(subject.empty?).to eq(false) end end end @@ -282,4 +282,111 @@ RSpec.describe BulkImports::ExportStatus, feature_category: :importers do end end end + + describe 'caching' do + let(:cached_status) do + subject.send(:status) + subject.send(:status_from_cache) + end + + shared_examples 'does not result in a cached status' do + specify do + expect(cached_status).to be_nil + end + end + + shared_examples 'results in a cached status' do + specify do + expect(cached_status).to include('status' => status) + end + + context 'when something goes wrong during export status fetch' do + before do + allow_next_instance_of(BulkImports::Clients::HTTP) do |client| + allow(client).to receive(:get).and_raise( + BulkImports::NetworkError.new("Unsuccessful response", response: nil) + ) + end + end + + include_examples 'does not result in a cached status' + end + end + + context 'when export status is started' do + let(:status) { BulkImports::Export::STARTED } + + it_behaves_like 'does not result in a cached status' + end + + context 'when export status is failed' do + let(:status) { BulkImports::Export::FAILED } + + it_behaves_like 'results in a cached status' + end + + context 'when export status is finished' do + let(:status) { BulkImports::Export::FINISHED } + + it_behaves_like 'results in a cached status' + end + + context 'when export status is not present' do + let(:status) { nil } + + it_behaves_like 'does not result in a cached status' + end + + context 'when the cache is empty' do + let(:status) { BulkImports::Export::FAILED } + + it 'fetches the status from the remote' do + expect(subject).to receive(:status_from_remote).and_call_original + expect(subject.send(:status)).to include('status' => status) + end + end + + context 'when the cache is not empty' do + let(:status) { BulkImports::Export::FAILED } + + before do + Gitlab::Cache::Import::Caching.write( + described_class.new(tracker, 'labels').send(:cache_key), + { 'status' => 'mock status' }.to_json + ) + end + + it 'does not fetch the status from the remote' do + expect(subject).not_to receive(:status_from_remote) + expect(subject.send(:status)).to eq({ 'status' => 'mock status' }) + end + + context 'with a different entity' do + before do + tracker.entity = create(:bulk_import_entity, bulk_import: import, source_full_path: 'foo') + end + + it 'fetches the status from the remote' do + expect(subject).to receive(:status_from_remote).and_call_original + expect(subject.send(:status)).to include('status' => status) + end + end + + context 'with a different relation' do + let_it_be(:relation) { 'merge_requests' } + + let(:response_double) do + instance_double(HTTParty::Response, parsed_response: [ + { 'relation' => 'labels', 'status' => status }, + { 'relation' => 'merge_requests', 'status' => status } + ]) + end + + it 'fetches the status from the remote' do + expect(subject).to receive(:status_from_remote).and_call_original + expect(subject.send(:status)).to include('status' => status) + end + end + end + end end diff --git a/spec/models/bulk_imports/export_upload_spec.rb b/spec/models/bulk_imports/export_upload_spec.rb index d9ae41af0db..ca45fe73b0e 100644 --- a/spec/models/bulk_imports/export_upload_spec.rb +++ b/spec/models/bulk_imports/export_upload_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::ExportUpload do +RSpec.describe BulkImports::ExportUpload, type: :model, feature_category: :importers do subject { described_class.new(export: create(:bulk_import_export)) } describe 'associations' do @@ -20,4 +20,18 @@ RSpec.describe BulkImports::ExportUpload do expect(subject.public_send(method).url).to eq(url) end + + describe 'ActiveRecord callbacks' do + let(:after_save_callbacks) { described_class._save_callbacks.select { |cb| cb.kind == :after } } + let(:after_commit_callbacks) { described_class._commit_callbacks.select { |cb| cb.kind == :after } } + + def find_callback(callbacks, key) + callbacks.find { |cb| cb.filter == key } + end + + it 'export file is stored in after_commit callback' do + expect(find_callback(after_commit_callbacks, :store_export_file!)).to be_present + expect(find_callback(after_save_callbacks, :store_export_file!)).to be_nil + end + end end diff --git a/spec/models/bulk_imports/tracker_spec.rb b/spec/models/bulk_imports/tracker_spec.rb index edd9adfa5f6..25cd5489a9f 100644 --- a/spec/models/bulk_imports/tracker_spec.rb +++ b/spec/models/bulk_imports/tracker_spec.rb @@ -83,5 +83,31 @@ RSpec.describe BulkImports::Tracker, type: :model, feature_category: :importers "'InexistingPipeline' is not a valid BulkImport Pipeline" ) end + + context 'when using delegation methods' do + context 'with group pipelines' do + let(:entity) { create(:bulk_import_entity) } + + it 'does not raise' do + entity.pipelines.each do |pipeline| + tracker = create(:bulk_import_tracker, entity: entity, pipeline_name: pipeline[:pipeline]) + expect { tracker.abort_on_failure? }.not_to raise_error + expect { tracker.file_extraction_pipeline? }.not_to raise_error + end + end + end + + context 'with project pipelines' do + let(:entity) { create(:bulk_import_entity, :project_entity) } + + it 'does not raise' do + entity.pipelines.each do |pipeline| + tracker = create(:bulk_import_tracker, entity: entity, pipeline_name: pipeline[:pipeline]) + expect { tracker.abort_on_failure? }.not_to raise_error + expect { tracker.file_extraction_pipeline? }.not_to raise_error + end + end + end + end end end diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 1d0c3bb5dee..ae8c5aea858 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -36,6 +36,24 @@ RSpec.describe Ci::Bridge, feature_category: :continuous_integration do expect(bridge).to have_one(:downstream_pipeline) end + describe 'no-op methods for compatibility with Ci::Build' do + it 'returns an empty array job_artifacts' do + expect(bridge.job_artifacts).to eq(Ci::JobArtifact.none) + end + + it 'return nil for artifacts_expire_at' do + expect(bridge.artifacts_expire_at).to be_nil + end + + it 'return nil for runner' do + expect(bridge.runner).to be_nil + end + + it 'returns an empty TagList for tag_list' do + expect(bridge.tag_list).to be_a(ActsAsTaggableOn::TagList) + end + end + describe '#retryable?' do let(:bridge) { create(:ci_bridge, :success) } @@ -595,6 +613,203 @@ RSpec.describe Ci::Bridge, feature_category: :continuous_integration do end end + describe 'variables expansion' do + let(:options) do + { + trigger: { + project: 'my/project', + branch: 'master', + forward: { yaml_variables: true, + pipeline_variables: true }.compact + } + } + end + + let(:yaml_variables) do + [ + { + key: 'EXPANDED_PROJECT_VAR6', + value: 'project value6 $PROJECT_PROTECTED_VAR' + }, + { + key: 'EXPANDED_GROUP_VAR6', + value: 'group value6 $GROUP_PROTECTED_VAR' + }, + + { + key: 'VAR7', + value: 'value7 $VAR1', + raw: true + } + ] + end + + let_it_be(:downstream_creator_user) { create(:user) } + let_it_be(:bridge_creator_user) { create(:user) } + + let_it_be(:bridge_group) { create(:group) } + let_it_be(:downstream_group) { create(:group) } + let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user, group: downstream_group) } + let_it_be(:project) { create(:project, :repository, :in_group, creator: bridge_creator_user, group: bridge_group) } + let(:bridge) { build(:ci_bridge, :playable, pipeline: pipeline, downstream: downstream_project) } + let!(:pipeline) { create(:ci_pipeline, project: project) } + + let!(:ci_variable) do + create(:ci_variable, + project: project, + key: 'PROJECT_PROTECTED_VAR', + value: 'this is a secret', + protected: is_variable_protected?) + end + + let!(:ci_group_variable) do + create(:ci_group_variable, + group: bridge_group, + key: 'GROUP_PROTECTED_VAR', + value: 'this is a secret', + protected: is_variable_protected?) + end + + before do + bridge.yaml_variables = yaml_variables + allow(bridge.project).to receive(:protected_for?).and_return(true) + end + + shared_examples 'expands variables from a project downstream' do + it do + vars = bridge.downstream_variables + expect(vars).to include({ key: 'EXPANDED_PROJECT_VAR6', value: 'project value6 this is a secret' }) + end + end + + shared_examples 'expands variables from a group downstream' do + it do + vars = bridge.downstream_variables + expect(vars).to include({ key: 'EXPANDED_GROUP_VAR6', value: 'group value6 this is a secret' }) + end + end + + shared_examples 'expands project and group variables downstream' do + it_behaves_like 'expands variables from a project downstream' + + it_behaves_like 'expands variables from a group downstream' + end + + shared_examples 'does not expand variables from a project downstream' do + it do + vars = bridge.downstream_variables + expect(vars).not_to include({ key: 'EXPANDED_PROJECT_VAR6', value: 'project value6 this is a secret' }) + end + end + + shared_examples 'does not expand variables from a group downstream' do + it do + vars = bridge.downstream_variables + expect(vars).not_to include({ key: 'EXPANDED_GROUP_VAR6', value: 'group value6 this is a secret' }) + end + end + + shared_examples 'feature flag is disabled' do + before do + stub_feature_flags(exclude_protected_variables_from_multi_project_pipeline_triggers: false) + end + + it_behaves_like 'expands project and group variables downstream' + end + + shared_examples 'does not expand project and group variables downstream' do + it_behaves_like 'does not expand variables from a project downstream' + + it_behaves_like 'does not expand variables from a group downstream' + end + + context 'when they are protected' do + let!(:is_variable_protected?) { true } + + context 'and downstream project group is different from bridge group' do + it_behaves_like 'does not expand project and group variables downstream' + + it_behaves_like 'feature flag is disabled' + end + + context 'and there is no downstream project' do + let(:downstream_project) { nil } + + it_behaves_like 'expands project and group variables downstream' + + it_behaves_like 'feature flag is disabled' + end + + context 'and downstream project equals bridge project' do + let(:downstream_project) { project } + + it_behaves_like 'expands project and group variables downstream' + + it_behaves_like 'feature flag is disabled' + end + + context 'and downstream project group is equal to bridge project group' do + let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user, group: bridge_group) } + + it_behaves_like 'expands variables from a group downstream' + + it_behaves_like 'does not expand variables from a project downstream' + + it_behaves_like 'feature flag is disabled' + end + + context 'and downstream project has no group' do + let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user) } + + it_behaves_like 'does not expand project and group variables downstream' + + it_behaves_like 'feature flag is disabled' + end + end + + context 'when they are not protected' do + let!(:is_variable_protected?) { false } + + context 'and downstream project group is different from bridge group' do + it_behaves_like 'expands project and group variables downstream' + + it_behaves_like 'feature flag is disabled' + end + + context 'and there is no downstream project' do + let(:downstream_project) { nil } + + it_behaves_like 'expands project and group variables downstream' + + it_behaves_like 'feature flag is disabled' + end + + context 'and downstream project equals bridge project' do + let(:downstream_project) { project } + + it_behaves_like 'expands project and group variables downstream' + + it_behaves_like 'feature flag is disabled' + end + + context 'and downstream project group is equal to bridge project group' do + let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user, group: bridge_group) } + + it_behaves_like 'expands project and group variables downstream' + + it_behaves_like 'feature flag is disabled' + end + + context 'and downstream project has no group' do + let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user) } + + it_behaves_like 'expands project and group variables downstream' + + it_behaves_like 'feature flag is disabled' + end + end + end + describe '#forward_pipeline_variables?' do using RSpec::Parameterized::TableSyntax @@ -824,8 +1039,9 @@ RSpec.describe Ci::Bridge, feature_category: :continuous_integration do end it 'creates the metadata record and assigns its partition' do - # the factory doesn't use any metadatable setters by default - # so the record will be initialized by the before_validation callback + # The record is initialized by the factory calling metadatable setters + bridge.metadata = nil + expect(bridge.metadata).to be_nil expect(bridge.save!).to be_truthy diff --git a/spec/models/ci/build_need_spec.rb b/spec/models/ci/build_need_spec.rb index 4f76a7650ec..7ce3c63458f 100644 --- a/spec/models/ci/build_need_spec.rb +++ b/spec/models/ci/build_need_spec.rb @@ -11,11 +11,21 @@ RSpec.describe Ci::BuildNeed, model: true, feature_category: :continuous_integra it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_length_of(:name).is_at_most(255) } - describe '.artifacts' do - let_it_be(:with_artifacts) { create(:ci_build_need, artifacts: true) } - let_it_be(:without_artifacts) { create(:ci_build_need, artifacts: false) } + describe 'scopes' do + describe '.scoped_build' do + subject(:scoped_build) { described_class.scoped_build } - it { expect(described_class.artifacts).to contain_exactly(with_artifacts) } + it 'includes partition_id filter' do + expect(scoped_build.where_values_hash).to match(a_hash_including('partition_id')) + end + end + + describe '.artifacts' do + let_it_be(:with_artifacts) { create(:ci_build_need, artifacts: true) } + let_it_be(:without_artifacts) { create(:ci_build_need, artifacts: false) } + + it { expect(described_class.artifacts).to contain_exactly(with_artifacts) } + end end describe 'BulkInsertSafe' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2e552c8d524..18c7e57d464 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -987,6 +987,28 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def describe '#artifacts_public?' do subject { build.artifacts_public? } + context 'artifacts with defaults - public' do + let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } + + it { is_expected.to be_truthy } + end + + context 'non public artifacts' do + let(:build) { create(:ci_build, :private_artifacts, pipeline: pipeline) } + + it { is_expected.to be_falsey } + end + + context 'no artifacts' do + let(:build) { create(:ci_build, pipeline: pipeline) } + + it { is_expected.to be_truthy } + end + end + + describe '#artifact_is_public_in_config?' do + subject { build.artifact_is_public_in_config? } + context 'artifacts with defaults' do let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } @@ -994,10 +1016,22 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end context 'non public artifacts' do - let(:build) { create(:ci_build, :artifacts, :with_private_artifacts_config, pipeline: pipeline) } + let(:build) { create(:ci_build, :with_private_artifacts_config, pipeline: pipeline) } it { is_expected.to be_falsey } end + + context 'public artifacts' do + let(:build) { create(:ci_build, :with_public_artifacts_config, pipeline: pipeline) } + + it { is_expected.to be_truthy } + end + + context 'no artifacts' do + let(:build) { create(:ci_build, pipeline: pipeline) } + + it { is_expected.to be_truthy } + end end describe '#artifacts_expired?' do diff --git a/spec/models/ci/catalog/components_project_spec.rb b/spec/models/ci/catalog/components_project_spec.rb index 79e1a113e47..5f739c244a5 100644 --- a/spec/models/ci/catalog/components_project_spec.rb +++ b/spec/models/ci/catalog/components_project_spec.rb @@ -97,6 +97,7 @@ RSpec.describe Ci::Catalog::ComponentsProject, feature_category: :pipeline_compo 'dast' | 'image: alpine_2' | 'templates/dast/template.yml' 'template' | 'image: alpine_3' | 'templates/template.yml' 'blank-yaml' | '' | 'templates/blank-yaml.yml' + 'non/exist' | nil | nil end with_them do diff --git a/spec/models/ci/catalog/listing_spec.rb b/spec/models/ci/catalog/listing_spec.rb index 7a1e12165ac..2ffffb9112c 100644 --- a/spec/models/ci/catalog/listing_spec.rb +++ b/spec/models/ci/catalog/listing_spec.rb @@ -3,59 +3,61 @@ require 'spec_helper' RSpec.describe Ci::Catalog::Listing, feature_category: :pipeline_composition do - let_it_be(:namespace) { create(:group) } - let_it_be(:project_x) { create(:project, namespace: namespace, name: 'X Project') } - let_it_be(:project_a) { create(:project, :public, namespace: namespace, name: 'A Project') } - let_it_be(:project_noaccess) { create(:project, namespace: namespace, name: 'C Project') } - let_it_be(:project_ext) { create(:project, :public, name: 'TestProject') } let_it_be(:user) { create(:user) } + let_it_be(:namespace) { create(:group) } + let_it_be(:public_namespace_project) do + create(:project, :public, namespace: namespace, name: 'A public namespace project') + end - let_it_be(:project_b) do + let_it_be(:public_project) { create(:project, :public, name: 'B public test project') } + let_it_be(:namespace_project_a) { create(:project, namespace: namespace, name: 'Test namespace project') } + let_it_be(:namespace_project_b) { create(:project, namespace: namespace, name: 'X namespace Project') } + let_it_be(:project_noaccess) { create(:project, namespace: namespace, name: 'Project with no access') } + let_it_be(:internal_project) { create(:project, :internal, name: 'Internal project') } + + let_it_be(:private_project) do create(:project, namespace: namespace, name: 'B Project', description: 'Rspec test framework') end let(:list) { described_class.new(user) } before_all do - project_x.add_reporter(user) - project_b.add_reporter(user) - project_a.add_reporter(user) - project_ext.add_reporter(user) + namespace_project_a.add_reporter(user) + namespace_project_b.add_reporter(user) + public_namespace_project.add_reporter(user) + public_project.add_reporter(user) + internal_project.add_owner(user) end describe '#resources' do subject(:resources) { list.resources(**params) } - context 'when user is anonymous' do - let(:user) { nil } - let(:params) { {} } + let(:params) { {} } - let!(:resource_1) { create(:ci_catalog_resource, project: project_a) } - let!(:resource_2) { create(:ci_catalog_resource, project: project_ext) } - let!(:resource_3) { create(:ci_catalog_resource, project: project_b) } + let_it_be(:public_resource_a) { create(:ci_catalog_resource, :published, project: public_namespace_project) } + let_it_be(:public_resource_b) { create(:ci_catalog_resource, :published, project: public_project) } + let_it_be(:internal_resource) { create(:ci_catalog_resource, :published, project: internal_project) } + let_it_be(:private_namespace_resource) { create(:ci_catalog_resource, :published, project: namespace_project_a) } + let_it_be(:unpublished_resource) { create(:ci_catalog_resource, project: namespace_project_b) } - it 'returns only resources for public projects' do - is_expected.to contain_exactly(resource_1, resource_2) - end + it 'by default returns all resources visible to the current user' do + is_expected.to contain_exactly(public_resource_a, public_resource_b, private_namespace_resource, + internal_resource) + end - context 'when sorting is provided' do - let(:params) { { sort: :name_desc } } + context 'when user is anonymous' do + let(:user) { nil } - it 'returns only resources for public projects sorted by name DESC' do - is_expected.to contain_exactly(resource_2, resource_1) - end + it 'returns only published resources for public projects' do + is_expected.to contain_exactly(public_resource_a, public_resource_b) end end context 'when search params are provided' do let(:params) { { search: 'test' } } - let!(:resource_1) { create(:ci_catalog_resource, project: project_a) } - let!(:resource_2) { create(:ci_catalog_resource, project: project_ext) } - let!(:resource_3) { create(:ci_catalog_resource, project: project_b) } - it 'returns the resources that match the search params' do - is_expected.to contain_exactly(resource_2, resource_3) + is_expected.to contain_exactly(public_resource_b, private_namespace_resource) end context 'when search term is too small' do @@ -65,117 +67,197 @@ RSpec.describe Ci::Catalog::Listing, feature_category: :pipeline_composition do end end - context 'when namespace is provided' do - let(:params) { { namespace: namespace } } + context 'when the scope is :namespaces' do + let_it_be(:public_resource_no_namespace) do + create(:ci_catalog_resource, project: create(:project, :public, name: 'public')) + end - context 'when namespace is not a root namespace' do - let(:namespace) { create(:group, :nested) } + let(:params) { { scope: :namespaces } } - it 'raises an exception' do - expect { resources }.to raise_error(ArgumentError, 'Namespace is not a root namespace') + context 'when the `ci_guard_query_for_catalog_resource_scope` ff is enabled' do + it "returns the catalog resources belonging to the user's authorized namespaces" do + is_expected.to contain_exactly(public_resource_a, public_resource_b, internal_resource, + private_namespace_resource) end end - context 'when the user has access to all projects in the namespace' do - context 'when the namespace has no catalog resources' do - it { is_expected.to be_empty } + context 'when the `ci_guard_query_for_catalog_resource_scope` ff is disabled' do + before do + stub_feature_flags(ci_guard_for_catalog_resource_scope: false) end - context 'when the namespace has catalog resources' do - let_it_be(:today) { Time.zone.now } - let_it_be(:yesterday) { today - 1.day } - let_it_be(:tomorrow) { today + 1.day } + it 'returns all resources visible to the current user' do + is_expected.to contain_exactly( + public_resource_a, public_resource_b, private_namespace_resource, + internal_resource) + end + end + end - let_it_be(:resource_1) do - create(:ci_catalog_resource, project: project_x, latest_released_at: yesterday, created_at: today) - end + context 'with a sort parameter' do + let_it_be(:today) { Time.zone.now } + let_it_be(:yesterday) { today - 1.day } + let_it_be(:tomorrow) { today + 1.day } - let_it_be(:resource_2) do - create(:ci_catalog_resource, project: project_b, latest_released_at: today, created_at: yesterday) - end + let(:params) { { sort: sort } } - let_it_be(:resource_3) do - create(:ci_catalog_resource, project: project_a, latest_released_at: nil, created_at: tomorrow) - end + before_all do + public_resource_a.update!(created_at: today, latest_released_at: yesterday) + public_resource_b.update!(created_at: yesterday, latest_released_at: today) + private_namespace_resource.update!(created_at: tomorrow, latest_released_at: tomorrow) + internal_resource.update!(created_at: tomorrow + 1) + end - let_it_be(:other_namespace_resource) do - create(:ci_catalog_resource, project: project_ext, latest_released_at: tomorrow) - end + context 'when the sort is created_at ascending' do + let_it_be(:sort) { :created_at_asc } + + it 'contains catalog resources sorted by created_at ascending' do + is_expected.to eq([public_resource_b, public_resource_a, private_namespace_resource, internal_resource]) + end + end + + context 'when the sort is created_at descending' do + let_it_be(:sort) { :created_at_desc } + + it 'contains catalog resources sorted by created_at descending' do + is_expected.to eq([internal_resource, private_namespace_resource, public_resource_a, public_resource_b]) + end + end + + context 'when the sort is name ascending' do + let_it_be(:sort) { :name_asc } + + it 'contains catalog resources for projects sorted by name ascending' do + is_expected.to eq([public_resource_a, public_resource_b, internal_resource, private_namespace_resource]) + end + end + + context 'when the sort is name descending' do + let_it_be(:sort) { :name_desc } + + it 'contains catalog resources for projects sorted by name descending' do + is_expected.to eq([private_namespace_resource, internal_resource, public_resource_b, public_resource_a]) + end + end - it 'contains only catalog resources for projects in that namespace' do - is_expected.to contain_exactly(resource_1, resource_2, resource_3) + context 'when the sort is latest_released_at ascending' do + let_it_be(:sort) { :latest_released_at_asc } + + it 'contains catalog resources sorted by latest_released_at ascending with nulls last' do + is_expected.to eq([public_resource_a, public_resource_b, private_namespace_resource, internal_resource]) + end + end + + context 'when the sort is latest_released_at descending' do + let_it_be(:sort) { :latest_released_at_desc } + + it 'contains catalog resources sorted by latest_released_at descending with nulls last' do + is_expected.to eq([private_namespace_resource, public_resource_b, public_resource_a, internal_resource]) + end + end + end + + context 'when namespace is provided' do + let(:params) { { namespace: namespace } } + + context 'when it is a root namespace' do + context 'when it has catalog resources' do + it 'returns resources in the namespace visible to the user' do + is_expected.to contain_exactly(public_resource_a, private_namespace_resource) end + end - context 'with a sort parameter' do - let(:params) { { namespace: namespace, sort: sort } } + context 'when the namespace has no catalog resources' do + let(:namespace) { build(:namespace) } - context 'when the sort is created_at ascending' do - let_it_be(:sort) { :created_at_asc } + it { is_expected.to be_empty } + end + end - it 'contains catalog resources sorted by created_at ascending' do - is_expected.to eq([resource_2, resource_1, resource_3]) - end - end + context 'when namespace is not a root namespace' do + let_it_be(:namespace) { create(:group, :nested) } - context 'when the sort is created_at descending' do - let_it_be(:sort) { :created_at_desc } + it 'raises an exception' do + expect { resources }.to raise_error(ArgumentError, 'Namespace is not a root namespace') + end + end + end + end - it 'contains catalog resources sorted by created_at descending' do - is_expected.to eq([resource_3, resource_1, resource_2]) - end - end + describe '#find_resource' do + let_it_be(:accessible_resource) { create(:ci_catalog_resource, :published, project: public_project) } + let_it_be(:inaccessible_resource) { create(:ci_catalog_resource, :published, project: project_noaccess) } + let_it_be(:draft_resource) { create(:ci_catalog_resource, project: public_namespace_project, state: :draft) } - context 'when the sort is name ascending' do - let_it_be(:sort) { :name_asc } + context 'when using the ID argument' do + subject { list.find_resource(id: id) } - it 'contains catalog resources for projects sorted by name ascending' do - is_expected.to eq([resource_3, resource_2, resource_1]) - end - end + context 'when the resource is published and visible to the user' do + let(:id) { accessible_resource.id } - context 'when the sort is name descending' do - let_it_be(:sort) { :name_desc } + it 'fetches the resource' do + is_expected.to eq(accessible_resource) + end + end - it 'contains catalog resources for projects sorted by name descending' do - is_expected.to eq([resource_1, resource_2, resource_3]) - end - end + context 'when the resource is not found' do + let(:id) { 'not-an-id' } - context 'when the sort is latest_released_at ascending' do - let_it_be(:sort) { :latest_released_at_asc } + it 'returns nil' do + is_expected.to be_nil + end + end - it 'contains catalog resources sorted by latest_released_at ascending with nulls last' do - is_expected.to eq([resource_1, resource_2, resource_3]) - end - end + context 'when the resource is not published' do + let(:id) { draft_resource.id } - context 'when the sort is latest_released_at descending' do - let_it_be(:sort) { :latest_released_at_desc } + it 'returns nil' do + is_expected.to be_nil + end + end - it 'contains catalog resources sorted by latest_released_at descending with nulls last' do - is_expected.to eq([resource_2, resource_1, resource_3]) - end - end - end + context "when the current user cannot read code on the resource's project" do + let(:id) { inaccessible_resource.id } + + it 'returns nil' do + is_expected.to be_nil end end + end - context 'when the user only has access to some projects in the namespace' do - let!(:accessible_resource) { create(:ci_catalog_resource, project: project_x) } - let!(:inaccessible_resource) { create(:ci_catalog_resource, project: project_noaccess) } + context 'when using the full_path argument' do + subject { list.find_resource(full_path: full_path) } - it 'only returns catalog resources for projects the user has access to' do - is_expected.to contain_exactly(accessible_resource) + context 'when the resource is published and visible to the user' do + let(:full_path) { accessible_resource.project.full_path } + + it 'fetches the resource' do + is_expected.to eq(accessible_resource) end end - context 'when the user does not have access to the namespace' do - let!(:project) { create(:project) } - let!(:resource) { create(:ci_catalog_resource, project: project) } + context 'when the resource is not found' do + let(:full_path) { 'not-a-path' } - let(:namespace) { project.namespace } + it 'returns nil' do + is_expected.to be_nil + end + end - it { is_expected.to be_empty } + context 'when the resource is not published' do + let(:full_path) { draft_resource.project.full_path } + + it 'returns nil' do + is_expected.to be_nil + end + end + + context "when the current user cannot read code on the resource's project" do + let(:full_path) { inaccessible_resource.project.full_path } + + it 'returns nil' do + is_expected.to be_nil + end end end end diff --git a/spec/models/ci/catalog/resource_spec.rb b/spec/models/ci/catalog/resource_spec.rb index 098772b1ea9..15d8b4f440b 100644 --- a/spec/models/ci/catalog/resource_spec.rb +++ b/spec/models/ci/catalog/resource_spec.rb @@ -3,50 +3,57 @@ require 'spec_helper' RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do - let_it_be(:today) { Time.zone.now } - let_it_be(:yesterday) { today - 1.day } - let_it_be(:tomorrow) { today + 1.day } + let_it_be(:current_user) { create(:user) } - let_it_be_with_reload(:project) { create(:project, name: 'A') } - let_it_be(:project_2) { build(:project, name: 'Z') } - let_it_be(:project_3) { build(:project, name: 'L', description: 'Z') } - let_it_be_with_reload(:resource) { create(:ci_catalog_resource, project: project, latest_released_at: tomorrow) } - let_it_be(:resource_2) { create(:ci_catalog_resource, project: project_2, latest_released_at: today) } - let_it_be(:resource_3) { create(:ci_catalog_resource, project: project_3, latest_released_at: nil) } + let_it_be(:project_a) { create(:project, name: 'A') } + let_it_be(:project_b) { create(:project, name: 'B') } + let_it_be(:project_c) { create(:project, name: 'C', description: 'B') } - let_it_be(:release1) { create(:release, project: project, released_at: yesterday) } - let_it_be(:release2) { create(:release, project: project, released_at: today) } - let_it_be(:release3) { create(:release, project: project, released_at: tomorrow) } + let_it_be_with_reload(:resource_a) do + create(:ci_catalog_resource, project: project_a, latest_released_at: '2023-02-01T00:00:00Z') + end + + let_it_be(:resource_b) do + create(:ci_catalog_resource, project: project_b, latest_released_at: '2023-01-01T00:00:00Z') + end + + let_it_be(:resource_c) { create(:ci_catalog_resource, project: project_c) } it { is_expected.to belong_to(:project) } it do is_expected.to( - have_many(:components).class_name('Ci::Catalog::Resources::Component').with_foreign_key(:catalog_resource_id) - ) + have_many(:components).class_name('Ci::Catalog::Resources::Component').with_foreign_key(:catalog_resource_id)) end - it { is_expected.to have_many(:versions).class_name('Ci::Catalog::Resources::Version') } + it do + is_expected.to( + have_many(:versions).class_name('Ci::Catalog::Resources::Version').with_foreign_key(:catalog_resource_id)) + end + + it do + is_expected.to( + have_many(:sync_events).class_name('Ci::Catalog::Resources::SyncEvent').with_foreign_key(:catalog_resource_id)) + end it { is_expected.to delegate_method(:avatar_path).to(:project) } it { is_expected.to delegate_method(:star_count).to(:project) } - it { is_expected.to delegate_method(:forks_count).to(:project) } it { is_expected.to define_enum_for(:state).with_values({ draft: 0, published: 1 }) } describe '.for_projects' do it 'returns catalog resources for the given project IDs' do - resources_for_projects = described_class.for_projects(project.id) + resources_for_projects = described_class.for_projects(project_a.id) - expect(resources_for_projects).to contain_exactly(resource) + expect(resources_for_projects).to contain_exactly(resource_a) end end describe '.search' do it 'returns catalog resources whose name or description match the search term' do - resources = described_class.search('Z') + resources = described_class.search('B') - expect(resources).to contain_exactly(resource_2, resource_3) + expect(resources).to contain_exactly(resource_b, resource_c) end end @@ -54,7 +61,7 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do it 'returns catalog resources sorted by descending created at' do ordered_resources = described_class.order_by_created_at_desc - expect(ordered_resources.to_a).to eq([resource_3, resource_2, resource]) + expect(ordered_resources.to_a).to eq([resource_c, resource_b, resource_a]) end end @@ -62,7 +69,7 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do it 'returns catalog resources sorted by ascending created at' do ordered_resources = described_class.order_by_created_at_asc - expect(ordered_resources.to_a).to eq([resource, resource_2, resource_3]) + expect(ordered_resources.to_a).to eq([resource_a, resource_b, resource_c]) end end @@ -70,13 +77,13 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do subject(:ordered_resources) { described_class.order_by_name_desc } it 'returns catalog resources sorted by descending name' do - expect(ordered_resources.pluck(:name)).to eq(%w[Z L A]) + expect(ordered_resources.pluck(:name)).to eq(%w[C B A]) end it 'returns catalog resources sorted by descending name with nulls last' do - resource.update!(name: nil) + resource_a.update!(name: nil) - expect(ordered_resources.pluck(:name)).to eq(['Z', 'L', nil]) + expect(ordered_resources.pluck(:name)).to eq(['C', 'B', nil]) end end @@ -84,13 +91,13 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do subject(:ordered_resources) { described_class.order_by_name_asc } it 'returns catalog resources sorted by ascending name' do - expect(ordered_resources.pluck(:name)).to eq(%w[A L Z]) + expect(ordered_resources.pluck(:name)).to eq(%w[A B C]) end it 'returns catalog resources sorted by ascending name with nulls last' do - resource.update!(name: nil) + resource_a.update!(name: nil) - expect(ordered_resources.pluck(:name)).to eq(['L', 'Z', nil]) + expect(ordered_resources.pluck(:name)).to eq(['B', 'C', nil]) end end @@ -98,7 +105,7 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do it 'returns catalog resources sorted by latest_released_at descending with nulls last' do ordered_resources = described_class.order_by_latest_released_at_desc - expect(ordered_resources).to eq([resource, resource_2, resource_3]) + expect(ordered_resources).to eq([resource_a, resource_b, resource_c]) end end @@ -106,96 +113,215 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do it 'returns catalog resources sorted by latest_released_at ascending with nulls last' do ordered_resources = described_class.order_by_latest_released_at_asc - expect(ordered_resources).to eq([resource_2, resource, resource_3]) + expect(ordered_resources).to eq([resource_b, resource_a, resource_c]) + end + end + + describe 'authorized catalog resources' do + let_it_be(:namespace) { create(:group) } + let_it_be(:other_namespace) { create(:group) } + let_it_be(:other_user) { create(:user) } + + let_it_be(:public_project) { create(:project, :public) } + let_it_be(:internal_project) { create(:project, :internal) } + let_it_be(:internal_namespace_project) { create(:project, :internal, namespace: namespace) } + let_it_be(:private_namespace_project) { create(:project, namespace: namespace) } + let_it_be(:other_private_namespace_project) { create(:project, namespace: other_namespace) } + + let_it_be(:public_resource) { create(:ci_catalog_resource, project: public_project) } + let_it_be(:internal_resource) { create(:ci_catalog_resource, project: internal_project) } + let_it_be(:internal_namespace_resource) { create(:ci_catalog_resource, project: internal_namespace_project) } + let_it_be(:private_namespace_resource) { create(:ci_catalog_resource, project: private_namespace_project) } + + let_it_be(:other_private_namespace_resource) do + create(:ci_catalog_resource, project: other_private_namespace_project) + end + + before_all do + namespace.add_reporter(current_user) + other_namespace.add_guest(other_user) + end + + describe '.public_or_visible_to_user' do + subject(:resources) { described_class.public_or_visible_to_user(current_user) } + + it 'returns all resources visible to the user' do + expect(resources).to contain_exactly( + public_resource, internal_resource, internal_namespace_resource, private_namespace_resource) + end + + context 'with a different user' do + let(:current_user) { other_user } + + it 'returns all resources visible to the user' do + expect(resources).to contain_exactly( + public_resource, internal_resource, internal_namespace_resource, other_private_namespace_resource) + end + end + + context 'when the user is nil' do + let(:current_user) { nil } + + it 'returns only public resources' do + expect(resources).to contain_exactly(public_resource) + end + end + end + + describe '.visible_to_user' do + subject(:resources) { described_class.visible_to_user(current_user) } + + it "returns resources belonging to the user's authorized namespaces" do + expect(resources).to contain_exactly(internal_namespace_resource, private_namespace_resource) + end + + context 'with a different user' do + let(:current_user) { other_user } + + it "returns resources belonging to the user's authorized namespaces" do + expect(resources).to contain_exactly(other_private_namespace_resource) + end + end + + context 'when the user is nil' do + let(:current_user) { nil } + + it 'does not return any resources' do + expect(resources).to be_empty + end + end end end describe '#state' do it 'defaults to draft' do - expect(resource.state).to eq('draft') + expect(resource_a.state).to eq('draft') end end describe '#publish!' do context 'when the catalog resource is in draft state' do it 'updates the state of the catalog resource to published' do - expect(resource.state).to eq('draft') + expect(resource_a.state).to eq('draft') - resource.publish! + resource_a.publish! - expect(resource.reload.state).to eq('published') + expect(resource_a.reload.state).to eq('published') end end - context 'when a catalog resource already has a published state' do + context 'when the catalog resource already has a published state' do it 'leaves the state as published' do - resource.update!(state: 'published') + resource_a.update!(state: :published) + expect(resource_a.state).to eq('published') - resource.publish! + resource_a.publish! - expect(resource.state).to eq('published') + expect(resource_a.state).to eq('published') end end end - describe '#unpublish!' do - context 'when the catalog resource is in published state' do - it 'updates the state to draft' do - resource.update!(state: :published) - expect(resource.state).to eq('published') + describe 'synchronizing denormalized columns with `projects` table', :sidekiq_inline do + let_it_be_with_reload(:project) { create(:project, name: 'Test project', description: 'Test description') } - resource.unpublish! + context 'when the catalog resource is created' do + let(:resource) { build(:ci_catalog_resource, project: project) } + + it 'updates the catalog resource columns to match the project' do + resource.save! + resource.reload - expect(resource.reload.state).to eq('draft') + expect(resource.name).to eq(project.name) + expect(resource.description).to eq(project.description) + expect(resource.visibility_level).to eq(project.visibility_level) end end - context 'when the catalog resource is already in draft state' do - it 'leaves the state as draft' do - expect(resource.state).to eq('draft') + context 'when the project is updated' do + let_it_be(:resource) { create(:ci_catalog_resource, project: project) } + + context 'when project name is updated' do + it 'updates the catalog resource name to match' do + project.update!(name: 'New name') + + expect(resource.reload.name).to eq(project.name) + end + end + + context 'when project description is updated' do + it 'updates the catalog resource description to match' do + project.update!(description: 'New description') + + expect(resource.reload.description).to eq(project.description) + end + end - resource.unpublish! + context 'when project visibility_level is updated' do + it 'updates the catalog resource visibility_level to match' do + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - expect(resource.reload.state).to eq('draft') + expect(resource.reload.visibility_level).to eq(project.visibility_level) + end end end end - describe 'sync with project' do - shared_examples 'denormalized columns of the catalog resource match the project' do - it do - expect(resource.name).to eq(project.name) - expect(resource.description).to eq(project.description) - expect(resource.visibility_level).to eq(project.visibility_level) - end + describe '#update_latest_released_at! triggered in model callbacks' do + let_it_be(:project) { create(:project) } + let_it_be(:resource) { create(:ci_catalog_resource, project: project) } + + let_it_be_with_refind(:january_release) do + create(:release, :with_catalog_resource_version, project: project, tag: 'v1', released_at: '2023-01-01T00:00:00Z') end - context 'when the catalog resource is created' do - it_behaves_like 'denormalized columns of the catalog resource match the project' + let_it_be_with_refind(:february_release) do + create(:release, :with_catalog_resource_version, project: project, tag: 'v2', released_at: '2023-02-01T00:00:00Z') end - context 'when the project name is updated' do - before do - project.update!(name: 'My new project name') - end + it 'has the expected latest_released_at value' do + expect(resource.reload.latest_released_at).to eq(february_release.released_at) + end + + context 'when a new catalog resource version is created' do + it 'updates the latest_released_at value' do + march_release = create(:release, :with_catalog_resource_version, project: project, tag: 'v3', + released_at: '2023-03-01T00:00:00Z') - it_behaves_like 'denormalized columns of the catalog resource match the project' + expect(resource.reload.latest_released_at).to eq(march_release.released_at) + end end - context 'when the project description is updated' do - before do - project.update!(description: 'My new description') + context 'when a catalog resource version is destroyed' do + it 'updates the latest_released_at value' do + february_release.catalog_resource_version.destroy! + + expect(resource.reload.latest_released_at).to eq(january_release.released_at) end + end + + context 'when the released_at value of a release is updated' do + it 'updates the latest_released_at value' do + january_release.update!(released_at: '2024-01-01T00:00:00Z') - it_behaves_like 'denormalized columns of the catalog resource match the project' + expect(resource.reload.latest_released_at).to eq(january_release.released_at) + end end - context 'when the project visibility_level is updated' do - before do - project.update!(visibility_level: 10) + context 'when a release is destroyed' do + it 'updates the latest_released_at value' do + february_release.destroy! + expect(resource.reload.latest_released_at).to eq(january_release.released_at) end + end - it_behaves_like 'denormalized columns of the catalog resource match the project' + context 'when all releases associated with the catalog resource are destroyed' do + it 'updates the latest_released_at value to nil' do + january_release.destroy! + february_release.destroy! + + expect(resource.reload.latest_released_at).to be_nil + end end end end diff --git a/spec/models/ci/catalog/resources/sync_event_spec.rb b/spec/models/ci/catalog/resources/sync_event_spec.rb new file mode 100644 index 00000000000..5d907aae9b6 --- /dev/null +++ b/spec/models/ci/catalog/resources/sync_event_spec.rb @@ -0,0 +1,190 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Catalog::Resources::SyncEvent, type: :model, feature_category: :pipeline_composition do + let_it_be_with_reload(:project1) { create(:project) } + let_it_be_with_reload(:project2) { create(:project) } + let_it_be(:resource1) { create(:ci_catalog_resource, project: project1) } + + it { is_expected.to belong_to(:catalog_resource).class_name('Ci::Catalog::Resource') } + it { is_expected.to belong_to(:project) } + + describe 'PG triggers' do + context 'when the associated project of a catalog resource is updated' do + context 'when project name is updated' do + it 'creates a sync event record' do + expect do + project1.update!(name: 'New name') + end.to change { described_class.count }.by(1) + end + end + + context 'when project description is updated' do + it 'creates a sync event record' do + expect do + project1.update!(description: 'New description') + end.to change { described_class.count }.by(1) + end + end + + context 'when project visibility_level is updated' do + it 'creates a sync event record' do + expect do + project1.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end.to change { described_class.count }.by(1) + end + end + end + + context 'when a project without an associated catalog resource is updated' do + it 'does not create a sync event record' do + expect do + project2.update!(name: 'New name') + end.not_to change { described_class.count } + end + end + end + + describe 'when there are sync event records' do + let_it_be(:resource2) { create(:ci_catalog_resource, project: project2) } + + before_all do + create(:ci_catalog_resource_sync_event, catalog_resource: resource1, status: :processed) + create(:ci_catalog_resource_sync_event, catalog_resource: resource1) + create_list(:ci_catalog_resource_sync_event, 2, catalog_resource: resource2) + end + + describe '.unprocessed_events' do + it 'returns the events in pending status' do + # 1 pending event from resource1 + 2 pending events from resource2 + expect(described_class.unprocessed_events.size).to eq(3) + end + + it 'selects the partition attribute in the result' do + described_class.unprocessed_events.each do |event| + expect(event.partition).not_to be_nil + end + end + end + + describe '.mark_records_processed' do + it 'updates the records to processed status' do + expect(described_class.status_pending.count).to eq(3) + expect(described_class.status_processed.count).to eq(1) + + described_class.mark_records_processed(described_class.unprocessed_events) + + expect(described_class.pluck(:status).uniq).to eq(['processed']) + + expect(described_class.status_pending.count).to eq(0) + expect(described_class.status_processed.count).to eq(4) + end + end + end + + describe '.upper_bound_count' do + it 'returns 0 when there are no records in the table' do + expect(described_class.upper_bound_count).to eq(0) + end + + it 'returns an estimated number of unprocessed records' do + create_list(:ci_catalog_resource_sync_event, 5, catalog_resource: resource1) + described_class.order(:id).limit(2).update_all(status: :processed) + + expect(described_class.upper_bound_count).to eq(3) + end + end + + describe 'sliding_list partitioning' do + let(:partition_manager) { Gitlab::Database::Partitioning::PartitionManager.new(described_class) } + + describe 'next_partition_if callback' do + let(:active_partition) { described_class.partitioning_strategy.active_partition } + + subject(:value) { described_class.partitioning_strategy.next_partition_if.call(active_partition) } + + context 'when the partition is empty' do + it { is_expected.to eq(false) } + end + + context 'when the partition has records' do + before do + create(:ci_catalog_resource_sync_event, catalog_resource: resource1, status: :processed) + create(:ci_catalog_resource_sync_event, catalog_resource: resource1) + end + + it { is_expected.to eq(false) } + end + + context 'when the first record of the partition is older than PARTITION_DURATION' do + before do + create(:ci_catalog_resource_sync_event, catalog_resource: resource1) + described_class.first.update!(created_at: (described_class::PARTITION_DURATION + 1.day).ago) + end + + it { is_expected.to eq(true) } + end + end + + describe 'detach_partition_if callback' do + let(:active_partition) { described_class.partitioning_strategy.active_partition } + + subject(:value) { described_class.partitioning_strategy.detach_partition_if.call(active_partition) } + + before_all do + create(:ci_catalog_resource_sync_event, catalog_resource: resource1, status: :processed) + create(:ci_catalog_resource_sync_event, catalog_resource: resource1) + end + + context 'when the partition contains unprocessed records' do + it { is_expected.to eq(false) } + end + + context 'when the partition contains only processed records' do + before do + described_class.update_all(status: :processed) + end + + it { is_expected.to eq(true) } + end + end + + describe 'strategy behavior' do + it 'moves records to new partitions as time passes', :freeze_time do + # We start with partition 1 + expect(described_class.partitioning_strategy.current_partitions.map(&:value)).to eq([1]) + + # Add one record so the initial partition is not empty + create(:ci_catalog_resource_sync_event, catalog_resource: resource1) + + # It's not a day old yet so no new partitions are created + partition_manager.sync_partitions + + expect(described_class.partitioning_strategy.current_partitions.map(&:value)).to eq([1]) + + # After traveling forward a day + travel(described_class::PARTITION_DURATION + 1.second) + + # a new partition is created + partition_manager.sync_partitions + + expect(described_class.partitioning_strategy.current_partitions.map(&:value)).to contain_exactly(1, 2) + + # and we can insert to the new partition + create(:ci_catalog_resource_sync_event, catalog_resource: resource1) + + # After processing records in partition 1 + described_class.mark_records_processed(described_class.for_partition(1).select_with_partition) + + partition_manager.sync_partitions + + # partition 1 is removed + expect(described_class.partitioning_strategy.current_partitions.map(&:value)).to eq([2]) + + # and we only have the newly created partition left. + expect(described_class.count).to eq(1) + end + end + end +end diff --git a/spec/models/ci/catalog/resources/version_spec.rb b/spec/models/ci/catalog/resources/version_spec.rb index 7114d2b6709..aafd51699b5 100644 --- a/spec/models/ci/catalog/resources/version_spec.rb +++ b/spec/models/ci/catalog/resources/version_spec.rb @@ -10,9 +10,6 @@ RSpec.describe Ci::Catalog::Resources::Version, type: :model, feature_category: it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:components).class_name('Ci::Catalog::Resources::Component') } - it { is_expected.to delegate_method(:name).to(:release) } - it { is_expected.to delegate_method(:description).to(:release) } - it { is_expected.to delegate_method(:tag).to(:release) } it { is_expected.to delegate_method(:sha).to(:release) } it { is_expected.to delegate_method(:released_at).to(:release) } it { is_expected.to delegate_method(:author_id).to(:release) } @@ -104,4 +101,51 @@ RSpec.describe Ci::Catalog::Resources::Version, type: :model, feature_category: end end end + + describe '#update_catalog_resource' do + let_it_be(:release) { create(:release, project: project1, tag: 'v1') } + let(:version) { build(:ci_catalog_resource_version, catalog_resource: resource1, release: release) } + + context 'when a version is created' do + it 'calls update_catalog_resource' do + expect(version).to receive(:update_catalog_resource).once + + version.save! + end + end + + context 'when a version is destroyed' do + it 'calls update_catalog_resource' do + version.save! + + expect(version).to receive(:update_catalog_resource).once + + version.destroy! + end + end + end + + describe '#name' do + it 'is equivalent to release.tag' do + release_v1_0.update!(name: 'Release v1.0') + + expect(v1_0.name).to eq(release_v1_0.tag) + end + end + + describe '#commit' do + subject(:commit) { v1_0.commit } + + it 'returns a commit' do + is_expected.to be_a(Commit) + end + + context 'when the sha is nil' do + it 'returns nil' do + release_v1_0.update!(sha: nil) + + is_expected.to be_nil + end + end + end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 48d46824c11..e65c1e2f577 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -176,16 +176,6 @@ RSpec.describe Ci::JobArtifact, feature_category: :build_artifacts do let!(:artifact) { build(:ci_job_artifact, :private) } it { is_expected.to be_falsey } - - context 'and the non_public_artifacts feature flag is disabled' do - let!(:artifact) { build(:ci_job_artifact, :private) } - - before do - stub_feature_flags(non_public_artifacts: false) - end - - it { is_expected.to be_truthy } - end end end diff --git a/spec/models/ci/job_token/scope_spec.rb b/spec/models/ci/job_token/scope_spec.rb index d41286f5a45..adb9f461f63 100644 --- a/spec/models/ci/job_token/scope_spec.rb +++ b/spec/models/ci/job_token/scope_spec.rb @@ -160,18 +160,6 @@ RSpec.describe Ci::JobToken::Scope, feature_category: :continuous_integration, f with_them do it { is_expected.to eq(result) } end - - context "with FF restrict_ci_job_token_for_public_and_internal_projects disabled" do - before do - stub_feature_flags(restrict_ci_job_token_for_public_and_internal_projects: false) - end - - let(:accessed_project) { unscoped_public_project } - - it "restricts public and internal outbound projects not in allowlist" do - is_expected.to eq(false) - end - end end end end diff --git a/spec/models/ci/pipeline_metadata_spec.rb b/spec/models/ci/pipeline_metadata_spec.rb index 977c90bcc2a..1a426118063 100644 --- a/spec/models/ci/pipeline_metadata_spec.rb +++ b/spec/models/ci/pipeline_metadata_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::PipelineMetadata do +RSpec.describe Ci::PipelineMetadata, feature_category: :pipeline_composition do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:pipeline) } @@ -10,5 +10,21 @@ RSpec.describe Ci::PipelineMetadata do it { is_expected.to validate_length_of(:name).is_at_least(1).is_at_most(255) } it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:pipeline) } + + it do + is_expected.to define_enum_for( + :auto_cancel_on_new_commit + ).with_values( + conservative: 0, interruptible: 1, disabled: 2 + ).with_prefix + end + + it do + is_expected.to define_enum_for( + :auto_cancel_on_job_failure + ).with_values( + none: 0, all: 1 + ).with_prefix + end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 9696ba7b3ee..024d3ae4240 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -86,6 +86,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: it { is_expected.to respond_to :short_sha } it { is_expected.to delegate_method(:full_path).to(:project).with_prefix } it { is_expected.to delegate_method(:name).to(:pipeline_metadata).allow_nil } + it { is_expected.to delegate_method(:auto_cancel_on_job_failure).to(:pipeline_metadata).allow_nil } describe 'validations' do it { is_expected.to validate_presence_of(:sha) } @@ -163,7 +164,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: before do stub_const('Ci::Refs::UnlockPreviousPipelinesWorker', unlock_previous_pipelines_worker_spy) - stub_feature_flags(ci_stop_unlock_pipelines: false) end shared_examples 'not unlocking pipelines' do |event:| @@ -202,42 +202,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: it_behaves_like 'unlocking pipelines', event: :skip it_behaves_like 'unlocking pipelines', event: :cancel it_behaves_like 'unlocking pipelines', event: :block - - context 'and ci_stop_unlock_pipelines is enabled' do - before do - stub_feature_flags(ci_stop_unlock_pipelines: true) - end - - it_behaves_like 'not unlocking pipelines', event: :succeed - it_behaves_like 'not unlocking pipelines', event: :drop - it_behaves_like 'not unlocking pipelines', event: :skip - it_behaves_like 'not unlocking pipelines', event: :cancel - it_behaves_like 'not unlocking pipelines', event: :block - end - - context 'and ci_unlock_non_successful_pipelines is disabled' do - before do - stub_feature_flags(ci_unlock_non_successful_pipelines: false) - end - - it_behaves_like 'unlocking pipelines', event: :succeed - it_behaves_like 'not unlocking pipelines', event: :drop - it_behaves_like 'not unlocking pipelines', event: :skip - it_behaves_like 'not unlocking pipelines', event: :cancel - it_behaves_like 'not unlocking pipelines', event: :block - - context 'and ci_stop_unlock_pipelines is enabled' do - before do - stub_feature_flags(ci_stop_unlock_pipelines: true) - end - - it_behaves_like 'not unlocking pipelines', event: :succeed - it_behaves_like 'not unlocking pipelines', event: :drop - it_behaves_like 'not unlocking pipelines', event: :skip - it_behaves_like 'not unlocking pipelines', event: :cancel - it_behaves_like 'not unlocking pipelines', event: :block - end - end end context 'when transitioning to a non-unlockable state' do @@ -246,14 +210,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end it_behaves_like 'not unlocking pipelines', event: :run - - context 'and ci_unlock_non_successful_pipelines is disabled' do - before do - stub_feature_flags(ci_unlock_non_successful_pipelines: false) - end - - it_behaves_like 'not unlocking pipelines', event: :run - end end end @@ -2028,17 +1984,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end - context 'when only_allow_merge_if_pipeline_succeeds? returns false and widget_pipeline_pass_subscription_update disabled' do - let(:only_allow_merge_if_pipeline_succeeds?) { false } - - before do - stub_feature_flags(widget_pipeline_pass_subscription_update: false) - end - - it_behaves_like 'state transition not triggering GraphQL subscription mergeRequestMergeStatusUpdated' - end - - context 'when only_allow_merge_if_pipeline_succeeds? returns false and widget_pipeline_pass_subscription_update enabled' do + context 'when only_allow_merge_if_pipeline_succeeds? returns false' do let(:only_allow_merge_if_pipeline_succeeds?) { false } it_behaves_like 'triggers GraphQL subscription mergeRequestMergeStatusUpdated' do @@ -3848,6 +3794,44 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end + describe '#set_failed' do + let(:pipeline) { build(:ci_pipeline) } + + it 'marks the pipeline as failed with the given reason without saving', :aggregate_failures do + pipeline.set_failed(:filtered_by_rules) + + expect(pipeline).to be_failed + expect(pipeline).to be_filtered_by_rules + expect(pipeline).not_to be_persisted + end + end + + describe '#filtered_as_empty?' do + let(:pipeline) { build_stubbed(:ci_pipeline) } + + subject { pipeline.filtered_as_empty? } + + it { is_expected.to eq false } + + context 'when the pipeline is failed' do + using RSpec::Parameterized::TableSyntax + + where(:drop_reason, :expected) do + :unknown_failure | false + :filtered_by_rules | true + :filtered_by_workflow_rules | true + end + + with_them do + before do + pipeline.set_failed(drop_reason) + end + + it { is_expected.to eq expected } + end + end + end + describe '#has_yaml_errors?' do let(:pipeline) { build_stubbed(:ci_pipeline) } @@ -4065,8 +4049,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: describe '#builds_in_self_and_project_descendants' do subject(:builds) { pipeline.builds_in_self_and_project_descendants } - let(:pipeline) { create(:ci_pipeline) } - let!(:build) { create(:ci_build, pipeline: pipeline) } + let_it_be_with_refind(:pipeline) { create(:ci_pipeline) } + let_it_be(:build) { create(:ci_build, pipeline: pipeline) } context 'when pipeline is standalone' do it 'returns the list of builds' do @@ -4093,6 +4077,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: expect(builds).to contain_exactly(build, child_build, child_of_child_build) end end + + it 'includes partition_id filter' do + expect(builds.where_values_hash).to match(a_hash_including('partition_id' => pipeline.partition_id)) + end end describe '#build_with_artifacts_in_self_and_project_descendants' do @@ -4118,7 +4106,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: describe '#jobs_in_self_and_project_descendants' do subject(:jobs) { pipeline.jobs_in_self_and_project_descendants } - let(:pipeline) { create(:ci_pipeline) } + let_it_be_with_refind(:pipeline) { create(:ci_pipeline) } shared_examples_for 'fetches jobs in self and project descendant pipelines' do |factory_type| let!(:job) { create(factory_type, pipeline: pipeline) } @@ -4151,6 +4139,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: expect(jobs).to contain_exactly(job, child_job, child_of_child_job, child_source_bridge, child_of_child_source_bridge) end end + + it 'includes partition_id filter' do + expect(jobs.where_values_hash).to match(a_hash_including('partition_id' => pipeline.partition_id)) + end end context 'when job is build' do @@ -5651,6 +5643,22 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end + describe '.current_partition_value' do + subject { described_class.current_partition_value } + + it { is_expected.to eq(101) } + + it 'accepts an optional argument' do + expect(described_class.current_partition_value(build_stubbed(:project))).to eq(101) + end + + it 'returns 100 when the flag is disabled' do + stub_feature_flags(ci_current_partition_value_101: false) + + is_expected.to eq(100) + end + end + describe '#notes=' do context 'when notes already exist' do it 'does not create duplicate notes', :aggregate_failures do diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 8c0143d5f18..5d457c4f213 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -58,7 +58,7 @@ RSpec.describe Ci::Processable, feature_category: :continuous_integration do let(:clone_accessors) do %i[pipeline project ref tag options name allow_failure stage stage_idx trigger_request yaml_variables when environment coverage_regex description tag_list protected needs_attributes job_variables_attributes - resource_group scheduling_type ci_stage partition_id id_tokens] + resource_group scheduling_type ci_stage partition_id id_tokens interruptible] end let(:reject_accessors) do @@ -76,7 +76,7 @@ RSpec.describe Ci::Processable, feature_category: :continuous_integration do job_artifacts_network_referee job_artifacts_dotenv job_artifacts_cobertura needs job_artifacts_accessibility job_artifacts_requirements job_artifacts_coverage_fuzzing - job_artifacts_requirements_v2 + job_artifacts_requirements_v2 job_artifacts_repository_xray job_artifacts_api_fuzzing terraform_state_versions job_artifacts_cyclonedx job_annotations job_artifacts_annotations].freeze end @@ -114,7 +114,8 @@ RSpec.describe Ci::Processable, feature_category: :continuous_integration do shared_examples_for 'clones the processable' do before_all do - processable.update!(stage: 'test', stage_id: stage.id) + processable.assign_attributes(stage: 'test', stage_id: stage.id, interruptible: true) + processable.save! create(:ci_build_need, build: processable) end @@ -187,7 +188,7 @@ RSpec.describe Ci::Processable, feature_category: :continuous_integration do Ci::Build.attribute_names.map(&:to_sym) + Ci::Build.attribute_aliases.keys.map(&:to_sym) + Ci::Build.reflect_on_all_associations.map(&:name) + - [:tag_list, :needs_attributes, :job_variables_attributes, :id_tokens] + [:tag_list, :needs_attributes, :job_variables_attributes, :id_tokens, :interruptible] current_accessors.uniq! diff --git a/spec/models/ci/runner_manager_build_spec.rb b/spec/models/ci/runner_manager_build_spec.rb index 3a381313b76..a4dd3a2c748 100644 --- a/spec/models/ci/runner_manager_build_spec.rb +++ b/spec/models/ci/runner_manager_build_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::RunnerManagerBuild, model: true, feature_category: :runner_fleet do +RSpec.describe Ci::RunnerManagerBuild, model: true, feature_category: :fleet_visibility do let_it_be(:runner) { create(:ci_runner) } let_it_be(:runner_manager) { create(:ci_runner_machine, runner: runner) } let_it_be(:build) { create(:ci_build, runner_manager: runner_manager) } diff --git a/spec/models/ci/runner_manager_spec.rb b/spec/models/ci/runner_manager_spec.rb index 01275ffd31c..02a72afe0c6 100644 --- a/spec/models/ci/runner_manager_spec.rb +++ b/spec/models/ci/runner_manager_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::RunnerManager, feature_category: :runner_fleet, type: :model do +RSpec.describe Ci::RunnerManager, feature_category: :fleet_visibility, type: :model do it_behaves_like 'having unique enum values' it_behaves_like 'it has loose foreign keys' do diff --git a/spec/models/ci/runner_version_spec.rb b/spec/models/ci/runner_version_spec.rb index bce1f2a6c39..32f840a8034 100644 --- a/spec/models/ci/runner_version_spec.rb +++ b/spec/models/ci/runner_version_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::RunnerVersion, feature_category: :runner_fleet do +RSpec.describe Ci::RunnerVersion, feature_category: :fleet_visibility do let_it_be(:runner_version_upgrade_recommended) do create(:ci_runner_version, version: 'abc234', status: :recommended) end diff --git a/spec/models/concerns/ci/partitionable_spec.rb b/spec/models/concerns/ci/partitionable_spec.rb index 6daafc78cff..735b81f54bc 100644 --- a/spec/models/concerns/ci/partitionable_spec.rb +++ b/spec/models/concerns/ci/partitionable_spec.rb @@ -5,6 +5,12 @@ require 'spec_helper' RSpec.describe Ci::Partitionable do let(:ci_model) { Class.new(Ci::ApplicationRecord) } + around do |ex| + Gitlab::Database::SharedModel.using_connection(ci_model.connection) do + ex.run + end + end + describe 'partitionable models inclusion' do subject { ci_model.include(described_class) } @@ -61,10 +67,58 @@ RSpec.describe Ci::Partitionable do context 'when partitioned is true' do let(:partitioned) { true } + let(:partitioning_strategy) { ci_model.partitioning_strategy } it { expect(ci_model.ancestors).to include(PartitionedTable) } - it { expect(ci_model.partitioning_strategy).to be_a(Gitlab::Database::Partitioning::CiSlidingListStrategy) } - it { expect(ci_model.partitioning_strategy.partitioning_key).to eq(:partition_id) } + it { expect(partitioning_strategy).to be_a(Gitlab::Database::Partitioning::CiSlidingListStrategy) } + it { expect(partitioning_strategy.partitioning_key).to eq(:partition_id) } + + describe 'next_partition_if callback' do + let(:active_partition) { partitioning_strategy.active_partition } + + let(:table_options) do + { + primary_key: [:id, :partition_id], + options: 'PARTITION BY LIST (partition_id)', + if_not_exists: false + } + end + + before do + ci_model.connection.create_table(:_test_table_name, **table_options) do |t| + t.bigserial :id, null: false + t.bigint :partition_id, null: false + end + + ci_model.table_name = :_test_table_name + end + + subject(:value) { partitioning_strategy.next_partition_if.call(active_partition) } + + context 'without any existing partitions' do + it { is_expected.to eq(true) } + end + + context 'with initial partition attached' do + before do + ci_model.connection.execute(<<~SQL) + CREATE TABLE IF NOT EXISTS _test_table_name_100 PARTITION OF _test_table_name FOR VALUES IN (100); + SQL + end + + it { is_expected.to eq(true) } + end + + context 'with an existing partition for partition_id = 101' do + before do + ci_model.connection.execute(<<~SQL) + CREATE TABLE IF NOT EXISTS _test_table_name_101 PARTITION OF _test_table_name FOR VALUES IN (101); + SQL + end + + it { is_expected.to eq(false) } + end + end end context 'when partitioned is false' do @@ -74,4 +128,30 @@ RSpec.describe Ci::Partitionable do it { expect(ci_model).not_to respond_to(:partitioning_strategy) } end end + + describe '.in_partition' do + before do + stub_const("#{described_class}::Testing::PARTITIONABLE_MODELS", [ci_model.name]) + ci_model.table_name = :p_ci_builds + ci_model.include(described_class) + end + + subject(:scope_values) { ci_model.in_partition(value).where_values_hash } + + context 'with integer parameters' do + let(:value) { 101 } + + it 'adds a partition_id filter' do + expect(scope_values).to include('partition_id' => 101) + end + end + + context 'with partitionable records' do + let(:value) { build_stubbed(:ci_pipeline, partition_id: 101) } + + it 'adds a partition_id filter' do + expect(scope_values).to include('partition_id' => 101) + end + end + end end diff --git a/spec/models/concerns/disables_sti_spec.rb b/spec/models/concerns/disables_sti_spec.rb new file mode 100644 index 00000000000..07eea635289 --- /dev/null +++ b/spec/models/concerns/disables_sti_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe DisablesSti, feature_category: :shared do + describe '.allow_legacy_sti_class' do + it 'is nil by default' do + expect(ApplicationRecord.allow_legacy_sti_class).to eq(nil) + end + + it 'is true on legacy models' do + expect(PersonalSnippet.allow_legacy_sti_class).to eq(true) + end + end +end diff --git a/spec/models/concerns/enums/sbom_spec.rb b/spec/models/concerns/enums/sbom_spec.rb index e2f56cc637d..3bbdf619a8c 100644 --- a/spec/models/concerns/enums/sbom_spec.rb +++ b/spec/models/concerns/enums/sbom_spec.rb @@ -3,9 +3,9 @@ require "spec_helper" RSpec.describe Enums::Sbom, feature_category: :dependency_management do - describe '.purl_types' do - using RSpec::Parameterized::TableSyntax + using RSpec::Parameterized::TableSyntax + describe '.purl_types' do subject(:actual_purl_type) { described_class.purl_types[package_manager] } where(:given_package_manager, :expected_purl_type) do @@ -35,5 +35,63 @@ RSpec.describe Enums::Sbom, feature_category: :dependency_management do expect(actual_purl_type).to eql(expected_purl_type) end end + + it 'contains all of the dependency scanning and container scanning purl types' do + expect(described_class::DEPENDENCY_SCANNING_PURL_TYPES + described_class::CONTAINER_SCANNING_PURL_TYPES) + .to eql(described_class::PURL_TYPES.keys) + end + end + + describe '.dependency_scanning_purl_type?' do + where(:purl_type, :expected) do + :composer | false + 'composer' | true + 'conan' | true + 'gem' | true + 'golang' | true + 'maven' | true + 'npm' | true + 'nuget' | true + 'pypi' | true + 'unknown' | false + 'apk' | false + 'rpm' | false + 'deb' | false + 'wolfi' | false + end + + with_them do + it 'returns true if the purl_type is for dependency_scanning' do + actual = described_class.dependency_scanning_purl_type?(purl_type) + expect(actual).to eql(expected) + end + end + end + + describe '.container_scanning_purl_type?' do + where(:purl_type, :expected) do + 'composer' | false + 'conan' | false + 'gem' | false + 'golang' | false + 'maven' | false + 'npm' | false + 'nuget' | false + 'pypi' | false + 'unknown' | false + :apk | false + 'apk' | true + 'rpm' | true + 'deb' | true + 'cbl-mariner' | true + 'wolfi' | true + end + + with_them do + it 'returns true if the purl_type is for container_scanning' do + actual = described_class.container_scanning_purl_type?(purl_type) + expect(actual).to eql(expected) + end + end end end diff --git a/spec/models/concerns/ignorable_columns_spec.rb b/spec/models/concerns/ignorable_columns_spec.rb index 339f06f9c45..44dc0bb6da6 100644 --- a/spec/models/concerns/ignorable_columns_spec.rb +++ b/spec/models/concerns/ignorable_columns_spec.rb @@ -27,6 +27,12 @@ RSpec.describe IgnorableColumns do expect { subject.ignore_columns(:name, remove_after: nil, remove_with: 12.6) }.to raise_error(ArgumentError, /Please indicate/) end + it 'allows setting remove_never: true and not setting other remove options' do + expect do + subject.ignore_columns(%i[name created_at], remove_never: true) + end.to change { subject.ignored_columns }.from([]).to(%w[name created_at]) + end + it 'requires remove_after attribute to be set' do expect { subject.ignore_columns(:name, remove_after: "not a date", remove_with: 12.6) }.to raise_error(ArgumentError, /Please indicate/) end @@ -73,9 +79,11 @@ RSpec.describe IgnorableColumns do end describe IgnorableColumns::ColumnIgnore do - subject { described_class.new(remove_after, remove_with) } + subject { described_class.new(remove_after, remove_with, remove_never) } + let(:remove_after) { nil } let(:remove_with) { double } + let(:remove_never) { false } describe '#safe_to_remove?' do context 'after remove_after date has passed' do @@ -93,6 +101,14 @@ RSpec.describe IgnorableColumns do expect(subject.safe_to_remove?).to be_falsey end end + + context 'with remove_never: true' do + let(:remove_never) { true } + + it 'is false' do + expect(subject.safe_to_remove?).to be_falsey + end + end end end end diff --git a/spec/models/concerns/pg_full_text_searchable_spec.rb b/spec/models/concerns/pg_full_text_searchable_spec.rb index 7da48489e12..f3289408643 100644 --- a/spec/models/concerns/pg_full_text_searchable_spec.rb +++ b/spec/models/concerns/pg_full_text_searchable_spec.rb @@ -87,6 +87,12 @@ RSpec.describe PgFullTextSearchable, feature_category: :global_search do [english, with_accent, japanese].each(&:update_search_data!) end + it 'builds a search query using `search_vector` from the search_data table' do + sql = model_class.pg_full_text_search('test').to_sql + + expect(sql).to include('"issue_search_data"."search_vector" @@ to_tsquery') + end + it 'searches across all fields' do expect(model_class.pg_full_text_search('title english')).to contain_exactly(english, japanese) end @@ -158,6 +164,14 @@ RSpec.describe PgFullTextSearchable, feature_category: :global_search do end end + describe '.pg_full_text_search_in_model' do + it 'builds a search query using `search_vector` from the model table' do + sql = model_class.pg_full_text_search_in_model('test').to_sql + + expect(sql).to include('"issues"."search_vector" @@ to_tsquery') + end + end + describe '#update_search_data!' do let(:model) { model_class.create!(project: project, namespace: project.project_namespace, title: 'title', description: 'description') } diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 7e324812b97..e71392f7bbc 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.shared_examples 'routable resource' do - shared_examples_for '.find_by_full_path' do |has_cross_join: false| + shared_examples_for '.find_by_full_path' do it 'finds records by their full path' do expect(described_class.find_by_full_path(record.full_path)).to eq(record) expect(described_class.find_by_full_path(record.full_path.upcase)).to eq(record) @@ -46,22 +46,98 @@ RSpec.shared_examples 'routable resource' do end end - if has_cross_join - it 'has a cross-join' do - expect(Gitlab::Database).to receive(:allow_cross_joins_across_databases) + it 'does not have cross-join' do + expect(Gitlab::Database).not_to receive(:allow_cross_joins_across_databases) - described_class.find_by_full_path(record.full_path) + described_class.find_by_full_path(record.full_path) + end + end + + it_behaves_like '.find_by_full_path', :aggregate_failures + + shared_examples_for '.where_full_path_in' do + context 'without any paths' do + it 'returns an empty relation' do + expect(described_class.where_full_path_in([])).to eq([]) + end + end + + context 'without any valid paths' do + it 'returns an empty relation' do + expect(described_class.where_full_path_in(%w[unknown])).to eq([]) + end + end + + context 'with valid paths' do + it 'returns the entities matching the paths' do + result = described_class.where_full_path_in([record.full_path, record_2.full_path]) + + expect(result).to contain_exactly(record, record_2) + end + + it 'returns entities regardless of the casing of paths' do + result = described_class.where_full_path_in([record.full_path.upcase, record_2.full_path.upcase]) + + expect(result).to contain_exactly(record, record_2) + end + end + + context 'on the usage of `use_includes` parameter' do + let_it_be(:klass) { record.class.to_s.downcase } + let_it_be(:record_3) { create(:"#{klass}") } + let_it_be(:record_4) { create(:"#{klass}") } + + context 'when use_includes: true' do + it 'includes route information when loading records' do + control_count = ActiveRecord::QueryRecorder.new do + described_class.where_full_path_in([record.full_path, record_2.full_path], use_includes: true) + .map(&:route) + end + + expect do + described_class.where_full_path_in( + [ + record.full_path, + record_2.full_path, + record_3.full_path, + record_4.full_path + ], use_includes: true) + .map(&:route) + end.to issue_same_number_of_queries_as(control_count) + end end - else - it 'does not have cross-join' do - expect(Gitlab::Database).not_to receive(:allow_cross_joins_across_databases) - described_class.find_by_full_path(record.full_path) + context 'when use_includes: false' do + it 'does not include route information when loading records' do + control_count = ActiveRecord::QueryRecorder.new do + described_class.where_full_path_in([record.full_path, record_2.full_path], use_includes: false) + .map(&:route) + end + + expect do + described_class.where_full_path_in( + [ + record.full_path, + record_2.full_path, + record_3.full_path, + record_4.full_path + ], use_includes: false) + .map(&:route) + end.not_to issue_same_number_of_queries_as(control_count) + end end end end - it_behaves_like '.find_by_full_path', :aggregate_failures + it_behaves_like '.where_full_path_in', :aggregate_failures + + context 'when the `optimize_where_full_path_in` feature flag is turned OFF' do + before do + stub_feature_flags(optimize_where_full_path_in: false) + end + + it_behaves_like '.where_full_path_in', :aggregate_failures + end end RSpec.shared_examples 'routable resource with parent' do @@ -105,10 +181,12 @@ RSpec.describe Group, 'Routable', :with_clean_rails_cache, feature_category: :gr it_behaves_like 'routable resource' do let_it_be(:record) { group } + let_it_be(:record_2) { nested_group } end it_behaves_like 'routable resource with parent' do let_it_be(:record) { nested_group } + let_it_be(:record_2) { group } end describe 'Validations' do @@ -169,34 +247,6 @@ RSpec.describe Group, 'Routable', :with_clean_rails_cache, feature_category: :gr expect(group.route.namespace).to eq(group) end - describe '.where_full_path_in' do - context 'without any paths' do - it 'returns an empty relation' do - expect(described_class.where_full_path_in([])).to eq([]) - end - end - - context 'without any valid paths' do - it 'returns an empty relation' do - expect(described_class.where_full_path_in(%w[unknown])).to eq([]) - end - end - - context 'with valid paths' do - it 'returns the projects matching the paths' do - result = described_class.where_full_path_in([group.to_param, nested_group.to_param]) - - expect(result).to contain_exactly(group, nested_group) - end - - it 'returns projects regardless of the casing of paths' do - result = described_class.where_full_path_in([group.to_param.upcase, nested_group.to_param.upcase]) - - expect(result).to contain_exactly(group, nested_group) - end - end - end - describe '#parent_loaded?' do before do group.parent = create(:group) @@ -232,9 +282,11 @@ end RSpec.describe Project, 'Routable', :with_clean_rails_cache, feature_category: :groups_and_projects do let_it_be(:namespace) { create(:namespace) } let_it_be(:project) { create(:project, namespace: namespace) } + let_it_be(:project_2) { create(:project) } it_behaves_like 'routable resource with parent' do let_it_be(:record) { project } + let_it_be(:record_2) { project_2 } end it 'creates route with namespace referencing project namespace' do @@ -252,6 +304,17 @@ RSpec.describe Project, 'Routable', :with_clean_rails_cache, feature_category: : expect(record).to be_nil end end + + describe '.where_full_path_in' do + it 'does not return records if the sources are different, but the IDs match' do + group = create(:group, id: 1992) + project = create(:project, id: 1992) + + records = described_class.where(id: project.id).where_full_path_in([group.full_path]) + + expect(records).to be_empty + end + end end RSpec.describe Namespaces::ProjectNamespace, 'Routable', :with_clean_rails_cache, feature_category: :groups_and_projects do diff --git a/spec/models/concerns/transitionable_spec.rb b/spec/models/concerns/transitionable_spec.rb index b80d363ef78..c8cba1ae226 100644 --- a/spec/models/concerns/transitionable_spec.rb +++ b/spec/models/concerns/transitionable_spec.rb @@ -22,19 +22,16 @@ RSpec.describe Transitionable, feature_category: :code_review_workflow do let(:object) { klass.new(transitioning) } describe '#transitioning?' do - where(:transitioning, :feature_flag, :result) do - true | true | true - false | false | false - true | false | false - false | true | false + context "when trasnitioning" do + let(:transitioning) { true } + + it { expect(object.transitioning?).to eq(true) } end - with_them do - before do - stub_feature_flags(skip_validations_during_transitions: feature_flag) - end + context "when not trasnitioning" do + let(:transitioning) { false } - it { expect(object.transitioning?).to eq(result) } + it { expect(object.transitioning?).to eq(false) } end end end diff --git a/spec/models/concerns/triggerable_hooks_spec.rb b/spec/models/concerns/triggerable_hooks_spec.rb index 28cda269458..c209d6476f3 100644 --- a/spec/models/concerns/triggerable_hooks_spec.rb +++ b/spec/models/concerns/triggerable_hooks_spec.rb @@ -10,6 +10,8 @@ RSpec.describe TriggerableHooks do include TriggerableHooks # rubocop:disable RSpec/DescribedClass triggerable_hooks [:push_hooks] + self.allow_legacy_sti_class = true + scope :executable, -> { all } end end diff --git a/spec/models/concerns/vulnerability_finding_helpers_spec.rb b/spec/models/concerns/vulnerability_finding_helpers_spec.rb deleted file mode 100644 index 023ecccb520..00000000000 --- a/spec/models/concerns/vulnerability_finding_helpers_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe VulnerabilityFindingHelpers do - let(:cls) do - Class.new do - include VulnerabilityFindingHelpers - - attr_accessor :report_type - - def initialize(report_type) - @report_type = report_type - end - end - end - - describe '#requires_manual_resolution?' do - it 'returns false if the finding does not require manual resolution' do - expect(cls.new('sast').requires_manual_resolution?).to eq(false) - end - - it 'returns true when the finding requires manual resolution' do - expect(cls.new('secret_detection').requires_manual_resolution?).to eq(true) - end - end -end diff --git a/spec/models/container_registry/protection/rule_spec.rb b/spec/models/container_registry/protection/rule_spec.rb index 9f162736efd..1706fcf76ae 100644 --- a/spec/models/container_registry/protection/rule_spec.rb +++ b/spec/models/container_registry/protection/rule_spec.rb @@ -38,9 +38,9 @@ RSpec.describe ContainerRegistry::Protection::Rule, type: :model, feature_catego describe 'validations' do subject { build(:container_registry_protection_rule) } - describe '#container_path_pattern' do - it { is_expected.to validate_presence_of(:container_path_pattern) } - it { is_expected.to validate_length_of(:container_path_pattern).is_at_most(255) } + describe '#repository_path_pattern' do + it { is_expected.to validate_presence_of(:repository_path_pattern) } + it { is_expected.to validate_length_of(:repository_path_pattern).is_at_most(255) } end describe '#delete_protected_up_to_access_level' do diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 027fd20462b..fb32c796016 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -713,7 +713,7 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont { name: 'latest', digest: 'sha256:6c3c624b58dbbcd3c0dd82b4c53f04191247c6eebdaab7c610cf7d66709b3', - config_digest: 'sha256:66b1132a0173910b01ee694462c99efbe1b9ab5bf8083231232312', + config_digest: nil, media_type: 'application/vnd.oci.image.manifest.v1+json', size_bytes: 1234567892, created_at: 10.minutes.ago, @@ -742,7 +742,7 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont expect(return_value[:pagination]).to eq(response_body[:pagination]) return_value[:tags].each_with_index do |tag, index| - expected_revision = tags_response[index][:config_digest].to_s.split(':')[1] + expected_revision = tags_response[index][:config_digest].to_s.split(':')[1].to_s expect(tag.is_a?(ContainerRegistry::Tag)).to eq(true) expect(tag).to have_attributes( diff --git a/spec/models/custom_emoji_spec.rb b/spec/models/custom_emoji_spec.rb index 15655d08556..cbdf05cf28f 100644 --- a/spec/models/custom_emoji_spec.rb +++ b/spec/models/custom_emoji_spec.rb @@ -48,4 +48,45 @@ RSpec.describe CustomEmoji do expect(emoji.errors.messages).to eq(file: ["is blocked: Only allowed schemes are http, https"]) end end + + describe '#for_resource' do + let_it_be(:group) { create(:group) } + let_it_be(:custom_emoji) { create(:custom_emoji, namespace: group) } + + context 'when custom_emoji feature flag is disabled' do + before do + stub_feature_flags(custom_emoji: false) + end + + it { expect(described_class.for_resource(group)).to eq([]) } + end + + context 'when group is nil' do + let_it_be(:group) { nil } + + it { expect(described_class.for_resource(group)).to eq([]) } + end + + context 'when resource is a project' do + let_it_be(:project) { create(:project) } + + it { expect(described_class.for_resource(project)).to eq([]) } + end + + it { expect(described_class.for_resource(group)).to eq([custom_emoji]) } + end + + describe '#for_namespaces' do + let_it_be(:group) { create(:group) } + let_it_be(:custom_emoji) { create(:custom_emoji, namespace: group, name: 'parrot') } + + it { expect(described_class.for_namespaces([group.id])).to eq([custom_emoji]) } + + context 'with subgroup' do + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:subgroup_emoji) { create(:custom_emoji, namespace: subgroup, name: 'parrot') } + + it { expect(described_class.for_namespaces([subgroup.id, group.id])).to eq([subgroup_emoji]) } + end + end end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 1b8dd62455e..36479dffe21 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -473,4 +473,12 @@ RSpec.describe DeployToken, feature_category: :continuous_delivery do expect(subject.impersonated?).to be(false) end end + + describe '.token' do + # Specify a blank token_encrypted so that the model's method is used + # instead of the factory value + subject(:plaintext) { create(:deploy_token, token_encrypted: nil).token } + + it { is_expected.to match(/gldt-[A-Za-z0-9_-]{20}/) } + end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index ee48e8cac6c..cb2c38c15e0 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -1270,10 +1270,15 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do shared_examples_for 'gracefully handling error' do it 'tracks an exception' do - expect(Gitlab::ErrorTracking).to receive(:track_exception).with( - instance_of(described_class::StatusSyncError), - deployment_id: deployment.id, - job_id: job.id) + expect(Gitlab::ErrorTracking).to( + receive(:track_exception).with( + instance_of(described_class::StatusSyncError), + deployment_id: deployment.id, + job_id: job.id + ) do |error| + expect(error.backtrace).to be_present + end + ) is_expected.to eq(false) diff --git a/spec/models/diff_viewer/base_spec.rb b/spec/models/diff_viewer/base_spec.rb index 8ab7b090928..17925ef2576 100644 --- a/spec/models/diff_viewer/base_spec.rb +++ b/spec/models/diff_viewer/base_spec.rb @@ -100,6 +100,28 @@ RSpec.describe DiffViewer::Base do end end + describe '#generated?' do + before do + allow(diff_file).to receive(:generated?).and_return(generated) + end + + context 'when the diff file is generated' do + let(:generated) { true } + + it 'returns true' do + expect(viewer.generated?).to be_truthy + end + end + + context 'when the diff file is not generated' do + let(:generated) { false } + + it 'returns true' do + expect(viewer.generated?).to be_falsey + end + end + end + describe '#render_error' do context 'when the combined blob size is larger than the size limit' do before do diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 3e4fe57c59b..6eb69bf958a 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -68,15 +68,6 @@ RSpec.describe Event, feature_category: :user_profile do end.not_to change { project.last_repository_updated_at } end end - - describe 'after_create UserInteractedProject.track' do - let(:event) { build(:push_event, project: project, author: project.first_owner) } - - it 'passes event to UserInteractedProject.track' do - expect(UserInteractedProject).to receive(:track).with(event) - event.save! - end - end end describe 'validations' do @@ -115,6 +106,18 @@ RSpec.describe Event, feature_category: :user_profile do end end + describe '.for_merge_request' do + let(:mr_event) { create(:event, :for_merge_request, project: project) } + + before do + create(:event, :for_issue, project: project) + end + + it 'returns events for MergeRequest target_type' do + expect(described_class.for_merge_request).to contain_exactly(mr_event) + end + end + describe '.created_at' do it 'can find the right event' do time = 1.day.ago @@ -128,6 +131,21 @@ RSpec.describe Event, feature_category: :user_profile do end end + describe '.created_between' do + it 'returns events created between given timestamps' do + start_time = 2.days.ago + end_time = Date.today + + create(:event, created_at: 3.days.ago) + e1 = create(:event, created_at: 2.days.ago) + e2 = create(:event, created_at: 1.day.ago) + + found = described_class.created_between(start_time, end_time) + + expect(found).to contain_exactly(e1, e2) + end + end + describe '.for_fingerprint' do let_it_be(:with_fingerprint) { create(:event, fingerprint: 'aaa', project: project) } @@ -152,16 +170,28 @@ RSpec.describe Event, feature_category: :user_profile do end describe '.contributions' do - let!(:merge_request_event) { create(:event, :created, :for_merge_request, project: project) } - let!(:issue_event) { create(:event, :created, :for_issue, project: project) } + let!(:merge_request_events) do + %i[created closed merged approved].map do |action| + create(:event, :for_merge_request, action: action, project: project) + end + end + let!(:work_item_event) { create(:event, :created, :for_work_item, project: project) } - let!(:design_event) { create(:design_event, project: project) } + let!(:issue_events) do + %i[created closed].map { |action| create(:event, :for_issue, action: action, project: project) } + end - it 'returns events for MergeRequest, Issue and WorkItem' do + let!(:push_event) { create_push_event(project, project.owner) } + let!(:comment_event) { create(:event, :commented, project: project) } + + before do + create(:design_event, project: project) # should not be in scope + end + + it 'returns events for MergeRequest, Issue, WorkItem and push, comment events' do expect(described_class.contributions).to contain_exactly( - merge_request_event, - issue_event, - work_item_event + *merge_request_events, *issue_events, work_item_event, + push_event, comment_event ) end end @@ -881,7 +911,7 @@ RSpec.describe Event, feature_category: :user_profile do context 'when a project was updated more than 1 hour ago', :clean_gitlab_redis_shared_state do before do ::Gitlab::Redis::SharedState.with do |redis| - redis.hset('inactive_projects_deletion_warning_email_notified', "project:#{project.id}", Date.current) + redis.hset('inactive_projects_deletion_warning_email_notified', "project:#{project.id}", Date.current.to_s) end end @@ -1146,4 +1176,11 @@ RSpec.describe Event, feature_category: :user_profile do event end + + context 'with loose foreign key on events.author_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let_it_be(:parent) { create(:user) } + let_it_be(:model) { create(:event, author: parent) } + end + end end diff --git a/spec/models/every_model_spec.rb b/spec/models/every_model_spec.rb new file mode 100644 index 00000000000..479fc8d3dfa --- /dev/null +++ b/spec/models/every_model_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Every model', feature_category: :shared do + describe 'disallows STI', :eager_load do + include_examples 'Model disables STI' do + let(:models) { ApplicationRecord.descendants.reject(&:abstract_class?) } + end + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 7a31067732f..1fafa64a535 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -11,10 +11,11 @@ RSpec.describe Group, feature_category: :groups_and_projects do describe 'associations' do it { is_expected.to have_many :projects } it { is_expected.to have_many(:all_group_members).dependent(:destroy) } + it { is_expected.to have_many(:all_owner_members) } it { is_expected.to have_many(:group_members).dependent(:destroy) } it { is_expected.to have_many(:namespace_members) } it { is_expected.to have_many(:users).through(:group_members) } - it { is_expected.to have_many(:owners).through(:all_group_members) } + it { is_expected.to have_many(:owners).through(:all_owner_members) } it { is_expected.to have_many(:requesters).dependent(:destroy) } it { is_expected.to have_many(:namespace_requesters) } it { is_expected.to have_many(:members_and_requesters) } @@ -391,12 +392,19 @@ RSpec.describe Group, feature_category: :groups_and_projects do expect(internal_group.errors[:visibility_level]).to include('private is not allowed since this group contains projects with higher visibility.') end - it 'is valid if higher visibility project is deleted' do + it 'is valid if higher visibility project is currently undergoing deletion' do internal_project.update_attribute(:pending_delete, true) internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE expect(internal_group).to be_valid end + + it 'is valid if higher visibility project is pending deletion via marked_for_deletion_at' do + internal_project.update_attribute(:marked_for_deletion_at, Time.current) + internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE + + expect(internal_group).to be_valid + end end context 'when group has a higher visibility' do @@ -917,6 +925,18 @@ RSpec.describe Group, feature_category: :groups_and_projects do it { is_expected.to eq([group_1, group_2, group_4, group_3]) } end + context 'when sort by path_asc' do + let(:sort) { 'path_asc' } + + it { is_expected.to eq([group_1, group_2, group_3, group_4].sort_by(&:path)) } + end + + context 'when sort by path_desc' do + let(:sort) { 'path_desc' } + + it { is_expected.to eq([group_1, group_2, group_3, group_4].sort_by(&:path).reverse) } + end + context 'when sort by recently_created' do let(:sort) { 'created_desc' } @@ -1257,40 +1277,8 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end - describe '#avatar_type' do - let(:user) { create(:user) } - - before do - group.add_member(user, GroupMember::MAINTAINER) - end - - it "is true if avatar is image" do - group.update_attribute(:avatar, 'uploads/avatar.png') - expect(group.avatar_type).to be_truthy - end - - it "is false if avatar is html page" do - group.update_attribute(:avatar, 'uploads/avatar.html') - group.avatar_type - - expect(group.errors.added?(:avatar, "file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp")).to be true - end - end - - describe '#avatar_url' do - let!(:group) { create(:group, :with_avatar) } - let(:user) { create(:user) } - - context 'when avatar file is uploaded' do - before do - group.add_maintainer(user) - end - - it 'shows correct avatar url' do - expect(group.avatar_url).to eq(group.avatar.url) - expect(group.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, group.avatar.url].join) - end - end + it_behaves_like Avatarable do + let(:model) { create(:group, :with_avatar) } end describe '.search' do @@ -3014,14 +3002,6 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end - describe '#activity_path' do - it 'returns the group activity_path' do - expected_path = "/groups/#{group.name}/-/activity" - - expect(group.activity_path).to eq(expected_path) - end - end - context 'with export' do let(:group) { create(:group, :with_export) } diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index da4771d801d..d48b411c800 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -127,7 +127,7 @@ RSpec.describe SystemHook, feature_category: :webhooks do end it 'group destroy hook' do - group.destroy! + create(:group).destroy! expect(WebMock).to have_requested(:post, system_hook.url).with( body: /group_destroy/, diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 5af6a592c66..3a96de4efe2 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -81,10 +81,9 @@ RSpec.describe Integration, feature_category: :integrations do let!(:integration1) { create(:jira_integration, project: project) } let!(:integration2) { create(:redmine_integration, project: project) } let!(:integration3) { create(:confluence_integration, project: project) } - let!(:integration4) { create(:shimo_integration, project: project) } it 'returns the right group integration' do - expect(described_class.third_party_wikis).to contain_exactly(integration3, integration4) + expect(described_class.third_party_wikis).to contain_exactly(integration3) end end diff --git a/spec/models/integrations/base_third_party_wiki_spec.rb b/spec/models/integrations/base_third_party_wiki_spec.rb deleted file mode 100644 index 763f7131b94..00000000000 --- a/spec/models/integrations/base_third_party_wiki_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Integrations::BaseThirdPartyWiki, feature_category: :integrations do - describe 'default values' do - it { expect(subject.category).to eq(:third_party_wiki) } - end - - describe 'Validations' do - let_it_be_with_reload(:project) { create(:project) } - - describe 'only one third party wiki per project' do - subject(:integration) { build(: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) { build(: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/field_spec.rb b/spec/models/integrations/field_spec.rb index 49eaecd1b2e..22ad71135e7 100644 --- a/spec/models/integrations/field_spec.rb +++ b/spec/models/integrations/field_spec.rb @@ -186,6 +186,22 @@ RSpec.describe ::Integrations::Field, feature_category: :integrations do end end + describe '#api_type' do + it 'returns String' do + expect(field.api_type).to eq(String) + end + + context 'when type is checkbox' do + before do + attrs[:type] = :checkbox + end + + it 'returns Boolean' do + expect(field.api_type).to eq(::API::Integrations::Boolean) + end + end + end + describe '#key?' do it { is_expected.to be_key(:type) } it { is_expected.not_to be_key(:foo) } diff --git a/spec/models/integrations/irker_spec.rb b/spec/models/integrations/irker_spec.rb index e98b8b54e03..2a733e67a3c 100644 --- a/spec/models/integrations/irker_spec.rb +++ b/spec/models/integrations/irker_spec.rb @@ -5,7 +5,7 @@ require 'socket' require 'timeout' require 'json' -RSpec.describe Integrations::Irker do +RSpec.describe Integrations::Irker, feature_category: :integrations do describe 'Validations' do context 'when integration is active' do before do @@ -30,10 +30,7 @@ RSpec.describe Integrations::Irker do let(:irker) { described_class.new } let(:irker_server) { TCPServer.new('localhost', 0) } - let(:sample_data) do - Gitlab::DataBuilder::Push.build_sample(project, user) - end - + let(:sample_data) { Gitlab::DataBuilder::Push.build_sample(project, user) } let(:recipients) { '#commits irc://test.net/#test ftp://bad' } let(:colorize_messages) { '1' } @@ -58,7 +55,7 @@ RSpec.describe Integrations::Irker do it 'sends valid JSON messages to an Irker listener', :sidekiq_might_not_need_inline do expect(Integrations::IrkerWorker).to receive(:perform_async) - .with(project.id, irker.channels, colorize_messages, sample_data, irker.settings) + .with(project.id, irker.channels, colorize_messages, sample_data.deep_stringify_keys, irker.settings) .and_call_original irker.execute(sample_data) diff --git a/spec/models/integrations/shimo_spec.rb b/spec/models/integrations/shimo_spec.rb deleted file mode 100644 index 95289343d0d..00000000000 --- a/spec/models/integrations/shimo_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ::Integrations::Shimo, feature_category: :integrations do - describe '#fields' do - let(:shimo_integration) { build(:shimo_integration) } - - it 'returns custom fields' do - expect(shimo_integration.fields.pluck(:name)).to eq(%w[external_wiki_url]) - end - end - - describe '#create' do - let_it_be(:project) { create(:project, :repository) } - let(:external_wiki_url) { 'https://shimo.example.com/desktop' } - let(:params) { { active: true, project: project, external_wiki_url: external_wiki_url } } - - context 'with valid params' do - it 'creates the Shimo integration' do - shimo = described_class.create!(params) - - expect(shimo.valid?).to be true - expect(shimo.render?).to be true - expect(shimo.external_wiki_url).to eq(external_wiki_url) - end - end - - context 'with invalid params' do - it 'cannot create the Shimo integration without external_wiki_url' do - params['external_wiki_url'] = nil - expect { described_class.create!(params) }.to raise_error(ActiveRecord::RecordInvalid) - end - - it 'cannot create the Shimo integration with invalid external_wiki_url' do - params['external_wiki_url'] = 'Fake Invalid URL' - expect { described_class.create!(params) }.to raise_error(ActiveRecord::RecordInvalid) - end - end - end - - describe 'Caching has_shimo on project_settings' do - let_it_be(:project) { create(:project) } - - subject { project.project_setting.has_shimo? } - - it 'sets the property to true when integration is active' do - create(:shimo_integration, project: project, active: true) - - is_expected.to be(true) - end - - it 'sets the property to false when integration is not active' do - create(:shimo_integration, project: project, active: false) - - is_expected.to be(false) - end - - it 'creates a project_setting record if one was not already created' do - expect { create(:shimo_integration) }.to change(ProjectSetting, :count).by(1) - end - end - - describe '#avatar_url' do - it 'returns the avatar image path' do - expect(subject.avatar_url).to eq(ActionController::Base.helpers.image_path('logos/shimo.svg')) - end - end -end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 594492a160d..48e19cd0ad5 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -380,6 +380,16 @@ RSpec.describe Issue, feature_category: :team_planning do end end + describe '.in_namespaces' do + let(:group) { create(:group) } + let!(:group_work_item) { create(:issue, :group_level, namespace: group) } + let!(:project_work_item) { create(:issue, project: reusable_project) } + + subject { described_class.in_namespaces(group) } + + it { is_expected.to contain_exactly(group_work_item) } + end + describe '.with_issue_type' do let_it_be(:issue) { create(:issue, project: reusable_project) } let_it_be(:incident) { create(:incident, project: reusable_project) } @@ -2195,4 +2205,21 @@ RSpec.describe Issue, feature_category: :team_planning do end end end + + describe '#gfm_reference' do + where(:issue_type, :expected_name) do + :issue | 'issue' + :incident | 'incident' + :test_case | 'test case' + :task | 'task' + end + + with_them do + it 'uses the issue type as the reference name' do + issue = create(:issue, issue_type, project: reusable_project) + + expect(issue.gfm_reference).to eq("#{expected_name} #{issue.to_reference}") + end + end + end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 7a46e5e7e53..dd4dbd53a94 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -348,56 +348,10 @@ RSpec.describe Key, :mailer do end end - context 'validate it meets key restrictions' do - where(:factory, :minimum, :result) do - forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE + context 'ssh key' do + subject { build(:key) } - [ - [:rsa_key_2048, 0, true], - [:dsa_key_2048, 0, true], - [:ecdsa_key_256, 0, true], - [:ed25519_key_256, 0, true], - [:ecdsa_sk_key_256, 0, true], - [:ed25519_sk_key_256, 0, true], - - [:rsa_key_2048, 1024, true], - [:rsa_key_2048, 2048, true], - [:rsa_key_2048, 4096, false], - - [:dsa_key_2048, 1024, true], - [:dsa_key_2048, 2048, true], - [:dsa_key_2048, 4096, false], - - [:ecdsa_key_256, 256, true], - [:ecdsa_key_256, 384, false], - - [:ed25519_key_256, 256, true], - [:ed25519_key_256, 384, false], - - [:ecdsa_sk_key_256, 256, true], - [:ecdsa_sk_key_256, 384, false], - - [:ed25519_sk_key_256, 256, true], - [:ed25519_sk_key_256, 384, false], - - [:rsa_key_2048, forbidden, false], - [:dsa_key_2048, forbidden, false], - [:ecdsa_key_256, forbidden, false], - [:ed25519_key_256, forbidden, false], - [:ecdsa_sk_key_256, forbidden, false], - [:ed25519_sk_key_256, forbidden, false] - ] - end - - with_them do - subject(:key) { build(factory) } - - before do - stub_application_setting("#{key.public_key.type}_key_restriction" => minimum) - end - - it { expect(key.valid?).to eq(result) } - end + it_behaves_like 'meets ssh key restrictions' end context 'callbacks' do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index b4941c71d6a..db2ae319bc9 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -1184,8 +1184,8 @@ RSpec.describe Member, feature_category: :groups_and_projects do context 'with loose foreign key on members.user_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:user) } - let!(:model) { create(:group_member, user: parent) } + let_it_be(:parent) { create(:user) } + let_it_be(:model) { create(:group_member, user: parent) } end end end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index a9725a796bf..70f843be0e1 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -57,6 +57,7 @@ RSpec.describe ProjectMember, feature_category: :groups_and_projects do let_it_be(:developer) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } + let_it_be(:admin) { create(:admin) } before do project.add_owner(owner) @@ -74,6 +75,30 @@ RSpec.describe ProjectMember, feature_category: :groups_and_projects do end end + context 'when member can manage owners via admin' do + let(:user) { admin } + + context 'with admin mode', :enable_admin_mode do + it 'returns Gitlab::Access.options_with_owner' do + expect(access_levels).to eq(Gitlab::Access.options_with_owner) + end + end + + context 'without admin mode' do + it 'returns empty hash' do + expect(access_levels).to eq({}) + end + end + end + + context 'when user is not a project member' do + let(:user) { create(:user) } + + it 'return an empty hash' do + expect(access_levels).to eq({}) + end + end + context 'when member cannot manage owners' do let(:user) { maintainer } diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index bcab2029942..2e68cd9e74a 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -172,6 +172,30 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end end + describe '#ensure_project_id' do + let_it_be(:merge_request) { create(:merge_request, :without_diffs) } + + let(:diff) { build(:merge_request_diff, merge_request: merge_request, project_id: project_id) } + + subject { diff.save! } + + context 'when project_id is null' do + let(:project_id) { nil } + + it do + expect { subject }.to change(diff, :project_id).from(nil).to(merge_request.target_project_id) + end + end + + context 'when project_id is already set' do + let(:project_id) { create(:project, :stubbed_repository).id } + + it do + expect { subject }.not_to change(diff, :project_id) + end + end + end + describe '#update_external_diff_store' do let_it_be(:merge_request) { create(:merge_request) } @@ -366,9 +390,10 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do shared_examples_for 'merge request diffs' do let(:merge_request) { create(:merge_request) } - let!(:diff) { merge_request.merge_request_diff.reload } context 'when it was not cleaned by the system' do + let!(:diff) { merge_request.merge_request_diff.reload } + it 'returns persisted diffs' do expect(diff).to receive(:load_diffs).and_call_original @@ -377,6 +402,8 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end context 'when diff was cleaned by the system' do + let!(:diff) { merge_request.merge_request_diff.reload } + before do diff.clean! end @@ -737,6 +764,63 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end end + describe '#get_patch_id_sha' do + let(:mr_diff) { create(:merge_request).merge_request_diff } + + context 'when the patch_id exists on the model' do + it 'returns the patch_id' do + expect(mr_diff.patch_id_sha).not_to be_nil + expect(mr_diff.get_patch_id_sha).to eq(mr_diff.patch_id_sha) + end + end + + context 'when the patch_id does not exist on the model' do + it 'retrieves the patch id, saves the model, and returns it' do + expect(mr_diff.patch_id_sha).not_to be_nil + + patch_id = mr_diff.patch_id_sha + mr_diff.update!(patch_id_sha: nil) + + expect(mr_diff.get_patch_id_sha).to eq(patch_id) + expect(mr_diff.reload.patch_id_sha).to eq(patch_id) + end + + context 'when base_sha is nil' do + before do + mr_diff.update!(patch_id_sha: nil) + allow(mr_diff).to receive(:base_commit_sha).and_return(nil) + end + + it 'returns nil' do + expect(mr_diff.reload.get_patch_id_sha).to be_nil + end + end + + context 'when head_sha is nil' do + before do + mr_diff.update!(patch_id_sha: nil) + allow(mr_diff).to receive(:head_commit_sha).and_return(nil) + end + + it 'returns nil' do + expect(mr_diff.reload.get_patch_id_sha).to be_nil + end + end + + context 'when base_sha and head_sha dont match' do + before do + mr_diff.update!(patch_id_sha: nil) + allow(mr_diff).to receive(:head_commit_sha).and_return('123123') + allow(mr_diff).to receive(:base_commit_sha).and_return('43121') + end + + it 'returns nil' do + expect(mr_diff.reload.get_patch_id_sha).to be_nil + end + end + end + end + describe '#save_diffs' do it 'saves collected state' do mr_diff = create(:merge_request).merge_request_diff @@ -825,6 +909,57 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do expect(diff_file.diff).to include(content) end end + + context 'handling generated files' do + let(:project) { create(:project, :repository) } + let(:target_branch) { project.default_branch } + let(:source_branch) { 'test-generated-diff-file' } + let(:generated_file_name) { 'generated.txt' } + let(:regular_file_name) { 'regular.rb' } + let(:merge_request) do + create( + :merge_request, + target_project: project, + source_project: project, + source_branch: source_branch, + target_branch: target_branch + ) + end + + let(:diff_files) do + merge_request.merge_request_diff.merge_request_diff_files + end + + before do + project.repository.update_file( + project.creator, + '.gitattributes', + '*.txt gitlab-generated', + message: 'Update', + branch_name: target_branch) + + create_file_in_repo(project, target_branch, source_branch, generated_file_name, "generated text\n") + create_file_in_repo(project, source_branch, source_branch, regular_file_name, "something else\n") + end + + context 'with collapse_generated_diff_files feature flag' do + it 'sets generated field correctly' do + expect(diff_files.find_by(new_path: generated_file_name)).to be_generated + expect(diff_files.find_by(new_path: regular_file_name)).not_to be_generated + end + end + + context 'without collapse_generated_diff_files feature flag' do + before do + stub_feature_flags(collapse_generated_diff_files: false) + end + + it 'sets generated field correctly' do + expect(diff_files.find_by(new_path: generated_file_name)).not_to be_generated + expect(diff_files.find_by(new_path: regular_file_name)).not_to be_generated + end + end + end end end @@ -905,6 +1040,7 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do include_examples 'merge request diffs' it 'stores up-to-date diffs in the database' do + diff = merge_request.merge_request_diff.reload expect(diff).not_to be_stored_externally end @@ -921,7 +1057,8 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end it 'stores diffs for old MR versions in external storage' do - old_diff = diff + old_diff = merge_request.merge_request_diff.reload + merge_request.create_merge_request_diff old_diff.migrate_files_to_external_storage! diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 1c6a29f065f..2b5f4165d8c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -404,7 +404,7 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end - context "with the skip_validations_during_transition_feature_flag" do + context "when transitioning between states" do let(:merge_request) { build(:merge_request, transitioning: transitioning) } where(:transitioning, :to_or_not_to) do @@ -6026,47 +6026,11 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev subject(:current_patch_id_sha) { merge_request.current_patch_id_sha } before do - allow(merge_request).to receive(:merge_request_diff).and_return(merge_request_diff) + allow(merge_request).to receive(:latest_merge_request_diff).and_return(merge_request_diff) allow(merge_request_diff).to receive(:patch_id_sha).and_return(patch_id) end it { is_expected.to eq(patch_id) } - - context 'when related merge_request_diff does not have a patch_id_sha' do - let(:diff_refs) { instance_double(Gitlab::Diff::DiffRefs, base_sha: base_sha, head_sha: head_sha) } - let(:base_sha) { 'abc123' } - let(:head_sha) { 'def456' } - - before do - allow(merge_request_diff).to receive(:patch_id_sha).and_return(nil) - allow(merge_request).to receive(:diff_refs).and_return(diff_refs) - - allow(merge_request.project.repository) - .to receive(:get_patch_id) - .with(diff_refs.base_sha, diff_refs.head_sha) - .and_return(patch_id) - end - - it { is_expected.to eq(patch_id) } - - context 'when base_sha is nil' do - let(:base_sha) { nil } - - it { is_expected.to be_nil } - end - - context 'when head_sha is nil' do - let(:head_sha) { nil } - - it { is_expected.to be_nil } - end - - context 'when base_sha and head_sha match' do - let(:head_sha) { base_sha } - - it { is_expected.to be_nil } - end - end end describe '#all_mergeability_checks_results' do @@ -6138,4 +6102,29 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev it { is_expected.to eq(false) } end end + + describe '#allow_merge_without_pipeline?' do + let(:merge_request) { build_stubbed(:merge_request) } + + subject(:result) { merge_request.allow_merge_without_pipeline? } + + before do + allow(merge_request.project) + .to receive(:allow_merge_without_pipeline?) + .with(inherit_group_setting: true) + .and_return(allow_merge_without_pipeline?) + end + + context 'when associated project allow_merge_without_pipeline? returns true' do + let(:allow_merge_without_pipeline?) { true } + + it { is_expected.to eq(true) } + end + + context 'when associated project allow_merge_without_pipeline? returns false' do + let(:allow_merge_without_pipeline?) { false } + + it { is_expected.to eq(false) } + end + end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 15bcbb3962c..79b663807b4 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -772,4 +772,24 @@ RSpec.describe Milestone, feature_category: :team_planning do it { is_expected.to eq(false) } end end + + describe '.with_ids_or_title' do + subject(:milestones) { described_class.with_ids_or_title(ids: ids, title: title) } + + let_it_be(:milestone1) { create(:milestone, title: 'Foo') } + let_it_be(:milestone2) { create(:milestone) } + + let(:ids) { [milestone1.id] } + let(:title) { milestone2.title } + + before do + # Milestones below should not be returned + create(:milestone, title: 'Bar') + create(:milestone, id: 10) + end + + it 'returns milestones with matching id or title' do + expect(milestones).to contain_exactly(milestone1, milestone2) + end + end end diff --git a/spec/models/ml/candidate_spec.rb b/spec/models/ml/candidate_spec.rb index d5b71e2c3f7..7b3dee2da7b 100644 --- a/spec/models/ml/candidate_spec.rb +++ b/spec/models/ml/candidate_spec.rb @@ -5,7 +5,12 @@ require 'spec_helper' RSpec.describe Ml::Candidate, factory_default: :keep, feature_category: :mlops do let_it_be(:candidate) { create(:ml_candidates, :with_metrics_and_params, :with_artifact, name: 'candidate0') } let_it_be(:candidate2) do - create(:ml_candidates, experiment: candidate.experiment, user: create(:user), name: 'candidate2') + create(:ml_candidates, experiment: candidate.experiment, name: 'candidate2', project: candidate.project) + end + + let_it_be(:existing_model) { create(:ml_models, project: candidate2.project) } + let_it_be(:existing_model_version) do + create(:ml_model_versions, model: existing_model, candidate: candidate2) end let(:project) { candidate.project } @@ -38,8 +43,8 @@ RSpec.describe Ml::Candidate, factory_default: :keep, feature_category: :mlops d describe 'validation' do let_it_be(:model) { create(:ml_models, project: candidate.project) } - let_it_be(:model_version1) { create(:ml_model_versions, model: model) } - let_it_be(:model_version2) { create(:ml_model_versions, model: model) } + let_it_be(:model_version1) { create(:ml_model_versions, model: model, candidate: nil) } + let_it_be(:model_version2) { create(:ml_model_versions, model: model, candidate: nil) } let_it_be(:validation_candidate) do create(:ml_candidates, model_version: model_version1, project: candidate.project) end @@ -231,6 +236,14 @@ RSpec.describe Ml::Candidate, factory_default: :keep, feature_category: :mlops d end end + describe '#without_model_version' do + subject { described_class.without_model_version } + + it 'finds only candidates without model version' do + expect(subject).to match_array([candidate]) + end + end + describe 'from_ci?' do subject { candidate } @@ -272,8 +285,8 @@ RSpec.describe Ml::Candidate, factory_default: :keep, feature_category: :mlops d context 'with loose foreign key on ml_candidates.ci_build_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:ci_build) } - let!(:model) { create(:ml_candidates, ci_build: parent) } + let_it_be(:parent) { create(:ci_build) } + let_it_be(:model) { create(:ml_candidates, ci_build: parent) } end end end diff --git a/spec/models/ml/model_spec.rb b/spec/models/ml/model_spec.rb index ae7c3f163f3..e1de44b0030 100644 --- a/spec/models/ml/model_spec.rb +++ b/spec/models/ml/model_spec.rb @@ -63,6 +63,19 @@ RSpec.describe Ml::Model, feature_category: :mlops do end end + describe 'candidates' do + let_it_be(:candidate1) { create(:ml_model_versions, model: existing_model).candidate } + let_it_be(:candidate2) do + create(:ml_candidates, experiment: existing_model.default_experiment, project: project1) + end + + let_it_be(:candidate3) { create(:ml_candidates, project: project1) } + + it 'returns only the candidates for default experiment that do not belong to a model version' do + expect(existing_model.candidates).to match_array([candidate2]) + end + end + describe '.by_project' do subject { described_class.by_project(project1) } @@ -128,4 +141,27 @@ RSpec.describe Ml::Model, feature_category: :mlops do it { is_expected.to be(nil) } end end + + describe '#all_packages' do + it 'returns an empty array when no model versions exist' do + expect(existing_model.all_packages).to eq([]) + end + + it 'returns one package when a single model version exists' do + version = create(:ml_model_versions, :with_package, model: existing_model) + + all_packages = existing_model.all_packages + expect(all_packages.length).to be(1) + expect(all_packages.first).to eq(version.package) + end + + it 'returns multiple packages when multiple model versions exist' do + version1 = create(:ml_model_versions, :with_package, model: existing_model) + version2 = create(:ml_model_versions, :with_package, model: existing_model) + + all_packages = existing_model.all_packages + expect(all_packages.length).to be(2) + expect(all_packages).to match_array([version1.package, version2.package]) + end + end end diff --git a/spec/models/namespace/package_setting_spec.rb b/spec/models/namespace/package_setting_spec.rb index e6096bc9267..f06490d7999 100644 --- a/spec/models/namespace/package_setting_spec.rb +++ b/spec/models/namespace/package_setting_spec.rb @@ -16,6 +16,9 @@ RSpec.describe Namespace::PackageSetting, feature_category: :package_registry do it { is_expected.to validate_inclusion_of(:nuget_duplicates_allowed).in_array([true, false]) } end + it { is_expected.to allow_value(true, false).for(:nuget_symbol_server_enabled) } + it { is_expected.not_to allow_value(nil).for(:nuget_symbol_server_enabled) } + describe 'regex values' do let_it_be(:package_settings) { create(:namespace_package_setting) } diff --git a/spec/models/namespace/root_storage_statistics_spec.rb b/spec/models/namespace/root_storage_statistics_spec.rb index 4b66b7532a7..9a66b4745c0 100644 --- a/spec/models/namespace/root_storage_statistics_spec.rb +++ b/spec/models/namespace/root_storage_statistics_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Namespace::RootStorageStatistics, type: :model, feature_category: describe '.for_namespace_ids' do it 'returns only requested namespaces' do stats = create_list(:namespace_root_storage_statistics, 3) - namespace_ids = stats[0..1].map { |s| s.namespace_id } + namespace_ids = stats[0..1].map(&:namespace_id) requested_stats = described_class.for_namespace_ids(namespace_ids).pluck(:namespace_id) diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 85569a68252..0e6513764f5 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -34,6 +34,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do it { is_expected.to have_many :namespace_members } it { is_expected.to have_one :cluster_enabled_grant } it { is_expected.to have_many(:work_items) } + it { is_expected.to have_many(:work_items_dates_source) } it { is_expected.to have_many :achievements } it { is_expected.to have_many(:namespace_commit_emails).class_name('Users::NamespaceCommitEmail') } it { is_expected.to have_many(:cycle_analytics_stages) } @@ -82,7 +83,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do describe 'validations' do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_length_of(:name).is_at_most(255) } - it { is_expected.to validate_length_of(:description).is_at_most(255) } + it { is_expected.to validate_length_of(:description).is_at_most(500) } it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_length_of(:path).is_at_most(255) } it { is_expected.to validate_presence_of(:owner) } @@ -2387,8 +2388,8 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do context 'with loose foreign key on organization_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:organization) } - let!(:model) { create(:namespace, organization: parent) } + let_it_be(:parent) { create(:organization) } + let_it_be(:model) { create(:namespace, organization: parent) } end end end diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index f19c0a68f87..65bf7aec269 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -440,8 +440,8 @@ RSpec.describe NotificationRecipient, feature_category: :team_planning do described_class.new(user, :participating, custom_action: :issue_due, target: target, project: project) end - it 'returns true' do - expect(recipient.suitable_notification_level?).to eq true + it 'returns false' do + expect(recipient.suitable_notification_level?).to eq false end end end diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 9bf95051730..cb1bbb91a67 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -193,7 +193,11 @@ RSpec.describe NotificationSetting do end it 'includes EXCLUDED_WATCHER_EVENTS' do - expect(subject).to include(*described_class::EXCLUDED_WATCHER_EVENTS) + expect(subject).to include( + :push_to_merge_request, + :issue_due, + :success_pipeline + ) end end @@ -224,8 +228,8 @@ RSpec.describe NotificationSetting do context 'with loose foreign key on notification_settings.user_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:user) } - let!(:model) { create(:notification_setting, user: parent) } + let_it_be(:parent) { create(:user) } + let_it_be(:model) { create(:notification_setting, user: parent) } end end end diff --git a/spec/models/onboarding/progress_spec.rb b/spec/models/onboarding/progress_spec.rb index c45d8c97385..9c5a55d3313 100644 --- a/spec/models/onboarding/progress_spec.rb +++ b/spec/models/onboarding/progress_spec.rb @@ -292,28 +292,4 @@ RSpec.describe Onboarding::Progress do it { is_expected.to eq(:subscription_created_at) } end - - describe '#number_of_completed_actions' do - subject do - build(:onboarding_progress, actions.map { |x| { x => Time.current } }.inject(:merge)).number_of_completed_actions - end - - context 'with 0 completed actions' do - let(:actions) { [:created_at, :updated_at] } - - it { is_expected.to eq(0) } - end - - context 'with 1 completed action' do - let(:actions) { [:created_at, :subscription_created_at] } - - it { is_expected.to eq(1) } - end - - context 'with 2 completed actions' do - let(:actions) { [:subscription_created_at, :git_write_at] } - - it { is_expected.to eq(2) } - end - end end diff --git a/spec/models/organizations/organization_detail_spec.rb b/spec/models/organizations/organization_detail_spec.rb new file mode 100644 index 00000000000..3f44a9cc637 --- /dev/null +++ b/spec/models/organizations/organization_detail_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Organizations::OrganizationDetail, type: :model, feature_category: :cell do + describe 'associations' do + it { is_expected.to belong_to(:organization).inverse_of(:organization_detail) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:organization) } + it { is_expected.to validate_length_of(:description).is_at_most(1024) } + end + + it_behaves_like Avatarable do + let(:model) { create(:organization_detail) } + end + + context 'with uploads' do + it_behaves_like 'model with uploads', false do + let(:model_object) { create(:organization_detail) } + let(:upload_attribute) { :avatar } + let(:uploader_class) { AttachmentUploader } + end + end +end diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index 0670002135c..756024b6437 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -7,6 +7,8 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel let_it_be(:default_organization) { create(:organization, :default) } describe 'associations' do + it { is_expected.to have_one(:organization_detail).inverse_of(:organization).autosave(true) } + it { is_expected.to have_many :namespaces } it { is_expected.to have_many :groups } it { is_expected.to have_many(:users).through(:organization_users).inverse_of(:organizations) } @@ -21,6 +23,7 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_presence_of(:path) } it { is_expected.to validate_length_of(:path).is_at_least(2).is_at_most(255) } + it { is_expected.to validate_uniqueness_of(:path).case_insensitive } describe 'path validator' do using RSpec::Parameterized::TableSyntax @@ -54,6 +57,16 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel end end + describe 'delegations' do + it { is_expected.to delegate_method(:description).to(:organization_detail) } + it { is_expected.to delegate_method(:avatar).to(:organization_detail) } + it { is_expected.to delegate_method(:avatar_url).to(:organization_detail) } + end + + describe 'nested attributes' do + it { is_expected.to accept_nested_attributes_for(:organization_detail) } + end + context 'when using scopes' do describe '.without_default' do it 'excludes default organization' do @@ -120,6 +133,13 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel end end + describe '#organization_detail' do + it 'ensures organization has organization_detail upon initialization' do + expect(organization.organization_detail).to be_present + expect(organization.organization_detail).not_to be_persisted + end + end + describe '#default?' do context 'when organization is default' do it 'returns true' do diff --git a/spec/models/packages/nuget/symbol_spec.rb b/spec/models/packages/nuget/symbol_spec.rb index f43f3a3bdeb..bae8f90c7d5 100644 --- a/spec/models/packages/nuget/symbol_spec.rb +++ b/spec/models/packages/nuget/symbol_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Packages::Nuget::Symbol, type: :model, feature_category: :package it { is_expected.to be_a FileStoreMounter } it { is_expected.to be_a ShaAttribute } + it { is_expected.to be_a Packages::Destructible } describe 'relationships' do it { is_expected.to belong_to(:package).inverse_of(:nuget_symbols) } @@ -26,6 +27,63 @@ RSpec.describe Packages::Nuget::Symbol, type: :model, feature_category: :package it { is_expected.to delegate_method(:project_id).to(:package) } end + describe 'scopes' do + describe '.stale' do + subject { described_class.stale } + + let_it_be(:symbol) { create(:nuget_symbol) } + let_it_be(:stale_symbol) { create(:nuget_symbol, :stale) } + + it { is_expected.to contain_exactly(stale_symbol) } + end + + describe '.pending_destruction' do + subject { described_class.pending_destruction } + + let_it_be(:symbol) { create(:nuget_symbol, :stale, :processing) } + let_it_be(:stale_symbol) { create(:nuget_symbol, :stale) } + + it { is_expected.to contain_exactly(stale_symbol) } + end + + describe '.with_signature' do + subject(:with_signature) { described_class.with_signature(signature) } + + let_it_be(:signature) { 'signature' } + let_it_be(:symbol) { create(:nuget_symbol, signature: signature) } + + it 'returns symbols with the given signature' do + expect(with_signature).to eq([symbol]) + end + end + + describe '.with_file_name' do + subject(:with_file_name) { described_class.with_file_name(file_name) } + + let_it_be(:file_name) { 'file_name' } + let_it_be(:symbol) { create(:nuget_symbol) } + + before do + symbol.update_column(:file, file_name) + end + + it 'returns symbols with the given file_name' do + expect(with_file_name).to eq([symbol]) + end + end + + describe '.with_file_sha256' do + subject(:with_file_sha256) { described_class.with_file_sha256(checksums) } + + let_it_be(:checksums) { OpenSSL::Digest.hexdigest('SHA256', 'checksums') } + let_it_be(:symbol) { create(:nuget_symbol, file_sha256: checksums) } + + it 'returns symbols with the given checksums' do + expect(with_file_sha256).to eq([symbol]) + end + end + end + describe 'callbacks' do describe 'before_validation' do describe '#set_object_storage_key' do diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 8e3b97e55f3..0ed6f058768 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1355,6 +1355,30 @@ RSpec.describe Packages::Package, type: :model, feature_category: :package_regis end end + describe '#sync_npm_metadata_cache' do + let_it_be(:package) { create(:npm_package) } + + subject { package.sync_npm_metadata_cache } + + it 'enqueues a sync worker job' do + expect(::Packages::Npm::CreateMetadataCacheWorker) + .to receive(:perform_async).with(package.project_id, package.name) + + subject + end + + context 'with a non npm package' do + let_it_be(:package) { create(:maven_package) } + + it 'does not enqueue a sync worker job' do + expect(::Packages::Npm::CreateMetadataCacheWorker) + .not_to receive(:perform_async) + + subject + end + end + end + describe '#mark_package_files_for_destruction' do let_it_be(:package) { create(:npm_package, :pending_destruction) } diff --git a/spec/models/packages/tag_spec.rb b/spec/models/packages/tag_spec.rb index 6842d1946e5..2d045615756 100644 --- a/spec/models/packages/tag_spec.rb +++ b/spec/models/packages/tag_spec.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true + require 'spec_helper' RSpec.describe Packages::Tag, type: :model, feature_category: :package_registry do - let!(:project) { create(:project) } - let!(:package) { create(:npm_package, version: '1.0.2', project: project, updated_at: 3.days.ago) } + let_it_be(:project) { create(:project) } + let_it_be(:package) { create(:npm_package, version: '1.0.2', project: project, updated_at: 3.days.ago) } describe '#ensure_project_id' do it 'sets the project_id before saving' do @@ -83,4 +84,25 @@ RSpec.describe Packages::Tag, type: :model, feature_category: :package_registry it { is_expected.to contain_exactly(tag1, tag3) } end end + + describe '.for_package_ids_with_distinct_names' do + let_it_be(:package2) { create(:package, project: project) } + let_it_be(:package3) { create(:package, project: project) } + let_it_be(:tag1) { create(:packages_tag, name: 'latest', package: package, updated_at: 4.days.ago) } + let_it_be(:tag2) { create(:packages_tag, name: 'latest', package: package2, updated_at: 3.days.ago) } + let_it_be(:tag3) { create(:packages_tag, name: 'latest', package: package2, updated_at: 2.days.ago) } + let_it_be(:tag4) { create(:packages_tag, name: 'tag4', package: package3, updated_at: 5.days.ago) } + let_it_be(:tag5) { create(:packages_tag, name: 'tag5', package: package3, updated_at: 4.days.ago) } + let_it_be(:tag6) { create(:packages_tag, name: 'tag6', package: package3, updated_at: 6.days.ago) } + + subject { described_class.for_package_ids_with_distinct_names(project.packages) } + + before do + stub_const("#{described_class}::FOR_PACKAGES_TAGS_LIMIT", 3) + end + + # `tag3` is returned because it's the most recently updated with the name `latest`. + # `tag5` is returned before `tag4` because it was updated more recently than `tag4`. + it { is_expected.to eq([tag3, tag5, tag4]) } + end end diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 570c369016b..ccca6e7a48a 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -3,19 +3,38 @@ require 'spec_helper' RSpec.describe Pages::LookupPath, feature_category: :pages do - let(:project) { create(:project, :pages_private, pages_https_only: true) } let(:trim_prefix) { nil } - let(:domain) { nil } + let(:path_prefix) { nil } + let(:file_store) { ::ObjectStorage::Store::REMOTE } + let(:group) { build(:group, path: 'mygroup') } + let(:deployment) do + build( + :pages_deployment, + id: 1, + project: project, + path_prefix: path_prefix, + file_store: file_store) + end + + let(:project) do + build( + :project, + :pages_private, + group: group, + path: 'myproject', + pages_https_only: true) + end - subject(:lookup_path) { described_class.new(project, trim_prefix: trim_prefix, domain: domain) } + subject(:lookup_path) { described_class.new(deployment: deployment, trim_prefix: trim_prefix) } before do stub_pages_setting( + enabled: true, access_control: true, external_https: ["1.1.1.1:443"], url: 'http://example.com', - protocol: 'http' - ) + protocol: 'http') + stub_pages_object_storage(::Pages::DeploymentUploader) end @@ -32,7 +51,11 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end describe '#https_only' do + subject(:lookup_path) { described_class.new(deployment: deployment, domain: domain) } + context 'when no domain provided' do + let(:domain) { nil } + it 'delegates to Project#pages_https_only?' do expect(lookup_path.https_only).to eq(true) end @@ -48,66 +71,55 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end describe '#source' do - let(:source) { lookup_path.source } - - it 'returns nil' do - expect(source).to eq(nil) + it 'uses deployment from object storage', :freeze_time do + expect(lookup_path.source).to eq( + type: 'zip', + path: deployment.file.url(expire_at: 1.day.from_now), + global_id: "gid://gitlab/PagesDeployment/#{deployment.id}", + sha256: deployment.file_sha256, + file_size: deployment.size, + file_count: deployment.file_count + ) end - context 'when there is pages deployment' do - let!(:deployment) { create(:pages_deployment, project: project) } - - it 'uses deployment from object storage' do - freeze_time do - expect(source).to eq( - type: 'zip', - path: deployment.file.url(expire_at: 1.day.from_now), - global_id: "gid://gitlab/PagesDeployment/#{deployment.id}", - sha256: deployment.file_sha256, - file_size: deployment.size, - file_count: deployment.file_count - ) - end - end + it 'does not recreate source hash' do + expect(deployment.file).to receive(:url_or_file_path).once + + 2.times { lookup_path.source } + end - context 'when deployment is in the local storage' do - before do - deployment.file.migrate!(::ObjectStorage::Store::LOCAL) - end - - it 'uses file protocol' do - freeze_time do - expect(source).to eq( - type: 'zip', - path: "file://#{deployment.file.path}", - global_id: "gid://gitlab/PagesDeployment/#{deployment.id}", - sha256: deployment.file_sha256, - file_size: deployment.size, - file_count: deployment.file_count - ) - end - end + context 'when deployment is in the local storage' do + let(:file_store) { ::ObjectStorage::Store::LOCAL } + + it 'uses file protocol', :freeze_time do + expect(lookup_path.source).to eq( + type: 'zip', + path: "file://#{deployment.file.path}", + global_id: "gid://gitlab/PagesDeployment/#{deployment.id}", + sha256: deployment.file_sha256, + file_size: deployment.size, + file_count: deployment.file_count + ) end end end describe '#prefix' do - let(:trim_prefix) { 'mygroup' } - - context 'when pages group root projects' do - let(:project) { instance_double(Project, full_path: "namespace/namespace.example.com") } + using RSpec::Parameterized::TableSyntax - it 'returns "/"' do - expect(lookup_path.prefix).to eq('/') - end + where(:full_path, :trim_prefix, :path_prefix, :result) do + 'mygroup/myproject' | nil | nil | '/' + 'mygroup/myproject' | 'mygroup' | nil | '/myproject/' + 'mygroup/myproject' | nil | 'PREFIX' | '/PREFIX/' + 'mygroup/myproject' | 'mygroup' | 'PREFIX' | '/myproject/PREFIX/' end - context 'when pages in the given prefix' do - let(:project) { instance_double(Project, full_path: 'mygroup/myproject') } - - it 'returns the project full path with the provided prefix removed' do - expect(lookup_path.prefix).to eq('/myproject/') + with_them do + before do + allow(project).to receive(:full_path).and_return(full_path) end + + it { expect(lookup_path.prefix).to eq(result) } end end @@ -122,6 +134,16 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end end + context 'when namespace_in_path is enabled' do + before do + stub_pages_setting(namespace_in_path: true) + end + + it 'returns nil' do + expect(lookup_path.unique_host).to be_nil + end + end + context 'when unique domain is enabled' do it 'returns the project unique domain' do project.project_setting.pages_unique_domain_enabled = true @@ -129,26 +151,12 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do expect(lookup_path.unique_host).to eq('unique-domain.example.com') end - - context 'when there is domain provided' do - let(:domain) { instance_double(PagesDomain) } - - it 'returns nil' do - expect(lookup_path.unique_host).to eq(nil) - end - end end end describe '#root_directory' do - context 'when there is no deployment' do - it 'returns nil' do - expect(lookup_path.root_directory).to be_nil - end - end - context 'when there is a deployment' do - let!(:deployment) { create(:pages_deployment, project: project, root_directory: 'foo') } + let(:deployment) { build_stubbed(:pages_deployment, project: project, root_directory: 'foo') } it 'returns the deployment\'s root_directory' do expect(lookup_path.root_directory).to eq('foo') diff --git a/spec/models/pages/virtual_domain_spec.rb b/spec/models/pages/virtual_domain_spec.rb index 02e3fd67f2d..5925b662ee8 100644 --- a/spec/models/pages/virtual_domain_spec.rb +++ b/spec/models/pages/virtual_domain_spec.rb @@ -3,9 +3,30 @@ require 'spec_helper' RSpec.describe Pages::VirtualDomain, feature_category: :pages do + let(:domain) { nil } + let(:trim_prefix) { nil } + + let_it_be(:group) { create(:group, path: 'mygroup') } + let_it_be(:project_a) { create(:project, group: group) } + let_it_be(:project_a_main_deployment) { create(:pages_deployment, project: project_a, path_prefix: nil) } + let_it_be(:project_a_versioned_deployment) { create(:pages_deployment, project: project_a, path_prefix: 'v1') } + let_it_be(:project_b) { create(:project, group: group) } + let_it_be(:project_b_main_deployment) { create(:pages_deployment, project: project_b, path_prefix: nil) } + let_it_be(:project_b_versioned_deployment) { create(:pages_deployment, project: project_b, path_prefix: 'v1') } + let_it_be(:project_c) { create(:project, group: group) } + let_it_be(:project_c_main_deployment) { create(:pages_deployment, project: project_c, path_prefix: nil) } + let_it_be(:project_c_versioned_deployment) { create(:pages_deployment, project: project_c, path_prefix: 'v1') } + + before_all do + # Those deployments are created to ensure that deactivated deployments won't be returned on the queries + deleted_at = 1.hour.ago + create(:pages_deployment, project: project_a, path_prefix: 'v2', deleted_at: deleted_at) + create(:pages_deployment, project: project_b, path_prefix: 'v2', deleted_at: deleted_at) + create(:pages_deployment, project: project_c, path_prefix: 'v2', deleted_at: deleted_at) + end + describe '#certificate and #key pair' do - let(:domain) { nil } - let(:project) { instance_double(Project) } + let(:project) { project_a } subject(:virtual_domain) { described_class.new(projects: [project], domain: domain) } @@ -25,51 +46,52 @@ RSpec.describe Pages::VirtualDomain, feature_category: :pages do end describe '#lookup_paths' do - let(:domain) { nil } - let(:trim_prefix) { nil } - let(:project_a) { instance_double(Project) } - let(:project_b) { instance_double(Project) } - let(:project_c) { instance_double(Project) } - let(:pages_lookup_path_a) { instance_double(Pages::LookupPath, prefix: 'aaa', source: { type: 'zip', path: 'https://example.com' }) } - let(:pages_lookup_path_b) { instance_double(Pages::LookupPath, prefix: 'bbb', source: { type: 'zip', path: 'https://example.com' }) } - let(:pages_lookup_path_without_source) { instance_double(Pages::LookupPath, prefix: 'ccc', source: nil) } + let(:project_list) { [project_a, project_b, project_c] } subject(:virtual_domain) do described_class.new(projects: project_list, domain: domain, trim_prefix: trim_prefix) end - before do - allow(Pages::LookupPath) - .to receive(:new) - .with(project_a, domain: domain, trim_prefix: trim_prefix) - .and_return(pages_lookup_path_a) - - allow(Pages::LookupPath) - .to receive(:new) - .with(project_b, domain: domain, trim_prefix: trim_prefix) - .and_return(pages_lookup_path_b) - - allow(Pages::LookupPath) - .to receive(:new) - .with(project_c, domain: domain, trim_prefix: trim_prefix) - .and_return(pages_lookup_path_without_source) - end + context 'when pages multiple versions is disabled' do + before do + allow(::Gitlab::Pages) + .to receive(:multiple_versions_enabled_for?) + .and_return(false) + end - context 'when there is pages domain provided' do - let(:domain) { instance_double(PagesDomain) } - let(:project_list) { [project_a, project_b, project_c] } + it 'returns only the main deployments for each project' do + global_ids = virtual_domain.lookup_paths.map do |lookup_path| + lookup_path.source[:global_id] + end - it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do - expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_b, pages_lookup_path_a]) + expect(global_ids).to match_array([ + project_a_main_deployment.to_gid.to_s, + project_b_main_deployment.to_gid.to_s, + project_c_main_deployment.to_gid.to_s + ]) end end - context 'when there is trim_prefix provided' do - let(:trim_prefix) { 'group/' } - let(:project_list) { [project_a, project_b] } + context 'when pages multiple versions is enabled' do + before do + allow(::Gitlab::Pages) + .to receive(:multiple_versions_enabled_for?) + .and_return(true) + end it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do - expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_b, pages_lookup_path_a]) + global_ids = virtual_domain.lookup_paths.map do |lookup_path| + lookup_path.source[:global_id] + end + + expect(global_ids).to match_array([ + project_a_main_deployment.to_gid.to_s, + project_a_versioned_deployment.to_gid.to_s, + project_b_main_deployment.to_gid.to_s, + project_b_versioned_deployment.to_gid.to_s, + project_c_main_deployment.to_gid.to_s, + project_c_versioned_deployment.to_gid.to_s + ]) end end end diff --git a/spec/models/preloaders/runner_manager_policy_preloader_spec.rb b/spec/models/preloaders/runner_manager_policy_preloader_spec.rb index 1977e2c5787..b1950273380 100644 --- a/spec/models/preloaders/runner_manager_policy_preloader_spec.rb +++ b/spec/models/preloaders/runner_manager_policy_preloader_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Preloaders::RunnerManagerPolicyPreloader, feature_category: :runner_fleet do +RSpec.describe Preloaders::RunnerManagerPolicyPreloader, feature_category: :fleet_visibility do let_it_be(:user) { create(:user) } let_it_be(:runner1) { create(:ci_runner) } let_it_be(:runner2) { create(:ci_runner) } diff --git a/spec/models/product_analytics_event_spec.rb b/spec/models/product_analytics_event_spec.rb deleted file mode 100644 index 801e6dd5e10..00000000000 --- a/spec/models/product_analytics_event_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true -require 'spec_helper' - -RSpec.describe ProductAnalyticsEvent, type: :model do - it { is_expected.to belong_to(:project) } - it { expect(described_class).to respond_to(:order_by_time) } - - describe 'validations' do - it { is_expected.to validate_presence_of(:project_id) } - it { is_expected.to validate_presence_of(:event_id) } - it { is_expected.to validate_presence_of(:v_collector) } - it { is_expected.to validate_presence_of(:v_etl) } - end - - describe '.timerange' do - let_it_be(:event_1) { create(:product_analytics_event, collector_tstamp: Time.zone.now - 1.day) } - let_it_be(:event_2) { create(:product_analytics_event, collector_tstamp: Time.zone.now - 5.days) } - let_it_be(:event_3) { create(:product_analytics_event, collector_tstamp: Time.zone.now - 15.days) } - - it { expect(described_class.timerange(3.days)).to match_array([event_1]) } - it { expect(described_class.timerange(7.days)).to match_array([event_1, event_2]) } - it { expect(described_class.timerange(30.days)).to match_array([event_1, event_2, event_3]) } - end - - describe '.count_by_graph' do - let_it_be(:events) do - [ - create(:product_analytics_event, platform: 'web'), - create(:product_analytics_event, platform: 'web'), - create(:product_analytics_event, platform: 'app'), - create(:product_analytics_event, platform: 'mobile', collector_tstamp: Time.zone.now - 10.days) - ] - end - - it { expect(described_class.count_by_graph('platform', 7.days)).to eq({ 'app' => 1, 'web' => 2 }) } - it { expect(described_class.count_by_graph('platform', 30.days)).to eq({ 'app' => 1, 'mobile' => 1, 'web' => 2 }) } - end - - describe '.count_collector_tstamp_by_day' do - let_it_be(:time_now) { Time.zone.now } - let_it_be(:time_ago) { Time.zone.now - 5.days } - - let_it_be(:events) do - create_list(:product_analytics_event, 3, collector_tstamp: time_now) + - create_list(:product_analytics_event, 2, collector_tstamp: time_ago) - end - - subject { described_class.count_collector_tstamp_by_day(7.days) } - - it { is_expected.to eq({ time_now.beginning_of_day => 3, time_ago.beginning_of_day => 2 }) } - end -end diff --git a/spec/models/project_authorization_spec.rb b/spec/models/project_authorization_spec.rb index a5f29fcbe8b..00376e1a871 100644 --- a/spec/models/project_authorization_spec.rb +++ b/spec/models/project_authorization_spec.rb @@ -34,7 +34,12 @@ RSpec.describe ProjectAuthorization, feature_category: :groups_and_projects do end context 'with duplicate user and project authorization' do - subject { project_auth.dup } + subject do + project_auth.dup.tap do |auth| + auth.project = project_1 + auth.user = user + end + end it { is_expected.to be_invalid } @@ -52,6 +57,8 @@ RSpec.describe ProjectAuthorization, feature_category: :groups_and_projects do context 'with multiple access levels for the same user and project' do subject do project_auth.dup.tap do |auth| + auth.project = project_1 + auth.user = user auth.access_level = Gitlab::Access::MAINTAINER end end @@ -103,6 +110,23 @@ RSpec.describe ProjectAuthorization, feature_category: :groups_and_projects do end end + describe '.owners' do + let_it_be(:project_original_owner_authorization) { project.owner.project_authorizations.first } + let_it_be(:project_authorization_owner) { create(:project_authorization, :owner, project: project) } + + before_all do + create(:project_authorization, :guest, project: project) + create(:project_authorization, :developer, project: project) + end + + it 'returns all records which only have Owners access' do + expect(described_class.owners.map(&:attributes)).to match_array([ + project_original_owner_authorization, + project_authorization_owner + ].map(&:attributes)) + end + end + describe '.for_project' do let_it_be(:project_2) { create(:project, namespace: user.namespace) } let_it_be(:project_3) { create(:project, namespace: user.namespace) } @@ -146,4 +170,11 @@ RSpec.describe ProjectAuthorization, feature_category: :groups_and_projects do expect(user.project_authorizations.pluck(:user_id, :project_id, :access_level)).to match_array(attributes.map(&:values)) end end + + context 'with loose foreign key on project_authorizations.user_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let_it_be(:parent) { create(:user) } + let_it_be(:model) { create(:project_authorization, user: parent) } + end + end end diff --git a/spec/models/project_authorizations/changes_spec.rb b/spec/models/project_authorizations/changes_spec.rb index 5f4dd963fb3..d6ccfccbcbe 100644 --- a/spec/models/project_authorizations/changes_spec.rb +++ b/spec/models/project_authorizations/changes_spec.rb @@ -37,6 +37,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro .with(data: project_data) .and_return(project_event) + allow(::Gitlab::EventStore).to receive(:publish) expect(::Gitlab::EventStore).to receive(:publish).with(project_event) end @@ -44,9 +45,51 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro end end - shared_examples_for 'does not publishes AuthorizationsChangedEvent' do - it 'does not publishes a AuthorizationsChangedEvent event' do + shared_examples_for 'publishes AuthorizationsRemovedEvent' do + it 'publishes a AuthorizationsRemovedEvent event with project id' do + project_ids.each do |project_id| + project_data = { project_id: project_id, user_ids: user_ids } + project_event = instance_double('::ProjectAuthorizations::AuthorizationsRemovedEvent', data: project_data) + + allow(::ProjectAuthorizations::AuthorizationsRemovedEvent).to receive(:new) + .with(data: project_data) + .and_return(project_event) + + allow(::Gitlab::EventStore).to receive(:publish) + expect(::Gitlab::EventStore).to receive(:publish).with(project_event) + end + + apply_project_authorization_changes + end + + context 'when feature flag "user_approval_rules_removal" is disabled' do + before do + stub_feature_flags(user_approval_rules_removal: false) + end + + it 'does not publish a AuthorizationsRemovedEvent event' do + expect(::Gitlab::EventStore).not_to( + receive(:publish).with(an_instance_of(::ProjectAuthorizations::AuthorizationsRemovedEvent)) + ) + + apply_project_authorization_changes + end + end + end + + shared_examples_for 'does not publish AuthorizationsChangedEvent' do + it 'does not publish a AuthorizationsChangedEvent event' do + expect(::Gitlab::EventStore).not_to receive(:publish) + .with(an_instance_of(::ProjectAuthorizations::AuthorizationsChangedEvent)) + + apply_project_authorization_changes + end + end + + shared_examples_for 'does not publish AuthorizationsRemovedEvent' do + it 'does not publish a AuthorizationsRemovedEvent event' do expect(::Gitlab::EventStore).not_to receive(:publish) + .with(an_instance_of(::ProjectAuthorizations::AuthorizationsRemovedEvent)) apply_project_authorization_changes end @@ -112,6 +155,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'logs the detail', batch_size: 2 it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsRemovedEvent' context 'when the GitLab installation does not have a replica database configured' do before do @@ -122,6 +166,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'inserts the rows in batches, as per the `per_batch` size, without a delay between batches' it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsRemovedEvent' end end @@ -133,6 +178,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'inserts the rows in batches, as per the `per_batch` size, without a delay between batches' it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsRemovedEvent' end end @@ -195,6 +241,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'logs the detail', batch_size: 2 it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'publishes AuthorizationsRemovedEvent' context 'when the GitLab installation does not have a replica database configured' do before do @@ -205,6 +252,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'removes project authorizations of the users in the current project, without a delay' it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'publishes AuthorizationsRemovedEvent' end end @@ -216,20 +264,23 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'removes project authorizations of the users in the current project, without a delay' it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'publishes AuthorizationsRemovedEvent' end context 'when the user_ids list is empty' do let(:user_ids) { [] } it_behaves_like 'does not removes project authorizations of the users in the current project' - it_behaves_like 'does not publishes AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsRemovedEvent' end context 'when the user_ids list is nil' do let(:user_ids) { nil } it_behaves_like 'does not removes project authorizations of the users in the current project' - it_behaves_like 'does not publishes AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsRemovedEvent' end end @@ -241,6 +292,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro let_it_be(:project_4) { create(:project) } let(:project_ids) { [project_1.id, project_2.id, project_3.id] } + let(:user_ids) { [user.id] } let(:project_authorization_changes) do ProjectAuthorizations::Changes.new do |changes| @@ -291,6 +343,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'logs the detail', batch_size: 2 it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'publishes AuthorizationsRemovedEvent' context 'when the GitLab installation does not have a replica database configured' do before do @@ -301,6 +354,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'removes project authorizations of projects from the current user, without a delay' it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'publishes AuthorizationsRemovedEvent' end end @@ -312,20 +366,23 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro it_behaves_like 'removes project authorizations of projects from the current user, without a delay' it_behaves_like 'does not log any detail' it_behaves_like 'publishes AuthorizationsChangedEvent' + it_behaves_like 'publishes AuthorizationsRemovedEvent' end context 'when the project_ids list is empty' do let(:project_ids) { [] } it_behaves_like 'does not removes any project authorizations from the current user' - it_behaves_like 'does not publishes AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsRemovedEvent' end context 'when the user_ids list is nil' do let(:project_ids) { nil } it_behaves_like 'does not removes any project authorizations from the current user' - it_behaves_like 'does not publishes AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsChangedEvent' + it_behaves_like 'does not publish AuthorizationsRemovedEvent' end end end diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index c0a78ff2f53..149b0d4df8c 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -31,6 +31,7 @@ RSpec.describe ProjectFeature, feature_category: :groups_and_projects do specify { expect(subject.package_registry_access_level).to eq(ProjectFeature::ENABLED) } specify { expect(subject.container_registry_access_level).to eq(ProjectFeature::ENABLED) } specify { expect(subject.model_experiments_access_level).to eq(ProjectFeature::ENABLED) } + specify { expect(subject.model_registry_access_level).to eq(ProjectFeature::ENABLED) } end describe 'PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT' do diff --git a/spec/models/project_group_link_spec.rb b/spec/models/project_group_link_spec.rb index f141b8e83d6..bfbb7c91f47 100644 --- a/spec/models/project_group_link_spec.rb +++ b/spec/models/project_group_link_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ProjectGroupLink do +RSpec.describe ProjectGroupLink, feature_category: :groups_and_projects do describe "Associations" do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:project) } @@ -18,6 +18,7 @@ RSpec.describe ProjectGroupLink do it { is_expected.to validate_uniqueness_of(:group_id).scoped_to(:project_id).with_message(/already shared/) } it { is_expected.to validate_presence_of(:group) } it { is_expected.to validate_presence_of(:group_access) } + it { is_expected.to validate_inclusion_of(:group_access).in_array(Gitlab::Access.all_values) } it "doesn't allow a project to be shared with the group it is in" do project_group_link.group = group @@ -30,12 +31,6 @@ RSpec.describe ProjectGroupLink do expect(project_group_link).not_to be_valid end - - it 'does not allow a project to be shared with `OWNER` access level' do - project_group_link.group_access = Gitlab::Access::OWNER - - expect(project_group_link).not_to be_valid - end end describe 'scopes' do @@ -62,4 +57,27 @@ RSpec.describe ProjectGroupLink do it { expect(described_class.search(group.name)).to eq([project_group_link]) } it { expect(described_class.search('not-a-group-name')).to be_empty } end + + describe '#owner_access?' do + it 'returns true for links with OWNER access' do + link = create(:project_group_link, :owner) + + expect(link.owner_access?).to eq(true) + end + + it 'returns false for links without OWNER access' do + link = create(:project_group_link, :guest) + + expect(link.owner_access?).to eq(false) + end + end + + describe '#human_access' do + it 'delegates to Gitlab::Access' do + project_group_link = create(:project_group_link, :reporter) + expect(Gitlab::Access).to receive(:human_access).with(project_group_link.group_access).and_call_original + + expect(project_group_link.human_access).to eq('Reporter') + end + end end diff --git a/spec/models/project_repository_spec.rb b/spec/models/project_repository_spec.rb index eba908b0fdb..a38782cfd51 100644 --- a/spec/models/project_repository_spec.rb +++ b/spec/models/project_repository_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ProjectRepository do +RSpec.describe ProjectRepository, feature_category: :source_code_management do describe 'associations' do it { is_expected.to belong_to(:shard) } it { is_expected.to belong_to(:project) } @@ -25,4 +25,28 @@ RSpec.describe ProjectRepository do expect(described_class.find_project('@@unexisting/path/to/project')).to be_nil end end + + describe '#object_format' do + subject { project_repository.object_format } + + let(:project_repository) { build(:project_repository, object_format: object_format) } + + context 'when object format is sha1' do + let(:object_format) { 'sha1' } + + it { is_expected.to eq 'sha1' } + end + + context 'when object format is sha256' do + let(:object_format) { 'sha256' } + + it { is_expected.to eq 'sha256' } + end + + context 'when object format is not set' do + let(:project_repository) { build(:project_repository) } + + it { is_expected.to eq 'sha1' } + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3ea5f6ea0ae..c256c4f10f8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -50,6 +50,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it { is_expected.to have_one(:catalog_resource) } it { is_expected.to have_many(:ci_components).class_name('Ci::Catalog::Resources::Component') } it { is_expected.to have_many(:catalog_resource_versions).class_name('Ci::Catalog::Resources::Version') } + it { is_expected.to have_many(:catalog_resource_sync_events).class_name('Ci::Catalog::Resources::SyncEvent') } it { is_expected.to have_one(:microsoft_teams_integration) } it { is_expected.to have_one(:mattermost_integration) } it { is_expected.to have_one(:hangouts_chat_integration) } @@ -1128,6 +1129,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) } it { is_expected.to delegate_method(:environments_access_level).to(:project_feature) } it { is_expected.to delegate_method(:model_experiments_access_level).to(:project_feature) } + it { is_expected.to delegate_method(:model_registry_access_level).to(:project_feature) } it { is_expected.to delegate_method(:feature_flags_access_level).to(:project_feature) } it { is_expected.to delegate_method(:releases_access_level).to(:project_feature) } it { is_expected.to delegate_method(:infrastructure_access_level).to(:project_feature) } @@ -2049,50 +2051,31 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#avatar_type' do - let(:project) { create(:project) } - - it 'is true if avatar is image' do - project.update_attribute(:avatar, 'uploads/avatar.png') - expect(project.avatar_type).to be_truthy - end - - it 'is false if avatar is html page' do - project.update_attribute(:avatar, 'uploads/avatar.html') - project.avatar_type - - expect(project.errors.added?(:avatar, "file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp")).to be true + context 'with avatar' do + it_behaves_like Avatarable do + let(:model) { create(:project, :with_avatar) } end - end - describe '#avatar_url' do - subject { project.avatar_url } + describe '#avatar_url' do + subject { project.avatar_url } - let(:project) { create(:project) } + let(:project) { create(:project) } - context 'when avatar file is uploaded' do - let(:project) { create(:project, :public, :with_avatar) } + context 'when avatar file in git' do + before do + allow(project).to receive(:avatar_in_git) { true } + end - it 'shows correct url' do - expect(project.avatar_url).to eq(project.avatar.url) - expect(project.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, project.avatar.url].join) - end - end + let(:avatar_path) { "/#{project.full_path}/-/avatar" } - context 'when avatar file in git' do - before do - allow(project).to receive(:avatar_in_git) { true } + it { is_expected.to eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" } end - let(:avatar_path) { "/#{project.full_path}/-/avatar" } - - it { is_expected.to eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" } - end - - context 'when git repo is empty' do - let(:project) { create(:project) } + context 'when git repo is empty' do + let(:project) { create(:project) } - it { is_expected.to eq nil } + it { is_expected.to eq nil } + end end end @@ -2176,19 +2159,9 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '.with_jira_dvcs_cloud' do - it 'returns the correct project' do - jira_dvcs_cloud_project = create(:project, :jira_dvcs_cloud) - create(:project, :jira_dvcs_server) - - expect(described_class.with_jira_dvcs_cloud).to contain_exactly(jira_dvcs_cloud_project) - end - end - describe '.with_jira_dvcs_server' do it 'returns the correct project' do jira_dvcs_server_project = create(:project, :jira_dvcs_server) - create(:project, :jira_dvcs_cloud) expect(described_class.with_jira_dvcs_server).to contain_exactly(jira_dvcs_server_project) end @@ -2470,17 +2443,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it 'returns custom email address' do expect(subject).to eq(custom_email) end - - context 'when feature flag service_desk_custom_email is disabled' do - before do - stub_feature_flags(service_desk_custom_email: false) - end - - it 'returns custom email address' do - # Don't check for a specific value. Just make sure it's not the custom email - expect(subject).not_to eq(custom_email) - end - end end end @@ -3102,23 +3064,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect(project.repository).not_to eq(previous_repository) end - - context 'when "replicate_object_pool_on_move" FF is disabled' do - before do - stub_feature_flags(replicate_object_pool_on_move: false) - end - - it 'does not update a memoized repository value' do - previous_repository = project.repository - - allow(project).to receive(:disk_path).and_return('fancy/new/path') - allow(project).to receive(:repository_storage).and_return('foo') - - project.track_project_repository - - expect(project.repository).to eq(previous_repository) - end - end end end @@ -3151,7 +3096,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end it 'passes through default branch' do - expect(project.repository).to receive(:create_repository).with('pineapple') + expect(project.repository).to receive(:create_repository).with('pineapple', object_format: nil) expect(project.create_repository(default_branch: 'pineapple')).to eq(true) end @@ -3165,6 +3110,13 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr project.create_repository end end + + context 'using a SHA256 repository' do + it 'creates the repository' do + expect(project.repository).to receive(:create_repository).with(nil, object_format: Repository::FORMAT_SHA256) + expect(project.create_repository(object_format: Repository::FORMAT_SHA256)).to eq(true) + end + end end describe '#ensure_repository' do @@ -6949,12 +6901,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr let(:repository_storage) { shard_to.name } before do - stub_storage_settings( - 'test_second_storage' => { - 'gitaly_address' => Gitlab.config.repositories.storages.default.gitaly_address, - 'path' => TestEnv::SECOND_STORAGE_PATH - } - ) + stub_storage_settings('test_second_storage' => {}) project.update!(pool_repository: project_pool, repository_storage: repository_storage) end @@ -6969,14 +6916,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect { swap_pool_repository! }.to change { project.reload.pool_repository }.from(pool1).to(pool2) end - context 'when feature flag replicate_object_pool_on_move is disabled' do - before do - stub_feature_flags(replicate_object_pool_on_move: false) - end - - it_behaves_like 'no pool repository swap' - end - context 'when repository does not exist' do let(:project) { build(:project) } @@ -7071,18 +7010,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr subject end - - context 'when feature flag replicate_object_pool_on_move is disabled' do - before do - stub_feature_flags(replicate_object_pool_on_move: false) - end - - it 'links pool repository to project repository' do - expect(pool).to receive(:link_repository).with(project.repository) - - subject - end - end end end @@ -7963,14 +7890,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end end - - describe '#activity_path' do - it 'returns the project activity_path' do - expected_path = "/#{project.full_path}/activity" - - expect(project.activity_path).to eq(expected_path) - end - end end describe '#default_branch_or_main' do @@ -9000,62 +8919,53 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - # TODO: Remove/update this spec after background syncing is implemented. See https://gitlab.com/gitlab-org/gitlab/-/issues/429376. - describe '#update_catalog_resource' do - let_it_be_with_reload(:project) { create(:project, name: 'My project name', description: 'My description') } - let_it_be_with_reload(:resource) { create(:ci_catalog_resource, project: project) } + describe 'catalog resource process sync events worker' do + let_it_be_with_reload(:project) { create(:project, name: 'Test project', description: 'Test description') } - shared_examples 'name, description, and visibility_level of the catalog resource match the project' do - it do - expect(project).to receive(:update_catalog_resource).once.and_call_original - - project.save! + context 'when the project has a catalog resource' do + let_it_be(:resource) { create(:ci_catalog_resource, project: project) } - expect(resource.name).to eq(project.name) - expect(resource.description).to eq(project.description) - expect(resource.visibility_level).to eq(project.visibility_level) - end - end + context 'when project name is updated' do + it 'enqueues Ci::Catalog::Resources::ProcessSyncEventsWorker' do + expect(Ci::Catalog::Resources::ProcessSyncEventsWorker).to receive(:perform_async).once - context 'when the project name is updated' do - before do - project.name = 'My new project name' + project.update!(name: 'New name') + end end - it_behaves_like 'name, description, and visibility_level of the catalog resource match the project' - end + context 'when project description is updated' do + it 'enqueues Ci::Catalog::Resources::ProcessSyncEventsWorker' do + expect(Ci::Catalog::Resources::ProcessSyncEventsWorker).to receive(:perform_async).once - context 'when the project description is updated' do - before do - project.description = 'My new description' + project.update!(description: 'New description') + end end - it_behaves_like 'name, description, and visibility_level of the catalog resource match the project' - end + context 'when project visibility_level is updated' do + it 'enqueues Ci::Catalog::Resources::ProcessSyncEventsWorker' do + expect(Ci::Catalog::Resources::ProcessSyncEventsWorker).to receive(:perform_async).once - context 'when the project visibility_level is updated' do - before do - project.visibility_level = 10 + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end end - it_behaves_like 'name, description, and visibility_level of the catalog resource match the project' - end - - context 'when neither the project name, description, nor visibility_level are updated' do - it 'does not call update_catalog_resource' do - expect(project).not_to receive(:update_catalog_resource) + context 'when neither the project name, description, nor visibility_level are updated' do + it 'does not enqueue Ci::Catalog::Resources::ProcessSyncEventsWorker' do + expect(Ci::Catalog::Resources::ProcessSyncEventsWorker).not_to receive(:perform_async) - project.update!(path: 'path') + project.update!(path: 'path') + end end end context 'when the project does not have a catalog resource' do - let_it_be(:project2) { create(:project) } - - it 'does not call update_catalog_resource' do - expect(project2).not_to receive(:update_catalog_resource) + it 'does not enqueue Ci::Catalog::Resources::ProcessSyncEventsWorker' do + expect(Ci::Catalog::Resources::ProcessSyncEventsWorker).not_to receive(:perform_async) - project.update!(name: 'name') + project.update!( + name: 'New name', + description: 'New description', + visibility_level: Gitlab::VisibilityLevel::INTERNAL) end end end diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index dd7989244d4..211ac257c53 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -15,8 +15,8 @@ RSpec.describe ProjectStatistics do describe '.for_project_ids' do it 'returns only requested projects' do stats = create_list(:project_statistics, 3) - project_ids = stats[0..1].map { |s| s.project_id } - expected_ids = stats[0..1].map { |s| s.id } + project_ids = stats[0..1].map(&:project_id) + expected_ids = stats[0..1].map(&:id) requested_stats = described_class.for_project_ids(project_ids).pluck(:id) diff --git a/spec/models/projects/repository_storage_move_spec.rb b/spec/models/projects/repository_storage_move_spec.rb index c5fbc92176f..afa9a8c4319 100644 --- a/spec/models/projects/repository_storage_move_spec.rb +++ b/spec/models/projects/repository_storage_move_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::RepositoryStorageMove, type: :model do +RSpec.describe Projects::RepositoryStorageMove, type: :model, feature_category: :source_code_management do it_behaves_like 'handles repository moves' do let_it_be_with_refind(:container) { create(:project) } diff --git a/spec/models/projects/topic_spec.rb b/spec/models/projects/topic_spec.rb index b3a55ccd370..ebe53f3761d 100644 --- a/spec/models/projects/topic_spec.rb +++ b/spec/models/projects/topic_spec.rb @@ -96,31 +96,8 @@ RSpec.describe Projects::Topic do end end - describe '#avatar_type' do - it "is true if avatar is image" do - topic.update_attribute(:avatar, 'uploads/avatar.png') - expect(topic.avatar_type).to be_truthy - end - - it "is false if avatar is html page" do - topic.update_attribute(:avatar, 'uploads/avatar.html') - topic.avatar_type - - expect(topic.errors.added?(:avatar, "file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp")).to be true - end - end - - describe '#avatar_url' do - context 'when avatar file is uploaded' do - before do - topic.update!(avatar: fixture_file_upload("spec/fixtures/dk.png")) - end - - it 'shows correct avatar url' do - expect(topic.avatar_url).to eq(topic.avatar.url) - expect(topic.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, topic.avatar.url].join) - end - end + it_behaves_like Avatarable do + let(:model) { create(:topic, :with_avatar) } end describe '#title_or_name' do diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 164cef95cb6..bff9f73e44a 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -39,6 +39,24 @@ RSpec.describe Release, feature_category: :release_orchestration do end end + describe 'scopes' do + let_it_be(:another_project) { create(:project) } + let_it_be(:release) { create(:release, project: project, author: user, tag: 'v1') } + let_it_be(:another_release) { create(:release, project: another_project, tag: 'v2') } + + describe '.for_projects' do + it 'returns releases for the given projects' do + expect(described_class.for_projects([project])).to eq([release]) + end + end + + describe '.by_tag' do + it 'returns releases with the given tag' do + expect(described_class.by_tag(release.tag)).to eq([release]) + end + end + end + context 'when description of a release is longer than the limit' do let(:description) { 'a' * (Gitlab::Database::MAX_TEXT_SIZE_LIMIT + 1) } let(:release) { build(:release, project: project, description: description) } @@ -86,6 +104,56 @@ RSpec.describe Release, feature_category: :release_orchestration do end end + describe '#update_catalog_resource' do + let_it_be(:project) { create(:project) } + let_it_be_with_refind(:release) { create(:release, project: project, author: user) } + + context 'when the project is a catalog resource' do + before_all do + create(:ci_catalog_resource, project: project) + end + + context 'when released_at is updated' do + it 'calls update_catalog_resource' do + expect(release).to receive(:update_catalog_resource).once + + release.update!(released_at: release.released_at + 1.day) + end + end + + context 'when the release is destroyed' do + it 'calls update_catalog_resource' do + expect(release).to receive(:update_catalog_resource).once + + release.destroy! + end + end + end + + context 'when the project is not a catalog resource' do + it 'does not call update_catalog_resource' do + expect(release).not_to receive(:update_catalog_resource) + + release.update!(released_at: release.released_at + 1.day) + release.destroy! + end + end + end + + describe 'tagged' do + # We only test for empty string since there's a not null constraint at the database level + it 'does not return the tagless release' do + empty_string_tag = create(:release, tag: 'v99.0.0') + empty_string_tag.update_column(:tag, '') + + expect(described_class.tagged).not_to include(empty_string_tag) + end + + it 'does return the tagged releases' do + expect(described_class.tagged).to include(release) + end + end + describe 'latest releases' do let_it_be(:yesterday) { Time.zone.now - 1.day } let_it_be(:tomorrow) { Time.zone.now + 1.day } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 606c4ea05b9..eeb0bbb8e7d 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1585,6 +1585,29 @@ RSpec.describe Repository, feature_category: :source_code_management do end end + describe "#jenkinsfile?" do + let_it_be(:project) { create(:project, :repository) } + + it 'returns valid file' do + files = [TestBlob.new('file'), TestBlob.new('Jenkinsfile'), TestBlob.new('copying')] + expect(repository.tree).to receive(:blobs).and_return(files) + + expect(repository.jenkinsfile?).to be(true) + end + + it 'is case-insensitive' do + files = [TestBlob.new('file'), TestBlob.new('JENKINSFILE'), TestBlob.new('copying')] + expect(repository.tree).to receive(:blobs).and_return(files) + + expect(repository.jenkinsfile?).to be(true) + end + + it 'returns false if does not exists' do + expect(repository.tree).to receive(:blobs).and_return([]) + expect(repository.jenkinsfile?).to be(false) + end + end + describe '#ambiguous_ref?' do subject { repository.ambiguous_ref?(ref) } @@ -2201,46 +2224,82 @@ RSpec.describe Repository, feature_category: :source_code_management do describe 'rolling back the `rebase_commit_sha`' do let(:new_sha) { Digest::SHA1.hexdigest('foo') } - it 'does not rollback when there are no errors' do - second_response = double(pre_receive_error: nil, git_error: nil) - mock_gitaly(second_response) + context 'when there are no errors' do + before do + responses = [ + double(rebase_sha: new_sha).as_null_object, + double + ] + + expect_any_instance_of( + Gitaly::OperationService::Stub + ).to receive(:user_rebase_confirmable).and_return(responses.each) + end - repository.rebase(user, merge_request) + it 'does not rollback when there are no errors' do + repository.rebase(user, merge_request) - expect(merge_request.reload.rebase_commit_sha).to eq(new_sha) + expect(merge_request.reload.rebase_commit_sha).to eq(new_sha) + end end - it 'does rollback when a PreReceiveError is encountered in the second step' do - second_response = double(pre_receive_error: 'my_error', git_error: nil) - mock_gitaly(second_response) + context 'when there was an error' do + let(:first_response) do + double(rebase_sha: new_sha).as_null_object + end - expect do - repository.rebase(user, merge_request) - end.to raise_error(Gitlab::Git::PreReceiveError) + before do + request_enum = double(push: nil).as_null_object + allow(Gitlab::GitalyClient::QueueEnumerator).to receive(:new).and_return(request_enum) - expect(merge_request.reload.rebase_commit_sha).to be_nil - end + expect_any_instance_of( + Gitaly::OperationService::Stub + ).to receive(:user_rebase_confirmable).and_return(first_response) + + # Faking second request failure + allow(request_enum).to receive(:push) + .with(Gitaly::UserRebaseConfirmableRequest.new(apply: true)) { raise(error) } + end - it 'does rollback when a GitError is encountered in the second step' do - second_response = double(pre_receive_error: nil, git_error: 'git error') - mock_gitaly(second_response) + context 'when a PreReceiveError is encountered in the second step' do + let(:error) do + new_detailed_error( + GRPC::Core::StatusCodes::INTERNAL, + 'something failed', + Gitaly::UserRebaseConfirmableError.new( + access_check: Gitaly::AccessCheckError.new( + error_message: 'something went wrong' + ))) + end - expect do - repository.rebase(user, merge_request) - end.to raise_error(Gitlab::Git::Repository::GitError) + it 'does rollback' do + expect do + repository.rebase(user, merge_request) + end.to raise_error(Gitlab::Git::PreReceiveError) - expect(merge_request.reload.rebase_commit_sha).to be_nil - end + expect(merge_request.reload.rebase_commit_sha).to be_nil + end + end - def mock_gitaly(second_response) - responses = [ - double(rebase_sha: new_sha).as_null_object, - second_response - ] + context 'when a when a GitError is encountered in the second step' do + let(:error) do + new_detailed_error( + GRPC::Core::StatusCodes::INTERNAL, + 'something failed', + Gitaly::UserSquashError.new( + rebase_conflict: Gitaly::MergeConflictError.new( + conflicting_files: ['conflicting-file'] + ))) + end - expect_any_instance_of( - Gitaly::OperationService::Stub - ).to receive(:user_rebase_confirmable).and_return(responses.each) + it 'does rollback' do + expect do + repository.rebase(user, merge_request) + end.to raise_error(Gitlab::Git::Repository::GitError) + + expect(merge_request.reload.rebase_commit_sha).to be_nil + end + end end end end @@ -3626,12 +3685,8 @@ RSpec.describe Repository, feature_category: :source_code_management do describe '.pick_storage_shard', :request_store do before do - storages = { - 'default' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories'), - 'picked' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/repositories') - } + stub_storage_settings('picked' => {}) - allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') Gitlab::CurrentSettings.current_application_settings @@ -3955,6 +4010,28 @@ RSpec.describe Repository, feature_category: :source_code_management do end end + describe '#object_format' do + subject { repository.object_format } + + context 'for SHA1 repository' do + it { is_expected.to eq('sha1') } + end + + context 'for SHA256 repository' do + let(:project) { create(:project, :empty_repo, object_format: Repository::FORMAT_SHA256) } + + it { is_expected.to eq('sha256') } + end + + context 'for missing repository' do + before do + allow(repository).to receive(:exists?).and_return(false) + end + + it { is_expected.to be_nil } + end + end + describe '#get_file_attributes' do let(:project) do create(:project, :custom_repo, files: { diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index aa5fc231e14..7cada013636 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -311,8 +311,8 @@ RSpec.describe Route do context 'with loose foreign key on routes.namespace_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:namespace) } - let!(:model) { parent.route } + let_it_be(:parent) { create(:namespace) } + let_it_be(:model) { parent.route } end end end diff --git a/spec/models/service_desk/custom_email_credential_spec.rb b/spec/models/service_desk/custom_email_credential_spec.rb index dbf47a8f6a7..6ba5906e264 100644 --- a/spec/models/service_desk/custom_email_credential_spec.rb +++ b/spec/models/service_desk/custom_email_credential_spec.rb @@ -21,7 +21,7 @@ RSpec.describe ServiceDesk::CustomEmailCredential, feature_category: :service_de it { is_expected.not_to allow_value('/example').for(:smtp_address) } it { is_expected.not_to allow_value('localhost').for(:smtp_address) } it { is_expected.not_to allow_value('127.0.0.1').for(:smtp_address) } - it { is_expected.not_to allow_value('192.168.12.12').for(:smtp_address) } # disallow local network + it { is_expected.to allow_value('192.168.12.12').for(:smtp_address) } # allow local network on self-managed it { is_expected.to validate_presence_of(:smtp_port) } it { is_expected.to validate_numericality_of(:smtp_port).only_integer.is_greater_than(0) } @@ -31,6 +31,10 @@ RSpec.describe ServiceDesk::CustomEmailCredential, feature_category: :service_de it { is_expected.to validate_presence_of(:smtp_password) } it { is_expected.to validate_length_of(:smtp_password).is_at_least(8).is_at_most(128) } + + context 'when SaaS', :saas do + it { is_expected.not_to allow_value('192.168.12.12').for(:smtp_address) } # Disallow local network on .com + end end describe 'encrypted #smtp_username' do diff --git a/spec/models/snippets/repository_storage_move_spec.rb b/spec/models/snippets/repository_storage_move_spec.rb index ed518faf6ff..b01ad823d68 100644 --- a/spec/models/snippets/repository_storage_move_spec.rb +++ b/spec/models/snippets/repository_storage_move_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Snippets::RepositoryStorageMove, type: :model do +RSpec.describe Snippets::RepositoryStorageMove, type: :model, feature_category: :source_code_management do it_behaves_like 'handles repository moves' do let_it_be_with_refind(:container) { create(:snippet) } diff --git a/spec/models/tree_spec.rb b/spec/models/tree_spec.rb index 20d786f311f..302cd9e9f10 100644 --- a/spec/models/tree_spec.rb +++ b/spec/models/tree_spec.rb @@ -2,69 +2,58 @@ require 'spec_helper' -RSpec.describe Tree do - let_it_be(:repository) { create(:project, :repository).repository } - - let(:sha) { repository.root_ref } - +RSpec.describe Tree, feature_category: :source_code_management do subject(:tree) { described_class.new(repository, '54fcc214') } - describe '#readme' do - before do - stub_const('FakeBlob', Class.new) - FakeBlob.class_eval do - attr_reader :name + let_it_be(:repository) { create(:project, :repository).repository } - def initialize(name) - @name = name - end + describe '#readme' do + subject { tree.readme } - def readme? - name =~ /^readme/i - end - end + before do + allow(tree).to receive(:blobs).and_return(files) end - it 'returns nil when repository does not contains a README file' do - files = [FakeBlob.new('file'), FakeBlob.new('license'), FakeBlob.new('copying')] - expect(subject).to receive(:blobs).and_return(files) + context 'when repository does not contains a README file' do + let(:files) { [fake_blob('file'), fake_blob('license'), fake_blob('copying')] } - expect(subject.readme).to eq nil + it { is_expected.to be_nil } end - it 'returns nil when repository does not contains a previewable README file' do - files = [FakeBlob.new('file'), FakeBlob.new('README.pages'), FakeBlob.new('README.png')] - expect(subject).to receive(:blobs).and_return(files) + context 'when repository does not contains a previewable README file' do + let(:files) { [fake_blob('file'), fake_blob('README.pages'), fake_blob('README.png')] } - expect(subject.readme).to eq nil + it { is_expected.to be_nil } end - it 'returns README when repository contains a previewable README file' do - files = [FakeBlob.new('README.png'), FakeBlob.new('README'), FakeBlob.new('file')] - expect(subject).to receive(:blobs).and_return(files) + context 'when repository contains a previewable README file' do + let(:files) { [fake_blob('README.png'), fake_blob('README'), fake_blob('file')] } - expect(subject.readme.name).to eq 'README' + it { is_expected.to have_attributes(name: 'README') } end - it 'returns first previewable README when repository contains more than one' do - files = [FakeBlob.new('file'), FakeBlob.new('README.md'), FakeBlob.new('README.asciidoc')] - expect(subject).to receive(:blobs).and_return(files) + context 'when repository contains more than one README file' do + let(:files) { [fake_blob('file'), fake_blob('README.md'), fake_blob('README.asciidoc')] } - expect(subject.readme.name).to eq 'README.md' - end + it 'returns first previewable README' do + is_expected.to have_attributes(name: 'README.md') + end - it 'returns first plain text README when repository contains more than one' do - files = [FakeBlob.new('file'), FakeBlob.new('README'), FakeBlob.new('README.txt')] - expect(subject).to receive(:blobs).and_return(files) + context 'when only plain-text READMEs' do + let(:files) { [fake_blob('file'), fake_blob('README'), fake_blob('README.txt')] } - expect(subject.readme.name).to eq 'README' + it 'returns first plain text README' do + is_expected.to have_attributes(name: 'README') + end + end end - it 'prioritizes previewable README file over one in plain text' do - files = [FakeBlob.new('file'), FakeBlob.new('README'), FakeBlob.new('README.md')] - expect(subject).to receive(:blobs).and_return(files) + context 'when the repository has a previewable and plain text READMEs' do + let(:files) { [fake_blob('file'), fake_blob('README'), fake_blob('README.md')] } - expect(subject.readme.name).to eq 'README.md' + it 'prefers previewable README file' do + is_expected.to have_attributes(name: 'README.md') + end end end @@ -73,4 +62,10 @@ RSpec.describe Tree do it { is_expected.to be_an_instance_of(Gitaly::PaginationCursor) } end + + private + + def fake_blob(name) + instance_double(Gitlab::Git::Blob, name: name) + end end diff --git a/spec/models/user_custom_attribute_spec.rb b/spec/models/user_custom_attribute_spec.rb index 4c27e8d8944..6eaa1452651 100644 --- a/spec/models/user_custom_attribute_spec.rb +++ b/spec/models/user_custom_attribute_spec.rb @@ -122,4 +122,80 @@ RSpec.describe UserCustomAttribute, feature_category: :user_profile do expect(user.custom_attributes.find_by(key: 'arkose_custom_score').value).to eq(custom_score) end end + + describe '.set_deleted_own_account_at' do + let_it_be(:user) { create(:user) } + + subject(:method_call) { described_class.set_deleted_own_account_at(user) } + + it 'creates a custom attribute with "deleted_own_account_at" key associated to the user' do + freeze_time do + expect { method_call }.to change { user.custom_attributes.count }.by(1) + + record = user.custom_attributes.find_by_key(UserCustomAttribute::DELETED_OWN_ACCOUNT_AT) + expect(record.value).to eq Time.zone.now.to_s + end + end + + context 'when passed in user is nil' do + let(:user) { nil } + + it 'does nothing' do + expect { method_call }.not_to change { UserCustomAttribute.count } + end + end + end + + describe '.set_skipped_account_deletion_at' do + let_it_be(:user) { create(:user) } + + subject(:method_call) { described_class.set_skipped_account_deletion_at(user) } + + it 'creates a custom attribute with "skipped_account_deletion_at" key associated to the user' do + freeze_time do + expect { method_call }.to change { user.custom_attributes.count }.by(1) + + record = user.custom_attributes.find_by_key(UserCustomAttribute::SKIPPED_ACCOUNT_DELETION_AT) + expect(record.value).to eq Time.zone.now.to_s + end + end + + context 'when passed in user is nil' do + let(:user) { nil } + + it 'does nothing' do + expect { method_call }.not_to change { UserCustomAttribute.count } + end + end + end + + describe '.set_assumed_high_risk_reason' do + let_it_be(:user) { create(:user) } + let(:reason) { 'Because' } + + subject(:call_method) { described_class.set_assumed_high_risk_reason(user: user, reason: reason) } + + it 'creates a custom attribute with correct attribute values for the user' do + expect { call_method }.to change { user.custom_attributes.count }.by(1) + + record = user.custom_attributes.find_by_key(UserCustomAttribute::ASSUMED_HIGH_RISK_REASON) + expect(record.value).to eq 'Because' + end + + context 'when passed in user is nil' do + let(:user) { nil } + + it 'does nothing' do + expect { call_method }.not_to change { UserCustomAttribute.count } + end + end + + context 'when there is no reason passed in' do + let(:reason) { nil } + + it 'does nothing' do + expect { call_method }.not_to change { UserCustomAttribute.count } + end + end + end end diff --git a/spec/models/user_highest_role_spec.rb b/spec/models/user_highest_role_spec.rb index 7ef04466b6f..d8cf09e7fd4 100644 --- a/spec/models/user_highest_role_spec.rb +++ b/spec/models/user_highest_role_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe UserHighestRole do +RSpec.describe UserHighestRole, feature_category: :sm_provisioning do describe 'associations' do it { is_expected.to belong_to(:user).required } end @@ -26,4 +26,22 @@ RSpec.describe UserHighestRole do end end end + + describe '.allowed_values' do + let(:expected_allowed_values) do + [ + Gitlab::Access::GUEST, + Gitlab::Access::REPORTER, + Gitlab::Access::DEVELOPER, + Gitlab::Access::MAINTAINER, + Gitlab::Access::OWNER + ] + end + + it 'returns all access values' do + expected_allowed_values << Gitlab::Access::MINIMAL_ACCESS if Gitlab.ee? + + expect(::UserHighestRole.allowed_values).to eq(expected_allowed_values) + end + end end diff --git a/spec/models/user_interacted_project_spec.rb b/spec/models/user_interacted_project_spec.rb deleted file mode 100644 index aa038b06d8d..00000000000 --- a/spec/models/user_interacted_project_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe UserInteractedProject do - let_it_be(:project) { create(:project) } - let_it_be(:author) { project.creator } - - describe '.track' do - subject { described_class.track(event) } - - let(:event) { build(:event, project: project, author: author) } - - Event.actions.each_key do |action| - context "for all actions (event types)" do - let(:event) { build(:event, project: project, author: author, action: action) } - - it 'creates a record' do - expect { subject }.to change { described_class.count }.from(0).to(1) - end - end - end - - it 'sets project accordingly' do - subject - expect(described_class.first.project).to eq(event.project) - end - - it 'sets user accordingly' do - subject - expect(described_class.first.user).to eq(event.author) - end - - it 'only creates a record once per user/project' do - expect do - subject - described_class.track(event) - end.to change { described_class.count }.from(0).to(1) - end - - describe 'with an event without a project' do - let(:event) { build(:event, project: nil) } - - it 'ignores the event' do - expect { subject }.not_to change { described_class.count } - end - end - end - - it { is_expected.to validate_presence_of(:project_id) } - it { is_expected.to validate_presence_of(:user_id) } -end diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 343576de4d3..ee3fbb97e47 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -64,6 +64,10 @@ RSpec.describe UserPreference, feature_category: :user_profile do end end + describe 'associations' do + it { is_expected.to belong_to(:home_organization).class_name('Organizations::Organization').optional } + 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]) @@ -291,4 +295,30 @@ RSpec.describe UserPreference, feature_category: :user_profile do expect(pref.read_attribute(:render_whitespace_in_code)).to eq(true) end end + + describe '#user_belongs_to_home_organization' do + let_it_be(:organization) { create(:organization) } + + context 'when user is an organization user' do + before do + create(:organization_user, organization: organization, user: user) + end + + it 'does not add any validation errors' do + user_preference.home_organization = organization + + expect(user_preference).to be_valid + expect(user_preference.errors).to be_empty + end + end + + context 'when user is not an organization user' do + it 'adds a validation error' do + user_preference.home_organization = organization + + expect(user_preference).to be_invalid + expect(user_preference.errors.messages[:user].first).to eq(_("is not part of the given organization")) + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index fe229ce836f..cc0ea69401e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -95,6 +95,10 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to delegate_method(:achievements_enabled).to(:user_preference) } it { is_expected.to delegate_method(:achievements_enabled=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:home_organization).to(:user_preference) } + it { is_expected.to delegate_method(:home_organization_id).to(:user_preference) } + it { is_expected.to delegate_method(:home_organization_id=).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 } @@ -175,7 +179,6 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to have_many(:uploads) } it { is_expected.to have_many(:abuse_reports).dependent(:nullify).inverse_of(:user) } it { is_expected.to have_many(:reported_abuse_reports).dependent(:nullify).class_name('AbuseReport').inverse_of(:reporter) } - it { is_expected.to have_many(:assigned_abuse_reports).class_name('AbuseReport').inverse_of(:assignee) } it { is_expected.to have_many(:resolved_abuse_reports).class_name('AbuseReport').inverse_of(:resolved_by) } it { is_expected.to have_many(:abuse_events).class_name('Abuse::Event').inverse_of(:user) } it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') } @@ -199,6 +202,13 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to have_many(:abuse_trust_scores).class_name('Abuse::TrustScore') } it { is_expected.to have_many(:issue_assignment_events).class_name('ResourceEvents::IssueAssignmentEvent') } it { is_expected.to have_many(:merge_request_assignment_events).class_name('ResourceEvents::MergeRequestAssignmentEvent') } + it { is_expected.to have_many(:admin_abuse_report_assignees).class_name('Admin::AbuseReportAssignee') } + + it do + is_expected.to have_many(:assigned_abuse_reports).class_name('AbuseReport') + .through(:admin_abuse_report_assignees) + .source(:abuse_report) + end it do is_expected.to have_many(:organization_users).class_name('Organizations::OrganizationUser').inverse_of(:user) @@ -461,7 +471,7 @@ RSpec.describe User, feature_category: :user_profile do describe 'validations' do describe 'password' do - let!(:user) { build_stubbed(:user) } + let!(:user) { build(:user) } before do allow(Devise).to receive(:password_length).and_return(8..128) @@ -541,9 +551,7 @@ RSpec.describe User, feature_category: :user_profile do context 'namespace_move_dir_allowed' do context 'when the user is not a new record' do - before do - expect(user.new_record?).to eq(false) - end + let!(:user) { create(:user) } it 'checks when username changes' do expect(user).to receive(:namespace_move_dir_allowed) @@ -1437,6 +1445,25 @@ RSpec.describe User, feature_category: :user_profile do end end + describe '#user_belongs_to_organization?' do + let_it_be(:user) { create(:user) } + let_it_be(:organization) { create(:organization) } + + subject { user.user_belongs_to_organization?(organization) } + + context 'when user is an organization user' do + before do + create(:organization_user, organization: organization, user: user) + end + + it { is_expected.to eq true } + end + + context 'when user is not an organization user' do + it { is_expected.to eq false } + end + end + context 'strip attributes' do context 'name' do let(:user) { described_class.new(name: ' John Smith ') } @@ -3537,32 +3564,8 @@ RSpec.describe User, feature_category: :user_profile do end end - describe '#avatar_type' do - let(:user) { create(:user) } - - it 'is true if avatar is image' do - user.update_attribute(:avatar, 'uploads/avatar.png') - - expect(user.avatar_type).to be_truthy - end - - it 'is false if avatar is html page' do - user.update_attribute(:avatar, 'uploads/avatar.html') - user.avatar_type - - expect(user.errors.added?(:avatar, "file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp")).to be true - end - end - - describe '#avatar_url' do - let(:user) { create(:user, :with_avatar) } - - context 'when avatar file is uploaded' do - it 'shows correct avatar url' do - expect(user.avatar_url).to eq(user.avatar.url) - expect(user.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, user.avatar.url].join) - end - end + it_behaves_like Avatarable do + let(:model) { create(:user, :with_avatar) } end describe '#clear_avatar_caches' do @@ -5430,8 +5433,7 @@ RSpec.describe User, feature_category: :user_profile do before do shared_project.project_group_links.create!( - group: group2, - group_access: ProjectGroupLink.default_access + group: group2 ) group2.add_member(user, GroupMember::OWNER) @@ -5682,21 +5684,35 @@ RSpec.describe User, feature_category: :user_profile do describe '#ensure_namespace_correct' do context 'for a new user' do - let(:user) { build(:user) } + let(:user) { described_class.new attributes_for(:user) } - it 'creates the namespace' do + it 'does not create the namespace' do expect(user.namespace).to be_nil - user.save! + user.valid? - expect(user.namespace).not_to be_nil - expect(user.namespace).to be_kind_of(Namespaces::UserNamespace) + expect(user.namespace).to be_nil end - it 'creates the namespace setting' do - user.save! + context 'when create_personal_ns_outside_model feature flag is disabled' do + before do + stub_feature_flags(create_personal_ns_outside_model: false) + end - expect(user.namespace.namespace_settings).to be_persisted + it 'creates the namespace' do + expect(user.namespace).to be_nil + + user.save! + + expect(user.namespace).to be_present + expect(user.namespace).to be_kind_of(Namespaces::UserNamespace) + end + + it 'creates the namespace setting' do + user.save! + + expect(user.namespace.namespace_settings).to be_persisted + end end end @@ -5764,6 +5780,37 @@ RSpec.describe User, feature_category: :user_profile do end end + describe '#assign_personal_namespace' do + subject(:personal_namespace) { user.assign_personal_namespace } + + context 'when namespace exists' do + let(:user) { build(:user) } + + it 'leaves the namespace untouched' do + expect { personal_namespace }.not_to change(user, :namespace) + end + + it 'returns the personal namespace' do + expect(personal_namespace).to eq(user.namespace) + end + end + + context 'when namespace does not exist' do + let(:user) { described_class.new attributes_for(:user) } + + it 'builds a new namespace' do + subject + + expect(user.namespace).to be_kind_of(Namespaces::UserNamespace) + expect(user.namespace.namespace_settings).to be_present + end + + it 'returns the personal namespace' do + expect(personal_namespace).to eq(user.namespace) + end + end + end + describe '#username_changed_hook' do context 'for a new user' do let(:user) { build(:user) } @@ -6130,6 +6177,15 @@ RSpec.describe User, feature_category: :user_profile do end end + it 'adds a custom attribute that indicates the user deleted their own account' do + freeze_time do + expect { user.delete_async(deleted_by: deleted_by) }.to change { user.custom_attributes.count }.by(1) + + expect(user.custom_attributes.last.key).to eq UserCustomAttribute::DELETED_OWN_ACCOUNT_AT + expect(user.custom_attributes.last.value).to eq Time.zone.now.to_s + end + end + context 'when delay_delete_own_user feature flag is disabled' do before do stub_feature_flags(delay_delete_own_user: false) @@ -6142,6 +6198,10 @@ RSpec.describe User, feature_category: :user_profile do it 'does not update the note' do expect { user.delete_async(deleted_by: deleted_by) }.not_to change { user.note } end + + it 'does not add any new custom attrribute' do + expect { user.delete_async(deleted_by: deleted_by) }.not_to change { user.custom_attributes.count } + end end describe '#trusted?' do @@ -6330,6 +6390,34 @@ RSpec.describe User, feature_category: :user_profile do end end + describe '#max_member_access_for_group' do + let(:user) { create(:user) } + let(:group) { create(:group) } + + context 'when user has no access' do + it 'returns Gitlab::Access::NO_ACCESS' do + expect(user.max_member_access_for_group(group.id)).to eq(Gitlab::Access::NO_ACCESS) + end + end + + context 'when user has access via a single permission' do + it 'returns Gitlab::Access::DEVELOPER' do + group.add_developer(user) + + expect(user.max_member_access_for_group(group.id)).to eq(Gitlab::Access::DEVELOPER) + end + end + + context 'when user has access via a multiple groups' do + it 'returns Gitlab::Access::MAINTAINER' do + group.add_developer(user) + group.add_maintainer(user) + + expect(user.max_member_access_for_group(group.id)).to eq(Gitlab::Access::MAINTAINER) + end + end + end + context 'changing a username' do let(:user) { create(:user, username: 'foo') } @@ -6381,6 +6469,49 @@ RSpec.describe User, feature_category: :user_profile do end end end + + context 'with multiple versions of terms' do + shared_examples 'terms acceptance' do + let(:another_term) { create :term } + let(:required_terms_are_accepted) { !required_terms_not_accepted } + + context 'when the latest term is not accepted' do + before do + accept_terms(user) + another_term + end + + it { expect(required_terms_are_accepted).to be result_for_latest_not_accepted } + end + + context 'when the latest term is accepted' do + before do + another_term + accept_terms(user) + end + + it { expect(required_terms_are_accepted).to be result_for_latest_accepted } + end + end + + context 'when enforce_acceptance_of_changed_terms is enabled' do + let(:result_for_latest_not_accepted) { false } + let(:result_for_latest_accepted) { true } + + include_examples 'terms acceptance' + end + + context 'when enforce_acceptance_of_changed_terms is disabled' do + let(:result_for_latest_not_accepted) { true } + let(:result_for_latest_accepted) { true } + + before do + stub_feature_flags(enforce_acceptance_of_changed_terms: false) + end + + include_examples 'terms acceptance' + end + end end end @@ -7966,4 +8097,24 @@ RSpec.describe User, feature_category: :user_profile do end end end + + describe '#deleted_own_account?' do + let_it_be(:user) { create(:user) } + + subject(:result) { user.deleted_own_account? } + + context 'when user has a DELETED_OWN_ACCOUNT_AT custom attribute' do + let_it_be(:custom_attr) do + create(:user_custom_attribute, user: user, key: UserCustomAttribute::DELETED_OWN_ACCOUNT_AT, value: 'now') + end + + it { is_expected.to eq true } + end + + context 'when user does not have a DELETED_OWN_ACCOUNT_AT custom attribute' do + let_it_be(:user) { create(:user) } + + it { is_expected.to eq false } + end + end end diff --git a/spec/models/users/in_product_marketing_email_spec.rb b/spec/models/users/in_product_marketing_email_spec.rb index d333a51ae3b..b1642383e42 100644 --- a/spec/models/users/in_product_marketing_email_spec.rb +++ b/spec/models/users/in_product_marketing_email_spec.rb @@ -134,15 +134,4 @@ RSpec.describe Users::InProductMarketingEmail, type: :model, feature_category: : 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/users/phone_number_validation_spec.rb b/spec/models/users/phone_number_validation_spec.rb index e41719d8ca3..15bbb507dee 100644 --- a/spec/models/users/phone_number_validation_spec.rb +++ b/spec/models/users/phone_number_validation_spec.rb @@ -39,16 +39,26 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie end context 'when banned user has the same international dial code and phone number' do - before do - create(:phone_number_validation, user: banned_user) + context 'and the matching record has not been verified' do + before do + create(:phone_number_validation, user: banned_user) + end + + it { is_expected.to eq(false) } end - it { is_expected.to eq(true) } + context 'and the matching record has been verified' do + before do + create(:phone_number_validation, :validated, user: banned_user) + end + + it { is_expected.to eq(true) } + end end context 'when banned user has the same international dial code and phone number, but different country code' do before do - create(:phone_number_validation, user: banned_user, country: 'CA') + create(:phone_number_validation, :validated, user: banned_user, country: 'CA') end it { is_expected.to eq(true) } @@ -56,7 +66,7 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie context 'when banned user does not have the same international dial code' do before do - create(:phone_number_validation, user: banned_user, international_dial_code: 61) + create(:phone_number_validation, :validated, user: banned_user, international_dial_code: 61) end it { is_expected.to eq(false) } @@ -64,7 +74,7 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie context 'when banned user does not have the same phone number' do before do - create(:phone_number_validation, user: banned_user, phone_number: '666') + create(:phone_number_validation, :validated, user: banned_user, phone_number: '666') end it { is_expected.to eq(false) } @@ -72,7 +82,7 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie context 'when not-banned user has the same international dial code and phone number' do before do - create(:phone_number_validation, user: user) + create(:phone_number_validation, :validated, user: user) end it { is_expected.to eq(false) } diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index 476d346db10..68e9e8ee50d 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -17,6 +17,13 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do .with_foreign_key('work_item_id') end + it 'has one `dates_source`' do + is_expected.to have_one(:dates_source) + .class_name('WorkItems::DatesSource') + .with_foreign_key('issue_id') + .inverse_of(:work_item) + end + it 'has many `work_item_children`' do is_expected.to have_many(:work_item_children) .class_name('WorkItem') @@ -79,16 +86,6 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do end end - describe '.in_namespaces' do - let(:group) { create(:group) } - let!(:group_work_item) { create(:work_item, namespace: group) } - let!(:project_work_item) { create(:work_item, project: reusable_project) } - - subject { described_class.in_namespaces(group) } - - it { is_expected.to contain_exactly(group_work_item) } - end - describe '.with_confidentiality_check' do let(:user) { create(:user) } let!(:authored_work_item) { create(:work_item, :confidential, project: reusable_project, author: user) } diff --git a/spec/models/work_items/dates_source_spec.rb b/spec/models/work_items/dates_source_spec.rb new file mode 100644 index 00000000000..d75500cab16 --- /dev/null +++ b/spec/models/work_items/dates_source_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::DatesSource, feature_category: :portfolio_management do + describe 'ssociations' do + it { is_expected.to belong_to(:namespace).inverse_of(:work_items_dates_source) } + it { is_expected.to belong_to(:work_item).with_foreign_key('issue_id').inverse_of(:dates_source) } + it { is_expected.to belong_to(:due_date_sourcing_work_item).class_name('WorkItem') } + it { is_expected.to belong_to(:start_date_sourcing_work_item).class_name('WorkItem') } + it { is_expected.to belong_to(:due_date_sourcing_milestone).class_name('Milestone') } + it { is_expected.to belong_to(:start_date_sourcing_milestone).class_name('Milestone') } + end + + it 'ensures to use work_item namespace' do + work_item = create(:work_item) + date_source = described_class.new(work_item: work_item) + + date_source.valid? + + expect(date_source.namespace).to eq(work_item.namespace) + end +end diff --git a/spec/models/work_items/type_spec.rb b/spec/models/work_items/type_spec.rb index 7f836ce4e90..135b7b97ce9 100644 --- a/spec/models/work_items/type_spec.rb +++ b/spec/models/work_items/type_spec.rb @@ -24,6 +24,43 @@ RSpec.describe WorkItems::Type, feature_category: :team_planning do expect(type.enabled_widget_definitions).to match_array([widget1]) end + + it 'has many `child_restrictions`' do + is_expected.to have_many(:child_restrictions) + .class_name('WorkItems::HierarchyRestriction') + .with_foreign_key('parent_type_id') + end + + describe 'allowed_child_types_by_name' do + it 'defines association' do + is_expected.to have_many(:allowed_child_types_by_name) + .through(:child_restrictions) + .class_name('::WorkItems::Type') + .with_foreign_key(:child_type_id) + end + + it 'sorts by name ascending' do + expected_type_names = %w[Atype Ztype gtype] + parent_type = create(:work_item_type) + + expected_type_names.shuffle.each do |name| + create(:hierarchy_restriction, parent_type: parent_type, child_type: create(:work_item_type, name: name)) + end + + expect(parent_type.allowed_child_types_by_name.pluck(:name)).to match_array(expected_type_names) + end + end + end + + describe 'callbacks' do + describe 'after_save' do + subject(:work_item_type) { build(:work_item_type) } + + it 'calls #clear_reactive_cache!' do + is_expected.to receive(:clear_reactive_cache!) + work_item_type.save!(name: 'foo') + end + end end describe 'scopes' do @@ -166,4 +203,48 @@ RSpec.describe WorkItems::Type, feature_category: :team_planning do end end end + + describe '#allowed_child_types' do + let_it_be(:work_item_type) { create(:work_item_type) } + let_it_be(:child_type) { create(:work_item_type) } + let_it_be(:restriction) { create(:hierarchy_restriction, parent_type: work_item_type, child_type: child_type) } + + subject { work_item_type.allowed_child_types(cache: cached) } + + context 'when cache is true' do + let(:cached) { true } + + before do + allow(work_item_type).to receive(:with_reactive_cache).and_call_original + end + + it 'returns the cached data' do + expect(work_item_type).to receive(:with_reactive_cache) + expect(Rails.cache).to receive(:exist?).with("work_items_type:#{work_item_type.id}:alive") + is_expected.to eq([child_type]) + end + end + + context 'when cache is false' do + let(:cached) { false } + + it 'returns queried data' do + expect(work_item_type).not_to receive(:with_reactive_cache) + is_expected.to eq([child_type]) + end + end + end + + describe '#calculate_reactive_cache' do + let(:work_item_type) { build(:work_item_type) } + + subject { work_item_type.calculate_reactive_cache } + + it 'returns cache data for allowed child types' do + child_types = create_list(:work_item_type, 2) + expect(work_item_type).to receive(:allowed_child_types_by_name).and_return(child_types) + + is_expected.to eq(child_types) + end + end end diff --git a/spec/models/work_items/widgets/assignees_spec.rb b/spec/models/work_items/widgets/assignees_spec.rb index 19c17658ce4..b44246855ce 100644 --- a/spec/models/work_items/widgets/assignees_spec.rb +++ b/spec/models/work_items/widgets/assignees_spec.rb @@ -17,6 +17,32 @@ RSpec.describe WorkItems::Widgets::Assignees do it { is_expected.to include(:assignee_ids) } end + describe '.can_invite_members?' do + let(:user) { build_stubbed(:user) } + + subject(:execute) { described_class.can_invite_members?(user, resource_parent) } + + context 'when resource_parent is a project' do + let(:resource_parent) { build_stubbed(:project) } + + it 'checks the ability with the correct permission' do + expect(user).to receive(:can?).with(:admin_project_member, resource_parent) + + execute + end + end + + context 'when resource_parent is a group' do + let(:resource_parent) { build_stubbed(:group) } + + it 'checks the ability with the correct permission' do + expect(user).to receive(:can?).with(:admin_group_member, resource_parent) + + execute + end + end + end + describe '#type' do subject { described_class.new(work_item).type } |