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
556 stars 111 forks source link

Country code wrapped in brackets is not recognized #170

Closed widescape closed 6 years ago

widescape commented 6 years ago

What I expect:

PhonyRails.normalize_number('(+49) 175 123 4567') => "+491751234567"

What I get instead:

PhonyRails.normalize_number('(+49) 175 123 4567') => "+49491751234567"
# The country code '49' appears twice

How is that possible?

The issue seems to be that PhonyRails.normalize_number_default_country(…) looks for a + at the first position of the string. If that fails, it appends the default country code and checks if that is a valid number.

Unfortunately:

Suggested solution

Try to detect the + and the default_country_number within the first characters, not at the first position only.

joost commented 6 years ago

For me it works, see spec.

p0wl commented 6 years ago

Thanks for the fast response and your work!

We can reproduce it when the default_country_code is matching the country prefix:

The following test case fails:

    it 'should pass Github issue #170' do
      PhonyRails.default_country_code = 'DE'
      phone = '(+49) 175 123 4567'
      phone = PhonyRails.normalize_number(phone)
      expect(phone).to eq('+491751234567')
    end

it is passing when default_country_code is set to anything other than 'DE'.

p0wl commented 6 years ago

Hey @joost, I saw that the new version (v0.14.7) includes your spec for this issue. But as said, your spec does not cover our issue. The spec should be different:

    it 'should pass Github issue #170' do
      PhonyRails.default_country_code = 'DE'
      phone = '(+49) 175 123 4567'
      phone = PhonyRails.normalize_number(phone)
      expect(phone).to eq('+491751234567')
    end

Can you please change the spec? Thanks!