From 5ddb5a75da20753893f1f6dc10d394518d48d95e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 13 Apr 2020 13:48:23 +0100 Subject: [PATCH] Use new properties of utils Templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We’ve added some new properties to the templates in utils that we can use instead of doing weird things like `WithSubjectTemplate.__str__(another_instance)` --- app/models.py | 14 +++++++------- app/notifications/rest.py | 9 +++------ app/schemas.py | 2 +- app/template/rest.py | 7 ++----- app/utils.py | 17 +++-------------- app/v2/notifications/post_notifications.py | 7 ++----- app/v2/template/template_schemas.py | 18 ++++-------------- requirements-app.txt | 2 +- requirements.txt | 3 +-- tests/app/test_model.py | 6 ++++-- 10 files changed, 28 insertions(+), 57 deletions(-) diff --git a/app/models.py b/app/models.py index ad3262be8..4c24fde92 100644 --- a/app/models.py +++ b/app/models.py @@ -1513,16 +1513,16 @@ class Notification(db.Model): @property def content(self): - from app.utils import get_template_instance - template_object = get_template_instance(self.template.__dict__, self.personalisation) - return str(template_object) + return self.template._as_utils_template_with_personalisation( + self.personalisation + ).content_with_placeholders_filled_in @property def subject(self): - from app.utils import get_template_instance - if self.notification_type != SMS_TYPE: - template_object = get_template_instance(self.template.__dict__, self.personalisation) - return template_object.subject + template_object = self.template._as_utils_template_with_personalisation( + self.personalisation + ) + return getattr(template_object, 'subject', None) @property def formatted_status(self): diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 4d20a609f..ed1c4a871 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -4,7 +4,6 @@ from flask import ( request, current_app ) -from notifications_utils.template import WithSubjectTemplate from app import api_user, authenticated_service from app.config import QueueNames @@ -151,13 +150,11 @@ def send_notification(notification_type): def get_notification_return_data(notification_id, notification, template): output = { 'template_version': notification['template_version'], - 'notification': {'id': notification_id} + 'notification': {'id': notification_id}, + 'body': template.content_with_placeholders_filled_in, } - if template.template_type == SMS_TYPE: - output['body'] = str(template) - else: - output['body'] = WithSubjectTemplate.__str__(template) + if hasattr(template, 'subject'): output['subject'] = template.subject return output diff --git a/app/schemas.py b/app/schemas.py index e53782b04..efa99e7d4 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -521,7 +521,7 @@ class NotificationWithPersonalisationSchema(NotificationWithTemplateSchema): def handle_template_merge(self, in_data): in_data['template'] = in_data.pop('template_history') template = get_template_instance(in_data['template'], in_data['personalisation']) - in_data['body'] = str(template) + in_data['body'] = template.content_with_placeholders_filled_in if in_data['template']['template_type'] != models.SMS_TYPE: in_data['subject'] = template.subject in_data['content_char_count'] = None diff --git a/app/template/rest.py b/app/template/rest.py index aeae18922..1ae8a3ac9 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, WithSubjectTemplate +from notifications_utils.template import SMSMessageTemplate from requests import post as requests_post from sqlalchemy.orm.exc import NoResultFound @@ -178,10 +178,7 @@ def preview_template_by_id_and_service_id(service_id, template_id): ) data['subject'] = template_object.subject - if template_object.template_type == SMS_TYPE: - data['content'] = str(template_object) - else: - data['content'] = WithSubjectTemplate.__str__(template_object) + data['content'] = template_object.content_with_placeholders_filled_in return jsonify(data) diff --git a/app/utils.py b/app/utils.py index d36291b6b..9c9fe915a 100644 --- a/app/utils.py +++ b/app/utils.py @@ -4,7 +4,8 @@ import pytz from flask import url_for from sqlalchemy import func from notifications_utils.timezones import convert_utc_to_bst -from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate, get_html_email_body +from notifications_utils.template import SMSMessageTemplate, HTMLEmailTemplate, LetterPrintTemplate + local_timezone = pytz.timezone("Europe/London") @@ -31,22 +32,10 @@ def url_with_token(data, url, config, base_url=None): def get_template_instance(template, values): from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE return { - SMS_TYPE: SMSMessageTemplate, EMAIL_TYPE: WithSubjectTemplate, LETTER_TYPE: WithSubjectTemplate + SMS_TYPE: SMSMessageTemplate, EMAIL_TYPE: HTMLEmailTemplate, LETTER_TYPE: LetterPrintTemplate }[template['template_type']](template, values) -def get_html_email_body_from_template(template_instance): - from app.models import EMAIL_TYPE - - if template_instance.template_type != EMAIL_TYPE: - return None - - return get_html_email_body( - template_instance.content, - template_instance.values, - ) - - def get_london_midnight_in_utc(date): """ This function converts date to midnight as BST (British Standard Time) to UTC, diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 9299b08ab..399940d44 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -7,7 +7,6 @@ from boto.exception import SQSError from flask import request, jsonify, current_app, abort from notifications_utils.postal_address import PostalAddress from notifications_utils.recipients import try_validate_and_format_phone_number -from notifications_utils.template import WithSubjectTemplate from app import ( api_user, @@ -167,26 +166,24 @@ def post_notification(notification_type): create_resp_partial = functools.partial( create_post_sms_response_from_notification, 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']), - 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, url_root=request.url_root, - scheduled_for=scheduled_for + scheduled_for=scheduled_for, + content=template_with_content.content_with_placeholders_filled_in, ) return jsonify(resp), 201 diff --git a/app/v2/template/template_schemas.py b/app/v2/template/template_schemas.py index 512b47ebd..378eb8121 100644 --- a/app/v2/template/template_schemas.py +++ b/app/v2/template/template_schemas.py @@ -1,8 +1,5 @@ -from notifications_utils.template import WithSubjectTemplate - -from app.models import SMS_TYPE, TEMPLATE_TYPES +from app.models import TEMPLATE_TYPES from app.schema_validation.definitions import uuid, personalisation -from app.utils import get_html_email_body_from_template get_template_by_id_request = { @@ -77,19 +74,12 @@ post_template_preview_response = { def create_post_template_preview_response(template, template_object): - if template.template_type == SMS_TYPE: - subject = None - body = str(template_object) - else: - subject = template_object.subject - body = WithSubjectTemplate.__str__(template_object) - return { "id": template.id, "type": template.template_type, "version": template.version, - "body": body, - "html": get_html_email_body_from_template(template_object), - "subject": subject, + "body": template_object.content_with_placeholders_filled_in, + "html": getattr(template_object, 'html_body', None), + "subject": getattr(template_object, 'subject', None), "postage": template.postage } diff --git a/requirements-app.txt b/requirements-app.txt index aa18a66f9..a6b276fdf 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -26,4 +26,4 @@ notifications-python-client==5.5.1 # PaaS awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@36.12.2#egg=notifications-utils==36.12.2 +git+https://github.com/alphagov/notifications-utils.git@37.0.0#egg=notifications-utils==37.0.0 diff --git a/requirements.txt b/requirements.txt index a5a59a6cb..8ba38cf6f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,8 +28,7 @@ notifications-python-client==5.5.1 # PaaS awscli-cwlogs>=1.4,<1.5 - -git+https://github.com/alphagov/notifications-utils.git@36.12.2#egg=notifications-utils==36.12.2 +git+https://github.com/alphagov/notifications-utils.git@37.0.0#egg=notifications-utils==37.0.0 ## The following requirements were added by pip freeze: alembic==1.4.2 diff --git a/tests/app/test_model.py b/tests/app/test_model.py index bb2961a6b..70d7f78ce 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -172,8 +172,10 @@ def test_notification_personalisation_setter_always_sets_empty_dict(input_value) assert noti._personalisation == encryption.encrypt({}) -def test_notification_subject_is_none_for_sms(): - assert Notification(notification_type=SMS_TYPE).subject is None +def test_notification_subject_is_none_for_sms(sample_service): + template = create_template(service=sample_service, template_type=SMS_TYPE) + notification = create_notification(template=template) + assert notification.subject is None @pytest.mark.parametrize('template_type', ['email', 'letter'])