Skip to content

Commit 975bf5b

Browse files
Tschuppi81Tschuppi81
committed
Extend the ValidPhoneNumber validator
1 parent 4018064 commit 975bf5b

4 files changed

Lines changed: 206 additions & 35 deletions

File tree

‎src/onegov/form/fields.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -775,7 +775,13 @@ class IconField(StringField):
775775
class PhoneNumberField(TelField):
776776
""" A string field with support for phone numbers. """
777777

778-
def __init__(self, *args: Any, country: str = 'CH', **kwargs: Any):
778+
def __init__(
779+
self,
780+
*args: Any,
781+
country: str = 'CH',
782+
number_type: str | None = None,
783+
**kwargs: Any
784+
):
779785

780786
self.country = country
781787
super().__init__(*args, **kwargs)

‎src/onegov/form/validators.py‎

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from datetime import datetime
1212
from decimal import Decimal
1313
from dateutil.relativedelta import relativedelta
14+
from enum import Enum
1415
from io import BytesIO
1516
from mimetypes import types_map
1617
from onegov.core.utils import binary_to_dictionary, dictionary_to_binary
@@ -665,25 +666,66 @@ def __call__(self, form: BaseForm, field: Field) -> None:
665666
raise StopValidation()
666667

667668

669+
class PhoneNumberType(Enum):
670+
ANY = 'any'
671+
MOBILE = 'mobile'
672+
FIXED_LINE = 'fixed_line'
673+
674+
668675
class ValidPhoneNumber:
669676
""" Makes sure the given input is valid phone number.
670677
671678
Expects an :class:`wtforms.StringField` instance.
672679
673680
"""
674681

675-
message = _('Not a valid phone number.')
682+
invalid_phone_number = _('Not a valid phone number.')
683+
invalid_country_code = _('Not a valid country code.')
684+
invalid_number_length = _('This phone number has an invalid length.')
685+
missing_area_code = _('Please include the area code.')
686+
unknown_number = _('This phone number does not exist.')
687+
mobile_required = _('Please enter a mobile phone number.')
688+
fixed_line_required = _('Please enter a landline phone number.')
689+
690+
length_errors = {
691+
phonenumbers.ValidationResult.INVALID_COUNTRY_CODE:
692+
invalid_country_code,
693+
phonenumbers.ValidationResult.TOO_SHORT: invalid_number_length,
694+
phonenumbers.ValidationResult.TOO_LONG: invalid_number_length,
695+
phonenumbers.ValidationResult.INVALID_LENGTH: invalid_number_length,
696+
phonenumbers.ValidationResult.IS_POSSIBLE_LOCAL_ONLY:
697+
missing_area_code,
698+
}
699+
700+
type_errors = {
701+
PhoneNumberType.FIXED_LINE.value: (
702+
(
703+
phonenumbers.PhoneNumberType.FIXED_LINE,
704+
phonenumbers.PhoneNumberType.FIXED_LINE_OR_MOBILE
705+
),
706+
fixed_line_required
707+
),
708+
PhoneNumberType.MOBILE.value: (
709+
(
710+
phonenumbers.PhoneNumberType.MOBILE,
711+
phonenumbers.PhoneNumberType.FIXED_LINE_OR_MOBILE
712+
),
713+
mobile_required
714+
),
715+
}
676716

677717
def __init__(
678718
self,
679719
country: str = 'CH',
680-
country_whitelist: Collection[str] | None = None
720+
country_whitelist: Collection[str] | None = None,
721+
phone_type: str = PhoneNumberType.ANY.value
681722
):
682723
if country_whitelist:
683724
assert country in country_whitelist
684725

685726
self.country = country
686727
self.country_whitelist = country_whitelist
728+
self.phone_type = phone_type
687729

688730
def __call__(self, form: Form, field: Field) -> None:
689731
if not field.data:
@@ -692,19 +734,33 @@ def __call__(self, form: Form, field: Field) -> None:
692734
try:
693735
number = phonenumbers.parse(field.data, self.country)
694736
except Exception as exception:
695-
raise ValidationError(self.message) from exception
737+
raise ValidationError(self.invalid_phone_number) from exception
738+
739+
reason = phonenumbers.is_possible_number_with_reason(number)
740+
if reason != phonenumbers.ValidationResult.IS_POSSIBLE:
741+
raise ValidationError(
742+
self.length_errors.get(reason, self.invalid_phone_number)
743+
)
744+
745+
if not phonenumbers.is_valid_number(number):
746+
raise ValidationError(self.unknown_number)
696747

