FEATURE: Multiple SCSS file support for themes (#7351)

Theme developers can include any number of scss files within the /scss/ directory of a theme. These can then be imported from the main common/desktop/mobile scss.

app/assets/javascripts/admin/components/admin-theme-editor.js.es6

@@ -27,6 +27,7 @@ export default Ember.Component.extend({
   @computed("currentTargetName", "fieldName")
   activeSectionMode(targetName, fieldName) {
     if (["settings", "translations"].includes(targetName)) return "yaml";
+    if (["extra_scss"].includes(targetName)) return "scss";
     return fieldName && fieldName.indexOf("scss") > -1 ? "scss" : "html";
   },
 
@@ -73,7 +74,7 @@ export default Ember.Component.extend({
 
     addField(name) {
       if (!name) return;
-      name = name.replace(/\W/g, "");
+      name = name.replace(/[^a-zA-Z0-9-_/]/g, "");
       this.get("theme").setField(this.get("currentTargetName"), name, "");
       this.setProperties({ newFieldName: "", addingField: false });
       this.fieldAdded(this.get("currentTargetName"), name);

app/assets/javascripts/admin/models/theme.js.es6

@@ -27,6 +27,13 @@ const Theme = RestModel.extend({
         icon: "globe",
         advanced: true,
         customNames: true
+      },
+      {
+        id: 5,
+        name: "extra_scss",
+        icon: "paint-brush",
+        advanced: true,
+        customNames: true
       }
     ].map(target => {
       target["edited"] = this.hasEdited(target.name);
@@ -46,6 +53,14 @@ const Theme = RestModel.extend({
       "footer"
     ];
 
+    const scss_fields = (this.get("theme_fields") || [])
+      .filter(f => f.target === "extra_scss" && f.name !== "")
+      .map(f => f.name);
+
+    if (scss_fields.length < 1) {
+      scss_fields.push("importable_scss");
+    }
+
     return {
       common: [...common, "embedded_scss"],
       desktop: common,
@@ -56,7 +71,8 @@ const Theme = RestModel.extend({
         ...(this.get("theme_fields") || [])
           .filter(f => f.target === "translations" && f.name !== "en")
           .map(f => f.name)
-      ]
+      ],
+      extra_scss: scss_fields
     };
   },
 
@@ -71,7 +87,7 @@ const Theme = RestModel.extend({
           error: this.hasError(target, fieldName)
         };
 
-        if (target === "translations") {
+        if (target === "translations" || target === "extra_scss") {
           field.translatedName = fieldName;
         } else {
           field.translatedName = I18n.t(

app/jobs/onceoff/rebake_all_html_theme_fields.rb

@@ -1,11 +0,0 @@
-module Jobs
-  class RebakeAllHtmlThemeFields < Jobs::Onceoff
-    def execute_onceoff(args)
-      ThemeField.where(type_id: ThemeField.types[:html]).find_each do |theme_field|
-        theme_field.update(value_baked: nil)
-      end
-
-      Theme.clear_cache!
-    end
-  end
-end

app/models/remote_theme.rb

@@ -124,13 +124,14 @@ class RemoteTheme < ActiveRecord::Base
     end
 
     theme_info = RemoteTheme.extract_theme_info(importer)
+    updated_fields = []
 
     theme_info["assets"]&.each do |name, relative_path|
       if path = importer.real_path(relative_path)
         new_path = "#{File.dirname(path)}/#{SecureRandom.hex}#{File.extname(path)}"
         File.rename(path, new_path) # OptimizedImage has strict file name restrictions, so rename temporarily
         upload = UploadCreator.new(File.open(new_path), File.basename(relative_path), for_theme: true).create_for(theme.user_id)
-        theme.set_field(target: :common, name: name, type: :theme_upload_var, upload_id: upload.id)
+        updated_fields << theme.set_field(target: :common, name: name, type: :theme_upload_var, upload_id: upload.id)
       end
     end
 
@@ -144,9 +145,13 @@ class RemoteTheme < ActiveRecord::Base
     importer.all_files.each do |filename|
       next unless opts = ThemeField.opts_from_file_path(filename)
       value = importer[filename]
-      theme.set_field(opts.merge(value: value))
+      updated_fields << theme.set_field(opts.merge(value: value))
     end
 
+    # Destroy fields that no longer exist in the remote theme
+    field_ids_to_destroy = theme.theme_fields.pluck(:id) - updated_fields.map(&:id)
+    ThemeField.where(id: field_ids_to_destroy).destroy_all
+
     if !skip_update
       self.remote_updated_at = Time.zone.now
       self.remote_version = importer.version

app/models/theme.rb

@@ -51,6 +51,10 @@ class Theme < ActiveRecord::Base
 
     Theme.expire_site_cache! if saved_change_to_user_selectable? || saved_change_to_name?
 
+    reload
+    settings_field&.ensure_baked! # Other fields require setting to be **baked**
+    theme_fields.each(&:ensure_baked!)
+
     remove_from_cache!
     clear_cached_settings!
     ColorScheme.hex_cache.clear
@@ -76,6 +80,8 @@ class Theme < ActiveRecord::Base
 
     Theme.expire_site_cache!
     ColorScheme.hex_cache.clear
+    CSP::Extension.clear_theme_extensions_cache!
+    SvgSprite.expire_cache
   end
 
   after_commit ->(theme) do
@@ -224,7 +230,7 @@ class Theme < ActiveRecord::Base
   end
 
   def self.targets
-    @targets ||= Enum.new(common: 0, desktop: 1, mobile: 2, settings: 3, translations: 4)
+    @targets ||= Enum.new(common: 0, desktop: 1, mobile: 2, settings: 3, translations: 4, extra_scss: 5)
   end
 
   def self.lookup_target(target_id)
@@ -267,15 +273,15 @@ class Theme < ActiveRecord::Base
 
   def self.list_baked_fields(theme_ids, target, name)
     target = target.to_sym
-    name = name.to_sym
+    name = name&.to_sym
 
     if target == :translations
       fields = ThemeField.find_first_locale_fields(theme_ids, I18n.fallbacks[name])
     else
       fields = ThemeField.find_by_theme_ids(theme_ids)
         .where(target_id: [Theme.targets[target], Theme.targets[:common]])
-        .where(name: name.to_s)
+      fields = fields.where(name: name.to_s) unless name.nil?
-        .order(:target_id)
+      fields = fields.order(:target_id)
     end
 
     fields.each(&:ensure_baked!)
@@ -325,6 +331,7 @@ class Theme < ActiveRecord::Base
           changed_fields << field
         end
       end
+      field
     else
       theme_fields.build(target_id: target_id, value: value, name: name, type_id: type_id, upload_id: upload_id) if value.present? || upload_id.present?
     end

app/models/theme_field.rb

@@ -7,11 +7,6 @@ class ThemeField < ActiveRecord::Base
   belongs_to :upload
   has_one :javascript_cache, dependent: :destroy
 
-  after_commit do |field|
-    SvgSprite.expire_cache if field.target_id == Theme.targets[:settings]
-    SvgSprite.expire_cache if field.name == SvgSprite.theme_sprite_variable_name
-  end
-
   scope :find_by_theme_ids, ->(theme_ids) {
     return none unless theme_ids.present?
 
@@ -221,18 +216,16 @@ class ThemeField < ActiveRecord::Base
     end
 
     self.error = errors.join("\n").presence
-    if !self.error && self.target_id == Theme.targets[:settings]
-      # when settings YAML changes, we need to re-transpile theme JS and CSS
-      theme.theme_fields.where.not(id: self.id).update_all(value_baked: nil)
-    end
   end
 
   def self.guess_type(name:, target:)
-    if html_fields.include?(name.to_s)
+    if basic_targets.include?(target.to_s) && html_fields.include?(name.to_s)
       types[:html]
-    elsif scss_fields.include?(name.to_s)
+    elsif basic_targets.include?(target.to_s) && scss_fields.include?(name.to_s)
       types[:scss]
-    elsif name.to_s == "yaml" || target.to_s == "translations"
+    elsif target.to_s == "extra_scss"
+      types[:scss]
+    elsif target.to_s == "settings" || target.to_s == "translations"
       types[:yaml]
     end
   end
@@ -245,46 +238,93 @@ class ThemeField < ActiveRecord::Base
     @scss_fields ||= %w(scss embedded_scss)
   end
 
+  def self.basic_targets
+    @basic_targets ||= %w(common desktop mobile)
+  end
+
+  def basic_html_field?
+    ThemeField.basic_targets.include?(Theme.targets[self.target_id].to_s) &&
+      ThemeField.html_fields.include?(self.name)
+  end
+
+  def basic_scss_field?
+    ThemeField.basic_targets.include?(Theme.targets[self.target_id].to_s) &&
+      ThemeField.scss_fields.include?(self.name)
+  end
+
+  def extra_scss_field?
+    Theme.targets[self.target_id] == :extra_scss
+  end
+
+  def settings_field?
+    Theme.targets[:settings] == self.target_id
+  end
+
+  def translation_field?
+    Theme.targets[:translations] == self.target_id
+  end
+
+  def svg_sprite_field?
+    ThemeField.theme_var_type_ids.include?(self.type_id) && self.name == SvgSprite.theme_sprite_variable_name
+  end
+
   def ensure_baked!
-    if ThemeField.html_fields.include?(self.name) || translation = Theme.targets[:translations] == self.target_id
+    needs_baking = !self.value_baked || compiler_version != COMPILER_VERSION
-      if !self.value_baked || compiler_version != COMPILER_VERSION
+    return unless needs_baking
-        self.value_baked, self.error = translation ? process_translation : process_html(self.value)
-        self.error = nil unless self.error.present?
-        self.compiler_version = COMPILER_VERSION
 
-        if self.will_save_change_to_value_baked? ||
+    if basic_html_field? || translation_field?
-           self.will_save_change_to_compiler_version? ||
+      self.value_baked, self.error = translation_field? ? process_translation : process_html(self.value)
-           self.will_save_change_to_error?
+      self.error = nil unless self.error.present?
-
+      self.compiler_version = COMPILER_VERSION
-          self.update_columns(value_baked: value_baked,
+    elsif basic_scss_field?
-                              compiler_version: compiler_version,
+      ensure_scss_compiles!
-                              error: error)
+      Stylesheet::Manager.clear_theme_cache!
-        end
+    elsif settings_field?
-      end
+      validate_yaml!
+      theme.clear_cached_settings!
+      CSP::Extension.clear_theme_extensions_cache!
+      SvgSprite.expire_cache
+      self.value_baked = "baked"
+      self.compiler_version = COMPILER_VERSION
+    elsif svg_sprite_field?
+      SvgSprite.expire_cache
+      self.error = nil
+      self.value_baked = "baked"
+      self.compiler_version = COMPILER_VERSION
     end
+
+    if self.will_save_change_to_value_baked? ||
+        self.will_save_change_to_compiler_version? ||
+        self.will_save_change_to_error?
+
+      self.update_columns(value_baked: value_baked,
+                          compiler_version: compiler_version,
+                          error: error)
+    end
+  end
+
+  def compile_scss
+    Stylesheet::Compiler.compile("@import \"common/foundation/variables\"; @import \"theme_variables\"; @import \"theme_field\";",
+      "theme.scss",
+      theme_field: self.value.dup,
+      theme: self.theme
+    )
   end
 
   def ensure_scss_compiles!
-    if ThemeField.scss_fields.include?(self.name)
+    result = ["failed"]
-      begin
+    begin
-        Stylesheet::Compiler.compile("@import \"common/foundation/variables\"; @import \"theme_variables\"; @import \"theme_field\";",
+      result = compile_scss
-                                     "theme.scss",
+      self.error = nil unless error.nil?
-                                     theme_field: self.value.dup,
+    rescue SassC::SyntaxError => e
-                                     theme: self.theme
+      self.error = e.message unless self.destroyed?
-                                    )
-        self.error = nil unless error.nil?
-      rescue SassC::SyntaxError => e
-        self.error = e.message unless self.destroyed?
-      end
-
-      if will_save_change_to_error?
-        update_columns(error: self.error)
-      end
     end
+    self.compiler_version = COMPILER_VERSION
+    self.value_baked = Digest::SHA1.hexdigest(result.join(",")) # We don't use the compiled CSS here, we just use it to invalidate the stylesheet cache
   end
 
   def target_name
-    Theme.targets.invert[target_id].to_s
+    Theme.targets[target_id].to_s
   end
 
   class ThemeFileMatcher
@@ -311,7 +351,7 @@ class ThemeField < ActiveRecord::Base
       hash = {}
       OPTIONS.each do |option|
         plural = :"#{option}s"
-        hash[option] = @allowed_values[plural][0] if @allowed_values[plural].length == 1
+        hash[option] = @allowed_values[plural][0] if @allowed_values[plural] && @allowed_values[plural].length == 1
         hash[option] = match[option] if hash[option].nil?
       end
       hash
@@ -337,6 +377,9 @@ class ThemeField < ActiveRecord::Base
     ThemeFileMatcher.new(regex: /^common\/embedded\.scss$/,
                          targets: :common, names: "embedded_scss", types: :scss,
                          canonical: -> (h) { "common/embedded.scss" }),
+    ThemeFileMatcher.new(regex: /^scss\/(?<name>.+)\.scss$/,
+                         targets: :extra_scss, names: nil, types: :scss,
+                         canonical: -> (h) { "scss/#{h[:name]}.scss" }),
     ThemeFileMatcher.new(regex: /^settings\.ya?ml$/,
                          names: "yaml", types: :yaml, targets: :settings,
                          canonical: -> (h) { "settings.yml" }),
@@ -370,24 +413,33 @@ class ThemeField < ActiveRecord::Base
     nil
   end
 
-  before_save do
+  def dependent_fields
-    validate_yaml!
+    if extra_scss_field?
+      return theme.theme_fields.where(target_id: ThemeField.basic_targets.map { |t| Theme.targets[t.to_sym] },
+                                      name: ThemeField.scss_fields)
+    elsif settings_field?
+      return theme.theme_fields.where(target_id: ThemeField.basic_targets.map { |t| Theme.targets[t.to_sym] },
+                                      name: ThemeField.scss_fields + ThemeField.html_fields)
+    end
+    ThemeField.none
+  end
 
+  def invalidate_baked!
+    update_column(:value_baked, nil)
+    dependent_fields.update_all(value_baked: nil)
+  end
+
+  before_save do
     if will_save_change_to_value? && !will_save_change_to_value_baked?
       self.value_baked = nil
     end
   end
 
+  after_save do
+    dependent_fields.each(&:invalidate_baked!)
+  end
+
   after_commit do
-    unless destroyed?
-      ensure_baked!
-      ensure_scss_compiles!
-      theme.clear_cached_settings!
-    end
-
-    Stylesheet::Manager.clear_theme_cache! if self.name.include?("scss")
-    CSP::Extension.clear_theme_extensions_cache! if name == 'yaml'
-
     # TODO message for mobile vs desktop
     MessageBus.publish "/header-change/#{theme.id}", self.value if theme && self.name == "header"
     MessageBus.publish "/footer-change/#{theme.id}", self.value if theme && self.name == "footer"

app/models/theme_setting.rb

@@ -5,13 +5,12 @@ class ThemeSetting < ActiveRecord::Base
   validates :data_type, numericality: { only_integer: true }
   validates :name, length: { maximum: 255 }
 
-  after_save do
+  after_save :clear_settings_cache
-    theme.clear_cached_settings!
+  after_destroy :clear_settings_cache
-    theme.remove_from_cache!
+
-    theme.theme_fields.update_all(value_baked: nil)
+  def clear_settings_cache
-    theme.theme_settings.reload
+    # All necessary caches will be cleared on next ensure_baked!
-    SvgSprite.expire_cache if self.name.to_s.include?("_icon")
+    theme.settings_field&.invalidate_baked!
-    CSP::Extension.clear_theme_extensions_cache! if name.to_s == CSP::Extension::THEME_SETTING
   end
 
   def self.types

config/locales/client.en.yml

@@ -3403,6 +3403,7 @@ en:
           mobile: "Mobile"
           settings: "Settings"
           translations: "Translations"
+          extra_scss: "Extra SCSS"
           preview: "Preview"
           show_advanced: "Show advanced fields"
           hide_advanced: "Hide advanced fields"

lib/stylesheet/compiler.rb

@@ -9,7 +9,7 @@ module Stylesheet
     def self.compile_asset(asset, options = {})
 
       if Importer.special_imports[asset.to_s]
-        filename = "theme.scss"
+        filename = "theme_#{options[:theme_id]}.scss"
         file = "@import \"common/foundation/variables\"; @import \"theme_variables\"; @import \"#{asset}\";"
       else
         filename = "#{asset}.scss"

lib/stylesheet/importer.rb

@@ -14,7 +14,7 @@ module Stylesheet
     end
 
     register_import "theme_field" do
-      Import.new("theme_field.scss", source: @theme_field)
+      Import.new("#{theme_dir}/theme_field.scss", source: @theme_field)
     end
 
     register_import "plugins" do
@@ -119,14 +119,14 @@ module Stylesheet
       fields.map do |field|
         value = field.value
         if value.present?
-          filename = "#{field.theme.id}/#{field.target_name}-#{field.name}-#{field.theme.name.parameterize}.scss"
+          filename = "theme_#{field.theme.id}/#{field.target_name}-#{field.name}-#{field.theme.name.parameterize}.scss"
-          with_comment = <<COMMENT
+          with_comment = <<~COMMENT
-// Theme: #{field.theme.name}
+          // Theme: #{field.theme.name}
-// Target: #{field.target_name} #{field.name}
+          // Target: #{field.target_name} #{field.name}
-// Last Edited: #{field.updated_at}
+          // Last Edited: #{field.updated_at}
 
-#{value}
+          #{value}
-COMMENT
+          COMMENT
           Import.new(filename, source: with_comment)
         end
       end.compact
@@ -139,6 +139,39 @@ COMMENT
       @theme == :nil ? nil : @theme
     end
 
+    def theme_dir
+      "theme_#{theme.id}"
+    end
+
+    def importable_theme_fields
+      return {} unless theme
+      @importable_theme_fields ||= begin
+        hash = {}
+        @theme.theme_fields.where(target_id: Theme.targets[:extra_scss]).each do |field|
+          hash[field.name] = field.value
+        end
+        hash
+      end
+    end
+
+    def match_theme_import(path, parent_path)
+      # Only allow importing theme stylesheets from within other theme stylesheets
+      return false unless theme && parent_path.start_with?("#{theme_dir}/")
+      parent_dir, _ = File.split(parent_path)
+
+      # Could be relative to the importing file, or relative to the root of the theme directory
+      search_paths = [parent_dir, theme_dir].uniq
+      search_paths.each do |search_path|
+        resolved = Pathname.new("#{search_path}/#{path}").cleanpath.to_s # Remove unnecessary ./ and ../
+        next unless resolved.start_with?("#{theme_dir}/")
+        resolved.sub!("#{theme_dir}/", "")
+        if importable_theme_fields.keys.include?(resolved)
+          return resolved
+        end
+      end
+      false
+    end
+
     def category_css(category)
       "body.category-#{category.full_slug} { background-image: url(#{upload_cdn_path(category.uploaded_background.url)}) }\n"
     end
@@ -155,6 +188,8 @@ COMMENT
         end
       elsif callback = Importer.special_imports[asset]
         instance_eval(&callback)
+      elsif resolved = match_theme_import(asset, parent_path)
+        Import.new("#{theme_dir}/#{resolved}", source: importable_theme_fields[resolved])
       else
         Import.new(asset + ".scss")
       end

lib/stylesheet/manager.rb

@@ -276,15 +276,15 @@ class Stylesheet::Manager
     scss = ""
 
     if [:mobile_theme, :desktop_theme].include?(@target)
-      scss = theme.resolve_baked_field(:common, :scss)
+      scss_digest = theme.resolve_baked_field(:common, :scss)
-      scss += theme.resolve_baked_field(@target.to_s.sub("_theme", ""), :scss)
+      scss_digest += theme.resolve_baked_field(@target.to_s.sub("_theme", ""), :scss)
     elsif @target == :embedded_theme
-      scss = theme.resolve_baked_field(:common, :embedded_scss)
+      scss_digest = theme.resolve_baked_field(:common, :embedded_scss)
     else
       raise "attempting to look up theme digest for invalid field"
     end
 
-    Digest::SHA1.hexdigest(scss.to_s + color_scheme_digest.to_s + settings_digest + plugins_digest + uploads_digest)
+    Digest::SHA1.hexdigest(scss_digest.to_s + color_scheme_digest.to_s + settings_digest + plugins_digest + uploads_digest)
   end
 
   # this protects us from situations where new versions of a plugin removed a file

lib/theme_store/tgz_exporter.rb

@@ -30,9 +30,9 @@ class ThemeStore::TgzExporter
         # Belt and braces approach here. All the user input should already be
         # sanitized, but check for attempts to leave the temp directory anyway
         pathname = Pathname.new("#{@export_name}/#{path}")
-        folder_path = pathname.parent.realdirpath
+        folder_path = pathname.parent.cleanpath
-        raise RuntimeError.new("Theme exporter tried to leave directory") unless folder_path.to_s.starts_with?("#{@temp_folder}/#{@export_name}")
+        raise RuntimeError.new("Theme exporter tried to leave directory") unless folder_path.to_s.starts_with?("#{@export_name}")
-        folder_path.mkpath
+        pathname.parent.mkpath
         path = pathname.realdirpath
         raise RuntimeError.new("Theme exporter tried to leave directory") unless path.to_s.starts_with?("#{@temp_folder}/#{@export_name}")
 

spec/components/stylesheet/importer_spec.rb

@@ -59,4 +59,52 @@ describe Stylesheet::Importer do
 
   end
 
+  context "extra_scss" do
+    let(:scss) { "body { background: red}" }
+    let(:theme) { Fabricate(:theme).tap { |t|
+      t.set_field(target: :extra_scss, name: "my_files/magic", value: scss)
+      t.save!
+    }}
+
+    let(:importer) { described_class.new(theme: theme) }
+
+    it "should be able to import correctly" do
+      # Import from regular theme file
+      expect(
+        importer.imports(
+          "my_files/magic",
+          "theme_#{theme.id}/desktop-scss-mytheme.scss"
+        ).source).to eq(scss)
+
+      # Import from some deep file
+      expect(
+        importer.imports(
+          "my_files/magic",
+          "theme_#{theme.id}/some/deep/folder/structure/myfile.scss"
+        ).source).to eq(scss)
+
+      # Import from parent dir
+      expect(
+        importer.imports(
+          "../../my_files/magic",
+          "theme_#{theme.id}/my_files/folder1/myfile.scss"
+        ).source).to eq(scss)
+
+      # Import from same dir without ./
+      expect(
+        importer.imports(
+          "magic",
+          "theme_#{theme.id}/my_files/myfile.scss"
+        ).source).to eq(scss)
+
+      # Import from same dir with ./
+      expect(
+        importer.imports(
+          "./magic",
+          "theme_#{theme.id}/my_files/myfile.scss"
+        ).source).to eq(scss)
+    end
+
+  end
+
 end

spec/components/svg_sprite/svg_sprite_spec.rb

@@ -73,19 +73,23 @@ describe SvgSprite do
 
     # Works when applying override
     theme.update_setting(:custom_icon, "gas-pump")
+    theme.save!
     expect(SvgSprite.all_icons([theme.id])).to include("gas-pump")
 
     # Works when changing override
     theme.update_setting(:custom_icon, "gamepad")
+    theme.save!
     expect(SvgSprite.all_icons([theme.id])).to include("gamepad")
     expect(SvgSprite.all_icons([theme.id])).not_to include("gas-pump")
 
     # FA5 syntax
     theme.update_setting(:custom_icon, "fab fa-bandcamp")
+    theme.save!
     expect(SvgSprite.all_icons([theme.id])).to include("fab-bandcamp")
 
     # Internal Discourse syntax + multiple icons
     theme.update_setting(:custom_icon, "fab-android|dragon")
+    theme.save!
     expect(SvgSprite.all_icons([theme.id])).to include("fab-android")
     expect(SvgSprite.all_icons([theme.id])).to include("dragon")
 
@@ -94,6 +98,7 @@ describe SvgSprite do
 
     # Check components are included
     theme.update(component: true)
+    theme.save!
     parent_theme = Fabricate(:theme)
     parent_theme.add_child_theme!(theme)
     expect(SvgSprite.all_icons([parent_theme.id])).to include("dragon")

spec/components/theme_store/tgz_exporter_spec.rb

@@ -108,7 +108,7 @@ describe ThemeStore::TgzExporter do
     # but protection is in place 'just in case'
     expect do
       theme.set_field(target: :translations, name: "en", value: "hacked")
-      theme.theme_fields[0].stubs(:file_path).returns("../../malicious")
+      ThemeField.any_instance.stubs(:file_path).returns("../../malicious")
       theme.save!
       package
     end.to raise_error(RuntimeError)

spec/jobs/rebake_all_html_theme_fields_spec.rb

@@ -1,15 +0,0 @@
-require 'rails_helper'
-
-describe Jobs::RebakeAllHtmlThemeFields do
-  let(:theme) { Fabricate(:theme) }
-  let(:theme_field) { ThemeField.create!(theme: theme, target_id: 0, name: "header", value: "<script>console.log(123)</script>") }
-
-  it 'extracts inline javascripts' do
-    theme_field.update_attributes(value_baked: 'need to be rebaked')
-
-    described_class.new.execute_onceoff({})
-
-    theme_field.reload
-    expect(theme_field.value_baked).to include('theme-javascripts')
-  end
-end

spec/models/remote_theme_spec.rb

@@ -9,7 +9,7 @@ describe RemoteTheme do
       `cd #{repo_dir} && git init . `
       `cd #{repo_dir} && git config user.email 'someone@cool.com'`
       `cd #{repo_dir} && git config user.name 'The Cool One'`
-      `cd #{repo_dir} && mkdir desktop mobile common assets locales`
+      `cd #{repo_dir} && mkdir desktop mobile common assets locales scss`
       files.each do |name, data|
         File.write("#{repo_dir}/#{name}", data)
         `cd #{repo_dir} && git add #{name}`
@@ -46,6 +46,7 @@ describe RemoteTheme do
       setup_git_repo(
         "about.json" => about_json,
         "desktop/desktop.scss" => scss_data,
+        "scss/file.scss" => ".class1{color:red}",
         "common/header.html" => "I AM HEADER",
         "common/random.html" => "I AM SILLY",
         "common/embedded.scss" => "EMBED",
@@ -77,7 +78,7 @@ describe RemoteTheme do
       expect(remote.theme_version).to eq("1.0")
       expect(remote.minimum_discourse_version).to eq("1.0.0")
 
-      expect(@theme.theme_fields.length).to eq(6)
+      expect(@theme.theme_fields.length).to eq(7)
 
       mapped = Hash[*@theme.theme_fields.map { |f| ["#{f.target_id}-#{f.name}", f.value] }.flatten]
 
@@ -91,7 +92,7 @@ describe RemoteTheme do
 
       expect(mapped["4-en"]).to eq("sometranslations")
 
-      expect(mapped.length).to eq(6)
+      expect(mapped.length).to eq(7)
 
       expect(@theme.settings.length).to eq(1)
       expect(@theme.settings.first.value).to eq(true)
@@ -112,6 +113,8 @@ describe RemoteTheme do
       `cd #{initial_repo} && git add settings.yml`
 
       File.delete("#{initial_repo}/settings.yaml")
+      File.delete("#{initial_repo}/scss/file.scss")
+
       `cd #{initial_repo} && git commit -am "update"`
 
       time = Time.new('2001')
@@ -122,7 +125,7 @@ describe RemoteTheme do
       expect(remote.remote_version).to eq(`cd #{initial_repo} && git rev-parse HEAD`.strip)
 
       remote.update_from_remote
-      @theme.save
+      @theme.save!
       @theme.reload
 
       scheme = ColorScheme.find_by(theme_id: @theme.id)
@@ -132,6 +135,9 @@ describe RemoteTheme do
 
       mapped = Hash[*@theme.theme_fields.map { |f| ["#{f.target_id}-#{f.name}", f.value] }.flatten]
 
+      # Scss file was deleted
+      expect(mapped["5-file"]).to eq(nil)
+
       expect(mapped["0-header"]).to eq("I AM UPDATED")
       expect(mapped["1-scss"]).to eq(scss_data)
 

spec/models/theme_field_spec.rb

@@ -29,6 +29,7 @@ describe ThemeField do
 
   it 'does not insert a script tag when there are no inline script' do
     theme_field = ThemeField.create!(theme_id: 1, target_id: 0, name: "body_tag", value: '<div>new div</div>')
+    theme_field.ensure_baked!
     expect(theme_field.value_baked).to_not include('<script')
   end
 
@@ -53,7 +54,7 @@ describe ThemeField do
     HTML
 
     theme_field = ThemeField.create!(theme_id: 1, target_id: 0, name: "header", value: html)
-
+    theme_field.ensure_baked!
     expect(theme_field.value_baked).to include("<script src=\"#{theme_field.javascript_cache.url}\"></script>")
     expect(theme_field.value_baked).to include("external-script.js")
     expect(theme_field.value_baked).to include('<script type="text/template"')
@@ -75,7 +76,7 @@ describe ThemeField do
     JavaScript
 
     theme_field = ThemeField.create!(theme_id: 1, target_id: 0, name: "header", value: html)
-
+    theme_field.ensure_baked!
     expect(theme_field.javascript_cache.content).to include(extracted)
   end
 
@@ -87,11 +88,13 @@ describe ThemeField do
 HTML
 
     field = ThemeField.create!(theme_id: 1, target_id: 0, name: "header", value: html)
+    field.ensure_baked!
     expect(field.error).not_to eq(nil)
     expect(field.value_baked).to include("<script src=\"#{field.javascript_cache.url}\"></script>")
     expect(field.javascript_cache.content).to include("Theme Transpilation Error:")
 
     field.update!(value: '')
+    field.ensure_baked!
     expect(field.error).to eq(nil)
   end
 
@@ -102,8 +105,9 @@ HTML
 </script>
 HTML
 
-    ThemeField.create!(theme_id: 1, target_id: 3, name: "yaml", value: "string_setting: \"test text \\\" 123!\"")
+    ThemeField.create!(theme_id: 1, target_id: 3, name: "yaml", value: "string_setting: \"test text \\\" 123!\"").ensure_baked!
     theme_field = ThemeField.create!(theme_id: 1, target_id: 0, name: "head_tag", value: html)
+    theme_field.ensure_baked!
     javascript_cache = theme_field.javascript_cache
 
     expect(theme_field.value_baked).to include("<script src=\"#{javascript_cache.url}\"></script>")
@@ -115,15 +119,33 @@ HTML
   it "correctly generates errors for transpiled css" do
     css = "body {"
     field = ThemeField.create!(theme_id: 1, target_id: 0, name: "scss", value: css)
-    field.reload
+    field.ensure_baked!
     expect(field.error).not_to eq(nil)
     field.value = "body {color: blue};"
     field.save!
-    field.reload
+    field.ensure_baked!
 
     expect(field.error).to eq(nil)
   end
 
+  it "allows importing scss files" do
+    theme = Fabricate(:theme)
+    main_field = theme.set_field(target: :common, name: :scss, value: ".class1{color: red}\n@import 'rootfile1';")
+    theme.set_field(target: :extra_scss, name: "rootfile1", value: ".class2{color:green}\n@import 'foldername/subfile1';")
+    theme.set_field(target: :extra_scss, name: "rootfile2", value: ".class3{color:green} ")
+    theme.set_field(target: :extra_scss, name: "foldername/subfile1", value: ".class4{color:yellow}\n@import 'subfile2';")
+    theme.set_field(target: :extra_scss, name: "foldername/subfile2", value: ".class5{color:yellow}\n@import '../rootfile2';")
+
+    theme.save!
+    result = main_field.compile_scss[0]
+
+    expect(result).to include(".class1")
+    expect(result).to include(".class2")
+    expect(result).to include(".class3")
+    expect(result).to include(".class4")
+    expect(result).to include(".class5")
+  end
+
   def create_upload_theme_field!(name)
     ThemeField.create!(
       theme_id: 1,
@@ -131,7 +153,7 @@ HTML
       value: "",
       type_id: ThemeField.types[:theme_upload_var],
       name: name,
-    )
+    ).tap { |tf| tf.ensure_baked! }
   end
 
   it "ensures we don't use invalid SCSS variable names" do
@@ -145,7 +167,7 @@ HTML
 
   def create_yaml_field(value)
     field = ThemeField.create!(theme_id: 1, target_id: Theme.targets[:settings], name: "yaml", value: value)
-    field.reload
+    field.ensure_baked!
     field
   end
 
@@ -179,7 +201,7 @@ HTML
 
     field.value = "valid_setting: true"
     field.save!
-    field.reload
+    field.ensure_baked!
     expect(field.error).to eq(nil)
   end
 
@@ -319,6 +341,7 @@ HTML
         HTML
 
         theme_field = ThemeField.create!(theme_id: theme.id, target_id: 0, name: "head_tag", value: html)
+        theme_field.ensure_baked!
         javascript_cache = theme_field.javascript_cache
         expect(javascript_cache.content).to include("inline discourse plugin")
         expect(javascript_cache.content).to include("theme_translations.#{theme.id}.")

spec/models/theme_spec.rb

@@ -238,6 +238,7 @@ HTML
   context "plugin api" do
     def transpile(html)
       f = ThemeField.create!(target_id: Theme.targets[:mobile], theme_id: 1, name: "after_header", value: html)
+      f.ensure_baked!
       return f.value_baked, f.javascript_cache
     end
 
@@ -307,6 +308,7 @@ HTML
 
       setting = theme.settings.find { |s| s.name == :font_size }
       setting.value = '30px'
+      theme.save!
 
       scss, _map = Stylesheet::Compiler.compile('@import "theme_variables"; @import "desktop_theme"; ', "theme.scss", theme_id: theme.id)
       expect(scss).to include("font-size:30px")
@@ -356,6 +358,7 @@ HTML
 
       setting = theme.settings.find { |s| s.name == :name }
       setting.value = 'bill'
+      theme.save!
 
       transpiled = <<~HTML
       (function() {
@@ -428,6 +431,7 @@ HTML
     expect(user_themes).to eq([])
 
     theme = Fabricate(:theme, name: "bob", user_selectable: true)
+    theme.save!
 
     json = Site.json_for(guardian)
     user_themes = JSON.parse(json)["user_themes"].map { |t| t["name"] }
@@ -485,6 +489,7 @@ HTML
     expect(cached_settings(theme.id)).to match(/\"boolean_setting\":true/)
 
     theme.settings.first.value = "false"
+    theme.save!
     expect(cached_settings(theme.id)).to match(/\"boolean_setting\":false/)
 
     child.set_field(target: :settings, name: "yaml", value: "integer_setting: 54")