Commit 37c8c97c authored by Douwe Maan's avatar Douwe Maan

Merge branch 'dz-fix-admin-import-sources' into 'master'

Don't reset application settings checkboxes when save another section

Closes #45343

See merge request gitlab-org/gitlab-ce!18366
parents a6a1b731 b6c6adfa
......@@ -57,22 +57,17 @@ 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
if params[:application_setting].key?(:enabled_oauth_sign_in_sources)
enabled_oauth_sign_in_sources = params[:application_setting].delete(:enabled_oauth_sign_in_sources)
enabled_oauth_sign_in_sources&.delete("")
enabled_oauth_sign_in_sources = params[:application_setting].delete(:enabled_oauth_sign_in_sources)
params[:application_setting][:disabled_oauth_sign_in_sources] =
AuthHelper.button_based_providers.map(&:to_s) -
Array(enabled_oauth_sign_in_sources)
params[:application_setting][:disabled_oauth_sign_in_sources] =
AuthHelper.button_based_providers.map(&:to_s) -
Array(enabled_oauth_sign_in_sources)
end
params[:application_setting][:import_sources]&.delete("")
params[:application_setting][:restricted_visibility_levels]&.delete("")
params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file]
......
......@@ -74,10 +74,12 @@ module ApplicationSettingsHelper
css_class = 'btn'
css_class << ' active' unless disabled
checkbox_name = 'application_setting[enabled_oauth_sign_in_sources][]'
name = Gitlab::Auth::OAuth::Provider.label_for(source)
label_tag(checkbox_name, class: css_class) do
check_box_tag(checkbox_name, source, !disabled,
autocomplete: 'off') + Gitlab::Auth::OAuth::Provider.label_for(source)
autocomplete: 'off',
id: name.tr(' ', '_')) + name
end
end
end
......
......@@ -24,6 +24,7 @@
- if omniauth_enabled? && button_based_providers.any?
.form-group
= f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2'
= hidden_field_tag 'application_setting[enabled_oauth_sign_in_sources][]'
.col-sm-10
.btn-group{ data: { toggle: 'buttons' } }
- oauth_providers_checkboxes.each do |source|
......
......@@ -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
......
......@@ -72,11 +72,10 @@ describe Admin::ApplicationSettingsController do
expect(ApplicationSetting.current.restricted_visibility_levels).to eq([10, 20])
end
it 'falls back to defaults when settings are omitted' do
put :update, application_setting: {}
it 'updates the restricted_visibility_levels when empty array is passed' do
put :update, application_setting: { restricted_visibility_levels: [] }
expect(response).to redirect_to(admin_application_settings_path)
expect(ApplicationSetting.current.default_project_visibility).to eq(Gitlab::VisibilityLevel::PRIVATE)
expect(ApplicationSetting.current.restricted_visibility_levels).to be_empty
end
end
......
......@@ -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'
......@@ -62,6 +85,26 @@ feature 'Admin updates settings' do
expect(page).to have_content "Application settings saved successfully"
end
scenario 'Modify oauth providers' do
expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).to be_empty
page.within('.as-signin') do
uncheck 'Google'
click_button 'Save changes'
end
expect(page).to have_content "Application settings saved successfully"
expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).to include('google_oauth2')
page.within('.as-signin') do
check "Google"
click_button 'Save changes'
end
expect(page).to have_content "Application settings saved successfully"
expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).not_to include('google_oauth2')
end
scenario 'Change Help page' do
page.within('.as-help-page') do
fill_in 'Help page text', with: 'Example text'
......@@ -211,16 +254,6 @@ feature 'Admin updates settings' do
expect(find('#service_push_channel').value).to eq '#test_channel'
end
context 'sign-in restrictions', :js do
it 'de-activates oauth sign-in source' do
page.within('.as-signin') do
find('input#application_setting_enabled_oauth_sign_in_sources_[value=gitlab]').send_keys(:return)
expect(find('.btn', text: 'GitLab.com')).not_to have_css('.active')
end
end
end
scenario 'Change Keys settings' do
page.within('.as-visibility-access') do
select 'Are forbidden', from: 'RSA SSH keys'
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment