Refactor theme fields so they support custom theme defined vars

This paves the way to allowing themes to specify uploads and so on.

app/controllers/admin/themes_controller.rb

@@ -194,7 +194,7 @@ class Admin::ThemesController < Admin::AdminController
       return unless fields = theme_params[:theme_fields]
 
       fields.each do |field|
-        @theme.set_field(field[:target], field[:name], field[:value])
+        @theme.set_field(target: field[:target], name: field[:name], value: field[:value], type_id: field[:type_id])
       end
     end
 

app/models/remote_theme.rb

@@ -1,6 +1,9 @@
 require_dependency 'git_importer'
 
 class RemoteTheme < ActiveRecord::Base
+
+  ALLOWED_FIELDS = %w{scss embedded_scss head_tag header after_header body_tag footer}
+
   has_one :theme
 
   def self.import_theme(url, user=Discourse.system_user)
@@ -44,7 +47,7 @@ class RemoteTheme < ActiveRecord::Base
     end
 
     Theme.targets.keys.each do |target|
-      Theme::ALLOWED_FIELDS.each do |field|
+      ALLOWED_FIELDS.each do |field|
         lookup =
           if field == "scss"
             "#{target}.scss"
@@ -55,7 +58,7 @@ class RemoteTheme < ActiveRecord::Base
           end
 
         value = importer["#{target}/#{lookup}"]
-        theme.set_field(target.to_sym, field, value)
+        theme.set_field(target: target.to_sym, name: field, value: value)
       end
     end
 

app/models/theme.rb

@@ -4,8 +4,6 @@ require_dependency 'stylesheet/manager'
 
 class Theme < ActiveRecord::Base
 
-  ALLOWED_FIELDS = %w{scss embedded_scss head_tag header after_header body_tag footer}
-
   @cache = DistributedCache.new('theme')
 
   belongs_to :color_scheme
@@ -206,13 +204,13 @@ class Theme < ActiveRecord::Base
     target = target.to_sym
 
     theme_ids = [self.id] + (included_themes.map(&:id) || [])
-    fields = ThemeField.where(target: [Theme.targets[target], Theme.targets[:common]])
+    fields = ThemeField.where(target_id: [Theme.targets[target], Theme.targets[:common]])
                        .where(name: name.to_s)
                        .includes(:theme)
                        .joins("JOIN (
                              SELECT #{theme_ids.map.with_index{|id,idx| "#{id} AS theme_id, #{idx} AS sort_column"}.join(" UNION ALL SELECT ")}
                             ) as X ON X.theme_id = theme_fields.theme_id")
-                       .order('sort_column, target')
+                       .order('sort_column, target_id')
     fields.each(&:ensure_baked!)
     fields
   end
@@ -229,13 +227,16 @@ class Theme < ActiveRecord::Base
     @changed_colors ||= []
   end
 
-  def set_field(target, name, value)
+  def set_field(target:, name:, value:, type: nil, type_id: nil)
     name = name.to_s
 
     target_id = Theme.targets[target.to_sym]
     raise "Unknown target #{target} passed to set field" unless target_id
 
-    field = theme_fields.find{|f| f.name==name && f.target == target_id}
+    type_id ||= type ? ThemeField.types[type.to_sym] : ThemeField.guess_type(name)
+    raise "Unknown type #{type} passed to set field" unless type_id
+
+    field = theme_fields.find{|f| f.name==name && f.target_id == target_id && f.type_id == type_id}
     if field
       if value.blank?
         theme_fields.delete field.destroy
@@ -246,7 +247,7 @@ class Theme < ActiveRecord::Base
         end
       end
     else
-      theme_fields.build(target: target_id, value: value, name: name) if value.present?
+      theme_fields.build(target_id: target_id, value: value, name: name, type_id: type_id) if value.present?
     end
   end
 

app/models/theme_field.rb

@@ -1,5 +1,17 @@
 class ThemeField < ActiveRecord::Base
 
