diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 297fa4d39..1b66bd36d 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -43,6 +43,7 @@ def persist_notification(template_id, reference=None, notification_id=None, simulated=False): + # if simulated create a Notification model to return but do not persist the Notification to the dB notification = Notification( id=notification_id, template_id=template_id, @@ -62,6 +63,9 @@ def persist_notification(template_id, if not simulated: dao_create_notification(notification) redis_store.incr(redis.daily_limit_cache_key(service.id)) + current_app.logger.info( + "{} {} created at {}".format(notification.notification_type, notification.id, notification.created_at) + ) return notification @@ -87,8 +91,9 @@ def send_notification_to_queue(notification, research_mode, queue=None): raise SendNotificationToQueueError() current_app.logger.info( - "{} {} created at {}".format(notification.notification_type, notification.id, notification.created_at) - ) + "{} {} sent to the {} queue for delivery".format(notification.notification_type, + notification.id, + queue)) def simulated_recipient(to_address, notification_type): diff --git a/app/notifications/rest.py b/app/notifications/rest.py index ecc93c7e4..da49eed42 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -217,7 +217,7 @@ def send_notification(notification_type): service = services_dao.dao_fetch_service_by_id(api_user.service_id) - notification, errors = ( + notification_form, errors = ( sms_template_notification_schema if notification_type == SMS_TYPE else email_notification_schema ).load(request.get_json()) if errors: @@ -225,40 +225,40 @@ def send_notification(notification_type): check_service_message_limit(api_user.key_type, service) - template = templates_dao.dao_get_template_by_id_and_service_id(template_id=notification['template'], + template = templates_dao.dao_get_template_by_id_and_service_id(template_id=notification_form['template'], service_id=service.id) 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', {})) + template_object = create_template_object_for_notification(template, notification_form.get('personalisation', {})) - _service_allowed_to_send_to(notification, service) + _service_allowed_to_send_to(notification_form, service) # Do not persist or send notification to the queue if it is a simulated recipient - simulated = simulated_recipient(notification['to'], notification_type) - saved_notification = persist_notification(template_id=template.id, + simulated = simulated_recipient(notification_form['to'], notification_type) + notification_model = persist_notification(template_id=template.id, template_version=template.version, - recipient=notification['to'], + recipient=notification_form['to'], service=service, - personalisation=notification.get('personalisation', None), + personalisation=notification_form.get('personalisation', None), notification_type=notification_type, api_key_id=api_user.id, key_type=api_user.key_type, simulated=simulated) if not simulated: queue_name = 'notify' if template.process_type == PRIORITY else None - send_notification_to_queue(notification=saved_notification, + send_notification_to_queue(notification=notification_model, research_mode=service.research_mode, queue=queue_name) else: - current_app.logger.info("POST simulated notification for id: {}".format(saved_notification.id)) - notification.update({"template_version": template.version}) + current_app.logger.info("POST simulated notification for id: {}".format(notification_model.id)) + notification_form.update({"template_version": template.version}) return jsonify( data=get_notification_return_data( - saved_notification.id, - notification, + notification_model.id, + notification_form, template_object) ), 201 diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index dd86fcc99..7473cb8c0 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -14,7 +14,7 @@ from app.v2.notifications.notification_schemas import get_notifications_request def get_notification_by_id(id): try: casted_id = uuid.UUID(id) - except ValueError: + except ValueError or AttributeError: abort(404) notification = notifications_dao.get_notification_with_personalisation( api_user.service_id, casted_id, key_type=None diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index db60e2f47..96f36722f 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -35,11 +35,12 @@ def test_get_sms_notification_by_id(client, sample_notification): assert not notification.get('subject') -def test_get_sms_notification_by_invalid_id(client, sample_notification): +@pytest.mark.parametrize("id", ["1234-badly-formatted-id-7890", "0"]) +def test_get_sms_notification_by_invalid_id(client, sample_notification, id): auth_header = create_authorization_header(service_id=sample_notification.service_id) response = client.get( - '/notifications/{}'.format("not_a_valid_id"), + '/notifications/{}'.format(id), headers=[auth_header]) assert response.status_code == 405 diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index e7a21ff8f..5c2c31af3 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -164,10 +164,11 @@ def test_get_notification_by_id_nonexistent_id(client, sample_notification): } -def test_get_notification_by_id_invalid_id(client, sample_notification): +@pytest.mark.parametrize("id", ["1234-badly-formatted-id-7890", "0"]) +def test_get_notification_by_id_invalid_id(client, sample_notification, id): auth_header = create_authorization_header(service_id=sample_notification.service_id) response = client.get( - path='/v2/notifications/1234-badly-formatted-id-7890', + path='/v2/notifications/{}'.format(id), headers=[('Content-Type', 'application/json'), auth_header]) assert response.status_code == 404 diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 3e1089b6a..8ad0204e8 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -159,7 +159,7 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ ('07700 900111', 'sms'), ('07700 900222', 'sms') ]) -def test_should_not_persist_notification_or_send_notification_if_simulated_recipient( +def test_should_not_persist_or_send_notification_if_simulated_recipient( client, recipient, notification_type,