From 22391da1261240ecc15aa15cbb19b089339902b5 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 11 Jan 2022 12:14:06 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .../ci/status/build/waiting_for_approval_spec.rb | 49 ++++++++++ spec/lib/gitlab/database/batch_count_spec.rb | 76 +-------------- .../loose_index_scan_distinct_count_spec.rb | 71 -------------- spec/lib/gitlab/logger_spec.rb | 94 +++++++++++++++++++ .../order_by_column_data_spec.rb | 35 +++++++ .../in_operator_optimization/query_builder_spec.rb | 73 +++++++++++++-- .../order_values_loader_strategy_spec.rb | 37 ++++++++ spec/policies/group_policy_spec.rb | 40 ++++---- .../packages/npm/package_presenter_spec.rb | 1 + .../groups/crm/contacts_controller_spec.rb | 2 +- .../groups/crm/organizations_controller_spec.rb | 2 +- .../google_cloud/deployments_controller_spec.rb | 103 +++++++++++++++++++++ .../ci/process_sync_events_service_spec.rb | 18 ++++ spec/services/issues/update_service_spec.rb | 8 +- .../policies/group_policy_shared_context.rb | 2 + 15 files changed, 428 insertions(+), 183 deletions(-) create mode 100644 spec/lib/gitlab/ci/status/build/waiting_for_approval_spec.rb delete mode 100644 spec/lib/gitlab/database/loose_index_scan_distinct_count_spec.rb create mode 100644 spec/lib/gitlab/logger_spec.rb create mode 100644 spec/lib/gitlab/pagination/keyset/in_operator_optimization/order_by_column_data_spec.rb create mode 100644 spec/requests/projects/google_cloud/deployments_controller_spec.rb (limited to 'spec') diff --git a/spec/lib/gitlab/ci/status/build/waiting_for_approval_spec.rb b/spec/lib/gitlab/ci/status/build/waiting_for_approval_spec.rb new file mode 100644 index 00000000000..b703a8a47ac --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/waiting_for_approval_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Status::Build::WaitingForApproval do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + + subject { described_class.new(Gitlab::Ci::Status::Core.new(build, user)) } + + describe '#illustration' do + let(:build) { create(:ci_build, :manual, environment: 'production', project: project) } + + before do + environment = create(:environment, name: 'production', project: project) + create(:deployment, :blocked, project: project, environment: environment, deployable: build) + end + + it { expect(subject.illustration).to include(:image, :size) } + it { expect(subject.illustration[:title]).to eq('Waiting for approval') } + it { expect(subject.illustration[:content]).to include('This job deploys to the protected environment "production"') } + end + + describe '.matches?' do + subject { described_class.matches?(build, user) } + + let(:build) { create(:ci_build, :manual, environment: 'production', project: project) } + + before do + create(:deployment, deployment_status, deployable: build, project: project) + end + + context 'when build is waiting for approval' do + let(:deployment_status) { :blocked } + + it 'is a correct match' do + expect(subject).to be_truthy + end + end + + context 'when build is not waiting for approval' do + let(:deployment_status) { :created } + + it 'does not match' do + expect(subject).to be_falsey + end + end + end +end diff --git a/spec/lib/gitlab/database/batch_count_spec.rb b/spec/lib/gitlab/database/batch_count_spec.rb index 9831510f014..028bdce852e 100644 --- a/spec/lib/gitlab/database/batch_count_spec.rb +++ b/spec/lib/gitlab/database/batch_count_spec.rb @@ -270,8 +270,6 @@ RSpec.describe Gitlab::Database::BatchCount do end it "defaults the batch size to #{Gitlab::Database::BatchCounter::DEFAULT_DISTINCT_BATCH_SIZE}" do - stub_feature_flags(loose_index_scan_for_distinct_values: false) - min_id = model.minimum(:id) relation = instance_double(ActiveRecord::Relation) allow(model).to receive_message_chain(:select, public_send: relation) @@ -317,85 +315,13 @@ RSpec.describe Gitlab::Database::BatchCount do end end - context 'when the loose_index_scan_for_distinct_values feature flag is off' do - it_behaves_like 'when batch fetch query is canceled' do - let(:mode) { :distinct } - let(:operation) { :count } - let(:operation_args) { nil } - let(:column) { nil } - - subject { described_class.method(:batch_distinct_count) } - - before do - stub_feature_flags(loose_index_scan_for_distinct_values: false) - end - end - end - - context 'when the loose_index_scan_for_distinct_values feature flag is on' do + it_behaves_like 'when batch fetch query is canceled' do let(:mode) { :distinct } let(:operation) { :count } let(:operation_args) { nil } let(:column) { nil } - let(:batch_size) { 10_000 } - subject { described_class.method(:batch_distinct_count) } - - before do - stub_feature_flags(loose_index_scan_for_distinct_values: true) - end - - it 'reduces batch size by half and retry fetch' do - too_big_batch_relation_mock = instance_double(ActiveRecord::Relation) - - count_method = double(send: 1) - - allow(too_big_batch_relation_mock).to receive(:send).and_raise(ActiveRecord::QueryCanceled) - allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive_message_chain(:new, :build_query).with(from: 0, to: batch_size).and_return(too_big_batch_relation_mock) - allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive_message_chain(:new, :build_query).with(from: 0, to: batch_size / 2).and_return(count_method) - allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive_message_chain(:new, :build_query).with(from: batch_size / 2, to: batch_size).and_return(count_method) - - subject.call(model, column, batch_size: batch_size, start: 0, finish: batch_size - 1) - end - - context 'when all retries fail' do - let(:batch_count_query) { 'SELECT COUNT(id) FROM relation WHERE id BETWEEN 0 and 1' } - - before do - relation = instance_double(ActiveRecord::Relation) - allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive_message_chain(:new, :build_query).and_return(relation) - allow(relation).to receive(:send).and_raise(ActiveRecord::QueryCanceled.new('query timed out')) - allow(relation).to receive(:to_sql).and_return(batch_count_query) - end - - it 'logs failing query' do - expect(Gitlab::AppJsonLogger).to receive(:error).with( - event: 'batch_count', - relation: model.table_name, - operation: operation, - operation_args: operation_args, - start: 0, - mode: mode, - query: batch_count_query, - message: 'Query has been canceled with message: query timed out' - ) - expect(subject.call(model, column, batch_size: batch_size, start: 0)).to eq(-1) - end - end - - context 'when LooseIndexScanDistinctCount raises error' do - let(:column) { :creator_id } - let(:error_class) { Gitlab::Database::LooseIndexScanDistinctCount::ColumnConfigurationError } - - it 'rescues ColumnConfigurationError' do - allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive(:new).and_raise(error_class.new('error message')) - - expect(Gitlab::AppJsonLogger).to receive(:error).with(a_hash_including(message: 'LooseIndexScanDistinctCount column error: error message')) - - expect(subject.call(Project, column, batch_size: 10_000, start: 0)).to eq(-1) - end - end end end diff --git a/spec/lib/gitlab/database/loose_index_scan_distinct_count_spec.rb b/spec/lib/gitlab/database/loose_index_scan_distinct_count_spec.rb deleted file mode 100644 index e0eac26e4d9..00000000000 --- a/spec/lib/gitlab/database/loose_index_scan_distinct_count_spec.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Database::LooseIndexScanDistinctCount do - context 'counting distinct users' do - let_it_be(:user) { create(:user) } - let_it_be(:other_user) { create(:user) } - - let(:column) { :creator_id } - - before_all do - create_list(:project, 3, creator: user) - create_list(:project, 1, creator: other_user) - end - - subject(:count) { described_class.new(Project, :creator_id).count(from: Project.minimum(:creator_id), to: Project.maximum(:creator_id) + 1) } - - it { is_expected.to eq(2) } - - context 'when STI model is queried' do - it 'does not raise error' do - expect { described_class.new(Group, :owner_id).count(from: 0, to: 1) }.not_to raise_error - end - end - - context 'when model with default_scope is queried' do - it 'does not raise error' do - expect { described_class.new(GroupMember, :id).count(from: 0, to: 1) }.not_to raise_error - end - end - - context 'when the fully qualified column is given' do - let(:column) { 'projects.creator_id' } - - it { is_expected.to eq(2) } - end - - context 'when AR attribute is given' do - let(:column) { Project.arel_table[:creator_id] } - - it { is_expected.to eq(2) } - end - - context 'when invalid value is given for the column' do - let(:column) { Class.new } - - it { expect { described_class.new(Group, column) }.to raise_error(Gitlab::Database::LooseIndexScanDistinctCount::ColumnConfigurationError) } - end - - context 'when null values are present' do - before do - create_list(:project, 2).each { |p| p.update_column(:creator_id, nil) } - end - - it { is_expected.to eq(2) } - end - end - - context 'counting STI models' do - let!(:groups) { create_list(:group, 3) } - let!(:namespaces) { create_list(:namespace, 2) } - - let(:max_id) { Namespace.maximum(:id) + 1 } - - it 'counts groups' do - count = described_class.new(Group, :id).count(from: 0, to: max_id) - expect(count).to eq(3) - end - end -end diff --git a/spec/lib/gitlab/logger_spec.rb b/spec/lib/gitlab/logger_spec.rb new file mode 100644 index 00000000000..ed22af8355f --- /dev/null +++ b/spec/lib/gitlab/logger_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Logger do + describe '.build' do + before do + allow(described_class).to receive(:file_name_noext).and_return('log') + end + + subject { described_class.build } + + it 'builds logger using Gitlab::Logger.log_level' do + expect(described_class).to receive(:log_level).and_return(:warn) + + expect(subject.level).to eq(described_class::WARN) + end + + it 'raises ArgumentError if invalid log level' do + allow(described_class).to receive(:log_level).and_return(:invalid) + + expect { subject.level }.to raise_error(ArgumentError, 'invalid log level: invalid') + end + + using RSpec::Parameterized::TableSyntax + + where(:env_value, :resulting_level) do + 0 | described_class::DEBUG + :debug | described_class::DEBUG + 'debug' | described_class::DEBUG + 'DEBUG' | described_class::DEBUG + 'DeBuG' | described_class::DEBUG + 1 | described_class::INFO + :info | described_class::INFO + 'info' | described_class::INFO + 'INFO' | described_class::INFO + 'InFo' | described_class::INFO + 2 | described_class::WARN + :warn | described_class::WARN + 'warn' | described_class::WARN + 'WARN' | described_class::WARN + 'WaRn' | described_class::WARN + 3 | described_class::ERROR + :error | described_class::ERROR + 'error' | described_class::ERROR + 'ERROR' | described_class::ERROR + 'ErRoR' | described_class::ERROR + 4 | described_class::FATAL + :fatal | described_class::FATAL + 'fatal' | described_class::FATAL + 'FATAL' | described_class::FATAL + 'FaTaL' | described_class::FATAL + 5 | described_class::UNKNOWN + :unknown | described_class::UNKNOWN + 'unknown' | described_class::UNKNOWN + 'UNKNOWN' | described_class::UNKNOWN + 'UnKnOwN' | described_class::UNKNOWN + end + + with_them do + it 'builds logger if valid log level' do + stub_env('GITLAB_LOG_LEVEL', env_value) + + expect(subject.level).to eq(resulting_level) + end + end + end + + describe '.log_level' do + context 'if GITLAB_LOG_LEVEL is set' do + before do + stub_env('GITLAB_LOG_LEVEL', described_class::ERROR) + end + + it 'returns value of GITLAB_LOG_LEVEL' do + expect(described_class.log_level).to eq(described_class::ERROR) + end + + it 'ignores fallback' do + expect(described_class.log_level(fallback: described_class::FATAL)).to eq(described_class::ERROR) + end + end + + context 'if GITLAB_LOG_LEVEL is not set' do + it 'returns default fallback DEBUG' do + expect(described_class.log_level).to eq(described_class::DEBUG) + end + + it 'returns passed fallback' do + expect(described_class.log_level(fallback: described_class::FATAL)).to eq(described_class::FATAL) + end + end + end +end diff --git a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/order_by_column_data_spec.rb b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/order_by_column_data_spec.rb new file mode 100644 index 00000000000..b4869f49081 --- /dev/null +++ b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/order_by_column_data_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::OrderByColumnData do + let(:arel_table) { Issue.arel_table } + + let(:column) do + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: :id, + column_expression: arel_table[:id], + order_expression: arel_table[:id].desc + ) + end + + subject(:column_data) { described_class.new(column, 'column_alias', arel_table) } + + describe '#arel_column' do + it 'delegates to column_expression' do + expect(column_data.arel_column).to eq(column.column_expression) + end + end + + describe '#column_for_projection' do + it 'returns the expression with AS using the original column name' do + expect(column_data.column_for_projection.to_sql).to eq('"issues"."id" AS id') + end + end + + describe '#projection' do + it 'returns the expression with AS using the specified column lias' do + expect(column_data.projection.to_sql).to eq('"issues"."id" AS column_alias') + end + end +end diff --git a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb index 00beacd4b35..58db22e5a9c 100644 --- a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb @@ -33,14 +33,14 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder ] end - shared_examples 'correct ordering examples' do - let(:iterator) do - Gitlab::Pagination::Keyset::Iterator.new( - scope: scope.limit(batch_size), - in_operator_optimization_options: in_operator_optimization_options - ) - end + let(:iterator) do + Gitlab::Pagination::Keyset::Iterator.new( + scope: scope.limit(batch_size), + in_operator_optimization_options: in_operator_optimization_options + ) + end + shared_examples 'correct ordering examples' do |opts = {}| let(:all_records) do all_records = [] iterator.each_batch(of: batch_size) do |records| @@ -49,8 +49,10 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder all_records end - it 'returns records in correct order' do - expect(all_records).to eq(expected_order) + unless opts[:skip_finder_query_test] + it 'returns records in correct order' do + expect(all_records).to eq(expected_order) + end end context 'when not passing the finder query' do @@ -248,4 +250,57 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder expect { described_class.new(**options).execute }.to raise_error(/The order on the scope does not support keyset pagination/) end + + context 'when ordering by SQL expression' do + let(:order) do + # ORDER BY (id * 10), id + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id_multiplied_by_ten', + order_expression: Arel.sql('(id * 10)').asc, + sql_type: 'integer' + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: :id, + order_expression: Issue.arel_table[:id].asc + ) + ]) + end + + let(:scope) { Issue.reorder(order) } + let(:expected_order) { issues.sort_by(&:id) } + + let(:in_operator_optimization_options) do + { + array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_mapping_scope: -> (id_expression) { Issue.where(Issue.arel_table[:project_id].eq(id_expression)) } + } + end + + context 'when iterating records one by one' do + let(:batch_size) { 1 } + + it_behaves_like 'correct ordering examples', skip_finder_query_test: true + end + + context 'when iterating records with LIMIT 3' do + let(:batch_size) { 3 } + + it_behaves_like 'correct ordering examples', skip_finder_query_test: true + end + + context 'when passing finder query' do + let(:batch_size) { 3 } + + it 'raises error, loading complete rows are not supported with SQL expressions' do + in_operator_optimization_options[:finder_query] = -> (_, _) { Issue.select(:id, '(id * 10)').where(id: -1) } + + expect(in_operator_optimization_options[:finder_query]).not_to receive(:call) + + expect do + iterator.each_batch(of: batch_size) { |records| records.to_a } + end.to raise_error /The "RecordLoaderStrategy" does not support/ + end + end + end end diff --git a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/strategies/order_values_loader_strategy_spec.rb b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/strategies/order_values_loader_strategy_spec.rb index fe95d5406dd..ab1037b318b 100644 --- a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/strategies/order_values_loader_strategy_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/strategies/order_values_loader_strategy_spec.rb @@ -31,4 +31,41 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::Strategies::O ]) end end + + context 'when an SQL expression is given' do + context 'when the sql_type attribute is missing' do + let(:order) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id_times_ten', + order_expression: Arel.sql('id * 10').asc + ) + ]) + end + + let(:keyset_scope) { Project.order(order) } + + it 'raises error' do + expect { strategy.initializer_columns }.to raise_error(Gitlab::Pagination::Keyset::SqlTypeMissingError) + end + end + + context 'when the sql_type_attribute is present' do + let(:order) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id_times_ten', + order_expression: Arel.sql('id * 10').asc, + sql_type: 'integer' + ) + ]) + end + + let(:keyset_scope) { Project.order(order) } + + it 'returns the initializer columns' do + expect(strategy.initializer_columns).to eq(['NULL::integer AS id_times_ten']) + end + end + end end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index d2dee3b781c..1d250d4f798 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -11,8 +11,6 @@ RSpec.describe GroupPolicy do it do expect_allowed(:read_group) - expect_allowed(:read_crm_organization) - expect_allowed(:read_crm_contact) expect_allowed(:read_counts) expect_allowed(*read_group_permissions) expect_disallowed(:upload_file) @@ -21,11 +19,13 @@ RSpec.describe GroupPolicy do expect_disallowed(*maintainer_permissions) expect_disallowed(*owner_permissions) expect_disallowed(:read_namespace) + expect_disallowed(:read_crm_organization) + expect_disallowed(:read_crm_contact) end end context 'with no user and public project' do - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, group: create(:group, :crm_enabled)) } let(:current_user) { nil } before do @@ -41,7 +41,7 @@ RSpec.describe GroupPolicy do end context 'with foreign user and public project' do - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, group: create(:group, :crm_enabled)) } let(:current_user) { create(:user) } before do @@ -67,7 +67,7 @@ RSpec.describe GroupPolicy do it { expect_allowed(*read_group_permissions) } context 'in subgroups' do - let(:subgroup) { create(:group, :private, parent: group) } + let(:subgroup) { create(:group, :private, :crm_enabled, parent: group) } let(:project) { create(:project, namespace: subgroup) } it { expect_allowed(*read_group_permissions) } @@ -235,7 +235,7 @@ RSpec.describe GroupPolicy do describe 'private nested group use the highest access level from the group and inherited permissions' do let_it_be(:nested_group) do - create(:group, :private, :owner_subgroup_creation_only, parent: group) + create(:group, :private, :owner_subgroup_creation_only, :crm_enabled, parent: group) end before_all do @@ -342,7 +342,7 @@ RSpec.describe GroupPolicy do let(:current_user) { owner } context 'when the group share_with_group_lock is enabled' do - let(:group) { create(:group, share_with_group_lock: true, parent: parent) } + let(:group) { create(:group, :crm_enabled, share_with_group_lock: true, parent: parent) } before do group.add_owner(owner) @@ -350,10 +350,10 @@ RSpec.describe GroupPolicy do context 'when the parent group share_with_group_lock is enabled' do context 'when the group has a grandparent' do - let(:parent) { create(:group, share_with_group_lock: true, parent: grandparent) } + let(:parent) { create(:group, :crm_enabled, share_with_group_lock: true, parent: grandparent) } context 'when the grandparent share_with_group_lock is enabled' do - let(:grandparent) { create(:group, share_with_group_lock: true) } + let(:grandparent) { create(:group, :crm_enabled, share_with_group_lock: true) } context 'when the current_user owns the parent' do before do @@ -379,7 +379,7 @@ RSpec.describe GroupPolicy do end context 'when the grandparent share_with_group_lock is disabled' do - let(:grandparent) { create(:group) } + let(:grandparent) { create(:group, :crm_enabled) } context 'when the current_user owns the parent' do before do @@ -396,7 +396,7 @@ RSpec.describe GroupPolicy do end context 'when the group does not have a grandparent' do - let(:parent) { create(:group, share_with_group_lock: true) } + let(:parent) { create(:group, :crm_enabled, share_with_group_lock: true) } context 'when the current_user owns the parent' do before do @@ -413,7 +413,7 @@ RSpec.describe GroupPolicy do end context 'when the parent group share_with_group_lock is disabled' do - let(:parent) { create(:group) } + let(:parent) { create(:group, :crm_enabled) } it { expect_allowed(:change_share_with_group_lock) } end @@ -698,7 +698,7 @@ RSpec.describe GroupPolicy do end it_behaves_like 'clusterable policies' do - let(:clusterable) { create(:group) } + let(:clusterable) { create(:group, :crm_enabled) } let(:cluster) do create(:cluster, :provided_by_gcp, @@ -708,7 +708,7 @@ RSpec.describe GroupPolicy do end describe 'update_max_artifacts_size' do - let(:group) { create(:group, :public) } + let(:group) { create(:group, :public, :crm_enabled) } context 'when no user' do let(:current_user) { nil } @@ -738,7 +738,7 @@ RSpec.describe GroupPolicy do end describe 'design activity' do - let_it_be(:group) { create(:group, :public) } + let_it_be(:group) { create(:group, :public, :crm_enabled) } let(:current_user) { nil } @@ -935,8 +935,6 @@ RSpec.describe GroupPolicy do it { is_expected.to be_allowed(:read_package) } it { is_expected.to be_allowed(:read_group) } - it { is_expected.to be_allowed(:read_crm_organization) } - it { is_expected.to be_allowed(:read_crm_contact) } it { is_expected.to be_disallowed(:create_package) } end @@ -946,8 +944,6 @@ RSpec.describe GroupPolicy do it { is_expected.to be_allowed(:create_package) } it { is_expected.to be_allowed(:read_package) } it { is_expected.to be_allowed(:read_group) } - it { is_expected.to be_allowed(:read_crm_organization) } - it { is_expected.to be_allowed(:read_crm_contact) } it { is_expected.to be_disallowed(:destroy_package) } end @@ -967,7 +963,7 @@ RSpec.describe GroupPolicy do it_behaves_like 'Self-managed Core resource access tokens' context 'support bot' do - let_it_be(:group) { create(:group, :private) } + let_it_be(:group) { create(:group, :private, :crm_enabled) } let_it_be(:current_user) { User.support_bot } before do @@ -977,7 +973,7 @@ RSpec.describe GroupPolicy do it { expect_disallowed(:read_label) } context 'when group hierarchy has a project with service desk enabled' do - let_it_be(:subgroup) { create(:group, :private, parent: group) } + let_it_be(:subgroup) { create(:group, :private, :crm_enabled, parent: group) } let_it_be(:project) { create(:project, group: subgroup, service_desk_enabled: true) } it { expect_allowed(:read_label) } @@ -1170,7 +1166,7 @@ RSpec.describe GroupPolicy do end context 'when crm_enabled is false' do - let(:group) { create(:group) } + let(:group) { create(:group, :crm_enabled) } let(:current_user) { owner } it { is_expected.to be_disallowed(:read_crm_contact) } diff --git a/spec/presenters/packages/npm/package_presenter_spec.rb b/spec/presenters/packages/npm/package_presenter_spec.rb index 7d54d017b26..2308f928c92 100644 --- a/spec/presenters/packages/npm/package_presenter_spec.rb +++ b/spec/presenters/packages/npm/package_presenter_spec.rb @@ -108,6 +108,7 @@ RSpec.describe ::Packages::Npm::PackagePresenter do context 'with packages_installable_package_files disabled' do before do stub_feature_flags(packages_installable_package_files: false) + package2.package_files.id_not_in(package_file_pending_destruction.id).delete_all end it 'returns them' do diff --git a/spec/requests/groups/crm/contacts_controller_spec.rb b/spec/requests/groups/crm/contacts_controller_spec.rb index 589834a07db..5d126c6ead5 100644 --- a/spec/requests/groups/crm/contacts_controller_spec.rb +++ b/spec/requests/groups/crm/contacts_controller_spec.rb @@ -73,7 +73,7 @@ RSpec.describe Groups::Crm::ContactsController do let(:group) { create(:group, :public, :crm_enabled) } context 'with anonymous user' do - it_behaves_like 'ok response with index template' + it_behaves_like 'response with 404 status' end end end diff --git a/spec/requests/groups/crm/organizations_controller_spec.rb b/spec/requests/groups/crm/organizations_controller_spec.rb index 899f223cb79..f38300c3c5b 100644 --- a/spec/requests/groups/crm/organizations_controller_spec.rb +++ b/spec/requests/groups/crm/organizations_controller_spec.rb @@ -73,7 +73,7 @@ RSpec.describe Groups::Crm::OrganizationsController do let(:group) { create(:group, :public, :crm_enabled) } context 'with anonymous user' do - it_behaves_like 'ok response with index template' + it_behaves_like 'response with 404 status' end end end diff --git a/spec/requests/projects/google_cloud/deployments_controller_spec.rb b/spec/requests/projects/google_cloud/deployments_controller_spec.rb new file mode 100644 index 00000000000..a5eccc43147 --- /dev/null +++ b/spec/requests/projects/google_cloud/deployments_controller_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::GoogleCloud::DeploymentsController do + let_it_be(:project) { create(:project, :public) } + + let_it_be(:user_guest) { create(:user) } + let_it_be(:user_developer) { create(:user) } + let_it_be(:user_maintainer) { create(:user) } + let_it_be(:user_creator) { project.creator } + + let_it_be(:unauthorized_members) { [user_guest, user_developer] } + let_it_be(:authorized_members) { [user_maintainer, user_creator] } + + let_it_be(:urls_list) { %W[#{project_google_cloud_deployments_cloud_run_path(project)} #{project_google_cloud_deployments_cloud_storage_path(project)}] } + + before do + project.add_guest(user_guest) + project.add_developer(user_developer) + project.add_maintainer(user_maintainer) + end + + describe "Routes must be restricted behind Google OAuth2" do + context 'when a public request is made' do + it 'returns not found on GET request' do + urls_list.each do |url| + get url + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when unauthorized members make requests' do + it 'returns not found on GET request' do + urls_list.each do |url| + unauthorized_members.each do |unauthorized_member| + sign_in(unauthorized_member) + + get url + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + context 'when authorized members make requests' do + it 'redirects on GET request' do + urls_list.each do |url| + authorized_members.each do |authorized_member| + sign_in(authorized_member) + + get url + + expect(response).to redirect_to(assigns(:authorize_url)) + end + end + end + end + end + + describe 'Authorized GET project/-/google_cloud/deployments/cloud_run' do + let_it_be(:url) { "#{project_google_cloud_deployments_cloud_run_path(project)}" } + + before do + allow_next_instance_of(GoogleApi::CloudPlatform::Client) do |client| + allow(client).to receive(:validate_token).and_return(true) + end + end + + it 'renders placeholder' do + authorized_members.each do |authorized_member| + sign_in(authorized_member) + + get url + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + describe 'Authorized GET project/-/google_cloud/deployments/cloud_storage' do + let_it_be(:url) { "#{project_google_cloud_deployments_cloud_storage_path(project)}" } + + before do + allow_next_instance_of(GoogleApi::CloudPlatform::Client) do |client| + allow(client).to receive(:validate_token).and_return(true) + end + end + + it 'renders placeholder' do + authorized_members.each do |authorized_member| + sign_in(authorized_member) + + get url + + expect(response).to have_gitlab_http_status(:ok) + end + end + end +end diff --git a/spec/services/ci/process_sync_events_service_spec.rb b/spec/services/ci/process_sync_events_service_spec.rb index c4d4a2a4776..8b7717fe4bf 100644 --- a/spec/services/ci/process_sync_events_service_spec.rb +++ b/spec/services/ci/process_sync_events_service_spec.rb @@ -71,6 +71,24 @@ RSpec.describe Ci::ProcessSyncEventsService do expect { execute }.not_to change(Projects::SyncEvent, :count) end end + + it 'does not delete non-executed events' do + new_project = create(:project) + sync_event_class.delete_all + + project1.update!(group: parent_group_2) + new_project.update!(group: parent_group_1) + project2.update!(group: parent_group_1) + + new_project_sync_event = new_project.sync_events.last + + allow(sync_event_class).to receive(:preload_synced_relation).and_return( + sync_event_class.where.not(id: new_project_sync_event) + ) + + expect { execute }.to change(Projects::SyncEvent, :count).from(3).to(1) + expect(new_project_sync_event.reload).to be_persisted + end end context 'for Namespaces::SyncEvent' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 48b61814142..98d2ab1341e 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -22,10 +22,10 @@ RSpec.describe Issues::UpdateService, :mailer do end before_all do - project.add_maintainer(user) - project.add_developer(user2) - project.add_developer(user3) - project.add_guest(guest) + group.add_maintainer(user) + group.add_developer(user2) + group.add_developer(user3) + group.add_guest(guest) end describe 'execute' do diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index db778086692..0827a46313a 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -28,6 +28,8 @@ RSpec.shared_context 'GroupPolicy context' do read_metrics_dashboard_annotation read_prometheus read_package_settings + read_crm_contact + read_crm_organization ] end -- cgit v1.2.3