DEV: Support description for properties in objects schema (#26172)

Why this change?

When editing a objects typed theme setting, the input fields which are
rendered should include a description so that the user knows the purpose
of the field which they are changing.

What does this change do?

This change adds support for adding description to each property in the
schema for an object by following a given convention in the locale file.

For a schema like this:

```
objects_setting:
  type: objects
  schema:
    name: section
    properties:
      name:
        type: string
        required: true
      links:
        type: objects
        schema:
          name: link
          properties:
            name:
              type: string
              required: true
              validations:
                max_length: 20
            url:
              type: string
```

Description for each property in the object can be added like so:

```
en:
  theme_metadata:
    settings:
      objects_setting:
        description: <description> for the setting
        schema:
          properties:
            name: <description for the name property>
            links:
              name: <description for the name property in link>
              url: <description for the url property in link>
```

If the a description is not present, the input field will simply not
have an description.

Also note that a description for a theme setting can now be added like
so:

```
en:
  theme_metadata:
    settings:
      some_other_setting: <This will be used as the description>
      objects_setting:
        description: <This will also be used as the description>
```

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

@@ -18,13 +18,15 @@ class Node {
   schema;
   index;
   active = false;
+  parentTree;
   trees = [];
 
-  constructor({ text, index, object, schema }) {
+  constructor({ text, index, object, schema, parentTree }) {
     this.text = text;
     this.index = index;
     this.object = object;
     this.schema = schema;
+    this.parentTree = parentTree;
   }
 }
 
@@ -47,20 +49,21 @@ export default class SchemaThemeSettingEditor extends Component {
   get tree() {
     let schema = this.schema;
     let data = this.data;
+    let tree = new Tree();
 
     for (const point of this.history) {
+      tree.propertyName = point.propertyName;
       data = data[point.node.index][point.propertyName];
       schema = schema.properties[point.propertyName].schema;
     }
 
-    const tree = new Tree();
-
     data.forEach((object, index) => {
       const node = new Node({
         index,
         schema,
         object,
-        text: object[schema.identifier],
+        text: object[schema.identifier] || `${schema.name} ${index + 1}`,
+        parentTree: tree,
       });
 
       if (index === this.activeIndex) {
@@ -78,10 +81,13 @@ export default class SchemaThemeSettingEditor extends Component {
             (childObj, childIndex) => {
               subtree.nodes.push(
                 new Node({
-                  text: childObj[childObjectsProperty.schema.identifier],
+                  text:
+                    childObj[childObjectsProperty.schema.identifier] ||
+                    `${childObjectsProperty.schema.name} ${childIndex + 1}`,
                   index: childIndex,
                   object: childObj,
                   schema: childObjectsProperty.schema,
+                  parentTree: subtree,
                 })
               );
             }
@@ -117,6 +123,7 @@ export default class SchemaThemeSettingEditor extends Component {
         name,
         spec,
         value: node.object[name],
+        description: this.fieldDescription(name),
       });
     }
 
@@ -198,6 +205,24 @@ export default class SchemaThemeSettingEditor extends Component {
     this.activeNode.object[field.name] = newVal;
   }
 
+  fieldDescription(fieldName) {
+    const descriptions = this.args.setting.objects_schema_property_descriptions;
+
+    if (!descriptions) {
+      return;
+    }
+
+    let key;
+
+    if (this.activeNode.parentTree.propertyName) {
+      key = `${this.activeNode.parentTree.propertyName}.${fieldName}`;
+    } else {
+      key = `${fieldName}`;
+    }
+
+    return descriptions[key];
+  }
+
   <template>
     <div class="schema-editor-navigation">
       {{#if this.backButtonText}}
@@ -243,6 +268,7 @@ export default class SchemaThemeSettingEditor extends Component {
           @value={{field.value}}
           @spec={{field.spec}}
           @onValueChange={{fn this.inputFieldChanged field}}
+          @description={{field.description}}
         />
       {{/each}}
     </div>

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

@@ -1,4 +1,6 @@
 import Component from "@glimmer/component";
+import { cached } from "@glimmer/tracking";
+import htmlSafe from "discourse-common/helpers/html-safe";
 import BooleanField from "./types/boolean";
 import CategoryField from "./types/category";
 import EnumField from "./types/enum";
@@ -32,16 +34,32 @@ export default class SchemaThemeSettingField extends Component {
     }
   }
 
+  @cached
+  get description() {
+    return this.args.description.trim().replace(/\n/g, "<br>");
+  }
+
+  get hasDescription() {
+    return this.args.description?.length > 0;
+  }
+
   <template>
     <div class="schema-field" data-name={{@name}}>
-      <label>{{@name}}</label>
+      <label class="schema-field__label">{{@name}}</label>
-      <div class="input">
+
+      <div class="schema-field__input">
         <this.component
           @value={{@value}}
           @spec={{@spec}}
           @onChange={{@onValueChange}}
         />
       </div>
+
+      {{#if this.hasDescription}}
+        <div class="schema-field__description">
+          {{htmlSafe this.description}}
+        </div>
+      {{/if}}
     </div>
   </template>
 }

app/assets/javascripts/discourse/tests/fixtures/theme-setting-schema-data.js

@@ -2,6 +2,7 @@ import ThemeSettings from "admin/models/theme-settings";
 
 export default function schemaAndData(version = 1) {
   let schema, data;
+
   if (version === 1) {
     schema = {
       name: "level1",

app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs

@@ -48,8 +48,8 @@ class InputFieldsFromDOM {
     [...queryAll(".schema-field")].forEach((field) => {
       this.count += 1;
       this.fields[field.dataset.name] = {
-        labelElement: field.querySelector("label"),
+        labelElement: field.querySelector(".schema-field__label"),
-        inputElement: field.querySelector(".input").children[0],
+        inputElement: field.querySelector(".schema-field__input").children[0],
         selector: `.schema-field[data-name="${field.dataset.name}"]`,
       };
     });
@@ -606,6 +606,69 @@ module(
       assert.strictEqual(groupSelector.header().value(), "74");
     });
 
+    test("generic identifier is used when identifier is not specified in the schema", async function (assert) {
+      const setting = ThemeSettings.create({
+        setting: "objects_setting",
+        objects_schema: {
+          name: "section",
+          properties: {
+            name: {
+              type: "string",
+            },
+            links: {
+              type: "objects",
+              schema: {
+                name: "link",
+                properties: {
+                  title: {
+                    type: "string",
+                  },
+                },
+              },
+            },
+          },
+        },
+        value: [
+          {
+            name: "some section",
+            links: [
+              {
+                title: "some title",
+              },
+              {
+                title: "some other title",
+              },
+            ],
+          },
+          {
+            name: "some section 2",
+            links: [
+              {
+                title: "some title 3",
+              },
+            ],
+          },
+        ],
+      });
+
+      await render(<template>
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
+      </template>);
+
+      const tree = new TreeFromDOM();
+
+      assert.dom(tree.nodes[0].element).hasText("section 1");
+      assert.dom(tree.nodes[0].children[0].element).hasText("link 1");
+      assert.dom(tree.nodes[0].children[1].element).hasText("link 2");
+      assert.dom(tree.nodes[1].element).hasText("section 2");
+
+      await click(tree.nodes[1].element);
+
+      tree.refresh();
+
+      assert.dom(tree.nodes[1].children[0].element).hasText("link 1");
+    });
+
     test("identifier field instantly updates in the navigation tree when the input field is changed", async function (assert) {
       const setting = schemaAndData(2);
 

app/assets/stylesheets/common/admin/admin_base.scss

@@ -1050,6 +1050,7 @@ a.inline-editable-field {
 @import "common/admin/badges";
 @import "common/admin/emails";
 @import "common/admin/json_schema_editor";
+@import "common/admin/schema_field";
 @import "common/admin/staff_logs";
 @import "common/admin/customize";
 @import "common/admin/customize-install-theme";

app/assets/stylesheets/common/admin/schema_field.scss

@@ -0,0 +1,16 @@
+.schema-field {
+  margin-bottom: 1em;
+
+  .schema-field__input {
+    input {
+      margin-bottom: 0;
+    }
+
+    margin-bottom: 0.3em;
+  }
+
+  .schema-field__description {
+    font-size: var(--font-down-1);
+    color: var(--primary-medium);
+  }
+}

app/serializers/theme_settings_serializer.rb

@@ -6,6 +6,7 @@ class ThemeSettingsSerializer < ApplicationSerializer
              :default,
              :value,
              :description,
+             :objects_schema_property_descriptions,
              :valid_values,
              :list_type,
              :textarea,
@@ -29,15 +30,31 @@ class ThemeSettingsSerializer < ApplicationSerializer
   end
 
   def description
+    description_regexp = /^theme_metadata\.settings\.#{setting}(\.description)?$/
+
     locale_file_description =
-      object
+      object.theme.internal_translations.find { |t| t.key.match?(description_regexp) }&.value
-        .theme
+
-        .internal_translations
-        .find { |t| t.key == "theme_metadata.settings.#{setting}" }
-        &.value
     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

spec/fixtures/theme_locales/objects_settings/en.yaml

@@ -0,0 +1,11 @@
+en:
+  theme_metadata:
+    settings:
+      objects_setting:
+        description: "This is a description for objects setting"
+        schema:
+          properties:
+            name: "Section Name"
+            links:
+              name: "Name of the link"
+              url: "URL of the link"

spec/fixtures/theme_settings/objects_settings.yaml

@@ -16,7 +16,7 @@ objects_setting:
         - name: "link 4"
           url: "http://example.com"
   schema:
-    name: sections
+    name: section
     properties:
       name:
         type: string

spec/serializers/theme_settings_serializer_spec.rb

@@ -3,7 +3,6 @@
 RSpec.describe ThemeSettingsSerializer do
   fab!(:theme)
 
-  describe "#objects_schema" do
   let(:objects_setting) do
     yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml")
     theme.set_field(target: :settings, name: "yaml", value: yaml)
@@ -11,12 +10,52 @@ RSpec.describe ThemeSettingsSerializer do
     theme.settings[:objects_setting]
   end
 
+  describe "#objects_schema" do
     before { SiteSetting.experimental_objects_type_for_theme_settings = true }
 
     it "should include the attribute when theme setting is typed objects" do
       payload = ThemeSettingsSerializer.new(objects_setting).as_json
 
-      expect(payload[:theme_settings][:objects_schema][:name]).to eq("sections")
+      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

@@ -28,6 +28,35 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do
   end
 
   describe "when editing a theme setting of objects type" do
+    it "should display description for each property if the description has been configured in a locale file" do
+      theme.set_field(
+        target: :translations,
+        name: "en",
+        value: File.read("#{Rails.root}/spec/fixtures/theme_locales/objects_settings/en.yaml"),
+      )
+
+      theme.save!
+
+      admin_objects_theme_setting_editor_page.visit(theme, "objects_setting")
+
+      expect(admin_objects_theme_setting_editor_page).to have_setting_field_description(
+        "name",
+        "Section Name",
+      )
+
+      admin_objects_theme_setting_editor_page.click_link("link 1")
+
+      expect(admin_objects_theme_setting_editor_page).to have_setting_field_description(
+        "name",
+        "Name of the link",
+      )
+
+      expect(admin_objects_theme_setting_editor_page).to have_setting_field_description(
+        "url",
+        "URL of the link",
+      )
+    end
+
     it "should allow admin to edit the theme setting of objects type" do
       visit("/admin/customize/themes/#{theme.id}")
 

spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb

@@ -3,10 +3,24 @@
 module PageObjects
   module Pages
     class AdminObjectsThemeSettingEditor < PageObjects::Pages::Base
+      def visit(theme, setting_name)
+        page.visit "/admin/customize/themes/#{theme.id}/schema/#{setting_name}"
+        self
+      end
+
       def has_setting_field?(field_name, value)
         expect(input_field(field_name).value).to eq(value)
       end
 
+      def has_setting_field_description?(field_name, description)
+        expect(input_field_description(field_name)).to have_text(description)
+      end
+
+      def click_link(name)
+        find(".schema-editor-navigation .node", text: name).click
+        self
+      end
+
       def fill_in_field(field_name, value)
         input_field(field_name).fill_in(with: value)
         self
@@ -20,7 +34,11 @@ module PageObjects
       private
 
       def input_field(field_name)
-        page.find(".schema-field[data-name=\"#{field_name}\"] input")
+        page.find(".schema-field[data-name=\"#{field_name}\"] .schema-field__input input")
+      end
+
+      def input_field_description(field_name)
+        page.find(".schema-field[data-name=\"#{field_name}\"] .schema-field__description")
       end
     end
   end