Merge pull request #3383 from alphagov/email-sms-created-alert-180344153

Add new log / alert for 'created' email / SMS
2026-08-23 15:56:45 -04:00 · 2021-12-13 12:56:05 +00:00
parent c25585fd60 a8edfeb941
commit 2fb432adaf
11 changed files with 96 additions and 248 deletions
--- a/tests/app/celery/test_nightly_tasks.py
+++ b/tests/app/celery/test_nightly_tasks.py
@@ -24,17 +24,11 @@ from app.celery.nightly_tasks import (
    save_daily_notification_processing_time,
    timeout_notifications,
 )
-from app.celery.service_callback_tasks import (
-    create_delivery_status_callback_data,
-)
-from app.config import QueueNames
-from app.exceptions import NotificationTechnicalFailureException
 from app.models import EMAIL_TYPE, LETTER_TYPE, SMS_TYPE, FactProcessingTime
 from tests.app.db import (
    create_job,
    create_notification,
    create_service,
-    create_service_callback_api,
    create_service_data_retention,
    create_template,
 )
@@ -148,87 +142,40 @@ def test_remove_csv_files_filters_by_type(mocker, sample_service):
    ]


-def test_should_call_delete_sms_notifications_more_than_week_in_task(notify_api, mocker):
+def test_delete_sms_notifications_older_than_retention_calls_child_task(notify_api, mocker):
    mocked = mocker.patch('app.celery.nightly_tasks.delete_notifications_older_than_retention_by_type')
    delete_sms_notifications_older_than_retention()
    mocked.assert_called_once_with('sms')


-def test_should_call_delete_email_notifications_more_than_week_in_task(notify_api, mocker):
+def test_delete_email_notifications_older_than_retentions_calls_child_task(notify_api, mocker):
    mocked_notifications = mocker.patch(
        'app.celery.nightly_tasks.delete_notifications_older_than_retention_by_type')
    delete_email_notifications_older_than_retention()
    mocked_notifications.assert_called_once_with('email')


-def test_should_call_delete_letter_notifications_more_than_week_in_task(notify_api, mocker):
+def test_delete_letter_notifications_older_than_retention_calls_child_task(notify_api, mocker):
    mocked = mocker.patch('app.celery.nightly_tasks.delete_notifications_older_than_retention_by_type')
    delete_letter_notifications_older_than_retention()
    mocked.assert_called_once_with('letter')


