Move leftovers from pipelines_settings_controller to settings/ci_cd_controlleruse-chronic-duration-attribute-for-project-build-timeout

11 files changed, 110 insertions, 96 deletions

diff --git a/app/controllers/projects/pipelines_settings_controller.rb b/app/controllers/projects/pipelines_settings_controller.rb
index 136a3b9f2f1..73c613b26f3 100644
--- a/app/controllers/projects/pipelines_settings_controller.rb
+++ b/app/controllers/projects/pipelines_settings_controller.rb
@@ -4,32 +4,4 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController
   def show
     redirect_to project_settings_ci_cd_path(@project, params: params)
   end
-
-  def update
-    Projects::UpdateService.new(project, current_user, update_params).tap do |service|
-      if service.execute
-        flash[:notice] = "Pipelines settings for '#{@project.name}' were successfully updated."
-
-        if service.run_auto_devops_pipeline?
-          CreatePipelineWorker.perform_async(project.id, current_user.id, project.default_branch, :web, ignore_skip_ci: true, save_on_errors: false)
-          flash[:success] = "A new Auto DevOps pipeline has been created, go to <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details".html_safe
-        end
-
-        redirect_to project_settings_ci_cd_path(@project)
-      else
-        render 'show'
-      end
-    end
-  end
-
-  private
-
-  def update_params
-    params.require(:project).permit(
-      :runners_token, :builds_enabled, :build_allow_git_fetch,
-      :build_timeout_human_readable, :build_coverage_regex, :public_builds,
-      :auto_cancel_pending_pipelines, :ci_config_path,
-      auto_devops_attributes: [:id, :domain, :enabled]
-    )
-  end
 end
diff --git a/app/controllers/projects/refs_controller.rb b/app/controllers/projects/refs_controller.rb
index 2376f469213..48a09e1ddb8 100644
--- a/app/controllers/projects/refs_controller.rb
+++ b/app/controllers/projects/refs_controller.rb
@@ -25,7 +25,7 @@ class Projects::RefsController < Projects::ApplicationController
           when "graphs_commits"
             commits_project_graph_path(@project, @id)
           when "badges"
-            project_pipelines_settings_path(@project, ref: @id)
+            project_settings_ci_cd_path(@project, ref: @id)
           else
             project_commits_path(@project, @id)
           end
diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb
index 259809f3429..e75002af683 100644
--- a/app/controllers/projects/settings/ci_cd_controller.rb
+++ b/app/controllers/projects/settings/ci_cd_controller.rb
@@ -2,13 +2,27 @@ module Projects
   module Settings
     class CiCdController < Projects::ApplicationController
       before_action :authorize_admin_pipeline!
+      before_action :define_variables
 
       def show
-        define_runners_variables
-        define_secret_variables
-        define_triggers_variables
-        define_badges_variables
-        define_auto_devops_variables
+      end
+
+      def update
+        Projects::UpdateService.new(project, current_user, update_params).tap do |service|
+          result = service.execute
+          if result[:status] == :success
+            flash[:notice] = "Pipelines settings for '#{@project.name}' were successfully updated."
+
+            if service.run_auto_devops_pipeline?
+              CreatePipelineWorker.perform_async(project.id, current_user.id, project.default_branch, :web, ignore_skip_ci: true, save_on_errors: false)
+              flash[:success] = "A new Auto DevOps pipeline has been created, go to <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details".html_safe
+            end
+
+            redirect_to project_settings_ci_cd_path(@project)
+          else
+            render 'show'
+          end
+        end
       end
 
       def reset_cache
@@ -25,6 +39,14 @@ module Projects
 
       private
 
+      def define_variables
+        define_runners_variables
+        define_secret_variables
+        define_triggers_variables
+        define_badges_variables
+        define_auto_devops_variables
+      end
+
       def define_runners_variables
         @project_runners = @project.runners.ordered
         @assignable_runners = current_user.ci_authorized_runners
@@ -59,6 +81,15 @@ module Projects
       def define_auto_devops_variables
         @auto_devops = @project.auto_devops || ProjectAutoDevops.new
       end
