DEV: Include theme_uploads and theme_uploads_local objects in theme tests ()

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.
This commit is contained in:
Osama Sayegh 2022-10-20 08:00:29 +03:00 committed by GitHub
parent 505aec123f
commit ce53152e53
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 68 additions and 14 deletions
app
assets/javascripts/discourse/app/lib
models
spec/requests

@ -1,4 +1,5 @@
import { get } from "@ember/object"; import { get } from "@ember/object";
import { cloneJSON } from "discourse-common/lib/object";
const originalSettings = {}; const originalSettings = {};
const settings = {}; const settings = {};
@ -11,7 +12,7 @@ export function registerSettings(
if (settings[themeId] && !force) { if (settings[themeId] && !force) {
return; return;
} }
originalSettings[themeId] = Object.assign({}, settingsObject); originalSettings[themeId] = cloneJSON(settingsObject);
const s = {}; const s = {};
Object.keys(settingsObject).forEach((key) => { Object.keys(settingsObject).forEach((key) => {
Object.defineProperty(s, key, { Object.defineProperty(s, key, {
@ -41,7 +42,14 @@ export function getObjectForTheme(themeId) {
export function resetSettings() { export function resetSettings() {
Object.keys(originalSettings).forEach((themeId) => { Object.keys(originalSettings).forEach((themeId) => {
Object.keys(originalSettings[themeId]).forEach((key) => { 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;
}
}); });
}); });
} }

@ -530,6 +530,13 @@ class Theme < ActiveRecord::Base
self.settings.each do |setting| self.settings.each do |setting|
settings_hash[setting.name] = setting.default settings_hash[setting.name] = setting.default
end 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 settings_hash
end end
end end
@ -540,24 +547,35 @@ class Theme < ActiveRecord::Base
hash[setting.name] = setting.value hash[setting.name] = setting.value
end end
theme_uploads = {} theme_uploads = build_theme_uploads_hash
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
hash['theme_uploads'] = theme_uploads if theme_uploads.present? 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['theme_uploads_local'] = theme_uploads_local if theme_uploads_local.present?
hash hash
end 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) def update_setting(setting_name, new_value)
target_setting = settings.find { |setting| setting.name == setting_name } target_setting = settings.find { |setting| setting.name == setting_name }
raise Discourse::NotFound unless target_setting raise Discourse::NotFound unless target_setting

@ -126,6 +126,34 @@ RSpec.describe ThemeJavascriptsController do
expect(response.body).to include("assert.ok(true);") expect(response.body).to include("assert.ok(true);")
end 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 it "responds with 404 if digest is not a 40 chars hex" do
digest = Rack::Utils.escape('../../../../../../../../../../etc/passwd').gsub('.', '%2E') digest = Rack::Utils.escape('../../../../../../../../../../etc/passwd').gsub('.', '%2E')
get "/theme-javascripts/tests/#{component.id}-#{digest}.js" get "/theme-javascripts/tests/#{component.id}-#{digest}.js"