Ignore SUCCESSFUL pinpoint receipts (#2176)

Author: sastels

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",

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()
 

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