From b34c5f0bcd9a0c30745fcd12af312db7ed6e8a70 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 17 Jan 2017 13:16:26 +0000 Subject: [PATCH] If the template.process_type = PRIROITY send the message to the notify queue. We are using the notify queue in this iteration because that queue is a low volume queue with it's own dedicated workers. This just saves us from building a new queue at this point, and a new queue may not be necessary. --- app/notifications/rest.py | 12 +- app/v2/notifications/post_notifications.py | 5 +- tests/app/conftest.py | 6 +- .../rest/test_send_notification.py | 33 ++++ .../notifications/test_post_notifications.py | 178 ++++++++++-------- 5 files changed, 145 insertions(+), 89 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 9695471c6..ecc93c7e4 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -15,7 +15,7 @@ from app.dao import ( services_dao, notifications_dao ) -from app.models import KEY_TYPE_TEAM +from app.models import KEY_TYPE_TEAM, PRIORITY from app.models import SMS_TYPE from app.notifications.process_client_response import ( validate_callback_data, @@ -215,8 +215,7 @@ def send_notification(notification_type): if notification_type not in ['sms', 'email']: assert False - service_id = str(api_user.service_id) - service = services_dao.dao_fetch_service_by_id(service_id) + service = services_dao.dao_fetch_service_by_id(api_user.service_id) notification, errors = ( sms_template_notification_schema if notification_type == SMS_TYPE else email_notification_schema @@ -227,7 +226,7 @@ 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'], - service_id=service_id) + service_id=service.id) check_template_is_for_notification_type(notification_type, template.template_type) check_template_is_active(template) @@ -248,7 +247,10 @@ def send_notification(notification_type): key_type=api_user.key_type, simulated=simulated) if not simulated: - send_notification_to_queue(saved_notification, service.research_mode) + queue_name = 'notify' if template.process_type == PRIORITY else None + send_notification_to_queue(notification=saved_notification, + 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}) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 136aca0b2..3674d3a90 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -3,7 +3,7 @@ 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, EMAIL_TYPE +from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY from app.notifications.process_notifications import (create_content_for_notification, persist_notification, send_notification_to_queue, @@ -50,7 +50,8 @@ def post_notification(notification_type): reference=form.get('reference', None), simulated=simulated) if not simulated: - send_notification_to_queue(notification, service.research_mode) + queue_name = 'notify' if template.process_type == PRIORITY else None + send_notification_to_queue(notification=notification, research_mode=service.research_mode, queue=queue_name) else: current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) if notification_type == SMS_TYPE: diff --git a/tests/app/conftest.py b/tests/app/conftest.py index ea702ecd4..14db86218 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -156,7 +156,8 @@ def sample_template(notify_db, subject_line='Subject', user=None, service=None, - created_by=None): + created_by=None, + process_type='normal'): if user is None: user = create_user() if service is None: @@ -169,7 +170,8 @@ def sample_template(notify_db, 'content': content, 'service': service, 'created_by': created_by, - 'archived': archived + 'archived': archived, + 'process_type': process_type } if template_type in ['email', 'letter']: data.update({ diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 89478db2f..baf655b9a 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -957,3 +957,36 @@ def test_create_template_raises_invalid_request_when_content_too_large( if not should_error: pytest.fail("do not expect an InvalidRequest") assert e.message == {'content': ['Content has a character count greater than the limit of {}'.format(limit)]} + + +@pytest.mark.parametrize("notification_type, send_to", + [("sms", "07700 900 855"), + ("email", "sample@email.com")]) +def test_send_notification_uses_priority_queue_when_template_is_marked_as_priority(client, notify_db, + notify_db_session, mocker, + notification_type, send_to): + sample = create_sample_template( + notify_db, + notify_db_session, + template_type=notification_type, + process_type='priority' + ) + mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type)) + + data = { + 'to': send_to, + 'template': str(sample.id) + } + + auth_header = create_authorization_header(service_id=sample.service_id) + + response = client.post( + path='/notifications/{}'.format(notification_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + response_data = json.loads(response.data)['data'] + notification_id = response_data['notification']['id'] + + assert response.status_code == 201 + mocked.assert_called_once_with([notification_id], queue='notify') diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 9c73400ab..3e1089b6a 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -1,9 +1,9 @@ import uuid - import pytest from flask import json from app.models import Notification from tests import create_authorization_header +from tests.app.conftest import sample_template as create_sample_template @pytest.mark.parametrize("reference", [None, "reference_from_client"]) @@ -43,71 +43,77 @@ def test_post_sms_notification_returns_201(notify_api, sample_template_with_plac assert mocked.called -def test_post_sms_notification_returns_404_and_missing_template(notify_api, sample_service): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - data = { - 'phone_number': '+447700900855', - 'template_id': str(uuid.uuid4()) - } - auth_header = create_authorization_header(service_id=sample_service.id) +@pytest.mark.parametrize("notification_type, key_send_to, send_to", + [("sms", "phone_number", "+447700900855"), + ("email", "email_address", "sample@email.com")]) +def test_post_sms_notification_returns_404_and_missing_template(client, sample_service, + notification_type, key_send_to, send_to): + data = { + key_send_to: send_to, + 'template_id': str(uuid.uuid4()) + } + auth_header = create_authorization_header(service_id=sample_service.id) - response = client.post( - path='/v2/notifications/sms', - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header]) + response = client.post( + path='/v2/notifications/{}'.format(notification_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 400 - assert response.headers['Content-type'] == 'application/json' + 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'}] + 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'}] -def test_post_sms_notification_returns_403_and_well_formed_auth_error(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) - } +@pytest.mark.parametrize("notification_type, key_send_to, send_to", + [("sms", "phone_number", "+447700900855"), + ("email", "email_address", "sample@email.com")]) +def test_post_notification_returns_403_and_well_formed_auth_error(client, sample_template, + notification_type, key_send_to, send_to): + data = { + key_send_to: send_to, + 'template_id': str(sample_template.id) + } - response = client.post( - path='/v2/notifications/sms', - data=json.dumps(data), - headers=[('Content-Type', 'application/json')]) + response = client.post( + path='/v2/notifications/{}'.format(notification_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json')]) - assert response.status_code == 401 - assert response.headers['Content-type'] == 'application/json' - error_resp = json.loads(response.get_data(as_text=True)) - assert error_resp['status_code'] == 401 - assert error_resp['errors'] == [{'error': "AuthError", - 'message': 'Unauthorized, authentication token must be provided'}] + assert response.status_code == 401 + assert response.headers['Content-type'] == 'application/json' + error_resp = json.loads(response.get_data(as_text=True)) + assert error_resp['status_code'] == 401 + assert error_resp['errors'] == [{'error': "AuthError", + 'message': 'Unauthorized, authentication token must be provided'}] -def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, sample_template): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - data = { - 'phone_number': '+447700900855', - 'template': str(sample_template.id) - } - auth_header = create_authorization_header(service_id=sample_template.service_id) +@pytest.mark.parametrize("notification_type, key_send_to, send_to", + [("sms", "phone_number", "+447700900855"), + ("email", "email_address", "sample@email.com")]) +def test_notification_returns_400_and_for_schema_problems(client, sample_template, notification_type, key_send_to, + send_to): + data = { + key_send_to: send_to, + 'template': 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]) + response = client.post( + path='/v2/notifications/{}'.format(notification_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 400 - assert response.headers['Content-type'] == 'application/json' - error_resp = json.loads(response.get_data(as_text=True)) - assert error_resp['status_code'] == 400 - assert error_resp['errors'] == [{'error': 'ValidationError', - 'message': "template_id is a required property" - }] + assert response.status_code == 400 + assert response.headers['Content-type'] == 'application/json' + error_resp = json.loads(response.get_data(as_text=True)) + assert error_resp['status_code'] == 400 + assert error_resp['errors'] == [{'error': 'ValidationError', + 'message': "template_id is a required property" + }] @pytest.mark.parametrize("reference", [None, "reference_from_client"]) @@ -131,9 +137,9 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ assert resp_json['id'] == str(notification.id) assert resp_json['reference'] == reference assert notification.reference is None - assert resp_json['content']['body'] == sample_email_template_with_placeholders.content\ + assert resp_json['content']['body'] == sample_email_template_with_placeholders.content \ .replace('((name))', 'Bob').replace('GOV.UK', u'GOV.\u200bUK') - assert resp_json['content']['subject'] == sample_email_template_with_placeholders.subject\ + assert resp_json['content']['subject'] == sample_email_template_with_placeholders.subject \ .replace('((name))', 'Bob') assert resp_json['content']['from_email'] == sample_email_template_with_placeholders.service.email_from assert 'v2/notifications/{}'.format(notification.id) in resp_json['uri'] @@ -145,29 +151,6 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ 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'}] - - @pytest.mark.parametrize('recipient, notification_type', [ ('simulate-delivered@notifications.service.gov.uk', 'email'), ('simulate-delivered-2@notifications.service.gov.uk', 'email'), @@ -206,3 +189,38 @@ def test_should_not_persist_notification_or_send_notification_if_simulated_recip assert response.status_code == 201 apply_async.assert_not_called() assert Notification.query.count() == 0 + + +@pytest.mark.parametrize("notification_type, key_send_to, send_to", + [("sms", "phone_number", "07700 900 855"), + ("email", "email_address", "sample@email.com")]) +def test_send_notification_uses_priority_queue_when_template_is_marked_as_priority(client, notify_db, + notify_db_session, + mocker, + notification_type, + key_send_to, + send_to): + sample = create_sample_template( + notify_db, + notify_db_session, + template_type=notification_type, + process_type='priority' + ) + mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(notification_type)) + + data = { + key_send_to: send_to, + 'template_id': str(sample.id) + } + + auth_header = create_authorization_header(service_id=sample.service_id) + + response = client.post( + path='/v2/notifications/{}'.format(notification_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + + notification_id = json.loads(response.data)['id'] + + assert response.status_code == 201 + mocked.assert_called_once_with([notification_id], queue='notify')