daddyz / phonelib

Ruby gem for phone validation and formatting using google libphonenumber library data
MIT License
1.04k stars 130 forks source link

Inconsistencies between ActiveRecord validator country_specifier and countries options #261

Closed morgant closed 1 year ago

morgant commented 1 year ago

Ruby: 2.7.4 Rails: 6.0.2 Phonelib: 0.7.0

Our application initially used the Phonelib ActiveRecord validator's countries option to specify all countries, but we have found this to be too permissive as the number of countries supported has grown. So, we're in the process of switching to using the validator's country_specifier option, but have run into some inconsistencies in validation results.

Specifically, in our test suite, we use a North Korean number for a number we don't expect to ever accept (the British Embassy in Pyongyang: "+850 2 381 7980"), but since switching to using country_specifier, it returns as valid.

In one of our models, we went from the following validation:

validates :contact_phone, phone: { countries: APP_CONFIG['countries'] }

To the following, with a new phone_validation_country method that returns an appropriate ISO3166 alpha2 string:

validates :contact_phone, phone: { country_specifier: :phone_validation_country, message: "invalid phone number for country" }

But with a contact_phone of "+850 2 381 7980" and phone_validation_country returning "US", it is considered valid.

I believe I have narrowed this down to Phonelib::Phone.parse/Phonelib::Phone.new/Phonelib::Phone#initialize vs Phonelib::Phone#valid_for_country?. For example:

Phonelib.parse("+850 2 381 7980", "US").countries # = ["KP"]
Phonelib.parse("+850 2 381 7980", "US").valid_countries # = ["KP"]
Phonelib.parse("+850 2 381 7980", "US").valid? # = true
Phonelib.parse("+850 2 381 7980", "US").valid_for_country?("US") # = false
Phonelib.parse("+850 2 381 7980", "US").valid_for_country?("KP") # = true

Note that #valid? returns true when passed "US" as a country, though #valid_for_country?("US") returns false.

Some examples of other test numbers that are at least still consistent between #valid? and #valid_for_country? (though I will note that the #countries and #valid_countries output are not necessarily what I would have expected):


# Canada 506 area code when parsed for "CA"; valid? == valid_for_country?("CA") == true
Phonelib.parse("+1 506-555-1212", "CA").countries # = ["CA"]
Phonelib.parse("+1 506-555-1212", "CA").valid_countries # = ["CA"]
Phonelib.parse("+1 506-555-1212", "CA").valid? # = true
Phonelib.parse("+1 506-555-1212", "CA").valid_for_country?("CA") # = true
Phonelib.parse("+1 506-555-1212", "CA").valid_for_country?("US") # = false

# Canada 506 area code when parsed for "US"; valid? == valid_for_country?("US") == false
Phonelib.parse("+1 506-555-1212", "US").countries # = ["US"]
Phonelib.parse("+1 506-555-1212", "US").valid_countries # = []
Phonelib.parse("+1 506-555-1212", "US").valid? # = false
Phonelib.parse("+1 506-555-1212", "US").valid_for_country?("CA") # = true
Phonelib.parse("+1 506-555-1212", "US").valid_for_country?("US") # = false

# United States 802 area code when parsed for "CA"; valid? == valid_for_country?("CA") == false
Phonelib.parse("+1 802-555-1212", "CA").countries # = ["CA"]
Phonelib.parse("+1 802-555-1212", "CA").valid_countries # = []
Phonelib.parse("+1 802-555-1212", "CA").valid? # = false
Phonelib.parse("+1 802-555-1212", "CA").valid_for_country?("CA") # = false
Phonelib.parse("+1 802-555-1212", "CA").valid_for_country?("US") # = true

# United States 802 area code when parsed for "US"; valid? == valid_for_country?("US") == true
Phonelib.parse("+1 802-555-1212", "US").countries # = ["US"]
Phonelib.parse("+1 802-555-1212", "US").valid_countries # = ["US"]
Phonelib.parse("+1 802-555-1212", "US").valid? # = true
Phonelib.parse("+1 802-555-1212", "US").valid_for_country?("CA") # = false
Phonelib.parse("+1 802-555-1212", "US").valid_for_country?("US") # = true
daddyz commented 1 year ago

@morgant thanks for reporting, the issue comes with ability of phonelib to ignore specified country in case it doesn't match the phone prefix. I just released 0.7.1 version introducing new option Phonelib.ignore_plus that allows to disable this behaviour. By default this is turned off, so place Phonelib.ignore_plus = true to your initializer.