Refactored settings 'show' panel to 'general'12307-errors-in-application-settings-geo-panel-shows-wrong-panel-ce

This keeps the routes consistent - each panel has it's own
specific route.  In addition, makes the naming more
consistent, using 'General' for the panel name.

20 files changed, 55 insertions, 48 deletions

diff --git a/app/assets/javascripts/pages/admin/application_settings/show/index.js b/app/assets/javascripts/pages/admin/application_settings/general/index.js
index 5ec9688a6e4..5ec9688a6e4 100644
--- a/app/assets/javascripts/pages/admin/application_settings/show/index.js
+++ b/app/assets/javascripts/pages/admin/application_settings/general/index.js
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 99411641874..23446741f52 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -6,15 +6,16 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   before_action :set_application_setting
   before_action :whitelist_query_limiting, only: [:usage_data]
 
-  VALID_SETTING_PANELS = %w(show integrations repository templates
+  VALID_SETTING_PANELS = %w(general integrations repository templates
                             ci_cd reporting metrics_and_profiling
                             network geo preferences).freeze
 
-  def show
+  VALID_SETTING_PANELS.each do |action|
+    define_method(action) { perform_update if submitted? }
   end
 
-  (VALID_SETTING_PANELS - %w(show)).each do |action|
-    define_method(action) { perform_update if submitted? }
+  def show
+    render :general
   end
 
   def update
@@ -141,7 +142,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   end
 
   def render_update_error
-    action = VALID_SETTING_PANELS.include?(action_name) ? action_name : :show
+    action = VALID_SETTING_PANELS.include?(action_name) ? action_name : :general
 
     render action
   end
diff --git a/app/views/admin/application_settings/_account_and_limit.html.haml b/app/views/admin/application_settings/_account_and_limit.html.haml
index 9ed4bc44aae..4358365504a 100644
--- a/app/views/admin/application_settings/_account_and_limit.html.haml
+++ b/app/views/admin/application_settings/_account_and_limit.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-account-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-account-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_diff_limits.html.haml b/app/views/admin/application_settings/_diff_limits.html.haml
index 408e569fe07..137b7281e0f 100644
--- a/app/views/admin/application_settings/_diff_limits.html.haml
+++ b/app/views/admin/application_settings/_diff_limits.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-merge-request-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-merge-request-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_external_authorization_service_form.html.haml b/app/views/admin/application_settings/_external_authorization_service_form.html.haml
index 7587ecbf9d3..73412133979 100644
--- a/app/views/admin/application_settings/_external_authorization_service_form.html.haml
+++ b/app/views/admin/application_settings/_external_authorization_service_form.html.haml
@@ -8,7 +8,7 @@
       = _('External Classification Policy Authorization')
   .settings-content
 
-    = form_for @application_setting, url: admin_application_settings_path(anchor: 'js-external-auth-settings'), html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-external-auth-settings'), html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting)
 
       %fieldset
diff --git a/app/views/admin/application_settings/_signin.html.haml b/app/views/admin/application_settings/_signin.html.haml
index 5f36358f599..0e45301b598 100644
--- a/app/views/admin/application_settings/_signin.html.haml
+++ b/app/views/admin/application_settings/_signin.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-signin-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-signin-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_signup.html.haml b/app/views/admin/application_settings/_signup.html.haml
index a0a58b811ee..7c1df78f30c 100644
--- a/app/views/admin/application_settings/_signup.html.haml
+++ b/app/views/admin/application_settings/_signup.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-signup-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-signup-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_terminal.html.haml b/app/views/admin/application_settings/_terminal.html.haml
index 49980e1e1a7..654aed54a15 100644
--- a/app/views/admin/application_settings/_terminal.html.haml
+++ b/app/views/admin/application_settings/_terminal.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-terminal-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-terminal-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_terms.html.haml b/app/views/admin/application_settings/_terms.html.haml
index ef58e9b1128..19e7ab7c99a 100644
--- a/app/views/admin/application_settings/_terms.html.haml
+++ b/app/views/admin/application_settings/_terms.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-terms-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-terms-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_visibility_and_access.html.haml b/app/views/admin/application_settings/_visibility_and_access.html.haml
index c07bafbe302..e57ef1ea18f 100644
--- a/app/views/admin/application_settings/_visibility_and_access.html.haml
+++ b/app/views/admin/application_settings/_visibility_and_access.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-visibility-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-visibility-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/show.html.haml b/app/views/admin/application_settings/general.html.haml
index 31f18ba0d56..b9f49fdc9de 100644
--- a/app/views/admin/application_settings/show.html.haml
+++ b/app/views/admin/application_settings/general.html.haml
@@ -1,5 +1,5 @@
-- breadcrumb_title _("Settings")
-- page_title _("Settings")
+- breadcrumb_title _("General")
+- page_title _("General")
 - @content_class = "limit-container-width" unless fluid_layout
 
 %section.settings.as-visibility-access.no-animate#js-visibility-settings{ class: ('expanded' if expanded_by_default?) }
@@ -90,7 +90,7 @@
     %p
       = _('Manage Web IDE features')
   .settings-content
-    = form_for @application_setting, url: admin_application_settings_path(anchor: "#js-web-ide-settings"), html: { class: 'fieldset-form' } do |f|
+    = form_for @application_setting, url: general_admin_application_settings_path(anchor: "#js-web-ide-settings"), html: { class: 'fieldset-form' } do |f|
       = form_errors(@application_setting)
 
       %fieldset
diff --git a/app/views/layouts/nav/sidebar/_admin.html.haml b/app/views/layouts/nav/sidebar/_admin.html.haml
index f76268bc29b..784fe556123 100644
--- a/app/views/layouts/nav/sidebar/_admin.html.haml
+++ b/app/views/layouts/nav/sidebar/_admin.html.haml
@@ -232,7 +232,7 @@
                 = _('Settings')
             %li.divider.fly-out-top-item
             = nav_link(path: 'application_settings#show') do
-              = link_to admin_application_settings_path, title: _('General'), class: 'qa-admin-settings-general-item' do
+              = link_to general_admin_application_settings_path, title: _('General'), class: 'qa-admin-settings-general-item' do
                 %span
                   = _('General')
             = nav_link(path: 'application_settings#integrations') do
diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index 6f9a5552564..a003ffca270 100644
--- a/config/routes/admin.rb
+++ b/config/routes/admin.rb
@@ -110,7 +110,7 @@ namespace :admin do
     put :reset_registration_token
     put :reset_health_check_token
     put :clear_repository_check_states
-    match :integrations, :repository, :templates, :ci_cd, :reporting, :metrics_and_profiling, :network, :geo, :preferences, via: [:get, :patch]
+    match :general, :integrations, :repository, :templates, :ci_cd, :reporting, :metrics_and_profiling, :network, :geo, :preferences, via: [:get, :patch]
     get :lets_encrypt_terms_of_service
   end
 
diff --git a/qa/qa/page/admin/settings/general.rb b/qa/qa/page/admin/settings/general.rb
index 93b290f7e03..150775f57d2 100644
--- a/qa/qa/page/admin/settings/general.rb
+++ b/qa/qa/page/admin/settings/general.rb
@@ -7,7 +7,7 @@ module QA
         class General < Page::Base
           include QA::Page::Settings::Common
 
-          view 'app/views/admin/application_settings/show.html.haml' do
+          view 'app/views/admin/application_settings/general.html.haml' do
             element :account_and_limit_settings
           end
 
diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb
index 4eb0545eb6c..d62e0a97609 100644
--- a/spec/controllers/admin/application_settings_controller_spec.rb
+++ b/spec/controllers/admin/application_settings_controller_spec.rb
@@ -118,32 +118,7 @@ describe Admin::ApplicationSettingsController do
     end
 
     describe 'verify panel actions' do
-      shared_examples 'renders correct panels' do
-        it 'renders correct action on error' do
-          expect_next_instance_of(ApplicationSettings::UpdateService) do |service|
-            allow(service).to receive(:execute).and_return(false)
-          end
-
-          patch action, params: { application_setting: { unused_param: true } }
-
-          expect(subject).to render_template(action)
-        end
-
-        it 'redirects to same panel on success' do
-          expect_next_instance_of(ApplicationSettings::UpdateService) do |service|
-            allow(service).to receive(:execute).and_return(true)
-          end
-
-          referer_path = public_send("#{action}_admin_application_settings_path")
-          request.env["HTTP_REFERER"] = referer_path
-
-          patch action, params: { application_setting: { unused_param: true } }
-
-          expect(subject).to redirect_to(referer_path)
-        end
-      end
-
-      (Admin::ApplicationSettingsController::VALID_SETTING_PANELS - %w(show templates geo)).each do |valid_action|
+      (Admin::ApplicationSettingsController::VALID_SETTING_PANELS - %w(templates geo)).each do |valid_action|
         it_behaves_like 'renders correct panels' do
           let(:action) { valid_action }
         end
diff --git a/spec/features/admin/admin_disables_git_access_protocol_spec.rb b/spec/features/admin/admin_disables_git_access_protocol_spec.rb
index 4c3c0904a06..bc757d72a49 100644
--- a/spec/features/admin/admin_disables_git_access_protocol_spec.rb
+++ b/spec/features/admin/admin_disables_git_access_protocol_spec.rb
@@ -76,6 +76,7 @@ describe 'Admin disables Git access protocol', :js do
   context 'with nothing disabled' do
     before do
       create(:personal_key, user: admin)
+      allow_all_protocols
     end
 
     it 'shows default SSH url and protocol selection dropdown' do
@@ -107,6 +108,10 @@ describe 'Admin disables Git access protocol', :js do
     visit project_path(project)
   end
 
+  def allow_all_protocols
+    switch_git_protocol(1)
+  end
+
   def disable_http_protocol
     switch_git_protocol(2)
   end
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index eb59de2e132..ccf38cf1bd1 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -15,7 +15,7 @@ describe 'Admin updates settings' do
 
   context 'General page' do
     before do
-      visit admin_application_settings_path
+      visit general_admin_application_settings_path
     end
 
     it 'Change visibility settings' do
diff --git a/spec/frontend/fixtures/application_settings.rb b/spec/frontend/fixtures/application_settings.rb
index 38a060580c1..afe5949ed3b 100644
--- a/spec/frontend/fixtures/application_settings.rb
+++ b/spec/frontend/fixtures/application_settings.rb
@@ -26,7 +26,7 @@ describe Admin::ApplicationSettingsController, '(JavaScript fixtures)', type: :c
   it 'application_settings/accounts_and_limit.html' do
     stub_application_setting(user_default_external: false)
 
-    get :show
+    get :general
 
     expect(response).to be_successful
   end
diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js
index ce453d7c483..c0a999cfaa6 100644
--- a/spec/javascripts/test_bundle.js
+++ b/spec/javascripts/test_bundle.js
@@ -210,7 +210,7 @@ if (process.env.BABEL_ENV === 'coverage') {
     './terminal/terminal_bundle.js',
     './users/users_bundle.js',
     './issue_show/index.js',
-    './pages/admin/application_settings/show/index.js',
+    './pages/admin/application_settings/general/index.js',
   ];
 
   describe('Uncovered files', function() {
diff --git a/spec/support/shared_examples/controllers/application_settings_shared_examples.rb b/spec/support/shared_examples/controllers/application_settings_shared_examples.rb
new file mode 100644
index 00000000000..9619451cd14
--- /dev/null
+++ b/spec/support/shared_examples/controllers/application_settings_shared_examples.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+shared_examples 'renders correct panels' do
+  it 'renders correct action on error' do
+    expect_next_instance_of(ApplicationSettings::UpdateService) do |service|
+      allow(service).to receive(:execute).and_return(false)
+    end
+
+    patch action, params: { application_setting: { unused_param: true } }
+
+    expect(subject).to render_template(action)
+  end
+
+  it 'redirects to same panel on success' do
+    expect_next_instance_of(ApplicationSettings::UpdateService) do |service|
+      allow(service).to receive(:execute).and_return(true)
+    end
+
+    referer_path = public_send("#{action}_admin_application_settings_path")
+    request.env["HTTP_REFERER"] = referer_path
+
+    patch action, params: { application_setting: { unused_param: true } }
+
+    expect(subject).to redirect_to(referer_path)
+  end
+end