+
+      def update_params
+        params.require(:project).permit(
+          :runners_token, :builds_enabled, :build_allow_git_fetch,
+          :build_timeout_human_readable, :build_coverage_regex, :public_builds,
+          :auto_cancel_pending_pipelines, :ci_config_path,
+          auto_devops_attributes: [:id, :domain, :enabled]
+        )
+      end
     end
   end
 end
diff --git a/app/views/projects/pipelines_settings/_badge.html.haml b/app/views/projects/settings/ci_cd/_badge.html.haml
index e8028059487..e8028059487 100644
--- a/app/views/projects/pipelines_settings/_badge.html.haml
+++ b/app/views/projects/settings/ci_cd/_badge.html.haml
diff --git a/app/views/projects/pipelines_settings/_show.html.haml b/app/views/projects/settings/ci_cd/_form.html.haml
index 845fa088ec0..a976ad12cff 100644
--- a/app/views/projects/pipelines_settings/_show.html.haml
+++ b/app/views/projects/settings/ci_cd/_form.html.haml
@@ -1,6 +1,7 @@
 .row.prepend-top-default
   .col-lg-12
-    = form_for @project, url: project_pipelines_settings_path(@project) do |f|
+    = form_for @project, url: project_settings_ci_cd_path(@project) do |f|
+      = form_errors(@project)
       %fieldset.builds-feature
         .form-group
           %h5 Auto DevOps (Beta)
@@ -157,4 +158,4 @@
 %hr
 
 .row.prepend-top-default
-  = render partial: 'projects/pipelines_settings/badge', collection: @badges
+  = render partial: 'badge', collection: @badges
diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml
index 756f31f91d9..a3d5f2f85f7 100644
--- a/app/views/projects/settings/ci_cd/show.html.haml
+++ b/app/views/projects/settings/ci_cd/show.html.haml
@@ -3,8 +3,9 @@
 - page_title "CI / CD"
 
 - expanded = Rails.env.test?
+- general_expanded = @project.errors.empty? ? expanded : true
 
-%section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if expanded) }
+%section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if general_expanded) }
   .settings-header
     %h4
       General pipelines settings
@@ -13,7 +14,7 @@
     %p
       Update your CI/CD configuration, like job timeout or Auto DevOps.
   .settings-content
-    = render 'projects/pipelines_settings/show'
+    = render 'form'
 
 %section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
diff --git a/config/routes/project.rb b/config/routes/project.rb
index c803737d40b..375cf71f6c3 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -416,7 +416,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       end
       namespace :settings do
         get :members, to: redirect("%{namespace_id}/%{project_id}/project_members")
-        resource :ci_cd, only: [:show], controller: 'ci_cd' do
+        resource :ci_cd, only: [:show, :update], controller: 'ci_cd' do
           post :reset_cache
         end
         resource :integrations, only: [:show]
diff --git a/spec/controllers/projects/pipelines_settings_controller_spec.rb b/spec/controllers/projects/pipelines_settings_controller_spec.rb
index 1cc488bef32..694896b6bcf 100644
--- a/spec/controllers/projects/pipelines_settings_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_settings_controller_spec.rb
@@ -11,60 +11,11 @@ describe Projects::PipelinesSettingsController do
     sign_in(user)
   end
 
