diff --git a/app/celery/tasks.py b/app/celery/tasks.py index cf24a75d6..a2d1e7b03 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -12,7 +12,7 @@ from app import ( encryption ) from app.aws import s3 -from app.celery.provider_tasks import send_sms_to_provider, send_email_to_provider +from app.celery import provider_tasks from app.dao.jobs_dao import ( dao_update_job, dao_get_job_by_id @@ -81,7 +81,7 @@ def process_job(job_id): create_uuid(), encrypted, datetime.utcnow().strftime(DATETIME_FORMAT)), - queue='db-sms' + queue='db-sms' if not service.research_mode else 'research-mode' ) if template.template_type == EMAIL_TYPE: @@ -90,7 +90,8 @@ def process_job(job_id): create_uuid(), encrypted, datetime.utcnow().strftime(DATETIME_FORMAT)), - queue='db-email') + queue='db-email' if not service.research_mode else 'research-mode' + ) finished = datetime.utcnow() job.status = 'finished' @@ -126,7 +127,10 @@ def send_sms(self, created_at, notification, notification_id, service.id, SMS_TYPE, api_key_id, key_type ) ) - send_sms_to_provider.apply_async((service_id, notification_id), queue='send-sms') + provider_tasks.deliver_sms.apply_async( + [notification_id], + queue='send-sms' if not service.research_mode else 'research-mode' + ) current_app.logger.info( "SMS {} created at {}".format(notification_id, created_at) @@ -165,7 +169,10 @@ def send_email(self, service_id, ) ) - send_email_to_provider.apply_async((service_id, notification_id), queue='send-email') + provider_tasks.deliver_email.apply_async( + [notification_id], + queue='send-email' if not service.research_mode else 'research-mode' + ) current_app.logger.info("Email {} created at {}".format(notification_id, created_at)) except SQLAlchemyError as e: diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 93d16b0a2..2147a6984 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -44,7 +44,7 @@ from app.errors import ( ) register_errors(notifications) -from app.celery.provider_tasks import send_sms_to_provider, send_email_to_provider +from app.celery import provider_tasks @notifications.route('/notifications/email/ses', methods=['POST']) @@ -297,10 +297,17 @@ def persist_notification( ) try: + research_mode = service.research_mode or key_type == KEY_TYPE_TEST if notification_type == SMS_TYPE: - send_sms_to_provider.apply_async((str(service.id), str(notification_id)), queue='send-sms') + provider_tasks.deliver_sms.apply_async( + [str(notification_id)], + queue='send-sms' if not research_mode else 'research-mode' + ) if notification_type == EMAIL_TYPE: - send_email_to_provider.apply_async((str(service.id), str(notification_id)), queue='send-email') + provider_tasks.deliver_email.apply_async( + [str(notification_id)], + queue='send-email' if not research_mode else 'research-mode' + ) except Exception as e: current_app.logger.exception("Failed to send to SQS exception", e) dao_delete_notifications_and_history_by_id(notification_id) diff --git a/config.py b/config.py index ec10c8515..fa676c79c 100644 --- a/config.py +++ b/config.py @@ -117,7 +117,6 @@ class Config(object): CELERY_QUEUES = [ Queue('periodic', Exchange('default'), routing_key='periodic'), Queue('process-job', Exchange('default'), routing_key='process-job'), - Queue('research-mode', Exchange('default'), routing_key='research-mode'), Queue('retry', Exchange('default'), routing_key='retry'), Queue('notify', Exchange('default'), routing_key='notify') ] @@ -156,7 +155,8 @@ class Development(Config): Queue('db-sms', Exchange('default'), routing_key='db-sms'), Queue('send-sms', Exchange('default'), routing_key='send-sms'), Queue('db-email', Exchange('default'), routing_key='db-email'), - Queue('send-email', Exchange('default'), routing_key='send-email') + Queue('send-email', Exchange('default'), routing_key='send-email'), + Queue('research-mode', Exchange('default'), routing_key='research-mode') ] @@ -173,7 +173,8 @@ class Test(Config): Queue('db-sms', Exchange('default'), routing_key='db-sms'), Queue('send-sms', Exchange('default'), routing_key='send-sms'), Queue('db-email', Exchange('default'), routing_key='db-email'), - Queue('send-email', Exchange('default'), routing_key='send-email') + Queue('send-email', Exchange('default'), routing_key='send-email'), + Queue('research-mode', Exchange('default'), routing_key='research-mode') ] diff --git a/scripts/aws_start_app.sh b/scripts/aws_start_app.sh index 9e871ebc2..0c48d4859 100755 --- a/scripts/aws_start_app.sh +++ b/scripts/aws_start_app.sh @@ -15,5 +15,6 @@ function start start "notifications-api" start "notifications-api-celery-worker" start "notifications-api-celery-worker-sender" +start "notifications-api-celery-worker-research" start "notifications-api-celery-worker-db" start "notifications-api-celery-beat" diff --git a/scripts/aws_stop_app.sh b/scripts/aws_stop_app.sh index f3c3a441a..4ccc4ebc9 100755 --- a/scripts/aws_stop_app.sh +++ b/scripts/aws_stop_app.sh @@ -19,4 +19,5 @@ stop "notifications-api" stop "notifications-api-celery-beat" stop "notifications-api-celery-worker" stop "notifications-api-celery-worker-sender" +stop "notifications-api-celery-worker-research" stop "notifications-api-celery-worker-db" diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index a2af75d36..7ab7a9c20 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -8,8 +8,8 @@ import app def test_should_have_decorated_tasks_functions(): - assert send_sms_to_provider.__wrapped__.__name__ == 'send_sms_to_provider' - assert send_email_to_provider.__wrapped__.__name__ == 'send_email_to_provider' + assert deliver_sms.__wrapped__.__name__ == 'deliver_sms' + assert deliver_email.__wrapped__.__name__ == 'deliver_email' def test_should_by_10_second_delay_as_default(): diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index f6a039698..59f2dece8 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1,10 +1,8 @@ import uuid -import itertools import pytest from datetime import datetime from freezegun import freeze_time -from unittest.mock import ANY from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.orm.exc import NoResultFound @@ -17,7 +15,7 @@ from app.celery.tasks import ( process_job, send_email ) -from app.dao import jobs_dao +from app.dao import jobs_dao, services_dao from app.models import Notification, KEY_TYPE_TEAM, KEY_TYPE_TEST, KEY_TYPE_NORMAL from tests.app import load_example_csv from tests.app.conftest import ( @@ -88,6 +86,61 @@ def test_should_process_sms_job(sample_job, mocker): assert job.status == 'finished' +@freeze_time("2016-01-01 11:09:00.061258") +def test_should_process_sms_job_into_research_mode_queue_if_research_mode_service(notify_db, notify_db_session, mocker): + mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('sms')) + mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") + + service = sample_service(notify_db, notify_db_session) + service.research_mode = True + services_dao.dao_update_service(service) + job = sample_job(notify_db, notify_db_session, service=service) + + process_job(job.id) + s3.get_job_from_s3.assert_called_once_with( + str(job.service.id), + str(job.id) + ) + tasks.send_sms.apply_async.assert_called_once_with( + (str(job.service_id), + "uuid", + "something_encrypted", + "2016-01-01T11:09:00.061258"), + queue="research-mode" + ) + + +@freeze_time("2016-01-01 11:09:00.061258") +def test_should_process_email_job_into_research_mode_queue_if_research_mode_service( + notify_db, notify_db_session, mocker +): + mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('sms')) + mocker.patch('app.celery.tasks.send_email.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") + + service = sample_service(notify_db, notify_db_session) + service.research_mode = True + services_dao.dao_update_service(service) + template = sample_email_template(notify_db, notify_db_session, service=service) + job = sample_job(notify_db, notify_db_session, template=template, service=service) + + process_job(job.id) + s3.get_job_from_s3.assert_called_once_with( + str(job.service.id), + str(job.id) + ) + tasks.send_email.apply_async.assert_called_once_with( + (str(job.service_id), + "uuid", + "something_encrypted", + "2016-01-01T11:09:00.061258"), + queue="research-mode" + ) + + @freeze_time("2016-01-01 11:09:00.061258") def test_should_not_process_sms_job_if_would_exceed_send_limits(notify_db, notify_db_session, @@ -272,7 +325,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi notification = _notification_json(sample_template_with_placeholders, to="+447234123123", personalisation={"name": "Jo"}) - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') notification_id = uuid.uuid4() @@ -283,9 +336,8 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi datetime.utcnow().strftime(DATETIME_FORMAT) ) - provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( - (sample_template_with_placeholders.service_id, - notification_id), + provider_tasks.deliver_sms.apply_async.assert_called_once_with( + [notification_id], queue="send-sms" ) @@ -304,13 +356,39 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi assert persisted_notification.notification_type == 'sms' +def test_should_put_send_sms_task_in_research_mode_queue_if_research_mode_service(notify_db, notify_db_session, mocker): + service = sample_service(notify_db, notify_db_session) + service.research_mode = True + services_dao.dao_update_service(service) + + template = sample_template(notify_db, notify_db_session, service=service) + + notification = _notification_json(template, to="+447234123123") + + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + + notification_id = uuid.uuid4() + + send_sms( + template.service_id, + notification_id, + encryption.encrypt(notification), + datetime.utcnow().strftime(DATETIME_FORMAT) + ) + + provider_tasks.deliver_sms.apply_async.assert_called_once_with( + [notification_id], + queue="research-mode" + ) + + def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notify_db_session, mocker): user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900890") 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, "+447700900890") # The user’s own number, but in a different format - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') notification_id = uuid.uuid4() encrypt_notification = encryption.encrypt(notification) @@ -321,9 +399,8 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif datetime.utcnow().strftime(DATETIME_FORMAT) ) - provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( - (service.id, - notification_id), + provider_tasks.deliver_sms.apply_async.assert_called_once_with( + [notification_id], queue="send-sms" ) @@ -349,7 +426,7 @@ def test_should_not_send_sms_if_restricted_service_and_invalid_number_with_test_ 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') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') notification_id = uuid.uuid4() send_sms( @@ -360,9 +437,8 @@ def test_should_not_send_sms_if_restricted_service_and_invalid_number_with_test_ key_type=KEY_TYPE_TEST ) - provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( - (service.id, - notification_id), + provider_tasks.deliver_sms.apply_async.assert_called_once_with( + [notification_id], queue="send-sms" ) @@ -380,7 +456,7 @@ def test_should_not_send_email_if_restricted_service_and_invalid_email_address_w ) notification = _notification_json(template, to="test@example.com") - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') notification_id = uuid.uuid4() send_email( @@ -391,9 +467,8 @@ def test_should_not_send_email_if_restricted_service_and_invalid_email_address_w key_type=KEY_TYPE_TEST ) - provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( - (service.id, - notification_id), + provider_tasks.deliver_email.apply_async.assert_called_once_with( + [notification_id], queue="send-email" ) @@ -407,7 +482,7 @@ def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, 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') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') notification_id = uuid.uuid4() send_sms( @@ -416,7 +491,7 @@ def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, encryption.encrypt(notification), datetime.utcnow().strftime(DATETIME_FORMAT) ) - provider_tasks.send_sms_to_provider.apply_async.assert_not_called() + provider_tasks.deliver_sms.apply_async.assert_not_called() with pytest.raises(NoResultFound): Notification.query.filter_by(id=notification_id).one() @@ -441,13 +516,41 @@ def test_should_not_send_email_if_restricted_service_and_invalid_email_address(n Notification.query.filter_by(id=notification_id).one() +def test_should_put_send_email_task_in_research_mode_queue_if_research_mode_service( + notify_db, notify_db_session, mocker +): + service = sample_service(notify_db, notify_db_session) + service.research_mode = True + services_dao.dao_update_service(service) + + template = sample_email_template(notify_db, notify_db_session, service=service) + + notification = _notification_json(template, to="test@test.com") + + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + + notification_id = uuid.uuid4() + + send_email( + template.service_id, + notification_id, + encryption.encrypt(notification), + datetime.utcnow().strftime(DATETIME_FORMAT) + ) + + provider_tasks.deliver_email.apply_async.assert_called_once_with( + [notification_id], + queue="research-mode" + ) + + def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_api_key, mocker): notification = _notification_json( sample_job.template, to="+447234123123", job_id=sample_job.id, row_number=2) - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') notification_id = uuid.uuid4() send_sms( @@ -458,10 +561,8 @@ def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_ api_key_id=str(sample_api_key.id), key_type=KEY_TYPE_NORMAL ) - - provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( - (sample_job.service.id, - notification_id), + provider_tasks.deliver_sms.apply_async.assert_called_once_with( + [notification_id], queue="send-sms" ) persisted_notification = Notification.query.filter_by(id=notification_id).one() @@ -487,13 +588,12 @@ def test_should_not_send_email_if_team_key_and_recipient_not_in_team(sample_emai "my_email@my_email.com", {"name": "Jo"}, row_number=1) + apply_async = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') notification_id = uuid.uuid4() team_members = [user.email_address for user in sample_email_template_with_placeholders.service.users] assert "my_email@my_email.com" not in team_members - apply_async = mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') - with freeze_time("2016-01-01 11:09:00.00000"): now = datetime.utcnow() @@ -542,7 +642,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh 'my_email@my_email.com', {"name": "Jo"}, row_number=1) - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') notification_id = uuid.uuid4() @@ -560,8 +660,8 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh ) persisted_notification = Notification.query.filter_by(id=notification_id).one() - provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( - (sample_email_template_with_placeholders.service_id, notification_id), queue='send-email') + provider_tasks.deliver_email.apply_async.assert_called_once_with( + [notification_id], queue='send-email') assert persisted_notification.id == notification_id assert persisted_notification.to == 'my_email@my_email.com' @@ -585,7 +685,7 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email # Change the template from app.dao.templates_dao import dao_update_template, dao_get_template_by_id sample_email_template.content = sample_email_template.content + " another version of the template" - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') dao_update_template(sample_email_template) t = dao_get_template_by_id(sample_email_template.id) assert t.version > version_on_notification @@ -598,8 +698,7 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email now.strftime(DATETIME_FORMAT) ) - provider_tasks.send_email_to_provider.apply_async.assert_called_once_with((sample_email_template.service_id, - notification_id), queue='send-email') + provider_tasks.deliver_email.apply_async.assert_called_once_with([notification_id], queue='send-email') persisted_notification = Notification.query.filter_by(id=notification_id).one() assert persisted_notification.id == notification_id @@ -616,7 +715,7 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email def test_should_use_email_template_subject_placeholders(sample_email_template_with_placeholders, mocker): notification = _notification_json(sample_email_template_with_placeholders, "my_email@my_email.com", {"name": "Jo"}) - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') notification_id = uuid.uuid4() now = datetime.utcnow() @@ -626,8 +725,8 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) - provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( - (sample_email_template_with_placeholders.service_id, notification_id, ), queue='send-email' + provider_tasks.deliver_email.apply_async.assert_called_once_with( + [notification_id], queue='send-email' ) persisted_notification = Notification.query.filter_by(id=notification_id).one() assert persisted_notification.id == notification_id @@ -642,7 +741,7 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi def test_should_use_email_template_and_persist_without_personalisation(sample_email_template, mocker): notification = _notification_json(sample_email_template, "my_email@my_email.com") - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') notification_id = uuid.uuid4() @@ -653,8 +752,7 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) - provider_tasks.send_email_to_provider.apply_async.assert_called_once_with((sample_email_template.service_id, - notification_id), queue='send-email') + provider_tasks.deliver_email.apply_async.assert_called_once_with([notification_id], queue='send-email') persisted_notification = Notification.query.filter_by(id=notification_id).one() assert persisted_notification.id == notification_id @@ -674,7 +772,7 @@ def test_send_sms_should_go_to_retry_queue_if_database_errors(sample_template, m expected_exception = SQLAlchemyError() - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') mocker.patch('app.celery.tasks.send_sms.retry', side_effect=Exception()) mocker.patch('app.celery.tasks.dao_create_notification', side_effect=expected_exception) now = datetime.utcnow() @@ -688,7 +786,7 @@ def test_send_sms_should_go_to_retry_queue_if_database_errors(sample_template, m encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) - provider_tasks.send_sms_to_provider.apply_async.assert_not_called() + provider_tasks.deliver_sms.apply_async.assert_not_called() tasks.send_sms.retry.assert_called_with(exc=expected_exception, queue='retry') with pytest.raises(NoResultFound) as e: @@ -701,7 +799,7 @@ def test_send_email_should_go_to_retry_queue_if_database_errors(sample_email_tem expected_exception = SQLAlchemyError() - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') mocker.patch('app.celery.tasks.send_email.retry', side_effect=Exception()) mocker.patch('app.celery.tasks.dao_create_notification', side_effect=expected_exception) now = datetime.utcnow() @@ -715,7 +813,7 @@ def test_send_email_should_go_to_retry_queue_if_database_errors(sample_email_tem encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) - provider_tasks.send_email_to_provider.apply_async.assert_not_called() + provider_tasks.deliver_email.apply_async.assert_not_called() tasks.send_email.retry.assert_called_with(exc=expected_exception, queue='retry') with pytest.raises(NoResultFound) as e: diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 836cc12ab..71e030604 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -33,7 +33,10 @@ def test_create_notification_should_reject_if_missing_required_fields(notify_api sample_api_key, mocker, template_type): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + if template_type == 'sms': + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + else: + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = {} auth_header = create_authorization_header(service_id=sample_api_key.service_id) @@ -44,7 +47,10 @@ def test_create_notification_should_reject_if_missing_required_fields(notify_api headers=[('Content-Type', 'application/json'), auth_header]) json_resp = json.loads(response.get_data(as_text=True)) - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called() + if template_type == 'sms': + app.celery.provider_tasks.deliver_sms.apply_async.assert_not_called() + else: + app.celery.provider_tasks.deliver_email.apply_async.assert_not_called() assert json_resp['result'] == 'error' assert 'Missing data for required field.' in json_resp['message']['to'][0] assert 'Missing data for required field.' in json_resp['message']['template'][0] @@ -54,7 +60,7 @@ def test_create_notification_should_reject_if_missing_required_fields(notify_api def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') data = { 'to': 'invalid', @@ -68,7 +74,7 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker): headers=[('Content-Type', 'application/json'), auth_header]) json_resp = json.loads(response.get_data(as_text=True)) - app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called() + app.celery.provider_tasks.deliver_sms.apply_async.assert_not_called() assert json_resp['result'] == 'error' assert len(json_resp['message'].keys()) == 1 assert 'Invalid phone number: Must not contain letters or symbols' in json_resp['message']['to'] @@ -81,7 +87,10 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker): def test_send_notification_invalid_template_id(notify_api, sample_template, mocker, fake_uuid, template_type, to): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + if template_type == 'sms': + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + else: + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = { 'to': to, @@ -95,8 +104,10 @@ def test_send_notification_invalid_template_id(notify_api, sample_template, mock headers=[('Content-Type', 'application/json'), auth_header]) json_resp = json.loads(response.get_data(as_text=True)) - app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called() - + if template_type == 'sms': + app.celery.provider_tasks.deliver_sms.apply_async.assert_not_called() + else: + app.celery.provider_tasks.deliver_email.apply_async.assert_not_called() assert response.status_code == 404 test_string = 'No result found' assert test_string in json_resp['message'] @@ -106,7 +117,7 @@ def test_send_notification_invalid_template_id(notify_api, sample_template, mock def test_send_notification_with_placeholders_replaced(notify_api, sample_email_template_with_placeholders, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = { 'to': 'ok@ok.com', @@ -126,9 +137,8 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_email_t notification_id = response_data['notification']['id'] data.update({"template_version": sample_email_template_with_placeholders.version}) - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( - (str(sample_email_template_with_placeholders.service.id), - str(notification_id)), + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( + [notification_id], queue="send-email" ) assert response.status_code == 201 @@ -167,7 +177,10 @@ def test_should_not_send_notification_if_restricted_and_not_a_service_user(notif to): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + if template_type == 'sms': + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + else: + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') template = sample_template if template_type == 'sms' else sample_email_template template.service.restricted = True dao_update_service(template.service) @@ -184,7 +197,10 @@ def test_should_not_send_notification_if_restricted_and_not_a_service_user(notif headers=[('Content-Type', 'application/json'), auth_header]) json_resp = json.loads(response.get_data(as_text=True)) - app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called() + if template_type == 'sms': + app.celery.provider_tasks.deliver_sms.apply_async.assert_not_called() + else: + app.celery.provider_tasks.deliver_email.apply_async.assert_not_called() assert response.status_code == 400 assert [( @@ -203,9 +219,9 @@ def test_should_send_notification_if_restricted_and_a_service_user(notify_api, with notify_api.test_request_context(): with notify_api.test_client() as client: if template_type == 'sms': - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') else: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') template = sample_template if template_type == 'sms' else sample_email_template to = template.service.created_by.mobile_number if template_type == 'sms' \ @@ -225,9 +241,9 @@ def test_should_send_notification_if_restricted_and_a_service_user(notify_api, headers=[('Content-Type', 'application/json'), auth_header]) if template_type == 'sms': - assert app.celery.provider_tasks.send_sms_to_provider.apply_async.called + assert app.celery.provider_tasks.deliver_sms.apply_async.called else: - assert app.celery.provider_tasks.send_email_to_provider.apply_async.called + assert app.celery.provider_tasks.deliver_email.apply_async.called assert response.status_code == 201 @@ -241,9 +257,9 @@ def test_should_not_allow_template_from_another_service(notify_api, with notify_api.test_request_context(): with notify_api.test_client() as client: if template_type == 'sms': - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') else: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') service_1 = service_factory.get('service 1', user=sample_user, email_from='service.1') service_2 = service_factory.get('service 2', user=sample_user, email_from='service.2') @@ -263,9 +279,9 @@ def test_should_not_allow_template_from_another_service(notify_api, json_resp = json.loads(response.get_data(as_text=True)) if template_type == 'sms': - app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called() + app.celery.provider_tasks.deliver_sms.apply_async.assert_not_called() else: - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called() + app.celery.provider_tasks.deliver_email.apply_async.assert_not_called() assert response.status_code == 404 test_string = 'No result found' assert test_string in json_resp['message'] @@ -275,7 +291,7 @@ def test_should_not_allow_template_from_another_service(notify_api, def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = { @@ -293,8 +309,8 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker response_data = json.loads(response.data)['data'] notification_id = response_data['notification']['id'] - app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( - (str(sample_template.service_id), notification_id), queue='send-sms') + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( + [notification_id], queue='send-sms') assert response.status_code == 201 assert notification_id assert 'subject' not in response_data @@ -305,7 +321,7 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker def test_should_reject_email_notification_with_bad_email(notify_api, sample_email_template, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') to_address = "bad-email" data = { 'to': to_address, @@ -319,7 +335,7 @@ def test_should_reject_email_notification_with_bad_email(notify_api, sample_emai headers=[('Content-Type', 'application/json'), auth_header]) data = json.loads(response.get_data(as_text=True)) - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called() + app.celery.provider_tasks.deliver_email.apply_async.assert_not_called() assert response.status_code == 400 assert data['result'] == 'error' assert data['message']['to'][0] == 'Not a valid email address' @@ -329,7 +345,7 @@ def test_should_reject_email_notification_with_bad_email(notify_api, sample_emai def test_should_allow_valid_email_notification(notify_api, sample_email_template, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = { @@ -346,8 +362,8 @@ def test_should_allow_valid_email_notification(notify_api, sample_email_template assert response.status_code == 201 response_data = json.loads(response.get_data(as_text=True))['data'] notification_id = response_data['notification']['id'] - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( - (str(sample_email_template.service_id), notification_id), + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( + [notification_id], queue="send-email" ) @@ -366,7 +382,7 @@ def test_should_not_block_api_call_if_over_day_limit_for_live_service( mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=False) @@ -399,7 +415,7 @@ def test_should_block_api_call_if_over_day_limit_for_restricted_service( mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") service = create_sample_service(notify_db, notify_db_session, limit=1, restricted=True) @@ -435,7 +451,7 @@ def test_should_allow_api_call_if_under_day_limit_regardless_of_type( restricted): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") service = create_sample_service(notify_db, notify_db_session, limit=2, restricted=restricted) @@ -461,7 +477,7 @@ def test_should_allow_api_call_if_under_day_limit_regardless_of_type( def test_should_not_return_html_in_body(notify_api, notify_db, notify_db_session, mocker): with notify_api.test_request_context(): with notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') email_template = create_sample_email_template(notify_db, notify_db.session, content='hello\nthere') data = { @@ -481,7 +497,7 @@ def test_should_not_return_html_in_body(notify_api, notify_db, notify_db_session def test_should_not_send_email_if_team_api_key_and_not_a_service_user(notify_api, sample_email_template, mocker): with notify_api.test_request_context(), notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = { 'to': "not-someone-we-trust@email-address.com", 'template': str(sample_email_template.id), @@ -496,7 +512,7 @@ def test_should_not_send_email_if_team_api_key_and_not_a_service_user(notify_api json_resp = json.loads(response.get_data(as_text=True)) - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_not_called() + app.celery.provider_tasks.deliver_email.apply_async.assert_not_called() assert response.status_code == 400 assert [ @@ -506,7 +522,7 @@ def test_should_not_send_email_if_team_api_key_and_not_a_service_user(notify_api def test_should_not_send_sms_if_team_api_key_and_not_a_service_user(notify_api, sample_template, mocker): with notify_api.test_request_context(), notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') data = { 'to': '07123123123', @@ -521,7 +537,7 @@ def test_should_not_send_sms_if_team_api_key_and_not_a_service_user(notify_api, headers=[('Content-Type', 'application/json'), auth_header]) json_resp = json.loads(response.get_data(as_text=True)) - app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_not_called() + app.celery.provider_tasks.deliver_sms.apply_async.assert_not_called() assert response.status_code == 400 assert [ @@ -531,7 +547,7 @@ def test_should_not_send_sms_if_team_api_key_and_not_a_service_user(notify_api, def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample_email_template, fake_uuid, mocker): with notify_api.test_request_context(), notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) data = { @@ -550,9 +566,40 @@ def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample data=json.dumps(data), headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( - (str(sample_email_template.service.id), fake_uuid), - queue='send-email') + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with([fake_uuid], queue='send-email') + assert response.status_code == 201 + + +@pytest.mark.parametrize('restricted', [True, False]) +@pytest.mark.parametrize('limit', [0, 1]) +def test_should_send_sms_to_anyone_with_test_key( + notify_api, sample_template, mocker, restricted, limit, fake_uuid +): + with notify_api.test_request_context(), notify_api.test_client() as client: + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) + + data = { + 'to': '07811111111', + 'template': sample_template.id + } + sample_template.service.restricted = restricted + sample_template.service.message_limit = limit + api_key = ApiKey( + service=sample_template.service, + name='test_key', + created_by=sample_template.created_by, + key_type=KEY_TYPE_TEST + ) + save_model_api_key(api_key) + auth_header = create_jwt_token(secret=api_key.unsigned_secret, client_id=str(api_key.service_id)) + + response = client.post( + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] + ) + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='research-mode') assert response.status_code == 201 @@ -562,7 +609,7 @@ def test_should_send_email_to_anyone_with_test_key( notify_api, sample_email_template, mocker, restricted, limit, fake_uuid ): with notify_api.test_request_context(), notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) data = { @@ -586,14 +633,13 @@ def test_should_send_email_to_anyone_with_test_key( headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] ) - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( - (str(sample_email_template.service.id), fake_uuid), queue='send-email') + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with([fake_uuid], queue='research-mode') assert response.status_code == 201 def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_template, fake_uuid, mocker): with notify_api.test_request_context(), notify_api.test_client() as client: - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) data = { @@ -612,8 +658,7 @@ def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_t data=json.dumps(data), headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) - app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( - (str(sample_template.service.id), fake_uuid), queue='send-sms') + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='send-sms') assert response.status_code == 201 @@ -625,9 +670,9 @@ def test_should_persist_notification(notify_api, sample_template, fake_uuid, mocker): with notify_api.test_request_context(), notify_api.test_client() as client: if template_type == 'sms': - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') else: - mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) template = sample_template if template_type == 'sms' else sample_email_template to = sample_template.service.created_by.mobile_number if template_type == 'sms' \ @@ -650,11 +695,9 @@ def test_should_persist_notification(notify_api, sample_template, headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) if template_type == 'sms': - app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( - (str(template.service.id), fake_uuid), queue='send-sms') + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='send-sms') else: - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( - (str(template.service.id), fake_uuid), queue='send-email') + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with([fake_uuid], queue='send-email') assert response.status_code == 201 notification = notifications_dao.get_notification_by_id(fake_uuid) @@ -675,12 +718,12 @@ def test_should_delete_notification_and_return_error_if_sqs_fails( with notify_api.test_request_context(), notify_api.test_client() as client: if template_type == 'email': mocker.patch( - 'app.celery.provider_tasks.send_email_to_provider.apply_async', + 'app.celery.provider_tasks.deliver_email.apply_async', side_effect=Exception("failed to talk to SQS") ) else: mocker.patch( - 'app.celery.provider_tasks.send_sms_to_provider.apply_async', + 'app.celery.provider_tasks.deliver_sms.apply_async', side_effect=Exception("failed to talk to SQS") ) mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid) @@ -705,11 +748,9 @@ def test_should_delete_notification_and_return_error_if_sqs_fails( headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) if template_type == 'email': - app.celery.provider_tasks.send_email_to_provider.apply_async.assert_called_once_with( - (str(template.service.id), fake_uuid), queue='send-email') + app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with([fake_uuid], queue='send-email') else: - app.celery.provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( - (str(template.service.id), fake_uuid), queue='send-sms') + app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with([fake_uuid], queue='send-sms') assert response.status_code == 500 assert not notifications_dao.get_notification_by_id(fake_uuid) @@ -726,7 +767,7 @@ def test_should_not_persist_notification_or_send_email_if_simulated_email( to_email, sample_email_template, mocker): - apply_async = mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + apply_async = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = { 'to': to_email, @@ -755,7 +796,7 @@ def test_should_not_persist_notification_or_send_sms_if_simulated_number( to_sms, sample_template, mocker): - apply_async = mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + apply_async = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') data = { 'to': to_sms, @@ -790,11 +831,11 @@ def test_should_not_send_notification_to_non_whitelist_recipient_in_trial_mode(c service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session, service=service) if notification_type == 'sms': - apply_async = mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + apply_async = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') template = create_sample_template(notify_db, notify_db_session, service=service) elif notification_type == 'email': - apply_async = mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + apply_async = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') template = create_sample_email_template(notify_db, notify_db_session, service=service) assert service_whitelist.service_id == service.id @@ -838,12 +879,12 @@ def test_should_send_notification_to_whitelist_recipient_in_trial_mode_with_live mocker): service = create_sample_service(notify_db, notify_db_session, limit=2, restricted=True) if notification_type == 'sms': - apply_async = mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') + apply_async = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') template = create_sample_template(notify_db, notify_db_session, service=service) service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session, service=service, mobile_number=to) elif notification_type == 'email': - apply_async = mocker.patch('app.celery.provider_tasks.send_email_to_provider.apply_async') + apply_async = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') template = create_sample_email_template(notify_db, notify_db_session, service=service) service_whitelist = create_sample_service_whitelist(notify_db, notify_db_session, service=service, email_address=to) diff --git a/tests/app/public_contracts/test_POST_notification.py b/tests/app/public_contracts/test_POST_notification.py index 734a4cefe..fe2468191 100644 --- a/tests/app/public_contracts/test_POST_notification.py +++ b/tests/app/public_contracts/test_POST_notification.py @@ -5,7 +5,7 @@ from tests import create_authorization_header def test_post_sms_contract(client, mocker, sample_template): - mocker.patch('app.celery.tasks.send_sms_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = { @@ -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_email_to_provider.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") data = {