diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-07-19 17:16:28 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-07-19 17:16:28 +0300 |
commit | e4384360a16dd9a19d4d2d25d0ef1f2b862ed2a6 (patch) | |
tree | 2fcdfa7dcdb9db8f5208b2562f4b4e803d671243 /spec/models | |
parent | ffda4e7bcac36987f936b4ba515995a6698698f0 (diff) |
Add latest changes from gitlab-org/gitlab@16-2-stable-eev16.2.0-rc42
Diffstat (limited to 'spec/models')
153 files changed, 2334 insertions, 1737 deletions
diff --git a/spec/models/abuse/trust_score_spec.rb b/spec/models/abuse/trust_score_spec.rb index 755309ac699..85fffcc167b 100644 --- a/spec/models/abuse/trust_score_spec.rb +++ b/spec/models/abuse/trust_score_spec.rb @@ -26,7 +26,6 @@ RSpec.describe Abuse::TrustScore, feature_category: :instance_resiliency do before do allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('123abc') - stub_const('Abuse::TrustScore::MAX_EVENTS', 2) end context 'if correlation ID is nil' do @@ -42,16 +41,5 @@ RSpec.describe Abuse::TrustScore, feature_category: :instance_resiliency do expect(subject.correlation_id_value).to eq('already-set') end end - - context 'if max events is exceeded' do - it 'removes the oldest events' do - first = create(:abuse_trust_score, user: user) - create(:abuse_trust_score, user: user) - create(:abuse_trust_score, user: user) - - expect(user.abuse_trust_scores.count).to eq(2) - expect(described_class.find_by_id(first.id)).to eq(nil) - end - end end end diff --git a/spec/models/abuse/user_trust_score_spec.rb b/spec/models/abuse/user_trust_score_spec.rb new file mode 100644 index 00000000000..6b2cfa6a504 --- /dev/null +++ b/spec/models/abuse/user_trust_score_spec.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Abuse::UserTrustScore, feature_category: :instance_resiliency do + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + let(:user_1_scores) { described_class.new(user1) } + let(:user_2_scores) { described_class.new(user2) } + + describe '#spammer?' do + context 'when the user is a spammer' do + before do + allow(user_1_scores).to receive(:spam_score).and_return(0.9) + end + + it 'classifies the user as a spammer' do + expect(user_1_scores).to be_spammer + end + end + + context 'when the user is not a spammer' do + before do + allow(user_1_scores).to receive(:spam_score).and_return(0.1) + end + + it 'does not classify the user as a spammer' do + expect(user_1_scores).not_to be_spammer + end + end + end + + describe '#spam_score' do + context 'when the user is a spammer' do + before do + create(:abuse_trust_score, user: user1, score: 0.8) + create(:abuse_trust_score, user: user1, score: 0.9) + end + + it 'returns the expected score' do + expect(user_1_scores.spam_score).to be_within(0.01).of(0.85) + end + end + + context 'when the user is not a spammer' do + before do + create(:abuse_trust_score, user: user1, score: 0.1) + create(:abuse_trust_score, user: user1, score: 0.0) + end + + it 'returns the expected score' do + expect(user_1_scores.spam_score).to be_within(0.01).of(0.05) + end + end + end + + describe '#telesign_score' do + context 'when the user has a telesign risk score' do + before do + create(:abuse_trust_score, user: user1, score: 12.0, source: :telesign) + create(:abuse_trust_score, user: user1, score: 24.0, source: :telesign) + end + + it 'returns the latest score' do + expect(user_1_scores.telesign_score).to be(24.0) + end + end + + context 'when the user does not have a telesign risk score' do + it 'defaults to zero' do + expect(user_2_scores.telesign_score).to be(0.0) + end + end + end + + describe '#arkose_global_score' do + context 'when the user has an arkose global risk score' do + before do + create(:abuse_trust_score, user: user1, score: 12.0, source: :arkose_global_score) + create(:abuse_trust_score, user: user1, score: 24.0, source: :arkose_global_score) + end + + it 'returns the latest score' do + expect(user_1_scores.arkose_global_score).to be(24.0) + end + end + + context 'when the user does not have an arkose global risk score' do + it 'defaults to zero' do + expect(user_2_scores.arkose_global_score).to be(0.0) + end + end + end + + describe '#arkose_custom_score' do + context 'when the user has an arkose custom risk score' do + before do + create(:abuse_trust_score, user: user1, score: 12.0, source: :arkose_custom_score) + create(:abuse_trust_score, user: user1, score: 24.0, source: :arkose_custom_score) + end + + it 'returns the latest score' do + expect(user_1_scores.arkose_custom_score).to be(24.0) + end + end + + context 'when the user does not have an arkose custom risk score' do + it 'defaults to zero' do + expect(user_2_scores.arkose_custom_score).to be(0.0) + end + end + end + + describe '#remove_old_scores' do + context 'if max events is exceeded' do + before do + stub_const('Abuse::UserTrustScore::MAX_EVENTS', 2) + end + + it 'removes the oldest events' do + first = create(:abuse_trust_score, user: user1) + create(:abuse_trust_score, user: user1) + create(:abuse_trust_score, user: user1) + + expect(user1.abuse_trust_scores.count).to eq(2) + expect(Abuse::TrustScore.find_by_id(first.id)).to eq(nil) + end + end + end +end diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 8717b2a1075..54169c254a6 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -24,19 +24,19 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do describe '#current?' do it 'returns true if the active session matches the current session' do - active_session = ActiveSession.new(session_private_id: rack_session.private_id) + active_session = described_class.new(session_private_id: rack_session.private_id) expect(active_session.current?(session)).to be true end it 'returns false if the active session does not match the current session' do - active_session = ActiveSession.new(session_id: Rack::Session::SessionId.new('59822c7d9fcdfa03725eff41782ad97d')) + active_session = described_class.new(session_id: Rack::Session::SessionId.new('59822c7d9fcdfa03725eff41782ad97d')) expect(active_session.current?(session)).to be false end it 'returns false if the session id is nil' do - active_session = ActiveSession.new(session_id: nil) + active_session = described_class.new(session_id: nil) session = double(:session, id: nil) expect(active_session.current?(session)).to be false @@ -96,7 +96,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do ) end - expect(ActiveSession.list(user)).to contain_exactly(session) + expect(described_class.list(user)).to contain_exactly(session) Gitlab::Redis::Sessions.with do |redis| expect(redis.sscan_each(lookup_key)).to contain_exactly session_id @@ -119,7 +119,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do end it 'returns an empty array if the user does not have any active session' do - expect(ActiveSession.list(user)).to be_empty + expect(described_class.list(user)).to be_empty end end @@ -138,7 +138,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do ) end - expect(ActiveSession.list_sessions(user)).to eq [{ _csrf_token: 'abcd' }] + expect(described_class.list_sessions(user)).to eq [{ _csrf_token: 'abcd' }] end end @@ -148,7 +148,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do redis.sadd(lookup_key, %w[a b c]) end - expect(ActiveSession.session_ids_for_user(user.id).map(&:to_s)).to match_array(%w[a b c]) + expect(described_class.session_ids_for_user(user.id).map(&:to_s)).to match_array(%w[a b c]) end end @@ -159,13 +159,13 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do redis.set("session:gitlab:#{rack_session.private_id}", Marshal.dump({ _csrf_token: 'abcd' })) end - expect(ActiveSession.sessions_from_ids([rack_session.private_id])).to eq [{ _csrf_token: 'abcd' }] + expect(described_class.sessions_from_ids([rack_session.private_id])).to eq [{ _csrf_token: 'abcd' }] end it 'avoids a redis lookup for an empty array' do expect(Gitlab::Redis::Sessions).not_to receive(:with) - expect(ActiveSession.sessions_from_ids([])).to eq([]) + expect(described_class.sessions_from_ids([])).to eq([]) end it 'uses redis lookup in batches' do @@ -178,13 +178,13 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do mget_responses = sessions.map { |session| [Marshal.dump(session)] } expect(redis).to receive(:mget).twice.times.and_return(*mget_responses) - expect(ActiveSession.sessions_from_ids([1, 2])).to eql(sessions) + expect(described_class.sessions_from_ids([1, 2])).to eql(sessions) end end describe '.set' do it 'sets a new redis entry for the user session and a lookup entry' do - ActiveSession.set(user, request) + described_class.set(user, request) session_id = "2::418729c72310bbf349a032f0bb6e3fce9f5a69df8f000d8ae0ac5d159d8f21ae" @@ -276,10 +276,10 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do end context 'destroy called with Rack::Session::SessionId#private_id' do - subject { ActiveSession.destroy_session(user, rack_session.private_id) } + subject { described_class.destroy_session(user, rack_session.private_id) } it 'calls .destroy_sessions' do - expect(ActiveSession).to( + expect(described_class).to( receive(:destroy_sessions) .with(anything, user, [rack_session.private_id])) @@ -287,7 +287,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do end context 'ActiveSession with session_private_id' do - let(:active_session) { ActiveSession.new(session_private_id: rack_session.private_id) } + let(:active_session) { described_class.new(session_private_id: rack_session.private_id) } let(:active_session_lookup_key) { rack_session.private_id } context 'when using old session key serialization' do @@ -311,7 +311,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do it 'gracefully handles a nil session ID' do expect(described_class).not_to receive(:destroy_sessions) - ActiveSession.destroy_all_but_current(user, nil) + described_class.destroy_all_but_current(user, nil) end shared_examples 'with user sessions' do @@ -338,15 +338,15 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do end it 'removes the entry associated with the all user sessions but current' do - expect { ActiveSession.destroy_all_but_current(user, request.session) } + expect { described_class.destroy_all_but_current(user, request.session) } .to(change { ActiveSession.session_ids_for_user(user.id).size }.from(2).to(1)) - expect(ActiveSession.session_ids_for_user(9999).size).to eq(1) + expect(described_class.session_ids_for_user(9999).size).to eq(1) end it 'removes the lookup entry of deleted sessions' do session_private_id = Rack::Session::SessionId.new(current_session_id).private_id - ActiveSession.destroy_all_but_current(user, request.session) + described_class.destroy_all_but_current(user, request.session) Gitlab::Redis::Sessions.with do |redis| expect(redis.smembers(lookup_key)).to contain_exactly session_private_id @@ -361,9 +361,9 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do redis.sadd?(lookup_key, impersonated_session_id) end - expect { ActiveSession.destroy_all_but_current(user, request.session) }.to change { ActiveSession.session_ids_for_user(user.id).size }.from(3).to(2) + expect { described_class.destroy_all_but_current(user, request.session) }.to change { ActiveSession.session_ids_for_user(user.id).size }.from(3).to(2) - expect(ActiveSession.session_ids_for_user(9999).size).to eq(1) + expect(described_class.session_ids_for_user(9999).size).to eq(1) end end @@ -407,7 +407,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do redis.sadd(lookup_key, [current_session_id, '59822c7d9fcdfa03725eff41782ad97d']) end - ActiveSession.cleanup(user) + described_class.cleanup(user) Gitlab::Redis::Sessions.with do |redis| expect(redis.smembers(lookup_key)).to contain_exactly current_session_id @@ -416,7 +416,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do end it 'does not bail if there are no lookup entries' do - ActiveSession.cleanup(user) + described_class.cleanup(user) end context 'cleaning up old sessions' do @@ -436,7 +436,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do end it 'removes obsolete active sessions entries' do - ActiveSession.cleanup(user) + described_class.cleanup(user) Gitlab::Redis::Sessions.with do |redis| sessions = described_class.list(user) @@ -450,7 +450,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do end it 'removes obsolete lookup entries' do - ActiveSession.cleanup(user) + described_class.cleanup(user) Gitlab::Redis::Sessions.with do |redis| lookup_entries = redis.smembers(lookup_key) @@ -465,7 +465,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do redis.sadd?(lookup_key, (max_number_of_sessions_plus_two + 1).to_s) end - ActiveSession.cleanup(user) + described_class.cleanup(user) Gitlab::Redis::Sessions.with do |redis| lookup_entries = redis.smembers(lookup_key) @@ -654,7 +654,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do let(:auth) { double(cookies: {}) } it 'sets marketing cookie' do - ActiveSession.set_active_user_cookie(auth) + described_class.set_active_user_cookie(auth) expect(auth.cookies[:about_gitlab_active_user][:value]).to be_truthy end end @@ -663,11 +663,11 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_sessions do let(:auth) { double(cookies: {}) } before do - ActiveSession.set_active_user_cookie(auth) + described_class.set_active_user_cookie(auth) end it 'unsets marketing cookie' do - ActiveSession.unset_active_user_cookie(auth) + described_class.unset_active_user_cookie(auth) expect(auth.cookies[:about_gitlab_active_user]).to be_nil end end diff --git a/spec/models/ai/service_access_token_spec.rb b/spec/models/ai/service_access_token_spec.rb new file mode 100644 index 00000000000..12ed24f3bd6 --- /dev/null +++ b/spec/models/ai/service_access_token_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ai::ServiceAccessToken, type: :model, feature_category: :application_performance do + describe '.expired', :freeze_time do + let_it_be(:expired_token) { create(:service_access_token, :code_suggestions, :expired) } + let_it_be(:active_token) { create(:service_access_token, :code_suggestions, :active) } + + it 'selects all expired tokens' do + expect(described_class.expired).to match_array([expired_token]) + end + end + + # There is currently only one category, please expand this test when a new category is added. + describe '.for_category' do + let(:code_suggestions_token) { create(:service_access_token, :code_suggestions) } + let(:category) { :code_suggestions } + + it 'only selects tokens from the selected category' do + expect(described_class.for_category(category)).to match_array([code_suggestions_token]) + end + end + + describe '#token' do + let(:token_value) { 'Abc' } + + it 'is encrypted' do + subject.token = token_value + + aggregate_failures do + expect(subject.encrypted_token_iv).to be_present + expect(subject.encrypted_token).to be_present + expect(subject.encrypted_token).not_to eq(token_value) + expect(subject.token).to eq(token_value) + end + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:token) } + it { is_expected.to validate_presence_of(:category) } + it { is_expected.to validate_presence_of(:expires_at) } + end + end +end diff --git a/spec/models/alert_management/alert_spec.rb b/spec/models/alert_management/alert_spec.rb index ff77ca2ab64..ed5cde3ca3b 100644 --- a/spec/models/alert_management/alert_spec.rb +++ b/spec/models/alert_management/alert_spec.rb @@ -2,13 +2,13 @@ require 'spec_helper' -RSpec.describe AlertManagement::Alert do +RSpec.describe AlertManagement::Alert, feature_category: :incident_management do let_it_be(:project) { create(:project) } let_it_be(:project2) { create(:project) } - let_it_be(:triggered_alert, reload: true) { create(:alert_management_alert, :triggered, project: project) } - let_it_be(:acknowledged_alert, reload: true) { create(:alert_management_alert, :acknowledged, project: project) } - let_it_be(:resolved_alert, reload: true) { create(:alert_management_alert, :resolved, project: project2) } - let_it_be(:ignored_alert, reload: true) { create(:alert_management_alert, :ignored, project: project2) } + let_it_be_with_refind(:triggered_alert) { create(:alert_management_alert, :triggered, project: project) } + let_it_be_with_refind(:acknowledged_alert) { create(:alert_management_alert, :acknowledged, project: project) } + let_it_be_with_refind(:resolved_alert) { create(:alert_management_alert, :resolved, project: project2) } + let_it_be_with_refind(:ignored_alert) { create(:alert_management_alert, :ignored, project: project2) } describe 'associations' do it { is_expected.to belong_to(:project) } @@ -168,7 +168,7 @@ RSpec.describe AlertManagement::Alert do let_it_be(:alert) { triggered_alert } let_it_be(:assignee) { create(:user) } - subject { AlertManagement::Alert.for_assignee_username(assignee_username) } + subject { described_class.for_assignee_username(assignee_username) } before_all do alert.update!(assignees: [assignee]) @@ -269,7 +269,7 @@ RSpec.describe AlertManagement::Alert do alert.update!(title: 'Title', description: 'Desc', service: 'Service', monitoring_tool: 'Monitor') end - subject { AlertManagement::Alert.search(query) } + subject { described_class.search(query) } context 'does not contain search string' do let(:query) { 'something else' } diff --git a/spec/models/alert_management/http_integration_spec.rb b/spec/models/alert_management/http_integration_spec.rb index 606b53aeacd..479ae8a4966 100644 --- a/spec/models/alert_management/http_integration_spec.rb +++ b/spec/models/alert_management/http_integration_spec.rb @@ -29,13 +29,13 @@ RSpec.describe AlertManagement::HttpIntegration, feature_category: :incident_man # Uniqueness spec saves integration with `validate: false` otherwise. subject { create(:alert_management_http_integration, :legacy) } - it { is_expected.to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id, :active) } + it { is_expected.to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id) } end context 'when inactive' do subject { create(:alert_management_http_integration, :legacy, :inactive) } - it { is_expected.not_to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id, :active) } + it { is_expected.to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id) } end context 'payload_attribute_mapping' do @@ -90,7 +90,7 @@ RSpec.describe AlertManagement::HttpIntegration, feature_category: :incident_man describe 'scopes' do let_it_be(:integration_1) { create(:alert_management_http_integration) } let_it_be(:integration_2) { create(:alert_management_http_integration, :inactive, project: project) } - let_it_be(:integration_3) { create(:alert_management_http_integration, :prometheus, project: project) } + let_it_be(:integration_3) { create(:alert_management_prometheus_integration, project: project) } let_it_be(:integration_4) { create(:alert_management_http_integration, :legacy, :inactive) } describe '.for_endpoint_identifier' do @@ -129,12 +129,6 @@ RSpec.describe AlertManagement::HttpIntegration, feature_category: :incident_man it { is_expected.to contain_exactly(integration_1, integration_3) } end - describe '.legacy' do - subject { described_class.legacy } - - it { is_expected.to contain_exactly(integration_4) } - end - describe '.ordered_by_type_and_id' do before do # Rearrange cache by saving to avoid false-positives @@ -232,9 +226,16 @@ RSpec.describe AlertManagement::HttpIntegration, feature_category: :incident_man end context 'when included in initialization args' do - let(:integration) { described_class.new(endpoint_identifier: 'legacy') } + let(:required_args) { { project: project, name: 'Name' } } - it { is_expected.to eq('legacy') } + AlertManagement::HttpIntegration::LEGACY_IDENTIFIERS.each do |identifier| + context "for endpoint identifier \"#{identifier}\"" do + let(:integration) { described_class.new(**required_args, endpoint_identifier: identifier) } + + it { is_expected.to eq(identifier) } + specify { expect(integration).to be_valid } + end + end end context 'when reassigning' do @@ -293,7 +294,7 @@ RSpec.describe AlertManagement::HttpIntegration, feature_category: :incident_man end context 'for a prometheus integration' do - let(:integration) { build(:alert_management_http_integration, :prometheus) } + let(:integration) { build(:alert_management_prometheus_integration) } it do is_expected.to eq( @@ -307,7 +308,7 @@ RSpec.describe AlertManagement::HttpIntegration, feature_category: :incident_man end context 'for a legacy integration' do - let(:integration) { build(:alert_management_http_integration, :prometheus, :legacy) } + let(:integration) { build(:alert_management_prometheus_integration, :legacy) } it do is_expected.to eq( diff --git a/spec/models/analytics/cycle_analytics/stage_spec.rb b/spec/models/analytics/cycle_analytics/stage_spec.rb index 960d8d3e964..54ae0feca2c 100644 --- a/spec/models/analytics/cycle_analytics/stage_spec.rb +++ b/spec/models/analytics/cycle_analytics/stage_spec.rb @@ -3,10 +3,23 @@ require 'spec_helper' RSpec.describe Analytics::CycleAnalytics::Stage, feature_category: :value_stream_management do - describe 'uniqueness validation on name' do + describe 'validations' do subject { build(:cycle_analytics_stage) } it { is_expected.to validate_uniqueness_of(:name).scoped_to([:group_id, :group_value_stream_id]) } + + it 'validates count of stages per value stream' do + stub_const("#{described_class.name}::MAX_STAGES_PER_VALUE_STREAM", 1) + value_stream = create(:cycle_analytics_value_stream, name: 'test') + create(:cycle_analytics_stage, name: "stage 1", value_stream: value_stream) + + new_stage = build(:cycle_analytics_stage, name: "stage 2", value_stream: value_stream) + + expect do + new_stage.save! + end.to raise_error(ActiveRecord::RecordInvalid, + _('Validation failed: Value stream Maximum number of stages per value stream exceeded')) + end end describe 'associations' do diff --git a/spec/models/analytics/cycle_analytics/value_stream_spec.rb b/spec/models/analytics/cycle_analytics/value_stream_spec.rb index f290cf25ae6..3b3187e0b51 100644 --- a/spec/models/analytics/cycle_analytics/value_stream_spec.rb +++ b/spec/models/analytics/cycle_analytics/value_stream_spec.rb @@ -25,6 +25,19 @@ RSpec.describe Analytics::CycleAnalytics::ValueStream, type: :model, feature_cat it_behaves_like 'value stream analytics namespace models' do let(:factory_name) { :cycle_analytics_value_stream } end + + it 'validates count of value streams per namespace' do + stub_const("#{described_class.name}::MAX_VALUE_STREAMS_PER_NAMESPACE", 1) + group = create(:group) + create(:cycle_analytics_value_stream, name: 'test', namespace: group) + + new_value_stream = build(:cycle_analytics_value_stream, name: 'test2', namespace: group) + + expect do + new_value_stream.save! + end.to raise_error(ActiveRecord::RecordInvalid, + _('Validation failed: Namespace Maximum number of value streams per namespace exceeded')) + end end describe 'scopes' do diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index ee3065cf8f2..9ea5c1ec92c 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -257,7 +257,7 @@ RSpec.describe ApplicationRecord do end before do - ApplicationRecord.connection.execute(<<~SQL) + described_class.connection.execute(<<~SQL) create table _test_tests ( id bigserial primary key not null, ignore_me text diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 12ab061fa03..8dcafaa90a0 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -59,8 +59,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to allow_value("dev.gitlab.com").for(:commit_email_hostname) } it { is_expected.not_to allow_value("@dev.gitlab").for(:commit_email_hostname) } - it { is_expected.to allow_value(true).for(:container_expiration_policies_enable_historic_entries) } - it { is_expected.to allow_value(false).for(:container_expiration_policies_enable_historic_entries) } + it { is_expected.to allow_value(true, false).for(:container_expiration_policies_enable_historic_entries) } it { is_expected.not_to allow_value(nil).for(:container_expiration_policies_enable_historic_entries) } it { is_expected.to allow_value("myemail@gitlab.com").for(:lets_encrypt_notification_email) } @@ -100,8 +99,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_numericality_of(:container_registry_cleanup_tags_service_max_list_size).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_data_repair_detail_worker_max_concurrency).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_expiration_policies_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to allow_value(true).for(:container_registry_expiration_policies_caching) } - it { is_expected.to allow_value(false).for(:container_registry_expiration_policies_caching) } + it { is_expected.to allow_value(true, false).for(:container_registry_expiration_policies_caching) } it { is_expected.to validate_numericality_of(:container_registry_import_max_tags_count).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_import_max_retries).only_integer.is_greater_than_or_equal_to(0) } @@ -134,8 +132,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_numericality_of(:snippet_size_limit).only_integer.is_greater_than(0) } it { is_expected.to validate_numericality_of(:wiki_page_max_content_bytes).only_integer.is_greater_than_or_equal_to(1024) } - it { is_expected.to allow_value(true).for(:wiki_asciidoc_allow_uri_includes) } - it { is_expected.to allow_value(false).for(:wiki_asciidoc_allow_uri_includes) } + it { is_expected.to allow_value(true, false).for(:wiki_asciidoc_allow_uri_includes) } it { is_expected.not_to allow_value(nil).for(:wiki_asciidoc_allow_uri_includes) } it { is_expected.to validate_presence_of(:max_artifacts_size) } it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } @@ -148,8 +145,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to validate_presence_of(:max_terraform_state_size_bytes) } it { is_expected.to validate_numericality_of(:max_terraform_state_size_bytes).only_integer.is_greater_than_or_equal_to(0) } - it { is_expected.to allow_value(true).for(:user_defaults_to_private_profile) } - it { is_expected.to allow_value(false).for(:user_defaults_to_private_profile) } + it { is_expected.to allow_value(true, false).for(:user_defaults_to_private_profile) } it { is_expected.not_to allow_value(nil).for(:user_defaults_to_private_profile) } it { is_expected.to allow_values([true, false]).for(:deny_all_requests_except_allowed) } @@ -250,16 +246,13 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it { is_expected.to allow_value(http).for(:jira_connect_proxy_url) } it { is_expected.to allow_value(https).for(:jira_connect_proxy_url) } - it { is_expected.to allow_value(true).for(:bulk_import_enabled) } - it { is_expected.to allow_value(false).for(:bulk_import_enabled) } + it { is_expected.to allow_value(true, false).for(:bulk_import_enabled) } it { is_expected.not_to allow_value(nil).for(:bulk_import_enabled) } - it { is_expected.to allow_value(true).for(:allow_runner_registration_token) } - it { is_expected.to allow_value(false).for(:allow_runner_registration_token) } + it { is_expected.to allow_value(true, false).for(:allow_runner_registration_token) } it { is_expected.not_to allow_value(nil).for(:allow_runner_registration_token) } - it { is_expected.to allow_value(true).for(:gitlab_dedicated_instance) } - it { is_expected.to allow_value(false).for(:gitlab_dedicated_instance) } + it { is_expected.to allow_value(true, false).for(:gitlab_dedicated_instance) } it { is_expected.not_to allow_value(nil).for(:gitlab_dedicated_instance) } it { is_expected.not_to allow_value(random: :value).for(:database_apdex_settings) } @@ -493,6 +486,37 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do end end + describe 'GitLab for Slack app settings' do + before do + setting.slack_app_enabled = slack_app_enabled + end + + context 'when GitLab for Slack app is disabled' do + let(:slack_app_enabled) { false } + + it { is_expected.to allow_value(nil).for(:slack_app_id) } + it { is_expected.to allow_value(nil).for(:slack_app_secret) } + it { is_expected.to allow_value(nil).for(:slack_app_signing_secret) } + it { is_expected.to allow_value(nil).for(:slack_app_verification_token) } + end + + context 'when GitLab for Slack app is enabled' do + let(:slack_app_enabled) { true } + + it { is_expected.to allow_value('123456789a').for(:slack_app_id) } + it { is_expected.not_to allow_value(nil).for(:slack_app_id) } + + it { is_expected.to allow_value('secret').for(:slack_app_secret) } + it { is_expected.not_to allow_value(nil).for(:slack_app_secret) } + + it { is_expected.to allow_value('signing-secret').for(:slack_app_signing_secret) } + it { is_expected.not_to allow_value(nil).for(:slack_app_signing_secret) } + + it { is_expected.to allow_value('token').for(:slack_app_verification_token) } + it { is_expected.not_to allow_value(nil).for(:slack_app_verification_token) } + end + end + describe 'default_artifacts_expire_in' do it 'sets an error if it cannot parse' do expect do @@ -1569,7 +1593,7 @@ RSpec.describe ApplicationSetting, feature_category: :shared, type: :model do it 'ignores the plaintext token' do subject - ApplicationSetting.update_all(static_objects_external_storage_auth_token: 'Test') + described_class.update_all(static_objects_external_storage_auth_token: 'Test') setting.reload expect(setting[:static_objects_external_storage_auth_token]).to be_nil diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index 99006f8ddce..586ec8f723a 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -141,36 +141,51 @@ RSpec.describe AwardEmoji do end end - describe 'expiring ETag cache' 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) } - it 'calls expire_etag_cache on the note when saved' do + it 'broadcasts updates on the note when saved' do expect(note).to receive(:expire_etag_cache) + expect(note).to receive(:trigger_note_subscription_update) award_emoji.save! end - it 'calls expire_etag_cache on the note when destroyed' do + it 'broadcasts updates on the note when destroyed' do expect(note).to receive(:expire_etag_cache) + 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) } + + it 'does not broadcast updates on the note when saved' do + expect(note).not_to receive(:expire_etag_cache) + expect(note).not_to receive(:trigger_note_subscription_update) + + award_emoji.save! + end + end end context 'on another awardable' do let(:issue) { create(:issue) } let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: issue) } - it 'does not call expire_etag_cache on the issue when saved' do + it 'does not broadcast updates on the issue when saved' do expect(issue).not_to receive(:expire_etag_cache) + expect(issue).not_to receive(:trigger_note_subscription_update) award_emoji.save! end - it 'does not call expire_etag_cache on the issue when destroyed' do + it 'does not broadcast updates on the issue when destroyed' do expect(issue).not_to receive(:expire_etag_cache) + expect(issue).not_to receive(:trigger_note_subscription_update) award_emoji.destroy! end diff --git a/spec/models/bulk_import_spec.rb b/spec/models/bulk_import_spec.rb index acb1f4a2ef7..a50fc6eaba4 100644 --- a/spec/models/bulk_import_spec.rb +++ b/spec/models/bulk_import_spec.rb @@ -75,4 +75,22 @@ RSpec.describe BulkImport, type: :model, feature_category: :importers do end end end + + describe '#supports_batched_export?' do + context 'when source version is greater than min supported version for batched migrations' do + it 'returns true' do + bulk_import = build(:bulk_import, source_version: '16.2.0') + + expect(bulk_import.supports_batched_export?).to eq(true) + end + end + + context 'when source version is less than min supported version for batched migrations' do + it 'returns false' do + bulk_import = build(:bulk_import, source_version: '15.5.0') + + expect(bulk_import.supports_batched_export?).to eq(false) + end + end + end end diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index c7ace3d2b78..7179ed7cb42 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -255,6 +255,27 @@ RSpec.describe BulkImports::Entity, type: :model, feature_category: :importers d expect(entity.export_relations_url_path).to eq("/projects/#{entity.source_xid}/export_relations") end end + + context 'when batched' do + context 'when source supports batched export' do + it 'returns batched export relations url' do + import = build(:bulk_import, source_version: '16.2.0') + entity = build(:bulk_import_entity, :project_entity, bulk_import: import) + + expect(entity.export_relations_url_path(batched: true)) + .to eq("/projects/#{entity.source_xid}/export_relations?batched=true") + end + end + + context 'when source does not support batched export' do + it 'returns export relations url' do + entity = build(:bulk_import_entity) + + expect(entity.export_relations_url_path(batched: true)) + .to eq("/groups/#{entity.source_xid}/export_relations") + end + end + end end describe '#relation_download_url_path' do @@ -264,6 +285,27 @@ RSpec.describe BulkImports::Entity, type: :model, feature_category: :importers d expect(entity.relation_download_url_path('test')) .to eq("/groups/#{entity.source_xid}/export_relations/download?relation=test") end + + context 'when batch number is present' do + context 'when source supports batched export' do + it 'returns export relations url with download query string and batch number' do + import = build(:bulk_import, source_version: '16.2.0') + entity = build(:bulk_import_entity, :project_entity, bulk_import: import) + + expect(entity.relation_download_url_path('test', 1)) + .to eq("/projects/#{entity.source_xid}/export_relations/download?batch_number=1&batched=true&relation=test") + end + end + + context 'when source does not support batched export' do + it 'returns export relations url' do + entity = build(:bulk_import_entity) + + expect(entity.relation_download_url_path('test', 1)) + .to eq("/groups/#{entity.source_xid}/export_relations/download?relation=test") + end + end + end end describe '#entity_type' do diff --git a/spec/models/bulk_imports/export_spec.rb b/spec/models/bulk_imports/export_spec.rb index 7173d032bc2..e5c0632b113 100644 --- a/spec/models/bulk_imports/export_spec.rb +++ b/spec/models/bulk_imports/export_spec.rb @@ -83,4 +83,29 @@ RSpec.describe BulkImports::Export, type: :model, feature_category: :importers d end end end + + describe '#remove_existing_upload!' do + context 'when upload exists' do + it 'removes the upload' do + export = create(:bulk_import_export) + upload = create(:bulk_import_export_upload, export: export) + upload.update!(export_file: fixture_file_upload('spec/fixtures/bulk_imports/gz/labels.ndjson.gz')) + + expect_any_instance_of(BulkImports::ExportUpload) do |upload| + expect(upload).to receive(:remove_export_file!) + expect(upload).to receive(:save!) + end + + export.remove_existing_upload! + end + end + + context 'when upload does not exist' do + it 'returns' do + export = build(:bulk_import_export) + + expect { export.remove_existing_upload! }.not_to change { export.upload } + end + end + end end diff --git a/spec/models/bulk_imports/export_status_spec.rb b/spec/models/bulk_imports/export_status_spec.rb index 0921c3bdce2..c3faa2db19c 100644 --- a/spec/models/bulk_imports/export_status_spec.rb +++ b/spec/models/bulk_imports/export_status_spec.rb @@ -2,16 +2,28 @@ require 'spec_helper' -RSpec.describe BulkImports::ExportStatus do +RSpec.describe BulkImports::ExportStatus, feature_category: :importers do let_it_be(:relation) { 'labels' } let_it_be(:import) { create(:bulk_import) } let_it_be(:config) { create(:bulk_import_configuration, bulk_import: import) } let_it_be(:entity) { create(:bulk_import_entity, bulk_import: import, source_full_path: 'foo') } let_it_be(:tracker) { create(:bulk_import_tracker, entity: entity) } + let(:batched) { false } + let(:batches) { [] } let(:response_double) do instance_double(HTTParty::Response, - parsed_response: [{ 'relation' => 'labels', 'status' => status, 'error' => 'error!' }] + parsed_response: [ + { + 'relation' => 'labels', + 'status' => status, + 'error' => 'error!', + 'batched' => batched, + 'batches' => batches, + 'batches_count' => 1, + 'total_objects_count' => 1 + } + ] ) end @@ -190,4 +202,84 @@ RSpec.describe BulkImports::ExportStatus do end end end + + describe 'batching information' do + let(:status) { BulkImports::Export::FINISHED } + + describe '#batched?' do + context 'when export is batched' do + let(:batched) { true } + + it 'returns true' do + expect(subject.batched?).to eq(true) + end + end + + context 'when export is not batched' do + it 'returns false' do + expect(subject.batched?).to eq(false) + end + end + + context 'when export batch information is missing' do + let(:response_double) do + instance_double(HTTParty::Response, parsed_response: [{ 'relation' => 'labels', 'status' => status }]) + end + + it 'returns false' do + expect(subject.batched?).to eq(false) + end + end + end + + describe '#batches_count' do + context 'when batches count is present' do + it 'returns batches count' do + expect(subject.batches_count).to eq(1) + end + end + + context 'when batches count is missing' do + let(:response_double) do + instance_double(HTTParty::Response, parsed_response: [{ 'relation' => 'labels', 'status' => status }]) + end + + it 'returns 0' do + expect(subject.batches_count).to eq(0) + end + end + end + + describe '#batch' do + context 'when export is batched' do + let(:batched) { true } + let(:batches) do + [ + { 'relation' => 'labels', 'status' => status, 'batch_number' => 1 }, + { 'relation' => 'milestones', 'status' => status, 'batch_number' => 2 } + ] + end + + context 'when batch number is in range' do + it 'returns batch information' do + expect(subject.batch(1)['relation']).to eq('labels') + expect(subject.batch(2)['relation']).to eq('milestones') + expect(subject.batch(3)).to eq(nil) + end + end + end + + context 'when batch number is less than 1' do + it 'raises error' do + expect { subject.batch(0) }.to raise_error(ArgumentError) + end + end + + context 'when export is not batched' do + it 'returns nil' do + expect(subject.batch(1)).to eq(nil) + end + end + end + end end diff --git a/spec/models/bulk_imports/file_transfer/group_config_spec.rb b/spec/models/bulk_imports/file_transfer/group_config_spec.rb index e50f52c728f..9e1e7cf6d6e 100644 --- a/spec/models/bulk_imports/file_transfer/group_config_spec.rb +++ b/spec/models/bulk_imports/file_transfer/group_config_spec.rb @@ -40,7 +40,12 @@ RSpec.describe BulkImports::FileTransfer::GroupConfig, feature_category: :import describe '#top_relation_tree' do it 'returns relation tree of a top level relation' do - expect(subject.top_relation_tree('labels')).to eq('priorities' => {}) + expect(subject.top_relation_tree('boards')).to include( + 'lists' => a_hash_including({ + 'board' => anything, + 'label' => anything + }) + ) end end diff --git a/spec/models/ci/artifact_blob_spec.rb b/spec/models/ci/artifact_blob_spec.rb index c00f46683b9..2e0b13c35ea 100644 --- a/spec/models/ci/artifact_blob_spec.rb +++ b/spec/models/ci/artifact_blob_spec.rb @@ -2,105 +2,93 @@ require 'spec_helper' -RSpec.describe Ci::ArtifactBlob do - let_it_be(:project) { create(:project, :public) } +RSpec.describe Ci::ArtifactBlob, feature_category: :continuous_integration do + let_it_be(:project) { create(:project, :public, path: 'project1') } let_it_be(:build) { create(:ci_build, :artifacts, project: project) } + let(:pages_port) { nil } let(:entry) { build.artifacts_metadata_entry('other_artifacts_0.1.2/another-subdirectory/banana_sample.gif') } - subject { described_class.new(entry) } + subject(:blob) { described_class.new(entry) } + + before do + stub_pages_setting( + enabled: true, + artifacts_server: true, + access_control: true, + port: pages_port + ) + end describe '#id' do it 'returns a hash of the path' do - expect(subject.id).to eq(Digest::SHA1.hexdigest(entry.path)) + expect(blob.id).to eq(Digest::SHA1.hexdigest(entry.path)) end end describe '#name' do it 'returns the entry name' do - expect(subject.name).to eq(entry.name) + expect(blob.name).to eq(entry.name) end end describe '#path' do it 'returns the entry path' do - expect(subject.path).to eq(entry.path) + expect(blob.path).to eq(entry.path) end end describe '#size' do it 'returns the entry size' do - expect(subject.size).to eq(entry.metadata[:size]) + expect(blob.size).to eq(entry.metadata[:size]) end end describe '#mode' do it 'returns the entry mode' do - expect(subject.mode).to eq(entry.metadata[:mode]) + expect(blob.mode).to eq(entry.metadata[:mode]) end end describe '#external_storage' do it 'returns :build_artifact' do - expect(subject.external_storage).to eq(:build_artifact) + expect(blob.external_storage).to eq(:build_artifact) end end describe '#external_url' do - before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) - allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true) - end + subject(:url) { blob.external_url(build) } - describe '.gif extension' do - it 'returns nil' do - expect(subject.external_url(build.project, build)).to be_nil - end + context 'with not allowed extension' do + it { is_expected.to be_nil } end - context 'txt extensions' do + context 'with allowed extension' do let(:path) { 'other_artifacts_0.1.2/doc_sample.txt' } let(:entry) { build.artifacts_metadata_entry(path) } - it 'returns a URL' do - url = subject.external_url(build.project, build) - - expect(url).not_to be_nil - expect(url).to eq("http://#{project.namespace.path}.#{Gitlab.config.pages.host}/-/#{project.path}/-/jobs/#{build.id}/artifacts/#{path}") - end + it { is_expected.to eq("http://#{project.namespace.path}.example.com/-/project1/-/jobs/#{build.id}/artifacts/other_artifacts_0.1.2/doc_sample.txt") } context 'when port is configured' do - let(:port) { 1234 } - - it 'returns an URL with port number' do - allow(Gitlab.config.pages).to receive(:url).and_return("#{Gitlab.config.pages.url}:#{port}") - - url = subject.external_url(build.project, build) + let(:pages_port) { 1234 } - expect(url).not_to be_nil - expect(url).to eq("http://#{project.namespace.path}.#{Gitlab.config.pages.host}:#{port}/-/#{project.path}/-/jobs/#{build.id}/artifacts/#{path}") - end + it { is_expected.to eq("http://#{project.namespace.path}.example.com:1234/-/project1/-/jobs/#{build.id}/artifacts/other_artifacts_0.1.2/doc_sample.txt") } end end end describe '#external_link?' do - before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) - allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true) - end - - context 'gif extensions' do + context 'with not allowed extensions' do it 'returns false' do - expect(subject.external_link?(build)).to be false + expect(blob.external_link?(build)).to be false end end - context 'txt extensions' do + context 'with allowed extensions' do let(:entry) { build.artifacts_metadata_entry('other_artifacts_0.1.2/doc_sample.txt') } it 'returns true' do - expect(subject.external_link?(build)).to be true + expect(blob.external_link?(build)).to be true end end end diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index ac994735928..d93250af177 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -378,6 +378,91 @@ RSpec.describe Ci::Bridge, feature_category: :continuous_integration do end end + describe '#variables' do + it 'returns bridge scoped variables and pipeline persisted variables' do + expect(bridge.variables.to_hash) + .to eq(bridge.scoped_variables.concat(bridge.pipeline.persisted_variables).to_hash) + end + end + + describe '#pipeline_variables' do + it 'returns the pipeline variables' do + expect(bridge.pipeline_variables).to eq(bridge.pipeline.variables) + end + end + + describe '#pipeline_schedule_variables' do + context 'when pipeline is on a schedule' do + let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) } + let(:pipeline) { create(:ci_pipeline, pipeline_schedule: pipeline_schedule) } + + it 'returns the pipeline schedule variables' do + create(:ci_pipeline_schedule_variable, key: 'FOO', value: 'foo', pipeline_schedule: pipeline.pipeline_schedule) + + pipeline_schedule_variables = bridge.reload.pipeline_schedule_variables + expect(pipeline_schedule_variables).to match_array([have_attributes({ key: 'FOO', value: 'foo' })]) + end + end + + context 'when pipeline is not on a schedule' do + it 'returns empty array' do + expect(bridge.pipeline_schedule_variables).to eq([]) + end + end + end + + describe '#forward_yaml_variables?' do + using RSpec::Parameterized::TableSyntax + + where(:forward, :result) do + true | true + false | false + nil | true + end + + with_them do + let(:options) do + { + trigger: { + project: 'my/project', + branch: 'master', + forward: { yaml_variables: forward }.compact + } + } + end + + let(:bridge) { build(:ci_bridge, options: options) } + + it { expect(bridge.forward_yaml_variables?).to eq(result) } + end + end + + describe '#forward_pipeline_variables?' do + using RSpec::Parameterized::TableSyntax + + where(:forward, :result) do + true | true + false | false + nil | false + end + + with_them do + let(:options) do + { + trigger: { + project: 'my/project', + branch: 'master', + forward: { pipeline_variables: forward }.compact + } + } + end + + let(:bridge) { build(:ci_bridge, options: options) } + + it { expect(bridge.forward_pipeline_variables?).to eq(result) } + end + end + describe 'metadata support' do it 'reads YAML variables from metadata' do expect(bridge.yaml_variables).not_to be_empty diff --git a/spec/models/ci/build_dependencies_spec.rb b/spec/models/ci/build_dependencies_spec.rb index 0709aa47ff1..ab32234eba3 100644 --- a/spec/models/ci/build_dependencies_spec.rb +++ b/spec/models/ci/build_dependencies_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::BuildDependencies do +RSpec.describe Ci::BuildDependencies, feature_category: :continuous_integration do let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project, :repository) } diff --git a/spec/models/ci/build_metadata_spec.rb b/spec/models/ci/build_metadata_spec.rb index 8ed0e50e4b0..bd21de1f05a 100644 --- a/spec/models/ci/build_metadata_spec.rb +++ b/spec/models/ci/build_metadata_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::BuildMetadata do +RSpec.describe Ci::BuildMetadata, feature_category: :continuous_integration do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, group: group, build_timeout: 2000) } diff --git a/spec/models/ci/build_report_result_spec.rb b/spec/models/ci/build_report_result_spec.rb index 90426f60c73..8f6c95053c9 100644 --- a/spec/models/ci/build_report_result_spec.rb +++ b/spec/models/ci/build_report_result_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::BuildReportResult do +RSpec.describe Ci::BuildReportResult, feature_category: :continuous_integration do let_it_be_with_reload(:build_report_result) { create(:ci_build_report_result, :with_junit_success) } it_behaves_like 'cleanup by a loose foreign key' do diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb index 002aff25593..dac7edbe6cc 100644 --- a/spec/models/ci/build_runner_session_spec.rb +++ b/spec/models/ci/build_runner_session_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Ci::BuildRunnerSession, model: true, feature_category: :continuou session = build_with_local_runner_session_url.reload.runner_session expect(session.errors).to be_empty - expect(session).to be_a(Ci::BuildRunnerSession) + expect(session).to be_a(described_class) expect(session.url).to eq(url) end end @@ -59,7 +59,7 @@ RSpec.describe Ci::BuildRunnerSession, model: true, feature_category: :continuou simple_build.save! session = simple_build.reload.runner_session - expect(session).to be_a(Ci::BuildRunnerSession) + expect(session).to be_a(described_class) expect(session.url).to eq(url) end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 51cd6efb85f..b7f457962a0 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2282,7 +2282,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def describe '.keep_artifacts!' do let!(:build) { create(:ci_build, artifacts_expire_at: Time.current + 7.days, pipeline: pipeline) } let!(:builds_for_update) do - Ci::Build.where(id: create_list(:ci_build, 3, artifacts_expire_at: Time.current + 7.days, pipeline: pipeline).map(&:id)) + described_class.where(id: create_list(:ci_build, 3, artifacts_expire_at: Time.current + 7.days, pipeline: pipeline).map(&:id)) end it 'resets expire_at' do @@ -2899,7 +2899,7 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def { key: 'CI_DEFAULT_BRANCH', value: project.default_branch, public: true, masked: false }, { key: 'CI_CONFIG_PATH', value: project.ci_config_path_or_default, public: true, masked: false }, { key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host, public: true, masked: false }, - { key: 'CI_PAGES_URL', value: project.pages_url, public: true, masked: false }, + { key: 'CI_PAGES_URL', value: Gitlab::Pages::UrlBuilder.new(project).pages_url, public: true, masked: false }, { key: 'CI_DEPENDENCY_PROXY_SERVER', value: Gitlab.host_with_port, public: true, masked: false }, { key: 'CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX', value: "#{Gitlab.host_with_port}/#{project.namespace.root_ancestor.path.downcase}#{DependencyProxy::URL_SUFFIX}", diff --git a/spec/models/ci/catalog/resource_spec.rb b/spec/models/ci/catalog/resource_spec.rb index 4c1ade5c308..45d49d65b02 100644 --- a/spec/models/ci/catalog/resource_spec.rb +++ b/spec/models/ci/catalog/resource_spec.rb @@ -22,6 +22,8 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do it { is_expected.to delegate_method(:star_count).to(:project) } it { is_expected.to delegate_method(:forks_count).to(:project) } + it { is_expected.to define_enum_for(:state).with_values({ draft: 0, published: 1 }) } + describe '.for_projects' do it 'returns catalog resources for the given project IDs' do resources_for_projects = described_class.for_projects(project.id) @@ -65,4 +67,10 @@ RSpec.describe Ci::Catalog::Resource, feature_category: :pipeline_composition do expect(resource.latest_version).to eq(release3) end end + + describe '#state' do + it 'defaults to draft' do + expect(resource.state).to eq('draft') + end + end end diff --git a/spec/models/ci/daily_build_group_report_result_spec.rb b/spec/models/ci/daily_build_group_report_result_spec.rb index 6f73d89d760..5de7f5a527f 100644 --- a/spec/models/ci/daily_build_group_report_result_spec.rb +++ b/spec/models/ci/daily_build_group_report_result_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::DailyBuildGroupReportResult do +RSpec.describe Ci::DailyBuildGroupReportResult, feature_category: :continuous_integration do let(:daily_build_group_report_result) { build(:ci_daily_build_group_report_result) } describe 'associations' do diff --git a/spec/models/external_pull_request_spec.rb b/spec/models/ci/external_pull_request_spec.rb index 10136dd0bdb..2a273146626 100644 --- a/spec/models/external_pull_request_spec.rb +++ b/spec/models/ci/external_pull_request_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ExternalPullRequest do +RSpec.describe Ci::ExternalPullRequest, feature_category: :continuous_integration do let_it_be(:project) { create(:project, :repository) } let(:source_branch) { 'the-branch' } @@ -228,12 +228,12 @@ RSpec.describe ExternalPullRequest do it 'returns modified paths' do expect(modified_paths).to eq ['bar/branch-test.txt', - 'files/js/commit.coffee', - 'with space/README.md'] + 'files/js/commit.coffee', + 'with space/README.md'] end end - context 'loose foreign key on external_pull_requests.project_id' do + context 'with a loose foreign key on external_pull_requests.project_id' do it_behaves_like 'cleanup by a loose foreign key' do let!(:parent) { create(:project) } let!(:model) { create(:external_pull_request, project: parent) } diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb index 5a8a2b391e1..bf2405a5936 100644 --- a/spec/models/ci/group_variable_spec.rb +++ b/spec/models/ci/group_variable_spec.rb @@ -13,13 +13,19 @@ RSpec.describe Ci::GroupVariable, feature_category: :secrets_management do it { is_expected.to include_module(Presentable) } it { is_expected.to include_module(Ci::Maskable) } it { is_expected.to include_module(HasEnvironmentScope) } - it { is_expected.to validate_uniqueness_of(:key).scoped_to([:group_id, :environment_scope]).with_message(/\(\w+\) has already been taken/) } + + describe 'validations' do + it { is_expected.to validate_uniqueness_of(:key).scoped_to([:group_id, :environment_scope]).with_message(/\(\w+\) has already been taken/) } + it { is_expected.to allow_values('').for(:description) } + it { is_expected.to allow_values(nil).for(:description) } + it { is_expected.to validate_length_of(:description).is_at_most(255) } + end describe '.by_environment_scope' do let!(:matching_variable) { create(:ci_group_variable, environment_scope: 'production ') } let!(:non_matching_variable) { create(:ci_group_variable, environment_scope: 'staging') } - subject { Ci::GroupVariable.by_environment_scope('production') } + subject { described_class.by_environment_scope('production') } it { is_expected.to contain_exactly(matching_variable) } end @@ -84,6 +90,49 @@ RSpec.describe Ci::GroupVariable, feature_category: :secrets_management do end end + describe 'sort_by_attribute' do + let_it_be(:group) { create(:group) } + let_it_be(:environment_scope) { 'env_scope' } + let_it_be(:variable1) { create(:ci_group_variable, key: 'd_var', group: group, environment_scope: environment_scope, created_at: 4.days.ago) } + let_it_be(:variable2) { create(:ci_group_variable, key: 'a_var', group: group, environment_scope: environment_scope, created_at: 3.days.ago) } + let_it_be(:variable3) { create(:ci_group_variable, key: 'c_var', group: group, environment_scope: environment_scope, created_at: 2.days.ago) } + let_it_be(:variable4) { create(:ci_group_variable, key: 'b_var', group: group, environment_scope: environment_scope, created_at: 1.day.ago) } + + let(:sort_by_attribute) { described_class.sort_by_attribute(method).pluck(:key) } + + describe '.created_at_asc' do + let(:method) { 'created_at_asc' } + + it 'order by created_at ascending' do + expect(sort_by_attribute).to eq(%w[d_var a_var c_var b_var]) + end + end + + describe '.created_at_desc' do + let(:method) { 'created_at_desc' } + + it 'order by created_at descending' do + expect(sort_by_attribute).to eq(%w[b_var c_var a_var d_var]) + end + end + + describe '.key_asc' do + let(:method) { 'key_asc' } + + it 'order by key ascending' do + expect(sort_by_attribute).to eq(%w[a_var b_var c_var d_var]) + end + end + + describe '.key_desc' do + let(:method) { 'key_desc' } + + it 'order by key descending' do + expect(sort_by_attribute).to eq(%w[d_var c_var b_var a_var]) + end + end + end + it_behaves_like 'cleanup by a loose foreign key' do let!(:model) { create(:ci_group_variable) } diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index a34657adf60..83c233fa942 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -204,7 +204,7 @@ RSpec.describe Ci::JobArtifact, feature_category: :build_artifacts do describe '.associated_file_types_for' do using RSpec::Parameterized::TableSyntax - subject { Ci::JobArtifact.associated_file_types_for(file_type) } + subject { described_class.associated_file_types_for(file_type) } where(:file_type, :result) do 'codequality' | %w(codequality) diff --git a/spec/models/ci/persistent_ref_spec.rb b/spec/models/ci/persistent_ref_spec.rb index e488580ae7b..ecaa8f59ecf 100644 --- a/spec/models/ci/persistent_ref_spec.rb +++ b/spec/models/ci/persistent_ref_spec.rb @@ -3,14 +3,28 @@ require 'spec_helper' RSpec.describe Ci::PersistentRef do - it 'cleans up persistent refs after pipeline finished' do + it 'cleans up persistent refs after pipeline finished', :sidekiq_inline do pipeline = create(:ci_pipeline, :running) - expect(pipeline.persistent_ref).to receive(:delete).once + expect(Ci::PipelineCleanupRefWorker).to receive(:perform_async).with(pipeline.id) pipeline.succeed! end + context 'when pipeline_cleanup_ref_worker_async is disabled' do + before do + stub_feature_flags(pipeline_cleanup_ref_worker_async: false) + end + + it 'cleans up persistent refs after pipeline finished' do + pipeline = create(:ci_pipeline, :running) + + expect(pipeline.persistent_ref).to receive(:delete).once + + pipeline.succeed! + end + end + describe '#exist?' do subject { pipeline.persistent_ref.exist? } @@ -72,7 +86,7 @@ RSpec.describe Ci::PersistentRef do describe '#delete' do subject { pipeline.persistent_ref.delete } - let(:pipeline) { create(:ci_pipeline, sha: sha, project: project) } + let(:pipeline) { create(:ci_pipeline, :success, sha: sha, project: project) } let(:project) { create(:project, :repository) } let(:sha) { project.repository.commit.sha } diff --git a/spec/models/ci/pipeline_artifact_spec.rb b/spec/models/ci/pipeline_artifact_spec.rb index 3038cdc944b..eb89c7af208 100644 --- a/spec/models/ci/pipeline_artifact_spec.rb +++ b/spec/models/ci/pipeline_artifact_spec.rb @@ -101,7 +101,7 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end describe '.report_exists?' do - subject(:pipeline_artifact) { Ci::PipelineArtifact.report_exists?(file_type) } + subject(:pipeline_artifact) { described_class.report_exists?(file_type) } context 'when file_type is code_coverage' do let(:file_type) { :code_coverage } @@ -149,7 +149,7 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end describe '.find_by_file_type' do - subject(:pipeline_artifact) { Ci::PipelineArtifact.find_by_file_type(file_type) } + subject(:pipeline_artifact) { described_class.find_by_file_type(file_type) } context 'when file_type is code_coverage' do let(:file_type) { :code_coverage } @@ -204,7 +204,7 @@ RSpec.describe Ci::PipelineArtifact, type: :model do let(:size) { file['tempfile'].size } subject do - Ci::PipelineArtifact.create_or_replace_for_pipeline!( + described_class.create_or_replace_for_pipeline!( pipeline: pipeline, file_type: file_type, file: file, @@ -231,7 +231,7 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end it "creates a new pipeline artifact with pipeline's locked state" do - artifact = Ci::PipelineArtifact.create_or_replace_for_pipeline!( + artifact = described_class.create_or_replace_for_pipeline!( pipeline: pipeline, file_type: file_type, file: file, diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b9e331affb1..ae3725a0b08 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1328,11 +1328,34 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: %w[succeed! drop! cancel! skip! block! delay!].each do |action| context "when the pipeline received #{action} event" do - it 'deletes a persistent ref' do - expect(pipeline.persistent_ref).to receive(:delete).once + it 'deletes a persistent ref asynchronously', :sidekiq_inline do + expect(pipeline.persistent_ref).not_to receive(:delete_refs) + + expect(Ci::PipelineCleanupRefWorker).to receive(:perform_async) + .with(pipeline.id).and_call_original + + expect_next_instance_of(Ci::PersistentRef) do |persistent_ref| + expect(persistent_ref).to receive(:delete_refs) + .with("refs/#{Repository::REF_PIPELINES}/#{pipeline.id}").once + end pipeline.public_send(action) end + + context 'when pipeline_cleanup_ref_worker_async is disabled' do + before do + stub_feature_flags(pipeline_cleanup_ref_worker_async: false) + end + + it 'deletes a persistent ref synchronously' do + expect(Ci::PipelineCleanupRefWorker).not_to receive(:perform_async).with(pipeline.id) + + expect(pipeline.persistent_ref).to receive(:delete_refs).once + .with("refs/#{Repository::REF_PIPELINES}/#{pipeline.id}") + + pipeline.public_send(action) + end + end end end @@ -5323,7 +5346,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep, feature_category: it 'raises an exception' do pipeline.save! - pipeline_id = Ci::Pipeline.where(id: pipeline.id).select(:id).first + pipeline_id = described_class.where(id: pipeline.id).select(:id).first expect { pipeline_id.age_in_minutes }.to raise_error(ArgumentError) end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 86894ebcf2d..e1c449e18ac 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Ci::Processable, feature_category: :continuous_integration do let_it_be(:pipeline) { create(:ci_pipeline, project: project) } describe 'delegations' do - subject { Ci::Processable.new } + subject { described_class.new } it { is_expected.to delegate_method(:merge_request?).to(:pipeline) } it { is_expected.to delegate_method(:merge_request_ref?).to(:pipeline) } @@ -401,7 +401,7 @@ RSpec.describe Ci::Processable, feature_category: :continuous_integration do let!(:another_build) { create(:ci_build, project: project) } before do - Ci::Processable.update_all(scheduling_type: nil) + described_class.update_all(scheduling_type: nil) end it 'populates scheduling_type of processables' do diff --git a/spec/models/ci/ref_spec.rb b/spec/models/ci/ref_spec.rb index eab5a40bc30..a60aed98a21 100644 --- a/spec/models/ci/ref_spec.rb +++ b/spec/models/ci/ref_spec.rb @@ -74,7 +74,7 @@ RSpec.describe Ci::Ref do it 'returns an existing ci_ref' do expect { subject }.not_to change { described_class.count } - expect(subject).to eq(Ci::Ref.find_by(project_id: project.id, ref_path: expected_ref_path)) + expect(subject).to eq(described_class.find_by(project_id: project.id, ref_path: expected_ref_path)) end end @@ -84,7 +84,7 @@ RSpec.describe Ci::Ref do it 'creates a new ci_ref' do expect { subject }.to change { described_class.count }.by(1) - expect(subject).to eq(Ci::Ref.find_by(project_id: project.id, ref_path: expected_ref_path)) + expect(subject).to eq(described_class.find_by(project_id: project.id, ref_path: expected_ref_path)) end end end diff --git a/spec/models/ci/runner_manager_spec.rb b/spec/models/ci/runner_manager_spec.rb index d69c9ef845e..80cffb98dff 100644 --- a/spec/models/ci/runner_manager_spec.rb +++ b/spec/models/ci/runner_manager_spec.rb @@ -69,6 +69,49 @@ RSpec.describe Ci::RunnerManager, feature_category: :runner_fleet, type: :model it { is_expected.to eq(7.days.ago) } end + describe '.for_runner' do + subject(:runner_managers) { described_class.for_runner(runner_arg) } + + let_it_be(:runner1) { create(:ci_runner) } + let_it_be(:runner_manager11) { create(:ci_runner_machine, runner: runner1) } + let_it_be(:runner_manager12) { create(:ci_runner_machine, runner: runner1) } + + context 'with single runner' do + let(:runner_arg) { runner1 } + + it { is_expected.to contain_exactly(runner_manager11, runner_manager12) } + end + + context 'with multiple runners' do + let(:runner_arg) { [runner1, runner2] } + + let_it_be(:runner2) { create(:ci_runner) } + let_it_be(:runner_manager2) { create(:ci_runner_machine, runner: runner2) } + + it { is_expected.to contain_exactly(runner_manager11, runner_manager12, runner_manager2) } + end + end + + describe '.aggregate_upgrade_status_by_runner_id' do + let!(:runner_version1) { create(:ci_runner_version, version: '16.0.0', status: :recommended) } + let!(:runner_version2) { create(:ci_runner_version, version: '16.0.1', status: :available) } + + let!(:runner1) { create(:ci_runner) } + let!(:runner2) { create(:ci_runner) } + let!(:runner_manager11) { create(:ci_runner_machine, runner: runner1, version: runner_version1.version) } + let!(:runner_manager12) { create(:ci_runner_machine, runner: runner1, version: runner_version2.version) } + let!(:runner_manager2) { create(:ci_runner_machine, runner: runner2, version: runner_version2.version) } + + subject { described_class.aggregate_upgrade_status_by_runner_id } + + it 'contains aggregate runner upgrade status by runner ID' do + is_expected.to eq({ + runner1.id => :recommended, + runner2.id => :available + }) + end + end + describe '#status', :freeze_time do let(:runner_manager) { build(:ci_runner_machine, created_at: 8.days.ago) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index b0ff070e4a6..50e2ded695c 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -316,8 +316,7 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do context 'when use_traversal_ids* are disabled' do before do stub_feature_flags( - use_traversal_ids: false, - use_traversal_ids_for_ancestors: false + use_traversal_ids: false ) end diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index 79e92082ee1..1be50083cd4 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Stage, :models do +RSpec.describe Ci::Stage, :models, feature_category: :continuous_integration do let_it_be(:pipeline) { create(:ci_empty_pipeline) } let(:stage) { create(:ci_stage, pipeline: pipeline, project: pipeline.project) } diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index 85327dbeb34..c93f355718a 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -15,13 +15,16 @@ RSpec.describe Ci::Variable, feature_category: :secrets_management do it { is_expected.to include_module(Ci::Maskable) } it { is_expected.to include_module(HasEnvironmentScope) } it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope).with_message(/\(\w+\) has already been taken/) } + it { is_expected.to allow_values('').for(:description) } + it { is_expected.to allow_values(nil).for(:description) } + it { is_expected.to validate_length_of(:description).is_at_most(255) } end describe '.by_environment_scope' do let!(:matching_variable) { create(:ci_variable, environment_scope: 'production ') } let!(:non_matching_variable) { create(:ci_variable, environment_scope: 'staging') } - subject { Ci::Variable.by_environment_scope('production') } + subject { described_class.by_environment_scope('production') } it { is_expected.to contain_exactly(matching_variable) } end diff --git a/spec/models/ci_platform_metric_spec.rb b/spec/models/ci_platform_metric_spec.rb index e59730792b8..1f25f10c5d2 100644 --- a/spec/models/ci_platform_metric_spec.rb +++ b/spec/models/ci_platform_metric_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe CiPlatformMetric, feature_category: :continuous_integration do subject { build(:ci_platform_metric) } - it_behaves_like 'a BulkInsertSafe model', CiPlatformMetric do + it_behaves_like 'a BulkInsertSafe model', described_class do let(:valid_items_for_bulk_insertion) { build_list(:ci_platform_metric, 10) } let(:invalid_items_for_bulk_insertion) { [] } # class does not have any non-constraint validations defined end diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb index 7c546f42d5d..6201b7b1861 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -198,14 +198,6 @@ RSpec.describe Clusters::Agent, feature_category: :deployment_management do it { is_expected.to eq(allowed) } end - - context 'when expose_authorized_cluster_agents feature flag is disabled' do - before do - stub_feature_flags(expose_authorized_cluster_agents: false) - end - - it { is_expected.to eq(false) } - end end context 'with group-level authorization' do @@ -226,14 +218,6 @@ RSpec.describe Clusters::Agent, feature_category: :deployment_management do it { is_expected.to eq(allowed) } end - - context 'when expose_authorized_cluster_agents feature flag is disabled' do - before do - stub_feature_flags(expose_authorized_cluster_agents: false) - end - - it { is_expected.to eq(false) } - end end end @@ -269,14 +253,6 @@ RSpec.describe Clusters::Agent, feature_category: :deployment_management do it { is_expected.to eq(allowed) } end - - context 'when expose_authorized_cluster_agents feature flag is disabled' do - before do - stub_feature_flags(expose_authorized_cluster_agents: false) - end - - it { is_expected.to eq(false) } - end end context 'with group-level authorization' do @@ -297,14 +273,6 @@ RSpec.describe Clusters::Agent, feature_category: :deployment_management do it { is_expected.to eq(allowed) } end - - context 'when expose_authorized_cluster_agents feature flag is disabled' do - before do - stub_feature_flags(expose_authorized_cluster_agents: false) - end - - it { is_expected.to eq(false) } - end end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 99932dc27d1..73df283d996 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -983,7 +983,7 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching, end describe '#make_cleanup_errored!' do - non_errored_states = Clusters::Cluster.state_machines[:cleanup_status].states.keys - [:cleanup_errored] + non_errored_states = described_class.state_machines[:cleanup_status].states.keys - [:cleanup_errored] non_errored_states.each do |state| it "transitions cleanup_status from #{state} to cleanup_errored" do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index edb856d34df..dd3d4f1865c 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -88,7 +88,7 @@ RSpec.describe Commit do it 'returns a Commit' do commit = described_class.build_from_sidekiq_hash(project, id: '123') - expect(commit).to be_an_instance_of(Commit) + expect(commit).to be_an_instance_of(described_class) end it 'parses date strings into Time instances' do diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 38c45e8c975..ac356bcd65a 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -113,7 +113,7 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do let!(:stale_scheduled) { create(:commit_status, scheduled_at: 1.day.ago) } let!(:fresh_scheduled) { create(:commit_status, scheduled_at: 1.minute.ago) } - subject { CommitStatus.scheduled_at_before(1.hour.ago) } + subject { described_class.scheduled_at_before(1.hour.ago) } it { is_expected.to contain_exactly(stale_scheduled) } end @@ -141,7 +141,7 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do commit_status.update!(retried: false, status: :pending) # another process does mark object as processed - CommitStatus.find(commit_status.id).update_column(:processed, true) + described_class.find(commit_status.id).update_column(:processed, true) # subsequent status transitions on the same instance # always saves processed=false to DB even though @@ -149,7 +149,7 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do commit_status.update!(retried: false, status: :running) # we look at a persisted state in DB - expect(CommitStatus.find(commit_status.id).processed).to eq(false) + expect(described_class.find(commit_status.id).processed).to eq(false) end end diff --git a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb index e8d84fe9630..256cd386ce2 100644 --- a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb +++ b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb @@ -27,7 +27,7 @@ RSpec.describe BatchDestroyDependentAssociations do let_it_be(:build) { create(:ci_build, project: project) } let_it_be(:notification_setting) { create(:notification_setting, project: project) } let_it_be(:note) { create(:note, project: project) } - let_it_be(:merge_request) { create(:merge_request, source_project: project) } + let_it_be(:merge_request) { create(:merge_request, :skip_diff_creation, source_project: project) } it 'destroys multiple notes' do create(:note, project: project) diff --git a/spec/models/concerns/counter_attribute_spec.rb b/spec/models/concerns/counter_attribute_spec.rb index c8224c64ba2..cde58e4088c 100644 --- a/spec/models/concerns/counter_attribute_spec.rb +++ b/spec/models/concerns/counter_attribute_spec.rb @@ -8,7 +8,7 @@ RSpec.describe CounterAttribute, :counter_attribute, :clean_gitlab_redis_shared_ let(:project_statistics) { create(:project_statistics) } let(:model) { CounterAttributeModel.find(project_statistics.id) } - it_behaves_like CounterAttribute, [:build_artifacts_size, :commit_count, :packages_size] do + it_behaves_like described_class, [:build_artifacts_size, :commit_count, :packages_size] do let(:model) { CounterAttributeModel.find(project_statistics.id) } end diff --git a/spec/models/concerns/database_event_tracking_spec.rb b/spec/models/concerns/database_event_tracking_spec.rb index 502cecaaf76..a99b4737537 100644 --- a/spec/models/concerns/database_event_tracking_spec.rb +++ b/spec/models/concerns/database_event_tracking_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe DatabaseEventTracking, :snowplow do +RSpec.describe DatabaseEventTracking, :snowplow, feature_category: :service_ping do before do allow(Gitlab::Tracking).to receive(:database_event).and_call_original end @@ -31,18 +31,6 @@ RSpec.describe DatabaseEventTracking, :snowplow do end end - context 'if product_intelligence_database_event_tracking FF is off' do - before do - stub_feature_flags(product_intelligence_database_event_tracking: false) - end - - it 'does not track the event' do - create_test_class_record - - expect_no_snowplow_event(tracking_method: :database_event) - end - end - describe 'event tracking' do let(:category) { test_class.to_s } let(:event) { 'database_event' } diff --git a/spec/models/concerns/expirable_spec.rb b/spec/models/concerns/expirable_spec.rb index 68a25917ce1..78fe265a6bb 100644 --- a/spec/models/concerns/expirable_spec.rb +++ b/spec/models/concerns/expirable_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Expirable do end describe '.expired' do - it { expect(ProjectMember.expired).to match_array([expired]) } + it { expect(ProjectMember.expired).to contain_exactly(expired) } it 'scopes the query when multiple models are expirable' do expired_access_token = create(:personal_access_token, :expired, user: no_expire.user) diff --git a/spec/models/concerns/group_descendant_spec.rb b/spec/models/concerns/group_descendant_spec.rb index d593d829dca..f27f3a5e6a0 100644 --- a/spec/models/concerns/group_descendant_spec.rb +++ b/spec/models/concerns/group_descendant_spec.rb @@ -19,16 +19,15 @@ RSpec.describe GroupDescendant do query_count = ActiveRecord::QueryRecorder.new { test_group.hierarchy }.count - # use_traversal_ids_for_ancestors_upto actor based feature flag check adds an extra query. - expect(query_count).to eq(2) + expect(query_count).to eq(1) end it 'only queries once for the ancestors when a top is given' do test_group = create(:group, parent: subsub_group).reload recorder = ActiveRecord::QueryRecorder.new { test_group.hierarchy(subgroup) } - # use_traversal_ids_for_ancestors_upto actor based feature flag check adds an extra query. - expect(recorder.count).to eq(2) + + expect(recorder.count).to eq(1) end it 'builds a hierarchy for a group' do diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index f9bf576d75b..49c3d11ed6b 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -15,7 +15,7 @@ RSpec.describe User, feature_category: :system_access do describe 'validations' do it 'validates type presence' do - expect(User.new).to validate_presence_of(:user_type) + expect(described_class.new).to validate_presence_of(:user_type) end end diff --git a/spec/models/concerns/integrations/enable_ssl_verification_spec.rb b/spec/models/concerns/integrations/enable_ssl_verification_spec.rb index 802e950c0c2..418f3f4dbc6 100644 --- a/spec/models/concerns/integrations/enable_ssl_verification_spec.rb +++ b/spec/models/concerns/integrations/enable_ssl_verification_spec.rb @@ -19,5 +19,5 @@ RSpec.describe Integrations::EnableSslVerification do let(:integration) { described_class.new } - include_context Integrations::EnableSslVerification + include_context described_class end diff --git a/spec/models/concerns/integrations/reset_secret_fields_spec.rb b/spec/models/concerns/integrations/reset_secret_fields_spec.rb index a372550c70f..3b15b95fea9 100644 --- a/spec/models/concerns/integrations/reset_secret_fields_spec.rb +++ b/spec/models/concerns/integrations/reset_secret_fields_spec.rb @@ -15,5 +15,5 @@ RSpec.describe Integrations::ResetSecretFields do let(:integration) { described_class.new } - it_behaves_like Integrations::ResetSecretFields + it_behaves_like described_class end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 4e99419a7f2..e4af778b967 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Issuable do +RSpec.describe Issuable, feature_category: :team_planning do include ProjectForksHelper using RSpec::Parameterized::TableSyntax diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 46a876f34e9..01efe66a419 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -356,4 +356,20 @@ RSpec.describe Milestone, 'Milestoneish', factory_default: :keep do expect(milestone.human_total_time_estimate).to be_nil end end + + describe '#expires_at' do + it 'returns the date when milestone expires' do + due_date = Date.today + 1.day + milestone.due_date = due_date + + expect(milestone.expires_at).to eq("expires on #{due_date.to_fs(:medium)}") + end + + it 'returns the date when milestone expires' do + due_date = Date.today - 1.day + milestone.due_date = due_date + + expect(milestone.expires_at).to eq("expired on #{due_date.to_fs(:medium)}") + end + end end diff --git a/spec/models/concerns/resolvable_note_spec.rb b/spec/models/concerns/resolvable_note_spec.rb index 69c58a5cfe5..09646f6c4eb 100644 --- a/spec/models/concerns/resolvable_note_spec.rb +++ b/spec/models/concerns/resolvable_note_spec.rb @@ -18,25 +18,25 @@ RSpec.describe Note, ResolvableNote do describe '.potentially_resolvable' do it 'includes diff and discussion notes on merge requests' do - expect(Note.potentially_resolvable).to match_array([note3, note4, note6]) + expect(described_class.potentially_resolvable).to match_array([note3, note4, note6]) end end describe '.resolvable' do it 'includes non-system diff and discussion notes on merge requests' do - expect(Note.resolvable).to match_array([note3, note4]) + expect(described_class.resolvable).to match_array([note3, note4]) end end describe '.resolved' do it 'includes resolved non-system diff and discussion notes on merge requests' do - expect(Note.resolved).to match_array([note3]) + expect(described_class.resolved).to match_array([note3]) end end describe '.unresolved' do it 'includes non-resolved non-system diff and discussion notes on merge requests' do - expect(Note.unresolved).to match_array([note4]) + expect(described_class.unresolved).to match_array([note4]) end end end diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 8a2fa6675e5..7ef0473aea8 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -71,8 +71,8 @@ RSpec.describe Spammable, feature_category: :instance_resiliency do expect(issue.check_for_spam?(user: issue.author)).to eq(true) end - it 'returns false for other visibility levels' do - expect(issue.check_for_spam?(user: issue.author)).to eq(false) + it 'returns true for other visibility levels' do + expect(issue.check_for_spam?(user: issue.author)).to eq(true) end end @@ -82,7 +82,7 @@ RSpec.describe Spammable, feature_category: :instance_resiliency do end context 'when the model is spam' do - where(model: [:issue, :merge_request, :snippet, :spammable_model]) + where(model: [:issue, :merge_request, :note, :snippet, :spammable_model]) with_them do subject do @@ -94,21 +94,7 @@ RSpec.describe Spammable, feature_category: :instance_resiliency do it 'has an error related to spam on the model' do expect(subject.errors.messages[:base]) - .to match_array /Your #{subject.class.model_name.human.downcase} has been recognized as spam./ - end - end - - context 'when the spammable model is a Note' do - subject do - Note.new.tap do |m| - m.spam! - m.invalidate_if_spam - end - end - - it 'has an error related to spam on the model' do - expect(subject.errors.messages[:base]) - .to match_array /Your comment has been recognized as spam./ + .to match_array /Your #{subject.spammable_entity_type} has been recognized as spam./ end end end @@ -293,7 +279,9 @@ RSpec.describe Spammable, feature_category: :instance_resiliency do end describe '#allow_possible_spam?' do - subject { issue.allow_possible_spam? } + let_it_be(:user) { build(:user) } + + subject { spammable_model.allow_possible_spam?(user) } context 'when the `allow_possible_spam` application setting is turned off' do it { is_expected.to eq(false) } diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 70123eaac26..cbfc1df64f1 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -112,8 +112,8 @@ RSpec.describe PersonalAccessToken, 'TokenAuthenticatable' do it 'sets new token' do subject - expect(personal_access_token.token).to eq("#{PersonalAccessToken.token_prefix}#{token_value}") - expect(personal_access_token.token_digest).to eq(Gitlab::CryptoHelper.sha256("#{PersonalAccessToken.token_prefix}#{token_value}")) + expect(personal_access_token.token).to eq("#{described_class.token_prefix}#{token_value}") + expect(personal_access_token.token_digest).to eq(Gitlab::CryptoHelper.sha256("#{described_class.token_prefix}#{token_value}")) end end @@ -138,7 +138,7 @@ RSpec.describe PersonalAccessToken, 'TokenAuthenticatable' do end describe '.find_by_token' do - subject { PersonalAccessToken.find_by_token(token_value) } + subject { described_class.find_by_token(token_value) } it 'finds the token' do personal_access_token.save! @@ -347,7 +347,7 @@ RSpec.describe Ci::Runner, 'TokenAuthenticatable', :freeze_time do end describe '.find_by_token' do - subject { Ci::Runner.find_by_token(runner.token) } + subject { described_class.find_by_token(runner.token) } context 'when runner has no token expiration' do let(:runner) { non_expirable_runner } diff --git a/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb b/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb index 0a71699971e..842020896d9 100644 --- a/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb +++ b/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb @@ -16,6 +16,7 @@ RSpec.describe VulnerabilityFindingSignatureHelpers do describe '#priority' do it 'returns numeric values of the priority string' do + expect(cls.new('scope_offset_compressed').priority).to eq(4) expect(cls.new('scope_offset').priority).to eq(3) expect(cls.new('location').priority).to eq(2) expect(cls.new('hash').priority).to eq(1) @@ -24,6 +25,7 @@ RSpec.describe VulnerabilityFindingSignatureHelpers do describe '#self.priority' do it 'returns the numeric value of the provided string' do + expect(cls.priority('scope_offset_compressed')).to eq(4) expect(cls.priority('scope_offset')).to eq(3) expect(cls.priority('location')).to eq(2) expect(cls.priority('hash')).to eq(1) diff --git a/spec/models/container_expiration_policy_spec.rb b/spec/models/container_expiration_policy_spec.rb index b88eddf19dc..e5f9fdd410e 100644 --- a/spec/models/container_expiration_policy_spec.rb +++ b/spec/models/container_expiration_policy_spec.rb @@ -11,8 +11,7 @@ RSpec.describe ContainerExpirationPolicy, type: :model do it { is_expected.to validate_presence_of(:project) } describe '#enabled' do - it { is_expected.to allow_value(true).for(:enabled) } - it { is_expected.to allow_value(false).for(:enabled) } + it { is_expected.to allow_value(true, false).for(:enabled) } it { is_expected.not_to allow_value(nil).for(:enabled) } end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index d8019e74c71..93fe070e5c4 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -810,7 +810,7 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont subject { repository.size } before do - allow(::Gitlab).to receive(:com?).and_return(on_com) + allow(::Gitlab).to receive(:com_except_jh?).and_return(on_com) allow(repository).to receive(:created_at).and_return(created_at) end @@ -1568,7 +1568,7 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont context 'on gitlab.com' do before do - allow(::Gitlab).to receive(:com?).and_return(true) + allow(::Gitlab).to receive(:com_except_jh?).and_return(true) end it { is_expected.to eq(true) } @@ -1576,7 +1576,7 @@ RSpec.describe ContainerRepository, :aggregate_failures, feature_category: :cont context 'not on gitlab.com' do before do - allow(::Gitlab).to receive(:com?).and_return(false) + allow(::Gitlab).to receive(:com_except_jh?).and_return(false) end it { is_expected.to eq(false) } diff --git a/spec/models/customer_relations/contact_spec.rb b/spec/models/customer_relations/contact_spec.rb index 6beb5323f60..3d78a9089ca 100644 --- a/spec/models/customer_relations/contact_spec.rb +++ b/spec/models/customer_relations/contact_spec.rb @@ -127,7 +127,7 @@ RSpec.describe CustomerRelations::Contact, type: :model do before do old_root_group.update!(parent: new_root_group) - CustomerRelations::Contact.move_to_root_group(old_root_group) + described_class.move_to_root_group(old_root_group) end it 'moves contacts with unique emails and deletes the rest' do diff --git a/spec/models/customer_relations/organization_spec.rb b/spec/models/customer_relations/organization_spec.rb index 350a4e613c6..8151bf18bed 100644 --- a/spec/models/customer_relations/organization_spec.rb +++ b/spec/models/customer_relations/organization_spec.rb @@ -65,7 +65,7 @@ RSpec.describe CustomerRelations::Organization, type: :model do before do old_root_group.update!(parent: new_root_group) - CustomerRelations::Organization.move_to_root_group(old_root_group) + described_class.move_to_root_group(old_root_group) end it 'moves organizations with unique names and deletes the rest' do diff --git a/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb index 9f6358e1286..a58e8df45e4 100644 --- a/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb +++ b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb @@ -11,8 +11,7 @@ RSpec.describe DependencyProxy::ImageTtlGroupPolicy, type: :model do it { is_expected.to validate_presence_of(:group) } describe '#enabled' do - it { is_expected.to allow_value(true).for(:enabled) } - it { is_expected.to allow_value(false).for(:enabled) } + it { is_expected.to allow_value(true, false).for(:enabled) } it { is_expected.not_to allow_value(nil).for(:enabled) } end diff --git a/spec/models/dependency_proxy/manifest_spec.rb b/spec/models/dependency_proxy/manifest_spec.rb index d43079f607a..174fa400d72 100644 --- a/spec/models/dependency_proxy/manifest_spec.rb +++ b/spec/models/dependency_proxy/manifest_spec.rb @@ -51,7 +51,7 @@ RSpec.describe DependencyProxy::Manifest, type: :model do let_it_be(:file_name) { 'foo' } let_it_be(:digest) { 'bar' } - subject { DependencyProxy::Manifest.find_by_file_name_or_digest(file_name: file_name, digest: digest) } + subject { described_class.find_by_file_name_or_digest(file_name: file_name, digest: digest) } context 'no manifest exists' do it { is_expected.to be_nil } diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index ede96d79656..227ac69133b 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -650,7 +650,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do context 'when there are no deployments and builds' do it do - expect(subject_method(environment)).to eq(Deployment.none) + expect(subject_method(environment)).to eq(described_class.none) end end @@ -663,7 +663,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do end it do - expect(subject_method(environment)).to eq(Deployment.none) + expect(subject_method(environment)).to eq(described_class.none) end end @@ -1285,7 +1285,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do let(:build_status) { :created } it_behaves_like 'gracefully handling error' do - let(:error_message) { %Q{Status cannot transition via \"create\"} } + let(:error_message) { %{Status cannot transition via \"create\"} } end end @@ -1315,7 +1315,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do let(:build_status) { :created } it_behaves_like 'gracefully handling error' do - let(:error_message) { %Q{Status cannot transition via \"create\"} } + let(:error_message) { %{Status cannot transition via \"create\"} } end end @@ -1323,7 +1323,7 @@ RSpec.describe Deployment, feature_category: :continuous_delivery do let(:build_status) { :running } it_behaves_like 'gracefully handling error' do - let(:error_message) { %Q{Status cannot transition via \"run\"} } + let(:error_message) { %{Status cannot transition via \"run\"} } end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 7b7b92a0b8d..066763645ab 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -15,6 +15,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ it { is_expected.to be_kind_of(ReactiveCaching) } it { is_expected.to nullify_if_blank(:external_url) } + it { is_expected.to nullify_if_blank(:kubernetes_namespace) } it { is_expected.to belong_to(:project).required } it { is_expected.to belong_to(:merge_request).optional } @@ -36,6 +37,7 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ it { is_expected.to validate_length_of(:slug).is_at_most(24) } it { is_expected.to validate_length_of(:external_url).is_at_most(255) } + it { is_expected.to validate_length_of(:kubernetes_namespace).is_at_most(63) } describe 'validation' do it 'does not become invalid record when external_url is empty' do @@ -80,15 +82,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ expect(env).to validate_presence_of(:tier).on(:update) end end - - context 'when FF is disabled' do - before do - stub_feature_flags(validate_environment_tier_presence: false) - end - - it { expect(env).to validate_presence_of(:tier).on(:create) } - it { expect(env).not_to validate_presence_of(:tier).on(:update) } - end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 527ee96ca86..01fd17bfe10 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -703,10 +703,6 @@ RSpec.describe Group, feature_category: :groups_and_projects do it { expect(group.ancestors.to_sql).not_to include 'traversal_ids <@' } end - describe '#ancestors_upto' do - it { expect(group.ancestors_upto.to_sql).not_to include "WITH ORDINALITY" } - end - describe '.shortest_traversal_ids_prefixes' do it { expect { described_class.shortest_traversal_ids_prefixes }.to raise_error /Feature not supported since the `:use_traversal_ids` is disabled/ } end @@ -737,14 +733,6 @@ RSpec.describe Group, feature_category: :groups_and_projects do it 'hierarchy order' do expect(group.ancestors(hierarchy_order: :asc).to_sql).to include 'ORDER BY "depth" ASC' end - - context 'ancestor linear queries feature flag disabled' do - before do - stub_feature_flags(use_traversal_ids_for_ancestors: false) - end - - it { expect(group.ancestors.to_sql).not_to include 'traversal_ids <@' } - end end describe '#ancestors_upto' do @@ -856,7 +844,29 @@ RSpec.describe Group, feature_category: :groups_and_projects do end it 'returns groups without integration' do - expect(Group.without_integration(instance_integration)).to contain_exactly(another_group) + expect(described_class.without_integration(instance_integration)).to contain_exactly(another_group) + end + end + + describe '.execute_integrations' do + let(:integration) { create(:integrations_slack, :group, group: group) } + let(:test_data) { { 'foo' => 'bar' } } + + before do + allow(group.integrations).to receive(:public_send).and_return([]) + allow(group.integrations).to receive(:public_send).with(:push_hooks).and_return([integration]) + end + + it 'executes integrations with a matching scope' do + expect(integration).to receive(:async_execute).with(test_data) + + group.execute_integrations(test_data, :push_hooks) + end + + it 'ignores integrations without a matching scope' do + expect(integration).not_to receive(:async_execute).with(test_data) + + group.execute_integrations(test_data, :note_hooks) end end @@ -1848,22 +1858,29 @@ RSpec.describe Group, feature_category: :groups_and_projects do end context 'user-related methods' do - let(:user_a) { create(:user) } - let(:user_b) { create(:user) } - let(:user_c) { create(:user) } - let(:user_d) { create(:user) } + let_it_be(:user_a) { create(:user) } + let_it_be(:user_b) { create(:user) } + let_it_be(:user_c) { create(:user) } + let_it_be(:user_d) { create(:user) } - let(:group) { create(:group) } - let(:nested_group) { create(:group, parent: group) } - let(:deep_nested_group) { create(:group, parent: nested_group) } - let(:project) { create(:project, namespace: group) } + let_it_be(:group) { create(:group) } + let_it_be(:nested_group) { create(:group, parent: group) } + let_it_be(:deep_nested_group) { create(:group, parent: nested_group) } + let_it_be(:project) { create(:project, namespace: group) } - before do + let_it_be(:another_group) { create(:group) } + let_it_be(:another_user) { create(:user) } + + before_all do group.add_developer(user_a) group.add_developer(user_c) nested_group.add_developer(user_b) deep_nested_group.add_developer(user_a) project.add_developer(user_d) + + another_group.add_developer(another_user) + + create(:group_group_link, shared_group: group, shared_with_group: another_group) end describe '#direct_and_indirect_users' do @@ -1876,6 +1893,13 @@ RSpec.describe Group, feature_category: :groups_and_projects do it 'does not return members of projects belonging to ancestor groups' do expect(nested_group.direct_and_indirect_users).not_to include(user_d) end + + context 'when share_with_groups is true' do + it 'also returns members of groups invited to this group' do + expect(group.direct_and_indirect_users(share_with_groups: true)) + .to contain_exactly(user_a, user_b, user_c, user_d, another_user) + end + end end describe '#direct_and_indirect_users_with_inactive' do @@ -2643,232 +2667,6 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end - describe '#update_shared_runners_setting!' do - context 'enabled' do - subject { group.update_shared_runners_setting!('enabled') } - - context 'group that its ancestors have shared runners disabled' do - let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled) } - let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, parent: parent) } - let_it_be(:project, reload: true) { create(:project, shared_runners_enabled: false, group: group) } - - it 'raises exception' do - expect { subject } - .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Shared runners enabled cannot be enabled because parent group has shared Runners disabled') - end - - it 'does not enable shared runners' do - expect do - begin - subject - rescue StandardError - nil - end - - parent.reload - group.reload - project.reload - end.to not_change { parent.shared_runners_enabled } - .and not_change { group.shared_runners_enabled } - .and not_change { project.shared_runners_enabled } - end - end - - context 'root group with shared runners disabled' do - let_it_be(:group) { create(:group, :shared_runners_disabled) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } - - it 'enables shared Runners only for itself' do - expect { subject_and_reload(group, sub_group, project) } - .to change { group.shared_runners_enabled }.from(false).to(true) - .and not_change { sub_group.shared_runners_enabled } - .and not_change { project.shared_runners_enabled } - end - end - end - - context 'disabled_and_unoverridable' do - let_it_be(:group) { create(:group) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent: group) } - let_it_be(:sub_group_2) { create(:group, parent: group) } - let_it_be(:project) { create(:project, group: group, shared_runners_enabled: true) } - let_it_be(:project_2) { create(:project, group: sub_group_2, shared_runners_enabled: true) } - - subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_AND_UNOVERRIDABLE) } - - it 'disables shared Runners for all descendant groups and projects' do - expect { subject_and_reload(group, sub_group, sub_group_2, project, project_2) } - .to change { group.shared_runners_enabled }.from(true).to(false) - .and not_change { group.allow_descendants_override_disabled_shared_runners } - .and not_change { sub_group.shared_runners_enabled } - .and change { sub_group.allow_descendants_override_disabled_shared_runners }.from(true).to(false) - .and change { sub_group_2.shared_runners_enabled }.from(true).to(false) - .and not_change { sub_group_2.allow_descendants_override_disabled_shared_runners } - .and change { project.shared_runners_enabled }.from(true).to(false) - .and change { project_2.shared_runners_enabled }.from(true).to(false) - end - - context 'with override on self' do - let_it_be(:group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } - - it 'disables it' do - expect { subject_and_reload(group) } - .to not_change { group.shared_runners_enabled } - .and change { group.allow_descendants_override_disabled_shared_runners }.from(true).to(false) - end - end - end - - context 'disabled_and_overridable' do - subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_AND_OVERRIDABLE) } - - context 'top level group' do - let_it_be(:group) { create(:group, :shared_runners_disabled) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } - - it 'enables allow descendants to override only for itself' do - expect { subject_and_reload(group, sub_group, project) } - .to change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) - .and not_change { group.shared_runners_enabled } - .and not_change { sub_group.allow_descendants_override_disabled_shared_runners } - .and not_change { sub_group.shared_runners_enabled } - .and not_change { project.shared_runners_enabled } - end - end - - context 'group that its ancestors have shared Runners disabled but allows to override' do - let_it_be(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } - let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } - - it 'enables allow descendants to override' do - expect { subject_and_reload(parent, group, project) } - .to not_change { parent.allow_descendants_override_disabled_shared_runners } - .and not_change { parent.shared_runners_enabled } - .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) - .and not_change { group.shared_runners_enabled } - .and not_change { project.shared_runners_enabled } - end - end - - context 'when parent does not allow' do - let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false) } - let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } - - it 'raises exception' do - expect { subject } - .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it') - end - - it 'does not allow descendants to override' do - expect do - begin - subject - rescue StandardError - nil - end - - parent.reload - group.reload - end.to not_change { parent.allow_descendants_override_disabled_shared_runners } - .and not_change { parent.shared_runners_enabled } - .and not_change { group.allow_descendants_override_disabled_shared_runners } - .and not_change { group.shared_runners_enabled } - end - end - - context 'top level group that has shared Runners enabled' do - let_it_be(:group) { create(:group, shared_runners_enabled: true) } - let_it_be(:sub_group) { create(:group, shared_runners_enabled: true, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: true, group: sub_group) } - - it 'enables allow descendants to override & disables shared runners everywhere' do - expect { subject_and_reload(group, sub_group, project) } - .to change { group.shared_runners_enabled }.from(true).to(false) - .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) - .and change { sub_group.shared_runners_enabled }.from(true).to(false) - .and change { project.shared_runners_enabled }.from(true).to(false) - end - end - end - - context 'disabled_with_override (deprecated)' do - subject { group.update_shared_runners_setting!(Namespace::SR_DISABLED_WITH_OVERRIDE) } - - context 'top level group' do - let_it_be(:group) { create(:group, :shared_runners_disabled) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } - - it 'enables allow descendants to override only for itself' do - expect { subject_and_reload(group, sub_group, project) } - .to change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) - .and not_change { group.shared_runners_enabled } - .and not_change { sub_group.allow_descendants_override_disabled_shared_runners } - .and not_change { sub_group.shared_runners_enabled } - .and not_change { project.shared_runners_enabled } - end - end - - context 'group that its ancestors have shared Runners disabled but allows to override' do - let_it_be(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } - let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } - - it 'enables allow descendants to override' do - expect { subject_and_reload(parent, group, project) } - .to not_change { parent.allow_descendants_override_disabled_shared_runners } - .and not_change { parent.shared_runners_enabled } - .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) - .and not_change { group.shared_runners_enabled } - .and not_change { project.shared_runners_enabled } - end - end - - context 'when parent does not allow' do - let_it_be(:parent, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false) } - let_it_be(:group, reload: true) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } - - it 'raises exception' do - expect { subject } - .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it') - end - - it 'does not allow descendants to override' do - expect do - begin - subject - rescue StandardError - nil - end - - parent.reload - group.reload - end.to not_change { parent.allow_descendants_override_disabled_shared_runners } - .and not_change { parent.shared_runners_enabled } - .and not_change { group.allow_descendants_override_disabled_shared_runners } - .and not_change { group.shared_runners_enabled } - end - end - - context 'top level group that has shared Runners enabled' do - let_it_be(:group) { create(:group, shared_runners_enabled: true) } - let_it_be(:sub_group) { create(:group, shared_runners_enabled: true, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: true, group: sub_group) } - - it 'enables allow descendants to override & disables shared runners everywhere' do - expect { subject_and_reload(group, sub_group, project) } - .to change { group.shared_runners_enabled }.from(true).to(false) - .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) - .and change { sub_group.shared_runners_enabled }.from(true).to(false) - .and change { project.shared_runners_enabled }.from(true).to(false) - end - end - end - end - describe "#default_branch_name" do context "when group.namespace_settings does not have a default branch name" do it "returns nil" do diff --git a/spec/models/import_failure_spec.rb b/spec/models/import_failure_spec.rb index a8ada156dd7..6da4045bc00 100644 --- a/spec/models/import_failure_spec.rb +++ b/spec/models/import_failure_spec.rb @@ -12,15 +12,15 @@ RSpec.describe ImportFailure do let_it_be(:unrelated_failure) { create(:import_failure, project: project) } it 'returns failures with external_identifiers' do - expect(ImportFailure.with_external_identifiers).to match_array([github_import_failure]) + expect(described_class.with_external_identifiers).to match_array([github_import_failure]) end it 'returns failures for the given correlation ID' do - expect(ImportFailure.failures_by_correlation_id(correlation_id)).to match_array([hard_failure, soft_failure]) + expect(described_class.failures_by_correlation_id(correlation_id)).to match_array([hard_failure, soft_failure]) end it 'returns hard failures for the given correlation ID' do - expect(ImportFailure.hard_failures_by_correlation_id(correlation_id)).to eq([hard_failure]) + expect(described_class.hard_failures_by_correlation_id(correlation_id)).to eq([hard_failure]) end it 'orders hard failures by newest first' do diff --git a/spec/models/incident_management/timeline_event_spec.rb b/spec/models/incident_management/timeline_event_spec.rb index 036f5affb87..40adf46e7d5 100644 --- a/spec/models/incident_management/timeline_event_spec.rb +++ b/spec/models/incident_management/timeline_event_spec.rb @@ -76,7 +76,7 @@ RSpec.describe IncidentManagement::TimelineEvent do end let(:expected_note_html) do - %Q(<p>note <strong>bold</strong> <em>italic</em> <code>code</code> #{expected_image_html} #{expected_emoji_html}</p>) + %(<p>note <strong>bold</strong> <em>italic</em> <code>code</code> #{expected_image_html} #{expected_emoji_html}</p>) end # rubocop:enable Layout/LineLength diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index ed49009d6d9..7fcd74cd37f 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -246,31 +246,41 @@ RSpec.describe Integration, feature_category: :integrations do end end + describe '#ci?' do + it 'is true when integration is a CI integration' do + expect(build(:jenkins_integration).ci?).to eq(true) + end + + it 'is false when integration is not a ci integration' do + expect(build(:integration).ci?).to eq(false) + end + end + describe '.find_or_initialize_non_project_specific_integration' do let!(:integration_1) { create(:jira_integration, project_id: nil, group_id: group.id) } let!(:integration_2) { create(:jira_integration, project: project) } it 'returns the right integration' do - expect(Integration.find_or_initialize_non_project_specific_integration('jira', group_id: group)) + expect(described_class.find_or_initialize_non_project_specific_integration('jira', group_id: group)) .to eq(integration_1) end it 'does not create a new integration' do - expect { Integration.find_or_initialize_non_project_specific_integration('redmine', group_id: group) } - .not_to change(Integration, :count) + expect { described_class.find_or_initialize_non_project_specific_integration('redmine', group_id: group) } + .not_to change(described_class, :count) end end describe '.find_or_initialize_all_non_project_specific' do shared_examples 'integration instances' do it 'returns the available integration instances' do - expect(Integration.find_or_initialize_all_non_project_specific(Integration.for_instance).map(&:to_param)) - .to match_array(Integration.available_integration_names(include_project_specific: false)) + expect(described_class.find_or_initialize_all_non_project_specific(described_class.for_instance).map(&:to_param)) + .to match_array(described_class.available_integration_names(include_project_specific: false)) end it 'does not create integration instances' do - expect { Integration.find_or_initialize_all_non_project_specific(Integration.for_instance) } - .not_to change(Integration, :count) + expect { described_class.find_or_initialize_all_non_project_specific(described_class.for_instance) } + .not_to change(described_class, :count) end end @@ -282,19 +292,19 @@ RSpec.describe Integration, feature_category: :integrations do end before do - attrs = Integration.available_integration_types(include_project_specific: false).map do + attrs = described_class.available_integration_types(include_project_specific: false).map do integration_hash(_1) end - Integration.insert_all(attrs) + described_class.insert_all(attrs) end it_behaves_like 'integration instances' context 'with a previous existing integration (:mock_ci) and a new integration (:asana)' do before do - Integration.insert(integration_hash(:mock_ci)) - Integration.delete_by(**integration_hash(:asana)) + described_class.insert(integration_hash(:mock_ci)) + described_class.delete_by(**integration_hash(:asana)) end it_behaves_like 'integration instances' @@ -1352,5 +1362,17 @@ RSpec.describe Integration, feature_category: :integrations do async_execute end end + + context 'when the Gitlab::SilentMode is enabled' do + before do + allow(Gitlab::SilentMode).to receive(:enabled?).and_return(true) + end + + it 'does not queue a worker' do + expect(Integrations::ExecuteWorker).not_to receive(:perform_async) + + async_execute + end + end end end diff --git a/spec/models/integrations/apple_app_store_spec.rb b/spec/models/integrations/apple_app_store_spec.rb index f3346acae0a..9864fe38d3f 100644 --- a/spec/models/integrations/apple_app_store_spec.rb +++ b/spec/models/integrations/apple_app_store_spec.rb @@ -13,8 +13,7 @@ RSpec.describe Integrations::AppleAppStore, feature_category: :mobile_devops do it { is_expected.to validate_presence_of :app_store_key_id } it { is_expected.to validate_presence_of :app_store_private_key } it { is_expected.to validate_presence_of :app_store_private_key_file_name } - it { is_expected.to allow_value(true).for(:app_store_protected_refs) } - it { is_expected.to allow_value(false).for(:app_store_protected_refs) } + it { is_expected.to allow_value(true, false).for(:app_store_protected_refs) } it { is_expected.not_to allow_value(nil).for(:app_store_protected_refs) } it { is_expected.to allow_value('aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee').for(:app_store_issuer_id) } it { is_expected.not_to allow_value('abcde').for(:app_store_issuer_id) } diff --git a/spec/models/integrations/asana_spec.rb b/spec/models/integrations/asana_spec.rb index 43e876a4f47..376aec1088e 100644 --- a/spec/models/integrations/asana_spec.rb +++ b/spec/models/integrations/asana_spec.rb @@ -2,20 +2,24 @@ require 'spec_helper' -RSpec.describe Integrations::Asana do +RSpec.describe Integrations::Asana, feature_category: :integrations do describe 'Validations' do - context 'active' do + context 'when active' do before do subject.active = true end it { is_expected.to validate_presence_of :api_key } end + + context 'when inactive' do + it { is_expected.not_to validate_presence_of :api_key } + end end - describe 'Execute' do - let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } + describe '#execute' do + let_it_be(:user) { build(:user) } + let_it_be(:project) { build(:project) } let(:gid) { "123456789ABCD" } let(:asana_task) { double(::Asana::Resources::Task) } diff --git a/spec/models/integrations/assembla_spec.rb b/spec/models/integrations/assembla_spec.rb index e9f4274952d..28cda0a1e75 100644 --- a/spec/models/integrations/assembla_spec.rb +++ b/spec/models/integrations/assembla_spec.rb @@ -2,34 +2,49 @@ require 'spec_helper' -RSpec.describe Integrations::Assembla do +RSpec.describe Integrations::Assembla, feature_category: :integrations do include StubRequests it_behaves_like Integrations::ResetSecretFields do let(:integration) { described_class.new } end - describe "Execute" do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + describe 'Validations' do + context 'when active' do + before do + subject.active = true + end + + it { is_expected.to validate_presence_of :token } + end + + context 'when inactive' do + it { is_expected.not_to validate_presence_of :token } + end + end + + describe "#execute" do + let_it_be(:user) { build(:user) } + let_it_be(:project) { create(:project, :repository) } + + let(:assembla_integration) { described_class.new } + let(:sample_data) { Gitlab::DataBuilder::Push.build_sample(project, user) } + let(:api_url) { 'https://atlas.assembla.com/spaces/project_name/github_tool?secret_key=verySecret' } before do - @assembla_integration = described_class.new - allow(@assembla_integration).to receive_messages( + allow(assembla_integration).to receive_messages( project_id: project.id, project: project, token: 'verySecret', subdomain: 'project_name' ) - @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) - @api_url = 'https://atlas.assembla.com/spaces/project_name/github_tool?secret_key=verySecret' - stub_full_request(@api_url, method: :post) + stub_full_request(api_url, method: :post) end it "calls Assembla API" do - @assembla_integration.execute(@sample_data) - expect(WebMock).to have_requested(:post, stubbed_hostname(@api_url)).with( - body: /#{@sample_data[:before]}.*#{@sample_data[:after]}.*#{project.path}/ + assembla_integration.execute(sample_data) + expect(WebMock).to have_requested(:post, stubbed_hostname(api_url)).with( + body: /#{sample_data[:before]}.*#{sample_data[:after]}.*#{project.path}/ ).once end end diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index 1d2c90dad51..3b459ab9d5b 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -2,26 +2,15 @@ require 'spec_helper' -RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do +RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching, feature_category: :integrations do include ReactiveCachingHelpers include StubRequests let(:bamboo_url) { 'http://gitlab.com/bamboo' } - let_it_be(:project) { create(:project) } - - subject(:integration) do - described_class.create!( - active: true, - project: project, - properties: { - bamboo_url: bamboo_url, - username: 'mic', - password: 'password', - build_key: 'foo' - } - ) - end + let_it_be(:project) { build(:project) } + + subject(:integration) { build(:bamboo_integration, project: project, bamboo_url: bamboo_url) } it_behaves_like Integrations::BaseCi diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index 13dd9e03ab1..675035095c5 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Integrations::BaseChatNotification, feature_category: :integratio it { expect(subject.category).to eq(:chat) } end - describe 'validations' do + describe 'Validations' do before do subject.active = active @@ -112,9 +112,9 @@ RSpec.describe Integrations::BaseChatNotification, feature_category: :integratio end context 'when the data object has a label' do - let_it_be(:label) { create(:label, project: project, name: 'Bug') } - let_it_be(:label_2) { create(:label, project: project, name: 'Community contribution') } - let_it_be(:label_3) { create(:label, project: project, name: 'Backend') } + let_it_be(:label) { build(:label, project: project, name: 'Bug') } + let_it_be(:label_2) { build(:label, project: project, name: 'Community contribution') } + let_it_be(:label_3) { build(:label, project: project, name: 'Backend') } let_it_be(:issue) { create(:labeled_issue, project: project, labels: [label, label_2, label_3]) } let_it_be(:note) { create(:note, noteable: issue, project: project) } diff --git a/spec/models/integrations/base_issue_tracker_spec.rb b/spec/models/integrations/base_issue_tracker_spec.rb index e1a764cd7cb..1bb24876222 100644 --- a/spec/models/integrations/base_issue_tracker_spec.rb +++ b/spec/models/integrations/base_issue_tracker_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe Integrations::BaseIssueTracker do - let(:integration) { Integrations::Redmine.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) } +RSpec.describe Integrations::BaseIssueTracker, feature_category: :integrations do + let(:integration) { build(:redmine_integration, project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) } - let_it_be_with_refind(:project) { create :project } + let_it_be_with_refind(:project) { create(:project) } describe 'default values' do it { expect(subject.category).to eq(:issue_tracker) } diff --git a/spec/models/integrations/base_slack_notification_spec.rb b/spec/models/integrations/base_slack_notification_spec.rb index 8f7f4e8858d..ab977ca8fcc 100644 --- a/spec/models/integrations/base_slack_notification_spec.rb +++ b/spec/models/integrations/base_slack_notification_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::BaseSlackNotification do +RSpec.describe Integrations::BaseSlackNotification, feature_category: :integrations do # This spec should only contain tests that cannot be tested through # `base_slack_notification_shared_examples.rb`. diff --git a/spec/models/integrations/base_third_party_wiki_spec.rb b/spec/models/integrations/base_third_party_wiki_spec.rb index dbead636cb9..763f7131b94 100644 --- a/spec/models/integrations/base_third_party_wiki_spec.rb +++ b/spec/models/integrations/base_third_party_wiki_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::BaseThirdPartyWiki do +RSpec.describe Integrations::BaseThirdPartyWiki, feature_category: :integrations do describe 'default values' do it { expect(subject.category).to eq(:third_party_wiki) } end @@ -11,7 +11,7 @@ RSpec.describe Integrations::BaseThirdPartyWiki do let_it_be_with_reload(:project) { create(:project) } describe 'only one third party wiki per project' do - subject(:integration) { create(:shimo_integration, project: project, active: true) } + subject(:integration) { build(:shimo_integration, project: project, active: true) } before_all do create(:confluence_integration, project: project, active: true) @@ -35,7 +35,7 @@ RSpec.describe Integrations::BaseThirdPartyWiki do end context 'when integration is not on the project level' do - subject(:integration) { create(:shimo_integration, :instance, active: true) } + subject(:integration) { build(:shimo_integration, :instance, active: true) } it 'executes the validation' do expect(integration.valid?(:manual_change)).to be_truthy diff --git a/spec/models/integrations/bugzilla_spec.rb b/spec/models/integrations/bugzilla_spec.rb index f05bc26d066..cef58589231 100644 --- a/spec/models/integrations/bugzilla_spec.rb +++ b/spec/models/integrations/bugzilla_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::Bugzilla do +RSpec.describe Integrations::Bugzilla, feature_category: :integrations do describe 'Validations' do context 'when integration is active' do before do diff --git a/spec/models/integrations/buildkite_spec.rb b/spec/models/integrations/buildkite_spec.rb index 29c649af6c6..f3231d50eae 100644 --- a/spec/models/integrations/buildkite_spec.rb +++ b/spec/models/integrations/buildkite_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do +RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching, feature_category: :integrations do include ReactiveCachingHelpers include StubRequests @@ -71,9 +71,7 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do describe '#hook_url' do it 'returns the webhook url' do - expect(integration.hook_url).to eq( - 'https://webhook.buildkite.com/deliver/{webhook_token}' - ) + expect(integration.hook_url).to eq('https://webhook.buildkite.com/deliver/{webhook_token}') end end @@ -103,9 +101,7 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do describe '#calculate_reactive_cache' do describe '#commit_status' do - let(:buildkite_full_url) do - 'https://gitlab.buildkite.com/status/secret-sauce-status-token.json?commit=123' - end + let(:buildkite_full_url) { 'https://gitlab.buildkite.com/status/secret-sauce-status-token.json?commit=123' } subject { integration.calculate_reactive_cache('123', 'unused')[:commit_status] } diff --git a/spec/models/integrations/chat_message/group_mention_message_spec.rb b/spec/models/integrations/chat_message/group_mention_message_spec.rb new file mode 100644 index 00000000000..6fa486f3dc3 --- /dev/null +++ b/spec/models/integrations/chat_message/group_mention_message_spec.rb @@ -0,0 +1,193 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::ChatMessage::GroupMentionMessage, feature_category: :integrations do + subject { described_class.new(args) } + + let(:color) { '#345' } + let(:args) do + { + object_kind: 'group_mention', + mentioned: { + object_kind: 'group', + name: 'test/group', + url: 'http://test/group' + }, + user: { + name: 'Test User', + username: 'test.user', + avatar_url: 'http://avatar' + }, + project_name: 'Test Project', + project_url: 'http://project' + } + end + + context 'for issue descriptions' do + let(:attachments) { [{ text: "Issue\ndescription\n123", color: color }] } + + before do + args[:object_attributes] = { + object_kind: 'issue', + iid: '42', + title: 'Test Issue', + description: "Issue\ndescription\n123", + url: 'http://issue' + } + end + + it 'returns the appropriate message' do + expect(subject.pretext).to eq( + 'Group <http://test/group|test/group> was mentioned ' \ + 'in <http://issue|issue #42> ' \ + 'of <http://project|Test Project>: ' \ + '*Test Issue*' + ) + expect(subject.attachments).to eq(attachments) + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns the appropriate message' do + expect(subject.pretext).to eq( + 'Group [test/group](http://test/group) was mentioned ' \ + 'in [issue #42](http://issue) ' \ + 'of [Test Project](http://project): ' \ + '*Test Issue*' + ) + expect(subject.attachments).to eq("Issue\ndescription\n123") + expect(subject.activity).to eq( + { + title: 'Group [test/group](http://test/group) was mentioned in [issue #42](http://issue)', + subtitle: 'of [Test Project](http://project)', + text: 'Test Issue', + image: 'http://avatar' + } + ) + end + end + end + + context 'for merge request descriptions' do + let(:attachments) { [{ text: "MR\ndescription\n123", color: color }] } + + before do + args[:object_attributes] = { + object_kind: 'merge_request', + iid: '42', + title: 'Test MR', + description: "MR\ndescription\n123", + url: 'http://merge_request' + } + end + + it 'returns the appropriate message' do + expect(subject.pretext).to eq( + 'Group <http://test/group|test/group> was mentioned ' \ + 'in <http://merge_request|merge request !42> ' \ + 'of <http://project|Test Project>: ' \ + '*Test MR*' + ) + expect(subject.attachments).to eq(attachments) + end + end + + context 'for notes' do + let(:attachments) { [{ text: 'Test Comment', color: color }] } + + before do + args[:object_attributes] = { + object_kind: 'note', + note: 'Test Comment', + url: 'http://note' + } + end + + context 'on commits' do + before do + args[:commit] = { + id: '5f163b2b95e6f53cbd428f5f0b103702a52b9a23', + title: 'Test Commit', + message: "Commit\nmessage\n123\n" + } + end + + it 'returns the appropriate message' do + expect(subject.pretext).to eq( + 'Group <http://test/group|test/group> was mentioned ' \ + 'in <http://note|commit 5f163b2b> ' \ + 'of <http://project|Test Project>: ' \ + '*Test Commit*' + ) + expect(subject.attachments).to eq(attachments) + end + end + + context 'on issues' do + before do + args[:issue] = { + iid: '42', + title: 'Test Issue' + } + end + + it 'returns the appropriate message' do + expect(subject.pretext).to eq( + 'Group <http://test/group|test/group> was mentioned ' \ + 'in <http://note|issue #42> ' \ + 'of <http://project|Test Project>: ' \ + '*Test Issue*' + ) + expect(subject.attachments).to eq(attachments) + end + end + + context 'on merge requests' do + before do + args[:merge_request] = { + iid: '42', + title: 'Test MR' + } + end + + it 'returns the appropriate message' do + expect(subject.pretext).to eq( + 'Group <http://test/group|test/group> was mentioned ' \ + 'in <http://note|merge request !42> ' \ + 'of <http://project|Test Project>: ' \ + '*Test MR*' + ) + expect(subject.attachments).to eq(attachments) + end + end + end + + context 'for unsupported object types' do + before do + args[:object_attributes] = { object_kind: 'unsupported' } + end + + it 'raises an error' do + expect { described_class.new(args) }.to raise_error(NotImplementedError) + end + end + + context 'for notes on unsupported object types' do + before do + args[:object_attributes] = { + object_kind: 'note', + note: 'Test Comment', + url: 'http://note' + } + # Not adding a supported object type's attributes + end + + it 'raises an error' do + expect { described_class.new(args) }.to raise_error(NotImplementedError) + end + end +end diff --git a/spec/models/integrations/confluence_spec.rb b/spec/models/integrations/confluence_spec.rb index 999a532527d..d267e4a71c2 100644 --- a/spec/models/integrations/confluence_spec.rb +++ b/spec/models/integrations/confluence_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::Confluence do +RSpec.describe Integrations::Confluence, feature_category: :integrations do let_it_be(:project) { create(:project) } describe 'Validations' do @@ -49,10 +49,11 @@ RSpec.describe Integrations::Confluence do end context 'when the project wiki is not enabled' do - it 'returns nil when both active or inactive', :aggregate_failures do - project = create(:project, :wiki_disabled) - subject.project = project + before do + allow(project).to receive(:wiki_enabled?).and_return(false) + end + it 'returns nil when both active or inactive', :aggregate_failures do [true, false].each do |active| subject.active = active diff --git a/spec/models/integrations/custom_issue_tracker_spec.rb b/spec/models/integrations/custom_issue_tracker_spec.rb index 11f98b99bbe..4be95a25a43 100644 --- a/spec/models/integrations/custom_issue_tracker_spec.rb +++ b/spec/models/integrations/custom_issue_tracker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::CustomIssueTracker do +RSpec.describe Integrations::CustomIssueTracker, feature_category: :integrations do describe 'Validations' do context 'when integration is active' do before do diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index c46face9702..2cb86fb680a 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -184,7 +184,7 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do "success" => "success" }.each do |drone_status, our_status| it "sets commit status to #{our_status.inspect} when returned status is #{drone_status.inspect}" do - stub_request(body: %Q({"status":"#{drone_status}"})) + stub_request(body: %({"status":"#{drone_status}"})) is_expected.to eq(our_status) end diff --git a/spec/models/integrations/microsoft_teams_spec.rb b/spec/models/integrations/microsoft_teams_spec.rb index f1d9071d232..4f5b4daad42 100644 --- a/spec/models/integrations/microsoft_teams_spec.rb +++ b/spec/models/integrations/microsoft_teams_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integrations::MicrosoftTeams do +RSpec.describe Integrations::MicrosoftTeams, feature_category: :integrations do it_behaves_like "chat integration", "Microsoft Teams" do let(:client) { ::MicrosoftTeams::Notifier } let(:client_arguments) { webhook_url } diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index 8aa9b12c4f0..da43d851b31 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' require 'googleauth' -RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, :snowplow do +RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, :snowplow, feature_category: :metrics do include PrometheusHelpers include ReactiveCachingHelpers @@ -37,8 +37,8 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, integration.manual_configuration = true end - it 'validates presence of api_url' do - expect(integration).to validate_presence_of(:api_url) + it 'does not validates presence of api_url' do + expect(integration).not_to validate_presence_of(:api_url) end end @@ -119,7 +119,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, context 'when configuration is not valid' do before do - integration.api_url = nil + integration.manual_configuration = nil end it 'returns failure message' do diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index e32088a2f79..c4c7202fae0 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -307,7 +307,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do def stub_post_to_build_queue(branch:) teamcity_full_url = "#{teamcity_url}/httpAuth/app/rest/buildQueue" - body ||= %Q(<build branchName=\"#{branch}\"><buildType id=\"foo\"/></build>) + body ||= %(<build branchName=\"#{branch}\"><buildType id=\"foo\"/></build>) auth = %w(mic password) stub_full_request(teamcity_full_url, method: :post).with( @@ -322,7 +322,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do def stub_request(status: 200, body: nil, build_status: 'success') auth = %w(mic password) - body ||= %Q({"build":{"status":"#{build_status}","id":"666"}}) + body ||= %({"build":{"status":"#{build_status}","id":"666"}}) stub_full_request(teamcity_full_url).with(basic_auth: auth).to_return( status: status, diff --git a/spec/models/integrations/unify_circuit_spec.rb b/spec/models/integrations/unify_circuit_spec.rb index 7a91b2d3c11..017443c799f 100644 --- a/spec/models/integrations/unify_circuit_spec.rb +++ b/spec/models/integrations/unify_circuit_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -RSpec.describe Integrations::UnifyCircuit do +RSpec.describe Integrations::UnifyCircuit, feature_category: :integrations do it_behaves_like "chat integration", "Unify Circuit" do let(:client_arguments) { webhook_url } let(:payload) do diff --git a/spec/models/integrations/webex_teams_spec.rb b/spec/models/integrations/webex_teams_spec.rb index b5cba6762aa..50a1383292b 100644 --- a/spec/models/integrations/webex_teams_spec.rb +++ b/spec/models/integrations/webex_teams_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -RSpec.describe Integrations::WebexTeams do +RSpec.describe Integrations::WebexTeams, feature_category: :integrations do it_behaves_like "chat integration", "Webex Teams" do let(:client_arguments) { webhook_url } let(:payload) do diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index 59ade8783e5..0d4756659d6 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -89,7 +89,7 @@ RSpec.describe InternalId do it 'increments counter with in_transaction: "false"' do allow(ApplicationRecord.connection).to receive(:transaction_open?) { false } - expect(InternalId.internal_id_transactions_total).to receive(:increment) + expect(described_class.internal_id_transactions_total).to receive(:increment) .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original subject @@ -98,7 +98,7 @@ RSpec.describe InternalId do context 'when executed within transaction' do it 'increments counter with in_transaction: "true"' do - expect(InternalId.internal_id_transactions_total).to receive(:increment) + expect(described_class.internal_id_transactions_total).to receive(:increment) .with(operation: :generate, usage: 'issues', in_transaction: 'true').and_call_original InternalId.transaction { subject } @@ -148,7 +148,7 @@ RSpec.describe InternalId do it 'increments counter with in_transaction: "false"' do allow(ApplicationRecord.connection).to receive(:transaction_open?) { false } - expect(InternalId.internal_id_transactions_total).to receive(:increment) + expect(described_class.internal_id_transactions_total).to receive(:increment) .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original subject @@ -159,7 +159,7 @@ RSpec.describe InternalId do let(:value) { 2 } it 'increments counter with in_transaction: "true"' do - expect(InternalId.internal_id_transactions_total).to receive(:increment) + expect(described_class.internal_id_transactions_total).to receive(:increment) .with(operation: :reset, usage: 'issues', in_transaction: 'true').and_call_original InternalId.transaction { subject } @@ -219,7 +219,7 @@ RSpec.describe InternalId do it 'increments counter with in_transaction: "false"' do allow(ApplicationRecord.connection).to receive(:transaction_open?) { false } - expect(InternalId.internal_id_transactions_total).to receive(:increment) + expect(described_class.internal_id_transactions_total).to receive(:increment) .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original subject @@ -228,7 +228,7 @@ RSpec.describe InternalId do context 'when executed within transaction' do it 'increments counter with in_transaction: "true"' do - expect(InternalId.internal_id_transactions_total).to receive(:increment) + expect(described_class.internal_id_transactions_total).to receive(:increment) .with(operation: :track_greatest, usage: 'issues', in_transaction: 'true').and_call_original InternalId.transaction { subject } diff --git a/spec/models/issue_assignee_spec.rb b/spec/models/issue_assignee_spec.rb index df8e91cd133..64afed37714 100644 --- a/spec/models/issue_assignee_spec.rb +++ b/spec/models/issue_assignee_spec.rb @@ -27,9 +27,9 @@ RSpec.describe IssueAssignee do context 'in_projects' do it 'returns issue assignees for given project' do - expect(IssueAssignee.count).to eq 2 + expect(described_class.count).to eq 2 - assignees = IssueAssignee.in_projects([project]) + assignees = described_class.in_projects([project]) expect(assignees.count).to eq 1 expect(assignees.first.user_id).to eq project_issue.issue_assignees.first.user_id @@ -39,9 +39,9 @@ RSpec.describe IssueAssignee do context 'on_issues' do it 'returns issue assignees for given issues' do - expect(IssueAssignee.count).to eq 2 + expect(described_class.count).to eq 2 - assignees = IssueAssignee.on_issues([project_issue]) + assignees = described_class.on_issues([project_issue]) expect(assignees.count).to eq 1 expect(assignees.first.issue_id).to eq project_issue.issue_assignees.first.issue_id diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index ee47f90fb40..8d25ac93263 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -127,22 +127,6 @@ RSpec.describe Issue, feature_category: :team_planning do end end - describe 'issue_type' do - let(:issue) { build(:issue, issue_type: issue_type) } - - context 'when a valid type' do - let(:issue_type) { :issue } - - it { is_expected.to eq(true) } - end - - context 'empty type' do - let(:issue_type) { nil } - - it { is_expected.to eq(false) } - end - end - describe '#allowed_work_item_type_change' do where(:old_type, :new_type, :is_valid) do :issue | :incident | true @@ -161,7 +145,7 @@ RSpec.describe Issue, feature_category: :team_planning do it 'is possible to change type only between selected types' do issue = create(:issue, old_type, project: reusable_project) - issue.assign_attributes(work_item_type: WorkItems::Type.default_by_type(new_type), issue_type: new_type) + issue.assign_attributes(work_item_type: WorkItems::Type.default_by_type(new_type)) expect(issue.valid?).to eq(is_valid) end @@ -177,7 +161,7 @@ RSpec.describe Issue, feature_category: :team_planning do let_it_be(:link) { create(:parent_link, work_item: child, work_item_parent: parent) } it 'does not allow to make child not-confidential' do - issue = Issue.find(child.id) + issue = described_class.find(child.id) issue.confidential = false expect(issue).not_to be_valid @@ -186,7 +170,7 @@ RSpec.describe Issue, feature_category: :team_planning do end it 'allows to make parent not-confidential' do - issue = Issue.find(parent.id) + issue = described_class.find(parent.id) issue.confidential = false expect(issue).to be_valid @@ -199,7 +183,7 @@ RSpec.describe Issue, feature_category: :team_planning do let_it_be(:link) { create(:parent_link, work_item: child, work_item_parent: parent) } it 'does not allow to make parent confidential' do - issue = Issue.find(parent.id) + issue = described_class.find(parent.id) issue.confidential = true expect(issue).not_to be_valid @@ -208,7 +192,7 @@ RSpec.describe Issue, feature_category: :team_planning do end it 'allows to make child confidential' do - issue = Issue.find(child.id) + issue = described_class.find(child.id) issue.confidential = true expect(issue).to be_valid @@ -272,7 +256,7 @@ RSpec.describe Issue, feature_category: :team_planning do expect(issue.work_item_type_id).to eq(issue_type.id) expect(WorkItems::Type).not_to receive(:default_by_type) - issue.update!(work_item_type: incident_type, issue_type: :incident) + issue.update!(work_item_type: incident_type) expect(issue.work_item_type_id).to eq(incident_type.id) end @@ -301,36 +285,13 @@ RSpec.describe Issue, feature_category: :team_planning do expect(issue.work_item_type_id).to be_nil expect(WorkItems::Type).not_to receive(:default_by_type) - issue.update!(work_item_type: incident_type, issue_type: :incident) + issue.update!(work_item_type: incident_type) expect(issue.work_item_type_id).to eq(incident_type.id) end end end - describe '#check_issue_type_in_sync' do - it 'raises an error if issue_type is out of sync' do - issue = build(:issue, issue_type: :issue, work_item_type: WorkItems::Type.default_by_type(:task)) - - expect do - issue.save! - end.to raise_error(Issue::IssueTypeOutOfSyncError) - end - - it 'uses attributes to compare both issue_type values' do - issue_type = WorkItems::Type.default_by_type(:issue) - issue = build(:issue, issue_type: :issue, work_item_type: issue_type) - - attributes = double(:attributes) - allow(issue).to receive(:attributes).and_return(attributes) - - expect(attributes).to receive(:[]).with('issue_type').twice.and_return('issue') - expect(issue_type).to receive(:base_type).and_call_original - - issue.save! - end - end - describe '#record_create_action' do it 'records the creation action after saving' do expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_created_action) @@ -338,11 +299,13 @@ RSpec.describe Issue, feature_category: :team_planning do create(:issue) end - it_behaves_like 'issue_edit snowplow tracking' do + it_behaves_like 'internal event tracking' do let(:issue) { create(:issue) } let(:project) { issue.project } - let(:property) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } let(:user) { issue.author } + let(:action) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } + let(:namespace) { project.namespace } + subject(:service_action) { issue } end end @@ -465,18 +428,6 @@ RSpec.describe Issue, feature_category: :team_planning do expect(described_class.with_issue_type([]).to_sql).to include('WHERE 1=0') end end - - context 'when the issue_type_uses_work_item_types_table feature flag is disabled' do - before do - stub_feature_flags(issue_type_uses_work_item_types_table: false) - end - - it 'uses the issue_type column for filtering' do - expect do - described_class.with_issue_type(:issue).to_a - end.to make_queries_matching(/"issues"\."issue_type" = 0/) - end - end end describe '.without_issue_type' do @@ -504,18 +455,6 @@ RSpec.describe Issue, feature_category: :team_planning do }x ) end - - context 'when the issue_type_uses_work_item_types_table feature flag is disabled' do - before do - stub_feature_flags(issue_type_uses_work_item_types_table: false) - end - - it 'uses the issue_type column for filtering' do - expect do - described_class.without_issue_type(:issue).to_a - end.to make_queries_matching(/"issues"\."issue_type" != 0/) - end - end end describe '.order_severity' do @@ -1532,52 +1471,58 @@ RSpec.describe Issue, feature_category: :team_planning do end describe '#publicly_visible?' do - context 'using a public project' do - let(:project) { create(:project, :public) } + let(:project) { build(:project, project_visiblity) } + let(:issue) { build(:issue, confidential: confidential, project: project) } - it 'returns true for a regular issue' do - issue = build(:issue, project: project) + subject { issue.send(:publicly_visible?) } - expect(issue).to be_truthy - end - - it 'returns false for a confidential issue' do - issue = build(:issue, :confidential, project: project) + where(:project_visiblity, :confidential, :expected_value) do + :public | false | true + :public | true | false + :internal | false | false + :internal | true | false + :private | false | false + :private | true | false + end - expect(issue).not_to be_falsy - end + with_them do + it { is_expected.to eq(expected_value) } end + end - context 'using an internal project' do - let(:project) { create(:project, :internal) } + describe '#allow_possible_spam?' do + let_it_be(:issue) { build(:issue) } - it 'returns false for a regular issue' do - issue = build(:issue, project: project) + subject { issue.allow_possible_spam?(issue.author) } - expect(issue).not_to be_falsy - end + context 'when the `allow_possible_spam` application setting is turned off' do + context 'when the issue is private' do + it { is_expected.to eq(true) } - it 'returns false for a confidential issue' do - issue = build(:issue, :confidential, project: project) + context 'when the user is the support bot' do + before do + allow(issue.author).to receive(:support_bot?).and_return(true) + end - expect(issue).not_to be_falsy + it { is_expected.to eq(false) } + end end - end - - context 'using a private project' do - let(:project) { create(:project, :private) } - it 'returns false for a regular issue' do - issue = build(:issue, project: project) + context 'when the issue is public' do + before do + allow(issue).to receive(:publicly_visible?).and_return(true) + end - expect(issue).not_to be_falsy + it { is_expected.to eq(false) } end + end - it 'returns false for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue).not_to be_falsy + context 'when the `allow_possible_spam` application setting is turned on' do + before do + stub_application_setting(allow_possible_spam: true) end + + it { is_expected.to eq(true) } end end @@ -1590,24 +1535,24 @@ RSpec.describe Issue, feature_category: :team_planning do false | Gitlab::VisibilityLevel::PUBLIC | false | { description: 'new' } | true false | Gitlab::VisibilityLevel::PUBLIC | false | { title: 'new' } | true # confidential to non-confidential - false | Gitlab::VisibilityLevel::PUBLIC | true | { confidential: false } | true + false | Gitlab::VisibilityLevel::PUBLIC | true | { confidential: false } | false # non-confidential to confidential false | Gitlab::VisibilityLevel::PUBLIC | false | { confidential: true } | false # spammable attributes changing on confidential - false | Gitlab::VisibilityLevel::PUBLIC | true | { description: 'new' } | false + false | Gitlab::VisibilityLevel::PUBLIC | true | { description: 'new' } | true # spammable attributes changing while changing to confidential - false | Gitlab::VisibilityLevel::PUBLIC | false | { title: 'new', confidential: true } | false + false | Gitlab::VisibilityLevel::PUBLIC | false | { title: 'new', confidential: true } | true # spammable attribute not changing false | Gitlab::VisibilityLevel::PUBLIC | false | { description: 'original description' } | false # non-spammable attribute changing false | Gitlab::VisibilityLevel::PUBLIC | false | { weight: 3 } | false # spammable attributes changing on non-public - false | Gitlab::VisibilityLevel::INTERNAL | false | { description: 'new' } | false - false | Gitlab::VisibilityLevel::PRIVATE | false | { description: 'new' } | false + false | Gitlab::VisibilityLevel::INTERNAL | false | { description: 'new' } | true + false | Gitlab::VisibilityLevel::PRIVATE | false | { description: 'new' } | true ### support-bot cases # confidential to non-confidential - true | Gitlab::VisibilityLevel::PUBLIC | true | { confidential: false } | true + true | Gitlab::VisibilityLevel::PUBLIC | true | { confidential: false } | false # non-confidential to confidential true | Gitlab::VisibilityLevel::PUBLIC | false | { confidential: true } | false # spammable attributes changing on confidential @@ -1877,39 +1822,17 @@ RSpec.describe Issue, feature_category: :team_planning do describe '#issue_type' do let_it_be(:issue) { create(:issue) } - context 'when the issue_type_uses_work_item_types_table feature flag is enabled' do - it 'gets the type field from the work_item_types table' do - expect(issue).to receive_message_chain(:work_item_type, :base_type) - - issue.issue_type - end + it 'gets the type field from the work_item_types table' do + expect(issue).to receive_message_chain(:work_item_type, :base_type) - context 'when the issue is not persisted' do - it 'uses the default work item type' do - non_persisted_issue = build(:issue, work_item_type: nil) - - expect(non_persisted_issue.issue_type).to eq(described_class::DEFAULT_ISSUE_TYPE.to_s) - end - end + issue.issue_type end - context 'when the issue_type_uses_work_item_types_table feature flag is disabled' do - before do - stub_feature_flags(issue_type_uses_work_item_types_table: false) - end - - it 'does not get the value from the work_item_types table' do - expect(issue).not_to receive(:work_item_type) + context 'when the issue is not persisted' do + it 'uses the default work item type' do + non_persisted_issue = build(:issue, work_item_type: nil) - issue.issue_type - end - - context 'when the issue is not persisted' do - it 'uses the default work item type' do - non_persisted_issue = build(:issue, work_item_type: nil) - - expect(non_persisted_issue.issue_type).to eq(described_class::DEFAULT_ISSUE_TYPE.to_s) - end + expect(non_persisted_issue.issue_type).to eq(described_class::DEFAULT_ISSUE_TYPE.to_s) end end end @@ -1944,7 +1867,7 @@ RSpec.describe Issue, feature_category: :team_planning do with_them do before do - issue.update!(issue_type: issue_type, work_item_type: WorkItems::Type.default_by_type(issue_type)) + issue.update!(work_item_type: WorkItems::Type.default_by_type(issue_type)) end specify do @@ -1964,7 +1887,7 @@ RSpec.describe Issue, feature_category: :team_planning do with_them do before do - issue.update!(issue_type: issue_type, work_item_type: WorkItems::Type.default_by_type(issue_type)) + issue.update!(work_item_type: WorkItems::Type.default_by_type(issue_type)) end specify do @@ -2080,7 +2003,7 @@ RSpec.describe Issue, feature_category: :team_planning do end describe '#work_item_type_with_default' do - subject { Issue.new.work_item_type_with_default } + subject { described_class.new.work_item_type_with_default } it { is_expected.to eq(WorkItems::Type.default_by_type(::Issue::DEFAULT_ISSUE_TYPE)) } end @@ -2108,51 +2031,4 @@ RSpec.describe Issue, feature_category: :team_planning do expect { issue1.unsubscribe_email_participant(email) }.not_to change { issue2.issue_email_participants.count } end end - - describe 'issue_type enum generated methods' do - describe '#<issue_type>?' do - let_it_be(:issue) { create(:issue, project: reusable_project) } - - where(issue_type: WorkItems::Type.base_types.keys) - - with_them do - it 'raises an error if called' do - expect { issue.public_send("#{issue_type}?".to_sym) }.to raise_error( - Issue::ForbiddenColumnUsed, - a_string_matching(/`issue\.#{issue_type}\?` uses the `issue_type` column underneath/) - ) - end - end - end - - describe '.<issue_type> scopes' do - where(issue_type: WorkItems::Type.base_types.keys) - - with_them do - it 'raises an error if called' do - expect { Issue.public_send(issue_type.to_sym) }.to raise_error( - Issue::ForbiddenColumnUsed, - a_string_matching(/`Issue\.#{issue_type}` uses the `issue_type` column underneath/) - ) - end - - context 'when called in a production environment' do - before do - stub_rails_env('production') - end - - it 'returns issues scoped by type instead of raising an error' do - issue = create( - :issue, - issue_type: issue_type, - work_item_type: WorkItems::Type.default_by_type(issue_type), - project: reusable_project - ) - - expect(Issue.public_send(issue_type.to_sym)).to contain_exactly(issue) - end - end - end - end - end end diff --git a/spec/models/label_link_spec.rb b/spec/models/label_link_spec.rb index c6bec215145..15931e18715 100644 --- a/spec/models/label_link_spec.rb +++ b/spec/models/label_link_spec.rb @@ -8,7 +8,7 @@ RSpec.describe LabelLink do it { is_expected.to belong_to(:label) } it { is_expected.to belong_to(:target) } - it_behaves_like 'a BulkInsertSafe model', LabelLink do + it_behaves_like 'a BulkInsertSafe model', described_class do let(:valid_items_for_bulk_insertion) { build_list(:label_link, 10) } let(:invalid_items_for_bulk_insertion) { [] } # class does not have any validations defined end diff --git a/spec/models/lfs_objects_project_spec.rb b/spec/models/lfs_objects_project_spec.rb index 7378beeed06..c9b43a55ca7 100644 --- a/spec/models/lfs_objects_project_spec.rb +++ b/spec/models/lfs_objects_project_spec.rb @@ -31,7 +31,7 @@ RSpec.describe LfsObjectsProject do expect do result = described_class.link_to_project!(subject.lfs_object, subject.project) - expect(result).to be_a(LfsObjectsProject) + expect(result).to be_a(described_class) end.not_to change { described_class.count } end diff --git a/spec/models/loose_foreign_keys/deleted_record_spec.rb b/spec/models/loose_foreign_keys/deleted_record_spec.rb index 2513a9043ad..0c16a725663 100644 --- a/spec/models/loose_foreign_keys/deleted_record_spec.rb +++ b/spec/models/loose_foreign_keys/deleted_record_spec.rb @@ -146,7 +146,7 @@ RSpec.describe LooseForeignKeys::DeletedRecord, type: :model do expect { described_class.create!(fully_qualified_table_name: table, primary_key_value: 5) }.not_to raise_error # after processing old records - LooseForeignKeys::DeletedRecord.for_partition(1).update_all(status: :processed) + described_class.for_partition(1).update_all(status: :processed) partition_manager.sync_partitions diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index b242de48be0..d21edea9751 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -254,18 +254,18 @@ RSpec.describe Member, feature_category: :groups_and_projects do ] end - subject { Member.in_hierarchy(project) } + subject { described_class.in_hierarchy(project) } it { is_expected.to contain_exactly(*hierarchy_members) } context 'with scope prefix' do - subject { Member.where.not(source: project).in_hierarchy(subgroup) } + subject { described_class.where.not(source: project).in_hierarchy(subgroup) } it { is_expected.to contain_exactly(root_ancestor_member, subgroup_member, subgroup_project_member) } end context 'with scope suffix' do - subject { Member.in_hierarchy(project).where.not(source: project) } + subject { described_class.in_hierarchy(project).where.not(source: project) } it { is_expected.to contain_exactly(root_ancestor_member, subgroup_member, subgroup_project_member) } end @@ -668,6 +668,15 @@ RSpec.describe Member, feature_category: :groups_and_projects do end end + describe '.with_user' do + it 'returns the member' do + not_a_member = create(:user) + + expect(described_class.with_user(@owner_user)).to eq([@owner]) + expect(described_class.with_user(not_a_member)).to be_empty + end + end + describe '.active_state' do let_it_be(:active_group_member) { create(:group_member, group: group) } let_it_be(:active_project_member) { create(:project_member, project: project) } @@ -859,7 +868,7 @@ RSpec.describe Member, feature_category: :groups_and_projects do expect(member.invite_accepted_at).to be_nil expect(member.invite_token).not_to be_nil - expect_any_instance_of(Member).not_to receive(:after_accept_invite) + expect_any_instance_of(described_class).not_to receive(:after_accept_invite) end it 'schedules a TasksToBeDone::CreateWorker task' do diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index e197d83b621..a07829abece 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -31,15 +31,6 @@ RSpec.describe GroupMember, feature_category: :cell do expect(described_class.of_ldap_type).to eq([group_member]) end end - - describe '.with_user' do - it 'returns requested user' do - group_member = create(:group_member, user: user_2) - create(:group_member, user: user_1) - - expect(described_class.with_user(user_2)).to eq([group_member]) - end - end end describe 'delegations' do diff --git a/spec/models/merge_request/diff_llm_summary_spec.rb b/spec/models/merge_request/diff_llm_summary_spec.rb deleted file mode 100644 index 860457add62..00000000000 --- a/spec/models/merge_request/diff_llm_summary_spec.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ::MergeRequest::DiffLlmSummary, feature_category: :code_review_workflow do - let_it_be_with_reload(:project) { create(:project, :repository) } - - subject(:merge_request_diff_llm_summary) { build(:merge_request_diff_llm_summary) } - - describe 'associations' do - it { is_expected.to belong_to(:merge_request_diff) } - it { is_expected.to belong_to(:user).optional } - it { is_expected.to validate_uniqueness_of(:merge_request_diff_id) } - it { is_expected.to validate_presence_of(:content) } - it { is_expected.to validate_length_of(:content).is_at_most(2056) } - it { is_expected.to validate_presence_of(:provider) } - end -end diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb index b1c2a9b1111..e9e4956dc41 100644 --- a/spec/models/merge_request/metrics_spec.rb +++ b/spec/models/merge_request/metrics_spec.rb @@ -94,7 +94,7 @@ RSpec.describe MergeRequest::Metrics do end end - it_behaves_like 'database events tracking batch 2' do + it_behaves_like 'database events tracking', feature_category: :service_ping do let(:merge_request) { create(:merge_request) } let(:record) { merge_request.metrics } diff --git a/spec/models/merge_request_assignee_spec.rb b/spec/models/merge_request_assignee_spec.rb index 73bf7d02468..fd13027bcc4 100644 --- a/spec/models/merge_request_assignee_spec.rb +++ b/spec/models/merge_request_assignee_spec.rb @@ -28,9 +28,9 @@ RSpec.describe MergeRequestAssignee do context 'in_projects' do it 'returns issue assignees for given project' do - expect(MergeRequestAssignee.count).to eq 2 + expect(described_class.count).to eq 2 - assignees = MergeRequestAssignee.in_projects([project]) + assignees = described_class.in_projects([project]) expect(assignees.count).to eq 1 expect(assignees.first.user_id).to eq project_merge_request.merge_request_assignees.first.user_id diff --git a/spec/models/merge_request_diff_commit_spec.rb b/spec/models/merge_request_diff_commit_spec.rb index 78f9fb5b7d3..5cb96809970 100644 --- a/spec/models/merge_request_diff_commit_spec.rb +++ b/spec/models/merge_request_diff_commit_spec.rb @@ -6,7 +6,7 @@ RSpec.describe MergeRequestDiffCommit, feature_category: :code_review_workflow d let(:merge_request) { create(:merge_request) } let(:project) { merge_request.project } - it_behaves_like 'a BulkInsertSafe model', MergeRequestDiffCommit do + it_behaves_like 'a BulkInsertSafe model', described_class do let(:valid_items_for_bulk_insertion) do build_list(:merge_request_diff_commit, 10) do |mr_diff_commit| mr_diff_commit.merge_request_diff = create(:merge_request_diff) @@ -82,7 +82,7 @@ RSpec.describe MergeRequestDiffCommit, feature_category: :code_review_workflow d described_class.create_bulk(diff.id, [commits.first]) - commit_row = MergeRequestDiffCommit + commit_row = described_class .find_by(merge_request_diff_id: diff.id, relative_order: 0) commit_user_row = diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb index eee7fe67ffb..1ad9de78c92 100644 --- a/spec/models/merge_request_diff_file_spec.rb +++ b/spec/models/merge_request_diff_file_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe MergeRequestDiffFile, feature_category: :code_review_workflow do - it_behaves_like 'a BulkInsertSafe model', MergeRequestDiffFile do + it_behaves_like 'a BulkInsertSafe model', described_class do let(:valid_items_for_bulk_insertion) do build_list(:merge_request_diff_file, 10) do |mr_diff_file| mr_diff_file.merge_request_diff = create(:merge_request_diff) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index e16f7a94eb7..bf71d289105 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -415,8 +415,8 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev it 'does not create duplicated metrics records when MR is concurrently updated' do merge_request.metrics.destroy! - instance1 = MergeRequest.find(merge_request.id) - instance2 = MergeRequest.find(merge_request.id) + instance1 = described_class.find(merge_request.id) + instance2 = described_class.find(merge_request.id) instance1.ensure_metrics! instance2.ensure_metrics! @@ -3259,6 +3259,20 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end + describe '#skipped_mergeable_checks' do + subject { build_stubbed(:merge_request).skipped_mergeable_checks(options) } + + where(:options, :skip_ci_check) do + {} | false + { auto_merge_requested: false } | false + { auto_merge_requested: true } | true + end + + with_them do + it { is_expected.to include(skip_ci_check: skip_ci_check) } + end + end + describe '#check_mergeability' do let(:mergeability_service) { double } @@ -5105,6 +5119,32 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end + describe '#schedule_cleanup_refs' do + subject { merge_request.schedule_cleanup_refs(only: :train) } + + let(:merge_request) { build(:merge_request, source_project: create(:project, :repository)) } + + it 'does schedule MergeRequests::CleanupRefWorker' do + expect(MergeRequests::CleanupRefWorker).to receive(:perform_async).with(merge_request.id, 'train') + + subject + end + + context 'when merge_request_cleanup_ref_worker_async is disabled' do + before do + stub_feature_flags(merge_request_cleanup_ref_worker_async: false) + end + + it 'deletes all refs from the target project' do + expect(merge_request.target_project.repository) + .to receive(:delete_refs) + .with(merge_request.train_ref_path) + + subject + end + end + end + describe '#cleanup_refs' do subject { merge_request.cleanup_refs(only: only) } @@ -5272,7 +5312,7 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev environment: envs[2] ) - merge_request_relation = MergeRequest.where(id: merge_request.id) + merge_request_relation = described_class.where(id: merge_request.id) created.link_merge_requests(merge_request_relation) success.link_merge_requests(merge_request_relation) failed.link_merge_requests(merge_request_relation) diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 1c43eafb576..1f0f89fea60 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Milestone do +RSpec.describe Milestone, feature_category: :team_planning do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :public) } let_it_be(:group) { create(:group) } @@ -146,11 +146,11 @@ RSpec.describe Milestone do let_it_be(:milestone) { create(:milestone, project: project) } it 'returns true for a predefined Milestone ID' do - expect(Milestone.predefined_id?(described_class::Upcoming.id)).to be true + expect(described_class.predefined_id?(described_class::Upcoming.id)).to be true end it 'returns false for a Milestone ID that is not predefined' do - expect(Milestone.predefined_id?(milestone.id)).to be false + expect(described_class.predefined_id?(milestone.id)).to be false end end @@ -732,4 +732,44 @@ RSpec.describe Milestone do expect(milestone.lock_version).to be_present end end + + describe '#check_for_spam?' do + let_it_be(:milestone) { build_stubbed(:milestone, project: project) } + + subject { milestone.check_for_spam? } + + context 'when spammable attribute title has changed' do + before do + milestone.title = 'New title' + end + + it { is_expected.to eq(true) } + end + + context 'when spammable attribute description has changed' do + before do + milestone.description = 'New description' + end + + it { is_expected.to eq(true) } + end + + context 'when spammable attribute has changed but parent is private' do + before do + milestone.title = 'New title' + milestone.parent.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) + end + + it { is_expected.to eq(false) } + end + + context 'when no spammable attribute has changed' do + before do + milestone.title = milestone.title_was + milestone.description = milestone.description_was + end + + it { is_expected.to eq(false) } + end + end end diff --git a/spec/models/ml/experiment_spec.rb b/spec/models/ml/experiment_spec.rb index 9738a88b5b8..1ee35d6da03 100644 --- a/spec/models/ml/experiment_spec.rb +++ b/spec/models/ml/experiment_spec.rb @@ -14,6 +14,21 @@ RSpec.describe Ml::Experiment, feature_category: :mlops do it { is_expected.to belong_to(:user) } it { is_expected.to have_many(:candidates) } it { is_expected.to have_many(:metadata) } + it { is_expected.to belong_to(:model).class_name('Ml::Model') } + end + + describe '#destroy' do + it 'allow experiment without model to be destroyed' do + experiment = create(:ml_experiments, project: exp.project) + + expect { experiment.destroy! }.to change { Ml::Experiment.count }.by(-1) + end + + it 'throws error when destroying experiment with model' do + experiment = create(:ml_models, project: exp.project).default_experiment + + expect { experiment.destroy! }.to raise_error(ActiveRecord::ActiveRecordError) + end end describe '.package_name' do diff --git a/spec/models/ml/model_spec.rb b/spec/models/ml/model_spec.rb new file mode 100644 index 00000000000..397ea23dd85 --- /dev/null +++ b/spec/models/ml/model_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ml::Model, feature_category: :mlops do + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to have_one(:default_experiment) } + it { is_expected.to have_many(:versions) } + end + + describe '#valid?' do + using RSpec::Parameterized::TableSyntax + + let_it_be(:project) { create(:project) } + let_it_be(:existing_model) { create(:ml_models, name: 'an_existing_model', project: project) } + let_it_be(:valid_name) { 'a_valid_name' } + let_it_be(:default_experiment) { create(:ml_experiments, name: valid_name, project: project) } + + let(:name) { valid_name } + + subject(:errors) do + m = described_class.new(name: name, project: project, default_experiment: default_experiment) + m.validate + m.errors + end + + it 'validates a valid model version' do + expect(errors).to be_empty + end + + describe 'name' do + where(:ctx, :name) do + 'name is blank' | '' + 'name is not valid package name' | '!!()()' + 'name is too large' | ('a' * 256) + 'name is not unique in the project' | 'an_existing_model' + end + with_them do + it { expect(errors).to include(:name) } + end + end + + describe 'default_experiment' do + context 'when experiment name name is different than model name' do + before do + allow(default_experiment).to receive(:name).and_return("#{name}a") + end + + it { expect(errors).to include(:default_experiment) } + end + + context 'when model version project is different than model project' do + before do + allow(default_experiment).to receive(:project_id).and_return(project.id + 1) + end + + it { expect(errors).to include(:default_experiment) } + end + end + end +end diff --git a/spec/models/ml/model_version_spec.rb b/spec/models/ml/model_version_spec.rb new file mode 100644 index 00000000000..ef53a1ac3a0 --- /dev/null +++ b/spec/models/ml/model_version_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ml::ModelVersion, feature_category: :mlops do + using RSpec::Parameterized::TableSyntax + + let_it_be(:base_project) { create(:project) } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:model) } + it { is_expected.to belong_to(:package) } + end + + describe 'validation' do + let_it_be(:valid_version) { 'valid_version' } + let_it_be(:model) { create(:ml_models, project: base_project) } + let_it_be(:valid_package) do + build_stubbed(:ml_model_package, project: base_project, version: valid_version, name: model.name) + end + + let(:package) { valid_package } + let(:version) { valid_version } + + subject(:errors) do + mv = described_class.new(version: version, model: model, package: package, project: model.project) + mv.validate + mv.errors + end + + it 'validates a valid model version' do + expect(errors).to be_empty + end + + describe 'version' do + where(:ctx, :version) do + 'version is blank' | '' + 'version is not valid package version' | '!!()()' + 'version is too large' | ('a' * 256) + end + with_them do + it { expect(errors).to include(:version) } + end + + context 'when version is not unique in project+name' do + let_it_be(:existing_model_version) do + create(:ml_model_versions, model: model) + end + + let(:version) { existing_model_version.version } + + it { expect(errors).to include(:version) } + end + end + + describe 'model' do + context 'when project is different' do + before do + allow(model).to receive(:project_id).and_return(non_existing_record_id) + end + + it { expect(errors[:model]).to include('model project must be the same') } + end + end + + describe 'package' do + where(:property, :value, :error_message) do + :name | 'another_name' | 'package name must be the same' + :version | 'another_version' | 'package version must be the same' + :project_id | 0 | 'package project must be the same' + end + with_them do + before do + allow(package).to receive(property).and_return(:value) + end + + 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: model.name, version: valid_version) + end + + it { expect(errors[:package]).to include('package must be ml_model') } + end + end + end +end diff --git a/spec/models/namespace/package_setting_spec.rb b/spec/models/namespace/package_setting_spec.rb index 72ecad42a70..9dfb58301b1 100644 --- a/spec/models/namespace/package_setting_spec.rb +++ b/spec/models/namespace/package_setting_spec.rb @@ -11,11 +11,9 @@ RSpec.describe Namespace::PackageSetting do it { is_expected.to validate_presence_of(:namespace) } describe '#maven_duplicates_allowed' do - it { is_expected.to allow_value(true).for(:maven_duplicates_allowed) } - it { is_expected.to allow_value(false).for(:maven_duplicates_allowed) } + it { is_expected.to allow_value(true, false).for(:maven_duplicates_allowed) } it { is_expected.not_to allow_value(nil).for(:maven_duplicates_allowed) } - it { is_expected.to allow_value(true).for(:generic_duplicates_allowed) } - it { is_expected.to allow_value(false).for(:generic_duplicates_allowed) } + it { is_expected.to allow_value(true, false).for(:generic_duplicates_allowed) } it { is_expected.not_to allow_value(nil).for(:generic_duplicates_allowed) } end diff --git a/spec/models/namespace/root_storage_statistics_spec.rb b/spec/models/namespace/root_storage_statistics_spec.rb index a7f21e3a07f..f2c661c1cfb 100644 --- a/spec/models/namespace/root_storage_statistics_spec.rb +++ b/spec/models/namespace/root_storage_statistics_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Namespace::RootStorageStatistics, type: :model do +RSpec.describe Namespace::RootStorageStatistics, type: :model, feature_category: :consumables_cost_management do it { is_expected.to belong_to :namespace } it { is_expected.to have_one(:route).through(:namespace) } @@ -123,10 +123,21 @@ RSpec.describe Namespace::RootStorageStatistics, type: :model do let_it_be(:group1) { create(:group, parent: root_group) } let_it_be(:subgroup1) { create(:group, parent: group1) } let_it_be(:group2) { create(:group, parent: root_group) } - let_it_be(:root_namespace_stat) { create(:namespace_statistics, namespace: root_group, storage_size: 100, dependency_proxy_size: 100) } - let_it_be(:group1_namespace_stat) { create(:namespace_statistics, namespace: group1, storage_size: 200, dependency_proxy_size: 200) } - let_it_be(:group2_namespace_stat) { create(:namespace_statistics, namespace: group2, storage_size: 300, dependency_proxy_size: 300) } - let_it_be(:subgroup1_namespace_stat) { create(:namespace_statistics, namespace: subgroup1, storage_size: 300, dependency_proxy_size: 100) } + let_it_be(:root_namespace_stat) do + create(:namespace_statistics, namespace: root_group, storage_size: 100, dependency_proxy_size: 100) + end + + let_it_be(:group1_namespace_stat) do + create(:namespace_statistics, namespace: group1, storage_size: 200, dependency_proxy_size: 200) + end + + let_it_be(:group2_namespace_stat) do + create(:namespace_statistics, namespace: group2, storage_size: 300, dependency_proxy_size: 300) + end + + let_it_be(:subgroup1_namespace_stat) do + create(:namespace_statistics, namespace: subgroup1, storage_size: 300, dependency_proxy_size: 100) + end let(:namespace) { root_group } @@ -148,8 +159,12 @@ RSpec.describe Namespace::RootStorageStatistics, type: :model do total_pipeline_artifacts_size = project_stat1.pipeline_artifacts_size + project_stat2.pipeline_artifacts_size total_uploads_size = project_stat1.uploads_size + project_stat2.uploads_size total_wiki_size = project_stat1.wiki_size + project_stat2.wiki_size - total_dependency_proxy_size = root_namespace_stat.dependency_proxy_size + group1_namespace_stat.dependency_proxy_size + group2_namespace_stat.dependency_proxy_size + subgroup1_namespace_stat.dependency_proxy_size - total_storage_size = project_stat1.storage_size + project_stat2.storage_size + root_namespace_stat.storage_size + group1_namespace_stat.storage_size + group2_namespace_stat.storage_size + subgroup1_namespace_stat.storage_size + total_dependency_proxy_size = root_namespace_stat.dependency_proxy_size + + group1_namespace_stat.dependency_proxy_size + group2_namespace_stat.dependency_proxy_size + + subgroup1_namespace_stat.dependency_proxy_size + total_storage_size = project_stat1.storage_size + project_stat2.storage_size + + root_namespace_stat.storage_size + group1_namespace_stat.storage_size + + group2_namespace_stat.storage_size + subgroup1_namespace_stat.storage_size expect(root_storage_statistics.repository_size).to eq(total_repository_size) expect(root_storage_statistics.lfs_objects_size).to eq(total_lfs_objects_size) @@ -209,7 +224,8 @@ RSpec.describe Namespace::RootStorageStatistics, type: :model do root_storage_statistics.recalculate! - expect(root_storage_statistics.snippets_size).to eq(total_personal_snippets_size + total_project_snippets_size) + total = total_personal_snippets_size + total_project_snippets_size + expect(root_storage_statistics.snippets_size).to eq(total) end context 'when personal snippets do not have statistics' do @@ -219,7 +235,8 @@ RSpec.describe Namespace::RootStorageStatistics, type: :model do root_storage_statistics.recalculate! - expect(root_storage_statistics.snippets_size).to eq(total_project_snippets_size + snippets.last.statistics.repository_size) + total = total_project_snippets_size + snippets.last.statistics.repository_size + expect(root_storage_statistics.snippets_size).to eq(total) end end end @@ -293,7 +310,9 @@ RSpec.describe Namespace::RootStorageStatistics, type: :model do root_storage_statistics.reload expect(root_storage_statistics.private_forks_storage_size).to eq(project_fork.statistics.storage_size) - expect(root_storage_statistics.storage_size).to eq(project.statistics.storage_size + project_fork.statistics.storage_size) + + total = project.statistics.storage_size + project_fork.statistics.storage_size + expect(root_storage_statistics.storage_size).to eq(total) end it 'sets the public forks storage size back to zero' do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 3d7d5062ca7..1c02b4754fa 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -15,6 +15,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do let(:repository_storage) { 'default' } describe 'associations' do + it { is_expected.to belong_to :organization } it { is_expected.to have_many :projects } it { is_expected.to have_many :project_statistics } it { is_expected.to belong_to :parent } @@ -375,7 +376,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do describe 'handling STI', :aggregate_failures do let(:namespace_type) { nil } let(:parent) { nil } - let(:namespace) { Namespace.find(create(:namespace, type: namespace_type, parent: parent).id) } + let(:namespace) { described_class.find(create(:namespace, type: namespace_type, parent: parent).id) } context 'creating a Group' do let(:namespace_type) { group_sti_name } @@ -392,7 +393,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do let(:parent) { create(:group) } it 'is the correct type of namespace' do - expect(Namespace.find(namespace.id)).to be_a(Namespaces::ProjectNamespace) + expect(described_class.find(namespace.id)).to be_a(Namespaces::ProjectNamespace) expect(namespace.kind).to eq('project') expect(namespace.project_namespace?).to be_truthy end @@ -402,7 +403,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do let(:namespace_type) { user_sti_name } it 'is the correct type of namespace' do - expect(Namespace.find(namespace.id)).to be_a(Namespaces::UserNamespace) + expect(described_class.find(namespace.id)).to be_a(Namespaces::UserNamespace) expect(namespace.kind).to eq('user') expect(namespace.user_namespace?).to be_truthy end @@ -421,7 +422,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do let(:namespace_type) { 'nonsense' } it 'creates a default Namespace' do - expect(Namespace.find(namespace.id)).to be_a(Namespace) + expect(described_class.find(namespace.id)).to be_a(described_class) expect(namespace.kind).to eq('user') expect(namespace.user_namespace?).to be_truthy end @@ -587,7 +588,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end it 'returns value that matches database' do - expect(namespace.traversal_ids).to eq Namespace.find(namespace.id).traversal_ids + expect(namespace.traversal_ids).to eq described_class.find(namespace.id).traversal_ids end end @@ -598,7 +599,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end it 'returns database value' do - expect(namespace.traversal_ids).to eq Namespace.find(namespace.id).traversal_ids + expect(namespace.traversal_ids).to eq described_class.find(namespace.id).traversal_ids end end @@ -684,14 +685,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do it_behaves_like 'makes recursive queries' end - - context 'when feature flag :use_traversal_ids_for_descendants_scopes is disabled' do - before do - stub_feature_flags(use_traversal_ids_for_descendants_scopes: false) - end - - it_behaves_like 'makes recursive queries' - end end describe '.self_and_descendant_ids' do @@ -708,14 +701,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do it_behaves_like 'makes recursive queries' end - - context 'when feature flag :use_traversal_ids_for_descendants_scopes is disabled' do - before do - stub_feature_flags(use_traversal_ids_for_descendants_scopes: false) - end - - it_behaves_like 'makes recursive queries' - end end end @@ -889,6 +874,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do context 'when Gitlab API is supported' do before do + allow(Gitlab).to receive(:com_except_jh?).and_return(true) allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(true) stub_container_registry_config(enabled: true, api_url: 'http://container-registry', key: 'spec/fixtures/x509_certificate_pk.key') end @@ -931,7 +917,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do before do allow(ContainerRegistry::GitlabApiClient).to receive(:one_project_with_container_registry_tag).and_return(nil) stub_container_registry_config(enabled: true, api_url: 'http://container-registry', key: 'spec/fixtures/x509_certificate_pk.key') - allow(Gitlab).to receive(:com?).and_return(true) + allow(Gitlab).to receive(:com_except_jh?).and_return(true) allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(gitlab_api_supported) allow(project_namespace).to receive_message_chain(:all_container_repositories, :empty?).and_return(no_container_repositories) allow(project_namespace).to receive_message_chain(:all_container_repositories, :all_migrated?).and_return(all_migrated) @@ -1142,8 +1128,9 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do project1 project2 statistics = described_class.with_statistics.find(namespace.id) + expected_storage_size = project1.statistics.storage_size + project2.statistics.storage_size - expect(statistics.storage_size).to eq 3995 + expect(statistics.storage_size).to eq expected_storage_size expect(statistics.repository_size).to eq 111 expect(statistics.wiki_size).to eq 555 expect(statistics.lfs_objects_size).to eq 222 @@ -1606,96 +1593,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end - describe '#use_traversal_ids_for_ancestors?' do - let_it_be(:namespace, reload: true) { create(:namespace) } - - subject { namespace.use_traversal_ids_for_ancestors? } - - context 'when use_traversal_ids_for_ancestors? feature flag is true' do - before do - stub_feature_flags(use_traversal_ids_for_ancestors: true) - end - - it { is_expected.to eq true } - - it_behaves_like 'disabled feature flag when traversal_ids is blank' - end - - context 'when use_traversal_ids_for_ancestors? feature flag is false' do - before do - stub_feature_flags(use_traversal_ids_for_ancestors: false) - end - - it { is_expected.to eq false } - end - - context 'when use_traversal_ids? feature flag is false' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it { is_expected.to eq false } - end - end - - describe '#use_traversal_ids_for_ancestors_upto?' do - let_it_be(:namespace, reload: true) { create(:namespace) } - - subject { namespace.use_traversal_ids_for_ancestors_upto? } - - context 'when use_traversal_ids_for_ancestors_upto feature flag is true' do - before do - stub_feature_flags(use_traversal_ids_for_ancestors_upto: true) - end - - it { is_expected.to eq true } - - it_behaves_like 'disabled feature flag when traversal_ids is blank' - end - - context 'when use_traversal_ids_for_ancestors_upto feature flag is false' do - before do - stub_feature_flags(use_traversal_ids_for_ancestors_upto: false) - end - - it { is_expected.to eq false } - end - - context 'when use_traversal_ids? feature flag is false' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it { is_expected.to eq false } - end - end - - describe '#use_traversal_ids_for_self_and_hierarchy?' do - let_it_be(:namespace, reload: true) { create(:namespace) } - - subject { namespace.use_traversal_ids_for_self_and_hierarchy? } - - it { is_expected.to eq true } - - it_behaves_like 'disabled feature flag when traversal_ids is blank' - - context 'when use_traversal_ids_for_self_and_hierarchy feature flag is false' do - before do - stub_feature_flags(use_traversal_ids_for_self_and_hierarchy: false) - end - - it { is_expected.to eq false } - end - - context 'when use_traversal_ids? feature flag is false' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it { is_expected.to eq false } - end - end - describe '#users_with_descendants' do let(:user_a) { create(:user) } let(:user_b) { create(:user) } @@ -2747,4 +2644,11 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end end + + context 'with loose foreign key on organization_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:organization) } + let!(:model) { create(:namespace, organization: parent) } + end + end end diff --git a/spec/models/oauth_access_token_spec.rb b/spec/models/oauth_access_token_spec.rb index 5fa590eab58..b21a2bf2079 100644 --- a/spec/models/oauth_access_token_spec.rb +++ b/spec/models/oauth_access_token_spec.rb @@ -32,7 +32,7 @@ RSpec.describe OauthAccessToken do end it 'finds a token by plaintext token' do - expect(described_class.by_token(token.plaintext_token)).to be_a(OauthAccessToken) + expect(described_class.by_token(token.plaintext_token)).to be_a(described_class) end context 'when the token is stored in plaintext' do @@ -43,7 +43,7 @@ RSpec.describe OauthAccessToken do end it 'falls back to plaintext token comparison' do - expect(described_class.by_token(plaintext_token)).to be_a(OauthAccessToken) + expect(described_class.by_token(plaintext_token)).to be_a(described_class) end end end @@ -57,7 +57,7 @@ RSpec.describe OauthAccessToken do describe '#expires_in' do context 'when token has expires_in value set' do it 'uses the expires_in value' do - token = OauthAccessToken.new(expires_in: 1.minute) + token = described_class.new(expires_in: 1.minute) expect(token).to be_valid end @@ -65,7 +65,7 @@ RSpec.describe OauthAccessToken do context 'when token has nil expires_in' do it 'uses default value' do - token = OauthAccessToken.new(expires_in: nil) + token = described_class.new(expires_in: nil) expect(token).to be_invalid end diff --git a/spec/models/organizations/organization_setting_spec.rb b/spec/models/organizations/organization_setting_spec.rb new file mode 100644 index 00000000000..376d0b7fe77 --- /dev/null +++ b/spec/models/organizations/organization_setting_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Organizations::OrganizationSetting, type: :model, feature_category: :cell do + let_it_be(:organization) { create(:organization) } + + describe 'associations' do + it { is_expected.to belong_to :organization } + end + + describe 'validations' do + context 'for json schema' do + let(:restricted_visibility_levels) { [] } + let(:settings) do + { + restricted_visibility_levels: restricted_visibility_levels + } + end + + it { is_expected.to allow_value(settings).for(:settings) } + + context 'when trying to store an unsupported key' do + let(:settings) do + { + unsupported_key: 'some_value' + } + end + + it { is_expected.not_to allow_value(settings).for(:settings) } + end + + context "when key 'restricted_visibility_levels' is invalid" do + let(:restricted_visibility_levels) { ['some_string'] } + + it { is_expected.not_to allow_value(settings).for(:settings) } + end + end + + context 'when setting restricted_visibility_levels' do + it 'is one or more of Gitlab::VisibilityLevel constants' do + setting = build(:organization_setting) + + setting.restricted_visibility_levels = [123] + + expect(setting.valid?).to be false + expect(setting.errors.full_messages).to include( + "Restricted visibility levels '123' is not a valid visibility level" + ) + + setting.restricted_visibility_levels = [Gitlab::VisibilityLevel::PUBLIC, Gitlab::VisibilityLevel::PRIVATE, + Gitlab::VisibilityLevel::INTERNAL] + expect(setting.valid?).to be true + end + end + end +end diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index 4a75f352b6f..a9cac30e9a1 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -6,6 +6,13 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel let_it_be(:organization) { create(:organization) } let_it_be(:default_organization) { create(:organization, :default) } + describe 'associations' do + it { is_expected.to have_many :namespaces } + it { is_expected.to have_many :groups } + it { is_expected.to have_many(:users).through(:organization_users).inverse_of(:organizations) } + it { is_expected.to have_many(:organization_users).inverse_of(:organization) } + end + describe 'validations' do subject { create(:organization) } @@ -141,4 +148,18 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel expect(organization.to_param).to eq('org_path') end end + + context 'on deleting organizations via SQL' do + it 'does not allow to delete default organization' do + expect { default_organization.delete }.to raise_error( + ActiveRecord::StatementInvalid, /Deletion of the default Organization is not allowed/ + ) + end + + it 'allows to delete any other organization' do + organization.delete + + expect(described_class.where(id: organization)).not_to exist + end + end end diff --git a/spec/models/organizations/organization_user_spec.rb b/spec/models/organizations/organization_user_spec.rb new file mode 100644 index 00000000000..392ffa1b5be --- /dev/null +++ b/spec/models/organizations/organization_user_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Organizations::OrganizationUser, type: :model, feature_category: :cell do + describe 'associations' do + it { is_expected.to belong_to(:organization).inverse_of(:organization_users).required } + it { is_expected.to belong_to(:user).inverse_of(:organization_users).required } + end +end diff --git a/spec/models/packages/dependency_spec.rb b/spec/models/packages/dependency_spec.rb index 80ec7f77fda..9918a2bdb14 100644 --- a/spec/models/packages/dependency_spec.rb +++ b/spec/models/packages/dependency_spec.rb @@ -27,7 +27,7 @@ RSpec.describe Packages::Dependency, type: :model, feature_category: :package_re let(:chunk_size) { 50 } let(:rows_limit) { 50 } - subject { Packages::Dependency.ids_for_package_names_and_version_patterns(names_and_version_patterns, chunk_size, rows_limit) } + subject { described_class.ids_for_package_names_and_version_patterns(names_and_version_patterns, chunk_size, rows_limit) } it { is_expected.to match_array(expected_ids) } @@ -97,7 +97,7 @@ RSpec.describe Packages::Dependency, type: :model, feature_category: :package_re let(:names_and_version_patterns) { build_names_and_version_patterns(package_dependency1, package_dependency2) } - subject { Packages::Dependency.for_package_names_and_version_patterns(names_and_version_patterns) } + subject { described_class.for_package_names_and_version_patterns(names_and_version_patterns) } it { is_expected.to match_array(expected_array) } diff --git a/spec/models/packages/maven/metadatum_spec.rb b/spec/models/packages/maven/metadatum_spec.rb index 0000543cb18..ef55bcdcd20 100644 --- a/spec/models/packages/maven/metadatum_spec.rb +++ b/spec/models/packages/maven/metadatum_spec.rb @@ -43,7 +43,7 @@ RSpec.describe Packages::Maven::Metadatum, type: :model do describe '.for_package_ids' do let_it_be(:metadata) { create_list(:maven_metadatum, 3, package: package) } - subject { Packages::Maven::Metadatum.for_package_ids(package.id) } + subject { described_class.for_package_ids(package.id) } it { is_expected.to match_array(metadata) } end diff --git a/spec/models/packages/npm/metadatum_spec.rb b/spec/models/packages/npm/metadatum_spec.rb index 418194bffdd..e5586dca15c 100644 --- a/spec/models/packages/npm/metadatum_spec.rb +++ b/spec/models/packages/npm/metadatum_spec.rb @@ -38,7 +38,9 @@ RSpec.describe Packages::Npm::Metadatum, type: :model, feature_category: :packag it { is_expected.not_to allow_value({}).for(:package_json) } - it { is_expected.not_to allow_value(test: 'test' * 10000).for(:package_json) } + it { + is_expected.not_to allow_value(test: 'test' * 10000).for(:package_json).with_message(/structure is too large/) + } def with_dist valid_json.tap do |h| diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 90a5d815427..120b7d72cd9 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -804,15 +804,6 @@ RSpec.describe Packages::Package, type: :model, feature_category: :package_regis let!(:package2) { create(:npm_package, version: '1.0.1') } let!(:package3) { create(:npm_package, version: '1.0.1') } - describe '.last_of_each_version' do - subject { described_class.last_of_each_version } - - it 'includes only latest package per version' do - is_expected.to include(package1, package3) - is_expected.not_to include(package2) - end - end - describe '.has_version' do subject { described_class.has_version } @@ -1023,6 +1014,32 @@ RSpec.describe Packages::Package, type: :model, feature_category: :package_regis end end + describe '.select_only_first_by_name' do + let_it_be(:project) { create(:project) } + let_it_be(:package1) { create(:package, name: 'p1', created_at: 1000, project: project) } + let_it_be(:package2) { create(:package, name: 'p1', created_at: 1001, project: project) } + let_it_be(:package3) { create(:package, name: 'p2', project: project) } + + subject { described_class.order_name_desc_version_desc.select_only_first_by_name } + + it 'returns only the most recent package by name' do + is_expected.to eq([package3, package2]) + end + end + + describe '.order_name_desc_version_desc' do + let_it_be(:project) { create(:project) } + let_it_be(:package1) { create(:package, name: 'p1', created_at: 1000, project: project) } + let_it_be(:package2) { create(:package, name: 'p1', created_at: 1001, project: project) } + let_it_be(:package3) { create(:package, name: 'p2', project: project) } + + subject { described_class.order_name_desc_version_desc } + + it 'sorts packages by name desc and created desc' do + is_expected.to eq([package3, package2, package1]) + end + end + context 'sorting' do let_it_be(:project) { create(:project, name: 'aaa') } let_it_be(:project2) { create(:project, name: 'bbb') } @@ -1032,22 +1049,22 @@ RSpec.describe Packages::Package, type: :model, feature_category: :package_regis let_it_be(:package4) { create(:package, project: project) } it 'orders packages by their projects name ascending' do - expect(Packages::Package.order_project_name).to eq([package1, package4, package2, package3]) + expect(described_class.order_project_name).to eq([package1, package4, package2, package3]) end it 'orders packages by their projects name descending' do - expect(Packages::Package.order_project_name_desc).to eq([package2, package3, package1, package4]) + expect(described_class.order_project_name_desc).to eq([package2, package3, package1, package4]) end shared_examples 'order_project_path scope' do it 'orders packages by their projects path asc, then package id asc' do - expect(Packages::Package.order_project_path).to eq([package1, package4, package2, package3]) + expect(described_class.order_project_path).to eq([package1, package4, package2, package3]) end end shared_examples 'order_project_path_desc scope' do it 'orders packages by their projects path desc, then package id desc' do - expect(Packages::Package.order_project_path_desc).to eq([package3, package2, package4, package1]) + expect(described_class.order_project_path_desc).to eq([package3, package2, package4, package1]) end end diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 88fd1bd9e56..62152f9d3a4 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -8,7 +8,12 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do subject(:lookup_path) { described_class.new(project) } before do - stub_pages_setting(access_control: true, external_https: ["1.1.1.1:443"]) + stub_pages_setting( + access_control: true, + external_https: ["1.1.1.1:443"], + url: 'http://example.com', + protocol: 'http' + ) stub_pages_object_storage(::Pages::DeploymentUploader) end @@ -120,18 +125,14 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do describe '#prefix' do it 'returns "/" for pages group root projects' do - project = instance_double(Project, pages_namespace_url: "namespace.test", pages_url: "namespace.test") + project = instance_double(Project, full_path: "namespace/namespace.example.com") lookup_path = described_class.new(project, trim_prefix: 'mygroup') expect(lookup_path.prefix).to eq('/') end it 'returns the project full path with the provided prefix removed' do - project = instance_double( - Project, - pages_namespace_url: "namespace.test", - pages_url: "namespace.other", - full_path: 'mygroup/myproject') + project = instance_double(Project, full_path: 'mygroup/myproject') lookup_path = described_class.new(project, trim_prefix: 'mygroup') expect(lookup_path.prefix).to eq('/myproject/') diff --git a/spec/models/pages_deployment_spec.rb b/spec/models/pages_deployment_spec.rb index 767db511d85..553491f6eff 100644 --- a/spec/models/pages_deployment_spec.rb +++ b/spec/models/pages_deployment_spec.rb @@ -184,7 +184,7 @@ RSpec.describe PagesDeployment, feature_category: :pages do # new deployment create(:pages_deployment) - expect(PagesDeployment.older_than(deployment.id)).to eq(old_deployments) + expect(described_class.older_than(deployment.id)).to eq(old_deployments) end end end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 2c63306bd0a..3030756a413 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -579,7 +579,7 @@ RSpec.describe PagesDomain do it 'lookup is case-insensitive' do pages_domain = create(:pages_domain, domain: "Pages.IO") - expect(PagesDomain.find_by_domain_case_insensitive('pages.io')).to eq(pages_domain) + expect(described_class.find_by_domain_case_insensitive('pages.io')).to eq(pages_domain) end end end diff --git a/spec/models/performance_monitoring/prometheus_dashboard_spec.rb b/spec/models/performance_monitoring/prometheus_dashboard_spec.rb index 21b16bdeb17..f338e5439ad 100644 --- a/spec/models/performance_monitoring/prometheus_dashboard_spec.rb +++ b/spec/models/performance_monitoring/prometheus_dashboard_spec.rb @@ -32,7 +32,7 @@ RSpec.describe PerformanceMonitoring::PrometheusDashboard do subject { described_class.from_json(json_content) } it 'creates a PrometheusDashboard object' do - expect(subject).to be_a PerformanceMonitoring::PrometheusDashboard + expect(subject).to be_a described_class expect(subject.dashboard).to eq(json_content['dashboard']) expect(subject.panel_groups).to all(be_a PerformanceMonitoring::PrometheusPanelGroup) end diff --git a/spec/models/performance_monitoring/prometheus_metric_spec.rb b/spec/models/performance_monitoring/prometheus_metric_spec.rb index b5b9cd58aa8..58bb59793cf 100644 --- a/spec/models/performance_monitoring/prometheus_metric_spec.rb +++ b/spec/models/performance_monitoring/prometheus_metric_spec.rb @@ -16,7 +16,7 @@ RSpec.describe PerformanceMonitoring::PrometheusMetric do subject { described_class.from_json(json_content) } it 'creates a PrometheusMetric object' do - expect(subject).to be_a PerformanceMonitoring::PrometheusMetric + 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']) diff --git a/spec/models/performance_monitoring/prometheus_panel_group_spec.rb b/spec/models/performance_monitoring/prometheus_panel_group_spec.rb index 9e92cb27954..497f80483eb 100644 --- a/spec/models/performance_monitoring/prometheus_panel_group_spec.rb +++ b/spec/models/performance_monitoring/prometheus_panel_group_spec.rb @@ -24,7 +24,7 @@ RSpec.describe PerformanceMonitoring::PrometheusPanelGroup do subject { described_class.from_json(json_content) } it 'creates a PrometheusPanelGroup object' do - expect(subject).to be_a PerformanceMonitoring::PrometheusPanelGroup + 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 diff --git a/spec/models/performance_monitoring/prometheus_panel_spec.rb b/spec/models/performance_monitoring/prometheus_panel_spec.rb index c5c6b1fdafd..42dcbbdb8e0 100644 --- a/spec/models/performance_monitoring/prometheus_panel_spec.rb +++ b/spec/models/performance_monitoring/prometheus_panel_spec.rb @@ -33,7 +33,7 @@ RSpec.describe PerformanceMonitoring::PrometheusPanel do subject { described_class.from_json(json_content) } it 'creates a PrometheusPanelGroup object' do - expect(subject).to be_a PerformanceMonitoring::PrometheusPanel + 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']) diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 8e86518912c..7437e9b463e 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -21,6 +21,12 @@ RSpec.describe PersonalAccessToken, feature_category: :system_access do end end + describe 'associations' do + subject(:project_access_token) { create(:personal_access_token) } + + it { is_expected.to belong_to(:previous_personal_access_token).class_name('PersonalAccessToken') } + end + describe 'scopes' do describe '.project_access_tokens' do let_it_be(:user) { create(:user, :project_bot) } @@ -257,7 +263,7 @@ RSpec.describe PersonalAccessToken, feature_category: :system_access do end context 'validates expires_at' do - let(:max_expiration_date) { described_class::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now } + let(:max_expiration_date) { Date.current + described_class::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS } it "can't be blank" do personal_access_token.expires_at = nil @@ -274,12 +280,14 @@ RSpec.describe PersonalAccessToken, feature_category: :system_access do end end - context 'when expires_in is more than MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS days' do + context 'when expires_in is more than MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS days', :freeze_time do it 'is invalid' do personal_access_token.expires_at = max_expiration_date + 1.day expect(personal_access_token).not_to be_valid - expect(personal_access_token.errors[:expires_at].first).to eq('must expire in 365 days') + expect(personal_access_token.errors.full_messages.to_sentence).to eq( + "Expiration date must be before #{max_expiration_date}" + ) end end end diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index d211499e9e9..bee1c4f47b0 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -302,94 +302,136 @@ RSpec.describe PlanLimits do end end - describe '#log_limits_changes', :freeze_time do + describe '#format_limits_history', :freeze_time do let(:user) { create(:user) } let(:plan_limits) { create(:plan_limits) } let(:current_timestamp) { Time.current.utc.to_i } - let(:history) { plan_limits.limits_history } - it 'logs a single attribute change' do - plan_limits.log_limits_changes(user, enforcement_limit: 5_000) - - expect(history).to eq( - { 'enforcement_limit' => [{ 'user_id' => user.id, 'username' => user.username, - 'timestamp' => current_timestamp, 'value' => 5_000 }] } + it 'formats a single attribute change' do + formatted_limits_history = plan_limits.format_limits_history(user, enforcement_limit: 5_000) + + expect(formatted_limits_history).to eq( + { + "enforcement_limit" => [ + { + "user_id" => user.id, + "username" => user.username, + "timestamp" => current_timestamp, + "value" => 5000 + } + ] + } ) end - it 'logs multiple attribute changes' do - plan_limits.log_limits_changes(user, enforcement_limit: 10_000, notification_limit: 20_000) + it 'does not format limits_history for non-allowed attributes' do + formatted_limits_history = plan_limits.format_limits_history(user, + { enforcement_limit: 20_000, pipeline_hierarchy_size: 10_000 }) - expect(history).to eq( - { 'enforcement_limit' => [{ 'user_id' => user.id, 'username' => user.username, - 'timestamp' => current_timestamp, 'value' => 10_000 }], - 'notification_limit' => [{ 'user_id' => user.id, 'username' => user.username, - 'timestamp' => current_timestamp, - 'value' => 20_000 }] } - ) + expect(formatted_limits_history).to eq({ + "enforcement_limit" => [ + { + "user_id" => user.id, + "username" => user.username, + "timestamp" => current_timestamp, + "value" => 20_000 + } + ] + }) end - it 'allows logging dashboard_limit_enabled_at from console (without user)' do - plan_limits.log_limits_changes(nil, dashboard_limit_enabled_at: current_timestamp) + it 'does not format attributes for values that do not change' do + plan_limits.update!(enforcement_limit: 20_000) + formatted_limits_history = plan_limits.format_limits_history(user, enforcement_limit: 20_000) - expect(history).to eq( - { 'dashboard_limit_enabled_at' => [{ 'user_id' => nil, 'username' => nil, 'timestamp' => current_timestamp, - 'value' => current_timestamp }] } + expect(formatted_limits_history).to eq({}) + end + + it 'formats multiple attribute changes' do + formatted_limits_history = plan_limits.format_limits_history(user, enforcement_limit: 10_000, + notification_limit: 20_000, dashboard_limit_enabled_at: current_timestamp) + + expect(formatted_limits_history).to eq( + { + "notification_limit" => [ + { + "user_id" => user.id, + "username" => user.username, + "timestamp" => current_timestamp, + "value" => 20000 + } + ], + "enforcement_limit" => [ + { + "user_id" => user.id, + "username" => user.username, + "timestamp" => current_timestamp, + "value" => 10000 + } + ], + "dashboard_limit_enabled_at" => [ + { + "user_id" => user.id, + "username" => user.username, + "timestamp" => current_timestamp, + "value" => current_timestamp + } + ] + } ) end - context 'with previous history avilable' do + context 'with previous history available' do let(:plan_limits) do - create(:plan_limits, - limits_history: { 'enforcement_limit' => [{ user_id: user.id, username: user.username, - timestamp: current_timestamp, - value: 20_000 }, - { user_id: user.id, username: user.username, timestamp: current_timestamp, - value: 50_000 }] }) + create( + :plan_limits, + limits_history: { + 'enforcement_limit' => [ + { + user_id: user.id, + username: user.username, + timestamp: current_timestamp, + value: 20_000 + }, + { + user_id: user.id, + username: user.username, + timestamp: current_timestamp, + value: 50_000 + } + ] + } + ) end it 'appends to it' do - plan_limits.log_limits_changes(user, enforcement_limit: 60_000) - expect(history).to eq( + formatted_limits_history = plan_limits.format_limits_history(user, enforcement_limit: 60_000) + + expect(formatted_limits_history).to eq( { - 'enforcement_limit' => [ - { 'user_id' => user.id, 'username' => user.username, 'timestamp' => current_timestamp, - 'value' => 20_000 }, - { 'user_id' => user.id, 'username' => user.username, 'timestamp' => current_timestamp, - 'value' => 50_000 }, - { 'user_id' => user.id, 'username' => user.username, 'timestamp' => current_timestamp, 'value' => 60_000 } + "enforcement_limit" => [ + { + "user_id" => user.id, + "username" => user.username, + "timestamp" => current_timestamp, + "value" => 20000 + }, + { + "user_id" => user.id, + "username" => user.username, + "timestamp" => current_timestamp, + "value" => 50000 + }, + { + "user_id" => user.id, + "username" => user.username, + "timestamp" => current_timestamp, + "value" => 60000 + } ] } ) end end end - - describe '#limit_attribute_changes', :freeze_time do - let(:user) { create(:user) } - let(:current_timestamp) { Time.current.utc.to_i } - let(:plan_limits) do - create(:plan_limits, - limits_history: { 'enforcement_limit' => [ - { user_id: user.id, username: user.username, timestamp: current_timestamp, - value: 20_000 }, { user_id: user.id, username: user.username, timestamp: current_timestamp, - value: 50_000 } - ] }) - end - - it 'returns an empty array for attribute with no changes' do - changes = plan_limits.limit_attribute_changes(:notification_limit) - - expect(changes).to eq([]) - end - - it 'returns the changes for a specific attribute' do - changes = plan_limits.limit_attribute_changes(:enforcement_limit) - - expect(changes).to eq( - [{ timestamp: current_timestamp, value: 20_000, username: user.username, user_id: user.id }, - { timestamp: current_timestamp, value: 50_000, username: user.username, user_id: user.id }] - ) - end - end end diff --git a/spec/models/postgresql/detached_partition_spec.rb b/spec/models/postgresql/detached_partition_spec.rb index aaa99e842b4..d9e77b70368 100644 --- a/spec/models/postgresql/detached_partition_spec.rb +++ b/spec/models/postgresql/detached_partition_spec.rb @@ -4,15 +4,15 @@ require 'spec_helper' RSpec.describe Postgresql::DetachedPartition do describe '#ready_to_drop' do - let_it_be(:drop_before) { Postgresql::DetachedPartition.create!(drop_after: 1.day.ago, table_name: 'old_table') } - let_it_be(:drop_after) { Postgresql::DetachedPartition.create!(drop_after: 1.day.from_now, table_name: 'new_table') } + let_it_be(:drop_before) { described_class.create!(drop_after: 1.day.ago, table_name: 'old_table') } + let_it_be(:drop_after) { described_class.create!(drop_after: 1.day.from_now, table_name: 'new_table') } it 'includes partitions that should be dropped before now' do - expect(Postgresql::DetachedPartition.ready_to_drop.to_a).to include(drop_before) + expect(described_class.ready_to_drop.to_a).to include(drop_before) end it 'does not include partitions that should be dropped after now' do - expect(Postgresql::DetachedPartition.ready_to_drop.to_a).not_to include(drop_after) + expect(described_class.ready_to_drop.to_a).not_to include(drop_after) end end end diff --git a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb index a2ab59f56ab..17db284c61e 100644 --- a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb +++ b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb @@ -32,7 +32,7 @@ RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do context 'when user is present' do before do - Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects_arg, user).execute + described_class.new(projects_arg, user).execute end it 'avoids N+1 queries' do @@ -61,7 +61,7 @@ RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do context 'when user is not present' do before do - Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects_arg, nil).execute + described_class.new(projects_arg, nil).execute end it 'does not avoid N+1 queries' do diff --git a/spec/models/project_label_spec.rb b/spec/models/project_label_spec.rb index ba9ea759c6a..62839f5fb4f 100644 --- a/spec/models/project_label_spec.rb +++ b/spec/models/project_label_spec.rb @@ -107,14 +107,14 @@ RSpec.describe ProjectLabel do context 'using name' do it 'returns cross reference with label name' do expect(label.to_reference(project, format: :name)) - .to eq %Q(#{label.project.full_path}~"#{label.name}") + .to eq %(#{label.project.full_path}~"#{label.name}") end end context 'using id' do it 'returns cross reference with label id' do expect(label.to_reference(project, format: :id)) - .to eq %Q(#{label.project.full_path}~#{label.id}) + .to eq %(#{label.project.full_path}~#{label.id}) end end end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 4b2760d7699..6928cc8ba08 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -27,8 +27,7 @@ RSpec.describe ProjectSetting, type: :model, feature_category: :groups_and_proje it { is_expected.to validate_length_of(:issue_branch_template).is_at_most(255) } it { is_expected.not_to allow_value(nil).for(:suggested_reviewers_enabled) } - it { is_expected.to allow_value(true).for(:suggested_reviewers_enabled) } - it { is_expected.to allow_value(false).for(:suggested_reviewers_enabled) } + it { is_expected.to allow_value(true, false).for(:suggested_reviewers_enabled) } it 'allows any combination of the allowed target platforms' do valid_target_platform_combinations.each do |target_platforms| diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f44331521e9..044408e86e9 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -342,8 +342,8 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr context 'when same project is being updated in 2 instances' do it 'syncs only changed attributes' do - project1 = Project.last - project2 = Project.last + project1 = described_class.last + project2 = described_class.last project_name = project1.name project_path = project1.path @@ -2170,6 +2170,32 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end + describe '.with_slack_integration' do + it 'returns projects with both active and inactive slack integrations' do + create(:project) + with_active_slack = create(:integrations_slack).project + with_disabled_slack = create(:integrations_slack, active: false).project + + expect(described_class.with_slack_integration).to contain_exactly( + with_active_slack, + with_disabled_slack + ) + end + end + + describe '.with_slack_slash_commands_integration' do + it 'returns projects with both active and inactive slack slash commands integrations' do + create(:project) + with_active_slash_commands = create(:slack_slash_commands_integration).project + with_disabled_slash_commands = create(:slack_slash_commands_integration, active: false).project + + expect(described_class.with_slack_slash_commands_integration).to contain_exactly( + with_active_slash_commands, + with_disabled_slash_commands + ) + end + end + describe '.cached_count', :use_clean_rails_memory_store_caching do let(:group) { create(:group, :public) } let!(:project1) { create(:project, :public, group: group) } @@ -2382,11 +2408,11 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr create(:service_desk_setting, project_key: 'key2') create(:service_desk_setting) - expect(Project.with_service_desk_key('key1')).to contain_exactly(project1, project2) + expect(described_class.with_service_desk_key('key1')).to contain_exactly(project1, project2) end it 'returns empty if there is no project with the key' do - expect(Project.with_service_desk_key('key1')).to be_empty + expect(described_class.with_service_desk_key('key1')).to be_empty end end @@ -2439,7 +2465,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr create(:jira_integration, project: project_3, inherit_from_id: nil) create(:integrations_slack, project: project_4, inherit_from_id: nil) - expect(Project.without_integration(instance_integration)).to contain_exactly(project_4) + expect(described_class.without_integration(instance_integration)).to contain_exactly(project_4) end end @@ -2780,224 +2806,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#pages_url', feature_category: :pages do - let(:group_name) { 'group' } - let(:project_name) { 'project' } - - let(:group) { create(:group, name: group_name) } - let(:nested_group) { create(:group, parent: group) } - - let(:project_path) { project_name.downcase } - let(:project) do - create( - :project, - namespace: group, - name: project_name, - path: project_path) - end - - let(:domain) { 'Example.com' } - let(:port) { nil } - - subject { project.pages_url } - - before do - allow(Settings.pages).to receive(:host).and_return(domain) - allow(Gitlab.config.pages) - .to receive(:url) - .and_return(['http://example.com', port].compact.join(':')) - end - - context 'when not using pages_unique_domain' do - subject { project.pages_url(with_unique_domain: false) } - - context 'when pages_unique_domain feature flag is disabled' do - before do - stub_feature_flags(pages_unique_domain: false) - end - - it { is_expected.to eq('http://group.example.com/project') } - end - - context 'when pages_unique_domain feature flag is enabled' do - before do - stub_feature_flags(pages_unique_domain: true) - - project.project_setting.update!( - pages_unique_domain_enabled: pages_unique_domain_enabled, - pages_unique_domain: 'unique-domain' - ) - end - - context 'when pages_unique_domain_enabled is false' do - let(:pages_unique_domain_enabled) { false } - - it { is_expected.to eq('http://group.example.com/project') } - end - - context 'when pages_unique_domain_enabled is true' do - let(:pages_unique_domain_enabled) { true } - - it { is_expected.to eq('http://group.example.com/project') } - end - end - end - - context 'when using pages_unique_domain' do - subject { project.pages_url(with_unique_domain: true) } - - context 'when pages_unique_domain feature flag is disabled' do - before do - stub_feature_flags(pages_unique_domain: false) - end - - it { is_expected.to eq('http://group.example.com/project') } - end - - context 'when pages_unique_domain feature flag is enabled' do - before do - stub_feature_flags(pages_unique_domain: true) - - project.project_setting.update!( - pages_unique_domain_enabled: pages_unique_domain_enabled, - pages_unique_domain: 'unique-domain' - ) - end - - context 'when pages_unique_domain_enabled is false' do - let(:pages_unique_domain_enabled) { false } - - it { is_expected.to eq('http://group.example.com/project') } - end - - context 'when pages_unique_domain_enabled is true' do - let(:pages_unique_domain_enabled) { true } - - it { is_expected.to eq('http://unique-domain.example.com') } - end - end - end - - context 'with nested group' do - let(:project) { create(:project, namespace: nested_group, name: project_name) } - let(:expected_url) { "http://group.example.com/#{nested_group.path}/#{project.path}" } - - context 'group page' do - let(:project_name) { 'group.example.com' } - - it { is_expected.to eq(expected_url) } - end - - context 'project page' do - let(:project_name) { 'Project' } - - it { is_expected.to eq(expected_url) } - end - end - - context 'when the project matches its namespace url' do - let(:project_name) { 'group.example.com' } - - it { is_expected.to eq('http://group.example.com') } - - context 'with different group name capitalization' do - let(:group_name) { 'Group' } - - it { is_expected.to eq("http://group.example.com") } - end - - context 'with different project path capitalization' do - let(:project_path) { 'Group.example.com' } - - it { is_expected.to eq("http://group.example.com") } - end - - context 'with different project name capitalization' do - let(:project_name) { 'Project' } - - it { is_expected.to eq("http://group.example.com/project") } - end - - context 'when there is an explicit port' do - let(:port) { 3000 } - - context 'when not in dev mode' do - before do - stub_rails_env('production') - end - - it { is_expected.to eq('http://group.example.com:3000/group.example.com') } - end - - context 'when in dev mode' do - before do - stub_rails_env('development') - end - - it { is_expected.to eq('http://group.example.com:3000') } - end - end - end - end - - describe '#pages_unique_url', feature_category: :pages do - let(:project_settings) { create(:project_setting, pages_unique_domain: 'unique-domain') } - let(:project) { build(:project, project_setting: project_settings) } - let(:domain) { 'example.com' } - - before do - allow(Settings.pages).to receive(:host).and_return(domain) - allow(Gitlab.config.pages).to receive(:url).and_return("http://#{domain}") - end - - it 'returns the pages unique url' do - expect(project.pages_unique_url).to eq('http://unique-domain.example.com') - end - end - - describe '#pages_unique_host', feature_category: :pages do - let(:project_settings) { create(:project_setting, pages_unique_domain: 'unique-domain') } - let(:project) { build(:project, project_setting: project_settings) } - let(:domain) { 'example.com' } - - before do - allow(Settings.pages).to receive(:host).and_return(domain) - allow(Gitlab.config.pages).to receive(:url).and_return("http://#{domain}") - end - - it 'returns the pages unique url' do - expect(project.pages_unique_host).to eq('unique-domain.example.com') - end - end - - describe '#pages_namespace_url', feature_category: :pages do - let(:group) { create(:group, name: group_name) } - let(:project) { create(:project, namespace: group, name: project_name) } - let(:domain) { 'Example.com' } - let(:port) { 1234 } - - subject { project.pages_namespace_url } - - before do - allow(Settings.pages).to receive(:host).and_return(domain) - allow(Gitlab.config.pages).to receive(:url).and_return("http://example.com:#{port}") - end - - context 'group page' do - let(:group_name) { 'Group' } - let(:project_name) { 'group.example.com' } - - it { is_expected.to eq("http://group.example.com:#{port}") } - end - - context 'project page' do - let(:group_name) { 'Group' } - let(:project_name) { 'Project' } - - it { is_expected.to eq("http://group.example.com:#{port}") } - end - end - describe '.search' do let_it_be(:project) { create(:project, description: 'kitten mittens') } @@ -6311,6 +6119,36 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect(recorder.count).to be_zero end + + context 'with a CI integration' do + let!(:ci_integration) do + create(:jenkins_integration, push_events: true, active: true, project: integration.project) + end + + it 'executes the integrations' do + [Integrations::Jenkins, Integrations::Slack].each do |integration_type| + expect_next_found_instance_of(integration_type) do |instance| + expect(instance).to receive(:async_execute).with('data').once + end + end + + integration.project.execute_integrations('data', :push_hooks) + end + + context 'and skipping ci' do + it 'does not execute ci integrations' do + expect_next_found_instance_of(Integrations::Jenkins) do |instance| + expect(instance).not_to receive(:async_execute) + end + + expect_next_found_instance_of(Integrations::Slack) do |instance| + expect(instance).to receive(:async_execute).with('data').once + end + + integration.project.execute_integrations('data', :push_hooks, skip_ci: true) + end + end + end end describe '#has_active_hooks?' do @@ -6338,6 +6176,14 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect(project.has_active_hooks?(:merge_request_hooks)).to eq(true) expect(project.has_active_hooks?).to eq(true) end + + context 'with :emoji_hooks scope' do + it 'returns true when a matching emoji hook exists' do + create(:project_hook, emoji_events: true, project: project) + + expect(project.has_active_hooks?(:emoji_hooks)).to eq(true) + end + end end describe '#has_active_integrations?' do @@ -6576,7 +6422,8 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it 'does not allow access to branches for which the merge request was closed' do create( - :merge_request, :closed, + :merge_request, + :closed, target_project: target_project, target_branch: 'target-branch', source_project: project, @@ -7542,7 +7389,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr describe 'with_issues_or_mrs_available_for_user' do before do - Project.delete_all + described_class.delete_all end it 'returns correct projects' do @@ -9028,6 +8875,67 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end + describe '.without_created_and_owned_by_banned_user' do + let_it_be(:other_project) { create(:project) } + + subject(:results) { described_class.without_created_and_owned_by_banned_user } + + context 'when project creator is not banned' do + let_it_be(:project_of_active_user) { create(:project, creator: create(:user)) } + + it 'includes the project' do + expect(results).to match_array([other_project, project_of_active_user]) + end + end + + context 'when project creator is banned' do + let_it_be(:banned_user) { create(:user, :banned) } + let_it_be(:project_of_banned_user) { create(:project, creator: banned_user) } + + context 'when project creator is also an owner' do + let_it_be(:project_auth) do + project = project_of_banned_user + create(:project_authorization, :owner, user: project.creator, project: project) + end + + it 'excludes the project' do + expect(results).to match_array([other_project]) + end + end + + context 'when project creator is not an owner' do + it 'includes the project' do + expect(results).to match_array([other_project, project_of_banned_user]) + end + end + end + end + + describe '#created_and_owned_by_banned_user?' do + subject { project.created_and_owned_by_banned_user? } + + context 'when creator is banned' do + let_it_be(:creator) { create(:user, :banned) } + let_it_be(:project) { create(:project, creator: creator) } + + it { is_expected.to eq false } + + context 'when creator is an owner' do + let_it_be(:project_auth) do + create(:project_authorization, :owner, user: project.creator, project: project) + end + + it { is_expected.to eq true } + end + end + + context 'when creator is not banned' do + let_it_be(:project) { create(:project) } + + it { is_expected.to eq false } + end + end + it_behaves_like 'something that has web-hooks' do let_it_be_with_reload(:object) { create(:project) } @@ -9081,7 +8989,9 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr def create_build(new_pipeline = pipeline, name = 'test') create( - :ci_build, :success, :artifacts, + :ci_build, + :success, + :artifacts, pipeline: new_pipeline, status: new_pipeline.status, name: name diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index a24903f8b4e..71c205fca7c 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -360,7 +360,6 @@ RSpec.describe ProjectStatistics do wiki_size: 4, lfs_objects_size: 3, snippets_size: 2, - pipeline_artifacts_size: 3, build_artifacts_size: 3, packages_size: 6, uploads_size: 5 @@ -368,7 +367,7 @@ RSpec.describe ProjectStatistics do statistics.reload - expect(statistics.storage_size).to eq 28 + expect(statistics.storage_size).to eq 25 end it 'excludes the container_registry_size' do @@ -383,6 +382,18 @@ RSpec.describe ProjectStatistics do expect(statistics.storage_size).to eq 7 end + it 'excludes the pipeline_artifacts_size' do + statistics.update!( + repository_size: 2, + uploads_size: 5, + pipeline_artifacts_size: 10 + ) + + statistics.reload + + expect(statistics.storage_size).to eq 7 + end + it 'works during wiki_size backfill' do statistics.update!( repository_size: 2, @@ -428,7 +439,7 @@ RSpec.describe ProjectStatistics do storage_size: 0 ) - expect { refresh_storage_size }.to change { statistics.reload.storage_size }.from(0).to(28) + expect { refresh_storage_size }.to change { statistics.reload.storage_size }.from(0).to(25) end context 'when nullable columns are nil' do @@ -464,10 +475,9 @@ RSpec.describe ProjectStatistics do .by(increment.amount) end - it 'increases also storage size by that amount' do + it 'does not increase the storage size by that amount' do expect { described_class.increment_statistic(project, stat, increment) } - .to change { statistics.reload.storage_size } - .by(increment.amount) + .not_to change { statistics.reload.storage_size } end it 'schedules a namespace aggregation worker' do @@ -572,10 +582,9 @@ RSpec.describe ProjectStatistics do .by(total_amount) end - it 'increases also storage size by that amount' do + it 'does not increase the storage size by that amount' do expect { described_class.bulk_increment_statistic(project, stat, increments) } - .to change { statistics.reload.storage_size } - .by(total_amount) + .not_to change { statistics.reload.storage_size } end it 'schedules a namespace aggregation worker' do diff --git a/spec/models/projects/topic_spec.rb b/spec/models/projects/topic_spec.rb index d0bda6f51a1..568a4166de7 100644 --- a/spec/models/projects/topic_spec.rb +++ b/spec/models/projects/topic_spec.rb @@ -27,8 +27,8 @@ RSpec.describe Projects::Topic do it { is_expected.to validate_uniqueness_of(:name).case_insensitive } it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_length_of(:description).is_at_most(1024) } - it { expect(Projects::Topic.new).to validate_presence_of(:title) } - it { expect(Projects::Topic.new).to validate_length_of(:title).is_at_most(255) } + it { expect(described_class.new).to validate_presence_of(:title) } + it { expect(described_class.new).to validate_length_of(:title).is_at_most(255) } it { is_expected.not_to allow_value("new\nline").for(:name).with_message(name_format_message) } it { is_expected.not_to allow_value("new\rline").for(:name).with_message(name_format_message) } it { is_expected.not_to allow_value("new\vline").for(:name).with_message(name_format_message) } diff --git a/spec/models/projects/triggered_hooks_spec.rb b/spec/models/projects/triggered_hooks_spec.rb index 3c885bdac8e..581ccb500e2 100644 --- a/spec/models/projects/triggered_hooks_spec.rb +++ b/spec/models/projects/triggered_hooks_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::TriggeredHooks do +RSpec.describe Projects::TriggeredHooks, feature_category: :webhooks do let_it_be(:project) { create(:project) } let_it_be(:universal_push_hook) { create(:project_hook, project: project, push_events: true) } @@ -10,6 +10,7 @@ RSpec.describe Projects::TriggeredHooks do let_it_be(:issues_hook) { create(:project_hook, project: project, issues_events: true, push_events: false) } let(:wh_service) { instance_double(::WebHookService, async_execute: true) } + let(:data) { { some: 'data', as: 'json' } } def run_hooks(scope, data) hooks = described_class.new(scope, data) @@ -18,8 +19,6 @@ RSpec.describe Projects::TriggeredHooks do end it 'executes hooks by scope' do - data = { some: 'data', as: 'json' } - expect_hook_execution(issues_hook, data, 'issue_hooks') run_hooks(:issue_hooks, data) @@ -42,6 +41,40 @@ RSpec.describe Projects::TriggeredHooks do run_hooks(:push_hooks, data) end + context 'with emoji hooks' do + let_it_be(:emoji_hook) { create(:project_hook, project: project, emoji_events: true) } + + it 'executes hook' do + expect_hook_execution(emoji_hook, data, 'emoji_hooks') + + run_hooks(:emoji_hooks, data) + end + + context 'when emoji_webhooks feature flag is disabled' do + before do + stub_feature_flags(emoji_webhooks: false) + end + + it 'does not execute the hook' do + expect(WebHookService).not_to receive(:new) + + run_hooks(:emoji_hooks, data) + end + end + + context 'when emoji_webhooks feature flag is enabled for the project' do + before do + stub_feature_flags(emoji_webhooks: emoji_hook.project) + end + + it 'executes the hook' do + expect_hook_execution(emoji_hook, data, 'emoji_hooks') + + run_hooks(:emoji_hooks, data) + end + end + end + def expect_hook_execution(hook, data, scope) expect(WebHookService).to receive(:new).with(hook, data, scope).and_return(wh_service) end diff --git a/spec/models/protected_branch/push_access_level_spec.rb b/spec/models/protected_branch/push_access_level_spec.rb index e56ff2241b1..05e10fd6763 100644 --- a/spec/models/protected_branch/push_access_level_spec.rb +++ b/spec/models/protected_branch/push_access_level_spec.rb @@ -4,81 +4,6 @@ require 'spec_helper' RSpec.describe ProtectedBranch::PushAccessLevel, feature_category: :source_code_management do include_examples 'protected branch access' + include_examples 'protected ref deploy_key access' include_examples 'protected ref access allowed_access_levels' - - describe 'associations' do - it { is_expected.to belong_to(:deploy_key) } - end - - describe 'validations' do - it 'is not valid when a record exists with the same access level' do - protected_branch = create(:protected_branch) - create(:protected_branch_push_access_level, protected_branch: protected_branch) - level = build(:protected_branch_push_access_level, protected_branch: protected_branch) - - expect(level).to be_invalid - end - - it 'is not valid when a record exists with the same access level' do - protected_branch = create(:protected_branch) - deploy_key = create(:deploy_key, projects: [protected_branch.project]) - create(:protected_branch_push_access_level, protected_branch: protected_branch, deploy_key: deploy_key) - level = build(:protected_branch_push_access_level, protected_branch: protected_branch, deploy_key: deploy_key) - - expect(level).to be_invalid - end - - it 'checks that a deploy key is enabled for the same project as the protected branch\'s' do - level = build(:protected_branch_push_access_level, deploy_key: create(:deploy_key)) - - expect { level.save! }.to raise_error(ActiveRecord::RecordInvalid) - expect(level.errors.full_messages).to contain_exactly('Deploy key is not enabled for this project') - end - end - - describe '#check_access' do - let_it_be(:project) { create(:project) } - let_it_be(:protected_branch) { create(:protected_branch, :no_one_can_push, project: project) } - let_it_be(:user) { create(:user) } - let_it_be(:deploy_key) { create(:deploy_key, user: user) } - - let!(:deploy_keys_project) { create(:deploy_keys_project, project: project, deploy_key: deploy_key, can_push: can_push) } - let(:can_push) { true } - - before_all do - project.add_maintainer(user) - end - - context 'when this push_access_level is tied to a deploy key' do - let(:push_access_level) { create(:protected_branch_push_access_level, protected_branch: protected_branch, deploy_key: deploy_key) } - - context 'when the deploy key is among the active keys for this project' do - specify do - expect(push_access_level.check_access(user)).to be_truthy - end - end - - context 'when the deploy key is not among the active keys of this project' do - let(:can_push) { false } - - it 'is false' do - expect(push_access_level.check_access(user)).to be_falsey - end - end - end - end - - describe '#type' do - let(:push_level_access) { build(:protected_branch_push_access_level) } - - it 'returns :deploy_key when a deploy key is tied to the protected branch' do - push_level_access.deploy_key = create(:deploy_key) - - expect(push_level_access.type).to eq(:deploy_key) - end - - it 'returns :role by default' do - expect(push_level_access.type).to eq(:role) - end - end end diff --git a/spec/models/protected_tag/create_access_level_spec.rb b/spec/models/protected_tag/create_access_level_spec.rb index 8eeccdc9b34..d795399060e 100644 --- a/spec/models/protected_tag/create_access_level_spec.rb +++ b/spec/models/protected_tag/create_access_level_spec.rb @@ -4,134 +4,6 @@ require 'spec_helper' RSpec.describe ProtectedTag::CreateAccessLevel, feature_category: :source_code_management do include_examples 'protected tag access' + include_examples 'protected ref deploy_key access' include_examples 'protected ref access allowed_access_levels' - - describe 'associations' do - it { is_expected.to belong_to(:deploy_key) } - end - - describe 'validations', :aggregate_failures do - let_it_be(:protected_tag) { create(:protected_tag) } - - context 'when deploy key enabled for the project' do - let(:deploy_key) { create(:deploy_key, projects: [protected_tag.project]) } - - it 'is valid' do - level = build(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: deploy_key) - - expect(level).to be_valid - end - end - - context 'when a record exists with the same access level' do - before do - create(:protected_tag_create_access_level, protected_tag: protected_tag) - end - - it 'is not valid' do - level = build(:protected_tag_create_access_level, protected_tag: protected_tag) - - expect(level).to be_invalid - expect(level.errors.full_messages).to include('Access level has already been taken') - end - end - - context 'when a deploy key already added for this access level' do - let!(:create_access_level) do - create(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: deploy_key) - end - - let(:deploy_key) { create(:deploy_key, projects: [protected_tag.project]) } - - it 'is not valid' do - level = build(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: deploy_key) - - expect(level).to be_invalid - expect(level.errors.full_messages).to contain_exactly('Deploy key has already been taken') - end - end - - context 'when deploy key is not enabled for the project' do - let(:create_access_level) do - build(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: create(:deploy_key)) - end - - it 'returns an error' do - expect(create_access_level).to be_invalid - expect(create_access_level.errors.full_messages).to contain_exactly( - 'Deploy key is not enabled for this project' - ) - end - end - end - - describe '#check_access' do - let_it_be(:project) { create(:project) } - let_it_be(:protected_tag) { create(:protected_tag, :no_one_can_create, project: project) } - let_it_be(:user) { create(:user) } - let_it_be(:deploy_key) { create(:deploy_key, user: user) } - - let!(:deploy_keys_project) do - create(:deploy_keys_project, project: project, deploy_key: deploy_key, can_push: can_push) - end - - let(:create_access_level) { protected_tag.create_access_levels.first } - let(:can_push) { true } - - before_all do - project.add_maintainer(user) - end - - it { expect(create_access_level.check_access(user)).to be_falsey } - - context 'when this create_access_level is tied to a deploy key' do - let(:create_access_level) do - create(:protected_tag_create_access_level, protected_tag: protected_tag, deploy_key: deploy_key) - end - - context 'when the deploy key is among the active keys for this project' do - it { expect(create_access_level.check_access(user)).to be_truthy } - end - - context 'when user is missing' do - it { expect(create_access_level.check_access(nil)).to be_falsey } - end - - context 'when deploy key does not belong to the user' do - let(:another_user) { create(:user) } - - it { expect(create_access_level.check_access(another_user)).to be_falsey } - end - - context 'when user cannot access the project' do - before do - allow(user).to receive(:can?).with(:read_project, project).and_return(false) - end - - it { expect(create_access_level.check_access(user)).to be_falsey } - end - - context 'when the deploy key is not among the active keys of this project' do - let(:can_push) { false } - - it { expect(create_access_level.check_access(user)).to be_falsey } - end - end - end - - describe '#type' do - let(:create_access_level) { build(:protected_tag_create_access_level) } - - it 'returns :role by default' do - expect(create_access_level.type).to eq(:role) - end - - context 'when a deploy key is tied to the protected branch' do - let(:create_access_level) { build(:protected_tag_create_access_level, deploy_key: build(:deploy_key)) } - - it 'returns :deploy_key' do - expect(create_access_level.type).to eq(:deploy_key) - end - end - end end diff --git a/spec/models/release_highlight_spec.rb b/spec/models/release_highlight_spec.rb index 50a607040b6..6369f0e5c26 100644 --- a/spec/models/release_highlight_spec.rb +++ b/spec/models/release_highlight_spec.rb @@ -12,12 +12,12 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache, feature_category: :r end after do - ReleaseHighlight.instance_variable_set(:@file_paths, nil) + described_class.instance_variable_set(:@file_paths, nil) end describe '.paginated_query' do context 'with page param' do - subject { ReleaseHighlight.paginated_query(page: page) } + subject { described_class.paginated_query(page: page) } context 'when there is another page of results' do let(:page) { 3 } @@ -49,7 +49,7 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache, feature_category: :r describe '.paginated' do context 'with no page param' do - subject { ReleaseHighlight.paginated } + subject { described_class.paginated } it 'uses multiple levels of cache' do expect(Rails.cache).to receive(:fetch).with("release_highlight:all_tiers:items:page-1:#{Gitlab.revision}", { expires_in: described_class::CACHE_DURATION }).and_call_original @@ -100,7 +100,7 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache, feature_category: :r end describe '.most_recent_item_count' do - subject { ReleaseHighlight.most_recent_item_count } + subject { described_class.most_recent_item_count } it 'uses process memory cache' do expect(Gitlab::ProcessMemoryCache.cache_backend).to receive(:fetch).with("release_highlight:all_tiers:recent_item_count:#{Gitlab.revision}", expires_in: described_class::CACHE_DURATION) @@ -110,7 +110,7 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache, feature_category: :r context 'when recent release items exist' do it 'returns the count from the most recent file' do - allow(ReleaseHighlight).to receive(:paginated).and_return(double(:paginated, items: [double(:item)])) + allow(described_class).to receive(:paginated).and_return(double(:paginated, items: [double(:item)])) expect(subject).to eq(1) end @@ -118,7 +118,7 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache, feature_category: :r context 'when recent release items do NOT exist' do it 'returns nil' do - allow(ReleaseHighlight).to receive(:paginated).and_return(nil) + allow(described_class).to receive(:paginated).and_return(nil) expect(subject).to be_nil end @@ -126,7 +126,7 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache, feature_category: :r end describe '.most_recent_version_digest' do - subject { ReleaseHighlight.most_recent_version_digest } + subject { described_class.most_recent_version_digest } it 'uses process memory cache' do expect(Gitlab::ProcessMemoryCache.cache_backend).to receive(:fetch).with("release_highlight:all_tiers:most_recent_version_digest:#{Gitlab.revision}", expires_in: described_class::CACHE_DURATION) @@ -143,7 +143,7 @@ RSpec.describe ReleaseHighlight, :clean_gitlab_redis_cache, feature_category: :r context 'when recent release items do NOT exist' do it 'returns nil' do - allow(ReleaseHighlight).to receive(:paginated).and_return(nil) + allow(described_class).to receive(:paginated).and_return(nil) expect(subject).to be_nil end diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index bddd0516400..446ef4180d2 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -174,7 +174,7 @@ RSpec.describe Release, feature_category: :release_orchestration do end describe '#assets_count' do - subject { Release.find(release.id).assets_count } + subject { described_class.find(release.id).assets_count } it 'returns the number of sources' do is_expected.to eq(Gitlab::Workhorse::ARCHIVE_FORMATS.count) @@ -188,7 +188,7 @@ RSpec.describe Release, feature_category: :release_orchestration do end it "excludes sources count when asked" do - assets_count = Release.find(release.id).assets_count(except: [:sources]) + assets_count = described_class.find(release.id).assets_count(except: [:sources]) expect(assets_count).to eq(1) end end diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 382718620f5..537fdbc7c8f 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -289,6 +289,14 @@ RSpec.describe RemoteMirror, :mailer do end end + context 'with silent mode enabled' do + it 'returns nil' do + allow(Gitlab::SilentMode).to receive(:enabled?).and_return(true) + + expect(remote_mirror.sync).to be_nil + end + end + context 'with remote mirroring enabled' do it 'defaults to disabling only protected branches' do expect(remote_mirror.only_protected_branches?).to be_falsey diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index 929eaca85f7..0bdaa4994e5 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -281,14 +281,14 @@ RSpec.describe Route do it 'does not delete the original route' do # before deleting the route, check its there - expect(Route.where(path: offending_route.path).count).to eq(1) + expect(described_class.where(path: offending_route.path).count).to eq(1) expect do Group.delete(conflicting_group) # delete group with conflicting route end.to change { described_class.count }.by(-1) # check the conflicting route is gone - expect(Route.where(path: offending_route.path).count).to eq(0) + expect(described_class.where(path: offending_route.path).count).to eq(0) expect(route.path).to eq(offending_route.path) expect(route.valid?).to be_truthy end diff --git a/spec/models/service_desk_setting_spec.rb b/spec/models/service_desk_setting_spec.rb index b9679b82bd0..34165fc2bf3 100644 --- a/spec/models/service_desk_setting_spec.rb +++ b/spec/models/service_desk_setting_spec.rb @@ -3,12 +3,9 @@ require 'spec_helper' RSpec.describe ServiceDeskSetting, feature_category: :service_desk do - let(:verification) { build(:service_desk_custom_email_verification) } - let(:project) { build(:project) } + subject(:setting) { build(:service_desk_setting) } describe 'validations' do - subject(:service_desk_setting) { create(:service_desk_setting) } - it { is_expected.to validate_presence_of(:project_id) } it { is_expected.to validate_length_of(:outgoing_name).is_at_most(255) } it { is_expected.to validate_length_of(:project_key).is_at_most(255) } @@ -18,14 +15,56 @@ RSpec.describe ServiceDeskSetting, feature_category: :service_desk do it { is_expected.to validate_length_of(:custom_email).is_at_most(255) } describe '#custom_email_enabled' do - it { expect(subject.custom_email_enabled).to be_falsey } + it { expect(setting.custom_email_enabled).to be_falsey } it { expect(described_class.new(custom_email_enabled: true).custom_email_enabled).to be_truthy } + + context 'when set to true' do + let(:expected_error_part) { 'cannot be enabled until verification process has finished.' } + + before do + setting.custom_email = 'user@example.com' + setting.custom_email_enabled = true + end + + it 'is not valid' do + is_expected.not_to be_valid + expect(setting.errors[:custom_email_enabled].join).to include(expected_error_part) + end + + context 'when custom email records exist' do + let_it_be(:project) { create(:project) } + let_it_be(:credential) { create(:service_desk_custom_email_credential, project: project) } + + let!(:verification) { create(:service_desk_custom_email_verification, project: project) } + + subject(:setting) { build_stubbed(:service_desk_setting, project: project) } + + before do + project.reset + end + + context 'when custom email verification started' do + it 'is not valid' do + is_expected.not_to be_valid + expect(setting.errors[:custom_email_enabled].join).to include(expected_error_part) + end + end + + context 'when custom email verification has been finished' do + before do + verification.mark_as_finished! + end + + it { is_expected.to be_valid } + end + end + end end context 'when custom_email_enabled is true' do before do # Test without ServiceDesk::CustomEmailVerification for simplicity - subject.custom_email_enabled = true + setting.custom_email_enabled = true end it { is_expected.to validate_presence_of(:custom_email) } @@ -66,13 +105,13 @@ RSpec.describe ServiceDeskSetting, feature_category: :service_desk do describe '#custom_email_address_for_verification' do it 'returns nil' do - expect(subject.custom_email_address_for_verification).to be_nil + expect(setting.custom_email_address_for_verification).to be_nil end context 'when custom_email exists' do it 'returns correct verification address' do - subject.custom_email = 'support@example.com' - expect(subject.custom_email_address_for_verification).to eq('support+verify@example.com') + setting.custom_email = 'support@example.com' + expect(setting.custom_email_address_for_verification).to eq('support+verify@example.com') end end end @@ -114,6 +153,8 @@ RSpec.describe ServiceDeskSetting, feature_category: :service_desk do end describe 'associations' do + let(:project) { build(:project) } + let(:verification) { build(:service_desk_custom_email_verification) } let(:custom_email_settings) do build_stubbed( :service_desk_setting, diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 8669db4af16..2d6a674d3ce 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -446,7 +446,7 @@ RSpec.describe Todo do end specify do - expect(Todo.count_grouped_by_user_id_and_state).to eq({ [user1.id, "done"] => 1, [user1.id, "pending"] => 2, [user2.id, "pending"] => 1 }) + expect(described_class.count_grouped_by_user_id_and_state).to eq({ [user1.id, "done"] => 1, [user1.id, "pending"] => 2, [user2.id, "pending"] => 1 }) end end diff --git a/spec/models/user_custom_attribute_spec.rb b/spec/models/user_custom_attribute_spec.rb index 934956926f0..7d3806fcdfa 100644 --- a/spec/models/user_custom_attribute_spec.rb +++ b/spec/models/user_custom_attribute_spec.rb @@ -22,19 +22,19 @@ RSpec.describe UserCustomAttribute, feature_category: :user_profile do let(:custom_attribute) { create(:user_custom_attribute, key: 'blocked_at', value: blocked_at, user_id: user.id) } describe '.by_user_id' do - subject { UserCustomAttribute.by_user_id(user.id) } + subject { described_class.by_user_id(user.id) } it { is_expected.to match_array([custom_attribute]) } end describe '.by_updated_at' do - subject { UserCustomAttribute.by_updated_at(Date.today.all_day) } + subject { described_class.by_updated_at(Date.today.all_day) } it { is_expected.to match_array([custom_attribute]) } end describe '.by_key' do - subject { UserCustomAttribute.by_key('blocked_at') } + subject { described_class.by_key('blocked_at') } it { is_expected.to match_array([custom_attribute]) } end @@ -44,7 +44,7 @@ RSpec.describe UserCustomAttribute, feature_category: :user_profile do let_it_be(:user) { create(:user) } let(:abuse_report) { create(:abuse_report, user: user) } - subject { UserCustomAttribute.set_banned_by_abuse_report(abuse_report) } + subject { described_class.set_banned_by_abuse_report(abuse_report) } it 'adds the abuse report ID to user custom attributes' do subject @@ -66,7 +66,7 @@ RSpec.describe UserCustomAttribute, feature_category: :user_profile do end describe '#upsert_custom_attributes' do - subject { UserCustomAttribute.upsert_custom_attributes(custom_attributes) } + subject { described_class.upsert_custom_attributes(custom_attributes) } let_it_be_with_reload(:user) { create(:user) } diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 17899012aaa..729635b5a27 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -49,8 +49,7 @@ RSpec.describe UserPreference, feature_category: :user_profile do end describe 'pass_user_identities_to_ci_jwt' do - it { is_expected.to allow_value(true).for(:pass_user_identities_to_ci_jwt) } - it { is_expected.to allow_value(false).for(:pass_user_identities_to_ci_jwt) } + it { is_expected.to allow_value(true, false).for(:pass_user_identities_to_ci_jwt) } it { is_expected.not_to allow_value(nil).for(:pass_user_identities_to_ci_jwt) } it { is_expected.not_to allow_value("").for(:pass_user_identities_to_ci_jwt) } end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 690c0be3b7a..059cbac638b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -137,6 +137,7 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to have_one(:banned_user) } it { is_expected.to have_many(:snippets).dependent(:destroy) } it { is_expected.to have_many(:members) } + it { is_expected.to have_many(:member_namespaces) } it { is_expected.to have_many(:project_members) } it { is_expected.to have_many(:group_members) } it { is_expected.to have_many(:groups) } @@ -186,6 +187,15 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to have_many(:merge_request_assignment_events).class_name('ResourceEvents::MergeRequestAssignmentEvent') } it do + is_expected.to have_many(:organization_users).class_name('Organizations::OrganizationUser').inverse_of(:user) + end + + it do + is_expected.to have_many(:organizations) + .through(:organization_users).class_name('Organizations::Organization').inverse_of(:users) + end + + it do is_expected.to have_many(:alert_assignees).class_name('::AlertManagement::AlertAssignee').inverse_of(:assignee) end @@ -358,13 +368,13 @@ RSpec.describe User, feature_category: :user_profile do context 'when password is updated' do context 'default behaviour' do it 'enqueues the `password changed` email' do - user.password = User.random_password + user.password = described_class.random_password expect { user.save! }.to have_enqueued_mail(DeviseMailer, :password_change) end it 'does not enqueue the `admin changed your password` email' do - user.password = User.random_password + user.password = described_class.random_password expect { user.save! }.not_to have_enqueued_mail(DeviseMailer, :password_change_by_admin) end @@ -372,21 +382,21 @@ RSpec.describe User, feature_category: :user_profile do context '`admin changed your password` email' do it 'is enqueued only when explicitly allowed' do - user.password = User.random_password + user.password = described_class.random_password user.send_only_admin_changed_your_password_notification! expect { user.save! }.to have_enqueued_mail(DeviseMailer, :password_change_by_admin) end it '`password changed` email is not enqueued if it is explicitly allowed' do - user.password = User.random_password + user.password = described_class.random_password user.send_only_admin_changed_your_password_notification! expect { user.save! }.not_to have_enqueued_mail(DeviseMailer, :password_changed) end it 'is not enqueued if sending notifications on password updates is turned off as per Devise config' do - user.password = User.random_password + user.password = described_class.random_password user.send_only_admin_changed_your_password_notification! allow(Devise).to receive(:send_password_change_notification).and_return(false) @@ -412,7 +422,7 @@ RSpec.describe User, feature_category: :user_profile do context 'when email is changed to another before performing the job that sends confirmation instructions for previous email change request' do it "mentions the recipient's email in the message body", :aggregate_failures do - same_user = User.find(user.id) + same_user = described_class.find(user.id) same_user.update!(email: unconfirmed_email) user.update!(email: another_unconfirmed_email) @@ -537,7 +547,7 @@ RSpec.describe User, feature_category: :user_profile do end it 'does not check if the user is a new record' do - user = User.new(username: 'newuser') + user = described_class.new(username: 'newuser') expect(user.new_record?).to eq(true) expect(user).not_to receive(:namespace_move_dir_allowed) @@ -1171,7 +1181,7 @@ RSpec.describe User, feature_category: :user_profile do let(:random_password) { described_class.random_password } before do - expect(User).to receive(:password_length).and_return(88..128) + expect(described_class).to receive(:password_length).and_return(88..128) end context 'length' do @@ -1405,7 +1415,7 @@ RSpec.describe User, feature_category: :user_profile do context 'strip attributes' do context 'name' do - let(:user) { User.new(name: ' John Smith ') } + let(:user) { described_class.new(name: ' John Smith ') } it 'strips whitespaces on validation' do expect { user.valid? }.to change { user.name }.to('John Smith') @@ -1677,7 +1687,7 @@ RSpec.describe User, feature_category: :user_profile do end it 'returns the correct highest role' do - users = User.includes(:user_highest_role).where(id: [user.id, another_user.id]) + users = described_class.includes(:user_highest_role).where(id: [user.id, another_user.id]) expect(users.collect { |u| [u.id, u.highest_role] }).to contain_exactly( [user.id, Gitlab::Access::MAINTAINER], @@ -2054,7 +2064,7 @@ RSpec.describe User, feature_category: :user_profile do describe '#generate_password' do it 'does not generate password by default' do - password = User.random_password + password = described_class.random_password user = create(:user, password: password) expect(user.password).to eq(password) @@ -2741,9 +2751,9 @@ RSpec.describe User, feature_category: :user_profile do let_it_be(:admin_issue_board_list) { create_list(:user, 12, :admin, :with_sign_ins) } it 'returns up to the ten most recently active instance admins' do - active_admins_in_recent_sign_in_desc_order = User.admins.active.order_recent_sign_in.limit(10) + active_admins_in_recent_sign_in_desc_order = described_class.admins.active.order_recent_sign_in.limit(10) - expect(User.instance_access_request_approvers_to_be_notified).to eq(active_admins_in_recent_sign_in_desc_order) + expect(described_class.instance_access_request_approvers_to_be_notified).to eq(active_admins_in_recent_sign_in_desc_order) end end @@ -2950,56 +2960,6 @@ RSpec.describe User, feature_category: :user_profile do end end - describe '#spammer?' do - let_it_be(:user) { create(:user) } - - context 'when the user is a spammer' do - before do - allow(user).to receive(:spam_score).and_return(0.9) - end - - it 'classifies the user as a spammer' do - expect(user).to be_spammer - end - end - - context 'when the user is not a spammer' do - before do - allow(user).to receive(:spam_score).and_return(0.1) - end - - it 'does not classify the user as a spammer' do - expect(user).not_to be_spammer - end - end - end - - describe '#spam_score' do - let_it_be(:user) { create(:user) } - - context 'when the user is a spammer' do - before do - create(:abuse_trust_score, user: user, score: 0.8) - create(:abuse_trust_score, user: user, score: 0.9) - end - - it 'returns the expected score' do - expect(user.spam_score).to be_within(0.01).of(0.85) - end - end - - context 'when the user is not a spammer' do - before do - create(:abuse_trust_score, user: user, score: 0.1) - create(:abuse_trust_score, user: user, score: 0.0) - end - - it 'returns the expected score' do - expect(user.spam_score).to be_within(0.01).of(0.05) - end - end - end - describe '.find_for_database_authentication' do it 'strips whitespace from login' do user = create(:user) @@ -4497,7 +4457,7 @@ RSpec.describe User, feature_category: :user_profile do it { is_expected.to include(group) } it 'avoids N+1 queries' do - fresh_user = User.find(user.id) + fresh_user = described_class.find(user.id) control_count = ActiveRecord::QueryRecorder.new do fresh_user.solo_owned_groups end.count @@ -6145,7 +6105,9 @@ RSpec.describe User, feature_category: :user_profile do context 'when the user is a spammer' do before do - allow(user).to receive(:spammer?).and_return(true) + user_scores = Abuse::UserTrustScore.new(user) + allow(Abuse::UserTrustScore).to receive(:new).and_return(user_scores) + allow(user_scores).to receive(:spammer?).and_return(true) end context 'when the user account is less than 7 days old' do @@ -6154,7 +6116,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: User.security_bot.id, + reporter_id: described_class.security_bot.id, user_id: user.id, category: "spam", message: 'Potential spammer account deletion' @@ -6175,7 +6137,7 @@ 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: User.security_bot, message: 'Existing') } + let!(:abuse_report) { create(:abuse_report, user: user, reporter: described_class.security_bot, message: 'Existing') } it 'updates the abuse report' do subject @@ -6230,6 +6192,29 @@ RSpec.describe User, feature_category: :user_profile do expect { user.delete_async(deleted_by: deleted_by) }.not_to change { user.note } end end + + describe '#allow_possible_spam?' do + context 'when no custom attribute is set' do + it 'is false' do + expect(user.allow_possible_spam?).to be_falsey + end + end + + context 'when the custom attribute is set' do + before do + user.custom_attributes.upsert_custom_attributes( + [{ + user_id: user.id, + key: UserCustomAttribute::ALLOW_POSSIBLE_SPAM, + value: "test" + }]) + end + + it '#allow_possible_spam? is true' do + expect(user.allow_possible_spam?).to be_truthy + end + end + end end end @@ -6414,9 +6399,8 @@ RSpec.describe User, feature_category: :user_profile do describe '#required_terms_not_accepted?' do let(:user) { build(:user) } - let(:project_bot) { create(:user, :project_bot) } - subject { user.required_terms_not_accepted? } + subject(:required_terms_not_accepted) { user.required_terms_not_accepted? } context 'when terms are not enforced' do it { is_expected.to be_falsey } @@ -6428,17 +6412,25 @@ RSpec.describe User, feature_category: :user_profile do end it 'is not accepted by the user' do - expect(subject).to be_truthy + expect(required_terms_not_accepted).to be_truthy end it 'is accepted by the user' do accept_terms(user) - expect(subject).to be_falsey + expect(required_terms_not_accepted).to be_falsey end - it 'auto accepts the term for project bots' do - expect(project_bot.required_terms_not_accepted?).to be_falsey + context "with bot users" do + %i[project_bot service_account security_policy_bot].each do |user_type| + context "when user is #{user_type}" do + let(:user) { build(:user, user_type) } + + it 'auto accepts the terms' do + expect(required_terms_not_accepted).to be_falsey + end + end + end end end end @@ -7690,7 +7682,7 @@ RSpec.describe User, feature_category: :user_profile do context 'when confirmation period is expired' do before do - travel_to(User.allow_unconfirmed_access_for.from_now + 1.day) + travel_to(described_class.allow_unconfirmed_access_for.from_now + 1.day) end it { is_expected.to be(true) } @@ -8110,70 +8102,4 @@ RSpec.describe User, feature_category: :user_profile do end end end - - describe '#telesign_score' do - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } - - context 'when the user has a telesign risk score' do - before do - create(:abuse_trust_score, user: user1, score: 12.0, source: :telesign) - create(:abuse_trust_score, user: user1, score: 24.0, source: :telesign) - end - - it 'returns the latest score' do - expect(user1.telesign_score).to be(24.0) - end - end - - context 'when the user does not have a telesign risk score' do - it 'defaults to zero' do - expect(user2.telesign_score).to be(0.0) - end - end - end - - describe '#arkose_global_score' do - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } - - context 'when the user has an arkose global risk score' do - before do - create(:abuse_trust_score, user: user1, score: 12.0, source: :arkose_global_score) - create(:abuse_trust_score, user: user1, score: 24.0, source: :arkose_global_score) - end - - it 'returns the latest score' do - expect(user1.arkose_global_score).to be(24.0) - end - end - - context 'when the user does not have an arkose global risk score' do - it 'defaults to zero' do - expect(user2.arkose_global_score).to be(0.0) - end - end - end - - describe '#arkose_custom_score' do - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } - - context 'when the user has an arkose custom risk score' do - before do - create(:abuse_trust_score, user: user1, score: 12.0, source: :arkose_custom_score) - create(:abuse_trust_score, user: user1, score: 24.0, source: :arkose_custom_score) - end - - it 'returns the latest score' do - expect(user1.arkose_custom_score).to be(24.0) - end - end - - context 'when the user does not have an arkose custom risk score' do - it 'defaults to zero' do - expect(user2.arkose_custom_score).to be(0.0) - end - end - end end diff --git a/spec/models/users/merge_request_interaction_spec.rb b/spec/models/users/merge_request_interaction_spec.rb index 0b1888bd9a6..c7ffb62e5f1 100644 --- a/spec/models/users/merge_request_interaction_spec.rb +++ b/spec/models/users/merge_request_interaction_spec.rb @@ -8,7 +8,7 @@ RSpec.describe ::Users::MergeRequestInteraction do let_it_be(:merge_request) { create(:merge_request, source_project: project) } subject(:interaction) do - ::Users::MergeRequestInteraction.new(user: user, merge_request: merge_request.reset) + described_class.new(user: user, merge_request: merge_request.reset) end describe 'declarative policy delegation' do diff --git a/spec/models/users_statistics_spec.rb b/spec/models/users_statistics_spec.rb index add9bd18755..d1ca9ff5125 100644 --- a/spec/models/users_statistics_spec.rb +++ b/spec/models/users_statistics_spec.rb @@ -63,7 +63,7 @@ RSpec.describe UsersStatistics do context 'when unsuccessful' do it 'raises an ActiveRecord::RecordInvalid exception' do - allow(UsersStatistics).to receive(:create!).and_raise(ActiveRecord::RecordInvalid) + allow(described_class).to receive(:create!).and_raise(ActiveRecord::RecordInvalid) expect { described_class.create_current_stats! }.to raise_error(ActiveRecord::RecordInvalid) end diff --git a/spec/models/wiki_directory_spec.rb b/spec/models/wiki_directory_spec.rb index c30e79f79ce..93e7ecd7646 100644 --- a/spec/models/wiki_directory_spec.rb +++ b/spec/models/wiki_directory_spec.rb @@ -33,18 +33,18 @@ RSpec.describe WikiDirectory do expect(entries).to match( [ - a_kind_of(WikiDirectory).and( + a_kind_of(described_class).and( having_attributes( slug: 'Home', entries: [homechild] ) ), toplevel1, - a_kind_of(WikiDirectory).and( + a_kind_of(described_class).and( having_attributes( slug: 'parent1', entries: [ child1, child2, - a_kind_of(WikiDirectory).and( + a_kind_of(described_class).and( having_attributes( slug: 'parent1/subparent', entries: [grandchild1, grandchild2] @@ -53,7 +53,7 @@ RSpec.describe WikiDirectory do ] ) ), - a_kind_of(WikiDirectory).and( + a_kind_of(described_class).and( having_attributes( slug: 'parent2', entries: [child3] diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index e0ec54fd5ff..7963c0898b3 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -87,6 +87,14 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do end end + describe '#todoable_target_type_name' do + it 'returns correct target name' do + work_item = build(:work_item) + + expect(work_item.todoable_target_type_name).to contain_exactly('Issue', 'WorkItem') + end + end + describe '#widgets' do subject { build(:work_item).widgets } @@ -176,17 +184,32 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do is_expected.not_to include(:due, :remove_due_date) end end + + context 'when work item supports the current user todos widget' do + it 'returns todos related quick action commands' do + is_expected.to include(:todo, :done) + end + end + + context 'when work item does not support current user todos widget' do + let(:work_item) { build(:work_item, :task) } + + before do + WorkItems::Type.default_by_type(:task).widget_definitions + .find_by_widget_type(:current_user_todos).update!(disabled: true) + end + + it 'omits todos related quick action commands' do + is_expected.not_to include(:todo, :done) + end + end end describe 'transform_quick_action_params' do + let(:command_params) { { title: 'bar', assignee_ids: ['foo'] } } let(:work_item) { build(:work_item, :task) } - subject(:transformed_params) do - work_item.transform_quick_action_params({ - title: 'bar', - assignee_ids: ['foo'] - }) - end + subject(:transformed_params) { work_item.transform_quick_action_params(command_params) } it 'correctly separates widget params from regular params' do expect(transformed_params).to eq({ @@ -200,6 +223,30 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do } }) end + + context 'with current user todos widget' do + let(:command_params) { { title: 'bar', todo_event: param } } + + where(:param, :expected) do + 'done' | 'mark_as_done' + 'add' | 'add' + end + + with_them do + it 'correctly transform todo_event param' do + expect(transformed_params).to eq({ + common: { + title: 'bar' + }, + widgets: { + current_user_todos_widget: { + action: expected + } + } + }) + end + end + end end describe 'callbacks' do @@ -212,11 +259,13 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do create(:work_item) end - it_behaves_like 'issue_edit snowplow tracking' do + it_behaves_like 'internal event tracking' do let(:work_item) { create(:work_item) } - let(:property) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } + let(:action) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_CREATED } let(:project) { work_item.project } let(:user) { work_item.author } + let(:namespace) { project.namespace } + subject(:service_action) { work_item } end end @@ -255,22 +304,6 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do describe 'validations' do subject { work_item.valid? } - describe 'issue_type' do - let(:work_item) { build(:work_item, issue_type: issue_type) } - - context 'when a valid type' do - let(:issue_type) { :issue } - - it { is_expected.to eq(true) } - end - - context 'empty type' do - let(:issue_type) { nil } - - it { is_expected.to eq(false) } - end - end - describe 'confidentiality' do let_it_be(:project) { create(:project) } diff --git a/spec/models/work_items/parent_link_spec.rb b/spec/models/work_items/parent_link_spec.rb index f1aa81f46d2..d7f87da1965 100644 --- a/spec/models/work_items/parent_link_spec.rb +++ b/spec/models/work_items/parent_link_spec.rb @@ -51,8 +51,8 @@ RSpec.describe WorkItems::ParentLink, feature_category: :portfolio_management do it 'validates if child can be added to the parent' do parent_type = WorkItems::Type.default_by_type(parent_type_sym) child_type = WorkItems::Type.default_by_type(child_type_sym) - parent = build(:work_item, issue_type: parent_type_sym, work_item_type: parent_type, project: project) - child = build(:work_item, issue_type: child_type_sym, work_item_type: child_type, project: project) + parent = build(:work_item, work_item_type: parent_type, project: project) + child = build(:work_item, work_item_type: child_type, project: project) link = build(:parent_link, work_item: child, work_item_parent: parent) expect(link.valid?).to eq(is_valid) diff --git a/spec/models/work_items/type_spec.rb b/spec/models/work_items/type_spec.rb index e5c88634b26..f5806c296ac 100644 --- a/spec/models/work_items/type_spec.rb +++ b/spec/models/work_items/type_spec.rb @@ -49,10 +49,10 @@ RSpec.describe WorkItems::Type do it 'deletes type but not unrelated issues' do type = create(:work_item_type) - expect(WorkItems::Type.count).to eq(8) + expect(described_class.count).to eq(8) expect { type.destroy! }.not_to change(Issue, :count) - expect(WorkItems::Type.count).to eq(7) + expect(described_class.count).to eq(7) end end diff --git a/spec/models/work_items/widgets/base_spec.rb b/spec/models/work_items/widgets/base_spec.rb index 9b4b4d9e98f..29b54a706c2 100644 --- a/spec/models/work_items/widgets/base_spec.rb +++ b/spec/models/work_items/widgets/base_spec.rb @@ -16,4 +16,10 @@ RSpec.describe WorkItems::Widgets::Base do it { is_expected.to eq(:base) } end + + describe '.process_quick_action_param' do + subject { described_class.process_quick_action_param(:label_ids, [1, 2]) } + + it { is_expected.to eq({ label_ids: [1, 2] }) } + end end diff --git a/spec/models/work_items/widgets/current_user_todos_spec.rb b/spec/models/work_items/widgets/current_user_todos_spec.rb new file mode 100644 index 00000000000..9fdf28beada --- /dev/null +++ b/spec/models/work_items/widgets/current_user_todos_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::Widgets::CurrentUserTodos, feature_category: :team_planning do + let_it_be(:project) { create(:project) } + let_it_be(:milestone) { create(:milestone, project: project) } + let_it_be(:work_item) { create(:work_item, :issue, project: project, milestone: milestone) } + + describe '.type' do + subject { described_class.type } + + it { is_expected.to eq(:current_user_todos) } + end + + describe '#type' do + subject { described_class.new(work_item).type } + + it { is_expected.to eq(:current_user_todos) } + end + + describe '.quick_action_params' do + subject { described_class.quick_action_params } + + it { is_expected.to contain_exactly(:todo_event) } + end + + describe '.quick_action_commands' do + subject { described_class.quick_action_commands } + + it { is_expected.to contain_exactly(:todo, :done) } + end + + describe '.process_quick_action_param' do + subject { described_class.process_quick_action_param(param_name, param_value) } + + context 'when quick action param is todo_event' do + let(:param_name) { :todo_event } + + context 'when param value is `done`' do + let(:param_value) { 'done' } + + it { is_expected.to eq({ action: 'mark_as_done' }) } + end + + context 'when param value is `add`' do + let(:param_value) { 'add' } + + it { is_expected.to eq({ action: 'add' }) } + end + end + + context 'when quick action param is not todo_event' do + let(:param_name) { :foo } + let(:param_value) { 'foo' } + + it { is_expected.to eq({ foo: 'foo' }) } + end + end +end diff --git a/spec/models/work_items/widgets/milestone_spec.rb b/spec/models/work_items/widgets/milestone_spec.rb index 7b2d661df29..385614984fe 100644 --- a/spec/models/work_items/widgets/milestone_spec.rb +++ b/spec/models/work_items/widgets/milestone_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe WorkItems::Widgets::Milestone do let_it_be(:project) { create(:project) } let_it_be(:milestone) { create(:milestone, project: project) } - let_it_be(:work_item) { create(:work_item, :issue, project: project, milestone: milestone) } + let_it_be(:work_item) { create(:work_item, project: project, milestone: milestone) } describe '.type' do subject { described_class.type } |