Merge pull request #2813 from alphagov/firetext-check-code-on-failure-only

Check failure code on failure only.

app/clients/sms/firetext.py

@@ -20,6 +20,7 @@ firetext_responses = {
 }
 
 firetext_codes = {
+    # code '000' means 'No errors reported'
     '101': {'status': 'permanent-failure', 'reason': 'Unknown Subscriber'},
     '102': {'status': 'temporary-failure', 'reason': 'Absent Subscriber'},
     '103': {'status': 'temporary-failure', 'reason': 'Subscriber Busy'},

app/dao/notifications_dao.py

@@ -91,16 +91,19 @@ def dao_create_notification(notification):
 
 
 def _decide_permanent_temporary_failure(status, notification, code=None):
-    # Firetext will send pending, then send either succes or fail.
-    # If we go from pending to failure we need to set failure type as temporary-failure,
-    # unless we get a detailed code from firetext. Then we should use that code to set status instead.
-    if code and code != '000':
+    # If we get failure status from Firetext, we want to know if this is temporary or permanent failure.
+    # So we check the failure code to learn that.
+    # If there is no failure code, or we do not recognise the failure code, we do the following:
+    # if notifitcation goes form status pending to status failure, we mark it as temporary failure;
+    # if notification goes straight to status failure, we mark it as permanent failure.
+    if status == NOTIFICATION_PERMANENT_FAILURE and code not in [None, '000']:
         try:
             status, reason = get_message_status_and_reason_from_firetext_code(code)
             current_app.logger.info(f'Updating notification id {notification.id} to status {status}, reason: {reason}')
             return status
         except KeyError:
             current_app.logger.warning(f'Failure code {code} from Firetext not recognised')
+    # fallback option:
     if notification.status == NOTIFICATION_PENDING and status == NOTIFICATION_PERMANENT_FAILURE:
         status = NOTIFICATION_TEMPORARY_FAILURE
     return status

tests/app/notifications/test_process_client_response.py

@@ -83,6 +83,19 @@ def test_process_sms_client_response_updates_notification_status_when_called_sec
     assert sample_notification.status == expected_notification_status
 
 
+@pytest.mark.parametrize('code', ['102', None, '000'])
+def test_process_sms_client_response_updates_notification_status_to_pending_with_and_without_failire_code_present(
+    sample_notification,
+    mocker,
+    code
+):
+    sample_notification.status = 'sending'
+
+    process_sms_client_response('2', str(sample_notification.id), 'Firetext', code)
+
+    assert sample_notification.status == 'pending'
+
+
 def test_process_sms_client_response_updates_notification_status_when_code_unknown(
     sample_notification,
     mocker,