From 22aff482a88a2c4336b3260a55acb11b2e2bb712 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 3 Sep 2019 16:49:03 +0100 Subject: [PATCH 1/4] add flag to return pdf content via api this is only applicable when getting a single notification by id. it's also ignored if the notification isn't a letter. Otherwise, it overwrites the 'body' field in the response. If the notification's pdf is available, it returns that, base64 encoded. If the pdf is not available, it returns an empty string. The pdf won't be available if the notification's status is: * pending-virus-scan * virus-scan-failed * validation-failed * technical-failure The pdf will be retrieved from the correct s3 bucket based on its type --- app/models.py | 2 +- app/v2/notifications/get_notifications.py | 17 ++++++++- tests/app/letters/test_letter_utils.py | 10 ++++++ .../notifications/test_get_notifications.py | 35 +++++++++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/app/models.py b/app/models.py index 62c8c36c1..c98cce026 100644 --- a/app/models.py +++ b/app/models.py @@ -1547,7 +1547,7 @@ class Notification(db.Model): elif self.status in [NOTIFICATION_DELIVERED, NOTIFICATION_RETURNED_LETTER]: return NOTIFICATION_STATUS_LETTER_RECEIVED else: - # Currently can only be technical-failure + # Currently can only be technical-failure OR pending-virus-check OR validation-failed return self.status def get_created_by_name(self): diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index d54df58cb..00e2adff0 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -1,9 +1,14 @@ +import base64 + from flask import jsonify, request, url_for, current_app + 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.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 @v2_notification_blueprint.route("/", methods=['GET']) @@ -14,7 +19,17 @@ def get_notification_by_id(notification_id): 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') + + return jsonify(response), 200 @v2_notification_blueprint.route("", methods=['GET']) diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index 1d3c427c8..504403a40 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -80,6 +80,16 @@ def test_get_bucket_name_and_prefix_for_notification_precompiled_letter_using_te sample_precompiled_letter_notification_using_test_key.reference).upper() +@freeze_time(FROZEN_DATE_TIME) +def test_get_bucket_name_and_prefix_for_notification_templated_letter_using_test_key(sample_letter_notification): + sample_letter_notification.key_type = KEY_TYPE_TEST + + bucket, bucket_prefix = get_bucket_name_and_prefix_for_notification(sample_letter_notification) + + assert bucket == current_app.config['TEST_LETTERS_BUCKET_NAME'] + assert bucket_prefix == 'NOTIFY.{}'.format(sample_letter_notification.reference).upper() + + @freeze_time(FROZEN_DATE_TIME) def test_get_bucket_name_and_prefix_for_failed_validation(sample_precompiled_letter_notification): sample_precompiled_letter_notification.status = NOTIFICATION_VALIDATION_FAILED diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 8bbe13db5..699f0fb0b 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -649,3 +649,38 @@ def test_get_notifications_renames_letter_statuses(client, sample_letter_templat json_response = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 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_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' + ), + 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 From 52f76207720b2f384750758fdb44e8f142835909 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 9 Sep 2019 12:44:28 +0100 Subject: [PATCH 2/4] create pdfs for test templated letters previously, we didn't create templated letters, and just marked them as delivered straight away. However, we may need to return PDFs for these letters, so we should create them the same as live letters. Then update the functions so that they know where to look for these letters. --- app/celery/letters_pdf_tasks.py | 5 ++-- app/letters/utils.py | 5 ++-- app/v2/notifications/post_notifications.py | 30 ++++++++++--------- .../test_post_letter_notifications.py | 12 ++++---- 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 7d659c876..3a9cac2fa 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -67,8 +67,9 @@ def create_letters_pdf(self, notification_id): upload_letter_pdf(notification, pdf_data) - notification.billable_units = billable_units - dao_update_notification(notification) + if notification.key_type != KEY_TYPE_TEST: + notification.billable_units = billable_units + dao_update_notification(notification) current_app.logger.info( 'Letter notification reference {reference}: billable units set to {billable_units}'.format( diff --git a/app/letters/utils.py b/app/letters/utils.py index e23542081..10e4310eb 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -50,11 +50,10 @@ def get_letter_pdf_filename(reference, crown, is_scan_letter=False, postage=SECO def get_bucket_name_and_prefix_for_notification(notification): - is_test_letter = notification.key_type == KEY_TYPE_TEST and notification.template.is_precompiled_letter folder = '' if notification.status == NOTIFICATION_VALIDATION_FAILED: bucket_name = current_app.config['INVALID_PDF_BUCKET_NAME'] - elif is_test_letter: + elif notification.key_type == KEY_TYPE_TEST: bucket_name = current_app.config['TEST_LETTERS_BUCKET_NAME'] else: bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] @@ -90,6 +89,8 @@ def upload_letter_pdf(notification, pdf_data, precompiled=False): if precompiled: bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME'] + elif notification.key_type == KEY_TYPE_TEST: + bucket_name = current_app.config['TEST_LETTERS_BUCKET_NAME'] else: bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 3d06c16d7..90801cb8a 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -259,10 +259,11 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text template=template, reply_to_text=reply_to_text) - should_send = not (api_key.key_type == KEY_TYPE_TEST) + test_key = api_key.key_type == KEY_TYPE_TEST # if we don't want to actually send the letter, then start it off in SENDING so we don't pick it up - status = NOTIFICATION_CREATED if should_send else NOTIFICATION_SENDING + status = NOTIFICATION_CREATED if not test_key else NOTIFICATION_SENDING + queue = QueueNames.CREATE_LETTERS_PDF if not test_key else QueueNames.RESEARCH_MODE notification = create_letter_notification(letter_data=letter_data, template=template, @@ -270,18 +271,19 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text status=status, reply_to_text=reply_to_text) - if should_send: - create_letters_pdf.apply_async( - [str(notification.id)], - queue=QueueNames.CREATE_LETTERS_PDF - ) - elif current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']: - create_fake_letter_response_file.apply_async( - (notification.reference,), - queue=QueueNames.RESEARCH_MODE - ) - else: - update_notification_status_by_reference(notification.reference, NOTIFICATION_DELIVERED) + create_letters_pdf.apply_async( + [str(notification.id)], + queue=queue + ) + + if test_key: + if current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']: + create_fake_letter_response_file.apply_async( + (notification.reference,), + queue=queue + ) + else: + update_notification_status_by_reference(notification.reference, NOTIFICATION_DELIVERED) return notification diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index f950135af..f891314f6 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -122,7 +122,7 @@ def test_post_letter_notification_sets_postage( 'staging', 'live', ]) -def test_post_letter_notification_with_test_key_set_status_to_delivered( +def test_post_letter_notification_with_test_key_creates_pdf_and_sets_status_to_delivered( notify_api, client, sample_letter_template, mocker, env): data = { @@ -148,7 +148,7 @@ def test_post_letter_notification_with_test_key_set_status_to_delivered( notification = Notification.query.one() - assert not fake_create_letter_task.called + fake_create_letter_task.assert_called_once_with([str(notification.id)], queue='research-mode-tasks') assert not fake_create_dvla_response_task.called assert notification.status == NOTIFICATION_DELIVERED @@ -157,7 +157,7 @@ def test_post_letter_notification_with_test_key_set_status_to_delivered( 'development', 'preview', ]) -def test_post_letter_notification_with_test_key_sets_status_to_sending_and_sends_fake_response_file( +def test_post_letter_notification_with_test_key_creates_pdf_and_sets_status_to_sending_and_sends_fake_response_file( notify_api, client, sample_letter_template, mocker, env): data = { @@ -183,7 +183,7 @@ def test_post_letter_notification_with_test_key_sets_status_to_sending_and_sends notification = Notification.query.one() - assert not fake_create_letter_task.called + fake_create_letter_task.assert_called_once_with([str(notification.id)], queue='research-mode-tasks') assert fake_create_dvla_response_task.called assert notification.status == NOTIFICATION_SENDING @@ -357,7 +357,7 @@ def test_post_letter_notification_doesnt_send_in_trial(client, sample_trial_lett {'error': 'BadRequestError', 'message': 'Cannot send letters when service is in trial mode'}] -def test_post_letter_notification_is_delivered_if_in_trial_mode_and_using_test_key( +def test_post_letter_notification_is_delivered_but_still_creates_pdf_if_in_trial_mode_and_using_test_key( client, sample_trial_letter_template, mocker @@ -373,7 +373,7 @@ def test_post_letter_notification_is_delivered_if_in_trial_mode_and_using_test_k notification = Notification.query.one() assert notification.status == NOTIFICATION_DELIVERED - assert not fake_create_letter_task.called + fake_create_letter_task.assert_called_once_with([str(notification.id)], queue='research-mode-tasks') def test_post_letter_notification_is_delivered_and_has_pdf_uploaded_to_test_letters_bucket_using_test_key( From 1ad32c91683934bb8383956ff88ccdcd287803a1 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 12 Sep 2019 17:06:22 +0100 Subject: [PATCH 3/4] move get_pdf functionality to its own endpoint it felt very awkward when the body of a pdf might be empty, might have things in it, and whether it is empty or not can change even when the status is the same (a created template notification might have a pdf, but might not, we don't know). So move it to its own endpoint, so we can hand craft some 400 errors that appropriately explain what's going on. --- app/v2/notifications/get_notifications.py | 55 +++++++++--- .../notifications/test_get_notifications.py | 87 +++++++++++++++---- 2 files changed, 111 insertions(+), 31 deletions(-) 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'}] From efaa4f2ad25955a1db0e2f8c4315f6a733a3faae Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 17 Sep 2019 12:16:24 +0100 Subject: [PATCH 4/4] karlify the exception messages also create a PDFNotReadyError class, separate to BadRequestError, to imply to the end user that this is something they should handle separately to all the other errors --- app/v2/errors.py | 5 ++++ app/v2/notifications/get_notifications.py | 30 +++++++------------ .../notifications/test_get_notifications.py | 14 ++++----- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/app/v2/errors.py b/app/v2/errors.py index a87b0f58e..51262df4a 100644 --- a/app/v2/errors.py +++ b/app/v2/errors.py @@ -57,6 +57,11 @@ class BadRequestError(InvalidRequest): self.message = message if message else self.message +class PDFNotReadyError(BadRequestError): + def __init__(self): + super().__init__(message='PDF not available yet, try again later', status_code=400) + + def register_errors(blueprint): @blueprint.errorhandler(InvalidEmailError) def invalid_format(error): diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index 12156e7f9..e5d267024 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -6,7 +6,7 @@ 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.errors import BadRequestError, PDFNotReadyError 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 ( @@ -36,29 +36,21 @@ def get_pdf_for_notification(notification_id): ) if notification.notification_type != LETTER_TYPE: - raise BadRequestError(message="Notification is not a letter", status_code=400) + raise BadRequestError(message="Notification is not a letter") - 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 - ) + if notification.status == NOTIFICATION_VIRUS_SCAN_FAILED: + raise BadRequestError(message='Document did not pass the virus scan') + + if notification.status == NOTIFICATION_TECHNICAL_FAILURE: + raise BadRequestError(message='PDF not available for letters in status {}'.format(notification.status)) + + if notification.status == NOTIFICATION_PENDING_VIRUS_CHECK: + raise PDFNotReadyError() 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) + raise PDFNotReadyError() return send_file(filename_or_fp=BytesIO(pdf_data), mimetype='application/pdf') diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index d2b126493..d38d95460 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -690,22 +690,22 @@ def test_get_pdf_for_notification_returns_400_if_pdf_not_found( assert response.status_code == 400 assert response.json['errors'] == [{ - 'error': 'BadRequestError', - 'message': 'PDF not available for letter, try again later' + 'error': 'PDFNotReadyError', + 'message': 'PDF not available yet, 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', +@pytest.mark.parametrize('status, expected_message', [ + ('virus-scan-failed', 'Document did not pass the virus scan'), + ('technical-failure', 'PDF not available for letters in status technical-failure'), ]) def test_get_pdf_for_notification_only_returns_pdf_content_if_right_status( client, sample_letter_notification, mocker, status, + expected_message ): mock_get_letter_pdf = mocker.patch('app.v2.notifications.get_notifications.get_letter_pdf', return_value=b'foo') sample_letter_notification.status = status @@ -719,7 +719,7 @@ def test_get_pdf_for_notification_only_returns_pdf_content_if_right_status( assert response.status_code == 400 assert response.json['errors'] == [{ 'error': 'BadRequestError', - 'message': 'PDF not available for letters in status {}'.format(status) + 'message': expected_message }] assert mock_get_letter_pdf.called is False