diff options
Diffstat (limited to 'spec/requests/api')
98 files changed, 2016 insertions, 1117 deletions
diff --git a/spec/requests/api/admin/ci/variables_spec.rb b/spec/requests/api/admin/ci/variables_spec.rb index f89964411f8..4bdc44cb583 100644 --- a/spec/requests/api/admin/ci/variables_spec.rb +++ b/spec/requests/api/admin/ci/variables_spec.rb @@ -71,7 +71,8 @@ RSpec.describe ::API::Admin::Ci::Variables do key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, - masked: true + masked: true, + raw: true } end.to change { ::Ci::InstanceVariable.count }.by(1) @@ -80,9 +81,19 @@ RSpec.describe ::API::Admin::Ci::Variables do expect(json_response['value']).to eq('PROTECTED_VALUE_2') expect(json_response['protected']).to be_truthy expect(json_response['masked']).to be_truthy + expect(json_response['raw']).to be_truthy expect(json_response['variable_type']).to eq('env_var') end + it 'masks the new value when logging' do + masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' } + + expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) + + post api("/admin/ci/variables", user), + params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true } + end + it 'creates variable with optional attributes', :aggregate_failures do expect do post api('/admin/ci/variables', admin), @@ -98,6 +109,7 @@ RSpec.describe ::API::Admin::Ci::Variables do expect(json_response['value']).to eq('VALUE_2') expect(json_response['protected']).to be_falsey expect(json_response['masked']).to be_falsey + expect(json_response['raw']).to be_falsey expect(json_response['variable_type']).to eq('file') end @@ -153,7 +165,8 @@ RSpec.describe ::API::Admin::Ci::Variables do variable_type: 'file', value: 'VALUE_1_UP', protected: true, - masked: true + masked: true, + raw: true } expect(response).to have_gitlab_http_status(:ok) @@ -161,6 +174,16 @@ RSpec.describe ::API::Admin::Ci::Variables do expect(variable.reload).to be_protected expect(json_response['variable_type']).to eq('file') expect(json_response['masked']).to be_truthy + expect(json_response['raw']).to be_truthy + end + + it 'masks the new value when logging' do + masked_params = { 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' } + + expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) + + put api("/admin/ci/variables/#{variable.key}", admin), + params: { value: 'SENSITIVE', protected: true, masked: true } end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/api/alert_management_alerts_spec.rb b/spec/requests/api/alert_management_alerts_spec.rb index 99293e5ae95..680a3883387 100644 --- a/spec/requests/api/alert_management_alerts_spec.rb +++ b/spec/requests/api/alert_management_alerts_spec.rb @@ -140,7 +140,7 @@ RSpec.describe API::AlertManagementAlerts do project.send("add_#{user_role}", user) end - it_behaves_like "#{params[:expected_status]}" + it_behaves_like params[:expected_status].to_s end context 'file size too large' do @@ -245,7 +245,7 @@ RSpec.describe API::AlertManagementAlerts do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) unless public_project end - it_behaves_like "#{params[:expected_status]}" + it_behaves_like params[:expected_status].to_s end end @@ -293,7 +293,7 @@ RSpec.describe API::AlertManagementAlerts do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) unless public_project end - it_behaves_like "#{params[:expected_status]}" + it_behaves_like params[:expected_status].to_s end context 'when user has access' do @@ -368,7 +368,7 @@ RSpec.describe API::AlertManagementAlerts do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) unless public_project end - it_behaves_like "#{params[:expected_status]}" + it_behaves_like params[:expected_status].to_s end context 'when user has access' do diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index 817e1324c7c..4d7256a1f03 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -7,7 +7,7 @@ RSpec.describe API::Boards do let_it_be(:non_member) { create(:user) } let_it_be(:guest) { create(:user) } let_it_be(:admin) { create(:user, :admin) } - let_it_be(:board_parent, reload: true) { create(:project, :public, creator_id: user.id, namespace: user.namespace ) } + let_it_be(:board_parent, reload: true) { create(:project, :public, creator_id: user.id, namespace: user.namespace) } let_it_be(:dev_label) do create(:label, title: 'Development', color: '#FFAABB', project: board_parent) @@ -97,7 +97,7 @@ RSpec.describe API::Boards do describe "POST /groups/:id/boards/:board_id/lists" do let_it_be(:group) { create(:group) } - let_it_be(:board_parent) { create(:group, parent: group ) } + let_it_be(:board_parent) { create(:group, parent: group) } let(:url) { "/groups/#{board_parent.id}/boards/#{board.id}/lists" } let_it_be(:board) { create(:board, group: board_parent) } diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index 2bf242f06ed..da9eb6b2216 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -389,8 +389,7 @@ RSpec.describe API::Ci::JobArtifacts do end end - context 'when Google CDN is enabled' do - let(:cdn_enabled) { true } + context 'when Google CDN is configured' do let(:cdn_config) do { 'provider' => 'Google', @@ -401,7 +400,6 @@ RSpec.describe API::Ci::JobArtifacts do end before do - stub_feature_flags(ci_job_artifacts_cdn: cdn_enabled) stub_object_storage_uploader(config: Gitlab.config.artifacts.object_store, uploader: JobArtifactUploader, proxy_download: proxy_download, @@ -418,18 +416,6 @@ RSpec.describe API::Ci::JobArtifacts do expect(response.redirect_url).to start_with("https://cdn.example.org/#{artifact.file.path}") end - - context 'when ci_job_artifacts_cdn feature flag is disabled' do - let(:cdn_enabled) { false } - - it 'returns the file remote URL' do - expect(Gitlab::ApplicationContext).to receive(:push).with(artifact_used_cdn: false).and_call_original - - subject - - expect(response).to redirect_to(artifact.file.url) - end - end end context 'authorized user' do diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 0e17db516f4..c1b7461f444 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -606,6 +606,32 @@ RSpec.describe API::Ci::Jobs do end end end + + context 'when ci_debug_services is set to true' do + before_all do + create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: true) + end + + where(:public_builds, :user_project_role, :expected_status) do + true | 'developer' | :ok + true | 'guest' | :forbidden + false | 'developer' | :ok + false | 'guest' | :forbidden + end + + with_them do + before do + project.update!(public_builds: public_builds) + project.add_role(user, user_project_role) + + get api("/projects/#{project.id}/jobs/#{job.id}/trace", api_user) + end + + it 'renders successfully to authorized users' do + expect(response).to have_gitlab_http_status(expected_status) + end + end + end end describe 'POST /projects/:id/jobs/:job_id/cancel' do diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 697fe16e222..c9d06f37c8b 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -940,7 +940,12 @@ RSpec.describe API::Ci::Pipelines do subject expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to contain_exactly({ "variable_type" => "env_var", "key" => "foo", "value" => "bar" }) + expect(json_response).to contain_exactly({ + "variable_type" => "env_var", + "key" => "foo", + "value" => "bar", + "raw" => false + }) end end end @@ -961,7 +966,12 @@ RSpec.describe API::Ci::Pipelines do subject expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to contain_exactly({ "variable_type" => "env_var", "key" => "foo", "value" => "bar" }) + expect(json_response).to contain_exactly({ + "variable_type" => "env_var", + "key" => "foo", + "value" => "bar", + "raw" => false + }) end end diff --git a/spec/requests/api/ci/resource_groups_spec.rb b/spec/requests/api/ci/resource_groups_spec.rb index 87df71f6096..2a67a3e4322 100644 --- a/spec/requests/api/ci/resource_groups_spec.rb +++ b/spec/requests/api/ci/resource_groups_spec.rb @@ -56,6 +56,31 @@ RSpec.describe API::Ci::ResourceGroups do expect(Time.parse(json_response['updated_at'])).to be_like_time(resource_group.updated_at) end + context 'when resource group key contains multiple dots' do + let!(:resource_group) { create(:ci_resource_group, project: project, key: 'test..test') } + + it 'returns the resource group', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(resource_group.id) + expect(json_response['key']).to eq(resource_group.key) + end + end + + context 'when resource group key contains a slash' do + let!(:resource_group) { create(:ci_resource_group, project: project, key: 'test/test') } + let(:key) { 'test%2Ftest' } + + it 'returns the resource group', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(resource_group.id) + expect(json_response['key']).to eq(resource_group.key) + end + end + context 'when user is reporter' do let(:user) { reporter } @@ -98,6 +123,25 @@ RSpec.describe API::Ci::ResourceGroups do expect(json_response[0]['status']).to eq(upcoming_processable.status) end + context 'when resource group key contains a slash' do + let_it_be(:resource_group) { create(:ci_resource_group, project: project, key: 'test/test') } + let_it_be(:upcoming_processable) do + create(:ci_processable, + :waiting_for_resource, + resource_group: resource_group) + end + + let(:key) { 'test%2Ftest' } + + it 'returns the resource group', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response[0]['id']).to eq(upcoming_processable.id) + expect(json_response[0]['name']).to eq(upcoming_processable.name) + end + end + context 'when user is reporter' do let(:user) { reporter } diff --git a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb index cd8c3dd2806..9af0541bd2c 100644 --- a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb +++ b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb @@ -238,7 +238,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do context 'authorization token is invalid' do it 'responds with forbidden' do - authorize_artifacts(token: 'invalid', filesize: 100 ) + authorize_artifacts(token: 'invalid', filesize: 100) expect(response).to have_gitlab_http_status(:forbidden) end @@ -881,11 +881,11 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end - shared_examples 'forbidden request' do - it 'responds with forbidden' do + shared_examples 'unauthorized request' do + it 'responds with unauthorized' do download_artifact - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) end end @@ -899,7 +899,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do job.success! end - it_behaves_like 'successful artifact download' + it_behaves_like 'unauthorized request' end end @@ -916,7 +916,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do dependent_job.success! end - it_behaves_like 'forbidden request' + it_behaves_like 'unauthorized request' end end @@ -942,7 +942,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do let(:token) { ci_build.token } - it_behaves_like 'forbidden request' + it_behaves_like 'unauthorized request' end context 'when using a token from a cross pipeline build' do @@ -981,19 +981,23 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do let!(:unrelated_ci_build) { create(:ci_build, :running, user: create(:user)) } let(:token) { unrelated_ci_build.token } - it_behaves_like 'forbidden request' + it 'responds with forbidden' do + download_artifact + + expect(response).to have_gitlab_http_status(:forbidden) + end end context 'when using runnners token' do let(:token) { job.project.runners_token } - it_behaves_like 'forbidden request' + it_behaves_like 'unauthorized request' end context 'when using an invalid token' do let(:token) { 'invalid-token' } - it_behaves_like 'forbidden request' + it_behaves_like 'unauthorized 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 d4f734e7bdd..1cb4cc93ea5 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -462,7 +462,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do request_job info: { 'config' => { 'gpus' => 'all', 'ignored' => 'hello' } } expect(response).to have_gitlab_http_status(:created) - expect(runner.reload.config).to eq( { 'gpus' => 'all' } ) + expect(runner.reload.config).to eq({ 'gpus' => 'all' }) end it "sets the runner's ip_address" do diff --git a/spec/requests/api/ci/runners_reset_registration_token_spec.rb b/spec/requests/api/ci/runners_reset_registration_token_spec.rb index e1dc347f8dd..b8e4370fd46 100644 --- a/spec/requests/api/ci/runners_reset_registration_token_spec.rb +++ b/spec/requests/api/ci/runners_reset_registration_token_spec.rb @@ -118,7 +118,7 @@ RSpec.describe API::Ci::Runners do end include_context 'when authorized', 'group' do - let_it_be(:user) { create_default(:group_member, :owner, user: create(:user), group: group ).user } + let_it_be(:user) { create_default(:group_member, :owner, user: create(:user), group: group).user } def get_token group.reload.runners_token diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index 69f26d3f257..dd9894f2972 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -399,7 +399,7 @@ RSpec.describe API::Ci::Runners do it 'unrelated runner attribute on an existing runner with too many tags' do # This test ensures that it is possible to update any attribute on a runner that currently fails the # validation that ensures that there aren't too many tags associated with a runner - existing_invalid_shared_runner = build(:ci_runner, :instance, tag_list: (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" } ) + existing_invalid_shared_runner = build(:ci_runner, :instance, tag_list: (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" }) existing_invalid_shared_runner.save!(validate: false) active = existing_invalid_shared_runner.active diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb index 0b8116d5e20..b0bca6e9125 100644 --- a/spec/requests/api/ci/secure_files_spec.rb +++ b/spec/requests/api/ci/secure_files_spec.rb @@ -143,6 +143,18 @@ 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['expires_at']).to be nil + expect(json_response['metadata']).to be nil + end + + it 'returns project secure file details with metadata when supported' do + secure_file_with_metadata = create(:ci_secure_file_with_metadata, project: project) + get api("/projects/#{project.id}/secure_files/#{secure_file_with_metadata.id}", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['name']).to eq(secure_file_with_metadata.name) + expect(json_response['expires_at']).to eq('2022-04-26T19:20:40.000Z') + expect(json_response['metadata'].keys).to match_array(%w[id issuer subject expires_at]) end it 'responds with 404 Not Found if requesting non-existing secure file' do diff --git a/spec/requests/api/ci/triggers_spec.rb b/spec/requests/api/ci/triggers_spec.rb index 953dcb8a483..f9b7880a4c4 100644 --- a/spec/requests/api/ci/triggers_spec.rb +++ b/spec/requests/api/ci/triggers_spec.rb @@ -81,7 +81,7 @@ RSpec.describe API::Ci::Triggers do end it 'validates variables needs to be a map of key-valued strings' do - post api("/projects/#{project.id}/trigger/pipeline"), params: options.merge(variables: { key: %w(1 2) }, ref: 'master') + post api("/projects/#{project.id}/trigger/pipeline"), params: options.merge(variables: { 'TRIGGER_KEY' => %w(1 2) }, ref: 'master') expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']).to eq('variables needs to be a map of key-valued strings') diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index 74ed8c1551d..cafb841995d 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -46,6 +46,7 @@ RSpec.describe API::Ci::Variables do expect(json_response['value']).to eq(variable.value) expect(json_response['protected']).to eq(variable.protected?) expect(json_response['masked']).to eq(variable.masked?) + expect(json_response['raw']).to eq(variable.raw?) expect(json_response['variable_type']).to eq('env_var') end @@ -115,7 +116,7 @@ RSpec.describe API::Ci::Variables do context 'authorized user with proper permissions' do it 'creates variable' do expect do - post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true } + post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true, raw: true } end.to change { project.variables.count }.by(1) expect(response).to have_gitlab_http_status(:created) @@ -123,12 +124,22 @@ RSpec.describe API::Ci::Variables do expect(json_response['value']).to eq('PROTECTED_VALUE_2') expect(json_response['protected']).to be_truthy expect(json_response['masked']).to be_truthy + expect(json_response['raw']).to be_truthy expect(json_response['variable_type']).to eq('env_var') end + it 'masks the new value when logging' do + masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' } + + expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) + + post api("/projects/#{project.id}/variables", user), + params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true } + end + it 'creates variable with optional attributes' do expect do - post api("/projects/#{project.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } + post api("/projects/#{project.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } end.to change { project.variables.count }.by(1) expect(response).to have_gitlab_http_status(:created) @@ -136,6 +147,7 @@ RSpec.describe API::Ci::Variables do expect(json_response['value']).to eq('VALUE_2') expect(json_response['protected']).to be_falsey expect(json_response['masked']).to be_falsey + expect(json_response['raw']).to be_falsey expect(json_response['variable_type']).to eq('file') end @@ -206,6 +218,15 @@ RSpec.describe API::Ci::Variables do expect(updated_variable.variable_type).to eq('file') end + it 'masks the new value when logging' do + masked_params = { 'value' => '[FILTERED]', 'protected' => 'true' } + + expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) + + put api("/projects/#{project.id}/variables/#{variable.key}", user), + params: { value: 'SENSITIVE', protected: true } + end + it 'responds with 404 Not Found if requesting non-existing variable' do put api("/projects/#{project.id}/variables/non_existing_variable", user) diff --git a/spec/requests/api/clusters/agent_tokens_spec.rb b/spec/requests/api/clusters/agent_tokens_spec.rb index 2dca21ca6f1..a33bef53b14 100644 --- a/spec/requests/api/clusters/agent_tokens_spec.rb +++ b/spec/requests/api/clusters/agent_tokens_spec.rb @@ -5,7 +5,7 @@ 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(:revoked_agent_token) { create(:cluster_agent_token, :revoked, agent: agent) } let_it_be(:project) { agent.project } let_it_be(:user) { agent.created_by_user } let_it_be(:unauthorized_user) { create(:user) } @@ -17,7 +17,7 @@ RSpec.describe API::Clusters::AgentTokens do describe 'GET /projects/:id/cluster_agents/:agent_id/tokens' do context 'with authorized user' do - it 'returns tokens' do + it 'returns tokens regardless of status' do get api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", user) aggregate_failures "testing response" do @@ -27,10 +27,16 @@ RSpec.describe API::Clusters::AgentTokens do 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['name']).to eq(revoked_agent_token.name) expect(json_response.second['agent_id']).to eq(agent.id) end end + + it 'returns a not_found error if agent_id does not exist' do + get api("/projects/#{project.id}/cluster_agents/#{non_existing_record_id}/tokens", user) + + expect(response).to have_gitlab_http_status(:not_found) + end end context 'with unauthorized user' do diff --git a/spec/requests/api/dependency_proxy_spec.rb b/spec/requests/api/dependency_proxy_spec.rb index a8617fcb0bf..7af4ed08cb8 100644 --- a/spec/requests/api/dependency_proxy_spec.rb +++ b/spec/requests/api/dependency_proxy_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe API::DependencyProxy, api: true do let_it_be(:user) { create(:user) } - let_it_be(:blob) { create(:dependency_proxy_blob ) } + let_it_be(:blob) { create(:dependency_proxy_blob) } let_it_be(:group, reload: true) { blob.group } before do diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index 24e0e5d3180..8124080abea 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -201,7 +201,7 @@ RSpec.describe API::Deployments do } ) - expect(response).to have_gitlab_http_status(:internal_server_error) + expect(response).to have_gitlab_http_status(:bad_request) end it 'links any merged merge requests to the deployment', :sidekiq_inline do @@ -325,7 +325,7 @@ RSpec.describe API::Deployments do context 'as non member' do it 'returns a 404 status code' do post( - api( "/projects/#{project.id}/deployments", non_member), + api("/projects/#{project.id}/deployments", non_member), params: { environment: 'production', sha: '123', diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index d0334cf6dd2..85dafef569d 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -193,7 +193,7 @@ RSpec.describe API::Features, stub_feature_flags: false do 'state' => 'conditional', 'gates' => [ { 'key' => 'boolean', 'value' => false }, - { 'key' => 'actors', 'value' => ["#{actor.class}:#{actor.id}"] } + { 'key' => 'actors', 'value' => [actor.flipper_id] } ], 'definition' => known_feature_flag_definition_hash ) @@ -269,6 +269,20 @@ RSpec.describe API::Features, stub_feature_flags: false do end end + context 'when enabling for a repository by path' do + context 'when the repository exists' do + it_behaves_like 'enables the flag for the actor', :repository do + let_it_be(:actor) { create(:project).repository } + end + end + + context 'when the repository does not exist' do + it_behaves_like 'does not enable the flag', :repository do + let(:actor_path) { 'not/a/repository' } + end + end + end + context 'with multiple users' do let_it_be(:users) { create_list(:user, 3) } @@ -361,6 +375,29 @@ RSpec.describe API::Features, stub_feature_flags: false do end end + context 'with multiple repository' do + let_it_be(:projects) { create_list(:project, 3) } + + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { repository: projects.map { |p| p.repository.full_path }.join(',') } } + let(:expected_gate_params) { projects.map { |p| p.repository.flipper_id } } + end + + context 'when empty value exists between comma' do + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { repository: "#{projects.first.repository.full_path},,,," } } + let(:expected_gate_params) { projects.first.repository.flipper_id } + end + end + + context 'when one of the projects does not exist' do + it_behaves_like 'does not enable the flag', :project do + let(:actor_path) { "#{projects.first.repository.full_path},inexistent-entry" } + let(:expected_inexistent_path) { "inexistent-entry" } + end + end + end + it 'creates a feature with the given percentage of time if passed an integer' do post api("/features/#{feature_name}", admin), params: { value: '50' } diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index e95a626b4aa..d4d3aace204 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -11,7 +11,7 @@ RSpec.describe API::Files do let_it_be(:inherited_reporter) { create(:user) } let_it_be(:inherited_developer) { create(:user) } - let!(:project) { create(:project, :repository, namespace: user.namespace ) } + let!(:project) { create(:project, :repository, namespace: user.namespace) } let(:guest) { create(:user) { |u| project.add_guest(u) } } let(:file_path) { 'files%2Fruby%2Fpopen%2Erb' } let(:file_name) { 'popen.rb' } @@ -935,7 +935,7 @@ RSpec.describe API::Files do end context 'and the repo is empty' do - let!(:project) { create(:project_empty_repo, namespace: user.namespace ) } + let!(:project) { create(:project_empty_repo, namespace: user.namespace) } it_behaves_like 'creates a new file in the project repo' do let(:current_user) { user } @@ -1253,4 +1253,35 @@ RSpec.describe API::Files do expect(json_response['content']).to eq(put_params[:content]) end end + + describe 'POST /projects/:id/repository/files with text encoding' do + let(:file_path) { 'test%2Etext' } + let(:put_params) do + { + branch: 'master', + content: 'test', + commit_message: 'Text file', + encoding: 'text' + } + end + + let(:get_params) do + { + ref: 'master' + } + end + + before do + post api(route(file_path), user), params: put_params + end + + it 'returns base64-encoded text file' do + get api(route(file_path), user), params: get_params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['file_path']).to eq(CGI.unescape(file_path)) + expect(json_response['file_name']).to eq(CGI.unescape(file_path)) + expect(Base64.decode64(json_response['content'])).to eq("test") + end + end end diff --git a/spec/requests/api/go_proxy_spec.rb b/spec/requests/api/go_proxy_spec.rb index 7c44fddc303..5498ed6df13 100644 --- a/spec/requests/api/go_proxy_spec.rb +++ b/spec/requests/api/go_proxy_spec.rb @@ -16,12 +16,12 @@ RSpec.describe API::GoProxy do let_it_be(:modules) do commits = [ - create(:go_module_commit, :files, project: project, tag: 'v1.0.0', files: { 'README.md' => 'Hi' } ), - create(:go_module_commit, :module, project: project, tag: 'v1.0.1' ), - create(:go_module_commit, :package, project: project, tag: 'v1.0.2', path: 'pkg' ), - create(:go_module_commit, :module, project: project, tag: 'v1.0.3', name: 'mod' ), - create(:go_module_commit, :files, project: project, files: { 'y.go' => "package a\n" } ), - create(:go_module_commit, :module, project: project, name: 'v2' ), + create(:go_module_commit, :files, project: project, tag: 'v1.0.0', files: { 'README.md' => 'Hi' }), + create(:go_module_commit, :module, project: project, tag: 'v1.0.1'), + create(:go_module_commit, :package, project: project, tag: 'v1.0.2', path: 'pkg'), + create(:go_module_commit, :module, project: project, tag: 'v1.0.3', name: 'mod'), + create(:go_module_commit, :files, project: project, files: { 'y.go' => "package a\n" }), + create(:go_module_commit, :module, project: project, name: 'v2'), create(:go_module_commit, :files, project: project, tag: 'v2.0.0', files: { 'v2/x.go' => "package a\n" }) ] @@ -288,10 +288,10 @@ RSpec.describe API::GoProxy do let_it_be(:base) { "#{Settings.build_gitlab_go_url}/#{project.full_path}" } let_it_be(:modules) do - create(:go_module_commit, :files, project: project, files: { 'a.go' => "package\a" } ) + create(:go_module_commit, :files, project: project, files: { 'a.go' => "package\a" }) create(:go_module_commit, :files, project: project, tag: 'v1.0.0', files: { 'go.mod' => "module not/a/real/module\n" }) - create(:go_module_commit, :files, project: project, files: { 'v2/a.go' => "package a\n" } ) - create(:go_module_commit, :files, project: project, tag: 'v2.0.0', files: { 'v2/go.mod' => "module #{base}\n" } ) + create(:go_module_commit, :files, project: project, files: { 'v2/a.go' => "package a\n" }) + create(:go_module_commit, :files, project: project, tag: 'v2.0.0', files: { 'v2/go.mod' => "module #{base}\n" }) end describe 'GET /projects/:id/packages/go/*module_name/@v/list' do @@ -406,6 +406,19 @@ RSpec.describe API::GoProxy do expect(response).to have_gitlab_http_status(:unauthorized) end end + + context 'with access to package registry for everyone' do + let_it_be(:user) { nil } + + before do + project.reload.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + end + + it_behaves_like 'a module version list resource', 'v1.0.1', 'v1.0.2', 'v1.0.3' + it_behaves_like 'a module version information resource', 'v1.0.1' + it_behaves_like 'a module file resource', 'v1.0.1' + it_behaves_like 'a module archive resource', 'v1.0.1', ['README.md', 'go.mod', 'a.go'] + end end context 'with a public project' do diff --git a/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb b/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb index 484ddc3469b..9bed720c815 100644 --- a/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'get board lists' do let_it_be(:user) { create(:user) } let_it_be(:unauth_user) { create(:user) } - let_it_be(:project) { create(:project, creator_id: user.id, namespace: user.namespace ) } + let_it_be(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let_it_be(:group) { create(:group, :private) } let_it_be(:project_label) { create(:label, project: project, name: 'Development') } let_it_be(:project_label2) { create(:label, project: project, name: 'Testing') } @@ -21,6 +21,7 @@ RSpec.describe 'get board lists' do let(:board_data) { graphql_data[board_parent_type]['boards']['nodes'][0] } let(:lists_data) { board_data['lists']['nodes'][0] } let(:issues_data) { lists_data['issues']['nodes'] } + let(:issue_params) { { filters: { label_name: label2.title, confidential: confidential }, first: 3 } } def query(list_params = params) graphql_query_for( @@ -31,7 +32,7 @@ RSpec.describe 'get board lists' do nodes { lists { nodes { - issues(filters: {labelName: "#{label2.title}", confidential: #{confidential}}, first: 3) { + issues(#{attributes_to_graphql(issue_params)}) { count nodes { #{all_graphql_fields_for('issues'.classify)} @@ -77,18 +78,23 @@ RSpec.describe 'get board lists' do end context 'when user can read the board' do - before do + before_all do board_parent.add_reporter(user) - post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user) end + subject { post_graphql(query("id: \"#{global_id_of(label_list)}\""), current_user: user) } + it 'can access the issues', :aggregate_failures do + subject + # ties for relative positions are broken by id in ascending order by default expect(issue_titles).to eq([issue2.title, issue1.title, issue3.title]) expect(issue_relative_positions).not_to include(nil) end it 'does not set the relative positions of the issues not being returned', :aggregate_failures do + subject + expect(issue_id).not_to include(issue6.id) expect(issue3.relative_position).to be_nil end @@ -97,10 +103,36 @@ RSpec.describe 'get board lists' do let(:confidential) { true } it 'returns matching issue' do + subject + expect(issue_titles).to match_array([issue7.title]) expect(issue_relative_positions).not_to include(nil) end end + + context 'when filtering by a unioned argument' do + let(:another_user) { create(:user) } + let(:issue_params) { { filters: { or: { assignee_usernames: [user.username, another_user.username] } } } } + + it 'returns correctly filtered issues' do + issue1.assignee_ids = user.id + issue2.assignee_ids = another_user.id + + subject + + expect(issue_id).to contain_exactly(issue1.to_gid.to_s, issue2.to_gid.to_s) + end + + context 'when feature flag is disabled' do + it 'returns an error' do + stub_feature_flags(or_issuable_queries: false) + + subject + + expect_graphql_errors_to_include("'or' arguments are only allowed when the `or_issuable_queries` feature flag is enabled.") + end + end + end end end 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 6fe2e41cf35..ad7df5c9344 100644 --- a/spec/requests/api/graphql/boards/board_lists_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_lists_query_spec.rb @@ -49,7 +49,7 @@ RSpec.describe 'get board lists' do end shared_examples 'group and project board lists query' do - let!(:board) { create(:board, resource_parent: board_parent) } + let_it_be(:board) { create(:board, resource_parent: board_parent) } context 'when the user does not have access to the board' do it 'returns nil' do @@ -107,16 +107,20 @@ RSpec.describe 'get board lists' do end context 'when querying for a single list' do + let_it_be(:label_list) { create(:list, board: board, label: label, position: 10) } + let_it_be(:issues) do + [ + create(:issue, project: project, labels: [label, label2]), + create(:issue, project: project, labels: [label, label2], confidential: true), + create(:issue, project: project, labels: [label]) + ] + end + before do board_parent.add_reporter(user) end it 'returns the correct list with issue count for matching issue filters' do - label_list = create(:list, board: board, label: label, position: 10) - create(:issue, project: project, labels: [label, label2]) - create(:issue, project: project, labels: [label, label2], confidential: true) - create(:issue, project: project, labels: [label]) - post_graphql( query( id: global_id_of(label_list), @@ -131,21 +135,56 @@ RSpec.describe 'get board lists' do expect(list_node['issuesCount']).to eq 1 end end + + context 'when filtering by a unioned argument' do + let_it_be(:another_user) { create(:user) } + + it 'returns correctly filtered issues' do + issues[0].assignee_ids = user.id + issues[1].assignee_ids = another_user.id + + post_graphql( + query( + id: global_id_of(label_list), + issueFilters: { or: { assignee_usernames: [user.username, another_user.username] } } + ), current_user: user + ) + + expect(lists_data[0]['node']['issuesCount']).to eq 2 + end + + context 'when feature flag is disabled' do + it 'returns an error' do + stub_feature_flags(or_issuable_queries: false) + + post_graphql( + query( + id: global_id_of(label_list), + issueFilters: { or: { assignee_usernames: [user.username, another_user.username] } } + ), current_user: user + ) + + expect_graphql_errors_to_include( + "'or' arguments are only allowed when the `or_issuable_queries` feature flag is enabled." + ) + end + end + end end end describe 'for a project' do - let(:board_parent) { project } - let(:label) { project_label } - let(:label2) { project_label2 } + let_it_be(:board_parent) { project } + let_it_be(:label) { project_label } + let_it_be(:label2) { project_label2 } it_behaves_like 'group and project board lists query' end describe 'for a group' do - let(:board_parent) { group } - let(:label) { group_label } - let(:label2) { group_label2 } + let_it_be(:board_parent) { group } + let_it_be(:label) { group_label } + let_it_be(:label2) { group_label2 } before do allow(board_parent).to receive(:multiple_issue_boards_available?).and_return(false) diff --git a/spec/requests/api/graphql/ci/jobs_spec.rb b/spec/requests/api/graphql/ci/jobs_spec.rb index 47e3221c567..a161c5c98ed 100644 --- a/spec/requests/api/graphql/ci/jobs_spec.rb +++ b/spec/requests/api/graphql/ci/jobs_spec.rb @@ -109,7 +109,7 @@ RSpec.describe 'Query.project.pipeline' do 'name' => 'docker 1 2', 'needs' => { 'nodes' => [] }, 'previousStageJobsOrNeeds' => { 'nodes' => [ - a_hash_including( 'name' => 'my test job' ) + a_hash_including('name' => 'my test job') ] } ), a_hash_including( @@ -129,7 +129,7 @@ RSpec.describe 'Query.project.pipeline' do 'name' => 'rspec 2 2', 'needs' => { 'nodes' => [a_hash_including('name' => 'my test job')] }, 'previousStageJobsOrNeeds' => { 'nodes' => [ - a_hash_including('name' => 'my test job' ) + a_hash_including('name' => 'my test job') ] } ) ) diff --git a/spec/requests/api/graphql/ci/pipelines_spec.rb b/spec/requests/api/graphql/ci/pipelines_spec.rb index f471a152603..948704e8770 100644 --- a/spec/requests/api/graphql/ci/pipelines_spec.rb +++ b/spec/requests/api/graphql/ci/pipelines_spec.rb @@ -419,7 +419,7 @@ RSpec.describe 'Query.project(fullPath).pipelines' do end before do - create(:ci_sources_pipeline, source_pipeline: upstream_pipeline, pipeline: pipeline ) + create(:ci_sources_pipeline, source_pipeline: upstream_pipeline, pipeline: pipeline) post_graphql(query, current_user: user) end @@ -441,10 +441,10 @@ RSpec.describe 'Query.project(fullPath).pipelines' do pipeline_2 = create(:ci_pipeline, project: project, user: user) upstream_pipeline_2 = create(:ci_pipeline, project: upstream_project, user: user) - create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_2, pipeline: pipeline_2 ) + create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_2, pipeline: pipeline_2) pipeline_3 = create(:ci_pipeline, project: project, user: user) upstream_pipeline_3 = create(:ci_pipeline, project: upstream_project, user: user) - create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_3, pipeline: pipeline_3 ) + create(:ci_sources_pipeline, source_pipeline: upstream_pipeline_3, pipeline: pipeline_3) expect do post_graphql(query, current_user: second_user) diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index bd90753f9ad..94c0a3c41bd 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -104,7 +104,8 @@ RSpec.describe 'Query.runner(id)' do 'userPermissions' => { 'readRunner' => true, 'updateRunner' => true, - 'deleteRunner' => true + 'deleteRunner' => true, + 'assignRunner' => true } ) expect(runner_data['tagList']).to match_array runner.tag_list diff --git a/spec/requests/api/graphql/group/work_item_types_spec.rb b/spec/requests/api/graphql/group/work_item_types_spec.rb index d6b0673e4f8..35090e2a89f 100644 --- a/spec/requests/api/graphql/group/work_item_types_spec.rb +++ b/spec/requests/api/graphql/group/work_item_types_spec.rb @@ -57,15 +57,4 @@ RSpec.describe 'getting a list of work item types for a group' do expect(graphql_data).to eq('group' => nil) end end - - context 'when the work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) - post_graphql(query, current_user: current_user) - end - - it 'returns null' do - expect(graphql_data.dig('group', 'workItemTypes')).to be_nil - end - end end diff --git a/spec/requests/api/graphql/issues_spec.rb b/spec/requests/api/graphql/issues_spec.rb new file mode 100644 index 00000000000..8838ad78f72 --- /dev/null +++ b/spec/requests/api/graphql/issues_spec.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting an issue list at root level' do + include GraphqlHelpers + + let_it_be(:developer) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:group1) { create(:group).tap { |group| group.add_developer(developer) } } + let_it_be(:group2) { create(:group).tap { |group| group.add_developer(developer) } } + let_it_be(:project_a) { create(:project, :repository, :public, group: group1) } + let_it_be(:project_b) { create(:project, :repository, :private, group: group1) } + let_it_be(:project_c) { create(:project, :repository, :public, group: group2) } + let_it_be(:project_d) { create(:project, :repository, :private, group: group2) } + let_it_be(:early_milestone) { create(:milestone, project: project_d, due_date: 10.days.from_now) } + let_it_be(:late_milestone) { create(:milestone, project: project_c, due_date: 30.days.from_now) } + let_it_be(:priority1) { create(:label, project: project_c, priority: 1) } + let_it_be(:priority2) { create(:label, project: project_d, priority: 5) } + let_it_be(:priority3) { create(:label, project: project_a, priority: 10) } + + let_it_be(:issue_a) { create(:issue, project: project_a, labels: [priority3]) } + let_it_be(:issue_b) { create(:issue, :with_alert, project: project_b, discussion_locked: true) } + let_it_be(:issue_c) do + create( + :issue, + project: project_c, + title: 'title matching issue plus', + labels: [priority1], + milestone: late_milestone + ) + end + + let_it_be(:issue_d) { create(:issue, :with_alert, project: project_d, discussion_locked: true, labels: [priority2]) } + let_it_be(:issue_e) { create(:issue, project: project_d, milestone: early_milestone) } + + let(:issue_filter_params) { {} } + + let(:fields) do + <<~QUERY + nodes { + #{all_graphql_fields_for('issues'.classify)} + } + QUERY + end + + before_all do + group2.add_reporter(reporter) + end + + context 'when the root_level_issues_query feature flag is disabled' do + before do + stub_feature_flags(root_level_issues_query: false) + end + + it 'the field returns null' do + post_graphql(query, current_user: developer) + + expect(graphql_data).to eq('issues' => nil) + end + end + + it_behaves_like 'graphql issue list request spec' do + subject(:post_query) { post_graphql(query, current_user: current_user) } + + let(:current_user) { developer } + let(:another_user) { reporter } + let(:issues_data) { graphql_data['issues']['nodes'] } + let(:issue_ids) { graphql_dig_at(issues_data, :id) } + + # filters + let(:expected_negated_assignee_issues) { [issue_b, issue_c, issue_d, issue_e] } + let(:expected_unioned_assignee_issues) { [issue_a, issue_c] } + let(:voted_issues) { [issue_a, issue_c] } + let(:no_award_issues) { [issue_b, issue_d, issue_e] } + let(:locked_discussion_issues) { [issue_b, issue_d] } + let(:unlocked_discussion_issues) { [issue_a, issue_c, issue_e] } + let(:search_title_term) { 'matching issue' } + let(:title_search_issue) { issue_c } + + # sorting + let(:data_path) { [:issues] } + let(:expected_severity_sorted_asc) { [issue_c, issue_a, issue_b, issue_e, issue_d] } + let(:expected_priority_sorted_asc) { [issue_e, issue_c, issue_d, issue_a, issue_b] } + let(:expected_priority_sorted_desc) { [issue_c, issue_e, issue_a, issue_d, issue_b] } + + before_all do + issue_a.assignee_ids = developer.id + issue_c.assignee_ids = reporter.id + + create(:award_emoji, :upvote, user: developer, awardable: issue_a) + create(:award_emoji, :upvote, user: developer, awardable: issue_c) + + # severity sorting + create(:issuable_severity, issue: issue_a, severity: :unknown) + create(:issuable_severity, issue: issue_b, severity: :low) + create(:issuable_severity, issue: issue_d, severity: :critical) + create(:issuable_severity, issue: issue_e, severity: :high) + end + + def pagination_query(params) + graphql_query_for( + :issues, + params, + "#{page_info} nodes { id }" + ) + end + end + + def query(params = issue_filter_params) + graphql_query_for( + :issues, + params, + fields + ) + end +end diff --git a/spec/requests/api/graphql/metadata_query_spec.rb b/spec/requests/api/graphql/metadata_query_spec.rb index 840bd7c018c..435e1b5b596 100644 --- a/spec/requests/api/graphql/metadata_query_spec.rb +++ b/spec/requests/api/graphql/metadata_query_spec.rb @@ -17,7 +17,8 @@ RSpec.describe 'getting project information' do 'enabled' => Gitlab::Kas.enabled?, 'version' => expected_kas_version, 'externalUrl' => expected_kas_external_url - } + }, + 'enterprise' => Gitlab.ee? } } end diff --git a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb index 7cd39f93ae7..e81621209fb 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb @@ -20,7 +20,7 @@ RSpec.describe 'Removing an AwardEmoji' do end def create_award_emoji(user) - create(:award_emoji, name: emoji_name, awardable: awardable, user: user ) + create(:award_emoji, name: emoji_name, awardable: awardable, user: user) end shared_examples 'a mutation that does not destroy an AwardEmoji' do diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb index 7ddffa1ab0a..b151da72b55 100644 --- a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -32,7 +32,7 @@ RSpec.describe 'Toggling an AwardEmoji' do end def create_award_emoji(user) - create(:award_emoji, name: emoji_name, awardable: awardable, user: user ) + create(:award_emoji, name: emoji_name, awardable: awardable, user: user) end context 'when the user has permission' do diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule_take_ownership_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_take_ownership_spec.rb new file mode 100644 index 00000000000..8dfbf20d00b --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_take_ownership_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineScheduleTakeOwnership' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:owner) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: owner) } + + let(:mutation) do + graphql_mutation( + :pipeline_schedule_take_ownership, + { id: pipeline_schedule_id }, + <<-QL + errors + QL + ) + end + + let(:pipeline_schedule_id) { pipeline_schedule.to_global_id.to_s } + + before_all do + project.add_maintainer(user) + end + + it 'returns an error if the user is not allowed to take ownership of the schedule' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + + it 'takes ownership of the schedule' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_nil + end +end 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 6818ba33e74..54e63df96a6 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 @@ -87,7 +87,7 @@ RSpec.describe 'RunnersRegistrationTokenReset' do 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 } + let_it_be(:user) { create_default(:group_member, :owner, user: create(:user), group: group).user } def get_token group.reload.runners_token diff --git a/spec/requests/api/graphql/mutations/container_repository/destroy_spec.rb b/spec/requests/api/graphql/mutations/container_repository/destroy_spec.rb index c4121cfed42..5a27d39ecbc 100644 --- a/spec/requests/api/graphql/mutations/container_repository/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/container_repository/destroy_spec.rb @@ -33,11 +33,11 @@ RSpec.describe 'Destroying a container repository' do end shared_examples 'destroying the container repository' do - it 'destroy the container repository' do + it 'marks the container repository as delete_scheduled' do expect(::Packages::CreateEventService) .to receive(:new).with(nil, user, event_name: :delete_repository, scope: :container).and_call_original expect(DeleteContainerRepositoryWorker) - .to receive(:perform_async).with(user.id, container_repository.id) + .not_to receive(:perform_async) expect { subject }.to change { ::Packages::Event.count }.by(1) @@ -80,6 +80,25 @@ RSpec.describe 'Destroying a container repository' do it_behaves_like params[:shared_examples_name] end + + context 'with container_registry_delete_repository_with_cron_worker disabled' do + before do + project.add_maintainer(user) + stub_feature_flags(container_registry_delete_repository_with_cron_worker: false) + end + + it 'enqueues a removal job' do + expect(::Packages::CreateEventService) + .to receive(:new).with(nil, user, event_name: :delete_repository, scope: :container).and_call_original + expect(DeleteContainerRepositoryWorker) + .to receive(:perform_async).with(user.id, container_repository.id) + + expect { subject }.to change { ::Packages::Event.count }.by(1) + + expect(container_repository_mutation_response).to match_schema('graphql/container_repository') + expect(container_repository_mutation_response['status']).to eq('DELETE_SCHEDULED') + end + end end context 'with invalid id' 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 index 923e12a3c06..fc3b666dd3d 100644 --- 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 @@ -10,8 +10,16 @@ RSpec.describe 'Creating an incident timeline event' do let_it_be(:incident) { create(:incident, project: project) } let_it_be(:event_occurred_at) { Time.current } let_it_be(:note) { 'demo note' } + let_it_be(:tag1) { create(:incident_management_timeline_event_tag, project: project, name: 'Tag 1') } + let_it_be(:tag2) { create(:incident_management_timeline_event_tag, project: project, name: 'Tag 2') } + + let(:input) do + { incident_id: incident.to_global_id.to_s, + note: note, + occurred_at: event_occurred_at, + timeline_event_tag_names: [tag1.name] } + end - 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 @@ -22,6 +30,7 @@ RSpec.describe 'Creating an incident timeline event' do author { id username } incident { id title } note + timelineEventTags { nodes { name } } editable action occurredAt @@ -57,4 +66,25 @@ RSpec.describe 'Creating an incident timeline event' do 'occurredAt' => event_occurred_at.iso8601 ) end + + context 'when note is more than 280 characters long' do + let_it_be(:note) { 'n' * 281 } + + it_behaves_like 'timeline event mutation responds with validation error', + error_message: 'Timeline text is too long (maximum is 280 characters)' + end + + context 'when timeline event tags are passed' do + it 'creates incident timeline event with tags', :aggregate_failures do + post_graphql_mutation(mutation, current_user: user) + + timeline_event_response = mutation_response['timelineEvent'] + tag_names = timeline_event_response['timelineEventTags']['nodes'] + + expect(response).to have_gitlab_http_status(:success) + expect(timeline_event_response).to include( + 'timelineEventTags' => { 'nodes' => tag_names } + ) + end + 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 index 85eaec90f47..62eeecb3fb7 100644 --- 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 @@ -9,7 +9,7 @@ RSpec.describe 'Promote an incident timeline event from a comment' do 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_it_be(:comment) { create(:note, project: project, noteable: incident, note: 'a' * 281) } let(:input) { { note_id: comment.to_global_id.to_s } } let(:mutation) do 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 index 1c4439cec6f..542d51b990f 100644 --- 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 @@ -13,11 +13,12 @@ RSpec.describe 'Updating an incident timeline event' do end let(:occurred_at) { 1.minute.ago.iso8601 } + let(:note) { 'Updated note' } let(:variables) do { id: timeline_event.to_global_id.to_s, - note: 'Updated note', + note: note, occurred_at: occurred_at } end @@ -70,11 +71,18 @@ RSpec.describe 'Updating an incident timeline event' do 'id' => incident.to_global_id.to_s, 'title' => incident.title }, - 'note' => 'Updated note', + 'note' => note, 'noteHtml' => timeline_event.note_html, 'occurredAt' => occurred_at, 'createdAt' => timeline_event.created_at.iso8601, 'updatedAt' => timeline_event.updated_at.iso8601 ) end + + context 'when note is more than 280 characters long' do + let(:note) { 'n' * 281 } + + it_behaves_like 'timeline event mutation responds with validation error', + error_message: 'Timeline text is too long (maximum is 280 characters)' + end end diff --git a/spec/requests/api/graphql/mutations/incident_management/timeline_event_tag/create_spec.rb b/spec/requests/api/graphql/mutations/incident_management/timeline_event_tag/create_spec.rb new file mode 100644 index 00000000000..7476499d9da --- /dev/null +++ b/spec/requests/api/graphql/mutations/incident_management/timeline_event_tag/create_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Creating a timeline event tag' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:name) { 'Test tag 1' } + + let(:input) { { project_path: project.full_path, name: name } } + let(:mutation) do + graphql_mutation(:timeline_event_tag_create, input) do + <<~QL + clientMutationId + errors + timelineEventTag { + id + name + } + QL + end + end + + let(:mutation_response) { graphql_mutation_response(:timeline_event_tag_create) } + + context 'when user has permissions to create timeline event tag' do + before do + project.add_maintainer(user) + end + + it 'creates timeline event tag', :aggregate_failures do + post_graphql_mutation(mutation, current_user: user) + + timeline_event_tag_response = mutation_response['timelineEventTag'] + + expect(response).to have_gitlab_http_status(:success) + expect(timeline_event_tag_response).to include( + 'name' => name + ) + end + end + + context 'when user does not have permissions to create timeline event tag' do + before do + project.add_developer(user) + end + + it 'raises error' do + post_graphql_mutation(mutation, current_user: user) + + expect(mutation_response).to be_nil + expect_graphql_errors_to_include(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) + end + end +end diff --git a/spec/requests/api/graphql/mutations/issues/create_spec.rb b/spec/requests/api/graphql/mutations/issues/create_spec.rb index 9345735afe4..a489b7424e8 100644 --- a/spec/requests/api/graphql/mutations/issues/create_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/create_spec.rb @@ -58,34 +58,15 @@ RSpec.describe 'Create an issue' do input['type'] = 'TASK' end - context 'when work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) - end + it 'creates an issue with TASK type' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change(Issue, :count).by(1) - it 'creates an issue with the default ISSUE type' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.to change(Issue, :count).by(1) + created_issue = Issue.last - created_issue = Issue.last - - expect(created_issue.work_item_type.base_type).to eq('issue') - expect(created_issue.issue_type).to eq('issue') - end - end - - context 'when work_items feature flag is enabled' do - it 'creates an issue with TASK type' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.to change(Issue, :count).by(1) - - created_issue = Issue.last - - expect(created_issue.work_item_type.base_type).to eq('task') - expect(created_issue.issue_type).to eq('task') - end + expect(created_issue.work_item_type.base_type).to eq('task') + expect(created_issue.issue_type).to eq('task') end end diff --git a/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb b/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb index e7f4917ddde..c6a980b5cef 100644 --- a/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/create_from_task_spec.rb @@ -71,19 +71,5 @@ RSpec.describe "Create a work item from a task in a work item's description" do it_behaves_like 'has spam protection' do let(:mutation_class) { ::Mutations::WorkItems::CreateFromTask } 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/mutations/work_items/create_spec.rb b/spec/requests/api/graphql/mutations/work_items/create_spec.rb index 8233821053f..be3917316c3 100644 --- a/spec/requests/api/graphql/mutations/work_items/create_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/create_spec.rb @@ -154,17 +154,65 @@ RSpec.describe 'Create a work item' do end end - context 'when the work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) + context 'with milestone widget input' do + let(:widgets_response) { mutation_response['workItem']['widgets'] } + let(:fields) do + <<~FIELDS + workItem { + widgets { + type + ... on WorkItemWidgetMilestone { + milestone { + id + } + } + } + } + errors + FIELDS end - it 'does not create the work item and returns an error' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.to not_change(WorkItem, :count) + let(:mutation) { graphql_mutation(:workItemCreate, input.merge('projectPath' => project.full_path), fields) } - expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') + context 'when setting milestone on work item creation' do + let_it_be(:project_milestone) { create(:milestone, project: project) } + let_it_be(:group_milestone) { create(:milestone, project: project) } + + let(:input) do + { + title: 'some WI', + workItemTypeId: WorkItems::Type.default_by_type(:task).to_global_id.to_s, + milestoneWidget: { 'milestoneId' => milestone.to_global_id.to_s } + } + end + + shared_examples "work item's milestone is set" do + it "sets the work item's milestone" do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change(WorkItem, :count).by(1) + + expect(response).to have_gitlab_http_status(:success) + expect(widgets_response).to include( + { + 'type' => 'MILESTONE', + 'milestone' => { 'id' => milestone.to_global_id.to_s } + } + ) + end + end + + context 'when assigning a project milestone' do + it_behaves_like "work item's milestone is set" do + let(:milestone) { project_milestone } + end + end + + context 'when assigning a group milestone' do + it_behaves_like "work item's milestone is set" do + let(:milestone) { group_milestone } + end + end end end end diff --git a/spec/requests/api/graphql/mutations/work_items/delete_spec.rb b/spec/requests/api/graphql/mutations/work_items/delete_spec.rb index 14c8b757a57..0a84225a7ab 100644 --- a/spec/requests/api/graphql/mutations/work_items/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/delete_spec.rb @@ -31,19 +31,5 @@ RSpec.describe 'Delete a work item' do expect(response).to have_gitlab_http_status(:success) expect(mutation_response['project']).to include('id' => work_item.project.to_global_id.to_s) end - - context 'when the work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it 'does not delete the work item' 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/mutations/work_items/delete_task_spec.rb b/spec/requests/api/graphql/mutations/work_items/delete_task_spec.rb index e576d0ee7ef..c44939c8d54 100644 --- a/spec/requests/api/graphql/mutations/work_items/delete_task_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/delete_task_spec.rb @@ -75,19 +75,5 @@ RSpec.describe "Delete a task in a work item's description" do 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/mutations/work_items/update_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_spec.rb index 6b0129c457f..96736457f26 100644 --- a/spec/requests/api/graphql/mutations/work_items/update_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/update_spec.rb @@ -5,8 +5,11 @@ require 'spec_helper' RSpec.describe 'Update a work item' do include GraphqlHelpers - let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } + let_it_be(:reporter) { create(:user).tap { |user| project.add_reporter(user) } } + let_it_be(:guest) { create(:user).tap { |user| project.add_guest(user) } } let_it_be(:work_item, refind: true) { create(:work_item, project: project) } let(:work_item_event) { 'CLOSE' } @@ -543,6 +546,91 @@ RSpec.describe 'Update a work item' do end end + context 'when updating milestone' do + let_it_be(:project_milestone) { create(:milestone, project: project) } + let_it_be(:group_milestone) { create(:milestone, project: project) } + + let(:input) { { 'milestoneWidget' => { 'milestoneId' => new_milestone&.to_global_id&.to_s } } } + + let(:fields) do + <<~FIELDS + workItem { + widgets { + type + ... on WorkItemWidgetMilestone { + milestone { + id + } + } + } + } + errors + FIELDS + end + + shared_examples "work item's milestone is updated" do + it "updates the work item's milestone" do + expect do + post_graphql_mutation(mutation, current_user: current_user) + + work_item.reload + end.to change(work_item, :milestone).from(old_milestone).to(new_milestone) + + expect(response).to have_gitlab_http_status(:success) + end + end + + shared_examples "work item's milestone is not updated" do + it "ignores the update request" do + expect do + post_graphql_mutation(mutation, current_user: current_user) + + work_item.reload + end.to not_change(work_item, :milestone) + + expect(response).to have_gitlab_http_status(:success) + end + end + + context 'when user cannot set work item metadata' do + let(:current_user) { guest } + let(:old_milestone) { nil } + + it_behaves_like "work item's milestone is not updated" do + let(:new_milestone) { project_milestone } + end + end + + context 'when user can set work item metadata' do + let(:current_user) { reporter } + + context 'when assigning a project milestone' do + it_behaves_like "work item's milestone is updated" do + let(:old_milestone) { nil } + let(:new_milestone) { project_milestone } + end + end + + context 'when assigning a group milestone' do + it_behaves_like "work item's milestone is updated" do + let(:old_milestone) { nil } + let(:new_milestone) { group_milestone } + end + end + + context "when unsetting the work item's milestone" do + it_behaves_like "work item's milestone is updated" do + let(:old_milestone) { group_milestone } + let(:new_milestone) { nil } + + before do + work_item.update!(milestone: old_milestone) + end + end + end + end + end + context 'when unsupported widget input is sent' do let_it_be(:test_case) { create(:work_item_type, :default, :test_case, name: 'some_test_case_name') } let_it_be(:work_item) { create(:work_item, work_item_type: test_case, project: project) } @@ -556,20 +644,5 @@ RSpec.describe 'Update a work item' do it_behaves_like 'a mutation that returns top-level errors', errors: ["Following widget keys are not supported by some_test_case_name type: [:hierarchy_widget]"] end - - context 'when the work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it 'does not update the work item and returns and error' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - work_item.reload - end.to not_change(work_item, :title) - - 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/mutations/work_items/update_task_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_task_spec.rb index 32468a46ace..55285be5a5d 100644 --- a/spec/requests/api/graphql/mutations/work_items/update_task_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/update_task_spec.rb @@ -74,22 +74,6 @@ RSpec.describe 'Update a work item task' do it_behaves_like 'has spam protection' do let(:mutation_class) { ::Mutations::WorkItems::UpdateTask } end - - context 'when the work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it 'does not update the task item and returns and error' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - work_item.reload - task.reload - end.to not_change(task, :title) - - expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') - end - end end context 'when user does not have permissions to update a work item' do diff --git a/spec/requests/api/graphql/packages/package_spec.rb b/spec/requests/api/graphql/packages/package_spec.rb index e9f82d66775..02a3206f587 100644 --- a/spec/requests/api/graphql/packages/package_spec.rb +++ b/spec/requests/api/graphql/packages/package_spec.rb @@ -206,5 +206,25 @@ RSpec.describe 'package details' do expect(graphql_data_at(:package, :composer_config_repository_url)).to eq("localhost/#{group.id}") end end + + context 'web_path' do + before do + subject + end + + it 'returns web_path correctly' do + expect(graphql_data_at(:package, :_links, :web_path)).to eq("/#{project.full_path}/-/packages/#{composer_package.id}") + end + + context 'with terraform module' do + let_it_be(:terraform_package) { create(:terraform_module_package, project: project) } + + let(:package_global_id) { global_id_of(terraform_package) } + + it 'returns web_path correctly' do + expect(graphql_data_at(:package, :_links, :web_path)).to eq("/#{project.full_path}/-/infrastructure_registry/#{terraform_package.id}") + end + end + end end end diff --git a/spec/requests/api/graphql/project/branch_protections/merge_access_levels_spec.rb b/spec/requests/api/graphql/project/branch_protections/merge_access_levels_spec.rb index cb5006ec8e4..a80f683ea93 100644 --- a/spec/requests/api/graphql/project/branch_protections/merge_access_levels_spec.rb +++ b/spec/requests/api/graphql/project/branch_protections/merge_access_levels_spec.rb @@ -3,107 +3,5 @@ require 'spec_helper' RSpec.describe 'getting merge access levels for a branch protection' do - include GraphqlHelpers - - let_it_be(:current_user) { create(:user) } - - let(:merge_access_level_data) { merge_access_levels_data[0] } - - let(:merge_access_levels_data) do - graphql_data_at('project', - 'branchRules', - 'nodes', - 0, - 'branchProtection', - 'mergeAccessLevels', - 'nodes') - end - - let(:project) { protected_branch.project } - - let(:merge_access_levels_count) { protected_branch.merge_access_levels.size } - - let(:variables) { { path: project.full_path } } - - let(:fields) { all_graphql_fields_for('MergeAccessLevel') } - - let(:query) do - <<~GQL - query($path: ID!) { - project(fullPath: $path) { - branchRules(first: 1) { - nodes { - branchProtection { - mergeAccessLevels { - nodes { - #{fields} - } - } - } - } - } - } - } - GQL - end - - context 'when the user does not have read_protected_branch abilities' do - let_it_be(:protected_branch) { create(:protected_branch) } - - before do - project.add_guest(current_user) - post_graphql(query, current_user: current_user, variables: variables) - end - - it_behaves_like 'a working graphql query' - - it { expect(merge_access_levels_data).not_to be_present } - end - - shared_examples 'merge access request' do - let(:merge_access) { protected_branch.merge_access_levels.first } - - before do - project.add_maintainer(current_user) - post_graphql(query, current_user: current_user, variables: variables) - end - - it_behaves_like 'a working graphql query' - - it 'returns all merge access levels' do - expect(merge_access_levels_data.size).to eq(merge_access_levels_count) - end - - it 'includes access_level' do - expect(merge_access_level_data['accessLevel']) - .to eq(merge_access.access_level) - end - - it 'includes access_level_description' do - expect(merge_access_level_data['accessLevelDescription']) - .to eq(merge_access.humanize) - end - end - - context 'when the user does have read_protected_branch abilities' do - let(:merge_access) { protected_branch.merge_access_levels.first } - - context 'when no one has access' do - let_it_be(:protected_branch) { create(:protected_branch, :no_one_can_merge) } - - it_behaves_like 'merge access request' - end - - context 'when developers have access' do - let_it_be(:protected_branch) { create(:protected_branch, :developers_can_merge) } - - it_behaves_like 'merge access request' - end - - context 'when maintainers have access' do - let_it_be(:protected_branch) { create(:protected_branch, :maintainers_can_merge) } - - it_behaves_like 'merge access request' - end - end + include_examples 'perform graphql requests for AccessLevel type objects', :merge end diff --git a/spec/requests/api/graphql/project/branch_protections/push_access_levels_spec.rb b/spec/requests/api/graphql/project/branch_protections/push_access_levels_spec.rb index 59f9c7d61cb..cfdaf1096c3 100644 --- a/spec/requests/api/graphql/project/branch_protections/push_access_levels_spec.rb +++ b/spec/requests/api/graphql/project/branch_protections/push_access_levels_spec.rb @@ -3,107 +3,5 @@ require 'spec_helper' RSpec.describe 'getting push access levels for a branch protection' do - include GraphqlHelpers - - let_it_be(:current_user) { create(:user) } - - let(:push_access_level_data) { push_access_levels_data[0] } - - let(:push_access_levels_data) do - graphql_data_at('project', - 'branchRules', - 'nodes', - 0, - 'branchProtection', - 'pushAccessLevels', - 'nodes') - end - - let(:project) { protected_branch.project } - - let(:push_access_levels_count) { protected_branch.push_access_levels.size } - - let(:variables) { { path: project.full_path } } - - let(:fields) { all_graphql_fields_for('PushAccessLevel'.classify) } - - let(:query) do - <<~GQL - query($path: ID!) { - project(fullPath: $path) { - branchRules(first: 1) { - nodes { - branchProtection { - pushAccessLevels { - nodes { - #{fields} - } - } - } - } - } - } - } - GQL - end - - context 'when the user does not have read_protected_branch abilities' do - let_it_be(:protected_branch) { create(:protected_branch) } - - before do - project.add_guest(current_user) - post_graphql(query, current_user: current_user, variables: variables) - end - - it_behaves_like 'a working graphql query' - - it { expect(push_access_levels_data).not_to be_present } - end - - shared_examples 'push access request' do - let(:push_access) { protected_branch.push_access_levels.first } - - before do - project.add_maintainer(current_user) - post_graphql(query, current_user: current_user, variables: variables) - end - - it_behaves_like 'a working graphql query' - - it 'returns all push access levels' do - expect(push_access_levels_data.size).to eq(push_access_levels_count) - end - - it 'includes access_level' do - expect(push_access_level_data['accessLevel']) - .to eq(push_access.access_level) - end - - it 'includes access_level_description' do - expect(push_access_level_data['accessLevelDescription']) - .to eq(push_access.humanize) - end - end - - context 'when the user does have read_protected_branch abilities' do - let(:push_access) { protected_branch.push_access_levels.first } - - context 'when no one has access' do - let_it_be(:protected_branch) { create(:protected_branch, :no_one_can_push) } - - it_behaves_like 'push access request' - end - - context 'when developers have access' do - let_it_be(:protected_branch) { create(:protected_branch, :developers_can_push) } - - it_behaves_like 'push access request' - end - - context 'when maintainers have access' do - let_it_be(:protected_branch) { create(:protected_branch, :maintainers_can_push) } - - it_behaves_like 'push access request' - end - end + include_examples 'perform graphql requests for AccessLevel type objects', :push end diff --git a/spec/requests/api/graphql/project/branch_rules_spec.rb b/spec/requests/api/graphql/project/branch_rules_spec.rb index 1aaf0e9edc7..ed866305445 100644 --- a/spec/requests/api/graphql/project/branch_rules_spec.rb +++ b/spec/requests/api/graphql/project/branch_rules_spec.rb @@ -7,10 +7,9 @@ RSpec.describe 'getting list of branch rules for a project' do let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:current_user) { create(:user) } - let_it_be(:branch_name_a) { 'branch_name_a' } - let_it_be(:branch_name_b) { 'wildcard-*' } + let_it_be(:branch_name_a) { TestEnv::BRANCH_SHA.each_key.first } + let_it_be(:branch_name_b) { 'diff-*' } let_it_be(:branch_rules) { [branch_rule_a, branch_rule_b] } - let_it_be(:branch_rule_a) do create(:protected_branch, project: project, name: branch_name_a) end @@ -21,7 +20,6 @@ RSpec.describe 'getting list of branch rules for a project' do let(:branch_rules_data) { graphql_data_at('project', 'branchRules', 'edges') } let(:variables) { { path: project.full_path } } - # fields must use let as the all_graphql_fields_for also configures some spies let(:fields) { all_graphql_fields_for('BranchRule') } let(:query) do <<~GQL @@ -60,49 +58,124 @@ RSpec.describe 'getting list of branch rules for a project' do context 'when the user does have read_protected_branch abilities' do before do project.add_maintainer(current_user) - post_graphql(query, current_user: current_user, variables: variables) end - it_behaves_like 'a working graphql query' + describe 'queries' do + before do + # rubocop:disable RSpec/AnyInstanceOf + allow_any_instance_of(User).to receive(:update_tracked_fields!) + allow_any_instance_of(Users::ActivityService).to receive(:execute) + # rubocop:enable RSpec/AnyInstanceOf + allow_next_instance_of(Resolvers::ProjectResolver) do |resolver| + allow(resolver).to receive(:resolve) + .with(full_path: project.full_path) + .and_return(project) + end + allow(project.repository).to receive(:branch_names).and_call_original + allow(project.repository.gitaly_ref_client).to receive(:branch_names).and_call_original + end + + it 'matching_branches_count avoids N+1 queries' do + query = <<~GQL + query($path: ID!) { + project(fullPath: $path) { + branchRules { + nodes { + matchingBranchesCount + } + } + } + } + GQL + + control = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: current_user, variables: variables) + end + + # Verify the response includes the field + expect_n_matching_branches_count_fields(2) + + create(:protected_branch, project: project) + create(:protected_branch, name: '*', project: project) + + expect do + post_graphql(query, current_user: current_user, variables: variables) + end.not_to exceed_all_query_limit(control) + + expect(project.repository).to have_received(:branch_names).at_least(2).times + expect(project.repository.gitaly_ref_client).to have_received(:branch_names).once - it 'returns branch rules data' do - expect(branch_rules_data.dig(0, 'node', 'name')).to be_present - expect(branch_rules_data.dig(0, 'node', 'isDefault')).to be(true).or be(false) - expect(branch_rules_data.dig(0, 'node', 'branchProtection')).to be_present - expect(branch_rules_data.dig(0, 'node', 'createdAt')).to be_present - expect(branch_rules_data.dig(0, 'node', 'updatedAt')).to be_present - - expect(branch_rules_data.dig(1, 'node', 'name')).to be_present - expect(branch_rules_data.dig(1, 'node', 'isDefault')).to be(true).or be(false) - expect(branch_rules_data.dig(1, 'node', 'branchProtection')).to be_present - expect(branch_rules_data.dig(1, 'node', 'createdAt')).to be_present - expect(branch_rules_data.dig(1, 'node', 'updatedAt')).to be_present + expect_n_matching_branches_count_fields(4) + end + + def expect_n_matching_branches_count_fields(count) + branch_rule_nodes = graphql_data_at('project', 'branchRules', 'nodes') + expect(branch_rule_nodes.count).to eq(count) + branch_rule_nodes.each do |node| + expect(node['matchingBranchesCount']).to be_present + end + end end - context 'when limiting the number of results' do - let(:branch_rule_limit) { 1 } - let(:variables) { { path: project.full_path, n: branch_rule_limit } } - let(:next_variables) do - { path: project.full_path, n: branch_rule_limit, cursor: last_cursor } + describe 'response' do + before do + post_graphql(query, current_user: current_user, variables: variables) end it_behaves_like 'a working graphql query' - it 'returns pagination information' do - expect(branch_rules_data.size).to eq(branch_rule_limit) - expect(has_next_page).to be_truthy - expect(has_prev_page).to be_falsey - post_graphql(query, current_user: current_user, variables: next_variables) - expect(branch_rules_data.size).to eq(branch_rule_limit) - expect(has_next_page).to be_falsey - expect(has_prev_page).to be_truthy + it 'includes all fields', :aggregate_failures do + # Responses will be sorted alphabetically. Branch names for this spec + # come from an external constant so we check which is first + br_a_idx = branch_name_a < branch_name_b ? 0 : 1 + br_b_idx = 1 - br_a_idx + + branch_rule_a_data = branch_rules_data.dig(br_a_idx, 'node') + branch_rule_b_data = branch_rules_data.dig(br_b_idx, 'node') + + expect(branch_rule_a_data['name']).to eq(branch_name_a) + expect(branch_rule_a_data['isDefault']).to be(true).or be(false) + expect(branch_rule_a_data['branchProtection']).to be_present + expect(branch_rule_a_data['matchingBranchesCount']).to eq(1) + expect(branch_rule_a_data['createdAt']).to be_present + expect(branch_rule_a_data['updatedAt']).to be_present + + wildcard_count = TestEnv::BRANCH_SHA.keys.count do |branch_name| + branch_name.starts_with?('diff-') + end + expect(branch_rule_b_data['name']).to eq(branch_name_b) + expect(branch_rule_b_data['isDefault']).to be(true).or be(false) + expect(branch_rule_b_data['branchProtection']).to be_present + expect(branch_rule_b_data['matchingBranchesCount']).to eq(wildcard_count) + expect(branch_rule_b_data['createdAt']).to be_present + expect(branch_rule_b_data['updatedAt']).to be_present end - context 'when no limit is provided' do - let(:branch_rule_limit) { nil } + context 'when limiting the number of results' do + let(:branch_rule_limit) { 1 } + let(:variables) { { path: project.full_path, n: branch_rule_limit } } + let(:next_variables) do + { path: project.full_path, n: branch_rule_limit, cursor: last_cursor } + end + + it_behaves_like 'a working graphql query' + + it 'returns pagination information' do + expect(branch_rules_data.size).to eq(branch_rule_limit) + expect(has_next_page).to be_truthy + expect(has_prev_page).to be_falsey + post_graphql(query, current_user: current_user, variables: next_variables) + expect(branch_rules_data.size).to eq(branch_rule_limit) + expect(has_next_page).to be_falsey + expect(has_prev_page).to be_truthy + end + + context 'when no limit is provided' do + let(:branch_rule_limit) { nil } - it 'returns all branch_rules' do - expect(branch_rules_data.size).to eq(branch_rules.size) + it 'returns all branch_rules' do + expect(branch_rules_data.size).to eq(branch_rules.size) + end end end 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 index bcbb1f11d43..544d2d7bd95 100644 --- a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb +++ b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb @@ -48,6 +48,7 @@ RSpec.describe 'getting incident timeline events' do note noteHtml promotedFromNote { id body } + timelineEventTags { nodes { name } } editable action occurredAt @@ -100,6 +101,7 @@ RSpec.describe 'getting incident timeline events' do 'id' => promoted_from_note.to_global_id.to_s, 'body' => promoted_from_note.note }, + 'timelineEventTags' => { 'nodes' => [] }, 'editable' => true, 'action' => timeline_event.action, 'occurredAt' => timeline_event.occurred_at.iso8601, @@ -108,6 +110,47 @@ RSpec.describe 'getting incident timeline events' do ) end + context 'when timelineEvent tags are linked' do + let_it_be(:tag1) { create(:incident_management_timeline_event_tag, project: project, name: 'Tag 1') } + let_it_be(:tag2) { create(:incident_management_timeline_event_tag, project: project, name: 'Tag 2') } + let_it_be(:timeline_event_tag_link) do + create(:incident_management_timeline_event_tag_link, + timeline_event: timeline_event, + timeline_event_tag: tag1) + end + + it_behaves_like 'a working graphql query' + + it 'returns the set tags' do + expect(timeline_events.first['timelineEventTags']['nodes'].first['name']).to eq(tag1.name) + end + + context 'when different timeline events are loaded' do + it 'avoids N+1 queries' do + control = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: current_user) + end + + new_event = create(:incident_management_timeline_event, + incident: incident, + project: project, + updated_by_user: updated_by_user, + promoted_from_note: promoted_from_note, + note: "Referencing #{issue.to_reference(full: true)} - Full URL #{issue_url}" + ) + + create(:incident_management_timeline_event_tag_link, + timeline_event: new_event, + timeline_event_tag: tag2 + ) + + expect(incident.incident_management_timeline_events.length).to eq(3) + expect(post_graphql(query, current_user: current_user)).not_to exceed_query_limit(control) + expect(timeline_events.count).to eq(3) + end + end + 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 } } diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index 3b8beb4f798..214165cb171 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -8,84 +8,74 @@ RSpec.describe 'getting an issue list for a project' do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, :public, group: group) } let_it_be(:current_user) { create(:user) } - let_it_be(:issue_a, reload: true) { create(:issue, project: project, discussion_locked: true) } - let_it_be(:issue_b, reload: true) { create(:issue, :with_alert, project: project) } - let_it_be(:issues, reload: true) { [issue_a, issue_b] } + let_it_be(:another_user) { create(:user).tap { |u| group.add_reporter(u) } } + let_it_be(:early_milestone) { create(:milestone, project: project, due_date: 10.days.from_now) } + let_it_be(:late_milestone) { create(:milestone, project: project, due_date: 30.days.from_now) } + let_it_be(:priority1) { create(:label, project: project, priority: 1) } + let_it_be(:priority2) { create(:label, project: project, priority: 5) } + let_it_be(:priority3) { create(:label, project: project, priority: 10) } + + let_it_be(:issue_a, reload: true) { create(:issue, project: project, discussion_locked: true, labels: [priority3]) } + let_it_be(:issue_b, reload: true) { create(:issue, :with_alert, project: project, title: 'title matching issue i') } + let_it_be(:issue_c) { create(:issue, project: project, labels: [priority1], milestone: late_milestone) } + let_it_be(:issue_d) { create(:issue, project: project, labels: [priority2]) } + let_it_be(:issue_e) { create(:issue, project: project, milestone: early_milestone) } + let_it_be(:issues, reload: true) { [issue_a, issue_b, issue_c, issue_d, issue_e] } let(:issue_a_gid) { issue_a.to_global_id.to_s } let(:issue_b_gid) { issue_b.to_global_id.to_s } - let(:issues_data) { graphql_data['project']['issues']['edges'] } + let(:issues_data) { graphql_data['project']['issues']['nodes'] } let(:issue_filter_params) { {} } let(:fields) do <<~QUERY - edges { - node { - #{all_graphql_fields_for('issues'.classify)} - } + nodes { + #{all_graphql_fields_for('issues'.classify)} } QUERY end - it_behaves_like 'a working graphql query' do - before do - post_graphql(query, current_user: current_user) - end - end - - it 'includes a web_url' do - post_graphql(query, current_user: current_user) - - expect(issues_data[0]['node']['webUrl']).to be_present - end - - it 'includes discussion locked' do - post_graphql(query, current_user: current_user) - - expect(issues_data[0]['node']['discussionLocked']).to eq(false) - expect(issues_data[1]['node']['discussionLocked']).to eq(true) - end - - context 'when both assignee_username filters are provided' do - let(:issue_filter_params) { { assignee_username: current_user.username, assignee_usernames: [current_user.username] } } - - it 'returns a mutually exclusive param error' do - post_graphql(query, current_user: current_user) - - expect_graphql_errors_to_include('only one of [assigneeUsernames, assigneeUsername] arguments is allowed at the same time.') - end - end - - context 'filtering by my_reaction_emoji' do - using RSpec::Parameterized::TableSyntax - - let_it_be(:upvote_award) { create(:award_emoji, :upvote, user: current_user, awardable: issue_a) } - - where(:value, :gids) do - 'thumbsup' | lazy { [issue_a_gid] } - 'ANY' | lazy { [issue_a_gid] } - 'any' | lazy { [issue_a_gid] } - 'AnY' | lazy { [issue_a_gid] } - 'NONE' | lazy { [issue_b_gid] } - 'thumbsdown' | lazy { [] } + # All new specs should be added to the shared example if the change also + # affects the `issues` query at the root level of the API. + # Shared example also used in spec/requests/api/graphql/issues_spec.rb + it_behaves_like 'graphql issue list request spec' do + subject(:post_query) { post_graphql(query, current_user: current_user) } + + # filters + let(:expected_negated_assignee_issues) { [issue_b, issue_c, issue_d, issue_e] } + let(:expected_unioned_assignee_issues) { [issue_a, issue_b] } + let(:voted_issues) { [issue_a] } + let(:no_award_issues) { [issue_b, issue_c, issue_d, issue_e] } + let(:locked_discussion_issues) { [issue_a] } + let(:unlocked_discussion_issues) { [issue_b, issue_c, issue_d, issue_e] } + let(:search_title_term) { 'matching issue' } + let(:title_search_issue) { issue_b } + + # sorting + let(:data_path) { [:project, :issues] } + let(:expected_severity_sorted_asc) { [issue_c, issue_a, issue_b, issue_e, issue_d] } + let(:expected_priority_sorted_asc) { [issue_e, issue_c, issue_d, issue_a, issue_b] } + let(:expected_priority_sorted_desc) { [issue_c, issue_e, issue_a, issue_d, issue_b] } + + before_all do + issue_a.assignee_ids = current_user.id + issue_b.assignee_ids = another_user.id + + create(:award_emoji, :upvote, user: current_user, awardable: issue_a) + + # severity sorting + create(:issuable_severity, issue: issue_a, severity: :unknown) + create(:issuable_severity, issue: issue_b, severity: :low) + create(:issuable_severity, issue: issue_d, severity: :critical) + create(:issuable_severity, issue: issue_e, severity: :high) end - with_them do - let(:issue_filter_params) { { my_reaction_emoji: value } } - - it 'returns correctly filtered issues' do - post_graphql(query, current_user: current_user) - - expect(issues_ids).to eq(gids) - end - end - end - - context 'when filtering by search' do - it_behaves_like 'query with a search term' do - let(:issuable_data) { issues_data } - let(:user) { current_user } - let_it_be(:issuable) { create(:issue, project: project, description: 'bar') } + def pagination_query(params) + graphql_query_for( + :project, + { full_path: project.full_path }, + query_graphql_field(:issues, params, "#{page_info} nodes { id }") + ) end end @@ -155,10 +145,10 @@ RSpec.describe 'getting an issue list for a project' do it 'returns issues without confidential issues' do post_graphql(query, current_user: current_user) - expect(issues_data.size).to eq(2) + expect(issues_data.size).to eq(5) issues_data.each do |issue| - expect(issue.dig('node', 'confidential')).to eq(false) + expect(issue['confidential']).to eq(false) end end @@ -178,7 +168,7 @@ RSpec.describe 'getting an issue list for a project' do it 'returns correctly filtered issues' do post_graphql(query, current_user: current_user) - expect(issues_ids).to contain_exactly(issue_a_gid, issue_b_gid) + expect(issue_ids).to match_array(issues.map { |i| i.to_gid.to_s }) end end end @@ -191,13 +181,13 @@ RSpec.describe 'getting an issue list for a project' do it 'returns issues with confidential issues' do post_graphql(query, current_user: current_user) - expect(issues_data.size).to eq(3) + expect(issues_data.size).to eq(6) confidentials = issues_data.map do |issue| - issue.dig('node', 'confidential') + issue['confidential'] end - expect(confidentials).to eq([true, false, false]) + expect(confidentials).to contain_exactly(true, false, false, false, false, false) end context 'filtering for confidential issues' do @@ -206,7 +196,7 @@ RSpec.describe 'getting an issue list for a project' do it 'returns correctly filtered issues' do post_graphql(query, current_user: current_user) - expect(issues_ids).to contain_exactly(confidential_issue_gid) + expect(issue_ids).to contain_exactly(confidential_issue_gid) end end @@ -216,7 +206,7 @@ RSpec.describe 'getting an issue list for a project' do it 'returns correctly filtered issues' do post_graphql(query, current_user: current_user) - expect(issues_ids).to contain_exactly(issue_a_gid, issue_b_gid) + expect(issue_ids).to match_array([issue_a, issue_b, issue_c, issue_d, issue_e].map { |i| i.to_gid.to_s }) end end end @@ -238,37 +228,7 @@ RSpec.describe 'getting an issue list for a project' do data.map { |issue| issue['iid'].to_i } end - context 'when sorting by severity' do - let_it_be(:severty_issue1) { create(:issue, project: sort_project) } - let_it_be(:severty_issue2) { create(:issue, project: sort_project) } - let_it_be(:severty_issue3) { create(:issue, project: sort_project) } - let_it_be(:severty_issue4) { create(:issue, project: sort_project) } - let_it_be(:severty_issue5) { create(:issue, project: sort_project) } - - before(:all) do - create(:issuable_severity, issue: severty_issue1, severity: :unknown) - create(:issuable_severity, issue: severty_issue2, severity: :low) - create(:issuable_severity, issue: severty_issue4, severity: :critical) - create(:issuable_severity, issue: severty_issue5, severity: :high) - end - - context 'when ascending' do - it_behaves_like 'sorted paginated query' do - let(:sort_param) { :SEVERITY_ASC } - let(:first_param) { 2 } - let(:all_records) { [severty_issue3.iid, severty_issue1.iid, severty_issue2.iid, severty_issue5.iid, severty_issue4.iid] } - end - end - - context 'when descending' do - it_behaves_like 'sorted paginated query' do - let(:sort_param) { :SEVERITY_DESC } - let(:first_param) { 2 } - let(:all_records) { [severty_issue4.iid, severty_issue5.iid, severty_issue2.iid, severty_issue1.iid, severty_issue3.iid] } - end - end - end - + # rubocop:disable RSpec/MultipleMemoizedHelpers context 'when sorting by due date' do let_it_be(:due_issue1) { create(:issue, project: sort_project, due_date: 3.days.from_now) } let_it_be(:due_issue2) { create(:issue, project: sort_project, due_date: nil) } @@ -314,41 +274,6 @@ RSpec.describe 'getting an issue list for a project' do end end - context 'when sorting by priority' do - let_it_be(:on_project) { { project: sort_project } } - let_it_be(:early_milestone) { create(:milestone, **on_project, due_date: 10.days.from_now) } - let_it_be(:late_milestone) { create(:milestone, **on_project, due_date: 30.days.from_now) } - let_it_be(:priority_1) { create(:label, **on_project, priority: 1) } - let_it_be(:priority_2) { create(:label, **on_project, priority: 5) } - let_it_be(:priority_issue1) { create(:issue, **on_project, labels: [priority_1], milestone: late_milestone) } - let_it_be(:priority_issue2) { create(:issue, **on_project, labels: [priority_2]) } - let_it_be(:priority_issue3) { create(:issue, **on_project, milestone: early_milestone) } - let_it_be(:priority_issue4) { create(:issue, **on_project) } - - context 'when ascending' do - it_behaves_like 'sorted paginated query' do - let(:sort_param) { :PRIORITY_ASC } - let(:first_param) { 2 } - let(:all_records) do - [ - priority_issue3.iid, priority_issue1.iid, - priority_issue2.iid, priority_issue4.iid - ] - end - end - end - - context 'when descending' do - it_behaves_like 'sorted paginated query' do - let(:sort_param) { :PRIORITY_DESC } - let(:first_param) { 2 } - let(:all_records) do - [priority_issue1.iid, priority_issue3.iid, priority_issue2.iid, priority_issue4.iid] - end - end - end - end - context 'when sorting by label priority' do let_it_be(:label1) { create(:label, project: sort_project, priority: 1) } let_it_be(:label2) { create(:label, project: sort_project, priority: 5) } @@ -374,6 +299,7 @@ RSpec.describe 'getting an issue list for a project' do end end end + # rubocop:enable RSpec/MultipleMemoizedHelpers context 'when sorting by milestone due date' do let_it_be(:early_milestone) { create(:milestone, project: sort_project, due_date: 10.days.from_now) } @@ -403,14 +329,17 @@ RSpec.describe 'getting an issue list for a project' do context 'when fetching alert management alert' do let(:fields) do <<~QUERY - edges { - node { + nodes { iid alertManagementAlert { title } + alertManagementAlerts { + nodes { + title + } + } } - } QUERY end @@ -430,11 +359,22 @@ RSpec.describe 'getting an issue list for a project' do it 'returns the alert data' do post_graphql(query, current_user: current_user) - alert_titles = issues_data.map { |issue| issue.dig('node', 'alertManagementAlert', 'title') } + alert_titles = issues_data.map { |issue| issue.dig('alertManagementAlert', 'title') } expected_titles = issues.map { |issue| issue.alert_management_alert&.title } expect(alert_titles).to contain_exactly(*expected_titles) end + + it 'returns the alerts data' do + post_graphql(query, current_user: current_user) + + alert_titles = issues_data.map { |issue| issue.dig('alertManagementAlerts', 'nodes') } + expected_titles = issues.map do |issue| + issue.alert_management_alerts.map { |alert| { 'title' => alert.title } } + end + + expect(alert_titles).to contain_exactly(*expected_titles) + end end context 'when fetching customer_relations_contacts' do @@ -469,13 +409,11 @@ RSpec.describe 'getting an issue list for a project' do context 'when fetching labels' do let(:fields) do <<~QUERY - edges { - node { - id - labels { - nodes { - id - } + nodes { + id + labels { + nodes { + id } } } @@ -491,8 +429,8 @@ RSpec.describe 'getting an issue list for a project' do end def response_label_ids(response_data) - response_data.map do |edge| - edge['node']['labels']['nodes'].map { |u| u['id'] } + response_data.map do |node| + node['labels']['nodes'].map { |u| u['id'] } end.flatten end @@ -502,7 +440,7 @@ RSpec.describe 'getting an issue list for a project' do it 'avoids N+1 queries', :aggregate_failures do control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - expect(issues_data.count).to eq(2) + expect(issues_data.count).to eq(5) expect(response_label_ids(issues_data)).to match_array(labels_as_global_ids(issues)) new_issues = issues + [create(:issue, project: project, labels: [create(:label, project: project)])] @@ -510,8 +448,8 @@ RSpec.describe 'getting an issue list for a project' do expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control) # graphql_data is memoized (see spec/support/helpers/graphql_helpers.rb) # so we have to parse the body ourselves the second time - issues_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['edges'] - expect(issues_data.count).to eq(3) + issues_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['nodes'] + expect(issues_data.count).to eq(6) expect(response_label_ids(issues_data)).to match_array(labels_as_global_ids(new_issues)) end end @@ -519,13 +457,11 @@ RSpec.describe 'getting an issue list for a project' do context 'when fetching assignees' do let(:fields) do <<~QUERY - edges { - node { - id - assignees { - nodes { - id - } + nodes { + id + assignees { + nodes { + id } } } @@ -541,8 +477,8 @@ RSpec.describe 'getting an issue list for a project' do end def response_assignee_ids(response_data) - response_data.map do |edge| - edge['node']['assignees']['nodes'].map { |node| node['id'] } + response_data.map do |node| + node['assignees']['nodes'].map { |node| node['id'] } end.flatten end @@ -552,7 +488,7 @@ RSpec.describe 'getting an issue list for a project' do it 'avoids N+1 queries', :aggregate_failures do control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - expect(issues_data.count).to eq(2) + expect(issues_data.count).to eq(5) expect(response_assignee_ids(issues_data)).to match_array(assignees_as_global_ids(issues)) new_issues = issues + [create(:issue, project: project, assignees: [create(:user)])] @@ -560,8 +496,8 @@ RSpec.describe 'getting an issue list for a project' do expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control) # graphql_data is memoized (see spec/support/helpers/graphql_helpers.rb) # so we have to parse the body ourselves the second time - issues_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['edges'] - expect(issues_data.count).to eq(3) + issues_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['nodes'] + expect(issues_data.count).to eq(6) expect(response_assignee_ids(issues_data)).to match_array(assignees_as_global_ids(new_issues)) end end @@ -572,11 +508,9 @@ RSpec.describe 'getting an issue list for a project' do let(:statuses) { issue_data.to_h { |issue| [issue['iid'], issue['escalationStatus']] } } let(:fields) do <<~QUERY - edges { - node { - id - escalationStatus - } + nodes { + id + escalationStatus } QUERY end @@ -588,9 +522,9 @@ RSpec.describe 'getting an issue list for a project' do it 'returns the escalation status values' do post_graphql(query, current_user: current_user) - statuses = issues_data.map { |issue| issue.dig('node', 'escalationStatus') } + statuses = issues_data.map { |issue| issue['escalationStatus'] } - expect(statuses).to contain_exactly(escalation_status.status_name.upcase.to_s, nil) + expect(statuses).to contain_exactly(escalation_status.status_name.upcase.to_s, nil, nil, nil, nil) end it 'avoids N+1 queries', :aggregate_failures do @@ -726,8 +660,8 @@ RSpec.describe 'getting an issue list for a project' do end end - def issues_ids - graphql_dig_at(issues_data, :node, :id) + def issue_ids + graphql_dig_at(issues_data, :id) end def query(params = issue_filter_params) diff --git a/spec/requests/api/graphql/project/languages_spec.rb b/spec/requests/api/graphql/project/languages_spec.rb new file mode 100644 index 00000000000..6ef500cde41 --- /dev/null +++ b/spec/requests/api/graphql/project/languages_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Project.languages' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:query) do + %( + query { + project(fullPath: "#{project.full_path}") { + languages { + name + share + color + } + } + } + ) + end + + let_it_be(:test_languages) do + [{ value: 66.69, label: "Ruby", color: "#701516", highlight: "#701516" }, + { value: 22.98, label: "JavaScript", color: "#f1e05a", highlight: "#f1e05a" }, + { value: 7.91, label: "HTML", color: "#e34c26", highlight: "#e34c26" }, + { value: 2.42, label: "CoffeeScript", color: "#244776", highlight: "#244776" }] + end + + let_it_be(:expected_languages) do + test_languages.map { |lang| { 'name' => lang[:label], 'share' => lang[:value], 'color' => lang[:color] } } + end + + before do + allow(project.repository).to receive(:languages).and_return(test_languages) + end + + context "when the languages haven't been detected yet" do + it 'returns expected languages on second request as detection is done asynchronously', :sidekiq_inline do + post_graphql(query, current_user: user) + + expect(graphql_data_at(:project, :languages)).to eq([]) + + post_graphql(query, current_user: user) + + expect(graphql_data_at(:project, :languages)).to eq(expected_languages) + end + end + + context 'when the languages were detected before' do + before do + Projects::DetectRepositoryLanguagesService.new(project, project.first_owner).execute + end + + it 'returns repository languages' do + post_graphql(query, current_user: user) + + expect(graphql_data_at(:project, :languages)).to eq(expected_languages) + end + end +end diff --git a/spec/requests/api/graphql/project/tree/tree_spec.rb b/spec/requests/api/graphql/project/tree/tree_spec.rb index 25e878a5b1a..e63e0d3dd04 100644 --- a/spec/requests/api/graphql/project/tree/tree_spec.rb +++ b/spec/requests/api/graphql/project/tree/tree_spec.rb @@ -4,7 +4,8 @@ require 'spec_helper' RSpec.describe 'getting a tree in a project' do include GraphqlHelpers - let(:project) { create(:project, :repository) } + let_it_be(:project) { create(:project, :repository) } + let(:current_user) { project.first_owner } let(:path) { "" } let(:ref) { "master" } @@ -82,6 +83,89 @@ RSpec.describe 'getting a tree in a project' do end end + context 'when the ref points to a gpg-signed commit with a user' do + let_it_be(:name) { GpgHelpers::User1.names.first } + let_it_be(:email) { GpgHelpers::User1.emails.first } + let_it_be(:current_user) { create(:user, name: name, email: email).tap { |user| project.add_owner(user) } } + let_it_be(:gpg_key) { create(:gpg_key, user: current_user, key: GpgHelpers::User1.public_key) } + + let(:ref) { GpgHelpers::SIGNED_AND_AUTHORED_SHA } + let(:fields) do + <<~QUERY + tree(path:"#{path}", ref:"#{ref}") { + lastCommit { + signature { + ... on GpgSignature { + #{all_graphql_fields_for('GpgSignature'.classify, max_depth: 2)} + } + } + } + } + QUERY + end + + before do + post_graphql(query, current_user: current_user) + end + + it 'returns the expected signature data' do + signature = graphql_data['project']['repository']['tree']['lastCommit']['signature'] + expect(signature['commitSha']).to eq(ref) + expect(signature['user']['id']).to eq("gid://gitlab/User/#{current_user.id}") + expect(signature['gpgKeyUserName']).to eq(name) + expect(signature['gpgKeyUserEmail']).to eq(email) + expect(signature['verificationStatus']).to eq('VERIFIED') + expect(signature['project']['id']).to eq("gid://gitlab/Project/#{project.id}") + end + end + + context 'when the ref points to a X.509-signed commit' do + let_it_be(:email) { X509Helpers::User1.certificate_email } + let_it_be(:current_user) { create(:user, email: email).tap { |user| project.add_owner(user) } } + + let(:ref) { X509Helpers::User1.commit } + let(:fields) do + <<~QUERY + tree(path:"#{path}", ref:"#{ref}") { + lastCommit { + signature { + ... on X509Signature { + #{all_graphql_fields_for('X509Signature'.classify, max_depth: 2)} + } + } + } + } + QUERY + end + + before do + store = OpenSSL::X509::Store.new + store.add_cert(OpenSSL::X509::Certificate.new(X509Helpers::User1.trust_cert)) + allow(OpenSSL::X509::Store).to receive(:new).and_return(store) + post_graphql(query, current_user: current_user) + end + + it 'returns the expected signature data' do + signature = graphql_data['project']['repository']['tree']['lastCommit']['signature'] + expect(signature['commitSha']).to eq(ref) + expect(signature['verificationStatus']).to eq('VERIFIED') + expect(signature['project']['id']).to eq("gid://gitlab/Project/#{project.id}") + end + + it 'returns expected certificate data' do + signature = graphql_data['project']['repository']['tree']['lastCommit']['signature'] + certificate = signature['x509Certificate'] + expect(certificate['certificateStatus']).to eq('good') + expect(certificate['email']).to eq(X509Helpers::User1.certificate_email) + expect(certificate['id']).to be_present + expect(certificate['serialNumber']).to eq(X509Helpers::User1.certificate_serial.to_s) + expect(certificate['subject']).to eq(X509Helpers::User1.certificate_subject) + expect(certificate['subjectKeyIdentifier']).to eq(X509Helpers::User1.certificate_subject_key_identifier) + expect(certificate['createdAt']).to be_present + expect(certificate['updatedAt']).to be_present + end + end + context 'when current user is nil' do it 'returns empty project' do post_graphql(query, current_user: nil) diff --git a/spec/requests/api/graphql/project/work_item_types_spec.rb b/spec/requests/api/graphql/project/work_item_types_spec.rb index 157961c3f66..3d30baab816 100644 --- a/spec/requests/api/graphql/project/work_item_types_spec.rb +++ b/spec/requests/api/graphql/project/work_item_types_spec.rb @@ -57,15 +57,4 @@ RSpec.describe 'getting a list of work item types for a project' do expect(graphql_data).to eq('project' => nil) end end - - context 'when the work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) - post_graphql(query, current_user: current_user) - end - - it 'returns null' do - expect(graphql_data.dig('project', 'workItemTypes')).to be_nil - end - end end diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index e82f6ad24a2..6d20799c9ec 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -10,6 +10,8 @@ RSpec.describe 'getting a work item list for a project' do let_it_be(:current_user) { create(:user) } let_it_be(:label1) { create(:label, project: project) } let_it_be(:label2) { create(:label, project: project) } + let_it_be(:milestone1) { create(:milestone, project: project) } + let_it_be(:milestone2) { create(:milestone, project: project) } let_it_be(:item1) { create(:work_item, project: project, discussion_locked: true, title: 'item1', labels: [label1]) } let_it_be(:item2) do @@ -19,7 +21,8 @@ RSpec.describe 'getting a work item list for a project' do title: 'item2', last_edited_by: current_user, last_edited_at: 1.day.ago, - labels: [label2] + labels: [label2], + milestone: milestone1 ) end @@ -55,7 +58,8 @@ RSpec.describe 'getting a work item list for a project' do :last_edited_by_user, last_edited_at: 1.week.ago, project: project, - labels: [label1, label2] + labels: [label1, label2], + milestone: milestone2 ) expect_graphql_errors_to_be_empty @@ -94,6 +98,11 @@ RSpec.describe 'getting a work item list for a project' do labels { nodes { id } } allowsScopedLabels } + ... on WorkItemWidgetMilestone { + milestone { + id + } + } } } GRAPHQL @@ -121,18 +130,6 @@ RSpec.describe 'getting a work item list for a project' do end end - context 'when work_items flag is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it 'returns an empty list' do - post_graphql(query) - - expect(items_data).to eq([]) - end - end - it 'returns only items visible to user' do post_graphql(query, current_user: current_user) diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index 2105e479ed2..a55de6adfb2 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -298,6 +298,40 @@ RSpec.describe 'Query.work_item(id)' do ) end end + + describe 'milestone widget' do + let_it_be(:milestone) { create(:milestone, project: project) } + + let(:work_item) { create(:work_item, project: project, milestone: milestone) } + + let(:work_item_fields) do + <<~GRAPHQL + id + widgets { + type + ... on WorkItemWidgetMilestone { + milestone { + id + } + } + } + GRAPHQL + end + + it 'returns widget information' do + expect(work_item_data).to include( + 'id' => work_item.to_gid.to_s, + 'widgets' => include( + hash_including( + 'type' => 'MILESTONE', + 'milestone' => { + 'id' => work_item.milestone.to_gid.to_s + } + ) + ) + ) + end + end end context 'when an Issue Global ID is provided' do @@ -323,16 +357,4 @@ RSpec.describe 'Query.work_item(id)' do ) end end - - context 'when the work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it 'returns nil' do - post_graphql(query) - - expect(work_item_data).to be_nil - end - end end diff --git a/spec/requests/api/group_boards_spec.rb b/spec/requests/api/group_boards_spec.rb index 6ce8b766807..cc110aa4017 100644 --- a/spec/requests/api/group_boards_spec.rb +++ b/spec/requests/api/group_boards_spec.rb @@ -13,7 +13,7 @@ RSpec.describe API::GroupBoards do board_parent.add_owner(user) end - let_it_be(:project) { create(:project, :public, namespace: board_parent ) } + let_it_be(:project) { create(:project, :public, namespace: board_parent) } let_it_be(:dev_label) do create(:group_label, title: 'Development', color: '#FFAABB', group: board_parent) diff --git a/spec/requests/api/group_container_repositories_spec.rb b/spec/requests/api/group_container_repositories_spec.rb index 413c37eaed9..82daab0e5e8 100644 --- a/spec/requests/api/group_container_repositories_spec.rb +++ b/spec/requests/api/group_container_repositories_spec.rb @@ -57,5 +57,13 @@ RSpec.describe API::GroupContainerRepositories do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'with URL-encoded path of the group' do + let(:url) { "/groups/#{group.full_path}/registry/repositories" } + + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + it_behaves_like 'returns repositories for allowed users', :reporter + end end end diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 4fed7dd7624..a07a8ae4292 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -90,7 +90,7 @@ RSpec.describe API::GroupVariables do it 'creates variable' do expect do - post api("/groups/#{group.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true } + post api("/groups/#{group.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true, raw: true } end.to change { group.variables.count }.by(1) expect(response).to have_gitlab_http_status(:created) @@ -100,6 +100,16 @@ RSpec.describe API::GroupVariables do expect(json_response['masked']).to be_truthy expect(json_response['variable_type']).to eq('env_var') expect(json_response['environment_scope']).to eq('*') + expect(json_response['raw']).to be_truthy + end + + it 'masks the new value when logging' do + masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' } + + expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) + + post api("/groups/#{group.id}/variables", user), + params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true } end it 'creates variable with optional attributes' do @@ -112,6 +122,7 @@ RSpec.describe API::GroupVariables do expect(json_response['value']).to eq('VALUE_2') expect(json_response['protected']).to be_falsey expect(json_response['masked']).to be_falsey + expect(json_response['raw']).to be_falsey expect(json_response['variable_type']).to eq('file') expect(json_response['environment_scope']).to eq('*') end @@ -152,7 +163,7 @@ RSpec.describe API::GroupVariables do initial_variable = group.variables.reload.first value_before = initial_variable.value - put api("/groups/#{group.id}/variables/#{variable.key}", user), params: { variable_type: 'file', value: 'VALUE_1_UP', protected: true, masked: true } + put api("/groups/#{group.id}/variables/#{variable.key}", user), params: { variable_type: 'file', value: 'VALUE_1_UP', protected: true, masked: true, raw: true } updated_variable = group.variables.reload.first @@ -162,6 +173,16 @@ RSpec.describe API::GroupVariables do expect(updated_variable).to be_protected expect(json_response['variable_type']).to eq('file') expect(json_response['masked']).to be_truthy + expect(json_response['raw']).to be_truthy + end + + it 'masks the new value when logging' do + masked_params = { 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' } + + expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) + + put api("/groups/#{group.id}/variables/#{variable.key}", user), + params: { value: 'SENSITIVE', protected: true, masked: true } end it 'responds with 404 Not Found if requesting non-existing variable' do diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 02d29601ceb..ce6140d8da8 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -91,11 +91,8 @@ RSpec.describe API::Groups do .to satisfy_one { |group| group['name'] == group1.name } end - it 'avoids N+1 queries' do - # Establish baseline - get api("/groups", admin) - - control = ActiveRecord::QueryRecorder.new do + it 'avoids N+1 queries', :use_sql_query_cache do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/groups", admin) end @@ -103,7 +100,7 @@ RSpec.describe API::Groups do expect do get api("/groups", admin) - end.not_to exceed_query_limit(control) + end.not_to exceed_all_query_limit(control) end context 'when statistics are requested' do @@ -1230,10 +1227,7 @@ RSpec.describe API::Groups do group1.reload end - it "only looks up root ancestor once and returns projects including those in subgroups" do - expect(Namespace).to receive(:find_by).with(id: group1.id.to_s).once.and_call_original # For the group sent in the API call - expect(Namespace).to receive(:joins).with(start_with('INNER JOIN (SELECT id, traversal_ids[1]')).once.and_call_original # All-in-one root_ancestor query - + it "returns projects including those in subgroups" do get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } expect(response).to have_gitlab_http_status(:ok) @@ -1241,6 +1235,18 @@ RSpec.describe API::Groups do expect(json_response).to be_an(Array) expect(json_response.length).to eq(6) end + + it 'avoids N+1 queries', :use_sql_query_cache do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } + end + + create_list(:project, 2, :public, namespace: group1) + + expect do + get api("/groups/#{group1.id}/projects", user1), params: { include_subgroups: true } + end.not_to exceed_all_query_limit(control.count) + end end context 'when include_ancestor_groups is true' do diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb index 015a09d41ab..4f95295c14d 100644 --- a/spec/requests/api/import_github_spec.rb +++ b/spec/requests/api/import_github_spec.rb @@ -89,6 +89,18 @@ RSpec.describe API::ImportGithub do expect(response).to have_gitlab_http_status(:unprocessable_entity) end + + context 'when unauthenticated user' do + it 'returns 403 response' do + post api("/import/github"), params: { + target_namespace: user.namespace_path, + personal_access_token: token, + repo_id: non_existing_record_id + } + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end end describe "POST /import/github/cancel" do @@ -127,5 +139,15 @@ RSpec.describe API::ImportGithub do expect(json_response['message']).to eq('The import cannot be canceled because it is finished') end end + + context 'when unauthenticated user' do + it 'returns 403 response' do + post api("/import/github/cancel"), params: { + project_id: project.id + } + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end end end diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index 67d8a18dfd8..3c6604cf409 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -69,48 +69,6 @@ RSpec.describe API::Internal::Kubernetes do context 'is authenticated for an agent' do let!(:agent_token) { create(:cluster_agent_token) } - # Todo: Remove gitops_sync_count and k8s_api_proxy_request_count in the next milestone - # https://gitlab.com/gitlab-org/gitlab/-/issues/369489 - # We're only keeping it for backwards compatibility until KAS is released - # using `counts:` instead - context 'deprecated events' do - it 'returns no_content for valid events' do - send_request(params: { gitops_sync_count: 10, k8s_api_proxy_request_count: 5 }) - - expect(response).to have_gitlab_http_status(:no_content) - end - - it 'returns no_content for counts of zero' do - send_request(params: { gitops_sync_count: 0, k8s_api_proxy_request_count: 0 }) - - expect(response).to have_gitlab_http_status(:no_content) - end - - it 'returns 400 for non number' do - send_request(params: { gitops_sync_count: 'string', k8s_api_proxy_request_count: 1 }) - - expect(response).to have_gitlab_http_status(:bad_request) - end - - it 'returns 400 for negative number' do - send_request(params: { gitops_sync_count: -1, k8s_api_proxy_request_count: 1 }) - - expect(response).to have_gitlab_http_status(:bad_request) - end - - it 'tracks events' do - counters = { gitops_sync_count: 10, k8s_api_proxy_request_count: 5 } - expected_counters = { - kubernetes_agent_gitops_sync: counters[:gitops_sync_count], - kubernetes_agent_k8s_api_proxy_request: counters[:k8s_api_proxy_request_count] - } - - send_request(params: counters) - - expect(Gitlab::UsageDataCounters::KubernetesAgentCounter.totals).to eq(expected_counters) - end - end - it 'returns no_content for valid events' do counters = { gitops_sync: 10, k8s_api_proxy_request: 5 } unique_counters = { agent_users_using_ci_tunnel: [10] } diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index a795b49c44e..c07d2e11363 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe API::Invitations do let_it_be(:maintainer) { create(:user, username: 'maintainer_user') } + let_it_be(:maintainer2) { create(:user, username: 'user-with-maintainer-role') } let_it_be(:developer) { create(:user) } let_it_be(:access_requester) { create(:user) } let_it_be(:stranger) { create(:user) } @@ -31,8 +32,8 @@ RSpec.describe API::Invitations do api("/#{source.model_name.plural}/#{source.id}/invitations", user) end - def invite_member_by_email(source, source_type, email, created_by) - create(:"#{source_type}_member", invite_token: '123', invite_email: email, source: source, user: nil, created_by: created_by) + def invite_member_by_email(source, source_type, email, created_by, access_level: :developer) + create(:"#{source_type}_member", access_level, invite_token: '123', invite_email: email, source: source, user: nil, created_by: created_by) end shared_examples 'POST /:source_type/:id/invitations' do |source_type| @@ -44,15 +45,42 @@ RSpec.describe API::Invitations do end end - context 'when authenticated as a non-member or member with insufficient rights' do - %i[access_requester stranger developer].each do |type| - context "as a #{type}" do - it 'returns 403' do - user = public_send(type) + context 'when authenticated as a non-member or member with insufficient membership management rights' do + context 'when the user does not have rights to manage members' do + %i[access_requester stranger developer].each do |type| + context "as a #{type}" do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + post invitations_url(source, public_send(type)), + params: { email: email, access_level: Member::MAINTAINER } + end + end + end + end + end + + context 'when the user has the rights to manage members but tries to manage members with a higher access level' do + let(:maintainer) { maintainer2 } - post invitations_url(source, user), params: { email: email, access_level: Member::MAINTAINER } + before do + source.add_maintainer(maintainer) + end - expect(response).to have_gitlab_http_status(:forbidden) + context 'when an invitee is added as OWNER' do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + post invitations_url(source, maintainer), + params: { email: email, access_level: Member::OWNER } + end + end + end + + context 'when an access_requester is added as OWNER' do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + post invitations_url(source, maintainer), + params: { user_id: access_requester.email, access_level: Member::OWNER } + end end end end @@ -348,6 +376,14 @@ RSpec.describe API::Invitations do it 'returns 400 when the email list is not a valid format' do post invitations_url(source, maintainer), + params: { email: %w[email1@example.com not-an-email], access_level: Member::MAINTAINER } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('email contains an invalid email address') + end + + it 'returns 400 when the comma-separated email list is not a valid format' do + post invitations_url(source, maintainer), params: { email: 'email1@example.com,not-an-email', access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:bad_request) @@ -495,14 +531,12 @@ RSpec.describe API::Invitations do end end - %i[developer access_requester stranger].each do |type| - context "when authenticated as a #{type}" do - it 'returns 403' do - user = public_send(type) - - get invitations_url(source, user) - - expect(response).to have_gitlab_http_status(:forbidden) + %i[access_requester stranger developer].each do |type| + context "as a #{type}" do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + get invitations_url(source, public_send(type)) + end end end end @@ -573,14 +607,14 @@ RSpec.describe API::Invitations do end context 'when authenticated as a non-member or member with insufficient rights' do - %i[access_requester stranger].each do |type| - context "as a #{type}" do - it 'returns 403' do - user = public_send(type) - - delete invite_api(source, user, invite.invite_email) - - expect(response).to have_gitlab_http_status(:forbidden) + context 'when the user does not have rights to manage members' do + %i[access_requester stranger].each do |type| + context "as a #{type}" do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + delete invite_api(source, public_send(type), invite.invite_email) + end + end end end end @@ -604,6 +638,23 @@ RSpec.describe API::Invitations do expect(response).to have_gitlab_http_status(:no_content) end.to change { source.members.count }.by(-1) end + + context 'when MAINTAINER tries to remove invitation of an OWNER' do + let_it_be(:maintainer) { maintainer2 } + let!(:owner_invite) do + invite_member_by_email(source, source_type, 'owner@owner.com', developer, access_level: :owner) + end + + before do + source.add_maintainer(maintainer) + end + + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + delete invite_api(source, maintainer, owner_invite.invite_email) + end + end + end end it 'returns 404 if member does not exist' do @@ -651,14 +702,15 @@ RSpec.describe API::Invitations do end context 'when authenticated as a non-member or member with insufficient rights' do - %i[access_requester stranger].each do |type| - context "as a #{type}" do - it 'returns 403' do - user = public_send(type) - - put update_api(source, user, invite.invite_email), params: { access_level: Member::MAINTAINER } - - expect(response).to have_gitlab_http_status(:forbidden) + context 'when the user does not have rights to manage members' do + %i[access_requester stranger].each do |type| + context "as a #{type}" do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + put update_api(source, public_send(type), invite.invite_email), + params: { access_level: Member::MAINTAINER } + end + end end end end @@ -673,6 +725,21 @@ RSpec.describe API::Invitations do expect(json_response['access_level']).to eq(Member::MAINTAINER) expect(invite.reload.access_level).to eq(Member::MAINTAINER) end + + context 'MAINTAINER tries to update access level to OWNER' do + let_it_be(:maintainer) { maintainer2 } + + before do + source.add_maintainer(maintainer) + end + + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + put update_api(source, maintainer, invite.invite_email), + params: { access_level: Member::OWNER } + end + end + end end it 'returns 409 if member does not exist' do diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index f5c73846173..0e20b2133db 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -92,7 +92,7 @@ RSpec.describe API::Issues do describe 'GET /issues/:id' do context 'when unauthorized' do it 'returns unauthorized' do - get api("/issues/#{issue.id}" ) + get api("/issues/#{issue.id}") expect(response).to have_gitlab_http_status(:unauthorized) end @@ -101,7 +101,7 @@ RSpec.describe API::Issues do context 'when authorized' do context 'as a normal user' do it 'returns forbidden' do - get api("/issues/#{issue.id}", user ) + get api("/issues/#{issue.id}", user) expect(response).to have_gitlab_http_status(:forbidden) end diff --git a/spec/requests/api/issues/post_projects_issues_spec.rb b/spec/requests/api/issues/post_projects_issues_spec.rb index 3883eb01391..deaf7be96ab 100644 --- a/spec/requests/api/issues/post_projects_issues_spec.rb +++ b/spec/requests/api/issues/post_projects_issues_spec.rb @@ -473,8 +473,8 @@ RSpec.describe API::Issues do end describe '/projects/:id/issues/:issue_iid/move' do - let!(:target_project) { create(:project, creator_id: user.id, namespace: user.namespace ) } - let!(:target_project2) { create(:project, creator_id: non_member.id, namespace: non_member.namespace ) } + let!(:target_project) { create(:project, creator_id: user.id, namespace: user.namespace) } + let!(:target_project2) { create(:project, creator_id: non_member.id, namespace: non_member.namespace) } it 'moves an issue' do post api("/projects/#{project.id}/issues/#{issue.iid}/move", user), diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index c1217292d5c..97ab90c9776 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -234,7 +234,7 @@ RSpec.describe API::Labels do before do create(:labeled_issue, project: project, labels: [group_label], author: user) create(:labeled_issue, project: project, labels: [label1], author: user, state: :closed) - create(:labeled_merge_request, labels: [priority_label], author: user, source_project: project ) + create(:labeled_merge_request, labels: [priority_label], author: user, source_project: project) end it 'includes counts in the response' do diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb index 3e702b05bc9..6239ac4e749 100644 --- a/spec/requests/api/markdown_spec.rb +++ b/spec/requests/api/markdown_spec.rb @@ -193,7 +193,7 @@ RSpec.describe API::Markdown do end let(:issue) { create(:issue, project: public_project, title: 'Team only title') } - let(:text) { "#{issue.to_reference}" } + let(:text) { issue.to_reference.to_s } let(:params) { { text: text, gfm: true, project: public_project.full_path } } shared_examples 'user without proper access' do diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index d771c1e2dcc..ac8c4aacdf2 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -720,6 +720,16 @@ RSpec.describe API::MavenPackages do expect(response).to have_gitlab_http_status(:not_found) end + context 'with access to package registry for everyone' do + subject { download_file(file_name: package_file.file_name) } + + before do + project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + end + + it_behaves_like 'successfully returning the file' + end + it_behaves_like 'downloads with a job token' it_behaves_like 'downloads with a deploy token' diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 9df9c75b020..69be574f38a 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -270,39 +270,42 @@ RSpec.describe API::Members do end end - context 'when authenticated as a non-member or member with insufficient rights' do - %i[access_requester stranger developer].each do |type| - context "as a #{type}" do - it 'returns 403' do - user = public_send(type) - post api("/#{source_type.pluralize}/#{source.id}/members", user), - params: { user_id: access_requester.id, access_level: Member::MAINTAINER } - - expect(response).to have_gitlab_http_status(:forbidden) + context 'when authenticated as a non-member or member with insufficient membership management rights' do + context 'when the user does not have rights to manage members' do + %i[access_requester stranger developer].each do |type| + context "as a #{type}" do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + post api("/#{source_type.pluralize}/#{source.id}/members", public_send(type)), + params: { user_id: access_requester.id, access_level: Member::MAINTAINER } + end + end end end + end - context 'adding a member of higher access level' do - before do - # the other 'maintainer' is in fact an owner of the group! - source.add_maintainer(maintainer2) - end + context 'when the user has the rights to manage members but tries to manage members with a higher access level' do + # the other 'maintainer' is in fact an owner of the group! + let(:maintainer) { maintainer2 } - context 'when an access requester' do - it 'is not successful' do - post api("/#{source_type.pluralize}/#{source.id}/members", maintainer2), - params: { user_id: access_requester.id, access_level: Member::OWNER } + before do + source.add_maintainer(maintainer) + end - expect(response).to have_gitlab_http_status(:forbidden) + context 'when an access requester is added as OWNER' do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + params: { user_id: access_requester.id, access_level: Member::OWNER } end end + end - context 'when a totally new user' do - it 'is not successful' do - post api("/#{source_type.pluralize}/#{source.id}/members", maintainer2), + context 'when a totally new user is added as OWNER' do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: stranger.id, access_level: Member::OWNER } - - expect(response).to have_gitlab_http_status(:forbidden) end end end @@ -561,27 +564,31 @@ RSpec.describe API::Members do context 'when authenticated as a non-member or member with insufficient rights' do %i[access_requester stranger developer].each do |type| context "as a #{type}" do - it 'returns 403' do - user = public_send(type) - put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user), - params: { access_level: Member::MAINTAINER } - - expect(response).to have_gitlab_http_status(:forbidden) + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", public_send(type)), + params: { access_level: Member::MAINTAINER } + end end end end context 'as a maintainer updating a member to one with higher access level than themselves' do + # the other 'maintainer' is in fact an owner of the group! + let(:maintainer) { maintainer2 } + before do # the other 'maintainer' is in fact an owner of the group! source.add_maintainer(maintainer2) end - it 'returns 403' do - put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer2), - params: { access_level: Member::OWNER } - - expect(response).to have_gitlab_http_status(:forbidden) + context 'updating a member to OWNER' do + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), + params: { access_level: Member::OWNER } + end + end end end end @@ -600,18 +607,19 @@ RSpec.describe API::Members do context 'when updating a member with higher access level' do let(:owner) { create(:user) } + # the other 'maintainer' is in fact an owner of the group! + let(:maintainer) { maintainer2 } before do source.add_owner(owner) - # the other 'maintainer' is in fact an owner of the group! - source.add_maintainer(maintainer2) + source.add_maintainer(maintainer) end - it 'returns 403' do - put api("/#{source_type.pluralize}/#{source.id}/members/#{owner.id}", maintainer2), - params: { access_level: Member::DEVELOPER } - - expect(response).to have_gitlab_http_status(:forbidden) + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + put api("/#{source_type.pluralize}/#{source.id}/members/#{owner.id}", maintainer), + params: { access_level: Member::OWNER } + end end end end @@ -676,11 +684,10 @@ RSpec.describe API::Members do context 'when authenticated as a non-member or member with insufficient rights' do %i[access_requester stranger].each do |type| context "as a #{type}" do - it 'returns 403' do - user = public_send(type) - delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", user) - - expect(response).to have_gitlab_http_status(:forbidden) + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", public_send(type)) + end end end end @@ -709,18 +716,18 @@ RSpec.describe API::Members do context 'when attempting to delete a member with higher access level' do let(:owner) { create(:user) } + # the other 'maintainer' is in fact an owner of the group! + let(:maintainer) { maintainer2 } before do source.add_owner(owner) - # the other 'maintainer' is in fact an owner of the group! - source.add_maintainer(maintainer2) + source.add_maintainer(maintainer) end - it 'returns 403' do - delete api("/#{source_type.pluralize}/#{source.id}/members/#{owner.id}", maintainer2), - params: { access_level: Member::DEVELOPER } - - expect(response).to have_gitlab_http_status(:forbidden) + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + delete api("/#{source_type.pluralize}/#{source.id}/members/#{owner.id}", maintainer) + end end end @@ -799,11 +806,11 @@ RSpec.describe API::Members do end context 'adding owner to project' do - it 'returns 403' do - post api("/projects/#{project.id}/members", maintainer), - params: { user_id: stranger.id, access_level: Member::OWNER } - - expect(response).to have_gitlab_http_status(:forbidden) + it_behaves_like 'a 403 response when user does not have rights to manage members of a specific access level' do + let(:route) do + post api("/projects/#{project.id}/members", maintainer), + params: { user_id: access_requester.id, access_level: Member::OWNER } + end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index d593e369d27..eea223485ce 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -3022,7 +3022,7 @@ RSpec.describe API::MergeRequests do describe "PUT /projects/:id/merge_requests/:merge_request_iid" do context 'updates force_remove_source_branch properly' do it 'sets to false' do - merge_request.update!(merge_params: { 'force_remove_source_branch' => true } ) + merge_request.update!(merge_params: { 'force_remove_source_branch' => true }) expect(merge_request.force_remove_source_branch?).to be_truthy @@ -3034,7 +3034,7 @@ RSpec.describe API::MergeRequests do end it 'sets to true' do - merge_request.update!(merge_params: { 'force_remove_source_branch' => false } ) + merge_request.update!(merge_params: { 'force_remove_source_branch' => false }) expect(merge_request.force_remove_source_branch?).to be false @@ -3279,7 +3279,7 @@ RSpec.describe API::MergeRequests do end it 'when removing labels, only removes those specified' do - put api_base, params: { remove_labels: "#{label.title}" } + put api_base, params: { remove_labels: label.title.to_s } expect(response).to have_gitlab_http_status(:ok) expect(json_response['labels']).to eq([label2.title]) diff --git a/spec/requests/api/metrics/dashboard/annotations_spec.rb b/spec/requests/api/metrics/dashboard/annotations_spec.rb index 5e64ac7d481..a09596f167d 100644 --- a/spec/requests/api/metrics/dashboard/annotations_spec.rb +++ b/spec/requests/api/metrics/dashboard/annotations_spec.rb @@ -64,7 +64,7 @@ RSpec.describe API::Metrics::Dashboard::Annotations do { 'starting_at' => starting_at.to_time, 'ending_at' => ending_at.to_time, - "#{source_type}" => source, + source_type.to_s => source, 'dashboard_path' => dashboard_unescaped, 'description' => params[:description] } diff --git a/spec/requests/api/ml/mlflow_spec.rb b/spec/requests/api/ml/mlflow_spec.rb index 09e9359c0b3..9448f009742 100644 --- a/spec/requests/api/ml/mlflow_spec.rb +++ b/spec/requests/api/ml/mlflow_spec.rb @@ -253,7 +253,7 @@ RSpec.describe API::Ml::Mlflow do it 'creates the experiment', :aggregate_failures do expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include('experiment_id' ) + expect(json_response).to include('experiment_id') end describe 'Error States' do @@ -295,7 +295,6 @@ RSpec.describe API::Ml::Mlflow do 'experiment_id' => params[:experiment_id], 'user_id' => current_user.id.to_s, 'start_time' => params[:start_time], - 'artifact_uri' => 'not_implemented', 'status' => "RUNNING", 'lifecycle_stage' => "active" } @@ -339,7 +338,7 @@ RSpec.describe API::Ml::Mlflow do 'experiment_id' => candidate.experiment.iid.to_s, 'user_id' => candidate.user.id.to_s, 'start_time' => candidate.start_time, - 'artifact_uri' => 'not_implemented', + 'artifact_uri' => "http://www.example.com/api/v4/projects/#{project_id}/packages/generic/ml_candidate_#{candidate.iid}/-/", 'status' => "RUNNING", 'lifecycle_stage' => "active" } @@ -378,7 +377,7 @@ RSpec.describe API::Ml::Mlflow do 'user_id' => candidate.user.id.to_s, 'start_time' => candidate.start_time, 'end_time' => params[:end_time], - 'artifact_uri' => 'not_implemented', + 'artifact_uri' => "http://www.example.com/api/v4/projects/#{project_id}/packages/generic/ml_candidate_#{candidate.iid}/-/", 'status' => 'FAILED', 'lifecycle_stage' => 'active' } diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index bdcd6e7278d..373327787a2 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -5,16 +5,29 @@ require 'spec_helper' RSpec.describe API::NpmProjectPackages do include_context 'npm api setup' - describe 'GET /api/v4/projects/:id/packages/npm/*package_name' do - it_behaves_like 'handling get metadata requests', scope: :project do - let(:url) { api("/projects/#{project.id}/packages/npm/#{package_name}") } + shared_examples 'accept get request on private project with access to package registry for everyone' do + subject { get(url) } + + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) end + + it_behaves_like 'returning response status', :ok + end + + describe 'GET /api/v4/projects/:id/packages/npm/*package_name' do + let(:url) { api("/projects/#{project.id}/packages/npm/#{package_name}") } + + it_behaves_like 'handling get metadata requests', scope: :project + it_behaves_like 'accept get request on private project with access to package registry for everyone' end describe 'GET /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags' do - it_behaves_like 'handling get dist tags requests', scope: :project do - let(:url) { api("/projects/#{project.id}/packages/npm/-/package/#{package_name}/dist-tags") } - end + let(:url) { api("/projects/#{project.id}/packages/npm/-/package/#{package_name}/dist-tags") } + + it_behaves_like 'handling get dist tags requests', scope: :project + it_behaves_like 'accept get request on private project with access to package registry for everyone' end describe 'PUT /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags/:tag' do @@ -108,6 +121,14 @@ RSpec.describe API::NpmProjectPackages do expect(response).to have_gitlab_http_status(:forbidden) end end + + context 'with access to package registry for everyone' do + before do + project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + end + + it_behaves_like 'successfully downloads the file' + end end context 'internal project' do diff --git a/spec/requests/api/pages/pages_spec.rb b/spec/requests/api/pages/pages_spec.rb index 7d44ff533aa..4a94bf90205 100644 --- a/spec/requests/api/pages/pages_spec.rb +++ b/spec/requests/api/pages/pages_spec.rb @@ -36,7 +36,7 @@ RSpec.describe API::Pages do end it 'removes the pages' do - delete api("/projects/#{project.id}/pages", admin ) + delete api("/projects/#{project.id}/pages", admin) expect(project.reload.pages_metadatum.deployed?).to be(false) end diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb index 31c4e8803e3..1fa2ad6ebfa 100644 --- a/spec/requests/api/personal_access_tokens_spec.rb +++ b/spec/requests/api/personal_access_tokens_spec.rb @@ -18,9 +18,10 @@ RSpec.describe API::PersonalAccessTokens do it "status, count and result as expected" do subject - if status == :bad_request + case status + when :bad_request expect(json_response).to eq(result) - elsif status == :ok + when :ok expect(map_id(json_response)).to a_collection_containing_exactly(*result) end @@ -87,7 +88,7 @@ RSpec.describe API::PersonalAccessTokens do end context 'filter with created parameter' do - let_it_be(:token1) { create(:personal_access_token, created_at: DateTime.new(2022, 01, 01, 12, 30, 25) ) } + let_it_be(:token1) { create(:personal_access_token, created_at: DateTime.new(2022, 01, 01, 12, 30, 25)) } context 'test created_before' do where(:created_at, :status, :result_count, :result) do @@ -121,7 +122,7 @@ RSpec.describe API::PersonalAccessTokens do end context 'filter with last_used parameter' do - let_it_be(:token1) { create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 01, 12, 30, 25) ) } + let_it_be(:token1) { create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 01, 12, 30, 25)) } let_it_be(:never_used_token) { create(:personal_access_token) } context 'test last_used_before' do @@ -209,13 +210,13 @@ RSpec.describe API::PersonalAccessTokens do expect(response).to have_gitlab_http_status(status) - expect(json_response.map { |pat| pat['id'] } ).to include(*result) if status == :ok + expect(json_response.map { |pat| pat['id'] }).to include(*result) if status == :ok end end end context 'filter last_used_before and last_used_after combined is valid' do - let_it_be(:token1) { create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 02) ) } + let_it_be(:token1) { create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 02)) } where(:last_used_before, :last_used_after, :status, :result) do '2022-01-02' | '2022-01-02' | :ok | lazy { [token1.id] } @@ -232,7 +233,7 @@ RSpec.describe API::PersonalAccessTokens do expect(response).to have_gitlab_http_status(status) - expect(json_response.map { |pat| pat['id'] } ).to include(*result) if status == :ok + expect(json_response.map { |pat| pat['id'] }).to include(*result) if status == :ok end end end @@ -304,7 +305,7 @@ RSpec.describe API::PersonalAccessTokens do # Here it is only tested whether PATs to which the user has no access right are excluded from the filter function. context 'filter with created parameter' do let_it_be(:token1) do - create(:personal_access_token, created_at: DateTime.new(2022, 01, 02, 12, 30, 25), user: current_user ) + create(:personal_access_token, created_at: DateTime.new(2022, 01, 02, 12, 30, 25), user: current_user) end let_it_be(:token2) { create(:personal_access_token, created_at: DateTime.new(2022, 01, 02, 12, 30, 25)) } @@ -332,7 +333,7 @@ RSpec.describe API::PersonalAccessTokens do create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 01, 12, 30, 25), user: current_user) end - let_it_be(:token2) { create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 01, 12, 30, 25) ) } + let_it_be(:token2) { create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 01, 12, 30, 25)) } let_it_be(:never_used_token) { create(:personal_access_token) } let_it_be(:status) { :ok } diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 0b4a96896d6..2ff4cd72f1e 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -162,6 +162,7 @@ project_setting: - show_diff_preview_in_email - suggested_reviewers_enabled - jitsu_key + - mirror_branch_regex build_service_desk_setting: # service_desk_setting unexposed_attributes: diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index 506e60d19a6..52ec06d76a9 100644 --- a/spec/requests/api/project_container_repositories_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -138,14 +138,26 @@ RSpec.describe API::ProjectContainerRepositories do context 'for maintainer' do let(:api_user) { maintainer } - it 'schedules removal of repository' do - expect(DeleteContainerRepositoryWorker).to receive(:perform_async) - .with(maintainer.id, root_repository.id) - - subject + it 'marks the repository as delete_scheduled' do + expect(DeleteContainerRepositoryWorker).not_to receive(:perform_async) + expect { subject }.to change { root_repository.reload.status }.from(nil).to('delete_scheduled') expect(response).to have_gitlab_http_status(:accepted) end + + context 'with container_registry_delete_repository_with_cron_worker disabled' do + before do + stub_feature_flags(container_registry_delete_repository_with_cron_worker: false) + end + + it 'schedules removal of repository' do + expect(DeleteContainerRepositoryWorker).to receive(:perform_async) + .with(maintainer.id, root_repository.id) + expect { subject }.to change { root_repository.reload.status }.from(nil).to('delete_scheduled') + + expect(response).to have_gitlab_http_status(:accepted) + end + end end end end diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 401db766589..05fe55b06a1 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -47,7 +47,7 @@ RSpec.describe API::ProjectImport, :aggregate_failures do it 'executes a limited number of queries' do control_count = ActiveRecord::QueryRecorder.new { subject }.count - expect(control_count).to be <= 110 + expect(control_count).to be <= 111 end it 'schedules an import using a namespace' do @@ -215,7 +215,7 @@ RSpec.describe API::ProjectImport, :aggregate_failures do subject expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq('Name has already been taken') + expect(json_response['message']).to eq('Project namespace name has already been taken') end context 'when param overwrite is true' do diff --git a/spec/requests/api/project_milestones_spec.rb b/spec/requests/api/project_milestones_spec.rb index b23fb86a9de..8294ca143d3 100644 --- a/spec/requests/api/project_milestones_spec.rb +++ b/spec/requests/api/project_milestones_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe API::ProjectMilestones do let_it_be(:user) { create(:user) } - let_it_be_with_reload(:project) { create(:project, namespace: user.namespace ) } + let_it_be_with_reload(:project) { create(:project, namespace: user.namespace) } let_it_be(:closed_milestone) { create(:closed_milestone, project: project, title: 'version1', description: 'closed milestone') } let_it_be(:milestone) { create(:milestone, project: project, title: 'version2', description: 'open milestone') } let_it_be(:route) { "/projects/#{project.id}/milestones" } diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 6e2dd6e76a9..1d255f7c1d8 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -89,7 +89,7 @@ RSpec.describe API::ProjectSnippets do expect(json_response['title']).to eq(snippet.title) expect(json_response['description']).to eq(snippet.description) expect(json_response['file_name']).to eq(snippet.file_name_on_repo) - expect(json_response['files']).to eq(snippet.blobs.map { |blob| snippet_blob_file(blob) } ) + expect(json_response['files']).to eq(snippet.blobs.map { |blob| snippet_blob_file(blob) }) expect(json_response['ssh_url_to_repo']).to eq(snippet.ssh_url_to_repo) expect(json_response['http_url_to_repo']).to eq(snippet.http_url_to_repo) end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 38f7d6e3eba..3831e8e1dfe 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -169,10 +169,8 @@ RSpec.describe API::Projects do shared_examples_for 'projects response without N + 1 queries' do |threshold| let(:additional_project) { create(:project, :public) } - it 'avoids N + 1 queries' do - get api('/projects', current_user) - - control = ActiveRecord::QueryRecorder.new do + it 'avoids N + 1 queries', :use_sql_query_cache do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api('/projects', current_user) end @@ -180,7 +178,7 @@ RSpec.describe API::Projects do expect do get api('/projects', current_user) - end.not_to exceed_query_limit(control).with_threshold(threshold) + end.not_to exceed_all_query_limit(control).with_threshold(threshold) end end @@ -209,16 +207,28 @@ RSpec.describe API::Projects do let(:current_user) { user } end - it 'includes container_registry_access_level', :aggregate_failures do - project.project_feature.update!(container_registry_access_level: ProjectFeature::DISABLED) + shared_examples 'includes container_registry_access_level', :aggregate_failures do + it do + project.project_feature.update!(container_registry_access_level: ProjectFeature::DISABLED) - get api('/projects', user) - project_response = json_response.find { |p| p['id'] == project.id } + get api('/projects', user) + project_response = json_response.find { |p| p['id'] == project.id } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - expect(project_response['container_registry_access_level']).to eq('disabled') - expect(project_response['container_registry_enabled']).to eq(false) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(project_response['container_registry_access_level']).to eq('disabled') + expect(project_response['container_registry_enabled']).to eq(false) + end + end + + include_examples 'includes container_registry_access_level' + + context 'when projects_preloader_fix is disabled' do + before do + stub_feature_flags(projects_preloader_fix: false) + end + + include_examples 'includes container_registry_access_level' end it 'includes releases_access_level', :aggregate_failures do @@ -1055,7 +1065,7 @@ RSpec.describe API::Projects do let_it_be(:admin) { create(:admin) } - it 'avoids N+1 queries' do + it 'avoids N+1 queries', :use_sql_query_cache do get api('/projects', admin) base_project = create(:project, :public, namespace: admin.namespace) @@ -1063,7 +1073,7 @@ RSpec.describe API::Projects do fork_project1 = fork_project(base_project, admin, namespace: create(:user).namespace) fork_project2 = fork_project(fork_project1, admin, namespace: create(:user).namespace) - control = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api('/projects', admin) end @@ -3428,18 +3438,6 @@ RSpec.describe API::Projects do end context 'when authenticated as project owner' do - it 'updates name' do - project_param = { name: 'bar' } - - put api("/projects/#{project.id}", user), params: project_param - - expect(response).to have_gitlab_http_status(:ok) - - project_param.each_pair do |k, v| - expect(json_response[k.to_s]).to eq(v) - end - end - it 'updates visibility_level' do project_param = { visibility: 'public' } @@ -3797,10 +3795,16 @@ RSpec.describe API::Projects do expect(json_response['message']['path']).to eq(['has already been taken']) end - it 'does not update name' do + it 'updates name' do project_param = { name: 'bar' } - put api("/projects/#{project3.id}", user4), params: project_param - expect(response).to have_gitlab_http_status(:forbidden) + + put api("/projects/#{project.id}", user), params: project_param + + expect(response).to have_gitlab_http_status(:ok) + + project_param.each_pair do |k, v| + expect(json_response[k.to_s]).to eq(v) + end end it 'does not update visibility_level' do diff --git a/spec/requests/api/protected_branches_spec.rb b/spec/requests/api/protected_branches_spec.rb index 9f10eb1bb9f..b46859a0e70 100644 --- a/spec/requests/api/protected_branches_spec.rb +++ b/spec/requests/api/protected_branches_spec.rb @@ -29,8 +29,7 @@ RSpec.describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(json_response).to be_an Array - + expect(response).to match_response_schema('protected_branches') protected_branch_names = json_response.map { |x| x['name'] } expect(protected_branch_names).to match_array(expected_branch_names) end @@ -71,6 +70,7 @@ RSpec.describe API::ProtectedBranches do get api(route, user) expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('protected_branch') expect(json_response['name']).to eq(branch_name) expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(::Gitlab::Access::MAINTAINER) @@ -130,6 +130,7 @@ RSpec.describe API::ProtectedBranches do post post_endpoint, params: { name: branch_name } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_branch') expect(json_response['name']).to eq(branch_name) expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) @@ -140,6 +141,7 @@ RSpec.describe API::ProtectedBranches do post post_endpoint, params: { name: branch_name, push_access_level: 30 } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_branch') expect(json_response['name']).to eq(branch_name) expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER) @@ -150,6 +152,7 @@ RSpec.describe API::ProtectedBranches do post post_endpoint, params: { name: branch_name, merge_access_level: 30 } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_branch') expect(json_response['name']).to eq(branch_name) expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) @@ -160,6 +163,7 @@ RSpec.describe API::ProtectedBranches do post post_endpoint, params: { name: branch_name, push_access_level: 30, merge_access_level: 30 } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_branch') expect(json_response['name']).to eq(branch_name) expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER) @@ -170,6 +174,7 @@ RSpec.describe API::ProtectedBranches do post post_endpoint, params: { name: branch_name, push_access_level: 0 } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_branch') expect(json_response['name']).to eq(branch_name) expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS) @@ -180,6 +185,7 @@ RSpec.describe API::ProtectedBranches do post post_endpoint, params: { name: branch_name, merge_access_level: 0 } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_branch') expect(json_response['name']).to eq(branch_name) expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) @@ -190,6 +196,7 @@ RSpec.describe API::ProtectedBranches do post post_endpoint, params: { name: branch_name, push_access_level: 0, merge_access_level: 0 } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_branch') expect(json_response['name']).to eq(branch_name) expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS) @@ -200,6 +207,7 @@ RSpec.describe API::ProtectedBranches do post post_endpoint, params: { name: branch_name, allow_force_push: true } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_branch') expect(json_response['name']).to eq(branch_name) expect(json_response['allow_force_push']).to eq(true) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) @@ -246,6 +254,49 @@ RSpec.describe API::ProtectedBranches do end end + describe 'PATCH /projects/:id/protected_branches/:name' do + let(:route) { "/projects/#{project.id}/protected_branches/#{branch_name}" } + + context 'when authenticated as a maintainer' do + let(:user) { maintainer } + + it "updates a single branch" do + expect do + patch api(route, user), params: { allow_force_push: true } + end.to change { protected_branch.reload.allow_force_push }.from(false).to(true) + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when returned protected branch is invalid' do + let(:user) { maintainer } + + before do + allow_next_found_instance_of(ProtectedBranch) do |instance| + allow(instance).to receive(:valid?).and_return(false) + end + end + + it "returns a 422" do + expect do + patch api(route, user), params: { allow_force_push: true } + end.not_to change { protected_branch.reload.allow_force_push } + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + + context 'when authenticated as a guest' do + let(:user) { guest } + + it "returns a 403 error" do + patch api(route, user), params: { allow_force_push: true } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + describe "DELETE /projects/:id/protected_branches/unprotect/:branch" do let(:user) { maintainer } let(:delete_endpoint) { api("/projects/#{project.id}/protected_branches/#{branch_name}", user) } diff --git a/spec/requests/api/protected_tags_spec.rb b/spec/requests/api/protected_tags_spec.rb index 84b7df86f31..f1db39ac204 100644 --- a/spec/requests/api/protected_tags_spec.rb +++ b/spec/requests/api/protected_tags_spec.rb @@ -22,7 +22,7 @@ RSpec.describe API::ProtectedTags do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(json_response).to be_an Array + expect(response).to match_response_schema('protected_tags') protected_tag_names = json_response.map { |x| x['name'] } expected_tags_names = project.protected_tags.map { |x| x['name'] } @@ -57,6 +57,7 @@ RSpec.describe API::ProtectedTags do get api(route, user) expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('protected_tag') expect(json_response['name']).to eq(tag_name) expect(json_response['create_access_levels'][0]['access_level']).to eq(::Gitlab::Access::MAINTAINER) end @@ -108,6 +109,7 @@ RSpec.describe API::ProtectedTags do post api("/projects/#{project.id}/protected_tags", user), params: { name: tag_name } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_tag') expect(json_response['name']).to eq(tag_name) expect(json_response['create_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) end @@ -117,6 +119,7 @@ RSpec.describe API::ProtectedTags do params: { name: tag_name, create_access_level: 30 } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_tag') expect(json_response['name']).to eq(tag_name) expect(json_response['create_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER) end @@ -126,6 +129,7 @@ RSpec.describe API::ProtectedTags do params: { name: tag_name, create_access_level: 0 } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_tag') expect(json_response['name']).to eq(tag_name) expect(json_response['create_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS) end @@ -142,6 +146,7 @@ RSpec.describe API::ProtectedTags do post api("/projects/#{project2.id}/protected_tags", user), params: { name: protected_name } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_tag') expect(json_response['name']).to eq(protected_name) end @@ -152,6 +157,7 @@ RSpec.describe API::ProtectedTags do post api("/projects/#{project.id}/protected_tags", user), params: { name: tag_name } expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('protected_tag') expect(json_response['name']).to eq(tag_name) expect(json_response['create_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) end diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 6c130bb4963..12091158a02 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -69,6 +69,7 @@ RSpec.describe API::PypiPackages do it_behaves_like 'rejects PyPI access with unknown project id' it_behaves_like 'deploy token for package GET requests' it_behaves_like 'job token for package GET requests' + it_behaves_like 'allow access for everyone with public package_registry_access_level' context 'with project path as id' do let(:url) { "/projects/#{CGI.escape(project.full_path)}/packages/pypi/simple" } @@ -130,6 +131,7 @@ RSpec.describe API::PypiPackages do it_behaves_like 'rejects PyPI access with unknown project id' it_behaves_like 'deploy token for package GET requests' it_behaves_like 'job token for package GET requests' + it_behaves_like 'allow access for everyone with public package_registry_access_level' context 'with project path as id' do let(:url) { "/projects/#{CGI.escape(project.full_path)}/packages/pypi/simple/#{package.name}" } @@ -377,6 +379,7 @@ RSpec.describe API::PypiPackages do it_behaves_like 'pypi file download endpoint' it_behaves_like 'rejects PyPI access with unknown project id' + it_behaves_like 'allow access for everyone with public package_registry_access_level' end end end diff --git a/spec/requests/api/release/links_spec.rb b/spec/requests/api/release/links_spec.rb index 57b2e005929..38166c5ce97 100644 --- a/spec/requests/api/release/links_spec.rb +++ b/spec/requests/api/release/links_spec.rb @@ -81,24 +81,20 @@ RSpec.describe API::Release::Links do end context 'when project is public' do - let(:project) { create(:project, :repository, :public) } + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end it 'allows the request' do get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) expect(response).to have_gitlab_http_status(:ok) end - end - - context 'when project is public and the repository is private' do - let(:project) { create(:project, :repository, :public, :repository_private) } - - it_behaves_like '403 response' do - let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) } - end - context 'when the release does not exists' do - let!(:release) {} + context 'and the releases are private' do + before do + project.project_feature.update!(releases_access_level: ProjectFeature::PRIVATE) + end it_behaves_like '403 response' do let(:request) { get api("/projects/#{project.id}/releases/v0.1/assets/links", non_project_member) } diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb index d9a12e7e148..24efac3128d 100644 --- a/spec/requests/api/resource_access_tokens_spec.rb +++ b/spec/requests/api/resource_access_tokens_spec.rb @@ -243,65 +243,33 @@ RSpec.describe API::ResourceAccessTokens do end context "when the user has valid permissions" do - context 'when user_destroy_with_limited_execution_time_worker is enabled' do - it "deletes the #{source_type} access token from the #{source_type}" do - delete_token - - expect(response).to have_gitlab_http_status(:no_content) - expect( - Users::GhostUserMigration.where(user: project_bot, - initiator_user: user) - ).to be_exists - end - - context "when using #{source_type} access token to DELETE other #{source_type} access token" do - let_it_be(:other_project_bot) { create(:user, :project_bot) } - let_it_be(:other_token) { create(:personal_access_token, user: other_project_bot) } - let_it_be(:token_id) { other_token.id } - - before do - resource.add_maintainer(other_project_bot) - end - - it "deletes the #{source_type} access token from the #{source_type}" do - delete_token + it "deletes the #{source_type} access token from the #{source_type}" do + delete_token - expect(response).to have_gitlab_http_status(:no_content) - expect( - Users::GhostUserMigration.where(user: other_project_bot, - initiator_user: user) - ).to be_exists - end - end + expect(response).to have_gitlab_http_status(:no_content) + expect( + Users::GhostUserMigration.where(user: project_bot, + initiator_user: user) + ).to be_exists end - context 'when user_destroy_with_limited_execution_time_worker is disabled' do + context "when using #{source_type} access token to DELETE other #{source_type} access token" do + let_it_be(:other_project_bot) { create(:user, :project_bot) } + let_it_be(:other_token) { create(:personal_access_token, user: other_project_bot) } + let_it_be(:token_id) { other_token.id } + before do - stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) + resource.add_maintainer(other_project_bot) end it "deletes the #{source_type} access token from the #{source_type}" do delete_token expect(response).to have_gitlab_http_status(:no_content) - expect(User.exists?(project_bot.id)).to be_falsy - end - - context "when using #{source_type} access token to DELETE other #{source_type} access token" do - let_it_be(:other_project_bot) { create(:user, :project_bot) } - let_it_be(:other_token) { create(:personal_access_token, user: other_project_bot) } - let_it_be(:token_id) { other_token.id } - - before do - resource.add_maintainer(other_project_bot) - end - - it "deletes the #{source_type} access token from the #{source_type}" do - delete_token - - expect(response).to have_gitlab_http_status(:no_content) - expect(User.exists?(other_project_bot.id)).to be_falsy - end + expect( + Users::GhostUserMigration.where(user: other_project_bot, + initiator_user: user) + ).to be_exists end end @@ -416,6 +384,41 @@ RSpec.describe API::ResourceAccessTokens do expect(response.body).to include("scopes is missing") end end + + context "when using invalid 'scopes'" do + let_it_be(:params) do + { + name: "test", + scopes: ["test"], + expires_at: 5.days.from_now + } + end + + it "does not create a #{source_type} access token with invalid 'scopes'", :aggregate_failures do + create_token + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include("scopes does not have a valid value") + end + end + + context "when using invalid 'access_level'" do + let_it_be(:params) do + { + name: "test", + scopes: ["api"], + expires_at: 5.days.from_now, + access_level: Gitlab::Access::NO_ACCESS + } + end + + it "does not create a #{source_type} access token with invalid 'access_level'", :aggregate_failures do + create_token + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include("access_level does not have a valid value") + end + end end context "when trying to create a token in a different #{source_type}" do diff --git a/spec/requests/api/rpm_project_packages_spec.rb b/spec/requests/api/rpm_project_packages_spec.rb index 6a646c26fd2..68511795c94 100644 --- a/spec/requests/api/rpm_project_packages_spec.rb +++ b/spec/requests/api/rpm_project_packages_spec.rb @@ -9,7 +9,8 @@ RSpec.describe API::RpmProjectPackages do using RSpec::Parameterized::TableSyntax - let_it_be_with_reload(:project) { create(:project, :public) } + let_it_be(:group) { create(:group, :public) } + let_it_be_with_reload(:project) { create(:project, :public, group: group) } let_it_be(:user) { create(:user) } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } @@ -36,7 +37,7 @@ RSpec.describe API::RpmProjectPackages do it_behaves_like 'returning response status', status end - shared_examples 'a deploy token for RPM requests' do + shared_examples 'a deploy token for RPM requests' do |success_status = :not_found| context 'with deploy token headers' do before do project.update_column(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) @@ -45,7 +46,7 @@ RSpec.describe API::RpmProjectPackages do let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token) } context 'when token is valid' do - it_behaves_like 'returning response status', :not_found + it_behaves_like 'returning response status', success_status end context 'when token is invalid' do @@ -56,7 +57,7 @@ RSpec.describe API::RpmProjectPackages do end end - shared_examples 'a job token for RPM requests' do + shared_examples 'a job token for RPM requests' do |success_status = :not_found| context 'with job token headers' do let(:headers) { basic_auth_header(::Gitlab::Auth::CI_JOB_USER, job.token) } @@ -66,7 +67,7 @@ RSpec.describe API::RpmProjectPackages do end context 'with valid token' do - it_behaves_like 'returning response status', :not_found + it_behaves_like 'returning response status', success_status end context 'with invalid token' do @@ -83,10 +84,10 @@ RSpec.describe API::RpmProjectPackages do end end - shared_examples 'a user token for RPM requests' do + shared_examples 'a user token for RPM requests' do |success_status = :not_found| context 'with valid project' do where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do - 'PUBLIC' | :developer | true | true | 'process rpm packages upload/download' | :not_found + 'PUBLIC' | :developer | true | true | 'process rpm packages upload/download' | success_status 'PUBLIC' | :guest | true | true | 'process rpm packages upload/download' | :forbidden 'PUBLIC' | :developer | true | false | 'rejects rpm packages access' | :unauthorized 'PUBLIC' | :guest | true | false | 'rejects rpm packages access' | :unauthorized @@ -95,7 +96,7 @@ RSpec.describe API::RpmProjectPackages do 'PUBLIC' | :developer | false | false | 'rejects rpm packages access' | :unauthorized 'PUBLIC' | :guest | false | false | 'rejects rpm packages access' | :unauthorized 'PUBLIC' | :anonymous | false | true | 'process rpm packages upload/download' | :unauthorized - 'PRIVATE' | :developer | true | true | 'process rpm packages upload/download' | :not_found + 'PRIVATE' | :developer | true | true | 'process rpm packages upload/download' | success_status 'PRIVATE' | :guest | true | true | 'rejects rpm packages access' | :forbidden 'PRIVATE' | :developer | true | false | 'rejects rpm packages access' | :unauthorized 'PRIVATE' | :guest | true | false | 'rejects rpm packages access' | :unauthorized @@ -123,26 +124,31 @@ RSpec.describe API::RpmProjectPackages do end describe 'GET /api/v4/projects/:project_id/packages/rpm/repodata/:filename' do - let(:url) { "/projects/#{project.id}/packages/rpm/repodata/#{package_name}" } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } + let(:repository_file) { create(:rpm_repository_file, project: project) } + let(:url) { "/projects/#{project.id}/packages/rpm/repodata/#{repository_file.file_name}" } subject { get api(url), headers: headers } - it_behaves_like 'a job token for RPM requests' - it_behaves_like 'a deploy token for RPM requests' - it_behaves_like 'a user token for RPM requests' + it_behaves_like 'a job token for RPM requests', :success + it_behaves_like 'a deploy token for RPM requests', :success + it_behaves_like 'a user token for RPM requests', :success end describe 'GET /api/v4/projects/:id/packages/rpm/:package_file_id/:filename' do + let(:snowplow_gitlab_standard_context) { { project: project, namespace: group } } let(:url) { "/projects/#{project.id}/packages/rpm/#{package_file_id}/#{package_name}" } subject { get api(url), headers: headers } + it_behaves_like 'a package tracking event', described_class.name, 'pull_package' it_behaves_like 'a job token for RPM requests' it_behaves_like 'a deploy token for RPM requests' it_behaves_like 'a user token for RPM requests' end describe 'POST /api/v4/projects/:project_id/packages/rpm' do + let(:snowplow_gitlab_standard_context) { { project: project, namespace: group, user: user } } let(:url) { "/projects/#{project.id}/packages/rpm" } let(:file_upload) { fixture_file_upload('spec/fixtures/packages/rpm/hello-0.0.1-1.fc29.x86_64.rpm') } @@ -150,25 +156,25 @@ RSpec.describe API::RpmProjectPackages do context 'with user token' do context 'with valid project' do - where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do - 'PUBLIC' | :developer | true | true | 'process rpm packages upload/download' | :not_found - 'PUBLIC' | :guest | true | true | 'rejects rpm packages access' | :forbidden - 'PUBLIC' | :developer | true | false | 'rejects rpm packages access' | :unauthorized - 'PUBLIC' | :guest | true | false | 'rejects rpm packages access' | :unauthorized - 'PUBLIC' | :developer | false | true | 'rejects rpm packages access' | :not_found - 'PUBLIC' | :guest | false | true | 'rejects rpm packages access' | :not_found - 'PUBLIC' | :developer | false | false | 'rejects rpm packages access' | :unauthorized - 'PUBLIC' | :guest | false | false | 'rejects rpm packages access' | :unauthorized - 'PUBLIC' | :anonymous | false | true | 'rejects rpm packages access' | :unauthorized - 'PRIVATE' | :developer | true | true | 'process rpm packages upload/download' | :not_found - 'PRIVATE' | :guest | true | true | 'rejects rpm packages access' | :forbidden - 'PRIVATE' | :developer | true | false | 'rejects rpm packages access' | :unauthorized - 'PRIVATE' | :guest | true | false | 'rejects rpm packages access' | :unauthorized - 'PRIVATE' | :developer | false | true | 'rejects rpm packages access' | :not_found - 'PRIVATE' | :guest | false | true | 'rejects rpm packages access' | :not_found - 'PRIVATE' | :developer | false | false | 'rejects rpm packages access' | :unauthorized - 'PRIVATE' | :guest | false | false | 'rejects rpm packages access' | :unauthorized - 'PRIVATE' | :anonymous | false | true | 'rejects rpm packages access' | :unauthorized + where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status, :tracked) do + 'PUBLIC' | :developer | true | true | 'process rpm packages upload/download' | :not_found | true + 'PUBLIC' | :guest | true | true | 'rejects rpm packages access' | :forbidden | false + 'PUBLIC' | :developer | true | false | 'rejects rpm packages access' | :unauthorized | false + 'PUBLIC' | :guest | true | false | 'rejects rpm packages access' | :unauthorized | false + 'PUBLIC' | :developer | false | true | 'rejects rpm packages access' | :not_found | false + 'PUBLIC' | :guest | false | true | 'rejects rpm packages access' | :not_found | false + 'PUBLIC' | :developer | false | false | 'rejects rpm packages access' | :unauthorized | false + 'PUBLIC' | :guest | false | false | 'rejects rpm packages access' | :unauthorized | false + 'PUBLIC' | :anonymous | false | true | 'rejects rpm packages access' | :unauthorized | false + 'PRIVATE' | :developer | true | true | 'process rpm packages upload/download' | :not_found | true + 'PRIVATE' | :guest | true | true | 'rejects rpm packages access' | :forbidden | false + 'PRIVATE' | :developer | true | false | 'rejects rpm packages access' | :unauthorized | false + 'PRIVATE' | :guest | true | false | 'rejects rpm packages access' | :unauthorized | false + 'PRIVATE' | :developer | false | true | 'rejects rpm packages access' | :not_found | false + 'PRIVATE' | :guest | false | true | 'rejects rpm packages access' | :not_found | false + 'PRIVATE' | :developer | false | false | 'rejects rpm packages access' | :unauthorized | false + 'PRIVATE' | :guest | false | false | 'rejects rpm packages access' | :unauthorized | false + 'PRIVATE' | :anonymous | false | true | 'rejects rpm packages access' | :unauthorized | false end with_them do @@ -180,6 +186,8 @@ RSpec.describe API::RpmProjectPackages do project.send("add_#{user_role}", user) if member && user_role != :anonymous end + tracking_example = params[:tracked] ? 'a package tracking event' : 'not a package tracking event' + it_behaves_like tracking_example, described_class.name, 'push_package' it_behaves_like params[:shared_examples_name], params[:expected_status] end end diff --git a/spec/requests/api/rubygem_packages_spec.rb b/spec/requests/api/rubygem_packages_spec.rb index f0408d94137..a7d461781b8 100644 --- a/spec/requests/api/rubygem_packages_spec.rb +++ b/spec/requests/api/rubygem_packages_spec.rb @@ -325,9 +325,10 @@ RSpec.describe API::RubygemPackages do let(:headers) { user_headers.merge(workhorse_headers) } let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: snowplow_user } } let(:snowplow_user) do - if token_type == :deploy_token + case token_type + when :deploy_token deploy_token - elsif token_type == :job_token + when :job_token job.user else user diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 05f38aff6ab..60acf6b71dd 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -29,6 +29,19 @@ RSpec.describe API::Search do end end + shared_examples 'apdex recorded' do |scope:, level:, search: ''| + it 'increments the custom search sli apdex' do + expect(Gitlab::Metrics::GlobalSearchSlis).to receive(:record_apdex).with( + elapsed: a_kind_of(Numeric), + search_scope: scope, + search_type: 'basic', + search_level: level + ) + + get api(endpoint, user), params: { scope: scope, search: search } + end + end + shared_examples 'orderable by created_at' do |scope:| it 'allows ordering results by created_at asc' do get api(endpoint, user), params: { scope: scope, search: 'sortable', order_by: 'created_at', sort: 'asc' } @@ -172,6 +185,8 @@ RSpec.describe API::Search do it_behaves_like 'pagination', scope: :projects it_behaves_like 'ping counters', scope: :projects + + it_behaves_like 'apdex recorded', scope: 'projects', level: 'global' end context 'for issues scope' do @@ -186,6 +201,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :issues + it_behaves_like 'apdex recorded', scope: 'issues', level: 'global' + it_behaves_like 'issues orderable by created_at' describe 'pagination' do @@ -248,6 +265,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :merge_requests + it_behaves_like 'apdex recorded', scope: 'merge_requests', level: 'global' + it_behaves_like 'merge_requests orderable by created_at' describe 'pagination' do @@ -293,6 +312,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :milestones + it_behaves_like 'apdex recorded', scope: 'milestones', level: 'global' + describe 'pagination' do before do create(:milestone, project: project, title: 'another milestone') @@ -330,6 +351,8 @@ RSpec.describe API::Search do it_behaves_like 'pagination', scope: :users it_behaves_like 'ping counters', scope: :users + + it_behaves_like 'apdex recorded', scope: 'users', level: 'global' end context 'for snippet_titles scope' do @@ -343,6 +366,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :snippet_titles + it_behaves_like 'apdex recorded', scope: 'snippet_titles', level: 'global' + describe 'pagination' do before do create(:snippet, :public, title: 'another snippet', content: 'snippet content') @@ -352,17 +377,6 @@ RSpec.describe API::Search do end end - it 'increments the custom search sli apdex' do - expect(Gitlab::Metrics::GlobalSearchSlis).to receive(:record_apdex).with( - elapsed: a_kind_of(Numeric), - search_scope: 'issues', - search_type: 'basic', - search_level: 'global' - ) - - get api(endpoint, user), params: { scope: 'issues', search: 'john doe' } - end - it 'increments the custom search sli error rate with error false if no error occurred' do expect(Gitlab::Metrics::GlobalSearchSlis).to receive(:record_error_rate).with( error: false, @@ -466,6 +480,8 @@ RSpec.describe API::Search do it_behaves_like 'pagination', scope: :projects it_behaves_like 'ping counters', scope: :projects + + it_behaves_like 'apdex recorded', scope: 'projects', level: 'group' end context 'for issues scope' do @@ -479,6 +495,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :issues + it_behaves_like 'apdex recorded', scope: 'issues', level: 'group' + it_behaves_like 'issues orderable by created_at' describe 'pagination' do @@ -501,6 +519,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :merge_requests + it_behaves_like 'apdex recorded', scope: 'merge_requests', level: 'group' + it_behaves_like 'merge_requests orderable by created_at' describe 'pagination' do @@ -523,6 +543,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :milestones + it_behaves_like 'apdex recorded', scope: 'milestones', level: 'group' + describe 'pagination' do before do create(:milestone, project: project, title: 'another milestone') @@ -556,6 +578,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :users + it_behaves_like 'apdex recorded', scope: 'users', level: 'group' + describe 'pagination' do before do create(:group_member, :developer, group: group) @@ -645,6 +669,8 @@ RSpec.describe API::Search do it_behaves_like 'issues orderable by created_at' + it_behaves_like 'apdex recorded', scope: 'issues', level: 'project' + describe 'pagination' do before do create(:issue, project: project, title: 'another issue') @@ -677,6 +703,8 @@ RSpec.describe API::Search do it_behaves_like 'merge_requests orderable by created_at' + it_behaves_like 'apdex recorded', scope: 'merge_requests', level: 'project' + describe 'pagination' do before do create(:merge_request, source_project: repo_project, title: 'another mr', target_branch: 'another_branch') @@ -700,6 +728,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :milestones + it_behaves_like 'apdex recorded', scope: 'milestones', level: 'project' + describe 'pagination' do before do create(:milestone, project: project, title: 'another milestone') @@ -737,6 +767,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :users + it_behaves_like 'apdex recorded', scope: 'users', level: 'project' + describe 'pagination' do before do create(:project_member, :developer, project: project) @@ -757,6 +789,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :notes + it_behaves_like 'apdex recorded', scope: 'notes', level: 'project' + describe 'pagination' do before do mr = create(:merge_request, source_project: project, target_branch: 'another_branch') @@ -780,6 +814,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :wiki_blobs + it_behaves_like 'apdex recorded', scope: 'wiki_blobs', level: 'project' + describe 'pagination' do before do create(:wiki_page, wiki: wiki, title: 'home 2', content: 'Another page') @@ -802,6 +838,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :commits + it_behaves_like 'apdex recorded', scope: 'commits', level: 'project' + describe 'pipeline visibility' do shared_examples 'pipeline information visible' do it 'contains status and last_pipeline' do @@ -899,6 +937,8 @@ RSpec.describe API::Search do end it_behaves_like 'response is correct', schema: 'public_api/v4/commits_details' + + it_behaves_like 'apdex recorded', scope: 'commits', level: 'project' end context 'for blobs scope' do @@ -914,6 +954,8 @@ RSpec.describe API::Search do it_behaves_like 'ping counters', scope: :blobs + it_behaves_like 'apdex recorded', scope: 'blobs', level: 'project' + context 'filters' do it 'by filename' do get api(endpoint, user), params: { scope: 'blobs', search: 'mon filename:PROCESS.md' } diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 031bcb612f4..9408d1cc248 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -28,7 +28,7 @@ RSpec.describe API::Snippets, factory_default: :keep do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.map { |snippet| snippet['id'] } ).to contain_exactly( + expect(json_response.map { |snippet| snippet['id'] }).to contain_exactly( public_snippet.id, internal_snippet.id, private_snippet.id) @@ -75,7 +75,7 @@ RSpec.describe API::Snippets, factory_default: :keep do it 'returns snippets available for user in given time range' do get api(path, personal_access_token: user_token) - expect(json_response.map { |snippet| snippet['id'] } ).to contain_exactly( + expect(json_response.map { |snippet| snippet['id'] }).to contain_exactly( private_snippet_in_time_range1.id, private_snippet_in_time_range2.id) end @@ -99,10 +99,10 @@ RSpec.describe API::Snippets, factory_default: :keep do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.map { |snippet| snippet['id'] } ).to contain_exactly( + expect(json_response.map { |snippet| snippet['id'] }).to contain_exactly( public_snippet.id, public_snippet_other.id) - expect(json_response.map { |snippet| snippet['web_url'] } ).to contain_exactly( + expect(json_response.map { |snippet| snippet['web_url'] }).to contain_exactly( "http://localhost/-/snippets/#{public_snippet.id}", "http://localhost/-/snippets/#{public_snippet_other.id}") expect(json_response[0]['files'].first).to eq snippet_blob_file(public_snippet_other.blobs.first) @@ -126,7 +126,7 @@ RSpec.describe API::Snippets, factory_default: :keep do it 'returns public snippets available to user in given time range' do get api(path, personal_access_token: user_token) - expect(json_response.map { |snippet| snippet['id'] } ).to contain_exactly( + expect(json_response.map { |snippet| snippet['id'] }).to contain_exactly( public_snippet_in_time_range.id) end end diff --git a/spec/requests/api/submodules_spec.rb b/spec/requests/api/submodules_spec.rb index 6b141d6d036..9840476ca27 100644 --- a/spec/requests/api/submodules_spec.rb +++ b/spec/requests/api/submodules_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe API::Submodules do let(:user) { create(:user) } - let!(:project) { create(:project, :repository, namespace: user.namespace ) } + let!(:project) { create(:project, :repository, namespace: user.namespace) } let(:guest) { create(:user) { |u| project.add_guest(u) } } let(:submodule) { 'six' } let(:commit_sha) { 'e25eda1fece24ac7a03624ed1320f82396f35bd8' } diff --git a/spec/requests/api/suggestions_spec.rb b/spec/requests/api/suggestions_spec.rb index 2393a268693..dfc5d169af6 100644 --- a/spec/requests/api/suggestions_spec.rb +++ b/spec/requests/api/suggestions_spec.rb @@ -91,7 +91,7 @@ RSpec.describe API::Suggestions do end context 'when suggestion is not found' do - let(:url) { "/suggestions/foo-123/apply" } + let(:url) { "/suggestions/9999/apply" } it 'renders a not found error and returns json content' do project.add_maintainer(user) @@ -103,6 +103,19 @@ RSpec.describe API::Suggestions do end end + context 'when suggestion ID is not valid' do + let(:url) { "/suggestions/foo-123/apply" } + + it 'renders a not found error and returns json content' do + project.add_maintainer(user) + + put api(url, user) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ 'error' => 'id is invalid' }) + end + end + context 'when unauthorized' do it 'renders a forbidden error and returns json content' do project.add_reporter(user) diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index c635d73efe3..3f2ca2a0938 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -71,7 +71,7 @@ RSpec.describe API::Tags do context 'searching' do it 'only returns searched tags' do - get api("#{route}", user), params: { search: 'v1.1.0' } + get api(route.to_s, user), params: { search: 'v1.1.0' } expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers diff --git a/spec/requests/api/terraform/modules/v1/packages_spec.rb b/spec/requests/api/terraform/modules/v1/packages_spec.rb index dff44a45de4..ae61017f5bb 100644 --- a/spec/requests/api/terraform/modules/v1/packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/packages_spec.rb @@ -585,9 +585,10 @@ RSpec.describe API::Terraform::Modules::V1::Packages do let(:headers) { user_headers.merge(workhorse_headers) } let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: snowplow_user } } let(:snowplow_user) do - if token_type == :deploy_token + case token_type + when :deploy_token deploy_token - elsif token_type == :job_token + when :job_token job.user else user diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index e8458db4a4d..38b08b4e214 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -46,26 +46,19 @@ RSpec.describe API::Terraform::State, :snowplow do let(:expected_value) { instance_of(Integer) } end - it 'tracks Snowplow event' do - request - - expect_snowplow_event( - category: described_class.to_s, - action: 'p_terraform_state_api_unique_users', - namespace: project.namespace.reload, - user: current_user - ) - end - - context 'when route_hll_to_snowplow_phase2 FF is disabled' do - before do - stub_feature_flags(route_hll_to_snowplow_phase2: false) - end - - it 'does not track Snowplow event' do - request - - expect_no_snowplow_event + it_behaves_like 'Snowplow event tracking with RedisHLL context' do + subject(:api_request) { request } + + let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } + let(:category) { described_class.name } + let(:action) { 'terraform_state_api_request' } + let(:label) { 'redis_hll_counters.terraform.p_terraform_state_api_unique_users_monthly' } + let(:namespace) { project.namespace.reload } + let(:user) { current_user } + let(:context) do + payload = Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, + event: 'p_terraform_state_api_unique_users').to_context + [Gitlab::Json.dump(payload)] end end end @@ -318,7 +311,7 @@ RSpec.describe API::Terraform::State, :snowplow do Version: '0.1', Operation: 'OperationTypePlan', Info: '', - Who: "#{current_user.username}", + Who: current_user.username.to_s, Created: Time.now.utc.iso8601(6), Path: '' } @@ -365,7 +358,7 @@ RSpec.describe API::Terraform::State, :snowplow do Version: '0.1', Operation: 'OperationTypePlan', Info: '', - Who: "#{current_user.username}", + Who: current_user.username.to_s, Created: Time.now.utc.iso8601(6), Path: '' } diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index 0fcb6412a2d..7a626ee4d29 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -231,7 +231,7 @@ RSpec.describe API::Todos do create(:on_commit_todo, project: new_todo.project, author: author_1, user: john_doe, target: merge_request_3) create(:todo, project: new_todo.project, author: author_2, user: john_doe, target: merge_request_3) - expect { get api('/todos', john_doe) }.not_to exceed_query_limit(control1).with_threshold(4) + expect { get api('/todos', john_doe) }.not_to exceed_query_limit(control1).with_threshold(5) control2 = ActiveRecord::QueryRecorder.new { get api('/todos', john_doe) } create_issue_todo_for(john_doe) diff --git a/spec/requests/api/unleash_spec.rb b/spec/requests/api/unleash_spec.rb index 51c567309b7..4d382f91023 100644 --- a/spec/requests/api/unleash_spec.rb +++ b/spec/requests/api/unleash_spec.rb @@ -202,18 +202,6 @@ RSpec.describe API::Unleash do 3.times { get api(features_url), params: params, headers: headers } end - - context 'when cache_unleash_client_api is disabled' do - before do - stub_feature_flags(cache_unleash_client_api: false) - end - - it 'serializes feature flags every time' do - expect(::API::Entities::UnleashFeature).to receive(:represent).exactly(5).times - - 5.times { get api(features_url), params: params, headers: headers } - end - end end context 'with version 2 feature flags' do diff --git a/spec/requests/api/user_counts_spec.rb b/spec/requests/api/user_counts_spec.rb index ab2aa87d1b7..369ae49de08 100644 --- a/spec/requests/api/user_counts_spec.rb +++ b/spec/requests/api/user_counts_spec.rb @@ -26,24 +26,22 @@ RSpec.describe API::UserCounts do expect(json_response['assigned_issues']).to eq(1) end - context 'merge requests' do - it 'returns assigned MR counts for current user' do - get api('/user_counts', user) + it 'returns assigned MR counts for current user' do + get api('/user_counts', user) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_a Hash - expect(json_response['merge_requests']).to eq(1) - end + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a Hash + expect(json_response['merge_requests']).to eq(1) + end - it 'updates the mr count when a new mr is assigned' do - create(:merge_request, source_project: project, author: user, assignees: [user]) + it 'updates the mr count when a new mr is assigned' do + create(:merge_request, source_project: project, author: user, assignees: [user]) - get api('/user_counts', user) + get api('/user_counts', user) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_a Hash - expect(json_response['merge_requests']).to eq(2) - end + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a Hash + expect(json_response['merge_requests']).to eq(2) end it 'returns pending todo counts for current_user' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 1b0a27e78e3..6688a998a1a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1291,6 +1291,20 @@ RSpec.describe API::Users do .to eq([Gitlab::PathRegex.namespace_format_message]) end + it 'tracks weak password errors' do + attributes = attributes_for(:user).merge({ password: "password" }) + post api('/users', admin), params: attributes + + expect(json_response['message']['password']) + .to eq(['must not contain commonly used combinations of words and letters']) + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'API::Users', + method: 'create' + ) + end + it "is not available for non admin users" do post api("/users", user), params: attributes_for(:user) expect(response).to have_gitlab_http_status(:forbidden) @@ -1492,6 +1506,21 @@ RSpec.describe API::Users do .not_to have_enqueued_mail(DeviseMailer, :password_change) end end + + context 'with a weak password' do + it 'tracks weak password errors' do + update_password(user, admin, "password") + + expect(json_response['message']['password']) + .to eq(['must not contain commonly used combinations of words and letters']) + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'API::Users', + method: 'update' + ) + end + end end it "updates user with new bio" do @@ -2535,32 +2564,12 @@ RSpec.describe API::Users do describe "DELETE /users/:id" do let_it_be(:issue) { create(:issue, author: user) } - context 'user deletion' do - context 'when user_destroy_with_limited_execution_time_worker is enabled' do - it "deletes user", :sidekiq_inline do - perform_enqueued_jobs { delete api("/users/#{user.id}", admin) } - - expect(response).to have_gitlab_http_status(:no_content) - expect(Users::GhostUserMigration.where(user: user, - initiator_user: admin)).to be_exists - end - end - - context 'when user_destroy_with_limited_execution_time_worker is disabled' do - before do - stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) - end - - it "deletes user", :sidekiq_inline do - namespace_id = user.namespace.id - - perform_enqueued_jobs { delete api("/users/#{user.id}", admin) } + it "deletes user", :sidekiq_inline do + perform_enqueued_jobs { delete api("/users/#{user.id}", admin) } - expect(response).to have_gitlab_http_status(:no_content) - expect { User.find(user.id) }.to raise_error ActiveRecord::RecordNotFound - expect { Namespace.find(namespace_id) }.to raise_error ActiveRecord::RecordNotFound - end - end + expect(response).to have_gitlab_http_status(:no_content) + expect(Users::GhostUserMigration.where(user: user, + initiator_user: admin)).to be_exists end context "sole owner of a group" do @@ -2624,55 +2633,26 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:not_found) end - context 'hard delete' do - context 'when user_destroy_with_limited_execution_time_worker is enabled' do - context "hard delete disabled" do - it "moves contributions to the ghost user", :sidekiq_might_not_need_inline do - perform_enqueued_jobs { delete api("/users/#{user.id}", admin) } - - expect(response).to have_gitlab_http_status(:no_content) - expect(issue.reload).to be_persisted - expect(Users::GhostUserMigration.where(user: user, - initiator_user: admin, - hard_delete: false)).to be_exists - end - end - - context "hard delete enabled" do - it "removes contributions", :sidekiq_might_not_need_inline do - perform_enqueued_jobs { delete api("/users/#{user.id}?hard_delete=true", admin) } + context "hard delete disabled" do + it "moves contributions to the ghost user", :sidekiq_might_not_need_inline do + perform_enqueued_jobs { delete api("/users/#{user.id}", admin) } - expect(response).to have_gitlab_http_status(:no_content) - expect(Users::GhostUserMigration.where(user: user, - initiator_user: admin, - hard_delete: true)).to be_exists - end - end + expect(response).to have_gitlab_http_status(:no_content) + expect(issue.reload).to be_persisted + expect(Users::GhostUserMigration.where(user: user, + initiator_user: admin, + hard_delete: false)).to be_exists end + end - context 'when user_destroy_with_limited_execution_time_worker is disabled' do - before do - stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) - end - - context "hard delete disabled" do - it "moves contributions to the ghost user", :sidekiq_might_not_need_inline do - perform_enqueued_jobs { delete api("/users/#{user.id}", admin) } - - expect(response).to have_gitlab_http_status(:no_content) - expect(issue.reload).to be_persisted - expect(issue.author.ghost?).to be_truthy - end - end - - context "hard delete enabled" do - it "removes contributions", :sidekiq_might_not_need_inline do - perform_enqueued_jobs { delete api("/users/#{user.id}?hard_delete=true", admin) } + context "hard delete enabled" do + it "removes contributions", :sidekiq_might_not_need_inline do + perform_enqueued_jobs { delete api("/users/#{user.id}?hard_delete=true", admin) } - expect(response).to have_gitlab_http_status(:no_content) - expect(Issue.exists?(issue.id)).to be_falsy - end - end + expect(response).to have_gitlab_http_status(:no_content) + expect(Users::GhostUserMigration.where(user: user, + initiator_user: admin, + hard_delete: true)).to be_exists end end end @@ -4395,6 +4375,74 @@ RSpec.describe API::Users do end end + describe 'GET /users/:id/associations_count' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :public, group: group) } + let(:associations) do + { + groups_count: 1, + projects_count: 1, + issues_count: 2, + merge_requests_count: 1 + }.as_json + end + + before :all do + group.add_member(user, Gitlab::Access::OWNER) + project.add_member(user, Gitlab::Access::OWNER) + create(:merge_request, source_project: project, source_branch: "my-personal-branch-1", author: user) + create_list(:issue, 2, project: project, author: user) + end + + context 'as an unauthorized user' do + it 'returns 401 unauthorized' do + get api("/users/#{user.id}/associations_count", nil) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'as a non-admin user' do + context 'with a different user id' do + it 'returns 403 Forbidden' do + get api("/users/#{omniauth_user.id}/associations_count", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with the current user id' do + it 'returns valid JSON response' do + get api("/users/#{user.id}/associations_count", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a Hash + expect(json_response).to match(associations) + end + end + end + + context 'as an admin user' do + context 'with invalid user id' do + it 'returns 404 User Not Found' do + get api("/users/#{non_existing_record_id}/associations_count", admin) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with valid user id' do + it 'returns valid JSON response' do + get api("/users/#{user.id}/associations_count", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a Hash + expect(json_response).to match(associations) + end + end + end + end + it_behaves_like 'custom attributes endpoints', 'users' do let(:attributable) { user } let(:other_attributable) { admin } |