github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-22 11:23:25 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

DEV: Support category type in theme setting object schema (#25760)

Browse Source 
Why this change?

This change supports a property of `type: category` in the schema that
is declared for a theme setting object. Example:

```
sections:
  type: objects
  schema:
    name: section
    properties:
      category_property:
        type: category
```

The value of a property declared as `type: category` will have to be a
valid id of a row in the `categories` table.

What does this change do?

Adds a property value validation step for `type: category`. Care has
been taken to ensure that we do not spam the database with a ton of
requests if there are alot of category typed properties. This is done by
walking through the entire object and collecting all the values for
properties typed category. After which, a single database query is
executed to validate which values are valid.
This commit is contained in:
Alan Guo Xiang Tan 2024-02-21 08:11:15 +08:00 committed by GitHub
parent 3e331b1725
commit cac60a2c6b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 125 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
config/locales/server.en.yml
View File

@ -144,6 +144,7 @@ en:
not_valid_float_value: "must be a float"
not_valid_boolean_value: "must be a boolean"
not_valid_enum_value: "must be one of the following: %{choices}"
not_valid_category_value: "must be a valid category id"
string_value_not_valid_min: "must be at least %{min} characters long"
string_value_not_valid_max: "must be at most %{max} characters long"
number_value_not_valid_min: "must be larger than or equal to %{min}"

81
lib/theme_settings_object_validator.rb
View File

@ -1,11 +1,12 @@
# frozen_string_literal: true
class ThemeSettingsObjectValidator
def initialize(schema:, object:)
def initialize(schema:, object:, valid_category_ids: nil)
@object = object
@schema_name = schema[:name]
@properties = schema[:properties]
@errors = {}
@valid_category_ids = valid_category_ids
end
def validate
@ -17,7 +18,10 @@ class ThemeSettingsObjectValidator
@errors[property_name] ||= []
@errors[property_name].push(
self.class.new(schema: property_attributes[:schema], object: child_object).validate,
self
.class
.new(schema: property_attributes[:schema], object: child_object, valid_category_ids:)
.validate,
)
end
end
@ -30,6 +34,7 @@ class ThemeSettingsObjectValidator
def validate_properties
@properties.each do |property_name, property_attributes|
next if property_attributes[:type] == "objects"
next if property_attributes[:required] && !is_property_present?(property_name)
next if !has_valid_property_value_type?(property_attributes, property_name)
next if !has_valid_property_value?(property_attributes, property_name)
@ -41,13 +46,12 @@ class ThemeSettingsObjectValidator
type = property_attributes[:type]
return true if (value.nil? && type != "enum")
return true if type == "objects"
is_value_valid =
case type
when "string"
value.is_a?(String)
when "integer"
when "integer", "category"
value.is_a?(Integer)
when "float"
value.is_a?(Float) || value.is_a?(Integer)
@ -73,39 +77,35 @@ class ThemeSettingsObjectValidator
def has_valid_property_value?(property_attributes, property_name)
validations = property_attributes[:validations]
return true if validations.blank?
type = property_attributes[:type]
value = @object[property_name]
case type
when "category"
if !valid_category_ids.include?(value)
add_error(property_name, I18n.t("themes.settings_errors.objects.not_valid_category_value"))
return false
end
when "string"
if validations[:min_length] && value.length < validations[:min_length]
if (min = validations&.dig(:min_length)) && value.length < min
add_error(
property_name,
I18n.t(
"themes.settings_errors.objects.string_value_not_valid_min",
min: validations[:min_length],
),
I18n.t("themes.settings_errors.objects.string_value_not_valid_min", min:),
)
return false
end
if validations[:max_length] && value.length > validations[:max_length]
if (max = validations&.dig(:max_length)) && value.length > max
add_error(
property_name,
I18n.t(
"themes.settings_errors.objects.string_value_not_valid_max",
max: validations[:max_length],
),
I18n.t("themes.settings_errors.objects.string_value_not_valid_max", max: max),
)
return false
end
if validations[:url] && !value.match?(URI.regexp)
if validations&.dig(:url) && !value.match?(URI.regexp)
add_error(
property_name,
I18n.t("themes.settings_errors.objects.string_value_not_valid_url"),
@ -114,25 +114,19 @@ class ThemeSettingsObjectValidator
return false
end
when "integer", "float"
if validations[:min] && value < validations[:min]
if (min = validations&.dig(:min)) && value < min
add_error(
property_name,
I18n.t(
"themes.settings_errors.objects.number_value_not_valid_min",
min: validations[:min],
),
I18n.t("themes.settings_errors.objects.number_value_not_valid_min", min:),
)
return false
end
if validations[:max] && value > validations[:max]
if (max = validations&.dig(:max)) && value > max
add_error(
property_name,
I18n.t(
"themes.settings_errors.objects.number_value_not_valid_max",
max: validations[:max],
),
I18n.t("themes.settings_errors.objects.number_value_not_valid_max", max:),
)
return false
@ -155,4 +149,35 @@ class ThemeSettingsObjectValidator
@errors[property_name] ||= []
@errors[property_name] << error
end
def valid_category_ids
@valid_category_ids ||=
Set.new(
Category.where(id: fetch_property_values_of_type(@properties, @object, "category")).pluck(
:id,
),
)
end
def fetch_property_values_of_type(properties, object, type)
values = Set.new
properties.each do |property_name, property_attributes|
if property_attributes[:type] == type
values << object[property_name]
elsif property_attributes[:type] == "objects"
object[property_name]&.each do |child_object|
values.merge(
fetch_property_values_of_type(
property_attributes[:schema][:properties],
child_object,
type,
),
)
end
end
end
values
end
end

71
spec/lib/theme_settings_object_validator_spec.rb
View File

@ -303,5 +303,76 @@ RSpec.describe ThemeSettingsObjectValidator do
).to eq(string_property: ["must be at most 10 characters long"])
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)
schema = { name: "section", properties: { category_property: { type: "category" } } }
expect(
described_class.new(schema: schema, object: { category_property: category.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: { category_property: { type: "category" } } }
expect(
described_class.new(schema: schema, object: { category_property: "string" }).validate,
).to eq(category_property: ["must be a valid category id"])
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)
schema = {
name: "section",
properties: {
category_property: {
type: "category",
},
category_property_2: {
type: "category",
},
child_categories: {
type: "objects",
schema: {
name: "child_category",
properties: {
category_property_3: {
type: "category",
},
},
},
},
},
}
queries =
track_sql_queries do
expect(
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,
).to eq(
category_property: ["must be a valid category id"],
category_property_2: ["must be a valid category id"],
child_categories: [{ category_property_3: ["must be a valid category id"] }, {}],
)
end
# only 1 SQL query should be executed to check if category ids are valid
expect(queries.length).to eq(1)
end
end
end
end