Add remaining endpoints for PDFs and PNGs

Right now we can show what a letter template looks like as a PDF or PNG. This commit completes the work so this is also possible when: - showing a template with the placeholders replaced - showing any version of a template Also removes dependency on `Exception().message`, which was deprecated in Python 2.6. See 97f82d565f for full details.
2026-02-06 03:13:42 -05:00 · 2016-12-20 14:38:34 +00:00
parent a159f351a8
commit 559433c5d2
8 changed files with 214 additions and 33 deletions
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -96,7 +96,7 @@ class UKMobileNumber(TelField):
        try:
            validate_phone_number(self.data)
        except InvalidPhoneError as e:
-            raise ValidationError(e.message)
+            raise ValidationError(str(e))


 def mobile_number():
--- a/app/main/views/send.py
+++ b/app/main/views/send.py
@@ -1,9 +1,11 @@
 import itertools
 from string import ascii_uppercase
+from io import BytesIO

 from contextlib import suppress
 from zipfile import BadZipFile
 from xlrd.biffh import XLRDError
+from wand.image import Image

 from flask import (
    request,
@@ -13,10 +15,13 @@ from flask import (
    flash,
    abort,
    session,
-    current_app
+    current_app,
+    send_file,
 )

 from flask_login import login_required, current_user
+from flask_weasyprint import HTML, render_pdf
+
 from notifications_utils.columns import Columns
 from notifications_utils.recipients import RecipientCSV, first_column_headings, validate_and_format_phone_number

@@ -87,7 +92,11 @@ def choose_template(service_id, template_type):
    return render_template(
        'views/templates/choose.html',
        templates=[
-            get_template(template, current_service)
+            get_template(
+                template,
+                current_service,
+                letter_preview_url=url_for('.view_template', service_id=service_id, template_id=template['id']),
+            )
            for template in service_api_client.get_service_templates(service_id)['data']
            if template['template_type'] == template_type
        ],
@@ -104,7 +113,8 @@ def send_messages(service_id, template_id):
    template = get_template(
        service_api_client.get_service_template(service_id, template_id)['data'],
        current_service,
-        show_recipient=True
+        show_recipient=True,
+        letter_preview_url=url_for('.view_template', service_id=service_id, template_id=template_id),
    )

    form = CsvUploadForm()
@@ -165,7 +175,8 @@ def send_test(service_id, template_id):
    template = get_template(
        service_api_client.get_service_template(service_id, template_id)['data'],
        current_service,
-        show_recipient=True
+        show_recipient=True,
+        letter_preview_url=url_for('.view_template', service_id=service_id, template_id=template_id),
    )

    if len(template.placeholders) == 0 or request.method == 'POST':
@@ -208,7 +219,9 @@ def send_from_api(service_id, template_id):
    return render_template(
        'views/send-from-api.html',
        template=get_template(
-            service_api_client.get_service_template(service_id, template_id)['data'], current_service
+            service_api_client.get_service_template(service_id, template_id)['data'],
+            current_service,
+            letter_preview_url=url_for('.view_template', service_id=service_id, template_id=template_id)
        )
    )

@@ -233,7 +246,13 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False):
            session['upload_data'].get('template_id')
        )['data'],
        current_service,
-        show_recipient=True
+        show_recipient=True,
+        letter_preview_url=url_for(
+            '.check_messages',
+            service_id=service_id,
+            template_type=template_type,
+            upload_id=upload_id
+        ) if not letters_as_pdf else None
    )

    recipients = RecipientCSV(
@@ -304,6 +323,30 @@ def check_messages(service_id, template_type, upload_id):
    )


