Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Country code not set when first two digits eq country code #50

Closed
espen opened this issue Mar 6, 2014 · 6 comments
Closed

Country code not set when first two digits eq country code #50

espen opened this issue Mar 6, 2014 · 6 comments

Comments

@espen
Copy link
Contributor

espen commented Mar 6, 2014

PhonyRails.normalize_number("47112233", :default_country_code => "NO")
=> "47112233"

Should be 4747112233. The check for adding country code is just 'if not number =~ /^(00|+|#{default_country_number})/'.

@espen
Copy link
Contributor Author

espen commented Mar 6, 2014

My first thought is to use Phony plausible. If the country code is set then it should pass plausible. If not then country code can be added. If that is not plausible then return nil.

@joost
Copy link
Owner

joost commented Mar 6, 2014

Sounds good :) pull request welcome!

@espen
Copy link
Contributor Author

espen commented Mar 6, 2014

Phony.plausible fails with current Phony version as in Gemfile.lock, but using latest Phony it works for the example phone number in this issue. However using plausible breaks 26 other tests, example: floere/phony#143

@espen
Copy link
Contributor Author

espen commented Mar 9, 2014

Are all phone numbers in the spec valid?

Phony.plausible?('310101234123')
=> false

@joost
Copy link
Owner

joost commented Mar 10, 2014

310101234123 is not a valid dutch number. It should be like '010 1234123' (without country code) or '+31 10 1234123' (with country code).

@espen
Copy link
Contributor Author

espen commented Mar 10, 2014

Thanks. I was just checking plausible after adding +31 which failed the test - but normalizing it first would fix that. Due this issue being quite critical for some of my users I had to remove phony_rails and just do some simple number formatting directly with Phony (trying to send SMS with incorrect number is less of a problem than not being able to format a correct number). Currently this is what I am using as code which works for my simple tests. I will try to implement this in phony_rails later:

  def format_phone_number(phone_number)
    default_country_code = Country[self.client.country_code].country_code
    if Phony.plausible?(phone_number)
      phone_number = Phony.normalize(phone_number)
    else
      phone_number = Phony.normalize("+#{default_country_code}#{phone_number}")
      if !Phony.plausible?(phone_number)
        return nil
      end
    end
    return Phony.formatted(phone_number, :format => :international, :spaces => '')
  end

@joost joost closed this as completed May 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants