DEV: Introduce run_theme_migration spec helper in test environment (#26845)

This commit introduces the `run_theme_migration` spec helper to allow
theme developers to write RSpec tests for theme migrations. For example,
this allows the following RSpec test to be written in themes:

```
RSpec.describe "0003-migrate-small-links-setting migration" do
  let!(:theme) { upload_theme_component }

  it "should set target property to `_blank` if previous target component is not valid or empty" do
    theme.theme_settings.create!(
      name: "small_links",
      theme: theme,
      data_type: ThemeSetting.types[:string],
      value: "some text, #|some text 2, #, invalid target",
    )

    run_theme_migration(theme, "0003-migrate-small-links-setting")

    expect(theme.settings[:small_links].value).to eq(
      [
        { "text" => "some text", "url" => "#", "target" => "_blank" },
        { "text" => "some text 2", "url" => "#", "target" => "_blank" },
      ],
    )
  end
end
```

This change is being introduced because we realised that writting just
javascript tests for the migrations is insufficient since javascript
tests do not ensure that the migrated theme settings can actually be
successfully saved into the database. Hence, we are introduce this
helper as a way for theme developers to write "end-to-end" migrations
tests.

app/models/theme.rb

@@ -857,10 +857,11 @@ class Theme < ActiveRecord::Base
     contents
   end
 
-  def migrate_settings(start_transaction: true)
+  def migrate_settings(start_transaction: true, fields: nil, allow_out_of_sequence_migration: false)
     block = -> do
       runner = ThemeSettingsMigrationsRunner.new(self)
-      results = runner.run
+      results =
+        runner.run(fields:, raise_error_on_out_of_sequence: !allow_out_of_sequence_migration)
 
       next if results.blank?
 
@@ -893,8 +894,12 @@ class Theme < ActiveRecord::Base
             name: res[:name],
             theme_field_id: res[:theme_field_id],
           )
+
         record.calculate_diff(res[:settings_before], res[:settings_after])
-        record.save!
+
+        # If out of sequence migration is allowed we don't want to raise an error if the record is invalid due to version
+        # conflicts
+        allow_out_of_sequence_migration ? record.save : record.save!
       end
 
       self.reload

app/models/theme_settings_migration.rb

@@ -4,7 +4,7 @@ class ThemeSettingsMigration < ActiveRecord::Base
   belongs_to :theme
   belongs_to :theme_field
 
-  validates :theme_id, presence: true
+  validates :theme_id, presence: true, uniqueness: { scope: :version }
   validates :theme_field_id, presence: true
 
   validates :version, presence: true

app/services/theme_settings_migrations_runner.rb

@@ -53,7 +53,7 @@ class ThemeSettingsMigrationsRunner
     }
   JS
 
-  private_constant :Migration, :MIGRATION_ENTRY_POINT_JS
+  private_constant :MIGRATION_ENTRY_POINT_JS
 
   def self.loader_js_lib_content
     @loader_js_lib_content ||=
@@ -67,8 +67,8 @@ class ThemeSettingsMigrationsRunner
     @memory = memory
   end
 
-  def run
-    fields = lookup_pending_migrations_fields
+  def run(fields: nil, raise_error_on_out_of_sequence: true)
+    fields ||= lookup_pending_migrations_fields
 
     count = fields.count
     return [] if count == 0
@@ -80,12 +80,13 @@ class ThemeSettingsMigrationsRunner
 
     current_migration_version =
       @theme.theme_settings_migrations.order(version: :desc).pick(:version)
+
     current_migration_version ||= -Float::INFINITY
 
     current_settings = lookup_overriden_settings
 
     migrations.map do |migration|
