From 681af5bc4fe646a981c1d0bbbeddef00f5cee3b8 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 18 Mar 2017 02:29:25 -0700
Subject: [PATCH] Fix Error 500 when application settings are saved

Due to a Rails bug, fetching the application settings from Redis
may prevent the attribute methods from being loaded for the `ApplicationSetting`
model. More details here: https://github.com/rails/rails/issues/27348

There was also a secondary problem introduced by overriding these
association methods which caused all default visibility levels to be
set to `nil`. Before, the previous implementation allowed the string
"20" to be saved as an integer, while now a table lookup happens
before that. We fix this by enforcing the integer value in the
controller and default to PRIVATE.

Closes #29674
---
 .../admin/application_settings_controller.rb         | 12 ++++++++++++
 app/models/application_setting.rb                    |  9 +++++++++
 spec/features/admin/admin_settings_spec.rb           |  8 ++++++++
 3 files changed, 29 insertions(+)

diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 8d831ffdd70..11f69c14a5d 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -45,6 +45,18 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   end
 
   def application_setting_params
+    default_visibilities = [:default_project_visibility, :default_snippet_visibility, :default_group_visibility]
+
+    default_visibilities.each do |visibility|
+      value = params[:application_setting][visibility]
+      params[:application_setting][visibility] =
+        if value.present?
+          value.to_i
+        else
+          Gitlab::VisibilityLevel::PRIVATE
+        end
+    end
+
     restricted_levels = params[:application_setting][:restricted_visibility_levels]
     if restricted_levels.nil?
       params[:application_setting][:restricted_visibility_levels] = []
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index be632930895..671a0fe98cc 100644
--- a/app/models/application_setting.rb
+++ b/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,
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index de42ab81fac..b54ba9f5f88 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -9,6 +9,14 @@ feature 'Admin updates settings', feature: true do
     visit admin_application_settings_path
   end
 
+  scenario 'Change visibility settings' do
+    first(:radio_button, 'Public').set(true)
+    click_button 'Save'
+
+    expect(page).to have_content "Application settings saved successfully"
+    expect(ApplicationSetting.current.default_project_visibility).to eq(Gitlab::VisibilityLevel::PUBLIC)
+  end
+
   scenario 'Change application settings' do
     uncheck 'Gravatar enabled'
     fill_in 'Home page URL', with: 'https://about.gitlab.com/'