Merge branch...

Merge branch '26165-combine-runners-and-variables-and-triggers-and-ci-cd-pipelines-settings-pages' into 'master'

Merge Triggers, Runners, Variables and Pipelines to a single option called "CI/CD Pipelines"

Closes #26165

See merge request !8430

app/assets/javascripts/dispatcher.js.es6

@@ -254,7 +254,7 @@
           new gl.ProtectedBranchCreate();
           new gl.ProtectedBranchEditList();
           break;
-        case 'projects:variables:index':
+        case 'projects:ci_cd:show':
           new gl.ProjectVariables();
           break;
         case 'ci:lints:create':

app/controllers/projects/pipelines_settings_controller.rb

@@ -2,20 +2,13 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController
   before_action :authorize_admin_pipeline!
 
   def show
-    @ref = params[:ref] || @project.default_branch || 'master'
-
-    @badges = [Gitlab::Badge::Build::Status,
-               Gitlab::Badge::Coverage::Report]
-
-    @badges.map! do |badge|
-      badge.new(@project, @ref).metadata
-    end
+    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_pipelines_settings_path(@project.namespace, @project)
+      redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project)
     else
       render 'show'
     end

app/controllers/projects/runners_controller.rb

@@ -5,11 +5,7 @@ class Projects::RunnersController < Projects::ApplicationController
   layout 'project_settings'
 
   def index
-    @project_runners = project.runners.ordered
-    @assignable_runners = current_user.ci_authorized_runners.
-      assignable_for(project).ordered.page(params[:page]).per(20)
-    @shared_runners = Ci::Runner.shared.active
-    @shared_runners_count = @shared_runners.count(:all)
+    redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project)
   end
 
   def edit
@@ -53,7 +49,7 @@ class Projects::RunnersController < Projects::ApplicationController
   def toggle_shared_runners
     project.toggle!(:shared_runners_enabled)
 
-    redirect_to namespace_project_runners_path(project.namespace, project)
+    redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project)
   end
 
   protected

app/controllers/projects/settings/ci_cd_controller.rb

+module Projects
+  module Settings
+    class CiCdController < Projects::ApplicationController
+      before_action :authorize_admin_pipeline!
+
+      def show
+        define_runners_variables
+        define_secret_variables
+        define_triggers_variables
+        define_badges_variables
+      end
+
+      private
+
+      def define_runners_variables
+        @project_runners = @project.runners.ordered
+        @assignable_runners = current_user.ci_authorized_runners.
+          assignable_for(project).ordered.page(params[:page]).per(20)
+        @shared_runners = Ci::Runner.shared.active
+        @shared_runners_count = @shared_runners.count(:all)
+      end
+
+      def define_secret_variables
+        @variable = Ci::Variable.new
+      end
+
+      def define_triggers_variables
+        @triggers = @project.triggers
+        @trigger = Ci::Trigger.new
+      end
+
+      def define_badges_variables
+        @ref = params[:ref] || @project.default_branch || 'master'
+
+        @badges = [Gitlab::Badge::Build::Status,
+                   Gitlab::Badge::Coverage::Report]
+
+        @badges.map! do |badge|
+          badge.new(@project, @ref).metadata
+        end
+      end
+    end
+  end
+end

app/controllers/projects/triggers_controller.rb

@@ -4,8 +4,7 @@ class Projects::TriggersController < Projects::ApplicationController
   layout 'project_settings'
 
   def index
-    @triggers = project.triggers
-    @trigger = Ci::Trigger.new
+    redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project)
   end
 
   def create
@@ -13,17 +12,18 @@ class Projects::TriggersController < Projects::ApplicationController
     @trigger.save
 
     if @trigger.valid?
-      redirect_to namespace_project_triggers_path(@project.namespace, @project)
+      redirect_to namespace_project_variables_path(project.namespace, project), notice: 'Trigger was created successfully.'
     else
       @triggers = project.triggers.select(&:persisted?)
-      render :index
+      render action: "show"
     end
   end
 
   def destroy
     trigger.destroy
+    flash[:alert] = "Trigger removed"
 
-    redirect_to namespace_project_triggers_path(@project.namespace, @project)
+    redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project)
   end
 
   private

app/controllers/projects/variables_controller.rb

@@ -4,7 +4,7 @@ class Projects::VariablesController < Projects::ApplicationController
   layout 'project_settings'
 
   def index
-    @variable = Ci::Variable.new
+    redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project)
   end
 
   def show
@@ -25,9 +25,10 @@ class Projects::VariablesController < Projects::ApplicationController
     @variable = Ci::Variable.new(project_params)
 
     if @variable.valid? && @project.variables << @variable
-      redirect_to namespace_project_variables_path(project.namespace, project), notice: 'Variables were successfully updated.'
+      flash[:notice] = 'Variables were successfully updated.'
+      redirect_to namespace_project_settings_ci_cd_path(project.namespace, project)
     else
