diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index 2eddce7d13c..b61df316e6a 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/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, diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb index f80bbf26980..8e9c8d3ce4b 100644 --- a/app/models/remote_theme.rb +++ b/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( diff --git a/lib/theme_store/base_importer.rb b/lib/theme_store/base_importer.rb new file mode 100644 index 00000000000..feb36bcd701 --- /dev/null +++ b/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 diff --git a/lib/theme_store/directory_importer.rb b/lib/theme_store/directory_importer.rb new file mode 100644 index 00000000000..50c041e406c --- /dev/null +++ b/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 diff --git a/lib/theme_store/git_importer.rb b/lib/theme_store/git_importer.rb index f23127a02c2..407dcfadbea 100644 --- a/lib/theme_store/git_importer.rb +++ b/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 diff --git a/lib/theme_store/zip_importer.rb b/lib/theme_store/zip_importer.rb index aed20bb83e2..ac75a482c5e 100644 --- a/lib/theme_store/zip_importer.rb +++ b/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 diff --git a/spec/fixtures/themes/discourse-test-theme/about.json b/spec/fixtures/themes/discourse-test-theme/about.json new file mode 100644 index 00000000000..333d16932e4 --- /dev/null +++ b/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" + } + } +} \ No newline at end of file diff --git a/spec/fixtures/themes/discourse-test-theme/assets/logo.png b/spec/fixtures/themes/discourse-test-theme/assets/logo.png new file mode 100644 index 00000000000..5c900b61f70 Binary files /dev/null and b/spec/fixtures/themes/discourse-test-theme/assets/logo.png differ diff --git a/spec/fixtures/themes/discourse-test-theme/common/body_tag.html b/spec/fixtures/themes/discourse-test-theme/common/body_tag.html new file mode 100644 index 00000000000..da472912a6f --- /dev/null +++ b/spec/fixtures/themes/discourse-test-theme/common/body_tag.html @@ -0,0 +1 @@ +testtheme1 \ No newline at end of file diff --git a/spec/fixtures/themes/discourse-test-theme/locales/en.yml b/spec/fixtures/themes/discourse-test-theme/locales/en.yml new file mode 100644 index 00000000000..0670a76c42f --- /dev/null +++ b/spec/fixtures/themes/discourse-test-theme/locales/en.yml @@ -0,0 +1,3 @@ +--- +en: + key: value diff --git a/spec/fixtures/themes/discourse-test-theme/mobile/mobile.scss b/spec/fixtures/themes/discourse-test-theme/mobile/mobile.scss new file mode 100644 index 00000000000..160017232d4 --- /dev/null +++ b/spec/fixtures/themes/discourse-test-theme/mobile/mobile.scss @@ -0,0 +1 @@ +body {background-color: $background_color; font-size: $font-size} \ No newline at end of file diff --git a/spec/fixtures/themes/discourse-test-theme/settings.yml b/spec/fixtures/themes/discourse-test-theme/settings.yml new file mode 100644 index 00000000000..deb5d034a24 --- /dev/null +++ b/spec/fixtures/themes/discourse-test-theme/settings.yml @@ -0,0 +1 @@ +somesetting: test \ No newline at end of file diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb index 659bd5a3324..6a052a10d7a 100644 --- a/spec/models/remote_theme_spec.rb +++ b/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 diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index 75571da04f5..36f0711be63 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/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") diff --git a/spec/support/system_helpers.rb b/spec/support/system_helpers.rb index 294276f49db..f59851c2a61 100644 --- a/spec/support/system_helpers.rb +++ b/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