Merge branch 'master' into caching-with-redis

Conflicts:
	tests/app/notifications/test_process_notification.py

app/models.py

@@ -519,6 +519,7 @@ class Notification(db.Model):
         onupdate=datetime.datetime.utcnow)
     status = db.Column(NOTIFICATION_STATUS_TYPES_ENUM, index=True, nullable=False, default='created')
     reference = db.Column(db.String, nullable=True, index=True)
+    client_reference = db.Column(db.String, index=True, nullable=True)
     _personalisation = db.Column(db.String, nullable=True)
 
     template_history = db.relationship('TemplateHistory', primaryjoin=and_(
@@ -561,6 +562,7 @@ class NotificationHistory(db.Model):
     updated_at = db.Column(db.DateTime, index=False, unique=False, nullable=True)
     status = db.Column(NOTIFICATION_STATUS_TYPES_ENUM, index=True, nullable=False, default='created')
     reference = db.Column(db.String, nullable=True, index=True)
+    client_reference = db.Column(db.String, nullable=True)
 
     @classmethod
     def from_notification(cls, notification):

app/notifications/process_notifications.py

@@ -47,7 +47,8 @@ def persist_notification(template_id,
                          key_type,
                          created_at=None,
                          job_id=None,
-                         job_row_number=None):
+                         job_row_number=None,
+                         reference=None):
     notification = Notification(
         template_id=template_id,
         template_version=template_version,
@@ -57,9 +58,10 @@ def persist_notification(template_id,
         notification_type=notification_type,
         api_key_id=api_key_id,
         key_type=key_type,
-        created_at=created_at if created_at else datetime.utcnow().strftime(DATETIME_FORMAT),
+        created_at=created_at or datetime.utcnow().strftime(DATETIME_FORMAT),
         job_id=job_id,
-        job_row_number=job_row_number
+        job_row_number=job_row_number,
+        client_reference=reference
     )
     dao_create_notification(notification)
     redis_store.incr(redis.daily_limit_cache_key(service_id))

app/schema_validation/__init__.py

@@ -1,27 +1,45 @@
 import json
 
-from jsonschema import Draft4Validator, ValidationError
+from jsonschema import (Draft4Validator, ValidationError, FormatChecker)
+from notifications_utils.recipients import (validate_phone_number, validate_email_address, InvalidPhoneError,
+                                            InvalidEmailError)
 
 
 def validate(json_to_validate, schema):
-    validator = Draft4Validator(schema)
+    format_checker = FormatChecker()
+
+    @format_checker.checks('phone_number', raises=InvalidPhoneError)
+    def validate_schema_phone_number(instance):
+        validate_phone_number(instance)
+        return True
+
+    @format_checker.checks('email_address', raises=InvalidEmailError)
+    def validate_schema_email_address(instance):
+        validate_email_address(instance)
+        return True
+
+    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))
+        raise ValidationError(build_error_message(errors))
     return json_to_validate
 
 
-def build_error_message(errors, schema):
+def build_error_message(errors):
     fields = []
     for e in errors:
-        field = "'{}' {}".format(e.path[0], e.schema.get('validationMessage')) if e.schema.get(
+        field = "{} {}".format(e.path[0], e.schema.get('validationMessage')) if e.schema.get(
-            'validationMessage') else e.message
+            'validationMessage') else __format_message(e)
-        s = field.split("'")
+        fields.append({"error": "ValidationError", "message": field})
-        field = {"error": "ValidationError", "message": "{}{}".format(s[1], s[2])}
-        fields.append(field)
     message = {
         "status_code": 400,
         "errors": fields
     }
 
     return json.dumps(message)
+
+
+def __format_message(e):
+    s = e.message.split("'")
+    msg = "{}{}".format(s[1], s[2])
+    return msg if not e.cause else "{} {}".format(e.path[0], e.cause.message)

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", "from_email", "subject"]
+}
+
+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
     },
@@ -58,11 +101,31 @@ post_sms_response = {
 
 def create_post_sms_response_from_notification(notification, body, from_number, url_root):
     return {"id": notification.id,
-            "reference": None,  # not yet implemented
+            "reference": notification.client_reference,
             "content": {'body': body,
                         'from_number': from_number},
             "uri": "{}/v2/notifications/{}".format(url_root, str(notification.id)),
-            "template": {"id": notification.template_id,
+            "template": __create_template_from_notification(notification=notification, url_root=url_root)
-                         "version": notification.template_version,
-                         "uri": "{}/v2/templates/{}".format(url_root, str(notification.template_id))}
             }
+
+
+def create_post_email_response_from_notification(notification, content, subject, email_from, url_root):
+    return {
+        "id": notification.id,
+        "reference": notification.client_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))
+    }

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,
@@ -36,26 +37,47 @@ def post_sms_notification():
                                         personalisation=form.get('personalisation', None),
                                         notification_type=SMS_TYPE,
                                         api_key_id=api_user.id,
-                                        key_type=api_user.key_type)
+                                        key_type=api_user.key_type,
+                                        reference=form['reference'])
     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
+    form = validate(request.get_json(), post_email_request)
-    # validate service
+    service = services_dao.dao_fetch_service_by_id(api_user.service_id)
-    # validate template
+
-    # persist notification
+    check_service_message_limit(api_user.key_type, service)
-    # send notification to queue
+    service_can_send_to_recipient(form['email_address'], api_user.key_type, service)
-    # create content
+
-    # return post_email_response schema
+    template, template_with_content = __validate_template(form, service, EMAIL_TYPE)
-    pass
+    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,
+                                        reference=form['reference'])
+
+    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 +86,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

