darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-08-12 18:08:05 -04:00
Code Issues Packages Projects Releases Wiki Activity

- Refactor version 1 of post notificaitons to use the common persist_notificaiton and send_notification_to_queue methods.

Browse Source 
- It would be nice to refactor the send_sms and send_email tasks to use these common functions as well, that way I can get rid of the new Notifications.from_v2_api_request method.
- Still not happy with the format of the errors. Would like to find a happy place, where the message is descript enough that we do not need external documentation to explain the error. Perhaps we still only need documentation to explain the trial mode concept.
This commit is contained in:
Rebecca Law
2016-10-28 17:10:00 +01:00
parent 6e4bad135a
commit 8cf2fc72a8
11 changed files with 109 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
app/dao/notifications_dao.py
View File

@@ -1,4 +1,3 @@
import uuid
import pytz
from datetime import (
datetime,
@@ -12,7 +11,7 @@ from werkzeug.datastructures import MultiDict
from sqlalchemy import (desc, func, or_, and_, asc, cast, Text)
from sqlalchemy.orm import joinedload
from app import db
from app import db, create_uuid
from app.dao import days_ago
from app.models import (
Service,
@@ -125,7 +124,7 @@ def dao_get_last_template_usage(template_id):
def dao_create_notification(notification):
if not notification.id:
# need to populate defaulted fields before we create the notification history object
notification.id = uuid.uuid4()
notification.id = create_uuid()
if not notification.status:
notification.status = 'created'

5
app/models.py
View File

@@ -22,7 +22,7 @@ from app.authentication.utils import get_secret
from app import (
db,
encryption,
DATETIME_FORMAT, create_uuid)
DATETIME_FORMAT)
from app.history_meta import Versioned
@@ -573,13 +573,12 @@ class Notification(db.Model):
api_key_id,
key_type):
return cls(
id=create_uuid(),
template_id=template_id,
template_version=template_version,
to=recipient,
service_id=service_id,
status='created',
created_at=datetime.datetime.strftime(datetime.datetime.utcnow(), DATETIME_FORMAT),
created_at=datetime.datetime.utcnow(),
personalisation=personalisation,
notification_type=notification_type,
api_key_id=api_key_id,

40
app/notifications/process_notifications.py
View File

@@ -4,7 +4,6 @@ from notifications_utils.template import Template
from app.celery import provider_tasks
from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id
from app.errors import InvalidRequest
from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE
from app.notifications.validators import check_sms_content_char_count
from app.v2.errors import BadRequestError
@@ -16,21 +15,24 @@ def create_content_for_notification(template, personalisation):
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)
check_placeholders(template_object)
if template_object.template_type == SMS_TYPE:
check_sms_content_char_count(template_object.replaced_content_count)
return template_object
def check_placeholders(template_object):
if template_object.missing_data:
message = 'Template missing personalisation: {}'.format(", ".join(template_object.missing_data))
raise BadRequestError(message=message)
if template_object.additional_data:
message = 'Template personalisation not needed for template: {}'.format(
", ".join(template_object.additional_data))
raise BadRequestError(message=message)
def persist_notification(template_id,
template_version,
recipient,
@@ -39,14 +41,14 @@ def persist_notification(template_id,
notification_type,
api_key_id,
key_type):
notification = Notification.from_v2_api_request(template_id,
template_version,
recipient,
service_id,
personalisation,
notification_type,
api_key_id,
key_type)
notification = Notification.from_v2_api_request(template_id=template_id,
template_version=template_version,
recipient=recipient,
service_id=service_id,
personalisation=personalisation,
notification_type=notification_type,
api_key_id=api_key_id,
key_type=key_type)
dao_create_notification(notification)
return notification
@@ -67,7 +69,7 @@ def send_notification_to_queue(notification, research_mode):
except Exception as e:
current_app.logger.exception("Failed to send to SQS exception")
dao_delete_notifications_and_history_by_id(notification.id)
raise InvalidRequest(message="Internal server error", status_code=500)
raise
current_app.logger.info(
"{} {} created at {}".format(notification.notification_type, notification.id, notification.created_at)

80
app/notifications/rest.py
View File

@@ -7,26 +7,25 @@ from flask import (
current_app,
json
)
from notifications_utils.template import Template
from notifications_utils.renderers import PassThrough
from notifications_utils.template import Template
from app import api_user, create_uuid, statsd_client
from app.clients.email.aws_ses import get_aws_responses
from app import api_user, create_uuid, DATETIME_FORMAT, statsd_client
from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id
from app.models import KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, KEY_TYPE_NORMAL, EMAIL_TYPE
from app.dao import (
templates_dao,
services_dao,
notifications_dao
)
from app.models import KEY_TYPE_TEAM
from app.models import SMS_TYPE
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.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,
sms_template_notification_schema,
@@ -35,6 +34,7 @@ from app.schemas import (
notifications_statistics_schema,
day_schema
)
from app.service.utils import service_allowed_to_send_to
from app.utils import pagination_links
notifications = Blueprint('notifications', __name__)
@@ -45,7 +45,6 @@ from app.errors import (
)
register_errors(notifications)
from app.celery import provider_tasks
@notifications.route('/notifications/email/ses', methods=['POST'])
@@ -219,10 +218,8 @@ 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
)
template = templates_dao.dao_get_template_by_id_and_service_id(template_id=notification['template'],
service_id=service_id)
check_template_is_for_notification_type(notification_type, template.template_type)
check_template_is_active(template)
@@ -231,18 +228,20 @@ def send_notification(notification_type):
_service_allowed_to_send_to(notification, service)
notification_id = create_uuid()
notification.update({"template_version": template.version})
saved_notification = None
if not _simulated_recipient(notification['to'], notification_type):
persist_notification(
service,
notification_id,
notification,
datetime.utcnow().strftime(DATETIME_FORMAT),
notification_type,
str(api_user.id),
api_user.key_type
)
saved_notification = persist_notification(template_id=template.id,
template_version=template.version,
recipient=notification['to'],
service_id=service.id,
personalisation=notification.get('personalisation', None),
notification_type=notification_type,
api_key_id=api_user.id,
key_type=api_user.key_type)
send_notification_to_queue(saved_notification, service.research_mode)
notification_id = create_uuid() if saved_notification is None else saved_notification.id
notification.update({"template_version": template.version})
return jsonify(
data=get_notification_return_data(
@@ -271,43 +270,6 @@ def _simulated_recipient(to_address, notification_type):
else to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES'])
def persist_notification(
service,
notification_id,
notification,
created_at,
notification_type,
api_key_id=None,
key_type=KEY_TYPE_NORMAL,
):
dao_create_notification(
Notification.from_api_request(
created_at, notification, notification_id, service.id, notification_type, api_key_id, key_type
)
)
try:
research_mode = service.research_mode or key_type == KEY_TYPE_TEST
if notification_type == SMS_TYPE:
provider_tasks.deliver_sms.apply_async(
[str(notification_id)],
queue='send-sms' if not research_mode else 'research-mode'
)
if notification_type == EMAIL_TYPE:
provider_tasks.deliver_email.apply_async(
[str(notification_id)],
queue='send-email' if not research_mode else 'research-mode'
)
except Exception as e:
current_app.logger.exception("Failed to send to SQS exception")
dao_delete_notifications_and_history_by_id(notification_id)
raise InvalidRequest(message="Internal server error", status_code=500)
current_app.logger.info(
"{} {} created at {}".format(notification_type, notification_id, created_at)
)
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:

18
app/notifications/validators.py
View File

@@ -18,18 +18,15 @@ def check_template_is_for_notification_type(notification_type, template_type):
if notification_type != template_type:
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)}])
notification_type))
def check_template_is_active(template):
if template.archived:
raise BadRequestError(fields=[{"template": "has been deleted"}],
message="Template has been deleted")
raise BadRequestError(message="Template has been deleted")
def service_can_send_to_recipient(send_to, key_type, service, recipient_type):
def service_can_send_to_recipient(send_to, key_type, service):
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'
@@ -38,9 +35,7 @@ def service_can_send_to_recipient(send_to, key_type, service, recipient_type):
'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]}
)
raise BadRequestError(message=message)
def check_sms_content_char_count(content_count):
@@ -48,6 +43,5 @@ def check_sms_content_char_count(content_count):
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)
message = 'Content for template has a character count greater than the limit of {}'.format(char_count_limit)
raise BadRequestError(message=message)

