diff --git a/README.md b/README.md index 30d3f7d2a..cca5660fe 100644 --- a/README.md +++ b/README.md @@ -25,9 +25,10 @@ Languages needed - [Node](https://nodejs.org/) 5.0.0 or greater - [npm](https://www.npmjs.com/) 3.0.0 or greater ```shell - brew install node imagemagick ghostscript cairo pango + brew install node ``` + [NPM](npmjs.org) is Node's package management tool. `n` is a tool for managing different versions of Node. The following installs `n` and uses the latest version of Node. diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index 88f352212..5ad48d854 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -27,6 +27,8 @@ def set_config_env_vars(vcap_services): extract_hosted_graphite_config(s) elif s['name'] == 'deskpro': extract_deskpro_config(s) + elif s['name'] == 'notify-template-preview': + extract_template_preview_config(s) def extract_notify_config(notify_config): @@ -49,3 +51,8 @@ def extract_hosted_graphite_config(hosted_graphite_config): def extract_deskpro_config(deskpro_config): os.environ['DESKPRO_API_HOST'] = deskpro_config['credentials']['api_host'] os.environ['DESKPRO_API_KEY'] = deskpro_config['credentials']['api_key'] + + +def extract_template_preview_config(template_preview_config): + os.environ['TEMPLATE_PREVIEW_API_HOST'] = template_preview_config['credentials']['api_host'] + os.environ['TEMPLATE_PREVIEW_API_KEY'] = template_preview_config['credentials']['api_key'] diff --git a/app/config.py b/app/config.py index 6cc115ae6..53eb1e8ac 100644 --- a/app/config.py +++ b/app/config.py @@ -20,6 +20,9 @@ class Config(object): # if we're not on cloudfoundry, we can get to this app from localhost. but on cloudfoundry its different ADMIN_BASE_URL = os.environ.get('ADMIN_BASE_URL', 'http://localhost:6012') + TEMPLATE_PREVIEW_API_HOST = os.environ.get('TEMPLATE_PREVIEW_API_HOST', 'http://localhost:6013') + TEMPLATE_PREVIEW_API_KEY = os.environ.get('TEMPLATE_PREVIEW_API_KEY', 'my-secret-key') + # Hosted graphite statsd prefix STATSD_PREFIX = os.getenv('STATSD_PREFIX') diff --git a/app/main/views/send.py b/app/main/views/send.py index 9feee1257..dd20ff41f 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -19,7 +19,6 @@ from flask import ( ) 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 @@ -36,9 +35,9 @@ from app.utils import ( get_errors_for_csv, Spreadsheet, get_help_argument, - get_template, - png_from_pdf, + get_template ) +from app.template_previews import TemplatePreview def get_page_headings(template_type): @@ -291,23 +290,17 @@ def check_messages(service_id, template_type, upload_id): ) -@main.route("/services///check/.pdf", methods=['GET']) +@main.route("/services///check/.", methods=['GET']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') -def check_messages_as_pdf(service_id, template_type, upload_id): +def check_messages_preview(service_id, template_type, upload_id, filetype): + if filetype not in ('pdf', 'png'): + abort(404) + 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): - return send_file(**png_from_pdf( - check_messages_as_pdf(service_id, template_type, upload_id) - )) + return TemplatePreview.from_utils_template(template, filetype) @main.route("/services///check/", methods=['POST']) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 203ae0b64..e034f5647 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -8,20 +8,17 @@ from flask import ( url_for, flash, abort, - send_file, - current_app ) from flask_login import login_required, current_user -from flask_weasyprint import HTML, render_pdf from dateutil.parser import parse from notifications_utils.formatters import escape_html -from notifications_utils.template import LetterPreviewTemplate from notifications_utils.recipients import first_column_headings from notifications_python_client.errors import HTTPError from app.main import main -from app.utils import user_has_permissions, get_template, png_from_pdf +from app.utils import user_has_permissions, get_template +from app.template_previews import TemplatePreview from app.main.forms import ( ChooseTemplateType, SMSTemplateForm, @@ -87,26 +84,15 @@ def view_template(service_id, template_id): ) -@main.route("/services//templates/.pdf") +@main.route("/services//templates/.") @login_required @user_has_permissions('view_activity', admin_override=True) -def view_letter_template_as_pdf(service_id, template_id): - return render_pdf(HTML(string=str( - LetterPreviewTemplate( - service_api_client.get_service_template(service_id, template_id)['data'], - contact_block=current_service['letter_contact_block'], - admin_base_url=current_app.config['ADMIN_BASE_URL'] - ) - ))) +def view_letter_template_preview(service_id, template_id, filetype): + if filetype not in ('pdf', 'png'): + abort(404) - -@main.route("/services//templates/.png") -@login_required -@user_has_permissions('view_activity', admin_override=True) -def view_letter_template_as_png(service_id, template_id): - return send_file(**png_from_pdf( - view_letter_template_as_pdf(service_id, template_id) - )) + db_template = service_api_client.get_service_template(service_id, template_id)['data'] + return TemplatePreview.from_database_object(db_template, filetype) def _view_template_version(service_id, template_id, version, letters_as_pdf=False): @@ -141,7 +127,7 @@ def view_template_version(service_id, template_id, version): ) -@main.route("/services//templates//version/.pdf") +@main.route("/services//templates//version/.") @login_required @user_has_permissions( 'view_activity', @@ -152,31 +138,9 @@ def view_template_version(service_id, template_id, version): 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'], - contact_block=current_service['letter_contact_block'], - admin_base_url=current_app.config['ADMIN_BASE_URL'] - ) - ))) - - -@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): - return send_file(**png_from_pdf( - view_template_version_as_pdf(service_id, template_id, version) - )) +def view_template_version_preview(service_id, template_id, version, filetype): + db_template = service_api_client.get_service_template(service_id, template_id, version=version)['data'] + return TemplatePreview.from_database_object(db_template, filetype) @main.route("/services//templates/add", methods=['GET', 'POST']) diff --git a/app/template_previews.py b/app/template_previews.py new file mode 100644 index 000000000..e3b5adf6c --- /dev/null +++ b/app/template_previews.py @@ -0,0 +1,28 @@ +from flask import current_app +import requests + +from app import current_service + + +class TemplatePreview: + @classmethod + def from_database_object(cls, template, filetype, values=None): + data = { + 'letter_contact_block': current_service['letter_contact_block'], + 'template': template, + 'values': values + } + resp = requests.post( + '{}/preview.{}'.format(current_app.config['TEMPLATE_PREVIEW_API_HOST'], filetype), + json=data, + headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} + ) + return (resp.content, resp.status_code, resp.headers.items()) + + @classmethod + def from_utils_template(cls, template, filetype): + return cls.from_database_object( + template._template, + filetype, + template.values + ) diff --git a/app/utils.py b/app/utils.py index ff4f7912b..d6e8703f3 100644 --- a/app/utils.py +++ b/app/utils.py @@ -16,8 +16,6 @@ from flask import ( ) from flask_login import current_user -from wand.image import Image - from notifications_utils.template import ( SMSPreviewTemplate, EmailPreviewTemplate, @@ -327,21 +325,6 @@ def get_template( ) -def png_from_pdf(pdf_endpoint): - output = BytesIO() - with Image( - blob=pdf_endpoint.get_data(), - resolution=150, - ) as image: - with image.convert('png') as converted: - converted.save(file=output) - output.seek(0) - return dict( - filename_or_fp=output, - mimetype='image/png', - ) - - def get_current_financial_year(): now = datetime.utcnow() current_month = int(now.strftime('%-m')) diff --git a/requirements.txt b/requirements.txt index af173bd58..ed4678d73 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,9 +4,7 @@ Flask==0.10.1 Flask-Script==2.0.5 Flask-WTF==0.11 Flask-Login==0.3.2 -Flask-WeasyPrint==0.5 -html5lib==1.0b10 credstash==1.8.0 boto3==1.4.4 Pygments==2.0.2 @@ -20,7 +18,6 @@ pyexcel-xlsx==0.1.0 pyexcel-ods3==0.1.1 pytz==2016.4 six==1.10.0 -wand==0.4.4 gunicorn==19.6.0 whitenoise==1.0.6 #manages static assets diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index f85470f4a..f643cce6a 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -2,14 +2,13 @@ import uuid from io import BytesIO from os import path from glob import glob -import re from itertools import repeat from functools import partial -from unittest.mock import patch import pytest from bs4 import BeautifulSoup from flask import url_for +from notifications_utils.template import LetterPreviewTemplate from app.main.views.send import get_check_messages_back_url @@ -524,18 +523,9 @@ def test_can_start_letters_job( assert response.status_code == 302 -@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='') +@pytest.mark.parametrize('filetype', ['pdf', 'png']) def test_should_show_preview_letter_message( - mock_letter_preview, - view, - expected_content_type, + filetype, logged_in_platform_admin_client, mock_get_service_letter_template, mock_get_users_by_service, @@ -554,6 +544,10 @@ def test_should_show_preview_letter_message( ['123 street, abc123'] ) ) + mocked_preview = mocker.patch( + 'app.main.views.send.TemplatePreview.from_utils_template', + return_value='foo' + ) service_id = service_one['id'] template_id = fake_uuid @@ -565,18 +559,41 @@ def test_should_show_preview_letter_message( 'valid': True } response = logged_in_platform_admin_client.get( - url_for(view, service_id=service_id, template_type='letter', upload_id=fake_uuid) + url_for( + 'main.check_messages_preview', + service_id=service_id, + template_type='letter', + upload_id=fake_uuid, + filetype=filetype + ) ) - 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]['subject'] == ( - 'Subject' - ) - assert mock_letter_preview.call_args[0][0]['message'] == ( - '

Template <em>content</em> with & entity

' + + assert response.status_code == 200 + assert response.get_data(as_text=True) == 'foo' + assert mocked_preview.call_args[0][0].id == template_id + assert type(mocked_preview.call_args[0][0]) == LetterPreviewTemplate + assert mocked_preview.call_args[0][1] == filetype + + +def test_dont_show_preview_letter_templates_for_bad_filetype( + logged_in_client, + mock_get_service_template, + service_one, + fake_uuid +): + resp = logged_in_client.get( + url_for( + 'main.check_messages_preview', + service_id=service_one['id'], + template_type='letter', + upload_id=fake_uuid, + filetype='blah' + ) ) + assert resp.status_code == 404 + assert mock_get_service_template.called is False def test_check_messages_should_revalidate_file_when_uploading_file( diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index b321ca50a..8f5d68ccb 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1,6 +1,5 @@ -from itertools import repeat from datetime import datetime -from unittest.mock import Mock, patch, ANY +from unittest.mock import Mock, ANY import pytest from bs4 import BeautifulSoup @@ -148,44 +147,60 @@ def test_should_show_page_template_with_priority_select_if_platform_admin( mock_get_service_template.assert_called_with('1234', template_id) -@pytest.mark.parametrize('view_suffix, expected_content_type', [ - ('as_pdf', 'application/pdf'), - ('as_png', 'image/png'), -]) +@pytest.mark.parametrize('filetype', ['pdf', 'png']) @pytest.mark.parametrize('view, extra_view_args', [ - ('.view_letter_template', {}), - ('.view_template_version', {'version': 1}), + ('.view_letter_template_preview', {}), + ('.view_template_version_preview', {'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, + filetype, logged_in_client, mock_get_service_email_template, - mock_get_user_by_email, service_one, - fake_uuid + fake_uuid, + mocker ): + mocked_preview = mocker.patch( + 'app.main.views.templates.TemplatePreview.from_database_object', + return_value='foo' + ) + service_id, template_id = service_one['id'], fake_uuid + response = logged_in_client.get(url_for( - '{}_{}'.format(view, view_suffix), + view, service_id=service_id, template_id=template_id, + filetype=filetype, **extra_view_args )) assert response.status_code == 200 - assert response.content_type == expected_content_type + assert response.get_data(as_text=True) == 'foo' mock_get_service_email_template.assert_called_with(service_id, template_id, **extra_view_args) - assert mock_letter_preview.call_args[0][0]['subject'] == ( - "Your ((thing)) is due soon" - ) - assert mock_letter_preview.call_args[0][0]['message'] == ( - "

Your vehicle tax expires on ((date))

" + assert mocked_preview.call_args[0][0]['id'] == template_id + assert mocked_preview.call_args[0][0]['service'] == service_id + assert mocked_preview.call_args[0][1] == filetype + + +def test_dont_show_preview_letter_templates_for_bad_filetype( + logged_in_client, + mock_get_service_template, + service_one, + fake_uuid +): + resp = logged_in_client.get( + url_for( + '.view_letter_template_preview', + service_id=service_one['id'], + template_id=fake_uuid, + filetype='blah' + ) ) + assert resp.status_code == 404 + assert mock_get_service_template.called is False def test_should_redirect_when_saving_a_template( diff --git a/tests/app/test_cloudfoundry_config.py b/tests/app/test_cloudfoundry_config.py index 5af028ca7..a3a8ab63a 100644 --- a/tests/app/test_cloudfoundry_config.py +++ b/tests/app/test_cloudfoundry_config.py @@ -52,12 +52,24 @@ def deskpro_config(): } +@pytest.fixture +def template_preview_config(): + return { + 'name': 'notify-template-preview', + 'credentials': { + 'api_host': 'template-preview api host', + 'api_key': 'template-preview api key' + } + } + + @pytest.fixture def cloudfoundry_config( notify_config, aws_config, hosted_graphite_config, deskpro_config, + template_preview_config, ): return { 'user-provided': [ @@ -65,6 +77,7 @@ def cloudfoundry_config( aws_config, hosted_graphite_config, deskpro_config, + template_preview_config, ] } @@ -128,3 +141,11 @@ def test_deskpro_config(): assert os.environ['DESKPRO_API_HOST'] == 'deskpro api host' assert os.environ['DESKPRO_API_KEY'] == 'deskpro api key' + + +@pytest.mark.usefixtures('os_environ', 'cloudfoundry_environ') +def test_template_preview_config(): + extract_cloudfoundry_config() + + assert os.environ['TEMPLATE_PREVIEW_API_HOST'] == 'template-preview api host' + assert os.environ['TEMPLATE_PREVIEW_API_KEY'] == 'template-preview api key' diff --git a/tests/app/test_template_previews.py b/tests/app/test_template_previews.py new file mode 100644 index 000000000..be50e642b --- /dev/null +++ b/tests/app/test_template_previews.py @@ -0,0 +1,35 @@ +from unittest.mock import Mock +from notifications_utils.template import LetterPreviewTemplate + +from app.template_previews import TemplatePreview + + +def test_from_utils_template_calls_through(mocker, mock_get_service_letter_template): + mock_from_db = mocker.patch('app.template_previews.TemplatePreview.from_database_object') + template = LetterPreviewTemplate(mock_get_service_letter_template(None, None)['data']) + + ret = TemplatePreview.from_utils_template(template, 'foo') + + assert ret == mock_from_db.return_value + mock_from_db.assert_called_once_with(template._template, 'foo', template.values) + + +def test_from_database_object_makes_request(mocker, client): + resp = Mock(content='a', status_code='b', headers={'c': 'd'}) + request_mock = mocker.patch('app.template_previews.requests.post', return_value=resp) + mocker.patch('app.template_previews.current_service', __getitem__=Mock(return_value='123')) + + ret = TemplatePreview.from_database_object(template='foo', filetype='bar') + + assert ret[0] == 'a' + assert ret[1] == 'b' + assert list(ret[2]) == [('c', 'd')] + url = 'http://localhost:6013/preview.bar' + data = { + 'letter_contact_block': '123', + 'template': 'foo', + 'values': None + } + headers = {'Authorization': 'Token my-secret-key'} + + request_mock.assert_called_once_with(url, json=data, headers=headers)