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
This commit is contained in:
Leo Hemsted
2019-09-17 12:16:24 +01:00
parent 1ad32c9168
commit efaa4f2ad2
3 changed files with 23 additions and 26 deletions

View File

@@ -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):

View File

@@ -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')

View File

@@ -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