From 5150e33212e66bab290d87c31af40b9521c3b591 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 1 Mar 2022 16:42:46 +0100 Subject: [PATCH 1/3] models: ensure DROM phone numbers are valid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They were accepted before, because they were 'possible' – but now they are explicitely considered as valid. --- app/models/champs/phone_champ.rb | 23 ++++++++++++++++++++++- lib/core_ext/phonelib.rb | 14 ++++++++++++++ spec/models/champs/phone_champ_spec.rb | 4 ++++ 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 lib/core_ext/phonelib.rb diff --git a/app/models/champs/phone_champ.rb b/app/models/champs/phone_champ.rb index baa032d5b..3e4fe39ee 100644 --- a/app/models/champs/phone_champ.rb +++ b/app/models/champs/phone_champ.rb @@ -20,12 +20,33 @@ # type_de_champ_id :integer # class Champs::PhoneChamp < Champs::TextChamp + # We want to allow: + # * international (e164) phone numbers + # * “french format” (ten digits with a leading 0) + # * DROM numbers + # + # However, we need to special-case some ten-digit numbers, + # because the ARCEP assigns some blocks of "O6 XX XX XX XX" numbers to DROM operators. + # Guadeloupe | GP | +590 | 0690XXXXXX, 0691XXXXXX + # Guyane | GF | +594 | 0694XXXXXX + # Martinique | MQ | +596 | 0696XXXXXX, 0697XXXXXX + # Réunion | RE | +262 | 0692XXXXXX, 0693XXXXXX + # Mayotte | YT | +262 | 0692XXXXXX, 0693XXXXXX + # Nouvelle-Calédonie | NC | +687 | + # Polynésie française | PF | +689 | 40XXXXXX, 45XXXXXX, 87XXXXXX, 88XXXXXX, 89XXXXXX + # + # Cf: Plan national de numérotation téléphonique, + # https://www.arcep.fr/uploads/tx_gsavis/05-1085.pdf “Numéros mobiles à 10 chiffres”, page 6 + # + # See issue #6996. + DEFAULT_COUNTRY_CODES = [:FR, :GP, :GF, :MQ, :RE, :YT, :NC, :PF].freeze + validates :value, phone: { possible: true, allow_blank: true, message: I18n.t(:not_a_phone, scope: 'activerecord.errors.messages') - }, unless: -> { Phonelib.valid_for_country?(value, :pf) } + }, unless: -> { Phonelib.valid_for_countries?(value, DEFAULT_COUNTRY_CODES) } def to_s value.present? ? Phonelib.parse(value).full_national : '' diff --git a/lib/core_ext/phonelib.rb b/lib/core_ext/phonelib.rb new file mode 100644 index 000000000..1345a0c75 --- /dev/null +++ b/lib/core_ext/phonelib.rb @@ -0,0 +1,14 @@ +# Class extensions to the Phonelib module, which allow parsing using several countries at once. +module Phonelib + # Variation of `.valid_for_country`, that can check several countries at once. + def self.valid_for_countries?(value, countries) + countries.any? { |country| valid_for_country?(value, country) } + end + + # Variation of `Phonelib.parse`, which parses the given string using the first country + # for which the phone number is valid. + def self.parse_for_countries(value, passed_countries = []) + valid_country = passed_countries.find { |country| valid_for_country?(value, country) } + parse(value, valid_country) + end +end diff --git a/spec/models/champs/phone_champ_spec.rb b/spec/models/champs/phone_champ_spec.rb index 4d1f12f6e..9b2bd7bdd 100644 --- a/spec/models/champs/phone_champ_spec.rb +++ b/spec/models/champs/phone_champ_spec.rb @@ -22,6 +22,10 @@ describe Champs::PhoneChamp do expect(champ_with_value("+1(0) - 123456789")).to be_valid expect(champ_with_value("+49 2109 87654321")).to be_valid expect(champ_with_value("012345678")).to be_valid + # DROM numbers should be valid + expect(champ_with_value("06 96 04 78 07")).to be_valid + expect(champ_with_value("05 94 22 31 31")).to be_valid + expect(champ_with_value("+594 5 94 22 31 31")).to be_valid # polynesian numbers should not return errors in any way ## landline numbers start with 40 or 45 expect(champ_with_value("45187272")).to be_valid From f35d18cd5c7c6ea09bd9e2a4b5d5df1bbf58d73d Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 1 Mar 2022 16:44:16 +0100 Subject: [PATCH 2/3] models: stop truncating DROM phone numbers Fix #6996 --- app/models/champs/phone_champ.rb | 3 ++- spec/models/champs/phone_champ_spec.rb | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/models/champs/phone_champ.rb b/app/models/champs/phone_champ.rb index 3e4fe39ee..ae06abe14 100644 --- a/app/models/champs/phone_champ.rb +++ b/app/models/champs/phone_champ.rb @@ -49,6 +49,7 @@ class Champs::PhoneChamp < Champs::TextChamp }, unless: -> { Phonelib.valid_for_countries?(value, DEFAULT_COUNTRY_CODES) } def to_s - value.present? ? Phonelib.parse(value).full_national : '' + return '' if value.blank? + Phonelib.parse_for_countries(value, DEFAULT_COUNTRY_CODES).full_national end end diff --git a/spec/models/champs/phone_champ_spec.rb b/spec/models/champs/phone_champ_spec.rb index 9b2bd7bdd..df7e9095a 100644 --- a/spec/models/champs/phone_champ_spec.rb +++ b/spec/models/champs/phone_champ_spec.rb @@ -40,9 +40,21 @@ describe Champs::PhoneChamp do expect(champ_with_value("88473500")).to be_valid expect(champ_with_value("89473500")).to be_valid end + end - def champ_with_value(number) - phone_champ.tap { |c| c.value = number } + describe '#to_s' do + context 'for valid phone numbers' do + it 'returns the national part of the number, formatted nicely' do + expect(champ_with_value("0115789055").to_s).to eq("01 15 78 90 55") + expect(champ_with_value("+33115789055").to_s).to eq("01 15 78 90 55") + # DROM phone numbers are formatted differently – but still formatted + expect(champ_with_value("0696047807").to_s).to eq("0696 04 78 07") + expect(champ_with_value("45187272").to_s).to eq("45187272") + end end end + + def champ_with_value(number) + phone_champ.tap { |c| c.value = number } + end end From e32c9a9f94e88321003d4b615804da49c957bed2 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 1 Mar 2022 16:45:44 +0100 Subject: [PATCH 3/3] models: don't attempt to format invalid phone numbers This is a defensive-programming measure, because formatting an invalid phone number may truncate some leading numbers. --- app/models/champs/phone_champ.rb | 9 ++++++++- spec/models/champs/phone_champ_spec.rb | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/models/champs/phone_champ.rb b/app/models/champs/phone_champ.rb index ae06abe14..ac2c51449 100644 --- a/app/models/champs/phone_champ.rb +++ b/app/models/champs/phone_champ.rb @@ -50,6 +50,13 @@ class Champs::PhoneChamp < Champs::TextChamp def to_s return '' if value.blank? - Phonelib.parse_for_countries(value, DEFAULT_COUNTRY_CODES).full_national + + if Phonelib.valid_for_countries?(value, DEFAULT_COUNTRY_CODES) + Phonelib.parse_for_countries(value, DEFAULT_COUNTRY_CODES).full_national + else + # When he phone number is possible for the default countries, but not strictly valid, + # `full_national` could mess up the formatting. In this case just return the original. + value + end end end diff --git a/spec/models/champs/phone_champ_spec.rb b/spec/models/champs/phone_champ_spec.rb index df7e9095a..275986c33 100644 --- a/spec/models/champs/phone_champ_spec.rb +++ b/spec/models/champs/phone_champ_spec.rb @@ -52,6 +52,12 @@ describe Champs::PhoneChamp do expect(champ_with_value("45187272").to_s).to eq("45187272") end end + + context 'for possible (but not valid) phone numbers' do + it 'returns the original' do + expect(champ_with_value("1234").to_s).to eq("1234") + end + end end def champ_with_value(number)