search

joost / phony_rails

This Gem adds useful methods to your Rails app to validate, display and save phone numbers. It uses the super awesome Phony gem (https://github.com/floere/phony).
MIT License
555 stars 111 forks source link

Fix default country code in brackets #172

Closed p0wl closed 6 years ago

p0wl commented 6 years ago

Hey @joost,

thanks for your work! As mentioned in #170, the error we found exists only when the default country code is set. I adjusted the test case and fixed the issue (all tests passed).

What do you think, is this a feasible solution? I also thought about removing brackets and then checking for a + in the beginning (unless number.gsub(/[\(\)]/, '') =~ /\A\+/), but it seems overdone to me.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 99.809% when pulling 635582c4afd888a2c6b32b5a66046b2898ee3401 on p0wl:fix-default-country-code-with-brackets into 8554e3ac4457ea415fa01a9071f9b1cdbb6fb89c on joost:master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 99.809% when pulling 635582c4afd888a2c6b32b5a66046b2898ee3401 on p0wl:fix-default-country-code-with-brackets into 8554e3ac4457ea415fa01a9071f9b1cdbb6fb89c on joost:master.

joost commented 6 years ago

I would like to keep the \A in the regex since otherwise numbers with weird extensions are also handled differently.

p0wl commented 6 years ago

can you add a testcase for an example (with a weird extension), so I can provide another fix? =)

joost commented 6 years ago

Going to be fixed in https://github.com/joost/phony_rails/pull/182.