Merge pull request #927 from alphagov/persist-notification-intl

Persist international notification fields

app/models.py

@@ -670,7 +670,7 @@ class Notification(db.Model):
 
     international = db.Column(db.Boolean, nullable=False, default=False)
     phone_prefix = db.Column(db.String, nullable=True)
-    rate_multiplier = db.Column(db.Numeric(), nullable=True)
+    rate_multiplier = db.Column(db.Float(), nullable=True)
 
     @property
     def personalisation(self):
@@ -847,7 +847,7 @@ class NotificationHistory(db.Model, HistoryModel):
 
     international = db.Column(db.Boolean, nullable=False, default=False)
     phone_prefix = db.Column(db.String, nullable=True)
-    rate_multiplier = db.Column(db.Numeric(), nullable=True)
+    rate_multiplier = db.Column(db.Float(), nullable=True)
 
     @classmethod
     def from_original(cls, notification):

app/notifications/process_notifications.py

@@ -2,9 +2,13 @@ from datetime import datetime
 
 from flask import current_app
 
+from notifications_utils.recipients import (
+    get_international_phone_info,
+    validate_and_format_phone_number
+)
+
 from app import redis_store
 from app.celery import provider_tasks
-from notifications_utils.recipients import validate_and_format_phone_number
 from notifications_utils.clients import redis
 from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id
 from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE
@@ -25,22 +29,23 @@ def check_placeholders(template_object):
         raise BadRequestError(fields=[{'template': message}], message=message)
 
 
-def persist_notification(template_id,
-                         template_version,
-                         recipient,
-                         service,
-                         personalisation,
-                         notification_type,
-                         api_key_id,
-                         key_type,
-                         created_at=None,
-                         job_id=None,
-                         job_row_number=None,
-                         reference=None,
-                         client_reference=None,
-                         notification_id=None,
-                         simulated=False):
-    # if simulated create a Notification model to return but do not persist the Notification to the dB
+def persist_notification(
+    template_id,
+    template_version,
+    recipient,
+    service,
+    personalisation,
+    notification_type,
+    api_key_id,
+    key_type,
+    created_at=None,
+    job_id=None,
+    job_row_number=None,
+    reference=None,
+    client_reference=None,
+    notification_id=None,
+    simulated=False
+):
     notification = Notification(
         id=notification_id,
         template_id=template_id,
@@ -58,6 +63,15 @@ def persist_notification(template_id,
         client_reference=client_reference,
         reference=reference
     )
+
+    if notification_type == SMS_TYPE:
+        formatted_recipient = validate_and_format_phone_number(recipient, international=True)
+        recipient_info = get_international_phone_info(formatted_recipient)
+        notification.international = recipient_info.international
+        notification.phone_prefix = recipient_info.country_prefix
+        notification.rate_multiplier = recipient_info.billable_units
+
+    # if simulated create a Notification model to return but do not persist the Notification to the dB
     if not simulated:
         dao_create_notification(notification)
         if key_type != KEY_TYPE_TEST:

tests/app/notifications/test_process_notification.py

@@ -1,7 +1,7 @@
 import datetime
 import uuid
-
 import pytest
+
 from boto3.exceptions import Boto3Error
 from sqlalchemy.exc import SQLAlchemyError
 from freezegun import freeze_time
@@ -199,8 +199,8 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker)
     assert persisted_notification.client_reference == "ref from client"
     assert persisted_notification.reference is None
     assert persisted_notification.international is False
-    assert persisted_notification.phone_prefix is None
-    assert persisted_notification.rate_multiplier is None
+    assert persisted_notification.phone_prefix == '44'
+    assert persisted_notification.rate_multiplier == 1
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
@@ -297,3 +297,64 @@ def test_simulated_recipient(notify_api, to_address, notification_type, expected
     is_simulated_address = simulated_recipient(formatted_address, notification_type)
 
     assert is_simulated_address == expected
+
+
+@pytest.mark.parametrize('recipient, expected_international, expected_prefix, expected_units', [
+    ('7900900123', False, '44', 1),  # UK
+    ('+447900900123', False, '44', 1),  # UK
+    ('07700900222', False, '44', 1),  # UK
+    ('73122345678', True, '7', 1),  # Russia
+    ('360623400400', True, '36', 3)]  # Hungary
+)
+def test_persist_notification_with_international_info_stores_correct_info(
+    sample_job,
+    sample_api_key,
+    mocker,
+    recipient,
+    expected_international,
+    expected_prefix,
+    expected_units
+):
+    persist_notification(
+        template_id=sample_job.template.id,
+        template_version=sample_job.template.version,
+        recipient=recipient,
+        service=sample_job.service,
+        personalisation=None,
+        notification_type='sms',
+        api_key_id=sample_api_key.id,
+        key_type=sample_api_key.key_type,
+        job_id=sample_job.id,
+        job_row_number=10,
+        client_reference="ref from client"
+    )
+    persisted_notification = Notification.query.all()[0]
+
+    assert persisted_notification.international is expected_international
+    assert persisted_notification.phone_prefix == expected_prefix
+    assert persisted_notification.rate_multiplier == expected_units
+
+
+def test_persist_notification_with_international_info_does_not_store_for_email(
+    sample_job,
+    sample_api_key,
+    mocker
+):
+    persist_notification(
+        template_id=sample_job.template.id,
+        template_version=sample_job.template.version,
+        recipient='foo@bar.com',
+        service=sample_job.service,
+        personalisation=None,
+        notification_type='email',
+        api_key_id=sample_api_key.id,
+        key_type=sample_api_key.key_type,
+        job_id=sample_job.id,
+        job_row_number=10,
+        client_reference="ref from client"
+    )
+    persisted_notification = Notification.query.all()[0]
+
+    assert persisted_notification.international is False
+    assert persisted_notification.phone_prefix is None
+    assert persisted_notification.rate_multiplier is None

tests/app/user/test_rest_verify.py

@@ -230,7 +230,7 @@ def test_send_user_code_for_sms_with_optional_to_field(client,
     """
     Tests POST endpoint /user/<user_id>/sms-code with optional to field
     """
-    to_number = '+441119876757'
+    to_number = '+447119876757'
     mocked = mocker.patch('app.user.rest.create_secret_code', return_value='11111')
     mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
     auth_header = create_authorization_header()