diff options
Diffstat (limited to 'spec')
102 files changed, 3461 insertions, 611 deletions
diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 73fff6eb5ca..b7257fac608 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -109,15 +109,17 @@ describe AutocompleteController do end context 'limited users per page' do - let(:per_page) { 2 } - before do + 25.times do + create(:user) + end + sign_in(user) - get(:users, per_page: per_page) + get(:users) end it { expect(json_response).to be_kind_of(Array) } - it { expect(json_response.size).to eq(per_page) } + it { expect(json_response.size).to eq(20) } end context 'unauthenticated user' do diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index 79bbc29e80d..4770e187db6 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -86,6 +86,7 @@ describe Boards::IssuesController do context 'with unauthorized user' do before do + allow(Ability).to receive(:allowed?).and_call_original allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) allow(Ability).to receive(:allowed?).with(user, :read_issue, project).and_return(false) end diff --git a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb new file mode 100644 index 00000000000..27f558e1b5d --- /dev/null +++ b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb @@ -0,0 +1,146 @@ +require 'spec_helper' + +describe ControllerWithCrossProjectAccessCheck do + let(:user) { create(:user) } + + before do + sign_in user + end + + render_views + + context 'When reading cross project is not allowed' do + before do + allow(Ability).to receive(:allowed).and_call_original + allow(Ability).to receive(:allowed?) + .with(user, :read_cross_project, :global) + .and_return(false) + end + + describe '#requires_cross_project_access' do + controller(ApplicationController) do + # `described_class` is not available in this context + include ControllerWithCrossProjectAccessCheck # rubocop:disable RSpec/DescribedClass + + requires_cross_project_access :index, show: false, + unless: -> { unless_condition }, + if: -> { if_condition } + + def index + render nothing: true + end + + def show + render nothing: true + end + + def unless_condition + false + end + + def if_condition + true + end + end + + it 'renders a 404 with trying to access a cross project page' do + message = "This page is unavailable because you are not allowed to read "\ + "information across multiple projects." + + get :index + + expect(response).to have_gitlab_http_status(404) + expect(response.body).to match(/#{message}/) + end + + it 'is skipped when the `if` condition returns false' do + expect(controller).to receive(:if_condition).and_return(false) + + get :index + + expect(response).to have_gitlab_http_status(200) + end + + it 'is skipped when the `unless` condition returns true' do + expect(controller).to receive(:unless_condition).and_return(true) + + get :index + + expect(response).to have_gitlab_http_status(200) + end + + it 'correctly renders an action that does not require cross project access' do + get :show, id: 'nothing' + + expect(response).to have_gitlab_http_status(200) + end + end + + describe '#skip_cross_project_access_check' do + controller(ApplicationController) do + # `described_class` is not available in this context + include ControllerWithCrossProjectAccessCheck # rubocop:disable RSpec/DescribedClass + + requires_cross_project_access + + skip_cross_project_access_check index: true, show: false, + unless: -> { unless_condition }, + if: -> { if_condition } + + def index + render nothing: true + end + + def show + render nothing: true + end + + def edit + render nothing: true + end + + def unless_condition + false + end + + def if_condition + true + end + end + + it 'renders a success when the check is skipped' do + get :index + + expect(response).to have_gitlab_http_status(200) + end + + it 'is executed when the `if` condition returns false' do + expect(controller).to receive(:if_condition).and_return(false) + + get :index + + expect(response).to have_gitlab_http_status(404) + end + + it 'is executed when the `unless` condition returns true' do + expect(controller).to receive(:unless_condition).and_return(true) + + get :index + + expect(response).to have_gitlab_http_status(404) + end + + it 'does not skip the check on an action that is not skipped' do + get :show, id: 'hello' + + expect(response).to have_gitlab_http_status(404) + end + + it 'does not skip the check on an action that was not defined to skip' do + get :edit, id: 'hello' + + expect(response).to have_gitlab_http_status(404) + end + end + end +end diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb index 775f9db1c6e..e14ba29fa70 100644 --- a/spec/controllers/projects/clusters/gcp_controller_spec.rb +++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb @@ -161,7 +161,7 @@ describe Projects::Clusters::GcpController do it 'renders the cluster form with an error' do go - expect(response).to set_flash[:alert] + expect(response).to set_flash.now[:alert] expect(response).to render_template('new') 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 92db7284e0e..24310b847e8 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -17,7 +17,7 @@ describe Projects::MergeRequests::CreationsController do before do fork_project.add_master(user) - + Projects::ForkService.new(project, user).execute(fork_project) sign_in(user) end @@ -125,4 +125,66 @@ describe Projects::MergeRequests::CreationsController do end end end + + describe 'GET #branch_to' do + before do + allow(Ability).to receive(:allowed?).and_call_original + end + + it 'fetches the commit if a user has access' do + expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true } + + get :branch_to, + namespace_id: fork_project.namespace, + project_id: fork_project, + target_project_id: project.id, + ref: 'master' + + expect(assigns(:commit)).not_to be_nil + expect(response).to have_gitlab_http_status(200) + end + + it 'does not load the commit when the user cannot read the project' do + expect(Ability).to receive(:allowed?).with(user, :read_project, project) { false } + + get :branch_to, + namespace_id: fork_project.namespace, + project_id: fork_project, + target_project_id: project.id, + ref: 'master' + + expect(assigns(:commit)).to be_nil + expect(response).to have_gitlab_http_status(200) + end + end + + describe 'GET #update_branches' do + before do + allow(Ability).to receive(:allowed?).and_call_original + end + + it 'lists the branches of another fork if the user has access' do + expect(Ability).to receive(:allowed?).with(user, :read_project, project) { true } + + get :update_branches, + namespace_id: fork_project.namespace, + project_id: fork_project, + target_project_id: project.id + + expect(assigns(:target_branches)).not_to be_empty + expect(response).to have_gitlab_http_status(200) + end + + it 'does not list branches when the user cannot read the project' do + expect(Ability).to receive(:allowed?).with(user, :read_project, project) { false } + + get :update_branches, + namespace_id: fork_project.namespace, + project_id: fork_project, + target_project_id: project.id + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:target_branches)).to eq([]) + end + end end diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb index e9e7d357d9c..2192fd5cae2 100644 --- a/spec/controllers/projects/pages_domains_controller_spec.rb +++ b/spec/controllers/projects/pages_domains_controller_spec.rb @@ -46,7 +46,46 @@ describe Projects::PagesDomainsController do post(:create, request_params.merge(pages_domain: pages_domain_params)) end.to change { PagesDomain.count }.by(1) - expect(response).to redirect_to(project_pages_path(project)) + created_domain = PagesDomain.reorder(:id).last + + expect(created_domain).to be_present + expect(response).to redirect_to(project_pages_domain_path(project, created_domain)) + end + end + + describe 'POST verify' do + let(:params) { request_params.merge(id: pages_domain.domain) } + + def stub_service + service = double(:service) + + expect(VerifyPagesDomainService).to receive(:new) { service } + + service + end + + it 'handles verification success' do + expect(stub_service).to receive(:execute).and_return(status: :success) + + post :verify, params + + expect(response).to redirect_to project_pages_domain_path(project, pages_domain) + expect(flash[:notice]).to eq('Successfully verified domain ownership') + end + + it 'handles verification failure' do + expect(stub_service).to receive(:execute).and_return(status: :failed) + + post :verify, params + + expect(response).to redirect_to project_pages_domain_path(project, pages_domain) + expect(flash[:alert]).to eq('Failed to verify domain ownership') + end + + it 'returns a 404 response for an unknown domain' do + post :verify, request_params.merge(id: 'unknown-domain') + + expect(response).to have_gitlab_http_status(404) end end diff --git a/spec/controllers/projects/prometheus_controller_spec.rb b/spec/controllers/projects/prometheus/metrics_controller_spec.rb index bbfe78d305a..f17f819feee 100644 --- a/spec/controllers/projects/prometheus_controller_spec.rb +++ b/spec/controllers/projects/prometheus/metrics_controller_spec.rb @@ -1,20 +1,20 @@ -require('spec_helper') +require 'spec_helper' -describe Projects::PrometheusController do +describe Projects::Prometheus::MetricsController do let(:user) { create(:user) } - let!(:project) { create(:project) } + let(:project) { create(:project) } let(:prometheus_service) { double('prometheus_service') } before do allow(controller).to receive(:project).and_return(project) - allow(project).to receive(:prometheus_service).and_return(prometheus_service) + allow(project).to receive(:find_or_initialize_service).with('prometheus').and_return(prometheus_service) project.add_master(user) sign_in(user) end - describe 'GET #active_metrics' do + describe 'GET #active_common' do context 'when prometheus metrics are enabled' do context 'when data is not present' do before do @@ -22,7 +22,7 @@ describe Projects::PrometheusController do end it 'returns no content response' do - get :active_metrics, project_params(format: :json) + get :active_common, project_params(format: :json) expect(response).to have_gitlab_http_status(204) end @@ -36,7 +36,7 @@ describe Projects::PrometheusController do end it 'returns no content response' do - get :active_metrics, project_params(format: :json) + get :active_common, project_params(format: :json) expect(response).to have_gitlab_http_status(200) expect(json_response).to eq(sample_response.deep_stringify_keys) @@ -45,7 +45,7 @@ describe Projects::PrometheusController do context 'when requesting non json response' do it 'returns not found response' do - get :active_metrics, project_params + get :active_common, project_params expect(response).to have_gitlab_http_status(404) end diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb index d572085661d..eca9baed9c9 100644 --- a/spec/controllers/projects/uploads_controller_spec.rb +++ b/spec/controllers/projects/uploads_controller_spec.rb @@ -7,4 +7,12 @@ describe Projects::UploadsController do end it_behaves_like 'handle uploads' + + context 'when the URL the old style, without /-/system' do + it 'responds with a redirect to the login page' do + get :show, namespace_id: 'project', project_id: 'avatar', filename: 'foo.png', secret: 'bar' + + expect(response).to redirect_to(new_user_session_path) + end + end end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 37f961d0c94..30c06ddf744 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -16,6 +16,32 @@ describe SearchController do 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 } + end + + it 'still allows accessing the search page' do + get :show + + expect(response).to have_gitlab_http_status(200) + end + + it 'still blocks searches without a project_id' do + get :show, search: 'hello' + + expect(response).to have_gitlab_http_status(404) + end + + it 'allows searches with a project_id' do + get :show, search: 'hello', project_id: create(:project, :public).id + + expect(response).to have_gitlab_http_status(200) + end + end + context 'on restricted projects' do context 'when signed out' do before do diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 2898c4b119e..b0acf4a49ac 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -74,6 +74,31 @@ describe UsersController do end end end + + context 'json with events' do + let(:project) { create(:project) } + before do + project.add_developer(user) + Gitlab::DataBuilder::Push.build_sample(project, user) + + sign_in(user) + end + + it 'loads events' do + get :show, username: user, format: :json + + expect(assigns(:events)).not_to be_empty + end + + it 'hides events if the user cannot read cross project' do + allow(Ability).to receive(:allowed?).and_call_original + expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + + get :show, username: user, format: :json + + expect(assigns(:events)).to be_empty + end + end end describe 'GET #calendar' do diff --git a/spec/factories/pages_domains.rb b/spec/factories/pages_domains.rb index 61b04708da2..35b44e1c52e 100644 --- a/spec/factories/pages_domains.rb +++ b/spec/factories/pages_domains.rb @@ -1,6 +1,25 @@ FactoryBot.define do factory :pages_domain, class: 'PagesDomain' do - domain 'my.domain.com' + sequence(:domain) { |n| "my#{n}.domain.com" } + verified_at { Time.now } + enabled_until { 1.week.from_now } + + trait :disabled do + verified_at nil + enabled_until nil + end + + trait :unverified do + verified_at nil + end + + trait :reverify do + enabled_until { 1.hour.from_now } + end + + trait :expired do + enabled_until { 1.hour.ago } + end trait :with_certificate do certificate '-----BEGIN CERTIFICATE----- diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index a01c129defd..7eeed7da998 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -19,7 +19,7 @@ describe "Admin Runners" do end it 'has all necessary texts' do - expect(page).to have_text "How to setup" + expect(page).to have_text "Setup a shared Runner manually" expect(page).to have_text "Runners with last contact more than a minute ago: 1" end @@ -54,7 +54,7 @@ describe "Admin Runners" do end it 'has all necessary texts including no runner message' do - expect(page).to have_text "How to setup" + expect(page).to have_text "Setup a shared Runner manually" expect(page).to have_text "Runners with last contact more than a minute ago: 0" expect(page).to have_text 'No runners found' end diff --git a/spec/features/auto_deploy_spec.rb b/spec/features/auto_deploy_spec.rb deleted file mode 100644 index 9aef68b7156..00000000000 --- a/spec/features/auto_deploy_spec.rb +++ /dev/null @@ -1,77 +0,0 @@ -require 'spec_helper' - -describe 'Auto deploy' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - - shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do - context 'when no deployment service is active' do - before do - trun_off - end - - it 'does not show a button to set up auto deploy' do - visit project_path(project) - expect(page).to have_no_content('Set up auto deploy') - end - end - - context 'when a deployment service is active' do - before do - trun_on - visit project_path(project) - end - - it 'shows a button to set up auto deploy' do - expect(page).to have_link('Set up auto deploy') - end - - it 'includes OpenShift as an available template', :js do - click_link 'Set up auto deploy' - click_button 'Apply a GitLab CI Yaml template' - - within '.gitlab-ci-yml-selector' do - expect(page).to have_content('OpenShift') - end - end - - it 'creates a merge request using "auto-deploy" branch', :js do - click_link 'Set up auto deploy' - click_button 'Apply a GitLab CI Yaml template' - within '.gitlab-ci-yml-selector' do - click_on 'OpenShift' - end - wait_for_requests - click_button 'Commit changes' - - expect(page).to have_content('New Merge Request From auto-deploy into master') - end - end - end - - context 'when user configured kubernetes from Integration > Kubernetes' do - before do - create :kubernetes_service, project: project - project.add_master(user) - sign_in user - end - - let(:trun_on) { project.deployment_platform.update!(active: true) } - let(:trun_off) { project.deployment_platform.update!(active: false) } - - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end - - context 'when user configured kubernetes from CI/CD > Clusters' do - before do - create(:cluster, :provided_by_gcp, projects: [project]) - project.add_master(user) - sign_in user - end - - let(:trun_on) { project.deployment_platform.cluster.update!(enabled: true) } - let(:trun_off) { project.deployment_platform.cluster.update!(enabled: false) } - - it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' - end -end diff --git a/spec/features/issues/filtered_search/recent_searches_spec.rb b/spec/features/issues/filtered_search/recent_searches_spec.rb index f355cec3ba9..41b9ada988a 100644 --- a/spec/features/issues/filtered_search/recent_searches_spec.rb +++ b/spec/features/issues/filtered_search/recent_searches_spec.rb @@ -39,8 +39,8 @@ describe 'Recent searches', :js do items = all('.filtered-search-history-dropdown-item', visible: false, count: 2) - expect(items[0].text).to eq('label:~qux garply') - expect(items[1].text).to eq('label:~foo bar') + expect(items[0].text).to eq('label: ~qux garply') + expect(items[1].text).to eq('label: ~foo bar') end it 'saved recent searches are restored last on the list' do diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index faf14be4818..c2c4b479a8a 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -271,6 +271,18 @@ describe 'New/edit issue', :js do end end + context 'inline edit' do + before do + visit project_issue_path(project, issue) + end + + it 'opens inline edit form with shortcut' do + find('body').send_keys('e') + + expect(page).to have_selector('.detail-page-description form') + end + end + describe 'sub-group project' do let(:group) { create(:group) } let(:nested_group_1) { create(:group, parent: group) } diff --git a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb index 8ac9821b879..7f1d1934103 100644 --- a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb +++ b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb @@ -11,7 +11,7 @@ feature 'project owner sees a link to create a license file in empty project', : scenario 'project master creates a license file from a template' do visit project_path(project) - click_on 'LICENSE' + click_on 'Add License' expect(page).to have_content('New file') expect(current_path).to eq( diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb index 6f097ad16c7..b5104747d00 100644 --- a/spec/features/projects/new_project_spec.rb +++ b/spec/features/projects/new_project_spec.rb @@ -140,7 +140,7 @@ feature 'New project' do find('#import-project-tab').click end - context 'from git repository url' do + context 'from git repository url, "Repo by URL"' do before do first('.import_git').click end @@ -157,6 +157,18 @@ feature 'New project' do expect(git_import_instructions).to be_visible expect(git_import_instructions).to have_content 'Git repository URL' end + + it 'keeps "Import project" tab open after form validation error' do + collision_project = create(:project, name: 'test-name-collision', namespace: user.namespace) + + fill_in 'project_import_url', with: collision_project.http_url_to_repo + fill_in 'project_path', with: collision_project.path + + click_on 'Create project' + + expect(page).to have_css('#import-project-pane.active') + expect(page).not_to have_css('.toggle-import-form.hide') + end end context 'from GitHub' do diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb index 3f1ef0b2a47..a96f2c186a4 100644 --- a/spec/features/projects/pages_spec.rb +++ b/spec/features/projects/pages_spec.rb @@ -60,7 +60,6 @@ feature 'Pages' do fill_in 'Domain', with: 'my.test.domain.com' click_button 'Create New Domain' - expect(page).to have_content('Domains (1)') expect(page).to have_content('my.test.domain.com') end end @@ -159,7 +158,6 @@ feature 'Pages' do fill_in 'Key (PEM)', with: certificate_key click_button 'Create New Domain' - expect(page).to have_content('Domains (1)') expect(page).to have_content('my.test.domain.com') end end diff --git a/spec/features/projects/show_project_spec.rb b/spec/features/projects/show_project_spec.rb index 0b94c9eae5d..0a014e9f080 100644 --- a/spec/features/projects/show_project_spec.rb +++ b/spec/features/projects/show_project_spec.rb @@ -17,4 +17,321 @@ describe 'Project show page', :feature do expect(page).to have_content("This project was scheduled for deletion, but failed with the following message: #{project.delete_error}") end end + + describe 'stat button existence' do + # For "New file", "Add License" functionality, + # see spec/features/projects/files/project_owner_creates_license_file_spec.rb + # see spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb + + let(:user) { create(:user) } + + describe 'empty project' do + let(:project) { create(:project, :public, :empty_repo) } + let(:presenter) { project.present(current_user: user) } + + describe 'as a normal user' do + before do + sign_in(user) + + visit project_path(project) + end + + it 'no Auto DevOps button if can not manage pipelines' do + page.within('.project-stats') do + expect(page).not_to have_link('Enable Auto DevOps') + expect(page).not_to have_link('Auto DevOps enabled') + end + end + + it '"Auto DevOps enabled" button not linked' do + project.create_auto_devops!(enabled: true) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).to have_text('Auto DevOps enabled') + end + end + end + + describe 'as a master' do + before do + project.add_master(user) + sign_in(user) + + visit project_path(project) + end + + it '"New file" button linked to new file page' do + page.within('.project-stats') do + expect(page).to have_link('New file', href: project_new_blob_path(project, project.default_branch || 'master')) + end + end + + it '"Add Readme" button linked to new file populated for a readme' do + page.within('.project-stats') do + expect(page).to have_link('Add Readme', href: presenter.add_readme_path) + end + end + + it '"Add License" button linked to new file populated for a license' do + page.within('.project-stats') do + expect(page).to have_link('Add License', href: presenter.add_license_path) + end + end + + describe 'Auto DevOps button' do + it '"Enable Auto DevOps" button linked to settings page' do + page.within('.project-stats') do + expect(page).to have_link('Enable Auto DevOps', href: project_settings_ci_cd_path(project, anchor: 'js-general-pipeline-settings')) + end + end + + it '"Auto DevOps enabled" anchor linked to settings page' do + project.create_auto_devops!(enabled: true) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).to have_link('Auto DevOps enabled', href: project_settings_ci_cd_path(project, anchor: 'js-general-pipeline-settings')) + end + end + end + + describe 'Kubernetes cluster button' do + it '"Add Kubernetes cluster" button linked to clusters page' do + page.within('.project-stats') do + expect(page).to have_link('Add Kubernetes cluster', href: new_project_cluster_path(project)) + end + end + + it '"Kubernetes cluster" anchor linked to cluster page' do + cluster = create(:cluster, :provided_by_gcp, projects: [project]) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).to have_link('Kubernetes configured', href: project_cluster_path(project, cluster)) + end + end + end + end + end + + describe 'populated project' do + let(:project) { create(:project, :public, :repository) } + let(:presenter) { project.present(current_user: user) } + + describe 'as a normal user' do + before do + sign_in(user) + + visit project_path(project) + end + + it 'no Auto DevOps button if can not manage pipelines' do + page.within('.project-stats') do + expect(page).not_to have_link('Enable Auto DevOps') + expect(page).not_to have_link('Auto DevOps enabled') + end + end + + it '"Auto DevOps enabled" button not linked' do + project.create_auto_devops!(enabled: true) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).to have_text('Auto DevOps enabled') + end + end + + it 'no Kubernetes cluster button if can not manage clusters' do + page.within('.project-stats') do + expect(page).not_to have_link('Add Kubernetes cluster') + expect(page).not_to have_link('Kubernetes configured') + end + end + end + + describe 'as a master' do + before do + allow_any_instance_of(AutoDevopsHelper).to receive(:show_auto_devops_callout?).and_return(false) + project.add_master(user) + sign_in(user) + + visit project_path(project) + end + + it 'no "Add Changelog" button if the project already has a changelog' do + expect(project.repository.changelog).not_to be_nil + + page.within('.project-stats') do + expect(page).not_to have_link('Add Changelog') + end + end + + it 'no "Add License" button if the project already has a license' do + expect(project.repository.license_blob).not_to be_nil + + page.within('.project-stats') do + expect(page).not_to have_link('Add License') + end + end + + it 'no "Add Contribution guide" button if the project already has a contribution guide' do + expect(project.repository.contribution_guide).not_to be_nil + + page.within('.project-stats') do + expect(page).not_to have_link('Add Contribution guide') + end + end + + describe 'GitLab CI configuration button' do + it '"Set up CI/CD" button linked to new file populated for a .gitlab-ci.yml' do + expect(project.repository.gitlab_ci_yml).to be_nil + + page.within('.project-stats') do + expect(page).to have_link('Set up CI/CD', href: presenter.add_ci_yml_path) + end + end + + it 'no "Set up CI/CD" button if the project already has a .gitlab-ci.yml' do + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add .gitlab-ci.yml", + file_path: '.gitlab-ci.yml', + file_content: File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) + ).execute + + expect(project.repository.gitlab_ci_yml).not_to be_nil + + visit project_path(project) + + page.within('.project-stats') do + expect(page).not_to have_link('Set up CI/CD') + end + end + + it 'no "Set up CI/CD" button if the project has Auto DevOps enabled' do + project.create_auto_devops!(enabled: true) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).not_to have_link('Set up CI/CD') + end + end + end + + describe 'Auto DevOps button' do + it '"Enable Auto DevOps" button linked to settings page' do + page.within('.project-stats') do + expect(page).to have_link('Enable Auto DevOps', href: project_settings_ci_cd_path(project, anchor: 'js-general-pipeline-settings')) + end + end + + it '"Enable Auto DevOps" button linked to settings page' do + project.create_auto_devops!(enabled: true) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).to have_link('Auto DevOps enabled', href: project_settings_ci_cd_path(project, anchor: 'js-general-pipeline-settings')) + end + end + + it 'no Auto DevOps button if Auto DevOps callout is shown' do + allow_any_instance_of(AutoDevopsHelper).to receive(:show_auto_devops_callout?).and_return(true) + + visit project_path(project) + + expect(page).to have_selector('.js-autodevops-banner') + + page.within('.project-stats') do + expect(page).not_to have_link('Enable Auto DevOps') + expect(page).not_to have_link('Auto DevOps enabled') + end + end + + it 'no "Enable Auto DevOps" button when .gitlab-ci.yml already exists' do + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add .gitlab-ci.yml", + file_path: '.gitlab-ci.yml', + file_content: File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) + ).execute + + expect(project.repository.gitlab_ci_yml).not_to be_nil + + visit project_path(project) + + page.within('.project-stats') do + expect(page).not_to have_link('Enable Auto DevOps') + expect(page).not_to have_link('Auto DevOps enabled') + end + end + end + + describe 'Kubernetes cluster button' do + it '"Add Kubernetes cluster" button linked to clusters page' do + page.within('.project-stats') do + expect(page).to have_link('Add Kubernetes cluster', href: new_project_cluster_path(project)) + end + end + + it '"Kubernetes cluster" button linked to cluster page' do + cluster = create(:cluster, :provided_by_gcp, projects: [project]) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).to have_link('Kubernetes configured', href: project_cluster_path(project, cluster)) + end + end + end + + describe '"Set up Koding" button' do + it 'no "Set up Koding" button if Koding disabled' do + stub_application_setting(koding_enabled?: false) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).not_to have_link('Set up Koding') + end + end + + it 'no "Set up Koding" button if the project already has a .koding.yml' do + stub_application_setting(koding_enabled?: true) + allow(Gitlab::CurrentSettings.current_application_settings).to receive(:koding_url).and_return('http://koding.example.com') + expect(project.repository.changelog).not_to be_nil + allow_any_instance_of(Repository).to receive(:koding_yml).and_return(project.repository.changelog) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).not_to have_link('Set up Koding') + end + end + + it '"Set up Koding" button linked to new file populated for a .koding.yml' do + stub_application_setting(koding_enabled?: true) + + visit project_path(project) + + page.within('.project-stats') do + expect(page).to have_link('Set up Koding', href: presenter.add_koding_stack_path) + end + end + end + end + end + end end diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index b66a7dea598..cfe979a8647 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -25,6 +25,24 @@ feature 'Project' do end end + describe 'shows tip about push to create git command' do + let(:user) { create(:user) } + + before do + sign_in user + visit new_project_path + end + + it 'shows the command in a popover', :js do + page.within '.profile-settings-sidebar' do + click_link 'Show command' + end + + expect(page).to have_css('.popover .push-to-create-popover #push_to_create_tip') + expect(page).to have_content 'Private projects can be created in your personal namespace with:' + end + end + describe 'description' do let(:project) { create(:project, :repository) } let(:path) { project_path(project) } @@ -128,8 +146,8 @@ feature 'Project' do end describe 'removal', :js do - let(:user) { create(:user, username: 'test', name: 'test') } - let(:project) { create(:project, namespace: user.namespace, name: 'project1') } + let(:user) { create(:user) } + let(:project) { create(:project, namespace: user.namespace) } before do sign_in(user) @@ -138,8 +156,8 @@ feature 'Project' do end it 'removes a project' do - expect { remove_with_confirm('Remove project', project.path) }.to change {Project.count}.by(-1) - expect(page).to have_content "Project 'test / project1' is in the process of being deleted." + expect { remove_with_confirm('Remove project', project.path) }.to change { Project.count }.by(-1) + expect(page).to have_content "Project '#{project.full_name}' is in the process of being deleted." expect(Project.all.count).to be_zero expect(project.issues).to be_empty expect(project.merge_requests).to be_empty diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index aec9de6c7ca..df65c2d2f83 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -7,6 +7,20 @@ feature 'Runners' do sign_in(user) end + context 'when user opens runners page' do + given(:project) { create(:project) } + + background do + project.add_master(user) + end + + scenario 'user can see a button to install runners on kubernetes clusters' do + visit runners_path(project) + + expect(page).to have_link('Install Runner on Kubernetes', href: project_clusters_path(project)) + end + end + context 'when a project has enabled shared_runners' do given(:project) { create(:project) } diff --git a/spec/features/tags/master_views_tags_spec.rb b/spec/features/tags/master_views_tags_spec.rb index 4662367d843..b625e7065cc 100644 --- a/spec/features/tags/master_views_tags_spec.rb +++ b/spec/features/tags/master_views_tags_spec.rb @@ -13,7 +13,7 @@ feature 'Master views tags' do before do visit project_path(project) - click_on 'README' + click_on 'Add Readme' fill_in :commit_message, with: 'Add a README file', visible: true click_button 'Commit changes' visit project_tags_path(project) diff --git a/spec/features/users/show_spec.rb b/spec/features/users/show_spec.rb new file mode 100644 index 00000000000..b5bbb2c0ea5 --- /dev/null +++ b/spec/features/users/show_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe 'User page' do + let(:user) { create(:user) } + + it 'shows all the tabs' do + visit(user_path(user)) + + page.within '.nav-links' do + expect(page).to have_link('Activity') + expect(page).to have_link('Groups') + expect(page).to have_link('Contributed projects') + expect(page).to have_link('Personal projects') + expect(page).to have_link('Snippets') + end + end +end diff --git a/spec/finders/concerns/finder_methods_spec.rb b/spec/finders/concerns/finder_methods_spec.rb new file mode 100644 index 00000000000..a4ad331f613 --- /dev/null +++ b/spec/finders/concerns/finder_methods_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe FinderMethods do + let(:finder_class) do + Class.new do + include FinderMethods + + attr_reader :current_user + + def initialize(user) + @current_user = user + end + + def execute + Project.all + end + end + end + + let(:user) { create(:user) } + let(:finder) { finder_class.new(user) } + let(:authorized_project) { create(:project) } + let(:unauthorized_project) { create(:project) } + + before do + authorized_project.add_developer(user) + end + + describe '#find_by!' do + it 'returns the project if the user has access' do + expect(finder.find_by!(id: authorized_project.id)).to eq(authorized_project) + end + + it 'raises not found when the project is not found' do + expect { finder.find_by!(id: 0) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'raises not found the user does not have access' do + expect { finder.find_by!(id: unauthorized_project.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + describe '#find' do + it 'returns the project if the user has access' do + expect(finder.find(authorized_project.id)).to eq(authorized_project) + end + + it 'raises not found when the project is not found' do + expect { finder.find(0) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'raises not found the user does not have access' do + expect { finder.find(unauthorized_project.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + describe '#find_by' do + it 'returns the project if the user has access' do + expect(finder.find_by(id: authorized_project.id)).to eq(authorized_project) + end + + it 'returns nil when the project is not found' do + expect(finder.find_by(id: 0)).to be_nil + end + + it 'returns nil when the user does not have access' do + expect(finder.find_by(id: unauthorized_project.id)).to be_nil + end + end +end diff --git a/spec/finders/concerns/finder_with_cross_project_access_spec.rb b/spec/finders/concerns/finder_with_cross_project_access_spec.rb new file mode 100644 index 00000000000..c784fb87972 --- /dev/null +++ b/spec/finders/concerns/finder_with_cross_project_access_spec.rb @@ -0,0 +1,118 @@ +require 'spec_helper' + +describe FinderWithCrossProjectAccess do + let(:finder_class) do + Class.new do + prepend FinderWithCrossProjectAccess + include FinderMethods + + requires_cross_project_access if: -> { requires_access? } + + attr_reader :current_user + + def initialize(user) + @current_user = user + end + + def execute + Issue.all + end + end + end + + let(:user) { create(:user) } + subject(:finder) { finder_class.new(user) } + let!(:result) { create(:issue) } + + before do + result.project.add_master(user) + end + + def expect_access_check_on_result + expect(finder).not_to receive(:requires_access?) + expect(Ability).to receive(:allowed?).with(user, :read_issue, result).and_call_original + 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) + .and_return(false) + end + + describe '#execute' do + it 'returns a issue if the check is disabled' do + expect(finder).to receive(:requires_access?).and_return(false) + + expect(finder.execute).to include(result) + end + + it 'returns an empty relation when the check is enabled' do + expect(finder).to receive(:requires_access?).and_return(true) + + expect(finder.execute).to be_empty + end + + it 'only queries once when check is enabled' do + expect(finder).to receive(:requires_access?).and_return(true) + + expect { finder.execute }.not_to exceed_query_limit(1) + end + + it 'only queries once when check is disabled' do + expect(finder).to receive(:requires_access?).and_return(false) + + expect { finder.execute }.not_to exceed_query_limit(1) + end + end + + describe '#find' do + it 'checks the accessibility of the subject directly' do + expect_access_check_on_result + + finder.find(result.id) + end + + it 'returns the issue' do + expect(finder.find(result.id)).to eq(result) + end + end + + describe '#find_by' do + it 'checks the accessibility of the subject directly' do + expect_access_check_on_result + + finder.find_by(id: result.id) + end + end + + describe '#find_by!' do + it 'checks the accessibility of the subject directly' do + expect_access_check_on_result + + finder.find_by!(id: result.id) + end + + it 're-enables the check after the find failed' do + finder.find_by!(id: 9999) rescue ActiveRecord::RecordNotFound + + expect(finder.instance_variable_get(:@should_skip_cross_project_check)) + .to eq(false) + end + end + end + + context 'when the user can read cross project' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_cross_project) + .and_return(true) + end + + it 'returns the result' do + expect(finder).not_to receive(:requires_access?) + + expect(finder.execute).to include(result) + end + end +end diff --git a/spec/finders/events_finder_spec.rb b/spec/finders/events_finder_spec.rb index 18d6c0cfd74..62968e83292 100644 --- a/spec/finders/events_finder_spec.rb +++ b/spec/finders/events_finder_spec.rb @@ -26,6 +26,14 @@ describe EventsFinder do expect(events).not_to include(opened_merge_request_event) end + + it 'returns nothing when the current user cannot read cross project' do + expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + + events = described_class.new(source: user, current_user: user).execute + + expect(events).to be_empty + end end context 'when targeting a project' do diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index 06031aee217..dc76efea35b 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/spec/finders/labels_finder_spec.rb @@ -5,6 +5,8 @@ describe LabelsFinder do let(:group_1) { create(:group) } let(:group_2) { create(:group) } let(:group_3) { create(:group) } + let(:private_group_1) { create(:group, :private) } + let(:private_subgroup_1) { create(:group, :private, parent: private_group_1) } let(:project_1) { create(:project, namespace: group_1) } let(:project_2) { create(:project, namespace: group_2) } @@ -20,6 +22,8 @@ describe LabelsFinder do let!(:group_label_1) { create(:group_label, group: group_1, title: 'Label 1 (group)') } let!(:group_label_2) { create(:group_label, group: group_1, title: 'Group Label 2') } let!(:group_label_3) { create(:group_label, group: group_2, title: 'Group Label 3') } + let!(:private_group_label_1) { create(:group_label, group: private_group_1, title: 'Private Group Label 1') } + let!(:private_subgroup_label_1) { create(:group_label, group: private_subgroup_1, title: 'Private Sub Group Label 1') } let(:user) { create(:user) } @@ -66,6 +70,25 @@ describe LabelsFinder do expect(finder.execute).to eq [group_label_2, group_label_1] end end + + context 'when including labels from group ancestors', :nested_groups do + it 'returns labels from group and its ancestors' do + private_group_1.add_developer(user) + private_subgroup_1.add_developer(user) + + finder = described_class.new(user, group_id: private_subgroup_1.id, only_group_labels: true, include_ancestor_groups: true) + + expect(finder.execute).to eq [private_group_label_1, private_subgroup_label_1] + end + + it 'ignores labels from groups which user can not read' do + private_subgroup_1.add_developer(user) + + finder = described_class.new(user, group_id: private_subgroup_1.id, only_group_labels: true, include_ancestor_groups: true) + + expect(finder.execute).to eq [private_subgroup_label_1] + end + end end context 'filtering by project_id' do diff --git a/spec/finders/milestones_finder_spec.rb b/spec/finders/milestones_finder_spec.rb index 0b3cf7ece5f..656d120311a 100644 --- a/spec/finders/milestones_finder_spec.rb +++ b/spec/finders/milestones_finder_spec.rb @@ -70,4 +70,12 @@ describe MilestonesFinder do expect(result.to_a).to contain_exactly(milestone_1) end end + + describe '#find_by' do + it 'finds a single milestone' do + finder = described_class.new(project_ids: [project_1.id], state: 'all') + + expect(finder.find_by(iid: milestone_3.iid)).to eq(milestone_3) + end + end end diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb index 54a07eccaba..1ae0bd988f2 100644 --- a/spec/finders/snippets_finder_spec.rb +++ b/spec/finders/snippets_finder_spec.rb @@ -162,8 +162,26 @@ describe SnippetsFinder do end end - describe "#execute" do - # Snippet visibility scenarios are included in more details in spec/support/snippet_visibility.rb - include_examples 'snippet visibility', described_class + describe '#execute' do + let(:project) { create(:project, :public) } + let!(:project_snippet) { create(:project_snippet, :public, project: project) } + let!(:personal_snippet) { create(:personal_snippet, :public) } + let(:user) { create(:user) } + subject(:finder) { described_class.new(user) } + + it 'returns project- and personal snippets' do + expect(finder.execute).to contain_exactly(project_snippet, personal_snippet) + 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) { false } + end + + it 'returns only personal snippets when the user cannot read cross project' do + expect(finder.execute).to contain_exactly(personal_snippet) + end + end end end diff --git a/spec/finders/user_recent_events_finder_spec.rb b/spec/finders/user_recent_events_finder_spec.rb new file mode 100644 index 00000000000..3ca0f7c3c89 --- /dev/null +++ b/spec/finders/user_recent_events_finder_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe UserRecentEventsFinder do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:project_owner) { project.creator } + let!(:event) { create(:event, project: project, author: project_owner) } + + subject(:finder) { described_class.new(user, project_owner) } + + describe '#execute' do + it 'does not include the event when a user does not have access to the project' do + expect(finder.execute).to be_empty + end + + context 'when the user has access to a project' do + before do + project.add_developer(user) + end + + it 'includes the event' do + expect(finder.execute).to include(event) + end + + it 'does not include the event if the user cannot read cross project' do + expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + expect(finder.execute).to be_empty + end + end + end +end diff --git a/spec/fixtures/api/schemas/public_api/v4/pages_domain/basic.json b/spec/fixtures/api/schemas/public_api/v4/pages_domain/basic.json index e8c17298b43..ed8ed9085c0 100644 --- a/spec/fixtures/api/schemas/public_api/v4/pages_domain/basic.json +++ b/spec/fixtures/api/schemas/public_api/v4/pages_domain/basic.json @@ -4,6 +4,9 @@ "domain": { "type": "string" }, "url": { "type": "uri" }, "project_id": { "type": "integer" }, + "verified": { "type": "boolean" }, + "verification_code": { "type": ["string", "null"] }, + "enabled_until": { "type": ["date", "null"] }, "certificate_expiration": { "type": "object", "properties": { @@ -14,6 +17,6 @@ "additionalProperties": false } }, - "required": ["domain", "url", "project_id"], + "required": ["domain", "url", "project_id", "verified", "verification_code", "enabled_until"], "additionalProperties": false } diff --git a/spec/fixtures/api/schemas/public_api/v4/pages_domain/detail.json b/spec/fixtures/api/schemas/public_api/v4/pages_domain/detail.json index 08db8d47050..b57d544f896 100644 --- a/spec/fixtures/api/schemas/public_api/v4/pages_domain/detail.json +++ b/spec/fixtures/api/schemas/public_api/v4/pages_domain/detail.json @@ -3,6 +3,9 @@ "properties": { "domain": { "type": "string" }, "url": { "type": "uri" }, + "verified": { "type": "boolean" }, + "verification_code": { "type": ["string", "null"] }, + "enabled_until": { "type": ["date", "null"] }, "certificate": { "type": "object", "properties": { @@ -15,6 +18,6 @@ "additionalProperties": false } }, - "required": ["domain", "url"], + "required": ["domain", "url", "verified", "verification_code", "enabled_until"], "additionalProperties": false } diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index a030796c54e..1fa194fe1b8 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -86,7 +86,7 @@ describe BlobHelper do it 'verifies blob is text' do expect(helper).not_to receive(:blob_text_viewable?) - button = edit_blob_link(project, 'refs/heads/master', 'README.md') + button = edit_blob_button(project, 'refs/heads/master', 'README.md') expect(button).to start_with('<button') end @@ -96,17 +96,17 @@ describe BlobHelper do expect(project.repository).not_to receive(:blob_at) - edit_blob_link(project, 'refs/heads/master', 'README.md', blob: blob) + edit_blob_button(project, 'refs/heads/master', 'README.md', blob: blob) end it 'returns a link with the proper route' do - link = edit_blob_link(project, 'master', 'README.md') + link = edit_blob_button(project, 'master', 'README.md') expect(Capybara.string(link).find_link('Edit')[:href]).to eq("/#{project.full_path}/edit/master/README.md") end it 'returns a link with the passed link_opts on the expected route' do - link = edit_blob_link(project, 'master', 'README.md', link_opts: { mr_id: 10 }) + link = edit_blob_button(project, 'master', 'README.md', link_opts: { mr_id: 10 }) expect(Capybara.string(link).find_link('Edit')[:href]).to eq("/#{project.full_path}/edit/master/README.md?mr_id=10") end diff --git a/spec/helpers/dashboard_helper_spec.rb b/spec/helpers/dashboard_helper_spec.rb new file mode 100644 index 00000000000..7ba24ba2956 --- /dev/null +++ b/spec/helpers/dashboard_helper_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe DashboardHelper do + let(:user) { build(:user) } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?) { true } + end + + describe '#dashboard_nav_links' do + it 'has all the expected links by default' do + menu_items = [:projects, :groups, :activity, :milestones, :snippets] + + expect(helper.dashboard_nav_links).to contain_exactly(*menu_items) + end + + it 'does not contain cross project elements when the user cannot read cross project' do + expect(helper).to receive(:can?).with(user, :read_cross_project) { false } + + expect(helper.dashboard_nav_links).not_to include(:activity, :milestones) + end + end +end diff --git a/spec/helpers/explore_helper_spec.rb b/spec/helpers/explore_helper_spec.rb new file mode 100644 index 00000000000..12651d80e36 --- /dev/null +++ b/spec/helpers/explore_helper_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe ExploreHelper do + let(:user) { build(:user) } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?) { true } + end + + describe '#explore_nav_links' do + it 'has all the expected links by default' do + menu_items = [:projects, :groups, :snippets] + + expect(helper.explore_nav_links).to contain_exactly(*menu_items) + end + end +end diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 5f608fe18d9..b48c252acd3 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -201,4 +201,39 @@ describe GroupsHelper do end end end + + describe '#group_sidebar_links' do + let(:group) { create(:group, :public) } + let(:user) { create(:user) } + before do + allow(helper).to receive(:current_user) { user } + allow(helper).to receive(:can?) { true } + helper.instance_variable_set(:@group, group) + end + + it 'returns all the expected links' do + links = [ + :overview, :activity, :issues, :labels, :milestones, :merge_requests, + :group_members, :settings + ] + + expect(helper.group_sidebar_links).to include(*links) + end + + it 'includes settings when the user can admin the group' do + expect(helper).to receive(:current_user) { user } + expect(helper).to receive(:can?).with(user, :admin_group, group) { false } + + expect(helper.group_sidebar_links).not_to include(:settings) + end + + it 'excludes cross project features when the user cannot read cross project' do + cross_project_features = [:activity, :issues, :labels, :milestones, + :merge_requests] + + expect(helper).to receive(:can?).with(user, :read_cross_project) { false } + + expect(helper.group_sidebar_links).not_to include(*cross_project_features) + end + end end diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 7fa665aecdc..2fecd1a3d27 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -173,23 +173,23 @@ describe IssuablesHelper do @project = issue.project expected_data = { - 'endpoint' => "/#{@project.full_path}/issues/#{issue.iid}", - 'updateEndpoint' => "/#{@project.full_path}/issues/#{issue.iid}.json", - 'canUpdate' => true, - 'canDestroy' => true, - 'issuableRef' => "##{issue.iid}", - 'markdownPreviewPath' => "/#{@project.full_path}/preview_markdown", - 'markdownDocsPath' => '/help/user/markdown', - 'issuableTemplates' => [], - 'projectPath' => @project.path, - 'projectNamespace' => @project.namespace.path, - 'initialTitleHtml' => issue.title, - 'initialTitleText' => issue.title, - 'initialDescriptionHtml' => '<p dir="auto">issue text</p>', - 'initialDescriptionText' => 'issue text', - 'initialTaskStatus' => '0 of 0 tasks completed' + endpoint: "/#{@project.full_path}/issues/#{issue.iid}", + updateEndpoint: "/#{@project.full_path}/issues/#{issue.iid}.json", + canUpdate: true, + canDestroy: true, + issuableRef: "##{issue.iid}", + markdownPreviewPath: "/#{@project.full_path}/preview_markdown", + markdownDocsPath: '/help/user/markdown', + issuableTemplates: [], + projectPath: @project.path, + projectNamespace: @project.namespace.path, + initialTitleHtml: issue.title, + initialTitleText: issue.title, + initialDescriptionHtml: '<p dir="auto">issue text</p>', + initialDescriptionText: 'issue text', + initialTaskStatus: '0 of 0 tasks completed' } - expect(JSON.parse(helper.issuable_initial_data(issue))).to eq(expected_data) + expect(helper.issuable_initial_data(issue)).to eq(expected_data) end end diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index ddf881a7b6f..aeef5352333 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -113,21 +113,6 @@ describe IssuesHelper do end end - describe "milestone_options" do - it "gets closed milestone from current issue" do - closed_milestone = create(:closed_milestone, project: project) - milestone1 = create(:milestone, project: project) - milestone2 = create(:milestone, project: project) - issue.update_attributes(milestone_id: closed_milestone.id) - - options = milestone_options(issue) - - expect(options).to have_selector('option[selected]', text: closed_milestone.title) - expect(options).to have_selector('option', text: milestone1.title) - expect(options).to have_selector('option', text: milestone2.title) - end - end - describe "#link_to_discussions_to_resolve" do describe "passing only a merge request" do let(:merge_request) { create(:merge_request) } diff --git a/spec/helpers/nav_helper_spec.rb b/spec/helpers/nav_helper_spec.rb new file mode 100644 index 00000000000..e840c927d59 --- /dev/null +++ b/spec/helpers/nav_helper_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe NavHelper do + describe '#header_links' do + before do + allow(helper).to receive(:session) { {} } + end + + context 'when the user is logged in' do + let(:user) { build(:user) } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?) { true } + end + + it 'has all the expected links by default' do + menu_items = [:user_dropdown, :search, :issues, :merge_requests, :todos] + + expect(helper.header_links).to contain_exactly(*menu_items) + end + + it 'contains the impersonation link while impersonating' do + expect(helper).to receive(:session) { { impersonator_id: 1 } } + + expect(helper.header_links).to include(:admin_impersonation) + end + + context 'when the user cannot read cross project' do + before do + allow(helper).to receive(:can?).with(user, :read_cross_project) { false } + end + + it 'does not contain cross project elements when the user cannot read cross project' do + expect(helper.header_links).not_to include(:issues, :merge_requests, :todos, :search) + end + + it 'shows the search box when the user cannot read cross project and he is visiting a project' do + helper.instance_variable_set(:@project, create(:project)) + + expect(helper.header_links).to include(:search) + end + end + end + + it 'returns only the sign in and search when the user is not logged in' do + allow(helper).to receive(:current_user).and_return(nil) + allow(helper).to receive(:can?).with(nil, :read_cross_project) { true } + + expect(helper.header_links).to contain_exactly(:sign_in, :search) + end + end +end diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb index 749aa25e632..e2a0c4322ff 100644 --- a/spec/helpers/preferences_helper_spec.rb +++ b/spec/helpers/preferences_helper_spec.rb @@ -77,103 +77,6 @@ describe PreferencesHelper do end end - describe '#default_project_view' do - context 'user not signed in' do - before do - helper.instance_variable_set(:@project, project) - stub_user - end - - context 'when repository is empty' do - let(:project) { create(:project_empty_repo, :public) } - - it 'returns activity if user has repository access' do - allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(true) - - expect(helper.default_project_view).to eq('activity') - end - - it 'returns activity if user does not have repository access' do - allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(false) - - expect(helper.default_project_view).to eq('activity') - end - end - - context 'when repository is not empty' do - let(:project) { create(:project, :public, :repository) } - - it 'returns files and readme if user has repository access' do - allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(true) - - expect(helper.default_project_view).to eq('files') - end - - it 'returns activity if user does not have repository access' do - allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(false) - - expect(helper.default_project_view).to eq('activity') - end - end - end - - context 'user signed in' do - let(:user) { create(:user, :readme) } - let(:project) { create(:project, :public, :repository) } - - before do - helper.instance_variable_set(:@project, project) - allow(helper).to receive(:current_user).and_return(user) - end - - context 'when the user is allowed to see the code' do - it 'returns the project view' do - allow(helper).to receive(:can?).with(user, :download_code, project).and_return(true) - - expect(helper.default_project_view).to eq('readme') - end - end - - context 'with wikis enabled and the right policy for the user' do - before do - project.project_feature.update_attribute(:issues_access_level, 0) - allow(helper).to receive(:can?).with(user, :download_code, project).and_return(false) - end - - it 'returns wiki if the user has the right policy' do - allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(true) - - expect(helper.default_project_view).to eq('wiki') - end - - it 'returns customize_workflow if the user does not have the right policy' do - allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(false) - - expect(helper.default_project_view).to eq('customize_workflow') - end - end - - context 'with issues as a feature available' do - it 'return issues' do - allow(helper).to receive(:can?).with(user, :download_code, project).and_return(false) - allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(false) - - expect(helper.default_project_view).to eq('projects/issues/issues') - end - end - - context 'with no activity, no wikies and no issues' do - it 'returns customize_workflow as default' do - project.project_feature.update_attribute(:issues_access_level, 0) - allow(helper).to receive(:can?).with(user, :download_code, project).and_return(false) - allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(false) - - expect(helper.default_project_view).to eq('customize_workflow') - end - end - end - end - def stub_user(messages = {}) if messages.empty? allow(helper).to receive(:current_user).and_return(nil) diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index b67fee2fcc0..ce96e90e2d7 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -75,6 +75,12 @@ describe ProjectsHelper do describe "#project_list_cache_key", :clean_gitlab_redis_shared_state do let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?).with(user, :read_cross_project) { true } + end it "includes the route" do expect(helper.project_list_cache_key(project)).to include(project.route.cache_key) @@ -106,6 +112,10 @@ describe ProjectsHelper do expect(helper.project_list_cache_key(project).last).to start_with('v') end + it 'includes wether or not the user can read cross project' do + expect(helper.project_list_cache_key(project)).to include('cross-project:true') + end + it "includes the pipeline status when there is a status" do create(:ci_pipeline, :success, project: project, sha: project.commit.sha) @@ -264,32 +274,6 @@ describe ProjectsHelper do end end - describe '#license_short_name' do - let(:project) { create(:project) } - - context 'when project.repository has a license_key' do - it 'returns the nickname of the license if present' do - allow(project.repository).to receive(:license_key).and_return('agpl-3.0') - - expect(helper.license_short_name(project)).to eq('GNU AGPLv3') - end - - it 'returns the name of the license if nickname is not present' do - allow(project.repository).to receive(:license_key).and_return('mit') - - expect(helper.license_short_name(project)).to eq('MIT License') - end - end - - context 'when project.repository has no license_key but a license_blob' do - it 'returns LICENSE' do - allow(project.repository).to receive(:license_key).and_return(nil) - - expect(helper.license_short_name(project)).to eq('LICENSE') - end - end - end - describe '#sanitized_import_error' do let(:project) { create(:project, :repository) } @@ -462,6 +446,22 @@ describe ProjectsHelper do end end + describe('#push_to_create_project_command') do + let(:user) { create(:user, username: 'john') } + + it 'returns the command to push to create project over HTTP' do + allow(Gitlab::CurrentSettings.current_application_settings).to receive(:enabled_git_access_protocol) { 'http' } + + expect(helper.push_to_create_project_command(user)).to eq('git push --set-upstream http://test.host/john/$(git rev-parse --show-toplevel | xargs basename).git $(git rev-parse --abbrev-ref HEAD)') + end + + it 'returns the command to push to create project over SSH' do + allow(Gitlab::CurrentSettings.current_application_settings).to receive(:enabled_git_access_protocol) { 'ssh' } + + expect(helper.push_to_create_project_command(user)).to eq('git push --set-upstream git@localhost:john/$(git rev-parse --show-toplevel | xargs basename).git $(git rev-parse --abbrev-ref HEAD)') + end + end + describe '#any_projects?' do let!(:project) { create(:project) } diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb index 03f78de8e91..6332217b920 100644 --- a/spec/helpers/users_helper_spec.rb +++ b/spec/helpers/users_helper_spec.rb @@ -14,4 +14,17 @@ describe UsersHelper do is_expected.to include("title=\"#{user.email}\"") end end + + describe '#profile_tabs' do + subject(:tabs) { helper.profile_tabs } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?).and_return(true) + end + + it 'includes all the expected tabs' do + expect(tabs).to include(:activity, :groups, :contributed, :projects, :snippets) + end + end end diff --git a/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js b/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js index 4a516c517ef..59bd2650081 100644 --- a/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js +++ b/spec/javascripts/filtered_search/components/recent_searches_dropdown_content_spec.js @@ -1,7 +1,6 @@ import Vue from 'vue'; import eventHub from '~/filtered_search/event_hub'; -import RecentSearchesDropdownContent from '~/filtered_search/components/recent_searches_dropdown_content'; - +import RecentSearchesDropdownContent from '~/filtered_search/components/recent_searches_dropdown_content.vue'; import FilteredSearchTokenKeys from '~/filtered_search/filtered_search_token_keys'; const createComponent = (propsData) => { diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 323b8a9572d..94fcc6c7f2b 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -113,7 +113,9 @@ if (process.env.BABEL_ENV === 'coverage') { // exempt these files from the coverage report const troubleMakers = [ './blob_edit/blob_bundle.js', - './boards/boards_bundle.js', + './boards/components/modal/empty_state.js', + './boards/components/modal/footer.js', + './boards/components/modal/header.js', './cycle_analytics/cycle_analytics_bundle.js', './cycle_analytics/components/stage_plan_component.js', './cycle_analytics/components/stage_staging_component.js', diff --git a/spec/lib/banzai/commit_renderer_spec.rb b/spec/lib/banzai/commit_renderer_spec.rb index 84adaebdcbe..e7ebb2a332f 100644 --- a/spec/lib/banzai/commit_renderer_spec.rb +++ b/spec/lib/banzai/commit_renderer_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Banzai::CommitRenderer do describe '.render' do it 'renders a commit description and title' do - user = double(:user) + user = build(:user) project = create(:project, :repository) expect(Banzai::ObjectRenderer).to receive(:new).with(project, user).and_call_original diff --git a/spec/lib/banzai/filter/issuable_state_filter_spec.rb b/spec/lib/banzai/filter/issuable_state_filter_spec.rb index cacb33d3372..17347768a49 100644 --- a/spec/lib/banzai/filter/issuable_state_filter_spec.rb +++ b/spec/lib/banzai/filter/issuable_state_filter_spec.rb @@ -77,6 +77,14 @@ describe Banzai::Filter::IssuableStateFilter do expect(doc.css('a').last.text).to eq("#{closed_issue.to_reference(other_project)} (closed)") end + it 'skips cross project references if the user cannot read cross project' do + expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + link = create_link(closed_issue.to_reference(other_project), issue: closed_issue.id, reference_type: 'issue') + doc = filter(link, context.merge(project: other_project)) + + expect(doc.css('a').last.text).to eq("#{closed_issue.to_reference(other_project)}") + end + it 'does not append state when filter is not enabled' do link = create_link('text', issue: closed_issue.id, reference_type: 'issue') context = { current_user: user } diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb index 5a7858e77f3..9a2e521fdcf 100644 --- a/spec/lib/banzai/filter/redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/redactor_filter_spec.rb @@ -6,7 +6,7 @@ describe Banzai::Filter::RedactorFilter do it 'ignores non-GFM links' do html = %(See <a href="https://google.com/">Google</a>) - doc = filter(html, current_user: double) + doc = filter(html, current_user: build(:user)) expect(doc.css('a').length).to eq 1 end diff --git a/spec/lib/banzai/redactor_spec.rb b/spec/lib/banzai/redactor_spec.rb index 2424c3fdc66..1fa89137972 100644 --- a/spec/lib/banzai/redactor_spec.rb +++ b/spec/lib/banzai/redactor_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Banzai::Redactor do - let(:user) { build(:user) } + let(:user) { create(:user) } let(:project) { build(:project) } let(:redactor) { described_class.new(project, user) } @@ -88,6 +88,55 @@ describe Banzai::Redactor do end end + context 'when the user cannot read cross project' do + include ActionView::Helpers::UrlHelper + let(:project) { create(:project) } + let(:other_project) { create(:project, :public) } + + def create_link(issuable) + type = issuable.class.name.underscore.downcase + link_to(issuable.to_reference, '', + class: 'gfm has-tooltip', + title: issuable.title, + data: { + reference_type: type, + "#{type}": issuable.id + }) + end + + before do + project.add_developer(user) + + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global) { false } + allow(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + end + + it 'skips links to issues within the same project' do + issue = create(:issue, project: project) + link = create_link(issue) + doc = Nokogiri::HTML.fragment(link) + + redactor.redact([doc]) + result = doc.css('a').last + + expect(result['class']).to include('has-tooltip') + expect(result['title']).to eq(issue.title) + end + + it 'removes info from a cross project reference' do + issue = create(:issue, project: other_project) + link = create_link(issue) + doc = Nokogiri::HTML.fragment(link) + + redactor.redact([doc]) + result = doc.css('a').last + + expect(result['class']).not_to include('has-tooltip') + expect(result['title']).to be_empty + end + end + describe '#redact_nodes' do it 'redacts an Array of nodes' do doc = Nokogiri::HTML.fragment('<a href="foo">foo</a>') diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb index 4cef3bdb24b..0a63567ee40 100644 --- a/spec/lib/banzai/reference_parser/issue_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb @@ -19,19 +19,58 @@ describe Banzai::ReferenceParser::IssueParser do it 'returns the nodes when the user can read the issue' do expect(Ability).to receive(:issues_readable_by_user) - .with([issue], user) - .and_return([issue]) + .with([issue], user) + .and_return([issue]) expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) end it 'returns an empty Array when the user can not read the issue' do expect(Ability).to receive(:issues_readable_by_user) - .with([issue], user) - .and_return([]) + .with([issue], user) + .and_return([]) expect(subject.nodes_visible_to_user(user, [link])).to eq([]) end + + context 'when the user cannot read cross project' do + let(:issue) { create(:issue) } + + before do + allow(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global) { false } + end + + it 'returns the nodes when the user can read the issue' do + expect(Ability).to receive(:allowed?) + .with(user, :read_issue_iid, issue) + .and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the user can not read the issue' do + expect(Ability).to receive(:allowed?) + .with(user, :read_issue_iid, issue) + .and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + + context 'when the issue is not cross project' do + let(:issue) { create(:issue, project: project) } + + it 'does not check `can_read_reference` if the issue is not cross project' do + expect(Ability).to receive(:issues_readable_by_user) + .with([issue], user) + .and_return([]) + + expect(subject).not_to receive(:can_read_reference?).with(user, issue) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + end end context 'when the link does not have a data-issue attribute' do diff --git a/spec/lib/gitlab/contributions_calendar_spec.rb b/spec/lib/gitlab/contributions_calendar_spec.rb index f1655854486..49a179ba875 100644 --- a/spec/lib/gitlab/contributions_calendar_spec.rb +++ b/spec/lib/gitlab/contributions_calendar_spec.rb @@ -118,6 +118,19 @@ describe Gitlab::ContributionsCalendar do expect(calendar.events_by_date(today)).to contain_exactly(e1) expect(calendar(contributor).events_by_date(today)).to contain_exactly(e1, e2, e3) end + + context 'when the user cannot read read cross project' do + before do + allow(Ability).to receive(:allowed?).and_call_original + expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + end + + it 'does not return any events' do + create_event(public_project, today) + + expect(calendar(user).events_by_date(today)).to be_empty + end + end end describe '#starting_year' do diff --git a/spec/lib/gitlab/cross_project_access/check_collection_spec.rb b/spec/lib/gitlab/cross_project_access/check_collection_spec.rb new file mode 100644 index 00000000000..a9e7575240e --- /dev/null +++ b/spec/lib/gitlab/cross_project_access/check_collection_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Gitlab::CrossProjectAccess::CheckCollection do + subject(:collection) { described_class.new } + + describe '#add_collection' do + it 'merges the checks of 2 collections' do + initial_check = double('check') + collection.add_check(initial_check) + + other_collection = described_class.new + other_check = double('other_check') + other_collection.add_check(other_check) + + shared_check = double('shared check') + other_collection.add_check(shared_check) + collection.add_check(shared_check) + + collection.add_collection(other_collection) + + expect(collection.checks).to contain_exactly(initial_check, shared_check, other_check) + end + end + + describe '#should_run?' do + def fake_check(run, skip) + check = double("Check: run=#{run} - skip={skip}") + allow(check).to receive(:should_run?).and_return(run) + allow(check).to receive(:should_skip?).and_return(skip) + allow(check).to receive(:skip).and_return(skip) + + check + end + + it 'returns true if one of the check says it should run' do + check = fake_check(true, false) + other_check = fake_check(false, false) + + collection.add_check(check) + collection.add_check(other_check) + + expect(collection.should_run?(double)).to be_truthy + end + + it 'returns false if one of the check says it should be skipped' do + check = fake_check(true, false) + other_check = fake_check(false, true) + + collection.add_check(check) + collection.add_check(other_check) + + expect(collection.should_run?(double)).to be_falsey + end + end +end diff --git a/spec/lib/gitlab/cross_project_access/check_info_spec.rb b/spec/lib/gitlab/cross_project_access/check_info_spec.rb new file mode 100644 index 00000000000..bc9dbf2bece --- /dev/null +++ b/spec/lib/gitlab/cross_project_access/check_info_spec.rb @@ -0,0 +1,111 @@ +require 'spec_helper' + +describe Gitlab::CrossProjectAccess::CheckInfo do + let(:dummy_controller) { double } + + before do + allow(dummy_controller).to receive(:action_name).and_return('index') + end + + describe '#should_run?' do + it 'runs when an action is defined' do + info = described_class.new({ index: true }, nil, nil, false) + + expect(info.should_run?(dummy_controller)).to be_truthy + end + + it 'runs when the action is missing' do + info = described_class.new({}, nil, nil, false) + + expect(info.should_run?(dummy_controller)).to be_truthy + end + + it 'does not run when the action is excluded' do + info = described_class.new({ index: false }, nil, nil, false) + + expect(info.should_run?(dummy_controller)).to be_falsy + end + + it 'runs when the `if` conditional is true' do + info = described_class.new({}, -> { true }, nil, false) + + expect(info.should_run?(dummy_controller)).to be_truthy + end + + it 'does not run when the if condition is false' do + info = described_class.new({}, -> { false }, nil, false) + + expect(info.should_run?(dummy_controller)).to be_falsy + end + + it 'does not run when the `unless` check is true' do + info = described_class.new({}, nil, -> { true }, false) + + expect(info.should_run?(dummy_controller)).to be_falsy + end + + it 'runs when the `unless` check is false' do + info = described_class.new({}, nil, -> { false }, false) + + expect(info.should_run?(dummy_controller)).to be_truthy + end + + it 'returns the the oposite of #should_skip? when the check is a skip' do + info = described_class.new({}, nil, nil, true) + + expect(info).to receive(:should_skip?).with(dummy_controller).and_return(false) + expect(info.should_run?(dummy_controller)).to be_truthy + end + end + + describe '#should_skip?' do + it 'skips when an action is defined' do + info = described_class.new({ index: true }, nil, nil, true) + + expect(info.should_skip?(dummy_controller)).to be_truthy + end + + it 'does not skip when the action is not defined' do + info = described_class.new({}, nil, nil, true) + + expect(info.should_skip?(dummy_controller)).to be_falsy + end + + it 'does not skip when the action is excluded' do + info = described_class.new({ index: false }, nil, nil, true) + + expect(info.should_skip?(dummy_controller)).to be_falsy + end + + it 'skips when the `if` conditional is true' do + info = described_class.new({ index: true }, -> { true }, nil, true) + + expect(info.should_skip?(dummy_controller)).to be_truthy + end + + it 'does not skip the `if` conditional is false' do + info = described_class.new({ index: true }, -> { false }, nil, true) + + expect(info.should_skip?(dummy_controller)).to be_falsy + end + + it 'does not skip when the `unless` check is true' do + info = described_class.new({ index: true }, nil, -> { true }, true) + + expect(info.should_skip?(dummy_controller)).to be_falsy + end + + it 'skips when `unless` check is false' do + info = described_class.new({ index: true }, nil, -> { false }, true) + + expect(info.should_skip?(dummy_controller)).to be_truthy + end + + it 'returns the the oposite of #should_run? when the check is not a skip' do + info = described_class.new({}, nil, nil, false) + + expect(info).to receive(:should_run?).with(dummy_controller).and_return(false) + expect(info.should_skip?(dummy_controller)).to be_truthy + end + end +end diff --git a/spec/lib/gitlab/cross_project_access/class_methods_spec.rb b/spec/lib/gitlab/cross_project_access/class_methods_spec.rb new file mode 100644 index 00000000000..5349685e633 --- /dev/null +++ b/spec/lib/gitlab/cross_project_access/class_methods_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe Gitlab::CrossProjectAccess::ClassMethods do + let(:dummy_class) do + Class.new do + extend Gitlab::CrossProjectAccess::ClassMethods + end + end + let(:dummy_proc) { lambda { false } } + + describe '#requires_cross_project_access' do + it 'creates a correct check when a hash is passed' do + expect(Gitlab::CrossProjectAccess) + .to receive(:add_check).with(dummy_class, + actions: { hello: true, world: false }, + positive_condition: dummy_proc, + negative_condition: dummy_proc) + + dummy_class.requires_cross_project_access( + hello: true, world: false, if: dummy_proc, unless: dummy_proc + ) + end + + it 'creates a correct check when an array is passed' do + expect(Gitlab::CrossProjectAccess) + .to receive(:add_check).with(dummy_class, + actions: { hello: true, world: true }, + positive_condition: nil, + negative_condition: nil) + + dummy_class.requires_cross_project_access(:hello, :world) + end + + it 'creates a correct check when an array and a hash is passed' do + expect(Gitlab::CrossProjectAccess) + .to receive(:add_check).with(dummy_class, + actions: { hello: true, world: true }, + positive_condition: dummy_proc, + negative_condition: dummy_proc) + + dummy_class.requires_cross_project_access( + :hello, :world, if: dummy_proc, unless: dummy_proc + ) + end + end +end diff --git a/spec/lib/gitlab/cross_project_access_spec.rb b/spec/lib/gitlab/cross_project_access_spec.rb new file mode 100644 index 00000000000..614b0473c7e --- /dev/null +++ b/spec/lib/gitlab/cross_project_access_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +describe Gitlab::CrossProjectAccess do + let(:super_class) { Class.new } + let(:descendant_class) { Class.new(super_class) } + let(:current_instance) { described_class.new } + + before do + allow(described_class).to receive(:instance).and_return(current_instance) + end + + describe '#add_check' do + it 'keeps track of the properties to check' do + expect do + described_class.add_check(super_class, + actions: { index: true }, + positive_condition: -> { true }, + negative_condition: -> { false }) + end.to change { described_class.checks.size }.by(1) + end + + it 'builds the check correctly' do + check_collection = described_class.add_check(super_class, + actions: { index: true }, + positive_condition: -> { 'positive' }, + negative_condition: -> { 'negative' }) + + check = check_collection.checks.first + + expect(check.actions).to eq(index: true) + expect(check.positive_condition.call).to eq('positive') + expect(check.negative_condition.call).to eq('negative') + end + + it 'merges the checks of a parent class into existing checks of a subclass' do + subclass_collection = described_class.add_check(descendant_class) + + expect(subclass_collection).to receive(:add_collection).and_call_original + + described_class.add_check(super_class) + end + + it 'merges the existing checks of a superclass into the checks of a subclass' do + super_collection = described_class.add_check(super_class) + descendant_collection = described_class.add_check(descendant_class) + + expect(descendant_collection.checks).to include(*super_collection.checks) + end + end + + describe '#find_check' do + it 'returns a check when it was defined for a superclass' do + expected_check = described_class.add_check(super_class, + actions: { index: true }, + positive_condition: -> { 'positive' }, + negative_condition: -> { 'negative' }) + + expect(described_class.find_check(descendant_class.new)) + .to eq(expected_check) + end + + it 'caches the result for a subclass' do + described_class.add_check(super_class, + actions: { index: true }, + positive_condition: -> { 'positive' }, + negative_condition: -> { 'negative' }) + + expect(described_class.instance).to receive(:closest_parent).once.and_call_original + + 2.times { described_class.find_check(descendant_class.new) } + end + + it 'returns the checks for the closest class if there are more checks available' do + described_class.add_check(super_class, + actions: { index: true }) + expected_check = described_class.add_check(descendant_class, + actions: { index: true, show: false }) + + check = described_class.find_check(descendant_class.new) + + expect(check).to eq(expected_check) + end + end +end diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb index cd602ccab8e..73d60c021c8 100644 --- a/spec/lib/gitlab/diff/highlight_spec.rb +++ b/spec/lib/gitlab/diff/highlight_spec.rb @@ -72,6 +72,28 @@ describe Gitlab::Diff::Highlight do expect(subject[5].text).to eq(code) expect(subject[5].text).to be_html_safe end + + context 'when the inline diff marker has an invalid range' do + before do + allow_any_instance_of(Gitlab::Diff::InlineDiffMarker).to receive(:mark).and_raise(RangeError) + end + + it 'keeps the original rich line' do + code = %q{+ raise RuntimeError, "System commands must be given as an array of strings"} + + expect(subject[5].text).to eq(code) + expect(subject[5].text).not_to be_html_safe + end + + it 'reports to Sentry if configured' do + allow(Gitlab::Sentry).to receive(:enabled?).and_return(true) + + expect(Gitlab::Sentry).to receive(:context) + expect(Raven).to receive(:capture_exception) + + subject + end + end end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index d601a383a98..13358995383 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2283,6 +2283,20 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(subject).to match(/\h{40}/) end end + + context 'with trailing whitespace in an invalid patch', :skip_gitaly_mock do + let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+ \n ====== \n \n Sample repo for testing gitlab features\n" } + + it 'does not include whitespace warnings in the error' do + allow(repository).to receive(:run_git!).and_call_original + allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT')) + + expect { subject }.to raise_error do |error| + expect(error).to be_a(described_class::GitError) + expect(error.message).not_to include('trailing whitespace') + end + end + end end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 19d3f55501e..6f07e423c1b 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -534,6 +534,19 @@ describe Gitlab::GitAccess do expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.') end + context 'when the project repository does not exist' do + it 'returns not found' do + project.add_guest(user) + repo = project.repository + FileUtils.rm_rf(repo.path) + + # Sanity check for rm_rf + expect(repo.exists?).to eq(false) + + expect { pull_access_check }.to raise_error(Gitlab::GitAccess::NotFoundError, 'A repository for this project does not exist yet.') + end + end + describe 'without access to project' do context 'pull code' do it { expect { pull_access_check }.to raise_not_found } diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 215f1ecc9c5..730ede99fc9 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -57,7 +57,7 @@ describe Gitlab::GitAccessWiki do # Sanity check for rm_rf expect(wiki_repo.exists?).to eq(false) - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'A repository for this project does not exist yet.') + expect { subject }.to raise_error(Gitlab::GitAccess::NotFoundError, 'A repository for this project does not exist yet.') end end end diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 001c4d3e10a..9be3fa633a7 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -113,7 +113,7 @@ describe Gitlab::GitalyClient::CommitService do .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) .and_return([]) - client.tree_entries(repository, revision, path) + client.tree_entries(repository, revision, path, false) end context 'with UTF-8 params strings' do @@ -126,7 +126,7 @@ describe Gitlab::GitalyClient::CommitService do .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) .and_return([]) - client.tree_entries(repository, revision, path) + client.tree_entries(repository, revision, path, false) end end end diff --git a/spec/lib/gitlab/middleware/go_spec.rb b/spec/lib/gitlab/middleware/go_spec.rb index 60a134be939..b24c9882c0c 100644 --- a/spec/lib/gitlab/middleware/go_spec.rb +++ b/spec/lib/gitlab/middleware/go_spec.rb @@ -3,19 +3,30 @@ require 'spec_helper' describe Gitlab::Middleware::Go do let(:app) { double(:app) } let(:middleware) { described_class.new(app) } + let(:env) do + { + 'rack.input' => '', + 'REQUEST_METHOD' => 'GET' + } + end describe '#call' do describe 'when go-get=0' do + before do + env['QUERY_STRING'] = 'go-get=0' + end + it 'skips go-import generation' do - env = { 'rack.input' => '', - 'QUERY_STRING' => 'go-get=0' } expect(app).to receive(:call).with(env).and_return('no-go') middleware.call(env) end end describe 'when go-get=1' do - let(:current_user) { nil } + before do + env['QUERY_STRING'] = 'go-get=1' + env['PATH_INFO'] = "/#{path}" + end shared_examples 'go-get=1' do |enabled_protocol:| context 'with simple 2-segment project path' do @@ -54,21 +65,75 @@ describe Gitlab::Middleware::Go do project.update_attribute(:visibility_level, Project::PRIVATE) end - context 'with access to the project' do + shared_examples 'unauthorized' do + it 'returns the 2-segment group path' do + expect_response_with_path(go, enabled_protocol, group.full_path) + end + end + + context 'when not authenticated' do + it_behaves_like 'unauthorized' + end + + context 'when authenticated' do let(:current_user) { project.creator } before do project.team.add_master(current_user) end - it 'returns the full project path' do - expect_response_with_path(go, enabled_protocol, project.full_path) + shared_examples 'authenticated' do + context 'with access to the project' do + it 'returns the full project path' do + expect_response_with_path(go, enabled_protocol, project.full_path) + end + end + + context 'without access to the project' do + before do + project.team.find_member(current_user).destroy + end + + it_behaves_like 'unauthorized' + end end - end - context 'without access to the project' do - it 'returns the 2-segment group path' do - expect_response_with_path(go, enabled_protocol, group.full_path) + context 'using warden' do + before do + env['warden'] = double(authenticate: current_user) + end + + context 'when active' do + it_behaves_like 'authenticated' + end + + context 'when blocked' do + before do + current_user.block! + end + + it_behaves_like 'unauthorized' + end + end + + context 'using a personal access token' do + let(:personal_access_token) { create(:personal_access_token, user: current_user) } + + before do + env['HTTP_PRIVATE_TOKEN'] = personal_access_token.token + end + + context 'with api scope' do + it_behaves_like 'authenticated' + end + + context 'with read_user scope' do + before do + personal_access_token.update_attribute(:scopes, [:read_user]) + end + + it_behaves_like 'unauthorized' + end end end end @@ -138,12 +203,6 @@ describe Gitlab::Middleware::Go do end def go - env = { - 'rack.input' => '', - 'QUERY_STRING' => 'go-get=1', - 'PATH_INFO' => "/#{path}", - 'warden' => double(authenticate: current_user) - } middleware.call(env) end diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb index 2b488101496..c95719eff1d 100644 --- a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb @@ -24,7 +24,7 @@ describe Gitlab::Prometheus::Queries::MatchedMetricsQuery do context 'with one group where two metrics is found' do before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group]) allow(client).to receive(:label_values).and_return(metric_names) end @@ -70,7 +70,7 @@ describe Gitlab::Prometheus::Queries::MatchedMetricsQuery do context 'with one group where only one metric is found' do before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group]) allow(client).to receive(:label_values).and_return('metric_a') end @@ -99,7 +99,7 @@ describe Gitlab::Prometheus::Queries::MatchedMetricsQuery do let(:second_metric_group) { simple_metric_group(name: 'nameb', metrics: simple_metrics(added_metric_name: 'metric_c')) } before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group, second_metric_group]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group, second_metric_group]) allow(client).to receive(:label_values).and_return('metric_c') end diff --git a/spec/lib/gitlab/prometheus_client_spec.rb b/spec/lib/gitlab/prometheus_client_spec.rb index 5d86007f71f..4c3b8deefb9 100644 --- a/spec/lib/gitlab/prometheus_client_spec.rb +++ b/spec/lib/gitlab/prometheus_client_spec.rb @@ -19,41 +19,41 @@ describe Gitlab::PrometheusClient do # - execute_query: A query call shared_examples 'failure response' do context 'when request returns 400 with an error message' do - it 'raises a Gitlab::PrometheusError error' do + it 'raises a Gitlab::PrometheusClient::Error error' do req_stub = stub_prometheus_request(query_url, status: 400, body: { error: 'bar!' }) expect { execute_query } - .to raise_error(Gitlab::PrometheusError, 'bar!') + .to raise_error(Gitlab::PrometheusClient::Error, 'bar!') expect(req_stub).to have_been_requested end end context 'when request returns 400 without an error message' do - it 'raises a Gitlab::PrometheusError error' do + it 'raises a Gitlab::PrometheusClient::Error error' do req_stub = stub_prometheus_request(query_url, status: 400) expect { execute_query } - .to raise_error(Gitlab::PrometheusError, 'Bad data received') + .to raise_error(Gitlab::PrometheusClient::Error, 'Bad data received') expect(req_stub).to have_been_requested end end context 'when request returns 500' do - it 'raises a Gitlab::PrometheusError error' do + it 'raises a Gitlab::PrometheusClient::Error error' do req_stub = stub_prometheus_request(query_url, status: 500, body: { message: 'FAIL!' }) expect { execute_query } - .to raise_error(Gitlab::PrometheusError, '500 - {"message":"FAIL!"}') + .to raise_error(Gitlab::PrometheusClient::Error, '500 - {"message":"FAIL!"}') expect(req_stub).to have_been_requested end end context 'when request returns non json data' do - it 'raises a Gitlab::PrometheusError error' do + it 'raises a Gitlab::PrometheusClient::Error error' do req_stub = stub_prometheus_request(query_url, status: 200, body: 'not json') expect { execute_query } - .to raise_error(Gitlab::PrometheusError, 'Parsing response failed') + .to raise_error(Gitlab::PrometheusClient::Error, 'Parsing response failed') expect(req_stub).to have_been_requested end end @@ -65,27 +65,27 @@ describe Gitlab::PrometheusClient do subject { described_class.new(RestClient::Resource.new(prometheus_url)) } context 'exceptions are raised' do - it 'raises a Gitlab::PrometheusError error when a SocketError is rescued' do + it 'raises a Gitlab::PrometheusClient::Error error when a SocketError is rescued' do req_stub = stub_prometheus_request_with_exception(prometheus_url, SocketError) expect { subject.send(:get, '/', {}) } - .to raise_error(Gitlab::PrometheusError, "Can't connect to #{prometheus_url}") + .to raise_error(Gitlab::PrometheusClient::Error, "Can't connect to #{prometheus_url}") expect(req_stub).to have_been_requested end - it 'raises a Gitlab::PrometheusError error when a SSLError is rescued' do + it 'raises a Gitlab::PrometheusClient::Error error when a SSLError is rescued' do req_stub = stub_prometheus_request_with_exception(prometheus_url, OpenSSL::SSL::SSLError) expect { subject.send(:get, '/', {}) } - .to raise_error(Gitlab::PrometheusError, "#{prometheus_url} contains invalid SSL data") + .to raise_error(Gitlab::PrometheusClient::Error, "#{prometheus_url} contains invalid SSL data") expect(req_stub).to have_been_requested end - it 'raises a Gitlab::PrometheusError error when a RestClient::Exception is rescued' do + it 'raises a Gitlab::PrometheusClient::Error error when a RestClient::Exception is rescued' do req_stub = stub_prometheus_request_with_exception(prometheus_url, RestClient::Exception) expect { subject.send(:get, '/', {}) } - .to raise_error(Gitlab::PrometheusError, "Network connection error") + .to raise_error(Gitlab::PrometheusClient::Error, "Network connection error") expect(req_stub).to have_been_requested end end diff --git a/spec/lib/gitlab/quick_actions/command_definition_spec.rb b/spec/lib/gitlab/quick_actions/command_definition_spec.rb index f44a562dc63..b03c1e23ca3 100644 --- a/spec/lib/gitlab/quick_actions/command_definition_spec.rb +++ b/spec/lib/gitlab/quick_actions/command_definition_spec.rb @@ -40,7 +40,7 @@ describe Gitlab::QuickActions::CommandDefinition do end describe "#available?" do - let(:opts) { { go: false } } + let(:opts) { OpenStruct.new(go: false) } context "when the command has a condition block" do before do @@ -78,7 +78,7 @@ describe Gitlab::QuickActions::CommandDefinition do it "doesn't execute the command" do expect(context).not_to receive(:instance_exec) - subject.execute(context, {}, nil) + subject.execute(context, nil) expect(context.run).to be false end @@ -95,7 +95,7 @@ describe Gitlab::QuickActions::CommandDefinition do end it "doesn't execute the command" do - subject.execute(context, {}, nil) + subject.execute(context, nil) expect(context.run).to be false end @@ -109,7 +109,7 @@ describe Gitlab::QuickActions::CommandDefinition do context "when the command is provided an argument" do it "executes the command" do - subject.execute(context, {}, true) + subject.execute(context, true) expect(context.run).to be true end @@ -117,7 +117,7 @@ describe Gitlab::QuickActions::CommandDefinition do context "when the command is not provided an argument" do it "executes the command" do - subject.execute(context, {}, nil) + subject.execute(context, nil) expect(context.run).to be true end @@ -131,7 +131,7 @@ describe Gitlab::QuickActions::CommandDefinition do context "when the command is provided an argument" do it "executes the command" do - subject.execute(context, {}, true) + subject.execute(context, true) expect(context.run).to be true end @@ -139,7 +139,7 @@ describe Gitlab::QuickActions::CommandDefinition do context "when the command is not provided an argument" do it "doesn't execute the command" do - subject.execute(context, {}, nil) + subject.execute(context, nil) expect(context.run).to be false end @@ -153,7 +153,7 @@ describe Gitlab::QuickActions::CommandDefinition do context "when the command is provided an argument" do it "executes the command" do - subject.execute(context, {}, true) + subject.execute(context, true) expect(context.run).to be true end @@ -161,7 +161,7 @@ describe Gitlab::QuickActions::CommandDefinition do context "when the command is not provided an argument" do it "executes the command" do - subject.execute(context, {}, nil) + subject.execute(context, nil) expect(context.run).to be true end @@ -175,7 +175,7 @@ describe Gitlab::QuickActions::CommandDefinition do end it 'executes the command passing the parsed param' do - subject.execute(context, {}, 'something ') + subject.execute(context, 'something ') expect(context.received_arg).to eq('something') end @@ -192,7 +192,7 @@ describe Gitlab::QuickActions::CommandDefinition do end it 'returns nil' do - result = subject.explain({}, {}, nil) + result = subject.explain({}, nil) expect(result).to be_nil end @@ -204,7 +204,7 @@ describe Gitlab::QuickActions::CommandDefinition do end it 'returns this static string' do - result = subject.explain({}, {}, nil) + result = subject.explain({}, nil) expect(result).to eq 'Explanation' end @@ -216,7 +216,7 @@ describe Gitlab::QuickActions::CommandDefinition do end it 'invokes the proc' do - result = subject.explain({}, {}, 'explanation') + result = subject.explain({}, 'explanation') expect(result).to eq 'Dynamic explanation' end diff --git a/spec/lib/gitlab/quick_actions/dsl_spec.rb b/spec/lib/gitlab/quick_actions/dsl_spec.rb index ff59dc48bcb..067a30fd7e2 100644 --- a/spec/lib/gitlab/quick_actions/dsl_spec.rb +++ b/spec/lib/gitlab/quick_actions/dsl_spec.rb @@ -76,7 +76,7 @@ describe Gitlab::QuickActions::Dsl do expect(dynamic_description_def.name).to eq(:dynamic_description) expect(dynamic_description_def.aliases).to eq([]) - expect(dynamic_description_def.to_h(noteable: 'issue')[:description]).to eq('A dynamic description for ISSUE') + expect(dynamic_description_def.to_h(OpenStruct.new(noteable: 'issue'))[:description]).to eq('A dynamic description for ISSUE') expect(dynamic_description_def.explanation).to eq('') expect(dynamic_description_def.params).to eq(['The first argument', 'The second argument']) expect(dynamic_description_def.condition_block).to be_nil diff --git a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb deleted file mode 100644 index 8fdbbacd04d..00000000000 --- a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb +++ /dev/null @@ -1,63 +0,0 @@ -require 'spec_helper' - -describe Gitlab::SidekiqMiddleware::MemoryKiller do - subject { described_class.new } - let(:pid) { 999 } - - let(:worker) { double(:worker, class: 'TestWorker') } - let(:job) { { 'jid' => 123 } } - let(:queue) { 'test_queue' } - - def run - thread = subject.call(worker, job, queue) { nil } - thread&.join - end - - before do - allow(subject).to receive(:get_rss).and_return(10.kilobytes) - allow(subject).to receive(:pid).and_return(pid) - end - - context 'when MAX_RSS is set to 0' do - before do - stub_const("#{described_class}::MAX_RSS", 0) - end - - it 'does nothing' do - expect(subject).not_to receive(:sleep) - - run - end - end - - context 'when MAX_RSS is exceeded' do - before do - stub_const("#{described_class}::MAX_RSS", 5.kilobytes) - end - - it 'sends the STP, TERM and KILL signals at expected times' do - expect(subject).to receive(:sleep).with(15 * 60).ordered - expect(Process).to receive(:kill).with('SIGSTP', pid).ordered - - expect(subject).to receive(:sleep).with(30).ordered - expect(Process).to receive(:kill).with('SIGTERM', pid).ordered - - expect(subject).to receive(:sleep).with(10).ordered - expect(Process).to receive(:kill).with('SIGKILL', pid).ordered - - run - end - end - - context 'when MAX_RSS is not exceeded' do - before do - stub_const("#{described_class}::MAX_RSS", 15.kilobytes) - end - - it 'does nothing' do - expect(subject).not_to receive(:sleep) - - run - end - end -end diff --git a/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb b/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb new file mode 100644 index 00000000000..0001795c3f0 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +describe Gitlab::SidekiqMiddleware::Shutdown do + subject { described_class.new } + + let(:pid) { Process.pid } + let(:worker) { double(:worker, class: 'TestWorker') } + let(:job) { { 'jid' => 123 } } + let(:queue) { 'test_queue' } + let(:block) { proc { nil } } + + def run + subject.call(worker, job, queue) { block.call } + described_class.shutdown_thread&.join + end + + def pop_trace + subject.trace.pop(true) + end + + before do + allow(subject).to receive(:get_rss).and_return(10.kilobytes) + described_class.clear_shutdown_thread + end + + context 'when MAX_RSS is set to 0' do + before do + stub_const("#{described_class}::MAX_RSS", 0) + end + + it 'does nothing' do + expect(subject).not_to receive(:sleep) + + run + end + end + + def expect_shutdown_sequence + expect(pop_trace).to eq([:sleep, 15 * 60]) + expect(pop_trace).to eq([:kill, 'SIGTSTP', pid]) + + expect(pop_trace).to eq([:sleep, 30]) + expect(pop_trace).to eq([:kill, 'SIGTERM', pid]) + + expect(pop_trace).to eq([:sleep, 10]) + expect(pop_trace).to eq([:kill, 'SIGKILL', pid]) + end + + context 'when MAX_RSS is exceeded' do + before do + stub_const("#{described_class}::MAX_RSS", 5.kilobytes) + end + + it 'sends the TSTP, TERM and KILL signals at expected times' do + run + + expect_shutdown_sequence + end + end + + context 'when MAX_RSS is not exceeded' do + before do + stub_const("#{described_class}::MAX_RSS", 15.kilobytes) + end + + it 'does nothing' do + expect(subject).not_to receive(:sleep) + + run + end + end + + context 'when WantShutdown is raised' do + let(:block) { proc { raise described_class::WantShutdown } } + + it 'starts the shutdown sequence and re-raises the exception' do + expect { run }.to raise_exception(described_class::WantShutdown) + + # We can't expect 'run' to have joined on the shutdown thread, because + # it hit an exception. + shutdown_thread = described_class.shutdown_thread + expect(shutdown_thread).not_to be_nil + shutdown_thread.join + + expect_shutdown_sequence + end + end +end diff --git a/spec/lib/gitlab/sql/pattern_spec.rb b/spec/lib/gitlab/sql/pattern_spec.rb index ef51e3cc8df..5b5052de372 100644 --- a/spec/lib/gitlab/sql/pattern_spec.rb +++ b/spec/lib/gitlab/sql/pattern_spec.rb @@ -154,6 +154,12 @@ describe Gitlab::SQL::Pattern do it 'returns a single equality condition' do expect(fuzzy_arel_match.to_sql).to match(/title.*I?LIKE 'fo'/) end + + it 'uses LOWER instead of ILIKE when LOWER is enabled' do + rel = Issue.fuzzy_arel_match(:title, query, lower_exact_match: true) + + expect(rel.to_sql).to match(/LOWER\(.*title.*\).*=.*'fo'/) + end end context 'with two words both equal to 3 chars' do diff --git a/spec/lib/google_api/cloud_platform/client_spec.rb b/spec/lib/google_api/cloud_platform/client_spec.rb index f65e41dfea3..db9d9158b29 100644 --- a/spec/lib/google_api/cloud_platform/client_spec.rb +++ b/spec/lib/google_api/cloud_platform/client_spec.rb @@ -115,6 +115,9 @@ describe GoogleApi::CloudPlatform::Client do "initial_node_count": cluster_size, "node_config": { "machine_type": machine_type + }, + "legacy_abac": { + "enabled": true } } } ) diff --git a/spec/mailers/emails/pages_domains_spec.rb b/spec/mailers/emails/pages_domains_spec.rb new file mode 100644 index 00000000000..fe428ea657d --- /dev/null +++ b/spec/mailers/emails/pages_domains_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' +require 'email_spec' + +describe Emails::PagesDomains do + include EmailSpec::Matchers + include_context 'gitlab email notification' + + set(:project) { create(:project) } + set(:domain) { create(:pages_domain, project: project) } + set(:user) { project.owner } + + shared_examples 'a pages domain email' do + it_behaves_like 'an email sent from GitLab' + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' + + it 'has the expected content' do + aggregate_failures do + is_expected.to have_subject(email_subject) + is_expected.to have_body_text(project.human_name) + is_expected.to have_body_text(domain.domain) + is_expected.to have_body_text domain.url + is_expected.to have_body_text project_pages_domain_url(project, domain) + is_expected.to have_body_text help_page_url('user/project/pages/getting_started_part_three.md', anchor: 'dns-txt-record') + end + end + end + + describe '#pages_domain_enabled_email' do + let(:email_subject) { "#{project.path} | GitLab Pages domain '#{domain.domain}' has been enabled" } + + subject { Notify.pages_domain_enabled_email(domain, user) } + + it_behaves_like 'a pages domain email' + + it { is_expected.to have_body_text 'has been enabled' } + end + + describe '#pages_domain_disabled_email' do + let(:email_subject) { "#{project.path} | GitLab Pages domain '#{domain.domain}' has been disabled" } + + subject { Notify.pages_domain_disabled_email(domain, user) } + + it_behaves_like 'a pages domain email' + + it { is_expected.to have_body_text 'has been disabled' } + end + + describe '#pages_domain_verification_succeeded_email' do + let(:email_subject) { "#{project.path} | Verification succeeded for GitLab Pages domain '#{domain.domain}'" } + + subject { Notify.pages_domain_verification_succeeded_email(domain, user) } + + it_behaves_like 'a pages domain email' + + it { is_expected.to have_body_text 'successfully verified' } + end + + describe '#pages_domain_verification_failed_email' do + let(:email_subject) { "#{project.path} | ACTION REQUIRED: Verification failed for GitLab Pages domain '#{domain.domain}'" } + + subject { Notify.pages_domain_verification_failed_email(domain, user) } + + it_behaves_like 'a pages domain email' + + it 'says verification has failed and when the domain is enabled until' do + is_expected.to have_body_text 'Verification has failed' + is_expected.to have_body_text domain.enabled_until.strftime('%F %T') + end + end +end diff --git a/spec/migrations/enqueue_verify_pages_domain_workers_spec.rb b/spec/migrations/enqueue_verify_pages_domain_workers_spec.rb new file mode 100644 index 00000000000..afcaefa0591 --- /dev/null +++ b/spec/migrations/enqueue_verify_pages_domain_workers_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20180216121030_enqueue_verify_pages_domain_workers') + +describe EnqueueVerifyPagesDomainWorkers, :sidekiq, :migration do + around do |example| + Sidekiq::Testing.fake! do + example.run + end + end + + describe '#up' do + it 'enqueues a verification worker for every domain' do + domains = 1.upto(3).map { |i| PagesDomain.create!(domain: "my#{i}.domain.com") } + + expect { migrate! }.to change(PagesDomainVerificationWorker.jobs, :size).by(3) + + enqueued_ids = PagesDomainVerificationWorker.jobs.map { |job| job['args'] } + expected_ids = domains.map { |domain| [domain.id] } + + expect(enqueued_ids).to match_array(expected_ids) + end + end +end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 38fb98d4f50..cd175dba6da 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -204,6 +204,78 @@ describe Ability do end end + describe '.merge_requests_readable_by_user' do + context 'with an admin' do + it 'returns all merge requests' do + user = build(:user, admin: true) + merge_request = build(:merge_request) + + expect(described_class.merge_requests_readable_by_user([merge_request], user)) + .to eq([merge_request]) + end + end + + context 'without a user' do + it 'returns merge_requests that are publicly visible' do + hidden_merge_request = build(:merge_request) + visible_merge_request = build(:merge_request, source_project: build(:project, :public)) + + merge_requests = described_class + .merge_requests_readable_by_user([hidden_merge_request, visible_merge_request]) + + expect(merge_requests).to eq([visible_merge_request]) + end + end + + context 'with a user' do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:cross_project_merge_request) do + create(:merge_request, source_project: create(:project, :public)) + end + let(:other_merge_request) { create(:merge_request) } + let(:all_merge_requests) do + [merge_request, cross_project_merge_request, other_merge_request] + end + + subject(:readable_merge_requests) do + described_class.merge_requests_readable_by_user(all_merge_requests, user) + end + + before do + project.add_developer(user) + end + + it 'returns projects visible to the user' do + expect(readable_merge_requests).to contain_exactly(merge_request, cross_project_merge_request) + end + + context 'when a user cannot read cross project and a filter is passed' do + before do + allow(described_class).to receive(:allowed?).and_call_original + expect(described_class).to receive(:allowed?).with(user, :read_cross_project) { false } + end + + subject(:readable_merge_requests) do + read_cross_project_filter = -> (merge_requests) do + merge_requests.select { |mr| mr.source_project == project } + end + described_class.merge_requests_readable_by_user( + all_merge_requests, user, + filters: { read_cross_project: read_cross_project_filter } + ) + end + + it 'returns only MRs of the specified project without checking access on others' do + expect(described_class).not_to receive(:allowed?).with(user, :read_merge_request, cross_project_merge_request) + + expect(readable_merge_requests).to contain_exactly(merge_request) + end + end + end + end + describe '.issues_readable_by_user' do context 'with an admin user' do it 'returns all given issues' do @@ -250,6 +322,29 @@ describe Ability do expect(issues).to eq([visible_issue]) end end + + context 'when the user cannot read cross project' do + let(:user) { create(:user) } + let(:issue) { create(:issue) } + let(:other_project_issue) { create(:issue) } + let(:project) { issue.project } + + before do + project.add_developer(user) + + allow(described_class).to receive(:allowed?).and_call_original + allow(described_class).to receive(:allowed?).with(user, :read_cross_project, any_args) { false } + end + + it 'excludes issues from other projects whithout checking separatly when passing a scope' do + expect(described_class).not_to receive(:allowed?).with(user, :read_issue, other_project_issue) + + filters = { read_cross_project: -> (issues) { issues.where(project: project) } } + result = described_class.issues_readable_by_user(Issue.all, user, filters: filters) + + expect(result).to contain_exactly(issue) + end + end end describe '.project_disabled_features_rules' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2b6b6a61182..c27313ed88b 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -277,7 +277,7 @@ describe Ci::Build do allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1) end - it { is_expected.to be_an(Array).and all(include(key: "key_1")) } + it { is_expected.to be_an(Array).and all(include(key: "key-1")) } end context 'when project does not have jobs_cache_index' do diff --git a/spec/models/concerns/protected_ref_access_spec.rb b/spec/models/concerns/protected_ref_access_spec.rb new file mode 100644 index 00000000000..a62ca391e25 --- /dev/null +++ b/spec/models/concerns/protected_ref_access_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe ProtectedRefAccess do + subject(:protected_ref_access) do + create(:protected_branch, :masters_can_push).push_access_levels.first + end + + let(:project) { protected_ref_access.project } + + describe '#check_access' do + it 'is always true for admins' do + admin = create(:admin) + + expect(protected_ref_access.check_access(admin)).to be_truthy + end + + it 'is true for masters' do + master = create(:user) + project.add_master(master) + + expect(protected_ref_access.check_access(master)).to be_truthy + end + + it 'is for developers of the project' do + developer = create(:user) + project.add_developer(developer) + + expect(protected_ref_access.check_access(developer)).to be_falsy + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index f5c9f551e65..feed7968f09 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -221,27 +221,55 @@ describe Issue do end describe '#referenced_merge_requests' do - it 'returns the referenced merge requests' do - project = create(:project, :public) - - mr1 = create(:merge_request, - source_project: project, - source_branch: 'master', - target_branch: 'feature') + let(:project) { create(:project, :public) } + let(:issue) do + create(:issue, description: merge_request.to_reference, project: project) + end + let!(:merge_request) do + create(:merge_request, + source_project: project, + source_branch: 'master', + target_branch: 'feature') + end + it 'returns the referenced merge requests' do mr2 = create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') - issue = create(:issue, description: mr1.to_reference, project: project) - create(:note_on_issue, noteable: issue, note: mr2.to_reference, project_id: project.id) - expect(issue.referenced_merge_requests).to eq([mr1, mr2]) + expect(issue.referenced_merge_requests).to eq([merge_request, mr2]) + end + + it 'returns cross project referenced merge requests' do + other_project = create(:project, :public) + cross_project_merge_request = create(:merge_request, source_project: other_project) + create(:note_on_issue, + noteable: issue, + note: cross_project_merge_request.to_reference(issue.project), + project_id: issue.project.id) + + expect(issue.referenced_merge_requests).to eq([merge_request, cross_project_merge_request]) + end + + it 'excludes cross project references if the user cannot read cross project' do + user = create(:user) + allow(Ability).to receive(:allowed?).and_call_original + expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + + other_project = create(:project, :public) + cross_project_merge_request = create(:merge_request, source_project: other_project) + create(:note_on_issue, + noteable: issue, + note: cross_project_merge_request.to_reference(issue.project), + project_id: issue.project.id) + + expect(issue.referenced_merge_requests(user)).to eq([merge_request]) end end @@ -309,7 +337,7 @@ describe Issue do end describe '#related_branches' do - let(:user) { build(:admin) } + let(:user) { create(:admin) } before do allow(subject.project.repository).to receive(:branch_names) diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb new file mode 100644 index 00000000000..eda0e1da835 --- /dev/null +++ b/spec/models/notification_recipient_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe NotificationRecipient do + let(:user) { create(:user) } + let(:project) { create(:project, namespace: user.namespace) } + let(:target) { create(:issue, project: project) } + + subject(:recipient) { described_class.new(user, :watch, target: target, project: project) } + + it 'denies access to a target when cross project access is denied' do + allow(Ability).to receive(:allowed?).and_call_original + expect(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(false) + + expect(recipient.has_access?).to be_falsy + end +end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 9d12f96c642..95713d8b85b 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -1,6 +1,10 @@ require 'spec_helper' describe PagesDomain do + using RSpec::Parameterized::TableSyntax + + subject(:pages_domain) { described_class.new } + describe 'associations' do it { is_expected.to belong_to(:project) } end @@ -64,19 +68,51 @@ describe PagesDomain do end end + describe 'validations' do + it { is_expected.to validate_presence_of(:verification_code) } + end + + describe '#verification_code' do + subject { pages_domain.verification_code } + + it 'is set automatically with 128 bits of SecureRandom data' do + expect(SecureRandom).to receive(:hex).with(16) { 'verification code' } + + is_expected.to eq('verification code') + end + end + + describe '#keyed_verification_code' do + subject { pages_domain.keyed_verification_code } + + it { is_expected.to eq("gitlab-pages-verification-code=#{pages_domain.verification_code}") } + end + + describe '#verification_domain' do + subject { pages_domain.verification_domain } + + it { is_expected.to be_nil } + + it 'is a well-known subdomain if the domain is present' do + pages_domain.domain = 'example.com' + + is_expected.to eq('_gitlab-pages-verification-code.example.com') + end + end + describe '#url' do subject { domain.url } context 'without the certificate' do let(:domain) { build(:pages_domain, certificate: '') } - it { is_expected.to eq('http://my.domain.com') } + it { is_expected.to eq("http://#{domain.domain}") } end context 'with a certificate' do let(:domain) { build(:pages_domain, :with_certificate) } - it { is_expected.to eq('https://my.domain.com') } + it { is_expected.to eq("https://#{domain.domain}") } end end @@ -154,4 +190,108 @@ describe PagesDomain do # We test only existence of output, since the output is long it { is_expected.not_to be_empty } end + + describe '#update_daemon' do + it 'runs when the domain is created' do + domain = build(:pages_domain) + + expect(domain).to receive(:update_daemon) + + domain.save! + end + + it 'runs when the domain is destroyed' do + domain = create(:pages_domain) + + expect(domain).to receive(:update_daemon) + + domain.destroy! + end + + it 'delegates to Projects::UpdatePagesConfigurationService' do + service = instance_double('Projects::UpdatePagesConfigurationService') + expect(Projects::UpdatePagesConfigurationService).to receive(:new) { service } + expect(service).to receive(:execute) + + create(:pages_domain) + end + + context 'configuration updates when attributes change' do + set(:project1) { create(:project) } + set(:project2) { create(:project) } + set(:domain) { create(:pages_domain) } + + where(:attribute, :old_value, :new_value, :update_expected) do + now = Time.now + future = now + 1.day + + :project | nil | :project1 | true + :project | :project1 | :project1 | false + :project | :project1 | :project2 | true + :project | :project1 | nil | true + + # domain can't be set to nil + :domain | 'a.com' | 'a.com' | false + :domain | 'a.com' | 'b.com' | true + + # verification_code can't be set to nil + :verification_code | 'foo' | 'foo' | false + :verification_code | 'foo' | 'bar' | false + + :verified_at | nil | now | false + :verified_at | now | now | false + :verified_at | now | future | false + :verified_at | now | nil | false + + :enabled_until | nil | now | true + :enabled_until | now | now | false + :enabled_until | now | future | false + :enabled_until | now | nil | true + end + + with_them do + it 'runs if a relevant attribute has changed' do + a = old_value.is_a?(Symbol) ? send(old_value) : old_value + b = new_value.is_a?(Symbol) ? send(new_value) : new_value + + domain.update!(attribute => a) + + if update_expected + expect(domain).to receive(:update_daemon) + else + expect(domain).not_to receive(:update_daemon) + end + + domain.update!(attribute => b) + end + end + + context 'TLS configuration' do + set(:domain_with_tls) { create(:pages_domain, :with_key, :with_certificate) } + + let(:cert1) { domain_with_tls.certificate } + let(:cert2) { cert1 + ' ' } + let(:key1) { domain_with_tls.key } + let(:key2) { key1 + ' ' } + + it 'updates when added' do + expect(domain).to receive(:update_daemon) + + domain.update!(key: key1, certificate: cert1) + end + + it 'updates when changed' do + expect(domain_with_tls).to receive(:update_daemon) + + domain_with_tls.update!(key: key2, certificate: cert2) + end + + it 'updates when removed' do + expect(domain_with_tls).to receive(:update_daemon) + + domain_with_tls.update!(key: nil, certificate: nil) + end + end + end + end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index ed17e019d42..6693e5783a5 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -223,8 +223,8 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do context 'with cluster for all environments without prometheus installed' do context 'without environment supplied' do - it 'raises PrometheusError because cluster was not found' do - expect { service.client }.to raise_error(Gitlab::PrometheusError, /couldn't find cluster with Prometheus installed/) + it 'raises PrometheusClient::Error because cluster was not found' do + expect { service.client }.to raise_error(Gitlab::PrometheusClient::Error, /couldn't find cluster with Prometheus installed/) end end @@ -242,8 +242,8 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do context 'with prod environment supplied' do let!(:environment) { create(:environment, project: project, name: 'prod') } - it 'raises PrometheusError because cluster was not found' do - expect { service.client }.to raise_error(Gitlab::PrometheusError, /couldn't find cluster with Prometheus installed/) + it 'raises PrometheusClient::Error because cluster was not found' do + expect { service.client }.to raise_error(Gitlab::PrometheusClient::Error, /couldn't find cluster with Prometheus installed/) end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ee04d74d848..56c2d7b953e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1473,6 +1473,13 @@ describe Project do expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end + + it 'returns false when the repo is not empty' do + project.add_master(user) + expect(project).to receive(:empty_repo?).and_return(false) + + expect(project.user_can_push_to_empty_repo?(user)).to be_falsey + end end describe '#container_registry_url' do diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb index 2cf669e8191..d1bf98995e7 100644 --- a/spec/policies/issuable_policy_spec.rb +++ b/spec/policies/issuable_policy_spec.rb @@ -1,12 +1,14 @@ require 'spec_helper' describe IssuablePolicy, models: true do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project) } + let(:policies) { described_class.new(user, issue) } + describe '#rules' do context 'when discussion is locked for the issuable' do - let(:user) { create(:user) } - let(:project) { create(:project, :public) } let(:issue) { create(:issue, project: project, discussion_locked: true) } - let(:policies) { described_class.new(user, issue) } context 'when the user is not a project member' do it 'can not create a note' do diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index a4af9361ea6..793b724bfca 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -30,41 +30,41 @@ describe IssuePolicy do end it 'does not allow non-members to read issues' do - expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(non_member, issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(non_member, issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows guests to read issues' do - expect(permissions(guest, issue)).to be_allowed(:read_issue) + expect(permissions(guest, issue)).to be_allowed(:read_issue, :read_issue_iid) expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue) - expect(permissions(guest, issue_no_assignee)).to be_allowed(:read_issue) + expect(permissions(guest, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) expect(permissions(guest, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue) end it 'allows reporters to read, update, and admin issues' do - expect(permissions(reporter, issue)).to be_allowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows reporters from group links to read, update, and admin issues' do - expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows issue authors to read and update their issues' do - expect(permissions(author, issue)).to be_allowed(:read_issue, :update_issue) + expect(permissions(author, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(author, issue)).to be_disallowed(:admin_issue) - expect(permissions(author, issue_no_assignee)).to be_allowed(:read_issue) + expect(permissions(author, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) expect(permissions(author, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue) end it 'allows issue assignees to read and update their issues' do - expect(permissions(assignee, issue)).to be_allowed(:read_issue, :update_issue) + expect(permissions(assignee, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(assignee, issue)).to be_disallowed(:admin_issue) - expect(permissions(assignee, issue_no_assignee)).to be_allowed(:read_issue) + expect(permissions(assignee, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) expect(permissions(assignee, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue) end @@ -73,37 +73,37 @@ describe IssuePolicy do let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } it 'does not allow non-members to read confidential issues' do - expect(permissions(non_member, confidential_issue)).to be_disallowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(non_member, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(non_member, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(non_member, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'does not allow guests to read confidential issues' do - expect(permissions(guest, confidential_issue)).to be_disallowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(guest, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows reporters to read, update, and admin confidential issues' do - expect(permissions(reporter, confidential_issue)).to be_allowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows reporters from group links to read, update, and admin confidential issues' do - expect(permissions(reporter_from_group_link, confidential_issue)).to be_allowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to be_allowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows issue authors to read and update their confidential issues' do - expect(permissions(author, confidential_issue)).to be_allowed(:read_issue, :update_issue) + expect(permissions(author, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(author, confidential_issue)).to be_disallowed(:admin_issue) - expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows issue assignees to read and update their confidential issues' do - expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :update_issue) + expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue) - expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end end end @@ -123,36 +123,36 @@ describe IssuePolicy do end it 'allows guests to read issues' do - expect(permissions(guest, issue)).to be_allowed(:read_issue) + expect(permissions(guest, issue)).to be_allowed(:read_issue, :read_issue_iid) expect(permissions(guest, issue)).to be_disallowed(:update_issue, :admin_issue) - expect(permissions(guest, issue_no_assignee)).to be_allowed(:read_issue) + expect(permissions(guest, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) expect(permissions(guest, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue) end it 'allows reporters to read, update, and admin issues' do - expect(permissions(reporter, issue)).to be_allowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(reporter, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows reporters from group links to read, update, and admin issues' do - expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows issue authors to read and update their issues' do - expect(permissions(author, issue)).to be_allowed(:read_issue, :update_issue) + expect(permissions(author, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(author, issue)).to be_disallowed(:admin_issue) - expect(permissions(author, issue_no_assignee)).to be_allowed(:read_issue) + expect(permissions(author, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) expect(permissions(author, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue) end it 'allows issue assignees to read and update their issues' do - expect(permissions(assignee, issue)).to be_allowed(:read_issue, :update_issue) + expect(permissions(assignee, issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(assignee, issue)).to be_disallowed(:admin_issue) - expect(permissions(assignee, issue_no_assignee)).to be_allowed(:read_issue) + expect(permissions(assignee, issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid) expect(permissions(assignee, issue_no_assignee)).to be_disallowed(:update_issue, :admin_issue) end @@ -161,32 +161,32 @@ describe IssuePolicy do let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } it 'does not allow guests to read confidential issues' do - expect(permissions(guest, confidential_issue)).to be_disallowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(guest, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(guest, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows reporters to read, update, and admin confidential issues' do - expect(permissions(reporter, confidential_issue)).to be_allowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(reporter, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows reporter from group links to read, update, and admin confidential issues' do - expect(permissions(reporter_from_group_link, confidential_issue)).to be_allowed(:read_issue, :update_issue, :admin_issue) - expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to be_allowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) + expect(permissions(reporter_from_group_link, confidential_issue_no_assignee)).to be_allowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows issue authors to read and update their confidential issues' do - expect(permissions(author, confidential_issue)).to be_allowed(:read_issue, :update_issue) + expect(permissions(author, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(author, confidential_issue)).to be_disallowed(:admin_issue) - expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end it 'allows issue assignees to read and update their confidential issues' do - expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :update_issue) + expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue) - expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :update_issue, :admin_issue) + expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end end end diff --git a/spec/presenters/project_presenter_spec.rb b/spec/presenters/project_presenter_spec.rb new file mode 100644 index 00000000000..f8c93d91ec5 --- /dev/null +++ b/spec/presenters/project_presenter_spec.rb @@ -0,0 +1,397 @@ +require 'spec_helper' + +describe ProjectPresenter do + let(:user) { create(:user) } + + describe '#license_short_name' do + let(:project) { create(:project) } + let(:presenter) { described_class.new(project, current_user: user) } + + context 'when project.repository has a license_key' do + it 'returns the nickname of the license if present' do + allow(project.repository).to receive(:license_key).and_return('agpl-3.0') + + expect(presenter.license_short_name).to eq('GNU AGPLv3') + end + + it 'returns the name of the license if nickname is not present' do + allow(project.repository).to receive(:license_key).and_return('mit') + + expect(presenter.license_short_name).to eq('MIT License') + end + end + + context 'when project.repository has no license_key but a license_blob' do + it 'returns LICENSE' do + allow(project.repository).to receive(:license_key).and_return(nil) + + expect(presenter.license_short_name).to eq('LICENSE') + end + end + end + + describe '#default_view' do + let(:presenter) { described_class.new(project, current_user: user) } + + context 'user not signed in' do + let(:user) { nil } + + context 'when repository is empty' do + let(:project) { create(:project_empty_repo, :public) } + + it 'returns activity if user has repository access' do + allow(presenter).to receive(:can?).with(nil, :download_code, project).and_return(true) + + expect(presenter.default_view).to eq('activity') + end + + it 'returns activity if user does not have repository access' do + allow(project).to receive(:can?).with(nil, :download_code, project).and_return(false) + + expect(presenter.default_view).to eq('activity') + end + end + + context 'when repository is not empty' do + let(:project) { create(:project, :public, :repository) } + + it 'returns files and readme if user has repository access' do + allow(presenter).to receive(:can?).with(nil, :download_code, project).and_return(true) + + expect(presenter.default_view).to eq('files') + end + + it 'returns activity if user does not have repository access' do + allow(presenter).to receive(:can?).with(nil, :download_code, project).and_return(false) + + expect(presenter.default_view).to eq('activity') + end + end + end + + context 'user signed in' do + let(:user) { create(:user, :readme) } + let(:project) { create(:project, :public, :repository) } + + context 'when the user is allowed to see the code' do + it 'returns the project view' do + allow(presenter).to receive(:can?).with(user, :download_code, project).and_return(true) + + expect(presenter.default_view).to eq('readme') + end + end + + context 'with wikis enabled and the right policy for the user' do + before do + project.project_feature.update_attribute(:issues_access_level, 0) + allow(presenter).to receive(:can?).with(user, :download_code, project).and_return(false) + end + + it 'returns wiki if the user has the right policy' do + allow(presenter).to receive(:can?).with(user, :read_wiki, project).and_return(true) + + expect(presenter.default_view).to eq('wiki') + end + + it 'returns customize_workflow if the user does not have the right policy' do + allow(presenter).to receive(:can?).with(user, :read_wiki, project).and_return(false) + + expect(presenter.default_view).to eq('customize_workflow') + end + end + + context 'with issues as a feature available' do + it 'return issues' do + allow(presenter).to receive(:can?).with(user, :download_code, project).and_return(false) + allow(presenter).to receive(:can?).with(user, :read_wiki, project).and_return(false) + + expect(presenter.default_view).to eq('projects/issues/issues') + end + end + + context 'with no activity, no wikies and no issues' do + it 'returns customize_workflow as default' do + project.project_feature.update_attribute(:issues_access_level, 0) + allow(presenter).to receive(:can?).with(user, :download_code, project).and_return(false) + allow(presenter).to receive(:can?).with(user, :read_wiki, project).and_return(false) + + expect(presenter.default_view).to eq('customize_workflow') + end + end + end + end + + describe '#can_current_user_push_code?' do + let(:project) { create(:project, :repository) } + let(:presenter) { described_class.new(project, current_user: user) } + + context 'empty repo' do + let(:project) { create(:project) } + + it 'returns true if user can push_code' do + project.add_developer(user) + + expect(presenter.can_current_user_push_code?).to be(true) + end + + it 'returns false if user cannot push_code' do + project.add_reporter(user) + + expect(presenter.can_current_user_push_code?).to be(false) + end + end + + context 'not empty repo' do + let(:project) { create(:project, :repository) } + + it 'returns true if user can push to default branch' do + project.add_developer(user) + + expect(presenter.can_current_user_push_code?).to be(true) + end + + it 'returns false if default branch is protected' do + project.add_developer(user) + create(:protected_branch, project: project, name: project.default_branch) + + expect(presenter.can_current_user_push_code?).to be(false) + end + end + end + + context 'statistics anchors' do + let(:project) { create(:project, :repository) } + let(:presenter) { described_class.new(project, current_user: user) } + + describe '#files_anchor_data' do + it 'returns files data' do + expect(presenter.files_anchor_data).to eq(OpenStruct.new(enabled: true, + label: 'Files (0 Bytes)', + link: presenter.project_tree_path(project))) + end + end + + describe '#commits_anchor_data' do + it 'returns commits data' do + expect(presenter.commits_anchor_data).to eq(OpenStruct.new(enabled: true, + label: 'Commits (0)', + link: presenter.project_commits_path(project, project.repository.root_ref))) + end + end + + describe '#branches_anchor_data' do + it 'returns branches data' do + expect(presenter.branches_anchor_data).to eq(OpenStruct.new(enabled: true, + label: "Branches (#{project.repository.branches.size})", + link: presenter.project_branches_path(project))) + end + end + + describe '#tags_anchor_data' do + it 'returns tags data' do + expect(presenter.tags_anchor_data).to eq(OpenStruct.new(enabled: true, + label: "Tags (#{project.repository.tags.size})", + link: presenter.project_tags_path(project))) + end + end + + describe '#new_file_anchor_data' do + it 'returns new file data if user can push' do + project.add_developer(user) + + expect(presenter.new_file_anchor_data).to eq(OpenStruct.new(enabled: false, + label: "New file", + link: presenter.project_new_blob_path(project, 'master'), + class_modifier: 'new')) + end + + it 'returns nil if user cannot push' do + expect(presenter.new_file_anchor_data).to be_nil + end + end + + describe '#readme_anchor_data' do + context 'when user can push and README does not exists' do + it 'returns anchor data' do + project.add_developer(user) + allow(project.repository).to receive(:readme).and_return(nil) + + expect(presenter.readme_anchor_data).to eq(OpenStruct.new(enabled: false, + label: 'Add Readme', + link: presenter.add_readme_path)) + end + end + + context 'when README exists' do + it 'returns anchor data' do + allow(project.repository).to receive(:readme).and_return(double(name: 'readme')) + + expect(presenter.readme_anchor_data).to eq(OpenStruct.new(enabled: true, + label: 'Readme', + link: presenter.readme_path)) + end + end + end + + describe '#changelog_anchor_data' do + context 'when user can push and CHANGELOG does not exists' do + it 'returns anchor data' do + project.add_developer(user) + allow(project.repository).to receive(:changelog).and_return(nil) + + expect(presenter.changelog_anchor_data).to eq(OpenStruct.new(enabled: false, + label: 'Add Changelog', + link: presenter.add_changelog_path)) + end + end + + context 'when CHANGELOG exists' do + it 'returns anchor data' do + allow(project.repository).to receive(:changelog).and_return(double(name: 'foo')) + + expect(presenter.changelog_anchor_data).to eq(OpenStruct.new(enabled: true, + label: 'Changelog', + link: presenter.changelog_path)) + end + end + end + + describe '#license_anchor_data' do + context 'when user can push and LICENSE does not exists' do + it 'returns anchor data' do + project.add_developer(user) + allow(project.repository).to receive(:license_blob).and_return(nil) + + expect(presenter.license_anchor_data).to eq(OpenStruct.new(enabled: false, + label: 'Add License', + link: presenter.add_license_path)) + end + end + + context 'when LICENSE exists' do + it 'returns anchor data' do + allow(project.repository).to receive(:license_blob).and_return(double(name: 'foo')) + + expect(presenter.license_anchor_data).to eq(OpenStruct.new(enabled: true, + label: presenter.license_short_name, + link: presenter.license_path)) + end + end + end + + describe '#contribution_guide_anchor_data' do + context 'when user can push and CONTRIBUTING does not exists' do + it 'returns anchor data' do + project.add_developer(user) + allow(project.repository).to receive(:contribution_guide).and_return(nil) + + expect(presenter.contribution_guide_anchor_data).to eq(OpenStruct.new(enabled: false, + label: 'Add Contribution guide', + link: presenter.add_contribution_guide_path)) + end + end + + context 'when CONTRIBUTING exists' do + it 'returns anchor data' do + allow(project.repository).to receive(:contribution_guide).and_return(double(name: 'foo')) + + expect(presenter.contribution_guide_anchor_data).to eq(OpenStruct.new(enabled: true, + label: 'Contribution guide', + link: presenter.contribution_guide_path)) + end + end + end + + describe '#autodevops_anchor_data' do + context 'when Auto Devops is enabled' do + it 'returns anchor data' do + allow(project).to receive(:auto_devops_enabled?).and_return(true) + + expect(presenter.autodevops_anchor_data).to eq(OpenStruct.new(enabled: true, + label: 'Auto DevOps enabled', + link: nil)) + end + end + + context 'when user can admin pipeline and CI yml does not exists' do + it 'returns anchor data' do + project.add_master(user) + allow(project).to receive(:auto_devops_enabled?).and_return(false) + allow(project.repository).to receive(:gitlab_ci_yml).and_return(nil) + + expect(presenter.autodevops_anchor_data).to eq(OpenStruct.new(enabled: false, + label: 'Enable Auto DevOps', + link: presenter.project_settings_ci_cd_path(project, anchor: 'js-general-pipeline-settings'))) + end + end + end + + describe '#kubernetes_cluster_anchor_data' do + context 'when user can create Kubernetes cluster' do + it 'returns link to cluster if only one exists' do + project.add_master(user) + cluster = create(:cluster, projects: [project]) + + expect(presenter.kubernetes_cluster_anchor_data).to eq(OpenStruct.new(enabled: true, + label: 'Kubernetes configured', + link: presenter.project_cluster_path(project, cluster))) + end + + it 'returns link to clusters page if more than one exists' do + project.add_master(user) + create(:cluster, projects: [project]) + create(:cluster, projects: [project]) + + expect(presenter.kubernetes_cluster_anchor_data).to eq(OpenStruct.new(enabled: true, + label: 'Kubernetes configured', + link: presenter.project_clusters_path(project))) + end + + it 'returns link to create a cluster if no cluster exists' do + project.add_master(user) + + expect(presenter.kubernetes_cluster_anchor_data).to eq(OpenStruct.new(enabled: false, + label: 'Add Kubernetes cluster', + link: presenter.new_project_cluster_path(project))) + end + end + + context 'when user cannot create Kubernetes cluster' do + it 'returns nil' do + expect(presenter.kubernetes_cluster_anchor_data).to be_nil + end + end + end + + describe '#koding_anchor_data' do + it 'returns link to setup Koding if user can push and no koding YML exists' do + project.add_developer(user) + allow(project.repository).to receive(:koding_yml).and_return(nil) + allow(Gitlab::CurrentSettings).to receive(:koding_enabled?).and_return(true) + + expect(presenter.koding_anchor_data).to eq(OpenStruct.new(enabled: false, + label: 'Set up Koding', + link: presenter.add_koding_stack_path)) + end + + it 'returns nil if user cannot push' do + expect(presenter.koding_anchor_data).to be_nil + end + + it 'returns nil if koding is not enabled' do + project.add_developer(user) + allow(Gitlab::CurrentSettings).to receive(:koding_enabled?).and_return(false) + + expect(presenter.koding_anchor_data).to be_nil + end + + it 'returns nil if koding YML already exists' do + project.add_developer(user) + allow(project.repository).to receive(:koding_yml).and_return(double) + allow(Gitlab::CurrentSettings).to receive(:koding_enabled?).and_return(true) + + expect(presenter.koding_anchor_data).to be_nil + end + end + end +end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index c6fdda203ad..1b0a5eac9b0 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -597,7 +597,7 @@ describe 'Git HTTP requests' do context "when a gitlab ci token is provided" do let(:project) { create(:project, :repository) } let(:build) { create(:ci_build, :running) } - let(:other_project) { create(:project) } + let(:other_project) { create(:project, :repository) } before do build.update!(project: project) # can't associate it on factory create @@ -648,10 +648,10 @@ describe 'Git HTTP requests' do context 'when the repo does not exist' do let(:project) { create(:project) } - it 'rejects pulls with 403 Forbidden' do + it 'rejects pulls with 404 Not Found' do clone_get path, env - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) expect(response.body).to eq(git_access_error(:no_repo)) end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index de829011e58..6bed8e812c0 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -68,10 +68,10 @@ describe 'OpenID Connect requests' do let!(:public_email) { build :email, email: 'public@example.com' } let!(:private_email) { build :email, email: 'private@example.com' } - let!(:group1) { create :group, path: 'group1' } - let!(:group2) { create :group, path: 'group2' } - let!(:group3) { create :group, path: 'group3', parent: group2 } - let!(:group4) { create :group, path: 'group4', parent: group3 } + let!(:group1) { create :group } + let!(:group2) { create :group } + let!(:group3) { create :group, parent: group2 } + let!(:group4) { create :group, parent: group3 } before do group1.add_user(user, GroupMember::OWNER) @@ -93,8 +93,8 @@ describe 'OpenID Connect requests' do 'groups' => anything })) - expected_groups = %w[group1 group2/group3] - expected_groups << 'group2/group3/group4' if Group.supports_nested_groups? + expected_groups = [group1.full_path, group3.full_path] + expected_groups << group4.full_path if Group.supports_nested_groups? expect(json_response['groups']).to match_array(expected_groups) end end diff --git a/spec/services/ci/create_trace_artifact_service_spec.rb b/spec/services/ci/create_trace_artifact_service_spec.rb index 847a88920fe..8c5e8e438c7 100644 --- a/spec/services/ci/create_trace_artifact_service_spec.rb +++ b/spec/services/ci/create_trace_artifact_service_spec.rb @@ -4,40 +4,60 @@ describe Ci::CreateTraceArtifactService do describe '#execute' do subject { described_class.new(nil, nil).execute(job) } - let(:job) { create(:ci_build) } - context 'when the job does not have trace artifact' do context 'when the job has a trace file' do - before do - allow_any_instance_of(Gitlab::Ci::Trace) - .to receive(:default_path) { expand_fixture_path('trace/sample_trace') } + let!(:job) { create(:ci_build, :trace_live) } + let!(:legacy_path) { job.trace.read { |stream| return stream.path } } + let!(:legacy_checksum) { Digest::SHA256.file(legacy_path).hexdigest } + let(:new_path) { job.job_artifacts_trace.file.path } + let(:new_checksum) { Digest::SHA256.file(new_path).hexdigest } - allow_any_instance_of(JobArtifactUploader).to receive(:move_to_cache) { false } - allow_any_instance_of(JobArtifactUploader).to receive(:move_to_store) { false } - end + it { expect(File.exist?(legacy_path)).to be_truthy } it 'creates trace artifact' do expect { subject }.to change { Ci::JobArtifact.count }.by(1) - expect(job.job_artifacts_trace.read_attribute(:file)).to eq('sample_trace') + expect(File.exist?(legacy_path)).to be_falsy + expect(File.exist?(new_path)).to be_truthy + expect(new_checksum).to eq(legacy_checksum) + expect(job.job_artifacts_trace.file.exists?).to be_truthy + expect(job.job_artifacts_trace.file.filename).to eq('job.log') end - context 'when the job has already had trace artifact' do + context 'when failed to create trace artifact record' do before do - create(:ci_job_artifact, :trace, job: job) + # When ActiveRecord error happens + allow_any_instance_of(Ci::JobArtifact).to receive(:save).and_return(false) + allow_any_instance_of(Ci::JobArtifact).to receive_message_chain(:errors, :full_messages) + .and_return("Error") + + subject rescue nil + + job.reload end - it 'does not create trace artifact' do - expect { subject }.not_to change { Ci::JobArtifact.count } + it 'keeps legacy trace and removes trace artifact' do + expect(File.exist?(legacy_path)).to be_truthy + expect(job.job_artifacts_trace).to be_nil end end end context 'when the job does not have a trace file' do + let!(:job) { create(:ci_build) } + it 'does not create trace artifact' do expect { subject }.not_to change { Ci::JobArtifact.count } end end end + + context 'when the job has already had trace artifact' do + let!(:job) { create(:ci_build, :trace_artifact) } + + it 'does not create trace artifact' do + expect { subject }.not_to change { Ci::JobArtifact.count } + end + end end end diff --git a/spec/services/labels/find_or_create_service_spec.rb b/spec/services/labels/find_or_create_service_spec.rb index 78aa5d442e7..68d5660445a 100644 --- a/spec/services/labels/find_or_create_service_spec.rb +++ b/spec/services/labels/find_or_create_service_spec.rb @@ -15,47 +15,79 @@ describe Labels::FindOrCreateService do context 'when acting on behalf of a specific user' do let(:user) { create(:user) } - subject(:service) { described_class.new(user, project, params) } - before do - project.add_developer(user) - end - context 'when label does not exist at group level' do - it 'creates a new label at project level' do - expect { service.execute }.to change(project.labels, :count).by(1) + context 'when finding labels on project level' do + subject(:service) { described_class.new(user, project, params) } + + before do + project.add_developer(user) end - end - context 'when label exists at group level' do - it 'returns the group label' do - group_label = create(:group_label, group: group, title: 'Security') + context 'when label does not exist at group level' do + it 'creates a new label at project level' do + expect { service.execute }.to change(project.labels, :count).by(1) + end + end - expect(service.execute).to eq group_label + context 'when label exists at group level' do + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') + + expect(service.execute).to eq group_label + end + end + + context 'when label exists at project level' do + it 'returns the project label' do + project_label = create(:label, project: project, title: 'Security') + + expect(service.execute).to eq project_label + end end end - context 'when label does not exist at group level' do - it 'creates a new label at project leve' do - expect { service.execute }.to change(project.labels, :count).by(1) + context 'when finding labels on group level' do + subject(:service) { described_class.new(user, group, params) } + + before do + group.add_developer(user) + end + + context 'when label does not exist at group level' do + it 'creates a new label at group level' do + expect { service.execute }.to change(group.labels, :count).by(1) + end + end + + context 'when label exists at group level' do + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') + + expect(service.execute).to eq group_label + end end end + end + + context 'when authorization is not required' do + context 'when finding labels on project level' do + subject(:service) { described_class.new(nil, project, params) } - context 'when label exists at project level' do it 'returns the project label' do project_label = create(:label, project: project, title: 'Security') - expect(service.execute).to eq project_label + expect(service.execute(skip_authorization: true)).to eq project_label end end - end - context 'when authorization is not required' do - subject(:service) { described_class.new(nil, project, params) } + context 'when finding labels on group level' do + subject(:service) { described_class.new(nil, group, params) } - it 'returns the project label' do - project_label = create(:label, project: project, title: 'Security') + it 'returns the group label' do + group_label = create(:group_label, group: group, title: 'Security') - expect(service.execute(skip_authorization: true)).to eq project_label + expect(service.execute(skip_authorization: true)).to eq group_label + end end end end diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb index 75553afc033..38d84cf0ceb 100644 --- a/spec/services/merge_requests/create_from_issue_service_spec.rb +++ b/spec/services/merge_requests/create_from_issue_service_spec.rb @@ -24,7 +24,7 @@ describe MergeRequests::CreateFromIssueService do end it 'delegates issue search to IssuesFinder' do - expect_any_instance_of(IssuesFinder).to receive(:execute).once.and_call_original + expect_any_instance_of(IssuesFinder).to receive(:find_by).once.and_call_original described_class.new(project, user, issue_iid: -1).execute end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 836ffb7cea0..62fdf870090 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -1678,6 +1678,78 @@ describe NotificationService, :mailer do end end + describe 'Pages domains' do + set(:project) { create(:project) } + set(:domain) { create(:pages_domain, project: project) } + set(:u_blocked) { create(:user, :blocked) } + set(:u_silence) { create_user_with_notification(:disabled, 'silent', project) } + set(:u_owner) { project.owner } + set(:u_master1) { create(:user) } + set(:u_master2) { create(:user) } + set(:u_developer) { create(:user) } + + before do + project.add_master(u_blocked) + project.add_master(u_silence) + project.add_master(u_master1) + project.add_master(u_master2) + project.add_developer(u_developer) + + reset_delivered_emails! + end + + %i[ + pages_domain_enabled + pages_domain_disabled + pages_domain_verification_succeeded + pages_domain_verification_failed + ].each do |sym| + describe "##{sym}" do + subject(:notify!) { notification.send(sym, domain) } + + it 'emails current watching masters' do + expect(Notify).to receive(:"#{sym}_email").at_least(:once).and_call_original + + notify! + + should_only_email(u_master1, u_master2, u_owner) + end + + it 'emails nobody if the project is missing' do + domain.project = nil + + notify! + + should_not_email_anyone + end + end + end + + describe '#pages_domain_verification_failed' do + it 'emails current watching masters' do + notification.pages_domain_verification_failed(domain) + + should_only_email(u_master1, u_master2, u_owner) + end + end + + describe '#pages_domain_enabled' do + it 'emails current watching masters' do + notification.pages_domain_enabled(domain) + + should_only_email(u_master1, u_master2, u_owner) + end + end + + describe '#pages_domain_disabled' do + it 'emails current watching masters' do + notification.pages_domain_disabled(domain) + + should_only_email(u_master1, u_master2, u_owner) + end + end + end + def build_team(project) @u_watcher = create_global_setting_for(create(:user), :watch) @u_participating = create_global_setting_for(create(:user), :participating) diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index ae160d104f1..f793f55e51b 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -522,6 +522,22 @@ describe QuickActions::InterpretService do let(:issuable) { merge_request } end + context 'only group milestones available' do + let(:group) { create(:group) } + let(:project) { create(:project, :public, namespace: group) } + let(:milestone) { create(:milestone, group: group, title: '10.0') } + + it_behaves_like 'milestone command' do + let(:content) { "/milestone %#{milestone.title}" } + let(:issuable) { issue } + end + + it_behaves_like 'milestone command' do + let(:content) { "/milestone %#{milestone.title}" } + let(:issuable) { merge_request } + end + end + it_behaves_like 'remove_milestone command' do let(:content) { '/remove_milestone' } let(:issuable) { issue } diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 5e6c24f5730..562b89e6767 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -943,7 +943,8 @@ describe TodoService do described_class.new.mark_todos_as_done_by_ids(todo, john_doe) - expect_any_instance_of(TodosFinder).not_to receive(:execute) + # Make sure no TodosFinder is inialized to perform counting + expect(TodosFinder).not_to receive(:new) expect(john_doe.todos_done_count).to eq(1) expect(john_doe.todos_pending_count).to eq(1) diff --git a/spec/services/verify_pages_domain_service_spec.rb b/spec/services/verify_pages_domain_service_spec.rb new file mode 100644 index 00000000000..576db1dde2d --- /dev/null +++ b/spec/services/verify_pages_domain_service_spec.rb @@ -0,0 +1,270 @@ +require 'spec_helper' + +describe VerifyPagesDomainService do + using RSpec::Parameterized::TableSyntax + include EmailHelpers + + let(:error_status) { { status: :error, message: "Couldn't verify #{domain.domain}" } } + + subject(:service) { described_class.new(domain) } + + describe '#execute' do + context 'verification code recognition (verified domain)' do + where(:domain_sym, :code_sym) do + :domain | :verification_code + :domain | :keyed_verification_code + + :verification_domain | :verification_code + :verification_domain | :keyed_verification_code + end + + with_them do + set(:domain) { create(:pages_domain) } + + let(:domain_name) { domain.send(domain_sym) } + let(:verification_code) { domain.send(code_sym) } + + it 'verifies and enables the domain' do + stub_resolver(domain_name => ['something else', verification_code]) + + expect(service.execute).to eq(status: :success) + expect(domain).to be_verified + expect(domain).to be_enabled + end + + it 'verifies and enables when the code is contained partway through a TXT record' do + stub_resolver(domain_name => "something #{verification_code} else") + + expect(service.execute).to eq(status: :success) + expect(domain).to be_verified + expect(domain).to be_enabled + end + + it 'does not verify when the code is not present' do + stub_resolver(domain_name => 'something else') + + expect(service.execute).to eq(error_status) + + expect(domain).not_to be_verified + expect(domain).to be_enabled + end + end + + context 'verified domain' do + set(:domain) { create(:pages_domain) } + + it 'unverifies (but does not disable) when the right code is not present' do + stub_resolver(domain.domain => 'something else') + + expect(service.execute).to eq(error_status) + expect(domain).not_to be_verified + expect(domain).to be_enabled + end + + it 'unverifies (but does not disable) when no records are present' do + stub_resolver + + expect(service.execute).to eq(error_status) + expect(domain).not_to be_verified + expect(domain).to be_enabled + end + end + + context 'expired domain' do + set(:domain) { create(:pages_domain, :expired) } + + it 'verifies and enables when the right code is present' do + stub_resolver(domain.domain => domain.keyed_verification_code) + + expect(service.execute).to eq(status: :success) + + expect(domain).to be_verified + expect(domain).to be_enabled + end + + it 'disables when the right code is not present' do + error_status[:message] += '. It is now disabled.' + + stub_resolver + + expect(service.execute).to eq(error_status) + + expect(domain).not_to be_verified + expect(domain).not_to be_enabled + end + end + end + + context 'timeout behaviour' do + let(:domain) { create(:pages_domain) } + + it 'sets a timeout on the DNS query' do + expect(stub_resolver).to receive(:timeouts=).with(described_class::RESOLVER_TIMEOUT_SECONDS) + + service.execute + end + end + + context 'email notifications' do + let(:notification_service) { instance_double('NotificationService') } + + where(:factory, :verification_succeeds, :expected_notification) do + nil | true | nil + nil | false | :verification_failed + :reverify | true | nil + :reverify | false | :verification_failed + :unverified | true | :verification_succeeded + :unverified | false | nil + :expired | true | nil + :expired | false | :disabled + :disabled | true | :enabled + :disabled | false | nil + end + + with_them do + let(:domain) { create(:pages_domain, *[factory].compact) } + + before do + allow(service).to receive(:notification_service) { notification_service } + + if verification_succeeds + stub_resolver(domain.domain => domain.verification_code) + else + stub_resolver + end + end + + it 'sends a notification if appropriate' do + if expected_notification + expect(notification_service).to receive(:"pages_domain_#{expected_notification}").with(domain) + end + + service.execute + end + end + + context 'pages verification disabled' do + let(:domain) { create(:pages_domain, :disabled) } + + before do + stub_application_setting(pages_domain_verification_enabled: false) + allow(service).to receive(:notification_service) { notification_service } + end + + it 'skips email notifications' do + expect(notification_service).not_to receive(:pages_domain_enabled) + + service.execute + end + end + end + + context 'pages configuration updates' do + context 'enabling a disabled domain' do + let(:domain) { create(:pages_domain, :disabled) } + + it 'schedules an update' do + stub_resolver(domain.domain => domain.verification_code) + + expect(domain).to receive(:update_daemon) + + service.execute + end + end + + context 'verifying an enabled domain' do + let(:domain) { create(:pages_domain) } + + it 'schedules an update' do + stub_resolver(domain.domain => domain.verification_code) + + expect(domain).not_to receive(:update_daemon) + + service.execute + end + end + + context 'disabling an expired domain' do + let(:domain) { create(:pages_domain, :expired) } + + it 'schedules an update' do + stub_resolver + + expect(domain).to receive(:update_daemon) + + service.execute + end + end + + context 'failing to verify a disabled domain' do + let(:domain) { create(:pages_domain, :disabled) } + + it 'does not schedule an update' do + stub_resolver + + expect(domain).not_to receive(:update_daemon) + + service.execute + end + end + end + + context 'no verification code' do + let(:domain) { create(:pages_domain) } + + it 'returns an error' do + domain.verification_code = '' + + disallow_resolver! + + expect(service.execute).to eq(status: :error, message: "No verification code set for #{domain.domain}") + end + end + + context 'pages domain verification is disabled' do + let(:domain) { create(:pages_domain, :disabled) } + + before do + stub_application_setting(pages_domain_verification_enabled: false) + end + + it 'extends domain validity by unconditionally reverifying' do + disallow_resolver! + + service.execute + + expect(domain).to be_verified + expect(domain).to be_enabled + end + + it 'does not shorten any grace period' do + grace = Time.now + 1.year + domain.update!(enabled_until: grace) + disallow_resolver! + + service.execute + + expect(domain.enabled_until).to be_like_time(grace) + end + end + end + + def disallow_resolver! + expect(Resolv::DNS).not_to receive(:open) + end + + def stub_resolver(stubbed_lookups = {}) + resolver = instance_double('Resolv::DNS') + allow(resolver).to receive(:timeouts=) + + expect(Resolv::DNS).to receive(:open).and_yield(resolver) + + allow(resolver).to receive(:getresources) { [] } + stubbed_lookups.each do |domain, records| + records = Array(records).map { |txt| Resolv::DNS::Resource::IN::TXT.new(txt) } + allow(resolver).to receive(:getresources).with(domain, Resolv::DNS::Resource::IN::TXT) { records } + end + + resolver + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5600c9c6ad5..c0f3366fb52 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -185,6 +185,14 @@ RSpec.configure do |config| config.around(:each, :postgresql) do |example| example.run if Gitlab::Database.postgresql? end + + # This makes sure the `ApplicationController#can?` method is stubbed with the + # original implementation for all view specs. + config.before(:each, type: :view) do + allow(view).to receive(:can?) do |*args| + Ability.allowed?(*args) + end + end end # add simpler way to match asset paths containing digest strings diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 5189c57b7db..8603b7f3e2c 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -78,8 +78,10 @@ RSpec.configure do |config| end config.after(:example, :js) do |example| - # prevent localstorage from introducing side effects based on test order - execute_script("localStorage.clear();") + # prevent localStorage from introducing side effects based on test order + unless ['', 'about:blank', 'data:,'].include? Capybara.current_session.driver.browser.current_url + execute_script("localStorage.clear();") + end # capybara/rspec already calls Capybara.reset_sessions! in an `after` hook, # but `block_and_wait_for_requests_complete` is called before it so by diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 1809ae1d141..5edc5de2a09 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -28,11 +28,11 @@ RSpec.configure do |config| end config.before(:each, :js) do - DatabaseCleaner.strategy = :deletion + DatabaseCleaner.strategy = :deletion, { cache_tables: false } end config.before(:each, :delete) do - DatabaseCleaner.strategy = :deletion + DatabaseCleaner.strategy = :deletion, { cache_tables: false } end config.before(:each, :migration) do diff --git a/spec/support/prometheus/additional_metrics_shared_examples.rb b/spec/support/prometheus/additional_metrics_shared_examples.rb index dbbd4ad4d40..c7c3346d39e 100644 --- a/spec/support/prometheus/additional_metrics_shared_examples.rb +++ b/spec/support/prometheus/additional_metrics_shared_examples.rb @@ -12,11 +12,12 @@ RSpec.shared_examples 'additional metrics query' do let(:client) { double('prometheus_client') } let(:query_result) { described_class.new(client).query(*query_params) } - let(:environment) { create(:environment, slug: 'environment-slug') } + let(:project) { create(:project) } + let(:environment) { create(:environment, slug: 'environment-slug', project: project) } before do allow(client).to receive(:label_values).and_return(metric_names) - allow(metric_group_class).to receive(:all).and_return([simple_metric_group(metrics: [simple_metric])]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group(metrics: [simple_metric])]) end context 'metrics query context' do @@ -24,13 +25,14 @@ RSpec.shared_examples 'additional metrics query' do shared_examples 'query context containing environment slug and filter' do it 'contains ci_environment_slug' do - expect(subject).to receive(:query_metrics).with(hash_including(ci_environment_slug: environment.slug)) + expect(subject).to receive(:query_metrics).with(project, hash_including(ci_environment_slug: environment.slug)) subject.query(*query_params) end it 'contains environment filter' do expect(subject).to receive(:query_metrics).with( + project, hash_including( environment_filter: "container_name!=\"POD\",environment=\"#{environment.slug}\"" ) @@ -48,7 +50,7 @@ RSpec.shared_examples 'additional metrics query' do it_behaves_like 'query context containing environment slug and filter' it 'query context contains kube_namespace' do - expect(subject).to receive(:query_metrics).with(hash_including(kube_namespace: kube_namespace)) + expect(subject).to receive(:query_metrics).with(project, hash_including(kube_namespace: kube_namespace)) subject.query(*query_params) end @@ -72,7 +74,7 @@ RSpec.shared_examples 'additional metrics query' do it_behaves_like 'query context containing environment slug and filter' it 'query context contains empty kube_namespace' do - expect(subject).to receive(:query_metrics).with(hash_including(kube_namespace: '')) + expect(subject).to receive(:query_metrics).with(project, hash_including(kube_namespace: '')) subject.query(*query_params) end @@ -81,7 +83,7 @@ RSpec.shared_examples 'additional metrics query' do context 'with one group where two metrics is found' do before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group]) end context 'some queries return results' do @@ -117,7 +119,7 @@ RSpec.shared_examples 'additional metrics query' do let(:metrics) { [simple_metric(queries: [simple_query])] } before do - allow(metric_group_class).to receive(:all).and_return( + allow(metric_group_class).to receive(:common_metrics).and_return( [ simple_metric_group(name: 'group_a', metrics: [simple_metric(queries: [simple_query])]), simple_metric_group(name: 'group_b', metrics: [simple_metric(title: 'title_b', queries: [simple_query('b')])]) diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb index 7ce80c82439..ea7dbade171 100644 --- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb +++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb @@ -89,6 +89,19 @@ shared_examples 'handle uploads' do end end + context "when neither the uploader nor the model exists" do + before do + allow_any_instance_of(Upload).to receive(:build_uploader).and_return(nil) + allow(controller).to receive(:find_model).and_return(nil) + end + + it "responds with status 404" do + show_upload + + expect(response).to have_gitlab_http_status(404) + end + end + context "when the file doesn't exist" do before do allow_any_instance_of(FileUploader).to receive(:exists?).and_return(false) diff --git a/spec/support/snippet_visibility.rb b/spec/support/snippet_visibility.rb index 1cb904823d2..3a7c69b7877 100644 --- a/spec/support/snippet_visibility.rb +++ b/spec/support/snippet_visibility.rb @@ -252,6 +252,15 @@ RSpec.shared_examples 'snippet visibility' do results = described_class.new(user).execute expect(results.include?(snippet)).to eq(outcome) end + + it 'returns no snippets when the user cannot read cross project' do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + + snippets = described_class.new(user).execute + + expect(snippets).to be_empty + end end end end @@ -298,6 +307,15 @@ RSpec.shared_examples 'snippet visibility' do results = described_class.new(user).execute expect(results.include?(snippet)).to eq(outcome) end + + it 'should return personal snippets when the user cannot read cross project' do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + + results = described_class.new(user).execute + + expect(results.include?(snippet)).to eq(outcome) + end end end end diff --git a/spec/views/shared/projects/_project.html.haml_spec.rb b/spec/views/shared/projects/_project.html.haml_spec.rb index f0a4f153699..3b14045e61f 100644 --- a/spec/views/shared/projects/_project.html.haml_spec.rb +++ b/spec/views/shared/projects/_project.html.haml_spec.rb @@ -5,6 +5,7 @@ describe 'shared/projects/_project.html.haml' do before do allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) + allow(view).to receive(:can?) { true } end it 'should render creator avatar if project has a creator' do diff --git a/spec/workers/authorized_projects_worker_spec.rb b/spec/workers/authorized_projects_worker_spec.rb index 0d6eb536c33..d095138f6b7 100644 --- a/spec/workers/authorized_projects_worker_spec.rb +++ b/spec/workers/authorized_projects_worker_spec.rb @@ -1,79 +1,6 @@ require 'spec_helper' describe AuthorizedProjectsWorker do - let(:project) { create(:project) } - - def build_args_list(*ids, multiply: 1) - args_list = ids.map { |id| [id] } - args_list * multiply - end - - describe '.bulk_perform_and_wait' do - it 'schedules the ids and waits for the jobs to complete' do - args_list = build_args_list(project.owner.id) - - project.owner.project_authorizations.delete_all - described_class.bulk_perform_and_wait(args_list) - - expect(project.owner.project_authorizations.count).to eq(1) - end - - it 'inlines workloads <= 3 jobs' do - args_list = build_args_list(project.owner.id, multiply: 3) - expect(described_class).to receive(:bulk_perform_inline).with(args_list) - - described_class.bulk_perform_and_wait(args_list) - end - - it 'runs > 3 jobs using sidekiq' do - project.owner.project_authorizations.delete_all - - expect(described_class).to receive(:bulk_perform_async).and_call_original - - args_list = build_args_list(project.owner.id, multiply: 4) - described_class.bulk_perform_and_wait(args_list) - - expect(project.owner.project_authorizations.count).to eq(1) - end - end - - describe '.bulk_perform_inline' do - it 'refreshes the authorizations inline' do - project.owner.project_authorizations.delete_all - - expect_any_instance_of(described_class).to receive(:perform).and_call_original - - described_class.bulk_perform_inline(build_args_list(project.owner.id)) - - expect(project.owner.project_authorizations.count).to eq(1) - end - - it 'enqueues jobs if an error is raised' do - invalid_id = -1 - args_list = build_args_list(project.owner.id, invalid_id) - - allow_any_instance_of(described_class).to receive(:perform).with(project.owner.id) - allow_any_instance_of(described_class).to receive(:perform).with(invalid_id).and_raise(ArgumentError) - expect(described_class).to receive(:bulk_perform_async).with(build_args_list(invalid_id)) - - described_class.bulk_perform_inline(args_list) - end - end - - describe '.bulk_perform_async' do - it "uses it's respective sidekiq queue" do - args_list = build_args_list(project.owner.id) - push_bulk_args = { - 'class' => described_class, - 'args' => args_list - } - - expect(Sidekiq::Client).to receive(:push_bulk).with(push_bulk_args).once - - described_class.bulk_perform_async(args_list) - end - end - describe '#perform' do let(:user) { create(:user) } @@ -85,12 +12,6 @@ describe AuthorizedProjectsWorker do job.perform(user.id) end - it 'notifies the JobWaiter when done if the key is provided' do - expect(Gitlab::JobWaiter).to receive(:notify).with('notify-key', job.jid) - - job.perform(user.id, 'notify-key') - end - context "when the user is not found" do it "does nothing" do expect_any_instance_of(User).not_to receive(:refresh_authorized_projects) diff --git a/spec/workers/concerns/waitable_worker_spec.rb b/spec/workers/concerns/waitable_worker_spec.rb new file mode 100644 index 00000000000..4af0de86ac9 --- /dev/null +++ b/spec/workers/concerns/waitable_worker_spec.rb @@ -0,0 +1,92 @@ +require 'spec_helper' + +describe WaitableWorker do + let(:worker) do + Class.new do + def self.name + 'Gitlab::Foo::Bar::DummyWorker' + end + + class << self + cattr_accessor(:counter) { 0 } + end + + include ApplicationWorker + prepend WaitableWorker + + def perform(i = 0) + self.class.counter += i + end + end + end + + subject(:job) { worker.new } + + describe '.bulk_perform_and_wait' do + it 'schedules the jobs and waits for them to complete' do + worker.bulk_perform_and_wait([[1], [2]]) + + expect(worker.counter).to eq(3) + end + + it 'inlines workloads <= 3 jobs' do + args_list = [[1], [2], [3]] + expect(worker).to receive(:bulk_perform_inline).with(args_list).and_call_original + + worker.bulk_perform_and_wait(args_list) + + expect(worker.counter).to eq(6) + end + + it 'runs > 3 jobs using sidekiq' do + expect(worker).to receive(:bulk_perform_async) + + worker.bulk_perform_and_wait([[1], [2], [3], [4]]) + end + end + + describe '.bulk_perform_inline' do + it 'runs the jobs inline' do + expect(worker).not_to receive(:bulk_perform_async) + + worker.bulk_perform_inline([[1], [2]]) + + expect(worker.counter).to eq(3) + end + + it 'enqueues jobs if an error is raised' do + expect(worker).to receive(:bulk_perform_async).with([['foo']]) + + worker.bulk_perform_inline([[1], ['foo']]) + end + end + + describe '#perform' do + shared_examples 'perform' do + it 'notifies the JobWaiter when done if the key is provided' do + key = Gitlab::JobWaiter.new.key + expect(Gitlab::JobWaiter).to receive(:notify).with(key, job.jid) + + job.perform(*args, key) + end + + it 'does not notify the JobWaiter when done if no key is provided' do + expect(Gitlab::JobWaiter).not_to receive(:notify) + + job.perform(*args) + end + end + + context 'when the worker takes arguments' do + let(:args) { [1] } + + it_behaves_like 'perform' + end + + context 'when the worker takes no arguments' do + let(:args) { [] } + + it_behaves_like 'perform' + end + end +end diff --git a/spec/workers/pages_domain_verification_cron_worker_spec.rb b/spec/workers/pages_domain_verification_cron_worker_spec.rb new file mode 100644 index 00000000000..8f780428c82 --- /dev/null +++ b/spec/workers/pages_domain_verification_cron_worker_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe PagesDomainVerificationCronWorker do + subject(:worker) { described_class.new } + + describe '#perform' do + it 'enqueues a PagesDomainVerificationWorker for domains needing verification' do + verified = create(:pages_domain) + reverify = create(:pages_domain, :reverify) + disabled = create(:pages_domain, :disabled) + + [reverify, disabled].each do |domain| + expect(PagesDomainVerificationWorker).to receive(:perform_async).with(domain.id) + end + + expect(PagesDomainVerificationWorker).not_to receive(:perform_async).with(verified.id) + + worker.perform + end + end +end diff --git a/spec/workers/pages_domain_verification_worker_spec.rb b/spec/workers/pages_domain_verification_worker_spec.rb new file mode 100644 index 00000000000..372fc95ab4a --- /dev/null +++ b/spec/workers/pages_domain_verification_worker_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe PagesDomainVerificationWorker do + subject(:worker) { described_class.new } + + let(:domain) { create(:pages_domain) } + + describe '#perform' do + it 'does nothing for a non-existent domain' do + domain.destroy + + expect(VerifyPagesDomainService).not_to receive(:new) + + expect { worker.perform(domain.id) }.not_to raise_error + end + + it 'delegates to VerifyPagesDomainService' do + service = double(:service) + expected_domain = satisfy { |obj| obj == domain } + + expect(VerifyPagesDomainService).to receive(:new).with(expected_domain) { service } + expect(service).to receive(:execute) + + worker.perform(domain.id) + end + end +end diff --git a/spec/workers/stuck_import_jobs_worker_spec.rb b/spec/workers/stuck_import_jobs_worker_spec.rb index a82eb54ffe4..069514552b1 100644 --- a/spec/workers/stuck_import_jobs_worker_spec.rb +++ b/spec/workers/stuck_import_jobs_worker_spec.rb @@ -2,35 +2,59 @@ require 'spec_helper' describe StuckImportJobsWorker do let(:worker) { described_class.new } - let(:exclusive_lease_uuid) { SecureRandom.uuid } - before do - allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(exclusive_lease_uuid) - end + shared_examples 'project import job detection' do + context 'when the job has completed' do + context 'when the import status was already updated' do + before do + allow(Gitlab::SidekiqStatus).to receive(:completed_jids) do + project.import_start + project.import_finish - describe 'with started import_status' do - let(:project) { create(:project, :import_started, import_jid: '123') } + [project.import_jid] + end + end + + it 'does not mark the project as failed' do + worker.perform + + expect(project.reload.import_status).to eq('finished') + end + end + + context 'when the import status was not updated' do + before do + allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([project.import_jid]) + end - describe 'long running import' do - it 'marks the project as failed' do - allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return(['123']) + it 'marks the project as failed' do + worker.perform - expect { worker.perform }.to change { project.reload.import_status }.to('failed') + expect(project.reload.import_status).to eq('failed') + end end end - describe 'running import' do - it 'does not mark the project as failed' do + context 'when the job is still in Sidekiq' do + before do allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([]) + end + it 'does not mark the project as failed' do expect { worker.perform }.not_to change { project.reload.import_status } end + end + end - describe 'import without import_jid' do - it 'marks the project as failed' do - expect { worker.perform }.to change { project.reload.import_status }.to('failed') - end - end + describe 'with scheduled import_status' do + it_behaves_like 'project import job detection' do + let(:project) { create(:project, :import_scheduled, import_jid: '123') } + end + end + + describe 'with started import_status' do + it_behaves_like 'project import job detection' do + let(:project) { create(:project, :import_started, import_jid: '123') } end end end |