Merge branch 'dz-improve-app-settings-3' into 'master'

Redesign application settings to match project settings (part 3)

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

6 files changed, 201 insertions, 137 deletions

diff --git a/app/views/admin/application_settings/_background_jobs.html.haml b/app/views/admin/application_settings/_background_jobs.html.haml
new file mode 100644
index 00000000000..8198a822a10
--- /dev/null
+++ b/app/views/admin/application_settings/_background_jobs.html.haml
@@ -0,0 +1,30 @@
+= form_for @application_setting, url: admin_application_settings_path, html: { class: 'form-horizontal fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    %p
+      These settings require a
+      = link_to 'restart', help_page_path('administration/restart_gitlab')
+      to take effect.
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :sidekiq_throttling_enabled do
+            = f.check_box :sidekiq_throttling_enabled
+            Enable Sidekiq Job Throttling
+          .help-block
+            Limit the amount of resources slow running jobs are assigned.
+    .form-group
+      = f.label :sidekiq_throttling_queues, 'Sidekiq queues to throttle', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.select :sidekiq_throttling_queues, sidekiq_queue_options_for_select, { include_hidden: false }, multiple: true, class: 'select2 select-wide', data: { field: 'sidekiq_throttling_queues' }
+        .help-block
+          Choose which queues you wish to throttle.
+    .form-group
+      = f.label :sidekiq_throttling_factor, 'Throttling Factor', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :sidekiq_throttling_factor, class: 'form-control', min: '0.01', max: '0.99', step: '0.01'
+        .help-block
+          The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive.
+
+  = f.submit 'Save changes', class: "btn btn-success"
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 636535fba84..309c7ed5dfa 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -10,111 +10,6 @@
           = f.number_field :container_registry_token_expire_delay, class: 'form-control'
 
   %fieldset
-    %legend Profiling - Performance Bar
-    %p
-      Enable the Performance Bar for a given group.
-      = link_to icon('question-circle'), help_page_path('administration/monitoring/performance/performance_bar')
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :performance_bar_enabled do
-            = f.check_box :performance_bar_enabled
-            Enable the Performance Bar
-    .form-group
-      = f.label :performance_bar_allowed_group_id, 'Allowed group', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.text_field :performance_bar_allowed_group_id, class: 'form-control', placeholder: 'my-org/my-group', value: @application_setting.performance_bar_allowed_group&.full_path
-
-  %fieldset
-    %legend Background Jobs
-    %p
-      These settings require a
-      = link_to 'restart', help_page_path('administration/restart_gitlab')
-      to take effect.
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :sidekiq_throttling_enabled do
-            = f.check_box :sidekiq_throttling_enabled
-            Enable Sidekiq Job Throttling
-          .help-block
-            Limit the amount of resources slow running jobs are assigned.
-    .form-group
-      = f.label :sidekiq_throttling_queues, 'Sidekiq queues to throttle', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.select :sidekiq_throttling_queues, sidekiq_queue_options_for_select, { include_hidden: false }, multiple: true, class: 'select2 select-wide', data: { field: 'sidekiq_throttling_queues' }
-        .help-block
-          Choose which queues you wish to throttle.
-    .form-group
-      = f.label :sidekiq_throttling_factor, 'Throttling Factor', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :sidekiq_throttling_factor, class: 'form-control', min: '0.01', max: '0.99', step: '0.01'
-        .help-block
-          The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive.
-
-  %fieldset
-    %legend Spam and Anti-bot Protection
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :recaptcha_enabled do
-            = f.check_box :recaptcha_enabled
-            Enable reCAPTCHA
-          %span.help-block#recaptcha_help_block Helps prevent bots from creating accounts
-
-    .form-group
-      = f.label :recaptcha_site_key, 'reCAPTCHA Site Key', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.text_field :recaptcha_site_key, class: 'form-control'
-        .help-block
-          Generate site and private keys at
-          %a{ href: 'http://www.google.com/recaptcha', target: 'blank' } http://www.google.com/recaptcha
-
-    .form-group
-      = f.label :recaptcha_private_key, 'reCAPTCHA Private Key', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.text_field :recaptcha_private_key, class: 'form-control'
-
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :akismet_enabled do
-            = f.check_box :akismet_enabled
-            Enable Akismet
-          %span.help-block#akismet_help_block Helps prevent bots from creating issues
-
-    .form-group
-      = f.label :akismet_api_key, 'Akismet API Key', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.text_field :akismet_api_key, class: 'form-control'
-        .help-block
-          Generate API key at
-          %a{ href: 'http://www.akismet.com', target: 'blank' } http://www.akismet.com
-
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :unique_ips_limit_enabled do
-            = f.check_box :unique_ips_limit_enabled
-            Limit sign in from multiple ips
-          %span.help-block#unique_ip_help_block
-            Helps prevent malicious users hide their activity
-
-    .form-group
-      = f.label :unique_ips_limit_per_user, 'IPs per user', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :unique_ips_limit_per_user, class: 'form-control'
-        .help-block
-          Maximum number of unique IPs per user
-
-    .form-group
-      = f.label :unique_ips_limit_time_window, 'IP expiration time', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :unique_ips_limit_time_window, class: 'form-control'
-        .help-block
-          How many seconds an IP will be counted towards the limit
-
-  %fieldset
     %legend Abuse reports
     .form-group
       = f.label :admin_notification_email, 'Abuse reports notification email', class: 'control-label col-sm-2'
diff --git a/app/views/admin/application_settings/_performance_bar.html.haml b/app/views/admin/application_settings/_performance_bar.html.haml
new file mode 100644
index 00000000000..5344f030c97
--- /dev/null
+++ b/app/views/admin/application_settings/_performance_bar.html.haml
@@ -0,0 +1,16 @@
+= form_for @application_setting, url: admin_application_settings_path, html: { class: 'form-horizontal fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :performance_bar_enabled do
+            = f.check_box :performance_bar_enabled
+            Enable the Performance Bar
+    .form-group
+      = f.label :performance_bar_allowed_group_id, 'Allowed group', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :performance_bar_allowed_group_id, class: 'form-control', placeholder: 'my-org/my-group', value: @application_setting.performance_bar_allowed_group&.full_path
+
+  = f.submit 'Save changes', class: "btn btn-success"
diff --git a/app/views/admin/application_settings/_spam.html.haml b/app/views/admin/application_settings/_spam.html.haml
new file mode 100644
index 00000000000..25e89097dfe
--- /dev/null
+++ b/app/views/admin/application_settings/_spam.html.haml
@@ -0,0 +1,65 @@
+= form_for @application_setting, url: admin_application_settings_path, html: { class: 'form-horizontal fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :recaptcha_enabled do
+            = f.check_box :recaptcha_enabled
+            Enable reCAPTCHA
+          %span.help-block#recaptcha_help_block Helps prevent bots from creating accounts
+
+    .form-group
+      = f.label :recaptcha_site_key, 'reCAPTCHA Site Key', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :recaptcha_site_key, class: 'form-control'
+        .help-block
+          Generate site and private keys at
+          %a{ href: 'http://www.google.com/recaptcha', target: 'blank' } http://www.google.com/recaptcha
+
+    .form-group
+      = f.label :recaptcha_private_key, 'reCAPTCHA Private Key', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :recaptcha_private_key, class: 'form-control'
+
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :akismet_enabled do
+            = f.check_box :akismet_enabled
+            Enable Akismet
+          %span.help-block#akismet_help_block Helps prevent bots from creating issues
+
+    .form-group
+      = f.label :akismet_api_key, 'Akismet API Key', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :akismet_api_key, class: 'form-control'
+        .help-block
+          Generate API key at
+          %a{ href: 'http://www.akismet.com', target: 'blank' } http://www.akismet.com
+
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :unique_ips_limit_enabled do
+            = f.check_box :unique_ips_limit_enabled
+            Limit sign in from multiple ips
+          %span.help-block#unique_ip_help_block
+            Helps prevent malicious users hide their activity
+
+    .form-group
+      = f.label :unique_ips_limit_per_user, 'IPs per user', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :unique_ips_limit_per_user, class: 'form-control'
+        .help-block
+          Maximum number of unique IPs per user
+
+    .form-group
+      = f.label :unique_ips_limit_time_window, 'IP expiration time', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :unique_ips_limit_time_window, class: 'form-control'
+        .help-block
+          How many seconds an IP will be counted towards the limit
+
+  = f.submit 'Save changes', class: "btn btn-success"
diff --git a/app/views/admin/application_settings/show.html.haml b/app/views/admin/application_settings/show.html.haml
index 17f2f37d24e..d0e612e62e5 100644
--- a/app/views/admin/application_settings/show.html.haml
+++ b/app/views/admin/application_settings/show.html.haml
@@ -7,7 +7,7 @@
   .settings-header
     %h4
       = _('Visibility and access controls')
-    %button.btn.js-settings-toggle
+    %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
     %p
       = _('Set default and restrict visibility levels. Configure import sources and git access protocol.')
@@ -18,7 +18,7 @@
   .settings-header
     %h4
       = _('Account and limit settings')
-    %button.btn.js-settings-toggle
+    %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
     %p
       = _('Session expiration, projects limit and attachment size.')
@@ -29,7 +29,7 @@
   .settings-header
     %h4
       = _('Sign-up restrictions')
-    %button.btn.js-settings-toggle
+    %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
     %p
       = _('Configure the way a user creates a new account.')
@@ -40,7 +40,7 @@
   .settings-header
     %h4
       = _('Sign-in restrictions')
-    %button.btn.js-settings-toggle
+    %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
     %p
       = _('Set requirements for a user to sign-in. Enable mandatory two-factor authentication.')
@@ -51,7 +51,7 @@
   .settings-header
     %h4
       = _('Help page')
-    %button.btn.js-settings-toggle
+    %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
     %p
       = _('Help page text and support page url.')
@@ -62,7 +62,7 @@
   .settings-header
     %h4
       = _('Pages')
-    %button.btn.js-settings-toggle
+    %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
     %p
       = _('Size and domain settings for static websites')
@@ -73,7 +73,7 @@
   .settings-header
     %h4
       = _('Continuous Integration and Deployment')
-    %button.btn.js-settings-toggle
+    %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
     %p
       = _('Auto DevOps, runners amd job artifacts')
@@ -84,7 +84,7 @@
   .settings-header
     %h4
       = _('Metrics - Influx')
-    %button.btn.js-settings-toggle
+    %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
     %p
       = _('Enable and configure InfluxDB metrics.')
@@ -95,12 +95,46 @@
   .settings-header
     %h4
       = _('Metrics - Prometheus')
-    %button.btn.js-settings-toggle
+    %button.btn.js-settings-toggle{ type: 'button' }
       = expanded ? 'Collapse' : 'Expand'
     %p
       = _('Enable and configure Prometheus metrics.')
   .settings-content
     = render 'prometheus'
 
+%section.settings.as-performance.no-animate#js-performance-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Profiling - Performance bar')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      = _('Enable the Performance Bar for a given group.')
+      = link_to icon('question-circle'), help_page_path('administration/monitoring/performance/performance_bar')
+  .settings-content
+    = render 'performance_bar'
+
+%section.settings.as-background.no-animate#js-background-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Background jobs')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      = _('Configure Sidekiq job throttling.')
+  .settings-content
+    = render 'background_jobs'
+
+%section.settings.as-spam.no-animate#js-spam-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Spam and Anti-bot Protection')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      = _('Enable reCAPTCHA or Akismet and set IP limits.')
+  .settings-content
+    = render 'spam'
+
 .prepend-top-20
   = render 'form'
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index c89bc54cad4..3005d74c3cf 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -120,6 +120,53 @@ feature 'Admin updates settings' do
     expect(page).to have_content "Application settings saved successfully"
   end
 
+  scenario 'Change Performance bar settings' do
+    group = create(:group)
+
+    page.within('.as-performance') do
+      check 'Enable the Performance Bar'
+      fill_in 'Allowed group', with: group.path
+      click_on 'Save changes'
+    end
+
+    expect(page).to have_content "Application settings saved successfully"
+    expect(find_field('Enable the Performance Bar')).to be_checked
+    expect(find_field('Allowed group').value).to eq group.path
+
+    page.within('.as-performance') do
+      uncheck 'Enable the Performance Bar'
+      click_on 'Save changes'
+    end
+
+    expect(page).to have_content 'Application settings saved successfully'
+    expect(find_field('Enable the Performance Bar')).not_to be_checked
+    expect(find_field('Allowed group').value).to be_nil
+  end
+
+  scenario 'Change Background jobs settings' do
+    page.within('.as-background') do
+      fill_in 'Throttling Factor', with: 1
+      click_button 'Save changes'
+    end
+
+    expect(Gitlab::CurrentSettings.sidekiq_throttling_factor).to eq(1)
+    expect(page).to have_content "Application settings saved successfully"
+  end
+
+  scenario 'Change Spam settings' do
+    page.within('.as-spam') do
+      check 'Enable reCAPTCHA'
+      fill_in 'reCAPTCHA Site Key', with: 'key'
+      fill_in 'reCAPTCHA Private Key', with: 'key'
+      fill_in 'IPs per user', with: 15
+      click_button 'Save changes'
+    end
+
+    expect(page).to have_content "Application settings saved successfully"
+    expect(Gitlab::CurrentSettings.recaptcha_enabled).to be true
+    expect(Gitlab::CurrentSettings.unique_ips_limit_per_user).to eq(15)
+  end
+
   scenario 'Change Slack Notifications Service template settings' do
     first(:link, 'Service Templates').click
     click_link 'Slack notifications'
@@ -172,29 +219,6 @@ feature 'Admin updates settings' do
     expect(find_field('ED25519 SSH keys').value).to eq(forbidden)
   end
 
-  scenario 'Change Performance Bar settings' do
-    group = create(:group)
-
-    check 'Enable the Performance Bar'
-    fill_in 'Allowed group', with: group.path
-
-    click_on 'Save'
-
-    expect(page).to have_content 'Application settings saved successfully'
-
-    expect(find_field('Enable the Performance Bar')).to be_checked
-    expect(find_field('Allowed group').value).to eq group.path
-
-    uncheck 'Enable the Performance Bar'
-
-    click_on 'Save'
-
-    expect(page).to have_content 'Application settings saved successfully'
-
-    expect(find_field('Enable the Performance Bar')).not_to be_checked
-    expect(find_field('Allowed group').value).to be_nil
-  end
-
   def check_all_events
     page.check('Active')
     page.check('Push')