From b439fd071878d8b1bea1d65082fd1b2dd665fc9a Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Thu, 24 Mar 2022 16:41:08 +0000 Subject: [PATCH] Add boilerplate for Reach SMS callbacks This is enough to update a notification in DB: 1. First create a notification in the UI and sent it. 2. Then reset its attributes to pretend it's for Reach. update notifications set sent_at = null, sent_by = null, notification_status='sending' where id='some-uuid'; 3. Change "notification_id" to "" in the code. 4. Call the boilerplate endpoint for Reach callbacks. curl -X POST localhost:6011/notifications/sms/reach Interestingly there's no foreign key constraint on "sent_by" in the DB, so this just works: the notification is updated. --- .../process_sms_client_response_tasks.py | 4 ++- app/clients/sms/reach.py | 25 +++++++++++++++++++ .../notifications_sms_callback.py | 22 ++++++++++++++++ manifest.yml.j2 | 6 ++--- .../test_process_sms_client_response_tasks.py | 5 +++- tests/app/clients/test_reach.py | 1 + .../test_notifications_sms_callbacks.py | 25 +++++++++++++++++++ 7 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 app/clients/sms/reach.py create mode 100644 tests/app/clients/test_reach.py diff --git a/app/celery/process_sms_client_response_tasks.py b/app/celery/process_sms_client_response_tasks.py index e16175646..b8f1f63bb 100644 --- a/app/celery/process_sms_client_response_tasks.py +++ b/app/celery/process_sms_client_response_tasks.py @@ -8,6 +8,7 @@ from app import notify_celery, statsd_client from app.clients import ClientException from app.clients.sms.firetext import get_firetext_responses from app.clients.sms.mmg import get_mmg_responses +from app.clients.sms.reach import get_reach_responses from app.dao import notifications_dao from app.dao.templates_dao import dao_get_template_by_id from app.models import NOTIFICATION_PENDING @@ -17,7 +18,8 @@ from app.notifications.notifications_ses_callback import ( sms_response_mapper = { 'MMG': get_mmg_responses, - 'Firetext': get_firetext_responses + 'Firetext': get_firetext_responses, + 'Reach': get_reach_responses } diff --git a/app/clients/sms/reach.py b/app/clients/sms/reach.py new file mode 100644 index 000000000..d9b016e2a --- /dev/null +++ b/app/clients/sms/reach.py @@ -0,0 +1,25 @@ +from app.clients.sms import SmsClient, SmsClientResponseException + + +def get_reach_responses(status, detailed_status_code=None): + if status == 'TODO-d': + return ("delivered", "TODO: Delivered") + elif status == 'TODO-tf': + return ("temporary-failure", "TODO: Temporary failure") + elif status == 'TODO-pf': + return ("permanent-failure", "TODO: Permanent failure") + else: + raise KeyError + + +class ReachClientResponseException(SmsClientResponseException): + pass # TODO (custom exception for errors) + + +class ReachClient(SmsClient): + + def get_name(self): + pass # TODO + + def send_sms(self, to, content, reference, international, multi=True, sender=None): + pass # TODO diff --git a/app/notifications/notifications_sms_callback.py b/app/notifications/notifications_sms_callback.py index f0896d745..f7353d45c 100644 --- a/app/notifications/notifications_sms_callback.py +++ b/app/notifications/notifications_sms_callback.py @@ -54,6 +54,28 @@ def process_firetext_response(): return jsonify(result='success'), 200 +@sms_callback_blueprint.route('/reach', methods=['POST']) +def process_reach_response(): + client_name = 'Reach' + + # TODO: validate request + errors = None + + if errors: + raise InvalidRequest(errors, status_code=400) + + status = 'TODO-d' # TODO + detailed_status_code = 'something' # TODO + provider_reference = 'notification_id' # TODO + + process_sms_client_response.apply_async( + [status, provider_reference, client_name, detailed_status_code], + queue=QueueNames.SMS_CALLBACKS, + ) + + return jsonify(result='success'), 200 + + def validate_callback_data(data, fields, client_name): errors = [] for f in fields: diff --git a/manifest.yml.j2 b/manifest.yml.j2 index 3560f7b32..dbd267fd3 100644 --- a/manifest.yml.j2 +++ b/manifest.yml.j2 @@ -27,9 +27,9 @@ 'STATSD_HOST': None }, 'routes': { - 'preview': ['api.notify.works/notifications/sms/mmg', 'api.notify.works/notifications/sms/firetext'], - 'staging': ['api.staging-notify.works/notifications/sms/mmg', 'api.staging-notify.works/notifications/sms/firetext'], - 'production': ['api.notifications.service.gov.uk/notifications/sms/mmg', 'api.notifications.service.gov.uk/notifications/sms/firetext'], + 'preview': ['api.notify.works/notifications/sms/mmg', 'api.notify.works/notifications/sms/firetext', 'api.notify.works/notifications/sms/reach'], + 'staging': ['api.staging-notify.works/notifications/sms/mmg', 'api.staging-notify.works/notifications/sms/firetext', 'api.staging-notify.works/notifications/sms/reach'], + 'production': ['api.notifications.service.gov.uk/notifications/sms/mmg', 'api.notifications.service.gov.uk/notifications/sms/firetext', 'api.notifications.service.gov.uk/notifications/sms/reach'], }, 'health-check-type': 'port', 'health-check-invocation-timeout': 3, diff --git a/tests/app/celery/test_process_sms_client_response_tasks.py b/tests/app/celery/test_process_sms_client_response_tasks.py index 6005fbce3..046566906 100644 --- a/tests/app/celery/test_process_sms_client_response_tasks.py +++ b/tests/app/celery/test_process_sms_client_response_tasks.py @@ -18,7 +18,7 @@ def test_process_sms_client_response_raises_error_if_reference_is_not_a_valid_uu status='000', provider_reference='something-bad', client_name='sms-client') -@pytest.mark.parametrize('client_name', ('Firetext', 'MMG')) +@pytest.mark.parametrize('client_name', ('Firetext', 'MMG', 'Reach')) def test_process_sms_response_raises_client_exception_for_unknown_status( sample_notification, mocker, @@ -43,6 +43,9 @@ def test_process_sms_response_raises_client_exception_for_unknown_status( ('3', '2', 'MMG', 'delivered', "Delivered to operator"), ('4', '27', 'MMG', 'temporary-failure', "Absent Subscriber"), ('5', '13', 'MMG', 'permanent-failure', "Sender id blacklisted"), + ('TODO-d', None, 'Reach', 'delivered', "TODO: Delivered"), + ('TODO-tf', None, 'Reach', 'temporary-failure', "TODO: Temporary failure"), + ('TODO-pf', None, 'Reach', 'permanent-failure', "TODO: Permanent failure"), ]) def test_process_sms_client_response_updates_notification_status( sample_notification, diff --git a/tests/app/clients/test_reach.py b/tests/app/clients/test_reach.py new file mode 100644 index 000000000..00cf4f034 --- /dev/null +++ b/tests/app/clients/test_reach.py @@ -0,0 +1 @@ +# TODO: all of the tests diff --git a/tests/app/notifications/test_notifications_sms_callbacks.py b/tests/app/notifications/test_notifications_sms_callbacks.py index b3bb7b46e..1c1a02f01 100644 --- a/tests/app/notifications/test_notifications_sms_callbacks.py +++ b/tests/app/notifications/test_notifications_sms_callbacks.py @@ -17,6 +17,13 @@ def mmg_post(client, data): headers=[('Content-Type', 'application/json')]) +def reach_post(client, data): + return client.post( + path='/notifications/sms/reach', + data=data, + headers=[('Content-Type', 'application/json')]) + + def test_firetext_callback_should_not_need_auth(client, mocker): 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' @@ -135,6 +142,24 @@ def test_mmg_callback_should_return_200_and_call_task_with_valid_data(client, mo ) +# TODO: more tests about edge cases for this provider +def test_reach_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({"data": "TODO"}) + + response = reach_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( + ['TODO-d', 'notification_id', 'Reach', 'something'], + queue='sms-callbacks', + ) + + def test_validate_callback_data_returns_none_when_valid(): form = {'status': 'good', 'reference': 'send-sms-code'}