FIX: Validate tags using Tag#name instead of Tag#id in ThemeSettingsObjectValidator (#26314)

Why this change?

Fortunately or unfortunately in Discourse core, we mainly use `Tag#name`
to look up tags and not its id. This assumption is built into the
frontend as well so we need to use the tag's name instead of the id
here.

config/locales/server.en.yml

@@ -159,8 +159,8 @@ 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 id."
-        not_valid_tag_value: "must be a valid tag 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_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,9 +118,9 @@ class ThemeSettingsObjectValidator
 
     is_value_valid =
       case type
-      when "string"
+      when "string", "tag"
         value.is_a?(String)
-      when "integer", "category", "topic", "post", "group", "upload", "tag"
+      when "integer", "category", "topic", "post", "group", "upload"
         value.is_a?(Integer)
       when "float"
         value.is_a?(Float) || value.is_a?(Integer)
@@ -208,21 +208,38 @@ class ThemeSettingsObjectValidator
   end
 
   TYPE_TO_MODEL_MAP = {
-    "category" => Category,
-    "topic" => Topic,
-    "post" => Post,
-    "group" => Group,
-    "upload" => Upload,
-    "tag" => Tag,
+    "category" => {
+      klass: Category,
+    },
+    "topic" => {
+      klass: Topic,
+    },
+    "post" => {
+      klass: Post,
+    },
+    "group" => {
+      klass: Group,
+    },
+    "upload" => {
+      klass: Upload,
+    },
+    "tag" => {
+      klass: Tag,
+      column: :name,
+    },
   }
   private_constant :TYPE_TO_MODEL_MAP
 
   def valid_ids(type)
-    valid_ids_lookup[type] ||= Set.new(
-      TYPE_TO_MODEL_MAP[type].where(
-        id: fetch_property_values_of_type(@properties, @object, type),
-      ).pluck(:id),
-    )
+    valid_ids_lookup[type] ||= begin
+      column = TYPE_TO_MODEL_MAP[type][:column] || :id
+
+      Set.new(
+        TYPE_TO_MODEL_MAP[type][:klass].where(
+          column => fetch_property_values_of_type(@properties, @object, type),
+        ).pluck(column),
+      )
+    end
   end
 
   def fetch_property_values_of_type(properties, object, type)

spec/lib/theme_settings_object_validator_spec.rb

@@ -708,13 +708,13 @@ 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 id of a tag record" 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)
 
         schema = { name: "section", properties: { tag_property: { type: "tag" } } }
 
         expect(
-          described_class.new(schema: schema, object: { tag_property: tag.id }).validate,
+          described_class.new(schema: schema, object: { tag_property: tag.name }).validate,
         ).to eq({})
       end
 
@@ -731,17 +731,17 @@ RSpec.describe ThemeSettingsObjectValidator do
         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 an integer" do
+      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 id")
+        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 id of a tag record" do
+      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: {
@@ -768,19 +768,19 @@ RSpec.describe ThemeSettingsObjectValidator do
               described_class.new(
                 schema:,
                 object: {
-                  tag_property: 99_999_999,
-                  child_tags: [{ tag_property_2: 99_999_999 }],
+                  tag_property: "some random tag name",
+                  child_tags: [{ tag_property_2: "some random tag name" }],
                 },
               ).validate
 
             expect(errors.keys).to eq(%w[/tag_property /child_tags/0/tag_property_2])
 
             expect(errors["/tag_property"].full_messages).to contain_exactly(
-              "must be a valid tag id",
+              "must be a valid tag name",
             )
 
             expect(errors["/child_tags/0/tag_property_2"].full_messages).to contain_exactly(
-              "must be a valid tag id",
+              "must be a valid tag name",
             )
           end