From 6fa938c08f2626094ae59b4af29ecc27b2607c12 Mon Sep 17 00:00:00 2001 From: Richard Parke Date: Fri, 15 Nov 2024 17:25:12 +0000 Subject: [PATCH] Update validation code for non_emergency contact numbers to accept non_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) --- app/main/forms.py | 15 ++++--- requirements.in | 2 +- requirements.txt | 2 +- requirements_for_test.txt | 2 +- .../test_service_contact_details_form.py | 40 +++++++++++++++++++ 5 files changed, 52 insertions(+), 9 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 449dd4046f..9f77d6a818 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -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 diff --git a/requirements.in b/requirements.in index 497c40692f..aa3c92a159 100644 --- a/requirements.in +++ b/requirements.in @@ -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 diff --git a/requirements.txt b/requirements.txt index b4f920702a..62c4c78b9b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -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 diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 8a8db0a3c1..389935b420 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -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 diff --git a/tests/app/main/forms/test_service_contact_details_form.py b/tests/app/main/forms/test_service_contact_details_form.py index bf9c97d65c..3c68ed719a 100644 --- a/tests/app/main/forms/test_service_contact_details_form.py +++ b/tests/app/main/forms/test_service_contact_details_form.py @@ -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 == {}