diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-19 12:08:42 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-19 12:08:42 +0300 |
commit | b76ae638462ab0f673e5915986070518dd3f9ad3 (patch) | |
tree | bdab0533383b52873be0ec0eb4d3c66598ff8b91 /spec/controllers | |
parent | 434373eabe7b4be9593d18a585fb763f1e5f1a6f (diff) |
Add latest changes from gitlab-org/gitlab@14-2-stable-eev14.2.0-rc42
Diffstat (limited to 'spec/controllers')
44 files changed, 1369 insertions, 1064 deletions
diff --git a/spec/controllers/admin/clusters/applications_controller_spec.rb b/spec/controllers/admin/clusters/applications_controller_spec.rb deleted file mode 100644 index d1ca64d6bd2..00000000000 --- a/spec/controllers/admin/clusters/applications_controller_spec.rb +++ /dev/null @@ -1,139 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Admin::Clusters::ApplicationsController do - include AccessMatchersForController - - def current_application - Clusters::Cluster::APPLICATIONS[application] - end - - shared_examples 'a secure endpoint' do - it { expect { subject }.to be_allowed_for(:admin) } - it { expect { subject }.to be_denied_for(:user) } - it { expect { subject }.to be_denied_for(:external) } - end - - let(:cluster) { create(:cluster, :instance, :provided_by_gcp) } - - describe 'POST create' do - subject do - post :create, params: params - end - - let(:application) { 'ingress' } - let(:params) { { application: application, id: cluster.id } } - - describe 'functionality' do - let(:admin) { create(:admin) } - - before do - sign_in(admin) - end - - it 'schedule an application installation' do - expect(ClusterInstallAppWorker).to receive(:perform_async).with(application, anything).once - - expect { subject }.to change { current_application.count } - expect(response).to have_gitlab_http_status(:no_content) - expect(cluster.application_ingress).to be_scheduled - end - - context 'when cluster do not exists' do - before do - cluster.destroy! - end - - it 'return 404' do - expect { subject }.not_to change { current_application.count } - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when application is unknown' do - let(:application) { 'unkwnown-app' } - - it 'return 404' do - is_expected.to have_gitlab_http_status(:not_found) - end - end - - context 'when application is already installing' do - before do - create(:clusters_applications_ingress, :installing, cluster: cluster) - end - - it 'returns 400' do - is_expected.to have_gitlab_http_status(:bad_request) - end - end - end - - describe 'security' do - before do - allow(ClusterInstallAppWorker).to receive(:perform_async) - end - - it_behaves_like 'a secure endpoint' - end - end - - describe 'PATCH update' do - subject do - patch :update, params: params - end - - let!(:application) { create(:clusters_applications_cert_manager, :installed, cluster: cluster) } - let(:application_name) { application.name } - let(:params) { { application: application_name, id: cluster.id, email: "new-email@example.com" } } - - describe 'functionality' do - let(:admin) { create(:admin) } - - before do - sign_in(admin) - end - - context "when cluster and app exists" do - it "schedules an application update" do - expect(ClusterPatchAppWorker).to receive(:perform_async).with(application.name, anything).once - - is_expected.to have_gitlab_http_status(:no_content) - - expect(cluster.application_cert_manager).to be_scheduled - end - end - - context 'when cluster do not exists' do - before do - cluster.destroy! - end - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'when application is unknown' do - let(:application_name) { 'unkwnown-app' } - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'when application is already scheduled' do - before do - application.make_scheduled! - end - - it { is_expected.to have_gitlab_http_status(:bad_request) } - end - end - - describe 'security' do - before do - allow(ClusterPatchAppWorker).to receive(:perform_async) - end - - it_behaves_like 'a secure endpoint' - end - end -end diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index 5a68bb2749b..64ae2a95b4e 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -43,15 +43,15 @@ RSpec.describe Admin::IntegrationsController do stub_jira_integration_test allow(PropagateIntegrationWorker).to receive(:perform_async) - put :update, params: { id: integration.class.to_param, service: { url: url } } + put :update, params: { id: integration.class.to_param, service: params } end context 'valid params' do - let(:url) { 'https://jira.gitlab-example.com' } + let(:params) { { url: 'https://jira.gitlab-example.com', password: 'password' } } it 'updates the integration' do expect(response).to have_gitlab_http_status(:found) - expect(integration.reload.url).to eq(url) + expect(integration.reload).to have_attributes(params) end it 'calls to PropagateIntegrationWorker' do @@ -60,12 +60,12 @@ RSpec.describe Admin::IntegrationsController do end context 'invalid params' do - let(:url) { 'invalid' } + let(:params) { { url: 'invalid', password: 'password' } } it 'does not update the integration' do expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:edit) - expect(integration.reload.url).not_to eq(url) + expect(integration.reload).not_to have_attributes(params) end it 'does not call to PropagateIntegrationWorker' do @@ -97,4 +97,40 @@ RSpec.describe Admin::IntegrationsController do .and change { Integrations::Jira.inherit_from_id(integration.id).count }.by(-1) end end + + describe '#overrides' do + let_it_be(:instance_integration) { create(:bugzilla_integration, :instance) } + let_it_be(:non_overridden_integration) { create(:bugzilla_integration, inherit_from_id: instance_integration.id) } + let_it_be(:overridden_integration) { create(:bugzilla_integration) } + let_it_be(:overridden_other_integration) { create(:confluence_integration) } + + subject do + get :overrides, params: { id: instance_integration.class.to_param }, format: format + end + + context 'when format is JSON' do + let(:format) { :json } + + include_context 'JSON response' + + it 'returns projects with overrides', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to contain_exactly(a_hash_including('full_name' => overridden_integration.project.full_name)) + end + end + + context 'when format is HTML' do + let(:format) { :html } + + it 'renders template' do + subject + + expect(response).to render_template 'shared/integrations/overrides' + expect(assigns(:integration)).to eq(instance_integration) + end + end + end end diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index b5e7af2c012..8e57b4f03a7 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -12,42 +12,11 @@ RSpec.describe Admin::RunnersController do describe '#index' do render_views - before do - stub_feature_flags(runner_list_view_vue_ui: false) - end - it 'lists all runners' do get :index expect(response).to have_gitlab_http_status(:ok) - end - - it 'avoids N+1 queries', :request_store do - get :index - - control_count = ActiveRecord::QueryRecorder.new { get :index }.count - - create_list(:ci_runner, 5, :tagged_only) - - # 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 - # 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') - expect(response.body).to have_content('tag2') - end - - it 'paginates runners' do - stub_const("Admin::RunnersController::NUMBER_OF_RUNNERS_PER_PAGE", 1) - - create(:ci_runner) - - get :index - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:runners).count).to be(1) + expect(response).to render_template(:index) end end diff --git a/spec/controllers/admin/services_controller_spec.rb b/spec/controllers/admin/services_controller_spec.rb deleted file mode 100644 index 06ff8f0db94..00000000000 --- a/spec/controllers/admin/services_controller_spec.rb +++ /dev/null @@ -1,75 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Admin::ServicesController do - let(:admin) { create(:admin) } - - before do - sign_in(admin) - end - - describe 'GET #edit' do - let(:service) do - create(:jira_integration, :template) - end - - it 'successfully displays the template' do - get :edit, params: { id: service.id } - - expect(response).to have_gitlab_http_status(:ok) - end - - context 'when integration does not exists' do - it 'redirects to the admin application integration page' do - get :edit, params: { id: 'invalid' } - - expect(response).to redirect_to(admin_application_settings_services_path) - end - end - - context 'when instance integration exists' do - before do - create(:jira_integration, :instance) - end - - it 'redirects to the admin application integration page' do - get :edit, params: { id: service.id } - - expect(response).to redirect_to(admin_application_settings_services_path) - end - end - end - - describe "#update" do - let(:project) { create(:project) } - let!(:service_template) do - Integrations::Redmine.create!( - project: nil, - active: false, - template: true, - properties: { - project_url: 'http://abc', - issues_url: 'http://abc', - new_issue_url: 'http://abc' - } - ) - end - - it 'calls the propagation worker when service is active' do - expect(PropagateServiceTemplateWorker).to receive(:perform_async).with(service_template.id) - - put :update, params: { id: service_template.id, service: { active: true } } - - expect(response).to have_gitlab_http_status(:found) - end - - it 'does not call the propagation worker when service is not active' do - expect(PropagateServiceTemplateWorker).not_to receive(:perform_async) - - put :update, params: { id: service_template.id, service: { properties: {} } } - - expect(response).to have_gitlab_http_status(:found) - end - end -end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 6dc5c38cb76..6e172f53257 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -359,13 +359,12 @@ RSpec.describe Admin::UsersController do end end - describe 'PUT ban/:id' do + describe 'PUT ban/:id', :aggregate_failures do context 'when ban_user_feature_flag is enabled' do it 'bans user' do put :ban, params: { id: user.username } - user.reload - expect(user.banned?).to be_truthy + expect(user.reload.banned?).to be_truthy expect(flash[:notice]).to eq _('Successfully banned') end @@ -390,21 +389,19 @@ RSpec.describe Admin::UsersController do it 'does not ban user, renders 404' do put :ban, params: { id: user.username } - user.reload - expect(user.banned?).to be_falsey + expect(user.reload.banned?).to be_falsey expect(response).to have_gitlab_http_status(:not_found) end end end - describe 'PUT unban/:id' do + describe 'PUT unban/:id', :aggregate_failures do let(:banned_user) { create(:user, :banned) } it 'unbans user' do put :unban, params: { id: banned_user.username } - banned_user.reload - expect(banned_user.banned?).to be_falsey + expect(banned_user.reload.banned?).to be_falsey expect(flash[:notice]).to eq _('Successfully unbanned') end end @@ -419,6 +416,7 @@ RSpec.describe Admin::UsersController do put :unlock, params: { id: user.username } user.reload expect(user.access_locked?).to be_falsey + expect(flash[:notice]).to eq _('Successfully unlocked') end end diff --git a/spec/controllers/concerns/redis_tracking_spec.rb b/spec/controllers/concerns/redis_tracking_spec.rb index 4077f4f5cce..178684ae2d0 100644 --- a/spec/controllers/concerns/redis_tracking_spec.rb +++ b/spec/controllers/concerns/redis_tracking_spec.rb @@ -3,6 +3,8 @@ require "spec_helper" RSpec.describe RedisTracking do + include TrackingHelpers + let(:user) { create(:user) } controller(ApplicationController) do @@ -60,7 +62,7 @@ RSpec.describe RedisTracking do end it 'tracks the event if DNT is not enabled' do - request.headers['DNT'] = '0' + stub_do_not_track('0') expect_tracking @@ -68,7 +70,7 @@ RSpec.describe RedisTracking do end it 'does not track the event if DNT is enabled' do - request.headers['DNT'] = '1' + stub_do_not_track('1') expect_no_tracking diff --git a/spec/controllers/concerns/spammable_actions/akismet_mark_as_spam_action_spec.rb b/spec/controllers/concerns/spammable_actions/akismet_mark_as_spam_action_spec.rb new file mode 100644 index 00000000000..7c10dccdcb9 --- /dev/null +++ b/spec/controllers/concerns/spammable_actions/akismet_mark_as_spam_action_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SpammableActions::AkismetMarkAsSpamAction do + include AfterNextHelpers + + controller(ActionController::Base) do + include SpammableActions::AkismetMarkAsSpamAction + + private + + def spammable_path + '/fake_spammable_path' + end + end + + let(:spammable_type) { 'SpammableType' } + let(:spammable) { double(:spammable, spammable_entity_type: double(:spammable_entity_type, titlecase: spammable_type)) } + let(:current_user) { create(:admin) } + + before do + allow(Gitlab::Recaptcha).to receive(:load_configurations!) { true } + routes.draw { get 'mark_as_spam' => 'anonymous#mark_as_spam' } + allow(controller).to receive(:spammable) { spammable } + allow(controller).to receive(:current_user) { double(:current_user, admin?: admin) } + allow(controller).to receive(:current_user).and_return(current_user) + end + + describe '#mark_as_spam' do + subject { post :mark_as_spam } + + before do + expect_next(Spam::AkismetMarkAsSpamService, target: spammable) + .to receive(:execute).and_return(execute_result) + end + + context 'when user is admin', :enable_admin_mode do + let(:admin) { true } + + context 'when service returns truthy' do + let(:execute_result) { true } + + it 'redirects with notice' do + expect(subject).to redirect_to('/fake_spammable_path') + expect(subject.request.flash[:notice]).to match(/#{spammable_type}.*submitted.*successfully/) + end + end + + context 'when service returns falsey' do + let(:execute_result) { false } + + it 'redirects with notice' do + expect(subject).to redirect_to('/fake_spammable_path') + expect(subject.request.flash[:alert]).to match(/Error/) + end + end + end + + context 'when user is not admin' do + let(:admin) { false } + let(:execute_result) { true } + + it 'calls #access_denied!' do + expect(controller).to receive(:access_denied!) { false } + + subject + end + end + end +end diff --git a/spec/controllers/concerns/spammable_actions/captcha_check/html_format_actions_support_spec.rb b/spec/controllers/concerns/spammable_actions/captcha_check/html_format_actions_support_spec.rb new file mode 100644 index 00000000000..53a78326397 --- /dev/null +++ b/spec/controllers/concerns/spammable_actions/captcha_check/html_format_actions_support_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SpammableActions::CaptchaCheck::HtmlFormatActionsSupport do + controller(ActionController::Base) do + include SpammableActions::CaptchaCheck::HtmlFormatActionsSupport + + def create + with_captcha_check_html_format { render :some_rendered_view } + end + end + + let(:spammable) { double(:spammable) } + + before do + allow(Gitlab::Recaptcha).to receive(:load_configurations!) { true } + routes.draw { get 'create' => 'anonymous#create' } + allow(controller).to receive(:spammable) { spammable } + expect(spammable).to receive(:render_recaptcha?).at_least(:once) { render_recaptcha } + end + + describe '#convert_html_spam_params_to_headers' do + let(:render_recaptcha) { false } + let(:g_recaptcha_response) { 'abc123' } + let(:spam_log_id) { 42 } + + let(:params) do + { + 'g-recaptcha-response' => g_recaptcha_response, + spam_log_id: spam_log_id + } + end + + # NOTE: `:update` has an identical `before_action` behavior to ``:create``, but `before_action` is + # declarative via the ``:only`` attribute, so there's little value in re-testing the behavior. + subject { post :create, params: params } + + before do + allow(controller).to receive(:render).with(:some_rendered_view) + end + + it 'converts params to headers' do + subject + + expect(controller.request.headers['X-GitLab-Captcha-Response']).to eq(g_recaptcha_response) + expect(controller.request.headers['X-GitLab-Spam-Log-Id']).to eq(spam_log_id.to_s) + end + end + + describe '#with_captcha_check_html_format' do + subject { post :create } + + context 'when spammable.render_recaptcha? is true' do + let(:render_recaptcha) { true } + + it 'renders :captcha_check' do + expect(controller).to receive(:render).with(:captcha_check) + + subject + end + end + + context 'when spammable.render_recaptcha? is false' do + let(:render_recaptcha) { false } + + it 'yields to block' do + expect(controller).to receive(:render).with(:some_rendered_view) + + subject + end + end + end +end diff --git a/spec/controllers/concerns/spammable_actions/captcha_check/json_format_actions_support_spec.rb b/spec/controllers/concerns/spammable_actions/captcha_check/json_format_actions_support_spec.rb new file mode 100644 index 00000000000..d7a44351ad8 --- /dev/null +++ b/spec/controllers/concerns/spammable_actions/captcha_check/json_format_actions_support_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SpammableActions::CaptchaCheck::JsonFormatActionsSupport do + controller(ActionController::Base) do + include SpammableActions::CaptchaCheck::JsonFormatActionsSupport + + def some_action + with_captcha_check_json_format { render :some_rendered_view } + end + end + + before do + allow(Gitlab::Recaptcha).to receive(:load_configurations!) { true } + end + + describe '#with_captcha_check_json_format' do + subject { post :some_action } + + let(:spammable) { double(:spammable) } + + before do + routes.draw { get 'some_action' => 'anonymous#some_action' } + allow(controller).to receive(:spammable) { spammable } + expect(spammable).to receive(:render_recaptcha?).at_least(:once) { render_recaptcha } + end + + context 'when spammable.render_recaptcha? is true' do + let(:render_recaptcha) { true } + let(:spam_log) { double(:spam_log, id: 1) } + let(:spammable) { double(:spammable, spam?: true, render_recaptcha?: render_recaptcha, spam_log: spam_log) } + let(:recaptcha_site_key) { 'abc123' } + let(:spam_action_response_fields) do + { + spam: true, + needs_captcha_response: render_recaptcha, + spam_log_id: 1, + captcha_site_key: recaptcha_site_key + } + end + + it 'renders json containing spam_action_response_fields' do + expect(controller).to receive(:render).with(json: spam_action_response_fields, status: :conflict) + allow(Gitlab::CurrentSettings).to receive(:recaptcha_site_key) { recaptcha_site_key } + subject + end + end + + context 'when spammable.render_recaptcha? is false' do + let(:render_recaptcha) { false } + + it 'yields to block' do + expect(controller).to receive(:render).with(:some_rendered_view) + + subject + end + end + end +end diff --git a/spec/controllers/concerns/spammable_actions_spec.rb b/spec/controllers/concerns/spammable_actions_spec.rb deleted file mode 100644 index 7bd5a76e60c..00000000000 --- a/spec/controllers/concerns/spammable_actions_spec.rb +++ /dev/null @@ -1,112 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe SpammableActions do - controller(ActionController::Base) do - include SpammableActions - - # #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' - - recaptcha_check_with_fallback(should_redirect) { render json: :ok } - end - - private - - def spammable_path - '/fake_spammable_path' - end - end - - before do - allow(Gitlab::Recaptcha).to receive(:load_configurations!) { true } - end - - describe '#recaptcha_check_with_fallback' do - shared_examples 'yields to block' do - it do - subject - - expect(json_response).to eq({ json: 'ok' }) - end - end - - let(:format) { :html } - - subject { post :update, format: format, params: params } - - let(:spammable) { double(:spammable) } - let(:should_redirect) { nil } - let(:params) do - { - should_redirect: should_redirect - } - end - - before do - routes.draw { get 'update' => 'anonymous#update' } - allow(controller).to receive(:spammable) { spammable } - end - - context 'when should_redirect is true and spammable is valid' do - let(:should_redirect) { true } - - before do - allow(spammable).to receive(:valid?) { true } - end - - it 'redirects to spammable_path' do - expect(subject).to redirect_to('/fake_spammable_path') - end - end - - context 'when should_redirect is false or spammable is not valid' do - before do - allow(spammable).to receive(:valid?) { false } - end - - context 'when spammable.render_recaptcha? is true' do - let(:spam_log) { instance_double(SpamLog, id: 123) } - let(:captcha_site_key) { 'abc123' } - - before do - expect(spammable).to receive(:render_recaptcha?).at_least(:once) { true } - end - - context 'when format is :html' do - it 'renders :verify' do - expect(controller).to receive(:render).with(:verify) - - subject - end - end - - context 'when format is :json' do - let(:format) { :json } - - before do - expect(spammable).to receive(:spam?) { false } - expect(spammable).to receive(:spam_log) { spam_log } - expect(Gitlab::CurrentSettings).to receive(:recaptcha_site_key) { captcha_site_key } - end - - it 'renders json with spam_action_response_fields' do - subject - - expected_json_response = HashWithIndifferentAccess.new( - { - spam: false, - needs_captcha_response: true, - spam_log_id: spam_log.id, - captcha_site_key: captcha_site_key - }) - expect(json_response).to eq(expected_json_response) - end - end - end - end - end -end diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index 0d9bd146778..9b13025cbe3 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -179,7 +179,7 @@ RSpec.describe Dashboard::ProjectsController, :aggregate_failures do expect(response).to render_template('dashboard/projects/index') expect(response.body).to include( "pushed to project", - "uploaded design #{design.to_reference}", + "added design #{design.to_reference}", "created wiki page #{wiki_page.title}", "joined project #{project.full_name}", "closed issue #{issue.to_reference}" diff --git a/spec/controllers/groups/clusters/applications_controller_spec.rb b/spec/controllers/groups/clusters/applications_controller_spec.rb deleted file mode 100644 index 5629e86c928..00000000000 --- a/spec/controllers/groups/clusters/applications_controller_spec.rb +++ /dev/null @@ -1,148 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Groups::Clusters::ApplicationsController do - include AccessMatchersForController - - def current_application - Clusters::Cluster::APPLICATIONS[application] - end - - shared_examples 'a secure endpoint' do - it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { expect { subject }.to be_allowed_for(:admin) } - it('is denied for admin when admin mode is disabled') { expect { subject }.to be_denied_for(:admin) } - it { expect { subject }.to be_allowed_for(:owner).of(group) } - it { expect { subject }.to be_allowed_for(:maintainer).of(group) } - it { expect { subject }.to be_denied_for(:developer).of(group) } - it { expect { subject }.to be_denied_for(:reporter).of(group) } - it { expect { subject }.to be_denied_for(:guest).of(group) } - it { expect { subject }.to be_denied_for(:user) } - it { expect { subject }.to be_denied_for(:external) } - end - - let(:cluster) { create(:cluster, :group, :provided_by_gcp) } - let(:group) { cluster.group } - - describe 'POST create' do - subject do - post :create, params: params.merge(group_id: group) - end - - let(:application) { 'ingress' } - let(:params) { { application: application, id: cluster.id } } - - describe 'functionality' do - let(:user) { create(:user) } - - before do - group.add_maintainer(user) - sign_in(user) - end - - it 'schedule an application installation' do - expect(ClusterInstallAppWorker).to receive(:perform_async).with(application, anything).once - - expect { subject }.to change { current_application.count } - expect(response).to have_gitlab_http_status(:no_content) - expect(cluster.application_ingress).to be_scheduled - end - - context 'when cluster do not exists' do - before do - cluster.destroy! - end - - it 'return 404' do - expect { subject }.not_to change { current_application.count } - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when application is unknown' do - let(:application) { 'unkwnown-app' } - - it 'return 404' do - is_expected.to have_gitlab_http_status(:not_found) - end - end - - context 'when application is already installing' do - before do - create(:clusters_applications_ingress, :installing, cluster: cluster) - end - - it 'returns 400' do - is_expected.to have_gitlab_http_status(:bad_request) - end - end - end - - describe 'security' do - before do - allow(ClusterInstallAppWorker).to receive(:perform_async) - end - - it_behaves_like 'a secure endpoint' - end - end - - describe 'PATCH update' do - subject do - patch :update, params: params.merge(group_id: group) - end - - let!(:application) { create(:clusters_applications_cert_manager, :installed, cluster: cluster) } - let(:application_name) { application.name } - let(:params) { { application: application_name, id: cluster.id, email: "new-email@example.com" } } - - describe 'functionality' do - let(:user) { create(:user) } - - before do - group.add_maintainer(user) - sign_in(user) - end - - context "when cluster and app exists" do - it "schedules an application update" do - expect(ClusterPatchAppWorker).to receive(:perform_async).with(application.name, anything).once - - is_expected.to have_gitlab_http_status(:no_content) - - expect(cluster.application_cert_manager).to be_scheduled - end - end - - context 'when cluster do not exists' do - before do - cluster.destroy! - end - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'when application is unknown' do - let(:application_name) { 'unkwnown-app' } - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'when application is already scheduled' do - before do - application.make_scheduled! - end - - it { is_expected.to have_gitlab_http_status(:bad_request) } - end - end - - describe 'security' do - before do - allow(ClusterPatchAppWorker).to receive(:perform_async) - end - - it_behaves_like 'a secure endpoint' - end - end -end diff --git a/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb b/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb index f67b2022219..50e19d5b482 100644 --- a/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb +++ b/spec/controllers/groups/dependency_proxy_auth_controller_spec.rb @@ -30,16 +30,31 @@ RSpec.describe Groups::DependencyProxyAuthController do end context 'with valid JWT' do - let_it_be(:user) { create(:user) } + context 'user' do + let_it_be(:user) { create(:user) } - let(:jwt) { build_jwt(user) } - let(:token_header) { "Bearer #{jwt.encoded}" } + let(:jwt) { build_jwt(user) } + let(:token_header) { "Bearer #{jwt.encoded}" } - before do - request.headers['HTTP_AUTHORIZATION'] = token_header + before do + request.headers['HTTP_AUTHORIZATION'] = token_header + end + + it { is_expected.to have_gitlab_http_status(:success) } end - it { is_expected.to have_gitlab_http_status(:success) } + context 'deploy token' do + let_it_be(:user) { create(:deploy_token) } + + let(:jwt) { build_jwt(user) } + let(:token_header) { "Bearer #{jwt.encoded}" } + + before do + request.headers['HTTP_AUTHORIZATION'] = token_header + end + + it { is_expected.to have_gitlab_http_status(:success) } + end end context 'with invalid JWT' do @@ -51,7 +66,7 @@ RSpec.describe Groups::DependencyProxyAuthController do request.headers['HTTP_AUTHORIZATION'] = token_header end - it { is_expected.to have_gitlab_http_status(:not_found) } + it { is_expected.to have_gitlab_http_status(:unauthorized) } end context 'token with no user id' do @@ -61,7 +76,7 @@ RSpec.describe Groups::DependencyProxyAuthController do request.headers['HTTP_AUTHORIZATION'] = token_header end - it { is_expected.to have_gitlab_http_status(:not_found) } + it { is_expected.to have_gitlab_http_status(:unauthorized) } end context 'expired token' do @@ -76,6 +91,32 @@ RSpec.describe Groups::DependencyProxyAuthController do it { is_expected.to have_gitlab_http_status(:unauthorized) } end + + context 'expired deploy token' do + let_it_be(:user) { create(:deploy_token, :expired) } + + let(:jwt) { build_jwt(user) } + let(:token_header) { "Bearer #{jwt.encoded}" } + + before do + request.headers['HTTP_AUTHORIZATION'] = token_header + end + + it { is_expected.to have_gitlab_http_status(:unauthorized) } + end + + context 'revoked deploy token' do + let_it_be(:user) { create(:deploy_token, :revoked) } + + let(:jwt) { build_jwt(user) } + let(:token_header) { "Bearer #{jwt.encoded}" } + + before do + request.headers['HTTP_AUTHORIZATION'] = token_header + end + + it { is_expected.to have_gitlab_http_status(:unauthorized) } + end end end end diff --git a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb index 9f30a850ca2..7415c2860c8 100644 --- a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb +++ b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb @@ -7,11 +7,12 @@ RSpec.describe Groups::DependencyProxyForContainersController do include DependencyProxyHelpers let_it_be(:user) { create(:user) } + let_it_be_with_reload(:group) { create(:group, :private) } - let(:group) { create(:group) } let(:token_response) { { status: :success, token: 'abcd1234' } } let(:jwt) { build_jwt(user) } let(:token_header) { "Bearer #{jwt.encoded}" } + let(:snowplow_gitlab_standard_context) { { namespace: group, user: user } } shared_examples 'without a token' do before do @@ -19,6 +20,8 @@ RSpec.describe Groups::DependencyProxyForContainersController do end context 'feature flag disabled' do + let_it_be(:group) { create(:group) } + before do stub_feature_flags(dependency_proxy_for_private_groups: false) end @@ -34,13 +37,12 @@ RSpec.describe Groups::DependencyProxyForContainersController do stub_feature_flags(dependency_proxy_for_private_groups: false) end - it 'redirects', :aggregate_failures do + it 'returns not found' do group.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) subject - expect(response).to have_gitlab_http_status(:redirect) - expect(response.location).to end_with(new_user_session_path) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -52,21 +54,52 @@ RSpec.describe Groups::DependencyProxyForContainersController do request.headers['HTTP_AUTHORIZATION'] = token_header end - it { is_expected.to have_gitlab_http_status(:not_found) } + it { is_expected.to have_gitlab_http_status(:unauthorized) } end context 'with valid user that does not have access' do - let(:group) { create(:group, :private) } - before do - user = double('bad_user', id: 999) - token_header = "Bearer #{build_jwt(user).encoded}" request.headers['HTTP_AUTHORIZATION'] = token_header end it { is_expected.to have_gitlab_http_status(:not_found) } end + context 'with deploy token from a different group,' do + let_it_be(:user) { create(:deploy_token, :group, :dependency_proxy_scopes) } + + it { is_expected.to have_gitlab_http_status(:not_found) } + end + + context 'with revoked deploy token' do + let_it_be(:user) { create(:deploy_token, :revoked, :group, :dependency_proxy_scopes) } + let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: user, group: group) } + + it { is_expected.to have_gitlab_http_status(:unauthorized) } + end + + context 'with expired deploy token' do + let_it_be(:user) { create(:deploy_token, :expired, :group, :dependency_proxy_scopes) } + let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: user, group: group) } + + it { is_expected.to have_gitlab_http_status(:unauthorized) } + end + + context 'with deploy token with insufficient scopes' do + let_it_be(:user) { create(:deploy_token, :group) } + let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: user, group: group) } + + it { is_expected.to have_gitlab_http_status(:not_found) } + end + + context 'when a group is not found' do + before do + expect(Group).to receive(:find_by_full_path).and_return(nil) + end + + it { is_expected.to have_gitlab_http_status(:not_found) } + end + context 'when user is not found' do before do allow(User).to receive(:find).and_return(nil) @@ -104,7 +137,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 } } + let(:pull_response) { { status: :success, manifest: manifest, from_cache: false } } before do allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance| @@ -132,6 +165,10 @@ RSpec.describe Groups::DependencyProxyForContainersController do } end + before do + group.add_guest(user) + end + it 'proxies status from the remote token request', :aggregate_failures do subject @@ -149,6 +186,10 @@ RSpec.describe Groups::DependencyProxyForContainersController do } end + before do + group.add_guest(user) + end + it 'proxies status from the remote manifest request', :aggregate_failures do subject @@ -157,21 +198,39 @@ RSpec.describe Groups::DependencyProxyForContainersController do end end - it 'sends a file' do - expect(controller).to receive(:send_file).with(manifest.file.path, type: manifest.content_type) + context 'a valid user' do + before do + group.add_guest(user) + end - subject + it_behaves_like 'a successful manifest pull' + it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest' + + context 'with a cache entry' do + let(:pull_response) { { status: :success, manifest: manifest, from_cache: true } } + + it_behaves_like 'returning response status', :success + it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest_from_cache' + end end - it 'returns Content-Disposition: attachment' do - subject + context 'a valid deploy token' do + let_it_be(:user) { create(:deploy_token, :dependency_proxy_scopes, :group) } + let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: user, group: group) } + + it_behaves_like 'a successful manifest pull' + + context 'pulling from a subgroup' do + let_it_be_with_reload(:parent_group) { create(:group) } + let_it_be_with_reload(:group) { create(:group, parent: parent_group) } + + before do + parent_group.create_dependency_proxy_setting!(enabled: true) + group_deploy_token.update_column(:group_id, parent_group.id) + end - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['Docker-Content-Digest']).to eq(manifest.digest) - expect(response.headers['Content-Length']).to eq(manifest.size) - expect(response.headers['Docker-Distribution-Api-Version']).to eq(DependencyProxy::DISTRIBUTION_API_VERSION) - expect(response.headers['Etag']).to eq("\"#{manifest.digest}\"") - expect(response.headers['Content-Disposition']).to match(/^attachment/) + it_behaves_like 'a successful manifest pull' + end end end @@ -186,7 +245,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do let_it_be(:blob) { create(:dependency_proxy_blob) } let(:blob_sha) { blob.file_name.sub('.gz', '') } - let(:blob_response) { { status: :success, blob: blob } } + let(:blob_response) { { status: :success, blob: blob, from_cache: false } } before do allow_next_instance_of(DependencyProxy::FindOrCreateBlobService) do |instance| @@ -214,6 +273,10 @@ RSpec.describe Groups::DependencyProxyForContainersController do } end + before do + group.add_guest(user) + end + it 'proxies status from the remote blob request', :aggregate_failures do subject @@ -222,17 +285,39 @@ RSpec.describe Groups::DependencyProxyForContainersController do end end - it 'sends a file' do - expect(controller).to receive(:send_file).with(blob.file.path, {}) + context 'a valid user' do + before do + group.add_guest(user) + end - subject + it_behaves_like 'a successful blob pull' + it_behaves_like 'a package tracking event', described_class.name, 'pull_blob' + + context 'with a cache entry' do + let(:blob_response) { { status: :success, blob: blob, from_cache: true } } + + it_behaves_like 'returning response status', :success + it_behaves_like 'a package tracking event', described_class.name, 'pull_blob_from_cache' + end end - it 'returns Content-Disposition: attachment', :aggregate_failures do - subject + context 'a valid deploy token' do + let_it_be(:user) { create(:deploy_token, :group, :dependency_proxy_scopes) } + let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: user, group: group) } + + it_behaves_like 'a successful blob pull' - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['Content-Disposition']).to match(/^attachment/) + context 'pulling from a subgroup' do + let_it_be_with_reload(:parent_group) { create(:group) } + let_it_be_with_reload(:group) { create(:group, parent: parent_group) } + + before do + parent_group.create_dependency_proxy_setting!(enabled: true) + group_deploy_token.update_column(:group_id, parent_group.id) + end + + it_behaves_like 'a successful blob pull' + end end end diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb index 2f1c6c813cf..1808969cd60 100644 --- a/spec/controllers/groups/runners_controller_spec.rb +++ b/spec/controllers/groups/runners_controller_spec.rb @@ -15,6 +15,33 @@ RSpec.describe Groups::RunnersController do sign_in(user) end + describe '#index' do + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'renders show with 200 status code' do + get :index, params: { group_id: group } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index) + 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 '#show' do context 'when user is owner' do before do diff --git a/spec/controllers/groups/settings/integrations_controller_spec.rb b/spec/controllers/groups/settings/integrations_controller_spec.rb index ef8f9f69710..931e726850a 100644 --- a/spec/controllers/groups/settings/integrations_controller_spec.rb +++ b/spec/controllers/groups/settings/integrations_controller_spec.rb @@ -69,25 +69,25 @@ RSpec.describe Groups::Settings::IntegrationsController do group.add_owner(user) stub_jira_integration_test - put :update, params: { group_id: group, id: integration.class.to_param, service: { url: url } } + put :update, params: { group_id: group, id: integration.class.to_param, service: params } end context 'valid params' do - let(:url) { 'https://jira.gitlab-example.com' } + let(:params) { { url: 'https://jira.gitlab-example.com', password: 'password' } } it 'updates the integration' do expect(response).to have_gitlab_http_status(:found) - expect(integration.reload.url).to eq(url) + expect(integration.reload).to have_attributes(params) end end context 'invalid params' do - let(:url) { 'invalid' } + let(:params) { { url: 'invalid', password: 'password' } } it 'does not update the integration' do expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:edit) - expect(integration.reload.url).not_to eq(url) + expect(integration.reload).not_to have_attributes(params) end end end diff --git a/spec/controllers/import/available_namespaces_controller_spec.rb b/spec/controllers/import/available_namespaces_controller_spec.rb index ebccc862a13..0f98d649338 100644 --- a/spec/controllers/import/available_namespaces_controller_spec.rb +++ b/spec/controllers/import/available_namespaces_controller_spec.rb @@ -4,26 +4,94 @@ require 'spec_helper' RSpec.describe Import::AvailableNamespacesController do let_it_be(:user) { create(:user) } - let_it_be(:manageable_groups) { [create(:group), create(:group)] } before do sign_in(user) - manageable_groups.each { |group| group.add_maintainer(user) } end describe "GET index" do - it "returns list of available namespaces" do - unrelated_group = create(:group) + context "when having group with role never allowed to create projects" do + using RSpec::Parameterized::TableSyntax - get :index + where( + role: [:guest, :reporter], + default_project_creation_access: [::Gitlab::Access::MAINTAINER_PROJECT_ACCESS, ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS], + group_project_creation_level: [nil, ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS, ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS]) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_kind_of(Array) + with_them do + before do + stub_application_setting(default_project_creation: default_project_creation_access) + end - response_ids = json_response.map { |n| n["id"] } + it "does not include group with access level #{params[:role]} in list" do + group = create(:group, project_creation_level: group_project_creation_level) + group.add_user(user, role) + get :index - expect(response_ids).not_to include(unrelated_group.id) - expect(response_ids).to contain_exactly(*manageable_groups.map(&:id)) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).not_to include({ + 'id' => group.id, + 'full_path' => group.full_path + }) + end + end + end + + context "when having group with role always allowed to create projects" do + using RSpec::Parameterized::TableSyntax + + where( + role: [:maintainer, :owner], + default_project_creation_access: [::Gitlab::Access::MAINTAINER_PROJECT_ACCESS, ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS], + group_project_creation_level: [nil, ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS, ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS]) + + with_them do + before do + stub_application_setting(default_project_creation: default_project_creation_access) + end + + it "does not include group with access level #{params[:role]} in list" do + group = create(:group, project_creation_level: group_project_creation_level) + group.add_user(user, role) + get :index + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include({ + 'id' => group.id, + 'full_path' => group.full_path + }) + end + end + end + + context "when having developer role" do + using RSpec::Parameterized::TableSyntax + + where(:default_project_creation_access, :project_creation_level, :is_visible) do + ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS | nil | false + ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS | ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS | true + ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS | nil | true + ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS | ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS | false + end + + with_them do + before do + stub_application_setting(default_project_creation: default_project_creation_access) + end + + it "#{params[:is_visible] ? 'includes' : 'does not include'} group with access level #{params[:role]} in list" do + group = create(:group, project_creation_level: project_creation_level) + group.add_user(user, :developer) + + get :index + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).send(is_visible ? 'to' : 'not_to', include({ + 'id' => group.id, + 'full_path' => group.full_path + })) + end + end end context "with an anonymous user" do diff --git a/spec/controllers/import/manifest_controller_spec.rb b/spec/controllers/import/manifest_controller_spec.rb index 6b21b45e698..d5a498e80d9 100644 --- a/spec/controllers/import/manifest_controller_spec.rb +++ b/spec/controllers/import/manifest_controller_spec.rb @@ -74,7 +74,6 @@ RSpec.describe Import::ManifestController, :clean_gitlab_redis_shared_state do expect(json_response.dig("imported_projects", 0, "id")).to eq(project.id) expect(json_response.dig("provider_repos", 0, "id")).to eq(repo1[:id]) expect(json_response.dig("provider_repos", 1, "id")).to eq(repo2[:id]) - expect(json_response.dig("namespaces", 0, "id")).to eq(group.id) end it "does not show already added project" do diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index fd7631edbbb..dc1fb0454df 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -71,7 +71,7 @@ RSpec.describe InvitesController do end end - describe 'GET #show' do + describe 'GET #show', :snowplow do subject(:request) { get :show, params: params } context 'when logged in' do @@ -83,34 +83,75 @@ RSpec.describe InvitesController do it_behaves_like 'invalid token' end - context 'when it is part of our invite email experiment' do + context 'when it is an initial invite email' do let(:extra_params) { { invite_type: 'initial_email' } } - it 'tracks the experiment' do - experiment = double(track: true) - allow(controller).to receive(:experiment).with('members/invite_email', actor: member).and_return(experiment) - + it 'tracks the initial join click from email' do request - expect(experiment).to have_received(:track).with(:join_clicked) + expect_snowplow_event( + category: described_class.name, + action: 'join_clicked', + label: 'invite_email', + property: member.id.to_s + ) + end + + context 'when it is part of the invite_email_preview_text experiment' do + let(:extra_params) { { invite_type: 'initial_email', experiment_name: 'invite_email_preview_text' } } + + it 'tracks the initial join click from email' do + experiment = double(track: true) + allow(controller).to receive(:experiment).with(:invite_email_preview_text, actor: member).and_return(experiment) + + request + + expect(experiment).to have_received(:track).with(:join_clicked) + end + + context 'when member does not exist' do + let(:raw_invite_token) { '_bogus_token_' } + + it 'does not track the experiment' do + expect(controller).not_to receive(:experiment).with(:invite_email_preview_text, actor: member) + + request + end + end end context 'when member does not exist' do let(:raw_invite_token) { '_bogus_token_' } - it 'does not track the experiment' do - expect(controller).not_to receive(:experiment).with('members/invite_email', actor: member) - + it 'does not track join click' do request + + expect_no_snowplow_event( + category: described_class.name, + action: 'join_clicked', + label: 'invite_email' + ) end end end - context 'when it is not part of our invite email experiment' do - it 'does not track via experiment' do - expect(controller).not_to receive(:experiment).with('members/invite_email', actor: member) - + context 'when it is not an initial email' do + it 'does not track the join click' do request + + expect_no_snowplow_event( + category: described_class.name, + action: 'join_clicked', + label: 'invite_email' + ) + end + + context 'when it is not part of our invite email experiment' do + it 'does not track via experiment' do + expect(controller).not_to receive(:experiment).with(:invite_email_preview_text, actor: member) + + request + end end end diff --git a/spec/controllers/jira_connect/app_descriptor_controller_spec.rb b/spec/controllers/jira_connect/app_descriptor_controller_spec.rb index 55bafa938a7..98f4db13a1d 100644 --- a/spec/controllers/jira_connect/app_descriptor_controller_spec.rb +++ b/spec/controllers/jira_connect/app_descriptor_controller_spec.rb @@ -4,20 +4,87 @@ require 'spec_helper' RSpec.describe JiraConnect::AppDescriptorController do describe '#show' do + let(:descriptor) do + json_response.deep_symbolize_keys + end + + let(:logo_url) { %r{\Ahttp://test\.host/assets/gitlab_logo-\h+\.png\z} } + + let(:common_module_properties) do + { + homeUrl: 'https://gitlab.com', + logoUrl: logo_url, + documentationUrl: 'https://docs.gitlab.com/ee/integration/jira/' + } + end + it 'returns JSON app descriptor' do get :show expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include( - 'baseUrl' => 'https://test.host/-/jira_connect', - 'lifecycle' => { - 'installed' => '/events/installed', - 'uninstalled' => '/events/uninstalled' + + expect(descriptor).to include( + name: Atlassian::JiraConnect.app_name, + description: kind_of(String), + key: Atlassian::JiraConnect.app_key, + baseUrl: 'https://test.host/-/jira_connect', + lifecycle: { + installed: '/events/installed', + uninstalled: '/events/uninstalled' + }, + vendor: { + name: 'GitLab', + url: 'https://gitlab.com' }, - 'links' => { - 'documentation' => 'http://test.host/help/integration/jira_development_panel#gitlabcom-1' + links: { + documentation: 'http://test.host/help/integration/jira_development_panel#gitlabcom-1' + }, + authentication: { + type: 'jwt' + }, + scopes: %w(READ WRITE DELETE), + apiVersion: 1, + apiMigrations: { + 'context-qsh': true, + gdpr: true } ) + + expect(descriptor[:modules]).to include( + postInstallPage: { + key: 'gitlab-configuration', + name: { value: 'GitLab Configuration' }, + url: '/subscriptions' + }, + jiraDevelopmentTool: { + actions: { + createBranch: { + templateUrl: 'http://test.host/-/jira_connect/branches/new?issue_key={issue.key}&issue_summary={issue.summary}' + } + }, + key: 'gitlab-development-tool', + application: { value: 'GitLab' }, + name: { value: 'GitLab' }, + url: 'https://gitlab.com', + logoUrl: logo_url, + capabilities: %w(branch commit pull_request) + }, + jiraBuildInfoProvider: common_module_properties.merge( + actions: {}, + name: { value: 'GitLab CI' }, + key: 'gitlab-ci' + ), + jiraDeploymentInfoProvider: common_module_properties.merge( + actions: {}, + name: { value: 'GitLab Deployments' }, + key: 'gitlab-deployments' + ), + jiraFeatureFlagInfoProvider: common_module_properties.merge( + actions: {}, + name: { value: 'GitLab Feature Flags' }, + key: 'gitlab-feature-flags' + ) + ) end end end diff --git a/spec/controllers/jira_connect/branches_controller_spec.rb b/spec/controllers/jira_connect/branches_controller_spec.rb new file mode 100644 index 00000000000..45daf3b5309 --- /dev/null +++ b/spec/controllers/jira_connect/branches_controller_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::BranchesController do + describe '#new' do + context 'when logged in' do + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'assigns the suggested branch name' do + get :new, params: { issue_key: 'ACME-123', issue_summary: 'My Issue !@#$%' } + + expect(response).to be_successful + expect(assigns(:new_branch_data)).to include( + initial_branch_name: 'ACME-123-my-issue', + success_state_svg_path: start_with('/assets/illustrations/merge_requests-') + ) + end + + it 'ignores missing summary' do + get :new, params: { issue_key: 'ACME-123' } + + expect(response).to be_successful + expect(assigns(:new_branch_data)).to include(initial_branch_name: 'ACME-123') + end + + it 'does not set a branch name if key is not passed' do + get :new, params: { issue_summary: 'My issue' } + + expect(response).to be_successful + expect(assigns(:new_branch_data)).to include('initial_branch_name': nil) + end + end + + context 'when not logged in' do + it 'redirects to the login page' do + get :new + + expect(response).to redirect_to(new_user_session_path) + end + end + end +end diff --git a/spec/controllers/jira_connect/subscriptions_controller_spec.rb b/spec/controllers/jira_connect/subscriptions_controller_spec.rb index 95b359a989a..e32915d55a1 100644 --- a/spec/controllers/jira_connect/subscriptions_controller_spec.rb +++ b/spec/controllers/jira_connect/subscriptions_controller_spec.rb @@ -7,9 +7,13 @@ RSpec.describe JiraConnect::SubscriptionsController do describe '#index' do before do + request.headers['Accept'] = content_type + get :index, params: { jwt: jwt } end + let(:content_type) { 'text/html' } + context 'without JWT' do let(:jwt) { nil } @@ -29,13 +33,55 @@ RSpec.describe JiraConnect::SubscriptionsController do it 'removes X-Frame-Options to allow rendering in iframe' do expect(response.headers['X-Frame-Options']).to be_nil end + + context 'with JSON format' do + let_it_be(:subscription) { create(:jira_connect_subscription, installation: installation) } + + let(:content_type) { 'application/json' } + + it 'renders the relevant data as JSON', :aggregate_failures do + expect(json_response).to include('groups_path' => api_v4_groups_path(params: { min_access_level: Gitlab::Access::MAINTAINER, skip_groups: [subscription.namespace_id] })) + expect(json_response).to include( + 'subscriptions' => [ + 'group' => { + 'name' => subscription.namespace.name, + 'avatar_url' => subscription.namespace.avatar_url, + 'full_name' => subscription.namespace.full_name, + 'description' => subscription.namespace.description + }, + 'created_at' => subscription.created_at.iso8601(3), + 'unlink_path' => jira_connect_subscription_path(subscription) + ] + ) + expect(json_response).to include('subscriptions_path' => jira_connect_subscriptions_path) + end + + context 'when not signed in to GitLab' do + it 'contains a login path' do + expect(json_response).to include('login_path' => jira_connect_users_path) + end + end + + context 'when signed in to GitLab' do + let(:user) { create(:user) } + + before do + sign_in(user) + + get :index, params: { jwt: jwt } + end + + it 'does not contain a login path' do + expect(json_response).to include('login_path' => nil) + end + end + end end end describe '#create' do let(:group) { create(:group) } let(:user) { create(:user) } - let(:current_user) { user } before do group.add_maintainer(user) diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb index 1ebf4363ba6..36ec36fb6f1 100644 --- a/spec/controllers/profiles/notifications_controller_spec.rb +++ b/spec/controllers/profiles/notifications_controller_spec.rb @@ -162,7 +162,7 @@ RSpec.describe Profiles::NotificationsController do it 'shows an error message if the params are invalid' do sign_in(user) - put :update, params: { user: { notification_email: '' } } + put :update, params: { user: { notification_email: 'unverified@example.com' } } expect(user.reload.notification_email).to eq('original@example.com') expect(controller).to set_flash[:alert].to('Failed to save new settings') diff --git a/spec/controllers/profiles/two_factor_auths_controller_spec.rb b/spec/controllers/profiles/two_factor_auths_controller_spec.rb index 59eb33f4bc6..818bf2a4ae6 100644 --- a/spec/controllers/profiles/two_factor_auths_controller_spec.rb +++ b/spec/controllers/profiles/two_factor_auths_controller_spec.rb @@ -70,6 +70,12 @@ RSpec.describe Profiles::TwoFactorAuthsController do go end + it 'dismisses the `ACCOUNT_RECOVERY_REGULAR_CHECK` callout' do + expect(controller.helpers).to receive(:dismiss_account_recovery_regular_check) + + go + end + it 'renders create' do go expect(response).to render_template(:create) @@ -117,6 +123,12 @@ RSpec.describe Profiles::TwoFactorAuthsController do user.reload expect(user.otp_backup_codes).not_to be_empty end + + it 'dismisses the `ACCOUNT_RECOVERY_REGULAR_CHECK` callout' do + expect(controller.helpers).to receive(:dismiss_account_recovery_regular_check) + + post :codes + end end describe 'DELETE destroy' do diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index 37a633afab4..b4019643baf 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -110,6 +110,17 @@ RSpec.describe ProfilesController, :request_store do expect(user.reload.pronouns).to eq(pronouns) expect(response).to have_gitlab_http_status(:found) end + + it 'allows updating user specified pronunciation', :aggregate_failures do + user = create(:user, name: 'Example') + pronunciation = 'uhg-zaam-pl' + sign_in(user) + + put :update, params: { user: { pronunciation: pronunciation } } + + expect(user.reload.pronunciation).to eq(pronunciation) + expect(response).to have_gitlab_http_status(:found) + end end describe 'GET audit_log' do diff --git a/spec/controllers/projects/analytics/cycle_analytics/stages_controller_spec.rb b/spec/controllers/projects/analytics/cycle_analytics/stages_controller_spec.rb index 3bb841c7c9f..1351ba35a71 100644 --- a/spec/controllers/projects/analytics/cycle_analytics/stages_controller_spec.rb +++ b/spec/controllers/projects/analytics/cycle_analytics/stages_controller_spec.rb @@ -7,26 +7,58 @@ RSpec.describe Projects::Analytics::CycleAnalytics::StagesController do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, group: group) } - let(:params) { { namespace_id: group, project_id: project, value_stream_id: 'default' } } + let(:params) do + { + namespace_id: group, + project_id: project, + value_stream_id: Analytics::CycleAnalytics::Stages::BaseService::DEFAULT_VALUE_STREAM_NAME + } + end before do sign_in(user) end - describe 'GET index' do - context 'when user is member of the project' do + shared_examples 'project-level value stream analytics endpoint' do + before do + project.add_developer(user) + end + + it 'succeeds' do + get action, params: params + + expect(response).to have_gitlab_http_status(:ok) + end + end + + shared_examples 'project-level value stream analytics request error examples' do + context 'when invalid value stream id is given' do before do - project.add_developer(user) + params[:value_stream_id] = 1 end - it 'succeeds' do - get :index, params: params + it 'renders 404' do + get action, params: params - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:not_found) end + end + context 'when user is not member of the project' do + it 'renders 404' do + get action, params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'GET index' do + let(:action) { :index } + + it_behaves_like 'project-level value stream analytics endpoint' do it 'exposes the default stages' do - get :index, params: params + get action, params: params expect(json_response['stages'].size).to eq(Gitlab::Analytics::CycleAnalytics::DefaultStages.all.size) end @@ -37,31 +69,109 @@ RSpec.describe Projects::Analytics::CycleAnalytics::StagesController do expect(list_service).to receive(:allowed?).and_return(false) end - get :index, params: params + get action, params: params expect(response).to have_gitlab_http_status(:forbidden) end end end - context 'when invalid value stream id is given' do - before do - params[:value_stream_id] = 1 + it_behaves_like 'project-level value stream analytics request error examples' + end + + describe 'GET median' do + let(:action) { :median } + + before do + params[:id] = 'issue' + end + + it_behaves_like 'project-level value stream analytics endpoint' do + it 'returns the median' do + result = 2 + + expect_next_instance_of(Gitlab::Analytics::CycleAnalytics::Median) do |instance| + expect(instance).to receive(:seconds).and_return(result) + end + + get action, params: params + + expect(json_response['value']).to eq(result) end + end - it 'renders 404' do - get :index, params: params + it_behaves_like 'project-level value stream analytics request error examples' + end - expect(response).to have_gitlab_http_status(:not_found) + describe 'GET average' do + let(:action) { :average } + + before do + params[:id] = 'issue' + end + + it_behaves_like 'project-level value stream analytics endpoint' do + it 'returns the average' do + result = 2 + + expect_next_instance_of(Gitlab::Analytics::CycleAnalytics::Average) do |instance| + expect(instance).to receive(:seconds).and_return(result) + end + + get action, params: params + + expect(json_response['value']).to eq(result) end end - context 'when user is not member of the project' do - it 'renders 404' do - get :index, params: params + it_behaves_like 'project-level value stream analytics request error examples' + end - expect(response).to have_gitlab_http_status(:not_found) + describe 'GET count' do + let(:action) { :count } + + before do + params[:id] = 'issue' + end + + it_behaves_like 'project-level value stream analytics endpoint' do + it 'returns the count' do + count = 2 + + expect_next_instance_of(Gitlab::Analytics::CycleAnalytics::DataCollector) do |instance| + expect(instance).to receive(:count).and_return(count) + end + + get action, params: params + + expect(json_response['count']).to eq(count) end end + + it_behaves_like 'project-level value stream analytics request error examples' + end + + describe 'GET records' do + let(:action) { :records } + + before do + params[:id] = 'issue' + end + + it_behaves_like 'project-level value stream analytics endpoint' do + it 'returns the records' do + result = Issue.none.page(1) + + expect_next_instance_of(Gitlab::Analytics::CycleAnalytics::RecordsFetcher) do |instance| + expect(instance).to receive(:serialized_records).and_yield(result).and_return([]) + end + + get action, params: params + + expect(json_response).to eq([]) + end + end + + it_behaves_like 'project-level value stream analytics request error examples' end end diff --git a/spec/controllers/projects/clusters/applications_controller_spec.rb b/spec/controllers/projects/clusters/applications_controller_spec.rb deleted file mode 100644 index cc6170252c1..00000000000 --- a/spec/controllers/projects/clusters/applications_controller_spec.rb +++ /dev/null @@ -1,215 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::Clusters::ApplicationsController do - include AccessMatchersForController - - def current_application - Clusters::Cluster::APPLICATIONS[application] - end - - shared_examples 'a secure endpoint' do - it 'is allowed for admin when admin mode enabled', :enable_admin_mode do - expect { subject }.to be_allowed_for(:admin) - end - it 'is denied for admin when admin mode disabled' do - expect { subject }.to be_denied_for(:admin) - end - it { expect { subject }.to be_allowed_for(:owner).of(project) } - it { expect { subject }.to be_allowed_for(:maintainer).of(project) } - it { expect { subject }.to be_denied_for(:developer).of(project) } - it { expect { subject }.to be_denied_for(:reporter).of(project) } - it { expect { subject }.to be_denied_for(:guest).of(project) } - it { expect { subject }.to be_denied_for(:user) } - it { expect { subject }.to be_denied_for(:external) } - end - - describe 'POST create' do - subject do - post :create, params: params.merge(namespace_id: project.namespace, project_id: project) - end - - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } - let(:application) { 'ingress' } - let(:params) { { application: application, id: cluster.id } } - - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end - - it 'schedule an application installation' do - expect(ClusterInstallAppWorker).to receive(:perform_async).with(application, anything).once - - expect { subject }.to change { current_application.count } - expect(response).to have_gitlab_http_status(:no_content) - expect(cluster.application_ingress).to be_scheduled - end - - context 'when cluster do not exists' do - before do - cluster.destroy! - end - - it 'return 404' do - expect { subject }.not_to change { current_application.count } - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when application is unknown' do - let(:application) { 'unkwnown-app' } - - it 'return 404' do - is_expected.to have_gitlab_http_status(:not_found) - end - end - - context 'when application is already installing' do - before do - create(:clusters_applications_ingress, :installing, cluster: cluster) - end - - it 'returns 400' do - is_expected.to have_gitlab_http_status(:bad_request) - end - end - end - - describe 'security' do - before do - allow(ClusterInstallAppWorker).to receive(:perform_async) - end - - it_behaves_like 'a secure endpoint' - end - end - - describe 'PATCH update' do - subject do - patch :update, params: params.merge(namespace_id: project.namespace, project_id: project) - end - - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } - let!(:application) { create(:clusters_applications_knative, :installed, cluster: cluster) } - let(:application_name) { application.name } - let(:params) { { application: application_name, id: cluster.id, hostname: "new.example.com" } } - - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end - - context "when cluster and app exists" do - it "schedules an application update" do - expect(ClusterPatchAppWorker).to receive(:perform_async).with(application.name, anything).once - - is_expected.to have_gitlab_http_status(:no_content) - - expect(cluster.application_knative).to be_scheduled - end - end - - context 'when cluster do not exists' do - before do - cluster.destroy! - end - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'when application is unknown' do - let(:application_name) { 'unkwnown-app' } - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'when application is already scheduled' do - before do - application.make_scheduled! - end - - it { is_expected.to have_gitlab_http_status(:bad_request) } - end - end - - describe 'security' do - before do - allow(ClusterPatchAppWorker).to receive(:perform_async) - end - - it_behaves_like 'a secure endpoint' - end - end - - describe 'DELETE destroy' do - subject do - delete :destroy, params: params.merge(namespace_id: project.namespace, project_id: project) - end - - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } - let!(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } - let(:application_name) { application.name } - let(:params) { { application: application_name, id: cluster.id } } - let(:worker_class) { Clusters::Applications::UninstallWorker } - - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_maintainer(user) - sign_in(user) - end - - context "when cluster and app exists" do - it "schedules an application update" do - expect(worker_class).to receive(:perform_async).with(application.name, application.id).once - - is_expected.to have_gitlab_http_status(:no_content) - - expect(cluster.application_prometheus).to be_scheduled - end - end - - context 'when cluster do not exists' do - before do - cluster.destroy! - end - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'when application is unknown' do - let(:application_name) { 'unkwnown-app' } - - it { is_expected.to have_gitlab_http_status(:not_found) } - end - - context 'when application is already scheduled' do - before do - application.make_scheduled! - end - - it { is_expected.to have_gitlab_http_status(:bad_request) } - end - end - - describe 'security' do - before do - allow(worker_class).to receive(:perform_async) - end - - it_behaves_like 'a secure endpoint' - end - end -end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 4cb90edb742..7103d7df5c5 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -200,11 +200,27 @@ RSpec.describe Projects::EnvironmentsController do end describe 'PATCH #update' do - it 'responds with a 302' do - patch_params = environment_params.merge(environment: { external_url: 'https://git.gitlab.com' }) - patch :update, params: patch_params + subject { patch :update, params: params } - expect(response).to have_gitlab_http_status(:found) + context "when environment params are valid" do + let(:params) { environment_params.merge(environment: { external_url: 'https://git.gitlab.com' }) } + + it 'returns ok and the path to the newly created environment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['path']).to eq("/#{project.full_path}/-/environments/#{environment.id}") + end + end + + context "when environment params are invalid" do + let(:params) { environment_params.merge(environment: { name: '/foo/', external_url: '/git.gitlab.com' }) } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end end end @@ -786,6 +802,31 @@ RSpec.describe Projects::EnvironmentsController do end end + describe 'POST #create' do + subject { post :create, params: params } + + context "when environment params are valid" do + let(:params) { { namespace_id: project.namespace, project_id: project, environment: { name: 'foo', external_url: 'https://foo.example.com' } } } + + it 'returns ok and the path to the newly created environment' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['path']).to eq("/#{project.full_path}/-/environments/#{json_response['environment']['id']}") + end + end + + context "when environment params are invalid" do + let(:params) { { namespace_id: project.namespace, project_id: project, environment: { name: 'foo/', external_url: '/foo.example.com' } } } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + def environment_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project, diff --git a/spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb b/spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb index 7c080504c31..19b6b597a84 100644 --- a/spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb +++ b/spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb @@ -40,7 +40,7 @@ RSpec.describe Projects::ErrorTracking::StackTracesController do context 'service result is successful' do let(:service_response) { { status: :success, latest_event: error_event } } - let(:error_event) { build(:error_tracking_error_event) } + let(:error_event) { build(:error_tracking_sentry_error_event) } it 'responds with success' do expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 5ea885e4fd6..822778779eb 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -95,7 +95,7 @@ RSpec.describe Projects::ErrorTrackingController do get :index, params: params end - let(:error) { build(:error_tracking_error) } + let(:error) { build(:error_tracking_sentry_error) } it 'returns a list of errors' do expect(response).to have_gitlab_http_status(:ok) @@ -126,7 +126,7 @@ RSpec.describe Projects::ErrorTrackingController do .and_return(external_url) end - let(:error) { build(:error_tracking_error) } + let(:error) { build(:error_tracking_sentry_error) } it 'returns a list of errors' do get :index, params: project_params(format: :json) @@ -221,7 +221,7 @@ RSpec.describe Projects::ErrorTrackingController do get :details, params: issue_params(issue_id: issue_id, format: :json) end - let(:error) { build(:detailed_error_tracking_error) } + let(:error) { build(:error_tracking_sentry_detailed_error) } it 'returns an error' do expected_error = error.as_json.except('first_release_version').merge( diff --git a/spec/controllers/projects/feature_flags_controller_spec.rb b/spec/controllers/projects/feature_flags_controller_spec.rb index f809dd31b3b..e038b247eff 100644 --- a/spec/controllers/projects/feature_flags_controller_spec.rb +++ b/spec/controllers/projects/feature_flags_controller_spec.rb @@ -652,7 +652,7 @@ RSpec.describe Projects::FeatureFlagsController do version: 'new_version_flag', strategies_attributes: [{ name: 'flexibleRollout', - parameters: { groupId: 'default', rollout: '15', stickiness: 'DEFAULT' }, + parameters: { groupId: 'default', rollout: '15', stickiness: 'default' }, scopes_attributes: [{ environment_scope: 'production' }] }] } @@ -666,7 +666,7 @@ RSpec.describe Projects::FeatureFlagsController do strategy_json = json_response['strategies'].first expect(strategy_json['name']).to eq('flexibleRollout') - expect(strategy_json['parameters']).to eq({ 'groupId' => 'default', 'rollout' => '15', 'stickiness' => 'DEFAULT' }) + expect(strategy_json['parameters']).to eq({ 'groupId' => 'default', 'rollout' => '15', 'stickiness' => 'default' }) expect(strategy_json['scopes'].count).to eq(1) scope_json = strategy_json['scopes'].first @@ -938,7 +938,7 @@ RSpec.describe Projects::FeatureFlagsController do it 'creates a flexibleRollout strategy' do put_request(new_version_flag, strategies_attributes: [{ name: 'flexibleRollout', - parameters: { groupId: 'default', rollout: '30', stickiness: 'DEFAULT' } + parameters: { groupId: 'default', rollout: '30', stickiness: 'default' } }]) expect(response).to have_gitlab_http_status(:ok) @@ -948,7 +948,7 @@ RSpec.describe Projects::FeatureFlagsController do expect(strategy_json['parameters']).to eq({ 'groupId' => 'default', 'rollout' => '30', - 'stickiness' => 'DEFAULT' + 'stickiness' => 'default' }) expect(strategy_json['scopes']).to eq([]) end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 922ecb6052a..0c29280316a 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1464,7 +1464,7 @@ RSpec.describe Projects::IssuesController do } end - it 'updates issue' do + it 'updates issue', :enable_admin_mode do post_spam expect(issue.submittable_as_spam?).to be_falsey end diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index a7a36d3a074..e9e7c3c3bb3 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -868,64 +868,85 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do end describe 'POST cancel' do - before do - project.add_developer(user) - sign_in(user) - end + context 'when user is authorized to cancel the build' do + before do + project.add_developer(user) + sign_in(user) + end - context 'when continue url is present' do - let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) } + context 'when continue url is present' do + let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) } - context 'when continue to is a safe url' do - let(:url) { '/test' } + context 'when continue to is a safe url' do + let(:url) { '/test' } - before do - post_cancel(continue: { to: url }) - end + before do + post_cancel(continue: { to: url }) + end - it 'redirects to the continue url' do - expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(url) + it 'redirects to the continue url' do + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(url) + end + + it 'transits to canceled' do + expect(job.reload).to be_canceled + end end - it 'transits to canceled' do - expect(job.reload).to be_canceled + context 'when continue to is not a safe url' do + let(:url) { 'http://example.com' } + + it 'raises an error' do + expect { cancel_with_redirect(url) }.to raise_error + end end end - context 'when continue to is not a safe url' do - let(:url) { 'http://example.com' } + context 'when continue url is not present' do + before do + post_cancel + end + + context 'when job is cancelable' do + let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) } + + it 'redirects to the builds page' do + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(builds_namespace_project_pipeline_path(id: pipeline.id)) + end + + it 'transits to canceled' do + expect(job.reload).to be_canceled + end + end + + context 'when job is not cancelable' do + let(:job) { create(:ci_build, :canceled, pipeline: pipeline) } - it 'raises an error' do - expect { cancel_with_redirect(url) }.to raise_error + it 'returns unprocessable_entity' do + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end end end end - context 'when continue url is not present' do + context 'when user is not authorized to cancel the build' do + let!(:job) { create(:ci_build, :cancelable, pipeline: pipeline) } + before do + project.add_reporter(user) + sign_in(user) + post_cancel end - context 'when job is cancelable' do - let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) } - - it 'redirects to the builds page' do - expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(builds_namespace_project_pipeline_path(id: pipeline.id)) - end - - it 'transits to canceled' do - expect(job.reload).to be_canceled - end + it 'responds with not_found' do + expect(response).to have_gitlab_http_status(:not_found) end - context 'when job is not cancelable' do - let(:job) { create(:ci_build, :canceled, pipeline: pipeline) } - - it 'returns unprocessable_entity' do - expect(response).to have_gitlab_http_status(:unprocessable_entity) - end + it 'does not transit to canceled' do + expect(job.reload).not_to be_canceled end end @@ -938,43 +959,60 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do describe 'POST unschedule' do before do - project.add_developer(user) + create(:protected_branch, :developers_can_merge, name: 'master', project: project) + end - create(:protected_branch, :developers_can_merge, - name: 'master', project: project) + context 'when user is authorized to unschedule the build' do + before do + project.add_developer(user) + sign_in(user) - sign_in(user) + post_unschedule + end - post_unschedule - end + context 'when job is scheduled' do + let(:job) { create(:ci_build, :scheduled, pipeline: pipeline) } - context 'when job is scheduled' do - let(:job) { create(:ci_build, :scheduled, pipeline: pipeline) } + it 'redirects to the unscheduled job page' do + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(namespace_project_job_path(id: job.id)) + end - it 'redirects to the unscheduled job page' do - expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(namespace_project_job_path(id: job.id)) + it 'transits to manual' do + expect(job.reload).to be_manual + end end - it 'transits to manual' do - expect(job.reload).to be_manual + context 'when job is not scheduled' do + let(:job) { create(:ci_build, pipeline: pipeline) } + + it 'renders unprocessable_entity' do + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end end end - context 'when job is not scheduled' do - let(:job) { create(:ci_build, pipeline: pipeline) } + context 'when user is not authorized to unschedule the build' do + let(:job) { create(:ci_build, :scheduled, pipeline: pipeline) } - it 'renders unprocessable_entity' do - expect(response).to have_gitlab_http_status(:unprocessable_entity) + before do + project.add_reporter(user) + sign_in(user) + + post_unschedule + end + + it 'responds with not_found' do + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'does not transit to scheduled' do + expect(job.reload).not_to be_manual end end def post_unschedule - post :unschedule, params: { - namespace_id: project.namespace, - project_id: project, - id: job.id - } + post :unschedule, params: { namespace_id: project.namespace, project_id: project, id: job.id } 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 707d074b5c1..3d7636b1f30 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Projects::MergeRequests::DiffsController do include ProjectForksHelper + include TrackingHelpers shared_examples '404 for unexistent diffable' do context 'when diffable does not exists' do @@ -141,6 +142,24 @@ RSpec.describe Projects::MergeRequests::DiffsController do end describe 'GET diffs_metadata' do + shared_examples_for 'serializes diffs metadata with expected arguments' do + it 'returns success' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'serializes paginated merge request diff collection' do + expect_next_instance_of(DiffsMetadataSerializer) do |instance| + expect(instance).to receive(:represent) + .with(an_instance_of(collection), expected_options) + .and_call_original + end + + subject + end + end + def go(extra_params = {}) params = { namespace_id: project.namespace.to_param, @@ -179,32 +198,25 @@ RSpec.describe Projects::MergeRequests::DiffsController do end context 'with valid diff_id' do - it 'returns success' do - go(diff_id: merge_request.merge_request_diff.id) - - expect(response).to have_gitlab_http_status(:ok) - end - - it 'serializes diffs metadata with expected arguments' do - expected_options = { - environment: nil, - merge_request: merge_request, - merge_request_diff: merge_request.merge_request_diff, - merge_request_diffs: merge_request.merge_request_diffs, - start_version: nil, - start_sha: nil, - commit: nil, - latest_diff: true, - only_context_commits: false - } + subject { go(diff_id: merge_request.merge_request_diff.id) } - expect_next_instance_of(DiffsMetadataSerializer) do |instance| - expect(instance).to receive(:represent) - .with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), expected_options) - .and_call_original + it_behaves_like 'serializes diffs metadata with expected arguments' do + let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff } + let(:expected_options) do + { + environment: nil, + merge_request: merge_request, + merge_request_diff: merge_request.merge_request_diff, + merge_request_diffs: merge_request.merge_request_diffs, + start_version: nil, + start_sha: nil, + commit: nil, + latest_diff: true, + only_context_commits: false, + allow_tree_conflicts: true, + merge_ref_head_diff: false + } end - - go(diff_id: merge_request.merge_request_diff.id) end end @@ -261,62 +273,75 @@ RSpec.describe Projects::MergeRequests::DiffsController do end context 'with MR regular diff params' do - it 'returns success' do - go + subject { go } - expect(response).to have_gitlab_http_status(:ok) + it_behaves_like 'serializes diffs metadata with expected arguments' do + let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff } + let(:expected_options) do + { + environment: nil, + merge_request: merge_request, + merge_request_diff: merge_request.merge_request_diff, + merge_request_diffs: merge_request.merge_request_diffs, + start_version: nil, + start_sha: nil, + commit: nil, + latest_diff: true, + only_context_commits: false, + allow_tree_conflicts: true, + merge_ref_head_diff: nil + } + end end + end - it 'serializes diffs metadata with expected arguments' do - expected_options = { - environment: nil, - merge_request: merge_request, - merge_request_diff: merge_request.merge_request_diff, - merge_request_diffs: merge_request.merge_request_diffs, - start_version: nil, - start_sha: nil, - commit: nil, - latest_diff: true, - only_context_commits: false - } + context 'with commit param' do + subject { go(commit_id: merge_request.diff_head_sha) } - expect_next_instance_of(DiffsMetadataSerializer) do |instance| - expect(instance).to receive(:represent) - .with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), expected_options) - .and_call_original + it_behaves_like 'serializes diffs metadata with expected arguments' do + let(:collection) { Gitlab::Diff::FileCollection::Commit } + let(:expected_options) do + { + environment: nil, + merge_request: merge_request, + merge_request_diff: nil, + merge_request_diffs: merge_request.merge_request_diffs, + start_version: nil, + start_sha: nil, + commit: merge_request.diff_head_commit, + latest_diff: nil, + only_context_commits: false, + allow_tree_conflicts: true, + merge_ref_head_diff: nil + } end - - go end end - context 'with commit param' do - it 'returns success' do - go(commit_id: merge_request.diff_head_sha) + context 'when display_merge_conflicts_in_diff is disabled' do + subject { go } - expect(response).to have_gitlab_http_status(:ok) + before do + stub_feature_flags(display_merge_conflicts_in_diff: false) end - it 'serializes diffs metadata with expected arguments' do - expected_options = { - environment: nil, - merge_request: merge_request, - merge_request_diff: nil, - merge_request_diffs: merge_request.merge_request_diffs, - start_version: nil, - start_sha: nil, - commit: merge_request.diff_head_commit, - latest_diff: nil, - only_context_commits: false - } - - expect_next_instance_of(DiffsMetadataSerializer) do |instance| - expect(instance).to receive(:represent) - .with(an_instance_of(Gitlab::Diff::FileCollection::Commit), expected_options) - .and_call_original + it_behaves_like 'serializes diffs metadata with expected arguments' do + let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff } + let(:expected_options) do + { + environment: nil, + merge_request: merge_request, + merge_request_diff: merge_request.merge_request_diff, + merge_request_diffs: merge_request.merge_request_diffs, + start_version: nil, + start_sha: nil, + commit: nil, + latest_diff: true, + only_context_commits: false, + allow_tree_conflicts: false, + merge_ref_head_diff: nil + } end - - go(commit_id: merge_request.diff_head_sha) end end end @@ -423,7 +448,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do context 'when DNT is enabled' do before do - request.headers['DNT'] = '1' + stub_do_not_track('1') end it 'does not track any mr_diffs event' do @@ -471,6 +496,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do merge_request: merge_request, diff_view: :inline, merge_ref_head_diff: nil, + allow_tree_conflicts: true, pagination_data: { total_pages: nil }.merge(pagination_data) @@ -589,6 +615,21 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'successful request' end + context 'when display_merge_conflicts_in_diff is disabled' do + before do + stub_feature_flags(display_merge_conflicts_in_diff: false) + end + + subject { go } + + it_behaves_like 'serializes diffs with expected arguments' do + let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } + let(:expected_options) { collection_arguments(total_pages: 20).merge(allow_tree_conflicts: false) } + end + + it_behaves_like 'successful request' + end + it_behaves_like 'forked project with submodules' it_behaves_like 'cached diff collection' diff --git a/spec/controllers/projects/packages/infrastructure_registry_controller_spec.rb b/spec/controllers/projects/packages/infrastructure_registry_controller_spec.rb index dc5a022eb7b..fc741d0f3f6 100644 --- a/spec/controllers/projects/packages/infrastructure_registry_controller_spec.rb +++ b/spec/controllers/projects/packages/infrastructure_registry_controller_spec.rb @@ -18,9 +18,9 @@ RSpec.describe Projects::Packages::InfrastructureRegistryController do it_behaves_like 'returning response status', :ok - context 'when the feature is disabled' do + context 'when the packages registry is not available' do before do - stub_feature_flags(infrastructure_registry_page: false) + stub_config(packages: { enabled: false }) end it_behaves_like 'returning response status', :not_found @@ -34,9 +34,9 @@ RSpec.describe Projects::Packages::InfrastructureRegistryController do it_behaves_like 'returning response status', :ok - context 'when the feature is disabled' do + context 'when the packages registry is not available' do before do - stub_feature_flags(infrastructure_registry_page: false) + stub_config(packages: { enabled: false }) end it_behaves_like 'returning response status', :not_found diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 9ed43a251a2..be5c1f0d428 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -93,13 +93,12 @@ RSpec.describe Projects::ProjectMembersController do let_it_be(:invited_member) { create(:project_member, :invited, project: project) } before do - project.add_maintainer(user) sign_in(user) end context 'when user has `admin_project_member` permissions' do before do - allow(controller.helpers).to receive(:can_manage_project_members?).with(project).and_return(true) + project.add_maintainer(user) end it 'lists invited members' do @@ -110,10 +109,6 @@ RSpec.describe Projects::ProjectMembersController do end context 'when user does not have `admin_project_member` permissions' do - before do - allow(controller.helpers).to receive(:can_manage_project_members?).with(project).and_return(false) - end - it 'does not list invited members' do get :index, params: { namespace_id: project.namespace, project_id: project } @@ -127,13 +122,12 @@ RSpec.describe Projects::ProjectMembersController do before do project.request_access(access_requester_user) - project.add_maintainer(user) sign_in(user) end context 'when user has `admin_project_member` permissions' do before do - allow(controller.helpers).to receive(:can_manage_project_members?).with(project).and_return(true) + project.add_maintainer(user) end it 'lists access requests' do @@ -144,10 +138,6 @@ RSpec.describe Projects::ProjectMembersController do end context 'when user does not have `admin_project_member` permissions' do - before do - allow(controller.helpers).to receive(:can_manage_project_members?).with(project).and_return(false) - end - it 'does not list access requests' do get :index, params: { namespace_id: project.namespace, project_id: project } diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index 5dee36ee7c2..2c25c7e20ea 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -33,15 +33,25 @@ RSpec.describe Projects::RawController do end context 'regular filename' do - let(:filepath) { 'master/README.md' } + let(:filepath) { 'master/CONTRIBUTING.md' } it 'delivers ASCII file' do + allow(Gitlab::Workhorse).to receive(:send_git_blob).and_call_original + subject expect(response).to have_gitlab_http_status(:ok) expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8') expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq 'true' expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:') + + expect(Gitlab::Workhorse).to have_received(:send_git_blob) do |repository, blob| + expected_blob = project.repository.blob_at('master', 'CONTRIBUTING.md') + + expect(repository).to eq(project.repository) + expect(blob.id).to eq(expected_blob.id) + expect(blob).to be_truncated + end end it_behaves_like 'project cache control headers' diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index baf3bde83bd..419b5c7e101 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -174,6 +174,8 @@ RSpec.describe Projects::ServicesController do let(:redirect_url) { edit_project_service_path(project, integration) } before do + stub_jira_integration_test + put :update, params: params end @@ -222,12 +224,48 @@ RSpec.describe Projects::ServicesController do end end - context 'when param `inherit_from_id` is set to some value' do - let(:instance_service) { create(:jira_integration, :instance) } - let(:integration_params) { { inherit_from_id: instance_service.id } } + context 'when param `inherit_from_id` is set to an instance integration' do + let(:instance_integration) { create(:jira_integration, :instance, url: 'http://instance.com', password: 'instance') } + let(:integration_params) { { inherit_from_id: instance_integration.id, url: 'http://custom.com', password: 'custom' } } + + it 'ignores submitted params and inherits instance settings' do + expect(integration.reload).to have_attributes( + inherit_from_id: instance_integration.id, + url: instance_integration.url, + password: instance_integration.password + ) + end + end + + context 'when param `inherit_from_id` is set to a group integration' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:jira_integration) { create(:jira_integration, project: project) } - it 'sets inherit_from_id to value' do - expect(integration.reload.inherit_from_id).to eq(instance_service.id) + let(:group_integration) { create(:jira_integration, group: group, project: nil, url: 'http://group.com', password: 'group') } + let(:integration_params) { { inherit_from_id: group_integration.id, url: 'http://custom.com', password: 'custom' } } + + it 'ignores submitted params and inherits group settings' do + expect(integration.reload).to have_attributes( + inherit_from_id: group_integration.id, + url: group_integration.url, + password: group_integration.password + ) + end + end + + context 'when param `inherit_from_id` is set to an unrelated group' do + let_it_be(:group) { create(:group) } + + let(:group_integration) { create(:jira_integration, group: group, project: nil, url: 'http://group.com', password: 'group') } + let(:integration_params) { { inherit_from_id: group_integration.id, url: 'http://custom.com', password: 'custom' } } + + it 'ignores the param and saves the submitted settings' do + expect(integration.reload).to have_attributes( + inherit_from_id: nil, + url: 'http://custom.com', + password: 'custom' + ) end end end @@ -239,22 +277,39 @@ RSpec.describe Projects::ServicesController do end context 'when update succeeds' do - let(:integration_params) { { url: 'http://example.com' } } + let(:integration_params) { { url: 'http://example.com', password: 'password' } } - it 'returns JSON response with no errors' do + it 'returns success response' do expect(response).to be_successful - expect(json_response).to include('active' => true, 'errors' => {}) + expect(json_response).to include( + 'active' => true, + 'errors' => {} + ) + end + end + + context 'when update fails with missing password' do + let(:integration_params) { { url: 'http://example.com' } } + + it 'returns JSON response errors' do + expect(response).not_to be_successful + expect(json_response).to include( + 'active' => true, + 'errors' => { + 'password' => ["can't be blank"] + } + ) end end - context 'when update fails' do - let(:integration_params) { { url: '' } } + context 'when update fails with invalid URL' do + let(:integration_params) { { url: '', password: 'password' } } it 'returns JSON response with errors' do expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(json_response).to include( 'active' => true, - 'errors' => { 'url' => ['must be a valid URL', %(can't be blank)] } + 'errors' => { 'url' => ['must be a valid URL', "can't be blank"] } ) end end diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index 1a6c0974f08..a388fc4620f 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -110,7 +110,7 @@ RSpec.describe Projects::SnippetsController do } end - it 'updates the snippet' do + it 'updates the snippet', :enable_admin_mode do mark_as_spam expect(snippet.reload).not_to be_submittable_as_spam @@ -181,6 +181,24 @@ RSpec.describe Projects::SnippetsController do end end end + + context 'when the project snippet is public' do + let_it_be(:project_snippet_public) { create(:project_snippet, :public, :repository, project: project, author: user) } + + context 'when attempting to access from a different project route' do + subject { get action, params: { namespace_id: project.namespace, project_id: 42, id: project_snippet_public.to_param } } + + before do + sign_in(user) + end + + it 'responds with status 404' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 46c17d6a6fe..8afb80d9cc5 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -435,32 +435,6 @@ 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, - initialize_with_readme: 1 - } - 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', - value: 1 - ).with_context(actor: user).on_next_instance - - 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) } @@ -793,8 +767,7 @@ RSpec.describe ProjectsController do id: project.path, project: { project_setting_attributes: { - show_default_award_emojis: boolean_value, - allow_editing_commit_messages: boolean_value + show_default_award_emojis: boolean_value } } } @@ -802,7 +775,33 @@ RSpec.describe ProjectsController do project.reload expect(project.show_default_award_emojis?).to eq(result) - expect(project.allow_editing_commit_messages?).to eq(result) + end + end + end + + context 'with project feature attributes' do + using RSpec::Parameterized::TableSyntax + + where(:feature, :initial_value, :update_to) do + :metrics_dashboard_access_level | ProjectFeature::PRIVATE | ProjectFeature::ENABLED + :container_registry_access_level | ProjectFeature::ENABLED | ProjectFeature::PRIVATE + end + + with_them do + it "updates the project_feature new" do + params = { + namespace_id: project.namespace, + id: project.path, + project: { + project_feature_attributes: { + "#{feature}": update_to + } + } + } + + expect { put :update, params: params }.to change { + project.reload.project_feature.public_send(feature) + }.from(initial_value).to(update_to) end end end diff --git a/spec/controllers/registrations/welcome_controller_spec.rb b/spec/controllers/registrations/welcome_controller_spec.rb index 6d34b56df09..034c9b3d1c0 100644 --- a/spec/controllers/registrations/welcome_controller_spec.rb +++ b/spec/controllers/registrations/welcome_controller_spec.rb @@ -60,10 +60,8 @@ 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', email_opted_in: email_opted_in } } + patch :update, params: { user: { role: 'software_developer', setup_for_company: 'false' } } end context 'without a signed in user' do @@ -100,24 +98,6 @@ RSpec.describe Registrations::WelcomeController do end end end - - 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/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 72aa9038c3e..301c60e89c8 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -155,34 +155,76 @@ RSpec.describe RegistrationsController do end context 'when registration is triggered from an accepted invite' do - context 'when it is part of our invite email experiment', :experiment do + context 'when it is part from the initial invite email', :snowplow do let_it_be(:member) { create(:project_member, :invited, invite_email: user_params.dig(:user, :email)) } let(:originating_member_id) { member.id } + let(:extra_session_params) { {} } let(:session_params) do { invite_email: user_params.dig(:user, :email), originating_member_id: originating_member_id - } + }.merge extra_session_params end context 'when member exists from the session key value' do - it 'tracks the experiment' do - expect(experiment('members/invite_email')).to track(:accepted) - .with_context(actor: member) - .on_next_instance - + it 'tracks the invite acceptance' do subject + + expect_snowplow_event( + category: 'RegistrationsController', + action: 'accepted', + label: 'invite_email', + property: member.id.to_s + ) end end context 'when member does not exist from the session key value' do let(:originating_member_id) { -1 } - it 'tracks the experiment' do - expect(experiment('members/invite_email')).not_to track(:accepted) - + it 'does not track invite acceptance' do subject + + expect_no_snowplow_event( + category: 'RegistrationsController', + action: 'accepted', + label: 'invite_email' + ) + end + end + + context 'with the invite_email_preview_text experiment', :experiment do + let(:extra_session_params) { { invite_email_experiment_name: 'invite_email_preview_text' } } + + context 'when member and invite_email_experiment_name exists from the session key value' do + it 'tracks the invite acceptance' do + expect(experiment(:invite_email_preview_text)).to track(:accepted) + .with_context(actor: member) + .on_next_instance + + subject + end + end + + context 'when member does not exist from the session key value' do + let(:originating_member_id) { -1 } + + it 'does not track invite acceptance' do + expect(experiment(:invite_email_preview_text)).not_to track(:accepted) + + subject + end + end + + context 'when invite_email_experiment_name does not exist from the session key value' do + let(:extra_session_params) { {} } + + it 'does not track invite acceptance' do + expect(experiment(:invite_email_preview_text)).not_to track(:accepted) + + subject + end end end end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 3a2986f6cbe..e0870e17d99 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -53,6 +53,20 @@ RSpec.describe SearchController do end end + shared_examples_for 'support for active record query timeouts' do |action, params, method_to_stub, format| + before do + allow_next_instance_of(SearchService) do |service| + allow(service).to receive(method_to_stub).and_raise(ActiveRecord::QueryCanceled) + end + end + + it 'renders a 408 when a timeout occurs' do + get action, params: params, format: format + + expect(response).to have_gitlab_http_status(:request_timeout) + end + 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 @@ -63,6 +77,7 @@ RSpec.describe SearchController do end it_behaves_like 'with external authorization service enabled', :show, { search: 'hello' } + it_behaves_like 'support for active record query timeouts', :show, { search: 'hello' }, :search_objects, :html context 'uses the right partials depending on scope' do using RSpec::Parameterized::TableSyntax @@ -230,6 +245,7 @@ RSpec.describe SearchController do 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' } + it_behaves_like 'support for active record query timeouts', :count, { search: 'hello', scope: 'projects' }, :search_results, :json it 'returns the result count for the given term and scope' do create(:project, :public, name: 'hello world') diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 50d6ac8f23d..a82c44fcc44 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -231,7 +231,7 @@ RSpec.describe SnippetsController do post :mark_as_spam, params: { id: public_snippet.id } end - it 'updates the snippet' do + it 'updates the snippet', :enable_admin_mode do mark_as_spam expect(public_snippet.reload).not_to be_submittable_as_spam |