#2011 - Eliminate Null Status Reason

app/celery/process_delivery_status_result_tasks.py

@@ -15,7 +15,7 @@
 from app.constants import DELIVERY_STATUS_CALLBACK_TYPE, NOTIFICATION_DELIVERED
 from app.dao.notifications_dao import (
     dao_get_notification_by_reference,
-    dao_update_notification_delivery_status,
+    dao_update_sms_notification_delivery_status,
 )
 from app.dao.service_callback_dao import dao_get_callback_include_payload_status
 from app.models import Notification
@@ -50,7 +50,9 @@ def process_delivery_status(
         statsd_client.incr('clients.sms.twilio.status_update.error')
         raise
 
-    notification_platform_status: SmsStatusRecord = get_notification_platform_status(provider, sqs_message.get('body'))
+    notification_platform_status: SmsStatusRecord = get_notification_platform_status(
+        provider, sqs_message.get('body', '')
+    )
 
     current_app.logger.info(
         'Processing %s result.  | reference: %s | notification_status: %s | '
@@ -196,8 +198,8 @@ def sms_status_update(
 
     Args:
         sms_status (SmsStatusRecord): The status record update
-        event_timestamp (str | None, optional): Timestamp the event came in. Defaults to None.
-        event_in_seconds (int, optional): How many seconds it has retried. Defaults to -1.
+        event_timestamp (str | None, optional): Timestamp the Pinpoint event came in. Defaults to None.
+        event_in_seconds (int, optional): How many seconds Twilio updates have retried. Defaults to 300
 
     Raises:
         NonRetryableException: Unable to update the notification
@@ -219,7 +221,7 @@ def sms_status_update(
         sms_status.status_reason = None
 
     try:
-        notification: Notification = dao_update_notification_delivery_status(
+        notification: Notification = dao_update_sms_notification_delivery_status(
             notification_id=notification.id,
             notification_type=notification.notification_type,
             new_status=sms_status.status,

app/clients/sms/aws_pinpoint.py

@@ -35,30 +35,30 @@ class AwsPinpointException(SmsClientResponseException):
     pass
 
 
-# This is not an exhaustive list of all possible record statuses.  See the documentation linked below.
-_sms_record_status_mapping = {
-    'SUCCESSFUL': (NOTIFICATION_DELIVERED, None),
-    'DELIVERED': (NOTIFICATION_DELIVERED, None),
-    'PENDING': (NOTIFICATION_SENDING, None),
-    'INVALID': (NOTIFICATION_TECHNICAL_FAILURE, UNEXPECTED_PROVIDER_RESULT),
-    'UNREACHABLE': (NOTIFICATION_TEMPORARY_FAILURE, RETRYABLE_AWS_RESPONSE),
-    'UNKNOWN': (NOTIFICATION_TEMPORARY_FAILURE, RETRYABLE_AWS_RESPONSE),
-    'BLOCKED': (NOTIFICATION_PERMANENT_FAILURE, BLOCKED_MESSAGE),
-    'CARRIER_UNREACHABLE': (NOTIFICATION_TEMPORARY_FAILURE, RETRYABLE_AWS_RESPONSE),
-    'SPAM': (NOTIFICATION_PERMANENT_FAILURE, REPORTED_AS_SPAM),
-    'INVALID_MESSAGE': (NOTIFICATION_TECHNICAL_FAILURE, UNEXPECTED_PROVIDER_RESULT),
-    'CARRIER_BLOCKED': (NOTIFICATION_PERMANENT_FAILURE, BLOCKED_MESSAGE),
-    'TTL_EXPIRED': (NOTIFICATION_TEMPORARY_FAILURE, RETRYABLE_AWS_RESPONSE),
-    'MAX_PRICE_EXCEEDED': (NOTIFICATION_TECHNICAL_FAILURE, PRICE_THRESHOLD_EXCEEDED),
-    'OPTED_OUT': (NOTIFICATION_PREFERENCES_DECLINED, OPT_OUT_MESSAGE),
-}
-
-
 class AwsPinpointClient(SmsClient):
     """
     AwsSns pinpoint client
     """
 
+    # https://docs.aws.amazon.com/pinpoint/latest/developerguide/event-streams-data-sms.html
+    # Maps record_status to (status, status_reason)
+    _sms_record_status_mapping = {
+        'SUCCESSFUL': (NOTIFICATION_DELIVERED, None),
+        'DELIVERED': (NOTIFICATION_DELIVERED, None),
+        'PENDING': (NOTIFICATION_SENDING, None),
+        'INVALID': (NOTIFICATION_TECHNICAL_FAILURE, UNEXPECTED_PROVIDER_RESULT),
+        'UNREACHABLE': (NOTIFICATION_TEMPORARY_FAILURE, RETRYABLE_AWS_RESPONSE),
+        'UNKNOWN': (NOTIFICATION_TEMPORARY_FAILURE, RETRYABLE_AWS_RESPONSE),
+        'BLOCKED': (NOTIFICATION_PERMANENT_FAILURE, BLOCKED_MESSAGE),
+        'CARRIER_UNREACHABLE': (NOTIFICATION_TEMPORARY_FAILURE, RETRYABLE_AWS_RESPONSE),
+        'SPAM': (NOTIFICATION_PERMANENT_FAILURE, REPORTED_AS_SPAM),
+        'INVALID_MESSAGE': (NOTIFICATION_TECHNICAL_FAILURE, UNEXPECTED_PROVIDER_RESULT),
+        'CARRIER_BLOCKED': (NOTIFICATION_PERMANENT_FAILURE, BLOCKED_MESSAGE),
+        'TTL_EXPIRED': (NOTIFICATION_TEMPORARY_FAILURE, RETRYABLE_AWS_RESPONSE),
+        'MAX_PRICE_EXCEEDED': (NOTIFICATION_TECHNICAL_FAILURE, PRICE_THRESHOLD_EXCEEDED),
+        'OPTED_OUT': (NOTIFICATION_PREFERENCES_DECLINED, OPT_OUT_MESSAGE),
+    }
+
     def __init__(self):
         self.name = 'pinpoint'
 
@@ -153,11 +153,11 @@ def _validate_response(
             raise AwsPinpointException(error_message)
 
     def _get_status_mapping(self, record_status) -> Tuple[str, str]:
-        if record_status not in _sms_record_status_mapping:
+        if record_status not in self._sms_record_status_mapping:
             # This is a programming error, or Pinpoint's response format has changed.
             self.logger.critical('Unanticipated Pinpoint record status: %s', record_status)
 
-        return _sms_record_status_mapping.get(
+        return self._sms_record_status_mapping.get(
             record_status, (NOTIFICATION_TECHNICAL_FAILURE, UNEXPECTED_PROVIDER_RESULT)
         )
 
@@ -209,7 +209,7 @@ def translate_delivery_status(
         event_type = delivery_status_message['event_type']
         record_status = pinpoint_attributes['record_status']
         status, status_reason = self._get_aws_status(event_type, record_status)
-        notification_platform_status = SmsStatusRecord(
+        return SmsStatusRecord(
             None,
             pinpoint_attributes['message_id'],
             status,
@@ -219,5 +219,3 @@ def translate_delivery_status(
             delivery_status_message['metrics']['price_in_millicents_usd'],
             datetime.fromtimestamp(delivery_status_message['event_timestamp'] / 1000),
         )
-
-        return notification_platform_status

app/clients/sms/twilio.py

@@ -262,7 +262,7 @@ def translate_delivery_status(
         provider_updated_at = (
             self._translate_raw_dlr_done_date(raw_dlr_done_date_list[0]) if raw_dlr_done_date_list else None
         )
-        notification_platform_status = SmsStatusRecord(
+        return SmsStatusRecord(
             decoded_msg,
             message_sid,
             status,
@@ -271,8 +271,6 @@ def translate_delivery_status(
             provider_updated_at=provider_updated_at,
         )
 
-        return notification_platform_status
-
     def _translate_raw_dlr_done_date(self, done_date: str) -> datetime:
         """Translate RawDlrDoneDate into a timezone unaware datetime object.
 

app/constants.py

@@ -16,7 +16,7 @@
 LETTER_TYPE = 'letter'
 PUSH_TYPE = 'push'
 SMS_TYPE = 'sms'
-NOTIFICATION_TYPE = [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE]
+NOTIFICATION_TYPE = (EMAIL_TYPE, SMS_TYPE, LETTER_TYPE)
 
 # Notification status
 NOTIFICATION_CANCELLED = 'cancelled'
@@ -36,7 +36,7 @@
 NOTIFICATION_CONTAINS_PII = 'pii-check-failed'
 NOTIFICATION_PREFERENCES_DECLINED = 'preferences-declined'
 
-NOTIFICATION_STATUS_TYPES = [
+NOTIFICATION_STATUS_TYPES = (
     NOTIFICATION_CANCELLED,
     NOTIFICATION_CREATED,
     NOTIFICATION_SENDING,
@@ -53,9 +53,9 @@
     NOTIFICATION_RETURNED_LETTER,
     NOTIFICATION_CONTAINS_PII,
     NOTIFICATION_PREFERENCES_DECLINED,
-]
+)
 
-NOTIFICATION_STATUS_TYPES_COMPLETED = [
+NOTIFICATION_STATUS_TYPES_COMPLETED = (
     NOTIFICATION_SENT,
     NOTIFICATION_DELIVERED,
     NOTIFICATION_FAILED,
@@ -65,9 +65,9 @@
     NOTIFICATION_RETURNED_LETTER,
     NOTIFICATION_CANCELLED,
     NOTIFICATION_PREFERENCES_DECLINED,
-]
+)
 
-NOTIFICATION_STATUS_TYPES_FAILED = [
+NOTIFICATION_STATUS_TYPES_FAILED = (
     NOTIFICATION_TECHNICAL_FAILURE,
     NOTIFICATION_TEMPORARY_FAILURE,
     NOTIFICATION_PERMANENT_FAILURE,
@@ -76,37 +76,38 @@
     NOTIFICATION_RETURNED_LETTER,
     NOTIFICATION_CONTAINS_PII,
     NOTIFICATION_PREFERENCES_DECLINED,
-]
+)
 
-NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS = [
+NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS = (
     NOTIFICATION_SENDING,
     NOTIFICATION_DELIVERED,
     NOTIFICATION_RETURNED_LETTER,
-]
+)
 
-NOTIFICATION_STATUS_TYPES_BILLABLE = [
+NOTIFICATION_STATUS_TYPES_BILLABLE = (
     NOTIFICATION_SENDING,
     NOTIFICATION_SENT,
     NOTIFICATION_DELIVERED,
     NOTIFICATION_FAILED,
     NOTIFICATION_TEMPORARY_FAILURE,
     NOTIFICATION_PERMANENT_FAILURE,
     NOTIFICATION_RETURNED_LETTER,
-]
+)
 
 # VA Profile types
 MOBILE_TYPE = 'mobile'
 
 # Template
-TEMPLATE_TYPES = [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE]
+TEMPLATE_TYPES = (SMS_TYPE, EMAIL_TYPE, LETTER_TYPE)
 TEMPLATE_PROCESS_NORMAL = 'normal'
 TEMPLATE_PROCESS_PRIORITY = 'priority'
-TEMPLATE_PROCESS_TYPE = [TEMPLATE_PROCESS_NORMAL, TEMPLATE_PROCESS_PRIORITY]
+TEMPLATE_PROCESS_TYPE = (TEMPLATE_PROCESS_NORMAL, TEMPLATE_PROCESS_PRIORITY)
 
 # Callbacks
 DELIVERY_STATUS_CALLBACK_TYPE = 'delivery_status'
 COMPLAINT_CALLBACK_TYPE = 'complaint'
 INBOUND_SMS_CALLBACK_TYPE = 'inbound_sms'
+# list for backwards compatibility
 SERVICE_CALLBACK_TYPES = [DELIVERY_STATUS_CALLBACK_TYPE, COMPLAINT_CALLBACK_TYPE, INBOUND_SMS_CALLBACK_TYPE]
 WEBHOOK_CHANNEL_TYPE = 'webhook'
 QUEUE_CHANNEL_TYPE = 'queue'
@@ -117,13 +118,13 @@
 BRANDING_BOTH = 'both'
 BRANDING_ORG_BANNER = 'org_banner'
 BRANDING_NO_BRANDING = 'no_branding'
-BRANDING_TYPES = [BRANDING_ORG, BRANDING_BOTH, BRANDING_ORG_BANNER, BRANDING_NO_BRANDING]
+BRANDING_TYPES = (BRANDING_ORG, BRANDING_BOTH, BRANDING_ORG_BANNER, BRANDING_NO_BRANDING)
 
 # Letters
 DVLA_RESPONSE_STATUS_SENT = 'Sent'
 FIRST_CLASS = 'first'
 SECOND_CLASS = 'second'
-POSTAGE_TYPES = [FIRST_CLASS, SECOND_CLASS]
+POSTAGE_TYPES = (FIRST_CLASS, SECOND_CLASS)
 RESOLVE_POSTAGE_FOR_FILE_NAME = {FIRST_CLASS: 1, SECOND_CLASS: 2}
 NOTIFICATION_STATUS_LETTER_ACCEPTED = 'accepted'
 NOTIFICATION_STATUS_LETTER_RECEIVED = 'received'
@@ -139,7 +140,7 @@
 EDIT_FOLDER_PERMISSIONS = 'edit_folder_permissions'
 UPLOAD_LETTERS = 'upload_letters'
 
-SERVICE_PERMISSION_TYPES = [
+SERVICE_PERMISSION_TYPES = (
     EMAIL_TYPE,
     SMS_TYPE,
     LETTER_TYPE,
@@ -152,7 +153,7 @@
     UPLOAD_DOCUMENT,
     EDIT_FOLDER_PERMISSIONS,
     UPLOAD_LETTERS,
-]
+)
 
 
 # Service Permissions
@@ -169,7 +170,7 @@
 
 # Default permissions for a service
 # Do not confuse this with DEFAULT_SERVICE_NOTIFICATION_PERMISSIONS for app/dao/services_dao.py.
-DEFAULT_SERVICE_MANAGEMENT_PERMISSIONS = [
+DEFAULT_SERVICE_MANAGEMENT_PERMISSIONS = (
     MANAGE_USERS,
     MANAGE_TEMPLATES,
     MANAGE_SETTINGS,
@@ -178,17 +179,17 @@
     SEND_LETTERS,
     MANAGE_API_KEYS,
     VIEW_ACTIVITY,
-]
+)
 
 # Do not confuse this with DEFAULT_SERVICE_MANAGEMENT_PERMISSIONS for app/dao/permissions_dao.py.
-DEFAULT_SERVICE_NOTIFICATION_PERMISSIONS = [
+DEFAULT_SERVICE_NOTIFICATION_PERMISSIONS = (
     SMS_TYPE,
     EMAIL_TYPE,
     INTERNATIONAL_SMS_TYPE,
-]
+)
 
 # List of permissions
-PERMISSION_LIST = [
+PERMISSION_LIST = (
     MANAGE_USERS,
     MANAGE_TEMPLATES,
     EDIT_TEMPLATES,
@@ -199,7 +200,7 @@
     MANAGE_API_KEYS,
     PLATFORM_ADMIN,
     VIEW_ACTIVITY,
-]
+)
 
 # API key types
 KEY_TYPE_NORMAL = 'normal'
@@ -211,19 +212,17 @@
 EMAIL_AUTH_TYPE = 'email_auth'
 
 # Orangaization
-ORGANISATION_TYPES = [
-    'other',
-]
+ORGANISATION_TYPES = ('other',)
 
 # Deprecated but necessary
 BRANDING_GOVUK = 'govuk'  # Deprecated outside migrations
 INVITE_PENDING = 'pending'
 INVITE_ACCEPTED = 'accepted'
 INVITE_CANCELLED = 'cancelled'
-INVITED_USER_STATUS_TYPES = [INVITE_PENDING, INVITE_ACCEPTED, INVITE_CANCELLED]
+INVITED_USER_STATUS_TYPES = (INVITE_PENDING, INVITE_ACCEPTED, INVITE_CANCELLED)
 
 # Whitelist
-WHITELIST_RECIPIENT_TYPE = [MOBILE_TYPE, EMAIL_TYPE]
+WHITELIST_RECIPIENT_TYPE = (MOBILE_TYPE, EMAIL_TYPE)
 
 # Providers
 MMG_PROVIDER = 'mmg'
@@ -244,7 +243,7 @@
 JOB_STATUS_SENT_TO_DVLA = 'sent to dvla'
 JOB_STATUS_ERROR = 'error'
 
-JOB_STATUS_TYPES = [
+JOB_STATUS_TYPES = (
     JOB_STATUS_PENDING,
     JOB_STATUS_IN_PROGRESS,
     JOB_STATUS_FINISHED,
@@ -254,4 +253,4 @@
     JOB_STATUS_READY_TO_SEND,
     JOB_STATUS_SENT_TO_DVLA,
     JOB_STATUS_ERROR,
-]
+)

app/dao/notifications_dao.py

@@ -289,7 +289,7 @@ def _update_notification_status(
     notification: Notification, status: str, status_reason: str | None = None
 ) -> Notification:
     """
-    Update the notification EMAIL status if it should be updated.
+    Update the notification status if it should be updated.
 
     Args:
         notification (Notification): The notification to update.
@@ -365,7 +365,7 @@ def update_notification_status_by_id(
 
 
 @statsd(namespace='dao')
-def dao_update_notification_delivery_status(
+def dao_update_sms_notification_delivery_status(
     notification_id: UUID,
     notification_type: str,
     new_status: str,
@@ -402,7 +402,9 @@ def dao_update_notification_delivery_status(
         )
         current_app.logger.debug('sms delivery status statement: %s', stmt)
     else:
-        raise NotImplementedError(f'dao_update_notification_delivery_status not configured for {notification_type} yet')
+        raise NotImplementedError(
+            f'dao_update_sms_notification_delivery_status not configured for {notification_type} yet'
+        )
 
     try:
         db.session.execute(stmt)
@@ -446,7 +448,7 @@ def dao_update_notification_by_id(
     **kwargs,
 ) -> Optional[Notification]:
     """
-    Update the SMS notification by ID, ensure kwargs paramaters are named appropriately according to the notification model.
+    Update the notification by ID, ensure kwargs paramaters are named appropriately according to the notification model.
 
     Args:
         notification_id (str): The notification uuid in string form