-def test_update_status_of_notifications_after_timeout(notify_api, sample_template):
-    with notify_api.test_request_context():
-        not1 = create_notification(
-            template=sample_template,
-            status='sending',
-            created_at=datetime.utcnow() - timedelta(
-                seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10))
-        not2 = create_notification(
-            template=sample_template,
-            status='created',
-            created_at=datetime.utcnow() - timedelta(
-                seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10))
-        not3 = create_notification(
-            template=sample_template,
-            status='pending',
-            created_at=datetime.utcnow() - timedelta(
-                seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10))
-        with pytest.raises(NotificationTechnicalFailureException) as e:
-            timeout_notifications()
-        assert str(not2.id) in str(e.value)
-        assert not1.status == 'temporary-failure'
-        assert not2.status == 'technical-failure'
-        assert not3.status == 'temporary-failure'
-
-
-def test_not_update_status_of_notification_before_timeout(notify_api, sample_template):
-    with notify_api.test_request_context():
-        not1 = create_notification(
-            template=sample_template,
-            status='sending',
-            created_at=datetime.utcnow() - timedelta(
-                seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') - 10))
-        timeout_notifications()
-        assert not1.status == 'sending'
-
-
-def test_should_not_update_status_of_letter_notifications(client, sample_letter_template):
-    created_at = datetime.utcnow() - timedelta(days=5)
-    not1 = create_notification(template=sample_letter_template, status='sending', created_at=created_at)
-    not2 = create_notification(template=sample_letter_template, status='created', created_at=created_at)
+def test_timeout_notifications(mocker, sample_notification):
+    mock_update = mocker.patch('app.celery.nightly_tasks.check_and_queue_callback_task')
+    mock_dao = mocker.patch('app.celery.nightly_tasks.dao_timeout_notifications')
+    mock_dao.return_value = [sample_notification]

    timeout_notifications()

-    assert not1.status == 'sending'
-    assert not2.status == 'created'
+    mock_dao.assert_called_once_with(
+        current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD')
+    )
+
+    mock_update.assert_called_once_with(sample_notification)


-def test_timeout_notifications_sends_status_update_to_service(client, sample_template, mocker):
-    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,
-        status='sending',
-        created_at=datetime.utcnow() - timedelta(
-            seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10))
-    timeout_notifications()
-
-    encrypted_data = create_delivery_status_callback_data(notification, callback_api)
-    mocked.assert_called_once_with([str(notification.id), encrypted_data], queue=QueueNames.CALLBACKS)
-
-
-def test_should_call_delete_inbound_sms(notify_api, mocker):
+def test_delete_inbound_sms_calls_child_task(notify_api, mocker):
    mocker.patch('app.celery.nightly_tasks.delete_inbound_sms_older_than_retention')
    delete_inbound_sms()
    assert nightly_tasks.delete_inbound_sms_older_than_retention.call_count == 1
--- a/tests/app/celery/test_process_ses_receipts_tasks.py
+++ b/tests/app/celery/test_process_ses_receipts_tasks.py
@@ -10,9 +10,6 @@ from app.celery.research_mode_tasks import (
    ses_notification_callback,
    ses_soft_bounce_callback,
 )
-from app.celery.service_callback_tasks import (
-    create_delivery_status_callback_data,
-)
 from app.dao.notifications_dao import get_notification_by_id
 from app.models import Complaint, Notification
 from app.notifications.notifications_ses_callback import (
@@ -72,15 +69,13 @@ def test_ses_callback_should_update_notification_status(
        mocker.patch('app.statsd_client.incr')
        mocker.patch('app.statsd_client.timing_with_dates')
        send_mock = mocker.patch(
-            'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+            'app.celery.process_ses_receipts_tasks.check_and_queue_callback_task'
        )
        notification = create_notification(
            template=sample_email_template,
            status='sending',
            reference='ref',
        )
-        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'

        assert process_ses_results(ses_notification_callback(reference='ref'))
@@ -90,8 +85,7 @@ def test_ses_callback_should_update_notification_status(
        )
        statsd_client.incr.assert_any_call("callback.ses.delivered")
        updated_notification = Notification.query.get(notification.id)
-        encrypted_data = create_delivery_status_callback_data(updated_notification, callback_api)
-        send_mock.assert_called_once_with([str(notification.id), encrypted_data], queue="service-callbacks")
+        send_mock.assert_called_once_with(updated_notification)


 def test_ses_callback_should_not_update_notification_status_if_already_delivered(sample_email_template, mocker):
@@ -138,30 +132,6 @@ def test_ses_callback_should_not_retry_if_notification_is_old(client, notify_db,
        assert mock_retry.call_count == 0


-def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry(
-        client,
-        notify_db_session,
-        sample_email_template,
-        mocker):
-    with freeze_time('2001-01-01T12:00:00'):
-
-        send_mock = mocker.patch(
-            'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-        )
-        notification = create_notification(
-            template=sample_email_template,
-            status='sending',
-            reference='ref',
-        )
-
-        assert get_notification_by_id(notification.id).status == 'sending'
-
-        assert process_ses_results(ses_notification_callback(reference='ref'))
-        assert get_notification_by_id(notification.id).status == 'delivered'
-
-        send_mock.assert_not_called()
-
-
 def test_ses_callback_should_update_multiple_notification_status_sent(
        client,
        notify_db_session,
@@ -169,7 +139,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
        mocker):

    send_mock = mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+        'app.celery.process_ses_receipts_tasks.check_and_queue_callback_task'
    )
    create_notification(
        template=sample_email_template,
@@ -186,7 +156,6 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
        status='sending',
        reference='ref3',
    )
-    create_service_callback_api(service=sample_email_template.service, url="https://original_url.com")
    assert process_ses_results(ses_notification_callback(reference='ref1'))
    assert process_ses_results(ses_notification_callback(reference='ref2'))
    assert process_ses_results(ses_notification_callback(reference='ref3'))
@@ -198,7 +167,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
                                                             sample_email_template,
                                                             mocker):
    send_mock = mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+        'app.celery.process_ses_receipts_tasks.check_and_queue_callback_task'
    )
    mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.info')
    notification = create_notification(
@@ -206,7 +175,6 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
        status='sending',
        reference='ref',
    )
-    create_service_callback_api(service=notification.service, url="https://original_url.com")
    assert get_notification_by_id(notification.id).status == 'sending'
    assert process_ses_results(ses_soft_bounce_callback(reference='ref'))
    assert get_notification_by_id(notification.id).status == 'temporary-failure'
@@ -219,7 +187,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
                                                             sample_email_template,
                                                             mocker):
    send_mock = mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+        'app.celery.process_ses_receipts_tasks.check_and_queue_callback_task'
    )
    mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.info')
    notification = create_notification(
@@ -227,7 +195,6 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
        status='sending',
        reference='ref',
    )
