diff options
Diffstat (limited to 'spec/controllers')
48 files changed, 1238 insertions, 751 deletions
diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 37cb0a1f289..6085f0e1239 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -43,5 +43,13 @@ RSpec.describe Admin::GroupsController do post :create, params: { group: { path: 'test', name: 'test', admin_note_attributes: { note: 'test' } } } end.to change { Namespace::AdminNote.count }.by(1) end + + it 'delegates to Groups::CreateService service instance' do + expect_next_instance_of(::Groups::CreateService) do |service| + expect(service).to receive(:execute).once.and_call_original + end + + post :create, params: { group: { path: 'test', name: 'test' } } + end end end diff --git a/spec/controllers/admin/hooks_controller_spec.rb b/spec/controllers/admin/hooks_controller_spec.rb index 14f4a2f40e7..82e4b873bf6 100644 --- a/spec/controllers/admin/hooks_controller_spec.rb +++ b/spec/controllers/admin/hooks_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Admin::HooksController do - let(:admin) { create(:admin) } + let_it_be(:admin) { create(:admin) } before do sign_in(admin) @@ -33,7 +33,23 @@ RSpec.describe Admin::HooksController do end describe 'POST #update' do - let!(:hook) { create(:system_hook) } + let_it_be_with_reload(:hook) { create(:system_hook) } + + context 'with an existing token' do + hook_params = { + token: WebHook::SECRET_MASK, + url: "http://example.com" + } + + it 'does not change a token' do + expect do + post :update, params: { id: hook.id, hook: hook_params } + end.not_to change { hook.reload.token } + + expect(response).to have_gitlab_http_status(:found) + expect(flash[:alert]).to be_blank + end + end it 'sets all parameters' do hook.update!(url_variables: { 'foo' => 'bar', 'baz' => 'woo' }) @@ -61,8 +77,8 @@ RSpec.describe Admin::HooksController do end describe 'DELETE #destroy' do - let!(:hook) { create(:system_hook) } - let!(:log) { create(:web_hook_log, web_hook: hook) } + let_it_be(:hook) { create(:system_hook) } + let_it_be(:log) { create(:web_hook_log, web_hook: hook) } let(:params) { { id: hook } } it_behaves_like 'Web hook destroyer' diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index 0e456858b49..e75f27589d7 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Admin::IntegrationsController do describe '#edit' do Integration.available_integration_names.each do |integration_name| - context "#{integration_name}" do + context integration_name.to_s do it 'successfully displays the template' do get :edit, params: { id: integration_name } diff --git a/spec/controllers/admin/spam_logs_controller_spec.rb b/spec/controllers/admin/spam_logs_controller_spec.rb index 48221f496fb..51f7ecdece6 100644 --- a/spec/controllers/admin/spam_logs_controller_spec.rb +++ b/spec/controllers/admin/spam_logs_controller_spec.rb @@ -27,34 +27,17 @@ RSpec.describe Admin::SpamLogsController do expect(response).to have_gitlab_http_status(:ok) end - context 'when user_destroy_with_limited_execution_time_worker is enabled' do - it 'initiates user removal', :sidekiq_inline do - expect do - delete :destroy, params: { id: first_spam.id, remove_user: true } - end.not_to change { SpamLog.count } - - expect(response).to have_gitlab_http_status(:found) - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin) - ).to be_exists - expect(flash[:notice]).to eq("User #{user.username} was successfully removed.") - end - end - - context 'when user_destroy_with_limited_execution_time_worker is disabled' do - before do - stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) - end - - it 'removes user and their spam logs when removing the user', :sidekiq_inline do + it 'initiates user removal', :sidekiq_inline do + expect do delete :destroy, params: { id: first_spam.id, remove_user: true } + end.not_to change { SpamLog.count } - expect(flash[:notice]).to eq "User #{user.username} was successfully removed." - expect(response).to have_gitlab_http_status(:found) - expect(SpamLog.count).to eq(0) - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - end + expect(response).to have_gitlab_http_status(:found) + expect( + Users::GhostUserMigration.where(user: user, + initiator_user: admin) + ).to be_exists + expect(flash[:notice]).to eq("User #{user.username} was successfully removed.") end end diff --git a/spec/controllers/admin/topics_controller_spec.rb b/spec/controllers/admin/topics_controller_spec.rb index 111fdcc3be6..e640f8bb7ec 100644 --- a/spec/controllers/admin/topics_controller_spec.rb +++ b/spec/controllers/admin/topics_controller_spec.rb @@ -176,7 +176,7 @@ RSpec.describe Admin::TopicsController do describe 'POST #merge' do let_it_be(:source_topic) { create(:topic, name: 'source_topic') } - let_it_be(:project) { create(:project, topic_list: source_topic.name ) } + let_it_be(:project) { create(:project, topic_list: source_topic.name) } it 'merges source topic into target topic' do post :merge, params: { source_topic_id: source_topic.id, target_topic_id: topic.id } diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 682399f4dd9..eecb803fb1a 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -63,130 +63,179 @@ RSpec.describe Admin::UsersController do expect(response).to be_redirect expect(response.location).to end_with(user.username) end - end - describe 'DELETE #destroy', :sidekiq_might_not_need_inline do - let(:project) { create(:project, namespace: user.namespace) } - let!(:issue) { create(:issue, author: user) } + describe 'impersonation_error_text' do + context 'when user can be impersonated' do + it 'sets impersonation_error_text to nil' do + get :show, params: { id: user.username.downcase } - before do - project.add_developer(user) - end + expect(assigns(:impersonation_error_text)).to eq(nil) + end + end - context 'when user_destroy_with_limited_execution_time_worker is enabled' do - it 'initiates user removal' do - delete :destroy, params: { id: user.username }, format: :json + context 'when impersonation is already in progress' do + let(:admin2) { create(:admin) } - expect(response).to have_gitlab_http_status(:ok) - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin, - hard_delete: false) - ).to be_exists + before do + post :impersonate, params: { id: admin2.username } + end + + it 'sets impersonation_error_text' do + get :show, params: { id: user.username.downcase } + + expect(assigns(:impersonation_error_text)).to eq(_("You are already impersonating another user")) + end end - it 'initiates user removal and passes hard delete option' do - delete :destroy, params: { id: user.username, hard_delete: true }, format: :json + context 'when user is blocked' do + before do + user.block + end - expect(response).to have_gitlab_http_status(:ok) - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin, - hard_delete: true) - ).to be_exists + it 'sets impersonation_error_text' do + get :show, params: { id: user.username.downcase } + + expect(assigns(:impersonation_error_text)).to eq(_("You cannot impersonate a blocked user")) + end end - context 'prerequisites for account deletion' do - context 'solo-owned groups' do - let(:group) { create(:group) } + context "when the user's password is expired" do + before do + user.update!(password_expires_at: 1.day.ago) + end - context 'if the user is the sole owner of at least one group' do - before do - create(:group_member, :owner, group: group, user: user) - end + it 'sets impersonation_error_text' do + get :show, params: { id: user.username.downcase } + + expect(assigns(:impersonation_error_text)).to eq(_("You cannot impersonate a user with an expired password")) + end + end - context 'soft-delete' do - it 'fails' do - delete :destroy, params: { id: user.username } + context "when the user is internal" do + before do + user.update!(user_type: :migration_bot) + end - message = s_('AdminUsers|You must transfer ownership or delete the groups owned by this user before you can delete their account') + it 'sets impersonation_error_text' do + get :show, params: { id: user.username.downcase } - expect(flash[:alert]).to eq(message) - expect(response).to have_gitlab_http_status(:see_other) - expect(response).to redirect_to admin_user_path(user) - expect(Users::GhostUserMigration).not_to exist - end - end + expect(assigns(:impersonation_error_text)).to eq(_("You cannot impersonate an internal user")) + end + end - context 'hard-delete' do - it 'succeeds' do - delete :destroy, params: { id: user.username, hard_delete: true } - - expect(response).to redirect_to(admin_users_path) - expect(flash[:notice]).to eq(_('The user is being deleted.')) - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin, - hard_delete: true) - ).to be_exists - end - end - end + context "when the user is a project bot" do + before do + user.update!(user_type: :project_bot) + end + + it 'sets impersonation_error_text' do + get :show, params: { id: user.username.downcase } + + expect(assigns(:impersonation_error_text)).to eq(_("You cannot impersonate a user who cannot log in")) end end end - context 'when user_destroy_with_limited_execution_time_worker is disabled' do - before do - stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) + describe 'can_impersonate' do + context 'when user can be impersonated' do + it 'sets can_impersonate to true' do + get :show, params: { id: user.username.downcase } + + expect(assigns(:can_impersonate)).to eq(true) + end end - it 'deletes user and ghosts their contributions' do - delete :destroy, params: { id: user.username }, format: :json + context 'when impersonation is already in progress' do + let(:admin2) { create(:admin) } - expect(response).to have_gitlab_http_status(:ok) - expect(User.exists?(user.id)).to be_falsy - expect(issue.reload.author).to be_ghost + before do + post :impersonate, params: { id: admin2.username } + end + + it 'sets can_impersonate to false' do + get :show, params: { id: user.username.downcase } + + expect(assigns(:can_impersonate)).to eq(false) + end end - it 'deletes the user and their contributions when hard delete is specified' do - delete :destroy, params: { id: user.username, hard_delete: true }, format: :json + context 'when user cannot log in' do + before do + user.update!(user_type: :project_bot) + end + + it 'sets can_impersonate to false' do + get :show, params: { id: user.username.downcase } - expect(response).to have_gitlab_http_status(:ok) - expect(User.exists?(user.id)).to be_falsy - expect(Issue.exists?(issue.id)).to be_falsy + expect(assigns(:can_impersonate)).to eq(false) + end end + end + end - context 'prerequisites for account deletion' do - context 'solo-owned groups' do - let(:group) { create(:group) } + describe 'DELETE #destroy', :sidekiq_might_not_need_inline do + let(:project) { create(:project, namespace: user.namespace) } + let!(:issue) { create(:issue, author: user) } - context 'if the user is the sole owner of at least one group' do - before do - create(:group_member, :owner, group: group, user: user) - end + before do + project.add_developer(user) + end - context 'soft-delete' do - it 'fails' do - delete :destroy, params: { id: user.username } + it 'initiates user removal' do + delete :destroy, params: { id: user.username }, format: :json - message = s_('AdminUsers|You must transfer ownership or delete the groups owned by this user before you can delete their account') + expect(response).to have_gitlab_http_status(:ok) + expect( + Users::GhostUserMigration.where(user: user, + initiator_user: admin, + hard_delete: false) + ).to be_exists + end - expect(flash[:alert]).to eq(message) - expect(response).to have_gitlab_http_status(:see_other) - expect(response).to redirect_to admin_user_path(user) - expect(User.exists?(user.id)).to be_truthy - end - end + it 'initiates user removal and passes hard delete option' do + delete :destroy, params: { id: user.username, hard_delete: true }, format: :json + + expect(response).to have_gitlab_http_status(:ok) + expect( + Users::GhostUserMigration.where(user: user, + initiator_user: admin, + hard_delete: true) + ).to be_exists + end - context 'hard-delete' do - it 'succeeds' do - delete :destroy, params: { id: user.username, hard_delete: true } + context 'prerequisites for account deletion' do + context 'solo-owned groups' do + let(:group) { create(:group) } + + context 'if the user is the sole owner of at least one group' do + before do + create(:group_member, :owner, group: group, user: user) + end + + context 'soft-delete' do + it 'fails' do + delete :destroy, params: { id: user.username } + + message = s_('AdminUsers|You must transfer ownership or delete the groups owned by this user before you can delete their account') + + expect(flash[:alert]).to eq(message) + expect(response).to have_gitlab_http_status(:see_other) + expect(response).to redirect_to admin_user_path(user) + expect(Users::GhostUserMigration).not_to exist + end + end - expect(response).to redirect_to(admin_users_path) - expect(flash[:notice]).to eq(_('The user is being deleted.')) - expect(User.exists?(user.id)).to be_falsy - end + context 'hard-delete' do + it 'succeeds' do + delete :destroy, params: { id: user.username, hard_delete: true } + + expect(response).to redirect_to(admin_users_path) + expect(flash[:notice]).to eq(_('The user is being deleted.')) + expect( + Users::GhostUserMigration.where(user: user, + initiator_user: admin, + hard_delete: true) + ).to be_exists end end end @@ -200,27 +249,13 @@ RSpec.describe Admin::UsersController do context 'when rejecting a pending user' do let(:user) { create(:user, :blocked_pending_approval) } - context 'when user_destroy_with_limited_execution_time_worker is enabled' do - it 'initiates user removal', :sidekiq_inline do - subject - - expect( - Users::GhostUserMigration.where(user: user, - initiator_user: admin) - ).to be_exists - end - end - - context 'when user_destroy_with_limited_execution_time_worker is disabled' do - before do - stub_feature_flags(user_destroy_with_limited_execution_time_worker: false) - end - - it 'hard deletes the user', :sidekiq_inline do - subject + it 'initiates user removal', :sidekiq_inline do + subject - expect(User.exists?(user.id)).to be_falsy - end + expect( + Users::GhostUserMigration.where(user: user, + initiator_user: admin) + ).to be_exists end it 'displays the rejection message' do @@ -909,6 +944,60 @@ RSpec.describe Admin::UsersController do expect(session[:github_access_token]).to be_nil end + + context "when the user's password is expired" do + before do + user.update!(password_expires_at: 1.day.ago) + end + + it "shows a notice" do + post :impersonate, params: { id: user.username } + + expect(flash[:alert]).to eq(_('You cannot impersonate a user with an expired password')) + end + + it "doesn't sign us in as the user" do + post :impersonate, params: { id: user.username } + + expect(warden.user).to eq(admin) + end + end + + context "when the user is internal" do + before do + user.update!(user_type: :migration_bot) + end + + it "shows a notice" do + post :impersonate, params: { id: user.username } + + expect(flash[:alert]).to eq(_("You cannot impersonate an internal user")) + end + + it "doesn't sign us in as the user" do + post :impersonate, params: { id: user.username } + + expect(warden.user).to eq(admin) + end + end + + context "when the user is a project bot" do + before do + user.update!(user_type: :project_bot) + end + + it "shows a notice" do + post :impersonate, params: { id: user.username } + + expect(flash[:alert]).to eq(_("You cannot impersonate a user who cannot log in")) + end + + it "doesn't sign us in as the user" do + post :impersonate, params: { id: user.username } + + expect(warden.user).to eq(admin) + end + end end context "when impersonation is disabled" do diff --git a/spec/controllers/concerns/issuable_actions_spec.rb b/spec/controllers/concerns/issuable_actions_spec.rb index c3fef591b91..37d9dc080e1 100644 --- a/spec/controllers/concerns/issuable_actions_spec.rb +++ b/spec/controllers/concerns/issuable_actions_spec.rb @@ -6,8 +6,8 @@ RSpec.describe IssuableActions do let(:project) { double('project') } let(:user) { double('user') } let(:issuable) { double('issuable') } - let(:finder_params_for_issuable) { {} } - let(:notes_result) { double('notes_result') } + let(:finder_params_for_issuable) { { project: project, target: issuable } } + let(:notes_result) { [] } let(:discussion_serializer) { double('discussion_serializer') } let(:controller) do @@ -55,13 +55,20 @@ RSpec.describe IssuableActions do end it 'instantiates and calls NotesFinder as expected' do + expect(issuable).to receive(:to_ability_name).and_return('issue') + expect(issuable).to receive(:project).and_return(project) + expect(Ability).to receive(:allowed?).at_least(1).and_return(true) expect(Discussion).to receive(:build_collection).and_return([]) expect(DiscussionSerializer).to receive(:new).and_return(discussion_serializer) expect(NotesFinder).to receive(:new).with(user, finder_params_for_issuable).and_call_original expect_any_instance_of(NotesFinder).to receive(:execute).and_return(notes_result) - expect(notes_result).to receive_messages(inc_relations_for_view: notes_result, includes: notes_result, fresh: notes_result) + expect(notes_result).to receive_messages( + with_web_entity_associations: notes_result, + inc_relations_for_view: notes_result, + fresh: notes_result + ) controller.discussions end diff --git a/spec/controllers/concerns/preferred_language_switcher_spec.rb b/spec/controllers/concerns/preferred_language_switcher_spec.rb new file mode 100644 index 00000000000..40d6ac10c37 --- /dev/null +++ b/spec/controllers/concerns/preferred_language_switcher_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe PreferredLanguageSwitcher, type: :controller do + controller(ActionController::Base) do + include PreferredLanguageSwitcher # rubocop:disable RSpec/DescribedClass + + before_action :init_preferred_language, only: :new + + def new + render html: 'new page' + end + end + + context 'when first visit' do + before do + get :new + end + + it 'sets preferred_language to default' do + expect(cookies[:preferred_language]).to eq Gitlab::CurrentSettings.default_preferred_language + end + end + + context 'when preferred language in cookies has been modified' do + let(:user_preferred_language) { nil } + + before do + cookies[:preferred_language] = user_preferred_language + + get :new + end + + context 'with a valid value' do + let(:user_preferred_language) { 'zh_CN' } + + it 'keeps preferred language unchanged' do + expect(cookies[:preferred_language]).to eq user_preferred_language + end + end + + context 'with an invalid value' do + let(:user_preferred_language) { 'xxx' } + + it 'sets preferred_language to default' do + expect(cookies[:preferred_language]).to eq Gitlab::CurrentSettings.default_preferred_language + end + end + end +end diff --git a/spec/controllers/concerns/renders_commits_spec.rb b/spec/controllers/concerns/renders_commits_spec.rb index acdeb98bb16..6a504681527 100644 --- a/spec/controllers/concerns/renders_commits_spec.rb +++ b/spec/controllers/concerns/renders_commits_spec.rb @@ -43,7 +43,7 @@ RSpec.describe RendersCommits do context 'rendering commits' do render_views - it 'avoids N + 1' do + it 'avoids N + 1', :request_store do stub_const("MergeRequestDiff::COMMITS_SAFE_SIZE", 5) control_count = ActiveRecord::QueryRecorder.new do @@ -59,7 +59,7 @@ RSpec.describe RendersCommits do end describe '.prepare_commits_for_rendering' do - it 'avoids N+1' do + it 'avoids N+1', :request_store do control = ActiveRecord::QueryRecorder.new do subject.prepare_commits_for_rendering(merge_request.commits.take(1)) end diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 32304815bbb..0b24387483b 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -18,6 +18,12 @@ RSpec.describe SendFileUpload do end end + let(:cdn_uploader_class) do + Class.new(uploader_class) do + include ObjectStorage::CDN::Concern + end + end + let(:controller_class) do Class.new do include SendFileUpload @@ -269,5 +275,28 @@ RSpec.describe SendFileUpload do it_behaves_like 'handles image resize requests' end + + context 'when CDN-enabled remote file is used' do + let(:uploader) { cdn_uploader_class.new(object, :file) } + let(:request) { instance_double('ActionDispatch::Request', remote_ip: '18.245.0.42') } + let(:signed_url) { 'https://cdn.example.org.test' } + let(:cdn_provider) { instance_double('ObjectStorage::CDN::GoogleCDN', signed_url: signed_url) } + + before do + stub_uploads_object_storage(uploader: cdn_uploader_class) + uploader.object_store = ObjectStorage::Store::REMOTE + uploader.store!(temp_file) + allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { false } + end + + it 'sends a file when CDN URL' do + expect(uploader).to receive(:use_cdn?).and_return(true) + expect(uploader).to receive(:cdn_provider).and_return(cdn_provider) + expect(controller).to receive(:request).and_return(request) + expect(controller).to receive(:redirect_to).with(signed_url) + + subject + end + end end end diff --git a/spec/controllers/confirmations_controller_spec.rb b/spec/controllers/confirmations_controller_spec.rb index 111bfb24c7e..773a416dcb4 100644 --- a/spec/controllers/confirmations_controller_spec.rb +++ b/spec/controllers/confirmations_controller_spec.rb @@ -10,17 +10,27 @@ RSpec.describe ConfirmationsController do end describe '#show' do + let_it_be_with_reload(:user) { create(:user, :unconfirmed) } + let(:confirmation_token) { user.confirmation_token } + render_views def perform_request get :show, params: { confirmation_token: confirmation_token } end - context 'user is already confirmed' do - let_it_be_with_reload(:user) { create(:user, :unconfirmed) } + context 'when signup info is required' do + before do + allow(controller).to receive(:current_user) { user } + user.set_role_required! + end - let(:confirmation_token) { user.confirmation_token } + it 'does not redirect' do + expect(perform_request).not_to redirect_to(users_sign_up_welcome_path) + end + end + context 'user is already confirmed' do before do user.confirm end @@ -57,10 +67,6 @@ RSpec.describe ConfirmationsController do end context 'user accesses the link after the expiry of confirmation token has passed' do - let_it_be_with_reload(:user) { create(:user, :unconfirmed) } - - let(:confirmation_token) { user.confirmation_token } - before do allow(Devise).to receive(:confirm_within).and_return(1.day) end @@ -133,6 +139,17 @@ RSpec.describe ConfirmationsController do stub_feature_flags(identity_verification: false) end + context 'when signup info is required' do + before do + allow(controller).to receive(:current_user) { user } + user.set_role_required! + end + + it 'does not redirect' do + expect(perform_request).not_to redirect_to(users_sign_up_welcome_path) + end + end + context 'when reCAPTCHA is disabled' do before do stub_application_setting(recaptcha_enabled: false) diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index 21810f64cb4..ea12b0c5ad7 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -41,20 +41,6 @@ RSpec.describe DashboardController do expect(assigns[:issues].map(&:id)).to include(task.id) end - - context 'when work_items is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it 'does not include tasks in issue list' do - task = create(:work_item, :task, project: project, author: user) - - get :issues, params: { author_id: user.id } - - expect(assigns[:issues].map(&:id)).not_to include(task.id) - end - end end describe 'GET merge requests' do diff --git a/spec/controllers/explore/groups_controller_spec.rb b/spec/controllers/explore/groups_controller_spec.rb index 310fe609cf1..a3bd8102462 100644 --- a/spec/controllers/explore/groups_controller_spec.rb +++ b/spec/controllers/explore/groups_controller_spec.rb @@ -9,31 +9,43 @@ RSpec.describe Explore::GroupsController do sign_in(user) end - it 'renders group trees' do - expect(described_class).to include(GroupTree) - end + shared_examples 'explore groups' do + it 'renders group trees' do + expect(described_class).to include(GroupTree) + end - it 'includes public projects' do - member_of_group = create(:group) - member_of_group.add_developer(user) - public_group = create(:group, :public) + it 'includes public projects' do + member_of_group = create(:group) + member_of_group.add_developer(user) + public_group = create(:group, :public) - get :index + get :index - expect(assigns(:groups)).to contain_exactly(member_of_group, public_group) - end + expect(assigns(:groups)).to contain_exactly(member_of_group, public_group) + end - context 'restricted visibility level is public' do - before do - sign_out(user) + context 'restricted visibility level is public' do + before do + sign_out(user) - stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + end + + it 'redirects to login page' do + get :index + + expect(response).to redirect_to new_user_session_path + end end + end - it 'redirects to login page' do - get :index + it_behaves_like 'explore groups' - expect(response).to redirect_to new_user_session_path + context 'generic_explore_groups flag is disabled' do + before do + stub_feature_flags(generic_explore_groups: false) end + + it_behaves_like 'explore groups' end end diff --git a/spec/controllers/explore/projects_controller_spec.rb b/spec/controllers/explore/projects_controller_spec.rb index bf578489916..5c977439af4 100644 --- a/spec/controllers/explore/projects_controller_spec.rb +++ b/spec/controllers/explore/projects_controller_spec.rb @@ -208,19 +208,26 @@ RSpec.describe Explore::ProjectsController do render_views # some N+1 queries still exist - it 'avoids N+1 queries' do - projects = create_list(:project, 3, :repository, :public) - projects.each do |project| - pipeline = create(:ci_pipeline, :success, project: project, sha: project.commit.id) - create(:commit_status, :success, pipeline: pipeline, ref: pipeline.ref) + it 'avoids N+1 queries', :request_store do + # Because we enable the request store for this spec, Gitaly may report too many invocations. + # Allow N+1s here and when creating additional objects below because we're just creating test objects. + Gitlab::GitalyClient.allow_n_plus_1_calls do + projects = create_list(:project, 3, :repository, :public) + + projects.each do |project| + pipeline = create(:ci_pipeline, :success, project: project, sha: project.commit.id) + create(:commit_status, :success, pipeline: pipeline, ref: pipeline.ref) + end end control = ActiveRecord::QueryRecorder.new { get endpoint } - new_projects = create_list(:project, 2, :repository, :public) - new_projects.each do |project| - pipeline = create(:ci_pipeline, :success, project: project, sha: project.commit.id) - create(:commit_status, :success, pipeline: pipeline, ref: pipeline.ref) + Gitlab::GitalyClient.allow_n_plus_1_calls do + new_projects = create_list(:project, 2, :repository, :public) + new_projects.each do |project| + pipeline = create(:ci_pipeline, :success, project: project, sha: project.commit.id) + create(:commit_status, :success, pipeline: pipeline, ref: pipeline.ref) + end end expect { get endpoint }.not_to exceed_query_limit(control).with_threshold(8) diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index 7c9236704ec..fe8b0291733 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -8,10 +8,6 @@ RSpec.describe GraphqlController do # two days is enough to make timezones irrelevant let_it_be(:last_activity_on) { 2.days.ago.to_date } - before do - stub_feature_flags(graphql: true) - end - describe 'rescue_from' do let_it_be(:message) { 'green ideas sleep furiously' } @@ -418,4 +414,114 @@ RSpec.describe GraphqlController do expect(log_payload.dig(:exception_object)).to eq(exception) end end + + describe 'removal of deprecated items' do + let(:mock_schema) do + Class.new(GraphQL::Schema) do + lazy_resolve ::Gitlab::Graphql::Lazy, :force + + query(Class.new(::Types::BaseObject) do + graphql_name 'Query' + + field :foo, GraphQL::Types::Boolean, + deprecated: { milestone: '0.1', reason: :renamed } + + field :bar, (Class.new(::Types::BaseEnum) do + graphql_name 'BarEnum' + + value 'FOOBAR', value: 'foobar', deprecated: { milestone: '0.1', reason: :renamed } + end) + + field :baz, GraphQL::Types::Boolean do + argument :arg, String, required: false, deprecated: { milestone: '0.1', reason: :renamed } + end + + def foo + false + end + + def bar + 'foobar' + end + + def baz(arg:) + false + end + end) + end + end + + before do + allow(GitlabSchema).to receive(:execute).and_wrap_original do |method, *args| + mock_schema.execute(*args) + end + end + + context 'without `remove_deprecated` param' do + let(:params) { { query: '{ foo bar baz(arg: "test") }' } } + + subject { post :execute, params: params } + + it "sets context's `remove_deprecated` value to false" do + subject + + expect(assigns(:context)[:remove_deprecated]).to be false + end + + it 'returns deprecated items in response' do + subject + + expect(json_response).to include('data' => { 'foo' => false, 'bar' => 'FOOBAR', 'baz' => false }) + end + end + + context 'with `remove_deprecated` param' do + let(:params) { { remove_deprecated: 'true' } } + + subject { post :execute, params: params } + + it "sets context's `remove_deprecated` value to true" do + subject + + expect(assigns(:context)[:remove_deprecated]).to be true + end + + it 'does not allow deprecated field' do + params[:query] = '{ foo }' + + subject + + expect(json_response).not_to include('data' => { 'foo' => false }) + expect(json_response).to include( + 'errors' => include(a_hash_including('message' => /Field 'foo' doesn't exist on type 'Query'/)) + ) + end + + it 'does not allow deprecated enum value' do + params[:query] = '{ bar }' + + subject + + expect(json_response).not_to include('data' => { 'bar' => 'FOOBAR' }) + expect(json_response).to include( + 'errors' => include( + a_hash_including( + 'message' => /`Query.bar` returned `"foobar"` at `bar`, but this isn't a valid value for `BarEnum`/ + ) + ) + ) + end + + it 'does not allow deprecated argument' do + params[:query] = '{ baz(arg: "test") }' + + subject + + expect(json_response).not_to include('data' => { 'bar' => 'FOOBAR' }) + expect(json_response).to include( + 'errors' => include(a_hash_including('message' => /Field 'baz' doesn't accept argument 'arg'/)) + ) + end + end + end end diff --git a/spec/controllers/groups/children_controller_spec.rb b/spec/controllers/groups/children_controller_spec.rb index 04cf7785f1e..f05551432fa 100644 --- a/spec/controllers/groups/children_controller_spec.rb +++ b/spec/controllers/groups/children_controller_spec.rb @@ -277,7 +277,7 @@ RSpec.describe Groups::ChildrenController do context 'with only projects' do let!(:other_project) { create(:project, :public, namespace: group) } - let!(:first_page_projects) { create_list(:project, per_page, :public, namespace: group ) } + let!(:first_page_projects) { create_list(:project, per_page, :public, namespace: group) } it 'has projects on the first page' do get :index, params: { group_id: group.to_param, sort: 'id_desc' }, format: :json diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index a3659ae9163..4e5dc01f466 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -342,6 +342,41 @@ RSpec.describe Groups::GroupMembersController do end end + context 'with owners from a parent' do + context 'when top-level group' do + context 'with group sharing' do + let!(:subgroup) { create(:group, parent: group) } + + before do + create(:group_group_link, :owner, shared_group: group, shared_with_group: subgroup) + create(:group_member, :owner, group: subgroup) + end + + it 'does not allow removal of last direct group owner' do + delete :leave, params: { group_id: group } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + context 'when subgroup' do + let!(:subgroup) { create(:group, parent: group) } + + before do + subgroup.add_owner(user) + end + + it 'allows removal of last direct group owner', :aggregate_failures do + delete :leave, params: { group_id: subgroup } + + expect(controller).to set_flash.to "You left the \"#{subgroup.human_name}\" group." + expect(response).to redirect_to(dashboard_groups_path) + expect(subgroup.users).not_to include user + end + end + end + context 'and there is another owner' do before do create(:group_member, :owner, source: group) diff --git a/spec/controllers/groups/registry/repositories_controller_spec.rb b/spec/controllers/groups/registry/repositories_controller_spec.rb index 9ac19b06718..62c15201a95 100644 --- a/spec/controllers/groups/registry/repositories_controller_spec.rb +++ b/spec/controllers/groups/registry/repositories_controller_spec.rb @@ -117,7 +117,7 @@ RSpec.describe Groups::Registry::RepositoriesController do it_behaves_like 'a package tracking event', described_class.name, 'list_repositories' context 'with project in subgroup' do - let_it_be(:test_group) { create(:group, parent: group ) } + let_it_be(:test_group) { create(:group, parent: group) } it_behaves_like 'renders a list of repositories' diff --git a/spec/controllers/groups/releases_controller_spec.rb b/spec/controllers/groups/releases_controller_spec.rb index 7dd0bc6206a..40e8cb4efc5 100644 --- a/spec/controllers/groups/releases_controller_spec.rb +++ b/spec/controllers/groups/releases_controller_spec.rb @@ -42,7 +42,7 @@ RSpec.describe Groups::ReleasesController do end it 'does not return any releases' do - expect(json_response.map { |r| r['tag'] } ).to be_empty + expect(json_response.map { |r| r['tag'] }).to be_empty end it 'returns OK' do @@ -56,7 +56,7 @@ RSpec.describe Groups::ReleasesController do index - expect(json_response.map { |r| r['tag'] } ).to match_array(%w(p2 p1 v2 v1)) + expect(json_response.map { |r| r['tag'] }).to match_array(%w(p2 p1 v2 v1)) end end diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb index 6dbf0803892..2add3cd3b18 100644 --- a/spec/controllers/groups/runners_controller_spec.rb +++ b/spec/controllers/groups/runners_controller_spec.rb @@ -168,7 +168,7 @@ RSpec.describe Groups::RunnersController do new_desc = runner.description.swapcase expect do - post :update, params: params.merge(runner: { description: new_desc } ) + post :update, params: params.merge(runner: { description: new_desc }) end.to change { runner.ensure_runner_queue_value } expect(response).to have_gitlab_http_status(:found) @@ -179,7 +179,7 @@ RSpec.describe Groups::RunnersController do new_desc = instance_runner.description.swapcase expect do - post :update, params: params_runner_instance.merge(runner: { description: new_desc } ) + post :update, params: params_runner_instance.merge(runner: { description: new_desc }) end.to not_change { instance_runner.ensure_runner_queue_value } .and not_change { instance_runner.description } @@ -190,7 +190,7 @@ RSpec.describe Groups::RunnersController do new_desc = project_runner.description.swapcase expect do - post :update, params: params_runner_project.merge(runner: { description: new_desc } ) + post :update, params: params_runner_project.merge(runner: { description: new_desc }) end.to change { project_runner.ensure_runner_queue_value } expect(response).to have_gitlab_http_status(:found) @@ -207,7 +207,7 @@ RSpec.describe Groups::RunnersController do old_desc = runner.description expect do - post :update, params: params.merge(runner: { description: old_desc.swapcase } ) + post :update, params: params.merge(runner: { description: old_desc.swapcase }) end.not_to change { runner.ensure_runner_queue_value } expect(response).to have_gitlab_http_status(:not_found) @@ -218,7 +218,7 @@ RSpec.describe Groups::RunnersController do old_desc = instance_runner.description expect do - post :update, params: params_runner_instance.merge(runner: { description: old_desc.swapcase } ) + post :update, params: params_runner_instance.merge(runner: { description: old_desc.swapcase }) end.not_to change { instance_runner.ensure_runner_queue_value } expect(response).to have_gitlab_http_status(:not_found) @@ -229,7 +229,7 @@ RSpec.describe Groups::RunnersController do old_desc = project_runner.description expect do - post :update, params: params_runner_project.merge(runner: { description: old_desc.swapcase } ) + post :update, params: params_runner_project.merge(runner: { description: old_desc.swapcase }) end.not_to change { project_runner.ensure_runner_queue_value } expect(response).to have_gitlab_http_status(:not_found) diff --git a/spec/controllers/groups/settings/repository_controller_spec.rb b/spec/controllers/groups/settings/repository_controller_spec.rb index cbf55218b94..73a205069f5 100644 --- a/spec/controllers/groups/settings/repository_controller_spec.rb +++ b/spec/controllers/groups/settings/repository_controller_spec.rb @@ -13,88 +13,73 @@ RSpec.describe Groups::Settings::RepositoryController do end describe 'POST create_deploy_token' do - context 'when ajax_new_deploy_token feature flag is disabled for the project' do - before do - stub_feature_flags(ajax_new_deploy_token: false) - entity.add_owner(user) - end + let(:good_deploy_token_params) do + { + name: 'name', + expires_at: 1.day.from_now.to_s, + username: 'deployer', + read_repository: '1', + deploy_token_type: DeployToken.deploy_token_types[:group_type] + } + end - it_behaves_like 'a created deploy token' do - let(:entity) { group } - let(:create_entity_params) { { group_id: group } } - let(:deploy_token_type) { DeployToken.deploy_token_types[:group_type] } - end + let(:request_params) do + { + group_id: group.to_param, + deploy_token: deploy_token_params + } end - context 'when ajax_new_deploy_token feature flag is enabled for the project' do - let(:good_deploy_token_params) do - { - name: 'name', - expires_at: 1.day.from_now.to_s, - username: 'deployer', - read_repository: '1', - deploy_token_type: DeployToken.deploy_token_types[:group_type] - } - end + before do + group.add_owner(user) + end + + subject { post :create_deploy_token, params: request_params, format: :json } - let(:request_params) do + context('a good request') do + let(:deploy_token_params) { good_deploy_token_params } + let(:expected_response) do { - group_id: group.to_param, - deploy_token: deploy_token_params + 'id' => be_a(Integer), + 'name' => deploy_token_params[:name], + 'username' => deploy_token_params[:username], + 'expires_at' => Time.zone.parse(deploy_token_params[:expires_at]), + 'token' => be_a(String), + 'expired' => false, + 'revoked' => false, + 'scopes' => deploy_token_params.inject([]) do |scopes, kv| + key, value = kv + key.to_s.start_with?('read_') && value.to_i != 0 ? scopes << key.to_s : scopes + end } end - before do - group.add_owner(user) - end + it 'creates the deploy token' do + subject - subject { post :create_deploy_token, params: request_params, format: :json } - - context('a good request') do - let(:deploy_token_params) { good_deploy_token_params } - let(:expected_response) do - { - 'id' => be_a(Integer), - 'name' => deploy_token_params[:name], - 'username' => deploy_token_params[:username], - 'expires_at' => Time.zone.parse(deploy_token_params[:expires_at]), - 'token' => be_a(String), - 'expired' => false, - 'revoked' => false, - 'scopes' => deploy_token_params.inject([]) do |scopes, kv| - key, value = kv - key.to_s.start_with?('read_') && value.to_i != 0 ? scopes << key.to_s : scopes - end - } - end - - it 'creates the deploy token' do - subject - - expect(response).to have_gitlab_http_status(:created) - expect(response).to match_response_schema('public_api/v4/deploy_token') - expect(json_response).to match(expected_response) - end + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/deploy_token') + expect(json_response).to match(expected_response) end + end - context('a bad request') do - let(:deploy_token_params) { good_deploy_token_params.except(:read_repository) } - let(:expected_response) { { 'message' => "Scopes can't be blank" } } + context('a bad request') do + let(:deploy_token_params) { good_deploy_token_params.except(:read_repository) } + let(:expected_response) { { 'message' => "Scopes can't be blank" } } - it 'does not create the deploy token' do - subject + it 'does not create the deploy token' do + subject - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response).to match(expected_response) - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to match(expected_response) end + end - context('an invalid request') do - let(:deploy_token_params) { good_deploy_token_params.except(:name) } + context('an invalid request') do + let(:deploy_token_params) { good_deploy_token_params.except(:name) } - it 'raises a validation error' do - expect { subject }.to raise_error(ActiveRecord::StatementInvalid) - end + it 'raises a validation error' do + expect { subject }.to raise_error(ActiveRecord::StatementInvalid) end end end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 5bbe236077c..22a406b3197 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -42,21 +42,15 @@ RSpec.describe GroupsController, factory_default: :keep do end end - shared_examples 'details view' do - let(:namespace) { group } + shared_examples 'details view as atom' do + let!(:event) { create(:event, project: project) } + let(:format) { :atom } it { is_expected.to render_template('groups/show') } - context 'as atom' do - let!(:event) { create(:event, project: project) } - let(:format) { :atom } - - it { is_expected.to render_template('groups/show') } - - it 'assigns events for all the projects in the group', :sidekiq_might_not_need_inline do - subject - expect(assigns(:events).map(&:id)).to contain_exactly(event.id) - end + it 'assigns events for all the projects in the group' do + subject + expect(assigns(:events).map(&:id)).to contain_exactly(event.id) end end @@ -70,7 +64,9 @@ RSpec.describe GroupsController, factory_default: :keep do subject { get :show, params: { id: group.to_param }, format: format } context 'when the group is not importing' do - it_behaves_like 'details view' + it { is_expected.to render_template('groups/show') } + + it_behaves_like 'details view as atom' it 'tracks page views', :snowplow do subject @@ -115,7 +111,9 @@ RSpec.describe GroupsController, factory_default: :keep do subject { get :details, params: { id: group.to_param }, format: format } - it_behaves_like 'details view' + it { is_expected.to redirect_to(group_path(group)) } + + it_behaves_like 'details view as atom' end describe 'GET edit' do @@ -672,7 +670,7 @@ RSpec.describe GroupsController, factory_default: :keep do end context 'when there is a conflicting group path' do - let!(:conflict_group) { create(:group, path: SecureRandom.hex(12) ) } + let!(:conflict_group) { create(:group, path: SecureRandom.hex(12)) } let!(:old_name) { group.name } it 'does not render references to the conflicting group' do diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index fb90a70d91d..5185aa64d9f 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -213,6 +213,75 @@ RSpec.describe Oauth::AuthorizationsController do expect(response).to redirect_to(new_user_session_path) end end + + context 'when the user is admin' do + context 'when disable_admin_oauth_scopes is set' do + before do + stub_application_setting(disable_admin_oauth_scopes: true) + scopes = Doorkeeper::OAuth::Scopes.from_string('api') + + allow(Doorkeeper.configuration).to receive(:scopes).and_return(scopes) + end + + let(:user) { create(:user, :admin) } + + it 'returns 200 and renders forbidden view' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/forbidden') + end + end + + context 'when disable_admin_oauth_scopes is set and the application is trusted' do + before do + stub_application_setting(disable_admin_oauth_scopes: true) + + application.update!(trusted: true) + end + + let(:application_scopes) { 'api' } + let(:user) { create(:user, :admin) } + + it 'returns 200 and renders redirect view' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/redirect') + end + end + + context 'when disable_admin_oauth_scopes is disabled' do + before do + stub_application_setting(disable_admin_oauth_scopes: false) + end + + let(:application_scopes) { 'api' } + let(:user) { create(:user, :admin) } + + it 'returns 200 and renders new view' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/new') + end + end + end + + context 'when the user is not admin' do + context 'when disable_admin_oauth_scopes is enabled' do + before do + stub_application_setting(disable_admin_oauth_scopes: true) + end + + it 'returns 200 and renders new view' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('doorkeeper/authorizations/new') + end + end + end end describe 'POST #create' do diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index df5da29495e..0560ccb25dd 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -10,7 +10,7 @@ RSpec.describe OmniauthCallbacksController, type: :controller do let(:additional_info) { {} } before do - @original_env_config_omniauth_auth = mock_auth_hash(provider.to_s, extern_uid, user.email, additional_info: additional_info ) + @original_env_config_omniauth_auth = mock_auth_hash(provider.to_s, extern_uid, user.email, additional_info: additional_info) stub_omniauth_provider(provider, context: request) end diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb index e4be2fbef3c..9494f55c631 100644 --- a/spec/controllers/passwords_controller_spec.rb +++ b/spec/controllers/passwords_controller_spec.rb @@ -78,6 +78,22 @@ RSpec.describe PasswordsController do end end + context 'password is weak' do + let(:password) { "password" } + + it 'tracks the event' do + subject + + expect(response.body).to have_content("must not contain commonly used combinations of words and letters") + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'PasswordsController', + method: 'create' + ) + end + end + it 'sets the username and caller_id in the context' do expect(controller).to receive(:update).and_wrap_original do |m, *args| m.call(*args) diff --git a/spec/controllers/profiles/personal_access_tokens_controller_spec.rb b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb index 8dee0490fd6..044ce8f397a 100644 --- a/spec/controllers/profiles/personal_access_tokens_controller_spec.rb +++ b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' RSpec.describe Profiles::PersonalAccessTokensController do - let(:user) { create(:user) } + let(:access_token_user) { create(:user) } let(:token_attributes) { attributes_for(:personal_access_token) } before do - sign_in(user) + sign_in(access_token_user) end describe '#create' do @@ -49,13 +49,27 @@ RSpec.describe Profiles::PersonalAccessTokensController do end end + describe 'GET /-/profile/personal_access_tokens' do + let(:get_access_tokens) do + get :index + response + end + + subject(:get_access_tokens_with_page) do + get :index, params: { page: 1 } + response + end + + it_behaves_like 'GET access tokens are paginated and ordered' + end + describe '#index' do - let!(:active_personal_access_token) { create(:personal_access_token, user: user) } + let!(:active_personal_access_token) { create(:personal_access_token, user: access_token_user) } before do # Impersonation and inactive personal tokens are ignored - create(:personal_access_token, :impersonation, user: user) - create(:personal_access_token, :revoked, user: user) + create(:personal_access_token, :impersonation, user: access_token_user) + create(:personal_access_token, :revoked, user: access_token_user) get :index end @@ -63,7 +77,7 @@ RSpec.describe Profiles::PersonalAccessTokensController do active_personal_access_tokens_detail = ::PersonalAccessTokenSerializer.new.represent([active_personal_access_token]) - expect(assigns(:active_personal_access_tokens).to_json).to eq(active_personal_access_tokens_detail.to_json) + expect(assigns(:active_access_tokens).to_json).to eq(active_personal_access_tokens_detail.to_json) end it "sets PAT name and scopes" do @@ -86,73 +100,10 @@ RSpec.describe Profiles::PersonalAccessTokensController do expect(response).to have_gitlab_http_status(:not_found) end - context "access_token_pagination feature flag is enabled" do - before do - stub_feature_flags(access_token_pagination: true) - allow(Kaminari.config).to receive(:default_per_page).and_return(1) - create(:personal_access_token, user: user) - end - - it "returns paginated response" do - get :index, params: { page: 1 } - expect(assigns(:active_personal_access_tokens).count).to eq(1) - end - - it 'adds appropriate headers' do - get :index, params: { page: 1 } - expect_header('X-Per-Page', '1') - expect_header('X-Page', '1') - expect_header('X-Next-Page', '2') - expect_header('X-Total', '2') - end - end - - context "tokens returned are ordered" do - let(:expires_1_day_from_now) { 1.day.from_now.to_date } - let(:expires_2_day_from_now) { 2.days.from_now.to_date } - - before do - create(:personal_access_token, user: user, name: "Token1", expires_at: expires_1_day_from_now) - create(:personal_access_token, user: user, name: "Token2", expires_at: expires_2_day_from_now) - end - - it "orders token list ascending on expires_at" do - get :index - - first_token = assigns(:active_personal_access_tokens).first.as_json - expect(first_token['name']).to eq("Token1") - expect(first_token['expires_at']).to eq(expires_1_day_from_now.strftime("%Y-%m-%d")) - end - - it "orders tokens on id in case token has same expires_at" do - create(:personal_access_token, user: user, name: "Token3", expires_at: expires_1_day_from_now) - - get :index - - first_token = assigns(:active_personal_access_tokens).first.as_json - expect(first_token['name']).to eq("Token3") - expect(first_token['expires_at']).to eq(expires_1_day_from_now.strftime("%Y-%m-%d")) - - second_token = assigns(:active_personal_access_tokens).second.as_json - expect(second_token['name']).to eq("Token1") - expect(second_token['expires_at']).to eq(expires_1_day_from_now.strftime("%Y-%m-%d")) - end - end - - context "access_token_pagination feature flag is disabled" do - before do - stub_feature_flags(access_token_pagination: false) - create(:personal_access_token, user: user) - end + it 'returns tokens for json format' do + get :index, params: { format: :json } - it "returns all tokens in system" do - get :index, params: { page: 1 } - expect(assigns(:active_personal_access_tokens).count).to eq(2) - end + expect(json_response.count).to eq(1) end end - - def expect_header(header_name, header_val) - expect(response.headers[header_name]).to eq(header_val) - end end diff --git a/spec/controllers/projects/alerting/notifications_controller_spec.rb b/spec/controllers/projects/alerting/notifications_controller_spec.rb index b3feeb7c07b..5ce2950f95f 100644 --- a/spec/controllers/projects/alerting/notifications_controller_spec.rb +++ b/spec/controllers/projects/alerting/notifications_controller_spec.rb @@ -16,9 +16,6 @@ RSpec.describe Projects::Alerting::NotificationsController do end shared_examples 'process alert payload' do |notify_service_class| - let(:alert_1) { build(:alert_management_alert, project: project) } - let(:alert_2) { build(:alert_management_alert, project: project) } - let(:service_response) { ServiceResponse.success(payload: { alerts: [alert_1, alert_2] }) } let(:notify_service) { instance_double(notify_service_class, execute: service_response) } before do @@ -35,11 +32,14 @@ RSpec.describe Projects::Alerting::NotificationsController do it 'responds with the alert data' do make_request - expect(json_response).to contain_exactly( - { 'iid' => alert_1.iid, 'title' => alert_1.title }, - { 'iid' => alert_2.iid, 'title' => alert_2.title } - ) - expect(response).to have_gitlab_http_status(:ok) + if service_response.payload.present? + expect(json_response).to contain_exactly( + { 'iid' => alert_1.iid, 'title' => alert_1.title }, + { 'iid' => alert_2.iid, 'title' => alert_2.title } + ) + end + + expect(response).to have_gitlab_http_status(service_response.http_status) end it 'does not pass excluded parameters to the notify service' do @@ -146,6 +146,9 @@ RSpec.describe Projects::Alerting::NotificationsController do context 'with generic alert payload' do it_behaves_like 'process alert payload', Projects::Alerting::NotifyService do + let(:alert_1) { build(:alert_management_alert, project: project) } + let(:alert_2) { build(:alert_management_alert, project: project) } + let(:service_response) { ServiceResponse.success(payload: { alerts: [alert_1, alert_2] }) } let(:payload) { { title: 'Alert title' } } end end @@ -154,6 +157,7 @@ RSpec.describe Projects::Alerting::NotificationsController do include PrometheusHelpers it_behaves_like 'process alert payload', Projects::Prometheus::Alerts::NotifyService do + let(:service_response) { ServiceResponse.success(http_status: :created) } let(:payload) { prometheus_alert_payload } end end diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index f79a2c6a6d0..00efd7d7b56 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -30,28 +30,10 @@ RSpec.describe Projects::ArtifactsController do stub_feature_flags(artifacts_management_page: true) end - it 'sets the artifacts variable' do + it 'renders the page' do subject - expect(assigns(:artifacts)).to contain_exactly(*project.job_artifacts) - end - - it 'sets the total size variable' do - subject - - expect(assigns(:total_size)).to eq(project.job_artifacts.total_size) - end - - describe 'pagination' do - before do - stub_const("#{described_class}::MAX_PER_PAGE", 1) - end - - it 'paginates artifacts' do - subject - - expect(assigns(:artifacts)).to contain_exactly(project.reload.job_artifacts.last) - end + expect(response).to have_gitlab_http_status(:ok) end end @@ -65,18 +47,6 @@ RSpec.describe Projects::ArtifactsController do expect(response).to have_gitlab_http_status(:no_content) end - - it 'does not set the artifacts variable' do - subject - - expect(assigns(:artifacts)).to eq(nil) - end - - it 'does not set the total size variable' do - subject - - expect(assigns(:total_size)).to eq(nil) - end end end @@ -183,12 +153,17 @@ RSpec.describe Projects::ArtifactsController do end context 'when file is stored remotely' do + let(:cdn_config) {} + before do - stub_artifacts_object_storage + stub_artifacts_object_storage(cdn: cdn_config) create(:ci_job_artifact, :remote_store, :codequality, job: job) + allow(Gitlab::ApplicationContext).to receive(:push).and_call_original end it 'sends the codequality report' do + expect(Gitlab::ApplicationContext).to receive(:push).with(artifact: an_instance_of(Ci::JobArtifact)).and_call_original + expect(controller).to receive(:redirect_to).and_call_original download_artifact(file_type: file_type) @@ -201,6 +176,30 @@ RSpec.describe Projects::ArtifactsController do download_artifact(file_type: file_type, proxy: true) end end + + context 'when Google CDN is configured' do + let(:cdn_config) do + { + 'provider' => 'Google', + 'url' => 'https://cdn.example.org', + 'key_name' => 'some-key', + 'key' => Base64.urlsafe_encode64(SecureRandom.hex) + } + end + + before do + request.env['action_dispatch.remote_ip'] = '18.245.0.42' + end + + it 'redirects to a Google CDN request' do + expect(Gitlab::ApplicationContext).to receive(:push).with(artifact: an_instance_of(Ci::JobArtifact)).and_call_original + expect(Gitlab::ApplicationContext).to receive(:push).with(artifact_used_cdn: true).and_call_original + + download_artifact(file_type: file_type) + + expect(response.redirect_url).to start_with("https://cdn.example.org/") + end + end end end end @@ -228,8 +227,9 @@ RSpec.describe Projects::ArtifactsController do expect(response).to have_gitlab_http_status(:forbidden) expect(response.body).to include( 'You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. ' \ - 'To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline configuration or CI/CD settings. ' \ - 'If you need to view this job log, a project maintainer or owner must add you to the project with developer permissions or higher.' + 'To disable debug trace, set the 'CI_DEBUG_TRACE' and 'CI_DEBUG_SERVICES' variables to 'false' ' \ + 'in your pipeline configuration or CI/CD settings. If you must view this job log, a project maintainer or owner must ' \ + 'add you to the project with developer permissions or higher.' ) end end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 16a43bae674..5927f20df97 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -76,7 +76,7 @@ RSpec.describe Projects::EnvironmentsController do it 'handles search option properly' do get :index, params: environment_params(format: :json, search: 'staging/r') - expect(environments.map { |env| env['name'] } ).to contain_exactly('staging/review-1', 'staging/review-2') + expect(environments.map { |env| env['name'] }).to contain_exactly('staging/review-1', 'staging/review-2') expect(json_response['available_count']).to eq 2 expect(json_response['stopped_count']).to eq 1 end @@ -84,7 +84,7 @@ RSpec.describe Projects::EnvironmentsController do it 'ignores search option if is shorter than a minimum' do get :index, params: environment_params(format: :json, search: 'st') - expect(environments.map { |env| env['name'] } ).to contain_exactly('production', + expect(environments.map { |env| env['name'] }).to contain_exactly('production', 'staging/review-1', 'staging/review-2') expect(json_response['available_count']).to eq 3 @@ -233,7 +233,7 @@ RSpec.describe Projects::EnvironmentsController do search: 'staging-1.0/z' }, format: :json) - expect(environments.map { |env| env['name'] } ).to eq(['staging-1.0/zzz']) + expect(environments.map { |env| env['name'] }).to eq(['staging-1.0/zzz']) expect(json_response['available_count']).to eq 1 expect(json_response['stopped_count']).to eq 0 end @@ -705,7 +705,7 @@ RSpec.describe Projects::EnvironmentsController do expect(json_response).to have_key('all_dashboards') expect(json_response['all_dashboards']).to be_an_instance_of(Array) - expect(json_response['all_dashboards']).to all( include('path', 'default', 'display_name') ) + expect(json_response['all_dashboards']).to all(include('path', 'default', 'display_name')) end end diff --git a/spec/controllers/projects/hooks_controller_spec.rb b/spec/controllers/projects/hooks_controller_spec.rb index ba7b712964c..18f16937505 100644 --- a/spec/controllers/projects/hooks_controller_spec.rb +++ b/spec/controllers/projects/hooks_controller_spec.rb @@ -29,6 +29,22 @@ RSpec.describe Projects::HooksController do { namespace_id: project.namespace, project_id: project, id: hook.id } end + context 'with an existing token' do + hook_params = { + token: WebHook::SECRET_MASK, + url: "http://example.com" + } + + it 'does not change a token' do + expect do + post :update, params: params.merge({ hook: hook_params }) + end.not_to change { hook.reload.token } + + expect(response).to have_gitlab_http_status(:found) + expect(flash[:alert]).to be_blank + end + end + it 'adds, updates and deletes URL variables' do hook.update!(url_variables: { 'a' => 'bar', 'b' => 'woo' }) @@ -106,8 +122,9 @@ RSpec.describe Projects::HooksController do it 'sets all parameters' do hook_params = { enable_ssl_verification: true, - token: "TEST TOKEN", - url: "http://example.com", + token: 'TEST TOKEN', + url: 'http://example.com', + branch_filter_strategy: 'regex', push_events: true, tag_push_events: true, @@ -124,13 +141,39 @@ RSpec.describe Projects::HooksController do url_variables: [{ key: 'token', value: 'some secret value' }] } - post :create, params: { namespace_id: project.namespace, project_id: project, hook: hook_params } + params = { namespace_id: project.namespace, project_id: project, hook: hook_params } + + expect { post :create, params: params }.to change(ProjectHook, :count).by(1) + + project_hook = ProjectHook.order_id_desc.take + + expect(project_hook).to have_attributes( + **hook_params.merge(url_variables: { 'token' => 'some secret value' }) + ) + expect(response).to have_gitlab_http_status(:found) + expect(flash[:alert]).to be_blank + end + + it 'ignores branch_filter_strategy when flag is disabled' do + stub_feature_flags(enhanced_webhook_support_regex: false) + hook_params = { + url: 'http://example.com', + branch_filter_strategy: 'regex', + push_events: true + } + params = { namespace_id: project.namespace, project_id: project, hook: hook_params } + + expect { post :create, params: params }.to change(ProjectHook, :count).by(1) + + project_hook = ProjectHook.order_id_desc.take + + expect(project_hook).to have_attributes( + url: 'http://example.com', + branch_filter_strategy: 'wildcard' + ) expect(response).to have_gitlab_http_status(:found) expect(flash[:alert]).to be_blank - expect(ProjectHook.count).to eq(1) - expect(ProjectHook.first).to have_attributes(hook_params.except(:url_variables)) - expect(ProjectHook.first).to have_attributes(url_variables: { 'token' => 'some secret value' }) end it 'alerts the user if the new hook is invalid' do @@ -186,7 +229,7 @@ RSpec.describe Projects::HooksController do context 'when the hook fails completely' do before do allow_next(::TestHooks::ProjectService) - .to receive(:execute).and_return({ message: 'All is woe' }) + .to receive(:execute).and_return(ServiceResponse.error(message: 'All is woe')) end it 'informs the user' do @@ -204,7 +247,7 @@ RSpec.describe Projects::HooksController do it 'prevents making test requests' do expect_next_instance_of(TestHooks::ProjectService) do |service| - expect(service).to receive(:execute).and_return(http_status: 200) + expect(service).to receive(:execute).and_return(ServiceResponse.success(payload: { http_status: 200 })) end 2.times { post :test, params: { namespace_id: project.namespace, project_id: project, id: hook } } diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 0c3795540e0..8f26be442a7 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -168,75 +168,56 @@ RSpec.describe Projects::IssuesController do let_it_be(:task) { create(:issue, :task, project: project) } - context 'when work_items feature flag is enabled' do - shared_examples 'redirects to show work item page' do - it 'redirects to work item page' do - expect(response).to redirect_to(project_work_items_path(project, task.id, query)) - end - end - - context 'show action' do - let(:query) { { query: 'any' } } - + shared_examples 'redirects to show work item page' do + context 'when use_iid_in_work_items_path feature flag is disabled' do before do - get :show, params: { namespace_id: project.namespace, project_id: project, id: task.iid, **query } + stub_feature_flags(use_iid_in_work_items_path: false) end - it_behaves_like 'redirects to show work item page' - end - - context 'edit action' do - let(:query) { { query: 'any' } } + it 'redirects to work item page' do + make_request - before do - get :edit, params: { namespace_id: project.namespace, project_id: project, id: task.iid, **query } + expect(response).to redirect_to(project_work_items_path(project, task.id, query)) end - - it_behaves_like 'redirects to show work item page' end - context 'update action' do - before do - put :update, params: { namespace_id: project.namespace, project_id: project, id: task.iid, issue: { title: 'New title' } } - end + it 'redirects to work item page using iid' do + make_request - it_behaves_like 'redirects to show work item page' + expect(response).to redirect_to(project_work_items_path(project, task.iid, query.merge(iid_path: true))) end end - context 'when work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) - end + context 'show action' do + let(:query) { { query: 'any' } } - shared_examples 'renders 404' do - it 'renders 404 for show action' do - expect(response).to have_gitlab_http_status(:not_found) + it_behaves_like 'redirects to show work item page' do + subject(:make_request) do + get :show, params: { namespace_id: project.namespace, project_id: project, id: task.iid, **query } end end + end - context 'show action' do - before do - get :show, params: { namespace_id: project.namespace, project_id: project, id: task.iid } - end - - it_behaves_like 'renders 404' - end + context 'edit action' do + let(:query) { { query: 'any' } } - context 'edit action' do - before do - get :edit, params: { namespace_id: project.namespace, project_id: project, id: task.iid } + it_behaves_like 'redirects to show work item page' do + subject(:make_request) do + get :edit, params: { namespace_id: project.namespace, project_id: project, id: task.iid, **query } end - - it_behaves_like 'renders 404' end + end - context 'update action' do - before do - put :update, params: { namespace_id: project.namespace, project_id: project, id: task.iid, issue: { title: 'New title' } } + context 'update action' do + it_behaves_like 'redirects to show work item page' do + subject(:make_request) do + put :update, params: { + namespace_id: project.namespace, + project_id: project, + id: task.iid, + issue: { title: 'New title' } + } end - - it_behaves_like 'renders 404' end end end @@ -1107,6 +1088,24 @@ RSpec.describe Projects::IssuesController do end end + context 'when trying to create a objective' do + it 'defaults to issue type' do + issue = post_new_issue(issue_type: 'objective') + + expect(issue.issue_type).to eq('issue') + expect(issue.work_item_type.base_type).to eq('issue') + end + end + + context 'when trying to create a key_result' do + it 'defaults to issue type' do + issue = post_new_issue(issue_type: 'key_result') + + expect(issue.issue_type).to eq('issue') + expect(issue.work_item_type.base_type).to eq('issue') + end + end + context 'when create service return an unrecoverable error with http_status' do let(:http_status) { 403 } @@ -1291,7 +1290,7 @@ RSpec.describe Projects::IssuesController do let!(:last_spam_log) { spam_logs.last } def post_verified_issue - post_new_issue({}, { spam_log_id: last_spam_log.id, 'g-recaptcha-response': 'abc123' } ) + post_new_issue({}, { spam_log_id: last_spam_log.id, 'g-recaptcha-response': 'abc123' }) end before do @@ -1311,7 +1310,7 @@ RSpec.describe Projects::IssuesController do it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do spam_log = create(:spam_log) - expect { post_new_issue({}, { spam_log_id: spam_log.id, 'g-recaptcha-response': true } ) } + expect { post_new_issue({}, { spam_log_id: spam_log.id, 'g-recaptcha-response': true }) } .not_to change { last_spam_log.recaptcha_verified } end end @@ -1709,19 +1708,6 @@ RSpec.describe Projects::IssuesController do expect(response).to redirect_to(project_issues_path(project)) expect(controller).to set_flash[:notice].to match(/\AYour CSV export has started/i) end - - context 'when work_items is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it 'does not include tasks in CSV export' do - expect(IssuableExportCsvWorker).to receive(:perform_async) - .with(:issue, viewer.id, project.id, hash_including('issue_types' => Issue::TYPES_FOR_LIST.excluding('task'))) - - request_csv - end - end end context 'when not logged in' do diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 556dd23c135..3dc89365530 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -660,6 +660,38 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do end end end + + context 'when CI_DEBUG_SERVICES enabled' do + let!(:variable) { create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: 'true') } + + context 'with proper permissions on a project' do + let(:user) { developer } + + before do + sign_in(user) + end + + it 'returns response ok' do + get_trace + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'without proper permissions for debug logging' do + let(:user) { guest } + + before do + sign_in(user) + end + + it 'returns response forbidden' do + get_trace + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end end context 'when job has a live trace' do @@ -1184,36 +1216,51 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" end - context 'when CI_DEBUG_TRACE enabled' do - before do - create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: 'true') + context 'when CI_DEBUG_TRACE and/or CI_DEBUG_SERVICES are enabled' do + using RSpec::Parameterized::TableSyntax + where(:ci_debug_trace, :ci_debug_services) do + 'true' | 'true' + 'true' | 'false' + 'false' | 'true' + 'false' | 'false' end - context 'with proper permissions for debug logging on a project' do - let(:user) { developer } - + with_them do before do - sign_in(user) + create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: ci_debug_trace) + create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: ci_debug_services) end - it 'returns response ok' do - response = subject + context 'with proper permissions for debug logging on a project' do + let(:user) { developer } - expect(response).to have_gitlab_http_status(:ok) - end - end + before do + sign_in(user) + end - context 'without proper permissions for debug logging on a project' do - let(:user) { reporter } + it 'returns response ok' do + response = subject - before do - sign_in(user) + expect(response).to have_gitlab_http_status(:ok) + end end - it 'returns response forbidden' do - response = subject + context 'without proper permissions for debug logging on a project' do + let(:user) { reporter } - expect(response).to have_gitlab_http_status(:forbidden) + before do + sign_in(user) + end + + it 'returns response forbidden if dev mode enabled' do + response = subject + + if ci_debug_trace == 'true' || ci_debug_services == 'true' + expect(response).to have_gitlab_http_status(:forbidden) + else + expect(response).to have_gitlab_http_status(:ok) + end + end end end end @@ -1380,7 +1427,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do { 'Channel' => { 'Subprotocols' => ["terminal.gitlab.com"], - 'Url' => 'wss://localhost/proxy/build/default_port/', + 'Url' => 'wss://gitlab.example.com/proxy/build/default_port/', 'Header' => { 'Authorization' => [nil] }, @@ -1536,7 +1583,8 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do allow(Gitlab::Workhorse).to receive(:verify_api_request!).and_return(nil) expect(job.runner_session_url).to start_with('https://') - expect(Gitlab::Workhorse).to receive(:channel_websocket).with(a_hash_including(url: "wss://localhost/proxy/build/default_port/")) + expect(Gitlab::Workhorse).to receive(:channel_websocket) + .with(a_hash_including(url: "wss://gitlab.example.com/proxy/build/default_port/")) make_request end diff --git a/spec/controllers/projects/learn_gitlab_controller_spec.rb b/spec/controllers/projects/learn_gitlab_controller_spec.rb index 2d00fcbccf3..a93da82d948 100644 --- a/spec/controllers/projects/learn_gitlab_controller_spec.rb +++ b/spec/controllers/projects/learn_gitlab_controller_spec.rb @@ -34,8 +34,15 @@ RSpec.describe Projects::LearnGitlabController do it { is_expected.to have_gitlab_http_status(:not_found) } end - it_behaves_like 'tracks assignment and records the subject', :invite_for_help_continuous_onboarding, :namespace do - subject { project.namespace } + context 'with invite_for_help_continuous_onboarding experiment' do + it 'tracks the assignment', :experiment do + stub_experiments(invite_for_help_continuous_onboarding: true) + + expect(experiment(:invite_for_help_continuous_onboarding)) + .to track(:assignment).with_context(namespace: project.namespace).on_next_instance + + action + end end end end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 367781c0e76..613d82efd06 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -213,7 +213,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, merge_ref_head_diff: false } end @@ -281,7 +281,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, merge_ref_head_diff: nil } end @@ -303,7 +303,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: merge_request.diff_head_commit, latest_diff: nil, only_context_commits: false, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, merge_ref_head_diff: nil } end @@ -329,7 +329,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - allow_tree_conflicts: false, + merge_conflicts_in_diff: false, merge_ref_head_diff: nil } end @@ -488,7 +488,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - allow_tree_conflicts: true, + merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) @@ -616,7 +616,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do 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) } + let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_conflicts_in_diff: false) } end it_behaves_like 'successful request' diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index a41abd8c16d..026cf19bde5 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Projects::MergeRequestsController do include ProjectForksHelper include Gitlab::Routing + using RSpec::Parameterized::TableSyntax let_it_be_with_refind(:project) { create(:project, :repository) } let_it_be_with_reload(:project_public_with_private_builds) { create(:project, :repository, :public, :builds_private) } @@ -708,12 +709,14 @@ RSpec.describe Projects::MergeRequestsController do end describe 'GET commits' do - def go(format: 'html') + def go(page: nil, per_page: 1, format: 'html') get :commits, params: { namespace_id: project.namespace.to_param, project_id: project, - id: merge_request.iid + id: merge_request.iid, + page: page, + per_page: per_page }, format: format end @@ -723,6 +726,27 @@ RSpec.describe Projects::MergeRequestsController do expect(response).to render_template('projects/merge_requests/_commits') expect(json_response).to have_key('html') + expect(json_response).to have_key('next_page') + expect(json_response['next_page']).to eq(2) + end + + describe 'pagination' do + where(:page, :next_page) do + 1 | 2 + 2 | 3 + 3 | nil + end + + with_them do + it "renders the commits for page #{params[:page]}" do + go format: 'json', page: page, per_page: 10 + + expect(response).to render_template('projects/merge_requests/_commits') + expect(json_response).to have_key('html') + expect(json_response).to have_key('next_page') + expect(json_response['next_page']).to eq(next_page) + end + end end end @@ -1756,7 +1780,7 @@ RSpec.describe Projects::MergeRequestsController do end it 'renders MergeRequest as JSON' do - expect(json_response.keys).to include('id', 'iid', 'title', 'has_ci', 'merge_status', 'can_be_merged', 'current_user') + expect(json_response.keys).to include('id', 'iid', 'title', 'has_ci', 'current_user') end end @@ -1790,7 +1814,7 @@ RSpec.describe Projects::MergeRequestsController do it 'renders MergeRequest as JSON' do subject - expect(json_response.keys).to include('id', 'iid', 'title', 'has_ci', 'merge_status', 'can_be_merged', 'current_user') + expect(json_response.keys).to include('id', 'iid', 'title', 'has_ci', 'current_user') end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index b132c0b5a69..f66e4b133ca 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -20,23 +20,11 @@ RSpec.describe Projects::PipelinesController do end shared_examples 'the show page' do |param| - it 'redirects to pipeline path with param' do + it 'renders the show template' do get param, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - expect(response).to redirect_to(pipeline_path(pipeline, tab: param)) - end - - context 'when the FF pipeline_tabs_vue is disabled' do - before do - stub_feature_flags(pipeline_tabs_vue: false) - end - - it 'renders the show template' do - get param, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template :show - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show end end @@ -311,14 +299,15 @@ RSpec.describe Projects::PipelinesController do stub_application_setting(auto_devops_enabled: false) end - def action - get :index, params: { namespace_id: project.namespace, project_id: project } - end + context 'with runners_availability_section experiment' do + it 'tracks the assignment', :experiment do + stub_experiments(runners_availability_section: true) - subject { project.namespace } + expect(experiment(:runners_availability_section)) + .to track(:assignment).with_context(namespace: project.namespace).on_next_instance - context 'runners_availability_section experiment' do - it_behaves_like 'tracks assignment and records the subject', :runners_availability_section, :namespace + get :index, params: { namespace_id: project.namespace, project_id: project } + end end end @@ -710,37 +699,25 @@ RSpec.describe Projects::PipelinesController do describe 'GET failures' do let(:pipeline) { create(:ci_pipeline, project: project) } - context 'with ff `pipeline_tabs_vue` disabled' do + context 'with failed jobs' do before do - stub_feature_flags(pipeline_tabs_vue: false) + create(:ci_build, :failed, pipeline: pipeline, name: 'hello') end - context 'with failed jobs' do - before do - create(:ci_build, :failed, pipeline: pipeline, name: 'hello') - end - - it 'shows the page' do - get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template :show - end - end - - context 'without failed jobs' do - it 'redirects to the main pipeline page' do - get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } + it 'shows the page' do + get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - expect(response).to redirect_to(pipeline_path(pipeline)) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show end end - it 'redirects to the pipeline page with `failures` query param' do - get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } + context 'without failed jobs' do + it 'redirects to the main pipeline page' do + get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - expect(response).to redirect_to(pipeline_path(pipeline, tab: 'failures')) + expect(response).to redirect_to(pipeline_path(pipeline)) + end end end diff --git a/spec/controllers/projects/product_analytics_controller_spec.rb b/spec/controllers/projects/product_analytics_controller_spec.rb deleted file mode 100644 index 47f1d96c70b..00000000000 --- a/spec/controllers/projects/product_analytics_controller_spec.rb +++ /dev/null @@ -1,95 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::ProductAnalyticsController do - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } - - before(:all) do - project.add_maintainer(user) - end - - before do - sign_in(user) - stub_feature_flags(product_analytics: true) - end - - describe 'GET #index' do - it 'renders index with 200 status code' do - get :index, params: project_params - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index) - end - - context 'with an anonymous user' do - before do - sign_out(user) - end - - it 'redirects to sign-in page' do - get :index, params: project_params - - expect(response).to redirect_to(new_user_session_path) - end - end - - context 'feature flag disabled' do - before do - stub_feature_flags(product_analytics: false) - end - - it 'returns not found' do - get :index, params: project_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - describe 'GET #test' do - it 'renders test with 200 status code' do - get :test, params: project_params - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:test) - end - end - - describe 'GET #setup' do - it 'renders setup with 200 status code' do - get :setup, params: project_params - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:setup) - end - end - - describe 'GET #graphs' do - it 'renders graphs with 200 status code' do - get :graphs, params: project_params - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:graphs) - end - - context 'feature flag disabled' do - before do - stub_feature_flags(product_analytics: false) - end - - it 'returns not found' do - get :graphs, params: project_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - private - - def project_params(opts = {}) - opts.reverse_merge(namespace_id: project.namespace, project_id: project) - end -end diff --git a/spec/controllers/projects/prometheus/alerts_controller_spec.rb b/spec/controllers/projects/prometheus/alerts_controller_spec.rb index 2c2c8180143..09b9f25c0c6 100644 --- a/spec/controllers/projects/prometheus/alerts_controller_spec.rb +++ b/spec/controllers/projects/prometheus/alerts_controller_spec.rb @@ -56,7 +56,7 @@ RSpec.describe Projects::Prometheus::AlertsController do describe 'POST #notify' do let(:alert_1) { build(:alert_management_alert, :prometheus, project: project) } let(:alert_2) { build(:alert_management_alert, :prometheus, project: project) } - let(:service_response) { ServiceResponse.success(payload: { alerts: [alert_1, alert_2] }) } + let(:service_response) { ServiceResponse.success(http_status: :created) } let(:notify_service) { instance_double(Projects::Prometheus::Alerts::NotifyService, execute: service_response) } before do @@ -68,17 +68,12 @@ RSpec.describe Projects::Prometheus::AlertsController do .and_return(notify_service) end - it 'returns ok if notification succeeds' do + it 'returns created if notification succeeds' do expect(notify_service).to receive(:execute).and_return(service_response) post :notify, params: project_params, session: { as: :json } - expect(json_response).to contain_exactly( - { 'iid' => alert_1.iid, 'title' => alert_1.title }, - { 'iid' => alert_2.iid, 'title' => alert_2.title } - ) - - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:created) end it 'returns unprocessable entity if notification fails' do diff --git a/spec/controllers/projects/registry/repositories_controller_spec.rb b/spec/controllers/projects/registry/repositories_controller_spec.rb index a5faaaf5969..f4f5c182850 100644 --- a/spec/controllers/projects/registry/repositories_controller_spec.rb +++ b/spec/controllers/projects/registry/repositories_controller_spec.rb @@ -103,10 +103,11 @@ RSpec.describe Projects::Registry::RepositoriesController do stub_container_registry_tags(repository: :any, tags: []) end - it 'schedules a job to delete a repository' do - expect(DeleteContainerRepositoryWorker).to receive(:perform_async).with(user.id, repository.id) + it 'marks the repository as delete_scheduled' do + expect(DeleteContainerRepositoryWorker).not_to receive(:perform_async).with(user.id, repository.id) - delete_repository(repository) + expect { delete_repository(repository) } + .to change { repository.reload.status }.from(nil).to('delete_scheduled') expect(repository.reload).to be_delete_scheduled expect(response).to have_gitlab_http_status(:no_content) @@ -119,6 +120,22 @@ RSpec.describe Projects::Registry::RepositoriesController do expect_snowplow_event(category: anything, action: 'delete_repository') end + + context 'with container_registry_delete_repository_with_cron_worker disabled' do + before do + stub_feature_flags(container_registry_delete_repository_with_cron_worker: false) + end + + it 'schedules a job to delete a repository' do + expect(DeleteContainerRepositoryWorker).to receive(:perform_async).with(user.id, repository.id) + + expect { delete_repository(repository) } + .to change { repository.reload.status }.from(nil).to('delete_scheduled') + + expect(repository.reload).to be_delete_scheduled + expect(response).to have_gitlab_http_status(:no_content) + end + end end end end @@ -137,7 +154,7 @@ RSpec.describe Projects::Registry::RepositoriesController do end end - def go_to_index(format: :html, params: {} ) + def go_to_index(format: :html, params: {}) get :index, params: params.merge({ namespace_id: project.namespace, project_id: project diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb index b307bb357fa..2afd080344d 100644 --- a/spec/controllers/projects/releases_controller_spec.rb +++ b/spec/controllers/projects/releases_controller_spec.rb @@ -112,7 +112,7 @@ RSpec.describe Projects::ReleasesController do it "returns the project's releases as JSON, ordered by released_at" do get_index - expect(json_response.map { |release| release["id"] } ).to eq([release_2.id, release_1.id]) + expect(json_response.map { |release| release["id"] }).to eq([release_2.id, release_1.id]) end it_behaves_like 'common access controls' diff --git a/spec/controllers/projects/runners_controller_spec.rb b/spec/controllers/projects/runners_controller_spec.rb index 57d1695b842..1066c4ec9f6 100644 --- a/spec/controllers/projects/runners_controller_spec.rb +++ b/spec/controllers/projects/runners_controller_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Projects::RunnersController do new_desc = runner.description.swapcase expect do - post :update, params: params.merge(runner: { description: new_desc } ) + post :update, params: params.merge(runner: { description: new_desc }) end.to change { runner.ensure_runner_queue_value } runner.reload diff --git a/spec/controllers/projects/settings/integrations_controller_spec.rb b/spec/controllers/projects/settings/integrations_controller_spec.rb index b76269f6f93..2b23f177a9d 100644 --- a/spec/controllers/projects/settings/integrations_controller_spec.rb +++ b/spec/controllers/projects/settings/integrations_controller_spec.rb @@ -334,6 +334,23 @@ RSpec.describe Projects::Settings::IntegrationsController do ) end end + + context 'with chat notification integration' do + let_it_be(:integration) { project.create_microsoft_teams_integration(webhook: 'http://webhook.com') } + let(:message) { 'Microsoft Teams notifications settings saved and active.' } + + it_behaves_like 'integration update' + + context 'with masked token' do + let(:integration_params) { { active: true, webhook: '************' } } + + it_behaves_like 'integration update' + + it 'does not update the webhook' do + expect(integration.reload.webhook).to eq('http://webhook.com') + end + end + end end describe 'as JSON' do diff --git a/spec/controllers/projects/settings/repository_controller_spec.rb b/spec/controllers/projects/settings/repository_controller_spec.rb index 22287fea82c..ea50ff6caa0 100644 --- a/spec/controllers/projects/settings/repository_controller_spec.rb +++ b/spec/controllers/projects/settings/repository_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Projects::Settings::RepositoryController do let(:project) { create(:project_empty_repo, :public) } let(:user) { create(:user) } + let(:base_params) { { namespace_id: project.namespace, project_id: project } } before do project.add_maintainer(user) @@ -13,7 +14,7 @@ RSpec.describe Projects::Settings::RepositoryController do describe 'GET show' do it 'renders show with 200 status code' do - get :show, params: { namespace_id: project.namespace, project_id: project } + get :show, params: base_params expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:show) @@ -29,7 +30,7 @@ RSpec.describe Projects::Settings::RepositoryController do .with(project, user, anything) .and_return(status: :success) - put :cleanup, params: { namespace_id: project.namespace, project_id: project, project: { bfg_object_map: object_map } } + put :cleanup, params: base_params.merge({ project: { bfg_object_map: object_map } }) expect(response).to redirect_to project_settings_repository_path(project) end @@ -41,7 +42,7 @@ RSpec.describe Projects::Settings::RepositoryController do .with(project, user, anything) .and_return(status: :error, message: 'error message') - put :cleanup, params: { namespace_id: project.namespace, project_id: project, project: { bfg_object_map: object_map } } + put :cleanup, params: base_params.merge({ project: { bfg_object_map: object_map } }) expect(controller).to set_flash[:alert].to('error message') expect(response).to redirect_to project_settings_repository_path(project) @@ -50,83 +51,138 @@ RSpec.describe Projects::Settings::RepositoryController do end describe 'POST create_deploy_token' do - context 'when ajax_new_deploy_token feature flag is disabled for the project' do - before do - stub_feature_flags(ajax_new_deploy_token: false) + let(:good_deploy_token_params) do + { + name: 'name', + expires_at: 1.day.from_now.to_s, + username: 'deployer', + read_repository: '1', + deploy_token_type: DeployToken.deploy_token_types[:project_type] + } + end + + let(:request_params) { base_params.merge({ deploy_token: deploy_token_params }) } + + subject { post :create_deploy_token, params: request_params, format: :json } + + context('a good request') do + let(:deploy_token_params) { good_deploy_token_params } + let(:expected_response) do + { + 'id' => be_a(Integer), + 'name' => deploy_token_params[:name], + 'username' => deploy_token_params[:username], + 'expires_at' => Time.zone.parse(deploy_token_params[:expires_at]), + 'token' => be_a(String), + 'expired' => false, + 'revoked' => false, + 'scopes' => deploy_token_params.inject([]) do |scopes, kv| + key, value = kv + key.to_s.start_with?('read_') && value.to_i != 0 ? scopes << key.to_s : scopes + end + } end - it_behaves_like 'a created deploy token' do - let(:entity) { project } - let(:create_entity_params) { { namespace_id: project.namespace, project_id: project } } - let(:deploy_token_type) { DeployToken.deploy_token_types[:project_type] } + it 'creates the deploy token' do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/deploy_token') + expect(json_response).to match(expected_response) end end - context 'when ajax_new_deploy_token feature flag is enabled for the project' do - let(:good_deploy_token_params) do - { - name: 'name', - expires_at: 1.day.from_now.to_s, - username: 'deployer', - read_repository: '1', - deploy_token_type: DeployToken.deploy_token_types[:project_type] - } + context('a bad request') do + let(:deploy_token_params) { good_deploy_token_params.except(:read_repository) } + let(:expected_response) { { 'message' => "Scopes can't be blank" } } + + it 'does not create the deploy token' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to match(expected_response) end + end - let(:request_params) do - { - namespace_id: project.namespace.to_param, - project_id: project.to_param, - deploy_token: deploy_token_params - } + context('an invalid request') do + let(:deploy_token_params) { good_deploy_token_params.except(:name) } + + it 'raises a validation error' do + expect { subject }.to raise_error(ActiveRecord::StatementInvalid) end + end + end - subject { post :create_deploy_token, params: request_params, format: :json } - - context('a good request') do - let(:deploy_token_params) { good_deploy_token_params } - let(:expected_response) do - { - 'id' => be_a(Integer), - 'name' => deploy_token_params[:name], - 'username' => deploy_token_params[:username], - 'expires_at' => Time.zone.parse(deploy_token_params[:expires_at]), - 'token' => be_a(String), - 'expired' => false, - 'revoked' => false, - 'scopes' => deploy_token_params.inject([]) do |scopes, kv| - key, value = kv - key.to_s.start_with?('read_') && value.to_i != 0 ? scopes << key.to_s : scopes - end - } + describe 'PUT update' do + let(:project) { create(:project, :repository) } + + context 'when updating default branch' do + let!(:previous_default_branch) { project.default_branch } + + let(:new_default_branch) { 'feature' } + let(:request_params) { base_params.merge({ project: project_params_attributes }) } + + subject { put :update, params: request_params } + + context('with a good request') do + let(:project_params_attributes) { { default_branch: new_default_branch } } + + it "updates default branch and redirect to project_settings_repository_path" do + expect do + subject + end.to change { + Project.find(project.id).default_branch # refind to reset the default branch cache + }.from(previous_default_branch).to(new_default_branch) + + expect(response).to redirect_to project_settings_repository_path(project) + expect(controller).to set_flash[:notice].to("Project settings were successfully updated.") end + end - it 'creates the deploy token' do - subject + context('with a bad input') do + let(:project_params_attributes) { { default_branch: 'non_existent_branch' } } - expect(response).to have_gitlab_http_status(:created) - expect(response).to match_response_schema('public_api/v4/deploy_token') - expect(json_response).to match(expected_response) + it "does not update default branch and shows an alert" do + expect do + subject + end.not_to change { + Project.find(project.id).default_branch # refind to reset the default branch cache + } + + expect(response).to redirect_to project_settings_repository_path(project) + expect(controller).to set_flash[:alert].to("Could not set the default branch") end end + end + + context 'when updating branch names template from issues' do + let(:branch_name_template) { 'feat/GL-%{id}-%{title}' } - context('a bad request') do - let(:deploy_token_params) { good_deploy_token_params.except(:read_repository) } - let(:expected_response) { { 'message' => "Scopes can't be blank" } } + let(:request_params) { base_params.merge({ project: project_params_attributes }) } - it 'does not create the deploy token' do + subject { put :update, params: request_params } + + context('with a good request') do + let(:project_params_attributes) { { issue_branch_template: branch_name_template } } + + it "updates issue_branch_template and redirect to project_settings_repository_path" do subject - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response).to match(expected_response) + expect(response).to redirect_to project_settings_repository_path(project) + expect(controller).to set_flash[:notice].to("Project settings were successfully updated.") + expect(project.reload.issue_branch_template).to eq(branch_name_template) end end - context('an invalid request') do - let(:deploy_token_params) { good_deploy_token_params.except(:name) } + context('with a bad input') do + let(:project_params_attributes) { { issue_branch_template: 'a' * 260 } } + + it "updates issue_branch_template and redirect to project_settings_repository_path" do + subject - it 'raises a validation error' do - expect { subject }.to raise_error(ActiveRecord::StatementInvalid) + expect(response).to redirect_to project_settings_repository_path(project) + expect(controller).to set_flash[:alert].to("Project setting issue branch template is too long (maximum is 255 characters)") + expect(project.reload.issue_branch_template).to eq(nil) end end end diff --git a/spec/controllers/projects/starrers_controller_spec.rb b/spec/controllers/projects/starrers_controller_spec.rb index 8d03600cd58..2148f495c31 100644 --- a/spec/controllers/projects/starrers_controller_spec.rb +++ b/spec/controllers/projects/starrers_controller_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Projects::StarrersController do let(:user_1) { create(:user, name: 'John') } let(:user_2) { create(:user, name: 'Michael') } let(:private_user) { create(:user, name: 'Michael Douglas', private_profile: true) } + let(:blocked_user) { create(:user, state: 'blocked') } let(:admin) { create(:user, admin: true) } let(:project) { create(:project, :public) } @@ -13,6 +14,7 @@ RSpec.describe Projects::StarrersController do user_1.toggle_star(project) user_2.toggle_star(project) private_user.toggle_star(project) + blocked_user.toggle_star(project) end describe 'GET index' do @@ -61,6 +63,10 @@ RSpec.describe Projects::StarrersController do expect(user_ids).to contain_exactly(user_1.id, user_2.id) end + it 'non-active users are not visible' do + expect(user_ids).not_to include(blocked_user.id) + end + include_examples 'starrers counts' end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index b5797e374f3..446e5e38865 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -921,6 +921,7 @@ RSpec.describe ProjectsController do feature_flags_access_level releases_access_level monitor_access_level + infrastructure_access_level ] end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 637c774c38b..8775f68a5de 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe RegistrationsController do include TermsHelper + include FullNameHelper before do stub_application_setting(require_admin_approval_after_user_signup: false) @@ -18,6 +19,8 @@ RSpec.describe RegistrationsController do expect(response).to have_gitlab_http_status(:ok) expect(assigns(:resource)).to be_a(User) end + + it_behaves_like "switches to user preferred language", 'Sign up' end describe '#create' do @@ -463,7 +466,7 @@ RSpec.describe RegistrationsController do expect(User.last.first_name).to eq(base_user_params[:first_name]) expect(User.last.last_name).to eq(base_user_params[:last_name]) - expect(User.last.name).to eq("#{base_user_params[:first_name]} #{base_user_params[:last_name]}") + expect(User.last.name).to eq full_name(base_user_params[:first_name], base_user_params[:last_name]) end it 'sets the caller_id in the context' do @@ -477,28 +480,6 @@ RSpec.describe RegistrationsController do subject end - describe 'logged_out_marketing_header experiment', :experiment do - before do - stub_experiments(logged_out_marketing_header: :candidate) - end - - it 'tracks signed_up event' do - expect(experiment(:logged_out_marketing_header)).to track(:signed_up).on_next_instance - - subject - end - - context 'when registration fails' do - let_it_be(:user_params) { { user: base_user_params.merge({ username: '' }) } } - - it 'does not track signed_up event' do - expect(experiment(:logged_out_marketing_header)).not_to track(:signed_up) - - subject - end - end - end - context 'when the password is weak' do render_views let_it_be(:new_user_params) { { new_user: base_user_params.merge({ password: "password" }) } } @@ -513,6 +494,16 @@ RSpec.describe RegistrationsController do expect(response).to render_template(:new) expect(response.body).to include(_('Password must not contain commonly used combinations of words and letters')) end + + it 'tracks the error' do + subject + expect_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error', + controller: 'RegistrationsController', + method: 'create' + ) + end end context 'when block_weak_passwords is disabled' do @@ -525,6 +516,42 @@ RSpec.describe RegistrationsController do end end end + + context 'when the password is not weak' do + it 'does not track a weak password error' do + subject + expect_no_snowplow_event( + category: 'Gitlab::Tracking::Helpers::WeakPasswordErrorEvent', + action: 'track_weak_password_error' + ) + end + end + + context 'with preferred language' do + let(:user_preferred_language) { nil } + + before do + cookies['preferred_language'] = user_preferred_language + + post :create, params: { new_user: base_user_params } + end + + subject { User.last.preferred_language } + + context 'with default behavior' do + it 'sets preferred language to default' do + is_expected.to eq(Gitlab::CurrentSettings.default_preferred_language) + end + end + + context 'when user sets preferred language' do + let(:user_preferred_language) { 'zh_CN' } + + it 'sets name from first and last name' do + is_expected.to eq(user_preferred_language) + end + end + end end describe '#destroy' do diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 392dc2229aa..21df53fb074 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -223,7 +223,14 @@ RSpec.describe SearchController do let(:project) { nil } let(:category) { described_class.to_s } - let(:action) { 'i_search_total' } + let(:action) { 'executed' } + let(:label) { 'redis_hll_counters.search.search_total_unique_counts_monthly' } + let(:property) { 'i_search_total' } + let(:context) do + [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, + event: property).to_context] + end + let(:namespace) { create(:group) } let(:feature_flag_name) { :route_hll_to_snowplow_phase2 } end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 80cf060bc45..69282f951f9 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -69,6 +69,8 @@ RSpec.describe SessionsController do expect(controller.stored_location_for(:redirect)).to eq(search_path) end + + it_behaves_like "switches to user preferred language", 'Sign in' end describe '#create' do |