Don't reset application settings import sources

If form does not have import sources checkboxes we should not reset
import sources to empty. This fixes issue when import sources got reset
after user modifies unrelated settings section like GitLab pages

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

3 files changed, 25 insertions, 9 deletions

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 145f74d9e59..5b1b26a0ed7 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -57,15 +57,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
 
   def application_setting_params
     params[:application_setting] ||= {}
-    import_sources = params[:application_setting][:import_sources]
-
-    if import_sources.nil?
-      params[:application_setting][:import_sources] = []
-    else
-      import_sources.map! do |source|
-        source.to_str
-      end
-    end
 
     enabled_oauth_sign_in_sources = params[:application_setting].delete(:enabled_oauth_sign_in_sources)
 
@@ -73,6 +64,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       AuthHelper.button_based_providers.map(&:to_s) -
       Array(enabled_oauth_sign_in_sources)
 
+    params[:application_setting][:import_sources]&.delete("")
     params[:application_setting][:restricted_visibility_levels]&.delete("")
     params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file]
 
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 cbc779548f6..a75dd90fe6b 100644
--- a/app/views/admin/application_settings/_visibility_and_access.html.haml
+++ b/app/views/admin/application_settings/_visibility_and_access.html.haml
@@ -32,6 +32,7 @@
     .form-group
       = f.label :import_sources, class: 'control-label col-sm-2'
       .col-sm-10
+        = hidden_field_tag 'application_setting[import_sources][]'
         - import_sources_checkboxes('import-sources-help').each do |source|
           .checkbox= source
         %span.help-block#import-sources-help
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index 846b8040be6..50d0a7abe59 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -32,6 +32,29 @@ feature 'Admin updates settings' do
     expect(find('#application_setting_visibility_level_20')).not_to be_checked
   end
 
+  scenario 'Modify import sources' do
+    expect(Gitlab::CurrentSettings.import_sources).not_to be_empty
+
+    page.within('.as-visibility-access') do
+      Gitlab::ImportSources.options.map do |name, _|
+        uncheck name
+      end
+
+      click_button 'Save changes'
+    end
+
+    expect(page).to have_content "Application settings saved successfully"
+    expect(Gitlab::CurrentSettings.import_sources).to be_empty
+
+    page.within('.as-visibility-access') do
+      check "Repo by URL"
+      click_button 'Save changes'
+    end
+
+    expect(page).to have_content "Application settings saved successfully"
+    expect(Gitlab::CurrentSettings.import_sources).to eq(['git'])
+  end
+
   scenario 'Change Visibility and Access Controls' do
     page.within('.as-visibility-access') do
       uncheck 'Project export enabled'