diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-18 13:50:51 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-18 13:50:51 +0300 |
commit | db384e6b19af03b4c3c82a5760d83a3fd79f7982 (patch) | |
tree | 34beaef37df5f47ccbcf5729d7583aae093cffa0 /spec/requests | |
parent | 54fd7b1bad233e3944434da91d257fa7f63c3996 (diff) |
Add latest changes from gitlab-org/gitlab@16-3-stable-eev16.3.0-rc42
Diffstat (limited to 'spec/requests')
82 files changed, 2212 insertions, 913 deletions
diff --git a/spec/requests/admin/abuse_reports_controller_spec.rb b/spec/requests/admin/abuse_reports_controller_spec.rb index 8d033a2e147..c443a441af8 100644 --- a/spec/requests/admin/abuse_reports_controller_spec.rb +++ b/spec/requests/admin/abuse_reports_controller_spec.rb @@ -53,16 +53,16 @@ RSpec.describe Admin::AbuseReportsController, type: :request, feature_category: end end - describe 'PUT #update' do + shared_examples 'moderates user' do let(:report) { create(:abuse_report) } let(:params) { { user_action: 'block_user', close: 'true', reason: 'spam', comment: 'obvious spam' } } let(:expected_params) { ActionController::Parameters.new(params).permit! } let(:message) { 'Service response' } - subject(:request) { put admin_abuse_report_path(report, params) } + subject(:request) { put path } - it 'invokes the Admin::AbuseReportUpdateService' do - expect_next_instance_of(Admin::AbuseReportUpdateService, report, admin, expected_params) do |service| + it 'invokes the Admin::AbuseReports::ModerateUserService' do + expect_next_instance_of(Admin::AbuseReports::ModerateUserService, report, admin, expected_params) do |service| expect(service).to receive(:execute).and_call_original end @@ -71,7 +71,7 @@ RSpec.describe Admin::AbuseReportsController, type: :request, feature_category: context 'when the service response is a success' do before do - allow_next_instance_of(Admin::AbuseReportUpdateService, report, admin, expected_params) do |service| + allow_next_instance_of(Admin::AbuseReports::ModerateUserService, report, admin, expected_params) do |service| allow(service).to receive(:execute).and_return(ServiceResponse.success(message: message)) end @@ -86,7 +86,7 @@ RSpec.describe Admin::AbuseReportsController, type: :request, feature_category: context 'when the service response is an error' do before do - allow_next_instance_of(Admin::AbuseReportUpdateService, report, admin, expected_params) do |service| + allow_next_instance_of(Admin::AbuseReports::ModerateUserService, report, admin, expected_params) do |service| allow(service).to receive(:execute).and_return(ServiceResponse.error(message: message)) end @@ -100,6 +100,18 @@ RSpec.describe Admin::AbuseReportsController, type: :request, feature_category: end end + describe 'PUT #update' do + let(:path) { admin_abuse_report_path(report, params) } + + it_behaves_like 'moderates user' + end + + describe 'PUT #moderate_user' do + let(:path) { moderate_user_admin_abuse_report_path(report, params) } + + it_behaves_like 'moderates user' + end + describe 'DELETE #destroy' do let!(:report) { create(:abuse_report) } let(:params) { {} } diff --git a/spec/requests/api/admin/batched_background_migrations_spec.rb b/spec/requests/api/admin/batched_background_migrations_spec.rb index 180b6c7abd6..2b205ca656f 100644 --- a/spec/requests/api/admin/batched_background_migrations_spec.rb +++ b/spec/requests/api/admin/batched_background_migrations_spec.rb @@ -100,6 +100,7 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations, feature_category: :datab expect(json_response.first['id']).to eq(migration.id) expect(json_response.first['job_class_name']).to eq(migration.job_class_name) expect(json_response.first['table_name']).to eq(migration.table_name) + expect(json_response.first['column_name']).to eq(migration.column_name) expect(json_response.first['status']).to eq(migration.status_name.to_s) expect(json_response.first['progress']).to be_zero end @@ -151,6 +152,7 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations, feature_category: :datab expect(json_response.first['id']).to eq(ci_database_migration.id) expect(json_response.first['job_class_name']).to eq(ci_database_migration.job_class_name) expect(json_response.first['table_name']).to eq(ci_database_migration.table_name) + expect(json_response.first['column_name']).to eq(ci_database_migration.column_name) expect(json_response.first['status']).to eq(ci_database_migration.status_name.to_s) expect(json_response.first['progress']).to be_zero end diff --git a/spec/requests/api/broadcast_messages_spec.rb b/spec/requests/api/admin/broadcast_messages_spec.rb index 530c81364a8..58347104a7a 100644 --- a/spec/requests/api/broadcast_messages_spec.rb +++ b/spec/requests/api/admin/broadcast_messages_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::BroadcastMessages, :aggregate_failures, feature_category: :onboarding do +RSpec.describe API::Admin::BroadcastMessages, :aggregate_failures, feature_category: :onboarding do let_it_be(:admin) { create(:admin) } let_it_be(:message) { create(:broadcast_message) } let_it_be(:path) { '/broadcast_messages' } @@ -17,7 +17,8 @@ RSpec.describe API::BroadcastMessages, :aggregate_failures, feature_category: :o expect(response).to include_pagination_headers expect(json_response).to be_kind_of(Array) expect(json_response.first.keys) - .to match_array(%w(id message starts_at ends_at color font active target_access_levels target_path broadcast_type dismissable)) + .to match_array(%w[id message starts_at ends_at color font active target_access_levels target_path + broadcast_type dismissable]) end end @@ -30,7 +31,8 @@ RSpec.describe API::BroadcastMessages, :aggregate_failures, feature_category: :o expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq message.id expect(json_response.keys) - .to match_array(%w(id message starts_at ends_at color font active target_access_levels target_path broadcast_type dismissable)) + .to match_array(%w[id message starts_at ends_at color font active target_access_levels target_path + broadcast_type dismissable]) end end @@ -130,6 +132,22 @@ RSpec.describe API::BroadcastMessages, :aggregate_failures, feature_category: :o expect(response).to have_gitlab_http_status(:created) expect(json_response['dismissable']).to eq true end + + context 'when create does not persist record' do + let_it_be(:message) { build(:broadcast_message) }.freeze + let_it_be(:stubbed_errors) { ActiveModel::Errors.new(double).tap { |e| e.add(:base, 'error') } }.freeze + + before do + allow(System::BroadcastMessage).to receive(:create).and_return(message) + allow(message).to receive(:errors).and_return(stubbed_errors) + end + + it 'calls render_validation_error!' do + post api(path, admin, admin_mode: true), params: { message: 'message' } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end end end @@ -222,6 +240,23 @@ RSpec.describe API::BroadcastMessages, :aggregate_failures, feature_category: :o expect(response).to have_gitlab_http_status(:ok) expect(json_response['dismissable']).to eq true end + + context 'when update fails' do + let_it_be(:message) { build(:broadcast_message) }.freeze + let_it_be(:stubbed_errors) { ActiveModel::Errors.new(double).tap { |e| e.add(:base, 'error') } }.freeze + + before do + allow(System::BroadcastMessage).to receive(:find).and_return(message) + allow(message).to receive(:update).and_return(false) + allow(message).to receive(:errors).and_return(stubbed_errors) + end + + it 'calls render_validation_error!' do + put api(path, admin, admin_mode: true), params: { message: 'message' } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end end end @@ -246,7 +281,7 @@ RSpec.describe API::BroadcastMessages, :aggregate_failures, feature_category: :o delete api(path, admin, admin_mode: true) expect(response).to have_gitlab_http_status(:no_content) - end.to change { BroadcastMessage.count }.by(-1) + end.to change { System::BroadcastMessage.count }.by(-1) end end end diff --git a/spec/requests/api/admin/plan_limits_spec.rb b/spec/requests/api/admin/plan_limits_spec.rb index 97eb8a2b13f..f0bc90efa50 100644 --- a/spec/requests/api/admin/plan_limits_spec.rb +++ b/spec/requests/api/admin/plan_limits_spec.rb @@ -102,7 +102,7 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits', feature_category: :shared d 'ci_registered_group_runners': 107, 'ci_registered_project_runners': 108, 'conan_max_file_size': 10, - 'enforcement_limit': 15, + 'enforcement_limit': 100, 'generic_packages_max_file_size': 20, 'helm_max_file_size': 25, 'maven_max_file_size': 30, @@ -124,11 +124,11 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits', feature_category: :shared d expect(json_response['ci_registered_group_runners']).to eq(107) expect(json_response['ci_registered_project_runners']).to eq(108) expect(json_response['conan_max_file_size']).to eq(10) - expect(json_response['enforcement_limit']).to eq(15) + expect(json_response['enforcement_limit']).to eq(100) expect(json_response['generic_packages_max_file_size']).to eq(20) expect(json_response['helm_max_file_size']).to eq(25) expect(json_response['limits_history']).to eq( - { "enforcement_limit" => [{ "user_id" => admin.id, "username" => admin.username, "timestamp" => current_timestamp, "value" => 15 }], + { "enforcement_limit" => [{ "user_id" => admin.id, "username" => admin.username, "timestamp" => current_timestamp, "value" => 100 }], "notification_limit" => [{ "user_id" => admin.id, "username" => admin.username, "timestamp" => current_timestamp, "value" => 90 }], "storage_size_limit" => [{ "user_id" => admin.id, "username" => admin.username, "timestamp" => current_timestamp, "value" => 80 }] } ) diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index 01bb8101f76..1d1d66ad125 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -229,7 +229,7 @@ RSpec.describe API::API, feature_category: :system_access do expect(data.stringify_keys).not_to include('meta.project', 'meta.root_namespace', 'meta.user') end - expect(BroadcastMessage).to receive(:all).and_raise('An error!') + expect(System::BroadcastMessage).to receive(:all).and_raise('An error!') get(api('/broadcast_messages')) diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index ed0cec46a42..c7b7131a600 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -238,7 +238,7 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do ] end - before(:all) do + before_all do project.update!(group: group) end diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb index d5f60e62b06..d760e4ddf28 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -311,8 +311,7 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end end - # Move this from `shared_context` to `describe` when `ci_refactoring_pipeline_schedule_create_service` is removed. - shared_context 'POST /projects/:id/pipeline_schedules' do # rubocop:disable RSpec/ContextWording + describe 'POST /projects/:id/pipeline_schedules' do let(:params) { attributes_for(:ci_pipeline_schedule) } context 'authenticated user with valid permissions' do @@ -369,8 +368,7 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end end - # Move this from `shared_context` to `describe` when `ci_refactoring_pipeline_schedule_create_service` is removed. - shared_context 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id' do + describe 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id' do let(:pipeline_schedule) do create(:ci_pipeline_schedule, project: project, owner: developer) end @@ -439,18 +437,6 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end end - it_behaves_like 'POST /projects/:id/pipeline_schedules' - it_behaves_like 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id' - - context 'when the FF ci_refactoring_pipeline_schedule_create_service is disabled' do - before do - stub_feature_flags(ci_refactoring_pipeline_schedule_create_service: false) - end - - it_behaves_like 'POST /projects/:id/pipeline_schedules' - it_behaves_like 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id' - end - describe 'POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do let(:pipeline_schedule) do create(:ci_pipeline_schedule, project: project, owner: developer) diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 869b0ec9dca..3544a6dd72a 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -43,19 +43,6 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do expect(json_response.first.keys).to contain_exactly(*%w[id iid project_id sha ref status web_url created_at updated_at source name]) end - - context 'when pipeline_name_in_api feature flag is off' do - before do - stub_feature_flags(pipeline_name_in_api: false) - end - - it 'does not include pipeline name in response and ignores name parameter' do - get api("/projects/#{project.id}/pipelines", user), params: { name: 'Chatops pipeline' } - - expect(json_response.length).to eq(1) - expect(json_response.first.keys).not_to include('name') - end - end end it 'avoids N+1 queries' do @@ -894,19 +881,6 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do expect(json_response["coverage"]).to eq('30.00') end end - - context 'with pipeline_name_in_api disabled' do - before do - stub_feature_flags(pipeline_name_in_api: false) - end - - it 'does not return name', :aggregate_failures do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}", user) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.keys).not_to include('name') - end - end end context 'unauthorized user' do @@ -971,19 +945,6 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do expect(json_response['sha']).to eq(second_branch.target) end end - - context 'with pipeline_name_in_api disabled' do - before do - stub_feature_flags(pipeline_name_in_api: false) - end - - it 'does not return name', :aggregate_failures do - get api("/projects/#{project.id}/pipelines/latest", user) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.keys).not_to include('name') - end - end end context 'unauthorized user' do diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 7540e19e278..2f0e64cd4da 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -152,6 +152,7 @@ RSpec.describe API::CommitStatuses, feature_category: :continuous_integration do expect(json_response['ref']).not_to be_empty expect(json_response['target_url']).to be_nil expect(json_response['description']).to be_nil + expect(json_response['pipeline_id']).not_to be_nil if status == 'failed' expect(CommitStatus.find(json_response['id'])).to be_api_failure diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 28126f1bdc2..687ce333ca5 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -573,13 +573,9 @@ RSpec.describe API::Commits, feature_category: :source_code_management do subject end - it_behaves_like 'Snowplow event tracking with RedisHLL context' do + it_behaves_like 'internal event tracking' do + let(:action) { ::Gitlab::UsageDataCounters::EditorUniqueCounter::EDIT_BY_WEB_IDE } let(:namespace) { project.namespace.reload } - let(:category) { 'Gitlab::UsageDataCounters::EditorUniqueCounter' } - let(:action) { 'ide_edit' } - let(:property) { 'g_edit_by_web_ide' } - let(:label) { 'usage_activity_by_stage_monthly.create.action_monthly_active_users_ide_edit' } - let(:context) { [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: event_name).to_context] } end context 'counts.web_ide_commits Snowplow event tracking' do diff --git a/spec/requests/api/composer_packages_spec.rb b/spec/requests/api/composer_packages_spec.rb index 2bb2ffa03c4..3652bee5e44 100644 --- a/spec/requests/api/composer_packages_spec.rb +++ b/spec/requests/api/composer_packages_spec.rb @@ -289,7 +289,7 @@ RSpec.describe API::ComposerPackages, feature_category: :package_registry do let(:url) { "/projects/#{project.id}/packages/composer" } let(:params) { {} } - before(:all) do + before_all do project.repository.add_tag(user, 'v1.2.99', 'master') end diff --git a/spec/requests/api/draft_notes_spec.rb b/spec/requests/api/draft_notes_spec.rb index 3911bb8bc00..f15ed6e2d5f 100644 --- a/spec/requests/api/draft_notes_spec.rb +++ b/spec/requests/api/draft_notes_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe API::DraftNotes, feature_category: :code_review_workflow do let_it_be(:user) { create(:user) } let_it_be(:user_2) { create(:user) } - let_it_be(:project) { create(:project, :public) } + let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } let_it_be(:private_project) { create(:project, :private) } @@ -184,6 +184,24 @@ RSpec.describe API::DraftNotes, feature_category: :code_review_workflow do end end + context "when using a diff with position" do + let!(:draft_note) { create(:draft_note_on_text_diff, merge_request: merge_request, author: user) } + + it_behaves_like 'diff draft notes API', 'iid' + + context "when position is for a previous commit on the merge request" do + it "returns a 400 bad request error because the line_code is old" do + # SHA taken from an earlier commit listed in spec/factories/merge_requests.rb + position = draft_note.position.to_h.merge(new_line: 'c1acaa58bbcbc3eafe538cb8274ba387047b69f8') + + post api("/projects/#{project.id}/merge_requests/#{merge_request['iid']}/draft_notes", user), + params: { body: 'hi!', position: position } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + context "when attempting to resolve a disscussion" do context "when providing a non-existant ID" do it "returns a 400 Bad Request" do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index ea341703301..01acb83360c 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -342,6 +342,23 @@ RSpec.describe API::Files, feature_category: :source_code_management do expect(response).to have_gitlab_http_status(:ok) end + context 'when a project is moved' do + let_it_be(:redirect_route) { 'new/project/location' } + let_it_be(:file_path) { 'files%2Fruby%2Fpopen.rb' } + + it 'redirects to the new project location' do + project.route.create_redirect(redirect_route) + + url = "/projects/#{CGI.escape(redirect_route)}/repository/files/#{file_path}" + get api(url, api_user, **options), params: params + + expect(response).to have_gitlab_http_status(:moved_permanently) + expect(response.headers['Location']).to start_with( + "#{request.base_url}/api/v4/projects/#{project.id}/repository/files/#{file_path}" + ) + end + end + it 'sets inline content disposition by default' do url = route(file_path) + '/raw' diff --git a/spec/requests/api/graphql/abuse_report_labels_spec.rb b/spec/requests/api/graphql/abuse_report_labels_spec.rb new file mode 100644 index 00000000000..bae8a7937fa --- /dev/null +++ b/spec/requests/api/graphql/abuse_report_labels_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'abuse_report_labels', feature_category: :insider_threat do + include GraphqlHelpers + + let_it_be(:current_user) { create(:admin) } + let_it_be(:project_label) { create(:label) } + let_it_be(:label_one) { create(:abuse_report_label, title: 'Uno') } + let_it_be(:label_two) { create(:abuse_report_label, title: 'Dos') } + + let(:fields) do + <<~GRAPHQL + nodes { + id + title + description + color + textColor + } + GRAPHQL + end + + let(:arguments) { { searchTerm: '' } } + let(:query) { graphql_query_for('abuseReportLabels', arguments, fields) } + + before do + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query that returns data' + + it 'returns abuse report labels sorted by title in ascending order' do + expect(graphql_data_at('abuseReportLabels', 'nodes').size).to eq 2 + expect(graphql_data_at('abuseReportLabels', 'nodes', 0)).to match(a_graphql_entity_for(label_two)) + expect(graphql_data_at('abuseReportLabels', 'nodes', 1)).to match(a_graphql_entity_for(label_one)) + end + + context 'when current user is not an admin' do + let_it_be(:current_user) { create(:user) } + + it_behaves_like 'a working graphql query' + + it 'does not contain any data' do + expect(graphql_data_at('abuseReportLabels', 'nodes')).to be_empty + end + end + + context 'with a search term param' do + let(:arguments) { { searchTerm: 'uno' } } + + it 'returns only matching abuse report labels' do + expect(graphql_data_at('abuseReportLabels', 'nodes').size).to eq 1 + expect(graphql_data_at('abuseReportLabels', 'nodes', 0)).to match(a_graphql_entity_for(label_one)) + end + end +end diff --git a/spec/requests/api/graphql/abuse_report_spec.rb b/spec/requests/api/graphql/abuse_report_spec.rb new file mode 100644 index 00000000000..7d0b8b35763 --- /dev/null +++ b/spec/requests/api/graphql/abuse_report_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'abuse_report', feature_category: :insider_threat do + include GraphqlHelpers + + let_it_be(:current_user) { create(:admin) } + let_it_be(:label) { create(:abuse_report_label, title: 'Uno') } + let_it_be(:report) { create(:abuse_report, labels: [label]) } + + let(:report_gid) { Gitlab::GlobalId.build(report, id: report.id).to_s } + + let(:fields) do + <<~GRAPHQL + labels { + nodes { + id + title + description + color + textColor + } + } + GRAPHQL + end + + let(:arguments) { { id: report_gid } } + let(:query) { graphql_query_for('abuseReport', arguments, fields) } + + before do + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query that returns data' + + it 'returns abuse report with labels' do + expect(graphql_data_at('abuseReport', 'labels', 'nodes', 0)).to match(a_graphql_entity_for(label)) + end + + context 'when current user is not an admin' do + let_it_be(:current_user) { create(:user) } + + it_behaves_like 'a working graphql query' + + it 'does not contain any data' do + expect(graphql_data_at('abuseReportLabel')).to be_nil + end + end +end diff --git a/spec/requests/api/graphql/achievements/user_achievements_query_spec.rb b/spec/requests/api/graphql/achievements/user_achievements_query_spec.rb index 080f375245d..fa47cf4988a 100644 --- a/spec/requests/api/graphql/achievements/user_achievements_query_spec.rb +++ b/spec/requests/api/graphql/achievements/user_achievements_query_spec.rb @@ -14,8 +14,10 @@ RSpec.describe 'UserAchievements', feature_category: :user_profile do <<~HEREDOC id achievements { + count nodes { userAchievements { + count nodes { id achievement { @@ -58,6 +60,11 @@ RSpec.describe 'UserAchievements', feature_category: :user_profile do ) end + it 'returns the correct achievement and user_achievement counts' do + expect(graphql_data_at(:namespace, :achievements, :count)).to be(1) + expect(graphql_data_at(:namespace, :achievements, :nodes, :userAchievements, :count)).to contain_exactly(1) + end + it 'can lookahead to eliminate N+1 queries', :use_clean_rails_memory_store_caching do control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do post_graphql(query, current_user: user) diff --git a/spec/requests/api/graphql/ci/application_setting_spec.rb b/spec/requests/api/graphql/ci/application_setting_spec.rb index 42ab1786fee..a0c3bedd493 100644 --- a/spec/requests/api/graphql/ci/application_setting_spec.rb +++ b/spec/requests/api/graphql/ci/application_setting_spec.rb @@ -27,9 +27,7 @@ RSpec.describe 'getting Application Settings', feature_category: :continuous_int post_graphql(query, current_user: user) end - it_behaves_like 'a working graphql query' - - specify { expect(settings_data).to be nil } + it_behaves_like 'a working graphql query that returns no data' end context 'with admin permissions' do @@ -39,7 +37,7 @@ RSpec.describe 'getting Application Settings', feature_category: :continuous_int post_graphql(query, current_user: user) end - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query that returns data' it 'fetches the settings data' do # assert against hash to ensure no additional fields are exposed diff --git a/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb b/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb index 13a3a128979..d224fdbdc32 100644 --- a/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb +++ b/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb @@ -33,36 +33,55 @@ RSpec.describe 'Query.group(fullPath).environmentScopes', feature_category: :sec end before do - group.add_developer(user) expected_environment_scopes.each_with_index do |env, index| create(:ci_group_variable, group: group, key: "var#{index + 1}", environment_scope: env) end end - context 'when query has no parameters' do - let(:environment_scopes_params) { "" } + context 'when the user can administer the group' do + before do + group.add_owner(user) + end - it 'returns all avaiable environment scopes' do - post_graphql(query, current_user: user) + context 'when query has no parameters' do + let(:environment_scopes_params) { "" } - expect(graphql_data.dig('group', 'environmentScopes', 'nodes')).to eq( - expected_environment_scopes.map { |env_scope| { 'name' => env_scope } } - ) + it 'returns all avaiable environment scopes' do + post_graphql(query, current_user: user) + + expect(graphql_data.dig('group', 'environmentScopes', 'nodes')).to eq( + expected_environment_scopes.map { |env_scope| { 'name' => env_scope } } + ) + end + end + + context 'when query has search parameters' do + let(:environment_scopes_params) { "(search: \"group1\")" } + + it 'returns only environment scopes with group1 prefix' do + post_graphql(query, current_user: user) + + expect(graphql_data.dig('group', 'environmentScopes', 'nodes')).to eq( + [ + { 'name' => 'group1_environment1' }, + { 'name' => 'group1_environment2' } + ] + ) + end end end - context 'when query has search parameters' do - let(:environment_scopes_params) { "(search: \"group1\")" } + context 'when the user cannot administer the group' do + let(:environment_scopes_params) { "" } + + before do + group.add_developer(user) + end - it 'returns only environment scopes with group1 prefix' do + it 'returns nothing' do post_graphql(query, current_user: user) - expect(graphql_data.dig('group', 'environmentScopes', 'nodes')).to eq( - [ - { 'name' => 'group1_environment1' }, - { 'name' => 'group1_environment2' } - ] - ) + expect(graphql_data.dig('group', 'environmentScopes')).to be_nil end end end diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 6acd705c982..3cfb98c57fd 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -109,9 +109,9 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do runner.maintainer_note.present? ? a_string_including('<strong>Test maintenance note</strong>') : '', job_count: runner.builds.count, jobs: a_hash_including( - "count" => runner.builds.count, - "nodes" => an_instance_of(Array), - "pageInfo" => anything + 'count' => runner.builds.count, + 'nodes' => an_instance_of(Array), + 'pageInfo' => anything ), project_count: nil, admin_url: "http://localhost/admin/runners/#{runner.id}", @@ -124,8 +124,21 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do 'assignRunner' => true }, managers: a_hash_including( - "count" => runner.runner_managers.count, - "nodes" => an_instance_of(Array), + 'count' => runner.runner_managers.count, + 'nodes' => runner.runner_managers.map do |runner_manager| + a_graphql_entity_for( + runner_manager, + system_id: runner_manager.system_xid, + version: runner_manager.version, + revision: runner_manager.revision, + ip_address: runner_manager.ip_address, + executor_name: runner_manager.executor_type&.dasherize, + architecture_name: runner_manager.architecture, + platform_name: runner_manager.platform, + status: runner_manager.status.to_s.upcase, + job_execution_status: runner_manager.builds.running.any? ? 'RUNNING' : 'IDLE' + ) + end, "pageInfo" => anything ) ) @@ -215,11 +228,19 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do end end - context 'with build running' do + context 'with build running', :freeze_time do + let!(:pipeline) { create(:ci_pipeline, project: project1) } + let!(:runner_manager) do + create(:ci_runner_machine, + runner: runner, ip_address: '127.0.0.1', version: '16.3', revision: 'a', architecture: 'arm', platform: 'osx', + contacted_at: 1.second.ago, executor_type: 'docker') + end + + let!(:runner) { create(:ci_runner) } + let!(:build) { create(:ci_build, :running, runner: runner, pipeline: pipeline) } + before do - project = create(:project, :repository) - pipeline = create(:ci_pipeline, project: project) - create(:ci_build, :running, runner: runner, pipeline: pipeline) + create(:ci_runner_machine_build, runner_manager: runner_manager, build: build) end it_behaves_like 'runner details fetch' diff --git a/spec/requests/api/graphql/ci/runners_spec.rb b/spec/requests/api/graphql/ci/runners_spec.rb index c8706ae9698..3f6d39435fd 100644 --- a/spec/requests/api/graphql/ci/runners_spec.rb +++ b/spec/requests/api/graphql/ci/runners_spec.rb @@ -34,67 +34,116 @@ RSpec.describe 'Query.runners', feature_category: :runner_fleet do QUERY end - let(:query) do - %( - query { - runners(type:#{runner_type},status:#{status}) { - #{fields} + context 'with filters' do + let(:query) do + %( + query { + runners(type: #{runner_type}, status: #{status}) { + #{fields} + } } - } - ) - end - - before do - allow_next_instance_of(::Gitlab::Ci::RunnerUpgradeCheck) do |instance| - allow(instance).to receive(:check_runner_upgrade_suggestion) + ) end - post_graphql(query, current_user: current_user) - end - - shared_examples 'a working graphql query returning expected runner' do - it_behaves_like 'a working graphql query' + before do + allow_next_instance_of(::Gitlab::Ci::RunnerUpgradeCheck) do |instance| + allow(instance).to receive(:check_runner_upgrade_suggestion) + end - it 'returns expected runner' do - expect(runners_graphql_data['nodes']).to contain_exactly(a_graphql_entity_for(expected_runner)) + post_graphql(query, current_user: current_user) end - it 'does not execute more queries per runner', :aggregate_failures do - # warm-up license cache and so on: - personal_access_token = create(:personal_access_token, user: current_user) - args = { current_user: current_user, token: { personal_access_token: personal_access_token } } - post_graphql(query, **args) - expect(graphql_data_at(:runners, :nodes)).not_to be_empty + shared_examples 'a working graphql query returning expected runner' do + it_behaves_like 'a working graphql query' + + it 'returns expected runner' do + expect(runners_graphql_data['nodes']).to contain_exactly(a_graphql_entity_for(expected_runner)) + end + + it 'does not execute more queries per runner', :aggregate_failures do + # warm-up license cache and so on: + personal_access_token = create(:personal_access_token, user: current_user) + args = { current_user: current_user, token: { personal_access_token: personal_access_token } } + post_graphql(query, **args) + expect(graphql_data_at(:runners, :nodes)).not_to be_empty - admin2 = create(:admin) - personal_access_token = create(:personal_access_token, user: admin2) - args = { current_user: admin2, token: { personal_access_token: personal_access_token } } - control = ActiveRecord::QueryRecorder.new { post_graphql(query, **args) } + admin2 = create(:admin) + personal_access_token = create(:personal_access_token, user: admin2) + args = { current_user: admin2, token: { personal_access_token: personal_access_token } } + control = ActiveRecord::QueryRecorder.new { post_graphql(query, **args) } - create(:ci_runner, :instance, version: '14.0.0', tag_list: %w[tag5 tag6], creator: admin2) - create(:ci_runner, :project, version: '14.0.1', projects: [project], tag_list: %w[tag3 tag8], - creator: current_user) + create(:ci_runner, :instance, version: '14.0.0', tag_list: %w[tag5 tag6], creator: admin2) + create(:ci_runner, :project, version: '14.0.1', projects: [project], tag_list: %w[tag3 tag8], + creator: current_user) - expect { post_graphql(query, **args) }.not_to exceed_query_limit(control) + expect { post_graphql(query, **args) }.not_to exceed_query_limit(control) + end end - end - context 'runner_type is INSTANCE_TYPE and status is ACTIVE' do - let(:runner_type) { 'INSTANCE_TYPE' } - let(:status) { 'ACTIVE' } + context 'runner_type is INSTANCE_TYPE and status is ACTIVE' do + let(:runner_type) { 'INSTANCE_TYPE' } + let(:status) { 'ACTIVE' } - let!(:expected_runner) { instance_runner } + let!(:expected_runner) { instance_runner } - it_behaves_like 'a working graphql query returning expected runner' - end + it_behaves_like 'a working graphql query returning expected runner' + end - context 'runner_type is PROJECT_TYPE and status is NEVER_CONTACTED' do - let(:runner_type) { 'PROJECT_TYPE' } - let(:status) { 'NEVER_CONTACTED' } + context 'runner_type is PROJECT_TYPE and status is NEVER_CONTACTED' do + let(:runner_type) { 'PROJECT_TYPE' } + let(:status) { 'NEVER_CONTACTED' } - let!(:expected_runner) { project_runner } + let!(:expected_runner) { project_runner } + + it_behaves_like 'a working graphql query returning expected runner' + end + end - it_behaves_like 'a working graphql query returning expected runner' + context 'without filters' do + context 'with managers requested for multiple runners' do + let(:fields) do + <<~QUERY + nodes { + managers { + nodes { + #{all_graphql_fields_for('CiRunnerManager', max_depth: 1)} + } + } + } + QUERY + end + + let(:query) do + %( + query { + runners { + #{fields} + } + } + ) + end + + it 'does not execute more queries per runner', :aggregate_failures do + # warm-up license cache and so on: + personal_access_token = create(:personal_access_token, user: current_user) + args = { current_user: current_user, token: { personal_access_token: personal_access_token } } + post_graphql(query, **args) + expect(graphql_data_at(:runners, :nodes)).not_to be_empty + + admin2 = create(:admin) + personal_access_token = create(:personal_access_token, user: admin2) + args = { current_user: admin2, token: { personal_access_token: personal_access_token } } + control = ActiveRecord::QueryRecorder.new { post_graphql(query, **args) } + + create(:ci_runner, :instance, :with_runner_manager, version: '14.0.0', tag_list: %w[tag5 tag6], + creator: admin2) + create(:ci_runner, :project, :with_runner_manager, version: '14.0.1', projects: [project], + tag_list: %w[tag3 tag8], + creator: current_user) + + expect { post_graphql(query, **args) }.not_to exceed_query_limit(control) + end + end end end diff --git a/spec/requests/api/graphql/current_user/todos_query_spec.rb b/spec/requests/api/graphql/current_user/todos_query_spec.rb index ee019a99f8d..790ae4b955e 100644 --- a/spec/requests/api/graphql/current_user/todos_query_spec.rb +++ b/spec/requests/api/graphql/current_user/todos_query_spec.rb @@ -40,7 +40,7 @@ RSpec.describe 'Query current user todos', feature_category: :source_code_manage post_graphql(query, current_user: current_user) end - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query that returns data' it 'contains the expected ids' do is_expected.to contain_exactly( diff --git a/spec/requests/api/graphql/current_user_query_spec.rb b/spec/requests/api/graphql/current_user_query_spec.rb index aceef77920d..b4e570bcaaa 100644 --- a/spec/requests/api/graphql/current_user_query_spec.rb +++ b/spec/requests/api/graphql/current_user_query_spec.rb @@ -25,7 +25,7 @@ RSpec.describe 'getting project information', feature_category: :system_access d context 'when there is a current_user' do let_it_be(:current_user) { create(:user) } - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query that returns data' it { is_expected.to include('name' => current_user.name, 'namespace' => { 'id' => current_user.namespace.to_global_id.to_s }) } end @@ -33,8 +33,6 @@ RSpec.describe 'getting project information', feature_category: :system_access d context 'when there is no current_user' do let(:current_user) { nil } - it_behaves_like 'a working graphql query' - - it { is_expected.to be_nil } + it_behaves_like 'a working graphql query that returns no data' end end diff --git a/spec/requests/api/graphql/environments/deployments_spec.rb b/spec/requests/api/graphql/environments/deployments_spec.rb index 0022a38d2d3..a4abf3f583a 100644 --- a/spec/requests/api/graphql/environments/deployments_spec.rb +++ b/spec/requests/api/graphql/environments/deployments_spec.rb @@ -314,14 +314,17 @@ RSpec.describe 'Environments Deployments query', feature_category: :continuous_d end def create_deployments - create_list(:deployment, 3, environment: environment, project: project).each do |deployment| - deployment.user = create(:user).tap { |u| project.add_developer(u) } - deployment.deployable = - create(:ci_build, project: project, environment: environment.name, deployment: deployment, - user: deployment.user) + deployments = create_list(:deployment, 2, environment: environment, project: project) + set_deployment_attributes(deployments.first, :ci_build) + set_deployment_attributes(deployments.second, :ci_bridge) + deployments.each(&:save!) + end - deployment.save! - end + def set_deployment_attributes(deployment, factory_type) + deployment.user = create(:user).tap { |u| project.add_developer(u) } + deployment.deployable = + create(factory_type, project: project, environment: environment.name, deployment: deployment, + user: deployment.user) end end @@ -432,7 +435,7 @@ RSpec.describe 'Environments Deployments query', feature_category: :continuous_d deployments.each do |deployment| deployment_in_record = project.deployments.find_by_iid(deployment['iid']) - expect(deployment_in_record.build.to_global_id.to_s).to eq(deployment['job']['id']) + expect(deployment_in_record.job.to_global_id.to_s).to eq(deployment['job']['id']) end end end diff --git a/spec/requests/api/graphql/group/autocomplete_users_spec.rb b/spec/requests/api/graphql/group/autocomplete_users_spec.rb new file mode 100644 index 00000000000..708604885c9 --- /dev/null +++ b/spec/requests/api/graphql/group/autocomplete_users_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'autocomplete users for a group', feature_category: :team_planning do + include GraphqlHelpers + + let_it_be(:parent_group) { create(:group) } + let_it_be(:group) { create(:group, parent: parent_group) } + + let_it_be(:parent_group_member) { create(:user).tap { |u| parent_group.add_guest(u) } } + let_it_be(:group_member) { create(:user).tap { |u| group.add_guest(u) } } + + let_it_be(:other_group) { create(:group) } + let_it_be(:other_group_member) { create(:user).tap { |u| other_group.add_guest(u) } } + + let(:params) { {} } + let(:query) do + graphql_query_for( + 'group', + { 'fullPath' => group.full_path }, + query_graphql_field('autocompleteUsers', params, 'id') + ) + end + + let(:response_user_ids) { graphql_data.dig('group', 'autocompleteUsers').pluck('id') } + + it 'returns members of the group and its ancestors' do + post_graphql(query, current_user: group_member) + + expected_user_ids = [ + parent_group_member, + group_member + ].map { |u| u.to_global_id.to_s } + + expect(response_user_ids).to match_array(expected_user_ids) + end + + context 'with search param' do + let(:params) { { search: group_member.username } } + + it 'only returns users matching the search query' do + post_graphql(query, current_user: group_member) + + expect(response_user_ids).to contain_exactly(group_member.to_global_id.to_s) + end + end +end diff --git a/spec/requests/api/graphql/group/work_items_spec.rb b/spec/requests/api/graphql/group/work_items_spec.rb new file mode 100644 index 00000000000..f6dad577b5e --- /dev/null +++ b/spec/requests/api/graphql/group/work_items_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting a work_item list for a group', feature_category: :team_planning do + include GraphqlHelpers + + let_it_be(:group) { create(:group, :public) } + let_it_be(:sub_group) { create(:group, parent: group) } + let_it_be(:project) { create(:project, :repository, :public, group: group) } + let_it_be(:user) { create(:user) } + let_it_be(:reporter) { create(:user).tap { |user| group.add_reporter(user) } } + + let_it_be(:project_work_item) { create(:work_item, project: project) } + let_it_be(:sub_group_work_item) do + create( + :work_item, + namespace: sub_group, + author: reporter + ) + end + + let_it_be(:group_work_item) do + create( + :work_item, + namespace: group, + author: reporter + ) + end + + let_it_be(:confidential_work_item) do + create(:work_item, :confidential, namespace: group, author: reporter) + end + + let_it_be(:other_work_item) { create(:work_item) } + + let(:work_items_data) { graphql_data['group']['workItems']['nodes'] } + let(:work_item_filter_params) { {} } + let(:current_user) { user } + let(:query_group) { group } + + let(:fields) do + <<~QUERY + nodes { + #{all_graphql_fields_for('workItems'.classify, max_depth: 2)} + } + QUERY + end + + context 'when the user can not see confidential work_items' do + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + it 'does not return confidential issues' do + post_graphql(query, current_user: current_user) + + expect(work_item_ids).to contain_exactly( + group_work_item.to_global_id.to_s + ) + end + end + + context 'when the user can see confidential work_items' do + let(:current_user) { reporter } + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + it 'returns also confidential work_items' do + post_graphql(query, current_user: current_user) + + expect(work_item_ids).to eq([ + confidential_work_item.to_global_id.to_s, group_work_item.to_global_id.to_s + ]) + end + + context 'when the namespace_level_work_items feature flag is disabled' do + before do + stub_feature_flags(namespace_level_work_items: false) + post_graphql(query, current_user: current_user) + end + + it 'returns null in the workItems field' do + expect(graphql_data['group']['workItems']).to be_nil + end + end + end + + def work_item_ids + graphql_dig_at(work_items_data, :id) + end + + def query(params = work_item_filter_params) + graphql_query_for( + 'group', + { 'fullPath' => query_group.full_path }, + query_graphql_field('workItems', params, fields) + ) + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb index 18cc85d36e0..dbace8f2b53 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -57,7 +57,7 @@ RSpec.describe 'Adding an AwardEmoji', feature_category: :shared do it_behaves_like 'a mutation that does not create an AwardEmoji' it_behaves_like 'a mutation that returns top-level errors', - errors: ['You cannot award emoji to this resource.'] + errors: ['You cannot add emoji reactions to this resource.'] end context 'when the given awardable is an Awardable' do diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb index 7c6a487cdd0..65a5fb87f9a 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -56,7 +56,7 @@ RSpec.describe 'Toggling an AwardEmoji', feature_category: :shared do it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' it_behaves_like 'a mutation that returns top-level errors', - errors: ['You cannot award emoji to this resource.'] + errors: ['You cannot add emoji reactions to this resource.'] end context 'when the given awardable is an Awardable' do diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb index 0d5e5f5d2fb..b2fe2754198 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule/create_spec.rb @@ -68,8 +68,7 @@ RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integrati end end - # Move this from `shared_context` to `context` when `ci_refactoring_pipeline_schedule_create_service` is removed. - shared_context 'when authorized' do # rubocop:disable RSpec/ContextWording + context 'when authorized' do before_all do project.add_developer(user) end @@ -149,14 +148,4 @@ RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integrati end end end - - it_behaves_like 'when authorized' - - context 'when the FF ci_refactoring_pipeline_schedule_create_service is disabled' do - before do - stub_feature_flags(ci_refactoring_pipeline_schedule_create_service: false) - end - - it_behaves_like 'when authorized' - end end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_trigger/create_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_trigger/create_spec.rb new file mode 100644 index 00000000000..1af12d51e1e --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_trigger/create_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineTriggerCreate', feature_category: :continuous_integration do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:project) { create(:project) } + + let(:mutation) { graphql_mutation(:pipeline_trigger_create, params) } + let(:project_path) { project.full_path } + let(:description) { 'Ye old pipeline trigger token' } + + let(:params) do + { + project_path: project_path, + description: description + } + end + + subject { post_graphql_mutation(mutation, current_user: user) } + + context 'when unauthorized' do + it 'returns an error' do + subject + + expect(graphql_errors).not_to be_empty + expect(graphql_errors[0]['message']) + .to eq( + "The resource that you are attempting to access does not exist " \ + "or you don't have permission to perform this action" + ) + end + end + + context 'when authorized' do + before_all do + project.add_owner(user) + end + + context 'when the params are invalid' do + let(:description) { nil } + + it 'does not create a pipeline trigger token and returns an error' do + expect { subject }.not_to change { project.triggers.count } + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors.to_s).to include('provided invalid value for description (Expected value to not be null)') + end + end + + context 'when the params are valid' do + it 'creates a pipeline trigger token' do + expect { subject }.to change { project.triggers.count }.by(1) + expect(graphql_errors.to_s).to eql("") + end + + it 'returns the new pipeline trigger token' do + subject + + expect(graphql_data_at(:pipeline_trigger_create, :pipeline_trigger)).to match a_hash_including( + 'owner' => a_hash_including( + 'id' => user.to_global_id.to_s, + 'username' => user.username, + 'name' => user.name + ), + 'description' => description, + "canAccessProject" => true, + "hasTokenExposed" => true, + "lastUsed" => nil + ) + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_trigger/delete_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_trigger/delete_spec.rb new file mode 100644 index 00000000000..5ff2da30cb6 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_trigger/delete_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineTriggerDelete', feature_category: :continuous_integration do + include GraphqlHelpers + + let_it_be(:current_user) { build(:user) } + let_it_be(:project) { build(:project) } + + let(:mutation) { graphql_mutation(:pipeline_trigger_delete, params) } + + let_it_be(:trigger) { create(:ci_trigger, owner: current_user, project: project) } + let(:id) { trigger.to_global_id.to_s } + + let(:params) do + { + id: id + } + end + + subject { post_graphql_mutation(mutation, current_user: current_user) } + + context 'when unauthorized' do + it_behaves_like 'a mutation on an unauthorized resource' + end + + context 'when authorized' do + before_all do + project.add_owner(current_user) + end + + context 'when the id is invalid' do + let(:id) { non_existing_record_id } + + it_behaves_like 'an invalid argument to the mutation', argument_name: :id + + it 'does not delete a pipeline trigger token' do + expect { subject }.not_to change { project.triggers.count } + expect(response).to have_gitlab_http_status(:success) + end + end + + context 'when the id is nil' do + let(:id) { nil } + + it_behaves_like 'an invalid argument to the mutation', argument_name: :id + + it 'does not delete a pipeline trigger token' do + expect { subject }.not_to change { project.triggers.count } + expect(response).to have_gitlab_http_status(:success) + end + end + + context 'when the params are valid' do + it_behaves_like 'a working GraphQL mutation' + + it 'deletes the pipeline trigger token' do + expect { subject }.to change { project.triggers.count }.by(-1) + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_trigger/update_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_trigger/update_spec.rb new file mode 100644 index 00000000000..ce6e20c088e --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_trigger/update_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineTriggerUpdate', feature_category: :continuous_integration do + include GraphqlHelpers + + let_it_be(:current_user) { build(:user) } + let_it_be(:project) { build(:project) } + + let(:mutation) { graphql_mutation(:pipeline_trigger_update, params) } + let_it_be(:old_description) { "Boring old description." } + let(:new_description) { 'Awesome new description!' } + let_it_be(:trigger) { create(:ci_trigger, owner: current_user, project: project, description: old_description) } + + let(:params) do + { + id: trigger.to_global_id.to_s, + description: new_description + } + end + + subject { post_graphql_mutation(mutation, current_user: current_user) } + + context 'when unauthorized' do + it_behaves_like 'a mutation on an unauthorized resource' + end + + context 'when authorized' do + before_all do + project.add_owner(current_user) + end + + context 'when the params are invalid' do + let(:new_description) { nil } + + it_behaves_like 'an invalid argument to the mutation', argument_name: 'description' + + it 'does not update a pipeline trigger token' do + expect { subject }.not_to change { trigger } + expect(response).to have_gitlab_http_status(:success) + end + end + + context 'when the params are valid' do + it_behaves_like 'a working GraphQL mutation' + + it 'updates the pipeline trigger token' do + expect { subject }.to change { trigger.reload.description }.to(new_description) + + expect(graphql_errors).to be_blank + end + + it 'returns the updated trigger token' do + subject + + expect(graphql_data_at(:pipeline_trigger_update, :pipeline_trigger)).to match a_hash_including( + 'owner' => a_hash_including( + 'id' => current_user.to_global_id.to_s, + 'username' => current_user.username, + 'name' => current_user.name + ), + 'description' => new_description, + "canAccessProject" => true, + "hasTokenExposed" => true, + "lastUsed" => nil + ) + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/issues/update_spec.rb b/spec/requests/api/graphql/mutations/issues/update_spec.rb index 97ead687a82..ff100d99628 100644 --- a/spec/requests/api/graphql/mutations/issues/update_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/update_spec.rb @@ -147,5 +147,10 @@ RSpec.describe 'Update of an existing issue', feature_category: :team_planning d end end end + + it_behaves_like 'updating time estimate' do + let(:resource) { issue } + let(:mutation_name) { 'updateIssue' } + end end end diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_time_estimate_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_time_estimate_spec.rb new file mode 100644 index 00000000000..6bc130a97cf --- /dev/null +++ b/spec/requests/api/graphql/mutations/merge_requests/set_time_estimate_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Setting time estimate of a merge request', feature_category: :code_review_workflow do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + + let(:input) do + { + iid: merge_request.iid.to_s + } + end + + let(:extra_params) { { project_path: project.full_path } } + let(:input_params) { input.merge(extra_params) } + let(:mutation) { graphql_mutation(:merge_request_update, input_params, nil, ['productAnalyticsState']) } + let(:mutation_response) { graphql_mutation_response(:merge_request_update) } + + context 'when the user is not allowed to update a merge request' do + before_all do + project.add_reporter(current_user) + end + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when updating a time estimate' do + before_all do + project.add_developer(current_user) + end + + it_behaves_like 'updating time estimate' do + let(:resource) { merge_request } + let(:mutation_name) { 'mergeRequestUpdate' } + end + end +end diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb index d81744abe1b..0e55b6f2c9f 100644 --- a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/create_spec.rb @@ -43,9 +43,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ project.add_reporter(current_user) end - it_behaves_like 'a mutation that returns top-level errors', - errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] - it 'does not create the annotation' do expect do post_graphql_mutation(mutation, current_user: current_user) @@ -58,25 +55,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ project.add_developer(current_user) end - it 'creates the annotation' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.to change { Metrics::Dashboard::Annotation.count }.by(1) - end - - it 'returns the created annotation' do - post_graphql_mutation(mutation, current_user: current_user) - - annotation = Metrics::Dashboard::Annotation.first - annotation_id = GitlabSchema.id_from_object(annotation).to_s - - expect(mutation_response['annotation']['description']).to match(description) - expect(mutation_response['annotation']['startingAt'].to_time).to match(starting_at.to_time) - expect(mutation_response['annotation']['endingAt'].to_time).to match(ending_at.to_time) - expect(mutation_response['annotation']['id']).to match(annotation_id) - expect(annotation.environment_id).to eq(environment.id) - end - context 'when environment_id is missing' do let(:mutation) do variables = { @@ -137,25 +115,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ project.add_developer(current_user) end - it 'creates the annotation' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.to change { Metrics::Dashboard::Annotation.count }.by(1) - end - - it 'returns the created annotation' do - post_graphql_mutation(mutation, current_user: current_user) - - annotation = Metrics::Dashboard::Annotation.first - annotation_id = GitlabSchema.id_from_object(annotation).to_s - - expect(mutation_response['annotation']['description']).to match(description) - expect(mutation_response['annotation']['startingAt'].to_time).to match(starting_at.to_time) - expect(mutation_response['annotation']['endingAt'].to_time).to match(ending_at.to_time) - expect(mutation_response['annotation']['id']).to match(annotation_id) - expect(annotation.cluster_id).to eq(cluster.id) - end - context 'when cluster_id is missing' do let(:mutation) do variables = { @@ -177,9 +136,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create, feature_categ project.add_guest(current_user) end - it_behaves_like 'a mutation that returns top-level errors', - errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] - it 'does not create the annotation' do expect do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb index 09977cd19d7..c81f6381398 100644 --- a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb @@ -7,8 +7,7 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete, feature_categ let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project, :private, :repository) } - let_it_be(:environment) { create(:environment, project: project) } - let_it_be(:annotation) { create(:metrics_dashboard_annotation, environment: environment) } + let_it_be(:annotation) { create(:metrics_dashboard_annotation) } let(:variables) { { id: GitlabSchema.id_from_object(annotation).to_s } } let(:mutation) { graphql_mutation(:delete_annotation, variables) } @@ -28,14 +27,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete, feature_categ project.add_developer(current_user) end - context 'with valid params' do - it 'deletes the annotation' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.to change { Metrics::Dashboard::Annotation.count }.by(-1) - end - end - context 'with invalid params' do let(:variables) { { id: GitlabSchema.id_from_object(project).to_s } } @@ -44,21 +35,6 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete, feature_categ end end - context 'when the delete fails' do - let(:service_response) { { message: 'Annotation has not been deleted', status: :error, last_step: :delete } } - - before do - allow_next_instance_of(Metrics::Dashboard::Annotations::DeleteService) do |delete_service| - allow(delete_service).to receive(:execute).and_return(service_response) - end - end - it 'returns the error' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(mutation_response['errors']).to eq([service_response[:message]]) - end - end - context 'when metrics dashboard feature is unavailable' do before do stub_feature_flags(remove_monitor_metrics: true) diff --git a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb index 2f26a2f92b2..480e184a60c 100644 --- a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb +++ b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb @@ -15,6 +15,8 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis maven_duplicate_exception_regex: 'foo-.*', generic_duplicates_allowed: false, generic_duplicate_exception_regex: 'bar-.*', + nuget_duplicates_allowed: false, + nuget_duplicate_exception_regex: 'bar-.*', maven_package_requests_forwarding: true, lock_maven_package_requests_forwarding: true, npm_package_requests_forwarding: true, @@ -32,6 +34,8 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis mavenDuplicateExceptionRegex genericDuplicatesAllowed genericDuplicateExceptionRegex + nugetDuplicatesAllowed + nugetDuplicateExceptionRegex mavenPackageRequestsForwarding lockMavenPackageRequestsForwarding npmPackageRequestsForwarding @@ -58,6 +62,8 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis expect(package_settings_response['mavenDuplicateExceptionRegex']).to eq(params[:maven_duplicate_exception_regex]) expect(package_settings_response['genericDuplicatesAllowed']).to eq(params[:generic_duplicates_allowed]) expect(package_settings_response['genericDuplicateExceptionRegex']).to eq(params[:generic_duplicate_exception_regex]) + expect(package_settings_response['nugetDuplicatesAllowed']).to eq(params[:nuget_duplicates_allowed]) + expect(package_settings_response['nugetDuplicateExceptionRegex']).to eq(params[:nuget_duplicate_exception_regex]) expect(package_settings_response['mavenPackageRequestsForwarding']).to eq(params[:maven_package_requests_forwarding]) expect(package_settings_response['lockMavenPackageRequestsForwarding']).to eq(params[:lock_maven_package_requests_forwarding]) expect(package_settings_response['pypiPackageRequestsForwarding']).to eq(params[:pypi_package_requests_forwarding]) @@ -98,6 +104,8 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis maven_duplicate_exception_regex: 'SNAPSHOT', generic_duplicates_allowed: true, generic_duplicate_exception_regex: 'foo', + nuget_duplicates_allowed: true, + nuget_duplicate_exception_regex: 'foo', maven_package_requests_forwarding: nil, lock_maven_package_requests_forwarding: false, npm_package_requests_forwarding: nil, @@ -109,6 +117,8 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis maven_duplicate_exception_regex: 'foo-.*', generic_duplicates_allowed: false, generic_duplicate_exception_regex: 'bar-.*', + nuget_duplicates_allowed: false, + nuget_duplicate_exception_regex: 'bar-.*', maven_package_requests_forwarding: true, lock_maven_package_requests_forwarding: true, npm_package_requests_forwarding: true, @@ -119,6 +129,26 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis it_behaves_like 'returning a success' it_behaves_like 'rejecting invalid regex' + + context 'when nuget_duplicates_option FF is disabled' do + let(:params) do + { + namespace_path: namespace.full_path, + 'nugetDuplicatesAllowed' => false + } + end + + before do + stub_feature_flags(nuget_duplicates_option: false) + end + + it 'raises an error', :aggregate_failures do + subject + + expect(graphql_errors.size).to eq(1) + expect(graphql_errors.first['message']).to include('feature flag is disabled') + end + end end RSpec.shared_examples 'accepting the mutation request creating the package settings' do diff --git a/spec/requests/api/graphql/mutations/snippets/update_spec.rb b/spec/requests/api/graphql/mutations/snippets/update_spec.rb index 7c5ab691b51..06594d89338 100644 --- a/spec/requests/api/graphql/mutations/snippets/update_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/update_spec.rb @@ -188,16 +188,10 @@ RSpec.describe 'Updating a Snippet', feature_category: :source_code_management d stub_session('warden.user.user.key' => [[current_user.id], current_user.authenticatable_salt]) end - it_behaves_like 'Snowplow event tracking with RedisHLL context' do + it_behaves_like 'internal event tracking' do + let(:action) { ::Gitlab::UsageDataCounters::EditorUniqueCounter::EDIT_BY_SNIPPET_EDITOR } let(:user) { current_user } - let(:property) { 'g_edit_by_snippet_ide' } let(:namespace) { project.namespace } - let(:category) { 'Gitlab::UsageDataCounters::EditorUniqueCounter' } - let(:action) { 'ide_edit' } - let(:label) { 'usage_activity_by_stage_monthly.create.action_monthly_active_users_ide_edit' } - let(:context) do - [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: event_name).to_context] - end end end end diff --git a/spec/requests/api/graphql/mutations/work_items/create_spec.rb b/spec/requests/api/graphql/mutations/work_items/create_spec.rb index fca3c84e534..78b93c3210b 100644 --- a/spec/requests/api/graphql/mutations/work_items/create_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/create_spec.rb @@ -140,7 +140,7 @@ RSpec.describe 'Create a work item', feature_category: :team_planning do } end - before(:all) do + before_all do create(:parent_link, work_item_parent: parent, work_item: adjacent, relative_position: 0) end @@ -264,6 +264,14 @@ RSpec.describe 'Create a work item', feature_category: :team_planning do let(:mutation) { graphql_mutation(:workItemCreate, input.merge('namespacePath' => project.full_path), fields) } it_behaves_like 'creates work item' + + context 'when the namespace_level_work_items feature flag is disabled' do + before do + stub_feature_flags(namespace_level_work_items: false) + end + + it_behaves_like 'creates work item' + end end end @@ -272,6 +280,16 @@ RSpec.describe 'Create a work item', feature_category: :team_planning do let(:mutation) { graphql_mutation(:workItemCreate, input.merge(namespacePath: group.full_path), fields) } it_behaves_like 'creates work item' + + context 'when the namespace_level_work_items feature flag is disabled' do + before do + stub_feature_flags(namespace_level_work_items: false) + end + + it_behaves_like 'a mutation that returns top-level errors', errors: [ + Mutations::WorkItems::Create::DISABLED_FF_ERROR + ] + end end context 'when both projectPath and namespacePath are passed' do diff --git a/spec/requests/api/graphql/mutations/work_items/linked_items/add_spec.rb b/spec/requests/api/graphql/mutations/work_items/linked_items/add_spec.rb new file mode 100644 index 00000000000..f18e0e44905 --- /dev/null +++ b/spec/requests/api/graphql/mutations/work_items/linked_items/add_spec.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe "Add linked items to a work item", feature_category: :portfolio_management do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :private) } + let_it_be(:reporter) { create(:user).tap { |user| project.add_reporter(user) } } + let_it_be(:work_item) { create(:work_item, project: project) } + let_it_be(:related1) { create(:work_item, project: project) } + let_it_be(:related2) { create(:work_item, project: project) } + + let(:mutation_response) { graphql_mutation_response(:work_item_add_linked_items) } + let(:mutation) { graphql_mutation(:workItemAddLinkedItems, input, fields) } + + let(:ids_to_link) { [related1.to_global_id.to_s, related2.to_global_id.to_s] } + let(:input) { { 'id' => work_item.to_global_id.to_s, 'workItemsIds' => ids_to_link } } + + let(:fields) do + <<~FIELDS + workItem { + id + widgets { + type + ... on WorkItemWidgetLinkedItems { + linkedItems { + edges { + node { + linkType + workItem { + id + } + } + } + } + } + } + } + errors + message + FIELDS + end + + context 'when the user is not allowed to read the work item' do + let(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to read the work item' do + let(:current_user) { reporter } + + it 'links the work items' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { WorkItems::RelatedWorkItemLink.count }.by(2) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']).to include('id' => work_item.to_global_id.to_s) + expect(mutation_response['message']).to eq("Successfully linked ID(s): #{related1.id} and #{related2.id}.") + expect(mutation_response['workItem']['widgets']).to include( + { + 'linkedItems' => { 'edges' => match_array([ + { 'node' => { 'linkType' => 'relates_to', 'workItem' => { 'id' => related1.to_global_id.to_s } } }, + { 'node' => { 'linkType' => 'relates_to', 'workItem' => { 'id' => related2.to_global_id.to_s } } } + ]) }, + 'type' => 'LINKED_ITEMS' + } + ) + end + + context 'when linking a work item fails' do + let_it_be(:private_project) { create(:project, :private) } + let_it_be(:related2) { create(:work_item, project: private_project) } + + it 'adds valid items and returns an error message for failed item' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { WorkItems::RelatedWorkItemLink.count }.by(1) + + expect(mutation_response['errors']).to contain_exactly( + "Item with ID: #{related2.id} cannot be added. " \ + "You don't have permission to perform this action." + ) + end + + context 'when a work item does not exist' do + let(:input) do + { + 'id' => work_item.to_global_id.to_s, + 'workItemsIds' => ["gid://gitlab/WorkItem/#{non_existing_record_id}"] + } + end + + it 'returns an error message' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { WorkItems::RelatedWorkItemLink.count } + + expect_graphql_errors_to_include("Couldn't find WorkItem with 'id'=#{non_existing_record_id}") + end + end + + context 'when there are more than the max allowed items to link' do + let(:max_work_items) { Mutations::WorkItems::LinkedItems::Base::MAX_WORK_ITEMS } + let(:error_msg) { "No more than #{max_work_items} work items can be linked at the same time." } + + before do + max_work_items.times { |i| ids_to_link.push("gid://gitlab/WorkItem/#{i}") } + end + + it 'returns an error message' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { WorkItems::RelatedWorkItemLink.count } + + expect_graphql_errors_to_include("No more than #{max_work_items} work items can be linked at the same time.") + end + end + end + + context 'when `linked_work_items` feature flag is disabled' do + before do + stub_feature_flags(linked_work_items: false) + end + + it_behaves_like 'a mutation that returns a top-level access error' + end + end +end diff --git a/spec/requests/api/graphql/mutations/work_items/subscribe_spec.rb b/spec/requests/api/graphql/mutations/work_items/subscribe_spec.rb new file mode 100644 index 00000000000..00672332082 --- /dev/null +++ b/spec/requests/api/graphql/mutations/work_items/subscribe_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Subscribe to a work item', feature_category: :team_planning do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :private) } + let_it_be(:guest) { create(:user).tap { |user| project.add_guest(user) } } + let_it_be(:work_item) { create(:work_item, project: project) } + + let(:subscribed_state) { true } + let(:mutation_input) { { 'id' => work_item.to_global_id.to_s, 'subscribed' => subscribed_state } } + let(:mutation) { graphql_mutation(:workItemSubscribe, mutation_input, fields) } + let(:mutation_response) { graphql_mutation_response(:work_item_subscribe) } + let(:fields) do + <<~FIELDS + workItem { + widgets { + type + ... on WorkItemWidgetNotifications { + subscribed + } + } + } + errors + FIELDS + end + + context 'when user is not allowed to update subscription work items' do + let(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context + + context 'when user has permissions to update its subscription to the work items' do + let(:current_user) { guest } + + it "subscribe the user to the work item's notifications" do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { work_item.subscribed?(current_user, project) }.to(true) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']['widgets']).to include({ + 'type' => 'NOTIFICATIONS', + 'subscribed' => true + }) + end + + context 'when unsunscribing' do + let(:subscribed_state) { false } + + before do + create(:subscription, project: project, user: current_user, subscribable: work_item, subscribed: true) + end + + it "unsubscribe the user from the work item's notifications" do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { work_item.subscribed?(current_user, project) }.to(false) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']['widgets']).to include({ + 'type' => 'NOTIFICATIONS', + 'subscribed' => false + }) + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/work_items/update_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_spec.rb index ea9516f256c..cff21c10a5a 100644 --- a/spec/requests/api/graphql/mutations/work_items/update_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/update_spec.rb @@ -573,7 +573,7 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do end context 'when updating relative position' do - before(:all) do + before_all do create(:parent_link, work_item_parent: valid_parent, work_item: valid_child1) create(:parent_link, work_item_parent: valid_parent, work_item: valid_child2) end @@ -655,7 +655,7 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do let_it_be(:work_item, reload: true) { create(:work_item, :task, project: project) } context "when parent is already assigned" do - before(:all) do + before_all do create(:parent_link, work_item_parent: valid_parent, work_item: work_item) create(:parent_link, work_item_parent: valid_parent, work_item: valid_child1) create(:parent_link, work_item_parent: valid_parent, work_item: valid_child2) diff --git a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb index 7f586edd510..55d223daf27 100644 --- a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb @@ -74,6 +74,7 @@ RSpec.describe 'getting Alert Management Alerts', feature_category: :incident_ma 'details' => { 'custom.alert' => 'payload', 'runbook' => 'runbook' }, 'createdAt' => triggered_alert.created_at.strftime('%Y-%m-%dT%H:%M:%SZ'), 'updatedAt' => triggered_alert.updated_at.strftime('%Y-%m-%dT%H:%M:%SZ'), + 'metricsDashboardUrl' => nil, 'detailsUrl' => triggered_alert.details_url, 'prometheusAlert' => nil, 'runbook' => 'runbook' diff --git a/spec/requests/api/graphql/project/autocomplete_users_spec.rb b/spec/requests/api/graphql/project/autocomplete_users_spec.rb new file mode 100644 index 00000000000..7c416465ed4 --- /dev/null +++ b/spec/requests/api/graphql/project/autocomplete_users_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'autocomplete users for a project', feature_category: :team_planning do + include GraphqlHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, :public, group: group) } + + let_it_be(:direct_member) { create(:user).tap { |u| project.add_guest(u) } } + let_it_be(:indirect_member) { create(:user).tap { |u| group.add_guest(u) } } + + let_it_be(:group_invited_to_project) do + create(:group).tap { |g| create(:project_group_link, project: project, group: g) } + end + + let_it_be(:member_from_project_share) { create(:user).tap { |u| group_invited_to_project.add_guest(u) } } + + let_it_be(:group_invited_to_parent_group) do + create(:group).tap { |g| create(:group_group_link, shared_group: group, shared_with_group: g) } + end + + let_it_be(:member_from_parent_group_share) { create(:user).tap { |u| group_invited_to_parent_group.add_guest(u) } } + + let_it_be(:sibling_project) { create(:project, :repository, :public, group: group) } + let_it_be(:sibling_member) { create(:user).tap { |u| sibling_project.add_guest(u) } } + + let(:params) { {} } + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('autocompleteUsers', params, 'id') + ) + end + + let(:response_user_ids) { graphql_data.dig('project', 'autocompleteUsers').pluck('id') } + + it 'returns members of the project' do + post_graphql(query, current_user: direct_member) + + expected_user_ids = [ + direct_member, + indirect_member, + member_from_project_share, + member_from_parent_group_share + ].map { |u| u.to_global_id.to_s } + + expect(response_user_ids).to match_array(expected_user_ids) + end + + context 'with search param' do + let(:params) { { search: indirect_member.username } } + + it 'only returns users matching the search query' do + post_graphql(query, current_user: direct_member) + + expect(response_user_ids).to contain_exactly(indirect_member.to_global_id.to_s) + end + end + + context 'with merge request interaction' do + let(:merge_request) { create(:merge_request, source_project: project) } + let(:fields) do + <<~FIELDS + id + mergeRequestInteraction(id: "#{merge_request.to_global_id}") { + canMerge + } + FIELDS + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('autocompleteUsers', params, fields) + ) + end + + it 'returns MR state related to the users' do + project.add_maintainer(direct_member) + + post_graphql(query, current_user: direct_member) + + expect(graphql_data.dig('project', 'autocompleteUsers')).to include( + a_hash_including( + 'id' => direct_member.to_global_id.to_s, + 'mergeRequestInteraction' => { 'canMerge' => true } + ), + a_hash_including( + 'id' => indirect_member.to_global_id.to_s, + 'mergeRequestInteraction' => { 'canMerge' => false } + ) + ) + end + end +end diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index e3c4396e7d8..05ed0ed8729 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -396,6 +396,28 @@ RSpec.describe 'getting merge request listings nested in a project', feature_cat include_examples 'N+1 query check', skip_cached: false end + + context 'when requesting diffStats' do + let(:requested_fields) { ['diffStats { path }'] } + + before do + create_list(:merge_request_diff, 2, merge_request: merge_request_a) + create_list(:merge_request_diff, 2, merge_request: merge_request_b) + create_list(:merge_request_diff, 2, merge_request: merge_request_c) + end + + include_examples 'N+1 query check', skip_cached: false + + context 'when each merge request diff has no head_commit_sha' do + before do + [merge_request_a, merge_request_b, merge_request_c].each do |mr| + mr.merge_request_diffs.update!(head_commit_sha: nil) + end + end + + include_examples 'N+1 query check', skip_cached: false + end + end end describe 'performance' do diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index 478112b687a..4aba83dae92 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -361,6 +361,59 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team end end + context 'when fetching work item linked items widget' do + let_it_be(:related_items) { create_list(:work_item, 3, project: project, milestone: milestone1) } + + let(:fields) do + <<~GRAPHQL + nodes { + widgets { + type + ... on WorkItemWidgetLinkedItems { + linkedItems { + nodes { + linkId + linkType + linkCreatedAt + linkUpdatedAt + workItem { + id + widgets { + ... on WorkItemWidgetMilestone { + milestone { + id + } + } + } + } + } + } + } + } + } + GRAPHQL + end + + before do + create(:work_item_link, source: item1, target: related_items[0], link_type: 'relates_to') + end + + it 'executes limited number of N+1 queries', :use_sql_query_cache do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post_graphql(query, current_user: current_user) + end + + create(:work_item_link, source: item1, target: related_items[1], link_type: 'relates_to') + create(:work_item_link, source: item1, target: related_items[2], link_type: 'relates_to') + + expect_graphql_errors_to_be_empty + # TODO: Fix N+1 queries executed for the linked work item widgets + # https://gitlab.com/gitlab-org/gitlab/-/issues/420605 + expect { post_graphql(query, current_user: current_user) } + .not_to exceed_all_query_limit(control).with_threshold(11) + end + end + def item_ids graphql_dig_at(items_data, :node, :id) end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index 54f141d9401..783e96861b1 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -8,6 +8,7 @@ RSpec.describe 'getting project information', feature_category: :groups_and_proj let_it_be(:group) { create(:group) } let_it_be(:project, reload: true) { create(:project, :repository, group: group) } let_it_be(:current_user) { create(:user) } + let_it_be(:other_user) { create(:user) } let(:project_fields) { all_graphql_fields_for('project'.to_s.classify, max_depth: 1) } @@ -23,7 +24,60 @@ RSpec.describe 'getting project information', feature_category: :groups_and_proj it 'includes the project', :use_clean_rails_memory_store_caching, :request_store do post_graphql(query, current_user: current_user) - expect(graphql_data['project']).not_to be_nil + expect(graphql_data['project']).to include('id' => global_id_of(project).to_s) + end + + context 'when querying for pipeline triggers' do + let(:project_fields) { query_nodes(:pipeline_triggers) } + let(:pipeline_trigger) { project.triggers.first } + + before do + create(:ci_trigger, project: project, owner: current_user) + end + + it 'fetches the pipeline trigger tokens' do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :pipeline_triggers, :nodes).first).to match({ + 'id' => pipeline_trigger.to_global_id.to_s, + 'canAccessProject' => true, + 'description' => pipeline_trigger.description, + 'hasTokenExposed' => true, + 'lastUsed' => nil, + 'token' => pipeline_trigger.token + }) + end + + it 'does not produce N+1 queries' do + baseline = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } + + build_list(:ci_trigger, 2, owner: current_user, project: project) + + expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(baseline) + end + + context 'when other project member is not authorized to see the full token' do + before do + project.add_maintainer(other_user) + post_graphql(query, current_user: other_user) + end + + it 'shows truncated token' do + expect(graphql_data_at(:project, :pipeline_triggers, + :nodes).first['token']).to eql pipeline_trigger.token[0, 4] + end + end + + context 'when user is not a member of a public project' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + post_graphql(query, current_user: other_user) + end + + it 'cannot read the token' do + expect(graphql_data_at(:project, :pipeline_triggers, :nodes)).to eql([]) + end + end end end @@ -35,10 +89,10 @@ RSpec.describe 'getting project information', feature_category: :groups_and_proj it 'includes the project' do post_graphql(query, current_user: current_user) - expect(graphql_data['project']).not_to be_nil + expect(graphql_data['project']).to include('id' => global_id_of(project).to_s) end - it_behaves_like 'a working graphql query' do + it_behaves_like 'a working graphql query that returns data' do before do post_graphql(query, current_user: current_user) end @@ -239,13 +293,7 @@ RSpec.describe 'getting project information', feature_category: :groups_and_proj end context 'when the user does not have access to the project' do - it 'returns an empty field' do - post_graphql(query, current_user: current_user) - - expect(graphql_data['project']).to be_nil - end - - it_behaves_like 'a working graphql query' do + it_behaves_like 'a working graphql query that returns no data' do before do post_graphql(query, current_user: current_user) end diff --git a/spec/requests/api/graphql/user/user_achievements_query_spec.rb b/spec/requests/api/graphql/user/user_achievements_query_spec.rb index 27d32d07372..2e6c3dcba61 100644 --- a/spec/requests/api/graphql/user/user_achievements_query_spec.rb +++ b/spec/requests/api/graphql/user/user_achievements_query_spec.rb @@ -13,6 +13,7 @@ RSpec.describe 'UserAchievements', feature_category: :user_profile do let_it_be(:fields) do <<~HEREDOC userAchievements { + count nodes { id achievement { @@ -54,6 +55,10 @@ RSpec.describe 'UserAchievements', feature_category: :user_profile do ) end + it 'returns the correct user_achievement count' do + expect(graphql_data_at(:user, :userAchievements, :count)).to be(1) + end + it 'can lookahead to eliminate N+1 queries', :use_clean_rails_memory_store_caching do control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do post_graphql(query, current_user: user) diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index 6702224f303..fa354bc1f66 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -539,6 +539,79 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do ) end end + + describe 'linked items widget' do + let_it_be(:related_item1) { create(:work_item, project: project) } + let_it_be(:related_item2) { create(:work_item, project: project) } + let_it_be(:related_item3) { create(:work_item) } + let_it_be(:link1) { create(:work_item_link, source: work_item, target: related_item1, link_type: 'relates_to') } + let_it_be(:link2) { create(:work_item_link, source: work_item, target: related_item2, link_type: 'relates_to') } + let_it_be(:link3) { create(:work_item_link, source: work_item, target: related_item3, link_type: 'relates_to') } + + let(:work_item_fields) do + <<~GRAPHQL + id + widgets { + type + ... on WorkItemWidgetLinkedItems { + linkedItems { + nodes { + linkId + linkType + linkCreatedAt + linkUpdatedAt + workItem { + id + } + } + } + } + } + GRAPHQL + end + + it 'returns widget information' do + expect(work_item_data).to include( + 'widgets' => include( + hash_including( + 'type' => 'LINKED_ITEMS', + 'linkedItems' => { 'nodes' => match_array( + [ + hash_including( + 'linkId' => link1.to_gid.to_s, 'linkType' => 'relates_to', + 'linkCreatedAt' => link1.created_at.iso8601, 'linkUpdatedAt' => link1.updated_at.iso8601, + 'workItem' => { 'id' => related_item1.to_gid.to_s } + ), + hash_including( + 'linkId' => link2.to_gid.to_s, 'linkType' => 'relates_to', + 'linkCreatedAt' => link2.created_at.iso8601, 'linkUpdatedAt' => link2.updated_at.iso8601, + 'workItem' => { 'id' => related_item2.to_gid.to_s } + ) + ] + ) } + ) + ) + ) + end + + context 'when `linked_work_items` feature flag is disabled' do + before do + stub_feature_flags(linked_work_items: false) + post_graphql(query, current_user: current_user) + end + + it 'returns empty result' do + expect(work_item_data).to include( + 'widgets' => include( + hash_including( + 'type' => 'LINKED_ITEMS', + 'linkedItems' => { "nodes" => [] } + ) + ) + ) + end + end + end end describe 'notes widget' do diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 12b7b8d7054..fa35e367420 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -1012,6 +1012,21 @@ RSpec.describe API::Internal::Base, feature_category: :system_access do end end + context 'when result is not ::Gitlab::GitAccessResult::Success or ::Gitlab::GitAccessResult::CustomAction' do + it 'responds with 500' do + personal_project = create(:project, namespace: user.namespace) + + allow_next_instance_of(Gitlab::GitAccess) do |access| + allow(access).to receive(:check).and_return(nil) + end + push(key, personal_project) + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(json_response['status']).to be_falsey + expect(json_response['message']).to eq(::API::Helpers::InternalHelpers::UNKNOWN_CHECK_RESULT_ERROR) + end + end + context "archived project" do before do project.add_developer(user) diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index 09170ca952f..ec30840dfd8 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -4,7 +4,11 @@ require 'spec_helper' RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_management do let(:jwt_auth_headers) do - jwt_token = JWT.encode({ 'iss' => Gitlab::Kas::JWT_ISSUER }, Gitlab::Kas.secret, 'HS256') + jwt_token = JWT.encode( + { 'iss' => Gitlab::Kas::JWT_ISSUER, 'aud' => Gitlab::Kas::JWT_AUDIENCE }, + Gitlab::Kas.secret, + 'HS256' + ) { Gitlab::Kas::INTERNAL_API_REQUEST_HEADER => jwt_token } end diff --git a/spec/requests/api/internal/pages_spec.rb b/spec/requests/api/internal/pages_spec.rb index 1006319eabf..65aa2326af5 100644 --- a/spec/requests/api/internal/pages_spec.rb +++ b/spec/requests/api/internal/pages_spec.rb @@ -151,20 +151,6 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do project.mark_pages_as_deployed end - context 'when the feature flag is disabled' do - before do - stub_feature_flags(pages_unique_domain: false) - end - - context 'when there are no pages deployed for the related project' do - it 'responds with 204 No Content' do - get api('/internal/pages'), headers: auth_header, params: { host: 'unique-domain.example.com' } - - expect(response).to have_gitlab_http_status(:no_content) - end - end - end - context 'when the unique domain is disabled' do before do project.project_setting.update!(pages_unique_domain_enabled: false) diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index b5d7d564749..a9bc38ae77c 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -484,6 +484,18 @@ RSpec.describe API::Labels, feature_category: :team_planning do let(:params) { { name: valid_label_title_1 } } end + context 'when lock_on_merge' do + let(:label_locked) { create(:label, title: 'Locked label', project: project, lock_on_merge: true) } + + it 'returns 400 because label could not be deleted' do + delete api("/projects/#{project.id}/labels", user), params: { label_id: label_locked.id } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('Label is locked and was not removed') + expect(project.labels).to include(label_locked) + end + end + context 'with group label' do let_it_be(:group) { create(:group) } let_it_be(:group_label) { create(:group_label, title: valid_group_label_title_1, group: group) } diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index f4cac0854e7..4edcd66e91a 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -2,15 +2,14 @@ require "spec_helper" -RSpec.describe API::MergeRequests, :aggregate_failures, feature_category: :source_code_management, - quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/418757' do +RSpec.describe API::MergeRequests, :aggregate_failures, feature_category: :source_code_management do include ProjectForksHelper let_it_be(:base_time) { Time.now } let_it_be(:user) { create(:user) } let_it_be(:user2) { create(:user) } let_it_be(:admin) { create(:user, :admin) } - let_it_be(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) } + let_it_be_with_refind(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) } let(:milestone1) { create(:milestone, title: '0.9', project: project) } let(:milestone) { create(:milestone, title: '1.0.0', project: project) } @@ -167,42 +166,6 @@ RSpec.describe API::MergeRequests, :aggregate_failures, feature_category: :sourc expect(merge_request.reload.merge_status).to eq('unchecked') end end - - context 'when restrict_merge_status_recheck FF is disabled' do - before do - stub_feature_flags(restrict_merge_status_recheck: false) - end - - context 'with batched_api_mergeability_checks FF on' do - it 'checks mergeability asynchronously in batch', :sidekiq_inline do - get(api(endpoint_path, user2), params: { with_merge_status_recheck: true }) - - expect_successful_response_with_paginated_array - - expect(merge_request.reload.merge_status).to eq('can_be_merged') - end - end - - context 'with batched_api_mergeability_checks FF off' do - before do - stub_feature_flags(batched_api_mergeability_checks: false) - end - - context 'with merge status recheck projection' do - it 'does enqueue a merge status recheck' do - expect_next_instances_of(check_service_class, (1..2)) do |service| - expect(service).not_to receive(:execute) - expect(service).to receive(:async_execute).and_call_original - end - - get(api(endpoint_path, user2), params: { with_merge_status_recheck: true }) - - expect_successful_response_with_paginated_array - expect(mr_entity['merge_status']).to eq('checking') - end - end - end - end end end diff --git a/spec/requests/api/metrics/dashboard/annotations_spec.rb b/spec/requests/api/metrics/dashboard/annotations_spec.rb index 250fe2a3ee3..6000fc2a6b7 100644 --- a/spec/requests/api/metrics/dashboard/annotations_spec.rb +++ b/spec/requests/api/metrics/dashboard/annotations_spec.rb @@ -21,77 +21,6 @@ RSpec.describe API::Metrics::Dashboard::Annotations, feature_category: :metrics end context "with :source_type == #{source_type.pluralize}" do - context 'with correct permissions' do - context 'with valid parameters' do - it 'creates a new annotation', :aggregate_failures do - post api(url, user), params: params - - expect(response).to have_gitlab_http_status(:created) - expect(json_response["#{source_type}_id"]).to eq(source.id) - expect(json_response['starting_at'].to_time).to eq(starting_at.to_time) - expect(json_response['ending_at'].to_time).to eq(ending_at.to_time) - expect(json_response['description']).to eq(params[:description]) - expect(json_response['dashboard_path']).to eq(dashboard) - end - end - - context 'with invalid parameters' do - it 'returns error message' do - post api(url, user), params: { dashboard_path: '', starting_at: nil, description: nil } - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to include({ "starting_at" => ["can't be blank"], "description" => ["can't be blank"], "dashboard_path" => ["can't be blank"] }) - end - end - - context 'with undeclared params' do - before do - params[:undeclared_param] = 'xyz' - end - - it 'filters out undeclared params' do - expect(::Metrics::Dashboard::Annotations::CreateService).to receive(:new).with(user, hash_excluding(:undeclared_param)) - - post api(url, user), params: params - end - end - - context 'with special characers in dashboard_path in request body' do - let(:dashboard_escaped) { 'config/prometheus/common_metrics%26copy.yml' } - let(:dashboard_unescaped) { 'config/prometheus/common_metrics©.yml' } - - shared_examples 'special characters unescaped' do - let(:expected_params) do - { - 'starting_at' => starting_at.to_time, - 'ending_at' => ending_at.to_time, - source_type.to_s => source, - 'dashboard_path' => dashboard_unescaped, - 'description' => params[:description] - } - end - - it 'unescapes the dashboard_path', :aggregate_failures do - expect(::Metrics::Dashboard::Annotations::CreateService).to receive(:new).with(user, expected_params) - - post api(url, user), params: params - end - end - - context 'with escaped characters' do - it_behaves_like 'special characters unescaped' do - let(:dashboard) { dashboard_escaped } - end - end - - context 'with unescaped characers' do - it_behaves_like 'special characters unescaped' do - let(:dashboard) { dashboard_unescaped } - end - end - end - end - context 'without correct permissions' do let_it_be(:guest) { create(:user) } @@ -102,7 +31,7 @@ RSpec.describe API::Metrics::Dashboard::Annotations, feature_category: :metrics it 'returns error message' do post api(url, guest), params: params - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) end end diff --git a/spec/requests/api/metrics/user_starred_dashboards_spec.rb b/spec/requests/api/metrics/user_starred_dashboards_spec.rb index 6fc98de0777..bdeba777350 100644 --- a/spec/requests/api/metrics/user_starred_dashboards_spec.rb +++ b/spec/requests/api/metrics/user_starred_dashboards_spec.rb @@ -15,54 +15,13 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d } end - before do - stub_feature_flags(remove_monitor_metrics: false) - end - describe 'POST /projects/:id/metrics/user_starred_dashboards' do before do project.add_reporter(user) end context 'with correct permissions' do - context 'with valid parameters' do - context 'dashboard_path as url param url escaped' do - it 'creates a new user starred metrics dashboard', :aggregate_failures do - post api(url, user), params: params - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['project_id']).to eq(project.id) - expect(json_response['user_id']).to eq(user.id) - expect(json_response['dashboard_path']).to eq(dashboard) - end - end - - context 'dashboard_path in request body unescaped' do - let(:params) do - { - dashboard_path: dashboard - } - end - - it 'creates a new user starred metrics dashboard', :aggregate_failures do - post api(url, user), params: params - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['project_id']).to eq(project.id) - expect(json_response['user_id']).to eq(user.id) - expect(json_response['dashboard_path']).to eq(dashboard) - end - end - end - context 'with invalid parameters' do - it 'returns error message' do - post api(url, user), params: { dashboard_path: '' } - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('dashboard_path is empty') - end - context 'user is missing' do it 'returns 404 not found' do post api(url, nil), params: params @@ -90,10 +49,6 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d end context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - it 'returns 404 not found' do post api(url, user), params: params @@ -113,44 +68,6 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d end context 'with correct permissions' do - context 'with valid parameters' do - context 'dashboard_path as url param url escaped' do - it 'deletes given user starred metrics dashboard', :aggregate_failures do - delete api(url, user), params: params - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['deleted_rows']).to eq(1) - expect(::Metrics::UsersStarredDashboard.all.pluck(:dashboard_path)).not_to include(dashboard) - end - end - - context 'dashboard_path in request body unescaped' do - let(:params) do - { - dashboard_path: dashboard - } - end - - it 'deletes given user starred metrics dashboard', :aggregate_failures do - delete api(url, user), params: params - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['deleted_rows']).to eq(1) - expect(::Metrics::UsersStarredDashboard.all.pluck(:dashboard_path)).not_to include(dashboard) - end - end - - context 'dashboard_path has not been specified' do - it 'deletes all starred dashboards for that user within given project', :aggregate_failures do - delete api(url, user), params: {} - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['deleted_rows']).to eq(2) - expect(::Metrics::UsersStarredDashboard.all).to contain_exactly(other_user_starred_dashboard, other_project_starred_dashboard) - end - end - end - context 'with invalid parameters' do context 'user is missing' do it 'returns 404 not found' do @@ -179,10 +96,6 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d end context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - it 'returns 404 not found' do delete api(url, user), params: params diff --git a/spec/requests/api/ml/mlflow/runs_spec.rb b/spec/requests/api/ml/mlflow/runs_spec.rb index a85fe4d867a..45479666e9a 100644 --- a/spec/requests/api/ml/mlflow/runs_spec.rb +++ b/spec/requests/api/ml/mlflow/runs_spec.rb @@ -39,6 +39,11 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do response end + before do + allow(Gitlab::Application.routes).to receive(:default_url_options) + .and_return(protocol: 'http', host: 'www.example.com', script_name: '') + end + RSpec.shared_examples 'MLflow|run_id param error cases' do context 'when run id is not passed' do let(:params) { {} } @@ -162,6 +167,17 @@ RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do }) end + context 'with a relative root URL' do + before do + allow(Gitlab::Application.routes).to receive(:default_url_options) + .and_return(protocol: 'http', host: 'www.example.com', script_name: '/gitlab/root') + end + + it 'gets a run including a valid artifact_uri' do + expect(json_response['run']['info']['artifact_uri']).to eql("http://www.example.com/gitlab/root/api/v4/projects/#{project_id}/packages/generic/ml_experiment_#{experiment.iid}/#{candidate.iid}/") + end + end + describe 'Error States' do it_behaves_like 'MLflow|run_id param error cases' it_behaves_like 'MLflow|shared error cases' diff --git a/spec/requests/api/npm_group_packages_spec.rb b/spec/requests/api/npm_group_packages_spec.rb index 431c59cf1b8..fe0bf1d8b46 100644 --- a/spec/requests/api/npm_group_packages_spec.rb +++ b/spec/requests/api/npm_group_packages_spec.rb @@ -2,8 +2,7 @@ require 'spec_helper' -RSpec.describe API::NpmGroupPackages, feature_category: :package_registry, - quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/418757' do +RSpec.describe API::NpmGroupPackages, feature_category: :package_registry do using RSpec::Parameterized::TableSyntax include_context 'npm api setup' diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index 8c0b9572af3..340420e46e0 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -2,8 +2,7 @@ require 'spec_helper' -RSpec.describe API::NpmProjectPackages, feature_category: :package_registry, - quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/418757' do +RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do include ExclusiveLeaseHelpers include_context 'npm api setup' @@ -72,6 +71,27 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry, it_behaves_like 'enqueue a worker to sync a metadata cache' end end + + context 'when user is not authorized after exception was raised' do + let(:exception) { Rack::Timeout::RequestTimeoutException.new('Request ran for longer than 60000ms') } + + subject { get(url) } + + before do + project.add_developer(user) + end + + it 'correctly reports an exception', :aggregate_failures do + allow_next_instance_of(Packages::Npm::GenerateMetadataService) do |instance| + allow(instance).to receive(:execute).and_raise(exception) + end + + allow(Gitlab::Auth::UniqueIpsLimiter).to receive(:limit_user!) + .and_invoke(-> { nil }, -> { raise Gitlab::Auth::UnauthorizedError }) + + subject + end + end end describe 'GET /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags' do diff --git a/spec/requests/api/nuget_group_packages_spec.rb b/spec/requests/api/nuget_group_packages_spec.rb index 07199119cb5..92eb869b871 100644 --- a/spec/requests/api/nuget_group_packages_spec.rb +++ b/spec/requests/api/nuget_group_packages_spec.rb @@ -31,6 +31,12 @@ RSpec.describe API::NugetGroupPackages, feature_category: :package_registry do end end + describe 'GET /api/v4/groups/:id/-/packages/nuget/v2' do + it_behaves_like 'handling nuget service requests', v2: true do + let(:url) { "/groups/#{target.id}/-/packages/nuget/v2" } + end + end + describe 'GET /api/v4/groups/:id/-/packages/nuget/metadata/*package_name/index' do it_behaves_like 'handling nuget metadata requests with package name', example_names_with_status: diff --git a/spec/requests/api/nuget_project_packages_spec.rb b/spec/requests/api/nuget_project_packages_spec.rb index 887dfd4beeb..da74409cd77 100644 --- a/spec/requests/api/nuget_project_packages_spec.rb +++ b/spec/requests/api/nuget_project_packages_spec.rb @@ -42,6 +42,52 @@ RSpec.describe API::NugetProjectPackages, feature_category: :package_registry do it_behaves_like 'accept get request on private project with access to package registry for everyone' end + describe 'GET /api/v4/projects/:id/packages/nuget/v2' do + let(:url) { "/projects/#{target.id}/packages/nuget/v2" } + + it_behaves_like 'handling nuget service requests', v2: true + + it_behaves_like 'accept get request on private project with access to package registry for everyone' + end + + describe 'GET /api/v4/projects/:id/packages/nuget/v2/$metadata' do + let(:url) { "/projects/#{target.id}/packages/nuget/v2/$metadata" } + + subject(:api_request) { get api(url) } + + it { is_expected.to have_request_urgency(:low) } + + context 'with valid target' do + using RSpec::Parameterized::TableSyntax + + where(:visibility_level, :user_role, :member, :expected_status) do + 'PUBLIC' | :developer | true | :success + 'PUBLIC' | :guest | true | :success + 'PUBLIC' | :developer | false | :success + 'PUBLIC' | :guest | false | :success + 'PUBLIC' | :anonymous | false | :success + 'PRIVATE' | :developer | true | :success + 'PRIVATE' | :guest | true | :success + 'PRIVATE' | :developer | false | :success + 'PRIVATE' | :guest | false | :success + 'PRIVATE' | :anonymous | false | :success + 'INTERNAL' | :developer | true | :success + 'INTERNAL' | :guest | true | :success + 'INTERNAL' | :developer | false | :success + 'INTERNAL' | :guest | false | :success + 'INTERNAL' | :anonymous | false | :success + end + + with_them do + before do + update_visibility_to(Gitlab::VisibilityLevel.const_get(visibility_level, false)) + end + + it_behaves_like 'process nuget v2 $metadata service request', params[:user_role], params[:expected_status], params[:member] + end + end + end + describe 'GET /api/v4/projects/:id/packages/nuget/metadata/*package_name/index' do let(:url) { "/projects/#{target.id}/packages/nuget/metadata/#{package_name}/index.json" } @@ -125,7 +171,7 @@ RSpec.describe API::NugetProjectPackages, feature_category: :package_registry do end describe 'GET /api/v4/projects/:id/packages/nuget/download/*package_name/*package_version/*package_filename' do - let_it_be(:package) { create(:nuget_package, :with_symbol_package, project: project, name: package_name) } + let_it_be(:package) { create(:nuget_package, :with_symbol_package, :with_metadatum, project: project, name: package_name, version: '0.1') } let(:format) { 'nupkg' } let(:url) { "/projects/#{target.id}/packages/nuget/download/#{package.name}/#{package.version}/#{package.name}.#{package.version}.#{format}" } @@ -183,75 +229,39 @@ RSpec.describe API::NugetProjectPackages, feature_category: :package_registry do end describe 'PUT /api/v4/projects/:id/packages/nuget/authorize' do - include_context 'workhorse headers' - - let(:url) { "/projects/#{target.id}/packages/nuget/authorize" } - let(:headers) { {} } - - subject { put api(url), headers: headers } - - it_behaves_like 'nuget authorize upload endpoint' + it_behaves_like 'nuget authorize upload endpoint' do + let(:url) { "/projects/#{target.id}/packages/nuget/authorize" } + end end describe 'PUT /api/v4/projects/:id/packages/nuget' do - include_context 'workhorse headers' - - let_it_be(:file_name) { 'package.nupkg' } - - let(:url) { "/projects/#{target.id}/packages/nuget" } - let(:headers) { {} } - let(:params) { { package: temp_file(file_name) } } - let(:file_key) { :package } - let(:send_rewritten_field) { true } - - subject do - workhorse_finalize( - api(url), - method: :put, - file_key: file_key, - params: params, - headers: headers, - send_rewritten_field: send_rewritten_field - ) + it_behaves_like 'nuget upload endpoint' do + let(:url) { "/projects/#{target.id}/packages/nuget" } end - - it_behaves_like 'nuget upload endpoint' end describe 'PUT /api/v4/projects/:id/packages/nuget/symbolpackage/authorize' do - include_context 'workhorse headers' - - let(:url) { "/projects/#{target.id}/packages/nuget/symbolpackage/authorize" } - let(:headers) { {} } - - subject { put api(url), headers: headers } - - it_behaves_like 'nuget authorize upload endpoint' + it_behaves_like 'nuget authorize upload endpoint' do + let(:url) { "/projects/#{target.id}/packages/nuget/symbolpackage/authorize" } + end end describe 'PUT /api/v4/projects/:id/packages/nuget/symbolpackage' do - include_context 'workhorse headers' - - let_it_be(:file_name) { 'package.snupkg' } - - let(:url) { "/projects/#{target.id}/packages/nuget/symbolpackage" } - let(:headers) { {} } - let(:params) { { package: temp_file(file_name) } } - let(:file_key) { :package } - let(:send_rewritten_field) { true } - - subject do - workhorse_finalize( - api(url), - method: :put, - file_key: file_key, - params: params, - headers: headers, - send_rewritten_field: send_rewritten_field - ) + it_behaves_like 'nuget upload endpoint', symbol_package: true do + let(:url) { "/projects/#{target.id}/packages/nuget/symbolpackage" } + end + end + + describe 'PUT /api/v4/projects/:id/packages/nuget/v2/authorize' do + it_behaves_like 'nuget authorize upload endpoint' do + let(:url) { "/projects/#{target.id}/packages/nuget/v2/authorize" } end + end - it_behaves_like 'nuget upload endpoint', symbol_package: true + describe 'PUT /api/v4/projects/:id/packages/nuget/v2' do + it_behaves_like 'nuget upload endpoint' do + let(:url) { "/projects/#{target.id}/packages/nuget/v2" } + end end def update_visibility_to(visibility) diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 86ff739da7e..aa8568d4951 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -60,6 +60,7 @@ itself: # project - container_registry_image_prefix - default_branch - empty_repo + - emails_disabled - forks_count - http_url_to_repo - import_status @@ -98,6 +99,7 @@ ci_cd_settings: remapped_attributes: default_git_depth: ci_default_git_depth forward_deployment_enabled: ci_forward_deployment_enabled + forward_deployment_rollback_allowed: ci_forward_deployment_rollback_allowed job_token_scope_enabled: ci_job_token_scope_enabled separated_caches: ci_separated_caches allow_fork_pipelines_to_run_in_parent_project: ci_allow_fork_pipelines_to_run_in_parent_project @@ -163,7 +165,6 @@ project_setting: - jitsu_key - mirror_branch_regex - allow_pipeline_trigger_approve_deployment - - emails_enabled - pages_unique_domain_enabled - pages_unique_domain - runner_registration_enabled diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index f5d1bbbc7e8..26132215404 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2583,7 +2583,6 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and link = create(:project_group_link, project: project, group: group) get api(path, admin, admin_mode: true) - expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq(project.id) expect(json_response['description']).to eq(project.description) @@ -2634,6 +2633,8 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect(json_response['feature_flags_access_level']).to be_present expect(json_response['infrastructure_access_level']).to be_present expect(json_response['monitor_access_level']).to be_present + expect(json_response).to have_key('emails_disabled') + expect(json_response).to have_key('emails_enabled') end it 'exposes all necessary attributes' do @@ -2707,7 +2708,6 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect(json_response['feature_flags_access_level']).to be_present expect(json_response['infrastructure_access_level']).to be_present expect(json_response['monitor_access_level']).to be_present - expect(json_response).to have_key('emails_disabled') expect(json_response['resolve_outdated_diff_discussions']).to eq(project.resolve_outdated_diff_discussions) expect(json_response['remove_source_branch_after_merge']).to be_truthy expect(json_response['container_registry_enabled']).to be_present @@ -2738,6 +2738,7 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) expect(json_response['ci_default_git_depth']).to eq(project.ci_default_git_depth) expect(json_response['ci_forward_deployment_enabled']).to eq(project.ci_forward_deployment_enabled) + expect(json_response['ci_forward_deployment_rollback_allowed']).to eq(project.ci_forward_deployment_rollback_allowed) expect(json_response['ci_allow_fork_pipelines_to_run_in_parent_project']).to eq(project.ci_allow_fork_pipelines_to_run_in_parent_project) expect(json_response['ci_separated_caches']).to eq(project.ci_separated_caches) expect(json_response['merge_method']).to eq(project.merge_method.to_s) @@ -2769,6 +2770,45 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect(json_response['name']).to eq(project.name) end + context 'when a project is moved' do + let(:redirect_route) { 'new/project/location' } + let(:perform_request) { get api("/projects/#{CGI.escape(redirect_route)}", user), params: { license: true } } + + before do + project.route.create_redirect(redirect_route) + end + + it 'redirects to the new project location' do + perform_request + + expect(response).to have_gitlab_http_status(:moved_permanently) + + url = response.headers['Location'] + expect(url).to start_with("#{request.base_url}/api/v4/projects/#{project.id}") + expect(CGI.parse(URI(url).query)).to include({ 'license' => ['true'] }) + end + + context 'when a user do not have access' do + let(:user) { create(:user) } + + it 'returns a 404 error' do + perform_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when api_redirect_moved_projects is disabled' do + it 'returns a 404 error' do + stub_feature_flags(api_redirect_moved_projects: false) + + perform_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + it 'returns a 404 error if not found' do get api("/projects/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) @@ -3081,6 +3121,7 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect(json_response).not_to include( 'ci_default_git_depth', 'ci_forward_deployment_enabled', + 'ci_forward_deployment_rollback_allowed', 'ci_job_token_scope_enabled', 'ci_separated_caches', 'ci_allow_fork_pipelines_to_run_in_parent_project', @@ -3654,7 +3695,7 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and end.to change { project.members.count }.by(2) expect(response).to have_gitlab_http_status(:created) - expect(json_response['message']).to eq('Successfully imported') + expect(json_response['status']).to eq('success') end it 'returns 404 if the source project does not exist' do @@ -3712,6 +3753,22 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(json_response['message']).to eq('Import failed') end + + context 'when importing of members did not work for some or all members' do + it 'fails to import some members' do + project_bot = create(:user, :project_bot) + project2.add_developer(project_bot) + + expect do + post api(path, user) + end.to change { project.members.count }.by(2) + + expect(response).to have_gitlab_http_status(:created) + error_message = { project_bot.username => 'User project bots cannot be added to other groups / projects' } + expect(json_response['message']).to eq(error_message) + expect(json_response['total_members_count']).to eq(3) + end + end end describe 'PUT /projects/:id' do @@ -3931,6 +3988,16 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect(json_response['emails_disabled']).to eq(true) end + it 'updates emails_enabled?' do + project_param = { emails_enabled: false } + + put api("/projects/#{project3.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(:ok) + + expect(json_response['emails_enabled']).to eq(false) + end + it 'updates build_git_strategy' do project_param = { build_git_strategy: 'clone' } @@ -4150,6 +4217,7 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and merge_method: 'ff', ci_default_git_depth: 20, ci_forward_deployment_enabled: false, + ci_forward_deployment_rollback_allowed: false, ci_allow_fork_pipelines_to_run_in_parent_project: false, ci_separated_caches: false, description: 'new description' } @@ -4425,6 +4493,29 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect(response).to have_gitlab_http_status(:forbidden) end end + + context 'when a project is moved' do + let_it_be(:redirect_route) { 'new/project/location' } + let_it_be(:path) { "/projects/#{CGI.escape(redirect_route)}/archive" } + + before do + project.route.create_redirect(redirect_route) + end + + it 'returns 405 error' do + post api(path, user) + + expect(response).to have_gitlab_http_status(:method_not_allowed) + end + + context 'when user do not have access to the project' do + it 'returns 404 error' do + post api(path, create(:user)) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end describe 'POST /projects/:id/unarchive' do diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 8853eff0b3e..a94ed63bf47 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -37,6 +37,52 @@ RSpec.describe API::Repositories, feature_category: :source_code_management do end end + context 'when path does not exist' do + let(:path) { 'bogus' } + + context 'when handle_structured_gitaly_errors feature is disabled' do + before do + stub_feature_flags(handle_structured_gitaly_errors: false) + end + + it 'returns an empty array' do + get api("#{route}?path=#{path}", current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an(Array) + expect(json_response).to be_an_empty + end + end + + context 'when handle_structured_gitaly_errors feature is enabled' do + before do + stub_feature_flags(handle_structured_gitaly_errors: true) + end + + it_behaves_like '404 response' do + let(:request) { get api("#{route}?path=#{path}", current_user) } + let(:message) { '404 invalid revision or path Not Found' } + end + end + end + + context 'when path is empty directory ' do + context 'when handle_structured_gitaly_errors feature is disabled' do + before do + stub_feature_flags(handle_structured_gitaly_errors: false) + end + + it 'returns an empty array' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an(Array) + end + end + end + context 'when repository is disabled' do include_context 'disabled repository' diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index dfaba969153..12af1fc1b79 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -59,6 +59,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu expect(json_response['group_runner_token_expiration_interval']).to be_nil expect(json_response['project_runner_token_expiration_interval']).to be_nil expect(json_response['max_export_size']).to eq(0) + expect(json_response['max_decompressed_archive_size']).to eq(25600) expect(json_response['max_terraform_state_size_bytes']).to eq(0) expect(json_response['pipeline_limit_per_project_user_sha']).to eq(0) expect(json_response['delete_inactive_projects']).to be(false) @@ -81,6 +82,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu expect(json_response['ci_max_includes']).to eq(150) expect(json_response['allow_account_deletion']).to eq(true) expect(json_response['gitlab_shell_operation_limit']).to eq(600) + expect(json_response['namespace_aggregation_schedule_lease_duration_in_seconds']).to eq(300) + expect(json_response['default_branch_protection_defaults']).to be_kind_of(Hash) end end @@ -154,6 +157,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu diff_max_files: 2000, diff_max_lines: 50000, default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE, + default_branch_protection_defaults: ::Gitlab::Access::BranchProtection.protected_against_developer_pushes.stringify_keys, local_markdown_version: 3, allow_local_requests_from_web_hooks_and_services: true, allow_local_requests_from_system_hooks: false, @@ -168,6 +172,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu mailgun_events_enabled: true, mailgun_signing_key: 'MAILGUN_SIGNING_KEY', max_export_size: 6, + max_decompressed_archive_size: 20000, max_terraform_state_size_bytes: 1_000, disabled_oauth_sign_in_sources: 'unknown', import_sources: 'github,bitbucket', @@ -186,6 +191,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu jira_connect_application_key: '123', jira_connect_proxy_url: 'http://example.com', bulk_import_enabled: false, + bulk_import_max_download_file_size: 1, allow_runner_registration_token: true, user_defaults_to_private_profile: true, default_syntax_highlighting_theme: 2, @@ -193,7 +199,9 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu silent_mode_enabled: true, valid_runner_registrars: ['group'], allow_account_deletion: false, - gitlab_shell_operation_limit: 500 + gitlab_shell_operation_limit: 500, + namespace_aggregation_schedule_lease_duration_in_seconds: 400, + max_import_remote_file_size: 2 } expect(response).to have_gitlab_http_status(:ok) @@ -230,6 +238,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu expect(json_response['diff_max_files']).to eq(2000) expect(json_response['diff_max_lines']).to eq(50000) expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + expect(json_response['default_branch_protection_defaults']).to eq(::Gitlab::Access::BranchProtection.protected_against_developer_pushes.stringify_keys) expect(json_response['local_markdown_version']).to eq(3) expect(json_response['allow_local_requests_from_web_hooks_and_services']).to eq(true) expect(json_response['allow_local_requests_from_system_hooks']).to eq(false) @@ -244,6 +253,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu expect(json_response['mailgun_events_enabled']).to be(true) expect(json_response['mailgun_signing_key']).to eq('MAILGUN_SIGNING_KEY') expect(json_response['max_export_size']).to eq(6) + expect(json_response['max_decompressed_archive_size']).to eq(20000) expect(json_response['max_terraform_state_size_bytes']).to eq(1_000) expect(json_response['disabled_oauth_sign_in_sources']).to eq([]) expect(json_response['import_sources']).to match_array(%w(github bitbucket)) @@ -270,6 +280,9 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting, featu expect(json_response['valid_runner_registrars']).to eq(['group']) expect(json_response['allow_account_deletion']).to be(false) expect(json_response['gitlab_shell_operation_limit']).to be(500) + expect(json_response['namespace_aggregation_schedule_lease_duration_in_seconds']).to be(400) + expect(json_response['max_import_remote_file_size']).to be(2) + expect(json_response['bulk_import_max_download_file_size']).to be(1) end end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 4ba2a768e01..0b97bb5c443 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -13,7 +13,8 @@ RSpec.describe API::Snippets, :aggregate_failures, factory_default: :keep, featu let_it_be_with_refind(:private_snippet) { create(:personal_snippet, :repository, :private, author: user) } let_it_be(:internal_snippet) { create(:personal_snippet, :repository, :internal, author: user) } - let_it_be(:user_token) { create(:personal_access_token, user: user) } + let_it_be(:user_token) { create(:personal_access_token, user: user) } + let_it_be(:admin_token) { create(:personal_access_token, :admin_mode, user: admin, scopes: [:sudo, :api]) } let_it_be(:other_user_token) { create(:personal_access_token, user: other_user) } let_it_be(:project) do create_default(:project, :public).tap do |p| @@ -21,9 +22,17 @@ RSpec.describe API::Snippets, :aggregate_failures, factory_default: :keep, featu end end - describe 'GET /snippets/' do + shared_examples "returns unauthorized when not authenticated" do + it 'returns 401 for non-authenticated' do + get api(path) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + shared_examples "returns filtered snippets for user" do it 'returns snippets available for user' do - get api("/snippets/", personal_access_token: user_token) + get api(path, personal_access_token: user_token) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -38,8 +47,32 @@ RSpec.describe API::Snippets, :aggregate_failures, factory_default: :keep, featu expect(json_response.last).to have_key('visibility') end + context 'filtering snippets by created_after/created_before' do + let_it_be(:private_snippet_before_time_range) { create(:personal_snippet, :repository, :private, author: user, created_at: Time.parse("2021-08-20T00:00:00Z")) } + let_it_be(:private_snippet_in_time_range1) { create(:personal_snippet, :repository, :private, author: user, created_at: Time.parse("2021-08-22T00:00:00Z")) } + let_it_be(:private_snippet_in_time_range2) { create(:personal_snippet, :repository, :private, author: user, created_at: Time.parse("2021-08-24T00:00:00Z")) } + let_it_be(:private_snippet_after_time_range) { create(:personal_snippet, :repository, :private, author: user, created_at: Time.parse("2021-08-26T00:00:00Z")) } + + let(:path) { "/snippets?created_after=2021-08-21T00:00:00Z&created_before=2021-08-25T00:00:00Z" } + + it 'returns snippets available for user in given time range' do + get api(path, personal_access_token: user_token) + + expect(json_response.map { |snippet| snippet['id'] }).to contain_exactly( + private_snippet_in_time_range1.id, + private_snippet_in_time_range2.id) + end + end + end + + describe 'GET /snippets/' do + let(:path) { "/snippets" } + + it_behaves_like "returns unauthorized when not authenticated" + it_behaves_like "returns filtered snippets for user" + it 'hides private snippets from regular user' do - get api("/snippets/", personal_access_token: other_user_token) + get api(path, personal_access_token: other_user_token) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -47,39 +80,16 @@ RSpec.describe API::Snippets, :aggregate_failures, factory_default: :keep, featu expect(json_response.size).to eq(0) end - it 'returns 401 for non-authenticated' do - get api("/snippets/") - - expect(response).to have_gitlab_http_status(:unauthorized) - end - it 'does not return snippets related to a project with disable feature visibility' do public_snippet = create(:project_snippet, :public, author: user, project: project) project.project_feature.update_attribute(:snippets_access_level, 0) - get api("/snippets/", personal_access_token: user_token) + get api(path, personal_access_token: user_token) json_response.each do |snippet| expect(snippet["id"]).not_to eq(public_snippet.id) end end - - context 'filtering snippets by created_after/created_before' do - let_it_be(:private_snippet_before_time_range) { create(:personal_snippet, :repository, :private, author: user, created_at: Time.parse("2021-08-20T00:00:00Z")) } - let_it_be(:private_snippet_in_time_range1) { create(:personal_snippet, :repository, :private, author: user, created_at: Time.parse("2021-08-22T00:00:00Z")) } - let_it_be(:private_snippet_in_time_range2) { create(:personal_snippet, :repository, :private, author: user, created_at: Time.parse("2021-08-24T00:00:00Z")) } - let_it_be(:private_snippet_after_time_range) { create(:personal_snippet, :repository, :private, author: user, created_at: Time.parse("2021-08-26T00:00:00Z")) } - - let(:path) { "/snippets?created_after=2021-08-21T00:00:00Z&created_before=2021-08-25T00:00:00Z" } - - it 'returns snippets available for user in given time range' do - get api(path, personal_access_token: user_token) - - expect(json_response.map { |snippet| snippet['id'] }).to contain_exactly( - private_snippet_in_time_range1.id, - private_snippet_in_time_range2.id) - end - end end describe 'GET /snippets/public' do @@ -92,6 +102,8 @@ RSpec.describe API::Snippets, :aggregate_failures, factory_default: :keep, featu let(:path) { "/snippets/public" } + it_behaves_like "returns unauthorized when not authenticated" + it 'returns only public snippets from all users when authenticated' do get api(path, personal_access_token: user_token) @@ -110,12 +122,6 @@ RSpec.describe API::Snippets, :aggregate_failures, factory_default: :keep, featu end end - it 'requires authentication' do - get api(path, nil) - - expect(response).to have_gitlab_http_status(:unauthorized) - end - context 'filtering public snippets by created_after/created_before' do let_it_be(:public_snippet_before_time_range) { create(:personal_snippet, :repository, :public, author: other_user, created_at: Time.parse("2021-08-20T00:00:00Z")) } let_it_be(:public_snippet_in_time_range) { create(:personal_snippet, :repository, :public, author: other_user, created_at: Time.parse("2021-08-22T00:00:00Z")) } @@ -132,6 +138,49 @@ RSpec.describe API::Snippets, :aggregate_failures, factory_default: :keep, featu end end + describe 'GET /snippets/all' do + let(:path) { "/snippets/all" } + + it_behaves_like "returns unauthorized when not authenticated" + it_behaves_like "returns filtered snippets for user" + + context 'with additional snippets' do + let!(:hidden_snippet) { create(:personal_snippet, :repository, :private, author: other_user) } + let!(:viewable_snippet) { create(:personal_snippet, :repository, :internal, author: user) } + + context 'and user is admin', :enable_admin_mode do + it 'returns all snippets' do + get api(path, personal_access_token: admin_token) + + ids = json_response.map { |snippet| snippet['id'] } + + expect(ids).to contain_exactly( + viewable_snippet.id, + hidden_snippet.id, + internal_snippet.id, + private_snippet.id, + public_snippet.id + ) + end + end + + context 'and user is not admin' do + it 'returns all internal and public snippets' do + get api(path, personal_access_token: user_token) + + ids = json_response.map { |snippet| snippet['id'] } + + expect(ids).to contain_exactly( + viewable_snippet.id, + internal_snippet.id, + private_snippet.id, + public_snippet.id + ) + end + end + end + end + describe 'GET /snippets/:id/raw' do let(:snippet) { private_snippet } @@ -448,10 +497,8 @@ RSpec.describe API::Snippets, :aggregate_failures, factory_default: :keep, featu end context "when admin" do - let_it_be(:token) { create(:personal_access_token, :admin_mode, user: admin, scopes: [:sudo]) } - subject do - put api("/snippets/#{snippet.id}", personal_access_token: token), params: { visibility: 'private', sudo: user.id } + put api("/snippets/#{snippet.id}", personal_access_token: admin_token), params: { visibility: 'private', sudo: user.id } end context 'when sudo is defined' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 2bbcf6b3f38..81881532240 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -4789,7 +4789,7 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile }.as_json end - before :all do + before_all do group.add_member(user, Gitlab::Access::OWNER) project.add_member(user, Gitlab::Access::OWNER) create(:merge_request, source_project: project, source_branch: "my-personal-branch-1", author: user) diff --git a/spec/requests/groups/work_items_controller_spec.rb b/spec/requests/groups/work_items_controller_spec.rb new file mode 100644 index 00000000000..c47b3f03ec1 --- /dev/null +++ b/spec/requests/groups/work_items_controller_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Group Level Work Items', feature_category: :team_planning do + let_it_be(:group) { create(:group, :private) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:developer) { create(:user).tap { |u| group.add_developer(u) } } + + describe 'GET /groups/:group/-/work_items' do + let(:work_items_path) { url_for(controller: 'groups/work_items', action: :index, group_id: group.full_path) } + + before do + sign_in(current_user) + end + + context 'when the user can read the group' do + let(:current_user) { developer } + + it 'renders index' do + get work_items_path + + expect(response).to have_gitlab_http_status(:ok) + end + + context 'when the namespace_level_work_items feature flag is disabled' do + before do + stub_feature_flags(namespace_level_work_items: false) + end + + it 'returns not found' do + get work_items_path + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when the user cannot read the group' do + let(:current_user) { create(:user) } + + it 'returns not found' do + get work_items_path + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/import/github_controller_spec.rb b/spec/requests/import/github_controller_spec.rb index 0f28f5e93f3..2499ef859dd 100644 --- a/spec/requests/import/github_controller_spec.rb +++ b/spec/requests/import/github_controller_spec.rb @@ -12,31 +12,13 @@ RSpec.describe Import::GithubController, feature_category: :importers do stub_application_setting(import_sources: ['github']) login_as(user) - end - - context 'with feature enabled' do - before do - stub_feature_flags(import_details_page: true) - - request - end - it 'responds with a 200 and shows the template', :aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:details) - end + request end - context 'with feature disabled' do - before do - stub_feature_flags(import_details_page: false) - - request - end - - it 'responds with a 404' do - expect(response).to have_gitlab_http_status(:not_found) - end + it 'responds with a 200 and shows the template', :aggregate_failures do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:details) end end end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 199138eb3a9..b07296a0df2 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -807,7 +807,7 @@ RSpec.describe 'Git LFS API and storage', feature_category: :source_code_managem end end - describe 'to one project', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/418757' do + describe 'to one project' do describe 'when user is authenticated' do describe 'when user has push access to the project' do before do diff --git a/spec/requests/organizations/organizations_controller_spec.rb b/spec/requests/organizations/organizations_controller_spec.rb index bd54b50de99..788d740504a 100644 --- a/spec/requests/organizations/organizations_controller_spec.rb +++ b/spec/requests/organizations/organizations_controller_spec.rb @@ -5,38 +5,60 @@ require 'spec_helper' RSpec.describe Organizations::OrganizationsController, feature_category: :cell do let_it_be(:organization) { create(:organization) } - RSpec.shared_examples 'basic organization controller action' do + shared_examples 'successful response' do + it 'renders 200 OK' do + gitlab_request + + expect(response).to have_gitlab_http_status(:ok) + end + end + + shared_examples 'action disabled by `ui_for_organizations` feature flag' do before do - sign_in(user) + stub_feature_flags(ui_for_organizations: false) end - context 'when the user does not have authorization' do - let_it_be(:user) { create(:user) } + it 'renders 404' do + gitlab_request - it 'renders 404' do - gitlab_request + expect(response).to have_gitlab_http_status(:not_found) + end + end - expect(response).to have_gitlab_http_status(:not_found) - end + shared_examples 'basic organization controller action' do + context 'when the user is not logged in' do + it_behaves_like 'successful response' + it_behaves_like 'action disabled by `ui_for_organizations` feature flag' end - context 'when the user has authorization', :enable_admin_mode do - let_it_be(:user) { create(:admin) } + context 'when the user is logged in' do + before do + sign_in(user) + end - it 'renders 200 OK' do - gitlab_request + context 'with no association to an organization' do + let_it_be(:user) { create(:user) } - expect(response).to have_gitlab_http_status(:ok) + it_behaves_like 'successful response' + it_behaves_like 'action disabled by `ui_for_organizations` feature flag' end - context 'when the feature flag `ui_for_organizations` is disabled' do - it 'renders 404' do - stub_feature_flags(ui_for_organizations: false) + context 'as as admin', :enable_admin_mode do + let_it_be(:user) { create(:admin) } - gitlab_request + it_behaves_like 'successful response' + it_behaves_like 'action disabled by `ui_for_organizations` feature flag' + end - expect(response).to have_gitlab_http_status(:not_found) + context 'as an organization user' do + let_it_be(:user) { create :user } + + before do + create :organization_user, organization: organization, user: user end + + it_behaves_like 'successful response' + it_behaves_like 'action disabled by `ui_for_organizations` feature flag' end end end diff --git a/spec/requests/projects/blob_spec.rb b/spec/requests/projects/blob_spec.rb deleted file mode 100644 index 7d62619e76a..00000000000 --- a/spec/requests/projects/blob_spec.rb +++ /dev/null @@ -1,87 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Blobs', feature_category: :source_code_management do - let_it_be(:project) { create(:project, :public, :repository, lfs: true) } - - describe 'GET /:namespace_id/:project_id/-/blob/:id' do - subject(:request) do - get namespace_project_blob_path(namespace_id: project.namespace, project_id: project, id: id) - end - - context 'with LFS file' do - let(:id) { 'master/files/lfs/lfs_object.iso' } - let(:object_store_host) { 'http://127.0.0.1:9000' } - let(:connect_src) do - csp = response.headers['Content-Security-Policy'] - csp.split('; ').find { |src| src.starts_with?('connect-src') } - end - - let(:gitlab_config) do - Gitlab.config.gitlab.deep_merge( - 'content_security_policy' => { - 'enabled' => content_security_policy_enabled - } - ) - end - - let(:lfs_config) do - Gitlab.config.lfs.deep_merge( - 'enabled' => lfs_enabled, - 'object_store' => { - 'remote_directory' => 'lfs-objects', - 'enabled' => true, - 'proxy_download' => proxy_download, - 'connection' => { - 'endpoint' => object_store_host, - 'path_style' => true - } - } - ) - end - - before do - stub_config_setting(gitlab_config) - stub_lfs_setting(lfs_config) - stub_lfs_object_storage(proxy_download: proxy_download) - - request - end - - describe 'directly downloading lfs file' do - let(:lfs_enabled) { true } - let(:proxy_download) { false } - let(:content_security_policy_enabled) { true } - - it { expect(response).to have_gitlab_http_status(:success) } - - it { expect(connect_src).to include(object_store_host) } - - context 'when lfs is disabled' do - let(:lfs_enabled) { false } - - it { expect(response).to have_gitlab_http_status(:success) } - - it { expect(connect_src).not_to include(object_store_host) } - end - - context 'when content_security_policy is disabled' do - let(:content_security_policy_enabled) { false } - - it { expect(response).to have_gitlab_http_status(:success) } - - it { expect(connect_src).not_to include(object_store_host) } - end - - context 'when proxy download is enabled' do - let(:proxy_download) { true } - - it { expect(response).to have_gitlab_http_status(:success) } - - it { expect(connect_src).not_to include(object_store_host) } - end - end - end - end -end diff --git a/spec/requests/projects/merge_requests/creations_spec.rb b/spec/requests/projects/merge_requests/creations_spec.rb index e8a073fef5f..8f55aa90bee 100644 --- a/spec/requests/projects/merge_requests/creations_spec.rb +++ b/spec/requests/projects/merge_requests/creations_spec.rb @@ -10,6 +10,17 @@ RSpec.describe 'merge requests creations', feature_category: :code_review_workfl let_it_be(:project) { create(:project, :repository, group: group) } let_it_be(:user) { create(:user) } + let(:get_params) do + { + namespace_id: project.namespace.to_param, + project_id: project, + merge_request: { + source_branch: 'two-commits', + target_branch: 'master' + } + } + end + before_all do group.add_developer(user) end @@ -18,16 +29,52 @@ RSpec.describe 'merge requests creations', feature_category: :code_review_workfl login_as(user) end - def get_new - get namespace_project_new_merge_request_path(namespace_id: project.namespace, project_id: project) + def get_new(params = get_params) + get namespace_project_new_merge_request_path(params) end - it 'avoids N+1 DB queries even with forked projects' do - control = ActiveRecord::QueryRecorder.new(skip_cached: false) { get_new } + describe 'GET new' do + context 'without merge_request params' do + it 'avoids N+1 DB queries even with forked projects' do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { get_new } + + 5.times { fork_project(project, user) } + + expect { get_new }.not_to exceed_query_limit(control) + end + + it 'renders branch selection screen' do + get_new(get_params.except(:merge_request)) + + expect(response).to be_successful + expect(response).to render_template(partial: '_new_compare') + end + end + + context 'with merge_request params' do + it 'renders new merge request widget template' do + get_new + + expect(response).to be_successful + expect(response).to render_template(partial: '_new_submit') + expect(response).not_to render_template(partial: '_new_compare') + end - 5.times { fork_project(project, user) } + context 'when existing merge request with same target and source branches' do + let_it_be(:existing_mr) { create(:merge_request) } - expect { get_new }.not_to exceed_query_limit(control) + it 'renders branch selection screen' do + allow_next_instance_of(MergeRequest) do |instance| + allow(instance).to receive(:existing_mrs_targeting_same_branch).and_return([existing_mr]) + end + + get_new + + expect(response).to be_successful + expect(response).to render_template(partial: '_new_compare') + end + end + end end it_behaves_like "observability csp policy", Projects::MergeRequests::CreationsController do diff --git a/spec/requests/projects/merge_requests_controller_spec.rb b/spec/requests/projects/merge_requests_controller_spec.rb index 955b6e53686..e6a281d8d59 100644 --- a/spec/requests/projects/merge_requests_controller_spec.rb +++ b/spec/requests/projects/merge_requests_controller_spec.rb @@ -132,4 +132,44 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code end end end + + describe 'GET #pipelines.json' do + before do + login_as(user) + end + + it 'avoids N+1 queries', :use_sql_query_cache do + create_pipeline + + # warm up + get pipelines_project_merge_request_path(project, merge_request, format: :json) + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get pipelines_project_merge_request_path(project, merge_request, format: :json) + end + + expect(response).to have_gitlab_http_status(:ok) + expect(Gitlab::Json.parse(response.body)['count']['all']).to eq(1) + + create_pipeline + + expect do + get pipelines_project_merge_request_path(project, merge_request, format: :json) + end.to issue_same_number_of_queries_as(control) + + expect(response).to have_gitlab_http_status(:ok) + expect(Gitlab::Json.parse(response.body)['count']['all']).to eq(2) + end + + private + + def create_pipeline + create( + :ci_pipeline, :with_job, :success, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha + ) + end + end end diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb index 644f26af006..24b6fb2f640 100644 --- a/spec/requests/projects/merge_requests_discussions_spec.rb +++ b/spec/requests/projects/merge_requests_discussions_spec.rb @@ -27,19 +27,15 @@ RSpec.describe 'merge requests discussions', feature_category: :source_code_mana end # rubocop:enable RSpec/InstanceVariable - shared_examples 'N+1 queries' do - it 'avoids N+1 DB queries', :request_store do - send_request # warm up + it 'avoids N+1 DB queries', :request_store do + send_request # warm up - create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) - control = ActiveRecord::QueryRecorder.new { send_request } + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) + control = ActiveRecord::QueryRecorder.new { send_request } - create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) - expect do - send_request - end.not_to exceed_query_limit(control).with_threshold(notes_metadata_threshold) - end + expect { send_request }.not_to exceed_query_limit(control) end it 'returns 200' do @@ -48,13 +44,6 @@ RSpec.describe 'merge requests discussions', feature_category: :source_code_mana expect(response).to have_gitlab_http_status(:ok) end - # https://docs.gitlab.com/ee/development/query_recorder.html#use-request-specs-instead-of-controller-specs - context 'with notes_metadata_threshold' do - let(:notes_metadata_threshold) { 1 } - - it_behaves_like 'N+1 queries' - end - it 'limits Gitaly queries', :request_store do Gitlab::GitalyClient.allow_n_plus_1_calls do create_list(:diff_note_on_merge_request, 7, noteable: merge_request, project: merge_request.project) diff --git a/spec/requests/projects/metrics/dashboards/builder_spec.rb b/spec/requests/projects/metrics/dashboards/builder_spec.rb deleted file mode 100644 index 8af2d1f1d25..00000000000 --- a/spec/requests/projects/metrics/dashboards/builder_spec.rb +++ /dev/null @@ -1,123 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Projects::Metrics::Dashboards::BuilderController', feature_category: :metrics do - let_it_be(:project) { create(:project) } - let_it_be(:environment) { create(:environment, project: project) } - let_it_be(:user) { create(:user) } - let_it_be(:valid_panel_yml) do - <<~YML - --- - title: "Super Chart A1" - type: "area-chart" - y_label: "y_label" - weight: 1 - max_value: 1 - metrics: - - id: metric_a1 - query_range: |+ - avg( - sum( - container_memory_usage_bytes{ - container_name!="POD", - pod_name=~"^{{ci_environment_slug}}-(.*)", - namespace="{{kube_namespace}}", - user_def_variable="{{user_def_variable}}" - } - ) by (job) - ) without (job) - /1024/1024/1024 - unit: unit - label: Legend Label - YML - end - - let_it_be(:invalid_panel_yml) do - <<~YML - --- - title: "Super Chart A1" - type: "area-chart" - y_label: "y_label" - weight: 1 - max_value: 1 - YML - end - - def send_request(params = {}) - post namespace_project_metrics_dashboards_builder_path(namespace_id: project.namespace, project_id: project, format: :json, **params) - end - - describe 'POST /:namespace/:project/-/metrics/dashboards/builder' do - before do - stub_feature_flags(remove_monitor_metrics: false) - end - - context 'as anonymous user' do - it 'redirects user to sign in page' do - send_request - - expect(response).to redirect_to(new_user_session_path) - end - end - - context 'as user with guest access' do - before do - project.add_guest(user) - login_as(user) - end - - it 'returns not found' do - send_request - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'as logged in user' do - before do - project.add_developer(user) - login_as(user) - end - - context 'valid yaml panel is supplied' do - it 'returns success' do - send_request(panel_yaml: valid_panel_yml) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include('title' => 'Super Chart A1', 'type' => 'area-chart') - end - end - - context 'invalid yaml panel is supplied' do - it 'returns unprocessable entity' do - send_request(panel_yaml: invalid_panel_yml) - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Each "panel" must define an array :metrics') - end - end - - context 'invalid panel_yaml is not a yaml string' do - it 'returns unprocessable entity' do - send_request(panel_yaml: 1) - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Invalid configuration format') - end - end - - context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - - it 'returns not found' do - send_request - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - end -end diff --git a/spec/requests/projects/ml/models_controller_spec.rb b/spec/requests/projects/ml/models_controller_spec.rb index d03748c8dff..8569f2396d3 100644 --- a/spec/requests/projects/ml/models_controller_spec.rb +++ b/spec/requests/projects/ml/models_controller_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' RSpec.describe Projects::Ml::ModelsController, feature_category: :mlops do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } - let_it_be(:model1_a) { create(:ml_model_package, project: project) } - let_it_be(:model1_b) { create(:ml_model_package, project: project, name: model1_a.name) } - let_it_be(:model2) { create(:ml_model_package, project: project) } + let_it_be(:model1) { create(:ml_models, :with_versions, project: project) } + let_it_be(:model2) { create(:ml_models, project: project) } + let_it_be(:model_in_different_project) { create(:ml_models) } let(:model_registry_enabled) { true } @@ -36,16 +36,17 @@ RSpec.describe Projects::Ml::ModelsController, feature_category: :mlops do index_request end - it 'prepares model view using the presenter' do - expect(::Ml::ModelsIndexPresenter).to receive(:new).and_call_original - + it 'fetches the correct models' do index_request + + expect(assigns(:models)).to match_array([model1, model2]) end it 'does not perform N+1 sql queries' do control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { list_models } - create_list(:ml_model_package, 4, project: project) + create_list(:ml_model_versions, 2, model: model1) + create_list(:ml_model_versions, 2, model: model2) expect { list_models }.not_to exceed_all_query_limit(control_count) end diff --git a/spec/requests/projects/noteable_notes_spec.rb b/spec/requests/projects/noteable_notes_spec.rb index 55540447da0..a490e059680 100644 --- a/spec/requests/projects/noteable_notes_spec.rb +++ b/spec/requests/projects/noteable_notes_spec.rb @@ -14,6 +14,8 @@ RSpec.describe 'Project noteable notes', feature_category: :team_planning do let(:response_etag) { response.headers['ETag'] } let(:stored_etag) { "W/\"#{etag_store.get(notes_path)}\"" } + let(:default_headers) { { 'X-Last-Fetched-At' => 0 } } + before do login_as(user) end @@ -21,7 +23,7 @@ RSpec.describe 'Project noteable notes', feature_category: :team_planning do it 'does not set a Gitlab::EtagCaching ETag if there is a note' do create(:note_on_merge_request, noteable: merge_request, project: merge_request.project) - get notes_path + get notes_path, headers: default_headers expect(response).to have_gitlab_http_status(:ok) @@ -31,7 +33,7 @@ RSpec.describe 'Project noteable notes', feature_category: :team_planning do end it 'sets a Gitlab::EtagCaching ETag if there is no note' do - get notes_path + get notes_path, headers: default_headers expect(response).to have_gitlab_http_status(:ok) expect(response_etag).to eq(stored_etag) @@ -68,7 +70,7 @@ RSpec.describe 'Project noteable notes', feature_category: :team_planning do ) ) - get notes_path, headers: { "if-none-match": stored_etag } + get notes_path, headers: default_headers.merge("if-none-match": stored_etag) expect(response).to have_gitlab_http_status(:not_modified) end diff --git a/spec/requests/projects/notes_controller_spec.rb b/spec/requests/projects/notes_controller_spec.rb new file mode 100644 index 00000000000..9cd8ba364ea --- /dev/null +++ b/spec/requests/projects/notes_controller_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::NotesController, feature_category: :team_planning do + let_it_be(:project) { create(:project, :public) } + let_it_be(:issue) { create(:issue, project: project) } + + describe '#index' do + def get_notes + get project_noteable_notes_path(project, target_type: 'issue', target_id: issue.id, format: :json), + headers: { 'X-Last-Fetched-At': 0 } + end + + it 'does not execute N+1 queries' do + get_notes + + create(:note_on_issue, project: project, noteable: issue) + + control = ActiveRecord::QueryRecorder.new { get_notes } + + create(:note_on_issue, project: project, noteable: issue) + + expect { get_notes }.not_to exceed_query_limit(control) + end + end +end diff --git a/spec/requests/projects/service_desk_controller_spec.rb b/spec/requests/projects/service_desk_controller_spec.rb index 54fe176e244..05e48c2c5c7 100644 --- a/spec/requests/projects/service_desk_controller_spec.rb +++ b/spec/requests/projects/service_desk_controller_spec.rb @@ -71,7 +71,7 @@ RSpec.describe Projects::ServiceDeskController, feature_category: :service_desk it 'toggles services desk incoming email' do project.update!(service_desk_enabled: false) - put project_service_desk_refresh_path(project, format: :json), params: { service_desk_enabled: true } + put project_service_desk_path(project, format: :json), params: { service_desk_enabled: true } expect(json_response["service_desk_address"]).to be_present expect(json_response["service_desk_enabled"]).to be_truthy @@ -79,7 +79,7 @@ RSpec.describe Projects::ServiceDeskController, feature_category: :service_desk end it 'sets issue_template_key' do - put project_service_desk_refresh_path(project, format: :json), params: { issue_template_key: 'service_desk' } + put project_service_desk_path(project, format: :json), params: { issue_template_key: 'service_desk' } settings = project.service_desk_setting expect(settings).to be_present @@ -89,7 +89,7 @@ RSpec.describe Projects::ServiceDeskController, feature_category: :service_desk end it 'returns an error when update of service desk settings fails' do - put project_service_desk_refresh_path(project, format: :json), params: { issue_template_key: 'invalid key' } + put project_service_desk_path(project, format: :json), params: { issue_template_key: 'invalid key' } expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(json_response['message']).to eq('Issue template key is empty or does not exist') @@ -100,7 +100,7 @@ RSpec.describe Projects::ServiceDeskController, feature_category: :service_desk it 'renders 404' do sign_in(other_user) - put project_service_desk_refresh_path(project, format: :json), params: { service_desk_enabled: true } + put project_service_desk_path(project, format: :json), params: { service_desk_enabled: true } expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/projects/tracing_controller_spec.rb b/spec/requests/projects/tracing_controller_spec.rb index eecaa0d962a..8996ea7f8d6 100644 --- a/spec/requests/projects/tracing_controller_spec.rb +++ b/spec/requests/projects/tracing_controller_spec.rb @@ -14,14 +14,12 @@ RSpec.describe Projects::TracingController, feature_category: :tracing do response end - describe 'GET #index' do - before do - stub_feature_flags(observability_tracing: observability_tracing_ff) - sign_in(user) - end - - let(:path) { project_tracing_index_path(project) } + before do + stub_feature_flags(observability_tracing: observability_tracing_ff) + sign_in(user) + end + shared_examples 'tracing route request' do it_behaves_like 'observability csp policy' do before_all do project.add_developer(user) @@ -45,6 +43,26 @@ RSpec.describe Projects::TracingController, feature_category: :tracing do expect(subject).to have_gitlab_http_status(:ok) end + context 'when feature is disabled' do + let(:observability_tracing_ff) { false } + + it 'returns 404' do + expect(subject).to have_gitlab_http_status(:not_found) + end + end + end + end + + describe 'GET #index' do + let(:path) { project_tracing_index_path(project) } + + it_behaves_like 'tracing route request' + + describe 'html response' do + before_all do + project.add_developer(user) + end + it 'renders the js-tracing element correctly' do element = Nokogiri::HTML.parse(subject.body).at_css('#js-tracing') @@ -55,13 +73,31 @@ RSpec.describe Projects::TracingController, feature_category: :tracing do }.to_json expect(element.attributes['data-view-model'].value).to eq(expected_view_model) end + end + end - context 'when feature is disabled' do - let(:observability_tracing_ff) { false } + describe 'GET #show' do + let(:path) { project_tracing_path(project, id: "test-trace-id") } - it 'returns 404' do - expect(subject).to have_gitlab_http_status(:not_found) - end + it_behaves_like 'tracing route request' + + describe 'html response' do + before_all do + project.add_developer(user) + end + + it 'renders the js-tracing element correctly' do + element = Nokogiri::HTML.parse(subject.body).at_css('#js-tracing-details') + + expected_view_model = { + tracingIndexUrl: project_tracing_index_path(project), + traceId: 'test-trace-id', + tracingUrl: Gitlab::Observability.tracing_url(project), + provisioningUrl: Gitlab::Observability.provisioning_url(project), + oauthUrl: Gitlab::Observability.oauth_url + }.to_json + + expect(element.attributes['data-view-model'].value).to eq(expected_view_model) end end end diff --git a/spec/requests/sessions_spec.rb b/spec/requests/sessions_spec.rb index 3bff9555834..8e069427678 100644 --- a/spec/requests/sessions_spec.rb +++ b/spec/requests/sessions_spec.rb @@ -38,6 +38,35 @@ RSpec.describe 'Sessions', feature_category: :system_access do end end + context 'when using two-factor authentication via OTP' do + let(:user) { create(:user, :two_factor, :invalid) } + let(:user_params) { { login: user.username, password: user.password } } + + def authenticate_2fa(otp_attempt:) + post(user_session_path(params: { user: user_params })) # First sign-in request for password, second for OTP + post(user_session_path(params: { user: user_params.merge(otp_attempt: otp_attempt) })) + end + + context 'with an invalid user' do + it 'raises StandardError when ActiveRecord::RecordInvalid is raised to return 500 instead of 422' do + otp = user.current_otp + + expect { authenticate_2fa(otp_attempt: otp) }.to raise_error(StandardError) + end + end + + context 'with an invalid record other than user' do + it 'raises ActiveRecord::RecordInvalid for invalid record to return 422f' do + otp = user.current_otp + allow_next_instance_of(ActiveRecord::RecordInvalid) do |instance| + allow(instance).to receive(:record).and_return(nil) # Simulate it's not a user + end + + expect { authenticate_2fa(otp_attempt: otp) }.to raise_error(ActiveRecord::RecordInvalid) + end + end + end + context 'when user signs out' do before do post user_session_path(user: { login: user.username, password: user.password }) diff --git a/spec/requests/verifies_with_email_spec.rb b/spec/requests/verifies_with_email_spec.rb index f3f8e4a1a83..cc85ebc7ade 100644 --- a/spec/requests/verifies_with_email_spec.rb +++ b/spec/requests/verifies_with_email_spec.rb @@ -21,6 +21,16 @@ RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_ expect(mail.to).to match_array([user.email]) expect(mail.subject).to eq(s_('IdentityVerification|Verify your identity')) end + + context 'when an unconfirmed verification email exists' do + let(:new_email) { 'new@email' } + let(:user) { create(:user, unconfirmed_email: new_email, confirmation_sent_at: 1.minute.ago) } + + it 'sends a verification instructions email to the unconfirmed email address' do + mail = ActionMailer::Base.deliveries.find { |d| d.to.include?(new_email) } + expect(mail.subject).to eq(s_('IdentityVerification|Verify your identity')) + end + end end shared_examples_for 'prompt for email verification' do @@ -147,12 +157,10 @@ RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_ post(user_session_path(user: { verification_token: 'token' })) end - it_behaves_like 'prompt for email verification' - it 'adds a verification error message' do - expect(response.body) - .to include("You've reached the maximum amount of tries. "\ - 'Wait 10 minutes or send a new code and try again.') + expect(json_response) + .to include('message' => "You've reached the maximum amount of tries. "\ + 'Wait 10 minutes or send a new code and try again.') end end @@ -161,11 +169,10 @@ RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_ post(user_session_path(user: { verification_token: 'invalid_token' })) end - it_behaves_like 'prompt for email verification' - it 'adds a verification error message' do - expect(response.body) - .to include((s_('IdentityVerification|The code is incorrect. Enter it again, or send a new code.'))) + expect(json_response) + .to include('message' => (s_('IdentityVerification|The code is incorrect. '\ + 'Enter it again, or send a new code.'))) end end @@ -175,27 +182,56 @@ RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_ post(user_session_path(user: { verification_token: 'token' })) end - it_behaves_like 'prompt for email verification' - it 'adds a verification error message' do - expect(response.body) - .to include((s_('IdentityVerification|The code has expired. Send a new code and try again.'))) + expect(json_response) + .to include('message' => (s_('IdentityVerification|The code has expired. Send a new code and try again.'))) end end context 'when a valid verification_token param exists' do - before do - post(user_session_path(user: { verification_token: 'token' })) + subject(:submit_token) { post(user_session_path(user: { verification_token: 'token' })) } + + it 'unlocks the user, create logs and records the activity', :freeze_time do + expect { submit_token }.to change { user.reload.unlock_token }.to(nil) + .and change { user.locked_at }.to(nil) + .and change { AuditEvent.count }.by(1) + .and change { AuthenticationEvent.count }.by(1) + .and change { user.last_activity_on }.to(Date.today) + .and change { user.email_reset_offered_at }.to(Time.current) + end + + it 'returns the success status and a redirect path' do + submit_token + expect(json_response).to eq('status' => 'success', 'redirect_path' => users_successful_verification_path) end - it 'unlocks the user' do - user.reload - expect(user.unlock_token).to be_nil - expect(user.locked_at).to be_nil + context 'when an unconfirmed verification email exists' do + before do + user.update!(email: new_email) + end + + let(:new_email) { 'new@email' } + + it 'confirms the email' do + expect { submit_token } + .to change { user.reload.email }.to(new_email) + .and change { user.confirmed_at } + .and change { user.unconfirmed_email }.from(new_email).to(nil) + end end - it 'redirects to the successful verification path' do - expect(response).to redirect_to(users_successful_verification_path) + context 'when email reset has already been offered' do + before do + user.update!(email_reset_offered_at: 1.hour.ago, email: 'new@email') + end + + it 'does not change the email_reset_offered_at field' do + expect { submit_token }.not_to change { user.reload.email_reset_offered_at } + end + + it 'does not confirm the email' do + expect { submit_token }.not_to change { user.reload.email } + end end end @@ -206,8 +242,8 @@ RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_ post user_session_path, params: { user: { login: another_user.username, password: another_user.password } } end - it 'does not redirect to the successful verification path' do - expect(response).not_to redirect_to(users_successful_verification_path) + it 'redirects to the root path' do + expect(response).to redirect_to(root_path) end end end @@ -277,7 +313,6 @@ RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_ end it_behaves_like 'send verification instructions' - it_behaves_like 'prompt for email verification' end context 'when exceeding the rate limit' do @@ -301,8 +336,79 @@ RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_ mail = find_email_for(user) expect(mail).to be_nil end + end + end - it_behaves_like 'prompt for email verification' + describe 'update_email' do + let(:new_email) { 'new@email' } + + subject(:do_request) { patch(users_update_email_path(user: { email: new_email })) } + + context 'when no verification_user_id session variable exists' do + it 'returns 204 No Content' do + do_request + + expect(response).to have_gitlab_http_status(:no_content) + expect(response.body).to be_empty + end + end + + context 'when a verification_user_id session variable exists' do + before do + stub_session(verification_user_id: user.id) + end + + it 'locks the user' do + do_request + + expect(user.reload.unlock_token).not_to be_nil + expect(user.locked_at).not_to be_nil + end + + it 'sends a changed notification to the primary email and verification instructions to the unconfirmed email' do + perform_enqueued_jobs { do_request } + + sent_mails = ActionMailer::Base.deliveries.map { |mail| { mail.to[0] => mail.subject } } + + expect(sent_mails).to match_array([ + { user.reload.unconfirmed_email => s_('IdentityVerification|Verify your identity') }, + { user.email => 'Email Changed' } + ]) + end + + it 'calls the UpdateEmailService and returns a success response' do + expect_next_instance_of(Users::EmailVerification::UpdateEmailService, user: user) do |instance| + expect(instance).to receive(:execute).with(email: new_email).and_call_original + end + + do_request + + expect(json_response).to eq('status' => 'success') + end + end + + context 'when failing to update the email address' do + let(:service_response) do + { + status: 'failure', + reason: 'the reason', + message: 'the message' + } + end + + before do + stub_session(verification_user_id: user.id) + end + + it 'calls the UpdateEmailService and returns an error response' do + expect_next_instance_of(Users::EmailVerification::UpdateEmailService, user: user) do |instance| + expect(instance).to receive(:execute).with(email: new_email).and_return(service_response) + end + + do_request + + expect(json_response).to eq(service_response.with_indifferent_access) + end end end |