From a12fe8654f69237139997d1a19c590b46dc3a8e6 Mon Sep 17 00:00:00 2001 From: Brian McManus Date: Wed, 28 Oct 2015 10:01:26 -0700 Subject: [PATCH 1/3] Numericality validation of virtual attributes Commit #18b2859d2522a4866c398b9a32ebc3de4ec79389 broke numericality validation of virtual attributes in ActiveRecord models. This commit added a `column_type` method that assumes if `columns_hash` is a thing then our attribute would be present within it. This is not true for virtual attributes and leads to: ``` NoMethodError: undefined method `type' for nil:NilClass # /Users/bdmac/.gem/ruby/2.2.2/gems/shoulda-matchers-3.0.1/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb:442:in # `column_type' ``` This adds a check to `column_type` to return nil instead of throwing an exception if a specified attribute does not exist in the `columns_hash` because it is virtual. --- .../validate_numericality_of_matcher.rb | 3 +- .../validate_numericality_of_matcher_spec.rb | 98 +++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb index 9d3d0f787..e066f482f 100644 --- a/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb @@ -438,7 +438,8 @@ def given_numeric_column? private def column_type - if @subject.class.respond_to?(:columns_hash) + if @subject.class.respond_to?(:columns_hash) && + @subject.class.columns_hash[@attribute.to_s] @subject.class.columns_hash[@attribute.to_s].type end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb index 31d2d8af3..eb5e99b5b 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb @@ -127,6 +127,13 @@ def default_validation_values expect(record).to validate_numericality end + context 'when the attribute is a virtual' do + it 'accepts' do + record = build_record_validating_numericality_of_virtual_attribute + expect(record).to validate_virtual_attribute_numericality + end + end + context 'when the column is an integer column' do it 'raises an IneffectiveTestError' do record = build_record_validating_numericality( @@ -224,6 +231,15 @@ def default_validation_values expect(record).to validate_numericality.odd end + context 'when the attribute is a virtual' do + it 'accepts' do + record = build_record_validating_numericality_of_virtual_attribute( + odd: true, + ) + expect(record).to validate_virtual_attribute_numericality.odd + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -278,6 +294,15 @@ def default_validation_values expect(record).to validate_numericality.even end + context 'when the attribute is a virtual' do + it 'accepts' do + record = build_record_validating_numericality_of_virtual_attribute( + even: true, + ) + expect(record).to validate_virtual_attribute_numericality.even + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -332,6 +357,16 @@ def default_validation_values expect(record).to validate_numericality.is_less_than_or_equal_to(18) end + context 'when the attribute is a virtual' do + it 'accepts' do + record = build_record_validating_numericality_of_virtual_attribute( + less_than_or_equal_to: 18, + ) + expect(record).to validate_virtual_attribute_numericality. + is_less_than_or_equal_to(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -390,6 +425,16 @@ def default_validation_values is_less_than(18) end + context 'when the attribute is a virtual' do + it 'accepts' do + record = build_record_validating_numericality_of_virtual_attribute( + less_than: 18, + ) + expect(record).to validate_virtual_attribute_numericality. + is_less_than(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -446,6 +491,16 @@ def default_validation_values expect(record).to validate_numericality.is_equal_to(18) end + context 'when the attribute is a virtual' do + it 'accepts' do + record = build_record_validating_numericality_of_virtual_attribute( + equal_to: 18, + ) + expect(record).to validate_virtual_attribute_numericality. + is_equal_to(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -504,6 +559,16 @@ def default_validation_values is_greater_than_or_equal_to(18) end + context 'when the attribute is a virtual' do + it 'accepts' do + record = build_record_validating_numericality_of_virtual_attribute( + greater_than_or_equal_to: 18, + ) + expect(record).to validate_virtual_attribute_numericality. + is_greater_than_or_equal_to(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -568,6 +633,16 @@ def default_validation_values is_greater_than(18) end + context 'when the attribute is a virtual' do + it 'accepts' do + record = build_record_validating_numericality_of_virtual_attribute( + greater_than: 18, + ) + expect(record).to validate_virtual_attribute_numericality. + is_greater_than(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -1141,6 +1216,21 @@ def define_model_validating_numericality(options = {}) end end + def define_model_validating_numericality_of_virtual_attribute(options = {}) + virtual_attr = options.delete(:virtual_attribute) do + virtual_attribute_name + end + + define_model 'Example' do |model| + model.send(:attr_accessor, virtual_attr) + model.validates_numericality_of(virtual_attr, options) + end + end + + def build_record_validating_numericality_of_virtual_attribute(options = {}) + define_model_validating_numericality_of_virtual_attribute(options).new + end + def build_record_validating_numericality(options = {}) define_model_validating_numericality(options).new end @@ -1157,7 +1247,15 @@ def validate_numericality validate_numericality_of(attribute_name) end + def validate_virtual_attribute_numericality + validate_numericality_of(virtual_attribute_name) + end + def attribute_name :attr end + + def virtual_attribute_name + :virtual_attr + end end From 79a796197afe7124cf530a32a19235aef3e186a0 Mon Sep 17 00:00:00 2001 From: Brian McManus Date: Thu, 29 Oct 2015 08:36:09 -0700 Subject: [PATCH 2/3] Remove virtual_attribute_name --- .../validate_numericality_of_matcher_spec.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb index eb5e99b5b..29a4556e1 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb @@ -1217,8 +1217,8 @@ def define_model_validating_numericality(options = {}) end def define_model_validating_numericality_of_virtual_attribute(options = {}) - virtual_attr = options.delete(:virtual_attribute) do - virtual_attribute_name + virtual_attr = options.delete(:attribute_name) do + attribute_name end define_model 'Example' do |model| @@ -1248,14 +1248,10 @@ def validate_numericality end def validate_virtual_attribute_numericality - validate_numericality_of(virtual_attribute_name) + validate_numericality_of(attribute_name) end def attribute_name :attr end - - def virtual_attribute_name - :virtual_attr - end end From 1cf0b8521cfecc1bf6c2b84c7448131f7d90ff4d Mon Sep 17 00:00:00 2001 From: Brian McManus Date: Thu, 29 Oct 2015 09:16:10 -0700 Subject: [PATCH 3/3] Remove validate_virtual_attribute_numericality --- .../validate_numericality_of_matcher_spec.rb | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb index 29a4556e1..830a7dcb6 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb @@ -130,7 +130,7 @@ def default_validation_values context 'when the attribute is a virtual' do it 'accepts' do record = build_record_validating_numericality_of_virtual_attribute - expect(record).to validate_virtual_attribute_numericality + expect(record).to validate_numericality end end @@ -236,7 +236,7 @@ def default_validation_values record = build_record_validating_numericality_of_virtual_attribute( odd: true, ) - expect(record).to validate_virtual_attribute_numericality.odd + expect(record).to validate_numericality.odd end end @@ -299,7 +299,7 @@ def default_validation_values record = build_record_validating_numericality_of_virtual_attribute( even: true, ) - expect(record).to validate_virtual_attribute_numericality.even + expect(record).to validate_numericality.even end end @@ -362,7 +362,7 @@ def default_validation_values record = build_record_validating_numericality_of_virtual_attribute( less_than_or_equal_to: 18, ) - expect(record).to validate_virtual_attribute_numericality. + expect(record).to validate_numericality. is_less_than_or_equal_to(18) end end @@ -430,7 +430,7 @@ def default_validation_values record = build_record_validating_numericality_of_virtual_attribute( less_than: 18, ) - expect(record).to validate_virtual_attribute_numericality. + expect(record).to validate_numericality. is_less_than(18) end end @@ -496,7 +496,7 @@ def default_validation_values record = build_record_validating_numericality_of_virtual_attribute( equal_to: 18, ) - expect(record).to validate_virtual_attribute_numericality. + expect(record).to validate_numericality. is_equal_to(18) end end @@ -564,7 +564,7 @@ def default_validation_values record = build_record_validating_numericality_of_virtual_attribute( greater_than_or_equal_to: 18, ) - expect(record).to validate_virtual_attribute_numericality. + expect(record).to validate_numericality. is_greater_than_or_equal_to(18) end end @@ -638,7 +638,7 @@ def default_validation_values record = build_record_validating_numericality_of_virtual_attribute( greater_than: 18, ) - expect(record).to validate_virtual_attribute_numericality. + expect(record).to validate_numericality. is_greater_than(18) end end @@ -1247,10 +1247,6 @@ def validate_numericality validate_numericality_of(attribute_name) end - def validate_virtual_attribute_numericality - validate_numericality_of(attribute_name) - end - def attribute_name :attr end