FIX: Do not use cached settings during theme compilation

We compile within a database transaction, so using a cached value from redis can cause unwanted side effects

app/controllers/admin/themes_controller.rb

@@ -270,13 +270,13 @@ class Admin::ThemesController < Admin::AdminController
     setting_name = params[:name].to_sym
     new_value = params[:value] || nil
 
-    previous_value = @theme.included_settings[setting_name]
+    previous_value = @theme.cached_settings[setting_name]
     @theme.update_setting(setting_name, new_value)
     @theme.save
 
     log_theme_setting_change(setting_name, previous_value, new_value)
 
-    updated_setting = @theme.included_settings.select { |key, val| key == setting_name }
+    updated_setting = @theme.cached_settings.select { |key, val| key == setting_name }
     render json: updated_setting, status: :ok
   end
 

app/models/theme.rb

@@ -22,6 +22,7 @@ class Theme < ActiveRecord::Base
   has_one :settings_field, -> { where(target_id: Theme.targets[:settings], name: "yaml") }, class_name: 'ThemeField'
   has_one :javascript_cache, dependent: :destroy
   has_many :locale_fields, -> { filter_locale_fields(I18n.fallbacks[I18n.locale]) }, class_name: 'ThemeField'
+  has_many :upload_fields, -> { where(type_id: ThemeField.types[:theme_upload_var]).preload(:upload) }, class_name: 'ThemeField'
 
   validate :component_validations
 
@@ -88,7 +89,8 @@ class Theme < ActiveRecord::Base
     if all_extra_js.present?
       js_compiler = ThemeJavascriptCompiler.new(id, name)
       js_compiler.append_raw_script(all_extra_js)
-      js_compiler.prepend_settings(cached_settings) if cached_settings.present?
+      settings_hash = build_settings_hash
+      js_compiler.prepend_settings(settings_hash) if settings_hash.present?
       javascript_cache || build_javascript_cache
       javascript_cache.update!(content: js_compiler.content)
     else
@@ -459,24 +461,25 @@ class Theme < ActiveRecord::Base
 
   def cached_settings
     Discourse.cache.fetch("settings_for_theme_#{self.id}", expires_in: 30.minutes) do
-      hash = {}
-      self.settings.each do |setting|
-        hash[setting.name] = setting.value
-      end
-
-      theme_uploads = {}
-      theme_fields
-        .joins(:upload)
-        .where(type_id: ThemeField.types[:theme_upload_var]).each do |field|
-
-        theme_uploads[field.name] = field.upload.url
-      end
-      hash['theme_uploads'] = theme_uploads if theme_uploads.present?
-
-      hash
+      build_settings_hash
     end
   end
 
+  def build_settings_hash
+    hash = {}
+    self.settings.each do |setting|
+      hash[setting.name] = setting.value
+    end
+
+    theme_uploads = {}
+    upload_fields.each do |field|
+      theme_uploads[field.name] = field.upload.url
+    end
+    hash['theme_uploads'] = theme_uploads if theme_uploads.present?
+
+    hash
+  end
+
   def clear_cached_settings!
     DB.after_commit do
       Discourse.cache.delete("settings_for_theme_#{self.id}")

app/models/theme_field.rb

@@ -119,7 +119,8 @@ class ThemeField < ActiveRecord::Base
       js_compiler.append_js_error(error)
     end
 
-    js_compiler.prepend_settings(theme.cached_settings) if js_compiler.content.present? && theme.cached_settings.present?
+    settings_hash = theme.build_settings_hash
+    js_compiler.prepend_settings(settings_hash) if js_compiler.content.present? && settings_hash.present?
     javascript_cache.content = js_compiler.content
     javascript_cache.save!
 

app/services/staff_action_logger.rb

@@ -222,7 +222,7 @@ class StaffActionLogger
 
   def log_theme_setting_change(setting_name, previous_value, new_value, theme, opts = {})
     raise Discourse::InvalidParameters.new(:theme) unless theme
-    raise Discourse::InvalidParameters.new(:setting_name) unless theme.included_settings.has_key?(setting_name)
+    raise Discourse::InvalidParameters.new(:setting_name) unless theme.cached_settings.has_key?(setting_name)
 
     UserHistory.create!(params(opts).merge(
       action: UserHistory.actions[:change_theme_setting],