- Refactor v2 post_notification to use a single method for sms and email.

- Added the `simulate` notification logic to version 2. We have 3 email addresses and phone numbers that are used
to simulate a successful post to /notifications. This was missed out of the version 2 endpoint.
- Added a test to template_dao to check for the default value of normal for new templates
- in v2 get_notifications, casted the path param to a uuid, if not uuid abort(404)

app/notifications/process_notifications.py

@@ -41,7 +41,8 @@ def persist_notification(template_id,
                          job_id=None,
                          job_row_number=None,
                          reference=None,
-                         notification_id=None):
+                         notification_id=None,
+                         simulated=False):
     notification = Notification(
         id=notification_id,
         template_id=template_id,
@@ -58,8 +59,9 @@ def persist_notification(template_id,
         job_row_number=job_row_number,
         client_reference=reference
     )
-    dao_create_notification(notification)
+    if not simulated:
-    redis_store.incr(redis.daily_limit_cache_key(service.id))
+        dao_create_notification(notification)
+        redis_store.incr(redis.daily_limit_cache_key(service.id))
     return notification
 
 
@@ -87,3 +89,9 @@ def send_notification_to_queue(notification, research_mode, queue=None):
     current_app.logger.info(
         "{} {} created at {}".format(notification.notification_type, notification.id, notification.created_at)
     )
+
+
+def simulated_recipient(to_address, notification_type):
+    return (to_address in current_app.config['SIMULATED_SMS_NUMBERS']
+            if notification_type == SMS_TYPE
+            else to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES'])

app/notifications/rest.py

@@ -8,7 +8,7 @@ from flask import (
     json
 )
 
-from app import api_user, create_uuid, statsd_client
+from app import api_user, statsd_client
 from app.clients.email.aws_ses import get_aws_responses
 from app.dao import (
     templates_dao,
@@ -21,9 +21,12 @@ from app.notifications.process_client_response import (
     validate_callback_data,
     process_sms_client_response
 )
-from app.notifications.process_notifications import persist_notification, send_notification_to_queue
+from app.notifications.process_notifications import (persist_notification,
-from app.notifications.validators import check_service_message_limit, check_template_is_for_notification_type, \
+                                                     send_notification_to_queue,
-    check_template_is_active
+                                                     simulated_recipient)
+from app.notifications.validators import (check_service_message_limit,
+                                          check_template_is_for_notification_type,
+                                          check_template_is_active)
 from app.schemas import (
     email_notification_schema,
     sms_template_notification_schema,
@@ -233,23 +236,26 @@ def send_notification(notification_type):
 
     _service_allowed_to_send_to(notification, service)
 
-    saved_notification = None
+    # Do not persist or send notification to the queue if it is a simulated recipient
-    if not _simulated_recipient(notification['to'], notification_type):
+    simulated = simulated_recipient(notification['to'], notification_type)
-        saved_notification = persist_notification(template_id=template.id,
+    saved_notification = persist_notification(template_id=template.id,
-                                                  template_version=template.version,
+                                              template_version=template.version,
-                                                  recipient=notification['to'],
+                                              recipient=notification['to'],
-                                                  service=service,
+                                              service=service,
-                                                  personalisation=notification.get('personalisation', None),
+                                              personalisation=notification.get('personalisation', None),
-                                                  notification_type=notification_type,
+                                              notification_type=notification_type,
-                                                  api_key_id=api_user.id,
+                                              api_key_id=api_user.id,
-                                                  key_type=api_user.key_type)
+                                              key_type=api_user.key_type,
+                                              simulated=simulated)
+    if not simulated:
         send_notification_to_queue(saved_notification, service.research_mode)
-
+    else:
-    notification_id = create_uuid() if saved_notification is None else saved_notification.id
+        current_app.logger.info("POST simulated notification for id: {}".format(saved_notification.id))
     notification.update({"template_version": template.version})
+
     return jsonify(
         data=get_notification_return_data(
-            notification_id,
+            saved_notification.id,
             notification,
             template_object)
     ), 201
@@ -268,12 +274,6 @@ def get_notification_return_data(notification_id, notification, template):
     return output
 
 
-def _simulated_recipient(to_address, notification_type):
-    return (to_address in current_app.config['SIMULATED_SMS_NUMBERS']
-            if notification_type == SMS_TYPE
-            else to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES'])
-
-
 def _service_allowed_to_send_to(notification, service):
     if not service_allowed_to_send_to(notification['to'], service, api_user.key_type):
         if api_user.key_type == KEY_TYPE_TEAM:

app/notifications/validators.py

@@ -1,7 +1,8 @@
 from flask import current_app
+from notifications_utils.recipients import validate_and_format_phone_number, validate_and_format_email_address
 
 from app.dao import services_dao
-from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM
+from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE
 from app.service.utils import service_allowed_to_send_to
 from app.v2.errors import TooManyRequestsError, BadRequestError
 from app import redis_store
@@ -44,6 +45,14 @@ def service_can_send_to_recipient(send_to, key_type, service):
         raise BadRequestError(message=message)
 
 
+def validate_and_format_recipient(send_to, key_type, service, notification_type):
+    service_can_send_to_recipient(send_to, key_type, service)
+    if notification_type == SMS_TYPE:
+        return validate_and_format_phone_number(number=send_to)
+    else:
+        return validate_and_format_email_address(email_address=send_to)
+
+
 def check_sms_content_char_count(content_count):
     char_count_limit = current_app.config.get('SMS_CHAR_COUNT_LIMIT')
     if (

app/v2/notifications/get_notifications.py

@@ -1,4 +1,8 @@
+import uuid
+
 from flask import jsonify, request, url_for
+from werkzeug.exceptions import abort
+
 from app import api_user
 from app.dao import notifications_dao
 from app.schema_validation import validate
@@ -6,10 +10,14 @@ from app.v2.notifications import notification_blueprint
 from app.v2.notifications.notification_schemas import get_notifications_request
 
 
-@notification_blueprint.route("/<uuid:id>", methods=['GET'])
+@notification_blueprint.route("/<id>", methods=['GET'])
 def get_notification_by_id(id):
+    try:
+        casted_id = uuid.UUID(id)
+    except ValueError:
+        abort(404)
     notification = notifications_dao.get_notification_with_personalisation(
-        str(api_user.service_id), id, key_type=None
+        api_user.service_id, casted_id, key_type=None
     )
 
     return jsonify(notification.serialize()), 200

app/v2/notifications/post_notifications.py

@@ -6,12 +6,13 @@ from app.dao import services_dao, templates_dao
 from app.models import SMS_TYPE, EMAIL_TYPE
 from app.notifications.process_notifications import (create_content_for_notification,
                                                      persist_notification,
-                                                     send_notification_to_queue)
+                                                     send_notification_to_queue,
+                                                     simulated_recipient)
 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,
-                                          check_sms_content_char_count)
+                                          validate_and_format_recipient)
 from app.schema_validation import validate
 from app.v2.errors import BadRequestError
 from app.v2.notifications import notification_blueprint
@@ -20,62 +21,52 @@ from app.v2.notifications.notification_schemas import (post_sms_request,
                                                        create_post_email_response_from_notification)
 
 
-@notification_blueprint.route('/sms', methods=['POST'])
+@notification_blueprint.route('/<notification_type>', methods=['POST'])
-def post_sms_notification():
+def post_notification(notification_type):
-    form = validate(request.get_json(), post_sms_request)
+    if notification_type == EMAIL_TYPE:
+        form = validate(request.get_json(), post_email_request)
+    else:
+        form = validate(request.get_json(), post_sms_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['phone_number'], api_user.key_type, service)
+    form_send_to = form['phone_number'] if notification_type == SMS_TYPE else form['email_address']
+    send_to = validate_and_format_recipient(send_to=form_send_to,
+                                            key_type=api_user.key_type,
+                                            service=service,
+                                            notification_type=notification_type)
 
-    template, template_with_content = __validate_template(form, service, SMS_TYPE)
+    template, template_with_content = __validate_template(form, service, notification_type)
 
+    # Do not persist or send notification to the queue if it is a simulated recipient
+    simulated = simulated_recipient(send_to, notification_type)
     notification = persist_notification(template_id=template.id,
                                         template_version=template.version,
-                                        recipient=form['phone_number'],
+                                        recipient=send_to,
                                         service=service,
                                         personalisation=form.get('personalisation', None),
-                                        notification_type=SMS_TYPE,
+                                        notification_type=notification_type,
                                         api_key_id=api_user.id,
                                         key_type=api_user.key_type,
-                                        reference=form.get('reference'))
+                                        reference=form.get('reference', None),
-    send_notification_to_queue(notification, service.research_mode)
+                                        simulated=simulated)
-    sms_sender = service.sms_sender if service.sms_sender else current_app.config.get('FROM_NUMBER')
+    if not simulated:
-    resp = create_post_sms_response_from_notification(notification=notification,
+        send_notification_to_queue(notification, service.research_mode)
-                                                      body=str(template_with_content),
+    else:
-                                                      from_number=sms_sender,
+        current_app.logger.info("POST simulated notification for id: {}".format(notification.id))
-                                                      url_root=request.url_root,
+    if notification_type == SMS_TYPE:
-                                                      service_id=service.id)
+        sms_sender = service.sms_sender if service.sms_sender else current_app.config.get('FROM_NUMBER')
-    return jsonify(resp), 201
+        resp = create_post_sms_response_from_notification(notification=notification,
-
+                                                          body=str(template_with_content),
-
+                                                          from_number=sms_sender,
-@notification_blueprint.route('/email', methods=['POST'])
+                                                          url_root=request.url_root,
-def post_email_notification():
+                                                          service_id=service.id)
-    form = validate(request.get_json(), post_email_request)
+    else:
-    service = services_dao.dao_fetch_service_by_id(api_user.service_id)
+        resp = create_post_email_response_from_notification(notification=notification,
-
+                                                            content=str(template_with_content),
-    check_service_message_limit(api_user.key_type, service)
+                                                            subject=template_with_content.subject,
-    service_can_send_to_recipient(form['email_address'], api_user.key_type, service)
+                                                            email_from=service.email_from,
-
+                                                            url_root=request.url_root,
-    template, template_with_content = __validate_template(form, service, EMAIL_TYPE)
+                                                            service_id=service.id)
-    notification = persist_notification(template_id=template.id,
-                                        template_version=template.version,
-                                        recipient=form['email_address'],
-                                        service=service,
-                                        personalisation=form.get('personalisation', None),
-                                        notification_type=EMAIL_TYPE,
-                                        api_key_id=api_user.id,
-                                        key_type=api_user.key_type,
-                                        reference=form.get('reference'))
-
-    send_notification_to_queue(notification, service.research_mode)
-
-    resp = create_post_email_response_from_notification(notification=notification,
-                                                        content=str(template_with_content),
-                                                        subject=template_with_content.subject,
-                                                        email_from=service.email_from,
-                                                        url_root=request.url_root,
-                                                        service_id=service.id)
     return jsonify(resp), 201
 
 

config.py

@@ -140,8 +140,8 @@ class Config(object):
     SENDING_NOTIFICATIONS_TIMEOUT_PERIOD = 259200  # 3 days
 
     SIMULATED_EMAIL_ADDRESSES = ('simulate-delivered@notifications.service.gov.uk',
-                                 'simulate-permanent-failure@notifications.service.gov.uk',
+                                 'simulate-delivered-2@notifications.service.gov.uk',
-                                 'simulate-temporary-failure@notifications.service.gov.uk',
+                                 'simulate-delivered-3@notifications.service.gov.uk',
                                  )
 
     SIMULATED_SMS_NUMBERS = ('+447700900000', '+447700900111', '+447700900222')

tests/app/dao/test_templates_dao.py

@@ -31,6 +31,7 @@ def test_create_template(sample_service, sample_user, template_type, subject):
     assert Template.query.count() == 1
     assert len(dao_get_all_templates_for_service(sample_service.id)) == 1
     assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template'
+    assert dao_get_all_templates_for_service(sample_service.id)[0].process_type == 'normal'
 
 
 def test_update_template(sample_service, sample_user):

tests/app/notifications/rest/test_send_notification.py

@@ -710,8 +710,8 @@ def test_should_delete_notification_and_return_error_if_sqs_fails(
 
 @pytest.mark.parametrize('to_email', [
     'simulate-delivered@notifications.service.gov.uk',
-    'simulate-permanent-failure@notifications.service.gov.uk',
+    'simulate-delivered-2@notifications.service.gov.uk',
-    'simulate-temporary-failure@notifications.service.gov.uk'
+    'simulate-delivered-3@notifications.service.gov.uk'
 ])
 def test_should_not_persist_notification_or_send_email_if_simulated_email(
         client,

tests/app/notifications/test_process_notification.py

@@ -10,7 +10,9 @@ from collections import namedtuple
 from app.models import Template, Notification, NotificationHistory
 from app.notifications import SendNotificationToQueueError
 from app.notifications.process_notifications import (create_content_for_notification,
-                                                     persist_notification, send_notification_to_queue)
+                                                     persist_notification,
+                                                     send_notification_to_queue,
+                                                     simulated_recipient)
 from app.v2.errors import BadRequestError
 
 
@@ -193,3 +195,24 @@ def test_send_notification_to_queue_throws_exception_deletes_notification(sample
 
     assert Notification.query.count() == 0
     assert NotificationHistory.query.count() == 0
+
+
+@pytest.mark.parametrize("to_address, notification_type, expected",
+                         [("+447700900000", "sms", True),
+                          ("+447700900111", "sms", True),
+                          ("+447700900222", "sms", True),
+                          ("simulate-delivered@notifications.service.gov.uk", "email", True),
+                          ("simulate-delivered-2@notifications.service.gov.uk", "email", True),
+                          ("simulate-delivered-3@notifications.service.gov.uk", "email", True),
+                          ("07515896969", "sms", False),
+                          ("valid_email@test.com", "email", False)])
+def test_simulated_recipient(notify_api, to_address, notification_type, expected):
+    # The values where the expected = 'research-mode' are listed in the config['SIMULATED_EMAIL_ADDRESSES']
+    # and config['SIMULATED_SMS_NUMBERS']. These values should result in using the research mode queue.
+    #  SIMULATED_EMAIL_ADDRESSES = ('simulate-delivered@notifications.service.gov.uk',
+    #                               'simulate-delivered-2@notifications.service.gov.uk',
+    #                               'simulate-delivered-2@notifications.service.gov.uk')
+    #  SIMULATED_SMS_NUMBERS = ('+447700900000', '+447700900111', '+447700900222')
+
+    actual = simulated_recipient(to_address, notification_type)
+    assert actual == expected

tests/app/notifications/test_rest.py

@@ -13,34 +13,43 @@ from tests import create_authorization_header
 from tests.app.conftest import sample_notification as create_sample_notification
 
 
-def test_get_sms_notification_by_id(notify_api, sample_notification):
+def test_get_sms_notification_by_id(client, sample_notification):
-    with notify_api.test_request_context():
+        auth_header = create_authorization_header(service_id=sample_notification.service_id)
-        with notify_api.test_client() as client:
-            auth_header = create_authorization_header(service_id=sample_notification.service_id)
 
-            response = client.get(
+        response = client.get(
-                '/notifications/{}'.format(sample_notification.id),
+            '/notifications/{}'.format(sample_notification.id),
-                headers=[auth_header])
+            headers=[auth_header])
 
-            assert response.status_code == 200
+        assert response.status_code == 200
-            notification = json.loads(response.get_data(as_text=True))['data']['notification']
+        notification = json.loads(response.get_data(as_text=True))['data']['notification']
-            assert notification['status'] == 'created'
+        assert notification['status'] == 'created'
-            assert notification['template'] == {
+        assert notification['template'] == {
-                'id': str(sample_notification.template.id),
+            'id': str(sample_notification.template.id),
-                'name': sample_notification.template.name,
+            'name': sample_notification.template.name,
-                'template_type': sample_notification.template.template_type,
+            'template_type': sample_notification.template.template_type,
-                'version': 1
+            'version': 1
-            }
+        }
-            assert notification['to'] == '+447700900855'
+        assert notification['to'] == '+447700900855'
-            assert notification['service'] == str(sample_notification.service_id)
+        assert notification['service'] == str(sample_notification.service_id)
-            assert notification['body'] == "This is a template:\nwith a newline"
+        assert notification['body'] == "This is a template:\nwith a newline"
-            assert not notification.get('subject')
+        assert not notification.get('subject')
+
+
+def test_get_sms_notification_by_invalid_id(client, sample_notification):
+        auth_header = create_authorization_header(service_id=sample_notification.service_id)
+
+        response = client.get(
+            '/notifications/{}'.format("not_a_valid_id"),
+            headers=[auth_header])
+
+        assert response.status_code == 405
 
 
 def test_get_email_notification_by_id(notify_api, notify_db, notify_db_session, sample_email_template):
 
     email_notification = create_sample_notification(notify_db,
                                                     notify_db_session,
+                                                    to_field="sample_email@example.com",
                                                     service=sample_email_template.service,
                                                     template=sample_email_template,
                                                     status='sending')
@@ -62,7 +71,7 @@ def test_get_email_notification_by_id(notify_api, notify_db, notify_db_session,
                 'template_type': email_notification.template.template_type,
                 'version': 1
             }
-            assert notification['to'] == '+447700900855'
+            assert notification['to'] == 'sample_email@example.com'
             assert notification['service'] == str(email_notification.service_id)
             assert response.status_code == 200
             assert notification['body'] == sample_email_template.content

tests/app/v2/notifications/test_post_notifications.py

@@ -166,3 +166,43 @@ def test_post_email_notification_returns_404_and_missing_template(notify_api, sa
             assert error_json['status_code'] == 400
             assert error_json['errors'] == [{"error": "BadRequestError",
                                              "message": 'Template not found'}]
+
+
+@pytest.mark.parametrize('recipient, notification_type', [
+    ('simulate-delivered@notifications.service.gov.uk', 'email'),
+    ('simulate-delivered-2@notifications.service.gov.uk', 'email'),
+    ('simulate-delivered-3@notifications.service.gov.uk', 'email'),
+    ('07700 900000', 'sms'),
+    ('07700 900111', 'sms'),
+    ('07700 900222', 'sms')
+])
+def test_should_not_persist_notification_or_send_notification_if_simulated_recipient(
+        client,
+        recipient,
+        notification_type,
+        sample_email_template,
+        sample_template,
+        mocker):
+    apply_async = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type))
+
+    if notification_type == 'sms':
+        data = {
+            'phone_number': recipient,
+            'template_id': str(sample_template.id)
+        }
+    else:
+        data = {
+            'email_address': recipient,
+            '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/{}'.format(notification_type),
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), auth_header])
+
+    assert response.status_code == 201
+    apply_async.assert_not_called()
+    assert Notification.query.count() == 0