+  def self.types
+    @types ||= Enum.new(html: 0,
+                        scss: 1,
+                        theme_upload_var: 2,
+                        theme_color_var: 3,
+                        theme_var: 4)
+  end
+
+  def self.theme_var_type_ids
+    @theme_var_type_ids ||= [2,3,4]
+  end
+
   COMPILER_VERSION = 5
 
   belongs_to :theme
@@ -60,13 +72,20 @@ COMPILED
     [doc.to_s, errors&.join("\n")]
   end
 
+  def self.guess_type(name)
+    if html_fields.include?(name.to_s)
+      types[:html]
+    elsif scss_fields.include?(name.to_s)
+      types[:scss]
+    end
+  end
 
   def self.html_fields
-    %w(body_tag head_tag header footer after_header)
+    @html_fields ||= %w(body_tag head_tag header footer after_header)
   end
 
   def self.scss_fields
-    %w(scss embedded_scss)
+    @scss_fields ||= %w(scss embedded_scss)
   end
 
 
@@ -105,7 +124,7 @@ COMPILED
   end
 
   def target_name
-    Theme.targets.invert[target].to_s
+    Theme.targets.invert[target_id].to_s
   end
 
   before_save do
@@ -132,16 +151,18 @@ end
 #
 #  id               :integer          not null, primary key
 #  theme_id         :integer          not null
-#  target           :integer          not null
+#  target_id        :integer          not null
-#  name             :string           not null
+#  name             :string(30)       not null
 #  value            :text             not null
 #  value_baked      :text
 #  created_at       :datetime
 #  updated_at       :datetime
 #  compiler_version :integer          default(0), not null
 #  error            :string
+#  upload_id        :integer
+#  type_id          :integer          default(0), not null
 #
 # Indexes
 #
-#  index_theme_fields_on_theme_id_and_target_and_name  (theme_id,target,name) UNIQUE
+#  theme_field_unique_index  (theme_id,target_id,type_id,name) UNIQUE
 #

app/serializers/theme_serializer.rb

@@ -1,8 +1,8 @@
 class ThemeFieldSerializer < ApplicationSerializer
-  attributes :name, :target, :value, :error
+  attributes :name, :target, :value, :error, :type_id
 
   def target
-    case object.target
+    case object.target_id
     when 0 then "common"
     when 1 then "desktop"
     when 2 then "mobile"

db/migrate/20170501191912_add_upload_id_to_theme_fields.rb

@@ -0,0 +1,21 @@
+class AddUploadIdToThemeFields < ActiveRecord::Migration
+  def up
+    remove_index :theme_fields, [:theme_id, :target, :name]
+    rename_column :theme_fields, :target, :target_id
+    change_column :theme_fields, :name, :string, null: false, limit: 30
+
+    add_column :theme_fields, :upload_id, :integer
+    add_column :theme_fields, :type_id, :integer, null: false, default: 0
+
+    add_index :theme_fields, [:theme_id, :target_id, :type_id, :name], unique: true, name: 'theme_field_unique_index'
+    execute "UPDATE theme_fields SET type_id = 1 WHERE name IN ('scss', 'embedded_scss')"
+  end
+
+  def down
+    execute 'drop index theme_field_unique_index'
+    rename_column :theme_fields, :target_id, :target
+    remove_column :theme_fields, :upload_id
+    remove_column :theme_fields, :type_id
+    add_index :theme_fields, [:theme_id, :target, :name], unique: true
+  end
+end

lib/stylesheet/importer.rb

@@ -39,6 +39,11 @@ module Stylesheet
       colors.each do |n, hex|
         contents << "$#{n}: ##{hex} !default;\n"
       end
+      theme&.theme_fields&.where(type_id: ThemeField.theme_var_type_ids)&.each do |field|
+        escaped = field.value.gsub('"', "\\22")
+        escaped.gsub!("\n", "\\A")
+        contents << "$#{field.name}: unquote(\"#{escaped}\");\n"
+      end
       Import.new("theme_variable.scss", source: contents)
     end
 
@@ -105,7 +110,10 @@ COMMENT
     end
 
     def theme
