diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index 2e8086d50..36692d29c 100644 --- a/app/clients/sms/firetext.py +++ b/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'}, diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index d959ecb13..a5a40db8a 100644 --- a/app/dao/notifications_dao.py +++ b/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 diff --git a/tests/app/notifications/test_process_client_response.py b/tests/app/notifications/test_process_client_response.py index 9457528e7..859e79eac 100644 --- a/tests/app/notifications/test_process_client_response.py +++ b/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,