darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-02-01 23:55:58 -05:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1635 from alphagov/remove-failed-as-a-status

Browse Source 
Remove failed as a possible status
This commit is contained in:
Rebecca Law
2018-02-14 14:35:01 +00:00
committed by GitHub
parent 15c2a9461e fa8cd780b0
commit 12352a4153
7 changed files with 72 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
app/clients/sms/firetext.py
View File

@@ -5,7 +5,6 @@ from monotonic import monotonic
from requests import request, RequestException
from app.clients.sms import (SmsClient, SmsClientResponseException)
from app.clients import STATISTICS_DELIVERED, STATISTICS_FAILURE
logger = logging.getLogger(__name__)
@@ -15,24 +14,9 @@ logger = logging.getLogger(__name__)
# the notification status to temporary-failure rather than permanent failure.
# See the code in the notification_dao.update_notifications_status_by_id
firetext_responses = {
'0': {
"message": 'Delivered',
"notification_statistics_status": STATISTICS_DELIVERED,
"success": True,
"notification_status": 'delivered'
},
'1': {
"message": 'Declined',
"success": False,
"notification_statistics_status": STATISTICS_FAILURE,
"notification_status": 'permanent-failure'
},
'2': {
"message": 'Undelivered (Pending with Network)',
"success": True,
"notification_statistics_status": None,
"notification_status": 'pending'
}
'0': 'delivered',
'1': 'permanent-failure',
'2': 'pending'
}

37
app/clients/sms/mmg.py
View File

@@ -1,45 +1,18 @@
import json
from monotonic import monotonic
from requests import (request, RequestException)
from app.clients import (STATISTICS_DELIVERED, STATISTICS_FAILURE)
from app.clients.sms import (SmsClient, SmsClientResponseException)
mmg_response_map = {
'2': {
"message": ' Permanent failure',
"notification_statistics_status": STATISTICS_FAILURE,
"success": False,
"notification_status": 'permanent-failure'
},
'3': {
"message": 'Delivered',
"notification_statistics_status": STATISTICS_DELIVERED,
"success": True,
"notification_status": 'delivered'
},
'4': {
"message": ' Temporary failure',
"notification_statistics_status": STATISTICS_FAILURE,
"success": False,
"notification_status": 'temporary-failure'
},
'5': {
"message": 'Permanent failure',
"notification_statistics_status": STATISTICS_FAILURE,
"success": False,
"notification_status": 'permanent-failure'
},
'default': {
"message": 'Declined',
"success": False,
"notification_statistics_status": STATISTICS_FAILURE,
"notification_status": 'failed'
}
'2': 'permanent-failure',
'3': 'delivered',
'4': 'temporary-failure',
'5': 'permanent-failure'
}
def get_mmg_responses(status):
return mmg_response_map.get(status, mmg_response_map.get('default'))
return mmg_response_map[status]
class MMGClientResponseException(SmsClientResponseException):

30
app/notifications/process_client_response.py
View File

@@ -4,6 +4,7 @@ from datetime import datetime
from flask import current_app
from app import statsd_client
from app.clients import ClientException
from app.dao import notifications_dao
from app.clients.sms.firetext import get_firetext_responses
from app.clients.sms.mmg import get_mmg_responses
@@ -39,8 +40,8 @@ def process_sms_client_response(status, reference, client_name):
try:
uuid.UUID(reference, version=4)
except ValueError:
message = "{} callback with invalid reference {}".format(client_name, reference)
return success, message
errors = "{} callback with invalid reference {}".format(client_name, reference)
return success, errors
try:
response_parser = sms_response_mapper[client_name]
@@ -49,32 +50,27 @@ def process_sms_client_response(status, reference, client_name):
# validate status
try:
response_dict = response_parser(status)
notification_status = response_parser(status)
current_app.logger.info('{} callback return status of {} for reference: {}'.format(
client_name, status, reference)
)
except KeyError:
msg = "{} callback failed: status {} not found.".format(client_name, status)
return success, msg
_process_for_status(notification_status='technical-failure', client_name=client_name, reference=reference)
raise ClientException("{} callback failed: status {} not found.".format(client_name, status))
notification_status = response_dict['notification_status']
notification_status_message = response_dict['message']
notification_success = response_dict['success']
success = _process_for_status(notification_status=notification_status, client_name=client_name, reference=reference)
return success, errors
def _process_for_status(notification_status, client_name, reference):
# record stats
notification = notifications_dao.update_notification_status_by_id(reference, notification_status)
if not notification:
current_app.logger.warning("{} callback failed: notification {} either not found or already updated "
"from sending. Status {}".format(client_name,
reference,
notification_status_message))
return success, errors
if not notification_success:
current_app.logger.debug(
"{} delivery failed: notification {} has error found. Status {}".format(client_name,
reference,
notification_status_message))
notification_status))
return
statsd_client.incr('callback.{}.{}'.format(client_name.lower(), notification_status))
if notification.sent_at:
@@ -92,4 +88,4 @@ def process_sms_client_response(status, reference, client_name):
send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.CALLBACKS)
success = "{} callback succeeded. reference {} updated".format(client_name, reference)
return success, errors
return success