-      @theme ||= Theme.find(@theme_id)
+      unless @theme
+        @theme = (@theme_id && Theme.find(@theme_id)) || :nil
+      end
+      @theme == :nil ? nil : @theme
     end
 
     def apply_cdn(url)

spec/components/stylesheet/manager_spec.rb

@@ -20,10 +20,10 @@ describe Stylesheet::Manager do
       user_id: -1
     )
 
-    theme.set_field(:common, "scss", ".common{.scss{color: red;}}")
+    theme.set_field(target: :common, name: "scss", value: ".common{.scss{color: red;}}")
-    theme.set_field(:desktop, "scss", ".desktop{.scss{color: red;}}")
+    theme.set_field(target: :desktop, name: "scss", value: ".desktop{.scss{color: red;}}")
-    theme.set_field(:mobile, "scss", ".mobile{.scss{color: red;}}")
+    theme.set_field(target: :mobile, name: "scss", value: ".mobile{.scss{color: red;}}")
-    theme.set_field(:common, "embedded_scss", ".embedded{.scss{color: red;}}")
+    theme.set_field(target: :common, name: "embedded_scss", value: ".embedded{.scss{color: red;}}")
 
     theme.save!
 
@@ -33,10 +33,10 @@ describe Stylesheet::Manager do
       user_id: -1,
     )
 
-    child_theme.set_field(:common, "scss", ".child_common{.scss{color: red;}}")
+    child_theme.set_field(target: :common, name: "scss", value: ".child_common{.scss{color: red;}}")
-    child_theme.set_field(:desktop, "scss", ".child_desktop{.scss{color: red;}}")
+    child_theme.set_field(target: :desktop, name: "scss", value: ".child_desktop{.scss{color: red;}}")
-    child_theme.set_field(:mobile, "scss", ".child_mobile{.scss{color: red;}}")
+    child_theme.set_field(target: :mobile, name: "scss", value: ".child_mobile{.scss{color: red;}}")
-    child_theme.set_field(:common, "embedded_scss", ".child_embedded{.scss{color: red;}}")
+    child_theme.set_field(target: :common, name: "embedded_scss", value: ".child_embedded{.scss{color: red;}}")
     child_theme.save!
 
     theme.add_child_theme!(child_theme)
@@ -55,7 +55,7 @@ describe Stylesheet::Manager do
     expect(css).to match(/\.desktop/)
 
 
-    child_theme.set_field(:desktop, :scss, ".nothing{color: green;}")
+    child_theme.set_field(target: :desktop, name: :scss, value: ".nothing{color: green;}")
     child_theme.save!
 
     new_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.key)

spec/controllers/admin/staff_action_logs_controller_spec.rb

@@ -19,12 +19,12 @@ describe Admin::StaffActionLogsController do
   context '.diff' do
     it 'can generate diffs for theme changes' do
       theme = Theme.new(user_id: -1, name: 'bob')
-      theme.set_field(:mobile, :scss, 'body {.up}')
+      theme.set_field(target: :mobile, name: :scss, value: 'body {.up}')
-      theme.set_field(:common, :scss, 'omit-dupe')
+      theme.set_field(target: :common, name: :scss, value: 'omit-dupe')
 
       original_json = ThemeSerializer.new(theme, root: false).to_json
 
-      theme.set_field(:mobile, :scss, 'body {.down}')
+      theme.set_field(target: :mobile, name: :scss, value: 'body {.down}')
 
       record = StaffActionLogger.new(Discourse.system_user)
         .log_theme_change(original_json, theme)

spec/controllers/admin/themes_controller_spec.rb

@@ -14,8 +14,8 @@ describe Admin::ThemesController do
     context ' .index' do
       it 'returns success' do
         theme = Theme.new(name: 'my name', user_id: -1)