-    create_service_callback_api(service=sample_email_template.service, url="https://original_url.com")

    assert get_notification_by_id(notification.id).status == 'sending'
    assert process_ses_results(ses_hard_bounce_callback(reference='ref'))
--- a/tests/app/dao/notification_dao/test_notification_dao.py
+++ b/tests/app/dao/notification_dao/test_notification_dao.py
@@ -675,12 +675,12 @@ def test_dao_timeout_notifications(sample_template):
    assert Notification.query.get(sending.id).status == 'sending'
    assert Notification.query.get(pending.id).status == 'pending'
    assert Notification.query.get(delivered.id).status == 'delivered'
-    technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1)
-    assert Notification.query.get(created.id).status == 'technical-failure'
+    temporary_failure_notifications = dao_timeout_notifications(1)
+    assert Notification.query.get(created.id).status == 'created'
    assert Notification.query.get(sending.id).status == 'temporary-failure'
    assert Notification.query.get(pending.id).status == 'temporary-failure'
    assert Notification.query.get(delivered.id).status == 'delivered'
-    assert len(technical_failure_notifications + temporary_failure_notifications) == 3
+    assert len(temporary_failure_notifications) == 2


 def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_template):
@@ -694,8 +694,8 @@ def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_t
    assert Notification.query.get(sending.id).status == 'sending'
    assert Notification.query.get(pending.id).status == 'pending'
    assert Notification.query.get(delivered.id).status == 'delivered'
-    technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1)
-    assert len(technical_failure_notifications + temporary_failure_notifications) == 0
+    temporary_failure_notifications = dao_timeout_notifications(1)
+    assert len(temporary_failure_notifications) == 0


 def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template):
@@ -709,8 +709,8 @@ def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template)
    assert Notification.query.get(sending.id).status == 'sending'
    assert Notification.query.get(pending.id).status == 'pending'
    assert Notification.query.get(delivered.id).status == 'delivered'
-
-    technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1)
+    temporary_failure_notifications = dao_timeout_notifications(1)
+    assert len(temporary_failure_notifications) == 0


 def test_should_return_notifications_excluding_jobs_by_default(sample_template, sample_job, sample_api_key):
--- a/tests/app/notifications/test_notifications_ses_callback.py
+++ b/tests/app/notifications/test_notifications_ses_callback.py
@@ -4,10 +4,14 @@ from sqlalchemy.exc import SQLAlchemyError

 from app.dao.notifications_dao import get_notification_by_id
 from app.models import Complaint
