- Add validation methods for post notification.

- Use these validation methods in post_sms_notification and the version 1 of post_notification.
- Create a v2 error handlers.
- InvalidRequest has a to_dict method for private and v1 error responses and a to_dict_v2 method to create the v2 of the error responses.
- Each validation method has extensive unit tests, so the unit test for the endpoint do not need to check every error case, but check that the error handle formats the message correctly.
- The format of the error messages is still a work on progress.
- This version of the api could be deployed without causing a problem to the application.
- The new endpoing is still a work in progress and is not being used yet.

app/__init__.py

@@ -1,7 +1,7 @@
 import uuid
 import os
 
-from flask import request, url_for, g
+from flask import request, url_for, g, jsonify
 from flask import Flask, _request_ctx_stack
 from flask.ext.sqlalchemy import SQLAlchemy
 from flask_marshmallow import Marshmallow
@@ -57,6 +57,13 @@ def create_app(app_name=None):
     encryption.init_app(application)
     clients.init_app(sms_clients=[firetext_client, mmg_client, loadtest_client], email_clients=[aws_ses_client])
 
+    register_blueprint(application)
+    register_v2_blueprints(application)
+
+    return application
+
+
+def register_blueprint(application):
     from app.service.rest import service_blueprint
     from app.user.rest import user as user_blueprint
     from app.template.rest import template as template_blueprint
@@ -81,14 +88,16 @@ def create_app(app_name=None):
     application.register_blueprint(invite_blueprint)
     application.register_blueprint(delivery_blueprint)
     application.register_blueprint(accept_invite, url_prefix='/invite')
-
     application.register_blueprint(template_statistics_blueprint)
     application.register_blueprint(events_blueprint)
     application.register_blueprint(provider_details_blueprint, url_prefix='/provider-details')
     application.register_blueprint(spec_blueprint, url_prefix='/spec')
     application.register_blueprint(organisation_blueprint, url_prefix='/organisation')
 
-    return application
+
+def register_v2_blueprints(application):
+    from app.v2.notifications.post_notifications import notification_blueprint
+    application.register_blueprint(notification_blueprint)
 
 
 def init_app(app):
@@ -120,6 +129,12 @@ def init_app(app):
         response.headers.add('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE')
         return response
 
+    @app.errorhandler(404)
+    def page_not_found(e):
+        msg = e.description or "Not found"
+        app.logger.exception(msg)
+        return jsonify(result='error', message=msg), 404
+
 
 def create_uuid():
     return str(uuid.uuid4())

app/errors.py

@@ -9,6 +9,9 @@ from app.authentication.auth import AuthError
 
 
 class InvalidRequest(Exception):
+    code = None
+    link = None
+    fields = []
 
     def __init__(self, message, status_code):
         super().__init__()
@@ -18,62 +21,67 @@ class InvalidRequest(Exception):
     def to_dict(self):
         return {'result': 'error', 'message': self.message}
 
+    def to_dict_v2(self):
+        '''
+        Version 2 of the public api error response.
+        '''
+        return {
+            "code": self.code,
+            "message": self.message,
+            "link": self.link,
+            "fields": self.fields
+        }
+
     def __str__(self):
         return str(self.to_dict())
 
 
 def register_errors(blueprint):
 
-    @blueprint.app_errorhandler(AuthError)
+    @blueprint.errorhandler(AuthError)
     def authentication_error(error):
         return jsonify(result='error', message=error.message), error.code
 
-    @blueprint.app_errorhandler(ValidationError)
+    @blueprint.errorhandler(ValidationError)
     def validation_error(error):
         current_app.logger.error(error)
         return jsonify(result='error', message=error.messages), 400
 
-    @blueprint.app_errorhandler(InvalidRequest)
+    @blueprint.errorhandler(InvalidRequest)
     def invalid_data(error):
         response = jsonify(error.to_dict())
         response.status_code = error.status_code
         current_app.logger.error(error)
         return response
 
-    @blueprint.app_errorhandler(400)
+    @blueprint.errorhandler(400)
     def bad_request(e):
         msg = e.description or "Invalid request parameters"
         current_app.logger.exception(msg)
         return jsonify(result='error', message=str(msg)), 400
 
