From fa8cd780b0a98db9bda58f1c8fb32df093711128 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 12 Feb 2018 12:06:43 +0000 Subject: [PATCH] Make the errors in the process_sms_client_reponse more obvious by changing variable name. Added a missing test if mmg sends a CID (or reference) that is not a UUID. NB: All client specific tests are in test_callbacks. --- app/notifications/process_client_response.py | 11 +++++------ tests/app/notifications/rest/test_callbacks.py | 12 +++++++++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/notifications/process_client_response.py b/app/notifications/process_client_response.py index 8996f8a2b..19aa2d290 100644 --- a/app/notifications/process_client_response.py +++ b/app/notifications/process_client_response.py @@ -40,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] @@ -55,15 +55,14 @@ def process_sms_client_response(status, reference, client_name): client_name, status, reference) ) except KeyError: - process_for_status(notification_status='technical-failure', client_name=client_name, reference=reference) + _process_for_status(notification_status='technical-failure', client_name=client_name, reference=reference) raise ClientException("{} callback failed: status {} not found.".format(client_name, status)) - success = process_for_status(notification_status=notification_status, client_name=client_name, reference=reference) + 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): - +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: diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index e3c3e7844..1654b3b68 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -176,7 +176,7 @@ def test_firetext_callback_should_set_status_technical_failure_if_status_unknown 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) @@ -438,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'):