+@main.route("/services/<service_id>/<template_type>/check/<upload_id>.pdf", methods=['GET'])
+@login_required
+@user_has_permissions('send_texts', 'send_emails', 'send_letters')
+def check_messages_as_pdf(service_id, template_type, upload_id):
+    template = _check_messages(
+        service_id, template_type, upload_id, letters_as_pdf=True
+    )['template']
+    return render_pdf(HTML(string=str(template)))
+
+
+@main.route("/services/<service_id>/<template_type>/check/<upload_id>.png", methods=['GET'])
+@login_required
+@user_has_permissions('send_texts', 'send_emails', 'send_letters')
+def check_messages_as_png(service_id, template_type, upload_id):
+    output = BytesIO()
+    with Image(
+        blob=check_messages_as_pdf(service_id, template_type, upload_id).get_data()
+    ) as image:
+        with image.convert('png') as converted:
+            converted.save(file=output)
+    output.seek(0)
+    return send_file(output, mimetype='image/png')
+
+
@main.route("/services/<service_id>/<template_type>/check/<upload_id>", methods=['POST'])
@login_required
@user_has_permissions('send_texts', 'send_emails', 'send_letters')
--- a/app/main/views/templates.py
+++ b/app/main/views/templates.py
@@ -47,7 +47,8 @@ def view_template(service_id, template_id):
        template=get_template(
            service_api_client.get_service_template(service_id, template_id)['data'],
            current_service,
-            expand_emails=True
+            expand_emails=True,
+            letter_preview_url=url_for('.view_template', service_id=service_id, template_id=template_id)
        )
    )

@@ -66,7 +67,7 @@ def view_letter_template_as_pdf(service_id, template_id):
@main.route("/services/<service_id>/templates/<template_id>.png")
@login_required
@user_has_permissions('view_activity', admin_override=True)
-def view_letter_template_as_image(service_id, template_id):
+def view_letter_template_as_png(service_id, template_id):
    output = BytesIO()
    with Image(
        blob=view_letter_template_as_pdf(service_id, template_id).get_data()
@@ -79,9 +80,15 @@ def view_letter_template_as_image(service_id, template_id):

 def _view_template_version(service_id, template_id, version, letters_as_pdf=False):
    return dict(template=get_template(
-        service_api_client.get_service_template(service_id, template_id, version)['data'],
+        service_api_client.get_service_template(service_id, template_id, version=version)['data'],
        current_service,
        expand_emails=True,
+        letter_preview_url=url_for(
+            '.view_template_version',
+            service_id=service_id,
+            template_id=template_id,
+            version=version,
+        ) if not letters_as_pdf else None
    ))


@@ -103,6 +110,47 @@ def view_template_version(service_id, template_id, version):
    )


+@main.route("/services/<service_id>/templates/<template_id>/version/<int:version>.pdf")
+@login_required
+@user_has_permissions(
+    'view_activity',
+    'send_texts',
+    'send_emails',
+    'manage_templates',
+    'manage_api_keys',
+    admin_override=True,
+    any_=True
+)
+def view_template_version_as_pdf(service_id, template_id, version):
+    return render_pdf(HTML(string=str(
+        LetterPreviewTemplate(
+            service_api_client.get_service_template(service_id, template_id, version=version)['data'],
+        )
+    )))
+
+
+@main.route("/services/<service_id>/templates/<template_id>/version/<int:version>.png")
+@login_required
+@user_has_permissions(
+    'view_activity',
+    'send_texts',
+    'send_emails',
+    'manage_templates',
+    'manage_api_keys',
+    admin_override=True,
+    any_=True
+)
+def view_template_version_as_png(service_id, template_id, version):
+    output = BytesIO()
+    with Image(
+        blob=view_template_version_as_pdf(service_id, template_id, version).get_data()
+    ) as image:
+        with image.convert('png') as converted:
+            converted.save(file=output)
+    output.seek(0)
+    return send_file(output, mimetype='image/png')
+
+
@main.route("/services/<service_id>/templates/add-<template_type>", methods=['GET', 'POST'])
@login_required
@user_has_permissions('manage_templates', admin_override=True)
@@ -266,7 +314,17 @@ def view_template_versions(service_id, template_id):
    return render_template(
        'views/templates/choose_history.html',
        versions=[
-            get_template(template, current_service, expand_emails=True)
+            get_template(
+                template,
+                current_service,
+                expand_emails=True,
+                letter_preview_url=url_for(
+                    '.view_template_version',
+                    service_id=service_id,
+                    template_id=template_id,
+                    version=template['version'],
+                )
+            )
            for template in service_api_client.get_service_template_versions(service_id, template_id)['data']
        ]
    )