-      if migration.version <= current_migration_version
+      if migration.version <= current_migration_version && raise_error_on_out_of_sequence
         raise_error(
           "themes.import_error.migrations.out_of_sequence",
           name: migration.original_name,
@@ -94,6 +95,7 @@ class ThemeSettingsMigrationsRunner
       end
 
       migrated_settings = execute(migration, current_settings)
+
       results = {
         version: migration.version,
         name: migration.name,
@@ -157,6 +159,7 @@ class ThemeSettingsMigrationsRunner
       .migration_fields
       .left_joins(:theme_settings_migration)
       .where(theme_settings_migration: { id: nil })
+      .order(created_at: :asc)
   end
 
   def convert_fields_to_migrations(fields)

spec/models/theme_spec.rb

@@ -341,10 +341,10 @@ HTML
       expect(scss).to include("font-size:30px")
 
       # Escapes correctly. If not, compiling this would throw an exception
-      setting.value = <<~CSS
+      setting.value = <<~SCSS
           \#{$fakeinterpolatedvariable}
           andanothervalue 'withquotes'; margin: 0;
-      CSS
+      SCSS
 
       theme.set_field(target: :common, name: :scss, value: "body {font-size: quote($font-size)}")
       theme.save!
@@ -1347,6 +1347,55 @@ HTML
         "deletions" => [{ "key" => "setting_that_will_be_removed", "val" => 1023 }],
       )
     end
+
+    it "does not raise an out of sequence error and does not create `ThemeSettingsMigration` record for out of sequence migration when `allow_out_of_sequence_migration` kwarg is set to true" do
+      second_migration_field =
+        Fabricate(
+          :migration_theme_field,
+          name: "0001-some-other-migration-name",
+          theme: theme,
+          value: <<~JS,
+          export default function migrate(settings) {
+            settings.set("integer_setting", 3);
+            return settings;
+          }
+        JS
+          version: 1,
+        )
+
+      expect do theme.migrate_settings end.to raise_error(
+        Theme::SettingsMigrationError,
+        /'0001-some-other-migration-name' is out of sequence/,
+      )
+
+      expect(theme.get_setting("integer_setting")).to eq(1)
+
+      theme.migrate_settings(allow_out_of_sequence_migration: true)
+
+      expect(theme.theme_settings_migrations.count).to eq(1)
+      expect(theme.theme_settings_migrations.first.theme_field_id).to eq(migration_field.id)
+      expect(theme.get_setting("integer_setting")).to eq(3)
+    end
+
+    it "allows custom migration fields to be run by specifing the `fields` kwarg" do
+      expect do theme.migrate_settings(fields: []) end.not_to change {
+        theme.theme_settings_migrations.count
+      }
+
+      second_migration_field =
+        Fabricate(:migration_theme_field, theme: theme, value: <<~JS, version: 2)
+          export default function migrate(settings) {
+            settings.set("integer_setting", 3);
+            return settings;
+          }
+        JS
+
+      theme.migrate_settings(fields: [second_migration_field])
+
+      expect(theme.theme_settings_migrations.count).to eq(1)
+      expect(theme.theme_settings_migrations.first.theme_field_id).to eq(second_migration_field.id)
+      expect(theme.get_setting("integer_setting")).to eq(3)
+    end
   end
 
   describe "development experience" do

spec/support/helpers.rb

@@ -229,4 +229,70 @@ module Helpers
   ensure
     SearchIndexer.disable
   end
+
+  # Uploads a theme from a directory.
+  #
+  # @param set_theme_as_default [Boolean] Whether to set the uploaded theme as the default theme for the site. Defaults to true.
+  #
+  # @return [Theme] The uploaded theme model given by `models/theme.rb`.
+  #
+  # @example Upload a theme and set it as default
+  #   upload_theme("/path/to/theme")
+  def upload_theme(set_theme_as_default: true)
+    theme = RemoteTheme.import_theme_from_directory(theme_dir_from_caller)
+
+    if theme.component
+      raise "Uploaded theme is a theme component, please use the `upload_theme_component` method instead."
+    end
+
+    theme.set_default! if set_theme_as_default
+    theme
+  end
+
+  # Uploads a theme component from a directory.
+  #
+  # @param parent_theme_id [Integer] The ID of the theme to add the theme component to. Defaults to `SiteSetting.default_theme_id`.
+  #
+  # @return [Theme] The uploaded theme model given by `models/theme.rb`.
+  #
+  # @example Upload a theme component
+  #   upload_theme_component("/path/to/theme_component")
+  #
+  # @example Upload a theme component and add it to a specific theme
+  #   upload_theme_component("/path/to/theme_component", parent_theme_id: 123)
+  def upload_theme_component(parent_theme_id: SiteSetting.default_theme_id)
+    theme = RemoteTheme.import_theme_from_directory(theme_dir_from_caller)
+
+    if !theme.component
+      raise "Uploaded theme is not a theme component, please use the `upload_theme` method instead."
+    end
+
+    Theme.find(parent_theme_id).child_themes << theme
+    theme
+  end
+
+  # Runs named migration for a given theme.
+  #
+  # @params [Theme] theme The theme to run the migration for.
+  # @params [String] migration_name The name of the migration to run.
+  #
+  # @return [nil]
+  #
+  # @example
+  #   run_theme_migration(theme, "0001-migrate-some-settings")
+  def run_theme_migration(theme, migration_name)
+    migration_theme_field = theme.theme_fields.find_by(name: migration_name)
+    theme.migrate_settings(fields: [migration_theme_field], allow_out_of_sequence_migration: true)
+    nil
+  end
+
+  private
+
+  def theme_dir_from_caller
+    caller.each do |line|
+      if (split = line.split(%r{/spec/*/.+_spec.rb})).length > 1
+        return split.first
+      end
+    end
+  end
 end

spec/support/system_helpers.rb

@@ -22,47 +22,6 @@ module SystemHelpers
     expect(page).to have_content("Signed in to #{user.encoded_username} successfully")
   end
 
-  # Uploads a theme from a directory.
-  #
-  # @param set_theme_as_default [Boolean] Whether to set the uploaded theme as the default theme for the site. Defaults to true.
-  #
-  # @return [Theme] The uploaded theme model given by `models/theme.rb`.
-  #
-  # @example Upload a theme and set it as default
-  #   upload_theme("/path/to/theme")
-  def upload_theme(set_theme_as_default: true)
-    theme = RemoteTheme.import_theme_from_directory(theme_dir_from_caller)
-
-    if theme.component
-      raise "Uploaded theme is a theme component, please use the `upload_theme_component` method instead."
-    end
-
-    theme.set_default! if set_theme_as_default
-    theme
-  end
-
-  # Uploads a theme component from a directory.
-  #
-  # @param parent_theme_id [Integer] The ID of the theme to add the theme component to. Defaults to `SiteSetting.default_theme_id`.
-  #
-  # @return [Theme] The uploaded theme model given by `models/theme.rb`.
-  #
-  # @example Upload a theme component
-  #   upload_theme_component("/path/to/theme_component")
-  #
-  # @example Upload a theme component and add it to a specific theme
-  #   upload_theme_component("/path/to/theme_component", parent_theme_id: 123)
-  def upload_theme_component(parent_theme_id: SiteSetting.default_theme_id)
-    theme = RemoteTheme.import_theme_from_directory(theme_dir_from_caller)
-
-    if !theme.component
-      raise "Uploaded theme is not a theme component, please use the `upload_theme` method instead."
-    end
-
-    Theme.find(parent_theme_id).child_themes << theme
-    theme
-  end
-
   def setup_system_test
     SiteSetting.login_required = false
     SiteSetting.has_login_hint = false
@@ -205,14 +164,4 @@ module SystemHelpers
       )
     end
   end
-
-  private
-
-  def theme_dir_from_caller
-    caller.each do |line|
-      if (split = line.split(%r{/spec/system/.+_spec.rb})).length > 1
-        return split.first
-      end
-    end
-  end
 end