From 86fb27b316d6b869b96994a39bfae4d1ea4558af Mon Sep 17 00:00:00 2001 From: Steve Astels Date: Mon, 13 May 2024 15:13:01 -0400 Subject: [PATCH] Ignore SUCCESSFUL pinpoint receipts (#2176) --- app/aws/mocks.py | 28 ++++++++++++- app/celery/process_pinpoint_receipts_tasks.py | 6 +++ .../test_process_pinpoint_receipts_tasks.py | 39 +++++++++++++++---- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/app/aws/mocks.py b/app/aws/mocks.py index 19aa7a4108..7a7943a0ac 100644 --- a/app/aws/mocks.py +++ b/app/aws/mocks.py @@ -193,7 +193,33 @@ def sns_failed_callback(provider_response, reference=None, timestamp="2016-06-28 # Note that 1467074434 = 2016-06-28 00:40:34.558 UTC -def pinpoint_success_callback(reference=None, timestamp=1467074434, destination="+1XXX5550100"): +def pinpoint_successful_callback(reference=None, timestamp=1467074434, destination="+1XXX5550100"): + body = { + "eventType": "TEXT_SUCCESSFUL", + "eventVersion": "1.0", + "eventTimestamp": timestamp, + "isFinal": False, + "originationPhoneNumber": "+18078061258", + "destinationPhoneNumber": destination, + "isoCountryCode": "CA", + "mcc": "302", + "mnc": "610", + "carrierName": "Bell Cellular Inc. / Aliant Telecom", + "messageId": reference, + "messageRequestTimestamp": timestamp, + "messageEncoding": "GSM", + "messageType": "TRANSACTIONAL", + "messageStatus": "SUCCESSFUL", + "messageStatusDescription": "Message has been accepted by phone carrier", + "totalMessageParts": 1, + "totalMessagePrice": 0.00581, + "totalCarrierFee": 0.00767, + } + + return _pinpoint_callback(body) + + +def pinpoint_delivered_callback(reference=None, timestamp=1467074434, destination="+1XXX5550100"): body = { "eventType": "TEXT_DELIVERED", "eventVersion": "1.0", diff --git a/app/celery/process_pinpoint_receipts_tasks.py b/app/celery/process_pinpoint_receipts_tasks.py index 5e9148b2cd..6192ee40cc 100644 --- a/app/celery/process_pinpoint_receipts_tasks.py +++ b/app/celery/process_pinpoint_receipts_tasks.py @@ -53,6 +53,10 @@ def process_pinpoint_results(self, response): provider_response = receipt["messageStatusDescription"] notification_status = determine_pinpoint_status(status, provider_response) + + if notification_status == NOTIFICATION_SENT: + return # we don't want to update the status to sent if it's already sent + if not notification_status: current_app.logger.warning(f"unhandled provider response for reference {reference}, received '{provider_response}'") notification_status = NOTIFICATION_TECHNICAL_FAILURE # revert to tech failure by default @@ -125,6 +129,8 @@ def determine_pinpoint_status(status: str, provider_response: str) -> Union[str, if status == "DELIVERED": return NOTIFICATION_DELIVERED + elif status == "SUCCESSFUL": # carrier has accepted the message but it hasn't gone to the phone yet + return NOTIFICATION_SENT response_lower = provider_response.lower() diff --git a/tests/app/celery/test_process_pinpoint_receipts_tasks.py b/tests/app/celery/test_process_pinpoint_receipts_tasks.py index 03932428bd..60b8096170 100644 --- a/tests/app/celery/test_process_pinpoint_receipts_tasks.py +++ b/tests/app/celery/test_process_pinpoint_receipts_tasks.py @@ -4,7 +4,11 @@ from freezegun import freeze_time from app import statsd_client -from app.aws.mocks import pinpoint_failed_callback, pinpoint_success_callback +from app.aws.mocks import ( + pinpoint_delivered_callback, + pinpoint_failed_callback, + pinpoint_successful_callback, +) from app.celery.process_pinpoint_receipts_tasks import process_pinpoint_results from app.dao.notifications_dao import get_notification_by_id from app.models import ( @@ -39,7 +43,7 @@ def test_process_pinpoint_results_delivered(sample_template, notify_db, notify_d ) assert get_notification_by_id(notification.id).status == NOTIFICATION_SENT - process_pinpoint_results(pinpoint_success_callback(reference="ref")) + process_pinpoint_results(pinpoint_delivered_callback(reference="ref")) assert mock_callback_task.called_once_with(get_notification_by_id(notification.id)) assert get_notification_by_id(notification.id).status == NOTIFICATION_DELIVERED @@ -48,6 +52,27 @@ def test_process_pinpoint_results_delivered(sample_template, notify_db, notify_d mock_logger.assert_called_once_with(f"Pinpoint callback return status of delivered for notification: {notification.id}") +def test_process_pinpoint_results_succeeded(sample_template, notify_db, notify_db_session, mocker): + mock_callback_task = mocker.patch("app.notifications.callbacks._check_and_queue_callback_task") + + notification = create_sample_notification( + notify_db, + notify_db_session, + template=sample_template, + reference="ref", + status=NOTIFICATION_SENT, + sent_by="pinpoint", + sent_at=datetime.utcnow(), + ) + assert get_notification_by_id(notification.id).status == NOTIFICATION_SENT + + process_pinpoint_results(pinpoint_successful_callback(reference="ref")) + + assert mock_callback_task.not_called() + assert get_notification_by_id(notification.id).status == NOTIFICATION_SENT + assert get_notification_by_id(notification.id).provider_response is None + + @pytest.mark.parametrize( "provider_response, expected_status, should_log_warning, should_save_provider_response", [ @@ -120,7 +145,7 @@ def test_pinpoint_callback_should_retry_if_notification_is_missing(notify_db, mo mock_retry = mocker.patch("app.celery.process_pinpoint_receipts_tasks.process_pinpoint_results.retry") mock_callback_task = mocker.patch("app.notifications.callbacks._check_and_queue_callback_task") - process_pinpoint_results(pinpoint_success_callback(reference="ref")) + process_pinpoint_results(pinpoint_delivered_callback(reference="ref")) mock_callback_task.assert_not_called() assert mock_retry.call_count == 1 @@ -134,7 +159,7 @@ def test_pinpoint_callback_should_give_up_after_max_tries(notify_db, mocker): mock_logger = mocker.patch("app.celery.process_pinpoint_receipts_tasks.current_app.logger.warning") mock_callback_task = mocker.patch("app.notifications.callbacks._check_and_queue_callback_task") - process_pinpoint_results(pinpoint_success_callback(reference="ref")) is None + process_pinpoint_results(pinpoint_delivered_callback(reference="ref")) is None mock_callback_task.assert_not_called() mock_logger.assert_called_with("notification not found for Pinpoint reference: ref (update to delivered). Giving up.") @@ -156,7 +181,7 @@ def test_process_pinpoint_results_retry_called(sample_template, mocker): side_effect=Exception("EXPECTED"), ) mocked = mocker.patch("app.celery.process_pinpoint_receipts_tasks.process_pinpoint_results.retry") - process_pinpoint_results(response=pinpoint_success_callback(reference="ref1")) + process_pinpoint_results(response=pinpoint_delivered_callback(reference="ref1")) assert mocked.call_count == 1 @@ -173,7 +198,7 @@ def test_process_pinpoint_results_does_not_process_other_providers(sample_templa ) ) - process_pinpoint_results(response=pinpoint_success_callback(reference="ref1")) is None + process_pinpoint_results(response=pinpoint_delivered_callback(reference="ref1")) is None assert mock_logger.called_once_with("") assert not mock_dao.called @@ -197,7 +222,7 @@ def test_process_pinpoint_results_calls_service_callback(sample_template, notify callback_api = create_service_callback_api(service=sample_template.service, url="https://example.com") assert get_notification_by_id(notification.id).status == NOTIFICATION_SENT - process_pinpoint_results(pinpoint_success_callback(reference="ref")) + process_pinpoint_results(pinpoint_delivered_callback(reference="ref")) assert mock_callback.called_once_with(get_notification_by_id(notification.id)) assert get_notification_by_id(notification.id).status == NOTIFICATION_DELIVERED