#2163 HOTFIX - Status Reason Updates

app/celery/process_ses_receipts_tasks.py

@@ -30,7 +30,7 @@
     NOTIFICATION_PERMANENT_FAILURE,
     NOTIFICATION_TEMPORARY_FAILURE,
     STATUS_REASON_RETRYABLE,
-    STATUS_REASON_UNDELIVERABLE,
+    STATUS_REASON_UNREACHABLE,
 )
 from app.clients.email.aws_ses import get_aws_responses
 from app.dao import notifications_dao, services_dao, templates_dao
@@ -177,7 +177,7 @@ def process_ses_results(  # noqa: C901 (too complex 14 > 10)
         aws_response_dict = get_aws_responses(notification_type)
 
         # This is the prospective, updated status.
-        notification_status = aws_response_dict['notification_status']
+        incoming_status = aws_response_dict['notification_status']
         reference = ses_message['mail']['messageId']
 
         try:
@@ -188,44 +188,39 @@ def process_ses_results(  # noqa: C901 (too complex 14 > 10)
                 self.retry(queue=QueueNames.RETRY)
             else:
                 current_app.logger.warning(
-                    'notification not found for reference: %s (update to %s)', reference, notification_status
+                    'notification not found for reference: %s (update to %s)', reference, incoming_status
                 )
             return
 
         # Prevent regressing bounce status.  Note that this is a test of the existing status; not the new status.
-        if (
-            notification.status_reason
-            and 'bounce' in notification.status_reason
-            and notification.status
-            in {
-                NOTIFICATION_TEMPORARY_FAILURE,
-                NOTIFICATION_PERMANENT_FAILURE,
-            }
-        ):
+        if notification.status_reason and notification.status in {
+            NOTIFICATION_TEMPORARY_FAILURE,
+            NOTIFICATION_PERMANENT_FAILURE,
+        }:
             # async from AWS means we may get a delivered status after a bounce, in rare cases
             current_app.logger.warning(
                 'Notification: %s was marked as a bounce, cannot be updated to: %s',
                 notification.id,
-                notification_status,
+                incoming_status,
             )
             return
 
         # This is a test of the new status.  Is it a bounce?
-        if notification_status in (NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE):
+        if incoming_status in (NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE):
             # Add the failure status reason to the notification.
-            if notification_status == NOTIFICATION_PERMANENT_FAILURE:
+            if incoming_status == NOTIFICATION_PERMANENT_FAILURE:
                 failure_reason = 'Failed to deliver email due to hard bounce'
-                status_reason = STATUS_REASON_UNDELIVERABLE
+                status_reason = STATUS_REASON_UNREACHABLE
             else:
                 failure_reason = 'Temporarily failed to deliver email due to soft bounce'
                 status_reason = STATUS_REASON_RETRYABLE
 
             notification.status_reason = status_reason
-            notification.status = notification_status
+            notification.status = incoming_status
 
             current_app.logger.warning(
                 '%s - %s - in process_ses_results for notification %s',
-                notification_status,
+                incoming_status,
                 failure_reason,
                 notification.id,
             )
@@ -235,15 +230,15 @@ def process_ses_results(  # noqa: C901 (too complex 14 > 10)
             check_and_queue_va_profile_notification_status_callback(notification)
 
             return
-        elif notification_status == NOTIFICATION_DELIVERED:
+        elif incoming_status == NOTIFICATION_DELIVERED:
             # Delivered messages should never have a status reason.
             notification.status_reason = None
 
         if notification.status not in (NOTIFICATION_SENDING, NOTIFICATION_PENDING):
-            notifications_dao.duplicate_update_warning(notification, notification_status)
+            notifications_dao.duplicate_update_warning(notification, incoming_status)
             return
 
-        notifications_dao._update_notification_status(notification=notification, status=notification_status)
+        notifications_dao._update_notification_status(notification=notification, status=incoming_status)
 
         if not aws_response_dict['success']:
             current_app.logger.info(
@@ -255,14 +250,14 @@ def process_ses_results(  # noqa: C901 (too complex 14 > 10)
         else:
             current_app.logger.info(
                 'SES callback return status of %s for notification: %s',
-                notification_status,
+                incoming_status,
                 notification.id,
             )
 
         log_notification_total_time(
             notification.id,
             notification.created_at,
-            notification_status,
+            incoming_status,
             'ses',
         )
 

app/clients/email/aws_ses.py

@@ -10,29 +10,31 @@
 from email.mime.text import MIMEText
 from email.mime.application import MIMEApplication
 
+from app.constants import NOTIFICATION_DELIVERED, NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_TEMPORARY_FAILURE
+
 ses_response_map = {
     'Permanent': {
         'message': 'Hard bounced',
         'success': False,
-        'notification_status': 'permanent-failure',
+        'notification_status': NOTIFICATION_PERMANENT_FAILURE,
         'notification_statistics_status': STATISTICS_FAILURE,
     },
     'Temporary': {
         'message': 'Soft bounced',
         'success': False,
-        'notification_status': 'temporary-failure',
+        'notification_status': NOTIFICATION_TEMPORARY_FAILURE,
         'notification_statistics_status': STATISTICS_FAILURE,
     },
     'Delivery': {
         'message': 'Delivered',
         'success': True,
-        'notification_status': 'delivered',
+        'notification_status': NOTIFICATION_DELIVERED,
         'notification_statistics_status': STATISTICS_DELIVERED,
     },
     'Complaint': {
         'message': 'Complaint',
         'success': True,
-        'notification_status': 'delivered',
+        'notification_status': NOTIFICATION_DELIVERED,
         'notification_statistics_status': STATISTICS_DELIVERED,
     },
 }

app/clients/sms/aws_pinpoint.py

@@ -20,6 +20,7 @@
     NOTIFICATION_PERMANENT_FAILURE,
     PINPOINT_PROVIDER,
     STATUS_REASON_BLOCKED,
+    STATUS_REASON_INVALID_NUMBER,
     STATUS_REASON_RETRYABLE,
     STATUS_REASON_UNDELIVERABLE,
 )
@@ -41,7 +42,7 @@ class AwsPinpointClient(SmsClient):
         'SUCCESSFUL': (NOTIFICATION_DELIVERED, None),
         'DELIVERED': (NOTIFICATION_DELIVERED, None),
         'PENDING': (NOTIFICATION_SENDING, None),
-        'INVALID': (NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_UNDELIVERABLE),
+        'INVALID': (NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_INVALID_NUMBER),
         'UNREACHABLE': (NOTIFICATION_TEMPORARY_FAILURE, STATUS_REASON_RETRYABLE),
         'UNKNOWN': (NOTIFICATION_TEMPORARY_FAILURE, STATUS_REASON_RETRYABLE),
         'BLOCKED': (NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_BLOCKED),

app/clients/sms/twilio.py

@@ -19,6 +19,7 @@
     NOTIFICATION_SENT,
     NOTIFICATION_TEMPORARY_FAILURE,
     STATUS_REASON_BLOCKED,
+    STATUS_REASON_INVALID_NUMBER,
     STATUS_REASON_RETRYABLE,
     STATUS_REASON_UNDELIVERABLE,
     STATUS_REASON_UNREACHABLE,
@@ -62,9 +63,9 @@ class TwilioSMSClient(SmsClient):
         '21610': TwilioStatus(21610, NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_BLOCKED),
         # 21612: 'Invalid to/from combo'
         '21612': TwilioStatus(21612, NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_UNREACHABLE),
-        '21614': TwilioStatus(21614, NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_UNREACHABLE),
+        '21614': TwilioStatus(21614, NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_INVALID_NUMBER),
         '21617': TwilioStatus(21617, NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_UNDELIVERABLE),
-        '21635': TwilioStatus(21635, NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_UNREACHABLE),
+        '21635': TwilioStatus(21635, NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_INVALID_NUMBER),
         '30001': TwilioStatus(30001, NOTIFICATION_TEMPORARY_FAILURE, STATUS_REASON_RETRYABLE),
         # 30002: 'Account suspended'
         '30002': TwilioStatus(30002, NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_UNDELIVERABLE),

tests/app/celery/test_process_pinpoint_receipt_tasks.py

@@ -16,6 +16,7 @@
     NOTIFICATION_PERMANENT_FAILURE,
     PINPOINT_PROVIDER,
     STATUS_REASON_BLOCKED,
+    STATUS_REASON_INVALID_NUMBER,
     STATUS_REASON_RETRYABLE,
     STATUS_REASON_UNDELIVERABLE,
 )
@@ -27,7 +28,7 @@
     [
         ('_SMS.BUFFERED', 'SUCCESSFUL', NOTIFICATION_DELIVERED, None),
         ('_SMS.SUCCESS', 'DELIVERED', NOTIFICATION_DELIVERED, None),
-        ('_SMS.FAILURE', 'INVALID', NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_UNDELIVERABLE),
+        ('_SMS.FAILURE', 'INVALID', NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_INVALID_NUMBER),
         ('_SMS.FAILURE', 'UNREACHABLE', NOTIFICATION_TEMPORARY_FAILURE, STATUS_REASON_RETRYABLE),
         ('_SMS.FAILURE', 'UNKNOWN', NOTIFICATION_TEMPORARY_FAILURE, STATUS_REASON_RETRYABLE),
         ('_SMS.FAILURE', 'BLOCKED', NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_BLOCKED),

tests/app/celery/test_process_ses_receipts_tasks.py

@@ -21,6 +21,7 @@
     NOTIFICATION_TEMPORARY_FAILURE,
     STATUS_REASON_RETRYABLE,
     STATUS_REASON_UNDELIVERABLE,
+    STATUS_REASON_UNREACHABLE,
 )
 from app.dao.notifications_dao import get_notification_by_id
 from app.models import Complaint, Notification, Service, Template
@@ -454,7 +455,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(
     assert process_ses_receipts_tasks.process_ses_results(ses_hard_bounce_callback(reference=ref)) is None
     db_notification = notify_db_session.session.get(Notification, notification_id)
     assert db_notification.status == NOTIFICATION_PERMANENT_FAILURE
-    assert db_notification.status_reason == STATUS_REASON_UNDELIVERABLE
+    assert db_notification.status_reason == STATUS_REASON_UNREACHABLE
     assert send_mock.called
 
 
@@ -618,12 +619,19 @@ def get_complaint_notification_and_email(mocker):
     return complaint, notification, recipient_email
 
 
-@pytest.mark.parametrize('status', (NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_TEMPORARY_FAILURE))
+@pytest.mark.parametrize(
+    'status, status_reason',
+    (
+        (NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_UNREACHABLE),
+        (NOTIFICATION_TEMPORARY_FAILURE, STATUS_REASON_RETRYABLE),
+    ),
+)
 def test_process_ses_results_no_bounce_regression(
     notify_db_session,
     sample_template,
     sample_notification,
     status,
+    status_reason,
 ):
     """
     If a bounce status has been persisted for a notificaiton, no further status updates should occur.
@@ -633,7 +641,7 @@ def test_process_ses_results_no_bounce_regression(
     notification = sample_notification(
         template=sample_template(template_type=EMAIL_TYPE),
         status=status,
-        status_reason='bounce',
+        status_reason=status_reason,
         reference=str(uuid4()),
     )
 
@@ -644,4 +652,4 @@ def test_process_ses_results_no_bounce_regression(
 
     notify_db_session.session.refresh(notification)
     assert notification.status == status, 'The status should not have changed.'
-    assert notification.status_reason == 'bounce'
+    assert notification.status_reason == status_reason