Move repository, storage, abuse settings and logging settings to expandable sections

Also reorganize application settings related to repository in more maninful sections
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

app/views/admin/application_settings/_abuse.html.haml

+= form_for @application_setting, url: admin_application_settings_path, html: { class: 'form-horizontal fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    .form-group
+      = f.label :admin_notification_email, 'Abuse reports notification email', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :admin_notification_email, class: 'form-control'
+        .help-block
+          Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.
+
+  = f.submit 'Save changes', class: "btn btn-success"

app/views/admin/application_settings/_form.html.haml

@@ -12,121 +12,6 @@
         .col-sm-10
           = f.number_field :container_registry_token_expire_delay, class: 'form-control'
 
-  %fieldset
-    %legend Abuse reports
-    .form-group
-      = f.label :admin_notification_email, 'Abuse reports notification email', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.text_field :admin_notification_email, class: 'form-control'
-        .help-block
-          Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.
-
-  %fieldset
-    %legend Error Reporting and Logging
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :sentry_enabled do
-            = f.check_box :sentry_enabled
-            Enable Sentry
-          .help-block
-            %p This setting requires a restart to take effect.
-            Sentry is an error reporting and logging tool which is currently not shipped with GitLab, get it here:
-            %a{ href: 'https://getsentry.com', target: '_blank', rel: 'noopener noreferrer' } https://getsentry.com
-
-    .form-group
-      = f.label :sentry_dsn, 'Sentry DSN', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.text_field :sentry_dsn, class: 'form-control'
-
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :clientside_sentry_enabled do
-            = f.check_box :clientside_sentry_enabled
-            Enable Clientside Sentry
-          .help-block
-            Sentry can also be used for reporting and logging clientside exceptions.
-            %a{ href: 'https://sentry.io/for/javascript/', target: '_blank', rel: 'noopener noreferrer' } https://sentry.io/for/javascript/
-
-    .form-group
-      = f.label :clientside_sentry_dsn, 'Clientside Sentry DSN', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.text_field :clientside_sentry_dsn, class: 'form-control'
-
-  %fieldset
-    %legend Repository Storage
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :hashed_storage_enabled do
-            = f.check_box :hashed_storage_enabled
-            Create new projects using hashed storage paths
-          .help-block
-            Enable immutable, hash-based paths and repository names to store repositories on disk. This prevents
-            repositories from having to be moved or renamed when the Project URL changes and may improve disk I/O performance.
-            %em (EXPERIMENTAL)
-    .form-group
-      = f.label :repository_storages, 'Storage paths for new projects', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.select :repository_storages, repository_storages_options_for_select(@application_setting.repository_storages),
-          {include_hidden: false}, multiple: true, class: 'form-control'
-        .help-block
-          Manage repository storage paths. Learn more in the
-          = succeed "." do
-            = link_to "repository storages documentation", help_page_path("administration/repository_storages")
-
-  %fieldset
-    %legend Git Storage Circuitbreaker settings
-    .form-group
-      = f.label :circuitbreaker_check_interval, _('Check interval'), class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :circuitbreaker_check_interval, class: 'form-control'
-        .help-block
-          = circuitbreaker_check_interval_help_text
-    .form-group
-      = f.label :circuitbreaker_access_retries, _('Number of access attempts'), class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :circuitbreaker_access_retries, class: 'form-control'
-        .help-block
-          = circuitbreaker_access_retries_help_text
-    .form-group
-      = f.label :circuitbreaker_storage_timeout, _('Seconds to wait for a storage access attempt'), class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :circuitbreaker_storage_timeout, class: 'form-control'
-        .help-block
-          = circuitbreaker_storage_timeout_help_text
-    .form-group
-      = f.label :circuitbreaker_failure_count_threshold, _('Maximum git storage failures'), class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :circuitbreaker_failure_count_threshold, class: 'form-control'
-        .help-block
-          = circuitbreaker_failure_count_help_text
-    .form-group
-      = f.label :circuitbreaker_failure_reset_time, _('Seconds before reseting failure information'), class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :circuitbreaker_failure_reset_time, class: 'form-control'
-        .help-block
-          = circuitbreaker_failure_reset_time_help_text
-
-  %fieldset
-    %legend Repository Checks
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :repository_checks_enabled do
-            = f.check_box :repository_checks_enabled
-            Enable Repository Checks
-          .help-block
-            GitLab will periodically run
-            %a{ href: 'https://www.kernel.org/pub/software/scm/git/docs/git-fsck.html', target: 'blank' } 'git fsck'
-            in all project and wiki repositories to look for silent disk corruption issues.
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        = link_to 'Clear all repository checks', clear_repository_check_states_admin_application_settings_path, data: { confirm: 'This will clear repository check states for ALL projects in the database. This cannot be undone. Are you sure?' }, method: :put, class: "btn btn-sm btn-remove"
-        .help-block
-          If you got a lot of false alarms from repository checks you can choose to clear all repository check information from the database.
-
   = render partial: 'elasticsearch_form', locals: { f: f }
 
   - if koding_enabled?
@@ -223,44 +108,6 @@
             By default GitLab sends emails in HTML and plain text formats so mail
             clients can choose what format to use. Disable this option if you only
             want to send emails in plain text format.
-  %fieldset
-    %legend Automatic Git repository housekeeping
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :housekeeping_enabled do
-            = f.check_box :housekeeping_enabled
-            Enable automatic repository housekeeping (git repack, git gc)
-          .help-block
-            If you keep automatic housekeeping disabled for a long time Git
-            repository access on your GitLab server will become slower and your
-            repositories will use more disk space. We recommend to always leave
-            this enabled.
-        .checkbox
-          = f.label :housekeeping_bitmaps_enabled do
-            = f.check_box :housekeeping_bitmaps_enabled
-            Enable Git pack file bitmap creation
-          .help-block
-            Creating pack file bitmaps makes housekeeping take a little longer but
-            bitmaps should accelerate 'git clone' performance.
-    .form-group
-      = f.label :housekeeping_incremental_repack_period, 'Incremental repack period', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :housekeeping_incremental_repack_period, class: 'form-control'
-        .help-block
-          Number of Git pushes after which an incremental 'git repack' is run.
-    .form-group
-      = f.label :housekeeping_full_repack_period, 'Full repack period', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :housekeeping_full_repack_period, class: 'form-control'
-        .help-block
-          Number of Git pushes after which a full 'git repack' is run.
-    .form-group
-      = f.label :housekeeping_gc_period, 'Git GC period', class: 'control-label col-sm-2'
-      .col-sm-10
-        = f.number_field :housekeeping_gc_period, class: 'form-control'
-        .help-block
-          Number of Git pushes after which 'git gc' is run.
 
   %fieldset
     %legend Gitaly Timeouts

app/views/admin/application_settings/_logging.html.haml

+= 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 :sentry_enabled do
+            = f.check_box :sentry_enabled
+            Enable Sentry
+          .help-block
+            %p This setting requires a restart to take effect.
+            Sentry is an error reporting and logging tool which is currently not shipped with GitLab, get it here:
+            %a{ href: 'https://getsentry.com', target: '_blank', rel: 'noopener noreferrer' } https://getsentry.com
+
+    .form-group
+      = f.label :sentry_dsn, 'Sentry DSN', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :sentry_dsn, class: 'form-control'
+
+    .form-group
+      .col-sm-offset-2.col-sm-10
+        .checkbox
+          = f.label :clientside_sentry_enabled do
+            = f.check_box :clientside_sentry_enabled
+            Enable Clientside Sentry
+          .help-block
+            Sentry can also be used for reporting and logging clientside exceptions.
+            %a{ href: 'https://sentry.io/for/javascript/', target: '_blank', rel: 'noopener noreferrer' } https://sentry.io/for/javascript/
+
+    .form-group
+      = f.label :clientside_sentry_dsn, 'Clientside Sentry DSN', class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :clientside_sentry_dsn, class: 'form-control'
+
+  = f.submit 'Save changes', class: "btn btn-success"

app/views/admin/application_settings/_repository_check.html.haml

+= form_for @application_setting, url: admin_application_settings_path, html: { class: 'form-horizontal fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    .sub-section
+      %h4 Repository checks
+      .form-group
+        .col-sm-offset-2.col-sm-10
+          .checkbox
+            = f.label :repository_checks_enabled do
+              = f.check_box :repository_checks_enabled
+              Enable Repository Checks
+            .help-block
+              GitLab will periodically run
+              %a{ href: 'https://www.kernel.org/pub/software/scm/git/docs/git-fsck.html', target: 'blank' } 'git fsck'
+              in all project and wiki repositories to look for silent disk corruption issues.
+      .form-group
+        .col-sm-offset-2.col-sm-10
+          = link_to 'Clear all repository checks', clear_repository_check_states_admin_application_settings_path, data: { confirm: 'This will clear repository check states for ALL projects in the database. This cannot be undone. Are you sure?' }, method: :put, class: "btn btn-sm btn-remove"
+          .help-block
+            If you got a lot of false alarms from repository checks you can choose to clear all repository check information from the database.
+
+    .sub-section
+      %h4 Housekeeping
+      .form-group
+        .col-sm-offset-2.col-sm-10
+          .checkbox
+            = f.label :housekeeping_enabled do
+              = f.check_box :housekeeping_enabled
+              Enable automatic repository housekeeping (git repack, git gc)
+            .help-block
+              If you keep automatic housekeeping disabled for a long time Git
+              repository access on your GitLab server will become slower and your
+              repositories will use more disk space. We recommend to always leave
+              this enabled.
+          .checkbox
+            = f.label :housekeeping_bitmaps_enabled do
+              = f.check_box :housekeeping_bitmaps_enabled
+              Enable Git pack file bitmap creation
+            .help-block
+              Creating pack file bitmaps makes housekeeping take a little longer but
+              bitmaps should accelerate 'git clone' performance.
+      .form-group
+        = f.label :housekeeping_incremental_repack_period, 'Incremental repack period', class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.number_field :housekeeping_incremental_repack_period, class: 'form-control'
+          .help-block
+            Number of Git pushes after which an incremental 'git repack' is run.
+      .form-group
+        = f.label :housekeeping_full_repack_period, 'Full repack period', class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.number_field :housekeeping_full_repack_period, class: 'form-control'
+          .help-block
+            Number of Git pushes after which a full 'git repack' is run.
+      .form-group
+        = f.label :housekeeping_gc_period, 'Git GC period', class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.number_field :housekeeping_gc_period, class: 'form-control'
+          .help-block
+            Number of Git pushes after which 'git gc' is run.
+
+  = f.submit 'Save changes', class: "btn btn-success"

app/views/admin/application_settings/_repository_storage.html.haml

+= form_for @application_setting, url: admin_application_settings_path, html: { class: 'form-horizontal fieldset-form' } do |f|
+  = form_errors(@application_setting)
+
+  %fieldset
+    .sub-section
+      .form-group
+        .col-sm-offset-2.col-sm-10
+          .checkbox
+            = f.label :hashed_storage_enabled do
+              = f.check_box :hashed_storage_enabled
+              Create new projects using hashed storage paths
+            .help-block
+              Enable immutable, hash-based paths and repository names to store repositories on disk. This prevents
+              repositories from having to be moved or renamed when the Project URL changes and may improve disk I/O performance.
+              %em (EXPERIMENTAL)
+      .form-group
+        = f.label :repository_storages, 'Storage paths for new projects', class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.select :repository_storages, repository_storages_options_for_select(@application_setting.repository_storages),
+            {include_hidden: false}, multiple: true, class: 'form-control'
+          .help-block
+            Manage repository storage paths. Learn more in the
+            = succeed "." do
+              = link_to "repository storages documentation", help_page_path("administration/repository_storages")
+    .sub-section
+      %h4 Circuit breaker
+      .form-group
+        = f.label :circuitbreaker_check_interval, _('Check interval'), class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.number_field :circuitbreaker_check_interval, class: 'form-control'
+          .help-block
+            = circuitbreaker_check_interval_help_text
+      .form-group
+        = f.label :circuitbreaker_access_retries, _('Number of access attempts'), class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.number_field :circuitbreaker_access_retries, class: 'form-control'
+          .help-block
+            = circuitbreaker_access_retries_help_text
+      .form-group
+        = f.label :circuitbreaker_storage_timeout, _('Seconds to wait for a storage access attempt'), class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.number_field :circuitbreaker_storage_timeout, class: 'form-control'
+          .help-block
+            = circuitbreaker_storage_timeout_help_text
+      .form-group
+        = f.label :circuitbreaker_failure_count_threshold, _('Maximum git storage failures'), class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.number_field :circuitbreaker_failure_count_threshold, class: 'form-control'
+          .help-block
+            = circuitbreaker_failure_count_help_text
+      .form-group
+        = f.label :circuitbreaker_failure_reset_time, _('Seconds before reseting failure information'), class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.number_field :circuitbreaker_failure_reset_time, class: 'form-control'
+          .help-block
+            = circuitbreaker_failure_reset_time_help_text
+
+  = f.submit 'Save changes', class: "btn btn-success"

app/views/admin/application_settings/show.html.haml

@@ -136,5 +136,49 @@
   .settings-content
     = render 'spam'
 
+%section.settings.as-abuse.no-animate#js-abuse-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Abuse reports')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      = _('Set notification email for abuse reports.')
+  .settings-content
+    = render 'abuse'
+
+%section.settings.as-logging.no-animate#js-logging-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Error Reporting and Logging')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      = _('Enable Sentry for error reporting and logging.')
+  .settings-content
+    = render 'logging'
+
+%section.settings.as-repository-storage.no-animate#js-repository-storage-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Repository storage')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      = _('Configure storage path and circuit breaker settings.')
+  .settings-content
+    = render 'repository_storage'
+
+%section.settings.as-repository-check.no-animate#js-repository-check-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Repository maintenance')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      = _('Configure automatic git checks and housekeeping on repositories.')
+  .settings-content
+    = render 'repository_check'
+
 .prepend-top-20
   = render 'form'

qa/qa.rb

@@ -90,6 +90,10 @@ module QA
       autoload :OAuth, 'qa/page/main/oauth'
     end
 
+    module Settings
+      autoload :Common, 'qa/page/settings/common'
+    end
+
     module Menu
       autoload :Main, 'qa/page/menu/main'
       autoload :Side, 'qa/page/menu/side'
@@ -150,7 +154,10 @@ module QA
     end
 
     module Admin
-      autoload :Settings, 'qa/page/admin/settings'
+      module Settings
+        autoload :RepositoryStorage, 'qa/page/admin/settings/repository_storage'
+        autoload :Main, 'qa/page/admin/settings/main'
+      end
     end
 
     module Mattermost

qa/qa/factory/settings/hashed_storage.rb

@@ -9,9 +9,11 @@ module QA
           Page::Menu::Main.act { go_to_admin_area }
           Page::Menu::Admin.act { go_to_settings }
 
-          Page::Admin::Settings.act do
-            enable_hashed_storage
-            save_settings
+          Page::Admin::Settings::Main.perform do |setting|
+            setting.expand_repository_storage do |page|
+              enable_hashed_storage
+              save_settings
+            end
           end
 
           QA::Page::Menu::Main.act { sign_out }

qa/qa/page/admin/settings.rb

-module QA
-  module Page
-    module Admin
-      class Settings < Page::Base
-        view 'app/views/admin/application_settings/_form.html.haml' do
-          element :form_actions, '.form-actions'
-          element :submit, "submit 'Save'"
-          element :repository_storage, '%legend Repository Storage'
-          element :hashed_storage,
-            'Create new projects using hashed storage paths'
-        end
-
-        def enable_hashed_storage
-          scroll_to 'legend', text: 'Repository Storage'
-          check 'Create new projects using hashed storage paths'
-        end
-
-        def save_settings
-          scroll_to '.form-actions' do
-            click_button 'Save'
-          end
-        end
-      end
-    end
-  end
-end

qa/qa/page/admin/settings/main.rb

+module QA
+  module Page
+    module Admin
+      module Settings
+        class Main < Page::Base
+          include QA::Page::Settings::Common
+
+          view 'app/views/admin/application_settings/show.html.haml' do
+            element :advanced_settings_section, 'Repository storage'
+          end
+
+          def expand_repository_storage(&block)
+            expand_section('Repository storage') do
+              RepositoryStorage.perform(&block)
+            end
+          end
+        end
+      end
+    end
+  end
+end

qa/qa/page/admin/settings/repository_storage.rb

+module QA
+  module Page
+    module Admin
+      module Settings
+        class RepositoryStorage < Page::Base
+          view 'app/views/admin/application_settings/_repository_storage.html.haml' do
+            element :submit, "submit 'Save changes'"
+            element :hashed_storage,
+              'Create new projects using hashed storage paths'
+          end
+
+          def enable_hashed_storage
+            within_repository_storage do
+              check 'Create new projects using hashed storage paths'
+            end
+          end
+
+          def save_settings
+            within_repository_storage do
+              click_button 'Save changes'
+            end
+          end
+
+          def within_repository_storage
+            page.within('.as-repository-storage') do
+              yield
+            end
+          end
+        end
+      end
+    end
+  end
+end

qa/qa/page/project/settings/common.rb

@@ -3,6 +3,8 @@ module QA
     module Project
       module Settings
         module Common
+          include QA::Page::Settings::Common
+
           def self.included(base)
             base.class_eval do
               view 'app/views/projects/edit.html.haml' do
@@ -10,24 +12,6 @@ module QA
               end
             end
           end
-
-          # Click the Expand button present in the specified section
-          #
-          # @param [String] name present in the container in the DOM
-          def expand_section(name)
-            page.within('#content-body') do
-              page.within('section', text: name) do
-                # Because it is possible to click the button before the JS toggle code is bound
-                wait(reload: false) do
-                  click_button 'Expand' unless first('button', text: 'Collapse')
-
-                  page.has_content?('Collapse')
-                end
-
-                yield if block_given?
-              end
-            end
-          end
         end
       end
     end

qa/qa/page/settings/common.rb

+module QA
+  module Page
+    module Settings
+      module Common
+        # Click the Expand button present in the specified section
+        #
+        # @param [String] name present in the container in the DOM
+        def expand_section(name)
+          page.within('#content-body') do
+            page.within('section', text: name) do
+              # Because it is possible to click the button before the JS toggle code is bound
+              wait(reload: false) do
+                click_button 'Expand' unless first('button', text: 'Collapse')
+
+                page.has_content?('Collapse')
+              end
+
+              yield if block_given?
+            end
+          end
+        end
+      end
+    end
+  end
+end

qa/spec/page/validator_spec.rb

@@ -30,7 +30,7 @@ describe QA::Page::Validator do
     let(:view) { spy('view') }
 
     before do
-      allow(QA::Page::Admin::Settings)
+      allow(QA::Page::Admin::Settings::Main)
         .to receive(:views).and_return([view])
     end