From 859a6fb938bb9ee2a317c46dfa4fcc1af49608f0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 18 Feb 2021 10:34:06 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-9-stable-ee --- .../admin/application_settings_controller_spec.rb | 7 + spec/controllers/admin/cohorts_controller_spec.rb | 34 +- spec/controllers/admin/runners_controller_spec.rb | 3 +- spec/controllers/admin/users_controller_spec.rb | 7 +- spec/controllers/chaos_controller_spec.rb | 19 + spec/controllers/concerns/redis_tracking_spec.rb | 115 +++--- .../controllers/concerns/spammable_actions_spec.rb | 99 +----- spec/controllers/graphql_controller_spec.rb | 20 ++ .../groups/group_members_controller_spec.rb | 12 + spec/controllers/groups_controller_spec.rb | 60 ---- spec/controllers/help_controller_spec.rb | 110 ++++-- .../import/bulk_imports_controller_spec.rb | 24 +- spec/controllers/invites_controller_spec.rb | 43 ++- .../profiles/notifications_controller_spec.rb | 3 +- .../profiles/preferences_controller_spec.rb | 35 +- spec/controllers/projects/blob_controller_spec.rb | 4 +- ...y_build_group_report_results_controller_spec.rb | 43 ++- .../projects/discussions_controller_spec.rb | 7 + spec/controllers/projects/forks_controller_spec.rb | 22 ++ .../controllers/projects/issues_controller_spec.rb | 57 +-- spec/controllers/projects/jobs_controller_spec.rb | 48 --- .../projects/learn_gitlab_controller_spec.rb | 44 +++ .../merge_requests/diffs_controller_spec.rb | 8 +- .../projects/merge_requests_controller_spec.rb | 105 +++++- spec/controllers/projects/notes_controller_spec.rb | 87 ++++- .../projects/pipelines/tests_controller_spec.rb | 24 +- .../projects/pipelines_controller_spec.rb | 66 ---- .../projects/project_members_controller_spec.rb | 12 + .../security/configuration_controller_spec.rb | 55 +++ .../projects/templates_controller_spec.rb | 3 +- spec/controllers/projects_controller_spec.rb | 61 +++- .../experience_levels_controller_spec.rb | 127 +++---- spec/controllers/registrations_controller_spec.rb | 12 + .../repositories/git_http_controller_spec.rb | 33 +- spec/controllers/search_controller_spec.rb | 392 +++++++++++---------- spec/controllers/snippets_controller_spec.rb | 2 +- 36 files changed, 1026 insertions(+), 777 deletions(-) create mode 100644 spec/controllers/projects/learn_gitlab_controller_spec.rb create mode 100644 spec/controllers/projects/security/configuration_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 f0b224484c6..71abf3191b8 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -150,6 +150,13 @@ RSpec.describe Admin::ApplicationSettingsController do expect(ApplicationSetting.current.repository_storages_weighted_default).to eq(75) end + it 'updates kroki_formats setting' do + put :update, params: { application_setting: { kroki_formats_excalidraw: '1' } } + + expect(response).to redirect_to(general_admin_application_settings_path) + expect(ApplicationSetting.current.kroki_formats_excalidraw).to eq(true) + end + it "updates default_branch_name setting" do put :update, params: { application_setting: { default_branch_name: "example_branch_name" } } diff --git a/spec/controllers/admin/cohorts_controller_spec.rb b/spec/controllers/admin/cohorts_controller_spec.rb index 9eb2a713517..77a9c8eb223 100644 --- a/spec/controllers/admin/cohorts_controller_spec.rb +++ b/spec/controllers/admin/cohorts_controller_spec.rb @@ -3,37 +3,15 @@ require 'spec_helper' RSpec.describe Admin::CohortsController do - context 'as admin' do - let(:user) { create(:admin) } + let(:user) { create(:admin) } - before do - sign_in(user) - end - - it 'renders 200' do - get :index - - expect(response).to have_gitlab_http_status(:success) - end - - describe 'GET #index' do - it_behaves_like 'tracking unique visits', :index do - let(:target_id) { 'i_analytics_cohorts' } - end - end + before do + sign_in(user) end - context 'as normal user' do - let(:user) { create(:user) } - - before do - sign_in(user) - end - - it 'renders a 404' do - get :index + it 'redirects to Overview->Users' do + get :index - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to redirect_to(admin_users_path(tab: 'cohorts')) end end diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index 3fffc50475c..cba25dbff95 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -27,7 +27,8 @@ RSpec.describe Admin::RunnersController do # There is still an N+1 query for `runner.builds.count` # We also need to add 1 because it takes 2 queries to preload tags - expect { get :index }.not_to exceed_query_limit(control_count + 6) + # also looking for token nonce requires database queries + expect { get :index }.not_to exceed_query_limit(control_count + 16) expect(response).to have_gitlab_http_status(:ok) expect(response.body).to have_content('tag1') diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index f902a3d2541..6faec315eb6 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -29,6 +29,11 @@ RSpec.describe Admin::UsersController do expect(assigns(:users).first.association(:authorized_projects)).to be_loaded end + + it_behaves_like 'tracking unique visits', :index do + let(:target_id) { 'i_analytics_cohorts' } + let(:request_params) { { tab: 'cohorts' } } + end end describe 'GET :id' do @@ -180,7 +185,7 @@ RSpec.describe Admin::UsersController do it 'displays the error' do subject - expect(flash[:alert]).to eq('The user you are trying to approve is not pending an approval') + expect(flash[:alert]).to eq('The user you are trying to approve is not pending approval') end it 'does not activate the user' do diff --git a/spec/controllers/chaos_controller_spec.rb b/spec/controllers/chaos_controller_spec.rb index 550303d292a..cb4f12ff829 100644 --- a/spec/controllers/chaos_controller_spec.rb +++ b/spec/controllers/chaos_controller_spec.rb @@ -124,4 +124,23 @@ RSpec.describe ChaosController do expect(response).to have_gitlab_http_status(:ok) end end + + describe '#gc' do + let(:gc_stat) { GC.stat.stringify_keys } + + it 'runs a full GC on the current web worker' do + expect(Prometheus::PidProvider).to receive(:worker_id).and_return('worker-0') + expect(Gitlab::Chaos).to receive(:run_gc).and_return(gc_stat) + + post :gc + + expect(response).to have_gitlab_http_status(:ok) + expect(response_json['worker_id']).to eq('worker-0') + expect(response_json['gc_stat']).to eq(gc_stat) + end + end + + def response_json + Gitlab::Json.parse(response.body) + end end diff --git a/spec/controllers/concerns/redis_tracking_spec.rb b/spec/controllers/concerns/redis_tracking_spec.rb index ef59adf8c1d..53b49dd30a6 100644 --- a/spec/controllers/concerns/redis_tracking_spec.rb +++ b/spec/controllers/concerns/redis_tracking_spec.rb @@ -3,18 +3,13 @@ require "spec_helper" RSpec.describe RedisTracking do - let(:feature) { 'approval_rule' } let(:user) { create(:user) } - before do - skip_feature_flags_yaml_validation - end - controller(ApplicationController) do include RedisTracking skip_before_action :authenticate_user!, only: :show - track_redis_hll_event :index, :show, name: 'g_compliance_approval_rules', feature: :approval_rule, feature_default_enabled: true, + track_redis_hll_event :index, :show, name: 'g_compliance_approval_rules', if: [:custom_condition_one?, :custom_condition_two?] def index @@ -49,97 +44,75 @@ RSpec.describe RedisTracking do expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) end - context 'with feature disabled' do - it 'does not track the event' do - stub_feature_flags(feature => false) - - expect_no_tracking - - get :index - end - end - - context 'with feature enabled' do + context 'when user is logged in' do before do - stub_feature_flags(feature => true) + sign_in(user) end - context 'when user is logged in' do - before do - sign_in(user) - end - - it 'tracks the event' do - expect_tracking - - get :index - end - - it 'passes default_enabled flag' do - expect(controller).to receive(:metric_feature_enabled?).with(feature.to_sym, true) + it 'tracks the event' do + expect_tracking - get :index - end + get :index + end - it 'tracks the event if DNT is not enabled' do - request.headers['DNT'] = '0' + it 'tracks the event if DNT is not enabled' do + request.headers['DNT'] = '0' - expect_tracking + expect_tracking - get :index - end + get :index + end - it 'does not track the event if DNT is enabled' do - request.headers['DNT'] = '1' + it 'does not track the event if DNT is enabled' do + request.headers['DNT'] = '1' - expect_no_tracking + expect_no_tracking - get :index - end + get :index + end - it 'does not track the event if the format is not HTML' do - expect_no_tracking + it 'does not track the event if the format is not HTML' do + expect_no_tracking - get :index, format: :json - end + get :index, format: :json + end - it 'does not track the event if a custom condition returns false' do - expect(controller).to receive(:custom_condition_two?).and_return(false) + it 'does not track the event if a custom condition returns false' do + expect(controller).to receive(:custom_condition_two?).and_return(false) - expect_no_tracking + expect_no_tracking - get :index - end + get :index + end - it 'does not track the event for untracked actions' do - expect_no_tracking + it 'does not track the event for untracked actions' do + expect_no_tracking - get :new - end + get :new end + end - context 'when user is not logged in and there is a visitor_id' do - let(:visitor_id) { SecureRandom.uuid } + context 'when user is not logged in and there is a visitor_id' do + let(:visitor_id) { SecureRandom.uuid } - before do - routes.draw { get 'show' => 'anonymous#show' } - end + before do + routes.draw { get 'show' => 'anonymous#show' } + end - it 'tracks the event' do - cookies[:visitor_id] = { value: visitor_id, expires: 24.months } + it 'tracks the event' do + cookies[:visitor_id] = { value: visitor_id, expires: 24.months } - expect_tracking + expect_tracking - get :show - end + get :show end + end - context 'when user is not logged in and there is no visitor_id' do - it 'does not track the event' do - expect_no_tracking + context 'when user is not logged in and there is no visitor_id' do + it 'does not track the event' do + expect_no_tracking - get :index - end + get :index end end end diff --git a/spec/controllers/concerns/spammable_actions_spec.rb b/spec/controllers/concerns/spammable_actions_spec.rb index 3b5b4d11a9b..25d5398c9da 100644 --- a/spec/controllers/concerns/spammable_actions_spec.rb +++ b/spec/controllers/concerns/spammable_actions_spec.rb @@ -6,21 +6,8 @@ RSpec.describe SpammableActions do controller(ActionController::Base) do include SpammableActions - # #create is used to test spammable_params - # for testing purposes - def create - spam_params = spammable_params - - # replace the actual request with a string in the JSON response, all we care is that it got set - spam_params[:request] = 'this is the request' if spam_params[:request] - - # just return the params in the response so they can be verified in this fake controller spec. - # Normally, they are processed further by the controller action - render json: spam_params.to_json, status: :ok - end - - # #update is used to test recaptcha_check_with_fallback - # for testing purposes + # #update is used here to test #recaptcha_check_with_fallback, but it could be invoked + # from #create or any other action which mutates a spammable via a controller. def update should_redirect = params[:should_redirect] == 'true' @@ -35,80 +22,7 @@ RSpec.describe SpammableActions do end before do - # Ordinarily we would not stub a method on the class under test, but :ensure_spam_config_loaded! - # returns false in the test environment, and is also strong_memoized, so we need to stub it - allow(controller).to receive(:ensure_spam_config_loaded!) { true } - end - - describe '#spammable_params' do - subject { post :create, format: :json, params: params } - - shared_examples 'expects request param only' do - it do - subject - - expect(response).to be_successful - expect(json_response).to eq({ 'request' => 'this is the request' }) - end - end - - shared_examples 'expects all spammable params' do - it do - subject - - expect(response).to be_successful - expect(json_response['request']).to eq('this is the request') - expect(json_response['recaptcha_verified']).to eq(true) - expect(json_response['spam_log_id']).to eq('1') - end - end - - let(:recaptcha_response) { nil } - let(:spam_log_id) { nil } - - context 'when recaptcha response is not present' do - let(:params) do - { - spam_log_id: spam_log_id - } - end - - it_behaves_like 'expects request param only' - end - - context 'when recaptcha response is present' do - let(:recaptcha_response) { 'abd123' } - let(:params) do - { - 'g-recaptcha-response': recaptcha_response, - spam_log_id: spam_log_id - } - end - - context 'when verify_recaptcha returns falsey' do - before do - expect(controller).to receive(:verify_recaptcha).with( - { - response: recaptcha_response - }) { false } - end - - it_behaves_like 'expects request param only' - end - - context 'when verify_recaptcha returns truthy' do - let(:spam_log_id) { 1 } - - before do - expect(controller).to receive(:verify_recaptcha).with( - { - response: recaptcha_response - }) { true } - end - - it_behaves_like 'expects all spammable params' - end - end + allow(Gitlab::Recaptcha).to receive(:load_configurations!) { true } end describe '#recaptcha_check_with_fallback' do @@ -154,12 +68,9 @@ RSpec.describe SpammableActions do allow(spammable).to receive(:valid?) { false } end - # NOTE: Not adding coverage of details of render_recaptcha?, the plan is to refactor it out - # of this module anyway as part of adding support for the GraphQL reCAPTCHA flow. - - context 'when render_recaptcha? is true' do + context 'when spammable.render_recaptcha? is true' do before do - expect(controller).to receive(:render_recaptcha?) { true } + expect(spammable).to receive(:render_recaptcha?) { true } end context 'when format is :html' do diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index e4aea688a69..f10fbf5ef2c 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -66,6 +66,16 @@ RSpec.describe GraphqlController do expect(assigns(:context)[:is_sessionless_user]).to be false end + + it 'calls the track api when trackable method' do + agent = 'vs-code-gitlab-workflow/3.11.1 VSCode/1.52.1 Node.js/12.14.1 (darwin; x64)' + request.env['HTTP_USER_AGENT'] = agent + + expect(Gitlab::UsageDataCounters::VSCodeExtensionActivityUniqueCounter) + .to receive(:track_api_request_when_trackable).with(user_agent: agent, user: user) + + post :execute + end end context 'when user uses an API token' do @@ -83,6 +93,16 @@ RSpec.describe GraphqlController do expect(assigns(:context)[:is_sessionless_user]).to be true end + + it 'calls the track api when trackable method' do + agent = 'vs-code-gitlab-workflow/3.11.1 VSCode/1.52.1 Node.js/12.14.1 (darwin; x64)' + request.env['HTTP_USER_AGENT'] = agent + + expect(Gitlab::UsageDataCounters::VSCodeExtensionActivityUniqueCounter) + .to receive(:track_api_request_when_trackable).with(user_agent: agent, user: user) + + subject + end end context 'when user is not logged in' do diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 4d78419e8eb..ff7a7f55863 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -225,6 +225,18 @@ RSpec.describe Groups::GroupMembersController do expect(requester.reload.expires_at).not_to eq(expires_at.to_date) end + + it 'returns error status' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + + it 'returns error message' do + subject + + expect(json_response).to eq({ 'message' => 'Expires at cannot be a date in the past' }) + end end context 'when set to a date in the future' do diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 939c36a98b2..9e5f68820d9 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -306,66 +306,6 @@ RSpec.describe GroupsController, factory_default: :keep do end end end - - describe 'tracking group creation for onboarding issues experiment' do - before do - sign_in(user) - end - - subject(:create_namespace) { post :create, params: { group: { name: 'new_group', path: 'new_group' } } } - - context 'experiment disabled' do - before do - stub_experiment(onboarding_issues: false) - end - - it 'does not track anything', :snowplow do - create_namespace - - expect_no_snowplow_event - end - end - - context 'experiment enabled' do - before do - stub_experiment(onboarding_issues: true) - end - - context 'and the user is part of the control group' do - before do - stub_experiment_for_subject(onboarding_issues: false) - end - - it 'tracks the event with the "created_namespace" action with the "control_group" property', :snowplow do - create_namespace - - expect_snowplow_event( - category: 'Growth::Conversion::Experiment::OnboardingIssues', - action: 'created_namespace', - label: anything, - property: 'control_group' - ) - end - end - - context 'and the user is part of the experimental group' do - before do - stub_experiment_for_subject(onboarding_issues: true) - end - - it 'tracks the event with the "created_namespace" action with the "experimental_group" property', :snowplow do - create_namespace - - expect_snowplow_event( - category: 'Growth::Conversion::Experiment::OnboardingIssues', - action: 'created_namespace', - label: anything, - property: 'experimental_group' - ) - end - end - end - end end describe 'GET #index' do diff --git a/spec/controllers/help_controller_spec.rb b/spec/controllers/help_controller_spec.rb index 6927df3b1c7..629d9b50d73 100644 --- a/spec/controllers/help_controller_spec.rb +++ b/spec/controllers/help_controller_spec.rb @@ -7,6 +7,43 @@ RSpec.describe HelpController do let(:user) { create(:user) } + shared_examples 'documentation pages local render' do + it 'renders HTML' do + aggregate_failures do + is_expected.to render_template('show.html.haml') + expect(response.media_type).to eq 'text/html' + end + end + end + + shared_examples 'documentation pages redirect' do |documentation_base_url| + let(:gitlab_version) { '13.4.0-ee' } + + before do + stub_version(gitlab_version, 'ignored_revision_value') + end + + it 'redirects user to custom documentation url with a specified version' do + is_expected.to redirect_to("#{documentation_base_url}/13.4/ee/#{path}.html") + end + + context 'when it is a pre-release' do + let(:gitlab_version) { '13.4.0-pre' } + + it 'redirects user to custom documentation url without a version' do + is_expected.to redirect_to("#{documentation_base_url}/ee/#{path}.html") + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(help_page_documentation_redirect: false) + end + + it_behaves_like 'documentation pages local render' + end + end + before do sign_in(user) end @@ -99,69 +136,70 @@ RSpec.describe HelpController do describe 'GET #show' do context 'for Markdown formats' do + subject { get :show, params: { path: path }, format: :md } + + let(:path) { 'ssh/README' } + context 'when requested file exists' do before do expect_file_read(File.join(Rails.root, 'doc/ssh/README.md'), content: fixture_file('blockquote_fence_after.md')) - get :show, params: { path: 'ssh/README' }, format: :md + subject end it 'assigns to @markdown' do expect(assigns[:markdown]).not_to be_empty end - it 'renders HTML' do - aggregate_failures do - expect(response).to render_template('show.html.haml') - expect(response.media_type).to eq 'text/html' - end - end + it_behaves_like 'documentation pages local render' end - context 'when a custom help_page_documentation_url is set' do + context 'when a custom help_page_documentation_url is set in database' do before do - stub_application_setting(help_page_documentation_base_url: documentation_base_url) - stub_version(gitlab_version, 'deadbeaf') + stub_application_setting(help_page_documentation_base_url: 'https://in-db.gitlab.com') end - subject { get :show, params: { path: path }, format: 'html' } + it_behaves_like 'documentation pages redirect', 'https://in-db.gitlab.com' + end - let(:gitlab_version) { '13.4.0-ee' } - let(:documentation_base_url) { 'https://docs.gitlab.com' } - let(:path) { 'ssh/README' } + context 'when a custom help_page_documentation_url is set in configuration file' do + let(:host) { 'https://in-yaml.gitlab.com' } + let(:docs_enabled) { true } - it 'redirects user to custom documentation url with a specified version' do - is_expected.to redirect_to("#{documentation_base_url}/13.4/ee/#{path}.html") + before do + allow(Settings).to receive(:gitlab_docs) { double(enabled: docs_enabled, host: host) } end - context 'when documentation url ends with a slash' do - let(:documentation_base_url) { 'https://docs.gitlab.com/' } + it_behaves_like 'documentation pages redirect', 'https://in-yaml.gitlab.com' - it 'redirects user to custom documentation url without slash duplicates' do - is_expected.to redirect_to("https://docs.gitlab.com/13.4/ee/#{path}.html") - end + context 'when gitlab_docs is disabled' do + let(:docs_enabled) { false } + + it_behaves_like 'documentation pages local render' end - context 'when it is a pre-release' do - let(:gitlab_version) { '13.4.0-pre' } + context 'when host is missing' do + let(:host) { nil } - it 'redirects user to custom documentation url without a version' do - is_expected.to redirect_to("#{documentation_base_url}/ee/#{path}.html") - end + it_behaves_like 'documentation pages local render' end + end - context 'when feature flag is disabled' do - before do - stub_feature_flags(help_page_documentation_redirect: false) - end + context 'when help_page_documentation_url is set in both db and configuration file' do + before do + stub_application_setting(help_page_documentation_base_url: 'https://in-db.gitlab.com') + allow(Settings).to receive(:gitlab_docs) { double(enabled: true, host: 'https://in-yaml.gitlab.com') } + end - it 'renders HTML' do - aggregate_failures do - is_expected.to render_template('show.html.haml') - expect(response.media_type).to eq 'text/html' - end - end + it_behaves_like 'documentation pages redirect', 'https://in-yaml.gitlab.com' + end + + context 'when help_page_documentation_url has a trailing slash' do + before do + allow(Settings).to receive(:gitlab_docs) { double(enabled: true, host: 'https://in-yaml.gitlab.com/') } end + + it_behaves_like 'documentation pages redirect', 'https://in-yaml.gitlab.com' end context 'when requested file is missing' do diff --git a/spec/controllers/import/bulk_imports_controller_spec.rb b/spec/controllers/import/bulk_imports_controller_spec.rb index d1c138617bb..08a54f112bb 100644 --- a/spec/controllers/import/bulk_imports_controller_spec.rb +++ b/spec/controllers/import/bulk_imports_controller_spec.rb @@ -59,7 +59,14 @@ RSpec.describe Import::BulkImportsController do parsed_response: [ { 'id' => 1, 'full_name' => 'group1', 'full_path' => 'full/path/group1', 'web_url' => 'http://demo.host/full/path/group1' }, { 'id' => 2, 'full_name' => 'group2', 'full_path' => 'full/path/group2', 'web_url' => 'http://demo.host/full/path/group1' } - ] + ], + headers: { + 'x-next-page' => '2', + 'x-page' => '1', + 'x-per-page' => '20', + 'x-total' => '37', + 'x-total-pages' => '2' + } ) end @@ -81,6 +88,17 @@ RSpec.describe Import::BulkImportsController do expect(json_response).to eq({ importable_data: client_response.parsed_response }.as_json) end + it 'forwards pagination headers' do + get :status, format: :json + + expect(response.headers['x-per-page']).to eq client_response.headers['x-per-page'] + expect(response.headers['x-page']).to eq client_response.headers['x-page'] + expect(response.headers['x-next-page']).to eq client_response.headers['x-next-page'] + expect(response.headers['x-prev-page']).to eq client_response.headers['x-prev-page'] + expect(response.headers['x-total']).to eq client_response.headers['x-total'] + expect(response.headers['x-total-pages']).to eq client_response.headers['x-total-pages'] + end + context 'when filtering' do it 'returns filtered result' do filter = 'test' @@ -167,6 +185,7 @@ RSpec.describe Import::BulkImportsController do describe 'POST create' do let(:instance_url) { "http://fake-intance" } + let(:bulk_import) { create(:bulk_import) } let(:pat) { "fake-pat" } before do @@ -183,12 +202,13 @@ RSpec.describe Import::BulkImportsController do expect_next_instance_of( BulkImportService, user, bulk_import_params, { url: instance_url, access_token: pat }) do |service| - expect(service).to receive(:execute) + allow(service).to receive(:execute).and_return(bulk_import) end post :create, params: { bulk_import: bulk_import_params } expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq({ id: bulk_import.id }.to_json) end end end diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index e863f5ef2fc..a8d38d12f23 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe InvitesController, :snowplow do +RSpec.describe InvitesController do let_it_be(:user) { create(:user) } let(:member) { create(:project_member, :invited, invite_email: user.email) } let(:raw_invite_token) { member.raw_invite_token } @@ -51,6 +51,28 @@ RSpec.describe InvitesController, :snowplow do end it_behaves_like 'invalid token' + + context 'when invite comes from the initial email invite' do + let(:params) { { id: raw_invite_token, invite_type: Members::InviteEmailExperiment::INVITE_TYPE } } + + it 'tracks via experiment', :aggregate_failures do + experiment = double(track: true) + allow(controller).to receive(:experiment).and_return(experiment) + + request + + expect(experiment).to have_received(:track).with(:opened) + expect(experiment).to have_received(:track).with(:accepted) + end + end + + context 'when invite does not come from initial email invite' do + it 'does not track via experiment' do + expect(controller).not_to receive(:experiment) + + request + end + end end context 'when not logged in' do @@ -82,6 +104,25 @@ RSpec.describe InvitesController, :snowplow do subject(:request) { post :accept, params: params } it_behaves_like 'invalid token' + + context 'when invite comes from the initial email invite' do + it 'tracks via experiment' do + experiment = double(track: true) + allow(controller).to receive(:experiment).and_return(experiment) + + post :accept, params: params, session: { invite_type: Members::InviteEmailExperiment::INVITE_TYPE } + + expect(experiment).to have_received(:track).with(:accepted) + end + end + + context 'when invite does not come from initial email invite' do + it 'does not track via experiment' do + expect(controller).not_to receive(:experiment) + + request + end + end end describe 'POST #decline for link in UI' do diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb index 90df7cc0991..03749366703 100644 --- a/spec/controllers/profiles/notifications_controller_spec.rb +++ b/spec/controllers/profiles/notifications_controller_spec.rb @@ -119,10 +119,11 @@ RSpec.describe Profiles::NotificationsController do it 'updates only permitted attributes' do sign_in(user) - put :update, params: { user: { notification_email: 'new@example.com', notified_of_own_activity: true, admin: true } } + put :update, params: { user: { notification_email: 'new@example.com', email_opted_in: true, notified_of_own_activity: true, admin: true } } user.reload expect(user.notification_email).to eq('new@example.com') + expect(user.email_opted_in).to eq(true) expect(user.notified_of_own_activity).to eq(true) expect(user.admin).to eq(false) expect(controller).to set_flash[:notice].to('Notification settings saved') diff --git a/spec/controllers/profiles/preferences_controller_spec.rb b/spec/controllers/profiles/preferences_controller_spec.rb index 4a68475c37f..b7870a63f9d 100644 --- a/spec/controllers/profiles/preferences_controller_spec.rb +++ b/spec/controllers/profiles/preferences_controller_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Profiles::PreferencesController do end describe 'PATCH update' do - def go(params: {}, format: :js) + def go(params: {}, format: :json) params.reverse_merge!( color_scheme_id: '1', dashboard: 'stars', @@ -35,9 +35,12 @@ RSpec.describe Profiles::PreferencesController do end context 'on successful update' do - it 'sets the flash' do + it 'responds with success' do go - expect(flash[:notice]).to eq _('Preferences saved.') + + expect(response).to have_gitlab_http_status(:ok) + expect(response.parsed_body['message']).to eq _('Preferences saved.') + expect(response.parsed_body['type']).to eq('notice') end it "changes the user's preferences" do @@ -59,36 +62,26 @@ RSpec.describe Profiles::PreferencesController do end context 'on failed update' do - it 'sets the flash' do + it 'responds with error' do expect(user).to receive(:save).and_return(false) go - expect(flash[:alert]).to eq(_('Failed to save preferences.')) + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.parsed_body['message']).to eq _('Failed to save preferences.') + expect(response.parsed_body['type']).to eq('alert') end end context 'on invalid dashboard setting' do - it 'sets the flash' do + it 'responds with error' do prefs = { dashboard: 'invalid' } go params: prefs - expect(flash[:alert]).to match(/\AFailed to save preferences \(.+\)\.\z/) - end - end - - context 'as js' do - it 'renders' do - go - expect(response).to render_template :update - end - end - - context 'as html' do - it 'redirects' do - go format: :html - expect(response).to redirect_to(profile_preferences_path) + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.parsed_body['message']).to match(/\AFailed to save preferences \(.+\)\.\z/) + expect(response.parsed_body['type']).to eq('alert') end end end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 16be7394174..68551ce4858 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -424,7 +424,7 @@ RSpec.describe Projects::BlobController do end end - it_behaves_like 'tracking unique hll events', :track_editor_edit_actions do + it_behaves_like 'tracking unique hll events' do subject(:request) { put :update, params: default_params } let(:target_id) { 'g_edit_by_sfe' } @@ -540,7 +540,7 @@ RSpec.describe Projects::BlobController do sign_in(user) end - it_behaves_like 'tracking unique hll events', :track_editor_edit_actions do + it_behaves_like 'tracking unique hll events' do subject(:request) { post :create, params: default_params } let(:target_id) { 'g_edit_by_sfe' } diff --git a/spec/controllers/projects/ci/daily_build_group_report_results_controller_spec.rb b/spec/controllers/projects/ci/daily_build_group_report_results_controller_spec.rb index 594c24bb7e3..81318b49cd9 100644 --- a/spec/controllers/projects/ci/daily_build_group_report_results_controller_spec.rb +++ b/spec/controllers/projects/ci/daily_build_group_report_results_controller_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Projects::Ci::DailyBuildGroupReportResultsController do let(:end_date) { '2020-03-09' } let(:allowed_to_read) { true } let(:user) { create(:user) } + let(:feature_enabled?) { true } before do create_daily_coverage('rspec', 79.0, '2020-03-09') @@ -24,6 +25,8 @@ RSpec.describe Projects::Ci::DailyBuildGroupReportResultsController do allow(Ability).to receive(:allowed?).and_call_original allow(Ability).to receive(:allowed?).with(user, :read_build_report_results, project).and_return(allowed_to_read) + stub_feature_flags(coverage_data_new_finder: feature_enabled?) + get :index, params: { namespace_id: project.namespace, project_id: project, @@ -55,9 +58,7 @@ RSpec.describe Projects::Ci::DailyBuildGroupReportResultsController do end end - context 'when format is CSV' do - let(:format) { :csv } - + shared_examples 'CSV results' do it 'serves the results in CSV' do expect(response).to have_gitlab_http_status(:ok) expect(response.headers['Content-Type']).to eq('text/csv; charset=utf-8') @@ -88,9 +89,7 @@ RSpec.describe Projects::Ci::DailyBuildGroupReportResultsController do it_behaves_like 'ensuring policy' end - context 'when format is JSON' do - let(:format) { :json } - + shared_examples 'JSON results' do it 'serves the results in JSON' do expect(response).to have_gitlab_http_status(:ok) @@ -137,6 +136,38 @@ RSpec.describe Projects::Ci::DailyBuildGroupReportResultsController do it_behaves_like 'validating param_type' it_behaves_like 'ensuring policy' end + + context 'when format is JSON' do + let(:format) { :json } + + context 'when coverage_data_new_finder flag is enabled' do + let(:feature_enabled?) { true } + + it_behaves_like 'JSON results' + end + + context 'when coverage_data_new_finder flag is disabled' do + let(:feature_enabled?) { false } + + it_behaves_like 'JSON results' + end + end + + context 'when format is CSV' do + let(:format) { :csv } + + context 'when coverage_data_new_finder flag is enabled' do + let(:feature_enabled?) { true } + + it_behaves_like 'CSV results' + end + + context 'when coverage_data_new_finder flag is disabled' do + let(:feature_enabled?) { false } + + it_behaves_like 'CSV results' + end + end end def create_daily_coverage(group_name, coverage, date) diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb index f9d16e761cb..8a793e29bfa 100644 --- a/spec/controllers/projects/discussions_controller_spec.rb +++ b/spec/controllers/projects/discussions_controller_spec.rb @@ -186,6 +186,13 @@ RSpec.describe Projects::DiscussionsController do expect(Note.find(note.id).discussion.resolved?).to be false end + it "tracks thread unresolve usage data" do + expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) + .to receive(:track_unresolve_thread_action).with(user: user) + + delete :unresolve, params: request_params + end + it "returns status 200" do delete :unresolve, params: request_params diff --git a/spec/controllers/projects/forks_controller_spec.rb b/spec/controllers/projects/forks_controller_spec.rb index e8b30294cdd..7da3d403b53 100644 --- a/spec/controllers/projects/forks_controller_spec.rb +++ b/spec/controllers/projects/forks_controller_spec.rb @@ -209,6 +209,13 @@ RSpec.describe Projects::ForksController do } end + let(:created_project) do + Namespace + .find_by_id(params[:namespace_key]) + .projects + .find_by_path(params.fetch(:path, project.path)) + end + subject do post :create, params: params end @@ -260,6 +267,21 @@ RSpec.describe Projects::ForksController do expect(response).to redirect_to(namespace_project_import_path(user.namespace, project, continue: continue_params)) end end + + context 'custom attributes set' do + let(:params) { super().merge(path: 'something_custom', name: 'Something Custom', description: 'Something Custom', visibility: 'private') } + + it 'creates a project with custom values' do + subject + + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(namespace_project_import_path(user.namespace, params[:path])) + expect(created_project.path).to eq(params[:path]) + expect(created_project.name).to eq(params[:name]) + expect(created_project.description).to eq(params[:description]) + expect(created_project.visibility).to eq(params[:visibility]) + end + end end context 'when user is not signed in' do diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index d3bdf1baaae..81ffd2c4512 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -63,53 +63,20 @@ RSpec.describe Projects::IssuesController do end end - describe 'the null hypothesis experiment', :snowplow do - it 'defines the expected before actions' do - expect(controller).to use_before_action(:run_null_hypothesis_experiment) - end - - context 'when rolled out to 100%' do - it 'assigns the candidate experience and tracks the event' do - get :index, params: { namespace_id: project.namespace, project_id: project } - - expect_snowplow_event( - category: 'null_hypothesis', - action: 'index', - context: [{ - schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/0-3-0', - data: { variant: 'candidate', experiment: 'null_hypothesis', key: anything } - }] - ) - end + describe 'the null hypothesis experiment', :experiment do + before do + stub_experiments(null_hypothesis: :candidate) end - context 'when not rolled out' do - before do - stub_feature_flags(null_hypothesis: false) - end - - it 'assigns the control experience and tracks the event' do - get :index, params: { namespace_id: project.namespace, project_id: project } - - expect_snowplow_event( - category: 'null_hypothesis', - action: 'index', - context: [{ - schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/0-3-0', - data: { variant: 'control', experiment: 'null_hypothesis', key: anything } - }] - ) - end + it 'defines the expected before actions' do + expect(controller).to use_before_action(:run_null_hypothesis_experiment) end - context 'when gitlab_experiments is disabled' do - it 'does not run the experiment at all' do - stub_feature_flags(gitlab_experiments: false) + 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) - expect(controller).not_to receive(:run_null_hypothesis_experiment) - - get :index, params: { namespace_id: project.namespace, project_id: project } - end + get :index, params: { namespace_id: project.namespace, project_id: project } end end end @@ -1314,11 +1281,13 @@ RSpec.describe Projects::IssuesController do let!(:last_spam_log) { spam_logs.last } def post_verified_issue - post_new_issue({}, { spam_log_id: last_spam_log.id, 'g-recaptcha-response': true } ) + post_new_issue({}, { spam_log_id: last_spam_log.id, 'g-recaptcha-response': 'abc123' } ) end before do - expect(controller).to receive_messages(verify_recaptcha: true) + expect_next_instance_of(Captcha::CaptchaVerificationService) do |instance| + expect(instance).to receive(:execute) { true } + end end it 'accepts an issue after reCAPTCHA is verified' do diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 430808e1c63..80e1268cb01 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -15,54 +15,6 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do end describe 'GET index' do - describe 'pushing tracking_data to Gon' do - before do - stub_experiment(jobs_empty_state: experiment_active) - stub_experiment_for_subject(jobs_empty_state: in_experiment_group) - - get_index - end - - context 'when experiment not active' do - let(:experiment_active) { false } - let(:in_experiment_group) { false } - - it 'does not push tracking_data to Gon' do - expect(Gon.tracking_data).to be_nil - end - end - - context 'when experiment active and user in control group' do - let(:experiment_active) { true } - let(:in_experiment_group) { false } - - it 'pushes tracking_data to Gon' do - expect(Gon.tracking_data).to match( - { - category: 'Growth::Activation::Experiment::JobsEmptyState', - action: 'click_button', - label: anything, - property: 'control_group' - } - ) - end - end - - context 'when experiment active and user in experimental group' do - let(:experiment_active) { true } - let(:in_experiment_group) { true } - - it 'pushes tracking_data to gon' do - expect(Gon.tracking_data).to match( - category: 'Growth::Activation::Experiment::JobsEmptyState', - action: 'click_button', - label: anything, - property: 'experimental_group' - ) - end - end - end - context 'when scope is pending' do before do create(:ci_build, :pending, pipeline: pipeline) diff --git a/spec/controllers/projects/learn_gitlab_controller_spec.rb b/spec/controllers/projects/learn_gitlab_controller_spec.rb new file mode 100644 index 00000000000..f633f7aa246 --- /dev/null +++ b/spec/controllers/projects/learn_gitlab_controller_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::LearnGitlabController do + describe 'GET #index' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, namespace: user.namespace) } + + let(:learn_gitlab_experiment_enabled) { true } + let(:params) { { namespace_id: project.namespace.to_param, project_id: project } } + + subject { get :index, params: params } + + before do + allow(controller.helpers).to receive(:learn_gitlab_experiment_enabled?).and_return(learn_gitlab_experiment_enabled) + end + + context 'unauthenticated user' do + it { is_expected.to have_gitlab_http_status(:redirect) } + end + + context 'authenticated user' do + before do + sign_in(user) + end + + it { is_expected.to render_template(:index) } + + it 'pushes experiment to frontend' do + expect(controller).to receive(:push_frontend_experiment).with(:learn_gitlab_a, subject: user) + expect(controller).to receive(:push_frontend_experiment).with(:learn_gitlab_b, subject: user) + + subject + end + + context 'learn_gitlab experiment not enabled' do + let(:learn_gitlab_experiment_enabled) { false } + + it { is_expected.to have_gitlab_http_status(:not_found) } + end + end + end +end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index f54a07de853..50f8942d9d5 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -226,11 +226,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do let(:diffable_merge_ref) { true } it 'compares diffs with the head' do - MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request) - - expect(CompareService).to receive(:new).with( - project, merge_request.merge_ref_head.sha - ).and_call_original + create(:merge_request_diff, :merge_head, merge_request: merge_request) go(diff_head: true) @@ -242,8 +238,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do let(:diffable_merge_ref) { false } it 'compares diffs with the base' do - expect(CompareService).not_to receive(:new) - go(diff_head: true) expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index cf8b4c564c4..9b37c46fd86 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -1118,6 +1118,108 @@ RSpec.describe Projects::MergeRequestsController do end end + describe 'GET codequality_mr_diff_reports' do + let_it_be(:merge_request) do + create(:merge_request, + :with_merge_request_pipeline, + target_project: project, + source_project: project) + end + + let(:pipeline) do + create(:ci_pipeline, + :success, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end + + before do + allow_any_instance_of(MergeRequest) + .to receive(:find_codequality_mr_diff_reports) + .and_return(report) + + allow_any_instance_of(MergeRequest) + .to receive(:actual_head_pipeline) + .and_return(pipeline) + end + + subject(:get_codequality_mr_diff_reports) do + get :codequality_mr_diff_reports, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid + }, + format: :json + end + + context 'permissions on a public project with private CI/CD' do + let(:project) { create :project, :repository, :public, :builds_private } + let(:report) { { status: :parsed, data: { 'files' => {} } } } + + context 'while signed out' do + before do + sign_out(user) + end + + it 'responds with a 404' do + get_codequality_mr_diff_reports + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to be_blank + end + end + + context 'while signed in as an unrelated user' do + before do + sign_in(create(:user)) + end + + it 'responds with a 404' do + get_codequality_mr_diff_reports + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to be_blank + end + end + end + + context 'when pipeline has jobs with codequality mr diff report' do + before do + allow_any_instance_of(MergeRequest) + .to receive(:has_codequality_mr_diff_report?) + .and_return(true) + end + + context 'when processing codequality mr diff report is in progress' do + let(:report) { { status: :parsing } } + + it 'sends polling interval' do + expect(Gitlab::PollingInterval).to receive(:set_header) + + get_codequality_mr_diff_reports + end + + it 'returns 204 HTTP status' do + get_codequality_mr_diff_reports + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when processing codequality mr diff report is completed' do + let(:report) { { status: :parsed, data: { 'files' => {} } } } + + it 'returns codequality mr diff report' do + get_codequality_mr_diff_reports + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'files' => {} }) + end + end + end + end + describe 'GET terraform_reports' do let_it_be(:merge_request) do create(:merge_request, @@ -1269,7 +1371,6 @@ RSpec.describe Projects::MergeRequestsController do describe 'GET test_reports' do let_it_be(:merge_request) do create(:merge_request, - :with_diffs, :with_merge_request_pipeline, target_project: project, source_project: project @@ -1380,7 +1481,6 @@ RSpec.describe Projects::MergeRequestsController do describe 'GET accessibility_reports' do let_it_be(:merge_request) do create(:merge_request, - :with_diffs, :with_merge_request_pipeline, target_project: project, source_project: project @@ -1501,7 +1601,6 @@ RSpec.describe Projects::MergeRequestsController do describe 'GET codequality_reports' do let_it_be(:merge_request) do create(:merge_request, - :with_diffs, :with_merge_request_pipeline, target_project: project, source_project: project diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index e96113c0133..edebaf294c4 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -150,7 +150,7 @@ RSpec.describe Projects::NotesController do end it 'returns an empty page of notes' do - expect(Gitlab::EtagCaching::Middleware).not_to receive(:skip!) + expect(Gitlab::EtagCaching::Middleware).to receive(:skip!) request.headers['X-Last-Fetched-At'] = microseconds(Time.zone.now) @@ -169,6 +169,8 @@ RSpec.describe Projects::NotesController do end it 'returns all notes' do + expect(Gitlab::EtagCaching::Middleware).to receive(:skip!) + get :index, params: request_params expect(json_response['notes'].count).to eq((page_1 + page_2 + page_3).size + 1) @@ -313,7 +315,7 @@ RSpec.describe Projects::NotesController do let(:note_text) { 'some note' } let(:request_params) do { - note: { note: note_text, noteable_id: merge_request.id, noteable_type: 'MergeRequest' }, + note: { note: note_text, noteable_id: merge_request.id, noteable_type: 'MergeRequest' }.merge(extra_note_params), namespace_id: project.namespace, project_id: project, merge_request_diff_head_sha: 'sha', @@ -323,6 +325,7 @@ RSpec.describe Projects::NotesController do end let(:extra_request_params) { {} } + let(:extra_note_params) { {} } let(:project_visibility) { Gitlab::VisibilityLevel::PUBLIC } let(:merge_requests_access_level) { ProjectFeature::ENABLED } @@ -421,6 +424,41 @@ RSpec.describe Projects::NotesController do end end + context 'when creating a confidential note' do + let(:extra_request_params) { { format: :json } } + + context 'when `confidential` parameter is not provided' do + it 'sets `confidential` to `false` in JSON response' do + create! + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['confidential']).to be false + end + end + + context 'when `confidential` parameter is `false`' do + let(:extra_note_params) { { confidential: false } } + + it 'sets `confidential` to `false` in JSON response' do + create! + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['confidential']).to be false + end + end + + context 'when `confidential` parameter is `true`' do + let(:extra_note_params) { { confidential: true } } + + it 'sets `confidential` to `true` in JSON response' do + create! + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['confidential']).to be true + end + end + end + context 'when creating a note with quick actions' do context 'with commands that return changes' do let(:note_text) { "/award :thumbsup:\n/estimate 1d\n/spend 3h" } @@ -725,6 +763,51 @@ RSpec.describe Projects::NotesController do end end end + + context 'when the endpoint receives requests above the limit' do + before do + stub_application_setting(notes_create_limit: 3) + end + + it 'prevents from creating more notes', :request_store do + 3.times { create! } + + expect { create! } + .to change { Gitlab::GitalyClient.get_request_count }.by(0) + + create! + expect(response.body).to eq(_('This endpoint has been requested too many times. Try again later.')) + expect(response).to have_gitlab_http_status(:too_many_requests) + end + + it 'logs the event in auth.log' do + attributes = { + message: 'Application_Rate_Limiter_Request', + env: :notes_create_request_limit, + remote_ip: '0.0.0.0', + request_method: 'POST', + path: "/#{project.full_path}/notes", + user_id: user.id, + username: user.username + } + + expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once + + project.add_developer(user) + sign_in(user) + + 4.times { create! } + end + + it 'allows user in allow-list to create notes, even if the case is different' do + user.update_attribute(:username, user.username.titleize) + stub_application_setting(notes_create_limit_allowlist: ["#{user.username.downcase}"]) + 3.times { create! } + + create! + expect(response).to have_gitlab_http_status(:found) + end + end end describe 'PUT update' do diff --git a/spec/controllers/projects/pipelines/tests_controller_spec.rb b/spec/controllers/projects/pipelines/tests_controller_spec.rb index 61118487e20..e6ff3a487ac 100644 --- a/spec/controllers/projects/pipelines/tests_controller_spec.rb +++ b/spec/controllers/projects/pipelines/tests_controller_spec.rb @@ -34,20 +34,38 @@ RSpec.describe Projects::Pipelines::TestsController do end describe 'GET #show.json' do - context 'when pipeline has build report results' do - let(:pipeline) { create(:ci_pipeline, :with_report_results, project: project) } + context 'when pipeline has builds with test reports' do + let(:main_pipeline) { create(:ci_pipeline, :with_test_reports_with_three_failures, project: project) } + let(:pipeline) { create(:ci_pipeline, :with_test_reports_with_three_failures, project: project, ref: 'new-feature') } let(:suite_name) { 'test' } let(:build_ids) { pipeline.latest_builds.pluck(:id) } + before do + build = main_pipeline.builds.last + build.update_column(:finished_at, 1.day.ago) # Just to be sure we are included in the report window + + # The JUnit fixture for the given build has 3 failures. + # This service will create 1 test case failure record for each. + Ci::TestFailureHistoryService.new(main_pipeline).execute + end + it 'renders test suite data' do get_tests_show_json(build_ids) expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq('test') + + # Each test failure in this pipeline has a matching failure in the default branch + recent_failures = json_response['test_cases'].map { |tc| tc['recent_failures'] } + expect(recent_failures).to eq([ + { 'count' => 1, 'base_branch' => 'master' }, + { 'count' => 1, 'base_branch' => 'master' }, + { 'count' => 1, 'base_branch' => 'master' } + ]) end end - context 'when pipeline does not have build report results' do + context 'when pipeline has no builds that matches the given build_ids' do let(:pipeline) { create(:ci_empty_pipeline) } let(:suite_name) { 'test' } diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index be4a1504fc9..e1405660ccb 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -272,72 +272,6 @@ RSpec.describe Projects::PipelinesController do end end - describe 'GET #index' do - subject(:request) { get :index, params: { namespace_id: project.namespace, project_id: project } } - - context 'experiment not active' do - it 'does not push tracking_data to gon' do - request - - expect(Gon.tracking_data).to be_nil - end - - it 'does not record experiment_user' do - expect { request }.not_to change(ExperimentUser, :count) - end - end - - context 'when experiment active' do - before do - stub_experiment(pipelines_empty_state: true) - stub_experiment_for_subject(pipelines_empty_state: true) - end - - it 'pushes tracking_data to Gon' do - request - - expect(Gon.experiments["pipelinesEmptyState"]).to eq(true) - expect(Gon.tracking_data).to match( - { - category: 'Growth::Activation::Experiment::PipelinesEmptyState', - action: 'view', - label: anything, - property: 'experimental_group', - value: anything - } - ) - end - - context 'no pipelines created an no CI set up' do - before do - stub_application_setting(auto_devops_enabled: false) - end - - it 'records experiment_user' do - expect { request }.to change(ExperimentUser, :count).by(1) - end - end - - context 'CI set up' do - it 'does not record experiment_user' do - expect { request }.not_to change(ExperimentUser, :count) - end - end - - context 'pipelines created' do - let!(:pipeline) { create(:ci_pipeline, project: project) } - - before do - stub_application_setting(auto_devops_enabled: false) - end - - it 'does not record experiment_user' do - expect { request }.not_to change(ExperimentUser, :count) - end - end - end - end - describe 'GET show.json' do let(:pipeline) { create(:ci_pipeline, project: project) } diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index d30cc8cbfd9..53a7c2ca069 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -325,6 +325,18 @@ RSpec.describe Projects::ProjectMembersController do expect(requester.reload.expires_at).not_to eq(expires_at.to_date) end + + it 'returns error status' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + + it 'returns error message' do + subject + + expect(json_response).to eq({ 'message' => 'Expires at cannot be a date in the past' }) + end end context 'when set to a date in the future' do diff --git a/spec/controllers/projects/security/configuration_controller_spec.rb b/spec/controllers/projects/security/configuration_controller_spec.rb new file mode 100644 index 00000000000..ef255d1efd0 --- /dev/null +++ b/spec/controllers/projects/security/configuration_controller_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::Security::ConfigurationController do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + + before do + allow(controller).to receive(:ensure_security_and_compliance_enabled!) + + sign_in(user) + end + + describe 'GET show' do + context 'when feature flag is disabled' do + before do + stub_feature_flags(secure_security_and_compliance_configuration_page_on_ce: false) + end + + it 'renders not found' do + get :show, params: { namespace_id: project.namespace, project_id: project } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when feature flag is enabled' do + context 'when user has guest access' do + before do + project.add_guest(user) + end + + it 'denies access' do + get :show, params: { namespace_id: project.namespace, project_id: project } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user has developer access' do + before do + project.add_developer(user) + end + + it 'grants access' do + get :show, params: { namespace_id: project.namespace, project_id: project } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + end + end + end + end +end diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb index 01593f4133c..fe282baf769 100644 --- a/spec/controllers/projects/templates_controller_spec.rb +++ b/spec/controllers/projects/templates_controller_spec.rb @@ -165,7 +165,8 @@ RSpec.describe Projects::TemplatesController do expect(response).to have_gitlab_http_status(:ok) expect(json_response.size).to eq(2) - expect(json_response).to match(expected_template_names) + expect(json_response.size).to eq(2) + expect(json_response.map { |x| x.slice('name') }).to match(expected_template_names) end it 'fails for user with no access' do diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index a611ac16cd9..1e4ec48b119 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -5,6 +5,7 @@ require('spec_helper') RSpec.describe ProjectsController do include ExternalAuthorizationServiceHelpers include ProjectForksHelper + using RSpec::Parameterized::TableSyntax let_it_be(:project, reload: true) { create(:project, service_desk_enabled: false) } let_it_be(:public_project) { create(:project, :public) } @@ -324,14 +325,39 @@ RSpec.describe ProjectsController do end end - context "redirection from http://someproject.git" do - it 'redirects to project page (format.html)' do - project = create(:project, :public) + context 'redirection from http://someproject.git' do + where(:user_type, :project_visibility, :expected_redirect) do + :anonymous | :public | :redirect_to_project + :anonymous | :internal | :redirect_to_signup + :anonymous | :private | :redirect_to_signup - get :show, params: { namespace_id: project.namespace, id: project }, format: :git + :signed_in | :public | :redirect_to_project + :signed_in | :internal | :redirect_to_project + :signed_in | :private | nil - expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(namespace_project_path) + :member | :public | :redirect_to_project + :member | :internal | :redirect_to_project + :member | :private | :redirect_to_project + end + + with_them do + let(:redirect_to_signup) { new_user_session_path } + let(:redirect_to_project) { project_path(project) } + + let(:expected_status) { expected_redirect ? :found : :not_found } + + before do + project.update!(visibility: project_visibility.to_s) + project.team.add_user(user, :guest) if user_type == :member + sign_in(user) unless user_type == :anonymous + end + + it 'returns the expected status' do + get :show, params: { namespace_id: project.namespace, id: project }, format: :git + + expect(response).to have_gitlab_http_status(expected_status) + expect(response).to redirect_to(send(expected_redirect)) if expected_status == :found + end end end @@ -384,6 +410,29 @@ RSpec.describe ProjectsController do end end + describe 'POST create' do + let!(:project_params) do + { + path: 'foo', + description: 'bar', + namespace_id: user.namespace.id, + visibility_level: Gitlab::VisibilityLevel::PUBLIC + } + end + + before do + sign_in(user) + end + + it 'tracks a created event for the new_project_readme experiment', :experiment do + expect(experiment(:new_project_readme)).to track(:created, property: 'blank').on_any_instance.with_context( + actor: user + ) + + post :create, params: { project: project_params } + end + end + describe 'POST #archive' do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } diff --git a/spec/controllers/registrations/experience_levels_controller_spec.rb b/spec/controllers/registrations/experience_levels_controller_spec.rb index 015daba8682..79fa3f1474a 100644 --- a/spec/controllers/registrations/experience_levels_controller_spec.rb +++ b/spec/controllers/registrations/experience_levels_controller_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Registrations::ExperienceLevelsController do + include AfterNextHelpers + let_it_be(:namespace) { create(:group, path: 'group-path' ) } let_it_be(:user) { create(:user) } @@ -19,20 +21,11 @@ RSpec.describe Registrations::ExperienceLevelsController do context 'with an authenticated user' do before do sign_in(user) - stub_experiment_for_subject(onboarding_issues: true) end it { is_expected.to have_gitlab_http_status(:ok) } - it { is_expected.to render_template('layouts/devise_experimental_onboarding_issues') } + it { is_expected.to render_template('layouts/signup_onboarding') } it { is_expected.to render_template(:show) } - - context 'when not part of the onboarding issues experiment' do - before do - stub_experiment_for_subject(onboarding_issues: false) - end - - it { is_expected.to have_gitlab_http_status(:not_found) } - end end end @@ -45,17 +38,11 @@ RSpec.describe Registrations::ExperienceLevelsController do end context 'with an authenticated user' do + let_it_be(:project) { build(:project, namespace: namespace, creator: user, path: 'project-path') } + let_it_be(:issues_board) { build(:board, id: 123, project: project) } + before do sign_in(user) - stub_experiment_for_subject(onboarding_issues: true) - end - - context 'when not part of the onboarding issues experiment' do - before do - stub_experiment_for_subject(onboarding_issues: false) - end - - it { is_expected.to have_gitlab_http_status(:not_found) } end context 'when user is successfully updated' do @@ -85,91 +72,57 @@ RSpec.describe Registrations::ExperienceLevelsController do end end - describe 'redirection' do - let(:project) { build(:project, namespace: namespace, creator: user, path: 'project-path') } - let(:issues_board) { build(:board, id: 123, project: project) } + context 'when "Learn GitLab" project exists' do + let(:learn_gitlab_available?) { true } before do - stub_experiment_for_subject( - onboarding_issues: true, - default_to_issues_board: default_to_issues_board_xp? - ) allow_next_instance_of(LearnGitlab) do |learn_gitlab| allow(learn_gitlab).to receive(:available?).and_return(learn_gitlab_available?) allow(learn_gitlab).to receive(:project).and_return(project) allow(learn_gitlab).to receive(:board).and_return(issues_board) + allow(learn_gitlab).to receive(:label).and_return(double(id: 1)) end end - context 'when namespace_path param is missing' do - let(:params) { super().merge(namespace_path: nil) } - - where( - default_to_issues_board_xp?: [true, false], - learn_gitlab_available?: [true, false] - ) - - with_them do - it { is_expected.to redirect_to('/') } - end - end - - context 'when we have a namespace_path param' do - using RSpec::Parameterized::TableSyntax + context 'redirection' do + context 'when namespace_path param is missing' do + let(:params) { super().merge(namespace_path: nil) } - where(:default_to_issues_board_xp?, :learn_gitlab_available?, :path) do - true | true | '/group-path/project-path/-/boards/123' - true | false | '/group-path' - false | true | '/group-path' - false | false | '/group-path' - end - - with_them do - it { is_expected.to redirect_to(path) } - end - end - end + where( + learn_gitlab_available?: [true, false] + ) - describe 'applying the chosen level' do - context 'when a "Learn GitLab" project is available' do - before do - allow_next_instance_of(LearnGitlab) do |learn_gitlab| - allow(learn_gitlab).to receive(:available?).and_return(true) - allow(learn_gitlab).to receive(:label).and_return(double(id: 1)) + with_them do + it { is_expected.to redirect_to('/') } end end - context 'when novice' do - let(:params) { super().merge(experience_level: :novice) } - - it 'adds a BoardLabel' do - expect_next_instance_of(Boards::UpdateService) do |service| - expect(service).to receive(:execute) - end + context 'when we have a namespace_path param' do + using RSpec::Parameterized::TableSyntax - subject + where(:learn_gitlab_available?, :path) do + true | '/group-path/project-path/-/boards/123' + false | '/group-path' end - end - - context 'when experienced' do - let(:params) { super().merge(experience_level: :experienced) } - it 'does not add a BoardLabel' do - expect(Boards::UpdateService).not_to receive(:new) - - subject + with_them do + it { is_expected.to redirect_to(path) } end end end - context 'when no "Learn GitLab" project exists' do + context 'when novice' do let(:params) { super().merge(experience_level: :novice) } - before do - allow_next_instance_of(LearnGitlab) do |learn_gitlab| - allow(learn_gitlab).to receive(:available?).and_return(false) - end + it 'adds a BoardLabel' do + expect_next(Boards::UpdateService).to receive(:execute) + + subject end + end + + context 'when experienced' do + let(:params) { super().merge(experience_level: :experienced) } it 'does not add a BoardLabel' do expect(Boards::UpdateService).not_to receive(:new) @@ -178,6 +131,20 @@ RSpec.describe Registrations::ExperienceLevelsController do end end end + + context 'when no "Learn GitLab" project exists' do + let(:params) { super().merge(experience_level: :novice) } + + before do + allow_next(LearnGitlab).to receive(:available?).and_return(false) + end + + it 'does not add a BoardLabel' do + expect(Boards::UpdateService).not_to receive(:new) + + subject + end + end end context 'when user update fails' do diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 737ec4f95c5..aac7c10d878 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -73,6 +73,18 @@ RSpec.describe RegistrationsController do end end end + + context 'audit events' do + context 'when not licensed' do + before do + stub_licensed_features(admin_audit_log: false) + end + + it 'does not log any audit event' do + expect { subject }.not_to change(AuditEvent, :count) + end + end + end end context 'when the `require_admin_approval_after_user_signup` setting is turned off' do diff --git a/spec/controllers/repositories/git_http_controller_spec.rb b/spec/controllers/repositories/git_http_controller_spec.rb index 34052496871..d21f602f90c 100644 --- a/spec/controllers/repositories/git_http_controller_spec.rb +++ b/spec/controllers/repositories/git_http_controller_spec.rb @@ -36,7 +36,7 @@ RSpec.describe Repositories::GitHttpController do context 'when project_statistics_sync feature flag is disabled' do before do - stub_feature_flags(project_statistics_sync: false) + stub_feature_flags(project_statistics_sync: false, disable_git_http_fetch_writes: false) end it 'updates project statistics async for projects' do @@ -47,17 +47,40 @@ RSpec.describe Repositories::GitHttpController do end it 'updates project statistics sync for projects' do + stub_feature_flags(disable_git_http_fetch_writes: false) + expect { send_request }.to change { Projects::DailyStatisticsFinder.new(container).total_fetch_count }.from(0).to(1) end - it 'records an onboarding progress action' do - expect_next_instance_of(OnboardingProgressService) do |service| - expect(service).to receive(:execute).with(action: :git_read) + it_behaves_like 'records an onboarding progress action', :git_read do + let(:namespace) { project.namespace } + + subject { send_request } + + before do + stub_feature_flags(disable_git_http_fetch_writes: false) + end + end + + context 'when disable_git_http_fetch_writes is enabled' do + before do + stub_feature_flags(disable_git_http_fetch_writes: true) + end + + it 'does not increment statistics' do + expect(Projects::FetchStatisticsIncrementService).not_to receive(:new) + expect(ProjectDailyStatisticsWorker).not_to receive(:perform_async) + + send_request end - send_request + it 'does not record onboarding progress' do + expect(OnboardingProgressService).not_to receive(:new) + + send_request + end end end end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index bbd39fd4c83..95cea10f0d0 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -5,278 +5,296 @@ require 'spec_helper' RSpec.describe SearchController do include ExternalAuthorizationServiceHelpers - let(:user) { create(:user) } + context 'authorized user' do + let(:user) { create(:user) } - before do - sign_in(user) - end - - shared_examples_for 'when the user cannot read cross project' do |action, params| before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?) - .with(user, :read_cross_project, :global) { false } + sign_in(user) end - it 'blocks access without a project_id' do - get action, params: params - - expect(response).to have_gitlab_http_status(:forbidden) - end + shared_examples_for 'when the user cannot read cross project' do |action, params| + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?) + .with(user, :read_cross_project, :global) { false } + end - it 'allows access with a project_id' do - get action, params: params.merge(project_id: create(:project, :public).id) + it 'blocks access without a project_id' do + get action, params: params - expect(response).to have_gitlab_http_status(:ok) - end - end + expect(response).to have_gitlab_http_status(:forbidden) + end - shared_examples_for 'with external authorization service enabled' do |action, params| - let(:project) { create(:project, namespace: user.namespace) } - let(:note) { create(:note_on_issue, project: project) } + it 'allows access with a project_id' do + get action, params: params.merge(project_id: create(:project, :public).id) - before do - enable_external_authorization_service_check + expect(response).to have_gitlab_http_status(:ok) + end end - it 'renders a 403 when no project is given' do - get action, params: params + shared_examples_for 'with external authorization service enabled' do |action, params| + let(:project) { create(:project, namespace: user.namespace) } + let(:note) { create(:note_on_issue, project: project) } - expect(response).to have_gitlab_http_status(:forbidden) - end + before do + enable_external_authorization_service_check + end - it 'renders a 200 when a project was set' do - get action, params: params.merge(project_id: project.id) + it 'renders a 403 when no project is given' do + get action, params: params - expect(response).to have_gitlab_http_status(:ok) - end - end + expect(response).to have_gitlab_http_status(:forbidden) + end - describe 'GET #show' do - it_behaves_like 'when the user cannot read cross project', :show, { search: 'hello' } do - it 'still allows accessing the search page' do - get :show + it 'renders a 200 when a project was set' do + get action, params: params.merge(project_id: project.id) expect(response).to have_gitlab_http_status(:ok) end end - it_behaves_like 'with external authorization service enabled', :show, { search: 'hello' } + describe 'GET #show' do + it_behaves_like 'when the user cannot read cross project', :show, { search: 'hello' } do + it 'still allows accessing the search page' do + get :show - context 'uses the right partials depending on scope' do - using RSpec::Parameterized::TableSyntax - render_views - - let_it_be(:project) { create(:project, :public, :repository, :wiki_repo) } - - before do - expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + expect(response).to have_gitlab_http_status(:ok) + end end - subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) } + it_behaves_like 'with external authorization service enabled', :show, { search: 'hello' } - where(:partial, :scope) do - '_blob' | :blobs - '_wiki_blob' | :wiki_blobs - '_commit' | :commits - end + context 'uses the right partials depending on scope' do + using RSpec::Parameterized::TableSyntax + render_views - with_them do - it do - project_wiki = create(:project_wiki, project: project, user: user) - create(:wiki_page, wiki: project_wiki, title: 'merge', content: 'merge') + let_it_be(:project) { create(:project, :public, :repository, :wiki_repo) } - expect(subject).to render_template("search/results/#{partial}") + before do + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original end - end - end - context 'global search' do - using RSpec::Parameterized::TableSyntax - render_views + subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) } - context 'when block_anonymous_global_searches is disabled' do - before do - stub_feature_flags(block_anonymous_global_searches: false) + where(:partial, :scope) do + '_blob' | :blobs + '_wiki_blob' | :wiki_blobs + '_commit' | :commits end - it 'omits pipeline status from load' do - project = create(:project, :public) - expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) + with_them do + it do + project_wiki = create(:project_wiki, project: project, user: user) + create(:wiki_page, wiki: project_wiki, title: 'merge', content: 'merge') - get :show, params: { scope: 'projects', search: project.name } - - expect(assigns[:search_objects].first).to eq project + expect(subject).to render_template("search/results/#{partial}") + end end + end - context 'check search term length' do - let(:search_queries) do - char_limit = SearchService::SEARCH_CHAR_LIMIT - term_limit = SearchService::SEARCH_TERM_LIMIT - { - chars_under_limit: ('a' * (char_limit - 1)), - chars_over_limit: ('a' * (char_limit + 1)), - terms_under_limit: ('abc ' * (term_limit - 1)), - terms_over_limit: ('abc ' * (term_limit + 1)) - } + context 'global search' do + using RSpec::Parameterized::TableSyntax + render_views + + context 'when block_anonymous_global_searches is disabled' do + before do + stub_feature_flags(block_anonymous_global_searches: false) end - where(:string_name, :expectation) do - :chars_under_limit | :not_to_set_flash - :chars_over_limit | :set_chars_flash - :terms_under_limit | :not_to_set_flash - :terms_over_limit | :set_terms_flash + it 'omits pipeline status from load' do + project = create(:project, :public) + expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) + + get :show, params: { scope: 'projects', search: project.name } + + expect(assigns[:search_objects].first).to eq project end - with_them do - it do - get :show, params: { scope: 'projects', search: search_queries[string_name] } - - case expectation - when :not_to_set_flash - expect(controller).not_to set_flash[:alert] - when :set_chars_flash - expect(controller).to set_flash[:alert].to(/characters/) - when :set_terms_flash - expect(controller).to set_flash[:alert].to(/terms/) + context 'check search term length' do + let(:search_queries) do + char_limit = SearchService::SEARCH_CHAR_LIMIT + term_limit = SearchService::SEARCH_TERM_LIMIT + { + chars_under_limit: ('a' * (char_limit - 1)), + chars_over_limit: ('a' * (char_limit + 1)), + terms_under_limit: ('abc ' * (term_limit - 1)), + terms_over_limit: ('abc ' * (term_limit + 1)) + } + end + + where(:string_name, :expectation) do + :chars_under_limit | :not_to_set_flash + :chars_over_limit | :set_chars_flash + :terms_under_limit | :not_to_set_flash + :terms_over_limit | :set_terms_flash + end + + with_them do + it do + get :show, params: { scope: 'projects', search: search_queries[string_name] } + + case expectation + when :not_to_set_flash + expect(controller).not_to set_flash[:alert] + when :set_chars_flash + expect(controller).to set_flash[:alert].to(/characters/) + when :set_terms_flash + expect(controller).to set_flash[:alert].to(/terms/) + end end end end end - end - context 'when block_anonymous_global_searches is enabled' do - context 'for unauthenticated user' do - before do - sign_out(user) - end + context 'when block_anonymous_global_searches is enabled' do + context 'for unauthenticated user' do + before do + sign_out(user) + end - it 'redirects to login page' do - get :show, params: { scope: 'projects', search: '*' } + it 'redirects to login page' do + get :show, params: { scope: 'projects', search: '*' } - expect(response).to redirect_to new_user_session_path + expect(response).to redirect_to new_user_session_path + end end - end - context 'for authenticated user' do - it 'succeeds' do - get :show, params: { scope: 'projects', search: '*' } + context 'for authenticated user' do + it 'succeeds' do + get :show, params: { scope: 'projects', search: '*' } - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:ok) + end end end end - end - it 'finds issue comments' do - project = create(:project, :public) - note = create(:note_on_issue, project: project) + it 'finds issue comments' do + project = create(:project, :public) + note = create(:note_on_issue, project: project) - get :show, params: { project_id: project.id, scope: 'notes', search: note.note } - - expect(assigns[:search_objects].first).to eq note - end + get :show, params: { project_id: project.id, scope: 'notes', search: note.note } - context 'unique users tracking' do - before do - allow(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) + expect(assigns[:search_objects].first).to eq note end - it_behaves_like 'tracking unique hll events', :search_track_unique_users do - subject(:request) { get :show, params: { scope: 'projects', search: 'term' } } + context 'unique users tracking' do + before do + allow(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) + end + + it_behaves_like 'tracking unique hll events' do + subject(:request) { get :show, params: { scope: 'projects', search: 'term' } } - let(:target_id) { 'i_search_total' } - let(:expected_type) { instance_of(String) } + let(:target_id) { 'i_search_total' } + let(:expected_type) { instance_of(String) } + end end - end - context 'on restricted projects' do - context 'when signed out' do - before do - sign_out(user) + context 'on restricted projects' do + context 'when signed out' do + before do + sign_out(user) + end + + it "doesn't expose comments on issues" do + project = create(:project, :public, :issues_private) + note = create(:note_on_issue, project: project) + + get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + + expect(assigns[:search_objects].count).to eq(0) + end end - it "doesn't expose comments on issues" do - project = create(:project, :public, :issues_private) - note = create(:note_on_issue, project: project) + it "doesn't expose comments on merge_requests" do + project = create(:project, :public, :merge_requests_private) + note = create(:note_on_merge_request, project: project) get :show, params: { project_id: project.id, scope: 'notes', search: note.note } expect(assigns[:search_objects].count).to eq(0) end - end - it "doesn't expose comments on merge_requests" do - project = create(:project, :public, :merge_requests_private) - note = create(:note_on_merge_request, project: project) + it "doesn't expose comments on snippets" do + project = create(:project, :public, :snippets_private) + note = create(:note_on_project_snippet, project: project) - get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + get :show, params: { project_id: project.id, scope: 'notes', search: note.note } - expect(assigns[:search_objects].count).to eq(0) + expect(assigns[:search_objects].count).to eq(0) + end end + end - it "doesn't expose comments on snippets" do - project = create(:project, :public, :snippets_private) - note = create(:note_on_project_snippet, project: project) - - get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + describe 'GET #count' do + it_behaves_like 'when the user cannot read cross project', :count, { search: 'hello', scope: 'projects' } + it_behaves_like 'with external authorization service enabled', :count, { search: 'hello', scope: 'projects' } - expect(assigns[:search_objects].count).to eq(0) - end - end - end + it 'returns the result count for the given term and scope' do + create(:project, :public, name: 'hello world') + create(:project, :public, name: 'foo bar') - describe 'GET #count' do - it_behaves_like 'when the user cannot read cross project', :count, { search: 'hello', scope: 'projects' } - it_behaves_like 'with external authorization service enabled', :count, { search: 'hello', scope: 'projects' } + get :count, params: { search: 'hello', scope: 'projects' } - it 'returns the result count for the given term and scope' do - create(:project, :public, name: 'hello world') - create(:project, :public, name: 'foo bar') + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ 'count' => '1' }) + end - get :count, params: { search: 'hello', scope: 'projects' } + it 'raises an error if search term is missing' do + expect do + get :count, params: { scope: 'projects' } + end.to raise_error(ActionController::ParameterMissing) + end - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq({ 'count' => '1' }) + it 'raises an error if search scope is missing' do + expect do + get :count, params: { search: 'hello' } + end.to raise_error(ActionController::ParameterMissing) + end end - it 'raises an error if search term is missing' do - expect do - get :count, params: { scope: 'projects' } - end.to raise_error(ActionController::ParameterMissing) + describe 'GET #autocomplete' do + it_behaves_like 'when the user cannot read cross project', :autocomplete, { term: 'hello' } + it_behaves_like 'with external authorization service enabled', :autocomplete, { term: 'hello' } end - it 'raises an error if search scope is missing' do - expect do - get :count, params: { search: 'hello' } - end.to raise_error(ActionController::ParameterMissing) - end - end + describe '#append_info_to_payload' do + it 'appends search metadata for logging' do + last_payload = nil + original_append_info_to_payload = controller.method(:append_info_to_payload) - describe 'GET #autocomplete' do - it_behaves_like 'when the user cannot read cross project', :autocomplete, { term: 'hello' } - it_behaves_like 'with external authorization service enabled', :autocomplete, { term: 'hello' } - end + expect(controller).to receive(:append_info_to_payload) do |payload| + original_append_info_to_payload.call(payload) + last_payload = payload + end - describe '#append_info_to_payload' do - it 'appends search metadata for logging' do - last_payload = nil - original_append_info_to_payload = controller.method(:append_info_to_payload) + get :show, params: { scope: 'issues', search: 'hello world', group_id: '123', project_id: '456', confidential: true, state: true, force_search_results: true } - expect(controller).to receive(:append_info_to_payload) do |payload| - original_append_info_to_payload.call(payload) - last_payload = payload + expect(last_payload[:metadata]['meta.search.group_id']).to eq('123') + expect(last_payload[:metadata]['meta.search.project_id']).to eq('456') + expect(last_payload[:metadata]).not_to have_key('meta.search.search') + expect(last_payload[:metadata]['meta.search.scope']).to eq('issues') + expect(last_payload[:metadata]['meta.search.force_search_results']).to eq('true') + expect(last_payload[:metadata]['meta.search.filters.confidential']).to eq('true') + expect(last_payload[:metadata]['meta.search.filters.state']).to eq('true') end + end + end - get :show, params: { scope: 'issues', search: 'hello world', group_id: '123', project_id: '456', confidential: true, state: true, force_search_results: true } + context 'unauthorized user' do + describe 'GET #opensearch' do + render_views + + it 'renders xml' do + get :opensearch, format: :xml + + doc = Nokogiri::XML.parse(response.body) - expect(last_payload[:metadata]['meta.search.group_id']).to eq('123') - expect(last_payload[:metadata]['meta.search.project_id']).to eq('456') - expect(last_payload[:metadata]).not_to have_key('meta.search.search') - expect(last_payload[:metadata]['meta.search.scope']).to eq('issues') - expect(last_payload[:metadata]['meta.search.force_search_results']).to eq('true') - expect(last_payload[:metadata]['meta.search.filters.confidential']).to eq('true') - expect(last_payload[:metadata]['meta.search.filters.state']).to eq('true') + expect(response).to have_gitlab_http_status(:ok) + expect(doc.css('OpenSearchDescription ShortName').text).to eq('GitLab') + expect(doc.css('OpenSearchDescription *').map(&:name)).to eq(%w[ShortName Description InputEncoding Image Url SearchForm]) + end end end end diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 51cecb348c8..50d6ac8f23d 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -173,7 +173,7 @@ RSpec.describe SnippetsController do expect(response).to have_gitlab_http_status(:ok) end - it_behaves_like 'tracking unique hll events', :usage_data_i_snippets_show do + it_behaves_like 'tracking unique hll events' do subject(:request) { get :show, params: { id: public_snippet.to_param } } let(:target_id) { 'i_snippets_show' } -- cgit v1.2.3