When a notification is timed out in the scheduled task that may happen because the notification has not been sent.

Which means the sent_at date for the notification could be empty causing the service callback to fail.

- Allow code to work if notification.sent_at or updated_at is None
- Update calls to send_delivery_status_to_service to send the data encrypted so that the task does not need to use the db.

tests/app/celery/test_scheduled_tasks.py

@@ -57,6 +57,7 @@ from app.models import (
     SMS_TYPE
 )
 from app.utils import get_london_midnight_in_utc
+from app.celery.service_callback_tasks import create_encrypted_callback_data
 from app.v2.errors import JobIncompleteError
 from tests.app.db import (
     create_notification, create_service, create_template, create_job, create_rate,
@@ -210,7 +211,7 @@ def test_should_not_update_status_of_letter_notifications(client, sample_letter_
 
 
 def test_timeout_notifications_sends_status_update_to_service(client, sample_template, mocker):
-    create_service_callback_api(service=sample_template.service)
+    callback_api = create_service_callback_api(service=sample_template.service)
     mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async')
     notification = create_notification(
         template=sample_template,
@@ -218,7 +219,9 @@ def test_timeout_notifications_sends_status_update_to_service(client, sample_tem
         created_at=datetime.utcnow() - timedelta(
             seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10))
     timeout_notifications()
-    mocked.assert_called_once_with([str(notification.id)], queue=QueueNames.CALLBACKS)
+
+    encrypted_data = create_encrypted_callback_data(notification, callback_api)
+    mocked.assert_called_once_with([str(notification.id), encrypted_data], queue=QueueNames.CALLBACKS)
 
 
 def test_should_update_scheduled_jobs_and_put_on_queue(notify_db, notify_db_session, mocker):

tests/app/celery/test_service_callback_tasks.py

@@ -110,6 +110,29 @@ def test_send_delivery_status_to_service_does_not_retries_if_request_returns_404
     assert mocked.call_count == 0
 
 
+def test_send_delivery_status_to_service_succeeds_if_sent_at_is_none(
+        notify_db_session,
+        mocker
+):
+    callback_api, template = _set_up_test_data('email')
+    datestr = datetime(2017, 6, 20)
+    notification = create_notification(template=template,
+                                       created_at=datestr,
+                                       updated_at=datestr,
+                                       sent_at=None,
+                                       status='technical-failure'
+                                       )
+    encrypted_data = _set_up_encrypted_data(callback_api, notification)
+    mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry')
+    with requests_mock.Mocker() as request_mock:
+        request_mock.post(callback_api.url,
+                          json={},
+                          status_code=404)
+        send_delivery_status_to_service(notification.id, encrypted_status_update=encrypted_data)
+
+    assert mocked.call_count == 0
+
+
 def _set_up_test_data(notification_type):
     service = create_service(restricted=True)
     template = create_template(service=service, template_type=notification_type, subject='Hello')
@@ -125,8 +148,9 @@ def _set_up_encrypted_data(callback_api, notification):
         "notification_to": notification.to,
         "notification_status": notification.status,
         "notification_created_at": notification.created_at.strftime(DATETIME_FORMAT),
-        "notification_updated_at": notification.updated_at.strftime(DATETIME_FORMAT),
-        "notification_sent_at": notification.sent_at.strftime(DATETIME_FORMAT),
+        "notification_updated_at": notification.updated_at.strftime(
+            DATETIME_FORMAT) if notification.updated_at else None,
+        "notification_sent_at": notification.sent_at.strftime(DATETIME_FORMAT) if notification.sent_at else None,
         "notification_type": notification.notification_type,
         "service_callback_api_url": callback_api.url,
         "service_callback_api_bearer_token": callback_api.bearer_token,

tests/app/notifications/test_notifications_ses_callback.py

@@ -5,8 +5,10 @@ from freezegun import freeze_time
 
 from app import statsd_client
 from app.dao.notifications_dao import get_notification_by_id
+from app.models import Notification
 from app.notifications.notifications_ses_callback import process_ses_response, remove_emails_from_bounce
 from app.celery.research_mode_tasks import ses_hard_bounce_callback, ses_soft_bounce_callback, ses_notification_callback
+from app.celery.service_callback_tasks import create_encrypted_callback_data
 
 from tests.app.conftest import sample_notification as create_sample_notification
 from tests.app.db import create_service_callback_api
@@ -32,7 +34,8 @@ def test_ses_callback_should_update_notification_status(
             status='sending',
             sent_at=datetime.utcnow()
         )
-        create_service_callback_api(service=sample_email_template.service, url="https://original_url.com")
+        callback_api = create_service_callback_api(service=sample_email_template.service,
+                                                   url="https://original_url.com")
         assert get_notification_by_id(notification.id).status == 'sending'
 
         errors = process_ses_response(ses_notification_callback(reference='ref'))
@@ -42,7 +45,9 @@ def test_ses_callback_should_update_notification_status(
             "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
         )
         statsd_client.incr.assert_any_call("callback.ses.delivered")
-        send_mock.assert_called_once_with([str(notification.id)], queue="service-callbacks")
+        updated_notification = Notification.query.get(notification.id)
+        encrypted_data = create_encrypted_callback_data(updated_notification, callback_api)
+        send_mock.assert_called_once_with([str(notification.id), encrypted_data], queue="service-callbacks")
 
 
 def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry(

tests/app/notifications/test_process_client_response.py

@@ -7,6 +7,7 @@ from app.notifications.process_client_response import (
     validate_callback_data,
     process_sms_client_response
 )
+from app.celery.service_callback_tasks import create_encrypted_callback_data
 from tests.app.db import create_service_callback_api
 
 
@@ -57,13 +58,15 @@ def test_outcome_statistics_called_for_successful_callback(sample_notification,
     send_mock = mocker.patch(
         'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
     )
-    create_service_callback_api(service=sample_notification.service, url="https://original_url.com")
+    callback_api = create_service_callback_api(service=sample_notification.service, url="https://original_url.com")
     reference = str(uuid.uuid4())
 
     success, error = process_sms_client_response(status='3', reference=reference, client_name='MMG')
     assert success == "MMG callback succeeded. reference {} updated".format(str(reference))
     assert error is None
-    send_mock.assert_called_once_with([str(sample_notification.id)], queue="service-callbacks")
+    encrypted_data = create_encrypted_callback_data(sample_notification, callback_api)
+    send_mock.assert_called_once_with([str(sample_notification.id), encrypted_data],
+                                      queue="service-callbacks")
 
 
 def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notification, mocker):