ianks / mini_phone

A fast phone number parser, validator and formatter for Ruby. This gem binds to Google's C++ libphonenumber for spec-compliance and performance.
MIT License
160 stars 8 forks source link

valid_for_country? returns true for valid phones with invalid country codes. #5

Closed desheikh closed 3 years ago

desheikh commented 3 years ago

Hello! When using a valid UK phone number: +447975777666'

MiniPhone.valid_for_country?('+447975777666', 'US')

or

MiniPhone.parse('+447975777666', 'US').valid?

both return true.

This is because we are using IsValidNumber instead of IsValidNumberForRegion here: https://github.com/ianks/mini_phone/blob/master/ext/mini_phone/mini_phone.cc#L55

This behavior feels somewhat unexpected as the country code argument is essentially overridden when either method is called with a phone number with a country code prefix.

I think we might want to change the behaviour to use the country_code if it is passed in. Happy to help push up a pr if that makes sense.

ianks commented 3 years ago

Good call @desheikh , and thanks for the report! Feel free to dive in if you want, otherwise I will get around to it soon!

ianks commented 3 years ago

@desheikh took a stab at it, turned out to be a little trickier than I thought, but I think #6 solves it. Let me know if you run into any other issues with it!

desheikh commented 3 years ago

Thanks @ianks, that was quick! Just reviewed the PR and it looks like that solves both issues 🚀