-    @blueprint.app_errorhandler(401)
+    @blueprint.errorhandler(401)
     def unauthorized(e):
         error_message = "Unauthorized, authentication token must be provided"
         return jsonify(result='error', message=error_message), 401, [('WWW-Authenticate', 'Bearer')]
 
-    @blueprint.app_errorhandler(403)
+    @blueprint.errorhandler(403)
     def forbidden(e):
         error_message = "Forbidden, invalid authentication token provided"
         return jsonify(result='error', message=error_message), 403
 
-    @blueprint.app_errorhandler(404)
-    def page_not_found(e):
-        msg = e.description or "Not found"
-        current_app.logger.exception(msg)
-        return jsonify(result='error', message=msg), 404
-
-    @blueprint.app_errorhandler(429)
+    @blueprint.errorhandler(429)
     def limit_exceeded(e):
         current_app.logger.exception(e)
         return jsonify(result='error', message=str(e.description)), 429
 
-    @blueprint.app_errorhandler(NoResultFound)
-    @blueprint.app_errorhandler(DataError)
+    @blueprint.errorhandler(NoResultFound)
+    @blueprint.errorhandler(DataError)
     def no_result_found(e):
         current_app.logger.exception(e)
         return jsonify(result='error', message="No result found"), 404
 
-    @blueprint.app_errorhandler(SQLAlchemyError)
+    @blueprint.errorhandler(SQLAlchemyError)
     def db_error(e):
         current_app.logger.exception(e)
         if e.orig.pgerror and \
@@ -89,7 +97,7 @@ def register_errors(blueprint):
 
     # this must be defined after all other error handlers since it catches the generic Exception object
     @blueprint.app_errorhandler(500)
-    @blueprint.app_errorhandler(Exception)
+    @blueprint.errorhandler(Exception)
     def internal_server_error(e):
         current_app.logger.exception(e)
         return jsonify(result='error', message="Internal server error"), 500

app/notifications/process_notifications.py

