Use firetext response code to see if temporary or permanent failure if available

This commit is contained in:
Pea Tyczynska
2020-04-14 15:55:17 +01:00
parent 91fe68eed4
commit a4507dbb3f
4 changed files with 37 additions and 12 deletions

View File

@@ -7,7 +7,7 @@ from notifications_utils.template import SMSMessageTemplate
from app import notify_celery, statsd_client from app import notify_celery, statsd_client
from app.clients import ClientException from app.clients import ClientException
from app.clients.sms.firetext import get_firetext_responses, get_message_status_from_firetext_code from app.clients.sms.firetext import get_firetext_responses
from app.clients.sms.mmg import get_mmg_responses from app.clients.sms.mmg import get_mmg_responses
from app.celery.service_callback_tasks import send_delivery_status_to_service, create_delivery_status_callback_data from app.celery.service_callback_tasks import send_delivery_status_to_service, create_delivery_status_callback_data
from app.config import QueueNames from app.config import QueueNames
@@ -51,16 +51,18 @@ def process_sms_client_response(self, status, provider_reference, client_name, c
_process_for_status( _process_for_status(
notification_status=notification_status, notification_status=notification_status,
client_name=client_name, client_name=client_name,
provider_reference=provider_reference provider_reference=provider_reference,
code=code
) )
def _process_for_status(notification_status, client_name, provider_reference): def _process_for_status(notification_status, client_name, provider_reference, code=None):
# record stats # record stats
notification = notifications_dao.update_notification_status_by_id( notification = notifications_dao.update_notification_status_by_id(
notification_id=provider_reference, notification_id=provider_reference,
status=notification_status, status=notification_status,
sent_by=client_name.lower() sent_by=client_name.lower(),
code=code
) )
if not notification: if not notification:
return return

View File

@@ -52,6 +52,7 @@ from app.models import (
) )
from app.utils import get_london_midnight_in_utc from app.utils import get_london_midnight_in_utc
from app.utils import midnight_n_days_ago, escape_special_characters from app.utils import midnight_n_days_ago, escape_special_characters
from app.clients.sms.firetext import get_message_status_from_firetext_code
@statsd(namespace="dao") @statsd(namespace="dao")
@@ -89,11 +90,15 @@ def dao_create_notification(notification):
db.session.add(notification) db.session.add(notification)
def _decide_permanent_temporary_failure(current_status, status): def _decide_permanent_temporary_failure(current_status, status, code=None):
# Firetext will send pending, then send either succes or fail. # Firetext will send pending, then send either succes or fail.
# If we go from pending to delivered we need to set failure type as temporary-failure # If we go from pending to failure we need to set failure type as temporary-failure
# if we get a detailed code from firetext, we should use that code to set status instead
if current_status == NOTIFICATION_PENDING and status == NOTIFICATION_PERMANENT_FAILURE: if current_status == NOTIFICATION_PENDING and status == NOTIFICATION_PERMANENT_FAILURE:
status = NOTIFICATION_TEMPORARY_FAILURE if code:
status = get_message_status_from_firetext_code(code)
else:
status = NOTIFICATION_TEMPORARY_FAILURE
return status return status
@@ -102,8 +107,8 @@ def country_records_delivery(phone_prefix):
return dlr and dlr.lower() == 'yes' return dlr and dlr.lower() == 'yes'
def _update_notification_status(notification, status): def _update_notification_status(notification, status, code=None):
status = _decide_permanent_temporary_failure(current_status=notification.status, status=status) status = _decide_permanent_temporary_failure(current_status=notification.status, status=status, code=code)
notification.status = status notification.status = status
dao_update_notification(notification) dao_update_notification(notification)
return notification return notification
@@ -111,7 +116,7 @@ def _update_notification_status(notification, status):
@statsd(namespace="dao") @statsd(namespace="dao")
@transactional @transactional
def update_notification_status_by_id(notification_id, status, sent_by=None): def update_notification_status_by_id(notification_id, status, sent_by=None, code=None):
notification = Notification.query.with_for_update().filter(Notification.id == notification_id).first() notification = Notification.query.with_for_update().filter(Notification.id == notification_id).first()
if not notification: if not notification:
@@ -137,7 +142,8 @@ def update_notification_status_by_id(notification_id, status, sent_by=None):
notification.sent_by = sent_by notification.sent_by = sent_by
return _update_notification_status( return _update_notification_status(
notification=notification, notification=notification,
status=status status=status,
code=code
) )

View File

@@ -147,7 +147,7 @@ def test_firetext_callback_should_return_200_and_call_task_with_valid_data(clien
assert json_resp['result'] == 'success' assert json_resp['result'] == 'success'
mock_celery.assert_called_once_with( mock_celery.assert_called_once_with(
['0', 'notification_id', 'Firetext'], ['0', 'notification_id', 'Firetext', None],
queue='sms-callbacks', queue='sms-callbacks',
) )

View File

@@ -57,6 +57,23 @@ def test_process_sms_client_response_updates_notification_status(
assert sample_notification.status == expected_notification_status assert sample_notification.status == expected_notification_status
@pytest.mark.parametrize('code, expected_notification_status', [
('101', 'permanent-failure'),
('102', 'temporary-failure'),
])
def test_process_sms_client_response_updates_notification_status_when_called_second_time(
sample_notification,
mocker,
code,
expected_notification_status,
):
sample_notification.status = 'sending'
process_sms_client_response('2', str(sample_notification.id), 'Firetext')
process_sms_client_response('1', str(sample_notification.id), 'Firetext', code)
assert sample_notification.status == expected_notification_status
def test_sms_response_does_not_send_callback_if_notification_is_not_in_the_db(sample_service, mocker): def test_sms_response_does_not_send_callback_if_notification_is_not_in_the_db(sample_service, mocker):
mocker.patch( mocker.patch(
'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service', 'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service',