18
tests/app/clients/test_firetext.py
View File

@@ -9,27 +9,15 @@ from app.clients.sms.firetext import get_firetext_responses, SmsClientResponseEx
def test_should_return_correct_details_for_delivery():
response_dict = get_firetext_responses('0')
assert response_dict['message'] == 'Delivered'
assert response_dict['notification_status'] == 'delivered'
assert response_dict['notification_statistics_status'] == 'delivered'
assert response_dict['success']
get_firetext_responses('0') == 'delivered'
def test_should_return_correct_details_for_bounced():
response_dict = get_firetext_responses('1')
assert response_dict['message'] == 'Declined'
assert response_dict['notification_status'] == 'permanent-failure'
assert response_dict['notification_statistics_status'] == 'failure'
assert not response_dict['success']
get_firetext_responses('1') == 'permanent-failure'
def test_should_return_correct_details_for_complaint():
response_dict = get_firetext_responses('2')
assert response_dict['message'] == 'Undelivered (Pending with Network)'
assert response_dict['notification_status'] == 'pending'
assert response_dict['notification_statistics_status'] is None
assert response_dict['success']
get_firetext_responses('2') == 'pending'
def test_should_be_none_if_unrecognised_status_code():

29
tests/app/clients/test_mmg.py
View File

@@ -10,27 +10,22 @@ from app.clients.sms.mmg import get_mmg_responses, MMGClientResponseException
def test_should_return_correct_details_for_delivery():
response_dict = get_mmg_responses('3')
assert response_dict['message'] == 'Delivered'
assert response_dict['notification_status'] == 'delivered'
assert response_dict['notification_statistics_status'] == 'delivered'
assert response_dict['success']
get_mmg_responses('3') == 'delivered'
def test_should_return_correct_details_for_bounced():
response_dict = get_mmg_responses('50')
assert response_dict['message'] == 'Declined'
assert response_dict['notification_status'] == 'failed'
assert response_dict['notification_statistics_status'] == 'failure'
assert not response_dict['success']
def test_should_return_correct_details_for_temporary_failure():
get_mmg_responses('4') == 'temporary-failure'
def test_should_be_none_if_unrecognised_status_code():
response_dict = get_mmg_responses('blah')
assert response_dict['message'] == 'Declined'
assert response_dict['notification_status'] == 'failed'
assert response_dict['notification_statistics_status'] == 'failure'
assert not response_dict['success']
@pytest.mark.parametrize('status', ['5', '2'])
def test_should_return_correct_details_for_bounced(status):
get_mmg_responses(status) == 'permanent-failure'
def test_should_be_raise_if_unrecognised_status_code():
with pytest.raises(KeyError) as e:
get_mmg_responses('99')
assert '99' in str(e.value)
def test_send_sms_successful_returns_mmg_response(notify_api, mocker):

44
tests/app/notifications/rest/test_callbacks.py
View File

