Refactor to handle API errors for preview

app/main/views/notifications.py

@@ -13,8 +13,8 @@ from flask import (
     url_for,
 )
 from flask_login import login_required
-from notifications_python_client.errors import APIError
 
+from notifications_python_client.errors import APIError
 from app import (
     current_service,
     format_date_numeric,
@@ -57,7 +57,6 @@ def view_notification(service_id, notification_id):
         show_recipient=True,
         redact_missing_personalisation=True,
     )
-
     template.values = get_all_personalisation_from_notification(notification)
     if notification['job']:
         job = job_api_client.get_job(service_id, notification['job']['id'])['data']
@@ -87,6 +86,12 @@ def view_notification(service_id, notification_id):
     )
 
 
+def get_preview_error_image():
+    path = os.path.join(os.path.dirname(__file__), "..", "..", "assets", "images", "preview_error.png")
+    with open(path, "rb") as file:
+        return file.read()
+
+
 @main.route("/services/<service_id>/notification/<uuid:notification_id>.<filetype>")
 @login_required
 @user_has_permissions('view_activity', admin_override=True)
@@ -95,22 +100,6 @@ def view_letter_notification_as_preview(service_id, notification_id, filetype):
     if filetype not in ('pdf', 'png'):
         abort(404)
 
-    # notification = notification_api_client.get_notification(service_id, notification_id)
-    # notification['template'].update({'reply_to_text': notification['reply_to_text']})
-
-    # template = get_template(
-    #     notification['template'],
-    #     current_service,
-    #     letter_preview_url=url_for(
-    #         '.view_letter_notification_as_preview',
-    #         service_id=service_id,
-    #         notification_id=notification_id,
-    #         filetype='png',
-    #     ),
-    # )
-
-    # template.values = notification['personalisation']
-
     try:
         preview = notification_api_client.get_notification_letter_preview(
             service_id,
@@ -121,8 +110,7 @@ def view_letter_notification_as_preview(service_id, notification_id, filetype):
 
         display_file = base64.b64decode(preview['content'])
     except APIError:
-        with open(os.path.join(os.path.dirname(__file__), "../../assets/images/preview_error.png"), "rb") as file:
+        display_file = get_preview_error_image()
-            display_file = file.read()
 
     return display_file
 

app/templates/views/notifications/notification.html

@@ -35,8 +35,8 @@
         Estimated delivery date: {{ estimated_letter_delivery_date|string|format_date_short }}
       </p>
       <p class="bottom-gutter">
-          <a href="{{ url_for('main.view_letter_notification_as_preview', service_id=current_service.id, notification_id=notification_id, filetype='pdf') }}" download>Download as a PDF</a>
+        <a href="{{ url_for('main.view_letter_notification_as_preview', service_id=current_service.id, notification_id=notification_id, filetype='pdf') }}" download>Download as a PDF</a>
-          </p>
+      </p>
     {% endif %}
 
     {{ template|string }}

tests/app/main/views/test_notifications.py

@@ -4,12 +4,11 @@ import pytest
 from flask import url_for
 from freezegun import freeze_time
 from functools import partial
-from notifications_utils.template import LetterImageTemplate
+import pytest
-from tests.conftest import (
+from unittest.mock import mock_open
-    SERVICE_ONE_ID,
+
-    mock_get_notification,
+from notifications_python_client.errors import APIError
-    normalize_spaces,
+from tests.conftest import mock_get_notification, SERVICE_ONE_ID, normalize_spaces
-)
 
 
 @pytest.mark.parametrize('notification_status, expected_status', [
@@ -164,7 +163,7 @@ def test_should_show_image_of_letter_notification(
 
     mock_get_notification(mocker, fake_uuid, template_type='letter')
 
-    mocked_api_client = mocker.patch(
+    mocker.patch(
         'app.notify_client.notification_api_client.NotificationApiClient.get',
         return_value={
             'content': base64.b64encode(b'foo').decode('utf-8')
@@ -182,6 +181,32 @@ def test_should_show_image_of_letter_notification(
     assert response.get_data(as_text=True) == 'foo'
 
 
+def test_should_show_preview_error_image_letter_notification_on_preview_error(
+    logged_in_client,
+    fake_uuid,
+    mocker,
+):
+
+    mock_get_notification(mocker, fake_uuid, template_type='letter')
+
+    mocker.patch(
+        'app.notify_client.notification_api_client.NotificationApiClient.get',
+        side_effect=APIError
+    )
+
+    mocker.patch("builtins.open", mock_open(read_data="preview error image"))
+
+    response = logged_in_client.get(url_for(
+        'main.view_letter_notification_as_preview',
+        service_id=SERVICE_ONE_ID,
+        notification_id=fake_uuid,
+        filetype='png'
+    ))
+
+    assert response.status_code == 200
+    assert response.get_data(as_text=True) == 'preview error image'
+
+
 def test_should_404_for_unknown_extension(
     client_request,
     fake_uuid,