diff --git a/app/main/forms.py b/app/main/forms.py index 45b40abb5..cf1d3d99d 100644 --- 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(): diff --git a/app/main/views/send.py b/app/main/views/send.py index 4aee5cfeb..88c7035cf 100644 --- 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///check/.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///check/.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///check/", methods=['POST']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') diff --git a/app/main/views/templates.py b/app/main/views/templates.py index e2954970f..3aa19655d 100644 --- 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//templates/.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//templates//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//templates//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//templates/add-", 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'] ] ) diff --git a/app/utils.py b/app/utils.py index 80f8ad9ec..041e8cef0 100644 --- 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 + ) diff --git a/requirements.txt b/requirements.txt index c6f611d1c..e55de9ad3 100644 --- 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 diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index dcc19c64e..73c7ddefa 100644 --- 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'] == ( + '

Subject

\n' + '

Your vehicle tax is about to expire

' + ) + + def test_check_messages_should_revalidate_file_when_uploading_file( app_, service_one, diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 70b5d6b37..149c88879 100644 --- 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'] == ( + "

Your ((thing)) is due soon

\n" + "

Your vehicle tax expires on ((date))

" + ) def test_should_redirect_when_saving_a_template(app_, diff --git a/tests/conftest.py b/tests/conftest.py index 39ddb2a78..2bb84f4be 100644 --- 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')