diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index 00e2adff0..12156e7f9 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -1,14 +1,20 @@ -import base64 +from io import BytesIO -from flask import jsonify, request, url_for, current_app +from flask import jsonify, request, url_for, current_app, send_file from app import api_user, authenticated_service from app.dao import notifications_dao from app.letters.utils import get_letter_pdf from app.schema_validation import validate +from app.v2.errors import BadRequestError from app.v2.notifications import v2_notification_blueprint from app.v2.notifications.notification_schemas import get_notifications_request, notification_by_id -from app.models import NOTIFICATION_CREATED, NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS, LETTER_TYPE +from app.models import ( + NOTIFICATION_PENDING_VIRUS_CHECK, + NOTIFICATION_VIRUS_SCAN_FAILED, + NOTIFICATION_TECHNICAL_FAILURE, + LETTER_TYPE, +) @v2_notification_blueprint.route("/", methods=['GET']) @@ -18,18 +24,43 @@ def get_notification_by_id(notification_id): notification = notifications_dao.get_notification_with_personalisation( authenticated_service.id, notification_id, key_type=None ) + return jsonify(notification.serialize()), 200 - response = notification.serialize() - if request.args.get('return_pdf_content') and notification.notification_type == LETTER_TYPE: - if notification.status in (NOTIFICATION_CREATED, *NOTIFICATION_STATUS_TYPES_BILLABLE_FOR_LETTERS): - pdf_data = get_letter_pdf(notification) - else: - # precompiled letters that are still being virus scanned, or that failed validation/virus scan - pdf_data = b'' - response['body'] = base64.b64encode(pdf_data).decode('utf-8') +@v2_notification_blueprint.route('//pdf', methods=['GET']) +def get_pdf_for_notification(notification_id): + _data = {"notification_id": notification_id} + validate(_data, notification_by_id) + notification = notifications_dao.get_notification_by_id( + notification_id, authenticated_service.id, _raise=True + ) - return jsonify(response), 200 + if notification.notification_type != LETTER_TYPE: + raise BadRequestError(message="Notification is not a letter", status_code=400) + + if notification.status in { + NOTIFICATION_PENDING_VIRUS_CHECK, + NOTIFICATION_VIRUS_SCAN_FAILED, + NOTIFICATION_TECHNICAL_FAILURE, + }: + raise BadRequestError( + message='PDF not available for letters in status {}'.format(notification.status), + status_code=400 + ) + + try: + pdf_data = get_letter_pdf(notification) + except Exception: + # this probably means it's a templated letter that hasn't been created yet + current_app.logger.info('PDF not found for notification id {} status {}'.format( + notification.id, + notification.status + )) + raise BadRequestError( + message='PDF not available for letter, try again later', + status_code=400) + + return send_file(filename_or_fp=BytesIO(pdf_data), mimetype='application/pdf') @v2_notification_blueprint.route("", methods=['GET']) diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 699f0fb0b..d2b126493 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -651,36 +651,85 @@ def test_get_notifications_renames_letter_statuses(client, sample_letter_templat assert json_response['status'] == expected_status -@pytest.mark.parametrize('status,expected_body,mock_called', [ - ('created', 'Zm9v', True), - ('sending', 'Zm9v', True), - ('pending-virus-check', '', False), - ('virus-scan-failed', '', False), - ('validation-failed', '', False), - ('technical-failure', '', False), +def test_get_pdf_for_notification_returns_pdf_content( + client, + sample_letter_notification, + mocker, +): + mock_get_letter_pdf = mocker.patch('app.v2.notifications.get_notifications.get_letter_pdf', return_value=b'foo') + sample_letter_notification.status = 'created' + + auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) + response = client.get( + path=url_for('v2_notifications.get_pdf_for_notification', notification_id=sample_letter_notification.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response.status_code == 200 + assert response.get_data() == b'foo' + mock_get_letter_pdf.assert_called_once_with(sample_letter_notification) + + +def test_get_pdf_for_notification_returns_400_if_pdf_not_found( + client, + sample_letter_notification, + mocker, +): + # if no files are returned get_letter_pdf throws StopIteration as the iterator runs out + mock_get_letter_pdf = mocker.patch( + 'app.v2.notifications.get_notifications.get_letter_pdf', + side_effect=StopIteration + ) + sample_letter_notification.status = 'created' + + auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) + response = client.get( + path=url_for('v2_notifications.get_pdf_for_notification', notification_id=sample_letter_notification.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response.status_code == 400 + assert response.json['errors'] == [{ + 'error': 'BadRequestError', + 'message': 'PDF not available for letter, try again later' + }] + mock_get_letter_pdf.assert_called_once_with(sample_letter_notification) + + +@pytest.mark.parametrize('status', [ + 'pending-virus-check', + 'virus-scan-failed', + 'technical-failure', ]) -def test_get_notification_only_returns_pdf_content_if_right_status( +def test_get_pdf_for_notification_only_returns_pdf_content_if_right_status( client, sample_letter_notification, mocker, status, - expected_body, - mock_called ): mock_get_letter_pdf = mocker.patch('app.v2.notifications.get_notifications.get_letter_pdf', return_value=b'foo') sample_letter_notification.status = status auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) response = client.get( - path=url_for( - 'v2_notifications.get_notification_by_id', - notification_id=sample_letter_notification.id, - return_pdf_content='true' - ), + path=url_for('v2_notifications.get_pdf_for_notification', notification_id=sample_letter_notification.id), headers=[('Content-Type', 'application/json'), auth_header] ) - json_response = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert json_response['body'] == expected_body - assert mock_get_letter_pdf.called == mock_called + assert response.status_code == 400 + assert response.json['errors'] == [{ + 'error': 'BadRequestError', + 'message': 'PDF not available for letters in status {}'.format(status) + }] + assert mock_get_letter_pdf.called is False + + +def test_get_pdf_for_notification_fails_for_non_letters(client, sample_notification): + auth_header = create_authorization_header(service_id=sample_notification.service_id) + response = client.get( + path=url_for('v2_notifications.get_pdf_for_notification', notification_id=sample_notification.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response.status_code == 400 + assert response.json['errors'] == [{'error': 'BadRequestError', 'message': 'Notification is not a letter'}]