From 5afcbe03ead9ada87621888a31a62652b10a7e4f Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 20 Sep 2023 11:18:08 +0000 Subject: Add latest changes from gitlab-org/gitlab@16-4-stable-ee --- spec/models/ability_spec.rb | 5 + spec/models/abuse_report_spec.rb | 27 +- spec/models/active_session_spec.rb | 18 +- .../alert_management/http_integration_spec.rb | 2 +- .../alerting/project_alerting_setting_spec.rb | 29 +- .../cycle_analytics/runtime_limiter_spec.rb | 55 ++++ spec/models/application_setting_spec.rb | 12 + spec/models/award_emoji_spec.rb | 33 ++- spec/models/bulk_imports/entity_spec.rb | 8 + spec/models/ci/build_spec.rb | 179 +++++------ spec/models/ci/catalog/listing_spec.rb | 12 +- spec/models/ci/catalog/resource_spec.rb | 6 +- spec/models/ci/catalog/resources/component_spec.rb | 2 +- spec/models/ci/runner_spec.rb | 7 +- spec/models/clusters/agent_token_spec.rb | 39 ++- spec/models/commit_status_spec.rb | 40 ++- spec/models/concerns/as_cte_spec.rb | 2 +- spec/models/concerns/each_batch_spec.rb | 20 ++ spec/models/concerns/expirable_spec.rb | 7 +- spec/models/concerns/has_user_type_spec.rb | 80 ++++- spec/models/concerns/issuable_spec.rb | 31 ++ spec/models/concerns/prometheus_adapter_spec.rb | 22 -- .../concerns/require_email_verification_spec.rb | 2 +- spec/models/concerns/resolvable_discussion_spec.rb | 8 +- spec/models/concerns/routable_spec.rb | 71 ++++- spec/models/concerns/transitionable_spec.rb | 40 +++ spec/models/deploy_key_spec.rb | 2 +- spec/models/design_management/design_spec.rb | 10 - spec/models/doorkeeper/application_spec.rb | 11 + spec/models/environment_status_spec.rb | 12 - spec/models/group_spec.rb | 21 +- spec/models/hooks/web_hook_log_spec.rb | 38 +++ spec/models/integration_spec.rb | 6 +- .../integrations/base_chat_notification_spec.rb | 21 +- .../chat_message/deployment_message_spec.rb | 36 ++- spec/models/integrations/confluence_spec.rb | 6 + spec/models/integrations/mattermost_spec.rb | 2 +- spec/models/integrations/prometheus_spec.rb | 28 ++ spec/models/integrations/shimo_spec.rb | 8 +- spec/models/integrations/slack_spec.rb | 2 +- spec/models/integrations/zentao_spec.rb | 8 +- spec/models/issue_spec.rb | 7 +- .../modification_tracker_spec.rb | 14 +- .../turbo_modification_tracker_spec.rb | 23 ++ spec/models/member_spec.rb | 7 + spec/models/members/group_member_spec.rb | 16 +- spec/models/merge_request_spec.rb | 160 +++++++++- spec/models/metrics/dashboard/annotation_spec.rb | 73 ----- .../models/metrics/users_starred_dashboard_spec.rb | 39 --- spec/models/ml/model_version_spec.rb | 10 +- spec/models/namespace_spec.rb | 326 +++++---------------- spec/models/note_spec.rb | 111 ++++--- spec/models/notification_setting_spec.rb | 7 + spec/models/oauth_access_token_spec.rb | 6 +- spec/models/organizations/organization_spec.rb | 1 + spec/models/packages/dependency_link_spec.rb | 68 ++++- spec/models/packages/ml_model/package_spec.rb | 67 +++++ spec/models/packages/nuget/metadatum_spec.rb | 23 +- spec/models/packages/nuget/symbol_spec.rb | 70 +++++ spec/models/packages/package_spec.rb | 43 ++- spec/models/packages/protection/rule_spec.rb | 40 +++ spec/models/pages/virtual_domain_spec.rb | 53 ++-- spec/models/pages_deployment_spec.rb | 51 ++++ spec/models/pages_domain_spec.rb | 11 + .../prometheus_metric_spec.rb | 67 ----- .../prometheus_panel_group_spec.rb | 62 ---- .../prometheus_panel_spec.rb | 85 ------ spec/models/plan_spec.rb | 12 + spec/models/pool_repository_spec.rb | 46 ++- spec/models/project_authorization_spec.rb | 37 +++ spec/models/project_authorizations/changes_spec.rb | 10 +- spec/models/project_ci_cd_setting_spec.rb | 2 +- spec/models/project_feature_spec.rb | 20 ++ spec/models/project_import_state_spec.rb | 8 + spec/models/project_metrics_setting_spec.rb | 63 ---- spec/models/project_spec.rb | 113 ++++--- spec/models/repository_spec.rb | 31 +- spec/models/resource_label_event_spec.rb | 17 +- spec/models/resource_state_event_spec.rb | 16 +- spec/models/review_spec.rb | 19 ++ spec/models/route_spec.rb | 10 + spec/models/snippet_repository_spec.rb | 2 +- spec/models/user_custom_attribute_spec.rb | 29 +- spec/models/user_preference_spec.rb | 14 + spec/models/user_spec.rb | 125 +------- spec/models/users/credit_card_validation_spec.rb | 143 ++++++++- spec/models/users/group_visit_spec.rb | 25 ++ spec/models/users/project_visit_spec.rb | 25 ++ spec/models/work_item_spec.rb | 81 ++++- .../work_items/related_work_item_link_spec.rb | 40 +++ spec/models/work_items/widgets/description_spec.rb | 2 +- .../models/work_items/widgets/linked_items_spec.rb | 4 +- spec/models/x509_certificate_spec.rb | 1 - spec/models/x509_issuer_spec.rb | 2 - 94 files changed, 2075 insertions(+), 1230 deletions(-) create mode 100644 spec/models/analytics/cycle_analytics/runtime_limiter_spec.rb create mode 100644 spec/models/concerns/transitionable_spec.rb create mode 100644 spec/models/doorkeeper/application_spec.rb create mode 100644 spec/models/loose_foreign_keys/turbo_modification_tracker_spec.rb delete mode 100644 spec/models/metrics/dashboard/annotation_spec.rb delete mode 100644 spec/models/metrics/users_starred_dashboard_spec.rb create mode 100644 spec/models/packages/ml_model/package_spec.rb create mode 100644 spec/models/packages/nuget/symbol_spec.rb create mode 100644 spec/models/packages/protection/rule_spec.rb delete mode 100644 spec/models/performance_monitoring/prometheus_metric_spec.rb delete mode 100644 spec/models/performance_monitoring/prometheus_panel_group_spec.rb delete mode 100644 spec/models/performance_monitoring/prometheus_panel_spec.rb delete mode 100644 spec/models/project_metrics_setting_spec.rb create mode 100644 spec/models/users/group_visit_spec.rb create mode 100644 spec/models/users/project_visit_spec.rb (limited to 'spec/models') diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 422dd9a463b..a808cb1c823 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -270,6 +270,11 @@ RSpec.describe Ability do end describe '.issues_readable_by_user' do + it 'is aliased to .work_items_readable_by_user' do + expect(described_class.method(:issues_readable_by_user)) + .to eq(described_class.method(:work_items_readable_by_user)) + end + context 'with an admin when admin mode is enabled', :enable_admin_mode do it 'returns all given issues' do user = build(:user, admin: true) diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 584f9b010ad..1fa60a210e2 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -385,13 +385,28 @@ RSpec.describe AbuseReport, feature_category: :insider_threat do end end - describe '#other_reports_for_user' do - let(:report) { create(:abuse_report) } - let(:another_user_report) { create(:abuse_report, user: report.user) } - let(:another_report) { create(:abuse_report) } + describe '#past_closed_reports_for_user' do + let(:report_1) { create(:abuse_report, :closed) } + let(:report_2) { create(:abuse_report, user: report.user) } + let(:report_3) { create(:abuse_report, :closed, user: report.user) } - it 'returns other reports for the same user' do - expect(report.other_reports_for_user).to match_array(another_user_report) + it 'returns past closed reports for the same user' do + expect(report.past_closed_reports_for_user).to match_array(report_3) + end + end + + describe '#similar_open_reports_for_user' do + let(:report_1) { create(:abuse_report, category: 'spam') } + let(:report_2) { create(:abuse_report, category: 'spam', user: report.user) } + let(:report_3) { create(:abuse_report, category: 'offensive', user: report.user) } + let(:report_4) { create(:abuse_report, :closed, category: 'spam', user: report.user) } + + it 'returns open reports for the same user and category' do + expect(report.similar_open_reports_for_user).to match_array(report_2) + end + + it 'returns no abuse reports when the report is closed' do + expect(report_4.similar_open_reports_for_user).to match_array(described_class.none) end end diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 54169c254a6..af884fdb83c 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -650,25 +650,13 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do end end - describe '.set_active_user_cookie' do + describe '.set_active_user_cookie', :freeze_time do let(:auth) { double(cookies: {}) } it 'sets marketing cookie' do described_class.set_active_user_cookie(auth) - expect(auth.cookies[:about_gitlab_active_user][:value]).to be_truthy - end - end - - describe '.unset_active_user_cookie' do - let(:auth) { double(cookies: {}) } - - before do - described_class.set_active_user_cookie(auth) - end - - it 'unsets marketing cookie' do - described_class.unset_active_user_cookie(auth) - expect(auth.cookies[:about_gitlab_active_user]).to be_nil + expect(auth.cookies[:gitlab_user][:value]).to be_truthy + expect(auth.cookies[:gitlab_user][:expires]).to be_within(1.minute).of(2.weeks.from_now) end end end diff --git a/spec/models/alert_management/http_integration_spec.rb b/spec/models/alert_management/http_integration_spec.rb index 479ae8a4966..dc26d0323d7 100644 --- a/spec/models/alert_management/http_integration_spec.rb +++ b/spec/models/alert_management/http_integration_spec.rb @@ -7,7 +7,7 @@ RSpec.describe AlertManagement::HttpIntegration, feature_category: :incident_man let_it_be(:project) { create(:project) } - subject(:integration) { build(:alert_management_http_integration) } + subject(:integration) { build(:alert_management_http_integration, project: project) } describe 'associations' do it { is_expected.to belong_to(:project) } diff --git a/spec/models/alerting/project_alerting_setting_spec.rb b/spec/models/alerting/project_alerting_setting_spec.rb index 90c5f8313b0..0424b94a1b7 100644 --- a/spec/models/alerting/project_alerting_setting_spec.rb +++ b/spec/models/alerting/project_alerting_setting_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Alerting::ProjectAlertingSetting do +RSpec.describe Alerting::ProjectAlertingSetting, feature_category: :incident_management do let_it_be(:project) { create(:project) } subject { create(:project_alerting_setting, project: project) } @@ -37,4 +37,31 @@ RSpec.describe Alerting::ProjectAlertingSetting do end end end + + describe '#sync_http_integration after_save callback' do + let_it_be_with_reload(:setting) { create(:project_alerting_setting, :with_http_integration, project: project) } + let_it_be_with_reload(:http_integration) { setting.project.alert_management_http_integrations.last! } + let_it_be(:new_token) { 'new_token' } + + context 'with corresponding HTTP integration' do + let_it_be(:original_token) { http_integration.token } + + it 'syncs the attribute' do + expect { setting.update!(token: new_token) } + .to change { http_integration.reload.token } + .from(original_token).to(new_token) + end + end + + context 'without corresponding HTTP integration' do + before do + http_integration.update_columns(endpoint_identifier: 'legacy') + end + + it 'does not sync the attribute or execute extra queries' do + expect { setting.update!(token: new_token) } + .not_to change { http_integration.reload.token } + end + end + end end diff --git a/spec/models/analytics/cycle_analytics/runtime_limiter_spec.rb b/spec/models/analytics/cycle_analytics/runtime_limiter_spec.rb new file mode 100644 index 00000000000..1a5200719c8 --- /dev/null +++ b/spec/models/analytics/cycle_analytics/runtime_limiter_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Analytics::CycleAnalytics::RuntimeLimiter, feature_category: :value_stream_management do + let(:max_runtime) { 321 } + let(:runtime_limiter) { described_class.new(max_runtime) } + + describe '#elapsed_time' do + it 'reports monotonic elapsed time since instantiation' do + elapsed = 123 + first_monotonic_time = 100 + second_monotonic_time = first_monotonic_time + elapsed + + expect(Gitlab::Metrics::System).to receive(:monotonic_time) + .and_return(first_monotonic_time, second_monotonic_time) + + expect(runtime_limiter.elapsed_time).to eq(elapsed) + end + end + + describe '#over_time?' do + it 'returns true if over time' do + start_time = 100 + allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(start_time, start_time + max_runtime - 1) + + expect(runtime_limiter.over_time?).to be(false) + + allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(start_time + max_runtime) + expect(runtime_limiter.over_time?).to be(true) + + allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(start_time + max_runtime + 1) + expect(runtime_limiter.over_time?).to be(true) + end + end + + describe '#was_over_time?' do + it 'returns true if over_time? returned true at an earlier step' do + first_monotonic_time = 10 + second_monotonic_time = first_monotonic_time + 50 + third_monotonic_time = second_monotonic_time + 50 # over time: 110 > 100 + + expect(Gitlab::Metrics::System).to receive(:monotonic_time) + .and_return(first_monotonic_time, second_monotonic_time, third_monotonic_time) + + runtime_limiter = described_class.new(100) + + expect(runtime_limiter.over_time?).to be(false) # uses the second_monotonic_time + expect(runtime_limiter.was_over_time?).to be(false) + + expect(runtime_limiter.over_time?).to be(true) # uses the third_monotonic_time + expect(runtime_limiter.was_over_time?).to be(true) + end + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 5a70bec8b33..3fc7d8f6fc8 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -25,6 +25,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { expect(setting.kroki_formats).to eq({}) } it { expect(setting.default_branch_protection_defaults).to eq({}) } it { expect(setting.max_decompressed_archive_size).to eq(25600) } + it { expect(setting.decompress_archive_file_timeout).to eq(210) } end describe 'validations' do @@ -134,6 +135,9 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_presence_of(:container_registry_import_target_plan) } it { is_expected.to validate_presence_of(:container_registry_import_created_before) } + it { is_expected.to validate_numericality_of(:decompress_archive_file_timeout).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.not_to allow_value(nil).for(:decompress_archive_file_timeout) } + it { is_expected.to validate_numericality_of(:dependency_proxy_ttl_group_policy_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.not_to allow_value(nil).for(:dependency_proxy_ttl_group_policy_worker_capacity) } @@ -237,6 +241,11 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.not_to allow_value(nil).for(:users_get_by_id_limit_allowlist) } it { is_expected.to allow_value([]).for(:users_get_by_id_limit_allowlist) } + it { is_expected.to allow_value(many_usernames(100)).for(:search_rate_limit_allowlist) } + it { is_expected.not_to allow_value(many_usernames(101)).for(:search_rate_limit_allowlist) } + it { is_expected.not_to allow_value(nil).for(:search_rate_limit_allowlist) } + it { is_expected.to allow_value([]).for(:search_rate_limit_allowlist) } + it { is_expected.to allow_value('all_tiers').for(:whats_new_variant) } it { is_expected.to allow_value('current_tier').for(:whats_new_variant) } it { is_expected.to allow_value('disabled').for(:whats_new_variant) } @@ -436,11 +445,14 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.not_to allow_value(nil).for(:snowplow_collector_hostname) } it { is_expected.to allow_value("snowplow.gitlab.com").for(:snowplow_collector_hostname) } + it { is_expected.to allow_value("db-snowplow.gitlab.com").for(:snowplow_database_collector_hostname) } + it { is_expected.not_to allow_value("#{'a' * 256}db-snowplow.gitlab.com").for(:snowplow_database_collector_hostname) } it { is_expected.not_to allow_value('/example').for(:snowplow_collector_hostname) } end context 'when snowplow is not enabled' do it { is_expected.to allow_value(nil).for(:snowplow_collector_hostname) } + it { is_expected.to allow_value(nil).for(:snowplow_database_collector_hostname) } end end diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index 586ec8f723a..b179f2df816 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -RSpec.describe AwardEmoji do +RSpec.describe AwardEmoji, feature_category: :team_planning do + let_it_be(:user) { create(:user) } + describe 'Associations' do it { is_expected.to belong_to(:awardable) } it { is_expected.to belong_to(:user) } @@ -60,7 +62,6 @@ RSpec.describe AwardEmoji do end context 'custom emoji' do - let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:emoji) { create(:custom_emoji, name: 'partyparrot', namespace: group) } let_it_be(:project) { create(:project, namespace: group) } @@ -144,27 +145,27 @@ RSpec.describe AwardEmoji do describe 'broadcasting updates' do context 'on a note' do let(:note) { create(:note_on_issue) } - let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: note) } + let(:award_emoji) { build(:award_emoji, user: user, awardable: note) } it 'broadcasts updates on the note when saved' do - expect(note).to receive(:expire_etag_cache) + expect(note).to receive(:broadcast_noteable_notes_changed) expect(note).to receive(:trigger_note_subscription_update) award_emoji.save! end it 'broadcasts updates on the note when destroyed' do - expect(note).to receive(:expire_etag_cache) + expect(note).to receive(:broadcast_noteable_notes_changed) expect(note).to receive(:trigger_note_subscription_update) award_emoji.destroy! end context 'when importing' do - let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: note, importing: true) } + let(:award_emoji) { build(:award_emoji, user: user, awardable: note, importing: true) } it 'does not broadcast updates on the note when saved' do - expect(note).not_to receive(:expire_etag_cache) + expect(note).not_to receive(:broadcast_noteable_notes_changed) expect(note).not_to receive(:trigger_note_subscription_update) award_emoji.save! @@ -174,17 +175,17 @@ RSpec.describe AwardEmoji do context 'on another awardable' do let(:issue) { create(:issue) } - let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: issue) } + let(:award_emoji) { build(:award_emoji, user: user, awardable: issue) } it 'does not broadcast updates on the issue when saved' do - expect(issue).not_to receive(:expire_etag_cache) + expect(issue).not_to receive(:broadcast_noteable_notes_changed) expect(issue).not_to receive(:trigger_note_subscription_update) award_emoji.save! end it 'does not broadcast updates on the issue when destroyed' do - expect(issue).not_to receive(:expire_etag_cache) + expect(issue).not_to receive(:broadcast_noteable_notes_changed) expect(issue).not_to receive(:trigger_note_subscription_update) award_emoji.destroy! @@ -194,7 +195,7 @@ RSpec.describe AwardEmoji do describe 'bumping updated at' do let(:note) { create(:note_on_issue) } - let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: note) } + let(:award_emoji) { build(:award_emoji, user: user, awardable: note) } it 'calls bump_updated_at on the note when saved' do expect(note).to receive(:bump_updated_at) @@ -210,7 +211,7 @@ RSpec.describe AwardEmoji do context 'on another awardable' do let(:issue) { create(:issue) } - let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: issue) } + let(:award_emoji) { build(:award_emoji, user: user, awardable: issue) } it 'does not error out when saved' do expect { award_emoji.save! }.not_to raise_error @@ -248,8 +249,8 @@ RSpec.describe AwardEmoji do describe 'updating upvotes_count' do context 'on an issue' do let(:issue) { create(:issue) } - let(:upvote) { build(:award_emoji, :upvote, user: build(:user), awardable: issue) } - let(:downvote) { build(:award_emoji, :downvote, user: build(:user), awardable: issue) } + let(:upvote) { build(:award_emoji, :upvote, user: user, awardable: issue) } + let(:downvote) { build(:award_emoji, :downvote, user: user, awardable: issue) } it 'updates upvotes_count on the issue when saved' do expect(issue).to receive(:update_column).with(:upvotes_count, 1).once @@ -268,7 +269,7 @@ RSpec.describe AwardEmoji do context 'on another awardable' do let(:merge_request) { create(:merge_request) } - let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: merge_request) } + let(:award_emoji) { build(:award_emoji, user: user, awardable: merge_request) } it 'does not update upvotes_count on the merge_request when saved' do expect(merge_request).not_to receive(:update_column) @@ -329,7 +330,7 @@ RSpec.describe AwardEmoji do describe '#to_ability_name' do let(:merge_request) { create(:merge_request) } - let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: merge_request) } + let(:award_emoji) { build(:award_emoji, user: user, awardable: merge_request) } it 'returns correct ability name' do expect(award_emoji.to_ability_name).to be('emoji') diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index da1eb12e9b8..3e98ba0973e 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -438,4 +438,12 @@ RSpec.describe BulkImports::Entity, type: :model, feature_category: :importers d end end end + + describe '#source_version' do + subject { build(:bulk_import_entity, :group_entity) } + + it 'pulls the source version from the associated BulkImport' do + expect(subject.source_version).to eq(subject.bulk_import.source_version_info) + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index a556244ae00..2a5d781edc7 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_default: :keep do + using RSpec::Parameterized::TableSyntax include Ci::TemplateHelpers include AfterNextHelpers @@ -1493,8 +1494,6 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end describe 'state transition metrics' do - using RSpec::Parameterized::TableSyntax - subject { build.send(event) } where(:state, :report_count, :trait) do @@ -2129,8 +2128,6 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end describe '#ref_slug' do - using RSpec::Parameterized::TableSyntax - where(:ref, :slug) do 'master' | 'master' '1-foo' | '1-foo' @@ -2468,8 +2465,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def context 'when build has environment and user-provided variables' do let(:expected_variables) do predefined_variables.map { |variable| variable.fetch(:key) } + - %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG - CI_ENVIRONMENT_ACTION CI_ENVIRONMENT_TIER CI_ENVIRONMENT_URL] + %w[YAML_VARIABLE CI_ENVIRONMENT_SLUG CI_ENVIRONMENT_URL] end before do @@ -2478,8 +2474,14 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def build.yaml_variables = [{ key: 'YAML_VARIABLE', value: 'var', public: true }] build.environment = 'staging' - # CI_ENVIRONMENT_NAME is set in predefined_variables when job environment is provided - predefined_variables.insert(18, { key: 'CI_ENVIRONMENT_NAME', value: 'staging', public: true, masked: false }) + insert_expected_predefined_variables( + [ + { key: 'CI_ENVIRONMENT_NAME', value: 'staging', public: true, masked: false }, + { key: 'CI_ENVIRONMENT_ACTION', value: 'start', public: true, masked: false }, + { key: 'CI_ENVIRONMENT_TIER', value: 'staging', public: true, masked: false }, + { key: 'CI_ENVIRONMENT_URL', value: 'https://gitlab.com', public: true, masked: false } + ], + after: 'CI_NODE_TOTAL') end it 'matches explicit variables ordering' do @@ -2550,6 +2552,11 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def expect(runner_vars).not_to include('CI_JOB_JWT_V2') end end + + def insert_expected_predefined_variables(variables, after:) + index = predefined_variables.index { |h| h[:key] == after } + predefined_variables.insert(index + 1, *variables) + end end context 'when build has user' do @@ -2583,34 +2590,28 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end context 'when build has an environment' do - let(:environment_variables) do + let(:expected_environment_variables) do [ { key: 'CI_ENVIRONMENT_NAME', value: 'production', public: true, masked: false }, - { key: 'CI_ENVIRONMENT_SLUG', value: 'prod-slug', public: true, masked: false }, - { key: 'CI_ENVIRONMENT_TIER', value: 'production', public: true, masked: false } + { key: 'CI_ENVIRONMENT_ACTION', value: 'start', public: true, masked: false }, + { key: 'CI_ENVIRONMENT_TIER', value: 'production', public: true, masked: false }, + { key: 'CI_ENVIRONMENT_URL', value: 'http://prd.example.com/$CI_JOB_NAME', public: true, masked: false } ] end - let!(:environment) do - create( - :environment, - project: build.project, - name: 'production', - slug: 'prod-slug', - tier: 'production', - external_url: '' - ) - end - - before do - build.update!(environment: 'production') - end + let(:build) { create(:ci_build, :with_deployment, :deploy_to_production, ref: pipeline.ref, pipeline: pipeline) } shared_examples 'containing environment variables' do - it { is_expected.to include(*environment_variables) } + it { is_expected.to include(*expected_environment_variables) } end context 'when no URL was set' do + before do + build.update!(options: { environment: { url: nil } }) + build.persisted_environment.update!(external_url: nil) + expected_environment_variables.delete_if { |var| var[:key] == 'CI_ENVIRONMENT_URL' } + end + it_behaves_like 'containing environment variables' it 'does not have CI_ENVIRONMENT_URL' do @@ -2620,12 +2621,26 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end end + context 'when environment is created dynamically' do + let(:build) { create(:ci_build, :with_deployment, :start_review_app, ref: pipeline.ref, pipeline: pipeline) } + + let(:expected_environment_variables) do + [ + { key: 'CI_ENVIRONMENT_NAME', value: 'review/master', public: true, masked: false }, + { key: 'CI_ENVIRONMENT_ACTION', value: 'start', public: true, masked: false }, + { key: 'CI_ENVIRONMENT_TIER', value: 'development', public: true, masked: false }, + { key: 'CI_ENVIRONMENT_URL', value: 'http://staging.example.com/$CI_JOB_NAME', public: true, masked: false } + ] + end + + it_behaves_like 'containing environment variables' + end + context 'when an URL was set' do let(:url) { 'http://host/test' } before do - environment_variables << - { key: 'CI_ENVIRONMENT_URL', value: url, public: true, masked: false } + expected_environment_variables.find { |var| var[:key] == 'CI_ENVIRONMENT_URL' }[:value] = url end context 'when the URL was set from the job' do @@ -2641,14 +2656,15 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def it_behaves_like 'containing environment variables' it 'puts $CI_ENVIRONMENT_URL in the last so all other variables are available to be used when runners are trying to expand it' do - expect(subject.to_runner_variables.last).to eq(environment_variables.last) + expect(subject.to_runner_variables.last).to eq(expected_environment_variables.last) end end end context 'when the URL was not set from the job, but environment' do before do - environment.update!(external_url: url) + build.update!(options: { environment: { url: nil } }) + build.persisted_environment.update!(external_url: url) end it_behaves_like 'containing environment variables' @@ -2682,10 +2698,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end context 'when environment scope matches build environment' do - before do - create(:environment, name: 'staging', project: project) - build.update!(environment: 'staging') - end + let(:build) { create(:ci_build, :with_deployment, :start_staging, ref: pipeline.ref, pipeline: pipeline) } it { is_expected.to include(environment_specific_variable) } end @@ -4001,73 +4014,61 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end end - describe 'pages deployments' do - let_it_be(:build, reload: true) { create(:ci_build, pipeline: pipeline, user: user) } + describe '#pages_generator?', feature_category: :pages do + where(:name, :enabled, :result) do + 'foo' | false | false + 'pages' | false | false + 'pages:preview' | true | false + 'pages' | true | true + end - context 'when job is "pages"' do + with_them do before do - build.name = 'pages' + stub_pages_setting(enabled: enabled) + build.update!(name: name) end - context 'when pages are enabled' do - before do - allow(Gitlab.config.pages).to receive_messages(enabled: true) - end - - it 'is marked as pages generator' do - expect(build).to be_pages_generator - end - - context 'job succeeds' do - it "calls pages worker" do - expect(PagesWorker).to receive(:perform_async).with(:deploy, build.id) + subject { build.pages_generator? } - build.success! - end - end + it { is_expected.to eq(result) } + end + end - context 'job fails' do - it "does not call pages worker" do - expect(PagesWorker).not_to receive(:perform_async) + describe 'pages deployments', feature_category: :pages do + let_it_be(:build, reload: true) { create(:ci_build, name: 'pages', pipeline: pipeline, user: user) } - build.drop! - end - end + context 'when pages are enabled' do + before do + stub_pages_setting(enabled: true) end - context 'when pages are disabled' do - before do - allow(Gitlab.config.pages).to receive_messages(enabled: false) - end + context 'and job succeeds' do + it "calls pages worker" do + expect(PagesWorker).to receive(:perform_async).with(:deploy, build.id) - it 'is not marked as pages generator' do - expect(build).not_to be_pages_generator + build.success! end + end - context 'job succeeds' do - it "does not call pages worker" do - expect(PagesWorker).not_to receive(:perform_async) + context 'and job fails' do + it "does not call pages worker" do + expect(PagesWorker).not_to receive(:perform_async) - build.success! - end + build.drop! end end end - context 'when job is not "pages"' do + context 'when pages are disabled' do before do - build.name = 'other-job' + stub_pages_setting(enabled: false) end - it 'is not marked as pages generator' do - expect(build).not_to be_pages_generator - end - - context 'job succeeds' do + context 'and job succeeds' do it "does not call pages worker" do expect(PagesWorker).not_to receive(:perform_async) - build.success + build.success! end end end @@ -5604,26 +5605,4 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end end end - - describe 'routing table switch' do - context 'with ff disabled' do - before do - stub_feature_flags(ci_partitioning_use_ci_builds_routing_table: false) - end - - it 'uses the legacy table' do - expect(described_class.table_name).to eq('ci_builds') - end - end - - context 'with ff enabled' do - before do - stub_feature_flags(ci_partitioning_use_ci_builds_routing_table: true) - end - - it 'uses the routing table' do - expect(described_class.table_name).to eq('p_ci_builds') - end - end - end end diff --git a/spec/models/ci/catalog/listing_spec.rb b/spec/models/ci/catalog/listing_spec.rb index 159b70d7f8f..f28a0e82bbd 100644 --- a/spec/models/ci/catalog/listing_spec.rb +++ b/spec/models/ci/catalog/listing_spec.rb @@ -34,9 +34,9 @@ RSpec.describe Ci::Catalog::Listing, feature_category: :pipeline_composition do end context 'when the namespace has catalog resources' do - let_it_be(:resource) { create(:catalog_resource, project: project_1) } - let_it_be(:resource_2) { create(:catalog_resource, project: project_2) } - let_it_be(:other_namespace_resource) { create(:catalog_resource, project: project_3) } + 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) } it 'contains only catalog resources for projects in that namespace' do is_expected.to contain_exactly(resource, resource_2) @@ -65,8 +65,8 @@ RSpec.describe Ci::Catalog::Listing, feature_category: :pipeline_composition do end context 'when the user only has access to some projects in the namespace' do - let!(:resource_1) { create(:catalog_resource, project: project_1) } - let!(:resource_2) { create(:catalog_resource, project: project_2) } + let!(:resource_1) { create(:ci_catalog_resource, project: project_1) } + let!(:resource_2) { create(:ci_catalog_resource, project: project_2) } before do project_1.add_developer(user) @@ -79,7 +79,7 @@ RSpec.describe Ci::Catalog::Listing, feature_category: :pipeline_composition do end context 'when the user does not have access to the namespace' do - let!(:resource) { create(:catalog_resource, project: project_1) } + let!(:resource) { create(:ci_catalog_resource, project: project_1) } it { is_expected.to be_empty } end diff --git a/spec/models/ci/catalog/resource_spec.rb b/spec/models/ci/catalog/resource_spec.rb index 4608e611ea1..082283bb7bc 100644 --- a/spec/models/ci/catalog/resource_spec.rb +++ b/spec/models/ci/catalog/resource_spec.rb @@ -6,9 +6,9 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do 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(:catalog_resource, project: project) } - let_it_be(:resource_2) { create(:catalog_resource, project: project_2) } - let_it_be(:resource_3) { create(:catalog_resource, project: project_3) } + 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(: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) } diff --git a/spec/models/ci/catalog/resources/component_spec.rb b/spec/models/ci/catalog/resources/component_spec.rb index caaf76e610d..e8c92ce0788 100644 --- a/spec/models/ci/catalog/resources/component_spec.rb +++ b/spec/models/ci/catalog/resources/component_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::Catalog::Resources::Component, type: :model, feature_category: :pipeline_composition do - let(:component) { build(:catalog_resource_component) } + let(:component) { build(:ci_catalog_resource_component) } it { is_expected.to belong_to(:catalog_resource).class_name('Ci::Catalog::Resource') } it { is_expected.to belong_to(:project) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 56e69cc2b9c..a8e9d36a3a7 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -1216,13 +1216,12 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do before do runner.tick_runner_queue - runner.destroy! end it 'cleans up the queue' do - Gitlab::Redis::Cache.with do |redis| - expect(redis.get(queue_key)).to be_nil - end + expect(Gitlab::Workhorse).to receive(:cleanup_key).with(queue_key) + + runner.destroy! end end end diff --git a/spec/models/clusters/agent_token_spec.rb b/spec/models/clusters/agent_token_spec.rb index 41f8215b713..bc158fc9117 100644 --- a/spec/models/clusters/agent_token_spec.rb +++ b/spec/models/clusters/agent_token_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Clusters::AgentToken do +RSpec.describe Clusters::AgentToken, feature_category: :deployment_management do it { is_expected.to belong_to(:agent).class_name('Clusters::Agent').required } it { is_expected.to belong_to(:created_by_user).class_name('User').optional } it { is_expected.to validate_length_of(:description).is_at_most(1024) } @@ -12,8 +12,9 @@ RSpec.describe Clusters::AgentToken do it_behaves_like 'having unique enum values' describe 'scopes' do + let_it_be(:agent) { create(:cluster_agent) } + describe '.order_last_used_at_desc' do - let_it_be(:agent) { create(:cluster_agent) } let_it_be(:token_1) { create(:cluster_agent_token, agent: agent, last_used_at: 7.days.ago) } let_it_be(:token_2) { create(:cluster_agent_token, agent: agent, last_used_at: nil) } let_it_be(:token_3) { create(:cluster_agent_token, agent: agent, last_used_at: 2.days.ago) } @@ -25,8 +26,8 @@ RSpec.describe Clusters::AgentToken do end describe 'status-related scopes' do - let!(:active_token) { create(:cluster_agent_token) } - let!(:revoked_token) { create(:cluster_agent_token, :revoked) } + let!(:active_token) { create(:cluster_agent_token, agent: agent) } + let!(:revoked_token) { create(:cluster_agent_token, :revoked, agent: agent) } describe '.with_status' do context 'when filtering by active status' do @@ -48,6 +49,29 @@ RSpec.describe Clusters::AgentToken do it { is_expected.to contain_exactly(active_token) } end end + + describe '.connected' do + let!(:token) { create(:cluster_agent_token, agent: agent, status: status, last_used_at: last_used_at) } + + let(:status) { :active } + let(:last_used_at) { 2.minutes.ago } + + subject { described_class.connected } + + it { is_expected.to contain_exactly(token) } + + context 'when the token has not been used recently' do + let(:last_used_at) { 2.hours.ago } + + it { is_expected.to be_empty } + end + + context 'when the token is not active' do + let(:status) { :revoked } + + it { is_expected.to be_empty } + end + end end describe '#token' do @@ -64,6 +88,13 @@ RSpec.describe Clusters::AgentToken do agent_token = create(:cluster_agent_token) expect(agent_token.token.length).to be >= 50 end + + it 'has a prefix' do + agent_token = build(:cluster_agent_token, token_encrypted: nil) + agent_token.save! + + expect(agent_token.token).to start_with described_class::TOKEN_PREFIX + end end describe '#to_ability_name' do diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 9ce9f0e13b5..e9257b08bca 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -44,6 +44,24 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do it { is_expected.not_to be_retried } it { expect(described_class.primary_key).to eq('id') } + describe '.switch_table_names' do + before do + stub_env('USE_CI_BUILDS_ROUTING_TABLE', flag_value) + end + + context 'with the env flag disabled' do + let(:flag_value) { 'false' } + + it { expect(described_class.switch_table_names).to eq(:ci_builds) } + end + + context 'with the env flag enabled' do + let(:flag_value) { 'true' } + + it { expect(described_class.switch_table_names).to eq(:p_ci_builds) } + end + end + describe '#author' do subject { commit_status.author } @@ -1067,26 +1085,4 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do it_behaves_like 'having enum with nil value' end - - describe 'routing table switch' do - context 'with ff disabled' do - before do - stub_feature_flags(ci_partitioning_use_ci_builds_routing_table: false) - end - - it 'uses the legacy table' do - expect(described_class.table_name).to eq('ci_builds') - end - end - - context 'with ff enabled' do - before do - stub_feature_flags(ci_partitioning_use_ci_builds_routing_table: true) - end - - it 'uses the routing table' do - expect(described_class.table_name).to eq('p_ci_builds') - end - end - end end diff --git a/spec/models/concerns/as_cte_spec.rb b/spec/models/concerns/as_cte_spec.rb index 06d9650ec46..c92d46ef25f 100644 --- a/spec/models/concerns/as_cte_spec.rb +++ b/spec/models/concerns/as_cte_spec.rb @@ -21,7 +21,7 @@ RSpec.describe AsCte do it { expect(subject.query).to eq(query) } it { expect(subject.table.name).to eq(name.to_s) } - context 'with materialized parameter', if: Gitlab::Database::AsWithMaterialized.materialized_supported? do + context 'with materialized parameter' do subject { query.as_cte(name, materialized: materialized).to_arel.to_sql } context 'as true' do diff --git a/spec/models/concerns/each_batch_spec.rb b/spec/models/concerns/each_batch_spec.rb index 75c5cac899b..f274f8c96ff 100644 --- a/spec/models/concerns/each_batch_spec.rb +++ b/spec/models/concerns/each_batch_spec.rb @@ -75,6 +75,26 @@ RSpec.describe EachBatch do expect(ids).to eq(ids.sort.reverse) end + shared_examples 'preloaded batch' do |method| + it 'respects preloading without N+1 queries' do + one, two = User.first(2) + + create(:key, user: one) + + scope = User.send(method, :keys) + + control = ActiveRecord::QueryRecorder.new { scope.each_batch(of: 5) { |batch| batch.each(&:keys) } } + + create(:key, user: one) + create(:key, user: two) + + expect { scope.each_batch(of: 5) { |batch| batch.each(&:keys) } }.not_to exceed_query_limit(control) + end + end + + it_behaves_like 'preloaded batch', :preload + it_behaves_like 'preloaded batch', :includes + describe 'current scope' do let(:entry) { create(:user, sign_in_count: 1) } let(:ids_with_new_relation) { model.where(id: entry.id).pluck(:id) } diff --git a/spec/models/concerns/expirable_spec.rb b/spec/models/concerns/expirable_spec.rb index 78fe265a6bb..f7f1ce611b4 100644 --- a/spec/models/concerns/expirable_spec.rb +++ b/spec/models/concerns/expirable_spec.rb @@ -17,8 +17,11 @@ RSpec.describe Expirable do it 'scopes the query when multiple models are expirable' do expired_access_token = create(:personal_access_token, :expired, user: no_expire.user) - expect(PersonalAccessToken.expired.joins(user: :members)).to match_array([expired_access_token]) - expect(PersonalAccessToken.joins(user: :members).merge(ProjectMember.expired)).to eq([]) + ::Gitlab::Database.allow_cross_joins_across_databases(url: + 'https://gitlab.com/gitlab-org/gitlab/-/issues/422405') do + expect(PersonalAccessToken.expired.joins(user: :members)).to match_array([expired_access_token]) + expect(PersonalAccessToken.joins(user: :members).merge(ProjectMember.expired)).to eq([]) + end end it 'works with a timestamp expired_at field', time_travel_to: '2022-03-14T11:30:00Z' do diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index 49c3d11ed6b..54614ec2b21 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -3,6 +3,13 @@ require 'spec_helper' RSpec.describe User, feature_category: :system_access do + User::USER_TYPES.keys.each do |type| # rubocop:disable RSpec/UselessDynamicDefinition + let_it_be(type) { create(:user, username: type, user_type: type) } + end + let(:bots) { User::BOT_USER_TYPES.map { |type| public_send(type) } } + let(:non_internal) { User::NON_INTERNAL_USER_TYPES.map { |type| public_send(type) } } + let(:everyone) { User::USER_TYPES.keys.map { |type| public_send(type) } } + specify 'types consistency checks', :aggregate_failures do expect(described_class::USER_TYPES.keys) .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot @@ -20,13 +27,6 @@ RSpec.describe User, feature_category: :system_access do end describe 'scopes & predicates' do - User::USER_TYPES.keys.each do |type| # rubocop:disable RSpec/UselessDynamicDefinition - let_it_be(type) { create(:user, username: type, user_type: type) } - end - let(:bots) { User::BOT_USER_TYPES.map { |type| public_send(type) } } - let(:non_internal) { User::NON_INTERNAL_USER_TYPES.map { |type| public_send(type) } } - let(:everyone) { User::USER_TYPES.keys.map { |type| public_send(type) } } - describe '.bots' do it 'includes all bots' do expect(described_class.bots).to match_array(bots) @@ -118,5 +118,71 @@ RSpec.describe User, feature_category: :system_access do end end end + + describe '#resource_bot_resource' do + let_it_be(:group) { create(:group) } + let_it_be(:group2) { create(:group) } + let_it_be(:project) { create(:project) } + let_it_be(:project2) { create(:project) } + + using RSpec::Parameterized::TableSyntax + + where(:bot_user, :member_of, :owning_resource) do + ref(:human) | [ref(:group)] | nil + ref(:project_bot) | [] | nil # orphaned project bot + ref(:project_bot) | [ref(:group)] | ref(:group) + ref(:project_bot) | [ref(:project)] | ref(:project) + + # Project bot can only be added to one group or project. + # That first group or project becomes the owning resource. + ref(:project_bot) | [ref(:group), ref(:project)] | ref(:group) + ref(:project_bot) | [ref(:group), ref(:group2)] | ref(:group) + ref(:project_bot) | [ref(:project), ref(:group)] | ref(:project) + ref(:project_bot) | [ref(:project), ref(:project2)] | ref(:project) + end + + with_them do + before do + member_of.each { |resource| resource.add_developer(bot_user) } + end + + it 'returns the owning resource' do + expect(bot_user.resource_bot_resource).to eq(owning_resource) + end + end + end + + describe 'resource_bot_owners' do + it 'returns nil when user is not a project bot' do + expect(human.resource_bot_resource).to be_nil + end + + context 'when the user is a project bot' do + let(:user1) { create(:user) } + let(:user2) { create(:user) } + + subject(:owners) { project_bot.resource_bot_owners } + + it 'returns an empty array when there is no owning resource' do + expect(owners).to match_array([]) + end + + it 'returns group owners when owned by a group' do + group = create(:group) + group.add_developer(project_bot) + group.add_owner(user1) + + expect(owners).to match_array([user1]) + end + + it 'returns project maintainers when owned by a project' do + project = create(:project) + project.add_developer(project_bot) + project.add_maintainer(user2) + + expect(owners).to match_array([user2]) + end + end + end end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index e4af778b967..705f8f46a90 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -626,6 +626,21 @@ RSpec.describe Issuable, feature_category: :team_planning do end end + describe "#importing_or_transitioning?" do + let(:merge_request) { build(:merge_request, transitioning: transitioning, importing: importing) } + + where(:transitioning, :importing, :result) do + true | false | true + false | true | true + true | true | true + false | false | false + end + + with_them do + it { expect(merge_request.importing_or_transitioning?).to eq(result) } + end + end + describe '#labels_array' do let(:project) { create(:project) } let(:bug) { create(:label, project: project, title: 'bug') } @@ -1023,6 +1038,22 @@ RSpec.describe Issuable, feature_category: :team_planning do end end + describe '#supports_lock_on_merge?' do + where(:issuable_type, :supports_lock_on_merge) do + :issue | false + :merge_request | false + :incident | false + end + + with_them do + let(:issuable) { build_stubbed(issuable_type) } + + subject { issuable.supports_lock_on_merge? } + + it { is_expected.to eq(supports_lock_on_merge) } + end + end + describe '#severity' do subject { issuable.severity } diff --git a/spec/models/concerns/prometheus_adapter_spec.rb b/spec/models/concerns/prometheus_adapter_spec.rb index 31ab8c23a84..a3f2e99f3da 100644 --- a/spec/models/concerns/prometheus_adapter_spec.rb +++ b/spec/models/concerns/prometheus_adapter_spec.rb @@ -100,28 +100,6 @@ RSpec.describe PrometheusAdapter, :use_clean_rails_memory_store_caching do end end end - - describe 'additional_metrics' do - let(:additional_metrics_environment_query) { Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery } - let(:environment) { build_stubbed(:environment, slug: 'env-slug') } - let(:time_window) { [1552642245.067, 1552642095.831] } - - around do |example| - freeze_time { example.run } - end - - context 'with valid data' do - subject { integration.query(:additional_metrics_environment, environment, *time_window) } - - before do - stub_reactive_cache(integration, prometheus_data, additional_metrics_environment_query, environment.id, *time_window) - end - - it 'returns reactive data' do - expect(subject).to eq(prometheus_data) - end - end - end end describe '#calculate_reactive_cache' do diff --git a/spec/models/concerns/require_email_verification_spec.rb b/spec/models/concerns/require_email_verification_spec.rb index 1fb54e4276f..63312d4e1f1 100644 --- a/spec/models/concerns/require_email_verification_spec.rb +++ b/spec/models/concerns/require_email_verification_spec.rb @@ -51,7 +51,7 @@ RSpec.describe RequireEmailVerification, feature_category: :insider_threat do context 'when failed_attempts is LT overridden amount' do before do - instance.failed_attempts = 5 + instance.failed_attempts = 2 end it { is_expected.to eq(false) } diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb index 1423b56fa5d..e83a3d3417e 100644 --- a/spec/models/concerns/resolvable_discussion_spec.rb +++ b/spec/models/concerns/resolvable_discussion_spec.rb @@ -446,8 +446,8 @@ RSpec.describe Discussion, ResolvableDiscussion, feature_category: :code_review_ expect(subject.resolved?).to be true end - it "expires the etag cache of the noteable" do - expect(subject.noteable).to receive(:expire_note_etag_cache) + it "broadcasts note change of the noteable" do + expect(subject.noteable).to receive(:broadcast_notes_changed) subject.resolve!(current_user) end @@ -532,8 +532,8 @@ RSpec.describe Discussion, ResolvableDiscussion, feature_category: :code_review_ expect(subject.resolved?).to be false end - it "expires the etag cache of the noteable" do - expect(subject.noteable).to receive(:expire_note_etag_cache) + it "broadcasts note change of the noteable" do + expect(subject.noteable).to receive(:broadcast_notes_changed) subject.unresolve! end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 0bbe3dea812..2b6f8535743 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -3,24 +3,20 @@ require 'spec_helper' RSpec.shared_examples 'routable resource' do - describe '.find_by_full_path', :aggregate_failures do + shared_examples_for '.find_by_full_path' do it 'finds records by their full path' do expect(described_class.find_by_full_path(record.full_path)).to eq(record) expect(described_class.find_by_full_path(record.full_path.upcase)).to eq(record) end - it 'returns nil for unknown paths' do - expect(described_class.find_by_full_path('unknown')).to be_nil - end + it 'checks if `optimize_routable` is enabled only once' do + expect(Routable).to receive(:optimize_routable_enabled?).once - 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 + described_class.find_by_full_path(record.full_path) + end - expect do - described_class.find_by_full_path(record.full_path).route - end.not_to exceed_all_query_limit(control_count) + it 'returns nil for unknown paths' do + expect(described_class.find_by_full_path('unknown')).to be_nil end context 'when path is a negative number' do @@ -56,6 +52,26 @@ RSpec.shared_examples 'routable resource' do 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 + + 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 + + expect do + described_class.find_by_full_path(record.full_path).route + end.not_to exceed_all_query_limit(control_count) + end + end end RSpec.shared_examples 'routable resource with parent' do @@ -93,7 +109,7 @@ RSpec.shared_examples 'routable resource with parent' do end end -RSpec.describe Group, 'Routable', :with_clean_rails_cache do +RSpec.describe Group, 'Routable', :with_clean_rails_cache, feature_category: :groups_and_projects do let_it_be_with_reload(:group) { create(:group, name: 'foo') } let_it_be(:nested_group) { create(:group, parent: group) } @@ -223,7 +239,7 @@ RSpec.describe Group, 'Routable', :with_clean_rails_cache do end end -RSpec.describe Project, 'Routable', :with_clean_rails_cache do +RSpec.describe Project, 'Routable', :with_clean_rails_cache, feature_category: :groups_and_projects do let_it_be(:namespace) { create(:namespace) } let_it_be(:project) { create(:project, namespace: namespace) } @@ -235,9 +251,20 @@ RSpec.describe Project, 'Routable', :with_clean_rails_cache do expect(project.route).not_to be_nil expect(project.route.namespace).to eq(project.project_namespace) end + + describe '.find_by_full_path' do + it 'does not return a record if the sources are different, but the IDs match' do + group = create(:group, id: 1992) + project = create(:project, id: 1992) + + record = described_class.where(id: project.id).find_by_full_path(group.full_path) + + expect(record).to be_nil + end + end end -RSpec.describe Namespaces::ProjectNamespace, 'Routable', :with_clean_rails_cache do +RSpec.describe Namespaces::ProjectNamespace, 'Routable', :with_clean_rails_cache, feature_category: :groups_and_projects do let_it_be(:group) { create(:group) } it 'skips route creation for the resource' do @@ -247,6 +274,22 @@ 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/concerns/transitionable_spec.rb b/spec/models/concerns/transitionable_spec.rb new file mode 100644 index 00000000000..b80d363ef78 --- /dev/null +++ b/spec/models/concerns/transitionable_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Transitionable, feature_category: :code_review_workflow do + using RSpec::Parameterized::TableSyntax + + let(:klass) do + Class.new do + include Transitionable + + def initialize(transitioning) + @transitioning = transitioning + end + + def project + Project.new + end + end + end + + let(:object) { klass.new(transitioning) } + + describe '#transitioning?' do + where(:transitioning, :feature_flag, :result) do + true | true | true + false | false | false + true | false | false + false | true | false + end + + with_them do + before do + stub_feature_flags(skip_validations_during_transitions: feature_flag) + end + + it { expect(object.transitioning?).to eq(result) } + end + end +end diff --git a/spec/models/deploy_key_spec.rb b/spec/models/deploy_key_spec.rb index 528b36babc6..2959556f5ae 100644 --- a/spec/models/deploy_key_spec.rb +++ b/spec/models/deploy_key_spec.rb @@ -67,7 +67,7 @@ RSpec.describe DeployKey, :mailer do context 'when user is not set' do it 'returns the ghost user' do - expect(deploy_key.user).to eq(User.ghost) + expect(deploy_key.user).to eq(Users::Internal.ghost) end end end diff --git a/spec/models/design_management/design_spec.rb b/spec/models/design_management/design_spec.rb index 72c0d1d1a64..98e5399f737 100644 --- a/spec/models/design_management/design_spec.rb +++ b/spec/models/design_management/design_spec.rb @@ -467,16 +467,6 @@ RSpec.describe DesignManagement::Design, feature_category: :design_management do end end - describe '#note_etag_key' do - it 'returns a correct etag key' do - design = design1 - - expect(design.note_etag_key).to eq( - ::Gitlab::Routing.url_helpers.designs_project_issue_path(design.project, design.issue, { vueroute: design.filename }) - ) - end - end - describe '#user_notes_count', :use_clean_rails_memory_store_caching do # Note: Cache invalidation tests are in `design_user_notes_count_service_spec.rb` it 'returns a count of user-generated notes' do diff --git a/spec/models/doorkeeper/application_spec.rb b/spec/models/doorkeeper/application_spec.rb new file mode 100644 index 00000000000..85b28346dfa --- /dev/null +++ b/spec/models/doorkeeper/application_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Doorkeeper::Application, type: :model, feature_category: :system_access do + let(:application) { create(:oauth_application) } + + it 'uses a prefixed secret' do + expect(application.plaintext_secret).to match(/gloas-\h{64}/) + end +end diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index 9814eed8b45..eeb8583251d 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -270,18 +270,6 @@ RSpec.describe EnvironmentStatus do context 'when environment is stopped' do before do - stub_feature_flags(review_apps_redeploy_mr_widget: false) - environment.stop! - end - - it 'does not return environment status' do - expect(subject.count).to eq(0) - end - end - - context 'when environment is stopped and review_apps_redeploy_mr_widget is turned on' do - before do - stub_feature_flags(review_apps_redeploy_mr_widget: true) environment.stop! end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 23e72f6663a..3f671fc3f70 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2666,14 +2666,16 @@ RSpec.describe Group, feature_category: :groups_and_projects do let(:group) { build(:group) } context 'the group has owners' do - before do - group.add_owner(create(:user)) - group.add_owner(create(:user)) - end - it 'is the first owner' do + user_1 = create(:user) + user_2 = create(:user) + group.add_owner(user_2) + group.add_owner(user_1) + + # The senior-most user (not member) who is an OWNER in the group + # is always treated as the first owner expect(group.first_owner) - .to eq(group.owners.first) + .to eq(user_1) .and be_a(User) end end @@ -3307,6 +3309,13 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end + describe '#supports_lock_on_merge?' do + it_behaves_like 'checks self and root ancestor feature flag' do + let(:feature_flag) { :enforce_locked_labels_on_merge } + let(:feature_flag_method) { :supports_lock_on_merge? } + end + end + describe 'group shares' do let!(:sub_group) { create(:group, parent: group) } let!(:sub_sub_group) { create(:group, parent: sub_group) } diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb index 4b88b3b3e65..e9a2635bf28 100644 --- a/spec/models/hooks/web_hook_log_spec.rb +++ b/spec/models/hooks/web_hook_log_spec.rb @@ -46,6 +46,20 @@ RSpec.describe WebHookLog, feature_category: :webhooks do end end + context 'with basic auth credentials and masked components' do + let(:web_hook_log) { build(:web_hook_log, web_hook: hook, url: 'http://test:123@{domain}.com:{port}') } + + subject { web_hook_log.save! } + + it { is_expected.to eq(true) } + + it 'obfuscates the basic auth credentials' do + subject + + expect(web_hook_log.url).to eq('http://*****:*****@{domain}.com:{port}') + end + end + context "with users' emails" do let(:author) { build(:user) } let(:user) { build(:user) } @@ -235,4 +249,28 @@ RSpec.describe WebHookLog, feature_category: :webhooks do it { expect(web_hook_log.request_headers).to eq(expected_headers) } end end + + describe '#url_current?' do + let(:url) { 'example@gitlab.com' } + + let(:hook) { build(:project_hook, url: url) } + let(:web_hook_log) do + build( + :web_hook_log, + web_hook: hook, + interpolated_url: hook.url, + url_hash: Gitlab::CryptoHelper.sha256('example@gitlab.com') + ) + end + + context 'with matching url' do + it { expect(web_hook_log.url_current?).to be_truthy } + end + + context 'with different url' do + let(:url) { 'example@gitlab2.com' } + + it { expect(web_hook_log.url_current?).to be_falsey } + end + end end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 0b41b46ae3d..67e12092e1a 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -1041,9 +1041,9 @@ RSpec.describe Integration, feature_category: :integrations do it 'returns all fields with type `password`' do allow(subject).to receive(:fields).and_return( [ - { name: 'password', type: :password }, - { name: 'secret', type: :password }, - { name: 'public', type: :text } + Integrations::Field.new(name: 'password', integration_class: subject.class, type: :password), + Integrations::Field.new(name: 'secret', integration_class: subject.class, type: :password), + Integrations::Field.new(name: 'public', integration_class: subject.class, type: :text) ]) expect(subject.secret_fields).to match_array(%w[password secret]) diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index 675035095c5..497f2f1e7c9 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -85,7 +85,7 @@ RSpec.describe Integrations::BaseChatNotification, feature_category: :integratio context 'when webhook is not required' do it 'returns true' do - allow(chat_integration).to receive(:requires_webhook?).and_return(false) + allow(chat_integration.class).to receive(:requires_webhook?).and_return(false) expect(chat_integration).to receive(:notify).and_return(true) expect(chat_integration.execute(data)).to be true @@ -347,6 +347,12 @@ RSpec.describe Integrations::BaseChatNotification, feature_category: :integratio end end + describe '#help' do + it 'raises an error' do + expect { subject.help }.to raise_error(NotImplementedError) + end + end + describe '#event_channel_name' do it 'returns the channel field name for the given event' do expect(subject.event_channel_name(:event)).to eq('event_channel') @@ -364,4 +370,17 @@ RSpec.describe Integrations::BaseChatNotification, feature_category: :integratio expect { subject.event_channel_value(:foo) }.to raise_error(NoMethodError) end end + + describe '#api_field_names' do + context 'when channels are masked' do + let(:project) { build(:project) } + let(:integration) { build(:discord_integration, project: project, webhook: 'https://discord.com/api/') } + + it 'does not include channel properties', :aggregate_failures do + integration.event_channel_names.each do |field| + expect(integration.api_field_names).not_to include(field) + end + end + 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 d16c191bd08..630ae902331 100644 --- a/spec/models/integrations/chat_message/deployment_message_spec.rb +++ b/spec/models/integrations/chat_message/deployment_message_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::ChatMessage::DeploymentMessage do +RSpec.describe Integrations::ChatMessage::DeploymentMessage, feature_category: :integrations do subject { described_class.new(args) } let_it_be(:user) { create(:user, name: 'John Smith', username: 'smith') } @@ -103,15 +103,33 @@ RSpec.describe Integrations::ChatMessage::DeploymentMessage do }.merge(params) end - 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) - commit_url = Gitlab::UrlBuilder.build(deployment.commit) - user_url = Gitlab::Routing.url_helpers.user_url(user) + 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) + commit_url = Gitlab::UrlBuilder.build(deployment.commit) + user_url = Gitlab::Routing.url_helpers.user_url(user) + + expect(subject.attachments).to eq([{ + text: "<#{project.web_url}|myspace/myproject> with job <#{job_url}|##{ci_build.id}> by <#{user_url}|John Smith (smith)>\n<#{commit_url}|#{deployment.short_sha}>: #{commit.title}", + color: "good" + }]) + end + end - expect(subject.attachments).to eq([{ - text: "[myspace/myproject](#{project.web_url}) with job [##{ci_build.id}](#{job_url}) by [John Smith (smith)](#{user_url})\n[#{deployment.short_sha}](#{commit_url}): #{commit.title}", - color: "good" - }]) + context 'with markdown' do + before do + args.merge!(markdown: true) + end + + 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) + commit_url = Gitlab::UrlBuilder.build(deployment.commit) + user_url = Gitlab::Routing.url_helpers.user_url(user) + + expect(subject.attachments).to eq( + "[myspace/myproject](#{project.web_url}) with job [##{ci_build.id}](#{job_url}) by [John Smith (smith)](#{user_url})\n[#{deployment.short_sha}](#{commit_url}): #{commit.title}" + ) + end end it 'returns attachments for a failed deployment' do diff --git a/spec/models/integrations/confluence_spec.rb b/spec/models/integrations/confluence_spec.rb index d267e4a71c2..a34b55c3c6b 100644 --- a/spec/models/integrations/confluence_spec.rb +++ b/spec/models/integrations/confluence_spec.rb @@ -63,6 +63,12 @@ RSpec.describe Integrations::Confluence, feature_category: :integrations do end end + describe '#avatar_url' do + it 'returns the avatar image path' do + expect(subject.avatar_url).to eq(ActionController::Base.helpers.image_path('confluence.svg')) + end + end + describe 'Caching has_confluence on project_settings' do subject { project.project_setting.has_confluence? } diff --git a/spec/models/integrations/mattermost_spec.rb b/spec/models/integrations/mattermost_spec.rb index f7702846b6c..224bc4acdda 100644 --- a/spec/models/integrations/mattermost_spec.rb +++ b/spec/models/integrations/mattermost_spec.rb @@ -2,6 +2,6 @@ require 'spec_helper' -RSpec.describe Integrations::Mattermost do +RSpec.describe Integrations::Mattermost, feature_category: :integrations do it_behaves_like Integrations::SlackMattermostNotifier, "Mattermost" end diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index da43d851b31..4a998efe665 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -422,6 +422,34 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, end end + describe '#sync_http_integration after_save callback' do + context 'with corresponding HTTP integration' do + let_it_be_with_reload(:http_integration) { create(:alert_management_prometheus_integration, :legacy, project: project) } + + it 'syncs the attribute' do + expect { integration.update!(manual_configuration: false) } + .to change { http_integration.reload.active } + .from(true).to(false) + end + + context 'when changing a different attribute' do + it 'does not sync the attribute or execute extra queries' do + expect { integration.update!(api_url: 'https://any.url') } + .to issue_fewer_queries_than { integration.update!(manual_configuration: false) } + end + end + end + + context 'without corresponding HTTP integration' do + let_it_be(:other_http_integration) { create(:alert_management_prometheus_integration, project: project) } + + it 'does not sync the attribute or execute extra queries' do + expect { integration.update!(manual_configuration: false) } + .not_to change { other_http_integration.reload.active } + end + end + end + describe '#editable?' do it 'is editable' do expect(integration.editable?).to be(true) diff --git a/spec/models/integrations/shimo_spec.rb b/spec/models/integrations/shimo_spec.rb index be626012ab2..95289343d0d 100644 --- a/spec/models/integrations/shimo_spec.rb +++ b/spec/models/integrations/shimo_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ::Integrations::Shimo do +RSpec.describe ::Integrations::Shimo, feature_category: :integrations do describe '#fields' do let(:shimo_integration) { build(:shimo_integration) } @@ -60,4 +60,10 @@ RSpec.describe ::Integrations::Shimo do expect { create(:shimo_integration) }.to change(ProjectSetting, :count).by(1) end end + + describe '#avatar_url' do + it 'returns the avatar image path' do + expect(subject.avatar_url).to eq(ActionController::Base.helpers.image_path('logos/shimo.svg')) + end + end end diff --git a/spec/models/integrations/slack_spec.rb b/spec/models/integrations/slack_spec.rb index 218d92ffe05..59ee3746d8f 100644 --- a/spec/models/integrations/slack_spec.rb +++ b/spec/models/integrations/slack_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::Slack do +RSpec.describe Integrations::Slack, feature_category: :integrations do it_behaves_like Integrations::SlackMattermostNotifier, 'Slack' it_behaves_like Integrations::BaseSlackNotification, factory: :integrations_slack do before do diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb index 2fa4df0e900..460ce7629cc 100644 --- a/spec/models/integrations/zentao_spec.rb +++ b/spec/models/integrations/zentao_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::Zentao do +RSpec.describe Integrations::Zentao, feature_category: :integrations do let(:url) { 'https://jihudemo.zentao.net' } let(:api_url) { 'https://jihudemo.zentao.net' } let(:api_token) { 'ZENTAO_TOKEN' } @@ -80,6 +80,12 @@ RSpec.describe Integrations::Zentao do end end + describe '#avatar_url' do + it 'returns the avatar image path' do + expect(subject.avatar_url).to eq(ActionController::Base.helpers.image_path('logos/zentao.svg')) + end + end + describe '#client_url' do subject(:integration) { build(:zentao_integration, api_url: api_url, url: 'url').client_url } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 9db710cb3cc..4e217e3a9f7 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -28,7 +28,6 @@ RSpec.describe Issue, feature_category: :team_planning do it { is_expected.to have_many(:resource_milestone_events) } it { is_expected.to have_many(:resource_state_events) } it { is_expected.to have_and_belong_to_many(:prometheus_alert_events) } - it { is_expected.to have_and_belong_to_many(:self_managed_prometheus_alert_events) } it { is_expected.to have_many(:prometheus_alerts) } it { is_expected.to have_many(:issue_email_participants) } it { is_expected.to have_one(:email) } @@ -953,7 +952,7 @@ RSpec.describe Issue, feature_category: :team_planning do subject { issue.from_service_desk? } context 'when issue author is support bot' do - let(:issue) { create(:issue, project: reusable_project, author: ::User.support_bot) } + let(:issue) { create(:issue, project: reusable_project, author: ::Users::Internal.support_bot) } it { is_expected.to be_truthy } end @@ -1527,7 +1526,7 @@ RSpec.describe Issue, feature_category: :team_planning do end describe '#check_for_spam?' do - let_it_be(:support_bot) { ::User.support_bot } + let_it_be(:support_bot) { ::Users::Internal.support_bot } where(:support_bot?, :visibility_level, :confidential, :new_attributes, :check_for_spam?) do ### non-support-bot cases @@ -1640,7 +1639,7 @@ RSpec.describe Issue, feature_category: :team_planning do describe '.service_desk' do it 'returns the service desk issue' do - service_desk_issue = create(:issue, project: reusable_project, author: ::User.support_bot) + service_desk_issue = create(:issue, project: reusable_project, author: ::Users::Internal.support_bot) regular_issue = create(:issue, project: reusable_project) expect(described_class.service_desk).to include(service_desk_issue) diff --git a/spec/models/loose_foreign_keys/modification_tracker_spec.rb b/spec/models/loose_foreign_keys/modification_tracker_spec.rb index 069ccf85141..afc62f28f92 100644 --- a/spec/models/loose_foreign_keys/modification_tracker_spec.rb +++ b/spec/models/loose_foreign_keys/modification_tracker_spec.rb @@ -2,12 +2,12 @@ require 'spec_helper' -RSpec.describe LooseForeignKeys::ModificationTracker do +RSpec.describe LooseForeignKeys::ModificationTracker, feature_category: :database do subject(:tracker) { described_class.new } describe '#over_limit?' do - it 'is true when deletion MAX_DELETES is exceeded' do - stub_const('LooseForeignKeys::ModificationTracker::MAX_DELETES', 5) + it 'is true when deletion max_deletes is exceeded' do + expect(tracker).to receive(:max_deletes).and_return(5) tracker.add_deletions('issues', 10) expect(tracker).to be_over_limit @@ -20,7 +20,7 @@ RSpec.describe LooseForeignKeys::ModificationTracker do end it 'is true when deletion MAX_UPDATES is exceeded' do - stub_const('LooseForeignKeys::ModificationTracker::MAX_UPDATES', 5) + expect(tracker).to receive(:max_updates).and_return(5) tracker.add_updates('issues', 3) tracker.add_updates('issues', 4) @@ -36,9 +36,11 @@ RSpec.describe LooseForeignKeys::ModificationTracker do it 'is true when max runtime is exceeded' do monotonic_time_before = 1 # this will be the start time - monotonic_time_after = described_class::MAX_RUNTIME.to_i + 1 # this will be returned when over_limit? is called + monotonic_time_after = 31 # this will be returned when over_limit? is called - allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after) + expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_return( + monotonic_time_before, monotonic_time_after + ) tracker diff --git a/spec/models/loose_foreign_keys/turbo_modification_tracker_spec.rb b/spec/models/loose_foreign_keys/turbo_modification_tracker_spec.rb new file mode 100644 index 00000000000..0916a0845f5 --- /dev/null +++ b/spec/models/loose_foreign_keys/turbo_modification_tracker_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe LooseForeignKeys::TurboModificationTracker, feature_category: :database do + subject(:tracker) { described_class.new } + + let(:normal_tracker) { LooseForeignKeys::ModificationTracker.new } + + context 'with limits should be higher than LooseForeignKeys::ModificationTracker' do + it 'expect max_deletes to be equal or higher' do + expect(tracker.max_deletes).to be >= normal_tracker.max_deletes + end + + it 'expect max_updates to be equal or higher' do + expect(tracker.max_updates).to be >= normal_tracker.max_updates + end + + it 'expect max_runtime to be equal or higher' do + expect(tracker.max_runtime).to be >= normal_tracker.max_runtime + end + end +end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index f8aaae3edad..6dd5f9dec8c 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -1177,4 +1177,11 @@ RSpec.describe Member, feature_category: :groups_and_projects do expect(described_class.sort_by_attribute('oldest_last_activity')).to eq([member3, member2, member1]) end end + + context 'with loose foreign key on members.user_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:user) } + let!(:model) { create(:group_member, user: parent) } + end + end end diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index a07829abece..7307e76272d 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -69,7 +69,7 @@ RSpec.describe GroupMember, feature_category: :cell do describe '#update_two_factor_requirement' do it 'is called after creation and deletion' do - user = build :user + user = create :user group = create :group group_member = build :group_member, user: user, group: group @@ -288,4 +288,18 @@ RSpec.describe GroupMember, feature_category: :cell do it_behaves_like 'calls AuthorizedProjectsWorker inline to recalculate authorizations' end end + + context 'group member welcome email', :sidekiq_inline, :saas do + let_it_be(:group) { create(:group) } + + let(:user) { create(:user) } + + it 'schedules plain welcome to the group email' do + expect_next_instance_of(NotificationService) do |notification| + expect(notification).to receive(:new_group_member) + end + + group.add_developer(user) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index da3f691b63a..b36737fc19d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -135,10 +135,22 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev let_it_be(:user1) { create(:user) } let_it_be(:user2) { create(:user) } - let_it_be(:merge_request1) { create(:merge_request, :unique_branches, reviewers: [user1]) } - let_it_be(:merge_request2) { create(:merge_request, :unique_branches, reviewers: [user2]) } - let_it_be(:merge_request3) { create(:merge_request, :unique_branches, reviewers: []) } - let_it_be(:merge_request4) { create(:merge_request, :draft_merge_request) } + let_it_be(:merge_request1) do + create(:merge_request, :prepared, :unique_branches, reviewers: [user1], created_at: + 2.days.ago) + end + + let_it_be(:merge_request2) do + create(:merge_request, :unprepared, :unique_branches, reviewers: [user2], created_at: + 3.hours.ago) + end + + let_it_be(:merge_request3) do + create(:merge_request, :unprepared, :unique_branches, reviewers: [], created_at: + Time.current) + end + + let_it_be(:merge_request4) { create(:merge_request, :prepared, :draft_merge_request) } describe '.preload_target_project_with_namespace' do subject(:mr) { described_class.preload_target_project_with_namespace.first } @@ -180,6 +192,14 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end + describe '.recently_unprepared' do + it 'only returns the recently unprepared mrs' do + merge_request5 = create(:merge_request, :unprepared, :unique_branches, created_at: merge_request3.created_at) + + expect(described_class.recently_unprepared).to eq([merge_request3, merge_request5]) + end + end + describe '.by_sorted_source_branches' do let(:fork_for_project) { fork_project(project) } @@ -340,6 +360,23 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end + describe "#validate_reviewer_size_length" do + let(:merge_request) { build(:merge_request, transitioning: transitioning) } + + where(:transitioning, :to_or_not_to) do + false | :to + true | :not_to + end + + with_them do + it do + expect(merge_request).send(to_or_not_to, receive(:validate_reviewer_size_length)) + + merge_request.valid? + end + end + end + describe '#validate_target_project' do let(:merge_request) do build(:merge_request, source_project: project, target_project: project, importing: importing) @@ -366,6 +403,23 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev it { expect(merge_request.valid?(false)).to eq true } end end + + context "with the skip_validations_during_transition_feature_flag" do + let(:merge_request) { build(:merge_request, transitioning: transitioning) } + + where(:transitioning, :to_or_not_to) do + false | :to + true | :not_to + end + + with_them do + it do + expect(merge_request).send(to_or_not_to, receive(:validate_target_project)) + + merge_request.valid? + end + end + end end end @@ -2099,6 +2153,16 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev subject.mark_as_merged! end + context 'and merged_commit_sha is present' do + before do + subject.update_attribute(:merged_commit_sha, pipeline.sha) + end + + it 'returns the pipeline associated with that merge request' do + expect(subject.merge_pipeline).to eq(pipeline) + end + end + context 'and there is a merge commit' do before do subject.update_attribute(:merge_commit_sha, pipeline.sha) @@ -2850,6 +2914,12 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev subject.mark_as_merged! end + it 'returns merged_commit_sha when there is a merged_commit_sha' do + subject.update_attribute(:merged_commit_sha, sha) + + expect(subject.merged_commit_sha).to eq(sha) + end + it 'returns merge_commit_sha when there is a merge_commit_sha' do subject.update_attribute(:merge_commit_sha, sha) @@ -3275,6 +3345,31 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev subject.mergeable?(check_mergeability_retry_lease: true) end end + + context 'with skip_rebase_check option' do + before do + allow(subject).to receive_messages( + mergeable_state?: true, + check_mergeability: nil, + can_be_merged?: true + ) + end + + where(:should_be_rebased, :skip_rebase_check, :expected_mergeable) do + false | false | true + false | true | true + true | false | false + true | true | true + end + + with_them do + it 'overrides should_be_rebased?' do + allow(subject).to receive(:should_be_rebased?) { should_be_rebased } + + expect(subject.mergeable?(skip_rebase_check: skip_rebase_check)).to eq(expected_mergeable) + end + end + end end describe '#skipped_mergeable_checks' do @@ -4442,6 +4537,7 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev shared_examples 'for an invalid state transition' do specify 'is not a valid state transition' do expect { transition! }.to raise_error(StateMachines::InvalidTransition) + expect(subject.transitioning?).to be_falsey end end @@ -4451,6 +4547,7 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev .to change { subject.merge_status } .from(merge_status.to_s) .to(expected_merge_status) + expect(subject.transitioning?).to be_falsey end end @@ -5493,7 +5590,8 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev let(:ref) { subject.target_project.repository.commit.id } before do - expect(subject.target_project).to receive(:mark_primary_write_location) + expect(subject.target_project.sticking).to receive(:stick) + .with(:project, subject.target_project.id) end it 'updates commit ID' do @@ -5806,4 +5904,56 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev it { is_expected.to eq(false) } end end + + describe '#supports_lock_on_merge?' do + let(:merge_request) { build_stubbed(:merge_request) } + + subject { merge_request.supports_lock_on_merge? } + + context 'when MR is open' do + it { is_expected.to eq(false) } + end + + context 'when MR is merged' do + before do + merge_request.state = :merged + end + + it { is_expected.to eq(true) } + + context 'when feature flag is disabled' do + before do + stub_feature_flags(enforce_locked_labels_on_merge: false) + end + + it { is_expected.to eq(false) } + end + end + end + + describe '#missing_required_squash?' do + using RSpec::Parameterized::TableSyntax + + where(:squash, :project_requires_squash, :expected) do + false | true | true + false | false | false + true | true | false + true | false | false + end + + with_them do + let(:merge_request) { build_stubbed(:merge_request, squash: squash) } + + subject { merge_request.missing_required_squash? } + + before do + allow(merge_request.target_project).to( + receive(:squash_always?) + .and_return(project_requires_squash) + ) + end + + it { is_expected.to eq(expected) } + end + end end diff --git a/spec/models/metrics/dashboard/annotation_spec.rb b/spec/models/metrics/dashboard/annotation_spec.rb deleted file mode 100644 index 7c4f392fcdc..00000000000 --- a/spec/models/metrics/dashboard/annotation_spec.rb +++ /dev/null @@ -1,73 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Metrics::Dashboard::Annotation do - using RSpec::Parameterized::TableSyntax - - describe 'validation' do - it { is_expected.to validate_presence_of(:description) } - it { is_expected.to validate_presence_of(:dashboard_path) } - it { is_expected.to validate_presence_of(:starting_at) } - it { is_expected.to validate_length_of(:dashboard_path).is_at_most(255) } - it { is_expected.to validate_length_of(:panel_xid).is_at_most(255) } - it { is_expected.to validate_length_of(:description).is_at_most(255) } - - context 'ending_at_after_starting_at' do - where(:starting_at, :ending_at, :valid?, :message) do - 2.days.ago.beginning_of_day | 1.day.ago.beginning_of_day | true | nil - 1.day.ago.beginning_of_day | nil | true | nil - 1.day.ago.beginning_of_day | 1.day.ago.beginning_of_day | true | nil - 1.day.ago.beginning_of_day | 2.days.ago.beginning_of_day | false | /Ending at can't be before starting_at time/ - nil | 2.days.ago.beginning_of_day | false | /Starting at can't be blank/ # validation is covered by other method, be we need to assure, that ending_at_after_starting_at will not break with nil as starting_at - nil | nil | false | /Starting at can't be blank/ # validation is covered by other method, be we need to assure, that ending_at_after_starting_at will not break with nil as starting_at - end - - with_them do - subject(:annotation) { build(:metrics_dashboard_annotation, starting_at: starting_at, ending_at: ending_at) } - - it do - expect(annotation.valid?).to be(valid?) - expect(annotation.errors.full_messages).to include(message) if message - end - end - end - end - - describe 'scopes' do - let_it_be(:nine_minutes_old_annotation) { create(:metrics_dashboard_annotation, starting_at: 9.minutes.ago) } - let_it_be(:fifteen_minutes_old_annotation) { create(:metrics_dashboard_annotation, starting_at: 15.minutes.ago) } - let_it_be(:just_created_annotation) { create(:metrics_dashboard_annotation) } - - describe '#after' do - it 'returns only younger annotations' do - expect(described_class.after(12.minutes.ago)).to match_array [nine_minutes_old_annotation, just_created_annotation] - end - end - - describe '#before' do - it 'returns only older annotations' do - expect(described_class.before(5.minutes.ago)).to match_array [fifteen_minutes_old_annotation, nine_minutes_old_annotation] - end - end - - describe '#for_dashboard' do - let!(:other_dashboard_annotation) { create(:metrics_dashboard_annotation, dashboard_path: 'other_dashboard.yml') } - - it 'returns annotations only for appointed dashboard' do - expect(described_class.for_dashboard('other_dashboard.yml')).to match_array [other_dashboard_annotation] - end - end - - describe '#ending_before' do - it 'returns annotations only for appointed dashboard' do - freeze_time do - twelve_minutes_old_annotation = create(:metrics_dashboard_annotation, starting_at: 15.minutes.ago, ending_at: 12.minutes.ago) - create(:metrics_dashboard_annotation, starting_at: 15.minutes.ago, ending_at: 11.minutes.ago) - - expect(described_class.ending_before(11.minutes.ago)).to match_array [fifteen_minutes_old_annotation, twelve_minutes_old_annotation] - end - end - end - end -end diff --git a/spec/models/metrics/users_starred_dashboard_spec.rb b/spec/models/metrics/users_starred_dashboard_spec.rb deleted file mode 100644 index c89344c0a1c..00000000000 --- a/spec/models/metrics/users_starred_dashboard_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Metrics::UsersStarredDashboard do - describe 'associations' do - it { is_expected.to belong_to(:project).inverse_of(:metrics_users_starred_dashboards) } - it { is_expected.to belong_to(:user).inverse_of(:metrics_users_starred_dashboards) } - end - - describe 'validation' do - subject { build(:metrics_users_starred_dashboard) } - - it { is_expected.to validate_presence_of(:user_id) } - it { is_expected.to validate_presence_of(:project_id) } - it { is_expected.to validate_presence_of(:dashboard_path) } - it { is_expected.to validate_length_of(:dashboard_path).is_at_most(255) } - it { is_expected.to validate_uniqueness_of(:dashboard_path).scoped_to(%i[user_id project_id]) } - end - - context 'scopes' do - let_it_be(:project) { create(:project) } - let_it_be(:starred_dashboard_a) { create(:metrics_users_starred_dashboard, project: project, dashboard_path: 'path_a') } - let_it_be(:starred_dashboard_b) { create(:metrics_users_starred_dashboard, project: project, dashboard_path: 'path_b') } - let_it_be(:starred_dashboard_c) { create(:metrics_users_starred_dashboard, dashboard_path: 'path_b') } - - describe '#for_project' do - it 'selects only starred dashboards belonging to project' do - expect(described_class.for_project(project)).to contain_exactly starred_dashboard_a, starred_dashboard_b - end - end - - describe '#for_project_dashboard' do - it 'selects only starred dashboards belonging to project with given dashboard path' do - expect(described_class.for_project_dashboard(project, 'path_b')).to contain_exactly starred_dashboard_b - end - end - end -end diff --git a/spec/models/ml/model_version_spec.rb b/spec/models/ml/model_version_spec.rb index 4bb272fef5d..83639fca9e1 100644 --- a/spec/models/ml/model_version_spec.rb +++ b/spec/models/ml/model_version_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:model) } - it { is_expected.to belong_to(:package) } + it { is_expected.to belong_to(:package).class_name('Packages::MlModel::Package') } end describe 'validation' do @@ -83,14 +83,6 @@ RSpec.describe Ml::ModelVersion, feature_category: :mlops do it { expect(errors[:package]).to include(error_message) } end - - context 'when package is not ml_model' do - let(:package) do - build_stubbed(:generic_package, project: base_project, name: model1.name, version: valid_version) - end - - it { expect(errors[:package]).to include('package must be ml_model') } - end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 623c9c7e07c..a0deee0f2d3 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -443,6 +443,15 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end + describe '.by_root_id' do + it 'returns correct namespaces' do + expect(described_class.by_root_id(namespace1.id)).to match_array([namespace1, namespace1sub]) + expect(described_class.by_root_id(namespace2.id)).to match_array([namespace2, namespace2sub]) + expect(described_class.by_root_id(namespace1sub.id)).to be_empty + expect(described_class.by_root_id(nil)).to be_empty + end + end + describe '.filter_by_path' do it 'includes correct namespaces' do expect(described_class.filter_by_path(namespace1.path)).to eq([namespace1]) @@ -1070,17 +1079,30 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end it 'defaults use_minimum_char_limit to true' do - expect(described_class).to receive(:fuzzy_search).with(anything, anything, use_minimum_char_limit: true).once + expect(described_class).to receive(:fuzzy_search).with(anything, anything, use_minimum_char_limit: true, exact_matches_first: false).once described_class.search('my namespace') end it 'passes use_minimum_char_limit if it is set' do - expect(described_class).to receive(:fuzzy_search).with(anything, anything, use_minimum_char_limit: false).once + expect(described_class).to receive(:fuzzy_search).with(anything, anything, use_minimum_char_limit: false, exact_matches_first: false).once described_class.search('my namespace', use_minimum_char_limit: false) end + context 'with multiple matching namespaces' do + let_it_be(:first_group) { create(:group, name: 'some name', path: 'z-path') } + let_it_be(:second_group) { create(:group, name: 'some name too', path: 'a-path') } + + it 'returns exact matches first' do + expect(described_class.search('some name', exact_matches_first: true).to_a).to eq([first_group, second_group]) + end + + it 'returns exact matches first when parents are included' do + expect(described_class.search('some name', include_parents: true, exact_matches_first: true).to_a).to eq([first_group, second_group]) + end + end + context 'with project namespaces' do let_it_be(:project) { create(:project, namespace: parent_group, path: 'some-new-path') } let_it_be(:project_namespace) { project.project_namespace } @@ -1186,8 +1208,11 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end describe '#move_dir', :request_store do - shared_examples "namespace restrictions" do - context "when any project has container images" 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 @@ -1207,190 +1232,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do ) end end - end - - context 'legacy storage' do - let(:namespace) { create(:namespace) } - let!(:project) { create(:project_empty_repo, :legacy_storage, namespace: namespace) } - - it_behaves_like 'namespace restrictions' - - it "raises error when directory exists" do - expect { namespace.move_dir }.to raise_error("namespace directory cannot be moved") - end - - it "moves dir if path changed" do - namespace.update!(path: namespace.full_path + '_new') - - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy - end - - context 'when #write_projects_repository_config raises an error' do - context 'in test environment' do - it 'raises an exception' do - expect(namespace).to receive(:write_projects_repository_config).and_raise('foo') - - expect do - namespace.update!(path: namespace.full_path + '_new') - end.to raise_error('foo') - end - end - - context 'in production environment' do - it 'does not cancel later callbacks' do - expect(namespace).to receive(:write_projects_repository_config).and_raise('foo') - expect(namespace).to receive(:move_dir).and_wrap_original do |m, *args| - move_dir_result = m.call(*args) - - expect(move_dir_result).to be_truthy # Must be truthy, or else later callbacks would be canceled - - move_dir_result - end - expect(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(false) # like prod - - namespace.update!(path: namespace.full_path + '_new') - end - end - end - - shared_examples 'move_dir without repository storage feature' do |storage_version| - let(:namespace) { create(:namespace) } - let(:gitlab_shell) { namespace.gitlab_shell } - let!(:project) { create(:project_empty_repo, namespace: namespace, storage_version: storage_version) } - - it 'calls namespace service' do - expect(gitlab_shell).to receive(:add_namespace).and_return(true) - expect(gitlab_shell).to receive(:mv_namespace).and_return(true) - - namespace.move_dir - end - end - - shared_examples 'move_dir with repository storage feature' do |storage_version| - let(:namespace) { create(:namespace) } - let(:gitlab_shell) { namespace.gitlab_shell } - let!(:project) { create(:project_empty_repo, namespace: namespace, storage_version: storage_version) } - - it 'does not call namespace service' do - expect(gitlab_shell).not_to receive(:add_namespace) - expect(gitlab_shell).not_to receive(:mv_namespace) - - namespace.move_dir - end - end - - context 'project is without repository storage feature' do - [nil, 0].each do |storage_version| - it_behaves_like 'move_dir without repository storage feature', storage_version - end - end - - context 'project has repository storage feature' do - [1, 2].each do |storage_version| - it_behaves_like 'move_dir with repository storage feature', storage_version - end - end - - context 'with subgroups' do - let(:parent) { create(:group, name: 'parent', path: 'parent') } - let(:new_parent) { create(:group, name: 'new_parent', path: 'new_parent') } - let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } - let!(:project) { create(:project_empty_repo, :legacy_storage, path: 'the-project', namespace: child, skip_disk_validation: true) } - let(:uploads_dir) { FileUploader.root } - let(:pages_dir) { File.join(TestEnv.pages_path) } - - def expect_project_directories_at(namespace_path, with_pages: true) - expected_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - File.join(TestEnv.repos_path, namespace_path, 'the-project.git') - end - expected_upload_path = File.join(uploads_dir, namespace_path, 'the-project') - expected_pages_path = File.join(pages_dir, namespace_path, 'the-project') - - expect(File.directory?(expected_repository_path)).to be_truthy - expect(File.directory?(expected_upload_path)).to be_truthy - expect(File.directory?(expected_pages_path)).to be(with_pages) - end - - before do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project.full_path}.git")) - end - FileUtils.mkdir_p(File.join(uploads_dir, project.full_path)) - FileUtils.mkdir_p(File.join(pages_dir, project.full_path)) - end - - after do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - FileUtils.remove_entry(File.join(TestEnv.repos_path, parent.full_path), true) - FileUtils.remove_entry(File.join(TestEnv.repos_path, new_parent.full_path), true) - FileUtils.remove_entry(File.join(TestEnv.repos_path, child.full_path), true) - end - FileUtils.remove_entry(File.join(uploads_dir, project.full_path), true) - FileUtils.remove_entry(pages_dir, true) - end - - context 'renaming child' do - context 'when no projects have pages deployed' do - it 'moves the repository and uploads', :sidekiq_inline do - project.pages_metadatum.update!(deployed: false) - child.update!(path: 'renamed') - - expect_project_directories_at('parent/renamed', with_pages: false) - end - end - end - - context 'renaming parent' do - context 'when no projects have pages deployed' do - it 'moves the repository and uploads', :sidekiq_inline do - project.pages_metadatum.update!(deployed: false) - parent.update!(path: 'renamed') - - expect_project_directories_at('renamed/child', with_pages: false) - end - end - end - - context 'moving from one parent to another' do - context 'when no projects have pages deployed' do - it 'moves the repository and uploads', :sidekiq_inline do - project.pages_metadatum.update!(deployed: false) - child.update!(parent: new_parent) - - expect_project_directories_at('new_parent/child', with_pages: false) - end - end - end - - context 'moving from having a parent to root' do - context 'when no projects have pages deployed' do - it 'moves the repository and uploads', :sidekiq_inline do - project.pages_metadatum.update!(deployed: false) - child.update!(parent: nil) - - expect_project_directories_at('child', with_pages: false) - end - end - end - - context 'moving from root to having a parent' do - context 'when no projects have pages deployed' do - it 'moves the repository and uploads', :sidekiq_inline do - project.pages_metadatum.update!(deployed: false) - parent.update!(parent: new_parent) - - expect_project_directories_at('new_parent/parent/child', with_pages: false) - end - end - end - end - end - - context 'hashed storage' do - let(:namespace) { create(:namespace) } - let!(:project) { create(:project_empty_repo, namespace: namespace) } - - it_behaves_like 'namespace restrictions' it "repository directory remains unchanged if path changed" do before_disk_path = project.disk_path @@ -1430,73 +1271,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end - describe '#rm_dir', 'callback' do - let(:repository_storage_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab.config.repositories.storages.default.legacy_disk_path - end - end - - let(:path_in_dir) { File.join(repository_storage_path, namespace.full_path) } - let(:deleted_path) { namespace.full_path.gsub(namespace.path, "#{namespace.full_path}+#{namespace.id}+deleted") } - let(:deleted_path_in_dir) { File.join(repository_storage_path, deleted_path) } - - context 'legacy storage' do - let!(:project) { create(:project_empty_repo, :legacy_storage, namespace: namespace) } - - it 'renames its dirs when deleted' do - allow(GitlabShellWorker).to receive(:perform_in) - - namespace.destroy! - - expect(File.exist?(deleted_path_in_dir)).to be(true) - end - - it 'schedules the namespace for deletion' do - expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path) - - namespace.destroy! - end - - context 'in sub-groups' do - let(:parent) { create(:group, path: 'parent') } - let(:child) { create(:group, parent: parent, path: 'child') } - let!(:project) { create(:project_empty_repo, :legacy_storage, namespace: child) } - let(:path_in_dir) { File.join(repository_storage_path, 'parent', 'child') } - let(:deleted_path) { File.join('parent', "child+#{child.id}+deleted") } - let(:deleted_path_in_dir) { File.join(repository_storage_path, deleted_path) } - - it 'renames its dirs when deleted' do - allow(GitlabShellWorker).to receive(:perform_in) - - child.destroy! - - expect(File.exist?(deleted_path_in_dir)).to be(true) - end - - it 'schedules the namespace for deletion' do - expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path) - - child.destroy! - end - end - end - - context 'hashed storage' do - let!(:project) { create(:project_empty_repo, namespace: namespace) } - - it 'has no repositories base directories to remove' do - expect(GitlabShellWorker).not_to receive(:perform_in) - - expect(File.exist?(path_in_dir)).to be(false) - - namespace.destroy! - - expect(File.exist?(deleted_path_in_dir)).to be(false) - end - end - end - describe '.find_by_path_or_name' do before do @namespace = create(:namespace, name: 'WoW', path: 'woW') @@ -2126,6 +1900,48 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end + describe '#first_auto_devops_config' do + let(:instance_autodevops_status) { Gitlab::CurrentSettings.auto_devops_enabled? } + + context 'when namespace.auto_devops_enabled is not set' do + let(:group) { create(:group) } + + it 'returns the config values using the instance setting' do + expect(group.first_auto_devops_config).to eq({ scope: :instance, status: instance_autodevops_status }) + end + + context 'when namespace does not have auto_deveops enabled but has a parent' do + let!(:parent) { create(:group, auto_devops_enabled: true) } + let!(:group) { create(:group, parent: parent) } + + it 'returns the first_auto_devops_config of the parent' do + expect(parent).to receive(:first_auto_devops_config).and_call_original + + expect(group.first_auto_devops_config).to eq({ scope: :group, status: true }) + end + + context 'then the parent is deleted' do + before do + parent.delete + group.reload + end + + it 'returns its own config with status based on the instance settings' do + expect(group.first_auto_devops_config).to eq({ scope: :instance, status: instance_autodevops_status }) + end + end + end + end + + context 'when namespace.auto_devops_enable is set' do + let(:group) { create(:group, auto_devops_enabled: false) } + + it 'returns the correct config values' do + expect(group.first_auto_devops_config).to eq({ scope: :group, status: false }) + end + end + end + describe '#user_namespace?' do subject { namespace.user_namespace? } diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 0fc689b9f6c..2b26c73aa7a 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Note, feature_category: :team_planning do describe 'associations' do it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:namespace) } it { is_expected.to belong_to(:noteable).touch(false) } it { is_expected.to belong_to(:author).class_name('User') } @@ -32,6 +33,7 @@ RSpec.describe Note, feature_category: :team_planning do it { is_expected.to validate_length_of(:note).is_at_most(1_000_000) } it { is_expected.to validate_presence_of(:note) } it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:namespace) } context 'when note is on commit' do before do @@ -310,6 +312,70 @@ RSpec.describe Note, feature_category: :team_planning do it { is_expected.to be false } end end + + describe '#ensure_namespace_id' do + context 'for a project noteable' do + let_it_be(:issue) { create(:issue) } + + it 'copies the project_namespace_id of the project' do + note = build(:note, noteable: issue, project: issue.project) + + note.valid? + + expect(note.namespace_id).to eq(issue.project.project_namespace_id) + end + + context 'when noteable is changed' do + let_it_be(:another_issue) { create(:issue) } + + it 'updates the namespace_id' do + note = create(:note, noteable: issue, project: issue.project) + + note.noteable = another_issue + note.project = another_issue.project + note.valid? + + expect(note.namespace_id).to eq(another_issue.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) + + expect { note.valid? }.not_to raise_error + end + end + end + + context 'for a personal snippet note' do + let_it_be(:snippet) { create(:personal_snippet) } + + it 'copies the personal namespace_id of the author' do + note = build(:note, noteable: snippet, project: nil) + + note.valid? + + expect(note.namespace_id).to eq(snippet.author.namespace.id) + end + + context 'when snippet author is missing' do + it 'does not raise an exception' do + note = build(:note, noteable: build(:personal_snippet, author: nil), project: nil) + + expect { note.valid? }.not_to raise_error + end + end + end + + context 'when noteable is missing' do + it 'does not raise an exception' do + note = build(:note, noteable: nil, project: nil) + + expect { note.valid? }.not_to raise_error + end + end + end end describe "Commit notes" do @@ -1595,53 +1661,30 @@ RSpec.describe Note, feature_category: :team_planning do end end - describe 'expiring ETag cache' do + describe 'broadcasting note changes' do let_it_be(:issue) { create(:issue) } let(:note) { build(:note, project: issue.project, noteable: issue) } - def expect_expiration(noteable) - expect_any_instance_of(Gitlab::EtagCaching::Store) - .to receive(:touch) - .with("/#{noteable.project.namespace.to_param}/#{noteable.project.to_param}/noteable/#{noteable.class.name.underscore}/#{noteable.id}/notes") - end - it 'broadcasts an Action Cable event for the noteable' do expect(Noteable::NotesChannel).to receive(:broadcast_to).with(note.noteable, event: 'updated') note.save! end - context 'when action_cable_notes is disabled' do - before do - stub_feature_flags(action_cable_notes: false) - end - - it 'does not broadcast an Action Cable event' do - expect(Noteable::NotesChannel).not_to receive(:broadcast_to) - - note.save! - end - end - - it "expires cache for note's issue when note is saved" do - expect_expiration(note.noteable) - + it 'broadcast an Action Cable event for the noteable when note is destroyed' do note.save! - end - it "expires cache for note's issue when note is destroyed" do - note.save! - expect_expiration(note.noteable) + expect(Noteable::NotesChannel).to receive(:broadcast_to).with(note.noteable, event: 'updated') note.destroy! end - context 'when issuable etag caching is disabled' do - it 'does not store cache key' do - allow(note.noteable).to receive(:etag_caching_enabled?).and_return(false) + context 'when issuable real_time_notes is disabled' do + it 'does not broadcast an Action Cable event' do + allow(note.noteable).to receive(:real_time_notes_enabled?).and_return(false) - expect_any_instance_of(Gitlab::EtagCaching::Store).not_to receive(:touch) + expect(Noteable::NotesChannel).not_to receive(:broadcast_to) note.save! end @@ -1653,8 +1696,8 @@ RSpec.describe Note, feature_category: :team_planning do context 'when adding a note to the MR' do let(:note) { build(:note, noteable: merge_request, project: merge_request.project) } - it 'expires the MR note etag cache' do - expect_expiration(merge_request) + it 'broadcasts an Action Cable event for the MR' do + expect(Noteable::NotesChannel).to receive(:broadcast_to).with(merge_request, event: 'updated') note.save! end @@ -1663,8 +1706,8 @@ RSpec.describe Note, feature_category: :team_planning do context 'when adding a note to a commit on the MR' do let(:note) { build(:note_on_commit, commit_id: merge_request.commits.first.id, project: merge_request.project) } - it 'expires the MR note etag cache' do - expect_expiration(merge_request) + it 'broadcasts an Action Cable event for the MR' do + expect(Noteable::NotesChannel).to receive(:broadcast_to).with(merge_request, event: 'updated') note.save! end diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 730a9045d7f..9bf95051730 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -221,4 +221,11 @@ RSpec.describe NotificationSetting do it { is_expected.to eq([notification_setting_1, notification_setting_3]) } end + + context 'with loose foreign key on notification_settings.user_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:user) } + let!(:model) { create(:notification_setting, user: parent) } + end + end end diff --git a/spec/models/oauth_access_token_spec.rb b/spec/models/oauth_access_token_spec.rb index b21a2bf2079..55c82369f33 100644 --- a/spec/models/oauth_access_token_spec.rb +++ b/spec/models/oauth_access_token_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe OauthAccessToken do +RSpec.describe OauthAccessToken, feature_category: :system_access do let(:app_one) { create(:oauth_application) } let(:app_two) { create(:oauth_application) } let(:app_three) { create(:oauth_application) } @@ -23,6 +23,10 @@ RSpec.describe OauthAccessToken do end describe 'Doorkeeper secret storing' do + it 'does not have a prefix' do + expect(token.plaintext_token).not_to start_with('gl') + end + it 'stores the token in hashed format' do expect(token.token).not_to eq(token.plaintext_token) end diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index 7838fc1c5a4..2f9f04fd3e6 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel it { is_expected.to have_many :groups } it { is_expected.to have_many(:users).through(:organization_users).inverse_of(:organizations) } it { is_expected.to have_many(:organization_users).inverse_of(:organization) } + it { is_expected.to have_many :projects } end describe 'validations' do diff --git a/spec/models/packages/dependency_link_spec.rb b/spec/models/packages/dependency_link_spec.rb index d8fde8f5eb3..3022a960c4c 100644 --- a/spec/models/packages/dependency_link_spec.rb +++ b/spec/models/packages/dependency_link_spec.rb @@ -1,7 +1,28 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Packages::DependencyLink, type: :model do +RSpec.describe Packages::DependencyLink, type: :model, feature_category: :package_registry do + let_it_be(:package1) { create(:package) } + let_it_be(:package2) { create(:package) } + let_it_be(:dependency1) { create(:packages_dependency) } + let_it_be(:dependency2) { create(:packages_dependency) } + + let_it_be(:dependency_link1) do + create(:packages_dependency_link, :dev_dependencies, package: package1, dependency: dependency1) + end + + let_it_be(:dependency_link2) do + create(:packages_dependency_link, :dependencies, package: package1, dependency: dependency2) + end + + let_it_be(:dependency_link3) do + create(:packages_dependency_link, :dependencies, package: package2, dependency: dependency1) + end + + let_it_be(:dependency_link4) do + create(:packages_dependency_link, :dependencies, package: package2, dependency: dependency2) + end + describe 'relationships' do it { is_expected.to belong_to(:package).inverse_of(:dependency_links) } it { is_expected.to belong_to(:dependency).inverse_of(:dependency_links) } @@ -53,4 +74,49 @@ RSpec.describe Packages::DependencyLink, type: :model do end end end + + describe '.dependency_ids_grouped_by_type' do + let(:packages) { Packages::Package.where(id: [package1.id, package2.id]) } + + subject { described_class.dependency_ids_grouped_by_type(packages) } + + it 'aggregates dependencies by type', :aggregate_failures do + result = Gitlab::Json.parse(subject.to_json) + + expect(result.count).to eq(2) + expect(result).to include( + hash_including( + 'package_id' => package1.id, + 'dependency_ids_by_type' => { + '1' => [dependency2.id], + '2' => [dependency1.id] + } + ), + hash_including( + 'package_id' => package2.id, + 'dependency_ids_by_type' => { + '1' => [dependency1.id, dependency2.id] + } + ) + ) + end + end + + describe '.for_packages' do + let(:packages) { Packages::Package.where(id: package1.id) } + + subject { described_class.for_packages(packages) } + + it 'returns dependency links for selected packages' do + expect(subject).to contain_exactly(dependency_link1, dependency_link2) + end + end + + describe '.select_dependency_id' do + subject { described_class.select_dependency_id } + + it 'returns only dependency_id' do + expect(subject[0].attributes).to eq('dependency_id' => dependency1.id, 'id' => nil) + end + end end diff --git a/spec/models/packages/ml_model/package_spec.rb b/spec/models/packages/ml_model/package_spec.rb new file mode 100644 index 00000000000..6a327197672 --- /dev/null +++ b/spec/models/packages/ml_model/package_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::MlModel::Package, feature_category: :mlops do + let_it_be(:project) { create(:project) } + let_it_be(:ml_model) { create(:ml_model_package, project: project) } + let_it_be(:generic_package) { create(:generic_package, project: project) } + + describe 'associations' do + it { is_expected.to have_one(:model_version) } + end + + describe 'all' do + it 'fetches only ml_model packages' do + expect(described_class.all).to eq([ml_model]) + end + end + + describe '#valid?' do + using RSpec::Parameterized::TableSyntax + + let_it_be(:valid_version) { '1.0.0' } + let_it_be(:valid_name) { 'some_model' } + + let(:version) { valid_version } + let(:name) { valid_name } + + let(:ml_model) { described_class.new(version: version, name: name, project: project) } + + subject(:errors) do + ml_model.validate + ml_model.errors + end + + it { expect(ml_model).to validate_presence_of(:name) } + it { expect(ml_model).to validate_presence_of(:version) } + + it 'validates a valid ml_model package' do + expect(errors).to be_empty + end + + describe 'name' do + where(:ctx, :name) do + 'name is blank' | '' + 'name is nil' | nil + 'name is not valid package name' | '!!()()' + 'name is too large' | ('a' * 256) + end + with_them do + it { expect(errors).to include(:name) } + end + end + + describe 'version' do + where(:ctx, :version) do + 'version is blank' | '' + 'version is nil' | nil + 'version is not valid semver' | 'v1.0.0' + 'version is too large' | ('a' * 256) + end + with_them do + it { expect(errors).to include(:version) } + end + end + end +end diff --git a/spec/models/packages/nuget/metadatum_spec.rb b/spec/models/packages/nuget/metadatum_spec.rb index e1520c0782f..c8e052baf6f 100644 --- a/spec/models/packages/nuget/metadatum_spec.rb +++ b/spec/models/packages/nuget/metadatum_spec.rb @@ -16,18 +16,7 @@ RSpec.describe Packages::Nuget::Metadatum, type: :model, feature_category: :pack it { is_expected.to validate_length_of(:authors).is_at_most(described_class::MAX_AUTHORS_LENGTH) } it { is_expected.to validate_presence_of(:description) } it { is_expected.to validate_length_of(:description).is_at_most(described_class::MAX_DESCRIPTION_LENGTH) } - - context 'for normalized_version presence' do - it { is_expected.to validate_presence_of(:normalized_version) } - - context 'when nuget_normalized_version feature flag is disabled' do - before do - stub_feature_flags(nuget_normalized_version: false) - end - - it { is_expected.not_to validate_presence_of(:normalized_version) } - end - end + it { is_expected.to validate_presence_of(:normalized_version) } %i[license_url project_url icon_url].each do |url| describe "##{url}" do @@ -87,16 +76,6 @@ RSpec.describe Packages::Nuget::Metadatum, type: :model, feature_category: :pack expect(nuget_metadatum.normalized_version).to eq(normalized_version) end - - context 'when the nuget_normalized_version feature flag is disabled' do - before do - stub_feature_flags(nuget_normalized_version: false) - end - - it 'does not save the normalized version' do - expect(nuget_metadatum.normalized_version).not_to eq(normalized_version) - end - end end end end diff --git a/spec/models/packages/nuget/symbol_spec.rb b/spec/models/packages/nuget/symbol_spec.rb new file mode 100644 index 00000000000..52e95c11939 --- /dev/null +++ b/spec/models/packages/nuget/symbol_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Nuget::Symbol, type: :model, feature_category: :package_registry do + subject(:symbol) { create(:nuget_symbol) } + + it { is_expected.to be_a FileStoreMounter } + + describe 'relationships' do + it { is_expected.to belong_to(:package).inverse_of(:nuget_symbols) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:file) } + it { is_expected.to validate_presence_of(:file_path) } + it { is_expected.to validate_presence_of(:signature) } + it { is_expected.to validate_presence_of(:object_storage_key) } + it { is_expected.to validate_presence_of(:size) } + it { is_expected.to validate_uniqueness_of(:signature).scoped_to(:file_path) } + it { is_expected.to validate_uniqueness_of(:object_storage_key).case_insensitive } + end + + describe 'delegations' do + it { is_expected.to delegate_method(:project_id).to(:package) } + end + + describe 'callbacks' do + describe 'before_validation' do + describe '#set_object_storage_key' do + context 'when signature and project_id are present' do + it 'sets the object_storage_key' do + expected_key = Gitlab::HashedPath.new( + 'packages', 'nuget', symbol.package_id, 'symbols', OpenSSL::Digest::SHA256.hexdigest(symbol.signature), + root_hash: symbol.project_id + ).to_s + + symbol.valid? + + expect(symbol.object_storage_key).to eq(expected_key) + end + end + + context 'when signature is not present' do + subject(:symbol) { build(:nuget_symbol, signature: nil) } + + it 'does not set the object_storage_key' do + symbol.valid? + + expect(symbol.object_storage_key).to be_nil + end + end + + context 'when project_id is not present' do + subject(:symbol) { build(:nuget_symbol) } + + before do + allow(symbol).to receive(:project_id).and_return(nil) + end + + it 'does not set the object_storage_key' do + symbol.valid? + + expect(symbol.object_storage_key).to be_nil + end + end + end + end + end +end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 381b5af117e..e113218e828 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -25,6 +25,7 @@ RSpec.describe Packages::Package, type: :model, feature_category: :package_regis it { is_expected.to have_one(:rubygems_metadatum).inverse_of(:package) } it { is_expected.to have_one(:npm_metadatum).inverse_of(:package) } it { is_expected.to have_one(:rpm_metadatum).inverse_of(:package) } + it { is_expected.to have_many(:nuget_symbols).inverse_of(:package) } end describe '.with_debian_codename' do @@ -875,14 +876,6 @@ RSpec.describe Packages::Package, type: :model, feature_category: :package_regis subject { described_class.with_npm_scope('test') } it { is_expected.to contain_exactly(package1) } - - context 'when npm_package_registry_fix_group_path_validation is disabled' do - before do - stub_feature_flags(npm_package_registry_fix_group_path_validation: false) - end - - it { is_expected.to contain_exactly(package1) } - end end describe '.without_nuget_temporary_name' do @@ -1505,4 +1498,38 @@ RSpec.describe Packages::Package, type: :model, feature_category: :package_regis end end end + + describe 'inheritance' do + let_it_be(:project) { create(:project) } + + let(:format) { "" } + let(:package) { create("#{format}_package", project: project) } + let(:package_id) { package.id } + + subject { described_class.find_by(id: package_id).class } + + described_class + .package_types + .keys + .map(&:to_sym) + .each do |package_format| + if described_class.inheritance_column_to_class_map[package_format].nil? + context "for package format #{package_format}" do + let(:format) { package_format } + + it 'maps to Packages::Package' do + is_expected.to eq(described_class) + end + end + else + context "for package format #{package_format}" do + let(:format) { package_format } + + it 'maps to the correct class' do + is_expected.to eq(described_class.inheritance_column_to_class_map[package_format].constantize) + end + end + end + end + end end diff --git a/spec/models/packages/protection/rule_spec.rb b/spec/models/packages/protection/rule_spec.rb new file mode 100644 index 00000000000..b368687e6d8 --- /dev/null +++ b/spec/models/packages/protection/rule_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Protection::Rule, type: :model, feature_category: :package_registry do + it_behaves_like 'having unique enum values' + + describe 'relationships' do + it { is_expected.to belong_to(:project).inverse_of(:package_protection_rules) } + 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 + end + + describe 'validations' do + subject { build(:package_protection_rule) } + + describe '#package_name_pattern' do + it { is_expected.to validate_presence_of(:package_name_pattern) } + it { is_expected.to validate_uniqueness_of(:package_name_pattern).scoped_to(:project_id, :package_type) } + it { is_expected.to validate_length_of(:package_name_pattern).is_at_most(255) } + end + + describe '#package_type' do + it { is_expected.to validate_presence_of(:package_type) } + end + + describe '#push_protected_up_to_access_level' do + it { is_expected.to validate_presence_of(:push_protected_up_to_access_level) } + + it { + is_expected.to validate_inclusion_of(:push_protected_up_to_access_level).in_array([Gitlab::Access::DEVELOPER, + Gitlab::Access::MAINTAINER, Gitlab::Access::OWNER]) + } + end + end +end diff --git a/spec/models/pages/virtual_domain_spec.rb b/spec/models/pages/virtual_domain_spec.rb index b5a421295b2..02e3fd67f2d 100644 --- a/spec/models/pages/virtual_domain_spec.rb +++ b/spec/models/pages/virtual_domain_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Pages::VirtualDomain do +RSpec.describe Pages::VirtualDomain, feature_category: :pages do describe '#certificate and #key pair' do let(:domain) { nil } let(:project) { instance_double(Project) } @@ -25,6 +25,8 @@ RSpec.describe Pages::VirtualDomain do end describe '#lookup_paths' do + let(:domain) { nil } + let(:trim_prefix) { nil } let(:project_a) { instance_double(Project) } let(:project_b) { instance_double(Project) } let(:project_c) { instance_double(Project) } @@ -32,44 +34,43 @@ RSpec.describe Pages::VirtualDomain do let(:pages_lookup_path_b) { instance_double(Pages::LookupPath, prefix: 'bbb', source: { type: 'zip', path: 'https://example.com' }) } let(:pages_lookup_path_without_source) { instance_double(Pages::LookupPath, prefix: 'ccc', source: nil) } + subject(:virtual_domain) do + described_class.new(projects: project_list, domain: domain, trim_prefix: trim_prefix) + end + + before do + allow(Pages::LookupPath) + .to receive(:new) + .with(project_a, domain: domain, trim_prefix: trim_prefix) + .and_return(pages_lookup_path_a) + + allow(Pages::LookupPath) + .to receive(:new) + .with(project_b, domain: domain, trim_prefix: trim_prefix) + .and_return(pages_lookup_path_b) + + allow(Pages::LookupPath) + .to receive(:new) + .with(project_c, domain: domain, trim_prefix: trim_prefix) + .and_return(pages_lookup_path_without_source) + end + context 'when there is pages domain provided' do let(:domain) { instance_double(PagesDomain) } - - subject(:virtual_domain) { described_class.new(projects: [project_a, project_b, project_c], domain: domain) } + let(:project_list) { [project_a, project_b, project_c] } it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do - expect(project_a).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_a) - expect(project_b).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_b) - expect(project_c).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_without_source) - expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_b, pages_lookup_path_a]) end end context 'when there is trim_prefix provided' do - subject(:virtual_domain) { described_class.new(projects: [project_a, project_b], trim_prefix: 'group/') } + let(:trim_prefix) { 'group/' } + let(:project_list) { [project_a, project_b] } it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do - expect(project_a).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_a) - expect(project_b).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_b) - expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_b, pages_lookup_path_a]) end end end - - describe '#cache_key' do - it 'returns the cache key based in the given cache_control' do - cache_control = instance_double(::Gitlab::Pages::CacheControl, cache_key: 'cache_key') - virtual_domain = described_class.new(projects: [instance_double(Project)], cache: cache_control) - - expect(virtual_domain.cache_key).to eq('cache_key') - end - - it 'returns nil when no cache_control is given' do - virtual_domain = described_class.new(projects: [instance_double(Project)]) - - expect(virtual_domain.cache_key).to be_nil - end - end end diff --git a/spec/models/pages_deployment_spec.rb b/spec/models/pages_deployment_spec.rb index bff69485e43..916197fe5e9 100644 --- a/spec/models/pages_deployment_spec.rb +++ b/spec/models/pages_deployment_spec.rb @@ -81,6 +81,57 @@ RSpec.describe PagesDeployment, feature_category: :pages do end end + describe '.deactivate_deployments_older_than', :freeze_time do + let!(:other_project_deployment) do + create(:pages_deployment) + end + + let!(:other_path_prefix_deployment) do + create(:pages_deployment, project: project, path_prefix: 'other') + end + + let!(:deactivated_deployment) do + create(:pages_deployment, project: project, deleted_at: 5.minutes.ago) + end + + it 'updates only older deployments for the same project and path prefix' do + deployment1 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) + deployment2 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) + deployment3 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) + + expect { described_class.deactivate_deployments_older_than(deployment2) } + .to change { deployment1.reload.deleted_at } + .from(nil).to(Time.zone.now) + .and change { deployment1.reload.updated_at } + .to(Time.zone.now) + + expect(deployment2.reload.deleted_at).to be_nil + expect(deployment3.reload.deleted_at).to be_nil + expect(other_project_deployment.deleted_at).to be_nil + expect(other_path_prefix_deployment.reload.deleted_at).to be_nil + expect(deactivated_deployment.reload.deleted_at).to eq(5.minutes.ago) + end + + it 'updates only older deployments for the same project with the given time' do + deployment1 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) + deployment2 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) + deployment3 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) + time = 30.minutes.from_now + + expect { described_class.deactivate_deployments_older_than(deployment2, time: time) } + .to change { deployment1.reload.deleted_at } + .from(nil).to(time) + .and change { deployment1.reload.updated_at } + .to(Time.zone.now) + + expect(deployment2.reload.deleted_at).to be_nil + expect(deployment3.reload.deleted_at).to be_nil + expect(other_project_deployment.deleted_at).to be_nil + expect(other_path_prefix_deployment.reload.deleted_at).to be_nil + expect(deactivated_deployment.reload.deleted_at).to eq(5.minutes.ago) + end + end + describe '#migrated?' do it 'returns false for normal deployment' do deployment = create(:pages_deployment) diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 3030756a413..cd740bca502 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -201,6 +201,17 @@ RSpec.describe PagesDomain do describe 'validations' do it { is_expected.to validate_presence_of(:verification_code) } + + context 'when validating max certificate key length' do + it 'validates the certificate key length' do + valid_domain = build(:pages_domain, :key_length_8192) + expect(valid_domain).to be_valid + + invalid_domain = build(:pages_domain, :extra_long_key) + expect(invalid_domain).to be_invalid + expect(invalid_domain.errors[:key]).to include('Certificate Key is too long. (Max 8192 bytes)') + end + end end describe 'default values' do diff --git a/spec/models/performance_monitoring/prometheus_metric_spec.rb b/spec/models/performance_monitoring/prometheus_metric_spec.rb deleted file mode 100644 index 58bb59793cf..00000000000 --- a/spec/models/performance_monitoring/prometheus_metric_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe PerformanceMonitoring::PrometheusMetric do - let(:json_content) do - { - "id" => "metric_of_ages", - "unit" => "count", - "label" => "Metric of Ages", - "query_range" => "http_requests_total" - } - end - - describe '.from_json' do - subject { described_class.from_json(json_content) } - - it 'creates a PrometheusMetric object' do - expect(subject).to be_a described_class - expect(subject.id).to eq(json_content['id']) - expect(subject.unit).to eq(json_content['unit']) - expect(subject.label).to eq(json_content['label']) - expect(subject.query_range).to eq(json_content['query_range']) - end - - describe 'validations' do - context 'json_content is not a hash' do - let(:json_content) { nil } - - subject { described_class.from_json(json_content) } - - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } - end - - context 'when unit is missing' do - before do - json_content['unit'] = nil - end - - subject { described_class.from_json(json_content) } - - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } - end - - context 'when query and query_range is missing' do - before do - json_content['query_range'] = nil - end - - subject { described_class.from_json(json_content) } - - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } - end - - context 'when query_range is missing but query is available' do - before do - json_content['query_range'] = nil - json_content['query'] = 'http_requests_total' - end - - subject { described_class.from_json(json_content) } - - it { is_expected.to be_valid } - end - end - end -end diff --git a/spec/models/performance_monitoring/prometheus_panel_group_spec.rb b/spec/models/performance_monitoring/prometheus_panel_group_spec.rb deleted file mode 100644 index 497f80483eb..00000000000 --- a/spec/models/performance_monitoring/prometheus_panel_group_spec.rb +++ /dev/null @@ -1,62 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe PerformanceMonitoring::PrometheusPanelGroup do - let(:json_content) do - { - "group" => "Group Title", - "panels" => [{ - "type" => "area-chart", - "title" => "Chart Title", - "y_label" => "Y-Axis", - "metrics" => [{ - "id" => "metric_of_ages", - "unit" => "count", - "label" => "Metric of Ages", - "query_range" => "http_requests_total" - }] - }] - } - end - - describe '.from_json' do - subject { described_class.from_json(json_content) } - - it 'creates a PrometheusPanelGroup object' do - expect(subject).to be_a described_class - expect(subject.group).to eq(json_content['group']) - expect(subject.panels).to all(be_a PerformanceMonitoring::PrometheusPanel) - end - - describe 'validations' do - context 'json_content is not a hash' do - let(:json_content) { nil } - - subject { described_class.from_json(json_content) } - - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } - end - - context 'when group is missing' do - before do - json_content.delete('group') - end - - subject { described_class.from_json(json_content) } - - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } - end - - context 'when panels are missing' do - before do - json_content['panels'] = [] - end - - subject { described_class.from_json(json_content) } - - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } - end - end - end -end diff --git a/spec/models/performance_monitoring/prometheus_panel_spec.rb b/spec/models/performance_monitoring/prometheus_panel_spec.rb deleted file mode 100644 index 42dcbbdb8e0..00000000000 --- a/spec/models/performance_monitoring/prometheus_panel_spec.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe PerformanceMonitoring::PrometheusPanel do - let(:json_content) do - { - "max_value" => 1, - "type" => "area-chart", - "title" => "Chart Title", - "y_label" => "Y-Axis", - "weight" => 1, - "metrics" => [{ - "id" => "metric_of_ages", - "unit" => "count", - "label" => "Metric of Ages", - "query_range" => "http_requests_total" - }] - } - end - - describe '#new' do - it 'accepts old schema format' do - expect { described_class.new(json_content) }.not_to raise_error - end - - it 'accepts new schema format' do - expect { described_class.new(json_content.merge("y_axis" => { "precision" => 0 })) }.not_to raise_error - end - end - - describe '.from_json' do - subject { described_class.from_json(json_content) } - - it 'creates a PrometheusPanelGroup object' do - expect(subject).to be_a described_class - expect(subject.type).to eq(json_content['type']) - expect(subject.title).to eq(json_content['title']) - expect(subject.y_label).to eq(json_content['y_label']) - expect(subject.weight).to eq(json_content['weight']) - expect(subject.metrics).to all(be_a PerformanceMonitoring::PrometheusMetric) - end - - describe 'validations' do - context 'json_content is not a hash' do - let(:json_content) { nil } - - subject { described_class.from_json(json_content) } - - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } - end - - context 'when title is missing' do - before do - json_content['title'] = nil - end - - subject { described_class.from_json(json_content) } - - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } - end - - context 'when metrics are missing' do - before do - json_content.delete('metrics') - end - - subject { described_class.from_json(json_content) } - - it { expect { subject }.to raise_error(ActiveModel::ValidationError) } - end - end - end - - describe '.id' do - it 'returns hexdigest of group_title, type and title as the panel id' do - group_title = 'Business Group' - panel_type = 'area-chart' - panel_title = 'New feature requests made' - - expect(Digest::SHA2).to receive(:hexdigest).with("#{group_title}#{panel_type}#{panel_title}").and_return('hexdigest') - expect(described_class.new(title: panel_title, type: panel_type).id(group_title)).to eql 'hexdigest' - end - end -end diff --git a/spec/models/plan_spec.rb b/spec/models/plan_spec.rb index 73e88a17e24..fe3365ca78f 100644 --- a/spec/models/plan_spec.rb +++ b/spec/models/plan_spec.rb @@ -3,6 +3,18 @@ require 'spec_helper' RSpec.describe Plan do + describe 'scopes', :aggregate_failures do + let_it_be(:default_plan) { create(:default_plan) } + + describe '.by_name' do + it 'returns plans by their name' do + expect(described_class.by_name('default')).to match_array([default_plan]) + expect(described_class.by_name(%w[default unknown])).to match_array([default_plan]) + expect(described_class.by_name(nil)).to be_empty + end + end + end + describe '#default?' do subject { plan.default? } diff --git a/spec/models/pool_repository_spec.rb b/spec/models/pool_repository_spec.rb index 93c1e59458d..bafda406774 100644 --- a/spec/models/pool_repository_spec.rb +++ b/spec/models/pool_repository_spec.rb @@ -13,15 +13,17 @@ RSpec.describe PoolRepository, feature_category: :source_code_management do let!(:pool_repository) { create(:pool_repository) } it { is_expected.to validate_presence_of(:shard) } - it { is_expected.to validate_presence_of(:source_project) } end describe 'scopes' do let_it_be(:project1) { create(:project) } let_it_be(:project2) { create(:project) } let_it_be(:new_shard) { create(:shard, name: 'new') } - let_it_be(:pool_repository1) { create(:pool_repository, source_project: project1) } - let_it_be(:pool_repository2) { create(:pool_repository, source_project: project1, shard: new_shard) } + let_it_be(:pool_repository1) { create(:pool_repository, source_project: project1, disk_path: 'disk_path') } + let_it_be(:pool_repository2) do + create(:pool_repository, source_project: project1, disk_path: 'disk_path', shard: new_shard) + end + let_it_be(:another_pool_repository) { create(:pool_repository, source_project: project2) } describe '.by_source_project' do @@ -32,8 +34,8 @@ RSpec.describe PoolRepository, feature_category: :source_code_management do end end - describe '.by_source_project_and_shard_name' do - subject { described_class.by_source_project_and_shard_name(project1, new_shard.name) } + describe '.by_disk_path_and_shard_name' do + subject { described_class.by_disk_path_and_shard_name('disk_path', new_shard.name) } it 'returns only a requested pool repository' do is_expected.to match_array([pool_repository2]) @@ -91,4 +93,38 @@ RSpec.describe PoolRepository, feature_category: :source_code_management do end end end + + describe '#object_pool' do + subject { pool.object_pool } + + let(:pool) { build(:pool_repository, :ready, source_project: project, disk_path: disk_path) } + let(:project) { build(:project) } + let(:disk_path) { 'disk_path' } + + it 'returns an object pool instance' do + is_expected.to be_a_kind_of(Gitlab::Git::ObjectPool) + + is_expected.to have_attributes( + storage: pool.shard.name, + relative_path: "#{pool.disk_path}.git", + source_repository: pool.source_project.repository.raw, + gl_project_path: pool.source_project.full_path + ) + end + + context 'when source project is missing' do + let(:project) { nil } + + it 'returns an object pool instance' do + is_expected.to be_a_kind_of(Gitlab::Git::ObjectPool) + + is_expected.to have_attributes( + storage: pool.shard.name, + relative_path: "#{pool.disk_path}.git", + source_repository: nil, + gl_project_path: nil + ) + end + end + end end diff --git a/spec/models/project_authorization_spec.rb b/spec/models/project_authorization_spec.rb index 2ba7f5c4ca4..9fed05342aa 100644 --- a/spec/models/project_authorization_spec.rb +++ b/spec/models/project_authorization_spec.rb @@ -3,6 +3,23 @@ require 'spec_helper' RSpec.describe ProjectAuthorization, feature_category: :groups_and_projects do + describe 'create' do + let_it_be(:user) { create(:user) } + let_it_be(:project_1) { create(:project) } + + let(:project_auth) do + build( + :project_authorization, + user: user, + project: project_1 + ) + end + + it 'sets is_unique' do + expect { project_auth.save! }.to change { project_auth.is_unique }.to(true) + end + end + describe 'unique user, project authorizations' do let_it_be(:user) { create(:user) } let_it_be(:project_1) { create(:project) } @@ -65,6 +82,26 @@ RSpec.describe ProjectAuthorization, feature_category: :groups_and_projects do it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.all_values) } end + describe 'scopes' do + 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) } + let_it_be(:project_authorization_developer) { create(:project_authorization, :developer, project: project) } + let_it_be(:project_authorization_maintainer) { create(:project_authorization, :maintainer, project: project) } + let_it_be(:project_authorization_owner) { create(:project_authorization, :owner, project: project) } + + it 'returns all records which are greater than Guests access' do + expect(described_class.non_guests.map(&:attributes)).to match_array([ + project_authorization_reporter, project_authorization_developer, + project_authorization_maintainer, project_authorization_owner, + project_original_owner_authorization + ].map(&:attributes)) + end + end + end + describe '.insert_all' do let_it_be(:user) { create(:user) } let_it_be(:project_1) { create(:project) } diff --git a/spec/models/project_authorizations/changes_spec.rb b/spec/models/project_authorizations/changes_spec.rb index d0718153d16..5f4dd963fb3 100644 --- a/spec/models/project_authorizations/changes_spec.rb +++ b/spec/models/project_authorizations/changes_spec.rb @@ -85,7 +85,7 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro apply_project_authorization_changes expect(user.project_authorizations.pluck(:user_id, :project_id, - :access_level)).to match_array(authorizations_to_add.map(&:values)) + :access_level, :is_unique)).to match_array(authorizations_to_add.map(&:values)) end end @@ -101,7 +101,13 @@ RSpec.describe ProjectAuthorizations::Changes, feature_category: :groups_and_pro apply_project_authorization_changes expect(user.project_authorizations.pluck(:user_id, :project_id, - :access_level)).to match_array(authorizations_to_add.map(&:values)) + :access_level, :is_unique)).to match_array(authorizations_to_add.map(&:values)) + end + + it 'writes is_unique' do + apply_project_authorization_changes + + expect(user.project_authorizations.pluck(:is_unique)).to all(be(true)) end it_behaves_like 'logs the detail', batch_size: 2 diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb index 0a818147bfc..1c53f6eae52 100644 --- a/spec/models/project_ci_cd_setting_spec.rb +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ProjectCiCdSetting do +RSpec.describe ProjectCiCdSetting, feature_category: :continuous_integration do using RSpec::Parameterized::TableSyntax describe 'validations' do diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 48c9567ebb3..39e77df1900 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -437,4 +437,24 @@ RSpec.describe ProjectFeature, feature_category: :groups_and_projects do end end # rubocop:enable Gitlab/FeatureAvailableUsage + + describe '#private?' do + where(:merge_requests_access_level, :expected_value) do + ProjectFeature::PUBLIC | false + ProjectFeature::ENABLED | false + ProjectFeature::PRIVATE | true + end + + with_them do + let(:project) { build_stubbed(:project) } + + subject { project.project_feature.private?(:merge_requests) } + + before do + project.project_feature.merge_requests_access_level = merge_requests_access_level + end + + it { is_expected.to be(expected_value) } + end + end end diff --git a/spec/models/project_import_state_spec.rb b/spec/models/project_import_state_spec.rb index 7ceb4931c4f..10f4791b216 100644 --- a/spec/models/project_import_state_spec.rb +++ b/spec/models/project_import_state_spec.rb @@ -125,6 +125,14 @@ RSpec.describe ProjectImportState, type: :model, feature_category: :importers do end end + describe '#completed?' do + it { expect(described_class.new(status: :failed)).to be_completed } + it { expect(described_class.new(status: :finished)).to be_completed } + it { expect(described_class.new(status: :canceled)).to be_completed } + it { expect(described_class.new(status: :scheduled)).not_to be_completed } + it { expect(described_class.new(status: :started)).not_to be_completed } + end + describe '#expire_etag_cache' do context 'when project import type has realtime changes endpoint' do before do diff --git a/spec/models/project_metrics_setting_spec.rb b/spec/models/project_metrics_setting_spec.rb deleted file mode 100644 index 6639f9cb208..00000000000 --- a/spec/models/project_metrics_setting_spec.rb +++ /dev/null @@ -1,63 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ProjectMetricsSetting do - describe 'Associations' do - it { is_expected.to belong_to(:project) } - end - - describe 'Validations' do - context 'when external_dashboard_url is over 255 chars' do - before do - subject.external_dashboard_url = 'https://' + 'a' * 250 - end - - it 'fails validation' do - expect(subject).not_to be_valid - expect(subject.errors.messages[:external_dashboard_url]) - .to include('is too long (maximum is 255 characters)') - end - end - - context 'with unsafe url' do - before do - subject.external_dashboard_url = %{https://replaceme.com/'>} - end - - it { is_expected.to be_invalid } - end - - context 'non ascii chars in external_dashboard_url' do - before do - subject.external_dashboard_url = 'http://gitlab.com/api/0/projects/project1/something€' - end - - it { is_expected.to be_invalid } - end - - context 'internal url in external_dashboard_url' do - before do - subject.external_dashboard_url = 'http://192.168.1.1' - end - - it { is_expected.to be_valid } - end - - context 'dashboard_timezone' do - it { is_expected.to define_enum_for(:dashboard_timezone).with_values({ local: 0, utc: 1 }) } - - it 'defaults to local' do - expect(subject.dashboard_timezone).to eq('local') - end - end - end - - describe '#dashboard_timezone=' do - it 'downcases string' do - subject.dashboard_timezone = 'UTC' - - expect(subject.dashboard_timezone).to eq('utc') - end - end -end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5d622b8eccd..aedfc7fca53 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -17,12 +17,14 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it_behaves_like 'ensures runners_token is prefixed', :project describe 'associations' do + it { is_expected.to belong_to(:organization) } it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:namespace) } it { is_expected.to belong_to(:project_namespace).class_name('Namespaces::ProjectNamespace').with_foreign_key('project_namespace_id').inverse_of(:project) } it { is_expected.to belong_to(:creator).class_name('User') } it { is_expected.to belong_to(:pool_repository) } it { is_expected.to have_many(:users) } + it { is_expected.to have_many(:maintainers).through(:project_members).source(:user).conditions(members: { access_level: Gitlab::Access::MAINTAINER }) } it { is_expected.to have_many(:events) } it { is_expected.to have_many(:merge_requests) } it { is_expected.to have_many(:merge_request_metrics).class_name('MergeRequest::Metrics') } @@ -139,11 +141,9 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it { is_expected.to have_many(:sourced_pipelines) } it { is_expected.to have_many(:source_pipelines) } it { is_expected.to have_many(:prometheus_alert_events) } - it { is_expected.to have_many(:self_managed_prometheus_alert_events) } it { is_expected.to have_many(:alert_management_alerts) } it { is_expected.to have_many(:alert_management_http_integrations) } it { is_expected.to have_many(:jira_imports) } - it { is_expected.to have_many(:metrics_users_starred_dashboards).inverse_of(:project) } it { is_expected.to have_many(:repository_storage_moves) } it { is_expected.to have_many(:reviews).inverse_of(:project) } it { is_expected.to have_many(:packages).class_name('Packages::Package') } @@ -152,6 +152,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::ProjectDistribution').dependent(:destroy) } it { is_expected.to have_many(:npm_metadata_caches).class_name('Packages::Npm::MetadataCache') } it { is_expected.to have_one(:packages_cleanup_policy).class_name('Packages::Cleanup::Policy').inverse_of(:project) } + it { is_expected.to have_many(:package_protection_rules).class_name('Packages::Protection::Rule').inverse_of(:project) } it { is_expected.to have_many(:pipeline_artifacts).dependent(:restrict_with_error) } it { is_expected.to have_many(:terraform_states).class_name('Terraform::State').inverse_of(:project) } it { is_expected.to have_many(:timelogs) } @@ -224,6 +225,23 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect(project.lfs_objects.to_a).to eql([lfs_object]) end + describe 'maintainers association' do + let_it_be(:project) { create(:project) } + let_it_be(:maintainer1) { create(:user) } + let_it_be(:maintainer2) { create(:user) } + let_it_be(:reporter) { create(:user) } + + before do + project.add_maintainer(maintainer1) + project.add_maintainer(maintainer2) + project.add_reporter(reporter) + end + + it 'returns only maintainers' do + expect(project.maintainers).to match_array([maintainer1, maintainer2]) + end + end + context 'after initialized' do it "has a project_feature" do expect(described_class.new.project_feature).to be_present @@ -1140,6 +1158,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr merge_pipelines_enabled merge_trains_enabled auto_rollback_enabled + merge_trains_skip_train_allowed ) end end @@ -2380,7 +2399,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#service_desk_address' do + describe '#service_desk_address', feature_category: :service_desk do let_it_be(:project, reload: true) { create(:project, service_desk_enabled: true) } subject { project.service_desk_address } @@ -2424,7 +2443,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end context 'when project_key is set' do - it 'returns custom address including the project_key' do + it 'returns Service Desk alias address including the project_key' do create(:service_desk_setting, project: project, project_key: 'key1') expect(subject).to eq("foo+#{project.full_path_slug}-key1@bar.com") @@ -2432,11 +2451,35 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end context 'when project_key is not set' do - it 'returns custom address including the project full path' do + it 'returns Service Desk alias address including the project full path' do expect(subject).to eq("foo+#{project.full_path_slug}-#{project.project_id}-issue-@bar.com") end end end + + context 'when custom email is enabled' do + let(:custom_email) { 'support@example.com' } + + before do + setting = ServiceDeskSetting.new(project: project, custom_email: custom_email, custom_email_enabled: true) + allow(project).to receive(:service_desk_setting).and_return(setting) + end + + it 'returns custom email address' do + expect(subject).to eq(custom_email) + end + + context 'when feature flag service_desk_custom_email is disabled' do + before do + stub_feature_flags(service_desk_custom_email: false) + end + + it 'returns custom email address' do + # Don't check for a specific value. Just make sure it's not the custom email + expect(subject).not_to eq(custom_email) + end + end + end end describe '.with_service_desk_key' do @@ -3882,16 +3925,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#mark_primary_write_location' do - let(:project) { create(:project) } - - it 'marks the location with project ID' do - expect(ApplicationRecord.sticking).to receive(:mark_primary_write_location).with(:project, project.id) - - project.mark_primary_write_location - end - end - describe '#mark_stuck_remote_mirrors_as_failed!' do it 'fails stuck remote mirrors' do project = create(:project, :repository, :remote_mirror) @@ -5086,7 +5119,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr subject { described_class.wrap_with_cte(projects) } it 'wrapped query matches original' do - expect(subject.to_sql).to match(/^WITH "projects_cte" AS #{Gitlab::Database::AsWithMaterialized.materialized_if_supported}/) + expect(subject.to_sql).to match(/^WITH "projects_cte" AS MATERIALIZED/) expect(subject).to match_array(projects) end end @@ -6986,8 +7019,11 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr let_it_be_with_reload(:project) { create(:project, :empty_repo) } let_it_be(:shard_to) { create(:shard, name: 'test_second_storage') } - let!(:pool1) { create(:pool_repository, source_project: project) } - let!(:pool2) { create(:pool_repository, shard: shard_to, source_project: project) } + let(:disk_path1) { '@pool/aa/bb' } + let(:disk_path2) { disk_path1 } + + let!(:pool1) { create(:pool_repository, disk_path: disk_path1, source_project: project) } + let!(:pool2) { create(:pool_repository, disk_path: disk_path2, shard: shard_to, source_project: project) } let(:project_pool) { pool1 } let(:repository_storage) { shard_to.name } @@ -7045,6 +7081,14 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect { swap_pool_repository! }.to raise_error(ActiveRecord::RecordNotFound) end end + + context 'when pool repository has a different disk path' do + let(:disk_path2) { '@pool/different' } + + it 'raises record not found error' do + expect { swap_pool_repository! }.to raise_error(ActiveRecord::RecordNotFound) + end + end end describe '#leave_pool_repository' do @@ -7412,17 +7456,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#pages_lookup_path' do - let(:pages_domain) { build(:pages_domain) } - let(:project) { build(:project) } - - it 'returns instance of Pages::LookupPath' do - expect(Pages::LookupPath).to receive(:new).with(project, domain: pages_domain, trim_prefix: 'mygroup').and_call_original - - expect(project.pages_lookup_path(domain: pages_domain, trim_prefix: 'mygroup')).to be_a(Pages::LookupPath) - end - end - describe '.with_pages_deployed' do it 'returns only projects that have pages deployed' do _project_without_pages = create(:project) @@ -8088,14 +8121,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it { is_expected.not_to include(user) } end - describe "#metrics_setting" do - let(:project) { build(:project) } - - it 'creates setting if it does not exist' do - expect(project.metrics_setting).to be_an_instance_of(ProjectMetricsSetting) - end - end - describe '#enabled_group_deploy_keys' do let_it_be(:project) { create(:project) } @@ -9170,6 +9195,20 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end + describe '#supports_lock_on_merge?' do + it_behaves_like 'checks self (project) and root ancestor feature flag' do + let(:feature_flag) { :enforce_locked_labels_on_merge } + let(:feature_flag_method) { :supports_lock_on_merge? } + end + end + + context 'with loose foreign key on organization_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let_it_be(:parent) { create(:organization) } + let_it_be(:model) { create(:project, organization: parent) } + end + end + private def finish_job(export_job) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index ea229ddf31f..af7457c78e2 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2113,6 +2113,17 @@ RSpec.describe Repository, feature_category: :source_code_management do end end + describe '#update_refs' do + let(:expected_return) { 'updated' } + let(:params) { double } + + it 'calls the update_refs method on the raw repo with the same params' do + expect(repository.raw_repository).to receive(:update_refs).with(params).and_return('updated') + + expect(repository.update_refs(params)).to eq(expected_return) + end + end + describe '#ff_merge' do let(:target_branch) { 'ff-target' } let(:merge_request) do @@ -2424,7 +2435,6 @@ RSpec.describe Repository, feature_category: :source_code_management do :has_visible_content?, :issue_template_names_hash, :merge_request_template_names_hash, - :user_defined_metrics_dashboard_paths, :xcode_project?, :has_ambiguous_refs? ] @@ -3822,9 +3832,24 @@ RSpec.describe Repository, feature_category: :source_code_management do end end + context 'when one of the param is nonexistant' do + it 'returns nil' do + expect(repository.get_patch_id('HEAD', "f" * 40)).to be_nil + end + end + context 'when two revisions are the same' do - it 'raises an Gitlab::Git::CommandError error' do - expect { repository.get_patch_id('HEAD', 'HEAD') }.to raise_error(Gitlab::Git::CommandError) + it 'returns nil' do + expect(repository.get_patch_id('HEAD', 'HEAD')).to be_nil + end + end + + context 'when a Gitlab::Git::CommandError is raised' do + it 'returns nil' do + expect(repository.raw_repository) + .to receive(:get_patch_id).and_raise(Gitlab::Git::CommandError) + + expect(repository.get_patch_id('HEAD', "f" * 40)).to be_nil end end end diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb index eb28010d57f..8cc89578e0e 100644 --- a/spec/models/resource_label_event_spec.rb +++ b/spec/models/resource_label_event_spec.rb @@ -52,24 +52,17 @@ RSpec.describe ResourceLabelEvent, feature_category: :team_planning, type: :mode end context 'callbacks' do - describe '#expire_etag_cache' do - def expect_expiration(issue) - expect_next_instance_of(Gitlab::EtagCaching::Store) do |instance| - expect(instance).to receive(:touch) - .with("/#{issue.project.namespace.to_param}/#{issue.project.to_param}/noteable/issue/#{issue.id}/notes") - end - end - - it 'expires resource note etag cache on event save' do - expect_expiration(subject.issuable) + describe '#broadcast_notes_changed' do + it 'broadcasts note change on event save' do + expect(subject.issuable).to receive(:broadcast_notes_changed) subject.save! end - it 'expires resource note etag cache on event destroy' do + it 'broadcasts note change on event destroy' do subject.save! - expect_expiration(subject.issuable) + expect(subject.issuable).to receive(:broadcast_notes_changed) subject.destroy! end diff --git a/spec/models/resource_state_event_spec.rb b/spec/models/resource_state_event_spec.rb index de101107268..5bd8b664d23 100644 --- a/spec/models/resource_state_event_spec.rb +++ b/spec/models/resource_state_event_spec.rb @@ -58,11 +58,13 @@ RSpec.describe ResourceStateEvent, feature_category: :team_planning, type: :mode close_issue end - it_behaves_like 'issue_edit snowplow tracking' do - let(:property) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CLOSED } + it_behaves_like 'internal event tracking' do + subject(:service_action) { close_issue } + + let(:action) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CLOSED } let(:project) { issue.project } + let(:namespace) { issue.project.namespace } let(:user) { issue.author } - subject(:service_action) { close_issue } end end @@ -81,11 +83,13 @@ RSpec.describe ResourceStateEvent, feature_category: :team_planning, type: :mode reopen_issue end - it_behaves_like 'issue_edit snowplow tracking' do - let(:property) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_REOPENED } + it_behaves_like 'internal event tracking' do + subject(:service_action) { reopen_issue } + + let(:action) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_REOPENED } let(:project) { issue.project } let(:user) { issue.author } - subject(:service_action) { reopen_issue } + let(:namespace) { issue.project.namespace } end end diff --git a/spec/models/review_spec.rb b/spec/models/review_spec.rb index 2683dc93a4b..75cdea5bca7 100644 --- a/spec/models/review_spec.rb +++ b/spec/models/review_spec.rb @@ -41,4 +41,23 @@ RSpec.describe Review do expect(review.participants).to include(review.author) end end + + describe '#from_merge_request_author?' do + let(:merge_request) { build_stubbed(:merge_request) } + let(:review) { build_stubbed(:review, merge_request: merge_request, author: author) } + + subject(:from_merge_request_author?) { review.from_merge_request_author? } + + context 'when review author is the merge request author' do + let(:author) { merge_request.author } + + it { is_expected.to eq(true) } + end + + context 'when review author is not the merge request author' do + let(:author) { build_stubbed(:user) } + + it { is_expected.to eq(false) } + end + end end diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index 0bdaa4994e5..aa5fc231e14 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Route do + include LooseForeignKeysHelper + let(:group) { create(:group, path: 'git_lab', name: 'git_lab') } let(:route) { group.route } @@ -285,6 +287,7 @@ RSpec.describe Route do expect do Group.delete(conflicting_group) # delete group with conflicting route + process_loose_foreign_key_deletions(record: conflicting_group) end.to change { described_class.count }.by(-1) # check the conflicting route is gone @@ -305,4 +308,11 @@ RSpec.describe Route do end end end + + context 'with loose foreign key on routes.namespace_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:namespace) } + let!(:model) { parent.route } + end + end end diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index c2fbede8ea9..8048f255272 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -293,7 +293,7 @@ RSpec.describe SnippetRepository do it_behaves_like 'snippet repository with git errors', 'README', described_class::CommitError context 'when user name is invalid' do - let(:user) { create(:user, name: '.') } + let(:user) { create(:user, name: ',') } it_behaves_like 'snippet repository with git errors', 'non_existing_file', described_class::InvalidSignatureError end diff --git a/spec/models/user_custom_attribute_spec.rb b/spec/models/user_custom_attribute_spec.rb index 7d3806fcdfa..4c27e8d8944 100644 --- a/spec/models/user_custom_attribute_spec.rb +++ b/spec/models/user_custom_attribute_spec.rb @@ -49,8 +49,8 @@ RSpec.describe UserCustomAttribute, feature_category: :user_profile do it 'adds the abuse report ID to user custom attributes' do subject - custom_attribute = user.custom_attributes.by_key(UserCustomAttribute::AUTO_BANNED_BY_ABUSE_REPORT_ID).first - expect(custom_attribute.value).to eq(abuse_report.id.to_s) + custom_attribute = user.custom_attributes.by_key(UserCustomAttribute::AUTO_BANNED_BY_ABUSE_REPORT_ID) + expect(custom_attribute.map(&:value)).to match([abuse_report.id.to_s]) end context 'when abuse report is nil' do @@ -65,6 +65,31 @@ RSpec.describe UserCustomAttribute, feature_category: :user_profile do end end + describe '.set_banned_by_spam_log' do + let_it_be(:user) { create(:user) } + let(:spam_log) { create(:spam_log, user: user) } + + subject { described_class.set_banned_by_spam_log(spam_log) } + + it 'adds the spam log ID to user custom attributes' do + subject + + custom_attribute = user.custom_attributes.by_key(UserCustomAttribute::AUTO_BANNED_BY_SPAM_LOG_ID) + expect(custom_attribute.map(&:value)).to match([spam_log.id.to_s]) + end + + context 'when the spam log is nil' do + let(:spam_log) { nil } + + it 'does not update custom attributes' do + subject + + custom_attribute = user.custom_attributes.by_key(UserCustomAttribute::AUTO_BANNED_BY_SPAM_LOG_ID).first + expect(custom_attribute).to be_nil + end + end + end + describe '#upsert_custom_attributes' do subject { described_class.upsert_custom_attributes(custom_attributes) } diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 729635b5a27..401a85e2f82 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -238,6 +238,20 @@ RSpec.describe UserPreference, feature_category: :user_profile do end end + describe '#keyboard_shortcuts_enabled' do + it 'is set to true by default' do + pref = described_class.new + + expect(pref.keyboard_shortcuts_enabled).to eq(true) + end + + it 'returns assigned value' do + pref = described_class.new(keyboard_shortcuts_enabled: false) + + expect(pref.keyboard_shortcuts_enabled).to eq(false) + end + end + describe '#render_whitespace_in_code' do it 'is set to false by default' do pref = described_class.new diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 788600194a5..c611c3c26e3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -65,6 +65,9 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to delegate_method(:project_shortcut_buttons).to(:user_preference) } it { is_expected.to delegate_method(:project_shortcut_buttons=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:keyboard_shortcuts_enabled).to(:user_preference) } + it { is_expected.to delegate_method(:keyboard_shortcuts_enabled=).to(:user_preference).with_arguments(:args) } + it { is_expected.to delegate_method(:render_whitespace_in_code).to(:user_preference) } it { is_expected.to delegate_method(:render_whitespace_in_code=).to(:user_preference).with_arguments(:args) } @@ -168,7 +171,6 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to have_many(:abuse_events).class_name('Abuse::Event').inverse_of(:user) } it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') } it { is_expected.to have_many(:releases).dependent(:nullify) } - it { is_expected.to have_many(:metrics_users_starred_dashboards).inverse_of(:user) } it { is_expected.to have_many(:reviews).inverse_of(:author) } it { is_expected.to have_many(:merge_request_assignees).inverse_of(:assignee) } it { is_expected.to have_many(:merge_request_reviewers).inverse_of(:reviewer) } @@ -2093,6 +2095,7 @@ RSpec.describe User, feature_category: :user_profile do it 'uses SecureRandom to generate the incoming email token' do allow_next_instance_of(User) do |user| allow(user).to receive(:update_highest_role) + allow(user).to receive(:associate_with_enterprise_group) end allow_next_instance_of(Namespaces::UserNamespace) do |namespace| @@ -2906,7 +2909,7 @@ RSpec.describe User, feature_category: :user_profile do it 'applies defaults to user' do expect(user.projects_limit).to eq(123) expect(user.can_create_group).to be_falsey - expect(user.theme_id).to eq(1) + expect(user.theme_id).to eq(3) end it 'does not undo projects_limit setting if it matches old DB default of 10' do @@ -5343,56 +5346,6 @@ RSpec.describe User, feature_category: :user_profile do end end - describe '.ghost' do - it "creates a ghost user if one isn't already present" do - ghost = described_class.ghost - - expect(ghost).to be_ghost - expect(ghost).to be_persisted - expect(ghost.namespace).not_to be_nil - expect(ghost.namespace).to be_persisted - expect(ghost.user_type).to eq 'ghost' - end - - it 'does not create a second ghost user if one is already present' do - expect do - described_class.ghost - described_class.ghost - end.to change { described_class.count }.by(1) - expect(described_class.ghost).to eq(described_class.ghost) - end - - context "when a regular user exists with the username 'ghost'" do - it 'creates a ghost user with a non-conflicting username' do - create(:user, username: 'ghost') - ghost = described_class.ghost - - expect(ghost).to be_persisted - expect(ghost.username).to eq('ghost1') - end - end - - context "when a regular user exists with the email 'ghost@example.com'" do - it 'creates a ghost user with a non-conflicting email' do - create(:user, email: 'ghost@example.com') - ghost = described_class.ghost - - expect(ghost).to be_persisted - expect(ghost.email).to eq('ghost1@example.com') - end - end - - context 'when a domain allowlist is in place' do - before do - stub_application_setting(domain_allowlist: ['gitlab.com']) - end - - it 'creates a ghost user' do - expect(described_class.ghost).to be_persisted - end - end - end - describe '#update_two_factor_requirement' do let(:user) { create :user } @@ -6088,7 +6041,7 @@ RSpec.describe User, feature_category: :user_profile do it 'creates an abuse report with the correct data' do expect { subject }.to change { AbuseReport.count }.from(0).to(1) expect(AbuseReport.last.attributes).to include({ - reporter_id: described_class.security_bot.id, + reporter_id: Users::Internal.security_bot.id, user_id: user.id, category: "spam", message: 'Potential spammer account deletion' @@ -6109,7 +6062,9 @@ RSpec.describe User, feature_category: :user_profile do end context 'when there is an existing abuse report' do - let!(:abuse_report) { create(:abuse_report, user: user, reporter: described_class.security_bot, message: 'Existing') } + let!(:abuse_report) do + create(:abuse_report, user: user, reporter: Users::Internal.security_bot, message: 'Existing') + end it 'updates the abuse report' do subject @@ -7535,66 +7490,6 @@ RSpec.describe User, feature_category: :user_profile do end end - context 'bot users' do - shared_examples 'bot users' do |bot_type| - it 'creates the user if it does not exist' do - expect do - described_class.public_send(bot_type) - end.to change { User.where(user_type: bot_type).count }.by(1) - end - - it 'creates a route for the namespace of the created user' do - bot_user = described_class.public_send(bot_type) - - expect(bot_user.namespace.route).to be_present - end - - it 'does not create a new user if it already exists' do - described_class.public_send(bot_type) - - expect do - described_class.public_send(bot_type) - end.not_to change { User.count } - end - end - - shared_examples 'bot user avatars' do |bot_type, avatar_filename| - it 'sets the custom avatar for the created bot' do - bot_user = described_class.public_send(bot_type) - - expect(bot_user.avatar.url).to be_present - expect(bot_user.avatar.filename).to eq(avatar_filename) - end - end - - it_behaves_like 'bot users', :alert_bot - it_behaves_like 'bot users', :support_bot - it_behaves_like 'bot users', :migration_bot - it_behaves_like 'bot users', :security_bot - it_behaves_like 'bot users', :ghost - it_behaves_like 'bot users', :automation_bot - it_behaves_like 'bot users', :admin_bot - - it_behaves_like 'bot user avatars', :alert_bot, 'alert-bot.png' - it_behaves_like 'bot user avatars', :support_bot, 'support-bot.png' - it_behaves_like 'bot user avatars', :security_bot, 'security-bot.png' - it_behaves_like 'bot user avatars', :automation_bot, 'support-bot.png' - it_behaves_like 'bot user avatars', :admin_bot, 'admin-bot.png' - - context 'when bot is the support_bot' do - subject { described_class.support_bot } - - it { is_expected.to be_confirmed } - end - - context 'when bot is the admin bot' do - subject { described_class.admin_bot } - - it { is_expected.to be_admin } - it { is_expected.to be_confirmed } - end - end - describe '#confirmation_required_on_sign_in?' do subject { user.confirmation_required_on_sign_in? } @@ -7868,7 +7763,7 @@ RSpec.describe User, feature_category: :user_profile do let_it_be(:external_user) { create(:user, :external) } let_it_be(:unconfirmed_user) { create(:user, confirmed_at: nil) } let_it_be(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') } - let_it_be(:internal_user) { User.alert_bot.tap { |u| u.confirm } } + let_it_be(:internal_user) { Users::Internal.alert_bot.tap { |u| u.confirm } } it 'does not return blocked or banned users' do expect(described_class.without_forbidden_states).to match_array( diff --git a/spec/models/users/credit_card_validation_spec.rb b/spec/models/users/credit_card_validation_spec.rb index 4db3683c057..486d1c6d3ea 100644 --- a/spec/models/users/credit_card_validation_spec.rb +++ b/spec/models/users/credit_card_validation_spec.rb @@ -2,14 +2,19 @@ require 'spec_helper' -RSpec.describe Users::CreditCardValidation do +RSpec.describe Users::CreditCardValidation, feature_category: :user_profile do it { is_expected.to belong_to(:user) } it { is_expected.to validate_length_of(:holder_name).is_at_most(50) } it { is_expected.to validate_length_of(:network).is_at_most(32) } it { is_expected.to validate_numericality_of(:last_digits).is_less_than_or_equal_to(9999) } - describe '.similar_records' do + it { is_expected.to validate_length_of(:last_digits_hash).is_at_most(44) } + it { is_expected.to validate_length_of(:holder_name_hash).is_at_most(44) } + it { is_expected.to validate_length_of(:expiration_date_hash).is_at_most(44) } + 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) end @@ -53,6 +58,22 @@ RSpec.describe Users::CreditCardValidation do end describe 'scopes' do + describe '.find_or_initialize_by_user' do + subject(:find_or_initialize_by_user) { described_class.find_or_initialize_by_user(user.id) } + + let_it_be(:user) { create(:user) } + + context 'with no existing credit card record' do + it { is_expected.to be_a_new_record } + end + + context 'with existing credit card record' do + let_it_be(:credit_card_validation) { create(:credit_card_validation, user: user) } + + it { is_expected.to eq(credit_card_validation) } + end + end + describe '.by_banned_user' do let(:banned_user) { create(:banned_user) } let!(:credit_card) { create(:credit_card_validation) } @@ -154,4 +175,122 @@ RSpec.describe Users::CreditCardValidation do it { is_expected.not_to be_used_by_banned_user } end end + + describe 'before_save' do + describe '#set_last_digits_hash' do + let(:credit_card_validation) { build(:credit_card_validation, last_digits: last_digits) } + + subject(:save_credit_card_validation) { credit_card_validation.save! } + + context 'when last_digits are nil' do + let(:last_digits) { nil } + + it { expect { save_credit_card_validation }.not_to change { credit_card_validation.last_digits_hash } } + end + + context 'when last_digits has a blank value' do + let(:last_digits) { ' ' } + + it { expect { save_credit_card_validation }.not_to change { credit_card_validation.last_digits_hash } } + end + + context 'when last_digits has a value' do + let(:last_digits) { 1111 } + let(:expected_last_digits_hash) { Gitlab::CryptoHelper.sha256(last_digits) } + + it 'assigns correct last_digits_hash value' do + expect { save_credit_card_validation }.to change { + credit_card_validation.last_digits_hash + }.from(nil).to(expected_last_digits_hash) + end + end + end + + describe '#set_holder_name_hash' do + let(:credit_card_validation) { build(:credit_card_validation, holder_name: holder_name) } + + subject(:save_credit_card_validation) { credit_card_validation.save! } + + context 'when holder_name is nil' do + let(:holder_name) { nil } + + it { expect { save_credit_card_validation }.not_to change { credit_card_validation.holder_name_hash } } + end + + context 'when holder_name has a blank value' do + let(:holder_name) { ' ' } + + it { expect { save_credit_card_validation }.not_to change { credit_card_validation.holder_name_hash } } + end + + context 'when holder_name has a value' do + let(:holder_name) { 'John Smith' } + let(:expected_holder_name_hash) { Gitlab::CryptoHelper.sha256(holder_name.downcase) } + + it 'lowercases holder_name and assigns correct holder_name_hash value' do + expect { save_credit_card_validation }.to change { + credit_card_validation.holder_name_hash + }.from(nil).to(expected_holder_name_hash) + end + end + end + + describe '#set_network_hash' do + let(:credit_card_validation) { build(:credit_card_validation, network: network) } + + subject(:save_credit_card_validation) { credit_card_validation.save! } + + context 'when network is nil' do + let(:network) { nil } + + it { expect { save_credit_card_validation }.not_to change { credit_card_validation.network_hash } } + end + + context 'when network has a blank value' do + let(:network) { ' ' } + + it { expect { save_credit_card_validation }.not_to change { credit_card_validation.network_hash } } + end + + context 'when network has a value' do + let(:network) { 'Visa' } + let(:expected_network_hash) { Gitlab::CryptoHelper.sha256(network.downcase) } + + it 'lowercases network and assigns correct network_hash value' do + expect { save_credit_card_validation }.to change { + credit_card_validation.network_hash + }.from(nil).to(expected_network_hash) + end + end + end + + describe '#set_expiration_date_hash' do + let(:credit_card_validation) { build(:credit_card_validation, expiration_date: expiration_date) } + + subject(:save_credit_card_validation) { credit_card_validation.save! } + + context 'when expiration_date is nil' do + let(:expiration_date) { nil } + + it { expect { save_credit_card_validation }.not_to change { credit_card_validation.expiration_date_hash } } + end + + context 'when expiration_date has a blank value' do + let(:expiration_date) { ' ' } + + it { expect { save_credit_card_validation }.not_to change { credit_card_validation.expiration_date_hash } } + end + + context 'when expiration_date has a value' do + let(:expiration_date) { 1.year.from_now.to_date } + let(:expected_expiration_date_hash) { Gitlab::CryptoHelper.sha256(expiration_date.to_s) } + + it 'assigns correct expiration_date_hash value' do + expect { save_credit_card_validation }.to change { + credit_card_validation.expiration_date_hash + }.from(nil).to(expected_expiration_date_hash) + end + end + end + end end diff --git a/spec/models/users/group_visit_spec.rb b/spec/models/users/group_visit_spec.rb new file mode 100644 index 00000000000..63c4631ad7d --- /dev/null +++ b/spec/models/users/group_visit_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::GroupVisit, feature_category: :navigation do + let_it_be(:entity) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:base_time) { DateTime.now } + + before do + described_class.create!(entity_id: entity.id, user_id: user.id, visited_at: base_time) + end + + it_behaves_like 'namespace visits model' + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:model) { create(:group_visit, entity_id: entity.id, user_id: user.id, visited_at: base_time) } + let!(:parent) { entity } + end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:model) { create(:group_visit, entity_id: entity.id, user_id: user.id, visited_at: base_time) } + let!(:parent) { user } + end +end diff --git a/spec/models/users/project_visit_spec.rb b/spec/models/users/project_visit_spec.rb new file mode 100644 index 00000000000..38747bd6462 --- /dev/null +++ b/spec/models/users/project_visit_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::ProjectVisit, feature_category: :navigation do + let_it_be(:entity) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:base_time) { DateTime.now } + + before do + described_class.create!(entity_id: entity.id, user_id: user.id, visited_at: base_time) + end + + it_behaves_like 'namespace visits model' + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:model) { create(:project_visit, entity_id: entity.id, user_id: user.id, visited_at: base_time) } + let!(:parent) { entity } + end + + it_behaves_like 'cleanup by a loose foreign key' do + let!(:model) { create(:project_visit, entity_id: entity.id, user_id: user.id, visited_at: base_time) } + let!(:parent) { user } + end +end diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index 541199e08cb..4b675faf99e 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -165,7 +165,8 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do subject { work_item.supported_quick_action_commands } it 'returns quick action commands supported for all work items' do - is_expected.to include(:title, :reopen, :close, :cc, :tableflip, :shrug, :type, :promote_to) + is_expected.to include(:title, :reopen, :close, :cc, :tableflip, :shrug, :type, :promote_to, :checkin_reminder, + :subscribe, :unsubscribe, :confidential, :award) end context 'when work item supports the assignee widget' do @@ -635,4 +636,82 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do end end end + + describe '#linked_work_items' do + let_it_be(:user) { create(:user) } + let_it_be(:authorized_project) { create(:project) } + let_it_be(:authorized_project2) { create(:project) } + let_it_be(:unauthorized_project) { create(:project, :private) } + + let_it_be(:authorized_item_a) { create(:work_item, project: authorized_project) } + let_it_be(:authorized_item_b) { create(:work_item, project: authorized_project) } + let_it_be(:authorized_item_c) { create(:work_item, project: authorized_project2) } + let_it_be(:unauthorized_item) { create(:work_item, project: unauthorized_project) } + + let_it_be(:work_item_link_a) { create(:work_item_link, source: authorized_item_a, target: authorized_item_b) } + let_it_be(:work_item_link_b) { create(:work_item_link, source: authorized_item_a, target: unauthorized_item) } + let_it_be(:work_item_link_c) { create(:work_item_link, source: authorized_item_a, target: authorized_item_c) } + + before_all do + authorized_project.add_guest(user) + authorized_project2.add_guest(user) + end + + it 'returns only authorized linked work items for given user' do + expect(authorized_item_a.linked_work_items(user)).to contain_exactly(authorized_item_b, authorized_item_c) + end + + it 'returns work items with valid work_item_link_type' do + link_types = authorized_item_a.linked_work_items(user).map(&:issue_link_type) + + expect(link_types).not_to be_empty + expect(link_types).not_to include(nil) + end + + it 'returns work items including the link creation time' do + dates = authorized_item_a.linked_work_items(user).map(&:issue_link_created_at) + + expect(dates).not_to be_empty + expect(dates).not_to include(nil) + end + + it 'returns work items including the link update time' do + dates = authorized_item_a.linked_work_items(user).map(&:issue_link_updated_at) + + expect(dates).not_to be_empty + expect(dates).not_to include(nil) + end + + context 'when a user cannot read cross project' do + it 'only returns work items within the same project' do + allow(Ability).to receive(:allowed?).with(user, :read_all_resources, :global).and_call_original + expect(Ability).to receive(:allowed?).with(user, :read_cross_project).and_return(false) + + expect(authorized_item_a.linked_work_items(user)).to contain_exactly(authorized_item_b) + end + end + + context 'when filtering by link type' do + before do + work_item_link_c.update!(link_type: 'blocks') + end + + it 'returns authorized work items with given link type' do + expect(authorized_item_a.linked_work_items(user, link_type: 'relates_to')).to contain_exactly(authorized_item_b) + end + end + + context 'when authorize option is true and current_user is nil' do + it 'returns empty result' do + expect(authorized_item_a.linked_work_items).to be_empty + end + end + + context 'when authorize option is false' do + it 'returns all work items linked to the work item' do + expect(authorized_item_a.linked_work_items(authorize: false)) + .to contain_exactly(authorized_item_b, authorized_item_c, unauthorized_item) + end + end + 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 349e4c0ba49..3217ac52489 100644 --- a/spec/models/work_items/related_work_item_link_spec.rb +++ b/spec/models/work_items/related_work_item_link_spec.rb @@ -21,6 +21,46 @@ 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 '#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.' } + + before do + create(:work_item_link, source: source, target: target) + stub_const("#{described_class}::MAX_LINKS_COUNT", 1) + end + + specify do + is_expected.to be_invalid + expect(link.errors.messages[error_item]).to include(error_msg) + end + end + + context 'when source exceeds max' do + let(:source) { task1 } + let(:target) { task3 } + let(:error_item) { :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 } + + it_behaves_like 'invalid due to exceeding max number of links' + end + end + end + describe '.issuable_type' do it { expect(described_class.issuable_type).to eq(:issue) } end diff --git a/spec/models/work_items/widgets/description_spec.rb b/spec/models/work_items/widgets/description_spec.rb index c24dc9cfb9c..a089cc32ecd 100644 --- a/spec/models/work_items/widgets/description_spec.rb +++ b/spec/models/work_items/widgets/description_spec.rb @@ -51,7 +51,7 @@ RSpec.describe WorkItems::Widgets::Description do work_item.update!(last_edited_by: nil) end - it { is_expected.to eq(User.ghost) } + it { is_expected.to eq(Users::Internal.ghost) } end end diff --git a/spec/models/work_items/widgets/linked_items_spec.rb b/spec/models/work_items/widgets/linked_items_spec.rb index b4a53b75561..fb3001b286b 100644 --- a/spec/models/work_items/widgets/linked_items_spec.rb +++ b/spec/models/work_items/widgets/linked_items_spec.rb @@ -19,7 +19,7 @@ RSpec.describe WorkItems::Widgets::LinkedItems, feature_category: :portfolio_man it { is_expected.to eq(:linked_items) } end - describe '#related_issues' do - it { expect(described_class.new(work_item).related_issues(user)).to eq(work_item.related_issues(user)) } + describe '#linked_work_items' do + it { expect(described_class.new(work_item).linked_work_items(user)).to eq(work_item.linked_work_items(user)) } end end diff --git a/spec/models/x509_certificate_spec.rb b/spec/models/x509_certificate_spec.rb index 5723bd80739..a550b2caa44 100644 --- a/spec/models/x509_certificate_spec.rb +++ b/spec/models/x509_certificate_spec.rb @@ -5,7 +5,6 @@ require 'spec_helper' RSpec.describe X509Certificate do describe 'validation' do it { is_expected.to validate_presence_of(:subject_key_identifier) } - it { is_expected.to validate_presence_of(:subject) } it { is_expected.to validate_presence_of(:email) } it { is_expected.to validate_presence_of(:serial_number) } it { is_expected.to validate_presence_of(:x509_issuer_id) } diff --git a/spec/models/x509_issuer_spec.rb b/spec/models/x509_issuer_spec.rb index 3d04adf7e26..31470a443a2 100644 --- a/spec/models/x509_issuer_spec.rb +++ b/spec/models/x509_issuer_spec.rb @@ -5,8 +5,6 @@ require 'spec_helper' RSpec.describe X509Issuer do describe 'validation' do it { is_expected.to validate_presence_of(:subject_key_identifier) } - it { is_expected.to validate_presence_of(:subject) } - it { is_expected.to validate_presence_of(:crl_url) } end describe '.safe_create!' do -- cgit v1.2.3