--- a/app/utils.py
+++ b/app/utils.py
@@ -12,6 +12,7 @@ from notifications_utils.template import (
    SMSPreviewTemplate,
    EmailPreviewTemplate,
    LetterPDFLinkTemplate,
+    LetterPreviewTemplate,
 )

 import pyexcel
@@ -224,7 +225,13 @@ def is_gov_user(email_address):
    return bool(re.search(email_regex, email_address.lower()))


-def get_template(template, service, show_recipient=False, expand_emails=False):
+def get_template(
+    template,
+    service,
+    show_recipient=False,
+    expand_emails=False,
+    letter_preview_url=None,
+):
    if 'email' == template['template_type']:
        return EmailPreviewTemplate(
            template,
@@ -241,7 +248,12 @@ def get_template(template, service, show_recipient=False, expand_emails=False):
            show_recipient=show_recipient
        )
    if 'letter' == template['template_type']:
-        return LetterPDFLinkTemplate(
-            template,
-            service_id=service['id'],
-        )
+        if letter_preview_url:
+            return LetterPDFLinkTemplate(
+                template,
+                preview_url=letter_preview_url,
+            )
+        else:
+            return LetterPreviewTemplate(
+                template
+            )
--- a/requirements.txt
+++ b/requirements.txt
@@ -25,4 +25,4 @@ wand==0.4.4

 git+https://github.com/alphagov/notifications-python-client.git@3.0.1#egg=notifications-python-client==3.0.1

-git+https://github.com/alphagov/notifications-utils.git@12.1.1#egg=notifications-utils==12.1.1
+git+https://github.com/alphagov/notifications-utils.git@13.0.1#egg=notifications-utils==13.0.1
--- a/tests/app/main/views/test_send.py
+++ b/tests/app/main/views/test_send.py
@@ -4,10 +4,12 @@ from glob import glob
 import re
 from itertools import repeat
 from functools import partial
+from unittest.mock import Mock, patch

 import pytest
 from bs4 import BeautifulSoup
 from flask import url_for
+from notifications_utils.template import LetterPreviewTemplate

 from tests import validate_route_permission

