Move leftovers from pipelines_settings_controller to settings/ci_cd_controller

app/controllers/projects/pipelines_settings_controller.rb

@@ -4,41 +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."
-
-        run_autodevops_pipeline(service)
-
-        redirect_to project_settings_ci_cd_path(@project)
-      else
-        render 'show'
-      end
-    end
-  end
-
-  private
-
-  def run_autodevops_pipeline(service)
-    return unless service.run_auto_devops_pipeline?
-
-    if @project.empty_repo?
-      flash[:warning] = "This repository is currently empty. A new Auto DevOps pipeline will be created after a new file has been pushed to a branch."
-      return
-    end
-
-    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
-
-  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

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

app/controllers/projects/settings/ci_cd_controller.rb

@@ -2,13 +2,24 @@ 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."
+
+            run_autodevops_pipeline(service)
+
+            redirect_to project_settings_ci_cd_path(@project)
+          else
+            render 'show'
+          end
+        end
       end
 
       def reset_cache
@@ -25,6 +36,35 @@ module Projects
 
       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
+
+      def run_autodevops_pipeline(service)
+        return unless service.run_auto_devops_pipeline?
+
+        if @project.empty_repo?
+          flash[:warning] = "This repository is currently empty. A new Auto DevOps pipeline will be created after a new file has been pushed to a branch."
+          return
+        end
+
+        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
+
+      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

app/views/projects/pipelines_settings/_show.html.haml → app/views/projects/settings/ci_cd/_form.html.haml

 .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

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

config/routes/project.rb

@@ -420,7 +420,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]

spec/controllers/projects/pipelines_settings_controller_spec.rb

@@ -11,82 +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
+  describe 'GET show' do
+    it 'redirects with 302 status code' do
+      get :show, namespace_id: project.namespace, project_id: project
 
       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
-
-        context 'when the project repository is empty' do
-          it 'sets a warning flash' do
-            expect(subject).to set_flash[:warning]
-          end
-
-          it 'does not queue a CreatePipelineWorker' do
-            expect(CreatePipelineWorker).not_to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
-
-            subject
-          end
-        end
-
-        context 'when the project repository is not empty' do
-          let(:project) { create(:project, :repository) }
-
-          it 'sets a success flash' do
-            allow(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
-
-            expect(subject).to set_flash[:success]
-          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
-      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

spec/controllers/projects/settings/ci_cd_controller_spec.rb

 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,83 @@ 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
+
+        context 'when the project repository is empty' do
+          it 'sets a warning flash' do
+            expect(subject).to set_flash[:warning]
+          end
+
+          it 'does not queue a CreatePipelineWorker' do
+            expect(CreatePipelineWorker).not_to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
+
+            subject
+          end
+        end
+
+        context 'when the project repository is not empty' do
+          let(:project) { create(:project, :repository) }
+
+          it 'sets a success flash' do
+            allow(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)
+
+            expect(subject).to set_flash[:success]
+          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
+      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

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

spec/views/projects/pipelines_settings/_show.html.haml_spec.rb → spec/views/projects/settings/ci_cd/_form.html.haml_spec.rb

 require 'spec_helper'
 
-describe 'projects/pipelines_settings/_show' do
+describe 'projects/settings/ci_cd/_form' do
   let(:project) { create(:project, :repository) }
 
   before do