diff --git a/app/notifications/process_letter_notifications.py b/app/notifications/process_letter_notifications.py index cf967eb23..2af7f5f7a 100644 --- a/app/notifications/process_letter_notifications.py +++ b/app/notifications/process_letter_notifications.py @@ -6,7 +6,14 @@ 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, updated_at=None + letter_data, + template, + service, + api_key, + status, + reply_to_text=None, + billable_units=None, + updated_at=None, ): notification = persist_notification( template_id=template.id, @@ -14,7 +21,7 @@ def create_letter_notification( 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 b185a88c3..3f1bc5515 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,34 @@ 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( + { + 'content': template.content, + 'subject': template.subject, + 'template_type': template.template_type, + }, + personalisation, + ) + if template.template_type == SMS_TYPE: + template_object = SMSMessageTemplate( + { + 'content': template.content, + 'template_type': template.template_type, + }, + personalisation, + ) + if template.template_type == LETTER_TYPE: + template_object = LetterPrintTemplate( + { + 'content': template.content, + 'subject': template.subject, + 'template_type': template.template_type, + }, + 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 692c960fd..81e70f6e2 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 32aaa3b9c..7eacd8ede 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -8,7 +8,7 @@ from notifications_utils.recipients import ( ) from notifications_utils.clients.redis import rate_limit_cache_key, daily_limit_cache_key -from app.dao import services_dao, templates_dao +from app.dao import services_dao from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id from app.models import ( INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, @@ -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.serialised_models import SerialisedTemplate from gds_metrics.metrics import Histogram @@ -156,11 +157,9 @@ def check_notification_content_is_not_empty(template_with_content): def validate_template(template_id, personalisation, service, notification_type): + try: - template = templates_dao.dao_get_template_by_id_and_service_id( - template_id=template_id, - service_id=service.id - ) + template = SerialisedTemplate.from_id_and_service_id(template_id, service.id) except NoResultFound: message = 'Template not found' raise BadRequestError(message=message, diff --git a/app/schemas.py b/app/schemas.py index c985d1812..85956e0cf 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -343,7 +343,9 @@ class BaseTemplateSchema(BaseSchema): class TemplateSchema(BaseTemplateSchema): - created_by = field_for(models.Template, 'created_by', required=True) + created_by_id = field_for( + models.Template, 'created_by_id', dump_to='created_by', dump_only=True + ) process_type = field_for(models.Template, 'process_type') redact_personalisation = fields.Method("redact") @@ -357,6 +359,9 @@ class TemplateSchema(BaseTemplateSchema): if not subject or subject.strip() == '': raise ValidationError('Invalid template subject', 'subject') + class Meta(BaseTemplateSchema.Meta): + exclude = BaseTemplateSchema.Meta.exclude + ('created_by',) + class TemplateHistorySchema(BaseSchema): diff --git a/app/serialised_models.py b/app/serialised_models.py new file mode 100644 index 000000000..f864171f7 --- /dev/null +++ b/app/serialised_models.py @@ -0,0 +1,57 @@ +from abc import ABC, abstractmethod + +from app.dao import templates_dao + + +class SerialisedModel(ABC): + + """ + A SerialisedModel takes a dictionary, typically created by + serialising a database object. It then takes the value of specified + keys from the dictionary and adds them to itself as properties, so + that it can be interacted with like a normal database model object, + but with no risk that it will actually go back to the database. + """ + + @property + @abstractmethod + def ALLOWED_PROPERTIES(self): + pass + + def __init__(self, _dict): + for property in self.ALLOWED_PROPERTIES: + setattr(self, property, _dict[property]) + + def __dir__(self): + return super().__dir__() + list(sorted(self.ALLOWED_PROPERTIES)) + + +class SerialisedTemplate(SerialisedModel): + ALLOWED_PROPERTIES = { + 'archived', + 'content', + 'id', + 'postage', + 'process_type', + 'reply_to_text', + 'subject', + 'template_type', + 'version', + } + + @classmethod + def from_id_and_service_id(cls, template_id, service_id): + return cls(cls.get_dict(template_id, service_id)) + + @staticmethod + def get_dict(template_id, service_id): + from app.schemas import template_schema + + fetched_template = templates_dao.dao_get_template_by_id_and_service_id( + template_id=template_id, + service_id=service_id + ) + + template_dict = template_schema.dump(fetched_template).data + + return template_dict diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 09aecb0a1..72019f48a 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,19 @@ 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 +238,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 +285,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 +337,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 +348,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) @@ -360,6 +370,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, @@ -414,7 +425,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']) @@ -422,6 +433,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, @@ -460,7 +472,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) @@ -470,10 +482,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..7744aa142 100644 --- a/tests/app/notifications/test_process_letter_notifications.py +++ b/tests/app/notifications/test_process_letter_notifications.py @@ -2,6 +2,7 @@ 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.serialised_models import SerialisedTemplate def test_create_letter_notification_creates_notification(sample_letter_template, sample_api_key): @@ -13,7 +14,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 = SerialisedTemplate.from_id_and_service_id( + 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 +49,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 = SerialisedTemplate.from_id_and_service_id( + 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 +73,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 = SerialisedTemplate.from_id_and_service_id( + 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 5fe5a513a..94521959a 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,41 @@ from app.notifications.process_notifications import ( send_notification_to_queue, simulated_recipient ) +from app.serialised_models import SerialisedTemplate 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 = SerialisedTemplate.from_id_and_service_id( + sample_email_template.id, sample_email_template.service_id + ) content = create_content_for_notification(template, 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) + template = SerialisedTemplate.from_id_and_service_id( + 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 '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 = SerialisedTemplate.from_id_and_service_id( + 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 = SerialisedTemplate.from_id_and_service_id( + 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..1c4db4190 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -23,6 +23,7 @@ from app.notifications.validators import ( validate_and_format_recipient, validate_template, ) +from app.serialised_models import SerialisedTemplate from app.utils import get_template_instance from app.v2.errors import ( @@ -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 = SerialisedTemplate.from_id_and_service_id( 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 = SerialisedTemplate.from_id_and_service_id( 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)