diff --git a/app/models.py b/app/models.py index 2bed6639d..ad3262be8 100644 --- a/app/models.py +++ b/app/models.py @@ -983,9 +983,14 @@ class TemplateBase(db.Model): if self.template_type == LETTER_TYPE: return LetterPrintTemplate( self.__dict__, - contact_block=self.service.get_default_letter_contact(), + contact_block=self.get_reply_to_text(), ) + def _as_utils_template_with_personalisation(self, values): + template = self._as_utils_template() + template.values = values + return template + def serialize(self): serialized = { "id": str(self.id), diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 651fad687..9c698e693 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -32,11 +32,10 @@ from app.dao.notifications_dao import ( ) from app.v2.errors import BadRequestError -from app.utils import get_template_instance def create_content_for_notification(template, personalisation): - template_object = get_template_instance(template.__dict__, personalisation) + template_object = template._as_utils_template_with_personalisation(personalisation) check_placeholders(template_object) return template_object diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 088f8479c..4d20a609f 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -4,6 +4,7 @@ from flask import ( request, current_app ) +from notifications_utils.template import WithSubjectTemplate from app import api_user, authenticated_service from app.config import QueueNames @@ -35,7 +36,7 @@ from app.schemas import ( notifications_filter_schema ) from app.service.utils import service_allowed_to_send_to -from app.utils import pagination_links, get_template_instance, get_public_notify_type_text +from app.utils import pagination_links, get_public_notify_type_text from notifications_utils import SMS_CHAR_COUNT_LIMIT from notifications_utils.recipients import get_international_phone_info @@ -149,13 +150,15 @@ def send_notification(notification_type): def get_notification_return_data(notification_id, notification, template): output = { - 'body': str(template), 'template_version': notification['template_version'], 'notification': {'id': notification_id} } - if template.template_type == 'email': - output.update({'subject': template.subject}) + if template.template_type == SMS_TYPE: + output['body'] = str(template) + else: + output['body'] = WithSubjectTemplate.__str__(template) + output['subject'] = template.subject return output @@ -187,7 +190,7 @@ def _service_allowed_to_send_to(notification, service): def create_template_object_for_notification(template, personalisation): - template_object = get_template_instance(template.__dict__, personalisation) + template_object = template._as_utils_template_with_personalisation(personalisation) if template_object.missing_data: message = 'Missing personalisation: {}'.format(", ".join(template_object.missing_data)) diff --git a/app/template/rest.py b/app/template/rest.py index 0db1e4786..aeae18922 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -10,7 +10,7 @@ from flask import ( request) from notifications_utils import SMS_CHAR_COUNT_LIMIT from notifications_utils.pdf import extract_page_from_pdf -from notifications_utils.template import SMSMessageTemplate +from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate from requests import post as requests_post from sqlalchemy.orm.exc import NoResultFound @@ -38,7 +38,7 @@ from app.notifications.validators import service_has_permission, check_reply_to from app.schema_validation import validate from app.schemas import (template_schema, template_history_schema) from app.template.template_schemas import post_create_template_schema -from app.utils import get_template_instance, get_public_notify_type_text +from app.utils import get_public_notify_type_text template_blueprint = Blueprint('template', __name__, url_prefix='/service//template') @@ -166,7 +166,9 @@ def get_template_by_id_and_service_id(service_id, template_id): def preview_template_by_id_and_service_id(service_id, template_id): fetched_template = dao_get_template_by_id_and_service_id(template_id=template_id, service_id=service_id) data = template_schema.dump(fetched_template).data - template_object = get_template_instance(data, values=request.args.to_dict()) + template_object = fetched_template._as_utils_template_with_personalisation( + request.args.to_dict() + ) if template_object.missing_data: raise InvalidRequest( @@ -175,7 +177,11 @@ def preview_template_by_id_and_service_id(service_id, template_id): ]}, status_code=400 ) - data['subject'], data['content'] = template_object.subject, str(template_object) + data['subject'] = template_object.subject + if template_object.template_type == SMS_TYPE: + data['content'] = str(template_object) + else: + data['content'] = WithSubjectTemplate.__str__(template_object) return jsonify(data) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index f2454e727..c0c586dd1 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -8,6 +8,7 @@ from flask import request, jsonify, current_app, abort from notifications_utils.recipients import ( format_postcode_for_printing, is_a_real_uk_postcode, try_validate_and_format_phone_number ) +from notifications_utils.template import WithSubjectTemplate from app import ( api_user, @@ -166,23 +167,25 @@ def post_notification(notification_type): if notification_type == SMS_TYPE: create_resp_partial = functools.partial( create_post_sms_response_from_notification, - from_number=reply_to + from_number=reply_to, + content=str(template_with_content), ) elif notification_type == EMAIL_TYPE: create_resp_partial = functools.partial( create_post_email_response_from_notification, subject=template_with_content.subject, - email_from='{}@{}'.format(authenticated_service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']) + email_from='{}@{}'.format(authenticated_service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']), + content=WithSubjectTemplate.__str__(template_with_content), ) elif notification_type == LETTER_TYPE: create_resp_partial = functools.partial( create_post_letter_response_from_notification, subject=template_with_content.subject, + content=WithSubjectTemplate.__str__(template_with_content), ) resp = create_resp_partial( notification=notification, - content=str(template_with_content), url_root=request.url_root, scheduled_for=scheduled_for ) diff --git a/app/v2/template/post_template.py b/app/v2/template/post_template.py index 23cc983a9..fd67460fe 100644 --- a/app/v2/template/post_template.py +++ b/app/v2/template/post_template.py @@ -3,7 +3,6 @@ from flask import jsonify, request from app import authenticated_service from app.dao import templates_dao from app.schema_validation import validate -from app.utils import get_template_instance from app.v2.errors import BadRequestError from app.v2.template import v2_template_blueprint from app.v2.template.template_schemas import ( @@ -29,8 +28,9 @@ def post_template_preview(template_id): template = templates_dao.dao_get_template_by_id_and_service_id( template_id, authenticated_service.id) - template_object = get_template_instance( - template.__dict__, values=data.get('personalisation')) + template_object = template._as_utils_template_with_personalisation( + data.get('personalisation') + ) check_placeholders(template_object) diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 629f040fa..9a1b42d71 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -29,7 +29,7 @@ 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) - assert str(content) == template.content + assert str(content) == template.content + '\n' def test_create_content_for_notification_with_placeholders_passes(sample_template_with_placeholders): diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 305506d9f..fc80ecef7 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -274,6 +274,41 @@ def test_post_letter_notification_returns_400_for_empty_personalisation( } +def test_post_notification_returns_400_for_missing_letter_contact_block_personalisation( + client, + sample_service, +): + letter_contact_block = create_letter_contact( + service=sample_service, contact_block='((contact block))', is_default=True + ) + template = create_template( + service=sample_service, + template_type='letter', + reply_to=letter_contact_block.id, + ) + data = { + 'template_id': str(template.id), + 'personalisation': { + 'address_line_1': 'Line 1', + 'address_line_2': 'Line 2', + 'postcode': 'SW1A 1AA', + }, + } + + error_json = letter_request( + client, + data, + service_id=sample_service.id, + _expected_status=400, + ) + + assert error_json['status_code'] == 400 + assert error_json['errors'] == [{ + 'error': 'BadRequestError', + 'message': 'Missing personalisation: contact block' + }] + + def test_notification_returns_400_for_missing_template_field( client, sample_service_full_permissions @@ -450,15 +485,14 @@ def test_post_letter_notification_is_delivered_and_has_pdf_uploaded_to_test_lett s3mock.assert_called_once_with(ANY, b'letter-content', precompiled=True) -def test_post_letter_notification_persists_notification_reply_to_text( +def test_post_letter_notification_ignores_reply_to_text_for_service( client, notify_db_session, mocker ): mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') service = create_service(service_permissions=[LETTER_TYPE]) - service_address = "12 Main Street, London" - letter_contact = create_letter_contact(service=service, contact_block=service_address, is_default=True) - template = create_template(service=service, template_type='letter', reply_to=letter_contact.id) + create_letter_contact(service=service, contact_block='ignored', is_default=True) + template = create_template(service=service, template_type='letter') data = { "template_id": template.id, "personalisation": {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'BA5 5AB'} @@ -467,7 +501,27 @@ def test_post_letter_notification_persists_notification_reply_to_text( notifications = Notification.query.all() assert len(notifications) == 1 - assert notifications[0].reply_to_text == service_address + assert notifications[0].reply_to_text is None + + +def test_post_letter_notification_persists_notification_reply_to_text_for_template( + client, notify_db_session, mocker +): + mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') + + service = create_service(service_permissions=[LETTER_TYPE]) + create_letter_contact(service=service, contact_block='the default', is_default=True) + template_letter_contact = create_letter_contact(service=service, contact_block='not the default', is_default=False) + template = create_template(service=service, template_type='letter', reply_to=template_letter_contact.id) + data = { + "template_id": template.id, + "personalisation": {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'BA5 5AB'} + } + letter_request(client, data=data, service_id=service.id, key_type=KEY_TYPE_NORMAL) + + notifications = Notification.query.all() + assert len(notifications) == 1 + assert notifications[0].reply_to_text == 'not the default' def test_post_precompiled_letter_with_invalid_base64(client, notify_user, mocker): diff --git a/tests/app/v2/template/test_get_template.py b/tests/app/v2/template/test_get_template.py index dadf82983..a6628cb49 100644 --- a/tests/app/v2/template/test_get_template.py +++ b/tests/app/v2/template/test_get_template.py @@ -5,7 +5,7 @@ from flask import json from app import DATETIME_FORMAT from app.models import TEMPLATE_TYPES, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE from tests import create_authorization_header -from tests.app.db import create_template +from tests.app.db import create_template, create_letter_contact valid_version_params = [None, 1] @@ -79,35 +79,17 @@ def test_get_template_by_id_returns_200( }, }, ), - ( - { - "template_type": LETTER_TYPE, - "subject": "((letterSubject))", - "content": "((letter_content))", - }, - { - "letterSubject": { - "required": True, - }, - "letter_content": { - "required": True, - }, - "contact block": { - "required": True, - }, - }, - ) ]) @pytest.mark.parametrize("version", valid_version_params) def test_get_template_by_id_returns_placeholders( client, - sample_service_custom_letter_contact_block, + sample_service, version, create_template_args, expected_personalisation, ): - template = create_template(sample_service_custom_letter_contact_block, **create_template_args) - auth_header = create_authorization_header(service_id=sample_service_custom_letter_contact_block.id) + template = create_template(sample_service, **create_template_args) + auth_header = create_authorization_header(service_id=sample_service.id) version_path = '/version/{}'.format(version) if version else '' @@ -118,6 +100,44 @@ def test_get_template_by_id_returns_placeholders( assert json_response['personalisation'] == expected_personalisation +@pytest.mark.parametrize("version", valid_version_params) +def test_get_letter_template_by_id_returns_placeholders( + client, + sample_service, + version, +): + contact_block = create_letter_contact( + service=sample_service, + contact_block='((contact block))', + ) + template = create_template( + sample_service, + template_type=LETTER_TYPE, + subject="((letterSubject))", + content="((letter_content))", + reply_to=contact_block.id, + ) + auth_header = create_authorization_header(service_id=sample_service.id) + + version_path = '/version/{}'.format(version) if version else '' + + response = client.get(path='/v2/template/{}{}'.format(template.id, version_path), + headers=[('Content-Type', 'application/json'), auth_header]) + + json_response = json.loads(response.get_data(as_text=True)) + assert json_response['personalisation'] == { + "letterSubject": { + "required": True, + }, + "letter_content": { + "required": True, + }, + "contact block": { + "required": True, + }, + } + + def test_get_template_with_non_existent_template_id_returns_404(client, fake_uuid, sample_service): auth_header = create_authorization_header(service_id=sample_service.id)