-from app.notifications.notifications_ses_callback import handle_complaint
+from app.notifications.notifications_ses_callback import (
+    check_and_queue_callback_task,
+    handle_complaint,
+)
 from tests.app.db import (
    create_notification,
    create_notification_history,
+    create_service_callback_api,
    ses_complaint_callback,
    ses_complaint_callback_malformed_message_id,
    ses_complaint_callback_with_missing_complaint_type,
@@ -64,3 +68,37 @@ def test_process_ses_results_in_complaint_save_complaint_with_null_complaint_typ
    assert len(complaints) == 1
    assert complaints[0].notification_id == notification.id
    assert not complaints[0].complaint_type
+
+
+def test_check_and_queue_callback_task(mocker, sample_notification):
+    mock_create = mocker.patch(
+        'app.notifications.notifications_ses_callback.create_delivery_status_callback_data'
+    )
+
+    mock_send = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
+
+    callback_api = create_service_callback_api(service=sample_notification.service)
+    mock_create.return_value = 'encrypted_status_update'
+
+    check_and_queue_callback_task(sample_notification)
+
+    # callback_api doesn't match by equality for some
+    # reason, so we need to take this approach instead
+    mock_create_args = mock_create.mock_calls[0][1]
+    assert mock_create_args[0] == sample_notification
+    assert mock_create_args[1].id == callback_api.id
+
+    mock_send.assert_called_once_with(
+        [str(sample_notification.id), mock_create.return_value], queue="service-callbacks"
+    )
+
+
+def test_check_and_queue_callback_task_no_callback_api(mocker, sample_notification):
+    mock_send = mocker.patch(
+        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
+    )
+
+    check_and_queue_callback_task(sample_notification)
+    mock_send.assert_not_called()
--- a/tests/app/notifications/test_process_client_response.py
+++ b/tests/app/notifications/test_process_client_response.py
@@ -8,12 +8,8 @@ from app import statsd_client
 from app.celery.process_sms_client_response_tasks import (
    process_sms_client_response,
 )
-from app.celery.service_callback_tasks import (
-    create_delivery_status_callback_data,
-)
 from app.clients import ClientException
 from app.models import NOTIFICATION_TECHNICAL_FAILURE
-from tests.app.db import create_service_callback_api


 def test_process_sms_client_response_raises_error_if_reference_is_not_a_valid_uuid(client):
@@ -121,12 +117,7 @@ def test_process_sms_client_response_updates_notification_status_when_detailed_s


 def test_sms_response_does_not_send_callback_if_notification_is_not_in_the_db(sample_service, mocker):
-    mocker.patch(
-        'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service',
-        return_value='mock-delivery-callback-for-service')
-    send_mock = mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
+    send_mock = mocker.patch('app.celery.process_sms_client_response_tasks.check_and_queue_callback_task')
    reference = str(uuid.uuid4())
    process_sms_client_response(status='3', provider_reference=reference, client_name='MMG')
    send_mock.assert_not_called()
@@ -156,26 +147,17 @@ def test_process_sms_updates_billable_units_if_zero(sample_notification):


 def test_process_sms_response_does_not_send_service_callback_for_pending_notifications(sample_notification, mocker):
-    mocker.patch(
-        'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service',
-        return_value='fake-callback')
-    send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async')
+    send_mock = mocker.patch('app.celery.process_sms_client_response_tasks.check_and_queue_callback_task')
    process_sms_client_response('2', str(sample_notification.id), 'Firetext')
    send_mock.assert_not_called()


 def test_outcome_statistics_called_for_successful_callback(sample_notification, mocker):
-    send_mock = mocker.patch(
-        'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
-    )
-    callback_api = create_service_callback_api(service=sample_notification.service, url="https://original_url.com")
+    send_mock = mocker.patch('app.celery.process_sms_client_response_tasks.check_and_queue_callback_task')
    reference = str(sample_notification.id)

    process_sms_client_response('3', reference, 'MMG')
-
-    encrypted_data = create_delivery_status_callback_data(sample_notification, callback_api)
-    send_mock.assert_called_once_with([reference, encrypted_data],
-                                      queue="service-callbacks")
+    send_mock.assert_called_once_with(sample_notification)


 def test_process_sms_updates_sent_by_with_client_name_if_not_in_noti(sample_notification):