diff options
author | Clement Ho <clemmakesapps@gmail.com> | 2018-08-07 00:51:37 +0300 |
---|---|---|
committer | Clement Ho <clemmakesapps@gmail.com> | 2018-08-07 00:51:37 +0300 |
commit | ac1d83c56b4aa550761a13f25d7650c94194553f (patch) | |
tree | a4ab1c6cdd871dd6f4134387bed9da4021c0ec0d | |
parent | d737abc537476bf2b500f550b0c733d22f338cf1 (diff) | |
parent | e3127132a498cd3a39a325a5c0912ca6121e2f8a (diff) |
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
-rw-r--r-- | app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js | 15 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/settings_ci_cd.scss | 5 | ||||
-rw-r--r-- | app/models/project.rb | 4 | ||||
-rw-r--r-- | app/views/admin/application_settings/_ci_cd.html.haml | 16 | ||||
-rw-r--r-- | app/views/projects/settings/ci_cd/_autodevops_form.html.haml | 37 | ||||
-rw-r--r-- | changelogs/unreleased/47156-improve-auto-devops-settings.yml | 5 | ||||
-rw-r--r-- | locale/gitlab.pot | 23 | ||||
-rw-r--r-- | qa/qa/page/project/settings/ci_cd.rb | 6 | ||||
-rw-r--r-- | spec/features/admin/admin_settings_spec.rb | 2 | ||||
-rw-r--r-- | spec/features/projects/settings/pipelines_settings_spec.rb | 60 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 46 |
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 |