Update application settings using correct action57973-errors-in-application-settings-panel-shows-wrong-panel

Updating multiple application settings panels through
a single action causes the incorrect action to be shown
when there are errors.  Instead, make each panel action
handle both updating and display.

26 files changed, 84 insertions, 43 deletions

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index d445be0eb19..49ba2c31132 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -4,56 +4,51 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   include InternalRedirect
   before_action :set_application_setting
 
+  VALID_SETTING_PANELS = %w(show integrations repository templates
+                            ci_cd reporting metrics_and_profiling
+                            network geo preferences).freeze
+
   def show
   end
 
   def integrations
+    perform_update if submitted?
   end
 
   def repository
+    perform_update if submitted?
   end
 
   def templates
+    perform_update if submitted?
   end
 
   def ci_cd
+    perform_update if submitted?
   end
 
   def reporting
+    perform_update if submitted?
   end
 
   def metrics_and_profiling
+    perform_update if submitted?
   end
 
   def network
+    perform_update if submitted?
   end
 
   def geo
+    perform_update if submitted?
   end
 
   def preferences
+    perform_update if submitted?
   end
 
   def update
-    successful = ApplicationSettings::UpdateService
-      .new(@application_setting, current_user, application_setting_params)
-      .execute
-
-    if recheck_user_consent?
-      session[:ask_for_usage_stats_consent] = current_user.requires_usage_stats_consent?
-    end
-
-    redirect_path = referer_path(request) || admin_application_settings_path
-
-    respond_to do |format|
-      if successful
-        format.json { head :ok }
-        format.html { redirect_to redirect_path, notice: _('Application settings saved successfully') }
-      else
-        format.json { head :bad_request }
-        format.html { render :show }
-      end
-    end
+    perform_update
   end
 
   def usage_data
@@ -136,6 +131,38 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
     ]
   end
 
+  def submitted?
+    request.patch?
+  end
+
+  def perform_update
+    successful = ApplicationSettings::UpdateService
+      .new(@application_setting, current_user, application_setting_params)
+      .execute
+
+    if recheck_user_consent?
+      session[:ask_for_usage_stats_consent] = current_user.requires_usage_stats_consent?
+    end
+
+    redirect_path = referer_path(request) || admin_application_settings_path
+
+    respond_to do |format|
+      if successful
+        format.json { head :ok }
+        format.html { redirect_to redirect_path, notice: _('Application settings saved successfully') }
+      else
+        format.json { head :bad_request }
+        format.html { render_update_error }
+      end
+    end
+  end
+
+  def render_update_error
+    action = VALID_SETTING_PANELS.include?(action_name) ? action_name : :show
+
+    render action
+  end
+
   def lets_encrypt_visible_attributes
     return [] unless Feature.enabled?(:pages_auto_ssl)
 
