From 79053dec93c2c3921b4dc4e653fe0f5bc4caaa37 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 9 Sep 2019 12:22:52 +0100 Subject: [PATCH] Allow uploaded letters to be sent if valid Added a send button which only appears on the page if the query string indicates that the PDF is valid. Before actually sending, we check that the service has the right permissions and that the metadata for the letter confirms the letter is valid (because the query string can be changed). --- app/main/views/uploads.py | 28 +++++++- app/navigation.py | 4 ++ app/notify_client/notification_api_client.py | 8 +++ app/templates/views/uploads/preview.html | 14 ++++ tests/app/main/views/test_uploads.py | 68 +++++++++++++++++++ .../notify_client/test_notification_client.py | 17 +++++ 6 files changed, 138 insertions(+), 1 deletion(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 28e40b8f6..5acb8a898 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -2,6 +2,7 @@ import uuid from io import BytesIO from flask import ( + abort, current_app, flash, redirect, @@ -13,7 +14,7 @@ from notifications_utils.pdf import pdf_page_count from PyPDF2.utils import PdfReadError from requests import RequestException -from app import current_service, service_api_client +from app import current_service, notification_api_client, service_api_client from app.extensions import antivirus_client from app.main import main from app.main.forms import PDFUploadForm @@ -115,6 +116,7 @@ def uploaded_letter_preview(service_id, file_id): original_filename=original_filename, template=template, status=status, + file_id=file_id, ) @@ -130,3 +132,27 @@ def view_letter_upload_as_preview(service_id, file_id): return TemplatePreview.from_invalid_pdf_file(pdf_file, page) else: return TemplatePreview.from_valid_pdf_file(pdf_file, page) + + +@main.route("/services//upload-letter/send", methods=['POST']) +@user_has_permissions('send_messages', restrict_admin_usage=True) +def send_uploaded_letter(service_id): + filename = request.form['filename'] + file_id = request.form['file_id'] + + if not (current_service.has_permission('letter') and current_service.has_permission('upload_letters')): + abort(403) + + file_location = get_transient_letter_file_location(service_id, file_id) + _, metadata = get_letter_pdf_and_metadata(file_location) + + if metadata.get('status') != 'valid': + abort(403) + + notification_api_client.send_precompiled_letter(service_id, filename, file_id) + + return redirect(url_for( + '.view_notification', + service_id=service_id, + notification_id=file_id, + )) diff --git a/app/navigation.py b/app/navigation.py index cbd05f76d..5cc6b833d 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -244,6 +244,7 @@ class HeaderNavigation(Navigation): 'send_test', 'send_test_preview', 'send_test_step', + 'send_uploaded_letter', 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', @@ -556,6 +557,7 @@ class MainNavigation(Navigation): 'robots', 'security', 'send_notification', + 'send_uploaded_letter', 'service_dashboard_updates', 'service_delete_email_reply_to', 'service_delete_letter_contact', @@ -792,6 +794,7 @@ class CaseworkNavigation(Navigation): 'send_messages', 'send_notification', 'send_test_preview', + 'send_uploaded_letter', 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', @@ -1074,6 +1077,7 @@ class OrgNavigation(Navigation): 'send_test', 'send_test_preview', 'send_test_step', + 'send_uploaded_letter', 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py index 66b16448a..d474214bc 100644 --- a/app/notify_client/notification_api_client.py +++ b/app/notify_client/notification_api_client.py @@ -59,6 +59,14 @@ class NotificationApiClient(NotifyAdminAPIClient): data = _attach_current_user(data) return self.post(url='/service/{}/send-notification'.format(service_id), data=data) + def send_precompiled_letter(self, service_id, filename, file_id): + data = { + 'filename': filename, + 'file_id': file_id, + } + data = _attach_current_user(data) + return self.post(url='/service/{}/send-pdf-letter'.format(service_id), data=data) + def get_notification(self, service_id, notification_id): return self.get(url='/service/{}/notifications/{}'.format(service_id, notification_id)) diff --git a/app/templates/views/uploads/preview.html b/app/templates/views/uploads/preview.html index 9001411b0..c8d954887 100644 --- a/app/templates/views/uploads/preview.html +++ b/app/templates/views/uploads/preview.html @@ -20,4 +20,18 @@
{{ template|string }}
+ + {% if status == 'valid' %} +
+
+ + + + +
+
+ {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 609ff116b..6638ef3ef 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -50,6 +50,10 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf' assert not page.find(id='validation-error-message') + assert page.find('input', {'type': 'hidden', 'name': 'filename', 'value': 'tests/test_pdf_files/one_page_pdf.pdf'}) + assert page.find('input', {'type': 'hidden', 'name': 'file_id', 'value': 'fake-uuid'}) + assert page.find('button', {'type': 'submit'}).text == 'Send 1 letter' + def test_post_upload_letter_shows_letter_preview_for_valid_file(mocker, client_request): letter_template = {'template_type': 'letter', @@ -182,6 +186,7 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): assert normalize_spaces( page.find(id='validation-error-message').text ) == 'Validation failed' + assert not page.find('button', {'type': 'submit'}) def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client_request): @@ -254,3 +259,66 @@ def test_uploaded_letter_preview(mocker, client_request): assert page.find('h1').text == 'my_letter.pdf' assert page.find('div', class_='letter-sent') + + +def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(mocker, service_one, client_request): + mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', {'status': 'valid'})) + mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter') + + service_one['permissions'] = ['letter', 'upload_letters'] + file_id = 'abcd-1234' + + client_request.post( + 'main.send_uploaded_letter', + service_id=SERVICE_ONE_ID, + _data={'filename': 'my_file.pdf', 'file_id': file_id}, + _expected_redirect=url_for( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=file_id, + _external=True + ) + ) + mock_send.assert_called_once_with(SERVICE_ONE_ID, 'my_file.pdf', file_id) + + +@pytest.mark.parametrize('permissions', [ + ['email'], + ['letter'], + ['upload_letters'], +]) +def test_send_uploaded_letter_when_service_does_not_have_correct_permissions( + mocker, + service_one, + client_request, + permissions, +): + mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', {'status': 'valid'})) + mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter') + + service_one['permissions'] = permissions + file_id = 'abcd-1234' + + client_request.post( + 'main.send_uploaded_letter', + service_id=SERVICE_ONE_ID, + _data={'filename': 'my_file.pdf', 'file_id': file_id}, + _expected_status=403 + ) + assert not mock_send.called + + +def test_send_uploaded_letter_when_metadata_states_pdf_is_invalid(mocker, service_one, client_request): + mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', {'status': 'invalid'})) + mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter') + + service_one['permissions'] = ['letter', 'upload_letters'] + file_id = 'abcd-1234' + + client_request.post( + 'main.send_uploaded_letter', + service_id=SERVICE_ONE_ID, + _data={'filename': 'my_file.pdf', 'file_id': file_id}, + _expected_status=403 + ) + assert not mock_send.called diff --git a/tests/app/notify_client/test_notification_client.py b/tests/app/notify_client/test_notification_client.py index ef48dec69..f9a90f702 100644 --- a/tests/app/notify_client/test_notification_client.py +++ b/tests/app/notify_client/test_notification_client.py @@ -59,6 +59,23 @@ def test_send_notification(mocker, logged_in_client, active_user_with_permission ) +def test_send_precompiled_letter(mocker, logged_in_client, active_user_with_permissions): + mock_post = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.post') + NotificationApiClient().send_precompiled_letter( + 'abcd-1234', + 'my_file.pdf', + 'file-ID' + ) + mock_post.assert_called_once_with( + url='/service/abcd-1234/send-pdf-letter', + data={ + 'filename': 'my_file.pdf', + 'file_id': 'file-ID', + 'created_by': active_user_with_permissions['id'] + } + ) + + def test_get_notification(mocker): mock_get = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.get') NotificationApiClient().get_notification('foo', 'bar')