DEV: Load theme objects typed setting metadata when routing to editor (#26354)

Why this change?

Previously, we were preloading the necessary metadata for
`adminCustomizeThemes.show.schema` route in the
`adminCustomizeThemes.show` route. This is wasteful because we're
loading data upfront when the objects setting editor may not be used.

This change also lays the ground work for a future commit where we need
to be shipping down additional metadata which may further add to the
payload.

app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs

@@ -243,7 +243,7 @@ export default class SchemaThemeSettingEditor extends Component {
   }
 
   fieldDescription(fieldName) {
-    const descriptions = this.args.setting.objects_schema_property_descriptions;
+    const descriptions = this.args.setting.metadata?.property_descriptions;
 
     if (!descriptions) {
       return;

app/assets/javascripts/admin/addon/models/theme-settings.js

@@ -1,5 +1,6 @@
 import EmberObject from "@ember/object";
 import { ajax } from "discourse/lib/ajax";
+import { popupAjaxError } from "discourse/lib/ajax-error";
 import Setting from "admin/mixins/setting-object";
 
 export default class ThemeSettings extends EmberObject.extend(Setting) {
@@ -16,4 +17,12 @@ export default class ThemeSettings extends EmberObject.extend(Setting) {
       },
     });
   }
+
+  loadMetadata(themeId) {
+    return ajax(
+      `/admin/themes/${themeId}/objects_setting_metadata/${this.setting}.json`
+    )
+      .then((result) => this.set("metadata", result))
+      .catch(popupAjaxError);
+  }
 }

app/assets/javascripts/admin/addon/routes/admin-customize-themes-show-schema.js

