diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 71dd37c18..ce26e2b04 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -1,10 +1,21 @@ import json -from jsonschema import Draft4Validator, ValidationError +from jsonschema import (Draft4Validator, ValidationError, FormatChecker) +from notifications_utils.recipients import (validate_phone_number, validate_email_address) def validate(json_to_validate, schema): - validator = Draft4Validator(schema) + format_checker = FormatChecker() + + @format_checker.checks('phone_number') + def validate_schema_phone_number(instance): + return validate_phone_number(instance) + + @format_checker.checks('email_address') + def validate_schema_email_address(instance): + return validate_email_address(instance) + + validator = Draft4Validator(schema, format_checker=format_checker) errors = list(validator.iter_errors(json_to_validate)) if errors.__len__() > 0: raise ValidationError(build_error_message(errors, schema)) diff --git a/app/v2/errors.py b/app/v2/errors.py index 26cc3b36e..872ce41ab 100644 --- a/app/v2/errors.py +++ b/app/v2/errors.py @@ -1,6 +1,7 @@ import json from flask import jsonify, current_app from jsonschema import ValidationError +from notifications_utils.recipients import InvalidPhoneError from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound from app.authentication.auth import AuthError @@ -47,6 +48,12 @@ def register_errors(blueprint): def auth_error(error): return jsonify(error.to_dict_v2()), error.code + @blueprint.errorhandler(InvalidPhoneError) + def invalid_phone_error(error): + current_app.logger.exception(error) + return jsonify(status_code=400, + errors=[{"error": error.__class__.__name__, "message": error.message}]), 400 + @blueprint.errorhandler(Exception) def internal_server_error(error): current_app.logger.exception(error) diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 3207d0b48..97402a288 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -7,16 +7,16 @@ post_sms_request = { "title": "POST v2/notifications/sms", "properties": { "reference": {"type": "string"}, - "phone_number": {"type": "string", "format": "sms"}, + "phone_number": {"type": "string", "format": "phone_number"}, "template_id": uuid, "personalisation": personalisation }, "required": ["phone_number", "template_id"] } -content = { +sms_content = { "$schema": "http://json-schema.org/draft-04/schema#", - "description": "POST sms notification response schema", + "description": "content schema for SMS notification response schema", "type": "object", "title": "notification content", "properties": { @@ -29,7 +29,7 @@ content = { # this may belong in a templates module template = { "$schema": "http://json-schema.org/draft-04/schema#", - "description": "POST sms notification response schema", + "description": "template schema", "type": "object", "title": "notification content", "properties": { @@ -48,7 +48,50 @@ post_sms_response = { "properties": { "id": uuid, "reference": {"type": "string"}, - "content": content, + "content": sms_content, + "uri": {"type": "string"}, + "template": template + }, + "required": ["id", "content", "uri", "template"] +} + + +post_email_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST email notification schema", + "type": "object", + "title": "POST v2/notifications/email", + "properties": { + "reference": {"type": "string"}, + "email_address": {"type": "string", "format": "email_address"}, + "template_id": uuid, + "personalisation": personalisation + }, + "required": ["email_address", "template_id"] +} + +email_content = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "Email content for POST email notification", + "type": "object", + "title": "notification email content", + "properties": { + "from_email": {"type": "string", "format": "email_address"}, + "body": {"type": "string"}, + "subject": {"type": "string"} + }, + "required": ["body"] +} + +post_email_response = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST sms notification response schema", + "type": "object", + "title": "response v2/notifications/email", + "properties": { + "id": uuid, + "reference": {"type": "string"}, + "content": email_content, "uri": {"type": "string"}, "template": template }, @@ -62,7 +105,27 @@ def create_post_sms_response_from_notification(notification, body, from_number, "content": {'body': body, 'from_number': from_number}, "uri": "{}/v2/notifications/{}".format(url_root, str(notification.id)), - "template": {"id": notification.template_id, - "version": notification.template_version, - "uri": "{}/v2/templates/{}".format(url_root, str(notification.template_id))} + "template": __create_template_from_notification(notification=notification, url_root=url_root) } + + +def create_post_email_response_from_notification(notification, content, subject, email_from, url_root): + return { + "id": notification.id, + "reference": notification.reference, + "content": { + "from_email": email_from, + "body": content, + "subject": subject + }, + "uri": "{}/v2/notifications/{}".format(url_root, str(notification.id)), + "template": __create_template_from_notification(notification=notification, url_root=url_root) + } + + +def __create_template_from_notification(notification, url_root): + return { + "id": notification.template_id, + "version": notification.template_version, + "uri": "{}/v2/templates/{}".format(url_root, str(notification.template_id)) + } diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 03982f081..b8c15081a 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -1,9 +1,9 @@ -from flask import request, jsonify, current_app +from flask import request, jsonify from sqlalchemy.orm.exc import NoResultFound from app import api_user from app.dao import services_dao, templates_dao -from app.models import SMS_TYPE +from app.models import SMS_TYPE, EMAIL_TYPE from app.notifications.process_notifications import (create_content_for_notification, persist_notification, send_notification_to_queue) @@ -16,7 +16,8 @@ from app.schema_validation import validate from app.v2.errors import BadRequestError from app.v2.notifications import notification_blueprint from app.v2.notifications.notification_schemas import (post_sms_request, - create_post_sms_response_from_notification) + create_post_sms_response_from_notification, post_email_request, + create_post_email_response_from_notification) @notification_blueprint.route('/sms', methods=['POST']) @@ -27,7 +28,7 @@ def post_sms_notification(): check_service_message_limit(api_user.key_type, service) service_can_send_to_recipient(form['phone_number'], api_user.key_type, service) - template, content = __validate_template(form, service) + template, template_with_content = __validate_template(form, service, SMS_TYPE) notification = persist_notification(template_id=template.id, template_version=template.version, @@ -39,23 +40,42 @@ def post_sms_notification(): key_type=api_user.key_type) send_notification_to_queue(notification, service.research_mode) - resp = create_post_sms_response_from_notification(notification, content, service.sms_sender, request.url_root) + resp = create_post_sms_response_from_notification(notification, + template_with_content.content, + service.sms_sender, + request.url_root) return jsonify(resp), 201 @notification_blueprint.route('/email', methods=['POST']) def post_email_notification(): - # validate post form against post_email_request schema - # validate service - # validate template - # persist notification - # send notification to queue - # create content - # return post_email_response schema - pass + form = validate(request.get_json(), post_email_request) + service = services_dao.dao_fetch_service_by_id(api_user.service_id) + + check_service_message_limit(api_user.key_type, service) + service_can_send_to_recipient(form['email_address'], api_user.key_type, service) + + template, template_with_content = __validate_template(form, service, EMAIL_TYPE) + notification = persist_notification(template_id=template.id, + template_version=template.version, + recipient=form['email_address'], + service_id=service.id, + personalisation=form.get('personalisation', None), + notification_type=EMAIL_TYPE, + api_key_id=api_user.id, + key_type=api_user.key_type) + + send_notification_to_queue(notification, service.research_mode) + + resp = create_post_email_response_from_notification(notification=notification, + content=template_with_content.content, + subject=template_with_content.subject, + email_from=service.email_from, + url_root=request.url_root) + return jsonify(resp), 201 -def __validate_template(form, service): +def __validate_template(form, service, notification_type): try: template = templates_dao.dao_get_template_by_id_and_service_id(template_id=form['template_id'], service_id=service.id) @@ -64,8 +84,8 @@ def __validate_template(form, service): raise BadRequestError(message=message, fields=[{'template': message}]) - check_template_is_for_notification_type(SMS_TYPE, template.template_type) + check_template_is_for_notification_type(notification_type, template.template_type) check_template_is_active(template) template_with_content = create_content_for_notification(template, form.get('personalisation', {})) check_sms_content_char_count(template_with_content.replaced_content_count) - return template, template_with_content.content + return template, template_with_content diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index 74c734c2f..a9aceef46 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -3,8 +3,10 @@ import uuid import pytest from flask import json from jsonschema import ValidationError +from notifications_utils.recipients import InvalidPhoneError, InvalidEmailError -from app.v2.notifications.notification_schemas import post_sms_request, post_sms_response +from app.v2.notifications.notification_schemas import post_sms_request, post_sms_response, post_email_request, \ + post_email_response from app.schema_validation import validate valid_json = {"phone_number": "07515111111", @@ -54,6 +56,15 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): assert len(error.keys()) == 2 +@pytest.mark.parametrize('invalid_phone_number', + ['notaphoneumber', '08515111111', '07515111*11']) +def test_post_sms_request_invalid_phone_number(invalid_phone_number): + j = {"phone_number": invalid_phone_number, + "template_id": str(uuid.uuid4()) + } + with pytest.raises(InvalidPhoneError): + validate(j, post_sms_request) + valid_response = { "id": str(uuid.uuid4()), "content": {"body": "contents of message", @@ -90,3 +101,58 @@ def test_post_sms_response_schema_missing_uri(): assert error['status_code'] == 400 assert error['errors'] == [{'error': 'ValidationError', 'message': "uri is a required property"}] + + +valid_post_email_json = {"email_address": "test@example.gov.uk", + "template_id": str(uuid.uuid4()) + } +valid_post_emaiL_json_with_optionals = { + "email_address": "test@example.gov.uk", + "template_id": str(uuid.uuid4()), + "reference": "reference from caller", + "personalisation": {"key": "value"} +} + + +@pytest.mark.parametrize("input", [valid_post_email_json, valid_post_emaiL_json_with_optionals]) +def test_post_email_schema_is_valid(input): + assert validate(input, post_email_request) == input + + +def test_post_email_schema_bad_uuid_and_missing_email_address(): + j = {"template_id": "bad_template"} + with pytest.raises(ValidationError): + validate(j, post_email_request) + + +def test_post_email_schema_invalid_email_address(): + j = {"template_id": str(uuid.uuid4()), + "email_address": "notavalidemail@address"} + with pytest.raises(InvalidEmailError): + validate(j, post_email_request) + + +valid_email_response = {"id": str(uuid.uuid4()), + "content": {"body": "the body of the message", + "subject": "subject of the message", + "from_email": "service@dig.gov.uk"}, + "uri": "/v2/notifications/id", + "template": {"id": str(uuid.uuid4()), + "version": 1, + "uri": "/v2/template/id"} + } +valid_email_response_with_optionals = {"id": str(uuid.uuid4()), + "reference": "some reference", + "content": {"body": "the body of the message", + "subject": "subject of the message", + "from_email": "service@dig.gov.uk"}, + "uri": "/v2/notifications/id", + "template": {"id": str(uuid.uuid4()), + "version": 1, + "uri": "/v2/template/id"} + } + + +@pytest.mark.parametrize("input", [valid_email_response, valid_email_response_with_optionals]) +def test_post_email_response(input): + assert validate(input, post_email_response) == input diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index c02e2d8e9..c67066387 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -1,7 +1,7 @@ import uuid +import pytest from flask import json - from app.models import Notification from tests import create_authorization_header @@ -102,3 +102,55 @@ def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, s assert error_resp['errors'] == [{'error': 'ValidationError', 'message': "template_id is a required property" }] + + +def test_post_email_notification_returns_201(client, sample_email_template, mocker): + mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + data = { + "reference": "reference from caller", + "email_address": sample_email_template.service.users[0].email_address, + "template_id": sample_email_template.id, + } + auth_header = create_authorization_header(service_id=sample_email_template.service_id) + response = client.post( + path="v2/notifications/email", + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + notifications = Notification.query.all() + assert len(notifications) == 1 + notification_id = notifications[0].id + assert resp_json['id'] is not None + assert resp_json['reference'] is None + assert resp_json['content']['body'] == sample_email_template.content + assert resp_json['content']['subject'] == sample_email_template.subject + assert resp_json['content']['from_email'] == sample_email_template.service.email_from + assert 'v2/notifications/{}'.format(notification_id) in resp_json['uri'] + assert resp_json['template']['id'] == str(sample_email_template.id) + assert resp_json['template']['version'] == sample_email_template.version + assert 'v2/templates/{}'.format(sample_email_template.id) in resp_json['template']['uri'] + assert mocked.called + + +def test_post_email_notification_returns_404_and_missing_template(notify_api, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = { + "email_address": sample_service.users[0].email_address, + 'template_id': str(uuid.uuid4()) + } + auth_header = create_authorization_header(service_id=sample_service.id) + + response = client.post( + path='/v2/notifications/email', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 400 + assert response.headers['Content-type'] == 'application/json' + + error_json = json.loads(response.get_data(as_text=True)) + assert error_json['status_code'] == 400 + assert error_json['errors'] == [{"error": "BadRequestError", + "message": 'Template not found'}] diff --git a/tests/app/v2/test_errors.py b/tests/app/v2/test_errors.py index 5f0ed0f81..4ce3f2868 100644 --- a/tests/app/v2/test_errors.py +++ b/tests/app/v2/test_errors.py @@ -1,6 +1,7 @@ import json import pytest from flask import url_for +from notifications_utils.recipients import InvalidPhoneError from sqlalchemy.exc import DataError @@ -39,6 +40,10 @@ def app_for_test(mocker): def raising_data_error(): raise DataError("There was a db problem", "params", "orig") + @blue.route("raise_phone_error", methods=["GET"]) + def raising_invalid_phone_error(): + raise InvalidPhoneError("The phone number is wrong") + @blue.route("raise_exception", methods=["GET"]) def raising_exception(): raise AssertionError("Raising any old exception") @@ -116,3 +121,13 @@ def test_internal_server_error_handler(app_for_test): error = json.loads(response.get_data(as_text=True)) assert error == {"status_code": 500, "errors": [{"error": "AssertionError", "message": "Internal server error"}]} + + +def test_invalid_phone_error_handler(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for("v2_under_test.raising_invalid_phone_error")) + assert response.status_code == 400 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 400, + "errors": [{"error": "InvalidPhoneError", "message": "The phone number is wrong"}]}