-      render action: "index"
+      render "show"
     end
   end
 
@@ -35,7 +36,7 @@ class Projects::VariablesController < Projects::ApplicationController
     @key = @project.variables.find(params[:id])
     @key.destroy
 
-    redirect_to namespace_project_variables_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

app/helpers/gitlab_routing_helper.rb

@@ -211,8 +211,12 @@ 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
+
+  def project_settings_ci_cd_path(project, *args)
+    namespace_project_settings_ci_cd_path(project.namespace, project, *args)
+  end
 end

app/views/layouts/nav/_project_settings.html.haml

@@ -18,20 +18,8 @@
         Protected Branches
 
   - if @project.feature_available?(:builds, current_user)
-    = nav_link(controller: :runners) do
-      = link_to namespace_project_runners_path(@project.namespace, @project), title: 'Runners' do
-        %span
-          Runners
-    = nav_link(controller: :variables) do
-      = link_to namespace_project_variables_path(@project.namespace, @project), title: 'Variables' do
-        %span
-          Variables
-    = nav_link(controller: :triggers) do
-      = link_to namespace_project_triggers_path(@project.namespace, @project), title: 'Triggers' do
-        %span
-          Triggers
-    = nav_link(controller: :pipelines_settings) do
-      = link_to namespace_project_pipelines_settings_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

app/views/projects/pipelines_settings/show.html.haml → app/views/projects/pipelines_settings/_show.html.haml

-- page_title "CI/CD Pipelines"
-
 .row.prepend-top-default
   .col-lg-3.profile-settings-sidebar
     %h4.prepend-top-0
-      = page_title
+      CI/CD Pipelines
   .col-lg-9
     = form_for @project, url: namespace_project_pipelines_settings_path(@project.namespace.becomes(Namespace), @project) do |f|
       %fieldset.builds-feature
@@ -95,4 +93,4 @@
 %hr
 
 .row.prepend-top-default
-  = render partial: 'badge', collection: @badges
+  = render partial: 'projects/pipelines_settings/badge', collection: @badges

app/views/projects/runners/index.html.haml → app/views/projects/runners/_index.html.haml

-- page_title "Runners"
-
 .light.prepend-top-default
   %p
     A 'Runner' is a process which runs a job.
@@ -22,6 +20,6 @@
 %p.lead To start serving your jobs you can either add specific Runners to your project or use shared Runners
 .row
   .col-sm-6
-    = render 'specific_runners'
+    = render 'projects/runners/specific_runners'
   .col-sm-6
-    = render 'shared_runners'
+    = render 'projects/runners/shared_runners'

app/views/projects/runners/_shared_runners.html.haml

@@ -22,7 +22,7 @@
 - else
   %h4.underlined-title Available shared Runners : #{@shared_runners_count}
   %ul.bordered-list.available-shared-runners
-    = render partial: 'runner', collection: @shared_runners, as: :runner
+    = render partial: 'projects/runners/runner', collection: @shared_runners, as: :runner
   - if @shared_runners_count > 10
     .light
       and #{@shared_runners_count - 10} more...

app/views/projects/runners/_specific_runners.html.haml

@@ -20,10 +20,10 @@
 - if @project_runners.any?
   %h4.underlined-title Runners activated for this project
   %ul.bordered-list.activated-specific-runners
-    = render partial: 'runner', collection: @project_runners, as: :runner
+    = render partial: 'projects/runners/runner', collection: @project_runners, as: :runner
 
 - if @assignable_runners.any?
   %h4.underlined-title Available specific runners
   %ul.bordered-list.available-specific-runners
-    = render partial: 'runner', collection: @assignable_runners, as: :runner
+    = render partial: 'projects/runners/runner', collection: @assignable_runners, as: :runner
   = paginate @assignable_runners, theme: "gitlab"

app/views/projects/settings/ci_cd/show.html.haml

+- page_title "CI/CD Pipelines"
+
+= render 'projects/runners/index'
+= render 'projects/variables/index'
+= render 'projects/triggers/index'
+= render 'projects/pipelines_settings/show'

app/views/projects/triggers/index.html.haml → app/views/projects/triggers/_index.html.haml

-- page_title "Triggers"
-
 .row.prepend-top-default.append-bottom-default
   .col-lg-3
     %h4.prepend-top-0
-      = page_title
+      Triggers
     %p.prepend-top-20
       Triggers can force a specific branch or tag to get rebuilt with an API call.
     %p.append-bottom-0
@@ -25,12 +23,12 @@
                 %th
                   %strong Last used
                 %th
-              = render partial: 'trigger', collection: @triggers, as: :trigger
+              = render partial: 'projects/triggers/trigger', collection: @triggers, as: :trigger
         - else
           %p.settings-message.text-center.append-bottom-default
             No triggers have been created yet. Add one using the button below.
 