@@ -1,3 +1,32 @@
+from notifications_utils.renderers import PassThrough
+from notifications_utils.template import Template
+
+from app.models import SMS_TYPE
+from app.notifications.validators import check_sms_content_char_count
+from app.v2.errors import BadRequestError
+
+
+def create_content_for_notification(template, personalisation):
+    template_object = Template(
+        template.__dict__,
+        personalisation,
+        renderer=PassThrough()
+    )
+    if template_object.missing_data:
+        message = 'Missing personalisation: {}'.format(", ".join(template_object.missing_data))
+        errors = {'template': [message]}
+        raise BadRequestError(errors)
+
+    if template_object.additional_data:
+        message = 'Personalisation not needed for template: {}'.format(", ".join(template_object.additional_data))
+        errors = {'template': [message]}
+        raise BadRequestError(fields=errors)
+
+    if template_object.template_type == SMS_TYPE:
+        check_sms_content_char_count(template_object.replaced_content_count)
+    return template_object
+
+
 def persist_notification():
     '''
     persist the notification

app/notifications/rest.py

@@ -24,6 +24,8 @@ from app.notifications.process_client_response import (
     validate_callback_data,
     process_sms_client_response
 )
+from app.notifications.validators import check_service_message_limit, check_template_is_for_notification_type, \
+    check_template_is_active
 from app.service.utils import service_allowed_to_send_to
 from app.schemas import (
     email_notification_schema,
@@ -31,8 +33,7 @@ from app.schemas import (
     notification_with_personalisation_schema,
     notifications_filter_schema,
     notifications_statistics_schema,
-    day_schema,
-    unarchived_template_schema
+    day_schema
 )
 from app.utils import pagination_links
 
@@ -216,26 +217,15 @@ def send_notification(notification_type):
     if errors:
         raise InvalidRequest(errors, status_code=400)
 
-    if all((api_user.key_type != KEY_TYPE_TEST,
-            service.restricted)):
-        service_stats = services_dao.fetch_todays_total_message_count(service.id)
-        if service_stats >= service.message_limit:
-            error = 'Exceeded send limits ({}) for today'.format(service.message_limit)
-            raise InvalidRequest(error, status_code=429)
+    check_service_message_limit(api_user.key_type, service)
 
     template = templates_dao.dao_get_template_by_id_and_service_id(
         template_id=notification['template'],
         service_id=service_id
     )
 
-    if notification_type != template.template_type:
-        raise InvalidRequest("{0} template is not suitable for {1} notification".format(template.template_type,
-                                                                                        notification_type),
-                             status_code=400)
-
-    errors = unarchived_template_schema.validate({'archived': template.archived})
-    if errors:
-        raise InvalidRequest(errors, status_code=400)
+    check_template_is_for_notification_type(notification_type, template.template_type)
+    check_template_is_active(template)
 
     template_object = create_template_object_for_notification(template, notification.get('personalisation', {}))
 

app/notifications/validators.py

@@ -1,6 +1,9 @@
+from flask import current_app
+
 from app.dao import services_dao
-from app.errors import InvalidRequest
-from app.models import KEY_TYPE_TEST
+from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM
+from app.service.utils import service_allowed_to_send_to
+from app.v2.errors import TooManyRequestsError, BadRequestError
 
 
 def check_service_message_limit(key_type, service):
@@ -8,18 +11,43 @@ def check_service_message_limit(key_type, service):
             service.restricted)):
         service_stats = services_dao.fetch_todays_total_message_count(service.id)
         if service_stats >= service.message_limit:
-            error = 'Exceeded send limits ({}) for today'.format(service.message_limit)
-
-            raise InvalidRequest(error, status_code=429)
+            raise TooManyRequestsError(service.message_limit)
 
 
 def check_template_is_for_notification_type(notification_type, template_type):
     if notification_type != template_type:
-        raise InvalidRequest("{0} template is not suitable for {1} notification".format(template_type,
-                                                                                        notification_type),
-                             status_code=400)
+        raise BadRequestError(
+            message="{0} template is not suitable for {1} notification".format(template_type,
+                                                                               notification_type),
+            fields=[{"template": "{0} template is not suitable for {1} notification".format(template_type,
+                                                                                            notification_type)}])
 
 
 def check_template_is_active(template):
     if template.archived:
-        raise InvalidRequest('Template has been deleted', status_code=400)
+        raise BadRequestError(fields=[{"template": "has been deleted"}],
+                              message="Template has been deleted")
+
+
+def service_can_send_to_recipient(send_to, key_type, service, recipient_type):
+    if not service_allowed_to_send_to(send_to, service, key_type):
+        if key_type == KEY_TYPE_TEAM:
+            message = 'Can’t send to this recipient using a team-only API key'
+        else:
+            message = (
+                'Can’t send to this recipient when service is in trial mode '
+                '– see https://www.notifications.service.gov.uk/trial-mode'
+            )
+        raise BadRequestError(
+            fields={recipient_type: [message]}
+        )
+
+
+def check_sms_content_char_count(content_count):
+    char_count_limit = current_app.config.get('SMS_CHAR_COUNT_LIMIT')
+    if (
+        content_count > char_count_limit
+    ):
+        message = 'Content has a character count greater than the limit of {}'.format(char_count_limit)
+        errors = {'content': [message]}
+        raise BadRequestError(fields=errors)

app/v2/errors.py

@@ -1,29 +1,37 @@
-from flask import jsonify
-
+from flask import jsonify, current_app
 from app.errors import InvalidRequest
 
 
-class BadRequestError(Exception):
-    status_code = 400
+class TooManyRequestsError(InvalidRequest):
+    status_code = 429
+    # code and link will be in a static file
+    code = "10429"
+    link = "link to docs"
+    message_template = 'Exceeded send limits ({}) for today'
 
-    def __init__(self, message, fields, code):
-        self.code = code
-        self.message = message
+    def __init__(self, sending_limit):
+        self.message = self.message_template.format(sending_limit)
+
+
+class BadRequestError(InvalidRequest):
+    status_code = 400
+    code = "10400"
+    link = "link to documentation"
+    message = "An error occurred"
+
+    def __init__(self, fields, message=None):
         self.fields = fields
+        self.message = message if message else self.message
 
 
 def register_errors(blueprint):
-    @blueprint.app_errorhandler(Exception)
+    @blueprint.errorhandler(InvalidRequest)
+    def invalid_data(error):
+        current_app.logger.error(error)
+        response = jsonify(error.to_dict_v2()), error.status_code
+        return response
+
+    @blueprint.errorhandler(Exception)
     def authentication_error(error):
         # v2 error format - NOT this
         return jsonify(result='error', message=error.message), error.code
-
-    @blueprint.app_errorhandler(InvalidRequest)
-    def handle_invalid_request(error):
-        # {
-        #     "code",
-        #     "link",
-        #     "message" ,
-        #     "fields":
-        # }
-        return "build_error_message"

app/v2/notifications/__init__.py

@@ -2,6 +2,6 @@ from flask import Blueprint
 
 from app.v2.errors import register_errors
 
-notification_blueprint = Blueprint(__name__, __name__, url_prefix='/v2/notifications')
+notification_blueprint = Blueprint("v2_notifications", __name__, url_prefix='/v2/notifications')
 
 register_errors(notification_blueprint)

app/v2/notifications/post_notifications.py

@@ -1,38 +1,40 @@
+from flask import request
+
+from app import api_user
+from app.dao import services_dao, templates_dao
+from app.models import SMS_TYPE
+from app.notifications.process_notifications import create_content_for_notification
+from app.notifications.validators import (check_service_message_limit,
+                                          check_template_is_for_notification_type,
+                                          check_template_is_active,
+                                          service_can_send_to_recipient,
+                                          check_sms_content_char_count)
+from app.schema_validation import validate
 from app.v2.notifications import notification_blueprint
+from app.v2.notifications.notification_schemas import post_sms_request
 
 
 @notification_blueprint.route('/sms', methods=['POST'])
 def post_sms_notification():
-    # # get service
-    # service = services_dao.dao_fetch_service_by_id(api_user.service_id)
-    # # validate input against json schema (not marshmallow)
-    # form = validate(request.get_json(), post_sms_request)
-    #
-    # # following checks will be in a common function for all versions of the endpoint.
-    # # check service has not exceeded the sending limit
-    # check_service_message_limit(api_user.key_type, service)
-    # template = templates_dao.dao_get_template_by_id_and_service_id(
-    #     template_id=form['template_id'],
-    #     service_id=service.id
-    # )
-    # # check template is for sms
-    # check_template_is_for_notification_type(SMS_TYPE, template.template_type)
-    # # check template is not archived
-    # check_template_is_active(template)
-    # # check service is allowed to send
-    # service_can_send_to_recipient(form['phone_number'], api_user.key_type, service)
-    # # create body of message (create_template_object_for_notification)
-    # create_template_object_for_notification(template, form.get('personalisation', {}))
-    # # persist notification
-    # # send sms to provider queue for research mode queue
-    #
+    form = validate(request.get_json(), post_sms_request)
+    service = services_dao.dao_fetch_service_by_id(api_user.service_id)
+
+    # following checks will be in a common function for all versions of the endpoint.
+    # check service has not exceeded the sending limit
+    check_service_message_limit(api_user.key_type, service)
+    service_can_send_to_recipient(form['phone_number'], api_user.key_type, service, SMS_TYPE)
+
+    template = templates_dao.dao_get_template_by_id_and_service_id(
+        template_id=form['template_id'],
+        service_id=service.id)
+
+    check_template_is_for_notification_type(SMS_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)
 
-    # validate post form against post_sms_request schema
-    # validate service
-    # validate template
-    # create content
     # persist notification
-    # send notification to queue
+    # send sms to provider queue for research mode queue
     # return post_sms_response schema
     return "post_sms_response schema", 201
 

tests/app/notifications/rest/test_send_notification.py

@@ -150,7 +150,7 @@ def test_should_not_send_notification_for_archived_template(notify_api, sample_t
                 headers=[('Content-Type', 'application/json'), auth_header])
             assert resp.status_code == 400
             json_resp = json.loads(resp.get_data(as_text=True))
-            assert 'Template has been deleted' in json_resp['message']['template']
+            assert 'Template has been deleted' in json_resp['message']
 
 
 @pytest.mark.parametrize('template_type, to',

tests/app/notifications/test_process_notification.py

@@ -0,0 +1,17 @@
+import pytest
+
+from app.models import Template
+from app.notifications.process_notifications import create_content_for_notification
+from app.v2.errors import BadRequestError
+
+
+def test_create_content_for_notification_passes(sample_email_template):
+    template = Template.query.get(sample_email_template.id)
+    content = create_content_for_notification(template, None)
+    assert content.content == template.content
+
+
+def test_create_content_for_notification_fails_with_missing_personalisation(sample_template_with_placeholders):
+    template = Template.query.get(sample_template_with_placeholders.id)
+    with pytest.raises(BadRequestError):
+        create_content_for_notification(template, None)

tests/app/notifications/test_validators.py

@@ -2,31 +2,32 @@ import pytest
 
 from app.errors import InvalidRequest
 from app.notifications.validators import check_service_message_limit, check_template_is_for_notification_type, \
-    check_template_is_active
+    check_template_is_active, service_can_send_to_recipient, check_sms_content_char_count
+from app.v2.errors import BadRequestError, TooManyRequestsError
 from tests.app.conftest import (sample_notification as create_notification,
-                                sample_service as create_service)
+                                sample_service as create_service, sample_service_whitelist)
 
 
-@pytest.mark.parametrize('key_type', ['test', 'team', 'live'])
+@pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
 def test_check_service_message_limit_with_unrestricted_service_passes(key_type,
                                                                       sample_service,
                                                                       sample_notification):
     assert check_service_message_limit(key_type, sample_service) is None
 
 
-@pytest.mark.parametrize('key_type', ['test', 'team', 'live'])
+@pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
 def test_check_service_message_limit_under_message_limit_passes(key_type,
                                                                 sample_service,
                                                                 sample_notification):
     assert check_service_message_limit(key_type, sample_service) is None
 
 
-@pytest.mark.parametrize('key_type', ['team', 'live'])
+@pytest.mark.parametrize('key_type', ['team', 'normal'])
 def test_check_service_message_limit_over_message_limit_fails(key_type, notify_db, notify_db_session):
     service = create_service(notify_db, notify_db_session, restricted=True, limit=4)
     for x in range(5):
         create_notification(notify_db, notify_db_session, service=service)
-    with pytest.raises(InvalidRequest):
+    with pytest.raises(TooManyRequestsError):
         check_service_message_limit(key_type, service)
 
 
@@ -43,7 +44,7 @@ def test_check_template_is_for_notification_type_pass(template_type, notificatio
                           ('email', 'sms')])
 def test_check_template_is_for_notification_type_fails_when_template_type_does_not_match_notification_type(
         template_type, notification_type):
-    with pytest.raises(InvalidRequest):
+    with pytest.raises(BadRequestError):
         check_template_is_for_notification_type(notification_type=notification_type,
                                                 template_type=template_type)
 
@@ -56,5 +57,91 @@ def test_check_template_is_active_passes(sample_template):
     sample_template.archived = True
     from app.dao.templates_dao import dao_update_template
     dao_update_template(sample_template)
-    with pytest.raises(InvalidRequest):
+    try:
         check_template_is_active(sample_template)
+    except BadRequestError as e:
+        assert e.status_code == 400
+        assert e.code == '10400'
+        assert e.message == 'Template has been deleted'
+        assert e.link == "link to documentation"
+        assert e.fields[0]["template"] == "has been deleted"
+
+
+@pytest.mark.parametrize('key_type',
+                         ['test', 'normal'])
+def test_service_can_send_to_recipient_passes(key_type, notify_db, notify_db_session):
+    trial_mode_service = create_service(notify_db, notify_db_session, service_name='trial mode', restricted=True)
+    assert service_can_send_to_recipient(trial_mode_service.users[0].email_address,
+                                         key_type,
+                                         trial_mode_service,
+                                         "email") is None
+    assert service_can_send_to_recipient(trial_mode_service.users[0].mobile_number,
+                                         key_type,
+                                         trial_mode_service,
+                                         "sms") is None
+
+
+@pytest.mark.parametrize('key_type',
+                         ['test', 'normal'])
+def test_service_can_send_to_recipient_passes_for_live_service_non_team_member(key_type, notify_db, notify_db_session):
+    live_service = create_service(notify_db, notify_db_session, service_name='live', restricted=False)
+    assert service_can_send_to_recipient("some_other_email@test.com",
+                                         key_type,
+                                         live_service,
+                                         "email") is None
+    assert service_can_send_to_recipient('07513332413',
+                                         key_type,
+                                         live_service,
+                                         "sms") is None
+
+
+@pytest.mark.parametrize('key_type',
+                         ['team'])
+def test_service_can_send_to_recipient_passes_for_whitelisted_recipient_passes(key_type, notify_db, notify_db_session,
+                                                                               sample_service):
+    sample_service_whitelist(notify_db, notify_db_session, email_address="some_other_email@test.com")
+    assert service_can_send_to_recipient("some_other_email@test.com",
+                                         key_type,
+                                         sample_service,
+                                         "email") is None
+    sample_service_whitelist(notify_db, notify_db_session, mobile_number='07513332413')
+    assert service_can_send_to_recipient('07513332413',
+                                         key_type,
+                                         sample_service,
+                                         "sms") is None
+
+
+@pytest.mark.parametrize('key_type',
+                         ['team', 'normal'])
+def test_service_can_send_to_recipient_fails_when_recipient_is_not_on_team(key_type, notify_db, notify_db_session):
+    trial_mode_service = create_service(notify_db, notify_db_session, service_name='trial mode', restricted=True)
+    with pytest.raises(BadRequestError):
+        assert service_can_send_to_recipient("some_other_email@test.com",
+                                             key_type,
+                                             trial_mode_service,
+                                             "email") is None
+    with pytest.raises(BadRequestError):
+        assert service_can_send_to_recipient('07513332413',
+                                             key_type,
+                                             trial_mode_service,
+                                             "sms") is None
+
+
+def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(notify_db, notify_db_session):
+    live_service = create_service(notify_db, notify_db_session, service_name='live mode', restricted=False)
+    with pytest.raises(BadRequestError):
+        assert service_can_send_to_recipient("0758964221",
+                                             'team',
+                                             live_service,
+                                             "sms") is None
+
+
+@pytest.mark.parametrize('char_count', [495, 0, 494, 200])
+def test_check_sms_content_char_count_passes(char_count, notify_api):
+    assert check_sms_content_char_count(char_count) is None
+
+
+@pytest.mark.parametrize('char_count', [496, 500, 6000])
+def test_check_sms_content_char_count_fails(char_count, notify_api):
+    with pytest.raises(BadRequestError):
+        check_sms_content_char_count(char_count)

tests/app/v2/notifications/test_post_notifications.py

@@ -0,0 +1,43 @@
+from flask import json
+
+from tests import create_authorization_header
+
+
+def test_post_sms_notification_returns_201(notify_api, sample_template):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            data = {
+                'phone_number': '+447700900855',
+                'template_id': str(sample_template.id)
+            }
+            auth_header = create_authorization_header(service_id=sample_template.service_id)
+
+            response = client.post(
+                path='/v2/notifications/sms',
+                data=json.dumps(data),
+                headers=[('Content-Type', 'application/json'), auth_header])
+
+            assert response.status_code == 201
+
+
+def test_post_sms_notification_returns_404_when_template_is_wrong_type(notify_api, sample_email_template):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            data = {
+                'phone_number': '+447700900855',
+                'template_id': str(sample_email_template.id)
+            }
+            auth_header = create_authorization_header(service_id=sample_email_template.service_id)
+
+            response = client.post(
+                path='/v2/notifications/sms',
+                data=json.dumps(data),
+                headers=[('Content-Type', 'application/json'), auth_header])
+
+            assert response.status_code == 400
+            resp_text = json.loads(response.get_data(as_text=True))
+            assert resp_text['code'] == '10400'
+            assert resp_text['message'] == '{0} template is not suitable for {1} notification'.format('email', 'sms')
+            assert resp_text['link'] == 'link to documentation'
+            field = "{0} template is not suitable for {1} notification".format("email", "sms")
+            assert resp_text['fields'][0]['template'] == field