diff --git a/app/json_models.py b/app/json_models.py new file mode 100644 index 000000000..1f260aaa8 --- /dev/null +++ b/app/json_models.py @@ -0,0 +1,29 @@ +from abc import ABC, abstractmethod + + +class JSONModel(ABC): + + @property + @abstractmethod + def ALLOWED_PROPERTIES(self): + pass + + def __init__(self, _dict): + self._dict = _dict + for property in self.ALLOWED_PROPERTIES: + setattr(self, property, _dict[property]) + + def __dir__(self): + return super().__dir__() + list(sorted(self.ALLOWED_PROPERTIES)) + + +class TemplateJSONModel(JSONModel): + ALLOWED_PROPERTIES = { + 'archived', + 'id', + 'postage', + 'process_type', + 'reply_to_text', + 'template_type', + 'version', + } diff --git a/app/notifications/process_letter_notifications.py b/app/notifications/process_letter_notifications.py index 5ba9c5a19..f7c408388 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, # 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 2700775ec..4b5b83360 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 @@ -44,7 +49,17 @@ REDIS_GET_AND_INCR_DAILY_LIMIT_DURATION_SECONDS = Histogram( def create_content_for_notification(template, personalisation): - template_object = template._as_utils_template_with_personalisation(personalisation) + if template.template_type == EMAIL_TYPE: + template_object = PlainTextEmailTemplate(template._dict, personalisation) + if template.template_type == SMS_TYPE: + template_object = SMSMessageTemplate(template._dict, personalisation) + if template.template_type == LETTER_TYPE: + template_object = LetterPrintTemplate( + template._dict, + personalisation, + contact_block=template.reply_to_text, + ) + check_placeholders(template_object) return template_object diff --git a/app/notifications/rest.py b/app/notifications/rest.py index ed1c4a871..1afe2fa72 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -128,7 +128,7 @@ 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.reply_to_text ) if not simulated: queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None diff --git a/app/notifications/validators.py b/app/notifications/validators.py index fb5880064..af043e330 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -21,6 +21,7 @@ from app.notifications.process_notifications import create_content_for_notificat from app.utils import get_public_notify_type_text from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id from app.dao.service_letter_contact_dao import dao_get_letter_contact_by_id +from app.json_models import TemplateJSONModel from gds_metrics.metrics import Histogram @@ -147,17 +148,28 @@ 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}]) + return template_schema.dump(fetched_template).data + + +def get_template_model(template_id, service_id): + return TemplateJSONModel(get_template_dict(template_id, service_id)) + + +def validate_template(template_id, personalisation, service, notification_type): + template = get_template_model(template_id, service.id) + check_template_is_for_notification_type(notification_type, template.template_type) check_template_is_active(template) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index ecaac90e0..c8559164d 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -98,14 +98,13 @@ 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, + service=authenticated_service, template=template, template_with_content=None, # not required for precompiled - reply_to_text=reply_to, + reply_to_text='', # not required for precompiled precompiled=True ) @@ -147,6 +146,7 @@ def post_notification(notification_type): notification = process_letter_notification( letter_data=form, api_key=api_user, + service=authenticated_service, template=template, template_with_content=template_with_content, reply_to_text=reply_to @@ -156,7 +156,8 @@ def post_notification(notification_type): form=form, notification_type=notification_type, api_key=api_user, - template=template_with_content, + template=template, + template_with_content=template_with_content, template_process_type=template.process_type, service=authenticated_service, reply_to_text=reply_to @@ -166,7 +167,15 @@ def post_notification(notification_type): def process_sms_or_email_notification( - *, form, notification_type, api_key, template, template_process_type, service, reply_to_text=None + *, + form, + notification_type, + api_key, + template, + template_with_content, + template_process_type, + service, + reply_to_text=None, ): notification_id = uuid.uuid4() form_send_to = form['email_address'] if notification_type == EMAIL_TYPE else form['phone_number'] @@ -186,19 +195,20 @@ def process_sms_or_email_notification( ) if document_download_count: # We changed personalisation which means we need to update the content - template.values = personalisation + template_with_content.values = personalisation key_type = api_key.key_type service_in_research_mode = service.research_mode resp = create_response_for_post_notification( notification_id=notification_id, client_reference=form.get('reference', None), template_id=template.id, - template_version=template._template['version'], + template_version=template.version, service_id=service.id, notification_type=notification_type, reply_to=reply_to_text, scheduled_for=form.get("scheduled_for", None), - template_with_content=template) + template_with_content=template_with_content, + ) if str(service.id) in current_app.config.get('HIGH_VOLUME_SERVICE') and api_key.key_type == KEY_TYPE_NORMAL \ and notification_type == EMAIL_TYPE: @@ -229,7 +239,7 @@ def process_sms_or_email_notification( persist_notification( notification_id=notification_id, template_id=template.id, - template_version=template._template['version'], + template_version=template.version, recipient=form_send_to, service=service, personalisation=personalisation, @@ -276,7 +286,7 @@ def save_email_to_queue( data = { "id": notification_id, "template_id": str(template.id), - "template_version": template._template['version'], + "template_version": template.version, "to": form['email_address'], "service_id": str(service_id), "personalisation": personalisation, @@ -328,7 +338,7 @@ def process_document_uploads(personalisation_data, service, simulated=False): def process_letter_notification( - *, letter_data, api_key, template, template_with_content, reply_to_text, precompiled=False + *, letter_data, api_key, service, template, template_with_content, 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) @@ -339,6 +349,7 @@ def process_letter_notification( if precompiled: return process_precompiled_letter_notifications(letter_data=letter_data, api_key=api_key, + service=service, template=template, reply_to_text=reply_to_text) @@ -358,6 +369,7 @@ def process_letter_notification( queue = QueueNames.CREATE_LETTERS_PDF if not test_key else QueueNames.RESEARCH_MODE notification = create_letter_notification(letter_data=letter_data, + service=service, template=template, api_key=api_key, status=status, @@ -410,7 +422,7 @@ def validate_address(api_key, letter_data): ) -def process_precompiled_letter_notifications(*, letter_data, api_key, template, reply_to_text): +def process_precompiled_letter_notifications(*, letter_data, api_key, service, template, reply_to_text): try: status = NOTIFICATION_PENDING_VIRUS_CHECK letter_content = base64.b64decode(letter_data['content']) @@ -418,6 +430,7 @@ def process_precompiled_letter_notifications(*, letter_data, api_key, template, raise BadRequestError(message='Cannot decode letter content (invalid base64 encoding)', status_code=400) notification = create_letter_notification(letter_data=letter_data, + service=service, template=template, api_key=api_key, status=status, @@ -456,7 +469,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.reply_to_text elif notification_type == SMS_TYPE: service_sms_sender_id = form.get("sms_sender_id", None) @@ -466,10 +479,10 @@ 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.reply_to_text elif notification_type == LETTER_TYPE: - reply_to = template.get_reply_to_text() + reply_to = 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..ca6f6c3ac 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.validators import get_template_dict +from app.json_models import TemplateJSONModel 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 = TemplateJSONModel(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 = TemplateJSONModel(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 = TemplateJSONModel(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 52f5fbfea..58355fd97 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -11,7 +11,6 @@ from app.models import ( Notification, NotificationHistory, ScheduledNotification, - Template, LETTER_TYPE ) from app.notifications.process_notifications import ( @@ -21,32 +20,39 @@ from app.notifications.process_notifications import ( send_notification_to_queue, simulated_recipient ) +from app.notifications.validators import get_template_model 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, create_api_key def test_create_content_for_notification_passes(sample_email_template): - template = Template.query.get(sample_email_template.id) + template = get_template_model(sample_email_template.id, sample_email_template.service_id) content = create_content_for_notification(template, None) - assert str(content) == template.content + '\n' + assert str(content) == template._dict['content'] + '\n' def test_create_content_for_notification_with_placeholders_passes(sample_template_with_placeholders): - template = Template.query.get(sample_template_with_placeholders.id) + template = get_template_model( + sample_template_with_placeholders.id, sample_template_with_placeholders.service_id + ) content = create_content_for_notification(template, {'name': 'Bobby'}) - assert content.content == template.content + assert content.content == template._dict['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 = get_template_model( + sample_template_with_placeholders.id, sample_template_with_placeholders.service_id + ) with pytest.raises(BadRequestError): create_content_for_notification(template, None) def test_create_content_for_notification_allows_additional_personalisation(sample_template_with_placeholders): - template = Template.query.get(sample_template_with_placeholders.id) + template = get_template_model( + sample_template_with_placeholders.id, sample_template_with_placeholders.service_id + ) create_content_for_notification(template, {'name': 'Bobby', 'Additional placeholder': 'Data'}) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 53c024f5e..49ad9615b 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_model, service_can_send_to_recipient, validate_and_format_recipient, validate_template, @@ -314,7 +315,7 @@ 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 = get_template_model( template_id=template_id, service_id=sample_service.id ) @@ -330,7 +331,7 @@ 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 = get_template_model( template_id=template_id, service_id=sample_service.id ) @@ -356,7 +357,7 @@ def test_validate_template_calls_all_validators(mocker, fake_uuid, sample_servic ) mock_check_not_empty = mocker.patch('app.notifications.validators.check_notification_content_is_not_empty') mock_check_message_is_too_long = mocker.patch('app.notifications.validators.check_content_char_count') - validate_template(template.id, {}, sample_service, "email") + template, template_with_content = 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)