diff options
Diffstat (limited to 'spec/controllers')
130 files changed, 3152 insertions, 2680 deletions
diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 32ac0f8dc07..a721722a5c3 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_setting do +RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_setting, feature_category: :shared do include StubENV include UsageDataHelpers @@ -204,8 +204,29 @@ RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_set expect(ApplicationSetting.current.valid_runner_registrars).to eq(['project']) end + it 'updates GitLab for Slack app settings' do + settings = { + slack_app_enabled: true, + slack_app_id: 'slack_app_id', + slack_app_secret: 'slack_app_secret', + slack_app_signing_secret: 'slack_app_signing_secret', + slack_app_verification_token: 'slack_app_verification_token' + } + + put :update, params: { application_setting: settings } + + expect(response).to redirect_to(general_admin_application_settings_path) + expect(ApplicationSetting.current).to have_attributes( + slack_app_enabled: true, + slack_app_id: 'slack_app_id', + slack_app_secret: 'slack_app_secret', + slack_app_signing_secret: 'slack_app_signing_secret', + slack_app_verification_token: 'slack_app_verification_token' + ) + end + context 'boolean attributes' do - shared_examples_for 'updates booolean attribute' do |attribute| + shared_examples_for 'updates boolean attribute' do |attribute| specify do existing_value = ApplicationSetting.current.public_send(attribute) new_value = !existing_value @@ -217,10 +238,11 @@ RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_set end end - it_behaves_like 'updates booolean attribute', :user_defaults_to_private_profile - it_behaves_like 'updates booolean attribute', :can_create_group - it_behaves_like 'updates booolean attribute', :admin_mode - it_behaves_like 'updates booolean attribute', :require_admin_approval_after_user_signup + it_behaves_like 'updates boolean attribute', :user_defaults_to_private_profile + it_behaves_like 'updates boolean attribute', :can_create_group + it_behaves_like 'updates boolean attribute', :admin_mode + it_behaves_like 'updates boolean attribute', :require_admin_approval_after_user_signup + it_behaves_like 'updates boolean attribute', :remember_me_enabled end context "personal access token prefix settings" do @@ -398,9 +420,20 @@ RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_set expect(application_settings.reload.invitation_flow_enforcement).to eq(true) end end + + context 'maximum includes' do + let(:application_settings) { ApplicationSetting.current } + + it 'updates ci_max_includes setting' do + put :update, params: { application_setting: { ci_max_includes: 200 } } + + expect(response).to redirect_to(general_admin_application_settings_path) + expect(application_settings.reload.ci_max_includes).to eq(200) + end + end end - describe 'PUT #reset_registration_token', feature_category: :credential_management do + describe 'PUT #reset_registration_token', feature_category: :user_management do before do sign_in(admin) end diff --git a/spec/controllers/admin/applications_controller_spec.rb b/spec/controllers/admin/applications_controller_spec.rb index bf7707f177c..1feda0ed36f 100644 --- a/spec/controllers/admin/applications_controller_spec.rb +++ b/spec/controllers/admin/applications_controller_spec.rb @@ -38,44 +38,49 @@ RSpec.describe Admin::ApplicationsController do end end - describe 'POST #create' do - context 'with hash_oauth_secrets flag off' do - before do - stub_feature_flags(hash_oauth_secrets: false) - end + describe 'PUT #renew' do + let(:oauth_params) do + { + id: application.id + } + end - it 'creates the application' do - create_params = attributes_for(:application, trusted: true, confidential: false, scopes: ['api']) + subject { put :renew, params: oauth_params } - expect do - post :create, params: { doorkeeper_application: create_params } - end.to change { Doorkeeper::Application.count }.by(1) + it { is_expected.to have_gitlab_http_status(:ok) } + it { expect { subject }.to change { application.reload.secret } } - application = Doorkeeper::Application.last + it 'returns the secret in json format' do + subject - expect(response).to redirect_to(admin_application_path(application)) - expect(application).to have_attributes(create_params.except(:uid, :owner_type)) - end + expect(json_response['secret']).not_to be_nil end - context 'with hash_oauth_secrets flag on' do + context 'when renew fails' do before do - stub_feature_flags(hash_oauth_secrets: true) + allow_next_found_instance_of(Doorkeeper::Application) do |application| + allow(application).to receive(:save).and_return(false) + end end - it 'creates the application' do - create_params = attributes_for(:application, trusted: true, confidential: false, scopes: ['api']) + it { expect { subject }.not_to change { application.reload.secret } } + it { is_expected.to have_gitlab_http_status(:unprocessable_entity) } + end + end - expect do - post :create, params: { doorkeeper_application: create_params } - end.to change { Doorkeeper::Application.count }.by(1) + describe 'POST #create' do + it 'creates the application' do + create_params = attributes_for(:application, trusted: true, confidential: false, scopes: ['api']) - application = Doorkeeper::Application.last + expect do + post :create, params: { doorkeeper_application: create_params } + end.to change { Doorkeeper::Application.count }.by(1) - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template :show - expect(application).to have_attributes(create_params.except(:uid, :owner_type)) - end + application = Doorkeeper::Application.last + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show + expect(application).to have_attributes(create_params.except(:uid, :owner_type)) end it 'renders the application form on errors' do @@ -88,43 +93,18 @@ RSpec.describe Admin::ApplicationsController do end context 'when the params are for a confidential application' do - context 'with hash_oauth_secrets flag off' do - before do - stub_feature_flags(hash_oauth_secrets: false) - end - - it 'creates a confidential application' do - create_params = attributes_for(:application, confidential: true, scopes: ['read_user']) - - expect do - post :create, params: { doorkeeper_application: create_params } - end.to change { Doorkeeper::Application.count }.by(1) - - application = Doorkeeper::Application.last - - expect(response).to redirect_to(admin_application_path(application)) - expect(application).to have_attributes(create_params.except(:uid, :owner_type)) - end - end + it 'creates a confidential application' do + create_params = attributes_for(:application, confidential: true, scopes: ['read_user']) - context 'with hash_oauth_secrets flag on' do - before do - stub_feature_flags(hash_oauth_secrets: true) - end - - it 'creates a confidential application' do - create_params = attributes_for(:application, confidential: true, scopes: ['read_user']) - - expect do - post :create, params: { doorkeeper_application: create_params } - end.to change { Doorkeeper::Application.count }.by(1) + expect do + post :create, params: { doorkeeper_application: create_params } + end.to change { Doorkeeper::Application.count }.by(1) - application = Doorkeeper::Application.last + application = Doorkeeper::Application.last - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template :show - expect(application).to have_attributes(create_params.except(:uid, :owner_type)) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show + expect(application).to have_attributes(create_params.except(:uid, :owner_type)) end end diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index 8e62aeed7d0..d04cd20f4e6 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Admin::ClustersController, feature_category: :kubernetes_management do +RSpec.describe Admin::ClustersController, feature_category: :deployment_management do include AccessMatchersForController include GoogleApi::CloudPlatformHelpers @@ -259,14 +259,6 @@ RSpec.describe Admin::ClustersController, feature_category: :kubernetes_manageme expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('cluster_status') end - - it 'invokes schedule_status_update on each application' do - expect_next_instance_of(Clusters::Applications::Ingress) do |instance| - expect(instance).to receive(:schedule_status_update) - end - - get_cluster_status - end end describe 'security' do @@ -292,20 +284,37 @@ RSpec.describe Admin::ClustersController, feature_category: :kubernetes_manageme end describe 'functionality' do - render_views + context 'when remove_monitor_metrics FF is disabled' do + before do + stub_feature_flags(remove_monitor_metrics: false) + end - it 'responds successfully' do - get_show + render_views - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:cluster)).to eq(cluster) + it 'responds successfully' do + get_show + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:cluster)).to eq(cluster) + end + + it 'renders integration tab view' do + get_show(tab: 'integrations') + + expect(response).to render_template('clusters/clusters/_integrations') + expect(response).to have_gitlab_http_status(:ok) + end end - it 'renders integration tab view' do - get_show(tab: 'integrations') + context 'when remove_monitor_metrics FF is enabled' do + render_views - expect(response).to render_template('clusters/clusters/_integrations') - expect(response).to have_gitlab_http_status(:ok) + it 'renders details tab view' do + get_show(tab: 'integrations') + + expect(response).to render_template('clusters/clusters/_details') + expect(response).to have_gitlab_http_status(:ok) + end end end diff --git a/spec/controllers/admin/cohorts_controller_spec.rb b/spec/controllers/admin/cohorts_controller_spec.rb index 50626a5da91..26f6540258e 100644 --- a/spec/controllers/admin/cohorts_controller_spec.rb +++ b/spec/controllers/admin/cohorts_controller_spec.rb @@ -17,7 +17,6 @@ RSpec.describe Admin::CohortsController do it_behaves_like 'Snowplow event tracking with RedisHLL context' do subject { get :index } - let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } let(:category) { described_class.name } let(:action) { 'perform_analytics_usage_action' } let(:label) { 'redis_hll_counters.analytics.analytics_total_unique_counts_monthly' } diff --git a/spec/controllers/admin/dev_ops_report_controller_spec.rb b/spec/controllers/admin/dev_ops_report_controller_spec.rb index 52a46b5e99a..d8166380760 100644 --- a/spec/controllers/admin/dev_ops_report_controller_spec.rb +++ b/spec/controllers/admin/dev_ops_report_controller_spec.rb @@ -32,7 +32,6 @@ RSpec.describe Admin::DevOpsReportController do it_behaves_like 'Snowplow event tracking with RedisHLL context' do subject { get :show, format: :html } - let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } let(:category) { described_class.name } let(:action) { 'perform_analytics_usage_action' } let(:label) { 'redis_hll_counters.analytics.analytics_total_unique_counts_monthly' } diff --git a/spec/controllers/admin/instance_review_controller_spec.rb b/spec/controllers/admin/instance_review_controller_spec.rb index 6eab135b3a6..f0225a71e00 100644 --- a/spec/controllers/admin/instance_review_controller_spec.rb +++ b/spec/controllers/admin/instance_review_controller_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Admin::InstanceReviewController, feature_category: :service_ping include UsageDataHelpers let(:admin) { create(:admin) } - let(:subscriptions_instance_review_url) { Gitlab::SubscriptionPortal.subscriptions_instance_review_url } + let(:subscriptions_instance_review_url) { ::Gitlab::Routing.url_helpers.subscription_portal_instance_review_url } before do sign_in(admin) diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index e75f27589d7..9e2a2900b33 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Admin::IntegrationsController do let(:admin) { create(:admin) } before do + stub_feature_flags(remove_monitor_metrics: false) sign_in(admin) end @@ -29,11 +30,7 @@ RSpec.describe Admin::IntegrationsController do end end - context 'when GitLab.com' do - before do - allow(::Gitlab).to receive(:com?) { true } - end - + context 'when GitLab.com', :saas do it 'returns 404' do get :edit, params: { id: Integration.available_integration_names.sample } diff --git a/spec/controllers/admin/runner_projects_controller_spec.rb b/spec/controllers/admin/runner_projects_controller_spec.rb index 38cc2d171ac..06a73984ac0 100644 --- a/spec/controllers/admin/runner_projects_controller_spec.rb +++ b/spec/controllers/admin/runner_projects_controller_spec.rb @@ -21,30 +21,32 @@ RSpec.describe Admin::RunnerProjectsController, feature_category: :runner_fleet } end - context 'assigning runner to same project' do - let(:project_runner) { create(:ci_runner, :project, projects: [project]) } + context 'when assigning to another project' do + let(:project_runner) { create(:ci_runner, :project, projects: [source_project]) } + let(:source_project) { create(:project) } it 'redirects to the admin runner edit page' do send_create + expect(flash[:success]).to be_present expect(response).to have_gitlab_http_status(:redirect) expect(response).to redirect_to edit_admin_runner_url(project_runner) end end - context 'assigning runner to another project' do - let(:project_runner) { create(:ci_runner, :project, projects: [source_project]) } - let(:source_project) { create(:project) } + context 'when assigning to same project' do + let(:project_runner) { create(:ci_runner, :project, projects: [project]) } it 'redirects to the admin runner edit page' do send_create + expect(flash[:alert]).to be_present expect(response).to have_gitlab_http_status(:redirect) expect(response).to redirect_to edit_admin_runner_url(project_runner) end end - context 'for unknown project' do + context 'when assigning to an unknown project' do let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project]) } let(:project_id) { 0 } @@ -70,7 +72,7 @@ RSpec.describe Admin::RunnerProjectsController, feature_category: :runner_fleet } end - context 'unassigning runner from project' do + context 'when unassigning runner from project' do let(:runner_project_id) { project_runner.runner_projects.last.id } it 'redirects to the admin runner edit page' do @@ -81,7 +83,7 @@ RSpec.describe Admin::RunnerProjectsController, feature_category: :runner_fleet end end - context 'for unknown project runner relationship' do + context 'when unassigning from unknown project' do let(:runner_project_id) { 0 } it 'shows 404 for unknown project runner relationship' do diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index a39a1f38a11..b1a2d90589a 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -35,26 +35,59 @@ RSpec.describe Admin::RunnersController, feature_category: :runner_fleet do end describe '#new' do - context 'when create_runner_workflow is enabled' do + it 'renders a :new template' do + get :new + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:new) + end + + context 'when create_runner_workflow_for_admin is disabled' do before do - stub_feature_flags(create_runner_workflow: true) + stub_feature_flags(create_runner_workflow_for_admin: false) end - it 'renders a :new template' do + it 'returns :not_found' do get :new + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe '#register' do + subject(:register) { get :register, params: { id: new_runner.id } } + + context 'when runner can be registered after creation' do + let_it_be(:new_runner) { create(:ci_runner, registration_type: :authenticated_user) } + + it 'renders a :register template' do + register + expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:new) + expect(response).to render_template(:register) end end - context 'when create_runner_workflow is disabled' do + context 'when runner cannot be registered after creation' do + let_it_be(:new_runner) { runner } + + it 'returns :not_found' do + register + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when create_runner_workflow_for_admin is disabled' do + let_it_be(:new_runner) { create(:ci_runner, registration_type: :authenticated_user) } + before do - stub_feature_flags(create_runner_workflow: false) + stub_feature_flags(create_runner_workflow_for_admin: false) end it 'returns :not_found' do - get :new + register expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/controllers/admin/sessions_controller_spec.rb b/spec/controllers/admin/sessions_controller_spec.rb index 5fa7a7f278d..07088eed6d4 100644 --- a/spec/controllers/admin/sessions_controller_spec.rb +++ b/spec/controllers/admin/sessions_controller_spec.rb @@ -220,7 +220,9 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do end end - shared_examples 'when using two-factor authentication via hardware device' do + context 'when using two-factor authentication via WebAuthn' do + let(:user) { create(:admin, :two_factor_via_webauthn) } + def authenticate_2fa(user_params) post(:create, params: { user: user_params }, session: { otp_user_id: user.id }) end @@ -237,10 +239,6 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do end it 'can login with valid auth' do - # we can stub both without an differentiation between webauthn / u2f - # as these not interfere with each other und this saves us passing aroud - # parameters - allow(U2fRegistration).to receive(:authenticate).and_return(true) allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(true) expect(controller.current_user_mode.admin_mode?).to be(false) @@ -255,7 +253,6 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do end it 'cannot login with invalid auth' do - allow(U2fRegistration).to receive(:authenticate).and_return(false) allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(false) expect(controller.current_user_mode.admin_mode?).to be(false) @@ -267,22 +264,6 @@ RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do expect(controller.current_user_mode.admin_mode?).to be(false) end end - - context 'when using two-factor authentication via U2F' do - it_behaves_like 'when using two-factor authentication via hardware device' do - let(:user) { create(:admin, :two_factor_via_u2f) } - - before do - stub_feature_flags(webauthn: false) - end - end - end - - context 'when using two-factor authentication via WebAuthn' do - it_behaves_like 'when using two-factor authentication via hardware device' do - let(:user) { create(:admin, :two_factor_via_webauthn) } - end - end end end diff --git a/spec/controllers/admin/spam_logs_controller_spec.rb b/spec/controllers/admin/spam_logs_controller_spec.rb index 51f7ecdece6..b39c3bd009b 100644 --- a/spec/controllers/admin/spam_logs_controller_spec.rb +++ b/spec/controllers/admin/spam_logs_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Admin::SpamLogsController do +RSpec.describe Admin::SpamLogsController, feature_category: :instance_resiliency do let(:admin) { create(:admin) } let(:user) { create(:user) } let!(:first_spam) { create(:spam_log, user: user) } @@ -13,9 +13,10 @@ RSpec.describe Admin::SpamLogsController do end describe '#index' do - it 'lists all spam logs' do + it 'lists paginated spam logs' do get :index + expect(assigns(:spam_logs)).to be_kind_of(Kaminari::PaginatableWithoutCount) expect(response).to have_gitlab_http_status(:ok) end end @@ -33,10 +34,7 @@ RSpec.describe Admin::SpamLogsController do end.not_to change { SpamLog.count } expect(response).to have_gitlab_http_status(:found) - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin) - ).to be_exists + expect(Users::GhostUserMigration.where(user: user, initiator_user: admin)).to be_exists expect(flash[:notice]).to eq("User #{user.username} was successfully removed.") end end diff --git a/spec/controllers/admin/usage_trends_controller_spec.rb b/spec/controllers/admin/usage_trends_controller_spec.rb index 87cf8988b4e..7b801d53f14 100644 --- a/spec/controllers/admin/usage_trends_controller_spec.rb +++ b/spec/controllers/admin/usage_trends_controller_spec.rb @@ -17,7 +17,6 @@ RSpec.describe Admin::UsageTrendsController do it_behaves_like 'Snowplow event tracking with RedisHLL context' do subject { get :index } - let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } let(:category) { described_class.name } let(:action) { 'perform_analytics_usage_action' } let(:label) { 'redis_hll_counters.analytics.analytics_total_unique_counts_monthly' } diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 63e68118066..9b00451de30 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -185,22 +185,14 @@ RSpec.describe Admin::UsersController do delete :destroy, params: { id: user.username }, format: :json expect(response).to have_gitlab_http_status(:ok) - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin, - hard_delete: false) - ).to be_exists + expect(Users::GhostUserMigration.where(user: user, initiator_user: admin, hard_delete: false)).to be_exists end it 'initiates user removal and passes hard delete option' do delete :destroy, params: { id: user.username, hard_delete: true }, format: :json expect(response).to have_gitlab_http_status(:ok) - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin, - hard_delete: true) - ).to be_exists + expect(Users::GhostUserMigration.where(user: user, initiator_user: admin, hard_delete: true)).to be_exists end context 'prerequisites for account deletion' do @@ -231,11 +223,7 @@ RSpec.describe Admin::UsersController do expect(response).to redirect_to(admin_users_path) expect(flash[:notice]).to eq(_('The user is being deleted.')) - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin, - hard_delete: true) - ).to be_exists + expect(Users::GhostUserMigration.where(user: user, initiator_user: admin, hard_delete: true)).to be_exists end end end @@ -252,10 +240,7 @@ RSpec.describe Admin::UsersController do it 'initiates user removal', :sidekiq_inline do subject - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin) - ).to be_exists + expect(Users::GhostUserMigration.where(user: user, initiator_user: admin)).to be_exists end it 'displays the rejection message' do @@ -403,7 +388,7 @@ RSpec.describe Admin::UsersController do put :deactivate, params: { id: user.username } user.reload expect(user.deactivated?).to be_falsey - expect(flash[:notice]).to eq("The user you are trying to deactivate has been active in the past #{Gitlab::CurrentSettings.deactivate_dormant_users_period} days and cannot be deactivated") + expect(flash[:alert]).to eq("The user you are trying to deactivate has been active in the past #{Gitlab::CurrentSettings.deactivate_dormant_users_period} days and cannot be deactivated") end end end @@ -425,7 +410,7 @@ RSpec.describe Admin::UsersController do put :deactivate, params: { id: user.username } user.reload expect(user.deactivated?).to be_falsey - expect(flash[:notice]).to eq('Error occurred. A blocked user cannot be deactivated') + expect(flash[:alert]).to eq('Error occurred. A blocked user cannot be deactivated') end end @@ -436,7 +421,7 @@ RSpec.describe Admin::UsersController do put :deactivate, params: { id: internal_user.username } expect(internal_user.reload.deactivated?).to be_falsey - expect(flash[:notice]).to eq('Internal users cannot be deactivated') + expect(flash[:alert]).to eq('Internal users cannot be deactivated') end end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f1adb9020fa..ce76be9f509 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe ApplicationController do +RSpec.describe ApplicationController, feature_category: :shared do include TermsHelper let(:user) { create(:user) } @@ -736,23 +736,11 @@ RSpec.describe ApplicationController do end end - context 'user not logged in' do - it 'sets the default headers' do - get :index - - expect(response.headers['Cache-Control']).to be_nil - expect(response.headers['Pragma']).to be_nil - end - end - - context 'user logged in' do - it 'sets the default headers' do - sign_in(user) - - get :index + it 'sets the default headers' do + get :index - expect(response.headers['Pragma']).to eq 'no-cache' - end + expect(response.headers['Cache-Control']).to be_nil + expect(response.headers['Pragma']).to be_nil end end @@ -779,7 +767,6 @@ RSpec.describe ApplicationController do subject expect(response.headers['Cache-Control']).to eq 'private, no-store' - expect(response.headers['Pragma']).to eq 'no-cache' expect(response.headers['Expires']).to eq 'Fri, 01 Jan 1990 00:00:00 GMT' end @@ -905,12 +892,12 @@ RSpec.describe ApplicationController do end end - describe 'rescue_from Gitlab::Auth::IpBlacklisted' do + describe 'rescue_from Gitlab::Auth::IpBlocked' do controller(described_class) do skip_before_action :authenticate_user! def index - raise Gitlab::Auth::IpBlacklisted + raise Gitlab::Auth::IpBlocked end end @@ -1130,4 +1117,28 @@ RSpec.describe ApplicationController do end end end + + context 'when Gitlab::Git::ResourceExhaustedError exception is raised' do + before do + sign_in user + end + + controller(described_class) do + def index + raise Gitlab::Git::ResourceExhaustedError.new( + "Upstream Gitaly has been exhausted: maximum time in concurrency queue reached. Try again later", 50 + ) + end + end + + it 'returns a plaintext error response with 429 status' do + get :index + + expect(response).to have_gitlab_http_status(:too_many_requests) + expect(response.body).to include( + "Upstream Gitaly has been exhausted: maximum time in concurrency queue reached. Try again later" + ) + expect(response.headers['Retry-After']).to eq(50) + end + end end diff --git a/spec/controllers/concerns/analytics/cycle_analytics/value_stream_actions_spec.rb b/spec/controllers/concerns/analytics/cycle_analytics/value_stream_actions_spec.rb index 246119a8118..28a0ce437de 100644 --- a/spec/controllers/concerns/analytics/cycle_analytics/value_stream_actions_spec.rb +++ b/spec/controllers/concerns/analytics/cycle_analytics/value_stream_actions_spec.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true + require 'spec_helper' RSpec.describe Analytics::CycleAnalytics::ValueStreamActions, type: :controller, -feature_category: :planning_analytics do + feature_category: :team_planning do subject(:controller) do Class.new(ApplicationController) do include Analytics::CycleAnalytics::ValueStreamActions diff --git a/spec/controllers/concerns/confirm_email_warning_spec.rb b/spec/controllers/concerns/confirm_email_warning_spec.rb index b8a4b94aa66..7cfbd86cdcb 100644 --- a/spec/controllers/concerns/confirm_email_warning_spec.rb +++ b/spec/controllers/concerns/confirm_email_warning_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe ConfirmEmailWarning, feature_category: :system_access do before do - stub_feature_flags(soft_email_confirmation: true) + stub_application_setting_enum('email_confirmation_setting', 'soft') end controller(ApplicationController) do diff --git a/spec/controllers/concerns/content_security_policy_patch_spec.rb b/spec/controllers/concerns/content_security_policy_patch_spec.rb index 6322950977c..9b4ddb35993 100644 --- a/spec/controllers/concerns/content_security_policy_patch_spec.rb +++ b/spec/controllers/concerns/content_security_policy_patch_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" # Based on https://github.com/rails/rails/pull/45115/files#diff-35ef6d1bd8b8d3b037ec819a704cd78db55db916a57abfc2859882826fc679b6 -RSpec.describe ContentSecurityPolicyPatch, feature_category: :not_owned do +RSpec.describe ContentSecurityPolicyPatch, feature_category: :shared do include Rack::Test::Methods let(:routes) do diff --git a/spec/controllers/concerns/continue_params_spec.rb b/spec/controllers/concerns/continue_params_spec.rb index ba600b8156a..9ac7087430e 100644 --- a/spec/controllers/concerns/continue_params_spec.rb +++ b/spec/controllers/concerns/continue_params_spec.rb @@ -31,10 +31,7 @@ RSpec.describe ContinueParams do it 'cleans up any params that are not allowed' do allow(controller).to receive(:params) do - strong_continue_params(to: '/hello', - notice: 'world', - notice_now: '!', - something: 'else') + strong_continue_params(to: '/hello', notice: 'world', notice_now: '!', something: 'else') end expect(controller.continue_params.keys).to contain_exactly(*%w(to notice notice_now)) diff --git a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb index a58b83dc42c..fc8b1efd226 100644 --- a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb +++ b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb @@ -25,9 +25,7 @@ RSpec.describe ControllerWithCrossProjectAccessCheck do # `described_class` is not available in this context include ControllerWithCrossProjectAccessCheck - requires_cross_project_access :index, show: false, - unless: -> { unless_condition }, - if: -> { if_condition } + requires_cross_project_access :index, show: false, unless: -> { unless_condition }, if: -> { if_condition } def index head :ok @@ -86,9 +84,10 @@ RSpec.describe ControllerWithCrossProjectAccessCheck do requires_cross_project_access - skip_cross_project_access_check index: true, show: false, - unless: -> { unless_condition }, - if: -> { if_condition } + skip_cross_project_access_check index: true, + show: false, + unless: -> { unless_condition }, + if: -> { if_condition } def index head :ok diff --git a/spec/controllers/concerns/kas_cookie_spec.rb b/spec/controllers/concerns/kas_cookie_spec.rb new file mode 100644 index 00000000000..d80df106cfd --- /dev/null +++ b/spec/controllers/concerns/kas_cookie_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe KasCookie, feature_category: :deployment_management do + describe '#set_kas_cookie' do + controller(ApplicationController) do + include KasCookie + + def index + set_kas_cookie + + render json: {}, status: :ok + end + end + + before do + allow(::Gitlab::Kas).to receive(:enabled?).and_return(true) + end + + subject(:kas_cookie) do + get :index + + request.env['action_dispatch.cookies'][Gitlab::Kas::COOKIE_KEY] + end + + context 'when user is signed out' do + it { is_expected.to be_blank } + end + + context 'when user is signed in' do + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'sets the KAS cookie', :aggregate_failures do + allow(::Gitlab::Kas::UserAccess).to receive(:cookie_data).and_return('foobar') + + expect(kas_cookie).to be_present + expect(kas_cookie).to eq('foobar') + expect(::Gitlab::Kas::UserAccess).to have_received(:cookie_data) + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(kas_user_access: false) + end + + it { is_expected.to be_blank } + end + end + end + + describe '#content_security_policy' do + let_it_be(:user) { create(:user) } + + controller(ApplicationController) do + include KasCookie + + def index + render json: {}, status: :ok + end + end + + before do + stub_config_setting(host: 'gitlab.example.com') + sign_in(user) + allow(::Gitlab::Kas).to receive(:enabled?).and_return(true) + allow(::Gitlab::Kas).to receive(:tunnel_url).and_return(kas_tunnel_url) + end + + subject(:kas_csp_connect_src) do + get :index + + request.env['action_dispatch.content_security_policy'].directives['connect-src'] + end + + context "when feature flag is disabled" do + let_it_be(:kas_tunnel_url) { 'ws://gitlab.example.com/-/k8s-proxy/' } + + before do + stub_feature_flags(kas_user_access: false) + end + + it 'does not add KAS url to connect-src directives' do + expect(kas_csp_connect_src).not_to include(::Gitlab::Kas.tunnel_url) + end + end + + context 'when feature flag is enabled' do + before do + stub_feature_flags(kas_user_access: true) + end + + context 'when KAS is on same domain as rails' do + let_it_be(:kas_tunnel_url) { 'ws://gitlab.example.com/-/k8s-proxy/' } + + it 'does not add KAS url to CSP connect-src directive' do + expect(kas_csp_connect_src).not_to include(::Gitlab::Kas.tunnel_url) + end + end + + context 'when KAS is on subdomain' do + let_it_be(:kas_tunnel_url) { 'ws://kas.gitlab.example.com/k8s-proxy/' } + + it 'adds KAS url to CSP connect-src directive' do + expect(kas_csp_connect_src).to include(::Gitlab::Kas.tunnel_url) + end + end + + context 'when KAS tunnel url is configured without trailing slash' do + let_it_be(:kas_tunnel_url) { 'ws://kas.gitlab.example.com/k8s-proxy' } + + it 'adds KAS url to CSP connect-src directive with trailing slash' do + expect(kas_csp_connect_src).to include("#{::Gitlab::Kas.tunnel_url}/") + end + end + end + end +end diff --git a/spec/controllers/concerns/metrics_dashboard_spec.rb b/spec/controllers/concerns/metrics_dashboard_spec.rb index 83546403ce5..d68a9d70ec6 100644 --- a/spec/controllers/concerns/metrics_dashboard_spec.rb +++ b/spec/controllers/concerns/metrics_dashboard_spec.rb @@ -113,7 +113,7 @@ RSpec.describe MetricsDashboard do it 'includes project_blob_path only for project dashboards' do expect(system_dashboard['project_blob_path']).to be_nil - expect(project_dashboard['project_blob_path']).to eq("/#{project.namespace.path}/#{project.name}/-/blob/master/.gitlab/dashboards/test.yml") + expect(project_dashboard['project_blob_path']).to eq("/#{project.namespace.path}/#{project.path}/-/blob/master/.gitlab/dashboards/test.yml") end it 'allows editing only for project dashboards' do diff --git a/spec/controllers/concerns/product_analytics_tracking_spec.rb b/spec/controllers/concerns/product_analytics_tracking_spec.rb index 12b4065b89c..65c2c77c027 100644 --- a/spec/controllers/concerns/product_analytics_tracking_spec.rb +++ b/spec/controllers/concerns/product_analytics_tracking_spec.rb @@ -8,7 +8,11 @@ RSpec.describe ProductAnalyticsTracking, :snowplow, feature_category: :product_a let(:user) { create(:user) } let(:event_name) { 'an_event' } + let(:event_action) { 'an_action' } + let(:event_label) { 'a_label' } + let!(:group) { create(:group) } + let_it_be(:project) { create(:project) } before do allow(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) @@ -19,8 +23,15 @@ RSpec.describe ProductAnalyticsTracking, :snowplow, feature_category: :product_a include ProductAnalyticsTracking skip_before_action :authenticate_user!, only: :show - track_event(:index, :show, name: 'an_event', destinations: [:redis_hll, :snowplow], - conditions: [:custom_condition_one?, :custom_condition_two?]) { |controller| controller.get_custom_id } + track_event( + :index, + :show, + name: 'an_event', + action: 'an_action', + label: 'a_label', + destinations: [:redis_hll, :snowplow], + conditions: [:custom_condition_one?, :custom_condition_two?] + ) { |controller| controller.get_custom_id } def index render html: 'index' @@ -44,6 +55,10 @@ RSpec.describe ProductAnalyticsTracking, :snowplow, feature_category: :product_a Group.first end + def tracking_project_source + Project.first + end + def custom_condition_one? true end @@ -64,7 +79,10 @@ RSpec.describe ProductAnalyticsTracking, :snowplow, feature_category: :product_a expect_snowplow_event( category: anything, - action: event_name, + action: event_action, + property: event_name, + label: event_label, + project: project, namespace: group, user: user, context: [context] @@ -89,20 +107,6 @@ RSpec.describe ProductAnalyticsTracking, :snowplow, feature_category: :product_a expect_snowplow_tracking(user) end - context 'when FF is disabled' do - before do - stub_const("#{described_class}::MIGRATED_EVENTS", []) - allow(Feature).to receive(:enabled?).and_call_original - allow(Feature).to receive(:enabled?).with('route_hll_to_snowplow', anything).and_return(false) - end - - it 'doesnt track snowplow event' do - get :index - - expect_no_snowplow_event - end - end - it 'tracks the event if DNT is not enabled' do stub_do_not_track('0') diff --git a/spec/controllers/concerns/redis_tracking_spec.rb b/spec/controllers/concerns/redis_tracking_spec.rb deleted file mode 100644 index 0ad8fa79e5e..00000000000 --- a/spec/controllers/concerns/redis_tracking_spec.rb +++ /dev/null @@ -1,135 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe RedisTracking do - include TrackingHelpers - - let(:user) { create(:user) } - - controller(ApplicationController) do - include RedisTracking - - skip_before_action :authenticate_user!, only: :show - track_redis_hll_event(:index, :show, - name: 'g_compliance_approval_rules', - if: [:custom_condition_one?, :custom_condition_two?]) { |controller| controller.get_custom_id } - - def index - render html: 'index' - end - - def new - render html: 'new' - end - - def show - render html: 'show' - end - - def get_custom_id - 'some_custom_id' - end - - private - - def custom_condition_one? - true - end - - def custom_condition_two? - true - end - end - - def expect_tracking - expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) - .with('g_compliance_approval_rules', values: instance_of(String)) - end - - def expect_no_tracking - expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) - end - - context 'when user is logged in' do - before do - sign_in(user) - end - - it 'tracks the event' do - expect_tracking - - get :index - end - - it 'tracks the event if DNT is not enabled' do - stub_do_not_track('0') - - expect_tracking - - get :index - end - - it 'does not track the event if DNT is enabled' do - stub_do_not_track('1') - - expect_no_tracking - - get :index - end - - it 'does not track the event if the format is not HTML' do - expect_no_tracking - - get :index, format: :json - end - - it 'does not track the event if a custom condition returns false' do - expect(controller).to receive(:custom_condition_two?).and_return(false) - - expect_no_tracking - - get :index - end - - it 'does not track the event for untracked actions' do - expect_no_tracking - - get :new - end - end - - context 'when user is not logged in' do - let(:visitor_id) { SecureRandom.uuid } - - it 'tracks the event when there is a visitor id' do - cookies[:visitor_id] = { value: visitor_id, expires: 24.months } - - expect_tracking - - get :show, params: { id: 1 } - end - end - - context 'when user is not logged in and there is no visitor_id' do - it 'does not track the event' do - expect_no_tracking - - get :index - end - - it 'tracks the event when there is custom id' do - expect_tracking - - get :show, params: { id: 1 } - end - - it 'does not track the event when there is no custom id' do - expect(controller).to receive(:get_custom_id).and_return(nil) - - expect_no_tracking - - get :show, params: { id: 2 } - end - end -end diff --git a/spec/controllers/concerns/renders_commits_spec.rb b/spec/controllers/concerns/renders_commits_spec.rb index 6a504681527..45f194b63e7 100644 --- a/spec/controllers/concerns/renders_commits_spec.rb +++ b/spec/controllers/concerns/renders_commits_spec.rb @@ -15,7 +15,7 @@ RSpec.describe RendersCommits do @merge_request = MergeRequest.find(params[:id]) @commits = set_commits_for_rendering( @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch), - commits_count: @merge_request.commits_count + commits_count: @merge_request.commits_count ) render json: { html: view_to_html_string('projects/merge_requests/_commits') } diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 6acbff6e745..bf6b68df54e 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -2,12 +2,12 @@ require 'spec_helper' -RSpec.describe SendFileUpload do +RSpec.describe SendFileUpload, feature_category: :user_profile do let(:uploader_class) do Class.new(GitlabUploader) do include ObjectStorage::Concern - storage_options Gitlab.config.uploads + storage_location :uploads private @@ -77,26 +77,6 @@ RSpec.describe SendFileUpload do allow(uploader).to receive(:model).and_return(image_owner) end - it_behaves_like 'handles image resize requests allowed by FF' - - context 'when FF is disabled' do - before do - stub_feature_flags(dynamic_image_resizing: false) - end - - it_behaves_like 'bypasses image resize requests not allowed by FF' - end - end - - shared_examples 'bypasses image resize requests not allowed by FF' do - it 'does not write workhorse command header' do - expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) - - subject - end - end - - shared_examples 'handles image resize requests allowed by FF' do context 'with valid width parameter' do it 'renders OK with workhorse command header' do expect(controller).not_to receive(:send_file) diff --git a/spec/controllers/concerns/sorting_preference_spec.rb b/spec/controllers/concerns/sorting_preference_spec.rb index 82a920215ca..6880d83142d 100644 --- a/spec/controllers/concerns/sorting_preference_spec.rb +++ b/spec/controllers/concerns/sorting_preference_spec.rb @@ -26,11 +26,14 @@ RSpec.describe SortingPreference do describe '#set_sort_order' do let(:group) { build(:group) } + let(:controller_name) { 'issues' } + let(:action_name) { 'issues' } let(:issue_weights_available) { true } before do allow(controller).to receive(:default_sort_order).and_return('updated_desc') - allow(controller).to receive(:action_name).and_return('issues') + allow(controller).to receive(:controller_name).and_return(controller_name) + allow(controller).to receive(:action_name).and_return(action_name) allow(controller).to receive(:can_sort_by_issue_weight?).and_return(issue_weights_available) user.user_preference.update!(issues_sort: sorting_field) end @@ -62,6 +65,42 @@ RSpec.describe SortingPreference do end end end + + context 'when user preference contains merged date sorting' do + let(:sorting_field) { 'merged_at_desc' } + let(:can_sort_by_merged_date?) { false } + + before do + allow(controller) + .to receive(:can_sort_by_merged_date?) + .with(can_sort_by_merged_date?) + .and_return(can_sort_by_merged_date?) + end + + it 'sets default sort order' do + is_expected.to eq('updated_desc') + end + + shared_examples 'user can sort by merged date' do + it 'sets sort order from user_preference' do + is_expected.to eq('merged_at_desc') + end + end + + context 'when controller_name is merge_requests' do + let(:controller_name) { 'merge_requests' } + let(:can_sort_by_merged_date?) { true } + + it_behaves_like 'user can sort by merged date' + end + + context 'when action_name is merge_requests' do + let(:action_name) { 'merge_requests' } + let(:can_sort_by_merged_date?) { true } + + it_behaves_like 'user can sort by merged date' + end + end end describe '#set_sort_order_from_user_preference' do diff --git a/spec/controllers/confirmations_controller_spec.rb b/spec/controllers/confirmations_controller_spec.rb index 773a416dcb4..fea43894f1c 100644 --- a/spec/controllers/confirmations_controller_spec.rb +++ b/spec/controllers/confirmations_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ConfirmationsController do +RSpec.describe ConfirmationsController, feature_category: :system_access do include DeviseHelpers before do @@ -58,8 +58,7 @@ RSpec.describe ConfirmationsController do m.call(*args) expect(Gitlab::ApplicationContext.current) - .to include('meta.user' => user.username, - 'meta.caller_id' => 'ConfirmationsController#show') + .to include('meta.user' => user.username, 'meta.caller_id' => 'ConfirmationsController#show') end perform_request @@ -94,8 +93,7 @@ RSpec.describe ConfirmationsController do m.call(*args) expect(Gitlab::ApplicationContext.current) - .to include('meta.user' => user.username, - 'meta.caller_id' => 'ConfirmationsController#show') + .to include('meta.user' => user.username, 'meta.caller_id' => 'ConfirmationsController#show') end travel_to(3.days.from_now) { perform_request } @@ -150,51 +148,67 @@ RSpec.describe ConfirmationsController do end end - context 'when reCAPTCHA is disabled' do + context "when `email_confirmation_setting` is set to `soft`" do before do - stub_application_setting(recaptcha_enabled: false) + stub_application_setting_enum('email_confirmation_setting', 'soft') end - it 'successfully sends password reset when reCAPTCHA is not solved' do - perform_request + context 'when reCAPTCHA is disabled' do + before do + stub_application_setting(recaptcha_enabled: false) + end - expect(response).to redirect_to(dashboard_projects_path) - end - end + it 'successfully sends password reset when reCAPTCHA is not solved' do + perform_request - context 'when reCAPTCHA is enabled' do - before do - stub_application_setting(recaptcha_enabled: true) + expect(response).to redirect_to(dashboard_projects_path) + end end - context 'when the reCAPTCHA is not solved' do + context 'when reCAPTCHA is enabled' do before do - Recaptcha.configuration.skip_verify_env.delete('test') + stub_application_setting(recaptcha_enabled: true) end - it 'displays an error' do - perform_request + context 'when the reCAPTCHA is not solved' do + before do + Recaptcha.configuration.skip_verify_env.delete('test') + end + + it 'displays an error' do + alert_text = _('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.') + + perform_request + + expect(response).to render_template(:new) + expect(flash[:alert]).to include alert_text + end - expect(response).to render_template(:new) - expect(flash[:alert]).to include _('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.') + it 'sets gon variables' do + Gon.clear + + perform_request + + expect(response).to render_template(:new) + expect(Gon.all_variables).not_to be_empty + end end - it 'sets gon variables' do - Gon.clear + it 'successfully sends password reset when reCAPTCHA is solved' do + Recaptcha.configuration.skip_verify_env << 'test' perform_request - expect(response).to render_template(:new) - expect(Gon.all_variables).not_to be_empty + expect(response).to redirect_to(dashboard_projects_path) end end + end - it 'successfully sends password reset when reCAPTCHA is solved' do - Recaptcha.configuration.skip_verify_env << 'test' - + context "when `email_confirmation_setting` is not set to `soft`" do + it 'redirects to the users_almost_there path' do perform_request - expect(response).to redirect_to(dashboard_projects_path) + expect(response).to redirect_to(users_almost_there_path) end end end diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index e8ee146a13a..893546def5a 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -17,6 +17,7 @@ RSpec.describe Dashboard::ProjectsController, :aggregate_failures, feature_categ before_all do project.add_developer(user) project2.add_developer(user) + user.toggle_star(project2) end before do @@ -39,6 +40,21 @@ RSpec.describe Dashboard::ProjectsController, :aggregate_failures, feature_categ expect(assigns(:projects)).to eq(projects) end + it 'assigns the correct all_user_projects' do + get :index + all_user_projects = assigns(:all_user_projects) + + expect(all_user_projects.count).to eq(2) + end + + it 'assigns the correct all_starred_projects' do + get :index + all_starred_projects = assigns(:all_starred_projects) + + expect(all_starred_projects.count).to eq(1) + expect(all_starred_projects).to include(project2) + end + context 'project sorting' do it_behaves_like 'set sort order from user preference' do let(:sorting_param) { 'created_asc' } @@ -62,6 +78,36 @@ RSpec.describe Dashboard::ProjectsController, :aggregate_failures, feature_categ end end + context 'with archived project' do + let_it_be(:archived_project) do + project2.tap { |p| p.update!(archived: true) } + end + + it 'does not display archived project' do + get :index + projects_result = assigns(:projects) + + expect(projects_result).not_to include(archived_project) + expect(projects_result).to include(project) + end + + it 'excludes archived project from all_user_projects' do + get :index + all_user_projects = assigns(:all_user_projects) + + expect(all_user_projects.count).to eq(1) + expect(all_user_projects).not_to include(archived_project) + end + + it 'excludes archived project from all_starred_projects' do + get :index + all_starred_projects = assigns(:all_starred_projects) + + expect(all_starred_projects.count).to eq(0) + expect(all_starred_projects).not_to include(archived_project) + end + end + context 'with deleted project' do let!(:pending_delete_project) do project.tap { |p| p.update!(pending_delete: true) } diff --git a/spec/controllers/every_controller_spec.rb b/spec/controllers/every_controller_spec.rb index 902872b6e92..b76da85ad72 100644 --- a/spec/controllers/every_controller_spec.rb +++ b/spec/controllers/every_controller_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' + RSpec.describe "Every controller" do context "feature categories" do let_it_be(:feature_categories) do @@ -52,7 +53,7 @@ RSpec.describe "Every controller" do non_existing_used_actions = used_actions - existing_actions expect(non_existing_used_actions).to be_empty, - "#{controller} used #{non_existing_used_actions} to define feature category, but the route does not exist" + "#{controller} used #{non_existing_used_actions} to define feature category, but the route does not exist" end end end diff --git a/spec/controllers/explore/groups_controller_spec.rb b/spec/controllers/explore/groups_controller_spec.rb index a3bd8102462..76bd94fd681 100644 --- a/spec/controllers/explore/groups_controller_spec.rb +++ b/spec/controllers/explore/groups_controller_spec.rb @@ -41,9 +41,9 @@ RSpec.describe Explore::GroupsController do it_behaves_like 'explore groups' - context 'generic_explore_groups flag is disabled' do + context 'gitlab.com' do before do - stub_feature_flags(generic_explore_groups: false) + allow(Gitlab).to receive(:com?).and_return(true) end it_behaves_like 'explore groups' diff --git a/spec/controllers/explore/projects_controller_spec.rb b/spec/controllers/explore/projects_controller_spec.rb index c4f0feb21e2..c2bdb0171e7 100644 --- a/spec/controllers/explore/projects_controller_spec.rb +++ b/spec/controllers/explore/projects_controller_spec.rb @@ -239,9 +239,14 @@ RSpec.describe Explore::ProjectsController, feature_category: :projects do context 'when user is signed in' do let(:user) { create(:user) } + let_it_be(:project) { create(:project, name: 'Project 1') } + let_it_be(:project2) { create(:project, name: 'Project 2') } before do sign_in(user) + project.add_developer(user) + project2.add_developer(user) + user.toggle_star(project2) end include_examples 'explore projects' @@ -260,6 +265,21 @@ RSpec.describe Explore::ProjectsController, feature_category: :projects do let(:controller_action) { :index } let(:params_with_name) { { name: 'some project' } } + it 'assigns the correct all_user_projects' do + get :index + all_user_projects = assigns(:all_user_projects) + + expect(all_user_projects.count).to eq(2) + end + + it 'assigns the correct all_starred_projects' do + get :index + all_starred_projects = assigns(:all_starred_projects) + + expect(all_starred_projects.count).to eq(1) + expect(all_starred_projects).to include(project2) + end + context 'when disable_anonymous_project_search is enabled' do before do stub_feature_flags(disable_anonymous_project_search: true) diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index 7aad67b01e8..92b228b6836 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -43,8 +43,9 @@ RSpec.describe GraphqlController, feature_category: :integrations do post :execute expect(json_response).to include( - 'errors' => include(a_hash_including('message' => /Internal server error/, - 'raisedAt' => /graphql_controller_spec.rb/)) + 'errors' => include( + a_hash_including('message' => /Internal server error/, 'raisedAt' => /graphql_controller_spec.rb/) + ) ) end @@ -64,6 +65,22 @@ RSpec.describe GraphqlController, feature_category: :integrations do ) expect(response).to have_gitlab_http_status(:forbidden) end + + it 'handles Gitlab::Git::ResourceExhaustedError', :aggregate_failures do + allow(controller).to receive(:execute) do + raise Gitlab::Git::ResourceExhaustedError.new("Upstream Gitaly has been exhausted. Try again later", 50) + end + + post :execute + + expect(json_response).to include( + 'errors' => include( + a_hash_including('message' => 'Upstream Gitaly has been exhausted. Try again later') + ) + ) + expect(response).to have_gitlab_http_status(:too_many_requests) + expect(response.headers['Retry-After']).to be(50) + end end describe 'POST #execute' do @@ -108,6 +125,41 @@ RSpec.describe GraphqlController, feature_category: :integrations do ]) end + it 'executes a multiplexed queries with variables with no errors' do + query = <<~GQL + mutation($a: String!, $b: String!) { + echoCreate(input: { messages: [$a, $b] }) { echoes } + } + GQL + multiplex = [ + { query: query, variables: { a: 'A', b: 'B' } }, + { query: query, variables: { a: 'a', b: 'b' } } + ] + + post :execute, params: { _json: multiplex } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq( + [ + { 'data' => { 'echoCreate' => { 'echoes' => %w[A B] } } }, + { 'data' => { 'echoCreate' => { 'echoes' => %w[a b] } } } + ]) + end + + it 'does not allow string as _json parameter (a malformed multiplex query)' do + post :execute, params: { _json: 'bad' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ + "errors" => [ + { + "message" => "Unexpected end of document", + "locations" => [] + } + ] + }) + end + it 'sets a limit on the total query size' do graphql_query = "{#{(['__typename'] * 1000).join(' ')}}" @@ -172,14 +224,28 @@ RSpec.describe GraphqlController, feature_category: :integrations do post :execute end - it 'calls the track gitlab cli when trackable method' do - agent = 'GLab - GitLab CLI' - request.env['HTTP_USER_AGENT'] = agent + context 'if using the GitLab CLI' do + it 'call trackable for the old UserAgent' do + agent = 'GLab - GitLab CLI' - expect(Gitlab::UsageDataCounters::GitLabCliActivityUniqueCounter) - .to receive(:track_api_request_when_trackable).with(user_agent: agent, user: user) + request.env['HTTP_USER_AGENT'] = agent - post :execute + expect(Gitlab::UsageDataCounters::GitLabCliActivityUniqueCounter) + .to receive(:track_api_request_when_trackable).with(user_agent: agent, user: user) + + post :execute + end + + it 'call trackable for the current UserAgent' do + agent = 'glab/v1.25.3-27-g7ec258fb (built 2023-02-16), darwin' + + request.env['HTTP_USER_AGENT'] = agent + + expect(Gitlab::UsageDataCounters::GitLabCliActivityUniqueCounter) + .to receive(:track_api_request_when_trackable).with(user_agent: agent, user: user) + + post :execute + end end it "assigns username in ApplicationContext" do diff --git a/spec/controllers/groups/children_controller_spec.rb b/spec/controllers/groups/children_controller_spec.rb index d0656ee47ce..2e37ed95c1c 100644 --- a/spec/controllers/groups/children_controller_spec.rb +++ b/spec/controllers/groups/children_controller_spec.rb @@ -275,6 +275,18 @@ RSpec.describe Groups::ChildrenController, feature_category: :subgroups do allow(Kaminari.config).to receive(:default_per_page).and_return(per_page) end + it 'rejects negative per_page parameter' do + get :index, params: { group_id: group.to_param, per_page: -1 }, format: :json + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'rejects non-numeric per_page parameter' do + get :index, params: { group_id: group.to_param, per_page: 'abc' }, format: :json + + expect(response).to have_gitlab_http_status(:bad_request) + end + context 'with only projects' do let!(:other_project) { create(:project, :public, namespace: group) } let!(:first_page_projects) { create_list(:project, per_page, :public, namespace: group) } diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 01ea7101f2e..f36494c3d78 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Groups::ClustersController, feature_category: :kubernetes_management do +RSpec.describe Groups::ClustersController, feature_category: :deployment_management do include AccessMatchersForController include GoogleApi::CloudPlatformHelpers @@ -322,12 +322,6 @@ RSpec.describe Groups::ClustersController, feature_category: :kubernetes_managem expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('cluster_status') end - - it 'invokes schedule_status_update on each application' do - expect_any_instance_of(Clusters::Applications::Ingress).to receive(:schedule_status_update) - - go - end end describe 'security' do @@ -360,20 +354,37 @@ RSpec.describe Groups::ClustersController, feature_category: :kubernetes_managem end describe 'functionality' do - render_views + context 'when remove_monitor_metrics FF is disabled' do + before do + stub_feature_flags(remove_monitor_metrics: false) + end - it 'renders view' do - go + render_views - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:cluster)).to eq(cluster) + it 'renders view' do + go + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:cluster)).to eq(cluster) + end + + it 'renders integration tab view', :aggregate_failures do + go(tab: 'integrations') + + expect(response).to render_template('clusters/clusters/_integrations') + expect(response).to have_gitlab_http_status(:ok) + end end - it 'renders integration tab view', :aggregate_failures do - go(tab: 'integrations') + context 'when remove_monitor_metrics FF is enabled' do + render_views - expect(response).to render_template('clusters/clusters/_integrations') - expect(response).to have_gitlab_http_status(:ok) + it 'renders details tab view', :aggregate_failures do + go(tab: 'integrations') + + expect(response).to render_template('clusters/clusters/_details') + expect(response).to have_gitlab_http_status(:ok) + end end end diff --git a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb index f1ca9e11a1a..a59c90a3cf2 100644 --- a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb +++ b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb @@ -249,7 +249,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do expect(send_data_type).to eq('send-dependency') expect(header).to eq( "Authorization" => ["Bearer abcd1234"], - "Accept" => ::ContainerRegistry::Client::ACCEPTED_TYPES + "Accept" => ::DependencyProxy::Manifest::ACCEPTED_TYPES ) expect(url).to eq(DependencyProxy::Registry.manifest_url(image, tag)) expect(response.headers['Content-Type']).to eq('application/gzip') diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 4e5dc01f466..fe4b80e12fe 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -55,6 +55,20 @@ RSpec.describe Groups::GroupMembersController do expect(assigns(:invited_members).count).to eq(1) end + + context 'when filtering by user type' do + let_it_be(:service_account) { create(:user, :service_account) } + + before do + group.add_developer(service_account) + end + + it 'returns only service accounts' do + get :index, params: { group_id: group, user_type: 'service_account' } + + expect(assigns(:members).map(&:user_id)).to match_array([service_account.id]) + end + end end context 'when user cannot manage members' do @@ -67,6 +81,21 @@ RSpec.describe Groups::GroupMembersController do expect(assigns(:invited_members)).to be_nil end + + context 'when filtering by user type' do + let_it_be(:service_account) { create(:user, :service_account) } + + before do + group.add_developer(user) + group.add_developer(service_account) + end + + it 'returns only service accounts' do + get :index, params: { group_id: group, user_type: 'service_account' } + + expect(assigns(:members).map(&:user_id)).to match_array([user.id, service_account.id]) + end + end end context 'when user has owner access to subgroup' do @@ -489,13 +518,11 @@ RSpec.describe Groups::GroupMembersController do describe 'PUT #update' do it 'is successful' do - put :update, - params: { - group_member: { access_level: Gitlab::Access::GUEST }, - group_id: group, - id: membership - }, - format: :json + put :update, params: { + group_member: { access_level: Gitlab::Access::GUEST }, + group_id: group, + id: membership + }, format: :json expect(response).to have_gitlab_http_status(:ok) end @@ -505,7 +532,7 @@ RSpec.describe Groups::GroupMembersController do it 'is successful' do delete :destroy, params: { group_id: group, id: membership } - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:see_other) end end diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index a3c4c47ab15..87030448b30 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -230,11 +230,10 @@ RSpec.describe Groups::MilestonesController do describe "#create" do it "creates group milestone with Chinese title" do - post :create, - params: { - group_id: group.to_param, - milestone: milestone_params - } + post :create, params: { + group_id: group.to_param, + milestone: milestone_params + } milestone = Milestone.find_by_title(title) @@ -251,17 +250,31 @@ RSpec.describe Groups::MilestonesController do it "updates group milestone" do milestone_params[:title] = "title changed" - put :update, - params: { - id: milestone.iid, - group_id: group.to_param, - milestone: milestone_params - } + put :update, params: { + id: milestone.iid, + group_id: group.to_param, + milestone: milestone_params + } milestone.reload expect(response).to redirect_to(group_milestone_path(group, milestone.iid)) expect(milestone.title).to eq("title changed") end + + it "handles ActiveRecord::StaleObjectError" do + milestone_params[:title] = "title changed" + # Purposely reduce the lock_version to trigger an ActiveRecord::StaleObjectError + milestone_params[:lock_version] = milestone.lock_version - 1 + + put :update, params: { + id: milestone.iid, + group_id: group.to_param, + milestone: milestone_params + } + + expect(response).not_to redirect_to(group_milestone_path(group, milestone.iid)) + expect(response).to render_template(:edit) + end end describe "#destroy" do @@ -390,21 +403,19 @@ RSpec.describe Groups::MilestonesController do context 'for a non-GET request' do context 'when requesting the canonical path with different casing' do it 'does not 404' do - post :create, - params: { - group_id: group.to_param, - milestone: { title: title } - } + post :create, params: { + group_id: group.to_param, + milestone: { title: title } + } expect(response).not_to have_gitlab_http_status(:not_found) end it 'does not redirect to the correct casing' do - post :create, - params: { - group_id: group.to_param, - milestone: { title: title } - } + post :create, params: { + group_id: group.to_param, + milestone: { title: title } + } expect(response).not_to have_gitlab_http_status(:moved_permanently) end @@ -414,11 +425,10 @@ RSpec.describe Groups::MilestonesController do let(:redirect_route) { group.redirect_routes.create!(path: 'old-path') } it 'returns not found' do - post :create, - params: { - group_id: redirect_route.path, - milestone: { title: title } - } + post :create, params: { + group_id: redirect_route.path, + milestone: { title: title } + } expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb index 1a60f7d824e..9ae5cb6f87c 100644 --- a/spec/controllers/groups/runners_controller_spec.rb +++ b/spec/controllers/groups/runners_controller_spec.rb @@ -6,8 +6,8 @@ RSpec.describe Groups::RunnersController, feature_category: :runner_fleet do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } + let_it_be(:runner) { create(:ci_runner, :group, groups: [group]) } - let!(:runner) { create(:ci_runner, :group, groups: [group]) } let!(:project_runner) { create(:ci_runner, :project, projects: [project]) } let!(:instance_runner) { create(:ci_runner, :instance) } @@ -37,6 +37,12 @@ RSpec.describe Groups::RunnersController, feature_category: :runner_fleet do expect_snowplow_event(category: described_class.name, action: 'index', user: user, namespace: group) end + + it 'assigns variables' do + get :index, params: { group_id: group } + + expect(assigns(:group_new_runner_path)).to eq(new_group_runner_path(group)) + end end context 'when user is not owner' do @@ -58,6 +64,130 @@ RSpec.describe Groups::RunnersController, feature_category: :runner_fleet do end end + describe '#new' do + context 'when create_runner_workflow_for_namespace is enabled' do + before do + stub_feature_flags(create_runner_workflow_for_namespace: [group]) + end + + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'renders new with 200 status code' do + get :new, params: { group_id: group } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:new) + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + get :new, params: { group_id: group } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when create_runner_workflow_for_namespace is disabled' do + before do + stub_feature_flags(create_runner_workflow_for_namespace: false) + end + + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'renders a 404' do + get :new, params: { group_id: group } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + describe '#register' do + subject(:register) { get :register, params: { group_id: group, id: new_runner } } + + context 'when create_runner_workflow_for_namespace is enabled' do + before do + stub_feature_flags(create_runner_workflow_for_namespace: [group]) + end + + context 'when user is owner' do + before do + group.add_owner(user) + end + + context 'when runner can be registered after creation' do + let_it_be(:new_runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } + + it 'renders a :register template' do + register + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:register) + end + end + + context 'when runner cannot be registered after creation' do + let_it_be(:new_runner) { runner } + + it 'returns :not_found' do + register + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + context 'when runner can be registered after creation' do + let_it_be(:new_runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } + + it 'returns :not_found' do + register + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + context 'when create_runner_workflow_for_namespace is disabled' do + let_it_be(:new_runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } + + before do + stub_feature_flags(create_runner_workflow_for_namespace: false) + end + + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'returns :not_found' do + register + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + describe '#show' do context 'when user is owner' do before do @@ -158,6 +288,8 @@ RSpec.describe Groups::RunnersController, feature_category: :runner_fleet do end describe '#update' do + let!(:runner) { create(:ci_runner, :group, groups: [group]) } + context 'when user is an owner' do before do group.add_owner(user) diff --git a/spec/controllers/groups/settings/applications_controller_spec.rb b/spec/controllers/groups/settings/applications_controller_spec.rb index b9457770ed6..c398fd044c2 100644 --- a/spec/controllers/groups/settings/applications_controller_spec.rb +++ b/spec/controllers/groups/settings/applications_controller_spec.rb @@ -71,43 +71,18 @@ RSpec.describe Groups::Settings::ApplicationsController do group.add_owner(user) end - context 'with hash_oauth_secrets flag on' do - before do - stub_feature_flags(hash_oauth_secrets: true) - end - - it 'creates the application' do - create_params = attributes_for(:application, trusted: false, confidential: false, scopes: ['api']) - - expect do - post :create, params: { group_id: group, doorkeeper_application: create_params } - end.to change { Doorkeeper::Application.count }.by(1) - - application = Doorkeeper::Application.last - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template :show - expect(application).to have_attributes(create_params.except(:uid, :owner_type)) - end - end + it 'creates the application' do + create_params = attributes_for(:application, trusted: false, confidential: false, scopes: ['api']) - context 'with hash_oauth_secrets flag off' do - before do - stub_feature_flags(hash_oauth_secrets: false) - end - - it 'creates the application' do - create_params = attributes_for(:application, trusted: false, confidential: false, scopes: ['api']) - - expect do - post :create, params: { group_id: group, doorkeeper_application: create_params } - end.to change { Doorkeeper::Application.count }.by(1) + expect do + post :create, params: { group_id: group, doorkeeper_application: create_params } + end.to change { Doorkeeper::Application.count }.by(1) - application = Doorkeeper::Application.last + application = Doorkeeper::Application.last - expect(response).to redirect_to(group_settings_application_path(group, application)) - expect(application).to have_attributes(create_params.except(:uid, :owner_type)) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show + expect(application).to have_attributes(create_params.except(:uid, :owner_type)) end it 'renders the application form on errors' do @@ -120,43 +95,18 @@ RSpec.describe Groups::Settings::ApplicationsController do end context 'when the params are for a confidential application' do - context 'with hash_oauth_secrets flag off' do - before do - stub_feature_flags(hash_oauth_secrets: false) - end - - it 'creates a confidential application' do - create_params = attributes_for(:application, confidential: true, scopes: ['read_user']) - - expect do - post :create, params: { group_id: group, doorkeeper_application: create_params } - end.to change { Doorkeeper::Application.count }.by(1) + it 'creates a confidential application' do + create_params = attributes_for(:application, confidential: true, scopes: ['read_user']) - application = Doorkeeper::Application.last - - expect(response).to redirect_to(group_settings_application_path(group, application)) - expect(application).to have_attributes(create_params.except(:uid, :owner_type)) - end - end - - context 'with hash_oauth_secrets flag on' do - before do - stub_feature_flags(hash_oauth_secrets: true) - end - - it 'creates a confidential application' do - create_params = attributes_for(:application, confidential: true, scopes: ['read_user']) - - expect do - post :create, params: { group_id: group, doorkeeper_application: create_params } - end.to change { Doorkeeper::Application.count }.by(1) + expect do + post :create, params: { group_id: group, doorkeeper_application: create_params } + end.to change { Doorkeeper::Application.count }.by(1) - application = Doorkeeper::Application.last + application = Doorkeeper::Application.last - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template :show - expect(application).to have_attributes(create_params.except(:uid, :owner_type)) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show + expect(application).to have_attributes(create_params.except(:uid, :owner_type)) end end @@ -188,6 +138,61 @@ RSpec.describe Groups::Settings::ApplicationsController do end end + describe 'PUT #renew' do + context 'when user is owner' do + before do + group.add_owner(user) + end + + let(:oauth_params) do + { + group_id: group, + id: application.id + } + end + + subject { put :renew, params: oauth_params } + + it { is_expected.to have_gitlab_http_status(:ok) } + it { expect { subject }.to change { application.reload.secret } } + + it 'returns the secret in json format' do + subject + + expect(json_response['secret']).not_to be_nil + end + + context 'when renew fails' do + before do + allow_next_found_instance_of(Doorkeeper::Application) do |application| + allow(application).to receive(:save).and_return(false) + end + end + + it { expect { subject }.not_to change { application.reload.secret } } + it { is_expected.to have_gitlab_http_status(:unprocessable_entity) } + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + let(:oauth_params) do + { + group_id: group, + id: application.id + } + end + + it 'renders a 404' do + put :renew, params: oauth_params + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'PATCH #update' do context 'when user is owner' do before do diff --git a/spec/controllers/groups/settings/integrations_controller_spec.rb b/spec/controllers/groups/settings/integrations_controller_spec.rb index 377c38ce087..3ae43c8ab7c 100644 --- a/spec/controllers/groups/settings/integrations_controller_spec.rb +++ b/spec/controllers/groups/settings/integrations_controller_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Groups::Settings::IntegrationsController do let_it_be(:group) { create(:group) } before do + stub_feature_flags(remove_monitor_metrics: false) sign_in(user) end diff --git a/spec/controllers/groups/variables_controller_spec.rb b/spec/controllers/groups/variables_controller_spec.rb index 6dbe75bb1df..8c6efae89c3 100644 --- a/spec/controllers/groups/variables_controller_spec.rb +++ b/spec/controllers/groups/variables_controller_spec.rb @@ -77,12 +77,10 @@ RSpec.describe Groups::VariablesController do describe 'PATCH #update' do it 'is successful' do - patch :update, - params: { - group_id: group, - variables_attributes: [{ id: variable.id, key: 'hello' }] - }, - format: :json + patch :update, params: { + group_id: group, + variables_attributes: [{ id: variable.id, key: 'hello' }] + }, format: :json expect(response).to have_gitlab_http_status(:ok) end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 9184cd2263e..8617cc8af8f 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -152,29 +152,6 @@ RSpec.describe GroupsController, factory_default: :keep, feature_category: :code end end end - - describe 'require_verification_for_namespace_creation experiment', :experiment do - before do - sign_in(owner) - stub_experiments(require_verification_for_namespace_creation: :candidate) - end - - it 'tracks a "start_create_group" event' do - expect(experiment(:require_verification_for_namespace_creation)).to track( - :start_create_group - ).on_next_instance.with_context(user: owner) - - get :new - end - - context 'when creating a sub-group' do - it 'does not track a "start_create_group" event' do - expect(experiment(:require_verification_for_namespace_creation)).not_to track(:start_create_group) - - get :new, params: { parent_id: group.id } - end - end - end end describe 'GET #activity' do diff --git a/spec/controllers/help_controller_spec.rb b/spec/controllers/help_controller_spec.rb index 2375146f346..056df213209 100644 --- a/spec/controllers/help_controller_spec.rb +++ b/spec/controllers/help_controller_spec.rb @@ -181,6 +181,7 @@ RSpec.describe HelpController do context 'when requested file exists' do before do stub_doc_file_read(file_name: 'user/ssh.md', content: fixture_file('blockquote_fence_after.md')) + stub_application_setting(help_page_documentation_base_url: '') subject end @@ -223,13 +224,13 @@ RSpec.describe HelpController do context 'when gitlab_docs is disabled' do let(:docs_enabled) { false } - it_behaves_like 'documentation pages local render' + it_behaves_like 'documentation pages redirect', 'https://docs.gitlab.com' end context 'when host is missing' do let(:host) { nil } - it_behaves_like 'documentation pages local render' + it_behaves_like 'documentation pages redirect', 'https://docs.gitlab.com' end end @@ -251,6 +252,10 @@ RSpec.describe HelpController do end context 'when requested file is missing' do + before do + stub_application_setting(help_page_documentation_base_url: '') + end + it 'renders not found' do get :show, params: { path: 'foo/bar' }, format: :md expect(response).to be_not_found @@ -261,11 +266,7 @@ RSpec.describe HelpController do context 'for image formats' do context 'when requested file exists' do it 'renders the raw file' do - get :show, - params: { - path: 'user/img/markdown_logo' - }, - format: :png + get :show, params: { path: 'user/img/markdown_logo' }, format: :png aggregate_failures do expect(response).to be_successful @@ -277,11 +278,7 @@ RSpec.describe HelpController do context 'when requested file is missing' do it 'renders not found' do - get :show, - params: { - path: 'foo/bar' - }, - format: :png + get :show, params: { path: 'foo/bar' }, format: :png expect(response).to be_not_found end end @@ -289,11 +286,7 @@ RSpec.describe HelpController do context 'for other formats' do it 'always renders not found' do - get :show, - params: { - path: 'user/ssh' - }, - format: :foo + get :show, params: { path: 'user/ssh' }, format: :foo expect(response).to be_not_found end end diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb index 35f712dc50d..906cc5cb336 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::BitbucketController do +RSpec.describe Import::BitbucketController, feature_category: :importers do include ImportSpecHelper let(:user) { create(:user) } @@ -48,11 +48,13 @@ RSpec.describe Import::BitbucketController do let(:expires_at) { Time.current + 1.day } let(:expires_in) { 1.day } let(:access_token) do - double(token: token, - secret: secret, - expires_at: expires_at, - expires_in: expires_in, - refresh_token: refresh_token) + double( + token: token, + secret: secret, + expires_at: expires_at, + expires_in: expires_in, + refresh_token: refresh_token + ) end before do @@ -63,10 +65,10 @@ RSpec.describe Import::BitbucketController do allow_any_instance_of(OAuth2::Client) .to receive(:get_token) .with(hash_including( - 'grant_type' => 'authorization_code', - 'code' => code, - 'redirect_uri' => users_import_bitbucket_callback_url), - {}) + 'grant_type' => 'authorization_code', + 'code' => code, + 'redirect_uri' => users_import_bitbucket_callback_url), + {}) .and_return(access_token) stub_omniauth_provider('bitbucket') @@ -443,5 +445,16 @@ RSpec.describe Import::BitbucketController do ) end end + + context 'when user can not import projects' do + let!(:other_namespace) { create(:group, name: 'other_namespace').tap { |other_namespace| other_namespace.add_developer(user) } } + + it 'returns 422 response' do + post :create, params: { target_namespace: other_namespace.name }, format: :json + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.parsed_body['errors']).to eq('You are not allowed to import projects in this namespace.') + end + end end end diff --git a/spec/controllers/import/bitbucket_server_controller_spec.rb b/spec/controllers/import/bitbucket_server_controller_spec.rb index ac56d3af54f..b2a56423253 100644 --- a/spec/controllers/import/bitbucket_server_controller_spec.rb +++ b/spec/controllers/import/bitbucket_server_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::BitbucketServerController do +RSpec.describe Import::BitbucketServerController, feature_category: :importers do let(:user) { create(:user) } let(:project_key) { 'test-project' } let(:repo_slug) { 'some-repo' } diff --git a/spec/controllers/import/bulk_imports_controller_spec.rb b/spec/controllers/import/bulk_imports_controller_spec.rb index a3992ae850e..c5e5aa03669 100644 --- a/spec/controllers/import/bulk_imports_controller_spec.rb +++ b/spec/controllers/import/bulk_imports_controller_spec.rb @@ -121,12 +121,12 @@ RSpec.describe Import::BulkImportsController, feature_category: :importers do params = { page: 1, per_page: 20, filter: '' }.merge(params_override) get :status, - params: params, - format: format, - session: { - bulk_import_gitlab_url: 'https://gitlab.example.com', - bulk_import_gitlab_access_token: 'demo-pat' - } + params: params, + format: format, + session: { + bulk_import_gitlab_url: 'https://gitlab.example.com', + bulk_import_gitlab_access_token: 'demo-pat' + } end include_context 'bulk imports requests context', 'https://gitlab.example.com' @@ -157,8 +157,7 @@ RSpec.describe Import::BulkImportsController, feature_category: :importers do end let(:source_version) do - Gitlab::VersionInfo.new(::BulkImport::MIN_MAJOR_VERSION, - ::BulkImport::MIN_MINOR_VERSION_FOR_PROJECT) + Gitlab::VersionInfo.new(::BulkImport::MIN_MAJOR_VERSION, ::BulkImport::MIN_MINOR_VERSION_FOR_PROJECT) end before do @@ -214,36 +213,41 @@ RSpec.describe Import::BulkImportsController, feature_category: :importers do end end - context 'when host url is local or not http' do - %w[https://localhost:3000 http://192.168.0.1 ftp://testing].each do |url| - before do - stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) - - session[:bulk_import_gitlab_access_token] = 'test' - session[:bulk_import_gitlab_url] = url - end + shared_examples 'unacceptable url' do |url, expected_error| + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) - it 'denies network request' do - get :status + session[:bulk_import_gitlab_access_token] = 'test' + session[:bulk_import_gitlab_url] = url + end - expect(controller).to redirect_to(new_group_path(anchor: 'import-group-pane')) - expect(flash[:alert]).to eq('Specified URL cannot be used: "Only allowed schemes are http, https"') - end + it 'denies network request' do + get :status + expect(controller).to redirect_to(new_group_path(anchor: 'import-group-pane')) + expect(flash[:alert]).to eq("Specified URL cannot be used: \"#{expected_error}\"") end + end + + context 'when host url is local or not http' do + include_examples 'unacceptable url', 'https://localhost:3000', "Only allowed schemes are http, https" + include_examples 'unacceptable url', 'http://192.168.0.1', "Only allowed schemes are http, https" + include_examples 'unacceptable url', 'ftp://testing', "Only allowed schemes are http, https" context 'when local requests are allowed' do %w[https://localhost:3000 http://192.168.0.1].each do |url| - before do - stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) + context "with #{url}" do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) - session[:bulk_import_gitlab_access_token] = 'test' - session[:bulk_import_gitlab_url] = url - end + session[:bulk_import_gitlab_access_token] = 'test' + session[:bulk_import_gitlab_url] = url + end - it 'allows network request' do - get :status + it 'allows network request' do + get :status - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:ok) + end end end end @@ -270,8 +274,7 @@ RSpec.describe Import::BulkImportsController, feature_category: :importers do context 'when connection error occurs' do let(:source_version) do - Gitlab::VersionInfo.new(::BulkImport::MIN_MAJOR_VERSION, - ::BulkImport::MIN_MINOR_VERSION_FOR_PROJECT) + Gitlab::VersionInfo.new(::BulkImport::MIN_MAJOR_VERSION, ::BulkImport::MIN_MINOR_VERSION_FOR_PROJECT) end before do diff --git a/spec/controllers/import/fogbugz_controller_spec.rb b/spec/controllers/import/fogbugz_controller_spec.rb index ed2a588eadf..3b099ba2613 100644 --- a/spec/controllers/import/fogbugz_controller_spec.rb +++ b/spec/controllers/import/fogbugz_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::FogbugzController do +RSpec.describe Import::FogbugzController, feature_category: :importers do include ImportSpecHelper let(:user) { create(:user) } @@ -11,6 +11,8 @@ RSpec.describe Import::FogbugzController do let(:namespace_id) { 5 } before do + stub_application_setting(import_sources: ['fogbugz']) + sign_in(user) end @@ -116,8 +118,7 @@ RSpec.describe Import::FogbugzController do describe 'GET status' do let(:repo) do - instance_double(Gitlab::FogbugzImport::Repository, - id: 'demo', name: 'vim', safe_name: 'vim', path: 'vim') + instance_double(Gitlab::FogbugzImport::Repository, id: 'demo', name: 'vim', safe_name: 'vim', path: 'vim') end it 'redirects to new page form when client is invalid' do diff --git a/spec/controllers/import/gitea_controller_spec.rb b/spec/controllers/import/gitea_controller_spec.rb index 568712d29cb..3dfda909a93 100644 --- a/spec/controllers/import/gitea_controller_spec.rb +++ b/spec/controllers/import/gitea_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::GiteaController do +RSpec.describe Import::GiteaController, feature_category: :importers do include ImportSpecHelper let(:provider) { :gitea } @@ -10,6 +10,10 @@ RSpec.describe Import::GiteaController do include_context 'a GitHub-ish import controller' + before do + stub_application_setting(import_sources: ['gitea']) + end + def assign_host_url session[:gitea_host_url] = host_url end @@ -42,19 +46,23 @@ RSpec.describe Import::GiteaController do expect(response).to have_gitlab_http_status(:ok) end - context 'when host url is local or not http' do - %w[https://localhost:3000 http://192.168.0.1 ftp://testing].each do |url| - let(:host_url) { url } + shared_examples "unacceptable url" do |url, expected_error| + let(:host_url) { url } - it 'denies network request' do - get :status, format: :json + it 'denies network request' do + get :status, format: :json - expect(controller).to redirect_to(new_import_url) - expect(flash[:alert]).to eq('Specified URL cannot be used: "Only allowed schemes are http, https"') - end + expect(controller).to redirect_to(new_import_url) + expect(flash[:alert]).to eq("Specified URL cannot be used: \"#{expected_error}\"") end end + context 'when host url is local or not http' do + include_examples 'unacceptable url', 'https://localhost:3000', 'Only allowed schemes are http, https' + include_examples 'unacceptable url', 'http://192.168.0.1', 'Only allowed schemes are http, https' + include_examples 'unacceptable url', 'ftp://testing', 'Only allowed schemes are http, https' + end + context 'when DNS Rebinding protection is enabled' do let(:token) { 'gitea token' } diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 406a3604b23..fdc0ddda9f4 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -139,7 +139,7 @@ RSpec.describe Import::GithubController, feature_category: :importers do expect_next_instance_of(Gitlab::GithubImport::Clients::Proxy) do |client| expect(client).to receive(:repos) .with(expected_filter, expected_options) - .and_return({ repos: [], page_info: {} }) + .and_return({ repos: [], page_info: {}, count: 0 }) end get :status, params: params, format: :json @@ -149,6 +149,7 @@ RSpec.describe Import::GithubController, feature_category: :importers do expect(json_response['provider_repos'].size).to eq 0 expect(json_response['incompatible_repos'].size).to eq 0 expect(json_response['page_info']).to eq({}) + expect(json_response['provider_repo_count']).to eq(0) end end @@ -161,9 +162,7 @@ RSpec.describe Import::GithubController, feature_category: :importers do let(:provider_repos) { [] } let(:expected_filter) { '' } let(:expected_options) do - pagination_params.merge(relation_params).merge( - first: 25, page: 1, per_page: 25 - ) + pagination_params.merge(relation_params).merge(first: 25) end before do @@ -171,6 +170,9 @@ RSpec.describe Import::GithubController, feature_category: :importers do if client_auth_success allow(proxy).to receive(:repos).and_return({ repos: provider_repos }) allow(proxy).to receive(:client).and_return(client_stub) + allow_next_instance_of(Gitlab::GithubImport::ProjectRelationType) do |instance| + allow(instance).to receive(:for).with('example/repo').and_return('owned') + end else allow(proxy).to receive(:repos).and_raise(Octokit::Unauthorized) end @@ -279,22 +281,12 @@ RSpec.describe Import::GithubController, feature_category: :importers do it_behaves_like 'calls repos through Clients::Proxy with expected args' end - - context 'when page is specified' do - let(:pagination_params) { { before: nil, after: nil, page: 2 } } - let(:params) { pagination_params } - let(:expected_options) do - pagination_params.merge(relation_params).merge(first: 25, page: 2, per_page: 25) - end - - it_behaves_like 'calls repos through Clients::Proxy with expected args' - end end context 'when relation type params present' do let(:organization_login) { 'test-login' } let(:params) { pagination_params.merge(relation_type: 'organization', organization_login: organization_login) } - let(:pagination_defaults) { { first: 25, page: 1, per_page: 25 } } + let(:pagination_defaults) { { first: 25 } } let(:expected_options) do pagination_defaults.merge(pagination_params).merge( relation_type: 'organization', organization_login: organization_login @@ -359,7 +351,13 @@ RSpec.describe Import::GithubController, feature_category: :importers do end end - describe "POST create" do + describe "POST create", :clean_gitlab_redis_cache do + before do + allow_next_instance_of(Gitlab::GithubImport::ProjectRelationType) do |instance| + allow(instance).to receive(:for).with("#{provider_username}/vim").and_return('owned') + end + end + it_behaves_like 'a GitHub-ish import controller: POST create' it_behaves_like 'project import rate limiter' @@ -387,14 +385,74 @@ RSpec.describe Import::GithubController, feature_category: :importers do end end + describe "GET failures" do + let_it_be_with_reload(:project) { create(:project, import_type: 'github', import_status: :started, import_source: 'example/repo', import_url: 'https://fake.url') } + let!(:import_failure) do + create(:import_failure, + project: project, + source: 'Gitlab::GithubImport::Importer::PullRequestImporter', + external_identifiers: { iid: 2, object_type: 'pull_request', title: 'My Pull Request' } + ) + end + + context 'when import is not finished' do + it 'return bad_request' do + get :failures, params: { project_id: project.id } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('The import is not complete.') + end + end + + context 'when import is finished' do + before do + project.import_state.finish + end + + it 'includes failure details in response' do + get :failures, params: { project_id: project.id } + + expect(json_response[0]['type']).to eq('pull_request') + expect(json_response[0]['title']).to eq('My Pull Request') + expect(json_response[0]['provider_url']).to eq("https://fake.url/example/repo/pull/2") + expect(json_response[0]['details']['source']).to eq(import_failure.source) + end + + it 'paginates records' do + issue_title = 'My Issue' + + create( + :import_failure, + project: project, + source: 'Gitlab::GithubImport::Importer::IssueAndLabelLinksImporter', + external_identifiers: { iid: 3, object_type: 'issue', title: issue_title } + ) + + get :failures, params: { project_id: project.id, page: 2, per_page: 1 } + + expect(json_response.size).to eq(1) + expect(json_response.first['title']).to eq(issue_title) + end + end + end + describe "POST cancel" do - let_it_be(:project) { create(:project, :import_started, import_type: 'github', import_url: 'https://fake.url') } + let_it_be(:project) do + create( + :project, :import_started, + import_type: 'github', import_url: 'https://fake.url', import_source: 'login/repo' + ) + end context 'when project import was canceled' do before do allow(Import::Github::CancelProjectImportService) .to receive(:new).with(project, user) .and_return(double(execute: { status: :success, project: project })) + + allow_next_instance_of(Gitlab::GithubImport::ProjectRelationType) do |instance| + allow(instance).to receive(:for).with('login/repo').and_return('owned') + end end it 'returns success' do @@ -471,4 +529,26 @@ RSpec.describe Import::GithubController, feature_category: :importers do end end end + + describe 'GET counts' do + let(:expected_result) do + { + 'owned' => 3, + 'collaborated' => 2, + 'organization' => 1 + } + end + + it 'returns repos count by type' do + expect_next_instance_of(Gitlab::GithubImport::Clients::Proxy) do |client_proxy| + expect(client_proxy).to receive(:count_repos_by).with('owned', user.id).and_return(3) + expect(client_proxy).to receive(:count_repos_by).with('collaborated', user.id).and_return(2) + expect(client_proxy).to receive(:count_repos_by).with('organization', user.id).and_return(1) + end + + get :counts + + expect(json_response).to eq(expected_result) + end + end end diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb deleted file mode 100644 index 7b3978297fb..00000000000 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ /dev/null @@ -1,313 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Import::GitlabController do - include ImportSpecHelper - - let(:user) { create(:user) } - let(:token) { "asdasd12345" } - let(:access_params) { { gitlab_access_token: token } } - - def assign_session_token - session[:gitlab_access_token] = token - end - - before do - sign_in(user) - allow(controller).to receive(:gitlab_import_enabled?).and_return(true) - end - - describe "GET callback" do - it "updates access token" do - allow_next_instance_of(Gitlab::GitlabImport::Client) do |instance| - allow(instance).to receive(:get_token).and_return(token) - end - stub_omniauth_provider('gitlab') - - get :callback - - expect(session[:gitlab_access_token]).to eq(token) - expect(controller).to redirect_to(status_import_gitlab_url) - end - - it "importable_repos should return an array" do - allow_next_instance_of(Gitlab::GitlabImport::Client) do |instance| - allow(instance).to receive(:projects).and_return([{ "id": 1 }].to_enum) - end - - expect(controller.send(:importable_repos)).to be_an_instance_of(Array) - end - - it "passes namespace_id query param to status if provided" do - namespace_id = 30 - - allow_next_instance_of(Gitlab::GitlabImport::Client) do |instance| - allow(instance).to receive(:get_token).and_return(token) - end - - get :callback, params: { namespace_id: namespace_id } - - expect(controller).to redirect_to(status_import_gitlab_url(namespace_id: namespace_id)) - end - end - - describe "GET status" do - let(:repo_fake) { Struct.new(:id, :path, :path_with_namespace, :web_url, keyword_init: true) } - let(:repo) { repo_fake.new(id: 1, path: 'vim', path_with_namespace: 'asd/vim', web_url: 'https://gitlab.com/asd/vim') } - - context 'when session contains access token' do - before do - assign_session_token - end - - it_behaves_like 'import controller status' do - let(:repo_id) { repo.id } - let(:import_source) { repo.path_with_namespace } - let(:provider_name) { 'gitlab' } - let(:client_repos_field) { :projects } - end - end - - it 'redirects to auth if session does not contain access token' do - remote_gitlab_url = 'https://test.host/auth/gitlab' - - allow(Gitlab::GitlabImport::Client) - .to receive(:new) - .and_return(double(authorize_url: remote_gitlab_url)) - - get :status - - expect(response).to redirect_to(remote_gitlab_url) - end - end - - describe "POST create" do - let(:project) { create(:project) } - let(:gitlab_username) { user.username } - let(:gitlab_user) do - { username: gitlab_username }.with_indifferent_access - end - - let(:gitlab_repo) do - { - path: 'vim', - path_with_namespace: "#{gitlab_username}/vim", - owner: { name: gitlab_username }, - namespace: { path: gitlab_username } - }.with_indifferent_access - end - - before do - stub_client(user: gitlab_user, project: gitlab_repo) - assign_session_token - end - - it 'returns 200 response when the project is imported successfully' do - allow(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, user.namespace, user, access_params) - .and_return(double(execute: project)) - - post :create, format: :json - - expect(response).to have_gitlab_http_status(:ok) - end - - it 'returns 422 response when the project could not be imported' do - allow(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, user.namespace, user, access_params) - .and_return(double(execute: build(:project))) - - post :create, format: :json - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - end - - context "when the repository owner is the GitLab.com user" do - context "when the GitLab.com user and GitLab server user's usernames match" do - it "takes the current user's namespace" do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, user.namespace, user, access_params) - .and_return(double(execute: project)) - - post :create, format: :json - end - end - - context "when the GitLab.com user and GitLab server user's usernames don't match" do - let(:gitlab_username) { "someone_else" } - - it "takes the current user's namespace" do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, user.namespace, user, access_params) - .and_return(double(execute: project)) - - post :create, format: :json - end - end - end - - context "when the repository owner is not the GitLab.com user" do - let(:other_username) { "someone_else" } - - before do - gitlab_repo["namespace"]["path"] = other_username - assign_session_token - end - - context "when a namespace with the GitLab.com user's username already exists" do - let!(:existing_namespace) { create(:group, name: other_username) } - - context "when the namespace is owned by the GitLab server user" do - before do - existing_namespace.add_owner(user) - end - - it "takes the existing namespace" do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, existing_namespace, user, access_params) - .and_return(double(execute: project)) - - post :create, format: :json - end - end - - context "when the namespace is not owned by the GitLab server user" do - it "doesn't create a project" do - expect(Gitlab::GitlabImport::ProjectCreator) - .not_to receive(:new) - - post :create, format: :json - end - end - end - - context "when a namespace with the GitLab.com user's username doesn't exist" do - context "when current user can create namespaces" do - it "creates the namespace" do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).and_return(double(execute: project)) - - expect { post :create, format: :json }.to change(Namespace, :count).by(1) - end - - it "takes the new namespace" do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, an_instance_of(Group), user, access_params) - .and_return(double(execute: project)) - - post :create, format: :json - end - end - - context "when current user can't create namespaces" do - before do - user.update_attribute(:can_create_group, false) - end - - it "doesn't create the namespace" do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).and_return(double(execute: project)) - - expect { post :create, format: :json }.not_to change(Namespace, :count) - end - - it "takes the current user's namespace" do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, user.namespace, user, access_params) - .and_return(double(execute: project)) - - post :create, format: :json - end - end - end - - context 'user has chosen an existing nested namespace for the project' do - let(:parent_namespace) { create(:group, name: 'foo') } - let(:nested_namespace) { create(:group, name: 'bar', parent: parent_namespace) } - - before do - parent_namespace.add_owner(user) - nested_namespace.add_owner(user) - end - - it 'takes the selected namespace and name' do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, nested_namespace, user, access_params) - .and_return(double(execute: project)) - - post :create, params: { target_namespace: nested_namespace.full_path }, format: :json - end - end - - context 'user has chosen a non-existent nested namespaces for the project' do - let(:test_name) { 'test_name' } - - it 'takes the selected namespace and name' do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params) - .and_return(double(execute: project)) - - post :create, params: { target_namespace: 'foo/bar' }, format: :json - end - - it 'creates the namespaces' do - allow(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params) - .and_return(double(execute: project)) - - expect { post :create, params: { target_namespace: 'foo/bar' }, format: :json } - .to change { Namespace.count }.by(2) - end - - it 'new namespace has the right parent' do - allow(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params) - .and_return(double(execute: project)) - - post :create, params: { target_namespace: 'foo/bar' }, format: :json - - expect(Namespace.find_by_path_or_name('bar').parent.path).to eq('foo') - end - end - - context 'user has chosen existent and non-existent nested namespaces and name for the project' do - let(:test_name) { 'test_name' } - let!(:parent_namespace) { create(:group, name: 'foo') } - - before do - parent_namespace.add_owner(user) - end - - it 'takes the selected namespace and name' do - expect(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params) - .and_return(double(execute: project)) - - post :create, params: { target_namespace: 'foo/foobar/bar' }, format: :json - end - - it 'creates the namespaces' do - allow(Gitlab::GitlabImport::ProjectCreator) - .to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params) - .and_return(double(execute: project)) - - expect { post :create, params: { target_namespace: 'foo/foobar/bar' }, format: :json } - .to change { Namespace.count }.by(2) - end - end - - context 'when user can not create projects in the chosen namespace' do - it 'returns 422 response' do - other_namespace = create(:group, name: 'other_namespace') - - post :create, params: { target_namespace: other_namespace.name }, format: :json - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - end - end - - it_behaves_like 'project import rate limiter' - end - end -end diff --git a/spec/controllers/import/manifest_controller_spec.rb b/spec/controllers/import/manifest_controller_spec.rb index 6f805b44e89..69eb736375c 100644 --- a/spec/controllers/import/manifest_controller_spec.rb +++ b/spec/controllers/import/manifest_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state do +RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state, feature_category: :importers do include ImportSpecHelper let_it_be(:user) { create(:user) } @@ -13,6 +13,8 @@ RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state do end before do + stub_application_setting(import_sources: ['manifest']) + sign_in(user) end @@ -45,7 +47,7 @@ RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state do end end - context 'when the user cannot create projects in the group' do + context 'when the user cannot import projects in the group' do it 'displays an error' do sign_in(create(:user)) diff --git a/spec/controllers/import/phabricator_controller_spec.rb b/spec/controllers/import/phabricator_controller_spec.rb deleted file mode 100644 index 9be85a40d82..00000000000 --- a/spec/controllers/import/phabricator_controller_spec.rb +++ /dev/null @@ -1,83 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Import::PhabricatorController do - let(:current_user) { create(:user) } - - before do - sign_in current_user - end - - describe 'GET #new' do - subject { get :new } - - context 'when the import source is not available' do - before do - stub_application_setting(import_sources: []) - end - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'when the import source is available' do - before do - stub_application_setting(import_sources: ['phabricator']) - end - - it { is_expected.to have_gitlab_http_status(:ok) } - end - end - - describe 'POST #create' do - subject(:post_create) { post :create, params: params } - - context 'with valid params' do - let(:params) do - { path: 'phab-import', - name: 'Phab import', - phabricator_server_url: 'https://phabricator.example.com', - api_token: 'hazaah', - namespace_id: current_user.namespace_id } - end - - it 'creates a project to import', :sidekiq_might_not_need_inline do - expect_next_instance_of(Gitlab::PhabricatorImport::Importer) do |importer| - expect(importer).to receive(:execute) - end - - expect { post_create }.to change { current_user.namespace.projects.reload.size }.from(0).to(1) - - expect(current_user.namespace.projects.last).to be_import - end - end - - context 'when an import param is missing' do - let(:params) do - { path: 'phab-import', - name: 'Phab import', - phabricator_server_url: nil, - api_token: 'hazaah', - namespace_id: current_user.namespace_id } - end - - it 'does not create the project' do - expect { post_create }.not_to change { current_user.namespace.projects.reload.size } - end - end - - context 'when a project param is missing' do - let(:params) do - { phabricator_server_url: 'https://phabricator.example.com', - api_token: 'hazaah', - namespace_id: current_user.namespace_id } - end - - it 'does not create the project' do - expect { post_create }.not_to change { current_user.namespace.projects.reload.size } - end - end - - it_behaves_like 'project import rate limiter' - end -end diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index b3b7753df61..f3b21e191c4 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -192,6 +192,26 @@ RSpec.describe InvitesController do expect(session[:invite_email]).to eq(member.invite_email) end + context 'with stored location for user' do + it 'stores the correct path for user' do + request + + expect(controller.stored_location_for(:user)).to eq(activity_project_path(member.source)) + end + + context 'with relative root' do + before do + stub_default_url_options(script_name: '/gitlab') + end + + it 'stores the correct path for user' do + request + + expect(controller.stored_location_for(:user)).to eq(activity_project_path(member.source)) + end + end + end + context 'when it is part of our invite email experiment' do let(:extra_params) { { invite_type: 'initial_email' } } diff --git a/spec/controllers/jira_connect/app_descriptor_controller_spec.rb b/spec/controllers/jira_connect/app_descriptor_controller_spec.rb index 4f8b2b90637..48b315646de 100644 --- a/spec/controllers/jira_connect/app_descriptor_controller_spec.rb +++ b/spec/controllers/jira_connect/app_descriptor_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JiraConnect::AppDescriptorController do +RSpec.describe JiraConnect::AppDescriptorController, feature_category: :integrations do describe '#show' do let(:descriptor) do json_response.deep_symbolize_keys diff --git a/spec/controllers/jira_connect/branches_controller_spec.rb b/spec/controllers/jira_connect/branches_controller_spec.rb index 45daf3b5309..1d3ddc2e33b 100644 --- a/spec/controllers/jira_connect/branches_controller_spec.rb +++ b/spec/controllers/jira_connect/branches_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JiraConnect::BranchesController do +RSpec.describe JiraConnect::BranchesController, feature_category: :integrations do describe '#new' do context 'when logged in' do let_it_be(:user) { create(:user) } @@ -17,7 +17,7 @@ RSpec.describe JiraConnect::BranchesController do expect(response).to be_successful expect(assigns(:new_branch_data)).to include( initial_branch_name: 'ACME-123-my-issue', - success_state_svg_path: start_with('/assets/illustrations/merge_requests-') + success_state_svg_path: start_with('/assets/illustrations/empty-state/empty-merge-requests-md-') ) end diff --git a/spec/controllers/jira_connect/events_controller_spec.rb b/spec/controllers/jira_connect/events_controller_spec.rb index 7da9eb7ac16..ffad3aa7b02 100644 --- a/spec/controllers/jira_connect/events_controller_spec.rb +++ b/spec/controllers/jira_connect/events_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JiraConnect::EventsController do +RSpec.describe JiraConnect::EventsController, feature_category: :integrations do shared_examples 'verifies asymmetric JWT token' do context 'when token is valid' do include_context 'valid JWT token' diff --git a/spec/controllers/jira_connect/subscriptions_controller_spec.rb b/spec/controllers/jira_connect/subscriptions_controller_spec.rb index e9c94f09c99..a05f18f1a16 100644 --- a/spec/controllers/jira_connect/subscriptions_controller_spec.rb +++ b/spec/controllers/jira_connect/subscriptions_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JiraConnect::SubscriptionsController do +RSpec.describe JiraConnect::SubscriptionsController, feature_category: :integrations do let_it_be(:installation) { create(:jira_connect_installation) } describe '#index' do @@ -56,26 +56,6 @@ RSpec.describe JiraConnect::SubscriptionsController do expect(json_response).to include('subscriptions_path' => jira_connect_subscriptions_path) end - context 'when not signed in to GitLab' do - it 'contains a login path' do - expect(json_response).to include('login_path' => jira_connect_users_path) - end - end - - context 'when signed in to GitLab' do - let(:user) { create(:user) } - - before do - sign_in(user) - - get :index, params: { jwt: jwt } - end - - it 'does not contain a login path' do - expect(json_response).to include('login_path' => nil) - end - end - context 'with context qsh' do # The JSON endpoint will be requested by frontend using a JWT that Atlassian provides via Javascript. # This JWT will likely use a context-qsh because Atlassian don't know for which endpoint it will be used. diff --git a/spec/controllers/oauth/applications_controller_spec.rb b/spec/controllers/oauth/applications_controller_spec.rb index 9b16dc9a463..5b9fd192ad4 100644 --- a/spec/controllers/oauth/applications_controller_spec.rb +++ b/spec/controllers/oauth/applications_controller_spec.rb @@ -71,6 +71,39 @@ RSpec.describe Oauth::ApplicationsController do it_behaves_like 'redirects to 2fa setup page when the user requires it' end + describe 'PUT #renew' do + let(:oauth_params) do + { + id: application.id + } + end + + subject { put :renew, params: oauth_params } + + it { is_expected.to have_gitlab_http_status(:ok) } + it { expect { subject }.to change { application.reload.secret } } + + it_behaves_like 'redirects to login page when the user is not signed in' + it_behaves_like 'redirects to 2fa setup page when the user requires it' + + it 'returns the secret in json format' do + subject + + expect(json_response['secret']).not_to be_nil + end + + context 'when renew fails' do + before do + allow_next_found_instance_of(Doorkeeper::Application) do |application| + allow(application).to receive(:save).and_return(false) + end + end + + it { expect { subject }.not_to change { application.reload.secret } } + it { is_expected.to have_gitlab_http_status(:unprocessable_entity) } + end + end + describe 'GET #show' do subject { get :show, params: { id: application.id } } @@ -113,30 +146,11 @@ RSpec.describe Oauth::ApplicationsController do subject { post :create, params: oauth_params } - context 'when hash_oauth_tokens flag set' do - before do - stub_feature_flags(hash_oauth_secrets: true) - end - - it 'creates an application' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template :show - end - end - - context 'when hash_oauth_tokens flag not set' do - before do - stub_feature_flags(hash_oauth_secrets: false) - end - - it 'creates an application' do - subject + it 'creates an application' do + subject - expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(oauth_application_path(Doorkeeper::Application.last)) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show end it 'redirects back to profile page if OAuth applications are disabled' do diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index 5185aa64d9f..3476c7b8465 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -7,8 +7,7 @@ RSpec.describe Oauth::AuthorizationsController do let(:application_scopes) { 'api read_user' } let!(:application) do - create(:oauth_application, scopes: application_scopes, - redirect_uri: 'http://example.com') + create(:oauth_application, scopes: application_scopes, redirect_uri: 'http://example.com') end let(:params) do diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index ab3f3fd397d..ebfa48870a9 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe OmniauthCallbacksController, type: :controller do +RSpec.describe OmniauthCallbacksController, type: :controller, feature_category: :system_access do include LoginHelpers describe 'omniauth' do @@ -202,20 +202,30 @@ RSpec.describe OmniauthCallbacksController, type: :controller do end end - context 'when user with 2FA is unconfirmed' do + context 'when a user has 2FA enabled' do render_views let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: provider) } - before do - user.update_column(:confirmed_at, nil) - end + context 'when a user is unconfirmed' do + before do + stub_application_setting_enum('email_confirmation_setting', 'hard') - it 'redirects to login page' do - post provider + user.update!(confirmed_at: nil) + end + + it 'redirects to login page' do + post provider + + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to match(/You have to confirm your email address before continuing./) + end + end - expect(response).to redirect_to(new_user_session_path) - expect(flash[:alert]).to match(/You have to confirm your email address before continuing./) + context 'when a user is confirmed' do + it 'returns 200 response' do + expect(response).to have_gitlab_http_status(:ok) + end end end @@ -324,9 +334,10 @@ RSpec.describe OmniauthCallbacksController, type: :controller do expect(controller).to receive(:atlassian_oauth2).and_wrap_original do |m, *args| m.call(*args) - expect(Gitlab::ApplicationContext.current) - .to include('meta.user' => user.username, - 'meta.caller_id' => 'OmniauthCallbacksController#atlassian_oauth2') + expect(Gitlab::ApplicationContext.current).to include( + 'meta.user' => user.username, + 'meta.caller_id' => 'OmniauthCallbacksController#atlassian_oauth2' + ) end post :atlassian_oauth2 @@ -419,6 +430,31 @@ RSpec.describe OmniauthCallbacksController, type: :controller do end end + describe '#openid_connect' do + let(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) } + let(:extern_uid) { 'my-uid' } + let(:provider) { 'openid_connect' } + + before do + prepare_provider_route('openid_connect') + + mock_auth_hash(provider, extern_uid, user.email, additional_info: {}) + + request.env['devise.mapping'] = Devise.mappings[:user] + request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth'] + end + + it_behaves_like 'known sign in' do + let(:post_action) { post provider } + end + + it 'allows sign in' do + post provider + + expect(request.env['warden']).to be_authenticated + end + end + describe '#saml' do let(:last_request_id) { 'ONELOGIN_4fee3b046395c4e751011e97f8900b5273d56685' } let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml') } @@ -431,8 +467,12 @@ RSpec.describe OmniauthCallbacksController, type: :controller do before do stub_last_request_id(last_request_id) - stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], - providers: [saml_config]) + stub_omniauth_saml_config( + enabled: true, + auto_link_saml_user: true, + allow_single_sign_on: ['saml'], + providers: [saml_config] + ) mock_auth_hash_with_saml_xml('saml', +'my-uid', user.email, mock_saml_response) request.env['devise.mapping'] = Devise.mappings[:user] request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth'] @@ -523,9 +563,10 @@ RSpec.describe OmniauthCallbacksController, type: :controller do expect(controller).to receive(:saml).and_wrap_original do |m, *args| m.call(*args) - expect(Gitlab::ApplicationContext.current) - .to include('meta.user' => user.username, - 'meta.caller_id' => 'OmniauthCallbacksController#saml') + expect(Gitlab::ApplicationContext.current).to include( + 'meta.user' => user.username, + 'meta.caller_id' => 'OmniauthCallbacksController#saml' + ) end post :saml, params: { SAMLResponse: mock_saml_response } diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb index 9494f55c631..aad946acad4 100644 --- a/spec/controllers/passwords_controller_spec.rb +++ b/spec/controllers/passwords_controller_spec.rb @@ -99,8 +99,7 @@ RSpec.describe PasswordsController do m.call(*args) expect(Gitlab::ApplicationContext.current) - .to include('meta.user' => user.username, - 'meta.caller_id' => 'PasswordsController#update') + .to include('meta.user' => user.username, 'meta.caller_id' => 'PasswordsController#update') end subject diff --git a/spec/controllers/profiles/accounts_controller_spec.rb b/spec/controllers/profiles/accounts_controller_spec.rb index ba349768b0f..f0ee2e178cf 100644 --- a/spec/controllers/profiles/accounts_controller_spec.rb +++ b/spec/controllers/profiles/accounts_controller_spec.rb @@ -16,18 +16,16 @@ RSpec.describe Profiles::AccountsController do expect(response).to have_gitlab_http_status(:not_found) end - [:saml, :cas3].each do |provider| - describe "#{provider} provider" do - let(:user) { create(:omniauth_user, provider: provider.to_s) } + describe "saml provider" do + let(:user) { create(:omniauth_user, provider: 'saml') } - it "does not allow to unlink connected account" do - identity = user.identities.last + it "does not allow to unlink connected account" do + identity = user.identities.last - delete :unlink, params: { provider: provider.to_s } + delete :unlink, params: { provider: 'saml' } - expect(response).to have_gitlab_http_status(:found) - expect(user.reload.identities).to include(identity) - end + expect(response).to have_gitlab_http_status(:found) + expect(user.reload.identities).to include(identity) end end diff --git a/spec/controllers/profiles/preferences_controller_spec.rb b/spec/controllers/profiles/preferences_controller_spec.rb index e2a216bb462..e2ade5e3de9 100644 --- a/spec/controllers/profiles/preferences_controller_spec.rb +++ b/spec/controllers/profiles/preferences_controller_spec.rb @@ -53,8 +53,7 @@ RSpec.describe Profiles::PreferencesController do first_day_of_week: '1', preferred_language: 'jp', tab_width: '5', - render_whitespace_in_code: 'true', - use_legacy_web_ide: 'true' + render_whitespace_in_code: 'true' }.with_indifferent_access expect(user).to receive(:assign_attributes).with(ActionController::Parameters.new(prefs).permit!) @@ -109,5 +108,33 @@ RSpec.describe Profiles::PreferencesController do expect(response.parsed_body['type']).to eq('alert') end end + + context 'on disable_follow_users feature flag' do + context 'with feature flag disabled' do + before do + stub_feature_flags(disable_follow_users: false) + end + + it 'does not update enabled_following preference of user' do + prefs = { enabled_following: false } + + go params: prefs + user.reload + + expect(user.enabled_following).to eq(true) + end + end + + context 'with feature flag enabled' do + it 'does not update enabled_following preference of user' do + prefs = { enabled_following: false } + + go params: prefs + user.reload + + expect(user.enabled_following).to eq(false) + end + end + end end end diff --git a/spec/controllers/profiles/two_factor_auths_controller_spec.rb b/spec/controllers/profiles/two_factor_auths_controller_spec.rb index 7d7cdededdb..dde0af3c543 100644 --- a/spec/controllers/profiles/two_factor_auths_controller_spec.rb +++ b/spec/controllers/profiles/two_factor_auths_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Profiles::TwoFactorAuthsController, feature_category: :authentication_and_authorization do +RSpec.describe Profiles::TwoFactorAuthsController, feature_category: :system_access do before do # `user` should be defined within the action-specific describe blocks sign_in(user) diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index daf0f36c28b..b1c43a33386 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -11,8 +11,7 @@ RSpec.describe ProfilesController, :request_store do sign_in(user) new_password = User.random_password expect do - post :update, - params: { user: { password: new_password, password_confirmation: new_password } } + post :update, params: { user: { password: new_password, password_confirmation: new_password } } end.not_to change { user.reload.encrypted_password } expect(response).to have_gitlab_http_status(:found) @@ -23,8 +22,7 @@ RSpec.describe ProfilesController, :request_store do it 'allows an email update from a user without an external email address' do sign_in(user) - put :update, - params: { user: { email: "john@gmail.com", name: "John", validation_password: password } } + put :update, params: { user: { email: "john@gmail.com", name: "John", validation_password: password } } user.reload @@ -37,8 +35,7 @@ RSpec.describe ProfilesController, :request_store do create(:email, :confirmed, user: user, email: 'john@gmail.com') sign_in(user) - put :update, - params: { user: { email: "john@gmail.com", name: "John" } } + put :update, params: { user: { email: "john@gmail.com", name: "John" } } user.reload @@ -54,8 +51,7 @@ RSpec.describe ProfilesController, :request_store do ldap_user.create_user_synced_attributes_metadata(provider: 'ldap', name_synced: true, email_synced: true) sign_in(ldap_user) - put :update, - params: { user: { email: "john@gmail.com", name: "John" } } + put :update, params: { user: { email: "john@gmail.com", name: "John" } } ldap_user.reload @@ -71,8 +67,7 @@ RSpec.describe ProfilesController, :request_store do ldap_user.create_user_synced_attributes_metadata(provider: 'ldap', name_synced: true, email_synced: true, location_synced: false) sign_in(ldap_user) - put :update, - params: { user: { email: "john@gmail.com", name: "John", location: "City, Country" } } + put :update, params: { user: { email: "john@gmail.com", name: "John", location: "City, Country" } } ldap_user.reload @@ -85,10 +80,7 @@ RSpec.describe ProfilesController, :request_store do it 'allows setting a user status', :freeze_time do sign_in(user) - put( - :update, - params: { user: { status: { message: 'Working hard!', availability: 'busy', clear_status_after: '8_hours' } } } - ) + put :update, params: { user: { status: { message: 'Working hard!', availability: 'busy', clear_status_after: '8_hours' } } } expect(user.reload.status.message).to eq('Working hard!') expect(user.reload.status.availability).to eq('busy') @@ -183,22 +175,14 @@ RSpec.describe ProfilesController, :request_store do end it 'updates a username using JSON request' do - put :update_username, - params: { - user: { username: new_username } - }, - format: :json + put :update_username, params: { user: { username: new_username } }, format: :json expect(response).to have_gitlab_http_status(:ok) expect(json_response['message']).to eq(s_('Profiles|Username successfully changed')) end it 'renders an error message when the username was not updated' do - put :update_username, - params: { - user: { username: 'invalid username.git' } - }, - format: :json + put :update_username, params: { user: { username: 'invalid username.git' } }, format: :json expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(json_response['message']).to match(/Username change failed/) diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index c707b5dc39d..c7b74b5cf68 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -9,11 +9,13 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:pipeline, reload: true) do - create(:ci_pipeline, - project: project, - sha: project.commit.sha, - ref: project.default_branch, - status: 'success') + create( + :ci_pipeline, + project: project, + sha: project.commit.sha, + ref: project.default_branch, + status: 'success' + ) end let!(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } @@ -25,31 +27,13 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts describe 'GET index' do subject { get :index, params: { namespace_id: project.namespace, project_id: project } } - context 'when feature flag is on' do - render_views - - before do - stub_feature_flags(artifacts_management_page: true) - end - - it 'renders the page with data for the artifacts app' do - subject + render_views - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template('projects/artifacts/index') - end - end - - context 'when feature flag is off' do - before do - stub_feature_flags(artifacts_management_page: false) - end - - it 'renders no content' do - subject + it 'renders the page with data for the artifacts app' do + subject - expect(response).to have_gitlab_http_status(:no_content) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('projects/artifacts/index') end end @@ -177,9 +161,10 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts end it 'sends the codequality report' do - expect(controller).to receive(:send_file) - .with(job.job_artifacts_codequality.file.path, - hash_including(disposition: 'attachment', filename: filename)).and_call_original + expect(controller).to receive(:send_file).with( + job.job_artifacts_codequality.file.path, + hash_including(disposition: 'attachment', filename: filename) + ).and_call_original download_artifact(file_type: file_type) @@ -557,8 +542,7 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts context 'with regular branch' do before do - pipeline.update!(ref: 'master', - sha: project.commit('master').sha) + pipeline.update!(ref: 'master', sha: project.commit('master').sha) get :latest_succeeded, params: params_from_ref('master') end @@ -568,8 +552,7 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts context 'with branch name containing slash' do before do - pipeline.update!(ref: 'improve/awesome', - sha: project.commit('improve/awesome').sha) + pipeline.update!(ref: 'improve/awesome', sha: project.commit('improve/awesome').sha) get :latest_succeeded, params: params_from_ref('improve/awesome') end @@ -579,8 +562,7 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts context 'with branch name and path containing slashes' do before do - pipeline.update!(ref: 'improve/awesome', - sha: project.commit('improve/awesome').sha) + pipeline.update!(ref: 'improve/awesome', sha: project.commit('improve/awesome').sha) get :latest_succeeded, params: params_from_ref('improve/awesome', job.name, 'file/README.md') end @@ -596,11 +578,13 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts before do create_file_in_repo(project, 'master', 'master', 'test.txt', 'This is test') - create(:ci_pipeline, + create( + :ci_pipeline, project: project, sha: project.commit.sha, ref: project.default_branch, - status: 'failed') + status: 'failed' + ) get :latest_succeeded, params: params_from_ref(project.default_branch) end diff --git a/spec/controllers/projects/badges_controller_spec.rb b/spec/controllers/projects/badges_controller_spec.rb index d41e8d6169f..ef2afd7ca38 100644 --- a/spec/controllers/projects/badges_controller_spec.rb +++ b/spec/controllers/projects/badges_controller_spec.rb @@ -98,6 +98,16 @@ RSpec.describe Projects::BadgesController do expect(response.body).to include('123') end end + + if badge_type == :release + context 'when value_width param is used' do + it 'sets custom value width' do + get_badge(badge_type, value_width: '123') + + expect(response.body).to include('123') + end + end + end end shared_examples 'a badge resource' do |badge_type| @@ -186,7 +196,7 @@ RSpec.describe Projects::BadgesController do namespace_id: project.namespace.to_param, project_id: project, ref: pipeline.ref - }.merge(args.slice(:style, :key_text, :key_width, :ignore_skipped)) + }.merge(args.slice(:style, :key_text, :key_width, :value_width, :ignore_skipped)) get badge, params: params, format: :svg end diff --git a/spec/controllers/projects/blame_controller_spec.rb b/spec/controllers/projects/blame_controller_spec.rb index 62a544bb3fc..50556bdb652 100644 --- a/spec/controllers/projects/blame_controller_spec.rb +++ b/spec/controllers/projects/blame_controller_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' -RSpec.describe Projects::BlameController do - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } +RSpec.describe Projects::BlameController, feature_category: :source_code_management do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } before do sign_in(user) @@ -13,37 +13,55 @@ RSpec.describe Projects::BlameController do controller.instance_variable_set(:@project, project) end - describe "GET show" do - render_views - - before do - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: id - }) - end - - context "valid branch, valid file" do + shared_examples 'blame_response' do + context 'valid branch, valid file' do let(:id) { 'master/files/ruby/popen.rb' } it { is_expected.to respond_with(:success) } end - context "valid branch, invalid file" do + context 'valid branch, invalid file' do let(:id) { 'master/files/ruby/invalid-path.rb' } it 'redirects' do - expect(subject) - .to redirect_to("/#{project.full_path}/-/tree/master") + expect(subject).to redirect_to("/#{project.full_path}/-/tree/master") end end - context "invalid branch, valid file" do + context 'invalid branch, valid file' do let(:id) { 'invalid-branch/files/ruby/missing_file.rb' } it { is_expected.to respond_with(:not_found) } end end + + describe 'GET show' do + render_views + + before do + get :show, params: { namespace_id: project.namespace, project_id: project, id: id } + end + + it_behaves_like 'blame_response' + end + + describe 'GET page' do + render_views + + before do + get :page, params: { namespace_id: project.namespace, project_id: project, id: id } + end + + it_behaves_like 'blame_response' + end + + describe 'GET streaming' do + render_views + + before do + get :streaming, params: { namespace_id: project.namespace, project_id: project, id: id } + end + + it_behaves_like 'blame_response' + end end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index ec92d92e2a9..b07cb7a228d 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -100,13 +100,7 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme let(:id) { 'master/README.md' } before do - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: id - }, - format: :json) + get :show, params: { namespace_id: project.namespace, project_id: project, id: id }, format: :json end it do @@ -120,14 +114,7 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme let(:id) { 'master/README.md' } before do - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: id, - viewer: 'none' - }, - format: :json) + get :show, params: { namespace_id: project.namespace, project_id: project, id: id, viewer: 'none' }, format: :json end it do @@ -140,12 +127,8 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme context 'with tree path' do before do - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: id - }) + get :show, params: { namespace_id: project.namespace, project_id: project, id: id } + controller.instance_variable_set(:@blob, nil) end @@ -387,11 +370,22 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme end end - it_behaves_like 'tracking unique hll events' do + context 'events tracking' do + let(:target_event) { 'g_edit_by_sfe' } + subject(:request) { put :update, params: default_params } - let(:target_event) { 'g_edit_by_sfe' } - let(:expected_value) { instance_of(Integer) } + it_behaves_like 'tracking unique hll events' do + let(:expected_value) { instance_of(Integer) } + end + + it_behaves_like 'Snowplow event tracking with RedisHLL context' do + let(:action) { 'perform_sfe_action' } + let(:category) { described_class.to_s } + let(:namespace) { project.namespace.reload } + let(:property) { target_event } + let(:label) { 'usage_activity_by_stage_monthly.create.action_monthly_active_users_sfe_edit' } + end end end @@ -519,6 +513,7 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme describe 'POST create' do let(:user) { create(:user) } + let(:target_event) { 'g_edit_by_sfe' } let(:default_params) do { namespace_id: project.namespace, @@ -540,10 +535,17 @@ RSpec.describe Projects::BlobController, feature_category: :source_code_manageme subject(:request) { post :create, params: default_params } it_behaves_like 'tracking unique hll events' do - let(:target_event) { 'g_edit_by_sfe' } let(:expected_value) { instance_of(Integer) } end + it_behaves_like 'Snowplow event tracking with RedisHLL context' do + let(:action) { 'perform_sfe_action' } + let(:category) { described_class.to_s } + let(:namespace) { project.namespace } + let(:property) { target_event } + let(:label) { 'usage_activity_by_stage_monthly.create.action_monthly_active_users_sfe_edit' } + end + it 'redirects to blob' do request diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index dcde22c1fd6..600f8047a1d 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -22,13 +22,12 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana before do sign_in(developer) - post :create, - params: { - namespace_id: project.namespace, - project_id: project, - branch_name: branch, - ref: ref - } + post :create, params: { + namespace_id: project.namespace, + project_id: project, + branch_name: branch, + ref: ref + } end context "valid branch name, valid source" do @@ -83,13 +82,12 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana end it 'redirects' do - post :create, - params: { - namespace_id: project.namespace, - project_id: project, - branch_name: branch, - issue_iid: issue.iid - } + post :create, params: { + namespace_id: project.namespace, + project_id: project, + branch_name: branch, + issue_iid: issue.iid + } expect(subject) .to redirect_to("/#{project.full_path}/-/tree/1-feature-branch") @@ -98,13 +96,12 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana it 'posts a system note' do expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, developer, "1-feature-branch", branch_project: project) - post :create, - params: { - namespace_id: project.namespace, - project_id: project, - branch_name: branch, - issue_iid: issue.iid - } + post :create, params: { + namespace_id: project.namespace, + project_id: project, + branch_name: branch, + issue_iid: issue.iid + } end context 'confidential_issue_project_id is present' do @@ -167,13 +164,12 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana expect_any_instance_of(::Branches::CreateService).to receive(:execute).and_return(result) expect(SystemNoteService).to receive(:new_issue_branch).and_return(true) - post :create, - params: { - namespace_id: project.namespace.to_param, - project_id: project.to_param, - branch_name: branch, - issue_iid: issue.iid - } + post :create, params: { + namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: branch, + issue_iid: issue.iid + } expect(response).to redirect_to project_tree_path(project, branch) end @@ -189,13 +185,12 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana expect_any_instance_of(::Branches::CreateService).to receive(:execute).and_return(result) expect(SystemNoteService).to receive(:new_issue_branch).and_return(true) - post :create, - params: { - namespace_id: project.namespace.to_param, - project_id: project.to_param, - branch_name: branch, - issue_iid: issue.iid - } + post :create, params: { + namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: branch, + issue_iid: issue.iid + } expect(response.location).to include(project_new_blob_path(project, branch)) expect(response).to have_gitlab_http_status(:found) @@ -210,13 +205,12 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana expect_any_instance_of(::Branches::CreateService).to receive(:execute).and_return(result) expect(SystemNoteService).to receive(:new_issue_branch).and_return(true) - post :create, - params: { - namespace_id: project.namespace.to_param, - project_id: project.to_param, - branch_name: branch, - issue_iid: issue.iid - } + post :create, params: { + namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: branch, + issue_iid: issue.iid + } expect(response.location).to include(project_new_blob_path(project, branch)) expect(response).to have_gitlab_http_status(:found) @@ -229,13 +223,12 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana it "doesn't post a system note" do expect(SystemNoteService).not_to receive(:new_issue_branch) - post :create, - params: { - namespace_id: project.namespace, - project_id: project, - branch_name: branch, - issue_iid: issue.iid - } + post :create, params: { + namespace_id: project.namespace, + project_id: project, + branch_name: branch, + issue_iid: issue.iid + } end end @@ -249,13 +242,12 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana it "doesn't post a system note" do expect(SystemNoteService).not_to receive(:new_issue_branch) - post :create, - params: { - namespace_id: project.namespace, - project_id: project, - branch_name: branch, - issue_iid: issue.iid - } + post :create, params: { + namespace_id: project.namespace, + project_id: project, + branch_name: branch, + issue_iid: issue.iid + } end end end @@ -285,18 +277,17 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana create_branch name: "<script>alert('merge');</script>", ref: "<script>alert('ref');</script>" expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.body).to include 'Failed to create branch' end end def create_branch(name:, ref:) - post :create, - format: :json, - params: { - namespace_id: project.namespace.to_param, - project_id: project.to_param, - branch_name: name, - ref: ref - } + post :create, format: :json, params: { + namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: name, + ref: ref + } end end @@ -345,13 +336,11 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana before do sign_in(developer) - post :destroy, - format: format, - params: { - id: branch, - namespace_id: project.namespace, - project_id: project - } + post :destroy, format: format, params: { + id: branch, + namespace_id: project.namespace, + project_id: project + } end context 'as JS' do @@ -445,11 +434,10 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana describe "DELETE destroy_all_merged" do def destroy_all_merged - delete :destroy_all_merged, - params: { - namespace_id: project.namespace, - project_id: project - } + delete :destroy_all_merged, params: { + namespace_id: project.namespace, + project_id: project + } end context 'when user is allowed to push' do @@ -492,13 +480,11 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana context 'when rendering a JSON format' do it 'filters branches by name' do - get :index, - format: :json, - params: { - namespace_id: project.namespace, - project_id: project, - search: 'master' - } + get :index, format: :json, params: { + namespace_id: project.namespace, + project_id: project, + search: 'master' + } expect(json_response.length).to eq 1 expect(json_response.first).to eq 'master' @@ -523,13 +509,11 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana status: :success, created_at: 2.months.ago) - get :index, - format: :html, - params: { - namespace_id: project.namespace, - project_id: project, - state: 'all' - } + get :index, format: :html, params: { + namespace_id: project.namespace, + project_id: project, + state: 'all' + } expect(assigns[:branch_pipeline_statuses]["master"].group).to eq("success") expect(assigns[:sort]).to eq('updated_desc') @@ -555,13 +539,11 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana status: :success, created_at: 2.months.ago) - get :index, - format: :html, - params: { - namespace_id: project.namespace, - project_id: project, - state: 'all' - } + get :index, format: :html, params: { + namespace_id: project.namespace, + project_id: project, + state: 'all' + } expect(assigns[:branch_pipeline_statuses]["master"].group).to eq("running") expect(assigns[:branch_pipeline_statuses]["test"].group).to eq("success") @@ -570,13 +552,11 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana context 'when a branch contains no pipelines' do it 'no commit statuses are received' do - get :index, - format: :html, - params: { - namespace_id: project.namespace, - project_id: project, - state: 'stale' - } + get :index, format: :html, params: { + namespace_id: project.namespace, + project_id: project, + state: 'stale' + } expect(assigns[:branch_pipeline_statuses]).to be_blank expect(assigns[:sort]).to eq('updated_asc') @@ -589,14 +569,12 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana # was not raised whenever the cache is enabled yet cold. context 'when cache is enabled yet cold', :request_store do it 'return with a status 200' do - get :index, - format: :html, - params: { - namespace_id: project.namespace, - project_id: project, - sort: 'name_asc', - state: 'all' - } + get :index, format: :html, params: { + namespace_id: project.namespace, + project_id: project, + sort: 'name_asc', + state: 'all' + } expect(response).to have_gitlab_http_status(:ok) expect(assigns[:sort]).to eq('name_asc') @@ -609,13 +587,11 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana end it 'return with a status 200' do - get :index, - format: :html, - params: { - namespace_id: project.namespace, - project_id: project, - state: 'all' - } + get :index, format: :html, params: { + namespace_id: project.namespace, + project_id: project, + state: 'all' + } expect(response).to have_gitlab_http_status(:ok) end @@ -623,37 +599,31 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana context 'when deprecated sort/search/page parameters are specified' do it 'returns with a status 301 when sort specified' do - get :index, - format: :html, - params: { - namespace_id: project.namespace, - project_id: project, - sort: 'updated_asc' - } + get :index, format: :html, params: { + namespace_id: project.namespace, + project_id: project, + sort: 'updated_asc' + } expect(response).to redirect_to project_branches_filtered_path(project, state: 'all') end it 'returns with a status 301 when search specified' do - get :index, - format: :html, - params: { - namespace_id: project.namespace, - project_id: project, - search: 'feature' - } + get :index, format: :html, params: { + namespace_id: project.namespace, + project_id: project, + search: 'feature' + } expect(response).to redirect_to project_branches_filtered_path(project, state: 'all') end it 'returns with a status 301 when page specified' do - get :index, - format: :html, - params: { - namespace_id: project.namespace, - project_id: project, - page: 2 - } + get :index, format: :html, params: { + namespace_id: project.namespace, + project_id: project, + page: 2 + } expect(response).to redirect_to project_branches_filtered_path(project, state: 'all') end @@ -747,13 +717,11 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana end it 'returns the commit counts behind and ahead of default branch' do - get :diverging_commit_counts, - format: :json, - params: { - namespace_id: project.namespace, - project_id: project, - names: %w[fix add-pdf-file branch-merged] - } + get :diverging_commit_counts, format: :json, params: { + namespace_id: project.namespace, + project_id: project, + names: %w[fix add-pdf-file branch-merged] + } expect(response).to have_gitlab_http_status(:ok) expect(json_response).to eq( @@ -766,12 +734,10 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana it 'returns the commits counts with no names provided' do allow_any_instance_of(Repository).to receive(:branch_count).and_return(Kaminari.config.default_per_page) - get :diverging_commit_counts, - format: :json, - params: { - namespace_id: project.namespace, - project_id: project - } + get :diverging_commit_counts, format: :json, params: { + namespace_id: project.namespace, + project_id: project + } expect(response).to have_gitlab_http_status(:ok) expect(json_response.count).to be > 1 @@ -783,25 +749,21 @@ RSpec.describe Projects::BranchesController, feature_category: :source_code_mana end it 'returns 422 if no names are specified' do - get :diverging_commit_counts, - format: :json, - params: { - namespace_id: project.namespace, - project_id: project - } + get :diverging_commit_counts, format: :json, params: { + namespace_id: project.namespace, + project_id: project + } expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(json_response['error']).to eq("Specify at least one and at most #{Kaminari.config.default_per_page} branch names") end it 'returns the list of counts' do - get :diverging_commit_counts, - format: :json, - params: { - namespace_id: project.namespace, - project_id: project, - names: %w[fix add-pdf-file branch-merged] - } + get :diverging_commit_counts, format: :json, params: { + namespace_id: project.namespace, + project_id: project, + names: %w[fix add-pdf-file branch-merged] + } expect(response).to have_gitlab_http_status(:ok) expect(json_response.count).to be > 1 diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index c7d2b1fa3af..f976b5bfe67 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::ClustersController, feature_category: :kubernetes_management do +RSpec.describe Projects::ClustersController, feature_category: :deployment_management do include AccessMatchersForController include GoogleApi::CloudPlatformHelpers include KubernetesHelpers @@ -123,7 +123,7 @@ RSpec.describe Projects::ClustersController, feature_category: :kubernetes_manag { id: proxyable.id.to_s, namespace_id: project.namespace.full_path, - project_id: project.name + project_id: project.path } end @@ -171,7 +171,7 @@ RSpec.describe Projects::ClustersController, feature_category: :kubernetes_manag { id: cluster.id, namespace_id: project.namespace.full_path, - project_id: project.name + project_id: project.path } end end @@ -358,12 +358,6 @@ RSpec.describe Projects::ClustersController, feature_category: :kubernetes_manag expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('cluster_status') end - - it 'invokes schedule_status_update on each application' do - expect_any_instance_of(Clusters::Applications::Ingress).to receive(:schedule_status_update) - - go - end end describe 'security' do @@ -403,20 +397,37 @@ RSpec.describe Projects::ClustersController, feature_category: :kubernetes_manag end describe 'functionality' do - render_views + context 'when remove_monitor_metrics FF is disabled' do + before do + stub_feature_flags(remove_monitor_metrics: false) + end - it "renders view" do - go + render_views - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:cluster)).to eq(cluster) + it "renders view" do + go + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:cluster)).to eq(cluster) + end + + it 'renders integration tab view' do + go(tab: 'integrations') + + expect(response).to render_template('clusters/clusters/_integrations') + expect(response).to have_gitlab_http_status(:ok) + end end - it 'renders integration tab view' do - go(tab: 'integrations') + context 'when remove_monitor_metrics FF is enabled' do + render_views - expect(response).to render_template('clusters/clusters/_integrations') - expect(response).to have_gitlab_http_status(:ok) + it 'renders details tab view', :aggregate_failures do + go(tab: 'integrations') + + expect(response).to render_template('clusters/clusters/_details') + expect(response).to have_gitlab_http_status(:ok) + end end end @@ -441,11 +452,12 @@ RSpec.describe Projects::ClustersController, feature_category: :kubernetes_manag describe 'PUT update' do def go(format: :html) - put :update, params: params.merge(namespace_id: project.namespace.to_param, - project_id: project.to_param, - id: cluster, - format: format - ) + put :update, params: params.merge( + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: cluster, + format: format + ) end before do diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 8d3939d8133..44486d0ed41 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::CommitController do +RSpec.describe Projects::CommitController, feature_category: :source_code_management do include ProjectForksHelper let_it_be(:project) { create(:project, :repository) } @@ -84,22 +84,6 @@ RSpec.describe Projects::CommitController do expect(response).to be_successful end - it 'only loads blobs in the current page' do - stub_feature_flags(async_commit_diff_files: false) - stub_const('Projects::CommitController::COMMIT_DIFFS_PER_PAGE', 1) - - commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') - - expect_next_instance_of(Repository) do |repository| - # This commit contains 3 changed files but we expect only the blobs for the first one to be loaded - expect(repository).to receive(:blobs_at).with([[commit.id, '.gitignore']], anything).and_call_original - end - - go(id: commit.id) - - expect(response).to be_ok - end - shared_examples "export as" do |format| it "does generally work" do go(id: commit.id, format: format) @@ -155,12 +139,7 @@ RSpec.describe Projects::CommitController do let(:commit) { fork_project.commit('remove-submodule') } it 'renders it' do - get(:show, - params: { - namespace_id: fork_project.namespace, - project_id: fork_project, - id: commit.id - }) + get :show, params: { namespace_id: fork_project.namespace, project_id: fork_project, id: commit.id } expect(response).to be_successful end @@ -174,10 +153,10 @@ RSpec.describe Projects::CommitController do go(id: commit.id, merge_request_iid: merge_request.iid) expect(assigns(:new_diff_note_attrs)).to eq({ - noteable_type: 'MergeRequest', - noteable_id: merge_request.id, - commit_id: commit.id - }) + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + commit_id: commit.id + }) expect(response).to be_ok end end @@ -187,12 +166,7 @@ RSpec.describe Projects::CommitController do it 'contains branch and tags information' do commit = project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e') - get(:branches, - params: { - namespace_id: project.namespace, - project_id: project, - id: commit.id - }) + get :branches, params: { namespace_id: project.namespace, project_id: project, id: commit.id } expect(assigns(:branches)).to include('master', 'feature_conflict') expect(assigns(:branches_limit_exceeded)).to be_falsey @@ -205,12 +179,7 @@ RSpec.describe Projects::CommitController do allow_any_instance_of(Repository).to receive(:branch_count).and_return(1001) allow_any_instance_of(Repository).to receive(:tag_count).and_return(1001) - get(:branches, - params: { - namespace_id: project.namespace, - project_id: project, - id: commit.id - }) + get :branches, params: { namespace_id: project.namespace, project_id: project, id: commit.id } expect(assigns(:branches)).to eq([]) expect(assigns(:branches_limit_exceeded)).to be_truthy @@ -234,12 +203,7 @@ RSpec.describe Projects::CommitController do describe 'POST revert' do context 'when target branch is not provided' do it 'renders the 404 page' do - post(:revert, - params: { - namespace_id: project.namespace, - project_id: project, - id: commit.id - }) + post :revert, params: { namespace_id: project.namespace, project_id: project, id: commit.id } expect(response).not_to be_successful expect(response).to have_gitlab_http_status(:not_found) @@ -248,13 +212,7 @@ RSpec.describe Projects::CommitController do context 'when the revert commit is missing' do it 'renders the 404 page' do - post(:revert, - params: { - namespace_id: project.namespace, - project_id: project, - start_branch: 'master', - id: '1234567890' - }) + post :revert, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: '1234567890' } expect(response).not_to be_successful expect(response).to have_gitlab_http_status(:not_found) @@ -263,13 +221,7 @@ RSpec.describe Projects::CommitController do context 'when the revert was successful' do it 'redirects to the commits page' do - post(:revert, - params: { - namespace_id: project.namespace, - project_id: project, - start_branch: 'master', - id: commit.id - }) + post :revert, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: commit.id } expect(response).to redirect_to project_commits_path(project, 'master') expect(flash[:notice]).to eq('The commit has been successfully reverted.') @@ -278,27 +230,53 @@ RSpec.describe Projects::CommitController do context 'when the revert failed' do before do - post(:revert, - params: { - namespace_id: project.namespace, - project_id: project, - start_branch: 'master', - id: commit.id - }) + post :revert, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: commit.id } end it 'redirects to the commit page' do # Reverting a commit that has been already reverted. - post(:revert, - params: { - namespace_id: project.namespace, - project_id: project, - start_branch: 'master', - id: commit.id - }) + post :revert, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: commit.id } expect(response).to redirect_to project_commit_path(project, commit.id) - expect(flash[:alert]).to match('Sorry, we cannot revert this commit automatically.') + expect(flash[:alert]).to match('Commit revert failed:') + end + end + + context 'in the context of a merge_request' do + let(:merge_request) { create(:merge_request, :merged, source_project: project) } + let(:repository) { project.repository } + + before do + merge_commit_id = repository.merge(user, + merge_request.diff_head_sha, + merge_request, + 'Test message') + + repository.commit(merge_commit_id) + merge_request.update!(merge_commit_sha: merge_commit_id) + end + + context 'when the revert was successful' do + it 'redirects to the merge request page' do + post :revert, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: merge_request.merge_commit_sha } + + expect(response).to redirect_to project_merge_request_path(project, merge_request) + expect(flash[:notice]).to eq('The merge request has been successfully reverted.') + end + end + + context 'when the revert failed' do + before do + post :revert, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: merge_request.merge_commit_sha } + end + + it 'redirects to the merge request page' do + # Reverting a merge request that has been already reverted. + post :revert, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: merge_request.merge_commit_sha } + + expect(response).to redirect_to project_merge_request_path(project, merge_request) + expect(flash[:alert]).to match('Merge request revert failed:') + end end end end @@ -306,12 +284,7 @@ RSpec.describe Projects::CommitController do describe 'POST cherry_pick' do context 'when target branch is not provided' do it 'renders the 404 page' do - post(:cherry_pick, - params: { - namespace_id: project.namespace, - project_id: project, - id: master_pickable_commit.id - }) + post :cherry_pick, params: { namespace_id: project.namespace, project_id: project, id: master_pickable_commit.id } expect(response).not_to be_successful expect(response).to have_gitlab_http_status(:not_found) @@ -320,13 +293,7 @@ RSpec.describe Projects::CommitController do context 'when the cherry-pick commit is missing' do it 'renders the 404 page' do - post(:cherry_pick, - params: { - namespace_id: project.namespace, - project_id: project, - start_branch: 'master', - id: '1234567890' - }) + post :cherry_pick, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: '1234567890' } expect(response).not_to be_successful expect(response).to have_gitlab_http_status(:not_found) @@ -335,13 +302,7 @@ RSpec.describe Projects::CommitController do context 'when the cherry-pick was successful' do it 'redirects to the commits page' do - post(:cherry_pick, - params: { - namespace_id: project.namespace, - project_id: project, - start_branch: 'master', - id: master_pickable_commit.id - }) + post :cherry_pick, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: master_pickable_commit.id } expect(response).to redirect_to project_commits_path(project, 'master') expect(flash[:notice]).to eq('The commit has been successfully cherry-picked into master.') @@ -350,27 +311,52 @@ RSpec.describe Projects::CommitController do context 'when the cherry_pick failed' do before do - post(:cherry_pick, - params: { - namespace_id: project.namespace, - project_id: project, - start_branch: 'master', - id: master_pickable_commit.id - }) + post :cherry_pick, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: master_pickable_commit.id } end it 'redirects to the commit page' do # Cherry-picking a commit that has been already cherry-picked. - post(:cherry_pick, - params: { - namespace_id: project.namespace, - project_id: project, - start_branch: 'master', - id: master_pickable_commit.id - }) + post :cherry_pick, params: { namespace_id: project.namespace, project_id: project, start_branch: 'master', id: master_pickable_commit.id } expect(response).to redirect_to project_commit_path(project, master_pickable_commit.id) - expect(flash[:alert]).to match('Sorry, we cannot cherry-pick this commit automatically.') + expect(flash[:alert]).to match('Commit cherry-pick failed:') + end + end + + context 'in the context of a merge_request' do + let(:merge_request) { create(:merge_request, :merged, source_project: project) } + let(:repository) { project.repository } + + before do + merge_commit_id = repository.merge(user, + merge_request.diff_head_sha, + merge_request, + 'Test message') + repository.commit(merge_commit_id) + merge_request.update!(merge_commit_sha: merge_commit_id) + end + + context 'when the cherry_pick was successful' do + it 'redirects to the merge request page' do + post :cherry_pick, params: { namespace_id: project.namespace, project_id: project, start_branch: 'merge-test', id: merge_request.merge_commit_sha } + + expect(response).to redirect_to project_merge_request_path(project, merge_request) + expect(flash[:notice]).to eq('The merge request has been successfully cherry-picked into merge-test.') + end + end + + context 'when the cherry_pick failed' do + before do + post :cherry_pick, params: { namespace_id: project.namespace, project_id: project, start_branch: 'merge-test', id: merge_request.merge_commit_sha } + end + + it 'redirects to the merge request page' do + # Reverting a merge request that has been already cherry-picked. + post :cherry_pick, params: { namespace_id: project.namespace, project_id: project, start_branch: 'merge-test', id: merge_request.merge_commit_sha } + + expect(response).to redirect_to project_merge_request_path(project, merge_request) + expect(flash[:alert]).to match('Merge request cherry-pick failed:') + end end end @@ -381,15 +367,14 @@ RSpec.describe Projects::CommitController do let(:create_merge_request) { nil } def send_request - post(:cherry_pick, - params: { - namespace_id: forked_project.namespace, - project_id: forked_project, - target_project_id: target_project.id, - start_branch: 'feature', - id: forked_project.commit.id, - create_merge_request: create_merge_request - }) + post :cherry_pick, params: { + namespace_id: forked_project.namespace, + project_id: forked_project, + target_project_id: target_project.id, + start_branch: 'feature', + id: forked_project.commit.id, + create_merge_request: create_merge_request + } end def merge_request_url(source_project, branch) @@ -458,6 +443,37 @@ RSpec.describe Projects::CommitController do end end + describe 'GET #diff_files' do + subject(:send_request) { get :diff_files, params: params } + + let(:format) { :html } + let(:params) do + { + namespace_id: project.namespace, + project_id: project, + id: commit.id, + format: format + } + end + + it 'renders diff files' do + send_request + + expect(assigns(:diffs)).to be_a(Gitlab::Diff::FileCollection::Commit) + expect(assigns(:environment)).to be_nil + end + + context 'when format is not html' do + let(:format) { :json } + + it 'returns 404 page' do + send_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'GET diff_for_path' do def diff_for_path(extra_params = {}) params = { @@ -478,8 +494,7 @@ RSpec.describe Projects::CommitController do diff_for_path(id: commit2.id, old_path: existing_path, new_path: existing_path) expect(assigns(:diff_notes_disabled)).to be_falsey - expect(assigns(:new_diff_note_attrs)).to eq(noteable_type: 'Commit', - commit_id: commit2.id) + expect(assigns(:new_diff_note_attrs)).to eq(noteable_type: 'Commit', commit_id: commit2.id) end it 'only renders the diffs for the path given' do diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb index 67aa82dacbb..956167ce838 100644 --- a/spec/controllers/projects/commits_controller_spec.rb +++ b/spec/controllers/projects/commits_controller_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' RSpec.describe Projects::CommitsController, feature_category: :source_code_management do - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:repository) { project.repository } + let_it_be(:user) { create(:user) } before do project.add_maintainer(user) @@ -18,11 +19,7 @@ RSpec.describe Projects::CommitsController, feature_category: :source_code_manag describe "GET commits_root" do context "no ref is provided" do it 'redirects to the default branch of the project' do - get(:commits_root, - params: { - namespace_id: project.namespace, - project_id: project - }) + get :commits_root, params: { namespace_id: project.namespace, project_id: project } expect(response).to redirect_to project_commits_path(project) end @@ -34,12 +31,7 @@ RSpec.describe Projects::CommitsController, feature_category: :source_code_manag context 'with file path' do before do - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: id - }) + get :show, params: { namespace_id: project.namespace, project_id: project, id: id } end context "valid branch, valid file" do @@ -48,6 +40,12 @@ RSpec.describe Projects::CommitsController, feature_category: :source_code_manag it { is_expected.to respond_with(:success) } end + context "HEAD, valid file" do + let(:id) { 'HEAD/README.md' } + + it { is_expected.to respond_with(:success) } + end + context "valid branch, invalid file" do let(:id) { 'master/invalid-path.rb' } @@ -78,13 +76,7 @@ RSpec.describe Projects::CommitsController, feature_category: :source_code_manag offset: 0 ).and_call_original - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: id, - limit: "foo" - }) + get :show, params: { namespace_id: project.namespace, project_id: project, id: id, limit: "foo" } expect(response).to be_successful end @@ -98,27 +90,44 @@ RSpec.describe Projects::CommitsController, feature_category: :source_code_manag offset: 0 ).and_call_original - get(:show, params: { + get :show, params: { namespace_id: project.namespace, project_id: project, id: id, limit: { 'broken' => 'value' } - }) + } expect(response).to be_successful end end end + it 'loads tags for commits' do + expect_next_instance_of(CommitCollection) do |collection| + expect(collection).to receive(:load_tags) + end + + get :show, params: { namespace_id: project.namespace, project_id: project, id: 'master/README.md' } + + expect(response).to have_gitlab_http_status(:ok) + end + + context 'when tag has a non-ASCII encoding' do + before do + repository.add_tag(user, 'tést', 'master') + end + + it 'does not raise an exception' do + get :show, params: { namespace_id: project.namespace, project_id: project, id: 'master' } + + expect(response).to have_gitlab_http_status(:ok) + end + end + context "when the ref name ends in .atom" do context "when the ref does not exist with the suffix" do before do - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: "master.atom" - }) + get :show, params: { namespace_id: project.namespace, project_id: project, id: "master.atom" } end it "renders as atom" do @@ -138,12 +147,11 @@ RSpec.describe Projects::CommitsController, feature_category: :source_code_manag allow_any_instance_of(Repository).to receive(:commit).and_call_original allow_any_instance_of(Repository).to receive(:commit).with('master.atom').and_return(commit) - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: "master.atom" - }) + get :show, params: { + namespace_id: project.namespace, + project_id: project, + id: "master.atom" + } end it "renders as HTML" do @@ -182,13 +190,11 @@ RSpec.describe Projects::CommitsController, feature_category: :source_code_manag before do expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original unless id.include?(' ') - get(:signatures, - params: { - namespace_id: project.namespace, - project_id: project, - id: id - }, - format: :json) + get :signatures, params: { + namespace_id: project.namespace, + project_id: project, + id: id + }, format: :json end context "valid branch" do diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 3751b89951c..a49f8b51c12 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -284,14 +284,18 @@ RSpec.describe Projects::CompareController do let(:to_ref) { '5937ac0a7beb003549fc5fd26fc247adbce4a52e' } let(:page) { 1 } - it 'shows the diff' do - show_request + shared_examples 'valid compare page' do + it 'shows the diff' do + show_request - expect(response).to be_successful - expect(assigns(:diffs).diff_files.first).to be_present - expect(assigns(:commits).length).to be >= 1 + expect(response).to be_successful + expect(assigns(:diffs).diff_files.first).to be_present + expect(assigns(:commits).length).to be >= 1 + end end + it_behaves_like 'valid compare page' + it 'only loads blobs in the current page' do stub_const('Projects::CompareController::COMMIT_DIFFS_PER_PAGE', 1) @@ -306,6 +310,19 @@ RSpec.describe Projects::CompareController do expect(response).to be_successful end + + context 'when from_ref is HEAD ref' do + let(:from_ref) { 'HEAD' } + let(:to_ref) { 'feature' } # Need to change to_ref too so there's something to compare with HEAD + + it_behaves_like 'valid compare page' + end + + context 'when to_ref is HEAD ref' do + let(:to_ref) { 'HEAD' } + + it_behaves_like 'valid compare page' + end end context 'when page is not valid' do diff --git a/spec/controllers/projects/cycle_analytics_controller_spec.rb b/spec/controllers/projects/cycle_analytics_controller_spec.rb index 034e6104f99..4ff8c21706b 100644 --- a/spec/controllers/projects/cycle_analytics_controller_spec.rb +++ b/spec/controllers/projects/cycle_analytics_controller_spec.rb @@ -15,11 +15,7 @@ RSpec.describe Projects::CycleAnalyticsController do it 'increases the counter' do expect(Gitlab::UsageDataCounters::CycleAnalyticsCounter).to receive(:count).with(:views) - get(:show, - params: { - namespace_id: project.namespace, - project_id: project - }) + get :show, params: { namespace_id: project.namespace, project_id: project } expect(response).to be_successful end @@ -35,7 +31,6 @@ RSpec.describe Projects::CycleAnalyticsController do subject { get :show, params: request_params, format: :html } let(:request_params) { { namespace_id: project.namespace, project_id: project } } - let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } let(:category) { described_class.name } let(:action) { 'perform_analytics_usage_action' } let(:namespace) { project.namespace } diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb index ec63bad22b5..52a605cf548 100644 --- a/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -276,9 +276,9 @@ RSpec.describe Projects::DeployKeysController do let(:extra_params) { {} } subject do - put :update, params: extra_params.reverse_merge(id: deploy_key.id, - namespace_id: project.namespace, - project_id: project) + put :update, params: extra_params.reverse_merge( + id: deploy_key.id, namespace_id: project.namespace, project_id: project + ) end def deploy_key_params(title, can_push) @@ -330,9 +330,7 @@ RSpec.describe Projects::DeployKeysController do context 'when a different deploy key id param is injected' do let(:extra_params) { deploy_key_params('updated title', '1') } let(:hacked_params) do - extra_params.reverse_merge(id: other_deploy_key_id, - namespace_id: project.namespace, - project_id: project) + extra_params.reverse_merge(id: other_deploy_key_id, namespace_id: project.namespace, project_id: project) end subject { put :update, params: hacked_params } diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index c6532e83441..a696eb933e9 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -210,8 +210,6 @@ RSpec.describe Projects::DeploymentsController do end def deployment_params(opts = {}) - opts.reverse_merge(namespace_id: project.namespace, - project_id: project, - environment_id: environment.id) + opts.reverse_merge(namespace_id: project.namespace, project_id: project, environment_id: environment.id) end end diff --git a/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb b/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb index 2d39e0e5317..a7f3212a6f9 100644 --- a/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb +++ b/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb @@ -80,8 +80,12 @@ RSpec.describe Projects::DesignManagement::Designs::RawImagesController do let(:oldest_version) { design.versions.ordered.last } shared_examples 'a successful request for sha' do + before do + allow(DesignManagement::GitRepository).to receive(:new).and_call_original + end + it do - expect_next_instance_of(DesignManagement::Repository) do |repository| + expect_next_instance_of(DesignManagement::GitRepository) do |repository| expect(repository).to receive(:blob_at).with(expected_ref, design.full_path).and_call_original end diff --git a/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb b/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb index 5cc6e1b1bb4..1bb5112681c 100644 --- a/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb +++ b/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb @@ -139,10 +139,13 @@ RSpec.describe Projects::DesignManagement::Designs::ResizedImageController, feat let(:sha) { newest_version.sha } before do - create(:design, :with_smaller_image_versions, - issue: create(:issue, project: project), - versions_count: 1, - versions_sha: sha) + create( + :design, + :with_smaller_image_versions, + issue: create(:issue, project: project), + versions_count: 1, + versions_sha: sha + ) end it 'serves the newest image' do diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb index 6b0c164e432..ef2d743c82f 100644 --- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb +++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb @@ -18,7 +18,7 @@ RSpec.describe Projects::Environments::PrometheusApiController do { id: proxyable.id.to_s, namespace_id: project.namespace.full_path, - project_id: project.name + project_id: project.path } end diff --git a/spec/controllers/projects/environments/sample_metrics_controller_spec.rb b/spec/controllers/projects/environments/sample_metrics_controller_spec.rb index 14e3ded76f2..b266c569edd 100644 --- a/spec/controllers/projects/environments/sample_metrics_controller_spec.rb +++ b/spec/controllers/projects/environments/sample_metrics_controller_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Projects::Environments::SampleMetricsController do { id: environment.id.to_s, namespace_id: project.namespace.full_path, - project_id: project.name, + project_id: project.path, identifier: 'sample_metric_query_result', start: '2019-12-02T23:31:45.000Z', end: '2019-12-03T00:01:45.000Z' diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 169fed1ab17..f097d08fe1b 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -15,6 +15,7 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d let!(:environment) { create(:environment, name: 'production', project: project) } before do + stub_feature_flags(remove_monitor_metrics: false) sign_in(user) end @@ -44,17 +45,9 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d allow_any_instance_of(Environment).to receive(:has_terminals?).and_return(true) allow_any_instance_of(Environment).to receive(:rollout_status).and_return(kube_deployment_rollout_status) - create(:environment, project: project, - name: 'staging/review-1', - state: :available) - - create(:environment, project: project, - name: 'staging/review-2', - state: :available) - - create(:environment, project: project, - name: 'staging/review-3', - state: :stopped) + create(:environment, project: project, name: 'staging/review-1', state: :available) + create(:environment, project: project, name: 'staging/review-2', state: :available) + create(:environment, project: project, name: 'staging/review-3', state: :stopped) end let(:environments) { json_response['environments'] } @@ -84,9 +77,7 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d it 'ignores search option if is shorter than a minimum' do get :index, params: environment_params(format: :json, search: 'st') - expect(environments.map { |env| env['name'] }).to contain_exactly('production', - 'staging/review-1', - 'staging/review-2') + expect(environments.map { |env| env['name'] }).to contain_exactly('production', 'staging/review-1', 'staging/review-2') expect(json_response['available_count']).to eq 3 expect(json_response['stopped_count']).to eq 1 end @@ -96,9 +87,7 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d get :index, params: environment_params(format: :json, search: 'review') - expect(environments.map { |env| env['name'] }).to contain_exactly('review-app', - 'staging/review-1', - 'staging/review-2') + expect(environments.map { |env| env['name'] }).to contain_exactly('review-app', 'staging/review-1', 'staging/review-2') expect(json_response['available_count']).to eq 3 expect(json_response['stopped_count']).to eq 1 end @@ -245,23 +234,18 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d context 'when using JSON format' do before do - create(:environment, project: project, - name: 'staging-1.0/review', - state: :available) - create(:environment, project: project, - name: 'staging-1.0/zzz', - state: :available) + create(:environment, project: project, name: 'staging-1.0/review', state: :available) + create(:environment, project: project, name: 'staging-1.0/zzz', state: :available) end let(:environments) { json_response['environments'] } it 'sorts the subfolders lexicographically' do get :folder, params: { - namespace_id: project.namespace, - project_id: project, - id: 'staging-1.0' - }, - format: :json + namespace_id: project.namespace, + project_id: project, + id: 'staging-1.0' + }, format: :json expect(response).to be_ok expect(response).not_to render_template 'folder' @@ -560,6 +544,18 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d expect(response).to redirect_to(project_metrics_dashboard_path(project)) end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'returns 404 not found' do + get :metrics_redirect, params: { namespace_id: project.namespace, project_id: project } + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'GET #metrics' do @@ -631,6 +627,20 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d expect(response).to redirect_to(project_metrics_dashboard_path(project, environment: environment)) end end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'returns 404 not found' do + expect(environment).not_to receive(:metrics) + + get :metrics, params: environment_params + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'GET #additional_metrics' do @@ -726,6 +736,18 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d expect(response).to have_gitlab_http_status(:ok) end end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'returns 404 not found' do + additional_metrics(window_params) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'GET #metrics_dashboard' do @@ -1016,98 +1038,8 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d end end - describe '#append_info_to_payload' do - let(:search_param) { 'my search param' } - - context 'when search_environment_logging feature is disabled' do - before do - stub_feature_flags(environments_search_logging: false) - end - - it 'does not log search params in meta.environment.search' do - expect(controller).to receive(:append_info_to_payload).and_wrap_original do |method, payload| - method.call(payload) - - expect(payload[:metadata]).not_to have_key('meta.environment.search') - expect(payload[:action]).to eq("search") - expect(payload[:controller]).to eq("Projects::EnvironmentsController") - end - - get :search, params: environment_params(format: :json, search: search_param) - end - - it 'logs params correctly when search params are missing' do - expect(controller).to receive(:append_info_to_payload).and_wrap_original do |method, payload| - method.call(payload) - - expect(payload[:metadata]).not_to have_key('meta.environment.search') - expect(payload[:action]).to eq("search") - expect(payload[:controller]).to eq("Projects::EnvironmentsController") - end - - get :search, params: environment_params(format: :json) - end - - it 'logs params correctly when search params is empty string' do - expect(controller).to receive(:append_info_to_payload).and_wrap_original do |method, payload| - method.call(payload) - - expect(payload[:metadata]).not_to have_key('meta.environment.search') - expect(payload[:action]).to eq("search") - expect(payload[:controller]).to eq("Projects::EnvironmentsController") - end - - get :search, params: environment_params(format: :json, search: "") - end - end - - context 'when search_environment_logging feature is enabled' do - before do - stub_feature_flags(environments_search_logging: true) - end - - it 'logs search params in meta.environment.search' do - expect(controller).to receive(:append_info_to_payload).and_wrap_original do |method, payload| - method.call(payload) - - expect(payload[:metadata]['meta.environment.search']).to eq(search_param) - expect(payload[:action]).to eq("search") - expect(payload[:controller]).to eq("Projects::EnvironmentsController") - end - - get :search, params: environment_params(format: :json, search: search_param) - end - - it 'logs params correctly when search params are missing' do - expect(controller).to receive(:append_info_to_payload).and_wrap_original do |method, payload| - method.call(payload) - - expect(payload[:metadata]).not_to have_key('meta.environment.search') - expect(payload[:action]).to eq("search") - expect(payload[:controller]).to eq("Projects::EnvironmentsController") - end - - get :search, params: environment_params(format: :json) - end - - it 'logs params correctly when search params is empty string' do - expect(controller).to receive(:append_info_to_payload).and_wrap_original do |method, payload| - method.call(payload) - - expect(payload[:metadata]).not_to have_key('meta.environment.search') - expect(payload[:action]).to eq("search") - expect(payload[:controller]).to eq("Projects::EnvironmentsController") - end - - get :search, params: environment_params(format: :json, search: "") - end - end - end - def environment_params(opts = {}) - opts.reverse_merge(namespace_id: project.namespace, - project_id: project, - id: environment.id) + opts.reverse_merge(namespace_id: project.namespace, project_id: project, id: environment.id) end def additional_metrics(opts = {}) diff --git a/spec/controllers/projects/feature_flags_controller_spec.rb b/spec/controllers/projects/feature_flags_controller_spec.rb index 29ad51d590f..ac2e4233709 100644 --- a/spec/controllers/projects/feature_flags_controller_spec.rb +++ b/spec/controllers/projects/feature_flags_controller_spec.rb @@ -193,8 +193,7 @@ RSpec.describe Projects::FeatureFlagsController do it 'routes based on iid' do other_project = create(:project) other_project.add_developer(user) - other_feature_flag = create(:operations_feature_flag, project: other_project, - name: 'other_flag') + other_feature_flag = create(:operations_feature_flag, project: other_project, name: 'other_flag') params = { namespace_id: other_project.namespace, project_id: other_project, @@ -485,8 +484,7 @@ RSpec.describe Projects::FeatureFlagsController do context 'when creating a version 2 feature flag with a gitlabUserList strategy' do let!(:user_list) do - create(:operations_feature_flag_user_list, project: project, - name: 'My List', user_xids: 'user1,user2') + create(:operations_feature_flag_user_list, project: project, name: 'My List', user_xids: 'user1,user2') end let(:params) do @@ -627,10 +625,7 @@ RSpec.describe Projects::FeatureFlagsController do context 'with a version 2 feature flag' do let!(:new_version_flag) do - create(:operations_feature_flag, - name: 'new-feature', - active: true, - project: project) + create(:operations_feature_flag, name: 'new-feature', active: true, project: project) end it 'creates a new strategy and scope' do diff --git a/spec/controllers/projects/find_file_controller_spec.rb b/spec/controllers/projects/find_file_controller_spec.rb index a6c71cff74b..68810bae368 100644 --- a/spec/controllers/projects/find_file_controller_spec.rb +++ b/spec/controllers/projects/find_file_controller_spec.rb @@ -18,12 +18,7 @@ RSpec.describe Projects::FindFileController do render_views before do - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: id - }) + get :show, params: { namespace_id: project.namespace, project_id: project, id: id } end context "valid branch" do @@ -41,13 +36,7 @@ RSpec.describe Projects::FindFileController do describe "GET #list" do def go(format: 'json') - get :list, - params: { - namespace_id: project.namespace, - project_id: project, - id: id - }, - format: format + get :list, params: { namespace_id: project.namespace, project_id: project, id: id }, format: format end context "valid branch" do diff --git a/spec/controllers/projects/forks_controller_spec.rb b/spec/controllers/projects/forks_controller_spec.rb index 25c722173c1..3ea7054a64c 100644 --- a/spec/controllers/projects/forks_controller_spec.rb +++ b/spec/controllers/projects/forks_controller_spec.rb @@ -168,12 +168,7 @@ RSpec.describe Projects::ForksController, feature_category: :source_code_managem let(:format) { :html } subject(:do_request) do - get :new, - format: format, - params: { - namespace_id: project.namespace, - project_id: project - } + get :new, format: format, params: { namespace_id: project.namespace, project_id: project } end context 'when user is signed in' do diff --git a/spec/controllers/projects/grafana_api_controller_spec.rb b/spec/controllers/projects/grafana_api_controller_spec.rb index 90ab49f9467..fa20fc5037f 100644 --- a/spec/controllers/projects/grafana_api_controller_spec.rb +++ b/spec/controllers/projects/grafana_api_controller_spec.rb @@ -15,6 +15,7 @@ RSpec.describe Projects::GrafanaApiController, feature_category: :metrics do end before do + stub_feature_flags(remove_monitor_metrics: false) sign_in(user) if user end @@ -23,7 +24,7 @@ RSpec.describe Projects::GrafanaApiController, feature_category: :metrics do let(:params) do { namespace_id: project.namespace.full_path, - project_id: project.name, + project_id: project.path, proxy_path: 'api/v1/query_range', datasource_id: '1', query: 'rate(relevant_metric)', @@ -87,13 +88,15 @@ RSpec.describe Projects::GrafanaApiController, feature_category: :metrics do it 'returns a grafana datasource response' do get :proxy, params: params - expect(Grafana::ProxyService) - .to have_received(:new) - .with(project, '1', 'api/v1/query_range', - { 'query' => params[:query], - 'start' => params[:start_time], - 'end' => params[:end_time], - 'step' => params[:step] }) + expect(Grafana::ProxyService).to have_received(:new).with( + project, '1', 'api/v1/query_range', + { + 'query' => params[:query], + 'start' => params[:start_time], + 'end' => params[:end_time], + 'step' => params[:step] + } + ) expect(response).to have_gitlab_http_status(:ok) expect(json_response).to eq({}) @@ -168,6 +171,14 @@ RSpec.describe Projects::GrafanaApiController, feature_category: :metrics do it_behaves_like 'accessible' end end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it_behaves_like 'not accessible' + end end describe 'GET #metrics_dashboard' do @@ -178,7 +189,7 @@ RSpec.describe Projects::GrafanaApiController, feature_category: :metrics do embedded: true, grafana_url: 'https://grafana.example.com', namespace_id: project.namespace.full_path, - project_id: project.name + project_id: project.path } end diff --git a/spec/controllers/projects/graphs_controller_spec.rb b/spec/controllers/projects/graphs_controller_spec.rb index 1e9d999311a..3e5bcbbc9ba 100644 --- a/spec/controllers/projects/graphs_controller_spec.rb +++ b/spec/controllers/projects/graphs_controller_spec.rb @@ -141,7 +141,6 @@ RSpec.describe Projects::GraphsController do end let(:request_params) { { namespace_id: project.namespace.path, project_id: project.path, id: 'master' } } - let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } let(:category) { described_class.name } let(:action) { 'perform_analytics_usage_action' } let(:namespace) { project.namespace } diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb index a5c00d24e30..2075dd3e7a7 100644 --- a/spec/controllers/projects/group_links_controller_spec.rb +++ b/spec/controllers/projects/group_links_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::GroupLinksController, feature_category: :authentication_and_authorization do +RSpec.describe Projects::GroupLinksController, feature_category: :system_access do let_it_be(:group) { create(:group, :private) } let_it_be(:group2) { create(:group, :private) } let_it_be(:project) { create(:project, :private, group: group2) } diff --git a/spec/controllers/projects/hooks_controller_spec.rb b/spec/controllers/projects/hooks_controller_spec.rb index 815370d428d..c056e7a33aa 100644 --- a/spec/controllers/projects/hooks_controller_spec.rb +++ b/spec/controllers/projects/hooks_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::HooksController do +RSpec.describe Projects::HooksController, feature_category: :integrations do include AfterNextHelpers let_it_be(:project) { create(:project) } @@ -173,6 +173,16 @@ RSpec.describe Projects::HooksController do let(:params) { { namespace_id: project.namespace, project_id: project, id: hook } } it_behaves_like 'Web hook destroyer' + + context 'when user does not have permission' do + let(:user) { create(:user, developer_projects: [project]) } + + it 'renders a 404' do + delete :destroy, params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe '#test' do diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb index 65a80b9e8ec..4502f3d7bd9 100644 --- a/spec/controllers/projects/imports_controller_spec.rb +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::ImportsController do +RSpec.describe Projects::ImportsController, feature_category: :importers do let(:user) { create(:user) } let(:project) { create(:project) } @@ -27,7 +27,7 @@ RSpec.describe Projects::ImportsController do project.add_maintainer(user) end - context 'when repository does not exists' do + context 'when repository does not exist' do it 'renders template' do get :show, params: { namespace_id: project.namespace.to_param, project_id: project } @@ -149,17 +149,7 @@ RSpec.describe Projects::ImportsController do import_state.update!(status: :started) end - context 'when group allows developers to create projects' do - let(:group) { create(:group, project_creation_level: Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) } - - it 'renders template' do - get :show, params: { namespace_id: project.namespace.to_param, project_id: project } - - expect(response).to render_template :show - end - end - - context 'when group prohibits developers to create projects' do + context 'when group prohibits developers to import projects' do let(:group) { create(:group, project_creation_level: Gitlab::Access::MAINTAINER_PROJECT_ACCESS) } it 'returns 404 response' do diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 9c272872a73..5f606b1f4f3 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -183,22 +183,10 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do let_it_be(:task) { create(:issue, :task, project: project) } shared_examples 'redirects to show work item page' do - context 'when use_iid_in_work_items_path feature flag is disabled' do - before do - stub_feature_flags(use_iid_in_work_items_path: false) - end - - it 'redirects to work item page' do - make_request - - expect(response).to redirect_to(project_work_items_path(project, task.id, query)) - end - end - it 'redirects to work item page using iid' do make_request - expect(response).to redirect_to(project_work_items_path(project, task.iid, query.merge(iid_path: true))) + expect(response).to redirect_to(project_work_items_path(project, task.iid, query)) end end @@ -255,7 +243,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do get :new, params: { namespace_id: project.namespace, project_id: project } expect(assigns(:issue)).to be_a_new(Issue) - expect(assigns(:issue).issue_type).to eq('issue') + expect(assigns(:issue).work_item_type.base_type).to eq('issue') end where(:conf_value, :conf_result) do @@ -292,7 +280,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do get :new, params: { namespace_id: project.namespace, project_id: project, issue: { issue_type: issue_type } } end - subject { assigns(:issue).issue_type } + subject { assigns(:issue).work_item_type.base_type } it { is_expected.to eq('issue') } @@ -585,15 +573,13 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do end def reorder_issue(issue, move_after_id: nil, move_before_id: nil) - put :reorder, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: issue.iid, - move_after_id: move_after_id, - move_before_id: move_before_id - }, - format: :json + put :reorder, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: issue.iid, + move_after_id: move_after_id, + move_before_id: move_before_id + }, format: :json end end @@ -601,14 +587,12 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do let(:issue_params) { { title: 'New title' } } subject do - put :update, - params: { - namespace_id: project.namespace, - project_id: project, - id: issue.to_param, - issue: issue_params - }, - format: :json + put :update, params: { + namespace_id: project.namespace, + project_id: project, + id: issue.to_param, + issue: issue_params + }, format: :json end before do @@ -635,7 +619,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do subject expect(response).to have_gitlab_http_status(:ok) - expect(issue.reload.issue_type).to eql('incident') + expect(issue.reload.work_item_type.base_type).to eq('incident') end end @@ -746,7 +730,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do go(id: issue.iid) expect(json_response).to include('title_text', 'description', 'description_text') - expect(json_response).to include('task_status', 'lock_version') + expect(json_response).to include('task_completion_status', 'lock_version') end end end @@ -1091,7 +1075,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it 'sets the correct issue_type' do issue = post_new_issue(issue_type: 'incident') - expect(issue.issue_type).to eq('incident') expect(issue.work_item_type.base_type).to eq('incident') end end @@ -1100,7 +1083,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it 'defaults to issue type' do issue = post_new_issue(issue_type: 'task') - expect(issue.issue_type).to eq('issue') expect(issue.work_item_type.base_type).to eq('issue') end end @@ -1109,7 +1091,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it 'defaults to issue type' do issue = post_new_issue(issue_type: 'objective') - expect(issue.issue_type).to eq('issue') expect(issue.work_item_type.base_type).to eq('issue') end end @@ -1118,7 +1099,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it 'defaults to issue type' do issue = post_new_issue(issue_type: 'key_result') - expect(issue.issue_type).to eq('issue') expect(issue.work_item_type.base_type).to eq('issue') end end @@ -1168,7 +1148,6 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do expect(issue).to be_a(Issue) expect(issue.persisted?).to eq(true) - expect(issue.issue_type).to eq('issue') expect(issue.work_item_type.base_type).to eq('issue') end @@ -1419,7 +1398,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do context 'setting issue type' do let(:issue_type) { 'issue' } - subject { post_new_issue(issue_type: issue_type)&.issue_type } + subject { post_new_issue(issue_type: issue_type)&.work_item_type&.base_type } it { is_expected.to eq('issue') } @@ -1484,7 +1463,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do it "deletes the issue" do delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid, destroy_confirm: true } - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:see_other) expect(controller).to set_flash[:notice].to(/The issue was successfully deleted\./) end @@ -1927,12 +1906,11 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do end it 'redirects from an old issue/designs correctly' do - get :designs, - params: { - namespace_id: project.namespace, - project_id: project, - id: issue - } + get :designs, params: { + namespace_id: project.namespace, + project_id: project, + id: issue + } expect(response).to redirect_to(designs_project_issue_path(new_project, issue)) expect(response).to have_gitlab_http_status(:moved_permanently) diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 2d047957430..ede26ebd032 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -1,11 +1,13 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, feature_category: :continuous_integration do +RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, feature_category: :continuous_integration, factory_default: :keep do include ApiHelpers include HttpIOHelpers + let_it_be(:namespace) { create_default(:namespace) } let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } let_it_be(:owner) { create(:owner) } let_it_be(:admin) { create(:admin) } let_it_be(:maintainer) { create(:user) } @@ -19,11 +21,16 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu project.add_developer(developer) project.add_reporter(reporter) project.add_guest(guest) + create_default(:owner) + create_default(:user) + create_default(:ci_trigger_request) + create_default(:ci_stage) end let(:user) { developer } - let(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be_with_reload(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:default_pipeline) { create_default(:ci_pipeline) } before do stub_feature_flags(ci_enable_live_trace: true) @@ -106,9 +113,10 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu def create_job(name, status) user = create(:user) pipeline = create(:ci_pipeline, project: project, user: user) - create(:ci_build, :tags, :triggered, :artifacts, - pipeline: pipeline, name: name, status: status, - user: user) + create( + :ci_build, :tags, :triggered, :artifacts, + pipeline: pipeline, name: name, status: status, user: user + ) end end @@ -151,7 +159,6 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu end context 'when requesting JSON' do - let(:merge_request) { create(:merge_request, source_project: project) } let(:user) { developer } before do @@ -210,9 +217,9 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu end context 'when job has artifacts' do - context 'with not expiry date' do - let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } + let_it_be(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } + context 'with not expiry date' do context 'when artifacts are unlocked' do before do job.pipeline.unlocked! @@ -233,7 +240,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu context 'when artifacts are locked' do before do - job.pipeline.artifacts_locked! + job.pipeline.reload.artifacts_locked! end it 'exposes needed information' do @@ -251,11 +258,13 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu end context 'with expired artifacts' do - let(:job) { create(:ci_build, :success, :artifacts, :expired, pipeline: pipeline) } + before do + job.update!(artifacts_expire_at: 1.minute.ago) + end context 'when artifacts are unlocked' do before do - job.pipeline.unlocked! + job.pipeline.reload.unlocked! end it 'exposes needed information' do @@ -274,7 +283,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu context 'when artifacts are locked' do before do - job.pipeline.artifacts_locked! + job.pipeline.reload.artifacts_locked! end it 'exposes needed information' do @@ -291,19 +300,17 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu end end end - end - - context 'when job passed with no trace' do - let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } - it 'exposes empty state illustrations' do - get_show_json + context 'when job passed with no trace' do + it 'exposes empty state illustrations' do + get_show_json - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('job/job_details') - expect(json_response['status']['illustration']).to have_key('image') - expect(json_response['status']['illustration']).to have_key('size') - expect(json_response['status']['illustration']).to have_key('title') + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['status']['illustration']).to have_key('image') + expect(json_response['status']['illustration']).to have_key('size') + expect(json_response['status']['illustration']).to have_key('title') + end end end @@ -319,7 +326,6 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu end context 'with deployment' do - let(:merge_request) { create(:merge_request, source_project: project) } let(:environment) { create(:environment, project: project, name: 'staging', state: :available) } let(:job) { create(:ci_build, :running, environment: environment.name, pipeline: pipeline) } @@ -511,7 +517,6 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu end context 'when requesting triggered job JSON' do - let!(:merge_request) { create(:merge_request, source_project: project) } let(:trigger) { create(:ci_trigger, project: project) } let(:trigger_request) { create(:ci_trigger_request, pipeline: pipeline, trigger: trigger) } let(:job) { create(:ci_build, pipeline: pipeline, trigger_request: trigger_request) } @@ -832,8 +837,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu retried_build = Ci::Build.last Ci::Build.clone_accessors.each do |accessor| - expect(job.read_attribute(accessor)) - .to eq(retried_build.read_attribute(accessor)), + expect(job.read_attribute(accessor)).to eq(retried_build.read_attribute(accessor)), "Mismatched attribute on \"#{accessor}\". " \ "It was \"#{job.read_attribute(accessor)}\" but changed to \"#{retried_build.read_attribute(accessor)}\"" end @@ -855,10 +859,10 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu def post_retry post :retry, params: { - namespace_id: project.namespace, - project_id: project, - id: job.id - } + namespace_id: project.namespace, + project_id: project, + id: job.id + } end end @@ -869,8 +873,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu before do project.add_developer(user) - create(:protected_branch, :developers_can_merge, - name: 'protected-branch', project: project) + create(:protected_branch, :developers_can_merge, name: 'protected-branch', project: project) sign_in(user) end diff --git a/spec/controllers/projects/mattermosts_controller_spec.rb b/spec/controllers/projects/mattermosts_controller_spec.rb index 19a04654114..b5092a0f091 100644 --- a/spec/controllers/projects/mattermosts_controller_spec.rb +++ b/spec/controllers/projects/mattermosts_controller_spec.rb @@ -19,11 +19,10 @@ RSpec.describe Projects::MattermostsController do end it 'accepts the request' do - get(:new, - params: { - namespace_id: project.namespace.to_param, - project_id: project - }) + get :new, params: { + namespace_id: project.namespace.to_param, + project_id: project + } expect(response).to have_gitlab_http_status(:ok) end @@ -33,12 +32,11 @@ RSpec.describe Projects::MattermostsController do let(:mattermost_params) { { trigger: 'http://localhost:3000/trigger', team_id: 'abc' } } subject do - post(:create, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - mattermost: mattermost_params - }) + post :create, params: { + namespace_id: project.namespace.to_param, + project_id: project, + mattermost: mattermost_params + } end context 'no request can be made to mattermost' do diff --git a/spec/controllers/projects/merge_requests/conflicts_controller_spec.rb b/spec/controllers/projects/merge_requests/conflicts_controller_spec.rb index 311af26abf6..926cd7ea681 100644 --- a/spec/controllers/projects/merge_requests/conflicts_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/conflicts_controller_spec.rb @@ -22,13 +22,11 @@ RSpec.describe Projects::MergeRequests::ConflictsController do allow(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) .to receive(:track_loading_conflict_ui_action) - get :show, - params: { - namespace_id: merge_request_with_conflicts.project.namespace.to_param, - project_id: merge_request_with_conflicts.project, - id: merge_request_with_conflicts.iid - }, - format: 'html' + get :show, params: { + namespace_id: merge_request_with_conflicts.project.namespace.to_param, + project_id: merge_request_with_conflicts.project, + id: merge_request_with_conflicts.iid + }, format: 'html' end it 'does tracks the resolve call' do @@ -45,13 +43,11 @@ RSpec.describe Projects::MergeRequests::ConflictsController do allow(Gitlab::Git::Conflict::Parser).to receive(:parse) .and_raise(Gitlab::Git::Conflict::Parser::UnmergeableFile) - get :show, - params: { - namespace_id: merge_request_with_conflicts.project.namespace.to_param, - project_id: merge_request_with_conflicts.project, - id: merge_request_with_conflicts.iid - }, - format: 'json' + get :show, params: { + namespace_id: merge_request_with_conflicts.project.namespace.to_param, + project_id: merge_request_with_conflicts.project, + id: merge_request_with_conflicts.iid + }, format: 'json' end it 'returns a 200 status code' do @@ -70,13 +66,11 @@ RSpec.describe Projects::MergeRequests::ConflictsController do context 'with valid conflicts' do before do - get :show, - params: { - namespace_id: merge_request_with_conflicts.project.namespace.to_param, - project_id: merge_request_with_conflicts.project, - id: merge_request_with_conflicts.iid - }, - format: 'json' + get :show, params: { + namespace_id: merge_request_with_conflicts.project.namespace.to_param, + project_id: merge_request_with_conflicts.project, + id: merge_request_with_conflicts.iid + }, format: 'json' end it 'matches the schema' do @@ -91,7 +85,7 @@ RSpec.describe Projects::MergeRequests::ConflictsController do end it 'includes each file that has conflicts' do - filenames = json_response['files'].map { |file| file['new_path'] } + filenames = json_response['files'].pluck('new_path') expect(filenames).to contain_exactly('files/ruby/popen.rb', 'files/ruby/regex.rb') end @@ -120,7 +114,7 @@ RSpec.describe Projects::MergeRequests::ConflictsController do it 'has unique section IDs across files' do section_ids = json_response['files'].flat_map do |file| - file['sections'].map { |section| section['id'] }.compact + file['sections'].pluck('id').compact end expect(section_ids.uniq).to eq(section_ids) @@ -130,15 +124,13 @@ RSpec.describe Projects::MergeRequests::ConflictsController do describe 'GET conflict_for_path' do def conflict_for_path(path) - get :conflict_for_path, - params: { - namespace_id: merge_request_with_conflicts.project.namespace.to_param, - project_id: merge_request_with_conflicts.project, - id: merge_request_with_conflicts.iid, - old_path: path, - new_path: path - }, - format: 'json' + get :conflict_for_path, params: { + namespace_id: merge_request_with_conflicts.project.namespace.to_param, + project_id: merge_request_with_conflicts.project, + id: merge_request_with_conflicts.iid, + old_path: path, + new_path: path + }, format: 'json' end context 'when the conflicts cannot be resolved in the UI' do @@ -178,11 +170,13 @@ RSpec.describe Projects::MergeRequests::ConflictsController do aggregate_failures do expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include('old_path' => path, - 'new_path' => path, - 'blob_icon' => 'doc-text', - 'blob_path' => a_string_ending_with(path), - 'content' => content) + expect(json_response).to include( + 'old_path' => path, + 'new_path' => path, + 'blob_icon' => 'doc-text', + 'blob_path' => a_string_ending_with(path), + 'content' => content + ) end end end @@ -197,15 +191,13 @@ RSpec.describe Projects::MergeRequests::ConflictsController do end def resolve_conflicts(files) - post :resolve_conflicts, - params: { - namespace_id: merge_request_with_conflicts.project.namespace.to_param, - project_id: merge_request_with_conflicts.project, - id: merge_request_with_conflicts.iid, - files: files, - commit_message: 'Commit message' - }, - format: 'json' + post :resolve_conflicts, params: { + namespace_id: merge_request_with_conflicts.project.namespace.to_param, + project_id: merge_request_with_conflicts.project, + id: merge_request_with_conflicts.iid, + files: files, + commit_message: 'Commit message' + }, format: 'json' end context 'with valid params' do diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index 3d4a884587f..c6a4dcbfdf0 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -99,9 +99,7 @@ RSpec.describe Projects::MergeRequests::CreationsController, feature_category: : describe 'GET pipelines' do before do - create(:ci_pipeline, sha: fork_project.commit('remove-submodule').id, - ref: 'remove-submodule', - project: fork_project) + create(:ci_pipeline, sha: fork_project.commit('remove-submodule').id, ref: 'remove-submodule', project: fork_project) end it 'renders JSON including serialized pipelines' do @@ -188,13 +186,12 @@ RSpec.describe Projects::MergeRequests::CreationsController, feature_category: : expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true } expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { true }.at_least(:once) - get :branch_to, - params: { - namespace_id: fork_project.namespace, - project_id: fork_project, - target_project_id: project.id, - ref: 'master' - } + get :branch_to, params: { + namespace_id: fork_project.namespace, + project_id: fork_project, + target_project_id: project.id, + ref: 'master' + } expect(assigns(:commit)).not_to be_nil expect(response).to have_gitlab_http_status(:ok) @@ -204,13 +201,12 @@ RSpec.describe Projects::MergeRequests::CreationsController, feature_category: : expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true } expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { false }.at_least(:once) - get :branch_to, - params: { - namespace_id: fork_project.namespace, - project_id: fork_project, - target_project_id: project.id, - ref: 'master' - } + get :branch_to, params: { + namespace_id: fork_project.namespace, + project_id: fork_project, + target_project_id: project.id, + ref: 'master' + } expect(assigns(:commit)).to be_nil expect(response).to have_gitlab_http_status(:ok) @@ -220,13 +216,12 @@ RSpec.describe Projects::MergeRequests::CreationsController, feature_category: : expect(Ability).to receive(:allowed?).with(user, :read_project, project) { false } expect(Ability).to receive(:allowed?).with(user, :create_merge_request_in, project) { true }.at_least(:once) - get :branch_to, - params: { - namespace_id: fork_project.namespace, - project_id: fork_project, - target_project_id: project.id, - ref: 'master' - } + get :branch_to, params: { + namespace_id: fork_project.namespace, + project_id: fork_project, + target_project_id: project.id, + ref: 'master' + } expect(assigns(:commit)).to be_nil expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 23a33d7e0b1..3b562b4c151 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -247,9 +247,11 @@ RSpec.describe Projects::MergeRequests::DiffsController, feature_category: :code straight: true) end - go(diff_head: true, - diff_id: merge_request.merge_request_diff.id, - start_sha: merge_request.merge_request_diff.start_commit_sha) + go( + diff_head: true, + diff_id: merge_request.merge_request_diff.id, + start_sha: merge_request.merge_request_diff.start_commit_sha + ) end end end @@ -329,15 +331,17 @@ RSpec.describe Projects::MergeRequests::DiffsController, feature_category: :code diff_for_path(old_path: existing_path, new_path: existing_path) expect(assigns(:diff_notes_disabled)).to be_falsey - expect(assigns(:new_diff_note_attrs)).to eq(noteable_type: 'MergeRequest', - noteable_id: merge_request.id, - commit_id: nil) + expect(assigns(:new_diff_note_attrs)).to eq( + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + commit_id: nil + ) end it 'only renders the diffs for the path given' do diff_for_path(old_path: existing_path, new_path: existing_path) - paths = json_response['diff_files'].map { |file| file['new_path'] } + paths = json_response['diff_files'].pluck('new_path') expect(paths).to include(existing_path) end @@ -528,8 +532,7 @@ RSpec.describe Projects::MergeRequests::DiffsController, feature_category: :code context 'with diff_id and start_sha params' do subject do - go(diff_id: merge_request.merge_request_diff.id, - start_sha: merge_request.merge_request_diff.start_commit_sha) + go(diff_id: merge_request.merge_request_diff.id, start_sha: merge_request.merge_request_diff.start_commit_sha) end it_behaves_like 'serializes diffs with expected arguments' do diff --git a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb index 39482938a8b..6632473a85c 100644 --- a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb @@ -299,8 +299,7 @@ RSpec.describe Projects::MergeRequests::DraftsController do it 'publishes a draft note with quick actions and applies them', :sidekiq_inline do project.add_developer(user2) - create(:draft_note, merge_request: merge_request, author: user, - note: "/assign #{user2.to_reference}") + create(:draft_note, merge_request: merge_request, author: user, note: "/assign #{user2.to_reference}") expect(merge_request.assignees).to be_empty @@ -350,12 +349,13 @@ RSpec.describe Projects::MergeRequests::DraftsController do let(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } def create_reply(discussion_id, resolves: false) - create(:draft_note, - merge_request: merge_request, - author: user, - discussion_id: discussion_id, - resolve_discussion: resolves - ) + create( + :draft_note, + merge_request: merge_request, + author: user, + discussion_id: discussion_id, + resolve_discussion: resolves + ) end it 'resolves a thread if the draft note resolves it' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index ceb3f803db5..f78d50bba24 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -210,9 +210,7 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review diff = merge_request.merge_request_diff diff.clean! - diff.update!(real_size: nil, - start_commit_sha: nil, - base_commit_sha: nil) + diff.update!(real_size: nil, start_commit_sha: nil, base_commit_sha: nil) go(format: :html) @@ -270,24 +268,22 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review end it 'redirects from an old merge request correctly' do - get :show, - params: { - namespace_id: project.namespace, - project_id: project, - id: merge_request - } + get :show, params: { + namespace_id: project.namespace, + project_id: project, + id: merge_request + } expect(response).to redirect_to(project_merge_request_path(new_project, merge_request)) expect(response).to have_gitlab_http_status(:moved_permanently) end it 'redirects from an old merge request commits correctly' do - get :commits, - params: { - namespace_id: project.namespace, - project_id: project, - id: merge_request - } + get :commits, params: { + namespace_id: project.namespace, + project_id: project, + id: merge_request + } expect(response).to redirect_to(commits_project_merge_request_path(new_project, merge_request)) expect(response).to have_gitlab_http_status(:moved_permanently) @@ -385,13 +381,12 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } def get_merge_requests(page = nil) - get :index, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - state: 'opened', - page: page.to_param - } + get :index, params: { + namespace_id: project.namespace.to_param, + project_id: project, + state: 'opened', + page: page.to_param + } end it_behaves_like "issuables list meta-data", :merge_request @@ -580,6 +575,16 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review it 'returns :failed' do expect(json_response).to eq('status' => 'failed') end + + context 'for logging' do + let(:expected_params) { { merge_action_status: 'failed' } } + let(:subject_proc) { proc { subject } } + + subject { post :merge, params: base_params } + + it_behaves_like 'storing arguments in the application context' + it_behaves_like 'not executing any extra queries for the application context' + end end context 'when the sha parameter does not match the source SHA' do @@ -590,6 +595,16 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review it 'returns :sha_mismatch' do expect(json_response).to eq('status' => 'sha_mismatch') end + + context 'for logging' do + let(:expected_params) { { merge_action_status: 'sha_mismatch' } } + let(:subject_proc) { proc { subject } } + + subject { post :merge, params: base_params.merge(sha: 'foo') } + + it_behaves_like 'storing arguments in the application context' + it_behaves_like 'not executing any extra queries for the application context' + end end context 'when the sha parameter matches the source SHA' do @@ -611,6 +626,16 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review merge_with_sha end + context 'for logging' do + let(:expected_params) { { merge_action_status: 'success' } } + let(:subject_proc) { proc { subject } } + + subject { merge_with_sha } + + it_behaves_like 'storing arguments in the application context' + it_behaves_like 'not executing any extra queries for the application context' + end + context 'when squash is passed as 1' do it 'updates the squash attribute on the MR to true' do merge_request.update!(squash: false) @@ -678,6 +703,16 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review merge_when_pipeline_succeeds end + context 'for logging' do + let(:expected_params) { { merge_action_status: 'merge_when_pipeline_succeeds' } } + let(:subject_proc) { proc { subject } } + + subject { merge_when_pipeline_succeeds } + + it_behaves_like 'storing arguments in the application context' + it_behaves_like 'not executing any extra queries for the application context' + end + context 'when project.only_allow_merge_if_pipeline_succeeds? is true' do before do project.update_column(:only_allow_merge_if_pipeline_succeeds, true) @@ -816,7 +851,7 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review it "deletes the merge request" do delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: merge_request.iid, destroy_confirm: true } - expect(response).to have_gitlab_http_status(:found) + expect(response).to have_gitlab_http_status(:see_other) expect(controller).to set_flash[:notice].to(/The merge request was successfully deleted\./) end @@ -842,15 +877,13 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review describe 'GET commits' do def go(page: nil, per_page: 1, format: 'html') - get :commits, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid, - page: page, - per_page: per_page - }, - format: format + get :commits, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid, + page: page, + per_page: per_page + }, format: format end it 'renders the commits template to a string' do @@ -884,17 +917,18 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review describe 'GET pipelines' do before do - create(:ci_pipeline, project: merge_request.source_project, - ref: merge_request.source_branch, - sha: merge_request.diff_head_sha) + create( + :ci_pipeline, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha + ) - get :pipelines, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid - }, - format: :json + get :pipelines, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid + }, format: :json end context 'with "enabled" builds on a public project' do @@ -1955,17 +1989,18 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review let(:issue2) { create(:issue, project: project) } def post_assign_issues - merge_request.update!(description: "Closes #{issue1.to_reference} and #{issue2.to_reference}", - author: user, - source_branch: 'feature', - target_branch: 'master') + merge_request.update!( + description: "Closes #{issue1.to_reference} and #{issue2.to_reference}", + author: user, + source_branch: 'feature', + target_branch: 'master' + ) - post :assign_related_issues, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid - } + post :assign_related_issues, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid + } end it 'displays an flash error message on fail' do @@ -2143,10 +2178,13 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review describe 'GET pipeline_status.json' do context 'when head_pipeline exists' do let!(:pipeline) do - create(:ci_pipeline, project: merge_request.source_project, - ref: merge_request.source_branch, - sha: merge_request.diff_head_sha, - head_pipeline_of: merge_request) + create( + :ci_pipeline, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha, + head_pipeline_of: merge_request + ) end let(:status) { pipeline.detailed_status(double('user')) } @@ -2199,11 +2237,10 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :code_review def get_pipeline_status get :pipeline_status, params: { - namespace_id: project.namespace, - project_id: project, - id: merge_request.iid - }, - format: :json + namespace_id: project.namespace, + project_id: project, + id: merge_request.iid + }, format: :json end end diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 28da7eff8fc..e2b73e55145 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -156,6 +156,27 @@ RSpec.describe Projects::MilestonesController do end end + describe "#update" do + let(:milestone_params) do + { title: "title changed" } + end + + it "handles ActiveRecord::StaleObjectError" do + # Purposely reduce the lock_version to trigger an ActiveRecord::StaleObjectError + milestone_params[:lock_version] = milestone.lock_version - 1 + + put :update, params: { + id: milestone.iid, + milestone: milestone_params, + namespace_id: project.namespace.id, + project_id: project.id + } + + expect(response).not_to redirect_to(project_milestone_path(project, milestone.iid)) + expect(response).to render_template(:edit) + end + end + describe "#destroy" do it "removes milestone" do expect(issue.milestone_id).to eq(milestone.id) diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 23b0b58158f..5e4e47be2c5 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -37,6 +37,8 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : project.add_developer(user) end + specify { expect(get(:index, params: request_params)).to have_request_urgency(:medium) } + it 'passes last_fetched_at from headers to NotesFinder and MergeIntoNotesService' do last_fetched_at = Time.zone.at(3.hours.ago.to_i) # remove nanoseconds @@ -244,6 +246,8 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : sign_in(user) end + specify { expect(create!).to have_request_urgency(:low) } + describe 'making the creation request' do before do create! @@ -432,6 +436,13 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : expect(json_response['commands_changes']).to include('emoji_award', 'time_estimate', 'spend_time') expect(json_response['commands_changes']).not_to include('target_project', 'title') end + + it 'includes command_names' do + create! + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['command_names']).to include('award', 'estimate', 'spend') + end end context 'with commands that do not return changes' do @@ -450,6 +461,13 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : expect(response).to have_gitlab_http_status(:ok) expect(json_response['commands_changes']).not_to include('target_project', 'title') end + + it 'includes command_names' do + create! + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['command_names']).to include('move', 'title') + end end end end @@ -484,10 +502,7 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : let(:commit) { create(:commit, project: project) } let(:existing_comment) do - create(:note_on_commit, - note: 'first', - project: project, - commit_id: merge_request.commit_shas.first) + create(:note_on_commit, note: 'first', project: project, commit_id: merge_request.commit_shas.first) end let(:discussion) { existing_comment.discussion } @@ -735,19 +750,21 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : end describe 'PUT update' do - context "should update the note with a valid issue" do - let(:request_params) do - { - namespace_id: project.namespace, - project_id: project, - id: note, - format: :json, - note: { - note: "New comment" - } + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + id: note, + format: :json, + note: { + note: "New comment" } - end + } + end + + specify { expect(put(:update, params: request_params)).to have_request_urgency(:low) } + context "should update the note with a valid issue" do before do sign_in(note.author) project.add_developer(note.author) @@ -793,6 +810,8 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : } end + specify { expect(delete(:destroy, params: request_params)).to have_request_urgency(:low) } + context 'user is the author of a note' do before do sign_in(note.author) @@ -834,6 +853,8 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : let(:emoji_name) { 'thumbsup' } + it { is_expected.to have_request_urgency(:low) } + it "toggles the award emoji" do expect do subject @@ -869,6 +890,8 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : sign_in user end + specify { expect(post(:resolve, params: request_params)).to have_request_urgency(:low) } + context "when the user is not authorized to resolve the note" do it "returns status 404" do post :resolve, params: request_params @@ -932,6 +955,8 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : note.resolve!(user) end + specify { expect(delete(:unresolve, params: request_params)).to have_request_urgency(:low) } + context "when the user is not authorized to resolve the note" do it "returns status 404" do delete :unresolve, params: request_params @@ -1001,6 +1026,8 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : expect(json_response.count).to eq(1) expect(json_response.first).to include({ "line_text" => "Test" }) end + + specify { expect(get(:outdated_line_change, params: request_params)).to have_request_urgency(:low) } end # Convert a time to an integer number of microseconds diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb index 136f98ac907..ded5dd57e3e 100644 --- a/spec/controllers/projects/pages_controller_spec.rb +++ b/spec/controllers/projects/pages_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::PagesController do +RSpec.describe Projects::PagesController, feature_category: :pages do let(:user) { create(:user) } let(:project) { create(:project, :public) } @@ -14,7 +14,12 @@ RSpec.describe Projects::PagesController do end before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + stub_config(pages: { + enabled: true, + external_https: true, + access_control: false + }) + sign_in(user) project.add_maintainer(user) end @@ -123,49 +128,99 @@ RSpec.describe Projects::PagesController do end describe 'PATCH update' do - let(:request_params) do - { - namespace_id: project.namespace, - project_id: project, - project: { pages_https_only: 'false' } - } - end + context 'when updating pages_https_only' do + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + project: { pages_https_only: 'true' } + } + end - let(:update_service) { double(execute: { status: :success }) } + it 'updates project field and redirects back to the pages settings' do + project.update!(pages_https_only: false) - before do - allow(Projects::UpdateService).to receive(:new) { update_service } - end + expect { patch :update, params: request_params } + .to change { project.reload.pages_https_only } + .from(false).to(true) - it 'returns 302 status' do - patch :update, params: request_params + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(project_pages_path(project)) + end - expect(response).to have_gitlab_http_status(:found) - end + context 'when it fails to update' do + it 'adds an error message' do + expect_next_instance_of(Projects::UpdateService) do |service| + expect(service) + .to receive(:execute) + .and_return(status: :error, message: 'some error happened') + end - it 'redirects back to the pages settings' do - patch :update, params: request_params + expect { patch :update, params: request_params } + .not_to change { project.reload.pages_https_only } - expect(response).to redirect_to(project_pages_path(project)) + expect(response).to redirect_to(project_pages_path(project)) + expect(flash[:alert]).to eq('some error happened') + end + end end - it 'calls the update service' do - expect(Projects::UpdateService) - .to receive(:new) - .with(project, user, ActionController::Parameters.new(request_params[:project]).permit!) - .and_return(update_service) + context 'when updating pages_unique_domain' do + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + project: { + project_setting_attributes: { + pages_unique_domain_enabled: 'true' + } + } + } + end - patch :update, params: request_params - end + before do + create(:project_setting, project: project, pages_unique_domain_enabled: false) + end - context 'when update_service returns an error message' do - let(:update_service) { double(execute: { status: :error, message: 'some error happened' }) } + context 'with pages_unique_domain feature flag disabled' do + it 'does not update pages unique domain' do + stub_feature_flags(pages_unique_domain: false) - it 'adds an error message' do - patch :update, params: request_params + expect { patch :update, params: request_params } + .not_to change { project.project_setting.reload.pages_unique_domain_enabled } + end + end - expect(response).to redirect_to(project_pages_path(project)) - expect(flash[:alert]).to eq('some error happened') + context 'with pages_unique_domain feature flag enabled' do + before do + stub_feature_flags(pages_unique_domain: true) + end + + it 'updates pages_https_only and pages_unique_domain and redirects back to pages settings' do + expect { patch :update, params: request_params } + .to change { project.project_setting.reload.pages_unique_domain_enabled } + .from(false).to(true) + + expect(project.project_setting.pages_unique_domain).not_to be_nil + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(project_pages_path(project)) + end + + context 'when it fails to update' do + it 'adds an error message' do + expect_next_instance_of(Projects::UpdateService) do |service| + expect(service) + .to receive(:execute) + .and_return(status: :error, message: 'some error happened') + end + + expect { patch :update, params: request_params } + .not_to change { project.project_setting.reload.pages_unique_domain_enabled } + + expect(response).to redirect_to(project_pages_path(project)) + expect(flash[:alert]).to eq('some error happened') + end + end end end end diff --git a/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb b/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb index 939366e5b0b..02407e31756 100644 --- a/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb +++ b/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb @@ -2,11 +2,11 @@ require 'spec_helper' -RSpec.describe Projects::PerformanceMonitoring::DashboardsController do +RSpec.describe Projects::PerformanceMonitoring::DashboardsController, feature_category: :metrics do let_it_be(:user) { create(:user) } let_it_be(:namespace) { create(:namespace) } - let!(:project) { create(:project, :repository, name: 'dashboard-project', namespace: namespace) } + let_it_be(:project) { create(:project, :repository, namespace: namespace) } let(:repository) { project.repository } let(:branch) { double(name: branch_name) } let(:commit_message) { 'test' } @@ -25,6 +25,10 @@ RSpec.describe Projects::PerformanceMonitoring::DashboardsController do } end + before do + stub_feature_flags(remove_monitor_metrics: false) + end + describe 'POST #create' do context 'authenticated user' do before do @@ -64,7 +68,7 @@ RSpec.describe Projects::PerformanceMonitoring::DashboardsController do post :create, params: params expect(response).to have_gitlab_http_status :created - expect(controller).to set_flash[:notice].to eq("Your dashboard has been copied. You can <a href=\"/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}\">edit it here</a>.") + expect(controller).to set_flash[:notice].to eq("Your dashboard has been copied. You can <a href=\"/-/ide/project/#{project.full_path}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}\">edit it here</a>.") expect(json_response).to eq('status' => 'success', 'dashboard' => { 'path' => ".gitlab/dashboards/#{file_name}" }) end @@ -102,6 +106,18 @@ RSpec.describe Projects::PerformanceMonitoring::DashboardsController do expect(json_response).to eq('error' => "Request parameter branch is missing.") end end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'returns 404 not found' do + post :create, params: params + + expect(response).to have_gitlab_http_status :not_found + end + end end end end @@ -120,7 +136,7 @@ RSpec.describe Projects::PerformanceMonitoring::DashboardsController do end context 'project without repository feature' do - let!(:project) { create(:project, name: 'dashboard-project', namespace: namespace) } + let_it_be(:project) { create(:project, namespace: namespace) } it 'responds with :not_found status code' do post :create, params: params @@ -203,7 +219,7 @@ RSpec.describe Projects::PerformanceMonitoring::DashboardsController do put :update, params: params expect(response).to have_gitlab_http_status :created - expect(controller).to set_flash[:notice].to eq("Your dashboard has been updated. You can <a href=\"/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}\">edit it here</a>.") + expect(controller).to set_flash[:notice].to eq("Your dashboard has been updated. You can <a href=\"/-/ide/project/#{project.full_path}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}\">edit it here</a>.") expect(json_response).to eq('status' => 'success', 'dashboard' => { 'default' => false, 'display_name' => "custom_dashboard.yml", 'path' => ".gitlab/dashboards/#{file_name}", 'system_dashboard' => false }) end @@ -217,6 +233,18 @@ RSpec.describe Projects::PerformanceMonitoring::DashboardsController do expect(json_response).to eq('error' => 'something went wrong') end end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'returns 404 not found' do + put :update, params: params + + expect(response).to have_gitlab_http_status :not_found + end + end end end @@ -246,7 +274,7 @@ RSpec.describe Projects::PerformanceMonitoring::DashboardsController do end context 'project without repository feature' do - let!(:project) { create(:project, name: 'dashboard-project', namespace: namespace) } + let_it_be(:project) { create(:project, namespace: namespace) } it 'responds with :not_found status code' do put :update, params: params diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index a628c1ab230..6d810fdcd51 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -410,9 +410,9 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu it { expect { go }.to be_denied_for(:visitor) } context 'when user is schedule owner' do - it { expect { go }.to be_denied_for(:owner).of(project).own(pipeline_schedule) } - it { expect { go }.to be_denied_for(:maintainer).of(project).own(pipeline_schedule) } - it { expect { go }.to be_denied_for(:developer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_allowed_for(:owner).of(project).own(pipeline_schedule) } + it { expect { go }.to be_allowed_for(:maintainer).of(project).own(pipeline_schedule) } + it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) } it { expect { go }.to be_denied_for(:reporter).of(project).own(pipeline_schedule) } it { expect { go }.to be_denied_for(:guest).of(project).own(pipeline_schedule) } it { expect { go }.to be_denied_for(:user).own(pipeline_schedule) } diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 4e0c098ad81..8c5f8fc6259 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -203,18 +203,16 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte def get_pipelines_index_html(params = {}) get :index, params: { - namespace_id: project.namespace, - project_id: project - }.merge(params), - format: :html + namespace_id: project.namespace, + project_id: project + }.merge(params), format: :html end def get_pipelines_index_json(params = {}) get :index, params: { - namespace_id: project.namespace, - project_id: project - }.merge(params), - format: :json + namespace_id: project.namespace, + project_id: project + }.merge(params), format: :json end def create_all_pipeline_types @@ -236,12 +234,15 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte def create_pipeline(status, sha, merge_request: nil) user = create(:user) - pipeline = create(:ci_empty_pipeline, status: status, - project: project, - sha: sha.id, - ref: sha.id.first(8), - user: user, - merge_request: merge_request) + pipeline = create( + :ci_empty_pipeline, + status: status, + project: project, + sha: sha.id, + ref: sha.id.first(8), + user: user, + merge_request: merge_request + ) build_stage = create(:ci_stage, name: 'build', pipeline: pipeline) test_stage = create(:ci_stage, name: 'test', pipeline: pipeline) @@ -279,23 +280,6 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte end end - describe 'GET #index' do - before do - stub_application_setting(auto_devops_enabled: false) - end - - context 'with runners_availability_section experiment' do - it 'tracks the assignment', :experiment do - stub_experiments(runners_availability_section: true) - - expect(experiment(:runners_availability_section)) - .to track(:assignment).with_context(namespace: project.namespace).on_next_instance - - get :index, params: { namespace_id: project.namespace, project_id: project } - end - end - end - describe 'GET #show' do def get_pipeline_html get :show, params: { namespace_id: project.namespace, project_id: project, id: pipeline }, format: :html @@ -378,9 +362,7 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte let(:project) { create(:project, :repository) } let(:pipeline) do - create(:ci_empty_pipeline, project: project, - user: user, - sha: project.commit.id) + create(:ci_empty_pipeline, project: project, user: user, sha: project.commit.id) end let(:build_stage) { create(:ci_stage, name: 'build', pipeline: pipeline) } @@ -598,9 +580,7 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte def create_pipeline(project) create(:ci_empty_pipeline, project: project).tap do |pipeline| - create(:ci_build, pipeline: pipeline, - ci_stage: create(:ci_stage, name: 'test', pipeline: pipeline), - name: 'rspec') + create(:ci_build, pipeline: pipeline, ci_stage: create(:ci_stage, name: 'test', pipeline: pipeline), name: 'rspec') end end @@ -771,11 +751,8 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte before do get :status, params: { - namespace_id: project.namespace, - project_id: project, - id: pipeline.id - }, - format: :json + namespace_id: project.namespace, project_id: project, id: pipeline.id + }, format: :json end it 'return a detailed pipeline status in json' do @@ -825,7 +802,6 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte subject { get :charts, params: request_params, format: :html } let(:request_params) { { namespace_id: project.namespace, project_id: project, id: pipeline.id, chart: tab[:chart_param] } } - let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } let(:category) { described_class.name } let(:action) { 'perform_analytics_usage_action' } let(:namespace) { project.namespace } @@ -868,9 +844,7 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte context 'when latest commit contains [ci skip]' do before do - project.repository.create_file(user, 'new-file.txt', 'A new file', - message: '[skip ci] This is a test', - branch_name: 'master') + project.repository.create_file(user, 'new-file.txt', 'A new file', message: '[skip ci] This is a test', branch_name: 'master') end it_behaves_like 'creates a pipeline' @@ -906,11 +880,8 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte subject do post :create, params: { - namespace_id: project.namespace, - project_id: project, - pipeline: { ref: 'master' } - }, - format: :json + namespace_id: project.namespace, project_id: project, pipeline: { ref: 'master' } + }, format: :json end before do @@ -969,11 +940,8 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte describe 'POST retry.json' do subject(:post_retry) do post :retry, params: { - namespace_id: project.namespace, - project_id: project, - id: pipeline.id - }, - format: :json + namespace_id: project.namespace, project_id: project, id: pipeline.id + }, format: :json end let!(:pipeline) { create(:ci_pipeline, :failed, project: project) } @@ -1036,11 +1004,8 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte before do post :cancel, params: { - namespace_id: project.namespace, - project_id: project, - id: pipeline.id - }, - format: :json + namespace_id: project.namespace, project_id: project, id: pipeline.id + }, format: :json end it 'cancels a pipeline without returning any content', :sidekiq_might_not_need_inline do @@ -1183,7 +1148,7 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte def clear_controller_memoization controller.clear_memoization(:pipeline_test_report) - controller.instance_variable_set(:@pipeline, nil) + controller.remove_instance_variable(:@pipeline) end end @@ -1192,17 +1157,11 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte let(:branch_secondary) { project.repository.branches[1] } let!(:pipeline_master) do - create(:ci_pipeline, - ref: branch_main.name, - sha: branch_main.target, - project: project) + create(:ci_pipeline, ref: branch_main.name, sha: branch_main.target, project: project) end let!(:pipeline_secondary) do - create(:ci_pipeline, - ref: branch_secondary.name, - sha: branch_secondary.target, - project: project) + create(:ci_pipeline, ref: branch_secondary.name, sha: branch_secondary.target, project: project) end before do @@ -1319,149 +1278,6 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte end end - describe 'GET config_variables.json', :use_clean_rails_memory_store_caching do - include ReactiveCachingHelpers - - let(:ci_config) { '' } - let(:files) { { '.gitlab-ci.yml' => YAML.dump(ci_config) } } - let(:project) { create(:project, :auto_devops_disabled, :custom_repo, files: files) } - let(:service) { Ci::ListConfigVariablesService.new(project, user) } - - before do - allow(Ci::ListConfigVariablesService) - .to receive(:new) - .and_return(service) - end - - context 'when sending a valid ref' do - let(:ref) { 'master' } - let(:ci_config) do - { - variables: { - KEY1: { value: 'val 1', description: 'description 1' } - }, - test: { - stage: 'test', - script: 'echo' - } - } - end - - before do - synchronous_reactive_cache(service) - end - - it 'returns variable list' do - get_config_variables - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['KEY1']).to eq({ 'value' => 'val 1', 'description' => 'description 1' }) - end - end - - context 'when sending an invalid ref' do - let(:ref) { 'invalid-ref' } - - before do - synchronous_reactive_cache(service) - end - - it 'returns empty json' do - get_config_variables - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq({}) - end - end - - context 'when sending an invalid config' do - let(:ref) { 'master' } - let(:ci_config) do - { - variables: { - KEY1: { value: 'val 1', description: 'description 1' } - }, - test: { - stage: 'invalid', - script: 'echo' - } - } - end - - before do - synchronous_reactive_cache(service) - end - - it 'returns empty result' do - get_config_variables - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq({}) - end - end - - context 'when the cache is empty' do - let(:ref) { 'master' } - let(:ci_config) do - { - variables: { - KEY1: { value: 'val 1', description: 'description 1' } - }, - test: { - stage: 'test', - script: 'echo' - } - } - end - - it 'returns no content' do - get_config_variables - - expect(response).to have_gitlab_http_status(:no_content) - end - end - - context 'when project uses external project ci config' do - let(:other_project) { create(:project, :custom_repo, files: other_project_files) } - let(:other_project_files) { { '.gitlab-ci.yml' => YAML.dump(other_project_ci_config) } } - let(:ref) { 'master' } - - let(:other_project_ci_config) do - { - variables: { - KEY1: { value: 'val 1', description: 'description 1' } - }, - test: { - stage: 'test', - script: 'echo' - } - } - end - - before do - other_project.add_developer(user) - project.update!(ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}:master") - synchronous_reactive_cache(service) - end - - it 'returns other project config variables' do - get_config_variables - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['KEY1']).to eq({ 'value' => 'val 1', 'description' => 'description 1' }) - end - end - - private - - def get_config_variables - get :config_variables, params: { namespace_id: project.namespace, - project_id: project, - sha: ref }, - format: :json - end - end - describe 'GET downloadable_artifacts.json' do context 'when pipeline is empty' do let(:pipeline) { create(:ci_empty_pipeline) } diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index ab33195eb83..dbea3592e24 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -560,12 +560,4 @@ RSpec.describe Projects::ProjectMembersController do end it_behaves_like 'controller actions' - - context 'when project_members_index_by_project_namespace feature flag is disabled' do - before do - stub_feature_flags(project_members_index_by_project_namespace: false) - end - - it_behaves_like 'controller actions' - end end diff --git a/spec/controllers/projects/prometheus/alerts_controller_spec.rb b/spec/controllers/projects/prometheus/alerts_controller_spec.rb index 09b9f25c0c6..91d3ba7e106 100644 --- a/spec/controllers/projects/prometheus/alerts_controller_spec.rb +++ b/spec/controllers/projects/prometheus/alerts_controller_spec.rb @@ -117,10 +117,7 @@ RSpec.describe Projects::Prometheus::AlertsController do describe 'GET #metrics_dashboard' do let!(:alert) do - create(:prometheus_alert, - project: project, - environment: environment, - prometheus_metric: metric) + create(:prometheus_alert, project: project, environment: environment, prometheus_metric: metric) end it 'returns a json object with the correct keys' do diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index 40252cf65cd..b15a37d8d90 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -12,13 +12,9 @@ RSpec.describe Projects::RawController, feature_category: :source_code_managemen describe 'GET #show' do def get_show - get(:show, - params: { - namespace_id: project.namespace, - project_id: project, - id: file_path, - inline: inline - }.merge(params)) + get :show, params: { + namespace_id: project.namespace, project_id: project, id: file_path, inline: inline + }.merge(params) end subject { get_show } diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb index 7a511ab676e..0b1d0b75de7 100644 --- a/spec/controllers/projects/refs_controller_spec.rb +++ b/spec/controllers/projects/refs_controller_spec.rb @@ -26,7 +26,7 @@ RSpec.describe Projects::RefsController, feature_category: :source_code_manageme 'tree' | nil | lazy { project_tree_path(project, id) } 'tree' | 'heads' | lazy { project_tree_path(project, id) } 'blob' | nil | lazy { project_blob_path(project, id) } - 'blob' | 'heads' | lazy { project_blob_path(project, id, ref_type: 'heads') } + 'blob' | 'heads' | lazy { project_blob_path(project, id) } 'graph' | nil | lazy { project_network_path(project, id) } 'graph' | 'heads' | lazy { project_network_path(project, id, ref_type: 'heads') } 'graphs' | nil | lazy { project_graph_path(project, id) } @@ -54,14 +54,9 @@ RSpec.describe Projects::RefsController, feature_category: :source_code_manageme let(:path) { 'foo/bar/baz.html' } def default_get(format = :html) - get :logs_tree, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: 'master', - path: path - }, - format: format + get :logs_tree, params: { + namespace_id: project.namespace.to_param, project_id: project, id: 'master', path: path + }, format: format end def xhr_get(format = :html, params = {}) diff --git a/spec/controllers/projects/registry/repositories_controller_spec.rb b/spec/controllers/projects/registry/repositories_controller_spec.rb index 59bc1ba04e7..834fdddd583 100644 --- a/spec/controllers/projects/registry/repositories_controller_spec.rb +++ b/spec/controllers/projects/registry/repositories_controller_spec.rb @@ -59,8 +59,7 @@ RSpec.describe Projects::Registry::RepositoriesController do context 'when root container repository is not created' do context 'when there are tags for this repository' do before do - stub_container_registry_tags(repository: :any, - tags: %w[rc1 latest]) + stub_container_registry_tags(repository: :any, tags: %w[rc1 latest]) end it 'creates a root container repository' do @@ -139,19 +138,12 @@ RSpec.describe Projects::Registry::RepositoriesController do end def go_to_index(format: :html, params: {}) - get :index, params: params.merge({ - namespace_id: project.namespace, - project_id: project - }), - format: format + get :index, params: params.merge({ namespace_id: project.namespace, project_id: project }), format: format end def delete_repository(repository) delete :destroy, params: { - namespace_id: project.namespace, - project_id: project, - id: repository - }, - format: :json + namespace_id: project.namespace, project_id: project, id: repository + }, format: :json end end diff --git a/spec/controllers/projects/registry/tags_controller_spec.rb b/spec/controllers/projects/registry/tags_controller_spec.rb index 7b786f4a8af..afa7bd6a60d 100644 --- a/spec/controllers/projects/registry/tags_controller_spec.rb +++ b/spec/controllers/projects/registry/tags_controller_spec.rb @@ -76,11 +76,8 @@ RSpec.describe Projects::Registry::TagsController do def get_tags get :index, params: { - namespace_id: project.namespace, - project_id: project, - repository_id: repository - }, - format: :json + namespace_id: project.namespace, project_id: project, repository_id: repository + }, format: :json end end @@ -121,12 +118,11 @@ RSpec.describe Projects::Registry::TagsController do def destroy_tag(name) post :destroy, params: { - namespace_id: project.namespace, - project_id: project, - repository_id: repository, - id: name - }, - format: :json + namespace_id: project.namespace, + project_id: project, + repository_id: repository, + id: name + }, format: :json end end @@ -162,12 +158,11 @@ RSpec.describe Projects::Registry::TagsController do def bulk_destroy_tags(names) post :bulk_destroy, params: { - namespace_id: project.namespace, - project_id: project, - repository_id: repository, - ids: names - }, - format: :json + namespace_id: project.namespace, + project_id: project, + repository_id: repository, + ids: names + }, format: :json end end diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb index 2afd080344d..17bf9308834 100644 --- a/spec/controllers/projects/releases_controller_spec.rb +++ b/spec/controllers/projects/releases_controller_spec.rb @@ -158,9 +158,9 @@ RSpec.describe Projects::ReleasesController do it_behaves_like 'successful request' - it 'is accesible at a URL encoded path' do + it 'is accessible at a URL encoded path' do expect(edit_project_release_path(project, release)) - .to eq("/#{project.namespace.path}/#{project.name}/-/releases/awesome%2Fv1.0/edit") + .to eq("/#{project.full_path}/-/releases/awesome%2Fv1.0/edit") end end @@ -199,7 +199,7 @@ RSpec.describe Projects::ReleasesController do it 'is accesible at a URL encoded path' do expect(project_release_path(project, release)) - .to eq("/#{project.namespace.path}/#{project.name}/-/releases/awesome%2Fv1.0") + .to eq("/#{project.full_path}/-/releases/awesome%2Fv1.0") end end diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index 8186176a46b..0efed45336f 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -106,19 +106,11 @@ RSpec.describe Projects::RepositoriesController, feature_category: :source_code_ end end - context "when the request format is HTML" do - it "renders 404" do - get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master' }, format: "html" - - expect(response).to have_gitlab_http_status(:not_found) - end - end - describe 'rate limiting' do it 'rate limits user when thresholds hit' do allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) - get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master' }, format: "html" + get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master' }, format: "zip" expect(response).to have_gitlab_http_status(:too_many_requests) end diff --git a/spec/controllers/projects/runner_projects_controller_spec.rb b/spec/controllers/projects/runner_projects_controller_spec.rb new file mode 100644 index 00000000000..beedaad0fa9 --- /dev/null +++ b/spec/controllers/projects/runner_projects_controller_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::RunnerProjectsController, feature_category: :runner_fleet do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:source_project) { create(:project) } + + before do + sign_in(user) + project.add_maintainer(user) + end + + describe '#create' do + subject(:send_create) do + post :create, params: { + namespace_id: group.path, + project_id: project.path, + runner_project: { runner_id: project_runner.id } + } + end + + context 'when assigning runner to another project' do + let(:project_runner) { create(:ci_runner, :project, projects: [source_project]) } + + it 'redirects to the project runners page' do + source_project.add_maintainer(user) + + send_create + + expect(flash[:success]).to be_present + expect(response).to have_gitlab_http_status(:redirect) + expect(response).to redirect_to project_runners_path(project) + end + end + end + + describe '#destroy' do + subject(:send_destroy) do + delete :destroy, params: { + namespace_id: group.path, + project_id: project.path, + id: runner_project_id + } + end + + context 'when unassigning runner from project' do + let(:project_runner) { create(:ci_runner, :project, projects: [project]) } + let(:runner_project_id) { project_runner.runner_projects.last.id } + + it 'redirects to the project runners page' do + send_destroy + + expect(flash[:success]).to be_present + expect(response).to have_gitlab_http_status(:redirect) + expect(response).to redirect_to project_runners_path(project) + end + end + end +end diff --git a/spec/controllers/projects/runners_controller_spec.rb b/spec/controllers/projects/runners_controller_spec.rb index 5733b8114d4..e0e4d0f7bc5 100644 --- a/spec/controllers/projects/runners_controller_spec.rb +++ b/spec/controllers/projects/runners_controller_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Projects::RunnersController, feature_category: :runner_fleet do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) } let(:params) do { @@ -20,6 +20,137 @@ RSpec.describe Projects::RunnersController, feature_category: :runner_fleet do project.add_maintainer(user) end + describe '#new' do + let(:params) do + { + namespace_id: project.namespace, + project_id: project + } + end + + context 'when create_runner_workflow_for_namespace is enabled' do + before do + stub_feature_flags(create_runner_workflow_for_namespace: [project.namespace]) + end + + context 'when user is maintainer' do + before do + project.add_maintainer(user) + end + + it 'renders new with 200 status code' do + get :new, params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:new) + end + end + + context 'when user is not maintainer' do + before do + project.add_developer(user) + end + + it 'renders a 404' do + get :new, params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when create_runner_workflow_for_namespace is disabled' do + before do + stub_feature_flags(create_runner_workflow_for_namespace: false) + end + + context 'when user is maintainer' do + before do + project.add_maintainer(user) + end + + it 'renders a 404' do + get :new, params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + describe '#register' do + subject(:register) { get :register, params: { namespace_id: project.namespace, project_id: project, id: new_runner } } + + context 'when create_runner_workflow_for_namespace is enabled' do + before do + stub_feature_flags(create_runner_workflow_for_namespace: [project.namespace]) + end + + context 'when user is maintainer' do + before do + project.add_maintainer(user) + end + + context 'when runner can be registered after creation' do + let_it_be(:new_runner) { create(:ci_runner, :project, projects: [project], registration_type: :authenticated_user) } + + it 'renders a :register template' do + register + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:register) + end + end + + context 'when runner cannot be registered after creation' do + let_it_be(:new_runner) { runner } + + it 'returns :not_found' do + register + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when user is not maintainer' do + before do + project.add_developer(user) + end + + context 'when runner can be registered after creation' do + let_it_be(:new_runner) { create(:ci_runner, :project, projects: [project], registration_type: :authenticated_user) } + + it 'returns :not_found' do + register + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + context 'when create_runner_workflow_for_namespace is disabled' do + let_it_be(:new_runner) { create(:ci_runner, :project, projects: [project], registration_type: :authenticated_user) } + + before do + stub_feature_flags(create_runner_workflow_for_namespace: false) + end + + context 'when user is maintainer' do + before do + project.add_maintainer(user) + end + + it 'returns :not_found' do + register + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + describe '#update' do it 'updates the runner and ticks the queue' do new_desc = runner.description.swapcase diff --git a/spec/controllers/projects/service_desk_controller_spec.rb b/spec/controllers/projects/service_desk_controller_spec.rb index e078bf9461e..6b914ac8f19 100644 --- a/spec/controllers/projects/service_desk_controller_spec.rb +++ b/spec/controllers/projects/service_desk_controller_spec.rb @@ -12,8 +12,8 @@ RSpec.describe Projects::ServiceDeskController do let_it_be(:user) { create(:user) } before do - allow(Gitlab::IncomingEmail).to receive(:enabled?) { true } - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } project.add_maintainer(user) sign_in(user) diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index ba917fa3a31..1c332eadc42 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -173,12 +173,11 @@ RSpec.describe Projects::Settings::CiCdController, feature_category: :continuous let(:params) { { ci_config_path: '' } } subject do - patch :update, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - project: params - } + patch :update, params: { + namespace_id: project.namespace.to_param, + project_id: project, + project: params + } end it 'redirects to the settings page' do @@ -241,9 +240,7 @@ RSpec.describe Projects::Settings::CiCdController, feature_category: :continuous end it 'creates a pipeline', :sidekiq_inline do - project.repository.create_file(user, 'Gemfile', 'Gemfile contents', - message: 'Add Gemfile', - branch_name: 'master') + project.repository.create_file(user, 'Gemfile', 'Gemfile contents', message: 'Add Gemfile', branch_name: 'master') expect { subject }.to change { Ci::Pipeline.count }.by(1) end diff --git a/spec/controllers/projects/settings/merge_requests_controller_spec.rb b/spec/controllers/projects/settings/merge_requests_controller_spec.rb index 106ec62bea0..398fc97a00d 100644 --- a/spec/controllers/projects/settings/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/settings/merge_requests_controller_spec.rb @@ -36,12 +36,11 @@ RSpec.describe Projects::Settings::MergeRequestsController do merge_method: :ff } - put :update, - params: { - namespace_id: project.namespace, - project_id: project.id, - project: params - } + put :update, params: { + namespace_id: project.namespace, + project_id: project.id, + project: params + } expect(response).to redirect_to project_settings_merge_requests_path(project) params.each do |param, value| diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 76d8191e342..04dbd9ab671 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Settings::OperationsController do +RSpec.describe Projects::Settings::OperationsController, feature_category: :incident_management do let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project) } @@ -11,6 +11,8 @@ RSpec.describe Projects::Settings::OperationsController do end before do + stub_feature_flags(remove_monitor_metrics: false) + sign_in(user) end @@ -65,6 +67,20 @@ RSpec.describe Projects::Settings::OperationsController do end end + shared_examples 'PATCHable without metrics dashboard' do + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + include_examples 'PATCHable' do + let(:permitted_params) do + ActionController::Parameters.new({}).permit! + end + end + end + end + describe 'GET #show' do it 'renders show template' do get :show, params: project_params(project) @@ -124,7 +140,7 @@ RSpec.describe Projects::Settings::OperationsController do end end - context 'incident management' do + context 'incident management', feature_category: :incident_management do describe 'GET #show' do context 'with existing setting' do let!(:incident_management_setting) do @@ -278,7 +294,7 @@ RSpec.describe Projects::Settings::OperationsController do end end - context 'error tracking' do + context 'error tracking', feature_category: :error_tracking do describe 'GET #show' do context 'with existing setting' do let!(:error_tracking_setting) do @@ -323,7 +339,7 @@ RSpec.describe Projects::Settings::OperationsController do end end - context 'metrics dashboard setting' do + context 'metrics dashboard setting', feature_category: :metrics do describe 'PATCH #update' do let(:params) do { @@ -333,11 +349,12 @@ RSpec.describe Projects::Settings::OperationsController do } end - it_behaves_like 'PATCHable' + include_examples 'PATCHable' + include_examples 'PATCHable without metrics dashboard' end end - context 'grafana integration' do + context 'grafana integration', feature_category: :metrics do describe 'PATCH #update' do let(:params) do { @@ -349,7 +366,8 @@ RSpec.describe Projects::Settings::OperationsController do } end - it_behaves_like 'PATCHable' + include_examples 'PATCHable' + include_examples 'PATCHable without metrics dashboard' end end diff --git a/spec/controllers/projects/snippets/blobs_controller_spec.rb b/spec/controllers/projects/snippets/blobs_controller_spec.rb index ca656705e07..4d12452e3d5 100644 --- a/spec/controllers/projects/snippets/blobs_controller_spec.rb +++ b/spec/controllers/projects/snippets/blobs_controller_spec.rb @@ -26,15 +26,14 @@ RSpec.describe Projects::Snippets::BlobsController do let(:inline) { nil } subject do - get(:raw, - params: { - namespace_id: project.namespace, - project_id: project, - snippet_id: snippet, - path: filepath, - ref: ref, - inline: inline - }) + get :raw, params: { + namespace_id: project.namespace, + project_id: project, + snippet_id: snippet, + path: filepath, + ref: ref, + inline: inline + } end context 'with a snippet without a repository' do diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index a388fc4620f..119e52480db 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -102,12 +102,11 @@ RSpec.describe Projects::SnippetsController do project.add_maintainer(admin) sign_in(admin) - post :mark_as_spam, - params: { - namespace_id: project.namespace, - project_id: project, - id: snippet.id - } + post :mark_as_spam, params: { + namespace_id: project.namespace, + project_id: project, + id: snippet.id + } end it 'updates the snippet', :enable_admin_mode do diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb index 37149e1d3ca..61998d516e8 100644 --- a/spec/controllers/projects/tree_controller_spec.rb +++ b/spec/controllers/projects/tree_controller_spec.rb @@ -136,12 +136,9 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme allow(::Gitlab::GitalyClient).to receive(:call).and_call_original expect(::Gitlab::GitalyClient).not_to receive(:call).with(anything, :commit_service, :find_commit, anything, anything) - get(:show, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: id - }) + get :show, params: { + namespace_id: project.namespace.to_param, project_id: project, id: id + } expect(response).to have_gitlab_http_status(:not_found) end @@ -151,12 +148,9 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme render_views before do - get(:show, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: id - }) + get :show, params: { + namespace_id: project.namespace.to_param, project_id: project, id: id + } end context 'redirect to blob' do @@ -164,8 +158,7 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme it 'redirects' do redirect_url = "/#{project.full_path}/-/blob/master/README.md" - expect(subject) - .to redirect_to(redirect_url) + expect(subject).to redirect_to(redirect_url) end end end @@ -174,15 +167,14 @@ RSpec.describe Projects::TreeController, feature_category: :source_code_manageme render_views before do - post(:create_dir, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: 'master', - dir_name: path, - branch_name: branch_name, - commit_message: 'Test commit message' - }) + post :create_dir, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: 'master', + dir_name: path, + branch_name: branch_name, + commit_message: 'Test commit message' + } end context 'successful creation' do diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index 7243588681d..353cd62686f 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::WikisController do +RSpec.describe Projects::WikisController, feature_category: :wiki do it_behaves_like 'wiki controller actions' do let(:container) { create(:project, :public, namespace: user.namespace) } let(:routing_params) { { namespace_id: container.namespace, project_id: container } } diff --git a/spec/controllers/projects/work_items_controller_spec.rb b/spec/controllers/projects/work_items_controller_spec.rb new file mode 100644 index 00000000000..e0f61a4977b --- /dev/null +++ b/spec/controllers/projects/work_items_controller_spec.rb @@ -0,0 +1,156 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::WorkItemsController, feature_category: :team_planning do + let_it_be(:reporter) { create(:user) } + let_it_be(:guest) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:work_item) { create(:work_item, project: project) } + + let(:file) { 'file' } + + before do + project.add_reporter(reporter) + project.add_guest(guest) + end + + shared_examples 'response with 404 status' do + it 'renders a not found message' do + expect(WorkItems::ImportWorkItemsCsvWorker).not_to receive(:perform_async) + + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + shared_examples 'redirects to new session path' do + it 'redirects to sign in' do + subject + + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(new_user_session_path) + end + end + + describe 'GET index' do + specify do + expect( + get(:index, params: { namespace_id: project.namespace, project_id: project, work_items_path: work_item.id }) + ).to have_request_urgency(:low) + end + end + + describe 'POST authorize' do + subject do + post(:authorize, params: { namespace_id: project.namespace, project_id: project, file: file }) + end + + specify do + expect(subject).to have_request_urgency(:high) + end + + context 'when user is anonymous' do + it_behaves_like 'redirects to new session path' + end + end + + describe 'POST import_csv' do + subject { post :import_csv, params: { namespace_id: project.namespace, project_id: project, file: file } } + + let(:upload_service) { double } + let(:uploader) { double } + let(:upload) { double } + let(:upload_id) { 99 } + + specify do + expect(subject).to have_request_urgency(:low) + end + + context 'with authorized user' do + before do + sign_in(reporter) + allow(controller).to receive(:file_is_valid?).and_return(true) + end + + context 'when feature is available' do + context 'when the upload is processed successfully' do + before do + mock_upload + end + + it 'renders the correct message' do + expect(WorkItems::ImportWorkItemsCsvWorker).to receive(:perform_async) + .with(reporter.id, project.id, upload_id) + + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['message']).to eq( + "Your work items are being imported. Once finished, you'll receive a confirmation email." + ) + end + end + + context 'when file is not valid' do + before do + allow(controller).to receive(:file_is_valid?).and_return(false) + end + + it 'renders the error message' do + expect(WorkItems::ImportWorkItemsCsvWorker).not_to receive(:perform_async) + + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['errors']) + .to eq('The uploaded file was invalid. Supported file extensions are .csv.') + end + end + + context 'when service response includes errors' do + before do + mock_upload(false) + end + + it 'renders the error message' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['errors']).to eq('File upload error.') + end + end + end + + context 'when feature is not available' do + before do + stub_feature_flags(import_export_work_items_csv: false) + end + + it_behaves_like 'response with 404 status' + end + end + + context 'with unauthorised user' do + before do + mock_upload + sign_in(guest) + allow(controller).to receive(:file_is_valid?).and_return(true) + end + + it_behaves_like 'response with 404 status' + end + + context 'with anonymous user' do + it 'redirects to sign in page' do + expect(WorkItems::ImportWorkItemsCsvWorker).not_to receive(:perform_async) + + subject + + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(new_user_session_path) + end + end + end +end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index c5ec6651ab3..b652aba1fff 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -571,11 +571,7 @@ RSpec.describe ProjectsController, feature_category: :projects do it 'allows an admin user to access the page', :enable_admin_mode do sign_in(create(:user, :admin)) - get :edit, - params: { - namespace_id: project.namespace.path, - id: project.path - } + get :edit, params: { namespace_id: project.namespace.path, id: project.path } expect(response).to have_gitlab_http_status(:ok) end @@ -584,11 +580,7 @@ RSpec.describe ProjectsController, feature_category: :projects do sign_in(user) project.add_maintainer(user) - get :edit, - params: { - namespace_id: project.namespace.path, - id: project.path - } + get :edit, params: { namespace_id: project.namespace.path, id: project.path } expect(assigns(:badge_api_endpoint)).not_to be_nil end @@ -606,10 +598,7 @@ RSpec.describe ProjectsController, feature_category: :projects do before do group.add_owner(user) - post :archive, params: { - namespace_id: project.namespace.path, - id: project.path - } + post :archive, params: { namespace_id: project.namespace.path, id: project.path } end it 'archives the project' do @@ -853,12 +842,7 @@ RSpec.describe ProjectsController, feature_category: :projects do merge_method: :ff } - put :update, - params: { - namespace_id: project.namespace, - id: project.id, - project: params - } + put :update, params: { namespace_id: project.namespace, id: project.id, project: params } expect(response).to have_gitlab_http_status(:found) params.each do |param, value| @@ -874,22 +858,12 @@ RSpec.describe ProjectsController, feature_category: :projects do } expect do - put :update, - params: { - namespace_id: project.namespace, - id: project.id, - project: params - } + put :update, params: { namespace_id: project.namespace, id: project.id, project: params } end.not_to change { project.namespace.reload } end def update_project(**parameters) - put :update, - params: { - namespace_id: project.namespace.path, - id: project.path, - project: parameters - } + put :update, params: { namespace_id: project.namespace.path, id: project.path, project: parameters } end end @@ -913,12 +887,9 @@ RSpec.describe ProjectsController, feature_category: :projects do it_behaves_like 'unauthorized when external service denies access' do subject do - put :update, - params: { - namespace_id: project.namespace, - id: project, - project: { description: 'Hello world' } - } + put :update, params: { + namespace_id: project.namespace, id: project, project: { description: 'Hello world' } + } project.reload end @@ -1038,13 +1009,9 @@ RSpec.describe ProjectsController, feature_category: :projects do old_namespace = project.namespace - put :transfer, - params: { - namespace_id: old_namespace.path, - new_namespace_id: new_namespace_id, - id: project.path - }, - format: :js + put :transfer, params: { + namespace_id: old_namespace.path, new_namespace_id: new_namespace_id, id: project.path + }, format: :js project.reload @@ -1057,13 +1024,9 @@ RSpec.describe ProjectsController, feature_category: :projects do it 'updates namespace' do sign_in(admin) - put :transfer, - params: { - namespace_id: project.namespace.path, - new_namespace_id: new_namespace.id, - id: project.path - }, - format: :js + put :transfer, params: { + namespace_id: project.namespace.path, new_namespace_id: new_namespace.id, id: project.path + }, format: :js project.reload @@ -1183,32 +1146,19 @@ RSpec.describe ProjectsController, feature_category: :projects do it "toggles star if user is signed in" do sign_in(user) expect(user.starred?(public_project)).to be_falsey - post(:toggle_star, - params: { - namespace_id: public_project.namespace, - id: public_project - }) + + post :toggle_star, params: { namespace_id: public_project.namespace, id: public_project } expect(user.starred?(public_project)).to be_truthy - post(:toggle_star, - params: { - namespace_id: public_project.namespace, - id: public_project - }) + + post :toggle_star, params: { namespace_id: public_project.namespace, id: public_project } expect(user.starred?(public_project)).to be_falsey end it "does nothing if user is not signed in" do - post(:toggle_star, - params: { - namespace_id: project.namespace, - id: public_project - }) + post :toggle_star, params: { namespace_id: project.namespace, id: public_project } expect(user.starred?(public_project)).to be_falsey - post(:toggle_star, - params: { - namespace_id: project.namespace, - id: public_project - }) + + post :toggle_star, params: { namespace_id: project.namespace, id: public_project } expect(user.starred?(public_project)).to be_falsey end end @@ -1223,12 +1173,9 @@ RSpec.describe ProjectsController, feature_category: :projects do let(:forked_project) { fork_project(create(:project, :public), user) } it 'removes fork from project' do - delete(:remove_fork, - params: { - namespace_id: forked_project.namespace.to_param, - id: forked_project.to_param - }, - format: :js) + delete :remove_fork, params: { + namespace_id: forked_project.namespace.to_param, id: forked_project.to_param + }, format: :js expect(forked_project.reload.forked?).to be_falsey expect(flash[:notice]).to eq(s_('The fork relationship has been removed.')) @@ -1240,12 +1187,9 @@ RSpec.describe ProjectsController, feature_category: :projects do let(:unforked_project) { create(:project, namespace: user.namespace) } it 'does nothing if project was not forked' do - delete(:remove_fork, - params: { - namespace_id: unforked_project.namespace, - id: unforked_project - }, - format: :js) + delete :remove_fork, params: { + namespace_id: unforked_project.namespace, id: unforked_project + }, format: :js expect(flash[:notice]).to be_nil expect(response).to redirect_to(edit_project_path(unforked_project)) @@ -1254,12 +1198,10 @@ RSpec.describe ProjectsController, feature_category: :projects do end it "does nothing if user is not signed in" do - delete(:remove_fork, - params: { - namespace_id: project.namespace, - id: project - }, - format: :js) + delete :remove_fork, params: { + namespace_id: project.namespace, id: project + }, format: :js + expect(response).to have_gitlab_http_status(:unauthorized) end end @@ -1352,6 +1294,19 @@ RSpec.describe ProjectsController, feature_category: :projects do expect(response).to have_gitlab_http_status(:success) end end + + context 'when sort param is invalid' do + let(:request) { get :refs, params: { namespace_id: project.namespace, id: project, sort: 'invalid' } } + + it 'uses default sort by name' do + request + + expect(response).to have_gitlab_http_status(:success) + expect(json_response['Branches']).to include('master') + expect(json_response['Tags']).to include('v1.0.0') + expect(json_response['Commits']).to be_nil + end + end end describe 'POST #preview_markdown' do @@ -1818,18 +1773,13 @@ RSpec.describe ProjectsController, feature_category: :projects do it 'updates Service Desk attributes' do project.add_maintainer(user) sign_in(user) - allow(Gitlab::IncomingEmail).to receive(:enabled?) { true } - allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } + allow(Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } params = { service_desk_enabled: true } - put :update, - params: { - namespace_id: project.namespace, - id: project, - project: params - } + put :update, params: { namespace_id: project.namespace, id: project, project: params } project.reload expect(response).to have_gitlab_http_status(:found) diff --git a/spec/controllers/registrations/welcome_controller_spec.rb b/spec/controllers/registrations/welcome_controller_spec.rb index b5416d226e1..4118754144c 100644 --- a/spec/controllers/registrations/welcome_controller_spec.rb +++ b/spec/controllers/registrations/welcome_controller_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe Registrations::WelcomeController, feature_category: :authentication_and_authorization do +RSpec.describe Registrations::WelcomeController, feature_category: :system_access do let(:user) { create(:user) } - describe '#welcome' do + describe '#show' do subject(:show) { get :show } context 'without a signed in user' do @@ -27,6 +27,14 @@ RSpec.describe Registrations::WelcomeController, feature_category: :authenticati end it { is_expected.to render_template(:show) } + + render_views + + it 'has the expected submission url' do + show + + expect(response.body).to include("action=\"#{users_sign_up_welcome_path}\"") + end end context 'when role and setup_for_company is set' do @@ -57,6 +65,32 @@ RSpec.describe Registrations::WelcomeController, feature_category: :authenticati expect(subject).not_to redirect_to(profile_two_factor_auth_path) end end + + context 'when welcome step is completed' do + before do + user.update!(setup_for_company: true) + end + + context 'when user is confirmed' do + before do + sign_in(user) + end + + it { is_expected.to redirect_to dashboard_projects_path } + end + + context 'when user is not confirmed' do + before do + stub_application_setting_enum('email_confirmation_setting', 'hard') + + sign_in(user) + + user.update!(confirmed_at: nil) + end + + it { is_expected.to redirect_to user_session_path } + end + end end describe '#update' do diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index b217b100349..9aa8a2ae605 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -12,15 +12,23 @@ RSpec.describe RegistrationsController, feature_category: :user_profile do end describe '#new' do - subject { get :new } + subject(:new) { get :new } it 'renders new template and sets the resource variable' do - expect(subject).to render_template(:new) + expect(new).to render_template(:new) expect(response).to have_gitlab_http_status(:ok) expect(assigns(:resource)).to be_a(User) end it_behaves_like "switches to user preferred language", 'Sign up' + + render_views + + it 'has the expected registration url' do + new + + expect(response.body).to include("action=\"#{user_registration_path}\"") + end end describe '#create' do @@ -75,7 +83,7 @@ RSpec.describe RegistrationsController, feature_category: :user_profile do end context 'email confirmation' do - context 'when `email_confirmation_setting` is set to `hard`' do + context 'when email confirmation setting is set to `hard`' do before do stub_application_setting_enum('email_confirmation_setting', 'hard') end @@ -122,7 +130,7 @@ RSpec.describe RegistrationsController, feature_category: :user_profile do end context 'email confirmation' do - context 'when `email_confirmation_setting` is set to `hard`' do + context 'when email confirmation setting is set to `hard`' do before do stub_application_setting_enum('email_confirmation_setting', 'hard') stub_feature_flags(identity_verification: false) @@ -157,7 +165,7 @@ RSpec.describe RegistrationsController, feature_category: :user_profile do stub_feature_flags(identity_verification: false) end - context 'when `email_confirmation_setting` is set to `off`' do + context 'when email confirmation setting is set to `off`' do it 'signs the user in' do stub_application_setting_enum('email_confirmation_setting', 'off') @@ -166,103 +174,97 @@ RSpec.describe RegistrationsController, feature_category: :user_profile do end end - context 'when `email_confirmation_setting` is set to `hard`' do + context 'when email confirmation setting is set to `hard`' do before do stub_application_setting_enum('email_confirmation_setting', 'hard') + allow(User).to receive(:allow_unconfirmed_access_for).and_return 0 end - context 'when soft email confirmation is not enabled' do - before do - stub_feature_flags(soft_email_confirmation: false) - allow(User).to receive(:allow_unconfirmed_access_for).and_return 0 - end + it 'does not authenticate the user and sends a confirmation email' do + expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).to be_nil + end - it 'does not authenticate the user and sends a confirmation email' do - expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) - expect(controller.current_user).to be_nil - end + it 'tracks an almost there redirect' do + post_create - it 'tracks an almost there redirect' do - post_create + expect_snowplow_event( + category: described_class.name, + action: 'render', + user: User.find_by(email: base_user_params[:email]) + ) + end - expect_snowplow_event( - category: described_class.name, - action: 'render', - user: User.find_by(email: base_user_params[:email]) - ) - end + context 'when registration is triggered from an accepted invite' do + context 'when it is part from the initial invite email', :snowplow do + let_it_be(:member) { create(:project_member, :invited, invite_email: user_params.dig(:user, :email)) } - context 'when registration is triggered from an accepted invite' do - context 'when it is part from the initial invite email', :snowplow do - let_it_be(:member) { create(:project_member, :invited, invite_email: user_params.dig(:user, :email)) } + let(:originating_member_id) { member.id } + let(:session_params) do + { + invite_email: user_params.dig(:user, :email), + originating_member_id: originating_member_id + } + end - let(:originating_member_id) { member.id } - let(:session_params) do - { - invite_email: user_params.dig(:user, :email), - originating_member_id: originating_member_id - } + context 'when member exists from the session key value' do + it 'tracks the invite acceptance' do + subject + + expect_snowplow_event( + category: 'RegistrationsController', + action: 'accepted', + label: 'invite_email', + property: member.id.to_s, + user: member.reload.user + ) + + expect_snowplow_event( + category: 'RegistrationsController', + action: 'create_user', + label: 'invited', + user: member.reload.user + ) end + end - context 'when member exists from the session key value' do - it 'tracks the invite acceptance' do - subject - - expect_snowplow_event( - category: 'RegistrationsController', - action: 'accepted', - label: 'invite_email', - property: member.id.to_s, - user: member.reload.user - ) - - expect_snowplow_event( - category: 'RegistrationsController', - action: 'create_user', - label: 'invited', - user: member.reload.user - ) - end - end + context 'when member does not exist from the session key value' do + let(:originating_member_id) { nil } + + it 'does not track invite acceptance' do + subject + + expect_no_snowplow_event( + category: 'RegistrationsController', + action: 'accepted', + label: 'invite_email' + ) - context 'when member does not exist from the session key value' do - let(:originating_member_id) { nil } - - it 'does not track invite acceptance' do - subject - - expect_no_snowplow_event( - category: 'RegistrationsController', - action: 'accepted', - label: 'invite_email' - ) - - expect_snowplow_event( - category: 'RegistrationsController', - action: 'create_user', - label: 'signup', - user: member.reload.user - ) - end + expect_snowplow_event( + category: 'RegistrationsController', + action: 'create_user', + label: 'signup', + user: member.reload.user + ) end end + end - context 'when invite email matches email used on registration' do - let(:session_params) { { invite_email: user_params.dig(:user, :email) } } + context 'when invite email matches email used on registration' do + let(:session_params) { { invite_email: user_params.dig(:user, :email) } } - it 'signs the user in without sending a confirmation email', :aggregate_failures do - expect { subject }.not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions) - expect(controller.current_user).to be_confirmed - end + it 'signs the user in without sending a confirmation email', :aggregate_failures do + expect { subject }.not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).to be_confirmed end + end - context 'when invite email does not match the email used on registration' do - let(:session_params) { { invite_email: 'bogus@email.com' } } + context 'when invite email does not match the email used on registration' do + let(:session_params) { { invite_email: 'bogus@email.com' } } - it 'does not authenticate the user and sends a confirmation email', :aggregate_failures do - expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) - expect(controller.current_user).to be_nil - end + it 'does not authenticate the user and sends a confirmation email', :aggregate_failures do + expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).to be_nil end end end @@ -286,45 +288,45 @@ RSpec.describe RegistrationsController, feature_category: :user_profile do expect(controller.current_user).to be_nil end end + end - context 'when soft email confirmation is enabled' do - before do - stub_feature_flags(soft_email_confirmation: true) - allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days - end + context 'when email confirmation setting is set to `soft`' do + before do + stub_application_setting_enum('email_confirmation_setting', 'soft') + allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days + end - it 'authenticates the user and sends a confirmation email' do - expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) - expect(controller.current_user).to be_present - expect(response).to redirect_to(users_sign_up_welcome_path) - end + it 'authenticates the user and sends a confirmation email' do + expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).to be_present + expect(response).to redirect_to(users_sign_up_welcome_path) + end - it 'does not track an almost there redirect' do - post_create + it 'does not track an almost there redirect' do + post_create - expect_no_snowplow_event( - category: described_class.name, - action: 'render', - user: User.find_by(email: base_user_params[:email]) - ) - end + expect_no_snowplow_event( + category: described_class.name, + action: 'render', + user: User.find_by(email: base_user_params[:email]) + ) + end - context 'when invite email matches email used on registration' do - let(:session_params) { { invite_email: user_params.dig(:user, :email) } } + context 'when invite email matches email used on registration' do + let(:session_params) { { invite_email: user_params.dig(:user, :email) } } - it 'signs the user in without sending a confirmation email', :aggregate_failures do - expect { subject }.not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions) - expect(controller.current_user).to be_confirmed - end + it 'signs the user in without sending a confirmation email', :aggregate_failures do + expect { subject }.not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).to be_confirmed end + end - context 'when invite email does not match the email used on registration' do - let(:session_params) { { invite_email: 'bogus@email.com' } } + context 'when invite email does not match the email used on registration' do + let(:session_params) { { invite_email: 'bogus@email.com' } } - it 'authenticates the user and sends a confirmation email without confirming', :aggregate_failures do - expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) - expect(controller.current_user).not_to be_confirmed - end + it 'authenticates the user and sends a confirmation email without confirming', :aggregate_failures do + expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions) + expect(controller.current_user).not_to be_confirmed end end end @@ -756,8 +758,7 @@ RSpec.describe RegistrationsController, feature_category: :user_profile do m.call(*args) expect(Gitlab::ApplicationContext.current) - .to include('meta.user' => user.username, - 'meta.caller_id' => 'RegistrationsController#destroy') + .to include('meta.user' => user.username, 'meta.caller_id' => 'RegistrationsController#destroy') end post :destroy diff --git a/spec/controllers/repositories/git_http_controller_spec.rb b/spec/controllers/repositories/git_http_controller_spec.rb index da62acb1fda..276bd9b65b9 100644 --- a/spec/controllers/repositories/git_http_controller_spec.rb +++ b/spec/controllers/repositories/git_http_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Repositories::GitHttpController do +RSpec.describe Repositories::GitHttpController, feature_category: :source_code_management do let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:personal_snippet) { create(:personal_snippet, :public, :repository) } let_it_be(:project_snippet) { create(:project_snippet, :public, :repository, project: project) } @@ -14,7 +14,7 @@ RSpec.describe Repositories::GitHttpController do request.headers.merge! auth_env(user.username, user.password, nil) end - context 'when Gitaly is unavailable' do + context 'when Gitaly is unavailable', :use_clean_rails_redis_caching do it 'responds with a 503 message' do expect(Gitlab::GitalyClient).to receive(:call).and_raise(GRPC::Unavailable) @@ -26,6 +26,58 @@ RSpec.describe Repositories::GitHttpController do end end + shared_examples 'handles user activity' do + it 'updates the user activity' do + activity_project = container.is_a?(PersonalSnippet) ? nil : project + + activity_service = instance_double(Users::ActivityService) + + args = { author: user, project: activity_project, namespace: activity_project&.namespace } + expect(Users::ActivityService).to receive(:new).with(args).and_return(activity_service) + + expect(activity_service).to receive(:execute) + + get :info_refs, params: params + end + end + + shared_examples 'handles logging git upload pack operation' do + before do + password = user.try(:password) || user.try(:token) + request.headers.merge! auth_env(user.username, password, nil) + end + + context 'with git pull/fetch/clone action' do + let(:params) { super().merge(service: 'git-upload-pack') } + + it_behaves_like 'handles user activity' + end + end + + shared_examples 'handles logging git receive pack operation' do + let(:params) { super().merge(service: 'git-receive-pack') } + + before do + request.headers.merge! auth_env(user.username, user.password, nil) + end + + context 'with git push action when log_user_git_push_activity is enabled' do + it_behaves_like 'handles user activity' + end + + context 'when log_user_git_push_activity is disabled' do + before do + stub_feature_flags(log_user_git_push_activity: false) + end + + it 'does not log user activity' do + expect(controller).not_to receive(:log_user_activity) + + get :info_refs, params: params + end + end + end + context 'when repository container is a project' do it_behaves_like Repositories::GitHttpController do let(:container) { project } @@ -33,6 +85,8 @@ RSpec.describe Repositories::GitHttpController do let(:access_checker_class) { Gitlab::GitAccess } it_behaves_like 'handles unavailable Gitaly' + it_behaves_like 'handles logging git upload pack operation' + it_behaves_like 'handles logging git receive pack operation' describe 'POST #git_upload_pack' do before do @@ -83,6 +137,8 @@ RSpec.describe Repositories::GitHttpController do let(:container) { project } let(:user) { create(:deploy_token, :project, projects: [project]) } let(:access_checker_class) { Gitlab::GitAccess } + + it_behaves_like 'handles logging git upload pack operation' end end end @@ -92,6 +148,9 @@ RSpec.describe Repositories::GitHttpController do let(:container) { create(:project_wiki, :empty_repo, project: project) } let(:user) { project.first_owner } let(:access_checker_class) { Gitlab::GitAccessWiki } + + it_behaves_like 'handles logging git upload pack operation' + it_behaves_like 'handles logging git receive pack operation' end end @@ -102,6 +161,8 @@ RSpec.describe Repositories::GitHttpController do let(:access_checker_class) { Gitlab::GitAccessSnippet } it_behaves_like 'handles unavailable Gitaly' + it_behaves_like 'handles logging git upload pack operation' + it_behaves_like 'handles logging git receive pack operation' end end @@ -112,6 +173,8 @@ RSpec.describe Repositories::GitHttpController do let(:access_checker_class) { Gitlab::GitAccessSnippet } it_behaves_like 'handles unavailable Gitaly' + it_behaves_like 'handles logging git upload pack operation' + it_behaves_like 'handles logging git receive pack operation' end end end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 0f7f4a1910b..497e2d84f4f 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -38,6 +38,41 @@ RSpec.describe SearchController, feature_category: :global_search do it_behaves_like 'with external authorization service enabled', :show, { search: 'hello' } it_behaves_like 'support for active record query timeouts', :show, { search: 'hello' }, :search_objects, :html + describe 'rate limit scope' do + it 'uses current_user and search scope' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user, scope]) + get :show, params: { search: 'hello', scope: scope } + end + end + + it 'uses just current_user when no search scope is used' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :show, params: { search: 'hello' } + end + + it 'uses just current_user when search scope is abusive' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get(:show, params: { search: 'hello', scope: 'hack-the-mainframe' }) + + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :show, params: { search: 'hello', scope: 'blobs' * 1000 } + end + + context 'when search_rate_limited_scopes feature flag is disabled' do + before do + stub_feature_flags(search_rate_limited_scopes: false) + end + + it 'uses just current_user' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :show, params: { search: 'hello', scope: scope } + end + end + end + end + context 'uses the right partials depending on scope' do using RSpec::Parameterized::TableSyntax render_views @@ -227,12 +262,10 @@ RSpec.describe SearchController, feature_category: :global_search do let(:label) { 'redis_hll_counters.search.search_total_unique_counts_monthly' } let(:property) { 'i_search_total' } let(:context) do - [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, - event: property).to_context] + [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: property).to_context] end let(:namespace) { create(:group) } - let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } end context 'on restricted projects' do @@ -347,6 +380,36 @@ RSpec.describe SearchController, feature_category: :global_search do expect(json_response).to eq({ 'count' => '1' }) end + describe 'rate limit scope' do + it 'uses current_user and search scope' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user, scope]) + get :count, params: { search: 'hello', scope: scope } + end + end + + it 'uses just current_user when search scope is abusive' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :count, params: { search: 'hello', scope: 'hack-the-mainframe' } + + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :count, params: { search: 'hello', scope: 'blobs' * 1000 } + end + + context 'when search_rate_limited_scopes feature flag is disabled' do + before do + stub_feature_flags(search_rate_limited_scopes: false) + end + + it 'uses just current_user' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :count, params: { search: 'hello', scope: scope } + end + end + end + end + it 'raises an error if search term is missing' do expect do get :count, params: { scope: 'projects' } @@ -408,6 +471,36 @@ RSpec.describe SearchController, feature_category: :global_search do expect(json_response).to match_array([]) end + describe 'rate limit scope' do + it 'uses current_user and search scope' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user, scope]) + get :autocomplete, params: { term: 'hello', scope: scope } + end + end + + it 'uses just current_user when search scope is abusive' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :autocomplete, params: { term: 'hello', scope: 'hack-the-mainframe' } + + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :autocomplete, params: { term: 'hello', scope: 'blobs' * 1000 } + end + + context 'when search_rate_limited_scopes feature flag is disabled' do + before do + stub_feature_flags(search_rate_limited_scopes: false) + end + + it 'uses just current_user' do + %w[projects blobs users issues merge_requests].each do |scope| + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) + get :autocomplete, params: { term: 'hello', scope: scope } + end + end + end + end + it_behaves_like 'rate limited endpoint', rate_limit_key: :search_rate_limit do let(:current_user) { user } @@ -428,6 +521,15 @@ RSpec.describe SearchController, feature_category: :global_search do get :autocomplete, params: { term: 'setting', filter: 'generic' } end + + it 'sets correct cache control headers' do + get :autocomplete, params: { term: 'setting', filter: 'generic' } + + expect(response).to have_gitlab_http_status(:ok) + + expect(response.headers['Cache-Control']).to eq('max-age=60, private') + expect(response.headers['Pragma']).to be_nil + end end describe '#append_info_to_payload' do @@ -518,6 +620,11 @@ RSpec.describe SearchController, feature_category: :global_search do get endpoint, params: params.merge(project_id: project.id) end end + + it 'uses request IP as rate limiting scope' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit_unauthenticated, scope: [request.ip]) + get endpoint, params: params.merge(project_id: project.id) + end end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 1f7d169bae5..80856512bba 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -375,8 +375,7 @@ RSpec.describe SessionsController do context 'when OTP is valid for another user' do it 'does not authenticate' do - authenticate_2fa(login: another_user.username, - otp_attempt: another_user.current_otp) + authenticate_2fa(login: another_user.username, otp_attempt: another_user.current_otp) expect(subject.current_user).not_to eq another_user end @@ -384,8 +383,7 @@ RSpec.describe SessionsController do context 'when OTP is invalid for another user' do it 'does not authenticate' do - authenticate_2fa(login: another_user.username, - otp_attempt: 'invalid') + authenticate_2fa(login: another_user.username, otp_attempt: 'invalid') expect(subject.current_user).not_to eq another_user end @@ -495,51 +493,49 @@ RSpec.describe SessionsController do end end - context 'when using two-factor authentication via U2F device' do - let(:user) { create(:user, :two_factor) } + context 'when using two-factor authentication via WebAuthn device' do + let(:user) { create(:user, :two_factor_via_webauthn) } - def authenticate_2fa_u2f(user_params) + def authenticate_2fa(user_params) post(:create, params: { user: user_params }, session: { otp_user_id: user.id }) end - before do - stub_feature_flags(webauthn: false) - end - context 'remember_me field' do it 'sets a remember_user_token cookie when enabled' do - allow(U2fRegistration).to receive(:authenticate).and_return(true) + allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(true) allow(controller).to receive(:find_user).and_return(user) - expect(controller) - .to receive(:remember_me).with(user).and_call_original + expect(controller).to receive(:remember_me).with(user).and_call_original - authenticate_2fa_u2f(remember_me: '1', login: user.username, device_response: "{}") + authenticate_2fa(remember_me: '1', login: user.username, device_response: "{}") expect(response.cookies['remember_user_token']).to be_present end it 'does nothing when disabled' do - allow(U2fRegistration).to receive(:authenticate).and_return(true) + allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(true) allow(controller).to receive(:find_user).and_return(user) expect(controller).not_to receive(:remember_me) - authenticate_2fa_u2f(remember_me: '0', login: user.username, device_response: "{}") + authenticate_2fa(remember_me: '0', login: user.username, device_response: "{}") expect(response.cookies['remember_user_token']).to be_nil end end it "creates an audit log record" do - allow(U2fRegistration).to receive(:authenticate).and_return(true) - expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { AuditEvent.count }.by(1) - expect(AuditEvent.last.details[:with]).to eq("two-factor-via-u2f-device") + allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(true) + + expect { authenticate_2fa(login: user.username, device_response: "{}") }.to( + change { AuditEvent.count }.by(1)) + expect(AuditEvent.last.details[:with]).to eq("two-factor-via-webauthn-device") end it "creates an authentication event record" do - allow(U2fRegistration).to receive(:authenticate).and_return(true) + allow_any_instance_of(Webauthn::AuthenticateService).to receive(:execute).and_return(true) - expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { AuthenticationEvent.count }.by(1) - expect(AuthenticationEvent.last.provider).to eq("two-factor-via-u2f-device") + expect { authenticate_2fa(login: user.username, device_response: "{}") }.to( + change { AuthenticationEvent.count }.by(1)) + expect(AuthenticationEvent.last.provider).to eq("two-factor-via-webauthn-device") end end end @@ -567,8 +563,7 @@ RSpec.describe SessionsController do it 'sets the username and caller_id in the context' do expect(controller).to receive(:destroy).and_wrap_original do |m, *args| expect(Gitlab::ApplicationContext.current) - .to include('meta.user' => user.username, - 'meta.caller_id' => 'SessionsController#destroy') + .to include('meta.user' => user.username, 'meta.caller_id' => 'SessionsController#destroy') m.call(*args) end @@ -607,8 +602,7 @@ RSpec.describe SessionsController do m.call(*args) end - post(:create, - params: { user: { login: user.username, password: user.password.succ } }) + post :create, params: { user: { login: user.username, password: user.password.succ } } end end end diff --git a/spec/controllers/snippets/blobs_controller_spec.rb b/spec/controllers/snippets/blobs_controller_spec.rb index b9f58587a58..b92621d4041 100644 --- a/spec/controllers/snippets/blobs_controller_spec.rb +++ b/spec/controllers/snippets/blobs_controller_spec.rb @@ -17,13 +17,7 @@ RSpec.describe Snippets::BlobsController do let(:inline) { nil } subject do - get(:raw, - params: { - snippet_id: snippet, - path: filepath, - ref: ref, - inline: inline - }) + get :raw, params: { snippet_id: snippet, path: filepath, ref: ref, inline: inline } end where(:snippet_visibility_level, :user, :status) do |