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

@@ -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[
+        values={} if not notification.personalisation else notification.personalisation,
-            'personalisation'] else {},
         prefix=service.name
     )
     try:

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,
+        Notification.id == notification_id).first()
-        or_(Notification.status == 'sending',
-            Notification.status == 'pending')).first()
 
     if not notification:
         return False

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,
+        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: 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
-
+    assert notification.personalisation == {"name": "Jo"}
-
-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)
-    )
 
 
 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,
+        db_notification.id
-        "encrypted-in-reality"
     )
 
     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,
+        sample_notification.id
-        "encrypted-in-reality"
     )
     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,
+        sample_notification.id
-        "encrypted-in-reality"
     )
 
     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,
+        sample_notification.id
-        "encrypted-in-reality"
     )
 
     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,
+        sample_notification.id
-        "encrypted-in-reality"
     )
 
     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(
+    notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session,
-        sample_notification.template,
+                                       service=sample_service, status='created')
-        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', 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,
+        notification.service_id,
-        sample_notification.id,
+        notification.id
-        "encrypted-in-reality"
     )
 
     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

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",
+        encryption.encrypt(notification),
-        now.strftime(DATETIME_FORMAT)
+        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",
+        encrypt_notification,
-        now.strftime(DATETIME_FORMAT)
+        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",
+        encryption.encrypt(notification),
-        now.strftime(DATETIME_FORMAT)
+        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",
+        encryption.encrypt(notification),
-        now.strftime(DATETIME_FORMAT)
+        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",
+        encryption.encrypt(notification),
-        now.strftime(DATETIME_FORMAT)
+        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" <sample.service@test.notify.com>',
         "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'
-
+    assert persisted_notification.personalisation == {"name": "Jo"}
-
+    assert persisted_notification.reference == '1234'
-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'
 
 
 def test_should_use_email_template_and_persist_without_personalisation(sample_email_template, mocker):
-    mocker.patch('app.encryption.decrypt',
+    notification = _notification_json(sample_email_template, "my_email@my_email.com")
-                 return_value=_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"})

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,