From 832375744185f35cdb7d739a49ccbaefa44ce6e6 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sun, 6 Mar 2016 13:21:46 +0000 Subject: [PATCH] Accept phone numbers in any valid format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This uses the `format_phone_number` method from utils to output phone numbers in a consistent format. It is added to the schemas, so will be applied before the API tries to do anything with a provided phone number. So now the API will accept any of the following: - 07123456789 - 07123 456789 - 07123-456-789 - 00447123456789 - 00 44 7123456789 - +447123456789 - +44 7123 456 789 - +44 (0)7123 456 789 …but the API will always hand off phone numbers to 3rd party APIs in the format - +447123456789 The test for this is slightly convoluted, because template IDs are still database IDs, and can’t consistently be mocked, therefore we have to ignore that part of the call to `encrypt()`. --- app/schemas.py | 13 ++++++++++++- tests/app/notifications/test_rest.py | 3 ++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 016cfadb7..b3aa0ac49 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -6,7 +6,11 @@ from . import models from app.dao.permissions_dao import permission_dao from marshmallow import (post_load, ValidationError, validates, validates_schema) from marshmallow_sqlalchemy import field_for -from utils.recipients import validate_email_address, InvalidEmailError, validate_phone_number, InvalidPhoneError +from utils.recipients import ( + validate_email_address, InvalidEmailError, + validate_phone_number, InvalidPhoneError, + format_phone_number +) # TODO I think marshmallow provides a better integration and error handling. @@ -130,6 +134,13 @@ class SmsNotificationSchema(NotificationSchema): except InvalidPhoneError as error: raise ValidationError('Invalid phone number: {}'.format(error)) + @post_load + def format_phone_number(self, item): + item['to'] = format_phone_number(validate_phone_number( + item['to']) + ) + return item + class EmailNotificationSchema(NotificationSchema): to = fields.Str(required=True) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 967a74d39..5d8ed0b73 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -504,7 +504,7 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = { - 'to': '+447700900855', + 'to': '07700 900 855', 'template': sample_template.id } @@ -521,6 +521,7 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker headers=[('Content-Type', 'application/json'), auth_header]) notification_id = json.loads(response.data)['notification_id'] + assert app.encryption.encrypt.call_args[0][0]['to'] == '+447700900855' app.celery.tasks.send_sms.apply_async.assert_called_once_with( (str(sample_template.service_id), notification_id,