@@ -2,10 +2,12 @@ import uuid
from datetime import datetime
import pytest
from flask import json
from freezegun import freeze_time
import app.celery.tasks
from app.clients import ClientException
from app.dao.notifications_dao import (
get_notification_by_id
)
@@ -161,18 +163,20 @@ def test_firetext_callback_should_return_400_if_no_status(client, mocker):
assert json_resp['message'] == ['Firetext callback failed: status missing']
def test_firetext_callback_should_return_400_if_unknown_status(client, mocker):
def test_firetext_callback_should_set_status_technical_failure_if_status_unknown(
client, notify_db, notify_db_session, mocker):
notification = create_sample_notification(
notify_db, notify_db_session, status='sending', sent_at=datetime.utcnow()
)
mocker.patch('app.statsd_client.incr')
data = 'mobile=441234123123&status=99&time=2016-03-10 14:17:00&reference={}'.format(uuid.uuid4())
response = firetext_post(client, data)
json_resp = json.loads(response.get_data(as_text=True))
assert response.status_code == 400
assert json_resp['result'] == 'error'
assert json_resp['message'] == 'Firetext callback failed: status 99 not found.'
data = 'mobile=441234123123&status=99&time=2016-03-10 14:17:00&reference={}'.format(notification.id)
with pytest.raises(ClientException) as e:
firetext_post(client, data)
assert get_notification_by_id(notification.id).status == 'technical-failure'
assert 'Firetext callback failed: status 99 not found.' in str(e.value)
def test_firetext_callback_returns_200_when_notification_id_not_found_or_already_updated(client, mocker):
def test_firetext_callback_returns_200_when_notification_id_is_not_a_valid_uuid(client, mocker):
mocker.patch('app.statsd_client.incr')
data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference=1234'
response = firetext_post(client, data)
@@ -389,7 +393,7 @@ def test_process_mmg_response_status_4_updates_notification_with_temporary_faile
assert get_notification_by_id(notification.id).status == 'temporary-failure'
def test_process_mmg_response_unknown_status_updates_notification_with_failed(
def test_process_mmg_response_unknown_status_updates_notification_with_technical_failure(
notify_db, notify_db_session, client, mocker
):
send_mock = mocker.patch(
@@ -403,12 +407,10 @@ def test_process_mmg_response_unknown_status_updates_notification_with_failed(
"MSISDN": "447777349060",
"status": 10})
create_service_callback_api(service=notification.service, url="https://original_url.com")
response = mmg_post(client, data)
assert response.status_code == 200
json_data = json.loads(response.data)
assert json_data['result'] == 'success'
assert json_data['message'] == 'MMG callback succeeded. reference {} updated'.format(notification.id)
assert get_notification_by_id(notification.id).status == 'failed'
with pytest.raises(ClientException) as e:
mmg_post(client, data)
assert 'MMG callback failed: status 10 not found.' in str(e.value)
assert get_notification_by_id(notification.id).status == 'technical-failure'
assert send_mock.called
@@ -436,6 +438,16 @@ def test_mmg_callback_returns_200_when_notification_id_not_found_or_already_upda
assert response.status_code == 200
def test_mmg_callback_returns_400_when_notification_id_is_not_a_valid_uuid(client):
data = '{"reference": "10100164", "CID": "1234", "MSISDN": "447775349060", "status": "3", \
"deliverytime": "2016-04-05 16:01:07"}'
response = mmg_post(client, data)
json_resp = json.loads(response.get_data(as_text=True))
assert response.status_code == 400
assert json_resp['message'] == 'MMG callback with invalid reference 1234'
def test_process_mmg_response_records_statsd(notify_db, notify_db_session, client, mocker):
with freeze_time('2001-01-01T12:00:00'):

15
tests/app/notifications/test_process_client_response.py
View File

@@ -1,5 +1,8 @@
import uuid
import pytest
from app.clients import ClientException
from app.notifications.process_client_response import (
validate_callback_data,
process_sms_client_response
@@ -96,7 +99,7 @@ def test_process_sms_response_returns_error_bad_reference(mocker):
stats_mock.assert_not_called()
def test_process_sms_response_returns_error_for_unknown_sms_client(mocker):
def test_process_sms_response_raises_client_exception_for_unknown_sms_client(mocker):
stats_mock = mocker.patch('app.notifications.process_client_response.create_outcome_notification_statistic_tasks')
success, error = process_sms_client_response(status='000', reference=str(uuid.uuid4()), client_name='sms-client')
@@ -105,10 +108,8 @@ def test_process_sms_response_returns_error_for_unknown_sms_client(mocker):
stats_mock.assert_not_called()
def test_process_sms_response_returns_error_for_unknown_status(mocker):
stats_mock = mocker.patch('app.notifications.process_client_response.create_outcome_notification_statistic_tasks')
def test_process_sms_response_raises_client_exception_for_unknown_status(mocker):
with pytest.raises(ClientException) as e:
process_sms_client_response(status='000', reference=str(uuid.uuid4()), client_name='Firetext')
success, error = process_sms_client_response(status='000', reference=str(uuid.uuid4()), client_name='Firetext')
assert success is None
assert error == "{} callback failed: status {} not found.".format('Firetext', '000')
stats_mock.assert_not_called()
assert "{} callback failed: status {} not found.".format('Firetext', '000') in str(e.value)