Merge branch '47156-improve-auto-devops-settings' into 'master'

Resolve "Improve Auto DevOps settings flow for admin and project"

Closes #47156

See merge request gitlab-org/gitlab-ce!20946

11 files changed, 138 insertions, 81 deletions

diff --git a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js b/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js
index 1faa59fb45b..8f5ac3d8082 100644
--- a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js
+++ b/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js
@@ -23,17 +23,12 @@ document.addEventListener('DOMContentLoaded', () => {
     saveEndpoint: variableListEl.dataset.saveEndpoint,
   });
 
-  // hide extra auto devops settings based on data-attributes
-  const autoDevOpsSettings = document.querySelector('.js-auto-devops-settings');
+  // hide extra auto devops settings based checkbox state
   const autoDevOpsExtraSettings = document.querySelector('.js-extra-settings');
-
-  autoDevOpsSettings.addEventListener('click', event => {
+  const instanceDefaultBadge = document.querySelector('.js-instance-default-badge');
+  document.querySelector('.js-toggle-extra-settings').addEventListener('click', event => {
     const { target } = event;
-    if (target.classList.contains('js-toggle-extra-settings')) {
-      autoDevOpsExtraSettings.classList.toggle(
-        'hidden',
-        !!(target.dataset && target.dataset.hideExtraSettings),
-      );
-    }
+    if (instanceDefaultBadge) instanceDefaultBadge.style.display = 'none';
+    autoDevOpsExtraSettings.classList.toggle('hidden', !target.checked);
   });
 });
diff --git a/app/assets/stylesheets/pages/settings_ci_cd.scss b/app/assets/stylesheets/pages/settings_ci_cd.scss
index 777fdb3581e..239123fc3ab 100644
--- a/app/assets/stylesheets/pages/settings_ci_cd.scss
+++ b/app/assets/stylesheets/pages/settings_ci_cd.scss
@@ -19,9 +19,4 @@
 
 .auto-devops-card {
   margin-bottom: $gl-vert-padding;
-
-  > .card-body {
-    border-radius: $card-border-radius;
-    padding: $gl-padding $gl-padding-24;
-  }
 }
diff --git a/app/models/project.rb b/app/models/project.rb
index 327ee901054..36089995ed3 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -507,6 +507,10 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def has_auto_devops_implicitly_enabled?
+    auto_devops&.enabled.nil? && Gitlab::CurrentSettings.auto_devops_enabled?
+  end
+
   def has_auto_devops_implicitly_disabled?
     auto_devops&.enabled.nil? && !Gitlab::CurrentSettings.auto_devops_enabled?
   end
diff --git a/app/views/admin/application_settings/_ci_cd.html.haml b/app/views/admin/application_settings/_ci_cd.html.haml
index 472616b1315..5037017e38a 100644
--- a/app/views/admin/application_settings/_ci_cd.html.haml
+++ b/app/views/admin/application_settings/_ci_cd.html.haml
@@ -3,13 +3,15 @@
 
   %fieldset
     .form-group
-      .form-check
-        = f.check_box :auto_devops_enabled, class: 'form-check-input'
-        = f.label :auto_devops_enabled, class: 'form-check-label' do
-          Enabled Auto DevOps for projects by default
-      .form-text.text-muted
-        It will automatically build, test, and deploy applications based on a predefined CI/CD configuration
-        = link_to icon('question-circle'), help_page_path('topics/autodevops/index.md')
+      .card.auto-devops-card
+        .card-body
+          .form-check
+            = f.check_box :auto_devops_enabled, class: 'form-check-input'
+            = f.label :auto_devops_enabled, class: 'form-check-label' do
+              Default to Auto DevOps pipeline for all projects
+            .form-text.text-muted
+              = s_('CICD|The Auto DevOps pipeline will run if no alternative CI configuration file is found.')
+              = link_to _('More information'), help_page_path('topics/autodevops/index.md'), target: '_blank'
     .form-group
       = f.label :auto_devops_domain, class: 'label-bold'
       = f.text_field :auto_devops_domain, class: 'form-control', placeholder: 'domain.com'
diff --git a/app/views/projects/settings/ci_cd/_autodevops_form.html.haml b/app/views/projects/settings/ci_cd/_autodevops_form.html.haml
index 31c2616d283..7d878b38e85 100644
--- a/app/views/projects/settings/ci_cd/_autodevops_form.html.haml
+++ b/app/views/projects/settings/ci_cd/_autodevops_form.html.haml
@@ -13,23 +13,15 @@
             .card.auto-devops-card
               .card-body
                 .form-check
