Merge pull request #764 from alphagov/more-breaking-utils-changes

Update utils to 12.1.1

app/delivery/send_to_providers.py

@@ -1,12 +1,10 @@
 from datetime import datetime
 
 from flask import current_app
-from notifications_utils.field import Field
 from notifications_utils.recipients import (
     validate_and_format_phone_number
 )
-from notifications_utils.template import Template
+from notifications_utils.template import HTMLEmailTemplate, PlainTextEmailTemplate, SMSMessageTemplate
-from notifications_utils.renderers import HTMLEmail, PlainTextEmail, SMSMessage
 
 from app import clients, statsd_client, create_uuid
 from app.dao.notifications_dao import dao_update_notification
@@ -23,10 +21,11 @@ def send_sms_to_provider(notification):
     provider = provider_to_use(SMS_TYPE, notification.id)
     if notification.status == 'created':
         template_model = dao_get_template_by_id(notification.template_id, notification.template_version)
-        template = Template(
+        template = SMSMessageTemplate(
             template_model.__dict__,
-            values={} if not notification.personalisation else notification.personalisation,
+            values=notification.personalisation,
-            renderer=SMSMessage(prefix=service.name, sender=service.sms_sender)
+            prefix=service.name,
+            sender=service.sms_sender
         )
         if service.research_mode or notification.key_type == KEY_TYPE_TEST:
             send_sms_response.apply_async(
@@ -36,11 +35,11 @@ def send_sms_to_provider(notification):
         else:
             provider.send_sms(
                 to=validate_and_format_phone_number(notification.to),
-                content=template.rendered,
+                content=str(template),
                 reference=str(notification.id),
                 sender=service.sms_sender
             )
-            notification.billable_units = template.sms_fragment_count
+            notification.billable_units = template.fragment_count
 
         notification.sent_at = datetime.utcnow()
         notification.sent_by = provider.get_name()
@@ -60,16 +59,15 @@ def send_email_to_provider(notification):
     if notification.status == 'created':
         template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__
 
-        html_email = Template(
+        html_email = HTMLEmailTemplate(
             template_dict,
             values=notification.personalisation,
-            renderer=get_html_email_renderer(service)
+            **get_html_email_options(service)
         )
 
-        plain_text_email = Template(
+        plain_text_email = PlainTextEmailTemplate(
             template_dict,
-            values=notification.personalisation,
+            values=notification.personalisation
-            renderer=PlainTextEmail()
         )
 
         if service.research_mode or notification.key_type == KEY_TYPE_TEST:
@@ -84,9 +82,9 @@ def send_email_to_provider(notification):
             reference = provider.send_email(
                 from_address,
                 notification.to,
-                str(Field(plain_text_email.subject, notification.personalisation)),
+                plain_text_email.subject,
-                body=plain_text_email.rendered,
+                body=str(plain_text_email),
-                html_body=html_email.rendered,
+                html_body=str(html_email),
                 reply_to_address=service.reply_to_email_address,
             )
 
@@ -117,7 +115,7 @@ def provider_to_use(notification_type, notification_id):
     return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type)
 
 
-def get_html_email_renderer(service):
+def get_html_email_options(service):
     govuk_banner = service.branding != BRANDING_ORG
     if service.organisation:
         logo = '{}{}{}'.format(
@@ -133,4 +131,4 @@ def get_html_email_renderer(service):
     else:
         branding = {}
 
-    return HTMLEmail(govuk_banner=govuk_banner, **branding)
+    return dict(govuk_banner=govuk_banner, **branding)

app/notifications/process_notifications.py

@@ -1,8 +1,6 @@
 from datetime import datetime
 
 from flask import current_app
-from notifications_utils.renderers import PassThrough
-from notifications_utils.template import Template
 
 from app import redis_store
 from app.celery import provider_tasks
@@ -11,14 +9,11 @@ from app.dao.notifications_dao import dao_create_notification, dao_delete_notifi
 from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE
 from app.notifications.validators import check_sms_content_char_count
 from app.v2.errors import BadRequestError, SendNotificationToQueueError
+from app.utils import get_template_instance
 
 
 def create_content_for_notification(template, personalisation):
-    template_object = Template(
+    template_object = get_template_instance(template.__dict__, personalisation)
-        template.__dict__,
-        personalisation,
-        renderer=PassThrough()
-    )
     check_placeholders(template_object)
 
     if template_object.template_type == SMS_TYPE:

app/notifications/rest.py

@@ -7,9 +7,6 @@ from flask import (
     current_app,
     json
 )
-from notifications_utils.field import Field
-from notifications_utils.renderers import PassThrough
-from notifications_utils.template import Template
 
 from app import api_user, create_uuid, statsd_client
 from app.clients.email.aws_ses import get_aws_responses
@@ -36,7 +33,7 @@ from app.schemas import (
     day_schema
 )
 from app.service.utils import service_allowed_to_send_to
-from app.utils import pagination_links
+from app.utils import pagination_links, get_template_instance
 
 notifications = Blueprint('notifications', __name__)
 
@@ -255,13 +252,13 @@ def send_notification(notification_type):
 
 def get_notification_return_data(notification_id, notification, template):
     output = {
-        'body': template.rendered,
+        'body': str(template),
         'template_version': notification['template_version'],
         'notification': {'id': notification_id}
     }
 
     if template.template_type == 'email':
-        output.update({'subject': str(Field(template.subject, template.values))})
+        output.update({'subject': template.subject})
 
     return output
 
@@ -288,11 +285,8 @@ def _service_allowed_to_send_to(notification, service):
 
 
 def create_template_object_for_notification(template, personalisation):
-    template_object = Template(
+    template_object = get_template_instance(template.__dict__, personalisation)
-        template.__dict__,
+
-        personalisation,
-        renderer=PassThrough()
-    )
     if template_object.missing_data:
         message = 'Missing personalisation: {}'.format(", ".join(template_object.missing_data))
         errors = {'template': [message]}

app/schemas.py

@@ -15,7 +15,6 @@ from marshmallow import (
 )
 from marshmallow_sqlalchemy import field_for
 
-from notifications_utils.field import Field
 from notifications_utils.recipients import (
     validate_email_address,
     InvalidEmailError,
@@ -24,11 +23,10 @@ from notifications_utils.recipients import (
     validate_and_format_phone_number
 )
 
-from notifications_utils.renderers import PassThrough
-
 from app import ma
 from app import models
 from app.dao.permissions_dao import permission_dao
+from app.utils import get_template_instance
 
 
 def _validate_positive_number(value, msg="Not a positive integer"):
@@ -390,19 +388,13 @@ class NotificationWithPersonalisationSchema(NotificationWithTemplateSchema):
     @post_dump
     def handle_template_merge(self, in_data):
         in_data['template'] = in_data.pop('template_history')
-        from notifications_utils.template import Template
+        template = get_template_instance(in_data['template'], in_data['personalisation'])
-        template = Template(
+        in_data['body'] = str(template)
-            in_data['template'],
+        if in_data['template']['template_type'] == models.EMAIL_TYPE:
-            in_data['personalisation'],
+            in_data['subject'] = template.subject
-            renderer=PassThrough()
-        )
-        in_data['body'] = template.rendered
-        template_type = in_data['template']['template_type']
-        if template_type == 'email':
-            in_data['subject'] = str(Field(template.subject, in_data['personalisation']))
             in_data['content_char_count'] = None
         else:
-            in_data['content_char_count'] = len(in_data['body'])
+            in_data['content_char_count'] = template.content_count
 
         in_data.pop('personalisation', None)
         in_data['template'].pop('content', None)

app/template/rest.py

@@ -13,9 +13,7 @@ from app.dao.templates_dao import (
     dao_get_all_templates_for_service,
     dao_get_template_versions
 )
-from notifications_utils.field import Field
+from notifications_utils.template import SMSMessageTemplate
-from notifications_utils.template import Template
-from notifications_utils.renderers import PassThrough
 from app.dao.services_dao import dao_fetch_service_by_id
 from app.models import SMS_TYPE
 from app.schemas import (template_schema, template_history_schema)
@@ -26,13 +24,16 @@ from app.errors import (
     register_errors,
     InvalidRequest
 )
+from app.utils import get_template_instance
 
 register_errors(template)
 
 
 def _content_count_greater_than_limit(content, template_type):
-    template = Template({'content': content, 'template_type': template_type})
+    if template_type != SMS_TYPE:
-    return template_type == SMS_TYPE and template.content_count > current_app.config.get('SMS_CHAR_COUNT_LIMIT')
+        return False
+    template = SMSMessageTemplate({'content': content, 'template_type': template_type})
+    return template.content_count > current_app.config.get('SMS_CHAR_COUNT_LIMIT')
 
 
 @template.route('', methods=['POST'])
@@ -93,7 +94,8 @@ 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 = Template(data, values=request.args.to_dict(), renderer=PassThrough())
+
+    template_object = get_template_instance(data, values=request.args.to_dict())
 
     if template_object.missing_data:
         raise InvalidRequest(
@@ -109,10 +111,7 @@ def preview_template_by_id_and_service_id(service_id, template_id):
             ]}, status_code=400
         )
 
-    data['subject'], data['content'] = (
+    data['subject'], data['content'] = template_object.subject, str(template_object)
-        str(Field(template_object.subject, template_object.values)),
-        template_object.rendered
-    )
 
     return jsonify(data)
 

app/utils.py

@@ -1,4 +1,6 @@
 from flask import url_for
+from app.models import SMS_TYPE, EMAIL_TYPE
+from notifications_utils.template import SMSMessageTemplate, PlainTextEmailTemplate
 
 
 def pagination_links(pagination, endpoint, **kwargs):
@@ -18,3 +20,9 @@ def url_with_token(data, url, config):
     token = generate_token(data, config['SECRET_KEY'], config['DANGEROUS_SALT'])
     base_url = config['ADMIN_BASE_URL'] + url
     return base_url + token
+
+
+def get_template_instance(template, values):
+    return {
+        SMS_TYPE: SMSMessageTemplate, EMAIL_TYPE: PlainTextEmailTemplate
+    }[template['template_type']](template, values)

app/v2/notifications/post_notifications.py

@@ -42,7 +42,7 @@ def post_sms_notification():
     send_notification_to_queue(notification, service.research_mode)
     sms_sender = service.sms_sender if service.sms_sender else current_app.config.get('FROM_NUMBER')
     resp = create_post_sms_response_from_notification(notification,
-                                                      template_with_content.rendered,
+                                                      str(template_with_content),
                                                       sms_sender,
                                                       request.url_root)
     return jsonify(resp), 201
@@ -70,7 +70,7 @@ def post_email_notification():
     send_notification_to_queue(notification, service.research_mode)
 
     resp = create_post_email_response_from_notification(notification=notification,
-                                                        content=template_with_content.rendered,
+                                                        content=str(template_with_content),
                                                         subject=template_with_content.subject,
                                                         email_from=service.email_from,
                                                         url_root=request.url_root)
@@ -89,5 +89,6 @@ def __validate_template(form, service, notification_type):
     check_template_is_for_notification_type(notification_type, template.template_type)
     check_template_is_active(template)
     template_with_content = create_content_for_notification(template, form.get('personalisation', {}))
-    check_sms_content_char_count(template_with_content.content_count)
+    if template.template_type == SMS_TYPE:
+        check_sms_content_char_count(template_with_content.content_count)
     return template, template_with_content

requirements.txt

@@ -22,6 +22,6 @@ Flask-Redis==0.1.0
 
 git+https://github.com/alphagov/notifications-python-client.git@3.0.0#egg=notifications-python-client==3.0.0
 
-git+https://github.com/alphagov/notifications-utils.git@11.1.1#egg=notifications-utils==11.1.1
+git+https://github.com/alphagov/notifications-utils.git@12.1.1#egg=notifications-utils==12.1.1
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

tests/app/dao/test_notification_dao.py

@@ -41,8 +41,6 @@ from app.dao.notifications_dao import (
     dao_timeout_notifications,
     get_financial_year)
 
-from notifications_utils.template import get_sms_fragment_count
-
 from tests.app.conftest import (sample_notification, sample_template, sample_email_template, sample_service, sample_job,
                                 sample_api_key)
 
@@ -820,22 +818,6 @@ def test_should_limit_notifications_return_by_day_limit_plus_one(notify_db, noti
     assert len(all_notifications) == 2
 
 
-@pytest.mark.parametrize(
-    "char_count, expected_sms_fragment_count",
-    [
-        (159, 1),
-        (160, 1),
-        (161, 2),
-        (306, 2),
-        (307, 3),
-        (459, 3),
-        (460, 4),
-        (461, 4)
-    ])
-def test_sms_fragment_count(char_count, expected_sms_fragment_count):
-    assert get_sms_fragment_count(char_count) == expected_sms_fragment_count
-
-
 def test_creating_notification_adds_to_notification_history(sample_template):
     data = _notification_json(sample_template)
     notification = Notification(**data)

tests/app/delivery/test_send_to_providers.py

@@ -355,11 +355,11 @@ def test_send_email_should_use_service_reply_to_email(
 
 
 def test_get_html_email_renderer_should_return_for_normal_service(sample_service):
-    renderer = send_to_providers.get_html_email_renderer(sample_service)
+    options = send_to_providers.get_html_email_options(sample_service)
-    assert renderer.govuk_banner
+    assert options['govuk_banner']
-    assert renderer.brand_colour is None
+    assert 'brand_colour' not in options.keys()
-    assert renderer.brand_logo is None
+    assert 'brand_logo' not in options.keys()
-    assert renderer.brand_name is None
+    assert 'brand_name' not in options.keys()
 
 
 @pytest.mark.parametrize('branding_type, govuk_banner', [
@@ -373,11 +373,11 @@ def test_get_html_email_renderer_with_branding_details(branding_type, govuk_bann
     notify_db.session.add_all([sample_service, org])
     notify_db.session.commit()
 
-    renderer = send_to_providers.get_html_email_renderer(sample_service)
+    options = send_to_providers.get_html_email_options(sample_service)
 
-    assert renderer.govuk_banner == govuk_banner
+    assert options['govuk_banner'] == govuk_banner
-    assert renderer.brand_colour == '000000'
+    assert options['brand_colour'] == '#000000'
-    assert renderer.brand_name == 'Justice League'
+    assert options['brand_name'] == 'Justice League'
 
 
 def test_get_html_email_renderer_prepends_logo_path(notify_db, sample_service):
@@ -387,9 +387,9 @@ def test_get_html_email_renderer_prepends_logo_path(notify_db, sample_service):
     notify_db.session.add_all([sample_service, org])
     notify_db.session.commit()
 
-    renderer = send_to_providers.get_html_email_renderer(sample_service)
+    renderer = send_to_providers.get_html_email_options(sample_service)
 
-    assert renderer.brand_logo == 'http://localhost:6012/static/images/email-template/crests/justice-league.png'
+    assert renderer['brand_logo'] == 'http://localhost:6012/static/images/email-template/crests/justice-league.png'
 
 
 def test_should_not_set_billable_units_if_research_mode(notify_db, sample_service, sample_notification, mocker):

tests/app/notifications/rest/test_send_notification.py

@@ -129,7 +129,7 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_email_t
                 queue="send-email"
             )
             assert response.status_code == 201
-            assert response_data['body'] == 'Hello Jo\nThis is an email from GOV.UK'
+            assert response_data['body'] == u'Hello Jo\nThis is an email from GOV.\u200BUK'
             assert response_data['subject'] == 'Jo'
 
 

tests/app/notifications/test_process_notification.py

@@ -18,14 +18,14 @@ from tests.app.conftest import sample_notification, sample_template, sample_emai
 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 content.rendered == template.content
+    assert str(content) == template.content
 
 
 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'})
     assert content.content == template.content
-    assert 'Bobby' in content.rendered
+    assert 'Bobby' in str(content)
 
 
 def test_create_content_for_notification_fails_with_missing_personalisation(sample_template_with_placeholders):

tests/app/notifications/test_rest.py

@@ -711,7 +711,7 @@ def test_get_notification_by_id_returns_merged_template_content_for_email(
 
         notification = json.loads(response.get_data(as_text=True))['data']['notification']
         assert response.status_code == 200
-        assert notification['body'] == 'Hello world\nThis is an email from GOV.UK'
+        assert notification['body'] == 'Hello world\nThis is an email from GOV.\u200BUK'
         assert notification['subject'] == 'world'
         assert notification['content_char_count'] is None
 

tests/app/v2/notifications/test_post_notifications.py

@@ -129,8 +129,10 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_
     assert resp_json['id'] == str(notification.id)
     assert resp_json['reference'] == reference
     assert notification.reference is None
-    assert resp_json['content']['body'] == sample_email_template_with_placeholders.content.replace('((name))', 'Bob')
+    assert resp_json['content']['body'] == sample_email_template_with_placeholders.content\
-    assert resp_json['content']['subject'] == sample_email_template_with_placeholders.subject
+        .replace('((name))', 'Bob').replace('GOV.UK', u'GOV.\u200bUK')
+    assert resp_json['content']['subject'] == sample_email_template_with_placeholders.subject\
+        .replace('((name))', 'Bob')
     assert resp_json['content']['from_email'] == sample_email_template_with_placeholders.service.email_from
     assert 'v2/notifications/{}'.format(notification.id) in resp_json['uri']
     assert resp_json['template']['id'] == str(sample_email_template_with_placeholders.id)