Skip to content

Commit

Permalink
Update validation code for non_emergency contact numbers to accept no…
Browse files Browse the repository at this point in the history
…n_emergency 3 digit numbers and numbers with extentions

The python port of libphonenumber is much stricter than our old bespoke validation code. As a result it does not accept phone numbers with extensions or phone numbers with 3 digits. This change incorporates the changes to utils that throw an appropriate error, and adds a crude method for handling numbers with extensions. The form will accept any number that returns a TOO_LONG error code, assuming it must be because of an extension (this shouldn't matter as we are not validating whether the phone number is real for notify to send sms notifications to. The service owner will be confident that they're providing the correct number for their service)
  • Loading branch information
rparke committed Nov 20, 2024
1 parent ee63899 commit 6fa938c
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 9 deletions.
15 changes: 9 additions & 6 deletions app/main/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -1805,13 +1805,16 @@ def validate(self, *args, **kwargs):
# and disallow emergency 3-digit numbers
def valid_non_emergency_phone_number(self, num):
try:
number = PhoneNumberUtils(num.data)
normalised_number = number.get_normalised_format()
PhoneNumberUtils(num.data, is_service_contact_number=True)
except InvalidPhoneError as e:
raise ValidationError("Enter a phone number in the correct format") from e

if normalised_number in {"999", "112"}:
raise ValidationError("Phone number cannot be an emergency number")
if e.code == InvalidPhoneError.Codes.UNSUPPORTED_EMERGENCY_NUMBER:
raise ValidationError(str(e)) from e
elif e.code == InvalidPhoneError.Codes.TOO_LONG:
# assume the number is an extension and return the number with minimal normalisation
return True

else:
raise ValidationError("Enter a phone number in the correct format") from e

return True

Expand Down
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ notifications-python-client==10.0.0
fido2==1.1.3

# Run `make bump-utils` to update to the latest version
notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@f676a72f212ba9d8b1e8cc4dc5fd3211a2214960
notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@de4d6573a0dfb5969156b5a7595f00c54f492cc0

govuk-frontend-jinja==3.4.0

Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ mistune==0.8.4
# via notifications-utils
notifications-python-client==10.0.0
# via -r requirements.in
notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@f676a72f212ba9d8b1e8cc4dc5fd3211a2214960
notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@de4d6573a0dfb5969156b5a7595f00c54f492cc0
# via -r requirements.in
openpyxl==3.0.10
# via pyexcel-xlsx
Expand Down
2 changes: 1 addition & 1 deletion requirements_for_test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ mypy-extensions==1.0.0
# via black
notifications-python-client==10.0.0
# via -r requirements.in
notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@f676a72f212ba9d8b1e8cc4dc5fd3211a2214960
notifications-utils @ git+https://github.com/alphagov/notifications-utils.git@de4d6573a0dfb5969156b5a7595f00c54f492cc0
# via -r requirements.in
openpyxl==3.0.10
# via pyexcel-xlsx
Expand Down
40 changes: 40 additions & 0 deletions tests/app/main/forms/test_service_contact_details_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,43 @@ def test_form_phone_number_validation_fails_with_invalid_phone_number_field(noti
assert not form.validate_on_submit()
assert len(form.errors) == 1
assert "Enter a phone number in the correct format" in form.errors["phone_number"]


@pytest.mark.parametrize(
"short_number, allowed",
(
("119", True),
("999", False),
("112", False),
(" 999 ", False),
("(9)99", False),
("9-9-9", False),
),
)
def test_form_phone_number_allows_non_emergency_3_digit_numbers(notify_admin, short_number, allowed):
data = {"contact_details_type": "phone_number", "phone_number": short_number}

with notify_admin.test_request_context(method="POST", data=data):
form = ServiceContactDetailsForm()
if allowed:
assert form.validate_on_submit()
assert len(form.errors) == 0
assert form.errors == {}
else:
assert not form.validate_on_submit()
assert len(form.errors) == 1
assert form.errors["phone_number"] == ["Phone number cannot be an emergency number"]


@pytest.mark.parametrize(
"short_number, allowed",
(("01572 812241 7081", True),),
)
def test_form_phone_number_allows_non_emergency_numbers_with_extensions(notify_admin, short_number, allowed):
data = {"contact_details_type": "phone_number", "phone_number": short_number}

with notify_admin.test_request_context(method="POST", data=data):
form = ServiceContactDetailsForm()
assert form.validate_on_submit()
assert len(form.errors) == 0
assert form.errors == {}

0 comments on commit 6fa938c

Please sign in to comment.