-                  = form.radio_button :enabled, 'true', class: 'form-check-input js-toggle-extra-settings'
-                  = form.label :enabled_true, class: 'form-check-label' do
-                    %strong= s_('CICD|Enable Auto DevOps')
+                  = form.check_box :enabled, class: 'form-check-input js-toggle-extra-settings', checked: @project.auto_devops_enabled?
+                  = form.label :enabled, class: 'form-check-label' do
+                    %strong= s_('CICD|Default to Auto DevOps pipeline')
+                    - if @project.has_auto_devops_implicitly_enabled?
+                      %span.badge.badge-info.js-instance-default-badge= s_('CICD|instance enabled')
                   .form-text.text-muted
-                    = s_('CICD|The Auto DevOps pipeline configuration will be used when there is no %{ci_file} in the project.').html_safe % { ci_file: ci_file_formatted }
-
-            .card.auto-devops-card
-              .card-body
-                .form-check
-                  = form.radio_button :enabled, '', class: 'form-check-input js-toggle-extra-settings'
-                  = form.label :enabled_, class: 'form-check-label' do
-                    %strong= s_('CICD|Instance default (%{state})') % { state: "#{Gitlab::CurrentSettings.auto_devops_enabled? ? _('enabled') : _('disabled')}" }
-                  .form-text.text-muted
-                    = s_('CICD|Follow the instance default to either have Auto DevOps enabled or disabled when there is no project specific %{ci_file}.').html_safe % { ci_file: ci_file_formatted }
-
-            .card.auto-devops-card.js-extra-settings{ class: form.object&.enabled == false ? 'hidden' : nil }
-              .card-body.bg-light
+                    = s_('CICD|The Auto DevOps pipeline will run if no alternative CI configuration file is found.')
+                    = link_to _('More information'), help_page_path('topics/autodevops/index.md'), target: '_blank'
+              .card-footer.js-extra-settings{ class: @project.auto_devops_enabled? || 'hidden' }
                 = form.label :domain do
                   %strong= _('Domain')
                 = form.text_field :domain, class: 'form-control', placeholder: 'domain.com'
@@ -46,21 +38,12 @@
                 .form-check
                   = form.radio_button :deploy_strategy, 'continuous', class: 'form-check-input'
                   = form.label :deploy_strategy_continuous, class: 'form-check-label' do
-                    %strong= s_('CICD|Continuous deployment to production')
+                    = s_('CICD|Continuous deployment to production')
                     = link_to icon('question-circle'), help_page_path('topics/autodevops/index.md', anchor: 'auto-deploy'), target: '_blank'
                 .form-check
                   = form.radio_button :deploy_strategy, 'manual', class: 'form-check-input'
                   = form.label :deploy_strategy_manual, class: 'form-check-label' do
-                    %strong= s_('CICD|Automatic deployment to staging, manual deployment to production')
+                    = s_('CICD|Automatic deployment to staging, manual deployment to production')
                     = link_to icon('question-circle'), help_page_path('ci/environments.md', anchor: 'manually-deploying-to-environments'), target: '_blank'
 
-            .card.auto-devops-card
-              .card-body
-                .form-check
-                  = form.radio_button :enabled, 'false', class: 'form-check-input js-toggle-extra-settings', data: { hide_extra_settings: true }
-                  = form.label :enabled_false, class: 'form-check-label' do
-                    %strong= s_('CICD|Disable Auto DevOps')
-                  .form-text.text-muted
-                    = s_('CICD|An explicit %{ci_file} needs to be specified before you can begin using Continuous Integration and Delivery.').html_safe % { ci_file: ci_file_formatted }
-
           = f.submit _('Save changes'), class: "btn btn-success prepend-top-15"
diff --git a/changelogs/unreleased/47156-improve-auto-devops-settings.yml b/changelogs/unreleased/47156-improve-auto-devops-settings.yml
new file mode 100644
index 00000000000..d8993565047
--- /dev/null
+++ b/changelogs/unreleased/47156-improve-auto-devops-settings.yml
@@ -0,0 +1,5 @@
+---
+title: Improve and simplify Auto DevOps settings flow
+merge_request: 20946
+author:
+type: other
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 1ec6f1a0389..c159cddcc60 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -990,9 +990,6 @@ msgstr ""
 msgid "CI/CD settings"
 msgstr ""
 
