PERF: Eradicate N+1 queries from the theme admin page

app/controllers/admin/themes_controller.rb

@@ -96,8 +96,15 @@ class Admin::ThemesController < Admin::AdminController
   end
 
   def index
-    @themes = Theme.order(:name).includes(:theme_fields, :remote_theme)
-    @color_schemes = ColorScheme.all.to_a
+    @themes = Theme.order(:name).includes(:child_themes,
+                                          :remote_theme,
+                                          :theme_settings,
+                                          :settings_field,
+                                          :user,
+                                          :color_scheme,
+                                          theme_fields: :upload
+                                          )
+    @color_schemes = ColorScheme.all.includes(:theme, color_scheme_colors: :color_scheme).to_a
     light = ColorScheme.new(name: I18n.t("color_schemes.light"))
     @color_schemes.unshift(light)
 

app/models/theme.rb

@@ -16,10 +16,12 @@ class Theme < ActiveRecord::Base
   has_many :theme_fields, dependent: :destroy
   has_many :theme_settings, dependent: :destroy
   has_many :child_theme_relation, class_name: 'ChildTheme', foreign_key: 'parent_theme_id', dependent: :destroy
-  has_many :child_themes, through: :child_theme_relation, source: :child_theme
+  has_many :child_themes, -> { order(:name) }, through: :child_theme_relation, source: :child_theme
   has_many :color_schemes
   belongs_to :remote_theme
 
+  has_one :settings_field, -> { where(target_id: Theme.targets[:settings], name: "yaml") }, class_name: 'ThemeField'
+
   validate :component_validations
 
   scope :user_selectable, ->() {
@@ -346,7 +348,7 @@ class Theme < ActiveRecord::Base
   end
 
   def settings
-    field = theme_fields.where(target_id: Theme.targets[:settings], name: "yaml").first
+    field = settings_field
     return [] unless field && field.error.nil?
 
     settings = []

app/models/theme_setting.rb

@@ -9,6 +9,7 @@ class ThemeSetting < ActiveRecord::Base
     theme.clear_cached_settings!
     theme.remove_from_cache!
     theme.theme_fields.update_all(value_baked: nil)
+    theme.theme_settings.reload
     SvgSprite.expire_cache if self.name.to_s.include?("_icon")
     CSP::Extension.clear_theme_extensions_cache! if name.to_s == CSP::Extension::THEME_SETTING
   end

app/serializers/theme_serializer.rb

@@ -75,7 +75,7 @@ class ThemeSerializer < ChildThemeSerializer
   end
 
   def child_themes
-    object.child_themes.order(:name)
+    object.child_themes
   end
 
   def settings

lib/theme_settings_manager.rb

@@ -45,7 +45,9 @@ class ThemeSettingsManager
   end
 
   def db_record
-    ThemeSetting.where(name: @name, data_type: type, theme: @theme).first
+    # theme.theme_settings will already be preloaded, so it is better to use
+    # `find` on an array, rather than make a round trip to the database
+    theme.theme_settings.to_a.find { |i| i.name.to_s == @name.to_s && i.data_type.to_s == type.to_s }
   end
 
   def has_record?

spec/components/theme_settings_manager_spec.rb

@@ -3,8 +3,8 @@ require 'theme_settings_manager'
 
 describe ThemeSettingsManager do
 
+  let(:theme) { Fabricate(:theme) }
   let(:theme_settings) do
-    theme = Fabricate(:theme)
     yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/valid_settings.yaml")
     theme.set_field(target: :settings, name: "yaml", value: yaml)
     theme.save!
@@ -37,12 +37,15 @@ describe ThemeSettingsManager do
       expect(bool_setting.value).to eq(true) # default
 
       bool_setting.value = "true"
+      theme.reload
       expect(bool_setting.value).to eq(true)
 
       bool_setting.value = "falsse" # intentionally misspelled
+      theme.reload
       expect(bool_setting.value).to eq(false)
 
       bool_setting.value = true
+      theme.reload
       expect(bool_setting.value).to eq(true)
     end
   end
@@ -51,15 +54,19 @@ describe ThemeSettingsManager do
     it "is always an integer" do
       int_setting = find_by_name(:integer_setting)
       int_setting.value = 1.6
+      theme.reload
       expect(int_setting.value).to eq(1)
 
       int_setting.value = "4.3"
+      theme.reload
       expect(int_setting.value).to eq(4)
 
       int_setting.value = "10"
+      theme.reload
       expect(int_setting.value).to eq(10)
 
       int_setting.value = "text"
+      theme.reload
       expect(int_setting.value).to eq(0)
     end
 
@@ -69,9 +76,11 @@ describe ThemeSettingsManager do
       expect { int_setting.value = 61 }.to raise_error(Discourse::InvalidParameters)
 
       int_setting.value = 60
+      theme.reload
       expect(int_setting.value).to eq(60)
 
       int_setting.value = 1
+      theme.reload
       expect(int_setting.value).to eq(1)
     end
   end
@@ -80,12 +89,15 @@ describe ThemeSettingsManager do
     it "is always a float" do
       float_setting = find_by_name(:float_setting)
       float_setting.value = 1.615
+      theme.reload
       expect(float_setting.value).to eq(1.615)
 
       float_setting.value = "3.1415"
+      theme.reload
       expect(float_setting.value).to eq(3.1415)
 
       float_setting.value = 10
+      theme.reload
       expect(float_setting.value).to eq(10)
     end
 
@@ -96,6 +108,7 @@ describe ThemeSettingsManager do
       expect { float_setting.value = "text" }.to raise_error(Discourse::InvalidParameters)
 
       float_setting.value = 9.521
+      theme.reload
       expect(float_setting.value).to eq(9.521)
     end
   end
@@ -106,9 +119,11 @@ describe ThemeSettingsManager do
       expect { string_setting.value = "a" }.to raise_error(Discourse::InvalidParameters)
 
       string_setting.value = "ab"
+      theme.reload
       expect(string_setting.value).to eq("ab")
 
       string_setting.value = "ab" * 10
+      theme.reload
       expect(string_setting.value).to eq("ab" * 10)
 
       expect { string_setting.value = ("a" * 21) }.to raise_error(Discourse::InvalidParameters)