Merge branch 'sh-fix-admin-application-settings' into 'master'

Fix Error 500 when application settings are saved

Closes #29674 and #29115

See merge request !10070

app/models/application_setting.rb

@@ -163,6 +163,8 @@ class ApplicationSetting < ActiveRecord::Base
   end
 
   def self.current
+    ensure_cache_setup
+
     Rails.cache.fetch(CACHE_KEY) do
       ApplicationSetting.last
     end
@@ -176,9 +178,16 @@ class ApplicationSetting < ActiveRecord::Base
   end
 
   def self.cached
+    ensure_cache_setup
     Rails.cache.fetch(CACHE_KEY)
   end
 
+  def self.ensure_cache_setup
+    # This is a workaround for a Rails bug that causes attribute methods not
+    # to be loaded when read from cache: https://github.com/rails/rails/issues/27348
+    ApplicationSetting.define_attribute_methods
+  end
+
   def self.defaults_ce
     {
       after_sign_up_text: nil,

lib/gitlab/visibility_level.rb

@@ -96,8 +96,8 @@ module Gitlab
       end
 
       def level_value(level)
-        return string_options[level] if level.is_a? String
-        level
+        return level.to_i if level.to_i.to_s == level.to_s && string_options.key(level.to_i)
+        string_options[level] || PRIVATE
       end
 
       def string_level(level)

spec/controllers/admin/application_settings_controller_spec.rb

+require 'spec_helper'
+
+describe Admin::ApplicationSettingsController do
+  include StubENV
+
+  let(:admin) { create(:admin) }
+
+  before do
+    sign_in(admin)
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+  end
+
+  describe 'PATCH #update' do
+    it 'updates the default_project_visibility for string value' do
+      patch :update, application_setting: { default_project_visibility: "20" }
+
+      expect(response).to redirect_to(admin_application_settings_path)
+      expect(ApplicationSetting.current.default_project_visibility).to eq Gitlab::VisibilityLevel::PUBLIC
+    end
+
+    it 'falls back to default with default_project_visibility setting is omitted' do
+      patch :update, application_setting: {}
+
+      expect(response).to redirect_to(admin_application_settings_path)
+      expect(ApplicationSetting.current.default_project_visibility).to eq Gitlab::VisibilityLevel::PRIVATE
+    end
+  end
+end

spec/features/admin/admin_settings_spec.rb

@@ -9,6 +9,13 @@ feature 'Admin updates settings', feature: true do
     visit admin_application_settings_path
   end
 
+  scenario 'Change visibility settings' do
+    choose "application_setting_default_project_visibility_20"
+    click_button 'Save'
+
+    expect(page).to have_content "Application settings saved successfully"
+  end
+
   scenario 'Change application settings' do
     uncheck 'Gravatar enabled'
     fill_in 'Home page URL', with: 'https://about.gitlab.com/'

spec/lib/gitlab/visibility_level_spec.rb

+require 'spec_helper'
+
+describe Gitlab::VisibilityLevel, lib: true do
+  describe '.level_value' do
+    it 'converts "public" to integer value' do
+      expect(described_class.level_value('public')).to eq(Gitlab::VisibilityLevel::PUBLIC)
+    end
+
+    it 'converts string integer to integer value' do
+      expect(described_class.level_value('20')).to eq(20)
+    end
+
+    it 'defaults to PRIVATE when string value is not valid' do
+      expect(described_class.level_value('invalid')).to eq(Gitlab::VisibilityLevel::PRIVATE)
+    end
+
+    it 'defaults to PRIVATE when integer value is not valid' do
+      expect(described_class.level_value(100)).to eq(Gitlab::VisibilityLevel::PRIVATE)
+    end
+  end
+end