From fcb5ca9ef48a857b953e9319feddc80de0fa5c7c Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 22 Jun 2016 13:32:27 +0100 Subject: [PATCH] The send_sms task will created the notification with a status = created. The encrypted_notification for the send_sms_to_provider task has been made optional. --- app/celery/provider_tasks.py | 5 +- app/dao/notifications_dao.py | 4 +- tests/app/celery/test_provider_tasks.py | 162 ++++---------------- tests/app/celery/test_tasks.py | 195 +++++++----------------- tests/app/notifications/test_rest.py | 5 +- 5 files changed, 83 insertions(+), 288 deletions(-) diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 071ba3065..ec2866e1d 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -55,12 +55,9 @@ def send_sms_to_provider(self, service_id, notification_id, encrypted_notificati provider = provider_to_use('sms', notification_id) notification = get_notification_by_id(notification_id) - # notification_json = encryption.decrypt(encrypted_notification) - template = Template( dao_get_template_by_id(notification.template_id, notification.template_version).__dict__, - values=notification.personalisation['personalisation'] if notification.personalisation[ - 'personalisation'] else {}, + values={} if not notification.personalisation else notification.personalisation, prefix=service.name ) try: diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index b0cef9dc4..febb13c7d 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -247,9 +247,7 @@ def _update_notification_status(notification, status, notification_statistics_st @transactional def update_notification_status_by_id(notification_id, status, notification_statistics_status=None): notification = Notification.query.filter( - Notification.id == notification_id, - or_(Notification.status == 'sending', - Notification.status == 'pending')).first() + Notification.id == notification_id).first() if not notification: return False diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 767b40413..45524a130 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -2,7 +2,7 @@ from celery.exceptions import MaxRetriesExceededError from mock import ANY, call -from app import statsd_client, mmg_client +from app import statsd_client, mmg_client, encryption from app.celery import provider_tasks from app.celery.provider_tasks import send_sms_to_provider from app.celery.tasks import provider_to_use @@ -84,93 +84,32 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( sample_template_with_placeholders, mocker): db_notification = sample_notification(notify_db, notify_db_session, template=sample_template_with_placeholders, - to_field="+447234123123", personalisation={"name": "Jo"}) + to_field="+447234123123", personalisation={"name": "Jo"}, + status='created') - notification = _notification_json( - sample_template_with_placeholders, - to="+447234123123", - personalisation={"name": "Jo"} - ) - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.get_name', return_value="mmg") mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing_with_dates') mocker.patch('app.statsd_client.timing') - freezer = freeze_time("2016-01-01 11:09:00.00000") - freezer.start() - now = datetime.utcnow() - freezer.stop() - - freezer = freeze_time("2016-01-01 11:10:00.00000") - freezer.start() - send_sms_to_provider( db_notification.service_id, - db_notification.id, - "encrypted-in-reality" + db_notification.id ) - freezer.stop() mmg_client.send_sms.assert_called_once_with( to=format_phone_number(validate_phone_number("+447234123123")), content="Sample service: Hello Jo", reference=str(db_notification.id) ) - - statsd_client.incr.assert_called_once_with("notifications.tasks.send-sms-to-provider") - statsd_client.timing.assert_has_calls([ - call("notifications.tasks.send-sms-to-provider.task-time", ANY), - call("notifications.sms.total-time", ANY) - ]) - notification = Notification.query.filter_by(id=db_notification.id).one() assert notification.status == 'sending' - assert notification.sent_at > now + assert notification.sent_at <= datetime.utcnow() assert notification.sent_by == 'mmg' assert notification.content_char_count == 24 - - -def test_should_send_template_to_correct_sms_provider_and_persist( - notify_db, - notify_db_session, - sample_template, - mocker): - db_notification = sample_notification(notify_db, notify_db_session, template=sample_template) - - notification = _notification_json( - sample_template, - to="+447234123123" - ) - mocker.patch('app.encryption.decrypt', return_value=notification) - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - mocker.patch('app.statsd_client.incr') - mocker.patch('app.statsd_client.timing_with_dates') - mocker.patch('app.statsd_client.timing') - - freezer = freeze_time("2016-01-01 11:09:00.00000") - freezer.start() - now = datetime.utcnow() - freezer.stop() - - freezer = freeze_time("2016-01-01 11:10:00.00000") - freezer.start() - - send_sms_to_provider( - db_notification.service_id, - db_notification.id, - "encrypted-in-reality" - ) - freezer.stop() - - mmg_client.send_sms.assert_called_once_with( - to=format_phone_number(validate_phone_number("+447234123123")), - content="Sample service: This is a template", - reference=str(db_notification.id) - ) + assert notification.personalisation == {"name": "Jo"} def test_send_sms_should_use_template_version_from_notification_not_latest( @@ -178,10 +117,9 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( notify_db_session, sample_template, mocker): - db_notification = sample_notification(notify_db, notify_db_session, template=sample_template) + db_notification = sample_notification(notify_db, notify_db_session, + template=sample_template, to_field='+447234123123') - notification = _notification_json(sample_template, '+447234123123') - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.get_name', return_value="mmg") version_on_notification = sample_template.version @@ -193,12 +131,9 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( t = dao_get_template_by_id(sample_template.id) assert t.version > version_on_notification - now = datetime.utcnow() - send_sms_to_provider( db_notification.service_id, - db_notification.id, - "encrypted-in-reality" + db_notification.id ) mmg_client.send_sms.assert_called_once_with( @@ -212,18 +147,12 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( assert persisted_notification.template_id == sample_template.id assert persisted_notification.template_version == version_on_notification assert persisted_notification.template_version != sample_template.version - assert persisted_notification.sent_at > now - assert persisted_notification.status == 'sending' - assert persisted_notification.sent_by == 'mmg' assert persisted_notification.content_char_count == len("Sample service: This is a template") + assert not persisted_notification.personalisation def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_service, sample_notification, mocker): - notification = _notification_json( - sample_notification.template, - to=sample_notification.to - ) - mocker.patch('app.encryption.decrypt', return_value=notification) + mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.get_name', return_value="mmg") mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') @@ -232,11 +161,9 @@ def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_s notify_db.session.add(sample_service) notify_db.session.commit() - now = datetime.utcnow() send_sms_to_provider( sample_notification.service_id, - sample_notification.id, - "encrypted-in-reality" + sample_notification.id ) assert not mmg_client.send_sms.called send_sms_response.apply_async.assert_called_once_with( @@ -247,27 +174,22 @@ def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_s assert persisted_notification.to == sample_notification.to assert persisted_notification.template_id == sample_notification.template_id assert persisted_notification.status == 'sending' - assert persisted_notification.sent_at > now + assert persisted_notification.sent_at <= datetime.utcnow() assert persisted_notification.sent_by == 'mmg' + assert not persisted_notification.personalisation def test_should_update_provider_stats_on_success(notify_db, sample_service, sample_notification, mocker): provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all() assert len(provider_stats) == 0 - notification = _notification_json( - sample_notification.template, - to=sample_notification.to - ) - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.get_name', return_value="mmg") mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') send_sms_to_provider( sample_notification.service_id, - sample_notification.id, - "encrypted-in-reality" + sample_notification.id ) updated_provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all() @@ -275,15 +197,11 @@ def test_should_update_provider_stats_on_success(notify_db, sample_service, samp assert updated_provider_stats[0].unit_count == 1 -def test_not_should_update_provider_stats_on_success(notify_db, sample_service, sample_notification, mocker): +def test_not_should_update_provider_stats_on_success_in_research_mode(notify_db, sample_service, sample_notification, + mocker): provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all() assert len(provider_stats) == 0 - notification = _notification_json( - sample_notification.template, - to=sample_notification.to - ) - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.get_name', return_value="mmg") mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') @@ -294,8 +212,7 @@ def test_not_should_update_provider_stats_on_success(notify_db, sample_service, send_sms_to_provider( sample_notification.service_id, - sample_notification.id, - "encrypted-in-reality" + sample_notification.id ) updated_provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all() @@ -303,22 +220,15 @@ def test_not_should_update_provider_stats_on_success(notify_db, sample_service, def test_statsd_updates(notify_db, notify_db_session, sample_service, sample_notification, mocker): - notification = _notification_json( - sample_notification.template, - to=sample_notification.to - ) - mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing') - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.mmg_client.send_sms') mocker.patch('app.mmg_client.get_name', return_value="mmg") mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') send_sms_to_provider( sample_notification.service_id, - sample_notification.id, - "encrypted-in-reality" + sample_notification.id ) statsd_client.incr.assert_called_once_with("notifications.tasks.send-sms-to-provider") @@ -332,50 +242,30 @@ def test_should_go_into_technical_error_if_exceeds_retries( notify_db, notify_db_session, sample_service, - sample_notification, mocker): - notification = _notification_json( - sample_notification.template, - to=sample_notification.to - ) + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + service=sample_service, status='created') mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing') - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.mmg_client.send_sms', side_effect=SmsClientException("EXPECTED")) mocker.patch('app.celery.provider_tasks.send_sms_to_provider.retry', side_effect=MaxRetriesExceededError()) send_sms_to_provider( - sample_notification.service_id, - sample_notification.id, - "encrypted-in-reality" + notification.service_id, + notification.id ) provider_tasks.send_sms_to_provider.retry.assert_called_with(queue='retry', countdown=10) assert statsd_client.incr.assert_not_called assert statsd_client.timing.assert_not_called - db_notification = Notification.query.get(sample_notification.id) + db_notification = Notification.query.filter_by(id=notification.id).one() assert db_notification.status == 'technical-failure' - notification_stats = NotificationStatistics.query.filter_by(service_id=sample_notification.service.id).first() + notification_stats = NotificationStatistics.query.filter_by(service_id=notification.service.id).first() assert notification_stats.sms_requested == 1 assert notification_stats.sms_failed == 1 - job = Job.query.get(sample_notification.job.id) + job = Job.query.get(notification.job.id) assert job.notification_count == 1 assert job.notifications_failed == 1 - - -def _notification_json(template, to, personalisation=None, job_id=None, row_number=None): - notification = { - "template": template.id, - "template_version": template.version, - "to": to, - } - if personalisation: - notification.update({"personalisation": personalisation}) - if job_id: - notification.update({"job": job_id}) - if row_number: - notification['row_number'] = row_number - return notification diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index acd942e2e..39452a2de 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -7,7 +7,7 @@ from mock import ANY from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.orm.exc import NoResultFound -from app import (aws_ses_client, encryption, DATETIME_FORMAT, statsd_client, db) +from app import (aws_ses_client, encryption, DATETIME_FORMAT, statsd_client) from app.celery import provider_tasks from app.celery import tasks from app.celery.research_mode_tasks import send_email_response @@ -19,7 +19,7 @@ from app.celery.tasks import ( send_email ) from app.clients.email.aws_ses import AwsSesClientException -from app.dao import notifications_dao, jobs_dao, provider_details_dao +from app.dao import jobs_dao, provider_details_dao from app.dao.provider_statistics_dao import get_provider_statistics from app.models import Notification from tests.app import load_example_csv @@ -313,7 +313,7 @@ def test_should_process_all_sms_job(sample_job, def test_should_send_template_to_correct_sms_task_and_persist(sample_template_with_placeholders, mocker): notification = _notification_json(sample_template_with_placeholders, to="+447234123123", personalisation={"name": "Jo"}) - mocker.patch('app.encryption.decrypt', return_value=notification) + mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing_with_dates') mocker.patch('app.statsd_client.timing') @@ -321,28 +321,19 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi notification_id = uuid.uuid4() - freezer = freeze_time("2016-01-01 11:09:00.00000") - freezer.start() - now = datetime.utcnow() - freezer.stop() - - freezer = freeze_time("2016-01-01 11:10:00.00000") - freezer.start() - send_sms( sample_template_with_placeholders.service_id, notification_id, - "encrypted-in-reality", - now.strftime(DATETIME_FORMAT) + encryption.encrypt(notification), + datetime.utcnow().strftime(DATETIME_FORMAT) ) - freezer.stop() statsd_client.timing.assert_called_once_with("notifications.tasks.send-sms.task-time", ANY) provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( (sample_template_with_placeholders.service_id, notification_id, - "encrypted-in-reality"), + encryption.encrypt(notification)), queue="sms" ) @@ -353,35 +344,35 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi assert persisted_notification.template_id == sample_template_with_placeholders.id assert persisted_notification.template_version == sample_template_with_placeholders.version assert persisted_notification.status == 'created' - assert persisted_notification.created_at == now + assert persisted_notification.created_at <= datetime.utcnow() assert not persisted_notification.sent_at assert not persisted_notification.sent_by assert not persisted_notification.job_id + assert persisted_notification.personalisation == {'name': 'Jo'} + assert persisted_notification._personalisation == encryption.encrypt({"name": "Jo"}) 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.encryption.decrypt', return_value=notification) mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') notification_id = uuid.uuid4() - now = datetime.utcnow() + encrypt_notification = encryption.encrypt(notification) send_sms( service.id, notification_id, - "encrypted-in-reality", - now.strftime(DATETIME_FORMAT) + encrypt_notification, + datetime.utcnow().strftime(DATETIME_FORMAT) ) provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( (service.id, notification_id, - "encrypted-in-reality"), + encrypt_notification), queue="sms" ) @@ -391,10 +382,11 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif assert persisted_notification.template_id == template.id assert persisted_notification.template_version == template.version assert persisted_notification.status == 'created' - assert persisted_notification.created_at == now + assert persisted_notification.created_at <= datetime.utcnow() assert not persisted_notification.sent_at assert not persisted_notification.sent_by assert not persisted_notification.job_id + assert not persisted_notification.personalisation def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker): @@ -403,16 +395,14 @@ 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.encryption.decrypt', return_value=notification) mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') notification_id = uuid.uuid4() - now = datetime.utcnow() send_sms( service.id, notification_id, - "encrypted-in-reality", - now.strftime(DATETIME_FORMAT) + encryption.encrypt(notification), + datetime.utcnow().strftime(DATETIME_FORMAT) ) provider_tasks.send_sms_to_provider.apply_async.assert_not_called() with pytest.raises(NoResultFound): @@ -429,7 +419,6 @@ def test_should_send_email_if_restricted_service_and_valid_email(notify_db, noti template_type='email') notification = _notification_json(template, "test@restricted.com") - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.aws_ses_client.send_email', return_value="1234") notification_id = uuid.uuid4() @@ -437,7 +426,7 @@ def test_should_send_email_if_restricted_service_and_valid_email(notify_db, noti send_email( service.id, notification_id, - "encrypted-in-reality", + encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) @@ -459,16 +448,14 @@ def test_should_not_send_email_if_restricted_service_and_invalid_email_address(n ) notification = _notification_json(template, to="test@example.com") - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.aws_ses_client.send_email') notification_id = uuid.uuid4() - now = datetime.utcnow() send_email( service.id, notification_id, - "encrypted-in-reality", - now.strftime(DATETIME_FORMAT) + encryption.encrypt(notification), + datetime.utcnow().strftime(DATETIME_FORMAT) ) aws_ses_client.send_email.assert_not_called() @@ -476,27 +463,25 @@ 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_send_template_to_and_persist_with_job_id(sample_job, mocker): +def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, mocker): notification = _notification_json( sample_job.template, to="+447234123123", job_id=sample_job.id, row_number=2) - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') notification_id = uuid.uuid4() - now = datetime.utcnow() send_sms( sample_job.service.id, notification_id, - "encrypted-in-reality", - now.strftime(DATETIME_FORMAT) + encryption.encrypt(notification), + datetime.utcnow().strftime(DATETIME_FORMAT) ) provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with( (sample_job.service.id, notification_id, - "encrypted-in-reality"), + encryption.encrypt(notification)), queue="sms" ) persisted_notification = Notification.query.filter_by(id=notification_id).one() @@ -506,7 +491,7 @@ def test_should_send_template_to_and_persist_with_job_id(sample_job, mocker): assert persisted_notification.template_id == sample_job.template.id assert persisted_notification.status == 'created' assert not persisted_notification.sent_at - assert persisted_notification.created_at == now + assert persisted_notification.created_at <= datetime.utcnow() assert not persisted_notification.sent_by assert persisted_notification.job_row_number == 2 @@ -517,7 +502,6 @@ 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.encryption.decrypt', return_value=notification) mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing_with_dates') mocker.patch('app.statsd_client.timing') @@ -533,11 +517,10 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh freezer = freeze_time("2016-01-01 11:10:00.00000") freezer.start() - send_email( sample_email_template_with_placeholders.service_id, notification_id, - "encrypted-in-reality", + encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) freezer.stop() @@ -545,7 +528,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh aws_ses_client.send_email.assert_called_once_with( '"Sample service" ', "my_email@my_email.com", - notification['personalisation']['name'], + "Jo", body="Hello Jo", html_body=AnyStringWith("Hello Jo"), reply_to_addresses=None @@ -569,6 +552,8 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh assert persisted_notification.status == 'sending' assert persisted_notification.sent_by == 'ses' assert persisted_notification.job_row_number == 1 + assert persisted_notification.personalisation == {'name': 'Jo'} + assert persisted_notification._personalisation == encryption.encrypt({"name": "Jo"}) def test_send_email_should_use_template_version_from_job_not_latest(sample_email_template, mocker): @@ -615,7 +600,6 @@ 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.encryption.decrypt', return_value=notification) mocker.patch('app.aws_ses_client.send_email', return_value="1234") mocker.patch('app.aws_ses_client.get_name', return_value='ses') @@ -624,7 +608,7 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi send_email( sample_email_template_with_placeholders.service_id, notification_id, - "encrypted-in-reality", + encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) aws_ses_client.send_email.assert_called_once_with( @@ -643,28 +627,12 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi assert persisted_notification.sent_at > now assert persisted_notification.status == 'sending' assert persisted_notification.sent_by == 'ses' - - -def test_should_use_email_template_and_persist_ses_reference(sample_email_template_with_placeholders, mocker): - notification = _notification_json(sample_email_template_with_placeholders, "my_email@my_email.com", {"name": "Jo"}) - mocker.patch('app.encryption.decrypt', return_value=notification) - mocker.patch('app.aws_ses_client.send_email', return_value='reference') - - notification_id = uuid.uuid4() - now = datetime.utcnow() - send_email( - sample_email_template_with_placeholders.service_id, - notification_id, - "encrypted-in-reality", - now.strftime(DATETIME_FORMAT) - ) - persisted_notification = Notification.query.filter_by(id=notification_id).one() - assert persisted_notification.reference == 'reference' + assert persisted_notification.personalisation == {"name": "Jo"} + assert persisted_notification.reference == '1234' def test_should_use_email_template_and_persist_without_personalisation(sample_email_template, mocker): - mocker.patch('app.encryption.decrypt', - return_value=_notification_json(sample_email_template, "my_email@my_email.com")) + notification = _notification_json(sample_email_template, "my_email@my_email.com") mocker.patch('app.aws_ses_client.send_email', return_value="ref") mocker.patch('app.aws_ses_client.get_name', return_value='ses') @@ -673,7 +641,7 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em send_email( sample_email_template.service_id, notification_id, - "encrypted-in-reality", + encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) aws_ses_client.send_email.assert_called_once_with( @@ -684,14 +652,23 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em html_body=AnyStringWith("This is a template"), reply_to_addresses=None ) + persisted_notification = Notification.query.filter_by(id=notification_id).one() + assert persisted_notification.id == notification_id + assert persisted_notification.to == 'my_email@my_email.com' + assert persisted_notification.template_id == sample_email_template.id + assert persisted_notification.created_at == now + assert persisted_notification.sent_at > now + assert persisted_notification.status == 'sending' + assert persisted_notification.sent_by == 'ses' + assert not persisted_notification.personalisation + assert persisted_notification.reference == 'ref' -def test_should_persist_notification_as_failed_if_database_fails(sample_template, mocker): +def test_should_go_to_retry_queue_if_database_errors(sample_template, mocker): notification = _notification_json(sample_template, "+447234123123") expected_exception = SQLAlchemyError() - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.celery.provider_tasks.send_sms_to_provider.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) @@ -703,7 +680,7 @@ def test_should_persist_notification_as_failed_if_database_fails(sample_template send_sms( sample_template.service_id, notification_id, - "encrypted-in-reality", + encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) provider_tasks.send_sms_to_provider.apply_async.assert_not_called() @@ -716,7 +693,6 @@ def test_should_persist_notification_as_failed_if_database_fails(sample_template def test_should_persist_notification_as_failed_if_email_client_fails(sample_email_template, mocker): notification = _notification_json(sample_email_template, "my_email@my_email.com") - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.aws_ses_client.send_email', side_effect=AwsSesClientException()) mocker.patch('app.aws_ses_client.get_name', return_value="ses") @@ -727,7 +703,7 @@ def test_should_persist_notification_as_failed_if_email_client_fails(sample_emai send_email( sample_email_template.service_id, notification_id, - "encrypted-in-reality", + encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) aws_ses_client.send_email.assert_called_once_with( @@ -746,12 +722,11 @@ def test_should_persist_notification_as_failed_if_email_client_fails(sample_emai assert persisted_notification.status == 'technical-failure' assert persisted_notification.created_at == now assert persisted_notification.sent_by == 'ses' - assert persisted_notification.sent_at > now + assert persisted_notification.sent_at >= now -def test_should_not_send_email_if_db_peristance_failed(sample_email_template, mocker): +def test_should_not_send_email_if_db_persistance_failed(sample_email_template, mocker): notification = _notification_json(sample_email_template, "my_email@my_email.com") - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.aws_ses_client.send_email') mocker.patch('app.db.session.add', side_effect=SQLAlchemyError()) now = datetime.utcnow() @@ -761,13 +736,10 @@ def test_should_not_send_email_if_db_peristance_failed(sample_email_template, mo send_email( sample_email_template.service_id, notification_id, - "encrypted-in-reality", + encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) aws_ses_client.send_email.assert_not_called() - with pytest.raises(NoResultFound) as e: - Notification.query.filter_by(id=notification_id).one() - assert 'No row was found for one' in str(e.value) def test_process_email_job_should_use_reply_to_email_if_present(sample_email_job, mocker, mock_celery_remove_job): @@ -838,7 +810,7 @@ def test_should_call_send_email_response_task_if_research_mode( assert persisted_notification.reference == str(reference) -def test_should_call_send_not_update_provider_email_stats_if_research_mode( +def test_should_use_research_mode_task_and_not_update_provider_stats( notify_db, sample_service, sample_email_template, @@ -852,7 +824,6 @@ def test_should_call_send_not_update_provider_email_stats_if_research_mode( reference = uuid.uuid4() mocker.patch('app.uuid.uuid4', return_value=reference) - mocker.patch('app.encryption.decrypt', return_value=notification) mocker.patch('app.aws_ses_client.send_email') mocker.patch('app.aws_ses_client.get_name', return_value="ses") mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') @@ -870,7 +841,7 @@ def test_should_call_send_not_update_provider_email_stats_if_research_mode( send_email( sample_service.id, notification_id, - "encrypted-in-reality", + encryption.encrypt(notification), now.strftime(DATETIME_FORMAT) ) assert not aws_ses_client.send_email.called @@ -881,65 +852,3 @@ def test_should_call_send_not_update_provider_email_stats_if_research_mode( assert not get_provider_statistics( sample_email_template.service, providers=[ses_provider.identifier]).first() - - -def test_email_template_personalisation_persisted(sample_email_template_with_placeholders, mocker): - encrypted_notification = encryption.encrypt(_notification_json( - sample_email_template_with_placeholders, - "my_email@my_email.com", - {"name": "Jo"}, - row_number=1)) - mocker.patch('app.statsd_client.incr') - mocker.patch('app.statsd_client.timing_with_dates') - mocker.patch('app.statsd_client.timing') - mocker.patch('app.aws_ses_client.get_name', return_value='ses') - mocker.patch('app.aws_ses_client.send_email', return_value='ses') - - notification_id = uuid.uuid4() - - send_email( - sample_email_template_with_placeholders.service_id, - notification_id, - encrypted_notification, - datetime.utcnow().strftime(DATETIME_FORMAT) - ) - - persisted_notification = Notification.query.filter_by(id=notification_id).one() - - assert persisted_notification.id == notification_id - assert persisted_notification.personalisation == {"name": "Jo"} - # personalisation data is encrypted in db - assert persisted_notification._personalisation == encryption.encrypt({"name": "Jo"}) - - -def test_sms_template_personalisation_persisted(sample_template_with_placeholders, mocker): - - encrypted_notification = encryption.encrypt(_notification_json(sample_template_with_placeholders, - to="+447234123123", personalisation={"name": "Jo"})) - mocker.patch('app.statsd_client.incr') - mocker.patch('app.statsd_client.timing_with_dates') - mocker.patch('app.statsd_client.timing') - mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async') - - notification_id = uuid.uuid4() - - send_sms( - sample_template_with_placeholders.service_id, - notification_id, - encrypted_notification, - 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, - encrypted_notification), - queue="sms" - ) - persisted_notification = Notification.query.filter_by(id=notification_id).one() - - assert persisted_notification.id == notification_id - assert persisted_notification.to == '+447234123123' - assert persisted_notification.personalisation == {"name": "Jo"} - # personalisation data is encrypted in db - assert persisted_notification._personalisation == encryption.encrypt({"name": "Jo"}) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 277762f27..ee62cc4cd 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import datetime, timedelta import uuid import pytest @@ -1259,7 +1259,8 @@ def test_get_notifications_for_service_returns_merged_template_content(notify_ap notify_db_session, service=sample_template_with_placeholders.service, template=sample_template_with_placeholders, - personalisation={"name": "merged with first"}) + personalisation={"name": "merged with first"}, + created_at=datetime.utcnow() - timedelta(seconds=1)) create_sample_notification(notify_db, notify_db_session,