From 68eaacaafb4aa3c0aab072c7d9686f4db95645b3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 29 Feb 2016 11:23:34 +0000 Subject: [PATCH] Accept and validate personalisation This commit allows the send notification endpoint to accept an extra parameter, `personalisation`, the contents of which will be used (later) to replace the placeholders in the template. It does validation in the following places: - at the schema level, to validate the type and (optional) presence of personalisation - at the endpoint, to check whether the personalisation provided matches exactly the placeholders in the template It does not do validation when processing CSV files, as these are assumed to already have been validated by the admin app. It explicitly does not persist either the names of the placeholders (these should always be derived from the template contents unless it really becomes a performance concern) or the values of the placeholders (because they might be personal data). --- app/notifications/rest.py | 26 +++++++- app/schemas.py | 1 + tests/app/conftest.py | 5 ++ tests/app/notifications/test_rest.py | 99 ++++++++++++++++++++++++++++ 4 files changed, 128 insertions(+), 3 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index b6f69e0d5..25dff99b5 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -8,7 +8,7 @@ from flask import ( url_for ) -from utils.template import Template +from utils.template import Template, NeededByTemplateError, NoPlaceholderForDataError from app import api_user, encryption, create_uuid from app.authentication.auth import require_admin @@ -135,10 +135,10 @@ def send_notification(notification_type): if errors: return jsonify(result="error", message=errors), 400 - template = Template(templates_dao.dao_get_template_by_id_and_service_id( + template = templates_dao.dao_get_template_by_id_and_service_id( template_id=notification['template'], service_id=service_id - )) + ) if not template: return jsonify( result="error", @@ -147,6 +147,26 @@ def send_notification(notification_type): } ), 404 + template_object = Template({'content': template.content}, notification.get('personalisation', {})) + if template_object.missing_data: + return jsonify( + result="error", + message={ + 'template': ['Missing personalisation: {}'.format( + ", ".join(template_object.missing_data) + )] + } + ), 400 + if template_object.additional_data: + return jsonify( + result="error", + message={ + 'template': ['Personalisation not needed for template: {}'.format( + ", ".join(template_object.additional_data) + )] + } + ), 400 + service = services_dao.dao_fetch_service_by_id(api_user['client']) notification_id = create_uuid() diff --git a/app/schemas.py b/app/schemas.py index d2e9d53c8..2114a2111 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -118,6 +118,7 @@ class RequestVerifyCodeSchema(ma.Schema): class NotificationSchema(ma.Schema): + personalisation = fields.Dict(required=False) pass diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 1c0a2657c..d640d11cf 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -142,6 +142,11 @@ def sample_template(notify_db, return template +@pytest.fixture(scope='function') +def sample_template_with_placeholders(notify_db, notify_db_session): + return sample_template(notify_db, notify_db_session, content="Hello ((name))") + + @pytest.fixture(scope='function') def sample_email_template( notify_db, diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index dce8b1aa9..66fb9ce3c 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -328,6 +328,105 @@ def test_send_notification_invalid_template_id(notify_api, sample_template, mock test_string = 'Template {} not found for service {}'.format(9999, sample_template.service.id) assert test_string in json_resp['message']['template'] +@freeze_time("2016-01-01 11:09:00.061258") +def test_send_notification_with_placeholders_replaced(notify_api, sample_template_with_placeholders, mocker): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') + mocker.patch('app.encryption.encrypt', return_value="something_encrypted") + + data = { + 'to': '+441234123123', + 'template': sample_template_with_placeholders.id, + 'personalisation': { + 'name': 'Jo' + } + } + auth_header = create_authorization_header( + service_id=sample_template_with_placeholders.service.id, + request_body=json.dumps(data), + path='/notifications/sms', + method='POST') + + response = client.post( + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + notification_id = json.loads(response.data)['notification_id'] + app.celery.tasks.send_sms.apply_async.assert_called_once_with( + (str(sample_template_with_placeholders.service.id), + notification_id, + "something_encrypted", + "2016-01-01 11:09:00.061258"), + queue="sms" + ) + assert response.status_code == 201 + + +def test_send_notification_with_missing_personalisation( + notify_api, sample_template_with_placeholders, mocker +): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') + + data = { + 'to': '+441234123123', + 'template': sample_template_with_placeholders.id, + 'personalisation': { + 'foo': 'bar' + } + } + auth_header = create_authorization_header( + service_id=sample_template_with_placeholders.service.id, + request_body=json.dumps(data), + path='/notifications/sms', + method='POST') + + response = client.post( + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_resp = json.loads(response.get_data(as_text=True)) + app.celery.tasks.send_sms.apply_async.assert_not_called() + + assert response.status_code == 400 + assert 'Missing personalisation: name' in json_resp['message']['template'] + + +def test_send_notification_with_too_much_personalisation_data( + notify_api, sample_template_with_placeholders, mocker +): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + mocker.patch('app.celery.tasks.send_sms.apply_async') + + data = { + 'to': '+441234123123', + 'template': sample_template_with_placeholders.id, + 'personalisation': { + 'name': 'Jo', 'foo': 'bar' + } + } + auth_header = create_authorization_header( + service_id=sample_template_with_placeholders.service.id, + request_body=json.dumps(data), + path='/notifications/sms', + method='POST') + + response = client.post( + path='/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_resp = json.loads(response.get_data(as_text=True)) + app.celery.tasks.send_sms.apply_async.assert_not_called() + + assert response.status_code == 400 + assert 'Personalisation not needed for template: foo' in json_resp['message']['template'] + def test_prevents_sending_to_any_mobile_on_restricted_service(notify_api, sample_template, mocker): with notify_api.test_request_context():