Merge pull request #2610 from alphagov/get-pdf-contents-via-api

add api endpoint to get pdf for letter

app/celery/letters_pdf_tasks.py

@@ -66,8 +66,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(

app/letters/utils.py

@@ -53,11 +53,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']
@@ -95,6 +94,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']
 

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

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

app/v2/notifications/get_notifications.py

@@ -1,9 +1,20 @@
-from flask import jsonify, request, url_for, current_app
+from io import BytesIO
+
+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, 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 (
+    NOTIFICATION_PENDING_VIRUS_CHECK,
+    NOTIFICATION_VIRUS_SCAN_FAILED,
+    NOTIFICATION_TECHNICAL_FAILURE,
+    LETTER_TYPE,
+)
 
 
 @v2_notification_blueprint.route("/<notification_id>", methods=['GET'])
@@ -13,10 +24,37 @@ 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
 
 
+@v2_notification_blueprint.route('/<notification_id>/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
+    )
+
+    if notification.notification_type != LETTER_TYPE:
+        raise BadRequestError(message="Notification is not a letter")
+
+    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:
+        raise PDFNotReadyError()
+
+    return send_file(filename_or_fp=BytesIO(pdf_data), mimetype='application/pdf')
+
+
 @v2_notification_blueprint.route("", methods=['GET'])
 def get_notifications():
     _data = request.args.to_dict(flat=False)

app/v2/notifications/post_notifications.py

@@ -256,10 +256,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,
@@ -267,18 +268,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
 

tests/app/letters/test_letter_utils.py

@@ -94,6 +94,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

tests/app/v2/notifications/test_get_notifications.py

@@ -649,3 +649,87 @@ 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
+
+
+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': 'PDFNotReadyError',
+        'message': 'PDF not available yet, try again later'
+    }]
+    mock_get_letter_pdf.assert_called_once_with(sample_letter_notification)
+
+
+@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
+
+    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': expected_message
+    }]
+    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'}]

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(