697748
if self.country_whitelist:
698749
region = phonenumbers.region_code_for_number(number)
699750
if region not in self.country_whitelist:
700-
# FIXME: Better error message?
701-
raise ValidationError(self.message)
751+
raise ValidationError(_(
752+
'Phone numbers from this country are not supported. '
753+
'Allowed countries: ${countries}', mapping={
754+
'countries': ', '.join(sorted(self.country_whitelist))
755+
}
756+
))
702757

703-
if not (
704-
phonenumbers.is_valid_number(number)
705-
and phonenumbers.is_possible_number(number)
706-
):
707-
raise ValidationError(self.message)
758+
if self.phone_type == PhoneNumberType.ANY.value:
759+
return
760+
761+
valid_types, error = self.type_errors[self.phone_type]
762+
if phonenumbers.number_type(number) not in valid_types:
763+
raise ValidationError(error)
708764

709765

710766
class ValidSwissSocialSecurityNumber:

‎tests/onegov/form/test_fields.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,42 @@ def test_phone_number_field() -> None:
520520
field = field.bind(form, 'phone_number') # type: ignore[attr-defined]
521521
assert field.validators[0].country == 'DE'
522522

523+
form = Form()
524+
field = PhoneNumberField(
525+
validators=[ValidPhoneNumber(country='CH', phone_type='any')]
526+
)
527+
field = field.bind(form, 'phone_number') # type: ignore[attr-defined]
528+
field.data = '0791112233'
529+
assert field.validate(form)
530+
field.data = '0761112233'
531+
assert field.validate(form)
532+
field.data = '0411112233'
533+
assert field.validate(form)
534+
535+
form = Form()
536+
field = PhoneNumberField(
537+
validators=[ValidPhoneNumber(country='CH', phone_type='mobile')]
538+
)
539+
field = field.bind(form, 'phone_number') # type: ignore[attr-defined]
540+
field.data = '0791112233'
541+
assert field.validate(form)
542+
field.data = '0761112233'
543+
assert field.validate(form)
544+
field.data = '0411112233'
545+
assert not field.validate(form)
546+
547+
form = Form()
548+
field = PhoneNumberField(
549+
validators=[ValidPhoneNumber(country='CH', phone_type='fixed_line')]
550+
)
551+
field = field.bind(form, 'phone_number') # type: ignore[attr-defined]
552+
field.data = '0791112233'
553+
assert not field.validate(form)
554+
field.data = '0761112233'
555+
assert not field.validate(form)
556+
field.data = '0411112233'
557+
assert field.validate(form)
558+
523559

524560
def test_chosen_select_field() -> None:
525561
form = Form()

‎tests/onegov/form/test_validators.py‎

Lines changed: 97 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from onegov.form import Form
77
from onegov.form import parse_form
88
from onegov.form.fields import UploadField
9-
from onegov.form.validators import ExpectedExtensions
9+
from onegov.form.validators import ExpectedExtensions, PhoneNumberType
1010
from onegov.form.validators import ImageSizeLimit
1111
from onegov.form.validators import InputRequiredIf
1212
from onegov.form.validators import ValidSwissSocialSecurityNumber
@@ -82,47 +82,120 @@ def __init__(self, data: int | str | None) -> None:
8282
validator(request, Field(None))
8383
validator(request, Field(''))
8484

85+
# non-swiss numbers are allowed by default
86+
validator(request, Field('+4909562181751')) # DE
87+
validator(request, Field('+4319876543')) # A
88+
validator(request, Field('+43695621817')) # A
89+
validator(request, Field('+330956218175')) # FR
90+
validator(request, Field('+390612345678')) # IT
91+
8592
validator(request, Field('+41791112233'))
8693
validator(request, Field('0041791112233'))
8794
validator(request, Field('0791112233'))
8895

89-
# non-swiss numbers are allowed by default
90-
validator(request, Field('+4909562181751'))
96+
def error(data: int | str | None) -> str:
97+
with raises(ValidationError) as exception:
98+
validator(request, Field(data))
99+
return exception.value.args[0].interpolate()
91100

92-
with raises(ValidationError):
93-
validator(request, Field(1234))
94-
with raises(ValidationError):
95-
validator(request, Field('1234'))
101+
# numbers that cannot be parsed at all
102+
assert error(1234) == 'Not a valid phone number.'
103+
assert error('not a number') == 'Not a valid phone number.'
96104

