diff --git a/app/notifications/process_letter_notifications.py b/app/notifications/process_letter_notifications.py index 5ba9c5a19..744bc8c68 100644 --- a/app/notifications/process_letter_notifications.py +++ b/app/notifications/process_letter_notifications.py @@ -5,14 +5,22 @@ from app.models import LETTER_TYPE from app.notifications.process_notifications import persist_notification -def create_letter_notification(letter_data, template, api_key, status, reply_to_text=None, billable_units=None): +def create_letter_notification( + letter_data, + template, + service, + api_key, + status, + reply_to_text=None, + billable_units=None, +): notification = persist_notification( template_id=template.id, - template_version=template.version, - template_postage=template.postage, + template_version=template._template['version'], + template_postage=template._template['postage'], # we only accept addresses_with_underscores from the API (from CSV we also accept dashes, spaces etc) recipient=PostalAddress.from_personalisation(letter_data['personalisation']).normalised, - service=template.service, + service=service, personalisation=letter_data['personalisation'], notification_type=LETTER_TYPE, api_key_id=api_key.id, diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 54d222891..f90e433b9 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -9,6 +9,11 @@ from notifications_utils.recipients import ( validate_and_format_phone_number, format_email_address ) +from notifications_utils.template import ( + PlainTextEmailTemplate, + SMSMessageTemplate, + LetterPrintTemplate, +) from notifications_utils.timezones import convert_bst_to_utc from app import redis_store @@ -34,8 +39,18 @@ from app.dao.notifications_dao import ( from app.v2.errors import BadRequestError -def create_content_for_notification(template, personalisation): - template_object = template._as_utils_template_with_personalisation(personalisation) +def create_content_for_notification(template_dict, personalisation): + if template_dict['template_type'] == EMAIL_TYPE: + template_object = PlainTextEmailTemplate(template_dict, personalisation) + if template_dict['template_type'] == SMS_TYPE: + template_object = SMSMessageTemplate(template_dict, personalisation) + if template_dict['template_type'] == LETTER_TYPE: + template_object = LetterPrintTemplate( + template_dict, + personalisation, + contact_block=template_dict['reply_to_text'], + ) + check_placeholders(template_object) return template_object diff --git a/app/notifications/rest.py b/app/notifications/rest.py index ed1c4a871..543f53189 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -100,12 +100,13 @@ def send_notification(notification_type): check_rate_limiting(authenticated_service, api_user) - template, template_with_content = validate_template( + template_with_content = validate_template( template_id=notification_form['template'], personalisation=notification_form.get('personalisation', {}), service=authenticated_service, notification_type=notification_type ) + template_dict = template_with_content._template _service_allowed_to_send_to(notification_form, authenticated_service) if not service_has_permission(notification_type, authenticated_service.permissions): @@ -118,9 +119,9 @@ def send_notification(notification_type): _service_can_send_internationally(authenticated_service, notification_form['to']) # Do not persist or send notification to the queue if it is a simulated recipient simulated = simulated_recipient(notification_form['to'], notification_type) - notification_model = persist_notification(template_id=template.id, - template_version=template.version, - template_postage=template.postage, + notification_model = persist_notification(template_id=template_dict['id'], + template_version=template_dict['version'], + template_postage=template_dict['postage'], recipient=request.get_json()['to'], service=authenticated_service, personalisation=notification_form.get('personalisation', None), @@ -128,16 +129,16 @@ def send_notification(notification_type): api_key_id=api_user.id, key_type=api_user.key_type, simulated=simulated, - reply_to_text=template.get_reply_to_text() + reply_to_text=template_dict['reply_to_text'] ) if not simulated: - queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None + queue_name = QueueNames.PRIORITY if template_dict['process_type'] == PRIORITY else None send_notification_to_queue(notification=notification_model, research_mode=authenticated_service.research_mode, queue=queue_name) else: current_app.logger.debug("POST simulated notification for id: {}".format(notification_model.id)) - notification_form.update({"template_version": template.version}) + notification_form.update({"template_version": template_dict['version']}) return jsonify( data=get_notification_return_data( diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 3b9cf6c62..ea12b9296 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -62,7 +62,7 @@ def check_template_is_for_notification_type(notification_type, template_type): def check_template_is_active(template): - if template.archived: + if template['archived']: raise BadRequestError(fields=[{'template': 'Template has been deleted'}], message="Template has been deleted") @@ -138,18 +138,25 @@ def check_notification_content_is_not_empty(template_with_content): raise BadRequestError(message=message) -def validate_template(template_id, personalisation, service, notification_type): +def get_template_dict(template_id, service_id): + from app.schemas import template_schema try: - template = templates_dao.dao_get_template_by_id_and_service_id( + fetched_template = templates_dao.dao_get_template_by_id_and_service_id( template_id=template_id, - service_id=service.id + service_id=service_id ) except NoResultFound: message = 'Template not found' raise BadRequestError(message=message, fields=[{'template': message}]) - check_template_is_for_notification_type(notification_type, template.template_type) + return template_schema.dump(fetched_template).data + + +def validate_template(template_id, personalisation, service, notification_type): + template = get_template_dict(template_id, service.id) + + check_template_is_for_notification_type(notification_type, template['template_type']) check_template_is_active(template) template_with_content = create_content_for_notification(template, personalisation) @@ -158,7 +165,7 @@ def validate_template(template_id, personalisation, service, notification_type): check_content_char_count(template_with_content) - return template, template_with_content + return template_with_content def check_reply_to(service_id, reply_to_id, type_): diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index ed7db67e5..c08477a81 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -42,6 +42,7 @@ from app.notifications.process_letter_notifications import ( create_letter_notification ) from app.notifications.process_notifications import ( + create_content_for_notification, persist_notification, persist_scheduled_notification, send_notification_to_queue, @@ -76,6 +77,7 @@ from app.v2.utils import get_valid_json @v2_notification_blueprint.route('/{}'.format(LETTER_TYPE), methods=['POST']) def post_precompiled_letter_notification(): + from app.schemas import template_schema request_json = get_valid_json() if 'content' not in (request_json or {}): return post_notification(LETTER_TYPE) @@ -88,6 +90,9 @@ def post_precompiled_letter_notification(): check_rate_limiting(authenticated_service, api_user) template = get_precompiled_letter_template(authenticated_service.id) + template = create_content_for_notification( + template_schema.dump(template).data, {} + ) # For precompiled letters the to field will be set to Provided as PDF until the validation passes, # then the address of the letter will be set as the to field @@ -95,13 +100,12 @@ def post_precompiled_letter_notification(): 'address_line_1': 'Provided as PDF' } - reply_to = get_reply_to_text(LETTER_TYPE, form, template) - notification = process_letter_notification( letter_data=form, api_key=api_user, template=template, - reply_to_text=reply_to, + service=authenticated_service, + reply_to_text=template._template['reply_to_text'], precompiled=True ) @@ -135,20 +139,21 @@ def post_notification(notification_type): check_rate_limiting(authenticated_service, api_user) - template, template_with_content = validate_template( + template_with_content = validate_template( form['template_id'], form.get('personalisation', {}), authenticated_service, notification_type, ) - reply_to = get_reply_to_text(notification_type, form, template) + reply_to = get_reply_to_text(notification_type, form, template_with_content) if notification_type == LETTER_TYPE: notification = process_letter_notification( letter_data=form, api_key=api_user, - template=template, + template=template_with_content, + service=authenticated_service, reply_to_text=reply_to ) else: @@ -156,11 +161,12 @@ def post_notification(notification_type): form=form, notification_type=notification_type, api_key=api_user, - template=template, + template=template_with_content, service=authenticated_service, reply_to_text=reply_to ) + # Think this is redundant template_with_content.values = notification.personalisation if notification_type == SMS_TYPE: @@ -237,7 +243,7 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ notification = persist_notification( notification_id=notification_id, template_id=template.id, - template_version=template.version, + template_version=template._template['version'], recipient=form_send_to, service=service, personalisation=personalisation, @@ -255,7 +261,7 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ persist_scheduled_notification(notification.id, form["scheduled_for"]) else: if not simulated: - queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None + queue_name = QueueNames.PRIORITY if template._template['process_type'] == PRIORITY else None send_notification_to_queue( notification=notification, research_mode=service.research_mode, @@ -282,7 +288,7 @@ def save_email_to_queue( data = { "id": notification_id, "template_id": str(template.id), - "template_version": template.version, + "template_version": template._template['version'], "to": form['email_address'], "service_id": str(service_id), "personalisation": personalisation, @@ -333,7 +339,7 @@ def process_document_uploads(personalisation_data, service, simulated=False): return personalisation_data, len(file_keys) -def process_letter_notification(*, letter_data, api_key, template, reply_to_text, precompiled=False): +def process_letter_notification(*, letter_data, api_key, template, service, reply_to_text, precompiled=False): if api_key.key_type == KEY_TYPE_TEAM: raise BadRequestError(message='Cannot send letters with a team api key', status_code=403) @@ -344,6 +350,7 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text return process_precompiled_letter_notifications(letter_data=letter_data, api_key=api_key, template=template, + service=service, reply_to_text=reply_to_text) address = PostalAddress.from_personalisation( @@ -378,6 +385,7 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text notification = create_letter_notification(letter_data=letter_data, template=template, + service=service, api_key=api_key, status=status, reply_to_text=reply_to_text) @@ -399,7 +407,7 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text return notification -def process_precompiled_letter_notifications(*, letter_data, api_key, template, reply_to_text): +def process_precompiled_letter_notifications(*, letter_data, api_key, template, service, reply_to_text): try: status = NOTIFICATION_PENDING_VIRUS_CHECK letter_content = base64.b64decode(letter_data['content']) @@ -408,6 +416,7 @@ def process_precompiled_letter_notifications(*, letter_data, api_key, template, notification = create_letter_notification(letter_data=letter_data, template=template, + service=service, api_key=api_key, status=status, reply_to_text=reply_to_text) @@ -439,7 +448,7 @@ def get_reply_to_text(notification_type, form, template): service_email_reply_to_id = form.get("email_reply_to_id", None) reply_to = check_service_email_reply_to_id( str(authenticated_service.id), service_email_reply_to_id, notification_type - ) or template.get_reply_to_text() + ) or template._template['reply_to_text'] elif notification_type == SMS_TYPE: service_sms_sender_id = form.get("sms_sender_id", None) @@ -449,9 +458,9 @@ def get_reply_to_text(notification_type, form, template): if sms_sender_id: reply_to = try_validate_and_format_phone_number(sms_sender_id) else: - reply_to = template.get_reply_to_text() + reply_to = template._template['reply_to_text'] elif notification_type == LETTER_TYPE: - reply_to = template.get_reply_to_text() + reply_to = template._template['reply_to_text'] return reply_to diff --git a/tests/app/notifications/test_process_letter_notifications.py b/tests/app/notifications/test_process_letter_notifications.py index 08117a2b2..0e3e82650 100644 --- a/tests/app/notifications/test_process_letter_notifications.py +++ b/tests/app/notifications/test_process_letter_notifications.py @@ -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 diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index f83470a10..03dd048ca 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -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") diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 53c024f5e..c8cc2558d 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -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")