-  describe 'PATCH update' do
-    subject do
-      patch :update,
-        namespace_id: project.namespace.to_param,
-        project_id: project,
-        project: {
-          auto_devops_attributes: params
-        }
-    end
-
-    context 'when updating the auto_devops settings' do
-      let(:params) { { enabled: '', domain: 'mepmep.md' } }
-
-      it 'redirects to the settings page' do
-        subject
-
-        expect(response).to have_gitlab_http_status(302)
-        expect(flash[:notice]).to eq("Pipelines settings for '#{project.name}' were successfully updated.")
-      end
-
-      context 'following the instance default' do
-        let(:params) { { enabled: '' } }
-
-        it 'allows enabled to be set to nil' do
-          subject
-          project_auto_devops.reload
-
-          expect(project_auto_devops.enabled).to be_nil
-        end
-      end
-
-      context 'when run_auto_devops_pipeline is true' do
-        before do
-          expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(true)
-        end
-
-        it 'queues a CreatePipelineWorker' do
-          expect(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
-
-          subject
-        end
-      end
-
-      context 'when run_auto_devops_pipeline is not true' do
-        before do
-          expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(false)
-        end
-
-        it 'does not queue a CreatePipelineWorker' do
-          expect(CreatePipelineWorker).not_to receive(:perform_async).with(project.id, user.id, :web, any_args)
+  describe 'GET show' do
+    it 'redirects with 302 status code' do
+      get :show, namespace_id: project.namespace, project_id: project
 
-          subject
-        end
-      end
+      expect(response).to have_gitlab_http_status(302)
     end
   end
 end
diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
index 293e76798ae..28859debaf9 100644
--- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb
+++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
@@ -1,8 +1,9 @@
 require('spec_helper')
 
 describe Projects::Settings::CiCdController do
-  let(:project) { create(:project, :public, :access_requestable) }
-  let(:user) { create(:user) }
+  set(:user) { create(:user) }
+  set(:project_auto_devops) { create(:project_auto_devops) }
+  let(:project) { project_auto_devops.project }
 
   before do
     project.add_master(user)
@@ -55,4 +56,61 @@ describe Projects::Settings::CiCdController do
       end
     end
   end
+
+  describe 'PATCH update' do
+    subject do
+      patch :update,
+            namespace_id: project.namespace.to_param,
+            project_id: project,
+            project: {
+                auto_devops_attributes: params
+            }
+    end
+
+    context 'when updating the auto_devops settings' do
+      let(:params) { { enabled: '', domain: 'mepmep.md' } }
+
+      it 'redirects to the settings page' do
+        subject
+
+        expect(response).to have_gitlab_http_status(302)
+        expect(flash[:notice]).to eq("Pipelines settings for '#{project.name}' were successfully updated.")
+      end
+
+      context 'following the instance default' do
+        let(:params) { { enabled: '' } }
+
+        it 'allows enabled to be set to nil' do
+          subject
+          project_auto_devops.reload
+
+          expect(project_auto_devops.enabled).to be_nil
+        end
+      end
+
+      context 'when run_auto_devops_pipeline is true' do
+        before do
+          expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(true)
+        end
+
+        it 'queues a CreatePipelineWorker' do
+          expect(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
+
+          subject
+        end
+      end
+
+      context 'when run_auto_devops_pipeline is not true' do
+        before do
+          expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(false)
+        end
+
+        it 'does not queue a CreatePipelineWorker' do
+          expect(CreatePipelineWorker).not_to receive(:perform_async).with(project.id, user.id, :web, any_args)
+
+          subject
+        end
+      end
+    end
+  end
 end
diff --git a/spec/features/projects/badges/list_spec.rb b/spec/features/projects/badges/list_spec.rb
index c705e479690..0abef4bc447 100644
--- a/spec/features/projects/badges/list_spec.rb
+++ b/spec/features/projects/badges/list_spec.rb
@@ -6,7 +6,7 @@ feature 'list of badges' do
     project = create(:project, :repository)
     project.add_master(user)
     sign_in(user)
-    visit project_pipelines_settings_path(project)
+    visit project_settings_ci_cd_path(project)
   end
 
   scenario 'user wants to see build status badge' do
diff --git a/spec/views/projects/pipelines_settings/_show.html.haml_spec.rb b/spec/views/projects/settings/ci_cd/_form.html.haml_spec.rb
index 7b300150874..be9a4d9c57c 100644
--- a/spec/views/projects/pipelines_settings/_show.html.haml_spec.rb
+++ b/spec/views/projects/settings/ci_cd/_form.html.haml_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe 'projects/pipelines_settings/_show' do
+describe 'projects/settings/ci_cd/_form' do
   let(:project) { create(:project, :repository) }
 
   before do