-        = form_for @trigger, url: url_for(controller: 'projects/triggers', action: 'create') do |f|
+        = form_for @trigger, url: url_for(controller: '/projects/triggers', action: 'create') do |f|
           = f.submit "Add trigger", class: 'btn btn-success'
 
       .panel-footer

app/views/projects/variables/index.html.haml → app/views/projects/variables/_index.html.haml

-- page_title "Variables"
-
 .row.prepend-top-default.append-bottom-default
   .col-lg-3
-    = render "content"
+    = render "projects/variables/content"
   .col-lg-9
     %h5.prepend-top-0
       Add a variable
-    = render "form", btn_text: "Add new variable"
+    = render "projects/variables/form", btn_text: "Add new variable"
     %hr
     %h5.prepend-top-0
       Your variables (#{@project.variables.size})
@@ -14,5 +12,5 @@
       %p.settings-message.text-center.append-bottom-0
         No variables found, add one with the form above.
     - else
-      = render "table"
+      = render "projects/variables/table"
       %button.btn.btn-info.js-btn-toggle-reveal-values{ "data-status" => 'hidden' } Reveal Values

config/routes/project.rb

@@ -315,6 +315,7 @@ constraints(ProjectUrlConstrainer.new) do
       end
       namespace :settings do
         resource :members, only: [:show]
+        resource :ci_cd, only: [:show], controller: 'ci_cd'
         resource :integrations, only: [:show]
       end
 

spec/controllers/projects/settings/ci_cd_controller_spec.rb

+require('spec_helper')
+
+describe Projects::Settings::CiCdController do
+  let(:project) { create(:empty_project, :public, :access_requestable) }
+  let(:user) { create(:user) }
+
+  before do
+    project.team << [user, :master]
+    sign_in(user)
+  end
+
+  describe 'GET show' do
+    it 'renders show with 200 status code' do
+      get :show, namespace_id: project.namespace, project_id: project
+
+      expect(response).to have_http_status(200)
+      expect(response).to render_template(:show)
+    end
+  end
+end

spec/controllers/projects/variables_controller_spec.rb

+require 'spec_helper'
+
+describe Projects::VariablesController do
+  let(:project) { create(:empty_project) }
+  let(:user) { create(:user) }
+
+  before do
+    sign_in(user)
+    project.team << [user, :master]
+  end
+
+  describe 'POST #create' do
+    context 'variable is valid' do
+      it 'shows a success flash message' do
+        post :create, namespace_id: project.namespace.to_param, project_id: project.to_param,
+                      variable: { key: "one", value: "two" }
+
+        expect(flash[:notice]).to include 'Variables were successfully updated.'
+        expect(response).to redirect_to(namespace_project_settings_ci_cd_path(project.namespace, project))
+      end
+    end
+
+    context 'variable is invalid' do
+      it 'shows an alert flash message' do
+        post :create, namespace_id: project.namespace.to_param, project_id: project.to_param,
+                      variable: { key: "..one", value: "two" }
+
+        expect(response).to render_template("projects/variables/show")
+      end
+    end
+  end
+
+  describe 'POST #update' do
+    let(:variable) { create(:ci_variable) }
+
+    context 'updating a variable with valid characters' do
+      before do
+        variable.gl_project_id = project.id
+        project.variables << variable
+      end
+
+      it 'shows a success flash message' do
+        post :update, namespace_id: project.namespace.to_param, project_id: project.to_param,
+                      id: variable.id, variable: { key: variable.key, value: 'two' }
+
+        expect(flash[:notice]).to include 'Variable was successfully updated.'
+        expect(response).to redirect_to(namespace_project_variables_path(project.namespace, project))
+      end
+
+      it 'renders the action #show if the variable key is invalid' do
+        post :update, namespace_id: project.namespace.to_param, project_id: project.to_param,
+                      id: variable.id, variable: { key: '?', value: variable.value }
+
+        expect(response).to have_http_status(200)
+        expect(response).to render_template :show
+      end
+    end
+  end
+end

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')) }

spec/features/security/project/private_access_spec.rb

@@ -92,8 +92,22 @@ describe "Private Project Access", feature: true  do
     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(:user) }
+    it { is_expected.to be_denied_for(:visitor) }
     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

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) }
 

spec/features/triggers_spec.rb

@@ -7,7 +7,7 @@ describe 'Triggers' do
   before do
     @project = FactoryGirl.create :empty_project
     @project.team << [user, :master]
-    visit namespace_project_triggers_path(@project.namespace, @project)
+    visit namespace_project_settings_ci_cd_path(@project.namespace, @project)
   end
 
   context 'create a trigger' do

spec/features/variables_spec.rb

@@ -10,7 +10,7 @@ describe 'Project variables', js: true do
     project.team << [user, :master]
     project.variables << variable
 
-    visit namespace_project_variables_path(project.namespace, project)
+    visit namespace_project_settings_ci_cd_path(project.namespace, project)
   end
 
   it 'shows list of variables' do