From 52a4912475e2283364c2bb70cc4e9af25e091e6f Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 27 Feb 2024 14:27:10 +0800 Subject: [PATCH] DEV: Support topic, post, group, upload and tag type for theme objects setting (#25907) Why this change? Previously in cac60a2c6b6b60dfe9b34824298b23e56dbc85eb, I added support for `type: "category"` for a property in the theme objects schema. This commit extend the work previously to add support for types `topic`, `post`, `group`, `upload` and `tag`. --- config/locales/server.en.yml | 10 + lib/theme_settings_object_validator.rb | 41 +- .../theme_settings_object_validator_spec.rb | 351 ++++++++++++++++++ 3 files changed, 388 insertions(+), 14 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b5b74a7868b..a1681a32bed 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -153,6 +153,16 @@ en: 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_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_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_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." diff --git a/lib/theme_settings_object_validator.rb b/lib/theme_settings_object_validator.rb index e641ba7f3b7..ab90861c4b6 100644 --- a/lib/theme_settings_object_validator.rb +++ b/lib/theme_settings_object_validator.rb @@ -60,13 +60,13 @@ class ThemeSettingsObjectValidator end end - def initialize(schema:, object:, valid_category_ids: nil, json_pointer_prefix: "", errors: {}) + def initialize(schema:, object:, json_pointer_prefix: "", errors: {}, valid_ids_lookup: {}) @object = object @schema_name = schema[:name] @properties = schema[:properties] @errors = errors - @valid_category_ids = valid_category_ids @json_pointer_prefix = json_pointer_prefix + @valid_ids_lookup = valid_ids_lookup end def validate @@ -80,8 +80,8 @@ class ThemeSettingsObjectValidator .new( schema: property_attributes[:schema], object: child_object, - valid_category_ids:, json_pointer_prefix: "#{@json_pointer_prefix}#{property_name}/#{index}/", + valid_ids_lookup:, errors: @errors, ) .validate @@ -113,7 +113,7 @@ class ThemeSettingsObjectValidator case type when "string" value.is_a?(String) - when "integer", "category" + when "integer", "category", "topic", "post", "group", "upload", "tag" value.is_a?(Integer) when "float" value.is_a?(Float) || value.is_a?(Integer) @@ -140,9 +140,9 @@ class ThemeSettingsObjectValidator value = @object[property_name] case type - when "category" - if !valid_category_ids.include?(value) - add_error(property_name, :not_valid_category_value) + when "topic", "category", "upload", "post", "group", "tag" + if !valid_ids(type).include?(value) + add_error(property_name, :"not_valid_#{type}_value") return false end when "string" @@ -194,13 +194,26 @@ class ThemeSettingsObjectValidator "/#{@json_pointer_prefix}#{property_name}" end - def valid_category_ids - @valid_category_ids ||= - Set.new( - Category.where(id: fetch_property_values_of_type(@properties, @object, "category")).pluck( - :id, - ), - ) + def valid_ids_lookup + @valid_ids_lookup ||= {} + end + + TYPE_TO_MODEL_MAP = { + "category" => Category, + "topic" => Topic, + "post" => Post, + "group" => Group, + "upload" => Upload, + "tag" => Tag, + } + 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), + ) end def fetch_property_values_of_type(properties, object, type) diff --git a/spec/lib/theme_settings_object_validator_spec.rb b/spec/lib/theme_settings_object_validator_spec.rb index c0c4ae79606..1f39e099ad7 100644 --- a/spec/lib/theme_settings_object_validator_spec.rb +++ b/spec/lib/theme_settings_object_validator_spec.rb @@ -431,6 +431,357 @@ RSpec.describe ThemeSettingsObjectValidator do end end + context "for topic properties" do + it "should not return any error message when the value of the property is a valid id of a topic record" do + topic = Fabricate(:topic) + + schema = { name: "section", properties: { topic_property: { type: "topic" } } } + + expect( + described_class.new(schema: schema, object: { topic_property: topic.id }).validate, + ).to eq({}) + end + + it "should return the right hash of error messages when value of property is not an integer" do + schema = { name: "section", properties: { topic_property: { type: "topic" } } } + + errors = described_class.new(schema: schema, object: { topic_property: "string" }).validate + + expect(errors.keys).to eq(["/topic_property"]) + + expect(errors["/topic_property"].full_messages).to contain_exactly( + "must be a valid topic id", + ) + end + + it "should return the right hash of error messages when value of property is not a valid id of a topic record" do + schema = { + name: "section", + properties: { + topic_property: { + type: "topic", + }, + child_topics: { + type: "objects", + schema: { + name: "child_topic", + properties: { + topic_property_2: { + type: "topic", + }, + }, + }, + }, + }, + } + + queries = + track_sql_queries do + errors = + described_class.new( + schema:, + object: { + topic_property: 99_999_999, + child_topics: [{ topic_property_2: 99_999_999 }], + }, + ).validate + + expect(errors.keys).to eq(%w[/topic_property /child_topics/0/topic_property_2]) + + expect(errors["/topic_property"].full_messages).to contain_exactly( + "must be a valid topic id", + ) + + expect(errors["/child_topics/0/topic_property_2"].full_messages).to contain_exactly( + "must be a valid topic id", + ) + end + + # only 1 SQL query should be executed to check if topic ids are valid + expect(queries.length).to eq(1) + end + end + + context "for upload properties" do + it "should not return any error message when the value of the property is a valid id of a upload record" do + upload = Fabricate(:upload) + + schema = { name: "section", properties: { upload_property: { type: "upload" } } } + + expect( + described_class.new(schema: schema, object: { upload_property: upload.id }).validate, + ).to eq({}) + end + + it "should return the right hash of error messages when value of property is not an integer" do + schema = { name: "section", properties: { upload_property: { type: "upload" } } } + + errors = described_class.new(schema: schema, object: { upload_property: "string" }).validate + + expect(errors.keys).to eq(["/upload_property"]) + + expect(errors["/upload_property"].full_messages).to contain_exactly( + "must be a valid upload id", + ) + end + + it "should return the right hash of error messages when value of property is not a valid id of a upload record" do + schema = { + name: "section", + properties: { + upload_property: { + type: "upload", + }, + child_uploads: { + type: "objects", + schema: { + name: "child_upload", + properties: { + upload_property_2: { + type: "upload", + }, + }, + }, + }, + }, + } + + queries = + track_sql_queries do + errors = + described_class.new( + schema:, + object: { + upload_property: 99_999_999, + child_uploads: [{ upload_property_2: 99_999_999 }], + }, + ).validate + + expect(errors.keys).to eq(%w[/upload_property /child_uploads/0/upload_property_2]) + + expect(errors["/upload_property"].full_messages).to contain_exactly( + "must be a valid upload id", + ) + + expect(errors["/child_uploads/0/upload_property_2"].full_messages).to contain_exactly( + "must be a valid upload id", + ) + end + + # only 1 SQL query should be executed to check if upload ids are valid + expect(queries.length).to eq(1) + end + 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 + tag = Fabricate(:tag) + + schema = { name: "section", properties: { tag_property: { type: "tag" } } } + + expect( + described_class.new(schema: schema, object: { tag_property: tag.id }).validate, + ).to eq({}) + end + + it "should return the right hash of error messages when value of property is not an integer" 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") + end + + it "should return the right hash of error messages when value of property is not a valid id of a tag record" do + schema = { + name: "section", + properties: { + tag_property: { + type: "tag", + }, + child_tags: { + type: "objects", + schema: { + name: "child_tag", + properties: { + tag_property_2: { + type: "tag", + }, + }, + }, + }, + }, + } + + queries = + track_sql_queries do + errors = + described_class.new( + schema:, + object: { + tag_property: 99_999_999, + child_tags: [{ tag_property_2: 99_999_999 }], + }, + ).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", + ) + + expect(errors["/child_tags/0/tag_property_2"].full_messages).to contain_exactly( + "must be a valid tag id", + ) + end + + # only 1 SQL query should be executed to check if tag ids are valid + expect(queries.length).to eq(1) + end + end + + context "for group properties" do + it "should not return any error message when the value of the property is a valid id of a group record" do + group = Fabricate(:group) + + schema = { name: "section", properties: { group_property: { type: "group" } } } + + expect( + described_class.new(schema: schema, object: { group_property: group.id }).validate, + ).to eq({}) + end + + it "should return the right hash of error messages when value of property is not an integer" do + schema = { name: "section", properties: { group_property: { type: "group" } } } + + errors = described_class.new(schema: schema, object: { group_property: "string" }).validate + + expect(errors.keys).to eq(["/group_property"]) + + expect(errors["/group_property"].full_messages).to contain_exactly( + "must be a valid group id", + ) + end + + it "should return the right hash of error messages when value of property is not a valid id of a group record" do + schema = { + name: "section", + properties: { + group_property: { + type: "group", + }, + child_groups: { + type: "objects", + schema: { + name: "child_group", + properties: { + group_property_2: { + type: "group", + }, + }, + }, + }, + }, + } + + queries = + track_sql_queries do + errors = + described_class.new( + schema:, + object: { + group_property: 99_999_999, + child_groups: [{ group_property_2: 99_999_999 }], + }, + ).validate + + expect(errors.keys).to eq(%w[/group_property /child_groups/0/group_property_2]) + + expect(errors["/group_property"].full_messages).to contain_exactly( + "must be a valid group id", + ) + + expect(errors["/child_groups/0/group_property_2"].full_messages).to contain_exactly( + "must be a valid group id", + ) + end + + # only 1 SQL query should be executed to check if group ids are valid + expect(queries.length).to eq(1) + end + end + + context "for post properties" do + it "should not return any error message when the value of the property is a valid id of a post record" do + post = Fabricate(:post) + + schema = { name: "section", properties: { post_property: { type: "post" } } } + + expect( + described_class.new(schema: schema, object: { post_property: post.id }).validate, + ).to eq({}) + end + + it "should return the right hash of error messages when value of property is not an integer" do + schema = { name: "section", properties: { post_property: { type: "post" } } } + + errors = described_class.new(schema: schema, object: { post_property: "string" }).validate + + expect(errors.keys).to eq(["/post_property"]) + + expect(errors["/post_property"].full_messages).to contain_exactly("must be a valid post id") + end + + it "should return the right hash of error messages when value of property is not a valid id of a post record" do + schema = { + name: "section", + properties: { + post_property: { + type: "post", + }, + child_posts: { + type: "objects", + schema: { + name: "child_post", + properties: { + post_property_2: { + type: "post", + }, + }, + }, + }, + }, + } + + queries = + track_sql_queries do + errors = + described_class.new( + schema:, + object: { + post_property: 99_999_999, + child_posts: [{ post_property_2: 99_999_999 }], + }, + ).validate + + expect(errors.keys).to eq(%w[/post_property /child_posts/0/post_property_2]) + + expect(errors["/post_property"].full_messages).to contain_exactly( + "must be a valid post id", + ) + + expect(errors["/child_posts/0/post_property_2"].full_messages).to contain_exactly( + "must be a valid post id", + ) + end + + # only 1 SQL query should be executed to check if post ids are valid + expect(queries.length).to eq(1) + end + 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)