From 476d91d233db687d7287cf47f855eba5e6a557c1 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 27 Mar 2024 10:54:30 +0800 Subject: [PATCH] DEV: Change category type to categories type for theme object schema (#26339) Why this change? This is a follow-up to 86b2e3aa3e8be30a308f1bff3664d76c5d56057a. Basically, we want to allow people to select more than 1 category as well. What does this change do? 1. Change `type: category` to `type: categories` and support `min` and `max` validations for `type: categories`. 2. Fix the `` component to support the `min` and `max` validations and switch it to use the `` component instead of the `` component which only supports selecting one category. --- .../schema-theme-setting/editor.gjs | 1 + .../components/schema-theme-setting/field.gjs | 7 +- .../schema-theme-setting/types/categories.gjs | 64 +++++++++++++ .../schema-theme-setting/types/category.gjs | 34 ------- .../fixtures/theme-setting-schema-data.js | 2 +- .../tests/helpers/select-kit-helper.js | 4 + .../editor-test.gjs | 52 ++++++++-- ...eme_objects_setting_metadata_serializer.rb | 11 ++- config/locales/client.en.yml | 4 + config/locales/server.en.yml | 22 ++++- lib/theme_settings_manager/objects.rb | 17 ++++ lib/theme_settings_object_validator.rb | 18 ++-- .../theme_settings/objects_settings.yaml | 18 +++- .../theme_settings_manager/objects_spec.rb | 41 +++++++- .../theme_settings_object_validator_spec.rb | 95 ++++++++++++------- spec/requests/admin/themes_controller_spec.rb | 29 ++++++ ...bjects_setting_metadata_serializer_spec.rb | 54 +++++++++++ .../theme_settings_serializer_spec.rb | 10 +- ...diting_objects_typed_theme_setting_spec.rb | 4 + 19 files changed, 387 insertions(+), 100 deletions(-) create mode 100644 app/assets/javascripts/admin/addon/components/schema-theme-setting/types/categories.gjs delete mode 100644 app/assets/javascripts/admin/addon/components/schema-theme-setting/types/category.gjs diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs index 0fc7f6596fa..27903be06ff 100644 --- a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs +++ b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs @@ -377,6 +377,7 @@ export default class SchemaThemeSettingEditor extends Component { @spec={{field.spec}} @onValueChange={{fn this.inputFieldChanged field}} @description={{field.description}} + @setting={{@setting}} /> {{/each}} {{#if (gt this.fields.length 0)}} 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 a8c7715e29a..127178cef00 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 @@ -2,7 +2,7 @@ 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 CategoriesField from "./types/categories"; import EnumField from "./types/enum"; import FloatField from "./types/float"; import GroupField from "./types/group"; @@ -25,8 +25,8 @@ export default class SchemaThemeSettingField extends Component { return BooleanField; case "enum": return EnumField; - case "category": - return CategoryField; + case "categories": + return CategoriesField; case "tags": return TagsField; case "group": @@ -58,6 +58,7 @@ export default class SchemaThemeSettingField extends Component { @spec={{@spec}} @onChange={{@onValueChange}} @description={{this.description}} + @setting={{@setting}} /> diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/categories.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/categories.gjs new file mode 100644 index 00000000000..170d8ea2239 --- /dev/null +++ b/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/categories.gjs @@ -0,0 +1,64 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { hash } from "@ember/helper"; +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 CategorySelector from "select-kit/components/category-selector"; + +export default class SchemaThemeSettingTypeCategories extends Component { + @tracked touched = false; + + @tracked + value = + this.args.value?.map((categoryId) => { + return this.args.setting.metadata.categories[categoryId]; + }) || []; + + required = this.args.spec.required; + min = this.args.spec.validations?.min; + max = this.args.spec.validations?.max; + + @action + onInput(categories) { + this.touched = true; + this.value = categories; + this.args.onChange(categories.map((category) => category.id)); + } + + 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.categories.at_least", { + count: this.min || 1, + }); + } + } + + +} diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/category.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/category.gjs deleted file mode 100644 index 58ca7b6afa2..00000000000 --- a/app/assets/javascripts/admin/addon/components/schema-theme-setting/types/category.gjs +++ /dev/null @@ -1,34 +0,0 @@ -import Component from "@glimmer/component"; -import { tracked } from "@glimmer/tracking"; -import { action } from "@ember/object"; -import FieldInputDescription from "admin/components/schema-theme-setting/field-input-description"; -import CategoryChooser from "select-kit/components/category-chooser"; - -export default class SchemaThemeSettingTypeCategory extends Component { - @tracked value = this.args.value; - required = this.args.spec.required; - - @action - onInput(newVal) { - this.value = newVal; - this.args.onChange(newVal); - } - - get categoryChooserOptions() { - return { - allowUncategorized: false, - none: !this.required, - clearable: !this.required, - }; - } - - -} 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 de743c9620d..1f8f41559c3 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 @@ -184,7 +184,7 @@ export default function schemaAndData(version = 1) { choices: ["nice", "awesome", "cool"] }, category_field: { - type: "category", + type: "categories", }, group_field: { type: "group", diff --git a/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js b/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js index d6438a6db2c..5e08b918de9 100644 --- a/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js +++ b/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js @@ -229,6 +229,10 @@ export default function selectKit(selector) { return filterHelper(query(selector).querySelector(".select-kit-filter")); }, + error() { + return query(selector).querySelector(".select-kit-error"); + }, + rows() { return query(selector).querySelectorAll(".select-kit-row"); }, 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 2dfca2521dc..12808e534e3 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 @@ -667,7 +667,7 @@ module( assert.strictEqual(enumSelector.header().value(), "nice"); }); - test("input fields of type category", async function (assert) { + test("input fields of type categories", async function (assert) { const setting = ThemeSettings.create({ setting: "objects_setting", objects_schema: { @@ -675,17 +675,29 @@ module( identifier: "id", properties: { required_category: { - type: "category", + type: "categories", required: true, }, not_required_category: { - type: "category", + type: "categories", + validations: { + min: 2, + max: 3, + }, + }, + }, + }, + metadata: { + categories: { + 6: { + id: 6, + name: "some category", }, }, }, value: [ { - required_category: 6, + required_category: [6], }, ], }); @@ -706,9 +718,17 @@ module( assert.strictEqual(categorySelector.header().value(), "6"); - assert - .dom(categorySelector.clearButton()) - .doesNotExist("is not clearable"); + await categorySelector.expand(); + await categorySelector.deselectItemByValue("6"); + await categorySelector.collapse(); + + inputFields.refresh(); + + assert.dom(inputFields.fields.required_category.errorElement).hasText( + I18n.t("admin.customize.theme.schema.fields.categories.at_least", { + count: 1, + }) + ); assert .dom(inputFields.fields.not_required_category.labelElement) @@ -722,8 +742,24 @@ module( await categorySelector.expand(); await categorySelector.selectRowByIndex(1); + await categorySelector.collapse(); - assert.dom(categorySelector.clearButton()).exists("is clearable"); + inputFields.refresh(); + + assert.dom(inputFields.fields.not_required_category.errorElement).hasText( + I18n.t("admin.customize.theme.schema.fields.categories.at_least", { + count: 2, + }) + ); + + await categorySelector.expand(); + await categorySelector.selectRowByIndex(2); + await categorySelector.selectRowByIndex(3); + await categorySelector.selectRowByIndex(4); + + assert + .dom(categorySelector.error()) + .hasText("You can only select 3 items."); }); test("input fields of type tags which is required", async function (assert) { diff --git a/app/serializers/theme_objects_setting_metadata_serializer.rb b/app/serializers/theme_objects_setting_metadata_serializer.rb index 0767903b55f..8a3d5b2b98c 100644 --- a/app/serializers/theme_objects_setting_metadata_serializer.rb +++ b/app/serializers/theme_objects_setting_metadata_serializer.rb @@ -1,7 +1,16 @@ # frozen_string_literal: true class ThemeObjectsSettingMetadataSerializer < ApplicationSerializer - attributes :property_descriptions + attributes :categories, :property_descriptions + + def categories + object + .categories(scope) + .reduce({}) do |acc, category| + acc[category.id] = BasicCategorySerializer.new(category, scope: scope, root: false).as_json + acc + end + end def property_descriptions locales = {} diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 8fd60ff999a..859cabbd199 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5652,6 +5652,10 @@ en: back_button: "Back to %{name}" fields: required: "*required" + categories: + at_least: + one: "at least %{count} category is required" + other: "at least %{count} categories are required" tags: at_least: one: "at least %{count} tag is required" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ed30a71c131..03beae8f3a0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -141,40 +141,58 @@ en: required: "must be present" humanize_invalid_type: "The property at JSON Pointer '%{property_json_pointer}' must be of type %{type}." invalid_type: "%{type} is not a valid type" + humanize_not_valid_string_value: "The property at JSON Pointer '%{property_json_pointer}' must be a string." not_valid_string_value: "must be a string" + humanize_not_valid_integer_value: "The property at JSON Pointer '%{property_json_pointer}' must be an integer." not_valid_integer_value: "must be an integer" + humanize_not_valid_float_value: "The property at JSON Pointer '%{property_json_pointer}' must be a float." not_valid_float_value: "must be a float" + humanize_not_valid_boolean_value: "The property at JSON Pointer '%{property_json_pointer}' must be a boolean." not_valid_boolean_value: "must be a boolean" + humanize_not_valid_enum_value: "The property at JSON Pointer '%{property_json_pointer}' must be one of the following %{choices}." not_valid_enum_value: "must be one of the following: %{choices}" - humanize_not_valid_category_value: "The property at JSON Pointer '%{property_json_pointer}' must be a valid category id." - not_valid_category_value: "must be a valid category id" + + humanize_not_valid_categories_value: "The property at JSON Pointer '%{property_json_pointer}' must be an array of valid category ids." + not_valid_categories_value: "must be an array of valid category ids" + humanize_categories_value_not_valid_min: "The property at JSON Pointer '%{property_json_pointer}' must have at least %{min} category ids." + categories_value_not_valid_min: "must have at least %{min} category ids" + humanize_categories_value_not_valid_max: "The property at JSON Pointer '%{property_json_pointer}' must have at most %{max} category ids." + categories_value_not_valid_max: "must have at most %{max} category ids" + humanize_not_valid_topic_value: "The property at JSON Pointer '%{property_json_pointer}' must be a valid topic id." not_valid_topic_value: "must be a valid topic id" + 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." not_valid_group_value: "must be a valid group id" + 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." string_value_not_valid_min: "must be at least %{min} characters long" humanize_string_value_not_valid_max: "The property at JSON Pointer '%{property_json_pointer}' must be at most %{max} characters long." string_value_not_valid_max: "must be at most %{max} characters long" + humanize_number_value_not_valid_min: "The property at JSON Pointer '%{property_json_pointer}' must be larger than or equal to %{min}." number_value_not_valid_min: "must be larger than or equal to %{min}" humanize_number_value_not_valid_max: "The property at JSON Pointer '%{property_json_pointer}' must be smaller than or equal to %{max}." number_value_not_valid_max: "must be smaller than or equal to %{max}" + humanize_string_value_not_valid_url: "The property at JSON Pointer '%{property_json_pointer}' must be a valid URL." string_value_not_valid_url: "must be a valid URL" locale_errors: diff --git a/lib/theme_settings_manager/objects.rb b/lib/theme_settings_manager/objects.rb index 71b9a9d0643..70a15d7260c 100644 --- a/lib/theme_settings_manager/objects.rb +++ b/lib/theme_settings_manager/objects.rb @@ -16,4 +16,21 @@ class ThemeSettingsManager::Objects < ThemeSettingsManager def schema @opts[:schema] end + + def categories(guardian) + category_ids = Set.new + + value.each do |theme_setting_object| + category_ids.merge( + ThemeSettingsObjectValidator.new( + schema:, + object: theme_setting_object, + ).property_values_of_type("categories"), + ) + end + + return [] if category_ids.empty? + + Category.secured(guardian).where(id: category_ids) + end end diff --git a/lib/theme_settings_object_validator.rb b/lib/theme_settings_object_validator.rb index a57d5ce8451..ad8c36d5c97 100644 --- a/lib/theme_settings_object_validator.rb +++ b/lib/theme_settings_object_validator.rb @@ -85,6 +85,10 @@ class ThemeSettingsObjectValidator @errors end + def property_values_of_type(type) + fetch_property_values_of_type(@properties, @object, type) + end + private def validate_child_objects(objects, property_name:, schema:) @@ -120,7 +124,7 @@ class ThemeSettingsObjectValidator case type when "string" value.is_a?(String) - when "integer", "category", "topic", "post", "group", "upload" + when "integer", "topic", "post", "group", "upload" value.is_a?(Integer) when "float" value.is_a?(Float) || value.is_a?(Integer) @@ -128,6 +132,8 @@ class ThemeSettingsObjectValidator [true, false].include?(value) when "enum" property_attributes[:choices].include?(value) + when "categories" + value.is_a?(Array) && value.all? { |id| id.is_a?(Integer) } when "tags" value.is_a?(Array) && value.all? { |tag| tag.is_a?(String) } else @@ -151,24 +157,24 @@ class ThemeSettingsObjectValidator return true if value.nil? case type - when "topic", "category", "upload", "post", "group" + when "topic", "upload", "post", "group" if !valid_ids(type).include?(value) add_error(property_name, :"not_valid_#{type}_value") return false end - when "tags" + when "tags", "categories" 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:) + add_error(property_name, :"#{type}_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:) + add_error(property_name, :"#{type}_value_not_valid_max", max:) return false end when "string" @@ -225,7 +231,7 @@ class ThemeSettingsObjectValidator end TYPE_TO_MODEL_MAP = { - "category" => { + "categories" => { klass: Category, }, "topic" => { diff --git a/spec/fixtures/theme_settings/objects_settings.yaml b/spec/fixtures/theme_settings/objects_settings.yaml index 1f8bdfeda0b..6c81f9bedbd 100644 --- a/spec/fixtures/theme_settings/objects_settings.yaml +++ b/spec/fixtures/theme_settings/objects_settings.yaml @@ -32,4 +32,20 @@ objects_setting: validations: max_length: 20 url: - type: string \ No newline at end of file + type: string + +objects_with_categories: + type: objects + default: [] + schema: + name: categories + properties: + category_ids: + type: categories + child_categories: + type: objects + schema: + name: child category + properties: + category_ids: + type: categories diff --git a/spec/lib/theme_settings_manager/objects_spec.rb b/spec/lib/theme_settings_manager/objects_spec.rb index 028bcba6fa7..084e3e52142 100644 --- a/spec/lib/theme_settings_manager/objects_spec.rb +++ b/spec/lib/theme_settings_manager/objects_spec.rb @@ -3,11 +3,11 @@ RSpec.describe ThemeSettingsManager::Objects do fab!(:theme) - let(:objects_setting) do + 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[:objects_setting] + theme.settings end before { SiteSetting.experimental_objects_type_for_theme_settings = true } @@ -27,7 +27,7 @@ RSpec.describe ThemeSettingsManager::Objects do }, ] - objects_setting.value = new_value + theme_setting[:objects_setting].value = new_value expect(theme.reload.settings[:objects_setting].value).to eq(new_value) end @@ -45,9 +45,42 @@ RSpec.describe ThemeSettingsManager::Objects do }, ] - expect { objects_setting.value = new_value }.to raise_error( + expect { theme_setting[:objects_setting].value = new_value }.to raise_error( Discourse::InvalidParameters, "The property at JSON Pointer '/0/links/0/name' must be present. The property at JSON Pointer '/1/name' must be present. The property at JSON Pointer '/1/links/0/name' must be at most 20 characters long.", ) end + + describe "#categories" do + fab!(:category_1) { Fabricate(:category) } + fab!(:category_2) { Fabricate(:category) } + fab!(:category_3) { Fabricate(:private_category, group: Fabricate(:group)) } + fab!(:admin) + + it "returns an empty array when there are no properties of `categories` type" do + expect(theme_setting[:objects_setting].categories(Guardian.new)).to eq([]) + end + + it "returns the categories record for all the properties of `categories` type in a flat array" do + new_value = [ + { + "category_ids" => [category_1.id, category_2.id], + "child_categories" => [{ "category_ids" => [category_3.id] }], + }, + ] + + theme_setting[:objects_with_categories].value = new_value + + expect(theme.reload.settings[:objects_with_categories].value).to eq(new_value) + + expect(theme.settings[:objects_with_categories].categories(Guardian.new)).to contain_exactly( + category_1, + category_2, + ) + + expect( + theme.settings[:objects_with_categories].categories(Guardian.new(admin)), + ).to contain_exactly(category_1, category_2, category_3) + end + end end diff --git a/spec/lib/theme_settings_object_validator_spec.rb b/spec/lib/theme_settings_object_validator_spec.rb index d1459ac8dbd..0148aead697 100644 --- a/spec/lib/theme_settings_object_validator_spec.rb +++ b/spec/lib/theme_settings_object_validator_spec.rb @@ -15,7 +15,7 @@ RSpec.describe ThemeSettingsObjectValidator do }, }, category_property: { - type: "category", + type: "categories", required: true, }, links: { @@ -49,10 +49,10 @@ RSpec.describe ThemeSettingsObjectValidator do objects: [ { title: "1234", - category_property: category.id, + category_property: [category.id], links: [{ position: 1, float: 4.5 }, { position: "string", float: 12 }], }, - { title: "12345678910", category_property: 99_999_999, links: [{ float: 5 }] }, + { title: "12345678910", category_property: [99_999_999], links: [{ float: 5 }] }, ], ) @@ -63,7 +63,7 @@ RSpec.describe ThemeSettingsObjectValidator do "The property at JSON Pointer '/0/links/1/position' must be an integer.", "The property at JSON Pointer '/0/links/1/float' must be smaller than or equal to 11.5.", "The property at JSON Pointer '/1/title' must be at most 10 characters long.", - "The property at JSON Pointer '/1/category_property' must be a valid category id.", + "The property at JSON Pointer '/1/category_property' must be an array of valid category ids.", "The property at JSON Pointer '/1/links/0/position' must be present.", "The property at JSON Pointer '/1/links/0/float' must be larger than or equal to 5.5.", ], @@ -1028,18 +1028,24 @@ RSpec.describe ThemeSettingsObjectValidator do end context "for category properties" do - it "should not return any error message when the value of the property is a valid id of a category record" do - category = Fabricate(:category) + fab!(:category_1) { Fabricate(:category) } + fab!(:category_2) { Fabricate(:category) } - schema = { name: "section", properties: { category_property: { type: "category" } } } + it "should not return any error message when the value of the property is an array of valid category ids" do + schema = { name: "section", properties: { category_property: { type: "categories" } } } expect( - described_class.new(schema: schema, object: { category_property: category.id }).validate, + described_class.new( + schema: schema, + object: { + category_property: [category_1.id, category_2.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: { category_property: { type: "category" } } } + schema = { name: "section", properties: { category_property: { type: "categories" } } } expect(described_class.new(schema: schema, object: {}).validate).to eq({}) end @@ -1048,7 +1054,7 @@ RSpec.describe ThemeSettingsObjectValidator do name: "section", properties: { category_property: { - type: "category", + type: "categories", required: true, }, }, @@ -1059,30 +1065,51 @@ RSpec.describe ThemeSettingsObjectValidator do expect(errors["/category_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 - schema = { name: "section", properties: { category_property: { type: "category" } } } + it "should return the right hash of error messages when value of property contains an array where not all values are integers" do + schema = { name: "section", properties: { category_property: { type: "categories" } } } errors = - described_class.new(schema: schema, object: { category_property: "string" }).validate + described_class.new(schema: schema, object: { category_property: ["string"] }).validate expect(errors.keys).to eq(["/category_property"]) expect(errors["/category_property"].full_messages).to contain_exactly( - "must be a valid category id", + "must be an array of valid category ids", ) end - it "should return the right hash of error messages when value of property is not a valid id of a category record" do - category = Fabricate(:category) - + it "should return the right hash of error messages when number of category ids does not satisfy min or max validations" do schema = { name: "section", properties: { category_property: { - type: "category", + type: "categories", + validations: { + min: 1, + max: 2, + }, + }, + }, + } + + errors = described_class.new(schema: schema, object: { category_property: [] }).validate + + expect(errors.keys).to eq(["/category_property"]) + + expect(errors["/category_property"].full_messages).to contain_exactly( + "must have at least 1 category ids", + ) + end + + it "should return the right hash of error messages when value of property is not an array of valid category ids" do + schema = { + name: "section", + properties: { + category_property: { + type: "categories", }, category_property_2: { - type: "category", + type: "categories", }, child_categories: { type: "objects", @@ -1090,7 +1117,7 @@ RSpec.describe ThemeSettingsObjectValidator do name: "child_category", properties: { category_property_3: { - type: "category", + type: "categories", }, }, }, @@ -1098,36 +1125,34 @@ RSpec.describe ThemeSettingsObjectValidator do }, } + object = { + category_property: [99_999_999, category_1.id], + category_property_2: [99_999_999], + child_categories: [ + { category_property_3: [99_999_999, category_2.id] }, + { category_property_3: [category_2.id] }, + ], + } + queries = track_sql_queries do - errors = - described_class.new( - schema: schema, - object: { - category_property: 99_999_999, - category_property_2: 99_999_999, - child_categories: [ - { category_property_3: 99_999_999 }, - { category_property_3: category.id }, - ], - }, - ).validate + errors = described_class.new(schema:, object:).validate expect(errors.keys).to eq( %w[/category_property /category_property_2 /child_categories/0/category_property_3], ) expect(errors["/category_property"].full_messages).to contain_exactly( - "must be a valid category id", + "must be an array of valid category ids", ) expect(errors["/category_property_2"].full_messages).to contain_exactly( - "must be a valid category id", + "must be an array of valid category ids", ) expect( errors["/child_categories/0/category_property_3"].full_messages, - ).to contain_exactly("must be a valid category id") + ).to contain_exactly("must be an array of valid category ids") end # only 1 SQL query should be executed to check if category ids are valid diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index 31ca2a3fcc8..f3707f46844 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -1390,6 +1390,35 @@ RSpec.describe Admin::ThemesController do }, ) end + + it "returns 200 with the right `categories` attribute for a theme setting with categories propertoes" do + category_1 = Fabricate(:category) + category_2 = Fabricate(:category) + category_3 = Fabricate(:category) + + theme_setting[:objects_with_categories].value = [ + { + "category_ids" => [category_1.id, category_2.id], + "child_categories" => [{ "category_ids" => [category_3.id] }], + }, + ] + + get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_with_categories.json" + + expect(response.status).to eq(200) + + categories = response.parsed_body["categories"] + + expect(categories.keys.map(&:to_i)).to contain_exactly( + category_1.id, + category_2.id, + category_3.id, + ) + + expect(categories[category_1.id.to_s]["name"]).to eq(category_1.name) + expect(categories[category_2.id.to_s]["name"]).to eq(category_2.name) + expect(categories[category_3.id.to_s]["name"]).to eq(category_3.name) + end end end end diff --git a/spec/serializers/theme_objects_setting_metadata_serializer_spec.rb b/spec/serializers/theme_objects_setting_metadata_serializer_spec.rb index 3df6fe4eaaf..51969b75ea3 100644 --- a/spec/serializers/theme_objects_setting_metadata_serializer_spec.rb +++ b/spec/serializers/theme_objects_setting_metadata_serializer_spec.rb @@ -37,4 +37,58 @@ RSpec.describe ThemeObjectsSettingMetadataSerializer do ) end end + + describe "#categories" do + fab!(:category_1) { Fabricate(:category) } + fab!(:category_2) { Fabricate(:category) } + fab!(:category_3) { Fabricate(:private_category, group: Fabricate(:group)) } + fab!(:admin) + + it "should return a hash of serialized categories" do + theme_setting[:objects_with_categories].value = [ + { + "category_ids" => [category_1.id, category_2.id], + "child_categories" => [{ "category_ids" => [category_3.id] }], + }, + ] + + scope = Guardian.new + + payload = + described_class.new(theme_setting[:objects_with_categories], scope:, root: false).as_json + + categories = payload[:categories] + + expect(categories.keys).to contain_exactly(category_1.id, category_2.id) + + expect(categories[category_1.id]).to eq( + BasicCategorySerializer.new(category_1, scope:, root: false).as_json, + ) + + expect(categories[category_2.id]).to eq( + BasicCategorySerializer.new(category_2, scope:, root: false).as_json, + ) + + scope = Guardian.new(admin) + + payload = + described_class.new(theme_setting[:objects_with_categories], scope:, root: false).as_json + + categories = payload[:categories] + + expect(categories.keys).to contain_exactly(category_1.id, category_2.id, category_3.id) + + expect(categories[category_1.id]).to eq( + BasicCategorySerializer.new(category_1, scope:, root: false).as_json, + ) + + expect(categories[category_2.id]).to eq( + BasicCategorySerializer.new(category_2, scope:, root: false).as_json, + ) + + expect(categories[category_3.id]).to eq( + BasicCategorySerializer.new(category_3, scope:, root: false).as_json, + ) + end + end end diff --git a/spec/serializers/theme_settings_serializer_spec.rb b/spec/serializers/theme_settings_serializer_spec.rb index f210b803015..9e8d54d62d6 100644 --- a/spec/serializers/theme_settings_serializer_spec.rb +++ b/spec/serializers/theme_settings_serializer_spec.rb @@ -3,18 +3,18 @@ RSpec.describe ThemeSettingsSerializer do fab!(:theme) - let(:objects_setting) do + 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[:objects_setting] + theme.settings end - describe "#objects_schema" do - before { SiteSetting.experimental_objects_type_for_theme_settings = true } + before { SiteSetting.experimental_objects_type_for_theme_settings = true } + describe "#objects_schema" do it "should include the attribute when theme setting is typed objects" do - payload = ThemeSettingsSerializer.new(objects_setting).as_json + payload = ThemeSettingsSerializer.new(theme_setting[:objects_setting]).as_json expect(payload[:theme_settings][:objects_schema][:name]).to eq("section") end diff --git a/spec/system/admin_editing_objects_typed_theme_setting_spec.rb b/spec/system/admin_editing_objects_typed_theme_setting_spec.rb index f64e0906edb..02d4a3182fc 100644 --- a/spec/system/admin_editing_objects_typed_theme_setting_spec.rb +++ b/spec/system/admin_editing_objects_typed_theme_setting_spec.rb @@ -113,6 +113,10 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do ] } ] + }, + { + "setting": "objects_with_categories", + "value": [] } ] SETTING