2
app/v2/errors.py
View File

@@ -19,7 +19,7 @@ class BadRequestError(InvalidRequest):
link = "link to documentation"
message = "An error occurred"
def __init__(self, fields, message=None):
def __init__(self, fields=None, message=None):
self.fields = fields
self.message = message if message else self.message

31
app/v2/notifications/post_notifications.py
View File

@@ -2,8 +2,9 @@ from flask import request, jsonify
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, persist_notification, \
send_notification_to_queue
from app.notifications.process_notifications import (create_content_for_notification,
persist_notification,
send_notification_to_queue)
from app.notifications.validators import (check_service_message_limit,
check_template_is_for_notification_type,
check_template_is_active,
@@ -20,21 +21,11 @@ def post_sms_notification():
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)
service_can_send_to_recipient(form['phone_number'], 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)
template, content = __validate_template(form, service)
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)
# persist notification
notification = persist_notification(template_id=template.id,
template_version=template.version,
recipient=form['phone_number'],
@@ -44,7 +35,7 @@ def post_sms_notification():
api_key_id=api_user.id,
key_type=api_user.key_type)
send_notification_to_queue(notification, service.research_mode)
resp = create_post_sms_response_from_notification(notification, template_with_content.content)
resp = create_post_sms_response_from_notification(notification, content)
return jsonify(resp), 201
@@ -58,3 +49,13 @@ def post_email_notification():
# create content
# return post_email_response schema
pass
def __validate_template(form, 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_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)
return template, template_with_content.replaced_content_count

