From 71786ddc8e28fbd3cb3fcc4b3ff15e5962a1c82e Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 20 Feb 2023 13:49:51 +0000 Subject: Add latest changes from gitlab-org/gitlab@15-9-stable-ee --- spec/requests/abuse_reports_controller_spec.rb | 38 +++- .../admin/background_migrations_controller_spec.rb | 2 +- .../admin/batched_background_migrations_spec.rb | 8 +- spec/requests/api/api_spec.rb | 49 ++++- spec/requests/api/appearance_spec.rb | 43 ++-- spec/requests/api/applications_spec.rb | 77 +++---- spec/requests/api/avatar_spec.rb | 2 +- spec/requests/api/branches_spec.rb | 2 +- spec/requests/api/bulk_imports_spec.rb | 48 +++-- spec/requests/api/ci/job_artifacts_spec.rb | 14 +- spec/requests/api/ci/jobs_spec.rb | 28 ++- spec/requests/api/ci/runner/jobs_put_spec.rb | 5 +- .../api/ci/runner/jobs_request_post_spec.rb | 59 +++++- spec/requests/api/ci/runner/runners_reset_spec.rb | 5 +- .../api/ci/runner/runners_verify_post_spec.rb | 100 +++++++-- spec/requests/api/ci/runners_spec.rb | 10 +- spec/requests/api/ci/secure_files_spec.rb | 6 +- spec/requests/api/ci/variables_spec.rb | 129 +++++++----- spec/requests/api/debian_group_packages_spec.rb | 12 ++ spec/requests/api/debian_project_packages_spec.rb | 41 ++++ spec/requests/api/discussions_spec.rb | 3 +- spec/requests/api/draft_notes_spec.rb | 178 ++++++++++++++++ spec/requests/api/events_spec.rb | 2 +- .../api/graphql/boards/board_list_query_spec.rb | 18 +- spec/requests/api/graphql/ci/ci_cd_setting_spec.rb | 1 + .../api/graphql/ci/config_variables_spec.rb | 6 +- .../api/graphql/ci/group_variables_spec.rb | 30 ++- spec/requests/api/graphql/ci/groups_spec.rb | 5 +- .../api/graphql/ci/instance_variables_spec.rb | 24 +++ spec/requests/api/graphql/ci/jobs_spec.rb | 2 +- .../api/graphql/ci/project_variables_spec.rb | 30 ++- spec/requests/api/graphql/ci/runner_spec.rb | 112 ++++++++++- spec/requests/api/graphql/gitlab_schema_spec.rb | 99 ++++++++- .../api/graphql/group/group_releases_spec.rb | 139 +++++++++++++ spec/requests/api/graphql/groups_query_spec.rb | 76 +++++++ spec/requests/api/graphql/issue/issue_spec.rb | 41 ++++ spec/requests/api/graphql/issues_spec.rb | 24 +-- .../graphql/mutations/achievements/create_spec.rb | 9 +- .../ci/job_token_scope/add_project_spec.rb | 2 +- .../ci/job_token_scope/remove_project_spec.rb | 22 +- .../mutations/ci/pipeline_schedule_play_spec.rb | 14 +- .../mutations/ci/pipeline_schedule_update_spec.rb | 151 ++++++++++++++ .../ci/project_ci_cd_settings_update_spec.rb | 12 +- .../graphql/mutations/issues/bulk_update_spec.rb | 177 ++++++++++++++++ .../mutations/merge_requests/set_milestone_spec.rb | 16 ++ .../graphql/mutations/notes/create/note_spec.rb | 4 +- .../api/graphql/mutations/notes/destroy_spec.rb | 3 +- .../graphql/mutations/notes/update/note_spec.rb | 3 +- .../mutations/user_preferences/update_spec.rb | 2 +- .../graphql/mutations/work_items/update_spec.rb | 214 +++++++++++++++++++- spec/requests/api/graphql/notes/note_spec.rb | 104 ++++++++++ .../graphql/notes/synthetic_note_resolver_spec.rb | 58 ++++++ spec/requests/api/graphql/packages/package_spec.rb | 108 ++++++++++ .../project/alert_management/alerts_spec.rb | 2 +- .../api/graphql/project/merge_request_spec.rb | 3 +- .../api/graphql/project/project_statistics_spec.rb | 2 +- spec/requests/api/graphql/project/releases_spec.rb | 224 +-------------------- .../api/graphql/project/work_items_spec.rb | 23 ++- .../graphql/subscriptions/notes/created_spec.rb | 177 ++++++++++++++++ .../graphql/subscriptions/notes/deleted_spec.rb | 72 +++++++ .../graphql/subscriptions/notes/updated_spec.rb | 67 ++++++ spec/requests/api/graphql/user_spec.rb | 2 +- spec/requests/api/graphql/work_item_spec.rb | 21 +- spec/requests/api/group_variables_spec.rb | 105 ++++++---- spec/requests/api/internal/base_spec.rb | 6 + spec/requests/api/internal/kubernetes_spec.rb | 2 +- spec/requests/api/invitations_spec.rb | 2 +- spec/requests/api/issue_links_spec.rb | 2 + spec/requests/api/issues/issues_spec.rb | 31 --- .../api/issues/post_projects_issues_spec.rb | 12 +- .../api/issues/put_projects_issues_spec.rb | 12 +- spec/requests/api/maven_packages_spec.rb | 2 + spec/requests/api/merge_requests_spec.rb | 12 -- spec/requests/api/namespaces_spec.rb | 9 + spec/requests/api/notes_spec.rb | 3 +- spec/requests/api/project_attributes.yml | 13 +- spec/requests/api/project_events_spec.rb | 2 +- spec/requests/api/project_packages_spec.rb | 14 +- spec/requests/api/project_snippets_spec.rb | 3 - spec/requests/api/projects_spec.rb | 154 ++++++++++++-- spec/requests/api/release/links_spec.rb | 12 +- spec/requests/api/releases_spec.rb | 69 +++++-- spec/requests/api/snippets_spec.rb | 2 - spec/requests/api/users_preferences_spec.rb | 2 +- spec/requests/api/users_spec.rb | 2 +- spec/requests/git_http_spec.rb | 39 ++-- .../groups/usage_quotas_controller_spec.rb | 2 +- .../jira_connect/public_keys_controller_spec.rb | 25 +-- spec/requests/openid_connect_spec.rb | 6 +- spec/requests/profiles/keys_controller_spec.rb | 31 +++ .../profiles/saved_replies_controller_spec.rb | 35 ++++ .../projects/airflow/dags_controller_spec.rb | 105 ++++++++++ spec/requests/projects/blob_spec.rb | 87 ++++++++ .../google_cloud/databases_controller_spec.rb | 84 +++++--- .../projects/ml/experiments_controller_spec.rb | 152 +++++++++++--- spec/requests/projects/network_controller_spec.rb | 11 - spec/requests/projects/noteable_notes_spec.rb | 36 ++++ .../requests/projects/pipelines_controller_spec.rb | 26 +++ spec/requests/projects/releases_controller_spec.rb | 40 +++- spec/requests/pwa_controller_spec.rb | 84 +++++--- spec/requests/user_activity_spec.rb | 2 +- spec/requests/user_avatar_spec.rb | 2 +- spec/requests/verifies_with_email_spec.rb | 9 + 103 files changed, 3426 insertions(+), 781 deletions(-) create mode 100644 spec/requests/api/draft_notes_spec.rb create mode 100644 spec/requests/api/graphql/group/group_releases_spec.rb create mode 100644 spec/requests/api/graphql/groups_query_spec.rb create mode 100644 spec/requests/api/graphql/mutations/ci/pipeline_schedule_update_spec.rb create mode 100644 spec/requests/api/graphql/mutations/issues/bulk_update_spec.rb create mode 100644 spec/requests/api/graphql/notes/note_spec.rb create mode 100644 spec/requests/api/graphql/notes/synthetic_note_resolver_spec.rb create mode 100644 spec/requests/api/graphql/subscriptions/notes/created_spec.rb create mode 100644 spec/requests/api/graphql/subscriptions/notes/deleted_spec.rb create mode 100644 spec/requests/api/graphql/subscriptions/notes/updated_spec.rb create mode 100644 spec/requests/profiles/keys_controller_spec.rb create mode 100644 spec/requests/profiles/saved_replies_controller_spec.rb create mode 100644 spec/requests/projects/airflow/dags_controller_spec.rb create mode 100644 spec/requests/projects/blob_spec.rb (limited to 'spec/requests') diff --git a/spec/requests/abuse_reports_controller_spec.rb b/spec/requests/abuse_reports_controller_spec.rb index 49a80689c65..934f123e45b 100644 --- a/spec/requests/abuse_reports_controller_spec.rb +++ b/spec/requests/abuse_reports_controller_spec.rb @@ -5,9 +5,12 @@ require 'spec_helper' RSpec.describe AbuseReportsController, feature_category: :insider_threat do let(:reporter) { create(:user) } let(:user) { create(:user) } + let(:abuse_category) { 'spam' } + let(:attrs) do attributes_for(:abuse_report) do |hash| hash[:user_id] = user.id + hash[:category] = abuse_category end end @@ -55,8 +58,6 @@ RSpec.describe AbuseReportsController, feature_category: :insider_threat do describe 'POST add_category', :aggregate_failures do subject(:request) { post add_category_abuse_reports_path, params: request_params } - let(:abuse_category) { 'spam' } - context 'when user is reported for abuse' do let(:ref_url) { 'http://example.com' } let(:request_params) do @@ -80,6 +81,17 @@ RSpec.describe AbuseReportsController, feature_category: :insider_threat do reported_from_url: ref_url ) end + + it 'tracks the snowplow event' do + subject + + expect_snowplow_event( + category: 'ReportAbuse', + action: 'select_abuse_category', + property: abuse_category, + user: user + ) + end end context 'when abuse_report is missing in params' do @@ -149,15 +161,35 @@ RSpec.describe AbuseReportsController, feature_category: :insider_threat do expect(response).to redirect_to root_path end + + it 'tracks the snowplow event' do + post abuse_reports_path(abuse_report: attrs) + + expect_snowplow_event( + category: 'ReportAbuse', + action: 'submit_form', + property: abuse_category, + user: user + ) + end end context 'with invalid attributes' do - it 'redirects back to root' do + before do attrs.delete(:user_id) + end + + it 'redirects back to root' do post abuse_reports_path(abuse_report: attrs) expect(response).to redirect_to root_path end + + it 'does not track the snowplow event' do + post abuse_reports_path(abuse_report: attrs) + + expect_no_snowplow_event + end end end end diff --git a/spec/requests/admin/background_migrations_controller_spec.rb b/spec/requests/admin/background_migrations_controller_spec.rb index db3e2fa0df6..88d81766e67 100644 --- a/spec/requests/admin/background_migrations_controller_spec.rb +++ b/spec/requests/admin/background_migrations_controller_spec.rb @@ -82,7 +82,7 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode, featur it 'returns CI database records' do # If we only have one DB we'll see both migrations - skip_if_multiple_databases_not_setup + skip_if_multiple_databases_not_setup(:ci) ci_database_migration = Gitlab::Database::SharedModel.using_connection(ci_model.connection) { create(:batched_background_migration, :active) } diff --git a/spec/requests/api/admin/batched_background_migrations_spec.rb b/spec/requests/api/admin/batched_background_migrations_spec.rb index 9712777d261..d946ac17f3f 100644 --- a/spec/requests/api/admin/batched_background_migrations_spec.rb +++ b/spec/requests/api/admin/batched_background_migrations_spec.rb @@ -37,7 +37,7 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations, feature_category: :datab context 'when multiple database is enabled' do before do - skip_if_multiple_databases_not_setup + skip_if_multiple_databases_not_setup(:ci) end let(:ci_model) { Ci::ApplicationRecord } @@ -121,7 +121,7 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations, feature_category: :datab it 'returns CI database records' do # If we only have one DB we'll see both migrations - skip_if_multiple_databases_not_setup + skip_if_multiple_databases_not_setup(:ci) ci_database_migration = Gitlab::Database::SharedModel.using_connection(ci_model.connection) do create(:batched_background_migration, :active, gitlab_schema: schema) @@ -194,7 +194,7 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations, feature_category: :datab let(:database) { :ci } before do - skip_if_multiple_databases_not_setup + skip_if_multiple_databases_not_setup(:ci) end it 'uses the correct connection' do @@ -262,7 +262,7 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations, feature_category: :datab let(:database) { :ci } before do - skip_if_multiple_databases_not_setup + skip_if_multiple_databases_not_setup(:ci) end it 'uses the correct connection' do diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index 9cf9c313f11..35851fff6c8 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -12,8 +12,22 @@ RSpec.describe API::API, feature_category: :authentication_and_authorization do let(:user) { create(:user, last_activity_on: Date.yesterday) } it 'updates the users last_activity_on to the current date' do + expect(Users::ActivityService).to receive(:new).with(author: user, project: nil, namespace: nil).and_call_original + expect { get api('/groups', user) }.to change { user.reload.last_activity_on }.to(Date.today) end + + context "with a project-specific path" do + let_it_be(:project) { create(:project, :public) } + let_it_be(:user) { project.first_owner } + + it "passes correct arguments to ActivityService" do + activity_args = { author: user, project: project, namespace: project.group } + expect(Users::ActivityService).to receive(:new).with(activity_args).and_call_original + + get(api("/projects/#{project.id}/issues", user)) + end + end end describe 'User with only read_api scope personal access token' do @@ -171,7 +185,7 @@ RSpec.describe API::API, feature_category: :authentication_and_authorization do 'meta.remote_ip' => an_instance_of(String), 'meta.client_id' => a_string_matching(%r{\Auser/.+}), 'meta.user' => user.username, - 'meta.feature_category' => 'users', + 'meta.feature_category' => 'user_profile', 'route' => '/api/:version/users') expect(data.stringify_keys).not_to include('meta.caller_id') @@ -312,4 +326,37 @@ RSpec.describe API::API, feature_category: :authentication_and_authorization do end end end + + describe 'admin mode support' do + let(:admin) { create(:admin) } + + subject do + get api("/admin/clusters", personal_access_token: token) + response + end + + context 'with `admin_mode` scope' do + let(:token) { create(:personal_access_token, user: admin, scopes: [:api, :admin_mode]) } + + context 'when admin mode setting is disabled', :do_not_mock_admin_mode_setting do + it { is_expected.to have_gitlab_http_status(:ok) } + end + + context 'when admin mode setting is enabled' do + it { is_expected.to have_gitlab_http_status(:ok) } + end + end + + context 'without `admin_mode` scope' do + let(:token) { create(:personal_access_token, user: admin, scopes: [:api]) } + + context 'when admin mode setting is disabled', :do_not_mock_admin_mode_setting do + it { is_expected.to have_gitlab_http_status(:ok) } + end + + context 'when admin mode setting is enabled' do + it { is_expected.to have_gitlab_http_status(:forbidden) } + end + end + end end diff --git a/spec/requests/api/appearance_spec.rb b/spec/requests/api/appearance_spec.rb index 5aba7e096a7..c08ecae28e8 100644 --- a/spec/requests/api/appearance_spec.rb +++ b/spec/requests/api/appearance_spec.rb @@ -5,21 +5,15 @@ require 'spec_helper' RSpec.describe API::Appearance, 'Appearance', feature_category: :navigation do let_it_be(:user) { create(:user) } let_it_be(:admin) { create(:admin) } + let_it_be(:path) { "/application/appearance" } describe "GET /application/appearance" do - context 'as a non-admin user' do - it "returns 403" do - get api("/application/appearance", user) - - expect(response).to have_gitlab_http_status(:forbidden) - end - end + it_behaves_like 'GET request permissions for admin mode' context 'as an admin user' do it "returns appearance" do - get api("/application/appearance", admin) + get api("/application/appearance", admin, admin_mode: true) - expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Hash expect(json_response['description']).to eq('') expect(json_response['email_header_and_footer_enabled']).to be(false) @@ -34,32 +28,29 @@ RSpec.describe API::Appearance, 'Appearance', feature_category: :navigation do expect(json_response['new_project_guidelines']).to eq('') expect(json_response['profile_image_guidelines']).to eq('') expect(json_response['title']).to eq('') + expect(json_response['pwa_name']).to eq('') expect(json_response['pwa_short_name']).to eq('') + expect(json_response['pwa_description']).to eq('') end end end describe "PUT /application/appearance" do - context 'as a non-admin user' do - it "returns 403" do - put api("/application/appearance", user), params: { title: "Test" } - - expect(response).to have_gitlab_http_status(:forbidden) - end - end + it_behaves_like 'PUT request permissions for admin mode', { title: "Test" } context 'as an admin user' do context "instance basics" do it "allows updating the settings" do - put api("/application/appearance", admin), params: { + put api("/application/appearance", admin, admin_mode: true), params: { title: "GitLab Test Instance", - pwa_short_name: "GitLab PWA", description: "gitlab-test.example.com", + pwa_name: "GitLab PWA Test", + pwa_short_name: "GitLab PWA", + pwa_description: "This is GitLab as PWA", new_project_guidelines: "Please read the FAQs for help.", profile_image_guidelines: "Custom profile image guidelines" } - expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Hash expect(json_response['description']).to eq('gitlab-test.example.com') expect(json_response['email_header_and_footer_enabled']).to be(false) @@ -74,7 +65,9 @@ RSpec.describe API::Appearance, 'Appearance', feature_category: :navigation do expect(json_response['new_project_guidelines']).to eq('Please read the FAQs for help.') expect(json_response['profile_image_guidelines']).to eq('Custom profile image guidelines') expect(json_response['title']).to eq('GitLab Test Instance') + expect(json_response['pwa_name']).to eq('GitLab PWA Test') expect(json_response['pwa_short_name']).to eq('GitLab PWA') + expect(json_response['pwa_description']).to eq('This is GitLab as PWA') end end @@ -88,7 +81,7 @@ RSpec.describe API::Appearance, 'Appearance', feature_category: :navigation do email_header_and_footer_enabled: true } - put api("/application/appearance", admin), params: settings + put api("/application/appearance", admin, admin_mode: true), params: settings expect(response).to have_gitlab_http_status(:ok) settings.each do |attribute, value| @@ -98,14 +91,14 @@ RSpec.describe API::Appearance, 'Appearance', feature_category: :navigation do context "fails on invalid color values" do it "with message_font_color" do - put api("/application/appearance", admin), params: { message_font_color: "No Color" } + put api("/application/appearance", admin, admin_mode: true), params: { message_font_color: "No Color" } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']['message_font_color']).to contain_exactly('must be a valid color code') end it "with message_background_color" do - put api("/application/appearance", admin), params: { message_background_color: "#1" } + put api("/application/appearance", admin, admin_mode: true), params: { message_background_color: "#1" } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']['message_background_color']).to contain_exactly('must be a valid color code') @@ -117,7 +110,7 @@ RSpec.describe API::Appearance, 'Appearance', feature_category: :navigation do let_it_be(:appearance) { create(:appearance) } it "allows updating the image files" do - put api("/application/appearance", admin), params: { + put api("/application/appearance", admin, admin_mode: true), params: { logo: fixture_file_upload("spec/fixtures/dk.png", "image/png"), header_logo: fixture_file_upload("spec/fixtures/dk.png", "image/png"), pwa_icon: fixture_file_upload("spec/fixtures/dk.png", "image/png"), @@ -133,14 +126,14 @@ RSpec.describe API::Appearance, 'Appearance', feature_category: :navigation do context "fails on invalid color images" do it "with string instead of file" do - put api("/application/appearance", admin), params: { logo: 'not-a-file.png' } + put api("/application/appearance", admin, admin_mode: true), params: { logo: 'not-a-file.png' } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']).to eq("logo is invalid") end it "with .svg file instead of .png" do - put api("/application/appearance", admin), params: { favicon: fixture_file_upload("spec/fixtures/logo_sample.svg", "image/svg") } + put api("/application/appearance", admin, admin_mode: true), params: { favicon: fixture_file_upload("spec/fixtures/logo_sample.svg", "image/svg") } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']['favicon']).to contain_exactly("You are not allowed to upload \"svg\" files, allowed types: png, ico") diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index e238a1fb554..b81cdcfea8e 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -3,21 +3,23 @@ require 'spec_helper' RSpec.describe API::Applications, :api, feature_category: :authentication_and_authorization do - let(:admin_user) { create(:user, admin: true) } - let(:user) { create(:user, admin: false) } - let(:scopes) { 'api' } + let_it_be(:admin) { create(:admin) } + let_it_be(:user) { create(:user) } + let_it_be(:scopes) { 'api' } + let_it_be(:path) { "/applications" } let!(:application) { create(:application, name: 'another_application', owner: nil, redirect_uri: 'http://other_application.url', scopes: scopes) } describe 'POST /applications' do + it_behaves_like 'POST request permissions for admin mode', { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'api' } + context 'authenticated and authorized user' do it 'creates and returns an OAuth application' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes } + post api(path, admin, admin_mode: true), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes } end.to change { Doorkeeper::Application.count }.by 1 application = Doorkeeper::Application.find_by(name: 'application_name', redirect_uri: 'http://application.url') - expect(response).to have_gitlab_http_status(:created) expect(json_response).to be_a Hash expect(json_response['application_id']).to eq application.uid expect(json_response['secret']).to eq application.secret @@ -28,7 +30,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au it 'does not allow creating an application with the wrong redirect_uri format' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://', scopes: scopes } + post api(path, admin, admin_mode: true), params: { name: 'application_name', redirect_uri: 'http://', scopes: scopes } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -38,7 +40,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au it 'does not allow creating an application with a forbidden URI format' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'javascript://alert()', scopes: scopes } + post api(path, admin, admin_mode: true), params: { name: 'application_name', redirect_uri: 'javascript://alert()', scopes: scopes } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -48,7 +50,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au it 'does not allow creating an application without a name' do expect do - post api('/applications', admin_user), params: { redirect_uri: 'http://application.url', scopes: scopes } + post api(path, admin, admin_mode: true), params: { redirect_uri: 'http://application.url', scopes: scopes } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -58,7 +60,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au it 'does not allow creating an application without a redirect_uri' do expect do - post api('/applications', admin_user), params: { name: 'application_name', scopes: scopes } + post api(path, admin, admin_mode: true), params: { name: 'application_name', scopes: scopes } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -68,7 +70,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au it 'does not allow creating an application without specifying `scopes`' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url' } + post api(path, admin, admin_mode: true), params: { name: 'application_name', redirect_uri: 'http://application.url' } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -78,7 +80,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au it 'does not allow creating an application with blank `scopes`' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '' } + post api(path, admin, admin_mode: true), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: '' } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -87,7 +89,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au it 'does not allow creating an application with invalid `scopes`' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'non_existent_scope' } + post api(path, admin, admin_mode: true), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'non_existent_scope' } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -97,7 +99,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au context 'multiple scopes' do it 'creates an application with multiple `scopes` when each scope specified is seperated by a space' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'api read_user' } + post api(path, admin, admin_mode: true), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'api read_user' } end.to change { Doorkeeper::Application.count }.by 1 application = Doorkeeper::Application.last @@ -108,7 +110,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au it 'does not allow creating an application with multiple `scopes` when one of the scopes is invalid' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'api non_existent_scope' } + post api(path, admin, admin_mode: true), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: 'api non_existent_scope' } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -118,7 +120,7 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au it 'defaults to creating an application with confidential' do expect do - post api('/applications', admin_user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes, confidential: nil } + post api(path, admin, admin_mode: true), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes, confidential: nil } end.to change { Doorkeeper::Application.count }.by(1) expect(response).to have_gitlab_http_status(:created) @@ -133,15 +135,13 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au expect do post api('/applications', user), params: { name: 'application_name', redirect_uri: 'http://application.url', scopes: scopes } end.not_to change { Doorkeeper::Application.count } - - expect(response).to have_gitlab_http_status(:forbidden) end end context 'non-authenticated user' do it 'does not create application' do expect do - post api('/applications'), params: { name: 'application_name', redirect_uri: 'http://application.url' } + post api(path), params: { name: 'application_name', redirect_uri: 'http://application.url' } end.not_to change { Doorkeeper::Application.count } expect(response).to have_gitlab_http_status(:unauthorized) @@ -150,26 +150,17 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au end describe 'GET /applications' do - context 'authenticated and authorized user' do - it 'can list application' do - get api('/applications', admin_user) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_a(Array) - end - end + it_behaves_like 'GET request permissions for admin mode' - context 'authorized user without authorization' do - it 'cannot list application' do - get api('/applications', user) + it 'can list application' do + get api(path, admin, admin_mode: true) - expect(response).to have_gitlab_http_status(:forbidden) - end + expect(json_response).to be_a(Array) end context 'non-authenticated user' do it 'cannot list application' do - get api('/applications') + get api(path) expect(response).to have_gitlab_http_status(:unauthorized) end @@ -177,33 +168,29 @@ RSpec.describe API::Applications, :api, feature_category: :authentication_and_au end describe 'DELETE /applications/:id' do + context 'user authorization' do + let!(:path) { "/applications/#{application.id}" } + + it_behaves_like 'DELETE request permissions for admin mode' + end + context 'authenticated and authorized user' do it 'can delete an application' do expect do - delete api("/applications/#{application.id}", admin_user) + delete api("#{path}/#{application.id}", admin, admin_mode: true) end.to change { Doorkeeper::Application.count }.by(-1) - - expect(response).to have_gitlab_http_status(:no_content) end it 'cannot delete non-existing application' do - delete api("/applications/#{non_existing_record_id}", admin_user) + delete api("#{path}/#{non_existing_record_id}", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:not_found) end end - context 'authorized user without authorization' do - it 'cannot delete an application' do - delete api("/applications/#{application.id}", user) - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - context 'non-authenticated user' do it 'cannot delete an application' do - delete api("/applications/#{application.id}") + delete api("#{path}/#{application.id}") expect(response).to have_gitlab_http_status(:unauthorized) end diff --git a/spec/requests/api/avatar_spec.rb b/spec/requests/api/avatar_spec.rb index 8affbe6ec2b..fcef5b6ca78 100644 --- a/spec/requests/api/avatar_spec.rb +++ b/spec/requests/api/avatar_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Avatar, feature_category: :users do +RSpec.describe API::Avatar, feature_category: :user_profile do let(:gravatar_service) { double('GravatarService') } describe 'GET /avatar' do diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index eba1a06b5e4..058ddaebd79 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -279,7 +279,7 @@ RSpec.describe API::Branches, feature_category: :source_code_management do expect do get api(route, current_user), params: { per_page: 100 } - end.not_to exceed_query_limit(control) + end.not_to exceed_query_limit(control).with_threshold(1) end end diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index 4fb4fbe6d5c..23dfe865ba3 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -13,6 +13,8 @@ RSpec.describe API::BulkImports, feature_category: :importers do before do stub_application_setting(bulk_import_enabled: true) + + allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(false) end shared_examples 'disabled feature' do @@ -73,6 +75,24 @@ RSpec.describe API::BulkImports, feature_category: :importers do end describe 'POST /bulk_imports' do + let(:request) { post api('/bulk_imports', user), params: params } + let(:destination_param) { { destination_slug: 'destination_slug' } } + let(:params) do + { + configuration: { + url: 'http://gitlab.example', + access_token: 'access_token' + }, + entities: [ + { + source_type: 'group_entity', + source_full_path: 'full_path', + destination_namespace: 'destination_namespace' + }.merge(destination_param) + ] + } + end + before do allow_next_instance_of(BulkImports::Clients::HTTP) do |instance| allow(instance) @@ -86,23 +106,6 @@ RSpec.describe API::BulkImports, feature_category: :importers do end shared_examples 'starting a new migration' do - let(:request) { post api('/bulk_imports', user), params: params } - let(:params) do - { - configuration: { - url: 'http://gitlab.example', - access_token: 'access_token' - }, - entities: [ - { - source_type: 'group_entity', - source_full_path: 'full_path', - destination_namespace: 'destination_namespace' - }.merge(destination_param) - ] - } - end - it 'starts a new migration' do request @@ -278,6 +281,17 @@ RSpec.describe API::BulkImports, feature_category: :importers do end include_examples 'disabled feature' + + context 'when request exceeds rate limits' do + it 'prevents user from starting a new migration' do + allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) + + request + + expect(response).to have_gitlab_http_status(:too_many_requests) + expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') + end + end end describe 'GET /bulk_imports/entities' do diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index a4a38179d11..ee390773f29 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe API::Ci::JobArtifacts, feature_category: :build_artifacts do include HttpBasicAuthHelpers include DependencyProxyHelpers + include Ci::JobTokenScopeHelpers include HttpIOHelpers @@ -312,7 +313,7 @@ RSpec.describe API::Ci::JobArtifacts, feature_category: :build_artifacts do context 'normal authentication' do context 'job with artifacts' do context 'when artifacts are stored locally' do - let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) } + let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, project: project) } subject { get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) } @@ -329,11 +330,12 @@ RSpec.describe API::Ci::JobArtifacts, feature_category: :build_artifacts do stub_licensed_features(cross_project_pipelines: true) end - it_behaves_like 'downloads artifact' - context 'when job token scope is enabled' do before do - other_job.project.ci_cd_settings.update!(job_token_scope_enabled: true) + other_job.project.ci_cd_settings.update!( + job_token_scope_enabled: true, + inbound_job_token_scope_enabled: true + ) end it 'does not allow downloading artifacts' do @@ -343,7 +345,9 @@ RSpec.describe API::Ci::JobArtifacts, feature_category: :build_artifacts do end context 'when project is added to the job token scope' do - let!(:link) { create(:ci_job_token_project_scope_link, source_project: other_job.project, target_project: job.project) } + before do + make_project_fully_accessible(other_job.project, job.project) + end it_behaves_like 'downloads artifact' end diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 875bfc5b94f..10dd9c3b556 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -126,6 +126,7 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do it 'returns specific job data' do expect(json_response['finished_at']).to be_nil + expect(json_response['erased_at']).to be_nil end it 'avoids N+1 queries', :skip_before_request do @@ -540,21 +541,6 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do expect(json_response.first['id']).to eq(job.id) expect(response.headers).not_to include("Link") end - - context 'with :jobs_api_keyset_pagination disabled' do - before do - stub_feature_flags(jobs_api_keyset_pagination: false) - end - - it 'defaults to offset pagination' do - get api("/projects/#{project.id}/jobs", api_user), params: { pagination: 'keyset', per_page: 1 } - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.size).to eq(1) - expect(json_response.first['id']).to eq(running_job.id) - expect(response.headers["Link"]).not_to include("cursor") - end - end end describe 'GET /projects/:id/jobs rate limited' do @@ -651,6 +637,18 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do end end + context 'when job is erased' do + let(:job) do + create(:ci_build, pipeline: pipeline, erased_at: Time.now) + end + + it 'returns specific job data' do + get api("/projects/#{project.id}/jobs/#{job.id}", api_user) + + expect(Time.parse(json_response['erased_at'])).to be_like_time(job.erased_at) + end + end + context 'when trace artifact record exists with no stored file', :skip_before_request do before do create(:ci_job_artifact, :unarchived_trace_artifact, job: job, project: job.project) diff --git a/spec/requests/api/ci/runner/jobs_put_spec.rb b/spec/requests/api/ci/runner/jobs_put_spec.rb index 22817922b1b..ef3b38e3fc4 100644 --- a/spec/requests/api/ci/runner/jobs_put_spec.rb +++ b/spec/requests/api/ci/runner/jobs_put_spec.rb @@ -21,11 +21,13 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let_it_be(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:runner_machine) { create(:ci_runner_machine, runner: runner) } let_it_be(:user) { create(:user) } describe 'PUT /api/v4/jobs/:id' do let_it_be_with_reload(:job) do - create(:ci_build, :pending, :trace_live, pipeline: pipeline, project: project, user: user, runner_id: runner.id) + create(:ci_build, :pending, :trace_live, pipeline: pipeline, project: project, user: user, + runner_id: runner.id, runner_machine: runner_machine) end before do @@ -38,6 +40,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego it 'updates runner info' do expect { update_job(state: 'success') }.to change { runner.reload.contacted_at } + .and change { runner_machine.reload.contacted_at } end context 'when status is given' do 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 d15bc9d2dd5..6e721d40560 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_category: :runner do +RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_category: :continuous_integration do include StubGitlabCalls include RedisHelpers include WorkhorseHelpers @@ -119,6 +119,63 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end end + context 'when system_id parameter is specified' do + subject(:request) { request_job(**args) } + + context 'with create_runner_machine FF enabled' do + before do + stub_feature_flags(create_runner_machine: true) + end + + context 'when ci_runner_machines with same system_xid does not exist' do + let(:args) { { system_id: 's_some_system_id' } } + + it 'creates respective ci_runner_machines record', :freeze_time do + expect { request }.to change { runner.runner_machines.reload.count }.from(0).to(1) + + machine = runner.runner_machines.last + expect(machine.system_xid).to eq args[:system_id] + expect(machine.runner).to eq runner + expect(machine.contacted_at).to eq Time.current + end + end + + context 'when ci_runner_machines with same system_xid already exists', :freeze_time do + let(:args) { { system_id: 's_existing_system_id' } } + let!(:runner_machine) do + create(:ci_runner_machine, runner: runner, system_xid: args[:system_id], contacted_at: 1.hour.ago) + end + + it 'does not create new ci_runner_machines record' do + expect { request }.not_to change { Ci::RunnerMachine.count } + end + + it 'updates the contacted_at field' do + request + + expect(runner_machine.reload.contacted_at).to eq Time.current + end + end + end + + context 'with create_runner_machine FF disabled' do + before do + stub_feature_flags(create_runner_machine: false) + end + + context 'when ci_runner_machines with same system_xid does not exist' do + let(:args) { { system_id: 's_some_system_id' } } + + it 'does not create respective ci_runner_machines record', :freeze_time, :aggregate_failures do + expect { request }.not_to change { runner.runner_machines.reload.count } + + expect(response).to have_gitlab_http_status(:created) + expect(runner.runner_machines).to be_empty + end + end + end + end + context 'when jobs are finished' do before do job.success diff --git a/spec/requests/api/ci/runner/runners_reset_spec.rb b/spec/requests/api/ci/runner/runners_reset_spec.rb index 6ab21138d26..2d1e366e820 100644 --- a/spec/requests/api/ci/runner/runners_reset_spec.rb +++ b/spec/requests/api/ci/runner/runners_reset_spec.rb @@ -34,9 +34,10 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego expect do post api("/runners/reset_authentication_token"), params: { token: group_runner.reload.token } + group_runner.reload expect(response).to have_gitlab_http_status(:success) - expect(json_response).to eq({ 'token' => group_runner.reload.token, 'token_expires_at' => group_runner.reload.token_expires_at.iso8601(3) }) - expect(group_runner.reload.token_expires_at).to eq(5.days.from_now) + expect(json_response).to eq({ 'token' => group_runner.token, 'token_expires_at' => group_runner.token_expires_at.iso8601(3) }) + expect(group_runner.token_expires_at).to eq(5.days.from_now) end.to change { group_runner.reload.token } end diff --git a/spec/requests/api/ci/runner/runners_verify_post_spec.rb b/spec/requests/api/ci/runner/runners_verify_post_spec.rb index 22a954cc444..a6a1ad947aa 100644 --- a/spec/requests/api/ci/runner/runners_verify_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_verify_post_spec.rb @@ -18,7 +18,11 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego describe '/api/v4/runners' do describe 'POST /api/v4/runners/verify' do - let(:runner) { create(:ci_runner) } + let_it_be_with_reload(:runner) { create(:ci_runner, token_expires_at: 3.days.from_now) } + + let(:params) {} + + subject(:verify) { post api('/runners/verify'), params: params } context 'when no token is provided' do it 'returns 400 error' do @@ -29,46 +33,116 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end context 'when invalid token is provided' do + let(:params) { { token: 'invalid-token' } } + it 'returns 403 error' do - post api('/runners/verify'), params: { token: 'invalid-token' } + verify expect(response).to have_gitlab_http_status(:forbidden) end end context 'when valid token is provided' do - subject { post api('/runners/verify'), params: { token: runner.token } } + let(:params) { { token: runner.token } } + + context 'with create_runner_machine FF enabled' do + before do + stub_feature_flags(create_runner_machine: true) + end + + it 'verifies Runner credentials' do + verify + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ + 'id' => runner.id, + 'token' => runner.token, + 'token_expires_at' => runner.token_expires_at.iso8601(3) + }) + end + + context 'with non-expiring runner token' do + before do + runner.update!(token_expires_at: nil) + end + + it 'verifies Runner credentials' do + verify + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ + 'id' => runner.id, + 'token' => runner.token, + 'token_expires_at' => nil + }) + end + end + + it_behaves_like 'storing arguments in the application context for the API' do + let(:expected_params) { { client_id: "runner/#{runner.id}" } } + end + + context 'when system_id is provided' do + let(:params) { { token: runner.token, system_id: 's_some_system_id' } } + + it 'creates a runner_machine' do + expect { verify }.to change { Ci::RunnerMachine.count }.by(1) + end + end + end - it 'verifies Runner credentials' do - subject + context 'with create_runner_machine FF disabled' do + before do + stub_feature_flags(create_runner_machine: false) + end - expect(response).to have_gitlab_http_status(:ok) - end + it 'verifies Runner credentials' do + verify + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ + 'id' => runner.id, + 'token' => runner.token, + 'token_expires_at' => runner.token_expires_at.iso8601(3) + }) + end + + context 'when system_id is provided' do + let(:params) { { token: runner.token, system_id: 's_some_system_id' } } + + it 'does not create a runner_machine', :aggregate_failures do + expect { verify }.not_to change { Ci::RunnerMachine.count } - it_behaves_like 'storing arguments in the application context for the API' do - let(:expected_params) { { client_id: "runner/#{runner.id}" } } + expect(response).to have_gitlab_http_status(:ok) + end + end end end context 'when non-expired token is provided' do - subject { post api('/runners/verify'), params: { token: runner.token } } + let(:params) { { token: runner.token } } it 'verifies Runner credentials' do runner["token_expires_at"] = 10.days.from_now runner.save! - subject + verify expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ + 'id' => runner.id, + 'token' => runner.token, + 'token_expires_at' => runner.token_expires_at.iso8601(3) + }) end end context 'when expired token is provided' do - subject { post api('/runners/verify'), params: { token: runner.token } } + let(:params) { { token: runner.token } } it 'does not verify Runner credentials' do runner["token_expires_at"] = 10.days.ago runner.save! - subject + verify expect(response).to have_gitlab_http_status(:forbidden) end diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index b07dd388390..ca051386265 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -794,7 +794,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end end - context 'when runner is specific' do + context 'when runner is a project runner' do it 'return jobs' do get api("/runners/#{project_runner.id}/jobs", admin) @@ -947,7 +947,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end end - context 'when runner is specific' do + context 'when runner is a project runner' do it 'return jobs' do get api("/runners/#{project_runner.id}/jobs", user) @@ -1203,7 +1203,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'authorized user' do let_it_be(:project_runner2) { create(:ci_runner, :project, projects: [project2]) } - it 'enables specific runner' do + it 'enables project runner' do expect do post api("/projects/#{project.id}/runners", user), params: { runner_id: project_runner2.id } end.to change { project.runners.count }.by(+1) @@ -1243,7 +1243,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when project runner is used' do let!(:new_project_runner) { create(:ci_runner, :project) } - it 'enables any specific runner' do + it 'enables any project runner' do expect do post api("/projects/#{project.id}/runners", admin), params: { runner_id: new_project_runner.id } end.to change { project.runners.count }.by(+1) @@ -1255,7 +1255,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do create(:plan_limits, :default_plan, ci_registered_project_runners: 1) end - it 'does not enable specific runner' do + it 'does not enable project runner' do expect do post api("/projects/#{project.id}/runners", admin), params: { runner_id: new_project_runner.id } end.not_to change { project.runners.count } diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb index 700fd97152a..fc988800b56 100644 --- a/spec/requests/api/ci/secure_files_spec.rb +++ b/spec/requests/api/ci/secure_files_spec.rb @@ -2,10 +2,9 @@ require 'spec_helper' -RSpec.describe API::Ci::SecureFiles, feature_category: :pipeline_authoring do +RSpec.describe API::Ci::SecureFiles, feature_category: :mobile_devops do before do stub_ci_secure_file_object_storage - stub_feature_flags(ci_secure_files: true) stub_feature_flags(ci_secure_files_read_only: false) end @@ -128,6 +127,7 @@ RSpec.describe API::Ci::SecureFiles, feature_category: :pipeline_authoring do expect(json_response['name']).to eq(secure_file.name) expect(json_response['expires_at']).to be nil expect(json_response['metadata']).to be nil + expect(json_response['file_extension']).to be nil end it 'returns project secure file details with metadata when supported' do @@ -138,6 +138,7 @@ RSpec.describe API::Ci::SecureFiles, feature_category: :pipeline_authoring do 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]) + expect(json_response['file_extension']).to eq('cer') end it 'responds with 404 Not Found if requesting non-existing secure file' do @@ -250,6 +251,7 @@ RSpec.describe API::Ci::SecureFiles, feature_category: :pipeline_authoring do expect(json_response['name']).to eq('upload-keystore.jks') expect(json_response['checksum']).to eq(secure_file.checksum) expect(json_response['checksum_algorithm']).to eq('sha256') + expect(json_response['file_extension']).to eq('jks') secure_file = Ci::SecureFile.find(json_response['id']) expect(secure_file.checksum).to eq( diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index c5d01afb7c4..0f9f1bc80d6 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -114,73 +114,92 @@ RSpec.describe API::Ci::Variables, feature_category: :pipeline_authoring do describe 'POST /projects/:id/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, raw: true } - end.to change { project.variables.count }.by(1) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - 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 + context 'when the project is below the plan limit for variables' 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, raw: true } + end.to change { project.variables.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + 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' } + 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))) + 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 + 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' } - end.to change { project.variables.count }.by(1) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - 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 + 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' } + end.to change { project.variables.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + 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 - it 'does not allow to duplicate variable key' do - expect do - post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } - end.to change { project.variables.count }.by(0) + it 'does not allow to duplicate variable key' do + expect do + post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } + end.to change { project.variables.count }.by(0) - expect(response).to have_gitlab_http_status(:bad_request) - end + expect(response).to have_gitlab_http_status(:bad_request) + end - it 'creates variable with a specific environment scope' do - expect do - post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'VALUE_2', environment_scope: 'review/*' } - end.to change { project.variables.reload.count }.by(1) + it 'creates variable with a specific environment scope' do + expect do + post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') + end + + it 'allows duplicated variable key given different environment scopes' do + variable = create(:ci_variable, project: project) - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - expect(json_response['value']).to eq('VALUE_2') - expect(json_response['environment_scope']).to eq('review/*') + expect do + post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq(variable.key) + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') + end end - it 'allows duplicated variable key given different environment scopes' do - variable = create(:ci_variable, project: project) + context 'when the project is at the plan limit for variables' do + before do + create(:plan_limits, :default_plan, project_ci_variables: 1) + end - expect do - post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2', environment_scope: 'review/*' } - end.to change { project.variables.reload.count }.by(1) + it 'returns a variable limit error' do + expect do + post api("/projects/#{project.id}/variables", user), params: { key: 'TOO_MANY_VARS', value: 'too many' } + end.not_to change { project.variables.count } - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq(variable.key) - expect(json_response['value']).to eq('VALUE_2') - expect(json_response['environment_scope']).to eq('review/*') + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['base']).to contain_exactly( + 'Maximum number of project ci variables (1) exceeded' + ) + end end end diff --git a/spec/requests/api/debian_group_packages_spec.rb b/spec/requests/api/debian_group_packages_spec.rb index f4d5ef3fe90..0c80b7d830f 100644 --- a/spec/requests/api/debian_group_packages_spec.rb +++ b/spec/requests/api/debian_group_packages_spec.rb @@ -36,6 +36,12 @@ RSpec.describe API::DebianGroupPackages, feature_category: :package_registry do it_behaves_like 'Debian packages read endpoint', 'GET', :success, /Description: This is an incomplete Packages file/ end + describe 'GET groups/:id/-/packages/debian/dists/*distribution/:component/binary-:architecture/Packages.gz' do + let(:url) { "/groups/#{container.id}/-/packages/debian/dists/#{distribution.codename}/#{component.name}/binary-#{architecture.name}/Packages.gz" } + + it_behaves_like 'Debian packages read endpoint', 'GET', :not_found, /Format gz is not supported/ + end + describe 'GET groups/:id/-/packages/debian/dists/*distribution/:component/binary-:architecture/by-hash/SHA256/:file_sha256' do let(:url) { "/groups/#{container.id}/-/packages/debian/dists/#{distribution.codename}/#{component.name}/binary-#{architecture.name}/by-hash/SHA256/#{component_file_older_sha256.file_sha256}" } @@ -60,6 +66,12 @@ RSpec.describe API::DebianGroupPackages, feature_category: :package_registry do it_behaves_like 'Debian packages read endpoint', 'GET', :success, /Description: This is an incomplete D-I Packages file/ end + describe 'GET groups/:id/-/packages/debian/dists/*distribution/:component/debian-installer/binary-:architecture/Packages.gz' do + let(:url) { "/groups/#{container.id}/-/packages/debian/dists/#{distribution.codename}/#{component.name}/debian-installer/binary-#{architecture.name}/Packages.gz" } + + it_behaves_like 'Debian packages read endpoint', 'GET', :not_found, /Format gz is not supported/ + end + describe 'GET groups/:id/-/packages/debian/dists/*distribution/:component/debian-installer/binary-:architecture/by-hash/SHA256/:file_sha256' do let(:url) { "/groups/#{container.id}/-/packages/debian/dists/#{distribution.codename}/#{component.name}/debian-installer/binary-#{architecture.name}/by-hash/SHA256/#{component_file_di_older_sha256.file_sha256}" } diff --git a/spec/requests/api/debian_project_packages_spec.rb b/spec/requests/api/debian_project_packages_spec.rb index 5258d26be17..46f79efd928 100644 --- a/spec/requests/api/debian_project_packages_spec.rb +++ b/spec/requests/api/debian_project_packages_spec.rb @@ -50,6 +50,12 @@ RSpec.describe API::DebianProjectPackages, feature_category: :package_registry d it_behaves_like 'accept GET request on private project with access to package registry for everyone' end + describe 'GET projects/:id/packages/debian/dists/*distribution/:component/binary-:architecture/Packages.gz' do + let(:url) { "/projects/#{container.id}/packages/debian/dists/#{distribution.codename}/#{component.name}/binary-#{architecture.name}/Packages.gz" } + + it_behaves_like 'Debian packages read endpoint', 'GET', :not_found, /Format gz is not supported/ + end + describe 'GET projects/:id/packages/debian/dists/*distribution/:component/binary-:architecture/by-hash/SHA256/:file_sha256' do let(:url) { "/projects/#{container.id}/packages/debian/dists/#{distribution.codename}/#{component.name}/binary-#{architecture.name}/by-hash/SHA256/#{component_file_older_sha256.file_sha256}" } @@ -78,6 +84,12 @@ RSpec.describe API::DebianProjectPackages, feature_category: :package_registry d it_behaves_like 'accept GET request on private project with access to package registry for everyone' end + describe 'GET projects/:id/packages/debian/dists/*distribution/:component/debian-installer/binary-:architecture/Packages.gz' do + let(:url) { "/projects/#{container.id}/packages/debian/dists/#{distribution.codename}/#{component.name}/debian-installer/binary-#{architecture.name}/Packages.gz" } + + it_behaves_like 'Debian packages read endpoint', 'GET', :not_found, /Format gz is not supported/ + end + describe 'GET projects/:id/packages/debian/dists/*distribution/:component/debian-installer/binary-:architecture/by-hash/SHA256/:file_sha256' do let(:url) { "/projects/#{container.id}/packages/debian/dists/#{distribution.codename}/#{component.name}/debian-installer/binary-#{architecture.name}/by-hash/SHA256/#{component_file_di_older_sha256.file_sha256}" } @@ -124,6 +136,35 @@ RSpec.describe API::DebianProjectPackages, feature_category: :package_registry d let(:file_name) { 'libsample0_1.2.3~alpha2_amd64.deb' } it_behaves_like 'Debian packages write endpoint', 'upload', :created, nil + + context 'with codename and component' do + let(:extra_params) { { distribution: distribution.codename, component: 'main' } } + + it_behaves_like 'Debian packages write endpoint', 'upload', :created, nil + end + + context 'with codename and without component' do + let(:extra_params) { { distribution: distribution.codename } } + + include_context 'Debian repository access', :public, :developer, :basic do + it_behaves_like 'Debian packages GET request', :bad_request, /component is missing/ + end + end + end + + context 'with a buildinfo' do + let(:file_name) { 'sample_1.2.3~alpha2_amd64.buildinfo' } + + include_context 'Debian repository access', :public, :developer, :basic do + it_behaves_like "Debian packages upload request", :created, nil + + context 'with codename and component' do + let(:extra_params) { { distribution: distribution.codename, component: 'main' } } + + it_behaves_like "Debian packages upload request", :bad_request, + /^file_name Only debs and udebs can be directly added to a distribution$/ + end + end end context 'with a changes file' do diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb index 38016375b8f..c5126dbd1c2 100644 --- a/spec/requests/api/discussions_spec.rb +++ b/spec/requests/api/discussions_spec.rb @@ -42,8 +42,7 @@ RSpec.describe API::Discussions, feature_category: :team_planning do context 'with work item without notes widget' do before do - stub_const('WorkItems::Type::BASE_TYPES', { issue: { name: 'NoNotesWidget', enum_value: 0 } }) - stub_const('WorkItems::Type::WIDGETS_FOR_TYPE', { issue: [::WorkItems::Widgets::Description] }) + WorkItems::Type.default_by_type(:issue).widget_definitions.find_by_widget_type(:notes).update!(disabled: true) end context 'when fetching discussions' do diff --git a/spec/requests/api/draft_notes_spec.rb b/spec/requests/api/draft_notes_spec.rb new file mode 100644 index 00000000000..e8f519e004d --- /dev/null +++ b/spec/requests/api/draft_notes_spec.rb @@ -0,0 +1,178 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::DraftNotes, feature_category: :code_review_workflow do + let_it_be(:user) { create(:user) } + let_it_be(:user_2) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } + + let_it_be(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) } + let!(:draft_note_by_current_user) { create(:draft_note, merge_request: merge_request, author: user) } + let!(:draft_note_by_random_user) { create(:draft_note, merge_request: merge_request) } + + let_it_be(:api_stub) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}" } + + before do + project.add_developer(user) + end + + describe "Get a list of merge request draft notes" do + it "returns 200 OK status" do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes", user) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "returns only draft notes authored by the current user" do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes", user) + + draft_note_ids = json_response.pluck("id") + + expect(draft_note_ids).to include(draft_note_by_current_user.id) + expect(draft_note_ids).not_to include(draft_note_by_random_user.id) + expect(draft_note_ids).not_to include(merge_request_note.id) + end + end + + describe "Get a single draft note" do + context "when requesting an existing draft note by the user" do + before do + get api( + "/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes/#{draft_note_by_current_user.id}", + user + ) + end + + it "returns 200 OK status" do + expect(response).to have_gitlab_http_status(:ok) + end + + it "returns the requested draft note" do + expect(json_response["id"]).to eq(draft_note_by_current_user.id) + end + + context "when requesting a non-existent draft note" do + it "returns a 404 Not Found response" do + get api( + "/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes/#{DraftNote.last.id + 1}", + user + ) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context "when requesting an existing draft note by another user" do + it "returns a 404 Not Found response" do + get api( + "/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes/#{draft_note_by_random_user.id}", + user + ) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + describe "delete a draft note" do + context "when deleting an existing draft note by the user" do + let!(:deleted_draft_note_id) { draft_note_by_current_user.id } + + before do + delete api( + "/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes/#{draft_note_by_current_user.id}", + user + ) + end + + it "returns 204 No Content status" do + expect(response).to have_gitlab_http_status(:no_content) + end + + it "deletes the specified draft note" do + expect(DraftNote.exists?(deleted_draft_note_id)).to eq(false) + end + end + + context "when deleting a non-existent draft note" do + it "returns a 404 Not Found" do + delete api( + "/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes/#{non_existing_record_id}", + user + ) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context "when deleting a draft note by a different user" do + it "returns a 404 Not Found" do + delete api( + "/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes/#{draft_note_by_random_user.id}", + user + ) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe "Publishing a draft note" do + let(:publish_draft_note) do + put api( + "#{api_stub}/draft_notes/#{draft_note_by_current_user.id}/publish", + user + ) + end + + context "when publishing an existing draft note by the user" do + it "returns 204 No Content status" do + publish_draft_note + + expect(response).to have_gitlab_http_status(:no_content) + end + + it "publishes the specified draft note" do + expect { publish_draft_note }.to change { Note.count }.by(1) + expect(DraftNote.exists?(draft_note_by_current_user.id)).to eq(false) + end + end + + context "when publishing a non-existent draft note" do + it "returns a 404 Not Found" do + put api( + "#{api_stub}/draft_notes/#{non_existing_record_id}/publish", + user + ) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context "when publishing a draft note by a different user" do + it "returns a 404 Not Found" do + put api( + "#{api_stub}/draft_notes/#{draft_note_by_random_user.id}/publish", + user + ) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context "when DraftNotes::PublishService returns a non-success" do + it "returns an :internal_server_error and a message" do + expect_next_instance_of(DraftNotes::PublishService) do |instance| + expect(instance).to receive(:execute).and_return({ status: :failure, message: "Error message" }) + end + + publish_draft_note + + expect(response).to have_gitlab_http_status(:internal_server_error) + end + end + end +end diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index 5c061a37ff3..f884aaabb53 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Events, feature_category: :users do +RSpec.describe API::Events, feature_category: :user_profile do let(:user) { create(:user) } let(:non_member) { create(:user) } let(:private_project) { create(:project, :private, creator_id: user.id, namespace: user.namespace) } diff --git a/spec/requests/api/graphql/boards/board_list_query_spec.rb b/spec/requests/api/graphql/boards/board_list_query_spec.rb index b5ed0fe35d5..6ddcf8a13fd 100644 --- a/spec/requests/api/graphql/boards/board_list_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_list_query_spec.rb @@ -9,9 +9,11 @@ RSpec.describe 'Querying a Board list', feature_category: :team_planning do let_it_be(:project) { create(:project) } let_it_be(:board) { create(:board, resource_parent: project) } let_it_be(:label) { create(:label, project: project, name: 'foo') } + let_it_be(:extra_label1) { create(:label, project: project) } + let_it_be(:extra_label2) { create(:label, project: project) } let_it_be(:list) { create(:list, board: board, label: label) } - let_it_be(:issue1) { create(:issue, project: project, labels: [label]) } - let_it_be(:issue2) { create(:issue, project: project, labels: [label], assignees: [current_user]) } + let_it_be(:issue1) { create(:issue, project: project, labels: [label, extra_label1]) } + let_it_be(:issue2) { create(:issue, project: project, labels: [label, extra_label2], assignees: [current_user]) } let_it_be(:issue3) { create(:issue, project: project, labels: [label], confidential: true) } let(:filters) { {} } @@ -66,6 +68,18 @@ RSpec.describe 'Querying a Board list', feature_category: :team_planning do is_expected.to include({ 'issuesCount' => 1, 'title' => list.title }) end end + + context 'when filtering by OR labels' do + let(:filters) { { or: { labelNames: [extra_label1.title, extra_label2.title] } } } + + before_all do + project.add_developer(current_user) + end + + it 'filters issues metadata' do + is_expected.to include({ 'issuesCount' => 2, 'title' => list.title }) + end + end end end diff --git a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb index 0437a30eccd..95cabfea2fc 100644 --- a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb +++ b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb @@ -50,6 +50,7 @@ RSpec.describe 'Getting Ci Cd Setting', feature_category: :continuous_integratio expect(settings_data['jobTokenScopeEnabled']).to eql project.ci_cd_settings.job_token_scope_enabled? expect(settings_data['inboundJobTokenScopeEnabled']).to eql( project.ci_cd_settings.inbound_job_token_scope_enabled?) + expect(settings_data['optInJwt']).to eql project.ci_cd_settings.opt_in_jwt? end end end diff --git a/spec/requests/api/graphql/ci/config_variables_spec.rb b/spec/requests/api/graphql/ci/config_variables_spec.rb index e6d73701b8f..f76bb8ff837 100644 --- a/spec/requests/api/graphql/ci/config_variables_spec.rb +++ b/spec/requests/api/graphql/ci/config_variables_spec.rb @@ -14,13 +14,13 @@ RSpec.describe 'Query.project(fullPath).ciConfigVariables(sha)', feature_categor let_it_be(:user) { create(:user) } let(:service) { Ci::ListConfigVariablesService.new(project, user) } - let(:sha) { project.repository.commit.sha } + let(:ref) { project.default_branch } let(:query) do %( query { project(fullPath: "#{project.full_path}") { - ciConfigVariables(sha: "#{sha}") { + ciConfigVariables(sha: "#{ref}") { key value valueOptions @@ -47,7 +47,7 @@ RSpec.describe 'Query.project(fullPath).ciConfigVariables(sha)', feature_categor it 'returns the CI variables for the config' do expect(service) .to receive(:execute) - .with(sha) + .with(ref) .and_call_original post_graphql(query, current_user: user) diff --git a/spec/requests/api/graphql/ci/group_variables_spec.rb b/spec/requests/api/graphql/ci/group_variables_spec.rb index 51cbb4719f7..d78b30787c9 100644 --- a/spec/requests/api/graphql/ci/group_variables_spec.rb +++ b/spec/requests/api/graphql/ci/group_variables_spec.rb @@ -47,7 +47,7 @@ RSpec.describe 'Query.group(fullPath).ciVariables', feature_category: :pipeline_ post_graphql(query, current_user: user) - expect(graphql_data.dig('group', 'ciVariables', 'limit')).to be(200) + expect(graphql_data.dig('group', 'ciVariables', 'limit')).to be(30000) expect(graphql_data.dig('group', 'ciVariables', 'nodes')).to contain_exactly({ 'id' => variable.to_global_id.to_s, 'key' => 'TEST_VAR', @@ -72,4 +72,32 @@ RSpec.describe 'Query.group(fullPath).ciVariables', feature_category: :pipeline_ expect(graphql_data.dig('group', 'ciVariables')).to be_nil end end + + describe 'sorting and pagination' do + let_it_be(:current_user) { user } + let_it_be(:data_path) { [:group, :ci_variables] } + let_it_be(:variables) do + [ + create(:ci_group_variable, group: group, key: 'd'), + create(:ci_group_variable, group: group, key: 'a'), + create(:ci_group_variable, group: group, key: 'c'), + create(:ci_group_variable, group: group, key: 'e'), + create(:ci_group_variable, group: group, key: 'b') + ] + end + + def pagination_query(params) + graphql_query_for( + :group, + { fullPath: group.full_path }, + query_graphql_field('ciVariables', params, "#{page_info} nodes { id }") + ) + end + + before do + group.add_owner(current_user) + end + + it_behaves_like 'sorted paginated variables' + end end diff --git a/spec/requests/api/graphql/ci/groups_spec.rb b/spec/requests/api/graphql/ci/groups_spec.rb index d1588833d8f..1874e1d35dd 100644 --- a/spec/requests/api/graphql/ci/groups_spec.rb +++ b/spec/requests/api/graphql/ci/groups_spec.rb @@ -10,8 +10,9 @@ RSpec.describe 'Query.project.pipeline.stages.groups', feature_category: :contin let(:group_graphql_data) { graphql_data_at(:project, :pipeline, :stages, :nodes, 0, :groups, :nodes) } let_it_be(:ref) { 'master' } - let_it_be(:job_a) { create(:commit_status, pipeline: pipeline, name: 'rspec 0 2', ref: ref) } - let_it_be(:job_b) { create(:ci_build, pipeline: pipeline, name: 'rspec 0 1', ref: ref) } + let_it_be(:stage) { create(:ci_stage, pipeline: pipeline) } + let_it_be(:job_a) { create(:commit_status, pipeline: pipeline, name: 'rspec 0 2', ref: ref, ci_stage: stage) } + let_it_be(:job_b) { create(:ci_build, pipeline: pipeline, name: 'rspec 0 1', ref: ref, ci_stage: stage) } let_it_be(:job_c) { create(:ci_bridge, pipeline: pipeline, name: 'spinach 0 1', ref: ref) } let(:params) { {} } diff --git a/spec/requests/api/graphql/ci/instance_variables_spec.rb b/spec/requests/api/graphql/ci/instance_variables_spec.rb index e0397e17923..5b65ae88426 100644 --- a/spec/requests/api/graphql/ci/instance_variables_spec.rb +++ b/spec/requests/api/graphql/ci/instance_variables_spec.rb @@ -69,4 +69,28 @@ RSpec.describe 'Query.ciVariables', feature_category: :pipeline_authoring do expect(graphql_data.dig('ciVariables')).to be_nil end end + + describe 'sorting and pagination' do + let_it_be(:current_user) { create(:admin) } + let_it_be(:data_path) { [:ci_variables] } + let_it_be(:variables) do + [ + create(:ci_instance_variable, key: 'd'), + create(:ci_instance_variable, key: 'a'), + create(:ci_instance_variable, key: 'c'), + create(:ci_instance_variable, key: 'e'), + create(:ci_instance_variable, key: 'b') + ] + end + + def pagination_query(params) + graphql_query_for( + :ci_variables, + params, + "#{page_info} nodes { id }" + ) + end + + it_behaves_like 'sorted paginated variables' + end end diff --git a/spec/requests/api/graphql/ci/jobs_spec.rb b/spec/requests/api/graphql/ci/jobs_spec.rb index 131cdb77107..674407c0a0e 100644 --- a/spec/requests/api/graphql/ci/jobs_spec.rb +++ b/spec/requests/api/graphql/ci/jobs_spec.rb @@ -96,7 +96,7 @@ RSpec.describe 'Query.project.pipeline', feature_category: :continuous_integrati create(:ci_build_need, build: test_job, name: 'my test job') end - it 'reports the build needs and execution requirements', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/347290' do + it 'reports the build needs and execution requirements' do post_graphql(query, current_user: user) expect(jobs_graphql_data).to contain_exactly( diff --git a/spec/requests/api/graphql/ci/project_variables_spec.rb b/spec/requests/api/graphql/ci/project_variables_spec.rb index 0338b58a0ea..0ddcac89b34 100644 --- a/spec/requests/api/graphql/ci/project_variables_spec.rb +++ b/spec/requests/api/graphql/ci/project_variables_spec.rb @@ -41,7 +41,7 @@ RSpec.describe 'Query.project(fullPath).ciVariables', feature_category: :pipelin post_graphql(query, current_user: user) - expect(graphql_data.dig('project', 'ciVariables', 'limit')).to be(200) + expect(graphql_data.dig('project', 'ciVariables', 'limit')).to be(8000) expect(graphql_data.dig('project', 'ciVariables', 'nodes')).to contain_exactly({ 'id' => variable.to_global_id.to_s, 'key' => 'TEST_VAR', @@ -66,4 +66,32 @@ RSpec.describe 'Query.project(fullPath).ciVariables', feature_category: :pipelin expect(graphql_data.dig('project', 'ciVariables')).to be_nil end end + + describe 'sorting and pagination' do + let_it_be(:current_user) { user } + let_it_be(:data_path) { [:project, :ci_variables] } + let_it_be(:variables) do + [ + create(:ci_variable, project: project, key: 'd'), + create(:ci_variable, project: project, key: 'a'), + create(:ci_variable, project: project, key: 'c'), + create(:ci_variable, project: project, key: 'e'), + create(:ci_variable, project: project, key: 'b') + ] + end + + def pagination_query(params) + graphql_query_for( + :project, + { fullPath: project.full_path }, + query_graphql_field('ciVariables', params, "#{page_info} nodes { id }") + ) + end + + before do + project.add_maintainer(current_user) + end + + it_behaves_like 'sorted paginated variables' + end end diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index ca08e780758..986e3ce9e52 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -92,6 +92,7 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do run_untagged: runner.run_untagged, ip_address: runner.ip_address, runner_type: runner.instance_type? ? 'INSTANCE_TYPE' : 'PROJECT_TYPE', + ephemeral_authentication_token: nil, executor_name: runner.executor_type&.dasherize, architecture_name: runner.architecture, platform_name: runner.platform, @@ -518,6 +519,110 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do end end + describe 'ephemeralAuthenticationToken', :freeze_time do + subject(:request) { post_graphql(query, current_user: user) } + + let_it_be(:creator) { create(:user) } + + let(:created_at) { Time.current } + let(:token_prefix) { registration_type == :authenticated_user ? 'glrt-' : '' } + let(:registration_type) {} + let(:query) do + %( + query { + runner(id: "#{runner.to_global_id}") { + id + ephemeralAuthenticationToken + } + } + ) + end + + let(:runner) do + create(:ci_runner, :group, + groups: [group], creator: creator, created_at: created_at, + registration_type: registration_type, token: "#{token_prefix}abc123") + end + + before_all do + group.add_owner(creator) # Allow creating runners in the group + end + + shared_examples 'an ephemeral_authentication_token' do + it 'returns token in ephemeral_authentication_token field' do + request + + runner_data = graphql_data_at(:runner) + expect(runner_data).not_to be_nil + expect(runner_data).to match a_graphql_entity_for(runner, ephemeral_authentication_token: runner.token) + end + end + + shared_examples 'a protected ephemeral_authentication_token' do + it 'returns nil ephemeral_authentication_token' do + request + + runner_data = graphql_data_at(:runner) + expect(runner_data).not_to be_nil + expect(runner_data).to match a_graphql_entity_for(runner, ephemeral_authentication_token: nil) + end + end + + context 'with request made by creator' do + let(:user) { creator } + + context 'with runner created in UI' do + let(:registration_type) { :authenticated_user } + + context 'with runner created in last 3 hours' do + let(:created_at) { (3.hours - 1.second).ago } + + context 'with no runner machine registed yet' do + it_behaves_like 'an ephemeral_authentication_token' + end + + context 'with first runner machine already registed' do + let!(:runner_machine) { create(:ci_runner_machine, runner: runner) } + + it_behaves_like 'a protected ephemeral_authentication_token' + end + end + + context 'with runner created almost too long ago' do + let(:created_at) { (3.hours - 1.second).ago } + + it_behaves_like 'an ephemeral_authentication_token' + end + + context 'with runner created too long ago' do + let(:created_at) { 3.hours.ago } + + it_behaves_like 'a protected ephemeral_authentication_token' + end + end + + context 'with runner registered from command line' do + let(:registration_type) { :registration_token } + + context 'with runner created in last 3 hours' do + let(:created_at) { (3.hours - 1.second).ago } + + it_behaves_like 'a protected ephemeral_authentication_token' + end + end + end + + context 'when request is made by non-creator of the runner' do + let(:user) { create(:admin) } + + context 'with runner created in UI' do + let(:registration_type) { :authenticated_user } + + it_behaves_like 'a protected ephemeral_authentication_token' + end + end + end + describe 'Query limits' do def runner_query(runner) <<~SINGLE @@ -578,7 +683,7 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do QUERY end - it 'does not execute more queries per runner', :aggregate_failures do + it 'does not execute more queries per runner', :aggregate_failures, quarantine: "https://gitlab.com/gitlab-org/gitlab/-/issues/391442" do # warm-up license cache and so on: personal_access_token = create(:personal_access_token, user: user) args = { current_user: user, token: { personal_access_token: personal_access_token } } @@ -647,6 +752,11 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do icon text } + project { + id + name + webUrl + } shortSha commitPath finishedAt diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb index 7937091ea7c..c5286b93251 100644 --- a/spec/requests/api/graphql/gitlab_schema_spec.rb +++ b/spec/requests/api/graphql/gitlab_schema_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'GitlabSchema configurations', feature_category: :not_owned do +RSpec.describe 'GitlabSchema configurations', feature_category: :integrations do include GraphqlHelpers let_it_be(:project) { create(:project) } @@ -223,4 +223,101 @@ RSpec.describe 'GitlabSchema configurations', feature_category: :not_owned do expect(parsed_id).to eq(project.to_global_id) end end + + describe 'removal of deprecated items' do + let(:mock_schema) do + Class.new(GraphQL::Schema) do + lazy_resolve ::Gitlab::Graphql::Lazy, :force + + query(Class.new(::Types::BaseObject) do + graphql_name 'Query' + + field :foo, GraphQL::Types::Boolean, + deprecated: { milestone: '0.1', reason: :renamed } + + field :bar, (Class.new(::Types::BaseEnum) do + graphql_name 'BarEnum' + + value 'FOOBAR', value: 'foobar', deprecated: { milestone: '0.1', reason: :renamed } + end) + + field :baz, GraphQL::Types::Boolean do + argument :arg, String, required: false, deprecated: { milestone: '0.1', reason: :renamed } + end + + def foo + false + end + + def bar + 'foobar' + end + + def baz(arg:) + false + end + end) + end + end + + let(:params) { {} } + let(:headers) { {} } + + before do + allow(GitlabSchema).to receive(:execute).and_wrap_original do |method, *args| + mock_schema.execute(*args) + end + end + + context 'without `remove_deprecated` param' do + it 'shows deprecated items' do + query = '{ foo bar baz(arg: "test") }' + + post_graphql(query, params: params, headers: headers) + + expect(json_response).to include('data' => { 'foo' => false, 'bar' => 'FOOBAR', 'baz' => false }) + end + end + + context 'with `remove_deprecated` param' do + let(:params) { { remove_deprecated: '1' } } + + it 'hides deprecated field' do + query = '{ foo }' + + post_graphql(query, params: params) + + expect(json_response).not_to include('data' => { 'foo' => false }) + expect(json_response).to include( + 'errors' => include(a_hash_including('message' => /Field 'foo' doesn't exist on type 'Query'/)) + ) + end + + it 'hides deprecated enum value' do + query = '{ bar }' + + post_graphql(query, params: params) + + expect(json_response).not_to include('data' => { 'bar' => 'FOOBAR' }) + expect(json_response).to include( + 'errors' => include( + a_hash_including( + 'message' => /`Query.bar` returned `"foobar"` at `bar`, but this isn't a valid value for `BarEnum`/ + ) + ) + ) + end + + it 'hides deprecated argument' do + query = '{ baz(arg: "test") }' + + post_graphql(query, params: params) + + expect(json_response).not_to include('data' => { 'bar' => 'FOOBAR' }) + expect(json_response).to include( + 'errors' => include(a_hash_including('message' => /Field 'baz' doesn't accept argument 'arg'/)) + ) + end + end + end end diff --git a/spec/requests/api/graphql/group/group_releases_spec.rb b/spec/requests/api/graphql/group/group_releases_spec.rb new file mode 100644 index 00000000000..931e7c19c18 --- /dev/null +++ b/spec/requests/api/graphql/group/group_releases_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Query.group(fullPath).releases()', feature_category: :release_orchestration do + include GraphqlHelpers + + include_context 'when releases and group releases shared context' + + let(:resource_type) { :group } + let(:resource) { group } + + describe "ensures that the correct data is returned based on the project's visibility and the user's access level" do + context 'when the group is private' do + let_it_be(:group) { create(:group, :private) } + let_it_be(:project) { create(:project, :repository, :private, group: group) } + let_it_be(:release) { create(:release, :with_evidence, project: project) } + + before_all do + group.add_guest(guest) + group.add_reporter(reporter) + group.add_developer(developer) + end + + context 'when the user is not logged in' do + let(:current_user) { stranger } + + it_behaves_like 'no access to any release data' + end + + context 'when the user has Guest permissions' do + let(:current_user) { guest } + + it_behaves_like 'no access to any repository-related fields' + end + + context 'when the user has Reporter permissions' do + let(:current_user) { reporter } + + it_behaves_like 'full access to all repository-related fields' + it_behaves_like 'no access to editUrl' + end + + context 'when the user has Developer permissions' do + let(:current_user) { developer } + + it_behaves_like 'full access to all repository-related fields' + it_behaves_like 'access to editUrl' + end + end + + context 'when the group is public' do + let_it_be(:group) { create(:group, :public) } + let_it_be(:project) { create(:project, :repository, :public, group: group) } + let_it_be(:release) { create(:release, :with_evidence, project: project) } + + before_all do + group.add_guest(guest) + group.add_reporter(reporter) + group.add_developer(developer) + end + + context 'when the user is not logged in' do + let(:current_user) { stranger } + + it_behaves_like 'no access to any release data' + end + + context 'when the user has Guest permissions' do + let(:current_user) { guest } + + it_behaves_like 'full access to all repository-related fields' + it_behaves_like 'no access to editUrl' + end + + context 'when the user has Reporter permissions' do + let(:current_user) { reporter } + + it_behaves_like 'full access to all repository-related fields' + it_behaves_like 'no access to editUrl' + end + + context 'when the user has Developer permissions' do + let(:current_user) { developer } + + it_behaves_like 'full access to all repository-related fields' + it_behaves_like 'access to editUrl' + end + end + end + + describe 'sorting and pagination' do + let_it_be(:group) { create(:group, :public) } + let_it_be(:project) { create(:project, :public, group: group) } + let(:current_user) { developer } + + let(:data_path) { [:group, :releases] } + + before_all do + group.add_developer(developer) + end + + def pagination_query(params) + graphql_query_for( + :group, + { full_path: group.full_path }, + query_graphql_field(:releases, params, "#{page_info} nodes { tagName }") + ) + end + + def pagination_results_data(nodes) + nodes.pluck('tagName') + end + + context 'when sorting by released_at' do + let_it_be(:release5) { create(:release, project: project, tag: 'v5.5.0', released_at: 3.days.from_now) } + let_it_be(:release1) { create(:release, project: project, tag: 'v5.1.0', released_at: 3.days.ago) } + let_it_be(:release4) { create(:release, project: project, tag: 'v5.4.0', released_at: 2.days.from_now) } + let_it_be(:release2) { create(:release, project: project, tag: 'v5.2.0', released_at: 2.days.ago) } + let_it_be(:release3) { create(:release, project: project, tag: 'v5.3.0', released_at: 1.day.ago) } + + context 'when ascending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :RELEASED_AT_ASC } + let(:first_param) { 2 } + let(:all_records) { [release1.tag, release2.tag, release3.tag, release4.tag, release5.tag] } + end + end + + context 'when descending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :RELEASED_AT_DESC } + let(:first_param) { 2 } + let(:all_records) { [release5.tag, release4.tag, release3.tag, release2.tag, release1.tag] } + end + end + end + end +end diff --git a/spec/requests/api/graphql/groups_query_spec.rb b/spec/requests/api/graphql/groups_query_spec.rb new file mode 100644 index 00000000000..84c8d3c3388 --- /dev/null +++ b/spec/requests/api/graphql/groups_query_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'searching groups', :with_license, feature_category: :subgroups do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:public_group) { create(:group, :public) } + let_it_be(:private_group) { create(:group, :private) } + + let(:fields) do + <<~FIELDS + nodes { + #{all_graphql_fields_for('Group')} + } + FIELDS + end + + let(:query) do + <<~QUERY + query { + groups { + #{fields} + } + } + QUERY + end + + subject { post_graphql(query, current_user: user) } + + describe "Query groups(search)" do + let(:groups) { graphql_data_at(:groups, :nodes) } + let(:names) { groups.map { |group| group["name"] } } # rubocop: disable Rails/Pluck + + it_behaves_like 'a working graphql query' do + before do + subject + end + end + + it 'includes public groups' do + subject + + expect(names).to eq([public_group.name]) + end + + it 'includes accessible private groups ordered by name' do + private_group.add_maintainer(user) + + subject + + expect(names).to eq([public_group.name, private_group.name]) + end + + context 'with `search` argument' do + let_it_be(:other_group) { create(:group, name: 'other-group') } + + let(:query) do + <<~QUERY + query { + groups(search: "oth") { + #{fields} + } + } + QUERY + end + + it 'filters groups by name' do + subject + + expect(names).to contain_exactly(other_group.name) + end + end + end +end diff --git a/spec/requests/api/graphql/issue/issue_spec.rb b/spec/requests/api/graphql/issue/issue_spec.rb index 101de692aa5..3665fbc2df8 100644 --- a/spec/requests/api/graphql/issue/issue_spec.rb +++ b/spec/requests/api/graphql/issue/issue_spec.rb @@ -154,6 +154,47 @@ RSpec.describe 'Query.issue(id)', feature_category: :team_planning do end end + context 'when selecting `related_merge_requests`' do + let(:issue_fields) { ['relatedMergeRequests { nodes { id } }'] } + let_it_be(:user) { create(:user) } + let_it_be(:mr_project) { project } + let!(:merge_request) do + attributes = { + author: user, + source_project: mr_project, + target_project: mr_project, + source_branch: 'master', + target_branch: 'test', + description: "See #{issue.to_reference}" + } + + create(:merge_request, attributes).tap do |merge_request| + create(:note, :system, project: issue.project, noteable: issue, + author: user, note: merge_request.to_reference(full: true)) + end + end + + before do + project.add_developer(current_user) + + post_graphql(query, current_user: current_user) + end + + it 'returns the related merge request' do + expect(issue_data['relatedMergeRequests']['nodes']).to include a_hash_including({ + 'id' => merge_request.to_global_id.to_s + }) + end + + context 'no permission to related merge request' do + let_it_be(:mr_project) { create(:project, :private) } + + it 'does not return the related merge request' do + expect(issue_data['relatedMergeRequests']['nodes']).to be_empty + end + end + end + context 'when there is a confidential issue' do let!(:confidential_issue) do create(:issue, :confidential, project: project) diff --git a/spec/requests/api/graphql/issues_spec.rb b/spec/requests/api/graphql/issues_spec.rb index e67c92d6c33..e437e1bbcb0 100644 --- a/spec/requests/api/graphql/issues_spec.rb +++ b/spec/requests/api/graphql/issues_spec.rb @@ -109,18 +109,6 @@ RSpec.describe 'getting an issue list at root level', feature_category: :team_pl end 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 - context 'when no filters are provided' do let(:all_query_params) { {} } @@ -187,15 +175,21 @@ RSpec.describe 'getting an issue list at root level', feature_category: :team_pl end context 'when fetching issues from multiple projects' do - it 'avoids N+1 queries' do + it 'avoids N+1 queries', :use_sql_query_cache do post_query # warm-up - control = ActiveRecord::QueryRecorder.new { post_query } + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { post_query } + expect_graphql_errors_to_be_empty new_private_project = create(:project, :private).tap { |project| project.add_developer(current_user) } create(:issue, project: new_private_project) - expect { post_query }.not_to exceed_query_limit(control) + private_group = create(:group, :private).tap { |group| group.add_developer(current_user) } + private_project = create(:project, :private, group: private_group) + create(:issue, project: private_project) + + expect { post_query }.not_to exceed_all_query_limit(control) + expect_graphql_errors_to_be_empty end end diff --git a/spec/requests/api/graphql/mutations/achievements/create_spec.rb b/spec/requests/api/graphql/mutations/achievements/create_spec.rb index 1713f050540..3082629d40f 100644 --- a/spec/requests/api/graphql/mutations/achievements/create_spec.rb +++ b/spec/requests/api/graphql/mutations/achievements/create_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Mutations::Achievements::Create, feature_category: :users do +RSpec.describe Mutations::Achievements::Create, feature_category: :user_profile do include GraphqlHelpers include WorkhorseHelpers @@ -13,15 +13,13 @@ RSpec.describe Mutations::Achievements::Create, feature_category: :users do let(:mutation) { graphql_mutation(:achievements_create, params) } let(:name) { 'Name' } let(:description) { 'Description' } - let(:revokeable) { false } let(:avatar) { fixture_file_upload("spec/fixtures/dk.png") } let(:params) do { namespace_id: group.to_global_id, name: name, avatar: avatar, - description: description, - revokeable: revokeable + description: description } end @@ -70,8 +68,7 @@ RSpec.describe Mutations::Achievements::Create, feature_category: :users do expect(graphql_data_at(:achievements_create, :achievement)).to match a_hash_including( 'name' => name, 'namespace' => a_hash_including('id' => group.to_global_id.to_s), - 'description' => description, - 'revokeable' => revokeable + 'description' => description ) end end diff --git a/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb b/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb index 490716ddbe2..55e728b2141 100644 --- a/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb @@ -60,7 +60,7 @@ RSpec.describe 'CiJobTokenScopeAddProject', feature_category: :continuous_integr post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) expect(mutation_response.dig('ciJobTokenScope', 'projects', 'nodes')).not_to be_empty - end.to change { Ci::JobToken::Scope.new(project).allows?(target_project) }.from(false).to(true) + end.to change { Ci::JobToken::ProjectScopeLink.outbound.count }.by(1) end context 'when invalid target project is provided' do diff --git a/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb b/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb index 607c6bd85c2..f1296c054f9 100644 --- a/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb @@ -5,7 +5,13 @@ require 'spec_helper' RSpec.describe 'CiJobTokenScopeRemoveProject', feature_category: :continuous_integration do include GraphqlHelpers - let_it_be(:project) { create(:project, ci_outbound_job_token_scope_enabled: true).tap(&:save!) } + let_it_be(:project) do + create(:project, + ci_outbound_job_token_scope_enabled: true, + ci_inbound_job_token_scope_enabled: true + ) + end + let_it_be(:target_project) { create(:project) } let_it_be(:link) do @@ -16,6 +22,7 @@ RSpec.describe 'CiJobTokenScopeRemoveProject', feature_category: :continuous_int let(:variables) do { + direction: 'OUTBOUND', project_path: project.full_path, target_project_path: target_project.full_path } @@ -61,12 +68,21 @@ RSpec.describe 'CiJobTokenScopeRemoveProject', feature_category: :continuous_int target_project.add_guest(current_user) end - it 'removes the target project from the job token scope' do + it 'removes the target project from the job token outbound scope' do expect do post_graphql_mutation(mutation, current_user: current_user) expect(response).to have_gitlab_http_status(:success) expect(mutation_response.dig('ciJobTokenScope', 'projects', 'nodes')).not_to be_empty - end.to change { Ci::JobToken::Scope.new(project).allows?(target_project) }.from(true).to(false) + end.to change { Ci::JobToken::ProjectScopeLink.outbound.count }.by(-1) + end + + it 'responds successfully' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(graphql_errors).to be_nil + expect(graphql_data_at(:ciJobTokenScopeRemoveProject, :ciJobTokenScope, :projects, :nodes)) + .to contain_exactly({ 'path' => project.path }) end context 'when invalid target project is provided' do diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule_play_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_play_spec.rb index 0e43fa024f3..492c6946c99 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_schedule_play_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_play_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'PipelineSchedulePlay', feature_category: :continuious_integration do +RSpec.describe 'PipelineSchedulePlay', feature_category: :continuous_integration do include GraphqlHelpers let_it_be(:user) { create(:user) } @@ -42,14 +42,18 @@ RSpec.describe 'PipelineSchedulePlay', feature_category: :continuious_integratio end end - context 'when authorized' do + context 'when authorized', :sidekiq_inline do before do project.add_maintainer(user) pipeline_schedule.update_columns(next_run_at: 2.hours.ago) end context 'when mutation succeeds' do + let(:service_response) { instance_double('ServiceResponse', payload: new_pipeline) } + let(:new_pipeline) { instance_double('Ci::Pipeline', persisted?: true) } + it do + expect(Ci::CreatePipelineService).to receive_message_chain(:new, :execute).and_return(service_response) post_graphql_mutation(mutation, current_user: user) expect(mutation_response['pipelineSchedule']['id']).to include(pipeline_schedule.id.to_s) @@ -61,14 +65,10 @@ RSpec.describe 'PipelineSchedulePlay', feature_category: :continuious_integratio end context 'when mutation fails' do - before do - allow(RunPipelineScheduleWorker).to receive(:perform_async).and_return(nil) - end - it do expect(RunPipelineScheduleWorker) .to receive(:perform_async) - .with(pipeline_schedule.id, user.id) + .with(pipeline_schedule.id, user.id).and_return(nil) post_graphql_mutation(mutation, current_user: user) diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule_update_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_update_spec.rb new file mode 100644 index 00000000000..c1da231a4a6 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_update_spec.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineScheduleUpdate', feature_category: :continuous_integration do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + + let(:mutation) do + variables = { + id: pipeline_schedule.to_global_id.to_s, + **pipeline_schedule_parameters + } + + graphql_mutation( + :pipeline_schedule_update, + variables, + <<-QL + pipelineSchedule { + id + description + cron + refForDisplay + active + cronTimezone + variables { + nodes { + key + value + } + } + } + errors + QL + ) + end + + let(:pipeline_schedule_parameters) { {} } + let(:mutation_response) { graphql_mutation_response(:pipeline_schedule_update) } + + context 'when unauthorized' do + it 'returns an error' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + expect(graphql_errors[0]['message']) + .to eq( + "The resource that you are attempting to access does not exist " \ + "or you don't have permission to perform this action" + ) + end + end + + context 'when authorized' do + before do + project.add_developer(user) + end + + context 'when success' do + let(:pipeline_schedule_parameters) do + { + description: 'updated_desc', + cron: '0 1 * * *', + cronTimezone: 'UTC', + ref: 'patch-x', + active: true, + variables: [ + { key: 'AAA', value: "AAA123", variableType: 'ENV_VAR' } + ] + } + end + + it do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + + expect_graphql_errors_to_be_empty + + expect(mutation_response['pipelineSchedule']['id']).to eq(pipeline_schedule.to_global_id.to_s) + + %w[description cron cronTimezone active].each do |key| + expect(mutation_response['pipelineSchedule'][key]).to eq(pipeline_schedule_parameters[key.to_sym]) + end + + expect(mutation_response['pipelineSchedule']['refForDisplay']).to eq(pipeline_schedule_parameters[:ref]) + + expect(mutation_response['pipelineSchedule']['variables']['nodes'][0]['key']).to eq('AAA') + expect(mutation_response['pipelineSchedule']['variables']['nodes'][0]['value']).to eq('AAA123') + end + end + + context 'when failure' do + context 'when params are invalid' do + let(:pipeline_schedule_parameters) do + { + description: '', + cron: 'abc', + cronTimezone: 'cCc', + ref: '', + active: true, + variables: [] + } + end + + it do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + + expect(mutation_response['errors']) + .to match_array( + [ + "Cron is invalid syntax", + "Cron timezone is invalid syntax", + "Ref can't be blank", + "Description can't be blank" + ] + ) + end + end + + context 'when params have duplicate variables' do + let(:pipeline_schedule_parameters) do + { + variables: [ + { key: 'AAA', value: "AAA123", variableType: 'ENV_VAR' }, + { key: 'AAA', value: "AAA123", variableType: 'ENV_VAR' } + ] + } + end + + it 'returns error' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + + expect(mutation_response['errors']) + .to match_array( + [ + "Variables have duplicate values (AAA)" + ] + ) + end + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb b/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb index 7a6ee7c2ecc..99e55c44773 100644 --- a/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb @@ -18,7 +18,8 @@ RSpec.describe 'ProjectCiCdSettingsUpdate', feature_category: :continuous_integr full_path: project.full_path, keep_latest_artifact: false, job_token_scope_enabled: false, - inbound_job_token_scope_enabled: false + inbound_job_token_scope_enabled: false, + opt_in_jwt: true } end @@ -117,6 +118,15 @@ RSpec.describe 'ProjectCiCdSettingsUpdate', feature_category: :continuous_integr end end + it 'updates ci_opt_in_jwt' do + post_graphql_mutation(mutation, current_user: user) + + project.reload + + expect(response).to have_gitlab_http_status(:success) + expect(project.ci_opt_in_jwt).to eq(true) + end + context 'when bad arguments are provided' do let(:variables) { { full_path: '', keep_latest_artifact: false } } diff --git a/spec/requests/api/graphql/mutations/issues/bulk_update_spec.rb b/spec/requests/api/graphql/mutations/issues/bulk_update_spec.rb new file mode 100644 index 00000000000..b9c83311908 --- /dev/null +++ b/spec/requests/api/graphql/mutations/issues/bulk_update_spec.rb @@ -0,0 +1,177 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Bulk update issues', feature_category: :team_planning do + include GraphqlHelpers + + let_it_be(:developer) { create(:user) } + let_it_be(:group) { create(:group).tap { |group| group.add_developer(developer) } } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:updatable_issues, reload: true) { create_list(:issue, 2, project: project) } + let_it_be(:milestone) { create(:milestone, group: group) } + + let(:parent) { project } + let(:max_issues) { Mutations::Issues::BulkUpdate::MAX_ISSUES } + let(:mutation) { graphql_mutation(:issues_bulk_update, base_arguments.merge(additional_arguments)) } + let(:mutation_response) { graphql_mutation_response(:issues_bulk_update) } + let(:current_user) { developer } + let(:base_arguments) { { parent_id: parent.to_gid.to_s, ids: updatable_issues.map { |i| i.to_gid.to_s } } } + + let(:additional_arguments) do + { + assignee_ids: [current_user.to_gid.to_s], + milestone_id: milestone.to_gid.to_s + } + end + + context 'when the `bulk_update_issues_mutation` feature flag is disabled' do + before do + stub_feature_flags(bulk_update_issues_mutation: false) + end + + it 'returns a resource not available error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to contain_exactly( + hash_including( + 'message' => '`bulk_update_issues_mutation` feature flag is disabled.' + ) + ) + end + end + + context 'when user can not update all issues' do + let_it_be(:forbidden_issue) { create(:issue) } + + it 'updates only issues that the user can update' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + + updatable_issues.each(&:reset) + forbidden_issue.reset + end.to change { updatable_issues.flat_map(&:assignee_ids) }.from([]).to([current_user.id] * 2).and( + not_change(forbidden_issue, :assignee_ids).from([]) + ) + + expect(mutation_response).to include( + 'updatedIssueCount' => updatable_issues.count + ) + end + end + + context 'when user can update all issues' do + it 'updates all issues' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + updatable_issues.each(&:reload) + end.to change { updatable_issues.flat_map(&:assignee_ids) }.from([]).to([current_user.id] * 2) + .and(change { updatable_issues.map(&:milestone_id) }.from([nil] * 2).to([milestone.id] * 2)) + + expect(mutation_response).to include( + 'updatedIssueCount' => updatable_issues.count + ) + end + + context 'when current user cannot read the specified project' do + let_it_be(:parent) { create(:project, :private) } + + it 'returns a resource not found error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to contain_exactly( + hash_including( + 'message' => "The resource that you are attempting to access does not exist or you don't have " \ + 'permission to perform this action' + ) + ) + end + end + + context 'when scoping to a parent group' do + let(:parent) { group } + + it 'updates all issues' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + updatable_issues.each(&:reload) + end.to change { updatable_issues.flat_map(&:assignee_ids) }.from([]).to([current_user.id] * 2) + .and(change { updatable_issues.map(&:milestone_id) }.from([nil] * 2).to([milestone.id] * 2)) + + expect(mutation_response).to include( + 'updatedIssueCount' => updatable_issues.count + ) + end + + context 'when current user cannot read the specified group' do + let(:parent) { create(:group, :private) } + + it 'returns a resource not found error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to contain_exactly( + hash_including( + 'message' => "The resource that you are attempting to access does not exist or you don't have " \ + 'permission to perform this action' + ) + ) + end + end + end + + context 'when setting arguments to null or none' do + let(:additional_arguments) { { assignee_ids: [], milestone_id: nil } } + + before do + updatable_issues.each do |issue| + issue.update!(assignees: [current_user], milestone: milestone) + end + end + + it 'updates all issues' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + updatable_issues.each(&:reload) + end.to change { updatable_issues.flat_map(&:assignee_ids) }.from([current_user.id] * 2).to([]) + .and(change { updatable_issues.map(&:milestone_id) }.from([milestone.id] * 2).to([nil] * 2)) + + expect(mutation_response).to include( + 'updatedIssueCount' => updatable_issues.count + ) + end + end + end + + context 'when update service returns an error' do + before do + allow_next_instance_of(Issuable::BulkUpdateService) do |update_service| + allow(update_service).to receive(:execute).and_return( + ServiceResponse.error(message: 'update error', http_status: 422) # rubocop:disable Gitlab/ServiceResponse + ) + end + end + + it 'returns an error message' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_data.dig('issuesBulkUpdate', 'errors')).to contain_exactly('update error') + end + end + + context 'when trying to update more than the max allowed' do + before do + stub_const('Mutations::Issues::BulkUpdate::MAX_ISSUES', updatable_issues.count - 1) + end + + it "restricts updating more than #{Mutations::Issues::BulkUpdate::MAX_ISSUES} issues at the same time" do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to contain_exactly( + hash_including( + 'message' => + format(_('No more than %{max_issues} issues can be updated at the same time'), max_issues: max_issues) + ) + ) + end + end +end diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_milestone_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_milestone_spec.rb index 3907ebad9ce..1898ee5a62d 100644 --- a/spec/requests/api/graphql/mutations/merge_requests/set_milestone_spec.rb +++ b/spec/requests/api/graphql/mutations/merge_requests/set_milestone_spec.rb @@ -63,4 +63,20 @@ RSpec.describe 'Setting milestone of a merge request', feature_category: :code_r expect(mutation_response['mergeRequest']['milestone']).to be_nil end end + + context 'when passing an invalid milestone_id' do + let(:input) { { milestone_id: GitlabSchema.id_from_object(create(:milestone)).to_s } } + + it 'does not set the milestone' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to include( + a_hash_including( + 'message' => "The resource that you are attempting to access does not exist " \ + "or you don't have permission to perform this action" + ) + ) + end + end end diff --git a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb index 00e25909746..a6253ba424b 100644 --- a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb @@ -122,8 +122,8 @@ RSpec.describe 'Adding a Note', feature_category: :team_planning do let(:variables_extra) { {} } before do - stub_const('WorkItems::Type::BASE_TYPES', { issue: { name: 'NoNotesWidget', enum_value: 0 } }) - stub_const('WorkItems::Type::WIDGETS_FOR_TYPE', { issue: [::WorkItems::Widgets::Description] }) + WorkItems::Type.default_by_type(:issue).widget_definitions.find_by_widget_type(:notes) + .update!(disabled: true) end it_behaves_like 'a Note mutation that does not create a Note' diff --git a/spec/requests/api/graphql/mutations/notes/destroy_spec.rb b/spec/requests/api/graphql/mutations/notes/destroy_spec.rb index eb45e2aa033..f40518a574b 100644 --- a/spec/requests/api/graphql/mutations/notes/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/destroy_spec.rb @@ -57,8 +57,7 @@ RSpec.describe 'Destroying a Note', feature_category: :team_planning do context 'without notes widget' do before do - stub_const('WorkItems::Type::BASE_TYPES', { issue: { name: 'NoNotesWidget', enum_value: 0 } }) - stub_const('WorkItems::Type::WIDGETS_FOR_TYPE', { issue: [::WorkItems::Widgets::Description] }) + WorkItems::Type.default_by_type(:issue).widget_definitions.find_by_widget_type(:notes).update!(disabled: true) end it 'does not update the Note' do diff --git a/spec/requests/api/graphql/mutations/notes/update/note_spec.rb b/spec/requests/api/graphql/mutations/notes/update/note_spec.rb index dff8a87314b..7918bc860fe 100644 --- a/spec/requests/api/graphql/mutations/notes/update/note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/update/note_spec.rb @@ -50,8 +50,7 @@ RSpec.describe 'Updating a Note', feature_category: :team_planning do context 'without notes widget' do before do - stub_const('WorkItems::Type::BASE_TYPES', { issue: { name: 'NoNotesWidget', enum_value: 0 } }) - stub_const('WorkItems::Type::WIDGETS_FOR_TYPE', { issue: [::WorkItems::Widgets::Description] }) + WorkItems::Type.default_by_type(:issue).widget_definitions.find_by_widget_type(:notes).update!(disabled: true) end it 'does not update the Note' do diff --git a/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb index 31d17401b9e..967ad75c906 100644 --- a/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb +++ b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Mutations::UserPreferences::Update, feature_category: :users do +RSpec.describe Mutations::UserPreferences::Update, feature_category: :user_profile do include GraphqlHelpers let_it_be(:current_user) { create(:user) } 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 b33a394d023..ddd294e8f82 100644 --- a/spec/requests/api/graphql/mutations/work_items/update_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/update_spec.rb @@ -127,7 +127,9 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do let(:fields) do <<~FIELDS workItem { + title description + state widgets { type ... on WorkItemWidgetDescription { @@ -179,6 +181,9 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do nodes { id } } } + ... on WorkItemWidgetDescription { + description + } } } errors @@ -201,6 +206,12 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do let(:expected_labels) { [] } it_behaves_like 'mutation updating work item labels' + + context 'with quick action' do + let(:input) { { 'descriptionWidget' => { 'description' => "/remove_label ~\"#{existing_label.name}\"" } } } + + it_behaves_like 'mutation updating work item labels' + end end context 'when only adding labels' do @@ -208,6 +219,14 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do let(:expected_labels) { [label1, label2, existing_label] } it_behaves_like 'mutation updating work item labels' + + context 'with quick action' do + let(:input) do + { 'descriptionWidget' => { 'description' => "/labels ~\"#{label1.name}\" ~\"#{label2.name}\"" } } + end + + it_behaves_like 'mutation updating work item labels' + end end context 'when adding and removing labels' do @@ -216,10 +235,47 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do let(:expected_labels) { [label1, label2] } it_behaves_like 'mutation updating work item labels' + + context 'with quick action' do + let(:input) do + { 'descriptionWidget' => { 'description' => + "/label ~\"#{label1.name}\" ~\"#{label2.name}\"\n/remove_label ~\"#{existing_label.name}\"" } } + end + + it_behaves_like 'mutation updating work item labels' + end + end + + context 'when the work item type does not support labels widget' do + let_it_be(:work_item) { create(:work_item, :task, project: project) } + + let(:input) { { 'descriptionWidget' => { 'description' => "Updating labels.\n/labels ~\"#{label1.name}\"" } } } + + before do + WorkItems::Type.default_by_type(:task).widget_definitions + .find_by_widget_type(:labels).update!(disabled: true) + end + + it 'ignores the quick action' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.not_to change(work_item.labels, :count) + + expect(work_item.labels).to be_empty + expect(mutation_response['workItem']['widgets']).to include( + 'description' => "Updating labels.", + 'type' => 'DESCRIPTION' + ) + expect(mutation_response['workItem']['widgets']).not_to include( + 'labels', + 'type' => 'LABELS' + ) + end end end - context 'with due and start date widget input' do + context 'with due and start date widget input', :freeze_time do let(:start_date) { Date.today } let(:due_date) { 1.week.from_now.to_date } let(:fields) do @@ -231,6 +287,9 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do startDate dueDate } + ... on WorkItemWidgetDescription { + description + } } } errors @@ -259,6 +318,81 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do ) end + context 'when using quick action' do + let(:due_date) { Date.today } + + context 'when removing due date' do + let(:input) { { 'descriptionWidget' => { 'description' => "/remove_due_date" } } } + + before do + work_item.update!(due_date: due_date) + end + + it 'updates start and due date' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to not_change(work_item, :start_date).and( + change(work_item, :due_date).from(due_date).to(nil) + ) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']['widgets']).to include({ + 'startDate' => nil, + 'dueDate' => nil, + 'type' => 'START_AND_DUE_DATE' + }) + end + end + + context 'when setting due date' do + let(:input) { { 'descriptionWidget' => { 'description' => "/due today" } } } + + it 'updates due date' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to not_change(work_item, :start_date).and( + change(work_item, :due_date).from(nil).to(due_date) + ) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']['widgets']).to include({ + 'startDate' => nil, + 'dueDate' => Date.today.to_s, + 'type' => 'START_AND_DUE_DATE' + }) + end + end + + context 'when the work item type does not support start and due date widget' do + let_it_be(:work_item) { create(:work_item, :task, project: project) } + + let(:input) { { 'descriptionWidget' => { 'description' => "Updating due date.\n/due today" } } } + + before do + WorkItems::Type.default_by_type(:task).widget_definitions + .find_by_widget_type(:start_and_due_date).update!(disabled: true) + end + + it 'ignores the quick action' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.not_to change(work_item, :due_date) + + expect(mutation_response['workItem']['widgets']).to include( + 'description' => "Updating due date.", + 'type' => 'DESCRIPTION' + ) + expect(mutation_response['workItem']['widgets']).not_to include({ + 'dueDate' => nil, + 'type' => 'START_AND_DUE_DATE' + }) + end + end + end + context 'when provided input is invalid' do let(:due_date) { 1.week.ago.to_date } @@ -516,6 +650,9 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do } } } + ... on WorkItemWidgetDescription { + description + } } } errors @@ -544,6 +681,81 @@ RSpec.describe 'Update a work item', feature_category: :team_planning do } ) end + + context 'when using quick action' do + context 'when assigning a user' do + let(:input) { { 'descriptionWidget' => { 'description' => "/assign @#{developer.username}" } } } + + it 'updates the work item assignee' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :assignee_ids).from([]).to([developer.id]) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']['widgets']).to include( + { + 'type' => 'ASSIGNEES', + 'assignees' => { + 'nodes' => [ + { 'id' => developer.to_global_id.to_s, 'username' => developer.username } + ] + } + } + ) + end + end + + context 'when unassigning a user' do + let(:input) { { 'descriptionWidget' => { 'description' => "/unassign @#{developer.username}" } } } + + before do + work_item.update!(assignee_ids: [developer.id]) + end + + it 'updates the work item assignee' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change(work_item, :assignee_ids).from([developer.id]).to([]) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['workItem']['widgets']).to include( + 'type' => 'ASSIGNEES', + 'assignees' => { + 'nodes' => [] + } + ) + end + end + end + + context 'when the work item type does not support the assignees widget' do + let_it_be(:work_item) { create(:work_item, :task, project: project) } + + let(:input) do + { 'descriptionWidget' => { 'description' => "Updating assignee.\n/assign @#{developer.username}" } } + end + + before do + WorkItems::Type.default_by_type(:task).widget_definitions + .find_by_widget_type(:assignees).update!(disabled: true) + end + + it 'ignores the quick action' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.not_to change(work_item, :assignee_ids) + + expect(mutation_response['workItem']['widgets']).to include({ + 'description' => "Updating assignee.", + 'type' => 'DESCRIPTION' + } + ) + expect(mutation_response['workItem']['widgets']).not_to include({ 'type' => 'ASSIGNEES' }) + end + end end context 'when updating milestone' do diff --git a/spec/requests/api/graphql/notes/note_spec.rb b/spec/requests/api/graphql/notes/note_spec.rb new file mode 100644 index 00000000000..daceaec0b94 --- /dev/null +++ b/spec/requests/api/graphql/notes/note_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Query.note(id)', feature_category: :team_planning do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:note) { create(:note, noteable: issue, project: project) } + let_it_be(:system_note) { create(:note, :system, noteable: issue, project: project) } + + let(:note_params) { { 'id' => global_id_of(note) } } + let(:note_data) { graphql_data['note'] } + let(:note_fields) { all_graphql_fields_for('Note'.classify) } + + let(:query) do + graphql_query_for('note', note_params, note_fields) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + context 'when the user does not have access to read the note' do + it 'returns nil' do + post_graphql(query, current_user: current_user) + + expect(note_data).to be nil + end + + context 'when it is a system note' do + let(:note_params) { { 'id' => global_id_of(system_note) } } + + it 'returns nil' do + post_graphql(query, current_user: current_user) + + expect(note_data).to be nil + end + end + end + + context 'when the user has access to read the note' do + before do + project.add_guest(current_user) + end + + it 'returns note' do + post_graphql(query, current_user: current_user) + + expect(note_data['id']).to eq(global_id_of(note).to_s) + end + + context 'when it is a system note' do + let(:note_params) { { 'id' => global_id_of(system_note) } } + + it 'returns note' do + post_graphql(query, current_user: current_user) + + expect(note_data['id']).to eq(global_id_of(system_note).to_s) + end + end + + context 'and notes widget is not available' do + before do + WorkItems::Type.default_by_type(:issue).widget_definitions + .find_by_widget_type(:notes).update!(disabled: true) + end + + it 'returns nil' do + post_graphql(query, current_user: current_user) + + expect(note_data).to be nil + end + end + + context 'when note is internal' do + let_it_be(:note) { create(:note, :confidential, noteable: issue, project: project) } + + it 'returns nil' do + post_graphql(query, current_user: current_user) + + expect(note_data).to be nil + end + + context 'and user can read confidential notes' do + let_it_be(:developer) { create(:user) } + + before do + project.add_developer(developer) + end + + it 'returns note' do + post_graphql(query, current_user: developer) + + expect(note_data['id']).to eq(global_id_of(note).to_s) + end + end + end + end +end diff --git a/spec/requests/api/graphql/notes/synthetic_note_resolver_spec.rb b/spec/requests/api/graphql/notes/synthetic_note_resolver_spec.rb new file mode 100644 index 00000000000..1199aeb4c39 --- /dev/null +++ b/spec/requests/api/graphql/notes/synthetic_note_resolver_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Query.synthetic_note(noteable_id, sha)', feature_category: :team_planning do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:label) { create(:label, project: project) } + let_it_be(:label_event, refind: true) do + create(:resource_label_event, user: current_user, issue: issue, label: label, action: 'add', created_at: 2.days.ago) + end + + let(:label_note) { LabelNote.from_events([label_event]) } + let(:global_id) { ::Gitlab::GlobalId.build(label_note, model_name: LabelNote.to_s, id: label_note.discussion_id) } + let(:note_params) { { sha: label_note.discussion_id, noteable_id: global_id_of(issue) } } + let(:note_data) { graphql_data['syntheticNote'] } + let(:note_fields) { all_graphql_fields_for('Note'.classify) } + + let(:query) do + graphql_query_for('synthetic_note', note_params, note_fields) + end + + context 'when the user does not have access to read the note' do + it 'returns nil' do + post_graphql(query, current_user: current_user) + + expect(note_data).to be nil + end + end + + context 'when the user has access to read the note' do + before do + project.add_guest(current_user) + end + + it 'returns synthetic note' do + post_graphql(query, current_user: current_user) + + expect(note_data['id']).to eq(global_id.to_s) + end + + context 'and notes widget is not available' do + before do + WorkItems::Type.default_by_type(:issue).widget_definitions + .find_by_widget_type(:notes).update!(disabled: true) + end + + it 'returns nil' do + post_graphql(query, current_user: current_user) + + expect(note_data).to be nil + end + end + end +end diff --git a/spec/requests/api/graphql/packages/package_spec.rb b/spec/requests/api/graphql/packages/package_spec.rb index 42927634119..82fcc5254ad 100644 --- a/spec/requests/api/graphql/packages/package_spec.rb +++ b/spec/requests/api/graphql/packages/package_spec.rb @@ -41,6 +41,7 @@ RSpec.describe 'package details', feature_category: :package_registry do context 'with unauthorized user' do before do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + project.add_guest(user) end it 'returns no packages' do @@ -48,6 +49,47 @@ RSpec.describe 'package details', feature_category: :package_registry do expect(graphql_data_at(:package)).to be_nil end + + context 'with access to package registry for everyone' do + before do + project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + subject + end + + it_behaves_like 'a working graphql query' do + it 'matches the JSON schema' do + expect(package_details).to match_schema('graphql/packages/package_details') + end + end + + it '`public_package` returns true' do + expect(graphql_data_at(:package, :public_package)).to eq(true) + end + end + end + + context 'when project is public' do + let_it_be(:public_project) { create(:project, :public, group: group) } + let_it_be(:composer_package) { create(:composer_package, project: public_project) } + let(:package_global_id) { global_id_of(composer_package) } + + before do + subject + end + + it_behaves_like 'a working graphql query' do + before do + subject + end + + it 'matches the JSON schema' do + expect(package_details).to match_schema('graphql/packages/package_details') + end + end + + it '`public_package` returns true' do + expect(graphql_data_at(:package, :public_package)).to eq(true) + end end context 'with authorized user' do @@ -113,6 +155,29 @@ RSpec.describe 'package details', feature_category: :package_registry do end end + context 'versions field', :aggregate_failures do + let_it_be(:composer_package2) { create(:composer_package, project: project, name: composer_package.name) } + let_it_be(:composer_package3) { create(:composer_package, :error, project: project, name: composer_package.name) } + let_it_be(:pending_destruction) { create(:composer_package, :pending_destruction, project: project, name: composer_package.name) } + + def run_query + versions_nodes = <<~QUERY + nodes { id } + QUERY + + query = graphql_query_for(:package, { id: package_global_id }, query_graphql_field("versions", {}, versions_nodes)) + post_graphql(query, current_user: user) + end + + it 'returns other versions' do + run_query + versions_ids = graphql_data.dig('package', 'versions', 'nodes').pluck('id') + expected_ids = [composer_package2, composer_package3].map(&:to_gid).map(&:to_s) + + expect(versions_ids).to contain_exactly(*expected_ids) + end + end + context 'pipelines field', :aggregate_failures do let(:pipelines) { create_list(:ci_pipeline, 6, project: project) } let(:pipeline_gids) { pipelines.sort_by(&:id).map(&:to_gid).map(&:to_s).reverse } @@ -227,6 +292,49 @@ RSpec.describe 'package details', feature_category: :package_registry do end end + context 'public_package' do + context 'when project is private' do + let_it_be(:private_project) { create(:project, :private, group: group) } + let_it_be(:composer_package) { create(:composer_package, project: private_project) } + let(:package_global_id) { global_id_of(composer_package) } + + before do + private_project.add_developer(user) + end + + it 'returns false' do + subject + + expect(graphql_data_at(:package, :public_package)).to eq(false) + end + + context 'with access to package registry for everyone' do + before do + private_project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + subject + end + + it 'returns true' do + expect(graphql_data_at(:package, :public_package)).to eq(true) + end + end + end + + context 'when project is public' do + let_it_be(:public_project) { create(:project, :public, group: group) } + let_it_be(:composer_package) { create(:composer_package, project: public_project) } + let(:package_global_id) { global_id_of(composer_package) } + + before do + subject + end + + it 'returns true' do + expect(graphql_data_at(:package, :public_package)).to eq(true) + end + end + end + context 'with package that has no default status' do before do composer_package.update!(status: :error) diff --git a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb index 304edfbf4e4..55d223daf27 100644 --- a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb @@ -16,7 +16,7 @@ RSpec.describe 'getting Alert Management Alerts', feature_category: :incident_ma let(:fields) do <<~QUERY nodes { - #{all_graphql_fields_for('AlertManagementAlert', excluded: ['assignees'])} + #{all_graphql_fields_for('AlertManagementAlert', excluded: %w[assignees relatedMergeRequests])} } QUERY end diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index 6aa96cfc070..76e5d687fd1 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -193,7 +193,8 @@ RSpec.describe 'getting merge request information nested in a project', feature_ 'cherryPickOnCurrentMergeRequest' => false, 'revertOnCurrentMergeRequest' => false, 'updateMergeRequest' => false, - 'canMerge' => false + 'canMerge' => false, + 'canApprove' => false } post_graphql(query, current_user: current_user) diff --git a/spec/requests/api/graphql/project/project_statistics_spec.rb b/spec/requests/api/graphql/project/project_statistics_spec.rb index d078659b954..444738cbc81 100644 --- a/spec/requests/api/graphql/project/project_statistics_spec.rb +++ b/spec/requests/api/graphql/project/project_statistics_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'rendering project statistics', feature_category: :project_statistics do +RSpec.describe 'rendering project statistics', feature_category: :shared do include GraphqlHelpers let(:project) { create(:project) } diff --git a/spec/requests/api/graphql/project/releases_spec.rb b/spec/requests/api/graphql/project/releases_spec.rb index aa454349fcf..bc47f5a0248 100644 --- a/spec/requests/api/graphql/project/releases_spec.rb +++ b/spec/requests/api/graphql/project/releases_spec.rb @@ -5,226 +5,10 @@ require 'spec_helper' RSpec.describe 'Query.project(fullPath).releases()', feature_category: :release_orchestration do include GraphqlHelpers - let_it_be(:stranger) { create(:user) } - let_it_be(:guest) { create(:user) } - let_it_be(:reporter) { create(:user) } - let_it_be(:developer) { create(:user) } - - let(:base_url_params) { { scope: 'all', release_tag: release.tag } } - let(:opened_url_params) { { state: 'opened', **base_url_params } } - let(:merged_url_params) { { state: 'merged', **base_url_params } } - let(:closed_url_params) { { state: 'closed', **base_url_params } } - - let(:query) do - graphql_query_for(:project, { fullPath: project.full_path }, - %{ - releases { - count - nodes { - tagName - tagPath - name - commit { - sha - } - assets { - count - sources { - nodes { - url - } - } - } - evidences { - nodes { - sha - } - } - links { - selfUrl - openedMergeRequestsUrl - mergedMergeRequestsUrl - closedMergeRequestsUrl - openedIssuesUrl - closedIssuesUrl - } - } - } - }) - end - - let(:params_for_issues_and_mrs) { { scope: 'all', state: 'opened', release_tag: release.tag } } - let(:post_query) { post_graphql(query, current_user: current_user) } - - let(:data) { graphql_data.dig('project', 'releases', 'nodes', 0) } - - before do - stub_default_url_options(host: 'www.example.com') - end - - shared_examples 'correct total count' do - let(:data) { graphql_data.dig('project', 'releases') } - - before do - create_list(:release, 2, project: project) - - post_query - end - - it 'returns the total count' do - expect(data['count']).to eq(project.releases.count) - end - end - - shared_examples 'full access to all repository-related fields' do - describe 'repository-related fields' do - before do - post_query - end - - it 'returns data for fields that are protected in private projects' do - expected_sources = release.sources.map do |s| - { 'url' => s.url } - end - - expected_evidences = release.evidences.map do |e| - { 'sha' => e.sha } - end - - expect(data).to eq( - 'tagName' => release.tag, - 'tagPath' => project_tag_path(project, release.tag), - 'name' => release.name, - 'commit' => { - 'sha' => release.commit.sha - }, - 'assets' => { - 'count' => release.assets_count, - 'sources' => { - 'nodes' => expected_sources - } - }, - 'evidences' => { - 'nodes' => expected_evidences - }, - 'links' => { - 'selfUrl' => project_release_url(project, release), - 'openedMergeRequestsUrl' => project_merge_requests_url(project, opened_url_params), - 'mergedMergeRequestsUrl' => project_merge_requests_url(project, merged_url_params), - 'closedMergeRequestsUrl' => project_merge_requests_url(project, closed_url_params), - 'openedIssuesUrl' => project_issues_url(project, opened_url_params), - 'closedIssuesUrl' => project_issues_url(project, closed_url_params) - } - ) - end - end - - it_behaves_like 'correct total count' - end - - shared_examples 'no access to any repository-related fields' do - describe 'repository-related fields' do - before do - post_query - end + include_context 'when releases and group releases shared context' - it 'does not return data for fields that expose repository information' do - tag_name = release.tag - release_name = release.name - expect(data).to eq( - 'tagName' => tag_name, - 'tagPath' => nil, - 'name' => release_name, - 'commit' => nil, - 'assets' => { - 'count' => release.assets_count(except: [:sources]), - 'sources' => { - 'nodes' => [] - } - }, - 'evidences' => { - 'nodes' => [] - }, - 'links' => { - 'closedIssuesUrl' => nil, - 'closedMergeRequestsUrl' => nil, - 'mergedMergeRequestsUrl' => nil, - 'openedIssuesUrl' => nil, - 'openedMergeRequestsUrl' => nil, - 'selfUrl' => project_release_url(project, release) - } - ) - end - end - - it_behaves_like 'correct total count' - end - - # editUrl is tested separately becuase its permissions - # are slightly different than other release fields - shared_examples 'access to editUrl' do - let(:query) do - graphql_query_for(:project, { fullPath: project.full_path }, - %{ - releases { - nodes { - links { - editUrl - } - } - } - }) - end - - before do - post_query - end - - it 'returns editUrl' do - expect(data).to eq( - 'links' => { - 'editUrl' => edit_project_release_url(project, release) - } - ) - end - end - - shared_examples 'no access to editUrl' do - let(:query) do - graphql_query_for(:project, { fullPath: project.full_path }, - %{ - releases { - nodes { - links { - editUrl - } - } - } - }) - end - - before do - post_query - end - - it 'does not return editUrl' do - expect(data).to eq( - 'links' => { - 'editUrl' => nil - } - ) - end - end - - shared_examples 'no access to any release data' do - before do - post_query - end - - it 'returns nil' do - expect(data).to eq(nil) - end - end + let(:resource_type) { :project } + let(:resource) { project } describe "ensures that the correct data is returned based on the project's visibility and the user's access level" do context 'when the project is private' do @@ -312,7 +96,7 @@ RSpec.describe 'Query.project(fullPath).releases()', feature_category: :release_ def pagination_query(params) graphql_query_for( - :project, + resource_type, { full_path: sort_project.full_path }, query_graphql_field(:releases, params, "#{page_info} nodes { tagName }") ) diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index de35c943749..f49165a88ea 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -8,6 +8,7 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team 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(:reporter) { create(:user).tap { |reporter| project.add_reporter(reporter) } } let_it_be(:label1) { create(:label, project: project) } let_it_be(:label2) { create(:label, project: project) } let_it_be(:milestone1) { create(:milestone, project: project) } @@ -43,10 +44,10 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team end shared_examples 'work items resolver without N + 1 queries' do - it 'avoids N+1 queries' do + it 'avoids N+1 queries', :use_sql_query_cache do post_graphql(query, current_user: current_user) # warm-up - control = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do post_graphql(query, current_user: current_user) end @@ -59,11 +60,12 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team last_edited_at: 1.week.ago, project: project, labels: [label1, label2], - milestone: milestone2 + milestone: milestone2, + author: reporter ) + expect { post_graphql(query, current_user: current_user) }.not_to exceed_all_query_limit(control) expect_graphql_errors_to_be_empty - expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control) end end @@ -212,6 +214,19 @@ RSpec.describe 'getting a work item list for a project', feature_category: :team end end + context 'when filtering by author username' do + let_it_be(:author) { create(:author) } + let_it_be(:item_3) { create(:work_item, project: project, author: author) } + + let(:item_filter_params) { { author_username: item_3.author.username } } + + it 'returns correct results' do + post_graphql(query, current_user: current_user) + + expect(item_ids).to match_array([item_3.to_global_id.to_s]) + end + end + describe 'sorting and pagination' do let(:data_path) { [:project, :work_items] } diff --git a/spec/requests/api/graphql/subscriptions/notes/created_spec.rb b/spec/requests/api/graphql/subscriptions/notes/created_spec.rb new file mode 100644 index 00000000000..f955c14ef3b --- /dev/null +++ b/spec/requests/api/graphql/subscriptions/notes/created_spec.rb @@ -0,0 +1,177 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe 'Subscriptions::Notes::Created', feature_category: :team_planning do + include GraphqlHelpers + include Graphql::Subscriptions::Notes::Helper + + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:task) { create(:work_item, :task, project: project) } + + let(:current_user) { nil } + let(:subscribe) { notes_subscription('workItemNoteCreated', task, current_user) } + let(:response_note) { graphql_dig_at(graphql_data(response[:result]), :workItemNoteCreated) } + let(:discussion) { graphql_dig_at(response_note, :discussion) } + let(:discussion_notes) { graphql_dig_at(discussion, :notes, :nodes) } + + before do + stub_const('GitlabSchema', Graphql::Subscriptions::ActionCable::MockGitlabSchema) + Graphql::Subscriptions::ActionCable::MockActionCable.clear_mocks + project.add_guest(guest) + project.add_reporter(reporter) + end + + subject(:response) do + subscription_response do + # this creates note defined with let lazily and triggers the subscription event + new_note + end + end + + context 'when user is unauthorized' do + let(:new_note) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + + it 'does not receive any data' do + expect(response).to be_nil + end + end + + context 'when user is authorized' do + let(:current_user) { guest } + let(:new_note) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + + it 'receives created note' do + response + note = Note.find(new_note.id) + + expect(response_note['id']).to eq(note.to_gid.to_s) + expect(discussion['id']).to eq(note.discussion.to_gid.to_s) + expect(discussion_notes.pluck('id')).to eq([note.to_gid.to_s]) + end + + context 'when a new note is created as a reply' do + let_it_be(:note, refind: true) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + + let(:new_note) do + create(:note, noteable: task, project: project, in_reply_to: note, discussion_id: note.discussion_id) + end + + it 'receives created note' do + response + reply = Note.find(new_note.id) + + expect(response_note['id']).to eq(reply.to_gid.to_s) + expect(discussion['id']).to eq(reply.discussion.to_gid.to_s) + expect(discussion_notes.pluck('id')).to eq([note.to_gid.to_s, reply.to_gid.to_s]) + end + end + + context 'when note is confidential' do + let(:current_user) { reporter } + let(:new_note) { create(:note, :confidential, noteable: task, project: project, type: 'DiscussionNote') } + + context 'and user has permission to read confidential notes' do + it 'receives created note' do + response + confidential_note = Note.find(new_note.id) + + expect(response_note['id']).to eq(confidential_note.to_gid.to_s) + expect(discussion['id']).to eq(confidential_note.discussion.to_gid.to_s) + expect(discussion_notes.pluck('id')).to eq([confidential_note.to_gid.to_s]) + end + + context 'and replying' do + let_it_be(:note, refind: true) do + create(:note, :confidential, noteable: task, project: project, type: 'DiscussionNote') + end + + let(:new_note) do + create(:note, :confidential, + noteable: task, project: project, in_reply_to: note, discussion_id: note.discussion_id) + end + + it 'receives created note' do + response + reply = Note.find(new_note.id) + + expect(response_note['id']).to eq(reply.to_gid.to_s) + expect(discussion['id']).to eq(reply.discussion.to_gid.to_s) + expect(discussion_notes.pluck('id')).to eq([note.to_gid.to_s, reply.to_gid.to_s]) + end + end + end + + context 'and user does not have permission to read confidential notes' do + let(:current_user) { guest } + let(:new_note) { create(:note, :confidential, noteable: task, project: project, type: 'DiscussionNote') } + + it 'does not receive note data' do + response + expect(response_note).to be_nil + end + end + end + end + + context 'when resource events are triggering note subscription' do + let_it_be(:label1) { create(:label, project: project, title: 'foo') } + let_it_be(:label2) { create(:label, project: project, title: 'bar') } + + subject(:response) do + subscription_response do + # this creates note defined with let lazily and triggers the subscription event + resource_event + end + end + + context 'when user is unauthorized' do + let(:resource_event) { create(:resource_label_event, issue: task, label: label1) } + + it "does not receive discussion data" do + expect(response).to be_nil + end + end + + context 'when user is authorized' do + let(:current_user) { guest } + let(:resource_event) { create(:resource_label_event, issue: task, label: label1) } + + it "receives created synthetic note as a discussion" do + response + + event = ResourceLabelEvent.find(resource_event.id) + discussion_id = event.discussion_id + discussion_gid = ::Gitlab::GlobalId.as_global_id(discussion_id, model_name: 'Discussion').to_s + note_gid = ::Gitlab::GlobalId.as_global_id(discussion_id, model_name: 'LabelNote').to_s + + expect(response_note['id']).to eq(note_gid) + expect(discussion['id']).to eq(discussion_gid) + expect(discussion_notes.size).to eq(1) + expect(discussion_notes.pluck('id')).to match_array([note_gid]) + end + + context 'when several label events are created' do + let(:resource_event) do + ResourceEvents::ChangeLabelsService.new(task, current_user).execute(added_labels: [label1, label2]) + end + + it "receives created synthetic note as a discussion" do + response + + event = ResourceLabelEvent.where(label_id: [label1, label2]).first + discussion_id = event.discussion_id + discussion_gid = ::Gitlab::GlobalId.as_global_id(discussion_id, model_name: 'Discussion').to_s + note_gid = ::Gitlab::GlobalId.as_global_id(discussion_id, model_name: 'LabelNote').to_s + + expect(response_note['id']).to eq(note_gid) + expect(discussion['id']).to eq(discussion_gid) + expect(discussion_notes.size).to eq(1) + expect(discussion_notes.pluck('id')).to match_array([note_gid]) + end + end + end + end +end diff --git a/spec/requests/api/graphql/subscriptions/notes/deleted_spec.rb b/spec/requests/api/graphql/subscriptions/notes/deleted_spec.rb new file mode 100644 index 00000000000..d98f1cfe77e --- /dev/null +++ b/spec/requests/api/graphql/subscriptions/notes/deleted_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe 'Subscriptions::Notes::Deleted', feature_category: :team_planning do + include GraphqlHelpers + include Graphql::Subscriptions::Notes::Helper + + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:task) { create(:work_item, :task, project: project) } + let_it_be(:note, refind: true) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + let_it_be(:reply_note, refind: true) do + create(:note, noteable: task, project: project, in_reply_to: note, discussion_id: note.discussion_id) + end + + let(:current_user) { nil } + let(:subscribe) { notes_subscription('workItemNoteDeleted', task, current_user) } + let(:deleted_note) { graphql_dig_at(graphql_data(response[:result]), :workItemNoteDeleted) } + + before do + stub_const('GitlabSchema', Graphql::Subscriptions::ActionCable::MockGitlabSchema) + Graphql::Subscriptions::ActionCable::MockActionCable.clear_mocks + project.add_guest(guest) + project.add_reporter(reporter) + end + + subject(:response) do + subscription_response do + note.destroy! + end + end + + context 'when user is unauthorized' do + it 'does not receive any data' do + expect(response).to be_nil + end + end + + context 'when user is authorized' do + let(:current_user) { guest } + + it 'receives note id that is removed' do + expect(deleted_note['id']).to eq(note.to_gid.to_s) + expect(deleted_note['discussionId']).to eq(note.discussion.to_gid.to_s) + expect(deleted_note['lastDiscussionNote']).to be false + end + + context 'when last discussion note is deleted' do + let_it_be(:note, refind: true) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + + it 'receives note id that is removed' do + expect(deleted_note['id']).to eq(note.to_gid.to_s) + expect(deleted_note['discussionId']).to eq(note.discussion.to_gid.to_s) + expect(deleted_note['lastDiscussionNote']).to be true + end + end + + context 'when note is confidential' do + let_it_be(:note, refind: true) do + create(:note, :confidential, noteable: task, project: project, type: 'DiscussionNote') + end + + it 'receives note id that is removed' do + expect(deleted_note['id']).to eq(note.to_gid.to_s) + expect(deleted_note['discussionId']).to eq(note.discussion.to_gid.to_s) + expect(deleted_note['lastDiscussionNote']).to be true + end + end + end +end diff --git a/spec/requests/api/graphql/subscriptions/notes/updated_spec.rb b/spec/requests/api/graphql/subscriptions/notes/updated_spec.rb new file mode 100644 index 00000000000..25c0a79e7aa --- /dev/null +++ b/spec/requests/api/graphql/subscriptions/notes/updated_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe 'Subscriptions::Notes::Updated', feature_category: :team_planning do + include GraphqlHelpers + include Graphql::Subscriptions::Notes::Helper + + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:task) { create(:work_item, :task, project: project) } + let_it_be(:note, refind: true) { create(:note, noteable: task, project: task.project, type: 'DiscussionNote') } + + let(:current_user) { nil } + let(:subscribe) { note_subscription('workItemNoteUpdated', task, current_user) } + let(:updated_note) { graphql_dig_at(graphql_data(response[:result]), :workItemNoteUpdated) } + + before do + stub_const('GitlabSchema', Graphql::Subscriptions::ActionCable::MockGitlabSchema) + Graphql::Subscriptions::ActionCable::MockActionCable.clear_mocks + project.add_guest(guest) + project.add_reporter(reporter) + end + + subject(:response) do + subscription_response do + note.update!(note: 'changing the note body') + end + end + + context 'when user is unauthorized' do + it 'does not receive any data' do + expect(response).to be_nil + end + end + + context 'when user is authorized' do + let(:current_user) { reporter } + + it 'receives updated note data' do + expect(updated_note['id']).to eq(note.to_gid.to_s) + expect(updated_note['body']).to eq('changing the note body') + end + + context 'when note is confidential' do + let_it_be(:note, refind: true) do + create(:note, :confidential, noteable: task, project: task.project, type: 'DiscussionNote') + end + + context 'and user has permission to read confidential notes' do + it 'receives updated note data' do + expect(updated_note['id']).to eq(note.to_gid.to_s) + expect(updated_note['body']).to eq('changing the note body') + end + end + + context 'and user does not have permission to read confidential notes' do + let(:current_user) { guest } + + it 'does not receive updated note data' do + expect(updated_note).to be_nil + end + end + end + end +end diff --git a/spec/requests/api/graphql/user_spec.rb b/spec/requests/api/graphql/user_spec.rb index 3e82d783a18..c19dfa6f3f3 100644 --- a/spec/requests/api/graphql/user_spec.rb +++ b/spec/requests/api/graphql/user_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User', feature_category: :users do +RSpec.describe 'User', feature_category: :user_profile do include GraphqlHelpers let_it_be(:current_user) { create(:user) } diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index 6b5d437df83..0fad4f4ff3a 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -55,7 +55,12 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do 'title' => work_item.title, 'confidential' => work_item.confidential, 'workItemType' => hash_including('id' => work_item.work_item_type.to_gid.to_s), - 'userPermissions' => { 'readWorkItem' => true, 'updateWorkItem' => true, 'deleteWorkItem' => false }, + 'userPermissions' => { + 'readWorkItem' => true, + 'updateWorkItem' => true, + 'deleteWorkItem' => false, + 'adminWorkItem' => true + }, 'project' => hash_including('id' => project.to_gid.to_s, 'fullPath' => project.full_path) ) end @@ -210,6 +215,20 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do it 'places the newest child item to the end of the children list' do expect(hierarchy_children.last['id']).to eq(newest_child.to_gid.to_s) end + + context 'when relative position is set' do + let_it_be(:first_child) { create(:work_item, :task, project: project, created_at: 5.minutes.from_now) } + + let_it_be(:first_link) do + create(:parent_link, work_item_parent: work_item, work_item: first_child, relative_position: 1) + end + + it 'places children according to relative_position at the beginning of the children list' do + ordered_list = [first_child, oldest_child, child_item1, child_item2, newest_child] + + expect(hierarchy_children.pluck('id')).to eq(ordered_list.map(&:to_gid).map(&:to_s)) + end + end end end diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 90b9606ec7b..e3d538d72ba 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -88,51 +88,70 @@ RSpec.describe API::GroupVariables, feature_category: :pipeline_authoring do context 'authorized user with proper permissions' do let(:access_level) { :owner } - 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, raw: true } - end.to change { group.variables.count }.by(1) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - 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['variable_type']).to eq('env_var') - expect(json_response['environment_scope']).to eq('*') - expect(json_response['raw']).to be_truthy + context 'when the group is below the plan limit for variables' 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, raw: true } + end.to change { group.variables.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + 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['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 + expect do + post api("/groups/#{group.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } + end.to change { group.variables.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + 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 + + it 'does not allow to duplicate variable key' do + expect do + post api("/groups/#{group.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } + end.to change { group.variables.count }.by(0) + + expect(response).to have_gitlab_http_status(:bad_request) + end 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 - expect do - post api("/groups/#{group.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } - end.to change { group.variables.count }.by(1) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - 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 - - it 'does not allow to duplicate variable key' do - expect do - post api("/groups/#{group.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } - end.to change { group.variables.count }.by(0) - - expect(response).to have_gitlab_http_status(:bad_request) + context 'when the group is at the plan limit for variables' do + before do + create(:plan_limits, :default_plan, group_ci_variables: 1) + end + + it 'returns a variable limit error' do + expect do + post api("/groups/#{group.id}/variables", user), params: { key: 'TOO_MANY_VARS', value: 'too many' } + end.not_to change { group.variables.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['base']).to contain_exactly( + 'Maximum number of group ci variables (1) exceeded' + ) + end end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 767f3e8b5b5..ca32271f573 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -651,6 +651,12 @@ RSpec.describe API::Internal::Base, feature_category: :authentication_and_author headers: gitlab_shell_internal_api_request_header ) end + + it "updates user's activity data" do + expect(::Users::ActivityService).to receive(:new).with(author: user, namespace: project.namespace, project: project) + + request + end end end end diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index dc631ad7921..be76e55269a 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -227,7 +227,7 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :kubernetes_manageme context 'an agent is found' do let_it_be(:agent_token) { create(:cluster_agent_token) } - shared_examples 'agent token tracking' + include_examples 'agent token tracking' context 'project is public' do let(:project) { create(:project, :public) } diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index 9d3ab269ca1..bb0f557cfee 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Invitations, feature_category: :users do +RSpec.describe API::Invitations, feature_category: :user_profile 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) } diff --git a/spec/requests/api/issue_links_spec.rb b/spec/requests/api/issue_links_spec.rb index 93bf17d72d7..40d8f6d2395 100644 --- a/spec/requests/api/issue_links_spec.rb +++ b/spec/requests/api/issue_links_spec.rb @@ -138,6 +138,8 @@ RSpec.describe API::IssueLinks, feature_category: :team_planning do params: { target_project_id: project.id, target_issue_iid: target_issue.iid, link_type: 'relates_to' } expect_link_response(link_type: 'relates_to') + expect(json_response['source_issue']['id']).to eq(issue.id) + expect(json_response['target_issue']['id']).to eq(target_issue.id) end it 'returns 201 when sending full path of target project' do diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index b89db82b150..4b60eaadcbc 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -139,12 +139,6 @@ RSpec.describe API::Issues, feature_category: :team_planning do expect(json_response).to be_an Array end - it_behaves_like 'issuable anonymous search' do - let(:url) { '/issues' } - let(:issuable) { issue } - let(:result) { issuable.id } - end - it_behaves_like 'issuable API rate-limited search' do let(:url) { '/issues' } let(:issuable) { issue } @@ -274,31 +268,6 @@ RSpec.describe API::Issues, feature_category: :team_planning do let(:counts) { { all: 1, closed: 0, opened: 1 } } it_behaves_like 'issues statistics' - - context 'with anonymous user' do - let(:user) { nil } - - context 'with disable_anonymous_search disabled' do - before do - stub_feature_flags(disable_anonymous_search: false) - end - - it_behaves_like 'issues statistics' - end - - context 'with disable_anonymous_search enabled' do - before do - stub_feature_flags(disable_anonymous_search: true) - end - - it 'returns a unprocessable entity 422' do - get api("/issues_statistics"), params: params - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to include('User must be authenticated to use search') - end - end - end end end end diff --git a/spec/requests/api/issues/post_projects_issues_spec.rb b/spec/requests/api/issues/post_projects_issues_spec.rb index 7305da1305a..265091fa698 100644 --- a/spec/requests/api/issues/post_projects_issues_spec.rb +++ b/spec/requests/api/issues/post_projects_issues_spec.rb @@ -432,11 +432,7 @@ RSpec.describe API::Issues, feature_category: :team_planning do } end - context 'when allow_possible_spam feature flag is false' do - before do - stub_feature_flags(allow_possible_spam: false) - end - + context 'when allow_possible_spam application setting is false' do it 'does not create a new project issue' do expect { post_issue }.not_to change(Issue, :count) end @@ -454,7 +450,11 @@ RSpec.describe API::Issues, feature_category: :team_planning do end end - context 'when allow_possible_spam feature flag is true' do + context 'when allow_possible_spam application setting is true' do + before do + stub_application_setting(allow_possible_spam: true) + end + it 'does creates a new project issue' do expect { post_issue }.to change(Issue, :count).by(1) end diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index 2d7439d65c1..f0d174c9e78 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -204,11 +204,7 @@ RSpec.describe API::Issues, feature_category: :team_planning do end end - context 'when allow_possible_spam feature flag is false' do - before do - stub_feature_flags(allow_possible_spam: false) - end - + context 'when allow_possible_spam application setting is false' do it 'does not update a project issue' do expect { update_issue }.not_to change { issue.reload.title } end @@ -226,7 +222,11 @@ RSpec.describe API::Issues, feature_category: :team_planning do end end - context 'when allow_possible_spam feature flag is true' do + context 'when allow_possible_spam application setting is true' do + before do + stub_application_setting(allow_possible_spam: true) + end + it 'updates a project issue' do expect { update_issue }.to change { issue.reload.title } end diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index 092eb442f1f..20aa660d95b 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -125,6 +125,8 @@ RSpec.describe API::MavenPackages, feature_category: :package_registry do expect_any_instance_of(Fog::AWS::Storage::Files).not_to receive(:head_url) subject + + expect(response).to have_gitlab_http_status(:redirect) end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4cd93603c31..19a630e5218 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -50,12 +50,6 @@ RSpec.describe API::MergeRequests, feature_category: :source_code_management do expect_successful_response_with_paginated_array end - it_behaves_like 'issuable anonymous search' do - let(:url) { endpoint_path } - let(:issuable) { merge_request } - let(:result) { [merge_request_merged.id, merge_request_locked.id, merge_request_closed.id, merge_request.id] } - end - it_behaves_like 'issuable API rate-limited search' do let(:url) { endpoint_path } let(:issuable) { merge_request } @@ -662,12 +656,6 @@ RSpec.describe API::MergeRequests, feature_category: :source_code_management do ) end - it_behaves_like 'issuable anonymous search' do - let(:url) { '/merge_requests' } - let(:issuable) { merge_request } - let(:result) { [merge_request_merged.id, merge_request_locked.id, merge_request_closed.id, merge_request.id] } - end - it_behaves_like 'issuable API rate-limited search' do let(:url) { '/merge_requests' } let(:issuable) { merge_request } diff --git a/spec/requests/api/namespaces_spec.rb b/spec/requests/api/namespaces_spec.rb index 30616964371..44574caf54a 100644 --- a/spec/requests/api/namespaces_spec.rb +++ b/spec/requests/api/namespaces_spec.rb @@ -263,6 +263,7 @@ RSpec.describe API::Namespaces, feature_category: :subgroups do describe 'GET /namespaces/:namespace/exists' do let_it_be(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') } let_it_be(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') } + let_it_be(:namespace_with_dot) { create(:group, name: 'With Dot', path: 'with.dot') } let_it_be(:namespace1sub) { create(:group, name: 'Sub Namespace 1', path: 'sub-namespace-1', parent: namespace1) } let_it_be(:namespace2sub) { create(:group, name: 'Sub Namespace 2', path: 'sub-namespace-2', parent: namespace2) } @@ -301,6 +302,14 @@ RSpec.describe API::Namespaces, feature_category: :subgroups do expect(response.body).to eq(expected_json) end + it 'supports dot in namespace path' do + get api("/namespaces/#{namespace_with_dot.path}/exists", user) + + expected_json = { exists: true, suggests: ["#{namespace_with_dot.path}1"] }.to_json + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(expected_json) + end + it 'returns JSON indicating the namespace does not exist without a suggestion' do get api("/namespaces/non-existing-namespace/exists", user) diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index c2d9db1e6fb..c0276e02eb7 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -210,8 +210,7 @@ RSpec.describe API::Notes, feature_category: :team_planning do let(:request_path) { "/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes" } before do - stub_const('WorkItems::Type::BASE_TYPES', { issue: { name: 'NoNotesWidget', enum_value: 0 } }) - stub_const('WorkItems::Type::WIDGETS_FOR_TYPE', { issue: [::WorkItems::Widgets::Description] }) + WorkItems::Type.default_by_type(:issue).widget_definitions.find_by_widget_type(:notes).update!(disabled: true) end it 'does not fetch notes' do diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index cc399d25429..60406f380a5 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -90,7 +90,6 @@ ci_cd_settings: unexposed_attributes: - id - project_id - - group_runners_enabled - merge_trains_enabled - merge_pipelines_enabled - auto_rollback_enabled @@ -115,6 +114,7 @@ build_import_state: # import_state - last_update_at - last_successful_update_at - correlation_id_value + - checksums remapped_attributes: status: import_status last_error: import_error @@ -161,6 +161,9 @@ project_setting: - jitsu_key - mirror_branch_regex - allow_pipeline_trigger_approve_deployment + - emails_enabled + - pages_unique_domain_enabled + - pages_unique_domain build_service_desk_setting: # service_desk_setting unexposed_attributes: @@ -168,5 +171,13 @@ build_service_desk_setting: # service_desk_setting - issue_template_key - file_template_project_id - outgoing_name + - custom_email_enabled + - custom_email + - custom_email_smtp_address + - custom_email_smtp_port + - custom_email_smtp_username + - encrypted_custom_email_smtp_password + - encrypted_custom_email_smtp_password_iv + - custom_email_smtp_password remapped_attributes: project_key: service_desk_address diff --git a/spec/requests/api/project_events_spec.rb b/spec/requests/api/project_events_spec.rb index 69d8eb76cf3..f904cd8fd6c 100644 --- a/spec/requests/api/project_events_spec.rb +++ b/spec/requests/api/project_events_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::ProjectEvents, feature_category: :users do +RSpec.describe API::ProjectEvents, feature_category: :user_profile do let(:user) { create(:user) } let(:non_member) { create(:user) } let(:private_project) { create(:project, :private, creator_id: user.id, namespace: user.namespace) } diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb index d3adef85f8d..c003ae9cd48 100644 --- a/spec/requests/api/project_packages_spec.rb +++ b/spec/requests/api/project_packages_spec.rb @@ -88,7 +88,7 @@ RSpec.describe API::ProjectPackages, feature_category: :package_registry do end context 'with JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: user) } + let(:job) { create(:ci_build, :running, user: user, project: project) } subject { get api(url, job_token: job.token) } @@ -130,7 +130,7 @@ RSpec.describe API::ProjectPackages, feature_category: :package_registry do end context 'with JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: user) } + let(:job) { create(:ci_build, :running, user: user, project: project) } subject { get api(url, job_token: job.token) } @@ -229,8 +229,8 @@ RSpec.describe API::ProjectPackages, feature_category: :package_registry do get api(package_url, user) end - pipeline = create(:ci_pipeline, user: user) - create(:ci_build, user: user, pipeline: pipeline) + pipeline = create(:ci_pipeline, user: user, project: project) + create(:ci_build, user: user, pipeline: pipeline, project: project) create(:package_build_info, package: package1, pipeline: pipeline) expect do @@ -262,7 +262,7 @@ RSpec.describe API::ProjectPackages, feature_category: :package_registry do it_behaves_like 'no destroy url' context 'with JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: user) } + let(:job) { create(:ci_build, :running, user: user, project: project) } subject { get api(package_url, job_token: job.token) } @@ -324,7 +324,7 @@ RSpec.describe API::ProjectPackages, feature_category: :package_registry do end context 'with JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: user) } + let(:job) { create(:ci_build, :running, user: user, project: project) } subject { get api(package_url, job_token: job.token) } @@ -430,7 +430,7 @@ RSpec.describe API::ProjectPackages, feature_category: :package_registry do end context 'with JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: user) } + let(:job) { create(:ci_build, :running, user: user, project: project) } it 'returns 403 for a user without enough permissions' do project.add_developer(user) diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 568486deb7f..267557b8137 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -256,7 +256,6 @@ RSpec.describe API::ProjectSnippets, feature_category: :source_code_management d allow_next_instance_of(Spam::AkismetService) do |instance| allow(instance).to receive(:spam?).and_return(true) end - stub_feature_flags(allow_possible_spam: false) project.add_developer(user) end @@ -312,8 +311,6 @@ RSpec.describe API::ProjectSnippets, feature_category: :source_code_management d allow_next_instance_of(Spam::AkismetService) do |instance| allow(instance).to receive(:spam?).and_return(true) end - - stub_feature_flags(allow_possible_spam: false) end context 'when the snippet is private' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index d62f8a32453..e78ef2f7630 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.shared_examples 'languages and percentages JSON response', feature_category: :projects do +RSpec.shared_examples 'languages and percentages JSON response' do let(:expected_languages) { project.repository.languages.to_h { |language| language.values_at(:label, :value) } } before do @@ -46,7 +46,7 @@ RSpec.shared_examples 'languages and percentages JSON response', feature_categor end end -RSpec.describe API::Projects do +RSpec.describe API::Projects, feature_category: :projects do include ProjectForksHelper include WorkhorseHelpers include StubRequests @@ -207,7 +207,7 @@ RSpec.describe API::Projects do let(:current_user) { user } end - shared_examples 'includes container_registry_access_level', :aggregate_failures do + shared_examples 'includes container_registry_access_level' do it do project.project_feature.update!(container_registry_access_level: ProjectFeature::DISABLED) @@ -2227,6 +2227,89 @@ RSpec.describe API::Projects do end end + describe 'GET /project/:id/share_locations' do + let_it_be(:root_group) { create(:group, :public, name: 'root group') } + let_it_be(:project_group1) { create(:group, :public, parent: root_group, name: 'group1') } + let_it_be(:project_group2) { create(:group, :public, parent: root_group, name: 'group2') } + let_it_be(:project) { create(:project, :private, group: project_group1) } + + shared_examples_for 'successful groups response' do + it 'returns an array of groups' do + request + + aggregate_failures 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 { |g| g['name'] }).to match_array(expected_groups.map(&:name)) + end + end + end + + context 'when unauthenticated' do + it 'does not return the groups for the given project' do + get api("/projects/#{project.id}/share_locations") + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when authenticated' do + context 'when user is not the owner of the project' do + it 'does not return the groups' do + get api("/projects/#{project.id}/share_locations", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is the owner of the project' do + let(:request) { get api("/projects/#{project.id}/share_locations", user), params: params } + let(:params) { {} } + + before do + project.add_owner(user) + project_group1.add_developer(user) + project_group2.add_developer(user) + end + + context 'with default search' do + it_behaves_like 'successful groups response' do + let(:expected_groups) { [project_group1, project_group2] } + end + end + + context 'when searching by group name' do + let(:params) { { search: 'group1' } } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [project_group1] } + end + end + end + end + + context 'when authenticated as admin' do + let(:request) { get api("/projects/#{project.id}/share_locations", admin), params: {} } + + context 'without share_with_group_lock' do + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group1, project_group2] } + end + end + + context 'with share_with_group_lock' do + before do + project.namespace.update!(share_with_group_lock: true) + end + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [] } + end + end + end + end + describe 'GET /projects/:id' do context 'when unauthenticated' do it 'does not return private projects' do @@ -2297,7 +2380,7 @@ RSpec.describe API::Projects do let(:project_attributes) { YAML.load_file(project_attributes_file) } let(:expected_keys) do - keys = project_attributes.map do |relation, relation_config| + keys = project_attributes.flat_map do |relation, relation_config| begin actual_keys = project.send(relation).attributes.keys rescue NoMethodError @@ -2307,7 +2390,7 @@ RSpec.describe API::Projects do remapped_attributes = relation_config['remapped_attributes'] || {} computed_attributes = relation_config['computed_attributes'] || [] actual_keys - unexposed_attributes - remapped_attributes.keys + remapped_attributes.values + computed_attributes - end.flatten + end unless Gitlab.ee? keys -= %w[ @@ -2359,6 +2442,7 @@ RSpec.describe API::Projects do expect(json_response['created_at']).to be_present expect(json_response['last_activity_at']).to be_present expect(json_response['shared_runners_enabled']).to be_present + expect(json_response['group_runners_enabled']).to be_present expect(json_response['creator_id']).to be_present expect(json_response['namespace']).to be_present expect(json_response['avatar_url']).to be_nil @@ -2463,6 +2547,7 @@ RSpec.describe API::Projects do expect(json_response['created_at']).to be_present expect(json_response['last_activity_at']).to be_present expect(json_response['shared_runners_enabled']).to be_present + expect(json_response['group_runners_enabled']).to be_present expect(json_response['creator_id']).to be_present expect(json_response['namespace']).to be_present expect(json_response['import_status']).to be_present @@ -3662,8 +3747,8 @@ RSpec.describe API::Projects do aggregate_failures "testing response" do expect(response).to have_gitlab_http_status(:ok) - expect(json_response['avatar_url']).to eq('http://localhost/uploads/'\ - '-/system/project/avatar/'\ + expect(json_response['avatar_url']).to eq('http://localhost/uploads/' \ + '-/system/project/avatar/' \ "#{project3.id}/banana_sample.gif") end end @@ -3678,8 +3763,8 @@ RSpec.describe API::Projects do aggregate_failures "testing response" do expect(response).to have_gitlab_http_status(:ok) - expect(json_response['avatar_url']).to eq('http://localhost/uploads/'\ - '-/system/project/avatar/'\ + expect(json_response['avatar_url']).to eq('http://localhost/uploads/' \ + '-/system/project/avatar/' \ "#{project_with_avatar.id}/rails_sample.png") end end @@ -3695,8 +3780,8 @@ RSpec.describe API::Projects do aggregate_failures "testing response" do expect(response).to have_gitlab_http_status(:ok) expect(json_response['description']).to eq('changed description') - expect(json_response['avatar_url']).to eq('http://localhost/uploads/'\ - '-/system/project/avatar/'\ + expect(json_response['avatar_url']).to eq('http://localhost/uploads/' \ + '-/system/project/avatar/' \ "#{project_with_avatar.id}/banana_sample.gif") end end @@ -4634,25 +4719,66 @@ RSpec.describe API::Projects do describe 'POST /projects/:id/housekeeping' do let(:housekeeping) { Repositories::HousekeepingService.new(project) } + let(:params) { {} } + + subject { post api("/projects/#{project.id}/housekeeping", user), params: params } before do - allow(Repositories::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping) + allow(Repositories::HousekeepingService).to receive(:new).with(project, :eager).and_return(housekeeping) end context 'when authenticated as owner' do it 'starts the housekeeping process' do expect(housekeeping).to receive(:execute).once - post api("/projects/#{project.id}/housekeeping", user) + subject expect(response).to have_gitlab_http_status(:created) end + it 'logs an audit event' do + expect(housekeeping).to receive(:execute).once.and_yield + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(a_hash_including( + name: 'manually_trigger_housekeeping', + author: user, + scope: project, + target: project, + message: "Housekeeping task: eager" + )) + + subject + end + + context 'when requesting prune' do + let(:params) { { task: :prune } } + + it 'triggers a prune' do + expect(Repositories::HousekeepingService).to receive(:new).with(project, :prune).and_return(housekeeping) + expect(housekeeping).to receive(:execute).once + + subject + + expect(response).to have_gitlab_http_status(:created) + end + end + + context 'when requesting an unsupported task' do + let(:params) { { task: :unsupported_task } } + + it 'responds with bad_request' do + expect(Repositories::HousekeepingService).not_to receive(:new) + + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + context 'when housekeeping lease is taken' do it 'returns conflict' do expect(housekeeping).to receive(:execute).once.and_raise(Repositories::HousekeepingService::LeaseTaken) - post api("/projects/#{project.id}/housekeeping", user) + subject expect(response).to have_gitlab_http_status(:conflict) expect(json_response['message']).to match(/Somebody already triggered housekeeping for this resource/) diff --git a/spec/requests/api/release/links_spec.rb b/spec/requests/api/release/links_spec.rb index 4a7821fcb0a..462cc1e3b5d 100644 --- a/spec/requests/api/release/links_spec.rb +++ b/spec/requests/api/release/links_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe API::Release::Links, feature_category: :release_orchestration do + include Ci::JobTokenScopeHelpers + let(:project) { create(:project, :repository, :private) } let(:maintainer) { create(:user) } let(:developer) { create(:user) } @@ -51,7 +53,7 @@ RSpec.describe API::Release::Links, feature_category: :release_orchestration do end context 'when using JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: maintainer) } + let(:job) { create(:ci_build, :running, user: maintainer, project: project) } it 'returns releases links' do get api("/projects/#{project.id}/releases/v0.1/assets/links", job_token: job.token) @@ -127,7 +129,7 @@ RSpec.describe API::Release::Links, feature_category: :release_orchestration do end context 'when using JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: maintainer) } + let(:job) { create(:ci_build, :running, user: maintainer, project: project) } it 'returns releases link' do get api("/projects/#{project.id}/releases/v0.1/assets/links/#{release_link.id}", job_token: job.token) @@ -241,7 +243,7 @@ RSpec.describe API::Release::Links, feature_category: :release_orchestration do end context 'when using JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: maintainer) } + let(:job) { create(:ci_build, :running, user: maintainer, project: project) } it 'creates a new release link' do expect do @@ -385,7 +387,7 @@ RSpec.describe API::Release::Links, feature_category: :release_orchestration do end context 'when using JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: maintainer) } + let(:job) { create(:ci_build, :running, user: maintainer, project: project) } it 'updates the release link' do put api("/projects/#{project.id}/releases/v0.1/assets/links/#{release_link.id}"), params: params.merge(job_token: job.token) @@ -496,7 +498,7 @@ RSpec.describe API::Release::Links, feature_category: :release_orchestration do end context 'when using JOB-TOKEN auth' do - let(:job) { create(:ci_build, :running, user: maintainer) } + let(:job) { create(:ci_build, :running, user: maintainer, project: project) } it 'deletes the release link' do expect do diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index e209ad2b2d5..c3f99872cef 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -1215,11 +1215,23 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do end context 'with a project milestone' do - let(:milestone_params) { { milestones: [milestone.title] } } + shared_examples 'adds milestone' do + it 'adds the milestone' do + expect(response).to have_gitlab_http_status(:created) + expect(returned_milestones).to match_array(['v1.0']) + end + end - it 'adds the milestone' do - expect(response).to have_gitlab_http_status(:created) - expect(returned_milestones).to match_array(['v1.0']) + context 'by title' do + let(:milestone_params) { { milestones: [milestone.title] } } + + it_behaves_like 'adds milestone' + end + + context 'by id' do + let(:milestone_params) { { milestone_ids: [milestone.id] } } + + it_behaves_like 'adds milestone' end end @@ -1408,18 +1420,14 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do context 'when a milestone is passed in' do let(:milestone) { create(:milestone, project: project, title: 'v1.0') } - let(:milestone_title) { milestone.title } - let(:params) { { milestones: [milestone_title] } } + let!(:milestone2) { create(:milestone, project: project, title: 'v2.0') } before do release.milestones << milestone end - context 'a different milestone' do - let(:milestone_title) { 'v2.0' } - let!(:milestone2) { create(:milestone, project: project, title: milestone_title) } - - it 'replaces the milestone' do + shared_examples 'updates milestone' do + it 'updates the milestone' do subject expect(response).to have_gitlab_http_status(:ok) @@ -1427,8 +1435,20 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do end end + context 'by title' do + let(:params) { { milestones: [milestone2.title] } } + + it_behaves_like 'updates milestone' + end + + context 'by id' do + let(:params) { { milestone_ids: [milestone2.id] } } + + it_behaves_like 'updates milestone' + end + context 'an identical milestone' do - let(:milestone_title) { 'v1.0' } + let(:params) { { milestones: [milestone.title] } } it 'does not change the milestone' do subject @@ -1439,7 +1459,7 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do end context 'an empty milestone' do - let(:milestone_title) { nil } + let(:params) { { milestones: [] } } it 'removes the milestone' do subject @@ -1476,13 +1496,26 @@ RSpec.describe API::Releases, feature_category: :release_orchestration do context 'with all new' do let!(:milestone2) { create(:milestone, project: project, title: 'milestone2') } let!(:milestone3) { create(:milestone, project: project, title: 'milestone3') } - let(:params) { { milestones: [milestone2.title, milestone3.title] } } - it 'replaces the milestones' do - subject + shared_examples 'update milestones' do + it 'replaces the milestones' do + subject - expect(response).to have_gitlab_http_status(:ok) - expect(returned_milestones).to match_array(%w(milestone2 milestone3)) + expect(response).to have_gitlab_http_status(:ok) + expect(returned_milestones).to match_array(%w(milestone2 milestone3)) + end + end + + context 'by title' do + let(:params) { { milestones: [milestone2.title, milestone3.title] } } + + it_behaves_like 'update milestones' + end + + context 'by id' do + let(:params) { { milestone_ids: [milestone2.id, milestone3.id] } } + + it_behaves_like 'update milestones' end end end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index dd0da0cb887..2bc4c177bc9 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -340,7 +340,6 @@ RSpec.describe API::Snippets, factory_default: :keep, feature_category: :source_ allow_next_instance_of(Spam::AkismetService) do |instance| allow(instance).to receive(:spam?).and_return(true) end - stub_feature_flags(allow_possible_spam: false) end context 'when the snippet is private' do @@ -406,7 +405,6 @@ RSpec.describe API::Snippets, factory_default: :keep, feature_category: :source_ allow_next_instance_of(Spam::AkismetService) do |instance| allow(instance).to receive(:spam?).and_return(true) end - stub_feature_flags(allow_possible_spam: false) end context 'when the snippet is private' do diff --git a/spec/requests/api/users_preferences_spec.rb b/spec/requests/api/users_preferences_spec.rb index 53f366371e5..ef9735fd8b0 100644 --- a/spec/requests/api/users_preferences_spec.rb +++ b/spec/requests/api/users_preferences_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Users, feature_category: :users do +RSpec.describe API::Users, feature_category: :user_profile do let_it_be(:user) { create(:user) } describe 'PUT /user/preferences/' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index c063187fdf4..34867b13db2 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Users, feature_category: :users do +RSpec.describe API::Users, feature_category: :user_profile do include WorkhorseHelpers let_it_be(:admin) { create(:admin) } diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 66337b94c75..02b99eba8ce 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -7,6 +7,7 @@ RSpec.describe 'Git HTTP requests', feature_category: :source_code_management do include TermsHelper include GitHttpHelpers include WorkhorseHelpers + include Ci::JobTokenScopeHelpers shared_examples 'pulls require Basic HTTP Authentication' do context "when no credentials are provided" do @@ -869,14 +870,15 @@ RSpec.describe 'Git HTTP requests', feature_category: :source_code_management do context "when a gitlab ci token is provided" do let(:project) { create(:project, :repository) } - let(:build) { create(:ci_build, :running) } - let(:other_project) { create(:project, :repository) } - - before do - build.update!(project: project) # can't associate it on factory create + let(:build) { create(:ci_build, :running, project: project, user: user) } + let(:other_project) do + create(:project, :repository).tap do |o| + make_project_fully_accessible(project, o) + end end context 'when build created by system is authenticated' do + let(:user) { nil } let(:path) { "#{project.full_path}.git" } let(:env) { { user: 'gitlab-ci-token', password: build.token } } @@ -899,12 +901,7 @@ RSpec.describe 'Git HTTP requests', feature_category: :source_code_management do context 'and build created by' do before do - build.update!(user: user) project.add_reporter(user) - create(:ci_job_token_project_scope_link, - source_project: project, - target_project: other_project, - added_by: user) end shared_examples 'can download code only' do @@ -1474,19 +1471,16 @@ RSpec.describe 'Git HTTP requests', feature_category: :source_code_management do context "when a gitlab ci token is provided" do let(:project) { create(:project, :repository) } - let(:build) { create(:ci_build, :running) } - let(:other_project) { create(:project, :repository) } - - before do - build.update!(project: project) # can't associate it on factory create - create(:ci_job_token_project_scope_link, - source_project: project, - target_project: other_project, - added_by: user) + let(:build) { create(:ci_build, :running, project: project, user: user) } + let(:other_project) do + create(:project, :repository).tap do |o| + make_project_fully_accessible(project, o) + end end # legacy behavior that is blocked/deprecated context 'when build created by system is authenticated' do + let(:user) { nil } let(:path) { "#{project.full_path}.git" } let(:env) { { user: 'gitlab-ci-token', password: build.token } } @@ -1505,7 +1499,6 @@ RSpec.describe 'Git HTTP requests', feature_category: :source_code_management do context 'and build created by' do before do - build.update!(user: user) project.add_reporter(user) end @@ -1862,13 +1855,9 @@ RSpec.describe 'Git HTTP requests', feature_category: :source_code_management do end context 'from CI' do - let(:build) { create(:ci_build, :running) } + let(:build) { create(:ci_build, :running, user: user, project: project) } let(:env) { { user: 'gitlab-ci-token', password: build.token } } - before do - build.update!(user: user, project: project) - end - it_behaves_like 'pulls are allowed' end end diff --git a/spec/requests/groups/usage_quotas_controller_spec.rb b/spec/requests/groups/usage_quotas_controller_spec.rb index 90fd08063f3..a329398aab3 100644 --- a/spec/requests/groups/usage_quotas_controller_spec.rb +++ b/spec/requests/groups/usage_quotas_controller_spec.rb @@ -23,7 +23,7 @@ RSpec.describe Groups::UsageQuotasController, :with_license, feature_category: : request expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to match(/Placeholder for usage quotas Vue app/) + expect(response.body).to match(/js-usage-quotas-view/) end it 'renders 404 page if subgroup' do diff --git a/spec/requests/jira_connect/public_keys_controller_spec.rb b/spec/requests/jira_connect/public_keys_controller_spec.rb index bf472469d85..7f0262eaf65 100644 --- a/spec/requests/jira_connect/public_keys_controller_spec.rb +++ b/spec/requests/jira_connect/public_keys_controller_spec.rb @@ -5,10 +5,11 @@ require 'spec_helper' RSpec.describe JiraConnect::PublicKeysController, feature_category: :integrations do describe 'GET /-/jira_connect/public_keys/:uuid' do let(:uuid) { non_existing_record_id } - let(:public_key_storage_enabled) { true } + let(:public_key_storage_enabled_config) { true } before do - allow(Gitlab.config.jira_connect).to receive(:enable_public_keys_storage).and_return(public_key_storage_enabled) + allow(Gitlab.config.jira_connect).to receive(:enable_public_keys_storage) + .and_return(public_key_storage_enabled_config) end it 'renders 404' do @@ -29,25 +30,25 @@ RSpec.describe JiraConnect::PublicKeysController, feature_category: :integration expect(response.body).to eq(public_key.key) end - context 'when public key storage disabled' do - let(:public_key_storage_enabled) { false } + context 'when public key storage config disabled' do + let(:public_key_storage_enabled_config) { false } it 'renders 404' do get jira_connect_public_key_path(id: uuid) expect(response).to have_gitlab_http_status(:not_found) end - end - context 'when jira_connect_oauth_self_managed disabled' do - before do - stub_feature_flags(jira_connect_oauth_self_managed: false) - end + context 'when public key storage setting is enabled' do + before do + stub_application_setting(jira_connect_public_key_storage_enabled: true) + end - it 'renders 404' do - get jira_connect_public_key_path(id: uuid) + it 'renders 404' do + get jira_connect_public_key_path(id: uuid) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:ok) + end end end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 49279024bd0..9035e723abe 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -192,7 +192,7 @@ RSpec.describe 'OpenID Connect requests', feature_category: :authentication_and_ end it 'does not include any unknown properties' do - expect(@payload.keys).to eq %w[iss sub aud exp iat auth_time sub_legacy email email_verified groups_direct] + expect(@payload.keys).to eq %w[iss sub aud exp iat auth_time sub_legacy name nickname preferred_username email email_verified website profile picture groups_direct] end it 'does include groups' do @@ -276,7 +276,7 @@ RSpec.describe 'OpenID Connect requests', feature_category: :authentication_and_ expect(response).to have_gitlab_http_status(:ok) expect(json_response['issuer']).to eq('http://localhost') expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys') - expect(json_response['scopes_supported']).to match_array %w[api read_user read_api read_repository write_repository sudo openid profile email] + expect(json_response['scopes_supported']).to match_array %w[admin_mode api read_user read_api read_repository write_repository sudo openid profile email] end context 'with a cross-origin request' do @@ -286,7 +286,7 @@ RSpec.describe 'OpenID Connect requests', feature_category: :authentication_and_ expect(response).to have_gitlab_http_status(:ok) expect(json_response['issuer']).to eq('http://localhost') expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys') - expect(json_response['scopes_supported']).to match_array %w[api read_user read_api read_repository write_repository sudo openid profile email] + expect(json_response['scopes_supported']).to match_array %w[admin_mode api read_user read_api read_repository write_repository sudo openid profile email] end it_behaves_like 'cross-origin GET request' diff --git a/spec/requests/profiles/keys_controller_spec.rb b/spec/requests/profiles/keys_controller_spec.rb new file mode 100644 index 00000000000..48c382e6230 --- /dev/null +++ b/spec/requests/profiles/keys_controller_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Profiles::KeysController, feature_category: :source_code_management do + let_it_be(:user) { create(:user) } + + before do + login_as(user) + end + + describe 'DELETE /-/profile/keys/:id/revoke' do + it 'returns 404 if a key not found' do + delete revoke_profile_key_path(non_existing_record_id) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'revokes ssh commit signatures' do + key = create(:key, user: user) + signature = create(:ssh_signature, key: key) + + expect do + delete revoke_profile_key_path(signature.key) + end.to change { signature.reload.key }.from(signature.key).to(nil) + .and change { signature.verification_status }.from('verified').to('revoked_key') + + expect(response).to have_gitlab_http_status(:found) + end + end +end diff --git a/spec/requests/profiles/saved_replies_controller_spec.rb b/spec/requests/profiles/saved_replies_controller_spec.rb new file mode 100644 index 00000000000..27a961a201f --- /dev/null +++ b/spec/requests/profiles/saved_replies_controller_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Profiles::SavedRepliesController, feature_category: :user_profile do + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + describe 'GET #index' do + describe 'feature flag disabled' do + before do + stub_feature_flags(saved_replies: false) + + get '/-/profile/saved_replies' + end + + it { expect(response).to have_gitlab_http_status(:not_found) } + end + + describe 'feature flag enabled' do + before do + get '/-/profile/saved_replies' + end + + it { expect(response).to have_gitlab_http_status(:ok) } + + it 'sets hide search settings ivar' do + expect(assigns(:hide_search_settings)).to eq(true) + end + end + end +end diff --git a/spec/requests/projects/airflow/dags_controller_spec.rb b/spec/requests/projects/airflow/dags_controller_spec.rb new file mode 100644 index 00000000000..2dcedf5f128 --- /dev/null +++ b/spec/requests/projects/airflow/dags_controller_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::Airflow::DagsController, feature_category: :dataops do + let_it_be(:non_member) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group).tap { |p| p.add_developer(user) } } + let_it_be(:project) { create(:project, group: group).tap { |p| p.add_developer(user) } } + + let(:current_user) { user } + let(:feature_flag) { true } + + let_it_be(:dags) do + create_list(:airflow_dags, 5, project: project) + end + + let(:params) { { namespace_id: project.namespace.to_param, project_id: project } } + let(:extra_params) { {} } + + before do + sign_in(current_user) if current_user + stub_feature_flags(airflow_dags: false) + stub_feature_flags(airflow_dags: project) if feature_flag + list_dags + end + + shared_examples 'returns a 404 if feature flag disabled' do + context 'when :airflow_dags disabled' do + let(:feature_flag) { false } + + it 'is 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'GET index' do + it 'renders the template' do + expect(response).to render_template('projects/airflow/dags/index') + end + + describe 'pagination' do + before do + stub_const("Projects::Airflow::DagsController::MAX_DAGS_PER_PAGE", 2) + dags + + list_dags + end + + context 'when out of bounds' do + let(:params) { extra_params.merge(page: 10000) } + + it 'redirects to last page' do + last_page = (dags.size + 1) / 2 + expect(response).to redirect_to(project_airflow_dags_path(project, page: last_page)) + end + end + + context 'when bad page' do + let(:params) { extra_params.merge(page: 's') } + + it 'uses first page' do + expect(assigns(:pagination)).to include( + page: 1, + is_last_page: false, + per_page: 2, + total_items: dags.size) + end + end + end + + it 'does not perform N+1 sql queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { list_dags } + + create_list(:airflow_dags, 1, project: project) + + expect { list_dags }.not_to exceed_all_query_limit(control_count) + end + + context 'when user is not logged in' do + let(:current_user) { nil } + + it 'redirects to login' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'when user is not a member' do + let(:current_user) { non_member } + + it 'returns a 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + + it_behaves_like 'returns a 404 if feature flag disabled' + end + + private + + def list_dags + get project_airflow_dags_path(project), params: params + end +end diff --git a/spec/requests/projects/blob_spec.rb b/spec/requests/projects/blob_spec.rb new file mode 100644 index 00000000000..7d62619e76a --- /dev/null +++ b/spec/requests/projects/blob_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Blobs', feature_category: :source_code_management do + let_it_be(:project) { create(:project, :public, :repository, lfs: true) } + + describe 'GET /:namespace_id/:project_id/-/blob/:id' do + subject(:request) do + get namespace_project_blob_path(namespace_id: project.namespace, project_id: project, id: id) + end + + context 'with LFS file' do + let(:id) { 'master/files/lfs/lfs_object.iso' } + let(:object_store_host) { 'http://127.0.0.1:9000' } + let(:connect_src) do + csp = response.headers['Content-Security-Policy'] + csp.split('; ').find { |src| src.starts_with?('connect-src') } + end + + let(:gitlab_config) do + Gitlab.config.gitlab.deep_merge( + 'content_security_policy' => { + 'enabled' => content_security_policy_enabled + } + ) + end + + let(:lfs_config) do + Gitlab.config.lfs.deep_merge( + 'enabled' => lfs_enabled, + 'object_store' => { + 'remote_directory' => 'lfs-objects', + 'enabled' => true, + 'proxy_download' => proxy_download, + 'connection' => { + 'endpoint' => object_store_host, + 'path_style' => true + } + } + ) + end + + before do + stub_config_setting(gitlab_config) + stub_lfs_setting(lfs_config) + stub_lfs_object_storage(proxy_download: proxy_download) + + request + end + + describe 'directly downloading lfs file' do + let(:lfs_enabled) { true } + let(:proxy_download) { false } + let(:content_security_policy_enabled) { true } + + it { expect(response).to have_gitlab_http_status(:success) } + + it { expect(connect_src).to include(object_store_host) } + + context 'when lfs is disabled' do + let(:lfs_enabled) { false } + + it { expect(response).to have_gitlab_http_status(:success) } + + it { expect(connect_src).not_to include(object_store_host) } + end + + context 'when content_security_policy is disabled' do + let(:content_security_policy_enabled) { false } + + it { expect(response).to have_gitlab_http_status(:success) } + + it { expect(connect_src).not_to include(object_store_host) } + end + + context 'when proxy download is enabled' do + let(:proxy_download) { true } + + it { expect(response).to have_gitlab_http_status(:success) } + + it { expect(connect_src).not_to include(object_store_host) } + end + end + end + end +end diff --git a/spec/requests/projects/google_cloud/databases_controller_spec.rb b/spec/requests/projects/google_cloud/databases_controller_spec.rb index e91a51ce2ef..98e83610600 100644 --- a/spec/requests/projects/google_cloud/databases_controller_spec.rb +++ b/spec/requests/projects/google_cloud/databases_controller_spec.rb @@ -94,23 +94,33 @@ RSpec.describe Projects::GoogleCloud::DatabasesController, :snowplow, feature_ca post project_google_cloud_databases_path(project) end - it 'calls EnableCloudsqlService and redirects on error' do - expect_next_instance_of(::GoogleCloud::EnableCloudsqlService) do |service| - expect(service).to receive(:execute) - .and_return({ status: :error, message: 'error' }) + context 'when EnableCloudsqlService fails' do + before do + allow_next_instance_of(::GoogleCloud::EnableCloudsqlService) do |service| + allow(service).to receive(:execute) + .and_return({ status: :error, message: 'error' }) + end end - subject + it 'redirects and track event on error' do + subject + + expect(response).to redirect_to(project_google_cloud_databases_path(project)) + + expect_snowplow_event( + category: 'Projects::GoogleCloud::DatabasesController', + action: 'error_enable_cloudsql_services', + label: nil, + project: project, + user: user + ) + end - expect(response).to redirect_to(project_google_cloud_databases_path(project)) + it 'shows a flash alert' do + subject - expect_snowplow_event( - category: 'Projects::GoogleCloud::DatabasesController', - action: 'error_enable_cloudsql_services', - label: nil, - project: project, - user: user - ) + expect(flash[:alert]).to eq(s_('CloudSeed|Google Cloud Error - error')) + end end context 'when EnableCloudsqlService is successful' do @@ -121,23 +131,33 @@ RSpec.describe Projects::GoogleCloud::DatabasesController, :snowplow, feature_ca end end - it 'calls CreateCloudsqlInstanceService and redirects on error' do - expect_next_instance_of(::GoogleCloud::CreateCloudsqlInstanceService) do |service| - expect(service).to receive(:execute) - .and_return({ status: :error, message: 'error' }) + context 'when CreateCloudsqlInstanceService fails' do + before do + allow_next_instance_of(::GoogleCloud::CreateCloudsqlInstanceService) do |service| + allow(service).to receive(:execute) + .and_return({ status: :error, message: 'error' }) + end end - subject + it 'redirects and track event on error' do + subject - expect(response).to redirect_to(project_google_cloud_databases_path(project)) + expect(response).to redirect_to(project_google_cloud_databases_path(project)) - expect_snowplow_event( - category: 'Projects::GoogleCloud::DatabasesController', - action: 'error_create_cloudsql_instance', - label: nil, - project: project, - user: user - ) + expect_snowplow_event( + category: 'Projects::GoogleCloud::DatabasesController', + action: 'error_create_cloudsql_instance', + label: nil, + project: project, + user: user + ) + end + + it 'shows a flash warning' do + subject + + expect(flash[:warning]).to eq(s_('CloudSeed|Google Cloud Error - error')) + end end context 'when CreateCloudsqlInstanceService is successful' do @@ -161,6 +181,18 @@ RSpec.describe Projects::GoogleCloud::DatabasesController, :snowplow, feature_ca user: user ) end + + it 'shows a flash notice' do + subject + + expect(flash[:notice]) + .to eq( + s_( + 'CloudSeed|Cloud SQL instance creation request successful. ' \ + 'Expected resolution time is ~5 minutes.' + ) + ) + end end end end diff --git a/spec/requests/projects/ml/experiments_controller_spec.rb b/spec/requests/projects/ml/experiments_controller_spec.rb index e8b6f806251..9b071efc1f1 100644 --- a/spec/requests/projects/ml/experiments_controller_spec.rb +++ b/spec/requests/projects/ml/experiments_controller_spec.rb @@ -38,31 +38,74 @@ RSpec.describe Projects::Ml::ExperimentsController, feature_category: :mlops do end describe 'GET index' do - before do - list_experiments - end + describe 'renderering' do + before do + list_experiments + end - it 'renders the template' do - expect(response).to render_template('projects/ml/experiments/index') + it 'renders the template' do + expect(response).to render_template('projects/ml/experiments/index') + end + + it 'does not perform N+1 sql queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { list_experiments } + + create_list(:ml_experiments, 2, project: project, user: user) + + expect { list_experiments }.not_to exceed_all_query_limit(control_count) + end end - it 'does not perform N+1 sql queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { list_experiments } + describe 'pagination' do + let_it_be(:experiments) do + create_list(:ml_experiments, 3, project: project_with_feature) + end - create_list(:ml_experiments, 2, project: project, user: user) + let(:params) { basic_params.merge(id: experiment.iid) } - expect { list_experiments }.not_to exceed_all_query_limit(control_count) + before do + stub_const("Projects::Ml::ExperimentsController::MAX_EXPERIMENTS_PER_PAGE", 2) + + list_experiments + end + + it 'fetches only MAX_CANDIDATES_PER_PAGE candidates' do + expect(assigns(:experiments).size).to eq(2) + end + + it 'paginates', :aggregate_failures do + page = assigns(:experiments) + + expect(page.first).to eq(experiments.last) + expect(page.last).to eq(experiments[1]) + + new_params = params.merge(cursor: assigns(:page_info)[:end_cursor]) + + list_experiments(new_params) + + new_page = assigns(:experiments) + + expect(new_page.first).to eq(experiments.first) + end end context 'when :ml_experiment_tracking is disabled for the project' do let(:project) { project_without_feature } + before do + list_experiments + end + it 'responds with a 404' do expect(response).to have_gitlab_http_status(:not_found) end end - it_behaves_like '404 if feature flag disabled' + it_behaves_like '404 if feature flag disabled' do + before do + list_experiments + end + end end describe 'GET show' do @@ -75,36 +118,85 @@ RSpec.describe Projects::Ml::ExperimentsController, feature_category: :mlops do end describe 'pagination' do - let_it_be(:candidates) { create_list(:ml_candidates, 5, experiment: experiment) } + let_it_be(:candidates) do + create_list(:ml_candidates, 5, experiment: experiment).tap do |c| + c.first.metrics.create!(name: 'metric1', value: 0.3) + c[1].metrics.create!(name: 'metric1', value: 0.2) + c.last.metrics.create!(name: 'metric1', value: 0.6) + end + end + + let(:params) { basic_params.merge(id: experiment.iid) } before do stub_const("Projects::Ml::ExperimentsController::MAX_CANDIDATES_PER_PAGE", 2) - candidates show_experiment end - context 'when out of bounds' do - let(:params) { basic_params.merge(id: experiment.iid, page: 10000) } + it 'fetches only MAX_CANDIDATES_PER_PAGE candidates' do + expect(assigns(:candidates).size).to eq(2) + end + + it 'paginates' do + received = assigns(:page_info) - it 'redirects to last page' do - last_page = (experiment.candidates.size + 1) / 2 + expect(received).to include({ + has_next_page: true, + has_previous_page: false, + start_cursor: nil + }) + end + + context 'when order by metric' do + let(:params) do + { + order_by: "metric1", + order_by_type: "metric", + sort: "desc" + } + end + + it 'paginates', :aggregate_failures do + page = assigns(:candidates) + + expect(page.first).to eq(candidates.last) + expect(page.last).to eq(candidates.first) + + new_params = params.merge(cursor: assigns(:page_info)[:end_cursor]) - expect(response).to redirect_to(project_ml_experiment_path(project, experiment.iid, page: last_page)) + show_experiment(new_params) + + new_page = assigns(:candidates) + + expect(new_page.first).to eq(candidates[1]) end end + end - context 'when bad page' do - let(:params) { basic_params.merge(id: experiment.iid, page: 's') } + describe 'search' do + let(:params) do + basic_params.merge( + id: experiment.iid, + name: 'some_name', + orderBy: 'name', + orderByType: 'metric', + sort: 'asc', + invalid: 'invalid' + ) + end - it 'uses first page' do - expect(assigns(:pagination)).to include( - page: 1, - is_last_page: false, - per_page: 2, - total_items: experiment.candidates&.size - ) + it 'formats and filters the parameters' do + expect(Projects::Ml::CandidateFinder).to receive(:new).and_call_original do |exp, params| + expect(params.to_h).to include({ + name: 'some_name', + order_by: 'name', + order_by_type: 'metric', + sort: 'asc' + }) end + + show_experiment end end @@ -125,11 +217,11 @@ RSpec.describe Projects::Ml::ExperimentsController, feature_category: :mlops do private - def show_experiment - get project_ml_experiment_path(project, experiment.iid), params: params + def show_experiment(new_params = nil) + get project_ml_experiment_path(project, experiment.iid), params: new_params || params end - def list_experiments - get project_ml_experiments_path(project), params: params + def list_experiments(new_params = nil) + get project_ml_experiments_path(project), params: new_params || params end end diff --git a/spec/requests/projects/network_controller_spec.rb b/spec/requests/projects/network_controller_spec.rb index 954f9655558..dee95c6e70e 100644 --- a/spec/requests/projects/network_controller_spec.rb +++ b/spec/requests/projects/network_controller_spec.rb @@ -35,17 +35,6 @@ RSpec.describe Projects::NetworkController, feature_category: :source_code_manag subject expect(assigns(:url)).to eq(project_network_path(project, ref, format: :json, ref_type: 'heads')) end - - context 'when the use_ref_type_parameter flag is disabled' do - before do - stub_feature_flags(use_ref_type_parameter: false) - end - - it 'assigns url without ref_type' do - subject - expect(assigns(:url)).to eq(project_network_path(project, ref, format: :json)) - end - end end it 'assigns url' do diff --git a/spec/requests/projects/noteable_notes_spec.rb b/spec/requests/projects/noteable_notes_spec.rb index 5699bf17b80..55540447da0 100644 --- a/spec/requests/projects/noteable_notes_spec.rb +++ b/spec/requests/projects/noteable_notes_spec.rb @@ -36,5 +36,41 @@ RSpec.describe 'Project noteable notes', feature_category: :team_planning do expect(response).to have_gitlab_http_status(:ok) expect(response_etag).to eq(stored_etag) end + + it "instruments cache hits correctly" do + etag_store.touch(notes_path) + + expect(Gitlab::Metrics::RailsSlis.request_apdex).to( + receive(:increment).with( + labels: { + request_urgency: :medium, + feature_category: "team_planning", + endpoint_id: "Projects::NotesController#index" + }, + success: be_in([true, false]) + ) + ) + allow(ActiveSupport::Notifications).to receive(:instrument).and_call_original + + expect(ActiveSupport::Notifications).to( + receive(:instrument).with( + 'process_action.action_controller', + a_hash_including( + { + request_urgency: :medium, + target_duration_s: 0.5, + metadata: a_hash_including({ + 'meta.feature_category' => 'team_planning', + 'meta.caller_id' => "Projects::NotesController#index" + }) + } + ) + ) + ) + + get notes_path, headers: { "if-none-match": stored_etag } + + expect(response).to have_gitlab_http_status(:not_modified) + end end end diff --git a/spec/requests/projects/pipelines_controller_spec.rb b/spec/requests/projects/pipelines_controller_spec.rb index 7f185ade339..73e002b63b1 100644 --- a/spec/requests/projects/pipelines_controller_spec.rb +++ b/spec/requests/projects/pipelines_controller_spec.rb @@ -19,6 +19,32 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte login_as(user) end + describe "GET index.json" do + it 'does not execute N+1 queries' do + get_pipelines_index + + control_count = ActiveRecord::QueryRecorder.new do + get_pipelines_index + end.count + + %w[pending running success failed canceled].each do |status| + create(:ci_pipeline, project: project, status: status) + end + + # There appears to be one extra query for Pipelines#has_warnings? for some reason + expect { get_pipelines_index }.not_to exceed_query_limit(control_count + 1) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['pipelines'].count).to eq 6 + end + + def get_pipelines_index + get namespace_project_pipelines_path( + namespace_id: project.namespace.to_param, + project_id: project.to_param, + format: :json) + end + end + describe "GET stages.json" do it 'does not execute N+1 queries' do request_build_stage diff --git a/spec/requests/projects/releases_controller_spec.rb b/spec/requests/projects/releases_controller_spec.rb index d331142583d..42fd55b5a43 100644 --- a/spec/requests/projects/releases_controller_spec.rb +++ b/spec/requests/projects/releases_controller_spec.rb @@ -8,17 +8,20 @@ RSpec.describe 'Projects::ReleasesController', feature_category: :release_orches before do project.add_developer(user) - login_as(user) end # Added as a request spec because of https://gitlab.com/gitlab-org/gitlab/-/issues/232386 describe 'GET #downloads' do - context 'filepath redirection' do - let_it_be(:release) { create(:release, project: project, tag: 'v11.9.0-rc2' ) } - let!(:link) { create(:release_link, release: release, name: 'linux-amd64 binaries', filepath: filepath, url: 'https://aws.example.com/s3/project/bin/hello-darwin-amd64') } - let_it_be(:url) { "#{project_releases_path(project)}/#{release.tag}/downloads/bin/darwin-amd64" } + let_it_be(:release) { create(:release, project: project, tag: 'v11.9.0-rc2' ) } + let!(:link) { create(:release_link, release: release, name: 'linux-amd64 binaries', filepath: filepath, url: 'https://aws.example.com/s3/project/bin/hello-darwin-amd64') } + let_it_be(:url) { "#{project_releases_path(project)}/#{release.tag}/downloads/bin/darwin-amd64" } - let(:subject) { get url } + let(:subject) { get url } + + context 'filepath redirection' do + before do + login_as(user) + end context 'valid filepath' do let(:filepath) { '/bin/darwin-amd64' } @@ -47,14 +50,29 @@ RSpec.describe 'Projects::ReleasesController', feature_category: :release_orches end end - context 'invalid filepath' do - let(:invalid_filepath) { 'bin/darwin-amd64' } + context 'sessionless download authentication' do + let(:personal_access_token) { create(:personal_access_token, user: user) } + let(:filepath) { '/bin/darwin-amd64' } + + subject { get url, params: { private_token: personal_access_token.token } } - let(:subject) { create(:release_link, name: 'linux-amd64 binaries', filepath: invalid_filepath, url: 'https://aws.example.com/s3/project/bin/hello-darwin-amd64') } + it 'will allow sessionless users to download the file' do + subject - it 'cannot create an invalid filepath' do - expect { subject }.to raise_error(ActiveRecord::RecordInvalid) + expect(controller.current_user).to eq(user) + expect(response).to have_gitlab_http_status(:redirect) + expect(response).to redirect_to(link.url) end end end + + context 'invalid filepath' do + let(:invalid_filepath) { 'bin/darwin-amd64' } + + let(:subject) { create(:release_link, name: 'linux-amd64 binaries', filepath: invalid_filepath, url: 'https://aws.example.com/s3/project/bin/hello-darwin-amd64') } + + it 'cannot create an invalid filepath' do + expect { subject }.to raise_error(ActiveRecord::RecordInvalid) + end + end end diff --git a/spec/requests/pwa_controller_spec.rb b/spec/requests/pwa_controller_spec.rb index a80d083c11f..08eeefd1dc4 100644 --- a/spec/requests/pwa_controller_spec.rb +++ b/spec/requests/pwa_controller_spec.rb @@ -4,27 +4,74 @@ require 'spec_helper' RSpec.describe PwaController, feature_category: :navigation do describe 'GET #manifest' do - it 'responds with json' do - get manifest_path(format: :json) + shared_examples 'text values' do |params, result| + let_it_be(:appearance) { create(:appearance, **params) } - expect(response.body).to include('The complete DevOps platform.') - expect(Gitlab::Json.parse(response.body)).to include({ 'short_name' => 'GitLab' }) - expect(response).to have_gitlab_http_status(:success) + it 'uses custom values', :aggregate_failures do + get manifest_path(format: :json) + + expect(Gitlab::Json.parse(response.body)).to include(result) + expect(response).to have_gitlab_http_status(:success) + end + end + + context 'with default appearance' do + it_behaves_like 'text values', {}, { + 'name' => 'GitLab', + 'short_name' => 'GitLab', + 'description' => 'The complete DevOps platform. ' \ + 'One application with endless possibilities. ' \ + 'Organizations rely on GitLab’s source code management, ' \ + 'CI/CD, security, and more to deliver software rapidly.' + } end context 'with customized appearance' do - let_it_be(:appearance) do - create(:appearance, title: 'Long name', pwa_short_name: 'Short name', description: 'This is a test') + context 'with custom text values' do + it_behaves_like 'text values', { pwa_name: 'PWA name' }, { 'name' => 'PWA name' } + it_behaves_like 'text values', { pwa_short_name: 'Short name' }, { 'short_name' => 'Short name' } + it_behaves_like 'text values', { pwa_description: 'This is a test' }, { 'description' => 'This is a test' } end - it 'uses custom values', :aggregate_failures do - get manifest_path(format: :json) + shared_examples 'icon paths' do + it 'returns expected icon paths', :aggregate_failures do + get manifest_path(format: :json) + + expect(Gitlab::Json.parse(response.body)["icons"]).to match_array(result) + expect(response).to have_gitlab_http_status(:success) + end + end + + context 'with custom icon' do + let_it_be(:appearance) { create(:appearance, :with_pwa_icon) } + let_it_be(:result) do + [{ "src" => "/uploads/-/system/appearance/pwa_icon/#{appearance.id}/dk.png?width=192", "sizes" => "192x192", + "type" => "image/png" }, + { "src" => "/uploads/-/system/appearance/pwa_icon/#{appearance.id}/dk.png?width=512", "sizes" => "512x512", + "type" => "image/png" }] + end + + it_behaves_like 'icon paths' + end - expect(Gitlab::Json.parse(response.body)).to include({ - 'description' => 'This is a test', - 'name' => 'Long name', - 'short_name' => 'Short name' - }) + context 'with no custom icon' do + let_it_be(:appearance) { create(:appearance) } + let_it_be(:result) do + [{ "src" => "/-/pwa-icons/logo-192.png", "sizes" => "192x192", "type" => "image/png" }, + { "src" => "/-/pwa-icons/logo-512.png", "sizes" => "512x512", "type" => "image/png" }, + { "src" => "/-/pwa-icons/maskable-logo.png", "sizes" => "512x512", "type" => "image/png", + "purpose" => "maskable" }] + end + + it_behaves_like 'icon paths' + end + end + + describe 'GET #offline' do + it 'responds with static HTML page' do + get offline_path + + expect(response.body).to include('You are currently offline') expect(response).to have_gitlab_http_status(:success) end end @@ -46,13 +93,4 @@ RSpec.describe PwaController, feature_category: :navigation do end end end - - describe 'GET #offline' do - it 'responds with static HTML page' do - get offline_path - - expect(response.body).to include('You are currently offline') - expect(response).to have_gitlab_http_status(:success) - end - end end diff --git a/spec/requests/user_activity_spec.rb b/spec/requests/user_activity_spec.rb index f9682d81640..16188ab6a41 100644 --- a/spec/requests/user_activity_spec.rb +++ b/spec/requests/user_activity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Update of user activity', feature_category: :users do +RSpec.describe 'Update of user activity', feature_category: :user_profile do paths_to_visit = [ '/group', '/group/project', diff --git a/spec/requests/user_avatar_spec.rb b/spec/requests/user_avatar_spec.rb index 4e3c2744d56..0a9f3784833 100644 --- a/spec/requests/user_avatar_spec.rb +++ b/spec/requests/user_avatar_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Loading a user avatar', feature_category: :users do +RSpec.describe 'Loading a user avatar', feature_category: :user_profile do let(:user) { create(:user, :with_avatar) } context 'when logged in' do diff --git a/spec/requests/verifies_with_email_spec.rb b/spec/requests/verifies_with_email_spec.rb index cac754a9cb1..8a6a7e717ff 100644 --- a/spec/requests/verifies_with_email_spec.rb +++ b/spec/requests/verifies_with_email_spec.rb @@ -223,6 +223,7 @@ feature_category: :user_management do context 'when the feature flag is toggled on' do before do stub_feature_flags(require_email_verification: user) + stub_feature_flags(skip_require_email_verification: false) end it_behaves_like 'verifying with email' @@ -242,6 +243,14 @@ feature_category: :user_management do it_behaves_like 'verifying with email' end + + context 'when the skip_require_email_verification feature flag is turned on' do + before do + stub_feature_flags(skip_require_email_verification: user) + end + + it_behaves_like 'not verifying with email' + end end end end -- cgit v1.2.3