DEV: Change category type to categories type for theme object schema (#26339)

Why this change?

This is a follow-up to 86b2e3aa3e.

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 `<SchemaThemeSetting::Types::Categories>` component to support the
   `min` and `max` validations and switch it to use the `<CategorySelector>` component
   instead of the `<CategoryChooser>` component which only supports selecting one category.

spec/fixtures/theme_settings/objects_settings.yaml

@@ -32,4 +32,20 @@ objects_setting:
               validations:
                 max_length: 20
             url:
-              type: string
+              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

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

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

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

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

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

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