diff --git a/app/views/admin/application_settings/_abuse.html.haml b/app/views/admin/application_settings/_abuse.html.haml
index 5f8bd799d23..ddffec32c41 100644
--- a/app/views/admin/application_settings/_abuse.html.haml
+++ b/app/views/admin/application_settings/_abuse.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-abuse-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: reporting_admin_application_settings_path(anchor: 'js-abuse-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_ci_cd.html.haml b/app/views/admin/application_settings/_ci_cd.html.haml
index b8c481df0d2..d1de4286ee7 100644
--- a/app/views/admin/application_settings/_ci_cd.html.haml
+++ b/app/views/admin/application_settings/_ci_cd.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-ci-cd-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: ci_cd_admin_application_settings_path(anchor: 'js-ci-cd-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_email.html.haml b/app/views/admin/application_settings/_email.html.haml
index 3f30c75fbb6..bd60ff0b99c 100644
--- a/app/views/admin/application_settings/_email.html.haml
+++ b/app/views/admin/application_settings/_email.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-email-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-email-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_gitaly.html.haml b/app/views/admin/application_settings/_gitaly.html.haml
index f39d5709811..1da02de0461 100644
--- a/app/views/admin/application_settings/_gitaly.html.haml
+++ b/app/views/admin/application_settings/_gitaly.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-gitaly-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-gitaly-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_help_page.html.haml b/app/views/admin/application_settings/_help_page.html.haml
index aa491c735d1..a869f1bd4df 100644
--- a/app/views/admin/application_settings/_help_page.html.haml
+++ b/app/views/admin/application_settings/_help_page.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-help-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-help-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_influx.html.haml b/app/views/admin/application_settings/_influx.html.haml
index dc5cbb8fa94..98c7a9659c3 100644
--- a/app/views/admin/application_settings/_influx.html.haml
+++ b/app/views/admin/application_settings/_influx.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-influx-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: metrics_and_profiling_admin_application_settings_path(anchor: 'js-influx-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_ip_limits.html.haml b/app/views/admin/application_settings/_ip_limits.html.haml
index 5a5681830b8..67a04fcf698 100644
--- a/app/views/admin/application_settings/_ip_limits.html.haml
+++ b/app/views/admin/application_settings/_ip_limits.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-ip-limits-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-ip-limits-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_localization.html.haml b/app/views/admin/application_settings/_localization.html.haml
index 95d016a94a5..bb4d1fa1241 100644
--- a/app/views/admin/application_settings/_localization.html.haml
+++ b/app/views/admin/application_settings/_localization.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-localization-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-localization-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_logging.html.haml b/app/views/admin/application_settings/_logging.html.haml
index 41b787515b5..f7a5c90fac8 100644
--- a/app/views/admin/application_settings/_logging.html.haml
+++ b/app/views/admin/application_settings/_logging.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-logging-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: reporting_admin_application_settings_path(anchor: 'js-logging-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_outbound.html.haml b/app/views/admin/application_settings/_outbound.html.haml
index f4bfb5af385..fde476376f1 100644
--- a/app/views/admin/application_settings/_outbound.html.haml
+++ b/app/views/admin/application_settings/_outbound.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-outbound-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-outbound-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_pages.html.haml b/app/views/admin/application_settings/_pages.html.haml
index 64e01fa2d00..96a3527f3d1 100644
--- a/app/views/admin/application_settings/_pages.html.haml
+++ b/app/views/admin/application_settings/_pages.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-pages-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-pages-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_performance.html.haml b/app/views/admin/application_settings/_performance.html.haml
index e7076e7ed2f..7821a83530f 100644
--- a/app/views/admin/application_settings/_performance.html.haml
+++ b/app/views/admin/application_settings/_performance.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-performance-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-performance-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_performance_bar.html.haml b/app/views/admin/application_settings/_performance_bar.html.haml
index f992d531ea5..865fd10c7d4 100644
--- a/app/views/admin/application_settings/_performance_bar.html.haml
+++ b/app/views/admin/application_settings/_performance_bar.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-performance-bar-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: metrics_and_profiling_admin_application_settings_path(anchor: 'js-performance-bar-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_plantuml.html.haml b/app/views/admin/application_settings/_plantuml.html.haml
index 5c2b7114426..86dc289dd7c 100644
--- a/app/views/admin/application_settings/_plantuml.html.haml
+++ b/app/views/admin/application_settings/_plantuml.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-plantuml-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: integrations_admin_application_settings_path(anchor: 'js-plantuml-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_prometheus.html.haml b/app/views/admin/application_settings/_prometheus.html.haml
index a923568e52d..4c0ff3a18e8 100644
--- a/app/views/admin/application_settings/_prometheus.html.haml
+++ b/app/views/admin/application_settings/_prometheus.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-prometheus-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: metrics_and_profiling_admin_application_settings_path(anchor: 'js-prometheus-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_realtime.html.haml b/app/views/admin/application_settings/_realtime.html.haml
index 92f0c02eb6a..8f6946534ea 100644
--- a/app/views/admin/application_settings/_realtime.html.haml
+++ b/app/views/admin/application_settings/_realtime.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-realtime-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-realtime-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_registry.html.haml b/app/views/admin/application_settings/_registry.html.haml
index 08c981db13f..77623e1495b 100644
--- a/app/views/admin/application_settings/_registry.html.haml
+++ b/app/views/admin/application_settings/_registry.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-registry-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: ci_cd_admin_application_settings_path(anchor: 'js-registry-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_repository_check.html.haml b/app/views/admin/application_settings/_repository_check.html.haml
index 925e39bc0a3..417916d8c25 100644
--- a/app/views/admin/application_settings/_repository_check.html.haml
+++ b/app/views/admin/application_settings/_repository_check.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-repository-check-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: repository_admin_application_settings_path(anchor: 'js-repository-check-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_repository_mirrors_form.html.haml b/app/views/admin/application_settings/_repository_mirrors_form.html.haml
index f2f2cd1282a..362f4a42464 100644
--- a/app/views/admin/application_settings/_repository_mirrors_form.html.haml
+++ b/app/views/admin/application_settings/_repository_mirrors_form.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-mirror-settings') do |f|
+= form_for @application_setting, url: repository_admin_application_settings_path(anchor: 'js-mirror-settings') do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_repository_storage.html.haml b/app/views/admin/application_settings/_repository_storage.html.haml
index 7a2bbfcdc4d..e5bcb180445 100644
--- a/app/views/admin/application_settings/_repository_storage.html.haml
+++ b/app/views/admin/application_settings/_repository_storage.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-repository-storage-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: repository_admin_application_settings_path(anchor: 'js-repository-storage-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_spam.html.haml b/app/views/admin/application_settings/_spam.html.haml
index 54cda531580..a34fc15acb1 100644
--- a/app/views/admin/application_settings/_spam.html.haml
+++ b/app/views/admin/application_settings/_spam.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-spam-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: reporting_admin_application_settings_path(anchor: 'js-spam-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_third_party_offers.html.haml b/app/views/admin/application_settings/_third_party_offers.html.haml
index fae5b0b965f..adde09f75e4 100644
--- a/app/views/admin/application_settings/_third_party_offers.html.haml
+++ b/app/views/admin/application_settings/_third_party_offers.html.haml
@@ -1,6 +1,6 @@
 - application_setting = local_assigns.fetch(:application_setting)
 
-= form_for application_setting, url: admin_application_settings_path(anchor: 'js-third-party-offers-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for application_setting, url: integrations_admin_application_settings_path(anchor: 'js-third-party-offers-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(application_setting)
 
   %fieldset
diff --git a/app/views/admin/application_settings/_usage.html.haml b/app/views/admin/application_settings/_usage.html.haml
index 788595877ea..d716b52be05 100644
--- a/app/views/admin/application_settings/_usage.html.haml
+++ b/app/views/admin/application_settings/_usage.html.haml
@@ -1,4 +1,4 @@
-= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-usage-settings'), html: { class: 'fieldset-form' } do |f|
+= form_for @application_setting, url: metrics_and_profiling_admin_application_settings_path(anchor: 'js-usage-settings'), html: { class: 'fieldset-form' } do |f|
   = form_errors(@application_setting)
 
   %fieldset
diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index a01003b6039..156fb6d1007 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
-    get :integrations, :repository, :templates, :ci_cd, :reporting, :metrics_and_profiling, :network, :geo, :preferences
+    match :integrations, :repository, :templates, :ci_cd, :reporting, :metrics_and_profiling, :network, :geo, :preferences, via: [:get, :patch]
   end
 
   resources :labels
diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb
index b89348b7a7e..8b83df13cbb 100644
--- a/spec/controllers/admin/application_settings_controller_spec.rb
+++ b/spec/controllers/admin/application_settings_controller_spec.rb
@@ -116,6 +116,20 @@ describe Admin::ApplicationSettingsController do
         end
       end
     end
+
+    context 'when validating' do
+      it 'renders correct action on error' do
+        put :network, params: { application_setting: { domain_blacklist_enabled: true } }
+
+        expect(subject).to render_template(:network)
+      end
+
+      it 'renders default action show' do
+        put :network, params: { application_setting: { domain_blacklist_enabled: true } }
+
+        expect(subject).to render_template(:show)
+      end
+    end
   end
 
   describe 'PUT #reset_registration_token' do