12
tests/app/notifications/rest/test_send_notification.py
View File

@@ -516,7 +516,7 @@ def test_should_not_send_sms_if_team_api_key_and_not_a_service_user(notify_api,
def test_should_send_email_if_team_api_key_and_a_service_user(notify_api, sample_email_template, fake_uuid, mocker):
with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
data = {
'to': sample_email_template.service.created_by.email_address,
@@ -545,7 +545,7 @@ def test_should_send_sms_to_anyone_with_test_key(
):
with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
data = {
'to': '07811111111',
@@ -578,7 +578,7 @@ def test_should_send_email_to_anyone_with_test_key(
):
with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
data = {
'to': 'anyone123@example.com',
@@ -608,7 +608,7 @@ def test_should_send_email_to_anyone_with_test_key(
def test_should_send_sms_if_team_api_key_and_a_service_user(notify_api, sample_template, fake_uuid, mocker):
with notify_api.test_request_context(), notify_api.test_client() as client:
mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
data = {
'to': sample_template.service.created_by.mobile_number,
@@ -638,7 +638,7 @@ def test_should_persist_notification(notify_api, sample_template,
fake_uuid, mocker):
with notify_api.test_request_context(), notify_api.test_client() as client:
mocked = mocker.patch('app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type))
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
template = sample_template if template_type == 'sms' else sample_email_template
to = sample_template.service.created_by.mobile_number if template_type == 'sms' \
else sample_email_template.service.created_by.email_address
@@ -682,7 +682,7 @@ def test_should_delete_notification_and_return_error_if_sqs_fails(
'app.celery.provider_tasks.deliver_{}.apply_async'.format(template_type),
side_effect=Exception("failed to talk to SQS")
)
mocker.patch('app.notifications.rest.create_uuid', return_value=fake_uuid)
m1 = mocker.patch('app.dao.notifications_dao.create_uuid', return_value=fake_uuid)
template = sample_template if template_type == 'sms' else sample_email_template
to = sample_template.service.created_by.mobile_number if template_type == 'sms' \
else sample_email_template.service.created_by.email_address

27
tests/app/notifications/test_process_notification.py
View File

@@ -1,7 +1,7 @@
import pytest
from boto3.exceptions import Boto3Error
from sqlalchemy.exc import SQLAlchemyError
from app.errors import InvalidRequest
from app.models import Template, Notification, NotificationHistory
from app.notifications.process_notifications import (create_content_for_notification,
persist_notification, send_notification_to_queue)
@@ -12,7 +12,14 @@ from tests.app.conftest import sample_notification, sample_template, sample_emai
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.replaced == template.content
def test_create_content_for_notification_with_placeholders_passes(sample_template_with_placeholders):
template = Template.query.get(sample_template_with_placeholders.id)
content = create_content_for_notification(template, {'name': 'Bobby'})
assert content.content == template.content
assert 'Bobby' in content.replaced
def test_create_content_for_notification_fails_with_missing_personalisation(sample_template_with_placeholders):
@@ -21,6 +28,12 @@ def test_create_content_for_notification_fails_with_missing_personalisation(samp
create_content_for_notification(template, None)
def test_create_content_for_notification_fails_with_additional_personalisation(sample_template_with_placeholders):
template = Template.query.get(sample_template_with_placeholders.id)
with pytest.raises(BadRequestError):
create_content_for_notification(template, {'name': 'Bobbhy', 'Additional': 'Data'})
def test_persist_notification_creates_and_save_to_db(sample_template, sample_api_key):
assert Notification.query.count() == 0
assert NotificationHistory.query.count() == 0
@@ -34,6 +47,7 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api
def test_persist_notification_throws_exception_when_missing_template(sample_template, sample_api_key):
assert Notification.query.count() == 0
assert NotificationHistory.query.count() == 0
with pytest.raises(SQLAlchemyError):
persist_notification(template_id=None,
template_version=None,
@@ -42,6 +56,8 @@ def test_persist_notification_throws_exception_when_missing_template(sample_temp
personalisation=None, notification_type='sms',
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type)
assert Notification.query.count() == 0
assert NotificationHistory.query.count() == 0
@pytest.mark.parametrize('research_mode, queue, notification_type, key_type',
@@ -62,8 +78,11 @@ def test_send_notification_to_queue(notify_db, notify_db_session,
mocked.assert_called_once_with([str(notification.id)], queue=queue)
def test_send_notification_to_queue(sample_notification, mocker):
mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async', side_effect=Exception("EXPECTED"))
with pytest.raises(InvalidRequest):
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(Boto3Error):
send_notification_to_queue(sample_notification, False)
mocked.assert_called_once_with([(str(sample_notification.id))], queue='send-sms')
assert Notification.query.count() == 0
assert NotificationHistory.query.count() == 0

31
tests/app/notifications/test_validators.py
View File

@@ -1,6 +1,5 @@
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, service_can_send_to_recipient, check_sms_content_char_count
from app.v2.errors import BadRequestError, TooManyRequestsError
@@ -53,7 +52,7 @@ def test_check_template_is_active_passes(sample_template):
assert check_template_is_active(sample_template) is None
def test_check_template_is_active_passes(sample_template):
def test_check_template_is_active_fails(sample_template):
sample_template.archived = True
from app.dao.templates_dao import dao_update_template
dao_update_template(sample_template)
@@ -64,7 +63,6 @@ def test_check_template_is_active_passes(sample_template):
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',
@@ -73,12 +71,10 @@ def test_service_can_send_to_recipient_passes(key_type, notify_db, notify_db_ses
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
trial_mode_service) is None
assert service_can_send_to_recipient(trial_mode_service.users[0].mobile_number,
key_type,
trial_mode_service,
"sms") is None
trial_mode_service) is None
@pytest.mark.parametrize('key_type',
@@ -87,12 +83,10 @@ def test_service_can_send_to_recipient_passes_for_live_service_non_team_member(k
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
live_service) is None
assert service_can_send_to_recipient('07513332413',
key_type,
live_service,
"sms") is None
live_service) is None
@pytest.mark.parametrize('key_type',
@@ -102,13 +96,11 @@ def test_service_can_send_to_recipient_passes_for_whitelisted_recipient_passes(k
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) 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
sample_service) is None
@pytest.mark.parametrize('key_type',
@@ -118,13 +110,11 @@ def test_service_can_send_to_recipient_fails_when_recipient_is_not_on_team(key_t
with pytest.raises(BadRequestError):
assert service_can_send_to_recipient("some_other_email@test.com",
key_type,
trial_mode_service,
"email") is None
trial_mode_service) is None
with pytest.raises(BadRequestError):
assert service_can_send_to_recipient('07513332413',
key_type,
trial_mode_service,
"sms") is None
trial_mode_service) is None
def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(notify_db, notify_db_session):
@@ -132,8 +122,7 @@ def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(n
with pytest.raises(BadRequestError):
assert service_can_send_to_recipient("0758964221",
'team',
live_service,
"sms") is None
live_service) is None
@pytest.mark.parametrize('char_count', [495, 0, 494, 200])

3
tests/app/v2/notifications/test_post_notifications.py
View File

@@ -46,5 +46,4 @@ def test_post_sms_notification_returns_404_when_template_is_wrong_type(notify_ap
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
assert resp_text.get('fields', None) is None