diff options
Diffstat (limited to 'spec/controllers')
85 files changed, 2608 insertions, 624 deletions
diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 5ad5f9cdeea..4eb0545eb6c 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -41,7 +41,7 @@ describe Admin::ApplicationSettingsController do it 'returns JSON data' do get :usage_data, format: :json - body = JSON.parse(response.body) + body = json_response expect(body["version"]).to eq(Gitlab::VERSION) expect(body).to include('counts') expect(response.status).to eq(200) diff --git a/spec/controllers/admin/clusters/applications_controller_spec.rb b/spec/controllers/admin/clusters/applications_controller_spec.rb index cf202d88acc..9d6edcd80c0 100644 --- a/spec/controllers/admin/clusters/applications_controller_spec.rb +++ b/spec/controllers/admin/clusters/applications_controller_spec.rb @@ -84,7 +84,7 @@ describe Admin::Clusters::ApplicationsController do patch :update, params: params end - let!(:application) { create(:clusters_applications_cert_managers, :installed, cluster: cluster) } + let!(:application) { create(:clusters_applications_cert_manager, :installed, cluster: cluster) } let(:application_name) { application.name } let(:params) { { application: application_name, id: cluster.id, email: "new-email@example.com" } } diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 509d8944e3a..1123563c1e3 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -68,5 +68,13 @@ describe Admin::GroupsController do post :update, params: { id: group.to_param, group: { project_creation_level: ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } } end.to change { group.reload.project_creation_level }.to(::Gitlab::Access::NO_ONE_PROJECT_ACCESS) end + + it 'updates the subgroup_creation_level successfully' do + expect do + post :update, + params: { id: group.to_param, + group: { subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS } } + end.to change { group.reload.subgroup_creation_level }.to(::Gitlab::Access::OWNER_SUBGROUP_ACCESS) + end end end diff --git a/spec/controllers/admin/requests_profiles_controller_spec.rb b/spec/controllers/admin/requests_profiles_controller_spec.rb index 10850cb4603..345f7720c25 100644 --- a/spec/controllers/admin/requests_profiles_controller_spec.rb +++ b/spec/controllers/admin/requests_profiles_controller_spec.rb @@ -10,38 +10,63 @@ describe Admin::RequestsProfilesController do end describe '#show' do - let(:basename) { "profile_#{Time.now.to_i}.html" } let(:tmpdir) { Dir.mktmpdir('profiler-test') } let(:test_file) { File.join(tmpdir, basename) } - let(:profile) { Gitlab::RequestProfiler::Profile.new(basename) } - let(:sample_data) do - <<~HTML - <!DOCTYPE html> - <html> - <body> - <h1>My First Heading</h1> - <p>My first paragraph.</p> - </body> - </html> - HTML + + subject do + get :show, params: { name: basename } end before do stub_const('Gitlab::RequestProfiler::PROFILES_DIR', tmpdir) - output = File.open(test_file, 'w') - output.write(sample_data) - output.close + File.write(test_file, sample_data) end after do - File.unlink(test_file) + FileUtils.rm_rf(tmpdir) end - it 'loads an HTML profile' do - get :show, params: { name: basename } + context 'when loading HTML profile' do + let(:basename) { "profile_#{Time.now.to_i}_execution.html" } + + let(:sample_data) do + '<html> <body> <h1>Heading</h1> <p>paragraph.</p> </body> </html>' + end + + it 'renders the data' do + subject + + expect(response).to have_gitlab_http_status(200) + expect(response.body).to eq(sample_data) + end + end + + context 'when loading TXT profile' do + let(:basename) { "profile_#{Time.now.to_i}_memory.txt" } + + let(:sample_data) do + <<~TXT + Total allocated: 112096396 bytes (1080431 objects) + Total retained: 10312598 bytes (53567 objects) + TXT + end + + it 'renders the data' do + subject + + expect(response).to have_gitlab_http_status(200) + expect(response.body).to eq(sample_data) + end + end + + context 'when loading PDF profile' do + let(:basename) { "profile_#{Time.now.to_i}_anything.pdf" } + + let(:sample_data) { 'mocked pdf content' } - expect(response).to have_gitlab_http_status(200) - expect(response.body).to eq(sample_data) + it 'fails to render the data' do + expect { subject }.to raise_error(ActionController::UrlGenerationError, /No route matches.*unmatched constraints:/) + end end end end diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index 78c5e2a2656..bbeda7dae0f 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -23,10 +23,11 @@ describe Admin::RunnersController do control_count = ActiveRecord::QueryRecorder.new { get :index }.count - create(:ci_runner, :tagged_only) + create_list(:ci_runner, 5, :tagged_only) # There is still an N+1 query for `runner.builds.count` - expect { get :index }.not_to exceed_query_limit(control_count + 1) + # We also need to add 1 because it takes 2 queries to preload tags + expect { get :index }.not_to exceed_query_limit(control_count + 6) expect(response).to have_gitlab_http_status(200) expect(response.body).to have_content('tag1') diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 89a0eba66f7..d7428f8b52c 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -279,6 +279,12 @@ describe Admin::UsersController do expect(warden.user).to eq(user) end + it 'logs the beginning of the impersonation event' do + expect(Gitlab::AppLogger).to receive(:info).with("User #{admin.username} has started impersonating #{user.username}").and_call_original + + post :impersonate, params: { id: user.username } + end + it "redirects to root" do post :impersonate, params: { id: user.username } diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 447a12b2fac..0b3833e6515 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -63,8 +63,6 @@ describe ApplicationController do sign_in user end - let(:json_response) { JSON.parse(response.body) } - controller(described_class) do def index render json: Gon.all_variables @@ -643,24 +641,32 @@ describe ApplicationController do end end - it 'does not set a custom header' do + it 'sets a custom header' do get :index, format: :json - expect(response.headers['X-GitLab-Custom-Error']).to be_nil + expect(response.headers['X-GitLab-Custom-Error']).to eq '1' end - end - context 'given a json response for an html request' do - controller do - def index - render json: {}, status: :unprocessable_entity + context 'for html request' do + it 'sets a custom header' do + get :index + + expect(response.headers['X-GitLab-Custom-Error']).to eq '1' end end - it 'does not set a custom header' do - get :index + context 'for 200 response' do + controller do + def index + render json: {}, status: :ok + end + end - expect(response.headers['X-GitLab-Custom-Error']).to be_nil + it 'does not set a custom header' do + get :index, format: :json + + expect(response.headers['X-GitLab-Custom-Error']).to be_nil + end end end end diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 3f1c0ae8ac4..6cdd61e7abd 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -222,6 +222,20 @@ describe AutocompleteController do expect(response_user_ids).to contain_exactly(non_member.id) end end + + context 'merge_request_iid parameter included' do + before do + sign_in(user) + end + + it 'includes can_merge option to users' do + merge_request = create(:merge_request, source_project: project) + + get(:users, params: { merge_request_iid: merge_request.iid, project_id: project.id }) + + expect(json_response.first).to have_key('can_merge') + end + end end context 'GET projects' do @@ -295,28 +309,6 @@ describe AutocompleteController do end end - context 'authorized projects with offset' do - before do - authorized_project2 = create(:project) - authorized_project3 = create(:project) - - authorized_project.add_maintainer(user) - authorized_project2.add_maintainer(user) - authorized_project3.add_maintainer(user) - end - - describe 'GET #projects with project ID and offset_id' do - before do - get(:projects, params: { project_id: project.id, offset_id: authorized_project.id }) - end - - it 'returns projects' do - expect(json_response).to be_kind_of(Array) - expect(json_response.size).to eq 2 # Of a total of 3 - end - end - end - context 'authorized projects without admin_issue ability' do before do authorized_project.add_guest(user) diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index c84bb913cad..d54f7ad33cf 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -52,10 +52,8 @@ describe Boards::IssuesController do list_issues user: user, board: board, list: list2 - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('entities/issue_boards') - expect(parsed_response['issues'].length).to eq 2 + expect(json_response['issues'].length).to eq 2 expect(development.issues.map(&:relative_position)).not_to include(nil) end @@ -87,7 +85,7 @@ describe Boards::IssuesController do expect { list_issues(user: user, board: group_board, list: list3) }.not_to exceed_query_limit(control_count + (2 * 8 - 1)) end - it 'avoids N+1 database queries when adding a subgroup, project, and issue', :nested_groups do + it 'avoids N+1 database queries when adding a subgroup, project, and issue' do create(:project, group: sub_group_1) create(:labeled_issue, project: project, labels: [development]) control_count = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: group_board, list: list3) }.count @@ -123,10 +121,8 @@ describe Boards::IssuesController do list_issues user: user, board: board - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('entities/issue_boards') - expect(parsed_response['issues'].length).to eq 2 + expect(json_response['issues'].length).to eq 2 end end @@ -164,6 +160,215 @@ describe Boards::IssuesController do end end + describe 'PUT bulk_move' do + let(:todo) { create(:group_label, group: group, name: 'Todo') } + let(:development) { create(:group_label, group: group, name: 'Development') } + let(:user) { create(:group_member, :maintainer, user: create(:user), group: group ).user } + let(:guest) { create(:group_member, :guest, user: create(:user), group: group ).user } + let(:project) { create(:project, group: group) } + let(:group) { create(:group) } + let(:board) { create(:board, project: project) } + let(:list1) { create(:list, board: board, label: todo, position: 0) } + let(:list2) { create(:list, board: board, label: development, position: 1) } + let(:issue1) { create(:labeled_issue, project: project, labels: [todo], author: user, relative_position: 10) } + let(:issue2) { create(:labeled_issue, project: project, labels: [todo], author: user, relative_position: 20) } + let(:issue3) { create(:labeled_issue, project: project, labels: [todo], author: user, relative_position: 30) } + let(:issue4) { create(:labeled_issue, project: project, labels: [development], author: user, relative_position: 100) } + + let(:move_params) do + { + board_id: board.id, + ids: [issue1.id, issue2.id, issue3.id], + from_list_id: list1.id, + to_list_id: list2.id, + move_before_id: issue4.id, + move_after_id: nil + } + end + + before do + project.add_maintainer(user) + project.add_guest(guest) + end + + shared_examples 'move issues endpoint provider' do + before do + sign_in(signed_in_user) + end + + it 'responds as expected' do + put :bulk_move, params: move_issues_params + expect(response).to have_gitlab_http_status(expected_status) + + if expected_status == 200 + expect(json_response).to include( + 'count' => move_issues_params[:ids].size, + 'success' => true + ) + + expect(json_response['issues'].pluck('id')).to match_array(move_issues_params[:ids]) + end + end + + it 'moves issues as expected' do + put :bulk_move, params: move_issues_params + expect(response).to have_gitlab_http_status(expected_status) + + list_issues user: requesting_user, board: board, list: list2 + expect(response).to have_gitlab_http_status(200) + + expect(response).to match_response_schema('entities/issue_boards') + + responded_issues = json_response['issues'] + expect(responded_issues.length).to eq expected_issue_count + + ids_in_order = responded_issues.pluck('id') + expect(ids_in_order).to eq(expected_issue_ids_in_order) + end + end + + context 'when items are moved to another list' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) { move_params } + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 4 } + let(:expected_issue_ids_in_order) { [issue4.id, issue1.id, issue2.id, issue3.id] } + end + end + + context 'when moving just one issue' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:ids] = [issue2.id] + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 2 } + let(:expected_issue_ids_in_order) { [issue4.id, issue2.id] } + end + end + + context 'when user is not allowed to move issue' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { guest } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:ids] = [issue2.id] + end + end + let(:requesting_user) { user } + let(:expected_status) { 403 } + let(:expected_issue_count) { 1 } + let(:expected_issue_ids_in_order) { [issue4.id] } + end + end + + context 'when issues should be moved visually above existing issue in list' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:move_after_id] = issue4.id + hash[:move_before_id] = nil + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 4 } + let(:expected_issue_ids_in_order) { [issue1.id, issue2.id, issue3.id, issue4.id] } + end + end + + context 'when destination list is empty' do + before do + # Remove issue from list + issue4.labels -= [development] + issue4.save! + end + + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:move_before_id] = nil + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 3 } + let(:expected_issue_ids_in_order) { [issue1.id, issue2.id, issue3.id] } + end + end + + context 'when no position arguments are given' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:move_before_id] = nil + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 4 } + let(:expected_issue_ids_in_order) { [issue1.id, issue2.id, issue3.id, issue4.id] } + end + end + + context 'when move_before_id and move_after_id are given' do + let(:issue5) { create(:labeled_issue, project: project, labels: [development], author: user, relative_position: 90) } + + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:move_before_id] = issue5.id + hash[:move_after_id] = issue4.id + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 5 } + let(:expected_issue_ids_in_order) { [issue5.id, issue1.id, issue2.id, issue3.id, issue4.id] } + end + end + + context 'when request contains too many issues' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:ids] = (0..51).to_a + end + end + let(:requesting_user) { user } + let(:expected_status) { 422 } + let(:expected_issue_count) { 1 } + let(:expected_issue_ids_in_order) { [issue4.id] } + end + end + + context 'when request is malformed' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:ids] = 'foobar' + end + end + let(:requesting_user) { user } + let(:expected_status) { 400 } + let(:expected_issue_count) { 1 } + let(:expected_issue_ids_in_order) { [issue4.id] } + end + end + end + def list_issues(user:, board:, list: nil) sign_in(user) diff --git a/spec/controllers/boards/lists_controller_spec.rb b/spec/controllers/boards/lists_controller_spec.rb index e1f75fa3395..418ca6f3210 100644 --- a/spec/controllers/boards/lists_controller_spec.rb +++ b/spec/controllers/boards/lists_controller_spec.rb @@ -26,10 +26,8 @@ describe Boards::ListsController do read_board_list user: user, board: board - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('lists') - expect(parsed_response.length).to eq 3 + expect(json_response.length).to eq 3 end context 'with unauthorized user' do diff --git a/spec/controllers/chaos_controller_spec.rb b/spec/controllers/chaos_controller_spec.rb new file mode 100644 index 00000000000..bafd4a70862 --- /dev/null +++ b/spec/controllers/chaos_controller_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ChaosController do + describe '#leakmem' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:leak_mem).with(100, 30.seconds) + + get :leakmem + + expect(response).to have_gitlab_http_status(200) + end + + it 'call synchronously with params' do + expect(Gitlab::Chaos).to receive(:leak_mem).with(1, 2.seconds) + + get :leakmem, params: { memory_mb: 1, duration_s: 2 } + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::LeakMemWorker).to receive(:perform_async).with(100, 30.seconds) + + get :leakmem, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end + + describe '#cpu_spin' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:cpu_spin).with(30.seconds) + + get :cpu_spin + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls synchronously with params' do + expect(Gitlab::Chaos).to receive(:cpu_spin).with(3.seconds) + + get :cpu_spin, params: { duration_s: 3 } + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::CpuSpinWorker).to receive(:perform_async).with(30.seconds) + + get :cpu_spin, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end + + describe '#db_spin' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:db_spin).with(30.seconds, 1.second) + + get :db_spin + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls synchronously with params' do + expect(Gitlab::Chaos).to receive(:db_spin).with(4.seconds, 5.seconds) + + get :db_spin, params: { duration_s: 4, interval_s: 5 } + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::DbSpinWorker).to receive(:perform_async).with(30.seconds, 1.second) + + get :db_spin, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end + + describe '#sleep' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:sleep).with(30.seconds) + + get :sleep + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls synchronously with params' do + expect(Gitlab::Chaos).to receive(:sleep).with(5.seconds) + + get :sleep, params: { duration_s: 5 } + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::SleepWorker).to receive(:perform_async).with(30.seconds) + + get :sleep, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end + + describe '#kill' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:kill).with(no_args) + + get :kill + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::KillWorker).to receive(:perform_async).with(no_args) + + get :kill, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end +end diff --git a/spec/controllers/concerns/confirm_email_warning_spec.rb b/spec/controllers/concerns/confirm_email_warning_spec.rb new file mode 100644 index 00000000000..0c598a360af --- /dev/null +++ b/spec/controllers/concerns/confirm_email_warning_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ConfirmEmailWarning do + before do + stub_feature_flags(soft_email_confirmation: true) + allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days + end + + controller(ApplicationController) do + # `described_class` is not available in this context + include ConfirmEmailWarning # rubocop:disable RSpec/DescribedClass + + def index + head :ok + end + end + + RSpec::Matchers.define :set_confirm_warning_for do |email| + match do |response| + expect(response).to set_flash.now[:warning].to include("Please check your email (#{email}) to verify that you own this address.") + end + end + + describe 'confirm email flash warning' do + context 'when not signed in' do + let(:user) { create(:user, confirmed_at: nil) } + + before do + get :index + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'when signed in' do + before do + sign_in(user) + end + + context 'with a confirmed user' do + let(:user) { create(:user) } + + before do + get :index + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'with an unconfirmed user' do + let(:user) { create(:user, confirmed_at: nil) } + + context 'when executing a peek request' do + before do + request.path = '/-/peek' + get :index + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'when executing a json request' do + before do + get :index, format: :json + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'when executing a post request' do + before do + post :index + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'when executing a get request' do + before do + get :index + end + + context 'with an unconfirmed email address present' do + let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'unconfirmed@gitlab.com') } + + it { is_expected.to set_confirm_warning_for(user.unconfirmed_email) } + end + + context 'without an unconfirmed email address present' do + it { is_expected.to set_confirm_warning_for(user.email) } + end + end + end + end + end +end diff --git a/spec/controllers/concerns/group_tree_spec.rb b/spec/controllers/concerns/group_tree_spec.rb index aa3cd690e3f..835c3d9b3af 100644 --- a/spec/controllers/concerns/group_tree_spec.rb +++ b/spec/controllers/concerns/group_tree_spec.rb @@ -30,7 +30,7 @@ describe GroupTree do expect(assigns(:groups)).to contain_exactly(other_group) end - context 'for subgroups', :nested_groups do + context 'for subgroups' do it 'only renders root groups when no parent was given' do create(:group, :public, parent: group) @@ -85,7 +85,7 @@ describe GroupTree do expect(json_response.first['id']).to eq(group.id) end - context 'nested groups', :nested_groups do + context 'nested groups' do it 'expands the tree when filtering' do subgroup = create(:group, :public, parent: group, name: 'filter') diff --git a/spec/controllers/concerns/issuable_actions_spec.rb b/spec/controllers/concerns/issuable_actions_spec.rb new file mode 100644 index 00000000000..7b0b4497f3f --- /dev/null +++ b/spec/controllers/concerns/issuable_actions_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +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(:discussion_serializer) { double('discussion_serializer') } + + let(:controller) do + klass = Class.new do + attr_reader :current_user, :project, :issuable + + def self.before_action(action, params = nil) + end + + include IssuableActions + + def initialize(issuable, project, user, finder_params) + @issuable = issuable + @project = project + @current_user = user + @finder_params = finder_params + end + + def finder_params_for_issuable + @finder_params + end + + def params + { + notes_filter: 1 + } + end + + def prepare_notes_for_rendering(notes) + [] + end + + def render(options) + end + end + + klass.new(issuable, project, user, finder_params_for_issuable) + end + + describe '#discussions' do + before do + allow(user).to receive(:set_notes_filter) + allow(user).to receive(:user_preference) + allow(discussion_serializer).to receive(:represent) + end + + it 'instantiates and calls NotesFinder as expected' do + 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) + + controller.discussions + end + end +end diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index f210537aad5..7bdf5c49425 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -24,78 +24,6 @@ describe IssuableCollections do controller end - describe '#set_sort_order_from_user_preference' do - describe 'when sort param given' do - let(:params) { { sort: 'updated_desc' } } - - context 'when issuable_sorting_field is defined' do - before do - controller.class.define_method(:issuable_sorting_field) { :issues_sort} - end - - it 'sets user_preference with the right value' do - controller.send(:set_sort_order_from_user_preference) - - expect(user.user_preference.reload.issues_sort).to eq('updated_desc') - end - end - - context 'when no issuable_sorting_field is defined on the controller' do - it 'does not touch user_preference' do - allow(user).to receive(:user_preference) - - controller.send(:set_sort_order_from_user_preference) - - expect(user).not_to have_received(:user_preference) - end - end - end - - context 'when a user sorting preference exists' do - let(:params) { {} } - - before do - controller.class.define_method(:issuable_sorting_field) { :issues_sort } - end - - it 'returns the set preference' do - user.user_preference.update(issues_sort: 'updated_asc') - - sort_preference = controller.send(:set_sort_order_from_user_preference) - - expect(sort_preference).to eq('updated_asc') - end - end - end - - describe '#set_set_order_from_cookie' do - describe 'when sort param given' do - let(:cookies) { {} } - let(:params) { { sort: 'downvotes_asc' } } - - it 'sets the cookie with the right values and flags' do - allow(controller).to receive(:cookies).and_return(cookies) - - controller.send(:set_sort_order_from_cookie) - - expect(cookies['issue_sort']).to eq({ value: 'popularity', secure: false, httponly: false }) - end - end - - describe 'when cookie exists' do - let(:cookies) { { 'issue_sort' => 'id_asc' } } - let(:params) { {} } - - it 'sets the cookie with the right values and flags' do - allow(controller).to receive(:cookies).and_return(cookies) - - controller.send(:set_sort_order_from_cookie) - - expect(cookies['issue_sort']).to eq({ value: 'created_asc', secure: false, httponly: false }) - end - end - end - describe '#page_count_for_relation' do let(:params) { { state: 'opened' } } diff --git a/spec/controllers/concerns/sorting_preference_spec.rb b/spec/controllers/concerns/sorting_preference_spec.rb new file mode 100644 index 00000000000..a36124c6776 --- /dev/null +++ b/spec/controllers/concerns/sorting_preference_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SortingPreference do + let(:user) { create(:user) } + + let(:controller_class) do + Class.new do + def self.helper_method(name); end + + include SortingPreference + include SortingHelper + end + end + + let(:controller) { controller_class.new } + + before do + allow(controller).to receive(:params).and_return(ActionController::Parameters.new(params)) + allow(controller).to receive(:current_user).and_return(user) + allow(controller).to receive(:legacy_sort_cookie_name).and_return('issuable_sort') + allow(controller).to receive(:sorting_field).and_return(:issues_sort) + end + + describe '#set_sort_order_from_user_preference' do + subject { controller.send(:set_sort_order_from_user_preference) } + + context 'when sort param given' do + let(:params) { { sort: 'updated_desc' } } + + context 'when sorting_field is defined' do + it 'sets user_preference with the right value' do + is_expected.to eq('updated_desc') + end + end + + context 'when no sorting_field is defined on the controller' do + before do + allow(controller).to receive(:sorting_field).and_return(nil) + end + + it 'does not touch user_preference' do + expect(user).not_to receive(:user_preference) + + subject + end + end + end + + context 'when a user sorting preference exists' do + let(:params) { {} } + + before do + user.user_preference.update!(issues_sort: 'updated_asc') + end + + it 'returns the set preference' do + is_expected.to eq('updated_asc') + end + end + end + + describe '#set_set_order_from_cookie' do + subject { controller.send(:set_sort_order_from_cookie) } + + before do + allow(controller).to receive(:cookies).and_return(cookies) + end + + context 'when sort param given' do + let(:cookies) { {} } + let(:params) { { sort: 'downvotes_asc' } } + + it 'sets the cookie with the right values and flags' do + subject + + expect(cookies['issue_sort']).to eq(value: 'popularity', secure: false, httponly: false) + end + end + + context 'when cookie exists' do + let(:cookies) { { 'issue_sort' => 'id_asc' } } + let(:params) { {} } + + it 'sets the cookie with the right values and flags' do + subject + + expect(cookies['issue_sort']).to eq(value: 'created_asc', secure: false, httponly: false) + end + end + end +end diff --git a/spec/controllers/dashboard/groups_controller_spec.rb b/spec/controllers/dashboard/groups_controller_spec.rb index 48373d29412..20a0951423b 100644 --- a/spec/controllers/dashboard/groups_controller_spec.rb +++ b/spec/controllers/dashboard/groups_controller_spec.rb @@ -26,7 +26,7 @@ describe Dashboard::GroupsController do expect(assigns(:groups)).to contain_exactly(member_of_group) end - context 'when rendering an expanded hierarchy with public groups you are not a member of', :nested_groups do + context 'when rendering an expanded hierarchy with public groups you are not a member of' do let!(:top_level_result) { create(:group, name: 'chef-top') } let!(:top_level_a) { create(:group, name: 'top-a') } let!(:sub_level_result_a) { create(:group, name: 'chef-sub-a', parent: top_level_a) } diff --git a/spec/controllers/dashboard/milestones_controller_spec.rb b/spec/controllers/dashboard/milestones_controller_spec.rb index 4de537ae6f8..67939aa4e6a 100644 --- a/spec/controllers/dashboard/milestones_controller_spec.rb +++ b/spec/controllers/dashboard/milestones_controller_spec.rb @@ -47,6 +47,8 @@ describe Dashboard::MilestonesController do describe "#index" do let(:public_group) { create(:group, :public) } let!(:public_milestone) { create(:milestone, group: public_group) } + let!(:closed_group_milestone) { create(:milestone, group: group, state: 'closed') } + let!(:closed_project_milestone) { create(:milestone, project: project, state: 'closed') } render_views @@ -59,6 +61,15 @@ describe Dashboard::MilestonesController do expect(json_response.map { |i| i["group_name"] }.compact).to match_array(group.name) end + it 'returns closed group and project milestones to which the user belongs' do + get :index, params: { state: 'closed' }, format: :json + + expect(response).to have_gitlab_http_status(200) + expect(json_response.size).to eq(2) + expect(json_response.map { |i| i["name"] }).to match_array([closed_group_milestone.name, closed_project_milestone.name]) + expect(json_response.map { |i| i["group_name"] }.compact).to match_array(group.name) + end + it 'searches legacy project milestones by title when search_title is given' do project_milestone = create(:milestone, title: 'Project milestone title', project: project) @@ -77,11 +88,11 @@ describe Dashboard::MilestonesController do expect(response.body).not_to include(project_milestone.title) end - it 'shows counts of group and project milestones to which the user belongs to' do + it 'shows counts of open and closed group and project milestones to which the user belongs to' do get :index expect(response.body).to include("Open\n<span class=\"badge badge-pill\">2</span>") - expect(response.body).to include("Closed\n<span class=\"badge badge-pill\">0</span>") + expect(response.body).to include("Closed\n<span class=\"badge badge-pill\">2</span>") end context 'external authorization' do diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index 6591901a9dc..8b95c9f2496 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -40,6 +40,14 @@ describe Dashboard::ProjectsController do expect(assigns(:projects)).to eq([project, project2]) end + + context 'project sorting' do + let(:project) { create(:project) } + + it_behaves_like 'set sort order from user preference' do + let(:sorting_param) { 'created_asc' } + end + end end end diff --git a/spec/controllers/explore/projects_controller_spec.rb b/spec/controllers/explore/projects_controller_spec.rb index 463586ee422..6752d2b8ebd 100644 --- a/spec/controllers/explore/projects_controller_spec.rb +++ b/spec/controllers/explore/projects_controller_spec.rb @@ -3,56 +3,91 @@ require 'spec_helper' describe Explore::ProjectsController do - describe 'GET #index.json' do - render_views + shared_examples 'explore projects' do + describe 'GET #index.json' do + render_views - before do - get :index, format: :json + before do + get :index, format: :json + end + + it { is_expected.to respond_with(:success) } end - it { is_expected.to respond_with(:success) } - end + describe 'GET #trending.json' do + render_views - describe 'GET #trending.json' do - render_views + before do + get :trending, format: :json + end - before do - get :trending, format: :json + it { is_expected.to respond_with(:success) } + end + + describe 'GET #starred.json' do + render_views + + before do + get :starred, format: :json + end + + it { is_expected.to respond_with(:success) } end - it { is_expected.to respond_with(:success) } + describe 'GET #trending' do + context 'sorting by update date' do + let(:project1) { create(:project, :public, updated_at: 3.days.ago) } + let(:project2) { create(:project, :public, updated_at: 1.day.ago) } + + before do + create(:trending_project, project: project1) + create(:trending_project, project: project2) + end + + it 'sorts by last updated' do + get :trending, params: { sort: 'updated_desc' } + + expect(assigns(:projects)).to eq [project2, project1] + end + + it 'sorts by oldest updated' do + get :trending, params: { sort: 'updated_asc' } + + expect(assigns(:projects)).to eq [project1, project2] + end + end + end end - describe 'GET #starred.json' do - render_views + context 'when user is signed in' do + let(:user) { create(:user) } before do - get :starred, format: :json + sign_in(user) end - it { is_expected.to respond_with(:success) } - end + include_examples 'explore projects' - describe 'GET #trending' do - context 'sorting by update date' do - let(:project1) { create(:project, :public, updated_at: 3.days.ago) } - let(:project2) { create(:project, :public, updated_at: 1.day.ago) } + context 'user preference sorting' do + let(:project) { create(:project) } - before do - create(:trending_project, project: project1) - create(:trending_project, project: project2) + it_behaves_like 'set sort order from user preference' do + let(:sorting_param) { 'created_asc' } end + end + end - it 'sorts by last updated' do - get :trending, params: { sort: 'updated_desc' } + context 'when user is not signed in' do + include_examples 'explore projects' - expect(assigns(:projects)).to eq [project2, project1] - end + context 'user preference sorting' do + let(:project) { create(:project) } + let(:sorting_param) { 'created_asc' } - it 'sorts by oldest updated' do - get :trending, params: { sort: 'updated_asc' } + it 'does not set sort order from user preference' do + expect_any_instance_of(UserPreference).not_to receive(:update) - expect(assigns(:projects)).to eq [project1, project2] + get :index, params: { sort: sorting_param } end end end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index c19a752b07b..9937bdf4061 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -7,6 +7,27 @@ describe GraphqlController do stub_feature_flags(graphql: true) end + describe 'ArgumentError' do + let(:user) { create(:user) } + let(:message) { 'green ideas sleep furiously' } + + before do + sign_in(user) + end + + it 'handles argument errors' do + allow(subject).to receive(:execute) do + raise Gitlab::Graphql::Errors::ArgumentError, message + end + + post :execute + + expect(json_response).to include( + 'errors' => include(a_hash_including('message' => message)) + ) + end + end + describe 'POST #execute' do context 'when user is logged in' do let(:user) { create(:user) } diff --git a/spec/controllers/groups/boards_controller_spec.rb b/spec/controllers/groups/boards_controller_spec.rb index 5e0f64ccca4..e4232c2c1ab 100644 --- a/spec/controllers/groups/boards_controller_spec.rb +++ b/spec/controllers/groups/boards_controller_spec.rb @@ -63,10 +63,8 @@ describe Groups::BoardsController do list_boards format: :json - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('boards') - expect(parsed_response.length).to eq 1 + expect(json_response.length).to eq 1 end context 'with unauthorized user' do diff --git a/spec/controllers/groups/children_controller_spec.rb b/spec/controllers/groups/children_controller_spec.rb index 02fb971bd9a..bced300a24c 100644 --- a/spec/controllers/groups/children_controller_spec.rb +++ b/spec/controllers/groups/children_controller_spec.rb @@ -46,7 +46,7 @@ describe Groups::ChildrenController do end end - context 'for subgroups', :nested_groups do + context 'for subgroups' do let!(:public_subgroup) { create(:group, :public, parent: group) } let!(:private_subgroup) { create(:group, :private, parent: group) } let!(:public_project) { create(:project, :public, namespace: group) } @@ -292,7 +292,7 @@ describe Groups::ChildrenController do end end - context 'with subgroups and projects', :nested_groups do + context 'with subgroups and projects' do let!(:first_page_subgroups) { create_list(:group, per_page, :public, parent: group) } let!(:other_subgroup) { create(:group, :public, parent: group) } let!(:next_page_projects) { create_list(:project, per_page, :public, namespace: group) } diff --git a/spec/controllers/groups/clusters/applications_controller_spec.rb b/spec/controllers/groups/clusters/applications_controller_spec.rb index 16a63536ea6..21533d1c89a 100644 --- a/spec/controllers/groups/clusters/applications_controller_spec.rb +++ b/spec/controllers/groups/clusters/applications_controller_spec.rb @@ -91,7 +91,7 @@ describe Groups::Clusters::ApplicationsController do patch :update, params: params.merge(group_id: group) end - let!(:application) { create(:clusters_applications_cert_managers, :installed, cluster: cluster) } + let!(:application) { create(:clusters_applications_cert_manager, :installed, cluster: cluster) } let(:application_name) { application.name } let(:params) { { application: application_name, id: cluster.id, email: "new-email@example.com" } } diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 413598ddde0..0c3dd971582 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -16,6 +16,39 @@ describe Groups::GroupMembersController do expect(response).to have_gitlab_http_status(200) expect(response).to render_template(:index) end + + context 'user with owner access' do + let!(:invited) { create_list(:group_member, 3, :invited, group: group) } + + before do + group.add_owner(user) + sign_in(user) + end + + it 'assigns invited members' do + get :index, params: { group_id: group } + + expect(assigns(:invited_members).map(&:invite_email)).to match_array(invited.map(&:invite_email)) + end + + it 'restricts search to one email' do + get :index, params: { group_id: group, search_invited: invited.first.invite_email } + + expect(assigns(:invited_members).map(&:invite_email)).to match_array(invited.first.invite_email) + end + + it 'paginates invited list' do + stub_const('Groups::GroupMembersController::MEMBER_PER_PAGE_LIMIT', 2) + + get :index, params: { group_id: group, invited_members_page: 1 } + + expect(assigns(:invited_members).count).to eq(2) + + get :index, params: { group_id: group, invited_members_page: 2 } + + expect(assigns(:invited_members).count).to eq(1) + end + end end describe 'POST create' do @@ -139,7 +172,7 @@ describe Groups::GroupMembersController do it '[JS] removes user from members' do delete :destroy, params: { group_id: group, id: member }, xhr: true - expect(response).to be_success + expect(response).to be_successful expect(group.members).not_to include member end end diff --git a/spec/controllers/groups/labels_controller_spec.rb b/spec/controllers/groups/labels_controller_spec.rb index 3cc6fc6f066..98a4c50fc49 100644 --- a/spec/controllers/groups/labels_controller_spec.rb +++ b/spec/controllers/groups/labels_controller_spec.rb @@ -24,7 +24,7 @@ describe Groups::LabelsController do expect(label_ids).to match_array([label_1.title, group_label_1.title]) end - context 'with ancestor group', :nested_groups do + context 'with ancestor group' do set(:subgroup) { create(:group, parent: group) } set(:subgroup_label_1) { create(:group_label, group: subgroup, title: 'subgroup_label_1') } @@ -32,7 +32,7 @@ describe Groups::LabelsController do subgroup.add_owner(user) end - it 'returns ancestor group labels', :nested_groups do + it 'returns ancestor group labels' do get :index, params: { group_id: subgroup, include_ancestor_groups: true, only_group_labels: true }, format: :json label_ids = json_response.map {|label| label['title']} diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index 19b18091aef..41927907fd1 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -73,7 +73,7 @@ describe Groups::MilestonesController do it 'lists legacy group milestones and group milestones' do get :index, params: { group_id: group.to_param }, format: :json - milestones = JSON.parse(response.body) + milestones = json_response expect(milestones.count).to eq(2) expect(milestones.first["title"]).to eq("group milestone") @@ -186,7 +186,7 @@ describe Groups::MilestonesController do it "removes milestone" do delete :destroy, params: { group_id: group.to_param, id: milestone.iid }, format: :js - expect(response).to be_success + expect(response).to be_successful expect { Milestone.find(milestone.id) }.to raise_exception(ActiveRecord::RecordNotFound) end end diff --git a/spec/controllers/groups/uploads_controller_spec.rb b/spec/controllers/groups/uploads_controller_spec.rb index 0f99a957581..60342bf8e3d 100644 --- a/spec/controllers/groups/uploads_controller_spec.rb +++ b/spec/controllers/groups/uploads_controller_spec.rb @@ -10,6 +10,11 @@ describe Groups::UploadsController do { group_id: model } end + let(:other_model) { create(:group, :public) } + let(:other_params) do + { group_id: other_model } + end + it_behaves_like 'handle uploads' do let(:uploader_class) { NamespaceFileUploader } end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index d2faef5b12b..404e61c5271 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -89,7 +89,7 @@ describe GroupsController do end describe 'GET #new' do - context 'when creating subgroups', :nested_groups do + context 'when creating subgroups' do [true, false].each do |can_create_group_status| context "and can_create_group is #{can_create_group_status}" do before do @@ -166,7 +166,7 @@ describe GroupsController do end end - context 'when creating subgroups', :nested_groups do + context 'when creating subgroups' do [true, false].each do |can_create_group_status| context "and can_create_group is #{can_create_group_status}" do context 'and logged in as Owner' do @@ -584,7 +584,7 @@ describe GroupsController do end end - describe 'PUT transfer', :postgresql do + describe 'PUT transfer' do before do sign_in(user) end diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb index 19d739fcf4f..b48b7dc86e0 100644 --- a/spec/controllers/health_check_controller_spec.rb +++ b/spec/controllers/health_check_controller_spec.rb @@ -5,7 +5,6 @@ require 'spec_helper' describe HealthCheckController do include StubENV - let(:json_response) { JSON.parse(response.body) } let(:xml_response) { Hash.from_xml(response.body)['hash'] } let(:token) { Gitlab::CurrentSettings.health_check_access_token } let(:whitelisted_ip) { '127.0.0.1' } @@ -34,14 +33,14 @@ describe HealthCheckController do get :index - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'text/plain' end it 'supports passing the token in query params' do get :index, params: { token: token } - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'text/plain' end end @@ -55,14 +54,14 @@ describe HealthCheckController do it 'supports successful plaintext response' do get :index - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'text/plain' end it 'supports successful json response' do get :index, format: :json - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'application/json' expect(json_response['healthy']).to be true end @@ -70,7 +69,7 @@ describe HealthCheckController do it 'supports successful xml response' do get :index, format: :xml - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'application/xml' expect(xml_response['healthy']).to be true end @@ -78,7 +77,7 @@ describe HealthCheckController do it 'supports successful responses for specific checks' do get :index, params: { checks: 'email' }, format: :json - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'application/json' expect(json_response['healthy']).to be true end diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb index fc62a8310aa..e82dcfcdb64 100644 --- a/spec/controllers/health_controller_spec.rb +++ b/spec/controllers/health_controller_spec.rb @@ -5,7 +5,6 @@ require 'spec_helper' describe HealthController do include StubENV - let(:json_response) { JSON.parse(response.body) } let(:token) { Gitlab::CurrentSettings.health_check_access_token } let(:whitelisted_ip) { '127.0.0.1' } let(:not_whitelisted_ip) { '127.0.0.2' } diff --git a/spec/controllers/help_controller_spec.rb b/spec/controllers/help_controller_spec.rb index dbfacf4e42e..03b6e85b653 100644 --- a/spec/controllers/help_controller_spec.rb +++ b/spec/controllers/help_controller_spec.rb @@ -111,10 +111,10 @@ describe HelpController do it 'renders the raw file' do get :show, params: { - path: 'user/project/img/labels_default' + path: 'user/project/img/labels_default_v12_1' }, format: :png - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'image/png' expect(response.headers['Content-Disposition']).to match(/^inline;/) end diff --git a/spec/controllers/ide_controller_spec.rb b/spec/controllers/ide_controller_spec.rb new file mode 100644 index 00000000000..0462f9520d5 --- /dev/null +++ b/spec/controllers/ide_controller_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe IdeController do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'increases the views counter' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_views_count) + + get :index + end +end diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb index 64a66502732..38388c21749 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -231,7 +231,7 @@ describe Import::BitbucketController do end end - context 'user has chosen an existing nested namespace and name for the project', :postgresql do + context 'user has chosen an existing nested namespace and name for the project' do let(:parent_namespace) { create(:group, name: 'foo') } let(:nested_namespace) { create(:group, name: 'bar', parent: parent_namespace) } let(:test_name) { 'test_name' } @@ -250,7 +250,7 @@ describe Import::BitbucketController do end end - context 'user has chosen a non-existent nested namespaces and name for the project', :postgresql do + context 'user has chosen a non-existent nested namespaces and name for the project' do let(:test_name) { 'test_name' } it 'takes the selected namespace and name' do @@ -281,7 +281,7 @@ describe Import::BitbucketController do end end - context 'user has chosen existent and non-existent nested namespaces and name for the project', :postgresql do + context 'user has chosen existent and non-existent nested namespaces and name for the project' do let(:test_name) { 'test_name' } let!(:parent_namespace) { create(:group, name: 'foo') } diff --git a/spec/controllers/import/bitbucket_server_controller_spec.rb b/spec/controllers/import/bitbucket_server_controller_spec.rb index b89d7317b9c..e1aeab46fca 100644 --- a/spec/controllers/import/bitbucket_server_controller_spec.rb +++ b/spec/controllers/import/bitbucket_server_controller_spec.rb @@ -134,6 +134,8 @@ describe Import::BitbucketServerController do describe 'GET status' do render_views + let(:repos) { instance_double(BitbucketServer::Collection) } + before do allow(controller).to receive(:bitbucket_client).and_return(client) @@ -145,7 +147,6 @@ describe Import::BitbucketServerController do it 'assigns repository categories' do created_project = create(:project, :import_finished, import_type: 'bitbucket_server', creator_id: user.id, import_source: @created_repo.browse_url) - repos = instance_double(BitbucketServer::Collection) expect(repos).to receive(:partition).and_return([[@repo, @created_repo], [@invalid_repo]]) expect(repos).to receive(:current_page).and_return(1) @@ -159,6 +160,17 @@ describe Import::BitbucketServerController do expect(assigns(:repos)).to eq([@repo]) expect(assigns(:incompatible_repos)).to eq([@invalid_repo]) end + + context 'when filtering' do + let(:filter) { 'test' } + + it 'passes filter param to bitbucket client' do + expect(repos).to receive(:partition).and_return([[@repo, @created_repo], [@invalid_repo]]) + expect(client).to receive(:repos).with(filter: filter, limit: 25, page_offset: 0).and_return(repos) + + get :status, params: { filter: filter }, as: :json + end + end end describe 'GET jobs' do diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 059354870b5..5675798ac33 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -33,6 +33,16 @@ describe Import::GithubController do expect(response).to have_http_status(200) end + + context 'when importing a CI/CD project' do + it 'always prompts for an access token' do + allow(controller).to receive(:github_import_configured?).and_return(true) + + get :new, params: { ci_cd_only: true } + + expect(response).to render_template(:new) + end + end end describe "GET callback" do diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb index 5af7572e74e..e465eca6c71 100644 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ b/spec/controllers/import/gitlab_controller_spec.rb @@ -197,7 +197,7 @@ describe Import::GitlabController do end end - context 'user has chosen an existing nested namespace for the project', :postgresql do + context 'user has chosen an existing nested namespace for the project' do let(:parent_namespace) { create(:group, name: 'foo') } let(:nested_namespace) { create(:group, name: 'bar', parent: parent_namespace) } @@ -215,7 +215,7 @@ describe Import::GitlabController do end end - context 'user has chosen a non-existent nested namespaces for the project', :postgresql do + context 'user has chosen a non-existent nested namespaces for the project' do let(:test_name) { 'test_name' } it 'takes the selected namespace and name' do @@ -246,7 +246,7 @@ describe Import::GitlabController do end end - context 'user has chosen existent and non-existent nested namespaces and name for the project', :postgresql do + context 'user has chosen existent and non-existent nested namespaces and name for the project' do let(:test_name) { 'test_name' } let!(:parent_namespace) { create(:group, name: 'foo') } diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index ee454a7818c..7fb3578cd0a 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -5,13 +5,19 @@ require 'spec_helper' describe MetricsController do include StubENV - let(:json_response) { JSON.parse(response.body) } - let(:metrics_multiproc_dir) { Dir.mktmpdir } + let(:metrics_multiproc_dir) { @metrics_multiproc_dir } let(:whitelisted_ip) { '127.0.0.1' } let(:whitelisted_ip_range) { '10.0.0.0/24' } let(:ip_in_whitelisted_range) { '10.0.0.1' } let(:not_whitelisted_ip) { '10.0.1.1' } + around do |example| + Dir.mktmpdir do |path| + @metrics_multiproc_dir = path + example.run + end + end + before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') allow(Prometheus::Client.configuration).to receive(:multiprocess_files_dir).and_return(metrics_multiproc_dir) diff --git a/spec/controllers/oauth/applications_controller_spec.rb b/spec/controllers/oauth/applications_controller_spec.rb index 228c97d591d..df836c2c3e3 100644 --- a/spec/controllers/oauth/applications_controller_spec.rb +++ b/spec/controllers/oauth/applications_controller_spec.rb @@ -17,7 +17,7 @@ describe Oauth::ApplicationsController do expect(response).to have_gitlab_http_status(200) end - it 'shows list of applications' do + it 'redirects back to profile page if OAuth applications are disabled' do disable_user_oauth get :index diff --git a/spec/controllers/profiles/keys_controller_spec.rb b/spec/controllers/profiles/keys_controller_spec.rb index 753eb432c5e..3bed117deb0 100644 --- a/spec/controllers/profiles/keys_controller_spec.rb +++ b/spec/controllers/profiles/keys_controller_spec.rb @@ -10,7 +10,7 @@ describe Profiles::KeysController do it "does not generally work" do get :get_keys, params: { username: 'not-existent' } - expect(response).not_to be_success + expect(response).not_to be_successful end end @@ -18,7 +18,7 @@ describe Profiles::KeysController do it "does generally work" do get :get_keys, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful end it "renders all keys separated with a new line" do @@ -41,7 +41,7 @@ describe Profiles::KeysController do it "does generally work" do get :get_keys, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful end it "renders all non deploy keys separated with a new line" do diff --git a/spec/controllers/projects/badges_controller_spec.rb b/spec/controllers/projects/badges_controller_spec.rb index 5ec8d8d41d7..4ae29ba7f54 100644 --- a/spec/controllers/projects/badges_controller_spec.rb +++ b/spec/controllers/projects/badges_controller_spec.rb @@ -7,51 +7,115 @@ describe Projects::BadgesController do let!(:pipeline) { create(:ci_empty_pipeline) } let(:user) { create(:user) } - before do - project.add_maintainer(user) - sign_in(user) - end + shared_examples 'a badge resource' do |badge_type| + context 'when pipelines are public' do + before do + project.update!(public_builds: true) + end + + context 'when project is public' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it "returns the #{badge_type} badge to unauthenticated users" do + get_badge(badge_type) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when project is restricted' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.add_guest(user) + sign_in(user) + end + + it "returns the #{badge_type} badge to guest users" do + get_badge(badge_type) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end - it 'requests the pipeline badge successfully' do - get_badge(:pipeline) + context 'format' do + before do + project.add_maintainer(user) + sign_in(user) + end - expect(response).to have_gitlab_http_status(:ok) - end + it 'renders the `flat` badge layout by default' do + get_badge(badge_type) - it 'requests the coverage badge successfully' do - get_badge(:coverage) + expect(response).to render_template('projects/badges/badge') + end - expect(response).to have_gitlab_http_status(:ok) - end + context 'when style param is set to `flat`' do + it 'renders the `flat` badge layout' do + get_badge(badge_type, 'flat') - it 'renders the `flat` badge layout by default' do - get_badge(:coverage) + expect(response).to render_template('projects/badges/badge') + end + end - expect(response).to render_template('projects/badges/badge') - end + context 'when style param is set to an invalid type' do + it 'renders the `flat` (default) badge layout' do + get_badge(badge_type, 'xxx') + + expect(response).to render_template('projects/badges/badge') + end + end - context 'when style param is set to `flat`' do - it 'renders the `flat` badge layout' do - get_badge(:coverage, 'flat') + context 'when style param is set to `flat-square`' do + it 'renders the `flat-square` badge layout' do + get_badge(badge_type, 'flat-square') - expect(response).to render_template('projects/badges/badge') + expect(response).to render_template('projects/badges/badge_flat-square') + end + end end - end - context 'when style param is set to an invalid type' do - it 'renders the `flat` (default) badge layout' do - get_badge(:coverage, 'xxx') + context 'when pipelines are not public' do + before do + project.update!(public_builds: false) + end - expect(response).to render_template('projects/badges/badge') + context 'when project is public' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it 'returns 404 to unauthenticated users' do + get_badge(badge_type) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when project is restricted to the user' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.add_guest(user) + sign_in(user) + end + + it 'defaults to project permissions' do + get_badge(:coverage) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end end - context 'when style param is set to `flat-square`' do - it 'renders the `flat-square` badge layout' do - get_badge(:coverage, 'flat-square') + describe '#pipeline' do + it_behaves_like 'a badge resource', :pipeline + end - expect(response).to render_template('projects/badges/badge_flat-square') - end + describe '#coverage' do + it_behaves_like 'a badge resource', :coverage end def get_badge(badge, style = nil) diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 44500d3cde3..45aebd1554c 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -160,7 +160,7 @@ describe Projects::BlobController do it 'renders diff context lines Gitlab::Diff::Line array' do do_get(since: 1, to: 2, offset: 0, from_merge_request: true) - lines = JSON.parse(response.body) + lines = json_response expect(lines.size).to eq(diff_lines.size) lines.each do |line| @@ -173,7 +173,7 @@ describe Projects::BlobController do it 'handles full being true' do do_get(full: true, from_merge_request: true) - lines = JSON.parse(response.body) + lines = json_response expect(lines.size).to eq(diff_lines.size) end diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb index c07afc57aea..543479d8dd5 100644 --- a/spec/controllers/projects/boards_controller_spec.rb +++ b/spec/controllers/projects/boards_controller_spec.rb @@ -69,10 +69,8 @@ describe Projects::BoardsController do list_boards format: :json - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('boards') - expect(parsed_response.length).to eq 2 + expect(json_response.length).to eq 2 end context 'with unauthorized user' do diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index b30966e70a7..f5bcea4a097 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -495,10 +495,8 @@ describe Projects::BranchesController do search: 'master' } - parsed_response = JSON.parse(response.body) - - expect(parsed_response.length).to eq 1 - expect(parsed_response.first).to eq 'master' + expect(json_response.length).to eq 1 + expect(json_response.first).to eq 'master' end end @@ -591,8 +589,7 @@ describe Projects::BranchesController do end it 'returns the commit counts behind and ahead of default branch' do - parsed_response = JSON.parse(response.body) - expect(parsed_response).to eq( + expect(json_response).to eq( "fix" => { "behind" => 29, "ahead" => 2 }, "branch-merged" => { "behind" => 1, "ahead" => 0 }, "add-pdf-file" => { "behind" => 0, "ahead" => 3 } diff --git a/spec/controllers/projects/ci/lints_controller_spec.rb b/spec/controllers/projects/ci/lints_controller_spec.rb index 96e82b7086c..14128fb5b0e 100644 --- a/spec/controllers/projects/ci/lints_controller_spec.rb +++ b/spec/controllers/projects/ci/lints_controller_spec.rb @@ -20,9 +20,7 @@ describe Projects::Ci::LintsController do get :show, params: { namespace_id: project.namespace, project_id: project } end - it 'is success' do - expect(response).to be_success - end + it { expect(response).to be_successful } it 'renders show page' do expect(response).to render_template :show @@ -78,9 +76,7 @@ describe Projects::Ci::LintsController do post :create, params: { namespace_id: project.namespace, project_id: project, content: content } end - it 'is success' do - expect(response).to be_success - end + it { expect(response).to be_successful } it 'render show page' do expect(response).to render_template :show diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index b5c6382a26d..afd5cb15e0f 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -45,14 +45,14 @@ describe Projects::CommitController do it 'handles binary files' do go(id: TestEnv::BRANCH_SHA['binary-encoding'], format: 'html') - expect(response).to be_success + expect(response).to be_successful end shared_examples "export as" do |format| it "does generally work" do go(id: commit.id, format: format) - expect(response).to be_success + expect(response).to be_successful end it "generates it" do @@ -110,7 +110,7 @@ describe Projects::CommitController do id: commit.id }) - expect(response).to be_success + expect(response).to be_successful end end @@ -177,7 +177,7 @@ describe Projects::CommitController do id: commit.id }) - expect(response).not_to be_success + expect(response).not_to be_successful expect(response).to have_gitlab_http_status(404) end end @@ -234,7 +234,7 @@ describe Projects::CommitController do id: master_pickable_commit.id }) - expect(response).not_to be_success + expect(response).not_to be_successful expect(response).to have_gitlab_http_status(404) end end @@ -378,8 +378,8 @@ describe Projects::CommitController do get_pipelines(id: commit.id, format: :json) expect(response).to be_ok - expect(JSON.parse(response.body)['pipelines']).not_to be_empty - expect(JSON.parse(response.body)['count']['all']).to eq 1 + expect(json_response['pipelines']).not_to be_empty + expect(json_response['count']['all']).to eq 1 expect(response).to include_pagination_headers end end diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb index 9db1ac2a46c..9c4d6fdcb2a 100644 --- a/spec/controllers/projects/commits_controller_spec.rb +++ b/spec/controllers/projects/commits_controller_spec.rb @@ -79,7 +79,7 @@ describe Projects::CommitsController do end it "renders as atom" do - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq('application/atom+xml') end @@ -104,7 +104,7 @@ describe Projects::CommitsController do end it "renders as HTML" do - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq('text/html') end end diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 92380a2bf09..9afc46c4be9 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -19,7 +19,7 @@ describe Projects::CompareController do end it 'returns successfully' do - expect(response).to be_success + expect(response).to be_successful end end @@ -49,7 +49,7 @@ describe Projects::CompareController do it 'shows some diffs with ignore whitespace change option' do show_request - expect(response).to be_success + expect(response).to be_successful diff_file = assigns(:diffs).diff_files.first expect(diff_file).not_to be_nil expect(assigns(:commits).length).to be >= 1 @@ -67,7 +67,7 @@ describe Projects::CompareController do it 'sets the diffs and commits ivars' do show_request - expect(response).to be_success + expect(response).to be_successful expect(assigns(:diffs).diff_files.first).not_to be_nil expect(assigns(:commits).length).to be >= 1 end @@ -81,7 +81,7 @@ describe Projects::CompareController do it 'sets empty diff and commit ivars' do show_request - expect(response).to be_success + expect(response).to be_successful expect(assigns(:diffs)).to eq([]) expect(assigns(:commits)).to eq([]) end @@ -94,7 +94,7 @@ describe Projects::CompareController do it 'sets empty diff and commit ivars' do show_request - expect(response).to be_success + expect(response).to be_successful expect(assigns(:diffs)).to eq([]) expect(assigns(:commits)).to eq([]) end @@ -302,8 +302,7 @@ describe Projects::CompareController do signatures_request expect(response).to have_gitlab_http_status(200) - parsed_body = JSON.parse(response.body) - signatures = parsed_body['signatures'] + signatures = json_response['signatures'] expect(signatures.size).to eq(1) expect(signatures.first['commit_sha']).to eq(signature_commit.sha) @@ -332,8 +331,7 @@ describe Projects::CompareController do signatures_request expect(response).to have_gitlab_http_status(200) - parsed_body = JSON.parse(response.body) - expect(parsed_body['signatures']).to be_empty + expect(json_response['signatures']).to be_empty end end @@ -345,8 +343,7 @@ describe Projects::CompareController do signatures_request expect(response).to have_gitlab_http_status(200) - parsed_body = JSON.parse(response.body) - expect(parsed_body['signatures']).to be_empty + expect(json_response['signatures']).to be_empty end end end diff --git a/spec/controllers/projects/cycle_analytics/events_controller_spec.rb b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb new file mode 100644 index 00000000000..b828c678d0c --- /dev/null +++ b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::CycleAnalytics::EventsController do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + + before do + sign_in(user) + project.add_maintainer(user) + end + + describe 'cycle analytics not set up flag' do + context 'with no data' do + it 'is empty' do + get_issue + + expect(response).to be_successful + expect(JSON.parse(response.body)['events']).to be_empty + end + end + + context 'with data' do + let(:milestone) { create(:milestone, project: project, created_at: 10.days.ago) } + let(:issue) { create(:issue, project: project, created_at: 9.days.ago) } + + before do + issue.update(milestone: milestone) + end + + it 'is not empty' do + get_issue + + expect(response).to be_successful + end + + it 'contains event detais' do + get_issue + + events = JSON.parse(response.body)['events'] + + expect(events).not_to be_empty + expect(events.first).to include('title', 'author', 'iid', 'total_time', 'created_at', 'url') + expect(events.first['title']).to eq(issue.title) + end + + context 'with data older than start date' do + it 'is empty' do + get_issue(additional_params: { cycle_analytics: { start_date: 7 } }) + + expect(response).to be_successful + + expect(JSON.parse(response.body)['events']).to be_empty + end + end + end + end + + def get_issue(additional_params: {}) + params = additional_params.merge(namespace_id: project.namespace, project_id: project) + get(:issue, params: params, format: :json) + end +end diff --git a/spec/controllers/projects/cycle_analytics_controller_spec.rb b/spec/controllers/projects/cycle_analytics_controller_spec.rb index 2dc97e18113..65eee7b8ead 100644 --- a/spec/controllers/projects/cycle_analytics_controller_spec.rb +++ b/spec/controllers/projects/cycle_analytics_controller_spec.rb @@ -11,6 +11,20 @@ describe Projects::CycleAnalyticsController do project.add_maintainer(user) end + context "counting page views for 'show'" do + it 'increases the counter' do + expect(Gitlab::UsageDataCounters::CycleAnalyticsCounter).to receive(:count).with(:views) + + get(:show, + params: { + namespace_id: project.namespace, + project_id: project + }) + + expect(response).to be_successful + end + end + describe 'cycle analytics not set up flag' do context 'with no data' do it 'is true' do @@ -20,7 +34,7 @@ describe Projects::CycleAnalyticsController do project_id: project }) - expect(response).to be_success + expect(response).to be_successful expect(assigns(:cycle_analytics_no_data)).to eq(true) end end @@ -41,7 +55,7 @@ describe Projects::CycleAnalyticsController do project_id: project }) - expect(response).to be_success + expect(response).to be_successful expect(assigns(:cycle_analytics_no_data)).to eq(false) end end diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb index fcd14f13863..ccad76eaddd 100644 --- a/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -52,12 +52,10 @@ describe Projects::DeployKeysController do it 'returns json in a correct format' do get :index, params: params.merge(format: :json) - json = JSON.parse(response.body) - - expect(json.keys).to match_array(%w(enabled_keys available_project_keys public_keys)) - expect(json['enabled_keys'].count).to eq(1) - expect(json['available_project_keys'].count).to eq(1) - expect(json['public_keys'].count).to eq(1) + expect(json_response.keys).to match_array(%w(enabled_keys available_project_keys public_keys)) + expect(json_response['enabled_keys'].count).to eq(1) + expect(json_response['available_project_keys'].count).to eq(1) + expect(json_response['public_keys'].count).to eq(1) end end end diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb index 4c29162cd0f..e30b28a4bd5 100644 --- a/spec/controllers/projects/discussions_controller_spec.rb +++ b/spec/controllers/projects/discussions_controller_spec.rb @@ -112,7 +112,7 @@ describe Projects::DiscussionsController do it "returns the name of the resolving user" do post :resolve, params: request_params - expect(JSON.parse(response.body)['resolved_by']['name']).to eq(user.name) + expect(json_response['resolved_by']['name']).to eq(user.name) end it "returns status 200" do @@ -135,7 +135,7 @@ describe Projects::DiscussionsController do it "returns truncated diff lines" do post :resolve, params: request_params - expect(JSON.parse(response.body)['truncated_diff_lines']).to be_present + expect(json_response['truncated_diff_lines']).to be_present end end end diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb index fdef9bc5638..45328482ad7 100644 --- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb +++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb @@ -176,7 +176,7 @@ describe Projects::Environments::PrometheusApiController do def environment_params(params = {}) { id: environment.id.to_s, - namespace_id: project.namespace.name, + namespace_id: project.namespace.full_path, project_id: project.name, proxy_path: 'query', query: '1' diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 4c2c6160c62..71ee1fd03bf 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Projects::EnvironmentsController do + include MetricsDashboardHelpers + set(:user) { create(:user) } set(:project) { create(:project) } @@ -445,131 +447,187 @@ describe Projects::EnvironmentsController do end end - describe 'metrics_dashboard' do - context 'when prometheus endpoint is disabled' do - before do - stub_feature_flags(environment_metrics_use_prometheus_endpoint: false) - end + describe 'GET #metrics_dashboard' do + shared_examples_for 'correctly formatted response' do |status_code| + it 'returns a json object with the correct keys' do + get :metrics_dashboard, params: environment_params(dashboard_params) - it 'responds with status code 403' do - get :metrics_dashboard, params: environment_params(format: :json) + # Exlcude `all_dashboards` to handle separately. + found_keys = json_response.keys - ['all_dashboards'] - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(status_code) + expect(found_keys).to contain_exactly(*expected_keys) end end - shared_examples_for '200 response' do |contains_all_dashboards: false| + shared_examples_for '200 response' do let(:expected_keys) { %w(dashboard status) } - before do - expected_keys << 'all_dashboards' if contains_all_dashboards - end - - it 'returns a json representation of the environment dashboard' do - get :metrics_dashboard, params: environment_params(dashboard_params) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.keys).to contain_exactly(*expected_keys) - expect(json_response['dashboard']).to be_an_instance_of(Hash) - end + it_behaves_like 'correctly formatted response', :ok end - shared_examples_for 'error response' do |status_code, contains_all_dashboards: false| + shared_examples_for 'error response' do |status_code| let(:expected_keys) { %w(message status) } - before do - expected_keys << 'all_dashboards' if contains_all_dashboards - end + it_behaves_like 'correctly formatted response', status_code + end - it 'returns an error response' do + shared_examples_for 'includes all dashboards' do + it 'includes info for all findable dashboard' do get :metrics_dashboard, params: environment_params(dashboard_params) - expect(response).to have_gitlab_http_status(status_code) - expect(json_response.keys).to contain_exactly(*expected_keys) + 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') ) end end - shared_examples_for 'has all dashboards' do - it 'includes an index of all available dashboards' do + shared_examples_for 'the default dashboard' do + all_dashboards = Feature.enabled?(:environment_metrics_show_multiple_dashboards) + + it_behaves_like '200 response' + it_behaves_like 'includes all dashboards' if all_dashboards + + it 'is the default dashboard' do get :metrics_dashboard, params: environment_params(dashboard_params) - expect(json_response.keys).to include('all_dashboards') - expect(json_response['all_dashboards']).to be_an_instance_of(Array) - expect(json_response['all_dashboards']).to all( include('path', 'default') ) + expect(json_response['dashboard']['dashboard']).to eq('Environment metrics') end end - context 'when multiple dashboards is disabled' do - before do - stub_feature_flags(environment_metrics_show_multiple_dashboards: false) - end + shared_examples_for 'the specified dashboard' do |expected_dashboard| + it_behaves_like '200 response' + it_behaves_like 'includes all dashboards' - let(:dashboard_params) { { format: :json } } + it 'has the correct name' do + get :metrics_dashboard, params: environment_params(dashboard_params) - it_behaves_like '200 response' + dashboard_name = json_response['dashboard']['dashboard'] - context 'when the dashboard could not be provided' do + # 'Environment metrics' is the default dashboard. + expect(dashboard_name).not_to eq('Environment metrics') + expect(dashboard_name).to eq(expected_dashboard) + end + + context 'when the dashboard cannot not be processed' do before do allow(YAML).to receive(:safe_load).and_return({}) end it_behaves_like 'error response', :unprocessable_entity end - - context 'when a dashboard param is specified' do - let(:dashboard_params) { { format: :json, dashboard: '.gitlab/dashboards/not_there_dashboard.yml' } } - - it_behaves_like '200 response' - end end - context 'when multiple dashboards is enabled' do - let(:dashboard_params) { { format: :json } } + shared_examples_for 'specified dashboard embed' do |expected_titles| + it_behaves_like '200 response' - it_behaves_like '200 response', contains_all_dashboards: true - it_behaves_like 'has all dashboards' + it 'contains only the specified charts' do + get :metrics_dashboard, params: environment_params(dashboard_params) - context 'when a dashboard could not be provided' do - before do - allow(YAML).to receive(:safe_load).and_return({}) - end + dashboard = json_response['dashboard'] + panel_group = dashboard['panel_groups'].first + titles = panel_group['panels'].map { |panel| panel['title'] } - it_behaves_like 'error response', :unprocessable_entity, contains_all_dashboards: true - it_behaves_like 'has all dashboards' + expect(dashboard['dashboard']).to be_nil + expect(dashboard['panel_groups'].length).to eq 1 + expect(panel_group['group']).to be_nil + expect(titles).to eq expected_titles end + end - context 'when a dashboard param is specified' do - let(:dashboard_params) { { format: :json, dashboard: '.gitlab/dashboards/test.yml' } } + shared_examples_for 'the default dynamic dashboard' do + it_behaves_like 'specified dashboard embed', ['Memory Usage (Total)', 'Core Usage (Total)'] + end + + shared_examples_for 'dashboard can be specified' do + context 'when dashboard is specified' do + let(:dashboard_path) { '.gitlab/dashboards/test.yml' } + let(:dashboard_params) { { format: :json, dashboard: dashboard_path } } - context 'when the dashboard is available' do + it_behaves_like 'error response', :not_found + + context 'when the project dashboard is available' do let(:dashboard_yml) { fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml') } - let(:dashboard_file) { { '.gitlab/dashboards/test.yml' => dashboard_yml } } - let(:project) { create(:project, :custom_repo, files: dashboard_file) } + let(:project) { project_with_dashboard(dashboard_path, dashboard_yml) } let(:environment) { create(:environment, name: 'production', project: project) } - it_behaves_like '200 response', contains_all_dashboards: true - it_behaves_like 'has all dashboards' + it_behaves_like 'the specified dashboard', 'Test Dashboard' end - context 'when the dashboard does not exist' do - it_behaves_like 'error response', :not_found, contains_all_dashboards: true - it_behaves_like 'has all dashboards' + context 'when the specified dashboard is the default dashboard' do + let(:dashboard_path) { system_dashboard_path } + + it_behaves_like 'the default dashboard' end end + end - context 'when the dashboard is intended for embedding' do + shared_examples_for 'dashboard can be embedded' do + context 'when the embedded flag is included' do let(:dashboard_params) { { format: :json, embedded: true } } - it_behaves_like '200 response' + it_behaves_like 'the default dynamic dashboard' + + context 'when incomplete dashboard params are provided' do + let(:dashboard_params) { { format: :json, embedded: true, title: 'Title' } } + + # The title param should be ignored. + it_behaves_like 'the default dynamic dashboard' + end + + context 'when invalid params are provided' do + let(:dashboard_params) { { format: :json, embedded: true, metric_id: 16 } } - context 'when a dashboard path is provided' do - let(:dashboard_params) { { format: :json, dashboard: '.gitlab/dashboards/test.yml', embedded: true } } + # The superfluous param should be ignored. + it_behaves_like 'the default dynamic dashboard' + end - # The dashboard path should simple be ignored. - it_behaves_like '200 response' + context 'when the dashboard is correctly specified' do + let(:dashboard_params) do + { + format: :json, + embedded: true, + dashboard: system_dashboard_path, + group: business_metric_title, + title: 'title', + y_label: 'y_label' + } + end + + it_behaves_like 'error response', :not_found + + context 'and exists' do + let!(:metric) { create(:prometheus_metric, project: project) } + + it_behaves_like 'specified dashboard embed', ['title'] + end end end end + + shared_examples_for 'dashboard cannot be specified' do + context 'when dashboard is specified' do + let(:dashboard_params) { { format: :json, dashboard: '.gitlab/dashboards/test.yml' } } + + it_behaves_like 'the default dashboard' + end + end + + let(:dashboard_params) { { format: :json } } + + it_behaves_like 'the default dashboard' + it_behaves_like 'dashboard can be specified' + it_behaves_like 'dashboard can be embedded' + + context 'when multiple dashboards is disabled' do + before do + stub_feature_flags(environment_metrics_show_multiple_dashboards: false) + end + + it_behaves_like 'the default dashboard' + it_behaves_like 'dashboard cannot be specified' + it_behaves_like 'dashboard can be embedded' + end end describe 'GET #search' do diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 844c61f1ace..d11ef24ef96 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'rails_helper' +require 'spec_helper' describe Projects::ErrorTrackingController do set(:project) { create(:project) } diff --git a/spec/controllers/projects/find_file_controller_spec.rb b/spec/controllers/projects/find_file_controller_spec.rb index 538dbb5ad0b..a493985f8a0 100644 --- a/spec/controllers/projects/find_file_controller_spec.rb +++ b/spec/controllers/projects/find_file_controller_spec.rb @@ -53,10 +53,9 @@ describe Projects::FindFileController do it 'returns an array of file path list' do go - json = JSON.parse(response.body) is_expected.to respond_with(:success) - expect(json).not_to eq(nil) - expect(json.length).to be >= 0 + expect(json_response).not_to eq(nil) + expect(json_response.length).to be >= 0 end end diff --git a/spec/controllers/projects/git_http_controller_spec.rb b/spec/controllers/projects/git_http_controller_spec.rb index bf099e8deeb..88fa2236e33 100644 --- a/spec/controllers/projects/git_http_controller_spec.rb +++ b/spec/controllers/projects/git_http_controller_spec.rb @@ -12,4 +12,15 @@ describe Projects::GitHttpController do expect(response.status).to eq(403) end end + + describe 'GET #info_refs' do + it 'returns 401 for unauthenticated requests to public repositories when http protocol is disabled' do + stub_application_setting(enabled_git_access_protocol: 'ssh') + project = create(:project, :public, :repository) + + get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } + + expect(response.status).to eq(401) + end + end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index bc5e0b4671e..187c7864ad7 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -251,15 +251,13 @@ describe Projects::IssuesController do end end - describe 'Redirect after sign in' do + # This spec runs as a request-style spec in order to invoke the + # Rails router. A controller-style spec matches the wrong route, and + # session['user_return_to'] becomes incorrect. + describe 'Redirect after sign in', type: :request do context 'with an AJAX request' do it 'does not store the visited URL' do - get :show, params: { - format: :json, - namespace_id: project.namespace, - project_id: project, - id: issue.iid - }, xhr: true + get project_issue_path(project, issue), xhr: true expect(session['user_return_to']).to be_blank end @@ -267,14 +265,9 @@ describe Projects::IssuesController do context 'without an AJAX request' do it 'stores the visited URL' do - get :show, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: issue.iid - } + get project_issue_path(project, issue) - expect(session['user_return_to']).to eq("/#{project.namespace.to_param}/#{project.to_param}/issues/#{issue.iid}") + expect(session['user_return_to']).to eq(project_issue_path(project, issue)) end end end @@ -444,7 +437,7 @@ describe Projects::IssuesController do it 'renders json with recaptcha_html' do subject - expect(JSON.parse(response.body)).to have_key('recaptcha_html') + expect(json_response).to have_key('recaptcha_html') end end end @@ -484,10 +477,8 @@ describe Projects::IssuesController do it 'returns last edited time' do go(id: issue.iid) - data = JSON.parse(response.body) - - expect(data).to include('updated_at') - expect(data['updated_at']).to eq(issue.last_edited_at.to_time.iso8601) + expect(json_response).to include('updated_at') + expect(json_response['updated_at']).to eq(issue.last_edited_at.to_time.iso8601) end end @@ -520,10 +511,8 @@ describe Projects::IssuesController do it 'returns the necessary data' do go(id: issue.iid) - data = JSON.parse(response.body) - - expect(data).to include('title_text', 'description', 'description_text') - expect(data).to include('task_status', 'lock_version') + expect(json_response).to include('title_text', 'description', 'description_text') + expect(json_response).to include('task_status', 'lock_version') end end end @@ -692,9 +681,7 @@ describe Projects::IssuesController do update_issue(issue_params: { assignee_ids: [assignee.id] }) - body = JSON.parse(response.body) - - expect(body['assignees'].first.keys) + expect(json_response['assignees'].first.keys) .to match_array(%w(id name username avatar_url state web_url)) end end @@ -1117,18 +1104,39 @@ describe Projects::IssuesController do project.add_developer(user) end + subject do + post(:toggle_award_emoji, params: { + namespace_id: project.namespace, + project_id: project, + id: issue.iid, + name: emoji_name + }) + end + let(:emoji_name) { 'thumbsup' } + it "toggles the award emoji" do expect do - post(:toggle_award_emoji, params: { - namespace_id: project.namespace, - project_id: project, - id: issue.iid, - name: "thumbsup" - }) + subject end.to change { issue.award_emoji.count }.by(1) expect(response).to have_gitlab_http_status(200) end + + it "removes the already awarded emoji" do + create(:award_emoji, awardable: issue, name: emoji_name, user: user) + + expect { subject }.to change { AwardEmoji.count }.by(-1) + + expect(response).to have_gitlab_http_status(200) + end + + it 'marks Todos on the Issue as done' do + todo = create(:todo, target: issue, project: project, user: user) + + subject + + expect(todo.reload).to be_done + end end describe 'POST create_merge_request' do @@ -1266,6 +1274,28 @@ describe Projects::IssuesController do sign_in(user) end + context do + it_behaves_like 'discussions provider' do + let!(:author) { create(:user) } + let!(:project) { create(:project) } + + let!(:issue) { create(:issue, project: project, author: user) } + + let!(:note_on_issue1) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, author: create(:user)) } + let!(:note_on_issue2) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, author: create(:user)) } + + let(:requested_iid) { issue.iid } + let(:expected_discussion_count) { 3 } + let(:expected_discussion_ids) do + [ + issue.notes.first.discussion_id, + note_on_issue1.discussion_id, + note_on_issue2.discussion_id + ] + end + end + end + it 'returns discussion json' do get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } @@ -1314,7 +1344,7 @@ describe Projects::IssuesController do it 'filters notes that the user should not see' do get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - expect(JSON.parse(response.body).count).to eq(1) + expect(json_response.count).to eq(1) end it 'does not result in N+1 queries' do diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 901402aa5fd..f076a5e769f 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -158,7 +158,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do get_show_json json_response.dig('pipeline', 'details', 'stages').tap do |stages| - expect(stages.map(&:keys).flatten) + expect(stages.flat_map(&:keys)) .to eq %w[name title status path dropdown_path] end end @@ -546,7 +546,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq job.id expect(json_response['status']).to eq job.status - expect(json_response['html']).to eq('<span class="">BUILD TRACE</span>') + expect(json_response['html']).to eq('<span>BUILD TRACE</span>') end end @@ -676,6 +676,8 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do end describe 'POST play' do + let(:variable_attributes) { [] } + before do project.add_developer(user) @@ -698,6 +700,14 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do it 'transits to pending' do expect(job.reload).to be_pending end + + context 'when job variables are specified' do + let(:variable_attributes) { [{ key: 'first', secret_value: 'first' }] } + + it 'assigns the job variables' do + expect(job.reload.job_variables.map(&:key)).to contain_exactly('first') + end + end end context 'when job is not playable' do @@ -712,7 +722,8 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do post :play, params: { namespace_id: project.namespace, project_id: project, - id: job.id + id: job.id, + job_variables_attributes: variable_attributes } end end diff --git a/spec/controllers/projects/merge_requests/content_controller_spec.rb b/spec/controllers/projects/merge_requests/content_controller_spec.rb index 2879e06aee4..818cf794ec6 100644 --- a/spec/controllers/projects/merge_requests/content_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/content_controller_spec.rb @@ -11,8 +11,8 @@ describe Projects::MergeRequests::ContentController do sign_in(user) end - def do_request - get :widget, params: { + def do_request(action = :cached_widget) + get action, params: { namespace_id: project.namespace.to_param, project_id: project, id: merge_request.iid, @@ -20,41 +20,65 @@ describe Projects::MergeRequests::ContentController do } end - describe 'GET widget' do - context 'user has access to the project' do - before do - expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + context 'user has access to the project' do + before do + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original - project.add_maintainer(user) - end + project.add_maintainer(user) + end + describe 'GET cached_widget' do it 'renders widget MR entity as json' do do_request - expect(response).to match_response_schema('entities/merge_request_widget') + expect(response).to match_response_schema('entities/merge_request_poll_cached_widget') end + it 'closes an MR with moved source project' do + merge_request.update_column(:source_project_id, nil) + + expect { do_request }.to change { merge_request.reload.open? }.from(true).to(false) + end + end + + describe 'GET widget' do it 'checks whether the MR can be merged' do controller.instance_variable_set(:@merge_request, merge_request) expect(merge_request).to receive(:check_mergeability) - do_request + do_request(:widget) end - it 'closes an MR with moved source project' do - merge_request.update_column(:source_project_id, nil) + context 'merged merge request' do + let(:merge_request) do + create(:merged_merge_request, :with_test_reports, target_project: project, source_project: project) + end - expect { do_request }.to change { merge_request.reload.open? }.from(true).to(false) + it 'renders widget MR entity as json' do + do_request(:widget) + + expect(response).to match_response_schema('entities/merge_request_poll_widget') + end end end + end - context 'user does not have access to the project' do - it 'renders widget MR entity as json' do + context 'user does not have access to the project' do + describe 'GET cached_widget' do + it 'returns 404' do do_request expect(response).to have_http_status(:not_found) end end + + describe 'GET widget' do + it 'returns 404' do + do_request(:widget) + + expect(response).to have_http_status(:not_found) + end + end end end diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index 5fefad86ef3..ce977f26ec6 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -28,7 +28,7 @@ describe Projects::MergeRequests::CreationsController do it 'renders new merge request widget template' do get :new, params: get_diff_params - expect(response).to be_success + expect(response).to be_successful end end @@ -56,7 +56,7 @@ describe Projects::MergeRequests::CreationsController do it 'limits total commits' do get :new, params: large_diff_params - expect(response).to be_success + expect(response).to be_successful total = assigns(:total_commit_count) expect(assigns(:commits)).to be_an Array @@ -70,7 +70,7 @@ describe Projects::MergeRequests::CreationsController do it 'shows total commits' do get :new, params: large_diff_params - expect(response).to be_success + expect(response).to be_successful total = assigns(:total_commit_count) expect(assigns(:commits)).to be_an CommitCollection @@ -89,7 +89,7 @@ describe Projects::MergeRequests::CreationsController do get :diffs, params: get_diff_params.merge(format: 'json') - expect(response).to be_success + expect(response).to be_successful expect(assigns[:diffs]).to be_nil end end @@ -212,4 +212,46 @@ describe Projects::MergeRequests::CreationsController do expect(response).to have_gitlab_http_status(200) end end + + describe 'POST create' do + let(:params) do + { + namespace_id: fork_project.namespace.to_param, + project_id: fork_project, + merge_request: { + title: 'Test merge request', + source_branch: 'remove-submodule', + target_branch: 'master' + } + } + end + + it 'creates merge request' do + expect do + post_request(params) + end.to change { MergeRequest.count }.by(1) + end + + context 'when the merge request is not created from the web ide' do + it 'counter is not increased' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_merge_requests_count) + + post_request(params) + end + end + + context 'when the merge request is created from the web ide' do + let(:nav_source) { { nav_source: 'webide' } } + + it 'counter is increased' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_merge_requests_count) + + post_request(params.merge(nav_source)) + end + end + + def post_request(merge_request_params) + post :create, params: merge_request_params + 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 13a28b738ca..ac3e9901123 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -66,7 +66,7 @@ describe Projects::MergeRequests::DiffsController do end it 'renders' do - expect(response).to be_success + expect(response).to be_successful expect(response.body).to have_content('Subproject commit') end end @@ -112,7 +112,7 @@ describe Projects::MergeRequests::DiffsController do it 'only renders the diffs for the path given' do diff_for_path(old_path: existing_path, new_path: existing_path) - paths = JSON.parse(response.body)["diff_files"].map { |file| file['new_path'] } + paths = json_response["diff_files"].map { |file| file['new_path'] } expect(paths).to include(existing_path) end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 9878f88a395..11b1eaf11b7 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -57,7 +57,7 @@ describe Projects::MergeRequestsController do go(format: :html) - expect(response).to be_success + expect(response).to be_successful end end @@ -66,7 +66,7 @@ describe Projects::MergeRequestsController do go(format: :html) - expect(response).to be_success + expect(response).to be_successful end context "that is invalid" do @@ -75,7 +75,7 @@ describe Projects::MergeRequestsController do it "renders merge request page" do go(format: :html) - expect(response).to be_success + expect(response).to be_successful end end end @@ -124,7 +124,7 @@ describe Projects::MergeRequestsController do it "renders merge request page" do go(format: :json) - expect(response).to be_success + expect(response).to be_successful end end end @@ -242,9 +242,7 @@ describe Projects::MergeRequestsController do update_merge_request({ assignee_ids: [assignee.id] }, format: :json) - body = JSON.parse(response.body) - - expect(body['assignees']).to all(include(*%w(name username avatar_url id state web_url))) + expect(json_response['assignees']).to all(include(*%w(name username avatar_url id state web_url))) end end @@ -623,10 +621,100 @@ describe Projects::MergeRequestsController do format: :json end - it 'responds with serialized pipelines' do - expect(json_response['pipelines']).not_to be_empty - expect(json_response['count']['all']).to eq 1 - expect(response).to include_pagination_headers + context 'with "enabled" builds on a public project' do + let(:project) { create(:project, :repository, :public) } + + context 'for a project owner' do + it 'responds with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + + context 'for an unassociated user' do + let(:user) { create :user } + + it 'responds with no pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + end + + context 'with private builds on a public project' do + let(:project) { create(:project, :repository, :public, :builds_private) } + + context 'for a project owner' do + it 'responds with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + + context 'for an unassociated user' do + let(:user) { create :user } + + it 'responds with no pipelines' do + expect(json_response['pipelines']).to be_empty + expect(json_response['count']['all']).to eq(0) + expect(response).to include_pagination_headers + end + end + + context 'from a project fork' do + let(:fork_user) { create :user } + let(:forked_project) { fork_project(project, fork_user, repository: true) } # Forked project carries over :builds_private + let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: forked_project) } + + context 'with private builds' do + context 'for the target project member' do + it 'does not respond with serialized pipelines' do + expect(json_response['pipelines']).to be_empty + expect(json_response['count']['all']).to eq(0) + expect(response).to include_pagination_headers + end + end + + context 'for the source project member' do + let(:user) { fork_user } + + it 'responds with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + end + + context 'with public builds' do + let(:forked_project) do + fork_project(project, fork_user, repository: true).tap do |new_project| + new_project.project_feature.update(builds_access_level: ProjectFeature::ENABLED) + end + end + + context 'for the target project member' do + it 'does not respond with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + + context 'for the source project member' do + let(:user) { fork_user } + + it 'responds with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + end + end end end @@ -878,7 +966,7 @@ describe Projects::MergeRequestsController do expect(control_count).to be <= 137 end - it 'has no N+1 issues for environments', :request_store, retry: 0 do + it 'has no N+1 SQL issues for environments', :request_store, retry: 0 do # First run to insert test data from lets, which does take up some 30 queries get_ci_environments_status @@ -887,24 +975,70 @@ describe Projects::MergeRequestsController do environment2 = create(:environment, project: forked) create(:deployment, :succeed, environment: environment2, sha: sha, ref: 'master', deployable: build) - # TODO address the last 11 queries + # TODO address the last 5 queries # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63952 (5 queries) - # And https://gitlab.com/gitlab-org/gitlab-ce/issues/64105 (6 queries) - leeway = 11 + leeway = 5 expect { get_ci_environments_status }.not_to exceed_all_query_limit(control_count + leeway) end + end - def get_ci_environments_status(extra_params = {}) - params = { - namespace_id: merge_request.project.namespace.to_param, - project_id: merge_request.project, - id: merge_request.iid, - format: 'json' - } + context 'when a merge request has multiple environments with deployments' do + let(:sha) { merge_request.diff_head_sha } + let(:ref) { merge_request.source_branch } + + let!(:build) { create(:ci_build, pipeline: pipeline) } + let!(:pipeline) { create(:ci_pipeline, sha: sha, project: project) } + let!(:environment) { create(:environment, name: 'env_a', project: project) } + let!(:another_environment) { create(:environment, name: 'env_b', project: project) } + + before do + merge_request.update_head_pipeline + + create(:deployment, :succeed, environment: environment, sha: sha, ref: ref, deployable: build) + create(:deployment, :succeed, environment: another_environment, sha: sha, ref: ref, deployable: build) + end + + it 'exposes multiple environment statuses' do + get_ci_environments_status + + expect(json_response.count).to eq 2 + end + + context 'when route map is not present in the project' do + it 'does not have N+1 Gitaly requests for environments', :request_store do + expect(merge_request).to be_present + + expect { get_ci_environments_status } + .to change { Gitlab::GitalyClient.get_request_count }.by_at_most(1) + end + end - get :ci_environments_status, params: params.merge(extra_params) + context 'when there is route map present in a project' do + before do + allow_any_instance_of(EnvironmentStatus) + .to receive(:has_route_map?) + .and_return(true) + end + + it 'does not have N+1 Gitaly requests for diff files', :request_store do + expect(merge_request.merge_request_diff.merge_request_diff_files).to be_many + + expect { get_ci_environments_status } + .to change { Gitlab::GitalyClient.get_request_count }.by_at_most(1) + end end end + + def get_ci_environments_status(extra_params = {}) + params = { + namespace_id: merge_request.project.namespace.to_param, + project_id: merge_request.project, + id: merge_request.iid, + format: 'json' + } + + get :ci_environments_status, params: params.merge(extra_params) + end end describe 'GET pipeline_status.json' do @@ -1076,6 +1210,22 @@ describe Projects::MergeRequestsController do end end end + + context do + it_behaves_like 'discussions provider' do + let!(:author) { create(:user) } + let!(:project) { create(:project) } + + let!(:merge_request) { create(:merge_request, source_project: project) } + + let!(:mr_note1) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + let!(:mr_note2) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + let(:requested_iid) { merge_request.iid } + let(:expected_discussion_count) { 2 } + let(:expected_discussion_ids) { [mr_note1.discussion_id, mr_note2.discussion_id] } + end + end end describe 'GET edit' do diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 767cee7d54a..cbf9d437909 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -115,7 +115,7 @@ describe Projects::MilestonesController do end end - context 'with nested groups', :nested_groups do + context 'with nested groups' do let!(:subgroup) { create(:group, :public, parent: group) } let!(:subgroup_milestone) { create(:milestone, group: subgroup) } @@ -139,7 +139,7 @@ describe Projects::MilestonesController do expect(issue.milestone_id).to eq(milestone.id) delete :destroy, params: { namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid }, format: :js - expect(response).to be_success + expect(response).to be_successful expect(Event.recent.first.action).to eq(Event::DESTROYED) diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 1db1963476c..4500c412521 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -29,7 +29,7 @@ describe Projects::NotesController do } end - let(:parsed_response) { JSON.parse(response.body).with_indifferent_access } + let(:parsed_response) { json_response.with_indifferent_access } let(:note_json) { parsed_response[:notes].first } before do @@ -43,7 +43,7 @@ describe Projects::NotesController do request.headers['X-Last-Fetched-At'] = last_fetched_at expect(NotesFinder).to receive(:new) - .with(anything, anything, hash_including(last_fetched_at: last_fetched_at)) + .with(anything, hash_including(last_fetched_at: last_fetched_at)) .and_call_original get :index, params: request_params @@ -543,23 +543,32 @@ describe Projects::NotesController do project.add_developer(user) end + subject { post(:toggle_award_emoji, params: request_params.merge(name: emoji_name)) } + let(:emoji_name) { 'thumbsup' } + it "toggles the award emoji" do expect do - post(:toggle_award_emoji, params: request_params.merge(name: "thumbsup")) + subject end.to change { note.award_emoji.count }.by(1) expect(response).to have_gitlab_http_status(200) end it "removes the already awarded emoji" do - post(:toggle_award_emoji, params: request_params.merge(name: "thumbsup")) + create(:award_emoji, awardable: note, name: emoji_name, user: user) - expect do - post(:toggle_award_emoji, params: request_params.merge(name: "thumbsup")) - end.to change { AwardEmoji.count }.by(-1) + expect { subject }.to change { AwardEmoji.count }.by(-1) expect(response).to have_gitlab_http_status(200) end + + it 'marks Todos on the Noteable as done' do + todo = create(:todo, target: note.noteable, project: project, user: user) + + subject + + expect(todo.reload).to be_done + end end describe "resolving and unresolving" do @@ -614,7 +623,7 @@ describe Projects::NotesController do it "returns the name of the resolving user" do post :resolve, params: request_params.merge(html: true) - expect(JSON.parse(response.body)["resolved_by"]).to eq(user.name) + expect(json_response["resolved_by"]).to eq(user.name) end it "returns status 200" do diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 9a50ea79f5e..212d8b15252 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -177,18 +177,22 @@ describe Projects::PipelinesController do end it 'does not perform N + 1 queries' do + # Set up all required variables + get_pipeline_json + control_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count - create_build('test', 1, 'rspec 1') - create_build('test', 1, 'spinach 0') - create_build('test', 1, 'spinach 1') - create_build('test', 1, 'audit') - create_build('post deploy', 3, 'pages 1') - create_build('post deploy', 3, 'pages 2') + first_build = pipeline.builds.first + first_build.tag_list << [:hello, :world] + create(:deployment, deployable: first_build) + + second_build = pipeline.builds.second + second_build.tag_list << [:docker, :ruby] + create(:deployment, deployable: second_build) new_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count - expect(new_count).to be_within(12).of(control_count) + expect(new_count).to be_within(1).of(control_count) end end @@ -393,4 +397,69 @@ describe Projects::PipelinesController do end end end + + describe 'GET latest' do + let(:branch_main) { project.repository.branches[0] } + let(:branch_secondary) { project.repository.branches[1] } + + let!(:pipeline_master) do + create(:ci_pipeline, + ref: branch_main.name, + sha: branch_main.target, + project: project) + end + + let!(:pipeline_secondary) do + create(:ci_pipeline, + ref: branch_secondary.name, + sha: branch_secondary.target, + project: project) + end + + before do + project.change_head(branch_main.name) + project.reload_default_branch + end + + context 'no ref provided' do + it 'shows latest pipeline for the default project branch' do + get :show, params: { namespace_id: project.namespace, project_id: project, latest: true, ref: nil } + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:pipeline)).to have_attributes(id: pipeline_master.id) + end + end + + context 'ref provided' do + before do + create(:ci_pipeline, ref: 'master', project: project) + end + + it 'shows the latest pipeline for the provided ref' do + get :show, params: { namespace_id: project.namespace, project_id: project, latest: true, ref: branch_secondary.name } + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:pipeline)).to have_attributes(id: pipeline_secondary.id) + end + + context 'newer pipeline exists for older sha' do + before do + create(:ci_pipeline, ref: branch_secondary.name, sha: project.commit(branch_secondary.name).parent, project: project) + end + + it 'shows the provided ref with the last sha/pipeline combo' do + get :show, params: { namespace_id: project.namespace, project_id: project, latest: true, ref: branch_secondary.name } + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:pipeline)).to have_attributes(id: pipeline_secondary.id) + end + end + end + + it 'renders a 404 if no pipeline is found for the ref' do + get :show, params: { namespace_id: project.namespace, project_id: project, ref: 'no-branch' } + + expect(response).to have_gitlab_http_status(404) + end + end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 4141e41c7a7..5130e26c928 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -158,7 +158,7 @@ describe Projects::ProjectMembersController do id: member }, xhr: true - expect(response).to be_success + expect(response).to be_successful expect(project.members).not_to include member end end diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index 97acd47b4da..8b43d1264b2 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Projects::RawController do + include RepoHelpers + let(:project) { create(:project, :public, :repository) } describe 'GET #show' do @@ -46,5 +48,98 @@ describe Projects::RawController do let(:filename) { 'lfs_object.iso' } let(:filepath) { "be93687/files/lfs/#{filename}" } end + + context 'when the endpoint receives requests above the limit', :clean_gitlab_redis_cache do + let(:file_path) { 'master/README.md' } + + before do + stub_application_setting(raw_blob_request_limit: 5) + end + + it 'prevents from accessing the raw file' do + execute_raw_requests(requests: 6, project: project, file_path: file_path) + + expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.') + expect(response).to have_gitlab_http_status(429) + end + + it 'logs the event on auth.log' do + attributes = { + message: 'Action_Rate_Limiter_Request', + env: :raw_blob_request_limit, + remote_ip: '0.0.0.0', + request_method: 'GET', + path: "/#{project.full_path}/raw/#{file_path}" + } + + expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once + + execute_raw_requests(requests: 6, project: project, file_path: file_path) + end + + context 'when the request uses a different version of a commit' do + it 'prevents from accessing the raw file' do + # 3 times with the normal sha + commit_sha = project.repository.commit.sha + file_path = "#{commit_sha}/README.md" + + execute_raw_requests(requests: 3, project: project, file_path: file_path) + + # 3 times with the modified version + modified_sha = commit_sha.gsub(commit_sha[0..5], commit_sha[0..5].upcase) + modified_path = "#{modified_sha}/README.md" + + execute_raw_requests(requests: 3, project: project, file_path: modified_path) + + expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.') + expect(response).to have_gitlab_http_status(429) + end + end + + context 'when the throttling has been disabled' do + before do + stub_application_setting(raw_blob_request_limit: 0) + end + + it 'does not prevent from accessing the raw file' do + execute_raw_requests(requests: 10, project: project, file_path: file_path) + + expect(response).to have_gitlab_http_status(200) + end + end + + context 'with case-sensitive files' do + it 'prevents from accessing the specific file' do + create_file_in_repo(project, 'master', 'master', 'readme.md', 'Add readme.md') + create_file_in_repo(project, 'master', 'master', 'README.md', 'Add README.md') + + commit_sha = project.repository.commit.sha + file_path = "#{commit_sha}/readme.md" + + # Accessing downcase version of readme + execute_raw_requests(requests: 6, project: project, file_path: file_path) + + expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.') + expect(response).to have_gitlab_http_status(429) + + # Accessing upcase version of readme + file_path = "#{commit_sha}/README.md" + + execute_raw_requests(requests: 1, project: project, file_path: file_path) + + expect(response).to have_gitlab_http_status(200) + end + end + end + end + + def execute_raw_requests(requests:, project:, file_path:) + requests.times do + get :show, params: { + namespace_id: project.namespace, + project_id: project, + id: file_path + } + end end end diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb index 6db98f2428b..646c7a7db7c 100644 --- a/spec/controllers/projects/refs_controller_spec.rb +++ b/spec/controllers/projects/refs_controller_spec.rb @@ -49,7 +49,7 @@ describe Projects::RefsController do expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original xhr_get(:js) - expect(response).to be_success + expect(response).to be_successful end it 'renders JSON' do @@ -57,7 +57,7 @@ describe Projects::RefsController do xhr_get(:json) - expect(response).to be_success + expect(response).to be_successful expect(json_response).to be_kind_of(Array) end end diff --git a/spec/controllers/projects/registry/tags_controller_spec.rb b/spec/controllers/projects/registry/tags_controller_spec.rb index ff35139ae2e..c6e063d8229 100644 --- a/spec/controllers/projects/registry/tags_controller_spec.rb +++ b/spec/controllers/projects/registry/tags_controller_spec.rb @@ -113,4 +113,37 @@ describe Projects::Registry::TagsController do format: :json end end + + describe 'POST bulk_destroy' do + context 'when user has access to registry' do + before do + project.add_developer(user) + end + + context 'when there is matching tag present' do + before do + stub_container_registry_tags(repository: repository.path, tags: %w[rc1 test.]) + end + + it 'makes it possible to delete tags in bulk' do + allow_any_instance_of(ContainerRegistry::Tag).to receive(:delete) { |*args| ContainerRegistry::Tag.delete(*args) } + expect(ContainerRegistry::Tag).to receive(:delete).exactly(2).times + + bulk_destroy_tags(['rc1', 'test.']) + end + end + end + + private + + def bulk_destroy_tags(names) + post :bulk_destroy, params: { + namespace_id: project.namespace, + project_id: project, + repository_id: repository, + ids: names + }, + format: :json + end + end end diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index 8fca9e680dd..fcab4d73dca 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -77,6 +77,53 @@ describe Projects::RepositoriesController do expect(response).to have_gitlab_http_status(404) end end + + describe 'caching' do + it 'sets appropriate caching headers' do + get_archive + + expect(response).to have_gitlab_http_status(200) + expect(response.header['ETag']).to be_present + expect(response.header['Cache-Control']).to include('max-age=60, private') + end + + context 'when project is public' do + let(:project) { create(:project, :repository, :public) } + + it 'sets appropriate caching headers' do + get_archive + + expect(response).to have_gitlab_http_status(200) + expect(response.header['ETag']).to be_present + expect(response.header['Cache-Control']).to include('max-age=60, public') + end + end + + context 'when ref is a commit SHA' do + it 'max-age is set to 3600 in Cache-Control header' do + get_archive('ddd0f15ae83993f5cb66a927a28673882e99100b') + + expect(response).to have_gitlab_http_status(200) + expect(response.header['Cache-Control']).to include('max-age=3600') + end + end + + context 'when If-None-Modified header is set' do + it 'returns a 304 status' do + # Get the archive cached first + get_archive + + request.headers['If-None-Match'] = response.headers['ETag'] + get_archive + + expect(response).to have_gitlab_http_status(304) + end + end + + def get_archive(id = 'feature') + get :archive, params: { namespace_id: project.namespace, project_id: project, id: id }, format: 'zip' + end + end end end end diff --git a/spec/controllers/projects/serverless/functions_controller_spec.rb b/spec/controllers/projects/serverless/functions_controller_spec.rb index 18c594acae0..9f1ef3a4be8 100644 --- a/spec/controllers/projects/serverless/functions_controller_spec.rb +++ b/spec/controllers/projects/serverless/functions_controller_spec.rb @@ -10,12 +10,16 @@ describe Projects::Serverless::FunctionsController do let(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:service) { cluster.platform_kubernetes } let(:project) { cluster.project } + let(:environment) { create(:environment, project: project) } + let!(:deployment) { create(:deployment, :success, environment: environment, cluster: cluster) } + let(:knative_services_finder) { environment.knative_services_finder } let(:namespace) do create(:cluster_kubernetes_namespace, cluster: cluster, cluster_project: cluster.cluster_project, - project: cluster.cluster_project.project) + project: cluster.cluster_project.project, + environment: environment) end before do @@ -47,12 +51,11 @@ describe Projects::Serverless::FunctionsController do end context 'when cache is ready' do - let(:knative_services_finder) { project.clusters.first.knative_services_finder(project) } let(:knative_state) { true } before do - allow_any_instance_of(Clusters::Cluster) - .to receive(:knative_services_finder) + allow(Clusters::KnativeServicesFinder) + .to receive(:new) .and_return(knative_services_finder) synchronous_reactive_cache(knative_services_finder) stub_kubeclient_service_pods( @@ -107,12 +110,12 @@ describe Projects::Serverless::FunctionsController do context 'valid data', :use_clean_rails_memory_store_caching do before do stub_kubeclient_service_pods - stub_reactive_cache(cluster.knative_services_finder(project), + stub_reactive_cache(knative_services_finder, { services: kube_knative_services_body(namespace: namespace.namespace, name: cluster.project.name)["items"], pods: kube_knative_pods_body(cluster.project.name, namespace.namespace)["items"] }, - *cluster.knative_services_finder(project).cache_args) + *knative_services_finder.cache_args) end it 'has a valid function name' do @@ -140,12 +143,12 @@ describe Projects::Serverless::FunctionsController do describe 'GET #index with data', :use_clean_rails_memory_store_caching do before do stub_kubeclient_service_pods - stub_reactive_cache(cluster.knative_services_finder(project), + stub_reactive_cache(knative_services_finder, { services: kube_knative_services_body(namespace: namespace.namespace, name: cluster.project.name)["items"], pods: kube_knative_pods_body(cluster.project.name, namespace.namespace)["items"] }, - *cluster.knative_services_finder(project).cache_args) + *knative_services_finder.cache_args) end it 'has data' do diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 68eabce8513..563b61962cf 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -159,7 +159,7 @@ describe Projects::ServicesController do context 'with approved services' do it 'renders edit page' do - expect(response).to be_success + expect(response).to be_successful end end end diff --git a/spec/controllers/projects/starrers_controller_spec.rb b/spec/controllers/projects/starrers_controller_spec.rb new file mode 100644 index 00000000000..5774ff7c576 --- /dev/null +++ b/spec/controllers/projects/starrers_controller_spec.rb @@ -0,0 +1,196 @@ +# frozen_string_literal: true + +require 'spec_helper' + +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(:admin) { create(:user, admin: true) } + let(:project) { create(:project, :public) } + + before do + user_1.toggle_star(project) + user_2.toggle_star(project) + private_user.toggle_star(project) + end + + describe 'GET index' do + def get_starrers(search: nil) + get :index, params: { namespace_id: project.namespace, project_id: project, search: search } + end + + def user_ids + assigns[:starrers].map { |s| s['user_id'] } + end + + shared_examples 'starrers counts' do + it 'starrers counts are correct' do + expect(assigns[:total_count]).to eq(3) + expect(assigns[:public_count]).to eq(2) + expect(assigns[:private_count]).to eq(1) + end + end + + context 'N+1 queries' do + render_views + + it 'avoids N+1s loading users', :request_store do + get_starrers + + control_count = ActiveRecord::QueryRecorder.new { get_starrers }.count + + create_list(:user, 5).each { |user| user.toggle_star(project) } + + expect { get_starrers }.not_to exceed_query_limit(control_count) + end + end + + context 'when project is public' do + before do + project.update_attribute(:visibility_level, Project::PUBLIC) + end + + context 'when no user is logged in' do + context 'with no searching' do + before do + get_starrers + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_1.id, user_2.id) + end + + include_examples 'starrers counts' + end + + context 'when searching by user' do + before do + get_starrers(search: 'Michael') + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_2.id) + end + + include_examples 'starrers counts' + end + end + + context 'when public user is logged in' do + before do + sign_in(user_1) + end + + context 'with no searching' do + before do + get_starrers + end + + it 'their star is also visible' do + expect(user_ids).to contain_exactly(user_1.id, user_2.id) + end + + include_examples 'starrers counts' + end + + context 'when searching by user' do + before do + get_starrers(search: 'Michael') + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_2.id) + end + + include_examples 'starrers counts' + end + end + + context 'when private user is logged in' do + before do + sign_in(private_user) + end + + context 'with no searching' do + before do + get_starrers + end + + it 'their star is also visible' do + expect(user_ids).to contain_exactly(user_1.id, user_2.id, private_user.id) + end + + include_examples 'starrers counts' + end + + context 'when searching by user' do + before do + get_starrers(search: 'Michael') + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_2.id, private_user.id) + end + + include_examples 'starrers counts' + end + end + + context 'when admin is logged in' do + before do + sign_in(admin) + end + + context 'with no searching' do + before do + get_starrers + end + + it 'all users are visible' do + expect(user_ids).to include(user_1.id, user_2.id, private_user.id) + end + + include_examples 'starrers counts' + end + + context 'when searching by user' do + before do + get_starrers(search: 'Michael') + end + + it 'public and private starrers are visible' do + expect(user_ids).to contain_exactly(user_2.id, private_user.id) + end + + include_examples 'starrers counts' + end + end + end + + context 'when project is private' do + before do + project.update(visibility_level: Project::PRIVATE) + end + + it 'starrers are not visible for non logged in users' do + get_starrers + + expect(assigns[:starrers]).to be_blank + end + + context 'when user is logged in' do + before do + sign_in(project.creator) + get_starrers + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_1.id, user_2.id) + end + + include_examples 'starrers counts' + end + end + end +end diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb index 9e7d34b10c0..d5ef2b0e114 100644 --- a/spec/controllers/projects/templates_controller_spec.rb +++ b/spec/controllers/projects/templates_controller_spec.rb @@ -7,7 +7,6 @@ describe Projects::TemplatesController do let(:user) { create(:user) } let(:file_path_1) { '.gitlab/issue_templates/issue_template.md' } let(:file_path_2) { '.gitlab/merge_request_templates/merge_request_template.md' } - let(:body) { JSON.parse(response.body) } let!(:file_1) { project.repository.create_file(user, file_path_1, 'issue content', message: 'message', branch_name: 'master') } let!(:file_2) { project.repository.create_file(user, file_path_2, 'merge request content', message: 'message', branch_name: 'master') } @@ -17,8 +16,8 @@ describe Projects::TemplatesController do get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) expect(response.status).to eq(200) - expect(body['name']).to eq('issue_template') - expect(body['content']).to eq('issue content') + expect(json_response['name']).to eq('issue_template') + expect(json_response['content']).to eq('issue content') end end @@ -27,8 +26,8 @@ describe Projects::TemplatesController do get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) expect(response.status).to eq(200) - expect(body['name']).to eq('merge_request_template') - expect(body['content']).to eq('merge request content') + expect(json_response['name']).to eq('merge_request_template') + expect(json_response['content']).to eq('merge request content') end end diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb index 776c1270977..661ed9840b1 100644 --- a/spec/controllers/projects/uploads_controller_spec.rb +++ b/spec/controllers/projects/uploads_controller_spec.rb @@ -10,6 +10,11 @@ describe Projects::UploadsController do { namespace_id: model.namespace.to_param, project_id: model } end + let(:other_model) { create(:project, :public) } + let(:other_params) do + { namespace_id: other_model.namespace.to_param, project_id: other_model } + end + it_behaves_like 'handle uploads' context 'when the URL the old style, without /-/system' do diff --git a/spec/controllers/projects/variables_controller_spec.rb b/spec/controllers/projects/variables_controller_spec.rb index a2a09e2580f..21e106660d0 100644 --- a/spec/controllers/projects/variables_controller_spec.rb +++ b/spec/controllers/projects/variables_controller_spec.rb @@ -36,5 +36,70 @@ describe Projects::VariablesController do end include_examples 'PATCH #update updates variables' + + context 'with environment scope' do + let!(:variable) { create(:ci_variable, project: project, environment_scope: 'custom_scope') } + + let(:variable_attributes) do + { id: variable.id, + key: variable.key, + secret_value: variable.value, + protected: variable.protected?.to_s, + environment_scope: variable.environment_scope } + end + let(:new_variable_attributes) do + { key: 'new_key', + secret_value: 'dummy_value', + protected: 'false', + environment_scope: 'new_scope' } + end + + context 'with same key and different environment scope' do + let(:variables_attributes) do + [ + variable_attributes, + new_variable_attributes.merge(key: variable.key) + ] + end + + it 'does not update the existing variable' do + expect { subject }.not_to change { variable.reload.value } + end + + it 'creates the new variable' do + expect { subject }.to change { owner.variables.count }.by(1) + end + + it 'returns a successful response including all variables' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('variables') + end + end + + context 'with same key and same environment scope' do + let(:variables_attributes) do + [ + variable_attributes, + new_variable_attributes.merge(key: variable.key, environment_scope: variable.environment_scope) + ] + end + + it 'does not update the existing variable' do + expect { subject }.not_to change { variable.reload.value } + end + + it 'does not create the new variable' do + expect { subject }.not_to change { owner.variables.count } + end + + it 'returns a bad request response' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end end end diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index f2e0b5e5c1d..6fea6bca4f2 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' describe Projects::WikisController do - let(:project) { create(:project, :public, :repository) } - let(:user) { project.owner } + set(:project) { create(:project, :public, :repository) } + set(:user) { project.owner } let(:project_wiki) { ProjectWiki.new(project, user) } let(:wiki) { project_wiki.wiki } - let(:wiki_title) { 'page-title-test' } + let(:wiki_title) { 'page title test' } before do create_page(wiki_title, 'hello world') @@ -19,6 +19,21 @@ describe Projects::WikisController do destroy_page(wiki_title) end + describe 'GET #new' do + subject { get :new, params: { namespace_id: project.namespace, project_id: project } } + + it 'redirects to #show and appends a `random_title` param' do + subject + + expect(response).to have_http_status(302) + expect(Rails.application.routes.recognize_path(response.redirect_url)).to include( + controller: 'projects/wikis', + action: 'show' + ) + expect(response.redirect_url).to match(/\?random_title=true\Z/) + end + end + describe 'GET #pages' do subject { get :pages, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } } @@ -31,43 +46,106 @@ describe Projects::WikisController do end end + describe 'GET #history' do + before do + allow(controller) + .to receive(:can?) + .with(any_args) + .and_call_original + + # The :create_wiki permission is irrelevant to reading history. + expect(controller) + .not_to receive(:can?) + .with(anything, :create_wiki, any_args) + + allow(controller) + .to receive(:can?) + .with(anything, :read_wiki, any_args) + .and_return(allow_read_wiki) + end + + shared_examples 'fetching history' do |expected_status| + before do + get :history, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } + end + + it "returns status #{expected_status}" do + expect(response).to have_http_status(expected_status) + end + end + + it_behaves_like 'fetching history', :ok do + let(:allow_read_wiki) { true } + + it 'assigns @page_versions' do + expect(assigns(:page_versions)).to be_present + end + end + + it_behaves_like 'fetching history', :not_found do + let(:allow_read_wiki) { false } + end + end + describe 'GET #show' do render_views - subject { get :show, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } } + let(:random_title) { nil } - it 'limits the retrieved pages for the sidebar' do - expect(controller).to receive(:load_wiki).and_return(project_wiki) + subject { get :show, params: { namespace_id: project.namespace, project_id: project, id: id, random_title: random_title } } - # empty? call - expect(project_wiki).to receive(:list_pages).with(limit: 1).and_call_original - # Sidebar entries - expect(project_wiki).to receive(:list_pages).with(limit: 15).and_call_original + context 'when page exists' do + let(:id) { wiki_title } - subject + it 'limits the retrieved pages for the sidebar' do + expect(controller).to receive(:load_wiki).and_return(project_wiki) + expect(project_wiki).to receive(:list_pages).with(limit: 15).and_call_original + + subject - expect(response).to have_http_status(:ok) - expect(response.body).to include(wiki_title) + expect(response).to have_http_status(:ok) + expect(assigns(:page).title).to eq(wiki_title) + end + + context 'when page content encoding is invalid' do + it 'sets flash error' do + allow(controller).to receive(:valid_encoding?).and_return(false) + + subject + + expect(response).to have_http_status(:ok) + expect(flash[:notice]).to eq('The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.') + end + end end - context 'when page content encoding is invalid' do - it 'sets flash error' do - allow(controller).to receive(:valid_encoding?).and_return(false) + context 'when the page does not exist' do + let(:id) { 'does not exist' } + before do subject + end - expect(response).to have_http_status(:ok) - expect(flash[:notice]).to eq 'The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.' + it 'builds a new wiki page with the id as the title' do + expect(assigns(:page).title).to eq(id) + end + + context 'when a random_title param is present' do + let(:random_title) { true } + + it 'builds a new wiki page with no title' do + expect(assigns(:page).title).to be_empty + end end end context 'when page is a file' do include WikiHelpers - let(:path) { upload_file_to_wiki(project, user, file_name) } + let(:id) { upload_file_to_wiki(project, user, file_name) } before do - get :show, params: { namespace_id: project.namespace, project_id: project, id: path } + subject end context 'when file is an image' do @@ -103,7 +181,7 @@ describe Projects::WikisController do it 'renders json in a correct format' do post :preview_markdown, params: { namespace_id: project.namespace, project_id: project, id: 'page/path', text: '*Markdown* text' } - expect(JSON.parse(response.body).keys).to match_array(%w(body references)) + expect(json_response.keys).to match_array(%w(body references)) end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 4e1cac67d23..c732caa6160 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -318,6 +318,102 @@ describe ProjectsController do end end + describe 'POST #archive' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + + before do + sign_in(user) + end + + context 'for a user with the ability to archive a project' do + before do + group.add_owner(user) + + post :archive, params: { + namespace_id: project.namespace.path, + id: project.path + } + end + + it 'archives the project' do + expect(project.reload.archived?).to be_truthy + end + + it 'redirects to projects path' do + expect(response).to have_gitlab_http_status(302) + expect(response).to redirect_to(project_path(project)) + end + end + + context 'for a user that does not have the ability to archive a project' do + before do + project.add_maintainer(user) + + post :archive, params: { + namespace_id: project.namespace.path, + id: project.path + } + end + + it 'does not archive the project' do + expect(project.reload.archived?).to be_falsey + end + + it 'returns 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + + describe 'POST #unarchive' do + let(:group) { create(:group) } + let(:project) { create(:project, :archived, group: group) } + + before do + sign_in(user) + end + + context 'for a user with the ability to unarchive a project' do + before do + group.add_owner(user) + + post :unarchive, params: { + namespace_id: project.namespace.path, + id: project.path + } + end + + it 'unarchives the project' do + expect(project.reload.archived?).to be_falsey + end + + it 'redirects to projects path' do + expect(response).to have_gitlab_http_status(302) + expect(response).to redirect_to(project_path(project)) + end + end + + context 'for a user that does not have the ability to unarchive a project' do + before do + project.add_maintainer(user) + + post :unarchive, params: { + namespace_id: project.namespace.path, + id: project.path + } + end + + it 'does not unarchive the project' do + expect(project.reload.archived?).to be_truthy + end + + it 'returns 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + describe '#housekeeping' do let(:group) { create(:group) } let(:project) { create(:project, group: group) } @@ -740,20 +836,18 @@ describe ProjectsController do it 'gets a list of branches and tags' do get :refs, params: { namespace_id: project.namespace, id: project, sort: 'updated_desc' } - parsed_body = JSON.parse(response.body) - expect(parsed_body['Branches']).to include('master') - expect(parsed_body['Tags'].first).to eq('v1.1.0') - expect(parsed_body['Tags'].last).to eq('v1.0.0') - expect(parsed_body['Commits']).to be_nil + expect(json_response['Branches']).to include('master') + expect(json_response['Tags'].first).to eq('v1.1.0') + expect(json_response['Tags'].last).to eq('v1.0.0') + expect(json_response['Commits']).to be_nil end it "gets a list of branches, tags and commits" do get :refs, params: { namespace_id: project.namespace, id: project, ref: "123456" } - parsed_body = JSON.parse(response.body) - expect(parsed_body["Branches"]).to include("master") - expect(parsed_body["Tags"]).to include("v1.0.0") - expect(parsed_body["Commits"]).to include("123456") + expect(json_response["Branches"]).to include("master") + expect(json_response["Tags"]).to include("v1.0.0") + expect(json_response["Commits"]).to include("123456") end context "when preferred language is Japanese" do @@ -765,10 +859,9 @@ describe ProjectsController do it "gets a list of branches, tags and commits" do get :refs, params: { namespace_id: project.namespace, id: project, ref: "123456" } - parsed_body = JSON.parse(response.body) - expect(parsed_body["Branches"]).to include("master") - expect(parsed_body["Tags"]).to include("v1.0.0") - expect(parsed_body["Commits"]).to include("123456") + expect(json_response["Branches"]).to include("master") + expect(json_response["Tags"]).to include("v1.0.0") + expect(json_response["Commits"]).to include("123456") end end @@ -797,7 +890,7 @@ describe ProjectsController do it 'renders json in a correct format' do post :preview_markdown, params: { namespace_id: public_project.namespace, id: public_project, text: '*Markdown* text' } - expect(JSON.parse(response.body).keys).to match_array(%w(body references)) + expect(json_response.keys).to match_array(%w(body references)) end context 'when not authorized' do @@ -821,8 +914,6 @@ describe ProjectsController do text: issue.to_reference } - json_response = JSON.parse(response.body) - expect(json_response['body']).to match(/\##{issue.iid} \(closed\)/) end @@ -833,8 +924,6 @@ describe ProjectsController do text: merge_request.to_reference } - json_response = JSON.parse(response.body) - expect(json_response['body']).to match(/\!#{merge_request.iid} \(closed\)/) end end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index faf3c990cb2..35487682462 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' describe RegistrationsController do include TermsHelper + before do + stub_feature_flags(invisible_captcha: false) + end + describe '#create' do let(:base_user_params) { { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } let(:user_params) { { user: base_user_params } } @@ -26,13 +30,36 @@ describe RegistrationsController do end context 'when send_user_confirmation_email is true' do - it 'does not authenticate user and sends confirmation email' do + before do stub_application_setting(send_user_confirmation_email: true) + end + + context 'when soft email confirmation is not enabled' do + before do + stub_feature_flags(soft_email_confirmation: false) + allow(User).to receive(:allow_unconfirmed_access_for).and_return 0 + end - post(:create, params: user_params) + it 'does not authenticate the user and sends a confirmation email' do + post(:create, params: user_params) - expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) - expect(subject.current_user).to be_nil + expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) + expect(subject.current_user).to be_nil + end + end + + context 'when soft email confirmation is enabled' do + before do + stub_feature_flags(soft_email_confirmation: true) + allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days + end + + it 'authenticates the user and sends a confirmation email' do + post(:create, params: user_params) + + expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) + expect(response).to redirect_to(dashboard_projects_path) + end end end @@ -88,6 +115,88 @@ describe RegistrationsController do end end + context 'when invisible captcha is enabled' do + before do + stub_feature_flags(invisible_captcha: true) + InvisibleCaptcha.timestamp_threshold = treshold + end + + let(:treshold) { 4 } + let(:session_params) { { invisible_captcha_timestamp: form_rendered_time.iso8601 } } + let(:form_rendered_time) { Time.current } + let(:submit_time) { form_rendered_time + treshold } + let(:auth_log_attributes) do + { + message: auth_log_message, + env: :invisible_captcha_signup_bot_detected, + remote_ip: '0.0.0.0', + request_method: 'POST', + path: '/users' + } + end + + describe 'the honeypot has not been filled and the signup form has not been submitted too quickly' do + it 'creates an account' do + travel_to(submit_time) do + expect { post(:create, params: user_params, session: session_params) }.to change(User, :count).by(1) + end + end + end + + describe 'honeypot spam detection' do + let(:user_params) { super().merge(firstname: 'Roy', lastname: 'Batty') } + let(:auth_log_message) { 'Invisible_Captcha_Honeypot_Request' } + + it 'logs the request, refuses to create an account and renders an empty body' do + travel_to(submit_time) do + expect(Gitlab::Metrics).to receive(:counter) + .with(:bot_blocked_by_invisible_captcha_honeypot, 'Counter of blocked sign up attempts with filled honeypot') + .and_call_original + expect(Gitlab::AuthLogger).to receive(:error).with(auth_log_attributes).once + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to have_gitlab_http_status(200) + expect(response.body).to be_empty + end + end + end + + describe 'timestamp spam detection' do + let(:auth_log_message) { 'Invisible_Captcha_Timestamp_Request' } + + context 'the sign up form has been submitted without the invisible_captcha_timestamp parameter' do + let(:session_params) { nil } + + it 'logs the request, refuses to create an account and displays a flash alert' do + travel_to(submit_time) do + expect(Gitlab::Metrics).to receive(:counter) + .with(:bot_blocked_by_invisible_captcha_timestamp, 'Counter of blocked sign up attempts with invalid timestamp') + .and_call_original + expect(Gitlab::AuthLogger).to receive(:error).with(auth_log_attributes).once + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + end + end + end + + context 'the sign up form has been submitted too quickly' do + let(:submit_time) { form_rendered_time } + + it 'logs the request, refuses to create an account and displays a flash alert' do + travel_to(submit_time) do + expect(Gitlab::Metrics).to receive(:counter) + .with(:bot_blocked_by_invisible_captcha_timestamp, 'Counter of blocked sign up attempts with invalid timestamp') + .and_call_original + expect(Gitlab::AuthLogger).to receive(:error).with(auth_log_attributes).once + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + end + end + end + end + end + context 'when terms are enforced' do before do enforce_terms diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 5a5c0a1f6ac..3e0d53a6573 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -11,151 +11,173 @@ describe SearchController do sign_in(user) end - context 'uses the right partials depending on scope' do - using RSpec::Parameterized::TableSyntax - render_views - - set(:project) { create(:project, :public, :repository, :wiki_repo) } - + shared_examples_for 'when the user cannot read cross project' do |action, params| before do - expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?) + .with(user, :read_cross_project, :global) { false } end - subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) } + it 'blocks access without a project_id' do + get action, params: params - where(:partial, :scope) do - '_blob' | :blobs - '_wiki_blob' | :wiki_blobs - '_commit' | :commits + expect(response).to have_gitlab_http_status(403) end - with_them do - it do - project_wiki = create(:project_wiki, project: project, user: user) - create(:wiki_page, wiki: project_wiki, attrs: { title: 'merge', content: 'merge' }) + it 'allows access with a project_id' do + get action, params: params.merge(project_id: create(:project, :public).id) - expect(subject).to render_template("search/results/#{partial}") - end + expect(response).to have_gitlab_http_status(200) end end - context 'global search' do - render_views - - it 'omits pipeline status from load' do - project = create(:project, :public) - expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) - - get :show, params: { scope: 'projects', search: project.name } + shared_examples_for 'with external authorization service enabled' do |action, params| + let(:project) { create(:project, namespace: user.namespace) } + let(:note) { create(:note_on_issue, project: project) } - expect(assigns[:search_objects].first).to eq project + before do + enable_external_authorization_service_check end - end - - it 'finds issue comments' do - project = create(:project, :public) - note = create(:note_on_issue, project: project) - get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + it 'renders a 403 when no project is given' do + get action, params: params - expect(assigns[:search_objects].first).to eq note - end - - context 'when the user cannot read cross project' do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?) - .with(user, :read_cross_project, :global) { false } + expect(response).to have_gitlab_http_status(403) end - it 'still allows accessing the search page' do - get :show + it 'renders a 200 when a project was set' do + get action, params: params.merge(project_id: project.id) expect(response).to have_gitlab_http_status(200) end + end - it 'still blocks searches without a project_id' do - get :show, params: { search: 'hello' } + describe 'GET #show' do + it_behaves_like 'when the user cannot read cross project', :show, { search: 'hello' } do + it 'still allows accessing the search page' do + get :show - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(200) + end end - it 'allows searches with a project_id' do - get :show, params: { search: 'hello', project_id: create(:project, :public).id } + it_behaves_like 'with external authorization service enabled', :show, { search: 'hello' } - expect(response).to have_gitlab_http_status(200) - end - end + context 'uses the right partials depending on scope' do + using RSpec::Parameterized::TableSyntax + render_views + + set(:project) { create(:project, :public, :repository, :wiki_repo) } - context 'on restricted projects' do - context 'when signed out' do before do - sign_out(user) + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original end - it "doesn't expose comments on issues" do - project = create(:project, :public, :issues_private) - note = create(:note_on_issue, project: project) + subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) } - get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + where(:partial, :scope) do + '_blob' | :blobs + '_wiki_blob' | :wiki_blobs + '_commit' | :commits + end - expect(assigns[:search_objects].count).to eq(0) + with_them do + it do + project_wiki = create(:project_wiki, project: project, user: user) + create(:wiki_page, wiki: project_wiki, attrs: { title: 'merge', content: 'merge' }) + + expect(subject).to render_template("search/results/#{partial}") + end end end - it "doesn't expose comments on merge_requests" do - project = create(:project, :public, :merge_requests_private) - note = create(:note_on_merge_request, project: project) + context 'global search' do + render_views - get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + it 'omits pipeline status from load' do + project = create(:project, :public) + expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) + + get :show, params: { scope: 'projects', search: project.name } - expect(assigns[:search_objects].count).to eq(0) + expect(assigns[:search_objects].first).to eq project + end end - it "doesn't expose comments on snippets" do - project = create(:project, :public, :snippets_private) - note = create(:note_on_project_snippet, project: project) + it 'finds issue comments' do + project = create(:project, :public) + note = create(:note_on_issue, project: project) get :show, params: { project_id: project.id, scope: 'notes', search: note.note } - expect(assigns[:search_objects].count).to eq(0) + expect(assigns[:search_objects].first).to eq note end - end - context 'with external authorization service enabled' do - let(:project) { create(:project, namespace: user.namespace) } - let(:note) { create(:note_on_issue, project: project) } + context 'on restricted projects' do + context 'when signed out' do + before do + sign_out(user) + end - before do - enable_external_authorization_service_check - end + it "doesn't expose comments on issues" do + project = create(:project, :public, :issues_private) + note = create(:note_on_issue, project: project) - describe 'GET #show' do - it 'renders a 403 when no project is given' do - get :show, params: { scope: 'notes', search: note.note } + get :show, params: { project_id: project.id, scope: 'notes', search: note.note } - expect(response).to have_gitlab_http_status(403) + expect(assigns[:search_objects].count).to eq(0) + end end - it 'renders a 200 when a project was set' do + it "doesn't expose comments on merge_requests" do + project = create(:project, :public, :merge_requests_private) + note = create(:note_on_merge_request, project: project) + get :show, params: { project_id: project.id, scope: 'notes', search: note.note } - expect(response).to have_gitlab_http_status(200) + expect(assigns[:search_objects].count).to eq(0) end - end - describe 'GET #autocomplete' do - it 'renders a 403 when no project is given' do - get :autocomplete, params: { term: 'hello' } + it "doesn't expose comments on snippets" do + project = create(:project, :public, :snippets_private) + note = create(:note_on_project_snippet, project: project) - expect(response).to have_gitlab_http_status(403) + get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + + expect(assigns[:search_objects].count).to eq(0) end + end + end - it 'renders a 200 when a project was set' do - get :autocomplete, params: { project_id: project.id, term: 'hello' } + describe 'GET #count' do + it_behaves_like 'when the user cannot read cross project', :count, { search: 'hello', scope: 'projects' } + it_behaves_like 'with external authorization service enabled', :count, { search: 'hello', scope: 'projects' } - expect(response).to have_gitlab_http_status(200) - end + it 'returns the result count for the given term and scope' do + create(:project, :public, name: 'hello world') + create(:project, :public, name: 'foo bar') + + get :count, params: { search: 'hello', scope: 'projects' } + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to eq({ 'count' => '1' }) + end + + it 'raises an error if search term is missing' do + expect do + get :count, params: { scope: 'projects' } + end.to raise_error(ActionController::ParameterMissing) end + + it 'raises an error if search scope is missing' do + expect do + get :count, params: { search: 'hello' } + end.to raise_error(ActionController::ParameterMissing) + end + end + + describe 'GET #autocomplete' do + it_behaves_like 'when the user cannot read cross project', :autocomplete, { term: 'hello' } + it_behaves_like 'with external authorization service enabled', :autocomplete, { term: 'hello' } end end diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 586d59c2d09..fd4b95ce226 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -26,7 +26,7 @@ describe Snippets::NotesController do end it "returns not empty array of notes" do - expect(JSON.parse(response.body)["notes"].empty?).to be_falsey + expect(json_response["notes"].empty?).to be_falsey end end @@ -97,7 +97,7 @@ describe Snippets::NotesController do it "returns 1 note" do get :index, params: { snippet_id: private_snippet } - expect(JSON.parse(response.body)['notes'].count).to eq(1) + expect(json_response['notes'].count).to eq(1) end end end @@ -114,7 +114,7 @@ describe Snippets::NotesController do it "does not return any note" do get :index, params: { snippet_id: public_snippet } - expect(JSON.parse(response.body)['notes'].count).to eq(0) + expect(json_response['notes'].count).to eq(0) end end end @@ -288,11 +288,13 @@ describe Snippets::NotesController do describe 'POST toggle_award_emoji' do let(:note) { create(:note_on_personal_snippet, noteable: public_snippet) } + let(:emoji_name) { 'thumbsup'} + before do sign_in(user) end - subject { post(:toggle_award_emoji, params: { snippet_id: public_snippet, id: note.id, name: "thumbsup" }) } + subject { post(:toggle_award_emoji, params: { snippet_id: public_snippet, id: note.id, name: emoji_name }) } it "toggles the award emoji" do expect { subject }.to change { note.award_emoji.count }.by(1) @@ -301,7 +303,7 @@ describe Snippets::NotesController do end it "removes the already awarded emoji when it exists" do - note.toggle_award_emoji('thumbsup', user) # create award emoji before + create(:award_emoji, awardable: note, name: emoji_name, user: user) expect { subject }.to change { AwardEmoji.count }.by(-1) diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 3aba02bf3ff..b0092bc8994 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -622,7 +622,7 @@ describe SnippetsController do post :preview_markdown, params: { id: snippet, text: '*Markdown* text' } - expect(JSON.parse(response.body).keys).to match_array(%w(body references)) + expect(json_response.keys).to match_array(%w(body references)) end end end diff --git a/spec/controllers/user_callouts_controller_spec.rb b/spec/controllers/user_callouts_controller_spec.rb index babc93a83e5..07eaff2da09 100644 --- a/spec/controllers/user_callouts_controller_spec.rb +++ b/spec/controllers/user_callouts_controller_spec.rb @@ -13,7 +13,7 @@ describe UserCalloutsController do subject { post :create, params: { feature_name: feature_name }, format: :json } context 'with valid feature name' do - let(:feature_name) { UserCallout.feature_names.keys.first } + let(:feature_name) { UserCallout.feature_names.first.first } context 'when callout entry does not exist' do it 'creates a callout entry with dismissed state' do @@ -28,7 +28,7 @@ describe UserCalloutsController do end context 'when callout entry already exists' do - let!(:callout) { create(:user_callout, feature_name: UserCallout.feature_names.keys.first, user: user) } + let!(:callout) { create(:user_callout, feature_name: UserCallout.feature_names.first.first, user: user) } it 'returns success' do subject diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index c3d6ea9cbcd..5566df0c216 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -19,7 +19,7 @@ describe UsersController do it 'renders the show template' do get :show, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful expect(response).to render_template('show') end end @@ -291,7 +291,7 @@ describe UsersController do it 'response with snippets json data' do get :snippets, params: { username: user.username }, format: :json expect(response).to have_gitlab_http_status(200) - expect(JSON.parse(response.body)).to have_key('html') + expect(json_response).to have_key('html') end end @@ -362,7 +362,7 @@ describe UsersController do it 'responds with success' do get :show, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful end end @@ -418,7 +418,7 @@ describe UsersController do it 'responds with success' do get :projects, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful end end |