diff options
Diffstat (limited to 'spec/controllers')
92 files changed, 1763 insertions, 1635 deletions
diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 32ac0f8dc07..253f66579f3 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -400,7 +400,7 @@ RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_set 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..edb17aefe86 100644 --- a/spec/controllers/admin/applications_controller_spec.rb +++ b/spec/controllers/admin/applications_controller_spec.rb @@ -38,44 +38,43 @@ 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 - - it 'creates the application' do - create_params = attributes_for(:application, trusted: true, confidential: false, scopes: ['api']) - - expect do - post :create, params: { doorkeeper_application: create_params } - end.to change { Doorkeeper::Application.count }.by(1) + describe 'PUT #renew' do + let(:oauth_params) do + { + id: application.id + } + end - application = Doorkeeper::Application.last + subject { put :renew, params: oauth_params } - expect(response).to redirect_to(admin_application_path(application)) - expect(application).to have_attributes(create_params.except(:uid, :owner_type)) - end - end + it { is_expected.to have_gitlab_http_status(:ok) } + it { expect { subject }.to change { application.reload.secret } } - 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 redirect_to(admin_application_url(application)) } + 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 +87,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/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/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index e75f27589d7..fed96f6a6c7 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -29,11 +29,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/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..ec2559550c3 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 diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f1adb9020fa..35e374d3b7f 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 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..a6a0f2c11b4 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: :planning_analytics 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 334c156e1ae..fca99d37000 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 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..e2ca19457ff --- /dev/null +++ b/spec/controllers/concerns/kas_cookie_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe KasCookie, feature_category: :kubernetes_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 +end diff --git a/spec/controllers/concerns/product_analytics_tracking_spec.rb b/spec/controllers/concerns/product_analytics_tracking_spec.rb index 12b4065b89c..b0074b52aa2 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] 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..3fe6d62329e 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -7,7 +7,7 @@ RSpec.describe SendFileUpload do Class.new(GitlabUploader) do include ObjectStorage::Concern - storage_options Gitlab.config.uploads + storage_location :uploads private 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..b1aa40d4d7b 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,71 @@ 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 - expect(response).to render_template(:new) - expect(flash[:alert]).to include _('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.') + 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 + + 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 + before do + stub_feature_flags(soft_email_confirmation: false) + end + 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..0e4771b20f7 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 total_user_projects_count' do + get :index + total_user_projects_count = assigns(:total_user_projects_count) + + expect(total_user_projects_count.count).to eq(2) + end + + it 'assigns the correct total_starred_projects_count' do + get :index + total_starred_projects_count = assigns(:total_starred_projects_count) + + expect(total_starred_projects_count.count).to eq(1) + expect(total_starred_projects_count).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 total_user_projects_count' do + get :index + total_user_projects_count = assigns(:total_user_projects_count) + + expect(total_user_projects_count.count).to eq(1) + expect(total_user_projects_count).not_to include(archived_project) + end + + it 'excludes archived project from total_starred_projects_count' do + get :index + total_starred_projects_count = assigns(:total_starred_projects_count) + + expect(total_starred_projects_count.count).to eq(0) + expect(total_starred_projects_count).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/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index 7aad67b01e8..cd72dbe394a 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 @@ -108,6 +109,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' 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 +208,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/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..35efcb664c0 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 diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index a3c4c47ab15..f4046cb97a0 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,12 +250,11 @@ 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)) @@ -390,21 +388,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 +410,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/settings/applications_controller_spec.rb b/spec/controllers/groups/settings/applications_controller_spec.rb index b9457770ed6..2fadac2dc17 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 - - 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']) + 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) - - 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']) + 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,55 @@ 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 } } + + 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 redirect_to(group_settings_application_url(group, application)) } + 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/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/help_controller_spec.rb b/spec/controllers/help_controller_spec.rb index 2375146f346..ac6715bacd5 100644 --- a/spec/controllers/help_controller_spec.rb +++ b/spec/controllers/help_controller_spec.rb @@ -261,11 +261,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 +273,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 +281,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..055c98ebdbc 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -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') diff --git a/spec/controllers/import/bulk_imports_controller_spec.rb b/spec/controllers/import/bulk_imports_controller_spec.rb index a3992ae850e..4e7f9572f65 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 @@ -270,8 +269,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..e2d59fc213a 100644 --- a/spec/controllers/import/fogbugz_controller_spec.rb +++ b/spec/controllers/import/fogbugz_controller_spec.rb @@ -116,8 +116,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/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 406a3604b23..4928cc46d7f 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' @@ -388,13 +386,22 @@ RSpec.describe Import::GithubController, feature_category: :importers do 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 +478,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/oauth/applications_controller_spec.rb b/spec/controllers/oauth/applications_controller_spec.rb index 9b16dc9a463..e7ec268a5a2 100644 --- a/spec/controllers/oauth/applications_controller_spec.rb +++ b/spec/controllers/oauth/applications_controller_spec.rb @@ -71,6 +71,33 @@ 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' + + 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 redirect_to(oauth_application_url(application)) } + end + end + describe 'GET #show' do subject { get :show, params: { id: application.id } } @@ -113,30 +140,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/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..c91aa562a85 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) } @@ -177,9 +179,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 +560,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 +570,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 +580,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 +596,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..f322c78b5e3 100644 --- a/spec/controllers/projects/blame_controller_spec.rb +++ b/spec/controllers/projects/blame_controller_spec.rb @@ -17,12 +17,7 @@ RSpec.describe Projects::BlameController 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, valid file" do @@ -35,8 +30,7 @@ RSpec.describe Projects::BlameController 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 diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 887a5ba598f..c091badd09d 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -75,13 +75,7 @@ RSpec.describe Projects::BlobController do 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 @@ -95,14 +89,7 @@ RSpec.describe Projects::BlobController do 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 @@ -115,12 +102,8 @@ RSpec.describe Projects::BlobController do 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 @@ -362,11 +345,23 @@ RSpec.describe Projects::BlobController do 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' } + let(:feature_flag_name) { 'route_hll_to_snowplow_phase4' } + end end end @@ -494,6 +489,7 @@ RSpec.describe Projects::BlobController do describe 'POST create' do let(:user) { create(:user) } + let(:target_event) { 'g_edit_by_sfe' } let(:default_params) do { namespace_id: project.namespace, @@ -515,10 +511,18 @@ RSpec.describe Projects::BlobController do 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' } + let(:feature_flag_name) { 'route_hll_to_snowplow_phase4' } + 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 a4f7c92f5cd..38f72c769f3 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -420,11 +420,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..36206a88786 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) } @@ -155,12 +155,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 +169,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 +182,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 +195,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 +219,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 +228,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 +237,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 +246,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 +300,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 +309,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 +318,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 +327,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 +383,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) @@ -478,8 +479,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..9e03d1f315b 100644 --- a/spec/controllers/projects/commits_controller_spec.rb +++ b/spec/controllers/projects/commits_controller_spec.rb @@ -18,11 +18,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 +30,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 @@ -78,13 +69,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 +83,45 @@ 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' } + end + + context 'when the show_tags_on_commits_view flag is disabled' do + let(:id) { "master/README.md" } + + before do + stub_feature_flags(show_tags_on_commits_view: false) + end + + it 'does not load tags' do + expect_next_instance_of(CommitCollection) do |collection| + expect(collection).not_to receive(:load_tags) + end + + get :show, params: { namespace_id: project.namespace, project_id: project, id: id } + 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 +141,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 +184,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/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/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_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 169fed1ab17..cbf632bfdb0 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -44,17 +44,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 +76,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 +86,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 +233,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' @@ -1016,98 +999,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..ae863918d14 100644 --- a/spec/controllers/projects/grafana_api_controller_spec.rb +++ b/spec/controllers/projects/grafana_api_controller_spec.rb @@ -87,13 +87,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({}) 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/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 9c272872a73..f1fe1940414 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -183,18 +183,6 @@ 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 @@ -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 @@ -1927,12 +1911,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..2e29d87dadd 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -106,9 +106,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 @@ -832,8 +833,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 +855,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 +869,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..356741fc4e2 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 @@ -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..a5dc351201d 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,9 +331,11 @@ 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 @@ -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..9e18089bb23 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 @@ -842,15 +837,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 +877,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 +1949,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 +2138,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 +2197,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/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/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..09b703a48d6 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -199,22 +199,36 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte check_pipeline_response(returned: 6, all: 6) end end + + context "with lazy_load_pipeline_dropdown_actions feature flag disabled" do + before do + stub_feature_flags(lazy_load_pipeline_dropdown_actions: false) + end + + it 'returns manual and scheduled actions' do + get_pipelines_index_json + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('pipeline') + + expect(json_response.dig('pipelines', 0, 'details')).to include('manual_actions') + expect(json_response.dig('pipelines', 0, 'details')).to include('scheduled_actions') + end + end end 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 +250,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) @@ -378,9 +395,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 +613,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 +784,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 +835,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 +877,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 +913,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 +973,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 +1037,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 @@ -1192,17 +1190,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 @@ -1455,10 +1447,9 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte private def get_config_variables - get :config_variables, params: { namespace_id: project.namespace, - project_id: project, - sha: ref }, - format: :json + get :config_variables, params: { + namespace_id: project.namespace, project_id: project, sha: ref + }, format: :json 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 a0d119baf16..0b1d0b75de7 100644 --- a/spec/controllers/projects/refs_controller_spec.rb +++ b/spec/controllers/projects/refs_controller_spec.rb @@ -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/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/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 9bc3065b6da..2b3adc719c1 100644 --- a/spec/controllers/projects/tree_controller_spec.rb +++ b/spec/controllers/projects/tree_controller_spec.rb @@ -21,12 +21,9 @@ RSpec.describe Projects::TreeController do before do expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original - 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 "valid branch, no path" do @@ -113,12 +110,9 @@ RSpec.describe Projects::TreeController do 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 @@ -128,12 +122,9 @@ RSpec.describe Projects::TreeController do 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 @@ -141,8 +132,7 @@ RSpec.describe Projects::TreeController do 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 @@ -151,15 +141,14 @@ RSpec.describe Projects::TreeController do 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_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 51f8a3b1197..cd6d3990309 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -508,11 +508,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 @@ -521,11 +517,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 @@ -543,10 +535,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 @@ -790,12 +779,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| @@ -811,22 +795,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 @@ -850,12 +824,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 @@ -975,13 +946,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 @@ -994,13 +961,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 @@ -1120,32 +1083,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 @@ -1160,12 +1110,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.')) @@ -1177,12 +1124,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)) @@ -1191,12 +1135,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 @@ -1289,6 +1231,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 @@ -1761,12 +1716,7 @@ RSpec.describe ProjectsController, feature_category: :projects do 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..3c631362119 100644 --- a/spec/controllers/registrations/welcome_controller_spec.rb +++ b/spec/controllers/registrations/welcome_controller_spec.rb @@ -2,7 +2,7 @@ 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 @@ -57,6 +57,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..92329b10426 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -75,7 +75,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 +122,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 +157,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 +166,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 - 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_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 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 +280,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 +750,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..ff24b754d7a 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -227,12 +227,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 @@ -428,6 +426,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 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 |