diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index d659f44f1..eb7287c0e 100644 --- a/app/clients/sms/firetext.py +++ b/app/clients/sms/firetext.py @@ -21,6 +21,7 @@ firetext_responses = { firetext_codes = { # code '000' means 'No errors reported' + '000': {'status': 'temporary-failure', 'reason': 'No error 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 599468810..a21fa757f 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -88,21 +88,21 @@ def dao_create_notification(notification): def _decide_permanent_temporary_failure(status, notification, detailed_status_code=None): - # 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 detailed_status_code not in [None, '000']: - try: - status, reason = get_message_status_and_reason_from_firetext_code(detailed_status_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 {detailed_status_code} from Firetext not recognised') - # fallback option: - if notification.status == NOTIFICATION_PENDING and status == NOTIFICATION_PERMANENT_FAILURE: - status = NOTIFICATION_TEMPORARY_FAILURE + # Firetext will send us a pending status, followed by a success or failure status. + # When we get a failure status we need to look at the detailed_status_code to determine if the failure type + # is a permanent-failure or temporary-failure. + if notification.sent_by == 'firetext': + if status == NOTIFICATION_PERMANENT_FAILURE and detailed_status_code: + try: + status, reason = get_message_status_and_reason_from_firetext_code(detailed_status_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 {detailed_status_code} from Firetext not recognised') + # fallback option: + if status == NOTIFICATION_PERMANENT_FAILURE and notification.status == NOTIFICATION_PENDING: + status = NOTIFICATION_TEMPORARY_FAILURE return status diff --git a/tests/app/clients/test_firetext.py b/tests/app/clients/test_firetext.py index 3494ef83b..a95655fab 100644 --- a/tests/app/clients/test_firetext.py +++ b/tests/app/clients/test_firetext.py @@ -9,7 +9,7 @@ from app.clients.sms.firetext import get_firetext_responses, SmsClientResponseEx @pytest.mark.parametrize('detailed_status_code, result', [ - (None, ('delivered', None)), ('000', ('delivered', None)) + (None, ('delivered', None)), ('000', ('delivered', 'No error reported')) ]) def test_get_firetext_responses_should_return_correct_details_for_delivery(detailed_status_code, result): assert get_firetext_responses('0', detailed_status_code) == result diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 322600c28..a7efba937 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -216,7 +216,7 @@ def test_should_by_able_to_update_status_by_id_from_pending_to_delivered(sample_ def test_should_by_able_to_update_status_by_id_from_pending_to_temporary_failure(sample_template, sample_job): - notification = create_notification(template=sample_template, job=sample_job, status='sending') + notification = create_notification(template=sample_template, job=sample_job, status='sending', sent_by='firetext') assert update_notification_status_by_id(notification_id=notification.id, status='pending') assert Notification.query.get(notification.id).status == 'pending' diff --git a/tests/app/notifications/test_process_client_response.py b/tests/app/notifications/test_process_client_response.py index d216b22d9..926599d01 100644 --- a/tests/app/notifications/test_process_client_response.py +++ b/tests/app/notifications/test_process_client_response.py @@ -67,7 +67,7 @@ def test_process_sms_client_response_updates_notification_status( ('101', 'permanent-failure', 'Unknown Subscriber'), ('102', 'temporary-failure', 'Absent Subscriber'), (None, 'temporary-failure', None), - ('000', 'temporary-failure', None) + ('000', 'temporary-failure', 'No error reported') ]) def test_process_sms_client_response_updates_notification_status_when_called_second_time( sample_notification, @@ -82,7 +82,7 @@ def test_process_sms_client_response_updates_notification_status_when_called_sec process_sms_client_response('1', str(sample_notification.id), 'Firetext', detailed_status_code) - if detailed_status_code and detailed_status_code != '000': + if detailed_status_code: message = f'Updating notification id {sample_notification.id} to status {expected_notification_status}, reason: {reason}' # noqa mock_logger.assert_called_with(message)