Merge branch 'tc-cleanup-application-settings-spec-ee' into 'master'

CE -> EE port: cleanup application settings spec ee

See merge request !1498

app/controllers/admin/application_settings_controller.rb

@@ -52,16 +52,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   end
 
   def application_setting_params
-    restricted_levels = params[:application_setting][:restricted_visibility_levels]
-
-    if restricted_levels.nil?
-      params[:application_setting][:restricted_visibility_levels] = []
-    else
-      restricted_levels.map! do |level|
-        level.to_i
-      end
-    end
-
     import_sources = params[:application_setting][:import_sources]
 
     if import_sources.nil?

app/models/application_setting.rb

@@ -262,7 +262,8 @@ class ApplicationSetting < ActiveRecord::Base
       elasticsearch_aws: false,
       elasticsearch_aws_region: ENV['ELASTIC_REGION'] || 'us-east-1',
       usage_ping_enabled: true,
-      minimum_mirror_sync_time: Gitlab::Mirror::FIFTEEN
+      minimum_mirror_sync_time: Gitlab::Mirror::FIFTEEN,
+      repository_size_limit: 0
     }
   end
 

app/services/application_settings/update_service.rb

@@ -3,7 +3,7 @@ module ApplicationSettings
     def execute
       # Repository size limit comes as MB from the view
       limit = @params.delete(:repository_size_limit)
-      @application_setting.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      @application_setting.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
 
       @application_setting.update(@params)
     end

app/services/groups/create_service.rb

@@ -15,7 +15,7 @@ module Groups
 
       # Repository size limit comes as MB from the view
       limit = params.delete(:repository_size_limit)
-      @group.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      @group.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
 
       if @group.parent && !can?(current_user, :admin_group, @group.parent)
         @group.parent = nil

app/services/groups/update_service.rb

@@ -14,7 +14,7 @@ module Groups
 
       # Repository size limit comes as MB from the view
       limit = @params.delete(:repository_size_limit)
-      group.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      group.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
 
       group.assign_attributes(params)
 

app/services/projects/create_service.rb

@@ -147,7 +147,7 @@ module Projects
 
     def set_repository_size_limit_as_bytes
       limit = params.delete(:repository_size_limit)
-      @project.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      @project.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
     end
 
     def set_project_name_from_path

app/services/projects/update_service.rb

@@ -15,7 +15,7 @@ module Projects
 
       # Repository size limit comes as MB from the view
       limit = params.delete(:repository_size_limit)
-      project.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      project.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
 
       new_branch = params.delete(:default_branch)
       new_repository_storage = params.delete(:repository_storage)

lib/gitlab/utils.rb

@@ -21,5 +21,11 @@ module Gitlab
 
       nil
     end
+
+    def try_megabytes_to_bytes(size)
+      Integer(size).megabytes
+    rescue ArgumentError
+      size
+    end
   end
 end

spec/controllers/admin/application_settings_controller_spec.rb

@@ -3,8 +3,6 @@ require 'spec_helper'
 describe Admin::ApplicationSettingsController do
   include StubENV
 
-  let(:group) { create(:group) }
-  let(:project) { create(:project, namespace: group) }
   let(:admin) { create(:admin) }
   let(:user) { create(:user)}
 
@@ -17,28 +15,54 @@ describe Admin::ApplicationSettingsController do
       sign_in(admin)
     end
 
-    context 'with valid params' do
-      subject { put :update, application_setting: { repository_size_limit: '100' } }
+    it 'updates the default_project_visibility for string value' do
+      put :update, application_setting: { default_project_visibility: "20" }
 
-      it 'redirect to application settings page' do
-        is_expected.to redirect_to(admin_application_settings_path)
+      expect(response).to redirect_to(admin_application_settings_path)
+      expect(ApplicationSetting.current.default_project_visibility).to eq(Gitlab::VisibilityLevel::PUBLIC)
     end
 
-      it 'set flash notice' do
-        is_expected.to set_flash[:notice].to('Application settings saved successfully')
+    it 'update the restricted levels for string values' do
+      put :update, application_setting: { restricted_visibility_levels: %w[10 20] }
+
+      expect(response).to redirect_to(admin_application_settings_path)
+      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: {}
+
+      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
 
-    context 'with invalid params' do
-      subject! { put :update, application_setting: { repository_size_limit: '-100' } }
+    it 'updates repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: '100' }
 
-      it 'render show template' do
-        is_expected.to render_template(:show)
+      expect(response).to redirect_to(admin_application_settings_path)
+      expect(response).to set_flash[:notice].to('Application settings saved successfully')
     end
 
-      it 'assigned @application_settings has errors' do
+    it 'does not accept negative repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: '-100' }
+
+      expect(response).to render_template(:show)
       expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
     end
+
+    it 'does not accept invalid repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: 'one thousand' }
+
+      expect(response).to render_template(:show)
+      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
+    end
+
+    it 'does not accept empty repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: '' }
+
+      expect(response).to render_template(:show)
+      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
     end
   end