DEV: Change tag type to tags type for theme object schema (discou…

…rse#26315)

Why this change?

While working on the tag selector for the theme object editor, I
realised that there is an extremely high possibility that users might want to select
more than one tag. By supporting the ability to select more than one
tag, it also means that we get support for a single tag for free as
well.

What does this change do?

1. Change `type: tag` to `type: tags` and support `min` and `max`
   validations for `type: tags`.

2. Fix the `<SchemaThemeSetting::Types::Tags>` component to support the
   `min` and `max` validations

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

@@ -8,10 +8,12 @@ import FloatField from "./types/float";
 import GroupField from "./types/group";
 import IntegerField from "./types/integer";
 import StringField from "./types/string";
-import TagField from "./types/tag";
+import TagsField from "./types/tags";
 
 export default class SchemaThemeSettingField extends Component {
   get component() {
+    const type = this.args.spec.type;
+
     switch (this.args.spec.type) {
       case "string":
         return StringField;
@@ -25,12 +27,12 @@ export default class SchemaThemeSettingField extends Component {
         return EnumField;
       case "category":
         return CategoryField;
-      case "tag":
-        return TagField;
+      case "tags":
+        return TagsField;
       case "group":
         return GroupField;
       default:
-        throw new Error("unknown type");
+        throw new Error(`unknown type ${type}`);
     }
   }
 

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

@@ -0,0 +1,65 @@
+import Component from "@glimmer/component";
+import { tracked } from "@glimmer/tracking";
+import { action } from "@ember/object";
+import { and, not } from "truth-helpers";
+import I18n from "discourse-i18n";
+import FieldInputDescription from "admin/components/schema-theme-setting/field-input-description";
+import TagChooser from "select-kit/components/tag-chooser";
+
+export default class SchemaThemeSettingTypeTags extends Component {
+  @tracked touched = false;
+  @tracked value = this.args.value;
+  required = this.args.spec.required;
+  min = this.args.spec.validations?.min;
+  max = this.args.spec.validations?.max;
+
+  @action
+  onInput(newVal) {
+    this.touched = true;
+    this.value = newVal;
+    this.args.onChange(newVal);
+  }
+
+  get tagChooserOption() {
+    return {
+      allowAny: false,
+      maximum: this.max,
+    };
+  }
+
+  get validationErrorMessage() {
+    if (!this.touched) {
+      return;
+    }
+
+    if (
+      (this.min && this.value.length < this.min) ||
+      (this.required && (!this.value || this.value.length === 0))
+    ) {
+      return I18n.t("admin.customize.theme.schema.fields.tags.at_least_tag", {
+        count: this.min,
+      });
+    }
+  }
+
+  <template>
+    <TagChooser
+      @tags={{this.value}}
+      @onChange={{this.onInput}}
+      @options={{this.tagChooserOption}}
+      class={{if this.validationErrorMessage "--invalid"}}
+    />
+
+    <div class="schema-field__input-supporting-text">
+      {{#if (and @description (not this.validationErrorMessage))}}
+        <FieldInputDescription @description={{@description}} />
+      {{/if}}
+
+      {{#if this.validationErrorMessage}}
+        <div class="schema-field__input-error">
+          {{this.validationErrorMessage}}
+        </div>
+      {{/if}}
+    </div>
+  </template>
+}

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

@@ -189,8 +189,8 @@ export default function schemaAndData(version = 1) {
         group_field: {
           type: "group",
         },
-        tag_field: {
-          type: "tag",
+        tags_field: {
+          type: "tags",
         }
       },
     };

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

@@ -726,34 +726,76 @@ module(
       assert.dom(categorySelector.clearButton()).exists("is clearable");
     });
 
-    test("input fields of type tag", async function (assert) {
-      const setting = schemaAndData(3);
+    test("input fields of type tags which is required", async function (assert) {
+      const setting = ThemeSettings.create({
+        setting: "objects_setting",
+        objects_schema: {
+          name: "something",
+          identifier: "id",
+          properties: {
+            required_tags: {
+              type: "tags",
+              required: true,
+              validations: {
+                min: 2,
+                max: 3,
+              },
+            },
+          },
+        },
+        value: [
+          {
+            required_tags: ["gazelle", "cat"],
+          },
+        ],
+      });
 
       await render(<template>
         <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       const inputFields = new InputFieldsFromDOM();
+
       const tagSelector = selectKit(
-        `${inputFields.fields.tag_field.selector} .select-kit`
+        `${inputFields.fields.required_tags.selector} .select-kit`
       );
 
-      assert.strictEqual(tagSelector.header().value(), null);
+      assert.strictEqual(tagSelector.header().value(), "gazelle,cat");
 
       await tagSelector.expand();
-      await tagSelector.selectRowByIndex(1);
-      await tagSelector.selectRowByIndex(3);
+      await tagSelector.selectRowByIndex(2);
+      await tagSelector.collapse();
 
-      assert.strictEqual(tagSelector.header().value(), "gazelle,cat");
+      assert.strictEqual(tagSelector.header().value(), "gazelle,cat,dog");
+
+      await tagSelector.expand();
+      await tagSelector.deselectItemByName("gazelle");
+      await tagSelector.deselectItemByName("cat");
+      await tagSelector.deselectItemByName("dog");
+      await tagSelector.collapse();
 
-      const tree = new TreeFromDOM();
-      await click(tree.nodes[1].element);
       assert.strictEqual(tagSelector.header().value(), null);
 
-      tree.refresh();
+      inputFields.refresh();
 
-      await click(tree.nodes[0].element);
-      assert.strictEqual(tagSelector.header().value(), "gazelle,cat");
+      assert.dom(inputFields.fields.required_tags.errorElement).hasText(
+        I18n.t("admin.customize.theme.schema.fields.tags.at_least_tag", {
+          count: 2,
+        })
+      );
+
+      await tagSelector.expand();
+      await tagSelector.selectRowByIndex(1);
+
+      assert.strictEqual(tagSelector.header().value(), "gazelle");
+
+      inputFields.refresh();
+
+      assert.dom(inputFields.fields.required_tags.errorElement).hasText(
+        I18n.t("admin.customize.theme.schema.fields.tags.at_least_tag", {
+          count: 2,
+        })
+      );
     });
 
     test("input fields of type group", async function (assert) {

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

@@ -30,6 +30,12 @@
 
     .select-kit {
       width: 100%;
+
+      &.--invalid {
+        summary {
+          border-color: var(--danger);
+        }
+      }
     }
 
     .schema-field__input-description {

config/locales/client.en.yml

@@ -5650,6 +5650,10 @@ en:
             back_button: "Back to %{name}"
             fields:
               required: "*required"
+              tags:
+                at_least_tag:
+                  one: "at least %{count} tag is required"
+                  other: "at least %{count} tags are required"
               string:
                 too_short: "must be at least %{count} characters"
               number:

config/locales/server.en.yml

@@ -159,8 +159,12 @@ en:
         not_valid_post_value: "must be a valid post id"
         humanize_not_valid_group_value: "The property at JSON Pointer '%{property_json_pointer}' must be a valid group id."
         not_valid_group_value: "must be a valid group id"
-        humanize_not_valid_tag_value: "The property at JSON Pointer '%{property_json_pointer}' must be a valid tag name."
-        not_valid_tag_value: "must be a valid tag name"
+        humanize_not_valid_tags_value: "The property at JSON Pointer '%{property_json_pointer}' must be an array of valid tag names."
+        not_valid_tags_value: "must be an array of valid tag names"
+        humanize_tags_value_not_valid_min: "The property at JSON Pointer '%{property_json_pointer}' must have at least %{min} tag names."
+        tags_value_not_valid_min: "must have at least %{min} tag names"
+        humanize_tags_value_not_valid_max: "The property at JSON Pointer '%{property_json_pointer}' must have at most %{max} tag names."
+        tags_value_not_valid_max: "must have at most %{max} tag names"
         humanize_not_valid_upload_value: "The property at JSON Pointer '%{property_json_pointer}' must be a valid upload id."
         not_valid_upload_value: "must be a valid upload id"
         humanize_string_value_not_valid_min: "The property at JSON Pointer '%{property_json_pointer}' must be at least %{min} characters long."

lib/theme_settings_object_validator.rb

@@ -118,7 +118,7 @@ def has_valid_property_value_type?(property_attributes, property_name)
 
     is_value_valid =
       case type
-      when "string", "tag"
+      when "string"
         value.is_a?(String)
       when "integer", "category", "topic", "post", "group", "upload"
         value.is_a?(Integer)
@@ -128,6 +128,8 @@ def has_valid_property_value_type?(property_attributes, property_name)
         [true, false].include?(value)
       when "enum"
         property_attributes[:choices].include?(value)
+      when "tags"
+        value.is_a?(Array) && value.all? { |tag| tag.is_a?(String) }
       else
         add_error(property_name, :invalid_type, type:)
         return false
@@ -149,11 +151,26 @@ def has_valid_property_value?(property_attributes, property_name)
     return true if value.nil?
 
     case type
-    when "topic", "category", "upload", "post", "group", "tag"
+    when "topic", "category", "upload", "post", "group"
       if !valid_ids(type).include?(value)
         add_error(property_name, :"not_valid_#{type}_value")
         return false
       end
+    when "tags"
+      if !Array(value).to_set.subset?(valid_ids(type))
+        add_error(property_name, :"not_valid_#{type}_value")
+        return false
+      end
+
+      if (min = validations&.dig(:min)) && value.length < min
+        add_error(property_name, :tags_value_not_valid_min, min:)
+        return false
+      end
+
+      if (max = validations&.dig(:max)) && value.length > max
+        add_error(property_name, :tags_value_not_valid_max, max:)
+        return false
+      end
     when "string"
       if (min = validations&.dig(:min_length)) && value.length < min
         add_error(property_name, :string_value_not_valid_min, min:)
@@ -223,7 +240,7 @@ def valid_ids_lookup
     "upload" => {
       klass: Upload,
     },
-    "tag" => {
+    "tags" => {
       klass: Tag,
       column: :name,
     },
@@ -247,7 +264,7 @@ def fetch_property_values_of_type(properties, object, type)
 
     properties.each do |property_name, property_attributes|
       if property_attributes[:type] == type
-        values << object[property_name]
+        values.merge(Array(object[property_name]))
       elsif property_attributes[:type] == "objects"
         object[property_name]&.each do |child_object|
           values.merge(