From 2215fa0c8e7c352e83f8753aa1ea5545cbbf664f Mon Sep 17 00:00:00 2001
From: Osama Sayegh <asooomaasoooma90@gmail.com>
Date: Thu, 25 Apr 2024 16:39:22 +0300
Subject: [PATCH] FIX: Pass values of objects typed settings to theme
 migrations (#26751)

This commit fixes a bug in theme settings migrations where values of `objects` typed theme settings aren't passed to migrations even when there are overriding values for those settings. What causes this bug is that, when creating the hash that contains all the overridden settings and will be passed to migrations, the values of `objects` typed settings are incorrectly retrieved from the `value` column (which is always nil for `objects` type) instead of `json_value`. `objects` settings are different from all other types in that they store their values in the `json_value` column and they need to be special-cased when retrieving their values.
---
 lib/theme_settings_manager.rb                 |  6 ++-
 lib/theme_settings_manager/objects.rb         |  4 ++
 .../theme_settings_migrations_runner_spec.rb  | 54 +++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/lib/theme_settings_manager.rb b/lib/theme_settings_manager.rb
index 300c026ff99..7fe1fd47073 100644
--- a/lib/theme_settings_manager.rb
+++ b/lib/theme_settings_manager.rb
@@ -10,7 +10,7 @@ class ThemeSettingsManager
   def self.cast_row_value(row)
     type_name = self.types.invert[row.data_type].downcase.capitalize
     klass = "ThemeSettingsManager::#{type_name}".constantize
-    klass.cast(row.value)
+    klass.cast(klass.extract_value_from_row(row))
   end
 
   def self.create(name, default, type, theme, opts = {})
@@ -23,6 +23,10 @@ class ThemeSettingsManager
     value
   end
 
+  def self.extract_value_from_row(row)
+    row.value
+  end
+
   def initialize(name, default, theme, opts = {})
     @name = name.to_sym
     @default = default
diff --git a/lib/theme_settings_manager/objects.rb b/lib/theme_settings_manager/objects.rb
index 70a15d7260c..47874373259 100644
--- a/lib/theme_settings_manager/objects.rb
+++ b/lib/theme_settings_manager/objects.rb
@@ -1,6 +1,10 @@
 # frozen_string_literal: true
 
 class ThemeSettingsManager::Objects < ThemeSettingsManager
+  def self.extract_value_from_row(row)
+    row.json_value
+  end
+
   def value
     has_record? ? db_record.json_value : default.map!(&:deep_stringify_keys)
   end
diff --git a/spec/services/theme_settings_migrations_runner_spec.rb b/spec/services/theme_settings_migrations_runner_spec.rb
index 2b1e760e003..a986e785a28 100644
--- a/spec/services/theme_settings_migrations_runner_spec.rb
+++ b/spec/services/theme_settings_migrations_runner_spec.rb
@@ -36,6 +36,60 @@ describe ThemeSettingsMigrationsRunner do
       )
     end
 
+    it "passes values of `objects` typed settings to migrations and the values are parsed (not json string)" do
+      settings_field.update!(value: <<~YAML)
+        objects_setting:
+          type: objects
+          default:
+            - text: "hello, default link"
+              url: "https://google.com"
+            - text: "hi, another default link"
+              url: "https://discourse.org"
+          schema:
+            name: link
+            properties:
+              text:
+                type: string
+              url:
+                type: string
+      YAML
+      theme.update_setting(
+        :objects_setting,
+        [{ text: "custom link 1", url: "https://meta.discourse.org" }],
+      )
+      theme.save!
+
+      migration_field.update!(value: <<~JS)
+        export default function migrate(settings) {
+          settings.get("objects_setting").push(
+            {
+              text: "another custom link",
+              url: "https://try.discourse.org"
+            }
+          )
+          return settings;
+        }
+      JS
+
+      results = described_class.new(theme).run
+
+      expect(results.first[:settings_before]).to eq(
+        {
+          "objects_setting" => [
+            { "url" => "https://meta.discourse.org", "text" => "custom link 1" },
+          ],
+        },
+      )
+      expect(results.first[:settings_after]).to eq(
+        {
+          "objects_setting" => [
+            { "url" => "https://meta.discourse.org", "text" => "custom link 1" },
+            { "url" => "https://try.discourse.org", "text" => "another custom link" },
+          ],
+        },
+      )
+    end
+
     it "passes the output of the previous migration as input to the next one" do
       theme.update_setting(:integer_setting, 1)