97-
with raises(ValidationError):
98-
validator(request, Field('+417911122333'))
99-
with raises(ValidationError):
100-
validator(request, Field('041791112233'))
101-
with raises(ValidationError):
102-
validator(request, Field('041791112233'))
103-
with raises(ValidationError):
104-
validator(request, Field('00791112233'))
105+
# swiss numbers are 9 or 12 digits long, so anything shorter, longer
106+
# or in between reports the same length error
107+
invalid_length = 'This phone number has an invalid length.'
108+
assert error('1234') == invalid_length
109+
assert error('00791112233') == invalid_length
110+
assert error('+417911122334455') == invalid_length
111+
assert error('079111223344556677') == invalid_length
112+
assert error('+417911122333') == invalid_length
113+
assert error('+4179111223344') == invalid_length
105114

115+
# plausible length, but no such number exists
116+
assert error('041791112233') == 'This phone number does not exist.'
106117

107-
def test_phone_number_validator_whitelist() -> None:
118+
validator = ValidPhoneNumber(country='US')
119+
assert error('2345678') == 'Please include the area code.'
108120

109-
class Field(BaseField):
110-
def __init__(self, data: str | None) -> None:
111-
self.data = data
121+
validator = ValidPhoneNumber(phone_type=PhoneNumberType.ANY.value)
122+
validator(request, Field('+41791112233'))
123+
validator(request, Field('0041791112233'))
124+
validator(request, Field('41791112233'))
125+
validator(request, Field('41791112233'))
126+
validator(request, Field('41781112233'))
127+
validator(request, Field('41771112233'))
128+
validator(request, Field('41761112233'))
129+
validator(request, Field('41411112233'))
130+
131+
validator = ValidPhoneNumber(phone_type=PhoneNumberType.MOBILE.value)
132+
validator(request, Field('+41791112233'))
133+
validator(request, Field('0041791112233'))
134+
validator(request, Field('41791112233'))
135+
validator(request, Field('41791112233'))
136+
validator(request, Field('41781112233'))
137+
validator(request, Field('41771112233'))
138+
validator(request, Field('41761112233'))
139+
assert error('41411112233') == 'Please enter a mobile phone number.'
140+
141+
validator = ValidPhoneNumber(phone_type=PhoneNumberType.FIXED_LINE.value)
142+
landline = 'Please enter a landline phone number.'
143+
assert error('+41791112233') == landline
144+
assert error('0041791112233') == landline
145+
assert error('41791112233') == landline
146+
assert error('41781112233') == landline
147+
assert error('41771112233') == landline
148+
assert error('41761112233') == landline
149+
validator(request, Field('41411112233'))
150+
151+
# the number type is only checked once the number itself is valid
152+
assert error('041791112233') == 'This phone number does not exist.'
112153

113154
validator = ValidPhoneNumber(country_whitelist={'CH'})
114-
115-
request: Any = None
116155
validator(request, Field(None))
117156
validator(request, Field(''))
118157

119158
validator(request, Field('+41791112233'))
120159
validator(request, Field('0041791112233'))
121160
validator(request, Field('0791112233'))
122161

123-
with raises(ValidationError):
124-
# not a swiss number
125-
validator(request, Field('+4909562181751'))
162+
# not a swiss number
163+
unsupported = 'Phone numbers from this country are not supported.'
164+
assert error('+4909562181751') == ( # DE
165+
f'{unsupported} Allowed countries: CH'
166+
)
167+
168+
# the whitelist is listed in a stable order
169+
validator = ValidPhoneNumber(country_whitelist={'LI', 'CH', 'AT'})
170+
assert error('+4909562181751') == ( # DE
171+
f'{unsupported} Allowed countries: AT, CH, LI'
172+
)
173+
174+
# every whitelisted country is accepted
175+
validator(request, Field('+41791112233')) # CH
176+
validator(request, Field('+4319876543')) # AT
177+
validator(request, Field('+4232371234')) # LI
178+
179+
# numbers from anywhere else are not, no matter how far away
180+
assert error('+390612345678').startswith(unsupported) # IT
181+
assert error('+12025550123').startswith(unsupported) # US
182+
183+
# an invalid number is reported as such, not as a country problem
184+
assert error('+417911122333') == invalid_length
185+
assert error('+42366012345') == invalid_length
186+
187+
# the whitelist is combined with the other checks
188+
validator = ValidPhoneNumber(
189+
country_whitelist={'CH', 'AT'},
190+
phone_type=PhoneNumberType.MOBILE.value
191+
)
192+
validator(request, Field('+41791112233'))
193+
assert error('+41411112233') == 'Please enter a mobile phone number.'
194+
assert error('+390612345678').startswith(unsupported) # IT
195+
196+
# the default country has to be part of the whitelist
197+
with raises(AssertionError):
198+
ValidPhoneNumber(country='DE', country_whitelist={'CH'})
126199

127200

128201
def test_input_required_if_validator() -> None:

0 commit comments

Comments
 (0)