diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/field.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/field.gjs index f1244071d03..a8c7715e29a 100644 --- a/app/assets/javascripts/admin/addon/components/schema-theme-setting/field.gjs +++ b/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}`); } } diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/tag.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/tag.gjs deleted file mode 100644 index 1fb8a7a8d9d..00000000000 --- a/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/tag.gjs +++ /dev/null @@ -1,26 +0,0 @@ -import Component from "@glimmer/component"; -import { tracked } from "@glimmer/tracking"; -import { hash } from "@ember/helper"; -import { action } from "@ember/object"; -import FieldInputDescription from "admin/components/schema-theme-setting/field-input-description"; -import TagChooser from "select-kit/components/tag-chooser"; - -export default class SchemaThemeSettingTypeTag extends Component { - @tracked value = this.args.value; - - @action - onInput(newVal) { - this.value = newVal; - this.args.onChange(newVal); - } - - -} diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/tags.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/tags.gjs new file mode 100644 index 00000000000..5ccc04b009d --- /dev/null +++ b/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, + }); + } + } + + +} diff --git a/app/assets/javascripts/discourse/tests/fixtures/theme-setting-schema-data.js b/app/assets/javascripts/discourse/tests/fixtures/theme-setting-schema-data.js index 77dc0d0025a..de743c9620d 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/theme-setting-schema-data.js +++ b/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", } }, }; diff --git a/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs index 2a694fbd25f..9ae8eee39ca 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs +++ b/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(); 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(), "gazelle,cat"); + + await tagSelector.expand(); + await tagSelector.selectRowByIndex(2); + await tagSelector.collapse(); + + 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(); + assert.strictEqual(tagSelector.header().value(), null); + inputFields.refresh(); + + 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); - await tagSelector.selectRowByIndex(3); - assert.strictEqual(tagSelector.header().value(), "gazelle,cat"); + assert.strictEqual(tagSelector.header().value(), "gazelle"); - const tree = new TreeFromDOM(); - await click(tree.nodes[1].element); - assert.strictEqual(tagSelector.header().value(), null); + inputFields.refresh(); - tree.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, + }) + ); }); test("input fields of type group", async function (assert) { diff --git a/app/assets/stylesheets/common/admin/schema_field.scss b/app/assets/stylesheets/common/admin/schema_field.scss index 1660c2eba24..e9b37171d43 100644 --- a/app/assets/stylesheets/common/admin/schema_field.scss +++ b/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 { diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 92a04329ff6..71134fa16e2 100644 --- a/config/locales/client.en.yml +++ b/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: diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 9a8b707c83b..5b9f189500f 100644 --- a/config/locales/server.en.yml +++ b/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." diff --git a/lib/theme_settings_object_validator.rb b/lib/theme_settings_object_validator.rb index 039120088c2..a57d5ce8451 100644 --- a/lib/theme_settings_object_validator.rb +++ b/lib/theme_settings_object_validator.rb @@ -118,7 +118,7 @@ class ThemeSettingsObjectValidator 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 @@ class ThemeSettingsObjectValidator [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 @@ class ThemeSettingsObjectValidator 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 @@ class ThemeSettingsObjectValidator "upload" => { klass: Upload, }, - "tag" => { + "tags" => { klass: Tag, column: :name, }, @@ -247,7 +264,7 @@ class ThemeSettingsObjectValidator 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( diff --git a/spec/lib/theme_settings_object_validator_spec.rb b/spec/lib/theme_settings_object_validator_spec.rb index 368de4012c3..d1459ac8dbd 100644 --- a/spec/lib/theme_settings_object_validator_spec.rb +++ b/spec/lib/theme_settings_object_validator_spec.rb @@ -708,53 +708,107 @@ RSpec.describe ThemeSettingsObjectValidator do end context "for tag properties" do - it "should not return any error message when the value of the property is a valid name of a tag record" do - tag = Fabricate(:tag) + fab!(:tag_1) { Fabricate(:tag) } + fab!(:tag_2) { Fabricate(:tag) } + fab!(:tag_3) { Fabricate(:tag) } - schema = { name: "section", properties: { tag_property: { type: "tag" } } } + it "should not return any error message when the value of the property is an array of valid tag names" do + schema = { name: "section", properties: { tags_property: { type: "tags" } } } expect( - described_class.new(schema: schema, object: { tag_property: tag.name }).validate, + described_class.new( + schema: schema, + object: { + tags_property: [tag_1.name, tag_2.name], + }, + ).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: { tag_property: { type: "tag" } } } + schema = { name: "section", properties: { tags_property: { type: "tags" } } } expect(described_class.new(schema: schema, object: {}).validate).to eq({}) end it "should return the right hash of error messages when value of property is not present and it's required" do - schema = { name: "section", properties: { tag_property: { type: "tag", required: true } } } - errors = described_class.new(schema: schema, object: {}).validate - - expect(errors.keys).to eq(["/tag_property"]) - expect(errors["/tag_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 a valid tag name" do - schema = { name: "section", properties: { tag_property: { type: "tag" } } } - - errors = described_class.new(schema: schema, object: { tag_property: "string" }).validate - - expect(errors.keys).to eq(["/tag_property"]) - - expect(errors["/tag_property"].full_messages).to contain_exactly("must be a valid tag name") - end - - it "should return the right hash of error messages when value of property is not a valid name of a tag record" do schema = { name: "section", properties: { - tag_property: { - type: "tag", + tags_property: { + type: "tags", + required: true, + }, + }, + } + errors = described_class.new(schema: schema, object: {}).validate + + expect(errors.keys).to eq(["/tags_property"]) + expect(errors["/tags_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 array of tag names" do + schema = { name: "section", properties: { tags_property: { type: "tags" } } } + + errors = described_class.new(schema: schema, object: { tags_property: "string" }).validate + + expect(errors.keys).to eq(["/tags_property"]) + + expect(errors["/tags_property"].full_messages).to contain_exactly( + "must be an array of valid tag names", + ) + end + + it "should return the right hash of error messages when number of tag names does not satisfy min or max validations" do + schema = { + name: "section", + properties: { + tags_property: { + type: "tags", + validations: { + min: 1, + max: 2, + }, + }, + }, + } + + errors = described_class.new(schema: schema, object: { tags_property: [] }).validate + + expect(errors.keys).to eq(["/tags_property"]) + + expect(errors["/tags_property"].full_messages).to contain_exactly( + "must have at least 1 tag names", + ) + + errors = + described_class.new( + schema: schema, + object: { + tags_property: [tag_1.name, tag_2.name, tag_3.name], + }, + ).validate + + expect(errors.keys).to eq(["/tags_property"]) + + expect(errors["/tags_property"].full_messages).to contain_exactly( + "must have at most 2 tag names", + ) + end + + it "should return the right hash of error messages when value of property contain tag names which are invalid" do + schema = { + name: "section", + properties: { + tags_property: { + type: "tags", }, child_tags: { type: "objects", schema: { name: "child_tag", properties: { - tag_property_2: { - type: "tag", + tags_property_2: { + type: "tags", }, }, }, @@ -762,25 +816,27 @@ RSpec.describe ThemeSettingsObjectValidator do }, } + tag_1 + queries = track_sql_queries do errors = described_class.new( schema:, object: { - tag_property: "some random tag name", - child_tags: [{ tag_property_2: "some random tag name" }], + tags_property: ["some random tag name", tag_1.name], + child_tags: [{ tags_property_2: ["some random tag name", tag_1.name, "abcdef"] }], }, ).validate - expect(errors.keys).to eq(%w[/tag_property /child_tags/0/tag_property_2]) + expect(errors.keys).to eq(%w[/tags_property /child_tags/0/tags_property_2]) - expect(errors["/tag_property"].full_messages).to contain_exactly( - "must be a valid tag name", + expect(errors["/tags_property"].full_messages).to contain_exactly( + "must be an array of valid tag names", ) - expect(errors["/child_tags/0/tag_property_2"].full_messages).to contain_exactly( - "must be a valid tag name", + expect(errors["/child_tags/0/tags_property_2"].full_messages).to contain_exactly( + "must be an array of valid tag names", ) end