-msgid "CICD|An explicit %{ci_file} needs to be specified before you can begin using Continuous Integration and Delivery."
-msgstr ""
-
 msgid "CICD|Auto DevOps"
 msgstr ""
 
@@ -1005,22 +1002,13 @@ msgstr ""
 msgid "CICD|Continuous deployment to production"
 msgstr ""
 
-msgid "CICD|Deployment strategy"
-msgstr ""
-
-msgid "CICD|Deployment strategy needs a domain name to work correctly."
-msgstr ""
-
-msgid "CICD|Disable Auto DevOps"
-msgstr ""
-
-msgid "CICD|Enable Auto DevOps"
+msgid "CICD|Default to Auto DevOps pipeline"
 msgstr ""
 
-msgid "CICD|Follow the instance default to either have Auto DevOps enabled or disabled when there is no project specific %{ci_file}."
+msgid "CICD|Deployment strategy"
 msgstr ""
 
-msgid "CICD|Instance default (%{state})"
+msgid "CICD|Deployment strategy needs a domain name to work correctly."
 msgstr ""
 
 msgid "CICD|Jobs"
@@ -1029,12 +1017,15 @@ msgstr ""
 msgid "CICD|Learn more about Auto DevOps"
 msgstr ""
 
-msgid "CICD|The Auto DevOps pipeline configuration will be used when there is no %{ci_file} in the project."
+msgid "CICD|The Auto DevOps pipeline will run if no alternative CI configuration file is found."
 msgstr ""
 
 msgid "CICD|You need to specify a domain if you want to use Auto Review Apps and Auto Deploy stages."
 msgstr ""
 
+msgid "CICD|instance enabled"
+msgstr ""
+
 msgid "Callback URL"
 msgstr ""
 
diff --git a/qa/qa/page/project/settings/ci_cd.rb b/qa/qa/page/project/settings/ci_cd.rb
index 0f739f61db9..752d3d93407 100644
--- a/qa/qa/page/project/settings/ci_cd.rb
+++ b/qa/qa/page/project/settings/ci_cd.rb
@@ -12,9 +12,9 @@ module QA # rubocop:disable Naming/FileName
           end
 
           view 'app/views/projects/settings/ci_cd/_autodevops_form.html.haml' do
-            element :enable_auto_devops_field, 'radio_button :enabled'
+            element :enable_auto_devops_field, 'check_box :enabled'
             element :domain_field, 'text_field :domain'
-            element :enable_auto_devops_button, "%strong= s_('CICD|Enable Auto DevOps')"
+            element :enable_auto_devops_button, "%strong= s_('CICD|Default to Auto DevOps pipeline')"
             element :domain_input, "%strong= _('Domain')"
             element :save_changes_button, "submit _('Save changes')"
           end
@@ -33,7 +33,7 @@ module QA # rubocop:disable Naming/FileName
 
           def enable_auto_devops_with_domain(domain)
             expand_section(:autodevops_settings) do
-              choose 'Enable Auto DevOps'
+              check 'Default to Auto DevOps pipeline'
               fill_in 'Domain', with: domain
               click_on 'Save changes'
             end
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index a852ca689e7..af1c153dec8 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -175,7 +175,7 @@ describe 'Admin updates settings' do
 
   it 'Change CI/CD settings' do
     page.within('.as-ci-cd') do
-      check 'Enabled Auto DevOps for projects by default'
+      check 'Default to Auto DevOps pipeline for all projects'
       fill_in 'Auto devops domain', with: 'domain.com'
       click_button 'Save changes'
     end
diff --git a/spec/features/projects/settings/pipelines_settings_spec.rb b/spec/features/projects/settings/pipelines_settings_spec.rb
index 742ecf82c38..30b0a5578ea 100644
--- a/spec/features/projects/settings/pipelines_settings_spec.rb
+++ b/spec/features/projects/settings/pipelines_settings_spec.rb
@@ -8,7 +8,6 @@ describe "Projects > Settings > Pipelines settings" do
   before do
     sign_in(user)
     project.add_role(user, role)
-    create(:project_auto_devops, project: project)
   end
 
   context 'for developer' do