migrations/versions/0061_add_client_reference.py

@@ -0,0 +1,24 @@
+"""empty message
+
+Revision ID: 0061_add_client_reference
+Revises: 0060_add_letter_template_type
+Create Date: 2016-11-17 13:19:25.820617
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '0061_add_client_reference'
+down_revision = '0060_add_letter_template_type'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.add_column('notifications', sa.Column('client_reference', sa.String(), index=True, nullable=True))
+    op.add_column('notification_history', sa.Column('client_reference', sa.String(), nullable=True))
+
+
+def downgrade():
+    op.drop_column('notifications', 'client_reference')
+    op.drop_column('notification_history', 'client_reference')

tests/app/notifications/test_process_notification.py

@@ -104,7 +104,7 @@ def test_cache_is_not_incremented_on_failure_to_persist_notification(sample_api_
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
-def test_persist_notification_with_job_and_created(sample_job, sample_api_key, mocker):
+def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker):
     assert Notification.query.count() == 0
     assert NotificationHistory.query.count() == 0
     mocked_redis = mocker.patch('app.notifications.process_notifications.redis_store.incr')
@@ -119,7 +119,8 @@ def test_persist_notification_with_job_and_created(sample_job, sample_api_key, m
                          key_type=sample_api_key.key_type,
                          created_at=created_at,
                          job_id=sample_job.id,
-                         job_row_number=10)
+                         job_row_number=10,
+                         reference="ref from client")
     assert Notification.query.count() == 1
     assert NotificationHistory.query.count() == 1
     persisted_notification = Notification.query.all()[0]
@@ -127,6 +128,8 @@ def test_persist_notification_with_job_and_created(sample_job, sample_api_key, m
     assert persisted_notification.job_row_number == 10
     assert persisted_notification.created_at == created_at
     mocked_redis.assert_called_once_with(str(sample_job.service_id) + "-2016-01-01-count")
+    assert persisted_notification.client_reference == "ref from client"
+    assert persisted_notification.reference is None
 
 
 @pytest.mark.parametrize('research_mode, queue, notification_type, key_type',
@@ -151,7 +154,7 @@ def test_send_notification_to_queue(notify_db, notify_db_session,
 
 def test_send_notification_to_queue_throws_exception_deletes_notification(sample_notification, mocker):
     mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async', side_effect=Boto3Error("EXPECTED"))
-    with pytest.raises(SendNotificationToQueueError):
+    with pytest.raises(Boto3Error):
         send_notification_to_queue(sample_notification, False)
         mocked.assert_called_once_with([(str(sample_notification.id))], queue='send-sms')
 

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,32 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict():
     assert len(error.keys()) == 2
 
 
+@pytest.mark.parametrize('invalid_phone_number, err_msg',
+                         [('08515111111', 'phone_number Not a UK mobile number'),
+                          ('07515111*11', 'phone_number Must not contain letters or symbols'),
+                          ('notaphoneumber', 'phone_number Must not contain letters or symbols')])
+def test_post_sms_request_invalid_phone_number(invalid_phone_number, err_msg):
+    j = {"phone_number": invalid_phone_number,
+         "template_id": str(uuid.uuid4())
+         }
+    with pytest.raises(ValidationError) as e:
+        validate(j, post_sms_request)
+    errors = json.loads(e.value.message).get('errors')
+    assert len(errors) == 1
+    assert {"error": "ValidationError", "message": err_msg} == errors[0]
+
+
+def test_post_sms_request_invalid_phone_number_and_missing_template():
+    j = {"phone_number": '08515111111',
+         }
+    with pytest.raises(ValidationError) as e:
+        validate(j, post_sms_request)
+    errors = json.loads(e.value.message).get('errors')
+    assert len(errors) == 2
+    assert {"error": "ValidationError", "message": "phone_number Not a UK mobile number"} in errors
+    assert {"error": "ValidationError", "message": "template_id is a required property"} in errors
+
+
 valid_response = {
     "id": str(uuid.uuid4()),
     "content": {"body": "contents of message",
@@ -90,3 +118,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(ValidationError):
+        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

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
 
@@ -12,7 +12,8 @@ def test_post_sms_notification_returns_201(notify_api, sample_template, mocker):
             mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
             data = {
                 'phone_number': '+447700900855',
-                'template_id': str(sample_template.id)
+                'template_id': str(sample_template.id),
+                'reference': 'reference_from_client'
             }
             auth_header = create_authorization_header(service_id=sample_template.service_id)
 
@@ -27,7 +28,7 @@ def test_post_sms_notification_returns_201(notify_api, sample_template, mocker):
             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['reference'] == 'reference_from_client'
             assert resp_json['content']['body'] == sample_template.content
             assert resp_json['content']['from_number'] == sample_template.service.sms_sender
             assert 'v2/notifications/{}'.format(notification_id) in resp_json['uri']
@@ -102,3 +103,54 @@ 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))
+    notification = Notification.query.first()
+    assert resp_json['id'] == str(notification.id)
+    assert resp_json['reference'] == "reference from caller"
+    assert notification.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'}]