@@ -432,6 +434,54 @@ def test_cant_start_letters_job(
    mock_create_job.assert_not_called()


+@pytest.mark.parametrize(
+    'view, expected_content_type',
+    [
+        ('.check_messages_as_pdf', 'application/pdf'),
+        ('.check_messages_as_png', 'image/png'),
+    ]
+)
+@patch('app.utils.LetterPreviewTemplate.jinja_template.render', return_value='')
+def test_should_show_preview_letter_message(
+    mock_letter_preview,
+    view,
+    expected_content_type,
+    logged_in_client,
+    mock_get_service_letter_template,
+    mock_get_users_by_service,
+    mock_get_detailed_service_for_today,
+    fake_uuid,
+    mocker,
+):
+
+    mocker.patch(
+        'app.main.views.send.s3download',
+        return_value='\n'.join(
+            ['address line 1, postcode'] +
+            ['123 street, abc123']
+        )
+    )
+
+    service_id = fake_uuid
+    template_id = fake_uuid
+    with logged_in_client.session_transaction() as session:
+        session['upload_data'] = {
+            'original_file_name': 'example.csv',
+            'template_id': fake_uuid,
+            'notification_count': 1,
+            'valid': True
+        }
+    response = logged_in_client.get(url_for(view, service_id=service_id, template_type='letter', upload_id=fake_uuid))
+
+    assert response.status_code == 200
+    assert response.content_type == expected_content_type
+    mock_get_service_letter_template.assert_called_with(service_id, template_id)
+    assert mock_letter_preview.call_args[0][0]['message'] == (
+        '<h2>Subject</h2>\n'
+        '<p>Your vehicle tax is about to expire</p>'
+    )
+
+
 def test_check_messages_should_revalidate_file_when_uploading_file(
    app_,
    service_one,
--- a/tests/app/main/views/test_templates.py
+++ b/tests/app/main/views/test_templates.py
@@ -1,4 +1,5 @@
 from functools import partial
+from itertools import repeat
 from datetime import datetime
 from unittest.mock import Mock, patch

@@ -9,6 +10,10 @@ from freezegun import freeze_time
 from notifications_python_client.errors import HTTPError

 from tests import validate_route_permission, template_json, single_notification_json
+from tests.conftest import (
+    mock_get_service_email_template,
+    mock_get_template_version,
+)
 from app.main.views.templates import get_last_use_message, get_human_readable_delta


@@ -40,17 +45,20 @@ def test_should_show_page_for_one_template(
        service_id, template_id)


-@pytest.mark.parametrize(
-    'view, expected_content_type',
-    [
-        ('.view_letter_template_as_pdf', 'application/pdf'),
-        ('.view_letter_template_as_image', 'image/png'),
-    ]
-)
-@patch("app.main.views.templates.LetterPreviewTemplate")
+@pytest.mark.parametrize('view_suffix, expected_content_type', [
+    ('as_pdf', 'application/pdf'),
+    ('as_png', 'image/png'),
+])
+@pytest.mark.parametrize('view, extra_view_args', [
+    ('.view_letter_template', {}),
+    ('.view_template_version', {'version': 1}),
+])
+@patch('app.main.views.templates.LetterPreviewTemplate.jinja_template.render', return_value='foo')
 def test_should_show_preview_letter_templates(
    mock_letter_preview,
    view,
+    extra_view_args,
+    view_suffix,
    expected_content_type,
    client,
    api_user_active,
@@ -60,17 +68,27 @@ def test_should_show_preview_letter_templates(
    mock_get_user,
    mock_get_user_by_email,
    mock_has_permissions,
-    fake_uuid
+    fake_uuid,
+    mocker
 ):
    client.login(api_user_active)
-    service_id = fake_uuid
-    template_id = fake_uuid
-    response = client.get(url_for(view, service_id=service_id, template_id=template_id))
+    service_id, template_id = repeat(fake_uuid, 2)
+    response = client.get(url_for(
+        '{}_{}'.format(view, view_suffix),
+        service_id=service_id,
+        template_id=template_id,
+        **extra_view_args
+    ))

    assert response.status_code == 200
    assert response.content_type == expected_content_type
-    mock_get_service_email_template.assert_called_with(service_id, template_id)
-    assert mock_letter_preview.call_args[0][0]['content'] == "Your vehicle tax expires on ((date))"
+    mock_get_service_email_template.assert_called_with(service_id, template_id, **extra_view_args)
+    print(mock_letter_preview)
+    print(mock_letter_preview.call_args)
+    assert mock_letter_preview.call_args[0][0]['message'] == (
+        "<h2>Your <span class='placeholder'>((thing))</span> is due soon</h2>\n"
+        "<p>Your vehicle tax expires on <span class='placeholder'>((date))</span></p>"
+    )


 def test_should_redirect_when_saving_a_template(app_,
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -316,7 +316,7 @@ def mock_get_service_template_with_placeholders(mocker):

@pytest.fixture(scope='function')
 def mock_get_service_email_template(mocker):
-    def _create(service_id, template_id):
+    def _get(service_id, template_id, version=None):
        template = template_json(
            service_id,
            template_id,
@@ -328,7 +328,7 @@ def mock_get_service_email_template(mocker):
        return {'data': template}

    return mocker.patch(
-        'app.service_api_client.get_service_template', side_effect=_create)
+        'app.service_api_client.get_service_template', side_effect=_get)


@pytest.fixture(scope='function')