FIX: Update themes javascript cache after running themes migrations (#25562)

Why this change?

This is caused by a regression in
59839e428f, where we stopped saving the
`Theme` object because it was unnecessary. However, it resulted in the
`after_save` callback not being called and hence
`Theme#update_javascript_cache!` not being called. As a result, some
sites were reporting that after runing a theme migration, the defaults
for the theme settings were used instead of the settings overrides
stored in the database.

What does this change do?

Add a call to `Theme#update_javascript_cache!` after running theme
migrations.
This commit is contained in:
Alan Guo Xiang Tan 2024-02-05 14:35:11 +08:00 committed by GitHub
parent cdceca9d1f
commit d460229ed8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 36 additions and 0 deletions

View File

@ -172,7 +172,9 @@ class Theme < ActiveRecord::Base
js_compiler = ThemeJavascriptCompiler.new(id, name) js_compiler = ThemeJavascriptCompiler.new(id, name)
js_compiler.append_tree(all_extra_js) js_compiler.append_tree(all_extra_js)
settings_hash = build_settings_hash settings_hash = build_settings_hash
js_compiler.prepend_settings(settings_hash) if settings_hash.present? js_compiler.prepend_settings(settings_hash) if settings_hash.present?
javascript_cache || build_javascript_cache javascript_cache || build_javascript_cache
javascript_cache.update!(content: js_compiler.content, source_map: js_compiler.source_map) javascript_cache.update!(content: js_compiler.content, source_map: js_compiler.source_map)
else else
@ -880,6 +882,7 @@ class Theme < ActiveRecord::Base
end end
self.reload self.reload
self.update_javascript_cache!
end end
if start_transaction if start_transaction

View File

@ -1095,11 +1095,42 @@ HTML
) )
end end
it "updates the theme's javascript cache after running migration" do
theme.set_field(target: :extra_js, name: "test.js.es6", value: "const hello = 'world';")
theme.save!
expect(theme.javascript_cache.content).to include('"list_setting":"aa,bb"')
settings_field.update!(value: <<~YAML)
integer_setting: 1
list_setting:
default: aa|bb
type: list
YAML
migration_field.update!(value: <<~JS)
export default function migrate(settings) {
settings.set("list_setting", "zz|aa");
return settings;
}
JS
theme.reload
theme.migrate_settings
setting_record = theme.theme_settings.where(name: "list_setting").first
expect(setting_record.data_type).to eq(ThemeSetting.types[:list])
expect(setting_record.value).to eq("zz|aa")
expect(theme.javascript_cache.content).to include('"list_setting":"zz|aa"')
end
it "allows changing a setting's type" do it "allows changing a setting's type" do
theme.update_setting(:list_setting, "zz,aa") theme.update_setting(:list_setting, "zz,aa")
theme.save! theme.save!
setting_record = theme.theme_settings.where(name: "list_setting").first setting_record = theme.theme_settings.where(name: "list_setting").first
expect(setting_record.data_type).to eq(ThemeSetting.types[:string]) expect(setting_record.data_type).to eq(ThemeSetting.types[:string])
expect(setting_record.value).to eq("zz,aa") expect(setting_record.value).to eq("zz,aa")
@ -1109,12 +1140,14 @@ HTML
default: aa|bb default: aa|bb
type: list type: list
YAML YAML
migration_field.update!(value: <<~JS) migration_field.update!(value: <<~JS)
export default function migrate(settings) { export default function migrate(settings) {
settings.set("list_setting", "zz|aa"); settings.set("list_setting", "zz|aa");
return settings; return settings;
} }
JS JS
theme.reload theme.reload
theme.migrate_settings theme.migrate_settings