@@ -61,19 +60,58 @@ describe "Projects > Settings > Pipelines settings" do
     end
 
     describe 'Auto DevOps' do
-      it 'update auto devops settings' do
-        visit project_settings_ci_cd_path(project)
+      context 'when auto devops is turned on instance-wide' do
+        before do
+          stub_application_setting(auto_devops_enabled: true)
+        end
+
+        it 'auto devops is on by default and can be manually turned off' do
+          visit project_settings_ci_cd_path(project)
 
-        page.within '#autodevops-settings' do
-          fill_in('project_auto_devops_attributes_domain', with: 'test.com')
-          page.choose('project_auto_devops_attributes_enabled_false')
-          click_on 'Save changes'
+          page.within '#autodevops-settings' do
+            expect(find_field('project_auto_devops_attributes_enabled')).to be_checked
+            expect(page).to have_content('instance enabled')
+            uncheck 'Default to Auto DevOps pipeline'
+            click_on 'Save changes'
+          end
+
+          expect(page.status_code).to eq(200)
+          expect(project.auto_devops).to be_present
+          expect(project.auto_devops).not_to be_enabled
+
+          page.within '#autodevops-settings' do
+            expect(find_field('project_auto_devops_attributes_enabled')).not_to be_checked
+            expect(page).not_to have_content('instance enabled')
+          end
         end
+      end
 
-        expect(page.status_code).to eq(200)
-        expect(project.auto_devops).to be_present
-        expect(project.auto_devops).not_to be_enabled
-        expect(project.auto_devops.domain).to eq('test.com')
+      context 'when auto devops is not turned on instance-wide' do
+        before do
+          stub_application_setting(auto_devops_enabled: false)
+        end
+
+        it 'auto devops is off by default and can be manually turned on' do
+          visit project_settings_ci_cd_path(project)
+
+          page.within '#autodevops-settings' do
+            expect(page).not_to have_content('instance enabled')
+            expect(find_field('project_auto_devops_attributes_enabled')).not_to be_checked
+            check 'Default to Auto DevOps pipeline'
+            fill_in('project_auto_devops_attributes_domain', with: 'test.com')
+            click_on 'Save changes'
+          end
+
+          expect(page.status_code).to eq(200)
+          expect(project.auto_devops).to be_present
+          expect(project.auto_devops).to be_enabled
+          expect(project.auto_devops.domain).to eq('test.com')
+
+          page.within '#autodevops-settings' do
+            expect(find_field('project_auto_devops_attributes_enabled')).to be_checked
+            expect(page).not_to have_content('instance enabled')
+          end
+        end
       end
 
       context 'when there is a cluster with ingress and external_ip' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 4313d52d60a..03beb9187ed 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -3307,6 +3307,50 @@ describe Project do
     end
   end
 
+  describe '#has_auto_devops_implicitly_enabled?' do
+    set(:project) { create(:project) }
+
+    context 'when disabled in settings' do
+      before do
+        stub_application_setting(auto_devops_enabled: false)
+      end
+
+      it 'does not have auto devops implicitly disabled' do
+        expect(project).not_to have_auto_devops_implicitly_enabled
+      end
+    end
+
+    context 'when enabled in settings' do
+      before do
+        stub_application_setting(auto_devops_enabled: true)
+      end
+
+      it 'auto devops is implicitly disabled' do
+        expect(project).to have_auto_devops_implicitly_enabled
+      end
+
+      context 'when explicitly disabled' do
+        before do
+          create(:project_auto_devops, project: project, enabled: false)
+        end
+
+        it 'does not have auto devops implicitly disabled' do
+          expect(project).not_to have_auto_devops_implicitly_enabled
+        end
+      end
+
+      context 'when explicitly enabled' do
+        before do
+          create(:project_auto_devops, project: project, enabled: true)
+        end
+
+        it 'does not have auto devops implicitly disabled' do
+          expect(project).not_to have_auto_devops_implicitly_enabled
+        end
+      end
+    end
+  end
+
   describe '#has_auto_devops_implicitly_disabled?' do
     set(:project) { create(:project) }
 
@@ -3341,7 +3385,7 @@ describe Project do
 
       context 'when explicitly enabled' do
         before do
-          create(:project_auto_devops, project: project)
+          create(:project_auto_devops, project: project, enabled: true)
         end
 
         it 'does not have auto devops implicitly disabled' do