From 1dee4990cd71d888cea495383d2b49fc5b989017 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 8 Dec 2020 23:25:56 +0900 Subject: [PATCH 1/2] `allow_nil` should work for casted value in `NumericalityValidator` It is a regression for 4cc438a1df75e4c230f19cafe9258dbab969cd27. `NumericalityValidator` basically takes the value before typecasting, but `allow_nil` should work for the typecasted value for the compatibility. Fixes #40750. --- .../lib/active_model/validations/numericality.rb | 6 ++++-- activemodel/lib/active_model/validator.rb | 11 +++++++---- .../cases/validations/numericality_validation_test.rb | 8 ++++++++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index a27ed97acac75..2df9b0ebe2e3c 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -109,7 +109,9 @@ def allow_only_integer?(record) end def read_attribute_for_validation(record, attr_name) - return super if record_attribute_changed_in_place?(record, attr_name) + value = super + + return value if record_attribute_changed_in_place?(record, attr_name) came_from_user = :"#{attr_name}_came_from_user?" @@ -126,7 +128,7 @@ def read_attribute_for_validation(record, attr_name) end end - raw_value || super + raw_value || value end def record_attribute_changed_in_place?(record, attr_name) diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb index 4a77c7fe5f002..5391252b14f54 100644 --- a/activemodel/lib/active_model/validator.rb +++ b/activemodel/lib/active_model/validator.rb @@ -147,9 +147,10 @@ def initialize(options) # override +validate_each+ with validation logic. def validate(record) attributes.each do |attribute| - value = read_attribute_for_validation(record, attribute) - next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank]) - validate_each(record, attribute, value) + catch(:allowed) do + value = read_attribute_for_validation(record, attribute) + validate_each(record, attribute, value) + end end end @@ -167,7 +168,9 @@ def check_validity! private def read_attribute_for_validation(record, attr_name) - record.read_attribute_for_validation(attr_name) + value = record.read_attribute_for_validation(attr_name) + throw(:allowed) if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank]) + value end end diff --git a/activerecord/test/cases/validations/numericality_validation_test.rb b/activerecord/test/cases/validations/numericality_validation_test.rb index ffb8fd17b2951..91cd6a0b0d1f9 100644 --- a/activerecord/test/cases/validations/numericality_validation_test.rb +++ b/activerecord/test/cases/validations/numericality_validation_test.rb @@ -110,4 +110,12 @@ def test_aliased_attribute assert_not_predicate subject, :valid? end + + def test_allow_nil_works_for_casted_value + model_class.validates_numericality_of(:bank_balance, greater_than: 0, allow_nil: true) + + subject = model_class.new(bank_balance: "") + + assert_predicate subject, :valid? + end end From 720a60e68cf1945b6f486faca322d3acfa59c652 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 9 Dec 2020 16:43:26 +0900 Subject: [PATCH 2/2] Split the options checks from `read_attribute_for_validation` --- .../lib/active_model/validations/numericality.rb | 4 +--- activemodel/lib/active_model/validator.rb | 12 +++++------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 2df9b0ebe2e3c..9c56417d89b3c 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -108,9 +108,7 @@ def allow_only_integer?(record) end end - def read_attribute_for_validation(record, attr_name) - value = super - + def read_attribute_for_validation(record, attr_name, value) return value if record_attribute_changed_in_place?(record, attr_name) came_from_user = :"#{attr_name}_came_from_user?" diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb index 5391252b14f54..9e2dee9a00f54 100644 --- a/activemodel/lib/active_model/validator.rb +++ b/activemodel/lib/active_model/validator.rb @@ -147,10 +147,10 @@ def initialize(options) # override +validate_each+ with validation logic. def validate(record) attributes.each do |attribute| - catch(:allowed) do - value = read_attribute_for_validation(record, attribute) - validate_each(record, attribute, value) - end + value = record.read_attribute_for_validation(attribute) + next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank]) + value = read_attribute_for_validation(record, attribute, value) + validate_each(record, attribute, value) end end @@ -167,9 +167,7 @@ def check_validity! end private - def read_attribute_for_validation(record, attr_name) - value = record.read_attribute_for_validation(attr_name) - throw(:allowed) if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank]) + def read_attribute_for_validation(record, attr_name, value) value end end