diff options
Diffstat (limited to 'spec/requests/api')
99 files changed, 2261 insertions, 621 deletions
diff --git a/spec/requests/api/admin/plan_limits_spec.rb b/spec/requests/api/admin/plan_limits_spec.rb index 03642ad617e..74ea3b0973f 100644 --- a/spec/requests/api/admin/plan_limits_spec.rb +++ b/spec/requests/api/admin/plan_limits_spec.rb @@ -23,6 +23,14 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Hash + expect(json_response['ci_pipeline_size']).to eq(Plan.default.actual_limits.ci_pipeline_size) + expect(json_response['ci_active_jobs']).to eq(Plan.default.actual_limits.ci_active_jobs) + expect(json_response['ci_active_pipelines']).to eq(Plan.default.actual_limits.ci_active_pipelines) + expect(json_response['ci_project_subscriptions']).to eq(Plan.default.actual_limits.ci_project_subscriptions) + expect(json_response['ci_pipeline_schedules']).to eq(Plan.default.actual_limits.ci_pipeline_schedules) + expect(json_response['ci_needs_size_limit']).to eq(Plan.default.actual_limits.ci_needs_size_limit) + expect(json_response['ci_registered_group_runners']).to eq(Plan.default.actual_limits.ci_registered_group_runners) + expect(json_response['ci_registered_project_runners']).to eq(Plan.default.actual_limits.ci_registered_project_runners) expect(json_response['conan_max_file_size']).to eq(Plan.default.actual_limits.conan_max_file_size) expect(json_response['generic_packages_max_file_size']).to eq(Plan.default.actual_limits.generic_packages_max_file_size) expect(json_response['helm_max_file_size']).to eq(Plan.default.actual_limits.helm_max_file_size) @@ -31,6 +39,7 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do expect(json_response['nuget_max_file_size']).to eq(Plan.default.actual_limits.nuget_max_file_size) expect(json_response['pypi_max_file_size']).to eq(Plan.default.actual_limits.pypi_max_file_size) expect(json_response['terraform_module_max_file_size']).to eq(Plan.default.actual_limits.terraform_module_max_file_size) + expect(json_response['storage_size_limit']).to eq(Plan.default.actual_limits.storage_size_limit) end end @@ -44,6 +53,14 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Hash + expect(json_response['ci_pipeline_size']).to eq(Plan.default.actual_limits.ci_pipeline_size) + expect(json_response['ci_active_jobs']).to eq(Plan.default.actual_limits.ci_active_jobs) + expect(json_response['ci_active_pipelines']).to eq(Plan.default.actual_limits.ci_active_pipelines) + expect(json_response['ci_project_subscriptions']).to eq(Plan.default.actual_limits.ci_project_subscriptions) + expect(json_response['ci_pipeline_schedules']).to eq(Plan.default.actual_limits.ci_pipeline_schedules) + expect(json_response['ci_needs_size_limit']).to eq(Plan.default.actual_limits.ci_needs_size_limit) + expect(json_response['ci_registered_group_runners']).to eq(Plan.default.actual_limits.ci_registered_group_runners) + expect(json_response['ci_registered_project_runners']).to eq(Plan.default.actual_limits.ci_registered_project_runners) expect(json_response['conan_max_file_size']).to eq(Plan.default.actual_limits.conan_max_file_size) expect(json_response['generic_packages_max_file_size']).to eq(Plan.default.actual_limits.generic_packages_max_file_size) expect(json_response['helm_max_file_size']).to eq(Plan.default.actual_limits.helm_max_file_size) @@ -52,6 +69,7 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do expect(json_response['nuget_max_file_size']).to eq(Plan.default.actual_limits.nuget_max_file_size) expect(json_response['pypi_max_file_size']).to eq(Plan.default.actual_limits.pypi_max_file_size) expect(json_response['terraform_module_max_file_size']).to eq(Plan.default.actual_limits.terraform_module_max_file_size) + expect(json_response['storage_size_limit']).to eq(Plan.default.actual_limits.storage_size_limit) end end @@ -84,6 +102,14 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do it 'updates multiple plan limits' do put api('/application/plan_limits', admin), params: { 'plan_name': 'default', + 'ci_pipeline_size': 101, + 'ci_active_jobs': 102, + 'ci_active_pipelines': 103, + 'ci_project_subscriptions': 104, + 'ci_pipeline_schedules': 105, + 'ci_needs_size_limit': 106, + 'ci_registered_group_runners': 107, + 'ci_registered_project_runners': 108, 'conan_max_file_size': 10, 'generic_packages_max_file_size': 20, 'helm_max_file_size': 25, @@ -91,11 +117,20 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do 'npm_max_file_size': 40, 'nuget_max_file_size': 50, 'pypi_max_file_size': 60, - 'terraform_module_max_file_size': 70 + 'terraform_module_max_file_size': 70, + 'storage_size_limit': 80 } expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Hash + expect(json_response['ci_pipeline_size']).to eq(101) + expect(json_response['ci_active_jobs']).to eq(102) + expect(json_response['ci_active_pipelines']).to eq(103) + expect(json_response['ci_project_subscriptions']).to eq(104) + expect(json_response['ci_pipeline_schedules']).to eq(105) + expect(json_response['ci_needs_size_limit']).to eq(106) + 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['generic_packages_max_file_size']).to eq(20) expect(json_response['helm_max_file_size']).to eq(25) @@ -104,6 +139,7 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do expect(json_response['nuget_max_file_size']).to eq(50) expect(json_response['pypi_max_file_size']).to eq(60) expect(json_response['terraform_module_max_file_size']).to eq(70) + expect(json_response['storage_size_limit']).to eq(80) end it 'updates single plan limits' do @@ -131,6 +167,14 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do it 'fails to update plan limits' do put api('/application/plan_limits', admin), params: { 'plan_name': 'default', + 'ci_pipeline_size': 'z', + 'ci_active_jobs': 'y', + 'ci_active_pipelines': 'x', + 'ci_project_subscriptions': 'w', + 'ci_pipeline_schedules': 'v', + 'ci_needs_size_limit': 'u', + 'ci_registered_group_runners': 't', + 'ci_registered_project_runners': 's', 'conan_max_file_size': 'a', 'generic_packages_max_file_size': 'b', 'helm_max_file_size': 'h', @@ -138,11 +182,20 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do 'npm_max_file_size': 'd', 'nuget_max_file_size': 'e', 'pypi_max_file_size': 'f', - 'terraform_module_max_file_size': 'g' + 'terraform_module_max_file_size': 'g', + 'storage_size_limit': 'j' } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']).to include( + 'ci_pipeline_size is invalid', + 'ci_active_jobs is invalid', + 'ci_active_pipelines is invalid', + 'ci_project_subscriptions is invalid', + 'ci_pipeline_schedules is invalid', + 'ci_needs_size_limit is invalid', + 'ci_registered_group_runners is invalid', + 'ci_registered_project_runners is invalid', 'conan_max_file_size is invalid', 'generic_packages_max_file_size is invalid', 'helm_max_file_size is invalid', @@ -150,7 +203,8 @@ RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do 'npm_max_file_size is invalid', 'nuget_max_file_size is invalid', 'pypi_max_file_size is invalid', - 'terraform_module_max_file_size is invalid' + 'terraform_module_max_file_size is invalid', + 'storage_size_limit is invalid' ) end end diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index 68b44bb89e0..1dd1ca4e115 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -263,6 +263,9 @@ RSpec.describe API::Ci::JobArtifacts do 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } end + let(:expected_params) { { artifact_size: job.artifacts_file.size } } + let(:subject_proc) { proc { subject } } + it 'returns specific job artifacts' do subject @@ -270,6 +273,9 @@ RSpec.describe API::Ci::JobArtifacts do expect(response.headers.to_h).to include(download_headers) expect(response.body).to match_file(job.artifacts_file.file.file) end + + it_behaves_like 'storing arguments in the application context' + it_behaves_like 'not executing any extra queries for the application context' end context 'normal authentication' do @@ -558,7 +564,8 @@ RSpec.describe API::Ci::JobArtifacts do expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h) .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/, + 'Gitlab-Workhorse-Detect-Content-Type' => 'true') end end @@ -628,7 +635,8 @@ RSpec.describe API::Ci::JobArtifacts do expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h) .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/, + 'Gitlab-Workhorse-Detect-Content-Type' => 'true') expect(response.parsed_body).to be_empty end end @@ -646,7 +654,8 @@ RSpec.describe API::Ci::JobArtifacts do expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h) .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/, + 'Gitlab-Workhorse-Detect-Content-Type' => 'true') end end diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index d3820e4948e..4bd9f81fd1d 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -471,7 +471,7 @@ RSpec.describe API::Ci::Jobs do end context 'authorized user' do - context 'when trace is in ObjectStorage' do + context 'when log is in ObjectStorage' do let!(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) } let(:url) { 'http://object-storage/trace' } let(:file_path) { expand_fixture_path('trace/sample_trace') } @@ -485,49 +485,49 @@ RSpec.describe API::Ci::Jobs do end end - it 'returns specific job trace' do + it 'returns specific job logs' do expect(response).to have_gitlab_http_status(:ok) expect(response.body).to eq(job.trace.raw) end end - context 'when trace is artifact' do + context 'when log is artifact' do let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) } - it 'returns specific job trace' do + it 'returns specific job log' do expect(response).to have_gitlab_http_status(:ok) expect(response.body).to eq(job.trace.raw) end end - context 'when live trace and uploadless trace artifact' do + context 'when incremental logging and uploadless log artifact' do let(:job) { create(:ci_build, :trace_live, :unarchived_trace_artifact, pipeline: pipeline) } - it 'returns specific job trace' do + it 'returns specific job log' do expect(response).to have_gitlab_http_status(:ok) expect(response.body).to eq(job.trace.raw) end end - context 'when trace is live' do + context 'when log is incremental' do let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) } - it 'returns specific job trace' do + it 'returns specific job log' do expect(response).to have_gitlab_http_status(:ok) expect(response.body).to eq(job.trace.raw) end end - context 'when no trace' do + context 'when no log' do let(:job) { create(:ci_build, pipeline: pipeline) } - it 'returns empty trace' do + it 'returns empty log' do expect(response).to have_gitlab_http_status(:ok) expect(response.body).to be_empty end end - context 'when trace artifact record exists with no stored file' do + context 'when log artifact record exists with no stored file' do let(:job) { create(:ci_build, pipeline: pipeline) } before do @@ -544,7 +544,7 @@ RSpec.describe API::Ci::Jobs do context 'unauthorized user' do let(:api_user) { nil } - it 'does not return specific job trace' do + it 'does not return specific job log' do expect(response).to have_gitlab_http_status(:unauthorized) end end diff --git a/spec/requests/api/ci/resource_groups_spec.rb b/spec/requests/api/ci/resource_groups_spec.rb index f5b68557a0d..864c363e6d3 100644 --- a/spec/requests/api/ci/resource_groups_spec.rb +++ b/spec/requests/api/ci/resource_groups_spec.rb @@ -9,6 +9,36 @@ RSpec.describe API::Ci::ResourceGroups do let(:user) { developer } + describe 'GET /projects/:id/resource_groups' do + subject { get api("/projects/#{project.id}/resource_groups", user) } + + let!(:resource_groups) { create_list(:ci_resource_group, 3, project: project) } + + it 'returns all resource groups for this project', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + resource_groups.each_index do |i| + expect(json_response[i]['id']).to eq(resource_groups[i].id) + expect(json_response[i]['key']).to eq(resource_groups[i].key) + expect(json_response[i]['process_mode']).to eq(resource_groups[i].process_mode) + expect(Time.parse(json_response[i]['created_at'])).to be_like_time(resource_groups[i].created_at) + expect(Time.parse(json_response[i]['updated_at'])).to be_like_time(resource_groups[i].updated_at) + end + end + + context 'when user is reporter' do + let(:user) { reporter } + + it 'returns forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + describe 'GET /projects/:id/resource_groups/:key' do subject { get api("/projects/#{project.id}/resource_groups/#{key}", user) } diff --git a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb index f627f207d98..5767fa4326e 100644 --- a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb +++ b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb @@ -7,8 +7,20 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do include RedisHelpers include WorkhorseHelpers + let_it_be_with_reload(:parent_group) { create(:group) } + let_it_be_with_reload(:group) { create(:group, parent: parent_group) } + let_it_be_with_reload(:project) { create(:project, namespace: group, shared_runners_enabled: false) } + + let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } + let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:user) { create(:user) } + let(:registration_token) { 'abcdefg123456' } + before_all do + project.add_developer(user) + end + before do stub_feature_flags(ci_enable_live_trace: true) stub_gitlab_calls @@ -17,12 +29,6 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end describe '/api/v4/jobs' do - let(:parent_group) { create(:group) } - let(:group) { create(:group, parent: parent_group) } - let(:project) { create(:project, namespace: group, shared_runners_enabled: false) } - let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } - let(:runner) { create(:ci_runner, :project, projects: [project]) } - let(:user) { create(:user) } let(:job) do create(:ci_build, :artifacts, :extended_options, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) @@ -571,14 +577,21 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do context 'when artifact_type is archive' do context 'when artifact_format is zip' do + subject(:request) { upload_artifacts(file_upload, headers_with_token, params) } + let(:params) { { artifact_type: :archive, artifact_format: :zip } } + let(:expected_params) { { artifact_size: job.reload.artifacts_size } } + let(:subject_proc) { proc { subject } } it 'stores junit test report' do - upload_artifacts(file_upload, headers_with_token, params) + subject expect(response).to have_gitlab_http_status(:created) expect(job.reload.job_artifacts_archive).not_to be_nil end + + it_behaves_like 'storing arguments in the application context' + it_behaves_like 'not executing any extra queries for the application context' end context 'when artifact_format is gzip' do @@ -817,25 +830,23 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end context 'when job has artifacts' do - let(:job) { create(:ci_build) } + let(:job) { create(:ci_build, pipeline: pipeline, user: user) } let(:store) { JobArtifactUploader::Store::LOCAL } before do create(:ci_job_artifact, :archive, file_store: store, job: job) end - context 'when using job token' do + shared_examples 'successful artifact download' do context 'when artifacts are stored locally' do let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } end - before do + it 'downloads artifacts' do download_artifact - end - it 'download artifacts' do expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h).to include download_headers end @@ -843,26 +854,20 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do context 'when artifacts are stored remotely' do let(:store) { JobArtifactUploader::Store::REMOTE } - let!(:job) { create(:ci_build) } context 'when proxy download is being used' do - before do + it 'uses workhorse send-url' do download_artifact(direct_download: false) - end - it 'uses workhorse send-url' do expect(response).to have_gitlab_http_status(:ok) - expect(response.headers.to_h).to include( - 'Gitlab-Workhorse-Send-Data' => /send-url:/) + expect(response.headers.to_h).to include('Gitlab-Workhorse-Send-Data' => /send-url:/) end end context 'when direct download is being used' do - before do + it 'receives redirect for downloading artifacts' do download_artifact(direct_download: true) - end - it 'receive redirect for downloading artifacts' do expect(response).to have_gitlab_http_status(:found) expect(response.headers).to include('Location') end @@ -870,16 +875,119 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end - context 'when using runnners token' do - let(:token) { job.project.runners_token } + shared_examples 'forbidden request' do + it 'responds with forbidden' do + download_artifact + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when using job token' do + let(:token) { job.token } + + it_behaves_like 'successful artifact download' + + context 'when the job is no longer running' do + before do + job.success! + end + + it_behaves_like 'successful artifact download' + end + end + + context 'when using token belonging to the dependent job' do + let!(:dependent_job) { create(:ci_build, :running, :dependent, user: user, pipeline: pipeline) } + let!(:job) { dependent_job.all_dependencies.first } + + let(:token) { dependent_job.token } + + it_behaves_like 'successful artifact download' + + context 'when the dependent job is no longer running' do + before do + dependent_job.success! + end + + it_behaves_like 'forbidden request' + end + end + + context 'when using token belonging to another job created by another project member' do + let!(:ci_build) { create(:ci_build, :running, :dependent, user: user, pipeline: pipeline) } + let!(:job) { ci_build.all_dependencies.first } + + let!(:another_dev) { create(:user) } + + let(:token) { ci_build.token } before do - download_artifact + project.add_developer(another_dev) + ci_build.update!(user: another_dev) end - it 'responds with forbidden' do - expect(response).to have_gitlab_http_status(:forbidden) + it_behaves_like 'successful artifact download' + end + + context 'when using token belonging to a pending dependent job' do + let!(:ci_build) { create(:ci_build, :pending, :dependent, user: user, project: project, pipeline: pipeline) } + let!(:job) { ci_build.all_dependencies.first } + + let(:token) { ci_build.token } + + it_behaves_like 'forbidden request' + end + + context 'when using a token from a cross pipeline build' do + let!(:ci_build) { create(:ci_build, :pending, :dependent, user: user, project: project, pipeline: pipeline) } + let!(:job) { ci_build.all_dependencies.first } + + let!(:options) do + { + cross_dependencies: [ + { + pipeline: pipeline.id, + job: job.name, + artifacts: true + } + ] + + } end + + let!(:cross_pipeline) { create(:ci_pipeline, project: project, child_of: pipeline) } + let!(:cross_pipeline_build) { create(:ci_build, :running, project: project, user: user, options: options, pipeline: cross_pipeline) } + + let(:token) { cross_pipeline_build.token } + + before do + job.success! + end + + it_behaves_like 'successful artifact download' + end + + context 'when using a token from an unrelated project' do + let!(:ci_build) { create(:ci_build, :running, :dependent, user: user, project: project, pipeline: pipeline) } + let!(:job) { ci_build.all_dependencies.first } + + let!(:unrelated_ci_build) { create(:ci_build, :running, user: create(:user)) } + let(:token) { unrelated_ci_build.token } + + it_behaves_like 'forbidden request' + end + + context 'when using runnners token' do + let(:token) { job.project.runners_token } + + it_behaves_like 'forbidden request' + end + + context 'when using an invalid token' do + let(:token) { 'invalid-token' } + + it_behaves_like 'forbidden request' end end diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index a662c77e5a2..dbc5f0e74e2 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -496,15 +496,15 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do job2.success end - it 'returns dependent jobs' do + it 'returns dependent jobs with the token of the test job' do request_job expect(response).to have_gitlab_http_status(:created) expect(json_response['id']).to eq(test_job.id) expect(json_response['dependencies'].count).to eq(2) expect(json_response['dependencies']).to include( - { 'id' => job.id, 'name' => job.name, 'token' => job.token }, - { 'id' => job2.id, 'name' => job2.name, 'token' => job2.token }) + { 'id' => job.id, 'name' => job.name, 'token' => test_job.token }, + { 'id' => job2.id, 'name' => job2.name, 'token' => test_job.token }) end describe 'preloading job_artifacts_archive' do @@ -526,14 +526,14 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do job.success end - it 'returns dependent jobs' do + it 'returns dependent jobs with the token of the test job' do request_job expect(response).to have_gitlab_http_status(:created) expect(json_response['id']).to eq(test_job.id) expect(json_response['dependencies'].count).to eq(1) expect(json_response['dependencies']).to include( - { 'id' => job.id, 'name' => job.name, 'token' => job.token, + { 'id' => job.id, 'name' => job.name, 'token' => test_job.token, 'artifacts_file' => { 'filename' => 'ci_build_artifacts.zip', 'size' => 107464 } }) end end @@ -552,13 +552,13 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do job2.success end - it 'returns dependent jobs' do + it 'returns dependent jobs with the token of the test job' do request_job expect(response).to have_gitlab_http_status(:created) expect(json_response['id']).to eq(test_job.id) expect(json_response['dependencies'].count).to eq(1) - expect(json_response['dependencies'][0]).to include('id' => job2.id, 'name' => job2.name, 'token' => job2.token) + expect(json_response['dependencies'][0]).to include('id' => job2.id, 'name' => job2.name, 'token' => test_job.token) end end diff --git a/spec/requests/api/ci/runner/jobs_trace_spec.rb b/spec/requests/api/ci/runner/jobs_trace_spec.rb index d6928969beb..c3c074d80d9 100644 --- a/spec/requests/api/ci/runner/jobs_trace_spec.rb +++ b/spec/requests/api/ci/runner/jobs_trace_spec.rb @@ -272,7 +272,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_trace_chunks do it { expect(response).to have_gitlab_http_status(:forbidden) } end - context 'when the job trace is too big' do + context 'when the job log is too big' do before do project.actual_limits.update!(ci_jobs_trace_size_limit: 1) end diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index d6ebc197ab0..3000bdc2ce7 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -274,7 +274,7 @@ RSpec.describe API::Ci::Runners do expect(response).to have_gitlab_http_status(:ok) expect(json_response['description']).to eq(shared_runner.description) expect(json_response['maximum_timeout']).to be_nil - expect(json_response['status']).to eq("not_connected") + expect(json_response['status']).to eq('never_contacted') expect(json_response['active']).to eq(true) expect(json_response['paused']).to eq(false) end @@ -1216,15 +1216,6 @@ RSpec.describe API::Ci::Runners do end end end - - it 'enables a instance type runner' do - expect do - post api("/projects/#{project.id}/runners", admin), params: { runner_id: shared_runner.id } - end.to change { project.runners.count }.by(1) - - expect(shared_runner.reload).not_to be_instance_type - expect(response).to have_gitlab_http_status(:created) - end end it 'raises an error when no runner_id param is provided' do diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb index 6de6d1ef222..6f16fe5460b 100644 --- a/spec/requests/api/ci/secure_files_spec.rb +++ b/spec/requests/api/ci/secure_files_spec.rb @@ -143,7 +143,6 @@ RSpec.describe API::Ci::SecureFiles do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(secure_file.name) - expect(json_response['permissions']).to eq(secure_file.permissions) end it 'responds with 404 Not Found if requesting non-existing secure file' do @@ -159,7 +158,6 @@ RSpec.describe API::Ci::SecureFiles do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(secure_file.name) - expect(json_response['permissions']).to eq(secure_file.permissions) end end @@ -250,12 +248,11 @@ RSpec.describe API::Ci::SecureFiles do context 'authenticated user with admin permissions' do it 'creates a secure file' do expect do - post api("/projects/#{project.id}/secure_files", maintainer), params: file_params.merge(permissions: 'execute') + post api("/projects/#{project.id}/secure_files", maintainer), params: file_params end.to change {project.secure_files.count}.by(1) expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq('upload-keystore.jks') - expect(json_response['permissions']).to eq('execute') expect(json_response['checksum']).to eq(secure_file.checksum) expect(json_response['checksum_algorithm']).to eq('sha256') @@ -267,14 +264,6 @@ RSpec.describe API::Ci::SecureFiles do expect(Time.parse(json_response['created_at'])).to be_like_time(secure_file.created_at) end - it 'creates a secure file with read_only permissions by default' do - expect do - post api("/projects/#{project.id}/secure_files", maintainer), params: file_params - end.to change {project.secure_files.count}.by(1) - - expect(json_response['permissions']).to eq('read_only') - end - it 'uploads and downloads a secure file' do post api("/projects/#{project.id}/secure_files", maintainer), params: file_params @@ -327,15 +316,6 @@ RSpec.describe API::Ci::SecureFiles do expect(json_response['message']['name']).to include('has already been taken') end - it 'returns an error when an unexpected permission is supplied' do - expect do - post api("/projects/#{project.id}/secure_files", maintainer), params: file_params.merge(permissions: 'foo') - end.not_to change { project.secure_files.count } - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('permissions does not have a valid value') - end - it 'returns an error when an unexpected validation failure happens' do allow_next_instance_of(Ci::SecureFile) do |instance| allow(instance).to receive(:valid?).and_return(false) diff --git a/spec/requests/api/clusters/agent_tokens_spec.rb b/spec/requests/api/clusters/agent_tokens_spec.rb new file mode 100644 index 00000000000..ba26faa45a3 --- /dev/null +++ b/spec/requests/api/clusters/agent_tokens_spec.rb @@ -0,0 +1,179 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Clusters::AgentTokens do + let_it_be(:agent) { create(:cluster_agent) } + let_it_be(:agent_token_one) { create(:cluster_agent_token, agent: agent) } + let_it_be(:agent_token_two) { create(:cluster_agent_token, agent: agent) } + let_it_be(:project) { agent.project } + let_it_be(:user) { agent.created_by_user } + let_it_be(:unauthorized_user) { create(:user) } + + before_all do + project.add_maintainer(user) + project.add_guest(unauthorized_user) + end + + describe 'GET /projects/:id/cluster_agents/:agent_id/tokens' do + context 'with authorized user' do + it 'returns tokens' do + get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", user) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('public_api/v4/agent_tokens') + expect(json_response.count).to eq(2) + expect(json_response.first['name']).to eq(agent_token_one.name) + expect(json_response.first['agent_id']).to eq(agent.id) + expect(json_response.second['name']).to eq(agent_token_two.name) + expect(json_response.second['agent_id']).to eq(agent.id) + end + end + end + + context 'with unauthorized user' do + it 'cannot access agent tokens' do + get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", unauthorized_user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + it 'avoids N+1 queries', :request_store do + # Establish baseline + get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", user) + + control = ActiveRecord::QueryRecorder.new do + get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", user) + expect(response).to have_gitlab_http_status(:ok) + end + + # Now create a second record and ensure that the API does not execute + # any more queries than before + create(:cluster_agent_token, agent: agent) + + expect do + get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", user) + end.not_to exceed_query_limit(control) + end + end + + describe 'GET /projects/:id/cluster_agents/:agent_id/tokens/:token_id' do + context 'with authorized user' do + it 'returns an agent token' do + get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens/#{agent_token_one.id}", user) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/agent_token') + expect(json_response['id']).to eq(agent_token_one.id) + expect(json_response['name']).to eq(agent_token_one.name) + expect(json_response['agent_id']).to eq(agent.id) + end + end + + it 'returns a 404 error if agent token id is not available' do + get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens/#{non_existing_record_id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with unauthorized user' do + it 'cannot access single agent token' do + get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens/#{agent_token_one.id}", unauthorized_user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'cannot access token from agent of another project' do + another_project = create(:project, namespace: unauthorized_user.namespace) + another_agent = create(:cluster_agent, project: another_project, created_by_user: unauthorized_user) + + get api("/projects/#{another_project.id}/cluster_agents/#{another_agent.id}/tokens/#{agent_token_one.id}", + unauthorized_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'POST /projects/:id/cluster_agents/:agent_id/tokens' do + it 'creates a new agent token' do + params = { + name: 'test-token', + description: 'Test description' + } + post(api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", user), params: params) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/agent_token_with_token') + expect(json_response['name']).to eq(params[:name]) + expect(json_response['description']).to eq(params[:description]) + expect(json_response['agent_id']).to eq(agent.id) + end + end + + it 'returns a 400 error if name not given' do + post api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", user) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 404 error if project does not exist' do + post api("/projects/#{non_existing_record_id}/cluster_agents/tokens", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 404 error if agent does not exist' do + post api("/projects/#{project.id}/cluster_agents/#{non_existing_record_id}/tokens", user), + params: { name: "some" } + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'with unauthorized user' do + it 'prevents to create agent token' do + post api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", unauthorized_user), + params: { name: "some" } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + describe 'DELETE /projects/:id/cluster_agents/:agent_id/tokens/:token_id' do + it 'revokes agent token' do + delete api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens/#{agent_token_one.id}", user) + + expect(response).to have_gitlab_http_status(:no_content) + expect(agent_token_one.reload).to be_revoked + end + + it 'returns a 404 error when revoking non existent agent token' do + delete api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens/#{non_existing_record_id}", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns a 404 if the user is unauthorized to revoke' do + delete api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens/#{agent_token_one.id}", unauthorized_user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'cannot revoke token from agent of another project' do + another_project = create(:project, namespace: unauthorized_user.namespace) + another_agent = create(:cluster_agent, project: another_project, created_by_user: unauthorized_user) + + delete api("/projects/#{another_project.id}/cluster_agents/#{another_agent.id}/tokens/#{agent_token_one.id}", + unauthorized_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end +end diff --git a/spec/requests/api/container_registry_event_spec.rb b/spec/requests/api/container_registry_event_spec.rb index 4d38ddddffd..767e6e0b2ff 100644 --- a/spec/requests/api/container_registry_event_spec.rb +++ b/spec/requests/api/container_registry_event_spec.rb @@ -12,7 +12,7 @@ RSpec.describe API::ContainerRegistryEvent do allow(Gitlab.config.registry).to receive(:notification_secret) { secret_token } end - subject do + subject(:post_events) do post api('/container_registry_event/events'), params: { events: events }.to_json, headers: registry_headers.merge('Authorization' => secret_token) @@ -23,7 +23,7 @@ RSpec.describe API::ContainerRegistryEvent do allow(::ContainerRegistry::Event).to receive(:new).and_return(event) expect(event).to receive(:supported?).and_return(true) - subject + post_events expect(event).to have_received(:handle!).once expect(event).to have_received(:track!).once @@ -37,5 +37,37 @@ RSpec.describe API::ContainerRegistryEvent do expect(response).to have_gitlab_http_status(:unauthorized) end + + context 'when the event should update project statistics' do + let_it_be(:project) { create(:project) } + + let(:events) do + [ + { + action: 'push', + target: { + tag: 'latest', + repository: project.full_path + } + }, + { + action: 'delete', + target: { + tag: 'latest', + repository: project.full_path + } + } + ] + end + + it 'enqueues a project statistics update twice' do + expect(ProjectCacheWorker) + .to receive(:perform_async) + .with(project.id, [], [:container_registry_size]) + .twice.and_call_original + + expect { post_events }.to change { ProjectCacheWorker.jobs.size }.from(0).to(1) + end + end end end diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index 5fb24dc91a4..8328b454122 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -23,6 +23,7 @@ RSpec.describe API::Environments do expect(json_response).to be_an Array expect(json_response.size).to eq(1) expect(json_response.first['name']).to eq(environment.name) + expect(json_response.first['tier']).to eq(environment.tier) expect(json_response.first['external_url']).to eq(environment.external_url) expect(json_response.first['project']).to match_schema('public_api/v4/project') expect(json_response.first['enable_advanced_logs_querying']).to eq(false) @@ -150,6 +151,13 @@ RSpec.describe API::Environments do expect(json_response).to be_an Array expect(json_response.size).to eq(0) end + + it 'returns a 400 status code with invalid states' do + get api("/projects/#{project.id}/environments?states=test", user) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include('Requested states are invalid') + end end end @@ -165,12 +173,13 @@ RSpec.describe API::Environments do describe 'POST /projects/:id/environments' do context 'as a member' do it 'creates a environment with valid params' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep" } + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", tier: 'staging' } expect(response).to have_gitlab_http_status(:created) expect(response).to match_response_schema('public_api/v4/environment') expect(json_response['name']).to eq('mepmep') expect(json_response['slug']).to eq('mepmep') + expect(json_response['tier']).to eq('staging') expect(json_response['external']).to be nil end @@ -219,6 +228,15 @@ RSpec.describe API::Environments do expect(json_response['external_url']).to eq(url) end + it 'returns a 200 if tier is changed' do + put api("/projects/#{project.id}/environments/#{environment.id}", user), + params: { tier: 'production' } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/environment') + expect(json_response['tier']).to eq('production') + end + it "won't allow slug to be changed" do slug = environment.slug api_url = api("/projects/#{project.id}/environments/#{environment.id}", user) diff --git a/spec/requests/api/error_tracking/client_keys_spec.rb b/spec/requests/api/error_tracking/client_keys_spec.rb index 00c1e8799e6..ba4d713dff2 100644 --- a/spec/requests/api/error_tracking/client_keys_spec.rb +++ b/spec/requests/api/error_tracking/client_keys_spec.rb @@ -81,6 +81,10 @@ RSpec.describe API::ErrorTracking::ClientKeys do it 'returns a correct status' do expect(response).to have_gitlab_http_status(:ok) end + + it 'returns specific fields using the entity' do + expect(json_response.keys).to match_array(%w[id active public_key sentry_dsn]) + end end end end diff --git a/spec/requests/api/error_tracking/collector_spec.rb b/spec/requests/api/error_tracking/collector_spec.rb index fa0b238dcad..c0d7eb5460f 100644 --- a/spec/requests/api/error_tracking/collector_spec.rb +++ b/spec/requests/api/error_tracking/collector_spec.rb @@ -152,7 +152,17 @@ RSpec.describe API::ErrorTracking::Collector do context 'collector fails with validation error' do before do allow(::ErrorTracking::CollectErrorService) - .to receive(:new).and_raise(ActiveRecord::RecordInvalid) + .to receive(:new).and_raise(Gitlab::ErrorTracking::ErrorRepository::DatabaseError) + end + + it_behaves_like 'bad request' + end + + context 'with platform field too long' do + let(:params) do + event = Gitlab::Json.parse(raw_event) + event['platform'] = 'a' * 256 + Gitlab::Json.dump(event) end it_behaves_like 'bad request' diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index a265f67115a..4e75b0510d0 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -26,6 +26,7 @@ RSpec.describe API::Features, stub_feature_flags: false do end skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check end describe 'GET /features' do @@ -309,6 +310,55 @@ RSpec.describe API::Features, stub_feature_flags: false do 'definition' => known_feature_flag_definition_hash ) end + + describe 'mutually exclusive parameters' do + shared_examples 'fails to set the feature flag' do + it 'returns an error' do + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to match(/key, \w+ are mutually exclusive/) + end + end + + context 'when key and feature_group are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', feature_group: 'some-value' } + end + + it_behaves_like 'fails to set the feature flag' + end + + context 'when key and user are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', user: 'some-user' } + end + + it_behaves_like 'fails to set the feature flag' + end + + context 'when key and group are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', group: 'somepath' } + end + + it_behaves_like 'fails to set the feature flag' + end + + context 'when key and namespace are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', namespace: 'somepath' } + end + + it_behaves_like 'fails to set the feature flag' + end + + context 'when key and project are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', project: 'somepath' } + end + + it_behaves_like 'fails to set the feature flag' + end + end end context 'when the feature exists' do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index cb0b5f6bfc3..06d22e7e218 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -462,6 +462,66 @@ RSpec.describe API::Files do expect(range['commit']['committer_email']).to eq('dmitriy.zaporozhets@gmail.com') end + context 'with a range parameter' do + let(:params) { super().merge(range: { start: 2, end: 4 }) } + + it 'returns file blame attributes as json for the range' do + get api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.size).to eq(2) + + lines = json_response.map { |x| x['lines'] } + + expect(lines.map(&:size)).to eq(expected_blame_range_sizes[1..2]) + expect(lines.flatten).to eq(["require 'open3'", '', 'module Popen']) + end + + context 'when start > end' do + let(:params) { super().merge(range: { start: 4, end: 2 }) } + + it 'returns 400 error' do + get api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('range[start] must be less than or equal to range[end]') + end + end + + context 'when range is incomplete' do + let(:params) { super().merge(range: { start: 1 }) } + + it 'returns 400 error' do + get api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('range[end] is missing, range[end] is empty') + end + end + + context 'when range contains negative integers' do + let(:params) { super().merge(range: { start: -2, end: -5 }) } + + it 'returns 400 error' do + get api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('range[start] does not have a valid value, range[end] does not have a valid value') + end + end + + context 'when range is missing' do + let(:params) { super().merge(range: { start: '', end: '' }) } + + it 'returns 400 error' do + get api(route(file_path) + '/blame', current_user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('range[start] is empty, range[end] is empty') + end + end + end + it 'returns blame file info for files with dots' do url = route('.gitignore') + '/blame' diff --git a/spec/requests/api/graphql/boards/board_lists_query_spec.rb b/spec/requests/api/graphql/boards/board_lists_query_spec.rb index e8fb9daa43b..eb206465bce 100644 --- a/spec/requests/api/graphql/boards/board_lists_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_lists_query_spec.rb @@ -69,6 +69,10 @@ RSpec.describe 'get board lists' do let(:data_path) { [board_parent_type, :boards, :nodes, 0, :lists] } + def pagination_results_data(lists) + lists + end + def pagination_query(params) graphql_query_for( board_parent_type, @@ -94,7 +98,7 @@ RSpec.describe 'get board lists' do it_behaves_like 'sorted paginated query' do let(:sort_param) { } let(:first_param) { 2 } - let(:all_records) { lists.map { |list| global_id_of(list) } } + let(:all_records) { lists.map { |list| a_graphql_entity_for(list) } } end end end diff --git a/spec/requests/api/graphql/ci/config_spec.rb b/spec/requests/api/graphql/ci/config_spec.rb index 62b15a8396c..5f8a895b16e 100644 --- a/spec/requests/api/graphql/ci/config_spec.rb +++ b/spec/requests/api/graphql/ci/config_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe 'Query.ciConfig' do include GraphqlHelpers + include StubRequests subject(:post_graphql_query) { post_graphql(query, current_user: user) } @@ -57,6 +58,16 @@ RSpec.describe 'Query.ciConfig' do } } } + mergedYaml + includes { + type + location + blob + raw + extra + contextProject + contextSha + } } } ) @@ -71,10 +82,12 @@ RSpec.describe 'Query.ciConfig' do it 'returns the correct structure' do post_graphql_query - expect(graphql_data['ciConfig']).to eq( + expect(graphql_data['ciConfig']).to include( "status" => "VALID", "errors" => [], "warnings" => [], + "includes" => [], + "mergedYaml" => a_kind_of(String), "stages" => { "nodes" => @@ -222,24 +235,6 @@ RSpec.describe 'Query.ciConfig' do ) end - context 'when using deprecated keywords' do - let_it_be(:content) do - YAML.dump( - rspec: { script: 'ls', type: 'test' }, - types: ['test'] - ) - end - - it 'returns a warning' do - post_graphql_query - - expect(graphql_data['ciConfig']['warnings']).to include( - 'root `types` is deprecated in 9.0 and will be removed in 15.0.', - 'jobs:rspec `type` is deprecated in 9.0 and will be removed in 15.0.' - ) - end - end - context 'when the config file includes other files' do let_it_be(:content) do YAML.dump( @@ -271,6 +266,18 @@ RSpec.describe 'Query.ciConfig' do "status" => "VALID", "errors" => [], "warnings" => [], + "includes" => [ + { + "type" => "local", + "location" => "other_file.yml", + "blob" => "http://localhost/#{project.full_path}/-/blob/#{project.commit.sha}/other_file.yml", + "raw" => "http://localhost/#{project.full_path}/-/raw/#{project.commit.sha}/other_file.yml", + "extra" => {}, + "contextProject" => project.full_path, + "contextSha" => project.commit.sha + } + ], + "mergedYaml" => "---\nbuild:\n script: build\nrspec:\n script: rspec\n", "stages" => { "nodes" => @@ -302,7 +309,7 @@ RSpec.describe 'Query.ciConfig' do "when" => "on_success", "tags" => [], "needs" => { "nodes" => [] } -} + } ] } }, @@ -337,4 +344,101 @@ RSpec.describe 'Query.ciConfig' do ) end end + + context 'when the config file has multiple includes' do + let_it_be(:other_project) { create(:project, :repository, creator: user, namespace: user.namespace) } + + let_it_be(:content) do + YAML.dump( + include: [ + { local: 'other_file.yml' }, + { remote: 'https://gitlab.com/gitlab-org/gitlab/raw/1234/.hello.yml' }, + { file: 'other_project_file.yml', project: other_project.full_path }, + { template: 'Jobs/Build.gitlab-ci.yml' } + ], + rspec: { + script: 'rspec' + } + ) + end + + let(:remote_file_content) do + YAML.dump( + remote_file_test: { + script: 'remote_file_test' + } + ) + end + + before do + allow_next_instance_of(Repository) do |repository| + allow(repository).to receive(:blob_data_at).with(an_instance_of(String), 'other_file.yml') do + YAML.dump( + build: { + script: 'build' + } + ) + end + + allow(repository).to receive(:blob_data_at).with(an_instance_of(String), 'other_project_file.yml') do + YAML.dump( + other_project_test: { + script: 'other_project_test' + } + ) + end + end + + stub_full_request('https://gitlab.com/gitlab-org/gitlab/raw/1234/.hello.yml').to_return(body: remote_file_content) + + post_graphql_query + end + + it_behaves_like 'a working graphql query' + + # rubocop:disable Layout/LineLength + it 'returns correct includes' do + expect(graphql_data['ciConfig']["includes"]).to eq( + [ + { + "type" => "local", + "location" => "other_file.yml", + "blob" => "http://localhost/#{project.full_path}/-/blob/#{project.commit.sha}/other_file.yml", + "raw" => "http://localhost/#{project.full_path}/-/raw/#{project.commit.sha}/other_file.yml", + "extra" => {}, + "contextProject" => project.full_path, + "contextSha" => project.commit.sha + }, + { + "type" => "remote", + "location" => "https://gitlab.com/gitlab-org/gitlab/raw/1234/.hello.yml", + "blob" => nil, + "raw" => "https://gitlab.com/gitlab-org/gitlab/raw/1234/.hello.yml", + "extra" => {}, + "contextProject" => project.full_path, + "contextSha" => project.commit.sha + }, + { + "type" => "file", + "location" => "other_project_file.yml", + "blob" => "http://localhost/#{other_project.full_path}/-/blob/#{other_project.commit.sha}/other_project_file.yml", + "raw" => "http://localhost/#{other_project.full_path}/-/raw/#{other_project.commit.sha}/other_project_file.yml", + "extra" => { "project" => other_project.full_path, "ref" => "HEAD" }, + "contextProject" => project.full_path, + "contextSha" => project.commit.sha + }, + { + "type" => "template", + "location" => "Jobs/Build.gitlab-ci.yml", + "blob" => nil, + "raw" => "https://gitlab.com/gitlab-org/gitlab/-/raw/master/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml", + "extra" => {}, + "contextProject" => project.full_path, + "contextSha" => project.commit.sha + } + ] + ) + end + # rubocop:enable Layout/LineLength + end end diff --git a/spec/requests/api/graphql/ci/job_spec.rb b/spec/requests/api/graphql/ci/job_spec.rb index ddb2664d353..2fb90dcd92b 100644 --- a/spec/requests/api/graphql/ci/job_spec.rb +++ b/spec/requests/api/graphql/ci/job_spec.rb @@ -47,10 +47,8 @@ RSpec.describe 'Query.project(fullPath).pipelines.job(id)' do ) post_graphql(query, current_user: user) - expect(graphql_data_at(*path)).to match a_hash_including( - 'id' => global_id_of(job_2), - 'name' => job_2.name, - 'allowFailure' => job_2.allow_failure, + expect(graphql_data_at(*path)).to match a_graphql_entity_for( + job_2, :name, :allow_failure, 'duration' => 25, 'kind' => 'BUILD', 'queuedDuration' => 2.0, @@ -66,10 +64,7 @@ RSpec.describe 'Query.project(fullPath).pipelines.job(id)' do it 'retrieves scalar fields' do post_graphql(query, current_user: user) - expect(graphql_data_at(*path)).to match a_hash_including( - 'id' => global_id_of(job_2), - 'name' => job_2.name - ) + expect(graphql_data_at(*path)).to match a_graphql_entity_for(job_2, :name) end end end @@ -102,8 +97,8 @@ RSpec.describe 'Query.project(fullPath).pipelines.job(id)' do 'name' => test_stage.name, 'jobs' => a_hash_including( 'nodes' => contain_exactly( - a_hash_including('id' => global_id_of(job_2)), - a_hash_including('id' => global_id_of(job_3)) + a_graphql_entity_for(job_2), + a_graphql_entity_for(job_3) ) ) ) diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 39f0f696b08..6fa455cbfca 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -11,7 +11,8 @@ RSpec.describe 'Query.runner(id)' do let_it_be(:active_instance_runner) do create(:ci_runner, :instance, description: 'Runner 1', contacted_at: 2.hours.ago, active: true, version: 'adfe156', revision: 'a', locked: true, ip_address: '127.0.0.1', maximum_timeout: 600, - access_level: 0, tag_list: %w[tag1 tag2], run_untagged: true, executor_type: :custom) + access_level: 0, tag_list: %w[tag1 tag2], run_untagged: true, executor_type: :custom, + maintenance_note: 'Test maintenance note') end let_it_be(:inactive_instance_runner) do @@ -27,10 +28,6 @@ RSpec.describe 'Query.runner(id)' do let_it_be(:active_project_runner) { create(:ci_runner, :project) } - before do - allow(Gitlab::Ci::RunnerUpgradeCheck.instance).to receive(:check_runner_upgrade_status) - end - shared_examples 'runner details fetch' do let(:query) do wrap_fields(query_graphql_path(query_path, all_graphql_fields_for('CiRunner'))) @@ -66,6 +63,9 @@ RSpec.describe 'Query.runner(id)' do 'ipAddress' => runner.ip_address, 'runnerType' => runner.instance_type? ? 'INSTANCE_TYPE' : 'PROJECT_TYPE', 'executorName' => runner.executor_type&.dasherize, + 'architectureName' => runner.architecture, + 'platformName' => runner.platform, + 'maintenanceNote' => runner.maintenance_note, 'jobCount' => 0, 'jobs' => a_hash_including("count" => 0, "nodes" => [], "pageInfo" => anything), 'projectCount' => nil, @@ -239,8 +239,8 @@ RSpec.describe 'Query.runner(id)' do stale_runner_data = graphql_data_at(:stale_runner) expect(stale_runner_data).to match a_hash_including( - 'status' => 'NOT_CONNECTED', - 'legacyStatusWithExplicitVersion' => 'NOT_CONNECTED', + 'status' => 'STALE', + 'legacyStatusWithExplicitVersion' => 'STALE', 'newStatus' => 'STALE' ) @@ -253,8 +253,8 @@ RSpec.describe 'Query.runner(id)' do never_contacted_instance_runner_data = graphql_data_at(:never_contacted_instance_runner) expect(never_contacted_instance_runner_data).to match a_hash_including( - 'status' => 'NOT_CONNECTED', - 'legacyStatusWithExplicitVersion' => 'NOT_CONNECTED', + 'status' => 'NEVER_CONTACTED', + 'legacyStatusWithExplicitVersion' => 'NEVER_CONTACTED', 'newStatus' => 'NEVER_CONTACTED' ) end diff --git a/spec/requests/api/graphql/ci/runners_spec.rb b/spec/requests/api/graphql/ci/runners_spec.rb index 6b88c82b025..d3e94671724 100644 --- a/spec/requests/api/graphql/ci/runners_spec.rb +++ b/spec/requests/api/graphql/ci/runners_spec.rb @@ -56,9 +56,9 @@ RSpec.describe 'Query.runners' do it_behaves_like 'a working graphql query returning expected runner' end - context 'runner_type is PROJECT_TYPE and status is NOT_CONNECTED' do + context 'runner_type is PROJECT_TYPE and status is NEVER_CONTACTED' do let(:runner_type) { 'PROJECT_TYPE' } - let(:status) { 'NOT_CONNECTED' } + let(:status) { 'NEVER_CONTACTED' } let!(:expected_runner) { project_runner } diff --git a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb index 922a9ab277e..847fa72522e 100644 --- a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb +++ b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb @@ -127,7 +127,7 @@ RSpec.describe 'container repository details' do let(:query) do <<~GQL - query($id: ID!, $n: Int) { + query($id: ContainerRepositoryID!, $n: Int) { containerRepository(id: $id) { tags(first: $n) { edges { @@ -157,7 +157,7 @@ RSpec.describe 'container repository details' do let(:query) do <<~GQL - query($id: ID!, $n: ContainerRepositoryTagSort) { + query($id: ContainerRepositoryID!, $n: ContainerRepositoryTagSort) { containerRepository(id: $id) { tags(sort: $n) { edges { @@ -194,7 +194,7 @@ RSpec.describe 'container repository details' do let(:query) do <<~GQL - query($id: ID!, $n: String) { + query($id: ContainerRepositoryID!, $n: String) { containerRepository(id: $id) { tags(name: $n) { edges { @@ -232,7 +232,7 @@ RSpec.describe 'container repository details' do let(:query) do <<~GQL - query($id: ID!) { + query($id: ContainerRepositoryID!) { containerRepository(id: $id) { size } diff --git a/spec/requests/api/graphql/current_user_todos_spec.rb b/spec/requests/api/graphql/current_user_todos_spec.rb index 7f37abba74a..da1c893ec2b 100644 --- a/spec/requests/api/graphql/current_user_todos_spec.rb +++ b/spec/requests/api/graphql/current_user_todos_spec.rb @@ -37,8 +37,8 @@ RSpec.describe 'A Todoable that implements the CurrentUserTodos interface' do post_graphql(query, current_user: current_user) expect(todoable_response).to contain_exactly( - a_hash_including('id' => global_id_of(done_todo)), - a_hash_including('id' => global_id_of(pending_todo)) + a_graphql_entity_for(done_todo), + a_graphql_entity_for(pending_todo) ) end @@ -63,7 +63,7 @@ RSpec.describe 'A Todoable that implements the CurrentUserTodos interface' do post_graphql(query, current_user: current_user) expect(todoable_response).to contain_exactly( - a_hash_including('id' => global_id_of(pending_todo)) + a_graphql_entity_for(pending_todo) ) end end @@ -75,7 +75,7 @@ RSpec.describe 'A Todoable that implements the CurrentUserTodos interface' do post_graphql(query, current_user: current_user) expect(todoable_response).to contain_exactly( - a_hash_including('id' => global_id_of(done_todo)) + a_graphql_entity_for(done_todo) ) end end diff --git a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb index de3dbc5c324..d21c3046c1a 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb @@ -47,14 +47,14 @@ RSpec.describe 'getting dependency proxy settings for a group' do context 'with different permissions' do where(:group_visibility, :role, :access_granted) do :private | :maintainer | true - :private | :developer | true - :private | :reporter | true - :private | :guest | true + :private | :developer | false + :private | :reporter | false + :private | :guest | false :private | :anonymous | false :public | :maintainer | true - :public | :developer | true - :public | :reporter | true - :public | :guest | true + :public | :developer | false + :public | :reporter | false + :public | :guest | false :public | :anonymous | false end diff --git a/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb index c8797d84906..40f4b082072 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb @@ -46,14 +46,14 @@ RSpec.describe 'getting dependency proxy image ttl policy for a group' do context 'with different permissions' do where(:group_visibility, :role, :access_granted) do :private | :maintainer | true - :private | :developer | true - :private | :reporter | true - :private | :guest | true + :private | :developer | false + :private | :reporter | false + :private | :guest | false :private | :anonymous | false :public | :maintainer | true - :public | :developer | true - :public | :reporter | true - :public | :guest | true + :public | :developer | false + :public | :reporter | false + :public | :guest | false :public | :anonymous | false end diff --git a/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb index 3527c8183f6..c7149c100b2 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_manifests_spec.rb @@ -122,12 +122,12 @@ RSpec.describe 'getting dependency proxy manifests in a group' do let(:current_user) { owner } context 'with default sorting' do - let_it_be(:descending_manifests) { manifests.reverse.map { |manifest| global_id_of(manifest)} } + let_it_be(:descending_manifests) { manifests.reverse.map { |manifest| global_id_of(manifest) } } it_behaves_like 'sorted paginated query' do let(:sort_param) { '' } let(:first_param) { 2 } - let(:all_records) { descending_manifests } + let(:all_records) { descending_manifests.map(&:to_s) } end end diff --git a/spec/requests/api/graphql/group/group_members_spec.rb b/spec/requests/api/graphql/group/group_members_spec.rb index 78852622835..fec866486ae 100644 --- a/spec/requests/api/graphql/group/group_members_spec.rb +++ b/spec/requests/api/graphql/group/group_members_spec.rb @@ -24,8 +24,8 @@ RSpec.describe 'getting group members information' do expect(graphql_errors).to be_nil expect(graphql_data_at(:group, :group_members, :edges, :node)).to contain_exactly( - { 'user' => { 'id' => global_id_of(user_1) } }, - { 'user' => { 'id' => global_id_of(user_2) } }, + { 'user' => a_graphql_entity_for(user_1) }, + { 'user' => a_graphql_entity_for(user_2) }, 'user' => nil ) end @@ -77,6 +77,48 @@ RSpec.describe 'getting group members information' do end end + context 'by access levels' do + before do + parent_group.add_owner(user_1) + parent_group.add_maintainer(user_2) + end + + subject(:by_access_levels) { fetch_members(group: parent_group, args: { access_levels: access_levels }) } + + context 'by owner' do + let(:access_levels) { :OWNER } + + it 'returns owner' do + by_access_levels + + expect(graphql_errors).to be_nil + expect_array_response(user_1) + end + end + + context 'by maintainer' do + let(:access_levels) { :MAINTAINER } + + it 'returns maintainer' do + by_access_levels + + expect(graphql_errors).to be_nil + expect_array_response(user_2) + end + end + + context 'by owner and maintainer' do + let(:access_levels) { [:OWNER, :MAINTAINER] } + + it 'returns owner and maintainer' do + by_access_levels + + expect(graphql_errors).to be_nil + expect_array_response(user_1, user_2) + end + end + end + context 'member relations' do let_it_be(:child_group) { create(:group, :public, parent: parent_group) } let_it_be(:grandchild_group) { create(:group, :public, parent: child_group) } @@ -182,8 +224,8 @@ RSpec.describe 'getting group members information' do def expect_array_response(*items) expect(response).to have_gitlab_http_status(:success) - member_gids = graphql_data_at(:group, :group_members, :edges, :node, :user, :id) + members = graphql_data_at(:group, :group_members, :edges, :node, :user) - expect(member_gids).to match_array(items.map { |u| global_id_of(u) }) + expect(members).to match_array(items.map { |u| a_graphql_entity_for(u) }) end end diff --git a/spec/requests/api/graphql/group/merge_requests_spec.rb b/spec/requests/api/graphql/group/merge_requests_spec.rb index c0faff11c8d..434b0d16569 100644 --- a/spec/requests/api/graphql/group/merge_requests_spec.rb +++ b/spec/requests/api/graphql/group/merge_requests_spec.rb @@ -39,7 +39,7 @@ RSpec.describe 'Query.group.mergeRequests' do end def expected_mrs(mrs) - mrs.map { |mr| a_hash_including('id' => global_id_of(mr)) } + mrs.map { |mr| a_graphql_entity_for(mr) } end describe 'not passing any arguments' do diff --git a/spec/requests/api/graphql/group/milestones_spec.rb b/spec/requests/api/graphql/group/milestones_spec.rb index 2b80b5239c8..7c51409f907 100644 --- a/spec/requests/api/graphql/group/milestones_spec.rb +++ b/spec/requests/api/graphql/group/milestones_spec.rb @@ -170,10 +170,8 @@ RSpec.describe 'Milestones through GroupQuery' do end it 'returns correct values for scalar fields' do - expect(post_query).to eq({ - 'id' => global_id_of(milestone), - 'title' => milestone.title, - 'description' => milestone.description, + expect(post_query).to match a_graphql_entity_for( + milestone, :title, :description, 'state' => 'active', 'webPath' => milestone_path(milestone), 'dueDate' => milestone.due_date.iso8601, @@ -183,7 +181,7 @@ RSpec.describe 'Milestones through GroupQuery' do 'projectMilestone' => false, 'groupMilestone' => true, 'subgroupMilestone' => false - }) + ) end context 'milestone statistics' do diff --git a/spec/requests/api/graphql/issue/issue_spec.rb b/spec/requests/api/graphql/issue/issue_spec.rb index 42ca3348384..05fd6bf3022 100644 --- a/spec/requests/api/graphql/issue/issue_spec.rb +++ b/spec/requests/api/graphql/issue/issue_spec.rb @@ -8,8 +8,8 @@ RSpec.describe 'Query.issue(id)' do let_it_be(:project) { create(:project) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:current_user) { create(:user) } - let_it_be(:issue_params) { { 'id' => issue.to_global_id.to_s } } + let(:issue_params) { { 'id' => global_id_of(issue) } } let(:issue_data) { graphql_data['issue'] } let(:issue_fields) { all_graphql_fields_for('Issue'.classify) } @@ -100,7 +100,8 @@ RSpec.describe 'Query.issue(id)' do let_it_be(:issue_fields) { ['moved', 'movedTo { title }'] } let_it_be(:new_issue) { create(:issue) } let_it_be(:issue) { create(:issue, project: project, moved_to: new_issue) } - let_it_be(:issue_params) { { 'id' => issue.to_global_id.to_s } } + + let(:issue_params) { { 'id' => global_id_of(issue) } } before_all do new_issue.project.add_developer(current_user) diff --git a/spec/requests/api/graphql/merge_request/merge_request_spec.rb b/spec/requests/api/graphql/merge_request/merge_request_spec.rb index 75dd01a0763..d89f381753e 100644 --- a/spec/requests/api/graphql/merge_request/merge_request_spec.rb +++ b/spec/requests/api/graphql/merge_request/merge_request_spec.rb @@ -8,8 +8,8 @@ RSpec.describe 'Query.merge_request(id)' do let_it_be(:project) { create(:project, :empty_repo) } let_it_be(:merge_request) { create(:merge_request, source_project: project) } let_it_be(:current_user) { create(:user) } - let_it_be(:merge_request_params) { { 'id' => merge_request.to_global_id.to_s } } + let(:merge_request_params) { { 'id' => global_id_of(merge_request) } } let(:merge_request_data) { graphql_data['mergeRequest'] } let(:merge_request_fields) { all_graphql_fields_for('MergeRequest'.classify) } diff --git a/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb b/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb index 30e7f196542..394d9ff53d1 100644 --- a/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'CiCdSettingsUpdate' do +RSpec.describe 'ProjectCiCdSettingsUpdate' do include GraphqlHelpers let_it_be(:project) do diff --git a/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb b/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb index 12368e7e9c5..6818ba33e74 100644 --- a/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb @@ -64,11 +64,10 @@ RSpec.describe 'RunnersRegistrationTokenReset' do context 'applied to project' do let_it_be(:project) { create_default(:project) } + let(:target) { project } let(:input) { { type: 'PROJECT_TYPE', id: project.to_global_id.to_s } } - include_context 'when unauthorized', 'project' do - let(:target) { project } - end + include_context('when unauthorized', 'project') include_context 'when authorized', 'project' do let_it_be(:user) { project.first_owner } @@ -82,11 +81,10 @@ RSpec.describe 'RunnersRegistrationTokenReset' do context 'applied to group' do let_it_be(:group) { create_default(:group) } + let(:target) { group } let(:input) { { type: 'GROUP_TYPE', id: group.to_global_id.to_s } } - include_context 'when unauthorized', 'group' do - let(:target) { group } - end + include_context('when unauthorized', 'group') include_context 'when authorized', 'group' do let_it_be(:user) { create_default(:group_member, :owner, user: create(:user), group: group ).user } @@ -99,10 +97,12 @@ RSpec.describe 'RunnersRegistrationTokenReset' do context 'applied to instance' do before do - ApplicationSetting.create_from_defaults + target stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') end + let_it_be(:target) { ApplicationSetting.create_from_defaults } + let(:input) { { type: 'INSTANCE_TYPE' } } context 'when unauthorized' do diff --git a/spec/requests/api/graphql/mutations/clusters/agents/delete_spec.rb b/spec/requests/api/graphql/mutations/clusters/agents/delete_spec.rb index 5f6822223ca..4891e64aab8 100644 --- a/spec/requests/api/graphql/mutations/clusters/agents/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/clusters/agents/delete_spec.rb @@ -26,7 +26,7 @@ RSpec.describe 'Delete a cluster agent' do 'or you don\'t have permission to perform this action'] it 'does not delete cluster agent' do - expect { cluster_agent.reload }.not_to raise_error(ActiveRecord::RecordNotFound) + expect { cluster_agent.reload }.not_to raise_error end end diff --git a/spec/requests/api/graphql/mutations/container_expiration_policy/update_spec.rb b/spec/requests/api/graphql/mutations/container_expiration_policy/update_spec.rb index 0156142dc6f..ca7c1b2ce5f 100644 --- a/spec/requests/api/graphql/mutations/container_expiration_policy/update_spec.rb +++ b/spec/requests/api/graphql/mutations/container_expiration_policy/update_spec.rb @@ -135,7 +135,7 @@ RSpec.describe 'Updating the container expiration policy' do context 'with existing container expiration policy' do where(:user_role, :shared_examples_name) do :maintainer | 'accepting the mutation request updating the container expiration policy' - :developer | 'accepting the mutation request updating the container expiration policy' + :developer | 'denying the mutation request' :reporter | 'denying the mutation request' :guest | 'denying the mutation request' :anonymous | 'denying the mutation request' @@ -155,7 +155,7 @@ RSpec.describe 'Updating the container expiration policy' do where(:user_role, :shared_examples_name) do :maintainer | 'accepting the mutation request creating the container expiration policy' - :developer | 'accepting the mutation request creating the container expiration policy' + :developer | 'denying the mutation request' :reporter | 'denying the mutation request' :guest | 'denying the mutation request' :anonymous | 'denying the mutation request' diff --git a/spec/requests/api/graphql/mutations/dependency_proxy/group_settings/update_spec.rb b/spec/requests/api/graphql/mutations/dependency_proxy/group_settings/update_spec.rb index f05bf23ad27..9eb13e534ac 100644 --- a/spec/requests/api/graphql/mutations/dependency_proxy/group_settings/update_spec.rb +++ b/spec/requests/api/graphql/mutations/dependency_proxy/group_settings/update_spec.rb @@ -50,7 +50,7 @@ RSpec.describe 'Updating the dependency proxy group settings' do context 'with permission' do before do - group.add_developer(user) + group.add_maintainer(user) end it 'returns the updated dependency proxy settings', :aggregate_failures do diff --git a/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb b/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb index c9e9a22ee0b..31ba7ecdf0e 100644 --- a/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb +++ b/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb @@ -52,7 +52,7 @@ RSpec.describe 'Updating the dependency proxy image ttl policy' do context 'with permission' do before do - group.add_developer(user) + group.add_maintainer(user) end it 'returns the updated dependency proxy image ttl policy', :aggregate_failures do diff --git a/spec/requests/api/graphql/mutations/incident_management/timeline_event/create_spec.rb b/spec/requests/api/graphql/mutations/incident_management/timeline_event/create_spec.rb new file mode 100644 index 00000000000..3ea8b38e20f --- /dev/null +++ b/spec/requests/api/graphql/mutations/incident_management/timeline_event/create_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Creating an incident timeline event' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:incident) { create(:incident, project: project) } + let_it_be(:event_occurred_at) { Time.current } + let_it_be(:note) { 'demo note' } + + let(:input) { { incident_id: incident.to_global_id.to_s, note: note, occurred_at: event_occurred_at } } + let(:mutation) do + graphql_mutation(:timeline_event_create, input) do + <<~QL + clientMutationId + errors + timelineEvent { + id + author { id username } + incident { id title } + note + editable + action + occurredAt + } + QL + end + end + + let(:mutation_response) { graphql_mutation_response(:timeline_event_create) } + + before do + project.add_developer(user) + end + + it 'creates incident timeline event', :aggregate_failures do + post_graphql_mutation(mutation, current_user: user) + + timeline_event_response = mutation_response['timelineEvent'] + + expect(response).to have_gitlab_http_status(:success) + expect(timeline_event_response).to include( + 'author' => { + 'id' => user.to_global_id.to_s, + 'username' => user.username + }, + 'incident' => { + 'id' => incident.to_global_id.to_s, + 'title' => incident.title + }, + 'note' => note, + 'action' => 'comment', + 'editable' => false, + 'occurredAt' => event_occurred_at.iso8601 + ) + end +end diff --git a/spec/requests/api/graphql/mutations/incident_management/timeline_event/destroy_spec.rb b/spec/requests/api/graphql/mutations/incident_management/timeline_event/destroy_spec.rb new file mode 100644 index 00000000000..faff3bfe23a --- /dev/null +++ b/spec/requests/api/graphql/mutations/incident_management/timeline_event/destroy_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Removing an incident timeline event' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:incident) { create(:incident, project: project) } + let_it_be(:timeline_event) { create(:incident_management_timeline_event, incident: incident, project: project) } + + let(:variables) { { id: timeline_event.to_global_id.to_s } } + + let(:mutation) do + graphql_mutation(:timeline_event_destroy, variables) do + <<~QL + clientMutationId + errors + timelineEvent { + id + author { id username } + incident { id title } + note + noteHtml + editable + action + occurredAt + createdAt + updatedAt + } + QL + end + end + + let(:mutation_response) { graphql_mutation_response(:timeline_event_destroy) } + + before do + project.add_developer(user) + end + + it 'removes incident timeline event', :aggregate_failures do + post_graphql_mutation(mutation, current_user: user) + + timeline_event_response = mutation_response['timelineEvent'] + + expect(response).to have_gitlab_http_status(:success) + expect(timeline_event_response).to include( + 'author' => { + 'id' => timeline_event.author.to_global_id.to_s, + 'username' => timeline_event.author.username + }, + 'incident' => { + 'id' => incident.to_global_id.to_s, + 'title' => incident.title + }, + 'note' => timeline_event.note, + 'noteHtml' => timeline_event.note_html, + 'editable' => false, + 'action' => timeline_event.action, + 'occurredAt' => timeline_event.occurred_at.iso8601, + 'createdAt' => timeline_event.created_at.iso8601, + 'updatedAt' => timeline_event.updated_at.iso8601 + ) + expect { timeline_event.reload }.to raise_error ActiveRecord::RecordNotFound + end +end diff --git a/spec/requests/api/graphql/mutations/incident_management/timeline_event/promote_from_note_spec.rb b/spec/requests/api/graphql/mutations/incident_management/timeline_event/promote_from_note_spec.rb new file mode 100644 index 00000000000..b92f6af1d3d --- /dev/null +++ b/spec/requests/api/graphql/mutations/incident_management/timeline_event/promote_from_note_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Promote an incident timeline event from a comment' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:incident) { create(:incident, project: project) } + let_it_be(:comment) { create(:note, project: project, noteable: incident) } + + let(:input) { { note_id: comment.to_global_id.to_s } } + let(:mutation) do + graphql_mutation(:timeline_event_promote_from_note, input) do + <<~QL + clientMutationId + errors + timelineEvent { + author { id username } + incident { id title } + promotedFromNote { id } + note + action + editable + occurredAt + } + QL + end + end + + let(:mutation_response) { graphql_mutation_response(:timeline_event_promote_from_note) } + + before do + project.add_developer(user) + end + + it 'creates incident timeline event from the note', :aggregate_failures do + post_graphql_mutation(mutation, current_user: user) + + timeline_event_response = mutation_response['timelineEvent'] + + expect(response).to have_gitlab_http_status(:success) + expect(timeline_event_response).to include( + 'author' => { + 'id' => user.to_global_id.to_s, + 'username' => user.username + }, + 'incident' => { + 'id' => incident.to_global_id.to_s, + 'title' => incident.title + }, + 'promotedFromNote' => { + 'id' => comment.to_global_id.to_s + }, + 'note' => comment.note, + 'action' => 'comment', + 'editable' => false, + 'occurredAt' => comment.created_at.iso8601 + ) + end +end diff --git a/spec/requests/api/graphql/mutations/incident_management/timeline_event/update_spec.rb b/spec/requests/api/graphql/mutations/incident_management/timeline_event/update_spec.rb new file mode 100644 index 00000000000..1c4439cec6f --- /dev/null +++ b/spec/requests/api/graphql/mutations/incident_management/timeline_event/update_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Updating an incident timeline event' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:incident) { create(:incident, project: project) } + let_it_be_with_reload(:timeline_event) do + create(:incident_management_timeline_event, incident: incident, project: project) + end + + let(:occurred_at) { 1.minute.ago.iso8601 } + + let(:variables) do + { + id: timeline_event.to_global_id.to_s, + note: 'Updated note', + occurred_at: occurred_at + } + end + + let(:mutation) do + graphql_mutation(:timeline_event_update, variables) do + <<~QL + clientMutationId + errors + timelineEvent { + id + author { id username } + updatedByUser { id username } + incident { id title } + note + noteHtml + occurredAt + createdAt + updatedAt + } + QL + end + end + + let(:mutation_response) { graphql_mutation_response(:timeline_event_update) } + + before do + project.add_developer(user) + end + + it 'updates the timeline event', :aggregate_failures do + post_graphql_mutation(mutation, current_user: user) + + timeline_event_response = mutation_response['timelineEvent'] + + timeline_event.reload + + expect(response).to have_gitlab_http_status(:success) + expect(timeline_event_response).to include( + 'id' => timeline_event.to_global_id.to_s, + 'author' => { + 'id' => timeline_event.author.to_global_id.to_s, + 'username' => timeline_event.author.username + }, + 'updatedByUser' => { + 'id' => user.to_global_id.to_s, + 'username' => user.username + }, + 'incident' => { + 'id' => incident.to_global_id.to_s, + 'title' => incident.title + }, + 'note' => 'Updated note', + 'noteHtml' => timeline_event.note_html, + 'occurredAt' => occurred_at, + 'createdAt' => timeline_event.created_at.iso8601, + 'updatedAt' => timeline_event.updated_at.iso8601 + ) + end +end diff --git a/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb b/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb index 02b79dac489..715507c3cc5 100644 --- a/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/set_crm_contacts_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Setting issues crm contacts' do let(:operation_mode) { Types::MutationOperationModeEnum.default_mode } let(:initial_contacts) { contacts[0..1] } let(:mutation_contacts) { contacts[1..2] } - let(:contact_ids) { contact_global_ids(mutation_contacts) } + let(:contact_ids) { mutation_contacts.map { global_id_of(_1) } } let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" } let(:mutation) do @@ -45,8 +45,8 @@ RSpec.describe 'Setting issues crm contacts' do graphql_mutation_response(:issue_set_crm_contacts) end - def contact_global_ids(contacts) - contacts.map { |contact| global_id_of(contact) } + def expected_contacts(contacts) + contacts.map { |contact| a_graphql_entity_for(contact) } end before do @@ -58,8 +58,8 @@ RSpec.describe 'Setting issues crm contacts' do it 'updates the issue with correct contacts' do post_graphql_mutation(mutation, current_user: user) - expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id)) - .to match_array(contact_global_ids(mutation_contacts)) + expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes)) + .to match_array(expected_contacts(mutation_contacts)) end end @@ -70,8 +70,8 @@ RSpec.describe 'Setting issues crm contacts' do it 'updates the issue with correct contacts' do post_graphql_mutation(mutation, current_user: user) - expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id)) - .to match_array(contact_global_ids(initial_contacts + mutation_contacts)) + expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes)) + .to match_array(expected_contacts(initial_contacts + mutation_contacts)) end end @@ -82,8 +82,8 @@ RSpec.describe 'Setting issues crm contacts' do it 'updates the issue with correct contacts' do post_graphql_mutation(mutation, current_user: user) - expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes, :id)) - .to match_array(contact_global_ids(initial_contacts - mutation_contacts)) + expect(graphql_data_at(:issue_set_crm_contacts, :issue, :customer_relations_contacts, :nodes)) + .to match_array(expected_contacts(initial_contacts - mutation_contacts)) end end end @@ -117,7 +117,7 @@ RSpec.describe 'Setting issues crm contacts' do it_behaves_like 'successful mutation' context 'when the contact does not exist' do - let(:contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] } + let(:contact_ids) { [global_id_of(model_name: 'CustomerRelations::Contact', id: non_existing_record_id)] } it 'returns expected error' do post_graphql_mutation(mutation, current_user: user) @@ -159,7 +159,7 @@ RSpec.describe 'Setting issues crm contacts' do context 'when trying to remove non-existent contact' do let(:operation_mode) { Types::MutationOperationModeEnum.enum[:remove] } - let(:contact_ids) { ["gid://gitlab/CustomerRelations::Contact/#{non_existing_record_id}"] } + let(:contact_ids) { [global_id_of(model_name: 'CustomerRelations::Contact', id: non_existing_record_id)] } it 'raises expected error' do post_graphql_mutation(mutation, current_user: user) diff --git a/spec/requests/api/graphql/mutations/merge_requests/request_attention_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/request_attention_spec.rb new file mode 100644 index 00000000000..9c751913827 --- /dev/null +++ b/spec/requests/api/graphql/mutations/merge_requests/request_attention_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Request attention' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:merge_request) { create(:merge_request, reviewers: [user]) } + let_it_be(:project) { merge_request.project } + + let(:input) { { user_id: global_id_of(user) } } + + let(:mutation) do + variables = { + project_path: project.full_path, + iid: merge_request.iid.to_s + } + graphql_mutation(:merge_request_request_attention, variables.merge(input), + <<-QL.strip_heredoc + clientMutationId + errors + QL + ) + end + + def mutation_response + graphql_mutation_response(:merge_request_request_attention) + end + + def mutation_errors + mutation_response['errors'] + end + + before_all do + project.add_developer(current_user) + project.add_developer(user) + end + + it 'is successful' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_errors).to be_empty + end + + context 'when current user is not allowed to update the merge request' do + it 'returns an error' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + end + + context 'when user is not a reviewer' do + let(:input) { { user_id: global_id_of(create(:user)) } } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_errors).not_to be_empty + end + end + + context 'feature flag is disabled' do + before do + stub_feature_flags(mr_attention_requests: false) + end + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors[0]["message"]).to eq "Feature disabled" + end + end +end 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 d335642d321..194e42bf59d 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 @@ -109,7 +109,7 @@ RSpec.describe 'Updating the package settings' do where(:user_role, :shared_examples_name) do :maintainer | 'accepting the mutation request updating the package settings' - :developer | 'accepting the mutation request updating the package settings' + :developer | 'denying the mutation request' :reporter | 'denying the mutation request' :guest | 'denying the mutation request' :anonymous | 'denying the mutation request' @@ -131,7 +131,7 @@ RSpec.describe 'Updating the package settings' do where(:user_role, :shared_examples_name) do :maintainer | 'accepting the mutation request creating the package settings' - :developer | 'accepting the mutation request creating the package settings' + :developer | 'denying the mutation request' :reporter | 'denying the mutation request' :guest | 'denying the mutation request' :anonymous | 'denying the mutation request' diff --git a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb index 63b94dccca0..22b5f2d5112 100644 --- a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb @@ -64,7 +64,7 @@ RSpec.describe 'Adding a Note' do it 'creates a Note in a discussion' do post_graphql_mutation(mutation, current_user: current_user) - expect(mutation_response['note']['discussion']['id']).to eq(discussion.to_global_id.to_s) + expect(mutation_response['note']['discussion']).to match a_graphql_entity_for(discussion) end context 'when the discussion_id is not for a Discussion' do @@ -109,7 +109,7 @@ RSpec.describe 'Adding a Note' do post_graphql_mutation(mutation, current_user: current_user) expect(mutation_response).to include( - 'errors' => [/Merged this merge request/], + 'errors' => include(/Merged this merge request/), 'note' => nil ) end diff --git a/spec/requests/api/graphql/mutations/notes/reposition_image_diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/reposition_image_diff_note_spec.rb index 89e3a71280f..0f7ccac3179 100644 --- a/spec/requests/api/graphql/mutations/notes/reposition_image_diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/reposition_image_diff_note_spec.rb @@ -39,7 +39,7 @@ RSpec.describe 'Repositioning an ImageDiffNote' do post_graphql_mutation(mutation, current_user: current_user) end.to change { note.reset.position.x }.to(10) - expect(mutation_response['note']).to eq('id' => global_id_of(note)) + expect(mutation_response['note']).to match a_graphql_entity_for(note) expect(mutation_response['errors']).to be_empty end @@ -59,7 +59,7 @@ RSpec.describe 'Repositioning an ImageDiffNote' do post_graphql_mutation(mutation, current_user: current_user) end.not_to change { note.reset.position.x } - expect(mutation_response['note']).to eq('id' => global_id_of(note)) + expect(mutation_response['note']).to match a_graphql_entity_for(note) expect(mutation_response['errors']).to be_empty end end diff --git a/spec/requests/api/graphql/mutations/remove_attention_request_spec.rb b/spec/requests/api/graphql/mutations/remove_attention_request_spec.rb new file mode 100644 index 00000000000..053559b039d --- /dev/null +++ b/spec/requests/api/graphql/mutations/remove_attention_request_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Remove attention request' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:merge_request) { create(:merge_request, reviewers: [user]) } + let_it_be(:project) { merge_request.project } + + let(:input) { { user_id: global_id_of(user) } } + + let(:mutation) do + variables = { + project_path: project.full_path, + iid: merge_request.iid.to_s + } + graphql_mutation(:merge_request_remove_attention_request, variables.merge(input), + <<-QL.strip_heredoc + clientMutationId + errors + QL + ) + end + + def mutation_response + graphql_mutation_response(:merge_request_remove_attention_request) + end + + def mutation_errors + mutation_response['errors'] + end + + before_all do + project.add_developer(current_user) + project.add_developer(user) + end + + it 'is successful' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_errors).to be_empty + end + + context 'when current user is not allowed to update the merge request' do + it 'returns an error' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + end + + context 'when user is not a reviewer' do + let(:input) { { user_id: global_id_of(create(:user)) } } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_errors).not_to be_empty + end + end + + context 'feature flag is disabled' do + before do + stub_feature_flags(mr_attention_requests: false) + end + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors[0]["message"]).to eq "Feature disabled" + end + end +end diff --git a/spec/requests/api/graphql/mutations/timelogs/delete_spec.rb b/spec/requests/api/graphql/mutations/timelogs/delete_spec.rb new file mode 100644 index 00000000000..b674e77f093 --- /dev/null +++ b/spec/requests/api/graphql/mutations/timelogs/delete_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Delete a timelog' do + include GraphqlHelpers + let_it_be(:author) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:timelog) { create(:timelog, user: author, issue: issue, time_spent: 1800)} + + let(:current_user) { nil } + let(:mutation) { graphql_mutation(:timelogDelete, { 'id' => timelog.to_global_id.to_s }) } + let(:mutation_response) { graphql_mutation_response(:timelog_delete) } + + context 'when the user is not allowed to delete a timelog' do + let(:current_user) { create(:user) } + + before do + post_graphql_mutation(mutation, current_user: current_user) + end + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to delete a timelog' do + let(:current_user) { author } + + it 'deletes the timelog' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change(Timelog, :count).by(-1) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['timelog']).to include('id' => timelog.to_global_id.to_s) + end + end +end diff --git a/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb b/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb index c5c34e16717..dc20fde8e3c 100644 --- a/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/mark_all_done_spec.rb @@ -46,8 +46,8 @@ RSpec.describe 'Marking all todos done' do expect(todo3.reload.state).to eq('done') expect(other_user_todo.reload.state).to eq('pending') - updated_todo_ids = mutation_response['todos'].map { |todo| todo['id'] } - expect(updated_todo_ids).to contain_exactly(global_id_of(todo1), global_id_of(todo3)) + updated_todos = mutation_response['todos'] + expect(updated_todos).to contain_exactly(a_graphql_entity_for(todo1), a_graphql_entity_for(todo3)) end context 'when target_id is given', :aggregate_failures do @@ -66,8 +66,8 @@ RSpec.describe 'Marking all todos done' do expect(todo1.reload.state).to eq('pending') expect(todo3.reload.state).to eq('pending') - updated_todo_ids = mutation_response['todos'].map { |todo| todo['id'] } - expect(updated_todo_ids).to contain_exactly(global_id_of(target_todo1), global_id_of(target_todo2)) + updated_todos = mutation_response['todos'] + expect(updated_todos).to contain_exactly(a_graphql_entity_for(target_todo1), a_graphql_entity_for(target_todo2)) end context 'when target does not exist' do diff --git a/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb b/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb index 70e3cc7f5cd..4316bd060c1 100644 --- a/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb +++ b/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb @@ -11,8 +11,8 @@ RSpec.describe 'Restoring many Todos' do let_it_be(:author) { create(:user) } let_it_be(:other_user) { create(:user) } - let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :done, target: issue) } - let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done, target: issue) } + let_it_be_with_reload(:todo1) { create(:todo, user: current_user, author: author, state: :done, target: issue) } + let_it_be_with_reload(:todo2) { create(:todo, user: current_user, author: author, state: :done, target: issue) } let_it_be(:other_user_todo) { create(:todo, user: other_user, author: author, state: :done) } @@ -50,8 +50,8 @@ RSpec.describe 'Restoring many Todos' do expect(mutation_response).to include( 'errors' => be_empty, 'todos' => contain_exactly( - { 'id' => global_id_of(todo1), 'state' => 'pending' }, - { 'id' => global_id_of(todo2), 'state' => 'pending' } + a_graphql_entity_for(todo1, 'state' => 'pending'), + a_graphql_entity_for(todo2, 'state' => 'pending') ) ) end diff --git a/spec/requests/api/graphql/mutations/work_items/delete_task_spec.rb b/spec/requests/api/graphql/mutations/work_items/delete_task_spec.rb new file mode 100644 index 00000000000..05d3587d342 --- /dev/null +++ b/spec/requests/api/graphql/mutations/work_items/delete_task_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe "Delete a task in a work item's description" do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } + let_it_be(:task) { create(:work_item, :task, project: project, author: developer) } + let_it_be(:work_item, refind: true) do + create(:work_item, project: project, description: "- [ ] #{task.to_reference}+", lock_version: 3) + end + + before_all do + create(:issue_link, source_id: work_item.id, target_id: task.id) + end + + let(:lock_version) { work_item.lock_version } + let(:input) do + { + 'id' => work_item.to_global_id.to_s, + 'lockVersion' => lock_version, + 'taskData' => { + 'id' => task.to_global_id.to_s, + 'lineNumberStart' => 1, + 'lineNumberEnd' => 1 + } + } + end + + let(:mutation) { graphql_mutation(:workItemDeleteTask, input) } + let(:mutation_response) { graphql_mutation_response(:work_item_delete_task) } + + context 'the user is not allowed to update a work item' do + let(:current_user) { create(:user) } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user can update the description but not delete the task' do + let(:current_user) { create(:user).tap { |u| project.add_developer(u) } } + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to remove a task' do + let(:current_user) { developer } + + it 'removes the task from the work item' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(WorkItem, :count).by(-1).and( + change(IssueLink, :count).by(-1) + ).and( + change(work_item, :description).from("- [ ] #{task.to_reference}+").to('') + ) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']).to include('id' => work_item.to_global_id.to_s) + end + + context 'when removing the task fails' do + let(:lock_version) { 2 } + + it 'makes no changes to the DB and returns an error message' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to not_change(WorkItem, :count).and( + not_change(work_item, :description) + ) + + expect(mutation_response['errors']).to contain_exactly('Stale work item. Check lock version') + end + end + + context 'when the work_items feature flag is disabled' do + before do + stub_feature_flags(work_items: false) + end + + it 'does nothing and returns and error' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to not_change(WorkItem, :count) + + expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') + end + end + end +end diff --git a/spec/requests/api/graphql/packages/conan_spec.rb b/spec/requests/api/graphql/packages/conan_spec.rb index 84c5af33e5d..1f3732980d9 100644 --- a/spec/requests/api/graphql/packages/conan_spec.rb +++ b/spec/requests/api/graphql/packages/conan_spec.rb @@ -37,22 +37,19 @@ RSpec.describe 'conan package details' do it_behaves_like 'a package with files' it 'has the correct metadata' do - expect(metadata_response).to include( - 'id' => global_id_of(package.conan_metadatum), - 'recipe' => package.conan_metadatum.recipe, - 'packageChannel' => package.conan_metadatum.package_channel, - 'packageUsername' => package.conan_metadatum.package_username, - 'recipePath' => package.conan_metadatum.recipe_path + expect(metadata_response).to match( + a_graphql_entity_for(package.conan_metadatum, + :recipe, :package_channel, :package_username, :recipe_path) ) end it 'has the correct file metadata' do - expect(first_file_response_metadata).to include( - 'id' => global_id_of(first_file.conan_file_metadatum), - 'packageRevision' => first_file.conan_file_metadatum.package_revision, - 'conanPackageReference' => first_file.conan_file_metadatum.conan_package_reference, - 'recipeRevision' => first_file.conan_file_metadatum.recipe_revision, - 'conanFileType' => first_file.conan_file_metadatum.conan_file_type.upcase + expect(first_file_response_metadata).to match( + a_graphql_entity_for( + first_file.conan_file_metadatum, + :package_revision, :conan_package_reference, :recipe_revision, + conan_file_type: first_file.conan_file_metadatum.conan_file_type.upcase + ) ) end end diff --git a/spec/requests/api/graphql/packages/maven_spec.rb b/spec/requests/api/graphql/packages/maven_spec.rb index d28d32b0df5..9d59a922660 100644 --- a/spec/requests/api/graphql/packages/maven_spec.rb +++ b/spec/requests/api/graphql/packages/maven_spec.rb @@ -11,12 +11,8 @@ RSpec.describe 'maven package details' do shared_examples 'correct maven metadata' do it 'has the correct metadata' do - expect(metadata_response).to include( - 'id' => global_id_of(package.maven_metadatum), - 'path' => package.maven_metadatum.path, - 'appGroup' => package.maven_metadatum.app_group, - 'appVersion' => package.maven_metadatum.app_version, - 'appName' => package.maven_metadatum.app_name + expect(metadata_response).to match a_graphql_entity_for( + package.maven_metadatum, :path, :app_group, :app_version, :app_name ) end end diff --git a/spec/requests/api/graphql/packages/nuget_spec.rb b/spec/requests/api/graphql/packages/nuget_spec.rb index ba8d2ca42d2..87cffc67ce5 100644 --- a/spec/requests/api/graphql/packages/nuget_spec.rb +++ b/spec/requests/api/graphql/packages/nuget_spec.rb @@ -22,24 +22,19 @@ RSpec.describe 'nuget package details' do it_behaves_like 'a package with files' it 'has the correct metadata' do - expect(metadata_response).to include( - 'id' => global_id_of(package.nuget_metadatum), - 'licenseUrl' => package.nuget_metadatum.license_url, - 'projectUrl' => package.nuget_metadatum.project_url, - 'iconUrl' => package.nuget_metadatum.icon_url + expect(metadata_response).to match a_graphql_entity_for( + package.nuget_metadatum, :license_url, :project_url, :icon_url ) end it 'has dependency links' do - expect(dependency_link_response).to include( - 'id' => global_id_of(dependency_link), + expect(dependency_link_response).to match a_graphql_entity_for( + dependency_link, 'dependencyType' => dependency_link.dependency_type.upcase ) - expect(dependency_response).to include( - 'id' => global_id_of(dependency_link.dependency), - 'name' => dependency_link.dependency.name, - 'versionPattern' => dependency_link.dependency.version_pattern + expect(dependency_response).to match a_graphql_entity_for( + dependency_link.dependency, :name, :version_pattern ) end diff --git a/spec/requests/api/graphql/packages/package_spec.rb b/spec/requests/api/graphql/packages/package_spec.rb index 365efc514d4..0335c1085b4 100644 --- a/spec/requests/api/graphql/packages/package_spec.rb +++ b/spec/requests/api/graphql/packages/package_spec.rb @@ -65,32 +65,6 @@ RSpec.describe 'package details' do end end - context 'there are other versions of this package' do - let(:depth) { 3 } - let(:excluded) { %w[metadata project tags pipelines] } # to limit the query complexity - - let_it_be(:siblings) { create_list(:composer_package, 2, project: project, name: composer_package.name) } - - it 'includes the sibling versions' do - subject - - expect(graphql_data_at(:package, :versions, :nodes)).to match_array( - siblings.map { |p| a_hash_including('id' => global_id_of(p)) } - ) - end - - context 'going deeper' do - let(:depth) { 6 } - - it 'does not create a cycle of versions' do - subject - - expect(graphql_data_at(:package, :versions, :nodes, :version)).to be_present - expect(graphql_data_at(:package, :versions, :nodes, :versions, :nodes)).to match_array [nil, nil] - end - end - end - context 'with package files pending destruction' do let_it_be(:package_file) { create(:package_file, package: composer_package) } let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: composer_package) } diff --git a/spec/requests/api/graphql/packages/pypi_spec.rb b/spec/requests/api/graphql/packages/pypi_spec.rb index 64fe7d29a7a..0cc5bd2e3b2 100644 --- a/spec/requests/api/graphql/packages/pypi_spec.rb +++ b/spec/requests/api/graphql/packages/pypi_spec.rb @@ -19,9 +19,8 @@ RSpec.describe 'pypi package details' do it_behaves_like 'a package with files' it 'has the correct metadata' do - expect(metadata_response).to include( - 'id' => global_id_of(package.pypi_metadatum), - 'requiredPython' => package.pypi_metadatum.required_python + expect(metadata_response).to match a_graphql_entity_for( + package.pypi_metadatum, :required_python ) end end diff --git a/spec/requests/api/graphql/project/alert_management/integrations_spec.rb b/spec/requests/api/graphql/project/alert_management/integrations_spec.rb index 1793d4961eb..773922c1864 100644 --- a/spec/requests/api/graphql/project/alert_management/integrations_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/integrations_spec.rb @@ -53,33 +53,24 @@ RSpec.describe 'getting Alert Management Integrations' do end context 'when no extra params given' do - let(:http_integration_response) { integrations.first } - let(:prometheus_integration_response) { integrations.second } - it_behaves_like 'a working graphql query' - it { expect(integrations.size).to eq(2) } - it 'returns the correct properties of the integrations' do - expect(http_integration_response).to include( - 'id' => global_id_of(active_http_integration), - 'type' => 'HTTP', - 'name' => active_http_integration.name, - 'active' => active_http_integration.active, - 'token' => active_http_integration.token, - 'url' => active_http_integration.url, - 'apiUrl' => nil - ) - - expect(prometheus_integration_response).to include( - 'id' => global_id_of(prometheus_integration), - 'type' => 'PROMETHEUS', - 'name' => 'Prometheus', - 'active' => prometheus_integration.manual_configuration?, - 'token' => project_alerting_setting.token, - 'url' => "http://localhost/#{project.full_path}/prometheus/alerts/notify.json", - 'apiUrl' => prometheus_integration.api_url - ) + expect(integrations).to match [ + a_graphql_entity_for( + active_http_integration, + :name, :active, :token, :url, type: 'HTTP', api_url: nil + ), + a_graphql_entity_for( + prometheus_integration, + 'type' => 'PROMETHEUS', + 'name' => 'Prometheus', + 'active' => prometheus_integration.manual_configuration?, + 'token' => project_alerting_setting.token, + 'url' => "http://localhost/#{project.full_path}/prometheus/alerts/notify.json", + 'apiUrl' => prometheus_integration.api_url + ) + ] end end @@ -88,17 +79,9 @@ RSpec.describe 'getting Alert Management Integrations' do it_behaves_like 'a working graphql query' - it { expect(integrations).to be_one } - it 'returns the correct properties of the HTTP integration' do - expect(integrations.first).to include( - 'id' => global_id_of(active_http_integration), - 'type' => 'HTTP', - 'name' => active_http_integration.name, - 'active' => active_http_integration.active, - 'token' => active_http_integration.token, - 'url' => active_http_integration.url, - 'apiUrl' => nil + expect(integrations).to contain_exactly a_graphql_entity_for( + active_http_integration, :name, :active, :token, :url, type: 'HTTP', api_url: nil ) end end @@ -108,11 +91,9 @@ RSpec.describe 'getting Alert Management Integrations' do it_behaves_like 'a working graphql query' - it { expect(integrations).to be_one } - it 'returns the correct properties of the Prometheus Integration' do - expect(integrations.first).to include( - 'id' => global_id_of(prometheus_integration), + expect(integrations).to contain_exactly a_graphql_entity_for( + prometheus_integration, 'type' => 'PROMETHEUS', 'name' => 'Prometheus', 'active' => prometheus_integration.manual_configuration?, diff --git a/spec/requests/api/graphql/project/cluster_agents_spec.rb b/spec/requests/api/graphql/project/cluster_agents_spec.rb index c9900fea277..a34df0ee6f4 100644 --- a/spec/requests/api/graphql/project/cluster_agents_spec.rb +++ b/spec/requests/api/graphql/project/cluster_agents_spec.rb @@ -29,7 +29,7 @@ RSpec.describe 'Project.cluster_agents' do post_graphql(query, current_user: current_user) expect(graphql_data_at(:project, :cluster_agents, :nodes)).to match_array( - agents.map { |agent| a_hash_including('id' => global_id_of(agent)) } + agents.map { |agent| a_graphql_entity_for(agent) } ) end @@ -62,9 +62,9 @@ RSpec.describe 'Project.cluster_agents' do tokens = graphql_data_at(:project, :cluster_agents, :nodes, :tokens, :nodes) expect(tokens).to match([ - a_hash_including('id' => global_id_of(token_3)), - a_hash_including('id' => global_id_of(token_2)), - a_hash_including('id' => global_id_of(token_1)) + a_graphql_entity_for(token_3), + a_graphql_entity_for(token_2), + a_graphql_entity_for(token_1) ]) end diff --git a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb new file mode 100644 index 00000000000..708fa96986c --- /dev/null +++ b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting incident timeline events' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user) } + let_it_be(:updated_by_user) { create(:user) } + let_it_be(:incident) { create(:incident, project: project) } + let_it_be(:another_incident) { create(:incident, project: project) } + let_it_be(:promoted_from_note) { create(:note, project: project, noteable: incident) } + + let_it_be(:timeline_event) do + create( + :incident_management_timeline_event, + incident: incident, + project: project, + updated_by_user: updated_by_user, + promoted_from_note: promoted_from_note + ) + end + + let_it_be(:second_timeline_event) do + create(:incident_management_timeline_event, incident: incident, project: project) + end + + let_it_be(:another_timeline_event) do + create(:incident_management_timeline_event, incident: another_incident, project: project) + end + + let(:params) { { incident_id: incident.to_global_id.to_s } } + + let(:timeline_event_fields) do + <<~QUERY + nodes { + id + author { id username } + updatedByUser { id username } + incident { id title } + note + noteHtml + promotedFromNote { id body } + editable + action + occurredAt + createdAt + updatedAt + } + QUERY + end + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('incidentManagementTimelineEvents', params, timeline_event_fields) + ) + end + + let(:timeline_events) do + graphql_data.dig('project', 'incidentManagementTimelineEvents', 'nodes') + end + + before do + project.add_guest(current_user) + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' + + it 'returns the correct number of timeline events' do + expect(timeline_events.count).to eq(2) + end + + it 'returns the correct properties of the incident timeline events' do + expect(timeline_events.first).to include( + 'author' => { + 'id' => timeline_event.author.to_global_id.to_s, + 'username' => timeline_event.author.username + }, + 'updatedByUser' => { + 'id' => updated_by_user.to_global_id.to_s, + 'username' => updated_by_user.username + }, + 'incident' => { + 'id' => incident.to_global_id.to_s, + 'title' => incident.title + }, + 'note' => timeline_event.note, + 'noteHtml' => timeline_event.note_html, + 'promotedFromNote' => { + 'id' => promoted_from_note.to_global_id.to_s, + 'body' => promoted_from_note.note + }, + 'editable' => false, + 'action' => timeline_event.action, + 'occurredAt' => timeline_event.occurred_at.iso8601, + 'createdAt' => timeline_event.created_at.iso8601, + 'updatedAt' => timeline_event.updated_at.iso8601 + ) + end + + context 'when filtering by id' do + let(:params) { { incident_id: incident.to_global_id.to_s, id: timeline_event.to_global_id.to_s } } + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('incidentManagementTimelineEvent', params, 'id occurredAt') + ) + end + + it_behaves_like 'a working graphql query' + + it 'returns a single timeline event', :aggregate_failures do + single_timeline_event = graphql_data.dig('project', 'incidentManagementTimelineEvent') + + expect(single_timeline_event).to include( + 'id' => timeline_event.to_global_id.to_s, + 'occurredAt' => timeline_event.occurred_at.iso8601 + ) + end + end +end diff --git a/spec/requests/api/graphql/project/issue/design_collection/version_spec.rb b/spec/requests/api/graphql/project/issue/design_collection/version_spec.rb index f544d78ecbb..8cda61f0628 100644 --- a/spec/requests/api/graphql/project/issue/design_collection/version_spec.rb +++ b/spec/requests/api/graphql/project/issue/design_collection/version_spec.rb @@ -71,11 +71,7 @@ RSpec.describe 'Query.project(fullPath).issue(iid).designCollection.version(sha) it 'finds all the designs as of the given version' do post_query - expect(data).to match( - a_hash_including( - 'id' => global_id_of(design_at_version), - 'filename' => design.filename - )) + expect(data).to match a_graphql_entity_for(design_at_version, filename: design.filename) end context 'when the current_user is not authorized' do @@ -119,7 +115,8 @@ RSpec.describe 'Query.project(fullPath).issue(iid).designCollection.version(sha) let(:results) do issue.designs.visible_at_version(version).map do |d| dav = build(:design_at_version, design: d, version: version) - { 'id' => global_id_of(dav), 'filename' => d.filename } + + a_graphql_entity_for(dav, filename: d.filename) end end @@ -132,8 +129,8 @@ RSpec.describe 'Query.project(fullPath).issue(iid).designCollection.version(sha) describe 'filtering' do let(:designs) { issue.designs.sample(3) } let(:filenames) { designs.map(&:filename) } - let(:ids) do - designs.map { |d| global_id_of(build(:design_at_version, design: d, version: version)) } + let(:expected_designs) do + designs.map { |d| a_graphql_entity_for(build(:design_at_version, design: d, version: version)) } end before do @@ -144,7 +141,7 @@ RSpec.describe 'Query.project(fullPath).issue(iid).designCollection.version(sha) let(:dav_params) { { filenames: filenames } } it 'finds the designs by filename' do - expect(data.map { |e| e.dig('node', 'id') }).to match_array(ids) + expect(data.map { |e| e['node'] }).to match_array expected_designs end end @@ -160,9 +157,9 @@ RSpec.describe 'Query.project(fullPath).issue(iid).designCollection.version(sha) describe 'pagination' do let(:end_cursor) { graphql_data_at(*path_prefix, :designs_at_version, :page_info, :end_cursor) } - let(:ids) do + let(:entities) do ::DesignManagement::Design.visible_at_version(version).order(:id).map do |d| - global_id_of(build(:design_at_version, design: d, version: version)) + a_graphql_entity_for(build(:design_at_version, design: d, version: version)) end end @@ -178,19 +175,19 @@ RSpec.describe 'Query.project(fullPath).issue(iid).designCollection.version(sha) let(:fields) { ['pageInfo { endCursor }', 'edges { node { id } }'] } def response_values(data = graphql_data) - data.dig(*path).map { |e| e.dig('node', 'id') } + data.dig(*path).map { |e| e['node'] } end it 'sorts designs for reliable pagination' do post_graphql(query, current_user: current_user) - expect(response_values).to match_array(ids.take(2)) + expect(response_values).to match_array(entities.take(2)) post_graphql(cursored_query, current_user: current_user) new_data = Gitlab::Json.parse(response.body).fetch('data') - expect(response_values(new_data)).to match_array(ids.drop(2)) + expect(response_values(new_data)).to match_array(entities.drop(2)) end end end @@ -202,9 +199,7 @@ RSpec.describe 'Query.project(fullPath).issue(iid).designCollection.version(sha) end let(:results) do - version.designs.map do |design| - { 'id' => global_id_of(design), 'filename' => design.filename } - end + version.designs.map { |design| a_graphql_entity_for(design, :filename) } end it 'finds all the designs as of the given version' do diff --git a/spec/requests/api/graphql/project/issue/designs/designs_spec.rb b/spec/requests/api/graphql/project/issue/designs/designs_spec.rb index f0205319983..02bc9457c07 100644 --- a/spec/requests/api/graphql/project/issue/designs/designs_spec.rb +++ b/spec/requests/api/graphql/project/issue/designs/designs_spec.rb @@ -58,8 +58,8 @@ RSpec.describe 'Getting designs related to an issue' do post_graphql(query, current_user: current_user) - expect(design_response).to eq( - 'id' => design.to_global_id.to_s, + expect(design_response).to match a_graphql_entity_for( + design, 'event' => 'CREATION', 'fullPath' => design.full_path, 'filename' => design.filename, @@ -93,7 +93,7 @@ RSpec.describe 'Getting designs related to an issue' do let(:end_cursor) { design_collection.dig('designs', 'pageInfo', 'endCursor') } - let(:ids) { issue.designs.order(:id).map { |d| global_id_of(d) } } + let(:expected_designs) { issue.designs.order(:id).map { |d| a_graphql_entity_for(d) } } let(:query) { make_query(designs_fragment(first: 2)) } @@ -107,19 +107,19 @@ RSpec.describe 'Getting designs related to an issue' do query_graphql_field(:designs, params, design_query_fields) end - def response_ids(data = graphql_data) + def response_designs(data = graphql_data) path = %w[project issue designCollection designs edges] - data.dig(*path).map { |e| e.dig('node', 'id') } + data.dig(*path).map { |e| e['node'] } end it 'sorts designs for reliable pagination' do - expect(response_ids).to match_array(ids.take(2)) + expect(response_designs).to match_array(expected_designs.take(2)) post_graphql(cursored_query, current_user: current_user) new_data = Gitlab::Json.parse(response.body).fetch('data') - expect(response_ids(new_data)).to match_array(ids.drop(2)) + expect(response_designs(new_data)).to match_array(expected_designs.drop(2)) end end @@ -273,8 +273,10 @@ RSpec.describe 'Getting designs related to an issue' do end it 'returns the correct v432x230-sized design images' do + v0 = design.actions.most_recent.first.version + expect(design_nodes).to contain_exactly( - a_hash_including('imageV432x230' => design_image_url(design, ref: version.sha, size: :v432x230)), + a_hash_including('imageV432x230' => design_image_url(design, ref: v0.sha, size: :v432x230)), a_hash_including('imageV432x230' => design_image_url(second_design, ref: version.sha, size: :v432x230)) ) end @@ -323,8 +325,10 @@ RSpec.describe 'Getting designs related to an issue' do end it 'returns the correct v432x230-sized design images' do + v0 = design.actions.most_recent.first.version + expect(design_nodes).to contain_exactly( - a_hash_including('imageV432x230' => design_image_url(design, ref: version.sha, size: :v432x230)), + a_hash_including('imageV432x230' => design_image_url(design, ref: v0.sha, size: :v432x230)), a_hash_including('imageV432x230' => design_image_url(second_design, ref: version.sha, size: :v432x230)) ) end diff --git a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb index de2ace95757..3b1eb0b4b02 100644 --- a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb +++ b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb @@ -51,7 +51,7 @@ RSpec.describe 'Getting designs related to an issue' do design_data = designs_data['nodes'].first note_data = design_data['notes']['nodes'].first - expect(note_data['id']).to eq(note.to_global_id.to_s) + expect(note_data).to match(a_graphql_entity_for(note)) end def query(note_fields = all_graphql_fields_for(Note, max_depth: 1)) diff --git a/spec/requests/api/graphql/project/issue_spec.rb b/spec/requests/api/graphql/project/issue_spec.rb index ddf63a8f2c9..2415e9ef60f 100644 --- a/spec/requests/api/graphql/project/issue_spec.rb +++ b/spec/requests/api/graphql/project/issue_spec.rb @@ -144,10 +144,7 @@ RSpec.describe 'Query.project(fullPath).issue(iid)' do data = graphql_data.dig(*path) - expect(data).to match( - a_hash_including('id' => global_id_of(version), - 'sha' => version.sha) - ) + expect(data).to match a_graphql_entity_for(version, :sha) end end @@ -184,6 +181,6 @@ RSpec.describe 'Query.project(fullPath).issue(iid)' do end def id_hash(object) - a_hash_including('id' => global_id_of(object)) + a_graphql_entity_for(object) end end diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index cefe88aafc8..d2f34080be3 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -66,7 +66,7 @@ RSpec.describe 'getting merge request information nested in a project' do it 'includes reviewers' do expected = merge_request.reviewers.map do |r| - a_hash_including('id' => global_id_of(r), 'username' => r.username) + a_graphql_entity_for(r, :username) end post_graphql(query, current_user: current_user) @@ -425,7 +425,7 @@ RSpec.describe 'getting merge request information nested in a project' do other_users.each do |user| assign_user(user) - merge_request.merge_request_reviewers.find_or_create_by!(reviewer: user) + merge_request.merge_request_reviewers.find_or_create_by!(reviewer: user, state: :attention_requested) end expect { post_graphql(query) }.not_to exceed_query_limit(baseline) @@ -466,7 +466,7 @@ RSpec.describe 'getting merge request information nested in a project' do let(:can_update) { false } def assign_user(user) - merge_request.merge_request_reviewers.create!(reviewer: user) + merge_request.merge_request_reviewers.create!(reviewer: user, state: :attention_requested) 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 303748bc70e..5daec5543c0 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'getting merge request listings nested in a project' do let_it_be(:current_user) { create(:user) } let_it_be(:label) { create(:label, project: project) } - let_it_be(:merge_request_a) do + let_it_be_with_reload(:merge_request_a) do create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label]) end @@ -96,7 +96,7 @@ RSpec.describe 'getting merge request listings nested in a project' do where(:field, :subfield, :is_connection) do nested_fields_of('MergeRequest').flat_map do |name, field| type = field_type(field) - is_connection = type.name.ends_with?('Connection') + is_connection = type.graphql_name.ends_with?('Connection') type = field_type(type.fields['nodes']) if is_connection type.fields @@ -412,6 +412,10 @@ RSpec.describe 'getting merge request listings nested in a project' do describe 'sorting and pagination' do let(:data_path) { [:project, :mergeRequests] } + def pagination_results_data(nodes) + nodes + end + def pagination_query(params) graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY) mergeRequests(#{params}) { @@ -429,7 +433,7 @@ RSpec.describe 'getting merge request listings nested in a project' do merge_request_c, merge_request_e, merge_request_a - ].map { |mr| global_id_of(mr) } + ].map { |mr| a_graphql_entity_for(mr) } end before do @@ -455,7 +459,7 @@ RSpec.describe 'getting merge request listings nested in a project' do query = pagination_query(params) post_graphql(query, current_user: current_user) - expect(results.map { |item| item["id"] }).to eq(all_records.last(2)) + expect(results).to match(all_records.last(2)) end end end @@ -469,7 +473,7 @@ RSpec.describe 'getting merge request listings nested in a project' do merge_request_c, merge_request_e, merge_request_a - ].map { |mr| global_id_of(mr) } + ].map { |mr| a_graphql_entity_for(mr) } end before do @@ -495,17 +499,19 @@ RSpec.describe 'getting merge request listings nested in a project' do query = pagination_query(params) post_graphql(query, current_user: current_user) - expect(results.map { |item| item["id"] }).to eq(all_records.last(2)) + expect(results).to match(all_records.last(2)) end end end end context 'when only the count is requested' do + let_it_be(:merged_at) { Time.new(2020, 1, 3) } + context 'when merged at filter is present' do let_it_be(:merge_request) do create(:merge_request, :unique_branches, source_project: project).tap do |mr| - mr.metrics.update!(merged_at: Time.new(2020, 1, 3)) + mr.metrics.update!(merged_at: merged_at, created_at: merged_at - 2.days) end end @@ -522,12 +528,18 @@ RSpec.describe 'getting merge request listings nested in a project' do it 'does not query the merge requests table for the count' do query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - queries = query_recorder.data.each_value.first[:occurrences] + queries = query_recorder.log expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) end context 'when total_time_to_merge and count is queried' do + let_it_be(:merge_request_2) do + create(:merge_request, :unique_branches, source_project: project).tap do |mr| + mr.metrics.update!(merged_at: merged_at, created_at: merged_at - 1.day) + end + end + let(:query) do graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY) mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0) { @@ -537,11 +549,18 @@ RSpec.describe 'getting merge request listings nested in a project' do QUERY end - it 'does not query the merge requests table for the total_time_to_merge' do + it 'uses the merge_request_metrics table for total_time_to_merge' do query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - queries = query_recorder.data.each_value.first[:occurrences] - expect(queries).to include(match(/SELECT.+SUM.+FROM "merge_request_metrics" WHERE/)) + expect(query_recorder.log).to include(match(/SELECT.+SUM.+FROM "merge_request_metrics" WHERE/)) + end + + it 'returns the correct total time to merge' do + post_graphql(query, current_user: current_user) + + sum = graphql_data_at(:project, :merge_requests, :total_time_to_merge) + + expect(sum).to eq(3.days.to_f) end end diff --git a/spec/requests/api/graphql/project/milestones_spec.rb b/spec/requests/api/graphql/project/milestones_spec.rb index 2fede4c7285..3e8948d83b1 100644 --- a/spec/requests/api/graphql/project/milestones_spec.rb +++ b/spec/requests/api/graphql/project/milestones_spec.rb @@ -33,7 +33,7 @@ RSpec.describe 'getting milestone listings nested in a project' do def result_list(expected) expected.map do |milestone| - a_hash_including('id' => global_id_of(milestone)) + a_graphql_entity_for(milestone) end end diff --git a/spec/requests/api/graphql/project/pipeline_spec.rb b/spec/requests/api/graphql/project/pipeline_spec.rb index 73e02e2a4b1..ccf97918021 100644 --- a/spec/requests/api/graphql/project/pipeline_spec.rb +++ b/spec/requests/api/graphql/project/pipeline_spec.rb @@ -89,17 +89,16 @@ RSpec.describe 'getting pipeline information nested in a project' do post_graphql(query, current_user: current_user) expect(graphql_data_at(*path, :jobs, :nodes)).to contain_exactly( - a_hash_including( - 'name' => build_job.name, - 'status' => build_job.status.upcase, - 'duration' => build_job.duration + a_graphql_entity_for( + build_job, :name, :duration, + 'status' => build_job.status.upcase ), - a_hash_including( - 'id' => global_id_of(failed_build), + a_graphql_entity_for( + failed_build, 'status' => failed_build.status.upcase ), - a_hash_including( - 'id' => global_id_of(bridge), + a_graphql_entity_for( + bridge, 'status' => bridge.status.upcase ) ) @@ -135,7 +134,7 @@ RSpec.describe 'getting pipeline information nested in a project' do post_graphql(query, current_user: current_user, variables: variables) expect(graphql_data_at(*path, :jobs, :nodes)) - .to contain_exactly(a_hash_including('id' => global_id_of(failed_build))) + .to contain_exactly(a_graphql_entity_for(failed_build)) end end @@ -166,7 +165,7 @@ RSpec.describe 'getting pipeline information nested in a project' do end let(:the_job) do - a_hash_including('name' => build_job.name, 'id' => global_id_of(build_job)) + a_graphql_entity_for(build_job, :name) end it 'can request a build by name' do diff --git a/spec/requests/api/graphql/project/project_members_spec.rb b/spec/requests/api/graphql/project/project_members_spec.rb index 315d44884ff..c3281b44954 100644 --- a/spec/requests/api/graphql/project/project_members_spec.rb +++ b/spec/requests/api/graphql/project/project_members_spec.rb @@ -60,7 +60,10 @@ RSpec.describe 'getting project members information' do fetch_members(project: parent_project, args: { relations: [:DIRECT] }) expect(graphql_errors).to be_nil - expect(graphql_data_at(:project, :project_members, :edges, :node)).to contain_exactly({ 'user' => { 'id' => global_id_of(user) } }, 'user' => nil) + expect(graphql_data_at(:project, :project_members, :edges, :node)).to contain_exactly( + a_graphql_entity_for(user: a_graphql_entity_for(user)), + { 'user' => nil } + ) end end @@ -238,7 +241,7 @@ RSpec.describe 'getting project members information' do def expect_array_response(*items) expect(response).to have_gitlab_http_status(:success) - member_gids = graphql_data_at(:project, :project_members, :edges, :node, :user, :id) - expect(member_gids).to match_array(items.map { |u| global_id_of(u) }) + members = graphql_data_at(:project, :project_members, :edges, :node, :user) + expect(members).to match_array(items.map { |u| a_graphql_entity_for(u) }) end end diff --git a/spec/requests/api/graphql/project/release_spec.rb b/spec/requests/api/graphql/project/release_spec.rb index 77abac4ef04..c4899dbb71e 100644 --- a/spec/requests/api/graphql/project/release_spec.rb +++ b/spec/requests/api/graphql/project/release_spec.rb @@ -77,10 +77,10 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do post_query expected = release.milestones.order_by_dates_and_title.map do |milestone| - { 'id' => global_id_of(milestone), 'title' => milestone.title } + a_graphql_entity_for(milestone, :title) end - expect(data).to eq(expected) + expect(data).to match(expected) end end @@ -94,10 +94,7 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do it 'finds the author of the release' do post_query - expect(data).to eq( - 'id' => global_id_of(release.author), - 'username' => release.author.username - ) + expect(data).to match a_graphql_entity_for(release.author, :username) end end @@ -142,13 +139,11 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do post_query expected = release.links.map do |link| - { - 'id' => global_id_of(link), - 'name' => link.name, - 'url' => link.url, + a_graphql_entity_for( + link, :name, :url, 'external' => link.external?, 'directAssetUrl' => link.filepath ? Gitlab::Routing.url_helpers.project_release_url(project, release) << "/downloads#{link.filepath}" : link.url - } + ) end expect(data).to match_array(expected) @@ -218,10 +213,8 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do evidence = release.evidences.first.present - expect(data["nodes"].first).to eq( - 'id' => global_id_of(evidence), - 'sha' => evidence.sha, - 'filepath' => evidence.filepath, + expect(data["nodes"].first).to match a_graphql_entity_for( + evidence, :sha, :filepath, 'collectedAt' => evidence.collected_at.utc.iso8601 ) end @@ -274,10 +267,10 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do post_query expected = release.milestones.order_by_dates_and_title.map do |milestone| - { 'id' => global_id_of(milestone), 'title' => milestone.title } + a_graphql_entity_for(milestone, :title) end - expect(data).to eq(expected) + expect(data).to match(expected) end end @@ -291,10 +284,7 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do it 'finds the author of the release' do post_query - expect(data).to eq( - 'id' => global_id_of(release.author), - 'username' => release.author.username - ) + expect(data).to match a_graphql_entity_for(release.author, :username) end end @@ -339,13 +329,11 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do post_query expected = release.links.map do |link| - { - 'id' => global_id_of(link), - 'name' => link.name, - 'url' => link.url, + a_graphql_entity_for( + link, :name, :url, 'external' => true, 'directAssetUrl' => link.filepath ? Gitlab::Routing.url_helpers.project_release_url(project, release) << "/downloads#{link.filepath}" : link.url - } + ) end expect(data).to match_array(expected) diff --git a/spec/requests/api/graphql/project/terraform/state_spec.rb b/spec/requests/api/graphql/project/terraform/state_spec.rb index 9f1d9ab204a..8f2d2cffef2 100644 --- a/spec/requests/api/graphql/project/terraform/state_spec.rb +++ b/spec/requests/api/graphql/project/terraform/state_spec.rb @@ -57,22 +57,22 @@ RSpec.describe 'query a single terraform state' do it_behaves_like 'a working graphql query' it 'returns terraform state data' do - expect(data).to match(a_hash_including({ - 'id' => global_id_of(terraform_state), - 'name' => terraform_state.name, + expect(data).to match a_graphql_entity_for( + terraform_state, + :name, 'lockedAt' => terraform_state.locked_at.iso8601, 'createdAt' => terraform_state.created_at.iso8601, 'updatedAt' => terraform_state.updated_at.iso8601, - 'lockedByUser' => { 'id' => global_id_of(terraform_state.locked_by_user) }, - 'latestVersion' => { - 'id' => eq(global_id_of(latest_version)), + 'lockedByUser' => a_graphql_entity_for(terraform_state.locked_by_user), + 'latestVersion' => a_graphql_entity_for( + latest_version, 'serial' => eq(latest_version.version), 'createdAt' => eq(latest_version.created_at.iso8601), 'updatedAt' => eq(latest_version.updated_at.iso8601), - 'createdByUser' => { 'id' => eq(global_id_of(latest_version.created_by_user)) }, + 'createdByUser' => a_graphql_entity_for(latest_version.created_by_user), 'job' => { 'name' => eq(latest_version.build.name) } - } - })) + ) + ) end context 'unauthorized users' do diff --git a/spec/requests/api/graphql/project/terraform/states_spec.rb b/spec/requests/api/graphql/project/terraform/states_spec.rb index 2879530acc5..a7ec6f69776 100644 --- a/spec/requests/api/graphql/project/terraform/states_spec.rb +++ b/spec/requests/api/graphql/project/terraform/states_spec.rb @@ -62,23 +62,22 @@ RSpec.describe 'query terraform states' do ) ) - expect(data['nodes']).to contain_exactly({ - 'id' => global_id_of(terraform_state), - 'name' => terraform_state.name, + expect(data['nodes']).to contain_exactly a_graphql_entity_for( + terraform_state, :name, 'lockedAt' => terraform_state.locked_at.iso8601, 'createdAt' => terraform_state.created_at.iso8601, 'updatedAt' => terraform_state.updated_at.iso8601, - 'lockedByUser' => { 'id' => global_id_of(terraform_state.locked_by_user) }, - 'latestVersion' => { - 'id' => eq(global_id_of(latest_version)), + 'lockedByUser' => a_graphql_entity_for(terraform_state.locked_by_user), + 'latestVersion' => a_graphql_entity_for( + latest_version, 'serial' => eq(latest_version.version), 'downloadPath' => eq(download_path), 'createdAt' => eq(latest_version.created_at.iso8601), 'updatedAt' => eq(latest_version.updated_at.iso8601), - 'createdByUser' => { 'id' => eq(global_id_of(latest_version.created_by_user)) }, + 'createdByUser' => a_graphql_entity_for(latest_version.created_by_user), 'job' => { 'name' => eq(latest_version.build.name) } - } - }) + ) + ) end it 'returns count of terraform states' do diff --git a/spec/requests/api/graphql/query_spec.rb b/spec/requests/api/graphql/query_spec.rb index d650acc8354..4aa9c4b8254 100644 --- a/spec/requests/api/graphql/query_spec.rb +++ b/spec/requests/api/graphql/query_spec.rb @@ -76,10 +76,8 @@ RSpec.describe 'Query' do it_behaves_like 'a working graphql query' it_behaves_like 'a query that needs authorization' - context 'the current user is able to read designs' do - it 'fetches the expected data' do - expect(query_result).to eq('id' => global_id_of(version), 'sha' => version.sha) - end + it 'fetches the expected data' do + expect(query_result).to match a_graphql_entity_for(version, :sha) end end @@ -106,13 +104,13 @@ RSpec.describe 'Query' do context 'the current user is able to read designs' do it 'fetches the expected data, including the correct associations' do - expect(query_result).to eq( - 'id' => global_id_of(design_at_version), + expect(query_result).to match a_graphql_entity_for( + design_at_version, 'filename' => design_at_version.design.filename, - 'version' => { 'id' => global_id_of(version), 'sha' => version.sha }, - 'design' => { 'id' => global_id_of(design) }, + 'version' => a_graphql_entity_for(version, :sha), + 'design' => a_graphql_entity_for(design), 'issue' => { 'title' => issue.title, 'iid' => issue.iid.to_s }, - 'project' => { 'id' => global_id_of(project), 'fullPath' => project.full_path } + 'project' => a_graphql_entity_for(project, :full_path) ) end end diff --git a/spec/requests/api/graphql/user/starred_projects_query_spec.rb b/spec/requests/api/graphql/user/starred_projects_query_spec.rb index a8c087d1fbf..37a85b98e5f 100644 --- a/spec/requests/api/graphql/user/starred_projects_query_spec.rb +++ b/spec/requests/api/graphql/user/starred_projects_query_spec.rb @@ -42,7 +42,7 @@ RSpec.describe 'Getting starredProjects of the user' do it 'found only public project' do expect(starred_projects).to contain_exactly( - a_hash_including('id' => global_id_of(project_a)) + a_graphql_entity_for(project_a) ) end @@ -51,9 +51,9 @@ RSpec.describe 'Getting starredProjects of the user' do it 'found all projects' do expect(starred_projects).to contain_exactly( - a_hash_including('id' => global_id_of(project_a)), - a_hash_including('id' => global_id_of(project_b)), - a_hash_including('id' => global_id_of(project_c)) + a_graphql_entity_for(project_a), + a_graphql_entity_for(project_b), + a_graphql_entity_for(project_c) ) end end @@ -69,8 +69,8 @@ RSpec.describe 'Getting starredProjects of the user' do it 'finds public and member projects' do expect(starred_projects).to contain_exactly( - a_hash_including('id' => global_id_of(project_a)), - a_hash_including('id' => global_id_of(project_b)) + a_graphql_entity_for(project_a), + a_graphql_entity_for(project_b) ) end end @@ -93,9 +93,9 @@ RSpec.describe 'Getting starredProjects of the user' do it 'finds all projects starred by the user, which the current user has access to' do expect(starred_projects).to contain_exactly( - a_hash_including('id' => global_id_of(project_a)), - a_hash_including('id' => global_id_of(project_b)), - a_hash_including('id' => global_id_of(project_c)) + a_graphql_entity_for(project_a), + a_graphql_entity_for(project_b), + a_graphql_entity_for(project_c) ) end end diff --git a/spec/requests/api/graphql/user_query_spec.rb b/spec/requests/api/graphql/user_query_spec.rb index 1cba3674d25..8f286180617 100644 --- a/spec/requests/api/graphql/user_query_spec.rb +++ b/spec/requests/api/graphql/user_query_spec.rb @@ -91,11 +91,11 @@ RSpec.describe 'getting user information' do presenter = UserPresenter.new(user) expect(graphql_data['user']).to match( - a_hash_including( - 'id' => global_id_of(user), + a_graphql_entity_for( + user, + :username, 'state' => presenter.state, 'name' => presenter.name, - 'username' => presenter.username, 'webUrl' => presenter.web_url, 'avatarUrl' => presenter.avatar_url, 'email' => presenter.public_email, @@ -121,9 +121,9 @@ RSpec.describe 'getting user information' do it 'can be found' do expect(assigned_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(assigned_mr)), - a_hash_including('id' => global_id_of(assigned_mr_b)), - a_hash_including('id' => global_id_of(assigned_mr_c)) + a_graphql_entity_for(assigned_mr), + a_graphql_entity_for(assigned_mr_b), + a_graphql_entity_for(assigned_mr_c) ) end @@ -145,7 +145,7 @@ RSpec.describe 'getting user information' do it 'selects the correct MRs' do expect(assigned_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(assigned_mr_b)) + a_graphql_entity_for(assigned_mr_b) ) end end @@ -157,8 +157,8 @@ RSpec.describe 'getting user information' do it 'selects the correct MRs' do expect(assigned_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(assigned_mr_b)), - a_hash_including('id' => global_id_of(assigned_mr_c)) + a_graphql_entity_for(assigned_mr_b), + a_graphql_entity_for(assigned_mr_c) ) end end @@ -169,7 +169,7 @@ RSpec.describe 'getting user information' do it 'finds the authored mrs' do expect(assigned_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(assigned_mr_b)) + a_graphql_entity_for(assigned_mr_b) ) end end @@ -185,8 +185,8 @@ RSpec.describe 'getting user information' do post_graphql(query, current_user: current_user) expect(assigned_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(assigned_mr_b)), - a_hash_including('id' => global_id_of(assigned_mr_c)) + a_graphql_entity_for(assigned_mr_b), + a_graphql_entity_for(assigned_mr_c) ) end end @@ -212,9 +212,9 @@ RSpec.describe 'getting user information' do it 'can be found' do expect(reviewed_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(reviewed_mr)), - a_hash_including('id' => global_id_of(reviewed_mr_b)), - a_hash_including('id' => global_id_of(reviewed_mr_c)) + a_graphql_entity_for(reviewed_mr), + a_graphql_entity_for(reviewed_mr_b), + a_graphql_entity_for(reviewed_mr_c) ) end @@ -236,7 +236,7 @@ RSpec.describe 'getting user information' do it 'selects the correct MRs' do expect(reviewed_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(reviewed_mr_b)) + a_graphql_entity_for(reviewed_mr_b) ) end end @@ -248,8 +248,8 @@ RSpec.describe 'getting user information' do it 'selects the correct MRs' do expect(reviewed_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(reviewed_mr_b)), - a_hash_including('id' => global_id_of(reviewed_mr_c)) + a_graphql_entity_for(reviewed_mr_b), + a_graphql_entity_for(reviewed_mr_c) ) end end @@ -260,7 +260,7 @@ RSpec.describe 'getting user information' do it 'finds the authored mrs' do expect(reviewed_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(reviewed_mr_b)) + a_graphql_entity_for(reviewed_mr_b) ) end end @@ -275,7 +275,7 @@ RSpec.describe 'getting user information' do post_graphql(query, current_user: current_user) expect(reviewed_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(reviewed_mr_c)) + a_graphql_entity_for(reviewed_mr_c) ) end end @@ -301,9 +301,9 @@ RSpec.describe 'getting user information' do it 'can be found' do expect(authored_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(authored_mr)), - a_hash_including('id' => global_id_of(authored_mr_b)), - a_hash_including('id' => global_id_of(authored_mr_c)) + a_graphql_entity_for(authored_mr), + a_graphql_entity_for(authored_mr_b), + a_graphql_entity_for(authored_mr_c) ) end @@ -329,8 +329,8 @@ RSpec.describe 'getting user information' do post_graphql(query, current_user: current_user) expect(authored_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(authored_mr)), - a_hash_including('id' => global_id_of(authored_mr_c)) + a_graphql_entity_for(authored_mr), + a_graphql_entity_for(authored_mr_c) ) end end @@ -346,8 +346,8 @@ RSpec.describe 'getting user information' do post_graphql(query, current_user: current_user) expect(authored_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(authored_mr_b)), - a_hash_including('id' => global_id_of(authored_mr_c)) + a_graphql_entity_for(authored_mr_b), + a_graphql_entity_for(authored_mr_c) ) end end @@ -359,7 +359,7 @@ RSpec.describe 'getting user information' do it 'selects the correct MRs' do expect(authored_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(authored_mr_b)) + a_graphql_entity_for(authored_mr_b) ) end end @@ -371,8 +371,8 @@ RSpec.describe 'getting user information' do it 'selects the correct MRs' do expect(authored_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(authored_mr_b)), - a_hash_including('id' => global_id_of(authored_mr_c)) + a_graphql_entity_for(authored_mr_b), + a_graphql_entity_for(authored_mr_c) ) end end @@ -417,7 +417,7 @@ RSpec.describe 'getting user information' do it 'can be found' do expect(group_memberships).to include( - a_hash_including('id' => global_id_of(membership_a)) + a_graphql_entity_for(membership_a) ) end end @@ -440,7 +440,7 @@ RSpec.describe 'getting user information' do it 'can be found' do expect(project_memberships).to include( - a_hash_including('id' => global_id_of(membership_a)) + a_graphql_entity_for(membership_a) ) end end @@ -460,7 +460,7 @@ RSpec.describe 'getting user information' do it 'can be found' do expect(authored_mrs).to include( - a_hash_including('id' => global_id_of(authored_mr)) + a_graphql_entity_for(authored_mr) ) end end @@ -480,9 +480,9 @@ RSpec.describe 'getting user information' do it 'can be found' do expect(assigned_mrs).to contain_exactly( - a_hash_including('id' => global_id_of(assigned_mr)), - a_hash_including('id' => global_id_of(assigned_mr_b)), - a_hash_including('id' => global_id_of(assigned_mr_c)) + a_graphql_entity_for(assigned_mr), + a_graphql_entity_for(assigned_mr_b), + a_graphql_entity_for(assigned_mr_c) ) end end diff --git a/spec/requests/api/graphql/users_spec.rb b/spec/requests/api/graphql/users_spec.rb index fe824834a2c..79ee3c2cb57 100644 --- a/spec/requests/api/graphql/users_spec.rb +++ b/spec/requests/api/graphql/users_spec.rb @@ -72,12 +72,12 @@ RSpec.describe 'Users' do post_query expect(graphql_data.dig('users', 'nodes')).to include( - { "id" => user0.to_global_id.to_s }, - { "id" => user1.to_global_id.to_s }, - { "id" => user2.to_global_id.to_s }, - { "id" => user3.to_global_id.to_s }, - { "id" => admin.to_global_id.to_s }, - { "id" => another_admin.to_global_id.to_s } + a_graphql_entity_for(user0), + a_graphql_entity_for(user1), + a_graphql_entity_for(user2), + a_graphql_entity_for(user3), + a_graphql_entity_for(admin), + a_graphql_entity_for(another_admin) ) end end @@ -91,15 +91,15 @@ RSpec.describe 'Users' do post_graphql(query, current_user: current_user) expect(graphql_data.dig('users', 'nodes')).to include( - { "id" => another_admin.to_global_id.to_s }, - { "id" => admin.to_global_id.to_s } + a_graphql_entity_for(another_admin), + a_graphql_entity_for(admin) ) expect(graphql_data.dig('users', 'nodes')).not_to include( - { "id" => user0.to_global_id.to_s }, - { "id" => user1.to_global_id.to_s }, - { "id" => user2.to_global_id.to_s }, - { "id" => user3.to_global_id.to_s } + a_graphql_entity_for(user0), + a_graphql_entity_for(user1), + a_graphql_entity_for(user2), + a_graphql_entity_for(user3) ) end end @@ -114,7 +114,7 @@ RSpec.describe 'Users' do end context 'when sorting by created_at' do - let_it_be(:ascending_users) { [user3, user2, user1, user0].map { |u| global_id_of(u) } } + let_it_be(:ascending_users) { [user3, user2, user1, user0].map { |u| global_id_of(u).to_s } } context 'when ascending' do it_behaves_like 'sorted paginated query' do diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index bc5a8b3e006..5b34c21989a 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -33,7 +33,8 @@ RSpec.describe 'Query.work_item(id)' do 'lockVersion' => work_item.lock_version, 'state' => "OPEN", 'title' => work_item.title, - 'workItemType' => hash_including('id' => work_item.work_item_type.to_gid.to_s) + 'workItemType' => hash_including('id' => work_item.work_item_type.to_gid.to_s), + 'userPermissions' => { 'readWorkItem' => true, 'updateWorkItem' => true, 'deleteWorkItem' => false } ) end diff --git a/spec/requests/api/group_container_repositories_spec.rb b/spec/requests/api/group_container_repositories_spec.rb index bf29bd91414..413c37eaed9 100644 --- a/spec/requests/api/group_container_repositories_spec.rb +++ b/spec/requests/api/group_container_repositories_spec.rb @@ -37,13 +37,10 @@ RSpec.describe API::GroupContainerRepositories do let(:url) { "/groups/#{group.id}/registry/repositories" } let(:snowplow_gitlab_standard_context) { { user: api_user, namespace: group } } - subject { get api(url, api_user), params: params } + subject { get api(url, api_user) } it_behaves_like 'rejected container repository access', :guest, :forbidden it_behaves_like 'rejected container repository access', :anonymous, :not_found - it_behaves_like 'handling network errors with the container registry' do - let(:params) { { tags: true } } - end it_behaves_like 'returns repositories for allowed users', :reporter, 'group' do let(:object) { group } diff --git a/spec/requests/api/group_milestones_spec.rb b/spec/requests/api/group_milestones_spec.rb index 2312d35c815..da84e98b905 100644 --- a/spec/requests/api/group_milestones_spec.rb +++ b/spec/requests/api/group_milestones_spec.rb @@ -85,7 +85,7 @@ RSpec.describe API::GroupMilestones do def setup_for_group context_group.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - context_group.add_developer(user) + context_group.add_reporter(user) public_project.update!(namespace: context_group) context_group.reload end diff --git a/spec/requests/api/import_bitbucket_server_spec.rb b/spec/requests/api/import_bitbucket_server_spec.rb index 970416c7444..8ab41f49549 100644 --- a/spec/requests/api/import_bitbucket_server_spec.rb +++ b/spec/requests/api/import_bitbucket_server_spec.rb @@ -9,7 +9,15 @@ RSpec.describe API::ImportBitbucketServer do let(:secret) { "sekrettt" } let(:project_key) { 'TES' } let(:repo_slug) { 'vim' } - let(:repo) { { name: 'vim' } } + let(:repo) do + double('repo', + name: repo_slug, + browse_url: "#{base_uri}/projects/#{project_key}/repos/#{repo_slug}/browse", + clone_url: "#{base_uri}/scm/#{project_key}/#{repo_slug}.git", + description: 'provider', + visibility_level: Gitlab::VisibilityLevel::PUBLIC + ) + end describe "POST /import/bitbucket_server" do context 'with no optional parameters' do @@ -20,7 +28,7 @@ RSpec.describe API::ImportBitbucketServer do before do Grape::Endpoint.before_each do |endpoint| allow(endpoint).to receive(:client).and_return(client.as_null_object) - allow(client).to receive(:repo).with(project_key, repo_slug).and_return(double(name: repo_slug)) + allow(client).to receive(:repo).with(project_key, repo_slug).and_return(repo) end end diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb index f0c4fcc4f29..7de72de3940 100644 --- a/spec/requests/api/import_github_spec.rb +++ b/spec/requests/api/import_github_spec.rb @@ -16,7 +16,11 @@ RSpec.describe API::ImportGithub do double('provider', name: 'vim', full_name: "#{provider_username}/vim", - owner: double('provider', login: provider_username) + owner: double('provider', login: provider_username), + description: 'provider', + private: false, + clone_url: 'https://fake.url/vim.git', + has_wiki?: true ) end diff --git a/spec/requests/api/integrations/jira_connect/subscriptions_spec.rb b/spec/requests/api/integrations/jira_connect/subscriptions_spec.rb new file mode 100644 index 00000000000..86f8992a624 --- /dev/null +++ b/spec/requests/api/integrations/jira_connect/subscriptions_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Integrations::JiraConnect::Subscriptions do + describe 'POST /integrations/jira_connect/subscriptions' do + subject(:post_subscriptions) { post api('/integrations/jira_connect/subscriptions') } + + it 'returns 401' do + post_subscriptions + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + context 'with user token' do + let(:group) { create(:group) } + let(:user) { create(:user) } + + subject(:post_subscriptions) do + post api('/integrations/jira_connect/subscriptions', user), params: { jwt: jwt, namespace_path: group.path } + end + + context 'with feature flag disabled' do + before do + stub_feature_flags(jira_connect_oauth: false) + end + + let(:jwt) { '123' } + + it 'returns 404' do + post_subscriptions + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with invalid JWT' do + let(:jwt) { '123' } + + it 'returns 401' do + post_subscriptions + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'with valid JWT' do + let_it_be(:installation) { create(:jira_connect_installation) } + let_it_be(:user) { create(:user) } + + let(:claims) { { iss: installation.client_key, qsh: 'context-qsh', sub: 1234 } } + let(:jwt) { Atlassian::Jwt.encode(claims, installation.shared_secret) } + let(:jira_user) { { 'groups' => { 'items' => [{ 'name' => jira_group_name }] } } } + let(:jira_group_name) { 'site-admins' } + + before do + WebMock + .stub_request(:get, "#{installation.base_url}/rest/api/3/user?accountId=1234&expand=groups") + .to_return(body: jira_user.to_json, status: 200, headers: { 'Content-Type' => 'application/json' }) + end + + it 'returns 401 if the user does not have access to the group' do + post_subscriptions + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + context 'user has access to the group' do + before do + group.add_maintainer(user) + end + + it 'creates a subscription' do + expect { post_subscriptions }.to change { installation.subscriptions.count }.from(0).to(1) + end + + it 'returns 201' do + post_subscriptions + + expect(response).to have_gitlab_http_status(:created) + end + end + end + end + end +end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 2b7963eadab..acfe476a864 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -612,30 +612,6 @@ RSpec.describe API::Internal::Base do expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'false') end end - - context "with a sidechannels enabled for a project" do - before do - stub_feature_flags(gitlab_shell_upload_pack_sidechannel: project) - end - - it "has the use_sidechannel field set to true for that project" do - pull(key, project) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response["gl_repository"]).to eq("project-#{project.id}") - expect(json_response["gitaly"]["use_sidechannel"]).to eq(true) - end - - it "has the use_sidechannel field set to false for other projects" do - other_project = create(:project, :public, :repository) - - pull(key, other_project) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response["gl_repository"]).to eq("project-#{other_project.id}") - expect(json_response["gitaly"]["use_sidechannel"]).to eq(false) - end - end end context "git push" do @@ -826,13 +802,13 @@ RSpec.describe API::Internal::Base do context 'git pull' do context 'with a key that has expired' do - let(:key) { create(:key, user: user, expires_at: 2.days.ago) } + let(:key) { create(:key, :expired, user: user) } - it 'includes the `key expired` message in the response' do + it 'includes the `key expired` message in the response and fails' do pull(key, project) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['gl_console_messages']).to eq(['INFO: Your SSH key has expired. Please generate a new key.']) + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['message']).to eq('Your SSH key has expired.') end end @@ -1490,6 +1466,89 @@ RSpec.describe API::Internal::Base do subject expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq 'Feature is not available' + end + end + + describe 'POST /internal/two_factor_manual_otp_check' do + let(:key_id) { key.id } + let(:otp) { '123456'} + + subject do + post api('/internal/two_factor_manual_otp_check'), + params: { + secret_token: secret_token, + key_id: key_id, + otp_attempt: otp + } + end + + it 'is not available' do + subject + + expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq 'Feature is not available' + end + end + + describe 'POST /internal/two_factor_push_otp_check' do + let(:key_id) { key.id } + let(:otp) { '123456'} + + subject do + post api('/internal/two_factor_push_otp_check'), + params: { + secret_token: secret_token, + key_id: key_id, + otp_attempt: otp + } + end + + it 'is not available' do + subject + + expect(json_response['success']).to be_falsey + expect(json_response['message']).to eq 'Feature is not available' + end + end + + describe 'POST /internal/two_factor_manual_otp_check' do + let(:key_id) { key.id } + let(:otp) { '123456'} + + subject do + post api('/internal/two_factor_manual_otp_check'), + params: { + secret_token: secret_token, + key_id: key_id, + otp_attempt: otp + } + end + + it 'is not available' do + subject + + expect(json_response['success']).to be_falsey + end + end + + describe 'POST /internal/two_factor_push_otp_check' do + let(:key_id) { key.id } + let(:otp) { '123456'} + + subject do + post api('/internal/two_factor_push_otp_check'), + params: { + secret_token: secret_token, + key_id: key_id, + otp_attempt: otp + } + end + + it 'is not available' do + subject + + expect(json_response['success']).to be_falsey end end diff --git a/spec/requests/api/internal/container_registry/migration_spec.rb b/spec/requests/api/internal/container_registry/migration_spec.rb index 35113c66f11..db2918e65f1 100644 --- a/spec/requests/api/internal/container_registry/migration_spec.rb +++ b/spec/requests/api/internal/container_registry/migration_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Internal::ContainerRegistry::Migration do +RSpec.describe API::Internal::ContainerRegistry::Migration, :aggregate_failures do let_it_be_with_reload(:repository) { create(:container_repository) } let(:secret_token) { 'secret_token' } @@ -127,6 +127,12 @@ RSpec.describe API::Internal::ContainerRegistry::Migration do it_behaves_like 'updating the repository migration status', from: 'pre_importing', to: 'import_aborted' end + + context 'with repository in unabortable migration state' do + let(:repository) { create(:container_repository, :import_skipped) } + + it_behaves_like 'returning an error', with_message: 'Wrong migration state (import_skipped)' + end end end @@ -147,6 +153,17 @@ RSpec.describe API::Internal::ContainerRegistry::Migration do it_behaves_like 'returning an error', returning_status: :not_found end + + context 'query read location' do + it 'reads from the primary' do + expect(ContainerRepository).to receive(:find_by_path!).and_wrap_original do |m, *args| + expect(::Gitlab::Database::LoadBalancing::Session.current.use_primary?).to eq(true) + m.call(*args) + end + + subject + end + end end context 'with an invalid sent token' do diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index ef7f5ee87dc..5d8ed3dd0f5 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -123,6 +123,7 @@ RSpec.describe API::Lint do expect(json_response['status']).to eq('valid') expect(json_response['warnings']).to match_array([]) expect(json_response['errors']).to match_array([]) + expect(json_response['includes']).to eq([]) end it 'outputs expanded yaml content' do @@ -153,19 +154,6 @@ RSpec.describe API::Lint do end end - context 'with valid .gitlab-ci.yml using deprecated keywords' do - let(:yaml_content) { { job: { script: 'ls', type: 'test' }, types: ['test'] }.to_yaml } - - it 'passes validation but returns warnings' do - post api('/ci/lint', api_user), params: { content: yaml_content } - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['status']).to eq('valid') - expect(json_response['warnings']).not_to be_empty - expect(json_response['errors']).to match_array([]) - end - end - context 'with an invalid .gitlab-ci.yml' do context 'with invalid syntax' do let(:yaml_content) { 'invalid content' } @@ -177,6 +165,7 @@ RSpec.describe API::Lint do expect(json_response['status']).to eq('invalid') expect(json_response['warnings']).to eq([]) expect(json_response['errors']).to eq(['Invalid configuration format']) + expect(json_response['includes']).to eq(nil) end it 'outputs expanded yaml content' do @@ -204,6 +193,7 @@ RSpec.describe API::Lint do expect(json_response['status']).to eq('invalid') expect(json_response['warnings']).to eq([]) expect(json_response['errors']).to eq(['jobs config should contain at least one visible job']) + expect(json_response['includes']).to eq([]) end it 'outputs expanded yaml content' do @@ -262,6 +252,17 @@ RSpec.describe API::Lint do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Hash expect(json_response['merged_yaml']).to eq(expected_yaml) + expect(json_response['includes']).to contain_exactly( + { + 'type' => 'local', + 'location' => 'another-gitlab-ci.yml', + 'blob' => "http://localhost/#{project.full_path}/-/blob/#{project.commit.sha}/another-gitlab-ci.yml", + 'raw' => "http://localhost/#{project.full_path}/-/raw/#{project.commit.sha}/another-gitlab-ci.yml", + 'extra' => {}, + 'context_project' => project.full_path, + 'context_sha' => project.commit.sha + } + ) expect(json_response['valid']).to eq(true) expect(json_response['warnings']).to eq([]) expect(json_response['errors']).to eq([]) @@ -274,6 +275,7 @@ RSpec.describe API::Lint do expect(response).to have_gitlab_http_status(:ok) expect(json_response['merged_yaml']).to eq(yaml_content) + expect(json_response['includes']).to eq([]) expect(json_response['valid']).to eq(false) expect(json_response['warnings']).to eq([]) expect(json_response['errors']).to eq(['jobs config should contain at least one visible job']) @@ -327,6 +329,7 @@ RSpec.describe API::Lint do expect(response).to have_gitlab_http_status(:ok) expect(json_response['merged_yaml']).to eq(nil) + expect(json_response['includes']).to eq(nil) expect(json_response['valid']).to eq(false) expect(json_response['warnings']).to eq([]) expect(json_response['errors']).to eq(['Insufficient permissions to create a new pipeline']) @@ -539,6 +542,17 @@ RSpec.describe API::Lint do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Hash expect(json_response['merged_yaml']).to eq(expected_yaml) + expect(json_response['includes']).to contain_exactly( + { + 'type' => 'local', + 'location' => 'another-gitlab-ci.yml', + 'blob' => "http://localhost/#{project.full_path}/-/blob/#{project.commit.sha}/another-gitlab-ci.yml", + 'raw' => "http://localhost/#{project.full_path}/-/raw/#{project.commit.sha}/another-gitlab-ci.yml", + 'extra' => {}, + 'context_project' => project.full_path, + 'context_sha' => project.commit.sha + } + ) expect(json_response['valid']).to eq(true) expect(json_response['errors']).to eq([]) end @@ -550,6 +564,7 @@ RSpec.describe API::Lint do expect(response).to have_gitlab_http_status(:ok) expect(json_response['merged_yaml']).to eq(yaml_content) + expect(json_response['includes']).to eq([]) expect(json_response['valid']).to eq(false) expect(json_response['errors']).to eq(['jobs config should contain at least one visible job']) end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 6bacb3a59b2..0db42e7439c 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -543,7 +543,7 @@ RSpec.describe API::Members do end it 'returns 409 if member does not exist' do - put api("/#{source_type.pluralize}/#{source.id}/members/123", maintainer), + put api("/#{source_type.pluralize}/#{source.id}/members/#{non_existing_record_id}", maintainer), params: { access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:not_found) @@ -618,7 +618,7 @@ RSpec.describe API::Members do end it 'returns 404 if member does not exist' do - delete api("/#{source_type.pluralize}/#{source.id}/members/123", maintainer) + delete api("/#{source_type.pluralize}/#{source.id}/members/#{non_existing_record_id}", maintainer) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index b1183bb10fa..a7ede7f4150 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -27,10 +27,10 @@ RSpec.describe API::MergeRequests do shared_context 'with merge requests' do let_it_be(:milestone1) { create(:milestone, title: '0.9', project: project) } - let_it_be(:merge_request) { create(:merge_request, :simple, milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) } - let_it_be(:merge_request_closed) { create(:merge_request, state: "closed", milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) } - let_it_be(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignees: [user], source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds, merge_commit_sha: '9999999999999999999999999999999999999999') } - let_it_be(:merge_request_locked) { create(:merge_request, state: "locked", milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Locked test", created_at: base_time + 1.second) } + let_it_be(:merge_request) { create(:merge_request, :simple, milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time, updated_at: base_time + 3.hours) } + let_it_be(:merge_request_closed) { create(:merge_request, state: "closed", milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second, updated_at: base_time) } + let_it_be(:merge_request_locked) { create(:merge_request, state: "locked", milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, title: "Locked test", created_at: base_time + 1.second, updated_at: base_time + 2.hours) } + let_it_be(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignees: [user], source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds, updated_at: base_time + 1.hour, merge_commit_sha: '9999999999999999999999999999999999999999') } let_it_be(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } let_it_be(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") } end @@ -348,19 +348,14 @@ RSpec.describe API::MergeRequests do end context 'with ordering' do - before do - @mr_later = mr_with_later_created_and_updated_at_time - @mr_earlier = mr_with_earlier_created_and_updated_at_time - end - it 'returns an array of merge_requests in ascending order' do path = endpoint_path + '?sort=asc' get api(path, user) expect_paginated_array_response([ - merge_request_closed.id, merge_request_locked.id, - merge_request_merged.id, merge_request.id + merge_request.id, merge_request_closed.id, + merge_request_locked.id, merge_request_merged.id ]) response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) @@ -372,42 +367,28 @@ RSpec.describe API::MergeRequests do get api(path, user) expect_paginated_array_response([ - merge_request.id, merge_request_merged.id, - merge_request_locked.id, merge_request_closed.id + merge_request_merged.id, merge_request_locked.id, + merge_request_closed.id, merge_request.id ]) response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end context '2 merge requests with equal created_at' do - let!(:closed_mr2) do - create :merge_request, - state: 'closed', - milestone: milestone1, - author: user, - assignees: [user], - source_project: project, - target_project: project, - title: "Test", - created_at: @mr_earlier.created_at - end - it 'page breaks first page correctly' do - get api("#{endpoint_path}?sort=desc&per_page=4", user) + get api("#{endpoint_path}?sort=desc&per_page=2", user) response_ids = json_response.map { |merge_request| merge_request['id'] } - expect(response_ids).to include(closed_mr2.id) - expect(response_ids).not_to include(@mr_earlier.id) + expect(response_ids).to contain_exactly(merge_request_merged.id, merge_request_locked.id) end it 'page breaks second page correctly' do - get api("#{endpoint_path}?sort=desc&per_page=4&page=2", user) + get api("#{endpoint_path}?sort=desc&per_page=2&page=2", user) response_ids = json_response.map { |merge_request| merge_request['id'] } - expect(response_ids).not_to include(closed_mr2.id) - expect(response_ids).to include(@mr_earlier.id) + expect(response_ids).to contain_exactly(merge_request_closed.id, merge_request.id) end end @@ -430,8 +411,8 @@ RSpec.describe API::MergeRequests do get api(path, user) expect_paginated_array_response([ - merge_request_closed.id, merge_request_locked.id, - merge_request_merged.id, merge_request.id + merge_request.id, merge_request_closed.id, + merge_request_locked.id, merge_request_merged.id ]) response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) @@ -3379,20 +3360,4 @@ RSpec.describe API::MergeRequests do include_examples 'time tracking endpoints', 'merge_request' end - - def mr_with_later_created_and_updated_at_time - merge_request - merge_request.created_at += 1.hour - merge_request.updated_at += 30.minutes - merge_request.save! - merge_request - end - - def mr_with_earlier_created_and_updated_at_time - merge_request_closed - merge_request_closed.created_at -= 1.hour - merge_request_closed.updated_at -= 30.minutes - merge_request_closed.save! - merge_request_closed - end end diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb index 0ff2c46e693..01f69f0aae2 100644 --- a/spec/requests/api/personal_access_tokens_spec.rb +++ b/spec/requests/api/personal_access_tokens_spec.rb @@ -73,6 +73,54 @@ RSpec.describe API::PersonalAccessTokens do end end + describe 'DELETE /personal_access_tokens/self' do + let(:path) { '/personal_access_tokens/self' } + let(:token) { create(:personal_access_token, user: current_user) } + + subject { delete api(path, current_user, personal_access_token: token) } + + shared_examples 'revoking token succeeds' do + it 'revokes token' do + subject + + expect(response).to have_gitlab_http_status(:no_content) + expect(token.reload).to be_revoked + end + end + + shared_examples 'revoking token denied' do |status| + it 'cannot revoke token' do + subject + + expect(response).to have_gitlab_http_status(status) + end + end + + context 'when current_user is an administrator', :enable_admin_mode do + let(:current_user) { create(:admin) } + + it_behaves_like 'revoking token succeeds' + end + + context 'when current_user is not an administrator' do + let(:current_user) { create(:user) } + + it_behaves_like 'revoking token succeeds' + + context 'with impersonated token' do + let(:token) { create(:personal_access_token, :impersonation, user: current_user) } + + it_behaves_like 'revoking token denied', :bad_request + end + + context 'with already revoked token' do + let(:token) { create(:personal_access_token, :revoked, user: current_user) } + + it_behaves_like 'revoking token denied', :unauthorized + end + end + end + describe 'DELETE /personal_access_tokens/:id' do let(:path) { "/personal_access_tokens/#{token1.id}" } diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index fbcaa404edb..eb6f81c2810 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -99,6 +99,7 @@ ci_cd_settings: default_git_depth: ci_default_git_depth forward_deployment_enabled: ci_forward_deployment_enabled job_token_scope_enabled: ci_job_token_scope_enabled + separated_caches: ci_separated_caches build_import_state: # import_state unexposed_attributes: diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index 196b0395ec0..506e60d19a6 100644 --- a/spec/requests/api/project_container_repositories_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -113,6 +113,10 @@ RSpec.describe API::ProjectContainerRepositories do it_behaves_like 'returns repositories for allowed users', :reporter, 'project' do let(:object) { project } end + + it_behaves_like 'returns tags for allowed users', :reporter, 'project' do + let(:object) { project } + end end end @@ -246,8 +250,7 @@ RSpec.describe API::ProjectContainerRepositories do name_regex_delete: 'v10.*', name_regex_keep: 'v10.1.*', keep_n: 100, - older_than: '1 day', - container_expiration_policy: false } + older_than: '1 day' } end let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" } @@ -293,8 +296,7 @@ RSpec.describe API::ProjectContainerRepositories do name_regex_delete: nil, name_regex_keep: 'v10.1.*', keep_n: 100, - older_than: '1 day', - container_expiration_policy: false } + older_than: '1 day' } end it 'schedules cleanup of tags repository' do diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index 07efd56fef4..8a8cd8512f8 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -410,6 +410,27 @@ RSpec.describe API::ProjectExport, :clean_gitlab_redis_cache do it_behaves_like 'post project export start' + context 'with project export size limit' do + before do + stub_application_setting(max_export_size: 1) + end + + it 'starts if limit not exceeded' do + post api(path, user) + + expect(response).to have_gitlab_http_status(:accepted) + end + + it '400 response if limit exceeded' do + project.statistics.update!(lfs_objects_size: 2.megabytes, repository_size: 2.megabytes) + + post api(path, user) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response["message"]).to include('The project size exceeds the export limit.') + end + end + context 'when rate limit is exceeded across projects' do before do allow(Gitlab::ApplicationRateLimiter) diff --git a/spec/requests/api/project_milestones_spec.rb b/spec/requests/api/project_milestones_spec.rb index 8c9a93cf9fa..b23fb86a9de 100644 --- a/spec/requests/api/project_milestones_spec.rb +++ b/spec/requests/api/project_milestones_spec.rb @@ -9,8 +9,8 @@ RSpec.describe API::ProjectMilestones do let_it_be(:milestone) { create(:milestone, project: project, title: 'version2', description: 'open milestone') } let_it_be(:route) { "/projects/#{project.id}/milestones" } - before do - project.add_developer(user) + before_all do + project.add_reporter(user) end it_behaves_like 'group and project milestones', "/projects/:id/milestones" diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 011300a038f..d2189ab02ea 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -739,6 +739,32 @@ RSpec.describe API::Projects do end end + context 'with default created_at desc order' do + let_it_be(:group_with_projects) { create(:group) } + let_it_be(:project_1) { create(:project, name: 'Project 1', created_at: 3.days.ago, path: 'project1', group: group_with_projects) } + let_it_be(:project_2) { create(:project, name: 'Project 2', created_at: 2.days.ago, path: 'project2', group: group_with_projects) } + let_it_be(:project_3) { create(:project, name: 'Project 3', created_at: 1.day.ago, path: 'project3', group: group_with_projects) } + + let(:current_user) { user } + let(:params) { {} } + + subject { get api('/projects', current_user), params: params } + + before do + group_with_projects.add_owner(current_user) if current_user + end + + it 'orders by id desc instead' do + projects_ordered_by_id_desc = /SELECT "projects".+ORDER BY "projects"."id" DESC/i + expect { subject }.to make_queries_matching projects_ordered_by_id_desc + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first['id']).to eq(project_3.id) + end + end + context 'sorting' do context 'by project statistics' do %w(repository_size storage_size wiki_size packages_size).each do |order_by| @@ -1180,9 +1206,15 @@ RSpec.describe API::Projects do end it 'disallows creating a project with an import_url when git import source is disabled' do + url = 'http://example.com' stub_application_setting(import_sources: nil) - project_params = { import_url: 'http://example.com', path: 'path-project-Foo', name: 'Foo Project' } + endpoint_url = "#{url}/info/refs?service=git-upload-pack" + stub_full_request(endpoint_url, method: :get).to_return({ status: 200, + body: '001e# service=git-upload-pack', + headers: { 'Content-Type': 'application/x-git-upload-pack-advertisement' } }) + + project_params = { import_url: url, path: 'path-project-Foo', name: 'Foo Project' } expect { post api('/projects', user), params: project_params } .not_to change { Project.count } @@ -2522,6 +2554,7 @@ RSpec.describe API::Projects do expect(links['labels']).to end_with("/api/v4/projects/#{project.id}/labels") expect(links['events']).to end_with("/api/v4/projects/#{project.id}/events") expect(links['members']).to end_with("/api/v4/projects/#{project.id}/members") + expect(links['cluster_agents']).to end_with("/api/v4/projects/#{project.id}/cluster_agents") end it 'filters related URIs when their feature is not enabled' do @@ -3556,6 +3589,20 @@ RSpec.describe API::Projects do expect(json_response['topics']).to eq(%w[topic2]) end + it 'updates enforce_auth_checks_on_uploads' do + project3.update!(enforce_auth_checks_on_uploads: false) + + project_param = { enforce_auth_checks_on_uploads: true } + + expect { put api("/projects/#{project3.id}", user), params: project_param } + .to change { project3.reload.enforce_auth_checks_on_uploads } + .from(false) + .to(true) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['enforce_auth_checks_on_uploads']).to eq(true) + end + it 'updates squash_option' do project3.update!(squash_option: 'always') @@ -4463,6 +4510,43 @@ RSpec.describe API::Projects do end end + describe 'POST /projects/:id/repository_size' do + let(:update_statistics_service) { Projects::UpdateStatisticsService.new(project, nil, statistics: [:repository_size, :lfs_objects_size]) } + + before do + allow(Projects::UpdateStatisticsService).to receive(:new).with(project, nil, statistics: [:repository_size, :lfs_objects_size]).and_return(update_statistics_service) + end + + context 'when authenticated as owner' do + it 'starts the housekeeping process' do + expect(update_statistics_service).to receive(:execute).once + + post api("/projects/#{project.id}/repository_size", user) + + expect(response).to have_gitlab_http_status(:created) + end + end + + context 'when authenticated as developer' do + before do + project_member + end + + it 'returns forbidden error' do + post api("/projects/#{project.id}/repository_size", user3) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when unauthenticated' do + it 'returns authentication error' do + post api("/projects/#{project.id}/repository_size") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end describe 'PUT /projects/:id/transfer' do context 'when authenticated as owner' do let(:group) { create :group } diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index c6bf72176a8..3c0f3a75f10 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -1399,14 +1399,6 @@ RSpec.describe API::Releases do expect(response).to have_gitlab_http_status(:not_found) end - - it 'returns not found unless :group_releases_finder_inoperator feature flag enabled' do - stub_feature_flags(group_releases_finder_inoperator: false) - - get api("/groups/#{group1.id}/releases", admin) - - expect(response).to have_gitlab_http_status(:not_found) - end end context 'when authenticated as guest' do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index c724c69045e..cfda06da8f3 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -54,6 +54,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['runner_token_expiration_interval']).to be_nil 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['pipeline_limit_per_project_user_sha']).to eq(0) end end @@ -138,6 +140,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do spam_check_api_key: 'SPAM_CHECK_API_KEY', mailgun_events_enabled: true, mailgun_signing_key: 'MAILGUN_SIGNING_KEY', + max_export_size: 6, disabled_oauth_sign_in_sources: 'unknown', import_sources: 'github,bitbucket', wiki_page_max_content_bytes: 12345, @@ -193,6 +196,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['spam_check_api_key']).to eq('SPAM_CHECK_API_KEY') 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['disabled_oauth_sign_in_sources']).to eq([]) expect(json_response['import_sources']).to match_array(%w(github bitbucket)) expect(json_response['wiki_page_max_content_bytes']).to eq(12345) @@ -736,5 +740,39 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do ) end end + + context 'with pipeline_limit_per_project_user_sha' do + it 'updates the settings' do + put api("/application/settings", admin), params: { + pipeline_limit_per_project_user_sha: 25 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'pipeline_limit_per_project_user_sha' => 25 + ) + end + + it 'updates the settings with zero value' do + put api("/application/settings", admin), params: { + pipeline_limit_per_project_user_sha: 0 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include( + 'pipeline_limit_per_project_user_sha' => 0 + ) + end + + it 'does not allow null values' do + put api("/application/settings", admin), params: { + pipeline_limit_per_project_user_sha: nil + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['pipeline_limit_per_project_user_sha']) + .to include(a_string_matching('is not a number')) + end + end end end diff --git a/spec/requests/api/sidekiq_metrics_spec.rb b/spec/requests/api/sidekiq_metrics_spec.rb index 23ac2ea5c0b..302d824e650 100644 --- a/spec/requests/api/sidekiq_metrics_spec.rb +++ b/spec/requests/api/sidekiq_metrics_spec.rb @@ -10,7 +10,18 @@ RSpec.describe API::SidekiqMetrics do get api('/sidekiq/queue_metrics', admin) expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_a Hash + expect(json_response).to match a_hash_including( + 'queues' => a_hash_including( + 'default' => { + 'backlog' => be_a(Integer), + 'latency' => be_a(Integer) + }, + 'mailers' => { + 'backlog' => be_a(Integer), + 'latency' => be_a(Integer) + } + ) + ) end it 'defines the `process_metrics` endpoint' do diff --git a/spec/requests/api/topics_spec.rb b/spec/requests/api/topics_spec.rb index 5c17ca9581e..e711414a895 100644 --- a/spec/requests/api/topics_spec.rb +++ b/spec/requests/api/topics_spec.rb @@ -117,7 +117,7 @@ RSpec.describe API::Topics do describe 'POST /topics', :aggregate_failures do context 'as administrator' do it 'creates a topic' do - post api('/topics/', admin), params: { name: 'my-topic' } + post api('/topics/', admin), params: { name: 'my-topic', title: 'My Topic' } expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq('my-topic') @@ -128,7 +128,7 @@ RSpec.describe API::Topics do workhorse_form_with_file( api('/topics/', admin), file_key: :avatar, - params: { name: 'my-topic', description: 'my description...', avatar: file } + params: { name: 'my-topic', title: 'My Topic', description: 'my description...', avatar: file } ) expect(response).to have_gitlab_http_status(:created) @@ -137,23 +137,30 @@ RSpec.describe API::Topics do end it 'returns 400 if name is missing' do - post api('/topics/', admin) + post api('/topics/', admin), params: { title: 'My Topic' } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']).to eql('name is missing') end it 'returns 400 if name is not unique (case insensitive)' do - post api('/topics/', admin), params: { name: topic_1.name.downcase } + post api('/topics/', admin), params: { name: topic_1.name.downcase, title: 'My Topic' } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']['name']).to eq(['has already been taken']) end + + it 'returns 400 if title is missing' do + post api('/topics/', admin), params: { name: 'my-topic' } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eql('title is missing') + end end context 'as normal user' do it 'returns 403 Forbidden' do - post api('/topics/', user), params: { name: 'my-topic' } + post api('/topics/', user), params: { name: 'my-topic', title: 'My Topic' } expect(response).to have_gitlab_http_status(:forbidden) end @@ -161,7 +168,7 @@ RSpec.describe API::Topics do context 'as anonymous' do it 'returns 401 Unauthorized' do - post api('/topics/'), params: { name: 'my-topic' } + post api('/topics/'), params: { name: 'my-topic', title: 'My Topic' } expect(response).to have_gitlab_http_status(:unauthorized) end diff --git a/spec/requests/api/usage_data_spec.rb b/spec/requests/api/usage_data_spec.rb index aefccc4fbf7..ea50c404d92 100644 --- a/spec/requests/api/usage_data_spec.rb +++ b/spec/requests/api/usage_data_spec.rb @@ -7,9 +7,7 @@ RSpec.describe API::UsageData do describe 'POST /usage_data/increment_counter' do let(:endpoint) { '/usage_data/increment_counter' } - let(:known_event) { "#{known_event_prefix}_#{known_event_postfix}" } - let(:known_event_prefix) { "static_site_editor" } - let(:known_event_postfix) { 'commits' } + let(:known_event) { "diff_searches" } let(:unknown_event) { 'unknown' } context 'without CSRF token' do @@ -44,7 +42,6 @@ RSpec.describe API::UsageData do context 'with authentication' do before do stub_feature_flags(usage_data_api: true) - stub_feature_flags("usage_data_#{known_event}" => true) stub_application_setting(usage_ping_enabled: true) allow(Gitlab::RequestForgeryProtection).to receive(:verified?).and_return(true) end @@ -58,28 +55,18 @@ RSpec.describe API::UsageData do end context 'with correct params' do - using RSpec::Parameterized::TableSyntax - - where(:prefix, :event) do - 'static_site_editor' | 'merge_requests' - 'static_site_editor' | 'commits' - end - before do stub_application_setting(usage_ping_enabled: true) stub_feature_flags(usage_data_api: true) allow(Gitlab::RequestForgeryProtection).to receive(:verified?).and_return(true) - stub_feature_flags("usage_data_#{prefix}_#{event}" => true) end - with_them do - it 'returns status :ok' do - expect(Gitlab::UsageDataCounters::BaseCounter).to receive(:count).with(event) + it 'returns status :ok' do + expect(Gitlab::UsageDataCounters::BaseCounter).to receive(:count).with("searches") - post api(endpoint, user), params: { event: "#{prefix}_#{event}" } + post api(endpoint, user), params: { event: known_event } - expect(response).to have_gitlab_http_status(:ok) - end + expect(response).to have_gitlab_http_status(:ok) end end @@ -137,7 +124,6 @@ RSpec.describe API::UsageData do context 'with authentication' do before do stub_feature_flags(usage_data_api: true) - stub_feature_flags("usage_data_#{known_event}" => true) stub_application_setting(usage_ping_enabled: true) allow(Gitlab::RequestForgeryProtection).to receive(:verified?).and_return(true) end diff --git a/spec/requests/api/user_counts_spec.rb b/spec/requests/api/user_counts_spec.rb index 27ebf02dd81..2d4705920cf 100644 --- a/spec/requests/api/user_counts_spec.rb +++ b/spec/requests/api/user_counts_spec.rb @@ -43,7 +43,7 @@ RSpec.describe API::UserCounts do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_a Hash expect(json_response['merge_requests']).to eq(2) - expect(json_response['attention_requests']).to eq(2) + expect(json_response['attention_requests']).to eq(0) end describe 'mr_attention_requests is disabled' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index c554463df76..040ac4f74a7 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1679,13 +1679,13 @@ RSpec.describe API::Users do end it 'creates SSH key with `expires_at` attribute' do - optional_attributes = { expires_at: '2016-01-21T00:00:00.000Z' } + optional_attributes = { expires_at: 3.weeks.from_now } attributes = attributes_for(:key).merge(optional_attributes) post api("/users/#{user.id}/keys", admin), params: attributes expect(response).to have_gitlab_http_status(:created) - expect(json_response['expires_at']).to eq(optional_attributes[:expires_at]) + expect(json_response['expires_at'].to_date).to eq(optional_attributes[:expires_at].to_date) end it "returns 400 for invalid ID" do @@ -2373,13 +2373,13 @@ RSpec.describe API::Users do end it 'creates SSH key with `expires_at` attribute' do - optional_attributes = { expires_at: '2016-01-21T00:00:00.000Z' } + optional_attributes = { expires_at: 3.weeks.from_now } attributes = attributes_for(:key).merge(optional_attributes) post api("/user/keys", user), params: attributes expect(response).to have_gitlab_http_status(:created) - expect(json_response['expires_at']).to eq(optional_attributes[:expires_at]) + expect(json_response['expires_at'].to_date).to eq(optional_attributes[:expires_at].to_date) end it "returns a 401 error if unauthorized" do |