ensure a non-nil error message for invalid emails and phone numbers

[bcm]

I found that I needed to add this because my email validity tests were failing without it. using shoulda-matchers, I wrote the following test with rspec:

it { is_expected.not_to allow_value('foo').for(:contact_email) }

the test failed when shoulda-matchers raised a TypeError with message can't dup NilClass. tracing this I found that after calling valid? on my record, the AM::Errors object had this error:

:contact_email => [
 [0] nil
]

for some reason passing a nil message to AM:Errors#add was not resulting in the default :invalid message. this patch, however, makes things work as expected.

it's not clear to me why the invalid error message test in this library passed before. but it's still passing now, and my application works as expected.

[franckverrot]

Never noticed this but I'm probably never passing a custom options[:message] :-)

Before I merge, could you please add a test that is passing an explicit nil message. That way we'll be sure we're fixing something broken ;-)

Thanks a lot!

[bcm]

I'm not passing an explicit nil message either. here's how I defined my validation (not sure why I didn't include it before):

validates :contact_email, :presence => true, :email => {:allow_blank => true},
          :length => {:maximum => 255, :allow_blank => true}

the problem my patch solves is with this line in the email validator:

record.errors.add attribute, (options[:message]) unless valid

because my validation doesn't define a message option, nil is being passed as the second parameter to AM::Errors#add, and for whatever reason that's not causing the message to default to :invalid.

although I can't explain this behavior, making sure that the email validator provides a value when the validation doesn't define a custom message causes the expected behavior.

[bcm]

aha! the issue appears when using ruby 2.2. it also occurs in the phone validator.

(weirdly, I had to update minitest to 5.4.3 in order to get the tests to run under 2.2.0.)

[franckverrot]

@bcm I think I'll investigate on the issue. Do you have any news on your side? Thanks!

[bcm]

sorry, I haven't looked into it anymore since then.