From 24b77726f43dc5b33f14a67e462548187df02111 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 17 Mar 2020 15:15:43 +0000 Subject: [PATCH] Add a task to process sms callback from our providers This runs on the new `sms-callbacks` queue. The function `process_sms_client_response` has been replaced with a task called `process_sms_client_response`. This involved some reorganisation of the existing code and tests. --- .../process_sms_client_response_tasks.py} | 57 +-- .../notifications_sms_callback.py | 54 ++- .../app/notifications/rest/test_callbacks.py | 339 +++--------------- .../test_process_client_response.py | 174 +++++---- 4 files changed, 189 insertions(+), 435 deletions(-) rename app/{notifications/process_client_response.py => celery/process_sms_client_response_tasks.py} (65%) diff --git a/app/notifications/process_client_response.py b/app/celery/process_sms_client_response_tasks.py similarity index 65% rename from app/notifications/process_client_response.py rename to app/celery/process_sms_client_response_tasks.py index b2a73f203..1ee86634e 100644 --- a/app/notifications/process_client_response.py +++ b/app/celery/process_sms_client_response_tasks.py @@ -1,20 +1,17 @@ import uuid - from datetime import datetime + from flask import current_app +from notifications_utils.statsd_decorators import statsd from notifications_utils.template import SMSMessageTemplate -from app import statsd_client +from app import notify_celery, 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 -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.dao.notifications_dao import dao_update_notification +from app.dao import notifications_dao from app.dao.service_callback_api_dao import get_service_delivery_status_callback_api_for_service from app.dao.templates_dao import dao_get_template_by_id from app.models import NOTIFICATION_PENDING @@ -25,39 +22,23 @@ sms_response_mapper = { } -def validate_callback_data(data, fields, client_name): - errors = [] - for f in fields: - if not str(data.get(f, '')): - error = "{} callback failed: {} missing".format(client_name, f) - errors.append(error) - return errors if len(errors) > 0 else None - - -def process_sms_client_response(status, provider_reference, client_name): - success = None - errors = None +@notify_celery.task(bind=True, name="process-sms-client-response", max_retries=5, default_retry_delay=300) +@statsd(namespace="tasks") +def process_sms_client_response(self, status, provider_reference, client_name): # validate reference - if provider_reference == 'send-sms-code': - success = "{} callback succeeded: send-sms-code".format(client_name) - return success, errors - try: uuid.UUID(provider_reference, version=4) - except ValueError: - errors = "{} callback with invalid reference {}".format(client_name, provider_reference) - return success, errors + except ValueError as e: + current_app.logger.exception(f'{client_name} callback with invalid reference {provider_reference}') + raise e - try: - response_parser = sms_response_mapper[client_name] - except KeyError: - return success, 'unknown sms client: {}'.format(client_name) + response_parser = sms_response_mapper[client_name] - # validate status + # validate status try: notification_status = response_parser(status) - current_app.logger.info('{} callback return status of {} for reference: {}'.format( - client_name, status, provider_reference) + current_app.logger.info( + f'{client_name} callback returned status of {status} for reference: {provider_reference}' ) except KeyError: _process_for_status( @@ -65,14 +46,13 @@ def process_sms_client_response(status, provider_reference, client_name): client_name=client_name, provider_reference=provider_reference ) - raise ClientException("{} callback failed: status {} not found.".format(client_name, status)) + raise ClientException(f'{client_name} callback failed: status {status} not found.') - success = _process_for_status( + _process_for_status( notification_status=notification_status, client_name=client_name, provider_reference=provider_reference ) - return success, errors def _process_for_status(notification_status, client_name, provider_reference): @@ -114,6 +94,3 @@ def _process_for_status(notification_status, client_name, provider_reference): encrypted_notification = create_delivery_status_callback_data(notification, service_callback_api) send_delivery_status_to_service.apply_async([str(notification.id), encrypted_notification], queue=QueueNames.CALLBACKS) - - success = "{} callback succeeded. reference {} updated".format(client_name, provider_reference) - return success diff --git a/app/notifications/notifications_sms_callback.py b/app/notifications/notifications_sms_callback.py index 81e7b80b8..afc705846 100644 --- a/app/notifications/notifications_sms_callback.py +++ b/app/notifications/notifications_sms_callback.py @@ -3,8 +3,9 @@ from flask import current_app from flask import json from flask import request, jsonify +from app.celery.process_sms_client_response_tasks import process_sms_client_response +from app.config import QueueNames from app.errors import InvalidRequest, register_errors -from app.notifications.process_client_response import validate_callback_data, process_sms_client_response sms_callback_blueprint = Blueprint("sms_callback", __name__, url_prefix="/notifications/sms") register_errors(sms_callback_blueprint) @@ -20,19 +21,21 @@ def process_mmg_response(): if errors: raise InvalidRequest(errors, status_code=400) - success, errors = process_sms_client_response(status=str(data.get('status')), - provider_reference=data.get('CID'), - client_name=client_name) + status = str(data.get('status')) + provider_reference = data.get('CID') + + process_sms_client_response.apply_async( + [status, provider_reference, client_name], + queue=QueueNames.SMS_CALLBACKS, + ) safe_to_log = data.copy() safe_to_log.pop("MSISDN") current_app.logger.debug( - "Full delivery response from {} for notification: {}\n{}".format(client_name, request.form.get('CID'), - safe_to_log)) - if errors: - raise InvalidRequest(errors, status_code=400) - else: - return jsonify(result='success', message=success), 200 + f"Full delivery response from {client_name} for notification: {provider_reference}\n{safe_to_log}" + ) + + return jsonify(result='success'), 200 @sms_callback_blueprint.route('/firetext', methods=['POST']) @@ -43,15 +46,28 @@ def process_firetext_response(): client_name=client_name) if errors: raise InvalidRequest(errors, status_code=400) + + status = request.form.get('status') + provider_reference = request.form.get('reference') + safe_to_log = dict(request.form).copy() safe_to_log.pop('mobile') current_app.logger.debug( - "Full delivery response from {} for notification: {}\n{}".format(client_name, request.form.get('reference'), - safe_to_log)) - success, errors = process_sms_client_response(status=request.form.get('status'), - provider_reference=request.form.get('reference'), - client_name=client_name) - if errors: - raise InvalidRequest(errors, status_code=400) - else: - return jsonify(result='success', message=success), 200 + f"Full delivery response from {client_name} for notification: {provider_reference}\n{safe_to_log}" + ) + + process_sms_client_response.apply_async( + [status, provider_reference, client_name], + queue=QueueNames.SMS_CALLBACKS, + ) + + return jsonify(result='success'), 200 + + +def validate_callback_data(data, fields, client_name): + errors = [] + for f in fields: + if not str(data.get(f, '')): + error = "{} callback failed: {} missing".format(client_name, f) + errors.append(error) + return errors if len(errors) > 0 else None diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index e72e62c2f..bbbc44e2c 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -1,17 +1,7 @@ -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 -) -from tests.app.db import create_notification, create_service_callback_api +from app.notifications.notifications_sms_callback import validate_callback_data def firetext_post(client, data): @@ -111,15 +101,14 @@ def test_dvla_ack_calls_does_not_call_letter_notifications_task(client, mocker): def test_firetext_callback_should_not_need_auth(client, mocker): - mocker.patch('app.statsd_client.incr') - data = 'mobile=441234123123&status=0&reference=send-sms-code&time=2016-03-10 14:17:00' + mocker.patch('app.notifications.notifications_sms_callback.process_sms_client_response') + data = 'mobile=441234123123&status=0&reference=notification_id&time=2016-03-10 14:17:00' response = firetext_post(client, data) assert response.status_code == 200 def test_firetext_callback_should_return_400_if_empty_reference(client, mocker): - mocker.patch('app.statsd_client.incr') data = 'mobile=441234123123&status=0&reference=&time=2016-03-10 14:17:00' response = firetext_post(client, data) @@ -130,7 +119,6 @@ def test_firetext_callback_should_return_400_if_empty_reference(client, mocker): def test_firetext_callback_should_return_400_if_no_reference(client, mocker): - mocker.patch('app.statsd_client.incr') data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00' response = firetext_post(client, data) json_resp = json.loads(response.get_data(as_text=True)) @@ -139,19 +127,8 @@ def test_firetext_callback_should_return_400_if_no_reference(client, mocker): assert json_resp['message'] == ['Firetext callback failed: reference missing'] -def test_firetext_callback_should_return_200_if_send_sms_reference(client, mocker): - mocker.patch('app.statsd_client.incr') - data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference=send-sms-code' - response = firetext_post(client, data) - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert json_resp['result'] == 'success' - assert json_resp['message'] == 'Firetext callback succeeded: send-sms-code' - - def test_firetext_callback_should_return_400_if_no_status(client, mocker): - mocker.patch('app.statsd_client.incr') - data = 'mobile=441234123123&time=2016-03-10 14:17:00&reference=send-sms-code' + data = 'mobile=441234123123&time=2016-03-10 14:17:00&reference=notification_id' response = firetext_post(client, data) json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 400 @@ -159,136 +136,24 @@ 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_set_status_technical_failure_if_status_unknown( - client, mocker, sample_notification): - sample_notification.status = 'sending' - # mocker.patch('app.statsd_client.incr') - data = 'mobile=441234123123&status=99&time=2016-03-10 14:17:00&reference={}'.format(sample_notification.id) - with pytest.raises(ClientException) as e: - firetext_post(client, data) - assert get_notification_by_id(sample_notification.id).status == 'technical-failure' - assert 'Firetext callback failed: status 99 not found.' in str(e.value) +def test_firetext_callback_should_return_200_and_call_task_with_valid_data(client, mocker): + mock_celery = mocker.patch( + 'app.notifications.notifications_sms_callback.process_sms_client_response.apply_async') - -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' + data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference=notification_id' 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 with invalid reference 1234' - - -def test_callback_should_return_200_if_cannot_find_notification_id( - notify_db, - notify_db_session, - client, - mocker -): - mocker.patch('app.statsd_client.incr') - missing_notification_id = uuid.uuid4() - data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference={}'.format( - missing_notification_id) - response = firetext_post(client, data) - json_resp = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 assert json_resp['result'] == 'success' - -def test_firetext_callback_should_update_notification_status( - client, mocker, sample_notification -): - mocker.patch('app.statsd_client.incr') - send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + mock_celery.assert_called_once_with( + ['0', 'notification_id', 'Firetext'], + queue='sms-callbacks', ) - sample_notification.status = 'sending' - - original = get_notification_by_id(sample_notification.id) - assert original.status == 'sending' - data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&reference={}'.format( - sample_notification.id) - response = firetext_post(client, data) - - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert json_resp['result'] == 'success' - assert json_resp['message'] == 'Firetext callback succeeded. reference {} updated'.format( - sample_notification.id - ) - updated = get_notification_by_id(sample_notification.id) - assert updated.status == 'delivered' - assert get_notification_by_id(sample_notification.id).status == 'delivered' - assert send_mock.called_once_with([sample_notification.id], queue="notify-internal-tasks") -def test_firetext_callback_should_update_notification_status_failed( - client, mocker, sample_template -): - mocker.patch('app.statsd_client.incr') - mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - notification = create_notification(template=sample_template, status='sending') - - original = get_notification_by_id(notification.id) - assert original.status == 'sending' - - data = 'mobile=441234123123&status=1&time=2016-03-10 14:17:00&reference={}'.format( - notification.id) - response = firetext_post(client, data) - - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert json_resp['result'] == 'success' - assert json_resp['message'] == 'Firetext callback succeeded. reference {} updated'.format( - notification.id - ) - assert get_notification_by_id(notification.id).status == 'permanent-failure' - - -def test_firetext_callback_should_update_notification_status_pending(client, sample_template, mocker): - mocker.patch('app.statsd_client.incr') - mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - notification = create_notification(template=sample_template, status='sending') - - original = get_notification_by_id(notification.id) - assert original.status == 'sending' - data = 'mobile=441234123123&status=2&time=2016-03-10 14:17:00&reference={}'.format( - notification.id) - response = firetext_post(client, data) - - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert json_resp['result'] == 'success' - assert json_resp['message'] == 'Firetext callback succeeded. reference {} updated'.format( - notification.id - ) - assert get_notification_by_id(notification.id).status == 'pending' - - -def test_process_mmg_response_return_200_when_cid_is_send_sms_code(client): - data = '{"reference": "10100164", "CID": "send-sms-code", "MSISDN": "447775349060", "status": "3", \ - "deliverytime": "2016-04-05 16:01:07"}' - - 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: send-sms-code' - - -def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id( - sample_notification, client, mocker -): - mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - sample_notification.status = 'sending' +def test_mmg_callback_should_not_need_auth(client, mocker, sample_notification): + mocker.patch('app.notifications.notifications_sms_callback.process_sms_client_response') data = json.dumps({"reference": "mmg_reference", "CID": str(sample_notification.id), "MSISDN": "447777349060", @@ -296,93 +161,7 @@ def test_process_mmg_response_returns_200_when_cid_is_valid_notification_id( "deliverytime": "2016-04-05 16:01:07"}) 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(sample_notification.id) - assert get_notification_by_id(sample_notification.id).status == 'delivered' - - -def test_process_mmg_response_status_5_updates_notification_with_permanently_failed( - sample_notification, client, mocker -): - mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - sample_notification.status = 'sending' - - data = json.dumps({"reference": "mmg_reference", - "CID": str(sample_notification.id), - "MSISDN": "447777349060", - "status": 5}) - - 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(sample_notification.id) - assert get_notification_by_id(sample_notification.id).status == 'permanent-failure' - - -def test_process_mmg_response_status_2_updates_notification_with_permanently_failed( - sample_notification, client, mocker -): - mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - sample_notification.status = 'sending' - data = json.dumps({"reference": "mmg_reference", - "CID": str(sample_notification.id), - "MSISDN": "447777349060", - "status": 2}) - - 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(sample_notification.id) - assert get_notification_by_id(sample_notification.id).status == 'permanent-failure' - - -def test_process_mmg_response_status_4_updates_notification_with_temporary_failed( - sample_notification, client, mocker -): - mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - sample_notification.status = 'sending' - - data = json.dumps({"reference": "mmg_reference", - "CID": str(sample_notification.id), - "MSISDN": "447777349060", - "status": 4}) - - 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(sample_notification.id) - assert get_notification_by_id(sample_notification.id).status == 'temporary-failure' - - -def test_process_mmg_response_unknown_status_updates_notification_with_technical_failure( - sample_notification, client, mocker -): - send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - sample_notification.status = 'sending' - data = json.dumps({"reference": "mmg_reference", - "CID": str(sample_notification.id), - "MSISDN": "447777349060", - "status": 10}) - create_service_callback_api(service=sample_notification.service, url="https://original_url.com") - 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(sample_notification.id).status == 'technical-failure' - assert send_mock.called def test_process_mmg_response_returns_400_for_malformed_data(client): @@ -401,68 +180,64 @@ def test_process_mmg_response_returns_400_for_malformed_data(client): assert "{} callback failed: {} missing".format('MMG', 'CID') in json_data['message'] -def test_mmg_callback_returns_200_when_notification_id_not_found_or_already_updated(client): - data = '{"reference": "10100164", "CID": "send-sms-code", "MSISDN": "447775349060", "status": "3", \ - "deliverytime": "2016-04-05 16:01:07"}' +def test_mmg_callback_should_return_200_and_call_task_with_valid_data(client, mocker): + mock_celery = mocker.patch( + 'app.notifications.notifications_sms_callback.process_sms_client_response.apply_async') + data = json.dumps({"reference": "mmg_reference", + "CID": "notification_id", + "MSISDN": "447777349060", + "status": "3", + "deliverytime": "2016-04-05 16:01:07"}) response = mmg_post(client, data) + assert response.status_code == 200 + json_data = json.loads(response.data) + assert json_data['result'] == 'success' + + mock_celery.assert_called_once_with( + ['3', 'notification_id', 'MMG'], + queue='sms-callbacks', + ) -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"}' +def test_validate_callback_data_returns_none_when_valid(): + form = {'status': 'good', + 'reference': 'send-sms-code'} + fields = ['status', 'reference'] + client_name = 'sms client' - 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' + assert validate_callback_data(form, fields, client_name) is None -def test_process_mmg_response_records_statsd(sample_notification, client, mocker): - with freeze_time('2001-01-01T12:00:00'): +def test_validate_callback_data_return_errors_when_fields_are_empty(): + form = {'monkey': 'good'} + fields = ['status', 'cid'] + client_name = 'sms client' - mocker.patch('app.statsd_client.incr') - mocker.patch('app.statsd_client.timing_with_dates') - mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - sample_notification.status = 'sending' - sample_notification.sent_at = datetime.now() - - data = json.dumps({"reference": "mmg_reference", - "CID": str(sample_notification.id), - "MSISDN": "447777349060", - "status": "3", - "deliverytime": "2016-04-05 16:01:07"}) - - mmg_post(client, data) - - app.statsd_client.incr.assert_any_call("callback.mmg.delivered") - app.statsd_client.timing_with_dates.assert_any_call( - "callback.mmg.elapsed-time", datetime.utcnow(), sample_notification.sent_at - ) + errors = validate_callback_data(form, fields, client_name) + assert len(errors) == 2 + assert "{} callback failed: {} missing".format(client_name, 'status') in errors + assert "{} callback failed: {} missing".format(client_name, 'cid') in errors -def test_firetext_callback_should_record_statsd(client, sample_notification, mocker): - with freeze_time('2001-01-01T12:00:00'): +def test_validate_callback_data_can_handle_integers(): + form = {'status': 00, 'cid': 'fsdfadfsdfas'} + fields = ['status', 'cid'] + client_name = 'sms client' - mocker.patch('app.statsd_client.incr') - mocker.patch('app.statsd_client.timing_with_dates') - mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - sample_notification.status = 'sending' - sample_notification.sent_at = datetime.now() + result = validate_callback_data(form, fields, client_name) + assert result is None - data = 'mobile=441234123123&status=0&time=2016-03-10 14:17:00&code=101&reference={}'.format( - sample_notification.id) - firetext_post(client, data) - app.statsd_client.timing_with_dates.assert_any_call( - "callback.firetext.elapsed-time", datetime.utcnow(), sample_notification.sent_at - ) - app.statsd_client.incr.assert_any_call("callback.firetext.delivered") +def test_validate_callback_data_returns_error_for_empty_string(): + form = {'status': '', 'cid': 'fsdfadfsdfas'} + fields = ['status', 'cid'] + client_name = 'sms client' + + result = validate_callback_data(form, fields, client_name) + assert result is not None + assert "{} callback failed: {} missing".format(client_name, 'status') in result def _sample_sns_s3_callback(filename): diff --git a/tests/app/notifications/test_process_client_response.py b/tests/app/notifications/test_process_client_response.py index 856cef931..9ccd05546 100644 --- a/tests/app/notifications/test_process_client_response.py +++ b/tests/app/notifications/test_process_client_response.py @@ -1,79 +1,66 @@ import uuid +from datetime import datetime import pytest +from freezegun import freeze_time +from app import statsd_client from app.clients import ClientException -from app.notifications.process_client_response import ( - validate_callback_data, - process_sms_client_response -) +from app.celery.process_sms_client_response_tasks import process_sms_client_response from app.celery.service_callback_tasks import create_delivery_status_callback_data +from app.models import NOTIFICATION_TECHNICAL_FAILURE from tests.app.db import create_service_callback_api -def test_validate_callback_data_returns_none_when_valid(): - form = {'status': 'good', - 'reference': 'send-sms-code'} - fields = ['status', 'reference'] - client_name = 'sms client' - - assert validate_callback_data(form, fields, client_name) is None +def test_process_sms_client_response_raises_error_if_reference_is_not_a_valid_uuid(client): + with pytest.raises(ValueError): + process_sms_client_response( + status='000', provider_reference='something-bad', client_name='sms-client') -def test_validate_callback_data_return_errors_when_fields_are_empty(): - form = {'monkey': 'good'} - fields = ['status', 'cid'] - client_name = 'sms client' +@pytest.mark.parametrize('client_name', ('Firetext', 'MMG')) +def test_process_sms_response_raises_client_exception_for_unknown_status( + sample_notification, + mocker, + client_name, +): + with pytest.raises(ClientException) as e: + process_sms_client_response( + status='000', + provider_reference=str(sample_notification.id), + client_name=client_name, + ) - errors = validate_callback_data(form, fields, client_name) - assert len(errors) == 2 - assert "{} callback failed: {} missing".format(client_name, 'status') in errors - assert "{} callback failed: {} missing".format(client_name, 'cid') in errors + assert f"{client_name} callback failed: status {'000'} not found." in str(e.value) + assert sample_notification.status == NOTIFICATION_TECHNICAL_FAILURE -def test_validate_callback_data_can_handle_integers(): - form = {'status': 00, 'cid': 'fsdfadfsdfas'} - fields = ['status', 'cid'] - client_name = 'sms client' +@pytest.mark.parametrize('status, sms_provider, expected_notification_status', [ + ('0', 'Firetext', 'delivered'), + ('1', 'Firetext', 'permanent-failure'), + ('2', 'Firetext', 'pending'), + ('2', 'MMG', 'permanent-failure'), + ('3', 'MMG', 'delivered'), + ('4', 'MMG', 'temporary-failure'), + ('5', 'MMG', 'permanent-failure'), +]) +def test_process_sms_client_response_updates_notification_status( + sample_notification, + mocker, + status, + sms_provider, + expected_notification_status, +): + sample_notification.status = 'sending' + process_sms_client_response(status, str(sample_notification.id), sms_provider) - result = validate_callback_data(form, fields, client_name) - assert result is None + assert sample_notification.status == expected_notification_status -def test_validate_callback_data_returns_error_for_empty_string(): - form = {'status': '', 'cid': 'fsdfadfsdfas'} - fields = ['status', 'cid'] - client_name = 'sms client' - - result = validate_callback_data(form, fields, client_name) - assert result is not None - assert "{} callback failed: {} missing".format(client_name, 'status') in result - - -def test_outcome_statistics_called_for_successful_callback(sample_notification, mocker): +def test_sms_response_does_not_send_callback_if_notification_is_not_in_the_db(sample_service, mocker): mocker.patch( - 'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id', - return_value=sample_notification - ) - send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - callback_api = create_service_callback_api(service=sample_notification.service, url="https://original_url.com") - reference = str(uuid.uuid4()) - - success, error = process_sms_client_response(status='3', provider_reference=reference, client_name='MMG') - assert success == "MMG callback succeeded. reference {} updated".format(str(reference)) - assert error is None - encrypted_data = create_delivery_status_callback_data(sample_notification, callback_api) - send_mock.assert_called_once_with([str(sample_notification.id), encrypted_data], - queue="service-callbacks") - - -def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notification, mocker): - mocker.patch( - 'app.notifications.process_client_response.notifications_dao.update_notification_status_by_id', - return_value=sample_notification - ) + 'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service', + return_value='mock-delivery-callback-for-service') send_mock = mocker.patch( 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' ) @@ -82,55 +69,54 @@ def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notificat send_mock.assert_not_called() -def test_process_sms_response_return_success_for_send_sms_code_reference(mocker): - success, error = process_sms_client_response( - status='000', provider_reference='send-sms-code', client_name='sms-client') - assert success == "{} callback succeeded: send-sms-code".format('sms-client') - assert error is None +@freeze_time('2001-01-01T12:00:00') +def test_process_sms_client_response_records_statsd_metrics(sample_notification, client, mocker): + mocker.patch('app.statsd_client.incr') + mocker.patch('app.statsd_client.timing_with_dates') + sample_notification.status = 'sending' + sample_notification.sent_at = datetime.utcnow() -def test_process_sms_response_does_not_send_status_update_for_pending(sample_notification, mocker): - send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async') - process_sms_client_response( - status='2', provider_reference=str(sample_notification.id), client_name='firetext') - send_mock.assert_not_called() + process_sms_client_response('0', str(sample_notification.id), 'Firetext') - -def test_process_sms_updates_sent_by_with_client_name_if_not_in_noti(sample_notification): - sample_notification.sent_by = None - success, error = process_sms_client_response( - status='3', provider_reference=str(sample_notification.id), client_name='MMG') - assert error is None - assert success == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id) - assert sample_notification.sent_by == 'mmg' + statsd_client.incr.assert_any_call("callback.firetext.delivered") + statsd_client.timing_with_dates.assert_any_call( + "callback.firetext.elapsed-time", datetime.utcnow(), sample_notification.sent_at + ) def test_process_sms_updates_billable_units_if_zero(sample_notification): sample_notification.billable_units = 0 - success, error = process_sms_client_response( - status='3', provider_reference=str(sample_notification.id), client_name='MMG') - assert error is None - assert success == 'MMG callback succeeded. reference {} updated'.format(sample_notification.id) + process_sms_client_response('3', str(sample_notification.id), 'MMG') + assert sample_notification.billable_units == 1 -def test_process_sms_response_returns_error_bad_reference(mocker): - success, error = process_sms_client_response( - status='000', provider_reference='something-bad', client_name='sms-client') - assert success is None - assert error == "{} callback with invalid reference {}".format('sms-client', 'something-bad') +def test_process_sms_response_does_not_send_service_callback_for_pending_notifications(sample_notification, mocker): + mocker.patch( + 'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service', + return_value='fake-callback') + send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async') + process_sms_client_response('2', str(sample_notification.id), 'Firetext') + send_mock.assert_not_called() -def test_process_sms_response_raises_client_exception_for_unknown_sms_client(mocker): - success, error = process_sms_client_response( - status='000', provider_reference=str(uuid.uuid4()), client_name='sms-client') +def test_outcome_statistics_called_for_successful_callback(sample_notification, mocker): + send_mock = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + callback_api = create_service_callback_api(service=sample_notification.service, url="https://original_url.com") + reference = str(sample_notification.id) - assert success is None - assert error == 'unknown sms client: {}'.format('sms-client') + process_sms_client_response('3', reference, 'MMG') + + encrypted_data = create_delivery_status_callback_data(sample_notification, callback_api) + send_mock.assert_called_once_with([reference, encrypted_data], + queue="service-callbacks") -def test_process_sms_response_raises_client_exception_for_unknown_status(mocker): - with pytest.raises(ClientException) as e: - process_sms_client_response(status='000', provider_reference=str(uuid.uuid4()), client_name='Firetext') +def test_process_sms_updates_sent_by_with_client_name_if_not_in_noti(sample_notification): + sample_notification.sent_by = None + process_sms_client_response('3', str(sample_notification.id), 'MMG') - assert "{} callback failed: status {} not found.".format('Firetext', '000') in str(e.value) + assert sample_notification.sent_by == 'mmg'