@@ -5,10 +5,12 @@ export default class AdminCustomizeThemesShowSchemaRoute extends Route {
     const theme = this.modelFor("adminCustomizeThemesShow");
     const setting = theme.settings.findBy("setting", params.setting_name);
 
-    return {
-      theme,
-      setting,
-    };
+    return setting.loadMetadata(theme.id).then(() => {
+      return {
+        theme,
+        setting,
+      };
+    });
   }
 
   setupController() {

app/controllers/admin/themes_controller.rb

@@ -173,10 +173,9 @@ class Admin::ThemesController < Admin::AdminController
     @color_schemes = ColorScheme.all.includes(:theme, color_scheme_colors: :color_scheme).to_a
 
     payload = {
-      themes: ActiveModel::ArraySerializer.new(@themes, each_serializer: ThemeSerializer),
+      themes: serialize_data(@themes, ThemeSerializer),
       extras: {
-        color_schemes:
-          ActiveModel::ArraySerializer.new(@color_schemes, each_serializer: ColorSchemeSerializer),
+        color_schemes: serialize_data(@color_schemes, ColorSchemeSerializer),
       },
     }
 
@@ -293,7 +292,7 @@ class Admin::ThemesController < Admin::AdminController
     @theme = Theme.include_relations.find_by(id: params[:id])
     raise Discourse::InvalidParameters.new(:id) unless @theme
 
-    render json: ThemeSerializer.new(@theme)
+    render_serialized(@theme, ThemeSerializer)
   end
 
   def export
@@ -352,6 +351,18 @@ class Admin::ThemesController < Admin::AdminController
     raise Discourse::InvalidAccess if !SiteSetting.experimental_objects_type_for_theme_settings
   end
 
+  def objects_setting_metadata
+    raise Discourse::InvalidAccess if !SiteSetting.experimental_objects_type_for_theme_settings
+
+    theme = Theme.find_by(id: params[:id])
+    raise Discourse::InvalidParameters.new(:id) unless theme
+
+    theme_setting = theme.settings[params[:setting_name].to_sym]
+    raise Discourse::InvalidParameters.new(:setting_name) unless theme_setting
+
+    render_serialized(theme_setting, ThemeObjectsSettingMetadataSerializer, root: false)
+  end
+
   private
 
   def ban_in_allowlist_mode!

app/serializers/theme_objects_setting_metadata_serializer.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class ThemeObjectsSettingMetadataSerializer < ApplicationSerializer
+  attributes :property_descriptions
+
+  def property_descriptions
+    locales = {}
+    key = "theme_metadata.settings.#{object.name}.schema.properties."
+
+    object.theme.internal_translations.each do |internal_translation|
+      if internal_translation.key.start_with?(key)
+        locales[internal_translation.key.delete_prefix(key)] = internal_translation.value
+      end
+    end
+
+    locales
+  end
+end

app/serializers/theme_serializer.rb

@@ -56,7 +56,9 @@ class ThemeSerializer < BasicThemeSerializer
   end
 
   def settings
-    object.settings.map { |_name, setting| ThemeSettingsSerializer.new(setting, root: false) }
+    object.settings.map do |_name, setting|
+      ThemeSettingsSerializer.new(setting, scope:, root: false)
+    end
   rescue ThemeSettingsParser::InvalidYaml => e
     @errors << e.message
     nil

app/serializers/theme_settings_serializer.rb

@@ -6,7 +6,6 @@ class ThemeSettingsSerializer < ApplicationSerializer
              :default,
              :value,
              :description,
-             :objects_schema_property_descriptions,
              :valid_values,
              :list_type,
              :textarea,
@@ -38,23 +37,6 @@ class ThemeSettingsSerializer < ApplicationSerializer
     locale_file_description || object.description
   end
 
-  def objects_schema_property_descriptions
-    locales = {}
-    key = "theme_metadata.settings.#{setting}.schema.properties."
-
-    object.theme.internal_translations.each do |internal_translation|
-      if internal_translation.key.start_with?(key)
-        locales[internal_translation.key.delete_prefix(key)] = internal_translation.value
-      end
-    end
-
-    locales
-  end
-
-  def include_objects_schema_property_descriptions?
-    include_objects_schema?
-  end
-
   def valid_values
     object.choices
   end

config/routes.rb

@@ -235,7 +235,9 @@ Discourse::Application.routes.draw do
           get "preview" => "themes#preview"
           get "translations/:locale" => "themes#get_translations"
           put "setting" => "themes#update_single_setting"
+          get "objects_setting_metadata/:setting_name" => "themes#objects_setting_metadata"
         end
+
         collection do
           post "import" => "themes#import"
           post "upload_asset" => "themes#upload_asset"

spec/requests/admin/themes_controller_spec.rb

@@ -1313,4 +1313,83 @@ RSpec.describe Admin::ThemesController do
       delete "/admin/themes/bulk_destroy.json", params: { theme_ids: theme_ids }
     end
   end
+
+  describe "#objects_setting_metadata" do
+    fab!(:theme)
+
+    let(:theme_setting) do
+      yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml")
+      field = theme.set_field(target: :settings, name: "yaml", value: yaml)
+      theme.save!
+      theme.settings
+    end
+
+    before { SiteSetting.experimental_objects_type_for_theme_settings = true }
+
+    it "returns 404 if user is not an admin" do
+      get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_with_categories.json"
+
+      expect(response.status).to eq(404)
+
+      sign_in(user)
+
+      get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_with_categories.json"
+
+      expect(response.status).to eq(404)
+
+      sign_in(moderator)
+
+      get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_with_categories.json"
+
+      expect(response.status).to eq(404)
+    end
+
+    context "when user is an admin" do
+      before { sign_in(admin) }
+
+      it "returns 403 if `experimental_objects_type_for_theme_settings` site setting is not enabled" do
+        SiteSetting.experimental_objects_type_for_theme_settings = false
+
+        get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_with_categories.json"
+
+        expect(response.status).to eq(403)
+      end
+
+      it "returns 400 if the `id` param is not the id of a valid theme" do
+        get "/admin/themes/some_invalid_id/objects_setting_metadata/objects_with_categories.json"
+
+        expect(response.status).to eq(400)
+      end
+
+      it "returns 400 if the `setting_name` param does not match a valid setting" do
+        get "/admin/themes/#{theme.id}/objects_setting_metadata/some_invalid_setting_name.json"
+
+        expect(response.status).to eq(400)
+      end
+
+      it "returns 200 with the right `property_descriptions` attributes" do
+        theme.set_field(
+          target: :translations,
+          name: "en",
+          value: File.read("#{Rails.root}/spec/fixtures/theme_locales/objects_settings/en.yaml"),
+        )
+
+        theme.save!
+
+        theme_setting
+
+        get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_setting.json"
+
+        expect(response.status).to eq(200)
+
+        expect(response.parsed_body["property_descriptions"]).to eq(
+          {
+            "links.name" => "Name of the link",
+            "links.url" => "URL of the link",
+            "name" => "Section Name",
+          },
+        )
+      end
+    end
+  end
 end

spec/serializers/theme_objects_setting_metadata_serializer_spec.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+RSpec.describe ThemeObjectsSettingMetadataSerializer do
+  fab!(:theme)
+
+  let(:theme_setting) do
+    yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml")
+    theme.set_field(target: :settings, name: "yaml", value: yaml)
+    theme.save!
+    theme.settings
+  end
+
+  before { SiteSetting.experimental_objects_type_for_theme_settings = true }
+
+  describe "#property_descriptions" do
+    let(:objects_setting_locale) do
+      theme.set_field(
+        target: :translations,
+        name: "en",
+        value: File.read("#{Rails.root}/spec/fixtures/theme_locales/objects_settings/en.yaml"),
+      )
+
+      theme.save!
+    end
+
+    it "should return a hash of the settings property descriptions" do
+      objects_setting_locale
+
+      payload = described_class.new(theme_setting[:objects_setting], root: false).as_json
+
+      expect(payload[:property_descriptions]).to eq(
+        {
+          "links.name" => "Name of the link",
+          "links.url" => "URL of the link",
+          "name" => "Section Name",
+        },
+      )
+    end
+  end
+end

spec/serializers/theme_settings_serializer_spec.rb

@@ -19,43 +19,4 @@ RSpec.describe ThemeSettingsSerializer do
       expect(payload[:theme_settings][:objects_schema][:name]).to eq("section")
     end
   end
-
-  describe "#objects_schema_property_descriptions" do
-    let(:objects_setting_locale) do
-      theme.set_field(
-        target: :translations,
-        name: "en",
-        value: File.read("#{Rails.root}/spec/fixtures/theme_locales/objects_settings/en.yaml"),
-      )
-
-      theme.save!
-    end
-
-    before { SiteSetting.experimental_objects_type_for_theme_settings = true }
-
-    it "should not include the attribute when theme setting is not typed objects" do
-      yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/valid_settings.yaml")
-      theme.set_field(target: :settings, name: "yaml", value: yaml)
-      theme.save!
-
-      payload = ThemeSettingsSerializer.new(theme.settings[:string_setting]).as_json
-
-      expect(payload[:theme_settings][:objects_schema_property_descriptions]).to be_nil
-    end
-
-    it "should include the attribute when theme setting is of typed objects" do
-      objects_setting_locale
-      objects_setting
-
-      payload = ThemeSettingsSerializer.new(objects_setting).as_json
-
-      expect(payload[:theme_settings][:objects_schema_property_descriptions]).to eq(
-        {
-          "links.name" => "Name of the link",
-          "links.url" => "URL of the link",
-          "name" => "Section Name",
-        },
-      )
-    end
-  end
 end

spec/system/admin_editing_objects_typed_theme_setting_spec.rb

@@ -78,12 +78,19 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do
 
       expect(admin_customize_themes_page).to have_overridden_setting("objects_setting")
 
+      admin_objects_theme_setting_editor =
+        admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting")
+
+      expect(admin_objects_theme_setting_editor).to have_setting_field("name", "some new name")
+
+      admin_objects_theme_setting_editor.back
+
       admin_customize_themes_page.reset_overridden_setting("objects_setting")
 
       admin_objects_theme_setting_editor =
         admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting")
 
-      expect(admin_objects_theme_setting_editor).to have_setting_field("name", "some new name")
+      expect(admin_objects_theme_setting_editor).to have_setting_field("name", "section 1")
     end
 
     it "allows an admin to edit a theme setting of objects type via the settings editor" do

spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb

@@ -35,6 +35,11 @@ module PageObjects
         self
       end
 
+      def back
+        find(".customize-themes-show-schema__back").click
+        self
+      end
+
       private
 
       def input_field(field_name)