mirror of
https://github.com/GSA/notifications-api.git
synced 2026-08-11 09:27:56 -04:00
Serialise template immediately after fetching
This commit changes the code in post notification endpoint to handle a serialised template (ie a `dict`) rather than a database object. This is the first step towards being able to cache the template and not hit the database on every request. There should be no functional changes here, it’s just refactoring. There are some changes to the tests where the signature of functions has changed. Importing of the template schema has to be done at a function level, otherwise Marshmallow gets weird.
This commit is contained in:
@@ -2,6 +2,8 @@ from app.models import LETTER_TYPE
|
||||
from app.models import Notification
|
||||
from app.models import NOTIFICATION_CREATED
|
||||
from app.notifications.process_letter_notifications import create_letter_notification
|
||||
from app.notifications.process_notifications import create_content_for_notification
|
||||
from app.notifications.validators import get_template_dict
|
||||
|
||||
|
||||
def test_create_letter_notification_creates_notification(sample_letter_template, sample_api_key):
|
||||
@@ -13,7 +15,17 @@ def test_create_letter_notification_creates_notification(sample_letter_template,
|
||||
}
|
||||
}
|
||||
|
||||
notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED)
|
||||
template = create_content_for_notification(get_template_dict(
|
||||
sample_letter_template.id, sample_letter_template.service_id
|
||||
), {})
|
||||
|
||||
notification = create_letter_notification(
|
||||
data,
|
||||
template,
|
||||
sample_letter_template.service,
|
||||
sample_api_key,
|
||||
NOTIFICATION_CREATED,
|
||||
)
|
||||
|
||||
assert notification == Notification.query.one()
|
||||
assert notification.job is None
|
||||
@@ -38,7 +50,17 @@ def test_create_letter_notification_sets_reference(sample_letter_template, sampl
|
||||
'reference': 'foo'
|
||||
}
|
||||
|
||||
notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED)
|
||||
template = create_content_for_notification(get_template_dict(
|
||||
sample_letter_template.id, sample_letter_template.service_id
|
||||
), {})
|
||||
|
||||
notification = create_letter_notification(
|
||||
data,
|
||||
template,
|
||||
sample_letter_template.service,
|
||||
sample_api_key,
|
||||
NOTIFICATION_CREATED,
|
||||
)
|
||||
|
||||
assert notification.client_reference == 'foo'
|
||||
|
||||
@@ -52,7 +74,17 @@ def test_create_letter_notification_sets_billable_units(sample_letter_template,
|
||||
},
|
||||
}
|
||||
|
||||
notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED,
|
||||
billable_units=3)
|
||||
template = create_content_for_notification(get_template_dict(
|
||||
sample_letter_template.id, sample_letter_template.service_id
|
||||
), {})
|
||||
|
||||
notification = create_letter_notification(
|
||||
data,
|
||||
template,
|
||||
sample_letter_template.service,
|
||||
sample_api_key,
|
||||
NOTIFICATION_CREATED,
|
||||
billable_units=3,
|
||||
)
|
||||
|
||||
assert notification.billable_units == 3
|
||||
|
||||
@@ -21,6 +21,7 @@ from app.notifications.process_notifications import (
|
||||
send_notification_to_queue,
|
||||
simulated_recipient
|
||||
)
|
||||
from app.notifications.validators import get_template_dict
|
||||
from notifications_utils.recipients import validate_and_format_phone_number, validate_and_format_email_address
|
||||
from app.v2.errors import BadRequestError
|
||||
from tests.app.db import create_service, create_template
|
||||
@@ -28,26 +29,30 @@ from tests.app.db import create_service, create_template
|
||||
|
||||
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)
|
||||
template_dict = get_template_dict(template.id, template.service_id)
|
||||
content = create_content_for_notification(template_dict, None)
|
||||
assert str(content) == template.content + '\n'
|
||||
|
||||
|
||||
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'})
|
||||
template_dict = get_template_dict(template.id, template.service_id)
|
||||
content = create_content_for_notification(template_dict, {'name': 'Bobby'})
|
||||
assert content.content == template.content
|
||||
assert 'Bobby' in str(content)
|
||||
|
||||
|
||||
def test_create_content_for_notification_fails_with_missing_personalisation(sample_template_with_placeholders):
|
||||
template = Template.query.get(sample_template_with_placeholders.id)
|
||||
template_dict = get_template_dict(template.id, template.service_id)
|
||||
with pytest.raises(BadRequestError):
|
||||
create_content_for_notification(template, None)
|
||||
create_content_for_notification(template_dict, None)
|
||||
|
||||
|
||||
def test_create_content_for_notification_allows_additional_personalisation(sample_template_with_placeholders):
|
||||
template = Template.query.get(sample_template_with_placeholders.id)
|
||||
create_content_for_notification(template, {'name': 'Bobby', 'Additional placeholder': 'Data'})
|
||||
template_dict = get_template_dict(template.id, template.service_id)
|
||||
create_content_for_notification(template_dict, {'name': 'Bobby', 'Additional placeholder': 'Data'})
|
||||
|
||||
|
||||
@freeze_time("2016-01-01 11:09:00.061258")
|
||||
|
||||
@@ -19,6 +19,7 @@ from app.notifications.validators import (
|
||||
check_service_sms_sender_id,
|
||||
check_service_letter_contact_id,
|
||||
check_reply_to,
|
||||
get_template_dict,
|
||||
service_can_send_to_recipient,
|
||||
validate_and_format_recipient,
|
||||
validate_template,
|
||||
@@ -175,15 +176,17 @@ def test_check_template_is_for_notification_type_fails_when_template_type_does_n
|
||||
|
||||
|
||||
def test_check_template_is_active_passes(sample_template):
|
||||
assert check_template_is_active(sample_template) is None
|
||||
template_dict = get_template_dict(sample_template.id, sample_template.service_id)
|
||||
assert check_template_is_active(template_dict) is None
|
||||
|
||||
|
||||
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)
|
||||
template_dict = get_template_dict(sample_template.id, sample_template.service_id)
|
||||
with pytest.raises(BadRequestError) as e:
|
||||
check_template_is_active(sample_template)
|
||||
check_template_is_active(template_dict)
|
||||
assert e.value.status_code == 400
|
||||
assert e.value.message == 'Template has been deleted'
|
||||
assert e.value.fields == [{'template': 'Template has been deleted'}]
|
||||
@@ -314,11 +317,11 @@ def test_check_content_char_count_passes_for_long_email_or_letter(sample_service
|
||||
|
||||
def test_check_notification_content_is_not_empty_passes(notify_api, mocker, sample_service):
|
||||
template_id = create_template(sample_service, content="Content is not empty").id
|
||||
template = templates_dao.dao_get_template_by_id_and_service_id(
|
||||
template_dict = get_template_dict(
|
||||
template_id=template_id,
|
||||
service_id=sample_service.id
|
||||
)
|
||||
template_with_content = create_content_for_notification(template, {})
|
||||
template_with_content = create_content_for_notification(template_dict, {})
|
||||
assert check_notification_content_is_not_empty(template_with_content) is None
|
||||
|
||||
|
||||
@@ -330,11 +333,11 @@ def test_check_notification_content_is_not_empty_fails(
|
||||
notify_api, mocker, sample_service, template_content, notification_values
|
||||
):
|
||||
template_id = create_template(sample_service, content=template_content).id
|
||||
template = templates_dao.dao_get_template_by_id_and_service_id(
|
||||
template_dict = get_template_dict(
|
||||
template_id=template_id,
|
||||
service_id=sample_service.id
|
||||
)
|
||||
template_with_content = create_content_for_notification(template, notification_values)
|
||||
template_with_content = create_content_for_notification(template_dict, notification_values)
|
||||
with pytest.raises(BadRequestError) as e:
|
||||
check_notification_content_is_not_empty(template_with_content)
|
||||
assert e.value.status_code == 400
|
||||
@@ -349,6 +352,10 @@ def test_validate_template(sample_service):
|
||||
|
||||
def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_service):
|
||||
template = create_template(sample_service, template_type="email")
|
||||
template_dict = get_template_dict(
|
||||
template_id=template.id,
|
||||
service_id=sample_service.id
|
||||
)
|
||||
mock_check_type = mocker.patch('app.notifications.validators.check_template_is_for_notification_type')
|
||||
mock_check_if_active = mocker.patch('app.notifications.validators.check_template_is_active')
|
||||
mock_create_conent = mocker.patch(
|
||||
@@ -359,8 +366,8 @@ def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_servic
|
||||
validate_template(template.id, {}, sample_service, "email")
|
||||
|
||||
mock_check_type.assert_called_once_with("email", "email")
|
||||
mock_check_if_active.assert_called_once_with(template)
|
||||
mock_create_conent.assert_called_once_with(template, {})
|
||||
mock_check_if_active.assert_called_once_with(template_dict)
|
||||
mock_create_conent.assert_called_once_with(template_dict, {})
|
||||
mock_check_not_empty.assert_called_once_with("content")
|
||||
mock_check_message_is_too_long.assert_called_once_with("content")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user