Save normalized phone format to DB

Pipfile

@@ -59,7 +59,7 @@ werkzeug = "~=2.1.1"
 prometheus-client = "==0.14.1"
 gds-metrics = {version = "==0.2.4", ref = "6f1840a57b6fb1ee40b7e84f2f18ec229de8aa72", git = "https://github.com/alphagov/gds_metrics_python.git"}
 packaging = "==21.3"
-notifications-utils = {editable = true, ref = "validate-us-numbers", git = "https://github.com/GSA/notifications-utils.git"}
+notifications-utils = {editable = true, ref = "main", git = "https://github.com/GSA/notifications-utils.git"}
 
 [dev-packages]
 flake8 = "==4.0.1"

Pipfile.lock

@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "2f43f1aefdea61dea472c0f41aa81166c852769a8156d3e9644e5535c30a1c2a"
+            "sha256": "d0a90dd082bf51ae47e04412c0848f00bc5e4b538804d1333e9264ca0ad2d4e9"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -739,7 +739,7 @@
         "notifications-utils": {
             "editable": true,
             "git": "https://github.com/GSA/notifications-utils.git",
-            "ref": "d88dff885f5db43f6573ccb5878c376dea92ec2e"
+            "ref": "d88d48026171991de4d8398e5f9f8c2063297885"
         },
         "numpy": {
             "hashes": [
@@ -2080,11 +2080,11 @@
         },
         "rich": {
             "hashes": [
-                "sha256:12b1d77ee7edf251b741531323f0d990f5f570a4e7c054d0bfb59fb7981ad977",
-                "sha256:3aa9eba7219b8c575c6494446a59f702552efe1aa261e7eeb95548fa586e1950"
+                "sha256:25f83363f636995627a99f6e4abc52ed0970ebbd544960cc63cbb43aaac3d6f0",
+                "sha256:41fe1d05f433b0f4724cda8345219213d2bfa472ef56b2f64f415b5b94d51b04"
             ],
             "markers": "python_full_version >= '3.7.0'",
-            "version": "==13.0.0"
+            "version": "==13.0.1"
         },
         "s3transfer": {
             "hashes": [

app/models.py

@@ -36,6 +36,7 @@ from sqlalchemy.dialects.postgresql import JSON, JSONB, UUID
 from sqlalchemy.ext.associationproxy import association_proxy
 from sqlalchemy.ext.declarative import declared_attr
 from sqlalchemy.ext.hybrid import hybrid_property
+from sqlalchemy.orm import validates
 from sqlalchemy.orm.collections import attribute_mapped_collection
 
 from app import db, encryption
@@ -140,6 +141,14 @@ class User(db.Model):
         secondary='user_to_organisation',
         backref='users')
 
+    @validates("mobile_number")
+    def validate_mobile_number(self, key, number):
+        try:
+            if number is not None:
+                return validate_phone_number(number, international=True)
+        except InvalidPhoneError as err:
+            raise ValueError(str(err)) from err
+
     @property
     def password(self):
         raise AttributeError("Password not readable")
@@ -708,11 +717,9 @@ class ServiceGuestList(db.Model):
 
         try:
             if recipient_type == MOBILE_TYPE:
-                validate_phone_number(recipient, international=True)
-                instance.recipient = recipient
+                instance.recipient = validate_phone_number(recipient, international=True)
             elif recipient_type == EMAIL_TYPE:
-                validate_email_address(recipient)
-                instance.recipient = recipient
+                instance.recipient = validate_email_address(recipient)
             else:
                 raise ValueError('Invalid recipient type')
         except InvalidPhoneError:

docs/data-storage.md

@@ -28,6 +28,7 @@ No db data is PII, but each job has a csv file in s3 containing phone numbers an
 #### notifications
 
 * to
+* normalized_to
 * _personalization<sup>2</sup>
 * phone_prefix<sup>3</sup>
 

tests/app/celery/test_tasks.py

@@ -572,11 +572,11 @@ def test_save_sms_should_save_default_sms_sender_notification_reply_to_text_on(n
 
 
 def test_should_not_save_sms_if_restricted_service_and_invalid_number(notify_db_session, mocker):
-    user = create_user(mobile_number="07700 900205")
+    user = create_user(mobile_number="2028675309")
     service = create_service(user=user, restricted=True)
     template = create_template(service=service)
 
-    notification = _notification_json(template, "07700 900849")
+    notification = _notification_json(template, "2028675400")
     mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
 
     notification_id = uuid.uuid4()
@@ -667,14 +667,14 @@ def test_should_save_sms_template_to_and_persist_with_job_id(sample_job, mocker)
 
 def test_should_not_save_sms_if_team_key_and_recipient_not_in_team(notify_db_session, mocker):
     assert Notification.query.count() == 0
-    user = create_user(mobile_number="07700 900205")
+    user = create_user(mobile_number="2028675309")
     service = create_service(user=user, restricted=True)
     template = create_template(service=service)
 
     team_members = [user.mobile_number for user in service.users]
     assert "07890 300000" not in team_members
 
-    notification = _notification_json(template, "07700 900849")
+    notification = _notification_json(template, "2028675400")
     mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
 
     notification_id = uuid.uuid4()

tests/app/dao/test_services_dao.py

@@ -748,7 +748,7 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions(not
         name='Other Test User',
         email_address='other_user@digital.cabinet-office.gov.uk',
         password='password',
-        mobile_number='+447700900987'
+        mobile_number='+12028672000'
     )
     save_model_user(other_user, validated_email_access=True)
     service_two = Service(name="service_two",

tests/app/dao/test_users_dao.py

@@ -37,11 +37,11 @@ from tests.app.db import (
 
 
 @freeze_time('2020-01-28T12:00:00')
-@pytest.mark.parametrize('phone_number', [
-    '+12028675309',
-    '+1-800-555-5555',
+@pytest.mark.parametrize('phone_number, expected_phone_number', [
+    ('2028675309', '+12028675309'),
+    ('+1-800-555-5555', '+18005555555'),
 ])
-def test_create_user(notify_db_session, phone_number):
+def test_create_user(notify_db_session, phone_number, expected_phone_number):
     email = 'notify@digital.cabinet-office.gov.uk'
     data = {
         'name': 'Test User',
@@ -55,7 +55,7 @@ def test_create_user(notify_db_session, phone_number):
     user_query = User.query.first()
     assert user_query.email_address == email
     assert user_query.id == user.id
-    assert user_query.mobile_number == phone_number
+    assert user_query.mobile_number == expected_phone_number
     assert user_query.email_access_validated_at == datetime.utcnow()
     assert not user_query.platform_admin
 

tests/app/service/send_notification/test_send_notification.py

@@ -842,15 +842,16 @@ def test_should_not_send_notification_to_non_guest_list_recipient_in_trial_mode(
 @pytest.mark.parametrize('key_type', [
     KEY_TYPE_NORMAL, KEY_TYPE_TEAM
 ])
-@pytest.mark.parametrize('notification_type, to', [
-    (SMS_TYPE, '2028675300'),
-    (EMAIL_TYPE, 'guest_list_recipient@mail.com')]
+@pytest.mark.parametrize('notification_type, to, normalized_to', [
+    (SMS_TYPE, '2028675300', '+12028675300'),
+    (EMAIL_TYPE, 'guest_list_recipient@mail.com', None)]
 )
 def test_should_send_notification_to_guest_list_recipient(
     client,
     sample_service,
     notification_type,
     to,
+    normalized_to,
     key_type,
     service_restricted,
     mocker
@@ -866,7 +867,7 @@ def test_should_send_notification_to_guest_list_recipient(
         service_guest_list = create_service_guest_list(sample_service, email_address=to)
 
     assert service_guest_list.service_id == sample_service.id
-    assert to in [member.recipient for member in sample_service.guest_list]
+    assert (normalized_to or to) in [member.recipient for member in sample_service.guest_list]
 
     create_notification(template=template)
 

tests/app/service/test_service_guest_list.py

@@ -32,7 +32,7 @@ def test_get_guest_list_separates_emails_and_phones(client, sample_service):
     assert response.status_code == 200
     json_resp = json.loads(response.get_data(as_text=True))
     assert json_resp['email_addresses'] == ['service@example.com']
-    assert sorted(json_resp['phone_numbers']) == sorted(['+1800-555-5555', '2028675309'])
+    assert sorted(json_resp['phone_numbers']) == sorted(['+18005555555', '+12028675309'])
 
 
 def test_get_guest_list_404s_with_unknown_service_id(client):
@@ -69,7 +69,7 @@ def test_update_guest_list_replaces_old_guest_list(client, sample_service_guest_
     assert response.status_code == 204
     guest_list = ServiceGuestList.query.order_by(ServiceGuestList.recipient).all()
     assert len(guest_list) == 2
-    assert guest_list[0].recipient == '2028765309'
+    assert guest_list[0].recipient == '+12028765309'
     assert guest_list[1].recipient == 'foo@bar.com'
 
 

tests/app/test_model.py

@@ -32,7 +32,7 @@ from tests.app.db import (
 
 
 @pytest.mark.parametrize('mobile_number', [
-    '2348675309',
+    '+447700900855',
     '+12348675309'
 ])
 def test_should_build_service_guest_list_from_mobile_number(mobile_number):