From 840b99b277b28b28b70d5565980ff583e3be69a0 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Wed, 7 Sep 2016 09:57:20 +0100 Subject: [PATCH 1/3] Fix issue where test key cannot send messages to external numbers --- app/celery/tasks.py | 13 ++++--- tests/app/celery/test_tasks.py | 62 +++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index a03df1878..0726bb220 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -28,7 +28,8 @@ from app.models import ( Notification, EMAIL_TYPE, SMS_TYPE, - KEY_TYPE_NORMAL + KEY_TYPE_NORMAL, + KEY_TYPE_TEST ) from app.statsd_decorators import statsd @@ -126,7 +127,7 @@ def send_sms(self, notification = encryption.decrypt(encrypted_notification) service = dao_fetch_service_by_id(service_id) - if not service_allowed_to_send_to(notification['to'], service): + if not service_allowed_to_send_to(notification['to'], service, key_type): current_app.logger.info( "SMS {} failed as restricted service".format(notification_id) ) @@ -160,10 +161,12 @@ def send_email(self, service_id, created_at, api_key_id=None, key_type=KEY_TYPE_NORMAL): + + notification = encryption.decrypt(encrypted_notification) service = dao_fetch_service_by_id(service_id) - if not service_allowed_to_send_to(notification['to'], service): + if not service_allowed_to_send_to(notification['to'], service, key_type): current_app.logger.info("Email {} failed as restricted service".format(notification_id)) return @@ -203,8 +206,8 @@ def _save_notification(created_at, notification, notification_id, service_id, no dao_create_notification(notification_db_object) -def service_allowed_to_send_to(recipient, service): - if not service.restricted: +def service_allowed_to_send_to(recipient, service, key_type): + if not service.restricted or key_type == KEY_TYPE_TEST: return True return allowed_to_send_to( diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 3b2159c38..8426947a1 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -17,7 +17,7 @@ from app.celery.tasks import ( send_email ) from app.dao import jobs_dao -from app.models import Notification, KEY_TYPE_TEAM +from app.models import Notification, KEY_TYPE_TEAM, KEY_TYPE_TEST from tests.app import load_example_csv from tests.app.conftest import ( sample_service, @@ -350,6 +350,66 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif assert persisted_notification.notification_type == 'sms' +def test_should_not_send_sms_if_restricted_service_and_invalid_number_with_test_key(notify_db, + notify_db_session, + mocker): + user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900205") + service = sample_service(notify_db, notify_db_session, user=user, restricted=True) + template = sample_template(notify_db, notify_db_session, service=service) + + notification = _notification_json(template, "07700 900849") + mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + + notification_id = uuid.uuid4() + send_sms( + service.id, + notification_id, + encryption.encrypt(notification), + datetime.utcnow().strftime(DATETIME_FORMAT), + key_type=KEY_TYPE_TEST + ) + + provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( + (service.id, + notification_id), + queue="send-sms" + ) + + persisted_notification = Notification.query.filter_by(id=notification_id).one() + assert persisted_notification.id == notification_id + + +def test_should_not_send_email_if_restricted_service_and_invalid_email_address_with_test_key(notify_db, + notify_db_session, + mocker): + user = sample_user(notify_db, notify_db_session) + service = sample_service(notify_db, notify_db_session, user=user, restricted=True) + template = sample_template( + notify_db, notify_db_session, service=service, template_type='email', subject_line='Hello' + ) + + notification = _notification_json(template, to="test@example.com") + mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + + notification_id = uuid.uuid4() + send_email( + service.id, + notification_id, + encryption.encrypt(notification), + datetime.utcnow().strftime(DATETIME_FORMAT), + key_type=KEY_TYPE_TEST + ) + + provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( + (service.id, + notification_id), + queue="send-email" + ) + + persisted_notification = Notification.query.filter_by(id=notification_id).one() + assert persisted_notification.id == notification_id + + def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker): user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900205") service = sample_service(notify_db, notify_db_session, user=user, restricted=True) From 94708bdee8b03f2a07739ea4d062a4dbd547f2f6 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Wed, 7 Sep 2016 11:03:16 +0100 Subject: [PATCH 2/3] Fix indentation issue --- app/celery/tasks.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 0726bb220..d655c28ce 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -161,8 +161,6 @@ def send_email(self, service_id, created_at, api_key_id=None, key_type=KEY_TYPE_NORMAL): - - notification = encryption.decrypt(encrypted_notification) service = dao_fetch_service_by_id(service_id) From 41681ac001b4f93902c3c894bf2d6862e8d435e5 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Wed, 7 Sep 2016 11:17:27 +0100 Subject: [PATCH 3/3] Fix mocker syntax issue --- tests/app/public_contracts/test_POST_notification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/public_contracts/test_POST_notification.py b/tests/app/public_contracts/test_POST_notification.py index ffb91210c..ce333cef3 100644 --- a/tests/app/public_contracts/test_POST_notification.py +++ b/tests/app/public_contracts/test_POST_notification.py @@ -25,7 +25,7 @@ def test_post_sms_contract(client, mocker, sample_template): def test_post_email_contract(client, mocker, sample_email_template): - mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.celery.tasks.send_email.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = {