From 8ffe586b778b905f57382485140efea4c3dcad93 Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Mon, 23 Jan 2017 16:19:39 -0600 Subject: [PATCH] Changed the controller/route name to 'ci/cd' and renamed the corresponding files Added tests to verify the access policy to the new controller --- .../projects/pipelines_settings_controller.rb | 4 ++-- app/controllers/projects/runners_controller.rb | 6 +----- ...pipelines_controller.rb => ci_cd_controller.rb} | 9 ++++++--- app/controllers/projects/triggers_controller.rb | 8 ++------ app/controllers/projects/variables_controller.rb | 10 +++------- app/helpers/gitlab_routing_helper.rb | 2 +- app/views/layouts/nav/_project_settings.html.haml | 4 ++-- .../{ci_cd_pipelines => ci_cd}/show.html.haml | 0 config/routes/project.rb | 2 +- spec/features/runners_spec.rb | 2 +- .../security/project/internal_access_spec.rb | 14 ++++++++++++++ .../security/project/private_access_spec.rb | 12 ++++++------ .../security/project/public_access_spec.rb | 14 ++++++++++++++ 13 files changed, 53 insertions(+), 34 deletions(-) rename app/controllers/projects/settings/{ci_cd_pipelines_controller.rb => ci_cd_controller.rb} (86%) rename app/views/projects/settings/{ci_cd_pipelines => ci_cd}/show.html.haml (100%) diff --git a/app/controllers/projects/pipelines_settings_controller.rb b/app/controllers/projects/pipelines_settings_controller.rb index 3e5ad6cebb0..c8c80551ac9 100644 --- a/app/controllers/projects/pipelines_settings_controller.rb +++ b/app/controllers/projects/pipelines_settings_controller.rb @@ -2,13 +2,13 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController before_action :authorize_admin_pipeline! def show - redirect_to namespace_project_settings_ci_cd_pipelines_path(@project.namespace, @project, params: params) + redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project, params: params) end def update if @project.update_attributes(update_params) flash[:notice] = "CI/CD Pipelines settings for '#{@project.name}' were successfully updated." - redirect_to namespace_project_settings_ci_cd_pipelines_path(@project.namespace, @project) + redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project) else render 'show' end diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb index 97009135fa1..9aa61a18f23 100644 --- a/app/controllers/projects/runners_controller.rb +++ b/app/controllers/projects/runners_controller.rb @@ -4,10 +4,6 @@ class Projects::RunnersController < Projects::ApplicationController layout 'project_settings' - def index - redirect_to namespace_project_settings_ci_cd_pipelines_path(@project.namespace, @project) - end - def edit end @@ -49,7 +45,7 @@ class Projects::RunnersController < Projects::ApplicationController def toggle_shared_runners project.toggle!(:shared_runners_enabled) - redirect_to namespace_project_settings_ci_cd_pipelines_path(@project.namespace, @project) + redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project) end protected diff --git a/app/controllers/projects/settings/ci_cd_pipelines_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb similarity index 86% rename from app/controllers/projects/settings/ci_cd_pipelines_controller.rb rename to app/controllers/projects/settings/ci_cd_controller.rb index f9707d2df41..8d33238aebd 100644 --- a/app/controllers/projects/settings/ci_cd_pipelines_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -1,12 +1,11 @@ module Projects module Settings - class CiCdPipelinesController < Projects::ApplicationController + class CiCdController < Projects::ApplicationController before_action :authorize_admin_pipeline! def show define_runners_variables - # variables - @variable = Ci::Variable.new + define_project_variables_variables define_triggers_variables define_badges_variables end @@ -21,6 +20,10 @@ module Projects @shared_runners_count = @shared_runners.count(:all) end + def define_project_variables_variables + @variable = Ci::Variable.new + end + def define_triggers_variables @triggers = @project.triggers @trigger = Ci::Trigger.new diff --git a/app/controllers/projects/triggers_controller.rb b/app/controllers/projects/triggers_controller.rb index 06e9793fba5..250b5becb84 100644 --- a/app/controllers/projects/triggers_controller.rb +++ b/app/controllers/projects/triggers_controller.rb @@ -3,16 +3,12 @@ class Projects::TriggersController < Projects::ApplicationController layout 'project_settings' - def index - redirect_to namespace_project_settings_ci_cd_pipelines_path(@project.namespace, @project) - end - def create @trigger = project.triggers.new @trigger.save if @trigger.valid? - redirect_to namespace_project_settings_ci_cd_pipelines_path(@project.namespace, @project) + redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project) else @triggers = project.triggers.select(&:persisted?) render :index @@ -22,7 +18,7 @@ class Projects::TriggersController < Projects::ApplicationController def destroy trigger.destroy - redirect_to namespace_project_settings_ci_cd_pipelines_path(@project.namespace, @project) + redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project) end private diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index eeb5b607be5..dd5f6a771d2 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -3,10 +3,6 @@ class Projects::VariablesController < Projects::ApplicationController layout 'project_settings' - def index - redirect_to namespace_project_settings_ci_cd_pipelines_path(project.namespace, project) - end - def show @variable = @project.variables.find(params[:id]) end @@ -15,7 +11,7 @@ class Projects::VariablesController < Projects::ApplicationController @variable = @project.variables.find(params[:id]) if @variable.update_attributes(project_params) - redirect_to namespace_project_settings_ci_cd_pipelines_path(project.namespace, project), notice: 'Variable was successfully updated.' + redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), notice: 'Variable was successfully updated.' else render action: "show" end @@ -25,7 +21,7 @@ class Projects::VariablesController < Projects::ApplicationController @variable = Ci::Variable.new(project_params) if @variable.valid? && @project.variables << @variable - redirect_to namespace_project_settings_ci_cd_pipelines_path(project.namespace, project), notice: 'Variables were successfully updated.' + redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), notice: 'Variables were successfully updated.' else render action: "index" end @@ -35,7 +31,7 @@ class Projects::VariablesController < Projects::ApplicationController @key = @project.variables.find(params[:id]) @key.destroy - redirect_to namespace_project_settings_ci_cd_pipelines_path(project.namespace, project), notice: 'Variable was successfully removed.' + redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), notice: 'Variable was successfully removed.' end private diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index 2159e4ce21a..d00c8865e0b 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -211,7 +211,7 @@ module GitlabRoutingHelper def project_settings_integrations_path(project, *args) namespace_project_settings_integrations_path(project.namespace, project, *args) end - + def project_settings_members_path(project, *args) namespace_project_settings_members_path(project.namespace, project, *args) end diff --git a/app/views/layouts/nav/_project_settings.html.haml b/app/views/layouts/nav/_project_settings.html.haml index bd170d5ca32..665725f6862 100644 --- a/app/views/layouts/nav/_project_settings.html.haml +++ b/app/views/layouts/nav/_project_settings.html.haml @@ -18,8 +18,8 @@ Protected Branches - if @project.feature_available?(:builds, current_user) - = nav_link(controller: :ci_cd_pipelines_settings) do - = link_to namespace_project_settings_ci_cd_pipelines_path(@project.namespace, @project), title: 'CI/CD Pipelines' do + = nav_link(controller: :ci_cd) do + = link_to namespace_project_settings_ci_cd_path(@project.namespace, @project), title: 'CI/CD Pipelines' do %span CI/CD Pipelines = nav_link(controller: :pages) do diff --git a/app/views/projects/settings/ci_cd_pipelines/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml similarity index 100% rename from app/views/projects/settings/ci_cd_pipelines/show.html.haml rename to app/views/projects/settings/ci_cd/show.html.haml diff --git a/config/routes/project.rb b/config/routes/project.rb index c028af30263..2ac98cf3842 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -315,7 +315,7 @@ constraints(ProjectUrlConstrainer.new) do end namespace :settings do resource :members, only: [:show] - resource :ci_cd_pipelines, only: [:show] + resource :ci_cd, only: [:show], controller: 'ci_cd' resource :integrations, only: [:show] end diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index dcc3bbdec61..0e1cc9a0f73 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -122,7 +122,7 @@ describe "Runners" do scenario 'user checks default configuration' do visit namespace_project_runner_path(project.namespace, project, runner) - + expect(page).to have_content 'Can run untagged jobs Yes' end diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 92d5a2fbc48..24af062d763 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -96,6 +96,20 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_denied_for(:external) } end + describe "GET /:project_path/settings/ci_cd" do + subject { namespace_project_settings_ci_cd_path(project.namespace, project) } + + it { is_expected.to be_allowed_for(:admin) } + it { is_expected.to be_allowed_for(:owner).of(project) } + it { is_expected.to be_allowed_for(:master).of(project) } + it { is_expected.to be_denied_for(:developer).of(project) } + it { is_expected.to be_denied_for(:reporter).of(project) } + it { is_expected.to be_denied_for(:guest).of(project) } + it { is_expected.to be_denied_for(:user) } + it { is_expected.to be_denied_for(:visitor) } + it { is_expected.to be_denied_for(:external) } + end + describe "GET /:project_path/blob" do let(:commit) { project.repository.commit } subject { namespace_project_blob_path(project.namespace, project, File.join(commit.id, '.gitignore')) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index b616e488487..1e0f6fa2b48 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -82,18 +82,18 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:visitor) } end - describe "GET /:project_path/settings/members" do - subject { namespace_project_settings_members_path(project.namespace, project) } + describe "GET /:project_path/settings/ci_cd" do + subject { namespace_project_settings_ci_cd_path(project.namespace, project) } it { is_expected.to be_allowed_for(:admin) } it { is_expected.to be_allowed_for(:owner).of(project) } it { is_expected.to be_allowed_for(:master).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_allowed_for(:guest).of(project) } + it { is_expected.to be_denied_for(:developer).of(project) } + it { is_expected.to be_denied_for(:reporter).of(project) } + it { is_expected.to be_denied_for(:guest).of(project) } it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } + it { is_expected.to be_denied_for(:external) } end describe "GET /:project_path/blob" do diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index ded85e837f4..d8cc012c27e 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -96,6 +96,20 @@ describe "Public Project Access", feature: true do it { is_expected.to be_allowed_for(:external) } end + describe "GET /:project_path/settings/ci_cd" do + subject { namespace_project_settings_ci_cd_path(project.namespace, project) } + + it { is_expected.to be_allowed_for(:admin) } + it { is_expected.to be_allowed_for(:owner).of(project) } + it { is_expected.to be_allowed_for(:master).of(project) } + it { is_expected.to be_denied_for(:developer).of(project) } + it { is_expected.to be_denied_for(:reporter).of(project) } + it { is_expected.to be_denied_for(:guest).of(project) } + it { is_expected.to be_denied_for(:user) } + it { is_expected.to be_denied_for(:visitor) } + it { is_expected.to be_denied_for(:external) } + end + describe "GET /:project_path/pipelines" do subject { namespace_project_pipelines_path(project.namespace, project) }