From a64f558f32d5e41dc70b9f17ea01c2bb74c0c94c Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Mon, 19 Feb 2024 13:19:35 +0800 Subject: [PATCH] DEV: Add property value validation to ThemeSettingsObjectValidator (#25718) Why this change? This change adds property value validation to `ThemeSettingsObjectValidator` for the following types: "string", "integer", "float", "boolean", "enum". Note that this class is not being used anywhere yet and is still in development. --- config/locales/server.en.yml | 6 + lib/theme_settings_object_validator.rb | 56 ++++++++- .../theme_settings_object_validator_spec.rb | 110 +++++++++++++++++- 3 files changed, 165 insertions(+), 7 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 62db818bb1d..0550183d2e7 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -136,6 +136,12 @@ en: string_value_not_valid_max: "It must be at most %{max} characters long." objects: required: "must be present" + invalid_type: "%{type} is not a valid type" + not_valid_string_value: "must be a string" + not_valid_integer_value: "must be an integer" + 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}" locale_errors: top_level_locale: "The top level key in a locale file must match the locale name" invalid_yaml: "Translation YAML invalid" diff --git a/lib/theme_settings_object_validator.rb b/lib/theme_settings_object_validator.rb index 5050682d074..9c795a8a73e 100644 --- a/lib/theme_settings_object_validator.rb +++ b/lib/theme_settings_object_validator.rb @@ -9,7 +9,7 @@ class ThemeSettingsObjectValidator end def validate - validate_required_properties + validate_properties @properties.each do |property_name, property_attributes| if property_attributes[:type] == "objects" @@ -28,12 +28,56 @@ class ThemeSettingsObjectValidator private - def validate_required_properties + def validate_properties @properties.each do |property_name, property_attributes| - if property_attributes[:required] && @object[property_name].nil? - @errors[property_name] ||= [] - @errors[property_name] << I18n.t("themes.settings_errors.objects.required") - end + next if property_attributes[:required] && validate_required_property(property_name) + validate_property_type(property_attributes, property_name) end end + + def validate_property_type(property_attributes, property_name) + value = @object[property_name] + type = property_attributes[:type] + + return if (value.nil? && type != "enum") + return if type == "objects" + + is_value_valid = + case type + when "string" + value.is_a?(String) + when "integer" + value.is_a?(Integer) + when "float" + value.is_a?(Float) || value.is_a?(Integer) + when "boolean" + [true, false].include?(value) + when "enum" + property_attributes[:choices].include?(value) + else + add_error(property_name, I18n.t("themes.settings_errors.objects.invalid_type", type:)) + return + end + + if !is_value_valid + add_error( + property_name, + I18n.t("themes.settings_errors.objects.not_valid_#{type}_value", property_attributes), + ) + end + end + + def validate_required_property(property_name) + if @object[property_name].nil? + add_error(property_name, I18n.t("themes.settings_errors.objects.required")) + true + else + false + end + end + + def add_error(property_name, error) + @errors[property_name] ||= [] + @errors[property_name] << error + end end diff --git a/spec/lib/theme_settings_object_validator_spec.rb b/spec/lib/theme_settings_object_validator_spec.rb index 49763f4f83b..7f42600dfb4 100644 --- a/spec/lib/theme_settings_object_validator_spec.rb +++ b/spec/lib/theme_settings_object_validator_spec.rb @@ -2,7 +2,7 @@ RSpec.describe ThemeSettingsObjectValidator do describe "#validate" do - it "should return the right array of error messages when properties are required but missing" do + it "should return the right hash of error messages when properties are required but missing" do schema = { name: "section", properties: { @@ -68,5 +68,113 @@ RSpec.describe ThemeSettingsObjectValidator do ], ) end + + context "for enum properties" do + let(:schema) do + { + name: "section", + properties: { + enum_property: { + type: "enum", + choices: ["choice 1", 2, false], + }, + }, + } + end + + it "should not return any error messages when the value of the property is in the enum" do + expect( + described_class.new(schema: schema, object: { enum_property: "choice 1" }).validate, + ).to eq({}) + end + + it "should return the right hash of error messages when value of property is not in the enum" do + expect( + described_class.new(schema: schema, object: { enum_property: "random_value" }).validate, + ).to eq(enum_property: ["must be one of the following: [\"choice 1\", 2, false]"]) + end + + it "should return the right hash of error messages when enum property is not present" do + expect(described_class.new(schema: schema, object: {}).validate).to eq( + enum_property: ["must be one of the following: [\"choice 1\", 2, false]"], + ) + end + end + + context "for boolean properties" do + let(:schema) { { name: "section", properties: { boolean_property: { type: "boolean" } } } } + + it "should not return any error messages when the value of the property is of type boolean" do + expect( + described_class.new(schema: schema, object: { boolean_property: true }).validate, + ).to eq({}) + + expect( + described_class.new(schema: schema, object: { boolean_property: false }).validate, + ).to eq({}) + end + + it "should return the right hash of error messages when value of property is not of type boolean" do + expect( + described_class.new(schema: schema, object: { boolean_property: "string" }).validate, + ).to eq(boolean_property: ["must be a boolean"]) + end + end + + context "for float properties" do + let(:schema) { { name: "section", properties: { float_property: { type: "float" } } } } + + it "should not return any error messages when the value of the property is of type integer or float" do + expect(described_class.new(schema: schema, object: { float_property: 1.5 }).validate).to eq( + {}, + ) + + expect(described_class.new(schema: schema, object: { float_property: 1 }).validate).to eq( + {}, + ) + end + + it "should return the right hash of error messages when value of property is not of type float" do + expect( + described_class.new(schema: schema, object: { float_property: "string" }).validate, + ).to eq(float_property: ["must be a float"]) + end + end + + context "for integer properties" do + let(:schema) { { name: "section", properties: { integer_property: { type: "integer" } } } } + + it "should not return any error messages when the value of the property is of type integer" do + expect(described_class.new(schema: schema, object: { integer_property: 1 }).validate).to eq( + {}, + ) + end + + it "should return the right hash of error messages when value of property is not of type integer" do + expect( + described_class.new(schema: schema, object: { integer_property: "string" }).validate, + ).to eq(integer_property: ["must be an integer"]) + + expect( + described_class.new(schema: schema, object: { integer_property: 1.0 }).validate, + ).to eq(integer_property: ["must be an integer"]) + end + end + + context "for string properties" do + let(:schema) { { name: "section", properties: { string_property: { type: "string" } } } } + + it "should not return any error messages when the value of the property is of type string" do + expect( + described_class.new(schema: schema, object: { string_property: "string" }).validate, + ).to eq({}) + end + + it "should return the right hash of error messages when value of property is not of type string" do + expect(described_class.new(schema: schema, object: { string_property: 1 }).validate).to eq( + string_property: ["must be a string"], + ) + end + end end end