DEV: Add support for uploading a theme from a directory in system tests (#23402)

Why this change?

Currently, we do not have an easy way to test themes and theme components
using Rails system tests. While we support QUnit acceptance tests for
themes and theme components, QUnit acceptance tests stubs out the server
and setting up the fixtures for server responses is difficult and can lead to a
frustrating experience. System tests on the other hand allow authors to
set up the test fixtures using our fabricator system which is much
easier to use.

What does this change do?

In order for us to allow authors to run system tests with their themes
installed, we are adding a `upload_theme` helper that is made available
when writing system tests. The `upload_theme` helper requires a single
`directory` parameter where `directory` is the directory of the theme
locally and returns a `Theme` record.

app/controllers/admin/themes_controller.rb

@@ -142,13 +142,11 @@ class Admin::ThemesController < Admin::AdminController
       bundle = params[:bundle] || params[:theme]
       theme_id = params[:theme_id]
       update_components = params[:components]
-      match_theme_by_name = !!params[:bundle] && !params.key?(:theme_id) # Old theme CLI behavior, match by name. Remove Jan 2020
       begin
         @theme =
           RemoteTheme.update_zipped_theme(
             bundle.path,
             bundle.original_filename,
-            match_theme: match_theme_by_name,
             user: theme_user,
             theme_id: theme_id,
             update_components: update_components,

app/models/remote_theme.rb

@@ -51,16 +51,34 @@ class RemoteTheme < ActiveRecord::Base
   def self.update_zipped_theme(
     filename,
     original_filename,
-    match_theme: false,
     user: Discourse.system_user,
     theme_id: nil,
     update_components: nil
   )
-    importer = ThemeStore::ZipImporter.new(filename, original_filename)
+    update_theme(
+      ThemeStore::ZipImporter.new(filename, original_filename),
+      user:,
+      theme_id:,
+      update_components:,
+    )
+  end
+
+  # This is only used in the tests environment and is currently not supported for other environments
+  if Rails.env.test?
+    def self.import_theme_from_directory(directory)
+      update_theme(ThemeStore::DirectoryImporter.new(directory))
+    end
+  end
+
+  def self.update_theme(
+    importer,
+    user: Discourse.system_user,
+    theme_id: nil,
+    update_components: nil
+  )
     importer.import!
 
     theme_info = RemoteTheme.extract_theme_info(importer)
-    theme = Theme.find_by(name: theme_info["name"]) if match_theme # Old theme CLI method, remove Jan 2020
     theme = Theme.find_by(id: theme_id) if theme_id # New theme CLI method
 
     existing = true
@@ -107,6 +125,7 @@ class RemoteTheme < ActiveRecord::Base
       Rails.logger.warn("Failed cleanup remote path #{e}")
     end
   end
+  private_class_method :update_theme
 
   def self.import_theme(url, user = Discourse.system_user, private_key: nil, branch: nil)
     importer = ThemeStore::GitImporter.new(url.strip, private_key: private_key, branch: branch)
@@ -232,6 +251,7 @@ class RemoteTheme < ActiveRecord::Base
     METADATA_PROPERTIES.each do |property|
       self.public_send(:"#{property}=", theme_info[property.to_s])
     end
+
     if !self.valid?
       raise ImportError,
             I18n.t(
@@ -246,6 +266,7 @@ class RemoteTheme < ActiveRecord::Base
         theme_info.dig("modifiers", modifier_name.to_s),
       )
     end
+
     if !theme.theme_modifier_set.valid?
       raise ImportError,
             I18n.t(

lib/theme_store/base_importer.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module ThemeStore
+  class BaseImporter
+    def import!
+      raise "Not implemented"
+    end
+
+    def [](value)
+      fullpath = real_path(value)
+      return nil unless fullpath
+      File.read(fullpath)
+    end
+
+    def real_path(relative)
+      fullpath = "#{temp_folder}/#{relative}"
+      return nil unless File.exist?(fullpath)
+
+      # careful to handle symlinks here, don't want to expose random data
+      fullpath = Pathname.new(fullpath).realpath.to_s
+
+      if fullpath && fullpath.start_with?(temp_folder)
+        fullpath
+      else
+        nil
+      end
+    end
+
+    def all_files
+      Dir.glob("**/**", base: temp_folder).reject { |f| File.directory?(File.join(temp_folder, f)) }
+    end
+
+    def cleanup!
+      FileUtils.rm_rf(temp_folder)
+    end
+
+    def temp_folder
+      @temp_folder ||= "#{Pathname.new(Dir.tmpdir).realpath}/discourse_theme_#{SecureRandom.hex}"
+    end
+  end
+end

lib/theme_store/directory_importer.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module ThemeStore
+  class DirectoryImporter < BaseImporter
+    def initialize(theme_dir)
+      @theme_dir = theme_dir
+    end
+
+    def import!
+      FileUtils.mkdir_p(temp_folder)
+      FileUtils.cp_r("#{@theme_dir}/.", temp_folder)
+    end
+  end
+end

lib/theme_store/git_importer.rb

@@ -1,16 +1,12 @@
 # frozen_string_literal: true
 
-module ThemeStore
-end
-
-class ThemeStore::GitImporter
+class ThemeStore::GitImporter < ThemeStore::BaseImporter
   COMMAND_TIMEOUT_SECONDS = 20
 
   attr_reader :url
 
   def initialize(url, private_key: nil, branch: nil)
     @url = GitUrl.normalize(url)
-    @temp_folder = "#{Pathname.new(Dir.tmpdir).realpath}/discourse_theme_#{SecureRandom.hex}"
     @private_key = private_key
     @branch = branch
   end
@@ -18,7 +14,7 @@ class ThemeStore::GitImporter
   def import!
     clone!
 
-    if version = Discourse.find_compatible_git_resource(@temp_folder)
+    if version = Discourse.find_compatible_git_resource(temp_folder)
       begin
         execute "git", "cat-file", "-e", version
       rescue RuntimeError => e
@@ -56,34 +52,6 @@ class ThemeStore::GitImporter
     execute("git", "rev-parse", "HEAD").strip
   end
 
-  def cleanup!
-    FileUtils.rm_rf(@temp_folder)
-  end
-
-  def real_path(relative)
-    fullpath = "#{@temp_folder}/#{relative}"
-    return nil unless File.exist?(fullpath)
-
-    # careful to handle symlinks here, don't want to expose random data
-    fullpath = Pathname.new(fullpath).realpath.to_s
-
-    if fullpath && fullpath.start_with?(@temp_folder)
-      fullpath
-    else
-      nil
-    end
-  end
-
-  def all_files
-    Dir.glob("**/*", base: @temp_folder).reject { |f| File.directory?(File.join(@temp_folder, f)) }
-  end
-
-  def [](value)
-    fullpath = real_path(value)
-    return nil unless fullpath
-    File.read(fullpath)
-  end
-
   protected
 
   def redirected_uri
@@ -135,7 +103,7 @@ class ThemeStore::GitImporter
 
     args.concat(["--single-branch", "-b", @branch]) if @branch.present?
 
-    args.concat([url, @temp_folder])
+    args.concat([url, temp_folder])
 
     args
   end
@@ -196,6 +164,6 @@ class ThemeStore::GitImporter
   end
 
   def execute(*args)
-    Discourse::Utils.execute_command(*args, chdir: @temp_folder, timeout: COMMAND_TIMEOUT_SECONDS)
+    Discourse::Utils.execute_command(*args, chdir: temp_folder, timeout: COMMAND_TIMEOUT_SECONDS)
   end
 end

lib/theme_store/zip_importer.rb

@@ -2,10 +2,7 @@
 
 require "compression/engine"
 
-module ThemeStore
-end
-
-class ThemeStore::ZipImporter
+class ThemeStore::ZipImporter < ThemeStore::BaseImporter
   attr_reader :url
 
   def initialize(filename, original_filename)
@@ -30,10 +27,6 @@ class ThemeStore::ZipImporter
     raise RemoteTheme::ImportError, I18n.t("themes.import_error.file_too_big")
   end
 
-  def cleanup!
-    FileUtils.rm_rf(@temp_folder)
-  end
-
   def version
     ""
   end
@@ -44,28 +37,4 @@ class ThemeStore::ZipImporter
       FileUtils.mv(Dir.glob("#{@temp_folder}/*/*"), @temp_folder)
     end
   end
-
-  def real_path(relative)
-    fullpath = "#{@temp_folder}/#{relative}"
-    return nil unless File.exist?(fullpath)
-
-    # careful to handle symlinks here, don't want to expose random data
-    fullpath = Pathname.new(fullpath).realpath.to_s
-
-    if fullpath && fullpath.start_with?(@temp_folder)
-      fullpath
-    else
-      nil
-    end
-  end
-
-  def all_files
-    Dir.glob("**/**", base: @temp_folder).reject { |f| File.directory?(File.join(@temp_folder, f)) }
-  end
-
-  def [](value)
-    fullpath = real_path(value)
-    return nil unless fullpath
-    File.read(fullpath)
-  end
 end

spec/fixtures/themes/discourse-test-theme/about.json

@@ -0,0 +1,21 @@
+{
+  "name": "Header Icons",
+  "about_url": "abouturl",
+  "license_url": "licenseurl",
+  "component": false,
+  "assets": {
+    "logo": "assets/logo.png"
+  },
+  "color_schemes": {
+    "Orphan Color Scheme": {
+      "header_primary": "F0F0F0",
+      "header_background": "1E1E1E",
+      "tertiary": "858585"
+    },
+    "Theme Color Scheme": {
+      "header_primary": "F0F0F0",
+      "header_background": "1E1E1E",
+      "tertiary": "858585"
+    }
+  }
+}

spec/fixtures/themes/discourse-test-theme/common/body_tag.html

@@ -0,0 +1 @@
+<b>testtheme1</b>

spec/fixtures/themes/discourse-test-theme/locales/en.yml

@@ -0,0 +1,3 @@
+---
+en:
+  key: value

spec/fixtures/themes/discourse-test-theme/mobile/mobile.scss

@@ -0,0 +1 @@
+body {background-color: $background_color; font-size: $font-size}

spec/fixtures/themes/discourse-test-theme/settings.yml

@@ -0,0 +1 @@
+somesetting: test

spec/models/remote_theme_spec.rb

@@ -281,4 +281,15 @@ RSpec.describe RemoteTheme do
       expect(described_class.unreachable_themes).to eq([])
     end
   end
+
+  describe ".import_theme_from_directory" do
+    let(:theme_dir) { "#{Rails.root}/spec/fixtures/themes/discourse-test-theme" }
+
+    it "imports a theme from a directory" do
+      theme = RemoteTheme.import_theme_from_directory(theme_dir)
+
+      expect(theme.name).to eq("Header Icons")
+      expect(theme.theme_fields.count).to eq(5)
+    end
+  end
 end

spec/requests/admin/themes_controller_spec.rb

@@ -326,21 +326,6 @@ RSpec.describe Admin::ThemesController do
         expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1)
       end
 
-      it "updates an existing theme from an archive by name" do
-        # Old theme CLI method, remove Jan 2020
-        _existing_theme = Fabricate(:theme, name: "Header Icons")
-
-        expect do
-          post "/admin/themes/import.json", params: { bundle: theme_archive }
-        end.to change { Theme.count }.by (0)
-        expect(response.status).to eq(201)
-        json = response.parsed_body
-
-        expect(json["theme"]["name"]).to eq("Header Icons")
-        expect(json["theme"]["theme_fields"].length).to eq(5)
-        expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1)
-      end
-
       it "updates an existing theme from an archive by id" do
         # Used by theme CLI
         _existing_theme = Fabricate(:theme, name: "Header Icons")

spec/support/system_helpers.rb

@@ -22,6 +22,10 @@ module SystemHelpers
     expect(page).to have_content("Signed in to #{user.encoded_username} successfully")
   end
 
+  def upload_theme(theme_dir)
+    RemoteTheme.import_theme_from_directory(theme_dir)
+  end
+
   def setup_system_test
     SiteSetting.login_required = false
     SiteSetting.has_login_hint = false