-        theme.set_field(:common, :scss, '.body{color: black;}')
+        theme.set_field(target: :common, name: :scss, value: '.body{color: black;}')
-        theme.set_field(:desktop, :after_header, '<b>test</b>')
+        theme.set_field(target: :desktop, name: :after_header, value: '<b>test</b>')
 
         theme.remote_theme = RemoteTheme.new(
           remote_url: 'awesome.git',
@@ -71,7 +71,7 @@ describe Admin::ThemesController do
       it 'updates a theme' do
         #focus
         theme = Theme.new(name: 'my name', user_id: -1)
-        theme.set_field(:common, :scss, '.body{color: black;}')
+        theme.set_field(target: :common, name: :scss, value: '.body{color: black;}')
         theme.save
 
         child_theme = Theme.create(name: 'my name', user_id: -1)

spec/models/remote_theme_spec.rb

@@ -67,7 +67,7 @@ JSON
 
       expect(@theme.theme_fields.length).to eq(3)
 
-      mapped = Hash[*@theme.theme_fields.map{|f| ["#{f.target}-#{f.name}", f.value]}.flatten]
+      mapped = Hash[*@theme.theme_fields.map{|f| ["#{f.target_id}-#{f.name}", f.value]}.flatten]
 
       expect(mapped["0-header"]).to eq("I AM HEADER")
       expect(mapped["1-scss"]).to eq("body {color: red;}")
@@ -101,7 +101,7 @@ JSON
       expect(scheme.name).to eq("Amazing")
       expect(scheme.colors.find_by(name: 'love').hex).to eq('eaeaea')
 
-      mapped = Hash[*@theme.theme_fields.map{|f| ["#{f.target}-#{f.name}", f.value]}.flatten]
+      mapped = Hash[*@theme.theme_fields.map{|f| ["#{f.target_id}-#{f.name}", f.value]}.flatten]
 
       expect(mapped["0-header"]).to eq("I AM UPDATED")
       expect(mapped["1-scss"]).to eq("body {color: red;}")

spec/models/theme_field_spec.rb

@@ -9,7 +9,7 @@ describe ThemeField do
    badJavaScript(;
 </script>
 HTML
-    field = ThemeField.create!(theme_id: 1, target: 0, name: "header", value: html)
+    field = ThemeField.create!(theme_id: 1, target_id: 0, name: "header", value: html)
     expect(field.error).not_to eq(nil)
     field.value = ""
     field.save!
@@ -18,7 +18,7 @@ HTML
 
   it "correctly generates errors for transpiled css" do
     css = "body {"
-    field = ThemeField.create!(theme_id: 1, target: 0, name: "scss", value: css)
+    field = ThemeField.create!(theme_id: 1, target_id: 0, name: "scss", value: css)
     field.reload
     expect(field.error).not_to eq(nil)
     field.value = "body {color: blue};"

spec/models/theme_spec.rb

@@ -26,9 +26,9 @@ describe Theme do
   it 'can support child themes' do
     child = Theme.new(name: '2', user_id: user.id)
 
-    child.set_field(:common, "header", "World")
+    child.set_field(target: :common, name: "header", value: "World")
-    child.set_field(:desktop, "header", "Desktop")
+    child.set_field(target: :desktop, name: "header", value: "Desktop")
-    child.set_field(:mobile, "header", "Mobile")
+    child.set_field(target: :mobile, name: "header", value: "Mobile")
 
     child.save!
 
@@ -36,15 +36,15 @@ describe Theme do
     expect(Theme.lookup_field(child.key, "mobile", :header)).to eq("World\nMobile")
 
 
-    child.set_field(:common, "header", "Worldie")
+    child.set_field(target: :common, name: "header", value: "Worldie")
     child.save!
 
     expect(Theme.lookup_field(child.key, :mobile, :header)).to eq("Worldie\nMobile")
 
     parent = Theme.new(name: '1', user_id: user.id)
 
-    parent.set_field(:common, "header", "Common Parent")
+    parent.set_field(target: :common, name: "header", value: "Common Parent")
-    parent.set_field(:mobile, "header", "Mobile Parent")
+    parent.set_field(target: :mobile, name: "header", value: "Mobile Parent")
 
     parent.save!
 
@@ -68,7 +68,7 @@ describe Theme do
 
   it 'should correct bad html in body_tag_baked and head_tag_baked' do
     theme = Theme.new(user_id: -1, name: "test")
-    theme.set_field(:common, "head_tag", "<b>I am bold")
+    theme.set_field(target: :common, name: "head_tag", value: "<b>I am bold")
     theme.save!
 
     expect(Theme.lookup_field(theme.key, :desktop, "head_tag")).to eq("<b>I am bold</b>")
@@ -84,7 +84,7 @@ describe Theme do
     </script>
 HTML
     theme = Theme.new(user_id: -1, name: "test")
-    theme.set_field(:common, "header", with_template)
+    theme.set_field(target: :common, name: "header", value: with_template)
     theme.save!
 
     baked = Theme.lookup_field(theme.key, :mobile, "header")
@@ -96,7 +96,7 @@ HTML
   it 'should create body_tag_baked on demand if needed' do
 
     theme = Theme.new(user_id: -1, name: "test")
-    theme.set_field(:common, :body_tag, "<b>test")
+    theme.set_field(target: :common, name: :body_tag, value: "<b>test")
     theme.save
 
     ThemeField.update_all(value_baked: nil)
@@ -106,7 +106,7 @@ HTML
 
   context "plugin api" do
     def transpile(html)
-      f = ThemeField.create!(target: Theme.targets[:mobile], theme_id: -1, name: "after_header", value: html)
+      f = ThemeField.create!(target_id: Theme.targets[:mobile], theme_id: -1, name: "after_header", value: html)
       f.value_baked
     end
 
@@ -137,6 +137,20 @@ HTML
     end
   end
 
+  context 'theme vars' do
+    it 'can generate scss based off theme vars' do
+      theme = Theme.new(name: 'theme', user_id: -1)
+      theme.set_field(target: :common, name: :scss, value: 'body {color: $magic; content: quote($content)}')
+      theme.set_field(target: :common, name: :magic, value: 'red', type: :theme_var)
+      theme.set_field(target: :common, name: :content, value: 'Sam\'s Test', type: :theme_var)
+      theme.save
+
+      scss,_map = Stylesheet::Compiler.compile('@import "theme_variables"; @import "desktop_theme"; ', "theme.scss", theme_id: theme.id)
+      expect(scss).to include("red")
+      expect(scss).to include('"Sam\'s Test"')
+    end
+  end
+
   it 'correctly caches theme keys' do
     theme = Theme.create!(name: "bob", user_id: -1)
 

spec/services/staff_action_logger_spec.rb

@@ -153,14 +153,14 @@ describe StaffActionLogger do
     it "logs updated site customizations" do
       old_json = ThemeSerializer.new(theme, root:false).to_json
 
-      theme.set_field(:common, :scss, "body{margin: 10px;}")
+      theme.set_field(target: :common, name: :scss, value: "body{margin: 10px;}")
 
       log_record = logger.log_theme_change(old_json, theme)
 
       expect(log_record.previous_value).to be_present
 
       json = ::JSON.parse(log_record.new_value)
-      expect(json['theme_fields']).to eq([{"name" => "scss", "target" => "common", "value" => "body{margin: 10px;}"}])
+      expect(json['theme_fields']).to eq([{"name" => "scss", "target" => "common", "value" => "body{margin: 10px;}", "type_id" => 1}])
     end
   end
 
@@ -171,14 +171,14 @@ describe StaffActionLogger do
 
     it "creates a new UserHistory record" do
       theme = Theme.new(name: 'Banana')
-      theme.set_field(:common, :scss, "body{margin: 10px;}")
+      theme.set_field(target: :common, name: :scss, value: "body{margin: 10px;}")
 
       log_record = logger.log_theme_destroy(theme)
       expect(log_record.previous_value).to be_present
       expect(log_record.new_value).to eq(nil)
       json = ::JSON.parse(log_record.previous_value)
 
-      expect(json['theme_fields']).to eq([{"name" => "scss", "target" => "common", "value" => "body{margin: 10px;}"}])
+      expect(json['theme_fields']).to eq([{"name" => "scss", "target" => "common", "value" => "body{margin: 10px;}", "type_id" => 1}])
     end
   end