From ce53152e5371176539a3cbff4739235dbd90b220 Mon Sep 17 00:00:00 2001
From: Osama Sayegh <asooomaasoooma90@gmail.com>
Date: Thu, 20 Oct 2022 08:00:29 +0300
Subject: [PATCH] DEV: Include theme_uploads and theme_uploads_local objects in
theme tests (#18645)
Our theme system injects a magical `settings` object at the top of themes JS modules to allow theme authors to access the settings as configured by admins in the UI. Within this `settings` object, there are a couple of special objects `theme_uploads` and `theme_uploads_local` that contain URLs for all the assets/uploads that the theme has.
For test modules/files, the theme system also injects a `settings` object at the top of tests modules, but it's not the same object as the object that's injected in non-test files. The difference is that in tests we want the settings to have their default values as opposed to any custom values that may exist in the site's database. This ensures that test results are consistent no matter the site that runs them.
However, the `settings` object in tests files currently doesn't have the special objects `theme_uploads` and `theme_uploads_local` which means that if a theme includes an asset that's lazy-loaded, it's not possible to write tests for anything that depends on the lazy-loaded asset because the theme will not be able to load the asset during the tests since `theme_uploads_local` and `theme_uploads` don't exist. This PR adds these special objects inside the `settings` object for test files.
Internal topic: t/71825/52.
---
.../discourse/app/lib/theme-settings-store.js | 12 +++++-
app/models/theme.rb | 42 +++++++++++++------
.../theme_javascripts_controller_spec.rb | 28 +++++++++++++
3 files changed, 68 insertions(+), 14 deletions(-)
diff --git a/app/assets/javascripts/discourse/app/lib/theme-settings-store.js b/app/assets/javascripts/discourse/app/lib/theme-settings-store.js
index 6503d6b932c..940a88cf4c2 100644
--- a/app/assets/javascripts/discourse/app/lib/theme-settings-store.js
+++ b/app/assets/javascripts/discourse/app/lib/theme-settings-store.js
@@ -1,4 +1,5 @@
import { get } from "@ember/object";
+import { cloneJSON } from "discourse-common/lib/object";
const originalSettings = {};
const settings = {};
@@ -11,7 +12,7 @@ export function registerSettings(
if (settings[themeId] && !force) {
return;
}
- originalSettings[themeId] = Object.assign({}, settingsObject);
+ originalSettings[themeId] = cloneJSON(settingsObject);
const s = {};
Object.keys(settingsObject).forEach((key) => {
Object.defineProperty(s, key, {
@@ -41,7 +42,14 @@ export function getObjectForTheme(themeId) {
export function resetSettings() {
Object.keys(originalSettings).forEach((themeId) => {
Object.keys(originalSettings[themeId]).forEach((key) => {
- settings[themeId][key] = originalSettings[themeId][key];
+ const original = originalSettings[themeId][key];
+ if (original && typeof original === "object") {
+ // special handling for the theme_uploads and theme_uploads_local magic
+ // objects in settings
+ settings[themeId][key] = cloneJSON(original);
+ } else {
+ settings[themeId][key] = original;
+ }
});
});
}
diff --git a/app/models/theme.rb b/app/models/theme.rb
index adce6dbabdb..6dbe460a22c 100644
--- a/app/models/theme.rb
+++ b/app/models/theme.rb
@@ -530,6 +530,13 @@ class Theme < ActiveRecord::Base
self.settings.each do |setting|
settings_hash[setting.name] = setting.default
end
+
+ theme_uploads = build_theme_uploads_hash
+ settings_hash['theme_uploads'] = theme_uploads if theme_uploads.present?
+
+ theme_uploads_local = build_local_theme_uploads_hash
+ settings_hash['theme_uploads_local'] = theme_uploads_local if theme_uploads_local.present?
+
settings_hash
end
end
@@ -540,24 +547,35 @@ class Theme < ActiveRecord::Base
hash[setting.name] = setting.value
end
- theme_uploads = {}
- theme_uploads_local = {}
-
- upload_fields.each do |field|
- if field.upload&.url
- theme_uploads[field.name] = Discourse.store.cdn_url(field.upload.url)
- end
- if field.javascript_cache
- theme_uploads_local[field.name] = field.javascript_cache.local_url
- end
- end
-
+ theme_uploads = build_theme_uploads_hash
hash['theme_uploads'] = theme_uploads if theme_uploads.present?
+
+ theme_uploads_local = build_local_theme_uploads_hash
hash['theme_uploads_local'] = theme_uploads_local if theme_uploads_local.present?
hash
end
+ def build_theme_uploads_hash
+ hash = {}
+ upload_fields.each do |field|
+ if field.upload&.url
+ hash[field.name] = Discourse.store.cdn_url(field.upload.url)
+ end
+ end
+ hash
+ end
+
+ def build_local_theme_uploads_hash
+ hash = {}
+ upload_fields.each do |field|
+ if field.javascript_cache
+ hash[field.name] = field.javascript_cache.local_url
+ end
+ end
+ hash
+ end
+
def update_setting(setting_name, new_value)
target_setting = settings.find { |setting| setting.name == setting_name }
raise Discourse::NotFound unless target_setting
diff --git a/spec/requests/theme_javascripts_controller_spec.rb b/spec/requests/theme_javascripts_controller_spec.rb
index 0f2c58ce584..84ec854ae19 100644
--- a/spec/requests/theme_javascripts_controller_spec.rb
+++ b/spec/requests/theme_javascripts_controller_spec.rb
@@ -126,6 +126,34 @@ RSpec.describe ThemeJavascriptsController do
expect(response.body).to include("assert.ok(true);")
end
+ it "includes theme uploads URLs in the settings object" do
+ SiteSetting.authorized_extensions = "*"
+ js_file = Tempfile.new(["vendorlib", ".js"])
+ js_file.write("console.log(123);\n")
+ js_file.rewind
+ js_upload = UploadCreator.new(js_file, "vendorlib.js").create_for(Discourse::SYSTEM_USER_ID)
+ component.set_field(
+ type: :theme_upload_var,
+ target: :common,
+ name: "vendorlib",
+ upload_id: js_upload.id
+ )
+ component.save!
+ _, digest = component.baked_js_tests_with_digest
+
+ get "/theme-javascripts/tests/#{component.id}-#{digest}.js"
+ expect(response.body).to include(
+ "require(\"discourse/lib/theme-settings-store\").registerSettings(" +
+ "#{component.id}, {\"num_setting\":5,\"theme_uploads\":{\"vendorlib\":" +
+ "\"/uploads/default/test_#{ENV['TEST_ENV_NUMBER']}/original/1X/#{js_upload.sha1}.js\"},\"theme_uploads_local\":{\"vendorlib\":" +
+ "\"/theme-javascripts/#{js_upload.sha1}.js?__ws=test.localhost\"}}, { force: true });"
+ )
+ expect(response.body).to include("assert.ok(true);")
+ ensure
+ js_file&.close
+ js_file&.unlink
+ end
+
it "responds with 404 if digest is not a 40 chars hex" do
digest = Rack::Utils.escape('../../../../../../../../../../etc/passwd').gsub('.', '%2E')
get "/theme-javascripts/tests/#{component.id}-#{digest}.js"