plausible_number? is far too permissive

[asilano]

Because normalize_number strips all non-phonelike characters from its input, you get weird results like the following:

PhonyRails.plausible_number?('https://doi.org/10.1002/rse2.195', country_code: 'GB')
# => true

[joost]

Any suggested fix? Open to pull requests.

On Thu, Jan 28, 2021, 12:19 Chris Howlett notifications@github.com wrote:

Because normalize_number strips all non-phonelike characters from its input, you get weird results like the following:

PhonyRails.plausible_number?('https://doi.org/10.1002/rse2.195', country_code: 'GB')

=> true

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/joost/phony_rails/issues/204, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAWKSXCSGCAALXLXJFVJDTS4FB2PANCNFSM4WW4E3AA .

[asilano]

Well, it depends on what you, as owner, want the behaviour to be. I see that you strip any non-(digit, bracket or plus), and I can see that stripping dashes from +44-1234-567-890 (for instance) makes sense. Letters feel like they're less likely to appear in a valid number (especially since extensions are dealt with earlier).

Perhaps it should strip all non-phonelike, non-alphabetic characters?

[asilano]

@joost I want to try and submit a pull request, but I can't get tests running. I've:

• Forked asilano/phony_rails
• Cloned
• Bundled
• bundle exec rspec spec

I get the following error output:

No DRb server is running. Running in local process instead ...
[Coveralls] Set up the SimpleCov formatter.
[Coveralls] Using SimpleCov's default settings.

An error occurred while loading ./spec/lib/phony_rails_spec.rb.
Failure/Error: require 'mongoid'

NameError:
  uninitialized constant ActiveModel::Serializers::Xml
# ./spec/spec_helper.rb:12:in `require'
# ./spec/spec_helper.rb:12:in `<top (required)>'
# ./spec/lib/phony_rails_spec.rb:3:in `require'
# ./spec/lib/phony_rails_spec.rb:3:in `<top (required)>'
[Coveralls] Set up the SimpleCov formatter.
[Coveralls] Using SimpleCov's default settings.

An error occurred while loading ./spec/lib/validators/phony_validator_spec.rb.
Failure/Error: require 'mongoid'

NameError:
  uninitialized constant ActiveModel::Serializers::Xml
# ./spec/spec_helper.rb:12:in `require'
# ./spec/spec_helper.rb:12:in `<top (required)>'
# ./spec/lib/validators/phony_validator_spec.rb:3:in `require'
# ./spec/lib/validators/phony_validator_spec.rb:3:in `<top (required)>'
No examples found.

Finished in 0.00006 seconds (files took 1.1 seconds to load)
0 examples, 0 failures, 2 errors occurred outside of examples

[Coveralls] Outside the CI environment, not sending data.

My Gemfile.lock now looks like this:

``` PATH remote: . specs: phony_rails (0.14.13) activesupport (>= 3.0) phony (> 2.15) GEM remote: https://rubygems.org/ specs: activemodel (6.1.3) activesupport (= 6.1.3) activerecord (6.1.3) activemodel (= 6.1.3) activesupport (= 6.1.3) activesupport (6.1.3) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) tzinfo (~> 2.0) zeitwerk (~> 2.3) ast (2.4.2) bson (3.2.7) coderay (1.1.3) concurrent-ruby (1.1.8) connection_pool (2.2.3) coveralls (0.8.23) json (>= 1.8, < 3) simplecov (~> 0.16.1) term-ansicolor (~> 1.3) thor (>= 0.19.4, < 2.0) tins (~> 1.6) diff-lcs (1.4.4) docile (1.3.5) ffi (1.14.2) formatador (0.2.5) guard (2.16.2) formatador (>= 0.2.4) listen (>= 2.7, < 4.0) lumberjack (>= 1.0.12, < 2.0) nenv (~> 0.1) notiffany (~> 0.0) pry (>= 0.9.12) shellany (~> 0.0) thor (>= 0.18.1) guard-bundler (3.0.0) bundler (>= 2.1, < 3) guard (~> 2.2) guard-compat (~> 1.1) guard-compat (1.2.1) guard-rspec (4.7.3) guard (~> 2.1) guard-compat (~> 1.1) rspec (>= 2.99.0, < 4.0) i18n (1.8.9) concurrent-ruby (~> 1.0) json (2.5.1) listen (3.4.1) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) lumberjack (1.2.8) method_source (1.0.0) minitest (5.14.3) mongoid (4.0.0.beta2) activemodel (>= 4.0.0) moped (~> 2.0.beta6) origin (~> 2.1) tzinfo (>= 0.3.37) moped (2.0.7) bson (~> 3.0) connection_pool (~> 2.0) optionable (~> 0.2.0) nenv (0.3.0) notiffany (0.1.3) nenv (~> 0.1) shellany (~> 0.0) optionable (0.2.0) origin (2.3.1) parallel (1.20.1) parser (3.0.0.0) ast (~> 2.4.1) phony (2.18.19) pry (0.14.0) coderay (~> 1.1) method_source (~> 1.0) rainbow (3.0.0) rake (13.0.3) rb-fsevent (0.10.4) rb-inotify (0.10.1) ffi (~> 1.0) regexp_parser (2.0.3) rexml (3.2.4) rspec (3.10.0) rspec-core (~> 3.10.0) rspec-expectations (~> 3.10.0) rspec-mocks (~> 3.10.0) rspec-core (3.10.1) rspec-support (~> 3.10.0) rspec-expectations (3.10.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.10.0) rspec-mocks (3.10.2) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.10.0) rspec-support (3.10.2) rubocop (1.10.0) parallel (~> 1.10) parser (>= 3.0.0.0) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) rexml rubocop-ast (>= 1.2.0, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 3.0) rubocop-ast (1.4.1) parser (>= 2.7.1.5) rubocop-performance (1.9.2) rubocop (>= 0.90.0, < 2.0) rubocop-ast (>= 0.4.0) ruby-progressbar (1.11.0) shellany (0.0.1) simplecov (0.16.1) docile (~> 1.1) json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) sqlite3 (1.4.2) sync (0.5.0) term-ansicolor (1.7.1) tins (~> 1.0) thor (1.1.0) tins (1.28.0) sync tzinfo (2.0.4) concurrent-ruby (~> 1.0) unicode-display_width (2.0.0) zeitwerk (2.4.2) PLATFORMS ruby DEPENDENCIES activerecord (>= 3.0) coveralls guard guard-bundler guard-rspec mongoid (>= 3.0) phony_rails! rake rspec rubocop rubocop-performance sqlite3 (>= 1.4.0) BUNDLED WITH 2.1.4 ```

[joost]

Hi @asilano! I see things broke when using phony_rails with the more recent gems. I've fixed the tests by some updates & by removing MongoDb support. I will push a branch for you to have a look at little later today.

[joost]

Please see the https://github.com/joost/phony_rails/tree/no_mongoid branch for a version with working specs. Pull request welcome!

[asilano]

Thanks, the branch works nicely.

Unfortunately, my fix doesn't - normalize_number is not massively to blame, and in fact Phony thinks that Phony.plausible?('https://example.com/15.55/a5b5c5/rse5.555') is true as well.

I'll have a think, and maybe I'll go bother them too :)