DEV: Change group type to groups type for theme object schema (#26417)

Why this change?

This is a follow-up to 86b2e3a.

Basically, we want to allow people to select more than 1 group as well.

What does this change do?

1. Change `type: group` to `type: groups` and support `min` and `max`
   validations for `type: groups`.

2. Fix the `<SchemaThemeSetting::Types::Groups>` component to support the
   `min` and `max` validations and switch it to use the `<GroupChooser>` component
   instead of the `<ComboBoxComponent>` component which previously only supported
   selecting a single group.

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

@@ -1,14 +1,14 @@
 import Component from "@glimmer/component";
 import { cached } from "@glimmer/tracking";
 import htmlSafe from "discourse-common/helpers/html-safe";
-import BooleanField from "./types/boolean";
+import BooleanField from "admin/components/schema-theme-setting/types/boolean";
-import CategoriesField from "./types/categories";
+import CategoriesField from "admin/components/schema-theme-setting/types/categories";
-import EnumField from "./types/enum";
+import EnumField from "admin/components/schema-theme-setting/types/enum";
-import FloatField from "./types/float";
+import FloatField from "admin/components/schema-theme-setting/types/float";
-import GroupField from "./types/group";
+import GroupsField from "admin/components/schema-theme-setting/types/groups";
-import IntegerField from "./types/integer";
+import IntegerField from "admin/components/schema-theme-setting/types/integer";
-import StringField from "./types/string";
+import StringField from "admin/components/schema-theme-setting/types/string";
-import TagsField from "./types/tags";
+import TagsField from "admin/components/schema-theme-setting/types/tags";
 
 export default class SchemaThemeSettingField extends Component {
   get component() {
@@ -29,8 +29,8 @@ export default class SchemaThemeSettingField extends Component {
         return CategoriesField;
       case "tags":
         return TagsField;
-      case "group":
+      case "groups":
-        return GroupField;
+        return GroupsField;
       default:
         throw new Error(`unknown type ${type}`);
     }

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

@@ -1,38 +0,0 @@
-import Component from "@glimmer/component";
-import { tracked } from "@glimmer/tracking";
-import { action } from "@ember/object";
-import { service } from "@ember/service";
-import FieldInputDescription from "admin/components/schema-theme-setting/field-input-description";
-import ComboBoxComponent from "select-kit/components/combo-box";
-
-export default class SchemaThemeSettingTypeGroup extends Component {
-  @service site;
-  @tracked value = this.args.value;
-
-  required = this.args.spec.required;
-
-  @action
-  onInput(newVal) {
-    this.value = newVal;
-    this.args.onChange(newVal);
-  }
-
-  get groupChooserOptions() {
-    return {
-      clearable: !this.required,
-      filterable: true,
-      none: null,
-    };
-  }
-
-  <template>
-    <ComboBoxComponent
-      @content={{this.site.groups}}
-      @value={{this.value}}
-      @onChange={{this.onInput}}
-      @options={{this.groupChooserOptions}}
-    />
-
-    <FieldInputDescription @description={{@description}} />
-  </template>
-}

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

@@ -0,0 +1,69 @@
+import Component from "@glimmer/component";
+import { tracked } from "@glimmer/tracking";
+import { action } from "@ember/object";
+import { service } from "@ember/service";
+import { and, not } from "truth-helpers";
+import I18n from "discourse-i18n";
+import FieldInputDescription from "admin/components/schema-theme-setting/field-input-description";
+import GroupChooser from "select-kit/components/group-chooser";
+
+export default class SchemaThemeSettingTypeGroups extends Component {
+  @service site;
+  @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 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.groups.at_least", {
+        count: this.min || 1,
+      });
+    }
+  }
+
+  get groupChooserOptions() {
+    return {
+      clearable: !this.required,
+      filterable: true,
+      maximum: this.max,
+    };
+  }
+
+  <template>
+    <GroupChooser
+      @content={{this.site.groups}}
+      @value={{this.value}}
+      @onChange={{this.onInput}}
+      @options={{this.groupChooserOptions}}
+    />
+
+    <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

