From 252c3235d71a1c9e48c4a75af9b480c4ddfb3844 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 27 Apr 2017 12:41:10 +0100 Subject: [PATCH] Persist intl fields on notification --- app/notifications/process_notifications.py | 48 ++++++++----- .../test_process_notification.py | 67 ++++++++++++++++++- 2 files changed, 95 insertions(+), 20 deletions(-) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 033943511..e8510ba63 100644 --- a/app/notifications/process_notifications.py +++ b/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: diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 766e3f250..f697047e7 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/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