From 9dc93a4519d9d5d7be48ff274127136236a3adb3 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 20 Apr 2021 23:50:22 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-11-stable-ee --- .../admin/application_settings_controller_spec.rb | 9 +- spec/controllers/admin/clusters_controller_spec.rb | 14 +- .../admin/dev_ops_report_controller_spec.rb | 8 +- spec/controllers/admin/groups_controller_spec.rb | 6 + .../admin/impersonations_controller_spec.rb | 2 +- spec/controllers/admin/runners_controller_spec.rb | 4 +- spec/controllers/admin/services_controller_spec.rb | 2 +- spec/controllers/application_controller_spec.rb | 4 +- spec/controllers/boards/issues_controller_spec.rb | 2 +- spec/controllers/chaos_controller_spec.rb | 22 ++- .../concerns/enforces_admin_authentication_spec.rb | 6 +- spec/controllers/concerns/redis_tracking_spec.rb | 32 ++- spec/controllers/concerns/renders_commits_spec.rb | 12 ++ .../dashboard/snippets_controller_spec.rb | 19 +- .../explore/snippets_controller_spec.rb | 4 + spec/controllers/graphql_controller_spec.rb | 32 ++- spec/controllers/groups/boards_controller_spec.rb | 18 ++ .../controllers/groups/clusters_controller_spec.rb | 14 +- .../dependency_proxy_auth_controller_spec.rb | 2 + ...endency_proxy_for_containers_controller_spec.rb | 3 + .../groups/group_links_controller_spec.rb | 9 +- .../groups/group_members_controller_spec.rb | 24 ++- spec/controllers/groups/labels_controller_spec.rb | 18 ++ .../groups/milestones_controller_spec.rb | 10 +- .../registry/repositories_controller_spec.rb | 1 + spec/controllers/groups/runners_controller_spec.rb | 16 +- .../settings/applications_controller_spec.rb | 219 +++++++++++++++++++++ spec/controllers/groups/uploads_controller_spec.rb | 2 +- .../groups/variables_controller_spec.rb | 1 + spec/controllers/groups_controller_spec.rb | 113 ++++++++++- spec/controllers/invites_controller_spec.rb | 81 +++++++- .../oauth/authorizations_controller_spec.rb | 4 +- .../omniauth_callbacks_controller_spec.rb | 8 +- .../profiles/notifications_controller_spec.rb | 50 ++++- .../alerting/notifications_controller_spec.rb | 2 + .../projects/artifacts_controller_spec.rb | 6 +- .../controllers/projects/boards_controller_spec.rb | 18 ++ .../projects/clusters_controller_spec.rb | 14 +- .../controllers/projects/commit_controller_spec.rb | 98 +++++++++ .../cycle_analytics/events_controller_spec.rb | 2 +- .../designs/raw_images_controller_spec.rb | 12 +- .../designs/resized_image_controller_spec.rb | 1 + .../projects/discussions_controller_spec.rb | 4 +- .../projects/environments_controller_spec.rb | 1 + .../projects/feature_flags_controller_spec.rb | 1 + spec/controllers/projects/forks_controller_spec.rb | 45 +++-- .../projects/group_links_controller_spec.rb | 11 +- .../projects/imports_controller_spec.rb | 8 +- .../projects/incidents_controller_spec.rb | 1 + .../controllers/projects/issues_controller_spec.rb | 68 ++++--- spec/controllers/projects/jobs_controller_spec.rb | 1 + .../controllers/projects/labels_controller_spec.rb | 26 ++- .../merge_requests/content_controller_spec.rb | 19 +- .../merge_requests/creations_controller_spec.rb | 32 +++ .../merge_requests/drafts_controller_spec.rb | 2 +- .../projects/merge_requests_controller_spec.rb | 49 +++-- .../projects/milestones_controller_spec.rb | 12 +- spec/controllers/projects/notes_controller_spec.rb | 6 +- .../dashboards_controller_spec.rb | 1 + .../projects/pipelines_controller_spec.rb | 64 +++--- .../projects/pipelines_settings_controller_spec.rb | 1 + .../projects/project_members_controller_spec.rb | 8 +- spec/controllers/projects/raw_controller_spec.rb | 1 + .../registry/repositories_controller_spec.rb | 58 ++---- .../projects/registry/tags_controller_spec.rb | 7 +- .../projects/releases/evidences_controller_spec.rb | 3 +- .../projects/releases_controller_spec.rb | 1 + .../projects/repositories_controller_spec.rb | 22 --- .../projects/runners_controller_spec.rb | 4 +- .../projects/services_controller_spec.rb | 34 +++- .../settings/access_tokens_controller_spec.rb | 23 ++- .../projects/settings/ci_cd_controller_spec.rb | 1 + .../settings/operations_controller_spec.rb | 1 + .../projects/snippets_controller_spec.rb | 4 + .../projects/starrers_controller_spec.rb | 2 +- .../projects/static_site_editor_controller_spec.rb | 1 + spec/controllers/projects/todos_controller_spec.rb | 1 + .../projects/uploads_controller_spec.rb | 2 +- spec/controllers/projects_controller_spec.rb | 30 +-- .../registrations/welcome_controller_spec.rb | 22 ++- spec/controllers/root_controller_spec.rb | 20 -- spec/controllers/sessions_controller_spec.rb | 14 +- 82 files changed, 1170 insertions(+), 365 deletions(-) create mode 100644 spec/controllers/groups/settings/applications_controller_spec.rb (limited to 'spec/controllers') diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 2b562e2dd64..6258dd30438 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Admin::ApplicationSettingsController do +RSpec.describe Admin::ApplicationSettingsController, :do_not_mock_admin_mode_setting do include StubENV include UsageDataHelpers @@ -164,6 +164,13 @@ RSpec.describe Admin::ApplicationSettingsController do expect(ApplicationSetting.current.default_branch_name).to eq("example_branch_name") end + it "updates admin_mode setting" do + put :update, params: { application_setting: { admin_mode: true } } + + expect(response).to redirect_to(general_admin_application_settings_path) + expect(ApplicationSetting.current.admin_mode).to be(true) + end + context "personal access token prefix settings" do let(:application_settings) { ApplicationSetting.current } diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index 85aa77d8473..bd0c2965906 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -546,20 +546,30 @@ RSpec.describe Admin::ClustersController do describe 'GET #show' do let(:cluster) { create(:cluster, :provided_by_gcp, :instance) } - def get_show + def get_show(tab: nil) get :show, params: { - id: cluster + id: cluster, + tab: tab } end describe 'functionality' do + render_views + it 'responds successfully' do get_show expect(response).to have_gitlab_http_status(:ok) expect(assigns(:cluster)).to eq(cluster) end + + it 'renders integration tab view' do + get_show(tab: 'integrations') + + expect(response).to render_template('clusters/clusters/_integrations') + expect(response).to have_gitlab_http_status(:ok) + end end describe 'security' do diff --git a/spec/controllers/admin/dev_ops_report_controller_spec.rb b/spec/controllers/admin/dev_ops_report_controller_spec.rb index 913921b9630..142db175a15 100644 --- a/spec/controllers/admin/dev_ops_report_controller_spec.rb +++ b/spec/controllers/admin/dev_ops_report_controller_spec.rb @@ -5,7 +5,13 @@ require 'spec_helper' RSpec.describe Admin::DevOpsReportController do describe 'show_adoption?' do it 'is always false' do - expect(controller.show_adoption?).to be false + expect(controller.show_adoption?).to be_falsey + end + end + + describe 'should_track_devops_score?' do + it 'is always true' do + expect(controller.should_track_devops_score?).to be_truthy end end diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 38f4ce54e5c..8441a52b454 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -37,6 +37,12 @@ RSpec.describe Admin::GroupsController do post :create, params: { group: { path: 'test', name: 'test' } } end.to change { NamespaceSetting.count }.by(1) end + + it 'creates admin_note for group' do + expect do + post :create, params: { group: { path: 'test', name: 'test', admin_note_attributes: { note: 'test' } } } + end.to change { Namespace::AdminNote.count }.by(1) + end end describe 'PUT #members_update' do diff --git a/spec/controllers/admin/impersonations_controller_spec.rb b/spec/controllers/admin/impersonations_controller_spec.rb index 326003acaf8..744c0712d6b 100644 --- a/spec/controllers/admin/impersonations_controller_spec.rb +++ b/spec/controllers/admin/impersonations_controller_spec.rb @@ -42,7 +42,7 @@ RSpec.describe Admin::ImpersonationsController do context "when the impersonator is not admin (anymore)" do before do impersonator.admin = false - impersonator.save + impersonator.save! end it "responds with status 404" do diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index cba25dbff95..45ea8949bf2 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -125,7 +125,7 @@ RSpec.describe Admin::RunnersController do describe '#resume' do it 'marks the runner as active and ticks the queue' do - runner.update(active: false) + runner.update!(active: false) expect do post :resume, params: { id: runner.id } @@ -140,7 +140,7 @@ RSpec.describe Admin::RunnersController do describe '#pause' do it 'marks the runner as inactive and ticks the queue' do - runner.update(active: true) + runner.update!(active: true) expect do post :pause, params: { id: runner.id } diff --git a/spec/controllers/admin/services_controller_spec.rb b/spec/controllers/admin/services_controller_spec.rb index 8e78cc75369..d5ec9907b48 100644 --- a/spec/controllers/admin/services_controller_spec.rb +++ b/spec/controllers/admin/services_controller_spec.rb @@ -44,7 +44,7 @@ RSpec.describe Admin::ServicesController do describe "#update" do let(:project) { create(:project) } let!(:service_template) do - RedmineService.create( + RedmineService.create!( project: nil, active: false, template: true, diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 4a729008e67..3d34db6c2c0 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -56,8 +56,6 @@ RSpec.describe ApplicationController do end end - it_behaves_like 'a Trackable Controller' - describe '#add_gon_variables' do before do Gon.clear @@ -900,7 +898,7 @@ RSpec.describe ApplicationController do feature_category :issue_tracking def index - Labkit::Context.with_context do |context| + Gitlab::ApplicationContext.with_raw_context do |context| render json: context.to_h end end diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index a7f3ab0089f..d23f099e382 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -404,7 +404,7 @@ RSpec.describe Boards::IssuesController do list_id: list.try(:to_param) } - unless board.try(:parent)&.is_a?(Group) + unless board.try(:parent).is_a?(Group) params[:namespace_id] = project.namespace.to_param params[:project_id] = project end diff --git a/spec/controllers/chaos_controller_spec.rb b/spec/controllers/chaos_controller_spec.rb index cb4f12ff829..26ae4a6b693 100644 --- a/spec/controllers/chaos_controller_spec.rb +++ b/spec/controllers/chaos_controller_spec.rb @@ -109,7 +109,7 @@ RSpec.describe ChaosController do describe '#kill' do it 'calls synchronously' do - expect(Gitlab::Chaos).to receive(:kill).with(no_args) + expect(Gitlab::Chaos).to receive(:kill).with('KILL') get :kill @@ -117,7 +117,7 @@ RSpec.describe ChaosController do end it 'calls asynchronously' do - expect(Chaos::KillWorker).to receive(:perform_async).with(no_args) + expect(Chaos::KillWorker).to receive(:perform_async).with('KILL') get :kill, params: { async: 1 } @@ -125,6 +125,24 @@ RSpec.describe ChaosController do end end + describe '#quit' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:kill).with('QUIT') + + get :quit + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'calls asynchronously' do + expect(Chaos::KillWorker).to receive(:perform_async).with('QUIT') + + get :quit, params: { async: 1 } + + expect(response).to have_gitlab_http_status(:ok) + end + end + describe '#gc' do let(:gc_stat) { GC.stat.stringify_keys } diff --git a/spec/controllers/concerns/enforces_admin_authentication_spec.rb b/spec/controllers/concerns/enforces_admin_authentication_spec.rb index c6ad1a00484..106b1d53fd2 100644 --- a/spec/controllers/concerns/enforces_admin_authentication_spec.rb +++ b/spec/controllers/concerns/enforces_admin_authentication_spec.rb @@ -19,7 +19,7 @@ RSpec.describe EnforcesAdminAuthentication do end end - context 'feature flag :user_mode_in_session is enabled' do + context 'application setting :admin_mode is enabled' do describe 'authenticate_admin!' do context 'as an admin' do let(:user) { create(:admin) } @@ -61,9 +61,9 @@ RSpec.describe EnforcesAdminAuthentication do end end - context 'feature flag :user_mode_in_session is disabled' do + context 'application setting :admin_mode is disabled' do before do - stub_feature_flags(user_mode_in_session: false) + stub_application_setting(admin_mode: false) end describe 'authenticate_admin!' do diff --git a/spec/controllers/concerns/redis_tracking_spec.rb b/spec/controllers/concerns/redis_tracking_spec.rb index 53b49dd30a6..4077f4f5cce 100644 --- a/spec/controllers/concerns/redis_tracking_spec.rb +++ b/spec/controllers/concerns/redis_tracking_spec.rb @@ -9,8 +9,8 @@ RSpec.describe RedisTracking do include RedisTracking skip_before_action :authenticate_user!, only: :show - track_redis_hll_event :index, :show, name: 'g_compliance_approval_rules', - if: [:custom_condition_one?, :custom_condition_two?] + track_redis_hll_event(:index, :show, name: 'g_compliance_approval_rules', + if: [:custom_condition_one?, :custom_condition_two?]) { |controller| controller.get_custom_id } def index render html: 'index' @@ -24,6 +24,10 @@ RSpec.describe RedisTracking do render html: 'show' end + def get_custom_id + 'some_custom_id' + end + private def custom_condition_one? @@ -92,19 +96,15 @@ RSpec.describe RedisTracking do end end - context 'when user is not logged in and there is a visitor_id' do + context 'when user is not logged in' do let(:visitor_id) { SecureRandom.uuid } - before do - routes.draw { get 'show' => 'anonymous#show' } - end - - it 'tracks the event' do + it 'tracks the event when there is a visitor id' do cookies[:visitor_id] = { value: visitor_id, expires: 24.months } expect_tracking - get :show + get :show, params: { id: 1 } end end @@ -114,5 +114,19 @@ RSpec.describe RedisTracking do get :index end + + it 'tracks the event when there is custom id' do + expect_tracking + + get :show, params: { id: 1 } + end + + it 'does not track the event when there is no custom id' do + expect(controller).to receive(:get_custom_id).and_return(nil) + + expect_no_tracking + + get :show, params: { id: 2 } + end end end diff --git a/spec/controllers/concerns/renders_commits_spec.rb b/spec/controllers/concerns/renders_commits_spec.rb index 7be5f75c19d..7b241fc29af 100644 --- a/spec/controllers/concerns/renders_commits_spec.rb +++ b/spec/controllers/concerns/renders_commits_spec.rb @@ -57,4 +57,16 @@ RSpec.describe RendersCommits do end.not_to exceed_all_query_limit(control_count) end end + + describe '.prepare_commits_for_rendering' do + it 'avoids N+1' do + control = ActiveRecord::QueryRecorder.new do + subject.prepare_commits_for_rendering(merge_request.commits.take(1)) + end + + expect do + subject.prepare_commits_for_rendering(merge_request.commits) + end.not_to exceed_all_query_limit(control.count) + end + end end diff --git a/spec/controllers/dashboard/snippets_controller_spec.rb b/spec/controllers/dashboard/snippets_controller_spec.rb index 016a9f53129..180369447b4 100644 --- a/spec/controllers/dashboard/snippets_controller_spec.rb +++ b/spec/controllers/dashboard/snippets_controller_spec.rb @@ -29,23 +29,6 @@ RSpec.describe Dashboard::SnippetsController do it_behaves_like 'snippets sort order' - context 'when views are rendered' do - render_views - - it 'avoids N+1 database queries' do - # Warming call to load everything non snippet related - get(:index) - - project = create(:project, namespace: user.namespace) - create(:project_snippet, project: project, author: user) - - control_count = ActiveRecord::QueryRecorder.new { get(:index) }.count - - project = create(:project, namespace: user.namespace) - create(:project_snippet, project: project, author: user) - - expect { get(:index) }.not_to exceed_query_limit(control_count) - end - end + it_behaves_like 'snippets views' end end diff --git a/spec/controllers/explore/snippets_controller_spec.rb b/spec/controllers/explore/snippets_controller_spec.rb index f7bd2ba917e..e93e8502dfc 100644 --- a/spec/controllers/explore/snippets_controller_spec.rb +++ b/spec/controllers/explore/snippets_controller_spec.rb @@ -32,5 +32,9 @@ RSpec.describe Explore::SnippetsController do expect(assigns(:snippets)).to all(be_a(PersonalSnippet)) expect(response).to have_gitlab_http_status(:ok) end + + it_behaves_like 'snippets views' do + let_it_be(:user) { create(:user) } + end end end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index f10fbf5ef2c..f2d86b1b166 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -175,22 +175,44 @@ RSpec.describe GraphqlController do end describe '#append_info_to_payload' do - let(:graphql_query) { graphql_query_for('project', { 'fullPath' => 'foo' }, %w(id name)) } - let(:mock_store) { { graphql_logs: { foo: :bar } } } + let(:query_1) { { query: graphql_query_for('project', { 'fullPath' => 'foo' }, %w(id name), 'getProject_1') } } + let(:query_2) { { query: graphql_query_for('project', { 'fullPath' => 'bar' }, %w(id), 'getProject_2') } } + let(:graphql_queries) { [query_1, query_2] } let(:log_payload) { {} } + let(:expected_logs) do + [ + { + operation_name: 'getProject_1', + complexity: 3, + depth: 2, + used_deprecated_fields: [], + used_fields: ['Project.id', 'Project.name', 'Query.project'], + variables: '{}' + }, + { + operation_name: 'getProject_2', + complexity: 2, + depth: 2, + used_deprecated_fields: [], + used_fields: ['Project.id', 'Query.project'], + variables: '{}' + } + ] + end before do - allow(RequestStore).to receive(:store).and_return(mock_store) + RequestStore.clear! + allow(controller).to receive(:append_info_to_payload).and_wrap_original do |method, *| method.call(log_payload) end end it 'appends metadata for logging' do - post :execute, params: { query: graphql_query, operationName: 'Foo' } + post :execute, params: { _json: graphql_queries } expect(controller).to have_received(:append_info_to_payload) - expect(log_payload.dig(:metadata, :graphql)).to eq({ operation_name: 'Foo', foo: :bar }) + expect(log_payload.dig(:metadata, :graphql)).to match_array(expected_logs) end end end diff --git a/spec/controllers/groups/boards_controller_spec.rb b/spec/controllers/groups/boards_controller_spec.rb index 6201cddecb0..ca4931bdc90 100644 --- a/spec/controllers/groups/boards_controller_spec.rb +++ b/spec/controllers/groups/boards_controller_spec.rb @@ -16,6 +16,15 @@ RSpec.describe Groups::BoardsController do expect { list_boards }.to change(group.boards, :count).by(1) end + it 'pushes swimlanes_buffered_rendering feature flag' do + allow(controller).to receive(:push_frontend_feature_flag).and_call_original + + expect(controller).to receive(:push_frontend_feature_flag) + .with(:swimlanes_buffered_rendering, group, default_enabled: :yaml) + + list_boards + end + context 'when format is HTML' do it 'renders template' do list_boards @@ -98,6 +107,15 @@ RSpec.describe Groups::BoardsController do describe 'GET show' do let!(:board) { create(:board, group: group) } + it 'pushes swimlanes_buffered_rendering feature flag' do + allow(controller).to receive(:push_frontend_feature_flag).and_call_original + + expect(controller).to receive(:push_frontend_feature_flag) + .with(:swimlanes_buffered_rendering, group, default_enabled: :yaml) + + read_board board: board + end + context 'when format is HTML' do it 'renders template' do expect { read_board board: board }.to change(BoardGroupRecentVisit, :count).by(1) diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 1334372a1f5..93c560b4753 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -641,21 +641,31 @@ RSpec.describe Groups::ClustersController do describe 'GET show' do let(:cluster) { create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group]) } - def go + def go(tab: nil) get :show, params: { group_id: group, - id: cluster + id: cluster, + tab: tab } end describe 'functionality' do + render_views + it 'renders view' do go expect(response).to have_gitlab_http_status(:ok) expect(assigns(:cluster)).to eq(cluster) end + + it 'renders integration tab view', :aggregate_failures do + go(tab: 'integrations') + + expect(response).to render_template('clusters/clusters/_integrations') + expect(response).to have_gitlab_http_status(:ok) + end end describe 'security' do diff --git a/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb b/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb index 857e0570621..f67b2022219 100644 --- a/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb +++ b/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb @@ -31,6 +31,7 @@ RSpec.describe Groups::DependencyProxyAuthController do context 'with valid JWT' do let_it_be(:user) { create(:user) } + let(:jwt) { build_jwt(user) } let(:token_header) { "Bearer #{jwt.encoded}" } @@ -65,6 +66,7 @@ RSpec.describe Groups::DependencyProxyAuthController do context 'expired token' do let_it_be(:user) { create(:user) } + let(:jwt) { build_jwt(user, expire_time: Time.zone.now - 1.hour) } let(:token_header) { "Bearer #{jwt.encoded}" } 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 83775dcdbdf..9f30a850ca2 100644 --- a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb +++ b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do include DependencyProxyHelpers let_it_be(:user) { create(:user) } + let(:group) { create(:group) } let(:token_response) { { status: :success, token: 'abcd1234' } } let(:jwt) { build_jwt(user) } @@ -102,6 +103,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do describe 'GET #manifest' do let_it_be(:manifest) { create(:dependency_proxy_manifest) } + let(:pull_response) { { status: :success, manifest: manifest } } before do @@ -182,6 +184,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do describe 'GET #blob' do let_it_be(:blob) { create(:dependency_proxy_blob) } + let(:blob_sha) { blob.file_name.sub('.gz', '') } let(:blob_response) { { status: :success, blob: blob } } diff --git a/spec/controllers/groups/group_links_controller_spec.rb b/spec/controllers/groups/group_links_controller_spec.rb index a2f7161ca41..94d3c1ffa0f 100644 --- a/spec/controllers/groups/group_links_controller_spec.rb +++ b/spec/controllers/groups/group_links_controller_spec.rb @@ -9,16 +9,17 @@ RSpec.describe Groups::GroupLinksController do let(:group_member) { create(:user) } let!(:project) { create(:project, group: shared_group) } - around do |example| - travel_to DateTime.new(2019, 4, 1) { example.run } - end - before do + travel_to DateTime.new(2019, 4, 1) sign_in(user) shared_with_group.add_developer(group_member) end + after do + travel_back + end + shared_examples 'placeholder is passed as `id` parameter' do |action| it 'returns a 404' do post( diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index ff7a7f55863..19655687028 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -9,8 +9,12 @@ RSpec.describe Groups::GroupMembersController do let(:group) { create(:group, :public) } let(:membership) { create(:group_member, group: group) } - around do |example| - travel_to DateTime.new(2019, 4, 1) { example.run } + before do + travel_to DateTime.new(2019, 4, 1) + end + + after do + travel_back end describe 'GET index' do @@ -288,7 +292,9 @@ RSpec.describe Groups::GroupMembersController do end describe 'DELETE destroy' do - let(:member) { create(:group_member, :developer, group: group) } + let(:sub_group) { create(:group, parent: group) } + let!(:member) { create(:group_member, :developer, group: group) } + let!(:sub_member) { create(:group_member, :developer, group: sub_group, user: member.user) } before do sign_in(user) @@ -324,9 +330,19 @@ RSpec.describe Groups::GroupMembersController do it '[HTML] removes user from members' do delete :destroy, params: { group_id: group, id: member } - expect(response).to set_flash.to 'User was successfully removed from group and any subresources.' + expect(response).to set_flash.to 'User was successfully removed from group.' + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.members).not_to include member + expect(sub_group.members).to include sub_member + end + + it '[HTML] removes user from members including subgroups and projects' do + delete :destroy, params: { group_id: group, id: member, remove_sub_memberships: true } + + expect(response).to set_flash.to 'User was successfully removed from group and any subgroups and projects.' expect(response).to redirect_to(group_group_members_path(group)) expect(group.members).not_to include member + expect(sub_group.members).not_to include sub_member end it '[JS] removes user from members' do diff --git a/spec/controllers/groups/labels_controller_spec.rb b/spec/controllers/groups/labels_controller_spec.rb index b2320615778..90da40cd5f0 100644 --- a/spec/controllers/groups/labels_controller_spec.rb +++ b/spec/controllers/groups/labels_controller_spec.rb @@ -46,6 +46,24 @@ RSpec.describe Groups::LabelsController do it_behaves_like 'disabled when using an external authorization service' end + + context 'with views rendered' do + render_views + + before do + get :index, params: { group_id: group.to_param } + end + + it 'avoids N+1 queries' do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { get :index, params: { group_id: group.to_param } } + + create_list(:group_label, 3, group: group) + + # some n+1 queries still exist + expect { get :index, params: { group_id: group.to_param } }.not_to exceed_all_query_limit(control.count).with_threshold(10) + expect(assigns(:labels).count).to eq(4) + end + end end describe 'POST #toggle_subscription' do diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index 05e93da18e7..a3c4c47ab15 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -322,7 +322,7 @@ RSpec.describe Groups::MilestonesController do end context 'when requesting a redirected path' do - let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + let(:redirect_route) { group.redirect_routes.create!(path: 'old-path') } it 'redirects to the canonical path' do get :merge_requests, params: { group_id: redirect_route.path, id: title } @@ -350,7 +350,7 @@ RSpec.describe Groups::MilestonesController do end context 'when the old group path is a substring of the scheme or host' do - let(:redirect_route) { group.redirect_routes.create(path: 'http') } + let(:redirect_route) { group.redirect_routes.create!(path: 'http') } it 'does not modify the requested host' do get :merge_requests, params: { group_id: redirect_route.path, id: title } @@ -362,7 +362,7 @@ RSpec.describe Groups::MilestonesController do context 'when the old group path is substring of groups' do # I.e. /groups/oups should not become /grfoo/oups - let(:redirect_route) { group.redirect_routes.create(path: 'oups') } + let(:redirect_route) { group.redirect_routes.create!(path: 'oups') } it 'does not modify the /groups part of the path' do get :merge_requests, params: { group_id: redirect_route.path, id: title } @@ -374,7 +374,7 @@ RSpec.describe Groups::MilestonesController do context 'when the old group path is substring of groups plus the new path' do # I.e. /groups/oups/oup should not become /grfoos - let(:redirect_route) { group.redirect_routes.create(path: 'oups/oup') } + let(:redirect_route) { group.redirect_routes.create!(path: 'oups/oup') } it 'does not modify the /groups part of the path' do get :merge_requests, params: { group_id: redirect_route.path, id: title } @@ -411,7 +411,7 @@ RSpec.describe Groups::MilestonesController do end context 'when requesting a redirected path' do - let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + let(:redirect_route) { group.redirect_routes.create!(path: 'old-path') } it 'returns not found' do post :create, diff --git a/spec/controllers/groups/registry/repositories_controller_spec.rb b/spec/controllers/groups/registry/repositories_controller_spec.rb index 70125087f30..35c9a80266e 100644 --- a/spec/controllers/groups/registry/repositories_controller_spec.rb +++ b/spec/controllers/groups/registry/repositories_controller_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Groups::Registry::RepositoriesController do let_it_be(:user) { create(:user) } let_it_be(:guest) { create(:user) } let_it_be(:group, reload: true) { create(:group) } + let(:additional_parameters) { {} } subject do diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb index 91ff0a53ec7..d6da9a4e8d0 100644 --- a/spec/controllers/groups/runners_controller_spec.rb +++ b/spec/controllers/groups/runners_controller_spec.rb @@ -220,7 +220,7 @@ RSpec.describe Groups::RunnersController do end it 'marks the runner as active, ticks the queue, and redirects' do - runner.update(active: false) + runner.update!(active: false) expect do post :resume, params: params @@ -231,7 +231,7 @@ RSpec.describe Groups::RunnersController do end it 'marks the project runner as active, ticks the queue, and redirects' do - runner_project.update(active: false) + runner_project.update!(active: false) expect do post :resume, params: params_runner_project @@ -248,7 +248,7 @@ RSpec.describe Groups::RunnersController do end it 'responds 404 and does not activate the runner' do - runner.update(active: false) + runner.update!(active: false) expect do post :resume, params: params @@ -259,7 +259,7 @@ RSpec.describe Groups::RunnersController do end it 'responds 404 and does not activate the project runner' do - runner_project.update(active: false) + runner_project.update!(active: false) expect do post :resume, params: params_runner_project @@ -278,7 +278,7 @@ RSpec.describe Groups::RunnersController do end it 'marks the runner as inactive, ticks the queue, and redirects' do - runner.update(active: true) + runner.update!(active: true) expect do post :pause, params: params @@ -289,7 +289,7 @@ RSpec.describe Groups::RunnersController do end it 'marks the project runner as inactive, ticks the queue, and redirects' do - runner_project.update(active: true) + runner_project.update!(active: true) expect do post :pause, params: params_runner_project @@ -306,7 +306,7 @@ RSpec.describe Groups::RunnersController do end it 'responds 404 and does not update the runner or queue' do - runner.update(active: true) + runner.update!(active: true) expect do post :pause, params: params @@ -317,7 +317,7 @@ RSpec.describe Groups::RunnersController do end it 'responds 404 and does not update the project runner or queue' do - runner_project.update(active: true) + runner_project.update!(active: true) expect do post :pause, params: params diff --git a/spec/controllers/groups/settings/applications_controller_spec.rb b/spec/controllers/groups/settings/applications_controller_spec.rb new file mode 100644 index 00000000000..0804a5536e0 --- /dev/null +++ b/spec/controllers/groups/settings/applications_controller_spec.rb @@ -0,0 +1,219 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::Settings::ApplicationsController do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:application) { create(:oauth_application, owner_id: group.id, owner_type: 'Namespace') } + + before do + sign_in(user) + end + + describe 'GET #index' do + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'renders the application form' do + get :index, params: { group_id: group } + + expect(response).to render_template :index + expect(assigns[:scopes]).to be_kind_of(Doorkeeper::OAuth::Scopes) + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + get :index, params: { group_id: group } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'GET #edit' do + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'renders the application form' do + get :edit, params: { group_id: group, id: application.id } + + expect(response).to render_template :edit + expect(assigns[:scopes]).to be_kind_of(Doorkeeper::OAuth::Scopes) + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + get :edit, params: { group_id: group, id: application.id } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'POST #create' do + context 'when user is owner' do + before do + group.add_owner(user) + 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 redirect_to(group_settings_application_path(group, application)) + expect(application).to have_attributes(create_params.except(:uid, :owner_type)) + end + + it 'renders the application form on errors' do + expect do + post :create, params: { group_id: group, doorkeeper_application: attributes_for(:application).merge(redirect_uri: nil) } + end.not_to change { Doorkeeper::Application.count } + + expect(response).to render_template :index + expect(assigns[:scopes]).to be_kind_of(Doorkeeper::OAuth::Scopes) + end + + context 'when the params are for a confidential application' do + 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 'when scopes are not present' do + it 'renders the application form on errors' do + create_params = attributes_for(:application, trusted: true, confidential: false) + + expect do + post :create, params: { group_id: group, doorkeeper_application: create_params } + end.not_to change { Doorkeeper::Application.count } + + expect(response).to render_template :index + end + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + create_params = attributes_for(:application, trusted: true, confidential: false, scopes: ['api']) + + post :create, params: { group_id: group, doorkeeper_application: create_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 + group.add_owner(user) + end + + it 'updates the application' do + doorkeeper_params = { redirect_uri: 'http://example.com/', trusted: true, confidential: false } + + patch :update, params: { group_id: group, id: application.id, doorkeeper_application: doorkeeper_params } + + application.reload + + expect(response).to redirect_to(group_settings_application_path(group, application)) + expect(application) + .to have_attributes(redirect_uri: 'http://example.com/', trusted: false, confidential: false) + end + + it 'renders the application form on errors' do + patch :update, params: { group_id: group, id: application.id, doorkeeper_application: { redirect_uri: nil } } + + expect(response).to render_template :edit + expect(assigns[:scopes]).to be_kind_of(Doorkeeper::OAuth::Scopes) + end + + context 'when updating the application to be confidential' do + it 'successfully sets the application to confidential' do + doorkeeper_params = { confidential: true } + + patch :update, params: { group_id: group, id: application.id, doorkeeper_application: doorkeeper_params } + + application.reload + + expect(response).to redirect_to(group_settings_application_path(group, application)) + expect(application).to be_confidential + end + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + doorkeeper_params = { redirect_uri: 'http://example.com/', trusted: true, confidential: false } + + patch :update, params: { group_id: group, id: application.id, doorkeeper_application: doorkeeper_params } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'DELETE #destroy' do + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'deletes the application' do + delete :destroy, params: { group_id: group, id: application.id } + + expect(Doorkeeper::Application.exists?(application.id)).to be_falsy + expect(response).to redirect_to(group_settings_applications_url(group)) + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + delete :destroy, params: { group_id: group, id: application.id } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/controllers/groups/uploads_controller_spec.rb b/spec/controllers/groups/uploads_controller_spec.rb index ea6a5ce8841..7dafb813545 100644 --- a/spec/controllers/groups/uploads_controller_spec.rb +++ b/spec/controllers/groups/uploads_controller_spec.rb @@ -23,7 +23,7 @@ RSpec.describe Groups::UploadsController do let!(:upload) { create(:upload, :issuable_upload, :with_file, model: model) } let(:group) { model } let(:old_path) { group.to_param + 'old' } - let!(:redirect_route) { model.redirect_routes.create(path: old_path) } + let!(:redirect_route) { model.redirect_routes.create!(path: old_path) } let(:upload_path) { File.basename(upload.path) } it 'redirects to a file with the proper extension' do diff --git a/spec/controllers/groups/variables_controller_spec.rb b/spec/controllers/groups/variables_controller_spec.rb index a450a4afb02..8c0aa83b9c4 100644 --- a/spec/controllers/groups/variables_controller_spec.rb +++ b/spec/controllers/groups/variables_controller_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Groups::VariablesController do let_it_be(:group) { create(:group) } let_it_be(:user) { create(:user) } let_it_be(:variable) { create(:ci_group_variable, group: group) } + let(:access_level) { :owner } before do diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index cce61c4534b..f47eac7ac25 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -312,6 +312,64 @@ RSpec.describe GroupsController, factory_default: :keep do end end end + + context 'when creating a group with captcha protection' do + before do + sign_in(user) + + stub_application_setting(recaptcha_enabled: true) + end + + after do + # Avoid test ordering issue and ensure `verify_recaptcha` returns true + unless Recaptcha.configuration.skip_verify_env.include?('test') + Recaptcha.configuration.skip_verify_env << 'test' + end + end + + it 'displays an error when the reCAPTCHA is not solved' do + allow(controller).to receive(:verify_recaptcha).and_return(false) + + post :create, params: { group: { name: 'new_group', path: "new_group" } } + + expect(response).to render_template(:new) + expect(flash[:alert]).to eq(_('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.')) + end + + it 'allows creating a group when the reCAPTCHA is solved' do + expect do + post :create, params: { group: { name: 'new_group', path: "new_group" } } + end.to change { Group.count }.by(1) + + expect(response).to have_gitlab_http_status(:found) + end + + it 'allows creating a sub-group without checking the captcha' do + expect(controller).not_to receive(:verify_recaptcha) + + expect do + post :create, params: { group: { name: 'new_group', path: "new_group", parent_id: group.id } } + end.to change { Group.count }.by(1) + + expect(response).to have_gitlab_http_status(:found) + end + + context 'with feature flag switched off' do + before do + stub_feature_flags(recaptcha_on_top_level_group_creation: false) + end + + it 'allows creating a group without the reCAPTCHA' do + expect(controller).not_to receive(:verify_recaptcha) + + expect do + post :create, params: { group: { name: 'new_group', path: "new_group" } } + end.to change { Group.count }.by(1) + + expect(response).to have_gitlab_http_status(:found) + end + end + end end describe 'GET #index' do @@ -556,6 +614,43 @@ RSpec.describe GroupsController, factory_default: :keep do end end + context "updating :resource_access_token_creation_allowed" do + subject do + put :update, + params: { + id: group.to_param, + group: { resource_access_token_creation_allowed: false } + } + end + + context 'when user is a group owner' do + before do + group.add_owner(user) + sign_in(user) + end + + it "updates the attribute" do + expect { subject } + .to change { group.namespace_settings.reload.resource_access_token_creation_allowed } + .from(true) + .to(false) + + expect(response).to have_gitlab_http_status(:found) + end + end + + context 'when not a group owner' do + before do + group.add_developer(user) + sign_in(user) + end + + it "does not update the attribute" do + expect { subject }.not_to change { group.namespace_settings.reload.resource_access_token_creation_allowed } + end + end + end + describe '#ensure_canonical_path' do before do sign_in(user) @@ -578,7 +673,7 @@ RSpec.describe GroupsController, factory_default: :keep do end context 'when requesting a redirected path' do - let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + let(:redirect_route) { group.redirect_routes.create!(path: 'old-path') } let(:group_full_path) { redirect_route.path } it 'redirects to the canonical path' do @@ -587,7 +682,7 @@ RSpec.describe GroupsController, factory_default: :keep do end context 'when the old group path is a substring of the scheme or host' do - let(:redirect_route) { group.redirect_routes.create(path: 'http') } + let(:redirect_route) { group.redirect_routes.create!(path: 'http') } it 'does not modify the requested host' do expect(response).to redirect_to(group) @@ -597,7 +692,7 @@ RSpec.describe GroupsController, factory_default: :keep do context 'when the old group path is substring of groups' do # I.e. /groups/oups should not become /grfoo/oups - let(:redirect_route) { group.redirect_routes.create(path: 'oups') } + let(:redirect_route) { group.redirect_routes.create!(path: 'oups') } it 'does not modify the /groups part of the path' do expect(response).to redirect_to(group) @@ -649,7 +744,7 @@ RSpec.describe GroupsController, factory_default: :keep do end context 'when requesting a redirected path' do - let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + let(:redirect_route) { group.redirect_routes.create!(path: 'old-path') } it 'redirects to the canonical path' do get :issues, params: { id: redirect_route.path } @@ -659,7 +754,7 @@ RSpec.describe GroupsController, factory_default: :keep do end context 'when the old group path is a substring of the scheme or host' do - let(:redirect_route) { group.redirect_routes.create(path: 'http') } + let(:redirect_route) { group.redirect_routes.create!(path: 'http') } it 'does not modify the requested host' do get :issues, params: { id: redirect_route.path } @@ -671,7 +766,7 @@ RSpec.describe GroupsController, factory_default: :keep do context 'when the old group path is substring of groups' do # I.e. /groups/oups should not become /grfoo/oups - let(:redirect_route) { group.redirect_routes.create(path: 'oups') } + let(:redirect_route) { group.redirect_routes.create!(path: 'oups') } it 'does not modify the /groups part of the path' do get :issues, params: { id: redirect_route.path } @@ -683,7 +778,7 @@ RSpec.describe GroupsController, factory_default: :keep do context 'when the old group path is substring of groups plus the new path' do # I.e. /groups/oups/oup should not become /grfoos - let(:redirect_route) { group.redirect_routes.create(path: 'oups/oup') } + let(:redirect_route) { group.redirect_routes.create!(path: 'oups/oup') } it 'does not modify the /groups part of the path' do get :issues, params: { id: redirect_route.path } @@ -711,7 +806,7 @@ RSpec.describe GroupsController, factory_default: :keep do end context 'when requesting a redirected path' do - let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + let(:redirect_route) { group.redirect_routes.create!(path: 'old-path') } it 'returns not found' do post :update, params: { id: redirect_route.path, group: { path: 'new_path' } } @@ -737,7 +832,7 @@ RSpec.describe GroupsController, factory_default: :keep do end context 'when requesting a redirected path' do - let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + let(:redirect_route) { group.redirect_routes.create!(path: 'old-path') } it 'returns not found' do delete :destroy, params: { id: redirect_route.path } diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index a8d38d12f23..5195f482084 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe InvitesController do let_it_be(:user) { create(:user) } - let(:member) { create(:project_member, :invited, invite_email: user.email) } + let_it_be(:member, reload: true) { create(:project_member, :invited, invite_email: user.email) } let(:raw_invite_token) { member.raw_invite_token } let(:project_members) { member.source.users } let(:md5_member_global_id) { Digest::MD5.hexdigest(member.to_global_id.to_s) } @@ -77,10 +77,83 @@ RSpec.describe InvitesController do context 'when not logged in' do context 'when inviter is a member' do - it 'is redirected to a new session with invite email param' do - request + context 'when instance allows sign up' do + it 'indicates an account can be created in notice' do + request + + expect(flash[:notice]).to include('or create an account') + end + + context 'when user exists with the invited email' do + it 'is redirected to a new session with invite email param' do + request + + expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + end + end + + context 'when user exists with the invited email as secondary email' do + before do + secondary_email = create(:email, user: user, email: 'foo@example.com') + member.update!(invite_email: secondary_email.email) + end + + it 'is redirected to a new session with invite email param' do + request + + expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + end + end + + context 'when user does not exist with the invited email' do + before do + member.update!(invite_email: 'bogus_email@example.com') + end + + it 'indicates an account can be created in notice' do + request + + expect(flash[:notice]).to include('create an account or sign in') + end + + it 'is redirected to a new registration with invite email param' do + request + + expect(response).to redirect_to(new_user_registration_path(invite_email: member.invite_email)) + end + end + end + + context 'when instance does not allow sign up' do + before do + stub_application_setting(allow_signup?: false) + end + + it 'does not indicate an account can be created in notice' do + request + + expect(flash[:notice]).not_to include('or create an account') + end + + context 'when user exists with the invited email' do + it 'is redirected to a new session with invite email param' do + request + + expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + end + end + + context 'when user does not exist with the invited email' do + before do + member.update!(invite_email: 'bogus_email@example.com') + end + + it 'is redirected to a new session with invite email param' do + request - expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + end + end end end diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index 2df94a06b3e..21124299b25 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -54,7 +54,7 @@ RSpec.describe Oauth::AuthorizationsController do shared_examples "Implicit grant can't be used in confidential application" do context 'when application is confidential' do before do - application.update(confidential: true) + application.update!(confidential: true) params[:response_type] = 'token' end @@ -96,7 +96,7 @@ RSpec.describe Oauth::AuthorizationsController do end it 'deletes session.user_return_to and redirects when skip authorization' do - application.update(trusted: true) + application.update!(trusted: true) request.session['user_return_to'] = 'http://example.com' subject diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index edd587389cb..4a47a4a2a53 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -60,7 +60,7 @@ RSpec.describe OmniauthCallbacksController, type: :controller do let(:extern_uid) { 'my-uid' } before do - user.update(failed_attempts: User.maximum_attempts.pred) + user.update!(failed_attempts: User.maximum_attempts.pred) subject.response = ActionDispatch::Response.new end @@ -233,7 +233,7 @@ RSpec.describe OmniauthCallbacksController, type: :controller do before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') settings = Gitlab::CurrentSettings.current_application_settings - settings.update(disabled_oauth_sign_in_sources: [provider.to_s]) + settings.update!(disabled_oauth_sign_in_sources: [provider.to_s]) end it 'prevents login via POST' do @@ -299,7 +299,7 @@ RSpec.describe OmniauthCallbacksController, type: :controller do before do stub_omniauth_setting(enabled: true, auto_link_user: true, allow_single_sign_on: ['atlassian_oauth2']) - user.destroy + user.destroy! end it 'denies sign-in if sign-up is enabled, but block_auto_created_users is set' do @@ -381,7 +381,7 @@ RSpec.describe OmniauthCallbacksController, type: :controller do context 'sign up' do before do - user.destroy + user.destroy! end it 'denies login if sign up is enabled, but block_auto_created_users is set' do diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb index 03749366703..1ebf4363ba6 100644 --- a/spec/controllers/profiles/notifications_controller_spec.rb +++ b/spec/controllers/profiles/notifications_controller_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' RSpec.describe Profiles::NotificationsController do let(:user) do create(:user) do |user| - user.emails.create(email: 'original@example.com', confirmed_at: Time.current) - user.emails.create(email: 'new@example.com', confirmed_at: Time.current) + user.emails.create!(email: 'original@example.com', confirmed_at: Time.current) + user.emails.create!(email: 'new@example.com', confirmed_at: Time.current) user.notification_email = 'original@example.com' user.save! end @@ -21,6 +21,30 @@ RSpec.describe Profiles::NotificationsController do expect(response).to render_template :show end + context 'when personal projects are present', :request_store do + let!(:personal_project_1) { create(:project, namespace: user.namespace) } + + context 'N+1 query check' do + render_views + + it 'does not have an N+1' do + sign_in(user) + + get :show + + control = ActiveRecord::QueryRecorder.new do + get :show + end + + create_list(:project, 2, namespace: user.namespace) + + expect do + get :show + end.not_to exceed_query_limit(control) + end + end + end + context 'with groups that do not have notification preferences' do let_it_be(:group) { create(:group) } let_it_be(:subgroup) { create(:group, parent: group) } @@ -37,18 +61,24 @@ RSpec.describe Profiles::NotificationsController do expect(assigns(:group_notifications).map(&:source_id)).to include(subgroup.id) end - it 'does not have an N+1' do - sign_in(user) + context 'N+1 query check' do + render_views + + it 'does not have an N+1' do + sign_in(user) - control = ActiveRecord::QueryRecorder.new do get :show - end - create_list(:group, 2, parent: group) + control = ActiveRecord::QueryRecorder.new do + get :show + end - expect do - get :show - end.not_to exceed_query_limit(control) + create_list(:group, 2, parent: group) + + expect do + get :show + end.not_to exceed_query_limit(control) + end end end diff --git a/spec/controllers/projects/alerting/notifications_controller_spec.rb b/spec/controllers/projects/alerting/notifications_controller_spec.rb index 3656cfbcc30..fe0c4ce00bf 100644 --- a/spec/controllers/projects/alerting/notifications_controller_spec.rb +++ b/spec/controllers/projects/alerting/notifications_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Projects::Alerting::NotificationsController do let_it_be(:project) { create(:project) } let_it_be(:environment) { create(:environment, project: project) } + let(:params) { project_params } describe 'POST #create' do @@ -68,6 +69,7 @@ RSpec.describe Projects::Alerting::NotificationsController do context 'with a corresponding integration' do context 'with integration parameters specified' do let_it_be_with_reload(:integration) { create(:alert_management_http_integration, project: project) } + let(:params) { project_params(endpoint_identifier: integration.endpoint_identifier, name: integration.name) } context 'the integration is active' do diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index 69ab9873b90..754b0ddfb94 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -448,7 +448,7 @@ RSpec.describe Projects::ArtifactsController do context 'with regular branch' do before do - pipeline.update(ref: 'master', + pipeline.update!(ref: 'master', sha: project.commit('master').sha) get :latest_succeeded, params: params_from_ref('master') @@ -459,7 +459,7 @@ RSpec.describe Projects::ArtifactsController do context 'with branch name containing slash' do before do - pipeline.update(ref: 'improve/awesome', + pipeline.update!(ref: 'improve/awesome', sha: project.commit('improve/awesome').sha) get :latest_succeeded, params: params_from_ref('improve/awesome') @@ -470,7 +470,7 @@ RSpec.describe Projects::ArtifactsController do context 'with branch name and path containing slashes' do before do - pipeline.update(ref: 'improve/awesome', + 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') diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb index cde3a8d4761..48a12a27911 100644 --- a/spec/controllers/projects/boards_controller_spec.rb +++ b/spec/controllers/projects/boards_controller_spec.rb @@ -22,6 +22,15 @@ RSpec.describe Projects::BoardsController do expect(assigns(:boards_endpoint)).to eq project_boards_path(project) end + it 'pushes swimlanes_buffered_rendering feature flag' do + allow(controller).to receive(:push_frontend_feature_flag).and_call_original + + expect(controller).to receive(:push_frontend_feature_flag) + .with(:swimlanes_buffered_rendering, project, default_enabled: :yaml) + + list_boards + end + context 'when format is HTML' do it 'renders template' do list_boards @@ -116,6 +125,15 @@ RSpec.describe Projects::BoardsController do describe 'GET show' do let!(:board) { create(:board, project: project) } + it 'pushes swimlanes_buffered_rendering feature flag' do + allow(controller).to receive(:push_frontend_feature_flag).and_call_original + + expect(controller).to receive(:push_frontend_feature_flag) + .with(:swimlanes_buffered_rendering, project, default_enabled: :yaml) + + read_board board: board + end + it 'sets boards_endpoint instance variable to a boards path' do read_board board: board diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index dd3440f7660..2a8feb09780 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -674,22 +674,32 @@ RSpec.describe Projects::ClustersController do describe 'GET show' do let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - def go + def go(tab: nil) get :show, params: { namespace_id: project.namespace, project_id: project, - id: cluster + id: cluster, + tab: tab } end describe 'functionality' do + render_views + it "renders view" do go expect(response).to have_gitlab_http_status(:ok) expect(assigns(:cluster)).to eq(cluster) end + + it 'renders integration tab view' do + go(tab: 'integrations') + + expect(response).to render_template('clusters/clusters/_integrations') + expect(response).to have_gitlab_http_status(:ok) + end end describe 'security' do diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 2d7f036be21..a231b54419e 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Projects::CommitController do + include ProjectForksHelper + let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } @@ -295,6 +297,102 @@ RSpec.describe Projects::CommitController do expect(flash[:alert]).to match('Sorry, we cannot cherry-pick this commit automatically.') end end + + context 'when a project has a fork' do + let(:project) { create(:project, :repository) } + let(:forked_project) { fork_project(project, user, namespace: user.namespace, repository: true) } + let(:target_project) { project } + 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 + }) + end + + def merge_request_url(source_project, branch) + project_new_merge_request_path( + source_project, + merge_request: { + source_project_id: source_project.id, + target_project_id: project.id, + source_branch: branch, + target_branch: 'feature' + } + ) + end + + before do + forked_project.add_maintainer(user) + end + + it 'successfully cherry picks a commit from fork to upstream project' do + send_request + + expect(response).to redirect_to project_commits_path(project, 'feature') + expect(flash[:notice]).to eq('The commit has been successfully cherry-picked into feature.') + expect(project.commit('feature').message).to include(forked_project.commit.id) + end + + context 'when the cherry pick is performed via merge request' do + let(:create_merge_request) { true } + + it 'successfully cherry picks a commit from fork to a cherry pick branch' do + branch = forked_project.commit.cherry_pick_branch_name + send_request + + expect(response).to redirect_to merge_request_url(project, branch) + expect(flash[:notice]).to start_with("The commit has been successfully cherry-picked into #{branch}") + expect(project.commit(branch).message).to include(forked_project.commit.id) + end + end + + context 'when a user cannot push to upstream project' do + let(:create_merge_request) { true } + + before do + project.add_reporter(user) + end + + it 'cherry picks a commit to the fork' do + branch = forked_project.commit.cherry_pick_branch_name + send_request + + expect(response).to redirect_to merge_request_url(forked_project, branch) + expect(flash[:notice]).to start_with("The commit has been successfully cherry-picked into #{branch}") + expect(project.commit('feature').message).not_to include(forked_project.commit.id) + expect(forked_project.commit(branch).message).to include(forked_project.commit.id) + end + end + + context 'when a user do not have access to the target project' do + let(:target_project) { create(:project, :private) } + + it 'cherry picks a commit to the fork' do + send_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'disable pick_into_project feature flag' do + before do + stub_feature_flags(pick_into_project: false) + end + + it 'does not cherry pick a commit from fork to upstream' do + send_request + + expect(project.commit('feature').message).not_to include(forked_project.commit.id) + end + end + end end describe 'GET diff_for_path' do diff --git a/spec/controllers/projects/cycle_analytics/events_controller_spec.rb b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb index f940da7ea35..6bbdda89b14 100644 --- a/spec/controllers/projects/cycle_analytics/events_controller_spec.rb +++ b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb @@ -26,7 +26,7 @@ RSpec.describe Projects::CycleAnalytics::EventsController do let(:issue) { create(:issue, project: project, created_at: 9.days.ago) } before do - issue.update(milestone: milestone) + issue.update!(milestone: milestone) end it 'is not empty' do diff --git a/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb b/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb index e0f86876f67..c78b838d0df 100644 --- a/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb +++ b/spec/controllers/projects/design_management/designs/raw_images_controller_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Projects::DesignManagement::Designs::RawImagesController do let_it_be(:project) { create(:project, :private) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:viewer) { issue.author } + let(:design_id) { design.id } let(:sha) { design.versions.first.sha } let(:filename) { design.filename } @@ -44,17 +45,6 @@ RSpec.describe Projects::DesignManagement::Designs::RawImagesController do expect(response).to have_gitlab_http_status(:ok) end - context 'when the feature flag attachment_with_filename is disabled' do - it 'serves files with just `attachment` in the disposition header' do - stub_feature_flags(attachment_with_filename: false) - - subject - - expect(response.header['Content-Disposition']).to eq('attachment') - expect(response).to have_gitlab_http_status(:ok) - end - end - it 'serves files with Workhorse' do subject 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 96ecbaf55b6..56c0ef592ca 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 @@ -9,6 +9,7 @@ RSpec.describe Projects::DesignManagement::Designs::ResizedImageController do let_it_be(:issue) { create(:issue, project: project) } let_it_be(:viewer) { issue.author } let_it_be(:size) { :v432x230 } + let(:design) { create(:design, :with_smaller_image_versions, issue: issue, versions_count: 2) } let(:design_id) { design.id } let(:sha) { design.versions.first.sha } diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb index 8a793e29bfa..0c8677ea4b9 100644 --- a/spec/controllers/projects/discussions_controller_spec.rb +++ b/spec/controllers/projects/discussions_controller_spec.rb @@ -85,7 +85,7 @@ RSpec.describe Projects::DiscussionsController do context "when the discussion is not resolvable" do before do - note.update(system: true) + note.update!(system: true) end it "returns status 404" do @@ -168,7 +168,7 @@ RSpec.describe Projects::DiscussionsController do context "when the discussion is not resolvable" do before do - note.update(system: true) + note.update!(system: true) end it "returns status 404" do diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 83ad36b217f..4cb90edb742 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -9,6 +9,7 @@ RSpec.describe Projects::EnvironmentsController do let_it_be(:project) { create(:project) } let_it_be(:maintainer) { create(:user, name: 'main-dos').tap { |u| project.add_maintainer(u) } } let_it_be(:reporter) { create(:user, name: 'repo-dos').tap { |u| project.add_reporter(u) } } + let(:user) { maintainer } let!(:environment) { create(:environment, name: 'production', project: project) } diff --git a/spec/controllers/projects/feature_flags_controller_spec.rb b/spec/controllers/projects/feature_flags_controller_spec.rb index f69cc0ddfd8..cd7d1ea0e8a 100644 --- a/spec/controllers/projects/feature_flags_controller_spec.rb +++ b/spec/controllers/projects/feature_flags_controller_spec.rb @@ -9,6 +9,7 @@ RSpec.describe Projects::FeatureFlagsController do let_it_be(:project) { create(:project) } let_it_be(:developer) { create(:user) } let_it_be(:reporter) { create(:user) } + let(:user) { developer } before_all do diff --git a/spec/controllers/projects/forks_controller_spec.rb b/spec/controllers/projects/forks_controller_spec.rb index 7da3d403b53..8ca3009e0c7 100644 --- a/spec/controllers/projects/forks_controller_spec.rb +++ b/spec/controllers/projects/forks_controller_spec.rb @@ -71,7 +71,7 @@ RSpec.describe Projects::ForksController do context 'when fork is internal' do before do - forked_project.update(visibility_level: Project::INTERNAL, group: group) + forked_project.update!(visibility_level: Project::INTERNAL, group: group) end it 'forks counts are correct' do @@ -86,7 +86,7 @@ RSpec.describe Projects::ForksController do context 'when fork is private' do before do - forked_project.update(visibility_level: Project::PRIVATE, group: group) + forked_project.update!(visibility_level: Project::PRIVATE, group: group) end shared_examples 'forks counts' do @@ -153,8 +153,11 @@ RSpec.describe Projects::ForksController do end describe 'GET new' do - subject do + let(:format) { :html } + + subject(:do_request) do get :new, + format: format, params: { namespace_id: project.namespace, project_id: project @@ -166,24 +169,32 @@ RSpec.describe Projects::ForksController do sign_in(user) end - context 'when JSON requested' do - it 'responds with available groups' do - get :new, - format: :json, - params: { - namespace_id: project.namespace, - project_id: project - } + it 'responds with status 200' do + request - expect(json_response['namespaces'].length).to eq(1) - expect(json_response['namespaces'].first['id']).to eq(group.id) - end + expect(response).to have_gitlab_http_status(:ok) end - it 'responds with status 200' do - subject + context 'when JSON is requested' do + let(:format) { :json } - expect(response).to have_gitlab_http_status(:ok) + it 'responds with user namespace + groups' do + do_request + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['namespaces'].length).to eq(2) + expect(json_response['namespaces'][0]['id']).to eq(user.namespace.id) + expect(json_response['namespaces'][1]['id']).to eq(group.id) + end + + it 'responds with group only when fork_project_form feature flag is disabled' do + stub_feature_flags(fork_project_form: false) + do_request + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['namespaces'].length).to eq(1) + expect(json_response['namespaces'][0]['id']).to eq(group.id) + end end end diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb index 084a807e162..d514c486f60 100644 --- a/spec/controllers/projects/group_links_controller_spec.rb +++ b/spec/controllers/projects/group_links_controller_spec.rb @@ -8,15 +8,16 @@ RSpec.describe Projects::GroupLinksController do let_it_be(:project) { create(:project, :private, group: group2) } let_it_be(:user) { create(:user) } - around do |example| - travel_to DateTime.new(2019, 4, 1) { example.run } - end - before do + travel_to DateTime.new(2019, 4, 1) project.add_maintainer(user) sign_in(user) end + after do + travel_back + end + describe '#create' do shared_context 'link project to group' do before do @@ -31,7 +32,7 @@ RSpec.describe Projects::GroupLinksController do context 'when project is not allowed to be shared with a group' do before do - group.update(share_with_group_lock: false) + group.update!(share_with_group_lock: false) end include_context 'link project to group' diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb index 5e09a50aa36..65a80b9e8ec 100644 --- a/spec/controllers/projects/imports_controller_spec.rb +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -47,7 +47,7 @@ RSpec.describe Projects::ImportsController do context 'when import is in progress' do before do - import_state.update(status: :started) + import_state.update!(status: :started) end it 'renders template' do @@ -65,7 +65,7 @@ RSpec.describe Projects::ImportsController do context 'when import failed' do before do - import_state.update(status: :failed) + import_state.update!(status: :failed) end it 'redirects to new_namespace_project_import_path' do @@ -77,7 +77,7 @@ RSpec.describe Projects::ImportsController do context 'when import finished' do before do - import_state.update(status: :finished) + import_state.update!(status: :finished) end context 'when project is a fork' do @@ -126,7 +126,7 @@ RSpec.describe Projects::ImportsController do context 'when import never happened' do before do - import_state.update(status: :none) + import_state.update!(status: :none) end it 'redirects to namespace_project_path' do diff --git a/spec/controllers/projects/incidents_controller_spec.rb b/spec/controllers/projects/incidents_controller_spec.rb index ddd15b9b1dd..460821634b0 100644 --- a/spec/controllers/projects/incidents_controller_spec.rb +++ b/spec/controllers/projects/incidents_controller_spec.rb @@ -69,6 +69,7 @@ RSpec.describe Projects::IncidentsController do end let_it_be(:resource) { create(:incident, project: project) } + let(:user) { developer } it 'renders incident page' do diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 74062038248..3e016a5e8d2 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Projects::IssuesController do let_it_be(:project, reload: true) { create(:project) } let_it_be(:user, reload: true) { create(:user) } + let(:issue) { create(:issue, project: project) } let(:spam_action_response_fields) { { 'stub_spam_action_response_fields' => true } } @@ -44,7 +45,7 @@ RSpec.describe Projects::IssuesController do let_it_be(:issue) { create(:issue, project: new_project) } before do - project.route.destroy + project.route.destroy! new_project.redirect_routes.create!(path: project.full_path) new_project.add_developer(user) end @@ -63,23 +64,6 @@ RSpec.describe Projects::IssuesController do expect(response).to have_gitlab_http_status(:moved_permanently) end end - - describe 'the null hypothesis experiment', :experiment do - before do - stub_experiments(null_hypothesis: :candidate) - end - - it 'defines the expected before actions' do - expect(controller).to use_before_action(:run_null_hypothesis_experiment) - end - - it 'assigns the candidate experience and tracks the event' do - expect(experiment(:null_hypothesis)).to track('index').on_any_instance.for(:candidate) - .with_context(project: project) - - get :index, params: { namespace_id: project.namespace, project_id: project } - end - end end context 'internal issue tracker' do @@ -209,6 +193,32 @@ RSpec.describe Projects::IssuesController do expect(response).to have_gitlab_http_status(:ok) expect(json_response['issue_email_participants']).to contain_exactly({ "email" => participants[0].email }, { "email" => participants[1].email }) end + + context 'with the invite_members_in_comment experiment', :experiment do + context 'when user can invite' do + before do + stub_experiments(invite_members_in_comment: :invite_member_link) + project.add_maintainer(user) + end + + it 'assigns the candidate experience and tracks the event' do + expect(experiment(:invite_members_in_comment)).to track(:view, property: project.root_ancestor.id.to_s) + .for(:invite_member_link) + .with_context(namespace: project.root_ancestor) + .on_next_instance + + get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + end + end + + context 'when user can not invite' do + it 'does not track the event' do + expect(experiment(:invite_members_in_comment)).not_to track(:view) + + get :show, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } + end + end + end end describe 'GET #new' do @@ -342,6 +352,7 @@ RSpec.describe Projects::IssuesController do end let_it_be(:issue) { create(:issue, project: project) } + let(:developer) { user } let(:params) do { @@ -685,7 +696,7 @@ RSpec.describe Projects::IssuesController do issue.update!(last_edited_by: deleted_user, last_edited_at: Time.current) - deleted_user.destroy + deleted_user.destroy! sign_in(user) end @@ -1038,10 +1049,10 @@ RSpec.describe Projects::IssuesController do labels = create_list(:label, 10, project: project).map(&:to_reference) issue = create(:issue, project: project, description: 'Test issue') - control_count = ActiveRecord::QueryRecorder.new { issue.update(description: [issue.description, label].join(' ')) }.count + control_count = ActiveRecord::QueryRecorder.new { issue.update!(description: [issue.description, label].join(' ')) }.count # Follow-up to get rid of this `2 * label.count` requirement: https://gitlab.com/gitlab-org/gitlab-foss/issues/52230 - expect { issue.update(description: [issue.description, labels].join(' ')) } + expect { issue.update!(description: [issue.description, labels].join(' ')) } .not_to exceed_query_limit(control_count + 2 * labels.count) end @@ -1158,6 +1169,7 @@ RSpec.describe Projects::IssuesController do context 'resolving discussions in MergeRequest' do let_it_be(:discussion) { create(:diff_note_on_merge_request).to_discussion } + let(:merge_request) { discussion.noteable } let(:project) { merge_request.source_project } @@ -1420,9 +1432,7 @@ RSpec.describe Projects::IssuesController do expect_next_instance_of(Spam::AkismetService) do |akismet_service| expect(akismet_service).to receive_messages(submit_spam: true) end - expect_next_instance_of(ApplicationSetting) do |setting| - expect(setting).to receive_messages(akismet_enabled: true) - end + stub_application_setting(akismet_enabled: true) end def post_spam @@ -1490,12 +1500,6 @@ RSpec.describe Projects::IssuesController do expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(json_response).to eq({ 'errors' => 'Destroy confirmation not provided for issue' }) end - - it 'delegates the update of the todos count cache to TodoService' do - expect_any_instance_of(TodoService).to receive(:destroy_target).with(issue).once - - delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: issue.iid, destroy_confirm: true } - end end end @@ -1623,6 +1627,7 @@ RSpec.describe Projects::IssuesController do describe 'POST #import_csv' do let_it_be(:project) { create(:project, :public) } + let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') } context 'unauthorized' do @@ -1822,6 +1827,7 @@ RSpec.describe Projects::IssuesController do context 'with cross-reference system note', :request_store do let_it_be(:new_issue) { create(:issue) } + let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" } before do @@ -1899,7 +1905,7 @@ RSpec.describe Projects::IssuesController do before do sign_in(user) - project.route.destroy + project.route.destroy! new_project.redirect_routes.create!(path: project.full_path) new_project.add_developer(user) end diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 80e1268cb01..a7a36d3a074 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -1275,6 +1275,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do let_it_be(:reporter) { create(:user) } let_it_be(:guest) { create(:user) } let_it_be(:project) { create(:project, :private, :repository, namespace: owner.namespace) } + let(:user) { maintainer } let(:pipeline) { create(:ci_pipeline, project: project, source: :webide, config_source: :webide_source, user: user) } let(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline, user: user) } diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index f452c22a5ca..081927ea73c 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -65,7 +65,7 @@ RSpec.describe Projects::LabelsController do end it 'does not include group labels when project does not belong to a group' do - project.update(namespace: create(:namespace)) + project.update!(namespace: create(:namespace)) list_labels @@ -93,6 +93,26 @@ RSpec.describe Projects::LabelsController do end end + context 'with views rendered' do + render_views + + before do + list_labels + end + + it 'avoids N+1 queries' do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { list_labels } + + create_list(:label, 3, project: project) + create_list(:group_label, 3, group: group) + + # some n+1 queries still exist + # calls to get max project authorization access level + expect { list_labels }.not_to exceed_all_query_limit(control.count).with_threshold(25) + expect(assigns(:labels).count).to eq(10) + end + end + def list_labels get :index, params: { namespace_id: project.namespace.to_param, project_id: project } end @@ -221,7 +241,7 @@ RSpec.describe Projects::LabelsController do end context 'when requesting a redirected path' do - let_it_be(:redirect_route) { project.redirect_routes.create(path: project.full_path + 'old') } + let_it_be(:redirect_route) { project.redirect_routes.create!(path: project.full_path + 'old') } it 'redirects to the canonical path' do get :index, params: { namespace_id: project.namespace, project_id: project.to_param + 'old' } @@ -267,7 +287,7 @@ RSpec.describe Projects::LabelsController do end context 'when requesting a redirected path' do - let_it_be(:redirect_route) { project.redirect_routes.create(path: project.full_path + 'old') } + let_it_be(:redirect_route) { project.redirect_routes.create!(path: project.full_path + 'old') } it 'returns not found' do post :generate, params: { namespace_id: project.namespace, project_id: project.to_param + 'old' } diff --git a/spec/controllers/projects/merge_requests/content_controller_spec.rb b/spec/controllers/projects/merge_requests/content_controller_spec.rb index 67d3ef6f4f0..0eaa528a330 100644 --- a/spec/controllers/projects/merge_requests/content_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/content_controller_spec.rb @@ -11,13 +11,13 @@ RSpec.describe Projects::MergeRequests::ContentController do sign_in(user) end - def do_request(action = :cached_widget) + def do_request(action = :cached_widget, params = {}) get action, params: { namespace_id: project.namespace.to_param, project_id: project, id: merge_request.iid, format: :json - } + }.merge(params) end context 'user has access to the project' do @@ -42,6 +42,10 @@ RSpec.describe Projects::MergeRequests::ContentController do end describe 'GET widget' do + before do + merge_request.mark_as_unchecked! + end + it 'checks whether the MR can be merged' do controller.instance_variable_set(:@merge_request, merge_request) @@ -53,6 +57,17 @@ RSpec.describe Projects::MergeRequests::ContentController do expect(response.headers['Poll-Interval']).to eq('10000') end + context 'when async_mergeability_check param is passed' do + it 'checks mergeability asynchronously' do + expect_next_instance_of(MergeRequests::MergeabilityCheckService) do |service| + expect(service).not_to receive(:execute) + expect(service).to receive(:async_execute).and_call_original + end + + do_request(:widget, { async_mergeability_check: true }) + end + end + context 'merged merge request' do let(:merge_request) do create(:merged_merge_request, :with_test_reports, target_project: project, source_project: project) diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index 091a44130a1..df2023b7356 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -213,6 +213,38 @@ RSpec.describe Projects::MergeRequests::CreationsController do expect(assigns(:commit)).to be_nil expect(response).to have_gitlab_http_status(:ok) end + + context 'no target_project_id provided' do + before do + project.add_maintainer(user) + end + + it 'selects itself as a target project' do + get :branch_to, + params: { + namespace_id: project.namespace, + project_id: project, + ref: 'master' + } + + expect(assigns(:target_project)).to eq(project) + expect(response).to have_gitlab_http_status(:ok) + end + + context 'project is a fork' do + it 'calls to project defaults to selects a correct target project' do + get :branch_to, + params: { + namespace_id: fork_project.namespace, + project_id: fork_project, + ref: 'master' + } + + expect(assigns(:target_project)).to eq(project) + expect(response).to have_gitlab_http_status(:ok) + end + end + end end describe 'POST create' do diff --git a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb index af39d4dec72..580211893dc 100644 --- a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb @@ -297,7 +297,7 @@ RSpec.describe Projects::MergeRequests::DraftsController do expect { post :publish, params: params }.to change { Note.count }.by(0).and change { DraftNote.count }.by(0) end - it 'publishes a draft note with quick actions and applies them' 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}") diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 93d5e7eff6c..337a4a19b2e 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Projects::MergeRequestsController do let_it_be_with_refind(:project) { create(:project, :repository) } let_it_be_with_reload(:project_public_with_private_builds) { create(:project, :repository, :public, :builds_private) } + let(:user) { project.owner } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } @@ -40,6 +41,32 @@ RSpec.describe Projects::MergeRequestsController do get :show, params: params.merge(extra_params) end + context 'with the invite_members_in_comment experiment', :experiment do + context 'when user can invite' do + before do + stub_experiments(invite_members_in_comment: :invite_member_link) + project.add_maintainer(user) + end + + it 'assigns the candidate experience and tracks the event' do + expect(experiment(:invite_members_in_comment)).to track(:view, property: project.root_ancestor.id.to_s) + .for(:invite_member_link) + .with_context(namespace: project.root_ancestor) + .on_next_instance + + go + end + end + + context 'when user can not invite' do + it 'does not track the event' do + expect(experiment(:invite_members_in_comment)).not_to track(:view) + + go + end + end + end + context 'with view param' do before do go(view: 'parallel') @@ -55,13 +82,19 @@ RSpec.describe Projects::MergeRequestsController do merge_request.mark_as_unchecked! end - it 'checks mergeability asynchronously' do - expect_next_instance_of(MergeRequests::MergeabilityCheckService) do |service| - expect(service).not_to receive(:execute) - expect(service).to receive(:async_execute) + context 'check_mergeability_async_in_widget feature flag is disabled' do + before do + stub_feature_flags(check_mergeability_async_in_widget: false) end - go + it 'checks mergeability asynchronously' do + expect_next_instance_of(MergeRequests::MergeabilityCheckService) do |service| + expect(service).not_to receive(:execute) + expect(service).to receive(:async_execute) + end + + go + end end end @@ -695,12 +728,6 @@ RSpec.describe Projects::MergeRequestsController do expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(json_response).to eq({ 'errors' => 'Destroy confirmation not provided for merge request' }) end - - it 'delegates the update of the todos count cache to TodoService' do - expect_any_instance_of(TodoService).to receive(:destroy_target).with(merge_request).once - - delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: merge_request.iid, destroy_confirm: true } - end end end diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index b93f1b41a7e..b62353784b3 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -105,7 +105,7 @@ RSpec.describe Projects::MilestonesController do context 'with a single group ancestor' do before do - project.update(namespace: group) + project.update!(namespace: group) get :index, params: { namespace_id: project.namespace.id, project_id: project.id }, format: :json end @@ -122,7 +122,7 @@ RSpec.describe Projects::MilestonesController do let!(:subgroup_milestone) { create(:milestone, group: subgroup) } before do - project.update(namespace: subgroup) + project.update!(namespace: subgroup) get :index, params: { namespace_id: project.namespace.id, project_id: project.id }, format: :json end @@ -158,7 +158,7 @@ RSpec.describe Projects::MilestonesController do let(:group) { create(:group) } before do - project.update(namespace: group) + project.update!(namespace: group) end context 'when user does not have permission to promote milestone' do @@ -234,7 +234,7 @@ RSpec.describe Projects::MilestonesController do end it 'renders 404' do - project.update(namespace: user.namespace) + project.update!(namespace: user.namespace) post :promote, params: { namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid } @@ -253,7 +253,7 @@ RSpec.describe Projects::MilestonesController do before do project.add_guest(guest_user) sign_in(guest_user) - issue.update(assignee_ids: issue_assignee.id) + issue.update!(assignee_ids: issue_assignee.id) end context "when issue is not confidential" do @@ -269,7 +269,7 @@ RSpec.describe Projects::MilestonesController do context "when issue is confidential" do before do - issue.update(confidential: true) + issue.update!(confidential: true) end it 'shows no milestone participants' do diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index add249e2c74..d92862f0ca3 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -334,7 +334,7 @@ RSpec.describe Projects::NotesController do before do project.update_attribute(:visibility_level, project_visibility) - project.project_feature.update(merge_requests_access_level: merge_requests_access_level) + project.project_feature.update!(merge_requests_access_level: merge_requests_access_level) sign_in(user) end @@ -917,7 +917,7 @@ RSpec.describe Projects::NotesController do context "when the note is not resolvable" do before do - note.update(system: true) + note.update!(system: true) end it "returns status 404" do @@ -980,7 +980,7 @@ RSpec.describe Projects::NotesController do context "when the note is not resolvable" do before do - note.update(system: true) + note.update!(system: true) end it "returns status 404" do diff --git a/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb b/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb index 8a344a72120..923581d9367 100644 --- a/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb +++ b/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Projects::PerformanceMonitoring::DashboardsController do let_it_be(:user) { create(:user) } let_it_be(:namespace) { create(:namespace) } + let!(:project) { create(:project, :repository, name: 'dashboard-project', namespace: namespace) } let(:repository) { project.repository } let(:branch) { double(name: branch_name) } diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index e1405660ccb..753223c5a4f 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -7,13 +7,14 @@ RSpec.describe Projects::PipelinesController do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :public, :repository) } + let(:feature) { ProjectFeature::ENABLED } before do allow(Sidekiq.logger).to receive(:info) stub_not_protect_default_branch project.add_developer(user) - project.project_feature.update(builds_access_level: feature) + project.project_feature.update!(builds_access_level: feature) sign_in(user) end @@ -272,6 +273,23 @@ RSpec.describe Projects::PipelinesController do end end + describe 'GET #index' do + context 'pipeline_empty_state_templates experiment' do + before do + stub_application_setting(auto_devops_enabled: false) + end + + it 'tracks the view', :experiment do + expect(experiment(:pipeline_empty_state_templates)) + .to track(:view, value: project.namespace_id) + .with_context(actor: user) + .on_next_instance + + get :index, params: { namespace_id: project.namespace, project_id: project } + end + end + end + describe 'GET show.json' do let(:pipeline) { create(:ci_pipeline, project: project) } @@ -628,44 +646,6 @@ RSpec.describe Projects::PipelinesController do end end - describe 'GET stages_ajax.json' do - let(:pipeline) { create(:ci_pipeline, project: project) } - - context 'when accessing existing stage' do - before do - create(:ci_build, pipeline: pipeline, stage: 'build') - - get_stage_ajax('build') - end - - it 'returns html source for stage dropdown' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template('projects/pipelines/_stage') - expect(json_response).to include('html') - end - end - - context 'when accessing unknown stage' do - before do - get_stage_ajax('test') - end - - it 'responds with not found' do - expect(response).to have_gitlab_http_status(:not_found) - end - end - - def get_stage_ajax(name) - get :stage_ajax, params: { - namespace_id: project.namespace, - project_id: project, - id: pipeline.id, - stage: name - }, - format: :json - end - end - describe 'GET status.json' do let(:pipeline) { create(:ci_pipeline, project: project) } let(:status) { pipeline.detailed_status(double('user')) } @@ -702,7 +682,7 @@ RSpec.describe Projects::PipelinesController do before do project.add_developer(user) - project.project_feature.update(builds_access_level: feature) + project.project_feature.update!(builds_access_level: feature) end context 'with a valid .gitlab-ci.yml file' do @@ -721,7 +701,7 @@ RSpec.describe Projects::PipelinesController do pipeline = project.ci_pipelines.last expected_redirect_path = Gitlab::Routing.url_helpers.project_pipeline_path(project, pipeline) - expect(pipeline).to be_pending + expect(pipeline).to be_created expect(response).to redirect_to(expected_redirect_path) end end @@ -777,7 +757,7 @@ RSpec.describe Projects::PipelinesController do before do project.add_developer(user) - project.project_feature.update(builds_access_level: feature) + project.project_feature.update!(builds_access_level: feature) end context 'with a valid .gitlab-ci.yml file' do diff --git a/spec/controllers/projects/pipelines_settings_controller_spec.rb b/spec/controllers/projects/pipelines_settings_controller_spec.rb index ad631b7c3da..39fb153e802 100644 --- a/spec/controllers/projects/pipelines_settings_controller_spec.rb +++ b/spec/controllers/projects/pipelines_settings_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Projects::PipelinesSettingsController do let_it_be(:user) { create(:user) } let_it_be(:project_auto_devops) { create(:project_auto_devops) } + let(:project) { project_auto_devops.project } before do diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 53a7c2ca069..46a0fc8edb0 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -7,8 +7,12 @@ RSpec.describe Projects::ProjectMembersController do let(:group) { create(:group, :public) } let(:project) { create(:project, :public) } - around do |example| - travel_to DateTime.new(2019, 4, 1) { example.run } + before do + travel_to DateTime.new(2019, 4, 1) + end + + after do + travel_back end describe 'GET index' do diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index b1c3c1c0276..5dee36ee7c2 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Projects::RawController do include RepoHelpers let_it_be(:project) { create(:project, :public, :repository) } + let(:inline) { nil } describe 'GET #show' do diff --git a/spec/controllers/projects/registry/repositories_controller_spec.rb b/spec/controllers/projects/registry/repositories_controller_spec.rb index 9b803edd463..0685e5a2055 100644 --- a/spec/controllers/projects/registry/repositories_controller_spec.rb +++ b/spec/controllers/projects/registry/repositories_controller_spec.rb @@ -16,19 +16,19 @@ RSpec.describe Projects::Registry::RepositoriesController do project.add_developer(user) end - shared_examples 'with name parameter' do - let_it_be(:repo) { create(:container_repository, project: project, name: 'my_searched_image') } - let_it_be(:another_repo) { create(:container_repository, project: project, name: 'bar') } - - it 'returns the searched repo' do - go_to_index(format: :json, params: { name: 'my_searched_image' }) + shared_examples 'renders 200 for html and 404 for json' do + it 'successfully renders container repositories', :snowplow do + go_to_index expect(response).to have_gitlab_http_status(:ok) - expect(json_response.length).to eq 1 - expect(json_response.first).to include( - 'id' => repo.id, - 'name' => repo.name - ) + # event tracked in GraphQL API: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44926 + expect_no_snowplow_event + end + + it 'returns 404 for request in json format' do + go_to_index(format: :json) + + expect(response).to have_gitlab_http_status(:not_found) end end @@ -50,33 +50,12 @@ RSpec.describe Projects::Registry::RepositoriesController do tags: %w[rc1 latest]) end - it 'successfully renders container repositories', :snowplow do - go_to_index - - expect_no_snowplow_event - expect(response).to have_gitlab_http_status(:ok) - end - - it 'tracks the event', :snowplow do - go_to_index(format: :json) - - expect_snowplow_event(category: anything, action: 'list_repositories') - end - it 'creates a root container repository' do expect { go_to_index }.to change { ContainerRepository.all.count }.by(1) expect(ContainerRepository.first).to be_root_repository end - it 'json has a list of projects' do - go_to_index(format: :json) - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('registry/repositories') - expect(response).to include_pagination_headers - end - - it_behaves_like 'with name parameter' + it_behaves_like 'renders 200 for html and 404 for json' end context 'when there are no tags for this repository' do @@ -84,22 +63,11 @@ RSpec.describe Projects::Registry::RepositoriesController do stub_container_registry_tags(repository: :any, tags: []) end - it 'successfully renders container repositories' do - go_to_index - - expect(response).to have_gitlab_http_status(:ok) - end - it 'does not ensure root container repository' do expect { go_to_index }.not_to change { ContainerRepository.all.count } end - it 'responds with json if asked' do - go_to_index(format: :json) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_kind_of(Array) - end + it_behaves_like 'renders 200 for html and 404 for json' end end end diff --git a/spec/controllers/projects/registry/tags_controller_spec.rb b/spec/controllers/projects/registry/tags_controller_spec.rb index 5bff89b4308..c03a280d2cd 100644 --- a/spec/controllers/projects/registry/tags_controller_spec.rb +++ b/spec/controllers/projects/registry/tags_controller_spec.rb @@ -42,7 +42,7 @@ RSpec.describe Projects::Registry::TagsController do it 'tracks the event', :snowplow do get_tags - expect_snowplow_event(category: anything, action: 'list_tags') + expect_snowplow_event(category: 'Projects::Registry::TagsController', action: 'list_tags') end end @@ -107,11 +107,12 @@ RSpec.describe Projects::Registry::TagsController do destroy_tag('test.') end - it 'tracks the event' do + it 'tracks the event', :snowplow do expect_delete_tags(%w[test.]) - expect(controller).to receive(:track_event).with(:delete_tag) destroy_tag('test.') + + expect_snowplow_event(category: 'Projects::Registry::TagsController', action: 'delete_tag') end end end diff --git a/spec/controllers/projects/releases/evidences_controller_spec.rb b/spec/controllers/projects/releases/evidences_controller_spec.rb index 0ec4cdf2a31..68433969d69 100644 --- a/spec/controllers/projects/releases/evidences_controller_spec.rb +++ b/spec/controllers/projects/releases/evidences_controller_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Projects::Releases::EvidencesController do let_it_be(:private_project) { create(:project, :repository, :private) } let_it_be(:developer) { create(:user) } let_it_be(:reporter) { create(:user) } + let(:user) { developer } before do @@ -62,7 +63,7 @@ RSpec.describe Projects::Releases::EvidencesController do context 'when the release was created before evidence existed' do before do - evidence.destroy + evidence.destroy! end it_behaves_like 'not found' diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb index fc7ab88bbe0..a1e36ec5c4c 100644 --- a/spec/controllers/projects/releases_controller_spec.rb +++ b/spec/controllers/projects/releases_controller_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Projects::ReleasesController do let_it_be(:reporter) { create(:user) } let_it_be(:guest) { create(:user) } let_it_be(:user) { developer } + let!(:release_1) { create(:release, project: project, released_at: Time.zone.parse('2018-10-18')) } let!(:release_2) { create(:release, project: project, released_at: Time.zone.parse('2019-10-19')) } diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index e6327a72a68..cb2579b800a 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -56,28 +56,6 @@ RSpec.describe Projects::RepositoriesController do expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") end - it 'handles legacy queries with no ref' do - get :archive, params: { namespace_id: project.namespace, project_id: project }, format: "zip" - - expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") - end - - it 'handles legacy queries with the ref specified as ref in params' do - get :archive, params: { namespace_id: project.namespace, project_id: project, ref: 'feature' }, format: 'zip' - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:ref)).to eq('feature') - expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") - end - - it 'handles legacy queries with the ref specified as id in params' do - get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'feature' }, format: 'zip' - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:ref)).to eq('feature') - expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") - end - it 'prioritizes the id param over the ref param when both are specified' do get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'feature', ref: 'feature_conflict' }, format: 'zip' diff --git a/spec/controllers/projects/runners_controller_spec.rb b/spec/controllers/projects/runners_controller_spec.rb index d63d88f8283..3021ad42c9f 100644 --- a/spec/controllers/projects/runners_controller_spec.rb +++ b/spec/controllers/projects/runners_controller_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Projects::RunnersController do describe '#resume' do it 'marks the runner as active and ticks the queue' do - runner.update(active: false) + runner.update!(active: false) expect do post :resume, params: params @@ -61,7 +61,7 @@ RSpec.describe Projects::RunnersController do describe '#pause' do it 'marks the runner as inactive and ticks the queue' do - runner.update(active: true) + runner.update!(active: true) expect do post :pause, params: params diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 8f928cf3382..488a34b74df 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Projects::ServicesController do include JiraServiceHelper + include AfterNextHelpers let(:project) { create(:project, :repository) } let(:user) { create(:user) } @@ -13,7 +14,6 @@ RSpec.describe Projects::ServicesController do before do sign_in(user) project.add_maintainer(user) - allow(Gitlab::UrlBlocker).to receive(:validate!).and_return([URI.parse('http://example.com'), nil]) end describe '#test' do @@ -114,7 +114,7 @@ RSpec.describe Projects::ServicesController do end context 'failure' do - it 'returns success status code and the error message' do + it 'returns an error response when the integration test fails' do stub_request(:get, 'http://example.com/rest/api/2/serverInfo') .to_return(status: 404) @@ -128,6 +128,36 @@ RSpec.describe Projects::ServicesController do 'test_failed' => true ) end + + context 'with the Slack integration' do + let_it_be(:service) { build(:slack_service) } + + it 'returns an error response when the URL is blocked' do + put :test, params: project_params(service: { webhook: 'http://127.0.0.1' }) + + expect(response).to be_successful + expect(json_response).to eq( + 'error' => true, + 'message' => 'Connection failed. Please check your settings.', + 'service_response' => "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed", + 'test_failed' => true + ) + end + + it 'returns an error response when a network exception is raised' do + expect_next(SlackService).to receive(:test).and_raise(Errno::ECONNREFUSED) + + put :test, params: project_params + + expect(response).to be_successful + expect(json_response).to eq( + 'error' => true, + 'message' => 'Connection failed. Please check your settings.', + 'service_response' => 'Connection refused', + 'test_failed' => true + ) + end + end end end diff --git a/spec/controllers/projects/settings/access_tokens_controller_spec.rb b/spec/controllers/projects/settings/access_tokens_controller_spec.rb index ff52b2a765a..2a7e3d0b322 100644 --- a/spec/controllers/projects/settings/access_tokens_controller_spec.rb +++ b/spec/controllers/projects/settings/access_tokens_controller_spec.rb @@ -4,7 +4,8 @@ require('spec_helper') RSpec.describe Projects::Settings::AccessTokensController do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } let_it_be(:bot_user) { create(:user, :project_bot) } before_all do @@ -40,6 +41,26 @@ RSpec.describe Projects::Settings::AccessTokensController do it_behaves_like 'feature unavailable' it_behaves_like 'project access tokens available #create' + + context 'when project access token creation is disabled' do + before do + group.namespace_settings.update_column(:resource_access_token_creation_allowed, false) + end + + it { is_expected.to have_gitlab_http_status(:not_found) } + + it 'does not create the token' do + expect { subject }.not_to change { PersonalAccessToken.count } + end + + it 'does not add the project bot as a member' do + expect { subject }.not_to change { Member.count } + end + + it 'does not create the project bot user' do + expect { subject }.not_to change { User.count } + end + end end describe '#revoke', :sidekiq_inline do diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index 7a6e11d53d4..d953249c139 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -5,6 +5,7 @@ require('spec_helper') RSpec.describe Projects::Settings::CiCdController do let_it_be(:user) { create(:user) } let_it_be(:project_auto_devops) { create(:project_auto_devops) } + let(:project) { project_auto_devops.project } before do diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 46f69eaf96a..d2934ec4e97 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -493,6 +493,7 @@ RSpec.describe Projects::Settings::OperationsController do describe 'PATCH #update' do let_it_be(:external_url) { 'https://gitlab.com' } + let(:params) do { tracing_setting_attributes: { diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index 793ffbbfad9..1a6c0974f08 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -46,6 +46,10 @@ RSpec.describe Projects::SnippetsController do let(:params) { base_params } end + it_behaves_like 'snippets views' do + let(:params) { base_params } + end + context 'when the project snippet is private' do let_it_be(:project_snippet) { create(:project_snippet, :private, project: project, author: user) } diff --git a/spec/controllers/projects/starrers_controller_spec.rb b/spec/controllers/projects/starrers_controller_spec.rb index 66888fa3024..8d03600cd58 100644 --- a/spec/controllers/projects/starrers_controller_spec.rb +++ b/spec/controllers/projects/starrers_controller_spec.rb @@ -170,7 +170,7 @@ RSpec.describe Projects::StarrersController do context 'when project is private' do before do - project.update(visibility_level: Project::PRIVATE) + project.update!(visibility_level: Project::PRIVATE) end it 'starrers are not visible for non logged in users' do diff --git a/spec/controllers/projects/static_site_editor_controller_spec.rb b/spec/controllers/projects/static_site_editor_controller_spec.rb index b563f3b667f..73b0e3bba69 100644 --- a/spec/controllers/projects/static_site_editor_controller_spec.rb +++ b/spec/controllers/projects/static_site_editor_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Projects::StaticSiteEditorController do let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:user) { create(:user) } + let(:data) { { key: 'value' } } describe 'GET index' do diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb index 0e35f401bc8..9a73417ffdb 100644 --- a/spec/controllers/projects/todos_controller_spec.rb +++ b/spec/controllers/projects/todos_controller_spec.rb @@ -5,6 +5,7 @@ require('spec_helper') RSpec.describe Projects::TodosController do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } + let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, source_project: project) } let(:design) { create(:design, project: project, issue: issue) } diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb index dda58f06a37..c008c7253d8 100644 --- a/spec/controllers/projects/uploads_controller_spec.rb +++ b/spec/controllers/projects/uploads_controller_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Projects::UploadsController do let!(:upload) { create(:upload, :issuable_upload, :with_file, model: model) } let(:project) { model } let(:upload_path) { File.basename(upload.path) } - let!(:redirect_route) { project.redirect_routes.create(path: project.full_path + 'old') } + let!(:redirect_route) { project.redirect_routes.create!(path: project.full_path + 'old') } it 'redirects to a file with the proper extension' do get :show, params: { namespace_id: project.namespace, project_id: project.to_param + 'old', filename: File.basename(upload.path), secret: upload.secret } diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 554487db8f2..ffe2d393b1e 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -10,6 +10,7 @@ RSpec.describe ProjectsController do let_it_be(:project, reload: true) { create(:project, service_desk_enabled: false) } let_it_be(:public_project) { create(:project, :public) } let_it_be(:user) { create(:user) } + let(:jpg) { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') } let(:txt) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') } @@ -159,7 +160,7 @@ RSpec.describe ProjectsController do before do setting = user.notification_settings_for(public_project) setting.level = :watch - setting.save + setting.save! end it "shows current notification setting" do @@ -221,24 +222,23 @@ RSpec.describe ProjectsController do allow(controller).to receive(:record_experiment_user) end - context 'when user can push to default branch' do + context 'when user can push to default branch', :experiment do let(:user) { empty_project.owner } - it 'creates an "view_project_show" experiment tracking event', :snowplow do - allow_next_instance_of(ApplicationExperiment) do |e| - allow(e).to receive(:should_track?).and_return(true) - end + it 'creates an "view_project_show" experiment tracking event' do + expect(experiment(:empty_repo_upload)).to track( + :view_project_show, + property: 'empty' + ).on_next_instance get :show, params: { namespace_id: empty_project.namespace, id: empty_project } - - expect_snowplow_event(category: 'empty_repo_upload', action: 'view_project_show', context: [{ schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/0-3-0', data: anything }], property: 'empty') end end User.project_views.keys.each do |project_view| context "with #{project_view} view set" do before do - user.update(project_view: project_view) + user.update!(project_view: project_view) get :show, params: { namespace_id: empty_project.namespace, id: empty_project } end @@ -261,7 +261,7 @@ RSpec.describe ProjectsController do User.project_views.keys.each do |project_view| context "with #{project_view} view set" do before do - user.update(project_view: project_view) + user.update!(project_view: project_view) get :show, params: { namespace_id: empty_project.namespace, id: empty_project } end @@ -444,7 +444,13 @@ RSpec.describe ProjectsController do :created, property: 'blank', value: 1 - ).on_any_instance.with_context(actor: user) + ).with_context(actor: user).on_next_instance + + post :create, params: { project: project_params } + end + + it 'tracks a created event for the new_repo experiment', :experiment do + expect(experiment(:new_repo, :candidate)).to track(:project_created).on_next_instance post :create, params: { project: project_params } end @@ -549,6 +555,7 @@ RSpec.describe ProjectsController do describe '#housekeeping' do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } + let(:housekeeping) { Repositories::HousekeepingService.new(project) } context 'when authenticated as owner' do @@ -1098,6 +1105,7 @@ RSpec.describe ProjectsController do context 'state filter on references' do let_it_be(:issue) { create(:issue, :closed, project: public_project) } + let(:merge_request) { create(:merge_request, :closed, target_project: public_project) } it 'renders JSON body with state filter for issues' do diff --git a/spec/controllers/registrations/welcome_controller_spec.rb b/spec/controllers/registrations/welcome_controller_spec.rb index d32c936b8c9..008259a8bfa 100644 --- a/spec/controllers/registrations/welcome_controller_spec.rb +++ b/spec/controllers/registrations/welcome_controller_spec.rb @@ -60,8 +60,10 @@ RSpec.describe Registrations::WelcomeController do end describe '#update' do + let(:email_opted_in) { '0' } + subject(:update) do - patch :update, params: { user: { role: 'software_developer', setup_for_company: 'false' } } + patch :update, params: { user: { role: 'software_developer', setup_for_company: 'false', email_opted_in: email_opted_in } } end context 'without a signed in user' do @@ -74,6 +76,24 @@ RSpec.describe Registrations::WelcomeController do end it { is_expected.to redirect_to(dashboard_projects_path)} + + context 'when the user opted in' do + let(:email_opted_in) { '1' } + + it 'sets the email_opted_in field' do + subject + + expect(controller.current_user.email_opted_in).to eq(true) + end + end + + context 'when the user opted out' do + it 'sets the email_opted_in field' do + subject + + expect(controller.current_user.email_opted_in).to eq(false) + end + end end end end diff --git a/spec/controllers/root_controller_spec.rb b/spec/controllers/root_controller_spec.rb index 49841aa61d7..01ff646274a 100644 --- a/spec/controllers/root_controller_spec.rb +++ b/spec/controllers/root_controller_spec.rb @@ -134,26 +134,6 @@ RSpec.describe RootController do expect(response).to render_template 'dashboard/projects/index' end - - context 'when customize_homepage is enabled' do - it 'renders the default dashboard' do - get :index - - expect(assigns[:customize_homepage]).to be true - end - end - - context 'when customize_homepage is not enabled' do - before do - stub_feature_flags(customize_homepage: false) - end - - it 'renders the default dashboard' do - get :index - - expect(assigns[:customize_homepage]).to be false - end - end end end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index c31ba6fe156..abdafa2880a 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -86,7 +86,7 @@ RSpec.describe SessionsController do post(:create, params: { user: { login: 'invalid', password: 'invalid' } }) expect(response) - .to set_flash.now[:alert].to(/Invalid Login or password/) + .to set_flash.now[:alert].to(/Invalid login or password/) end end @@ -348,7 +348,7 @@ RSpec.describe SessionsController do otp_user_id: user.id ) - expect(response).to set_flash.now[:alert].to(/Invalid Login or password/) + expect(response).to set_flash.now[:alert].to(/Invalid login or password/) end end @@ -524,7 +524,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(Labkit::Context.current.to_h) + expect(Gitlab::ApplicationContext.current) .to include('meta.user' => user.username, 'meta.caller_id' => 'SessionsController#destroy') @@ -538,9 +538,9 @@ RSpec.describe SessionsController do context 'when not signed in' do it 'sets the caller_id in the context' do expect(controller).to receive(:new).and_wrap_original do |m, *args| - expect(Labkit::Context.current.to_h) + expect(Gitlab::ApplicationContext.current) .to include('meta.caller_id' => 'SessionsController#new') - expect(Labkit::Context.current.to_h) + expect(Gitlab::ApplicationContext.current) .not_to include('meta.user') m.call(*args) @@ -557,9 +557,9 @@ RSpec.describe SessionsController do it 'sets the caller_id in the context' do allow_any_instance_of(User).to receive(:lock_access!).and_wrap_original do |m, *args| - expect(Labkit::Context.current.to_h) + expect(Gitlab::ApplicationContext.current) .to include('meta.caller_id' => 'SessionsController#create') - expect(Labkit::Context.current.to_h) + expect(Gitlab::ApplicationContext.current) .not_to include('meta.user') m.call(*args) -- cgit v1.2.1