From a8ffc02d06a12325f07f7cb7a8ff87119c3d3d0c Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Tue, 26 Feb 2019 14:22:02 +0000
Subject: [PATCH] PERF: Reduce N+1s on theme admin page

---
 app/controllers/admin/themes_controller.rb |  1 +
 app/models/color_scheme.rb                 |  6 +++--
 app/models/theme.rb                        |  3 ++-
 app/models/theme_field.rb                  | 27 +++++++++++-----------
 spec/models/theme_field_spec.rb            |  7 +++---
 5 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb
index edf6fa470d6..b491f1699ae 100644
--- a/app/controllers/admin/themes_controller.rb
+++ b/app/controllers/admin/themes_controller.rb
@@ -107,6 +107,7 @@ class Admin::ThemesController < Admin::AdminController
                                           :remote_theme,
                                           :theme_settings,
                                           :settings_field,
+                                          # :locale_fields, # Fails due to https://github.com/rails/rails/issues/34456
                                           :user,
                                           :color_scheme,
                                           theme_fields: :upload
diff --git a/app/models/color_scheme.rb b/app/models/color_scheme.rb
index 526b2a0aaf7..d954c0b0811 100644
--- a/app/models/color_scheme.rb
+++ b/app/models/color_scheme.rb
@@ -142,13 +142,15 @@ class ColorScheme < ActiveRecord::Base
   @mutex = Mutex.new
 
   def self.base_colors
+    return @base_colors if @base_colors
     @mutex.synchronize do
       return @base_colors if @base_colors
-      @base_colors = {}
+      base_colors = {}
       File.readlines(BASE_COLORS_FILE).each do |line|
         matches = /\$([\w]+):\s*#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})(?:[;]|\s)/.match(line.strip)
-        @base_colors[matches[1]] = matches[2] if matches
+        base_colors[matches[1]] = matches[2] if matches
       end
+      @base_colors = base_colors
     end
     @base_colors
   end
diff --git a/app/models/theme.rb b/app/models/theme.rb
index 46bd55fde9b..6cef53eead1 100644
--- a/app/models/theme.rb
+++ b/app/models/theme.rb
@@ -23,6 +23,7 @@ class Theme < ActiveRecord::Base
   belongs_to :remote_theme, autosave: true
 
   has_one :settings_field, -> { where(target_id: Theme.targets[:settings], name: "yaml") }, class_name: 'ThemeField'
+  has_many :locale_fields, -> { filter_locale_fields(I18n.fallbacks[I18n.locale]) }, class_name: 'ThemeField'
 
   validate :component_validations
 
@@ -357,7 +358,7 @@ class Theme < ActiveRecord::Base
   def translations(internal: false)
     fallbacks = I18n.fallbacks[I18n.locale]
     begin
-      data = theme_fields.find_first_locale_fields([id], fallbacks).first&.translation_data(with_overrides: false, internal: internal)
+      data = locale_fields.first&.translation_data(with_overrides: false, internal: internal, fallback_fields: locale_fields)
       return {} if data.nil?
       best_translations = {}
       fallbacks.reverse.each do |locale|
diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index 9d62dc27610..420d161bf99 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -22,22 +22,23 @@ class ThemeField < ActiveRecord::Base
       .order("theme_sort_column")
   }
 
-  scope :find_locale_fields, ->(theme_ids, locale_codes) {
-    return none unless theme_ids.present? && locale_codes.present?
+  scope :filter_locale_fields, ->(locale_codes) {
+    return none unless locale_codes.present?
 
-    find_by_theme_ids(theme_ids)
-      .where(target_id: Theme.targets[:translations], name: locale_codes)
+    where(target_id: Theme.targets[:translations], name: locale_codes)
       .joins(self.sanitize_sql_array([
-        "JOIN (
-          SELECT * FROM (VALUES #{locale_codes.map { "(?)" }.join(",")}) as Y (locale_code, locale_sort_column)
-        ) as Y ON Y.locale_code = theme_fields.name",
-        *locale_codes.map.with_index { |code, index| [code, index] }
-      ]))
-      .reorder("X.theme_sort_column", "Y.locale_sort_column")
+      "JOIN (
+        SELECT * FROM (VALUES #{locale_codes.map { "(?)" }.join(",")}) as Y (locale_code, locale_sort_column)
+      ) as Y ON Y.locale_code = theme_fields.name",
+      *locale_codes.map.with_index { |code, index| [code, index] }
+    ]))
+      .order("Y.locale_sort_column")
   }
 
   scope :find_first_locale_fields, ->(theme_ids, locale_codes) {
-    find_locale_fields(theme_ids, locale_codes)
+    find_by_theme_ids(theme_ids)
+      .filter_locale_fields(locale_codes)
+      .reorder("X.theme_sort_column", "Y.locale_sort_column")
       .select("DISTINCT ON (X.theme_sort_column) *")
   }
 
@@ -128,8 +129,8 @@ class ThemeField < ActiveRecord::Base
     ThemeTranslationParser.new(self, internal: internal).load
   end
 
-  def translation_data(with_overrides: true, internal: false)
-    fallback_fields = theme.theme_fields.find_locale_fields([theme.id], I18n.fallbacks[name])
+  def translation_data(with_overrides: true, internal: false, fallback_fields: nil)
+    fallback_fields ||= theme.theme_fields.filter_locale_fields(I18n.fallbacks[name])
 
     fallback_data = fallback_fields.each_with_index.map do |field, index|
       begin
diff --git a/spec/models/theme_field_spec.rb b/spec/models/theme_field_spec.rb
index 17718e764f7..067a2ff27a8 100644
--- a/spec/models/theme_field_spec.rb
+++ b/spec/models/theme_field_spec.rb
@@ -212,9 +212,10 @@ HTML
     let!(:en3) { ThemeField.create!(theme: theme3, target_id: Theme.targets[:translations], name: "en", value: "") }
 
     describe "scopes" do
-      it "find_locale_fields returns results in the correct order" do
-        expect(ThemeField.find_locale_fields(
-          [theme3.id, theme.id, theme2.id], ["en", "fr"]
+      it "filter_locale_fields returns results in the correct order" do
+        expect(ThemeField.find_by_theme_ids([theme3.id, theme.id, theme2.id])
+          .filter_locale_fields(
+           ["en", "fr"]
         )).to eq([en3, en1, fr1, en2, fr2])
       end