@@ -187,7 +187,7 @@ export default function schemaAndData(version = 1) {
           type: "categories",
         },
         group_field: {
-          type: "group",
+          type: "groups",
         },
         tags_field: {
           type: "tags",

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

@@ -859,25 +859,28 @@ module(
       );
     });
 
-    test("input fields of type group", async function (assert) {
+    test("input fields of type groups", async function (assert) {
       const setting = ThemeSettings.create({
         setting: "objects_setting",
         objects_schema: {
           name: "something",
-          identifier: "id",
           properties: {
-            required_group: {
+            required_groups: {
-              type: "group",
+              type: "groups",
               required: true,
             },
-            not_required_group: {
+            groups_with_validations: {
-              type: "group",
+              type: "groups",
+              validations: {
+                min: 2,
+                max: 3,
+              },
             },
           },
         },
         value: [
           {
-            required_group: 6,
+            required_groups: [0, 1],
           },
         ],
       });
@@ -888,29 +891,59 @@ module(
 
       const inputFields = new InputFieldsFromDOM();
 
-      assert
+      let groupsSelector = selectKit(
-        .dom(inputFields.fields.required_group.labelElement)
+        `${inputFields.fields.required_groups.selector} .select-kit`
-        .hasText("required_group*");
-
-      let groupSelector = selectKit(
-        `${inputFields.fields.required_group.selector} .select-kit`
       );
 
-      assert.strictEqual(groupSelector.header().value(), "6");
+      assert.strictEqual(groupsSelector.header().value(), "0,1");
-      assert.dom(groupSelector.clearButton()).doesNotExist("is not clearable");
 
-      assert
+      await groupsSelector.expand();
-        .dom(inputFields.fields.not_required_group.labelElement)
+      await groupsSelector.deselectItemByValue("0");
-        .hasText("not_required_group");
+      await groupsSelector.deselectItemByValue("1");
+      await groupsSelector.collapse();
 
-      groupSelector = selectKit(
+      inputFields.refresh();
-        `${inputFields.fields.not_required_group.selector} .select-kit`
+
+      assert.dom(inputFields.fields.required_groups.errorElement).hasText(
+        I18n.t("admin.customize.theme.schema.fields.groups.at_least", {
+          count: 1,
+        })
       );
 
-      await groupSelector.expand();
+      assert
-      await groupSelector.selectRowByIndex(1);
+        .dom(inputFields.fields.groups_with_validations.labelElement)
+        .hasText("groups_with_validations");
 
-      assert.dom(groupSelector.clearButton()).exists("is clearable");
+      groupsSelector = selectKit(
+        `${inputFields.fields.groups_with_validations.selector} .select-kit`
+      );
+
+      assert.strictEqual(groupsSelector.header().value(), null);
+
+      await groupsSelector.expand();
+      await groupsSelector.selectRowByIndex(1);
+      await groupsSelector.collapse();
+
+      assert.strictEqual(groupsSelector.header().value(), "1");
+
+      inputFields.refresh();
+
+      assert
+        .dom(inputFields.fields.groups_with_validations.errorElement)
+        .hasText(
+          I18n.t("admin.customize.theme.schema.fields.groups.at_least", {
+            count: 2,
+          })
+        );
+
+      await groupsSelector.expand();
+      await groupsSelector.selectRowByIndex(2);
+      await groupsSelector.selectRowByIndex(3);
+      await groupsSelector.selectRowByIndex(4);
+
+      assert
+        .dom(groupsSelector.error())
+        .hasText("You can only select 3 items.");
     });
 
     test("generic identifier is used when identifier is not specified in the schema", async function (assert) {

config/locales/client.en.yml

@@ -5654,6 +5654,10 @@ en:
             back_button: "Back to %{name}"
             fields:
               required: "*required"
+              groups:
+                at_least:
+                  one: "at least %{count} group is required"
+                  other: "at least %{count} groups are required"
               categories:
                 at_least:
                   one: "at least %{count} category is required"

config/locales/server.en.yml

@@ -170,8 +170,12 @@ en:
         humanize_not_valid_post_value: "The property at JSON Pointer '%{property_json_pointer}' must be a valid post id."
         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."
+        humanize_not_valid_groups_value: "The property at JSON Pointer '%{property_json_pointer}' must be an array of valid group ids."
-        not_valid_group_value: "must be a valid group id"
+        not_valid_groups_value: "must be an array of valid group ids"
+        humanize_groups_value_not_valid_min: "The property at JSON Pointer '%{property_json_pointer}' must have at least %{min} group ids."
+        groups_value_not_valid_min: "must have at least %{min} group ids"
+        humanize_groups_value_not_valid_max: "The property at JSON Pointer '%{property_json_pointer}' must have at most %{max} group ids."
+        groups_value_not_valid_max: "must have at most %{max} group ids"
 
         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"

lib/theme_settings_object_validator.rb

@@ -124,7 +124,7 @@ class ThemeSettingsObjectValidator
       case type
       when "string"
         value.is_a?(String)
-      when "integer", "topic", "post", "group", "upload"
+      when "integer", "topic", "post", "upload"
         value.is_a?(Integer)
       when "float"
         value.is_a?(Float) || value.is_a?(Integer)
@@ -132,7 +132,7 @@ class ThemeSettingsObjectValidator
         [true, false].include?(value)
       when "enum"
         property_attributes[:choices].include?(value)
-      when "categories"
+      when "categories", "groups"
         value.is_a?(Array) && value.all? { |id| id.is_a?(Integer) }
       when "tags"
         value.is_a?(Array) && value.all? { |tag| tag.is_a?(String) }
@@ -157,12 +157,12 @@ class ThemeSettingsObjectValidator
     return true if value.nil?
 
     case type
-    when "topic", "upload", "post", "group"
+    when "topic", "upload", "post"
       if !valid_ids(type).include?(value)
         add_error(property_name, :"not_valid_#{type}_value")
         return false
       end
-    when "tags", "categories"
+    when "tags", "categories", "groups"
       if !Array(value).to_set.subset?(valid_ids(type))
         add_error(property_name, :"not_valid_#{type}_value")
         return false
@@ -240,7 +240,7 @@ class ThemeSettingsObjectValidator
     "post" => {
       klass: Post,
     },
-    "group" => {
+    "groups" => {
       klass: Group,
     },
     "upload" => {

spec/lib/theme_settings_object_validator_spec.rb

@@ -845,19 +845,19 @@ RSpec.describe ThemeSettingsObjectValidator do
       end
     end
 
-    context "for group properties" do
+    context "for groups properties" do
-      it "should not return any error message when the value of the property is a valid id of a group record" do
+      it "should not return any error message when the value of the property is an array of valid group record ids" do
         group = Fabricate(:group)
 
-        schema = { name: "section", properties: { group_property: { type: "group" } } }
+        schema = { name: "section", properties: { groups_property: { type: "groups" } } }
 
         expect(
-          described_class.new(schema: schema, object: { group_property: group.id }).validate,
+          described_class.new(schema: schema, object: { groups_property: [group.id] }).validate,
         ).to eq({})
       end
 
       it "should not return any error messages when the value is not present and it's not required in the schema" do
-        schema = { name: "section", properties: { group_property: { type: "group" } } }
+        schema = { name: "section", properties: { groups_property: { type: "groups" } } }
         expect(described_class.new(schema: schema, object: {}).validate).to eq({})
       end
 
@@ -865,44 +865,85 @@ RSpec.describe ThemeSettingsObjectValidator do
         schema = {
           name: "section",
           properties: {
-            group_property: {
+            groups_property: {
-              type: "group",
+              type: "groups",
               required: true,
             },
           },
         }
         errors = described_class.new(schema: schema, object: {}).validate
 
-        expect(errors.keys).to eq(["/group_property"])
+        expect(errors.keys).to eq(["/groups_property"])
-        expect(errors["/group_property"].full_messages).to contain_exactly("must be present")
+        expect(errors["/groups_property"].full_messages).to contain_exactly("must be present")
       end
 
-      it "should return the right hash of error messages when value of property is not an integer" do
+      it "should return the right hash of error messages when value of property is not an array of valid group ids" do
-        schema = { name: "section", properties: { group_property: { type: "group" } } }
+        schema = { name: "section", properties: { groups_property: { type: "groups" } } }
 
-        errors = described_class.new(schema: schema, object: { group_property: "string" }).validate
+        errors = described_class.new(schema: schema, object: { groups_property: "string" }).validate
 
-        expect(errors.keys).to eq(["/group_property"])
+        expect(errors.keys).to eq(["/groups_property"])
 
-        expect(errors["/group_property"].full_messages).to contain_exactly(
+        expect(errors["/groups_property"].full_messages).to contain_exactly(
-          "must be a valid group id",
+          "must be an array of valid group ids",
         )
       end
 
-      it "should return the right hash of error messages when value of property is not a valid id of a group record" do
+      it "should return the right hash of error messages when number of groups ids does not satisfy min or max validations" do
+        group_1 = Fabricate(:group)
+        group_2 = Fabricate(:group)
+        group_3 = Fabricate(:group)
+
         schema = {
           name: "section",
           properties: {
             group_property: {
-              type: "group",
+              type: "groups",
+              validations: {
+                min: 1,
+                max: 2,
+              },
+            },
+          },
+        }
+
+        errors = described_class.new(schema: schema, object: { group_property: [] }).validate
+
+        expect(errors.keys).to eq(["/group_property"])
+
+        expect(errors["/group_property"].full_messages).to contain_exactly(
+          "must have at least 1 group ids",
+        )
+
+        errors =
+          described_class.new(
+            schema: schema,
+            object: {
+              group_property: [group_1.id, group_2.id, group_3.id],
+            },
+          ).validate
+
+        expect(errors.keys).to eq(["/group_property"])
+
+        expect(errors["/group_property"].full_messages).to contain_exactly(
+          "must have at most 2 group ids",
+        )
+      end
+
+      it "should return the right hash of error messages when value of property is an array containing invalid group ids" do
+        schema = {
+          name: "section",
+          properties: {
+            groups_property: {
+              type: "groups",
             },
             child_groups: {
               type: "objects",
               schema: {
                 name: "child_group",
                 properties: {
-                  group_property_2: {
+                  groups_property_2: {
-                    type: "group",
+                    type: "groups",
                   },
                 },
               },
@@ -916,19 +957,19 @@ RSpec.describe ThemeSettingsObjectValidator do
               described_class.new(
                 schema:,
                 object: {
-                  group_property: 99_999_999,
+                  groups_property: [99_999_999],
-                  child_groups: [{ group_property_2: 99_999_999 }],
+                  child_groups: [{ groups_property_2: [99_999_999] }],
                 },
               ).validate
 
-            expect(errors.keys).to eq(%w[/group_property /child_groups/0/group_property_2])
+            expect(errors.keys).to eq(%w[/groups_property /child_groups/0/groups_property_2])
 
-            expect(errors["/group_property"].full_messages).to contain_exactly(
+            expect(errors["/groups_property"].full_messages).to contain_exactly(
-              "must be a valid group id",
+              "must be an array of valid group ids",
             )
 
-            expect(errors["/child_groups/0/group_property_2"].full_messages).to contain_exactly(
+            expect(errors["/child_groups/0/groups_property_2"].full_messages).to contain_exactly(
-              "must be a valid group id",
+              "must be an array of valid group ids",
             )
           end