diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-10-19 15:57:54 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-10-19 15:57:54 +0300 |
commit | 419c53ec62de6e97a517abd5fdd4cbde3a942a34 (patch) | |
tree | 1f43a548b46bca8a5fb8fe0c31cef1883d49c5b6 /spec/models | |
parent | 1da20d9135b3ad9e75e65b028bffc921aaf8deb7 (diff) |
Add latest changes from gitlab-org/gitlab@16-5-stable-eev16.5.0-rc42
Diffstat (limited to 'spec/models')
92 files changed, 2490 insertions, 1589 deletions
diff --git a/spec/models/abuse/reports/user_mention_spec.rb b/spec/models/abuse/reports/user_mention_spec.rb new file mode 100644 index 00000000000..c5048134382 --- /dev/null +++ b/spec/models/abuse/reports/user_mention_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Abuse::Reports::UserMention, feature_category: :insider_threat do + describe 'associations' do + it { is_expected.to belong_to(:abuse_report).optional(false) } + it { is_expected.to belong_to(:note).optional(false) } + end + + it_behaves_like 'has user mentions' +end diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 1fa60a210e2..6500e5fac02 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -18,6 +18,8 @@ RSpec.describe AbuseReport, feature_category: :insider_threat do 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 "aliases reporter to author" do expect(subject.author).to be(subject.reporter) @@ -263,7 +265,7 @@ RSpec.describe AbuseReport, feature_category: :insider_threat do let_it_be(:merge_request) { create(:merge_request) } let_it_be(:user) { create(:user) } - subject { report.report_type } + subject(:report_type) { report.report_type } context 'when reported from an issue' do let(:url) { project_issue_url(issue.project, issue) } @@ -322,7 +324,7 @@ RSpec.describe AbuseReport, feature_category: :insider_threat do let_it_be(:merge_request) { create(:merge_request, description: 'mr description') } let_it_be(:user) { create(:user) } - subject { report.reported_content } + subject(:reported_content) { report.reported_content } context 'when reported from an issue' do let(:url) { project_issue_url(issue.project, issue) } diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 3fc7d8f6fc8..78bf410075b 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -69,8 +69,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to allow_value("dev.gitlab.com").for(:commit_email_hostname) } it { is_expected.not_to allow_value("@dev.gitlab").for(:commit_email_hostname) } - it { is_expected.to allow_value(true, false).for(:container_expiration_policies_enable_historic_entries) } - it { is_expected.not_to allow_value(nil).for(:container_expiration_policies_enable_historic_entries) } + it { is_expected.to validate_inclusion_of(:container_expiration_policies_enable_historic_entries).in_array([true, false]) } it { is_expected.to allow_value("myemail@gitlab.com").for(:lets_encrypt_notification_email) } it { is_expected.to allow_value(nil).for(:lets_encrypt_notification_email) } @@ -113,7 +112,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_numericality_of(:container_registry_cleanup_tags_service_max_list_size).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_data_repair_detail_worker_max_concurrency).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_expiration_policies_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to allow_value(true, false).for(:container_registry_expiration_policies_caching) } + it { is_expected.to validate_inclusion_of(:container_registry_expiration_policies_caching).in_array([true, false]) } it { is_expected.to validate_numericality_of(:container_registry_import_max_tags_count).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_import_max_retries).only_integer.is_greater_than_or_equal_to(0) } @@ -149,8 +148,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_numericality_of(:snippet_size_limit).only_integer.is_greater_than(0) } it { is_expected.to validate_numericality_of(:wiki_page_max_content_bytes).only_integer.is_greater_than_or_equal_to(1024) } - it { is_expected.to allow_value(true, false).for(:wiki_asciidoc_allow_uri_includes) } - it { is_expected.not_to allow_value(nil).for(:wiki_asciidoc_allow_uri_includes) } + it { is_expected.to validate_inclusion_of(:wiki_asciidoc_allow_uri_includes).in_array([true, false]) } it { is_expected.to validate_presence_of(:max_artifacts_size) } it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } it { is_expected.to validate_presence_of(:max_yaml_size_bytes) } @@ -162,11 +160,9 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_presence_of(:max_terraform_state_size_bytes) } it { is_expected.to validate_numericality_of(:max_terraform_state_size_bytes).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to allow_value(true, false).for(:user_defaults_to_private_profile) } - it { is_expected.not_to allow_value(nil).for(:user_defaults_to_private_profile) } + it { is_expected.to validate_inclusion_of(:user_defaults_to_private_profile).in_array([true, false]) } - it { is_expected.to allow_values([true, false]).for(:deny_all_requests_except_allowed) } - it { is_expected.not_to allow_value(nil).for(:deny_all_requests_except_allowed) } + it { is_expected.to validate_inclusion_of(:deny_all_requests_except_allowed).in_array([true, false]) } it 'ensures max_pages_size is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do is_expected.to validate_numericality_of(:max_pages_size).only_integer.is_greater_than_or_equal_to(0) @@ -254,8 +250,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to allow_value('http://example.com/').for(:public_runner_releases_url) } it { is_expected.not_to allow_value(nil).for(:public_runner_releases_url) } - it { is_expected.to allow_value([true, false]).for(:update_runner_versions_enabled) } - it { is_expected.not_to allow_value(nil).for(:update_runner_versions_enabled) } + it { is_expected.to validate_inclusion_of(:update_runner_versions_enabled).in_array([true, false]) } it { is_expected.not_to allow_value(['']).for(:valid_runner_registrars) } it { is_expected.not_to allow_value(['OBVIOUSLY_WRONG']).for(:valid_runner_registrars) } @@ -268,21 +263,17 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to allow_value(http).for(:jira_connect_proxy_url) } it { is_expected.to allow_value(https).for(:jira_connect_proxy_url) } - it { is_expected.to allow_value(true, false).for(:bulk_import_enabled) } - it { is_expected.not_to allow_value(nil).for(:bulk_import_enabled) } + it { is_expected.to validate_inclusion_of(:bulk_import_enabled).in_array([true, false]) } - it { is_expected.to allow_value(true, false).for(:allow_runner_registration_token) } - it { is_expected.not_to allow_value(nil).for(:allow_runner_registration_token) } + it { is_expected.to validate_inclusion_of(:allow_runner_registration_token).in_array([true, false]) } - it { is_expected.to allow_value(true, false).for(:gitlab_dedicated_instance) } - it { is_expected.not_to allow_value(nil).for(:gitlab_dedicated_instance) } + it { is_expected.to validate_inclusion_of(:gitlab_dedicated_instance).in_array([true, false]) } it { is_expected.not_to allow_value(apdex_slo: '10').for(:prometheus_alert_db_indicators_settings) } it { is_expected.to allow_value(nil).for(:prometheus_alert_db_indicators_settings) } it { is_expected.to allow_value(valid_prometheus_alert_db_indicators_settings).for(:prometheus_alert_db_indicators_settings) } - it { is_expected.to allow_value([true, false]).for(:silent_mode_enabled) } - it { is_expected.not_to allow_value(nil).for(:silent_mode_enabled) } + it { is_expected.to validate_inclusion_of(:silent_mode_enabled).in_array([true, false]) } it { is_expected.to allow_value(0).for(:ci_max_includes) } it { is_expected.to allow_value(200).for(:ci_max_includes) } @@ -298,16 +289,16 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.not_to allow_value(10.5).for(:ci_max_total_yaml_size_bytes) } it { is_expected.not_to allow_value(-1).for(:ci_max_total_yaml_size_bytes) } - it { is_expected.to allow_value([true, false]).for(:remember_me_enabled) } - it { is_expected.not_to allow_value(nil).for(:remember_me_enabled) } + it { is_expected.to validate_inclusion_of(:remember_me_enabled).in_array([true, false]) } it { is_expected.to validate_numericality_of(:namespace_aggregation_schedule_lease_duration_in_seconds).only_integer.is_greater_than(0) } - it { is_expected.to allow_values([true, false]).for(:instance_level_code_suggestions_enabled) } - it { is_expected.not_to allow_value(nil).for(:instance_level_code_suggestions_enabled) } + it { is_expected.to validate_inclusion_of(:instance_level_code_suggestions_enabled).in_array([true, false]) } - it { is_expected.to allow_values([true, false]).for(:package_registry_allow_anyone_to_pull_option) } - it { is_expected.not_to allow_value(nil).for(:package_registry_allow_anyone_to_pull_option) } + it { is_expected.to validate_inclusion_of(:package_registry_allow_anyone_to_pull_option).in_array([true, false]) } + + it { is_expected.to allow_value([true, false]).for(:math_rendering_limits_enabled) } + it { is_expected.not_to allow_value(nil).for(:math_rendering_limits_enabled) } context 'when deactivate_dormant_users is enabled' do before do @@ -639,6 +630,18 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do end specify do + is_expected.to validate_numericality_of(:failed_login_attempts_unlock_period_in_minutes) + .only_integer + .is_greater_than(0) + end + + specify do + is_expected.to validate_numericality_of(:max_login_attempts) + .only_integer + .is_greater_than(0) + end + + specify do is_expected.to validate_numericality_of(:local_markdown_version) .only_integer .is_greater_than_or_equal_to(0) diff --git a/spec/models/approval_spec.rb b/spec/models/approval_spec.rb index 3d382c1712a..ff2f7408941 100644 --- a/spec/models/approval_spec.rb +++ b/spec/models/approval_spec.rb @@ -13,4 +13,16 @@ RSpec.describe Approval, feature_category: :code_review_workflow do it { is_expected.to validate_uniqueness_of(:user_id).scoped_to([:merge_request_id]) } end + + describe '.with_invalid_patch_id_sha' do + let(:patch_id_sha) { 'def456' } + let!(:approval_1) { create(:approval, patch_id_sha: 'abc123') } + let!(:approval_2) { create(:approval, patch_id_sha: nil) } + let!(:approval_3) { create(:approval, patch_id_sha: patch_id_sha) } + + it 'returns approvals with patch_id_sha not matching specified patch_id_sha' do + expect(described_class.with_invalid_patch_id_sha(patch_id_sha)) + .to match_array([approval_1, approval_2]) + end + end end diff --git a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb index 36b75e5338a..b6321b9aaf3 100644 --- a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb +++ b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb @@ -55,7 +55,8 @@ RSpec.describe BlobViewer::GitlabCiYml, feature_category: :source_code_managemen context 'when a project ref does not contain the sha' do it 'returns an error' do - expect(validation_message).to match(/Could not validate configuration/) + expect(validation_message).to match( + /configuration originates from an external project or a commit not associated with a Git reference/) end end end diff --git a/spec/models/bulk_import_spec.rb b/spec/models/bulk_import_spec.rb index a50fc6eaba4..ff24f57f7c4 100644 --- a/spec/models/bulk_import_spec.rb +++ b/spec/models/bulk_import_spec.rb @@ -40,6 +40,14 @@ RSpec.describe BulkImport, type: :model, feature_category: :importers do it { expect(described_class.min_gl_version_for_project_migration.to_s).to eq('14.4.0') } end + describe '#completed?' do + it { expect(described_class.new(status: -1)).to be_completed } + it { expect(described_class.new(status: 0)).not_to be_completed } + it { expect(described_class.new(status: 1)).not_to be_completed } + it { expect(described_class.new(status: 2)).to be_completed } + it { expect(described_class.new(status: 3)).to be_completed } + end + describe '#source_version_info' do it 'returns source_version as Gitlab::VersionInfo' do bulk_import = build(:bulk_import, source_version: '9.13.2') diff --git a/spec/models/bulk_imports/tracker_spec.rb b/spec/models/bulk_imports/tracker_spec.rb index a618a12df6b..edd9adfa5f6 100644 --- a/spec/models/bulk_imports/tracker_spec.rb +++ b/spec/models/bulk_imports/tracker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::Tracker, type: :model do +RSpec.describe BulkImports::Tracker, type: :model, feature_category: :importers do describe 'associations' do it do is_expected.to belong_to(:entity).required.class_name('BulkImports::Entity') @@ -30,19 +30,14 @@ RSpec.describe BulkImports::Tracker, type: :model do end end - describe '.stage_running?' do - it 'returns true if there is any unfinished pipeline in the given stage' do - tracker = create(:bulk_import_tracker) - - expect(described_class.stage_running?(tracker.entity.id, 0)) - .to eq(true) - end - - it 'returns false if there are no unfinished pipeline in the given stage' do - tracker = create(:bulk_import_tracker, :finished) + describe '.running_trackers' do + it 'returns trackers that are running for a given entity' do + entity = create(:bulk_import_entity) + BulkImports::Tracker.state_machines[:status].states.map(&:value).each do |status| + create(:bulk_import_tracker, status: status, entity: entity) + end - expect(described_class.stage_running?(tracker.entity.id, 0)) - .to eq(false) + expect(described_class.running_trackers(entity.id).pluck(:status)).to include(1, 3) end end diff --git a/spec/models/chat_name_spec.rb b/spec/models/chat_name_spec.rb index 9d6b1a56458..dbe013f3872 100644 --- a/spec/models/chat_name_spec.rb +++ b/spec/models/chat_name_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe ChatName, feature_category: :integrations do - let_it_be(:chat_name) { create(:chat_name) } + let_it_be_with_reload(:chat_name) { create(:chat_name) } subject { chat_name } @@ -33,6 +33,22 @@ RSpec.describe ChatName, feature_category: :integrations do expect(subject.last_used_at).to eq(time) end + + it 'updates last_used_at if it was not recently updated' do + allow_next_instance_of(Gitlab::ExclusiveLease) do |lease| + allow(lease).to receive(:try_obtain).and_return('successful_lease_guid') + end + + subject.update_last_used_at + + new_time = ChatName::LAST_USED_AT_INTERVAL.from_now + 5.minutes + + travel_to(new_time) do + subject.update_last_used_at + end + + expect(subject.last_used_at).to be_like_time(new_time) + end end it_behaves_like 'it has loose foreign keys' do diff --git a/spec/models/ci/build_need_spec.rb b/spec/models/ci/build_need_spec.rb index e46a2b8cf85..4f76a7650ec 100644 --- a/spec/models/ci/build_need_spec.rb +++ b/spec/models/ci/build_need_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Ci::BuildNeed, model: true, feature_category: :continuous_integra it { is_expected.to validate_presence_of(:build) } it { is_expected.to validate_presence_of(:name) } - it { is_expected.to validate_length_of(:name).is_at_most(128) } + 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) } diff --git a/spec/models/ci/catalog/components_project_spec.rb b/spec/models/ci/catalog/components_project_spec.rb new file mode 100644 index 00000000000..d7e0ee2079c --- /dev/null +++ b/spec/models/ci/catalog/components_project_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::Catalog::ComponentsProject, feature_category: :pipeline_composition do + using RSpec::Parameterized::TableSyntax + + let_it_be(:files) do + { + 'templates/secret-detection.yml' => "spec:\n inputs:\n website:\n---\nimage: alpine_1", + 'templates/dast/template.yml' => 'image: alpine_2', + 'templates/template.yml' => 'image: alpine_3', + 'templates/blank-yaml.yml' => '', + 'templates/dast/sub-folder/template.yml' => 'image: alpine_4', + 'tests/test.yml' => 'image: alpine_5', + 'README.md' => 'Read me' + } + end + + let_it_be(:project) do + create( + :project, :custom_repo, + description: 'Simple, complex, and other components', + files: files + ) + end + + let_it_be(:catalog_resource) { create(:ci_catalog_resource, project: project) } + + let(:components_project) { described_class.new(project, project.default_branch) } + + describe '#fetch_component_paths' do + it 'retrieves all the paths for valid components' do + paths = components_project.fetch_component_paths(project.default_branch) + + expect(paths).to contain_exactly( + 'templates/blank-yaml.yml', 'templates/dast/template.yml', 'templates/secret-detection.yml', + 'templates/template.yml' + ) + end + end + + describe '#extract_component_name' do + context 'with invalid component path' do + it 'raises an error' do + expect(components_project.extract_component_name('not-template/this-is-wrong.yml')).to be_nil + end + end + + context 'with valid component paths' do + where(:path, :name) do + 'templates/secret-detection.yml' | 'secret-detection' + 'templates/dast/template.yml' | 'dast' + 'templates/template.yml' | 'template' + 'templates/blank-yaml.yml' | 'blank-yaml' + end + + with_them do + it 'extracts the component name from the path' do + expect(components_project.extract_component_name(path)).to eq(name) + end + end + end + end + + describe '#extract_inputs' do + context 'with valid inputs' do + it 'extracts the inputs from a blob' do + blob = "spec:\n inputs:\n website:\n---\nimage: alpine_1" + + expect(components_project.extract_inputs(blob)).to eq({ website: nil }) + end + end + + context 'with invalid inputs' do + it 'raises InvalidFormatError' do + blob = "spec:\n inputs:\n website:\n---\nsome: invalid: string" + + expect do + components_project.extract_inputs(blob) + end.to raise_error(::Gitlab::Config::Loader::FormatError, + /mapping values are not allowed in this context/) + end + end + end + + describe '#fetch_component' do + where(:component_name, :content, :path) do + 'secret-detection' | "spec:\n inputs:\n website:\n---\nimage: alpine_1" | 'templates/secret-detection.yml' + 'dast' | 'image: alpine_2' | 'templates/dast/template.yml' + 'template' | 'image: alpine_3' | 'templates/template.yml' + 'blank-yaml' | '' | 'templates/blank-yaml.yml' + end + + with_them do + it 'fetches the content for a component' do + data = components_project.fetch_component(component_name) + + expect(data.path).to eq(path) + expect(data.content).to eq(content) + end + end + end +end diff --git a/spec/models/ci/catalog/listing_spec.rb b/spec/models/ci/catalog/listing_spec.rb index f28a0e82bbd..7524d908252 100644 --- a/spec/models/ci/catalog/listing_spec.rb +++ b/spec/models/ci/catalog/listing_spec.rb @@ -6,7 +6,8 @@ RSpec.describe Ci::Catalog::Listing, feature_category: :pipeline_composition do let_it_be(:namespace) { create(:group) } let_it_be(:project_1) { create(:project, namespace: namespace, name: 'X Project') } let_it_be(:project_2) { create(:project, namespace: namespace, name: 'B Project') } - let_it_be(:project_3) { create(:project) } + let_it_be(:project_3) { create(:project, namespace: namespace, name: 'A Project') } + let_it_be(:project_4) { create(:project) } let_it_be(:user) { create(:user) } let(:list) { described_class.new(namespace, user) } @@ -34,12 +35,20 @@ RSpec.describe Ci::Catalog::Listing, feature_category: :pipeline_composition do end context 'when the namespace has catalog resources' do - let_it_be(:resource) { create(:ci_catalog_resource, project: project_1) } - let_it_be(:resource_2) { create(:ci_catalog_resource, project: project_2) } - let_it_be(:other_namespace_resource) { create(:ci_catalog_resource, project: project_3) } + 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) { create(:ci_catalog_resource, project: project_1, latest_released_at: yesterday) } + 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(:other_namespace_resource) do + create(:ci_catalog_resource, project: project_4, latest_released_at: tomorrow) + end it 'contains only catalog resources for projects in that namespace' do - is_expected.to contain_exactly(resource, resource_2) + is_expected.to contain_exactly(resource, resource_2, resource_3) end context 'with a sort parameter' do @@ -48,16 +57,32 @@ RSpec.describe Ci::Catalog::Listing, feature_category: :pipeline_composition do context 'when the sort is name ascending' do let_it_be(:sort) { :name_asc } - it 'contains catalog resources for projects sorted by name' do - is_expected.to eq([resource_2, resource]) + it 'contains catalog resources for projects sorted by name ascending' do + is_expected.to eq([resource_3, resource_2, 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' do - is_expected.to eq([resource, resource_2]) + it 'contains catalog resources for projects sorted by name descending' do + is_expected.to eq([resource, resource_2, resource_3]) + end + end + + 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([resource, resource_2, resource_3]) + 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([resource_2, resource, resource_3]) end end end diff --git a/spec/models/ci/catalog/resource_spec.rb b/spec/models/ci/catalog/resource_spec.rb index 082283bb7bc..4ce1433e015 100644 --- a/spec/models/ci/catalog/resource_spec.rb +++ b/spec/models/ci/catalog/resource_spec.rb @@ -3,16 +3,20 @@ 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(:project) { create(:project, name: 'A') } let_it_be(:project_2) { build(:project, name: 'Z') } let_it_be(:project_3) { build(:project, name: 'L') } - let_it_be(:resource) { create(:ci_catalog_resource, project: project) } - let_it_be(:resource_2) { create(:ci_catalog_resource, project: project_2) } - let_it_be(:resource_3) { create(:ci_catalog_resource, project: project_3) } + let_it_be(: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(:release1) { create(:release, project: project, released_at: Time.zone.now - 2.days) } - let_it_be(:release2) { create(:release, project: project, released_at: Time.zone.now - 1.day) } - let_it_be(:release3) { create(:release, project: project, released_at: Time.zone.now) } + 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) } it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:components).class_name('Ci::Catalog::Resources::Component') } @@ -58,6 +62,22 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do end end + describe '.order_by_latest_released_at_desc' 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]) + end + end + + describe '.order_by_latest_released_at_asc' 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]) + end + end + describe '#versions' do it 'returns releases ordered by released date descending' do expect(resource.versions).to eq([release3, release2, release1]) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 7e572e2fdc6..887ec48ec8f 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2925,7 +2925,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: let(:pipeline) { create(:ci_pipeline, :created) } it 'returns detailed status for created pipeline' do - expect(subject.text).to eq s_('CiStatusText|created') + expect(subject.text).to eq s_('CiStatusText|Created') end end @@ -2933,7 +2933,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: let(:pipeline) { create(:ci_pipeline, status: :pending) } it 'returns detailed status for pending pipeline' do - expect(subject.text).to eq s_('CiStatusText|pending') + expect(subject.text).to eq s_('CiStatusText|Pending') end end @@ -2941,7 +2941,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: let(:pipeline) { create(:ci_pipeline, status: :running) } it 'returns detailed status for running pipeline' do - expect(subject.text).to eq s_('CiStatus|running') + expect(subject.text).to eq s_('CiStatusText|Running') end end @@ -2949,7 +2949,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: let(:pipeline) { create(:ci_pipeline, status: :success) } it 'returns detailed status for successful pipeline' do - expect(subject.text).to eq s_('CiStatusText|passed') + expect(subject.text).to eq s_('CiStatusText|Passed') end end @@ -2957,7 +2957,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: let(:pipeline) { create(:ci_pipeline, status: :failed) } it 'returns detailed status for failed pipeline' do - expect(subject.text).to eq s_('CiStatusText|failed') + expect(subject.text).to eq s_('CiStatusText|Failed') end end @@ -2965,7 +2965,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: let(:pipeline) { create(:ci_pipeline, status: :canceled) } it 'returns detailed status for canceled pipeline' do - expect(subject.text).to eq s_('CiStatusText|canceled') + expect(subject.text).to eq s_('CiStatusText|Canceled') end end @@ -2973,7 +2973,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: let(:pipeline) { create(:ci_pipeline, status: :skipped) } it 'returns detailed status for skipped pipeline' do - expect(subject.text).to eq s_('CiStatusText|skipped') + expect(subject.text).to eq s_('CiStatusText|Skipped') end end @@ -2981,7 +2981,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: let(:pipeline) { create(:ci_pipeline, status: :manual) } it 'returns detailed status for blocked pipeline' do - expect(subject.text).to eq s_('CiStatusText|blocked') + expect(subject.text).to eq s_('CiStatusText|Blocked') end end @@ -3250,22 +3250,23 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: shared_examples 'a method that returns all merge requests for a given pipeline' do let(:pipeline) { create(:ci_empty_pipeline, status: 'created', project: pipeline_project, ref: 'master') } + let(:merge_request) do + create( + :merge_request, + source_project: pipeline_project, + target_project: project, + source_branch: pipeline.ref + ) + end it 'returns all merge requests having the same source branch and the pipeline sha' do - merge_request = create(:merge_request, source_project: pipeline_project, target_project: project, source_branch: pipeline.ref) - - create(:merge_request_diff, merge_request: merge_request).tap do |diff| - create(:merge_request_diff_commit, merge_request_diff: diff, sha: pipeline.sha) - end + create(:merge_request_diff_commit, merge_request_diff: merge_request.merge_request_diff, sha: pipeline.sha) expect(pipeline.all_merge_requests).to eq([merge_request]) end it "doesn't return merge requests having the same source branch without the pipeline sha" do - merge_request = create(:merge_request, source_project: pipeline_project, target_project: project, source_branch: pipeline.ref) - create(:merge_request_diff, merge_request: merge_request).tap do |diff| - create(:merge_request_diff_commit, merge_request_diff: diff, sha: 'unrelated') - end + create(:merge_request_diff_commit, merge_request_diff: merge_request.merge_request_diff, sha: 'unrelated') expect(pipeline.all_merge_requests).to be_empty end @@ -5577,4 +5578,25 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: end end end + + describe '#reduced_build_attributes_list_for_rules?' do + subject { pipeline.reduced_build_attributes_list_for_rules? } + + let(:pipeline) { build_stubbed(:ci_pipeline, project: project, user: user) } + + it { is_expected.to be_truthy } + + it 'memoizes the result' do + expect { subject } + .to change { pipeline.strong_memoized?(:reduced_build_attributes_list_for_rules?) } + end + + context 'with the FF disabled' do + before do + stub_feature_flags(reduced_build_attributes_list_for_rules: false) + end + + it { is_expected.to be_falsey } + end + end end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index c6af7609778..8c0143d5f18 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -93,7 +93,7 @@ RSpec.describe Ci::Processable, feature_category: :continuous_integration do pipeline_id report_results pending_state pages_deployments queuing_entry runtime_metadata trace_metadata dast_site_profile dast_scanner_profile stage_id dast_site_profiles_build - dast_scanner_profiles_build].freeze + dast_scanner_profiles_build auto_canceled_by_partition_id].freeze end before_all do diff --git a/spec/models/ci/ref_spec.rb b/spec/models/ci/ref_spec.rb index a60aed98a21..75071a17fa9 100644 --- a/spec/models/ci/ref_spec.rb +++ b/spec/models/ci/ref_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Ref do +RSpec.describe Ci::Ref, feature_category: :continuous_integration do using RSpec::Parameterized::TableSyntax it { is_expected.to belong_to(:project) } @@ -10,13 +10,13 @@ RSpec.describe Ci::Ref do describe 'state machine transitions' do context 'unlock artifacts transition' do let(:ci_ref) { create(:ci_ref) } - let(:unlock_artifacts_worker_spy) { class_spy(::Ci::PipelineSuccessUnlockArtifactsWorker) } + let(:unlock_previous_pipelines_worker_spy) { class_spy(::Ci::Refs::UnlockPreviousPipelinesWorker) } before do - stub_const('Ci::PipelineSuccessUnlockArtifactsWorker', unlock_artifacts_worker_spy) + stub_const('Ci::Refs::UnlockPreviousPipelinesWorker', unlock_previous_pipelines_worker_spy) end - context 'pipline is locked' do + context 'pipeline is locked' do let!(:pipeline) { create(:ci_pipeline, ci_ref_id: ci_ref.id, locked: :artifacts_locked) } where(:initial_state, :action, :count) do @@ -41,10 +41,10 @@ RSpec.describe Ci::Ref do ci_ref.update!(status: status_value) end - it 'calls unlock artifacts service' do + it 'calls pipeline complete unlock artifacts service' do ci_ref.send(action) - expect(unlock_artifacts_worker_spy).to have_received(:perform_async).exactly(count).times + expect(unlock_previous_pipelines_worker_spy).to have_received(:perform_async).exactly(count).times end end end @@ -53,10 +53,10 @@ RSpec.describe Ci::Ref do context 'pipeline is unlocked' do let!(:pipeline) { create(:ci_pipeline, ci_ref_id: ci_ref.id, locked: :unlocked) } - it 'does not call unlock artifacts service' do + it 'does not unlock pipelines' do ci_ref.succeed! - expect(unlock_artifacts_worker_spy).not_to have_received(:perform_async) + expect(unlock_previous_pipelines_worker_spy).not_to have_received(:perform_async) end end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index a8e9d36a3a7..3a3ef072b28 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -309,19 +309,7 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end end - context 'when use_traversal_ids* are enabled' do - it_behaves_like '.belonging_to_parent_groups_of_project' - end - - context 'when use_traversal_ids* are disabled' do - before do - stub_feature_flags( - use_traversal_ids: false - ) - end - - it_behaves_like '.belonging_to_parent_groups_of_project' - end + it_behaves_like '.belonging_to_parent_groups_of_project' context 'with instance runners sharing enabled' do # group specific diff --git a/spec/models/ci/unlock_pipeline_request_spec.rb b/spec/models/ci/unlock_pipeline_request_spec.rb new file mode 100644 index 00000000000..ddfc6210349 --- /dev/null +++ b/spec/models/ci/unlock_pipeline_request_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::UnlockPipelineRequest, :unlock_pipelines, :clean_gitlab_redis_shared_state, feature_category: :build_artifacts do + describe '.enqueue' do + let(:pipeline_id) { 123 } + + subject(:enqueue) { described_class.enqueue(pipeline_id) } + + it 'creates a redis entry for the given pipeline ID and returns the number of added entries' do + freeze_time do + expect(described_class).to receive(:log_event).with(:enqueued, [pipeline_id]) + expect { enqueue } + .to change { pipeline_ids_waiting_to_be_unlocked } + .from([]) + .to([pipeline_id]) + + expect(enqueue).to eq(1) + expect_to_have_pending_unlock_pipeline_request(pipeline_id, Time.current.utc.to_i) + end + end + + context 'when the pipeline ID is already in the queue' do + before do + travel_to(3.minutes.ago) do + described_class.enqueue(pipeline_id) + end + end + + it 'does not create another redis entry for the same pipeline ID nor update it' do + expect(described_class).not_to receive(:log_event) + + expect { enqueue } + .to not_change { pipeline_ids_waiting_to_be_unlocked } + .and not_change { timestamp_of_pending_unlock_pipeline_request(pipeline_id) } + + expect(enqueue).to eq(0) + end + end + + context 'when given an array of pipeline IDs' do + let(:pipeline_ids) { [1, 2, 1] } + + subject(:enqueue) { described_class.enqueue(pipeline_ids) } + + it 'creates a redis entry for each unique pipeline ID' do + freeze_time do + expect(described_class).to receive(:log_event).with(:enqueued, pipeline_ids.uniq) + expect { enqueue } + .to change { pipeline_ids_waiting_to_be_unlocked } + .from([]) + .to([1, 2]) + + expect(enqueue).to eq(2) + + unix_timestamp = Time.current.utc.to_i + expect_to_have_pending_unlock_pipeline_request(1, unix_timestamp) + expect_to_have_pending_unlock_pipeline_request(2, unix_timestamp) + end + end + end + end + + describe '.next!' do + subject(:next_result) { described_class.next! } + + context 'when there are pending pipeline IDs' do + it 'pops and returns the oldest pipeline ID from the queue (FIFO)' do + expected_enqueue_time = nil + expected_pipeline_id = 1 + travel_to(3.minutes.ago) do + expected_enqueue_time = Time.current.utc.to_i + described_class.enqueue(expected_pipeline_id) + end + + travel_to(2.minutes.ago) { described_class.enqueue(2) } + travel_to(1.minute.ago) { described_class.enqueue(3) } + + expect(described_class).to receive(:log_event).with(:picked_next, 1) + + expect { next_result } + .to change { pipeline_ids_waiting_to_be_unlocked } + .from([1, 2, 3]) + .to([2, 3]) + + pipeline_id, enqueue_timestamp = next_result + + expect(pipeline_id).to eq(expected_pipeline_id) + expect(enqueue_timestamp).to eq(expected_enqueue_time) + end + end + + context 'when the queue is empty' do + it 'does nothing' do + expect(described_class).not_to receive(:log_event) + expect(next_result).to be_nil + end + end + end + + describe '.total_pending' do + subject { described_class.total_pending } + + before do + described_class.enqueue(1) + described_class.enqueue(2) + described_class.enqueue(3) + end + + it { is_expected.to eq(3) } + end +end diff --git a/spec/models/clusters/agent_token_spec.rb b/spec/models/clusters/agent_token_spec.rb index bc158fc9117..5f731336b4b 100644 --- a/spec/models/clusters/agent_token_spec.rb +++ b/spec/models/clusters/agent_token_spec.rb @@ -95,6 +95,15 @@ RSpec.describe Clusters::AgentToken, feature_category: :deployment_management do expect(agent_token.token).to start_with described_class::TOKEN_PREFIX end + + it 'is revoked on revoke!' do + agent_token = build(:cluster_agent_token, token_encrypted: nil) + agent_token.save! + + agent_token.revoke! + + expect(agent_token.active?).to be_falsey + end end describe '#to_ability_name' do diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 7dafec2536f..5fc5bbd41ff 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -26,7 +26,6 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching, it { is_expected.to have_many(:kubernetes_namespaces) } it { is_expected.to have_one(:cluster_project) } it { is_expected.to have_many(:deployment_clusters) } - it { is_expected.to have_many(:successful_deployments) } it { is_expected.to have_many(:environments).through(:deployments) } it { is_expected.to delegate_method(:status).to(:provider) } diff --git a/spec/models/concerns/integrations/enable_ssl_verification_spec.rb b/spec/models/concerns/integrations/enable_ssl_verification_spec.rb index 418f3f4dbc6..c9a9d33631b 100644 --- a/spec/models/concerns/integrations/enable_ssl_verification_spec.rb +++ b/spec/models/concerns/integrations/enable_ssl_verification_spec.rb @@ -2,18 +2,14 @@ require 'spec_helper' -RSpec.describe Integrations::EnableSslVerification do +RSpec.describe Integrations::EnableSslVerification, feature_category: :integrations do let(:described_class) do Class.new(Integration) do prepend Integrations::EnableSslVerification - def fields - [ - { name: 'main_url' }, - { name: 'other_url' }, - { name: 'username' } - ] - end + field :main_url + field :other_url + field :username end end diff --git a/spec/models/concerns/integrations/has_web_hook_spec.rb b/spec/models/concerns/integrations/has_web_hook_spec.rb index 9061cb90f90..69617b29f12 100644 --- a/spec/models/concerns/integrations/has_web_hook_spec.rb +++ b/spec/models/concerns/integrations/has_web_hook_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::HasWebHook do +RSpec.describe Integrations::HasWebHook, feature_category: :webhooks do let(:integration_class) do Class.new(Integration) do include Integrations::HasWebHook @@ -21,7 +21,7 @@ RSpec.describe Integrations::HasWebHook do end context 'when integration responds to enable_ssl_verification' do - let(:integration) { build(:drone_ci_integration) } + let(:integration) { build(:drone_ci_integration, enable_ssl_verification: true) } it { expect(integration.hook_ssl_verification).to eq true } end diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index dd180749e94..82c63eea33a 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -493,4 +493,24 @@ RSpec.describe Noteable, feature_category: :code_review_workflow do end end end + + describe '#supports_resolvable_notes' do + context 'when noteable is an abuse report' do + let(:abuse_report) { build(:abuse_report) } + + it 'returns true' do + expect(abuse_report.supports_resolvable_notes?).to be(true) + end + end + end + + describe '#supports_replying_to_individual_notes' do + context 'when noteable is an abuse report' do + let(:abuse_report) { build(:abuse_report) } + + it 'returns true' do + expect(abuse_report.supports_replying_to_individual_notes?).to be(true) + end + end + end end diff --git a/spec/models/concerns/prometheus_adapter_spec.rb b/spec/models/concerns/prometheus_adapter_spec.rb index a3f2e99f3da..d17059ccc6d 100644 --- a/spec/models/concerns/prometheus_adapter_spec.rb +++ b/spec/models/concerns/prometheus_adapter_spec.rb @@ -15,144 +15,6 @@ RSpec.describe PrometheusAdapter, :use_clean_rails_memory_store_caching do end end - let(:environment_query) { Gitlab::Prometheus::Queries::EnvironmentQuery } - - describe '#query' do - describe 'validate_query' do - let(:environment) { build_stubbed(:environment, slug: 'env-slug') } - let(:validation_query) { Gitlab::Prometheus::Queries::ValidateQuery.name } - let(:query) { 'avg(response)' } - let(:validation_respone) { { data: { valid: true } } } - - around do |example| - freeze_time { example.run } - end - - context 'with valid data' do - subject { integration.query(:validate, query) } - - before do - stub_reactive_cache(integration, validation_respone, validation_query, query) - end - - it 'returns query data' do - is_expected.to eq(query: { valid: true }) - end - end - end - - describe 'environment' do - let(:environment) { build_stubbed(:environment, slug: 'env-slug') } - - around do |example| - freeze_time { example.run } - end - - context 'with valid data' do - subject { integration.query(:environment, environment) } - - before do - stub_reactive_cache(integration, prometheus_data, environment_query, environment.id) - end - - it 'returns reactive data' do - is_expected.to eq(prometheus_metrics_data) - end - end - end - - describe 'matched_metrics' do - let(:matched_metrics_query) { Gitlab::Prometheus::Queries::MatchedMetricQuery } - let(:prometheus_client) { double(:prometheus_client, label_values: nil) } - - context 'with valid data' do - subject { integration.query(:matched_metrics) } - - before do - allow(integration).to receive(:prometheus_client).and_return(prometheus_client) - synchronous_reactive_cache(integration) - end - - it 'returns reactive data' do - expect(subject[:success]).to be_truthy - expect(subject[:data]).to eq([]) - end - end - end - - describe 'deployment' do - let(:deployment) { build_stubbed(:deployment) } - let(:deployment_query) { Gitlab::Prometheus::Queries::DeploymentQuery } - - around do |example| - freeze_time { example.run } - end - - context 'with valid data' do - subject { integration.query(:deployment, deployment) } - - before do - stub_reactive_cache(integration, prometheus_data, deployment_query, deployment.id) - end - - it 'returns reactive data' do - expect(subject).to eq(prometheus_metrics_data) - end - end - end - end - - describe '#calculate_reactive_cache' do - let(:environment) { create(:environment, slug: 'env-slug') } - - before do - integration.manual_configuration = true - integration.active = true - end - - subject do - integration.calculate_reactive_cache(environment_query.name, environment.id) - end - - around do |example| - freeze_time { example.run } - end - - context 'when integration is inactive' do - before do - integration.active = false - end - - it { is_expected.to be_nil } - end - - context 'when Prometheus responds with valid data' do - before do - stub_all_prometheus_requests(environment.slug) - end - - it { expect(subject.to_json).to eq(prometheus_data.to_json) } - end - - [404, 500].each do |status| - context "when Prometheus responds with #{status}" do - before do - stub_all_prometheus_requests(environment.slug, status: status, body: "QUERY FAILED!") - end - - it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } - end - end - - context "when client raises Gitlab::PrometheusClient::ConnectionError" do - before do - stub_any_prometheus_request.to_raise(Gitlab::PrometheusClient::ConnectionError) - end - - it { is_expected.to include(success: false, result: kind_of(String)) } - end - end - describe '#build_query_args' do subject { integration.build_query_args(*args) } diff --git a/spec/models/concerns/reset_on_column_errors_spec.rb b/spec/models/concerns/reset_on_column_errors_spec.rb new file mode 100644 index 00000000000..38ba0f447f5 --- /dev/null +++ b/spec/models/concerns/reset_on_column_errors_spec.rb @@ -0,0 +1,243 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ResetOnColumnErrors, :delete, feature_category: :shared do + let(:test_reviewer_model) do + Class.new(ApplicationRecord) do + self.table_name = '_test_reviewers_table' + + def self.name + 'TestReviewer' + end + end + end + + let(:test_attribute_reviewer_model) do + Class.new(ApplicationRecord) do + self.table_name = '_test_attribute_reviewers_table' + + belongs_to :test_attribute, class_name: 'TestAttribute' + belongs_to :test_reviewer, class_name: 'TestReviewer' + + def self.name + 'TestAttributeReviewer' + end + end + end + + let(:test_attribute_model) do + Class.new(ApplicationRecord) do + include FromUnion + + self.table_name = '_test_attribute_table' + + has_many :attribute_reviewers, class_name: 'TestAttributeReviewer' + has_many :reviewers, class_name: 'TestReviewer', through: :attribute_reviewers, source: :test_reviewer + + def self.name + 'TestAttribute' + end + end + end + + before do + stub_const('TestReviewer', test_reviewer_model) + stub_const('TestAttributeReviewer', test_attribute_reviewer_model) + stub_const('TestAttribute', test_attribute_model) + end + + before(:context) do + ApplicationRecord.connection.execute(<<~SQL) + CREATE TABLE _test_attribute_table ( + id serial NOT NULL PRIMARY KEY, + created_at timestamptz NOT NULL + ); + + CREATE TABLE _test_attribute_reviewers_table ( + test_attribute_id bigint, + test_reviewer_id bigint + ); + + CREATE TABLE _test_reviewers_table ( + id serial NOT NULL PRIMARY KEY, + created_at timestamptz NOT NULL + ); + + CREATE UNIQUE INDEX index_test_attribute_reviewers_table_unique + ON _test_attribute_reviewers_table + USING btree (test_attribute_id, test_reviewer_id); + SQL + end + + after(:context) do + ApplicationRecord.connection.execute(<<~SQL) + DROP TABLE _test_attribute_table; + DROP TABLE _test_attribute_reviewers_table; + DROP TABLE _test_reviewers_table; + SQL + end + + describe 'resetting on union errors' do + let(:expected_error_message) { /must have the same number of columns/ } + + def load_query + scopes = [ + TestAttribute.select('*'), + TestAttribute.select(TestAttribute.column_names.join(',')) + ] + + TestAttribute.from_union(scopes).load + end + + context 'with mismatched columns due to schema cache' do + before do + load_query + + ApplicationRecord.connection.execute(<<~SQL) + ALTER TABLE _test_attribute_table ADD COLUMN _test_new_column int; + SQL + end + + after do + ApplicationRecord.connection.execute(<<~SQL) + ALTER TABLE _test_attribute_table DROP COLUMN _test_new_column; + SQL + + TestAttribute.reset_column_information + end + + it 'resets column information when encountering an UNION error' do + expect do + load_query + end.to raise_error(ActiveRecord::StatementInvalid, expected_error_message) + .and change { TestAttribute.column_names } + .from(%w[id created_at]).to(%w[id created_at _test_new_column]) + + # Subsequent query load from new schema cache, so no more error + expect do + load_query + end.not_to raise_error + end + + it 'logs when column is reset' do + expect(Gitlab::ErrorTracking::Logger).to receive(:error) + .with(hash_including("extra.reset_model_name" => "TestAttribute")) + .and_call_original + + expect do + load_query + end.to raise_error(ActiveRecord::StatementInvalid, expected_error_message) + end + end + + context 'with mismatched columns due to coding error' do + def load_mismatched_query + scopes = [ + TestAttribute.select("id"), + TestAttribute.select("id, created_at") + ] + + TestAttribute.from_union(scopes).load + end + + it 'limits reset_column_information calls' do + expect(TestAttribute).to receive(:reset_column_information).and_call_original + + expect do + load_mismatched_query + end.to raise_error(ActiveRecord::StatementInvalid, expected_error_message) + + expect(TestAttribute).not_to receive(:reset_column_information) + + expect do + load_mismatched_query + end.to raise_error(ActiveRecord::StatementInvalid, expected_error_message) + end + + it 'does reset_column_information after some time has passed' do + expect do + load_mismatched_query + end.to raise_error(ActiveRecord::StatementInvalid, expected_error_message) + + travel_to(described_class::MAX_RESET_PERIOD.from_now + 1.minute) + expect(TestAttribute).to receive(:reset_column_information).and_call_original + + expect do + load_mismatched_query + end.to raise_error(ActiveRecord::StatementInvalid, expected_error_message) + end + end + + it 'handles ActiveRecord::StatementInvalid on the instance level' do + t = TestAttribute.create! + reviewer = TestReviewer.create! + + expect do + t.assign_attributes(reviewer_ids: [reviewer.id, reviewer.id]) + end.to raise_error(ActiveRecord::RecordNotUnique) + end + end + + describe 'resetting on missing column error on save' do + let(:expected_error_message) { /unknown attribute '_test_new_column'/ } + + context 'with mismatched columns due to schema cache' do + let!(:attrs) { TestAttribute.new.attributes } + + def initialize_with_new_column + TestAttribute.new(attrs.merge(_test_new_column: 123)) + end + + before do + ApplicationRecord.connection.execute(<<~SQL) + ALTER TABLE _test_attribute_table ADD COLUMN _test_new_column int; + SQL + end + + after do + ApplicationRecord.connection.execute(<<~SQL) + ALTER TABLE _test_attribute_table DROP COLUMN _test_new_column; + SQL + + TestAttribute.reset_column_information + end + + it 'resets column information when encountering an UnknownAttributeError' do + expect do + initialize_with_new_column + end.to raise_error(ActiveModel::UnknownAttributeError, expected_error_message) + .and change { TestAttribute.column_names } + .from(%w[id created_at]).to(%w[id created_at _test_new_column]) + + # Subsequent query load from new schema cache, so no more error + expect do + initialize_with_new_column + end.not_to raise_error + end + + it 'logs when column is reset' do + expect(Gitlab::ErrorTracking::Logger).to receive(:error) + .with(hash_including("extra.reset_model_name" => "TestAttribute")) + .and_call_original + + expect do + initialize_with_new_column + end.to raise_error(ActiveModel::UnknownAttributeError, expected_error_message) + end + + context 'when reset_column_information_on_statement_invalid FF is disabled' do + before do + stub_feature_flags(reset_column_information_on_statement_invalid: false) + end + + it 'does not reset column information' do + expect do + initialize_with_new_column + end.to raise_error(ActiveModel::UnknownAttributeError, expected_error_message) + .and not_change { TestAttribute.column_names } + end + end + end + end +end diff --git a/spec/models/concerns/reset_on_union_error_spec.rb b/spec/models/concerns/reset_on_union_error_spec.rb deleted file mode 100644 index 70993b92c90..00000000000 --- a/spec/models/concerns/reset_on_union_error_spec.rb +++ /dev/null @@ -1,132 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ResetOnUnionError, :delete, feature_category: :shared do - let(:test_unioned_model) do - Class.new(ApplicationRecord) do - include FromUnion - - self.table_name = '_test_unioned_model' - - def self.name - 'TestUnion' - end - end - end - - before(:context) do - ApplicationRecord.connection.execute(<<~SQL) - CREATE TABLE _test_unioned_model ( - id serial NOT NULL PRIMARY KEY, - created_at timestamptz NOT NULL - ); - SQL - end - - after(:context) do - ApplicationRecord.connection.execute(<<~SQL) - DROP TABLE _test_unioned_model - SQL - end - - context 'with mismatched columns due to schema cache' do - def load_query - scopes = [ - test_unioned_model.select('*'), - test_unioned_model.select(test_unioned_model.column_names.join(',')) - ] - - test_unioned_model.from_union(scopes).load - end - - before do - load_query - - ApplicationRecord.connection.execute(<<~SQL) - ALTER TABLE _test_unioned_model ADD COLUMN _test_new_column int; - SQL - end - - after do - ApplicationRecord.connection.execute(<<~SQL) - ALTER TABLE _test_unioned_model DROP COLUMN _test_new_column; - SQL - - test_unioned_model.reset_column_information - end - - it 'resets column information when encountering an UNION error' do - expect do - load_query - end.to raise_error(ActiveRecord::StatementInvalid, /must have the same number of columns/) - .and change { test_unioned_model.column_names }.from(%w[id created_at]).to(%w[id created_at _test_new_column]) - - # Subsequent query load from new schema cache, so no more error - expect do - load_query - end.not_to raise_error - end - - it 'logs when column is reset' do - expect(Gitlab::ErrorTracking::Logger).to receive(:error) - .with(hash_including("extra.reset_model_name" => "TestUnion")) - .and_call_original - - expect do - load_query - end.to raise_error(ActiveRecord::StatementInvalid, /must have the same number of columns/) - end - - context 'when reset_column_information_on_statement_invalid FF is disabled' do - before do - stub_feature_flags(reset_column_information_on_statement_invalid: false) - end - - it 'does not reset column information' do - expect do - load_query - end.to raise_error(ActiveRecord::StatementInvalid, /must have the same number of columns/) - .and not_change { test_unioned_model.column_names } - end - end - end - - context 'with mismatched columns due to coding error' do - def load_mismatched_query - scopes = [ - test_unioned_model.select("id"), - test_unioned_model.select("id, created_at") - ] - - test_unioned_model.from_union(scopes).load - end - - it 'limits reset_column_information calls' do - expect(test_unioned_model).to receive(:reset_column_information).and_call_original - - expect do - load_mismatched_query - end.to raise_error(ActiveRecord::StatementInvalid, /must have the same number of columns/) - - expect(test_unioned_model).not_to receive(:reset_column_information) - - expect do - load_mismatched_query - end.to raise_error(ActiveRecord::StatementInvalid, /must have the same number of columns/) - end - - it 'does reset_column_information after some time has passed' do - expect do - load_mismatched_query - end.to raise_error(ActiveRecord::StatementInvalid, /must have the same number of columns/) - - travel_to(described_class::MAX_RESET_PERIOD.from_now + 1.minute) - expect(test_unioned_model).to receive(:reset_column_information).and_call_original - - expect do - load_mismatched_query - end.to raise_error(ActiveRecord::StatementInvalid, /must have the same number of columns/) - end - end -end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 2b6f8535743..7e324812b97 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -3,18 +3,12 @@ require 'spec_helper' RSpec.shared_examples 'routable resource' do - shared_examples_for '.find_by_full_path' do + shared_examples_for '.find_by_full_path' do |has_cross_join: false| 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) end - it 'checks if `optimize_routable` is enabled only once' do - expect(Routable).to receive(:optimize_routable_enabled?).once - - described_class.find_by_full_path(record.full_path) - end - it 'returns nil for unknown paths' do expect(described_class.find_by_full_path('unknown')).to be_nil end @@ -51,27 +45,23 @@ RSpec.shared_examples 'routable resource' do end end end - end - - it_behaves_like '.find_by_full_path', :aggregate_failures - - context 'when the `optimize_routable` feature flag is turned OFF' do - before do - stub_feature_flags(optimize_routable: false) - end - it_behaves_like '.find_by_full_path', :aggregate_failures + if has_cross_join + it 'has a cross-join' do + expect(Gitlab::Database).to receive(:allow_cross_joins_across_databases) - it 'includes route information when loading a record' do - control_count = ActiveRecord::QueryRecorder.new do described_class.find_by_full_path(record.full_path) - end.count + end + else + it 'does not have cross-join' do + expect(Gitlab::Database).not_to receive(:allow_cross_joins_across_databases) - expect do - described_class.find_by_full_path(record.full_path).route - end.not_to exceed_all_query_limit(control_count) + described_class.find_by_full_path(record.full_path) + end end end + + it_behaves_like '.find_by_full_path', :aggregate_failures end RSpec.shared_examples 'routable resource with parent' do @@ -274,22 +264,6 @@ RSpec.describe Namespaces::ProjectNamespace, 'Routable', :with_clean_rails_cache end end -RSpec.describe Routable, feature_category: :groups_and_projects do - describe '.optimize_routable_enabled?' do - subject { described_class.optimize_routable_enabled? } - - it { is_expected.to eq(true) } - - context 'when the `optimize_routable` feature flag is turned OFF' do - before do - stub_feature_flags(optimize_routable: false) - end - - it { is_expected.to eq(false) } - end - end -end - def forcibly_hit_cached_lookup(record, method) stub_feature_flags(cached_route_lookups: true) expect(record).to receive(:persisted?).and_return(true) diff --git a/spec/models/container_expiration_policy_spec.rb b/spec/models/container_expiration_policy_spec.rb index e5f9fdd410e..1e911af5670 100644 --- a/spec/models/container_expiration_policy_spec.rb +++ b/spec/models/container_expiration_policy_spec.rb @@ -11,8 +11,7 @@ RSpec.describe ContainerExpirationPolicy, type: :model do it { is_expected.to validate_presence_of(:project) } describe '#enabled' do - it { is_expected.to allow_value(true, false).for(:enabled) } - it { is_expected.not_to allow_value(nil).for(:enabled) } + it { is_expected.to validate_inclusion_of(:enabled).in_array([true, false]) } end describe '#cadence' do diff --git a/spec/models/container_registry/protection/rule_spec.rb b/spec/models/container_registry/protection/rule_spec.rb new file mode 100644 index 00000000000..9f162736efd --- /dev/null +++ b/spec/models/container_registry/protection/rule_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ContainerRegistry::Protection::Rule, type: :model, feature_category: :container_registry do + it_behaves_like 'having unique enum values' + + describe 'relationships' do + it { is_expected.to belong_to(:project).inverse_of(:container_registry_protection_rules) } + end + + describe 'enums' do + it { + is_expected.to( + define_enum_for(:push_protected_up_to_access_level) + .with_values( + developer: Gitlab::Access::DEVELOPER, + maintainer: Gitlab::Access::MAINTAINER, + owner: Gitlab::Access::OWNER + ) + .with_prefix(:push_protected_up_to) + ) + } + + it { + is_expected.to( + define_enum_for(:delete_protected_up_to_access_level) + .with_values( + developer: Gitlab::Access::DEVELOPER, + maintainer: Gitlab::Access::MAINTAINER, + owner: Gitlab::Access::OWNER + ) + .with_prefix(:delete_protected_up_to) + ) + } + end + + 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) } + end + + describe '#delete_protected_up_to_access_level' do + it { is_expected.to validate_presence_of(:delete_protected_up_to_access_level) } + end + + describe '#push_protected_up_to_access_level' do + it { is_expected.to validate_presence_of(:push_protected_up_to_access_level) } + end + end +end diff --git a/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb index a58e8df45e4..203f477c1a0 100644 --- a/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb +++ b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb @@ -11,8 +11,7 @@ RSpec.describe DependencyProxy::ImageTtlGroupPolicy, type: :model do it { is_expected.to validate_presence_of(:group) } describe '#enabled' do - it { is_expected.to allow_value(true, false).for(:enabled) } - it { is_expected.not_to allow_value(nil).for(:enabled) } + it { is_expected.to validate_inclusion_of(:enabled).in_array([true, false]) } end describe '#ttl' do diff --git a/spec/models/discussion_note_spec.rb b/spec/models/discussion_note_spec.rb index 6e1b39cc438..09adf4a95b5 100644 --- a/spec/models/discussion_note_spec.rb +++ b/spec/models/discussion_note_spec.rb @@ -8,4 +8,12 @@ RSpec.describe DiscussionNote do it { is_expected.to eq('note') } end + + describe 'validations' do + context 'when noteable is an abuse report' do + subject { build(:discussion_note, noteable: build_stubbed(:abuse_report)) } + + it { is_expected.to be_valid } + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 9d4699cb91e..dcfee7fcc8c 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1516,42 +1516,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end end - describe '#metrics' do - let_it_be(:project) { create(:project, :with_prometheus_integration) } - - subject { environment.metrics } - - context 'when the environment has metrics' do - before do - allow(environment).to receive(:has_metrics?).and_return(true) - end - - it 'returns the metrics from the deployment service' do - expect(environment.prometheus_adapter) - .to receive(:query).with(:environment, environment) - .and_return(:fake_metrics) - - is_expected.to eq(:fake_metrics) - end - - context 'and the prometheus client is not present' do - before do - allow(environment.prometheus_adapter).to receive(:promethus_client).and_return(nil) - end - - it { is_expected.to be_nil } - end - end - - context 'when the environment does not have metrics' do - before do - allow(environment).to receive(:has_metrics?).and_return(false) - end - - it { is_expected.to be_nil } - end - end - describe '#additional_metrics' do let_it_be(:project) { create(:project, :with_prometheus_integration) } let(:metric_params) { [] } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index ddeab16908d..96ef36a5b75 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -683,160 +683,126 @@ RSpec.describe Group, feature_category: :groups_and_projects do context 'traversal queries' do let_it_be(:group, reload: true) { create(:group, :nested) } - context 'recursive' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it_behaves_like 'namespace traversal' + it_behaves_like 'namespace traversal' - describe '#self_and_descendants' do - it { expect(group.self_and_descendants.to_sql).not_to include 'traversal_ids @>' } - end + describe '#self_and_descendants' do + it { expect(group.self_and_descendants.to_sql).to include 'traversal_ids @>' } + end - describe '#self_and_descendant_ids' do - it { expect(group.self_and_descendant_ids.to_sql).not_to include 'traversal_ids @>' } - end + describe '#self_and_descendant_ids' do + it { expect(group.self_and_descendant_ids.to_sql).to include 'traversal_ids @>' } + end - describe '#descendants' do - it { expect(group.descendants.to_sql).not_to include 'traversal_ids @>' } - end + describe '#descendants' do + it { expect(group.descendants.to_sql).to include 'traversal_ids @>' } + end - describe '#self_and_hierarchy' do - it { expect(group.self_and_hierarchy.to_sql).not_to include 'traversal_ids @>' } - end + describe '#self_and_hierarchy' do + it { expect(group.self_and_hierarchy.to_sql).to include 'traversal_ids @>' } + end - describe '#ancestors' do - it { expect(group.ancestors.to_sql).not_to include 'traversal_ids <@' } - end + describe '#ancestors' do + it { expect(group.ancestors.to_sql).to include "\"namespaces\".\"id\" = #{group.parent_id}" } - describe '.shortest_traversal_ids_prefixes' do - it { expect { described_class.shortest_traversal_ids_prefixes }.to raise_error /Feature not supported since the `:use_traversal_ids` is disabled/ } + it 'hierarchy order' do + expect(group.ancestors(hierarchy_order: :asc).to_sql).to include 'ORDER BY "depth" ASC' end end - context 'linear' do - it_behaves_like 'namespace traversal' + describe '#ancestors_upto' do + it { expect(group.ancestors_upto.to_sql).to include "WITH ORDINALITY" } + end - describe '#self_and_descendants' do - it { expect(group.self_and_descendants.to_sql).to include 'traversal_ids @>' } - end + describe '.shortest_traversal_ids_prefixes' do + subject { filter.shortest_traversal_ids_prefixes } - describe '#self_and_descendant_ids' do - it { expect(group.self_and_descendant_ids.to_sql).to include 'traversal_ids @>' } - end + context 'for many top-level namespaces' do + let!(:top_level_groups) { create_list(:group, 4) } - describe '#descendants' do - it { expect(group.descendants.to_sql).to include 'traversal_ids @>' } - end + context 'when querying all groups' do + let(:filter) { described_class.id_in(top_level_groups) } - describe '#self_and_hierarchy' do - it { expect(group.self_and_hierarchy.to_sql).to include 'traversal_ids @>' } - end + it "returns all traversal_ids" do + is_expected.to contain_exactly( + *top_level_groups.map { |group| [group.id] } + ) + end + end - describe '#ancestors' do - it { expect(group.ancestors.to_sql).to include "\"namespaces\".\"id\" = #{group.parent_id}" } + context 'when querying selected groups' do + let(:filter) { described_class.id_in(top_level_groups.first) } - it 'hierarchy order' do - expect(group.ancestors(hierarchy_order: :asc).to_sql).to include 'ORDER BY "depth" ASC' + it "returns only a selected traversal_ids" do + is_expected.to contain_exactly([top_level_groups.first.id]) + end end end - describe '#ancestors_upto' do - it { expect(group.ancestors_upto.to_sql).to include "WITH ORDINALITY" } - end + context 'for namespace hierarchy' do + let!(:group_a) { create(:group) } + let!(:group_a_sub_1) { create(:group, parent: group_a) } + let!(:group_a_sub_2) { create(:group, parent: group_a) } + let!(:group_b) { create(:group) } + let!(:group_b_sub_1) { create(:group, parent: group_b) } + let!(:group_c) { create(:group) } - describe '.shortest_traversal_ids_prefixes' do - subject { filter.shortest_traversal_ids_prefixes } + context 'when querying all groups' do + let(:filter) { described_class.id_in([group_a, group_a_sub_1, group_a_sub_2, group_b, group_b_sub_1, group_c]) } - context 'for many top-level namespaces' do - let!(:top_level_groups) { create_list(:group, 4) } - - context 'when querying all groups' do - let(:filter) { described_class.id_in(top_level_groups) } - - it "returns all traversal_ids" do - is_expected.to contain_exactly( - *top_level_groups.map { |group| [group.id] } - ) - end - end - - context 'when querying selected groups' do - let(:filter) { described_class.id_in(top_level_groups.first) } - - it "returns only a selected traversal_ids" do - is_expected.to contain_exactly([top_level_groups.first.id]) - end + it 'returns only shortest prefixes of top-level groups' do + is_expected.to contain_exactly( + [group_a.id], + [group_b.id], + [group_c.id] + ) end end - context 'for namespace hierarchy' do - let!(:group_a) { create(:group) } - let!(:group_a_sub_1) { create(:group, parent: group_a) } - let!(:group_a_sub_2) { create(:group, parent: group_a) } - let!(:group_b) { create(:group) } - let!(:group_b_sub_1) { create(:group, parent: group_b) } - let!(:group_c) { create(:group) } + context 'when sub-group is reparented' do + let(:filter) { described_class.id_in([group_b_sub_1, group_c]) } - context 'when querying all groups' do - let(:filter) { described_class.id_in([group_a, group_a_sub_1, group_a_sub_2, group_b, group_b_sub_1, group_c]) } - - it 'returns only shortest prefixes of top-level groups' do - is_expected.to contain_exactly( - [group_a.id], - [group_b.id], - [group_c.id] - ) - end + before do + group_b_sub_1.update!(parent: group_c) end - context 'when sub-group is reparented' do - let(:filter) { described_class.id_in([group_b_sub_1, group_c]) } - - before do - group_b_sub_1.update!(parent: group_c) - end - - it 'returns a proper shortest prefix of a new group' do - is_expected.to contain_exactly( - [group_c.id] - ) - end + it 'returns a proper shortest prefix of a new group' do + is_expected.to contain_exactly( + [group_c.id] + ) end + end - context 'when querying sub-groups' do - let(:filter) { described_class.id_in([group_a_sub_1, group_b_sub_1, group_c]) } + context 'when querying sub-groups' do + let(:filter) { described_class.id_in([group_a_sub_1, group_b_sub_1, group_c]) } - it 'returns sub-groups as they are shortest prefixes' do - is_expected.to contain_exactly( - [group_a.id, group_a_sub_1.id], - [group_b.id, group_b_sub_1.id], - [group_c.id] - ) - end + it 'returns sub-groups as they are shortest prefixes' do + is_expected.to contain_exactly( + [group_a.id, group_a_sub_1.id], + [group_b.id, group_b_sub_1.id], + [group_c.id] + ) end + end - context 'when querying group and sub-group of this group' do - let(:filter) { described_class.id_in([group_a, group_a_sub_1, group_c]) } + context 'when querying group and sub-group of this group' do + let(:filter) { described_class.id_in([group_a, group_a_sub_1, group_c]) } - it 'returns parent groups as this contains all sub-groups' do - is_expected.to contain_exactly( - [group_a.id], - [group_c.id] - ) - end + it 'returns parent groups as this contains all sub-groups' do + is_expected.to contain_exactly( + [group_a.id], + [group_c.id] + ) end end end + end - context 'when project namespace exists in the group' do - let!(:project) { create(:project, group: group) } - let!(:project_namespace) { project.project_namespace } + context 'when project namespace exists in the group' do + let!(:project) { create(:project, group: group) } + let!(:project_namespace) { project.project_namespace } - it 'filters out project namespace' do - expect(group.descendants.find_by_id(project_namespace.id)).to be_nil - end + it 'filters out project namespace' do + expect(group.descendants.find_by_id(project_namespace.id)).to be_nil end end end @@ -921,6 +887,143 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end + describe '.sort_by_attribute' do + before do + group.destroy! + end + + let!(:group_1) { create(:group, name: 'Y group') } + let!(:group_2) { create(:group, name: 'J group', created_at: 2.days.ago, updated_at: 1.day.ago) } + let!(:group_3) { create(:group, name: 'A group') } + let!(:group_4) { create(:group, name: 'F group', created_at: 1.day.ago, updated_at: 1.day.ago) } + + subject { described_class.with_statistics.with_route.sort_by_attribute(sort) } + + context 'when sort by is not provided (id desc by default)' do + let(:sort) { nil } + + it { is_expected.to eq([group_1, group_2, group_3, group_4]) } + end + + context 'when sort by name_asc' do + let(:sort) { 'name_asc' } + + it { is_expected.to eq([group_3, group_4, group_2, group_1]) } + end + + context 'when sort by name_desc' do + let(:sort) { 'name_desc' } + + it { is_expected.to eq([group_1, group_2, group_4, group_3]) } + end + + context 'when sort by recently_created' do + let(:sort) { 'created_desc' } + + it { is_expected.to eq([group_3, group_1, group_4, group_2]) } + end + + context 'when sort by oldest_created' do + let(:sort) { 'created_asc' } + + it { is_expected.to eq([group_2, group_4, group_1, group_3]) } + end + + context 'when sort by latest_activity' do + let(:sort) { 'latest_activity_desc' } + + it { is_expected.to eq([group_1, group_2, group_3, group_4]) } + end + + context 'when sort by oldest_activity' do + let(:sort) { 'latest_activity_asc' } + + it { is_expected.to eq([group_1, group_2, group_3, group_4]) } + end + + context 'when sort by storage_size_desc' do + let!(:project_1) do + create(:project, + namespace: group_1, + statistics: build( + :project_statistics, + namespace: group_1, + repository_size: 2178370, + storage_size: 1278370, + wiki_size: 505, + lfs_objects_size: 202, + build_artifacts_size: 303, + pipeline_artifacts_size: 707, + packages_size: 404, + snippets_size: 605, + uploads_size: 808 + ) + ) + end + + let!(:project_2) do + create(:project, + namespace: group_2, + statistics: build( + :project_statistics, + namespace: group_2, + repository_size: 3178370, + storage_size: 3178370, + wiki_size: 505, + lfs_objects_size: 202, + build_artifacts_size: 303, + pipeline_artifacts_size: 707, + packages_size: 404, + snippets_size: 605, + uploads_size: 808 + ) + ) + end + + let!(:project_3) do + create(:project, + namespace: group_3, + statistics: build( + :project_statistics, + namespace: group_3, + repository_size: 1278370, + storage_size: 1178370, + wiki_size: 505, + lfs_objects_size: 202, + build_artifacts_size: 303, + pipeline_artifacts_size: 707, + packages_size: 404, + snippets_size: 605, + uploads_size: 808 + ) + ) + end + + let!(:project_4) do + create(:project, + namespace: group_4, + statistics: build( + :project_statistics, + namespace: group_4, + repository_size: 2178370, + storage_size: 2278370, + wiki_size: 505, + lfs_objects_size: 202, + build_artifacts_size: 303, + pipeline_artifacts_size: 707, + packages_size: 404, + snippets_size: 605, + uploads_size: 808 + ) + ) + end + + let(:sort) { 'storage_size_desc' } + + it { is_expected.to eq([group_2, group_4, group_1, group_3]) } + end + end + describe 'scopes' do let_it_be(:private_group) { create(:group, :private) } let_it_be(:internal_group) { create(:group, :internal) } @@ -1152,21 +1255,6 @@ RSpec.describe Group, feature_category: :groups_and_projects do expect(group.group_members.developers.map(&:user)).to include(user) expect(group.group_members.guests.map(&:user)).not_to include(user) end - - context 'when `tasks_to_be_done` and `tasks_project_id` are passed' do - let!(:project) { create(:project, group: group) } - - before do - group.add_members([create(:user)], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) - end - - it 'creates a member_task with the correct attributes', :aggregate_failures do - member = group.group_members.last - - expect(member.tasks_to_be_done).to match_array([:ci, :code]) - expect(member.member_task.project).to eq(project) - end - end end describe '#avatar_type' do @@ -1340,6 +1428,11 @@ RSpec.describe Group, feature_category: :groups_and_projects do group.add_member(user, GroupMember::OWNER) end + before do + # Add an invite to the group, which should be filtered out + create(:group_member, :invited, source: group) + end + it 'returns the member-owners' do expect(group.member_owners_excluding_project_bots).to contain_exactly(member_owner) end @@ -1367,6 +1460,16 @@ RSpec.describe Group, feature_category: :groups_and_projects do it 'returns only direct member-owners' do expect(group.member_owners_excluding_project_bots).to contain_exactly(member_owner) end + + context 'when there is an invite in the linked group' do + before do + create(:group_member, :invited, source: subgroup) + end + + it 'returns only direct member-owners' do + expect(group.member_owners_excluding_project_bots).to contain_exactly(member_owner) + end + end end end @@ -1382,6 +1485,31 @@ RSpec.describe Group, feature_category: :groups_and_projects do it 'returns member-owners including parents' do expect(subgroup.member_owners_excluding_project_bots).to contain_exactly(member_owner, member_owner_2) end + + context 'with group sharing' do + let_it_be(:invited_group) { create(:group) } + + let!(:invited_group_owner) { invited_group.add_member(user, GroupMember::OWNER) } + + before do + create(:group_group_link, :owner, shared_group: subgroup, shared_with_group: invited_group) + end + + it 'returns member-owners including parents, and member-owners of the invited group' do + expect(subgroup.member_owners_excluding_project_bots).to contain_exactly(member_owner, member_owner_2, invited_group_owner) + end + + context 'when there is an invite in the linked group' do + before do + # Add an invite to this group, which should be filtered out + create(:group_member, :invited, source: invited_group) + end + + it 'returns member-owners including parents, and member-owners of the invited group' do + expect(subgroup.member_owners_excluding_project_bots).to contain_exactly(member_owner, member_owner_2, invited_group_owner) + end + end + end end end @@ -1561,6 +1689,14 @@ RSpec.describe Group, feature_category: :groups_and_projects do it 'returns correct access level' do expect(group.max_member_access_for_user(group_user)).to eq(Gitlab::Access::OWNER) end + + context 'when user is not active' do + let_it_be(:group_user) { create(:user, :deactivated) } + + it 'returns NO_ACCESS' do + expect(group.max_member_access_for_user(group_user)).to eq(Gitlab::Access::NO_ACCESS) + end + end end context 'when user is nil' do @@ -3320,13 +3456,6 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end - describe '#content_editor_on_issues_feature_flag_enabled?' do - it_behaves_like 'checks self and root ancestor feature flag' do - let(:feature_flag) { :content_editor_on_issues } - let(:feature_flag_method) { :content_editor_on_issues_feature_flag_enabled? } - end - end - describe '#work_items_feature_flag_enabled?' do it_behaves_like 'checks self and root ancestor feature flag' do let(:feature_flag) { :work_items } diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 67e12092e1a..d7b69546de6 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -598,23 +598,7 @@ RSpec.describe Integration, feature_category: :integrations do end end - context 'recursive' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - include_examples 'correct ancestor order' - end - - context 'linear' do - before do - stub_feature_flags(use_traversal_ids: true) - - sub_subgroup.reload # make sure traversal_ids are reloaded - end - - include_examples 'correct ancestor order' - end + include_examples 'correct ancestor order' end end end @@ -1206,11 +1190,10 @@ RSpec.describe Integration, feature_category: :integrations do end end - describe 'boolean_accessor' do + describe 'Checkbox field booleans' do let(:klass) do Class.new(Integration) do - prop_accessor :test_value - boolean_accessor :test_value + field :test_value, type: :checkbox end end @@ -1284,24 +1267,6 @@ RSpec.describe Integration, feature_category: :integrations do test_value?: be(false) ) end - - context 'when getter is not defined' do - let(:input) { true } - let(:klass) do - Class.new(Integration) do - boolean_accessor :test_value - end - end - - it 'defines a prop_accessor' do - expect(integration).to have_attributes( - test_value: true, - test_value?: true - ) - - expect(integration.properties['test_value']).to be(true) - end - end end describe '#attributes' do diff --git a/spec/models/integrations/apple_app_store_spec.rb b/spec/models/integrations/apple_app_store_spec.rb index 9864fe38d3f..ea66c382726 100644 --- a/spec/models/integrations/apple_app_store_spec.rb +++ b/spec/models/integrations/apple_app_store_spec.rb @@ -13,8 +13,7 @@ RSpec.describe Integrations::AppleAppStore, feature_category: :mobile_devops do it { is_expected.to validate_presence_of :app_store_key_id } it { is_expected.to validate_presence_of :app_store_private_key } it { is_expected.to validate_presence_of :app_store_private_key_file_name } - it { is_expected.to allow_value(true, false).for(:app_store_protected_refs) } - it { is_expected.not_to allow_value(nil).for(:app_store_protected_refs) } + it { is_expected.to validate_inclusion_of(:app_store_protected_refs).in_array([true, false]) } it { is_expected.to allow_value('aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee').for(:app_store_issuer_id) } it { is_expected.not_to allow_value('abcde').for(:app_store_issuer_id) } it { is_expected.to allow_value(File.read('spec/fixtures/ssl_key.pem')).for(:app_store_private_key) } diff --git a/spec/models/integrations/asana_spec.rb b/spec/models/integrations/asana_spec.rb index 376aec1088e..70c56d35a04 100644 --- a/spec/models/integrations/asana_spec.rb +++ b/spec/models/integrations/asana_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Integrations::Asana, feature_category: :integrations do let_it_be(:project) { build(:project) } let(:gid) { "123456789ABCD" } - let(:asana_task) { double(::Asana::Resources::Task) } + let(:asana_task) { double(data: { gid: gid }) } let(:asana_integration) { described_class.new } let(:ref) { 'main' } let(:restrict_to_branch) { nil } @@ -41,6 +41,15 @@ RSpec.describe Integrations::Asana, feature_category: :integrations do } end + let(:completed_message) do + { + body: { + completed: true + }, + headers: { "Authorization" => "Bearer verySecret" } + } + end + before do allow(asana_integration).to receive_messages( project: project, @@ -60,9 +69,10 @@ RSpec.describe Integrations::Asana, feature_category: :integrations do let(:ref) { 'main' } it 'calls the Asana integration' do - expect(asana_task).to receive(:add_comment) - expect(asana_task).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '456789').once.and_return(asana_task) + expect(Gitlab::HTTP).to receive(:post) + .with("https://app.asana.com/api/1.0/tasks/456789/stories", anything).once.and_return(asana_task) + expect(Gitlab::HTTP).to receive(:put) + .with("https://app.asana.com/api/1.0/tasks/456789", completed_message).once.and_return(asana_task) execute_integration end @@ -72,8 +82,8 @@ RSpec.describe Integrations::Asana, feature_category: :integrations do let(:ref) { 'mai' } it 'does not call the Asana integration' do - expect(asana_task).not_to receive(:add_comment) - expect(::Asana::Resources::Task).not_to receive(:find_by_id) + expect(Gitlab::HTTP).not_to receive(:post) + expect(Gitlab::HTTP).not_to receive(:put) execute_integration end @@ -83,12 +93,17 @@ RSpec.describe Integrations::Asana, feature_category: :integrations do context 'when creating a story' do let(:message) { "Message from commit. related to ##{gid}" } let(:expected_message) do - "#{user.name} pushed to branch main of #{project.full_name} ( https://gitlab.com/ ): #{message}" + { + body: { + text: "#{user.name} pushed to branch main of #{project.full_name} ( https://gitlab.com/ ): #{message}" + }, + headers: { "Authorization" => "Bearer verySecret" } + } end it 'calls Asana integration to create a story' do - expect(asana_task).to receive(:add_comment).with(text: expected_message) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, gid).once.and_return(asana_task) + expect(Gitlab::HTTP).to receive(:post) + .with("https://app.asana.com/api/1.0/tasks/#{gid}/stories", expected_message).once.and_return(asana_task) execute_integration end @@ -98,9 +113,10 @@ RSpec.describe Integrations::Asana, feature_category: :integrations do let(:message) { 'fix #456789' } it 'calls Asana integration to create a story and close a task' do - expect(asana_task).to receive(:add_comment) - expect(asana_task).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '456789').once.and_return(asana_task) + expect(Gitlab::HTTP).to receive(:post) + .with("https://app.asana.com/api/1.0/tasks/456789/stories", anything).once.and_return(asana_task) + expect(Gitlab::HTTP).to receive(:put) + .with("https://app.asana.com/api/1.0/tasks/456789", completed_message).once.and_return(asana_task) execute_integration end @@ -110,9 +126,10 @@ RSpec.describe Integrations::Asana, feature_category: :integrations do let(:message) { 'closes https://app.asana.com/19292/956299/42' } it 'calls Asana integration to close via url' do - expect(asana_task).to receive(:add_comment) - expect(asana_task).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '42').once.and_return(asana_task) + expect(Gitlab::HTTP).to receive(:post) + .with("https://app.asana.com/api/1.0/tasks/42/stories", anything).once.and_return(asana_task) + expect(Gitlab::HTTP).to receive(:put) + .with("https://app.asana.com/api/1.0/tasks/42", completed_message).once.and_return(asana_task) execute_integration end @@ -127,27 +144,30 @@ RSpec.describe Integrations::Asana, feature_category: :integrations do end it 'allows multiple matches per line' do - expect(asana_task).to receive(:add_comment) - expect(asana_task).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '123').once.and_return(asana_task) - - asana_task_2 = double(Asana::Resources::Task) - expect(asana_task_2).to receive(:add_comment) - expect(asana_task_2).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '456').once.and_return(asana_task_2) - - asana_task_3 = double(Asana::Resources::Task) - expect(asana_task_3).to receive(:add_comment) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '789').once.and_return(asana_task_3) - - asana_task_4 = double(Asana::Resources::Task) - expect(asana_task_4).to receive(:add_comment) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '42').once.and_return(asana_task_4) - - asana_task_5 = double(Asana::Resources::Task) - expect(asana_task_5).to receive(:add_comment) - expect(asana_task_5).to receive(:update).with(completed: true) - expect(::Asana::Resources::Task).to receive(:find_by_id).with(anything, '12').once.and_return(asana_task_5) + expect(Gitlab::HTTP).to receive(:post) + .with("https://app.asana.com/api/1.0/tasks/123/stories", anything).once.and_return(asana_task) + expect(Gitlab::HTTP).to receive(:put) + .with("https://app.asana.com/api/1.0/tasks/123", completed_message).once.and_return(asana_task) + + asana_task_2 = double(double(data: { gid: 456 })) + expect(Gitlab::HTTP).to receive(:post) + .with("https://app.asana.com/api/1.0/tasks/456/stories", anything).once.and_return(asana_task_2) + expect(Gitlab::HTTP).to receive(:put) + .with("https://app.asana.com/api/1.0/tasks/456", completed_message).once.and_return(asana_task_2) + + asana_task_3 = double(double(data: { gid: 789 })) + expect(Gitlab::HTTP).to receive(:post) + .with("https://app.asana.com/api/1.0/tasks/789/stories", anything).once.and_return(asana_task_3) + + asana_task_4 = double(double(data: { gid: 42 })) + expect(Gitlab::HTTP).to receive(:post) + .with("https://app.asana.com/api/1.0/tasks/42/stories", anything).once.and_return(asana_task_4) + + asana_task_5 = double(double(data: { gid: 12 })) + expect(Gitlab::HTTP).to receive(:post) + .with("https://app.asana.com/api/1.0/tasks/12/stories", anything).once.and_return(asana_task_5) + expect(Gitlab::HTTP).to receive(:put) + .with("https://app.asana.com/api/1.0/tasks/12", completed_message).once.and_return(asana_task_5) execute_integration end diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index 3b459ab9d5b..62080fa7a12 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -116,7 +116,7 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching, feat is_expected.to eq('http://gitlab.com/bamboo/browse/42') end - context 'bamboo_url has trailing slash' do + context 'when bamboo_url has trailing slash' do let(:bamboo_url) { 'http://gitlab.com/bamboo/' } it 'returns a build URL' do @@ -198,13 +198,22 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching, feat context 'when Bamboo API returns an array of results and we only consider the last one' do let(:bamboo_response_template) do - %q({"results":{"results":{"size":"2","result":[{"buildState":"%{build_state}","planResultKey":{"key":"41"}},{"buildState":"%{build_state}","planResultKey":{"key":"42"}}]}}}) + '{"results":{"results":{"size":"2","result":[{"buildState":"%{build_state}","planResultKey":{"key":"41"}}, ' \ + '{"buildState":"%{build_state}","planResultKey":{"key":"42"}}]}}}' end it_behaves_like 'reactive cache calculation' end end + describe '#avatar_url' do + it 'returns the avatar image path' do + expect(subject.avatar_url).to eq(ActionController::Base.helpers.image_path( + 'illustrations/third-party-logos/integrations-logos/atlassian-bamboo.svg' + )) + end + end + def stub_update_and_build_request(status: 200, body: nil) bamboo_full_url = 'http://gitlab.com/bamboo/updateAndBuild.action?buildKey=foo&os_authType=basic' @@ -222,11 +231,11 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching, feat status: status, headers: { 'Content-Type' => 'application/json' }, body: body - ).with(basic_auth: %w(mic password)) + ).with(basic_auth: %w[mic password]) end def bamboo_response(build_state: 'success') # reference: https://docs.atlassian.com/atlassian-bamboo/REST/6.2.5/#d2e786 - bamboo_response_template % { build_state: build_state } + format(bamboo_response_template, build_state: build_state) end end diff --git a/spec/models/integrations/chat_message/alert_message_spec.rb b/spec/models/integrations/chat_message/alert_message_spec.rb index 162df1a774c..a9db9e14883 100644 --- a/spec/models/integrations/chat_message/alert_message_spec.rb +++ b/spec/models/integrations/chat_message/alert_message_spec.rb @@ -57,4 +57,10 @@ RSpec.describe Integrations::ChatMessage::AlertMessage do expect(time_item[:value]).to eq(expected_time) end end + + describe '#attachment_color' do + it 'returns the correct color' do + expect(subject.attachment_color).to eq('#C95823') + end + end end diff --git a/spec/models/integrations/chat_message/deployment_message_spec.rb b/spec/models/integrations/chat_message/deployment_message_spec.rb index 630ae902331..afbf1d1c0d1 100644 --- a/spec/models/integrations/chat_message/deployment_message_spec.rb +++ b/spec/models/integrations/chat_message/deployment_message_spec.rb @@ -19,6 +19,29 @@ RSpec.describe Integrations::ChatMessage::DeploymentMessage, feature_category: : it_behaves_like Integrations::ChatMessage + def deployment_data(params) + { + object_kind: "deployment", + status: "success", + deployable_id: 3, + deployable_url: "deployable_url", + environment: "sandbox", + project: { + name: "greatproject", + web_url: "project_web_url", + path_with_namespace: "project_path_with_namespace" + }, + user: { + name: "Jane Person", + username: "jane" + }, + user_url: "user_url", + short_sha: "12345678", + commit_url: "commit_url", + commit_title: "commit title text" + }.merge(params) + end + describe '#pretext' do it 'returns a message with the data returned by the deployment data builder' do expect(subject.pretext).to eq("Deploy to myenvironment succeeded") @@ -80,29 +103,6 @@ RSpec.describe Integrations::ChatMessage::DeploymentMessage, feature_category: : end describe '#attachments' do - def deployment_data(params) - { - object_kind: "deployment", - status: "success", - deployable_id: 3, - deployable_url: "deployable_url", - environment: "sandbox", - project: { - name: "greatproject", - web_url: "project_web_url", - path_with_namespace: "project_path_with_namespace" - }, - user: { - name: "Jane Person", - username: "jane" - }, - user_url: "user_url", - short_sha: "12345678", - commit_url: "commit_url", - commit_title: "commit title text" - }.merge(params) - end - context 'without markdown' do it 'returns attachments with the data returned by the deployment data builder' do job_url = Gitlab::Routing.url_helpers.project_job_url(project, ci_build) @@ -165,4 +165,23 @@ RSpec.describe Integrations::ChatMessage::DeploymentMessage, feature_category: : }]) end end + + describe '#attachment_color' do + using RSpec::Parameterized::TableSyntax + where(:status, :expected_color) do + 'success' | 'good' + 'canceled' | 'warning' + 'failed' | 'danger' + 'blub' | '#334455' + end + + with_them do + it 'returns the correct color' do + data = deployment_data(status: status) + message = described_class.new(data) + + expect(message.attachment_color).to eq(expected_color) + end + end + end end diff --git a/spec/models/integrations/chat_message/issue_message_spec.rb b/spec/models/integrations/chat_message/issue_message_spec.rb index 14451427a5a..7b09b5d08b0 100644 --- a/spec/models/integrations/chat_message/issue_message_spec.rb +++ b/spec/models/integrations/chat_message/issue_message_spec.rb @@ -125,4 +125,10 @@ RSpec.describe Integrations::ChatMessage::IssueMessage, feature_category: :integ end end end + + describe '#attachment_color' do + it 'returns the correct color' do + expect(subject.attachment_color).to eq('#C95823') + end + end end diff --git a/spec/models/integrations/chat_message/pipeline_message_spec.rb b/spec/models/integrations/chat_message/pipeline_message_spec.rb index 4d371ca0899..5eb3915018e 100644 --- a/spec/models/integrations/chat_message/pipeline_message_spec.rb +++ b/spec/models/integrations/chat_message/pipeline_message_spec.rb @@ -388,4 +388,31 @@ RSpec.describe Integrations::ChatMessage::PipelineMessage do ) end end + + describe '#attachment_color' do + context 'when success' do + before do + args[:object_attributes][:status] = 'success' + end + + it { expect(subject.attachment_color).to eq('good') } + end + + context 'when passed with warnings' do + before do + args[:object_attributes][:status] = 'success' + args[:object_attributes][:detailed_status] = 'passed with warnings' + end + + it { expect(subject.attachment_color).to eq('warning') } + end + + context 'when failed' do + before do + args[:object_attributes][:status] = 'failed' + end + + it { expect(subject.attachment_color).to eq('danger') } + end + end end diff --git a/spec/models/integrations/chat_message/push_message_spec.rb b/spec/models/integrations/chat_message/push_message_spec.rb index 5c9c5c64d7e..a9d0f801406 100644 --- a/spec/models/integrations/chat_message/push_message_spec.rb +++ b/spec/models/integrations/chat_message/push_message_spec.rb @@ -214,4 +214,10 @@ RSpec.describe Integrations::ChatMessage::PushMessage do end end end + + describe '#attachment_color' do + it 'returns the correct color' do + expect(subject.attachment_color).to eq('#345') + end + end end diff --git a/spec/models/integrations/discord_spec.rb b/spec/models/integrations/discord_spec.rb index 7ab7308ac1c..89c4dcd7e0e 100644 --- a/spec/models/integrations/discord_spec.rb +++ b/spec/models/integrations/discord_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" RSpec.describe Integrations::Discord, feature_category: :integrations do - it_behaves_like "chat integration", "Discord notifications" do + it_behaves_like "chat integration", "Discord notifications", supports_deployments: true do let(:client) { Discordrb::Webhooks::Client } let(:client_arguments) { { url: webhook_url } } let(:payload) do @@ -18,6 +18,8 @@ RSpec.describe Integrations::Discord, feature_category: :integrations do ] } end + + it_behaves_like 'supports group mentions', :discord_integration end describe 'validations' do @@ -77,7 +79,7 @@ RSpec.describe Integrations::Discord, feature_category: :integrations do icon_url: start_with('https://www.gravatar.com/avatar/'), name: user.name ), - color: 16543014, + color: 3359829, timestamp: Time.now.utc.iso8601 ) end diff --git a/spec/models/integrations/google_play_spec.rb b/spec/models/integrations/google_play_spec.rb index a0bc73378d3..c5b0c058809 100644 --- a/spec/models/integrations/google_play_spec.rb +++ b/spec/models/integrations/google_play_spec.rb @@ -20,8 +20,7 @@ RSpec.describe Integrations::GooglePlay, feature_category: :mobile_devops do it { is_expected.to allow_value('a.a.a').for(:package_name) } it { is_expected.to allow_value('com.example').for(:package_name) } it { is_expected.not_to allow_value('com').for(:package_name) } - it { is_expected.to allow_value(true, false).for(:google_play_protected_refs) } - it { is_expected.not_to allow_value(nil).for(:google_play_protected_refs) } + it { is_expected.to validate_inclusion_of(:google_play_protected_refs).in_array([true, false]) } it { is_expected.not_to allow_value('com.example.my app').for(:package_name) } it { is_expected.not_to allow_value('1com.example.myapp').for(:package_name) } it { is_expected.not_to allow_value('com.1example.myapp').for(:package_name) } diff --git a/spec/models/integrations/hangouts_chat_spec.rb b/spec/models/integrations/hangouts_chat_spec.rb index bcb80768ffb..a1ecfd436c2 100644 --- a/spec/models/integrations/hangouts_chat_spec.rb +++ b/spec/models/integrations/hangouts_chat_spec.rb @@ -4,7 +4,7 @@ require "spec_helper" RSpec.describe Integrations::HangoutsChat, feature_category: :integrations do it_behaves_like "chat integration", "Hangouts Chat" do - let(:client) { HangoutsChat::Sender } + let(:client) { Gitlab::HTTP } let(:client_arguments) { webhook_url } let(:payload) do { diff --git a/spec/models/integrations/integration_list_spec.rb b/spec/models/integrations/integration_list_spec.rb new file mode 100644 index 00000000000..b7ccbcecf6b --- /dev/null +++ b/spec/models/integrations/integration_list_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::IntegrationList, feature_category: :integrations do + let_it_be(:projects) { create_pair(:project, :small_repo) } + let(:batch) { Project.where(id: projects.pluck(:id)) } + let(:integration_hash) { { 'active' => 'true', 'category' => 'common' } } + let(:association) { 'project' } + + subject { described_class.new(batch, integration_hash, association) } + + describe '#to_array' do + it 'returns array of Integration, columns, and values' do + expect(subject.to_array).to eq([ + Integration, + %w[active category project_id], + [['true', 'common', projects.first.id], ['true', 'common', projects.second.id]] + ]) + end + end +end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 9bb77f6d6d4..c87128db221 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -597,7 +597,7 @@ RSpec.describe Integrations::Jira, feature_category: :integrations do it 'uses the default GitLab::HTTP timeouts' do timeouts = Gitlab::HTTP::DEFAULT_TIMEOUT_OPTIONS - expect(Gitlab::HTTP).to receive(:httparty_perform_request) + expect(Gitlab::HTTP_V2::Client).to receive(:httparty_perform_request) .with(Net::HTTP::Get, '/foo', hash_including(timeouts)).and_call_original jira_integration.client.get('/foo') @@ -1372,4 +1372,12 @@ RSpec.describe Integrations::Jira, feature_category: :integrations do end end end + + describe '#avatar_url' do + it 'returns the avatar image path' do + expect(subject.avatar_url).to eq( + ActionController::Base.helpers.image_path('illustrations/third-party-logos/integrations-logos/jira.svg') + ) + end + end end diff --git a/spec/models/integrations/pivotaltracker_spec.rb b/spec/models/integrations/pivotaltracker_spec.rb index bf8458a376c..babe9119ccf 100644 --- a/spec/models/integrations/pivotaltracker_spec.rb +++ b/spec/models/integrations/pivotaltracker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::Pivotaltracker do +RSpec.describe Integrations::Pivotaltracker, feature_category: :integrations do include StubRequests describe 'Validations' do @@ -93,4 +93,14 @@ RSpec.describe Integrations::Pivotaltracker do end end end + + describe '#avatar_url' do + it 'returns the avatar image path' do + expect(subject.avatar_url).to eq( + ActionController::Base.helpers.image_path( + 'illustrations/third-party-logos/integrations-logos/pivotal-tracker.svg' + ) + ) + end + end end diff --git a/spec/models/integrations/pushover_spec.rb b/spec/models/integrations/pushover_spec.rb index 8286fd20669..c576340a78a 100644 --- a/spec/models/integrations/pushover_spec.rb +++ b/spec/models/integrations/pushover_spec.rb @@ -62,4 +62,12 @@ RSpec.describe Integrations::Pushover do expect(WebMock).to have_requested(:post, 'https://8.8.8.8/1/messages.json').once end end + + describe '#avatar_url' do + it 'returns the avatar image path' do + expect(subject.avatar_url).to eq( + ActionController::Base.helpers.image_path('illustrations/third-party-logos/integrations-logos/pushover.svg') + ) + end + end end diff --git a/spec/models/integrations/slack_spec.rb b/spec/models/integrations/slack_spec.rb index 59ee3746d8f..0d82abd9e3d 100644 --- a/spec/models/integrations/slack_spec.rb +++ b/spec/models/integrations/slack_spec.rb @@ -9,4 +9,6 @@ RSpec.describe Integrations::Slack, feature_category: :integrations do stub_request(:post, integration.webhook) end end + + it_behaves_like 'supports group mentions', :integrations_slack end diff --git a/spec/models/integrations/telegram_spec.rb b/spec/models/integrations/telegram_spec.rb index c3a66c84f09..4c814dedd66 100644 --- a/spec/models/integrations/telegram_spec.rb +++ b/spec/models/integrations/telegram_spec.rb @@ -50,4 +50,12 @@ RSpec.describe Integrations::Telegram, feature_category: :integrations do end end end + + describe '#avatar_url' do + it 'returns the avatar image path' do + expect(subject.avatar_url).to eq( + ActionController::Base.helpers.image_path('illustrations/third-party-logos/integrations-logos/telegram.svg') + ) + end + end end diff --git a/spec/models/issue_link_spec.rb b/spec/models/issue_link_spec.rb index 9af667c2960..24f0b9f2a5c 100644 --- a/spec/models/issue_link_spec.rb +++ b/spec/models/issue_link_spec.rb @@ -7,7 +7,9 @@ RSpec.describe IssueLink, feature_category: :portfolio_management do it_behaves_like 'issuable link' do let_it_be_with_reload(:issuable_link) { create(:issue_link) } - let_it_be(:issuable) { create(:issue) } + let_it_be(:issuable) { create(:issue, project: project) } + let_it_be(:issuable2) { create(:issue, project: project) } + let_it_be(:issuable3) { create(:issue, project: project) } let(:issuable_class) { 'Issue' } let(:issuable_link_factory) { :issue_link } end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 4e217e3a9f7..e7a5a53c6a0 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -69,8 +69,7 @@ RSpec.describe Issue, feature_category: :team_planning do end describe 'validations' do - it { is_expected.not_to allow_value(nil).for(:confidential) } - it { is_expected.to allow_value(true, false).for(:confidential) } + it { is_expected.to validate_inclusion_of(:confidential).in_array([true, false]) } end describe 'custom validations' do @@ -302,7 +301,7 @@ RSpec.describe Issue, feature_category: :team_planning do let(:issue) { create(:issue) } let(:project) { issue.project } let(:user) { issue.author } - let(:action) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } + let(:event) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } let(:namespace) { project.namespace } subject(:service_action) { issue } @@ -867,6 +866,29 @@ RSpec.describe Issue, feature_category: :team_planning do .to contain_exactly(authorized_issue_b, authorized_incident_a) end end + + context 'when authorize argument is false' do + it 'returns all related issues' do + expect(authorized_issue_a.related_issues(authorize: false)) + .to contain_exactly(authorized_issue_b, authorized_issue_c, authorized_incident_a, unauthorized_issue) + end + end + + context 'when current_user argument is nil' do + let_it_be(:public_issue) { create(:issue, project: create(:project, :public)) } + + it 'returns public linked issues only' do + create(:issue_link, source: authorized_issue_a, target: public_issue) + + expect(authorized_issue_a.related_issues).to contain_exactly(public_issue) + end + end + + context 'when issue is a new record' do + let(:new_issue) { build(:issue, project: authorized_project) } + + it { expect(new_issue.related_issues(user)).to be_empty } + end end describe '#can_move?' do @@ -2038,4 +2060,134 @@ RSpec.describe Issue, feature_category: :team_planning do expect(issue.search_data.namespace_id).to eq(issue.namespace_id) end end + + describe '#linked_items_count' do + let_it_be(:issue1) { create(:issue, project: reusable_project) } + let_it_be(:issue2) { create(:issue, project: reusable_project) } + let_it_be(:issue3) { create(:issue, project: reusable_project) } + let_it_be(:issue4) { build(:issue, project: reusable_project) } + + it 'returns number of issues linked to the issue' do + create(:issue_link, source: issue1, target: issue2) + create(:issue_link, source: issue1, target: issue3) + + expect(issue1.linked_items_count).to eq(2) + expect(issue2.linked_items_count).to eq(1) + expect(issue3.linked_items_count).to eq(1) + expect(issue4.linked_items_count).to eq(0) + end + end + + describe '#readable_by?' do + let_it_be(:admin_user) { create(:user, :admin) } + + subject { issue_subject.readable_by?(user) } + + context 'when issue belongs directly to a project' do + let_it_be_with_reload(:project_issue) { create(:issue, project: reusable_project) } + let_it_be(:project_reporter) { create(:user).tap { |u| reusable_project.add_reporter(u) } } + let_it_be(:project_guest) { create(:user).tap { |u| reusable_project.add_guest(u) } } + + let(:issue_subject) { project_issue } + + context 'when user is in admin mode', :enable_admin_mode do + let(:user) { admin_user } + + it { is_expected.to be_truthy } + end + + context 'when user is a reporter' do + let(:user) { project_reporter } + + it { is_expected.to be_truthy } + + context 'when issues project feature is not enabled' do + before do + reusable_project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED) + end + + it { is_expected.to be_falsey } + end + + context 'when issue is hidden (banned author)' do + before do + issue_subject.author.ban! + end + + it { is_expected.to be_falsey } + end + end + + context 'when user is a guest' do + let(:user) { project_guest } + + context 'when issue is confidential' do + before do + issue_subject.update!(confidential: true) + end + + it { is_expected.to be_falsey } + + context 'when user is assignee of the issue' do + before do + issue_subject.update!(assignees: [user]) + end + + it { is_expected.to be_truthy } + end + end + end + end + + context 'when issue belongs directly to the group' do + let_it_be(:group) { create(:group) } + let_it_be_with_reload(:group_issue) { create(:issue, :group_level, namespace: group) } + let_it_be(:group_reporter) { create(:user).tap { |u| group.add_reporter(u) } } + let_it_be(:group_guest) { create(:user).tap { |u| group.add_guest(u) } } + + let(:issue_subject) { group_issue } + + context 'when user is in admin mode', :enable_admin_mode do + let(:user) { admin_user } + + it { is_expected.to be_truthy } + end + + context 'when user is a reporter' do + let(:user) { group_reporter } + + it { is_expected.to be_truthy } + + context 'when issue is hidden (banned author)' do + before do + issue_subject.author.ban! + end + + it { is_expected.to be_falsey } + end + end + + context 'when user is a guest' do + let(:user) { group_guest } + + it { is_expected.to be_truthy } + + context 'when issue is confidential' do + before do + issue_subject.update!(confidential: true) + end + + it { is_expected.to be_falsey } + + context 'when user is assignee of the issue' do + before do + issue_subject.update!(assignees: [user]) + end + + it { is_expected.to be_truthy } + end + end + end + end + end end diff --git a/spec/models/lfs_download_object_spec.rb b/spec/models/lfs_download_object_spec.rb index d82e432b7d6..f69c6efb0a7 100644 --- a/spec/models/lfs_download_object_spec.rb +++ b/spec/models/lfs_download_object_spec.rb @@ -24,6 +24,19 @@ RSpec.describe LfsDownloadObject do end end + describe '#to_hash' do + it 'returns specified Hash' do + expected_hash = { + 'oid' => oid, + 'size' => size, + 'link' => link, + 'headers' => headers + } + + expect(subject.to_hash).to eq(expected_hash) + end + end + describe '#has_authorization_header?' do it 'returns false' do expect(subject.has_authorization_header?).to be false diff --git a/spec/models/loose_foreign_keys/deleted_record_spec.rb b/spec/models/loose_foreign_keys/deleted_record_spec.rb index ed80f5c1516..619f77b6bec 100644 --- a/spec/models/loose_foreign_keys/deleted_record_spec.rb +++ b/spec/models/loose_foreign_keys/deleted_record_spec.rb @@ -16,30 +16,20 @@ RSpec.describe LooseForeignKeys::DeletedRecord, type: :model, feature_category: let(:records) { described_class.load_batch_for_table(table, 10) } describe '.load_batch_for_table' do - where(:union_feature_flag_value) do - [true, false] + it 'loads records and orders them by creation date' do + expect(records).to eq([deleted_record_1, deleted_record_2, deleted_record_4]) end - with_them do - before do - stub_feature_flags('loose_foreign_keys_batch_load_using_union' => union_feature_flag_value) - end - - it 'loads records and orders them by creation date' do - expect(records).to eq([deleted_record_1, deleted_record_2, deleted_record_4]) - end + it 'supports configurable batch size' do + records = described_class.load_batch_for_table(table, 2) - it 'supports configurable batch size' do - records = described_class.load_batch_for_table(table, 2) - - expect(records).to eq([deleted_record_1, deleted_record_2]) - end + expect(records).to eq([deleted_record_1, deleted_record_2]) + end - it 'returns the partition number in each returned record' do - records = described_class.load_batch_for_table(table, 4) + it 'returns the partition number in each returned record' do + records = described_class.load_batch_for_table(table, 4) - expect(records).to all(have_attributes(partition: (a_value > 0))) - end + expect(records).to all(have_attributes(partition: (a_value > 0))) end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 6dd5f9dec8c..fdd8a610fe4 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -16,7 +16,6 @@ RSpec.describe Member, feature_category: :groups_and_projects do describe 'Associations' do it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:member_namespace) } - it { is_expected.to have_one(:member_task) } end describe 'Validation' do @@ -883,17 +882,6 @@ RSpec.describe Member, feature_category: :groups_and_projects do expect(member.invite_token).not_to be_nil expect_any_instance_of(described_class).not_to receive(:after_accept_invite) end - - it 'schedules a TasksToBeDone::CreateWorker task' do - member_task = create(:member_task, member: member, project: member.project) - - expect(TasksToBeDone::CreateWorker) - .to receive(:perform_async) - .with(member_task.id, member.created_by_id, [user.id]) - .once - - member.accept_invite!(user) - end end describe '#decline_invite!' do diff --git a/spec/models/members/last_group_owner_assigner_spec.rb b/spec/models/members/last_group_owner_assigner_spec.rb index 2539388c667..5e135665585 100644 --- a/spec/models/members/last_group_owner_assigner_spec.rb +++ b/spec/models/members/last_group_owner_assigner_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe LastGroupOwnerAssigner do +RSpec.describe LastGroupOwnerAssigner, feature_category: :groups_and_projects do describe "#execute" do let_it_be(:user, reload: true) { create(:user) } let_it_be(:group) { create(:group) } diff --git a/spec/models/members/member_task_spec.rb b/spec/models/members/member_task_spec.rb deleted file mode 100644 index b06aa05c255..00000000000 --- a/spec/models/members/member_task_spec.rb +++ /dev/null @@ -1,124 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe MemberTask do - describe 'Associations' do - it { is_expected.to belong_to(:member) } - it { is_expected.to belong_to(:project) } - end - - describe 'Validations' do - it { is_expected.to validate_presence_of(:member) } - it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_inclusion_of(:tasks).in_array(MemberTask::TASKS.values) } - - describe 'unique tasks validation' do - subject do - build(:member_task, tasks: [0, 0]) - end - - it 'expects the task values to be unique' do - expect(subject).to be_invalid - expect(subject.errors[:tasks]).to include('are not unique') - end - end - - describe 'project validations' do - let_it_be(:project) { create(:project) } - - subject do - build(:member_task, member: member, project: project, tasks_to_be_done: [:ci, :code]) - end - - context 'when the member source is a group' do - let_it_be(:member) { create(:group_member) } - - it "expects the project to be part of the member's group projects" do - expect(subject).to be_invalid - expect(subject.errors[:project]).to include('is not in the member group') - end - - context "when the project is part of the member's group projects" do - let_it_be(:project) { create(:project, namespace: member.source) } - - it { is_expected.to be_valid } - end - end - - context 'when the member source is a project' do - let_it_be(:member) { create(:project_member) } - - it "expects the project to be the member's project" do - expect(subject).to be_invalid - expect(subject.errors[:project]).to include('is not the member project') - end - - context "when the project is the member's project" do - let_it_be(:project) { member.source } - - it { is_expected.to be_valid } - end - end - end - end - - describe '.for_members' do - it 'returns the member_tasks for multiple members' do - member1 = create(:group_member) - member_task1 = create(:member_task, member: member1) - create(:member_task) - expect(described_class.for_members([member1])).to match_array([member_task1]) - end - end - - describe '#tasks_to_be_done' do - subject { member_task.tasks_to_be_done } - - let_it_be(:member_task) { build(:member_task) } - - before do - member_task[:tasks] = [0, 1] - end - - it 'returns an array of symbols for the corresponding integers' do - expect(subject).to match_array([:ci, :code]) - end - end - - describe '#tasks_to_be_done=' do - let_it_be(:member_task) { build(:member_task) } - - context 'when passing valid values' do - subject { member_task[:tasks] } - - before do - member_task.tasks_to_be_done = tasks - end - - context 'when passing tasks as strings' do - let_it_be(:tasks) { %w(ci code) } - - it 'sets an array of integers for the corresponding tasks' do - expect(subject).to match_array([0, 1]) - end - end - - context 'when passing a single task' do - let_it_be(:tasks) { :ci } - - it 'sets an array of integers for the corresponding tasks' do - expect(subject).to match_array([1]) - end - end - - context 'when passing a task twice' do - let_it_be(:tasks) { %w(ci ci) } - - it 'is set only once' do - expect(subject).to match_array([1]) - end - end - end - end -end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index bf9af73fe1b..806ce3f21b5 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -51,6 +51,7 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do it { expect(subject.head_commit_sha).to eq('b83d6e391c22777fca1ed3012fce84f633d7fed0') } it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') } it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') } + it { expect(subject.patch_id_sha).to eq('1e05e04d4c2a6414d9d4ab38208511a3bbe715f2') } context 'when diff_type is merge_head' do let_it_be(:merge_request) { create(:merge_request) } @@ -703,6 +704,39 @@ RSpec.describe MergeRequestDiff, feature_category: :code_review_workflow do end end + describe "#set_patch_id_sha" do + let(:mr_diff) { create(:merge_request).merge_request_diff } + + it "sets the patch_id_sha attribute" do + expect(mr_diff.set_patch_id_sha).not_to be_nil + end + + context "when base_commit_sha is nil" do + it "records patch_id_sha as nil" do + expect(mr_diff).to receive(:base_commit_sha).and_return(nil) + + expect(mr_diff.set_patch_id_sha).to be_nil + end + end + + context "when head_commit_sha is nil" do + it "records patch_id_sha as nil" do + expect(mr_diff).to receive(:head_commit_sha).and_return(nil) + + expect(mr_diff.set_patch_id_sha).to be_nil + end + end + + context "when head_commit_sha and base_commit_sha match" do + it "records patch_id_sha as nil" do + expect(mr_diff).to receive(:base_commit_sha).at_least(:once).and_return("abc123") + expect(mr_diff).to receive(:head_commit_sha).at_least(:once).and_return("abc123") + + expect(mr_diff.set_patch_id_sha).to be_nil + end + end + end + describe '#save_diffs' do it 'saves collected state' do mr_diff = create(:merge_request).merge_request_diff diff --git a/spec/models/merge_request_reviewer_spec.rb b/spec/models/merge_request_reviewer_spec.rb index 5a29966e4b9..fb1e43a426d 100644 --- a/spec/models/merge_request_reviewer_spec.rb +++ b/spec/models/merge_request_reviewer_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MergeRequestReviewer do +RSpec.describe MergeRequestReviewer, feature_category: :code_review_workflow do let(:reviewer) { create(:user) } let(:merge_request) { create(:merge_request) } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index b36737fc19d..40f85c92851 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -570,6 +570,16 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end + describe '.by_merged_commit_sha' do + it 'returns merge requests that match the given merged commit' do + mr = create(:merge_request, :merged, merged_commit_sha: '123abc') + + create(:merge_request, :merged, merged_commit_sha: '123def') + + expect(described_class.by_merged_commit_sha('123abc')).to eq([mr]) + end + end + describe '.by_merge_commit_sha' do it 'returns merge requests that match the given merge commit' do mr = create(:merge_request, :merged, merge_commit_sha: '123abc') @@ -591,16 +601,18 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end - describe '.by_merge_or_squash_commit_sha' do - subject { described_class.by_merge_or_squash_commit_sha([sha1, sha2]) } + describe '.by_merged_or_merge_or_squash_commit_sha' do + subject { described_class.by_merged_or_merge_or_squash_commit_sha([sha1, sha2, sha3]) } let(:sha1) { '123abc' } let(:sha2) { '456abc' } + let(:sha3) { '111111' } let(:mr1) { create(:merge_request, :merged, squash_commit_sha: sha1) } let(:mr2) { create(:merge_request, :merged, merge_commit_sha: sha2) } + let(:mr3) { create(:merge_request, :merged, merged_commit_sha: sha3) } - it 'returns merge requests that match the given squash and merge commits' do - is_expected.to include(mr1, mr2) + it 'returns merge requests that match the given squash, merge and merged commits' do + is_expected.to include(mr1, mr2, mr3) end end @@ -644,6 +656,13 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev it { is_expected.to eq([merge_request]) } end + context 'when commit is a rebased fast-forward commit' do + let!(:merge_request) { create(:merge_request, :merged, merged_commit_sha: sha) } + let(:sha) { '123abc' } + + it { is_expected.to eq([merge_request]) } + end + context 'when commit is not found' do let(:sha) { '0000' } @@ -2416,6 +2435,19 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev expect(merge_request.has_terraform_reports?).to be_falsey end end + + context 'when head pipeline is not finished and has terraform reports' do + before do + stub_feature_flags(mr_show_reports_immediately: false) + end + + it 'returns true' do + merge_request = create(:merge_request, :with_terraform_reports) + merge_request.actual_head_pipeline.update!(status: :running) + + expect(merge_request.has_terraform_reports?).to be_truthy + end + end end describe '#has_sast_reports?' do @@ -3474,6 +3506,10 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev it 'returns false' do expect(subject.mergeable_state?).to be_falsey end + + it 'returns true when skipping draft check' do + expect(subject.mergeable_state?(skip_draft_check: true)).to be(true) + end end context 'when broken' do @@ -4554,7 +4590,7 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev describe '#unlock_mr' do subject { create(:merge_request, state: 'locked', source_project: project, merge_jid: 123) } - it 'updates merge request head pipeline and sets merge_jid to nil', :sidekiq_might_not_need_inline do + it 'updates merge request head pipeline and sets merge_jid to nil', :sidekiq_inline do pipeline = create(:ci_empty_pipeline, project: subject.project, ref: subject.source_branch, sha: subject.source_branch_sha) subject.unlock_mr @@ -5956,4 +5992,77 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev it { is_expected.to eq(expected) } end end + + describe '#current_patch_id_sha' do + let(:merge_request) { build_stubbed(:merge_request) } + let(:merge_request_diff) { build_stubbed(:merge_request_diff) } + let(:patch_id) { 'ghi789' } + + 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_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_next_instance_of(Repository) do |repo| + allow(repo) + .to receive(:get_patch_id) + .with(diff_refs.base_sha, diff_refs.head_sha) + .and_return(patch_id) + end + 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 + let(:merge_request) { build_stubbed(:merge_request) } + let(:result) { instance_double(ServiceResponse, payload: { results: ['result'] }) } + + it 'executes MergeRequests::Mergeability::RunChecksService with all mergeability checks' do + expect_next_instance_of( + MergeRequests::Mergeability::RunChecksService, + merge_request: merge_request, + params: {} + ) do |svc| + expect(svc) + .to receive(:execute) + .with(described_class.all_mergeability_checks, execute_all: true) + .and_return(result) + end + + expect(merge_request.all_mergeability_checks_results).to eq(result.payload[:results]) + end + end end diff --git a/spec/models/ml/model_spec.rb b/spec/models/ml/model_spec.rb index 42d8ed5c0c5..e22989f3ce2 100644 --- a/spec/models/ml/model_spec.rb +++ b/spec/models/ml/model_spec.rb @@ -118,4 +118,47 @@ RSpec.describe Ml::Model, feature_category: :mlops do end end end + + describe 'with_version_count' do + let(:model) { existing_model } + + subject { described_class.with_version_count.find_by(id: model.id).version_count } + + context 'when model has versions' do + before do + create(:ml_model_versions, model: model) + end + + it { is_expected.to eq(1) } + end + + context 'when model has no versions' do + let(:model) { another_existing_model } + + it { is_expected.to eq(0) } + end + end + + describe '#by_project_and_id' do + let(:id) { existing_model.id } + let(:project_id) { existing_model.project.id } + + subject { described_class.by_project_id_and_id(project_id, id) } + + context 'if exists' do + it { is_expected.to eq(existing_model) } + end + + context 'if id has no match' do + let(:id) { non_existing_record_id } + + it { is_expected.to be(nil) } + end + + context 'if project id does not match' do + let(:project_id) { non_existing_record_id } + + it { is_expected.to be(nil) } + end + end end diff --git a/spec/models/namespace/package_setting_spec.rb b/spec/models/namespace/package_setting_spec.rb index f3fda200fda..e6096bc9267 100644 --- a/spec/models/namespace/package_setting_spec.rb +++ b/spec/models/namespace/package_setting_spec.rb @@ -11,13 +11,9 @@ RSpec.describe Namespace::PackageSetting, feature_category: :package_registry do it { is_expected.to validate_presence_of(:namespace) } describe '#maven_duplicates_allowed' do - it { is_expected.to allow_value(true, false).for(:maven_duplicates_allowed) } - it { is_expected.not_to allow_value(nil).for(:maven_duplicates_allowed) } - it { is_expected.to allow_value(true, false).for(:generic_duplicates_allowed) } - it { is_expected.not_to allow_value(nil).for(:generic_duplicates_allowed) } - it { is_expected.to allow_value(true).for(:nuget_duplicates_allowed) } - it { is_expected.to allow_value(false).for(:nuget_duplicates_allowed) } - it { is_expected.not_to allow_value(nil).for(:nuget_duplicates_allowed) } + it { is_expected.to validate_inclusion_of(:maven_duplicates_allowed).in_array([true, false]) } + it { is_expected.to validate_inclusion_of(:generic_duplicates_allowed).in_array([true, false]) } + it { is_expected.to validate_inclusion_of(:nuget_duplicates_allowed).in_array([true, false]) } end describe 'regex values' do diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index a937a3e8988..e9822d97447 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -12,7 +12,7 @@ RSpec.describe NamespaceSetting, feature_category: :groups_and_projects, type: : end it { is_expected.to define_enum_for(:jobs_to_be_done).with_values([:basics, :move_repository, :code_storage, :exploring, :ci, :other]).with_suffix } - it { is_expected.to define_enum_for(:enabled_git_access_protocol).with_values([:all, :ssh, :http]).with_suffix } + it { is_expected.to define_enum_for(:enabled_git_access_protocol).with_suffix } describe 'default values' do subject(:setting) { described_class.new } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index a0deee0f2d3..9974aac3c6c 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -206,18 +206,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do expect { parent.update!(name: 'Foo') }.not_to raise_error end end - - context 'when restrict_special_characters_in_namespace_path feature flag is disabled' do - before do - stub_feature_flags(restrict_special_characters_in_namespace_path: false) - end - - it 'allows special character at the start or end of project namespace path' do - namespace = build(:namespace, type: project_sti_name, parent: parent, path: '_path_') - - expect(namespace).to be_valid - end - end end describe '1 char path length' do @@ -673,23 +661,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end context 'traversal scopes' do - context 'recursive' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it_behaves_like 'namespace traversal scopes' - end - - context 'linear' do - it_behaves_like 'namespace traversal scopes' - end - - shared_examples 'makes recursive queries' do - specify do - expect { subject }.to make_queries_matching(/WITH RECURSIVE/) - end - end + it_behaves_like 'namespace traversal scopes' shared_examples 'does not make recursive queries' do specify do @@ -703,14 +675,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do subject { described_class.where(id: namespace).self_and_descendants.load } it_behaves_like 'does not make recursive queries' - - context 'when feature flag :use_traversal_ids is disabled' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it_behaves_like 'makes recursive queries' - end end describe '.self_and_descendant_ids' do @@ -719,14 +683,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do subject { described_class.where(id: namespace).self_and_descendant_ids.load } it_behaves_like 'does not make recursive queries' - - context 'when feature flag :use_traversal_ids is disabled' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it_behaves_like 'makes recursive queries' - end end end @@ -845,6 +801,14 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do describe '#human_name' do it { expect(namespace.human_name).to eq(namespace.owner_name) } + + context 'when the owner is missing' do + before do + namespace.update_column(:owner_id, non_existing_record_id) + end + + it { expect(namespace.human_name).to eq(namespace.path) } + end end describe '#any_project_has_container_registry_tags?' do @@ -1207,70 +1171,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end - describe '#move_dir', :request_store do - context 'hashed storage' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:project) { create(:project_empty_repo, namespace: namespace) } - - context 'when any project has container images' do - let(:container_repository) { create(:container_repository) } - - before do - stub_container_registry_config(enabled: true) - stub_container_registry_tags(repository: :any, tags: ['tag']) - - create(:project, namespace: namespace, container_repositories: [container_repository]) - - allow(namespace).to receive(:path_was).and_return(namespace.path) - allow(namespace).to receive(:path).and_return('new_path') - allow(namespace).to receive(:first_project_with_container_registry_tags).and_return(project) - end - - it 'raises an error about not movable project' do - expect { namespace.move_dir }.to raise_error( - Gitlab::UpdatePathError, /Namespace .* cannot be moved/ - ) - end - end - - it "repository directory remains unchanged if path changed" do - before_disk_path = project.disk_path - namespace.update!(path: namespace.full_path + '_new') - - expect(before_disk_path).to eq(project.disk_path) - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy - end - end - - context 'for each project inside the namespace' do - let!(:parent) { create(:group, name: 'mygroup', path: 'mygroup') } - let!(:subgroup) { create(:group, name: 'mysubgroup', path: 'mysubgroup', parent: parent) } - let!(:project_in_parent_group) { create(:project, :legacy_storage, :repository, namespace: parent, name: 'foo1') } - let!(:hashed_project_in_subgroup) { create(:project, :repository, namespace: subgroup, name: 'foo2') } - let!(:legacy_project_in_subgroup) { create(:project, :legacy_storage, :repository, namespace: subgroup, name: 'foo3') } - - it 'updates project full path in .git/config' do - parent.update!(path: 'mygroup_new') - - expect(project_in_parent_group.reload.repository.full_path).to eq "mygroup_new/#{project_in_parent_group.path}" - expect(hashed_project_in_subgroup.reload.repository.full_path).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}" - expect(legacy_project_in_subgroup.reload.repository.full_path).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}" - end - - it 'updates the project storage location' do - repository_project_in_parent_group = project_in_parent_group.project_repository - repository_hashed_project_in_subgroup = hashed_project_in_subgroup.project_repository - repository_legacy_project_in_subgroup = legacy_project_in_subgroup.project_repository - - parent.update!(path: 'mygroup_moved') - - expect(repository_project_in_parent_group.reload.disk_path).to eq "mygroup_moved/#{project_in_parent_group.path}" - expect(repository_hashed_project_in_subgroup.reload.disk_path).to eq hashed_project_in_subgroup.disk_path - expect(repository_legacy_project_in_subgroup.reload.disk_path).to eq "mygroup_moved/mysubgroup/#{legacy_project_in_subgroup.path}" - end - end - end - describe '.find_by_path_or_name' do before do @namespace = create(:namespace, name: 'WoW', path: 'woW') @@ -1360,30 +1260,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do it { is_expected.to eq false } end - describe '#use_traversal_ids?' do - let_it_be(:namespace, reload: true) { create(:namespace) } - - subject { namespace.use_traversal_ids? } - - context 'when use_traversal_ids feature flag is true' do - before do - stub_feature_flags(use_traversal_ids: true) - end - - it { is_expected.to eq true } - - it_behaves_like 'disabled feature flag when traversal_ids is blank' - end - - context 'when use_traversal_ids feature flag is false' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it { is_expected.to eq false } - end - end - describe '#users_with_descendants' do let(:user_a) { create(:user) } let(:user_b) { create(:user) } @@ -1487,28 +1363,14 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end describe '#all_projects' do - context 'with use_traversal_ids feature flag enabled' do - before do - stub_feature_flags(use_traversal_ids: true) - end - - include_examples '#all_projects' - - # Using #self_and_descendant instead of #self_and_descendant_ids can produce - # very slow queries. - it 'calls self_and_descendant_ids' do - namespace = create(:group) - expect(namespace).to receive(:self_and_descendant_ids) - namespace.all_projects - end - end - - context 'with use_traversal_ids feature flag disabled' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - include_examples '#all_projects' + include_examples '#all_projects' + + # Using #self_and_descendant instead of #self_and_descendant_ids can produce + # very slow queries. + it 'calls self_and_descendant_ids' do + namespace = create(:group) + expect(namespace).to receive(:self_and_descendant_ids) + namespace.all_projects end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 2b26c73aa7a..5aa3ac3a2ea 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -81,6 +81,14 @@ RSpec.describe Note, feature_category: :team_planning do end end + context 'when noteable is an abuse report' do + subject { build(:note, noteable: build_stubbed(:abuse_report), project: nil, namespace: nil) } + + it 'is valid without project or namespace' do + is_expected.to be_valid + end + end + describe 'max notes limit' do let_it_be(:noteable) { create(:issue) } let_it_be(:existing_note) { create(:note, project: noteable.project, noteable: noteable) } @@ -314,34 +322,59 @@ RSpec.describe Note, feature_category: :team_planning do end describe '#ensure_namespace_id' do + context 'for issues' do + let!(:issue) { create(:issue) } + + it 'copies the namespace_id of the issue' do + note = build(:note, noteable: issue) + + note.valid? + + expect(note.namespace_id).to eq(issue.namespace_id) + end + end + + context 'for group-level work items' do + let!(:group) { create(:group) } + let!(:work_item) { create(:work_item, namespace: group) } + + it 'copies the namespace_id of the work item' do + note = build(:note, noteable: work_item) + + note.valid? + + expect(note.namespace_id).to eq(group.id) + end + end + context 'for a project noteable' do - let_it_be(:issue) { create(:issue) } + let_it_be(:merge_request) { create(:merge_request) } it 'copies the project_namespace_id of the project' do - note = build(:note, noteable: issue, project: issue.project) + note = build(:note, noteable: merge_request, project: merge_request.project) note.valid? - expect(note.namespace_id).to eq(issue.project.project_namespace_id) + expect(note.namespace_id).to eq(merge_request.project.project_namespace_id) end context 'when noteable is changed' do - let_it_be(:another_issue) { create(:issue) } + let_it_be(:another_mr) { create(:merge_request) } it 'updates the namespace_id' do - note = create(:note, noteable: issue, project: issue.project) + note = create(:note, noteable: merge_request, project: merge_request.project) - note.noteable = another_issue - note.project = another_issue.project + note.noteable = another_mr + note.project = another_mr.project note.valid? - expect(note.namespace_id).to eq(another_issue.project.project_namespace_id) + expect(note.namespace_id).to eq(another_mr.project.project_namespace_id) end end context 'when project is missing' do it 'does not raise an exception' do - note = build(:note, noteable: issue, project: nil) + note = build(:note, noteable: merge_request, project: nil) expect { note.valid? }.not_to raise_error end @@ -1325,6 +1358,20 @@ RSpec.describe Note, feature_category: :team_planning do end end + describe '#for_abuse_report' do + it 'is true when the noteable is an abuse report' do + note = build(:note, noteable: build(:abuse_report)) + + expect(note).to be_for_abuse_report + end + + it 'is not true when the noteable is not an abuse report' do + note = build(:note, noteable: build(:design)) + + expect(note).not_to be_for_abuse_report + end + end + describe '#to_ability_name' do it 'returns note' do expect(build(:note).to_ability_name).to eq('note') diff --git a/spec/models/packages/build_info_spec.rb b/spec/models/packages/build_info_spec.rb index db8ac605d72..9bb8062005a 100644 --- a/spec/models/packages/build_info_spec.rb +++ b/spec/models/packages/build_info_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Packages::BuildInfo, type: :model do +RSpec.describe Packages::BuildInfo, type: :model, feature_category: :package_registry do describe 'relationships' do it { is_expected.to belong_to(:package) } it { is_expected.to belong_to(:pipeline) } diff --git a/spec/models/packages/protection/rule_spec.rb b/spec/models/packages/protection/rule_spec.rb index b368687e6d8..320c265239c 100644 --- a/spec/models/packages/protection/rule_spec.rb +++ b/spec/models/packages/protection/rule_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :package_registry do + using RSpec::Parameterized::TableSyntax + it_behaves_like 'having unique enum values' describe 'relationships' do @@ -10,9 +12,19 @@ RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :pack end describe 'enums' do - describe '#package_type' do - it { is_expected.to define_enum_for(:package_type).with_values(npm: Packages::Package.package_types[:npm]) } - end + it { is_expected.to define_enum_for(:package_type).with_values(npm: Packages::Package.package_types[:npm]) } + + it { + is_expected.to( + define_enum_for(:push_protected_up_to_access_level) + .with_values( + developer: Gitlab::Access::DEVELOPER, + maintainer: Gitlab::Access::MAINTAINER, + owner: Gitlab::Access::OWNER + ) + .with_prefix(:push_protected_up_to) + ) + } end describe 'validations' do @@ -30,11 +42,219 @@ RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :pack describe '#push_protected_up_to_access_level' do it { is_expected.to validate_presence_of(:push_protected_up_to_access_level) } + end + end + + describe 'before_save' do + describe '#set_package_name_pattern_ilike_query' do + subject { create(:package_protection_rule, package_name_pattern: package_name_pattern) } + + context 'with different package name patterns' do + where(:package_name_pattern, :expected_pattern_query) do + '@my-scope/my-package' | '@my-scope/my-package' + '*@my-scope/my-package-with-wildcard-start' | '%@my-scope/my-package-with-wildcard-start' + '@my-scope/my-package-with-wildcard-end*' | '@my-scope/my-package-with-wildcard-end%' + '@my-scope/*my-package-with-wildcard-inbetween' | '@my-scope/%my-package-with-wildcard-inbetween' + '**@my-scope/**my-package-with-wildcard-multiple**' | '%%@my-scope/%%my-package-with-wildcard-multiple%%' + '@my-scope/my-package-with_____underscore' | '@my-scope/my-package-with\_\_\_\_\_underscore' + '@my-scope/my-package-with-percent-sign-%' | '@my-scope/my-package-with-percent-sign-\%' + '@my-scope/my-package-with-regex-characters.+' | '@my-scope/my-package-with-regex-characters.+' + end + + with_them do + it { is_expected.to have_attributes(package_name_pattern_ilike_query: expected_pattern_query) } + end + end + end + end + + describe '.for_package_name' do + let_it_be(:package_protection_rule) do + create(:package_protection_rule, package_name_pattern: '@my-scope/my_package') + end + + let_it_be(:ppr_with_wildcard_start) do + create(:package_protection_rule, package_name_pattern: '*@my-scope/my_package-with-wildcard-start') + end + + let_it_be(:ppr_with_wildcard_end) do + create(:package_protection_rule, package_name_pattern: '@my-scope/my_package-with-wildcard-end*') + end + + let_it_be(:ppr_with_wildcard_inbetween) do + create(:package_protection_rule, package_name_pattern: '@my-scope/*my_package-with-wildcard-inbetween') + end + + let_it_be(:ppr_with_wildcard_multiples) do + create(:package_protection_rule, package_name_pattern: '**@my-scope/**my_package-with-wildcard-multiple**') + end + + let_it_be(:ppr_with_underscore) do + create(:package_protection_rule, package_name_pattern: '@my-scope/my_package-with_____underscore') + end + + let_it_be(:ppr_with_regex_characters) do + create(:package_protection_rule, package_name_pattern: '@my-scope/my_package-with-regex-characters.+') + end + + let(:package_name) { package_protection_rule.package_name_pattern } + + subject { described_class.for_package_name(package_name) } + + context 'with several package protection rule scenarios' do + where(:package_name, :expected_package_protection_rules) do + '@my-scope/my_package' | [ref(:package_protection_rule)] + '@my-scope/my2package' | [] + '@my-scope/my_package-2' | [] + + # With wildcard pattern at the start + '@my-scope/my_package-with-wildcard-start' | [ref(:ppr_with_wildcard_start)] + '@my-scope/my_package-with-wildcard-start-any' | [] + 'prefix-@my-scope/my_package-with-wildcard-start' | [ref(:ppr_with_wildcard_start)] + 'prefix-@my-scope/my_package-with-wildcard-start-any' | [] + + # With wildcard pattern at the end + '@my-scope/my_package-with-wildcard-end' | [ref(:ppr_with_wildcard_end)] + '@my-scope/my_package-with-wildcard-end:1234567890' | [ref(:ppr_with_wildcard_end)] + 'prefix-@my-scope/my_package-with-wildcard-end' | [] + 'prefix-@my-scope/my_package-with-wildcard-end:1234567890' | [] + + # With wildcard pattern inbetween + '@my-scope/my_package-with-wildcard-inbetween' | [ref(:ppr_with_wildcard_inbetween)] + '@my-scope/any-my_package-with-wildcard-inbetween' | [ref(:ppr_with_wildcard_inbetween)] + '@my-scope/any-my_package-my_package-wildcard-inbetween-any' | [] + + # With multiple wildcard pattern are used + '@my-scope/my_package-with-wildcard-multiple' | [ref(:ppr_with_wildcard_multiples)] + 'prefix-@my-scope/any-my_package-with-wildcard-multiple-any' | [ref(:ppr_with_wildcard_multiples)] + '****@my-scope/****my_package-with-wildcard-multiple****' | [ref(:ppr_with_wildcard_multiples)] + 'prefix-@other-scope/any-my_package-with-wildcard-multiple-any' | [] + + # With underscore + '@my-scope/my_package-with_____underscore' | [ref(:ppr_with_underscore)] + '@my-scope/my_package-with_any_underscore' | [] + + '@my-scope/my_package-with-regex-characters.+' | [ref(:ppr_with_regex_characters)] + '@my-scope/my_package-with-regex-characters.' | [] + '@my-scope/my_package-with-regex-characters' | [] + '@my-scope/my_package-with-regex-characters-any' | [] + + # Special cases + nil | [] + '' | [] + 'any_package' | [] + end + + with_them do + it { is_expected.to match_array(expected_package_protection_rules) } + end + end + + context 'with multiple matching package protection rules' do + let!(:package_protection_rule_second_match) do + create(:package_protection_rule, package_name_pattern: "#{package_name}*") + end + + it { is_expected.to contain_exactly(package_protection_rule_second_match, package_protection_rule) } + end + end + + describe '.push_protected_from?' do + let_it_be(:project_with_ppr) { create(:project) } + let_it_be(:project_without_ppr) { create(:project) } + + let_it_be(:ppr_for_developer) do + create(:package_protection_rule, + package_name_pattern: '@my-scope/my-package-stage*', + project: project_with_ppr, + package_type: :npm, + push_protected_up_to_access_level: :developer + ) + end + + let_it_be(:ppr_for_maintainer) do + create(:package_protection_rule, + package_name_pattern: '@my-scope/my-package-prod*', + project: project_with_ppr, + package_type: :npm, + push_protected_up_to_access_level: :maintainer + ) + end + + let_it_be(:ppr_owner) do + create(:package_protection_rule, + package_name_pattern: '@my-scope/my-package-release*', + project: project_with_ppr, + package_type: :npm, + push_protected_up_to_access_level: :owner + ) + end + + let_it_be(:ppr_2_for_developer) do + create(:package_protection_rule, + package_name_pattern: '@my-scope/my-package-*', + project: project_with_ppr, + package_type: :npm, + push_protected_up_to_access_level: :developer + ) + end + + subject do + project + .package_protection_rules + .push_protected_from?( + access_level: access_level, + package_name: package_name, + package_type: package_type + ) + end + + describe "with different users and protection levels" do + # rubocop:disable Layout/LineLength + where(:project, :access_level, :package_name, :package_type, :push_protected) do + ref(:project_with_ppr) | Gitlab::Access::REPORTER | '@my-scope/my-package-stage-sha-1234' | :npm | true + ref(:project_with_ppr) | :developer | '@my-scope/my-package-stage-sha-1234' | :npm | true + ref(:project_with_ppr) | :maintainer | '@my-scope/my-package-stage-sha-1234' | :npm | false + ref(:project_with_ppr) | :maintainer | '@my-scope/my-package-stage-sha-1234' | :npm | false + ref(:project_with_ppr) | :owner | '@my-scope/my-package-stage-sha-1234' | :npm | false + ref(:project_with_ppr) | Gitlab::Access::ADMIN | '@my-scope/my-package-stage-sha-1234' | :npm | false + + ref(:project_with_ppr) | :developer | '@my-scope/my-package-prod-sha-1234' | :npm | true + ref(:project_with_ppr) | :maintainer | '@my-scope/my-package-prod-sha-1234' | :npm | true + ref(:project_with_ppr) | :owner | '@my-scope/my-package-prod-sha-1234' | :npm | false + ref(:project_with_ppr) | Gitlab::Access::ADMIN | '@my-scope/my-package-prod-sha-1234' | :npm | false + + ref(:project_with_ppr) | :developer | '@my-scope/my-package-release-v1' | :npm | true + ref(:project_with_ppr) | :owner | '@my-scope/my-package-release-v1' | :npm | true + ref(:project_with_ppr) | Gitlab::Access::ADMIN | '@my-scope/my-package-release-v1' | :npm | false + + ref(:project_with_ppr) | :developer | '@my-scope/my-package-any-suffix' | :npm | true + ref(:project_with_ppr) | :maintainer | '@my-scope/my-package-any-suffix' | :npm | false + ref(:project_with_ppr) | :owner | '@my-scope/my-package-any-suffix' | :npm | false + + # For non-matching package_name + ref(:project_with_ppr) | :developer | '@my-scope/non-matching-package' | :npm | false + + # For non-matching package_type + ref(:project_with_ppr) | :developer | '@my-scope/my-package-any-suffix' | :conan | false + + # For no access level + ref(:project_with_ppr) | Gitlab::Access::NO_ACCESS | '@my-scope/my-package-prod' | :npm | true + + # Edge cases + ref(:project_with_ppr) | 0 | '' | nil | true + ref(:project_with_ppr) | nil | nil | nil | true + + # For projects that have no package protection rules + ref(:project_without_ppr) | :developer | '@my-scope/my-package-prod' | :npm | false + ref(:project_without_ppr) | :maintainer | '@my-scope/my-package-prod' | :npm | false + ref(:project_without_ppr) | :owner | '@my-scope/my-package-prod' | :npm | false + end + # rubocop:enable Layout/LineLength - it { - is_expected.to validate_inclusion_of(:push_protected_up_to_access_level).in_array([Gitlab::Access::DEVELOPER, - Gitlab::Access::MAINTAINER, Gitlab::Access::OWNER]) - } + with_them do + it { is_expected.to eq push_protected } + end end end end diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 62152f9d3a4..08ba823f8fa 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -4,8 +4,10 @@ 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 } - subject(:lookup_path) { described_class.new(project) } + subject(:lookup_path) { described_class.new(project, trim_prefix: trim_prefix, domain: domain) } before do stub_pages_setting( @@ -30,11 +32,7 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end describe '#https_only' do - subject(:lookup_path) { described_class.new(project, 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 @@ -101,41 +99,26 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end end end - - context 'when deployment were created during migration' do - before do - allow(deployment).to receive(:migrated?).and_return(true) - end - - 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 - end end end describe '#prefix' do - it 'returns "/" for pages group root projects' do - project = instance_double(Project, full_path: "namespace/namespace.example.com") - lookup_path = described_class.new(project, trim_prefix: 'mygroup') + let(:trim_prefix) { 'mygroup' } + + context 'when pages group root projects' do + let(:project) { instance_double(Project, full_path: "namespace/namespace.example.com") } - expect(lookup_path.prefix).to eq('/') + it 'returns "/"' do + expect(lookup_path.prefix).to eq('/') + end end - it 'returns the project full path with the provided prefix removed' do - project = instance_double(Project, full_path: 'mygroup/myproject') - lookup_path = described_class.new(project, trim_prefix: 'mygroup') + context 'when pages in the given prefix' do + let(:project) { instance_double(Project, full_path: 'mygroup/myproject') } - expect(lookup_path.prefix).to eq('/myproject/') + it 'returns the project full path with the provided prefix removed' do + expect(lookup_path.prefix).to eq('/myproject/') + end end end @@ -157,12 +140,18 @@ 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 - subject(:lookup_path) { described_class.new(project) } - context 'when there is no deployment' do it 'returns nil' do expect(lookup_path.root_directory).to be_nil diff --git a/spec/models/pages_deployment_spec.rb b/spec/models/pages_deployment_spec.rb index 916197fe5e9..e74c7ee8612 100644 --- a/spec/models/pages_deployment_spec.rb +++ b/spec/models/pages_deployment_spec.rb @@ -28,16 +28,6 @@ RSpec.describe PagesDeployment, feature_category: :pages do end end - describe '.migrated_from_legacy_storage' do - it 'only returns migrated deployments' do - migrated_deployment = create_migrated_deployment(project) - # create one other deployment - create(:pages_deployment, project: project) - - expect(described_class.migrated_from_legacy_storage).to eq([migrated_deployment]) - end - end - context 'with deployments stored locally and remotely' do before do stub_pages_object_storage(::Pages::DeploymentUploader) @@ -132,34 +122,6 @@ RSpec.describe PagesDeployment, feature_category: :pages do end end - describe '#migrated?' do - it 'returns false for normal deployment' do - deployment = create(:pages_deployment) - - expect(deployment.migrated?).to eq(false) - end - - it 'returns true for migrated deployment' do - deployment = create_migrated_deployment(project) - - expect(deployment.migrated?).to eq(true) - end - end - - def create_migrated_deployment(project) - public_path = File.join(project.pages_path, "public") - FileUtils.mkdir_p(public_path) - File.open(File.join(project.pages_path, "public/index.html"), "w") do |f| - f.write("Hello!") - end - - expect(::Pages::MigrateLegacyStorageToDeploymentService.new(project).execute[:status]).to eq(:success) - - project.reload.pages_metadatum.pages_deployment - ensure - FileUtils.rm_rf(public_path) - end - describe 'default for file_store' do let(:deployment) do filepath = Rails.root.join("spec/fixtures/pages.zip") diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index cd740bca502..5a4eca11f71 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PagesDomain do +RSpec.describe PagesDomain, feature_category: :pages do using RSpec::Parameterized::TableSyntax subject(:pages_domain) { described_class.new } diff --git a/spec/models/preloaders/project_root_ancestor_preloader_spec.rb b/spec/models/preloaders/project_root_ancestor_preloader_spec.rb index 2462e305597..b690bd3162c 100644 --- a/spec/models/preloaders/project_root_ancestor_preloader_spec.rb +++ b/spec/models/preloaders/project_root_ancestor_preloader_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Preloaders::ProjectRootAncestorPreloader do +RSpec.describe Preloaders::ProjectRootAncestorPreloader, feature_category: :system_access do let_it_be(:root_parent1) { create(:group, :private, name: 'root-1', path: 'root-1') } let_it_be(:root_parent2) { create(:group, name: 'root-2', path: 'root-2') } let_it_be(:guest_project) { create(:project, name: 'public guest', path: 'public-guest') } @@ -43,87 +43,47 @@ RSpec.describe Preloaders::ProjectRootAncestorPreloader do end end - context 'when use_traversal_ids FF is enabled' do - context 'when the preloader is used' do - context 'when no additional preloads are provided' do - before do - preload_ancestors(:group) - end - - it_behaves_like 'executes N matching DB queries', 0 - end - - context 'when additional preloads are provided' do - let(:additional_preloads) { [:route] } - let(:root_query_regex) { /\ASELECT.+FROM "routes" WHERE "routes"."source_id" = \d+/ } - - before do - preload_ancestors - end - - it_behaves_like 'executes N matching DB queries', 0, :full_path - end - - context 'when projects are an array and not an ActiveRecord::Relation' do - before do - described_class.new(projects, :namespace, additional_preloads).execute - end - - it_behaves_like 'executes N matching DB queries', 4 - end - end - - context 'when the preloader is not used' do - it_behaves_like 'executes N matching DB queries', 4 - end - - context 'when using a :group sti name and passing projects in a user namespace' do - let(:projects) { [private_developer_project] } - let(:additional_preloads) { [:ip_restrictions, :saml_provider] } - - it 'does not load a nil value for root_ancestor' do + context 'when the preloader is used' do + context 'when no additional preloads are provided' do + before do preload_ancestors(:group) - - expect(pristine_projects.first.root_ancestor).to eq(private_developer_project.root_ancestor) end - end - end - context 'when use_traversal_ids FF is disabled' do - before do - stub_feature_flags(use_traversal_ids: false) + it_behaves_like 'executes N matching DB queries', 0 end - context 'when the preloader is used' do + context 'when additional preloads are provided' do + let(:additional_preloads) { [:route] } + let(:root_query_regex) { /\ASELECT.+FROM "routes" WHERE "routes"."source_id" = \d+/ } + before do preload_ancestors end - context 'when no additional preloads are provided' do - it_behaves_like 'executes N matching DB queries', 4 - end - - context 'when additional preloads are provided' do - let(:additional_preloads) { [:route] } - let(:root_query_regex) { /\ASELECT.+FROM "routes" WHERE "routes"."source_id" = \d+/ } + it_behaves_like 'executes N matching DB queries', 0, :full_path + end - it_behaves_like 'executes N matching DB queries', 4, :full_path + context 'when projects are an array and not an ActiveRecord::Relation' do + before do + described_class.new(projects, :namespace, additional_preloads).execute end - end - context 'when the preloader is not used' do it_behaves_like 'executes N matching DB queries', 4 end + end - context 'when using a :group sti name and passing projects in a user namespace' do - let(:projects) { [private_developer_project] } - let(:additional_preloads) { [:ip_restrictions, :saml_provider] } + context 'when the preloader is not used' do + it_behaves_like 'executes N matching DB queries', 4 + end - it 'does not load a nil value for root_ancestor' do - preload_ancestors(:group) + context 'when using a :group sti name and passing projects in a user namespace' do + let(:projects) { [private_developer_project] } + let(:additional_preloads) { [:ip_restrictions, :saml_provider] } - expect(pristine_projects.first.root_ancestor).to eq(private_developer_project.root_ancestor) - end + it 'does not load a nil value for root_ancestor' do + preload_ancestors(:group) + + expect(pristine_projects.first.root_ancestor).to eq(private_developer_project.root_ancestor) end end diff --git a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb index 5befa3ab66f..3dc409cbcc2 100644 --- a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb +++ b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb @@ -34,46 +34,31 @@ RSpec.describe Preloaders::UserMaxAccessLevelInGroupsPreloader, feature_category let(:groups) { [group1, group2, group3, child_maintainer, child_indirect_access] } - context 'when traversal_ids feature flag is disabled' do - it_behaves_like 'executes N max member permission queries to the DB' do - before do - stub_feature_flags(use_traversal_ids: false) - described_class.new(groups, user).execute - end - - # One query for group with no access and another one per group where the user is not a direct member - let(:expected_query_count) { 2 } + it_behaves_like 'executes N max member permission queries to the DB' do + before do + described_class.new(groups, user).execute end - end - - context 'when traversal_ids feature flag is enabled' do - it_behaves_like 'executes N max member permission queries to the DB' do - before do - stub_feature_flags(use_traversal_ids: true) - described_class.new(groups, user).execute - end - let(:expected_query_count) { 0 } - end + let(:expected_query_count) { 0 } + end - context 'for groups arising from group shares' do - let_it_be(:group4) { create(:group, :private) } - let_it_be(:group4_subgroup) { create(:group, :private, parent: group4) } + context 'for groups arising from group shares' do + let_it_be(:group4) { create(:group, :private) } + let_it_be(:group4_subgroup) { create(:group, :private, parent: group4) } - let(:groups) { [group4, group4_subgroup] } + let(:groups) { [group4, group4_subgroup] } - before do - create(:group_group_link, :guest, shared_with_group: group1, shared_group: group4) - end + before do + create(:group_group_link, :guest, shared_with_group: group1, shared_group: group4) + end - it 'sets the right access level in cache for groups arising from group shares' do - described_class.new(groups, user).execute + it 'sets the right access level in cache for groups arising from group shares' do + described_class.new(groups, user).execute - groups.each do |group| - cached_access_level = group.max_member_access_for_user(user) + groups.each do |group| + cached_access_level = group.max_member_access_for_user(user) - expect(cached_access_level).to eq(Gitlab::Access::GUEST) - end + expect(cached_access_level).to eq(Gitlab::Access::GUEST) end end end diff --git a/spec/models/project_authorization_spec.rb b/spec/models/project_authorization_spec.rb index 9fed05342aa..a5f29fcbe8b 100644 --- a/spec/models/project_authorization_spec.rb +++ b/spec/models/project_authorization_spec.rb @@ -83,8 +83,10 @@ RSpec.describe ProjectAuthorization, feature_category: :groups_and_projects do end describe 'scopes' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, namespace: user.namespace) } + describe '.non_guests' do - let_it_be(:project) { create(:project) } let_it_be(:project_original_owner_authorization) { project.owner.project_authorizations.first } let_it_be(:project_authorization_guest) { create(:project_authorization, :guest, project: project) } let_it_be(:project_authorization_reporter) { create(:project_authorization, :reporter, project: project) } @@ -100,6 +102,28 @@ RSpec.describe ProjectAuthorization, feature_category: :groups_and_projects do ].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) } + + let_it_be(:project_authorization_3) { project_3.project_authorizations.first } + let_it_be(:project_authorization_2) { project_2.project_authorizations.first } + let_it_be(:project_authorization) { project.project_authorizations.first } + + it 'returns all records for the project' do + expect(described_class.for_project(project).map(&:attributes)).to match_array([ + project_authorization + ].map(&:attributes)) + end + + it 'returns all records for multiple projects' do + expect(described_class.for_project([project, project_3]).map(&:attributes)).to match_array([ + project_authorization, + project_authorization_3 + ].map(&:attributes)) + end + end end describe '.insert_all' do diff --git a/spec/models/project_pages_metadatum_spec.rb b/spec/models/project_pages_metadatum_spec.rb deleted file mode 100644 index 31a533e0363..00000000000 --- a/spec/models/project_pages_metadatum_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ProjectPagesMetadatum do - describe '.only_on_legacy_storage' do - it 'returns only deployed records without deployment' do - create(:project) # without pages deployed - - legacy_storage_project = create(:project) - legacy_storage_project.mark_pages_as_deployed - - project_with_deployment = create(:project) - deployment = create(:pages_deployment, project: project_with_deployment) - project_with_deployment.mark_pages_as_deployed - project_with_deployment.update_pages_deployment!(deployment) - - expect(described_class.only_on_legacy_storage).to eq([legacy_storage_project.pages_metadatum]) - end - end -end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 3b890e75064..719e51018ac 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -26,8 +26,7 @@ RSpec.describe ProjectSetting, type: :model, feature_category: :groups_and_proje it { is_expected.to allow_value([]).for(:target_platforms) } it { is_expected.to validate_length_of(:issue_branch_template).is_at_most(255) } - it { is_expected.not_to allow_value(nil).for(:suggested_reviewers_enabled) } - it { is_expected.to allow_value(true, false).for(:suggested_reviewers_enabled) } + it { is_expected.to validate_inclusion_of(:suggested_reviewers_enabled).in_array([true, false]) } it 'allows any combination of the allowed target platforms' do valid_target_platform_combinations.each do |target_platforms| diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 46bf80b1e8f..c27ed2cc82c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -103,6 +103,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it { is_expected.to have_one(:mock_monitoring_integration) } it { is_expected.to have_one(:service_desk_custom_email_verification).class_name('ServiceDesk::CustomEmailVerification') } it { is_expected.to have_one(:container_registry_data_repair_detail).class_name('ContainerRegistry::DataRepairDetail') } + it { is_expected.to have_many(:container_registry_protection_rules).class_name('ContainerRegistry::Protection::Rule') } it { is_expected.to have_many(:commit_statuses) } it { is_expected.to have_many(:ci_pipelines) } it { is_expected.to have_many(:ci_refs) } @@ -820,6 +821,28 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end + describe 'name format validation' do + context 'name is unchanged' do + let_it_be(:invalid_path_project) do + project = create(:project) + project.update_attribute(:name, '.invalid_name') + project + end + + it 'does not raise validation error for name for existing project' do + expect { invalid_path_project.update!(description: 'Foo') }.not_to raise_error + end + end + + %w[. - $].each do |special_character| + it "rejects a name starting with '#{special_character}'" do + project = build(:project, name: "#{special_character}foo") + + expect(project).not_to be_valid + end + end + end + describe 'path validation' do it 'allows paths reserved on the root namespace' do project = build(:project, path: 'api') @@ -2218,7 +2241,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr context 'when the Slack app setting is not enabled' do before do stub_application_setting(slack_app_enabled: false) - allow(Rails.env).to receive(:test?).and_return(false, true) + allow(Rails.env).to receive(:test?).and_return(false) end it 'includes all projects' do @@ -5124,28 +5147,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#pages_available?' do - let(:project) { create(:project, group: group) } - - subject { project.pages_available? } - - before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) - end - - context 'when the project is in a top level namespace' do - let(:group) { create(:group) } - - it { is_expected.to be(true) } - end - - context 'when the project is in a subgroup' do - let(:group) { create(:group, :nested) } - - it { is_expected.to be(true) } - end - end - describe '#remove_private_deploy_keys' do let!(:project) { create(:project) } @@ -5296,62 +5297,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect(project.hashed_storage?(:repository)).to be_falsey end end - - describe '#pages_path' do - it 'returns a path where pages are stored' do - expect(project.pages_path).to eq(File.join(Settings.pages.path, project.namespace.full_path, project.path)) - end - end - - describe '#migrate_to_hashed_storage!' do - let(:project) { create(:project, :empty_repo, :legacy_storage) } - - it 'returns true' do - expect(project.migrate_to_hashed_storage!).to be_truthy - end - - it 'does not run validation' do - expect(project).not_to receive(:valid?) - - project.migrate_to_hashed_storage! - end - - it 'schedules HashedStorage::ProjectMigrateWorker with delayed start when the project repo is in use' do - Gitlab::ReferenceCounter.new(Gitlab::GlRepository::PROJECT.identifier_for_container(project)).increase - - expect(HashedStorage::ProjectMigrateWorker).to receive(:perform_in) - - project.migrate_to_hashed_storage! - end - - it 'schedules HashedStorage::ProjectMigrateWorker with delayed start when the wiki repo is in use' do - Gitlab::ReferenceCounter.new(Gitlab::GlRepository::WIKI.identifier_for_container(project.wiki)).increase - - expect(HashedStorage::ProjectMigrateWorker).to receive(:perform_in) - - project.migrate_to_hashed_storage! - end - - it 'schedules HashedStorage::ProjectMigrateWorker' do - expect(HashedStorage::ProjectMigrateWorker).to receive(:perform_async).with(project.id) - - project.migrate_to_hashed_storage! - end - end - - describe '#rollback_to_legacy_storage!' do - let(:project) { create(:project, :empty_repo, :legacy_storage) } - - it 'returns nil' do - expect(project.rollback_to_legacy_storage!).to be_nil - end - - it 'does not run validations' do - expect(project).not_to receive(:valid?) - - project.rollback_to_legacy_storage! - end - end end context 'hashed storage' do @@ -5391,58 +5336,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect(project.disk_path).to eq(hashed_path) end end - - describe '#pages_path' do - it 'returns a path where pages are stored' do - expect(project.pages_path).to eq(File.join(Settings.pages.path, project.namespace.full_path, project.path)) - end - end - - describe '#migrate_to_hashed_storage!' do - let(:project) { create(:project, :repository, skip_disk_validation: true) } - - it 'returns nil' do - expect(project.migrate_to_hashed_storage!).to be_nil - end - - it 'does not flag as read-only' do - expect { project.migrate_to_hashed_storage! }.not_to change { project.repository_read_only } - end - - context 'when partially migrated' do - it 'enqueues a job' do - project = create(:project, storage_version: 1, skip_disk_validation: true) - - Sidekiq::Testing.fake! do - expect { project.migrate_to_hashed_storage! }.to change(HashedStorage::ProjectMigrateWorker.jobs, :size).by(1) - end - end - end - end - - describe '#rollback_to_legacy_storage!' do - let(:project) { create(:project, :repository, skip_disk_validation: true) } - - it 'returns true' do - expect(project.rollback_to_legacy_storage!).to be_truthy - end - - it 'does not run validations' do - expect(project).not_to receive(:valid?) - - project.rollback_to_legacy_storage! - end - - it 'does not flag as read-only' do - expect { project.rollback_to_legacy_storage! }.not_to change { project.repository_read_only } - end - - it 'enqueues a job' do - Sidekiq::Testing.fake! do - expect { project.rollback_to_legacy_storage! }.to change(HashedStorage::ProjectRollbackWorker.jobs, :size).by(1) - end - end - end end describe '#has_ci?' do @@ -6908,6 +6801,17 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end + describe '.with_package_registry_enabled' do + subject { described_class.with_package_registry_enabled } + + it 'returns projects with the package registry enabled' do + project_1 = create(:project) + create(:project, package_registry_access_level: ProjectFeature::DISABLED, packages_enabled: false) + + expect(subject).to contain_exactly(project_1) + end + end + describe '.deployments' do subject { project.deployments } @@ -7435,7 +7339,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#has_pool_repsitory?' do + describe '#has_pool_repository?' do it 'returns false when it does not have a pool repository' do subject = create(:project, :repository) @@ -8807,16 +8711,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#content_editor_on_issues_feature_flag_enabled?' do - let_it_be(:group_project) { create(:project, :in_subgroup) } - - it_behaves_like 'checks parent group feature flag' do - let(:feature_flag_method) { :content_editor_on_issues_feature_flag_enabled? } - let(:feature_flag) { :content_editor_on_issues } - let(:subject_project) { group_project } - end - end - describe '#work_items_mvc_feature_flag_enabled?' do let_it_be(:group_project) { create(:project, :in_subgroup) } @@ -9274,4 +9168,11 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr name: name ) end + + context 'with loose foreign key on projects.creator_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let_it_be(:parent) { create(:user) } + let_it_be(:model) { create(:project, creator: parent) } + end + end end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index e557990c7e9..10a2e967b14 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -328,19 +328,6 @@ RSpec.describe ProjectTeam, feature_category: :groups_and_projects do expect(project.team.reporter?(user1)).to be(true) expect(project.team.reporter?(user2)).to be(true) end - - context 'when `tasks_to_be_done` and `tasks_project_id` are passed' do - before do - project.team.add_members([user1], :developer, tasks_to_be_done: %w(ci code), tasks_project_id: project.id) - end - - it 'creates a member_task with the correct attributes', :aggregate_failures do - member = project.project_members.last - - expect(member.tasks_to_be_done).to match_array([:ci, :code]) - expect(member.member_task.project).to eq(project) - end - end end describe '#add_member' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index af7457c78e2..2265d1b39af 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -3845,11 +3845,50 @@ RSpec.describe Repository, feature_category: :source_code_management do end context 'when a Gitlab::Git::CommandError is raised' do - it 'returns nil' do + before do expect(repository.raw_repository) .to receive(:get_patch_id).and_raise(Gitlab::Git::CommandError) + end - expect(repository.get_patch_id('HEAD', "f" * 40)).to be_nil + it 'returns nil' do + expect(repository.get_patch_id('HEAD', 'HEAD')).to be_nil + end + + it 'reports the exception' do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .with( + instance_of(Gitlab::Git::CommandError), + project_id: repository.project.id, + old_revision: 'HEAD', + new_revision: 'HEAD' + ) + + repository.get_patch_id('HEAD', 'HEAD') + end + end + + context 'when a Gitlab::Git::Repository::NoRepository is raised' do + before do + expect(repository.raw_repository) + .to receive(:get_patch_id).and_raise(Gitlab::Git::Repository::NoRepository) + end + + it 'returns nil' do + expect(repository.get_patch_id('HEAD', 'f' * 40)).to be_nil + end + + it 'reports the exception' do + expect(Gitlab::ErrorTracking) + .to receive(:track_exception) + .with( + instance_of(Gitlab::Git::Repository::NoRepository), + project_id: repository.project.id, + old_revision: 'HEAD', + new_revision: 'HEAD' + ) + + repository.get_patch_id('HEAD', 'HEAD') end end end @@ -3942,4 +3981,61 @@ RSpec.describe Repository, feature_category: :source_code_management do end end end + + describe '#get_file_attributes' do + let(:project) do + create(:project, :custom_repo, files: { + '.gitattributes' => gitattr_content, + 'file1.txt' => 'test content' + }) + end + + let(:gitattr_content) { '' } + let(:rev) { 'master' } + let(:paths) { ['file1.txt', 'README'] } + let(:attrs) { %w[text diff] } + + subject(:file_attributes) { repository.get_file_attributes(rev, paths, attrs) } + + context 'when the given attributes are defined' do + let(:gitattr_content) { "* -text\n*.txt text\n*.txt diff" } + + it 'returns expected attributes' do + expect(file_attributes.count).to eq 3 + expect(file_attributes[0]).to eq({ path: 'file1.txt', attribute: 'text', value: 'set' }) + expect(file_attributes[1]).to eq({ path: 'file1.txt', attribute: 'diff', value: 'set' }) + expect(file_attributes[2]).to eq({ path: 'README', attribute: 'text', value: 'unset' }) + end + end + + context 'when the attribute is not defined for a given file' do + let(:gitattr_content) { "*.txt text" } + + let(:rev) { 'master' } + let(:paths) { ['README'] } + let(:attrs) { ['text'] } + + it 'returns an empty array' do + expect(file_attributes).to eq [] + end + end + + context 'when revision is an empty string' do + let(:rev) { '' } + + it { expect { file_attributes }.to raise_error(ArgumentError) } + end + + context 'when paths list is empty' do + let(:paths) { [] } + + it { expect { file_attributes }.to raise_error(ArgumentError) } + end + + context 'when attributes list is empty' do + let(:attrs) { [] } + + it { expect { file_attributes }.to raise_error(ArgumentError) } + end + end end diff --git a/spec/models/resource_state_event_spec.rb b/spec/models/resource_state_event_spec.rb index 5bd8b664d23..5a3f21631ca 100644 --- a/spec/models/resource_state_event_spec.rb +++ b/spec/models/resource_state_event_spec.rb @@ -61,7 +61,7 @@ RSpec.describe ResourceStateEvent, feature_category: :team_planning, type: :mode it_behaves_like 'internal event tracking' do subject(:service_action) { close_issue } - let(:action) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CLOSED } + let(:event) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CLOSED } let(:project) { issue.project } let(:namespace) { issue.project.namespace } let(:user) { issue.author } @@ -86,7 +86,7 @@ RSpec.describe ResourceStateEvent, feature_category: :team_planning, type: :mode it_behaves_like 'internal event tracking' do subject(:service_action) { reopen_issue } - let(:action) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_REOPENED } + let(:event) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_REOPENED } let(:project) { issue.project } let(:user) { issue.author } let(:namespace) { issue.project.namespace } diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 4c6f1476481..ec2dfb2634f 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -487,6 +487,18 @@ RSpec.describe Snippet do end end + describe '.without_created_by_banned_user', feature_category: :insider_threat do + let_it_be(:user) { create(:user) } + let_it_be(:banned_user) { create(:user, :banned) } + + let_it_be(:snippet) { create(:snippet, author: user) } + let_it_be(:snippet_by_banned_user) { create(:snippet, author: banned_user) } + + subject(:without_created_by_banned_user) { described_class.without_created_by_banned_user } + + it { is_expected.to match_array(snippet) } + end + describe '#participants' do let_it_be(:project) { create(:project, :public) } let_it_be(:snippet) { create(:snippet, content: 'foo', project: project) } @@ -962,4 +974,30 @@ RSpec.describe Snippet do it_behaves_like 'can move repository storage' do let_it_be(:container) { create(:snippet, :repository) } end + + describe '#hidden_due_to_author_ban?', feature_category: :insider_threat do + let(:snippet) { build(:snippet, author: author) } + + subject(:hidden_due_to_author_ban) { snippet.hidden_due_to_author_ban? } + + context 'when the author is not banned' do + let_it_be(:author) { build(:user) } + + it { is_expected.to eq(false) } + end + + context 'when author is banned' do + let_it_be(:author) { build(:user, :banned) } + + it { is_expected.to eq(true) } + + context 'when the `hide_snippets_of_banned_users` feature flag is disabled' do + before do + stub_feature_flags(hide_snippets_of_banned_users: false) + end + + it { is_expected.to eq(false) } + end + end + end end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 2d6a674d3ce..316d1343512 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -396,6 +396,19 @@ RSpec.describe Todo do end end + describe '.not_in_users' do + it 'returns the expected todos' do + user1 = create(:user) + user2 = create(:user) + + todo1 = create(:todo, user: user1) + todo2 = create(:todo, user: user1) + create(:todo, user: user2) + + expect(described_class.not_in_users(user2)).to contain_exactly(todo1, todo2) + end + end + describe '.for_group_ids_and_descendants' do it 'returns the todos for a group and its descendants' do parent_group = create(:group) diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 401a85e2f82..343576de4d3 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -49,8 +49,7 @@ RSpec.describe UserPreference, feature_category: :user_profile do end describe 'pass_user_identities_to_ci_jwt' do - it { is_expected.to allow_value(true, false).for(:pass_user_identities_to_ci_jwt) } - it { is_expected.not_to allow_value(nil).for(:pass_user_identities_to_ci_jwt) } + it { is_expected.to validate_inclusion_of(:pass_user_identities_to_ci_jwt).in_array([true, false]) } it { is_expected.not_to allow_value("").for(:pass_user_identities_to_ci_jwt) } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c611c3c26e3..947d83badf6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5073,14 +5073,6 @@ RSpec.describe User, feature_category: :user_profile do describe '#ci_owned_runners' do it_behaves_like '#ci_owned_runners' - - context 'when FF use_traversal_ids is disabled fallbacks to inefficient implementation' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it_behaves_like '#ci_owned_runners' - end end describe '#projects_with_reporter_access_limited_to' do @@ -6120,25 +6112,23 @@ RSpec.describe User, feature_category: :user_profile do end end - describe '#allow_possible_spam?' do + describe '#trusted?' do context 'when no custom attribute is set' do - it 'is false' do - expect(user.allow_possible_spam?).to be_falsey + it 'is falsey' do + expect(user.trusted?).to be_falsey end end context 'when the custom attribute is set' do before do - user.custom_attributes.upsert_custom_attributes( - [{ - user_id: user.id, - key: UserCustomAttribute::ALLOW_POSSIBLE_SPAM, - value: "test" - }]) + user.custom_attributes.create!( + key: UserCustomAttribute::TRUSTED_BY, + value: "test" + ) end - it '#allow_possible_spam? is true' do - expect(user.allow_possible_spam?).to be_truthy + it 'is truthy' do + expect(user.trusted?).to be_truthy end end end diff --git a/spec/models/users/credit_card_validation_spec.rb b/spec/models/users/credit_card_validation_spec.rb index 486d1c6d3ea..7faddb2384c 100644 --- a/spec/models/users/credit_card_validation_spec.rb +++ b/spec/models/users/credit_card_validation_spec.rb @@ -15,41 +15,43 @@ RSpec.describe Users::CreditCardValidation, feature_category: :user_profile do it { is_expected.to validate_length_of(:network_hash).is_at_most(44) } describe '#similar_records' do - let(:card_details) do - subject.attributes.with_indifferent_access.slice(:expiration_date, :last_digits, :network, :holder_name) + let_it_be(:credit_card_validation) { create(:credit_card_validation) } + + let_it_be(:card_details) do + credit_card_validation.attributes.with_indifferent_access.slice( + :expiration_date, :last_digits, :network, :holder_name + ) end - subject!(:credit_card_validation) { create(:credit_card_validation, holder_name: 'Alice') } + let_it_be(:match_1) { create(:credit_card_validation, card_details) } + let_it_be(:match_2) { create(:credit_card_validation, card_details.merge(holder_name: 'Bob')) } - let!(:match1) { create(:credit_card_validation, card_details) } - let!(:match2) { create(:credit_card_validation, card_details.merge(holder_name: 'Bob')) } - let!(:non_match1) { create(:credit_card_validation, card_details.merge(last_digits: 9)) } - let!(:non_match2) { create(:credit_card_validation, card_details.merge(network: 'unknown')) } - let!(:non_match3) do - create(:credit_card_validation, card_details.dup.tap { |h| h[:expiration_date] += 1.year }) + let_it_be(:non_match_1) { create(:credit_card_validation, card_details.merge(last_digits: 9999)) } + let_it_be(:non_match_2) { create(:credit_card_validation, card_details.merge(network: 'Mastercard')) } + let_it_be(:non_match_3) do + create(:credit_card_validation, card_details.merge(expiration_date: 2.years.from_now.to_date)) end it 'returns matches with the same last_digits, expiration and network, ordered by credit_card_validated_at' do - expect(subject.similar_records).to eq([match2, match1, subject]) + # eq is used instead of match_array because rows are sorted by credit_card_validated_at in desc order + expect(credit_card_validation.similar_records).to eq([match_2, match_1, credit_card_validation]) end end describe '#similar_holder_names_count' do - subject!(:credit_card_validation) { create(:credit_card_validation, holder_name: holder_name) } - context 'when holder_name is present' do - let(:holder_name) { 'ALICE M SMITH' } + let_it_be(:credit_card_validation) { create(:credit_card_validation, holder_name: 'ALICE M SMITH') } - let!(:match) { create(:credit_card_validation, holder_name: 'Alice M Smith') } - let!(:non_match) { create(:credit_card_validation, holder_name: 'Bob B Brown') } + let_it_be(:match) { create(:credit_card_validation, holder_name: 'Alice M Smith') } + let_it_be(:non_match) { create(:credit_card_validation, holder_name: 'Bob B Brown') } it 'returns the count of cards with similar case insensitive holder names' do - expect(subject.similar_holder_names_count).to eq(2) + expect(credit_card_validation.similar_holder_names_count).to eq(2) end end context 'when holder_name is nil' do - let(:holder_name) { nil } + let_it_be(:credit_card_validation) { create(:credit_card_validation, holder_name: nil) } it 'returns 0' do expect(subject.similar_holder_names_count).to eq(0) @@ -75,104 +77,117 @@ RSpec.describe Users::CreditCardValidation, feature_category: :user_profile do end describe '.by_banned_user' do - let(:banned_user) { create(:banned_user) } - let!(:credit_card) { create(:credit_card_validation) } - let!(:banned_user_credit_card) { create(:credit_card_validation, user: banned_user.user) } + subject(:by_banned_user) { described_class.by_banned_user } + + let_it_be(:banned_user) { create(:banned_user) } + let_it_be(:credit_card) { create(:credit_card_validation) } + let_it_be(:banned_user_credit_card) { create(:credit_card_validation, user: banned_user.user) } it 'returns only records associated to banned users' do - expect(described_class.by_banned_user).to match_array([banned_user_credit_card]) + expect(by_banned_user).to match_array([banned_user_credit_card]) end end describe '.similar_by_holder_name' do - let!(:credit_card) { create(:credit_card_validation, holder_name: 'CARD MCHODLER') } - let!(:credit_card2) { create(:credit_card_validation, holder_name: 'RICHIE RICH') } + subject(:similar_by_holder_name) { described_class.similar_by_holder_name(holder_name_hash) } - it 'returns only records that case-insensitive match the given holder name' do - expect(described_class.similar_by_holder_name('card mchodler')).to match_array([credit_card]) - end + let_it_be(:credit_card_validation) { create(:credit_card_validation, holder_name: 'Alice M Smith') } + let_it_be(:match) { create(:credit_card_validation, holder_name: 'ALICE M SMITH') } + + context 'when holder_name_hash is present' do + let_it_be(:holder_name_hash) { credit_card_validation.holder_name_hash } - context 'when given holder name is falsey' do - it 'returns [] when given holder name is ""' do - expect(described_class.similar_by_holder_name('')).to match_array([]) + it 'returns records with similar holder names case-insensitively' do + expect(similar_by_holder_name).to match_array([credit_card_validation, match]) end + end + + context 'when holder_name_hash is nil' do + let_it_be(:holder_name_hash) { nil } - it 'returns [] when given holder name is nil' do - expect(described_class.similar_by_holder_name(nil)).to match_array([]) + it 'returns an empty array' do + expect(similar_by_holder_name).to match_array([]) end end end describe '.similar_to' do - let(:credit_card) { create(:credit_card_validation) } + subject(:similar_to) { described_class.similar_to(credit_card_validation) } + + let_it_be(:credit_card_validation) { create(:credit_card_validation) } - let!(:credit_card2) do + let_it_be(:match) do create(:credit_card_validation, - expiration_date: credit_card.expiration_date, - last_digits: credit_card.last_digits, - network: credit_card.network + expiration_date: credit_card_validation.expiration_date, + last_digits: credit_card_validation.last_digits, + network: credit_card_validation.network ) end - let!(:credit_card3) do + let_it_be(:non_match) do create(:credit_card_validation, - expiration_date: credit_card.expiration_date, - last_digits: credit_card.last_digits, - network: 'UnknownCCNetwork' + expiration_date: credit_card_validation.expiration_date, + last_digits: credit_card_validation.last_digits, + network: 'Mastercard' ) end it 'returns only records with similar expiration_date, last_digits, and network attribute values' do - expect(described_class.similar_to(credit_card)).to match_array([credit_card, credit_card2]) + expect(similar_to).to match_array([credit_card_validation, match]) end end end describe '#used_by_banned_user?' do - let(:credit_card_details) do - { - holder_name: 'Christ McLovin', - expiration_date: 2.years.from_now.end_of_month, - last_digits: 4242, - network: 'Visa' - } - end - - let!(:credit_card) { create(:credit_card_validation, credit_card_details) } + subject(:used_by_banned_user) { credit_card_validation.used_by_banned_user? } - subject { credit_card } + let_it_be(:credit_card_validation) { create(:credit_card_validation) } - context 'when there is a similar credit card associated to a banned user' do - let_it_be(:banned_user) { create(:banned_user) } - - let(:attrs) { credit_card_details.merge({ user: banned_user.user }) } - let!(:similar_credit_card) { create(:credit_card_validation, attrs) } + let_it_be(:card_details) do + credit_card_validation.attributes.with_indifferent_access.slice( + :expiration_date, :last_digits, :network, :holder_name + ) + end - it { is_expected.to be_used_by_banned_user } + let_it_be(:banned_user) { create(:banned_user) } - context 'when holder names do not match' do - let!(:similar_credit_card) do - create(:credit_card_validation, attrs.merge({ holder_name: 'Mary Goody' })) + context 'when there is a similar credit card associated to a banned user' do + context 'when holder names match exactly' do + before do + create(:credit_card_validation, card_details.merge(user: banned_user.user)) end - it { is_expected.not_to be_used_by_banned_user } + it { is_expected.to be(true) } end - context 'when .similar_to returns nothing' do - let!(:similar_credit_card) do - create(:credit_card_validation, attrs.merge({ network: 'DifferentNetwork' })) + context 'when holder names do not match exactly' do + before do + create(:credit_card_validation, card_details.merge(user: banned_user.user, holder_name: 'John M Smith')) end - it { is_expected.not_to be_used_by_banned_user } + it { is_expected.to be(false) } end end - context 'when there is a similar credit card not associated to a banned user' do - let!(:similar_credit_card) do - create(:credit_card_validation, credit_card_details) + context 'when there are no similar credit cards associated to a banned user' do + before do + create(:credit_card_validation, + user: banned_user.user, + network: 'Mastercard', + last_digits: 1111, + holder_name: 'Jane Smith' + ) + end + + it { is_expected.to be(false) } + end + + context 'when there is a similar credit card but it is not associated to a banned user' do + before do + create(:credit_card_validation, card_details) end - it { is_expected.not_to be_used_by_banned_user } + it { is_expected.to be(false) } 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 78de9ad8bdb..d333a51ae3b 100644 --- a/spec/models/users/in_product_marketing_email_spec.rb +++ b/spec/models/users/in_product_marketing_email_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Users::InProductMarketingEmail, type: :model do +RSpec.describe Users::InProductMarketingEmail, type: :model, feature_category: :onboarding do let(:track) { :create } let(:series) { 0 } @@ -15,7 +15,7 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do it { is_expected.to validate_presence_of(:user) } - context 'for a track+series email' do + context 'when track+series email' do it { is_expected.to validate_presence_of(:track) } it { is_expected.to validate_presence_of(:series) } @@ -24,28 +24,6 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do .scoped_to([:track, :series]).with_message('track series email has already been sent') } end - - context 'for a campaign email' do - subject { build(:in_product_marketing_email, :campaign) } - - it { is_expected.to validate_presence_of(:campaign) } - it { is_expected.not_to validate_presence_of(:track) } - it { is_expected.not_to validate_presence_of(:series) } - - it { - is_expected.to validate_uniqueness_of(:user_id) - .scoped_to(:campaign).with_message('campaign email has already been sent') - } - - it { is_expected.to validate_inclusion_of(:campaign).in_array(described_class::CAMPAIGNS) } - end - - context 'when mixing campaign and track+series' do - it 'is not valid' do - expect(build(:in_product_marketing_email, :campaign, track: :create)).not_to be_valid - expect(build(:in_product_marketing_email, :campaign, series: 0)).not_to be_valid - end - end end describe '.without_track_and_series' do @@ -78,33 +56,9 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do context 'when no track or series for a user exists' do let(:track) { :create } let(:series) { 0 } + let(:other_user) { create(:user) } - before do - @other_user = create(:user) - end - - it { expect(without_track_and_series).to eq [@other_user] } - end - end - - describe '.without_campaign' do - let_it_be(:user) { create(:user) } - let_it_be(:other_user) { create(:user) } - - let(:campaign) { Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE } - - subject(:without_campaign) { User.merge(described_class.without_campaign(campaign)) } - - context 'when record for campaign already exists' do - before do - create(:in_product_marketing_email, :campaign, campaign: campaign, user: user) - end - - it { is_expected.to match_array [other_user] } - end - - context 'when record for campaign does not exist' do - it { is_expected.to match_array [user, other_user] } + it { expect(without_track_and_series).to eq [other_user] } end end @@ -112,7 +66,9 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do let_it_be(:user) { create(:user) } let_it_be(:in_product_marketing_email) { create(:in_product_marketing_email, series: 0, track: 0, user: user) } - subject(:for_user_with_track_and_series) { described_class.for_user_with_track_and_series(user, track, series).first } + subject(:for_user_with_track_and_series) do + described_class.for_user_with_track_and_series(user, track, series).first + end context 'when record for user with given track and series exists' do it { is_expected.to eq(in_product_marketing_email) } @@ -165,7 +121,7 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do end end - context 'cta_clicked_at is already set' do + context 'when cta_clicked_at is already set' do it 'does not update' do create(:in_product_marketing_email, user: user, track: track, series: series, cta_clicked_at: Time.zone.now) diff --git a/spec/models/vs_code/settings/vs_code_setting_spec.rb b/spec/models/vs_code/settings/vs_code_setting_spec.rb new file mode 100644 index 00000000000..d22cc815877 --- /dev/null +++ b/spec/models/vs_code/settings/vs_code_setting_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe VsCode::Settings::VsCodeSetting, feature_category: :web_ide do + let!(:user) { create(:user) } + let!(:setting) { create(:vscode_setting, user: user, setting_type: 'settings') } + + describe 'validates the presence of required attributes' do + it { is_expected.to validate_presence_of(:setting_type) } + it { is_expected.to validate_presence_of(:content) } + end + + describe 'relationship validation' do + it { is_expected.to belong_to(:user) } + end + + describe '.by_setting_type' do + subject { described_class.by_setting_type('settings') } + + it { is_expected.to contain_exactly(setting) } + end + + describe '.by_user' do + subject { described_class.by_user(user) } + + it { is_expected.to contain_exactly(setting) } + end +end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index ee61f191f05..2e1cb9d3d9b 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -394,6 +394,22 @@ RSpec.describe WikiPage, feature_category: :wiki do expect { subject.create(title: '') }.not_to change { wiki.list_pages.length } end end + + context "with front matter context" do + let(:attributes) do + { + title: SecureRandom.hex, + content: "---\nxxx: abc\n---\nHome Page", + format: "markdown", + message: 'Custom Commit Message' + } + end + + it 'create the page with front matter' do + subject.create(attributes) + expect(wiki.find_page(title).front_matter).to eq({ xxx: "abc" }) + end + end end describe "dot in the title" do diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index 4b675faf99e..3294d53e364 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -287,7 +287,7 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do it_behaves_like 'internal event tracking' do let(:work_item) { create(:work_item) } - let(:action) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } + let(:event) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } let(:project) { work_item.project } let(:user) { work_item.author } let(:namespace) { project.namespace } @@ -713,5 +713,28 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do .to contain_exactly(authorized_item_b, authorized_item_c, unauthorized_item) end end + + context 'when work item is a new record' do + let(:new_work_item) { build(:work_item, project: authorized_project) } + + it { expect(new_work_item.linked_work_items(user)).to be_empty } + end + end + + describe '#linked_items_count' do + let_it_be(:item1) { create(:work_item, :issue, project: reusable_project) } + let_it_be(:item2) { create(:work_item, :issue, project: reusable_project) } + let_it_be(:item3) { create(:work_item, :issue, project: reusable_project) } + let_it_be(:item4) { build(:work_item, :issue, project: reusable_project) } + + it 'returns number of items linked to the work item' do + create(:work_item_link, source: item1, target: item2) + create(:work_item_link, source: item1, target: item3) + + expect(item1.linked_items_count).to eq(2) + expect(item2.linked_items_count).to eq(1) + expect(item3.linked_items_count).to eq(1) + expect(item4.linked_items_count).to eq(0) + end end end diff --git a/spec/models/work_items/parent_link_spec.rb b/spec/models/work_items/parent_link_spec.rb index 3fcfa856db4..301a019dbeb 100644 --- a/spec/models/work_items/parent_link_spec.rb +++ b/spec/models/work_items/parent_link_spec.rb @@ -109,11 +109,29 @@ RSpec.describe WorkItems::ParentLink, feature_category: :portfolio_management do end end - it 'is not valid if parent is in other project' do - link = build(:parent_link, work_item_parent: task1, work_item: build(:work_item)) + context 'when assigning parent from different project' do + let_it_be(:cross_project_issue) { create(:work_item, project: create(:project)) } - expect(link).not_to be_valid - expect(link.errors[:work_item_parent]).to include('parent must be in the same project as child.') + let(:restriction) do + WorkItems::HierarchyRestriction + .find_by_parent_type_id_and_child_type_id(cross_project_issue.work_item_type_id, task1.work_item_type_id) + end + + it 'is valid when cross-hierarchy is enabled' do + restriction.update!(cross_hierarchy_enabled: true) + link = build(:parent_link, work_item_parent: cross_project_issue, work_item: task1) + + expect(link).to be_valid + expect(link.errors).to be_empty + end + + it 'is not valid when cross-hierarchy is not enabled' do + restriction.update!(cross_hierarchy_enabled: false) + link = build(:parent_link, work_item_parent: cross_project_issue, work_item: task1) + + expect(link).not_to be_valid + expect(link.errors[:work_item_parent]).to include('parent must be in the same project or group as child.') + end end context 'when parent already has maximum number of links' do diff --git a/spec/models/work_items/related_link_restriction_spec.rb b/spec/models/work_items/related_link_restriction_spec.rb new file mode 100644 index 00000000000..764ada53f8b --- /dev/null +++ b/spec/models/work_items/related_link_restriction_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::RelatedLinkRestriction, feature_category: :portfolio_management do + describe 'associations' do + it { is_expected.to belong_to(:source_type) } + it { is_expected.to belong_to(:target_type) } + end + + describe 'validations' do + before do + # delete seeded records to prevent non-unique record error + described_class.delete_all + end + + subject { build(:related_link_restriction) } + + it { is_expected.to validate_presence_of(:source_type) } + it { is_expected.to validate_presence_of(:target_type) } + it { is_expected.to validate_uniqueness_of(:target_type).scoped_to([:source_type_id, :link_type]) } + end + + describe '.link_type' do + it { is_expected.to define_enum_for(:link_type).with_values(relates_to: 0, blocks: 1) } + end +end diff --git a/spec/models/work_items/related_work_item_link_spec.rb b/spec/models/work_items/related_work_item_link_spec.rb index 3217ac52489..d4a07997052 100644 --- a/spec/models/work_items/related_work_item_link_spec.rb +++ b/spec/models/work_items/related_work_item_link_spec.rb @@ -9,6 +9,8 @@ RSpec.describe WorkItems::RelatedWorkItemLink, type: :model, feature_category: : it_behaves_like 'issuable link' do let_it_be_with_reload(:issuable_link) { create(:work_item_link) } let_it_be(:issuable) { issue } + let_it_be(:issuable2) { create(:work_item, :issue, project: project) } + let_it_be(:issuable3) { create(:work_item, :issue, project: project) } let(:issuable_class) { 'WorkItem' } let(:issuable_link_factory) { :work_item_link } end @@ -21,51 +23,48 @@ RSpec.describe WorkItems::RelatedWorkItemLink, type: :model, feature_category: : let_it_be(:item_type) { described_class.issuable_name } end - describe 'validations' do - let_it_be(:task1) { create(:work_item, :task, project: project) } - let_it_be(:task2) { create(:work_item, :task, project: project) } - let_it_be(:task3) { create(:work_item, :task, project: project) } - - subject(:link) { build(:work_item_link, source_id: task1.id, target_id: task2.id) } + describe '.issuable_type' do + it { expect(described_class.issuable_type).to eq(:issue) } + end - describe '#validate_max_number_of_links' do - shared_examples 'invalid due to exceeding max number of links' do - let(:error_msg) { 'This work item would exceed the maximum number of linked items.' } + describe '.issuable_name' do + it { expect(described_class.issuable_name).to eq('work item') } + end - before do - create(:work_item_link, source: source, target: target) - stub_const("#{described_class}::MAX_LINKS_COUNT", 1) - end + describe 'validations' do + describe '#validate_related_link_restrictions' do + using RSpec::Parameterized::TableSyntax - specify do - is_expected.to be_invalid - expect(link.errors.messages[error_item]).to include(error_msg) - end + where(:source_type_sym, :target_types, :valid) do + :incident | [:incident, :test_case, :issue, :task, :ticket] | false + :ticket | [:incident, :test_case, :issue, :task, :ticket] | false + :test_case | [:incident, :test_case, :issue, :task, :ticket] | false + :task | [:incident, :test_case, :ticket] | false + :issue | [:incident, :test_case, :ticket] | false + :task | [:task, :issue] | true + :issue | [:task, :issue] | true end - context 'when source exceeds max' do - let(:source) { task1 } - let(:target) { task3 } - let(:error_item) { :source } + with_them do + it 'validates the related link' do + target_types.each do |target_type_sym| + source_type = WorkItems::Type.default_by_type(source_type_sym) + target_type = WorkItems::Type.default_by_type(target_type_sym) + source = build(:work_item, work_item_type: source_type, project: project) + target = build(:work_item, work_item_type: target_type, project: project) + link = build(:work_item_link, source: source, target: target) + opposite_link = build(:work_item_link, source: target, target: source) - it_behaves_like 'invalid due to exceeding max number of links' - end - - context 'when target exceeds max' do - let(:source) { task2 } - let(:target) { task3 } - let(:error_item) { :target } + expect(link.valid?).to eq(valid) + expect(opposite_link.valid?).to eq(valid) + next if valid - it_behaves_like 'invalid due to exceeding max number of links' + expect(link.errors.messages[:source]).to contain_exactly( + "#{source_type.name.downcase.pluralize} cannot be related to #{target_type.name.downcase.pluralize}" + ) + end + end end end end - - describe '.issuable_type' do - it { expect(described_class.issuable_type).to eq(:issue) } - end - - describe '.issuable_name' do - it { expect(described_class.issuable_name).to eq('work item') } - end end diff --git a/spec/models/work_items/type_spec.rb b/spec/models/work_items/type_spec.rb index e4d2ccdfc5a..7f836ce4e90 100644 --- a/spec/models/work_items/type_spec.rb +++ b/spec/models/work_items/type_spec.rb @@ -83,6 +83,8 @@ RSpec.describe WorkItems::Type, feature_category: :team_planning do expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).not_to receive(:upsert_types).and_call_original expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).not_to receive(:upsert_widgets) expect(Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter).not_to receive(:upsert_restrictions) + expect(Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter) + .not_to receive(:upsert_restrictions) expect(subject).to eq(default_issue_type) end @@ -96,6 +98,7 @@ RSpec.describe WorkItems::Type, feature_category: :team_planning do expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).to receive(:upsert_types).and_call_original expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).to receive(:upsert_widgets) expect(Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter).to receive(:upsert_restrictions) + expect(Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter).to receive(:upsert_restrictions) expect(subject).to eq(default_issue_type) end |