FEATURE: Theme settings migrations (#24071)
This commit introduces a new feature that allows theme developers to manage the transformation of theme settings over time. Similar to Rails migrations, the theme settings migration system enables developers to write and execute migrations for theme settings, ensuring a smooth transition when changes are required in the format or structure of setting values.
Example use cases for the theme settings migration system:
1. Renaming a theme setting.
2. Changing the data type of a theme setting (e.g., transforming a string setting containing comma-separated values into a proper list setting).
3. Altering the format of data stored in a theme setting.
All of these use cases and more are now possible while preserving theme setting values for sites that have already modified their theme settings.
Usage:
1. Create a top-level directory called `migrations` in your theme/component, and then within the `migrations` directory create another directory called `settings`.
2. Inside the `migrations/settings` directory, create a JavaScript file using the format `XXXX-some-name.js`, where `XXXX` is a unique 4-digit number, and `some-name` is a descriptor of your choice that describes the migration.
3. Within the JavaScript file, define and export (as the default) a function called `migrate`. This function will receive a `Map` object and must also return a `Map` object (it's acceptable to return the same `Map` object that the function received).
4. The `Map` object received by the `migrate` function will include settings that have been overridden or changed by site administrators. Settings that have never been changed from the default will not be included.
5. The keys and values contained in the `Map` object that the `migrate` function returns will replace all the currently changed settings of the theme.
6. Migrations are executed in numerical order based on the XXXX segment in the migration filenames. For instance, `0001-some-migration.js` will be executed before `0002-another-migration.js`.
Here's a complete example migration script that renames a setting from `setting_with_old_name` to `setting_with_new_name`:
```js
// File name: 0001-rename-setting.js
export default function migrate(settings) {
if (settings.has("setting_with_old_name")) {
settings.set("setting_with_new_name", settings.get("setting_with_old_name"));
}
return settings;
}
```
Internal topic: t/109980
2023-11-02 01:10:15 -04:00
|
|
|
# frozen_string_literal: true
|
|
|
|
|
|
|
|
describe ThemeSettingsMigrationsRunner do
|
2023-11-09 17:47:59 -05:00
|
|
|
fab!(:theme)
|
FEATURE: Theme settings migrations (#24071)
This commit introduces a new feature that allows theme developers to manage the transformation of theme settings over time. Similar to Rails migrations, the theme settings migration system enables developers to write and execute migrations for theme settings, ensuring a smooth transition when changes are required in the format or structure of setting values.
Example use cases for the theme settings migration system:
1. Renaming a theme setting.
2. Changing the data type of a theme setting (e.g., transforming a string setting containing comma-separated values into a proper list setting).
3. Altering the format of data stored in a theme setting.
All of these use cases and more are now possible while preserving theme setting values for sites that have already modified their theme settings.
Usage:
1. Create a top-level directory called `migrations` in your theme/component, and then within the `migrations` directory create another directory called `settings`.
2. Inside the `migrations/settings` directory, create a JavaScript file using the format `XXXX-some-name.js`, where `XXXX` is a unique 4-digit number, and `some-name` is a descriptor of your choice that describes the migration.
3. Within the JavaScript file, define and export (as the default) a function called `migrate`. This function will receive a `Map` object and must also return a `Map` object (it's acceptable to return the same `Map` object that the function received).
4. The `Map` object received by the `migrate` function will include settings that have been overridden or changed by site administrators. Settings that have never been changed from the default will not be included.
5. The keys and values contained in the `Map` object that the `migrate` function returns will replace all the currently changed settings of the theme.
6. Migrations are executed in numerical order based on the XXXX segment in the migration filenames. For instance, `0001-some-migration.js` will be executed before `0002-another-migration.js`.
Here's a complete example migration script that renames a setting from `setting_with_old_name` to `setting_with_new_name`:
```js
// File name: 0001-rename-setting.js
export default function migrate(settings) {
if (settings.has("setting_with_old_name")) {
settings.set("setting_with_new_name", settings.get("setting_with_old_name"));
}
return settings;
}
```
Internal topic: t/109980
2023-11-02 01:10:15 -04:00
|
|
|
fab!(:migration_field) { Fabricate(:migration_theme_field, version: 1, theme: theme) }
|
|
|
|
fab!(:settings_field) { Fabricate(:settings_theme_field, theme: theme, value: <<~YAML) }
|
|
|
|
integer_setting: 1
|
|
|
|
boolean_setting: true
|
|
|
|
string_setting: ""
|
|
|
|
YAML
|
|
|
|
|
|
|
|
describe "#run" do
|
|
|
|
it "passes values of overridden settings only to migrations" do
|
|
|
|
theme.update_setting(:integer_setting, 1)
|
|
|
|
theme.update_setting(:string_setting, "osama")
|
|
|
|
theme.save!
|
|
|
|
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
export default function migrate(settings) {
|
|
|
|
if (settings.get("integer_setting") !== 1) {
|
|
|
|
throw new Error(`expected integer_setting to equal 1, but it's actually ${settings.get("integer_setting")}`);
|
|
|
|
}
|
|
|
|
if (settings.get("string_setting") !== "osama") {
|
|
|
|
throw new Error(`expected string_setting to equal "osama", but it's actually "${settings.get("string_setting")}"`);
|
|
|
|
}
|
|
|
|
if (settings.size !== 2) {
|
|
|
|
throw new Error(`expected the settings map to have only 2 keys, but instead got ${settings.size} keys`);
|
|
|
|
}
|
|
|
|
return settings;
|
|
|
|
}
|
|
|
|
JS
|
|
|
|
results = described_class.new(theme).run
|
|
|
|
expect(results.first[:theme_field_id]).to eq(migration_field.id)
|
|
|
|
expect(results.first[:settings_before]).to eq(
|
|
|
|
{ "integer_setting" => 1, "string_setting" => "osama" },
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "passes the output of the previous migration as input to the next one" do
|
|
|
|
theme.update_setting(:integer_setting, 1)
|
|
|
|
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
export default function migrate(settings) {
|
|
|
|
settings.set("integer_setting", 111);
|
|
|
|
return settings;
|
|
|
|
}
|
|
|
|
JS
|
|
|
|
|
|
|
|
another_migration_field =
|
|
|
|
Fabricate(:migration_theme_field, theme: theme, version: 2, value: <<~JS)
|
|
|
|
export default function migrate(settings) {
|
|
|
|
if (settings.get("integer_setting") !== 111) {
|
|
|
|
throw new Error(`expected integer_setting to equal 111, but it's actually ${settings.get("integer_setting")}`);
|
|
|
|
}
|
|
|
|
settings.set("integer_setting", 222);
|
|
|
|
return settings;
|
|
|
|
}
|
|
|
|
JS
|
|
|
|
|
|
|
|
results = described_class.new(theme).run
|
|
|
|
|
|
|
|
expect(results.size).to eq(2)
|
|
|
|
|
|
|
|
expect(results[0][:theme_field_id]).to eq(migration_field.id)
|
|
|
|
expect(results[1][:theme_field_id]).to eq(another_migration_field.id)
|
|
|
|
|
|
|
|
expect(results[0][:settings_before]).to eq({})
|
|
|
|
expect(results[0][:settings_after]).to eq({ "integer_setting" => 111 })
|
|
|
|
|
|
|
|
expect(results[1][:settings_before]).to eq({ "integer_setting" => 111 })
|
|
|
|
expect(results[1][:settings_after]).to eq({ "integer_setting" => 222 })
|
|
|
|
end
|
|
|
|
|
|
|
|
it "doesn't run migrations that have already been ran" do
|
|
|
|
Fabricate(:theme_settings_migration, theme: theme, theme_field: migration_field)
|
|
|
|
|
|
|
|
pending_field = Fabricate(:migration_theme_field, theme: theme, version: 23)
|
|
|
|
|
|
|
|
results = described_class.new(theme).run
|
|
|
|
|
|
|
|
expect(results.size).to eq(1)
|
|
|
|
|
|
|
|
expect(results.first[:version]).to eq(23)
|
|
|
|
expect(results.first[:theme_field_id]).to eq(pending_field.id)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "doesn't error when no migrations have been ran yet" do
|
|
|
|
results = described_class.new(theme).run
|
|
|
|
|
|
|
|
expect(results.size).to eq(1)
|
|
|
|
expect(results.first[:version]).to eq(1)
|
|
|
|
expect(results.first[:theme_field_id]).to eq(migration_field.id)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "doesn't error when there are no pending migrations" do
|
|
|
|
Fabricate(:theme_settings_migration, theme: theme, theme_field: migration_field)
|
|
|
|
|
|
|
|
results = described_class.new(theme).run
|
|
|
|
|
|
|
|
expect(results.size).to eq(0)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "raises an error when there are too many pending migrations" do
|
|
|
|
Fabricate(:migration_theme_field, theme: theme, version: 2)
|
|
|
|
|
|
|
|
expect do described_class.new(theme, limit: 1).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t("themes.import_error.migrations.too_many_pending_migrations"),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "raises an error if a migration field has a badly formatted name" do
|
|
|
|
migration_field.update_attribute(:name, "020-some-name")
|
|
|
|
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t("themes.import_error.migrations.invalid_filename", filename: "020-some-name"),
|
|
|
|
)
|
|
|
|
|
|
|
|
migration_field.update_attribute(:name, "0020some-name")
|
|
|
|
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t("themes.import_error.migrations.invalid_filename", filename: "0020some-name"),
|
|
|
|
)
|
|
|
|
|
|
|
|
migration_field.update_attribute(:name, "0020")
|
|
|
|
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t("themes.import_error.migrations.invalid_filename", filename: "0020"),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "raises an error if a pending migration has version lower than the last ran migration" do
|
|
|
|
migration_field.update!(name: "0020-some-name")
|
|
|
|
Fabricate(:theme_settings_migration, theme: theme, theme_field: migration_field, version: 20)
|
|
|
|
|
|
|
|
Fabricate(:migration_theme_field, theme: theme, version: 19, name: "0019-failing-migration")
|
|
|
|
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t(
|
|
|
|
"themes.import_error.migrations.out_of_sequence",
|
|
|
|
name: "0019-failing-migration",
|
|
|
|
current: 20,
|
|
|
|
),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "detects bad syntax in migrations and raises an error" do
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
export default function migrate() {
|
|
|
|
JS
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t(
|
|
|
|
"themes.import_error.migrations.syntax_error",
|
|
|
|
name: "0001-some-name",
|
|
|
|
error:
|
|
|
|
'SyntaxError: "/discourse/theme/migration: Unexpected token (2:0)\n\n 1 | export default function migrate() {\n> 2 |\n | ^"',
|
|
|
|
),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "imposes memory limit on migrations and raises an error if they exceed the limit" do
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
export default function migrate(settings) {
|
|
|
|
let a = new Array(10000);
|
|
|
|
while(true) {
|
|
|
|
a = a.concat(new Array(10000));
|
|
|
|
}
|
|
|
|
return settings;
|
|
|
|
}
|
|
|
|
JS
|
|
|
|
|
|
|
|
expect do described_class.new(theme, memory: 10.kilobytes).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t("themes.import_error.migrations.exceeded_memory_limit", name: "0001-some-name"),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "imposes time limit on migrations and raises an error if they exceed the limit" do
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
export default function migrate(settings) {
|
|
|
|
let a = 1;
|
|
|
|
while(true) {
|
|
|
|
a += 1;
|
|
|
|
}
|
|
|
|
return settings;
|
|
|
|
}
|
|
|
|
JS
|
|
|
|
|
|
|
|
expect do described_class.new(theme, timeout: 10).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t("themes.import_error.migrations.timed_out", name: "0001-some-name"),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "raises a clear error message when the migration file doesn't export anything" do
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
function migrate(settings) {
|
|
|
|
return settings;
|
|
|
|
}
|
|
|
|
JS
|
|
|
|
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t("themes.import_error.migrations.no_exported_function", name: "0001-some-name"),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "raises a clear error message when the migration file exports the default as something that's not a function" do
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
export function migrate(settings) {
|
|
|
|
return settings;
|
|
|
|
}
|
|
|
|
|
|
|
|
const AA = 1;
|
|
|
|
export default AA;
|
|
|
|
JS
|
|
|
|
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t(
|
|
|
|
"themes.import_error.migrations.default_export_not_a_function",
|
|
|
|
name: "0001-some-name",
|
|
|
|
),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "raises a clear error message when the migration function doesn't return anything" do
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
export default function migrate(settings) {}
|
|
|
|
JS
|
|
|
|
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t("themes.import_error.migrations.no_returned_value", name: "0001-some-name"),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "raises a clear error message when the migration function doesn't return a Map" do
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
export default function migrate(settings) {
|
|
|
|
return {};
|
|
|
|
}
|
|
|
|
JS
|
|
|
|
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t("themes.import_error.migrations.wrong_return_type", name: "0001-some-name"),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "surfaces runtime errors that occur within the migration" do
|
|
|
|
migration_field.update!(value: <<~JS)
|
|
|
|
export default function migrate(settings) {
|
|
|
|
null.toString();
|
|
|
|
return settings;
|
|
|
|
}
|
|
|
|
JS
|
|
|
|
|
|
|
|
expect do described_class.new(theme).run end.to raise_error(
|
|
|
|
Theme::SettingsMigrationError,
|
|
|
|
I18n.t(
|
|
|
|
"themes.import_error.migrations.runtime_error",
|
|
|
|
name: "0001-some-name",
|
|
|
|
error: "TypeError: Cannot read properties of null (reading 'toString')",
|
|
|
|
),
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
it "returns a list of objects that each has data representing the migration and the results" do
|
|
|
|
results = described_class.new(theme).run
|
|
|
|
|
|
|
|
expect(results[0][:version]).to eq(1)
|
|
|
|
expect(results[0][:name]).to eq("some-name")
|
|
|
|
expect(results[0][:original_name]).to eq("0001-some-name")
|
|
|
|
expect(results[0][:theme_field_id]).to eq(migration_field.id)
|
|
|
|
expect(results[0][:settings_before]).to eq({})
|
|
|
|
expect(results[0][:settings_after]).to eq({})
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|