From d3b9f10d9c25c92a2b12cf2ccbdd49c48da2de0c Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Thu, 28 Apr 2016 12:18:31 -0400 Subject: [PATCH] Read message value directly from options **Why**: With the changes in PR #139, specifying a symbolized value for the `message` option would cause that method to be called on the Model object. If that method didn't exist, it resulted in a `NoMethodError` exception. If that method did exist in the Model, and also as an i18n key, then the Model method was used, as opposed to the i18n key. The `message` option is meant to either be a String, or a symbol corresponding to an i18n key. Therefore, it should be read directly from `options[:message]` and not sent for further processing by the `options_value` method. **How**: - Revert the `error_message` method back to its value prior to PR #139, so that it uses `options[:message]` - Add 3 new specs to cover these scenarios --- lib/validators/phony_validator.rb | 12 ++--- spec/lib/validators/phony_validator_spec.rb | 54 +++++++++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/lib/validators/phony_validator.rb b/lib/validators/phony_validator.rb index 9fcb3f2..9f1b462 100644 --- a/lib/validators/phony_validator.rb +++ b/lib/validators/phony_validator.rb @@ -15,7 +15,7 @@ def validate_each(record, attribute, value) private def error_message - options_value(:message) || :improbable_phone + options[:message] || :improbable_phone end def country_number @@ -43,11 +43,11 @@ def normalized_country_code end def options_value(option) - if options[option].is_a?(Symbol) - @record.send(options[option]) - else - options[option] - end + option_value = options[option] + + return option_value unless option_value.is_a?(Symbol) + + @record.send(option_value) end end diff --git a/spec/lib/validators/phony_validator_spec.rb b/spec/lib/validators/phony_validator_spec.rb index fcd2217..7783db2 100644 --- a/spec/lib/validators/phony_validator_spec.rb +++ b/spec/lib/validators/phony_validator_spec.rb @@ -143,6 +143,28 @@ class SymbolizableHelpfulHome < ActiveRecord::Base attr_accessor :phone_number, :phone_number_country_code validates_plausible_phone :phone_number, country_code: :phone_number_country_code end + +#-------------------- +class NoModelMethod < HelpfulHome + attr_accessor :phone_number + validates_plausible_phone :phone_number, country_code: :nonexistent_method +end + +#-------------------- +class MessageOptionUndefinedInModel < HelpfulHome + attr_accessor :phone_number + validates_plausible_phone :phone_number, message: :email +end + +#-------------------- +class MessageOptionSameAsModelMethod < HelpfulHome + attr_accessor :phone_number + validates_plausible_phone :phone_number, message: :email + + def email + 'user@example.com' + end +end #----------------------------------------------------------------------------------------------------------------------- # Tests #----------------------------------------------------------------------------------------------------------------------- @@ -544,5 +566,37 @@ class SymbolizableHelpfulHome < ActiveRecord::Base expect(@home.errors.messages).to include(phone_number: ['is an invalid number']) end end + + #-------------------- + context 'when a nonexistent method is passed as a symbol to an option other than message' do + it 'raises NoMethodError' do + @home = NoModelMethod.new + @home.phone_number = FRENCH_NUMBER_WITH_COUNTRY_CODE + + expect { @home.save }.to raise_error(NoMethodError) + end + end + + #-------------------- + context 'when a nonexistent method is passed as a symbol to the message option' do + it 'does not raise an error' do + @home = MessageOptionUndefinedInModel.new + @home.phone_number = INVALID_NUMBER + + expect { @home.save }.to_not raise_error + end + end + + #-------------------- + context 'when an existing Model method is passed as a symbol to the message option' do + it 'does not use the Model method' do + @home = MessageOptionSameAsModelMethod.new + @home.phone_number = INVALID_NUMBER + + expect(@home).to_not receive(:email) + + @home.save + end + end end end